Skip to content

check run: a declared check that SKIPs is banked as a PASS — the headline, the summary.yml and the exit code all read green (charly#904) #132

Closed

Description

@aitrawog

Finding (the product half of opencharly/charly#904)

A bed run whose declared check SKIPPED reports PASS and exits 0. The defect is in THIS repo: the check-runner's bed runner and its summary writer own the verdict. opencharly/charly#904 is the consumer-facing issue (the charly binary's charly check run); this is the owning repo, and charly's half of the fix is a pin bump to whatever lands here.

Measured (opencharly/charly#904, on the charly side):

$ charly check run check-registry-command          # rc=0
  SKIP  check probe the runner step environment and load path (SPIKE)  [rc-probe-env]
        unresolved variables: CHARLY_BIN:-UNSET, GOFLAGS:-UNSET, GOMODCACHE:-UNSET, GOPATH:-UNSET, HOME:-UNSET
charly check run check-registry-command: PASS (steps=6)

and the acceptance record agrees — .check/check-registry-command/<calver>/summary.yml: all six steps ok: true, errors: 0, warnings: 0, with one declared check resolved to SKIP and never executed.

A PASS (steps=6) headline currently means one of at least three different things, and two of them prove nothing:

  1. every declared check ran and passed (the only meaning anyone reads it as);
  2. no plan steps were declared where the runner executes them — six lifecycle PASSes whose log was No plan steps defined for this image.;
  3. a declared check was skipped (unresolved check variable, absent service/credential) — folded into the same green headline.

The skip itself is NOT the defect: a skip is a deliberate contract (R8 — an absent service or credential skips VISIBLY, and is never mocked). The defect is that the skip is invisible in the verdict, so the deliberate-skip contract becomes a false-green acceptance record. Every summary.yml in the org IS the acceptance record — R10 evidence, PR bodies and the attribution tier all rest on it.

RCA

  1. Expected. A bed run's verdict distinguishes "every declared check ran and passed" from "a declared check was skipped" — in the headline, in summary.yml, and in the exit code.
  2. Actual. PASS (steps=6), exit 0, and a summary.yml whose six steps are all ok: true — with one declared check never executed.
  3. Mechanism. The skip is captured ONE LAYER BELOW the verdict and never reaches it:
    • the engine returns a per-check SKIP and the child exits 0 (a skip is deliberately not a failure — R8);
    • bed_run.go:898 banks the step with OK: true and Skipped: skippedFromRollup(logText) — the count is already there;
    • bed_run.go:900 clears res.OK only on a FAIL, so that count never reaches the run's verdict;
    • run_cmd.go:138 bedVerdictLine renders the verdict from res.OK alone → PASS (steps=N);
    • bed_run.go:1611 writeBedSummary writes ok: per step and ok: at the run level, and the run level has no place to say "a declared check did not run"; the per-check status is nowhere in the record.
  4. Missed control. The run-level verdict vocabulary is BINARY — summaryStatus(ok bool) at bed_run.go:135 returns PASS or FAIL and nothing else — so a skip has nowhere to go at the run level but silently into the step count. The per-step skip_reason (added by feat(check): the bed record says when a step SKIPPED — summary.yml carried only ok: (a skipped phase read exactly like a passing one) #99) covers only the case where the CHILD exited 3 (a phase that resolved NOTHING, check live: a bed's child phase runs the add_candy's image-shaped plan in the DEPLOY venue — and returns SUCCESS when it resolves no plan at all #110); a child that exits 0 with skipped checks inside it — the common case, and the one measured above — is not covered at all.
  5. Blast radius. Every bed in the org, because every summary.yml is an R10 acceptance record; every PR body quoting PASS (steps=N); and the umbrella's own scripts/check-bed-watch.sh, whose classify_exit can only tell a skip from a pass by the EXIT CODE — which a bed with a skipped declared check never sets (3) printf 'skip').
  6. Root cause. The run-level verdict is derived from a boolean, while the skip signal is a COUNT one layer below it. Nothing connects them, so the skip has no verdict to reach.
  7. Root fix. Give the run a three-state verdict computed from the per-check statuses, record those statuses in the record, render the headline so it cannot read as a bare PASS, and let the exit code say "skip" the way this repo's own vocabulary already says it.
  8. Classification. Blocking for the change that fixes it: without the fix, the record this change writes still cannot express the thing it is about.

The deliverable

  • summary.yml carries the per-check status. Each step that ran checks records them (checks: — the id, the status, and the reason), each step gets a status: PASS|SKIP|FAIL, and the run gets a run-level status: plus skipped_checks:. The acceptance predicate — every declared check id resolved to PASS, and no declared check id resolved to SKIP — becomes checkable from the record itself.
  • The headline cannot read as a bare PASS while a declared check skipped. It carries the skip count and the skipped check's reason.
  • The exit code says "skip". A run whose verdict is SKIP exits 3 — CheckSkippedExitCode (check_cmd.go:25), the code this repo already reserves for "not a verdict", and the one the umbrella's classify_exit already reads as "a SKIP, never a pass". A skip stays a skip: not a failure (R8) and not a pass (R7 — a check that cannot run is not a check that passed). A FAIL still wins over a SKIP.
  • The bed still completes. A skipped declared check does not abort the run: the bed tears down as it would after a pass, and the run's verdict is what changes.

Scope boundary (stated, not deferred)

charly check live — the child the bed drives — keeps its own exit-code contract: its report already prints the per-check SKIP line and the rollup count (2 steps: 1 passed, 0 failed, 1 skipped), so a reader of that output sees the skip. This change is about the BED's record and verdict, which is where the skip was folded into steps=N. The related instances of the same class at the other layers are filed where they belong: the plan runner is opencharly/charly#914, and the gate-script case is opencharly/charly#588 (the umbrella's own occurrence).

Agent: charly

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)

Activity

  1. aitrawog commented on Oct 25, 2026

    @aitrawog
    ContributorAuthor

    CLAIM — I am taking this; the fix is the bed runner's verdict, in this repo, and charly's half is a pin bump to whatever lands here.

    Agent: charly

    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)

    Why here and not in charly. The bed runner, the summary writer and the verdict line are this repo's: candy/plugin-check/bed_run.go (stepResult, summaryStatus, writeBedSummary) and candy/plugin-check/run_cmd.go (bedVerdictLine). The charly repo reaches them as the compiled-in command:check plugin, so charly's half of this fix is a go.mod/go.sum bump — the consumer leg — and this issue is the producer.

    The scope, and where it stops. The run-level verdict becomes three-state (PASS/SKIP/FAIL), computed from the per-check statuses the child's report already states; summary.yml records those statuses, the per-step status:, and the run's status: + skipped_checks:; the headline carries the skip count and reason so it cannot read as a bare PASS (steps=N); and a SKIP run exits 3 — CheckSkippedExitCode, the code this repo already reserves for "not a verdict", and the one the umbrella's scripts/check-bed-watch.sh already classifies as "a SKIP, never a pass". A FAIL still wins over a SKIP, and a skipped declared check does not abort the bed: the run completes and tears down as it would after a pass.

    What it does NOT change. charly check live's own exit-code contract: its report already prints the per-check SKIP line and the rollup count, so that output is visible; this issue is about the BED's record, which is where the skip was folded into steps=N. The same class at the other two layers stays filed where it belongs — the plan runner is opencharly/charly#914, the gate-script case is opencharly/charly#588.

    The proof. A Go test that fails without the change (the verdict, the summary writer and the per-check parser), plus a live bed run whose declared check is skipped: before the change the run prints PASS (steps=N) and exits 0; after it, the run prints a SKIP verdict with the count and the reason and exits 3, and its summary.yml carries the skipped check's id and status. The bed fixture that produces a skipped declared check is authored in this repo's own charly.yml, in the idiom of the #78/#86/#110 witnesses.

    Coordination. opencharly/charly#904 is the consumer-facing issue for the same defect; it is claimed in the same breath, and it closes when charly's pin bump lands. No other open issue or PR in this repo touches the bed runner's verdict.

    Next. The work happens in a session worktree on feat/bed-skip-is-not-a-pass. Next update when the PR is open, and within 30 minutes — whichever comes first.

  2. self-assigned this
    on Oct 25, 2026
  3. aitrawog commented on Oct 25, 2026

    @aitrawog
    ContributorAuthor

    STATUS — the fix is implemented, proved live, and the PR is open: #133 (head 9e02853).

    Agent: charly

    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

    What is done. The bed runner, the summary writer and the verdict line now carry a
    three-state verdict computed from the per-check statuses: a run that proved NOTHING is a
    SKIP (headline SKIP (steps=6, skipped_checks=1) — check-live: declared check not-applicable-here did not run: skip: true, exit code 3 = CheckSkippedExitCode), a run
    that proved something is a PASS that is never bare (it names the count and the reason of
    every declared check that did not run), and a FAIL still wins over a SKIP. The record
    gains status: (per step and per run), checks: (id, status, text, reason) and
    skipped_checks:; ok: keeps its one meaning — NO FAILURE — so every existing reader is
    unaffected.

    The proof is discriminating, not ceremonial. Two new in-repo witness beds:
    check-skip-verdict-witness (its only declared check is authored skip: true) and
    check-skip-verdict-probe, which runs the witness through charly check run and asserts
    the exit code is 3 and the verdict names the skipped check. On the released plugin the
    witness printed PASS (steps=6) and exited 0, so the probe's step FAILED (exit 2); with the
    fix the witness prints SKIP (steps=6, skipped_checks=1) … and exits 3, and the probe
    passes. The three pre-existing in-repo beds still pass on the same fresh build.

    One finding, fixed here rather than filed. The first cut read "any skipping step ⇒ the
    run skips", which turned the #110 witness bed into a false SKIP (a vacuous member phase
    beside a passing check). Measured live, then root-fixed: the run's verdict reads the check
    outcomes, and a phase that declares no check id cannot unprove a check another phase proved.

    What is next. The org charly/pr-validator runs on the PR head; every finding is fixed
    in one commit per round, and this issue closes with the merge.

    Next update due. when the validator's verdict lands, or when a round's fixes are pushed.

  4. aitrawog commented on Oct 25, 2026

    @aitrawog
    ContributorAuthor

    RESOLVED — the fix is landed, tagged and validated: #133 (squash 3d1e723, tag v2026.298.0059, CHANGELOG 72247a9).

    Agent: charly

    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

    What landed. The bed runner's verdict is three-state and computed from the per-check
    statuses the child's own report states: a run that proved nothing is a SKIP (headline
    SKIP (steps=6, skipped_checks=1) — check-live: declared check not-applicable-here did not run: skip: true, exit code 3 = CheckSkippedExitCode, the code the umbrella's own
    scripts/check-bed-watch.sh already classifies as "a SKIP, never a pass"); a run that proved
    something is a PASS that is never bare (it names the count and the reason of every declared
    check that did not run); a FAIL still wins over a SKIP. The record gains status: (per
    step and per run), checks: (id, status, text, reason) and skipped_checks: — and ok:
    keeps its one meaning, NO FAILURE, so every existing reader of it is unaffected.

    The proof was discriminating. check-skip-verdict-probe runs the witness through
    charly check run and asserts the exit code is 3 and the verdict names the skipped check. On
    the released plugin the witness printed PASS (steps=6) and exited 0, so the probe's step
    FAILED (exit 2); with the fix the witness prints the SKIP verdict and exits 3, and the
    probe passes. The three pre-existing in-repo beds still pass on the same fresh build, and
    every run reports errors: 0, warnings: 0.

    The validator's PASS armed native auto-merge (squash); all three checks are SUCCESS. The
    two non-blocking observations it raised are answered on the PR's own thread: the first is
    fixed in #134, the second does not occur (the prereq branch
    returns before the Unproven block is reachable, with the code quoted on the thread).

    The consumer leg is next. opencharly/charly#904 closes with the charly go.mod/go.sum
    bump that carries this fix into the charly binary — and with it the reader hardening in
    #134, so the pinned version is the sound one.

    Next update due. when the charly leg's PR is open, or when a round's fixes are pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions