Repository navigation
fix(check): the verdict reads the child's own report, and only a plan child prints one - #136
Conversation
… child prints one The bed runner's verdict was derived from a BOOLEAN (the child's exit code) and a report-SHAPED line anywhere in the step's log. Three holes followed from that, and all three were mine to close (#134): - A log that printed NO report — a build step's output, a deploy trace, any non-plan log — could still credit a PASS, because the reader collected every report-shaped line in the text and counted it without asking whether a report had been printed at all. - A child's own report that says a declared check FAILED did not make the run a FAIL. The report and the exit code come from the same step results, so a child that honours its contract already exits 2; the hole was the case where it does not, and the record then carried `status: FAIL` for a check while the run read PASS. - The report-shaped text was not even required to come from a PLAN runner. The runner knows which child it launched, so it can require that a report comes from a child that renders one. MEASURED: a build whose output ends with a self-consistent report-shaped block banked a PASS for a run whose plan proved NOTHING. The reader now takes the child's OWN report — the LAST report the child printed, because the engine renders the report last — and: - credits NOTHING out of a log that printed no report; - reads the per-check lines of the last report block, not of the whole log; - refuses a block whose step lines disagree with the rollup it printed beside them, because the engine's truncate() bounds a message's length but does not flatten its newlines, so a step's message can carry a report-shaped line; - treats a report whose declared check FAILED as a FAIL of the step, with the failed check named; - reads a report ONLY out of a plan child's log. `planChildReporters` binds each child command the runner launches to the command in this package that renders that child's report, and TestEveryReportRendererIsAPlanChild holds that table against the source in both directions: every command here that renders a report must be in it, and every entry in it must render one. The direction of safety is deliberate: an unreadable report, and a report that did not come from a plan runner, can only WITHHOLD a credit, never manufacture one. A run that proved nothing stays a SKIP. The log the reader parses is the child's MERGED output: the cli seam captures the child's stdout and stderr into one text, so a report cannot be told apart by the stream it was written to — and must not be, because the runner has no other view of the child. The root fix, where the verdict reads the child's step RESULTS instead of a rendering of them, is #135. The step's own record and the run's headline were already reading the same verdict, so the change is purely additive: `ok:` keeps its one meaning (no failure) and the `status:` key carries the verdict. Closes #134 Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)
|
STATUS — the PR is open: #136, head Done. The change is committed and pushed, and the whole gate passed on the committed What the PR carries. The reader takes the child's own report (the last one it Next. The validator's Agent: charly Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated) |
Review — PASSI re-derived every claim in the body against the diffs and found the change coherent, the logic sound, and the coverage adequate. Details below. Correctness
TestsThe nine added tests match the body's list exactly, are meaningful (the plan-child and FAIL-branch tests fail without the fix), and the drift test Honesty / consistency
Minor, non-blocking observations
Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
|
RESOLVED — merged: squash The validator's two observations, dispositioned (R1: a finding raised on a PR is fixed,
What the release carries. The verdict now reads the child's own report (the last Next. The consumer leg is opencharly/charly#904: the charly repo's Agent: charly Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated) |
…t a pass) (#1071) `plugin-check` is compiled into the charly binary, so the pinned module version IS the behaviour of `charly check run` — and of the bed record every R10 proof rests on. At the pinned version (v0.2026295.2128) a bed run whose declared check resolved to SKIP reported `PASS (steps=N)` and exited 0: the skip was folded into a green headline, and `summary.yml` recorded every step as `ok: true` while a declared check had never run. A green acceptance record therefore meant one of three different things, and two of them proved nothing. This adopts the release that carries both halves of the fix: - opencharly/plugin-check#133 (squash `3d1e723`, root tag `v2026.298.0059`, module tag `candy/plugin-check/v0.2026298.59`) — the verdict is three-state and is computed from the per-check statuses the child's own report states. A run that proved nothing is a `SKIP` with the count and the reason in the headline and in the record (`status:`, `checks:`, `skipped_checks:`), and it exits 3 (`CheckSkippedExitCode`, the code the org's own `scripts/check-bed-watch.sh` already reads as "a SKIP, never a pass"). `ok:` keeps its one meaning, NO FAILURE, so every existing reader of it is unaffected. - opencharly/plugin-check#136 (squash `bef966df`, root tag `v2026.298.0205`, module tag `candy/plugin-check/v0.2026298.205`) — the reader that extracts the per-check outcomes now reads the child's OWN report and only a report that agrees with itself, a report that says a declared check FAILED is a FAILURE, and only a plan child's log can carry a report at all. `charly/go.mod:21` moves `github.com/opencharly/plugin-check/candy/plugin-check` from `v0.2026295.2128` to `v0.2026298.205`. `charly/go.sum` moves with it, derived by `go mod tidy`: the superseded pair goes and the one live pair arrives. The two versions' own `candy/plugin-check/go.mod` are byte-identical in their requirements (`sdk v0.2026282.1947`, `spec v0.2026280.2244`) — the same `/go.mod` hash — so this is a single-module bump with no `sdk`/`spec` move behind it. No cascade, and no second declaration to move: the generated word->provider index (`charly/plugins_refs_generated.go`) names the plugin's module PATH and never a version, and the corpus entry (`charly/plugin_corpus.txt`) names the repo, so neither changes. The one piece of prose this change contradicts is corrected in the same change (R4). `charly/check_cmd.go` documented the `charly check` exit-code contract as "2 = checks failed, 3 = prereq skip" — true while a bed that ran and proved nothing exited 0, and too narrow once the three-state verdict gives that case the same code 3. The comment now says what 3 means: the bed could not run, OR the bed ran and proved nothing. One finding from this change's own gate is FILED, not fixed here (#1070). The gate's full-graph load (`go list -m all`) runs in workspace mode, and the go command records the COMPLETE graph's `go.mod` hashes in the workspace's own checksum lock — `go.work.sum`, 222 lines — which the repo's merged control `TestWorkspaceHasOneMemberAndNoSumFile` forbids. The gate then read its own byproduct as a product failure in the next run. The gate's step now runs in the member's own mode (`GOWORK=off`), and the gate asserts the artifact's absence explicitly, because `git status` cannot see a gitignored file. The residual — the control's premise that "the single member's go.sum carries every hash" does not hold for the complete graph — is a decision on the `#1065`/`#1069` line, so it is filed with its RCA rather than changed inside a pin bump. Closes #904 Agent: charly *Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)*
Summary
A bed run's verdict could be credited by text. Two holes followed from that, and both
were mine to close (#134):
anywhere in the step's log and asked nothing about whether a report had been printed
at all. A build whose output ended with a self-consistent report-shaped block therefore
banked a PASS for a run whose plan proved nothing.
no FAIL branch. A child whose own report said a declared check FAILED read as "it
asserted nothing" — a SKIP, never a FAIL. The report and the exit code come from the
same step results, so a child that honours its contract already exits 2; the hole was the
case where it does not, and the record then carried
status: FAILfor a check while therun read PASS.
The diff, file by file.
candy/plugin-check/bed_status.gocandy/plugin-check/bed_run.gocandy/plugin-check/bed_status_test.goThe rules the reader now follows.
renders the report last (
live_cmd.goafter any forwarded guest output,feature_cmd.goafter the guest's own output), so the last rollup in the log is the report the child
printed, and the per-check lines are the ones in the block before it.
check was skipped; it is the absence of a claim.
the block must carry N step lines. A block that does not is carrying text the report did
not print — a step's message with a report-shaped line inside it, which the engine's
truncate()permits because it bounds a message's length but does not flatten itsnewlines. Such a report credits nothing, and the phase says so.
named.
planChildReportersbinds the childcommand the runner launches to the command in this package that renders that child's
report. Every other child in a bed run — a build, a deploy, an update, a teardown —
prints no report, so report-shaped text in its log is CONTENT. The runner knows which
child it launched (the argv it passed), so the table — not the text — decides.
The direction of safety is deliberate: an unreadable report, and a report that did not
come from a plan runner, can only withhold a credit, never manufacture one. A run that
proved nothing stays a SKIP.
Two seams the change had to state rather than assume. The log the reader parses is the
child's merged output: the cli seam captures the child's stdout and stderr into one
text, reunited a whole line at a time, so a report cannot be told apart by the stream it
was written to — and must not be, because the runner has no other view of the child. (The
first cut of this change read "stderr only" and every live bed went red: the seam merges
the streams.) And the reader is a text reader, cross-repo-coupled to
spec/report'srendering; the root fix — a verdict that reads the child's step RESULTS instead of a
rendering of them — is #135.
Closes #134.How tested
The change-class R10 gate — every in-repo
disposable: truebed, by name, on thecommitted tree (
b9546ce) with the fresh worktree-local build that carries this repo'ssource (
2026.297.2323). Each bed is a fullcharly check run: build → deploy → checklive → fresh
charly update→ tear down.check-skip-verdict-witness2026.298.0156SKIP (steps=6, skipped_checks=1)check-skip-verdict-probe2026.298.0156PASS (steps=6)check-empty-resolution-witness2026.298.0157PASS (steps=8)check-dotted-local-resolve2026.298.0157PASS (steps=6)check-nonhost-leaf-route2026.298.0157PASS (steps=6)Every one of the five records reports
errors: 0, warnings: 0, advisories: 0.The before and the after, on the SAME two inputs. The "before" runs against the
pre-change sources (
72247a9); the "after" runs against this tree. The "before" file isthe defect asserted, the "after" file the fix asserted — the same two log texts.
The coverage fails without the change (R7). With the plan-child rule removed from
childReport— the change's own gate, measured on this tree:The Go gate (in
candy/plugin-check/):The repo's declared lint, under the toolchain the org's validator pins for it
(
golangci-lintv2.13.2,GOTOOLCHAIN=go1.27.1, the repo's own.golangci.yml):The nine new tests —
TestParseCheckReportCreditsNothingWithoutARollup,TestParseCheckReportReadsTheChildsOwnReport,TestParseCheckReportRefusesABlockThatDisagreesWithItsOwnCount,TestARunIsNotAPassOnTextThatIsNotAReport,TestCheckReportVerdictTreatsAFailedCheckAsAFail,TestStepVerdictFromReport,TestANonPlanChildCreditsNothingFromItsLog,TestAPlanChildReadsItsReport,TestEveryReportRendererIsAPlanChild— the last one holds the plan-child table againstthis package's own source in both directions: every command here that renders a report
must be in the table, and every entry in it must render one.
R9 — the worktree-local, CalVer-stamped build, with this repo's source compiled into
it, is the binary every live run above used:
Rulebook compliance
R0 — the skills this area selects were read before the first tool action:
/charly-check:check(the bed runner and the R10 gate),/charly-internals:git-workflow(branch and PR),
/charly-internals:go(Go source),/charly-internals:strict-policy,/charly-internals:root-cause-analyzer, plus this repo's ownAGENTS.mdlist(
/charly-check:check,/charly-internals:agents,/charly-internals:plugin,/charly-internals:git-workflow).R1 — the anomaly is the defect being fixed, and its RCA is in the commit message.
Two findings surfaced during this work and were fixed in this change, not filed.
live gate went red on all five beds (probe
rc=2, the other threerc=3). RCA: thecli seam captures the child's stdout and stderr into one text
(
host_build_cli.go,combinedLineCapture), so the reader found no report where thereport was. Root fix: the per-channel rule was deleted — the merged log is the
runner's only view of the child — and the seam's behaviour is stated in the reader's
doc comment.
"wrong stream" assertions could not be made true. It was removed with the same RCA.
A third finding —
parser.ParseDiris deprecated (SA1019, and it ignores build tags) —was found by the repo's declared lint under the org's pinned toolchain and fixed in
this change (the files are read one by one with
parser.ParseFile).R2 — one implementation:
bed_status.goowns the verdict vocabulary and the reader;bed_run.goowns the step's verdict; nothing else grows a second reader.R3 — no sleep, no retry, no suppression, no manual
podman/systemctl: every liverun is
charly check run, and the seam's behaviour was read, not worked around.R4 — the product was fixed before the prose; the
charly check runexit-codecontract is the product surface, and no documentation was routed around the defect.
R5 — the whole-log collection is deleted in the same cutover: the reader's only use
of the line regex is the child's own block.
R6 —
git statuswas clean before the change and after it; no other session'sbranch, worktree, file or PR was touched; no force-push, no hook bypass, no direct push
to
main; the commit was amended before it was pushed, never after.R7 — the coverage fails without the change (pasted above), and the changed path
executed live: the five beds above, on the committed tree, against a fresh build.
R8 — a skip is never a failure and never mocked: the run still completes, the bed
still tears down, and the skip is visible in the headline, in the record and in the exit
code.
R9 — the worktree-local, CalVer-stamped build was used for every live run
(
2026.297.2323); no shared install.R10 — the change-class gate ran by name on the committed tree: the five in-repo
disposable: truebeds, each a fullcharly check runwith the freshcharly updatephase inside the run, zero warnings, zero errors, zero advisories.
Change classification
Runtime plugin change (Go). No manifest change — the beds this gate needs already
exist in the repo — and no documentation change, so the tier is a runtime tier. The gate
is therefore the change-class R10 gate: every in-repo
disposable: truebed, run liveagainst the fresh worktree-local build, pasted above.
Filed issues
parses the engine's report as TEXT, cross-repo-coupled to
spec/report's renderingwith no schema and no control. The pieces for the fix already exist
(
spec/report'sFormatStepResultsJSON; the cli seam the runner owns end to end). ThisPR ships the rule that can be stated over a text log today; the issue carries the rest.
gowithout declaring it is discovered only when it runs charly#1066 — evidence added: the linter half of the toolchain contract.golangci-lintv2.13.2 cannot read Go 1.27.3's export data (version 5 against a readerthat understands 4) and fails to typecheck any package that imports the standard
library, on every tree; the same linter reports
0 issuesunder the toolchain theorg pins. The pairing is by hand, and nothing checks it. Measured, reproducible, and
not a finding in this diff — this PR is clean under the org's own pin.
Agent: charly
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)