One runner for every git call, and a read that stops triggering itself

[cjimti]

Closes #53. Closes #61.

Two tickets, one subsystem: the git read path. #53 is the extraction that makes a second git reader possible; #61 is the loop that the extracted runner's own comment describes and could not close on its own.

#53 — the seam was the runner, not the parser

internal/git was at 1072 of its 1100-line ceiling with #35 still to come. Two prior tickets looked for the split and both landed on the parsers, and both hit the same wall: a sibling package cannot reach runGit, and depguard forbids one service importing another, so every proposed split only compiled by extracting the runner first — which each of them then declined as too expensive and paid for with a ceiling raise instead. The #9 note called "a third package and a second dependency-root exception" the argument against, while naming the parser as the seam that did exist. Both halves were backwards: the runner is what has more than one consumer, and the parsers have exactly one each.

run.go is now internal/gitexec, a dependency root beside internal/buildinfo. It cost 278 lines and one reviewed config change — less than either raise taken instead of paying it. That paragraph in package_budget_test.go is struck rather than left to read as still true.

What the runner holds is not a service's behavior. It is the conditions every git call must run under, and each fails silently when copied wrong: LC_ALL pinned so the not-a-repository check matches git's own words, --no-optional-locks so a read does not rewrite the index, a local deadline against a hung mount and a remote one against a socket that stopped answering, and git's stderr carried out unedited. None of the four has a test that would fail in a second copy of them, which is why there must not be one.

The seam is three functions, not an exported options struct. Read, Write, WriteRemote. The deadline is chosen by naming the operation rather than by remembering to set a field, and the fourth combination — a read over the network — does not exist until something needs it.

#61 — a status read fed the watcher that triggered it

An idle window with one project open ran git status --porcelain=v2 about four times a second, forever. The read opens .git/index; macOS reports the access-time bump as NOTE_ATTRIB; fsnotify surfaces that as Chmod; the resulting batch tells the frontend its status is stale; the frontend reads again. --no-optional-locks was already stopping the read from writing the index — nothing can stop the kernel reporting the read. The tree's own content reads (ReadFile, ReadPrefixes) sat in the same loop through a different door, so this was never only a git problem.

An event whose only op is Chmod is now dropped, and what that gives up is stated beside it: chmod +x on a tracked file, with nothing else happening, no longer refreshes the badges — git reports a mode change, and that badge waits for the next real event. Everything that edits, adds, moves or removes still arrives as Write, Create, Rename or Remove, and a checkout that changes a mode writes .git/HEAD, which is watched.

The two alternatives were rejected in the diff rather than in a comment thread. Stat-ing on each attribute event and comparing against the last-known mode needs a metadata table that grows with every path touched, and the field that actually separates a chmod from a read is ctime, which Go exposes only through platform-specific syscall types — DESIGN.md targets Windows too. Suppressing events around a read is timing-dependent and would swallow a real concurrent change, which nothing later corrects.

The test that would have caught it

It had to talk to real git. A fake cannot produce an event that only exists because a real read touched a real file and a real kernel reported it. Five status reads of an unchanged repository now publish nothing; verified to fail at three batches with the fix removed. Its counterweight asserts git add still publishes, so the loop is not closed by going deaf. A third test pins the predicate itself on a hand-built watcher — Chmod alone is dropped, Write|Chmod is not, because kqueue combines bits and a fix written as "has the Chmod bit" would drop real writes with nothing else noticing.

Things to look at

One behavior changed that #53 did not ask for. classify read the local timeout constant regardless of the invocation, so a push cut off at ten minutes reported timed out after 30s and sent the user looking for a problem they did not have. It is a method on the invocation now, with a test. I fixed it rather than move a message I knew was wrong — but it is scope you did not authorize, and reverting it is a small diff.

Two config changes, not suppressions. wrapcheck gains ignore-package-globs for internal/gitexec: its errors are already the finished message — they name the argv, which contains the repository since git runs with -C, and carry git's stderr verbatim — and internal/app is what puts the operation's name in front of them. A wrap in between turns pulling in /repo: git -C /repo pull: … into pulling in /repo: pull: git -C /repo pull: …. Scoped to that one package; every other cross-package error still has to be wrapped. If you would rather eat the doubled message, wrapping instead is a five-line change. gosec's G204 path list swaps internal/git for internal/gitexec, which is a narrowing — internal/git starts no process at all now.

internal/git's ceiling ratcheted down rather than making room for #35. #53's criterion reads "back under its ceiling with room for #35's parser, and its ceiling ratchets down to what it actually measures", which pulls two ways. I took the second: #35's diff viewer now lands as its own package over gitexec, which was the point of the extraction, so it does not need room here. Say the word if you read it the other way.

Ratchets

	before	after
internal/git LOC	1100	950 (measured 882)
internal/git exported	25	25 — run.go exported nothing
internal/gitexec	—	320 LOC / 5 exported
internal/watch LOC	1300	1300, unmoved

internal/gitexec gets deliberately the smallest allowance in the table. The only thing that would fit under a larger one is knowledge of what git prints, and that would couple every git reader in the repository to it.

internal/watch is at 1288 of 1300 and the ceiling does not move for a bug fix's doc comment — I trimmed the comment instead. The pin now says the next arrival there should be a decomposition, and names the split that exists: fsnotify.go and poll.go are two change-detection strategies behind one Events seam.

depguard gains one rule (gitexec-is-a-dependency-root) and one allow entry, reviewed once here rather than argued about in #35. The import graph pin carries the new edge and the note that internal/app must never acquire one — the binding layer talks to the git service, not to the binary.

Gates

make verify green. Patch coverage 100% (92/92 changed lines), lint 0 issues, internal/gitexec 100% covered. The new watcher tests run clean 10× under -race.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.83%. Comparing base (ebd18c6) to head (05af5ee).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
+ Coverage   94.72%   94.83%   +0.10%     
==========================================
  Files          42       42              
  Lines        2179     2186       +7     
==========================================
+ Hits         2064     2073       +9     
+ Misses         71       70       -1     
+ Partials       44       43       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cjimti]

CI note, for the record rather than for action on this PR.

The Test job failed on the first run and passed on re-run with no change:

--- FAIL: TestPublishTreeIsPublishedToEveryEventSubscriber (10.00s)
    events_test.go:52: reading from the socket: read tcp 127.0.0.1:38502->127.0.0.1:36747: i/o timeout

It is not a finding against this diff and it is not a CI wrapper error, so it is neither of the two things CLAUDE.md says a post-verify failure usually is. internal/stream imports nothing first-party and is untouched here; the test builds the server in-process with a fake Terminals, so nothing in this branch can reach it.

The cause is a registration race in that package's own test: dialer.Dial returns on the 101 response, which is written inside Upgrade, while register runs afterward in the handler goroutine. Publishing immediately after dialing can snapshot the subscribers before one of them is in s.conns, and the missed frame never repeats. Filed as #71 rather than fixed here — it is a second ticket, and it belongs to the package that owns the seam.

All checks green on the re-run.