Skip to content

fix(kernel): route the last abort linker through the one cascade - #952

Merged
drewstone merged 1 commit into
mainfrom
fix/last-abort-linker
Aug 21, 2026
Merged

fix(kernel): route the last abort linker through the one cascade#952
drewstone merged 1 commit into
mainfrom
fix/last-abort-linker

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Problem

#942 collapsed the supervision tree's abort cascade to one owner and reported "6 implementations → 1". The real count was 7. This copy survived because it is named linkSignals and lives in supervise/worktree-cli-executor.ts — a third name, in a file the search for mergeAbortSignals and abortIfSignalled never covered. I found it re-measuring after the merge, not before.

It carries the same defect #942 fixed: c.abort() with no argument on both branches, so a cascaded death through the worktree CLI executor still reaches the worker's down record naming no cause — the exact behaviour abort-reason.test.ts forbids.

Its docstring also claims it "returns undefined when neither is present"; the body has no path that returns undefined.

Change

The one call site uses linkAbort from supervise/abortable.ts. The copy and its stale docstring are deleted.

Why long-term right

Same reason as #942 — one cascade, one reason rule — and this PR is the evidence for it: a primitive with three names in four files cannot be found by searching for any one of them. With the last copy gone, grep -rn 'function linkSignals\|function mergeAbortSignals' src/ returns nothing.

Cost

1 file, net −16 lines. No public API change (check:version-bump: consumer surface unchanged).

Proof

  • pnpm run typecheck, pnpm run lint (613 files), pnpm run docs:check — clean.
  • tests/runtime/worktree-cli-executor.test.ts, tests/mcp/worktree-harness.test.ts, plus both abort suites — 52 passed.

Simplification

Simplification: the seventh and last hand-written abort cascade routed through the one owner; a docstring describing a return the function never makes, deleted with it.
Net: +2 / -18 lines, 1 file, 1 copy removed.
Not done here: none — this closes the primitive. abortError still has three factories and awaitAbortable two implementations, which #942 already named as its follow-up boundary and which remain out of scope for the same reason: collapsing them changes the message throwIfAborted produces at 135 call sites.
Tests: +0. The contract is already pinned on linkAbort by abort-reason.test.ts (re-pointed at production in #942); this PR moves a call site onto that owner and introduces no behaviour of its own to pin.

#942 collapsed the abort cascade to one owner and missed this copy: it is
named linkSignals, in a file the search for mergeAbortSignals and
abortIfSignalled never covered. It carries the same defect — c.abort() with no
argument on both branches — so a cascaded death through the worktree CLI
executor still reached the worker's down record naming no cause.

Its docstring also claimed to return undefined when neither signal is present,
which the body never did.
@drewstone
drewstone force-pushed the fix/last-abort-linker branch from 2c3d4ab to 15b4e92 Compare August 21, 2026 10:50

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 15b4e92f

This PR was opened by the trusted drewstone account.

This approval is provisional and was applied by the local stand-in because the pr-reviewer webhook host is unreachable (2026-08-21). CI on this head is fully green. The full PR reviewer audit re-runs via the resweep when the service returns and will publish findings if it detects issues.

@drewstone
drewstone merged commit 1f11e47 into main Aug 21, 2026
4 checks passed
@drewstone
drewstone deleted the fix/last-abort-linker branch August 21, 2026 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants