Skip to content

refactor(marketplace): the corpus reads authored bodies through candywalk's ONE authority (sdk#456) - #43

Merged
github-actions[bot] merged 1 commit into
mainfrom
fix/agent-node-body-authority
Oct 23, 2026
Merged

github-actions[bot] merged 1 commit into
mainfrom
fix/agent-node-body-authority

Conversation

@aitrawog

Copy link
Copy Markdown
Contributor

Summary

The corpus generator now reads authored bodies through candywalk's ONE authority instead of
carrying its own copy of "where does a skill: node's body live" — the second caller of the
resolution opencharly/sdk#463 put in the shared kit.

opencharly/sdk#456 was a drift between three encodings of one rule: deploykit's collector
resolved .agents/skills for every node (wrong), while this generator held the correct rule
twice — a type: agent branch in skillfile.go's read path and the same branch in
extract.go's write path — and used the classifier again in four emission decisions. sdk#463 made
candywalk the authority (ShapeOf / ReadNodeBody / NodeBodyLocation); this PR makes the
corpus consume it, which is what "one authority, both callers" means. Nothing is left in this repo
that decides where a body lives.

  • fillSkillFromFile reads through candywalk.ReadNodeBody and reconciles the node against
    the file. fillAgentFromFile is DELETED — the agent shape needs no branch: AuthoredBody
    carries it, its References are empty by construction, and the one reconciliation serves both
    classes.
  • extract asks candywalk.ShapeOf for its destination, and the four emission decisions
    (kimi manifest, R0 dispatcher ×2, opencode index, the skill/agent synthesis) ask the same
    function instead of comparing the raw type: string.
  • The "no body" message names the shape's own file (candywalk.NodeBodyLocation), so an
    agent node can no longer be told to look under .agents/skills — the misdirection that made
    sdk#456's red gate unfixable for the author reading it.
  • Pin: github.com/opencharly/sdk v0.2026289.157 → v0.2026296.1141, the tag sdk#463's
    merge minted. go mod tidy also advances the grpc/protobuf/genproto transitives that pin
    requires.

The change is a consolidation, and the invariant is that the corpus does not move: proven at the
emitted-artifact boundary below, byte for byte.

How tested

The head this body describes:

$ git rev-parse HEAD HEAD^{tree}
4b8981828725b030c067df5c808b2998a97b6e5b
36d4c586f4eee29073dae2834e2162090916e666

$ git diff --stat origin/main...HEAD
 candy/plugin-marketplace/emit_catalogs.go      |  6 +-
 candy/plugin-marketplace/emit_dispatcher.go    |  6 +-
 candy/plugin-marketplace/emit_skills.go        |  3 +-
 candy/plugin-marketplace/extract.go            |  4 +-
 candy/plugin-marketplace/extract_agent_test.go | 24 ++++---
 candy/plugin-marketplace/go.mod                | 22 +++----
 candy/plugin-marketplace/go.sum                | 82 ++++++++++++++----------
 candy/plugin-marketplace/skillfile.go          | 88 ++++++++------------------
 candy/plugin-marketplace/skillfile_test.go     | 85 +++++++++++++++++++++++++
 9 files changed, 201 insertions(+), 119 deletions(-)

The repo's declared checks (its AGENTS.md "Build / validate / test": go build, go test,
and charly box validate at the root), run on the committed tree, against the real pin:

$ test -z "$(gofmt -l .)"
rc=0

$ go vet ./...
rc=0

$ go build ./...
rc=0

$ go test ./...
rc=0

$ grep -c '^ok' /tmp/pm-test-final.log
1
$ grep -v '^ok\|no test files' /tmp/pm-test-final.log

$ GOTOOLCHAIN=go1.26.8 golangci-lint run ./...
0 issues.
rc=0

$ charly box validate   # at the repo root
charly box validate: OK — checked 1 candy, 1 box, 1 deploy, 7 distros, 7 builders; 0 warnings, 0 errors

GOTOOLCHAIN=go1.26.8 because this host's Go is 1.27.3 and the pinned linter build cannot read
1.27 export data; the module declares go 1.26.8.

The artifact-boundary proof — the same project emitted by the OLD and the NEW generator, diffed.
The repo has no per-repo CI workflow (only tag-on-merge.yml), and its R10 witness bed
(check-marketplace) gates the PUBLISHED corpus's shape — an artifact this diff cannot change, as
the following proves directly. A throwaway harness (never committed) writes the standard
writeFixture project plus a file-sourced skill node, a file-sourced agent-type node and a hook
entity, runs generate, and copies the emitted tree out:

$ AB_DUMP=/tmp/ab2-new go test -run TestZZAuthoringABDump -count=1   # THIS head
PASS
$ ls /tmp/ab2-new/core/agents /tmp/ab2-new/core/skills
/tmp/ab2-new/core/agents:
pr-validator.md
/tmp/ab2-new/core/skills:
charly-status
filed-skill

$ git checkout origin/main -- skillfile.go extract.go emit_catalogs.go emit_dispatcher.go emit_skills.go
$ AB_DUMP=/tmp/ab2-old go test -run TestZZAuthoringABDump -count=1   # PRE-CHANGE generator
PASS
$ git checkout HEAD -- skillfile.go extract.go emit_catalogs.go emit_dispatcher.go emit_skills.go

$ diff -rq /tmp/ab2-old /tmp/ab2-new ; echo "DIFF_RC=$?"
DIFF_RC=0
$ echo "old: $(find /tmp/ab2-old -type f | wc -l)  new: $(find /tmp/ab2-new -type f | wc -l)"
old: 16  new: 16

Sixteen emitted files on each side — the agent artifact, both skills, both per-plugin manifests in
each family, marketplace.json (both shapes), DISPATCHER.md, .well-known/skills/index.json,
kimi.plugin.json, package.json, profiles.json — and zero differing bytes. The corpus
does not move.

The control that FAILS without the change. The one behavioural delta this consolidation carries
is the message a missing body produces; under the pre-change generator the skill branch named its
trees as a Go slice and the agent branch had a separate message, neither derived from the shape
mapping:

$ git checkout origin/main -- skillfile.go   # PRE-CHANGE generator, new tests unchanged
$ go test ./... -run 'TestFillSkillFromFile_NoBodyMessageNamesTheShapesOwnFile' -count=1
--- FAIL: TestFillSkillFromFile_NoBodyMessageNamesTheShapesOwnFile (0.00s)
    --- FAIL: TestFillSkillFromFile_NoBodyMessageNamesTheShapesOwnFile/skill (0.00s)
        skillfile_test.go:125: error "skill \"ghost\": no body — neither an inline `content:` nor a SKILL.md under [.agents/skills]" does not name ".agents/skills/ghost/SKILL.md" — the message must be the shape's own file
FAIL
TEST_RC=1

restored, tree clean at the head:

$ git checkout HEAD -- skillfile.go
$ git status --short

The claim that this repo no longer decides the rule — pasted, not asserted:

$ grep -rn 'Type == "agent"' --include=*.go . || echo "(no matches)"
./skillfile.go:33:// used to carry its own `if s.Type == "agent"` branch, i.e. a second copy of the rule.

$ grep -n 'opencharly/sdk' go.mod
8:	github.com/opencharly/sdk v0.2026296.1141

The only remaining occurrence of the string is the comment recording that the copy is gone. Every
decision now asks the kit:

$ grep -rn 'candywalk\.\(ShapeOf\|ReadNodeBody\|NodeBodyLocation\)' --include=*.go . | grep -v _test
./emit_catalogs.go:63:			if candywalk.ShapeOf(s.Type) == candywalk.ShapeAgentFile {
./emit_dispatcher.go:36:			if candywalk.ShapeOf(s.Type) == candywalk.ShapeAgentFile || len(s.Triggers) == 0 {
./emit_dispatcher.go:105:			if candywalk.ShapeOf(s.Type) == candywalk.ShapeAgentFile {
./emit_skills.go:20:			if candywalk.ShapeOf(s.Type) == candywalk.ShapeAgentFile {
./extract.go:109:		if candywalk.ShapeOf(s.Type) == candywalk.ShapeAgentFile {
./skillfile.go:107:	body, err := candywalk.ReadNodeBody(base, name, s.Type, trees)
./skillfile.go:113:			return fmt.Errorf("skill %q: no body — neither an inline `content:` nor %s", name, candywalk.NodeBodyLocation(s.Type, name, trees))

Test arms on this head: 54 passing entries across the module's suite.

$ go test ./... -count=1 -v 2>&1 | grep -cE '^(--- PASS|    --- PASS)'
54

Rulebook compliance

  • R0 (skills first) — /charly-internals:marketplace (this repo's AGENTS.md names it first
    for an emitter change), :plugin, :skills, :git-workflow, :go, plus this repo's own
    AGENTS.md (Part II fix: advance MARKETPLACE_REF to the marketplace's current main head #4).
  • R1 (RCA, fix or file) — the mechanism is stated in the summary: one rule, three encodings,
    nothing comparing them. No failure surfaced in this leg; both controls above are green, and the
    sdk leg's two findings (the label wire shape; the carrier whose gate fails for an unrelated
    owner reason) are disposed of on their own threads — the ai.opencharly.skill wire entry cannot express a sub-agent definition: type/model/tools are dropped, so an agent node rides the skill label as a plain skill spec#276 and
    Policy: a skill lives with the code it describes — retire the skills-only repo class (22 repos / 94 skills measured) opencharly#600.
  • R2 (no duplication) — this is the duplication's REMOVAL: fillAgentFromFile is deleted, the
    four classifier copies become calls, and the path rule now exists only in candywalk.
  • R3 (no workaround) — no fallback, retry or second path; a body that is absent is an error.
  • R4 (product first) — no prose is routed around the code; the schema comment, the skills
    skill and AGENTS.md Part II fix(marketplace.json): put description and version at top level per the spec #5 all state the two shapes, and the code now agrees with them.
  • R5 (legacy deleted) — the per-shape branch and the raw type: comparisons are gone, not
    deprecated; the grep above is the claim-keyed sweep.
  • R6 (git safety) — one commit on a branch off fresh origin/main; no force-push, no amend
    after push, no push to main.
  • R7 (prove behavior) — the emitted-artifact A/B (16 files, diff -rq rc=0) is the boundary
    proof for a generator change; the message control fails on the pre-change generator and passes
    here.
  • R8 (live or skip) — N/A — a generator reads and writes files; it crosses no live service
    boundary.
  • R9 (binary equals source) — N/A for this repo — it publishes a plugin module; the pin and
    go.sum are the source-of-record, and the consumer that builds a binary is charly.
  • R10 (fresh gate) — the change class is a Go generator change: the repo's own checks
    (go build/go test/gofmt/vet/lint/charly box validate) ran on the committed tree, and
    the artifact the change could affect is proven byte-identical. This repo declares no per-repo CI
    workflow (tag-on-merge.yml only); its check-marketplace bed gates the published corpus's
    shape, which the A/B shows this diff cannot move.
  • plugin-marketplace AGENTS.md — candy entity + Go source together — the candy entity is
    untouched: its description: still describes the same generator, and plugin.providers is
    unchanged. Only the Go source and the module pin move.
  • plugin-marketplace AGENTS.md — generate and drift stay in step — both run the same
    emission pipeline, which is why the A/B was run through generate itself and not a helper.
  • plugin-marketplace AGENTS.md — landing — PR-only, one commit; the tag is minted at merge.
  • HC (one cutover per PR) — this PR is the corpus-consumer leg of sdk#456's cutover and is
    complete on its own: the pin bump and every in-repo caller of the rule. The charly module-pin
    bump is a separate repo and a separate PR.
  • COORD — sdk#456 carries the CLAIM; sdk#463 (the producer) is merged and tagged
    v0.2026296.1141, which is what this pin names. STATUS on Policy: a skill lives with the code it describes — retire the skills-only repo class (22 repos / 94 skills measured) opencharly#600 follows
    the charly leg, when the two held carriers' drainability is measured rather than promised.

A3 — attribution evidence, as delivered in this session

The runtime's own system message reads, verbatim:

You are a coding agent powered by the deepseek-v4.1-flash model.

Harness DeepSeek Harness, model deepseek-v4.1-flash — the pair the footer names. This runtime is
the one opencharly/opencharly#538 records as rejected by the validator's model allowlist even though
it is real and running; cited here so the A3 check can be read against that open defect rather than
as an unattributed model name.

Change classification

Code change (Go generator + module pin), not documentation. Nine files: six Go sources, two
Go tests, go.mod/go.sum. The change-class gate is this repo's declared checks, pasted above,
run on the committed tree; the attribution tier the evidence supports is
fully tested and validated — the module's full suite passes and the changed path (emission)
executed for real, with its output diffed byte-for-byte against the pre-change generator's.

Filed issues

None. Both findings this work surfaced are disposed of outside this diff and named under R1:
opencharly/spec#276 (the label wire entry cannot express a sub-agent definition) and the
layer-charly-internals-extra owner mismatch reported on opencharly/opencharly#600.

Agent: charly

Assisted-by: DeepSeek Harness deepseek-v4.1-flash (fully tested and validated)

…walk's ONE authority

The generator carried its own copy of "where does a `skill:` node's body live": a
`type: agent` branch in skillfile.go's read path, the same branch in extract.go's write path, and
the same classifier again in four emission decisions. candywalk now owns that rule
(opencharly/sdk#463 — ShapeOf / ReadNodeBody / NodeBodyLocation), and this repo consumes it, so
the generator and deploykit's validator resolve ONE rule instead of two copies
(opencharly/sdk#456).

- `fillSkillFromFile` reads through `candywalk.ReadNodeBody` and reconciles the node against the
  file. `fillAgentFromFile` is DELETED: the agent shape needs no branch, because AuthoredBody
  carries it and its References are empty by construction, so one reconciliation serves both.
- `extract`'s destination and the four emission decisions ask `candywalk.ShapeOf` instead of
  comparing the raw `type:` string. No `Type == "agent"` comparison remains in the module.
- The "no body" message names the shape's own file (`candywalk.NodeBodyLocation`), so an agent
  node can no longer be told to look under `.agents/skills` — the misdirection that made
  opencharly/sdk#456's red gate unfixable for the author reading it.
- Pin: `github.com/opencharly/sdk` v0.2026289.157 -> v0.2026296.1141, the tag carrying the
  authority.

Proven at the emitted-artifact boundary: the same fixture project (a file-sourced skill, a
file-sourced agent definition, a hook entity) emits a BYTE-IDENTICAL corpus under the pre-change
and post-change generator — `diff -rq` reports no difference, agent artifact included.

Assisted-by: DeepSeek Harness deepseek-v4.1-flash (fully tested and validated)
@github-actions

Copy link
Copy Markdown

Review — PASS

What I verified against the current state

Correctness / claim-vs-diff.

  • fillAgentFromFile is genuinely gone (skillfile.go, −61 lines), and fillSkillFromFile now routes through candywalk.ReadNodeBody with one reconciliation for both shapes. The References reasoning in the comment (agent shape has no references/ half ⇒ body.References empty ⇒ the two reference rules cannot fire for agents) is consistent with the deleted helper, which had no references handling.
  • All five former s.Type == "agent" decision sites are converted to candywalk.ShapeOf(s.Type) == candywalk.ShapeAgentFile: emit_catalogs.go:63 (kimi), emit_dispatcher.go:36 and :105 (R0 dispatcher, opencode index), emit_skills.go:20 (synthesis), extract.go:109 (destination). The comment left at skillfile.go:33 is the only remaining occurrence of the literal string, matching the body's grep output.
  • The "no body" message now derives from candywalk.NodeBodyLocation(s.Type, name, trees) instead of the raw trees slice / the separate agent message — this is the behavioural delta the body claims, and it is the one the new test pins.
  • File list, per-file ±counts and the git diff --stat in the body (9 files, 201/119) all reconcile with the supplied diffs, including the extract_agent_test.go 14/10 and skillfile.go 27/61 counts.

Tests. TestFillSkillFromFile_NoBodyMessageNamesTheShapesOwnFile is a genuine regression test: the body's control run shows the skill arm failing on the pre-change generator (old slice-form message under [.agents/skills]) while the agent arm passes on both (old fillAgentFromFile already named .agents/agents/<name>.md). TestFillSkillFromFile_BothShapesThroughOneFunnel is a behavioural pin (each shape reads only its own tree; a mis-placed body is not published) that would break on a re-split or a wrong-tree read; it is written to vary the body, not just frontmatter, which is the right discriminator. extract_agent_test.go correctly retargets the fail-loud arms from the deleted helper to the single funnel, preserving all five arms.

Security / scope. No secrets, no new input surface, no privilege change. Dependency movement is a pin of the org's own sdk/spec plus the transitives that pin forces (grpc 1.61→1.83.2, protobuf →1.36.12, otel/genproto/x-*); all upgrades, no additions of untrusted third-party code. Scope is coherent: the refactor, its tests, and the pin — no dead code left behind, no fallback path.

Comments. There are zero comments, so there is nothing to disposition and no maintainer sign-off to weigh; the change stands on its own evidence.

Non-blocking observations (do not gate)

  1. Body miscount. "Nine files: six Go sources, two Go tests, go.mod/go.sum" — the diff has five Go sources (emit_catalogs.go, emit_dispatcher.go, emit_skills.go, extract.go, skillfile.go), and 6+2+2 = 10 ≠ 9. The enumerated file list and the diffstat are correct; only the category breakdown is wrong.
  2. Unmentioned direct bump. github.com/opencharly/spec v0.2026288.1242 → v0.2026296.1036 is a first-class dependency change; the body's "Pin" bullet attributes the churn only to grpc/protobuf/genproto transitives. It is most likely MVS-forced by the sdk pin, but it should be named.
  3. A/B does not isolate the dependency bump. The control reverts only the five source files and keeps the new pin on both sides, so it proves the source refactor is byte-preserving under the new deps — not that the new sdk/spec pin leaves the published corpus unchanged relative to origin/main. Given the pin is the org's own tag and the source-level equivalence is the claim that matters here, this is acceptable, but the "the corpus does not move" phrasing is slightly stronger than the evidence.
  4. Message prefix. An agent node with no body now reports skill "<name>": no body — … (path is correct: .agents/agents/<name>.md). Cosmetic, and the path — the thing #456 got wrong — is right.

None of these affect the correctness, safety, or test coverage of the change.

Verdict: PASS


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

View action run

@github-actions
github-actions Bot enabled auto-merge (squash) October 23, 2026 11:51
@github-actions
github-actions Bot merged commit 972193e into main Oct 23, 2026
3 checks passed
charly-auto-merge Bot added a commit that referenced this pull request Oct 23, 2026
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.

2 participants