fix(import): harden external session boundaries

[wutongyuonce]

Summary

Harden the six remaining external-session boundaries tracked by #5402 while keeping each rule at its existing authority:

• keep Codex filesystem cursors wire-bounded with a fixed-size adapter-owned identity;
• restore source Session ID matching in the shared Core query matcher;
• finish deterministic import projection before the commit-start boundary;
• make the memory execution store hide staged imports like SQLite;
• use one Codex timestamp normalization rule for SQL ordering, cursor positions, and displayed timestamps;
• coalesce TUI search edits while immediately retiring stale in-flight responses.

The English and Chinese external-session design documents now describe the resulting boundaries and normalization contract. No protocol fields, cross-package cache, TTL/LRU, or client recovery ledger were added.

Fixes #5402

Verification

• npm run build
• npm run lint
• npm run format:check
• npm run typecheck
• node --test packages/core/dist/__tests__/external-session-query.test.js (18 passed)
• node --test packages/storage/dist/__tests__/codex-session-adapter.test.js (28 passed)
• node --test packages/storage/dist/__tests__/execution-provider-conformance.test.js (89 passed)
• node --test --test-name-pattern='external|Host external catalog' packages/cli/dist/__tests__/pi-tui-runner.test.js (8 passed)

Review focus

Each commit corresponds to one issue checkbox and its owner-level regression. Please review the authority boundary in that commit rather than as a single cross-layer mechanism.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Codex(GPT 5.6 sol) inspected the affected authority seams, implemented the six fixes, updated the design documents, and added focused regressions. Every authored commit carries a Generated-by: Codex trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[hqhq1025]

The six boundary fixes are directionally correct, and the new regressions fail on the exact base. I found one user-visible query/cursor race plus two remaining lifecycle/reference-backend gaps that should be fixed before merge.

Local verification on Node 24.18.1: build:test, workspace typecheck, Core 847/847, Storage 1395 passed / 11 skipped, CLI 1063 passed / 3 skipped, focused 135/135 plus the new TUI test, lint, format, ASF headers, and git diff --check. The merge tree with current main c980b93a is clean. Hosted label is green; hosted test is still running.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

[P2] Retire the displayed rows and cursor when the query changes. Once the debounce timer has fired, cancelScheduledSearch is already cleared while the new first-page request is still pending, but sessions and nextCursor still belong to the previous query. Selecting Load more in that window calls this branch with the new query and the old query-bound cursor; a real adapter rejects that as invalid_request, and this request also advances revision, so the correct in-flight first page is discarded. On this head I reproduced an initial old-next cursor, typed code, waited for the text=code request to start, then selected Load more and observed { text: 'code', cursor: 'old-next' }. Ordinary stale rows are also still selectable during the same window. Bind displayed choices to the query/revision that produced them, or disable/reload row actions until the current query lands.

[wutongyuonce]

@hqhq1025 Fixed in commit 2884233. A query change now immediately retires the displayed rows and cursor and advances the request revision before debounce. The first page is requested without the old cursor, and stale responses can no longer repaint or paginate the catalog. Added a regression assertion for the old cursor and stale rows.

[hqhq1025]

[P3] Cancel this timer from the runner-wide shutdown path. cancelScheduledSearch is local to this overlay and is only called by closeOverlay() or another picker action; beginClose()/restoreTerminal() cannot reach it. I reproduced typing one search character and immediately emitting SIGTERM: runMakaPiTui() returned and the terminal was stopped, but 120 ms later listSessions({ text: 'x' }) was still invoked. At that point the outer Runtime Host context may already be closing, so shutdown can initiate fresh I/O and then report an error into a closed UI. Register the timer with teardown, or check closed/pageClosed before invoking the surface.

[wutongyuonce]

@hqhq1025 Fixed in commit 2884233. Search timers are now registered with the runner-wide teardown path, and restoreTerminal cancels them before stopping the TUI. The timer also checks closed/pageClosed before starting I/O. Added a SIGTERM-style shutdown regression test.

[hqhq1025]

[P3] Apply the staged-session visibility rule to the Memory provider's public catalog reads as well. This fixes lookupExternalSessionImports, but readCatalogRecord() and selectCatalog() still admit transcriptLedgerVersion: 0, unlike SQLite's catalog predicate. A parameterized production-path probe on this head passed for Local and failed for Memory: after createImportedSession() without publication, lookup returned [], while listCatalogPage() still contained the staged Session and readCatalogRecord() succeeded. That leaves the reference backend able to green-light Host/catalog behavior that production rejects. Add the version predicate to those catalog paths and cover version 0 hidden / version 1 visible in the shared conformance suite.

[wutongyuonce]

@hqhq1025 Fixed in commit 2884233. The Memory provider now applies transcriptLedgerVersion !== 0 to readCatalogRecord and catalog selection, matching SQLite. The shared Local/Memory conformance test now verifies staged imports are hidden and published imports remain readable.

[wutongyuonce]

@hqhq1025 Thanks for the detailed review. I pushed commit 2884233, which addresses all three comments:

• Query changes now retire old rows and the cursor immediately, so the new first-page request cannot reuse a cursor from the previous query.
• External search debounce timers are cancelled by runner-wide teardown before the TUI stops.
• Memory catalog reads and pagination now hide staged imports, matching SQLite; the shared conformance test covers staged versus published visibility.

The hosted test failure is in packages/cli/src/tests/runtime-host-session-driver.test.ts:3810: it expected 2 attempts but observed 3. This PR does not modify that test or runtime-host-session-driver.ts, and the same test passed on the main baseline. Local CLI and Storage checks pass, including the new regressions.

[hqhq1025]

Re-reviewed at 2884233db5a45dc2a2d9c2801f49bd40cd1453ec.

The three previously reported boundary gaps are resolved:

• pi-tui-runner.ts now clears the visible rows, selection map, and pagination cursor synchronously when the query or scope changes. An old row or query-bound cursor therefore cannot remain actionable while the replacement first page is debounced or in flight.
• Pending external-catalog search timers are registered with runner teardown, and delayed callbacks are fenced by the runner/page closed state.
• The Memory reference store now excludes staged imports (transcriptLedgerVersion: 0) from catalog listing, pagination, and direct catalog reads, matching the SQLite backend. The provider conformance test exercises both implementations.

I also ran the three regressions against the prior reviewed head. The old code sends the new query with the old cursor, starts the delayed search after shutdown, and exposes the staged record through Memory while Local hides it. The current head passes all three paths.

I found no P0-P3 issues in the current diff. Local verification passed build:test, full workspace typecheck, Core 847/847, Storage 1395 passed / 11 skipped, CLI 1064 passed / 3 skipped, focused CLI 221/221, Storage conformance 89/89, lint, format, ASF headers, changed-file Biome, and git diff --check. The head is directly based on current main, and the hosted test check is successful. I did not exercise real Claude/Codex/OpenCode data directories, a remote Host, or native Windows/macOS terminal behavior.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Astro-Han]

All six tracker slices verified closed at their owning authority:

• Cursor bound: identity is now a 43-char sha256/base64url derivation, cursors ~80B regardless of nesting depth, and (mtimeMs DESC, identity ASC) keeps deterministic keyset continuation. Equal-mtime ordering moves from path-lexical to hash order — a display-order change only, documented.
• Shared matcher: externalSessionMatchesQuery matches summary.id with the same normalizer; both Desktop and TUI send raw text to the Host and consume the shared matcher, no parallel client filtering.
• Projection ordering: projectSessionCatalogMessages runs before onCommitStarted() in both stores; a projection failure is now a definite pre-commit failure that creates no staged Session, and the conformance test pins it on both backends.
• Memory store: staged Sessions are excluded from import counts, catalog record reads, and paged catalog listing, matching SQLite.
• Timestamp authority: one normalizeEpochMs registered as a SQLite UDF drives SQL ordering, cursor position, and display. This is the right shape — the normalization rule now lives in one place and SQL consumes it rather than redeclaring a parallel rule.
• TUI search: 120ms debounce coalesces keystrokes, ++revision + resetCatalogPage retires in-flight responses immediately, and every revision bump drops the pending timer — no race.

One P2 inline, P3s below.

P3 — f: cursor shape is unversioned (②, Host upgrade crossing an open pagination session). decodeCatalogKeyset accepts any 43-char base64url fourth segment; an old-format pathKey of the right length passes the check and silently resumes at a wrong position (skipped/duplicated rows). Bump the tag to f2: or reject the legacy shape explicitly. codex-session-adapter.ts ~1147.

P3 — memory lookupExternalSessionImports uses === 1 where SQLite uses COALESCE(...,1) <> 0 (②, artificial header patch). The same file's selectCatalog already uses !== 0; align it. memory-execution-session.ts:448.

P3 — conformance fixture does not pin the predicate on Local (test gap). The staged import uses [] messages, so Local's INNER JOIN would exclude it with or without the ledger predicate. Stage a non-empty message. execution-provider-conformance.test.ts:662-699.

P3 — memory append now projects canonical values while SQLite appendMessages projects raw input (② boundary inputs). Identical for well-formed messages; divergent for inputs decodeCanonicalMessage rewrites (ts/type). memory-execution-session.ts:264-285 vs session-store.ts:729.

P3 — dead branch: dropScheduledSearch() in the external:load-more arm is unreachable — a pending search implies nextCursor is null. pi-tui-runner.ts:3243-3246.

P3 — no normalized-term dedup (①): keystrokes that do not change the normalized term (trailing space, case-only) still clear rows and rescan after 120ms. if (text === query) return in onQuery removes the flash and the redundant scan, at the cost of losing keystroke-as-retry.

P3 — stale docs: ExternalSessionQuery.text JSDoc still says "title and cwd" (no id); createImportedSession contract does not state the onCommitStarted boundary semantics the conformance test pins.

Not introduced here (noting only): TUI applyQuery uses toLocaleLowerCase() while the shared matcher uses toLowerCase() — pre-existing divergence under e.g. Turkish locale.

[Astro-Han]

P2 (import-in-flight / pre-recovery window): selectCatalog also serves memory list(), so this predicate now hides staged Sessions from list() — but SQLite's non-paged list() has no ledger predicate at all (metadata.list filters only subagentParent + role; the store-level post-filter blocks only conversationCopy==='preparing'). Result: a staged import Session is visible in the real backend's list() (which feeds session-manager listings) but invisible in the memory double — the two stores now diverge in the opposite direction from the one this PR fixed. Since a staged Session is not a usable Session, the likely fix is on the SQLite side: COALESCE(transcriptLedgerVersion,1) <> 0 on metadata.list. Either way the two stores must agree.

[Astro-Han]

Comment-only review. Overall I think this is a good, well-scoped hardening PR: each of the six changes keeps its existing authority, and the two places where untrusted input crosses a boundary (the Codex cursor and the untrusted timestamp column) got strictly tighter. No correctness or security defects found; nits below.

What I verified

Codex filesystem cursor is now genuinely wire-bounded (codex-session-adapter.ts)
The f: keyset embedded base64url(relativePath), so the encoded length grew with directory depth. The Host enforces EXTERNAL_SESSION_CURSOR_MAX_BYTES = 512 in cursor() (packages/runtime-host/src/protocol/external-session.ts:51, applied at :356) and decodeExternalSessionCatalogQueryResult runs cursor(result.nextCursor) per page (:216), so an oversized cursor made the whole page frame invalid — one deeply nested CODEX_HOME layout failed the entire catalog query rather than one row. Replacing the path with a 43-char sha256/base64url digest makes the cursor fixed-size (f: + 22-char query hash + 11-char mtime + 43-char digest), which is the right fix.

• pathIdentity is only ever compared (compareRolloutCandidates, rolloutCandidateIsAfter) and re-emitted; no path material reaches the wire at all, which is defense-in-depth even though the old value was never used for I/O. resolveRolloutPath's realpath containment under CODEX_HOME (codex-session-adapter.ts:319-331) is untouched, so the symlink-escape boundary is unchanged.
• Decode-side validation is strict (parts.length !== 4, /^[A-Za-z0-9_-]{43}$/) and the alphabet stays inside the wire regex /^[A-Za-z0-9][A-Za-z0-9:_-]*$/, so the cursor can't be rejected by the protocol layer after the adapter accepted it.
• Dropping localeCompare for digest order also removes a locale-dependent ordering, which is a small correctness win: two hosts with different ICU data no longer disagree on tie order.

One epoch normalizer, cursor position still derived from SQL
registerCodexEpochNormalization is called on both connections that build codexThreadQuery SQL (readStateCatalogKeysetPage, readCodexThreadRows), so there is no path that emits maka_codex_epoch_ms(...) unregistered. The function is per-connection on a readOnly handle, so nothing is written into the untrusted Codex DB. sort_key is still selected with the row and read back via codexRowSortTimestamp, so "the position a cursor names == the position the query ordered by" still holds by construction, and coalesce(..., 0) keeps the expression non-NULL so the keyset expr < ? still admits rows whose timestamps are unparseable.

Import commit boundary
Moving projectSessionCatalogMessages(...) ahead of options.onCommitStarted?.() in both session-store.ts:createImportedSession and the memory store is the substance of the fix: previously the argument was evaluated after the callback, so a deterministic projection throw set commitAttempted = true and the Host classified it as commit_outcome_unknown ("check the Session list before retrying") plus a spurious requestDrain() — for a failure that committed nothing (external-session-coordinator.ts:#importSession). Now it lands as source_unreadable, matching the doc.

Memory/SQLite parity
SQLite already excluded staged imports (sqlite-session-catalog-query.ts:45, the <> 0 in readCatalogRecord, and the <> 0 in lookupExternalSessionImports); the memory store now does the same, including omitting zero-match source ids like SQLite does. Recovery is unaffected because listForRecovery still delegates to listHeaders, so recover() keeps seeing version-0 sessions. The project() refactor also makes the memory preview/lastMessageAt derive from the canonical stored values instead of the raw inputs, which matches what SQLite stores.

Core matcher + TUI debounce
The new summary.id clause goes through the same normalizeExternalSessionMatchText/normalizeExternalSessionQueryText pair as title and cwd, so NFC/case/separator folding can't diverge between the three. On the TUI side I traced the revision/pageClosed/cancelScheduledExternalSearches state machine and could not find a path where a stale response repaints or paginates: every edit bumps revision before the debounce, closeOverlay/toggleScope drop the timer and retire in-flight work, the debounce fires with its own requestRevision, and the shutdown cancel in restoreTerminal() runs on every exit path (beginClose).

Nits

1. codex-session-adapter.test.ts:661 now passes by digest luck. 'filesystem keyset paging uses one path order across equal-mtime pages' asserts ['codex_a'] then ['codex-a']. Under localeCompare that followed from path order; now it follows from sha256, and it happens to still hold — sha256('s/2026/08/08/rollout-…-codex_a.jsonl') → lTy4D57-… vs sha256('…-codex-a.jsonl') → ugGNEUKR…, so l < u. Nothing in the test states that invariant any more, and renaming either fixture id would fail it for a reason unrelated to keyset correctness. The new deep-nesting test does this correctly (asserts the set across both pages); the same shape, or a comment that the tie order is digest-defined, would pin the real contract.

2. The verification command skipped 6 of the ~14 external-session TUI tests. --test-name-pattern='external|Host external catalog' matches exactly 8 of them (which lines up with the "8 passed" in the description), and the ones it misses include several that drive code this PR edits — onSelect → closeOverlay() and the load() signature: 'opens the latest imported task without importing another copy' (:5990), 'reports the catalog task id when opening the latest import fails' (:6060), 'opens an existing task while the Host imports another copy of its source' (:6399), 'reports the durable Session id when import succeeds but opening fails' (:6509), plus 'allows another import after an unknown outcome…' (:6211) and 'keeps Maka sessions usable when Host external source discovery fails' (:6614). Worth running the whole file before merge.

3. Per-row JS in the SQL ordering key costs ~2x on a full scan. Measured on a synthetic 20k-row threads table (Node 26, same DB, same keyset page query): ~3.6 ms/query with the maka_codex_epoch_ms UDF vs ~1.8 ms with the previous pure-SQL CAST(... AS REAL) expression. There's no index on the expression, so this scales linearly with the size of Codex's threads table and is paid on every page. Fine if the single-normalizer rule is worth it — but if it shows up, the fast path could stay pure SQL (CAST, plus SQLite's own unixepoch()/strftime() for ISO text) with the UDF as the fallback for shapes SQL can't parse.

4. transcriptLedgerVersion === 1 vs SQLite's COALESCE(..., 1) <> 0 in lookupExternalSessionImports (test-only/memory-execution-session.ts). Behaviourally equivalent for anything the memory store can produce (headers always carry 0 or 1), but SQLite's predicate deliberately treats a missing field as published for legacy rows. !== 0 would mirror the SQL literally and stay correct if the memory store ever loads a pre-field header.

5. No test pins the new f: cursor shape. The decode-side validation changed from a 320-byte/path-regex/canonical-base64url check to a fixed 43-char alphabet check, and nothing asserts rejection of a near-miss (42 or 44 chars, or $/+ in the digest). external-session-coordinator.test.ts:158 covers a structurally bogus cursor (not-a-codex-cursor), which fails earlier on the query hash. One small case beside the deep-nesting test would pin this boundary, since it's the field a client controls.

6. String.prototype.replace patching in execution-provider-conformance.test.ts (<backend>: imported message projection finishes before commit starts). It's correctly restored in finally and node:test runs a file's tests sequentially today, so it's safe as written — but it mutates a global builtin across an await window, and any future concurrency option on that file (or an unrelated rejection escaping) turns it into a cross-test landmine. A seam — an injected projection function, or asserting on a message shape that legitimately throws — would be more robust. Related: the user-visible half of this fix is the Host error code (source_unreadable vs commit_outcome_unknown + drainRequests()), and the coordinator fixture's fake store calls onCommitStarted() itself (external-session-coordinator.test.ts:1073/:1094), so nothing currently pins that classification for a projection failure. One coordinator-level case would close the loop.

[wutongyuonce]

[@Astro-Han] Thanks for the detailed review. I pushed e8590d4fe with the remaining boundary and review fixes.

Addressed:

• SQLite ordinary list() now hides staged imports (transcriptLedgerVersion === 0) while the recovery/header path remains unchanged.
• Filesystem cursors use the versioned f2: tag; legacy f: cursors and malformed digest shapes are rejected, with regression coverage.
• Memory import lookup now matches SQLite with transcriptLedgerVersion !== 0.
• The conformance fixture uses non-empty staged imports and asserts ordinary list() visibility on both Local and Memory.
• SQLite append canonicalizes messages before both persistence and catalog projection.
• The unreachable load-more debounce branch was removed.
• TUI search now compares normalized terms, so whitespace/case-only edits do not clear rows or rescan the source.
• The projection-failure test uses node:test t.mock.method instead of manually mutating/restoring the global prototype.
• Added a coordinator-level regression proving pre-commit projection failure maps to source_unreadable without draining, while an explicit post-commit failure maps to commit_outcome_unknown.
• Runtime Host Session search now reuses core foldForMatch, removing the locale-dependent toLocaleLowerCase() divergence.
• Updated the English/Chinese design docs and contracts.

I also benchmarked the SQL UDF concern. On a synthetic 100k-row table, the current UDF query was about 176 ms for ten runs, while the tested MATERIALIZED CTE alternative was about 247 ms and does not preserve a simpler equivalent for all mixed timestamp formats. I therefore kept the single-normalizer UDF path rather than introducing a slower or semantically divergent optimization.

Validation passed locally: CLI TUI 221 tests, Storage conformance 113 tests, Runtime Host external-session 24 tests, Runtime Host execution composition 33 tests, Core/Storage/Runtime Host/CLI typechecks and builds, full Biome lint, and git diff --check.