test: re-green executor-workspace onto FN-6756's contract (+ flag a dead branch) (#2617)

**Test-only.** One file. No production changes.
`executor-workspace.test.ts`: **2 failed / 10 passed → 13 passed**.

(This commit was pushed earlier and I failed to open its PR — the work
was finished and sitting on a dangling branch, which is why
`executor-workspace` still shows in main's failure census.)

## Why it was red

Both cases asserted that `clearPhantomExecutorBinding` **succeeds**
while session-registry paths are held.

PR #2531 (FN-6756, P0: *"stop reaping worktrees out from under live
planners"*) inverted that. `hasLiveSessionSurface` now includes
`activeSessionRegistry.pathsForTask(taskId).length > 0`, and that guard
runs **before** both branches — so any registered path refuses the
clear. The FNXC note at `executor.ts:2729` says the kind-blind guard is
deliberate: *"A leaked entry now blocks THIS sweep rather than a live
planner losing its worktree — the strictly safer failure."*

So the old expectations describe the pre-#2531 contract. Rewritten to
the current one, which had **no direct coverage**: a refusal leaves
`activeWorktrees` and the registry entries untouched. The FN-6736 KTD2
invariant ("every held path, not one") is kept, exercised with no
registry paths so the guard permits it.

**Verified the new tests guard:** removing the registry term from
`hasLiveSessionSurface` fails 2 of them (`NEW-failures=2`), and only
them.

## Flagged, not fixed — a possible dead branch

The guard appears to make **both branches it precedes** unreachable for
their stated purpose:

- the default branch exists to unregister every held registry path
(FN-6736);
- `preserveWorktrees: true` exists to **keep** those paths so a
`moveTask(preserveWorktree: true)` re-dispatch reattaches to the same
worktree (FN-7249) — and its **only** production caller is the
self-healing reclaim at `self-healing.ts:3565`.

Both need registered paths to do anything, and the guard rejects exactly
that case. With none registered, one sweeps nothing and the other
preserves nothing.

The third new test pins this so the conflict is **executable rather than
prose**: `preserveWorktrees: true` returns `false` while the path it
exists to preserve is registered.

I did not "fix" it by rewriting the assertion to match production — that
would bury a possible regression in FN-7249's reattach path. Resolving
it (exempting the non-destructive `preserveWorktrees` path, or narrowing
the guard by kind) is a product decision for the FN-6756 owner.

## Main's engine-default census, measured just now

| Point | Failed | Files |
|---|---|---|
| when I started this sweep | 283 | 28 |
| after the logger-mock fix (#2573) | 106 | 23 |
| **now** | **65** | **12** |

This PR clears one of the remaining 12.
`executor-review-verdicts.test.ts` is newly red and in my lane — taking
that next.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-29 21:37:14 -07:00
committed by GitHub
parent d438cd1d13
commit 88df46bedb

View File

@@ -63,6 +63,20 @@ describeIfGit("workspace fixture", () => {
describeIfGit("U1 KTD2 — activeWorktrees Set + every enumerated consumer", () => {
let fx: WorkspaceFixture;
/*
FNXC:TestInfrastructure 2026-07-29-22:05 (#2617 review — greptile P2):
`activeSessionRegistry` is PROCESS-GLOBAL and is itself a liveness signal
(hasLiveSessionSurface reads it), so a registration leaked by a failing assertion
makes the NEXT test see a phantom live session and refuse a clear it should permit.
Unregistering after the assertions only works on the happy path. Sweep in
guaranteed teardown instead — this suite is precisely the one that measures that
registry, so it must not be the one polluting it.
*/
afterEach(() => {
for (const path of activeSessionRegistry.pathsForTask("FN-WS-1")) {
activeSessionRegistry.unregisterPath(path);
}
});
afterEach(() => fx?.cleanup());
function workspaceExecutor() {
@@ -146,7 +160,34 @@ describeIfGit("U1 KTD2 — activeWorktrees Set + every enumerated consumer", ()
expect(removeSpy).not.toHaveBeenCalled();
});
it("clearPhantomExecutorBinding (FN-6736) unregisters every held path, not one", async () => {
/*
FNXC:NodeWorktreeIsolation 2026-07-29-18:20 (U9 re-green; consequence of PR #2531):
These two cases used to assert that `clearPhantomExecutorBinding` SUCCEEDS while
session-registry paths are held, and swept (or preserved) them. FN-6756 inverted
that: `hasLiveSessionSurface` now includes
`activeSessionRegistry.pathsForTask(taskId).length > 0`, and that guard runs
BEFORE both branches — so any registered path refuses the clear outright. The old
expectations describe the pre-#2531 contract.
Rewritten to assert the CURRENT contract, which is the P0 fix's actual promise and
had no direct coverage: a registered surface of any kind means someone is working
in that worktree, so the reclaim refuses rather than reaping it.
⚠️ FLAG FOR THE #2531 OWNER — not fixed here, because it is a product decision:
the guard appears to make BOTH of the branches it precedes unreachable for their
stated purpose.
- the default branch exists to unregister every held registry path (FN-6736);
- `preserveWorktrees: true` exists to KEEP those registry paths so a
`moveTask(preserveWorktree:true)` re-dispatch reattaches to the same worktree
(FN-7249), and its ONLY production caller is the self-healing reclaim at
self-healing.ts:3565.
Both require registered paths to do anything, and the guard rejects exactly that
case. With no registered paths, one sweeps nothing and the other preserves
nothing. The third case below pins this so the conflict is executable rather than
prose. Resolving it — e.g. exempting the non-destructive `preserveWorktrees` path,
or narrowing the guard by kind — belongs to whoever owns FN-6756.
*/
it("clearPhantomExecutorBinding refuses while a session-registry path is held (FN-6756)", async () => {
fx = await createWorkspaceFixture();
const executor = workspaceExecutor();
const pA = repoAPath(fx);
@@ -156,34 +197,40 @@ describeIfGit("U1 KTD2 — activeWorktrees Set + every enumerated consumer", ()
activeSessionRegistry.registerPath(pA, { taskId: "FN-WS-1", kind: "executor", ownerKey: "exec:FN-WS-1:a" });
activeSessionRegistry.registerPath(pB, { taskId: "FN-WS-1", kind: "executor", ownerKey: "exec:FN-WS-1:b" });
const ok = (executor as any).clearPhantomExecutorBinding("FN-WS-1");
expect(ok).toBe(true);
expect((executor as any).clearPhantomExecutorBinding("FN-WS-1")).toBe(false);
// Nothing may be torn down on a refusal — that is the whole point of the guard.
expect((executor as any).activeWorktrees.has("FN-WS-1")).toBe(true);
expect(activeSessionRegistry.pathsForTask("FN-WS-1")).toEqual(expect.arrayContaining([pA, pB]));
// Cleanup is in afterEach, so a failure above cannot leak these registrations.
});
it("clearPhantomExecutorBinding (FN-6736) clears every held path once no session surface remains", async () => {
fx = await createWorkspaceFixture();
const executor = workspaceExecutor();
const pA = repoAPath(fx);
const pB = repoBPath(fx);
(executor as any).addActiveWorktree("FN-WS-1", pA);
(executor as any).addActiveWorktree("FN-WS-1", pB);
// No registry paths held, so the guard permits the clear. KTD2: a workspace task
// holds N worktrees and ALL of them must leave the binding, not just the first.
expect((executor as any).clearPhantomExecutorBinding("FN-WS-1")).toBe(true);
expect((executor as any).activeWorktrees.has("FN-WS-1")).toBe(false);
// Default path must sweep the session-registry entries (inverse of the preserveWorktrees branch).
expect((executor as any).executing.has("FN-WS-1")).toBe(false);
expect(activeSessionRegistry.pathsForTask("FN-WS-1")).toEqual([]);
});
it("clearPhantomExecutorBinding (FN-7249) preserveWorktrees keeps session-registry paths for re-dispatch", async () => {
it("FN-7249 preserveWorktrees cannot run while the paths it exists to preserve are registered", async () => {
fx = await createWorkspaceFixture();
const executor = workspaceExecutor();
const pA = repoAPath(fx);
const pB = repoBPath(fx);
(executor as any).addActiveWorktree("FN-WS-1", pA);
(executor as any).addActiveWorktree("FN-WS-1", pB);
activeSessionRegistry.registerPath(pA, { taskId: "FN-WS-1", kind: "executor", ownerKey: "exec:FN-WS-1:a" });
activeSessionRegistry.registerPath(pB, { taskId: "FN-WS-1", kind: "executor", ownerKey: "exec:FN-WS-1:b" });
const ok = (executor as any).clearPhantomExecutorBinding("FN-WS-1", { preserveWorktrees: true });
expect(ok).toBe(true);
// In-memory executor/lock bookkeeping is cleared so the scheduler can re-dispatch.
expect((executor as any).activeWorktrees.has("FN-WS-1")).toBe(false);
expect((executor as any).executing.has("FN-WS-1")).toBe(false);
// The held worktree session-registry entries are preserved so re-dispatch
// reattaches to the same worktree instead of acquiring a new one.
expect(activeSessionRegistry.pathsForTask("FN-WS-1")).toEqual(expect.arrayContaining([pA, pB]));
activeSessionRegistry.unregisterPath(pA);
activeSessionRegistry.unregisterPath(pB);
// The guard precedes the preserveWorktrees branch, so the one scenario that
// branch was written for is the one it can never reach.
expect((executor as any).clearPhantomExecutorBinding("FN-WS-1", { preserveWorktrees: true })).toBe(false);
expect((executor as any).activeWorktrees.has("FN-WS-1")).toBe(true);
});
});