Skip to content

feat(acp): contain terminal workers and preserve final output - #166

Draft
tee8z wants to merge 1 commit into
tee8z/sandbox-attach-clifrom
tee8z/sandbox-terminal-worker-linear
Draft

tee8z wants to merge 1 commit into
tee8z/sandbox-attach-clifrom
tee8z/sandbox-terminal-worker-linear

Conversation

@tee8z

@tee8z tee8z commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

Behavior

Relay terminal access needs a PTY whose jobs stop when the attachment ends, including jobs that detach with setsid or double-fork. This layer adds a contained local worker inside buzz-acp. The relay authorization and audit consumer follows above it; this PR does not open a relay terminal by itself.

Replaces #165 while preserving its published history. The reviewed worker and final-output fix are copied unchanged onto terminal CLI #163, creating one linear foundation for the snapshot and relay consumers. No merge commit or shared-history rewrite was used.

Depends on #163, which depends on the protocol in #159 and gateway foundation #154.

sequenceDiagram
    participant Harness
    participant Worker
    participant Kernel
    Harness->>Worker: Workspace, bounded expiry and Open
    Worker->>Kernel: Verify subreaper and pidfd support
    Worker->>Kernel: Spawn PTY shell
    Worker-->>Harness: Ready and bounded data
    Harness->>Worker: Close, EOF or termination
    Worker->>Kernel: Stop, kill and reap owned descendants
    Worker-->>Harness: End metadata and containment status
Loading

The worker runs as a separate Linux subreaper, so adopted descendants belong to that terminal and harness siblings remain outside its tree. A safe maintained PTY crate provides native async I/O; the lock retains existing Tokio and Rustix versions. No new image helper or network channel is required.

Check Bound or outcome
Linux subreaper, pidfd and process scan setup Checked before shell creation; unsupported setup refuses
Input/output 32-KiB JSON lines, 16-KiB decoded data, dimensions 1–1000
Lifetime and idle At most one hour; five-minute idle; empty data does not renew idle
I/O and cleanup Five-second deadlines; bounded shutdown after cancelled stdin reads
Process ownership 256 descendants, 4096 scanned processes, start-time-fenced pidfd signals
Cleanup failure Distinct fatal status; runtime consumer must terminate the harness
Shell environment Whitelisted values; no ambient Buzz/model credential variables

The shell still has the sandbox user's filesystem privileges. Explicit per-session owner approval is required in the upcoming relay consumer; a global gateway grant alone never authorizes it. Worker documentation describes the local protocol and failure contract.

Validation

  • Five production-binary tests pass with a real PTY: resize/environment, EOF, expiry, invalid input, buffered final output and real setsid/double-fork cleanup while a sibling survives.
  • Two production process-selection tests cover sibling exclusion and the descendant cap.
  • Full ACP library suite: 1134 passed, 3 ignored.
  • Seven guard mutations fail regression assertions: subreaper setup, complete cleanup, expiry, input-line bound, sibling selection, descendant cap and final output drain. Sources restored; the five PTY tests and two selection tests pass again.
  • Locked all-target check, all-target Clippy with warnings denied, formatting, differential file-size and diff checks pass.
  • Full repository CI and human confirmation remain pending. This PR stays draft; no deployment or cluster changes.

The worker source, tests and documentation are byte-identical to 15b80d6. The only lockfile difference versus that revision is the already-published CLI dependencies. Locked offline metadata, formatting and diff checks pass on this replacement; the combined CLI/worker source was also built in the isolated integration worktree. Independent review of the final-output fix is resolved.

Preserves the reviewed implementation from 48d2444 and the final-output fix from 15b80d6, on the published terminal CLI foundation.

Signed-off-by: Tee8z <tee8z@protonmail.com>
@tee8z tee8z self-assigned this Nov 10, 2026
@coderabbitai

coderabbitai Bot commented Nov 10, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@tee8z

tee8z commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review carried forward from #165 to bc7b0978940231a2f39f318bafc329714af45140.

I verified that every worker source, test, dependency declaration, and worker documentation file is byte-identical to the reviewed 15b80d609. The lockfile difference is limited to the already-published CLI dependencies inherited from #163. No new worker behavior was introduced by this replacement.

The final-output blocker is resolved. After natural shell exit, the worker preserves the PTY, stops/reaps descendants that could retain its slave, and drains buffered output under one five-second deadline and a 64-KiB budget. Timeout or overflow reports failure instead of successful truncation. I independently reran all five production-binary PTY tests and my original pause/release probe on the identical reviewed worker; they pass, with FINAL_OUTPUT delivered before a successful end receipt. The parent-death probe also verified cleanup while worker stdin remained open.

No remaining concrete blocker in this layer. Minimalism 9/10, elegance 9/10, correctness 9/10. The upcoming relay consumer still needs its own review and combined runtime exercise. Human confirmation remains outstanding; this PR remains draft.

This branch has not been deployed

No deployments
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