diff --git a/.agents/skills/pull/SKILL.md b/.agents/skills/pull/SKILL.md index 743c46f..c611755 100644 --- a/.agents/skills/pull/SKILL.md +++ b/.agents/skills/pull/SKILL.md @@ -28,7 +28,6 @@ recommendation pointing here. | Pull fully-qualified ref | `charly box pull ghcr.io/opencharly/jupyter:2026.108.56` | Pulls as-is, works from anywhere | | Pull remote project ref | `charly box pull @github.com/org/repo/image:v1` | Downloads repo, reads its `charly.yml`, pulls registry ref | | Override tag (short name) | `charly box pull jupyter --tag 2026.108.1` | Pull a specific CalVer tag | -| Override platform | `charly box pull jupyter --platform linux/arm64` | Pull a specific platform | ## Three Input Forms @@ -129,8 +128,6 @@ Use `charly box pull ` once to seed local storage; use `charly update - `--tag ` (default: `latest`) — Image tag when resolving a short name. Ignored for fully-qualified or remote-project refs (the tag is embedded in the ref). -- `--platform ` — Target platform (default: host). Passed to - `podman pull --platform`. ## Typical Workflows diff --git a/candy/plugin-box/box.go b/candy/plugin-box/box.go index f246157..8621d10 100644 --- a/candy/plugin-box/box.go +++ b/candy/plugin-box/box.go @@ -224,12 +224,20 @@ func dispatchPull(hc *hostClient, args []string) error { // --- box build --- -// buildGrammar is the `charly box build [boxes…] [flags]` CLI surface — byte-identical to the -// former static core build leaf (P8b relocated its Run body into dispatchBuild): same positional, -// same nine flags (including the three env-var-backed tunables), same help text, so -// `charly box build --help` renders unchanged and CHARLY_BUILD_CACHE/CHARLY_BUILD_JOBS/ -// CHARLY_PODMAN_JOBS keep working — Kong resolves them in-plugin at parse time and dispatchBuild -// threads the resolved values into the spec.BuildRequest it invokes build:box with. +// buildGrammar is the `charly box build [boxes…] [flags]` CLI surface — the former static core +// build leaf (P8b relocated its Run body into dispatchBuild): same positional, same nine flags +// (including the three env-var-backed tunables). Kong resolves the env bindings in-plugin at parse +// time and dispatchBuild threads the resolved values into the spec.BuildRequest it invokes +// build:box with. +// +// ONE flag's SPELLING changed, deliberately: the inner-concurrency flag is `--stage-parallelism` / +// `CHARLY_STAGE_PARALLELISM`, not `--podman-jobs` / `CHARLY_PODMAN_JOBS`. The CONTRACT renamed that +// knob `podman_jobs` → `stage_parallelism` (spec/schema/buildwire.cue, sdk/buildkit. +// ResolveStageParallelism) and the builder is BuildKit — the value drives the daemon's +// `[worker.oci] max-parallelism`, not `podman build --jobs N` — so the retired driver's name was the +// one false word left on the surface (the flag's own help read "Stages per podman build"). The +// published build skill, the generated docs recipe and the contract all name the contract's +// spelling, and a command that fails AT THE FLAG is the defect (opencharly/plugin-box#65). type buildGrammar struct { Boxes []string `arg:"" optional:"" help:"Boxes to build (default: all enabled; the sentinel 'all' is equivalent). Supports remote refs (github.com/org/repo/box[@version])"` Push bool `name:"push" help:"Push to registry after building"` @@ -238,7 +246,7 @@ type buildGrammar struct { Cache string `name:"cache" help:"Build cache type: registry, image, gha, none (default: auto)" env:"CHARLY_BUILD_CACHE"` NoCache bool `name:"no-cache" help:"Disable build cache entirely"` Jobs int `name:"jobs" help:"Max concurrent image builds per DAG level (0=auto: defaults.jobs, else 4)" env:"CHARLY_BUILD_JOBS"` - StageParallelism int `name:"podman-jobs" help:"Stages per podman build (0=auto: min(NCPU, defaults.stage_parallelism_cap))" env:"CHARLY_PODMAN_JOBS"` + StageParallelism int `name:"stage-parallelism" help:"Stages per build (0=auto: min(NCPU, defaults.stage_parallelism_cap))" env:"CHARLY_STAGE_PARALLELISM"` IncludeDisabled bool `name:"include-disabled" help:"Build boxes with enabled: false in charly.yml (does not modify the file). Use for one-off operational rebuilds without flipping authored config."` DevLocalPkg bool `name:"dev-local-pkg" help:"Build localpkg candies (the charly toolchain) from LOCAL in-development source instead of downloading the published release. Set automatically for disposable check-bed image builds so a bed tests in-development code; never on a production box build."` } diff --git a/candy/plugin-box/box_test.go b/candy/plugin-box/box_test.go index 9ebc944..3dd1625 100644 --- a/candy/plugin-box/box_test.go +++ b/candy/plugin-box/box_test.go @@ -150,7 +150,7 @@ func TestBuildGrammar_Parse(t *testing.T) { "jupyter", "arch-builder", "--push", "--tag", "2026.100.0000", "--platform", "linux/amd64", "--cache", "registry", "--no-cache", - "--jobs", "8", "--podman-jobs", "2", + "--jobs", "8", "--stage-parallelism", "2", "--include-disabled", "--dev-local-pkg", } if _, err := parseLeaf("build", &g, args); err != nil { @@ -233,12 +233,12 @@ func TestAcquireBuildActivityLock_WritesContentAndReleases(t *testing.T) { } // TestBuildGrammar_JobsEnvBindings verifies the Kong env bindings on the build parallelism + -// cache flags — CHARLY_BUILD_JOBS → Jobs, CHARLY_PODMAN_JOBS → StageParallelism, CHARLY_BUILD_CACHE → -// Cache. Moved from charly/build_jobs_test.go (P8b — the former core BuildCmd is DELETED; the env -// bindings now live on this grammar), so the bindings can't silently regress. +// cache flags — CHARLY_BUILD_JOBS → Jobs, CHARLY_STAGE_PARALLELISM → StageParallelism, +// CHARLY_BUILD_CACHE → Cache. Moved from charly/build_jobs_test.go (P8b — the former core BuildCmd +// is DELETED; the env bindings now live on this grammar), so the bindings can't silently regress. func TestBuildGrammar_JobsEnvBindings(t *testing.T) { t.Setenv("CHARLY_BUILD_JOBS", "6") - t.Setenv("CHARLY_PODMAN_JOBS", "9") + t.Setenv("CHARLY_STAGE_PARALLELISM", "9") t.Setenv("CHARLY_BUILD_CACHE", "registry") var cli struct { @@ -255,7 +255,7 @@ func TestBuildGrammar_JobsEnvBindings(t *testing.T) { t.Errorf("Jobs from CHARLY_BUILD_JOBS = %d, want 6", cli.Build.Jobs) } if cli.Build.StageParallelism != 9 { - t.Errorf("StageParallelism from CHARLY_PODMAN_JOBS = %d, want 9", cli.Build.StageParallelism) + t.Errorf("StageParallelism from CHARLY_STAGE_PARALLELISM = %d, want 9", cli.Build.StageParallelism) } if cli.Build.Cache != "registry" { t.Errorf("Cache from CHARLY_BUILD_CACHE = %q, want registry", cli.Build.Cache) diff --git a/candy/plugin-box/skill_doc_flags_test.go b/candy/plugin-box/skill_doc_flags_test.go new file mode 100644 index 0000000..a382f64 --- /dev/null +++ b/candy/plugin-box/skill_doc_flags_test.go @@ -0,0 +1,303 @@ +package box + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/alecthomas/kong" +) + +// skill_doc_flags_test.go — the control that keeps this repo's skill bodies TRUE. +// +// Every `.agents/skills//SKILL.md` in this repo is published VERBATIM into the +// opencharly/marketplace corpus, and every harness loads that corpus. A body that documents a flag +// this plugin's grammar does not define therefore publishes a command that fails AT THE FLAG. +// Two spellings have carried that class into the corpus: `--platform`, deleted from the pull +// grammar (opencharly/plugin-box#40) while the body still documented it, and the inner-concurrency +// flag, whose contract spelling (`--stage-parallelism` / `CHARLY_STAGE_PARALLELISM`) the grammar +// did not accept until this cutover (opencharly/plugin-box#65). +// +// The control reads each body from the SAME path the corpus generator reads it from, and asserts +// that the surface the body presents for its OWN command is one this plugin's Kong grammar +// actually defines. Kong is the authority, not a hand-written list: the flag names and the env +// bindings come from the grammar structs themselves, so the surface cannot drift. +// +// It is a Go test rather than a prose review because a prose review cannot fail, and a control that +// cannot fail is not coverage (R7). + +// What the control reads — the surface Kong owns, and nothing else: +// +// - A USAGE CLAIM is a text that presents an invocation of the command: a line inside a fenced +// code block that invokes the command, or an inline code span that begins with the command. +// Every `--flag` in a usage claim must be a flag the command accepts. +// - An ALIAS ROW is a markdown table row whose first cell is a `--flag`. Such a row is the body +// claiming "this flag has this environment variable", and Kong binds an environment variable +// only as the alias of a flag — so both names in such a row must be the grammar's. +// - A FLAG BULLET is a list item under a heading that names flags. Each of these bodies +// documents exactly ONE command, so a flags section in it is that command's flags. A bullet +// that names a flag and an environment variable makes the same alias claim as an alias row. +// +// What the control deliberately does NOT read, and why: +// +// - PROSE that mentions the command without invoking it. The project-directory override paragraph +// every body carries ("`charly box build` resolves `charly.yml` via `os.Getwd()`. Override with +// `-C ` / `--dir ` / `CHARLY_PROJECT_DIR=`") names the HOST's project-directory +// surface, not this plugin's: `-C`, `--dir` and `CHARLY_PROJECT_DIR` belong to the core's +// dispatch, so holding the plugin's grammar responsible for them would be a false claim. +// - An environment variable that an implementation reads directly rather than through Kong (a +// diagnostic hook such as `CHARLY_MERGE_KEEP_TMP`, whose body cites the code that reads it). +// Kong does not bind it, and this repo's grammar is not the place that owns it. +// +// Neither exclusion is an allowlist of names: both are stated RULES about which SURFACE is read, +// so a later prose rewrite cannot smuggle a false flag in through a sentence. + +// flagToken matches a CLI flag as it is written in prose: `--name`, with the name's characters +// restricted to what Kong accepts for a flag name. +var flagToken = regexp.MustCompile(`--[a-z0-9][a-z0-9-]*`) + +// envToken matches an environment variable as it is written in prose. +var envToken = regexp.MustCompile(`CHARLY_[A-Z0-9_]+`) + +// flagHeading matches a heading that names flags. +var flagHeading = regexp.MustCompile(`(?i)\bflags?\b`) + +// skillDocCase is one skill body and the command surface it documents. +type skillDocCase struct { + skill string // the skill's directory under .agents/skills/ + command string // the command whose usage claims the body is scanned for + grammar any // the Kong-tagged grammar struct that command parses into (nil: no flag surface) +} + +// skillDocCases is the set of bodies this control reads. The grammar is the struct the command +// actually parses into (see dispatchBuild and its siblings), so a flag the grammar defines and a +// flag the body documents are compared against the SAME source of truth. +// +// `list` has no Kong grammar: `charly box list` dispatches its subcommands by hand (dispatchList), +// so it has no flag surface at all and its body may document no flag. +var skillDocCases = []skillDocCase{ + {skill: "build", command: "charly box build", grammar: &buildGrammar{}}, + {skill: "generate", command: "charly box generate", grammar: &generateGrammar{}}, + {skill: "validate", command: "charly box validate", grammar: &validateGrammar{}}, + {skill: "pull", command: "charly box pull", grammar: &pullGrammar{}}, + {skill: "merge", command: "charly box merge", grammar: &mergeGrammar{}}, + {skill: "inspect", command: "charly box inspect", grammar: &inspectGrammar{}}, + {skill: "load", command: "charly box load", grammar: &loadGrammar{}}, + {skill: "reconcile", command: "charly box reconcile", grammar: &reconcileGrammar{}}, + {skill: "new", command: "charly box new", grammar: &newGrammar{}}, + {skill: "list", command: "charly box list", grammar: nil}, +} + +// kongSurface returns the flag names and env bindings a Kong grammar defines. It walks Kong's own +// model, so the answer is what the parser actually accepts — not a list maintained beside it. +func kongSurface(t *testing.T, grammar any) (flags map[string]bool, envs map[string]bool) { + t.Helper() + flags = map[string]bool{} + envs = map[string]bool{} + if grammar == nil { + return flags, envs + } + parser, err := kong.New(grammar) + if err != nil { + t.Fatalf("kong.New(%T): %v", grammar, err) + } + add := func(fs []*kong.Flag) { + for _, f := range fs { + flags[f.Name] = true + for _, e := range f.Envs { + envs[e] = true + } + } + } + add(parser.Model.Flags) + var walk func(n *kong.Node) + walk = func(n *kong.Node) { + for _, c := range n.Children { + add(c.Flags) + walk(c) + } + } + walk(parser.Model.Node) + return flags, envs +} + +// docClaim is one place in a body where the body makes a claim about the command's surface. +type docClaim struct { + line int // the 1-based line the claim is on + text string // the text the claim is made in +} + +// invocationText returns the text when it invokes the command — with an optional leading +// environment assignment (`CHARLY_X=1 charly box build …`) and an optional leading backtick +// stripped — and reports false otherwise. +func invocationText(text, command string) (string, bool) { + text = strings.TrimSpace(text) + text = strings.TrimPrefix(text, "`") + if eq := strings.Index(text, "="); eq > 0 && strings.HasPrefix(text, "CHARLY_") { + text = strings.TrimSpace(text[eq+1:]) + } + if !strings.HasPrefix(text, command) { + return "", false + } + return text, true +} + +// inlineSpans returns the inline code spans of a line, in order. +func inlineSpans(line string) []string { + var out []string + rest := line + for { + i := strings.Index(rest, "`") + if i < 0 { + return out + } + rest = rest[i+1:] + j := strings.Index(rest, "`") + if j < 0 { + return out + } + out = append(out, rest[:j]) + rest = rest[j+1:] + } +} + +// usageClaims returns the body's usage claims for the command: the lines inside a fenced code +// block that invoke it, and the inline code spans that begin with it. A prose sentence that merely +// mentions the command is not a usage claim — it is prose, and prose is not what a reader copies. +func usageClaims(body []string, command string) []docClaim { + var out []docClaim + inBlock := false + for i, raw := range body { + line := strings.TrimSpace(raw) + if strings.HasPrefix(line, "```") { + inBlock = !inBlock + continue + } + if inBlock { + if text, ok := invocationText(line, command); ok { + out = append(out, docClaim{line: i + 1, text: text}) + } + continue + } + for _, span := range inlineSpans(line) { + if text, ok := invocationText(span, command); ok { + out = append(out, docClaim{line: i + 1, text: text}) + } + } + } + return out +} + +// aliasRows returns the body's alias rows: the markdown table rows whose first cell is a flag. +// Such a row is the body claiming "this flag has this environment variable". +func aliasRows(body []string) []docClaim { + var out []docClaim + for i, raw := range body { + line := strings.TrimSpace(raw) + if !strings.HasPrefix(line, "|") { + continue + } + cells := strings.Split(line, "|") + if len(cells) < 3 { + continue + } + flagCell := strings.TrimSpace(cells[1]) + if !flagToken.MatchString(flagCell) { + continue + } + out = append(out, docClaim{line: i + 1, text: line}) + } + return out +} + +// flagSectionBullets returns the body's flag bullets: the list items under a heading that names +// flags. These bodies document one command each, so such a bullet is a claim about that command's +// flags. +func flagSectionBullets(body []string) []docClaim { + var out []docClaim + inFlagSection := false + for i, raw := range body { + line := strings.TrimSpace(raw) + if strings.HasPrefix(line, "#") { + inFlagSection = flagHeading.MatchString(line) + continue + } + if !inFlagSection { + continue + } + if line == "" { + continue + } + if !strings.HasPrefix(line, "- ") && !strings.HasPrefix(line, "* ") && !strings.HasPrefix(line, "+ ") { + continue + } + out = append(out, docClaim{line: i + 1, text: line}) + } + return out +} + +// TestSkillBodiesDocumentOnlyRealFlags is the control. Every flag the body offers for its own +// command must be a flag the command accepts, and every environment variable it pairs with one of +// its flags must be one of the command's own bindings. +func TestSkillBodiesDocumentOnlyRealFlags(t *testing.T) { + for _, tc := range skillDocCases { + t.Run(tc.skill, func(t *testing.T) { + path := filepath.Join("..", "..", ".agents", "skills", tc.skill, "SKILL.md") + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + body := strings.Split(string(raw), "\n") + flags, envs := kongSurface(t, tc.grammar) + + for _, claim := range usageClaims(body, tc.command) { + for _, m := range flagToken.FindAllString(claim.text, -1) { + name := strings.TrimPrefix(m, "--") + if !flags[name] { + t.Errorf("%s:%d: the body documents %s, but %s does not define that flag", + path, claim.line, m, tc.command) + } + } + } + + for _, claim := range aliasRows(body) { + cells := strings.Split(claim.text, "|") + for _, m := range flagToken.FindAllString(cells[1], -1) { + name := strings.TrimPrefix(m, "--") + if !flags[name] { + t.Errorf("%s:%d: the body pairs %s with an environment variable, but %s does not define that flag", + path, claim.line, m, tc.command) + } + } + for _, m := range envToken.FindAllString(claim.text, -1) { + if !envs[m] { + t.Errorf("%s:%d: the body pairs a flag with %s, but %s does not bind that environment variable", + path, claim.line, m, tc.command) + } + } + } + + for _, claim := range flagSectionBullets(body) { + flagNames := flagToken.FindAllString(claim.text, -1) + for _, m := range flagNames { + name := strings.TrimPrefix(m, "--") + if !flags[name] { + t.Errorf("%s:%d: the body lists %s among the command's flags, but %s does not define that flag", + path, claim.line, m, tc.command) + } + } + if len(flagNames) == 0 { + continue + } + for _, m := range envToken.FindAllString(claim.text, -1) { + if !envs[m] { + t.Errorf("%s:%d: the body pairs a flag with %s, but %s does not bind that environment variable", + path, claim.line, m, tc.command) + } + } + } + }) + } +} diff --git a/charly.yml b/charly.yml index 8119fb4..7f83472 100644 --- a/charly.yml +++ b/charly.yml @@ -454,6 +454,12 @@ check-box-local: # (`base:` / `from:`) may not own a `skill:`. The node is re-homed onto a package # candy the image composes, and until it is, validate must say so LOUDLY and name # that fix. This is the product-boundary witness for validateSkillOwners. + # + # The control's body lives at the OWNING CANDY'S REPO ROOT, which for a candy inside + # a project is the PROJECT ROOT — the layout every real skill owner uses (the candy + # sits under `candy/`, its `.agents/skills/` at the repo root). The sdk's body + # lookup resolves that root from the candy's source dir (deploykit/skills_collect.go + # → candyRepoRoot) and looks there; it did NOT look beside the candy. # --------------------------------------------------------------------------- - check: >- an IMAGE candy owning a `skill:` is REJECTED with the ruling's message — it names @@ -462,8 +468,9 @@ check-box-local: fixture carries an IN-LINE `content:`, which is the arm that validated CLEAN (exit 0) before the rule: the shape is what is forbidden, not the missing body, so this step FAILS on the pre-fix engine. Its control — the SAME skill re-owned - by the package candy the image composes, with its body beside that candy — must - validate clean, so the step cannot pass by rejecting everything. + by the package candy the image composes, with its body at that candy's REPO ROOT + (the project root here, where every real skill owner keeps `.agents/skills/`) — + must validate clean, so the step cannot pass by rejecting everything. id: box-skill-image-owner exit_status: 0 stdout: @@ -477,7 +484,7 @@ check-box-local: mkdir -p "$(dirname "$EV")" D=$(mktemp -d) trap 'rm -rf "$D"' EXIT - mkdir -p "$D/candy/pcandy/.agents/skills/psk1" + mkdir -p "$D/candy/pcandy" cat > "$D/charly.yml" <<'PROJ' discover: - path: candy @@ -515,8 +522,6 @@ check-box-local: exit_status: 0 command: "true" CANDY - printf -- '---\nname: psk1\ndescription: package-candy-owned skill\n---\n\nBody prose owned by the package candy.\n' \ - > "$D/candy/pcandy/.agents/skills/psk1/SKILL.md" { echo "binary under test: ${CHARLY_BIN}" echo "version: $("${CHARLY_BIN}" version 2>/dev/null | tail -1)" @@ -539,12 +544,16 @@ check-box-local: grep -q 'opencharly/sdk#434' "$EV" || { echo 'FAIL: the diagnostic does not cite the ruling'; cat "$EV"; exit 1; } grep -q 'no SKILL.md exists under' "$EV" && { echo 'FAIL: the diagnostic still claims a file search that never ran'; cat "$EV"; exit 1; } # CONTROL: the ruling's own fix — the SAME skill owned by the package candy the image - # composes, with NO in-line content and its body beside that candy — must validate - # CLEAN. A second scratch project, so the control proves the fixed SHAPE *and* the - # body resolution the re-home relies on, not just that the first fixture was rewritten. + # composes, with NO in-line content and its body at that candy's REPO ROOT — must + # validate CLEAN. A second scratch project, so the control proves the fixed SHAPE *and* + # the body resolution the re-home relies on, not just that the first fixture was + # rewritten. The body goes to the PROJECT ROOT because that is the owning candy's + # repo root (sdk/deploykit/skills_collect.go → candyRepoRoot), and it is the layout + # every real skill owner uses: the candy under `candy/`, its `.agents/skills/` at the + # repo root. Beside the candy is NOT where the lookup looks. D2=$(mktemp -d) trap 'rm -rf "$D" "$D2"' EXIT - mkdir -p "$D2/candy/pcandy/.agents/skills/psk1" + mkdir -p "$D2/candy/pcandy" "$D2/.agents/skills/psk1" cat > "$D2/charly.yml" <<'PROJ2' discover: - path: candy @@ -569,9 +578,10 @@ check-box-local: description: owned by the package candy the image composes - the ruled fix PROJ2 cp "$D/candy/pcandy/charly.yml" "$D2/candy/pcandy/charly.yml" - cp "$D/candy/pcandy/.agents/skills/psk1/SKILL.md" "$D2/candy/pcandy/.agents/skills/psk1/SKILL.md" + printf -- '---\nname: psk1\ndescription: package-candy-owned skill\n---\n\nBody prose owned by the package candy.\n' \ + > "$D2/.agents/skills/psk1/SKILL.md" { - echo "--- control: owner re-homed to the package candy pcandy, body beside it ---" + echo "--- control: owner re-homed to the package candy pcandy, body at that candy's repo root ---" set +e CTL=$("${CHARLY_BIN}" -C "$D2" box validate 2>&1) CRC=$?