native dependency guard fails on main: module graph is 740 modules, baseline max is 727 (Preflight / static checks, pre-existing) (gc-na313) - #159
Conversation
zook-bot
left a comment
zook-bot
left a comment
There was a problem hiding this comment.
VERDICT: COMMENT
Reviewed branch: polecat/gc-na313
Reviewed base: main
Reviewed commit: 8d7f0cf
Scope checked: Read the complete diff for scripts/check-native-dependency-surface.sh, engdocs/contributors/beads-version-bump-anchors.md, and engdocs/contributors/index.md; checked anchor bead gc-na313; checked PR #159 metadata and CI status; verified the dependency-growth attribution against the pre-bump graph.
Findings: None.
Verification:
bash scripts/check-native-dependency-surface.shat the reviewed commit: pass (modules=740 aws=25 azure=9 dolthub=15 googleapi=1 binary_bytes=270665464).make check-docsat the reviewed commit: pass (test/docsyncok).git diff --check origin/main...8d7f0cf3cf99c5d815f31cc6c9d3ef7a7394dde1: pass.- Dependency attribution check: graph count is 727 at
255d35ae0^and 740 at the reviewed commit; all 13 added module paths are absent fromgo.mod, have reach=0 ingo list -deps ./cmd/gc, andgo mod why -mreports them as not needed by the main module. - GitHub PR checks: required CI/preflight checks are green. The separate
Go module vulnerabilitiesjob is red becausegovulncheck@v1.4.0panicked (ForEachElement called on type containing *types.TypeParam) and then uploaded truncated SARIF; this PR does not change Go source or module files, so I did not classify that tool failure as a branch defect.
Not rerun locally: go vet ./... and make test-fast-parallel; the reviewed diff has no Go source changes, and the locally-run touched checks plus required CI cover this change.
…bump breached (gc-na313) `Preflight / static checks` has been red on main since 255d35a (beads pin -> v1.2.1, #138): native dependency guard: module graph has 740 modules; max is 727 Attribution is exact: the graph was 727 at 255d35a^ — sitting precisely on the ceiling — and 740 at the bump. Nothing since has moved it. All 13 new module paths are graph-only artifacts, not new native surface: none appears in our go.mod, `go mod why -m` reports "main module does not need module X" for all 13, and each has zero reach in `go list -deps ./cmd/gc`. Eleven are requirements declared in beads' own go.mod (the oapi-codegen OpenAPI generator toolchain); `cloud.google.com/go/pubsub/v2` and `github.com/zeebo/errs` ride in on version bumps of Google modules already present. They are in the build list only because Go's module-graph pruning admits every requirement declared in a directly-required module's go.mod, including its build tooling — so there is nothing here for us to trim short of `exclude` directives that fight upstream. Raising the ceiling is the correct call, and it follows upstream's own precedent: the beads v1.0.4 -> v1.1.0 bump raised it 725 -> 727 in the same commit. The bead reported the module ceiling as the only breach. It is not — that was measured without running the guard to completion. The script is a straight-line `set -e` sequence, so the binary is never built until the module check passes, and a second breach was hidden behind the first: native dependency guard: gc binary is 270733176 bytes; max is 270000000 That one is also the bump's doing. Measured: the same pre-bump commit built 267,809,008 bytes on CI (go 1.26.5) and 268,260,480 locally (go 1.26.6), so the environment spread is 0.17%; the bump adds 2,472,696 bytes (0.92%), putting CI at a projected ~270,281,704 — over the cap. Raising `max_modules` alone would have handed back a differently-red board. The two ceilings get different treatment on purpose. Module count is deterministic, so it stays an exact ratchet at 740. Binary size is not reproducible across toolchains, so an exact cap would go red on a Go patch bump alone; it becomes a round 280,000,000, a few percent above today's 270.7M. Both now carry a comment saying which kind of number they are. Also lands engdocs/contributors/beads-version-bump-anchors.md. The beads-bump anchor list has now been lost twice — first to a session scratchpad under /tmp (reaped), then into the notes of a closed bead (gc-lbfmj) — and both of the traps recorded there fail somewhere that never mentions beads. This puts it in the repo with the two ceilings added as traps 3 and 4, including the note that the binary cap eroded from 14% headroom (2026-05-31) to 0.8% (2026-08-14) without anyone noticing. Validation: `bash scripts/check-native-dependency-surface.sh` now runs to completion — modules=740 aws=25 azure=9 dolthub=15 googleapi=1 binary_bytes=270733176, exit 0. `make check-docs` passes (docsync link-validates engdocs/). No Go source is touched.
…a313) Three precision fixes to the page added in the previous commit, all found re-reading it end to end: - The index entry said three anchors fail somewhere that never mentions beads. There are four — the binary ceiling was added to the page after that bullet was written. - The intro claimed the version "appears in ... two guards". It does not: the guards measure what the new pin drags in, they do not name it. - The reach check used `grep -c '^<path>'`, which matches any module whose path merely starts with that prefix. Anchored to `(/|$)`, so a sibling module cannot report a false non-zero reach and talk someone out of a ceiling raise that was actually correct. The worktree recipe now removes the worktree it creates, and says why it compares paths and not raw `go list -m all` lines (every version bump reads as add+remove otherwise). `make check-docs` passes.
8d7f0cf to
22eb082
Compare
zook-bot
left a comment
zook-bot
left a comment
There was a problem hiding this comment.
VERDICT: request-changes
Reviewed branch: polecat/gc-na313
Reviewed base: main
Reviewed commit: 22eb082
Scope checked: Dispatch bead gc-69v80, anchor bead gc-na313, the full diff for scripts/check-native-dependency-surface.sh, engdocs/contributors/beads-version-bump-anchors.md, and engdocs/contributors/index.md. I checked the operator profile from the pack checkout, verified the referenced anchors against the current tree, and ran the touched gates at the reviewed commit. I did not run the full repository test suite because this branch changes one shell guard and contributor docs.
Findings:
P1 - scripts/check-native-dependency-surface.sh:11 carries environment-specific build measurements and "Today's build is ~270.7M" in a code comment. The operator profile for this pack says code comments only state constraints the code cannot show and never carry incident history. This comment embeds a dated measurement, and it is already false in my review run at the same commit: the guard reported binary_bytes=266686800, not about 270.7M. That makes the threshold rationale stale at merge time, and future bump reviewers can misread the remaining headroom. Fix by leaving only the stable constraint in the script, such as binary size varies by toolchain and the cap needs round headroom, and keep the CI/local measurements in engdocs/contributors/beads-version-bump-anchors.md.
P2 - engdocs/contributors/beads-version-bump-anchors.md:85 uses fixed scratch paths under /var/tmp/pre, /var/tmp/pre.paths, and /var/tmp/post.paths in a recipe contributors are supposed to copy during a dependency bump. A stale previous run or a concurrent bump investigation can make the command fail or compare against the wrong files. Fix by deriving a unique temp directory with mktemp -d, storing both path lists under it, and removing that worktree through git worktree remove --force in the cleanup path.
Verification:
- PASS:
TMPDIR="$REVIEW_WT/.tmp" bash scripts/check-native-dependency-surface.shat the reviewed commit. Output:modules=740 aws=25 azure=9 dolthub=15 googleapi=1 binary_bytes=266686800. - PASS:
make check-docsat the reviewed commit. Output:ok github.com/gastownhall/gascity/test/docsync 5.494s. - PASS:
git diff --check origin/main...22eb082ced96a5c49884a7f2e670cc3c56c45bc4. - NOTE: the first native-guard attempt from
/tmpfailed before the guard assertions because the Go linker returneddisk quota exceeded. Rerunning from/var/tmpwithTMPDIRinside the detached worktree succeeded.
Anchor: gc-na313 — check.codex @ 22eb082
zook-bot
left a comment
zook-bot
left a comment
There was a problem hiding this comment.
VERDICT: request-changes
Reviewed branch: polecat/gc-na313
Reviewed base: main
Reviewed commit: 22eb082
Scope checked: Review bead gc-8aqx6, anchor bead gc-na313, the full diff for scripts/check-native-dependency-surface.sh, engdocs/contributors/beads-version-bump-anchors.md, and engdocs/contributors/index.md. I checked the operator profile from the pack checkout, verified the referenced anchors against the reviewed tree, and ran the touched gates at the reviewed commit. I did not run the full repository test suite or Docker image build because this branch changes one shell guard and contributor docs.
Findings:
P1 - scripts/check-native-dependency-surface.sh:11 carries environment-specific build measurements and "Today's build is ~270.7M" in a code comment. The operator profile for this pack says code comments only state constraints the code cannot show and never carry incident history. This comment embeds a dated measurement, and it is already false in my review run at the same commit: the guard reported binary_bytes=266678448, not about 270.7M. That makes the threshold rationale stale at merge time, and future bump reviewers can misread the remaining headroom. Fix by leaving only the stable constraint in the script, such as binary size varies by toolchain and the cap needs round headroom, and keep the CI/local measurements in engdocs/contributors/beads-version-bump-anchors.md.
P2 - engdocs/contributors/beads-version-bump-anchors.md:85 uses fixed scratch paths under /var/tmp/pre, /var/tmp/pre.paths, and /var/tmp/post.paths in a recipe contributors are supposed to copy during a dependency bump. A stale previous run or a concurrent bump investigation can make the command fail or compare against the wrong files. Fix by deriving a unique temp directory with mktemp -d, storing both path lists under it, and removing that worktree through git worktree remove --force in the cleanup path.
Verification:
- PASS:
GOCACHE=/var/tmp/gc-review-cache-gc-8aqx6.UWKe2h GOTMPDIR=/var/tmp/gc-review-gotmp-gc-8aqx6.Ui4MaK TMPDIR=/var/tmp/gc-review-tmp-gc-8aqx6.zC1wZr bash scripts/check-native-dependency-surface.shat the reviewed commit. Output:modules=740 aws=25 azure=9 dolthub=15 googleapi=1 binary_bytes=266678448. - PASS:
GOCACHE=/var/tmp/gc-review-cache-gc-8aqx6.UWKe2h GOTMPDIR=/var/tmp/gc-review-gotmp-gc-8aqx6.Ui4MaK TMPDIR=/var/tmp/gc-review-tmp-gc-8aqx6.zC1wZr make check-docsat the reviewed commit. Output:ok github.com/gastownhall/gascity/test/docsync 6.696s. - PASS:
git diff --check origin/main...22eb082ced96a5c49884a7f2e670cc3c56c45bc4. - NOTE: the first native-guard attempt failed before the guard assertions because the Go compiler returned
disk quota exceededwhile using the default home Go cache. Rerunning with isolatedGOCACHE,GOTMPDIR, andTMPDIRunder/var/tmpsucceeded.
Anchor: gc-na313 — check.codex @ 22eb082
zook-bot
left a comment
zook-bot
left a comment
There was a problem hiding this comment.
VERDICT: approve
Reviewed branch: polecat/gc-na313
Reviewed base: main
Reviewed commit: 22eb082
Scope checked: Full diff for engdocs/contributors/beads-version-bump-anchors.md, engdocs/contributors/index.md, and scripts/check-native-dependency-surface.sh; anchor bead gc-na313; operator profile from template-fragments/operator-profile.template.md.
Findings: None.
Verification: bash scripts/check-native-dependency-surface.sh at the reviewed commit with TMPDIR on /var/tmp -> pass, modules=740 aws=25 azure=9 dolthub=15 googleapi=1 binary_bytes=266686776. make check-docs -> pass, test/docsync 6.649s. go vet ./... -> pass with GOTMPDIR on /var/tmp. Initial attempts using the default /tmp path failed before assertions with linker/compiler disk quota errors, so the retry moved only temporary build storage and kept the reviewed commit fixed.
Anchor: gc-na313 — check.codex @ 22eb082
1 participant
Raises both native-dependency ceilings (module count and binary size) that the beads v1.2.1 pin breached. Also lands
engdocs/contributors/beads-version-bump-anchors.mdas the durable home for the beads-bump anchor checklist referenced in the bead.Full attribution and validation detail is in bead
gc-na313's notes.Rebased onto current main; local gate results:
bash scripts/check-native-dependency-surface.sh— exit 0 (modules=740, binary_bytes=270691040)make check-docs— okgo vet ./...— cleanmake test-fast-parallel— all 10 jobs passedCloses gc-na313.