Skip to content

Check for a free terminal slot before New Tab and Split claim a session - #688

Merged
coneilen merged 3 commits into
mainfrom
coneilen-layout-rollback-ghost-pane
Oct 10, 2026
Merged

coneilen merged 3 commits into
mainfrom
coneilen-layout-rollback-ghost-pane

Conversation

@coneilen

@coneilen coneilen commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

New Tab and Split now check for a free surface slot before they save the layout claim, so a full workspace fails without writing a claim or an ownership record. This removes the one deterministic way to reach the failed-rollback-save limit left by #682, and pins the remaining limits with tests and documentation. Restore, retire, and kill-safety behavior are unchanged.

Changes

  • TerminalSurface.zig: one free-slot predicate (surfaceSlotFree / firstFreeSurfaceSlot: no surface, no attach, no active launch wait) is used by createAttachedSurfaceFrom and by a pre-check at the top of newTab and splitFocused, which returns SurfaceCapacityExceeded before the layout is touched.
  • TerminalSurface.zig: a test-only layout_save_fault seam in persistLayout (number of saves that still succeed before one fails, once), in the style of teardown_input_sink and restore_retry_limit. It is null in production.
  • App.zig: three tests against the fake zmx. (1) No free slot: New Tab and Split fail, no save is attempted, the disk layout is byte-identical, no record is written, no attach starts. (2) A session start that fails after the claim, plus a failed rollback save: memory is rolled back, no record exists, the disk keeps the ghost pane, and retiring the loop from another loop kills nothing and sets the layout aside as .refused. (3) Restore of a loop layout drops a claimed pane whose session zmx ls does not list as running.
  • README.md: new "Per-loop terminal layouts: known limits" section.

Known limits (unchanged by this PR, documented in the README)

  • A failed rollback save is not handled. If the session start or the native surface fails after the claim is saved and the compensating save also fails, the disk keeps a pane that names either a session that was never created (the start failed) or one that survived the failure (the native surface failed after the attach was spawned). The record is gone, so it cannot authorize a kill. A later successful save clears it. A restore clears it only after a successful zmx ls shows the session not running; when the listing fails the pane stays saved and is retried, and a surviving session is re-attached as a tab with no record. Retiring such a loop sets its layout aside as .refused. A deferred retry of the save was deliberately not built: Layout.save is an atomic replace with no compare-and-swap, so a retry could overwrite a newer layout written by another shell, and scopeToLoop, retireLoopGuarded and rebindProject swap the layout and its path.
  • attachNode starts the session before saving a claim and never records shell ownership. Its only production caller of openNode is quick chat, whose session is named by a graph node and owned by the daemon, so refusing it at retire is intended. A stop between the start and the save leaves a live session with no claim.
  • A native surface failure after the attach was started may leave a zmx session. New Tab and Split roll back the claim and record, but the attach process is only killed and zmx attach creates the session on connect. Whether a session survives depends on zmx timing and is not established without a real zmx.

Not verified: any real zmx behavior (whether a killed early attach leaves a session), real filesystem sharing violations on the layout rename, winghostty surface-create failures, and any live walkthrough. The parity ledger is not changed.

Test plan

RED: zig test src\App.zig (shell test command from WindowsShell.Tests.ps1) --test-filter "workspace layout: " on the seam and tests commit alone -> 28 passed, 1 failed: the no-free-slot test saw the ghost pane in the disk layout (found tabs with an extra pane a9f4f042-1dd9-4e4b-6f38-803a147d703c, expected only loop-a), TestExpectedEqual at App.zig:15511; a behavioral failure, not a compile error. The two characterization tests passed on that same run, as expected.
GREEN: same zig test command with --test-filter "workspace layout: " after the fix -> All 29 tests passed.
REGRESSION: same zig test command for src\App.zig with no filter, Zig 0.15.2, pinned Winghostty 6286560 -> All 1080 tests passed.

Checklist

  • I have read the Contributing Guidelines
  • I have signed off my commits (git commit -s) per the DCO
  • Tests pass locally (make test)
  • Code follows the existing style (make check)
  • I added the test/contract before the implementation and observed the intended RED failure

coneilen and others added 3 commits October 10, 2026 09:52
The capacity test is RED until the slot pre-check lands in the next commit.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Colin Neilens <coneilen@microsoft.com>
createAttachedSurfaceFrom and a new pre-check share one free-slot predicate, so a
full workspace fails before any layout claim or ownership record is written
instead of relying on a compensating save that can itself fail. Documents the
remaining per-loop layout limits.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Colin Neilens <coneilen@microsoft.com>
…save

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Colin Neilens <coneilen@microsoft.com>
@coneilen
coneilen force-pushed the coneilen-layout-rollback-ghost-pane branch from cb4a2ba to 636179b Compare October 10, 2026 16:56
@coneilen
coneilen merged commit 8851343 into main Oct 10, 2026
24 checks passed
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.

1 participant