Skip to content

fix(server): skip imports of native agent sessions - #10950

Open
Gigioxx wants to merge 2 commits into
pingdotgg:mainfrom
Gigioxx:fix/native-session-import-dedup
Open

fix(server): skip imports of native agent sessions#10950
Gigioxx wants to merge 2 commits into
pingdotgg:mainfrom
Gigioxx:fix/native-session-import-dedup

Conversation

@Gigioxx

@Gigioxx Gigioxx commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Bulk import could create a second thread for a Claude or Codex session already owned by a native T3 thread. The importer only checked the generated import: thread ID.

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 insert that reserves each new import binding, covering bindings created during transcript scanning. Existing import retries keep their current behavior.

Fixes #10933's same-environment duplication. This does not remove existing duplicates or deduplicate across environments. Project discovery keeps its existing path-based meaning; related #10631 handles individual Claude attachment.

Validation: reproduced duplicate creation before the initial fix and reproduced the concurrent binding race before this follow-up, for both providers. All 26 importer and directory tests pass, including unchanged native threads, no duplicate bindings, and provider-instance isolation. Server typecheck and targeted lint pass. No UI changes.

Implemented with GPT-6 in Codex.

Summary by CodeRabbit

  • Bug Fixes
    • Prevented native Claude and Codex sessions from being imported again.
    • Existing native sessions are now skipped without creating duplicate imported threads or bindings.
    • Improved session import handling when sessions are already associated with another provider instance.
    • Session imports now safely handle timing differences when native session associations are created during an import.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 9, 2026
return;
}
const thread = outcome.thread;
if (nativeSessions.has(`${thread.providerInstanceId}\0${thread.providerSessionId}`)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium project/AgentSessionImporter.ts:188

The importer creates an import: thread even when a native Claude/Codex session has been started or resumed for the same provider session after listBindings() runs, producing duplicate conversations with different threadId values. Because nativeSessions is only a one-time snapshot, and insert-ignore applies to the import threadId, this race is not prevented; perform the native-session check atomically with the import/reservation (or under the relevant binding lock).

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/project/AgentSessionImporter.ts around line 188:

The importer creates an `import:` thread even when a native Claude/Codex session has been started or resumed for the same provider session after `listBindings()` runs, producing duplicate conversations with different `threadId` values. Because `nativeSessions` is only a one-time snapshot, and insert-ignore applies to the import `threadId`, this race is not prevented; perform the native-session check atomically with the import/reservation (or under the relevant binding lock).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed. Both Claude and Codex regression cases failed when a native binding was persisted immediately before the import reservation. The insert-ignore reservation now checks native ownership in the same SQLite INSERT ... SELECT WHERE NOT EXISTS statement; the importer skips publication when no binding was reserved. The batch snapshot remains a fast path. All 26 importer and directory tests pass, including provider-instance isolation; server typecheck and targeted lint pass.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

@macroscopeapp

macroscopeapp Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Would Approve

Macroscope's review found this PR approvable — This is a localized, well-tested bug fix that prevents native Claude/Codex sessions from being imported as duplicate conversations without changing schemas or product defaults. An unresolved Medium finding identifies a possible concurrent-import race, which should be addressed separately as a correctness concern.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

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: 4e667f5f-55d9-457f-b5d8-64a8c833d8ac

📥 Commits

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

📒 Files selected for processing (4)
  • apps/server/src/persistence/ProviderSessionRuntime.ts
  • apps/server/src/project/AgentSessionImporter.test.ts
  • apps/server/src/project/AgentSessionImporter.ts
  • apps/server/src/provider/Services/ProviderSessionDirectory.ts

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


📝 Walkthrough

Walkthrough

The importer now checks native provider bindings before creating imported threads. It extracts Claude and Codex session identifiers, skips matching sessions, and protects the reservation against concurrent native bindings.

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
The ignore-conflict upsert accepts unlessNativeSessionId and skips insertion when a matching non-imported native session exists.
Native binding detection and import skip
apps/server/src/project/AgentSessionImporter.ts
The importer collects valid Claude and Codex session identifiers from non-import bindings. Matching sessions are skipped and counted. A post-upsert binding check handles native bindings created during import.
Deduplication validation
apps/server/src/project/AgentSessionImporter.test.ts
Tests cover Codex and Claude sources, native bindings created before scanning or reservation, provider instances, and empty binding responses.

Priority: ➖ Normal

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

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 2c309

Bulk imports now skip sessions already owned by matching native threads while preserving imports for other provider instances. The duplicate-prevention behavior and reservation race are covered, with no remaining concrete merge-blocking risk.

Suggested reviewers: t3dotgg, juliusmarminge

Sequence Diagram(s)

sequenceDiagram
  participant AgentSessionImporter
  participant ProviderSessionDirectory
  participant ProviderSessionRuntime
  participant ThreadStore
  AgentSessionImporter->>ProviderSessionDirectory: list native bindings
  ProviderSessionDirectory-->>AgentSessionImporter: native session identifiers
  AgentSessionImporter->>ThreadStore: inspect imported thread
  AgentSessionImporter->>ProviderSessionDirectory: upsert with unlessNativeSessionId
  ProviderSessionDirectory->>ProviderSessionRuntime: reserve imported session
  ProviderSessionRuntime-->>ProviderSessionDirectory: create or skip reservation
  ProviderSessionDirectory-->>AgentSessionImporter: binding state
  AgentSessionImporter-->>ThreadStore: create imported thread only when reservation succeeds
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #10933 by detecting native Claude and Codex sessions, scoping checks to the provider instance, preventing atomic reservation races, and preserving import retry behavior. The …
Out of Scope Changes check ✅ Passed The changed importer, persistence, provider directory, and test files directly support native-session detection and duplicate-import prevention. No unrelated code or UI changes are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4…
Title check ✅ Passed The title clearly and concisely describes the primary change: preventing imports of native agent sessions.
Description check ✅ Passed The description explains what changed, why it changed, scope limits, validation, and the absence of UI changes. It does not use the template headings or explicit checklist, but it provides the require…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@Gigioxx

Gigioxx commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

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.

@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 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.

Imported agent sessions duplicate T3-native threads: import dedup never consults the native thread namespace

1 participant