Skip to content

fix(server): queue messages sent during context compaction - #9620

Closed
flamboh wants to merge 1 commit into
pingdotgg:mainfrom
flamboh:fix/queue-messages-during-compaction
Closed

fix(server): queue messages sent during context compaction#9620
flamboh wants to merge 1 commit into
pingdotgg:mainfrom
flamboh:fix/queue-messages-during-compaction

Conversation

@flamboh

@flamboh flamboh commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 Fable 5.1 on behalf of Oliver

Problem

Sending a message while a thread is compacting its context (after /compact) fails with "Wait for context compaction to finish before sending another message." The web client removes the optimistic message and restores the draft, so the message is never delivered. This affects every provider because the guard lives in the orchestration reactor, not in an adapter.

Fix

The reactor now holds messages that arrive during compaction in a per-thread FIFO and sends them once the compaction fiber settles, using the same send path a fresh message takes.

  • Compaction success or a failed compaction that still restores the session to ready both drain the queue in order.
  • If the thread stops or the session ends in error, each queued message gets its own provider.turn.start.failed activity so clients restore the draft instead of silently losing it.
  • A second /compact during compaction keeps its existing failure.
  • The projection now lets a queued message replace the pending /compact turn-start row and keeps that row when the compaction restores the session, so the replayed turn still projects starting and keeps its proposed-plan reference.

No contract, decider, web, or mobile changes. Messages were already persisted immediately, so clients see them optimistically and get acknowledged when the turn starts.

Tests

  • ProviderCommandReactor.test.ts: queued message is sent after a failed compaction restores the session, two messages drain in FIFO order, queued message fails when the thread stops mid-compaction.
  • ProjectionPipeline.test.ts: a turn start deferred behind compaction becomes the pending row after the session restores.

Implemented by GPT-5.6 Sol via codex exec, orchestrated and reviewed by Claude Fable 5.1 in Claude Code.

Note

Queue messages sent during context compaction

Ensures messages sent while context compaction is in progress are queued and retained after the session is restored. Adds an integration test in ProjectionPipeline.test.ts verifying that a deferred non-compaction turn-start request survives compaction completion and preserves its message, proposed-plan metadata, and original request timestamp.

📊 Macroscope summarized 6014e20. 3 files reviewed, 4 issues evaluated, 0 issues filtered, 4 comments posted

🗂️ Filtered Issues

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 4, 2026
@flamboh
flamboh marked this pull request as draft September 4, 2026 07:53
if (compactRequestIds.has(event.payload.threadId)) {
return;
}
compactRequestIds.set(event.payload.threadId, event.payload.messageId);

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.

🟡 Medium Layers/ProjectionPipeline.ts:1294

A failed replacePendingTurnStart leaves compactRequestIds populated even though the database transaction rolled back, so the next /compact for that thread returns at the duplicate check without creating a pending-start row. Move the map update until after replacePendingTurnStart succeeds.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProjectionPipeline.ts around line 1294:

A failed `replacePendingTurnStart` leaves `compactRequestIds` populated even though the database transaction rolled back, so the next `/compact` for that thread returns at the duplicate check without creating a pending-start row. Move the map update until after `replacePendingTurnStart` succeeds.

});
const compactRequestId = compactRequestIds.get(event.payload.threadId);
const pendingTurnBelongsToCompaction =
compactRequestId === undefined ||

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.

🟡 Medium Layers/ProjectionPipeline.ts:1347

After a restart, a restoring ready session deletes a queued message's pending turn-start row, so the queued turn loses its pending-message/proposed-plan metadata and is no longer projected as starting. Because compactRequestIds is in-memory, compactRequestId === undefined must not be treated as proof that the pending row belongs to /compact; only delete it when a known compaction request matches (while terminal sessions can still clear it unconditionally).

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProjectionPipeline.ts around line 1347:

After a restart, a restoring `ready` session deletes a queued message's pending turn-start row, so the queued turn loses its pending-message/proposed-plan metadata and is no longer projected as `starting`. Because `compactRequestIds` is in-memory, `compactRequestId === undefined` must not be treated as proof that the pending row belongs to `/compact`; only delete it when a known compaction request matches (while terminal sessions can still clear it unconditionally).

readonly message: ThreadTitleMessage;
};
// Compaction fibers replay these events in arrival order after restoring the session.
const queuedTurnStarts = new Map<ThreadId, Array<QueuedTurnStart>>();

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.

🟠 High Layers/ProviderCommandReactor.ts:357

When the server restarts during compaction, queued user messages are never delivered and no failure activity is recorded. queuedTurnStarts exists only in memory, while start subscribes to a hot stream, so the already-persisted thread.turn-start-requested events are not replayed after the map is lost; persist the queue or replay pending requests during startup.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderCommandReactor.ts around line 357:

When the server restarts during compaction, queued user messages are never delivered and no failure activity is recorded. `queuedTurnStarts` exists only in memory, while `start` subscribes to a hot stream, so the already-persisted `thread.turn-start-requested` events are not replayed after the map is lost; persist the queue or replay pending requests during startup.

@@ -1400,37 +1449,70 @@ const make = Effect.gen(function* () {
Effect.andThen(restoreCompaction(event.payload.threadId, true)),
Effect.catchCause(recoverCompactionFailure),
Effect.ensuring(Effect.sync(() => void compactingThreadIds.delete(event.payload.threadId))),

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.

🟠 High Layers/ProviderCommandReactor.ts:1451

A turn start arriving after compaction completes but before drainQueuedTurnStarts finishes is sent immediately, so it can reach the provider before earlier queued prompts and breaks the claimed per-thread FIFO order. Effect.ensuring clears compactingThreadIds before the drain runs; keep the thread marked compacting through the entire drain and ensure arrivals during the drain are consumed before releasing it.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderCommandReactor.ts around line 1451:

A turn start arriving after compaction completes but before `drainQueuedTurnStarts` finishes is sent immediately, so it can reach the provider before earlier queued prompts and breaks the claimed per-thread FIFO order. `Effect.ensuring` clears `compactingThreadIds` before the drain runs; keep the thread marked compacting through the entire drain and ensure arrivals during the drain are consumed before releasing it.

@macroscopeapp

macroscopeapp Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This substantial server-side behavior change adds an in-memory per-thread FIFO and modifies compaction/session-restoration projection semantics, allowing prompts to be delivered asynchronously across lifecycle boundaries. Restart, persistence, and ordering edge cases remain material, so the change warrants human review.

Not approved because:

  • 4 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@flamboh

flamboh commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Probably worth doing with orchestrator v2, rather than now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant