Skip to content

build: gate pushes on generated-artifact manifest freshness - #765

Merged
mparrett merged 2 commits into
mainfrom
build/manifest-prepush-gate
Aug 20, 2026
Merged

build: gate pushes on generated-artifact manifest freshness#765
mparrett merged 2 commits into
mainfrom
build/manifest-prepush-gate

Conversation

@mparrett

@mparrett mparrett commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Problem

#641 made pkg/rt/generated.manifest record a SHA-256 per declared generator input. Editing any of those inputs restages core_compiled.lgb, core_go_lowered/ and zz_primitives_generated.go, whether or not the generated content moves. pkg/rt/os.go is an ordinary source file, and nothing about editing it signals that a regeneration is now owed.

Nothing local catches the omission. No hook ran check-generated at any stage: the config has twelve hooks — nine at pre-commit, three at pre-push, with forbid-compiled-binaries at both — and none of them is this one. make install-hooks registers only the two merge drivers.

And no server-side operation can supply it, because none of them has a generator. A rebase moves a base. A merge driver resolves a file. The stack UI does both and force-pushes the result. None recomputes a hash.

The cost so far:

There is also no backstop above it: per #751 the main ruleset has no required_status_checks, so a red branch reads as merely unreviewed.

Change

Add check-generated as a pre-push hook.

It is a hash comparison of the manifest's declared inputs against the tree, with no regeneration and no build, so it costs about 1.3 seconds. That makes it the cheapest hook in the file — the three already at pre-push are go test, the vendored Clojure compat suite, and the ir-stress ratchet.

always_run is set rather than a types filter, because a declared input can be a .go or a .lg file and a filter on either would miss half the sources.

Verification

With prek, the hook passes on a clean tree, and a committed edit to pkg/rt/os.go fails it:

generated-artifact manifest freshness (#641).............................Failed
  check-generated: dependency manifest stale for 3 output(s): pkg/rt/core_compiled.lgb, pkg/rt/core_go_lowered/, pkg/rt/zz_primitives_generated.go
  Run `make generate` to regenerate the bundle + lowered Go tree and refresh the manifest.

The failure only reproduces when the edit is committed. prek stashes unstaged changes before running hooks, so probing with a dirty working tree passes vacuously.

What this does not cover

This is the local layer only, and it is worth being clear about the two it leaves open.

It would not have prevented either main outage. Those were stale-green merges#743's checks ran 19 minutes before #641 landed, and GitHub does not recompute a merge ref when the base moves. That is #751's missing required_status_checks, and it needs a ruleset change rather than a hook.

It also does not settle the merge=manifest question in #747. My read is that this check weakens the case for a driver rather than complementing it: the sharp edge you identified there is a manifest that merges cleanly into a wrong state, since the file is sorted and one record per line, and a hash comparison against the tree catches exactly that. #747 already notes the risk that a regenerating driver "could assert outputs are ready when they have not been rebuilt". I would rather land this, see whether the driver still has a job, and decide that separately. You run these workflows locally more than I do, so if there is a conflict pattern this check misses, that changes the answer.

Refs #641, #747, #751.

Editing any file declared as a generator input in pkg/rt/generated.manifest
restages core_compiled.lgb, core_go_lowered/ and zz_primitives_generated.go,
because #641 records a SHA-256 per input rather than comparing output content.
Nothing local catches the omission: no hook ran check-generated at any stage,
and no server-side operation can refresh the manifest, since a rebase, a merge
driver and the stack UI all lack a generator. The gap has cost main two
outages (#749 and #750 were hand-written repairs) and, most recently, three
rebase-and-regenerate cycles on the #698/#699/#717 stack, where every branch
stayed red through two rounds of restacking.

This adds check-generated as a pre-push hook. It is a hash comparison against
the tree with no regeneration and no build, so it costs ~1.3s — the cheapest
hook in the file, against go-test, the compat suite and the ir-stress ratchet
already at that stage. always_run is set because a declared input can be a .go
or a .lg file, so a types filter would miss half the sources.

Verified with prek: the hook passes on a clean tree, and a committed edit to
pkg/rt/os.go fails it with the message naming the three stale outputs and
`make generate`. Note the failure only reproduces when the edit is committed —
prek stashes unstaged changes before running hooks, so an uncommitted probe
passes vacuously.

Refs #641, #747, #751.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mparrett
mparrett requested a review from nnunley August 19, 2026 22:15
Comment thread .pre-commit-config.yaml
The target passed -stale, which lists outputs whose recorded input hashes no
longer match the tree and stops there. It never compares generated.sums against
generated.manifest, so a corrupted or wrongly-merged digest passed silently:

    # digest in generated.sums replaced with zeroes, manifest untouched
    $ go run ./cmd/check-generated -stale ; echo $?
    0
    $ go run ./cmd/check-generated ; echo $?
    check-generated: STALE — .lg/lgbgen sources changed without regeneration.
    1

That is the #747 case — a manifest and digest that disagree after a merge — and
it is the one this target is relied on to catch. It also broke the parity the
new pre-push hook documents: the hook runs the unflagged command, so a git
contributor was blocked where a jj contributor running the Makefile target was
not, which is the reverse of what that note promises.

Dropping the flag is strictly stronger. The unflagged mode is a superset:
stale inputs still exit 1 with the same output, and a digest mismatch now exits
1 as well. The two callers, `make check-generated` and the generated-artifacts
CI job, both get the wider check; clean main passes it unchanged.

Reported by @nooga on #765.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mparrett
mparrett merged commit a665761 into main Aug 20, 2026
14 checks passed
@mparrett
mparrett deleted the build/manifest-prepush-gate branch August 20, 2026 19:39
mparrett added a commit that referenced this pull request Aug 21, 2026
The UI rebase moved this branch onto a665761 without running the merge
drivers — GitHub does not run them server-side — so the manifest's recorded
input hashes stopped matching the tree and both `build` and
`generated-artifacts` went red. Regenerating reproduces every artifact
byte-identically; only the provenance records and the digest move.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Aug 26, 2026
The UI rebase moved this branch onto a665761 without running the merge
drivers — GitHub does not run them server-side — so the manifest's recorded
input hashes stopped matching the tree and both `build` and
`generated-artifacts` went red. Regenerating reproduces every artifact
byte-identically; only the provenance records and the digest move.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Aug 28, 2026
…in (#767)

A closure that captures a local cannot be lifted to a top-level fn, so it
lowers to an inline rt.BoxNativeFn. Closures are compile-time entities in this
pass: they carry no runtime Go local, and closure-expr re-materializes them at
each use site.

closure-info*'s :block-arg case resolved a block parameter with `some`, taking
the first incoming edge that resolved to a closure. At a join whose edges
disagree — two different fn literals, or a closure against nil — the parameter
was still classified as a closure value, so it got no local and both edges
skipped assignment. The single use site then materialized whichever closure
came first:

    (defn both-branches [flag x y]
      (if flag (fn [] x) (fn [] y)))

(both-branches false 1 2) returned the then closure. The failure is silent: Go
rejects the result only when the lowering happens to strand a dead temporary,
so a sibling of the same shape compiled clean and returned the wrong branch's
value from a binary that built and exited 0.

Resolve every incoming edge and treat the parameter as a closure value only
when they unanimously agree; a disagreeing join falls back to a real local, and
each edge materializes its own rt.BoxNativeFn into it. The visited guard yields
a :cycle sentinel rather than nil so unanimity can tell a loop's back edge from
an edge that carries no closure, and a :cycle source is set aside rather than
counted as disagreement — which preserves the local-free treatment a
loop-carried closure depends on (#266).

Setting a cycle aside makes an answer path-dependent, so each result carries
the set of still-on-the-stack nids it leaned on, and only results that leaned
on none are memoized. A parameter's own back edge is discharged where it is
found rather than recorded: `p = agree(inits…, p)` has `agree(inits…)` as its
fixed point, a property of the block graph rather than of the walk, so the
ordinary loop-carried closure stays fully cacheable. A plain boolean taint
fails in both directions — caching a concrete cycle-derived answer drops a
branch of a disagreeing join nested in a loop, and excluding every
cycle-derived answer restores the per-param-per-edge re-walk that made lowering
time quadratic in #266.

Two native-entry fixtures lower through the production path, match a committed
AST shape pinning the per-branch assignment, build, and print byte-exact output
the pre-fix lowering got wrong: conditional_closure.lg covers the acyclic
disagreeing joins, loop_carried_closure.lg the loop-carried case. Both semantic
mutants die, so the assertions are not vacuous.

pkg/rt/generated.manifest and generated.sums move because lower_go.lg is a
declared generator input (#765).

Closes #766

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Aug 28, 2026
The UI rebase moved this branch onto a665761 without running the merge
drivers — GitHub does not run them server-side — so the manifest's recorded
input hashes stopped matching the tree and both `build` and
`generated-artifacts` went red. Regenerating reproduces every artifact
byte-identically; only the provenance records and the digest move.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Aug 30, 2026
The UI rebase moved this branch onto a665761 without running the merge
drivers — GitHub does not run them server-side — so the manifest's recorded
input hashes stopped matching the tree and both `build` and
`generated-artifacts` went red. Regenerating reproduces every artifact
byte-identically; only the provenance records and the digest move.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Sep 1, 2026
The UI rebase moved this branch onto a665761 without running the merge
drivers — GitHub does not run them server-side — so the manifest's recorded
input hashes stopped matching the tree and both `build` and
`generated-artifacts` went red. Regenerating reproduces every artifact
byte-identically; only the provenance records and the digest move.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Sep 2, 2026
The UI rebase moved this branch onto a665761 without running the merge
drivers — GitHub does not run them server-side — so the manifest's recorded
input hashes stopped matching the tree and both `build` and
`generated-artifacts` went red. Regenerating reproduces every artifact
byte-identically; only the provenance records and the digest move.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants