max / makeover-timing

Grade what the siblings have pushed, not only what is on this disk The pre-push gate had two views, working copy and as-pushed, and both read every OTHER repo's version off the filesystem. Cargo resolves a git dependency against the branch head at the URL, so an unpushed sibling passed both and failed everywhere else: MNW required quasi "^0.63" while quasi's mnw/main was 0.56.0, this gate printed "internal deps coherent", and Sando build 72 could not resolve (infra 5c4928c1). The third view reads each provider's version at its last-fetched remote-tracking ref. No network on the happy path; a requirement that fails is re-checked after fetching that one repo's one branch, so a stale ref cannot refuse a good push. A repo with no fetched remote is reported ungraded rather than passing. The failure message now names which view broke, because that is the diagnosis: working copy clean and published broken means the sibling is unpushed, not that a manifest edit is uncommitted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: Max Johnson <me@maxj.phd> · 2026-08-27 02:03 UTC

Signed with PGP, not checked

Commit: b56b629c597f762962f1d64cb23b6a1a4cb27d8e

Parent: 1cdc429

1 file changed, +272 insertions, -45 deletions

M scripts/githooks/internal-deps.py +272 -45

			@@ -12,11 +12,16 @@
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:
	15	+	The run grades THREE views and fails on any of them:
16	16
17	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.
	18	+	as pushed the same question asked of the pushing repo's manifests AS THEY
	19	+	EXIST AT THAT COMMIT, against the rest of the tree on disk.
	20	+	Needs a pushed sha, so it is skipped in the by-hand report.
	21	+	as published every requirement against the version the provider has actually
	22	+	PUSHED, read from its last-fetched remote-tracking ref. This is
	23	+	the only view that predicts a build on a machine that is not
	24	+	this one.
20	25
21	26		WHY THE SECOND VIEW EXISTS. The first one reads manifests off the filesystem, so
22	27		an uncommitted forward-fix makes it grade text that git is not publishing. That
			@@ -33,17 +38,22 @@
33	38		that broke: a requirement of "0.11" against a sibling that has moved to 0.14 is
34	39		not a warning, it is a graph that will not resolve on any machine.
35	40
36		-	WHY WORKING COPIES AND NOT REMOTES. `~/Code/.cargo/config.toml` patches every one
37		-	of these dependencies to the working copy in the tree, so what is on disk here is
38		-	what every local build reads. A bump that has not been pushed yet breaks its
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.
	41	+	WHY DISK AND REMOTES BOTH. `~/Code/.cargo/config.toml` patches every one of these
	42	+	dependencies to the working copy in the tree, so what is on disk here is what
	43	+	every local build reads: a bump that has not been pushed yet breaks its consumers
	44	+	on this machine just as thoroughly, and finding that out at push time is the
	45	+	point. But cargo resolves a git dependency against the branch head at the URL, so
	46	+	the `[patch]` block also HIDES an unpushed sibling from every local check. That
	47	+	gap cost a production build on 2026-08-26 (infra `5c4928c1`): MNW required quasi
	48	+	"^0.63", quasi's working copy was 0.63.0 and its `mnw/main` was 0.56.0, both disk
	49	+	views were clean, and Sando could not resolve. The three views answer three
	50	+	different questions and all of them matter.
43	51
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.
	52	+	NETWORK. None on the happy path. The published view reads the last-fetched
	53	+	remote-tracking ref, and only when a requirement FAILS against it does it fetch
	54	+	that one repo's one branch and re-check, so a ref nobody has fetched since the
	55	+	sibling was pushed cannot refuse a good push. A repo with no fetched remote at
	56	+	all is reported as ungraded, never as passing.
47	57
48	58		WHAT IT DOES NOT GRADE, on purpose:
49	59
			@@ -230,14 +240,171 @@
230	240		return out
231	241
232	242
233		-	def analyze(docs, tree):
234		-	"""Grade every in-house git+version pair in `docs`.
	243	+	def repo_of(path, tree):
	244	+	"""The git repo `path` belongs to, or None if it is not in one under `tree`."""
	245	+	d = os.path.dirname(path)
	246	+	while d.startswith(tree):
	247	+	if os.path.exists(os.path.join(d, ".git")):
	248	+	return d
	249	+	if d == tree:
	250	+	break
	251	+	parent = os.path.dirname(d)
	252	+	if parent == d:
	253	+	break
	254	+	d = parent
	255	+	return None
235	256
236		-	Returns (broken, unchecked, absent, graded), where a broken entry is
237		-	(consumer manifest, crate, requirement, version found, provider manifest).
	257	+
	258	+	def publishing_ref(repo, cache):
	259	+	"""The remote-tracking ref a git dependency on `repo` would resolve against.
	260	+
	261	+	Cargo reads the branch head at the URL, and every in-house dependency URL is
	262	+	on one of our forges (CLAUDE.md, "what each remote is for": `mnw` is the
	263	+	public face, `srht` a backup, `astra` the private mirror). So prefer `mnw`,
	264	+	then any other remote whose URL is ours, and fall back to `origin`.
	265	+
	266	+	Returns `<remote>/<branch>` or None when the repo has no such remote or the
	267	+	ref has never been fetched. None is not a verdict: the caller reports the
	268	+	repo as ungraded rather than inventing one.
	269	+	"""
	270	+	if repo in cache:
	271	+	return cache[repo]
	272	+	ref = None
	273	+	lines = git_lines(repo, "remote", "-v") or []
	274	+	urls = {}
	275	+	for line in lines:
	276	+	parts = line.split()
	277	+	if len(parts) >= 2:
	278	+	urls.setdefault(parts[0], parts[1])
	279	+	order = [r for r in ("mnw",) if r in urls]
	280	+	order += [r for r, u in urls.items() if r not in order and OURS.search(u)]
	281	+	order += [r for r in ("origin",) if r in urls and r not in order]
	282	+	for remote in order:
	283	+	head = git_lines(repo, "symbolic-ref", "--quiet", f"refs/remotes/{remote}/HEAD")
	284	+	candidates = []
	285	+	if head:
	286	+	candidates.append(head[0].rsplit("/", 1)[-1])
	287	+	candidates += ["main", "master"]
	288	+	for branch in candidates:
	289	+	if git_lines(repo, "rev-parse", "--verify", "--quiet",
	290	+	f"refs/remotes/{remote}/{branch}"):
	291	+	ref = f"{remote}/{branch}"
	292	+	break
	293	+	if ref:
	294	+	break
	295	+	cache[repo] = ref
	296	+	return ref
	297	+
	298	+
	299	+	def version_at(repo, ref, rel, cache):
	300	+	"""A crate's version in `repo` at `ref`, following a workspace inheritance.
	301	+
	302	+	`rel` is the manifest's path relative to the repo. Returns None when the
	303	+	manifest is not at that ref at all, which is what a crate added since the
	304	+	last push looks like.
	305	+	"""
	306	+	key = (repo, ref, rel)
	307	+	if key in cache:
	308	+	return cache[key]
	309	+	version = None
	310	+	doc = load_at(repo, ref, rel)
	311	+	if doc:
	312	+	pkg = doc.get("package")
	313	+	if isinstance(pkg, dict):
	314	+	v = pkg.get("version")
	315	+	if isinstance(v, str):
	316	+	version = v
	317	+	elif isinstance(v, dict) and v.get("workspace") is True:
	318	+	# Walk up to the workspace root as it exists at the same ref.
	319	+	d = os.path.dirname(rel)
	320	+	while True:
	321	+	root_rel = os.path.join(d, "Cargo.toml") if d else "Cargo.toml"
	322	+	root = load_at(repo, ref, root_rel) if root_rel != rel else None
	323	+	inherited = (
	324	+	((root or {}).get("workspace") or {}).get("package") or {}
	325	+	).get("version")
	326	+	if isinstance(inherited, str):
	327	+	version = inherited
	328	+	break
	329	+	if not d:
	330	+	break
	331	+	d = os.path.dirname(d)
	332	+	cache[key] = version
	333	+	return version
	334	+
	335	+
	336	+	def analyze_published(docs, disk_docs, tree, repo, sha):
	337	+	"""Grade every requirement against what its provider has actually PUSHED.
	338	+
	339	+	This is the view that predicts a build somewhere other than this machine.
	340	+	The other two read the provider's version off the filesystem, and the
	341	+	`[patch]` block in ~/Code/.cargo/config.toml means that is what a local
	342	+	build resolves -- but a git dependency resolves against the branch head at
	343	+	the URL, so an unpushed sibling passes both of them and fails everywhere
	344	+	else. That is exactly what happened on 2026-08-26: MNW required quasi
	345	+	"^0.63", quasi's working copy was 0.63.0 and `mnw/main` was 0.56.0, both
	346	+	existing views were clean, and Sando build 72 could not resolve.
	347	+
	348	+	The repo being pushed is read at `sha` rather than at its remote, since what
	349	+	it is about to publish is the thing to grade. Every other repo is read at
	350	+	its last-fetched remote ref: no network on the happy path. A break is
	351	+	re-checked after fetching that one repo, so a stale ref cannot refuse a push
	352	+	on its own.
	353	+
	354	+	Returns (broken, graded, ungraded), where ungraded maps a repo to why.
	355	+	"""
	356	+	ref_cache, version_cache, fetched = {}, {}, set()
	357	+	versions_on_disk = crate_index(disk_docs, tree)
	358	+	broken, graded, ungraded = [], 0, {}
	359	+
	360	+	for consumer_manifest, name, req in requirements(docs):
	361	+	known = versions_on_disk.get(name)
	362	+	if known is None:
	363	+	continue # Not in this tree; the disk views already say so.
	364	+	provider_manifest = known[1]
	365	+	provider_repo = repo_of(provider_manifest, tree)
	366	+	if provider_repo is None:
	367	+	ungraded.setdefault(os.path.dirname(provider_manifest), "not a git repo")
	368	+	continue
	369	+	if repo is not None and provider_repo == repo and sha:
	370	+	# The repo under the hook: what it is about to publish is `sha`,
	371	+	# which the "as pushed" view already read off disk into `docs`.
	372	+	continue
	373	+	ref = publishing_ref(provider_repo, ref_cache)
	374	+	if ref is None:
	375	+	ungraded.setdefault(provider_repo, "no fetched remote to read")
	376	+	continue
	377	+	rel = os.path.relpath(provider_manifest, provider_repo)
	378	+	have = version_at(provider_repo, ref, rel, version_cache)
	379	+	if have is None:
	380	+	ungraded.setdefault(provider_repo, f"{name} is not at {ref} yet")
	381	+	continue
	382	+	verdict = satisfies(req, have)
	383	+	if verdict is None:
	384	+	continue
	385	+	if not verdict and provider_repo not in fetched:
	386	+	# Only now, and only for this one repo: a ref nobody has fetched
	387	+	# since the sibling was pushed would otherwise refuse a good push.
	388	+	fetched.add(provider_repo)
	389	+	remote, branch = ref.split("/", 1)
	390	+	git_lines(provider_repo, "fetch", "--quiet", remote, branch)
	391	+	version_cache.pop((provider_repo, ref, rel), None)
	392	+	have = version_at(provider_repo, ref, rel, version_cache) or have
	393	+	verdict = satisfies(req, have)
	394	+	graded += 1
	395	+	if not verdict:
	396	+	broken.append(
	397	+	(consumer_manifest, name, req, have, provider_manifest, ref)
	398	+	)
	399	+	return broken, graded, ungraded
	400	+
	401	+
	402	+	def crate_index(docs, tree):
	403	+	"""Crate name -> (version, manifest path), workspace inheritance resolved.
	404	+
	405	+	A member saying `version.workspace = true` gets its number from the root,
	406	+	and reporting it as 0.0.0 would be a false break.
238	407		"""
239		-	# Workspace versions first: a member saying `version.workspace = true` gets
240		-	# its number from the root, and reporting it as 0.0.0 would be a false break.
241	408		ws_version = {}
242	409		for p, doc in docs.items():
243	410		if not doc:
			@@ -254,10 +421,12 @@
254	421		while d.startswith(tree):
255	422		if d in ws_version:
256	423		return ws_version[d]
257		-	d = os.path.dirname(d)
	424	+	parent = os.path.dirname(d)
	425	+	if parent == d:
	426	+	break
	427	+	d = parent
258	428		return None
259	429
260		-	# crate name -> (version, manifest path)
261	430		versions = {}
262	431		for p, doc in docs.items():
263	432		if not doc:
			@@ -268,8 +437,17 @@
268	437		v = resolve_version(p, pkg)
269	438		if v:
270	439		versions[pkg["name"]] = (v, p)
	440	+	return versions
271	441
272		-	broken, unchecked, absent, graded = [], 0, set(), 0
	442	+
	443	+	def requirements(docs):
	444	+	"""Every in-house git+version pair: (consumer manifest, crate, requirement).
	445	+
	446	+	A dependency qualifies when it carries both a `git` URL on one of our forges
	447	+	and a `version`. That is the pairing cargo enforces and the one that breaks:
	448	+	a requirement of "0.11" against a sibling that has moved to 0.14 is not a
	449	+	warning, it is a graph that will not resolve on any machine.
	450	+	"""
273	451		for p, doc in docs.items():
274	452		if not doc:
275	453		continue
			@@ -284,19 +462,33 @@
284	462		if not OURS.search(git):
285	463		continue
286	464		name = spec.get("package") if isinstance(spec.get("package"), str) else key
287		-	known = versions.get(name)
288		-	if known is None:
289		-	# A repo that is not on this machine (ripgrow lives on mbp
290		-	# only). Not a finding: nothing here can be wrong about it.
291		-	absent.add(name)
292		-	continue
293		-	verdict = satisfies(req, known[0])
294		-	if verdict is None:
295		-	unchecked += 1
296		-	continue
297		-	graded += 1
298		-	if not verdict:
299		-	broken.append((p, name, req, known[0], known[1]))
	465	+	yield p, name, req
	466	+
	467	+
	468	+	def analyze(docs, tree):
	469	+	"""Grade every in-house git+version pair in `docs` against the tree on disk.
	470	+
	471	+	Returns (broken, unchecked, absent, graded), where a broken entry is
	472	+	(consumer manifest, crate, requirement, version found, provider manifest,
	473	+	source label). The source label is None here: this view reads the version
	474	+	off a manifest, and naming the manifest already says where it came from.
	475	+	"""
	476	+	versions = crate_index(docs, tree)
	477	+	broken, unchecked, absent, graded = [], 0, set(), 0
	478	+	for p, name, req in requirements(docs):
	479	+	known = versions.get(name)
	480	+	if known is None:
	481	+	# A repo that is not on this machine (ripgrow lives on mbp only).
	482	+	# Not a finding: nothing here can be wrong about it.
	483	+	absent.add(name)
	484	+	continue
	485	+	verdict = satisfies(req, known[0])
	486	+	if verdict is None:
	487	+	unchecked += 1
	488	+	continue
	489	+	graded += 1
	490	+	if not verdict:
	491	+	broken.append((p, name, req, known[0], known[1], None))
300	492		return broken, unchecked, absent, graded
301	493
302	494
			@@ -304,7 +496,7 @@
304	496		"""Breaks this push owns, and breaks that were already there."""
305	497		ours, theirs = [], []
306	498		for item in broken:
307		-	consumer_manifest, _name, _req, _have, provider_manifest = item
	499	+	consumer_manifest, _name, _req, _have, provider_manifest, _src = item
308	500		mine = repo is not None and (
309	501		consumer_manifest.startswith(repo + os.sep)
310	502		or provider_manifest.startswith(repo + os.sep)
			@@ -320,10 +512,14 @@
320	512		def rel(path):
321	513		return os.path.relpath(path, tree)
322	514
323		-	for consumer_manifest, name, req, have, provider_manifest in ours + theirs:
	515	+	for consumer_manifest, name, req, have, provider_manifest, src in ours + theirs:
	516	+	where = (
	517	+	f"{src} has {have} ({rel(provider_manifest)})"
	518	+	if src
	519	+	else f"the tree has {have} ({rel(provider_manifest)})"
	520	+	)
324	521		print(
325		-	f" [{label}] {rel(consumer_manifest)}: requires {name} \"{req}\", "
326		-	f"the tree has {have} ({rel(provider_manifest)})",
	522	+	f" [{label}] {rel(consumer_manifest)}: requires {name} \"{req}\", {where}",
327	523		file=sys.stderr,
328	524		)
329	525		if repo is None:
			@@ -368,7 +564,20 @@
368	564		if broken and report(broken, repo, tree, label):
369	565		refuse = True
370	566
	567	+	# The third view: what the rest of the tree has actually PUSHED. Graded from
	568	+	# the most authoritative consumer view available, so the requirements read
	569	+	# are the ones about to be published.
	570	+	consumer_view = views[-1][1]
	571	+	pub_broken, pub_graded, ungraded = analyze_published(
	572	+	consumer_view, docs, tree, repo, sha
	573	+	)
	574	+	summaries.append(("as published", pub_graded, 0, set(), bool(pub_broken)))
	575	+	if pub_broken and report(pub_broken, repo, tree, "as published"):
	576	+	refuse = True
	577	+
371	578		if refuse:
	579	+	bad = {lbl for lbl, _g, _u, _a, broke in summaries if broke}
	580	+	clean = [lbl for lbl, _g, _u, _a, broke in summaries if not broke]
372	581		print("", file=sys.stderr)
373	582		print(
374	583		"pre-push: this push leaves a dependency that cannot resolve.\n"
			@@ -379,13 +588,26 @@
379	588		" compile, and push them with this one.",
380	589		file=sys.stderr,
381	590		)
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.
	591	+	# Which view broke is the diagnosis, so say what the views disagree
	592	+	# about rather than only that they disagree.
	593	+	if "as published" in bad and "working copy" not in bad:
386	594		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.",
	595	+	" The working copy is fine and the published tree is not, so the\n"
	596	+	" difference is what has been PUSHED: a git dependency resolves against\n"
	597	+	" the branch head at the URL, and ~/Code/.cargo/config.toml's [patch]\n"
	598	+	" block hides that locally. Push the sibling named above first.",
	599	+	file=sys.stderr,
	600	+	)
	601	+	elif "as pushed" in bad and "working copy" not in bad:
	602	+	print(
	603	+	" The working copy is fine and the commit is not, so the difference is\n"
	604	+	" what is COMMITTED. An uncommitted manifest edit is the usual cause.",
	605	+	file=sys.stderr,
	606	+	)
	607	+	elif clean:
	608	+	print(
	609	+	f" Note: the {clean[0]} view is clean, so the views disagree; the one\n"
	610	+	" that failed is named on each line above.",
389	611		file=sys.stderr,
390	612		)
391	613		return 1
			@@ -397,6 +619,11 @@
397	619		+ (f", {len(absent)} crates not in this tree" if absent else "")
398	620		+ ")."
399	621		)
	622	+	for where, why in sorted(ungraded.items()):
	623	+	# Never silently: a repo nobody could read is not a repo that passed.
	624	+	print(
	625	+	f"pre-push: [as published] {os.path.relpath(where, tree)} not graded ({why})."
	626	+	)
400	627		if skipped_push_view:
401	628		# Never silently: a view that did not run must not read as one that passed.
402	629		print("pre-push: could not read the pushed commit; graded the working copy only.")