Repository navigation
Check for a free terminal slot before New Tab and Split claim a session - #688
Merged
Merged
Conversation
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
force-pushed
the
coneilen-layout-rollback-ghost-pane
branch
from
October 10, 2026 16:56
cb4a2ba to
636179b
Compare
3 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 bycreateAttachedSurfaceFromand by a pre-check at the top ofnewTabandsplitFocused, which returnsSurfaceCapacityExceededbefore the layout is touched.TerminalSurface.zig: a test-onlylayout_save_faultseam inpersistLayout(number of saves that still succeed before one fails, once), in the style ofteardown_input_sinkandrestore_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 sessionzmx lsdoes not list as running.README.md: new "Per-loop terminal layouts: known limits" section.Known limits (unchanged by this PR, documented in the README)
zmx lsshows 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.saveis an atomic replace with no compare-and-swap, so a retry could overwrite a newer layout written by another shell, andscopeToLoop,retireLoopGuardedandrebindProjectswap the layout and its path.attachNodestarts the session before saving a claim and never records shell ownership. Its only production caller ofopenNodeis 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.zmx attachcreates 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
git commit -s) per the DCOmake test)make check)