diff --git a/internal/cli/frontdoor.go b/internal/cli/frontdoor.go index 43284d08..8af6e012 100644 --- a/internal/cli/frontdoor.go +++ b/internal/cli/frontdoor.go @@ -217,7 +217,10 @@ func realpath(path string) string { // would otherwise slip through as "compatible". gateHost prints the actionable // remedy for every non-Compatible verdict EXCEPT NoPluginFound, whose message the // caller owns (it auto-installs by default and only refuses under --no-install, -// so the right wording depends on the caller's choice). +// so the right wording depends on the caller's choice). On Compatible it stays +// silent on the bare OK line but surfaces the opt-in upgrade hint when the plugin +// is contract-compatible yet behind a strictly-newer binary; the hint never +// blocks — the caller still proceeds to launch. func gateHost(ops hostOps, host string, stderr io.Writer) contract.Verdict { manifestPath, err := ops.ResolveManifest(host) if err != nil { @@ -235,6 +238,13 @@ func gateHost(ops hostOps, host string, stderr io.Writer) contract.Verdict { } if res.Verdict != contract.Compatible { fmt.Fprintln(stderr, res.Message) + return res.Verdict + } + // Compatible but behind: surface the opt-in upgrade hint (the front door + // stays silent on the bare OK line). The hint never blocks — the caller still + // proceeds to launch on Compatible. + if res.Hint != "" { + fmt.Fprintln(stderr, res.Hint) } return res.Verdict } diff --git a/internal/cli/frontdoor_test.go b/internal/cli/frontdoor_test.go index eddbb34e..5505ae81 100644 --- a/internal/cli/frontdoor_test.go +++ b/internal/cli/frontdoor_test.go @@ -70,6 +70,17 @@ func withExecutablePath(t *testing.T, path string, err error) { t.Cleanup(func() { executablePath = orig }) } +// withVersion stamps the package Version (the binary display semver the gate +// feeds to the upgrade-hint compare), restoring it after the test. The package +// default is the `dev` sentinel, which suppresses the hint, so a test that +// exercises the behind-plugin hint must stamp a real semver. +func withVersion(t *testing.T, v string) { + t.Helper() + orig := Version + Version = v + t.Cleanup(func() { Version = orig }) +} + func executableFixture(t *testing.T) string { t.Helper() p := filepath.Join(t.TempDir(), "spacedock") @@ -111,6 +122,71 @@ func TestClaudeFrontDoorLaunchesOnCompatible(t *testing.T) { } } +// TestFrontDoorUpgradeHintOnBehindPlugin is AC-4: the front-door gate prints the +// opt-in upgrade hint to stderr when the resolved plugin is contract-compatible +// but behind the binary, then proceeds to launch (the hint never blocks). The +// compatible.json fixture is plugin 0.12.1; stamping the binary Version to a +// strictly-newer semver makes it behind-but-compatible. A paired equal-version +// case (binary == plugin) asserts the gate stays silent. Both arms observe the +// recorded launch + stderr, never a source grep. +func TestFrontDoorUpgradeHintOnBehindPlugin(t *testing.T) { + cases := []struct { + name string + host string + run func(args []string, dir string, fake *fakeHost, stderr *bytes.Buffer) int + }{ + {"claude", "claude", func(args []string, dir string, fake *fakeHost, stderr *bytes.Buffer) int { + var stdout bytes.Buffer + return runClaude(context.Background(), args, dir, fake, lookFound, &stdout, stderr) + }}, + {"codex", "codex", func(args []string, dir string, fake *fakeHost, stderr *bytes.Buffer) int { + var stdout bytes.Buffer + return runCodex(context.Background(), args, dir, fake, lookFound, &stdout, stderr) + }}, + } + for _, tc := range cases { + t.Run(tc.name+" behind-plugin hints + launches", func(t *testing.T) { + withVersion(t, "0.20.0") // strictly newer than the fixture plugin 0.12.1 + fake := &fakeHost{manifest: compatibleManifest(t)} + var stderr bytes.Buffer + + code := tc.run(nil, t.TempDir(), fake, &stderr) + + if code != 0 { + t.Fatalf("exit = %d, want 0 (the hint must not block launch) (stderr=%q)", code, stderr.String()) + } + if fake.launchedArg == nil { + t.Fatalf("launch seam not invoked — the hint must not change the launch path") + } + out := stderr.String() + if !strings.Contains(out, "newer plugin") { + t.Fatalf("stderr missing the behind-plugin upgrade hint: %q", out) + } + if !strings.Contains(out, "spacedock install --host "+tc.host) { + t.Fatalf("stderr hint missing host install command for %s: %q", tc.host, out) + } + }) + + t.Run(tc.name+" equal-version stays silent", func(t *testing.T) { + withVersion(t, "0.12.1") // exactly the fixture plugin version + fake := &fakeHost{manifest: compatibleManifest(t)} + var stderr bytes.Buffer + + code := tc.run(nil, t.TempDir(), fake, &stderr) + + if code != 0 { + t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) + } + if fake.launchedArg == nil { + t.Fatalf("launch seam not invoked on an equal-version compatible plugin") + } + if strings.Contains(stderr.String(), "newer plugin") || strings.Contains(stderr.String(), "spacedock install") { + t.Fatalf("equal-version gate must stay silent (no upgrade hint): %q", stderr.String()) + } + }) + } +} + // TestClaudeFrontDoorFailFastOnMismatch: on a mismatch verdict the launch seam is // NOT invoked and the process exits non-zero with the pinned remedy on stderr. func TestClaudeFrontDoorInjectsResolvedLauncherBin(t *testing.T) { diff --git a/internal/cli/init.go b/internal/cli/init.go index 9bac79e7..c1fb3888 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -44,11 +44,22 @@ func runInit(ctx context.Context, args []string, ops hostOps, stdout, stderr io. fmt.Fprintf(stderr, "spacedock init: could not resolve the installed codex plugin: %v\n", err) return 1 } - if resolved != "" || check { - code := contract.RunDoctor(resolved, "codex", devBranch, Version, stdout, stderr) - if code != 0 || resolved != "" || check { - return code + if check { + return contract.RunDoctor(resolved, "codex", devBranch, Version, stdout, stderr) + } + if resolved != "" { + // An already-present plugin is refreshed on `install` like the claude + // arm — drive the install seam, then run doctor. Without this the codex + // arm was a doctor-only no-op that left a behind plugin in place. + out, err := ops.Install("codex", marketplaceSource, devBranch) + if err != nil { + fmt.Fprintf(stderr, "spacedock install: host install failed: %v\n", err) + return 1 + } + if out != "" { + fmt.Fprintln(stdout, out) } + return runDoctor(ctx, []string{"--host", "codex"}, ops, stdout, stderr) } // Codex install is documented prose when no installed plugin resolves: the diff --git a/internal/cli/init_test.go b/internal/cli/init_test.go index 888579a3..56e96be7 100644 --- a/internal/cli/init_test.go +++ b/internal/cli/init_test.go @@ -102,6 +102,12 @@ func TestInitCheckRunsDoctorWithoutInstalling(t *testing.T) { } func TestInitCodexInstallReadiness(t *testing.T) { + // compatible-installed: `spacedock install --host codex` re-installs an + // already-present compatible plugin instead of short-circuiting to a + // doctor-only no-op. The codex arm must drive the install seam exactly like + // the claude arm — the recorded {host, source, branch} call is the + // independent source of truth that the refresh actually fired, not the + // no-op the prior assertion codified. t.Run("compatible-installed", func(t *testing.T) { fake := &fakeHost{manifest: compatibleManifest(t)} var stdout, stderr bytes.Buffer @@ -111,14 +117,33 @@ func TestInitCodexInstallReadiness(t *testing.T) { if code != 0 { t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) } + wantInstall := []string{"codex", marketplaceSource, devBranch} + if !equalArgv(fake.installCmds, wantInstall) { + t.Fatalf("install seam = %v, want %v — codex init on a present plugin must refresh, not no-op", fake.installCmds, wantInstall) + } + // After install, init runs doctor — a compatible report on stdout. out := stdout.String() if !strings.Contains(out, "OK: spacedock binary "+Version+" and plugin 0.12.1") { - t.Fatalf("codex init should report compatible installed plugin first; stdout = %q", out) + t.Fatalf("codex init should run doctor after install and report compatible; stdout = %q", out) } - for _, banned := range []string{"codex plugin marketplace add", "codex plugin add spacedock@spacedock"} { - if strings.Contains(out, banned) { - t.Errorf("compatible codex init must not print manual add command %q:\n%s", banned, out) - } + }) + + // compatible-installed-check: `--check` keeps the no-install report — it runs + // doctor without driving the install seam. + t.Run("compatible-installed-check", func(t *testing.T) { + fake := &fakeHost{manifest: compatibleManifest(t)} + var stdout, stderr bytes.Buffer + + code := runInit(context.Background(), []string{"--host", "codex", "--check"}, fake, &stdout, &stderr) + + if code != 0 { + t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) + } + if len(fake.installCmds) != 0 { + t.Fatalf("--check must not install: %v", fake.installCmds) + } + if !strings.Contains(stdout.String(), "OK") { + t.Fatalf("--check should print the doctor report; stdout = %q", stdout.String()) } }) diff --git a/internal/cli/install_behavior_codex_test.go b/internal/cli/install_behavior_codex_test.go index 0e1c5877..0c003690 100644 --- a/internal/cli/install_behavior_codex_test.go +++ b/internal/cli/install_behavior_codex_test.go @@ -77,12 +77,84 @@ func TestCodexPluginInstallIsHostNative(t *testing.T) { } } +// TestCodexInitRefreshAdvancesBehindPlugin is the AC-2 live proof for the wired +// codex arm: seed an older plugin (0.0.1) via the production Install seam, then +// run that same seam against a newer (0.0.2) local marketplace — the path +// runInit's codex arm now drives on a present plugin. The resolved cache +// manifest must advance to 0.0.2, read from the on-disk manifest (not the +// install command's claim). This pins the spike's finding as a regression test +// on the production refresh path. Skips when `codex` is not on PATH; hermetic +// via CODEX_HOME isolation + local-path marketplaces (empty branch → no --ref → +// offline). +func TestCodexInitRefreshAdvancesBehindPlugin(t *testing.T) { + if _, err := exec.LookPath("codex"); err != nil { + t.Skip("codex not on PATH; refresh-advances smoke requires the host CLI") + } + + tmp := t.TempDir() + behind := buildCodexMarketplaceAtVersion(t, filepath.Join(tmp, "behind"), "0.0.1") + newer := buildCodexMarketplaceAtVersion(t, filepath.Join(tmp, "newer"), "0.0.2") + codexHomeDir := filepath.Join(tmp, "codexhome") + mustMkdir(t, codexHomeDir) + t.Setenv("CODEX_HOME", codexHomeDir) + + // Seed the behind install (0.0.1) through the production seam. + if out, err := (execHost{}).Install("codex", behind, ""); err != nil { + t.Fatalf("seed Install(codex, 0.0.1) failed: %v\nout=%q", err, out) + } + if got := resolvedCodexManifestVersion(t); got != "0.0.1" { + t.Fatalf("after seed, resolved manifest version = %q, want 0.0.1", got) + } + + // Refresh-on-present (0.0.2) — the wired runInit codex arm calls exactly this + // Install seam when a plugin is already resolved. + if out, err := (execHost{}).Install("codex", newer, ""); err != nil { + t.Fatalf("refresh Install(codex, 0.0.2) failed: %v\nout=%q", err, out) + } + if got := resolvedCodexManifestVersion(t); got != "0.0.2" { + t.Fatalf("after refresh, resolved manifest version = %q, want 0.0.2 (refresh did not advance the behind plugin)", got) + } +} + +// resolvedCodexManifestVersion resolves the cached spacedock@spacedock manifest +// via the production resolver and returns its on-disk version field — the +// independent source of truth that an install advanced the plugin, not the +// command's stdout. +func resolvedCodexManifestVersion(t *testing.T) string { + t.Helper() + manifest, err := execHost{}.resolveCodexManifest() + if err != nil { + t.Fatalf("resolveCodexManifest: %v", err) + } + if manifest == "" { + t.Fatalf("resolveCodexManifest returned empty after an install") + } + data, err := os.ReadFile(manifest) + if err != nil { + t.Fatalf("read resolved manifest %s: %v", manifest, err) + } + var m struct { + Version string `json:"version"` + } + if err := json.Unmarshal(data, &m); err != nil { + t.Fatalf("parse resolved manifest %s: %v", manifest, err) + } + return m.Version +} + // buildLocalCodexMarketplace writes a minimal valid local-path marketplace under // root and returns the marketplace directory. Codex reads the marketplace // manifest from .claude-plugin/marketplace.json (it reuses the claude manifest // layout) and the plugin manifest from the plugin's .codex-plugin/plugin.json. // The plugin manifest carries a requires-contract bracketing CONTRACT_VERSION. func buildLocalCodexMarketplace(t *testing.T, root string) string { + return buildCodexMarketplaceAtVersion(t, root, "0.0.0") +} + +// buildCodexMarketplaceAtVersion is buildLocalCodexMarketplace parameterized by +// the plugin's display version, so a test can seed a behind plugin then refresh +// it from a newer marketplace and observe the resolved version advance. +func buildCodexMarketplaceAtVersion(t *testing.T, root, version string) string { t.Helper() marketplace := filepath.Join(root, "marketplace") plugin := filepath.Join(marketplace, "spacedock") @@ -98,7 +170,8 @@ func buildLocalCodexMarketplace(t *testing.T, root string) string { ] } `) - mustWrite(t, filepath.Join(plugin, ".codex-plugin", "plugin.json"), `{ "name": "spacedock", "version": "0.0.0", "requires-contract": ">=1,<2", "skills": "./skills/" } + mustWrite(t, filepath.Join(plugin, ".codex-plugin", "plugin.json"), + `{ "name": "spacedock", "version": "`+version+`", "requires-contract": ">=1,<2", "skills": "./skills/" } `) mustWrite(t, filepath.Join(plugin, "skills", "demo", "SKILL.md"), "---\nname: demo\ndescription: demo skill\n---\ndemo\n") return marketplace diff --git a/internal/contract/contract.go b/internal/contract/contract.go index 7aafff4d..1261720d 100644 --- a/internal/contract/contract.go +++ b/internal/contract/contract.go @@ -68,10 +68,14 @@ func (v Verdict) String() string { // Result carries a comparison's verdict and the operator-facing message. For // Compatible the message is a one-line "OK" report; for every mismatch it is the -// shared-shape actionable message with the per-class remedy. +// shared-shape actionable message with the per-class remedy. Hint is the opt-in +// upgrade hint for a compatible-but-behind plugin — empty unless the binary is a +// strictly-newer semver than the plugin. Doctor folds it into Message; the front +// door surfaces Hint alone (it stays silent on the bare OK line). type Result struct { Verdict Verdict Message string + Hint string } // ParseRange parses a requires-contract value of the form ">=N,= hi: return Result{Verdict: TooOldPlugin, Message: mismatchMessage(binaryVersion, pluginVersion, "Update the plugin to continue.", tooOldPluginRemedy(host))} default: - return Result{Verdict: Compatible, Message: fmt.Sprintf("OK: spacedock binary %s and plugin %s are compatible.", binaryVersion, pluginVersion)} + msg := fmt.Sprintf("OK: spacedock binary %s and plugin %s are compatible.", binaryVersion, pluginVersion) + hint := upgradeHint(host, pluginVersion, binaryVersion) + if hint != "" { + msg += "\n" + hint + } + return Result{Verdict: Compatible, Message: msg, Hint: hint} + } +} + +// upgradeHint returns the opt-in upgrade hint appended to a Compatible message +// when the binary's display semver is strictly newer than the plugin's — a +// plugin that still works (the contract is compatible) but is behind. It returns +// "" (no hint) unless BOTH versions are valid dotted-int semver and the binary +// is strictly greater: an unstamped `dev` binary, a non-semver, or an +// equal/older binary emits nothing, so the gate never fires a false "you must +// upgrade". The hint names the host-specific refresh command. +func upgradeHint(host, pluginVersion, binaryVersion string) string { + if semverCompare(binaryVersion, pluginVersion) <= 0 { + return "" + } + return fmt.Sprintf( + "A newer plugin is available — run spacedock install --host %s to refresh.", + host) +} + +// semverCompare orders two dotted-int versions (e.g. `0.20.0`), returning -1, 0, +// or 1. It returns 0 (treat as not-greater, so no hint) when EITHER side is not +// a valid dotted-int version — the defensive gate that keeps a non-semver (`dev`) +// or empty value from triggering the upgrade hint. Unlike the cli resolver's +// lexical fallback, a non-integer component here is a parse failure, not a +// lexical tiebreak: the hint must not fire on anything but a clean semver skew. +func semverCompare(a, b string) int { + an, aok := parseDottedInts(a) + bn, bok := parseDottedInts(b) + if !aok || !bok { + return 0 + } + for i := 0; i < len(an) || i < len(bn); i++ { + var av, bv int + if i < len(an) { + av = an[i] + } + if i < len(bn) { + bv = bn[i] + } + if av != bv { + if av < bv { + return -1 + } + return 1 + } + } + return 0 +} + +// parseDottedInts splits a dotted version into its integer components, reporting +// ok=false when the string is empty or any component is not a non-negative +// integer. A pre-release/build suffix (e.g. `-rc1`) makes its component +// non-integer and so fails the parse — the conservative read for the upgrade +// hint, which only fires on a clean numeric skew. +func parseDottedInts(v string) ([]int, bool) { + v = strings.TrimSpace(v) + if v == "" { + return nil, false + } + parts := strings.Split(v, ".") + out := make([]int, len(parts)) + for i, p := range parts { + n, err := strconv.Atoi(p) + if err != nil || n < 0 { + return nil, false + } + out[i] = n } + return out, true } // mismatchMessage assembles the shared-shape mismatch message: a header naming diff --git a/internal/contract/contract_test.go b/internal/contract/contract_test.go index 3a0a8b31..adfce674 100644 --- a/internal/contract/contract_test.go +++ b/internal/contract/contract_test.go @@ -96,6 +96,109 @@ func TestCompare(t *testing.T) { } } +// TestCompatibleUpgradeHint locks AC-3: when the binary and plugin display +// versions are both valid semver and the binary is strictly newer while the +// contract is compatible, the Compatible message carries an opt-in upgrade hint +// naming a newer plugin AND the host-specific `spacedock install --host {host}` +// command — but the verdict stays Compatible and the OK line is preserved. The +// hint NEVER fires on equal versions or a non-semver (`dev`) binary version. The +// trigger (the semver skew) comes from the version inputs, not the message. +func TestCompatibleUpgradeHint(t *testing.T) { + t.Run("behind-plugin-hints", func(t *testing.T) { + for _, host := range []string{"claude", "codex"} { + res := Compare(CONTRACT_VERSION, ">=1,<2", host, "", "0.19.8", "0.20.0") + if res.Verdict != Compatible { + t.Fatalf("host %s: verdict = %v, want Compatible (the hint must not change the verdict)", host, res.Verdict) + } + if !strings.Contains(res.Message, "OK: spacedock binary 0.20.0 and plugin 0.19.8 are compatible.") { + t.Fatalf("host %s: OK line not preserved alongside the hint: %q", host, res.Message) + } + if !strings.Contains(res.Message, "newer plugin") { + t.Fatalf("host %s: hint missing newer-plugin notice: %q", host, res.Message) + } + if !strings.Contains(res.Message, "spacedock install --host "+host) { + t.Fatalf("host %s: hint missing host install command: %q", host, res.Message) + } + } + }) + + // Negative: equal versions carry no hint — there is nothing to upgrade to. + t.Run("equal-version-no-hint", func(t *testing.T) { + res := Compare(CONTRACT_VERSION, ">=1,<2", "claude", "", "0.20.0", "0.20.0") + if res.Verdict != Compatible { + t.Fatalf("verdict = %v, want Compatible", res.Verdict) + } + if strings.Contains(res.Message, "newer plugin") || strings.Contains(res.Message, "spacedock install") { + t.Fatalf("equal versions must not emit an upgrade hint: %q", res.Message) + } + }) + + // Negative: an unstamped `dev` binary version is not valid semver — the hint + // must not fire (no false "you must upgrade" against a dev build). + t.Run("dev-binary-no-hint", func(t *testing.T) { + res := Compare(CONTRACT_VERSION, ">=1,<2", "claude", "", "0.19.8", "dev") + if res.Verdict != Compatible { + t.Fatalf("verdict = %v, want Compatible", res.Verdict) + } + if strings.Contains(res.Message, "newer plugin") || strings.Contains(res.Message, "spacedock install") { + t.Fatalf("dev binary version must not emit an upgrade hint: %q", res.Message) + } + }) + + // Negative: a binary OLDER than the plugin (but still contract-compatible) + // carries no hint — the hint is for a behind plugin, not a behind binary. + t.Run("older-binary-no-hint", func(t *testing.T) { + res := Compare(CONTRACT_VERSION, ">=1,<2", "claude", "", "0.21.0", "0.20.0") + if res.Verdict != Compatible { + t.Fatalf("verdict = %v, want Compatible", res.Verdict) + } + if strings.Contains(res.Message, "newer plugin") { + t.Fatalf("a newer plugin than the binary must not emit the behind-plugin hint: %q", res.Message) + } + }) + + // Positive across the single-vs-double-digit boundary: binary 0.10.0 is + // numerically NEWER than plugin 0.9.0 and MUST hint — but lexically "0.10.0" + // sorts BEFORE "0.9.0" ("1" < "9"), so a lexical-compare regression of + // semverCompare would wrongly suppress the hint here. Pins the integer compare. + t.Run("behind-plugin-double-digit-minor-hints", func(t *testing.T) { + res := Compare(CONTRACT_VERSION, ">=1,<2", "claude", "", "0.9.0", "0.10.0") + if res.Verdict != Compatible { + t.Fatalf("verdict = %v, want Compatible", res.Verdict) + } + if !strings.Contains(res.Message, "newer plugin") { + t.Fatalf("binary 0.10.0 is numerically newer than plugin 0.9.0 — the hint MUST fire: %q", res.Message) + } + }) + + // Negative mirror across the boundary: binary 0.9.0 is numerically OLDER than + // plugin 0.10.0 and MUST NOT hint — but lexically "0.9.0" sorts AFTER "0.10.0", + // so a lexical-compare regression would wrongly FIRE the hint on this older + // binary. The two boundary cases together RED any lexical compare. + t.Run("older-binary-double-digit-minor-no-hint", func(t *testing.T) { + res := Compare(CONTRACT_VERSION, ">=1,<2", "claude", "", "0.10.0", "0.9.0") + if res.Verdict != Compatible { + t.Fatalf("verdict = %v, want Compatible", res.Verdict) + } + if strings.Contains(res.Message, "newer plugin") { + t.Fatalf("binary 0.9.0 is numerically older than plugin 0.10.0 — the hint MUST NOT fire: %q", res.Message) + } + }) + + // Negative: a pre-release binary version (e.g. 0.20.0-rc1) is not clean + // dotted-int semver — the conservative gate emits no hint. Pins that + // parseDottedInts rejects a `-rc1` suffix rather than stripping it. + t.Run("prerelease-binary-no-hint", func(t *testing.T) { + res := Compare(CONTRACT_VERSION, ">=1,<2", "claude", "", "0.19.8", "0.20.0-rc1") + if res.Verdict != Compatible { + t.Fatalf("verdict = %v, want Compatible", res.Verdict) + } + if strings.Contains(res.Message, "newer plugin") || strings.Contains(res.Message, "spacedock install") { + t.Fatalf("a pre-release binary version must not emit an upgrade hint: %q", res.Message) + } + }) +} + // TestCompareMessageShape locks the shared mismatch-message shape: the leading // "Spacedock version mismatch" line names both display versions, and the message // ends with the "Run spacedock doctor" pointer — for every mismatch class except