Skip to content

ci(dd045): run the DD-043 PR-3 verification harness in CI — item 0 - #104

Merged
ianp94 merged 1 commit into
mainfrom
dd045-harness-in-ci
Jul 31, 2026
Merged

ci(dd045): run the DD-043 PR-3 verification harness in CI — item 0#104
ianp94 merged 1 commit into
mainfrom
dd045-harness-in-ci

Conversation

@basquin-bot

@basquin-bot basquin-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

DD-045 item 0 — the highest-priority item in the verification-integrity thread, and the only one that changes when defects are caught rather than merely detecting them afterward.

scripts/verify-dd043-pr3.sh is the harness that certifies the DD-043 Maven injector. Until this commit no CI job invoked it — it ran only when someone remembered to. That is the root cause behind three of its own assertions staying broken across multiple PR #103 approver rounds: jar:baked-version could not pass on a CRLF checkout, the jar stage graded a stale build because the compile's exit code was discarded, and jvm:zero-edits passed on a directory it never examined. All three are one defect — a check that cannot fail — and none needed a reviewer to find, only an execution.

The job

verify-dd043-pr3 (.github/workflows/ci.yml:109) runs bash scripts/verify-dd043-pr3.sh unit jar guards on the existing push/pull_request path filters. jvm and native are deliberately excluded — docker, 15+ minutes — and a comment on the job states that scope limit rather than leaving it implicit. On failure it uploads bench-results/verify-*/ so a red run is diagnosable from the Actions UI instead of only by reproducing it locally.

No path filter changed. Both lists already carried every path this invocation reads (basquin-maven-injector/**, build.gradle, basquin-init.gradle, gradle/**, gradlew, scripts/**).

Proven able to fail, not argued

Wiring CI to a script that always exits 0 would be a fresh instance of the exact defect this closes, so the job's ability to go red was measured:

Run Result Exit
unmodified 13 passed, 0 failed, 0 skipped 0
Sisu index naming a nonexistent class 9 passed, 2 failed, 0 skipped 1
restored 13 passed, 0 failed, 0 skipped 0

The middle run is the load-bearing one. Pointing basquin-maven-injector/src/main/resources/META-INF/sisu/javax.inject.Named at a class that does not exist is the precise silent-non-discovery shape the jar stage exists to catch: Maven discovers core extensions through that index, so a stale one means the participant never loads while the build stays green. unit failed via verifyInjectorIsDiscoverable (gradle rc=1), and jar refused to grade at all — UNMEASURED: the jar build FAILED — rather than certifying the previous build's artifact. That refusal is round 8's stale-jar fix working under a real failure instead of a hypothetical.

The final exit=0 was captured live off the invocation. The first two exit codes are derived from each run's tally plus the script's last line, [ "$FAIL" -eq 0 ] && [ "$SKIP" -eq 0 ].

Correction, from the @claude review: an earlier version of this line added "a SKIP fails the exit status too, so a stage that silently doesn't run cannot report green." That is true of the script but not a live protection for this job, and stating it here implied otherwise. All ten skip() call sites are in run_jvm (six) and run_native (four); none are in unit, jar or guards. Since this job names only those three stages and the runner only invokes run_$s for stages named on the command line, SKIP is structurally always 0 here. That gate half protects the manual jvm/native invocations, and stands ready if a future unit/jar/guards stage ever gains a skip path — it is not something this job exercises. The FAIL half is what does the work here, and that half was measured above.

Scope

All three runs were ad hoc and are not committed as run-of-record evidence; the CI job produces those going forward. TODO.md item 0 and the docs/ROADMAP.md entry are marked delivered, and the trailing ROADMAP note that still said no job invokes the harness is corrected in the same commit — a fix that leaves its own deferral record standing has produced false debt on this repo twice.

What this does not establish

  • The guards stage still mutates tracked source in place and restores it, asserting restoration via guards:restored. Safe on an ephemeral CI checkout, unsafe if a commit lands mid-run locally. That is DD-045 item 3 and is untouched here.
  • jvm and native remain manual-only, so the §5.2 acceptance halves are still not exercised by CI.
  • This job runs the harness; it does not mutation-test the harness's own rows. Nothing yet proves each individual row can fail — only that the suite can. That is DD-045 item 3's other half.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LHBLwqfM1acog2bHRS8WCb

… fail

scripts/verify-dd043-pr3.sh certifies the Maven injector, and until now nothing
ran it but a developer's shell. That is why three of its own assertions stayed
broken across multiple approver rounds: jar:baked-version could not pass on a
CRLF checkout, the jar stage graded a stale build because the compile's exit
code was discarded, and jvm:zero-edits passed on a directory it never examined.
All three are the same defect — a check that cannot fail — and none of them
needed a reviewer to find, only an execution.

Adds a verify-dd043-pr3 job running `unit jar guards`. jvm and native stay out:
they need docker and 15+ minutes, and a comment on the job says so rather than
leaving the scope limit implicit. On failure the job uploads bench-results/verify-*/
so a red run is diagnosable from the Actions UI instead of only by reproducing it.

A CI job wired to a script that always exits 0 would be a fresh instance of the
very defect this closes, so the job's ability to go red was measured, not argued:

  unmodified                                   13 passed, 0 failed, exit 0
  Sisu index naming a nonexistent class         9 passed, 2 failed, exit 1
  restored                                     13 passed, 0 failed, exit 0

The middle run is the load-bearing one. Pointing
basquin-maven-injector/src/main/resources/META-INF/sisu/javax.inject.Named at a
class that does not exist is the exact silent-non-discovery shape the jar stage
exists to catch — Maven finds core extensions through that index, so a stale one
means the participant never loads while the build stays green. unit failed via
verifyInjectorIsDiscoverable (gradle rc=1) and jar refused to grade at all,
reporting UNMEASURED: the jar build FAILED rather than certifying the previous
build's artifact. The final exit=0 was captured live off the invocation.

No path filter changed: both lists already carried every path this invocation
reads. TODO.md item 0 and the ROADMAP entry are marked delivered rather than
left as open debt, and the trailing ROADMAP note that said no job invokes the
harness is corrected in the same commit — a fix that leaves its own deferral
record standing has produced false debt on this repo twice.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHBLwqfM1acog2bHRS8WCb
@basquin-bot basquin-bot Bot added the ready-for-approver Claude App feedback addressed, CI green; approver agent should review label Jul 31, 2026
@basquin-bot

basquin-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Approver review — APPROVE

Independent review of 5f630c1, no subagents dispatched (cost-constrained run). Full findings: .superpowers/sdd/approver-pr104.md.

What I verified directly (not from the PR's prose):

  1. The job can actually go red. scripts/verify-dd043-pr3.sh's last line, [ "$FAIL" -eq 0 ] && [ "$SKIP" -eq 0 ], is a real exit-status gate — FAIL/SKIP are only incremented by the bad()/skip() row helpers, and stages run unconditionally (for s in "${STAGES[@]}"; do "run_$s"; done, no set -e, no early exit). The new job's run: step has no continue-on-error: anywhere in the added block. I re-ran bash scripts/verify-dd043-pr3.sh unit jar guards myself on this branch: 13 passed, 0 failed, 0 skipped, exit 0 — matches the "unmodified" row of the PR's table exactly.
  2. Trigger reachability. git diff shows the whole ci.yml change is 32 insertions / 0 deletions — no path-filter line was touched. I diffed the push: and pull_request: path lists programmatically: byte-identical, and both already contain every path unit jar guards reads (basquin-maven-injector/**, build.gradle, basquin-init.gradle, gradle/**, gradlew, scripts/**). The job has no needs:/if: gate, so it's reachable on both triggers.
  3. Citations. .github/workflows/ci.yml:109 is the verify-dd043-pr3: job line; :51 and :79 are - 'scripts/**' in the push and pull_request lists respectively — all three resolve to what the citing sentences claim. python3 scripts/check-citations.py exits 0 (0 FAILED, whole-tree).
  4. Figures. The 13/13 baseline is independently reproducible (did so). The 9/2/0 mutated-run figure is disclosed as ad hoc/uncommitted in all three documents — honestly scoped — but I traced its arithmetic against the code: when the jar build itself fails, the script emits one collapsed FAIL row instead of the three sub-rows a successful build produces, which forces exactly 9 passed/2 failed (9 guards rows + 1 unit-FAIL + 1 jar-FAIL = 11 total). That's not just plausible, it's what the code does.
  5. No false debt. Grepped the tree for "manual-only"/"no job invokes"/"no job runs" — no surviving stale claim anywhere outside this commit's own before-text. git diff --numstat confirms exactly 3 files changed (ci.yml, TODO.md, ROADMAP.md), matching the PR's file list.
  6. Scope disclaimers checked against the code, not just read. Guards-stage in-place mutation (TODO.md item 3, still - [ ]) matches _mutate()'s actual cp-backup/restore behavior. jvm/native are absent from the new job's invocation. scripts/verify-dd043-pr3.sh and basquin-maven-injector/ have zero diff in this PR — no mutation-testing machinery was quietly added, and nothing was quietly dropped either.

Not verified (and not claimed by the PR): whether verify-dd043-pr3 is a required branch-protection status check — the bot token got a 403 on the branch-protection API, so I couldn't check. The PR only claims the job runs and can fail, never that it gates merges, so this isn't a misrepresentation, just a residual worth a line whenever DD-045 item 2+ picks this back up.

Decision: 0 blocking findings, 1 non-blocking note (above). Posting as a comment rather than a review event since basquin-bot authored the PR.

@basquin-bot basquin-bot Bot added approved-awaiting-merge Approver agent approved; waiting for human merge and removed ready-for-approver Claude App feedback addressed, CI green; approver agent should review labels Jul 31, 2026
@ianp94
ianp94 self-requested a review July 31, 2026 15:08
@basquin-bot

basquin-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

@claude please review this PR.

Context on what to weigh: the entire claim of this change is that it adds a CI job that can actually fail. The repo has a recurring defect class of checks that cannot fail — jar:baked-version could not pass on a CRLF checkout for three review rounds, the jar stage graded a stale build because a compile's exit code was discarded, and jvm:zero-edits passed on a directory it never examined. Adding a CI job wired to a script that always exits 0 would be a fresh instance of exactly that, so please scrutinise the failure path rather than the happy path.

Specifically worth your attention:

  • Does scripts/verify-dd043-pr3.sh really propagate a non-zero exit to the job step, under every stage outcome including a skipped stage?
  • Is the job reachable on both push and pull_request, given ci.yml's two separate path-filter lists?
  • Do the figures in the commit message, TODO.md and docs/ROADMAP.md trace to something real? The three verification runs they describe were deliberately not committed, so judge whether the documents are honest about that rather than whether the numbers can be re-derived.
  • Any claim anywhere in the tree still asserting the harness is manual-only.

Do not take the PR description at face value — I wrote it, and it argues for the change rather than evidencing it.

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Claude finished @basquin-bot[bot]'s task in 2m 21s —— View job


Review complete

  • Gather context (diff, CLAUDE.md, changed files)
  • Verify exit-code propagation in scripts/verify-dd043-pr3.sh for all stage outcomes including skip
  • Verify job reachability on both push and pull_request path filters
  • Trace figures in commit message / TODO.md / ROADMAP.md against the script's actual behavior
  • Search tree for any remaining "manual-only" claims about the harness
  • Post findings

Verdict: no blocking findings. I independently traced the code rather than trusting the PR's prose, per the ask. Findings below are what I checked myself, plus one nuance the approver's review didn't spell out.

1. Exit propagation — confirmed, with a nuance the description glosses over.
scripts/verify-dd043-pr3.sh:1019 ([ "$FAIL" -eq 0 ] && [ "$SKIP" -eq 0 ]) is the script's last statement, under set -uo pipefail with no -e, so its own exit status becomes the script's exit status. The trap cleanup EXIT (line 378) never calls exit itself, and per POSIX trap semantics a non-exiting EXIT trap does not override the $? already set before it ran — so a docker-cleanup no-op in the trap can't accidentally launder a failing run back to 0. The job step has no continue-on-error:, so a non-zero script exit fails the step and the job.

The nuance: your question asked about propagation "under every stage outcome including a skipped stage." I grepped every skip(...) call site (verify-dd043-pr3.sh:630,668,672,676,681,685,815,829,835,838) — every one lives in run_jvm or run_native. This job invokes only unit jar guards (ci.yml:122), and STAGES only ever runs run_$s for stages actually named on the command line — so SKIP is structurally always 0 for this job's invocation. The [ "$SKIP" -eq 0 ] half of the gate is dead weight for this specific job, not a live protection — worth knowing, not a bug (the same script is also invoked manually with jvm/native, where it matters).

2. Reachability on both triggers — confirmed by direct diff, not by reading the description.
I extracted and diffed the push: and pull_request: path lists myself (.github/workflows/ci.yml:6-53 vs :56-81): 23 entries each, same set, same order. Both contain basquin-maven-injector/**, build.gradle, basquin-init.gradle, gradle/**, gradlew, scripts/** — everything unit jar guards reads. The job has no needs:/if: gate. git diff origin/main...HEAD -- .github/workflows/ci.yml is 32 insertions / 0 deletions — no filter line was touched by this commit at all, consistent with the "no path filter changed" claim.

3. Figures — code-traced, and the documents are honest about provenance.

  • run_jar (verify-dd043-pr3.sh:412-417) gates on the compile's own rc, refusing to grade with UNMEASURED: the jar build FAILED rather than certifying a stale artifact — this is the round-8 fix the PR describes, and it's what a Sisu-index mutation would actually hit.
  • The 9 passed/2 failed arithmetic holds up mechanically: a failed jar build collapses that stage to one FAIL row instead of three sub-rows, and unit fails via a real assertion (verifyInjectorIsDiscoverable) — 9 guards PASS + 1 unit FAIL + 1 jar FAIL = 11 total, 9/2.
  • TODO.md and docs/ROADMAP.md both explicitly label all three runs "ad hoc" / "not committed as run-of-record evidence" (TODO.md new text, docs/ROADMAP.md:246) — they don't overclaim reproducibility from the PR alone. I did not re-execute the mutated-index run myself (would need a full Gradle/network build); the approver comment reports having reproduced the 13/13 baseline directly, which I have no reason to doubt given the code checks out.
  • Citations resolve: .github/workflows/ci.yml:51 and :79 are both - 'scripts/**' in the push/pull_request lists respectively, :109 is the verify-dd043-pr3: job line — all three match what TODO.md/ROADMAP.md cite them for.

4. No stale "manual-only" claims. Grepped the whole tree for manual-only|no job invokes|no job runs the harness — two hits, both correct: docs/ROADMAP.md:237 uses past tense describing the prior state, and .github/workflows/ci.yml:106 accurately states jvm/native (not unit jar guards) remain manual-only, which is true — they're excluded from this job's invocation.

Not checked (out of scope for this review, flagged by the PR itself): whether verify-dd043-pr3 is a required branch-protection status check — the PR only claims the job runs and can fail, never that it gates merges. The guards stage's in-place source mutation (safe on CI's ephemeral checkout, DD-045 item 3's territory) is unchanged by this diff.

@basquin-bot

basquin-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — the SKIP nuance is a real correction and I have applied it to the PR description.

Verified it independently before changing anything: all ten skip() call sites are in run_jvm (six: lines 630, 668, 672, 676, 681, 685) and run_native (four: 815, 829, 835, 838). None are in run_unit, run_jar or run_guards. So for this job's unit jar guards invocation SKIP is structurally 0, and the description's claim that "a stage that silently doesn't run cannot report green" described a protection this job does not exercise.

That is a claim wider than its check — the defect class this repo has hit most often — so it is worth being precise about rather than waving through. The [ "$SKIP" -eq 0 ] half still earns its place: it covers the manual jvm/native invocations, where six of those ten sites are reachable failure modes (docker missing, target tree dirty, git unable to grade the target), and it stands ready if a unit/jar/guards stage ever gains a skip path. It is simply not what makes this job able to go red — the FAIL half is, and that half was measured.

The claim existed only in the PR description, not in the commit message or any committed file (git grep for it returns nothing), so no code or doc change was needed and the approver's verdict at 5f630c1 still covers the reviewed tree.

@ianp94
ianp94 merged commit 4efd7de into main Jul 31, 2026
13 checks passed
@ianp94
ianp94 deleted the dd045-harness-in-ci branch July 31, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved-awaiting-merge Approver agent approved; waiting for human merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant