Skip to content

fix(server): a blocked .git read no longer wedges the server during the session scan - #15977

Closed
azgul wants to merge 1 commit into
pingdotgg:mainfrom
azgul:fix/scanner-blocking-git-config-read
Closed

azgul wants to merge 1 commit into
pingdotgg:mainfrom
azgul:fix/scanner-blocking-git-config-read

Conversation

@azgul

@azgul azgul commented Oct 5, 2026

Copy link
Copy Markdown

Problem

On a headless macOS server (t3 service), the agent-session scan reads .git/config for each transcript cwd. Under a TCC-protected folder (~/Documents, ~/Desktop) that open() waits on a privacy prompt nobody can answer and never returns. Fiber interruption doesn't release the libuv pool thread, so client retries fill the pool and every fs and DNS operation in the process hangs. Details and live-process evidence: #15971.

Change

readGitIdentity reads .git and .git/config through makeBoundedFileReader (project/boundedFileRead.ts):

  • O_NONBLOCK open plus an fstat check that it's a regular file of at most 64 KB (covers FIFOs)
  • at most one native read in flight, released when the OS call returns rather than when the fiber is interrupted, so a blocked open() holds one pool thread at most
  • a 2 s timeout, after which the candidate gets the existing unreadable-config result (git: { remoteKey: null, repository: null })

Scope and approval

Fixes #15971 (not yet triaged). One problem: the scanner's two .git reads. No contract or client changes.

Verification

  • New scanner test (scan with a git config whose open() never returns): .git/config is a FIFO, the scan runs pool-size + 1 times, then an unrelated read must succeed within 3 s. On main it hangs past the 60 s test timeout and teardown then stalls too. With the fix it passes in 21 ms.
  • boundedFileRead.test.ts: a read that never settles (the TCC case, where O_NONBLOCK doesn't help) is issued once and later calls return none; reading resumes once the stuck read returns; concurrent reads are serialized, not dropped; a rejected read returns none.
  • vp test run src/project/: 198 passed. tsc --noEmit in apps/server: 0 errors. vp lint and vp fmt --check are clean on src/project.
  • Not checked: a real TCC prompt end to end. The root cause was confirmed on a live 0.0.45 server (see [Bug]: Session scan reading a TCC-protected .git/config exhausts the libuv pool and hangs the server #15971), but the fix was only exercised with the FIFO and the simulated never-settling read.

Model and harness: Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

A .git/config under a TCC-protected folder blocks open() on a headless
macOS server until a privacy prompt nobody sees is answered. Each scan
retry parked another libuv pool thread until the pool was exhausted and
every fs and DNS operation hung. Read .git metadata with O_NONBLOCK, a
regular-file check, a 2 s timeout, and at most one native read in flight.

Fixes pingdotgg#15971

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 5, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This production server bug fix introduces a new bounded native filesystem reader and changes existing session-scan I/O behavior. It also adds file-level nodeBuiltinImport:off diagnostic suppressions, so the change requires human review.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
docs/internals/effect-services.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 464b7f09-51e5-4520-9cea-8a93cb05cec2
📥 Commits

Reviewing files that changed from the base of the PR and between cf3e714 and 388deb2.

📒 Files selected for processing (4)
  • apps/server/src/project/AgentSessionScanner.test.ts
  • apps/server/src/project/AgentSessionScanner.ts
  • apps/server/src/project/boundedFileRead.test.ts
  • apps/server/src/project/boundedFileRead.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Git metadata reads in AgentSessionScanner now use a bounded reader that limits file size and read time. Tests cover stuck and concurrent reads, and a live FIFO test checks that session scans and a filesystem probe complete.

Changes

Bounded Git Metadata Reads

Layer / File(s) Summary
Bounded file reader
apps/server/src/project/boundedFileRead.ts, apps/server/src/project/boundedFileRead.test.ts
Adds a reader that serializes native reads and returns None on timeout or rejection. The file helper opens only regular files up to the configured size and closes the handle. Tests cover stuck reads, retries, concurrent reads, and rejection.
Bounded Git metadata scanning
apps/server/src/project/AgentSessionScanner.ts, apps/server/src/project/AgentSessionScanner.test.ts
The scanner reads Git pointers and config through the bounded reader, with a two-second bound and 64 KiB file limit. Missing or failed reads still produce empty strings. A live test uses a FIFO at .git/config and checks that concurrent scans return null Git metadata while a filesystem probe succeeds.

Priority: ⬆️ High

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

Change: Bug fix · Severity of issue fixed: High

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 388de

The bounded Git metadata reads preserve scan completion and filesystem responsiveness in the tested blocked-read scenario. No identified issue needs resolution before merge.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 388de

The change contains blocked Git metadata reads without adding a public endpoint, increasing privileges, or changing authorization. Unreadable metadata keeps its existing fallback behavior. No material security risk was found to be introduced or worsened.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A transcript working directory or its Git metadata can affect the shared scanner reader. A stuck operation can therefore suppress Git enrichment for other scans in the same server module instance, but the inspected transition prevents additional pending reads through that instance.

Trust Boundaries and Controls

  • observed — The inspected Git metadata consumer produces optional scan-result fields rather than an authorization decision. The existing importer uses project identity and workspace-root checks, not Git metadata, for import routing. These controls and the scan route are unchanged by the PR.

Resilience and Maintainability Implications

  • observed — The inspected tests assert containment of repeated calls during a stuck read, recovery after late completion, serialization of concurrent reads, and rejection-to-none behavior. They do not directly establish real privacy-prompt behavior or exercise fiber interruption.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: preventing a blocked .git read from stalling the server during session scans.
Description check ✅ Passed The description includes the required sections and detailed problem, implementation, and verification information. However, Scope and approval says issue #15971 is not yet triaged and does not explain…
Linked Issues check ✅ Passed Issue #15971 requires the scan to skip Git identity when metadata cannot be read and keep the server responsive. AgentSessionScanner routes both .git reads through a module-scoped bounded reader. …
Out of Scope Changes check ✅ Passed The changes support issue #15971. The bounded reader and its tests implement and verify the protection for the scanner’s .git reads. The scanner test verifies the issue’s FIFO reproduction and serve…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 11, 2026 — with ChatGPT Codex Connector
@juliusmarminge juliusmarminge self-assigned this Oct 19, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one for maintainer batch worktree-git-10. I am checking the native filesystem-boundary approach and the macOS verification requirement before rebuilding or publishing changes.

@juliusmarminge

Copy link
Copy Markdown
Member

Blocked on native macOS verification; no implementation changes or push.

The current scanner still reads .git pointer/config metadata through native filesystem calls without a native-call bound. The contribution identifies the right failure mode: interrupting an Effect fiber does not release a libuv worker parked inside an OS filesystem call. Its module-wide outstanding-read limit therefore needs real platform evidence; Linux fake promises and FIFO tests cannot verify macOS privacy prompts or a stalled filesystem.

Required human verification before rebuilding/publishing:

  1. On macOS, launch the server and Electron in the actual user context against a disposable repository on a privacy-protected or deliberately stalled mount. Hold the OS open/read operation, rather than only delaying a JavaScript promise. Verify the session scan returns within its documented bound while omitting unavailable Git metadata.
  2. Repeat scans more times than UV_THREADPOOL_SIZE, including concurrent scanner instances. Confirm outstanding native work stays bounded and unrelated filesystem operations, DNS and server requests remain responsive. Include blocked stat/open/read paths and cancellation/server shutdown.
  3. Release or deny the blocked operation. Confirm the reader recovers, descriptors close, and later scans again identify ordinary repositories, linked worktrees and pointer-file repositories correctly. Check the regular-file and metadata-size limits against real supported repositories.
  4. Review and approve the native filesystem boundary against project Effect conventions, then run focused scanner/reader suites, scoped server typecheck and lint. Repeat ordinary Linux and Windows scanner paths after the final implementation.

I am leaving the original PR open and credited to @azgul. This Linux batch cannot provide the required macOS proof, so publishing a rebuilt native-reader implementation would be unverified.

Copy link
Copy Markdown
Member

Closing in favour of #23690, which makes the same change. Thanks for the work here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews 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.

[Bug]: Session scan reading a TCC-protected .git/config exhausts the libuv pool and hangs the server

2 participants