Skip to content

fix(server): a thread with an unknown event type can still be resynced after a downgrade - #14206

Closed
shivamhwp wants to merge 782 commits into
mainfrom
fix/server-replay-skips-unknown-events
Closed

shivamhwp wants to merge 782 commits into
mainfrom
fix/server-replay-skips-unknown-events

Conversation

@shivamhwp

@shivamhwp shivamhwp commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

If someone runs an older build after a newer one, the database can contain a thread event whose type the older build doesn't know. The server still starts, and opening the thread fresh still works. But any client resuming that thread from a cached position before that event, which is common for mobile and for web after sleep, gets "Could not synchronize the thread." and never recovers until a full reload. Every retry replays from the same position and fails the same way.

Client replay (loadReplayThrough in ws.ts) now skips a row whose event type this build doesn't recognize and logs one warning with the thread, the sequence and the type's length (no raw values, per the repo's logging convention). Everything else stays strict: appends, the provider ingestor, projection writes and every other reader still reject unknown types, and a known type with a broken payload still fails. That's the same line #13941 drew on the client side, where the server stays strict about what it accepts.

The live tail resumes from highWater, which is computed separately from the replay, so skipping a row can't skip real work. If the skipped row is the last one, the client's cursor stays just below it, so a later resubscribe skips it again. That repeat is cheap and harmless.

Reproduced first against the real EventStoreV2 and ProjectionMaintenanceV2 code, simulating a realistic downgrade with the row written and the projection cursor advanced past it:

  • server boot: fine
  • fresh open: fine
  • resubscribe from before the row: fails every time, with no retry

The new tests replay across an unknown row in the middle and at the end, and check that a known type with a broken payload still fails. Without the fix, the replay test fails with PersistenceDecodeError.

Related: #8896 (part 2).

Model: Claude Sonnet 5 and Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code

juliusmarminge and others added 30 commits September 24, 2026 22:54
Co-authored-by: Julius Marminge <julius0216@outlook.com>
@github-actions github-actions Bot added the size:M 30-99 changed lines (additions + deletions). label Sep 29, 2026
Comment thread apps/server/src/persistence/Layers/OrchestrationEventStore.ts Outdated
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 4.9 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.4 KiB — 29.3 KiB ✅
Codex Live turn messages — 2 — 8 ✅
Claude Total thread wire — 4.9 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 20.8 KiB — 29.3 KiB ✅
Claude Live turn messages — 2 — 8 ✅

Baseline: unavailable · PR result: 16e4ef2 · Source CI: success

Scenario and decoded snapshot size

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: 106.1 KiB
  • Claude decoded thread snapshot: 106.4 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@macroscopeapp

macroscopeapp Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The specified range contains a very large cross-product change set, including orchestration rewrites, new provider and scheduled-task capabilities, authentication/authorization changes, and deployment configuration changes. The focused unknown-event replay fix is therefore not sufficiently isolated for automatic approval.

Not approved because:

  • Per-review cost limit exceeded (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings, or comment @macroscope-app review this PR to bypass the limit and review now. You can add or adjust custom eligibility rules. Learn more.

@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Sep 29, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 29, 2026
juliusmarminge and others added 13 commits September 28, 2026 20:45
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Merges origin/main (ff1db03, v0.0.44) into t3code/codex-turn-mapping
(cb64162). No new migrations on main.

V1 files V2 deleted (kept deleted; main's changes considered for V2):
- orchestration/Layers/ProviderRuntimeIngestion(.test).ts,
  provider/Layers/CodexAdapter(.test).ts, provider/Layers/ProviderService(.test).ts,
  server.test.ts: #14290 changes. V2 equivalents ported or listed as follow-ups below.
- provider/Layers/OpenCodeAdapter(.test).ts: e518866. OpenCodeAdapterV2
  settles admissionSettled in an ensuring on both the prompt and /compact
  paths, and interrupt waits with a 10s timeout, so it does not hang the same
  way. Not changed here.

Content conflicts (both sides kept unless noted):
- server.ts: V2 layer structure; main's CodexInstallation layer added next to
  AntigravityInstallation; the installation refresh now watches Codex and
  Antigravity (renamed ProviderInstallationRefreshLive, per main).
- ws.ts, contracts rpc.ts/server.ts, settingsSearch.ts, docs/user/install.md
  (Pi row kept, Codex row from main): take both.
- CodexDriver.test.ts, ProviderRegistry.test.ts, ProviderInstanceRegistryLive
  .test.ts: V2's adapter/ID layers plus main's CodexInstallation,
  ServerSecretStore and ServerEnvironmentIdentity; main's mocks rewritten for
  V2's namespace ServerSecretStore import, and V2's real ServerSecretStore
  layer kept where it already provided one.
- desktop main.ts and DesktopClerk.test.ts: V2's pre-ready FileSystem plus main's
  ElectronShell; main's `shell` test parameter appended after V2's
  platform/fileSystemLayer parameters.
- ui/wizard.tsx: main's `size="wide"` variant replaces V2's className escape
  hatch, whose only caller (AddProviderInstanceDialog) now uses size="wide".
  Wide is max-w-3xl, where V2 used max-w-2xl.
- AddProviderInstanceDialog.tsx: V2's ACP/auth wizard structure. Applied main's
  Codex step-0 "Configure manually" and "Continue with ChatGPT" actions, the
  ghost-muted Cancel, and setupMode "existing" on manual Codex saves. Kept V2's
  "Add provider" title and used main's description.
- ProviderInstanceCard.tsx: main's folded Runtime section for managed Codex;
  V2's rule of hiding Runtime when a driver has no settings fields still
  applies to other drivers.
- ProviderSettingsPanel.tsx: main's CodexSetupSection comes before V2's
  generic ProviderAuthenticationSection, because managed Codex also reports
  setup.canAuthenticate.
- WelcomeWizard.tsx: V2's ProviderInstanceIcon with main's displayName
  and padding.
- ThreadErrorBanner.tsx: V2's usage-limit warning variant plus main's ChatGPT
  usage-limit presentation. V2's plain dismiss icon kept.
- ChatView.tsx: V2's timeline error wiring kept. Main's chatGptUsageLimit prop
  dropped (see below).

Ported for #14290 (managed ChatGPT auth):
- CodexAdapterV2 gains an optional resolveRuntime that is resolved on each
  openSession and replaces the launch settings and environment (the
  T3-installed binary, token-sharing launch args, ACCESS_TOKEN and CODEX_HOME).
  Managed sessions drop serviceTier, matching V1.
- CodexManagedProvider builds its orchestrationAdapter with
  createCodexAdapterV2 in place of the deleted V1 makeCodexAdapter. The adapter
  registry already wraps openSession in auth.withAccess.
- CodexTextGeneration: added the Scope import that main's resolveRuntime
  signature needs (V2 had dropped it).
- Live orchestrator tests provide CodexInstallation and ServerEnvironmentIdentity,
  which CodexDriver now requires.
- CodexDriver test: the managed account now also opens a V2 session and checks
  the launch uses the installed binary and account token, and not an ambient
  OPENAI_API_KEY.

Dropped main behavior, because it reads V1 thread activities that V2 removed:
- shared isChatGptUsageLimitError and its test, the web ChatView
  chatGptUsageLimit prop, and mobile ChatGptUsageLimitNotice. The
  ThreadErrorBanner chatGptUsageLimit prop stays, currently unset. Follow-up
  work maps it onto V2 provider failures.

Other merge fixes:
- providerMaintenanceRunner.test.ts: main added CURSOR_DRIVER to a
  compatibility list. V2 no longer updates the Cursor CLI, so it is removed.
- AddProviderInstanceDialog.environment.test.tsx: Codex step 0 now offers
  "Configure manually", and a manual save stores setupMode "existing".
- model-manifest.json and packages/effect-codex-app-server/src/_generated are
  taken from main (#14198 OpenCode 2 incompatible; #14311 Codex 0.159 bindings).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Merges origin/main (0fcd5f9) into t3code/codex-turn-mapping (1f2311c).
Brings in 8792f95 (#14323), which unpins the Codex install advisory in
CodexInstallation.test.ts. No new migrations on main.

V1 files V2 deleted (kept deleted):
- provider/Layers/GrokAdapter(.test).ts: 050cfad (#10607) retires a Grok
  session whose ACP process crashed so the next turn can resume. V2 already
  recovers from this. AcpAdapterV2 marks the runtime for restart on
  termination, fails the in-flight prompt, and the next turn spawns a fresh
  runtime and resumes the same ACP session. Nothing ported. Main's
  T3_ACP_CRASH_PROMPT hook in acp-mock-agent.ts merged cleanly. It is
  env-gated, and nothing in V2 sets it.

Content conflicts:
- ThreadStatusIndicators.tsx: V2's badge props (pullRequests mini-list,
  number/status, onOpenPullRequest with a url argument) plus main's
  onOpenStack -> onOpenList rename. Main's opensList rule (stacks and
  multi-PR badges open the pull requests panel) came in cleanly.
- BranchToolbarBranchSelector.tsx: V2's toolbar-only badge with the
  pullRequests list and per-URL open handler, using main's onOpenList.
- chat/MessagesTimeline.tsx: #12900's intent applied to V2's work-log rows.
  LiveWorkEntryTimelineRow leads with the question text, and shows the answer
  preview only when there is an answer (V2 has no AgentSpawnRow branch).
  SimpleWorkEntryRow keeps V2's reasoning preview. The question text now
  outranks a lone tool row's displayLabel, because V2 gives user-input rows
  tone "tool" and so passes the generic "Input requested" label there. The
  answer preview shows only when answered, and the heading can shrink
  (min-w-0) beside it.
- chat/MessagesTimeline.test.tsx: kept V2's deletion of the answered
  "attachment-only question history" test. Its tone "info" fixture does not
  reach V2's row toggle. Took main's new "leads an unanswered question row
  with the question text" test and added the ApprovalRequestId import it
  needs.

V2-only content such as the Pi entry in model-manifest.json is untouched.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…14213)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: PR Batch Tester <agent@local.test>
A downgraded server can leave application-event rows behind whose
event_type a newer build wrote and this build has never learned. Client
thread resubscription replayed those rows strictly, so the whole
subscription failed instead of just resuming.

Only loadReplayThrough (the client resubscribe path in ws.ts) now opts
OrchestrationEventStore.readAgentEvents into skipUnknownEventTypes,
which logs a warning with thread id, sequence, and event type and
drops the row instead of failing. Everything else that reads this
store - appends, the provider ingestor, projection writes, and
EventStoreV2 - stays strict by default, and a known type with a
payload that fails to decode still fails even with the flag set. The
live tail resumes from a highWater computed independently of replay,
so dropping an unknown row from replay cannot skip past real work
within a session.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Refine pass: one filterMapEffect instead of an Option round trip, shorter
comments, and one replay test covering interspersed and trailing unknown rows.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@shivamhwp
shivamhwp force-pushed the fix/server-replay-skips-unknown-events branch from ff78fd0 to 16e4ef2 Compare September 30, 2026 13:38
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 1, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 1, 2026 05:23

Dismissing prior approval to re-evaluate 16e4ef2

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 1, 2026
Base automatically changed from t3code/codex-turn-mapping to main October 2, 2026 19:22
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 2, 2026 19:22

Dismissing prior approval to re-evaluate 16e4ef2

@macroscopeapp

macroscopeapp Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Macroscope skipped reviewing this pull request. Per-review cost limit exceeded (workspace setting).

This review would cost an estimated $15.26, which exceeds your per-review limit of $15.00.

The top 3 files driving up this estimate:

File Diff Size Estimate
packages/contracts/src/orchestrationV2.ts 127.32KB $2.09
apps/server/src/mcp/OrchestratorMcpService.ts 75.70KB $1.24
apps/server/src/orchestration-v2/EventSink.ts 33.91KB $0.56

Tip

To get this pull request reviewed, you can:

  1. Comment @macroscope-app on this PR to request a manual review (monthly spend limits still apply).
  2. Exclude the file(s) above from review by adding a pattern to your .macroscope/ignore.md — note that creating this file replaces Macroscope's built-in default ignores rather than extending them.
  3. Raise your cost limit in your workspace billing settings.

Turn off this reminder going forward

@shivamhwp

Copy link
Copy Markdown
Collaborator Author

Closing: this targeted the v2 branch, which is now merged and deleted. The bug still reproduces on main, so it's refiled as #15937.

@shivamhwp shivamhwp closed this Oct 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:S 10-29 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.