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 +}