Repository navigation
fix(host-service): pin git's message locale so environmental failures classify - #6863
Conversation
… classify Git translates its diagnostics into the user's locale, but the classifier that keeps environmental git failures off the 500 path recognises git's own English wording. For anyone not on an English-locale machine the wording never matches, so a condition that is silent for an English user reports as a 500 — HOST-SERVICE-3Q, 2,166 events in 7 days from three machines. Pin LC_ALL=C on the env every git invocation is handed, so the anchored patterns that already exist match for every user. No new patterns. Claude-Session: https://claude.ai/code/session_018RT6cwVgxwCJ2MpnZaVpJz
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughGit invocations now force ChangesGit locale normalization
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change pins Git diagnostics to a consistent locale so environmental failures classify uniformly across users; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description clearly explains the problem, root cause, fix, behavior impact, scope, and verification results. It does not use the exact template headings or include a Checklist section, but it provides the required substantive information and is mostly complete. ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Problem
A workspace status poll runs git in a directory that is not a git repository. We already decided that is a user-environment condition, not a bug:
getStatuscatches the failure and rethrows it as a typed non-500 whenever it recognises git's own wording. The recognition is a set of anchored English regular expressions.Git translates its diagnostics into the user's locale. For anyone not running an English environment the text never matches, the failure falls through unclassified, and the same condition that is silent for an English user reports as a 500. The captured message in HOST-SERVICE-3Q is the French rendering of git's "not a git repository" text — 2,166 events over 7 days from three machines, none of them English-locale.
This is not specific to that one pattern. Every pattern in the classifier is English-only, so the gap applies to all of them, and to every pattern added later.
Reality check (asked of every fix on this path):
git.getStatuson 1.23.0; that path is unchanged on main at 1.24.2.getStatus→runStatusSnapshot→resolveGitTaskEnv→createGitEnvResolveris the only env the status subprocesses get, andrethrowEnvironmentalGitErrorhas exactly one host-service call site (git.ts:246), on that path. The change ships in the next host-service/desktop release and reaches every host on it.Root cause
createGitEnvResolverbuilds the env every git invocation is handed. It spreads the user's own environment through the credential provider, so the user'sLANG/LC_ALL/LANGUAGEreach git and git answers in that language. Nothing pins the locale.Fix
Add
LC_ALL: "C"alongsideGIT_OPTIONAL_LOCKSin the envcreateGitEnvResolverreturns, after the credential spreads so it wins over the user's own value.LC_ALLrather thanLC_MESSAGES. Both were live options; this is why the whole-locale one is right here:LC_MESSAGESloses to the user. POSIX resolvesLC_ALLahead ofLC_MESSAGES, so on a machine that exportsLC_ALL=fr_FR.UTF-8anLC_MESSAGESwe set is simply ignored — the fix would silently not work for a subset of the very users it targets. Making it work would mean deleting the user'sLC_ALLfrom the child env, which changes the effective value of every other category as a side effect: a strictly larger behaviour change than pinning one variable.LANGUAGEoutranks both, and glibc's gettext ignores it only when the resolved locale is exactly"C"(guess_category_valuecomparesstrcmp(locale, "C")).LC_ALL=Ctherefore neutralisesLANGUAGEtoo;LC_ALL=POSIXwould not, which is why the value isC.LC_ALLis not real for git. PinningLC_CTYPEsounds like it should change how non-ASCII path names are decoded and quoted in the output we parse. It does not: git reports paths as bytes and quotes them bycore.quotePath, not by locale, and the snapshot reads status, numstat and untracked expansion through-z/--null, which disables quoting outright. Verified byte-for-byte on a repo with accented, CJK and Cyrillic filenames —git status --porcelain --null,git diff --numstat -zandgit ls-filesproduce identical output underLC_ALL=CandLC_ALL=fr_FR.UTF-8. There is a test pinning this.No typed cause added. The change adds no new throw, so no new
cause.kind. (The existing kinds on this classifier are read by nothing but its own tests; that is pre-existing and out of scope here.)Behaviour change worth naming: git's own prose in error surfaces is now English for non-English users rather than their language. The app UI is English-only, so this is a consistency gain, but it is a real user-visible difference.
Which git failures on this path still report as 500s
Everything the classifier does not name — now reliably in English, so the boundary is the same on every machine. From the classifier's own negative tests:
index.lockcontention, dubious ownership, bad revisions and pathspecs,cannot chdir to work tree, installed filters that ran and failed (external filter ... failed),unable to readfor non-tree objects, spawn syscall failures (spawn git EAGAIN), and our own simple-git misconfiguration other than a missing base directory. Those are bugs or conditions we have not yet decided about, and they keep reporting.Also unchanged: host-service builds a few git clients with no env override at all (project import, repo resolution, git identity). They still inherit the user's locale — nothing classifies their messages, so pinning them would be machinery for a hypothetical. They are named, not touched.
Verification
Every new test was written first and confirmed failing before the fix. The status-path test reproduces the Sentry message end-to-end — before the change:
The one test that passed before and after is the negative control: it asserts that the unpinned env still produces the French wording and that the classifier does not match it. That is the standing record of why this is fixed in the environment rather than with a French pattern.
CI runs on machines whose git has no translation catalogs, so the localized case is driven against a
gitstub that resolves the message locale exactly the way GNU gettext does (LC_ALLoverLC_MESSAGESoverLANG;LANGUAGEconsulted only when the resolved locale is not"C") and answers in that language. The stub is reached the way a real git is — through the child environment'sPATH— so the resolver,createUserSimpleGit, the realgetGitStatusSnapshotand the real classifier are all exercised.After the change:
Full package suite, since the env resolver feeds every git worker task:
No desktop git code was touched, so
apps/desktop/src/no-main-process-blocking.test.tsis not in scope.packages/host-service/src/no-main-loop-blocking.test.tspasses unchanged — the change is insideruntime/git/, and no git client was added.Adjacent, deliberately not touched
apps/desktop/src/lib/trpc/routers/workspaces/utils/git-errors.tscarries a second copy of the same English-only patterns for the desktop process, with its own git clients and its own env. It has the same locale gap and is left alone here.Refs HOST-SERVICE-3Q
https://claude.ai/code/session_018RT6cwVgxwCJ2MpnZaVpJz
Summary by CodeRabbit
Bug Fixes
Tests