fix(codex): surface app permission requests as approvable - #7861
fix(codex): surface app permission requests as approvable#7861Exotic209093 wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| threadId: event.payload.threadId, | ||
| requestId, | ||
| turnId, | ||
| createdAt: event.payload.createdAt, |
There was a problem hiding this comment.
🟡 Medium Layers/ProviderCommandReactor.ts:1272
The synthetic user-input.resolved activity can be ordered before its user-input.requested activity, leaving the request pending and causing a spurious “User input cancelled” resolution later. pendingUserInputRequests sorts by createdAt and then activity ID, but this uses the interrupt's client timestamp, which may precede the provider request or tie and sort before it. Use a server-side timestamp guaranteed to follow the request, or make resolution ordering independent of timestamps.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderCommandReactor.ts around line 1272:
The synthetic `user-input.resolved` activity can be ordered before its `user-input.requested` activity, leaving the request pending and causing a spurious “User input cancelled” resolution later. `pendingUserInputRequests` sorts by `createdAt` and then activity ID, but this uses the interrupt's client timestamp, which may precede the provider request or tie and sort before it. Use a server-side timestamp guaranteed to follow the request, or make resolution ordering independent of timestamps.
| yield* providerService.interruptTurn({ threadId: event.payload.threadId }); | ||
| // Some providers discard their callbacks without emitting a matching | ||
| // resolution event. Close those requests after the interrupt succeeds. | ||
| yield* Effect.forEach(pendingUserInputRequests(thread.activities), ({ requestId, turnId }) => |
There was a problem hiding this comment.
🟡 Medium Layers/ProviderCommandReactor.ts:1267
processTurnInterruptRequested can append a synthetic user-input.resolved with cancelled: true after the provider has already emitted a real resolution for the same request, causing the activity feed and downstream folds to report an answered prompt as cancelled. pendingUserInputRequests(thread.activities) uses the pre-interrupt snapshot, so re-read the thread after interruptTurn before appending cancellations (or make the append idempotent by request ID).
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderCommandReactor.ts around line 1267:
`processTurnInterruptRequested` can append a synthetic `user-input.resolved` with `cancelled: true` after the provider has already emitted a real resolution for the same request, causing the activity feed and downstream folds to report an answered prompt as cancelled. `pendingUserInputRequests(thread.activities)` uses the pre-interrupt snapshot, so re-read the thread after `interruptTurn` before appending cancellations (or make the append idempotent by request ID).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbd0869b8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "item/permissions/requestApproval": { | ||
| const payload = readPayload( | ||
| EffectCodexSchema.ServerRequest__PermissionsRequestApprovalParams, | ||
| event.payload, | ||
| ); | ||
| return payload?.reason ?? undefined; |
There was a problem hiding this comment.
When reason is absent—which the protocol schema explicitly permits—or does not enumerate the requested capabilities, this returns no useful detail. runtimeEventToActivities subsequently drops the request's args, so web and mobile only show a generic “App permission approval” card even though approving may grant filesystem or network access, potentially for the entire session. Summarize payload.permissions into the canonical request detail or carry a structured canonical permission field so users can review what they are granting without adding Codex-specific parsing to each client.
AGENTS.md reference: AGENTS.md:L146-L146
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dbd0869. Configure here.
| turnId, | ||
| createdAt: event.payload.createdAt, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Interrupt leaves permission approvals open
Medium Severity
After interrupt succeeds, this path only synthesizes cancelled user-input.resolved activities. Pending permission approvals (and other approval kinds) are left open: Codex interruptTurn does not settle pendingApprovalsRef, and no matching approval.resolved is appended. The new app-permission card can stay visible after Stop, and a late allow/deny may answer a request the interrupted turn already abandoned.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit dbd0869. Configure here.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a new capability - permission approval requests - with new runtime logic and user-facing UI. There are also unresolved Medium-severity findings about interrupt not settling pending permission approvals, which warrants human attention. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
Codex Apps ask for extra permissions via the JSON-RPC method item/permissions/requestApproval. The Codex adapter had no handler or request-type mapping for it, so the request fell through to methodNotFound and the tool call stalled with no approval UI outside Auto mode. Add a permission_approval canonical request type and a matching permission request kind, handle the method in CodexSessionRuntime (approve grants the requested profile, deny answers an empty grant), map it in CodexAdapter, and thread the kind through ingestion plus the web and mobile approval folds so the card renders everywhere. ox-alpha via opencode
dbd0869 to
0fe0748
Compare
1 participant
Problem
Codex Apps (connectors) request extra permissions over JSON-RPC via
item/permissions/requestApproval. The Codex adapter had no handler or request-type mapping for that method, so the request fell through tomethodNotFoundand the tool call stalled with no approval UI in any mode except Auto (whereapprovalsReviewer: "auto_review"answers it server-side).Fixes #7825
How I fixed it
permission_approvalto the canonical request types (packages/contracts/src/providerRuntime.ts) and a matching"permission"kind toProviderRequestKind(packages/contracts/src/orchestration.ts).item/permissions/requestApprovalhandler inCodexSessionRuntime.ts: it parks a pending approval, emits a canonical request event, and translates the user decision into the protocol response — accept grants the requested permission profile (acceptForSessionsets scope to session), decline answers with an empty grant.CodexAdapter.ts(toRequestTypeFromMethod,toRequestTypeFromKind) plus detail extraction from the request'sreason.ProviderRuntimeIngestion.ts(activity summaries/payloads) and both client folds — websession-logic.ts/ComposerPendingApprovalPanel.tsxand mobilethreadActivity.ts/thread-work-log.tsx(new lock icon) — so the approval card renders with allow/deny on every surface.request.openedevent withrequestType: "permission_approval".Verification
vp test runon CodexAdapter.test.ts (28 tests incl. the new one), CodexSessionRuntime.test.ts, ProviderRuntimeIngestion tests, web session-logic/ComposerPendingApprovalPanel/MessagesTimeline.logic tests — all pass.Notes
The teardown-settlement gap suggested in the issue did not reproduce: Codex's runtime already settles pending approvals as cancelled in its
closepath (settlePendingApprovals("cancel")), mirroring the Cursor/Grok pattern, so no change was needed there.ox-alpha via opencode
Note
Medium Risk
Touches the Codex approval path that can grant app permissions (e.g. network). Also changes interrupt handling so pending user-input prompts are auto-cancelled.
Overview
Codex app connectors can now request extra permissions (
item/permissions/requestApproval) through the same allow/deny approval UI as commands and file changes, instead of stalling withmethodNotFound.Adds a
permissionrequest kind andpermission_approvaltype, handles the JSON-RPC method in the Codex session runtime (accept grants the requested profile; decline returns an empty grant;acceptForSessionscopes to the session), and threads it through ingestion plus web/mobile pending-approval cards (lock icon on mobile).Separately, interrupting a turn now closes leftover user-input prompts with a cancelled
user-input.resolvedactivity, because some providers drop those callbacks without a matching event.Reviewed by Cursor Bugbot for commit dbd0869. Configure here.
Note
Add app permission approval as first-class approvable request kind
permissiontoProviderRequestKindandpermission_approvaltoCanonicalRequestTypein orchestration.ts and providerRuntime.tsitem/permissions/requestApprovalthrough the adapter and session runtime in CodexAdapter.ts and CodexSessionRuntime.ts, emittingrequest.openedevents and awaiting a decision before responding with granted permissionsrequestKindFromRequestTypeandrequestKindFromCanonicalRequestTypenow return a newpermissionvalue forpermission_approval; any out-of-tree consumers of these unions must handle the new literalMacroscope summarized 0fe0748.