fix(server): skip imports of native agent sessions

[Gigioxx]

Problem

Bulk import could create a second thread for a Claude or Codex session that a native T3 thread already owned. The importer only checked the generated import: thread ID, so the same session showed up twice in one environment (#10933).

Change

Read runtime bindings once per import batch to skip known native sessions, scoped to the provider instance. Check native ownership again atomically in the SQLite statement that reserves each import binding (INSERT ... SELECT ... WHERE NOT EXISTS), so bindings created during transcript scanning are caught too. The same guarded statement runs when a retry reuses a stopped reservation, and upsert reports whether publication is allowed. The importer skips publication when a native owner exists. Ordinary retries with no native owner still recover.

This does not remove existing duplicates or deduplicate across environments. Project discovery keeps its path-based meaning; #10631 handles individual Claude attachment.

Scope and approval

Fixes the same-environment duplication in #10933. Maintainer triage identifies the bulk importer as the verified defect and makes a session-level discovery flag optional: #10933 (comment).

Verification

• Original: reproduced duplicate creation before the first fix and the concurrent binding race before the follow-up, for both Codex and Claude. The retry-bypass fix from review is covered by the rechecks a reserved ... cases below.
• Re-ran in this session after rebasing onto current main:
  • vp test run src/project/AgentSessionImporter.test.ts src/provider/Layers/ProviderSessionDirectory.test.ts src/serverRuntimeStartup.reconcile.test.ts in apps/server: 59 passed. This includes rechecks a reserved {codex,claudeAgent} import with native owner in {same instance,other instance,none}, unchanged native threads, no duplicate bindings, and provider-instance isolation.
  • tsc --noEmit in apps/server: no errors. vp fmt --check on the changed files is clean, and vp lint reports only existing warnings outside the changed lines.
• Not checked: a real import against live Claude or Codex session stores. No UI change.

Original change made by GPT-6 via Codex. Rebase and verification refresh by Claude Opus 5.5 via Claude Code.

[macroscopeapp]

Approvability

Verdict: Approved at f27bba3

Macroscope's review found this PR approvable — This is a focused server bug fix that prevents duplicate native-session imports while preserving ordinary retries and provider-instance isolation. The shared persistence change is localized, atomic, and supported by targeted integration coverage.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 2840732d-ab2a-4524-ab37-571177c70dd5

📥 Commits

Reviewing files that changed from the base of the PR and between 2c3095f and a28e060.

📒 Files selected for processing (10)

• apps/server/src/persistence/ProviderSessionRuntime.ts
• apps/server/src/project/AgentSessionImporter.test.ts
• apps/server/src/project/AgentSessionImporter.ts
• apps/server/src/provider/Layers/CodexAdapter.test.ts
• apps/server/src/provider/Layers/OpenCodeAdapter.test.ts
• apps/server/src/provider/Layers/ProviderService.test.ts
• apps/server/src/provider/Layers/ProviderSessionDirectory.ts
• apps/server/src/provider/Services/ProviderSessionDirectory.ts
• apps/server/src/server.test.ts
• apps/server/src/serverRuntimeStartup.reconcile.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

The importer now checks native Claude and Codex session bindings before creating imported threads. It skips matching sessions and uses reservation results to handle native ownership that appears during import.

Changes

Native session import deduplication

Layer / File(s)	Summary
Native reservation guard apps/server/src/persistence/ProviderSessionRuntime.ts, apps/server/src/provider/Services/ProviderSessionDirectory.ts, apps/server/src/provider/Layers/ProviderSessionDirectory.ts	The ignore-conflict upsert accepts unlessNativeSessionId and returns whether it inserted a reservation.
Native binding detection and import skip apps/server/src/project/AgentSessionImporter.ts	The importer extracts valid Claude and Codex session identifiers from non-import bindings. Matching sessions are skipped. Reservation results also skip imports when native ownership appears during reservation.
Deduplication validation and contract updates apps/server/src/project/AgentSessionImporter.test.ts, apps/server/src/provider/Layers/*test.ts, apps/server/src/server.test.ts, apps/server/src/serverRuntimeStartup.reconcile.test.ts	Tests and mocks use the boolean upsert contract. Integration tests cover native ownership before scanning, during retry, across provider instances, and when ownership is absent.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg, juliusmarminge

Sequence Diagram(s)

sequenceDiagram
  participant AgentSessionImporter
  participant ProviderSessionDirectory
  participant ProviderSessionRuntime
  participant ThreadStore
  AgentSessionImporter->>ProviderSessionDirectory: list native bindings
  ProviderSessionDirectory-->>AgentSessionImporter: return native session identifiers
  AgentSessionImporter->>ProviderSessionDirectory: reserve with unlessNativeSessionId
  ProviderSessionDirectory->>ProviderSessionRuntime: execute guarded upsert
  ProviderSessionRuntime-->>ProviderSessionDirectory: return inserted boolean
  ProviderSessionDirectory-->>AgentSessionImporter: return reservation result
  AgentSessionImporter->>ThreadStore: create imported thread when reservation succeeds

Loading

Merge Risk: ⚪ Minimal · up to a28e0

The importer now avoids duplicate native sessions while preserving normal import retries and provider-instance isolation. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 10 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 #10933 requires native-session deduplication by durable provider identity, scoped to the provider instance, while preserving import-namespace deduplication and retry behavior. `AgentSessionImpor…
Out of Scope Changes check	✅ Passed	The changes stay within Issue #10933. They update importer ownership checks, the persistence and directory contracts required for the atomic reservation, and related test doubles and integration tests…
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing imports of native agent sessions. It uses a valid conventional commit format and is specific to the server fix.
Description check	✅ Passed	The description includes all required sections. It explains the problem, implementation, scope and approval, targeted verification results, limitations, and agent attribution. The verification details…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[Gigioxx]

Reviewed CodeRabbit's linked-issue warning. Keeping project discovery unchanged is intentional: alreadyImported is defined as project-path state, not session ownership. The maintainer triage explicitly makes a session-level flag optional and identifies the bulk importer as the verified same-environment defect: #10933 (comment).

Removing an entire project candidate because it contains native history would also hide unrelated importable sessions. This PR protects the shared bulk-import path, including an atomic native-ownership check when reserving a new import binding. Session-level discovery UX and cross-environment deduplication remain separate scope.

[shivamhwp]

Note: GPT-6 on behalf of shivam (@shivamhwp).

The native-ownership check is bypassed when retrying an import that already reserved its binding but failed before creating the thread. If the native binding appears after listBindings() during that retry, existingBinding is present, so the guarded INSERT never runs and the importer publishes a second thread. This still occurs for both Codex and Claude.

Apply the ownership guard when reusing a stopped reservation as well as when inserting a new one. Its result must tell the importer whether publication is allowed; checking that the import binding exists cannot distinguish an old reservation from one allowed by the current ownership check. Preserve ordinary retry recovery when no native owner exists.

[Gigioxx]

[claude-opus-5-5] Responding on behalf of Guille

@shivamhwp addressed in f27bba3 (fix(server): recheck native ownership when retrying imports, rebased onto current main). The importer now always calls the guarded reservation, including when reusing a stopped reservation from a failed earlier attempt. The insert uses INSERT ... SELECT ... WHERE NOT EXISTS (native owner) with a no-op ON CONFLICT DO UPDATE ... RETURNING, so upsert returns true only when the current ownership check allows publication (new or reused reservation) and false when a native binding owns the session. The importer skips publication on false. An ordinary retry with no native owner still recovers.

Regression coverage: rechecks a reserved {codex,claudeAgent} import with native owner in {same instance,other instance,none} in AgentSessionImporter.test.ts. vp test run on AgentSessionImporter.test.ts, ProviderSessionDirectory.test.ts and serverRuntimeStartup.reconcile.test.ts (59 passed), plus server tsc --noEmit, are clean on f27bba3.

[juliusmarminge]

Thanks for working on this. We merged the orchestrator V2 rewrite in #2829, and we are closing this PR as part of that transition.

This change touches ProviderSessionDirectory.ts, ProviderSessionDirectory.ts, which the V2 merge removed. V2 replaces ProviderService and ProviderSessionDirectory with its session manager.

Sorry for the extra work this creates. If the change is still needed on V2, please rebuild it on current main, verify it there, and open a new PR linking back here. We're closing the current implementation without assuming the underlying request is resolved.