ci: add stable runtime-closure check (inbox#21) - #280
Conversation
A report-only helper + workflow: given a package set (or the PR diff), compute the transitive runtime closure and report whether branch `stable` already contains it (missing = closure - stable - co-promoted), de-risking the "stable must stay dependency-closed" promotion invariant. Replicates the engine's needed_for_internet injection (transitives.rs pulls ca-certificates into the runtime closure of any needs.internet package) so the BFS doesn't under-count and false-PASS (e.g. promoting gh/curl before ca-certificates). Runtime-only, name-presence-only -- both honest caveats for #20. Additive, contents:read, continue-on-error + --no-fail; keep off required checks until #20 turns it into a gate (no code change needed to flip it). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a new Python script ( ChangesStable Runtime-Closure Check
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/stable-closure-check.py:
- Around line 203-208: The early exit when no seed packages are found (in the
condition checking if not seeds) returns exit code 2 unconditionally, but does
not respect the args.no_fail flag which is meant to enable report-only mode.
When --no-fail is set, a PR with no package changes should be treated as a
successful case (nothing to promote) rather than an error. Modify the early exit
logic to check if args.no_fail is True before returning 2; if --no-fail is
enabled, return 0 instead to allow the script to complete successfully for PRs
that only modify non-package files like .github/.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9069f554-06c2-4a08-ac1d-da3f205b7613
📒 Files selected for processing (2)
.github/scripts/stable-closure-check.py.github/workflows/stable-closure-check.yml
There was a problem hiding this comment.
One line-level finding in the workflow shell. The Python script itself is well-structured — the BFS + internet-injection parity logic is sound, error handling is consistent, and the --no-fail / exit-code contract is clearly documented. CodeRabbit's existing finding about the "no seeds → exit 2" path not respecting --no-fail is valid and complementary to this shell issue.
| - uses: ./.github/actions/setup-minimal # puts `minimal` on PATH | ||
| - name: Runtime-closure vs stable | ||
| run: | | ||
| set -uo pipefail |
There was a problem hiding this comment.
Bug: missing -e makes the step always exit 0, masking script failures.
set -uo pipefail without -e (errexit) means non-zero exit codes from intermediate commands don't halt execution. If the Python script exits 2 (operational error — not caught by --no-fail), the shell discards that exit code and continues to the step-summary block. Since the final command (} >> "$GITHUB_STEP_SUMMARY") always succeeds, the step's exit code will be 0 regardless of the script's actual result.
This undermines the intent of --no-fail being the sole control for exit behavior — the shell inadvertently makes every run appear successful.
| set -uo pipefail | |
| set -euo pipefail |
With -e, a script exit-2 (e.g. the "no seeds" case when it's eventually fixed, or minimal dump failing) will properly propagate through the step, letting continue-on-error: true at the job level show the step as red in the summary while still not blocking the workflow.
A report-only run that touches no packages (the workflow's --changed mode on a PR with no packages/* changes, e.g. this PR's own CI) found no seeds and exited 2, which under set -o pipefail turned the closure job red. Report-only checks must not fail on no-op PRs. Treat no-seeds as "nothing to check" (exit 0) in --changed or --no-fail mode; keep exit 2 only for an explicit bare invocation. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…contract
An adversarial verification against the engine's runtime-closure semantics
(transitives.rs) confirmed the good news first: the gated needed_for_internet /
ca-certificates injection is faithfully replicated, so curl/gh/go correctly
require ca-certificates -- no false-PASS from the closure math itself.
But the guard could still go green without verifying anything, via two
fail-OPEN input paths:
- An unknown / namespace-mismatched seed only warned, then its closure =={itself}
and was subtracted away -> empty missing -> exit 0 PASS. A typo or a natural
name (python3, nodejs) silently verified nothing. Now: verdict UNVERIFIED,
exit 2 (blocking); --no-fail still exits 0 but prints UNVERIFIED, never PASS.
- An empty-but-valid dump ([]) made every closure == its seeds -> vacuous PASS.
Now guarded in build_runtime_map (covers the --stable-worktree path too) ->
exit 2.
Also:
- --no-fail now genuinely never blocks: operational failures (missing minimal
binary, unfetched origin/stable, dump error) route through a no-fail-aware
fail() that returns 0 with an explicit UNVERIFIED verdict instead of exiting
2 before the guard. Exit 1 stays reserved for a computed non-empty `missing`.
- git calls now honor --repo (cwd=args.repo), not just `minimal dump`.
- A missing stable ref is detected up front (git rev-parse) with the actionable
fetch hint, instead of a raw git error.
Verified against the real catalog: ca-certificates injection is load-bearing
(gh's closure includes it only with the injection); PASS / NOT CLOSED /
UNVERIFIED / empty-dump exit codes are 0 / 1 / 2 / 2.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Verified against the engine's runtime-closure semantics + closed two fail-open holesRan an adversarial verification of this helper against the real engine ( The good news (the part I was most worried about): the closure math is faithful. The helper correctly replicates the engine's gated The fixes (latest commit): the guard could still go green without verifying anything, via two fail-open input paths:
Plus: Verified against the real catalog: PASS / NOT CLOSED / UNVERIFIED / empty-dump exit codes are 0 / 1 / 2 / 2. Deferred (residual, for whoever wires the workflow): the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/stable-closure-check.py:
- Around line 53-56: The early exit paths in the script, specifically the
NO_FAIL block where it prints the unverified verdict message and the no-seed
success path around line 242-244, print plain text output using print() without
checking the --format json flag. To fix this, add a conditional check before
both print statements to determine if JSON format has been requested, and if so,
construct and output a JSON-formatted response instead of the plain text
message. The fix should apply to both the NO_FAIL exit path and the other early
exit path mentioned, ensuring that when --format json is specified, the output
is machine-readable JSON instead of plain text, while preserving plain text
output when JSON format is not requested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f88c9ec9-5410-4ae3-a263-936b1316187f
📒 Files selected for processing (1)
.github/scripts/stable-closure-check.py
| if NO_FAIL: | ||
| print(f"VERDICT: UNVERIFIED — could not complete the closure check " | ||
| f"({msg}). Non-blocking (--no-fail).") | ||
| sys.exit(0) |
There was a problem hiding this comment.
Honor --format json on early exit paths.
Both the fail-open path and the no-seed success path can exit 0 while writing plain text despite --format json, which breaks machine-readable consumers of this report-only tool.
Proposed fix
NO_FAIL = False
+OUTPUT_FORMAT = "text"
def fail(msg: str, code: int = 2):
eprint(f"error: {msg}")
if NO_FAIL:
- print(f"VERDICT: UNVERIFIED — could not complete the closure check "
- f"({msg}). Non-blocking (--no-fail).")
+ if OUTPUT_FORMAT == "json":
+ print(json.dumps({
+ "seeds": [],
+ "unknown_seeds": [],
+ "arches": [],
+ "stable_source": None,
+ "closure": [],
+ "on_stable": [],
+ "missing": [],
+ "verdict": "UNVERIFIED",
+ "closed": False,
+ "error": msg,
+ }, indent=2))
+ else:
+ print(f"VERDICT: UNVERIFIED — could not complete the closure check "
+ f"({msg}). Non-blocking (--no-fail).")
sys.exit(0)
sys.exit(code)- global NO_FAIL
+ global NO_FAIL, OUTPUT_FORMAT
NO_FAIL = args.no_fail
+ OUTPUT_FORMAT = args.format
arches = args.arches or ["amd64", "arm64"] # --changed with no changed packages (or any --no-fail run) is normal,
# not an error: there is simply nothing to check. Exit 0 so the
# report-only workflow stays green on PRs that touch no packages.
if args.changed is not None or args.no_fail:
- print("no seed packages (no packages/* changed) -- nothing to check.")
+ if args.format == "json":
+ print(json.dumps({
+ "seeds": [],
+ "unknown_seeds": [],
+ "arches": arches,
+ "stable_source": None,
+ "closure": [],
+ "on_stable": [],
+ "missing": [],
+ "verdict": "PASS",
+ "closed": True,
+ "note": "no seed packages",
+ }, indent=2))
+ else:
+ print("no seed packages (no packages/* changed) -- nothing to check.")
return 0Also applies to: 242-244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/stable-closure-check.py around lines 53 - 56, The early exit
paths in the script, specifically the NO_FAIL block where it prints the
unverified verdict message and the no-seed success path around line 242-244,
print plain text output using print() without checking the --format json flag.
To fix this, add a conditional check before both print statements to determine
if JSON format has been requested, and if so, construct and output a
JSON-formatted response instead of the plain text message. The fix should apply
to both the NO_FAIL exit path and the other early exit path mentioned, ensuring
that when --format json is specified, the output is machine-readable JSON
instead of plain text, while preserving plain text output when JSON format is
not requested.
What
A report-only dependency-closure helper for the
stablechannel (gominimal/inbox#21)..github/scripts/stable-closure-check.py— given a package set (or--changedfrom the PR diff), computes the transitive runtime closure and reports whetherstablealready satisfies it (missing = closure − stable − co-promoted), with a PASS / NOT-CLOSED verdict..github/workflows/stable-closure-check.yml— runs it onpull_request,continue-on-error, posts to the job summary.Why it's safe (two-way door)
Additive new files; read-only;
continue-on-error+--no-failso it never blocks. De-risks the highest-stakes promotion invariant ("stablemust stay dependency-closed") without touching builds/cache/branches. Revert = delete the two files.The correctness fix worth noting
A naive BFS over
runtime_depsunder-counts: the engine (transitives.rs) injects everyneeded_for_internetprovider (ca-certificates) into the runtime closure of anyneeds.internetpackage (~60:gh,curl,go,rust…). Without replicating that it would false-PASS (e.g. promotingghbeforeca-certificates). The injection is replicated here, with a regression test (closure(gh)must containca-certificates).Honest caveats (for #20)
Runtime-only (if
stableis a buildable channel, addbuild_deps— one flag) and name-presence-only (not version/ABI-skew aware). Withstablecurrently frozen ~48 commits behindmain, expect near-universal NOT-CLOSED today — correct, and why it must stay non-blocking for now.Refs gominimal/inbox#21.
Summary by CodeRabbit
Chores
stablechannel preserves the expected runtime dependency closure.main(and manual runs), and reports a clear PASS/NOT CLOSED/UNVERIFIED verdict in the build summary to help prevent regressions.