[AI-2696] Let the app-server runtime answer approvals in an envelope-sourced session - #877
Open
realtonyyoung wants to merge 5 commits into
Open
[AI-2696] Let the app-server runtime answer approvals in an envelope-sourced session#877realtonyyoung wants to merge 5 commits into
realtonyyoung wants to merge 5 commits into
Conversation
…ssion Codex runs the user's PermissionRequest hook before it asks its client, and in a hosted session that hook bounced the approval through the daemon bridge — a channel built for Claude with no deadline — so the runtime's requestApproval, and the fail-closed decline it carries, never happened. The hook now yields whenever the session is envelope-sourced, keyed on the same marker that already suppresses its transcript watcher there.
The server holds an interaction open until something answers it; nothing told it when this side gave up, so a timed-out approval stayed a pending card for the rest of the session. The cancelled wait now resolves the entry best-effort before the cancellation propagates.
PR Summary by QodoRoute hosted Codex approvals through the app-server runtime
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
The hub is already stopping when daemon shutdown cancels the wait, so the resolve could not land; session end closes the entry on the server instead.
The server answers an abandoned interaction anyway — with the cancel this side asked for — and request ids are a per-server sequence, so buffering that echo would hand a stale cancel to whichever later request reuses the id after a server restart. A fresh wait on the id supersedes the mark.
A cleared mark left its order entry behind, so the list grew without bound and an id abandoned a second time could be evicted by the age of its first mark.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No GitHub issue — AI-2696
What & why
In a hosted app-server Codex session, an approval never failed closed: Codex runs the user's
PermissionRequesthook before it asks its client, and that hook bounced the request through the daemon'sLocalPermissionBridge— the Claude channel, deliberately built with no deadline — so the runtime'srequestApproval(andCodexApprovalBridge's 45 s decline) never happened. The hook now yields in an envelope-sourced session, keyed on the sameKCAP_HOSTED_APPSERVERmarker that already suppresses its transcript watcher there. Separately, the server keeps an interaction open until something answers it and nothing told it when the daemon stopped waiting, so a timed-out request stayed a pending card for the rest of the session; the cancelled wait now resolves the entry ascancelbefore the cancellation propagates.Where to look
The hook change is one guard at the top of
HandlePermissionRequest; everything hangs on the marker being set only for envelope-sourced launches (CodexHostedAgentRuntimeFactory.BuildEnv).ResolveAbandonedInteractionAsyncreuses the existingRespondToPermissionhub call — the hub already routes an ACP request id toacpTracker.TryComplete— with the canonical 32-hex thread id, because the hub does not normalise that argument.Verification
KCAP_ACP_DEBUG_FRAMES=1, approval timeout 15 s): a full approval cycle showed norequestApprovalJSON-RPC frame at all — onlyhook/startedframes andLocalPermissionBridge → RequestPermissionAsync → PendingPermissionRegistrystacks; Codex's rollout shows the exec cell blocked until the dashboard answered (:2ran 80 ms after a late allow, 150 s in).CodexHookCommandTests32/32 incl. the new yield test; mutant (marker never matches) fails it plus the two guard-1 tests that share the check.ServerConnectionAbandonedInteractionTests1/1; mutant (resolve never reaches the server) fails with a 5 s timeout.CodexApprovalTimeoutChainTestspins that bridge + real registry + real retry decline at the deadline (they did; the bridge was simply never asked).