[devswha]

Summary

Copilot CLI 1.0 opens its own process log a second time while it copies it to rotate (about every 10 MiB). A Copilot Chat lookup that landed in that moment saw two descriptors on process-<start>-<pid>.log and stored copilot_log_ambiguous, which kept Chat unavailable (Terminal fallback) until Copilot registered a session again. Now a second descriptor counts as that one log when it resolves to the same path and reaches the same file (device and inode) the reader opened; a path renamed and reused in between, or two different logs of the same process, are still ambiguous.

This is the one part of #3086 that cannot pick a session: the reader still reads exactly one file, the same one it read before. Nothing in the rotation-recovery path changes. Credit to @YuuuXie, whose #3086 started this work; the rest of #3086 stays there.

How Copilot 1.0.105 rotates

Measured by driving its own native logger (runnerCreateFileLogger in runtime.node) in a scratch folder:

• It renames the copies up (.1.log to .2.log and so on, keeping four).
• It writes the new copy to .1.log.rotation-staging and renames it to .1.log, then empties the open log in place.
• While it copies, /proc/<pid>/fd shows the open log twice.

Validation

• New test reads a log Copilot opens a second time to copy it as the one log fails on main (copilot_log_ambiguous) and passes here. It also checks that a later registration still binds, that a different file reported under the same path is still ambiguous (this part fails on the first commit, 68eb323, which compared paths only; Codex reproduced that path-reuse race), and that a second log is still ambiguous.
• bun test server/copilot.test.ts: 96 pass.
• bun run check fast: green.
• Real-logger harness (run on 68eb323; 8 more runs on the final head gave the same: no wrong session, bound to A after the rotations, B after its registration) (scratch, not committed): Copilot's own logger writes and rotates 4 times while the reader looks up the session continuously, then registers B. Over 20 interleaved runs each for main and this branch:
  • Wrong session (A shown after B's line was on disk): 0 on main, 0 here.
  • Unavailable after the rotations (copilot_log_ambiguous until the next registration): 2/20 on main, 0/20 here.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 7 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: fix/copilot-log-second-descriptor
• File: .coderabbit.yaml
• Commit: 624d801
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 2792b100-05ce-469a-b16a-b24936273898

Commits

Reviewing files that changed from the base of the PR and between 554332a and 624d801.

Files selected for processing: 4

• changelog.d/copilot-log-second-descriptor.md
• docs/copilot-chat.md
• server/copilot-store.ts
• server/copilot.test.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[devswha]

Review notes for 624d801 (omo CLEAR; Codex Terra xhigh run twice):

• Path reuse race (Codex, HIGH on 68eb323, fixed in 624d801). Equal readlink strings did not prove the same file. The second descriptor is now skipped only when /proc/<pid>/fd/<n> reaches the device and inode of the file the reader opened. Test: the same-path-different-file case in reads a log Copilot opens a second time to copy it as the one log fails on 68eb323.

• Log rewritten or replaced after the scan read it (Codex, HIGH on 624d801): not a regression. This is the snapshot every read has. I ran the same interleaving (truncate and write B right after the scan read A) with one descriptor and with two:

  • main, one descriptor: A, then B on the next lookup.
  • main, two descriptors: a lasting copilot_log_ambiguous.
  • this branch, either case: A, then B on the next lookup.

  So with Copilot's second descriptor the branch behaves exactly like main does with one. The next lookup catches the change through the existing prefix check.

• Second descriptor closed between readlink and stat (omo, MEDIUM, reproduced only by forcing ENOENT in a mock). That lookup ends ambiguous, as every such lookup does on main today, in a much narrower window. Not changed here. If it shows up in practice, the fix is to treat ENOENT there like the readlink ENOENT above it (skip the vanished descriptor) and retry on transient errors.

Real-logger harness on the final head: 8 more runs, no wrong session, bound to A after the rotations and B after its registration.

[devswha]

Local merge gate (owner-approved): bun run check fast passed at this head, 96 copilot tests pass, omo CLEAR at this head; Codex: the path-reuse HIGH is fixed here, and the 'log rewritten after the scan read it' case behaves the same as main (evidence in the comment above), so it is not introduced by this PR. Real-logger harness: wrong session 0/20 on main and 0/20 here; lasting ambiguous after rotations 2/20 on main, 0/20 here. Only the /proc fd scan changes; nothing in rotation recovery. Co-authored with YuuuXie (from #3086, kept open as a draft). Merged without waiting for GitHub CI.