Skip to content

fix: an approval runs its stored connection action only once - #137

Merged
jerelvelarde merged 1 commit into
CopilotKit:mainfrom
charan-rathore:fix/approval-single-use
Oct 14, 2026
Merged

jerelvelarde merged 1 commit into
CopilotKit:mainfrom
charan-rathore:fix/approval-single-use

Conversation

@charan-rathore

Copy link
Copy Markdown
Contributor

What this changes

An approval-gated connection action could be executed more than once under the same approval.

claimAction keys execution claims on (threadId, toolCallId), which stops a double click or a retried request from running twice. But nothing bound the approval itself to one run: after an approved action completed, POSTing the same approvalId with a fresh toolCallId passed every check (approval found, conversation matches, inside the one-hour TTL, tool still bound) and ran the stored request again. While the first run was still in flight, a second toolCallId could even start a concurrent execution of the same approved call.

The POST path now looks up the action an approval was already claimed for and refuses a different toolCallId with a 409. Same-toolCallId retries still recover their saved result, and this composes with the interrupted-claim reclaim in #135, which reclaims under the same toolCallId.

Tests

New regression test in tests/connections.test.ts: approve once (runs, receipt done), then POST the same approvalId under a new toolCallId and expect a 409 with the send mock called exactly once. The test failed before the change (the stored request ran a second time) and passes after. Full suite: 303 passed across 48 files, tsc --noEmit and eslint clean.

Signed-off-by: Charan Rathore <180254320+charan-rathore@users.noreply.github.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@charan-rathore

Copy link
Copy Markdown
Contributor Author

This is green on CI and conflict-free on my end. Is there anything you'd like changed before it can land? Happy to adjust.

@jerelvelarde jerelvelarde left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a useful, narrowly scoped authorization fix: one stored approval can no longer dispatch its MCP action again under a fresh toolCallId, while same-id requests retain receipt recovery. The synchronous lookup and claim occur before dispatch; in-flight overlap, cross-conversation reuse, persisted claims, and existing disabled-tool/expiry checks were verified with local mocks. No actionable correctness or security findings. CONTRIBUTING.md is satisfied for this server-only change; the repository has no pull-request template, and the description accurately explains the defect and regression coverage.

Verified exact head 9c8b41a: 303 tests across 48 files, formatter, lint, typecheck, production build. Additional overlap/cross-thread diagnostics and database-reopen probe passed. Prospective integration with main 4650232 merged cleanly with matching newly installed lockfile dependencies; formatter, lint, typecheck and build pass. The integrated suite passes all 308 tests across 50 files with two workers. The first concurrent run had a telemetry child-process timeout; its isolated 8-test file and the bounded full rerun passed. All execution used credential-free, network-restricted Node 24; no live providers or messages were invoked.

@jerelvelarde
jerelvelarde merged commit 831f330 into CopilotKit:main Oct 14, 2026
1 check passed
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.

2 participants