Skip to content

test(skills): the skill-body control reads the tree, so no body is left unchecked - #68

Merged
github-actions[bot] merged 1 commit into
mainfrom
feat/skill-body-coverage-67
Oct 28, 2026
Merged

github-actions[bot] merged 1 commit into
mainfrom
feat/skill-body-coverage-67

Conversation

@aitrawog

Copy link
Copy Markdown
Contributor

Closes #67

Summary

Head 350f885c923742f89e9f40afdd703879ca92b160 — the commit this PR pushes: 2 files changed, 167 insertions(+), 3 deletions(-).

The defect. The control that keeps this repo's skill bodies true — candy/plugin-box/skill_doc_flags_test.go — compared the bodies it reads against a hand-kept list of ten names (skillDocCases). That list was the control's only source of the SET of bodies: it read exactly the ten bodies it named, and asserted only what those bodies claim. A body added under .agents/skills/ was therefore read by NOTHING, and the control stayed green over it.

That is not a hypothetical gap. Every .agents/skills/<verb>/SKILL.md in this repo is published VERBATIM into the opencharly/marketplace corpus, and every harness loads that corpus. The class of defect this control exists to catch is a body that documents a flag the grammar does not define — the class that produced --platform (#40) and the inner-concurrency flag (#65), each of which published a command that fails AT THE FLAG. A body the control never reads is a body that can carry that class into the corpus with nothing to see it.

The RCA.

  • Mechanism — skillDocCases was a hand-kept list of the ten body names, and the control iterated it. The corpus generator, in contrast, reads the TREE. Two sets: the published one (the tree) and the checked one (the list). The control never compared them, so the difference was invisible from the inside: the list is only ever wrong in the one direction no assertion could see.
  • Missed control — nothing bound the control's read-set to the tree. Every other assertion in the control is about the CONTENT of a body it reads; there was no assertion about WHICH bodies it reads.
  • Blast radius — the whole published corpus of this repo's verb skills. A body that documents a flag the grammar does not define could reach the corpus, and the control that exists to catch exactly that would not see it.
  • Root fix — the TREE names the bodies. The control derives the set from .agents/skills/ (skillBodyNames) and asserts BOTH directions: every body in the tree has a case, and every case names a body that exists. It also asserts that every body it reads makes at least one usage claim, so a case cannot pass by comparing nothing.

A second defect, found while proving the new arm. The bed's step wrote its evidence with >> and never truncated the log, so an assertion could be satisfied by a PREVIOUS run's output. MEASURED: the new arm, mutated so the body sits at the repo root — which makes the arm's expectation false — still PASSED, against the log the green run had left behind. Both logs are now truncated at the start of the step, so every assertion in the step reads THIS run.

What lands. Two files:

file what it carries
candy/plugin-box/skill_doc_flags_test.go the coverage control: the tree names the bodies, both directions are asserted, and no body may be read vacuously
charly.yml the bed's skill-owner step: a new arm that pins the skill-body lookup's base, plus the truncation of the step's two logs

The lookup's base, pinned. The bed's skill-owner step proves that a skill's prose is readable when the owner is a package candy; its control puts the body at that candy's REPO ROOT. MEASURED, and now held by an arm: the body lookup reads the owning candy's repo root, and the SAME body BESIDE the candy is REFUSED — "no .agents/skills/psk1/SKILL.md exists at the repo root of the owning candy". The arm holds that base, so the fixture's placement cannot drift back to a claim the lookup does not make.

How tested

Everything below ran on the committed tree this PR pushes.

$ git rev-parse HEAD
350f885c923742f89e9f40afdd703879ca92b160
$ git diff --stat origin/main...HEAD
 candy/plugin-box/skill_doc_flags_test.go | 126 ++++++++++++++++++++++++++++++-
 charly.yml                               |  44 ++++++++++-
 2 files changed, 167 insertions(+), 3 deletions(-)

1. The repo's own Go gate — the gate this repo's AGENTS.md names.

$ cd candy/plugin-box && gofmt -l . && echo GOFMT_CLEAN=1
GOFMT_CLEAN=1
$ cd candy/plugin-box && go build ./... && echo BUILD_OK=1
BUILD_OK=1
$ cd candy/plugin-box && go vet ./... && echo VET_OK=1
VET_OK=1
$ cd candy/plugin-box && go test -count=1 ./...
ok  	github.com/opencharly/plugin-box/candy/plugin-box	0.023s
?   	github.com/opencharly/plugin-box/candy/plugin-box/cmd/serve	[no test files]

2. R7 (a) — the coverage control's RED ARM. A body no case covers must turn the suite RED, naming the body. The arm adds one body under .agents/skills/ and runs the whole suite:

$ mkdir -p .agents/skills/zzprobe
$ printf -- '---\nname: zzprobe\ndescription: a body no case covers\n---\n\nProse.\n' > .agents/skills/zzprobe/SKILL.md
$ cd candy/plugin-box && go test -count=1 ./...
--- FAIL: TestSkillBodiesDocumentOnlyRealFlags (0.00s)
    skill_doc_flags_test.go:308: the skill body zzprobe is not covered: add its command and grammar to skillDocCases, or the control reads nothing in it
FAIL	github.com/opencharly/plugin-box/candy/plugin-box	0.023s
RED_ARM_COVERAGE_EXIT=1
$ rm -rf .agents/skills/zzprobe

The tree is restored (git status --short prints nothing), and the suite is green again — the green run is the one in block 1.

3. R7 (b) — the bed arm's RED ARM. The arm pins the skill-body lookup's base, so it must FAIL when the base is wrong. The arm's shell is the step's own text, extracted from the committed charly.yml and mutated in one line: the body moves from the candy's own tree to the project's root, so the lookup finds it and the arm's expectation (a refusal) is false:

$ python3 -I extract-step.py bed-step-image-owner.sh
$ sed -i 's|mkdir -p "$D3/candy/pcandy/.agents/skills/psk1"|mkdir -p "$D3/candy/pcandy" "$D3/.agents/skills/psk1"|; s|> "$D3/candy/pcandy/.agents/skills/psk1/SKILL.md"|> "$D3/.agents/skills/psk1/SKILL.md"|' bed-step-image-owner.sh
$ bash bed-step-image-owner.sh
FAIL: a body BESIDE the candy validated clean, so the lookup base is not the repo root
--- arm: the same body BESIDE the candy, not at that candy's repo root ---
exit: 0
charly box validate: OK — checked 1 candy, 7 distros, 7 builders; 0 warnings, 0 errors
RED_ARM_EXIT=1

4. The GREEN step. The step as committed refuses the "beside" placement and accepts the repo-root placement:

$ bash bed-step-image-owner.sh
--- arm: the same body BESIDE the candy, not at that candy's repo root ---
exit: 1
charly: error: command "validate": validation error: skills: skill "psk1": no body — the node carries no in-line `content:` and no .agents/skills/psk1/SKILL.md exists at the repo root of the owning candy "pcandy"
--- control: owner re-homed to the package candy pcandy, body at that candy's repo root ---
exit: 0
charly box validate: OK — checked 1 candy, 7 distros, 7 builders; 0 warnings, 0 errors
BOX-SKILL-IMAGE-OWNER-OK
GREEN_STEP_EXIT=0

5. The pre-flight — the project's own manifest validates on the tree that carries the change.

$ charly -C plugin-box box validate
charly box validate: OK — checked 1 candy, 1 deploy, 7 distros, 7 builders; 0 warnings, 0 errors
BOX_VALIDATE_EXIT=0

6. R10 — the live bed. check-box-local is this repo's disposable witness (a kind:local deploy on host: local), and it ran green on the committed tree:

$ charly -C plugin-box check run check-box-local
charly check run check-box-local: [deploy-add] PASS after 1.133s (log: .check/check-box-local/2026.301.1938/deploy-add.log)
charly check run check-box-local: [bring-up-members] PASS after 0s
charly check run check-box-local: [check-live] PASS after 14.372s (log: .check/check-box-local/2026.301.1938/check-live.log)
charly check run check-box-local: [update] PASS after 1.13s (log: .check/check-box-local/2026.301.1938/update.log)
charly check run check-box-local: [cleanup] PASS after 1.089s (log: .check/check-box-local/2026.301.1938/cleanup.log)
charly check run check-box-local: [cleanup-members] PASS after 0s
charly check run check-box-local: PASS (steps=6)
BED_EXIT=0

The bed's own summary carries the verdicts, with zero diagnostics:

$ cat .check/check-box-local/2026.301.1938/summary.yml
bed: check-box-local
calver: 2026.301.1938
driver: /home/atrawog/Sync/Atrapub/a/claude/opencharly/.worktrees/umb-stagepar/charly/bin/charly
driver_version: 2026.301.1632
steps:
  - name: deploy-add
    status: PASS
  - name: bring-up-members
    status: PASS
  - name: check-live
    status: PASS
  - name: update
    status: PASS
  - name: cleanup
    status: PASS
  - name: cleanup-members
    status: PASS
diagnostics:
  errors: 0
  warnings: 0
  advisories: 0
ok: true
status: PASS

and the new arm's own evidence, from the bed run itself:

$ cat .check/grammar-arms/box-skill-body-base.log
--- arm: the same body BESIDE the candy, not at that candy's repo root ---
exit: 1
charly: error: command "validate": validation error: skills: skill "psk1": no body — the node carries no in-line `content:` and no .agents/skills/psk1/SKILL.md exists at the repo root of the owning candy "pcandy"

7. R5 — the sweep. The claim: the set of bodies the control reads is READ from the tree, so no hand-kept list of body names survives as its source of truth. The list itself survives as the per-body CASE table (which command, which grammar) — and it is now bound to the tree in both directions, which is what block 2 proves.

$ grep -rn 'skillDocCases' --include='*.go' .
candy/plugin-box/skill_doc_flags_test.go:82:// skillDocCases is the set of bodies this control reads. The grammar is the struct the command
candy/plugin-box/skill_doc_flags_test.go:88:var skillDocCases = []skillDocCase{
candy/plugin-box/skill_doc_flags_test.go:303:	uncovered, stale, err := skillDocCoverage(skillsRoot, skillDocCases)
candy/plugin-box/skill_doc_flags_test.go:308:		t.Errorf("the skill body %s is not covered: add its command and grammar to skillDocCases, or the control reads nothing in it", name)
candy/plugin-box/skill_doc_flags_test.go:311:		t.Errorf("skillDocCases names %s, which is no skill body in %s", name, skillsRoot)
candy/plugin-box/skill_doc_flags_test.go:314:	for _, tc := range skillDocCases {

Every hit is in the one control file, and the file's two new directions are the ones that keep the table honest: skillDocCoverage returns the bodies NO case covers and the cases that name NO body, and the control fails on either.

Rulebook compliance

  • R0 — skills first. /charly-image:image, /charly-build:build, /charly-build:validate, /charly-internals:plugin and /charly-internals:git-workflow are the skills this repo's AGENTS.md names for this area; they were read before the first edit, and the git-workflow mechanics (branch off fresh origin/main, commit, gate, write the whole body, push, PR-only landing) are the ones this PR follows.
  • R1 — every anomaly gets an RCA. Two anomalies, both RCA'd above with mechanism, missed control, blast radius and root fix: the hand-kept body set (the finding this issue carries), and the appended log that let a broken arm pass. The second was found by the red arm, not by reading — and it is fixed in the same change, because it is the same class (a control that can pass over a broken fixture).
  • R2 — no duplication. One implementation of the body set: skillBodyNames reads it from the tree, skillDocCoverage compares it with the cases, and the control calls those two. There is no second list of body names anywhere in the repo (block 7).
  • R3 — no workaround. No sleep, no retry, no suppression. The control fails with a message that names the body and says what to do; the bed arm fails with a message that names the wrong base.
  • R4 — product before prose. The change is in the control and the bed, not in prose. The one prose this PR adds (the two comments explaining the tree-derived set and the truncation) describes code that already exists in the same commit.
  • R5 — legacy deleted completely. Block 7 is the claim-keyed sweep: the list survives only as the per-body case table, and the tree is what names the bodies. The truncated-log defect's own precedent (appending to a log and asserting on it) is deleted from BOTH logs of the step, not one.
  • R6 — preserve user work and Git safety. git status was run first in this worktree; the branch is feat/skill-body-coverage-67 off a fresh origin/main; no force-push, no history rewrite, no bypass, no push to main. Nothing outside this repo's two files was touched.
  • R7 — prove behavior, not compilation. Two red arms, both live and both pasted above: the coverage control goes RED on a body no case covers (block 2), and the bed arm goes RED when the lookup's base is wrong (block 3). Neither arm can pass vacuously: the second one failed against a green run's residue until the logs were truncated, which is the proof that it can fail.
  • R8 — live or skip, never fake a live service. The bed ran live against a disposable kind:local deploy and was removed again (its cleanup and cleanup-members steps are in the summary above). Nothing is mocked.
  • R9 — binary equals source. The binary under test is this session's worktree-local CalVer-stamped build (2026.301.1632), named by the bed's own first step and by the summary's driver line. No Go production code changed — the only .go file in the diff is a TEST file — so the binary is unaffected by this commit, and no new runtime OS dependency is introduced, so packaging/ is untouched.
  • R10 — fresh disposable proof. The change class is a test control plus a bed step in a project's manifest, so the gate is this repo's Go gate, the charly box validate pre-flight, and the disposable bed that composes the changed entity (check-box-local) — all pasted above, all on the committed tree, all with zero warnings.
  • Cutover. One change, one PR: the control's read-set moves from a hand-kept list to the tree, and the log defect is fixed in the same commit, because the arm that exposes it is the arm that lands here. Nothing is deferred and no scope is hidden inside it.

Change classification

  • Class: a test control and a check-bed step in a project's manifest — the union of the Go class's gate (go build / go vet / go test) and the project-configuration class's gate (the charly box validate pre-flight plus the disposable bed that composes the changed entity). NOT documentation: the control's behaviour and the bed's assertions are what change, and both red arms above are what prove it.
  • Attribution tier: fully tested and validated — the change-class gate passed on the final tree (the repo's Go gate, the pre-flight, and the disposable R10 bed), the changed paths were executed live, and both red arms demonstrate the coverage can fail.
  • Scope. This is the follow-up to refactor(cli)!: the build concurrency flag is --stage-parallelism, and a control holds every skill body to the grammar #66 (the stage_parallelism cutover's producer leg), not a second cutover of that surface: it changes no verb, no flag, and no published prose. It closes the gap that leg's validator recorded.

Filed issues

Agent: charly · session ses_8efb271d-f2c0-47d3-85e3-2a1e00b11ae0

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

…ft unchecked

The control that keeps this repo's skill bodies true compared the bodies it
reads against a hand-kept list of ten names. A body added under .agents/skills/
was read by NOTHING and the control stayed green — the drift is invisible,
because the hand-kept list is only ever wrong in the one direction no assertion
can see (#67).

The TREE now names the bodies: the control derives the set from .agents/skills/
and asserts BOTH directions — every body in the tree has a case, every case
names a body that exists — and that every body it reads makes at least one usage
claim, so a case cannot pass by comparing nothing.

The bed's skill-owner arm now pins the lookup's base, MEASURED: the body lookup
reads the owning candy's REPO ROOT, and the same body BESIDE the candy is
refused. Both that arm's log and the step's own log are TRUNCATED, so their
assertions read this run and never a previous run's residue — a control that can
pass over a broken arm is not coverage.

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

Copy link
Copy Markdown

Review — PASS

Correctness — the diff does what the body claims

  • Tree-derived read-set. skillBodyNames (skill_doc_flags_test.go) reads ../../.agents/skills (the same path the old code built by hand) and returns every directory holding a SKILL.md. skillDocCoverage compares that set against skillDocCases and returns both uncovered (body with no case) and stale (case with no body). TestSkillBodiesDocumentOnlyRealFlags now fails on either list and additionally fails on len(claims) == 0 so a case cannot pass vacuously. All three assertions the body advertises are present and wired to the real tree.
  • The control's own red arm. TestSkillDocCoverageIsEffective exercises skillDocCoverage directly: covered tree → no findings; dropped case → extra reported uncovered; case naming an absent body → absent reported stale. The three branches are correct against the function's implementation.
  • Bed arm + truncation. charly.yml adds EV2 = .check/grammar-arms/box-skill-body-base.log, truncates both EV (: > "$EV") and EV2 at step start, and pins the lookup base: body inside the candy tree must be refused with exit: 1 and the message exists at the repo root of the owning candy; the mutation in the body (block 3) moves the body to $D3/.agents/... and correctly flips the arm to FAIL. The separate log and the truncation are each justified in the comment and are consistent with the measured evidence.

Honesty of the body — verified against the diff

  • Counts. File headers are +124/−2 and +43/−1 → 167 insertions, 3 deletions, matching the body's git diff --stat (126/44 total-changed lines). Head SHA 350f885c… matches the listed commit.
  • Line numbers. The body cites skill_doc_flags_test.go:308 for the uncovered-body error and :303/:311/:314 in block 7. Counting the hunk @@ -242,17 +300,37 @@ (including the two // context lines) lands the t.Errorf("the skill body %s is not covered…") at exactly 308, skillDocCoverage(...) at 303, the stale error at 311, and for _, tc := range skillDocCases at 314. The evidence is internally consistent and cannot have been pasted from a different tree.
  • Block 4 ordering. The committed step emits cat "$EV2" (new arm) before cat "$EV" (earlier control: owner re-homed… arm), which is exactly the order the green-step excerpt shows — consistent, not reordered.

Tests

Coverage ships a self-contained unit test for the new comparison function plus the live bed red arm. The only observation (non-blocking): the len(claims) == 0 vacuous guard has no dedicated red-arm demonstration in "How tested", though it is a simple, correct assertion and the primary coverage red arm (block 2) is proven. Not a defect.

Security / scope / cleanliness

No secrets, no injection surface (all shell expansions are double-quoted; set +e/set -e bracket the exit-code capture), no new dependency (slices is stdlib). Scope is coherent: one test control + one bed step, no production Go changed, no verb/flag/prose touched. The trap is extended to "$D" "$D2" "$D3" so the new temp dir is cleaned alongside the others.

Comments

There are no comments on this PR, so nothing to disposition. No maintainer sign-off is present, but the change stands on its own reproduced evidence (line-accurate, count-accurate red/green arms).

Verdict

The change closes the reported gap (hand-kept body list → tree-derived set with both-directions and non-vacuous assertions), fixes the appended-log defect in the same step, and every numeric and line-level claim in the body reconciles with the supplied diff.

Verdict: PASS


ollama-cloud/deepseek-v4.1-flash — action-review.

View action run

@github-actions
github-actions Bot enabled auto-merge (squash) October 28, 2026 19:43
@github-actions
github-actions Bot merged commit 3746ccc into main Oct 28, 2026
3 checks passed
charly-auto-merge Bot added a commit that referenced this pull request Oct 28, 2026
github-actions Bot pushed a commit that referenced this pull request Oct 28, 2026
The control's second half — a body that IS read but whose prose never invokes
the command it documents, so the control compares nothing in it — was asserted
inline, with nothing in the suite exercising its FAILING branch. The suite's
other red arm covers the COVERAGE rule; it does not touch this one.

The rule moves into skillDocIsVacuous, so the control calls the function the
test exercises (one implementation), and TestSkillDocVacuityIsEffective gives
it two arms: a body that never invokes the command (the guard must fire) and a
body that does (the guard must stay silent).

This is the one non-blocking observation #68's validator
review raised, and it is fixed here rather than filed away
(#69).

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)
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.

the skill-body flag control's skill set is a hand list: a newly added skill body is not covered

1 participant