Harden Windows per-loop terminal layout cleanup (kill safety, restore retry, sweep, data safety)

[coneilen]

Summary

Hardens the per-loop terminal layout cleanup that #674 added, after independent reviews found it could end sessions it did not own and could lose or skip saved tabs. Windows shell only (graphcode-windows/src); nothing under macOS or GraphcodeKit changes.

Every review finding was checked against the code first and was real: retireLoop killed every agent:false pane with no validation and no listing; a failed zmx ls dropped shell tabs without any retry and scopeToLoop returned early for the open loop; the first-snapshot sweep acted on a single, possibly empty or partial snapshot and was only tested by calling endDeletedLoops directly; and a corrupt or unknown-schema layout was overwritten by the next save.

Changes

• Kill safety. The shell ends a zmx session only if all of these hold: (1) its name has the shape graphcode- plus a lowercase 8-4-4-4-12 uuid, so agent sessions (uppercase uuid), foreign prefixes and unprefixed names are excluded; (2) a GraphCode shell recorded it in the shared support root for that loop: a marker file named for the session under shell-sessions beside the layouts, whose content is the loop it was minted for, and which only that loop's retirement (or closing that loop's pane) accepts. New Tab and Split save the layout that claims the new session first, then write the record, then start the session, and, if a step fails, undo the in-memory claim, save the layout again without it and remove the record (the compensating save is best effort; see the limits below). The record is removed once the kill exits 0 or the session is found already gone; (3) it is not any graph node's id (case-insensitive, including ids nested in a composite's stored subgraph) and not the open loop's id; (4) neither the open layout nor any other saved layout names it, and the scan of the other layouts completed (a scan cut short by the 1024-file cap, an unreadable directory or file, or a file that is not a layout is "unknown" and refuses; unknown is retryable, so the layout is kept as it was for the next sweep rather than set aside); (5) a fresh successful zmx ls shows it running at kill time. Anything else is refused and logged, never killed; a layout that named a permanently refused entry is renamed .refused, not deleted. Duplicate names are ended once. The same gate covers closing a tab or split. A harness's GRAPHCODE_SHELL_SESSION_PREFIX ids have the shape under that exact prefix only.
• List failures. A saved shell tab whose restore could not list sessions is queued for a list-only retry (backoff from 1s up to 8s, 0.5s probe, 5 attempts). It is attached only after a later successful list shows its session running, and dropped from the layout only after a successful list shows it gone. Reopening the already-open loop retries the restore with a fresh budget instead of returning early. After the budget the tab stays saved.
• Deleted-loop sweep. An empty snapshot proves nothing and never sweeps. A loop must be missing from two successive snapshots of its project before its sessions are ended. On a project's first snapshot only layouts older than this shell are considered. Project paths match exactly, so an alias spelling (C:/ against C:\) is not swept. Loop ids match case-insensitively. If the listing fails, the layout is kept for the next sweep.
• Data safety. A corrupt, foreign-project or unknown-schema layout is renamed .bad (never over an earlier one) and ends nothing. If it cannot be set aside (for example every .bad name is taken), the load fails and nothing is written over it: opening that loop reports "Unable to open selected loop", a project-level rebind fails, and at startup persistence stays off. A layout that is merely unreadable (locked) is left alone and the open fails rather than overwriting it. Layout saves use a per-writer temporary name, sync before the rename, and delete the temporary file on failure. Legacy adoption no longer proceeds if its marker file cannot be written.

What is and is not proven

The ownership record is a file some GraphCode shell wrote; it is not cryptographic provenance and it does not identify which shell wrote it. The layout directory is shared by every GraphCode shell of the user: the workspace reservation (WorkspaceReservation, App.zig) is per workspace folder, not per support directory, so two shells of different workspaces can run against the same layout root at once. Because of that, a record names the loop it was minted for and only that loop's retirement can use it, so shell B retiring a stale layout of a different loop cannot end a session shell A has just recorded for its own loop. The remaining limits: a hand-created record for a foreign session, with the right loop name, in the layout root would still make that session killable (after passing every other check); and two shells operating on the same loop's layout at once are not coordinated beyond the claim-before-start order.

Upgrade effect: sessions from tabs created by builds before this change have no record, so deleting their loop or closing their tab will not end them. They are left running (a leak) rather than guessed at.

What each test proves

New or changed tests (App root unless noted; they use the fake zmx.cmd fixture, real winghostty child surfaces and real child processes):

• a live session that only a layout names is never ended without proof the shell made it: a live, lowercase-uuid, non-node session named by the retiring layout and no other layout, with no record, is not killed; a recorded one beside it is. RED before the fix.
• a tab's session is recorded as the shell's own when minted and forgotten once ended: New Tab and Split write the record; closing the tab kills both and removes both records. RED in the previous push (no record existed).
• a session another loop's record names is never ended by a different loop's retirement: the window of a second shell sharing the layout directory, simulated with one Workspace object and a hand-written record for another loop. RED before the fix (killed).
• a new tab's layout claim and ownership record exist before its session starts: the fake zmx attach records what it found when it started (the loop layout file and the record), and the record names the loop. This test passed before the reordering fix (the pre-fix order raced too fast for the fake attach to observe), so the claim-before-start change itself has no RED run; only the record-names-the-loop part failed before.
• an unreadable unrelated layout defers the cleanup, and the next sweep ends the shell: a malformed unrelated layout blocks the cleanup once, leaving the layout and record in place; after it is removed the next retire kills the session. RED before the fix (layout was set aside).
• a layout scan cut short by its cap refuses the kill: with a claimant sorted beyond 1024 layout files, the kill is refused. RED before the fix.
• a corrupt layout that cannot be set aside is left as it is and its loop is not opened: with every .bad name taken, the corrupt layout's bytes and the earlier .bad file are unchanged, the loop is not opened and the status says so. RED before the fix.
• deleting a loop ends only recorded, live, shell-shaped sessions nothing else names: with an uppercase agent id, a foreign prefix, an unprefixed name, a session another loop's layout names, a duplicate spelling, a dead session and one good session all recorded as plain panes, the recorded zmx kill argv is exactly one session. RED in the first push (before any production change); it now also needs the record.
• a loop's shells are ended only after a successful session listing: a failing and a hanging zmx ls end nothing and keep the layout; a later good listing ends the session. RED in the first push.
• a saved shell tab is restored by retry after a failed session listing and reopening the open loop restores a shell tab that a failed listing left out. RED in the first push (the tab was never attached).
• a project's first snapshot ... once a second snapshot agrees, an empty or partial first snapshot sweeps nothing ..., a layout saved after this shell started is another shell's and is never swept, a loop deleted after the shell has seen the project is ended on the next snapshot that agrees, a shell session whose uuid is a graph node's id is never ended: all go through the real onFrameWithAccessibilityPublish path. RED in the first push.
• a corrupt, foreign or newer-schema loop layout is kept as .bad and ends nothing. RED in the first push.
• closing a tab never ends a session that another loop's layout also names: not run against the old code, so no RED run.
• ZmxSession.zig (2 tests: name shape and harness prefix) and WorkspaceLayout.zig (6 tests: set-aside naming, load-error classification, claim scan including the cap and unreadable files, ownership records, atomic save): unit tests of new functions, so their only "RED" would be a compile error; none was run.

Not tested, and limits

• No Dev Box, no real zmx, no real daemon: zmx is a .cmd stand-in and daemon snapshots are hand-built frames. Behavior against a real zmx ls format, a real daemon restart, or hardware was not exercised.
• The wire has no completeness marker for a snapshot, so "complete" is approximated by non-empty plus two agreeing snapshots plus layout age. A daemon that sends the same partial snapshot twice would still sweep.
• Deleting a loop now ends its shells on the second snapshot, not the first. If no further snapshot arrives before the shell exits, the next run's first-snapshot sweep handles it. A failed listing at kill time is not retried in-process, only at the next first-snapshot sweep.
• The retire and close paths run one synchronous zmx ls on the UI thread (bounded at 2s). A wedged zmx can stall it for that long.
• A failed rollback save is not handled or tested: if starting a new tab's or split's session fails and the compensating layout save also fails, the layout on disk can keep a pane for a session that never started (a ghost pane). Its record has been removed, so it cannot authorize a kill, but the pane is only dropped by a later successful restore listing.
• If a tab's ownership record cannot be written the tab still works but its session will never be ended by the shell.
• No keyboard, accessibility, glyph, or visual behavior was touched or checked; the parity ledger is not changed.
• make test and make check are macOS targets and were not run. zig fmt --check fails on these files before and after this change, so it was not used.

Test plan

RED: zig test src\App.zig (App shell flags) --test-filter "workspace layout" with the latest review-round tests against the previous push's production code -> 23 passed, 3 failed (a session another loop's record names was killed: expected 0, found 1; record content empty where the loop name was expected; unrelated unreadable layout did not defer)
GREEN: zig test src\App.zig (App shell flags) --test-filter "workspace layout" with the fix -> All 26 tests passed
REGRESSION: zig test for App 1077/1077, TerminalSurface 249/249, WorkspaceLayout 17/17, ZmxSession 5/5, LoopLaunchWait 15/15 -> all passed

Earlier rounds' RED runs: the first push's (8 passed, 10 failed: kill argv expected 1, found 7; an empty snapshot killed both sessions; shell tab restore timed out; .bad file not found) was against the unfixed production code of the original review; the second round's 19 passed, 4 failed covered the missing ownership record, the scan cap and the set-aside overwrite. All runs used the pinned Zig 0.15.2 from Tools\windows\bootstrap.ps1 and a process-only scratch profile (USERPROFILE, GRAPHCODE_SUPPORT_DIR, LOCALAPPDATA, APPDATA, TEMP, TMP). The unit-test groups above are GREEN-only. Tools\windows\Tests\WindowsShell.Tests.ps1 was not run end to end; the Zig roots were run directly with the same flags it uses.

Checklist

• I have read the Contributing Guidelines
• I have signed off my commits (git commit -s) per the DCO
• Tests pass locally (make test) (macOS target; the Windows Zig roots above were run instead)
• Code follows the existing style (make check) (macOS target; not run)
• I added the test/contract before the implementation and observed the intended RED failure (for the behavioral tests listed above; not for the closing-a-shared-tab test or the unit-test groups)