[orchestrator-v2] Fix shell cache hydration and multi-env project grouping - #3640
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Needs human review 3 blocking correctness issues found. This PR introduces substantial new logic for shell cache hydration and multi-environment project grouping. Unresolved review comments identify potential bugs including a high-severity issue where cache decoding may silently fail for projects, plus concerns about the persistence loop and identity clearing logic. You can customize Macroscope's approvability policy. Learn more. |
8866ebd to
191408f
Compare
191408f to
d6a0e95
Compare
Dismissing prior approval to re-evaluate d6a0e95
d6a0e95 to
275c738
Compare
275c738 to
a9a74af
Compare
fcce425 to
daa9283
Compare
Persisted shell snapshots encode settledAt as an ISO string. Decode it through DateTimeUtcFromString and cover the non-null cache round trip. Co-authored-by: codex <codex@users.noreply.github.com>
dcb3561 to
6764a41
Compare
| const latestSnapshot = yield* Ref.get(latestLiveSnapshot); | ||
| // Same sequence can still mean newer content (e.g. repository enrichment). | ||
| if (Option.isNone(latestSnapshot) || latestSnapshot.value === nextSnapshot) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🟡 Medium state/shell.ts:96
The persist loop never terminates while shell events keep arriving: after each cache.saveShell completes, it reads latestLiveSnapshot and compares by reference equality. Every new snapshot is a fresh object allocation, so the comparison always fails and the loop starts another save immediately, bypassing the 500 ms debounce and producing continuous storage writes for the entire session. Consider limiting the re-save to only the same-sequence race (stop once the sequence differs from what was just saved) or draining via the queue rather than chasing an indefinitely advancing live reference.
const latestSnapshot = yield* Ref.get(latestLiveSnapshot);
// Same sequence can still mean newer content (e.g. repository enrichment).
- if (Option.isNone(latestSnapshot) || latestSnapshot.value === nextSnapshot) {
+ if (
+ Option.isNone(latestSnapshot) ||
+ latestSnapshot.value === nextSnapshot ||
+ latestSnapshot.value.snapshotSequence !== nextSnapshot.snapshotSequence
+ ) {
return;
}🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/shell.ts around lines 96-100:
The `persist` loop never terminates while shell events keep arriving: after each `cache.saveShell` completes, it reads `latestLiveSnapshot` and compares by reference equality. Every new snapshot is a fresh object allocation, so the comparison always fails and the loop starts another save immediately, bypassing the 500 ms debounce and producing continuous storage writes for the entire session. Consider limiting the re-save to only the same-sequence race (stop once the sequence differs from what was just saved) or draining via the queue rather than chasing an indefinitely advancing live reference.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6764a41. Configure here.
| initial, | ||
| tail: Stream.concat(Stream.concat(replay, completionMarker), liveFrom(highWater)), | ||
| enrichment: enrichmentRefreshes, | ||
| }); |
There was a problem hiding this comment.
Stale shell snapshot after sequence reset
Medium Severity
subscribeShell now loads the initial shell snapshot before reading highWater. If the application sequence moves backward between those reads (server DB rebuild/reset), the already-captured snapshot can stay ahead of the new high water. That stale frame is still emitted as the authoritative prefix, so the client can adopt ghost projects/threads and ignore the post-reset replay and live deltas.
Reviewed by Cursor Bugbot for commit 6764a41. Configure here.
aee7ff3
into
pingdotgg:t3code/codex-turn-mapping
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.
…ojects (#3640) Restore versioned shell-cache hydration, reconcile repository enrichment across multiple environments, and decode non-null settled shell timestamps correctly.


Summary
Problem and Fix
afterSequenceresumes, then continue with replay and live deltas.Defensive Fixes
snapshotmember and add optional metadata. Effect's legacy struct decoder accepts and strips the excess field, covered by a compatibility test.Validation
vp test run packages/contracts/src/orchestrationV2.test.ts packages/client-runtime/src/state/shellReducer.test.ts packages/client-runtime/src/state/shell-sync.test.ts apps/server/src/orchestration-v2/ShellStream.test.ts apps/server/src/project/ProjectEnrichmentService.test.ts: 49 tests passvp fmt --checkandvp lintfor the touched files: pass, with pre-existing inline-schema warnings only@t3tools/contracts,@t3tools/client-runtime, andt3: pass, with pre-existing Effect suggestions onlyNote
Fix shell cache hydration and multi-env project grouping in orchestrator-v2
mergeShellSnapshotProjectsto shellReducer.ts to merge incoming snapshots with prior state, preservingrepositoryIdentityfor unchanged roots in authoritative snapshots and avoiding structural rollbacks for enrichment snapshots at or below the cached sequence.EnvironmentShellStatein shell.ts to track the latest live snapshot and re-save it if a newer same-sequence snapshot arrives while a prior save is in flight; flushes on disconnect and scope teardown without blocking state updates.resolvedRepositoryIdentityRoots, before any enrichment refresh can appear.*Jsonvariants) in orchestrationV2.ts and updates orchestrationCache.ts to use them for persisted shell and thread snapshots, affecting date/timestamp serialization.repositoryIdentityResolvedtoProjectEnrichmentin ProjectEnrichmentService.ts to distinguish resolved (including cached null) from pending/failed identity resolution.Macroscope summarized 6764a41.
Note
Medium Risk
Touches live shell sync, WebSocket ordering, and client cache persistence across server resets and same-sequence enrichment; mistakes could mis-group projects or serve stale shell state, but behavior is heavily test-covered.
Overview
Fixes orchestrator v2 shell sync so clients hydrate from cache reliably, group projects by git identity across environments, and stay correct when server sequence resets or enrichment races the initial snapshot.
Server: HTTP and WebSocket shell paths use non-blocking
getAvailablefor project enrichment and exposerepositoryIdentityResolved. Shell subscribe always rehydrates a full snapshot (includingafterSequenceresumes), emits an unmarked authoritative frame first, optionally a same-sequence frame withresolvedRepositoryIdentityRoots, then merges batched enrichment refreshes only after that prefix viacomposeShellStreamWithEnrichment.Client:
mergeShellSnapshotProjectsdistinguishes authoritative resets (lower sequence still replaces structure) from enrichment-only updates (identity patches, no structural rollback). Shell state tracks the latest live snapshot, flushes on disconnect/scope close without blocking UI, and re-persists when a same-sequence enriched snapshot lands during an in-flight save. Orchestration cache v3 uses JSON-safe snapshot schemas for date fields.ACP: Drops JSON-RPC responses with non-numeric ids before they hit the Effect RPC client (Grok agent-internal messages).
Reviewed by Cursor Bugbot for commit 6764a41. Bugbot is set up for automated code reviews on this repo. Configure here.