Skip to content

fix(harness): retain local Claude bridges while approval is pending - #6567

Closed
chelojimenez wants to merge 1 commit into
fix/local-codex-approval-historyfrom
fix/local-claude-approval-park
Closed

chelojimenez wants to merge 1 commit into
fix/local-codex-approval-historyfrom
fix/local-claude-approval-park

Conversation

@chelojimenez

@chelojimenez chelojimenez commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Task, spec and plan

User story

As a local Claude Code Playground user, I want my approval to answer the command already waiting, so that the command does not need to be proposed again.

Acceptance criteria

  • Claude Code retains its live bridge after an approval pause and HTTP response finalization.
  • Approval and denial resume the same process generation across streamed HTTP responses; duplicate decisions do not execute again.
  • Reload reads preserve pending approval, while Stop and park expiry invalidate it and empty the process registry.
  • The real local preparation path rebinds a fresh lease to the same Claude model gateway.
  • Harness layer generation and pack pin/digest checks pass.

What this does

Claude Code's canUseTool waits inside its bridge, so a pending approval must retain that bridge just as Codex does. Enable the existing live runtime parking path for Claude, preserve its continuation token, and exercise the actual UI stream finalizer across successive responses. The local preparation tests now exercise lease renewal and teardown with both runtimes. The vendor bridge's requestToolApproval waits on its pending promise without a timer, while the existing park registry enforces the 30-minute lifetime.

Documentation

Nothing applicable

Architecture

flowchart LR
  Pause[Claude pending tool] -->|continue-turn bridge token| Park[Local parked session]
  Park -->|new request and decision| Claim[Strict approval claim]
  Claim -->|fresh lease, same gateway| Claude[Same Claude bridge]
  Park -->|Stop or expiry| Cleanup[Registry teardown]
Loading

Screenshots and recordings

None: no UI change

Deleted

The incorrect lifecycle comment claiming a pending Claude approval can be resumed by restarting from disk.

Reused

The existing liveApprovalRuntime capability, approval-park and session-registry state machines, adoptParkedLocalTurn lease rebind, UI stream finalization, and local turn preparation tests.

Production signal

A local Claude approval completes on the already displayed command; existing parked-session and teardown signals identify its lifetime. No new telemetry is introduced.

Checklist

  • I can explain every line of this diff without the agent that wrote it.
  • Under 400 hand-written changed lines, or labelled large-pr / mechanical.
  • No new as any, empty catch, .catch(() => {}) or raw console on the server.
  • Comments say why the code is this way, not how it got here.

The process stream and external broker are test fixtures. This validates the HTTP stream lifecycle and real parking/claim/registry boundaries; native runtime qualification on installed packs and production remains a separate release gate.

Validation

97 targeted tests pass (5 HTTP stream lifecycle tests and 92 local preparation, layer and pack tests). Locked dependency drift and slop checks pass; the harness layer was regenerated.

CI run 38986408656 at head 386e4c8a3 has two unresolved test failures:

  • Inspector Tests 1/6: hosted upstream timeout test returns the expected 424/TIMEOUT response and disconnects, but its final aborted assertion observes false.
  • Inspector Tests 4/6: the rewind URL test still observes conversation=original-chat-session.

Local comparison used the exact PR head 386e4c8a3 and exact base c0b7c52a4 with the same installed dependencies and single-worker Vitest. On each revision, the complete hosted-upstream file passed 6/6, the complete PlaygroundMain file passed 138/138, and the isolated rewind case passed. The relevant UI, URL synchronization, hosted-route, and test sources are unchanged by this PR. These runs did not reproduce either CI failure and do not establish that CI is resolved.

The hosted test still uses its existing unsynchronized 50 ms real-time timeout; the separate request-start synchronization fix is not included in this head. The rewind test mock keeps the original live session ID after its branch callback, which may allow URL synchronization to restore it; this remains a hypothesis because neither isolated nor full-file comparison reproduced the CI result. No production behavior or assertions were changed to force these checks green. Keep this PR draft pending resolution of the failed checks.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@chelojimenez

chelojimenez commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor Author

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Internal preview

Preview URL: https://mcp-inspector-pr-6567.up.railway.app
Deployed commit: 064b679
PR head commit: 386e4c8
Backend target: staging fallback.
Health: ✅ Convex reachable
Access is employee-only in non-production environments.

@chelojimenez

Copy link
Copy Markdown
Contributor Author

Replaced by #6730, merged 2026-10-19 as part of the KES-670 release (I5a in the plan). Closing in favor of it.

This branch was successfully deployed

1 active deployment
preview-pr-6567 — 386e4c8a Deployed Oct 17, 2026 by chelojimenez via upsert-preview #34045
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