Skip to content

perf(server): status refreshes reuse git config until a file changes - #30838

Open
navidnadali wants to merge 3 commits into
pingdotgg:mainfrom
navidnadali:perf/git-config-snapshot
Open

navidnadali wants to merge 3 commits into
pingdotgg:mainfrom
navidnadali:perf/git-config-snapshot

Conversation

@navidnadali

@navidnadali navidnadali commented Nov 4, 2026 •

Copy link
Copy Markdown

Problem

Each status refresh runs about seven git config reads and git remote twice, for values that cannot change unless a file changes. That is about 9 git processes per turn for nothing. On Linux each one forks the server.

Change

  • One git config --list -z snapshot per repository answers those reads until a file changes.
  • The snapshot is keyed on the stat of every config file it read (includes too) and of the git dir, including the mode and owners that repository discovery and the safe.directory check read. It is guarded against GIT_* environment and work-tree overrides, and it sees HEAD by content and the on-disk case of the path.
  • The node:fs stat is behind a nodeBuiltinImport suppression, as in antigravityUsageReader.ts, because Effect's File.Info has no ctime or nanosecond mtime.
  • Hosts with git older than 2.42 (2.31 for some reads) fall back to one command per read, as today.

Server only (apps/server/src/vcs/); no client or contract change; provider adapters are not involved.

Scope and approval

No product behaviour, default, setting or contract change. The cache is invalidated by file changes, not by time, and returns what git would return. Complementary to #29721, which stops a different layer re-spawning git remote. A separate follow-up PR builds on this file. Open PRs #29155 and #28577 also edit GitVcsDriverCore.ts, for different reads.

Background and measurements: #31002 (context, not an approval request).

Verification

Failing first (vp test run apps/server/src/vcs/GitVcsDriverCore.test.ts), counting the git commands the driver spawns:

  • -t "runs no config reads on a refresh while the config is unchanged": the second refresh spawns 0 git config and 0 git remote.

  • -t "on the next read": a table of changes the snapshot must notice (a relative include, a branch switch under an onbranch include, a repository losing its HEAD, a nested repository's HEAD becoming invalid or empty, a half-initialised repository completing, objects or refs losing search permission, a new owner of the top level or of a linked worktree's git dir); each is seen on the next read. Without root a test cannot chown, so the owner rows have stat report another owner and run git with GIT_TEST_ASSUME_DIFFERENT_OWNER=1.

  • -t "reuses config read through a differently cased path" and -t "sees a repository initialised below a directory outside GIT_WORK_TREE".

  • -t "reads what git config --get and git remote read": the snapshot's answers equal git's.

  • gitConfigSnapshot.test.ts: parsing, include dependencies (a Windows origin too) and the environment signature.

  • On main @ 8b44f42b1 without the change, 18 of these driver cases fail by assertion; the GIT_WORK_TREE case passes there, as a guard, and gitConfigSnapshot.test.ts needs the new module. With the change: 274 pass. One upstream test fails on this macOS machine without the change too ("keeps generated worktree folders short…", a path-spelling case).

  • vp run check:changed: format, lint, knip and typecheck (3 packages) pass; in the 3 affected test files 276 tests pass and 1 fails, the same upstream worktree-folder test named above, which fails on main on this macOS machine too. On macOS with Node 26.12.1. The server integration suites (vp test run apps/server/integration) also pass, 300 tests in 26 files, and so do the provider replay fixture suites (vp test run apps/server/src/orchestration-v2/testkit), 234 tests in 16 files.

Local run, macOS, Node 26.12.1, Apple Git 2.50.1; upstream vs this change; 60 fake agent sessions (20 Claude, 20 Codex, 20 ACP), provider-only, synthetic database; median of 3 interleaved runs (min–max), 90 s window:

Metric Upstream This change Change
Git commands per turn 29.22 (28.60–29.67) 20.87 (20.22–21.38) −29 %
git config per turn 7.24 (7.09–7.36) 1.03 (1.00–1.06) −86 %
git remote per turn 2.07 (2.03–2.11) 0.00 −100 %
Main-thread synchronous spawn time per turn (ms) 17.1 (13.5–18.8) 10.0 (7.3–13.8) −42 %
Turns completed (90 s) 410 (374–478) 449 (412–624) within noise

Git commands and main-thread spawn time per turn

The command counts separate in every run; the spawn-time ranges overlap slightly. A warm status refresh runs 7 git commands instead of 15. The stat checks that replace the reads cost about 1 ms/s of main-thread time.

How this was measured

Local numbers come from one developer machine, before vs after the change on the same tree, interleaved. They show direction and mechanism; the magnitudes are not bench-machine numbers. One shared macOS machine (load average 37–93), so treat timings as indicative; the counts are deterministic. With no spawn worker, every git command starts on the main thread.

Not checked

  • A bench-machine A/B on Linux, where each avoided command is a fork of the server.
  • Counts on main @ 8b44f42b1: the runs above were against an older upstream base.
  • A build-specific extra config file (such as Xcode's) is tracked only once it has entries.
  • A permission change made only through an ACL, with no change of mode or owner.
  • Windows.

Made with Claude Opus 5.5 via Claude Code. Reviewed by GPT-6.1 Sol via Codex and, independently, by Claude Opus 5.5 via Claude Code.

🤖 Generated with Claude Code

Each status refresh ran about seven git config reads and git remote
twice, for values that only change when a file changes. One
`git config --list -z` snapshot per repository now answers those reads
until a config file it read, an include or the git dir changes. Git
older than 2.42 falls back to one command per read.

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:XL 500-999 changed lines (additions + deletions). labels Nov 4, 2026
return;
}
if (dependency.kind === "presence" || dependency.kind === "absent") {
resolve(stats.isDirectory() ? "directory" : "file");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium vcs/gitConfigSnapshot.ts:179

When .git/objects or .git/refs loses search permission, this fingerprint remains directory, so isCurrent keeps cached config-derived results valid even though Git no longer recognizes the repository. presence dependencies discard stats.mode; include the permission bits in the fingerprint so a permission change invalidates the snapshot.

-          resolve(stats.isDirectory() ? "directory" : "file");
+          resolve(`${stats.isDirectory() ? "directory" : "file"}:${stats.mode}`);
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/vcs/gitConfigSnapshot.ts around line 179:

When `.git/objects` or `.git/refs` loses search permission, this fingerprint remains `directory`, so `isCurrent` keeps cached config-derived results valid even though Git no longer recognizes the repository. `presence` dependencies discard `stats.mode`; include the permission bits in the fingerprint so a permission change invalidates the snapshot.

// Discovery: the directory, what would make Git stop below the top level (a `.git`, or HEAD of a
// bare repository), the `.git` it found, and what keeps that a repository. The prefix comes from
// Git, cased as on disk like the top level, whatever case the directory was given in.
const dependencies: GitConfigDependency[] = [{ path: cwd, kind: "identity" }];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium vcs/gitConfigSnapshot.ts:368

Changing the owner of the worktree root or the actual Git directory leaves the cached snapshot current, so read() continues serving repository config and cached remote results after Git would reject the repository as dubiously owned. The dependency list tracks cwd and toplevel/.git, but not toplevel or the directory returned by --absolute-git-dir when .git is a gitfile; track identity and ownership of both directories as well.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/vcs/gitConfigSnapshot.ts around line 368:

Changing the owner of the worktree root or the actual Git directory leaves the cached snapshot current, so `read()` continues serving repository config and cached remote results after Git would reject the repository as dubiously owned. The dependency list tracks `cwd` and `toplevel/.git`, but not `toplevel` or the directory returned by `--absolute-git-dir` when `.git` is a gitfile; track identity and ownership of both directories as well.

@macroscopeapp

macroscopeapp Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — The change caches Git configuration and config-derived remote results until tracked dependencies change. It also introduces a static-analysis suppression requiring human review; two Medium findings identify invalidation gaps around repository permission and ownership changes.

Needs human approval because:

  • 2 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The change adds per-directory Git configuration snapshots and routes selected config and remote reads through them. It tracks configuration dependencies and environment inputs, reuses eligible command results, and falls back to Git when snapshots are unavailable or stale.

Changes: Git configuration snapshot flow

Layer / File(s) Summary
Configuration inputs and dependency fingerprints
apps/server/src/vcs/gitConfigSnapshot.ts, apps/server/src/vcs/gitConfigSnapshot.test.ts
Adds config-list parsing, include resolution, environment signatures, and filesystem dependency fingerprints. Tests cover parsed entries, dependency paths, and environment changes.
Snapshot discovery and reuse
apps/server/src/vcs/gitConfigSnapshot.ts
Discovers repository and config paths, then loads and reuses snapshots when the environment signature and dependency fingerprints remain current. Unsupported or unstable states return no snapshot.
Snapshot-backed driver reads
apps/server/src/vcs/GitVcsDriverCore.ts, apps/server/src/vcs/GitVcsDriverCore.test.ts, apps/server/src/vcs/gitConfigSnapshot.ts
Routes selected config and remote reads through snapshots and caches eligible command results. Tests cover reuse, invalidation, fallback reads, and comparison with Git output. Shared exit validation retains nonzero-exit handling.

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant GitVcsDriverCore
  participant makeGitConfigSnapshots
  participant Git
  participant runWithGitConfigSnapshot
  GitVcsDriverCore->>makeGitConfigSnapshots: Request a config snapshot
  makeGitConfigSnapshots->>Git: Discover repository paths and config entries
  Git-->>makeGitConfigSnapshots: Return paths and config entries
  makeGitConfigSnapshots-->>GitVcsDriverCore: Return snapshot or null
  GitVcsDriverCore->>runWithGitConfigSnapshot: Run selected config-dependent command
  runWithGitConfigSnapshot->>Git: Execute command when no cached result exists
  runWithGitConfigSnapshot-->>GitVcsDriverCore: Return command result
Loading

Possibly related issues

  • pingdotgg/t3code issue 27696: The issue reports repeated readConfigValue calls during PR polling. This change caches config reads and routes listRemoteNames and remoteExists through the snapshot path. The issue’s concurrency and queue starvation problems are not addressed.

Suggested reviewers: juliusmarminge, saphid, t3dotgg

Merge Risk: Minimal · up to a9534

This change reduces repeated git config and remote process spawns during status refreshes and falls back to the existing Git commands whenever the snapshot cannot be trusted. No actionable merge-blocking risk was identified.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description clearly documents the problem, implementation, scope, verification results, benchmark data, and unverified cases. However, the Scope and approval section does not provide explicit main… Add a link to the triaged issue or discussion containing explicit maintainer approval of the direction and scope, including the approval comment. If the change qualifies as a focused obvious fix without prior approval, explain why that exce…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: reusing Git configuration during server status refreshes until a file changes.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description clearly documents the problem, implementation, scope, verification results, benchmark data, and unverified cases. However, the Scope and approval section does not provide explicit maintainer approval or explain why the change qualifies for the no-approval exception; it states that the linked discussion is only background.

Resolution

Add a link to the triaged issue or discussion containing explicit maintainer approval of the direction and scope, including the approval comment. If the change qualifies as a focused obvious fix without prior approval, explain why that exception applies.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 2fe6d52 and a95349e.

Files selected for processing: 2
  • apps/server/src/vcs/GitVcsDriverCore.test.ts
  • apps/server/src/vcs/gitConfigSnapshot.ts

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

navidnadali and others added 2 commits November 4, 2026 13:52
A config snapshot stayed current when the top level of the work tree or
a linked worktree's git dir changed owner, or when .git/objects or
.git/refs lost search permission. Git then refuses or no longer finds
the repository while the snapshot kept answering. Fingerprints now
carry the mode and owners, and the top level and git dir are tracked,
without another git command.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@maria-rcks

maria-rcks commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head a95349e3d9437cd16e6d6146fa000bc5228c24e3 against github/main at 9e85538535c8 on linux, git 2.43.0. installed both checkouts with vp i and ran one background dev server at a time, with isolated empty state.

used the repo's playwright and /usr/bin/google-chrome. created the project and repository through the app, then ran real codex gpt-6.1-sol turns to create/commit a readme and verify that same repository on the pr.

opening git action options sends the real vcs.refreshStatus request. git trace2 subprocess starts between the click and successful rpc response showed:

three warm menu opens       git config        direct git remote
main                         19, 19, 19         8, 8, 8
pr                            0,  0,  0         2, 2, 2

these are counts across the menu-triggered refresh work, not one driver invocation. other app paths still run remote commands. no wall-clock speedup claim.

a real provider turn configured an included local remote. after warming the cache, editing only that included file changed the next observed refresh's comparison base from refs/heads/verification-neighbor to refs/heads/main. .git/config retained the same hash and stat. absolute includes reused the snapshot; the relative include case returned correct values through fallback.

neighbor checks passed: branch picker switched away and back; a provider edit appeared as +2 -0 in status and the actual diff; committing it returned to a clean tree with upstream tracking and ahead count 1.

blacksmith: focused vcs checks passed (274 passed, 1 skipped). vp run check:changed github/main passed format, lint, knip, graphql, typecheck, and tests (276 passed, 1 skipped). applicable github ci checks also passed. no pr source changes were needed.

command transcript

main: real codex turn committed the verification readme

pr: real codex turn verified the same clean repository

pr: provider edit rendered in the actual diff with the included comparison ref

pr: actual branch picker round trip and git action menu

Not verified

native desktop, ios/android, windows/macos filesystem behavior, a live git older than 2.42, live ownership/permission transitions, acl-only changes, remote/relay/tunnel clients, and external forge services. the filesystem case-insensitivity test was skipped on this linux filesystem. uploaded video playback and seeking were checked.

@navidnadali

Copy link
Copy Markdown
Author

Note

Written by Claude Opus 5.5 (Claude Code) on behalf of Navid

Thanks for checking this end to end, and for the trace2 counts. The relative include falling back is expected: git reports the repository's own config with a relative origin (file:.git/config), so an include relative to it can't be resolved to a file to watch, and those reads run git as before. The "a relative include" test row asserts exactly that. Of the items you couldn't verify, owner and permission changes are covered by the test table (the owner rows simulate another owner, because a test can't chown without root); ACL-only changes and Windows are listed under "Not checked".

This branch has not been deployed

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

Labels

size:XL 500-999 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.

2 participants