From ee1508573a3795e0c92ab10d7ced030e5c598607 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Tue, 21 Jul 2026 21:49:12 +0100 Subject: [PATCH] feat: stamp the version into the release payload and smoke-test it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4 of the derived-versioning programme (itd-67 / spc-11), and the last. THE VERSION LANDS ON THE PAYLOAD, NEVER ON THE TREE `RenderPayload` stamps the derived version into the payload's copies of plugin.json and marketplace.json — primary, marketplace secondary, and the changelog entry — in lockstep. The working tree's manifests stay version-absent (ADR-19) and a test asserts a render leaves them byte-identical. The new .abcd/config/version-location.json is a decision artefact: it says WHERE a version goes (an RFC-6901 path), never what it is. THE LOCKSTEP GATE WAS INERT AND IS NOW LIVE version-location.json was absent, so CheckLockstep returned unreadable and `launch --dry-run` could never pass. Adding it exposed a second fault: DryRun and Ship asserted the PUBLIC polarity against the working tree, which under ADR-19 is version-absent forever, so the gate could never be satisfied and its diagnostic told the operator to add the very key the policy forbids. Both now assert TreeDev over the source tree; TreePublic stays where it belongs, over the rendered payload. The dry-run reports lockstep dev/ok and no refusals for the first time. THE LIGHT SMOKE Manifests parse, `source` resolves, and every declared command/agent/hook path exists in the resolved payload bundle — not in the working tree, because a file present in the tree but excluded from the payload is exactly the bug this catches. Wired into DryRun and Ship, so it gates rather than merely tests. Resolution is split from assertion behind a PayloadTree interface, with both implementations pinned to the same resolved list, so itd-66's deferred deep tier reuses the resolution unchanged. NOTHING PARTIAL, EVEN ACROSS STEPS The version-free half of the render — destination checks, bundle resolution and the smoke — is hoisted into PrecheckPayload and runs BEFORE the changelog write, so a payload refusal can no longer leave a written release record behind a staged directory. A rollback backstop restores the record if a later error slips through, and the message says which happened. Assisted-by: Claude:claude-opus-4-8 --- .abcd/config/version-location.json | 9 + .../brief/04-surfaces/04-launch.md | 16 +- .abcd/development/release/surface.json | 7 + README.md | 27 + commands/abcd/launch.md | 17 + docs/reference/cli/commands.md | 3 +- internal/core/launch/dryrun.go | 80 +-- internal/core/launch/dryrun_polarity_test.go | 98 +++ internal/core/launch/dryrun_test.go | 22 +- internal/core/launch/installsurface.go | 639 ++++++++++++++++++ internal/core/launch/installsurface_test.go | 283 ++++++++ internal/core/launch/lockstep.go | 10 +- internal/core/launch/lockstep_repo_test.go | 46 ++ internal/core/launch/lockstep_test.go | 5 +- internal/core/launch/readme_install_test.go | 98 +++ internal/core/launch/render.go | 440 ++++++++++++ internal/core/launch/render_test.go | 259 +++++++ internal/core/launch/semver.go | 20 + internal/core/launch/ship.go | 26 +- internal/core/launch/smoke.go | 155 +++++ internal/core/launch/smoke_test.go | 160 +++++ internal/surface/cli/cli.go | 5 +- internal/surface/cli/ship.go | 167 ++++- internal/surface/cli/ship_payload_test.go | 278 ++++++++ 24 files changed, 2785 insertions(+), 85 deletions(-) create mode 100644 .abcd/config/version-location.json create mode 100644 internal/core/launch/dryrun_polarity_test.go create mode 100644 internal/core/launch/installsurface.go create mode 100644 internal/core/launch/installsurface_test.go create mode 100644 internal/core/launch/lockstep_repo_test.go create mode 100644 internal/core/launch/readme_install_test.go create mode 100644 internal/core/launch/render.go create mode 100644 internal/core/launch/render_test.go create mode 100644 internal/core/launch/smoke.go create mode 100644 internal/core/launch/smoke_test.go create mode 100644 internal/surface/cli/ship_payload_test.go diff --git a/.abcd/config/version-location.json b/.abcd/config/version-location.json new file mode 100644 index 0000000..ceaaa00 --- /dev/null +++ b/.abcd/config/version-location.json @@ -0,0 +1,9 @@ +{ + "outcome": "accept", + "blocked": false, + "manifest_path": ".claude-plugin/plugin.json", + "json_pointer": "/version", + "version_property_clause": "claude-code-plugin-manifest.schema.json#/properties/version", + "marketplace_source_resolution_base": "repo_root", + "marketplace_source_to_root": "./" +} diff --git a/.abcd/development/brief/04-surfaces/04-launch.md b/.abcd/development/brief/04-surfaces/04-launch.md index 9d7380c..edbd52a 100644 --- a/.abcd/development/brief/04-surfaces/04-launch.md +++ b/.abcd/development/brief/04-surfaces/04-launch.md @@ -7,7 +7,7 @@ The shipped verb surface is the `--dry-run` flag on `abcd launch` — a read-only preview of the bundle and gates. Bare `abcd launch` never mutates state: it refuses (exit 1) with a hint to pass `--dry-run`; a bare-as-status render is a design target, unshipped. The sub-verb design: - **`/abcd:launch ship`** — **partly shipped: the RELEASE CUT only** (itd-73 derived versioning + itd-67's changelog slice). `abcd launch ship` derives the version and the record set from what shipped since the newest tag, runs the surface guardrail, and — with `--changelog-json` — validates the host-composed prose against the record set (the completeness bijection) and writes the dated `CHANGELOG.md` heading `.github/workflows/auto-release.yml` turns into a tag; `commands/abcd/launch.md` carries the emit → compose → ingest orchestration and the `release-changelog-composer` agent it dispatches. The ARTEFACT half stays a design target (itd-65 gate suite + itd-72 publishing): `internal/core/launch` carries the `Ship` engine, unwired, and the verb neither packages, commits, tags, nor publishes. The design for that half: cut a curated release artefact from the one repo: run pre-flight gates, filter the artefact (default-deny, `.abcd/**` excluded by packaging), stamp the version, and on a `v*` tag publish a GitHub Release ([adr-28](../../decisions/adrs/0028-single-repo-curated-release.md)). The flow described in §§ 1–6 below is this sub-verb's behaviour. Flag-shaped modifiers `--allow-dirty` and `--allow-doc-warnings` belong to this sub-verb's design; the shipped verb accepts only `--dry-run` and the global `--json`. There is no version flag — the version is derived, never authored ([adr-31](../../decisions/adrs/0031-derived-versioning-from-intents.md), see [§ 3](#3-versioning--marketplace)). -- **`/abcd:launch dry-run`** — shipped as the `--dry-run` flag (the plugin command `commands/abcd/launch.md` maps its `[dry-run]` argument hint onto `abcd launch --dry-run`; the binary has no `dry-run` subcommand). **Report-only preview, always exit-0** (a preview never blocks). It runs the parts of the pre-flight suite that exist today: as of spc-64 the **secret + PII scan gate** (the native scanners, see [§ 1](#1-pre-flight-gates)) runs for real in report-only mode and prints what it *would* refuse on (a finding, or a fail-closed reason such as "scanner unavailable"); the remaining gates (marker-block, `plugin.json` parse, documentation-auditor) are the gate-suite intent's (itd-65) and render as "(not yet implemented)". It also produces the would-be artefact manifest, without writing the release artefact. dry-run is **not** "ship minus publish": running the *full* gate suite and **hard-failing** on a finding (exit non-zero) is the full `ship` verb's behaviour (itd-65 + itd-72), not dry-run's. +- **`/abcd:launch dry-run`** — shipped as the `--dry-run` flag (the plugin command `commands/abcd/launch.md` maps its `[dry-run]` argument hint onto `abcd launch --dry-run`; the binary has no `dry-run` subcommand). **Report-only preview, always exit-0** (a preview never blocks). It runs the parts of the pre-flight suite that exist today: as of spc-64 the **secret + PII scan gate** (the native scanners, see [§ 1](#1-pre-flight-gates)) runs for real in report-only mode and prints what it *would* refuse on (a finding, or a fail-closed reason such as "scanner unavailable"); the **installability smoke** runs for real at its light tier (see [§ 1](#1-pre-flight-gates)); the remaining gates (marker-block, documentation-auditor) are the gate-suite intent's (itd-65) and render as "(not yet implemented)". It also produces the would-be artefact manifest, without writing the release artefact. dry-run is **not** "ship minus publish": running the *full* gate suite and **hard-failing** on a finding (exit non-zero) is the full `ship` verb's behaviour (itd-65 + itd-72), not dry-run's. ## 1. Pre-flight gates @@ -17,7 +17,10 @@ The shipped verb surface is the `--dry-run` flag on `abcd launch` — a read-onl - **TruffleHog** — opt-in deep scan when `scan.deep=true` — hard-fail (live credential verification) - **Hook compliance** check — warn-fail - **Marker block sanity** — hard-fail on malformed -- **`plugin.json` parse + `marketplace.json` references** — hard-fail +- **Installability smoke** (`plugin.json` parse + `marketplace.json` references) — hard-fail. Two tiers over ONE surface resolution (`internal/core/launch/installsurface.go`), so the deeper tier upgrades the assertions and never redefines the surface: + - **The declared surface** is the UNION of two registers, because the plugin-manifest schema defines every explicit key as declaring entries *in addition to* the convention directory: **convention** — `commands/**/*.md`, `agents/*.md` (a flat glob; iss-110 is the evidence), `skills/*/SKILL.md`, `hooks/hooks.json` — and **manifest** — the optional `commands`/`agents`/`skills`/`hooks` keys in `plugin.json`. A hook's `$CLAUDE_PLUGIN_ROOT`-rooted command adds the payload file it invokes; the plugin executable itself is install-supplied, so its absence from the payload proves nothing. Every entry records which register declared it. This is NOT the compatibility surface (`internal/core/surface`), which records manifest KEYS and discards values; installability is the mirror question, over the values. + - **Light tier** (itd-67, shipped) — both manifests parse, each local marketplace `source` resolves to a manifest whose name matches the listing, and every declared path the payload is responsible for is carried. Resolution reads the **resolved bundle**, so a file present in the tree but excluded from the payload fails here. `dry-run` reports it; `RenderPayload` refuses on it, because the render is the only step that materialises an artefact. + - **Deep tier** (itd-66, deferred) — over the same resolved list: import every shipped entrypoint and render each command's help/frontmatter in an isolated subprocess rooted at the rendered payload. - **Dirty tree** — refuse unless `--allow-dirty` - **OWASP / vulnerability check** (folded into the pre-flight suite) — warn-fail - **Documentation auditor** (subagent) — runs over `docs/` to verify user-facing documentation is well-formed before release — warn-fail @@ -79,11 +82,10 @@ the spc-77.1 decision artifact (`.abcd/config/version-location.json`) as `manifest_path` + `json_pointer` (see [adr-19](../../decisions/adrs/0019-plugin-json-version-carve-out.md)). The -artifact is absent from the tree, and the shipped lockstep checker fails closed -on it — `dry-run` reports the lockstep contract unreadable — until spc-77.1 -records the decision; a -`blocked: true` decision has no schema-valid location, so version-writing -refuses and the escalation stands. Concretely, `ship`: +artifact records spc-77.1's ACCEPT outcome — `.claude-plugin/plugin.json` at +`/version` — and the shipped lockstep checker fails closed when it is missing or +malformed; a `blocked: true` decision has no schema-valid location, so +version-writing refuses and the escalation stands. Concretely, `ship`: 1. Stamps the bumped version into the **release artefact** at the selected `manifest_path` + `json_pointer` — the manifest renderer reads the decision diff --git a/.abcd/development/release/surface.json b/.abcd/development/release/surface.json index b0fa166..bef0d09 100644 --- a/.abcd/development/release/surface.json +++ b/.abcd/development/release/surface.json @@ -454,6 +454,13 @@ "type": "string", "required": false, "hidden": false + }, + { + "name": "payload-dir", + "shorthand": "", + "type": "string", + "required": false, + "hidden": false } ] }, diff --git a/README.md b/README.md index a34301f..5249f7f 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,33 @@ and copy the binary anywhere on your `PATH`. Every release is built and published by CI from the exact tagged commit, with the checksums generated over the same bytes that are uploaded. +## Plugin + +This repository is also its own plugin marketplace, so a compatible agent +harness can install the `/abcd:*` surface — the commands under +[`commands/`](commands/), the agents under [`agents/`](agents/) and the hook +wiring in [`hooks/`](hooks/) — straight from it. Add the marketplace, then +install the plugin: + +```text +/plugin marketplace add REPPL/abcd-cli +/plugin install abcd@abcd-marketplace +``` + +`abcd-marketplace` is the marketplace name declared in +[`.claude-plugin/`](.claude-plugin/); `abcd` is the single plugin it lists, +sourced from the repository root. Pull the current state of the marketplace with: + +```text +/plugin update abcd +``` + +The marketplace is served from the repository itself, so an install tracks the +repository rather than a versioned artefact: the manifests here carry no version +key, and a release publishes the `abcd` binaries and their checksums. The +commands drive the `abcd` binary, so keep the [install](#install) above in place +alongside the plugin. + ## Build ```bash diff --git a/commands/abcd/launch.md b/commands/abcd/launch.md index a212b41..44ed6a5 100644 --- a/commands/abcd/launch.md +++ b/commands/abcd/launch.md @@ -29,6 +29,9 @@ Then summarise the JSON for the user: - `version` — the version the release would carry. - `files` — how many files the bundle would include. - `scan.hard_fails` — secret/PII findings that would block the release. +- `smoke.ok` — whether the payload would install: both plugin manifests parse, + the marketplace source resolves, and every declared command, agent, skill and + hook path is carried. `smoke.findings` names any path that is not. - `would_publish` — whether every gate passes. - `would_refuse_on` — if non-empty, the gates that would refuse, so the user knows what to fix before a real launch. @@ -92,8 +95,22 @@ Write the agent's payload to a file and hand it back to the binary: ```bash abcd launch ship --changelog-json # or - for stdin +abcd launch ship --changelog-json --payload-dir # also stage the payload ``` +With `--payload-dir` the binary additionally stages the release payload in that +directory — an empty directory outside the repository — with the derived version +stamped into the payload's copies of `plugin.json` and `marketplace.json`. The +repository's own manifests are never touched: they carry no version, and the +version belongs to the artefact. The staged payload is proved consistent before +the command returns, so a stamp that missed a pinned location is a refusal rather +than a published half-state. Every refusal the staging step can make is checked +BEFORE the dated heading is written, and a refusal that slips past that check +rolls the heading back — so a ship that exits non-zero leaves no release record +behind for the next attempt to trip over. Without the flag nothing is staged; +`--payload-dir` on its own (no `--changelog-json`) is an operand error, because +only a completed cut has a version to stamp. + The binary re-derives the cut, then proves the prose describes it — the **completeness bijection**: the set of record ids the payload cites must equal `(added ∪ removed)` minus the records marked `in_changelog: false`. On any diff --git a/docs/reference/cli/commands.md b/docs/reference/cli/commands.md index e5884df..aec15f8 100644 --- a/docs/reference/cli/commands.md +++ b/docs/reference/cli/commands.md @@ -342,12 +342,13 @@ Preview the public launch bundle and release gates (read-only) Cut a release: derive the version and the record set from what shipped (exit 1 when the cut refuses) -**Usage:** `abcd launch ship [--changelog-json ] [flags]` +**Usage:** `abcd launch ship [--changelog-json ] [--payload-dir ] [flags]` **Flags:** ``` --changelog-json string path to the host-composed changelog JSON (or - for stdin); absent runs the deterministic emit step + --payload-dir string stage the versioned release payload in this directory (must be empty and outside the repository) ``` ### `abcd memory` diff --git a/internal/core/launch/dryrun.go b/internal/core/launch/dryrun.go index dae9480..623de2f 100644 --- a/internal/core/launch/dryrun.go +++ b/internal/core/launch/dryrun.go @@ -11,9 +11,14 @@ const versionLocationRelPath = ".abcd/config/version-location.json" // DryRunRequest is the input to a dry-run. type DryRunRequest struct { - RepoRoot string - VersionOverride string // --version; empty → read from the resolved manifests - ExistingTags []Semver // injected; nil → default `git tag -l v*` provider + RepoRoot string + // Version is the release version this launch would publish, SUPPLIED by the + // caller. adr-19 leaves no version key in the source tree, so there is + // nothing here for the core to read: the version is a fact about the release + // cut, and the front door that knows the cut injects it. Empty is honest — + // it means the caller could not name one, and retention says so. + Version string + ExistingTags []Semver // injected; nil → default `git tag -l v*` provider } // GateSummary records one gate's dry-run disposition. @@ -30,6 +35,7 @@ type DryRunReport struct { Scan scanner.ScanResult `json:"scan"` Lockstep LockstepResult `json:"lockstep"` Retention RetentionPlan `json:"retention"` + Smoke SmokeReport `json:"smoke"` Gates []GateSummary `json:"gates"` WouldPublish bool `json:"would_publish"` // always false in dry-run WouldRefuseOn []string `json:"would_refuse_on,omitempty"` @@ -51,23 +57,33 @@ func DryRun(req DryRunRequest) (DryRunReport, error) { scan := scanBundle(req.RepoRoot, bundle) report.Scan = scan + // The DEV polarity is the one the SOURCE TREE must satisfy: adr-19 keeps the + // version keys out of the committed manifests, so a public check here would + // accuse a correct repository of drift and prescribe the exact key the ADR + // forbids. The public polarity belongs over the rendered payload, where + // RenderPayload applies it to its own output. vlPath := filepath.Join(req.RepoRoot, versionLocationRelPath) - lockstep := CheckLockstep(TreePublic, req.RepoRoot, vlPath) + lockstep := CheckLockstep(TreeDev, req.RepoRoot, vlPath) report.Lockstep = lockstep - version := resolveVersion(req.VersionOverride, req.RepoRoot, lockstep) - report.Version = version + report.Version = req.Version + report.Retention = computeRetentionForReport(req.Version, req) - report.Retention = computeRetentionForReport(version, req) + // The smoke reads the RESOLVED BUNDLE, not the working tree: a file present + // in the tree but excluded from the payload is exactly the break it exists + // to catch. It subsumes itd-65's placeholder `plugin.json-parse` gate, which + // asserted a strict subset of what the light tier asserts. + smoke := SmokeLight(NewBundleTree(bundle)) + report.Smoke = smoke report.Gates = []GateSummary{ {Name: "secret+pii-scan", Status: "ran", Detail: scanDetail(scan)}, {Name: "marker-block", Status: "not_implemented", Detail: "Phase-5 deferred"}, - {Name: "plugin.json-parse", Status: "not_implemented", Detail: "Phase-5 deferred"}, + {Name: "installability-smoke", Status: "ran", Detail: smokeDetail(smoke)}, {Name: "documentation-auditor", Status: "not_implemented", Detail: "Phase-5 deferred"}, } - report.WouldRefuseOn = wouldRefuseOn(bundle, scan, lockstep, report.Retention) + report.WouldRefuseOn = wouldRefuseOn(bundle, scan, lockstep, report.Retention, smoke) report.WouldPublish = false return report, nil } @@ -86,46 +102,6 @@ func scanBundle(repoRoot string, bundle Bundle) scanner.ScanResult { return res } -// resolveVersion picks the version input: an explicit override, else the -// lockstep primary version when readable. -func resolveVersion(override, repoRoot string, lockstep LockstepResult) string { - if override != "" { - return override - } - if lockstep.OK { - if v := primaryVersion(repoRoot); v != "" { - return v - } - } - return "" -} - -// primaryVersion reads the primary version string via the version-location -// contract (best-effort; empty when unreadable). -func primaryVersion(repoRoot string) string { - vlPath := filepath.Join(repoRoot, versionLocationRelPath) - decision, err := loadJSON(vlPath) - if err != nil { - return "" - } - mp, ptr, verr := validateVersionLocation(decision) - if verr != "" { - return "" - } - doc, err := loadJSON(filepath.Join(repoRoot, mp)) - if err != nil { - return "" - } - v, present := resolvePointer(doc, ptr) - if !present { - return "" - } - if s, ok := v.(string); ok { - return s - } - return "" -} - // computeRetentionForReport builds the retention preview, resolving the existing // tag list from the injected slice or the default git provider. func computeRetentionForReport(version string, req DryRunRequest) RetentionPlan { @@ -144,8 +120,9 @@ func computeRetentionForReport(version string, req DryRunRequest) RetentionPlan } // wouldRefuseOn collects everything a real ship WOULD block on: scan hard-fails, -// bundle rejections, lockstep drift/unreadable, and retention refusal. -func wouldRefuseOn(bundle Bundle, scan scanner.ScanResult, lockstep LockstepResult, retention RetentionPlan) []string { +// bundle rejections, lockstep drift/unreadable, retention refusal, and an +// uninstallable declared surface. +func wouldRefuseOn(bundle Bundle, scan scanner.ScanResult, lockstep LockstepResult, retention RetentionPlan, smoke SmokeReport) []string { var reasons []string if scan.Unavailable { reasons = append(reasons, "scanner unavailable: "+scan.UnavailableReason) @@ -165,6 +142,7 @@ func wouldRefuseOn(bundle Bundle, scan scanner.ScanResult, lockstep LockstepResu if retention.Refused { reasons = append(reasons, "retention refused: "+retention.RefusalReason) } + reasons = append(reasons, smokeRefusals(smoke)...) return reasons } diff --git a/internal/core/launch/dryrun_polarity_test.go b/internal/core/launch/dryrun_polarity_test.go new file mode 100644 index 0000000..7c7bf5d --- /dev/null +++ b/internal/core/launch/dryrun_polarity_test.go @@ -0,0 +1,98 @@ +package launch + +import ( + "strings" + "testing" +) + +// TestDryRunAssertsTheDevPolarity is the adr-19 detector for the gate a +// maintainer actually runs. +// +// The dry-run and the ship read the SOURCE TREE, and post-adr-19 the source tree +// carries no version key by design. Asserting the public polarity there accuses +// a correct tree of drift and tells the operator to add the very key adr-19 +// keeps out; the public polarity belongs over the RENDERED PAYLOAD, where +// RenderPayload already applies it. +// +// The cases are the two polarities of that one rule: a version-absent tree is +// clean, and a tree that somehow acquired a version key is drift. +func TestDryRunAssertsTheDevPolarity(t *testing.T) { + cases := []struct { + name string + // version is the key written into both manifests; empty writes none. + version string + wantOK bool + wantDrift string + }{ + { + name: "an adr-19 version-absent tree is clean", + wantOK: true, + }, + { + name: "a version key in the working tree is drift", + version: "1.2.3", + wantOK: false, + wantDrift: "DRIFT dev .claude-plugin/plugin.json/version", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + writeFile(t, root, ".abcd/config/launch-payload.json", + `{"includes": [".claude-plugin", "README.md"]}`) + writeFile(t, root, "README.md", "clean readme\n") + writeLockstepTree(t, root, tc.version, tc.version, tc.version) + + report, err := DryRun(DryRunRequest{RepoRoot: root, Version: "1.2.3"}) + if err != nil { + t.Fatalf("dry-run preflight must succeed: %v", err) + } + if report.Lockstep.Tree != TreeDev { + t.Errorf("the source tree must be checked under %q, got %q", TreeDev, report.Lockstep.Tree) + } + if report.Lockstep.OK != tc.wantOK { + t.Fatalf("lockstep OK = %v, want %v (%+v)", report.Lockstep.OK, tc.wantOK, report.Lockstep) + } + if tc.wantOK { + for _, r := range report.WouldRefuseOn { + if strings.Contains(r, "lockstep") { + t.Errorf("a version-absent tree must not be refused: %q", r) + } + } + return + } + var saw bool + for _, d := range report.Lockstep.Drifts { + if strings.Contains(d, tc.wantDrift) { + saw = true + } + } + if !saw { + t.Errorf("expected a drift naming %q, got %v", tc.wantDrift, report.Lockstep.Drifts) + } + }) + } +} + +// TestDryRunVersionComesFromTheCaller pins where the previewed version comes +// from: the caller supplies it, because adr-19 leaves nothing in the tree to +// read. A dry-run that resolved it from a manifest would report the empty string +// on every correct repository and then refuse retention for it. +func TestDryRunVersionComesFromTheCaller(t *testing.T) { + root := t.TempDir() + writeFile(t, root, ".abcd/config/launch-payload.json", `{"includes": [".claude-plugin", "README.md"]}`) + writeFile(t, root, "README.md", "clean readme\n") + writeLockstepTree(t, root, "", "", "") + + report, err := DryRun(DryRunRequest{RepoRoot: root, Version: "2.5.0"}) + if err != nil { + t.Fatalf("dry-run preflight must succeed: %v", err) + } + if report.Version != "2.5.0" { + t.Errorf("Version = %q, want the supplied 2.5.0", report.Version) + } + if report.Retention.Refused { + t.Errorf("a supplied strict-SemVer version must not refuse retention: %s", report.Retention.RefusalReason) + } +} diff --git a/internal/core/launch/dryrun_test.go b/internal/core/launch/dryrun_test.go index 9e37e71..ae5a973 100644 --- a/internal/core/launch/dryrun_test.go +++ b/internal/core/launch/dryrun_test.go @@ -19,7 +19,7 @@ func TestDryRunSecretRefusesButExitsZero(t *testing.T) { writeFile(t, root, ".abcd/config/launch-payload.json", `{"includes": ["commands"]}`) writeFile(t, root, "commands/x.md", "# doc\ntoken = "+fakeSecret+"\n") - report, err := DryRun(DryRunRequest{RepoRoot: root, VersionOverride: "1.0.0"}) + report, err := DryRun(DryRunRequest{RepoRoot: root, Version: "1.0.0"}) if err != nil { t.Fatalf("dry-run must return nil error on a finding, got %v", err) } @@ -53,7 +53,7 @@ func TestShipBlocksOnSecret(t *testing.T) { writeFile(t, root, ".abcd/config/launch-payload.json", `{"includes": ["commands"]}`) writeFile(t, root, "commands/x.md", "token = "+fakeSecret+"\n") - report, err := Ship(ShipRequest{RepoRoot: root, VersionOverride: "1.0.0"}) + report, err := Ship(ShipRequest{RepoRoot: root, Version: "1.0.0"}) if !errors.Is(err, ErrShipBlocked) { t.Fatalf("ship must return ErrShipBlocked, got %v", err) } @@ -76,12 +76,16 @@ func TestDryRunPreflightError(t *testing.T) { // WouldPublish=true with no error and no network call. func TestShipCleanWouldPublish(t *testing.T) { root := t.TempDir() - writeFile(t, root, ".abcd/config/launch-payload.json", `{"includes": ["commands", "README.md"]}`) + // The payload must carry .claude-plugin: a bundle without the manifests is + // not an installable plugin, which the installability gate now says out loud. + writeFile(t, root, ".abcd/config/launch-payload.json", `{"includes": [".claude-plugin", "commands", "README.md"]}`) writeFile(t, root, "commands/a.md", "clean content\n") writeFile(t, root, "README.md", "clean readme\n") - writeLockstepTree(t, root, "1.2.3", "1.2.3", "1.2.3") + // adr-19: a clean SOURCE tree is version-ABSENT; the version keys appear + // only in the rendered payload. + writeLockstepTree(t, root, "", "", "") - report, err := Ship(ShipRequest{RepoRoot: root, VersionOverride: "1.2.3"}) + report, err := Ship(ShipRequest{RepoRoot: root, Version: "1.2.3"}) if err != nil { t.Fatalf("clean tree must not error: %v (reasons %v)", err, report.BlockReasons) } @@ -102,9 +106,11 @@ func TestZeroCoverageRefuses(t *testing.T) { writeFile(t, root, ".abcd/config/launch-payload.json", `{"includes": ["commands"]}`) writeFile(t, root, "commands/a.md", "clean content\n") writeFile(t, root, ".abcd/config/pii.json", `{"skip_extensions": [".md"]}`) - writeLockstepTree(t, root, "1.2.3", "1.2.3", "1.2.3") + // adr-19: a clean SOURCE tree is version-ABSENT; the version keys appear + // only in the rendered payload. + writeLockstepTree(t, root, "", "", "") - report, err := DryRun(DryRunRequest{RepoRoot: root, VersionOverride: "1.2.3"}) + report, err := DryRun(DryRunRequest{RepoRoot: root, Version: "1.2.3"}) if err != nil { t.Fatalf("dry-run preflight must succeed: %v", err) } @@ -124,7 +130,7 @@ func TestZeroCoverageRefuses(t *testing.T) { t.Errorf("WouldRefuseOn must cite scanner unavailable, got %v", report.WouldRefuseOn) } - ship, err := Ship(ShipRequest{RepoRoot: root, VersionOverride: "1.2.3"}) + ship, err := Ship(ShipRequest{RepoRoot: root, Version: "1.2.3"}) if !errors.Is(err, ErrShipBlocked) { t.Fatalf("ship must be blocked on zero coverage, got %v", err) } diff --git a/internal/core/launch/installsurface.go b/internal/core/launch/installsurface.go new file mode 100644 index 0000000..8449e39 --- /dev/null +++ b/internal/core/launch/installsurface.go @@ -0,0 +1,639 @@ +package launch + +// installsurface.go — the single resolver of "what does this payload DECLARE?". +// +// # Why this is not the compatibility surface +// +// internal/core/surface already answers a different question: which cobra +// commands and which manifest KEYS exist, so a later release can be told it +// removed one. That is the compatibility surface — it records key paths and +// deliberately discards VALUES, because a changed description must not read as a +// break. The installability question is the mirror image: it cares only about +// the values (the paths a manifest points at) and not at all about which keys +// carry them. Sharing one walker would force the compatibility snapshot to start +// recording values, which would report every prose edit as a surface change — +// the exact failure its doc comment rules out. So this is a separate resolver +// over the same two manifests, and the justification is that the two answer +// disjoint questions about disjoint halves of the same JSON. +// +// It is likewise not internal/core/ahoy's plugin-root check, which asks whether +// the plugin INSTALLED ON THIS MACHINE is healthy. This asks whether the payload +// about to be published COULD install anywhere. +// +// # What "the declared surface" is +// +// The union of two registers, because the plugin-manifest schema defines every +// explicit key as declaring entries "in addition to those in the / +// directory, if it exists": +// +// - CONVENTION — the auto-discovery roots a harness loads with no manifest +// help at all: commands/**/*.md (nested directories namespace the command), +// agents/*.md (a flat glob — iss-110 is the evidence: agents/README.md IS +// registered), skills/*/SKILL.md, and hooks/hooks.json. +// - MANIFEST — the optional commands/agents/skills/hooks keys in plugin.json, +// each a path, a list of paths, or an inline definition. +// +// Every entry records which register it came from, so a later, stricter tier can +// treat the two differently WITHOUT re-resolving. The resolver reports what a +// harness would register, including the iss-110 mis-registrations; filtering +// those here would hide the defect that issue tracks. +// +// # Why resolution is separate from assertion +// +// Resolution answers "what does this payload declare"; smoke.go's light tier +// asserts each resolved path exists. itd-66's deep tier asserts far more about +// the SAME list — import each entrypoint, render each command's frontmatter, in +// an isolated subprocess — and does that by consuming this list rather than +// rebuilding one. PayloadTree is what makes that a drop-in: the light tier reads +// the RESOLVED BUNDLE (no materialisation), the deep tier reads a MATERIALISED +// payload directory, and TestPayloadTreeImplementationsResolveIdentically pins +// the two to the same answer. + +import ( + "encoding/json" + "fmt" + "io/fs" + "os" + "path" + "path/filepath" + "sort" + "strings" + + "github.com/REPPL/abcd-cli/internal/fsutil" +) + +// pluginManifestFile is the plugin's own manifest. Unlike the VERSION location +// (which version-location.json makes negotiable per adr-19), the manifest's own +// path is fixed by the harness's discovery rule — a plugin.json anywhere else is +// not found at all — so it is a constant here rather than a contract read. +const pluginManifestFile = ".claude-plugin/plugin.json" + +// maxSurfaceManifestBytes caps every guarded manifest read. A plugin manifest and +// a hooks manifest are short declarations, so a file that is not one must not +// stream unbounded input into a gate that runs in CI. +const maxSurfaceManifestBytes = 1 << 20 + +// PayloadTree is the read side of a payload: the only thing surface resolution +// needs from "the thing that would ship". Two implementations satisfy it — a +// resolved bundle (nothing materialised) and a rendered directory — so the same +// resolver serves the light tier and itd-66's deep tier unchanged. +type PayloadTree interface { + // Has reports whether a FILE exists at rel, a payload-relative, + // slash-separated path. A path escaping the payload is never present. + Has(rel string) bool + // Read returns the bytes at rel, guarded against oversized input. + Read(rel string) ([]byte, error) + // List returns every file at or beneath the directory dirRel, sorted, as + // payload-relative slash paths. An empty result means "no such directory". + List(dirRel string) []string +} + +// SurfaceKind is one register of the installable surface. +type SurfaceKind string + +const ( + SurfaceCommand SurfaceKind = "command" + SurfaceAgent SurfaceKind = "agent" + SurfaceSkill SurfaceKind = "skill" + SurfaceHook SurfaceKind = "hook" +) + +// SurfaceOrigin records HOW an entry came to be declared. It is carried on every +// entry so a stricter tier can weigh the registers differently without +// re-resolving them. +type SurfaceOrigin string + +const ( + // OriginConvention — found by auto-discovery under a well-known directory. + OriginConvention SurfaceOrigin = "convention" + // OriginManifest — named by an explicit plugin.json key. + OriginManifest SurfaceOrigin = "manifest" + // OriginHookCommand — referenced by a hook's command string. + OriginHookCommand SurfaceOrigin = "hook-command" +) + +// SurfaceRequirement says who is expected to supply an entry's path. +type SurfaceRequirement string + +const ( + // RequirePayload — the payload must carry it; absence is a failed install. + RequirePayload SurfaceRequirement = "payload" + // RequireInstalled — the install supplies it, so the payload never carries + // it and its absence proves nothing. + RequireInstalled SurfaceRequirement = "installed" +) + +// SurfaceEntry is one declared piece of the installable surface. +type SurfaceEntry struct { + Kind SurfaceKind `json:"kind"` + Path string `json:"path"` + Origin SurfaceOrigin `json:"origin"` + Requirement SurfaceRequirement `json:"requirement"` + // DeclaredAs is the raw declaration this entry was expanded from, when it + // differs from Path (a directory declaration, a hook command string). + DeclaredAs string `json:"declared_as,omitempty"` + // Reason explains a requirement other than RequirePayload. + Reason string `json:"reason,omitempty"` +} + +// SourceKind classifies a marketplace entry's source. +type SourceKind string + +const ( + // SourceLocal — a path inside the payload; resolvable offline. + SourceLocal SourceKind = "local" + // SourceExternal — a remote or non-path source; resolving it needs the + // network, so no offline gate asserts against it. + SourceExternal SourceKind = "external" + // SourceMissing — the entry declares no source at all. + SourceMissing SourceKind = "missing" +) + +// MarketplaceEntry is one plugin listing in the marketplace manifest. +type MarketplaceEntry struct { + Name string `json:"name"` + Source string `json:"source,omitempty"` + SourceKind SourceKind `json:"source_kind"` + // Root is the payload-relative plugin root a local source resolves to; the + // empty string is the payload root itself (adr-28: the single repo is its + // own marketplace, so the canonical source is "./"). + Root string `json:"root"` +} + +// InstallSurface is everything a payload declares about what installing it +// would register. +type InstallSurface struct { + PluginName string `json:"plugin_name"` + Marketplace []MarketplaceEntry `json:"marketplace"` + Entries []SurfaceEntry `json:"entries"` +} + +// conventionRules are the auto-discovery roots a harness loads without any +// manifest declaration. match receives the payload-relative path, so a rule can +// pin depth: agents are a FLAT glob (iss-110) while commands nest to namespace +// themselves. +var conventionRules = []struct { + kind SurfaceKind + root string + match func(rel string) bool +}{ + {SurfaceCommand, "commands", func(rel string) bool { return strings.HasSuffix(rel, ".md") }}, + {SurfaceAgent, "agents", func(rel string) bool { + return strings.HasSuffix(rel, ".md") && strings.Count(rel, "/") == 1 + }}, + {SurfaceSkill, "skills", func(rel string) bool { + return path.Base(rel) == "SKILL.md" && strings.Count(rel, "/") == 2 + }}, + {SurfaceHook, "hooks", func(rel string) bool { return rel == "hooks/hooks.json" }}, +} + +// declarationKeys are the plugin.json keys that name additional surface. +var declarationKeys = []struct { + key string + kind SurfaceKind +}{ + {"commands", SurfaceCommand}, + {"agents", SurfaceAgent}, + {"skills", SurfaceSkill}, + {"hooks", SurfaceHook}, +} + +// ResolveInstallSurface returns everything tree declares: the plugin's name, the +// marketplace listings with their sources resolved, and the union of the +// convention and manifest surface entries. +// +// It returns an error only when a manifest cannot be read or parsed at all — a +// payload whose declarations cannot even be enumerated. Everything else, +// including a declaration pointing at nothing, is DATA: it becomes an entry, and +// judging it is the assertion tier's job, not resolution's. +func ResolveInstallSurface(tree PayloadTree) (InstallSurface, error) { + var surface InstallSurface + + plugin, err := readManifest(tree, pluginManifestFile) + if err != nil { + return surface, err + } + surface.PluginName, _ = plugin["name"].(string) + + market, err := readManifest(tree, marketplaceFile) + if err != nil { + return surface, err + } + surface.Marketplace, err = resolveMarketplace(market) + if err != nil { + return surface, err + } + + entries := conventionEntries(tree) + entries = append(entries, manifestEntries(tree, plugin)...) + entries = append(entries, hookCommandEntries(tree, plugin, surface.PluginName, entries)...) + surface.Entries = dedupeEntries(entries) + return surface, nil +} + +// readManifest reads and decodes one JSON manifest object from the payload. +func readManifest(tree PayloadTree, rel string) (map[string]any, error) { + data, err := tree.Read(rel) + if err != nil { + return nil, fmt.Errorf("%s is not readable in the payload: %w", rel, err) + } + var doc map[string]any + if err := json.Unmarshal(data, &doc); err != nil { + return nil, fmt.Errorf("%s does not parse as a JSON object: %w", rel, err) + } + return doc, nil +} + +// resolveMarketplace classifies every plugin listing's source. A source is local +// only when it is an explicit relative path; anything else (a remote object, a +// shorthand, a URL) needs the network, and an offline gate must record that +// rather than pretend to have checked it. +func resolveMarketplace(market map[string]any) ([]MarketplaceEntry, error) { + raw, ok := market["plugins"].([]any) + if !ok { + return nil, fmt.Errorf("%s declares no plugins array", marketplaceFile) + } + out := make([]MarketplaceEntry, 0, len(raw)) + for i, item := range raw { + obj, ok := item.(map[string]any) + if !ok { + return nil, fmt.Errorf("%s: plugins[%d] is not an object", marketplaceFile, i) + } + entry := MarketplaceEntry{SourceKind: SourceMissing} + entry.Name, _ = obj["name"].(string) + switch src := obj["source"].(type) { + case string: + entry.Source = src + switch { + case src == "." || src == "./": + entry.SourceKind, entry.Root = SourceLocal, "" + case strings.HasPrefix(src, "./") || strings.HasPrefix(src, "../"): + entry.SourceKind, entry.Root = SourceLocal, normaliseDeclared(src) + default: + entry.SourceKind = SourceExternal + } + case nil: + // left as SourceMissing + default: + entry.SourceKind = SourceExternal + } + out = append(out, entry) + } + return out, nil +} + +// conventionEntries enumerates the auto-discovered surface. +func conventionEntries(tree PayloadTree) []SurfaceEntry { + var out []SurfaceEntry + for _, rule := range conventionRules { + for _, rel := range tree.List(rule.root) { + if rule.match(rel) { + out = append(out, SurfaceEntry{ + Kind: rule.kind, Path: rel, + Origin: OriginConvention, Requirement: RequirePayload, + }) + } + } + } + return out +} + +// manifestEntries expands the explicit declaration keys. +func manifestEntries(tree PayloadTree, plugin map[string]any) []SurfaceEntry { + var out []SurfaceEntry + for _, dk := range declarationKeys { + for _, decl := range declaredPaths(plugin[dk.key]) { + out = append(out, expandDeclared(tree, dk.kind, decl)...) + } + } + return out +} + +// declaredPaths flattens one declaration value into the paths it names. A string +// is a path; an array is a list of them; an object is an inline definition, +// which names a path only through a member's "source". +func declaredPaths(v any) []string { + switch d := v.(type) { + case string: + return []string{d} + case []any: + var out []string + for _, item := range d { + out = append(out, declaredPaths(item)...) + } + return out + case map[string]any: + var out []string + for _, member := range d { + obj, ok := member.(map[string]any) + if !ok { + continue + } + if src, ok := obj["source"].(string); ok { + out = append(out, src) + } + } + sort.Strings(out) // map iteration is unordered; entries must be stable + return out + default: + return nil + } +} + +// expandDeclared turns one declaration into entries: the file it names, or every +// file beneath the directory it names. +// +// A declaration that matches NOTHING in the payload still yields an entry at its +// own path. Dropping it would turn the loudest failure this gate exists to catch +// — a manifest pointing at a file that does not ship — into a silent pass. +func expandDeclared(tree PayloadTree, kind SurfaceKind, decl string) []SurfaceEntry { + norm := normaliseDeclared(decl) + if tree.Has(norm) { + return []SurfaceEntry{{Kind: kind, Path: norm, Origin: OriginManifest, Requirement: RequirePayload}} + } + if files := tree.List(norm); len(files) > 0 { + out := make([]SurfaceEntry, 0, len(files)) + for _, rel := range files { + out = append(out, SurfaceEntry{ + Kind: kind, Path: rel, Origin: OriginManifest, + Requirement: RequirePayload, DeclaredAs: decl, + }) + } + return out + } + return []SurfaceEntry{{Kind: kind, Path: norm, Origin: OriginManifest, Requirement: RequirePayload}} +} + +// normaliseDeclared makes a declaration comparable with a payload-relative path. +// It does NOT neutralise an escaping path: "../x" stays "../x" so the entry +// survives and fails its existence check by name, rather than being silently +// rewritten into something that might exist. +func normaliseDeclared(decl string) string { + d := strings.TrimSpace(decl) + d = strings.TrimPrefix(d, "./") + if d == "" { + return d + } + return path.Clean(d) +} + +// hookCommandEntries resolves the payload files a hook's command string invokes. +// +// This is the class of break an existence check over declared files alone cannot +// see: a hook that runs a script the payload does not carry installs cleanly and +// then fails at runtime. Only the `$CLAUDE_PLUGIN_ROOT`-rooted references are +// resolvable — anything else is a PATH lookup on the user's machine, which no +// release gate can assert. +func hookCommandEntries(tree PayloadTree, plugin map[string]any, pluginName string, found []SurfaceEntry) []SurfaceEntry { + var docs []any + if len(plugin) > 0 { + docs = append(docs, plugin["hooks"]) + } + for _, e := range found { + if e.Kind != SurfaceHook || e.Requirement != RequirePayload { + continue + } + data, err := tree.Read(e.Path) + if err != nil { + continue // absence is the existence check's finding, not this one's + } + var doc any + if json.Unmarshal(data, &doc) == nil { + docs = append(docs, doc) + } + } + + var out []SurfaceEntry + for _, doc := range docs { + for _, command := range collectCommandStrings(doc) { + for _, ref := range pluginRootRefs(command) { + entry := SurfaceEntry{ + Kind: SurfaceHook, Path: ref, Origin: OriginHookCommand, + Requirement: RequirePayload, DeclaredAs: command, + } + if ref == pluginName && pluginName != "" { + entry.Requirement = RequireInstalled + entry.Reason = "the plugin executable is supplied by the install, not carried in the payload" + } + out = append(out, entry) + } + } + } + return out +} + +// collectCommandStrings walks a decoded hooks document and returns every +// "command" string in it, at any depth. Walking generically rather than +// following the hooks schema means a future event name or nesting level costs +// this gate nothing. +func collectCommandStrings(doc any) []string { + var out []string + var walk func(any) + walk = func(v any) { + switch t := v.(type) { + case map[string]any: + keys := make([]string, 0, len(t)) + for k := range t { + keys = append(keys, k) + } + sort.Strings(keys) // map iteration is unordered; entries must be stable + for _, k := range keys { + if k == "command" { + if s, ok := t[k].(string); ok { + out = append(out, s) + continue + } + } + walk(t[k]) + } + case []any: + for _, item := range t { + walk(item) + } + } + } + walk(doc) + return out +} + +// pluginRootRefs extracts every path a command string reads out of the plugin +// root, in either `$CLAUDE_PLUGIN_ROOT/x` or `${CLAUDE_PLUGIN_ROOT}/x` form. +// +// It deliberately does not emulate a shell: the marker plus a literal suffix is +// the whole contract a hook command needs to name a shipped file, and a path +// assembled at runtime is not knowable at release time. A reference this misses +// yields no entry, never a false failure. +func pluginRootRefs(command string) []string { + const name = "CLAUDE_PLUGIN_ROOT" + var out []string + for i := 0; i < len(command); { + j := strings.Index(command[i:], name) + if j < 0 { + break + } + start := i + j + i = start + len(name) + if !dollarRooted(command, start) { + continue + } + rest := command[i:] + rest = strings.TrimPrefix(rest, "}") + if !strings.HasPrefix(rest, "/") { + continue + } + ref := rest[1:] + if k := strings.IndexAny(ref, " \t\"';)|&"); k >= 0 { + ref = ref[:k] + } + if ref = normaliseDeclared(ref); ref != "" { + out = append(out, ref) + } + } + return out +} + +// dollarRooted reports whether the variable name at start is an actual expansion +// (`$NAME` or `${NAME}`) rather than the literal word appearing in prose. +func dollarRooted(command string, start int) bool { + if start >= 1 && command[start-1] == '$' { + return true + } + return start >= 2 && command[start-1] == '{' && command[start-2] == '$' +} + +// dedupeEntries collapses paths declared twice (a convention file also named +// explicitly) and sorts the result, so the entry list is a stable set that two +// PayloadTree implementations can be compared on. +func dedupeEntries(entries []SurfaceEntry) []SurfaceEntry { + sort.SliceStable(entries, func(i, j int) bool { + if entries[i].Kind != entries[j].Kind { + return entries[i].Kind < entries[j].Kind + } + return entries[i].Path < entries[j].Path + }) + out := entries[:0] + var lastKind SurfaceKind + var lastPath string + for i, e := range entries { + if i > 0 && e.Kind == lastKind && e.Path == lastPath { + continue + } + lastKind, lastPath = e.Kind, e.Path + out = append(out, e) + } + return out +} + +// --------------------------------------------------------------------------- +// PayloadTree implementations +// --------------------------------------------------------------------------- + +// bundleTree reads a payload that has not been materialised: the file list is +// the bundle's Included set and each read goes to the file's resolved source. +type bundleTree struct { + files map[string]string // logical path -> resolved on-disk path + sorted []string +} + +// NewBundleTree views a resolved bundle as a payload tree, so the light tier can +// assert against WHAT WOULD SHIP without writing anything. A file present in the +// working tree but excluded from the payload is absent here — which is exactly +// the bug this gate catches. +func NewBundleTree(b Bundle) PayloadTree { + t := &bundleTree{files: make(map[string]string, len(b.Included))} + for _, f := range b.Included { + t.files[f.LogicalPath] = f.ResolvedPath + } + t.sorted = make([]string, 0, len(t.files)) + for rel := range t.files { + t.sorted = append(t.sorted, rel) + } + sort.Strings(t.sorted) + return t +} + +func (t *bundleTree) Has(rel string) bool { + _, ok := t.files[rel] + return ok +} + +func (t *bundleTree) Read(rel string) ([]byte, error) { + resolved, ok := t.files[rel] + if !ok { + return nil, fs.ErrNotExist + } + return fsutil.ReadGuarded(resolved, maxSurfaceManifestBytes) +} + +func (t *bundleTree) List(dirRel string) []string { + return withinDir(t.sorted, dirRel) +} + +// dirTree reads a materialised payload directory — the shape itd-66's deep tier +// works from, since it runs its imports in a subprocess rooted at the rendered +// snapshot. +type dirTree struct{ root string } + +// NewDirTree views a rendered payload directory as a payload tree. +func NewDirTree(root string) PayloadTree { return &dirTree{root: root} } + +func (t *dirTree) resolve(rel string) (string, bool) { + if rel == "" || path.IsAbs(rel) || rel == ".." || strings.HasPrefix(rel, "../") { + return "", false + } + return filepath.Join(t.root, filepath.FromSlash(rel)), true +} + +func (t *dirTree) Has(rel string) bool { + abs, ok := t.resolve(rel) + if !ok { + return false + } + info, err := os.Stat(abs) + return err == nil && info.Mode().IsRegular() +} + +func (t *dirTree) Read(rel string) ([]byte, error) { + abs, ok := t.resolve(rel) + if !ok { + return nil, fs.ErrNotExist + } + return fsutil.ReadGuarded(abs, maxSurfaceManifestBytes) +} + +func (t *dirTree) List(dirRel string) []string { + abs, ok := t.resolve(dirRel) + if !ok { + return nil + } + var out []string + _ = filepath.WalkDir(abs, func(p string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() || !d.Type().IsRegular() { + return nil //nolint:nilerr // an unreadable branch is "not present", not a gate fault + } + rel, rerr := filepath.Rel(t.root, p) + if rerr != nil { + return nil + } + out = append(out, filepath.ToSlash(rel)) + return nil + }) + sort.Strings(out) + return out +} + +// withinDir returns the members of a sorted path list that lie at or beneath +// dirRel. +func withinDir(sorted []string, dirRel string) []string { + if dirRel == "" { + return nil + } + prefix := dirRel + "/" + var out []string + for _, rel := range sorted { + if rel == dirRel || strings.HasPrefix(rel, prefix) { + out = append(out, rel) + } + } + return out +} diff --git a/internal/core/launch/installsurface_test.go b/internal/core/launch/installsurface_test.go new file mode 100644 index 0000000..21a42e6 --- /dev/null +++ b/internal/core/launch/installsurface_test.go @@ -0,0 +1,283 @@ +package launch + +import ( + "path/filepath" + "reflect" + "testing" + "time" +) + +// bundleTreeFor resolves root's payload the way a ship would and returns the +// read side of it, so every resolution test runs against WHAT WOULD SHIP rather +// than against the working tree. +func bundleTreeFor(t *testing.T, root string) PayloadTree { + t.Helper() + bundle, err := ResolveBundle(root, nil) + if err != nil { + t.Fatalf("resolve the payload bundle: %v", err) + } + return NewBundleTree(bundle) +} + +// findEntry returns the resolved entry at kind/path, or false. +func findEntry(s InstallSurface, kind SurfaceKind, path string) (SurfaceEntry, bool) { + for _, e := range s.Entries { + if e.Kind == kind && e.Path == path { + return e, true + } + } + return SurfaceEntry{}, false +} + +// TestResolveInstallSurfaceDeclarations pins what "the declared surface" IS: the +// UNION of the manifest's explicit declarations and the auto-discovery +// conventions, because the plugin-manifest schema defines every explicit key as +// declaring entries "in addition to those in the / directory". Each case +// fixes one rule of that union so the deep tier inherits the same list. +func TestResolveInstallSurfaceDeclarations(t *testing.T) { + type want struct { + kind SurfaceKind + path string + origin SurfaceOrigin + requirement SurfaceRequirement + } + cases := []struct { + name string + files map[string]string + want []want + absent []want + wantErr bool + }{ + { + name: "convention discovery finds the auto-loaded surfaces", + files: map[string]string{ + "commands/a.md": "cmd\n", + "commands/ns/b.md": "namespaced cmd\n", + "commands/README.md": "doc\n", + "agents/x.md": "agent\n", + "agents/README.md": "doc\n", + "agents/x/fixture.txt": "not an agent\n", + "skills/s/SKILL.md": "skill\n", + "hooks/hooks.json": `{"hooks": {}}`, + }, + want: []want{ + {SurfaceCommand, "commands/a.md", OriginConvention, RequirePayload}, + {SurfaceCommand, "commands/ns/b.md", OriginConvention, RequirePayload}, + {SurfaceCommand, "commands/README.md", OriginConvention, RequirePayload}, + {SurfaceAgent, "agents/x.md", OriginConvention, RequirePayload}, + // iss-110: the loader globs agents/*.md, so a README in that + // directory IS registered as an agent. The resolver reports what + // the harness registers; filtering it here would hide the very + // defect iss-110 tracks. + {SurfaceAgent, "agents/README.md", OriginConvention, RequirePayload}, + {SurfaceSkill, "skills/s/SKILL.md", OriginConvention, RequirePayload}, + {SurfaceHook, "hooks/hooks.json", OriginConvention, RequirePayload}, + }, + absent: []want{{SurfaceAgent, "agents/x/fixture.txt", OriginConvention, RequirePayload}}, + }, + { + name: "an explicit declaration supplements, never replaces, the convention", + files: map[string]string{ + "plugin_extra": `"commands": "./extra/one.md", "agents": ["./extra/two.md"]`, + "commands/a.md": "cmd\n", + "extra/one.md": "extra cmd\n", + "extra/two.md": "extra agent\n", + "agents/keep.md": "agent\n", + }, + want: []want{ + {SurfaceCommand, "commands/a.md", OriginConvention, RequirePayload}, + {SurfaceCommand, "extra/one.md", OriginManifest, RequirePayload}, + {SurfaceAgent, "agents/keep.md", OriginConvention, RequirePayload}, + {SurfaceAgent, "extra/two.md", OriginManifest, RequirePayload}, + }, + }, + { + name: "a declaration naming a directory expands to the files beneath it", + files: map[string]string{ + "plugin_extra": `"commands": "./extra"`, + "extra/one.md": "cmd\n", + "extra/deep/two.md": "cmd\n", + }, + want: []want{ + {SurfaceCommand, "extra/one.md", OriginManifest, RequirePayload}, + {SurfaceCommand, "extra/deep/two.md", OriginManifest, RequirePayload}, + }, + }, + { + name: "a declaration naming nothing in the payload survives as its own entry", + files: map[string]string{"plugin_extra": `"commands": "./commands/ghost.md"`}, + want: []want{{SurfaceCommand, "commands/ghost.md", OriginManifest, RequirePayload}}, + }, + { + name: "a declaration escaping the payload survives as its own entry", + files: map[string]string{"plugin_extra": `"agents": "../outside.md"`}, + want: []want{{SurfaceAgent, "../outside.md", OriginManifest, RequirePayload}}, + }, + { + name: "a hook command referencing a payload file is a required entry", + files: map[string]string{ + "hooks/hooks.json": `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "bash \"$CLAUDE_PLUGIN_ROOT/scripts/go.sh\""}]}]}}`, + "scripts/go.sh": "#!/bin/sh\n", + }, + want: []want{ + {SurfaceHook, "hooks/hooks.json", OriginConvention, RequirePayload}, + {SurfaceHook, "scripts/go.sh", OriginHookCommand, RequirePayload}, + }, + }, + { + name: "a hook command referencing the plugin executable is install-supplied", + files: map[string]string{ + "hooks/hooks.json": `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "\"${CLAUDE_PLUGIN_ROOT}/abcd\" hook session-start"}]}]}}`, + }, + want: []want{{SurfaceHook, "abcd", OriginHookCommand, RequireInstalled}}, + }, + { + name: "an unparseable plugin manifest is a resolution error", + files: map[string]string{"plugin_broken": "yes"}, + wantErr: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + writeSurfaceFixture(t, root, tc.files) + surface, err := ResolveInstallSurface(bundleTreeFor(t, root)) + if tc.wantErr { + if err == nil { + t.Fatalf("expected a resolution error, got surface %+v", surface) + } + return + } + if err != nil { + t.Fatalf("resolve: %v", err) + } + for _, w := range tc.want { + got, ok := findEntry(surface, w.kind, w.path) + if !ok { + t.Errorf("%s %s was not resolved; entries=%+v", w.kind, w.path, surface.Entries) + continue + } + if got.Origin != w.origin { + t.Errorf("%s %s: origin %q, want %q", w.kind, w.path, got.Origin, w.origin) + } + if got.Requirement != w.requirement { + t.Errorf("%s %s: requirement %q, want %q", w.kind, w.path, got.Requirement, w.requirement) + } + } + for _, w := range tc.absent { + if got, ok := findEntry(surface, w.kind, w.path); ok { + t.Errorf("%s %s must NOT be resolved, got %+v", w.kind, w.path, got) + } + } + }) + } +} + +// writeSurfaceFixture writes a shippable plugin payload: a launch-payload config +// that includes everything the case names, a plugin.json (optionally carrying +// extra declaration keys via the "plugin_extra" pseudo-file) and a marketplace +// listing that sources the payload root. +func writeSurfaceFixture(t *testing.T, root string, files map[string]string) { + t.Helper() + extra := files["plugin_extra"] + broken := files["plugin_broken"] + includes := map[string]struct{}{".claude-plugin": {}} + for rel, content := range files { + if rel == "plugin_extra" || rel == "plugin_broken" { + continue + } + writeFile(t, root, rel, content) + includes[firstSegment(rel)] = struct{}{} + } + list := "" + for inc := range includes { + if list != "" { + list += ", " + } + list += `"` + inc + `"` + } + writeFile(t, root, ".abcd/config/launch-payload.json", `{"includes": [`+list+`]}`) + plugin := `{"name": "abcd"` + if extra != "" { + plugin += ", " + extra + } + plugin += "}" + if broken != "" { + plugin = "{not json" + } + writeFile(t, root, ".claude-plugin/plugin.json", plugin) + writeFile(t, root, ".claude-plugin/marketplace.json", + `{"name": "abcd-marketplace", "plugins": [{"name": "abcd", "source": "./"}]}`) +} + +// TestResolveInstallSurfaceCommittedPayload runs the resolver over THIS +// repository's real payload, so the seam is pinned against the surface that +// actually ships rather than only against fixtures. +func TestResolveInstallSurfaceCommittedPayload(t *testing.T) { + root := repoRootForTest(t) + surface, err := ResolveInstallSurface(bundleTreeFor(t, root)) + if err != nil { + t.Fatalf("resolve the committed payload's surface: %v", err) + } + if surface.PluginName != "abcd" { + t.Errorf("plugin name %q, want abcd", surface.PluginName) + } + if len(surface.Marketplace) != 1 { + t.Fatalf("expected exactly one marketplace plugin entry, got %+v", surface.Marketplace) + } + mp := surface.Marketplace[0] + if mp.Name != "abcd" || mp.SourceKind != SourceLocal || mp.Root != "" { + t.Errorf("adr-28 says the single repo is its own marketplace: got %+v", mp) + } + for _, want := range []struct { + kind SurfaceKind + path string + }{ + {SurfaceCommand, "commands/abcd/launch.md"}, + {SurfaceAgent, "agents/ruthless-reviewer.md"}, + {SurfaceHook, "hooks/hooks.json"}, + } { + if _, ok := findEntry(surface, want.kind, want.path); !ok { + t.Errorf("the committed payload must declare %s %s", want.kind, want.path) + } + } + // The hooks manifest invokes the plugin executable, which the install + // supplies and the payload never carries. + bin, ok := findEntry(surface, SurfaceHook, "abcd") + if !ok || bin.Requirement != RequireInstalled { + t.Errorf("the plugin executable must resolve as install-supplied, got %+v (ok=%v)", bin, ok) + } +} + +// TestPayloadTreeImplementationsResolveIdentically is the drop-in-upgrade proof: +// resolution over the RESOLVED BUNDLE (what the light tier reads) and over a +// MATERIALISED payload directory (what itd-66's deep tier reads) must produce +// the identical entry list. If these ever diverge, the two tiers disagree about +// what the surface is — which is precisely the divergence this seam exists to +// prevent. +func TestPayloadTreeImplementationsResolveIdentically(t *testing.T) { + root := repoRootForTest(t) + dest := filepath.Join(t.TempDir(), "payload") + if _, err := RenderPayload(PayloadRenderRequest{ + RepoRoot: root, Dest: dest, Version: "9.9.9", + Entry: ChangelogEntry{Tier: "patch", Reason: "seam parity", Date: time.Now(), SourceSHA: "deadbeef"}, + }); err != nil { + t.Fatalf("render the payload: %v", err) + } + + fromBundle, err := ResolveInstallSurface(bundleTreeFor(t, root)) + if err != nil { + t.Fatalf("resolve from the bundle: %v", err) + } + fromDir, err := ResolveInstallSurface(NewDirTree(dest)) + if err != nil { + t.Fatalf("resolve from the rendered directory: %v", err) + } + if !reflect.DeepEqual(fromBundle.Entries, fromDir.Entries) { + t.Errorf("bundle and directory resolution disagree:\n bundle=%+v\n dir=%+v", fromBundle.Entries, fromDir.Entries) + } + if !reflect.DeepEqual(fromBundle.Marketplace, fromDir.Marketplace) { + t.Errorf("marketplace resolution disagrees:\n bundle=%+v\n dir=%+v", fromBundle.Marketplace, fromDir.Marketplace) + } +} diff --git a/internal/core/launch/lockstep.go b/internal/core/launch/lockstep.go index d135189..31029ee 100644 --- a/internal/core/launch/lockstep.go +++ b/internal/core/launch/lockstep.go @@ -132,7 +132,7 @@ func resolvePointer(doc any, pointer string) (any, bool) { } cur := doc for _, raw := range strings.Split(pointer, "/")[1:] { - tok := strings.ReplaceAll(strings.ReplaceAll(raw, "~1", "/"), "~0", "~") + tok := unescapePointerToken(raw) switch c := cur.(type) { case map[string]any: v, ok := c[tok] @@ -153,6 +153,14 @@ func resolvePointer(doc any, pointer string) (any, bool) { return cur, true } +// unescapePointerToken decodes one RFC-6901 reference token (~1 → /, ~0 → ~). +// The order matters and is the spec's: decoding ~0 first would turn "~01" into +// "~1" and then into "/". It is shared by the pointer reader here and the +// pointer writer in render.go so the two can never disagree about a token. +func unescapePointerToken(raw string) string { + return strings.ReplaceAll(strings.ReplaceAll(raw, "~1", "/"), "~0", "~") +} + func atoiIndex(tok string) (int, bool) { if tok == "" { return 0, false diff --git a/internal/core/launch/lockstep_repo_test.go b/internal/core/launch/lockstep_repo_test.go new file mode 100644 index 0000000..b6dfb4d --- /dev/null +++ b/internal/core/launch/lockstep_repo_test.go @@ -0,0 +1,46 @@ +package launch + +import ( + "path/filepath" + "testing" +) + +// TestCommittedTreeSatisfiesDevPolarity is the adr-19 detector for THIS +// repository, not a synthetic fixture: the committed manifests must carry no +// version key, and the committed version-location contract must be readable +// enough to say where that key would be. +// +// It is pinned here rather than left to the synthetic lockstep tests because the +// two halves fail in opposite directions and only the real tree proves both at +// once: an unreadable contract (no version-location.json) makes the whole gate +// inert, and a version key added to a working-tree manifest silently breaks the +// premise that the version is an output of the release cut. +func TestCommittedTreeSatisfiesDevPolarity(t *testing.T) { + root := repoRootForTest(t) + res := CheckLockstep(TreeDev, root, filepath.Join(root, versionLocationRelPath)) + if res.Unreadable { + t.Fatalf("the committed version-location contract must be readable, got %s", res.Detail) + } + if !res.OK || res.ExitCode != 0 { + t.Fatalf("adr-19: the committed tree must be version-ABSENT, got drifts %v", res.Drifts) + } +} + +// TestCommittedContractSelectsPluginManifest pins the version location the +// render writes to. A silent relocation would leave the render stamping a +// version somewhere the harness never reads, and the lockstep check would still +// pass because it reads the same moved pointer. +func TestCommittedContractSelectsPluginManifest(t *testing.T) { + root := repoRootForTest(t) + decision, err := loadJSON(filepath.Join(root, versionLocationRelPath)) + if err != nil { + t.Fatalf("read the committed version-location contract: %v", err) + } + path, ptr, verr := validateVersionLocation(decision) + if verr != "" { + t.Fatalf("the committed contract must validate, got %s", verr) + } + if path != ".claude-plugin/plugin.json" || ptr != "/version" { + t.Errorf("expected adr-19's ACCEPT outcome (.claude-plugin/plugin.json /version), got %q %q", path, ptr) + } +} diff --git a/internal/core/launch/lockstep_test.go b/internal/core/launch/lockstep_test.go index a8a0026..1942227 100644 --- a/internal/core/launch/lockstep_test.go +++ b/internal/core/launch/lockstep_test.go @@ -17,7 +17,10 @@ func writeLockstepTree(t *testing.T, root, primaryVer, marketVer, changelogVer s } else { writeFile(t, root, ".claude-plugin/plugin.json", `{"name": "abcd", "version": "`+primaryVer+`"}`) } - mk := `{"plugins": [{"name": "abcd"` + // source is not part of the lockstep contract, but a listing without one is + // not a well-formed marketplace entry — the installability smoke reads the + // same manifest, so the fixture carries the canonical adr-28 "./". + mk := `{"plugins": [{"name": "abcd", "source": "./"` if marketVer != "" { mk += `, "version": "` + marketVer + `"` } diff --git a/internal/core/launch/readme_install_test.go b/internal/core/launch/readme_install_test.go new file mode 100644 index 0000000..6bab4cc --- /dev/null +++ b/internal/core/launch/readme_install_test.go @@ -0,0 +1,98 @@ +package launch + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestREADMEDocumentsTheInstallAndUpdatePath is the itd-67 acceptance detector +// for the documented distribution path, run against THIS repository rather than +// a synthetic fixture. +// +// It is pinned to the real tree because every name in the documented commands is +// owned by a different file: the marketplace slug by the module path (and so by +// the git remote), the marketplace and plugin names by +// .claude-plugin/marketplace.json. Prose repeating those names drifts silently — +// a renamed marketplace, or an install line naming a repository that is not this +// one, still reads perfectly while sending users somewhere that does not resolve. +// Deriving each expected string from its owning file makes the README fail with +// the rename instead of after it. +func TestREADMEDocumentsTheInstallAndUpdatePath(t *testing.T) { + root := repoRootForTest(t) + + readme, err := os.ReadFile(filepath.Join(root, "README.md")) + if err != nil { + t.Fatalf("read the committed README: %v", err) + } + prose := string(readme) + + slug := moduleRepoSlug(t, root) + marketplace, plugin := marketplaceNames(t, root) + + cases := []struct { + name string + want string + }{ + {"marketplace add names this repository", "/plugin marketplace add " + slug}, + {"install names the plugin in its marketplace", "/plugin install " + plugin + "@" + marketplace}, + {"update names the plugin", "/plugin update " + plugin}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if !strings.Contains(prose, tc.want) { + t.Errorf("README does not document %q", tc.want) + } + }) + } +} + +// moduleRepoSlug derives the "/" a marketplace add must name from +// go.mod's module path. go.mod is the one place the repository identity is +// already load-bearing for the build, so it cannot drift unnoticed. +func moduleRepoSlug(t *testing.T, root string) string { + t.Helper() + gomod, err := os.ReadFile(filepath.Join(root, "go.mod")) + if err != nil { + t.Fatalf("read go.mod: %v", err) + } + for _, line := range strings.Split(string(gomod), "\n") { + path, ok := strings.CutPrefix(strings.TrimSpace(line), "module ") + if !ok { + continue + } + parts := strings.Split(strings.TrimSpace(path), "/") + if len(parts) < 3 { + t.Fatalf("module path %q is not host/owner/repo", path) + } + return strings.Join(parts[len(parts)-2:], "/") + } + t.Fatal("go.mod declares no module path") + return "" +} + +// marketplaceNames reads the marketplace name and its single plugin name from +// the committed manifest, so the documented install line is checked against the +// names the harness actually resolves. +func marketplaceNames(t *testing.T, root string) (marketplace, plugin string) { + t.Helper() + raw, err := os.ReadFile(filepath.Join(root, ".claude-plugin", "marketplace.json")) + if err != nil { + t.Fatalf("read the committed marketplace manifest: %v", err) + } + var manifest struct { + Name string `json:"name"` + Plugins []struct { + Name string `json:"name"` + } `json:"plugins"` + } + if err := json.Unmarshal(raw, &manifest); err != nil { + t.Fatalf("parse the committed marketplace manifest: %v", err) + } + if manifest.Name == "" || len(manifest.Plugins) == 0 || manifest.Plugins[0].Name == "" { + t.Fatalf("marketplace manifest names no marketplace or plugin: %+v", manifest) + } + return manifest.Name, manifest.Plugins[0].Name +} diff --git a/internal/core/launch/render.go b/internal/core/launch/render.go new file mode 100644 index 0000000..a5f9f0a --- /dev/null +++ b/internal/core/launch/render.go @@ -0,0 +1,440 @@ +package launch + +// render.go — the single writer of a plugin version (adr-19, adr-20). +// +// The version is an OUTPUT of the release cut, not a fact about the source. The +// working tree's `.claude-plugin/*.json` therefore stay version-absent forever, +// and the derived version is stamped into the payload's COPIES of them, here, +// at ship time. Nothing else in the repository writes a version into a manifest; +// the lockstep checker beside this file only reads. +// +// The two polarities that make that safe are encoded in lockstep.go, and each is +// asserted where it belongs. The render proves TreePublic (keys present and +// agreeing) over its OWN OUTPUT, finishing with the public check, so a stamp +// that missed a pinned location is a refusal rather than a published half-state. +// TreeDev (keys absent) over the SOURCE tree is not the render's assertion: it +// is the launch gates' — DryRun and Ship run it on every preview — and +// lockstep_repo_test.go pins it over this repository's committed manifests. + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/REPPL/abcd-cli/internal/fsutil" +) + +// bumpTiers are the tiers adr-20's marketplace changelog entry admits. The +// render validates against this set rather than accepting any string, because a +// misspelt tier lands in a published manifest that no later check inspects. +var bumpTiers = map[string]struct{}{"patch": {}, "minor": {}, "major": {}} + +// ChangelogEntry is the marketplace plugin entry's per-release record (adr-20 +// R3), minus its version. +// +// The version is deliberately NOT a field: the render writes the one derived +// version into this entry and into both version pointers from a single input, so +// the entry cannot disagree with the manifest it travels in. Everything here is +// supplied by the caller — a core that read the clock or the git HEAD itself +// would put two unpinnable inputs inside a durable release artefact. +type ChangelogEntry struct { + // Tier is the SemVer component the cut moved: patch, minor or major. + Tier string + // Reason is the human sentence explaining the bump, e.g. "additive itd-67 shipped". + Reason string + // Date is the release date; only its calendar day is recorded. + Date time.Time + // SourceSHA is the commit the release was cut from. + SourceSHA string +} + +// PayloadRenderRequest is the input to RenderPayload. +type PayloadRenderRequest struct { + // RepoRoot is the source tree the payload is cut from. It is read only. + RepoRoot string + // Dest is the staging directory the payload is written to. It must live + // OUTSIDE RepoRoot and be empty or absent. + Dest string + // Version is the derived release version, strict SemVer with no leading "v". + // It is an input rather than something read back out of the tree — the tree + // has no version to read, which is the whole point of adr-19. + Version string + // Entry is the marketplace changelog record for this release. + Entry ChangelogEntry +} + +// PayloadRenderResult is a completed render. +type PayloadRenderResult struct { + // Dest is the staging directory the payload was written to. + Dest string `json:"dest"` + // Version is the version stamped at every pinned location. + Version string `json:"version"` + // Bundle is the resolution the payload was written from, so a caller can + // render the same file classification a dry-run would. + Bundle Bundle `json:"bundle"` + // Files counts the payload files written. + Files int `json:"files"` + // Manifests names the manifests that were version-stamped, repo-relative. + Manifests []string `json:"manifests"` + // Lockstep is the public check over the rendered payload — the proof that + // what was written is internally consistent. + Lockstep LockstepResult `json:"lockstep"` + // Smoke is the light installability check over the rendered payload — the + // proof that what was written would actually install. + Smoke SmokeReport `json:"smoke"` +} + +// ErrPayloadDrift reports that the rendered payload failed the public lockstep +// check. It is its own error because it means the render itself is wrong (a +// pinned location was missed), not that the caller's input was. +var ErrPayloadDrift = errors.New("the rendered payload failed the public lockstep check") + +// ErrPayloadUninstallable reports that the rendered payload declares a surface +// it does not carry. It is distinct from ErrPayloadDrift because the fault is +// the payload's CONTENTS, not the version stamp: the manifests agree perfectly +// and the plugin still would not install. +var ErrPayloadUninstallable = errors.New("the rendered payload failed the installability smoke") + +// payloadLockstep is the render's self-check over its own output. It is a +// package var for the same reason ignoreChecker is: the drift branch guards +// against a PINNED LOCATION THE RENDER FORGOT TO STAMP, and while the render and +// the checker read the same adr-20 list that state cannot be provoked from the +// outside — so substituting a checker that reports drift is the only way to +// prove the backstop refuses rather than publishes. +var payloadLockstep = CheckLockstep + +// PayloadPrecheck is everything a render refuses on WITHOUT knowing the version: +// the resolved locations, the payload the render would write, and the light +// installability verdict over it. +// +// It exists as a named result because the ship verb writes a durable release +// record (the dated CHANGELOG heading) before it has a version to stamp, and a +// render that refuses after that write leaves a release in flight that can never +// be retried. Separating the version-free half lets the caller prove the render +// will be ACCEPTED before it writes anything. +type PayloadPrecheck struct { + // Root is the symlink-resolved source tree. + Root string + // Dest is the symlink-resolved staging directory. Nothing is created here. + Dest string + // VersionLocationPath is the absolute path to the adr-19 contract. + VersionLocationPath string + // PrimaryPath and PrimaryPointer are the pinned primary version location the + // contract selected, repo-relative and RFC-6901. + PrimaryPath, PrimaryPointer string + // Bundle is the payload resolution the render would write from. + Bundle Bundle + // Smoke is the light installability tier over the RESOLVED bundle — the same + // assertions the render makes over its written output, made early enough to + // refuse before any durable write. + Smoke SmokeReport +} + +// PrecheckPayload resolves the release payload and runs every refusal a render +// makes that does not depend on the version. +// +// It performs ZERO writes — not even the destination directory — so a caller may +// run it speculatively and a refused cut leaves the filesystem exactly as it +// found it. RenderPayload runs it as its own first step, so the two can never +// disagree about what is refusable. +// +// It refuses when: dest overlaps repoRoot or is already populated; the +// version-location contract is unreadable or blocked (adr-19 — a blocked +// decision has no schema-valid place to write); the bundle carries a violation; +// either manifest is not in the payload at all; or the payload declares a +// surface it does not carry. +func PrecheckPayload(repoRoot, dest string) (PayloadPrecheck, error) { + var pre PayloadPrecheck + + pre.Root = resolveExistingPrefix(repoRoot) + pre.Dest = resolveExistingPrefix(dest) + if pre.Dest == "" || pre.Root == "" { + return pre, errors.New("both the repository root and the payload destination must be named") + } + if pre.Dest == pre.Root || strings.HasPrefix(pre.Dest, pre.Root+string(filepath.Separator)) { + return pre, errors.New("the payload destination is inside the repository — the payload is an output, not a tracked directory") + } + if strings.HasPrefix(pre.Root, pre.Dest+string(filepath.Separator)) { + return pre, errors.New("the payload destination contains the repository") + } + // An existing NON-directory is refused explicitly: DirHasEntries reads it as + // "no entries", and the render's own MkdirAll would then fail after the + // caller had already written the release record. + if info, err := os.Lstat(pre.Dest); err == nil && !info.IsDir() { + return pre, errors.New("the payload destination exists and is not a directory") + } + populated, err := fsutil.DirHasEntries(pre.Dest) + if err != nil { + return pre, err + } + if populated { + return pre, errors.New("the payload destination is not empty — refusing to render over existing content") + } + + // The contract says WHERE the version goes. It is read from the source tree + // (it is a decision artefact, never shipped) and it is the only thing that + // tells the render which manifest and pointer adr-19 selected. + pre.VersionLocationPath = filepath.Join(pre.Root, versionLocationRelPath) + decision, err := loadJSON(pre.VersionLocationPath) + if err != nil { + return pre, fmt.Errorf("version-location.json not readable: %w", err) + } + primaryPath, primaryPtr, verr := validateVersionLocation(decision) + if verr != "" { + return pre, errors.New(verr) + } + pre.PrimaryPath, pre.PrimaryPointer = primaryPath, primaryPtr + + bundle, err := ResolveBundle(pre.Root, nil) + if err != nil { + return pre, err + } + pre.Bundle = bundle + if bundle.HasViolation() { + return pre, fmt.Errorf("the payload carries %d rejected file(s); resolve them before rendering a release", len(bundle.Rejected)) + } + + included := make(map[string]struct{}, len(bundle.Included)) + for _, f := range bundle.Included { + included[f.LogicalPath] = struct{}{} + } + for _, rel := range []string{primaryPath, marketplaceFile} { + if _, ok := included[rel]; !ok { + return pre, fmt.Errorf("%s is not in the payload — a version stamped there would never ship", rel) + } + } + + // The smoke over the resolved bundle asserts the same declared surface the + // render's post-write smoke does, minus the version stamp — which adds keys + // and names no path. So a payload that would fail installability fails HERE, + // before a release record exists. + pre.Smoke = SmokeLight(NewBundleTree(bundle)) + if !pre.Smoke.OK { + return pre, fmt.Errorf("%w: %s", ErrPayloadUninstallable, strings.Join(smokeDetails(pre.Smoke), "; ")) + } + return pre, nil +} + +// RenderPayload writes the release payload to req.Dest with the derived version +// stamped into its manifests, and proves the result with CheckLockstep. +// +// It resolves the payload through the SAME ResolveBundle/LoadIncludes machinery +// the dry-run and ship gates use, so there is exactly one notion of "what +// ships"; the render adds only the version stamp on the way out. It never +// touches the source tree: every write lands under Dest. +// +// Everything refusable that does not need the version is delegated to +// PrecheckPayload, so a caller may run those refusals FIRST; on top of them this +// refuses when the version is not strict SemVer or the changelog entry is +// incomplete. +func RenderPayload(req PayloadRenderRequest) (PayloadRenderResult, error) { + res := PayloadRenderResult{Version: req.Version} + + if !IsStrictSemver(req.Version) { + return res, fmt.Errorf("the release version %q is not strict SemVer (major.minor.patch, no leading v)", req.Version) + } + if _, ok := bumpTiers[req.Entry.Tier]; !ok { + return res, fmt.Errorf("the changelog entry's bump tier %q is not one of patch, minor, major", req.Entry.Tier) + } + if req.Entry.Reason == "" || req.Entry.SourceSHA == "" || req.Entry.Date.IsZero() { + return res, errors.New("the changelog entry needs a reason, a source SHA and a date") + } + + pre, err := PrecheckPayload(req.RepoRoot, req.Dest) + if err != nil { + return res, err + } + dest, vlPath := pre.Dest, pre.VersionLocationPath + primaryPath, primaryPtr := pre.PrimaryPath, pre.PrimaryPointer + bundle := pre.Bundle + res.Dest = dest + res.Bundle = bundle + + if err := os.MkdirAll(dest, 0o755); err != nil { + return res, err + } + + for _, f := range bundle.Included { + if err := copyPayloadFile(dest, f); err != nil { + return res, err + } + } + res.Files = len(bundle.Included) + + if err := stampPointer(dest, primaryPath, primaryPtr, req.Version); err != nil { + return res, err + } + if err := stampMarketplace(dest, req); err != nil { + return res, err + } + res.Manifests = []string{primaryPath, marketplaceFile} + + res.Lockstep = payloadLockstep(TreePublic, dest, vlPath) + if !res.Lockstep.OK { + return res, fmt.Errorf("%w: %s", ErrPayloadDrift, strings.Join(lockstepDetail(res.Lockstep), "; ")) + } + + // The render is the only step that MATERIALISES a release artefact, so it is + // where "a missing declared path FAILS" has to bite: a dry-run previews, and + // nothing downstream of here re-reads the payload before it ships. The smoke + // runs over the WRITTEN payload rather than the resolved bundle so it judges + // the exact bytes a user would install, stamped manifests included. + res.Smoke = SmokeLight(NewDirTree(dest)) + if !res.Smoke.OK { + return res, fmt.Errorf("%w: %s", ErrPayloadUninstallable, strings.Join(smokeDetails(res.Smoke), "; ")) + } + return res, nil +} + +// smokeDetails flattens a failed smoke into the lines a refusal quotes, so an +// operator sees WHICH declared path is missing rather than a count. +func smokeDetails(report SmokeReport) []string { + out := make([]string, 0, len(report.Findings)) + for _, f := range report.Findings { + out = append(out, f.Detail) + } + return out +} + +// lockstepDetail flattens a failing lockstep result into the lines a refusal +// quotes, so an operator sees WHICH pinned location disagreed. +func lockstepDetail(res LockstepResult) []string { + if res.Unreadable { + return []string{res.Detail} + } + return res.Drifts +} + +// copyPayloadFile writes one resolved bundle file under dest, carrying the git +// mode the bundle recorded so an executable hook stays executable in the +// artefact. +func copyPayloadFile(dest string, f IncludedFile) error { + data, err := os.ReadFile(f.ResolvedPath) + if err != nil { + return err + } + perm := os.FileMode(0o644) + if f.GitMode == "100755" { + perm = 0o755 + } + return fsutil.WriteFileAtomic(filepath.Join(dest, filepath.FromSlash(f.LogicalPath)), data, perm) +} + +// stampMarketplace writes the two pinned marketplace locations — the published +// version and the changelog entry — from the one version input, so they cannot +// disagree with the primary or with each other. +func stampMarketplace(dest string, req PayloadRenderRequest) error { + return editManifest(dest, marketplaceFile, func(doc any) error { + if err := setPointer(doc, secondaryVersionPointer, req.Version); err != nil { + return err + } + return setPointer(doc, secondaryChangelogPtr, map[string]any{ + "version": req.Version, + "tier": req.Entry.Tier, + "reason": req.Entry.Reason, + "date": req.Entry.Date.UTC().Format("2006-01-02"), + "source_sha": req.Entry.SourceSHA, + }) + }) +} + +// stampPointer writes value at one RFC-6901 pointer in a payload manifest. +func stampPointer(dest, rel, pointer string, value any) error { + return editManifest(dest, rel, func(doc any) error { return setPointer(doc, pointer, value) }) +} + +// editManifest reads a manifest from the PAYLOAD (never the source tree), +// applies mutate to the decoded document and writes it back indented. Reading +// the payload copy rather than the original is what makes "the source tree is +// never mutated" structural rather than a convention a later edit could break. +func editManifest(dest, rel string, mutate func(any) error) error { + path := filepath.Join(dest, filepath.FromSlash(rel)) + doc, err := loadJSON(path) + if err != nil { + return fmt.Errorf("%s not readable in the payload: %w", rel, err) + } + if err := mutate(doc); err != nil { + return fmt.Errorf("%s: %w", rel, err) + } + data, err := json.MarshalIndent(doc, "", " ") + if err != nil { + return err + } + return fsutil.WriteFileAtomic(path, append(data, '\n'), 0o644) +} + +// setPointer writes value at an RFC-6901 pointer over a decoded JSON document. +// +// It is deliberately NOT a creating writer: every container on the way to the +// final token must already exist, and an array index must already be in range. +// A manifest that lacks the shape the pinned pointers describe is a manifest +// this render does not understand, and inventing the missing structure would +// publish a plausible-looking artefact no harness reads. +func setPointer(doc any, pointer string, value any) error { + if !strings.HasPrefix(pointer, "/") { + return fmt.Errorf("%q is not an RFC-6901 pointer", pointer) + } + tokens := strings.Split(pointer, "/")[1:] + cur := doc + for i, raw := range tokens { + tok := unescapePointerToken(raw) + last := i == len(tokens)-1 + switch c := cur.(type) { + case map[string]any: + if last { + c[tok] = value + return nil + } + next, ok := c[tok] + if !ok { + return fmt.Errorf("pointer %s: no %q to descend into", pointer, tok) + } + cur = next + case []any: + idx, ok := atoiIndex(tok) + if !ok || idx >= len(c) { + return fmt.Errorf("pointer %s: index %q is out of range", pointer, tok) + } + if last { + c[idx] = value + return nil + } + cur = c[idx] + default: + return fmt.Errorf("pointer %s: %q is not inside an object or array", pointer, tok) + } + } + return fmt.Errorf("pointer %s resolves to nothing writable", pointer) +} + +// resolveExistingPrefix returns path made absolute with its longest EXISTING +// prefix symlink-resolved. Containment checks compare a destination that may not +// exist yet against a repo root that does; on a platform where the temp root is +// itself a symlink (/var → /private/var on macOS), comparing unresolved paths +// would silently miss an overlap. +func resolveExistingPrefix(path string) string { + if path == "" { + return "" + } + abs, err := filepath.Abs(path) + if err != nil { + return filepath.Clean(path) + } + rest := "" + cur := abs + for { + if real, err := filepath.EvalSymlinks(cur); err == nil { + return filepath.Join(real, rest) + } + parent := filepath.Dir(cur) + if parent == cur { + return abs + } + rest = filepath.Join(filepath.Base(cur), rest) + cur = parent + } +} diff --git a/internal/core/launch/render_test.go b/internal/core/launch/render_test.go new file mode 100644 index 0000000..e6e87eb --- /dev/null +++ b/internal/core/launch/render_test.go @@ -0,0 +1,259 @@ +package launch + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// renderFixture writes a version-ABSENT dev tree with a readable contract and a +// payload config that ships the manifests, and returns its root. +func renderFixture(t *testing.T) string { + t.Helper() + root := t.TempDir() + writeFile(t, root, ".abcd/config/launch-payload.json", `{"includes": [".claude-plugin", "README.md"]}`) + writeFile(t, root, "README.md", "readme\n") + writeLockstepTree(t, root, "", "", "") + return root +} + +func sampleEntry() ChangelogEntry { + return ChangelogEntry{ + Tier: "minor", + Reason: "additive itd-67 shipped", + Date: time.Date(2026, 7, 21, 0, 0, 0, 0, time.UTC), + SourceSHA: "a1b2c3d4e5f6", + } +} + +// TestRenderPayloadSatisfiesPublicLockstep is the core detector: the rendered +// payload carries the derived version at all three pinned locations and passes +// the SAME public checker a ship would run against it. +func TestRenderPayloadSatisfiesPublicLockstep(t *testing.T) { + root := renderFixture(t) + dest := filepath.Join(t.TempDir(), "payload") + + res, err := RenderPayload(PayloadRenderRequest{ + RepoRoot: root, Dest: dest, Version: "0.4.0", Entry: sampleEntry(), + }) + if err != nil { + t.Fatalf("render must succeed on a clean tree: %v", err) + } + if res.Lockstep.ExitCode != 0 || !res.Lockstep.OK { + t.Fatalf("rendered payload must pass the public check, got %+v", res.Lockstep) + } + + vl := filepath.Join(root, versionLocationRelPath) + if got := CheckLockstep(TreePublic, dest, vl); !got.OK { + t.Fatalf("independent public check over the payload must pass, got %+v", got) + } + + plugin, err := loadJSON(filepath.Join(dest, ".claude-plugin/plugin.json")) + if err != nil { + t.Fatalf("read the rendered primary manifest: %v", err) + } + market, err := loadJSON(filepath.Join(dest, ".claude-plugin/marketplace.json")) + if err != nil { + t.Fatalf("read the rendered marketplace: %v", err) + } + cases := []struct { + name string + doc any + ptr string + want any + }{ + {"primary version", plugin, "/version", "0.4.0"}, + {"marketplace version", market, secondaryVersionPointer, "0.4.0"}, + {"changelog version", market, changelogVersionPointer, "0.4.0"}, + {"changelog tier", market, "/plugins/0/changelog/tier", "minor"}, + {"changelog date", market, "/plugins/0/changelog/date", "2026-07-21"}, + {"changelog source_sha", market, "/plugins/0/changelog/source_sha", "a1b2c3d4e5f6"}, + {"preserved name", plugin, "/name", "abcd"}, + } + for _, tc := range cases { + got, present := resolvePointer(tc.doc, tc.ptr) + if !present || got != tc.want { + t.Errorf("%s: expected %v at %s, got %v (present=%v)", tc.name, tc.want, tc.ptr, got, present) + } + } + + // The payload is a payload, not a manifest pair: the non-manifest includes + // must be copied too, or "renders the release payload" would be a lie. + if _, err := os.Stat(filepath.Join(dest, "README.md")); err != nil { + t.Errorf("payload must carry the non-manifest includes: %v", err) + } +} + +// TestRenderPayloadLeavesSourceTreeUnversioned is the adr-19 detector: rendering +// this repository's own payload must leave the committed manifests +// BYTE-IDENTICAL and the dev polarity still OK. A render that stamped the +// working tree would pass every other test here and silently break the premise. +func TestRenderPayloadLeavesSourceTreeUnversioned(t *testing.T) { + root := repoRootForTest(t) + manifests := []string{".claude-plugin/plugin.json", ".claude-plugin/marketplace.json"} + before := make(map[string][]byte, len(manifests)) + for _, rel := range manifests { + data, err := os.ReadFile(filepath.Join(root, rel)) + if err != nil { + t.Fatalf("read %s: %v", rel, err) + } + before[rel] = data + } + + dest := filepath.Join(t.TempDir(), "payload") + if _, err := RenderPayload(PayloadRenderRequest{ + RepoRoot: root, Dest: dest, Version: "9.9.9", Entry: sampleEntry(), + }); err != nil { + t.Fatalf("render this repository's payload: %v", err) + } + + for _, rel := range manifests { + after, err := os.ReadFile(filepath.Join(root, rel)) + if err != nil { + t.Fatalf("re-read %s: %v", rel, err) + } + if string(after) != string(before[rel]) { + t.Errorf("%s was mutated by the render — adr-19 requires the working tree stay version-absent", rel) + } + } + if res := CheckLockstep(TreeDev, root, filepath.Join(root, versionLocationRelPath)); !res.OK { + t.Errorf("the working tree must still satisfy the dev polarity after a render, got %+v", res) + } +} + +// TestRenderPayloadRefusals pins every input the render must refuse rather than +// stamp. Each one would otherwise publish a manifest that is wrong in a way the +// lockstep check cannot catch, because the check reads the same wrong value. +func TestRenderPayloadRefusals(t *testing.T) { + cases := []struct { + name string + mutate func(t *testing.T, root string, req *PayloadRenderRequest) + wantErr string + }{ + { + name: "empty version", + mutate: func(_ *testing.T, _ string, req *PayloadRenderRequest) { req.Version = "" }, + wantErr: "SemVer", + }, + { + name: "leading v", + mutate: func(_ *testing.T, _ string, req *PayloadRenderRequest) { req.Version = "v0.4.0" }, + wantErr: "SemVer", + }, + { + name: "two-component version", + mutate: func(_ *testing.T, _ string, req *PayloadRenderRequest) { req.Version = "0.4" }, + wantErr: "SemVer", + }, + { + name: "unknown bump tier", + mutate: func(_ *testing.T, _ string, req *PayloadRenderRequest) { req.Entry.Tier = "huge" }, + wantErr: "tier", + }, + { + name: "blocked contract", + mutate: func(t *testing.T, root string, _ *PayloadRenderRequest) { + writeFile(t, root, versionLocationRelPath, `{"blocked": true}`) + }, + wantErr: "blocked", + }, + { + name: "destination inside the repository", + mutate: func(_ *testing.T, root string, req *PayloadRenderRequest) { + req.Dest = filepath.Join(root, "payload") + }, + wantErr: "inside the repository", + }, + { + name: "destination already populated", + mutate: func(t *testing.T, _ string, req *PayloadRenderRequest) { + if err := os.MkdirAll(req.Dest, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(req.Dest, "stale.txt"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + }, + wantErr: "not empty", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := renderFixture(t) + req := PayloadRenderRequest{ + RepoRoot: root, Dest: filepath.Join(t.TempDir(), "payload"), + Version: "0.4.0", Entry: sampleEntry(), + } + tc.mutate(t, root, &req) + _, err := RenderPayload(req) + if err == nil { + t.Fatalf("expected a refusal mentioning %q, got none", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("expected a refusal mentioning %q, got %v", tc.wantErr, err) + } + }) + } +} + +// TestRenderPayloadRefusesWhenManifestsAreNotShipped proves the render fails +// closed when the payload config would not carry the manifests at all: stamping +// a version into files nobody ships is a silent no-op today and an unversioned +// release tomorrow. +func TestRenderPayloadRefusesWhenManifestsAreNotShipped(t *testing.T) { + root := renderFixture(t) + writeFile(t, root, ".abcd/config/launch-payload.json", `{"includes": ["README.md"]}`) + _, err := RenderPayload(PayloadRenderRequest{ + RepoRoot: root, Dest: filepath.Join(t.TempDir(), "payload"), + Version: "0.4.0", Entry: sampleEntry(), + }) + if err == nil || !strings.Contains(err.Error(), "not in the payload") { + t.Fatalf("expected a refusal naming the missing manifest, got %v", err) + } +} + +// TestRenderPayloadRefusesAnUnstampableMarketplace proves a marketplace with no +// plugins[0] refuses rather than being invented into existence: a stamp that +// created the missing entry would publish a plausible-looking artefact whose +// plugin record no harness put there. +func TestRenderPayloadRefusesAnUnstampableMarketplace(t *testing.T) { + root := renderFixture(t) + writeFile(t, root, ".claude-plugin/marketplace.json", `{"plugins": []}`) + _, err := RenderPayload(PayloadRenderRequest{ + RepoRoot: root, Dest: filepath.Join(t.TempDir(), "payload"), + Version: "0.4.0", Entry: sampleEntry(), + }) + if err == nil || !strings.Contains(err.Error(), "out of range") { + t.Fatalf("expected a refusal naming the unwritable pointer, got %v", err) + } +} + +// TestRenderPayloadRefusesOnItsOwnDrift proves the backstop: if the public check +// over the rendered payload ever disagrees — the shape a newly-pinned location +// the render does not stamp would take — the render REFUSES and carries the +// failing verdict, rather than returning a payload it just proved inconsistent. +func TestRenderPayloadRefusesOnItsOwnDrift(t *testing.T) { + root := renderFixture(t) + restore := payloadLockstep + t.Cleanup(func() { payloadLockstep = restore }) + payloadLockstep = func(tree LockstepTree, repoRoot, vl string) LockstepResult { + return LockstepResult{Tree: tree, Drifts: []string{"DRIFT public /pinned/later: expected 0.4.0"}, ExitCode: 1} + } + + res, err := RenderPayload(PayloadRenderRequest{ + RepoRoot: root, Dest: filepath.Join(t.TempDir(), "payload"), + Version: "0.4.0", Entry: sampleEntry(), + }) + if !errors.Is(err, ErrPayloadDrift) { + t.Fatalf("expected ErrPayloadDrift, got %v", err) + } + if !strings.Contains(err.Error(), "/pinned/later") { + t.Errorf("the refusal must name the drifting location, got %v", err) + } + if res.Lockstep.OK || len(res.Lockstep.Drifts) != 1 { + t.Errorf("the failing verdict must travel with the result, got %+v", res.Lockstep) + } +} diff --git a/internal/core/launch/semver.go b/internal/core/launch/semver.go index 3d6dbf2..ea4c811 100644 --- a/internal/core/launch/semver.go +++ b/internal/core/launch/semver.go @@ -46,6 +46,26 @@ func ParseSemver(value string) (Semver, error) { return Semver{Major: major, Minor: minor, Patch: patch, Prerelease: m[4], Build: m[5]}, nil } +// BumpTier names the SemVer component that moved from prev to next: "major", +// "minor", "patch", or "" when the two cores are equal. +// +// It reads the actual version delta rather than mapping an impact, because the +// mapping is not fixed: pre-1.0, changelog.DeriveNext turns a breaking impact +// into a MINOR bump and an additive one into a PATCH. A tier derived from the +// impact would therefore mislabel every pre-1.0 release in the published +// manifest, while the delta is true at any point on the 0.x/1.x boundary. +func BumpTier(prev, next Semver) string { + switch { + case next.Major != prev.Major: + return "major" + case next.Minor != prev.Minor: + return "minor" + case next.Patch != prev.Patch: + return "patch" + } + return "" +} + // Line is the MAJOR.MINOR retention line a version belongs to. func (s Semver) Line() string { return fmt.Sprintf("%d.%d", s.Major, s.Minor) } diff --git a/internal/core/launch/ship.go b/internal/core/launch/ship.go index 7a69851..4f7020d 100644 --- a/internal/core/launch/ship.go +++ b/internal/core/launch/ship.go @@ -9,10 +9,12 @@ import ( // ShipRequest is the input to a ship run. type ShipRequest struct { - RepoRoot string - VersionOverride string - AllowDirty bool - ExistingTags []Semver + RepoRoot string + // Version is the release version, supplied by the caller for the reason + // DryRunRequest.Version states: adr-19 leaves nothing in the tree to read. + Version string + AllowDirty bool + ExistingTags []Semver } // ShipReport is the outcome of a ship run. It stops at WouldPublish before any @@ -23,6 +25,7 @@ type ShipReport struct { Scan scanner.ScanResult `json:"scan"` Lockstep LockstepResult `json:"lockstep"` Retention RetentionPlan `json:"retention"` + Smoke SmokeReport `json:"smoke"` Blocked bool `json:"blocked"` BlockReasons []string `json:"block_reasons,omitempty"` WouldPublish bool `json:"would_publish"` // true iff all gates pass @@ -50,17 +53,20 @@ func Ship(req ShipRequest) (ShipReport, error) { scan := scanBundle(req.RepoRoot, bundle) report.Scan = scan + // The source tree is checked under the DEV polarity, for the reason DryRun + // states: adr-19 keeps the version out of the committed manifests, and the + // public polarity is proved over the rendered payload instead. vlPath := filepath.Join(req.RepoRoot, versionLocationRelPath) - lockstep := CheckLockstep(TreePublic, req.RepoRoot, vlPath) + lockstep := CheckLockstep(TreeDev, req.RepoRoot, vlPath) report.Lockstep = lockstep - version := resolveVersion(req.VersionOverride, req.RepoRoot, lockstep) - report.Version = version - report.Retention = computeRetentionForReport(version, DryRunRequest{ - RepoRoot: req.RepoRoot, VersionOverride: req.VersionOverride, ExistingTags: req.ExistingTags, + report.Version = req.Version + report.Retention = computeRetentionForReport(req.Version, DryRunRequest{ + RepoRoot: req.RepoRoot, Version: req.Version, ExistingTags: req.ExistingTags, }) - report.BlockReasons = wouldRefuseOn(bundle, scan, lockstep, report.Retention) + report.Smoke = SmokeLight(NewBundleTree(bundle)) + report.BlockReasons = wouldRefuseOn(bundle, scan, lockstep, report.Retention, report.Smoke) if len(report.BlockReasons) > 0 { report.Blocked = true report.WouldPublish = false diff --git a/internal/core/launch/smoke.go b/internal/core/launch/smoke.go new file mode 100644 index 0000000..49da7c4 --- /dev/null +++ b/internal/core/launch/smoke.go @@ -0,0 +1,155 @@ +package launch + +// smoke.go — the installability smoke: the ASSERTION tier over the surface +// installsurface.go resolves. +// +// The light tier asserts the three things itd-67 names: both manifests parse, +// the marketplace source resolves, and every declared path the payload is +// responsible for is actually carried. It asserts nothing about what those files +// CONTAIN — that is itd-66's deep tier, which consumes the identical resolved +// list and adds import/frontmatter/help assertions in an isolated subprocess. +// Because the tiers share the resolution step, the upgrade replaces the +// assertions here and leaves the surface definition untouched. + +import "fmt" + +// SmokeTier names how deep an installability check went, so a report never +// implies more assurance than it earned. +type SmokeTier string + +// SmokeTierLight is manifest-parse + source-resolve + declared-path-exists. +const SmokeTierLight SmokeTier = "light" + +// Finding kinds. They are stable strings because an operator greps them and a +// gate summary counts them. +const ( + findingManifestUnreadable = "manifest-unreadable" + findingSourceUnresolved = "source-unresolved" + findingNameMismatch = "plugin-name-mismatch" + findingMissingPath = "missing-declared-path" +) + +// SmokeFinding is one reason the payload would not install. +type SmokeFinding struct { + Kind string `json:"kind"` + Path string `json:"path,omitempty"` + Detail string `json:"detail"` +} + +// SmokeReport is one installability check. +type SmokeReport struct { + Tier SmokeTier `json:"tier"` + OK bool `json:"ok"` + Surface InstallSurface `json:"surface"` + // Checked counts the assertions actually made, so a pass over a payload that + // declared nothing is visibly vacuous rather than reassuring. + Checked int `json:"checked"` + Findings []SmokeFinding `json:"findings,omitempty"` +} + +// SmokeLight runs the light installability tier over a payload. +// +// It never returns an error: an unreadable manifest is the most serious FINDING +// it can make, and reporting it as a finding keeps the gate's output shape the +// same whether the payload is perfect or unparseable — the dry-run preview +// depends on always having a report to render. +func SmokeLight(tree PayloadTree) SmokeReport { + report := SmokeReport{Tier: SmokeTierLight} + + surface, err := ResolveInstallSurface(tree) + if err != nil { + report.Findings = append(report.Findings, SmokeFinding{ + Kind: findingManifestUnreadable, Detail: err.Error(), + }) + return report + } + report.Surface = surface + + for _, mp := range surface.Marketplace { + switch mp.SourceKind { + case SourceMissing: + report.Checked++ + report.Findings = append(report.Findings, SmokeFinding{ + Kind: findingSourceUnresolved, + Path: mp.Name, + Detail: fmt.Sprintf("marketplace plugin %q declares no source", mp.Name), + }) + continue + case SourceExternal: + // A remote source is resolvable only over the network. An offline + // gate records it and does NOT count it as checked — a Checked total + // that included unasserted entries would overstate the assurance. + continue + } + report.Checked++ + manifest := joinPayloadPath(mp.Root, pluginManifestFile) + if !tree.Has(manifest) { + report.Findings = append(report.Findings, SmokeFinding{ + Kind: findingSourceUnresolved, + Path: manifest, + Detail: fmt.Sprintf("marketplace plugin %q sources %q, which carries no plugin manifest", mp.Name, mp.Source), + }) + continue + } + named, err := readManifest(tree, manifest) + if err != nil { + report.Findings = append(report.Findings, SmokeFinding{ + Kind: findingSourceUnresolved, Path: manifest, Detail: err.Error(), + }) + continue + } + if name, _ := named["name"].(string); name != mp.Name { + report.Findings = append(report.Findings, SmokeFinding{ + Kind: findingNameMismatch, + Path: manifest, + Detail: fmt.Sprintf("marketplace lists %q but the sourced manifest is named %q — the install id would not resolve", mp.Name, name), + }) + } + } + + for _, e := range surface.Entries { + if e.Requirement != RequirePayload { + continue + } + report.Checked++ + if tree.Has(e.Path) { + continue + } + detail := fmt.Sprintf("declared %s %q is not in the payload", e.Kind, e.Path) + if e.DeclaredAs != "" { + detail += fmt.Sprintf(" (declared as %q)", e.DeclaredAs) + } + report.Findings = append(report.Findings, SmokeFinding{ + Kind: findingMissingPath, Path: e.Path, Detail: detail, + }) + } + + report.OK = len(report.Findings) == 0 + return report +} + +// joinPayloadPath joins a payload-relative plugin root with a path beneath it. +func joinPayloadPath(root, rel string) string { + if root == "" { + return rel + } + return root + "/" + rel +} + +// smokeRefusals renders a failed smoke as the refusal lines a launch gate +// reports, so an operator reads WHICH path broke rather than a count. +func smokeRefusals(report SmokeReport) []string { + var out []string + for _, f := range report.Findings { + out = append(out, "installability smoke: "+f.Detail) + } + return out +} + +// smokeDetail summarises the smoke for the gate line. +func smokeDetail(report SmokeReport) string { + if report.OK { + return "checked " + itoa(report.Checked) + " declared paths, 0 findings" + } + return "checked " + itoa(report.Checked) + " declared paths, " + itoa(len(report.Findings)) + " findings" +} diff --git a/internal/core/launch/smoke_test.go b/internal/core/launch/smoke_test.go new file mode 100644 index 0000000..da0cf4e --- /dev/null +++ b/internal/core/launch/smoke_test.go @@ -0,0 +1,160 @@ +package launch + +import ( + "path/filepath" + "strings" + "testing" + "time" +) + +// TestSmokeLightPassesOnCommittedPayload is the positive detector: this +// repository's real payload is installable — both manifests parse, the +// marketplace source resolves, and every declared path is carried. +func TestSmokeLightPassesOnCommittedPayload(t *testing.T) { + report := SmokeLight(bundleTreeFor(t, repoRootForTest(t))) + if !report.OK { + t.Fatalf("the committed payload must pass the light smoke, got %+v", report.Findings) + } + if report.Checked == 0 { + t.Error("a passing smoke that checked nothing is a vacuous pass") + } +} + +// TestSmokeLightFailsAndNamesTheMissingPath is the negative detector, one case +// per way a payload can be uninstallable. Every failure must NAME the path or +// manifest at fault — a smoke that only says "failed" cannot be acted on. +func TestSmokeLightFailsAndNamesTheMissingPath(t *testing.T) { + cases := []struct { + name string + files map[string]string + marketplace string + wantKind string + wantNamed string + }{ + { + name: "a declared command missing from the payload", + files: map[string]string{"plugin_extra": `"commands": "./commands/ghost.md"`}, + wantKind: findingMissingPath, + wantNamed: "commands/ghost.md", + }, + { + name: "a declared agent missing from the payload", + files: map[string]string{"plugin_extra": `"agents": ["./agents/absent.md"]`}, + wantKind: findingMissingPath, + wantNamed: "agents/absent.md", + }, + { + name: "a declared skill missing from the payload", + files: map[string]string{"plugin_extra": `"skills": "./skills/nowhere"`}, + wantKind: findingMissingPath, + wantNamed: "skills/nowhere", + }, + { + name: "a hook command pointing at a file the payload excludes", + files: map[string]string{ + "hooks/hooks.json": `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "\"$CLAUDE_PLUGIN_ROOT/scripts/absent.sh\""}]}]}}`, + }, + wantKind: findingMissingPath, + wantNamed: "scripts/absent.sh", + }, + { + name: "a marketplace source that resolves to no plugin manifest", + files: map[string]string{}, + marketplace: `{"name": "m", "plugins": [{"name": "abcd", "source": "./nested"}]}`, + wantKind: findingSourceUnresolved, + wantNamed: "nested", + }, + { + name: "a marketplace entry naming a plugin the manifest does not", + files: map[string]string{}, + marketplace: `{"name": "m", "plugins": [{"name": "other", "source": "./"}]}`, + wantKind: findingNameMismatch, + wantNamed: "other", + }, + { + name: "an unparseable plugin manifest", + files: map[string]string{"plugin_broken": "yes"}, + wantKind: findingManifestUnreadable, + wantNamed: ".claude-plugin/plugin.json", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + writeSurfaceFixture(t, root, tc.files) + if tc.marketplace != "" { + writeFile(t, root, ".claude-plugin/marketplace.json", tc.marketplace) + } + report := SmokeLight(bundleTreeFor(t, root)) + if report.OK { + t.Fatalf("the smoke must FAIL: %+v", report) + } + var named bool + for _, f := range report.Findings { + if f.Kind == tc.wantKind && strings.Contains(f.Path+" "+f.Detail, tc.wantNamed) { + named = true + } + } + if !named { + t.Errorf("expected a %q finding naming %q, got %+v", tc.wantKind, tc.wantNamed, report.Findings) + } + }) + } +} + +// TestRenderPayloadRefusesAnUninstallablePayload is the enforcement detector: the +// render is the only step that materialises a release artefact, so it is where a +// missing declared path must stop the cut rather than publish. +func TestRenderPayloadRefusesAnUninstallablePayload(t *testing.T) { + root := t.TempDir() + writeSurfaceFixture(t, root, map[string]string{"plugin_extra": `"commands": "./commands/ghost.md"`}) + writeFile(t, root, ".abcd/config/version-location.json", + `{"manifest_path": ".claude-plugin/plugin.json", "json_pointer": "/version"}`) + + dest := filepath.Join(t.TempDir(), "payload") + _, err := RenderPayload(PayloadRenderRequest{ + RepoRoot: root, Dest: dest, Version: "1.0.0", + Entry: ChangelogEntry{Tier: "patch", Reason: "r", Date: time.Now(), SourceSHA: "abc"}, + }) + if err == nil { + t.Fatal("the render must refuse a payload whose declared surface is incomplete") + } + if !strings.Contains(err.Error(), "commands/ghost.md") { + t.Errorf("the refusal must name the missing path, got %v", err) + } +} + +// TestDryRunReportsTheInstallabilitySmoke proves the gate is reachable from the +// preview a maintainer actually runs, and that a finding reaches WouldRefuseOn +// rather than sitting silently in the report. +func TestDryRunReportsTheInstallabilitySmoke(t *testing.T) { + root := t.TempDir() + writeSurfaceFixture(t, root, map[string]string{"plugin_extra": `"commands": "./commands/ghost.md"`}) + + report, err := DryRun(DryRunRequest{RepoRoot: root, Version: "1.0.0"}) + if err != nil { + t.Fatalf("dry-run must still produce a report: %v", err) + } + if report.Smoke.OK { + t.Fatalf("the smoke must fail on a missing declared path: %+v", report.Smoke) + } + var gated bool + for _, g := range report.Gates { + if g.Name == "installability-smoke" && g.Status == "ran" { + gated = true + } + } + if !gated { + t.Errorf("the installability smoke must appear as a gate that ran, got %+v", report.Gates) + } + var refused bool + for _, r := range report.WouldRefuseOn { + if strings.Contains(r, "commands/ghost.md") { + refused = true + } + } + if !refused { + t.Errorf("WouldRefuseOn must name the missing path, got %v", report.WouldRefuseOn) + } +} diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index 31d0c1c..90502bb 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -107,7 +107,10 @@ func NewRootCommand() *cobra.Command { if !launchDryRun { return fmt.Errorf("abcd launch: pass --dry-run to preview the bundle (publishing is not wired at this stage)") } - rep, err := launch.DryRun(launch.DryRunRequest{RepoRoot: cwd}) + rep, err := launch.DryRun(launch.DryRunRequest{ + RepoRoot: cwd, + Version: publishedVersion(cwd), + }) if err != nil { return err } diff --git a/internal/surface/cli/ship.go b/internal/surface/cli/ship.go index adbce17..67b5b52 100644 --- a/internal/surface/cli/ship.go +++ b/internal/surface/cli/ship.go @@ -4,11 +4,15 @@ import ( "fmt" "io" "os" + "path/filepath" "strings" "time" "github.com/REPPL/abcd-cli/internal/core/changelog" + "github.com/REPPL/abcd-cli/internal/core/launch" "github.com/REPPL/abcd-cli/internal/core/release" + "github.com/REPPL/abcd-cli/internal/fsutil" + "github.com/REPPL/abcd-cli/internal/gitutil" "github.com/REPPL/abcd-cli/internal/termsafe" "github.com/spf13/cobra" ) @@ -29,12 +33,114 @@ func emitCut(repoRoot string) (release.Cut, error) { // two things a core must never reach for itself — the untrusted payload, and the // clock. The date lands in a durable release heading, so it is an argument a test // can pin rather than a wall-clock read buried in a writer. -func ingestCut(repoRoot string, raw []byte) (release.IngestResult, error) { +func ingestCut(repoRoot string, raw []byte, at time.Time) (release.IngestResult, error) { current, err := SurfaceSnapshot(repoRoot) if err != nil { return release.IngestResult{}, err } - return release.Ingest(repoRoot, current, raw, time.Now()) + return release.Ingest(repoRoot, current, raw, at) +} + +// shipResult is what the ingest step reports: the release record that landed, +// plus the staged payload when one was asked for. The IngestResult is embedded +// so its JSON shape is unchanged for a ship that stages nothing — the payload is +// an addition to the report, not a new dialect of it. +type shipResult struct { + release.IngestResult + Payload *launch.PayloadRenderResult `json:"payload,omitempty"` +} + +// renderReleasePayload stages the versioned release payload for a completed cut. +// +// It is the one place the derived version crosses from the cut into a manifest. +// Everything the core needs is assembled HERE — the version from the cut, the +// tier from the version delta, the date from the same clock the heading was +// dated with, the source commit from git — because internal/core/launch must not +// read a clock or shell out to decide what a durable artefact says. +func renderReleasePayload(repoRoot, dest string, cut release.Cut, at time.Time) (launch.PayloadRenderResult, error) { + next, err := launch.ParseSemver(strings.TrimPrefix(cut.NextTag, "v")) + if err != nil { + return launch.PayloadRenderResult{}, err + } + prev, err := launch.ParseSemver(strings.TrimPrefix(cut.BaseTag, "v")) + if err != nil { + return launch.PayloadRenderResult{}, err + } + sha, err := gitutil.Run(repoRoot, "rev-parse", "HEAD") + if err != nil { + return launch.PayloadRenderResult{}, err + } + return launch.RenderPayload(launch.PayloadRenderRequest{ + RepoRoot: repoRoot, + Dest: dest, + Version: next.String(), + Entry: launch.ChangelogEntry{ + Tier: launch.BumpTier(prev, next), + Reason: bumpReason(cut), + Date: at, + SourceSHA: sha, + }, + }) +} + +// publishedVersion is the version a launch would publish: the version of the +// newest dated CHANGELOG heading (adr-37), which is the release auto-release.yml +// turns into a tag. +// +// It is resolved at the front door rather than inside internal/core/launch for +// two reasons. adr-19 leaves no version key in the committed manifests, so there +// is nothing there for the launch core to read; and the one reader of that +// heading lives in internal/core/changelog, which imports launch — so the front +// door is the only place the two can meet without a cycle. +// +// An absent or unreadable release record yields the empty string, which the +// retention gate reports as a refusal. Naming no version is the honest answer; a +// launch preview must never invent one. +func publishedVersion(repoRoot string) string { + v, found, err := changelog.LatestChangelogVersion(repoRoot) + if err != nil || !found { + return "" + } + return v.String() +} + +// changelogPath names the release record the ingest step writes. The ship verb +// holds its pre-ingest bytes so a refused render can put them back. +func changelogPath(repoRoot string) string { + return filepath.Join(repoRoot, "CHANGELOG.md") +} + +// rollbackCut undoes a cut whose payload render refused: it restores the +// pre-ingest release record and removes the staging directory the precheck +// proved was empty or absent, so nothing outside the render's own output is +// touched. +// +// It returns the sentence appended to the refusal rather than an error, because +// what an operator reading exit 2 most needs to know is whether a durable write +// survived — and a rollback that itself failed is the one case where they must +// recover by hand. +func rollbackCut(repoRoot, dest string, before []byte) string { + var failures []string + if err := fsutil.WriteFileAtomicPreserveMode(changelogPath(repoRoot), before); err != nil { + failures = append(failures, "CHANGELOG.md: "+scrubPaths(err)) + } + if err := os.RemoveAll(dest); err != nil { + failures = append(failures, "the payload destination: "+scrubPaths(err)) + } + if len(failures) > 0 { + return "\n THE ROLLBACK FAILED — recover by hand: " + strings.Join(failures, "; ") + } + return "\n the release record was rolled back and nothing was staged" +} + +// bumpReason is the human sentence adr-20's changelog entry records: the impact +// that decided the version and the records that carried it, so a published +// manifest says WHY it is this version and not merely what it is. +func bumpReason(cut release.Cut) string { + if len(cut.DecidedBy) == 0 { + return string(cut.Impact) + } + return string(cut.Impact) + ": " + strings.Join(cut.DecidedBy, ", ") } // newLaunchShipCommand builds `abcd launch ship`, the release-cut write verb. @@ -64,8 +170,9 @@ func ingestCut(repoRoot string, raw []byte) (release.IngestResult, error) { // diagnostic is path-scrubbed. Nothing is written on this path. func newLaunchShipCommand(asJSON *bool) *cobra.Command { var changelogJSON string + var payloadDir string cmd := &cobra.Command{ - Use: "ship [--changelog-json ]", + Use: "ship [--changelog-json ] [--payload-dir ]", Short: "Cut a release: derive the version and the record set from what shipped (exit 1 when the cut refuses)", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { @@ -73,6 +180,13 @@ func newLaunchShipCommand(asJSON *bool) *cobra.Command { if err != nil { return err } + // The payload is rendered by the INGEST step, from a version only a + // completed cut has. Asking the emit step for one is an operand + // error, refused before anything is read or staged. + if payloadDir != "" && changelogJSON == "" { + return &exitError{Code: 2, Msg: "abcd launch ship: --payload-dir needs --changelog-json — " + + "the release payload is staged by the ingest step, which the deterministic emit step does not run"} + } // Read the payload before anything else: it is untrusted host input, // and reading it through the shared guarded-operand path keeps the // ingest seam behind exactly the trust boundary the other delegated @@ -82,10 +196,43 @@ func newLaunchShipCommand(asJSON *bool) *cobra.Command { return &exitError{Code: 2, Msg: "abcd launch ship: " + scrubPaths(err)} } if raw != nil { - res, err := ingestCut(cwd, raw) + // Every render refusal that does not need a version is made + // BEFORE the ingest step writes the dated heading. That heading + // is a durable release record: a render that refuses after it + // lands leaves a release in flight, and every retry is then + // refused for being in flight. The version-free half of the + // render is therefore run first, and it writes nothing. + var before []byte + if payloadDir != "" { + if _, perr := launch.PrecheckPayload(cwd, payloadDir); perr != nil { + return &exitError{Code: 2, Msg: "abcd launch ship: " + scrubPaths(perr)} + } + before, err = os.ReadFile(changelogPath(cwd)) + if err != nil { + return &exitError{Code: 2, Msg: "abcd launch ship: " + scrubPaths(err)} + } + } + at := time.Now() + ingested, err := ingestCut(cwd, raw, at) if err != nil { return &exitError{Code: 2, Msg: "abcd launch ship: " + scrubPaths(err)} } + res := shipResult{IngestResult: ingested} + // Stage only behind a written record: a refused cut has no + // version to stamp, and a refused document must leave the + // filesystem exactly as it found it. + if payloadDir != "" && ingested.Written { + staged, rerr := renderReleasePayload(cwd, payloadDir, ingested.Cut, at) + if rerr != nil { + // The precheck cleared everything version-free, so a + // refusal here is version-shaped and rare — but the + // record is already on disk, so it is rolled back rather + // than left as an untaggable release in flight. + return &exitError{Code: 2, Msg: "abcd launch ship: " + scrubPaths(rerr) + + rollbackCut(cwd, payloadDir, before)} + } + res.Payload = &staged + } if rerr := render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) { renderIngest(w, res) }); rerr != nil { @@ -114,6 +261,8 @@ func newLaunchShipCommand(asJSON *bool) *cobra.Command { } cmd.Flags().StringVar(&changelogJSON, "changelog-json", "", "path to the host-composed changelog JSON (or - for stdin); absent runs the deterministic emit step") + cmd.Flags().StringVar(&payloadDir, "payload-dir", "", + "stage the versioned release payload in this directory (must be empty and outside the repository)") return cmd } @@ -186,7 +335,7 @@ func renderCut(w io.Writer, verb string, cut release.Cut) { // The cut is rendered FIRST and unchanged, so the two halves of one verb read // identically — an operator comparing a dry preview with the ship that followed // is comparing the same lines, not two dialects of the same report. -func renderIngest(w io.Writer, res release.IngestResult) { +func renderIngest(w io.Writer, res shipResult) { renderCut(w, "abcd launch ship", res.Cut) if !res.Written { return @@ -194,6 +343,14 @@ func renderIngest(w io.Writer, res release.IngestResult) { fmt.Fprintf(w, " wrote: %s\n", res.Path) fmt.Fprintf(w, " %s\n", res.Heading) fmt.Fprintf(w, " %d line(s), citing %s\n", res.Lines, termsafe.Sanitize(strings.Join(res.Cited, ", "))) + if res.Payload == nil { + return + } + // The staged path is an operator-supplied absolute location, so it is + // reported through the same sanitiser every other outside string uses. + fmt.Fprintf(w, " payload: %s\n", termsafe.Sanitize(res.Payload.Dest)) + fmt.Fprintf(w, " %d file(s), version %s in %s\n", + res.Payload.Files, res.Payload.Version, strings.Join(res.Payload.Manifests, ", ")) } // guardLine renders the guardrail verdict, keeping a REFUSED guard visually diff --git a/internal/surface/cli/ship_payload_test.go b/internal/surface/cli/ship_payload_test.go new file mode 100644 index 0000000..759d7d9 --- /dev/null +++ b/internal/surface/cli/ship_payload_test.go @@ -0,0 +1,278 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/REPPL/abcd-cli/internal/core/launch" + "github.com/REPPL/abcd-cli/internal/gittest" +) + +// shipRenderableRepo is shipReadyRepo plus the two config artefacts a payload +// render needs: the adr-19 version-location contract (WHERE the version goes) +// and the payload includes (WHAT ships). +func shipRenderableRepo(t *testing.T) *gittest.Repo { + t.Helper() + r := shipReadyRepo(t) + r.Write(".abcd/config/version-location.json", + `{"manifest_path": ".claude-plugin/plugin.json", "json_pointer": "/version"}`+"\n") + r.Write(".abcd/config/launch-payload.json", + `{"includes": [".claude-plugin", "CHANGELOG.md"]}`+"\n") + r.Commit("the release configuration") + return r +} + +// TestLaunchShipRendersTheVersionedPayload is the wiring detector for the +// release-payload render: a ship that lands the changelog heading also stages a +// payload whose manifests carry the DERIVED version, while the working tree's +// own manifests stay byte-identical and version-absent (adr-19). +func TestLaunchShipRendersTheVersionedPayload(t *testing.T) { + r := shipRenderableRepo(t) + dest := filepath.Join(t.TempDir(), "payload") + payload := composedPayload(t, t.TempDir(), "v0.4.1", "itd-73") + + before, err := os.ReadFile(filepath.Join(r.Root(), ".claude-plugin/plugin.json")) + if err != nil { + t.Fatal(err) + } + + out, err := shipIn(t, r, "launch", "ship", "--changelog-json", payload, "--payload-dir", dest) + if code := exitCodeOf(err); code != 0 { + t.Fatalf("exit = %d, want 0\n%s\n%v", code, out, err) + } + if !strings.Contains(string(out), "payload:") { + t.Errorf("the render is not reported:\n%s", out) + } + + staged := readJSON(t, filepath.Join(dest, ".claude-plugin/plugin.json")) + if got := staged["version"]; got != "0.4.1" { + t.Errorf("staged plugin.json version = %v, want the derived 0.4.1", got) + } + market := readJSON(t, filepath.Join(dest, ".claude-plugin/marketplace.json")) + entry := market["plugins"].([]any)[0].(map[string]any) + if entry["version"] != "0.4.1" { + t.Errorf("staged marketplace version = %v, want 0.4.1", entry["version"]) + } + changelog, ok := entry["changelog"].(map[string]any) + if !ok || changelog["version"] != "0.4.1" || changelog["tier"] != "patch" { + t.Errorf("staged marketplace changelog entry = %v, want version 0.4.1 tier patch", entry["changelog"]) + } + + after, err := os.ReadFile(filepath.Join(r.Root(), ".claude-plugin/plugin.json")) + if err != nil { + t.Fatal(err) + } + if string(after) != string(before) { + t.Error("adr-19: the working tree's plugin.json was mutated by the ship") + } + if res := launch.CheckLockstep(launch.TreeDev, r.Root(), filepath.Join(r.Root(), ".abcd/config/version-location.json")); !res.OK { + t.Errorf("the working tree must stay version-absent after a ship, got %+v", res) + } + if res := launch.CheckLockstep(launch.TreePublic, dest, filepath.Join(r.Root(), ".abcd/config/version-location.json")); !res.OK { + t.Errorf("the staged payload must satisfy the public polarity, got %+v", res) + } +} + +// TestLaunchShipPayloadDirNeedsTheIngestStep pins the flag's scope: the emit +// step composes nothing and writes nothing, so asking it for a payload is an +// operand error rather than a half-rendered directory. +func TestLaunchShipPayloadDirNeedsTheIngestStep(t *testing.T) { + r := shipRenderableRepo(t) + dest := filepath.Join(t.TempDir(), "payload") + + out, err := shipIn(t, r, "launch", "ship", "--payload-dir", dest) + if code := exitCodeOf(err); code != 2 { + t.Fatalf("exit = %d, want 2\n%s", code, out) + } + if !strings.Contains(err.Error(), "--changelog-json") { + t.Errorf("the error should name the missing operand, got %v", err) + } + if _, statErr := os.Stat(dest); statErr == nil { + t.Error("nothing may be staged when the flag combination is rejected") + } +} + +// TestLaunchShipWithoutPayloadDirStagesNothing keeps the render opt-in: the +// existing ship path is unchanged when no destination is named. +func TestLaunchShipWithoutPayloadDirStagesNothing(t *testing.T) { + r := shipRenderableRepo(t) + payload := composedPayload(t, t.TempDir(), "v0.4.1", "itd-73") + + out, err := shipIn(t, r, "launch", "ship", "--changelog-json", payload) + if code := exitCodeOf(err); code != 0 { + t.Fatalf("exit = %d, want 0\n%s", code, out) + } + if strings.Contains(string(out), "payload:") { + t.Errorf("no payload was requested, so none may be reported:\n%s", out) + } +} + +// TestLaunchShipRefusedPayloadWritesNothing is the atomicity detector for the +// two-step ship: every reason the render can REFUSE must be found before the +// dated CHANGELOG heading is written, because a written heading is a durable +// release record that permanently refuses the retry as release-in-flight. +// +// Each case drives the shipped verb end to end and then asserts the whole of the +// filesystem contract the refusal claims: CHANGELOG.md byte-identical, and no +// half-staged payload left behind. +func TestLaunchShipRefusedPayloadWritesNothing(t *testing.T) { + cases := []struct { + name string + // setup prepares the repository and returns the payload destination. + setup func(t *testing.T, r *gittest.Repo) string + want string + }{ + { + name: "the destination is not empty", + setup: func(t *testing.T, _ *gittest.Repo) string { + dest := filepath.Join(t.TempDir(), "payload") + if err := os.MkdirAll(dest, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dest, "leftover.txt"), []byte("x\n"), 0o644); err != nil { + t.Fatal(err) + } + return dest + }, + want: "not empty", + }, + { + name: "a declared hook is not in the payload", + setup: func(t *testing.T, r *gittest.Repo) string { + r.Write("hooks/hooks.json", + `{"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"$CLAUDE_PLUGIN_ROOT/scripts/go.sh"}]}]}}`+"\n") + r.Write("scripts/go.sh", "#!/bin/sh\nexit 0\n") + r.Write(".abcd/config/launch-payload.json", + `{"includes": [".claude-plugin", "hooks", "CHANGELOG.md"]}`+"\n") + r.Commit("a hook whose script the payload excludes") + return filepath.Join(t.TempDir(), "payload") + }, + want: "is not in the payload", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r := shipRenderableRepo(t) + dest := tc.setup(t, r) + payload := composedPayload(t, t.TempDir(), "v0.4.1", "itd-73") + + before, err := os.ReadFile(filepath.Join(r.Root(), "CHANGELOG.md")) + if err != nil { + t.Fatal(err) + } + + out, err := shipIn(t, r, "launch", "ship", "--changelog-json", payload, "--payload-dir", dest) + if code := exitCodeOf(err); code != 2 { + t.Fatalf("exit = %d, want 2\n%s\n%v", code, out, err) + } + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("the refusal should name %q, got %v", tc.want, err) + } + + after, err := os.ReadFile(filepath.Join(r.Root(), "CHANGELOG.md")) + if err != nil { + t.Fatal(err) + } + if string(after) != string(before) { + t.Errorf("a refused render wrote the release record anyway:\n%s", after) + } + for _, staged := range []string{".claude-plugin/plugin.json", ".claude-plugin/marketplace.json"} { + if _, statErr := os.Stat(filepath.Join(dest, staged)); statErr == nil { + t.Errorf("a refused render left %s staged in the destination", staged) + } + } + }) + } +} + +// TestRollbackCutRestoresTheRecord covers the backstop behind the precheck: if a +// refusal ever does slip past it and land after the write, the release record +// goes back and the half-staged directory goes away, and the operator is TOLD +// which of the two happened. +// +// It is exercised directly because the precheck is what makes that path +// unreachable through the verb — and an untested rollback is exactly the code +// that fails the one time it runs. +func TestRollbackCutRestoresTheRecord(t *testing.T) { + cases := []struct { + name string + // breakIt makes the rollback fail; nil leaves it able to succeed. + breakIt func(t *testing.T, repoRoot, dest string) + want string + }{ + { + name: "the record goes back and the staging goes away", + want: "rolled back", + }, + { + name: "an unrestorable record is reported, not swallowed", + breakIt: func(t *testing.T, repoRoot, _ string) { + // A directory where CHANGELOG.md belongs: the restore cannot + // write it, which is precisely the case an operator must be told + // to recover by hand. + if err := os.RemoveAll(filepath.Join(repoRoot, "CHANGELOG.md")); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(repoRoot, "CHANGELOG.md"), 0o755); err != nil { + t.Fatal(err) + } + }, + want: "THE ROLLBACK FAILED", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + repoRoot := t.TempDir() + dest := filepath.Join(t.TempDir(), "payload") + before := []byte("# Changelog\n\n## [Unreleased]\n") + if err := os.WriteFile(filepath.Join(repoRoot, "CHANGELOG.md"), []byte("written by the cut\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(dest, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dest, "staged.json"), []byte("{}\n"), 0o644); err != nil { + t.Fatal(err) + } + if tc.breakIt != nil { + tc.breakIt(t, repoRoot, dest) + } + + got := rollbackCut(repoRoot, dest, before) + if !strings.Contains(got, tc.want) { + t.Errorf("rollback report = %q, want it to mention %q", got, tc.want) + } + if tc.breakIt != nil { + return + } + restored, err := os.ReadFile(filepath.Join(repoRoot, "CHANGELOG.md")) + if err != nil { + t.Fatal(err) + } + if string(restored) != string(before) { + t.Errorf("the release record was not restored, got %q", restored) + } + if _, err := os.Stat(dest); err == nil { + t.Error("the staging directory the render created must be removed") + } + }) + } +} + +func readJSON(t *testing.T, path string) map[string]any { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + var doc map[string]any + if err := json.Unmarshal(data, &doc); err != nil { + t.Fatalf("parse %s: %v", path, err) + } + return doc +}