diff --git a/.gitignore b/.gitignore index b7b32147..07902168 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,11 @@ dist/ # committed to the code branch (zero code-branch churn). A fresh clone re-creates # the checkout with `spacedock state init`. **/.spacedock-state/ + +# Agent and sandbox working dirs — never committed, and never treated as repo +# content by workflow discovery (the discovery walk reads these `.gitignore` +# directory patterns, so the agent-worktree `docs/dev` copies under .claude/ drop +# out rather than being counted as workflows). .safehouse is the launch sandbox +# profile; .claude holds Claude Code's agent worktrees (full repo checkouts). +.safehouse +.claude/ diff --git a/internal/cli/frontdoor.go b/internal/cli/frontdoor.go index bad12421..eb2106a7 100644 --- a/internal/cli/frontdoor.go +++ b/internal/cli/frontdoor.go @@ -15,6 +15,7 @@ import ( "github.com/spacedock-dev/spacedock/internal/contract" "github.com/spacedock-dev/spacedock/internal/safehouse" + "github.com/spacedock-dev/spacedock/internal/status" ) // bootstrapPrompt is the fixed launch-and-go message appended as the last inner @@ -117,6 +118,81 @@ func executableFile(path string) bool { return info.Mode().Perm()&0o111 != 0 } +// noPluginRemedy is the host-correct manual remedy printed when no plugin is +// installed and the operator opted out of auto-install with --no-install. It +// names the host's own install and bootstrap commands — a codex run never says +// `claude`. The caller owns this message (not gateHost) because the response to +// NoPluginFound is non-uniform: auto-install by default, refuse-and-instruct +// under --no-install. +func noPluginRemedy(host string) string { + return fmt.Sprintf( + "Spacedock: no installed %s plugin found. "+ + "Run `spacedock install --host %s` (or `spacedock %s --skip-contract-check` to bootstrap).", + host, host, host) +} + +// launchBanner writes a short pre-launch orientation banner to w before the host +// is handed control: the spacedock version, the workflow detected from dir, and a +// one-line orientation pointer. Callers suppress it on a resume (the operator is +// continuing a session, not starting one). +func launchBanner(host, dir string, w io.Writer) { + fmt.Fprintf(w, "spacedock %s · first officer launching %s\n", Version, host) + fmt.Fprintf(w, "Workflow: %s\n", detectedWorkflow(dir)) + fmt.Fprintf(w, "%s is starting as your first officer; run `spacedock status` inside the session for the queue.\n", host) +} + +// detectedWorkflow names the workflow the launch belongs to, found from ANY +// launch dir. When dir is inside a git repo, the whole repo is scanned downward +// (status.DiscoverWorkflows, which prunes the linked/agent-worktree + VCS noise), +// so launching from the repo root resolves the repo's real workflow rather than +// missing it. A single workflow is named by its path relative to the repo root +// (e.g. `docs/dev`); two or more report the count plus a pointer to `spacedock +// status`. With no enclosing git repo, a bounded walk-up names a workflow at or +// above dir. The result is never the cwd-relative `.`/`..`; "none detected +// (launching anyway)" is shown when no workflow is found. +func detectedWorkflow(dir string) string { + const noneDetected = "none detected (launching anyway)" + repoRoot := status.FindGitRoot(dir) + gitEntry := filepath.Join(repoRoot, ".git") + if dirExists(gitEntry) || fileExists(gitEntry) { // .git is a dir (repo) or a file (worktree gitlink) + workflows := status.DiscoverWorkflows(repoRoot) + switch len(workflows) { + case 0: + return noneDetected + case 1: + return workflowLabel(repoRoot, workflows[0]) + default: + return fmt.Sprintf("%d workflows detected (run `spacedock status` to pick)", len(workflows)) + } + } + // No enclosing git repo: a bounded walk-up names a workflow at or above dir. + if workflowDir, ok := status.DiscoverWorkflowDir(dir); ok { + return workflowLabel(repoRoot, workflowDir) + } + return noneDetected +} + +// workflowLabel renders a workflow dir as a recognizable path relative to base +// (e.g. `docs/dev` relative to the repo root), falling back to the workflow dir's +// own base name. It never returns `.` or a `..`-escaping path. Both paths are +// resolved through symlinks first so a base that is a symlinked parent (e.g. +// macOS's /var → /private/var temp dirs) still yields the clean relative path +// rather than a spurious `..`-escape. +func workflowLabel(base, workflowDir string) string { + if rel, err := filepath.Rel(realpath(base), realpath(workflowDir)); err == nil && rel != "." && !strings.HasPrefix(rel, "..") { + return rel + } + return filepath.Base(workflowDir) +} + +// realpath resolves path through symlinks, returning the original on error. +func realpath(path string) string { + if resolved, err := filepath.EvalSymlinks(path); err == nil { + return resolved + } + return path +} + // gateHost resolves the installed manifest for host and compares it against // CONTRACT_VERSION, returning the verdict so the caller can distinguish a // missing plugin (NoPluginFound — recoverable by installing) from an @@ -127,8 +203,9 @@ func executableFile(path string) bool { // inspects the VERDICT, not a doctor exit code: RunDoctor maps no-plugin-found to // exit 0 (a non-fatal report), so a non-empty installPath to a missing manifest // would otherwise slip through as "compatible". gateHost prints the actionable -// remedy for every non-Compatible verdict; the caller decides whether to act on -// it (auto-install) or fail fast. +// 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). func gateHost(ops hostOps, host string, stderr io.Writer) contract.Verdict { manifestPath, err := ops.ResolveManifest(host) if err != nil { @@ -138,17 +215,10 @@ func gateHost(ops hostOps, host string, stderr io.Writer) contract.Verdict { return contract.MalformedRange } if manifestPath == "" { - fmt.Fprintf(stderr, - "Spacedock: no installed %s plugin found. "+ - "Run `spacedock install --host %s` (or `spacedock claude --skip-contract-check` to bootstrap).\n", host, host) return contract.NoPluginFound } res := contract.ManifestVerdict(manifestPath, host, devBranch, Version) if res.Verdict == contract.NoPluginFound { - fmt.Fprintf(stderr, - "Spacedock: the installed %s plugin reported a manifest path that does not exist (%s). "+ - "Run `spacedock install --host %s` (or `spacedock claude --skip-contract-check` to bootstrap).\n", - host, manifestPath, host) return contract.NoPluginFound } if res.Verdict != contract.Compatible { @@ -193,11 +263,14 @@ func runClaude(ctx context.Context, args []string, dir string, ops hostOps, look case contract.NoPluginFound: // No plugin on disk: the single command the user typed should yield a // working session. Auto-install the plugin then launch, unless the - // operator opted out with --no-install (gateHost already printed the - // instruct remedy, so just fail fast). + // operator opted out with --no-install. The caller owns the NoPluginFound + // message (gateHost stays silent for it): announce the install on the + // default arm, print the host-correct manual remedy on the refuse arm. if fd.noInstall { + fmt.Fprintln(stderr, noPluginRemedy("claude")) return 1 } + fmt.Fprintf(stderr, "Installing the %s plugin…\n", "claude") if _, err := ops.Install("claude", marketplaceSource, devBranch); err != nil { fmt.Fprintf(stderr, "spacedock claude: auto-install failed: %v\n", err) return 1 @@ -213,6 +286,9 @@ func runClaude(ctx context.Context, args []string, dir string, ops hostOps, look wrap := safehouse.Present(dir) || fd.forceSafehouse || len(fd.safehouseFlags) > 0 resume := containsResume(fd.passthrough) + if !resume { + launchBanner("claude", dir, stderr) + } inner := []string{"claude"} if wrap { inner = append(inner, "--dangerously-skip-permissions") @@ -345,11 +421,14 @@ func runCodex(ctx context.Context, args []string, dir string, ops hostOps, lookP // proceed to launch case contract.NoPluginFound: // No plugin on disk: auto-install then launch, unless the operator opted - // out with --no-install (gateHost already printed the instruct remedy, so - // just fail fast). + // out with --no-install. The caller owns the NoPluginFound message + // (gateHost stays silent for it): announce the install on the default arm, + // print the host-correct manual remedy on the refuse arm. if fd.noInstall { + fmt.Fprintln(stderr, noPluginRemedy("codex")) return 1 } + fmt.Fprintf(stderr, "Installing the %s plugin…\n", "codex") if _, err := ops.Install("codex", marketplaceSource, devBranch); err != nil { fmt.Fprintf(stderr, "spacedock codex: auto-install failed: %v\n", err) return 1 @@ -365,6 +444,9 @@ func runCodex(ctx context.Context, args []string, dir string, ops hostOps, lookP wrap := safehouse.Present(dir) || fd.forceSafehouse || len(fd.safehouseFlags) > 0 resume := codexResume(fd.passthrough) + if !resume { + launchBanner("codex", dir, stderr) + } inner := []string{"codex"} if wrap { inner = append(inner, "--dangerously-bypass-approvals-and-sandbox") diff --git a/internal/cli/frontdoor_test.go b/internal/cli/frontdoor_test.go index dce2623a..4ae272eb 100644 --- a/internal/cli/frontdoor_test.go +++ b/internal/cli/frontdoor_test.go @@ -295,35 +295,53 @@ func TestClaudeFrontDoorNonEmptyMissingManifestAutoInstalls(t *testing.T) { }) } -// TestGateRemedyNamesLiveInstallCommand: every gateHost remedy must point at a -// command the binary actually recognizes. After the init->install rename a user -// who hits the gate and is told to "run spacedock init --host …" runs a command -// that now exits 2 (unknown command). Drive each remedy branch (resolve error, -// no plugin, missing manifest) and assert the printed remedy names `spacedock -// install` and never `spacedock init`; then prove the named command resolves by -// feeding it through cli.Run and asserting it is not the unknown-command exit 2. +// TestGateRemedyNamesLiveInstallCommand: every remedy must point at a command +// the binary actually recognizes. After the init->install rename a user who hits +// the gate and is told to "run spacedock init --host …" runs a command that now +// exits 2 (unknown command). gateHost owns only the always-fail-fast remedies +// (resolve error → MalformedRange); the NoPluginFound message is the caller's +// (it auto-installs by default, refuses under --no-install), so the no-plugin / +// missing-manifest remedies are asserted on the launcher's --no-install output. +// Each remedy must name `spacedock install` and never `spacedock init`. func TestGateRemedyNamesLiveInstallCommand(t *testing.T) { missing := filepath.Join(t.TempDir(), "no-such-dir", ".claude-plugin", "plugin.json") - cases := []struct { - name string - fake *fakeHost + + // gateHost owns the resolve-error remedy (a host-CLI failure is a hard fail, + // not a missing plugin). It MUST print here. + t.Run("resolve error (gateHost-owned)", func(t *testing.T) { + var stderr bytes.Buffer + if v := gateHost(&fakeHost{resolveErr: errors.New("host CLI failed")}, "claude", &stderr); v == contract.Compatible { + t.Fatalf("gateHost = Compatible, want denied") + } + assertRemedyNamesInstall(t, stderr.String()) + }) + + // The NoPluginFound remedies (no plugin, phantom manifest) are caller-owned: + // gateHost no longer prints for them, so assert the remedy on the launcher's + // --no-install refuse output. + noPluginCases := []struct { + name string + manifest string }{ - {"resolve error", &fakeHost{resolveErr: errors.New("host CLI failed")}}, - {"no plugin", &fakeHost{manifest: ""}}, - {"missing manifest", &fakeHost{manifest: missing}}, + {"no plugin", ""}, + {"missing manifest", missing}, } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - var stderr bytes.Buffer - if v := gateHost(tc.fake, "claude", &stderr); v == contract.Compatible { - t.Fatalf("gateHost = Compatible, want denied for %s", tc.name) + for _, tc := range noPluginCases { + t.Run(tc.name+" (caller-owned via --no-install)", func(t *testing.T) { + fake := &fakeHost{manifest: tc.manifest} + var stdout, stderr bytes.Buffer + if code := runClaude(context.Background(), []string{"--no-install"}, t.TempDir(), fake, lookFound, &stdout, &stderr); code == 0 { + t.Fatalf("exit = 0, want non-zero with --no-install and no plugin") } - remedy := stderr.String() - if !strings.Contains(remedy, "spacedock install") { - t.Fatalf("remedy does not name the live install command: %q", remedy) + assertRemedyNamesInstall(t, stderr.String()) + }) + t.Run(tc.name+" (gateHost stays silent)", func(t *testing.T) { + var stderr bytes.Buffer + if v := gateHost(&fakeHost{manifest: tc.manifest}, "claude", &stderr); v != contract.NoPluginFound { + t.Fatalf("gateHost verdict = %v, want NoPluginFound", v) } - if strings.Contains(remedy, "spacedock init") { - t.Fatalf("remedy names the removed init command (exits 2): %q", remedy) + if stderr.Len() != 0 { + t.Fatalf("gateHost printed for NoPluginFound (caller owns the message): %q", stderr.String()) } }) } @@ -343,6 +361,118 @@ func TestGateRemedyNamesLiveInstallCommand(t *testing.T) { } } +// TestNoPluginAutoInstallAnnouncesHostCorrectly (AC-A, auto-install arm): with +// no installed plugin the launcher announces `Installing the {host} plugin…` on +// stderr before installing, and a codex run never names `claude` (the old +// gateHost remedy hardcoded a `spacedock claude` hint that was wrong in a codex +// run). Install + launch are observed via the recorded seams, not a string +// match. +func TestNoPluginAutoInstallAnnouncesHostCorrectly(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, func(t *testing.T) { + fake := &fakeHost{manifest: ""} // no plugin found + var stderr bytes.Buffer + + code := tc.run(nil, t.TempDir(), fake, &stderr) + + if code != 0 { + t.Fatalf("exit = %d, want 0 (auto-install + launch) (stderr=%q)", code, stderr.String()) + } + want := "Installing the " + tc.host + " plugin" + if !strings.Contains(stderr.String(), want) { + t.Fatalf("stderr missing %q announcement: %q", want, stderr.String()) + } + if tc.host == "codex" && strings.Contains(stderr.String(), "spacedock claude") { + t.Fatalf("codex auto-install stderr names claude: %q", stderr.String()) + } + if len(fake.installCmds) == 0 { + t.Fatalf("install seam not invoked: auto-install did not run") + } + if fake.launchedArg == nil { + t.Fatalf("launch seam not invoked after auto-install") + } + }) + } +} + +// TestNoPluginNoInstallRemedyIsHostCorrect (AC-A, refuse arm): with --no-install +// and no plugin the launcher prints the manual remedy naming the host-correct +// install/bootstrap commands (`spacedock install --host {host}`, `spacedock +// {host} --skip-contract-check`) and NEVER a `claude` hint in a codex run, then +// exits non-zero without installing or launching. This is the message the caller +// now owns (gateHost stopped printing for NoPluginFound). +func TestNoPluginNoInstallRemedyIsHostCorrect(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, func(t *testing.T) { + fake := &fakeHost{manifest: ""} // no plugin found + var stderr bytes.Buffer + + code := tc.run([]string{"--no-install"}, t.TempDir(), fake, &stderr) + + if code == 0 { + t.Fatalf("exit = 0, want non-zero with --no-install and no plugin") + } + out := stderr.String() + if !strings.Contains(out, "spacedock install --host "+tc.host) { + t.Fatalf("remedy missing host-correct install command for %s: %q", tc.host, out) + } + if !strings.Contains(out, "spacedock "+tc.host+" --skip-contract-check") { + t.Fatalf("remedy missing host-correct bootstrap command for %s: %q", tc.host, out) + } + if tc.host == "codex" && strings.Contains(out, "claude") { + t.Fatalf("codex --no-install remedy names claude: %q", out) + } + if len(fake.installCmds) != 0 { + t.Fatalf("install seam invoked despite --no-install: %v", fake.installCmds) + } + if fake.launchedArg != nil { + t.Fatalf("launch seam invoked despite --no-install: %v", fake.launchedArg) + } + }) + } +} + +// assertRemedyNamesInstall: every gate / refuse remedy must name the live +// `spacedock install` command and never the removed `spacedock init` (which now +// exits 2). +func assertRemedyNamesInstall(t *testing.T, remedy string) { + t.Helper() + if !strings.Contains(remedy, "spacedock install") { + t.Fatalf("remedy does not name the live install command: %q", remedy) + } + if strings.Contains(remedy, "spacedock init") { + t.Fatalf("remedy names the removed init command (exits 2): %q", remedy) + } +} + // TestClaudeFrontDoorSkipContractCheckBootstrap: the --skip-contract-check // override launches without resolving the manifest (bootstrap case where the // plugin is being installed for the first time). diff --git a/internal/cli/launch_banner_test.go b/internal/cli/launch_banner_test.go new file mode 100644 index 00000000..8e3639ed --- /dev/null +++ b/internal/cli/launch_banner_test.go @@ -0,0 +1,232 @@ +// ABOUTME: AC-B oracles for the pre-launch info banner — version line + detected +// ABOUTME: workflow (repo-wide discovery, noise-pruned) vs none/multiple. +package cli + +import ( + "bytes" + "context" + "os" + "path/filepath" + "strings" + "testing" +) + +// commissionWorkflowAt writes a README.md whose frontmatter declares +// `commissioned-by: spacedock@…` — the same predicate the workflow discovery +// recognizes — at the given absolute dir, creating it first. +func commissionWorkflowAt(t *testing.T, dir string) { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + readme := "---\ncommissioned-by: spacedock@1.0\nid-style: sequential\n---\n# WF\n" + if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte(readme), 0o644); err != nil { + t.Fatal(err) + } +} + +// gitRepoFixture returns a temp dir holding a `.git` directory so FindGitRoot +// resolves it as the enclosing repository root. +func gitRepoFixture(t *testing.T) string { + t.Helper() + repo := t.TempDir() + if err := os.Mkdir(filepath.Join(repo, ".git"), 0o755); err != nil { + t.Fatal(err) + } + return repo +} + +// TestLaunchBannerNamesDetectedWorkflow (AC-B): the banner finds the repo's REAL +// top-level workflow from ANY launch dir (the enclosing git repo is scanned +// downward, with linked/agent-worktree + VCS noise pruned), and names it relative +// to the repo root (e.g. `docs/dev`). From the repo root, from inside the +// workflow, and from a deep subdir it names the same workflow. A noise copy under +// `.claude/worktrees/...` (a real concern: agent worktrees are full repo checkouts +// each carrying a `docs/dev`) is NOT named and does NOT inflate the count. Outside +// any workflow it reads "none detected"; with more than one top-level workflow it +// reads the count + a pointer to `spacedock status`. The fixture layout is the +// independent expected value. +func TestLaunchBannerNamesDetectedWorkflow(t *testing.T) { + // repoWithRealWorkflowAndNoise builds a temp git repo with the single real + // workflow at /docs/dev plus noise copies that each exercise an exclusion + // MECHANISM (not a hardcoded skip): the `.claude/worktrees/agent-x/docs/dev` + // copy is dropped because the repo `.gitignore` lists `.claude/`, and the + // `.worktrees/wt-y/docs/dev` copy is dropped because its root is a nested git + // checkout (a `.git` gitlink). Only docs/dev is a real top-level workflow. + gitlinkAt := func(t *testing.T, dir string) { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, ".git"), []byte("gitdir: /elsewhere\n"), 0o644); err != nil { + t.Fatal(err) + } + } + repoWithRealWorkflowAndNoise := func(t *testing.T) (repo, realWorkflow string) { + t.Helper() + repo = gitRepoFixture(t) + if err := os.WriteFile(filepath.Join(repo, ".gitignore"), []byte(".claude/\n.safehouse\n"), 0o644); err != nil { + t.Fatal(err) + } + realWorkflow = filepath.Join(repo, "docs", "dev") + commissionWorkflowAt(t, realWorkflow) + // (a) gitignored .claude subtree — no .git at the copy; the .gitignore prunes it. + commissionWorkflowAt(t, filepath.Join(repo, ".claude", "worktrees", "agent-x", "docs", "dev")) + // (b) a nested linked worktree — its .git gitlink stops the descent. + gitlinkAt(t, filepath.Join(repo, ".worktrees", "wt-y")) + commissionWorkflowAt(t, filepath.Join(repo, ".worktrees", "wt-y", "docs", "dev")) + return repo, realWorkflow + } + + t.Run("from the repo root names the real workflow, not the .claude/.worktrees noise", func(t *testing.T) { + repo, _ := repoWithRealWorkflowAndNoise(t) + var buf bytes.Buffer + launchBanner("claude", repo, &buf) + + out := buf.String() + if !strings.Contains(out, "spacedock "+Version) { + t.Fatalf("banner missing version line naming %q: %q", "spacedock "+Version, out) + } + if !strings.Contains(out, "Workflow: "+filepath.Join("docs", "dev")+"\n") { + t.Fatalf("banner from repo root does not name the real workflow docs/dev: %q", out) + } + if strings.Contains(out, "none detected") { + t.Fatalf("banner reads none detected from the repo root of a repo with a real workflow: %q", out) + } + if strings.Contains(out, "workflows detected") { + t.Fatalf("banner counted the .claude/.worktrees noise copies as extra workflows: %q", out) + } + }) + + t.Run("from a deep subdir of the workflow names the same workflow", func(t *testing.T) { + repo, realWorkflow := repoWithRealWorkflowAndNoise(t) + sub := filepath.Join(realWorkflow, "nested", "deep") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + launchBanner("codex", sub, &buf) + + out := buf.String() + if !strings.Contains(out, "Workflow: "+filepath.Join("docs", "dev")+"\n") { + t.Fatalf("banner from a deep subdir of repo %s does not name docs/dev: %q", repo, out) + } + }) + + t.Run("more than one real top-level workflow reports the count + status pointer", func(t *testing.T) { + repo := gitRepoFixture(t) + commissionWorkflowAt(t, filepath.Join(repo, "docs", "dev")) + commissionWorkflowAt(t, filepath.Join(repo, "ops", "release")) + var buf bytes.Buffer + launchBanner("claude", repo, &buf) + + out := buf.String() + if !strings.Contains(out, "2 workflows detected (run `spacedock status` to pick)") { + t.Fatalf("banner does not report the 2-workflow count + pointer: %q", out) + } + if strings.Contains(out, "none detected") { + t.Fatalf("banner reads none detected with two real workflows: %q", out) + } + }) + + t.Run("commissioned workflow with no enclosing git repo falls back to the workflow name", func(t *testing.T) { + // A workflow dir with no `.git` on the way up: the discovery falls back to + // the bounded walk-up and renders the workflow dir's own name — never `.`/`..`. + workflow := t.TempDir() + commissionWorkflowAt(t, workflow) + var buf bytes.Buffer + launchBanner("codex", workflow, &buf) + + out := buf.String() + if !strings.Contains(out, "Workflow: "+filepath.Base(workflow)+"\n") { + t.Fatalf("banner fallback does not name the workflow dir base %q: %q", filepath.Base(workflow), out) + } + if strings.Contains(out, "Workflow: .\n") || strings.Contains(out, "Workflow: ..") { + t.Fatalf("banner fallback is the cwd-relative `.`/`..` form: %q", out) + } + if strings.Contains(out, "none detected") { + t.Fatalf("banner reads none detected inside a commissioned workflow: %q", out) + } + }) + + t.Run("outside any workflow", func(t *testing.T) { + bare := t.TempDir() // no commissioned README in or above this temp dir + var buf bytes.Buffer + launchBanner("codex", bare, &buf) + + out := buf.String() + if !strings.Contains(out, "spacedock "+Version) { + t.Fatalf("banner missing version line naming %q: %q", "spacedock "+Version, out) + } + if !strings.Contains(out, "none detected") { + t.Fatalf("banner does not read none detected outside a workflow: %q", out) + } + }) +} + +// TestLaunchBannerReachesStderrBeforeLaunch (AC-B launcher half): the banner's +// version line reaches stderr on a real launch (gate compatible, no resume), and +// the launch seam is reached. The fakeHost records Launch, so a banner on stderr +// PLUS a recorded launch proves the banner was emitted before the host launched. +func TestLaunchBannerReachesStderrBeforeLaunch(t *testing.T) { + cases := []struct { + name string + run func(dir string, fake *fakeHost, stderr *bytes.Buffer) int + }{ + {"claude", func(dir string, fake *fakeHost, stderr *bytes.Buffer) int { + var stdout bytes.Buffer + return runClaude(context.Background(), nil, dir, fake, lookFound, &stdout, stderr) + }}, + {"codex", func(dir string, fake *fakeHost, stderr *bytes.Buffer) int { + var stdout bytes.Buffer + return runCodex(context.Background(), nil, dir, fake, lookFound, &stdout, stderr) + }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fake := &fakeHost{manifest: compatibleManifest(t)} + var stderr bytes.Buffer + + code := tc.run(t.TempDir(), fake, &stderr) + + if code != 0 { + t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) + } + if !strings.Contains(stderr.String(), "spacedock "+Version) { + t.Fatalf("banner version line did not reach stderr: %q", stderr.String()) + } + if fake.launchedArg == nil { + t.Fatalf("launch seam not reached after banner") + } + }) + } +} + +// TestLaunchBannerSuppressedOnResume (AC-B polish): a resume continues an +// existing session, not a fresh launch, so the banner is suppressed — its +// version line must NOT appear on stderr. claude's resume is a flag; codex's is +// the leading `resume` subcommand. +func TestLaunchBannerSuppressedOnResume(t *testing.T) { + t.Run("claude --resume", func(t *testing.T) { + fake := &fakeHost{manifest: compatibleManifest(t)} + var stdout, stderr bytes.Buffer + if code := runClaude(context.Background(), []string{"--", "--resume"}, t.TempDir(), fake, lookFound, &stdout, &stderr); code != 0 { + t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) + } + if strings.Contains(stderr.String(), "spacedock "+Version) { + t.Fatalf("banner emitted on --resume (should be suppressed): %q", stderr.String()) + } + }) + + t.Run("codex resume subcommand", func(t *testing.T) { + dir := safehouseFixtureDir(t) + fake := &fakeHost{manifest: compatibleManifest(t)} + var stdout, stderr bytes.Buffer + if code := runCodex(context.Background(), []string{"--", "resume", "abc123"}, dir, fake, lookFound, &stdout, &stderr); code != 0 { + t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) + } + if strings.Contains(stderr.String(), "spacedock "+Version) { + t.Fatalf("banner emitted on codex resume (should be suppressed): %q", stderr.String()) + } + }) +} diff --git a/internal/contract/doctor.go b/internal/contract/doctor.go index 3c20ab94..15a55d09 100644 --- a/internal/contract/doctor.go +++ b/internal/contract/doctor.go @@ -77,8 +77,6 @@ func RunDoctor(manifestPath, host, branch, binaryVersion string, stdout, stderr switch res.Verdict { case Compatible: fmt.Fprintln(stdout, res.Message) - fmt.Fprintln(stdout, "Note: hosts emit a load-time warning for the 'requires-contract' field; "+ - "this is expected — the host ignores the field and spacedock reads it itself.") return 0 case NoPluginFound: fmt.Fprintln(stdout, res.Message) diff --git a/internal/status/discover_worktree_noise_test.go b/internal/status/discover_worktree_noise_test.go new file mode 100644 index 00000000..e272ee7a --- /dev/null +++ b/internal/status/discover_worktree_noise_test.go @@ -0,0 +1,134 @@ +// ABOUTME: discoverWorkflows prunes agent/linked-worktree noise via .gitignore +// ABOUTME: dir patterns AND nested-checkout (.git) skip — not a hardcoded list. +package status + +import ( + "os" + "path/filepath" + "testing" +) + +// TestDiscoverWorkflowsPrunesWorktreeNoise: agent and linked worktrees are full +// repo checkouts that each carry a copy of the repo's commissioned workflow +// (e.g. `docs/dev`). They live under `.claude/worktrees//…` (an untracked +// agent dir the repo `.gitignore` lists as `.claude/`) and `.worktrees//…` +// (each a linked worktree rooted by its own `.git` gitlink). Discovery must +// resolve the ONE real workflow by two composable mechanisms, NOT a hardcoded +// path skip: (a) honoring the `.gitignore` directory patterns so the `.claude` +// subtree drops out, and (b) not descending into a nested git checkout. +func TestDiscoverWorkflowsPrunesWorktreeNoise(t *testing.T) { + repo := t.TempDir() + commissioned := "---\ncommissioned-by: spacedock@1.0\nid-style: sequential\n---\n# WF\n" + write := func(rel string) { + p := filepath.Join(repo, rel, "README.md") + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(commissioned), 0o644); err != nil { + t.Fatal(err) + } + } + // gitlink plants a `.git` regular file at a worktree root, matching how git + // records a linked worktree (a `gitdir: …` pointer file, not a directory). + gitlink := func(rel string) { + p := filepath.Join(repo, rel) + if err := os.MkdirAll(p, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(p, ".git"), []byte("gitdir: /elsewhere\n"), 0o644); err != nil { + t.Fatal(err) + } + } + + // The repo .gitignore lists .claude/ as an ignored directory — the mechanism + // that drops the agent-worktree copies under it (no .git needed at that copy). + if err := os.WriteFile(filepath.Join(repo, ".gitignore"), []byte(".claude/\n"), 0o644); err != nil { + t.Fatal(err) + } + + // (real) the single workflow, part of the main checkout (no own .git). + write(filepath.Join("docs", "dev")) + // (a) gitignored .claude subtree — pruned by the .gitignore dir pattern. + write(filepath.Join(".claude", "worktrees", "agent-a", "docs", "dev")) + write(filepath.Join(".claude", "worktrees", "agent-b", "docs", "dev")) + // (b) a linked worktree rooted by a .git gitlink — pruned as a nested checkout. + gitlink(filepath.Join(".worktrees", "wt-x")) + write(filepath.Join(".worktrees", "wt-x", "docs", "dev")) + + got := discoverWorkflows(repo) + want := realpathOf(filepath.Join(repo, "docs", "dev")) + if len(got) != 1 || got[0] != want { + t.Fatalf("discoverWorkflows = %v, want exactly [%s] (worktree noise must be pruned)", got, want) + } +} + +// TestDiscoverWorkflowsHonorsGitignoreDirPattern isolates mechanism (a): a noise +// workflow copy under a dir the .gitignore lists is excluded purely by the +// gitignore pattern — no `.git` gitlink at the copy, no hardcoded basename. +func TestDiscoverWorkflowsHonorsGitignoreDirPattern(t *testing.T) { + repo := t.TempDir() + commissioned := "---\ncommissioned-by: spacedock@1.0\nid-style: sequential\n---\n# WF\n" + write := func(rel string) { + p := filepath.Join(repo, rel, "README.md") + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(commissioned), 0o644); err != nil { + t.Fatal(err) + } + } + // An arbitrarily-named ignored dir (not in discoverIgnoreDirs) proves the + // gitignore pattern — not a hardcoded list — does the exclusion. + if err := os.WriteFile(filepath.Join(repo, ".gitignore"), []byte("agent-cache/\n"), 0o644); err != nil { + t.Fatal(err) + } + write(filepath.Join("docs", "dev")) + write(filepath.Join("agent-cache", "copy", "docs", "dev")) + + got := discoverWorkflows(repo) + want := realpathOf(filepath.Join(repo, "docs", "dev")) + if len(got) != 1 || got[0] != want { + t.Fatalf("discoverWorkflows = %v, want exactly [%s] (gitignore dir pattern must prune the copy)", got, want) + } +} + +// TestDiscoverWorkflowsSkipsNestedCheckout isolates mechanism (b): a nested git +// checkout's workflow copy is pruned by the `.git`-presence skip ALONE, with no +// help from a basename or gitignore rule. The copy lives under `submods/checkout-z` +// — a dir name that is NEITHER in discoverIgnoreDirs NOR any `.gitignore` — so the +// ONLY thing that can drop it is hasGitEntry seeing the nested `.git` gitlink. This +// is the unmasked check: neuter hasGitEntry → `return false` and this test reds +// (the copy re-appears as a second workflow), which the existing `.worktrees`-sited +// fixtures do NOT (the `.worktrees` basename in discoverIgnoreDirs masks them). +func TestDiscoverWorkflowsSkipsNestedCheckout(t *testing.T) { + repo := t.TempDir() + commissioned := "---\ncommissioned-by: spacedock@1.0\nid-style: sequential\n---\n# WF\n" + write := func(rel string) { + p := filepath.Join(repo, rel, "README.md") + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(commissioned), 0o644); err != nil { + t.Fatal(err) + } + } + + // The real workflow, part of the main checkout (no own .git). + write(filepath.Join("docs", "dev")) + // A nested checkout under a NON-pruned, NON-gitignored dir name, rooted by a + // `.git` gitlink — pruned only by hasGitEntry. + nested := filepath.Join(repo, "submods", "checkout-z") + if err := os.MkdirAll(nested, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(nested, ".git"), []byte("gitdir: /elsewhere\n"), 0o644); err != nil { + t.Fatal(err) + } + write(filepath.Join("submods", "checkout-z", "docs", "dev")) + + got := discoverWorkflows(repo) + want := realpathOf(filepath.Join(repo, "docs", "dev")) + if len(got) != 1 || got[0] != want { + t.Fatalf("discoverWorkflows = %v, want exactly [%s] (nested-checkout .git skip must prune the copy)", got, want) + } +} diff --git a/internal/status/handlers.go b/internal/status/handlers.go index c1c623ec..a122391b 100644 --- a/internal/status/handlers.go +++ b/internal/status/handlers.go @@ -523,18 +523,29 @@ func discoverWorkflows(root string) []string { } } for _, ent := range entries { + child := filepath.Join(dir, ent.Name()) // os.Stat follows symlinks, matching os.walk(followlinks=True). - st, err := os.Stat(filepath.Join(dir, ent.Name())) + st, err := os.Stat(child) if err != nil || !st.IsDir() { continue } if ignore[ent.Name()] { continue } - if prunedStateDirs[realpathOf(filepath.Join(dir, ent.Name()))] { + if prunedStateDirs[realpathOf(child)] { continue } - walk(filepath.Join(dir, ent.Name())) + // Skip a nested git checkout (a linked or agent worktree): it is a full + // copy of the repo, so any commissioned workflow inside it is a duplicate + // of one the outer repo already carries, not a distinct workflow. This is + // the host-neutral generalization of the .worktrees prune — it catches + // agent worktrees under any directory (e.g. `.claude/worktrees/`) + // without naming a host-specific path. The start root's own `.git` is never + // inspected here: this check runs only on descended children. + if hasGitEntry(child) { + continue + } + walk(child) } } walk(absRoot) @@ -543,6 +554,23 @@ func discoverWorkflows(root string) []string { return results } +// hasGitEntry reports whether dir holds a `.git` entry — a directory (a normal +// repository checkout) or a regular file (a linked/agent worktree's gitlink). A +// dir with one is a self-contained checkout, so the discovery walk does not +// descend into it (its workflows are copies of the outer repo's). +func hasGitEntry(dir string) bool { + st, err := os.Stat(filepath.Join(dir, ".git")) + return err == nil && (st.IsDir() || st.Mode().IsRegular()) +} + +// DiscoverWorkflows returns the commissioned workflow dirs under root (realpath'd, +// sorted), with the same linked/agent-worktree + VCS noise pruned as the +// `--discover` boot walk — so a caller outside this package (e.g. the front-door +// launch banner) resolves the same single real workflow the first officer sees. +func DiscoverWorkflows(root string) []string { + return discoverWorkflows(root) +} + // runGitCmd runs git in dir and returns stdout, or an error on failure. func runGitCmd(dir string, args ...string) (string, error) { cmd := exec.Command("git", args...)