feat(server): session-owned pinned geometry (run --size, pty resize) - #108
Closed
schickling-assistant wants to merge 2 commits into
Closed
Conversation
Restores the session-role half of the geometry work proposed in compoundingtech#92, which compoundingtech#96 narrowed out when it landed only the client-role half (`attach --no-resize`). The two are complementary, not redundant: a neutral CLIENT opts out of size negotiation, but only a pinned SESSION revokes the vote from every client — including a plain attacher that never opted in. - `pty run --size <cols>x<rows>` pins geometry at spawn. - `pty resize <ref> <cols>x<rows>` sets the authoritative size on a running session and pins it, emitting exactly one SIGWINCH (no redraw nudge). - `RESIZE_FLAG_AUTHORITATIVE` / `encodeResizeAuthoritative` reuse the same optional byte-4 flag slot as ATTACH, so legacy 4-byte frames stay byte-identical and old daemons keep parsing them. - `negotiateSize()` is inert while a session is pinned. - Stats gains a `geometry: { owner, pinned }` block plus a `capabilities.pinnedGeometry` flag. Main's existing surface is preserved as-is: `capabilities.geometryNeutralAttach`, `clients.geometryNeutral`, `decodeAttachFlags` and the `--no-resize` spelling are unchanged, and no `--neutral` alias is added. Also fixes a pre-existing drift failure in tests/help.test.ts: the `completions` dispatch case merged in compoundingtech#106 was never added to the help coverage lists, so the no-drift test was already red on main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T1xLDkqUDCYUMMADdMqVgP
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T1xLDkqUDCYUMMADdMqVgP
Contributor
Author
|
Closing unmerged: the consumer need this PR was written to serve turned out not to exist.
The only remaining consumer of Reopening this would need concrete evidence that headless sessions rendering at the daemon |
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.
Restores the session-owned geometry half of #92, which #96 narrowed out when it landed only the client-role half. Problem statement and evidence: #95.
#96 shipped
attach --no-resize,ATTACH_FLAG_GEOMETRY_NEUTRAL,capabilities.geometryNeutralAttachandclients.geometryNeutral. Main still has norun --size, no pinned session geometry, nopty resize, noRESIZE_FLAG_AUTHORITATIVE, and nogeometryblock in stats. This PR adds those, ported onto current main rather than cherry-picked (main moved 34+ commits and the surrounding code changed).Why this is not redundant with #96
1. Neutral attach does not protect a running TUI.
--no-resizelets a client opt out of size negotiation. It does nothing about a plain, non-neutral attacher, which still drags the session to its own dimensions via min-wins negotiation. Only a pinned session revokes the vote from every client. The two are complementary: neutral attach is opt-in on the observer, pinned geometry is enforced by the session.2.
--sizeis load-bearing for an existing downstream consumer. A consumer invokespty run -d -a --size 160x48 --id <name>. On main--sizeis unknown, and therunflag loop (src/cli.ts) ends inelse break— so parsing halts at--sizeand--idis never parsed. The session silently gets a random id instead of its intended name, and session identity is load-bearing there. This is a silent failure, not an error. There is now a regression test for exactly this ordering.3.
fractal(a TUI agent-IDE, shipped) is fully degraded without it. It gates its non-disturbing attach on a top-levelgeometryblock in stats. Main never emits one, so every session probes as incapable and fractal silently falls back to read-only attach — total feature loss even though main's neutral attach works.4. A fixed render size prevents the 80x24 TUI-freeze class of bug for headless / no-tty agent sessions.
What's in this PR
pty run --size <cols>x<rows>— pins the session's geometry at spawn.pty resize <ref> <cols>x<rows>— authoritative resize on a running session; pins it and emits exactly one SIGWINCH (no redraw nudge, since this resize is legitimate).RESIZE_FLAG_AUTHORITATIVE/encodeResizeAuthoritativeinprotocol.ts.negotiateSize()is inert while a session is pinned, so plain attachers cannot reflow it.geometry: { owner, pinned }and acapabilities.pinnedGeometryflag.src/completions.ts(SSOT) coversresizeandrun --size; committedcompletions/pty.{bash,fish,zsh}regenerated from it.tests/session-geometry.test.ts(12 tests).Compatibility with what #96 landed
Main's surface is added to, never reverted.
capabilities.geometryNeutralAttach,clients.geometryNeutral,decodeAttachFlagsand the--no-resizespelling are unchanged, and no--neutralalias is introduced (no consumer uses it).RESIZE_FLAG_AUTHORITATIVEreuses the same optional byte-4 flag slot as ATTACH, so a legacy 4-byte RESIZE frame stays byte-identical and old daemons keep parsing it. There is a test pinning that.Note on capability advertisement
Support is advertised both ways, on purpose:
capabilities.pinnedGeometry: trueis the clean contract and the recommended thing to probe.geometryblock is emitted unconditionally, becausefractalcurrently sniffs block presence. Keeping it means fractal works against this build with no change; the capability flag is the migration path.Verification
Run locally in a clean worktree:
npm run typecheck— clean.npm test— 1397 passed, 2 failed. Both failures (shells > zsh,screenshot > vim) reproduce identically on unmodified0a9be8cand are environmental (nozshon this machine); confirmed against a clean checkout of that commit.npm run verify-docs— 12 passed, 1 failed (doc example 12, a vim example); also reproduces identically on unmodified0a9be8c.nix build .#pty— succeeds.run --size(confirmed--idafter--sizeis honored), a plain 80x24 attach against a pinned 160x48 session (session stayed 160x48, client reported as non-neutral),pty resize,pty stats --json,pty completions fish.New tests cover:
run --sizeparses and pins (including the--id-after---sizeregression); a plain attach does not reflow a pinned session; a plain attach does still reflow an unpinned one (default behavior untouched);pty resizeemits exactly one WINCH; stats exposesgeometryfor both pinned and unpinned sessions; and legacy-frame byte compatibility.One note on the WINCH test: the baseline is not zero, because attaching a client triggers the existing redraw nudge (resize -1 col and back), which is itself two real WINCHes. The test measures the delta the authoritative resize adds, which is 1.
Deviation from scope, flagged for review
tests/help.test.ts's no-drift check was already red on main: thecompletionsdispatch case merged in #106 was never added to its coverage lists. Since this PR adds a new subcommand to that same list, I fixed the pre-existing failure in passing — which required capitalizingusage:toUsage:in thecompletionshelp synopsis to match the repo-wideUsage: pty ...contract, updating the assertion intests/completions.test.ts, and adding apty completionsline to the top-level help. Happy to split that into its own PR if you would rather keep this one narrow.No nix/packaging changes — this is a source-only PR.