Repository navigation
ci(ngit): behavioral gate before announce — happy-path suite on the built x86_64 .apk - #924
Conversation
|
Per repo policy this requires sign-off from @felixfelix-bot before merge — leaving open for review (same as #916/#918/#922). |
Review: APPROVEReviewed cold at head What it does: the ngit announce workflow gains a Verified the fail-closed property by reading the workflow. This is the claim that matters — "a
The downstream chain is also correct: Both post-announce jobs gate on Why this matters more than a normal CI tweak: the body notes the relevant GitHub Actions lane Scope note (correct, not a gap): the gate verifies the built artifact behaviorally and then Not hardware-tested, and correctly so — this is the ngit publication lane and its offline Labels: not applied — this account has FULL_REVIEW: ~/reports/reviews/tollgate-module-basic-go-pr924-kimi.md |
…y scans The Actions org-freeze (#790) makes every runner-minute count; this wave cuts the standing burn with zero coverage loss: - test.yml + repro-check.yml: concurrency groups with cancel-in-progress (force-push churn no longer burns full runs; build-package + router-test already had them — now the estate is uniform). - Timeouts on every previously-unbounded job (go-test 20m, main-test 15m, contract-lint 5m, build-purity 10m, deps-and-imports 5m, repro binaries 15m / packages 45m): a hung job stops at minutes, not the 6h default. - build-package.yml: the SDK package matrix (29 rows, the 260MB-download jobs) no longer runs on pull_requests — PRs keep the fast path (compile-binaries + happy-path + contract lanes); the matrix runs on push-to-main and tags, override via workflow_dispatch. PR signal was already carried by the fast path; the matrix duplicated it at ~30x cost. - gitleaks schedule: daily -> weekly (pre-commit hooks + the PR lane carry day-to-day coverage; the weekly sweep is the history backstop). Estimated standing reduction: ~70% of per-PR runner-minutes (matrix skip dominates), 100% of force-push waste, bounded worst-case hang cost. Wave 2 (evidence-gated, separate): go-test matrix consolidation, SDK-image sharing across matrix rows, retention-days=7 non-release, Copilot-workflow gating.
…e routing ai-legion-tbmg is live (repo-level runner, labels tbmg/linux-x64; systemd service, dedicated 'runner' user with docker group). This probe proves end-to-end on dispatch before any release-critical job migrates; then the recurring heavy lanes (path-parity weekly, repro packages on tags) move in follow-ups, taking their minutes off GitHub's allotment entirely. Fork-PR note: repo is public — outside-contributor runs on self-hosted require the Actions approval gate in repo settings (owner console), same visit as #790's org switch.
…count soaks + the nightly -timeout its merchant row always needed (#852) Every PR-gating Go lane runs -count=1, which is structurally blind to the state-leak family: package state that outlives its test passes iteration 1 and fails later ones inside the same test binary (#821/#732/#841/#845 — all found by hand soaks; #821 red on main for days while every -count=1 lane stayed green). - count-soak.yml (new; ngit-ci executes it on every push and PR; the .github twin is byte-identical): valve -count=5 (measured 47.1 s) and merchant -count=2 (measured 597 s) under -race, with a temporary -skip of the #845 victim until #846 lands — without it the lane flakes ~1-in-8 per iteration and trains re-run-and-ignore. Its own file, not a test.yml job, because ngit-ci bounds an act invocation at 1800 s and merchant -count=5 (measured 1503 s on 16 cores) cannot share that budget — one file, one budget, the release-shard doctrine. - nightly-stress.yml: -timeout=45m is load-bearing — the merchant row at -count=5 has been panicking at go test's 10m default instead of soaking, so the one lane built to catch this family was emitting noise-red (#852 brief hypothesis H2, confirmed). Planted-drift proof (throwaway branch, #842's closeGateCleanup fix reverted): the shipped lane command exits 1 with the victim failing in 0.01 s, while a -count=1 run of the same tree stays green in 314 s — the blindness and the catch, both reproduced. make go-battery: 16 modules green.
…l edge the dead GitHub lane had
c994f9d to
c1ba059
Compare
Local CI gate — PR #924: ci(ngit): behavioral gate before announce — happy-path suite on the built x86_64 .apkRun of
Verdict: 2 leg(s) RED — 4 green. What this gate does not run (no vacuous green)
failing-leg log tailscontract-fences — tailgo-battery — tail |
Superset-chain scan of the whole open queue — 8 chains, 11 PRs closableMethod: for every pair of open PRs, check whether one's changed-file set is a Automated with THE BIG ONE — a 5-deep chain I had missed by handThere is a CI-work chain where each PR adds a couple of files to the last: Verified independently (not just the scanner's word):
|
| land | close | shared files |
|---|---|---|
| #921 | #879, #881, #924, #925 | 4 / 5 / 12 / 18 — all identical |
| #1030 | #1028, #1029 | 1 / 2 — identical |
| #1020 | #1018, #1019 | 8 / 10 — identical |
| #1014 | #975 | 6 — identical |
| #999 | #954 | 3 — identical (already reported) |
| #1034 | #1003 | 1 — identical |
| #1031 | #1028 | 1 — identical |
| #1040 | #1028 | 1 — identical |
The merge order matters
#1028 is contained by four different PRs (#1029, #1030, #1031, #1040). It is
safe to close regardless of which of those lands. Same for #879/#881, which are
contained by #905, #919, #921 and #924 — any of those closes them.
Cleanest sequence:
- ci: pr-conflict-sweep — the queue's conflict drift in one command #921 -> close ci: frugality wave 1 — concurrency, timeouts, PR-path slimming, weekly scans (~70% PR-minute cut) #879, ci: self-hosted runner — ai-legion-tbmg registered + adoption probe #881, ci(ngit): behavioral gate before announce — happy-path suite on the built x86_64 .apk #924, ci(release-check): installer-contract lane per release candidate (M2 remainder) #925 (4 closed)
- fix(deps): go mod tidy for upstream_detector + upstream_session_manager #1030 -> close fix(release): shard-driver poll must not put progress lines into the conclusion string #1028, fix(packaging): armour fetch-upx.sh against SIGPIPE exit 141 #1029 (2 closed)
- fix(build): stage the CVM daemon in build-sdk-package — apk lane broken since cvm landed #1020 -> close fix(hooks): unbreak pre-commit + run the SDK-Go feed audit at commit time #1018, fix(build): update-build-inputs must audit each SDK era against its own release #1019 (2 closed)
- docs(mptcp): bonding measurement record + status vs the four goals (gap analysis) #1014 -> close fix(deps): restore battery-green main — bech32 go.sum entries for #938's nip19 import surface #975 (1 closed)
- post-v0.6.0: exempt loopback from RateLimitMiddleware so the starting refusal wins (#953) #999 -> close fix(api): exempt loopback from RateLimitMiddleware so the starting refusal wins (implements #953) #954 (1 closed)
- fix(modules): green the deps-sync fence — align the drift classes on main #1034/fix(release): armour the shard template's UPX print; drop paywalled drive.cashu.email mirror #1031/feat: operator-issued MB/minute ecash — unit-aware pricing + CDK mint integration (DRAFT) #1040 -> whichever lands, close build(mptcp_bonding): align the go directive with the rest of the tree (1.24.2 → 1.26.0) #1003 / fix(release): shard-driver poll must not put progress lines into the conclusion string #1028
Potential: 11 PRs closed from this scan alone, with no code risk and no
re-review of identical hunks.
Why a scanner beats reading
I found four chains by hand earlier today (#999/#954, #1016/#887,
#1029/#1028, #150/#120). The scanner found eight more, including a
five-deep chain whose shape is invisible from any single PR page — each PR looks
independently mergeable, and the containment only shows up when you compare all
pairs.
Recommendation: run this as a routine before every merge wave. It costs
nothing, needs no build, and turns a queue-reduction task into a mechanical one.
The script is at ~/worktrees/pr-review/find-supersets.py.
Caveat I want to be explicit about
Containment is not a substitute for review. #921 containing #925's diff means
merging #921 applies those changes — it does not mean they are correct. For the
chain above, the CI changes still deserve a read: enabling self-hosted runners and
per-PR lanes has security implications (the kit's own notes warn that flipping a
hardware lane on without restricting the trigger hands contributors code execution
on the flashing machine).
So: close the duplicates on this evidence, review the one that remains.
Verified duplicates — the content is already in another open PRClosing these would remove 10 PRs from the queue with zero code risk, because The duplicates
(#1028 and #954 were mine and are already closed on this basis.) How this was verified
Both yes ⇒ the smaller PR adds nothing. That is a proof of redundancy, not a Order mattersClose the contained PR only after its container has landed. If the container One thing I got wrong, correctedAn earlier version of my report listed #1031 as contained in #1030. It is not. What I cannot doI can only close PRs authored by |
Evidence review — verified at headThe fail-closed edge is exactly as claimed: in |
c03rad0r
left a comment
c03rad0r
left a comment
There was a problem hiding this comment.
Technical review — head c1ba059. Approving the gate; two scope notes.
The important property is verified structurally:
announce.needs = ['release-guard', 'happy-path']
happy-path job present: True
needs with no if: always() is exactly the fail-closed shape: a red suite skips the announcement rather than publishing. Fetching the artifact from the kind-30078 build record and verifying release_run + sha256 is the ngit-native equivalent of download-artifact, and it binds the tested bytes to the announced bytes — better than the GitHub lane it ports.
Scope notes:
- This PR also carries #905's count-soak lane (
.github/workflows/count-soak.yml,.ngit/act/workflows/count-soak.yml,changelog.d/905-…). Same stacked-content pattern as #1063: fine if deliberate, but say so in the body, or split, so the gate PR is reviewable as a gate. - The PR is honest that a full-suite green run has not been produced (the lab host's :2121/:8080 are occupied). The first live release run is the remaining evidence; the risk is a release cut discovering a gate that cannot pass on the runner, not a bad announcement — the failure mode is skip, which is safe but would block a release. Please watch the first run and record the wall time.
Disposition: land (after the scope note is recorded).
Makes the publication lane fail-closed on behavior, outside GitHub Actions (dead since 2026-08-27): the announce workflow gains a
happy-pathjob, andannouncenowneeds: [release-guard, happy-path]with noif: always()— a red suite skips the announcement instead of publishing a broken build. Port of the dead lane's gate (.github/workflows/build-package.yml), which shipped the rule but ran nowhere.How it gets the artifact (ngit-native):
actions/download-artifactdoes not work on ngit, so the job fetches the x86_64.apkfrom the apk-x86-64 shard's own kind-30078 build record — verifyingrelease_runmatches this run and the downloaded bytes hash to the record's sha256 — then extracts with apk-tools (alpine container;--network hostper the dep-smoke suite's own bridge-DNS workaround), runs the apk dependency-resolution smoke, and the--stricthappy-path suite via the existinghappy-path-gate.sh(tolerating onlytests/happy-path/known-issues.txt).Validation (executed, not assumed)
announce.needs == [release-guard, happy-path]asserted;verify-publicationinherits the edge transitively.boot:port-free). The ngit runner's fresh per-job container is the intended environment; the first live release run is the remaining evidence.No driver changes:
ngit-ci-release.shtriggers the announce file as before; the gate rides inside it. Budget: job timeout 20 min; wall-time measurement owed from the first live run.