Skip to content

REVIEW SLICE 1/2 — v1.139 gate tooling + CI workflows. DO NOT MERGE - #458

Closed
avrabe wants to merge 1 commit into
review/v1.139-basefrom
review/v1.139-gates
Closed

avrabe wants to merge 1 commit into
review/v1.139-basefrom
review/v1.139-gates

Conversation

@avrabe

@avrabe avrabe commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Review-only slice of the falcon-v1.139.0 candidate. DO NOT MERGE. Base is pinned at the falcon-v1.138.0 tag; this branch adds only scripts/ and .github/ from main. 19 files, +2 928 / −34. Deleted once triaged.

This is the half that decides whether a release may call itself verified, so a wrong verdict here is worse than no verdict at all.

  • scripts/check-evidence.rs — refuses artifacts whose cited evidence cannot have run.
  • scripts/check-component-claims.rs — refuses "formally verified" wording for tracks that do not cover the crate.
  • scripts/capability-reachability.rs — derives "delivered" from the dependency graph rather than prose.
  • scripts/verification-tracks.rs — renders per-track state and the structural gaps into the release notes; exit 2 must print UNKNOWN, never green.
  • scripts/release-readiness.rs — the tag gate; exit 0/1/2 and what it does with unparseable artifacts.
  • scripts/ci-wedge-watch.rs — flags a job stuck past 5× its own median.
  • .github/workflows/{kani,gazebo,fleet-status,verification-gate,release}.yml and .github/actions/setup-gh.

The failure mode to hunt: a check that passes having verified nothing — empty scope treated as success, a masked step, a swallowed API error, an exit code lost to shell: bash's -e. Three of those were found by hand in this release (#432's || rc=$?, the Format gate's continue-on-error, the soak ladder's lost measurement); the question is what is left.

Known and deliberate: the Verus track fails on every commit (#405) and the verification gate never runs on main (#410). Both are named as structural gaps in the release notes rather than fixed here.

🤖 Generated with Claude Code

avrabe commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Claude Code review — 7 findings on commit f01e895

The review reached its time or budget limit and stopped early, so it may not have covered the whole change.

🔴 Normal — .github/workflows/soak.yml:137

soak.yml interpolates workflow_dispatch input directly into a run: block, violating the explicit convention stated in fleet-status.yml added in the same PR ("no ${{ }} is expanded inside any run: block; every input is bound through env: and dereferenced as a quoted shell variable").

🔴 Normal — .github/workflows/soak.yml:141-142

OUT=$(timeout ... falcon-sitl-gz ...) runs under GitHub Actions' default bash -e, and falcon-sitl-gz exits 1 on a FAIL verdict (examples/falcon-sitl-gz/src/main.rs:99), so the ladder's else-branch ('hold breaks at ${D}s') is unreachable — the step dies at the first FAIL before BEST is echoed or compared to GZ_FLOOR_S.

🔴 Normal — scripts/check-component-claims.rs:175

Three new gates the PR headlines as its purpose are never invoked — check-component-claims.rs, check-evidence.rs, capability-reachability.rs have no caller in any workflow, artifact evidence step, or build/rivet config, so the checks they perform gate nothing. [also at: scripts/check-evidence.rs:425 - Three of the five new gate scripts (check-evidence.rs, check-component-claims.rs, capability-reachability.rs) are invoked by no workflow — the PR's headline gating capability is absent.]

🔴 Normal — scripts/check-evidence.rs:117

The unset-variable check reads the tool's own runtime env (std::env::var(v).is_err()), so the same artifact's evidence census reaches different verdicts depending on which machine runs the tool — a placeholder like $WITNESS fails on a laptop but is silently accepted on any runner or dev box that happens to export that name.

🟡 Nit — scripts/capability-reachability.rs:155

relay_imports is implemented twice with divergent boundary logic: audit-component-deps.rs excludes a preceding _, capability-reachability.rs (line 155) does not, so my_relay_foo yields a phantom relay-foo root there. Extract a shared helper so both scanners agree.

🟡 Nit — .github/workflows/release.yml:415

release.yml lines 409–416 (else --generate-notes branch) are now dead: the new notes step unconditionally writes notes_path=$OUT, so [ -n "${NOTES_PATH}" ] is always true.

🟡 Nit — .github/actions/setup-gh/action.yml:32

The cache guard if [ ! -x "$dir/bin/gh" ] under $RUNNER_TEMP never hits — GitHub empties $RUNNER_TEMP at the start and end of every job — so gh is re-downloaded (curl + sha256sum + tar) on every job that composes this action, at least 2× per 15-minute fleet-status tick.


Generated by Claude Code

avrabe added a commit that referenced this pull request Sep 18, 2026
… two gates that stop measuring, and a machine-dependent verdict (#460)

From the review of the release candidate (#458). Four real findings and
three phantoms; the phantoms were artefacts of how the candidate was
sliced for review and are recorded in the PR, not here.

1. INJECTION. soak.yml interpolated `${{ github.event.inputs.gz_max_s }}`
   straight into a run block, which Actions expands before bash sees it:
   a dispatch-capable actor could end the assignment and run commands
   with the job's environment. The repo's own convention (stated in
   fleet-status.yml, followed by release.yml) is to bind inputs through
   `env:` and dereference a quoted shell variable. Now it does.

2. THE ANALYTIC TIER LOST ITS RUNGS the same way the gz ladder did.
   `shell: bash` runs with -e, the bench exits 1 on a FAIL verdict, so
   `OUT=$(...)` ended the step before the FAIL=1/continue arm could walk
   the remaining durations — a red step with no measurement. The exit
   code is captured now, as in the gz ladder.

3. A MACHINE-DEPENDENT VERDICT. check-evidence.rs decided whether a
   `$VAR` in an artifact step was an unfilled placeholder by reading the
   TOOL'S OWN environment, so `$WITNESS` was refused on a laptop and
   accepted on any runner that happened to export it — the census
   disagreed with itself across machines. It now resolves only what the
   step assigns itself plus the gate-supplied names; measured identical
   with and without WITNESS exported.

4. The two `relay_` scanners disagreed on an identifier boundary
   (`my_relay_foo` synthesised a phantom `relay-foo` root in one of
   them); setup-gh cached under $RUNNER_TEMP, which Actions empties at
   both ends of every job, so the guard never hit and gh was
   re-downloaded twice per fleet tick; and release.yml's
   `--generate-notes` fallback could not fire. That last one is now a
   loud failure rather than a dead branch — deleting it outright would
   have left a silent no-op if the notes body were ever empty, which is
   worse than the dead code.

All seven gate scripts' own test suites pass (7/4/6/0/8/4/4).

Refs #458


Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@avrabe avrabe closed this Sep 18, 2026
@avrabe
avrabe deleted the review/v1.139-gates branch September 18, 2026 06:55
avrabe added a commit that referenced this pull request Sep 18, 2026
…un `gh`

The first draft of FV-RELAY-REVIEW-139's steps cited
`gh pr view 458/459` as the evidence of the findings. The verification
gate failed them with rc=127: gh is not installed on the self-hosted
runners — the same lesson as #153/#436, walked into again by writing a
step that only runs on a laptop.

The deeper problem is that it made a release's audit trail depend on
GitHub PR comments, which can be edited and belong to PRs that can be
deleted. The findings and their nine dispositions now live in
docs/reviews/falcon-v1.139.0-review.md, and the steps check that file:
it exists, and it carries exactly nine dispositions.

All four steps verified by running them as the gate does (sh -c).

Refs #458, #459

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG
avrabe added a commit that referenced this pull request Sep 18, 2026
… dispositioned (#461)

* verify(v1.139): the review checkpoint is done — 9 findings, every one dispositioned

/code-review ultra ran on the candidate falcon-v1.138.0..9f1df80 on
2026-09-18. It could not be reviewed as one diff (186 files, 21 420
lines, ~9.5 k of them lockfile churn), so it was split against a base
pinned at the previous tag: #458 (gate tooling + CI, 19 files) and #459
(the 28 source files with 50+ changed lines). Neither was ever merged.

  FIXED in #460 (f9ea446, on main before any tag): an injectable
  workflow_dispatch input; the analytic soak tier losing its rungs to
  -e; check-evidence.rs reading its own environment to decide whether a
  $VAR was a placeholder; two relay_ scanners disagreeing on an
  identifier boundary; setup-gh caching where Actions wipes it; a dead
  --generate-notes fallback, now a loud failure rather than a silent
  no-op.

  FILED into v1.140 by maintainer decision: losing the battery sense in
  flight raised no failsafe (#413's in-flight half) — reproduced, fixed
  on the v1.140 branch, SWREQ-FALCON-BATTERY-P03 + FV-FALCON-BATTERY-003.

  DISMISSED with measurements: three findings that were artefacts of the
  slicing (impls already updated on main; scripts cited in rivet steps
  the slice excluded; all three scripts exit 0 on main at 9f1df80).

NOT claimed: the reviewed diff excluded artifact YAML, the component
descriptions, wasm/, tests/, examples/, docs and lockfiles — the claims
TEXT a partner reads was checked mechanically, not by a reader.

Code-free promotion (two-commit rule): SWREQ-RELAY-REVIEW-P01 and
FV-RELAY-REVIEW-139 proposed -> verified.

Refs #458, #459

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG

* verify(v1.139): put the review record in the repo — the gate cannot run `gh`

The first draft of FV-RELAY-REVIEW-139's steps cited
`gh pr view 458/459` as the evidence of the findings. The verification
gate failed them with rc=127: gh is not installed on the self-hosted
runners — the same lesson as #153/#436, walked into again by writing a
step that only runs on a laptop.

The deeper problem is that it made a release's audit trail depend on
GitHub PR comments, which can be edited and belong to PRs that can be
deleted. The findings and their nine dispositions now live in
docs/reviews/falcon-v1.139.0-review.md, and the steps check that file:
it exists, and it carries exactly nine dispositions.

All four steps verified by running them as the gate does (sh -c).

Refs #458, #459

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
avrabe added a commit that referenced this pull request Sep 18, 2026
…otes call that DARK (#463)

#460 fixed the ANALYTIC tier's -e trap and left the gz ladder's on the
v1.140 branch, so every soak run on main still dies at the first
breaking rung: exit 1, no rung printed, no reason, and no
longest-holding number — the value this job exists to publish.

The cost is not just a red job. scripts/verification-tracks.rs renders
the track into the release body, so falcon-v1.139.0 would have shipped
notes reading "Nightly endurance soak — DARK, failure", which a reader
takes as "the hold is broken". The truth is narrower and already
documented in #403: the gz hold breaks at 60 s and holds 40 s, and the
ladder threw that measurement away.

Same fix as the analytic tier: capture the rung's exit code, and say
which failure it was (timeout, verdict FAIL, or a bench that exited
without a verdict).

Found while reading what the release notes would actually say.

Refs #403, #458


Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG

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.

1 participant