docs(adr): PTY Mode — remote sandboxed terminal sessions - #1477
docs(adr): PTY Mode — remote sandboxed terminal sessions#1477chaodu-obk[bot] wants to merge 2 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Important CHANGES REQUESTED What This PR DoesProposes an opt-in PTY mode: a second session backend where OAB spawns a CLI as a PTY subprocess inside its pod and streams the raw terminal bidirectionally over WebSocket, making OAB dual-persona (ACP message broker + terminal server). ADR only -- no code changes. How It WorksSession-pool reuse ( Scope of This CommentThis round-3 comment is the final consolidated review for head Findings
Finding Details🔴 C1: Reconcile the product identity before the design details matterDESIGN.md pillar #1 defines OAB as a deliberately thin transport whose non-decisions are features. A terminal server that owns subprocesses, scrollback, render surfaces, and input streams is a different product category; a sandboxed terminal server is still a terminal server. The ADR also leads with an observability pain that PTY mode bypasses rather than fixes -- Alternative A (ACP Requested change: either (a) explicitly amend DESIGN.md pillar #1 and accept the positioning shift, or (b) reshape the decision: a separately deployable PTY runtime (own pod/binary/token, with OAB acting only as an authenticated discovery/notification bridge) plus Alternative A pursued for the in-bridge visibility gap. Add Alternative E and demand evidence either way. The review does not reject the remote-PTY need itself -- it rejects the in-process dual-persona form. 🔴 C2: Define a real isolation boundaryThe broker process and the PTY subprocess share one PID namespace, cgroup, network namespace, and mounted credential plane. Requested change: commit to one of: separate pod per PTY session; sidecar container (separate PID namespace and cgroup); or accept and document same-pod privilege with enumerated compensating controls and a recommendation that PTY-enabled replicas deploy separately from ACP-serving replicas. Remove or rewrite "sandbox posture unchanged". 🔴 C3: Match the auth contract to shell-equivalent capabilityRequired: WSS/TLS mandate (or a documented TLS-terminating boundary); fail-closed startup when the key is unset; browser-safe credential transport reusing the validated 🔴 C4: Resolve the scrollback contradictionMake Phase 1 match the claimed adoption: replay off by default or behind explicit opt-in; bounded buffer with a hard cap; cleared on teardown; never persisted to disk; documented as sensitive memory that is not a redaction boundary. 🔴 C5: Decide and cost the pool integration shapeChoose one explicitly: a backend-neutral lifecycle/quota/pgid abstraction with separate ACP and PTY backend state, or a dedicated PTY manager sharing only capacity and termination primitives. State which invariants are shared, which ACP behaviors do not apply, define what "idle" means for an unattached PTY (a detached long-running build is active, not idle), and estimate the regression surface on the shipped ACP path. 🟡 I1-I20Each item in the table above carries its requested change inline; the fastest path is to treat I1-I14 as the Phase 1 contract checklist (lifecycle, capacity, replay cursor, writer arbitration, create/kill API, naming/command authority, env allowlist, backpressure, teardown, recovery, rollback, audit, K8s posture, feature wiring) and I15-I20 as document-consistency repairs (phase-mapping reconciliation, A2A re-scoping, citation parity, AgentCore non-goals, Phase 4 heuristic, naming/reference cleanups). Baseline Check
What's Good (🟢)
5. Three Reasons We Might Not Need This PR
DispositionVerdict: CHANGES REQUESTED -- the ADR should not be accepted as written. The maintainer will choose among: full revision in place, trimming to a Phase-1-only bounded experiment, replacing with a separately-deployable PTY runtime ADR (the review's convergent recommendation), or closing in favor of an ACP observability ADR. The sec. 6 research is portable to any of these outcomes. |
|
Closing per maintainer decision after the consolidated review above: the in-process dual-persona form is superseded by an optional Successor: #1478 The prior-art survey (sec. 6) carries over to the new ADR with adopt-in targets normalized against its phase plan. The review record here remains the rationale for rejecting the in-process form. |
Summary
Proposes an opt-in PTY mode as a second session backend alongside the existing ACP session pool, based on the Discord discussion comparing OAB with OpenDray and Herdr.
Review Contract
Goal
Record an ADR (Status: Proposed) that captures the design decision space for a PTY-based remote terminal capability: the user need, the architecture options, the security trade-offs, and a prior-art survey of OpenDray, Herdr, and Claude Code cross-session messaging mapped to implementation phases.
Non-goals
openab-ptyruntime vs ACP observability extension) — that decision is pending maintainer direction based on review feedback.Accepted Residual Risks
OpenAB PR Reviewstatus gate block acceptance of the ADR as-is.Acceptance Criteria
docs/adr/pty-mode.mdfollows the established ADR format (header block, numbered sections, alternatives with rationale).Follow-ups
openab-ptyas a separately deployable runtime; ACP observability extension (Alternative A); JSONL transcript as a structured side channel.Architecture
flowchart LR subgraph clients_acp["Persona 1 — Agent Broker (existing)"] discord["Discord / Slack / Telegram<br/>turn-based messages"] end subgraph clients_pty["Persona 2 — Terminal Server (new)"] web["Web terminal<br/>(xterm.js)"] mobile["Mobile / desktop app<br/>(terminal widget)"] end subgraph pod["OpenAB Unified Binary — K8s pod (sandbox unchanged)"] adapters["Platform adapters"] ws["GET /pty/{session}<br/>WebSocket + Bearer token"] pool["Session pool<br/>(lifecycle · TTL · eviction · pgid kill)"] acp_backend["ACP backend<br/>stdio JSON-RPC"] pty_backend["PTY backend (new)<br/>portable-pty + scrollback ring buffer"] agent_acp["Agent subprocess<br/>(structured ACP events)"] cli_pty["CLI subprocess<br/>bash / claude / codex / kiro"] end discord --> adapters --> pool web -- "raw PTY bytes (binary frames)<br/>resize / detach (text frames)" --> ws mobile --> ws ws --> pool pool --> acp_backend --> agent_acp pool --> pty_backend --> cli_pty pty_backend -. "Phase 4: idle/blocked notification" .-> adapters style pty_backend fill:#1a7f37,color:#fff style ws fill:#1a7f37,color:#fff style cli_pty fill:#1a7f37,color:#fffKey points:
Prior art
§6 of the ADR maps concrete techniques from three adjacent projects to implementation phases:
working/blocked/idle/done), race-safe waits, layered restore taxonomy, secrets-safe scrollback defaultNotes
Context: Discord thread discussion on 2026-08-15 (OAB vs OpenDray comparison).