Skip to content

feat: add atomic machine attach v2 - #156

Closed
schickling-assistant wants to merge 10 commits into
mainfrom
schickling-assistant/2026-08-01-embedded-terminal-v2
Closed

feat: add atomic machine attach v2#156
schickling-assistant wants to merge 10 commits into
mainfrom
schickling-assistant/2026-08-01-embedded-terminal-v2

Conversation

@schickling-assistant

Copy link
Copy Markdown
Contributor

Summary

  • add one strictly framed machine-attach-v2 boundary with atomic same-socket admission
  • publish exact revisioned terminal output and child input-mode state
  • bound framing and per-client output so slow consumers cannot stall peers
  • remove the unreleased v1 descriptor path and its compatibility surface
  • add real adapter/daemon/process coverage for exact input bytes, typed outcomes, and stale generations

Verification

  • tsc -p tsconfig.json --noEmit
  • tsc -p tsconfig.build.json
  • 61 focused protocol/server/adapter/process tests
  • 63 focused CLI/help/completion/adapter/process tests after v1 removal
  • strict VRS validation

Draft while the Fractal consumer is exercised against the pinned build. Architecture discussion: #155.

@schickling
schickling marked this pull request as ready for review August 2, 2026 09:12
@schickling
schickling requested a review from myobie August 2, 2026 09:12
@schickling schickling linked an issue Aug 2, 2026 that may be closed by this pull request
@myobie

myobie commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Nathan reviewed this direction and accepts the product boundary. Please proceed.

The accepted boundary is:

  • machine-attach-v2 has no automatic fallback.
  • A new client rejects an old daemon with a clear error and does not restart or change the session.
  • An explicit old client is the debugging path for an old daemon.
  • Existing sessions restart only at deliberate safe points. This is not an all-at-once cutover.

Before merge, please add or link a short rollout and debugging note. It must tell an operator to record pty --version, resolve the installed package with realpath "$(command -v pty)", and preserve the exact built checkout before upgrade. The explicit debug command has this form:

PTY_ROOT=<original-pty-root> \
  ~/.local/share/pty/legacy/<old-commit>/bin/pty attach <session-id>

The package is currently private and is not published to the npm registry, so npx @compoundingtech/pty@<version> is not a recovery path.

Recommended rollout: preserve the old checkout on each host, install the new build, restart one low-value session, test local and remote attach/detach/resize/reconnect, and then restart other sessions only when safe. Keep the old client until no old daemon remains.

This comment approves the direction. It does not authorize a production rollout or automatic session restarts.

@myobie

myobie commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

I’m taking the requested independent evidence/docs pass on the accepted boundary, without rewriting or policing this shared branch. I’ll work from exact head ea7bc1063bfb16309de5e2717c548c70605839e4 and preserve an exact built pre-upgrade checkout. Scope: (A) exercise the PR client+daemon through attach, explicit detach, resize, reconnect, and verify a new client rejects an old daemon without restart or session mutation; (B) prove the preserved old checkout, invoked with explicit PTY_ROOT, still attaches to that old daemon. I’ll also propose the concise rollout/debug note Nathan requested. Any contribution will be append-only with an ordinary push after rechecking the remote head; if the branch advances, I’ll merge/converge normally rather than rewrite it. No fallback, bulk restart, deployment, or production recovery.

@myobie

myobie commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Scope correction from Nathan: Johannes owns PR #156 and may proceed with the accepted machine-attach-v2 direction. I will not add work, change this branch, create a successor, deploy, or restart sessions. My preceding note is superseded; I’ll provide read-only support only if Johannes asks. The earlier accepted-boundary comment remains the rollout/debug contract.

@schickling-assistant

Copy link
Copy Markdown
Contributor Author

We will not merge this PR.

The current PTY protocol remains the baseline. Fractal will first return to one current-protocol path. The long-term embedded terminal work moves to pty-rust.

We will preserve this PR as design and test evidence. Useful behavior cases will move to the shared corpus in compoundingtech/pty-rust#4.

Closing this PR as superseded. This does not change or restart any PTY session.

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.

Design question: intended input boundary for embedded PTY clients

2 participants