Skip to content

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

Merged
awlevin merged 1 commit into
mainfrom
remove-runs/pr2-to-main
Feb 24, 2026
Merged

feat: add pending-interactions tracker and standalone approval endpoints#8407
awlevin merged 1 commit into
mainfrom
remove-runs/pr2-to-main

Conversation

@awlevin
Copy link
Copy Markdown
Contributor

@awlevin awlevin commented Feb 24, 2026

Summary

PR 2/6 in the remove-runs-centralize-messages plan (cherry-picked to main).

  • New pending-interactions tracker: maps requestId → { session, conversationId, confirmationDetails }
  • POST /v1/confirm — resolve pending confirmations by requestId
  • POST /v1/secret — resolve pending secret requests by requestId
  • POST /v1/trust-rules — add trust rules for pending confirmations
  • Hub publisher auto-registers pending interactions on confirmation_request/secret_request events
  • 21 tests covering happy path + error cases

Plan: .private/plans/REMOVE_RUNS_CENTRALIZE_MESSAGES.md


Open with Devin

…nts (#8405)

Co-authored-by: Claude <noreply@anthropic.com>
@awlevin awlevin self-assigned this Feb 24, 2026
@awlevin awlevin merged commit fa3babe into main Feb 24, 2026
@awlevin awlevin deleted the remove-runs/pr2-to-main branch February 24, 2026 23:15
Copy link
Copy Markdown
Contributor

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

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.

🔴 Missing kind validation allows /v1/confirm to consume secret interactions (and vice versa), orphaning the real pending request

The handleConfirm endpoint calls pendingInteractions.resolve(requestId) on line 36 without checking that the interaction's kind is 'confirmation'. Similarly, handleSecret on line 71 doesn't check that kind is 'secret'.

Root Cause and Impact

If a secret_request interaction is registered under a given requestId, and a client mistakenly (or maliciously) sends a POST to /v1/confirm with that same requestId:

  1. pendingInteractions.resolve(requestId) at approval-routes.ts:36 removes the interaction from the map.
  2. interaction.session.handleConfirmationResponse(requestId, decision) is called at approval-routes.ts:44.
  3. Inside session.ts:326, this.prompter.resolveConfirmation(...) looks up the requestId in the confirmation prompter's pending map — but it's not there (the request lives in the secret prompter). It logs a warning and returns silently.
  4. The HTTP response is { accepted: true } — the caller thinks it succeeded.
  5. The secret request is now orphaned: removed from pendingInteractions but never resolved in the secret prompter. The agent loop hangs forever waiting for a secret response that will never come.

The same issue exists in reverse: /v1/secret can consume a confirmation interaction, orphaning the confirmation prompt.

Impact: Cross-kind resolution silently orphans the real pending prompt, causing the agent loop to hang indefinitely (until the permission timeout fires).

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 || interaction.kind !== 'confirmation') {
// If we consumed a non-confirmation interaction, re-register it
if (interaction) pendingInteractions.register(requestId, interaction);
return Response.json(
{ error: 'No pending confirmation found for this requestId' },
{ status: 404 },
);
}
interaction.session.handleConfirmationResponse(requestId, decision);
return Response.json({ accepted: true });
Open in Devin Review

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

Copy link
Copy Markdown

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

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: 940cbcecc9

ℹ️ 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".

Comment on lines +122 to +126
const interaction = pendingInteractions.get(requestId);
if (!interaction) {
return Response.json(
{ error: 'No pending interaction found for this requestId' },
{ status: 404 },
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 Require active confirmation before adding trust rules

handleTrustRule only checks whether the requestId exists in the in-memory tracker, but it never verifies that the confirmation is still pending in the session. Since confirmation prompts can auto-timeout/resolve independently, stale tracker entries can remain and still pass this check, allowing POST /v1/trust-rules to persist a rule after the approval flow is no longer active. That violates the endpoint’s pending-confirmation contract and can apply persistent allow/deny behavior outside the intended decision window.

Useful? React with 👍 / 👎.

Comment on lines +36 to +37
const interaction = pendingInteractions.resolve(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 Validate interaction kind before consuming request IDs

/v1/confirm and /v1/secret immediately call pendingInteractions.resolve(requestId) without checking the stored interaction kind first. If a client accidentally sends a secret requestId to /v1/confirm (or vice versa), the entry is deleted before the mismatch is detected, so the correct endpoint can no longer resolve that prompt and the request is effectively forced to timeout/deny.

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