perf(server): ACP subagent replies no longer rewrite the database on every chunk#29422

External image Minor · Data Integrity & Integration · Quick win

Flush pending subagent text before a restarted child clears it.

When a native child restarts, setNativeChildState clears pendingAssistantMessages without writing the pending text. A delayed write that fires later finds its id no longer pending and returns without writing. The last appended chunks of the previous message can therefore stay unpersisted, and the stored message stays truncated. The newDelegation restart branch in syncNativeChildren does not flush before it resets the child.

Call flushSubagentAssistant for the subagent before the reset:

✅ Addressed in commit 3fbde75

Valid, thanks. I reproduced it with a Claude replay: a child streams Review + done., goes idle while a grandchild is still working (so no status is published and nothing flushes), then restarts. Only Review was ever emitted; done. was dropped when the restart cleared pendingAssistantMessages.

Fixed in 3fbde75b20: setNativeChildState is now an Effect, and it calls flushSubagentAssistant before it resets a restarted child. That covers the newDelegation branch in syncNativeChildren and both state-update paths. Regression test: writes a child's coalesced text before a restart resets it in AcpRegistryAdapterV2.test.ts. It fails on the previous commit and passes now; the 880 tests in the 20 ACP-related server test files pass.

🤖 Generated with Claude Code (Claude Opus 5.5 in wisp)

@M0xoo Thanks for the reproduction and fix. The supplied code confirms that setNativeChildState flushes pending text before clearing the restarted child’s message state. All three call sites yield the effect, so the fix covers each reset path.

The reported regression test exercises the missing case: a child goes idle while a grandchild remains active, then restarts before the delayed flush.

📝 Learnings were not changed: only organization members or users with write access to this repository can add or remove learnings.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Review thread resolved.

_{You are interacting with an AI system.}

External image Major · Data Integrity & Integration · Heavy lift

Flush pending assistant text before terminalization.

When a failed prompt does not set childTurnsContinue, finalizeTurn emits turn.terminal without flushing pendingAssistantMessages. RunExecutionService then stops consuming the provider-event stream. The delayed write may run in sessionScope, but no active subscription consumes its events, so the pending text can remain unpersisted.

The same gap exists in terminalizeCarryoverSubagents: it projects terminal status without flushing pending assistant text. Flush pending messages before both root terminal emission and carryover terminalization.

✅ Addressed in commit 6ddfbbc

Valid, thanks. I reproduced it: with the second and third chunks still waiting on the delayed flush, a failed prompt emitted only 'O' before turn.terminal, so the rest of the answer was never ingested. Fixed in 6ddfbbc288:

• finalizeTurn flushes every subagent's pending text right after closeTextStreams, before turn.terminal.
• projectCarryoverSubagentStatus (used by terminalizeCarryoverSubagents) flushes before projecting the status.

Regression test: emits coalesced subagent text before a failed turn's terminal in AcpAdapterV2.test.ts. It fails on the previous commit (expected 'O' to equal 'ONE') and passes now; the 879 tests in the 20 ACP-related server test files pass.

🤖 Generated with Claude Code (Claude Opus 5.5 in wisp)

@M0xoo Thanks for the reproduction and fix. The supplied code confirms both flushes complete before terminal events:

• finalizeTurn flushes every subagent after closeTextStreams and before turn.terminal.
• projectCarryoverSubagentStatus flushes before projecting carryover status.

These changes address the finding. Your reported regression test covers the failed-prompt case; I did not rerun the tests.

📝 Learnings were not changed: only organization members or users with write access to this repository can add or remove learnings.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Review thread resolved.

_{You are interacting with an AI system.}

External image Major · Data Integrity & Integration · Heavy lift

Wait for in-progress text writes before turn.terminal.

writeSubagentAssistant removes the pending ID while holding textEmissionPermit. If finalization snapshots pendingAssistantMessages after that removal, flushSubagentAssistant performs no write and does not acquire the permit. finalizeTurn can then emit turn.terminal while the delayed writer is paused before emitting its events. Ingestion may stop before the final child text is persisted.

Make finalization wait for writes already holding textEmissionPermit. Add a test that pauses a delayed writer after it removes the pending entry.

✅ Addressed in commit 5795776

Agreed on the ordering gap, fixed in 579577676e. The flush read pendingAssistantMessages without taking textEmissionPermit, so a delayed write that had already removed its entry but not yet emitted wasn't waited for.

• flushSubagentAssistant now always takes the permit and reads the pending set while holding it (Effect.suspend inside withPermit), via an unlocked writer.
• finalizeTurn takes the permit once around flushing all its subagents, so it always waits for a write in progress before turn.terminal. This matches the root stream, where closeTextStreams → emitTextSegment always takes the same permit.

I didn't add the requested test. events is an unbounded queue, so Queue.offer never suspends, and the steps between the delete and the two emits (DateTime.now, building the ordinal and payloads) are synchronous. The writer can only pause there through scheduler preemption, which a test can't trigger deterministically without a test-only hook in the writer. The guarantee is now structural instead. The existing failed-turn and restart regression tests still pass, as do the 880 tests in the 20 ACP-related server test files and vp run check:changed.

🤖 Generated with Claude Code (Claude Opus 5.5 in wisp)

@M0xoo The supplied code addresses the ordering gap. finalizeTurn now acquires textEmissionPermit even when the pending sets are empty. It therefore waits for an in-progress writer to enqueue both text events before emitting turn.terminal.

Effect.suspend keeps the pending-set read inside the permit. The unlocked writer avoids nested permit acquisition.

I accept omitting the test-only hook for this fix. The ordering guarantee is explicit in the structure. I have not independently rerun the reported checks.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Review thread resolved.

_{You are interacting with an AI system.}