perf(native-chat): probe WSL transcript paths asynchronously - #13265
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughTranscript path checks now use asynchronous 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: faeb5b8f-373b-456a-be23-1821b8f7293b
📒 Files selected for processing (6)
src/main/native-chat/host-readable-transcript-path.test.tssrc/main/native-chat/host-readable-transcript-path.tssrc/main/native-chat/session-file-resolver-wsl.test.tssrc/main/native-chat/session-file-resolver.tssrc/main/native-chat/transcript-watch-wsl-exact-path.test.tssrc/main/native-chat/transcript-watch.ts
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 593e551c-cf15-49d0-b0ce-65e11cb8f4f1
📒 Files selected for processing (2)
src/main/native-chat/session-file-resolver-codex-roots.test.tssrc/main/native-chat/session-file-resolver.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/main/native-chat/wsl-transcript-fs-gate.test.ts (1)
30-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBoth suites depend on process-wide caches with no reset hook.
wsl-transcript-fs-gate.tsholdsactiveTaskCount,activeScanCount,activeLaneKeys,queuedTasks, andinFlightTasksat module scope, andwsl-codex-session-path-scan.tsholdsinFlightScans. Neither module exposes a reset. Both suites pass only because every test drains its own deferred work. A single failed or timed-out test leaks state into every later test in the same file.
src/main/native-chat/wsl-transcript-fs-gate.test.ts#L30-L42: add abeforeEachthat resets the scheduler counters, lane set, queue, and in-flight map before each test.src/main/native-chat/wsl-codex-session-path-scan.test.ts#L16-L18: extend the existingbeforeEachto clearinFlightScansin addition tomocks.walk.Use
vi.resetModules()with a dynamic import in each suite, or export a test-only reset from each module.src/main/native-chat/wsl-transcript-fs-gate.ts (2)
117-131: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winOne stalled route blocks scans on every other route.
nextTaskIndexallows one active scan across all routes.activeLaneKeysisolates routes, but theactiveScanCount > 0check is global. A scan on a stopped distro holds the single scan slot until itsreaddirreturns, so scans for healthy distros stay queued.The test at
wsl-transcript-fs-gate.test.tsLines 44-57 proves only that exact probes bypass a stalled lane. Scan bypass is not covered.
wsl-codex-session-path-scan.tsissues one gatedreaddirper directory, so a deep tree walk consumes that single slot for its whole traversal.If the global scan cap is intentional, add a comment that records the tradeoff. Otherwise, track the scan count per route.
36-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the shared WSL path parser instead of a second regex.
routeKeyre-implements the UNC match thatparseWslUncPathalready performs insrc/shared/wsl-paths.tsLines 6-17, then falls back toparseWslUncPathfor the same input. The local regex differs only by dropping the trailing-segment group, so the fallback at Line 42 is unreachable for any path the local regex rejects.Extract the provider prefix from a single parser so both files stay consistent.
src/main/native-chat/wsl-codex-session-path-scan.ts (1)
77-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the Codex session-id match rule.
findSessionPathrepeats thefilePredicatebody insession-file-resolver.tsLines 176-206. The WSL branch and the non-WSL branch must agree on thename === sessionId || name.endsWith('-' + sessionId)rule. Two copies can diverge.Export one predicate and use it in both branches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ba9f3ed4-9bdd-4844-88f8-696c2789cc7d
📒 Files selected for processing (12)
src/main/ai-vault/session-scanner-directory-reader.test.tssrc/main/ai-vault/session-scanner-discovery.tssrc/main/native-chat/host-readable-transcript-path-fs-gate.test.tssrc/main/native-chat/host-readable-transcript-path.tssrc/main/native-chat/session-file-resolver-wsl-scan-gate.test.tssrc/main/native-chat/session-file-resolver.tssrc/main/native-chat/transcript-watch-resolve-poll.test.tssrc/main/native-chat/transcript-watch.tssrc/main/native-chat/wsl-codex-session-path-scan.test.tssrc/main/native-chat/wsl-codex-session-path-scan.tssrc/main/native-chat/wsl-transcript-fs-gate.test.tssrc/main/native-chat/wsl-transcript-fs-gate.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/main/native-chat/session-file-resolver-wsl-scan-gate.test.ts
- src/main/native-chat/host-readable-transcript-path-fs-gate.test.ts
- src/main/native-chat/transcript-watch-resolve-poll.test.ts
- src/main/ai-vault/session-scanner-discovery.ts
- src/main/native-chat/session-file-resolver.ts
- src/main/native-chat/host-readable-transcript-path.ts
|
@coderabbitai Fixing. Will be in the next commit |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
6081e45 to
0f263ec
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a29caf1-ef9c-44b7-9c36-788492433ff2
📥 Commits
Reviewing files that changed from the base of the PR and between eacb9a9 and 0f263ec592dae2f2d2360d2995e4b6942a3acf92.
📒 Files selected for processing (21)
src/main/ai-vault/session-scanner-directory-reader.test.tssrc/main/ai-vault/session-scanner-discovery.tssrc/main/ipc/native-chat-subscribe-lifecycle.test.tssrc/main/ipc/native-chat.tssrc/main/native-chat/host-readable-transcript-path-fs-gate.test.tssrc/main/native-chat/host-readable-transcript-path.test.tssrc/main/native-chat/host-readable-transcript-path.tssrc/main/native-chat/session-file-resolver-codex-roots.test.tssrc/main/native-chat/session-file-resolver-wsl-scan-gate.test.tssrc/main/native-chat/session-file-resolver-wsl.test.tssrc/main/native-chat/session-file-resolver.tssrc/main/native-chat/transcript-tail-reader.tssrc/main/native-chat/transcript-watch-resolve-poll.test.tssrc/main/native-chat/transcript-watch-wsl-exact-path.test.tssrc/main/native-chat/transcript-watch.tssrc/main/native-chat/wsl-codex-session-path-scan.test.tssrc/main/native-chat/wsl-codex-session-path-scan.tssrc/main/native-chat/wsl-transcript-fs-gate.test.tssrc/main/native-chat/wsl-transcript-fs-gate.tssrc/main/runtime/rpc/methods/native-chat.test.tssrc/main/runtime/rpc/methods/native-chat.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- src/main/native-chat/session-file-resolver-codex-roots.test.ts
- src/main/native-chat/session-file-resolver-wsl.test.ts
- src/main/native-chat/session-file-resolver-wsl-scan-gate.test.ts
- src/main/native-chat/host-readable-transcript-path-fs-gate.test.ts
- src/main/native-chat/host-readable-transcript-path.test.ts
- src/main/native-chat/wsl-transcript-fs-gate.ts
- src/main/ai-vault/session-scanner-directory-reader.test.ts
- src/main/native-chat/transcript-watch.ts
- src/main/native-chat/wsl-codex-session-path-scan.ts
- src/main/native-chat/transcript-watch-wsl-exact-path.test.ts
- src/main/native-chat/host-readable-transcript-path.ts
- src/main/ai-vault/session-scanner-discovery.ts
- src/main/native-chat/session-file-resolver.ts
The WSL transcript resolvers probed `\wsl.localhost` UNC candidates with existsSync. Those paths are served over 9P, so a stopped or unreachable distro blocked the Electron main thread instead of falling through to the next candidate. - host-readable-transcript-path: the `pathExists` dep is now async, defaulting to fs/promises.access. The per-distro loop stays sequential — the ranked order exists so the owning distro wins, and probing every distro at once would fan 9P calls out to ones the user deliberately left stopped. - session-file-resolver: dropped the existsSync guard outright rather than converting it. walkSessionFiles already yields [] for a missing or unreadable root, so the guard was redundant as well as blocking.
…home codexSessionsDirs() only needs the managed home's path to scan it, but called getOrcaManagedCodexHomePath(), which mkdirSyncs. That put a synchronous fs call back on the 500ms-5s resolve poll the surrounding async probe work just cleared, and materialized the runtime home as a side effect of a read-only lookup. Use the resolve-only variant the module already exposes for this case. A missing root walks to no matches, so behavior is unchanged.
Serializes and dedupes WSL 9P filesystem probes (access/readdir) so a stopped or slow distro can't exhaust the shared libuv threadpool or block unrelated local filesystem work. Routes Codex session-tree scans and path-existence checks through the new wsl-transcript-fs-gate, while sharing in-flight scans across concurrent callers.
- Add cancellation (AbortSignal) throughout session resolution and directory walking so an unsubscribed transcript watch stops in-flight WSL filesystem work instead of leaking it. - Split the WSL fs task gate into exact vs scan priority lanes with per-route concurrency, so a live transcript access probe is never queued behind a directory scan on another distro/provider. - Extract Codex WSL session path scanning into its own module that shares one root snapshot across concurrent session-id lookups and refreshes on a shared miss to see post-start file creation. - Skip the async WSL probe entirely for local paths via existsSync.
0f263ec to
391219b
Compare
1 participant
Summary
Prevents native-chat transcript discovery from blocking Electron's main thread on stopped or unreachable WSL distros. WSL UNC
access/readdirwork now runs asynchronously behind a bounded, priority-aware, cancellation-aware gate; concurrent Codex scans share traversal without retaining unrelated paths or canceled callers. Resolve polling no longer creates the managed Codex home, and IPC/RPC subscription setup/read cancellation now propagates through transcript resolution.Screenshots
No visual change.
Testing
pnpm lint— fresh static-analysis CI passed; local normal, native-plugin, type-aware, reliability, and max-lines checks also pass. The local aggregate command alone reports bundled-skill manifest files that are byte-identical to currentmain.pnpm typecheckpnpm test— 149/149 rebased targeted tests pass, and all 32 Node 24/26 PR test shards pass. The unsharded local Windows run has unrelated platform/environment fixture failures.pnpm buildAI Review Report
Three adversarial review lanes repeatedly audited correctness, performance, tests, and compatibility. They checked async ownership, cancellation before/during/after setup, stale-generation ABA, late callbacks, permanently stalled WSL I/O, queue bounds/fairness, high-cardinality scans, memory retention, Windows/macOS/Linux branching, SSH and folder workspaces, and mixed-version remote clients.
The review found and fixed:
After the fixes, independent rebased-head passes are clean. A range-diff confirmed all five reviewed patches survived the rebase unchanged. Targeted validation passed 149/149 tests, full typecheck, direct lint modes, reliability/max-lines gates, formatting/diff checks, and the production build.
Cross-platform review confirmed WSL-specific behavior remains behind runtime/path checks; local macOS/Linux, SSH, and folder-workspace resolution paths are unchanged. No keyboard/UI behavior is touched.
Security Audit
Reviewed path handling, IPC/RPC lifecycle ownership, cancellation, task deduplication keys, resource exhaustion, command execution, auth, secrets, and remote-wire exposure. The change adds no shell command construction, dependency, credential, auth, schema, or stream-opcode changes. Filesystem work remains read-only and is bounded to two concurrent WSL transcript operations, with one scan slot and identity-safe cleanup. No follow-up security work is required.
Notes
HOME/os.homedir(), POSIX symlink/shell, and Xterm artifact fixture failures outside this PR. Fresh GitHub checks are required before merge.