Repository navigation
fix(codex): surface app permission requests as approvable #7861
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d5005ad
0102483
b487c98
ade2d18
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2232,6 +2232,69 @@ export const makeCodexSessionRuntime = ( | |
| }), | ||
| ); | ||
|
|
||
| yield* client.handleServerRequest("item/permissions/requestApproval", (payload) => | ||
| Effect.gen(function* () { | ||
| const requestId = ApprovalRequestId.make( | ||
| yield* randomUUIDv4("app-permission-approval-request"), | ||
| ); | ||
| const turnId = TurnId.make(payload.turnId); | ||
| const itemId = ProviderItemId.make(payload.itemId); | ||
| const decision = yield* Deferred.make<ProviderApprovalDecision>(); | ||
|
|
||
| yield* Ref.update(pendingApprovalsRef, (current) => { | ||
| const next = new Map(current); | ||
| next.set(requestId, { | ||
| requestId, | ||
| jsonRpcId: payload.itemId, | ||
| requestKind: "permission", | ||
| turnId, | ||
| itemId, | ||
| decision, | ||
| }); | ||
| return next; | ||
| }); | ||
| yield* Ref.update(approvalCorrelationsRef, (current) => { | ||
| const next = new Map(current); | ||
| next.set(payload.itemId, { | ||
| requestId, | ||
| requestKind: "permission", | ||
| turnId, | ||
| itemId, | ||
| }); | ||
| return next; | ||
| }); | ||
|
|
||
| yield* emitEvent({ | ||
| kind: "request", | ||
| threadId: options.threadId, | ||
| method: "item/permissions/requestApproval", | ||
| requestId, | ||
| requestKind: "permission", | ||
| ...(turnId ? { turnId } : {}), | ||
| ...(itemId ? { itemId } : {}), | ||
| payload, | ||
| }); | ||
|
|
||
| const resolved = yield* Deferred.await(decision).pipe( | ||
| Effect.ensuring( | ||
| Ref.update(pendingApprovalsRef, (current) => { | ||
| const next = new Map(current); | ||
| next.delete(requestId); | ||
| return next; | ||
| }), | ||
| ), | ||
| ); | ||
| // Approving grants the requested profile; denying answers with an | ||
| // empty grant so the app-server treats the permission as withheld. | ||
| const grantedPermissions = | ||
| resolved === "accept" || resolved === "acceptForSession" ? payload.permissions : {}; | ||
| return { | ||
| permissions: grantedPermissions, | ||
| ...(resolved === "acceptForSession" ? { scope: "session" as const } : {}), | ||
| } satisfies EffectCodexSchema.PermissionsRequestApprovalResponse; | ||
| }), | ||
| ); | ||
|
|
||
| yield* client.handleServerRequest("item/tool/requestUserInput", (payload) => | ||
| Effect.gen(function* () { | ||
| const requestId = ApprovalRequestId.make(yield* randomUUIDv4("user-input-request")); | ||
|
|
@@ -2499,6 +2562,16 @@ export const makeCodexSessionRuntime = ( | |
| Effect.gen(function* () { | ||
| const providerThreadId = yield* readProviderThreadId; | ||
| const session = yield* Ref.get(sessionRef); | ||
| // Settle parked approvals FIRST. The transport answers server | ||
| // requests inline on its stdin read loop, so a pending | ||
| // command/file/app-permission prompt blocks every incoming message, | ||
| // including the turn/interrupt response itself - cancelling after | ||
| // the RPC would deadlock Stop exactly when a card is open. Settling | ||
| // releases the handler, which answers the peer and unblocks the | ||
| // loop before the interrupts below are sent. | ||
| yield* settlePendingApprovals("cancel"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 High When Stop is pressed with an 🤖 Copy this AI Prompt to have your agent fix this: |
||
| // Pending user-input prompts block the same way; settle them too. | ||
| yield* settlePendingUserInputs({}); | ||
| // Stop-everything: children are full threads with their own turns; | ||
| // interrupting only the parent leaves the fleet running. Interrupt | ||
| // each live child turn first, best-effort per child, BOUNDED: the | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
Repository: pingdotgg/t3code
Length of output: 12119
🏁 Script executed:
Repository: pingdotgg/t3code
Length of output: 25180
🏁 Script executed:
Repository: pingdotgg/t3code
Length of output: 31517
🏁 Script executed:
Repository: pingdotgg/t3code
Length of output: 13154
🏁 Script executed:
Repository: pingdotgg/t3code
Length of output: 10009
🏁 Script executed:
Repository: pingdotgg/t3code
Length of output: 16705
🏁 Script executed:
Repository: pingdotgg/t3code
Length of output: 11604
🌐 Web query:
Codex app-server protocol "serverRequest/resolved" requestId itemId JSON-RPC💡 Result:
<search_synthesis>
In the Codex app-server protocol, serverRequest/resolved is a server-initiated JSON-RPC notification used to inform the client that a previously issued server-to-client request has been finalized [1][2][3]. Protocol Details: 1. Server-Initiated Request: The server initiates a request (e.g., item/commandExecution/requestApproval or item/tool/requestUserInput) to the client, providing a unique requestId, itemId (if applicable), and context params (threadId, turnId) [1][4][5][3]. 2. Client Response: The client responds to the original JSON-RPC request ID with its decision or user input [1][4][3]. 3. Completion Notification: Once the server processes the client&
#39;s response, it emits the serverRequest/resolved notification [1][6][3]. This notification acts as an acknowledgment that the lifecycle of the specific request has ended [7]. Key Fields in serverRequest/resolved: - threadId: The identifier for the conversation thread [5][7][8]. - requestId: The unique identifier matching the original request [4][5][7]. This notification is critical for clients to clear pending states in their UI, such as closing an approval dialog or hiding a user input prompt, after the server has acted upon the user's input [7][8]. Clients typically correlate these events using the requestId [7].</search_synthesis>
<source_evidence>
Citations:
Correlate approval resolutions with the original JSON-RPC request ID.
CodexAppServerIncomingRequest.idis separate frompayload.itemId, buthandleServerRequestcurrently passes only the decoded payload to its handler. The permission handler therefore keysapprovalCorrelationsRefbypayload.itemId.serverRequest/resolved.params.requestIdidentifies the original server request, and these values can differ. A missed lookup leaves the emitted resolution without the canonicalrequestIdorrequestKind, so Allow, Deny, or Stop can leave the pending approval unresolved.Expose
request.idthroughhandleServerRequestinpackages/effect-codex-app-server/src/client.ts. Use its normalized value for the permission handler'sjsonRpcIdandapprovalCorrelationsRef. UpdatecodexCollabMockPeer.mjsto emit the original wire ID and assert that ID in the integration test.🤖 Prompt for AI Agents