Skip to content

feat: add pending-interactions tracker and standalone approval endpoints#8405

Merged
awlevin merged 1 commit into
feature/remove-runs-centralize-messagesfrom
remove-runs/pr2
Feb 24, 2026
Merged

feat: add pending-interactions tracker and standalone approval endpoints#8405
awlevin merged 1 commit into
feature/remove-runs-centralize-messagesfrom
remove-runs/pr2

Conversation

@awlevin

@awlevin awlevin commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

PR 2/6: remove-runs plan. Adds pending-interactions tracker, POST /v1/confirm, /v1/secret, /v1/trust-rules endpoints.


Open with Devin

@awlevin awlevin self-assigned this Feb 24, 2026
@awlevin awlevin merged commit 7b0a697 into feature/remove-runs-centralize-messages Feb 24, 2026
@awlevin awlevin deleted the remove-runs/pr2 branch February 24, 2026 23:14
awlevin added a commit that referenced this pull request Feb 24, 2026
…nts (#8405)

Co-authored-by: Claude <noreply@anthropic.com>
awlevin added a commit that referenced this pull request Feb 24, 2026
…nts (#8405) (#8407)

Co-authored-by: Claude <noreply@anthropic.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

Comment on lines +36 to +45
const interaction = pendingInteractions.resolve(requestId);
if (!interaction) {
return Response.json(
{ error: 'No pending interaction found for this requestId' },
{ status: 404 },
);
}

interaction.session.handleConfirmationResponse(requestId, decision);
return Response.json({ accepted: true });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 handleConfirm and handleSecret don't validate interaction kind before consuming it

The handleConfirm endpoint resolves (removes) a pending interaction from the map and calls handleConfirmationResponse, but never checks that interaction.kind === 'confirmation'. Similarly, handleSecret resolves and calls handleSecretResponse without checking interaction.kind === 'secret'.

Root Cause and Impact

If a client sends a confirmation decision to a requestId that belongs to a secret interaction (or vice versa), the following happens:

  1. pendingInteractions.resolve(requestId) at approval-routes.ts:36 removes the entry from the map
  2. interaction.session.handleConfirmationResponse(requestId, decision) at approval-routes.ts:44 is called, but the session's PermissionPrompter has no pending confirmation for that requestId — it only has a pending secret in the SecretPrompter
  3. The prompter logs a warning ('No pending prompt for confirmation response' at permissions/prompter.ts:108) and returns without doing anything
  4. The pending interaction is already consumed from the map, so the client can never resolve it via the correct /v1/secret endpoint
  5. The secret request in the session's SecretPrompter is permanently stuck, and the response { accepted: true } misleads the client into thinking the operation succeeded

The same issue applies symmetrically to handleSecret being called with a confirmation requestId.

Suggested change
const interaction = pendingInteractions.resolve(requestId);
if (!interaction) {
return Response.json(
{ error: 'No pending interaction found for this requestId' },
{ status: 404 },
);
}
interaction.session.handleConfirmationResponse(requestId, decision);
return Response.json({ accepted: true });
const interaction = pendingInteractions.resolve(requestId);
if (!interaction) {
return Response.json(
{ error: 'No pending interaction found for this requestId' },
{ status: 404 },
);
}
if (interaction.kind !== 'confirmation') {
// Re-register the interaction so it can be resolved via the correct endpoint
pendingInteractions.register(requestId, interaction);
return Response.json(
{ error: 'This requestId is for a secret interaction, not a confirmation' },
{ status: 400 },
);
}
interaction.session.handleConfirmationResponse(requestId, decision);
return Response.json({ accepted: true });
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e3fd71661

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

);
}

const interaction = pendingInteractions.resolve(requestId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject mismatched interaction type before consuming requestId

handleConfirm removes the entry with pendingInteractions.resolve(requestId) without validating interaction.kind, and handleSecret follows the same pattern. If a client posts a secret requestId to /v1/confirm (or a confirmation requestId to /v1/secret), the tracker entry is deleted first, then Session.handleConfirmationResponse/handleSecretResponse no-ops because the corresponding prompter has no pending request, leaving the original prompt unrecoverable. Validate kind and reject with a conflict response before consuming the interaction.

Useful? React with 👍 / 👎.

Comment on lines +122 to +123
const interaction = pendingInteractions.get(requestId);
if (!interaction) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Verify confirmation is still pending before adding trust rule

handleTrustRule gates rule creation on pendingInteractions.get(requestId) only, but never checks whether that confirmation is still pending in the session. Since this tracker is only cleaned by pendingInteractions.resolve(...) (used by /v1/confirm and /v1/secret), entries can become stale when confirmations resolve via timeout or another channel, and addRule(...) can still succeed for an already-finished request. Require interaction.kind === 'confirmation' and interaction.session.hasPendingConfirmation(requestId) before accepting trust-rule writes.

Useful? React with 👍 / 👎.

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.

1 participant