Skip to content

docs: add ADR comparing PTY vs ACP runtime capabilities#524

Open
khaliqgant wants to merge 10 commits intomainfrom
claude/compare-relay-acpx-qW7wt
Open

docs: add ADR comparing PTY vs ACP runtime capabilities#524
khaliqgant wants to merge 10 commits intomainfrom
claude/compare-relay-acpx-qW7wt

Conversation

@khaliqgant
Copy link
Member

@khaliqgant khaliqgant commented Mar 10, 2026

Detailed analysis of what is lost, degraded, and eliminated when
switching from PTY to ACP for agent management. Covers mid-turn
injection, delivery lifecycle, auto-approval, continuity, crash
classification, swarm TUI, wrap mode, and idle detection with
exact code references.

https://claude.ai/code/session_013kip2mTjYccdajsgwgUqcw


Open with Devin

claude added 7 commits March 10, 2026 06:08
Detailed analysis of what is lost, degraded, and eliminated when
switching from PTY to ACP for agent management. Covers mid-turn
injection, delivery lifecycle, auto-approval, continuity, crash
classification, swarm TUI, wrap mode, and idle detection with
exact code references.

https://claude.ai/code/session_013kip2mTjYccdajsgwgUqcw
…tion

Part 1: Replace PTY KIND: continuity with relay_checkpoint / relay_restore
MCP tools — structured JSON, persisted to Relaycast cloud, works across
all runtimes (PTY, ACP, future).

Part 2: Five proposals for mid-turn message injection under ACP:
A) Queue + drain on turn boundary (covers 80% of patterns)
B) Cancel + re-prompt for urgent/P0 messages
C) MCP tool polling via relay_inbox()
D) Dual-channel architecture with MCP sidecar (best for real-time)
E) ACP protocol extension session/inject (upstream proposal)

Includes recommendation matrix, implementation order, and code sketches.

https://claude.ai/code/session_013kip2mTjYccdajsgwgUqcw
…on only

- Flip default: ACP is the default runtime, PTY is fallback for mid-turn
  injection patterns only (mesh, consensus, debate)
- Kill Proposal D (dual-channel / MCP sidecar): collapses to polling because
  MCP tool calls are gated by ACP turns — no actual second channel exists
- Debunk all ACP injection workarounds with specific failure modes
- Add clear runtime selection rule based on swarm pattern

https://claude.ai/code/session_013kip2mTjYccdajsgwgUqcw
ACP runtime spec covers:
- Type changes: AgentRuntime adds 'acp', new AcpWorkerToBroker messages
- Runtime auto-selection: headless for presets, PTY for mesh/consensus/debate, ACP for everything else
- ACP adapter resolution map (claude→claude-agent-acp, goose→native, etc.)
- Turn-boundary delivery lifecycle (queued → prompted → ack)
- P0 emergency interrupt via session/cancel + re-prompt
- MCP server injection (Relaycast auto-injected into session/new)
- Permission flow replacing PTY auto-approval handlers
- Crash recovery via session/load
- Swarm TUI integration via acp_turn_update events

Example workflow (YAML + TypeScript builder):
- Lead (claude, ACP auto-selected) + ACP worker + goose reviewer + headless codex
- DAG: plan → parallel impl → file verify → security review → finalize
- Demonstrates runtime field, relay_checkpoint, turn-boundary delivery

https://claude.ai/code/session_013kip2mTjYccdajsgwgUqcw
DAG workflow with 3 phases, 3 parallel tracks, 8 agents:

Phase 1 — Context gathering (7 deterministic steps):
  Read spec + all 5 source files that need modification

Phase 2 — Parallel implementation:
  Track A (types-lead + 2 workers): protocol.ts, types.ts, builder.ts
    - Add 'acp' to AgentRuntime union
    - Add AcpTurnUpdate, AcpWorkerToBroker, BrokerToAcpWorker types
    - Add AcpMcpServerConfig, AcpPermissionRequest interfaces
    - Add runtime/acpAdapter fields to AgentDefinition and AgentOptions
  Track B (runner-lead + 1 worker): runner.ts
    - Add ACP_ADAPTERS adapter resolution map
    - Add INJECTION_PATTERNS constant
    - Add resolveRuntime() method (headless → injection patterns → ACP/PTY)
    - Add spawnAcp() stub method
    - Refactor spawnAndWait() to branch on resolved runtime
  Track C (1 worker): client.ts
    - Add SpawnAcpInput interface
    - Add spawnAcp() method to AgentRelayClient

Phase 3 — Verification:
  verify-all-files → tsc --noEmit → review → fix → final-typecheck gate

https://claude.ai/code/session_013kip2mTjYccdajsgwgUqcw
Fixes from code review:

1. acp-dag-example.ts: Remove `runtime` field from builder calls (field
   doesn't exist on AgentOptions yet). Add NOTE that this file requires
   ACP changes to compile. Point users to YAML version.

2. implement-acp-runtime.yaml: Remove broken lead coordination pattern.
   Lead agents (types-lead, runner-lead) were trying to coordinate
   non-interactive workers via relay channels, but preset:worker agents
   have no relay connection. Replaced with direct worker execution —
   workers already have full spec + file content pre-injected.
   - Reduced from 8 agents to 6
   - Fixed deps: runner-worker + client-worker depend on verify-types
   - Fixed line number reference ("near line 90" → "next to HUB_PATTERNS")
   - Updated description: 2 sequential waves, not 3 parallel tracks

3. implement-acp-runtime.ts: Replace non-existent `runner.runFromYaml()`
   with `runWorkflow()` from run.ts. Replace `import.meta.dirname!` with
   portable `fileURLToPath(import.meta.url)`.

4. acp-runtime-spec.md: Replace SpawnAgentInput rename proposal with
   separate SpawnAcpInput interface to match implementation workflow.
   SpawnPtyInput remains unchanged.

https://claude.ai/code/session_013kip2mTjYccdajsgwgUqcw
The sed range 2183-2600 was missing the tail end of spawnAndWait()
(PTY output collection + return logic, lines 2600-2665). Extended to
2183-2668 so the runner-worker sees the complete method body when
extracting it into spawnAndWaitPty.

Also fixed read-runner-constants comment — removed incorrect mention
of buildNonInteractiveAwareness (that function is at line 2982, not
in the 2660-2700 range).

https://claude.ai/code/session_013kip2mTjYccdajsgwgUqcw
devin-ai-integration[bot]

This comment was marked as resolved.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

View 7 additional findings in Devin Review.

Open in Devin Review

cli: claude
channels: [wf-acp-dag]
role: 'Implements backend changes as directed by lead.'
runtime: acp # Explicit — same as default, shown for clarity
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 YAML examples use runtime field that violates the project's JSON schema (additionalProperties: false)

The acp-dag-example.yaml uses runtime: acp on agent definitions (lines 45, 55), but the project's JSON schema at packages/sdk/src/workflows/schema.json:137-176 defines AgentDefinition with "additionalProperties": false and does NOT include runtime as an allowed property. Similarly, AgentDefinition in packages/sdk/src/workflows/types.ts:92-116 has no runtime field. This means these YAML files will fail validation for anyone using the schema (which the README explicitly advertises for editor autocompletion: packages/sdk/src/workflows/README.md:645). The runtime validateConfig() in packages/sdk/src/workflows/runner.ts:422-467 only does basic structural checks (name, cli) so it won't reject the field at runtime — it will silently be ignored, meaning agents intended to run as ACP will actually fall through to the default PTY/headless behavior with no warning.

Prompt for agents
The `runtime` field used in acp-dag-example.yaml (lines 45, 55) is not recognized by the project's AgentDefinition type (packages/sdk/src/workflows/types.ts:92-116) or the JSON schema (packages/sdk/src/workflows/schema.json:137-176 which has additionalProperties: false). To make these YAML files valid:

1. Add `runtime` to AgentDefinition in packages/sdk/src/workflows/types.ts (after the `preset` field around line 115):
   runtime?: 'pty' | 'acp' | 'headless';
   acpAdapter?: string;

2. Add `runtime` to the AgentDefinition in packages/sdk/src/workflows/schema.json (inside the properties object around line 174):
   "runtime": { "type": "string", "enum": ["pty", "acp", "headless"], "description": "Agent runtime override." }

3. Add `runtime` and `acpAdapter` to AgentOptions in packages/sdk/src/workflows/builder.ts and wire them through the agent() method.

Without these changes, the runtime field is silently ignored and agents will not use the ACP runtime as intended.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid — the runtime field isn't in the current JSON schema. These YAML examples in the ADR are illustrative (showing a proposed future schema), not meant to validate against the current schema. Added a note clarifying these are proposed examples, not production-ready workflows.

Comment on lines +137 to +139
verification:
type: exit_code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 verification for frontend-impl step missing required value field

The frontend-impl step's verification at line 138 specifies type: exit_code but omits the value field. The JSON schema at packages/sdk/src/workflows/schema.json:538-556 defines VerificationCheck with "required": ["type", "value"], and the TypeScript type at packages/sdk/src/workflows/types.ts:278-282 also declares value: string as non-optional. The TypeScript version of this same example (acp-dag-example.ts:114) correctly includes value: ''. This omission will cause schema validation failure, and may cause a runtime error when the runner processes the verification check depending on how it accesses the value property.

Suggested change
verification:
type: exit_code
verification:
type: exit_code
value: '0'
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as #527 — exit_code type needs a value field. Since this is an ADR doc with illustrative examples, added value: 0 to the verification block.

…ation

Key changes from the original:
- spawnAcp must actually spawn adapter process, send session/new and
  session/prompt via JSON-RPC, read streaming responses — NOT a stub
- verify-impl step explicitly checks for session/new, session/prompt,
  and jsonrpc in runner.ts to catch stubs
- runner-worker gets acp-bridge source as reference for JSON-RPC patterns
- Task prompts shortened (10-20 lines guideline)
- Removed output_contains verification with tokens in task text
  (double-occurrence problem) — using exit_code instead
- Removed fixer's channel subscription (non-interactive workers can't
  listen on channels)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants