fix(server): preserve message order across lineage reads#24442

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 108.5 KiB
• Claude decoded thread snapshot: 108.8 KiB

External image Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: fix/issue-24335
• File: .coderabbit.config.ts
• Commit: f94282f
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Team
• Run ID: 7e6bca37-b783-4b89-a449-c2ee74c51770

External image Commits

Reviewing files that changed from the base of the PR and between d53873b6a4799e87afd63c8d288ef0089f5a3748 and f94282faa30e1091c1fc8c29f85e6387c06a3920.

External image Files selected for processing: 3

• apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts
• apps/server/src/orchestration-v2/Orchestrator.ts
• apps/server/src/orchestration-v2/testkit/ProviderReplayHarness.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Message dispatch now serializes intake per thread before checking subagent lineage. A deterministic test delays a lineage read and verifies that a later agent message does not receive an earlier run ordinal.

Changes: Message intake ordering

Layer / File(s)	Summary
Serialize message intake apps/server/src/orchestration-v2/Orchestrator.ts	Message dispatch acquires a per-thread intake lock before checking for an app-owned subagent parent. User-created and MCP messages also acquire the parent thread’s dispatch lock when a parent is found.
Test delayed lineage reads apps/server/src/orchestration-v2/testkit/ProviderReplayHarness.ts, apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts	The replay harness accepts a projection-store wrapper. The test pauses a targeted getThread call, dispatches user and agent messages, and verifies their run order.

Priority: External image Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Sequence Diagram(s)

mermaid

1	sequenceDiagram
2	participant User
3	participant Agent
4	participant Orchestrator
5	participant KeyedLock
6	participant ProjectionStore
7	User->>Orchestrator: message.dispatch
8	Orchestrator->>KeyedLock: acquire thread intake lock
9	Orchestrator->>ProjectionStore: getThread for lineage lookup
10	Agent->>Orchestrator: message.dispatch while lookup is pending
11	Orchestrator->>KeyedLock: wait for the same thread intake lock
12	ProjectionStore-->>Orchestrator: lineage result

Possibly related PRs

• pingdotgg/t3code#21813: Both changes modify message-dispatch locking and include parent-lock handling in the dispatch path.

Suggested labels: bugfix, size:M

Suggested reviewers: maria-rcks, macroscopeapp

No concrete issue remains that should block merging. The ordering regression test was not independently run here.

Check name	Status	Explanation
Title check	Passed	The title clearly and concisely summarizes the primary server fix: preserving message order across lineage reads.
Description check	Passed	The description includes all required sections. It explains the race, the implementation, issue approval, focused verification, limitations, and evidence.
Linked Issues check	Passed	[#24335] Objective: keep a user or MCP message's dispatch order ahead of a later agent-queued message for the same thread on the file-database path, so the user message becomes run 1 and the agent mes…
Out of Scope Changes check	Passed	The changes described and visible in the diff are limited to the dispatch-ordering fix and its test support: - Orchestrator.ts: the per-thread intake lock and the restructured message.dispatch path. T…

Full details: Linked Issues check

Explanation

[#24335] Objective: keep a user or MCP message's dispatch order ahead of a later agent-queued message for the same thread on the file-database path, so the user message becomes run 1 and the agent message run 2.

Evidence at head, from the whole-PR diff of apps/server/src/orchestration-v2/Orchestrator.ts:

• A new KeyedLock messageIntake is created per orchestrator layer.
• For every message.dispatch, the handler takes messageIntake.withLock(command.threadId, ...) before the appOwnedSubagentParentThreadId lineage read. The read therefore runs while the thread's intake is held. A later message for the same thread waits on intake and cannot overtake it.
• Inside intake, a user or MCP message resolves the parent thread. If a parent exists, the dispatch runs under threadDispatch.withLock(parentThreadId, dispatch). The child lock is then taken inside that, so the order is intake(child), parent, child. The existing parent-before-child thread lock order is kept.
• Other message sources run dispatch under intake alone. Unrelated threads use different intake keys and are not serialized against each other.

Test evidence: the head file apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts imports Deferred and Fiber, which matches the described deterministic test. The added test body was not read, because the changed-file diff was unavailable and the head file read was truncated before the new test. The regression result rests on the PR description and change summary for that part.

Residual risk, not established either way: a parent-thread operation that calls a child message.dispatch through the public dispatch while holding the parent lock would take intake(child) under the parent lock. That is the reverse of the user path. No such path was inspected, so no deadlock finding is made.

Full details: Out of Scope Changes check

Explanation

The changes described and visible in the diff are limited to the dispatch-ordering fix and its test support:

• Orchestrator.ts: the per-thread intake lock and the restructured message.dispatch path. This implements #24335 directly.
• Orchestrator.control-reads.test.ts: the deterministic overtaking regression test. It uses Deferred and Fiber, which the test needs to hold the lineage read. This supports the fix.
• ProviderReplayHarness.ts: an optional wrapProjectionStore hook, per the change summary. It is used only to gate the projection read in the test. This supports the test.

No unrelated behavior, contract, persistence, provider, or client change was reported or observed. The ProviderReplayHarness.ts and test-file hunks were not read directly, because the changed-file diff was unavailable. This verdict relies on the change summary for those files.

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR