Repository navigation
refactor(code-intelligence): two divergent lists of repo-overriding git vars — production strips 7, the test strips 9 #14023
Description
Self-inflicted, introduced in #13949 during this session. Filing rather than quietly fixing, because the divergence is a latent production gap and not only a tidiness problem.
The fork
Two constants describe the same concept — "git environment variables that override the repository named by -C" — under different names, in the same PR:
| location | name | members |
|---|---|---|
code_intelligence/code_evolution_miner.py (production) |
_REPO_OVERRIDING_GIT_VARS |
7 |
code_intelligence/co_change_test.py (test) |
_INHERITED_GIT_VARS |
9 |
in the test but NOT stripped in production: ['GIT_CEILING_DIRECTORIES', 'GIT_CONFIG']
Production has the smaller list. GitHistoryCrawler._run_git therefore does not strip:
GIT_CONFIG— redirects config resolution, socore.quotepath=false(passed explicitly via-cfor a reason, feat(code-intelligence): git co-change coupling is never computed — hidden dependencies invisible to blast radius #13639) can be joined by settings the caller never intended.GIT_CEILING_DIRECTORIES— bounds upward repository discovery.
Neither is as severe as the GIT_DIR redirect that #13949 fixed, but the whole point of that fix was that a crawler asked to analyse one repository must not silently analyse another, and this leaves two of the levers unhandled.
Why it happened, since the mechanism matters
I wrote the test helper first, hardened it, then wrote the production helper from memory rather than from the constant I had just written. Names diverged (_INHERITED_ vs _REPO_OVERRIDING_), which hid the duplication from any same-name search — the failure mode recorded in #13845/#13846: compare member sets, not names.
Fix
One constant, exported from the production module and imported by the test, with the union of both lists. The test's extra two are correct and should win.
A guard test should assert the test helper and _run_git strip the same set, so a future addition to one cannot silently skip the other — otherwise this recurs the moment someone adds GIT_ATTR_NOSYSTEM to whichever file they happen to be in.
Acceptance criteria
- One definition; the test imports it rather than restating it.
-
GIT_CONFIGandGIT_CEILING_DIRECTORIESare stripped in production. - A test asserts both call sites use the identical set — mutation evidence: adding a var to one and not the other fails.
-
co_change_test.pystill passes underGIT_INDEX_FILE+-n 4(the original bug(ci): co_change_test corrupts its own repos under xdist — an inherited GIT_INDEX_FILE makes -C advisory #13983 reproduction).
Related
- fix(ci): isolate git from the ambient environment so -C is authoritative (#13948) #13949 / bug(ci): co_change_test corrupts its own repos under xdist — an inherited GIT_INDEX_FILE makes -C advisory #13983 — where both constants were introduced
- canonical(enums): CommandRisk and CommandRiskLevel model the same concept with divergent tails — FORBIDDEN has no wire representation #13845 / canonical(enums): SecretType and SecretRequirement duplicate the secret taxonomy — agents cannot express an OAuth requirement #13846 — same-concept different-name forks; compare member sets
Activity
github-actions commentedon Aug 11, 2026
👋 Auto-triage could not confidently classify this issue (no confident signal). Could a maintainer add the appropriate labels (
frontend,backend,infrastructure,docs,testingandgood-first-issue,intermediate,advanced)?mrveiss commentedon Aug 11, 2026
Second self-inflicted item from the same session, recorded here so there is one pickup rather than two trivial ones.
services/deploy_activity.py::_self_update_unit_activeis 36 lines, against the ≤30-line rule inCLAUDE.md. Shipped in #13946.Worth stating the trade-off rather than just splitting it: roughly a third of those lines are comments explaining why a timeout and a non-zero exit must both yield
Nonerather thanFalse, which is the subtle part of the function. Extracting the subprocess call would satisfy the line count while separating that reasoning from the code it guards.My suggestion is to split on the decision boundary — one helper that runs the query and returns
(returncode, stdout)or raises, and keep the tri-state interpretation in the caller — rather than to split for length alone. If that reads worse in review, documenting the exception is the better outcome; the rule exists for readability, not for the number.Neither item here changes behaviour; both are cleanup of code I shipped today.