Skip to content

fix(server): preserve imported Codex titles in orchestrator V2 - #14906

Open
debugrror wants to merge 1 commit into
pingdotgg:mainfrom
debugrror:fix/codex-imported-thread-titles-v2
Open

debugrror wants to merge 1 commit into
pingdotgg:mainfrom
debugrror:fix/codex-imported-thread-titles-v2

Conversation

@debugrror

@debugrror debugrror commented Oct 2, 2026 •

Copy link
Copy Markdown

Problem

Importing a Codex session whose first user record contains injected setup can still name the thread <recommended_plugins>, <environment_context>, or an AGENTS instruction heading on Orchestrator V2. Codex shows the saved chat name for the same session. Existing imports retain those placeholders when their transcripts have not changed.

Change

Read the configured Codex home's session_index.jsonl and prefer its saved title, matched by the authoritative session ID from transcript metadata. Reads are bounded to 16 MiB and 5,000 index entries; malformed or unavailable indexes fall back to the first actual user request after recognized leading Codex context. Imported message text is preserved.

Repair recognized placeholders on existing imports through V2's normal metadata command. Two optional command preconditions protect concurrent manual renames and title regeneration. Repair failures leave import bookkeeping intact. Eligibility checks preserve custom titles and exclude other projects, provider instances, deleted threads, and native V2 history. Normal V2 metadata updates advance the thread's metadata timestamp; conversation and settled timestamps remain unchanged.

Scope and approval

Closes #10513, the accepted bug report.

Replaces #11919 after the maintainer requested a fresh implementation on current V2 main. Rebuilt on bf7121d; the V1 dispatch and projection APIs from the old PR are not used.

Verification

Node 24.19.0 on macOS. Regression tests reproduced wrong canonical/fallback titles and unsafe stale title writes before the fixes.

  • vp test run apps/server/src/project/AgentSessionScanner.test.ts: 95 passed; the existing excludes sandboxes reached through a symlink into the worktrees dir test fails identically on clean base b4d3d51 on this machine. Independent diagnosis confirmed that macOS resolves the candidate from /var/folders/... to /private/var/folders/..., while configured exclusion roots retain /var/...; that pre-existing path-canonicalization mismatch is outside this title fix. No test was disabled or changed to hide the failure. New title/index/context regressions passed, including filename/session-ID disagreement, account isolation, malformed/duplicate entries, index growth, and preserved history.
  • vp test run apps/server/src/project/AgentSessionImporter.test.ts apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts apps/server/src/orchestration-v2/ThreadTitleRegenerationService.test.ts packages/contracts/src/orchestrationV2.test.ts: 66 passed. Covers existing-import repair, manual-title preservation, regeneration and rename races, and continued tracking after repair failure.
  • Scoped tsc --noEmit for server and contracts: passed.
  • Lint, formatting, and whitespace checks on the eight changed files: passed with one pre-existing unused-variable lint warning in Orchestrator.ts.
  • Two independent agent reviews found no remaining blocking issues. No client UI changes or live-data migration were performed; browser verification and the repo-wide suite were not run.

Implemented with GPT-6.1-Sol through the Codex harness in T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 2, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 2, 2026 — with ChatGPT Codex Connector
@macroscopeapp

macroscopeapp Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 0400517

Macroscope's review found this PR approvable — This is a focused server-side bug fix that preserves canonical Codex titles, repairs only recognized legacy placeholders, and protects repairs from concurrent renames or regeneration. The new file reads and command fields are bounded and backward-compatible, with extensive regression coverage.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 3786c6bf-c833-40a1-af63-b951a3ca098e

📥 Commits

Reviewing files that changed from the base of the PR and between bf7121d and 0400517.

📒 Files selected for processing (8)
  • apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts
  • apps/server/src/orchestration-v2/Orchestrator.ts
  • apps/server/src/project/AgentSessionImporter.test.ts
  • apps/server/src/project/AgentSessionImporter.ts
  • apps/server/src/project/AgentSessionScanner.test.ts
  • apps/server/src/project/AgentSessionScanner.ts
  • packages/contracts/src/orchestrationV2.test.ts
  • packages/contracts/src/orchestrationV2.ts

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


📝 Walkthrough

Walkthrough

Codex imports now use session-index titles or derive titles from transcript content. Eligible legacy imported threads can be renamed using guarded metadata updates that reject stale title or regeneration-request values.

Changes

Codex title discovery

Layer / File(s) Summary
Discover and derive imported titles
apps/server/src/project/AgentSessionScanner.ts, apps/server/src/project/AgentSessionScanner.test.ts
The scanner parses bounded Codex session-index data and uses indexed titles when available. It derives fallback titles after skipping recognized Codex context and AGENTS instructions, while keeping those messages in imported history. Tests cover index parsing, title derivation, and bounded reads.

Guarded legacy title repair

Layer / File(s) Summary
Guard metadata updates
packages/contracts/src/orchestrationV2.ts, packages/contracts/src/orchestrationV2.test.ts, apps/server/src/orchestration-v2/Orchestrator.ts, apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts
Metadata-update commands can include expected title and regeneration-request values. The orchestrator rejects a mismatch before applying the update.
Repair legacy imported titles
apps/server/src/project/AgentSessionImporter.ts, apps/server/src/project/AgentSessionImporter.test.ts
The importer repairs eligible Codex legacy titles from a canonical title or imported user message. It skips ineligible threads and logs and ignores repair failures.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant AgentSessionScanner
  participant AgentSessionImporter
  participant Orchestrator
  AgentSessionScanner->>AgentSessionImporter: provide canonical title or imported thread
  AgentSessionImporter->>Orchestrator: dispatch guarded metadata update
Loading

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: ⚪ Minimal · up to 04005

Codex import titles and guarded legacy-title repair have no identified merge-blocking issue. The change is ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 04005

The change is limited to imported chat titles and adds safeguards against concurrent renaming and title generation. No privilege expansion or new security vulnerability was established. Commit-time provider eligibility and broader integration coverage remain partly unresolved.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A writer able to influence the configured session index can influence imported display titles. The inspected title flow does not select credentials, grant execution privileges, change workspace ownership, or replace the native resume binding. Its demonstrated new reach is thread-title metadata.

Trust Boundaries and Controls

  • observed — Filesystem input is bounded to 16 MiB, 5,000 index entries, and 100-character titles. The reader rejects oversized or changing files. Transcript workspace identity is checked before title application, and repair eligibility excludes mismatched projects or provider instances, native history, deleted threads, active regeneration, and non-placeholder titles.
  • inferred — Provider-instance eligibility is checked before dispatch but is not included in the command preconditions. A concurrent provider switch that leaves the title and regeneration state unchanged can therefore precede repair. The switch preserves the same thread and project, and repair preserves the switched provider; no cross-owner access or privilege change was established.

Resilience and Maintainability Implications

  • observed — Same-thread commands hold a serialization lock through durable commit. Title and regeneration mismatches reject repair before mutation. Events, projections, effects, and command receipts are committed in one transaction; accepted command replay returns stored results rather than applying the repair again.

Hardening Proposals

  • proposed — If provider-instance eligibility must remain true at commit, add an expected-provider precondition or recheck repair eligibility within the serialized command path. This would strengthen the stated exclusion guarantee; it is not remediation for a verified security vulnerability.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #10513 requires Codex imports to use the saved task title or derive a title from the actual user request after excluding injected context. The PR adds bounded parsing of Codex `session_index.jso…
Out of Scope Changes check ✅ Passed The changes stay within issue #10513. The orchestration metadata preconditions, contract fields, importer repair logic, and related tests support safe repair of existing imported titles and prevent ov…
Title check ✅ Passed The title clearly and concisely describes the main change: preserving imported Codex titles in Orchestrator V2.
Description check ✅ Passed The description covers the problem, implementation, scope and approval, verification results, known pre-existing test failure, and unrun checks. It is specific and complete.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
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:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Imported Codex tasks use <recommended_plugins> as their titles

2 participants