Skip to main content

max / makeover-timing

Grade the pushed commit's manifests, not just the working copy The gate read manifests off the filesystem, so an uncommitted forward-fix made it grade text git was not publishing. On 2026-08-24 it printed "internal deps coherent" over an mnw-cli requirement advanced on disk and never committed, and Sando failed to resolve it minutes later. It now grades two views and fails on either: the working copy, which catches a local bump that breaks every build on this machine, and the repo's manifests at the pushed commit, which catches what is actually being published. When only one view is clean it says so, since that difference is the diagnosis.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-24 22:20 UTC
Signed with PGP, not checked
Commit: b8cd37b41889c9ee283c583539bff16b916f1f73
Parent: 250a553
2 files changed, +182 insertions, -44 deletions
M scripts/githooks/internal-deps.py +165 -42
@@ -4,7 +4,7 @@
4 4 DO NOT EDIT IN PLACE. The master is _private/infra/bootstrap/githooks/internal-deps.py.
5 5
6 6 Usage:
7 - internal-deps.py <tree-root> [repo-root]
7 + internal-deps.py <tree-root> [repo-root] [pushed-sha]
8 8
9 9 With a repo root, only pairs that repo is on either side of can fail the run;
10 10 everything else is reported as a note. Without one, every pair is graded, which
@@ -12,6 +12,21 @@
12 12
13 13 python3 internal-deps.py ~/Code
14 14
15 + With a pushed sha as well, the run grades TWO views and fails on either:
16 +
17 + working copy what this machine builds today. The historical check.
18 + as pushed the same question asked of the repo's manifests AS THEY EXIST
19 + AT THAT COMMIT, against the rest of the tree on disk.
20 +
21 + WHY THE SECOND VIEW EXISTS. The first one reads manifests off the filesystem, so
22 + an uncommitted forward-fix makes it grade text that git is not publishing. That
23 + is not hypothetical: on 2026-08-24 mnw-cli's `synckit-client` requirement had
24 + been advanced to "0.9" in the working copy and never committed, this gate printed
25 + `internal deps coherent (42 requirements)`, the push went out, and Sando failed
26 + to resolve `^0.8` against 0.9.0 minutes later. The gate was checking a tree that
27 + was not the tree being published, and nothing distinguished that from real
28 + coherence.
29 +
15 30 WHAT IT GRADES. Every dependency in the tree that carries both a `git` URL on one
16 31 of our forges and a `version` requirement, against the version in the working
17 32 copy of the crate that URL names. That is the pairing cargo enforces and the one
@@ -22,6 +37,13 @@
22 37 of these dependencies to the working copy in the tree, so what is on disk here is
23 38 what every local build reads. A bump that has not been pushed yet breaks its
24 39 consumers just as thoroughly, and finding that out at push time is the point.
40 + That is why the as-pushed view ADDS a check rather than replacing this one:
41 + grading only the commit would stop catching the unpushed bump that breaks every
42 + build on this machine. The two views answer different questions and both matter.
43 +
44 + The rest of the tree is read from disk in both views, deliberately. Reading other
45 + repos' remotes would need a fetch per repo, and the same `[patch]` block means
46 + disk is what a local build resolves against anyway.
25 47
26 48 WHAT IT DOES NOT GRADE, on purpose:
27 49
@@ -36,6 +58,7 @@
36 58
37 59 import os
38 60 import re
61 + import subprocess
39 62 import sys
40 63 import tomllib
41 64
@@ -153,16 +176,66 @@
153 176 return v[0] == 0 and v[1] == 0
154 177
155 178
156 - def main():
157 - if len(sys.argv) < 2:
158 - print(__doc__.strip(), file=sys.stderr)
159 - return 2
160 - tree = os.path.realpath(sys.argv[1])
161 - repo = os.path.realpath(sys.argv[2]) if len(sys.argv) > 2 else None
179 + def git_lines(repo, *args):
180 + """Run git in `repo` and return stdout lines, or None if it failed."""
181 + try:
182 + out = subprocess.run(
183 + ["git", "-C", repo, *args],
184 + capture_output=True, text=True, check=True,
185 + )
186 + except (OSError, subprocess.CalledProcessError):
187 + return None
188 + return out.stdout.splitlines()
162 189
163 - paths = manifests(tree)
164 - docs = {p: load(p) for p in paths}
165 190
191 + def git_manifests(repo, sha):
192 + """Repo-relative paths of every Cargo.toml at `sha`, or None if unreadable."""
193 + lines = git_lines(repo, "ls-tree", "-r", "--name-only", sha)
194 + if lines is None:
195 + return None
196 + out = []
197 + for rel in lines:
198 + if os.path.basename(rel) != "Cargo.toml":
199 + continue
200 + if any(part in SKIP_DIRS for part in rel.split("/")):
201 + continue
202 + out.append(rel)
203 + return out
204 +
205 +
206 + def load_at(repo, sha, rel):
207 + """One manifest as it exists at `sha`. None if missing or unparseable."""
208 + lines = git_lines(repo, "show", f"{sha}:{rel}")
209 + if lines is None:
210 + return None
211 + try:
212 + return tomllib.loads("\n".join(lines))
213 + except tomllib.TOMLDecodeError:
214 + return None
215 +
216 +
217 + def pushed_view(docs, repo, sha):
218 + """`docs` with everything under `repo` replaced by its content at `sha`.
219 +
220 + The rest of the tree stays as it is on disk, which is what a local build
221 + resolves against either way. Returns None if the commit cannot be read, so
222 + the caller can skip the view rather than invent a verdict about it.
223 + """
224 + rels = git_manifests(repo, sha)
225 + if rels is None:
226 + return None
227 + out = {k: v for k, v in docs.items() if not k.startswith(repo + os.sep)}
228 + for rel in rels:
229 + out[os.path.join(repo, rel)] = load_at(repo, sha, rel)
230 + return out
231 +
232 +
233 + def analyze(docs, tree):
234 + """Grade every in-house git+version pair in `docs`.
235 +
236 + Returns (broken, unchecked, absent, graded), where a broken entry is
237 + (consumer manifest, crate, requirement, version found, provider manifest).
238 + """
166 239 # Workspace versions first: a member saying `version.workspace = true` gets
167 240 # its number from the root, and reporting it as 0.0.0 would be a false break.
168 241 ws_version = {}
@@ -224,60 +297,110 @@
224 297 graded += 1
225 298 if not verdict:
226 299 broken.append((p, name, req, known[0], known[1]))
300 + return broken, unchecked, absent, graded
227 301
228 - if not broken:
229 - print(
230 - f"pre-push: internal deps coherent ({graded} requirements"
231 - + (f", {unchecked} unchecked" if unchecked else "")
232 - + (f", {len(absent)} crates not in this tree" if absent else "")
233 - + ")."
234 - )
235 - return 0
236 -
237 - def rel(path):
238 - return os.path.relpath(path, tree)
239 302
303 + def split_blame(broken, repo):
304 + """Breaks this push owns, and breaks that were already there."""
240 305 ours, theirs = [], []
241 306 for item in broken:
242 - consumer_manifest, name, req, have, provider_manifest = item
307 + consumer_manifest, _name, _req, _have, provider_manifest = item
243 308 mine = repo is not None and (
244 309 consumer_manifest.startswith(repo + os.sep)
245 310 or provider_manifest.startswith(repo + os.sep)
246 311 )
247 312 (ours if mine else theirs).append(item)
313 + return ours, theirs
314 +
315 +
316 + def report(broken, repo, tree, label):
317 + """Print one view's breaks. Returns True if this push has to be refused."""
318 + ours, theirs = split_blame(broken, repo)
319 +
320 + def rel(path):
321 + return os.path.relpath(path, tree)
248 322
249 323 for consumer_manifest, name, req, have, provider_manifest in ours + theirs:
250 324 print(
251 - f" {rel(consumer_manifest)}: requires {name} \"{req}\", "
325 + f" [{label}] {rel(consumer_manifest)}: requires {name} \"{req}\", "
252 326 f"the tree has {have} ({rel(provider_manifest)})",
253 327 file=sys.stderr,
254 328 )
255 -
256 329 if repo is None:
257 - print(f"internal deps: {len(broken)} unresolvable requirements.", file=sys.stderr)
258 - return 1
259 -
330 + return bool(broken)
260 331 if not ours:
261 332 # Somebody else's skew. Worth seeing, never worth blocking this push on:
262 333 # a gate that fails for a reason the pusher cannot fix is a gate that
263 334 # gets bypassed by reflex, and then it is not a gate.
264 - print(
265 - f"pre-push: {len(theirs)} unresolvable requirements elsewhere in the "
266 - "tree (listed above, not this push's).",
267 - )
268 - return 0
335 + if theirs:
336 + print(
337 + f"pre-push: [{label}] {len(theirs)} unresolvable requirements "
338 + "elsewhere in the tree (listed above, not this push's).",
339 + )
340 + return False
341 + return True
269 342
270 - print("", file=sys.stderr)
271 - print(
272 - "pre-push: this push leaves a dependency that cannot resolve.\n"
273 - " A version requirement states which major a consumer was written against,\n"
274 - " so bumping a library and fixing its consumers is one pass (CLAUDE.md,\n"
275 - " \"a breaking bump of an in-house crate is forward-fixed, in the same pass\").\n"
276 - " Fix: bump the requirement in the manifests above, make the consumers\n"
277 - " compile, and push them with this one.",
278 - file=sys.stderr,
279 - )
280 - return 1
343 +
344 + def main():
345 + if len(sys.argv) < 2:
346 + print(__doc__.strip(), file=sys.stderr)
347 + return 2
348 + tree = os.path.realpath(sys.argv[1])
349 + repo = os.path.realpath(sys.argv[2]) if len(sys.argv) > 2 else None
350 + sha = sys.argv[3] if len(sys.argv) > 3 else None
351 +
352 + docs = {p: load(p) for p in manifests(tree)}
353 +
354 + views = [("working copy", docs)]
355 + skipped_push_view = False
356 + if repo and sha:
357 + pushed = pushed_view(docs, repo, sha)
358 + if pushed is None:
359 + skipped_push_view = True
360 + else:
361 + views.append(("as pushed", pushed))
362 +
363 + refuse = False
364 + summaries = []
365 + for label, view in views:
366 + broken, unchecked, absent, graded = analyze(view, tree)
367 + summaries.append((label, graded, unchecked, absent, bool(broken)))
368 + if broken and report(broken, repo, tree, label):
369 + refuse = True
370 +
371 + if refuse:
372 + print("", file=sys.stderr)
373 + print(
374 + "pre-push: this push leaves a dependency that cannot resolve.\n"
375 + " A version requirement states which major a consumer was written against,\n"
376 + " so bumping a library and fixing its consumers is one pass (CLAUDE.md,\n"
377 + " \"a breaking bump of an in-house crate is forward-fixed, in the same pass\").\n"
378 + " Fix: bump the requirement in the manifests above, make the consumers\n"
379 + " compile, and push them with this one.",
380 + file=sys.stderr,
381 + )
382 + clean = [lbl for lbl, _g, _u, _a, bad in summaries if not bad]
383 + if clean:
384 + # The whole point of the second view. Saying which one passed is what
385 + # turns "it worked on my machine" into a diagnosis.
386 + print(
387 + f" Note: the {clean[0]} view is clean, so the difference is what is\n"
388 + " committed. An uncommitted manifest edit is the usual cause.",
389 + file=sys.stderr,
390 + )
391 + return 1
392 +
393 + for label, graded, unchecked, absent, _bad in summaries:
394 + print(
395 + f"pre-push: internal deps coherent [{label}] ({graded} requirements"
396 + + (f", {unchecked} unchecked" if unchecked else "")
397 + + (f", {len(absent)} crates not in this tree" if absent else "")
398 + + ")."
399 + )
400 + if skipped_push_view:
401 + # Never silently: a view that did not run must not read as one that passed.
402 + print("pre-push: could not read the pushed commit; graded the working copy only.")
403 + return 0
281 404
282 405
283 406 if __name__ == "__main__":
M scripts/githooks/pre-push +17 -2
@@ -25,6 +25,14 @@
25 25 # redirects every one of these dependencies to the working copy -- so a local bump
26 26 # breaks a consumer's build here whether or not it has been pushed anywhere.
27 27 #
28 + # It also grades this repo's manifests AS THEY EXIST AT THE PUSHED COMMIT, and
29 + # fails on either view. Reading the working copy alone means an uncommitted
30 + # forward-fix is graded instead of the text git is publishing: on 2026-08-24 that
31 + # printed "internal deps coherent" over an mnw-cli requirement that had been
32 + # advanced on disk and never committed, and Sando failed to resolve it minutes
33 + # later. Both views are kept because they catch different things -- the commit
34 + # view cannot see a local bump that breaks every build on this machine.
35 + #
28 36 # `cargo check` and `cargo clippy` both compile only the lib and bin targets, so a
29 37 # break confined to `tests/` or a `#[cfg(test)]` module is clean under both and
30 38 # lands unnoticed (goingson's sqlx 0.9 upgrade shipped exactly that way).
@@ -47,10 +55,17 @@
47 55 # Refs arrive on stdin as "<local ref> <local sha> <remote ref> <remote sha>".
48 56 # A branch deletion has an all-zero local sha and no tree to push. Read once,
49 57 # ahead of both gates: stdin is not seekable and a second reader gets nothing.
58 + #
59 + # The sha is KEPT, not just tested. Gate 1 grades the manifests at that commit as
60 + # well as the ones on disk, because they are not always the same text and the
61 + # difference is invisible in the good case. Last sha wins on a multi-ref push:
62 + # the gate wants a commit whose tree it can read, and grading one of them beats
63 + # grading none. A deletion contributes an all-zero sha and is skipped.
50 64 pushing=0
65 + pushed_sha=""
51 66 while read -r _local_ref local_sha _remote_ref _remote_sha; do
52 67 case "$local_sha" in
53 - *[!0]*) pushing=1 ;;
68 + *[!0]*) pushing=1; pushed_sha="$local_sha" ;;
54 69 esac
55 70 done
56 71 [ "$pushing" -eq 1 ] || exit 0
@@ -62,7 +77,7 @@
62 77 # exactly the state being detected.
63 78 CODE_ROOT="${CODE_ROOT:-$HOME/Code}"
64 79 if [ -d "$CODE_ROOT" ] && command -v python3 >/dev/null 2>&1; then
65 - if ! python3 "$ROOT/scripts/githooks/internal-deps.py" "$CODE_ROOT" "$ROOT"; then
80 + if ! python3 "$ROOT/scripts/githooks/internal-deps.py" "$CODE_ROOT" "$ROOT" "$pushed_sha"; then
66 81 echo "pre-push: push aborted (use --no-verify to bypass)."
67 82 exit 1
68 83 fi