Repository navigation
fix: an approval runs its stored connection action only once - #137
Conversation
Signed-off-by: Charan Rathore <180254320+charan-rathore@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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
left a comment
There was a problem hiding this comment.
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.
What this changes
An approval-gated connection action could be executed more than once under the same approval.
claimActionkeys 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 sameapprovalIdwith a freshtoolCallIdpassed 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 secondtoolCallIdcould 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
toolCallIdwith a 409. Same-toolCallIdretries still recover their saved result, and this composes with the interrupted-claim reclaim in #135, which reclaims under the sametoolCallId.Tests
New regression test in
tests/connections.test.ts: approve once (runs, receiptdone), then POST the sameapprovalIdunder a newtoolCallIdand 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 --noEmitand eslint clean.