Skip to content

fix(confirm-write): approve one write at a time per conversation - #28

Open
jerelvelarde wants to merge 1 commit into
mainfrom
jerel/one-write-at-a-time
Open

fix(confirm-write): approve one write at a time per conversation#28
jerelvelarde wants to merge 1 commit into
mainfrom
jerel/one-write-at-a-time

Conversation

@jerelvelarde

Copy link
Copy Markdown
Collaborator

Follow-up to #23 (merged). Fixes the second production failure in the same approval gate.

The error

RuntimeError: When there are multiple pending interrupts, you must
specify the interrupt id when resuming.

It escaped as an ASGI error and killed the run.

Cause

A model turn can emit several tool calls at once, and LangGraph runs each as its own task. Two mutating calls in one turn therefore both reach copilotkit_interrupt in the same super-step and leave two pending interrupts. Nothing can answer that: the Channel posts one card, and a bare Command(resume=...) is rejected outright.

I reproduced both candidate paths against LangGraph directly:

scenario pending interrupts bare resume
a new turn arrives while a card is unanswered 1 OK
one turn emits two mutating tool calls 2 fails — the production error

So the abandoned-card theory is out: a new turn replaces the pending interrupt rather than stacking. The accumulation is strictly within a single turn.

Fix

Only the first write on a conversation may pause for approval. The rest come back unrun, with a result telling the model to re-issue them — which it does on the next turn. Writes queue behind one card at a time instead of racing for the same resume.

  • The claim is keyed by the call itself, so it survives the replay that resume performs while still turning away a different write from the same turn.
  • It is released only when an approval resolves — approved or declined — so cancelling can't wedge the thread.
  • The gate is shared across interceptors, because each MCP server gets its own. Two writes to different servers in one turn is exactly the case a per-server gate would miss.
  • Reads are never deferred; only approval-gated calls queue.

Verification

End to end with the real interceptor inside a real graph, real interrupt(), two different MCP servers:

pending interrupts after a 2-write turn = 1     (was 2)
bare resume OK                                  (was RuntimeError)
results: ['A:written', 'B:Not run: another write o']
tools actually executed: ['save_project']

77 agent tests pass (7 new): parallel writes deferring, a replayed call reclaiming its own pause, release on both approve and decline, cross-server sharing, reads never deferred, and — with no thread id available — the write still gated rather than silently executed.

Heads-up: main is currently red on the TS side

Not caused by this PR, which is Python-only. On main at 9478b3c, four test files fail to load:

TypeError: defineChannelComponent is not a function
  ❯ app/tools/render-chart.tsx:100:28

The chart work in #24/#26 needs a newer @copilotkit/channels than the installed tree — package.json declares 0.6.1, node_modules has 0.2.2-canary.rc-1. Same skew that makes pnpm check-types report 4 errors and app/cleanup.test.ts fail. Probably wants the dependency bump in #15/#16 to land.

The step-budget / GraphRecursionError fix from the other log is separate, in #27.

A model turn can emit several tool calls at once, and LangGraph runs each
as its own task. Two mutating calls in one turn therefore reach
copilotkit_interrupt in the same super-step and leave two pending
interrupts. Nothing can answer that: the Channel posts one card, and
resuming without an interrupt id raises "When there are multiple pending
interrupts, you must specify the interrupt id when resuming", which
escapes as an ASGI error and kills the run.

Reproduced against LangGraph directly -- two parallel tasks that both
interrupt leave two pending, and a bare Command(resume=...) fails exactly
as it did in production. A second turn arriving while a card is still
unanswered does NOT stack, so the accumulation is within one turn only.

Let only the first write on a thread pause for approval. The rest come
back unrun with a result telling the model to re-issue them, which it
does on the next turn, so the writes queue up behind one card at a time
instead of racing for the same resume. The claim is keyed by the call
itself so it survives the replay that resume performs, and it is released
only when an approval resolves -- approved or declined.

The gate is shared across interceptors because each MCP server gets its
own: two writes to different servers in one turn is precisely the case a
per-server gate would miss.
@jerelvelarde
jerelvelarde force-pushed the jerel/one-write-at-a-time branch from 99d338e to 1dd2129 Compare August 3, 2026 23:42
@jerelvelarde

Copy link
Copy Markdown
Collaborator Author

Correction to the "main is red" note in the description above — that was wrong, and it was my environment.

package.json and pnpm-lock.yaml both pin @copilotkit/channels@0.7.0. My node_modules was still on 0.2.2-canary.rc-1 from the start of the session, which is what produced defineChannelComponent is not a function, the four tsc errors, and the cleanup.test.ts failure. After pnpm install, main at 8ba2265 is completely clean:

Test Files  20 passed (20)
     Tests  162 passed (162)
type errors: 0

So there's nothing to chase in #15/#16 on that account, and the "pre-existing failure" I mentioned in #23 wasn't real either. Sorry for the noise.

Rebased onto 8ba2265. Green on the actual dependencies:

agent:   77 passed
channel: 162 passed
tsc:     0 errors

(The one channel.test.ts failure this branch showed before the rebase came from its old base at 9478b3c and was fixed by your #29 — not from this change, which is Python-only.)

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