Skip to content

feat: add bounded TTY command execution - #451

Open
yattdev wants to merge 3 commits into
agentclientprotocol:mainfrom
yattdev:feature/add-guarded-tty-brid-6q9
Open

feat: add bounded TTY command execution#451
yattdev wants to merge 3 commits into
agentclientprotocol:mainfrom
yattdev:feature/add-guarded-tty-brid-6q9

Conversation

@yattdev

@yattdev yattdev commented Aug 30, 2026

Copy link
Copy Markdown

Overview

codex-acp maps Codex App Server commands and terminal output into ACP, but integrations that need terminal semantics cannot currently request a bounded one-shot TTY execution while preserving the active session context.

This contribution adds a narrow, versioned extension that dispatches one command through the already-running App Server with tty: true. It is implemented entirely inside codex-acp and has no dependency on an external orchestration service.

Why this is useful

Some command-line programs change behavior when no pseudo-terminal is present. Reusing the active Codex session gives the adapter an authoritative working directory and sandbox policy while avoiding a second executor or a caller-controlled security context.

The proposed bridge complements the existing command and terminal-event mapping:

  • one bounded argv request instead of an interactive terminal lifecycle;
  • active-session cwd and sandbox policy instead of caller-controlled values;
  • streamed App Server output correlated by a generated process ID;
  • deterministic cancellation, timeout, overflow, and stale-session handling;
  • no change to ordinary non-TTY command execution.

Changes

  • Advertise guarded-TTY capability version 1.
  • Register the provisional extension methods:
    • _kandev/guarded_tty/capability
    • _kandev/guarded_tty/exec
  • Accept only { sessionId, argv } with strict argument-count, per-argument, total-byte, and NUL bounds.
  • Derive cwd and sandboxPolicy from the trusted active session.
  • Generate the process ID in the adapter and dispatch App Server command/exec with TTY and streaming enabled.
  • Validate base64 output, preserve split UTF-8 sequences, and enforce a 64 KiB aggregate cap.
  • Return a stable receipt containing capability, session, process, cwd, TTY, output, exit, and timestamp evidence.
  • Terminate the process best-effort and idempotently on cancellation, stale session, timeout, overflow, invalid output, or App Server rejection.

The current method prefix is retained from the originating implementation for compatibility. It does not introduce a runtime dependency; I am happy to rename it to an upstream-preferred experimental namespace during review.

Security boundaries

The request cannot control cwd, sandbox policy, permission profile, process ID, TTY flags, environment, stdin, resize, attach, or an interactive lifecycle. The change does not create a host executor, mutate Codex configuration, alter mounts or credentials, or enable unrelated execution surfaces.

Testing

  • npx vitest run src/__tests__/CodexACPAgent/guarded-tty-exec.test.ts src/__tests__/CodexACPAgent/terminal-output-events.test.ts --no-file-parallelism — 28 passed
  • npm run typecheck — passed
  • npm run build — passed
  • npm test — 495 passed, 26 skipped
  • git diff --check origin/main...HEAD — passed

Credentialed npm run test:e2e was not rerun because the required API keys are unavailable. Prior authenticated App Server probes verified TTY stdin/stdout, stty, trusted-worktree pwd, and exit code 0. Bun-only release bundling was not run because Bun is unavailable; the production build passed.

Compatibility

No existing ACP or codex-acp behavior changes. Clients that do not negotiate and call this versioned extension continue using the existing command path.

Screenshots

N/A — no UI-visible changes.

@yattdev
yattdev marked this pull request as ready for review August 30, 2026 10:01
@yattdev

yattdev commented Aug 30, 2026

Copy link
Copy Markdown
Author

@nikita-ashihmin This first-time fork PR is ready to run CI at exact head 1a5d8b9cf1f70a8677ead500088a8e022cdc65bb, but GitHub stopped both workflows at the fork-approval gate with zero jobs: CI run 33305349645 and Conventional PRs run 33305349647. Could you approve those workflow runs? This is only a workflow-approval request; I will wait for terminal green checks before requesting code review.

@yattdev
yattdev marked this pull request as draft September 4, 2026 19:24
@yattdev
yattdev marked this pull request as ready for review September 8, 2026 00:17
@yattdev yattdev changed the title feat: add guarded TTY ACP bridge feat: add bounded TTY command execution Sep 8, 2026
@yattdev

yattdev commented Sep 8, 2026

Copy link
Copy Markdown
Author

@nikita-ashihmin PR #451 is now ready for review at exact head 0bd0f8fb74a0dfec93eb3a8e586dc0d7c0e8488d. I revised the title and description to present the change in codex-acp terms: a bounded one-shot TTY execution improvement that reuses the active session context, with explicit security and compatibility boundaries. The current extension prefix is called out as provisional and can be renamed to the namespace you prefer.

The current CI run and Conventional PRs run are still action_required because this is a first-time fork contribution. Could you approve those workflows and review the contribution shape? No existing ACP behavior changes for clients that do not negotiate/call the extension.

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.

2 participants