Skip to content

feat(acp): isolate terminal workers with bounded process cleanup - #165

Closed
tee8z wants to merge 2 commits into
tee8z/sandbox-attachfrom
tee8z/sandbox-terminal-worker
Closed

tee8z wants to merge 2 commits into
tee8z/sandbox-attachfrom
tee8z/sandbox-terminal-worker

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.

Depends on #159, whose packet shapes the local worker reuses. The gateway foundation remains #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.

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 follow-up review of 15b80d609: the final-output blocker is resolved. No remaining concrete blocker in the worker layer. Minimalism 9/10, elegance 9/10, correctness 9/10.

After natural shell exit, the worker preserves the PTY, stops/reaps descendants that could retain its slave, and drains buffered output. The drain has 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; they pass. The scheduling-barrier regression includes a background holder and catches both early output loss and an indefinite EOF drain. My original standalone pause/release probe now delivers FINAL_OUTPUT before a successful end receipt. The earlier parent-death probe also verified cleanup while worker stdin remained open.

The remaining relay consumer must enforce owner approval, publish durable acceptance before shell creation, wait for readiness before Open acknowledgement, and treat containment exit 2 as harness-fatal. This review covers the worker layer; human runtime confirmation remains outstanding. The PR remains draft.

Signed-off-by: Tee8z <tee8z@protonmail.com>
@tee8z

tee8z commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #166 to join the published terminal CLI and worker into one linear stack for the snapshot/relay consumers. Worker source, tests, and documentation are byte-identical to 15b80d6, including the independently rechecked final-output fix. Existing history is preserved; no force push or shared-history rewrite. The new PR remains draft.

@tee8z tee8z closed this Nov 10, 2026
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