Repository navigation
Recover the working directory for imported Gemini sessions - #974
Conversation
PR Summary by QodoRecover working directories for imported Gemini sessions
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Gemini names its workspace only in the <session_context> bootstrap that opens a recording — the header carries a project hash, and the per-project tmp dir is that same hash — so import had no directory to place a session with. Capture scope could not match excluded_paths or excluded_repos against one, and an allow list, which does not admit what it cannot place, skipped every Gemini session. Discovery now reads the bootstrap and carries the directory onto DiscoveredSession.Cwd, so the one central gate that scopes every other source scopes these too, and session-start forwards cwd and workspace_root the way the live hook does. A recording that names no workspace stays unplaceable. --cwd now compares whole paths against the recorded workspace rather than the project dir's basename, which is the tmp-dir hash and matched nothing. Antigravity stays unplaceable: nothing in its transcript records a workspace.
The model writes into the same transcript the workspace is read from, so a turn that opens with a syntactically valid <session_context> block would hand capture scope an invented directory — enough to place a session the profile's lists meant to exclude. Only a user record is ever a bootstrap, which is the rule the server normalizer already applies to the same block.
2973cd4 to
c7d0428
Compare
| var scanned = 0; | ||
|
|
||
| try { | ||
| foreach (var line in File.ReadLines(path)) { |
There was a problem hiding this comment.
ReadHead now keeps File.ReadLines open while it scans and parses up to eight records of a Gemini-owned transcript. File.ReadLines opens via StreamReader with FileShare.Read, which denies a concurrent writer on Windows; Gemini appending during import can hit a sharing violation (or this read can fail), even though the later ReadTranscriptStatsAsync path correctly uses FileShare.ReadWrite. Please open this head scan with a FileStream using FileShare.ReadWrite and a StreamReader too. The old one-line header read had the same underlying risk, but this change lengthens that write-denying window while doing workspace extraction.
There was a problem hiding this comment.
Confirmed and fixed in 820443b.
Switched to File.ReadLinesShared, Core's streamed shared read (FileShare.ReadWrite | FileShare.Delete), which is what the other transcript scans already use — SecondaryRepoRoots, WatchCommand, and ImportCommand's evidence read. Its own doc comment names this exact failure: File.ReadLines opens FileShare.Read, which "on Windows is mandatory sharing and locks a coding agent out of the file it owns".
Your point about the lengthened window is the part that made it worth fixing rather than noting. The previous one-line header read opened and closed inside a single MoveNext; the head scan holds the enumerator across up to eight records while JSON-parsing each one, so the window now spans the workspace extraction rather than a single read.
Gemini suites after the change: Core 37, Unit 80, Integration 9, all passing.
There was a problem hiding this comment.
Verified on 820443b: ReadHead now uses File.ReadLinesShared, whose stream opens with FileShare.ReadWrite | FileShare.Delete. This resolves the Windows sharing finding. I am approving this head.
realtonyyoung
left a comment
realtonyyoung
left a comment
There was a problem hiding this comment.
One actionable Windows file-sharing issue in the new transcript head scan; details are in the inline comment. Static review only; I did not build or run tests.
The head scan holds the file open across up to eight records while parsing each, and File.ReadLines opens FileShare.Read — mandatory sharing on Windows, so a Gemini process appending to the transcript it owns is locked out for that window. File.ReadLinesShared is the streamed shared read the rest of the transcript scans already use.
|
NO FINDINGS on 820443b. Re-reviewed the Gemini workspace extraction and verified the shared-read fix resolves my prior Windows file-sharing finding. Static review only; I did not build or run tests. |
realtonyyoung
left a comment
realtonyyoung
left a comment
There was a problem hiding this comment.
Approved after static re-review; the prior finding is resolved.
2 participants
AI-2849 — no GitHub issue (Linear-first for our own work)
What & why
Gemini names its workspace only in the
<session_context>bootstrap that opens a recording: the header carries a project hash, and the per-project tmp dir is that same hash. Import therefore had no directory to place a session with, soexcluded_pathsandexcluded_reposnever matched one — and an allow list, which does not admit what it cannot place, skipped every Gemini session. Discovery now reads the bootstrap ontoDiscoveredSession.Cwd, so the central capture-scope gate scopes these like every other source, and session-start forwardscwd/workspace_rootas the live hook does. Antigravity stays unplaceable: nothing in its transcript records a workspace.Where to look
GeminiSessionContextrejects far more than it accepts — a turn merely quoting the tag, a drifted heading, an empty list all read as "no workspace". A guessed directory would admit a session the allow list meant to exclude, so unplaceable is the safe answer.--cwdrides on the same recovery, matching whole paths against the recorded workspace; the project-dir basename it can otherwise see is the tmp-dir hash.Verification
13 new tests. Each behavioural guarantee falsified by sabotaging the code it covers:
cwddropped from session-start--treenode-filter '/*/*/*Gemini*/*': 80 unit, 9 integration, 36 core — 0 failing.