From 5c828e1aef7332a79c224e513c70ed9b019032be Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 20:59:49 -0400 Subject: [PATCH 01/12] Add design spec: Choose agent harnesses during init and with configure-harnesses --- ...harnesses-during-repository-init-design.md | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-07-choose-agent-harnesses-during-repository-init-design.md diff --git a/docs/superpowers/specs/2026-10-07-choose-agent-harnesses-during-repository-init-design.md b/docs/superpowers/specs/2026-10-07-choose-agent-harnesses-during-repository-init-design.md new file mode 100644 index 000000000..deae374ef --- /dev/null +++ b/docs/superpowers/specs/2026-10-07-choose-agent-harnesses-during-repository-init-design.md @@ -0,0 +1,135 @@ +# Choose agent harnesses during init and with configure-harnesses: design + +Change 0538 — Choose agent harnesses during init and with configure-harnesses + +Change #538, groomed interactively on 2026-10-06. Discovered while hand-checking #535. Related: #351 (only a repository-level `agent_harnesses` authorizes repository writes), #531 (private mode), #533 (switching visibility). + +## Summary + +`docket repository init` writes the parent-facing dispatch instructions only when the repository declares `agent_harnesses`, but nothing ever asks for that setting, and nothing says when it is missing. This change adds one shared "choose harnesses" step with two entry points: + +- `docket repository init` runs it on first setup: from `--harnesses`, from an interactive checkbox picker on a terminal, or, with neither, it succeeds and warns. +- A new `docket repository configure-harnesses` runs it any time afterwards, without init's clean-checkout and at-remote-tip precondition. + +The step writes `agent_harnesses` to the repository-level config and refreshes the instructions in the same run, through the machinery `docket install` already uses. `docket repository check` gains a non-blocking warning while the key is absent. + +## Evidence gathered at grooming + +- **Init already writes the instructions when authorized.** `RunRepositoryInit` calls `installAuthorizedSurfaces` (shared mode only when `facts.SurfacesAuthorized`; private mode unconditionally, after `ensurePrivateConfig`). That function runs `ResolveRepoPhase` and `applyRepoPhaseSurfaces`, the same planning and proof-gated removals as `docket install`. The gap is only that the setting is never asked for and its absence is silent. +- **Authority rule (#351).** `ResolveRepoPhase`, `gatherRepoFacts` and `reposetup.authorizedHarnesses` each authorize only `Explicit && isRepositoryLayer(...)`: `.docket.yml`, `.docket.local.yml`, or the private `.git/dckt/config.yml`. A global value resolves but never authorizes. Absent means touch nothing; explicit `[]` retires every surface docket owns. +- **Shared init's authorization is decided too early.** `facts.SurfacesAuthorized` is computed in `GatherSetupFacts` before init writes anything, so a key written during init would not authorize that same run's surface install. +- **The clean-checkout check ignores docket-managed paths.** `primaryCleanPresence` skips `docketManagedWorktreePaths` (`.gitignore`, `.docket.yml`, `CLAUDE.md`, `AGENTS.md`, `.cursor/rules/docket-dispatch.mdc`), so an uncommitted `.docket.yml` edit does not block init. +- **`configure-tests` is the pattern to follow.** `RunRepositoryConfigureTests` and init share `ensureTestPolicyConfig`; writes go through `writeTargetConfig` → `repoConfigTarget` → `writeRepoConfig` (shared: `.docket.yml`, unstaged, a pending path; private: `.git/dckt/config.yml`, nothing pending). Its guard `configureTestsGuard` admits only `StateHealthy`; a docket-managed uncommitted edit classifies as `needs-review`. +- **No writer emits a YAML sequence.** `reposetup/testconfigedit.go` (`renderOwnerPairs`, `verifyOwnerPairs`) is hard-wired to the `build:`/`finalize:` blocks; `configedit.go` (`RemoveMetadataBranchKey`) removes a top-level key by line splice using `topLevelMapping`, `maxNodeLine`, `spliceOutLines`. +- **Silence everywhere.** `repository check`, `status`, `diagnostic config` and the preflight say nothing about a missing key; `docket install` only lists a `notAuthorizedAction` keep line. +- **Detection exists.** `harness.Adapter.Detect` stats `~/.claude`, `~/.codex`, `~/.cursor`, `$XDG_CONFIG_HOME/opencode`. +- **No prompt UI exists.** The only prompt is the hand-rolled y/N for `migrate`/`repair` (`repositoryConfirmInteractive` checks `os.ModeCharDevice` on stdin). `go.mod` has four direct dependencies and no terminal library. +- **Binary-size measurement** (current `main`, a reachable call to each option): + + | Build | Unchanged | + `golang.org/x/term` | + `charmbracelet/huh` | + |---|---|---|---| + | default `go build` | 14,751 KB | 14,769 KB (+17 KB) | 16,556 KB (+1,762 KB, +12%) | + | stripped (`-s -w`) | 10,583 KB | 10,583 KB (+0.3 KB) | 11,950 KB (+1,335 KB, +13%) | + | new modules in the graph | | 2 | 42 | + +## Decisions + +1. **A dedicated command for later changes.** `docket repository configure-harnesses` changes the selection on an already set-up repository. Re-running `init` to change harnesses was rejected because it inherits init's clean and at-tip precondition; leaving later changes as a hand edit plus `docket install` was rejected as the status quo the change exists to fix. +2. **One shared step, two entry points.** Init does not invoke the command; both call the same internal function, the way init and `configure-tests` share `ensureTestPolicyConfig`. Flag parsing, validation, the picker, the write, the local-override warning and the refresh are identical; only two caller-supplied behaviours differ (below). +3. **An arrow-key checkbox picker built on `charmbracelet/huh`.** Chosen over a numbered line prompt and over a hand-rolled `golang.org/x/term` picker, with the size cost above known. It is docket's first interactive UI dependency, and later prompts reuse it. +4. **`repository check` warns while the key is absent.** "No agents" is a valid setup, so the warning fires only while no repository-level layer declares the key; writing `[]` or any list silences it permanently. It never blocks. +5. **Choosing none is a recorded decision.** `--harnesses none`, or confirming the picker with nothing checked, writes `agent_harnesses: []`. + +## Design + +### The shared step + +One app-layer function (working name `chooseHarnesses`) takes: + +- the explicit flag value, if `--harnesses` was given; +- a chooser: a function the CLI supplies when interactive, `nil` otherwise; +- two caller policies: + + | | `init` | `configure-harnesses` | + |---|---|---| + | a repository-level value exists and no flag | skip; keep the value | show the picker, pre-checked with it | + | no flag and no chooser | succeed and warn | refuse | + +It then: + +1. **Resolves the selection.** + - The flag wins. `--harnesses claude,cursor` (a comma list; repeating the flag also works) or `--harnesses none`. Every token is validated against the `agent_harnesses` allowed set (`claude`, `codex`, `cursor`, `opencode`) and duplicates are refused, before anything is written. `none` cannot be combined with a name. + - Otherwise, with a chooser, the picker runs, pre-checked with the first of: the repository-level value, the global `agent_harnesses`, the harnesses `harness.Adapter.Detect` finds. Options are listed in `harness.Order`. Confirming with nothing checked selects `[]`; cancelling (Ctrl-C) writes nothing and the command reports a cancellation. + - Otherwise the caller policy applies. Init's warning names the exact line to add (`agent_harnesses: [claude]` in `.docket.yml`, or in `.git/dckt/config.yml` for a private repository) and the command `docket repository configure-harnesses`. The command's refusal says to pass `--harnesses `. +2. **Writes the key** through `writeTargetConfig` / `repoConfigTarget`, unchanged: shared → `.docket.yml`, unstaged, reported as a pending path; private → `.git/dckt/config.yml`, nothing pending. Writing the value already present is a no-op. +3. **Refreshes the instructions** by calling `installAuthorizedSurfaces`, which re-resolves config from disk. Dropped harnesses lose their surfaces through the existing proof-gated removals; `[]` retires everything docket owns there. A surface conflict refuses exactly as it does for init and install today, mapped by `mapSurfaceFailure`. +4. **Warns on a local override.** In a shared repository whose `.docket.local.yml` also declares `agent_harnesses`, the write still goes to `.docket.yml`, and the result warns that `.docket.local.yml` overrides it and names the value that was actually applied. (Private mode already refuses a `.docket.local.yml` beside the private config.) + +### `docket repository init` + +- New flag `--harnesses`. +- The shared step runs after the managed `.gitignore` write and **before** the surface install. The shared-mode `if facts.SurfacesAuthorized` gate is replaced by authorization re-evaluated after the step, so a key written in this run authorizes this run's surfaces. Private mode keeps its unconditional call, now after the step. +- The picker is offered only when there is no flag, the CLI is interactive, and no repository-level layer declares the key. +- Init's guard (clean checkout, at the remote integration tip, layout states) is unchanged. + +### `docket repository configure-harnesses` (new) + +- Flags: `--harnesses`, plus the standard `--repo-dir` and `--json`. +- Guard: like `configureTestsGuard`, but it admits `needs-review` as well as `healthy`, so it can run right after `init` or twice in a row before the human commits. Fresh → "run `docket repository init`"; legacy → "run `docket repository migrate`"; any other state → "run `docket repository check`". +- No clean-checkout or at-tip precondition. +- Result: the standard `RepositoryOpResult`, with the written value, pending paths, surface actions and any warnings. + +### The config writer + +A new `reposetup` render function (working name `RenderAgentHarnessesEdit(existing []byte, harnesses []string)`): + +- Emits one flow-sequence line, `agent_harnesses: [claude, cursor]` or `agent_harnesses: []`. +- If the top-level key exists, in block or flow form, it replaces that key's lines (located with `topLevelMapping`, `maxNodeLine`, `spliceOutLines`); otherwise it appends the line at the end of the file. Every other byte is preserved. +- It refuses multi-document files, non-mapping roots and undecodable YAML, as `topLevelMapping` does. +- It re-parses the result and refuses unless the only change is `agent_harnesses` holding exactly the requested list, mirroring `verifyOwnerPairs`. +- It is given nothing to write when the value already matches. + +### Interactive UI + +- The picker lives in `internal/cli` as a thin adapter over `huh.NewMultiSelect`. The app layer never imports huh. +- The CLI supplies the chooser only when stdin **and** stdout are terminals and `--json` is off; otherwise it passes `nil`. Detection follows the existing `repositoryConfirmInteractive` seam style, so tests can override it. + +### `repository check` warning + +- A new warning condition in `reposetup/health.go`, code `harnesses-unset`, raised when `SurfacesAuthorized` is false (no repository-level layer declares `agent_harnesses`). It works for shared and private layouts alike. +- Message: docket writes no dispatch instructions here because `agent_harnesses` is unset. Remedy: `docket repository configure-harnesses` (or `--harnesses none` to record that no agents are used). +- Severity warning; it does not change the classified state and never blocks prepare, implement or any other operation. + +### Catalog and documentation + +- `repository.configure-harnesses` joins the capability catalog with effect `local-write`, plus its request/result schema. +- Command help for `init` and `configure-harnesses`, the `agent_harnesses` section of `.docket.example.yml`, and the convention's configuration prose name the command and the picker. Documentation describes current behavior only. + +## Acceptance criteria + +1. Writer: key absent → appended; block-form and flow-form key → replaced in place; neighbouring comments and keys byte-identical; `[]` written; multi-document and non-mapping input refused; an unchanged value writes nothing. +2. `init --harnesses claude` on a fresh shared repo writes `agent_harnesses: [claude]` to `.docket.yml` and the Claude dispatch surface, both reported as pending paths, in one run. +3. `init --harnesses claude` on a fresh private repo writes `.git/dckt/config.yml` and the dispatch block in `.git/dckt/AGENTS.md`, with nothing pending. +4. Interactive `init` with no repository-level value calls the chooser with the documented pre-check order (repo, then global, then detected); a fake chooser's selection is what gets written; an empty selection writes `[]`; cancellation writes nothing. +5. Non-interactive `init` with no flag and no value succeeds and its output names the exact line and `docket repository configure-harnesses`. +6. `init` on a repo that already declares the key never calls the chooser and leaves the value unchanged. +7. `configure-harnesses --harnesses claude` on a repo with `[claude, cursor]` removes `.cursor/rules/docket-dispatch.mdc` through the proof-gated removal; `--harnesses none` retires every owned surface. +8. `configure-harnesses` succeeds with a dirty checkout, behind origin, and in `needs-review`; it refuses a fresh and a legacy repository with the documented remedies. +9. `configure-harnesses` with no flag and no terminal refuses and writes nothing. +10. An unknown or duplicate token, or `none` mixed with a name, is refused before any write, for both commands. +11. With `.docket.local.yml` declaring the key, the result warns and names the applied value. +12. `repository check` reports `harnesses-unset` when the key is absent, and does not for `[]` or a list, in both shared and private layouts; the classified state is unaffected. +13. Every new assert is mutation-checked: removing the behavior it guards turns it red. + +## ADRs expected + +One new ADR: docket adopts `charmbracelet/huh` as its interactive terminal UI dependency, chosen over a hand-rolled `golang.org/x/term` picker despite about 1.3–1.8 MB (around 12%) more binary and 42 modules in the graph; later interactive prompts reuse it rather than adding another library. + +## Out of scope + +- Making init refuse to run until harnesses are chosen. +- Copying the global `agent_harnesses` into the repository config without asking. +- Changing #351's rule that only a repository-level `agent_harnesses` authorizes repository writes. +- Relaxing init's clean-checkout and at-remote-tip precondition. +- Converting the existing y/N prompts or any other command to huh. +- A huh accessible-mode switch. From 062a68cbfd8aa1d952e5e998d045a5b0d3c89dc1 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 21:32:42 -0400 Subject: [PATCH 02/12] feat(reposetup): validate an agent_harnesses selection against the config's token set --- internal/config/schema.go | 5 ++ internal/config/schema_test.go | 13 +++++ internal/reposetup/harnesses.go | 73 ++++++++++++++++++++++++++++ internal/reposetup/harnesses_test.go | 70 ++++++++++++++++++++++++++ 4 files changed, 161 insertions(+) create mode 100644 internal/reposetup/harnesses.go create mode 100644 internal/reposetup/harnesses_test.go diff --git a/internal/config/schema.go b/internal/config/schema.go index 561c01081..96179e9d0 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -123,6 +123,11 @@ var ( agentHarnessTokens = []string{"claude", "codex", "cursor", "opencode"} ) +// AgentHarnessTokens returns a fresh copy of the closed agent_harnesses token +// set: the accept set every writer of the key validates against, so a writer +// can never accept a token the reader refuses. +func AgentHarnessTokens() []string { return append([]string(nil), agentHarnessTokens...) } + // changeTypeToken is the shape of a change type; `all` and `untyped` match it // but are reserved selector words, so they are rejected separately. var changeTypeToken = regexp.MustCompile(`^[a-z][a-z0-9-]*$`) diff --git a/internal/config/schema_test.go b/internal/config/schema_test.go index cdedf28eb..0a27c380b 100644 --- a/internal/config/schema_test.go +++ b/internal/config/schema_test.go @@ -494,3 +494,16 @@ func TestDiagnosticsNeverEchoTheDocument(t *testing.T) { } } } + +// TestAgentHarnessTokensIsACopy pins the exported accept set and proves a +// caller mutating the returned slice cannot change the package's own set. +func TestAgentHarnessTokensIsACopy(t *testing.T) { + got := AgentHarnessTokens() + if strings.Join(got, ",") != "claude,codex,cursor,opencode" { + t.Fatalf("AgentHarnessTokens() = %v", got) + } + got[0] = "mutated" + if again := AgentHarnessTokens(); again[0] != "claude" { + t.Fatalf("mutation leaked into the next call: %v", again) + } +} diff --git a/internal/reposetup/harnesses.go b/internal/reposetup/harnesses.go new file mode 100644 index 000000000..2287a7066 --- /dev/null +++ b/internal/reposetup/harnesses.go @@ -0,0 +1,73 @@ +package reposetup + +// harnesses.go — parsing and validation of an agent_harnesses selection (the +// --harnesses flag or a picker's result) against the config package's own +// token set, plus the canonical one-line rendering of the key. + +import ( + "errors" + "fmt" + "strings" + + "github.com/danielhanold/docket/internal/config" +) + +// HarnessesNone is the standalone selection token meaning "no harness" ([]). +const HarnessesNone = "none" + +// ConfigureHarnessesCommand is the command that changes the selection later. +const ConfigureHarnessesCommand = "docket repository configure-harnesses" + +// ErrInvalidHarnesses is wrapped by every ParseHarnessSelection refusal. +var ErrInvalidHarnesses = errors.New("invalid harness selection") + +// ParseHarnessSelection validates trimmed tokens against +// config.AgentHarnessTokens, classifying every token before refusing, and +// returns a non-nil selection in canonical order. A lone `none` yields []. +func ParseHarnessSelection(tokens []string) ([]string, error) { + allowed := config.AgentHarnessTokens() + empty := len(tokens) == 0 + none := false + var unknown, dups []string + seen := map[string]bool{} + for _, raw := range tokens { + tok := strings.TrimSpace(raw) + switch { + case tok == "": + empty = true + case tok == HarnessesNone: + none = true + case !containsString(allowed, tok): + unknown = append(unknown, tok) + case seen[tok]: + if !containsString(dups, tok) { + dups = append(dups, tok) + } + default: + seen[tok] = true + } + } + switch { + case empty: + return nil, fmt.Errorf("%w: --harnesses has an empty value; pass a comma list such as `claude,cursor`, or `none`", ErrInvalidHarnesses) + case len(unknown) > 0: + return nil, fmt.Errorf("%w: unknown harness %s; choose from %s, or `none`", ErrInvalidHarnesses, strings.Join(unknown, ", "), strings.Join(allowed, ", ")) + case len(dups) > 0: + return nil, fmt.Errorf("%w: %s is listed more than once", ErrInvalidHarnesses, strings.Join(dups, ", ")) + case none && len(seen) > 0: + return nil, fmt.Errorf("%w: `none` cannot be combined with a harness name", ErrInvalidHarnesses) + } + out := []string{} + for _, h := range allowed { + if seen[h] { + out = append(out, h) + } + } + return out, nil +} + +// FormatHarnessList renders a selection as a YAML flow list: "[claude, cursor]". +func FormatHarnessList(h []string) string { return "[" + strings.Join(h, ", ") + "]" } + +// AgentHarnessesLine renders the single top-level config line for a selection. +func AgentHarnessesLine(h []string) string { return "agent_harnesses: " + FormatHarnessList(h) } diff --git a/internal/reposetup/harnesses_test.go b/internal/reposetup/harnesses_test.go new file mode 100644 index 000000000..de0565603 --- /dev/null +++ b/internal/reposetup/harnesses_test.go @@ -0,0 +1,70 @@ +package reposetup + +import ( + "errors" + "reflect" + "strings" + "testing" +) + +func TestParseHarnessSelection(t *testing.T) { + accepted := []struct { + in []string + want []string + }{ + {[]string{"claude"}, []string{"claude"}}, + {[]string{"cursor", "claude"}, []string{"claude", "cursor"}}, + {[]string{" claude", "cursor "}, []string{"claude", "cursor"}}, + {[]string{"opencode", "cursor", "codex", "claude"}, []string{"claude", "codex", "cursor", "opencode"}}, + {[]string{"none"}, []string{}}, + {[]string{" none "}, []string{}}, + } + for _, tc := range accepted { + got, err := ParseHarnessSelection(tc.in) + if err != nil { + t.Errorf("ParseHarnessSelection(%q) error: %v", tc.in, err) + continue + } + if got == nil || !reflect.DeepEqual(got, tc.want) { + t.Errorf("ParseHarnessSelection(%q) = %#v, want %#v", tc.in, got, tc.want) + } + } + + refused := []struct { + in []string + words []string + }{ + {nil, []string{"empty"}}, + {[]string{}, []string{"empty"}}, + {[]string{""}, []string{"empty"}}, + {[]string{"claude", ""}, []string{"empty"}}, + {[]string{"claude", "claude"}, []string{"claude", "more than once"}}, + {[]string{"none", "claude"}, []string{"none"}}, + {[]string{"bogus", "claude", "nope"}, []string{"bogus", "nope", "claude, codex, cursor, opencode"}}, + {[]string{"Claude"}, []string{"Claude"}}, + } + for _, tc := range refused { + got, err := ParseHarnessSelection(tc.in) + if err == nil { + t.Errorf("ParseHarnessSelection(%q) = %v, want refusal", tc.in, got) + continue + } + if !errors.Is(err, ErrInvalidHarnesses) { + t.Errorf("ParseHarnessSelection(%q) error %v does not wrap ErrInvalidHarnesses", tc.in, err) + } + for _, w := range tc.words { + if !strings.Contains(err.Error(), w) { + t.Errorf("ParseHarnessSelection(%q) error %q lacks %q", tc.in, err, w) + } + } + } +} + +func TestAgentHarnessesLine(t *testing.T) { + if got := AgentHarnessesLine([]string{"claude", "cursor"}); got != "agent_harnesses: [claude, cursor]" { + t.Errorf("got %q", got) + } + if got := AgentHarnessesLine([]string{}); got != "agent_harnesses: []" { + t.Errorf("got %q", got) + } +} From 11741d4704bfe72aea127252728cf541f0055ac4 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 21:34:06 -0400 Subject: [PATCH 03/12] feat(reposetup): write agent_harnesses as one flow line, preserving every other byte --- internal/reposetup/harnessesedit.go | 154 +++++++++++++++++++++++ internal/reposetup/harnessesedit_test.go | 86 +++++++++++++ 2 files changed, 240 insertions(+) create mode 100644 internal/reposetup/harnessesedit.go create mode 100644 internal/reposetup/harnessesedit_test.go diff --git a/internal/reposetup/harnessesedit.go b/internal/reposetup/harnessesedit.go new file mode 100644 index 000000000..0c4080275 --- /dev/null +++ b/internal/reposetup/harnessesedit.go @@ -0,0 +1,154 @@ +package reposetup + +// harnessesedit.go — byte-preserving write of the top-level agent_harnesses +// key of a docket config file. The key is located with a YAML AST parse and +// replaced or appended by a line splice on the raw bytes, never re-serialized +// (like RemoveMetadataBranchKey), so every other byte of the file survives. +// The result is re-parsed and refused unless the only change is the key (like +// verifyOwnerPairs). + +import ( + "bytes" + "errors" + "fmt" + "reflect" + "slices" + + "go.yaml.in/yaml/v3" +) + +const agentHarnessesKey = "agent_harnesses" + +// RenderAgentHarnessesEdit returns existing with its top-level agent_harnesses +// key set to the one flow line AgentHarnessesLine(harnesses): an existing key +// (flow or block form) is replaced in place, a missing one is appended. A nil +// existing means no file. harnesses must be non-nil and canonical (as +// ParseHarnessSelection returns it). When the key already holds that value it +// returns (existing, false, nil) and nothing should be written. +// +// It refuses, returning (nil, false, err), on: a nil, invalid, or +// non-canonical selection; a multi-document file; a root that is not a mapping +// or is a single-line {…} mapping; undecodable YAML; the key declared more +// than once at the top level; the key sharing a line with another setting; and +// any edit whose re-parse shows a change beyond the key (a block scalar value, +// for one). +func RenderAgentHarnessesEdit(existing []byte, harnesses []string) (edited []byte, changed bool, err error) { + if harnesses == nil { + return nil, false, errors.New("reposetup: no agent_harnesses selection to write") + } + if len(harnesses) > 0 { + canon, perr := ParseHarnessSelection(harnesses) + if perr != nil { + return nil, false, perr + } + if !slices.Equal(canon, harnesses) { + return nil, false, fmt.Errorf("reposetup: agent_harnesses selection %v is not in canonical order", harnesses) + } + } + root, err := topLevelMapping(existing) + if err != nil { + return nil, false, err + } + byHand := "; refusing to edit — set agent_harnesses by hand" + if root != nil && root.Style&yaml.FlowStyle != 0 { + return nil, false, errors.New("reposetup: the config is a single-line {…} mapping" + byHand) + } + eol := "\n" + if bytes.Contains(existing, []byte("\r\n")) { + eol = "\r\n" + } + line := AgentHarnessesLine(harnesses) + keyIdx := -1 + if root != nil { + for i := 0; i+1 < len(root.Content); i += 2 { + if k := root.Content[i]; k.Kind == yaml.ScalarNode && k.Value == agentHarnessesKey { + if keyIdx >= 0 { + return nil, false, fmt.Errorf("reposetup: %q declared more than once at the top level%s", agentHarnessesKey, byHand) + } + keyIdx = i + } + } + } + var out []byte + if keyIdx < 0 { + out = append([]byte(nil), existing...) + if len(out) > 0 && out[len(out)-1] != '\n' { + out = append(out, eol...) + } + out = append(out, line+eol...) + } else { + val := root.Content[keyIdx+1] + if harnessSequenceEquals(val, harnesses) { + return existing, false, nil + } + start, end := root.Content[keyIdx].Line, maxNodeLine(val) + if end < start || val.Kind == yaml.ScalarNode && val.Tag == "!!null" && val.Value == "" { + end = start // an empty value lives on the key's own line + } + if keyIdx > 0 && maxNodeLine(root.Content[keyIdx-1]) >= start || + keyIdx+2 < len(root.Content) && root.Content[keyIdx+2].Line <= end { + return nil, false, fmt.Errorf("reposetup: %q shares a line with another setting%s", agentHarnessesKey, byHand) + } + starts := lineOffsets(existing) + from, to := starts[start-1], lineEndByte(existing, starts, end) + text := line + if !(to == len(existing) && (to == 0 || existing[to-1] != '\n')) { + text += eol + } + out = append(append(append(make([]byte, 0, len(existing)+len(text)), existing[:from]...), text...), existing[to:]...) + } + if verr := verifyHarnessesEdit(root, out, harnesses); verr != nil { + return nil, false, verr + } + return out, true, nil +} + +// harnessSequenceEquals reports whether n is a sequence of exactly the string +// scalars want, in order. +func harnessSequenceEquals(n *yaml.Node, want []string) bool { + if n == nil || n.Kind != yaml.SequenceNode || len(n.Content) != len(want) { + return false + } + for i, item := range n.Content { + if item.Kind != yaml.ScalarNode || item.Tag != "!!str" || item.Value != want[i] { + return false + } + } + return true +} + +// verifyHarnessesEdit re-parses the edited bytes and refuses unless the +// top-level keys are the original ones (plus an appended agent_harnesses), +// every other value decodes identically, and agent_harnesses equals want. It +// is what catches a block scalar value, whose node under-reports its last line. +func verifyHarnessesEdit(origRoot *yaml.Node, edited []byte, want []string) error { + refuse := func(format string, args ...any) error { + return fmt.Errorf("reposetup: the agent_harnesses edit cannot be made safely ("+format+"); refusing to edit — set agent_harnesses by hand", args...) + } + after, err := topLevelMapping(edited) + if err != nil || after == nil { + return refuse("the edited file does not parse as a mapping: %v", err) + } + wantKeys := mappingKeys(origRoot) + if !containsString(wantKeys, agentHarnessesKey) { + wantKeys = append(append([]string(nil), wantKeys...), agentHarnessesKey) + } + if got := mappingKeys(after); !slices.Equal(got, wantKeys) { + return refuse("the top-level settings would be %v, want %v", got, wantKeys) + } + for i := 0; i+1 < len(after.Content); i += 2 { + k, v := after.Content[i].Value, after.Content[i+1] + if k == agentHarnessesKey { + if !harnessSequenceEquals(v, want) { + return refuse("agent_harnesses would not be %s", FormatHarnessList(want)) + } + continue + } + _, orig, _ := findChild(origRoot, k) // reached only for original keys, so origRoot != nil + var a, b any + if orig == nil || orig.Decode(&a) != nil || v.Decode(&b) != nil || !reflect.DeepEqual(a, b) { + return refuse("setting %q would change", k) + } + } + return nil +} diff --git a/internal/reposetup/harnessesedit_test.go b/internal/reposetup/harnessesedit_test.go new file mode 100644 index 000000000..e392d13f0 --- /dev/null +++ b/internal/reposetup/harnessesedit_test.go @@ -0,0 +1,86 @@ +package reposetup + +import ( + "strings" + "testing" +) + +func TestRenderAgentHarnessesEditRewrites(t *testing.T) { + cases := []struct { + name string + in string + harnesses []string + want string + }{ + {"append after a key", "integration_branch: main\n", []string{"claude"}, "integration_branch: main\nagent_harnesses: [claude]\n"}, + {"no file writes none", "", []string{}, "agent_harnesses: []\n"}, + {"append with no final newline", "a: 1", []string{"claude"}, "a: 1\nagent_harnesses: [claude]\n"}, + {"replace flow with trailing comment", "# top\nagent_harnesses: [codex] # mine\nb: 2 # keep\n", []string{"claude", "cursor"}, "# top\nagent_harnesses: [claude, cursor]\nb: 2 # keep\n"}, + {"replace block items", "x: 1\n# Harnesses here.\nagent_harnesses:\n - claude\n - opencode\nfinalize:\n gate: local\n", []string{"claude"}, "x: 1\n# Harnesses here.\nagent_harnesses: [claude]\nfinalize:\n gate: local\n"}, + {"replace keeps CRLF", "a: 1\r\nagent_harnesses: [codex]\r\nb: 2\r\n", []string{"claude"}, "a: 1\r\nagent_harnesses: [claude]\r\nb: 2\r\n"}, + {"append keeps CRLF", "a: 1\r\n", []string{}, "a: 1\r\nagent_harnesses: []\r\n"}, + {"replace final line without newline", "a: 1\nagent_harnesses: [codex]", []string{}, "a: 1\nagent_harnesses: []"}, + {"replace empty value", "agent_harnesses:\nb: 2\n", []string{"cursor"}, "agent_harnesses: [cursor]\nb: 2\n"}, + {"nested key is not top-level", "build:\n agent_harnesses: x\n", []string{"claude"}, "build:\n agent_harnesses: x\nagent_harnesses: [claude]\n"}, + {"comments-only file", "# only a comment\n", []string{"claude"}, "# only a comment\nagent_harnesses: [claude]\n"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var in []byte + if tc.in != "" { + in = []byte(tc.in) + } + got, changed, err := RenderAgentHarnessesEdit(in, tc.harnesses) + if err != nil || !changed || string(got) != tc.want { + t.Fatalf("RenderAgentHarnessesEdit(%q, %v) = (%q, %v, %v), want (%q, true, nil)", tc.in, tc.harnesses, got, changed, err, tc.want) + } + }) + } +} + +func TestRenderAgentHarnessesEditUnchangedWritesNothing(t *testing.T) { + cases := []struct { + in string + harnesses []string + }{ + {"agent_harnesses: [claude, cursor]\n", []string{"claude", "cursor"}}, + {"a: 1\nagent_harnesses:\n - claude\n - cursor\n", []string{"claude", "cursor"}}, + {"agent_harnesses: []\n", []string{}}, + } + for _, tc := range cases { + got, changed, err := RenderAgentHarnessesEdit([]byte(tc.in), tc.harnesses) + if err != nil || changed || string(got) != tc.in { + t.Errorf("RenderAgentHarnessesEdit(%q, %v) = (%q, %v, %v), want the input back unchanged", tc.in, tc.harnesses, got, changed, err) + } + } +} + +func TestRenderAgentHarnessesEditRefuses(t *testing.T) { + cases := []struct { + name string + in string + harnesses []string + contains string + }{ + {"multi-document", "a: 1\n---\nb: 2\n", []string{"claude"}, ""}, + {"sequence root", "- a\n- b\n", []string{"claude"}, ""}, + {"undecodable", "a: [\n", []string{"claude"}, ""}, + {"duplicate key", "agent_harnesses: []\nagent_harnesses: [claude]\n", []string{"claude"}, ""}, + {"flow mapping root", "{a: 1, agent_harnesses: []}\n", []string{"claude"}, ""}, + {"block scalar", "agent_harnesses: |\n claude\nb: 2\n", []string{"claude"}, "by hand"}, + {"nil selection", "a: 1\n", nil, ""}, + {"unknown token", "a: 1\n", []string{"bogus"}, ""}, + {"non-canonical order", "a: 1\n", []string{"cursor", "claude"}, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, changed, err := RenderAgentHarnessesEdit([]byte(tc.in), tc.harnesses) + if err == nil || changed || got != nil { + t.Fatalf("RenderAgentHarnessesEdit(%q, %v) = (%q, %v, %v), want a refusal", tc.in, tc.harnesses, got, changed, err) + } + if tc.contains != "" && !strings.Contains(err.Error(), tc.contains) { + t.Fatalf("error %q does not contain %q", err, tc.contains) + } + }) + } +} From 2582cd5d845b5a94aa5128dc11b22362b7a10c1b Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 21:36:30 -0400 Subject: [PATCH 04/12] feat(app): add the shared choose-harnesses step for init and configure-harnesses --- internal/app/repository_harnesses.go | 293 ++++++++++++++++++++++ internal/app/repository_harnesses_test.go | 224 +++++++++++++++++ internal/app/repository_init.go | 31 ++- 3 files changed, 538 insertions(+), 10 deletions(-) create mode 100644 internal/app/repository_harnesses.go create mode 100644 internal/app/repository_harnesses_test.go diff --git a/internal/app/repository_harnesses.go b/internal/app/repository_harnesses.go new file mode 100644 index 000000000..04015ce90 --- /dev/null +++ b/internal/app/repository_harnesses.go @@ -0,0 +1,293 @@ +// The shared choose-harnesses step that repository init and repository +// configure-harnesses both run, in two halves. +// +// resolveHarnessChoice decides what to write — from the --harnesses flag, an +// interactive chooser, or the caller's policy when neither is available — and +// runs before any repository write, so a cancelled chooser leaves nothing +// behind. +// +// applyHarnessChoice writes a decided selection to the repository's config +// target, then refreshes the agent surfaces through installAuthorizedSurfaces. +// That function re-reads configuration from disk rather than trusting the +// gather-time snapshot, so a key written in this run authorizes this run's +// surfaces: the predicate asks the post-write state, never the pre-write one. +package app + +import ( + "context" + "errors" + "fmt" + "os" + "slices" + + "github.com/danielhanold/docket/internal/config" + "github.com/danielhanold/docket/internal/gitcli" + "github.com/danielhanold/docket/internal/harness" + "github.com/danielhanold/docket/internal/install" + "github.com/danielhanold/docket/internal/layout" + "github.com/danielhanold/docket/internal/reposetup" +) + +// HarnessChoiceRequest is what an interactive chooser is shown: every +// selectable harness in planning order, the ones to pre-check, and the config +// path the answer is written to. +type HarnessChoiceRequest struct { + Options []string + Preselected []string + ConfigPath string +} + +// HarnessChooser asks a human which harnesses get docket's dispatch +// instructions. It returns the chosen tokens (empty means none) or +// ErrHarnessChoiceCancelled when the human backs out. +type HarnessChooser func(ctx context.Context, req HarnessChoiceRequest) ([]string, error) + +// ErrHarnessChoiceCancelled is the sentinel a HarnessChooser returns when the +// human cancels the choice. +var ErrHarnessChoiceCancelled = errors.New("the harness choice was cancelled") + +// errHarnessesFlagRequired is configure-harnesses' refusal when it has neither a +// --harnesses value nor a chooser to ask with. +var errHarnessesFlagRequired = errors.New("no --harnesses value and no terminal to show the checklist on") + +// HarnessesOptions carries the caller's harness input: whether --harnesses was +// given, its raw tokens, and the chooser to ask with (nil when the caller has no +// terminal to ask on). +type HarnessesOptions struct { + Set bool + Tokens []string + Chooser HarnessChooser +} + +// parseHarnessesFlag validates a --harnesses value before any repository read +// or write. An unset flag returns nil, nil; a valid one returns the canonical, +// non-nil selection; an invalid one returns an invalid-input refusal. +func parseHarnessesFlag(operation string, opts HarnessesOptions) ([]string, *RepositoryOpResult) { + if !opts.Set { + return nil, nil + } + sel, err := reposetup.ParseHarnessSelection(opts.Tokens) + if err != nil { + out := newRepositoryOpResult(operation, ResultInvalidInput, RepositoryOpResult{}) + out.human = fmt.Sprintf("%s: %s: %s", operation, ResultInvalidInput, err.Error()) + return nil, &out + } + return sel, nil +} + +// harnessPolicy is the caller's rule for when neither a flag nor a chooser +// decides, and for whether an existing repository value is kept unasked. +type harnessPolicy int + +const ( + harnessPolicyInit harnessPolicy = iota // keep a repository value; warn when it cannot ask + harnessPolicyConfigure // always ask; refuse when it cannot +) + +// harnessChoiceInput is everything resolveHarnessChoice decides from. +type harnessChoiceInput struct { + flag []string // parsed --harnesses; nil when not passed + chooser HarnessChooser + policy harnessPolicy + cfg config.Effective + repoDeclared bool + configDisplay string + detect func() []string +} + +// harnessChoice is resolveHarnessChoice's answer: a decided selection to write, +// or nothing to write plus an optional warning. +type harnessChoice struct { + decided bool + selection []string // non-nil when decided + warning string +} + +// resolveHarnessChoice decides the selection before any write: the flag wins; +// init keeps a repository-declared value unasked; otherwise a chooser is asked +// with the current value (or detection) pre-checked; with no chooser, init warns +// and configure refuses with errHarnessesFlagRequired. +func resolveHarnessChoice(ctx context.Context, in harnessChoiceInput) (harnessChoice, error) { + if in.flag != nil { + return harnessChoice{decided: true, selection: in.flag}, nil + } + if in.repoDeclared && in.policy == harnessPolicyInit { + return harnessChoice{}, nil + } + if in.chooser != nil { + got, err := in.chooser(ctx, HarnessChoiceRequest{ + Options: append([]string(nil), harness.Order...), Preselected: harnessPreselection(in.cfg, in.detect), ConfigPath: in.configDisplay, + }) + if err != nil { + return harnessChoice{}, err + } + sel := []string{} + if len(got) > 0 { + if sel, err = reposetup.ParseHarnessSelection(got); err != nil { + return harnessChoice{}, fmt.Errorf("the harness chooser returned an invalid selection: %w", err) + } + } + return harnessChoice{decided: true, selection: sel}, nil + } + if in.policy == harnessPolicyInit { + return harnessChoice{warning: fmt.Sprintf("agent_harnesses is not set, so docket wrote no agent dispatch instructions; add a line such as `%s` to %s, or run `%s`", + reposetup.AgentHarnessesLine([]string{"claude"}), in.configDisplay, reposetup.ConfigureHarnessesCommand)}, nil + } + return harnessChoice{}, errHarnessesFlagRequired +} + +// harnessRepoDeclared reports whether the repository or repository-local layer +// explicitly declares agent_harnesses — the same predicate that authorizes +// surface writes (Facts.SurfacesAuthorized). +func harnessRepoDeclared(cfg config.Effective) bool { + return cfg.AgentHarnesses.Explicit && isRepositoryLayer(cfg.AgentHarnesses.Provenance.Layer) +} + +// harnessPreselection is what the chooser pre-checks: the resolved value when +// any layer declares it, else what detect finds (nothing when detect is nil). +func harnessPreselection(cfg config.Effective, detect func() []string) []string { + if cfg.AgentHarnesses.Explicit { + return append([]string{}, cfg.AgentHarnesses.Value...) + } + if detect == nil { + return []string{} + } + return detect() +} + +// detectHarnesses names, in planning order, every harness whose user-level +// root is present in the invoking user's home. It is a hint only: an +// unresolvable home detects nothing. +func detectHarnesses() []string { + found := []string{} + roots, err := install.ResolveRoots(os.UserHomeDir, os.Getenv) + if err != nil { + return found + } + for _, p := range Planners(roots, config.AgentsTable{}) { + if p.Detect == nil { + continue + } + if present, _ := p.Detect(roots); present { + found = append(found, p.Name) + } + } + return found +} + +// harnessApplied reports what applyHarnessChoice changed: the config's pending +// path (if any), whether the config was written, the surface paths pending +// review, whether any surface changed, and the warnings to show. +type harnessApplied struct { + pendingConfig string + wroteConfig bool + surfacePending []string + wroteSurfaces bool + warnings []string +} + +// harnessConfigError marks a failure writing or re-reading agent_harnesses in +// the config at display, so harnessApplyFailure can tell it from a surface +// failure. +type harnessConfigError struct { + display string + err error +} + +// Error names the config path and the underlying failure. +func (e *harnessConfigError) Error() string { + return fmt.Sprintf("writing agent_harnesses to %s: %v", e.display, e.err) +} + +// Unwrap exposes the underlying failure. +func (e *harnessConfigError) Unwrap() error { return e.err } + +// applyHarnessChoice writes a decided selection, ALWAYS refreshes the surfaces +// (a kept value still installs them; dropped harnesses lose theirs), and warns +// when .docket.local.yml overrides the write — keyed on provenance. +func applyHarnessChoice(ctx context.Context, git *gitcli.Client, sc setupContext, choice harnessChoice) (harnessApplied, error) { + var out harnessApplied + _, display, _ := repoConfigTarget(sc) + if choice.decided { + pending, wrote, err := writeTargetConfig(sc, func(existing []byte) ([]byte, error) { + edited, changed, rerr := reposetup.RenderAgentHarnessesEdit(existing, choice.selection) + if rerr != nil || !changed { + return nil, rerr + } + return edited, nil + }) + if err != nil { + return out, &harnessConfigError{display: display, err: err} + } + out.pendingConfig, out.wroteConfig = pending, wrote + } + var err error + if out.surfacePending, out.wroteSurfaces, err = installAuthorizedSurfaces(ctx, git, sc.repo.PrimaryWorktree); err != nil { + return out, err + } + if choice.warning != "" { + out.warnings = append(out.warnings, choice.warning) + } + if choice.decided && sc.layout.Mode != layout.Private { + eff, rerr := resolveSetupConfig(sc.repo.PrimaryWorktree, sc.defaultBranch) + if rerr != nil { + return out, &harnessConfigError{display: display, err: rerr} + } + if eff.AgentHarnesses.Provenance.Layer == config.LayerRepositoryLocal { + out.warnings = append(out.warnings, fmt.Sprintf("`.docket.local.yml` also sets agent_harnesses and overrides %s in this clone; the applied value is `%s`", + display, reposetup.FormatHarnessList(eff.AgentHarnesses.Value))) + } + } + return out, nil +} + +// harnessApplyFailure maps an applyHarnessChoice error to operation's result: a +// config write failure is an internal error naming the config path; anything +// else is a surface failure. +func harnessApplyFailure(operation string, state reposetup.State, err error) RepositoryOpResult { + var ce *harnessConfigError + if errors.As(err, &ce) { + return repositoryInternalFailure(operation, state, "writing agent_harnesses to "+ce.display, ce.err) + } + return mapSurfaceFailureFor(operation, state, err) +} + +// harnessChoiceCancelled is the interrupted result for a cancelled chooser; +// resolveHarnessChoice runs before any write, so nothing was written. +func harnessChoiceCancelled(operation string, state reposetup.State) RepositoryOpResult { + out := newRepositoryOpResult(operation, ResultInterrupted, RepositoryOpResult{ + RepositoryState: string(state), + }) + out.human = fmt.Sprintf("%s: %s: the harness choice was cancelled; nothing was written", operation, ResultInterrupted) + return out +} + +// appendPending appends each path to dst, skipping "" and any path already +// present. +func appendPending(dst []string, paths ...string) []string { + for _, p := range paths { + if p == "" || slices.Contains(dst, p) { + continue + } + dst = append(dst, p) + } + return dst +} + +// setHarnessResult records the harness outcome on out: the selection when one +// was decided, and the warnings, each also appended to the human text. +func setHarnessResult(out *RepositoryOpResult, choice harnessChoice, applied harnessApplied) { + if choice.decided { + sel := append([]string{}, choice.selection...) + out.AgentHarnesses = &sel + } + if len(applied.warnings) == 0 { + return + } + out.Warnings = append(out.Warnings, applied.warnings...) + human := out.HumanText() + for _, w := range applied.warnings { + human += "\nwarning: " + w + } + out.human = human +} diff --git a/internal/app/repository_harnesses_test.go b/internal/app/repository_harnesses_test.go new file mode 100644 index 000000000..5f2e2cb59 --- /dev/null +++ b/internal/app/repository_harnesses_test.go @@ -0,0 +1,224 @@ +package app + +import ( + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/config" + "github.com/danielhanold/docket/internal/harness" + "github.com/danielhanold/docket/internal/reposetup" + "github.com/danielhanold/docket/internal/testsupport" +) + +// recordingChooser returns a HarnessChooser that records the request it was +// shown, counts its calls, and answers ret, err. Integration tests reuse it. +func recordingChooser(got *HarnessChoiceRequest, calls *int, ret []string, err error) HarnessChooser { + return func(_ context.Context, req HarnessChoiceRequest) ([]string, error) { + if got != nil { + *got = req + } + if calls != nil { + *calls++ + } + return ret, err + } +} + +// harnessCfg builds an Effective whose agent_harnesses is explicit at layer, +// or unset when layer is "". +func harnessCfg(layer config.LayerKind, v ...string) config.Effective { + var cfg config.Effective + if layer != "" { + if v == nil { + v = []string{} + } + cfg.AgentHarnesses = config.Value[[]string]{Value: v, Explicit: true, Provenance: config.Provenance{Layer: layer}} + } + return cfg +} + +func TestHarnessTokensMatchHarnessOrder(t *testing.T) { + if !reflect.DeepEqual(config.AgentHarnessTokens(), harness.Order) { + t.Fatalf("config tokens %v != harness.Order %v", config.AgentHarnessTokens(), harness.Order) + } +} + +func TestResolveHarnessChoice(t *testing.T) { + ctx := context.Background() + noDetect := func(t *testing.T) func() []string { + return func() []string { t.Fatal("detect must not be called"); return nil } + } + + t.Run("flag wins without asking", func(t *testing.T) { + calls := 0 + ch, err := resolveHarnessChoice(ctx, harnessChoiceInput{ + flag: []string{"claude"}, chooser: recordingChooser(nil, &calls, []string{"codex"}, nil), + policy: harnessPolicyInit, cfg: harnessCfg(config.LayerRepository, "cursor"), repoDeclared: true, + }) + if err != nil || !ch.decided || !reflect.DeepEqual(ch.selection, []string{"claude"}) || calls != 0 { + t.Fatalf("got %+v err=%v calls=%d", ch, err, calls) + } + }) + + t.Run("init keeps a repository value", func(t *testing.T) { + calls := 0 + ch, err := resolveHarnessChoice(ctx, harnessChoiceInput{ + chooser: recordingChooser(nil, &calls, []string{"codex"}, nil), + policy: harnessPolicyInit, cfg: harnessCfg(config.LayerRepository, "claude"), repoDeclared: true, + }) + if err != nil || ch.decided || ch.warning != "" || calls != 0 { + t.Fatalf("got %+v err=%v calls=%d", ch, err, calls) + } + }) + + t.Run("configure asks with the repository value pre-checked", func(t *testing.T) { + var req HarnessChoiceRequest + calls := 0 + ch, err := resolveHarnessChoice(ctx, harnessChoiceInput{ + chooser: recordingChooser(&req, &calls, []string{"cursor"}, nil), + policy: harnessPolicyConfigure, cfg: harnessCfg(config.LayerRepository, "claude", "cursor"), + repoDeclared: true, configDisplay: ".docket.yml", detect: noDetect(t), + }) + if err != nil || !ch.decided || !reflect.DeepEqual(ch.selection, []string{"cursor"}) || calls != 1 { + t.Fatalf("got %+v err=%v calls=%d", ch, err, calls) + } + want := HarnessChoiceRequest{Options: harness.Order, Preselected: []string{"claude", "cursor"}, ConfigPath: ".docket.yml"} + if !reflect.DeepEqual(req, want) { + t.Fatalf("request %+v, want %+v", req, want) + } + }) + + t.Run("a global value pre-checks without detection", func(t *testing.T) { + var req HarnessChoiceRequest + _, err := resolveHarnessChoice(ctx, harnessChoiceInput{ + chooser: recordingChooser(&req, nil, []string{"codex"}, nil), + policy: harnessPolicyInit, cfg: harnessCfg(config.LayerGlobal, "codex"), detect: noDetect(t), + }) + if err != nil || !reflect.DeepEqual(req.Preselected, []string{"codex"}) { + t.Fatalf("preselected %v err=%v", req.Preselected, err) + } + }) + + t.Run("unset pre-checks detection; an empty confirmation decides none", func(t *testing.T) { + var req HarnessChoiceRequest + ch, err := resolveHarnessChoice(ctx, harnessChoiceInput{ + chooser: recordingChooser(&req, nil, []string{}, nil), + policy: harnessPolicyInit, cfg: harnessCfg(""), detect: func() []string { return []string{"cursor"} }, + }) + if err != nil || !reflect.DeepEqual(req.Preselected, []string{"cursor"}) { + t.Fatalf("preselected %v err=%v", req.Preselected, err) + } + if !ch.decided || ch.selection == nil || len(ch.selection) != 0 { + t.Fatalf("want decided non-nil empty, got %+v (nil=%v)", ch, ch.selection == nil) + } + }) + + t.Run("chooser output is canonicalized and validated", func(t *testing.T) { + ch, err := resolveHarnessChoice(ctx, harnessChoiceInput{ + chooser: recordingChooser(nil, nil, []string{"opencode", "claude"}, nil), policy: harnessPolicyConfigure, + }) + if err != nil || !reflect.DeepEqual(ch.selection, []string{"claude", "opencode"}) { + t.Fatalf("got %+v err=%v", ch, err) + } + if _, err := resolveHarnessChoice(ctx, harnessChoiceInput{ + chooser: recordingChooser(nil, nil, []string{"bogus"}, nil), policy: harnessPolicyConfigure, + }); err == nil { + t.Fatal("want an error for an invalid chooser selection") + } + }) + + t.Run("cancel propagates", func(t *testing.T) { + _, err := resolveHarnessChoice(ctx, harnessChoiceInput{ + chooser: recordingChooser(nil, nil, nil, ErrHarnessChoiceCancelled), policy: harnessPolicyInit, + }) + if !errors.Is(err, ErrHarnessChoiceCancelled) { + t.Fatalf("err = %v", err) + } + }) + + t.Run("init with no input warns", func(t *testing.T) { + ch, err := resolveHarnessChoice(ctx, harnessChoiceInput{policy: harnessPolicyInit, configDisplay: ".git/dckt/config.yml"}) + if err != nil || ch.decided { + t.Fatalf("got %+v err=%v", ch, err) + } + for _, want := range []string{"`agent_harnesses: [claude]`", ".git/dckt/config.yml", reposetup.ConfigureHarnessesCommand} { + if !strings.Contains(ch.warning, want) { + t.Fatalf("warning %q lacks %q", ch.warning, want) + } + } + }) + + t.Run("configure with no input refuses", func(t *testing.T) { + _, err := resolveHarnessChoice(ctx, harnessChoiceInput{policy: harnessPolicyConfigure}) + if !errors.Is(err, errHarnessesFlagRequired) { + t.Fatalf("err = %v", err) + } + }) +} + +func TestParseHarnessesFlag(t *testing.T) { + got, refusal := parseHarnessesFlag(OperationRepositoryInit, HarnessesOptions{}) + if got != nil || refusal != nil { + t.Fatalf("unset: got %v refusal %v", got, refusal) + } + got, refusal = parseHarnessesFlag(OperationRepositoryInit, HarnessesOptions{Set: true, Tokens: []string{"none"}}) + if refusal != nil || got == nil || len(got) != 0 { + t.Fatalf("none: got %v (nil=%v) refusal %v", got, got == nil, refusal) + } + _, refusal = parseHarnessesFlag(OperationRepositoryInit, HarnessesOptions{Set: true, Tokens: []string{"bogus"}}) + if refusal == nil || refusal.Result != ResultInvalidInput || !strings.Contains(refusal.HumanText(), "bogus") { + t.Fatalf("bogus: refusal %+v", refusal) + } +} + +func TestDetectHarnessesReadsTheHome(t *testing.T) { + home := testsupport.TempDir(t) + xdg := filepath.Join(home, "xdg") + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", xdg) + if got := detectHarnesses(); got == nil || len(got) != 0 { + t.Fatalf("empty home: got %v", got) + } + for _, d := range []string{filepath.Join(home, ".cursor"), filepath.Join(xdg, "opencode")} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + if got := detectHarnesses(); !reflect.DeepEqual(got, []string{"cursor", "opencode"}) { + t.Fatalf("got %v", got) + } +} + +func TestRepositoryOpResultHarnessFields(t *testing.T) { + b, err := json.Marshal(RepositoryOpResult{AgentHarnesses: &[]string{}, Warnings: []string{"w"}}) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{`"agent_harnesses":[]`, `"warnings":["w"]`} { + if !strings.Contains(string(b), want) { + t.Fatalf("%s lacks %s", b, want) + } + } + b, err = json.Marshal(RepositoryOpResult{}) + if err != nil { + t.Fatal(err) + } + for _, absent := range []string{"agent_harnesses", "warnings"} { + if strings.Contains(string(b), absent) { + t.Fatalf("%s carries %s", b, absent) + } + } +} + +func TestAppendPendingDedupes(t *testing.T) { + got := appendPending([]string{".gitignore"}, "", ".docket.yml", ".gitignore", ".docket.yml") + if !reflect.DeepEqual(got, []string{".gitignore", ".docket.yml"}) { + t.Fatalf("got %v", got) + } +} diff --git a/internal/app/repository_init.go b/internal/app/repository_init.go index 54aff96bf..654bcc081 100644 --- a/internal/app/repository_init.go +++ b/internal/app/repository_init.go @@ -49,7 +49,12 @@ type RepositoryOpResult struct { MetadataTip string `json:"metadata_revision,omitempty"` SourceRevision string `json:"source_revision,omitempty"` Findings []reposetup.Finding `json:"findings,omitempty"` - human string + // AgentHarnesses is the agent_harnesses selection written or confirmed this + // run; an empty list is a recorded "no agents". Nil when no choice was made. + AgentHarnesses *[]string `json:"agent_harnesses,omitempty"` + // Warnings carries non-fatal notices the operation wants a human to read. + Warnings []string `json:"warnings,omitempty"` + human string } // HumanText renders the one-line human summary; a refusal carries its remedy in @@ -691,25 +696,31 @@ func repositoryInternalFailure(operation string, state reposetup.State, stage st return out } -// mapSurfaceFailure maps a surface-installation failure to its result: an -// unprovable surface or ownership conflict is invalid-state (a human must -// resolve it), a resolution error carries its own reason, and everything else is -// an external failure. +// mapSurfaceFailure maps an init surface-installation failure to its result; +// it delegates to mapSurfaceFailureFor with the init operation. func mapSurfaceFailure(state reposetup.State, err error) RepositoryOpResult { + return mapSurfaceFailureFor(OperationRepositoryInit, state, err) +} + +// mapSurfaceFailureFor maps a surface-installation failure to operation's +// result: an unprovable surface or ownership conflict is invalid-state (a human +// must resolve it), a resolution error carries its own reason, and everything +// else is an external failure. +func mapSurfaceFailureFor(operation string, state reposetup.State, err error) RepositoryOpResult { var rre *RepoResolutionError if errors.As(err, &rre) { - out := newRepositoryOpResult(OperationRepositoryInit, ResultInvalidState, RepositoryOpResult{ + out := newRepositoryOpResult(operation, ResultInvalidState, RepositoryOpResult{ RepositoryState: string(state), }) - out.human = fmt.Sprintf("%s: %s: %s", OperationRepositoryInit, ResultInvalidState, rre.Error()) + out.human = fmt.Sprintf("%s: %s: %s", operation, ResultInvalidState, rre.Error()) return out } if strings.Contains(err.Error(), "not provably docket's") { - out := newRepositoryOpResult(OperationRepositoryInit, ResultInvalidState, RepositoryOpResult{ + out := newRepositoryOpResult(operation, ResultInvalidState, RepositoryOpResult{ RepositoryState: string(state), }) - out.human = fmt.Sprintf("%s: %s: %s", OperationRepositoryInit, ResultInvalidState, err.Error()) + out.human = fmt.Sprintf("%s: %s: %s", operation, ResultInvalidState, err.Error()) return out } - return repositoryExternalFailure(OperationRepositoryInit, state, "installing parent-facing surfaces", err) + return repositoryExternalFailure(operation, state, "installing parent-facing surfaces", err) } From f4840f02e306664500754e3636c20deaf5b95540 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 21:44:34 -0400 Subject: [PATCH 05/12] feat(repository): choose agent harnesses during init and write them in the same run --- .../reposetup_harnesses_integration_test.go | 260 ++++++++++++++++++ internal/app/repository_init.go | 70 +++-- internal/app/repository_init_private.go | 37 ++- 3 files changed, 326 insertions(+), 41 deletions(-) create mode 100644 internal/app/reposetup_harnesses_integration_test.go diff --git a/internal/app/reposetup_harnesses_integration_test.go b/internal/app/reposetup_harnesses_integration_test.go new file mode 100644 index 000000000..81744d210 --- /dev/null +++ b/internal/app/reposetup_harnesses_integration_test.go @@ -0,0 +1,260 @@ +//go:build integration + +package app + +import ( + "context" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/reposetup" +) + +// This shard file carries `docket repository init`'s harness choice (prefix +// TestIntegrationRepoSetup, matched by tests/test_go_integration_app_reposetup.sh): +// init resolves agent_harnesses from --harnesses or the interactive chooser +// before any write, writes it to the repository's config target, and installs +// the dispatch surfaces that value authorizes in the same run. + +// harnessFlag is a --harnesses value as the CLI passes it: Set, with tokens. +func harnessFlag(tokens ...string) HarnessesOptions { + return HarnessesOptions{Set: true, Tokens: tokens} +} + +// runInitInHome runs init with the user roots pinned to a fresh temp home in +// which each of homeDirs exists, so harness detection sees exactly those. +func (r *initRepo) runInitInHome(t *testing.T, o InitOptions, homeDirs ...string) RepositoryOpResult { + t.Helper() + pinInitUserRoots(t) + home := os.Getenv("HOME") + for _, dir := range homeDirs { + if err := os.MkdirAll(filepath.Join(home, dir), 0o755); err != nil { + t.Fatal(err) + } + } + client := newGitClient(t) + return RunRepositoryInit(context.Background(), SetupDeps{Git: client, RepoDir: r.invocation}, o) +} + +// failingChooser is a chooser the test expects never to be asked. +func failingChooser(t *testing.T) HarnessChooser { + return func(context.Context, HarnessChoiceRequest) ([]string, error) { + t.Error("the harness chooser was called; want it never asked") + return nil, ErrHarnessChoiceCancelled + } +} + +func TestIntegrationRepoSetupInitHarnessesFlagShared(t *testing.T) { + r := newInitRepo(t, defaultSetupYML, nil) + res := r.runInitWith(t, InitOptions{Harnesses: harnessFlag("claude")}) + if res.Result != ResultApplied { + t.Fatalf("Result = %q (%s), want applied", res.Result, res.HumanText()) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, ".docket.yml"))); !strings.Contains(got, "\nagent_harnesses: [claude]\n") { + t.Errorf(".docket.yml = %q, want it to carry agent_harnesses: [claude]", got) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, "CLAUDE.md"))); !strings.Contains(got, "docket:dispatch") { + t.Errorf("CLAUDE.md = %q, want the docket:dispatch block written this run", got) + } + for _, want := range []string{".docket.yml", "CLAUDE.md", ".gitignore"} { + if !contains(res.PendingPaths, want) { + t.Errorf("PendingPaths = %v, want %s", res.PendingPaths, want) + } + } + if res.AgentHarnesses == nil || !reflect.DeepEqual(*res.AgentHarnesses, []string{"claude"}) { + t.Errorf("AgentHarnesses = %v, want [claude]", res.AgentHarnesses) + } + if staged := runGit(t, r.invocation, "diff", "--cached", "--name-only"); staged != "" { + t.Errorf("init staged %q; want nothing staged", staged) + } +} + +func TestIntegrationRepoSetupPrivateInitHarnessesFlag(t *testing.T) { + r, data := newPrivateInitRepo(t, nil) + res := r.runInitWith(t, InitOptions{Private: true, Harnesses: harnessFlag("claude")}) + if res.Result != ResultApplied { + t.Fatalf("Result = %q (%s), want applied", res.Result, res.HumanText()) + } + // The trap first: the gather-time layout of a fresh repository is shared, + // and writing through it would create .docket.yml in the working tree. + if _, err := os.Stat(filepath.Join(r.invocation, ".docket.yml")); !os.IsNotExist(err) { + t.Errorf(".docket.yml exists in the clone (err=%v); a private init must never create it", err) + } + if len(res.PendingPaths) != 0 { + t.Errorf("PendingPaths = %v, want none for a private init", res.PendingPaths) + } + if status := runGit(t, r.invocation, "status", "--porcelain"); status != "" { + t.Errorf("git status --porcelain = %q, want a clean working tree", status) + } + lay := privateLayoutOf(t, r.invocation, data) + if got := string(mustReadFile(t, lay.ConfigPath)); !strings.Contains(got, "agent_harnesses: [claude]\n") { + t.Errorf("%s = %q, want agent_harnesses: [claude]", lay.ConfigPath, got) + } + agents := filepath.Join(filepath.Dir(lay.ConfigPath), "AGENTS.md") + if got, err := os.ReadFile(agents); err != nil || !strings.Contains(string(got), "docket:dispatch") { + t.Errorf("%s = %q (%v), want the docket:dispatch block", agents, got, err) + } + if res.AgentHarnesses == nil || !reflect.DeepEqual(*res.AgentHarnesses, []string{"claude"}) { + t.Errorf("AgentHarnesses = %v, want [claude]", res.AgentHarnesses) + } +} + +func TestIntegrationRepoSetupInitInteractivePrecheckGlobal(t *testing.T) { + r := newInitRepo(t, defaultSetupYML, nil) + var req HarnessChoiceRequest + calls := 0 + res := r.runInitWithGlobal(t, "agent_harnesses: [codex]\n", + InitOptions{Harnesses: HarnessesOptions{Chooser: recordingChooser(&req, &calls, []string{"cursor"}, nil)}}) + if res.Result != ResultApplied { + t.Fatalf("Result = %q (%s), want applied", res.Result, res.HumanText()) + } + if calls != 1 { + t.Errorf("chooser calls = %d, want 1", calls) + } + if !reflect.DeepEqual(req.Preselected, []string{"codex"}) { + t.Errorf("Preselected = %v, want the global [codex]", req.Preselected) + } + if req.ConfigPath != ".docket.yml" { + t.Errorf("ConfigPath = %q, want .docket.yml", req.ConfigPath) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, ".docket.yml"))); !strings.Contains(got, "agent_harnesses: [cursor]\n") { + t.Errorf(".docket.yml = %q, want agent_harnesses: [cursor]", got) + } + if _, err := os.Stat(filepath.Join(r.invocation, ".cursor", "rules", "docket-dispatch.mdc")); err != nil { + t.Errorf("the Cursor dispatch rule was not written this run: %v", err) + } +} + +func TestIntegrationRepoSetupInitInteractivePrecheckDetected(t *testing.T) { + r := newInitRepo(t, defaultSetupYML, nil) + var req HarnessChoiceRequest + calls := 0 + res := r.runInitInHome(t, InitOptions{Harnesses: HarnessesOptions{Chooser: recordingChooser(&req, &calls, []string{}, nil)}}, ".cursor") + if res.Result != ResultApplied { + t.Fatalf("Result = %q (%s), want applied", res.Result, res.HumanText()) + } + if !reflect.DeepEqual(req.Preselected, []string{"cursor"}) { + t.Errorf("Preselected = %v, want the detected [cursor]", req.Preselected) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, ".docket.yml"))); !strings.Contains(got, "agent_harnesses: []\n") { + t.Errorf(".docket.yml = %q, want agent_harnesses: []", got) + } + if res.AgentHarnesses == nil || *res.AgentHarnesses == nil || len(*res.AgentHarnesses) != 0 { + t.Errorf("AgentHarnesses = %v, want a non-nil empty selection", res.AgentHarnesses) + } + if _, err := os.Stat(filepath.Join(r.invocation, "CLAUDE.md")); !os.IsNotExist(err) { + t.Errorf("CLAUDE.md exists (err=%v); an empty selection writes no surface", err) + } +} + +func TestIntegrationRepoSetupInitInteractiveCancelWritesNothing(t *testing.T) { + r := newInitRepo(t, defaultSetupYML, nil) + before := mustReadFile(t, filepath.Join(r.invocation, ".docket.yml")) + res := r.runInitWith(t, InitOptions{Harnesses: HarnessesOptions{Chooser: recordingChooser(nil, nil, nil, ErrHarnessChoiceCancelled)}}) + if res.Result != ResultInterrupted { + t.Fatalf("Result = %q (%s), want interrupted", res.Result, res.HumanText()) + } + if r.remoteBranchExists(t, "docket") { + t.Error("a cancelled init published the remote docket branch") + } + if _, err := os.Stat(filepath.Join(r.invocation, ".docket")); !os.IsNotExist(err) { + t.Errorf(".docket exists after a cancelled init (err=%v)", err) + } + gi, err := os.ReadFile(filepath.Join(r.invocation, ".gitignore")) + if err != nil && !os.IsNotExist(err) { + t.Fatal(err) + } + if reposetup.ValidGitignoreBlock(gi) { + t.Error("a cancelled init wrote the managed .gitignore block") + } + if after := mustReadFile(t, filepath.Join(r.invocation, ".docket.yml")); string(after) != string(before) { + t.Errorf(".docket.yml changed on a cancelled init:\n%s", after) + } +} + +func TestIntegrationRepoSetupInitNoInputWarns(t *testing.T) { + r := newInitRepo(t, defaultSetupYML, nil) + res := r.runInitWith(t, InitOptions{}) + if res.Result != ResultApplied { + t.Fatalf("Result = %q (%s), want applied", res.Result, res.HumanText()) + } + text := res.HumanText() + for _, want := range []string{"`agent_harnesses: [claude]`", ".docket.yml", reposetup.ConfigureHarnessesCommand} { + if !strings.Contains(text, want) { + t.Errorf("human text lacks %q:\n%s", want, text) + } + } + if len(res.Warnings) != 1 { + t.Errorf("Warnings = %v, want exactly one", res.Warnings) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, ".docket.yml"))); strings.Contains(got, "agent_harnesses") { + t.Errorf(".docket.yml = %q, want no agent_harnesses written", got) + } + if _, err := os.Stat(filepath.Join(r.invocation, "CLAUDE.md")); !os.IsNotExist(err) { + t.Errorf("CLAUDE.md exists (err=%v); want none without a choice", err) + } +} + +func TestIntegrationRepoSetupInitKeepsDeclaredValue(t *testing.T) { + r := newInitRepo(t, defaultSetupYML+"agent_harnesses: [codex]\n", nil) + res := r.runInitWith(t, InitOptions{Harnesses: HarnessesOptions{Chooser: failingChooser(t)}}) + if res.Result != ResultApplied { + t.Fatalf("Result = %q (%s), want applied", res.Result, res.HumanText()) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, ".docket.yml"))); strings.Count(got, "agent_harnesses: [codex]") != 1 { + t.Errorf(".docket.yml = %q, want agent_harnesses: [codex] exactly once", got) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, "AGENTS.md"))); !strings.Contains(got, "docket:dispatch") { + t.Errorf("AGENTS.md = %q, want the docket:dispatch block", got) + } + if res.AgentHarnesses != nil { + t.Errorf("AgentHarnesses = %v, want nil for a kept value", *res.AgentHarnesses) + } + if len(res.Warnings) != 0 { + t.Errorf("Warnings = %v, want none", res.Warnings) + } +} + +func TestIntegrationRepoSetupInitRerunDoesNotReask(t *testing.T) { + r := newInitRepo(t, defaultSetupYML, nil) + first := r.runInitWith(t, InitOptions{Harnesses: harnessFlag("claude")}) + if first.Result != ResultApplied { + t.Fatalf("first init = %q (%s), want applied", first.Result, first.HumanText()) + } + second := r.runInitWith(t, InitOptions{Harnesses: HarnessesOptions{Chooser: failingChooser(t)}}) + if second.Result != ResultNoOp && second.Result != ResultApplied { + t.Fatalf("second init = %q (%s), want no-op or applied", second.Result, second.HumanText()) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, ".docket.yml"))); strings.Count(got, "agent_harnesses") != 1 { + t.Errorf(".docket.yml = %q, want exactly one agent_harnesses line", got) + } +} + +func TestIntegrationRepoSetupInitRefusesBadHarnessesBeforeAnyWrite(t *testing.T) { + cases := []HarnessesOptions{ + harnessFlag("bogus", "claude"), + harnessFlag("claude", "claude"), + harnessFlag("none", "claude"), + harnessFlag(""), + } + for i, opts := range cases { + r := newInitRepo(t, defaultSetupYML, nil) + before := mustReadFile(t, filepath.Join(r.invocation, ".docket.yml")) + res := r.runInitWith(t, InitOptions{Harnesses: opts}) + if res.Result != ResultInvalidInput { + t.Errorf("%v: Result = %q (%s), want invalid-input", opts.Tokens, res.Result, res.HumanText()) + } + if i == 0 && !strings.Contains(res.HumanText(), "bogus") { + t.Errorf("%v: human text %q does not name bogus", opts.Tokens, res.HumanText()) + } + if r.remoteBranchExists(t, "docket") { + t.Errorf("%v: a refused init published the remote docket branch", opts.Tokens) + } + if after := mustReadFile(t, filepath.Join(r.invocation, ".docket.yml")); string(after) != string(before) { + t.Errorf("%v: .docket.yml changed on a refused init:\n%s", opts.Tokens, after) + } + } +} diff --git a/internal/app/repository_init.go b/internal/app/repository_init.go index 654bcc081..7f6579d6a 100644 --- a/internal/app/repository_init.go +++ b/internal/app/repository_init.go @@ -83,9 +83,10 @@ func newRepositoryOpResult(operation string, result Result, out RepositoryOpResu // effects: a parentless empty-tree root with a versioned receipt, a create-only // publish that adopts an already-published exact shape and refuses a foreign one, // the local branch + persistent .docket worktree, disabled worktree hooks, the -// unstaged managed .gitignore edit plus (only when authorized) the parent-facing -// dispatch surfaces, and the ownership record for the surfaces it owns. It never -// prompts and never reads stdin. +// unstaged managed .gitignore edit plus the chosen agent_harnesses and the +// dispatch surfaces it authorizes, and the ownership record for the surfaces it +// owns. It never reads stdin itself; its only interaction is the harness chooser +// the CLI supplies on a terminal, asked before any write. // // o selects the visibility a fresh repository is set up in (decideInitMode): // the effective `visibility` value, overridden by --private / --shared. A @@ -98,6 +99,10 @@ func RunRepositoryInit(ctx context.Context, d SetupDeps, o InitOptions) Reposito out.human = fmt.Sprintf("%s: %s: --private and --shared are mutually exclusive", OperationRepositoryInit, ResultInvalidInput) return out } + flagSel, hrefusal := parseHarnessesFlag(OperationRepositoryInit, o.Harnesses) + if hrefusal != nil { + return *hrefusal + } facts, sc, err := GatherSetupFacts(ctx, d, true) if err != nil { @@ -120,8 +125,26 @@ func RunRepositoryInit(ctx context.Context, d SetupDeps, o InitOptions) Reposito if why != "" { return initRefusal(cls.State, why) } + + // Resolved before any metadata, config, or working-tree write, so a + // cancelled picker leaves nothing behind. A fresh repository set up private + // reads only the private config this run creates. + repoDeclared := harnessRepoDeclared(sc.cfg) + _, configDisplay, _ := repoConfigTarget(sc) + if mode == layout.Private && sc.layout.Mode != layout.Private { + repoDeclared, configDisplay = false, layout.PrivateConfigDisplay + } + choice, cerr := resolveHarnessChoice(ctx, harnessChoiceInput{flag: flagSel, chooser: o.Harnesses.Chooser, policy: harnessPolicyInit, + cfg: sc.cfg, repoDeclared: repoDeclared, configDisplay: configDisplay, detect: detectHarnesses}) + if errors.Is(cerr, ErrHarnessChoiceCancelled) { + return harnessChoiceCancelled(OperationRepositoryInit, cls.State) + } + if cerr != nil { + return repositoryInternalFailure(OperationRepositoryInit, cls.State, "choosing agent harnesses", cerr) + } + if mode == layout.Private { - return runPrivateInit(ctx, d, sc, cls, o, debris) + return runPrivateInit(ctx, d, sc, cls, o, debris, choice) } // Effects 1–2: build a parentless empty-tree root with the OpInitRoot receipt @@ -151,25 +174,24 @@ func RunRepositoryInit(ctx context.Context, d SetupDeps, o InitOptions) Reposito return repositoryExternalFailure(OperationRepositoryInit, cls.State, "disabling metadata-worktree hooks", err) } - // Effect 5: prepare the managed .gitignore edit (unstaged) and, only when an - // explicit repository/repository-local agent_harnesses declaration authorizes - // them, the parent-facing dispatch surfaces via the same install machinery the - // installer's repository phase uses. + // Effect 5: prepare the managed .gitignore edit (unstaged), write the chosen + // agent_harnesses (unstaged), and install the parent-facing dispatch surfaces + // via the same install machinery the installer's repository phase uses. The + // surfaces are authorized from the post-write config re-read from disk, so a + // value chosen this run authorizes this run's surfaces, and only an explicit + // repository/repository-local declaration ever does. pending := []string{gitignoreRel} wroteGitignore, err := ensureManagedGitignore(sc.repo.PrimaryWorktree) if err != nil { return repositoryInternalFailure(OperationRepositoryInit, cls.State, "preparing the managed .gitignore block", err) } - wroteSurfaces := false - if facts.SurfacesAuthorized { - surfacePending, changed, serr := installAuthorizedSurfaces(ctx, d.Git, sc.repo.PrimaryWorktree) - if serr != nil { - return mapSurfaceFailure(cls.State, serr) - } - pending = append(pending, surfacePending...) - wroteSurfaces = changed + applied, herr := applyHarnessChoice(ctx, d.Git, sc, choice) + if herr != nil { + return harnessApplyFailure(OperationRepositoryInit, cls.State, herr) } + pending = appendPending(pending, applied.pendingConfig) + pending = appendPending(pending, applied.surfacePending...) // Test policy: discover the suite from the primary worktree and write the // generated `.docket.yml` edit as another pending, UNSTAGED review path — @@ -180,11 +202,8 @@ func RunRepositoryInit(ctx context.Context, d SetupDeps, o InitOptions) Reposito if derr != nil { return repositoryInternalFailure(OperationRepositoryInit, cls.State, "generating the test-policy config", derr) } - if docketYMLPending != "" { - pending = append(pending, docketYMLPending) - } - - pending = append(pending, debris.pending()...) + pending = appendPending(pending, docketYMLPending) + pending = appendPending(pending, debris.pending()...) sort.Strings(pending) // The metadata topology is established with pending integration-worktree @@ -192,7 +211,7 @@ func RunRepositoryInit(ctx context.Context, d SetupDeps, o InitOptions) Reposito // one that created a remote branch, worktree, block, or surface reports // applied. Both exit 0. result := ResultNoOp - if createdRemote || createdWorktree || wroteGitignore || wroteSurfaces || wroteDocketYML { + if createdRemote || createdWorktree || wroteGitignore || applied.wroteConfig || applied.wroteSurfaces || wroteDocketYML { result = ResultApplied } @@ -207,6 +226,7 @@ func RunRepositoryInit(ctx context.Context, d SetupDeps, o InitOptions) Reposito if note := testDiscoveryNote(discovery); note != "" { out.human += "\n" + note } + setHarnessResult(&out, choice, applied) return out } @@ -696,12 +716,6 @@ func repositoryInternalFailure(operation string, state reposetup.State, stage st return out } -// mapSurfaceFailure maps an init surface-installation failure to its result; -// it delegates to mapSurfaceFailureFor with the init operation. -func mapSurfaceFailure(state reposetup.State, err error) RepositoryOpResult { - return mapSurfaceFailureFor(OperationRepositoryInit, state, err) -} - // mapSurfaceFailureFor maps a surface-installation failure to operation's // result: an unprovable surface or ownership conflict is invalid-state (a human // must resolve it), a resolution error carries its own reason, and everything diff --git a/internal/app/repository_init_private.go b/internal/app/repository_init_private.go index ea2603ecd..11f9a0d00 100644 --- a/internal/app/repository_init_private.go +++ b/internal/app/repository_init_private.go @@ -23,10 +23,12 @@ import ( // InitOptions carries init's mode flags. Private and Shared override the // effective `visibility` value for a fresh repository; MetadataRemote, valid // only for a private result, names the git URL the dckt branch is pushed to in -// place of the default bare repository. +// place of the default bare repository. Harnesses carries --harnesses and the +// CLI's interactive chooser. type InitOptions struct { Private, Shared bool MetadataRemote string + Harnesses HarnessesOptions } // privateInitRecovery is appended to every private-init failure after the @@ -105,8 +107,10 @@ func isRemoteUnconfigured(err error) bool { } // runPrivateInit sets a repository up private. Every step is idempotent and -// writes nothing into the working tree, so `git status` stays clean. -func runPrivateInit(ctx context.Context, d SetupDeps, sc setupContext, cls reposetup.Classification, o InitOptions, debris setupDebrisReport) RepositoryOpResult { +// writes nothing into the working tree, so `git status` stays clean. choice is +// the harness choice RunRepositoryInit resolved before any write; step 8 writes +// it to the private config. +func runPrivateInit(ctx context.Context, d SetupDeps, sc setupContext, cls reposetup.Classification, o InitOptions, debris setupDebrisReport, choice harnessChoice) RepositoryOpResult { common := sc.repo.CommonDir changed := false @@ -273,16 +277,22 @@ func runPrivateInit(ctx context.Context, d SetupDeps, sc setupContext, cls repos return fail(repositoryExternalFailure(OperationRepositoryInit, cls.State, "disabling metadata-checkout hooks", err)) } - // 8. The parent-facing instructions: when the private config's - // agent_harnesses authorizes them, the dispatch block goes into - // .git/dckt/AGENTS.md through the installer's repository phase (which - // authorizes itself). Private init edits no .gitignore and writes nothing in - // the working tree. - _, wroteSurfaces, serr := installAuthorizedSurfaces(ctx, d.Git, sc.repo.PrimaryWorktree) - if serr != nil { - return fail(mapSurfaceFailure(cls.State, serr)) - } - changed = changed || wroteSurfaces + // 8. The chosen agent_harnesses goes into the private config, then the + // parent-facing instructions: when the private config's agent_harnesses + // authorizes them, the dispatch block goes into .git/dckt/AGENTS.md through + // the installer's repository phase (which authorizes itself from the config + // re-read after this write). Private init edits no .gitignore and writes + // nothing in the working tree. + // + // The layout resolved in step 3: a fresh repository's gather-time layout is + // still shared and would point the write at .docket.yml. + psc := sc + psc.layout = lay + applied, herr := applyHarnessChoice(ctx, d.Git, psc, choice) + if herr != nil { + return fail(harnessApplyFailure(OperationRepositoryInit, cls.State, herr)) + } + changed = changed || applied.wroteConfig || applied.wroteSurfaces // 9. Report the state the repository now classifies in. facts, sc2, err := GatherSetupFacts(ctx, d, false) @@ -311,6 +321,7 @@ func runPrivateInit(ctx context.Context, d SetupDeps, sc setupContext, cls repos if len(pending) > 0 { out.human += "\n" + strings.Join(pending, "\n") } + setHarnessResult(&out, choice, applied) return out } From b425e6164575e949095a65e3db2b6b6fd83092bc Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 21:48:32 -0400 Subject: [PATCH 06/12] feat(repository): add configure-harnesses to change a repository's agent harnesses later Pending paths come from git status filtered to the docket-managed paths, so a tracked surface removed this run (CLAUDE.md under --harnesses none) is listed. The Cursor dispatch rule is ignored by the managed .gitignore block, so its removal is not a reviewable change and is not listed. --- .../reposetup_harnesses_integration_test.go | 229 ++++++++++++++++++ .../app/repository_configure_harnesses.go | 159 ++++++++++++ .../repository_configure_harnesses_test.go | 87 +++++++ internal/app/schema_registry.go | 1 + 4 files changed, 476 insertions(+) create mode 100644 internal/app/repository_configure_harnesses.go create mode 100644 internal/app/repository_configure_harnesses_test.go diff --git a/internal/app/reposetup_harnesses_integration_test.go b/internal/app/reposetup_harnesses_integration_test.go index 81744d210..3c8038b15 100644 --- a/internal/app/reposetup_harnesses_integration_test.go +++ b/internal/app/reposetup_harnesses_integration_test.go @@ -258,3 +258,232 @@ func TestIntegrationRepoSetupInitRefusesBadHarnessesBeforeAnyWrite(t *testing.T) } } } + +// runConfigureHarnesses runs configure-harnesses against the invocation clone +// with the user roots pinned and a fresh isolated client. +func (r *initRepo) runConfigureHarnesses(t *testing.T, o ConfigureHarnessesOptions) RepositoryOpResult { + t.Helper() + pinInitUserRoots(t) + client := newGitClient(t) + res := RunRepositoryConfigureHarnesses(context.Background(), SetupDeps{Git: client, RepoDir: r.invocation}, o) + got, ok := res.(RepositoryOpResult) + if !ok { + t.Fatalf("configure-harnesses result is %T, want RepositoryOpResult", res) + } + return got +} + +// newHarnessRepo inits a shared repository with --harnesses tokens and commits +// and pushes every pending path, so it starts healthy with that selection. +func newHarnessRepo(t *testing.T, tokens ...string) *initRepo { + t.Helper() + r := newInitRepo(t, "integration_branch: main\n", nil) + res := r.runInitWith(t, InitOptions{Harnesses: harnessFlag(tokens...)}) + if res.Result != ResultApplied { + t.Fatalf("init = %q (%s), want applied", res.Result, res.HumanText()) + } + // Init also reports the Cursor rule, which the managed .gitignore block + // ignores; commit only the paths git will track. + var tracked []string + for _, p := range res.PendingPaths { + if _, err := tryGit(r.invocation, "check-ignore", "-q", "--", p); err != nil { + tracked = append(tracked, p) + } + } + r.commitAndPushMain(t, "commit init's pending edits", tracked...) + return r +} + +func TestIntegrationRepoSetupConfigureHarnessesDropsAHarness(t *testing.T) { + r := newHarnessRepo(t, "claude", "cursor") + mdc := filepath.Join(r.invocation, ".cursor", "rules", "docket-dispatch.mdc") + if _, err := os.Stat(mdc); err != nil { + t.Fatalf("fixture: the Cursor rule is missing: %v", err) + } + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("claude")}) + if res.Result != ResultApplied || res.RepositoryState != string(reposetup.StateNeedsReview) { + t.Fatalf("Result = %q state %q (%s), want applied needs-review", res.Result, res.RepositoryState, res.HumanText()) + } + if _, err := os.Stat(mdc); !os.IsNotExist(err) { + t.Errorf("the Cursor rule still exists (err=%v); a dropped harness loses its surface", err) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, "CLAUDE.md"))); !strings.Contains(got, "docket:dispatch") { + t.Errorf("CLAUDE.md = %q, want the docket:dispatch block kept", got) + } + // The Cursor rule is ignored by the managed .gitignore block, so its removal + // is no reviewable change; the config edit is. + if !contains(res.PendingPaths, ".docket.yml") { + t.Errorf("PendingPaths = %v, want .docket.yml", res.PendingPaths) + } + + res = r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("none")}) + if res.Result != ResultApplied { + t.Fatalf("none: Result = %q (%s), want applied", res.Result, res.HumanText()) + } + if got, err := os.ReadFile(filepath.Join(r.invocation, "CLAUDE.md")); err == nil && strings.Contains(string(got), "docket:dispatch") { + t.Errorf("CLAUDE.md = %q, want no docket:dispatch block after none", got) + } + // CLAUDE.md is tracked, so retiring its block (or the whole file) is a + // pending review path even when the file was deleted. + if !contains(res.PendingPaths, "CLAUDE.md") { + t.Errorf("none: PendingPaths = %v, want CLAUDE.md", res.PendingPaths) + } + yml := string(mustReadFile(t, filepath.Join(r.invocation, ".docket.yml"))) + if strings.Count(yml, "agent_harnesses:") != 1 || !strings.Contains(yml, "agent_harnesses: []\n") { + t.Errorf(".docket.yml = %q, want exactly one agent_harnesses: [] line", yml) + } +} + +func TestIntegrationRepoSetupConfigureHarnessesNoPreconditions(t *testing.T) { + t.Run("dirty", func(t *testing.T) { + r := newHarnessRepo(t, "claude") + writeRepoFile(t, r.invocation, "README.md", "edited\n") + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("codex")}) + if res.Result == ResultInvalidState { + t.Errorf("a dirty checkout was refused: %s", res.HumanText()) + } + }) + t.Run("behind", func(t *testing.T) { + r := newHarnessRepo(t, "claude") + runGit(t, r.writer, "pull", "-q", "--ff-only", "origin", "main") + r.advanceIntegration(t, "later.txt", "x\n") + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("codex")}) + if res.Result == ResultInvalidState { + t.Errorf("a checkout behind the tip was refused: %s", res.HumanText()) + } + }) + t.Run("needs-review", func(t *testing.T) { + r := newInitRepo(t, defaultSetupYML, nil) + if init := r.runInitWith(t, InitOptions{}); init.Result != ResultApplied { + t.Fatalf("init = %q (%s), want applied", init.Result, init.HumanText()) + } + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("codex")}) + if res.Result == ResultInvalidState { + t.Errorf("a needs-review repository was refused: %s", res.HumanText()) + } + }) +} + +func TestIntegrationRepoSetupConfigureHarnessesRefusesFreshAndLegacy(t *testing.T) { + cases := []struct { + name string + files map[string]string + remedy string + }{ + {"fresh", nil, "docket repository init"}, + {"legacy", map[string]string{"docs/changes/active/0001-x.md": "---\nid: 1\n---\n"}, "docket repository migrate"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r := newInitRepo(t, defaultSetupYML, tc.files) + before := mustReadFile(t, filepath.Join(r.invocation, ".docket.yml")) + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("claude")}) + if res.Result != ResultInvalidState || !strings.Contains(res.HumanText(), tc.remedy) { + t.Errorf("Result = %q (%s), want invalid-state naming %q", res.Result, res.HumanText(), tc.remedy) + } + if after := mustReadFile(t, filepath.Join(r.invocation, ".docket.yml")); string(after) != string(before) { + t.Errorf(".docket.yml changed on a refusal:\n%s", after) + } + }) + } +} + +func TestIntegrationRepoSetupConfigureHarnessesNoFlagNoTerminalRefuses(t *testing.T) { + r := newHarnessRepo(t, "claude") + before := mustReadFile(t, filepath.Join(r.invocation, ".docket.yml")) + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{}) + if res.Result != ResultInvalidInput || !strings.Contains(res.HumanText(), "--harnesses") { + t.Errorf("Result = %q (%s), want invalid-input naming --harnesses", res.Result, res.HumanText()) + } + if after := mustReadFile(t, filepath.Join(r.invocation, ".docket.yml")); string(after) != string(before) { + t.Errorf(".docket.yml changed:\n%s", after) + } +} + +func TestIntegrationRepoSetupConfigureHarnessesRefusesBadTokens(t *testing.T) { + r := newHarnessRepo(t, "claude") + before := mustReadFile(t, filepath.Join(r.invocation, ".docket.yml")) + for _, opts := range []HarnessesOptions{harnessFlag("claude", "claude"), harnessFlag("none", "claude"), harnessFlag("bogus")} { + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: opts}) + if res.Result != ResultInvalidInput { + t.Errorf("%v: Result = %q (%s), want invalid-input", opts.Tokens, res.Result, res.HumanText()) + } + if after := mustReadFile(t, filepath.Join(r.invocation, ".docket.yml")); string(after) != string(before) { + t.Errorf("%v: .docket.yml changed:\n%s", opts.Tokens, after) + } + } +} + +func TestIntegrationRepoSetupConfigureHarnessesPrechecksRepoValue(t *testing.T) { + r := newHarnessRepo(t, "claude", "cursor") + var req HarnessChoiceRequest + calls := 0 + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: HarnessesOptions{Chooser: recordingChooser(&req, &calls, []string{"claude"}, nil)}}) + if res.Result != ResultApplied { + t.Fatalf("Result = %q (%s), want applied", res.Result, res.HumanText()) + } + if calls != 1 || !reflect.DeepEqual(req.Preselected, []string{"claude", "cursor"}) { + t.Errorf("calls = %d, Preselected = %v; want 1 call pre-checking [claude cursor]", calls, req.Preselected) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, ".docket.yml"))); !strings.Contains(got, "agent_harnesses: [claude]\n") { + t.Errorf(".docket.yml = %q, want agent_harnesses: [claude]", got) + } +} + +func TestIntegrationRepoSetupConfigureHarnessesTwiceInNeedsReview(t *testing.T) { + r := newHarnessRepo(t, "cursor") + first := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("claude")}) + if first.Result != ResultApplied || first.RepositoryState != string(reposetup.StateNeedsReview) { + t.Fatalf("first = %q state %q (%s), want applied needs-review", first.Result, first.RepositoryState, first.HumanText()) + } + second := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("claude")}) + if second.Result != ResultNoOp { + t.Errorf("second = %q (%s), want no-op", second.Result, second.HumanText()) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, ".docket.yml"))); strings.Count(got, "agent_harnesses") != 1 { + t.Errorf(".docket.yml = %q, want exactly one agent_harnesses line", got) + } +} + +func TestIntegrationRepoSetupConfigureHarnessesLocalOverrideWarns(t *testing.T) { + r := newHarnessRepo(t, "claude") + writeRepoFile(t, r.invocation, ".docket.local.yml", "agent_harnesses: [cursor]\n") + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("codex")}) + if res.Result != ResultApplied { + t.Fatalf("Result = %q (%s), want applied", res.Result, res.HumanText()) + } + if len(res.Warnings) != 1 || !strings.Contains(res.Warnings[0], ".docket.local.yml") || !strings.Contains(res.Warnings[0], "`[cursor]`") { + t.Errorf("Warnings = %v, want one naming .docket.local.yml and `[cursor]`", res.Warnings) + } + if got := string(mustReadFile(t, filepath.Join(r.invocation, ".docket.yml"))); !strings.Contains(got, "agent_harnesses: [codex]\n") { + t.Errorf(".docket.yml = %q, want agent_harnesses: [codex]", got) + } + if _, err := os.Stat(filepath.Join(r.invocation, ".cursor", "rules", "docket-dispatch.mdc")); err != nil { + t.Errorf("the applied value is cursor, but its rule is missing: %v", err) + } + if got, err := os.ReadFile(filepath.Join(r.invocation, "AGENTS.md")); err == nil && strings.Contains(string(got), "docket:dispatch") { + t.Errorf("AGENTS.md = %q, want no docket:dispatch block while the override applies cursor", got) + } +} + +func TestIntegrationRepoSetupPrivateConfigureHarnesses(t *testing.T) { + r, data := initPrivateHealthy(t) + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("claude")}) + if res.Result != ResultApplied || res.RepositoryState != string(reposetup.StateHealthy) { + t.Fatalf("Result = %q state %q (%s), want applied healthy", res.Result, res.RepositoryState, res.HumanText()) + } + if len(res.PendingPaths) != 0 { + t.Errorf("PendingPaths = %v, want none for a private repository", res.PendingPaths) + } + if !strings.Contains(res.HumanText(), ".git/dckt/config.yml") { + t.Errorf("human text %q does not name .git/dckt/config.yml", res.HumanText()) + } + lay := privateLayoutOf(t, r.invocation, data) + if got := string(mustReadFile(t, lay.ConfigPath)); !strings.Contains(got, "agent_harnesses: [claude]\n") { + t.Errorf("%s = %q, want agent_harnesses: [claude]", lay.ConfigPath, got) + } + agents := filepath.Join(filepath.Dir(lay.ConfigPath), "AGENTS.md") + if got, err := os.ReadFile(agents); err != nil || !strings.Contains(string(got), "docket:dispatch") { + t.Errorf("%s = %q (%v), want the docket:dispatch block", agents, got, err) + } +} diff --git a/internal/app/repository_configure_harnesses.go b/internal/app/repository_configure_harnesses.go new file mode 100644 index 000000000..07fb84be7 --- /dev/null +++ b/internal/app/repository_configure_harnesses.go @@ -0,0 +1,159 @@ +package app + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + + "github.com/danielhanold/docket/internal/gitcli" + "github.com/danielhanold/docket/internal/layout" + "github.com/danielhanold/docket/internal/reposetup" +) + +// OperationRepositoryConfigureHarnesses is the operation key `repository +// configure-harnesses` records. +const OperationRepositoryConfigureHarnesses = "repository.configure-harnesses" + +// ConfigureHarnessesOptions carries configure-harnesses' one input: the +// --harnesses value and the CLI's interactive chooser. +type ConfigureHarnessesOptions struct { + Harnesses HarnessesOptions +} + +// RunRepositoryConfigureHarnesses changes an initialized repository's +// agent_harnesses through init's shared choose-harnesses step +// (resolveHarnessChoice, then applyHarnessChoice): it writes the selection to +// the repository's config target and refreshes the dispatch surfaces the new +// value authorizes, removing those of dropped harnesses. +// +// Input is validated before any repository read: with neither --harnesses nor a +// chooser it refuses invalid-input, and a bad selection is invalid-input. It +// needs no clean checkout and no at-tip primary, and it admits needs-review, so +// it can run again before the previous edit is committed. A fresh repository is +// pointed at `docket repository init`, a legacy one at `docket repository +// migrate`, and any other unhealthy state at `docket repository check`. It never +// commits and never stages: a shared repository's edits ride back as pending +// review paths. +func RunRepositoryConfigureHarnesses(ctx context.Context, d SetupDeps, o ConfigureHarnessesOptions) OperationResult { + const op = OperationRepositoryConfigureHarnesses + if !o.Harnesses.Set && o.Harnesses.Chooser == nil { + out := newRepositoryOpResult(op, ResultInvalidInput, RepositoryOpResult{}) + out.human = fmt.Sprintf("%s: %s: there is no terminal to show the checklist on; pass --harnesses (for example --harnesses claude,cursor), or --harnesses none to record that no agents are used", + op, ResultInvalidInput) + return out + } + flagSel, refusal := parseHarnessesFlag(op, o.Harnesses) + if refusal != nil { + return *refusal + } + + facts, sc, err := GatherSetupFacts(ctx, d, true) + if err != nil { + return repositoryGatherFailure(op, err) + } + // The healthy postconditions are the check-only augmentation probes, as for + // configure-tests; they are meaningful only once the remote docket branch is + // present. + if facts.RemoteMetadata.Presence == reposetup.PresencePresent { + augmentCheckFacts(ctx, d.Git, &facts, sc) + } + cls, refused := configureHarnessesGuard(facts) + if refused != nil { + return *refused + } + + _, display, _ := repoConfigTarget(sc) + choice, cerr := resolveHarnessChoice(ctx, harnessChoiceInput{flag: flagSel, chooser: o.Harnesses.Chooser, policy: harnessPolicyConfigure, + cfg: sc.cfg, repoDeclared: harnessRepoDeclared(sc.cfg), configDisplay: display, detect: detectHarnesses}) + if errors.Is(cerr, ErrHarnessChoiceCancelled) { + return harnessChoiceCancelled(op, cls.State) + } + if cerr != nil { + return repositoryInternalFailure(op, cls.State, "choosing agent harnesses", cerr) + } + + applied, herr := applyHarnessChoice(ctx, d.Git, sc, choice) + if herr != nil { + return harnessApplyFailure(op, cls.State, herr) + } + pending, perr := workingTreePendingPaths(ctx, d.Git, sc) + if perr != nil { + return repositoryExternalFailure(op, cls.State, "listing the pending review paths", perr) + } + + result := ResultNoOp + if applied.wroteConfig || applied.wroteSurfaces { + result = ResultApplied + } + state := cls.State + if len(pending) > 0 && state == reposetup.StateHealthy { + state = reposetup.StateNeedsReview + } + out := newRepositoryOpResult(op, result, RepositoryOpResult{ + RepositoryState: string(state), + PendingPaths: pending, + SourceRevision: sc.sourceRevision, + }) + list := "`" + reposetup.FormatHarnessList(choice.selection) + "`" + switch { + case result == ResultNoOp: + out.human = fmt.Sprintf("%s: %s (%s): agent_harnesses is already %s and its instructions are current; nothing to write", op, ResultNoOp, state, list) + case display == layout.PrivateConfigDisplay: + out.human = fmt.Sprintf("agent harnesses set to %s (%s); wrote %s", list, state, layout.PrivateConfigDisplay) + default: + out.human = fmt.Sprintf("agent harnesses set to %s (%s); review and commit the pending paths: %s", list, state, strings.Join(pending, ", ")) + } + setHarnessResult(&out, choice, applied) + return out +} + +// configureHarnessesGuard classifies once and admits healthy, needs-review, or +// either once init's clean-checkout and at-tip preconditions are set aside; +// every other state is an invalid-state refusal whose remedy is valid there. +// Pure over the gathered facts. +func configureHarnessesGuard(facts reposetup.Facts) (reposetup.Classification, *RepositoryOpResult) { + cls := reposetup.Classify(facts) + refuse := func(remedy string) (reposetup.Classification, *RepositoryOpResult) { + out := newRepositoryOpResult(OperationRepositoryConfigureHarnesses, ResultInvalidState, RepositoryOpResult{RepositoryState: string(cls.State)}) + out.human = fmt.Sprintf("%s: %s (%s): %s", OperationRepositoryConfigureHarnesses, ResultInvalidState, cls.State, remedy) + return cls, &out + } + switch cls.State { + case reposetup.StateHealthy, reposetup.StateNeedsReview: + return cls, nil + case reposetup.StateFresh: + return refuse("repository is not initialized; run `docket repository init` (it takes --harnesses too)") + case reposetup.StateLegacy: + return refuse("repository has a legacy single-branch layout; run `docket repository migrate`, then re-run `" + reposetup.ConfigureHarnessesCommand + "`") + } + relaxed := facts + relaxed.PrimaryClean, relaxed.PrimaryAtRemoteTip = reposetup.PresencePresent, reposetup.PresencePresent + if s := reposetup.Classify(relaxed).State; s == reposetup.StateHealthy || s == reposetup.StateNeedsReview { + return cls, nil + } + return refuse("repository is not in a healthy state; run `docket repository check` and resolve the reported findings first") +} + +// workingTreePendingPaths lists, sorted, every docket-managed working-tree path +// that differs from HEAD in the primary worktree — read from git status, so a +// surface removed this run counts as pending too. A private repository has +// none: its config and surfaces live outside the working tree. +func workingTreePendingPaths(ctx context.Context, git *gitcli.Client, sc setupContext) ([]string, error) { + if sc.layout.Mode == layout.Private { + return nil, nil + } + changes, err := git.ChangedPaths(ctx, sc.repo.PrimaryWorktree) + if err != nil { + return nil, err + } + var pending []string + for _, ch := range changes { + if rel := string(ch.Path); docketManagedWorktreePaths[rel] { + pending = appendPending(pending, rel) + } + } + sort.Strings(pending) + return pending, nil +} diff --git a/internal/app/repository_configure_harnesses_test.go b/internal/app/repository_configure_harnesses_test.go new file mode 100644 index 000000000..df6bc9a73 --- /dev/null +++ b/internal/app/repository_configure_harnesses_test.go @@ -0,0 +1,87 @@ +package app + +import ( + "context" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/reposetup" +) + +// TestConfigureHarnessesGuard proves configure-harnesses admits healthy, +// needs-review, and the states that are only init's clean-checkout or at-tip +// preconditions away from those, and refuses every other state with the remedy +// valid there. +func TestConfigureHarnessesGuard(t *testing.T) { + admitted := map[string]func(*reposetup.Facts){ + "healthy": func(*reposetup.Facts) {}, + "needs-review": func(f *reposetup.Facts) { f.PendingReviewPaths = []string{".docket.yml"} }, + "dirty primary": func(f *reposetup.Facts) { f.PrimaryClean = reposetup.PresenceAbsent }, + "behind the tip": func(f *reposetup.Facts) { f.PrimaryAtRemoteTip = reposetup.PresenceAbsent }, + } + for name, mutate := range admitted { + t.Run("admits "+name, func(t *testing.T) { + facts := healthyConfigureFacts() + mutate(&facts) + cls, refusal := configureHarnessesGuard(facts) + if refusal != nil { + t.Fatalf("state %q refused: %s", cls.State, refusal.HumanText()) + } + }) + } + + refused := []struct { + name string + mutate func(*reposetup.Facts) + remedy string + }{ + {"fresh", func(f *reposetup.Facts) { + f.RemoteMetadata = reposetup.BranchFact{Presence: reposetup.PresenceAbsent} + }, "docket repository init"}, + {"legacy", func(f *reposetup.Facts) { + f.RemoteMetadata = reposetup.BranchFact{Presence: reposetup.PresenceAbsent} + f.LiveSurface = reposetup.PresencePresent + }, "docket repository migrate"}, + {"foreign worktree", func(f *reposetup.Facts) { f.DocketWorktree.Foreign = true }, "docket repository check"}, + {"hooks on", func(f *reposetup.Facts) { f.DocketWorktree.HooksOff = reposetup.PresenceAbsent }, "docket repository check"}, + } + for _, tc := range refused { + t.Run("refuses "+tc.name, func(t *testing.T) { + facts := healthyConfigureFacts() + tc.mutate(&facts) + cls, refusal := configureHarnessesGuard(facts) + if refusal == nil { + t.Fatalf("state %q admitted; want a refusal", cls.State) + } + if refusal.Result != ResultInvalidState { + t.Errorf("Result = %q, want invalid-state", refusal.Result) + } + if !strings.Contains(refusal.HumanText(), tc.remedy) { + t.Errorf("refusal %q does not name %q", refusal.HumanText(), tc.remedy) + } + }) + } +} + +// Missing and invalid input are refused before ANY repository read: the nil Git +// client would fail the gather if validation ran after it. +func TestRunRepositoryConfigureHarnessesRefusesInputBeforeGather(t *testing.T) { + cases := map[string]HarnessesOptions{ + "no flag, no chooser": {}, + "bogus": {Set: true, Tokens: []string{"bogus"}}, + "duplicate": {Set: true, Tokens: []string{"claude", "claude"}}, + "none with a name": {Set: true, Tokens: []string{"none", "claude"}}, + } + for name, opts := range cases { + t.Run(name, func(t *testing.T) { + res := RunRepositoryConfigureHarnesses(context.Background(), SetupDeps{}, ConfigureHarnessesOptions{Harnesses: opts}) + got, ok := res.(RepositoryOpResult) + if !ok { + t.Fatalf("result is %T, want RepositoryOpResult", res) + } + if got.Result != ResultInvalidInput { + t.Errorf("Result = %q (%s), want invalid-input", got.Result, got.HumanText()) + } + }) + } +} diff --git a/internal/app/schema_registry.go b/internal/app/schema_registry.go index 43a97d179..ed7a6413b 100644 --- a/internal/app/schema_registry.go +++ b/internal/app/schema_registry.go @@ -103,6 +103,7 @@ var operationBindings = []OperationBinding{ {ID: "maintenance.sweep", Request: nil, Result: MaintenanceResult{}}, // MaintenanceSweep {ID: "pr.publish", Request: PRPublishInput{}, Result: PRPublishResult{}}, // PRPublish {ID: "repository.check", Request: nil, Result: RepositoryCheckResult{}}, // RunRepositoryCheck + {ID: "repository.configure-harnesses", Request: nil, Result: RepositoryOpResult{}}, // RunRepositoryConfigureHarnesses {ID: "repository.configure-tests", Request: nil, Result: RepositoryOpResult{}}, // RunRepositoryConfigureTests {ID: "repository.init", Request: nil, Result: RepositoryOpResult{}}, // RunRepositoryInit {ID: "repository.migrate", Request: nil, Result: RepositoryMigrateResult{}}, // RunRepositoryMigrate From 816ad221ec2c10d27fac60c1ab309d2c05bd1a8d Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 21:50:53 -0400 Subject: [PATCH 07/12] feat(cli): add --harnesses, configure-harnesses, and a checkbox picker built on huh --- go.mod | 31 ++++++- go.sum | 75 +++++++++++++++++ internal/cli/harness_picker.go | 49 +++++++++++ internal/cli/harness_picker_test.go | 74 +++++++++++++++++ internal/cli/install.go | 123 ++++++++++++++-------------- internal/cli/repository.go | 65 ++++++++++++++- internal/cli/repository_test.go | 105 +++++++++++++++++++++++- 7 files changed, 455 insertions(+), 67 deletions(-) create mode 100644 internal/cli/harness_picker.go create mode 100644 internal/cli/harness_picker_test.go diff --git a/go.mod b/go.mod index 6d7420a14..d032b9f86 100644 --- a/go.mod +++ b/go.mod @@ -5,10 +5,39 @@ go 1.26.0 toolchain go1.26.5 require ( + github.com/charmbracelet/huh v1.0.0 github.com/pelletier/go-toml/v2 v2.2.4 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.9 go.yaml.in/yaml/v3 v3.0.4 ) -require github.com/inconshreveable/mousetrap v1.1.0 // indirect +require ( + github.com/atotto/clipboard v0.1.4 // indirect + github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect + github.com/catppuccin/go v0.3.0 // indirect + github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 // indirect + github.com/charmbracelet/bubbletea v1.3.6 // indirect + github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc // indirect + github.com/charmbracelet/lipgloss v1.1.0 // indirect + github.com/charmbracelet/x/ansi v0.9.3 // indirect + github.com/charmbracelet/x/cellbuf v0.0.13 // indirect + github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0 // indirect + github.com/charmbracelet/x/term v0.2.1 // indirect + github.com/dustin/go-humanize v1.0.1 // indirect + github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect + github.com/inconshreveable/mousetrap v1.1.0 // indirect + github.com/lucasb-eyer/go-colorful v1.2.0 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect + github.com/mattn/go-localereader v0.0.1 // indirect + github.com/mattn/go-runewidth v0.0.16 // indirect + github.com/mitchellh/hashstructure/v2 v2.0.2 // indirect + github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect + github.com/muesli/cancelreader v0.2.2 // indirect + github.com/muesli/termenv v0.16.0 // indirect + github.com/rivo/uniseg v0.4.7 // indirect + github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect + golang.org/x/sync v0.15.0 // indirect + golang.org/x/sys v0.33.0 // indirect + golang.org/x/text v0.23.0 // indirect +) diff --git a/go.sum b/go.sum index 6aec3acbf..dbe141cfa 100644 --- a/go.sum +++ b/go.sum @@ -1,14 +1,89 @@ +github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ= +github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE= +github.com/atotto/clipboard v0.1.4 h1:EH0zSVneZPSuFR11BlR9YppQTVDbh5+16AmcJi4g1z4= +github.com/atotto/clipboard v0.1.4/go.mod h1:ZY9tmq7sm5xIbd9bOK4onWV4S6X0u6GY7Vn0Yu86PYI= +github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= +github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= +github.com/aymanbagabas/go-udiff v0.3.1 h1:LV+qyBQ2pqe0u42ZsUEtPiCaUoqgA9gYRDs3vj1nolY= +github.com/aymanbagabas/go-udiff v0.3.1/go.mod h1:G0fsKmG+P6ylD0r6N/KgQD/nWzgfnl8ZBcNLgcbrw8E= +github.com/catppuccin/go v0.3.0 h1:d+0/YicIq+hSTo5oPuRi5kOpqkVA5tAsU6dNhvRu+aY= +github.com/catppuccin/go v0.3.0/go.mod h1:8IHJuMGaUUjQM82qBrGNBv7LFq6JI3NnQCF6MOlZjpc= +github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 h1:JFgG/xnwFfbezlUnFMJy0nusZvytYysV4SCS2cYbvws= +github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7/go.mod h1:ISC1gtLcVilLOf23wvTfoQuYbW2q0JevFxPfUzZ9Ybw= +github.com/charmbracelet/bubbletea v1.3.6 h1:VkHIxPJQeDt0aFJIsVxw8BQdh/F/L2KKZGsK6et5taU= +github.com/charmbracelet/bubbletea v1.3.6/go.mod h1:oQD9VCRQFF8KplacJLo28/jofOI2ToOfGYeFgBBxHOc= +github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc h1:4pZI35227imm7yK2bGPcfpFEmuY1gc2YSTShr4iJBfs= +github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc/go.mod h1:X4/0JoqgTIPSFcRA/P6INZzIuyqdFY5rm8tb41s9okk= +github.com/charmbracelet/huh v1.0.0 h1:wOnedH8G4qzJbmhftTqrpppyqHakl/zbbNdXIWJyIxw= +github.com/charmbracelet/huh v1.0.0/go.mod h1:5YVc+SlZ1IhQALxRPpkGwwEKftN/+OlJlnJYlDRFqN4= +github.com/charmbracelet/lipgloss v1.1.0 h1:vYXsiLHVkK7fp74RkV7b2kq9+zDLoEU4MZoFqR/noCY= +github.com/charmbracelet/lipgloss v1.1.0/go.mod h1:/6Q8FR2o+kj8rz4Dq0zQc3vYf7X+B0binUUBwA0aL30= +github.com/charmbracelet/x/ansi v0.9.3 h1:BXt5DHS/MKF+LjuK4huWrC6NCvHtexww7dMayh6GXd0= +github.com/charmbracelet/x/ansi v0.9.3/go.mod h1:3RQDQ6lDnROptfpWuUVIUG64bD2g2BgntdxH0Ya5TeE= +github.com/charmbracelet/x/cellbuf v0.0.13 h1:/KBBKHuVRbq1lYx5BzEHBAFBP8VcQzJejZ/IA3iR28k= +github.com/charmbracelet/x/cellbuf v0.0.13/go.mod h1:xe0nKWGd3eJgtqZRaN9RjMtK7xUYchjzPr7q6kcvCCs= +github.com/charmbracelet/x/conpty v0.1.0 h1:4zc8KaIcbiL4mghEON8D72agYtSeIgq8FSThSPQIb+U= +github.com/charmbracelet/x/conpty v0.1.0/go.mod h1:rMFsDJoDwVmiYM10aD4bH2XiRgwI7NYJtQgl5yskjEQ= +github.com/charmbracelet/x/errors v0.0.0-20240508181413-e8d8b6e2de86 h1:JSt3B+U9iqk37QUU2Rvb6DSBYRLtWqFqfxf8l5hOZUA= +github.com/charmbracelet/x/errors v0.0.0-20240508181413-e8d8b6e2de86/go.mod h1:2P0UgXMEa6TsToMSuFqKFQR+fZTO9CNGUNokkPatT/0= +github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91 h1:payRxjMjKgx2PaCWLZ4p3ro9y97+TVLZNaRZgJwSVDQ= +github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U= +github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0 h1:qko3AQ4gK1MTS/de7F5hPGx6/k1u0w4TeYmBFwzYVP4= +github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0/go.mod h1:pBhA0ybfXv6hDjQUZ7hk1lVxBiUbupdw5R31yPUViVQ= +github.com/charmbracelet/x/term v0.2.1 h1:AQeHeLZ1OqSXhrAWpYUtZyX1T3zVxfpZuEQMIQaGIAQ= +github.com/charmbracelet/x/term v0.2.1/go.mod h1:oQ4enTYFV7QN4m0i9mzHrViD7TQKvNEEkHUMCmsxdUg= +github.com/charmbracelet/x/termios v0.1.1 h1:o3Q2bT8eqzGnGPOYheoYS8eEleT5ZVNYNy8JawjaNZY= +github.com/charmbracelet/x/termios v0.1.1/go.mod h1:rB7fnv1TgOPOyyKRJ9o+AsTU/vK5WHJ2ivHeut/Pcwo= +github.com/charmbracelet/x/xpty v0.1.2 h1:Pqmu4TEJ8KeA9uSkISKMU3f+C1F6OGBn8ABuGlqCbtI= +github.com/charmbracelet/x/xpty v0.1.2/go.mod h1:XK2Z0id5rtLWcpeNiMYBccNNBrP2IJnzHI0Lq13Xzq4= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= +github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= +github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= +github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= +github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6baUTXGLOoWe4PQhGxaX0KpnayAqC48p4= +github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY= +github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2JC/oIi4= +github.com/mattn/go-localereader v0.0.1/go.mod h1:8fBrzywKY7BI3czFoHkuzRoWE9C+EiG4R1k4Cjx5p88= +github.com/mattn/go-runewidth v0.0.16 h1:E5ScNMtiwvlvB5paMFdw9p4kSQzbXFikJ5SQO6TULQc= +github.com/mattn/go-runewidth v0.0.16/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= +github.com/mitchellh/hashstructure/v2 v2.0.2 h1:vGKWl0YJqUNxE8d+h8f6NJLcCJrgbhC4NcD46KavDd4= +github.com/mitchellh/hashstructure/v2 v2.0.2/go.mod h1:MG3aRVU/N29oo/V/IhBX8GR/zz4kQkprJgF2EVszyDE= +github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 h1:ZK8zHtRHOkbHy6Mmr5D264iyp3TiX5OmNcI5cIARiQI= +github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6/go.mod h1:CJlz5H+gyd6CUWT45Oy4q24RdLyn7Md9Vj2/ldJBSIo= +github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= +github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= +github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc= +github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= +github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= +github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= +github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= +github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= +golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= +golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= +golang.org/x/sync v0.15.0 h1:KWH3jNZsfyT6xfAfKiz6MRNmd46ByHDYaZ7KSkCtdW8= +golang.org/x/sync v0.15.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= +golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/text v0.23.0 h1:D71I7dUrlY+VX0gQShAThNGHFxZ13dGLBHQLVl1mJlY= +golang.org/x/text v0.23.0/go.mod h1:/BLNzu4aZCJ1+kcD0DNRotWKage4q2rGVAg4o22unh4= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/cli/harness_picker.go b/internal/cli/harness_picker.go new file mode 100644 index 000000000..a9ab3a494 --- /dev/null +++ b/internal/cli/harness_picker.go @@ -0,0 +1,49 @@ +// The interactive harness picker `repository init` and `repository +// configure-harnesses` show on a terminal. This file is docket's only importer +// of github.com/charmbracelet/huh: internal/app sees only the app.HarnessChooser +// seam, so the terminal UI dependency never reaches the service layer. +package cli + +import ( + "context" + "errors" + + "github.com/charmbracelet/huh" + + "github.com/danielhanold/docket/internal/app" +) + +// runHarnessForm runs the picker form. It is a seam so a test can stand in for +// the terminal. +var runHarnessForm = func(ctx context.Context, f *huh.Form) error { return f.RunWithContext(ctx) } + +// newHarnessForm builds a checklist of every harness in req.Options order with +// req.Preselected pre-checked (huh selects the options already in *picked). +func newHarnessForm(req app.HarnessChoiceRequest, picked *[]string) *huh.Form { + *picked = append([]string{}, req.Preselected...) + field := huh.NewMultiSelect[string](). + Title("Which coding agents should get docket's instructions in this repository?"). + Description("Space toggles, Enter confirms. Written to " + req.ConfigPath + "; confirm with nothing checked to record that no agents are used."). + Value(picked). + Options(huh.NewOptions(req.Options...)...) + return huh.NewForm(huh.NewGroup(field)) +} + +// huhHarnessChooser is the production app.HarnessChooser: it shows the picker +// and maps a user abort (Ctrl-C or Esc) to app.ErrHarnessChoiceCancelled. A +// confirmed empty checklist is a non-nil empty selection, which records that no +// agents are used. +func huhHarnessChooser(ctx context.Context, req app.HarnessChoiceRequest) ([]string, error) { + var picked []string + err := runHarnessForm(ctx, newHarnessForm(req, &picked)) + if errors.Is(err, huh.ErrUserAborted) { + return nil, app.ErrHarnessChoiceCancelled + } + if err != nil { + return nil, err + } + if picked == nil { + picked = []string{} + } + return picked, nil +} diff --git a/internal/cli/harness_picker_test.go b/internal/cli/harness_picker_test.go new file mode 100644 index 000000000..436d82096 --- /dev/null +++ b/internal/cli/harness_picker_test.go @@ -0,0 +1,74 @@ +package cli + +import ( + "context" + "errors" + "io" + "slices" + "strings" + "testing" + "testing/iotest" + + "github.com/charmbracelet/huh" + + "github.com/danielhanold/docket/internal/app" +) + +func harnessRequest(pre ...string) app.HarnessChoiceRequest { + return app.HarnessChoiceRequest{Options: []string{"claude", "codex", "cursor", "opencode"}, Preselected: pre, ConfigPath: ".docket.yml"} +} + +// TestHarnessFormPrechecksAndToggles drives the form in huh's accessible mode: +// cursor arrives pre-checked, toggling option 1 adds claude, and 0 confirms. +func TestHarnessFormPrechecksAndToggles(t *testing.T) { + var picked []string + f := newHarnessForm(harnessRequest("cursor"), &picked) + in := iotest.OneByteReader(strings.NewReader("1\n0\n")) + if err := f.WithAccessible(true).WithInput(in).WithOutput(io.Discard).Run(); err != nil { + t.Fatal(err) + } + slices.Sort(picked) + if !slices.Equal(picked, []string{"claude", "cursor"}) { + t.Fatalf("picked = %v, want [claude cursor]", picked) + } +} + +// TestHarnessFormConfirmWithNothingChecked proves confirming an empty checklist +// is a valid answer (no agents), not a refusal. +func TestHarnessFormConfirmWithNothingChecked(t *testing.T) { + var picked []string + f := newHarnessForm(harnessRequest(), &picked) + in := iotest.OneByteReader(strings.NewReader("0\n")) + if err := f.WithAccessible(true).WithInput(in).WithOutput(io.Discard).Run(); err != nil { + t.Fatal(err) + } + if len(picked) != 0 { + t.Fatalf("picked = %v, want empty", picked) + } +} + +func stubHarnessForm(t *testing.T, fn func(ctx context.Context, f *huh.Form) error) { + t.Helper() + old := runHarnessForm + runHarnessForm = fn + t.Cleanup(func() { runHarnessForm = old }) +} + +func TestHuhHarnessChooserMapsAbortToCancel(t *testing.T) { + stubHarnessForm(t, func(context.Context, *huh.Form) error { return huh.ErrUserAborted }) + _, err := huhHarnessChooser(context.Background(), harnessRequest("claude")) + if !errors.Is(err, app.ErrHarnessChoiceCancelled) { + t.Fatalf("err = %v, want ErrHarnessChoiceCancelled", err) + } +} + +func TestHuhHarnessChooserReturnsNonNilEmpty(t *testing.T) { + stubHarnessForm(t, func(context.Context, *huh.Form) error { return nil }) + got, err := huhHarnessChooser(context.Background(), harnessRequest()) + if err != nil { + t.Fatal(err) + } + if got == nil || len(got) != 0 { + t.Fatalf("got = %#v, want a non-nil empty selection", got) + } +} diff --git a/internal/cli/install.go b/internal/cli/install.go index 2a58386b2..b5b136967 100644 --- a/internal/cli/install.go +++ b/internal/cli/install.go @@ -66,67 +66,68 @@ var assetIndependent = map[string]bool{ "evidence verify": true, "evidence recertify": true, // composes the local gate + GitHub PR edit; reads no installed assets - "pr": true, // the group itself; it reports a missing command - "pr publish": true, - "run": true, // the group itself; it reports a missing command - "run verify": true, - "run start": true, - "run verdict": true, - "run continue": true, - "run cancel": true, - "learning": true, // the group itself; it reports a missing command - "learning record": true, - "learning update": true, - "adr": true, // the group itself; it reports a missing command - "adr record": true, - "adr supersede": true, - "adr reverse": true, - "gate": true, // the group itself; it reports a missing command - "gate launch": true, - "gate observe": true, - "gate stop": true, - "gate recover": true, - "gate cleanup": true, - "gate drive": true, // the group itself; it reports a missing command - "gate drive start": true, - "gate drive advance": true, - "gate drive handoff": true, - "gate drive claim": true, - "finalize": true, // the group itself; it reports a missing command - "finalize retarget-children": true, - "finalize rebase": true, - "finalize rebase-continue": true, - "finalize rebase-abort": true, - "finalize resolver-reserve": true, - "finalize publish": true, - "finalize block": true, - "finalize clear-block": true, - "finalize merge": true, - "finalize closeout": true, - "finalize cleanup": true, - "maintenance": true, // the group itself; it reports a missing command - "maintenance sweep": true, - "maintenance preflight": true, - "repository": true, // the group itself; it reports a missing command - "repository init": true, - "repository check": true, - "repository migrate": true, - "repository repair": true, - "repository prepare": true, - "repository configure-tests": true, - "repository sync-integration": true, - "agent": true, - "instructions": true, // session hooks must answer on a machine mid-install; it reads only the repository's own file - "diagnostic": true, // the group itself; it reports a missing command - "diagnostic runtime": true, - "diagnostic config": true, - "install": true, - "install check": true, - "install collect": true, - "uninstall": true, - "development": true, - "development install": true, - "development test": true, // the Go-native whole-suite runner reads this checkout, never installed assets (change 0318) + "pr": true, // the group itself; it reports a missing command + "pr publish": true, + "run": true, // the group itself; it reports a missing command + "run verify": true, + "run start": true, + "run verdict": true, + "run continue": true, + "run cancel": true, + "learning": true, // the group itself; it reports a missing command + "learning record": true, + "learning update": true, + "adr": true, // the group itself; it reports a missing command + "adr record": true, + "adr supersede": true, + "adr reverse": true, + "gate": true, // the group itself; it reports a missing command + "gate launch": true, + "gate observe": true, + "gate stop": true, + "gate recover": true, + "gate cleanup": true, + "gate drive": true, // the group itself; it reports a missing command + "gate drive start": true, + "gate drive advance": true, + "gate drive handoff": true, + "gate drive claim": true, + "finalize": true, // the group itself; it reports a missing command + "finalize retarget-children": true, + "finalize rebase": true, + "finalize rebase-continue": true, + "finalize rebase-abort": true, + "finalize resolver-reserve": true, + "finalize publish": true, + "finalize block": true, + "finalize clear-block": true, + "finalize merge": true, + "finalize closeout": true, + "finalize cleanup": true, + "maintenance": true, // the group itself; it reports a missing command + "maintenance sweep": true, + "maintenance preflight": true, + "repository": true, // the group itself; it reports a missing command + "repository init": true, + "repository check": true, + "repository migrate": true, + "repository repair": true, + "repository prepare": true, + "repository configure-tests": true, + "repository configure-harnesses": true, + "repository sync-integration": true, + "agent": true, + "instructions": true, // session hooks must answer on a machine mid-install; it reads only the repository's own file + "diagnostic": true, // the group itself; it reports a missing command + "diagnostic runtime": true, + "diagnostic config": true, + "install": true, + "install check": true, + "install collect": true, + "uninstall": true, + "development": true, + "development install": true, + "development test": true, // the Go-native whole-suite runner reads this checkout, never installed assets (change 0318) } // assetDependent is the explicit complement for shipped commands that require diff --git a/internal/cli/repository.go b/internal/cli/repository.go index 6eb33a84d..600c18359 100644 --- a/internal/cli/repository.go +++ b/internal/cli/repository.go @@ -25,7 +25,9 @@ import ( // flow, which lives here because it is a terminal interaction: the service // returns the plan, and the CLI prints it and re-invokes with an explicit // authorization keyed on exactly the pinned revision the human saw (learning -// decide-and-act-on-the-same-copy). +// decide-and-act-on-the-same-copy). The CLI also owns the harness picker +// (harness_picker.go): init and configure-harnesses hand the app a chooser only +// on an interactive, non-JSON session with no --harnesses value. // repositoryInitRunner, repositoryCheckRunner, and repositoryMigrateRunner are // the app entry points the repository subcommands dispatch to. They are package @@ -59,6 +61,9 @@ var ( repositoryConfigureTestsRunner = func(ctx context.Context, d app.SetupDeps, o app.ConfigureTestsOptions) app.OperationResult { return app.RunRepositoryConfigureTests(ctx, d, o) } + repositoryConfigureHarnessesRunner = func(ctx context.Context, d app.SetupDeps, o app.ConfigureHarnessesOptions) app.OperationResult { + return app.RunRepositoryConfigureHarnesses(ctx, d, o) + } repositorySyncIntegrationRunner = func(ctx context.Context, d app.SetupDeps) app.OperationResult { return app.RunRepositorySyncIntegration(ctx, d) } @@ -73,6 +78,44 @@ var repositoryConfirmInteractive = func() bool { return err == nil && fi.Mode()&os.ModeCharDevice != 0 } +// repositoryHarnessesInteractive reports whether init and configure-harnesses +// may show the harness picker: stdin and stdout must both be terminals, since +// the picker both reads keys and redraws the screen. It is a seam so a test can +// force either branch without a real TTY. +var repositoryHarnessesInteractive = func() bool { + return isCharDevice(os.Stdin) && isCharDevice(os.Stdout) +} + +// isCharDevice reports whether f is a character device (a terminal). +func isCharDevice(f *os.File) bool { + fi, err := f.Stat() + return err == nil && fi.Mode()&os.ModeCharDevice != 0 +} + +// repositoryHarnessChooser is the chooser harnessesOptions supplies on an +// interactive session; a seam so a test never opens the real picker. +var repositoryHarnessChooser app.HarnessChooser = huhHarnessChooser + +// harnessesUsage documents --harnesses on init and configure-harnesses. +const harnessesUsage = "comma `list` of coding agents to write docket's instructions for (claude, codex, cursor, opencode), or none; without it, a terminal shows a checklist" + +// harnessesOptions reads --harnesses by Changed (an explicit empty value must +// reach the app, which refuses it) and supplies the picker only when no flag +// was given, --json is off, and the session is interactive. +func harnessesOptions(c *cobra.Command) app.HarnessesOptions { + var o app.HarnessesOptions + if c.Flags().Changed("harnesses") { + o.Set = true + if o.Tokens, _ = c.Flags().GetStringSlice("harnesses"); o.Tokens == nil { + o.Tokens = []string{} + } + } + if jsonMode, _ := c.Flags().GetBool("json"); !o.Set && !jsonMode && repositoryHarnessesInteractive() { + o.Chooser = repositoryHarnessChooser + } + return o +} + // newRepositoryCommand builds the `repository` command group. setResult is the // closure that hands a computed operation result back to Run's single // presentation point, mirroring every other command family. @@ -97,6 +140,7 @@ func newRepositoryCommand(setResult func(app.OperationResult)) *cobra.Command { o.Private, _ = c.Flags().GetBool("private") o.Shared, _ = c.Flags().GetBool("shared") o.MetadataRemote, _ = c.Flags().GetString("metadata-remote") + o.Harnesses = harnessesOptions(c) setResult(repositoryInitRunner(c.Context(), deps, o)) }, // metadata-write (the parentless metadata root, published create-only to @@ -113,6 +157,7 @@ func newRepositoryCommand(setResult func(app.OperationResult)) *cobra.Command { "keep docket's metadata on a docket branch pushed to origin with a .docket/ worktree (overrides the visibility setting)") initCmd.Flags().String("metadata-remote", "", "with --private: push the dckt branch to this git `url` instead of the default bare repository") + initCmd.Flags().StringSlice("harnesses", nil, harnessesUsage) checkCmd := repositorySubcommand("check", "Report repository health with machine-readable findings (read-only)", func(c *cobra.Command, deps app.SetupDeps) { @@ -150,6 +195,17 @@ func newRepositoryCommand(setResult func(app.OperationResult)) *cobra.Command { configureTestsCmd.Flags().String("command", "", "suite `command` to set as both build.test_command and finalize.test_command with both gates local (skips discovery)") + configureHarnessesCmd := repositorySubcommand("configure-harnesses", + "Choose which coding agents get docket's instructions in this repository, and refresh them", + func(c *cobra.Command, deps app.SetupDeps) { + setResult(repositoryConfigureHarnessesRunner(c.Context(), deps, app.ConfigureHarnessesOptions{Harnesses: harnessesOptions(c)})) + }, + // local-write: writes agent_harnesses to the repository config (the + // unstaged .docket.yml, or the private config) and refreshes the agent + // instruction surfaces; never commits, never stages, never pushes. + EffectLocalWrite) + configureHarnessesCmd.Flags().StringSlice("harnesses", nil, harnessesUsage) + syncIntegrationCmd := repositorySubcommand("sync-integration", "Fast-forward the primary checkout to the freshly fetched integration tip when it is safe (explicit skips otherwise)", func(c *cobra.Command, deps app.SetupDeps) { @@ -159,7 +215,7 @@ func newRepositoryCommand(setResult func(app.OperationResult)) *cobra.Command { // the primary checkout; it never pushes and changes no planning metadata. EffectLocalWrite) - repositoryCmd.AddCommand(initCmd, checkCmd, migrateCmd, repairCmd, prepareCmd, configureTestsCmd, syncIntegrationCmd) + repositoryCmd.AddCommand(initCmd, checkCmd, migrateCmd, repairCmd, prepareCmd, configureTestsCmd, configureHarnessesCmd, syncIntegrationCmd) return repositoryCmd } @@ -320,8 +376,9 @@ func repositoryReadYes(r io.Reader) bool { // unavailable Git client) is an argument-shaped error returned before the // operation runs. // The capability id is the dotted command path "repository."+name; effects are -// declared by each caller (the repository verbs differ — check reads, prepare -// and configure-tests write locally, init writes metadata + local) rather than +// declared by each caller (the repository verbs differ — check reads, prepare, +// configure-tests, and configure-harnesses write locally, init writes metadata +// + local) rather than // looked up from a name inside the helper. func repositorySubcommand(name, short string, run func(c *cobra.Command, deps app.SetupDeps), effects ...Effect) *cobra.Command { cmd := &cobra.Command{ diff --git a/internal/cli/repository_test.go b/internal/cli/repository_test.go index 6a14b7790..c1c0665b6 100644 --- a/internal/cli/repository_test.go +++ b/internal/cli/repository_test.go @@ -14,7 +14,7 @@ import ( // group and an unknown subcommand both fail rather than silently succeeding. func TestRepositoryCommandsRegistered(t *testing.T) { root := captureTree(t) - for _, sub := range []string{"init", "check", "migrate", "prepare", "configure-tests", "repair"} { + for _, sub := range []string{"init", "check", "migrate", "prepare", "configure-tests", "configure-harnesses", "repair"} { cmd, _, err := root.Find([]string{"repository", sub}) if err != nil || cmd == nil || cmd.Name() != sub { t.Fatalf("repository %s not registered: cmd=%v err=%v", sub, cmd, err) @@ -624,3 +624,106 @@ func TestRepositoryConfigureTestsHelpNamesCommandFlag(t *testing.T) { t.Errorf("configure-tests --help must document --command:\n%s", out) } } + +// TestRepositoryHarnessesFlagAndChooserFlow proves configure-harnesses reads +// --harnesses by Changed (repeated and comma-separated values accumulate, an +// explicit empty value still arrives as Set) and supplies the picker only when +// no flag was given, --json is off, and the session is interactive. +func TestRepositoryHarnessesFlagAndChooserFlow(t *testing.T) { + tmp := testsupport.TempDir(t) + var got app.ConfigureHarnessesOptions + oldRunner := repositoryConfigureHarnessesRunner + repositoryConfigureHarnessesRunner = func(ctx context.Context, d app.SetupDeps, o app.ConfigureHarnessesOptions) app.OperationResult { + got = o + return fakeSyncResult{Envelope: app.NewEnvelope("repository.configure-harnesses", app.ResultNoOp)} + } + interactive := false + oldInteractive := repositoryHarnessesInteractive + repositoryHarnessesInteractive = func() bool { return interactive } + defer func() { + repositoryConfigureHarnessesRunner = oldRunner + repositoryHarnessesInteractive = oldInteractive + }() + + run := func(t *testing.T, args ...string) app.HarnessesOptions { + t.Helper() + got = app.ConfigureHarnessesOptions{} + argv := append([]string{"repository", "configure-harnesses", "--repo-dir", tmp}, args...) + if _, _, code := runCLI(t, argv...); code != 0 { + t.Fatalf("exit = %d, want 0 (the stubbed runner decides)", code) + } + return got.Harnesses + } + + t.Run("repeated and comma flags accumulate", func(t *testing.T) { + interactive = false + h := run(t, "--harnesses", "claude,cursor", "--harnesses", "codex") + if !h.Set || strings.Join(h.Tokens, "|") != "claude|cursor|codex" || h.Chooser != nil { + t.Fatalf("got Set=%v Tokens=%v chooser=%v", h.Set, h.Tokens, h.Chooser != nil) + } + }) + t.Run("explicit empty value is set", func(t *testing.T) { + interactive = false + h := run(t, "--harnesses", "") + if !h.Set || h.Tokens == nil { + t.Fatalf("an explicit empty --harnesses must arrive Set with non-nil tokens, got Set=%v Tokens=%#v", h.Set, h.Tokens) + } + }) + t.Run("non-interactive without flag", func(t *testing.T) { + interactive = false + h := run(t) + if h.Set || h.Chooser != nil { + t.Fatalf("got Set=%v chooser=%v, want unset and no chooser", h.Set, h.Chooser != nil) + } + }) + t.Run("interactive supplies chooser", func(t *testing.T) { + interactive = true + if h := run(t); h.Set || h.Chooser == nil { + t.Fatalf("got Set=%v chooser=%v, want a chooser", h.Set, h.Chooser != nil) + } + }) + t.Run("interactive with json has no chooser", func(t *testing.T) { + interactive = true + if h := run(t, "--json"); h.Chooser != nil { + t.Fatal("--json must never show the picker") + } + }) + t.Run("interactive with flag has no chooser", func(t *testing.T) { + interactive = true + if h := run(t, "--harnesses", "none"); !h.Set || h.Chooser != nil { + t.Fatalf("got Set=%v chooser=%v, want set and no chooser", h.Set, h.Chooser != nil) + } + }) +} + +// TestRepositoryInitHarnessesFlagFlows proves init passes --harnesses through to +// the app options. +func TestRepositoryInitHarnessesFlagFlows(t *testing.T) { + tmp := testsupport.TempDir(t) + var got app.InitOptions + old := repositoryInitRunner + repositoryInitRunner = func(ctx context.Context, d app.SetupDeps, o app.InitOptions) app.OperationResult { + got = o + return fakeInitResult{Envelope: app.NewEnvelope("repository.init", app.ResultApplied)} + } + defer func() { repositoryInitRunner = old }() + + if _, _, code := runCLI(t, "repository", "init", "--repo-dir", tmp, "--harnesses", "claude", "--harnesses", "cursor"); code != 0 { + t.Fatalf("exit = %d, want 0", code) + } + if !got.Harnesses.Set || strings.Join(got.Harnesses.Tokens, "|") != "claude|cursor" { + t.Fatalf("Harnesses = %+v, want Set with [claude cursor]", got.Harnesses) + } +} + +func TestRepositoryHarnessesHelp(t *testing.T) { + for _, sub := range []string{"init", "configure-harnesses"} { + out, _, _ := runCLI(t, "repository", sub, "--help") + if !strings.Contains(out, "--harnesses") || !strings.Contains(out, "none") { + t.Errorf("%s --help must document --harnesses and none:\n%s", sub, out) + } + } + if !assetIndependent["repository configure-harnesses"] { + t.Error(`assetIndependent["repository configure-harnesses"] must be true`) + } +} From 6f9c92cae0262a8180a8e320425d381a3bb0dac7 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 21:58:54 -0400 Subject: [PATCH 08/12] feat(repository): warn harnesses-unset in repository check until agents are chosen --- docs/release/upgrading-from-bash.md | 18 ++++- .../repocheck_harnesses_integration_test.go | 71 +++++++++++++++++++ internal/app/repocheck_integration_test.go | 5 +- internal/app/repository_check.go | 9 ++- .../repository_private_integration_test.go | 7 +- internal/bashupgrade/registry_test.go | 1 + internal/reposetup/health.go | 18 +++++ internal/reposetup/health_test.go | 27 +++++++ 8 files changed, 148 insertions(+), 8 deletions(-) create mode 100644 internal/app/repocheck_harnesses_integration_test.go diff --git a/docs/release/upgrading-from-bash.md b/docs/release/upgrading-from-bash.md index 5b887afed..94c73d65d 100644 --- a/docs/release/upgrading-from-bash.md +++ b/docs/release/upgrading-from-bash.md @@ -124,6 +124,7 @@ On a Bash repository it exits with an error and lists some of these findings: | `committed-ignore-invalid` | The docket block in `.gitignore` is the Bash version. | Replace the block, below. | | `legacy-config-key-present` | `.docket.yml` still sets `metadata_branch`. | Remove the setting, below. | | `test-config-missing` | No test command is set up for docket's build checks. | `docket repository configure-tests`, below. | +| `harnesses-unset` | No coding agents are chosen for this repository, so docket writes no instructions for them. | `docket repository configure-harnesses`, below. | | `board-stale` | `BOARD.md` on the `docket` branch is in the Bash format. | `docket repository repair`, below. | | `artifact-links-stale` | A change record's links block is in the Bash format. | `docket repository repair`, below. | @@ -166,17 +167,30 @@ git push ``` Now set up the test command. `docket repository configure-tests` writes the test settings into -`.docket.yml`. Review them, then commit and push. +`.docket.yml`. Review them. ```sh docket repository configure-tests ``` +Next, choose the coding agents docket writes instructions for. On a terminal, +`docket repository configure-harnesses` shows a checklist. The command below records "none yet", +because Bash docket's `CLAUDE.md` block would get in the way. After you remove that block (the last +step), run `docket repository configure-harnesses --harnesses claude` (or your agents) and commit +the result. + + +```sh +docket repository configure-harnesses --harnesses none +``` + +Commit and push both settings in `.docket.yml`, the test command and the agents: + ```sh git add .docket.yml -git commit -m "Configure docket's test command" +git commit -m "Configure docket's test command and agents" git push ``` diff --git a/internal/app/repocheck_harnesses_integration_test.go b/internal/app/repocheck_harnesses_integration_test.go new file mode 100644 index 000000000..7e7402821 --- /dev/null +++ b/internal/app/repocheck_harnesses_integration_test.go @@ -0,0 +1,71 @@ +//go:build integration + +package app + +import ( + "testing" + + "github.com/danielhanold/docket/internal/reposetup" +) + +// This shard file carries `docket repository check`'s harnesses-unset warning +// (prefix TestIntegrationRepoCheck, matched by +// tests/test_go_integration_app_repocheck.sh): while no repository-level layer +// declares agent_harnesses the check warns, never changing the classified +// state, and a recorded choice (`[]` included) silences it. + +// assertHarnessesUnset checks the classified state, whether the +// harnesses-unset finding is present, and the exit code. +func assertHarnessesUnset(t *testing.T, label string, chk RepositoryCheckResult, wantFinding bool, wantExit int) { + t.Helper() + if chk.RepositoryState != string(reposetup.StateHealthy) { + t.Fatalf("%s: state = %q, want healthy:\n%s", label, chk.RepositoryState, chk.HumanText()) + } + if got := len(findingsWithCode(chk.Findings, reposetup.HarnessesUnsetCode)) > 0; got != wantFinding { + t.Errorf("%s: harnesses-unset present = %v, want %v:\n%s", label, got, wantFinding, chk.HumanText()) + } + if code := chk.CheckExitCode(); code != wantExit { + t.Errorf("%s: exit = %d, want %d:\n%s", label, code, wantExit, chk.HumanText()) + } +} + +func TestIntegrationRepoCheckHarnessesUnsetShared(t *testing.T) { + r := newInitRepo(t, "integration_branch: main\n", nil) + if res := r.runInitWith(t, InitOptions{}); res.Result != ResultApplied { + t.Fatalf("init = %q (%s), want applied", res.Result, res.HumanText()) + } + r.commitAndPushMain(t, "commit managed .gitignore and generated test policy", ".gitignore", ".docket.yml") + assertHarnessesUnset(t, "unset", r.runCheck(t), true, 1) + + none := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag(reposetup.HarnessesNone)}) + if none.Result != ResultApplied { + t.Fatalf("configure-harnesses none = %q (%s), want applied", none.Result, none.HumanText()) + } + r.commitAndPushMain(t, "record no agents", ".docket.yml") + assertHarnessesUnset(t, "none", r.runCheck(t), false, 0) + + claude := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("claude")}) + if claude.Result != ResultApplied { + t.Fatalf("configure-harnesses claude = %q (%s), want applied", claude.Result, claude.HumanText()) + } + r.commitAndPushMain(t, "choose claude", claude.PendingPaths...) + assertHarnessesUnset(t, "claude", r.runCheck(t), false, 0) +} + +func TestIntegrationRepoCheckHarnessesUnsetPrivate(t *testing.T) { + r, _ := newPrivateInitRepo(t, nil) + if res := r.runInitWith(t, InitOptions{Private: true}); res.Result != ResultApplied { + t.Fatalf("init = %q (%s), want applied", res.Result, res.HumanText()) + } + cmd := "true" + if ct := r.runConfigureTestsWith(t, ConfigureTestsOptions{Command: &cmd}); ct.Result != ResultApplied { + t.Fatalf("configure-tests = %q (%s), want applied", ct.Result, ct.HumanText()) + } + assertHarnessesUnset(t, "unset", checkIn(t, r.invocation), true, 1) + + none := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag(reposetup.HarnessesNone)}) + if none.Result != ResultApplied { + t.Fatalf("configure-harnesses none = %q (%s), want applied", none.Result, none.HumanText()) + } + assertHarnessesUnset(t, "none", checkIn(t, r.invocation), false, 0) +} diff --git a/internal/app/repocheck_integration_test.go b/internal/app/repocheck_integration_test.go index 9d731aee9..df41179d0 100644 --- a/internal/app/repocheck_integration_test.go +++ b/internal/app/repocheck_integration_test.go @@ -26,8 +26,9 @@ import ( // healthySetupYML is docket-mode config WITHOUT the legacy metadata_branch key: // the metadata branch resolves to its default (docket). A healthy repository -// carries no metadata_branch key. -const healthySetupYML = "integration_branch: main\n" +// carries no metadata_branch key. The healthy baseline records "no agents" +// (agent_harnesses: []), so check raises no harnesses-unset warning. +const healthySetupYML = "integration_branch: main\nagent_harnesses: []\n" // runCheck runs RunRepositoryCheck against the invocation clone with a fresh // isolated client. diff --git a/internal/app/repository_check.go b/internal/app/repository_check.go index a94512be0..2076d340c 100644 --- a/internal/app/repository_check.go +++ b/internal/app/repository_check.go @@ -90,7 +90,7 @@ func RunRepositoryCheck(ctx context.Context, d SetupDeps) RepositoryCheckResult // its own errors to the safe Unknown value, never to a false absence. var fm []reposetup.RepairFinding var corpusExtra []reposetup.Finding - var testConfig *reposetup.Finding + var testConfig, harnessesUnset *reposetup.Finding if facts.RemoteMetadata.Presence == reposetup.PresencePresent { augmentCheckFacts(ctx, d.Git, &facts, sc) corpus, rerr := readCheckCorpus(ctx, d.Git, sc) @@ -102,6 +102,10 @@ func RunRepositoryCheck(ctx context.Context, d SetupDeps) RepositoryCheckResult // exactly what the resolver read (a private repository's // .git/dckt/config.yml, through repoConfigTarget). testConfig = reposetup.TestConfigFinding(sc.cfg, readRepoConfigBytes(sc)) + // An unchosen agent_harnesses is a setup gap reported like the test + // policy: a warning with the configure-harnesses remedy, never a state + // change. + harnessesUnset = reposetup.HarnessesUnsetFinding(facts) } cls := reposetup.Classify(facts) @@ -110,6 +114,9 @@ func RunRepositoryCheck(ctx context.Context, d SetupDeps) RepositoryCheckResult if testConfig != nil { findings = append(findings, *testConfig) } + if harnessesUnset != nil { + findings = append(findings, *harnessesUnset) + } // The visibility findings explain; they never change the classified state. // A mismatch is meaningful only once the repository is set up (a fresh // repository's setting is what init will use). diff --git a/internal/app/repository_private_integration_test.go b/internal/app/repository_private_integration_test.go index cae8fcc21..98246df7c 100644 --- a/internal/app/repository_private_integration_test.go +++ b/internal/app/repository_private_integration_test.go @@ -532,12 +532,13 @@ func TestIntegrationRepoSetupPrivateMigrateRefuses(t *testing.T) { } } -// initPrivateHealthy inits a private repository and sets its test policy with -// configure-tests --command true, the path to a healthy private repository. +// initPrivateHealthy inits a private repository recording no agents +// (--harnesses none) and sets its test policy with configure-tests --command +// true, the path to a healthy private repository. func initPrivateHealthy(t *testing.T) (*initRepo, string) { t.Helper() r, data := newPrivateInitRepo(t, nil) - if res := r.runInitWith(t, InitOptions{Private: true}); res.Result != ResultApplied { + if res := r.runInitWith(t, InitOptions{Private: true, Harnesses: harnessFlag("none")}); res.Result != ResultApplied { t.Fatalf("init = %q (%s), want applied", res.Result, res.HumanText()) } cmd := "true" diff --git a/internal/bashupgrade/registry_test.go b/internal/bashupgrade/registry_test.go index 44f31b1cc..4aab7e125 100644 --- a/internal/bashupgrade/registry_test.go +++ b/internal/bashupgrade/registry_test.go @@ -69,6 +69,7 @@ var stepRegistry = map[string]stepAction{ "config-cleanup": mirrorConfigCleanup, "commit-fixes": runCommitFixes, "configure-tests": runPlain, + "configure-harnesses": runPlain, "commit-config": runCommit, "repair-preview": observeRepairPreview, "repair-apply": runRepairApply, diff --git a/internal/reposetup/health.go b/internal/reposetup/health.go index a1b468996..a13c87c27 100644 --- a/internal/reposetup/health.go +++ b/internal/reposetup/health.go @@ -279,6 +279,24 @@ func TestConfigFinding(cfg config.Effective, committedYML []byte) *Finding { } } +// HarnessesUnsetCode: no repository-level layer declares agent_harnesses, so +// docket writes no dispatch instructions here. `[]` is a recorded decision and +// silences it. +const HarnessesUnsetCode = "harnesses-unset" + +// HarnessesUnsetFinding explains; it never changes the classified state. +func HarnessesUnsetFinding(f Facts) *Finding { + if f.SurfacesAuthorized { + return nil + } + return &Finding{ + Code: HarnessesUnsetCode, + Severity: SeverityWarning, + Message: "docket writes no agent dispatch instructions in this repository because agent_harnesses is not set in its repository config.", + Remedy: "Run `" + ConfigureHarnessesCommand + "` to choose the coding agents, or `" + ConfigureHarnessesCommand + " --harnesses " + HarnessesNone + "` to record that none are used.", + } +} + // localGateNeedsCommand reports whether a gate owner is a local gate with no // resolved command — the configuration gap a setup-time edit must close. func localGateNeedsCommand(gate, command string) bool { diff --git a/internal/reposetup/health_test.go b/internal/reposetup/health_test.go index d9cb74d98..a76ec0c26 100644 --- a/internal/reposetup/health_test.go +++ b/internal/reposetup/health_test.go @@ -1190,3 +1190,30 @@ func TestAttachmentRemedyFollowsLayout(t *testing.T) { } } } + +// TestHarnessesUnsetFinding: with no repository-level agent_harnesses the +// finding warns and points at configure-harnesses (including the explicit +// "none" spelling); a declared value, `[]` included, silences it. +func TestHarnessesUnsetFinding(t *testing.T) { + got := HarnessesUnsetFinding(Facts{}) + if got == nil { + t.Fatal("HarnessesUnsetFinding(Facts{}) = nil, want a finding") + } + if HarnessesUnsetCode != "harnesses-unset" || got.Code != HarnessesUnsetCode { + t.Errorf("Code = %q, want %q", got.Code, "harnesses-unset") + } + if got.Severity != SeverityWarning { + t.Errorf("Severity = %q, want %q", got.Severity, SeverityWarning) + } + if !strings.Contains(got.Message, "agent_harnesses") { + t.Errorf("Message = %q, want it to name agent_harnesses", got.Message) + } + for _, want := range []string{ConfigureHarnessesCommand, "--harnesses none"} { + if !strings.Contains(got.Remedy, want) { + t.Errorf("Remedy = %q, want it to contain %q", got.Remedy, want) + } + } + if got := HarnessesUnsetFinding(Facts{SurfacesAuthorized: true}); got != nil { + t.Errorf("HarnessesUnsetFinding(SurfacesAuthorized) = %+v, want nil", got) + } +} From 9f6de8d46ba344652772aa9814cc0a04449991a0 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 21:59:53 -0400 Subject: [PATCH 09/12] docs: describe choosing agent harnesses with init and configure-harnesses --- .docket.example.yml | 6 +++++- docs/guide/where-the-metadata-lives.md | 5 ++++- docs/reference/config-keys.md | 4 +++- docs/reference/glossary.md | 6 ++++-- internal/assets/embedded/manifest.json | 10 +++++----- internal/assets/embedded/tree/.docket.example.yml | 6 +++++- .../skills/docket-convention/references/agent-layer.md | 4 ++++ internal/repoguard/budgets_test.go | 2 +- skills/docket-convention/references/agent-layer.md | 4 ++++ 9 files changed, 35 insertions(+), 12 deletions(-) diff --git a/.docket.example.yml b/.docket.example.yml index 61fe04df7..b5159bf8e 100644 --- a/.docket.example.yml +++ b/.docket.example.yml @@ -243,7 +243,11 @@ change_types: [chore, docs, feat, fix, refactor, perf] # no default: while it is absent, install touches no repository surface. Listing harnesses # (claude, codex, cursor, opencode) installs the managed block in CLAUDE.md / AGENTS.md and, for # cursor, .cursor/rules/docket-dispatch.mdc. Only .docket.yml or .docket.local.yml can opt a -# repository in; the installer ignores a value in the global config. +# repository in; the installer ignores a value in the global config. `docket repository init` asks +# for it (a checklist on a terminal, or `--harnesses claude,cursor` / `--harnesses none`), +# `docket repository configure-harnesses` changes it later and refreshes the instructions in the +# same run, and `docket repository check` warns `harnesses-unset` while it is not set. +# `agent_harnesses: []` records that no agents are used. # scope: any layer # agent_harnesses: [claude] diff --git a/docs/guide/where-the-metadata-lives.md b/docs/guide/where-the-metadata-lives.md index 021cd1117..4824a6cfb 100644 --- a/docs/guide/where-the-metadata-lives.md +++ b/docs/guide/where-the-metadata-lives.md @@ -109,7 +109,10 @@ All of these operate on the repository in your current directory (or the one `-- `docket` branch, pushes it, and attaches the `.docket/` worktree. It also writes the managed `.gitignore` block (which ignores `.docket/`, `.worktrees/`, `.docket.local.yml`, and docket's other machine-local files) and a starting `.docket.yml` test policy, and leaves those edits - unstaged for you to review and commit. + unstaged for you to review and commit. It also asks which coding agents get docket's instructions + (`--harnesses` answers up front). +- **`docket repository configure-harnesses`** — changes that choice any time, without a clean + checkout, and leaves the `.docket.yml` edit unstaged. - **`docket repository migrate`** — only for a repository on the old single-branch layout, where the planning records still live on the integration branch. It prints its plan and asks for confirmation before changing anything (pass `--yes` to authorize it without the prompt). It then diff --git a/docs/reference/config-keys.md b/docs/reference/config-keys.md index 1039bc3a5..c43765b00 100644 --- a/docs/reference/config-keys.md +++ b/docs/reference/config-keys.md @@ -64,7 +64,9 @@ Each key has one scope: managed block in `CLAUDE.md` / `AGENTS.md` and, for Cursor, `.cursor/rules/docket-dispatch.mdc`. It takes `claude`, `codex`, `cursor`, and `opencode`. While it is absent, `docket install` touches no repository surface. Only `.docket.yml` or `.docket.local.yml` can opt a repository in; the -installer ignores a value in the global config. +installer ignores a value in the global config. `docket repository init` asks for it (or takes +`--harnesses |none`), `docket repository configure-harnesses` changes it later and refreshes +the surfaces in the same run, and `docket repository check` warns `harnesses-unset` while it is unset. **`agents`** pins apply per agent and per field; anything you leave out keeps its built-in value. The built-in table is compiled into the binary, and `agents/harness-defaults.yml` ships the same diff --git a/docs/reference/glossary.md b/docs/reference/glossary.md index 4611ec869..6d22b3d26 100644 --- a/docs/reference/glossary.md +++ b/docs/reference/glossary.md @@ -47,9 +47,10 @@ or from `--command ""` when discovery cannot find the suite. ```sh cd -docket repository init # a repository that has never used docket +docket repository init # a repository that has never used docket (--harnesses picks the agents up front) docket repository migrate # a legacy single-branch repository (human-typed) docket repository configure-tests # set the build and finalize test commands +docket repository configure-harnesses # choose which coding agents get docket's instructions docket repository configure-tests --command "sh ./test.sh" # set both gates to local with this command ``` @@ -1889,7 +1890,8 @@ exactly those harnesses), or `[]` (retire every docket-owned repository surface) **Used for:** opting a repository in to dispatch surfaces. Only `.docket.yml` or `.docket.local.yml` can opt a repository in; the installer ignores a global value, and an unknown token is an error. -Re-run the install after changing it. +`docket repository init` asks for it; `docket repository configure-harnesses` changes it and +refreshes the surfaces; `docket repository check` warns `harnesses-unset` while it is unset. ```yaml # .docket.yml (whole team) or .docket.local.yml (this clone) diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index 988979af5..5ea2d31d0 100644 --- a/internal/assets/embedded/manifest.json +++ b/internal/assets/embedded/manifest.json @@ -1,14 +1,14 @@ { "format_version": 1, "asset_protocol": 1, - "asset_set_id": "sha256:c7cbcdc5291b858312ab91b9c886add0dcaf40f6ab2415d09fe040cf7d3a6b80", + "asset_set_id": "sha256:ebf6576613d6042482da69471a7ebd3ff22bce38c13bccfee634bd5ee8ae043d", "entries": [ { "path": ".docket.example.yml", "role": "config-schema", "mode": 420, - "size": 20147, - "sha256": "7f19aeb4b5f1fbe3d4fd6c004925373e1350c7516556c8250190fdecf63e0c05" + "size": 20509, + "sha256": "bbe397f9b29e6971e63578b5e0affb4e75ae63976ec601f43cc016dea0b7085e" }, { "path": "agents/docket-adr.md", @@ -329,8 +329,8 @@ "path": "skills/docket-convention/references/agent-layer.md", "role": "skill", "mode": 420, - "size": 11302, - "sha256": "1584ba36c700ddf4cb2ea82fdcbc79e8d1325692ee4ed99ecdb68528d82d9e52" + "size": 11621, + "sha256": "ad7e44f72a6e26739caff068b61bf5600a38b3324d4f7fa44771054ca5a7addc" }, { "path": "skills/docket-convention/references/close-out.md", diff --git a/internal/assets/embedded/tree/.docket.example.yml b/internal/assets/embedded/tree/.docket.example.yml index 61fe04df7..b5159bf8e 100644 --- a/internal/assets/embedded/tree/.docket.example.yml +++ b/internal/assets/embedded/tree/.docket.example.yml @@ -243,7 +243,11 @@ change_types: [chore, docs, feat, fix, refactor, perf] # no default: while it is absent, install touches no repository surface. Listing harnesses # (claude, codex, cursor, opencode) installs the managed block in CLAUDE.md / AGENTS.md and, for # cursor, .cursor/rules/docket-dispatch.mdc. Only .docket.yml or .docket.local.yml can opt a -# repository in; the installer ignores a value in the global config. +# repository in; the installer ignores a value in the global config. `docket repository init` asks +# for it (a checklist on a terminal, or `--harnesses claude,cursor` / `--harnesses none`), +# `docket repository configure-harnesses` changes it later and refreshes the instructions in the +# same run, and `docket repository check` warns `harnesses-unset` while it is not set. +# `agent_harnesses: []` records that no agents are used. # scope: any layer # agent_harnesses: [claude] diff --git a/internal/assets/embedded/tree/skills/docket-convention/references/agent-layer.md b/internal/assets/embedded/tree/skills/docket-convention/references/agent-layer.md index 37c1409dd..76c958a70 100644 --- a/internal/assets/embedded/tree/skills/docket-convention/references/agent-layer.md +++ b/internal/assets/embedded/tree/skills/docket-convention/references/agent-layer.md @@ -82,6 +82,10 @@ blocks that route a requested docket workflow to its wrapper. Only a value decla - **an explicit empty list** (`agent_harnesses: []`) — the install retires every docket-owned repository surface the repository had. +`repository.init` asks for the value on a terminal or takes `--harnesses |none`; the +`repository.configure-harnesses` operation changes it later and refreshes the dispatch blocks in the same run, with +no clean checkout needed; `repository.check` reports `harnesses-unset` while no repository-level value exists. + `--repo-dir ` targets another repository. `agent_harnesses` decides which harnesses get dispatch blocks; it never decides which pins a wrapper carries. diff --git a/internal/repoguard/budgets_test.go b/internal/repoguard/budgets_test.go index ef168488f..00962a337 100644 --- a/internal/repoguard/budgets_test.go +++ b/internal/repoguard/budgets_test.go @@ -199,7 +199,7 @@ var skillBudgets = []skillBudget{ {"docket-convention/SKILL.md", 361, 7263}, // 0532: the private-repository writing rule and visibility hand-off (359/7160 -> 361/7263); 0531 review: skills say "the metadata branch", naming `docket` once for shared mode (359/7155 -> 359/7160); 0531: the metadata worktree and remote come from the repository.prepare context (357/7062 -> 359/7155); 0530: plan, results, and build evidence live on the docket branch — the plan-writer's one metadata operation, the attach operations as file-backlink writers, relative same-branch links, and the new ## Build evidence record section (357/7026 -> 357/7062); 0529 review: the backlink block names its PR-body writers beside artifact.backlink, one shared renderer (357/7004 -> 357/7026); 0515: the repair sign-off is retired; a green repair merges (357/7007 -> 357/7004); 0502 rebase onto 0510: kept the backlog-match follow-up clause in the results-artifact paragraph (357/7004 -> 357/7007); 0502 review: the review-tier dispatch is the controller's own (357/6995 -> 357/7004); 0502: configuration read paths corrected — operations read .docket.yml from origin's default-branch tip, the repository setup operations from the primary worktree (357/6959 -> 357/6995); 0502: "loop" means /loop only (357/7036 -> 357/7037); 0502: one metadata layout — the docket branch and the .docket/ worktree (357/7035 -> 357/7036); 0502: agent layer names user-level wrappers and global-only pins (357/7024 -> 357/7035); 0502: typed operations replace the Bash-era script names; ## Run halted facts corrected (359/7092 -> 357/7024); 0502: terminal publish, the learnings cap, and the repository auto_groom default are deleted (362/7256 -> 359/7092); 0502: the auto-capture export paragraph is deleted (370/7334 -> 362/7256); 0502: the configuration contract rewritten from the schema (371/7488 -> 370/7334); 0502: fixed role skills; halt posture (372/7660 -> 371/7488); 0502: record blocks mirror the renderers (367/7619 -> 372/7660); 0502: the dummy-mode shared definition is deleted (390/7848 -> 367/7619); 0410: +required-results lifecycle prose; 0399: +schema request/result contract prose; 0388: +sync-integration prose (see note above); 0502: drop change citations and Go v1 narration (357/7037 -> 357/6958); 0502: the harness adapter is named without a docket source path (357/6958 -> 357/6959) // 0154: docket-convention/github-board-mirror.md removed — the GitHub mirror is // retired (unsupported, mutation-blocking); its budget row is deleted with it. - {"docket-convention/references/agent-layer.md", 164, 1553}, // 0535: a private repository's instructions file and its user-level triggers (148/1419 -> 164/1553); 0502: rewritten around user-level wrappers and global-only pins (202/2346 -> 148/1419); 0502: the legacy agents-file migration clause is deleted (202/2349 -> 202/2346) + {"docket-convention/references/agent-layer.md", 168, 1593}, // 0538: init and repository.configure-harnesses set agent_harnesses; repository.check reports harnesses-unset (164/1553 -> 168/1593); 0535: a private repository's instructions file and its user-level triggers (148/1419 -> 164/1553); 0502: rewritten around user-level wrappers and global-only pins (202/2346 -> 148/1419); 0502: the legacy agents-file migration clause is deleted (202/2349 -> 202/2346) {"docket-convention/references/close-out.md", 141, 1347}, // 0531: the metadata worktree and remote come from the repository.prepare context (141/1349 -> 141/1347); 0502: "loop" means /loop only (144/1375 -> 144/1374); 0502: one metadata layout — the docket branch and the .docket/ worktree (144/1376 -> 144/1375); 0502: the scripts/.md contract pointer becomes the schema operation (144/1374 -> 144/1376); 0502: the terminal-publication step is deleted (240/2150 -> 144/1374); 0474: reference renamed (ADR-0129 row 59a); ceilings unchanged; 0502: drop change citations and Go v1 narration (144/1374 -> 144/1356); 0502: the docket-source test citations are deleted (144/1356 -> 141/1349) {"docket-convention/references/learnings.md", 69, 487}, // 0535: promotion lands in the private instructions file in a private repository (66/457 -> 69/487); 0502: one metadata layout — the docket branch and the .docket/ worktree (66/455 -> 66/457); 0502: learning.record/learning.update are the writers; the capacity section is deleted (84/580 -> 66/455) {"docket-convention/references/stacked-changes.md", 214, 2149}, // 0502: typed operations and binary-emitted findings replace the Bash-era names (213/2125 -> 214/2149); 0502: the deferred descendant-publication sentence is deleted (215/2140 -> 213/2125); 0327: +carry-preservation contract prose (see note above) diff --git a/skills/docket-convention/references/agent-layer.md b/skills/docket-convention/references/agent-layer.md index 37c1409dd..76c958a70 100644 --- a/skills/docket-convention/references/agent-layer.md +++ b/skills/docket-convention/references/agent-layer.md @@ -82,6 +82,10 @@ blocks that route a requested docket workflow to its wrapper. Only a value decla - **an explicit empty list** (`agent_harnesses: []`) — the install retires every docket-owned repository surface the repository had. +`repository.init` asks for the value on a terminal or takes `--harnesses |none`; the +`repository.configure-harnesses` operation changes it later and refreshes the dispatch blocks in the same run, with +no clean checkout needed; `repository.check` reports `harnesses-unset` while no repository-level value exists. + `--repo-dir ` targets another repository. `agent_harnesses` decides which harnesses get dispatch blocks; it never decides which pins a wrapper carries. From f099de00fde75b1742dff41eaed20c730048d151 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 22:15:59 -0400 Subject: [PATCH 10/12] fix(repository): list init's pending surfaces from git status, not planned targets Review finding: init told the user to commit the gitignored Cursor rule and unchanged surface files because it echoed every planned surface target, so 'git add' on the pending list failed. Init now reads the surfaces pending review from git status, as configure-harnesses does. --- .../reposetup_harnesses_integration_test.go | 32 +++++++++++++------ internal/app/repository_harnesses.go | 16 +++++----- internal/app/repository_init.go | 9 +++++- 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/internal/app/reposetup_harnesses_integration_test.go b/internal/app/reposetup_harnesses_integration_test.go index 3c8038b15..747cbcaee 100644 --- a/internal/app/reposetup_harnesses_integration_test.go +++ b/internal/app/reposetup_harnesses_integration_test.go @@ -72,6 +72,28 @@ func TestIntegrationRepoSetupInitHarnessesFlagShared(t *testing.T) { } } +// Init's pending list is what the human commits: the Cursor rule the managed +// .gitignore block ignores is never on it, and `git add` takes the whole list. +func TestIntegrationRepoSetupInitPendingOmitsIgnoredCursorRule(t *testing.T) { + r := newInitRepo(t, defaultSetupYML, nil) + res := r.runInitWith(t, InitOptions{Harnesses: harnessFlag("claude", "cursor")}) + if res.Result != ResultApplied { + t.Fatalf("Result = %q (%s), want applied", res.Result, res.HumanText()) + } + if _, err := os.Stat(filepath.Join(r.invocation, ".cursor", "rules", "docket-dispatch.mdc")); err != nil { + t.Fatalf("the cursor rule was not installed: %v", err) + } + if contains(res.PendingPaths, ".cursor/rules/docket-dispatch.mdc") { + t.Errorf("PendingPaths = %v, want the gitignored cursor rule left off", res.PendingPaths) + } + if strings.Contains(res.HumanText(), "docket-dispatch.mdc") { + t.Errorf("human text %q names the gitignored cursor rule", res.HumanText()) + } + if out, err := tryGit(r.invocation, append([]string{"add", "--"}, res.PendingPaths...)...); err != nil { + t.Errorf("git add of the pending paths %v failed: %v (%s)", res.PendingPaths, err, out) + } +} + func TestIntegrationRepoSetupPrivateInitHarnessesFlag(t *testing.T) { r, data := newPrivateInitRepo(t, nil) res := r.runInitWith(t, InitOptions{Private: true, Harnesses: harnessFlag("claude")}) @@ -282,15 +304,7 @@ func newHarnessRepo(t *testing.T, tokens ...string) *initRepo { if res.Result != ResultApplied { t.Fatalf("init = %q (%s), want applied", res.Result, res.HumanText()) } - // Init also reports the Cursor rule, which the managed .gitignore block - // ignores; commit only the paths git will track. - var tracked []string - for _, p := range res.PendingPaths { - if _, err := tryGit(r.invocation, "check-ignore", "-q", "--", p); err != nil { - tracked = append(tracked, p) - } - } - r.commitAndPushMain(t, "commit init's pending edits", tracked...) + r.commitAndPushMain(t, "commit init's pending edits", res.PendingPaths...) return r } diff --git a/internal/app/repository_harnesses.go b/internal/app/repository_harnesses.go index 04015ce90..41c3737c2 100644 --- a/internal/app/repository_harnesses.go +++ b/internal/app/repository_harnesses.go @@ -176,14 +176,14 @@ func detectHarnesses() []string { } // harnessApplied reports what applyHarnessChoice changed: the config's pending -// path (if any), whether the config was written, the surface paths pending -// review, whether any surface changed, and the warnings to show. +// path (if any), whether the config was written, whether any surface changed, +// and the warnings to show. The surfaces pending review are read from git +// status by the caller, never from the planned targets. type harnessApplied struct { - pendingConfig string - wroteConfig bool - surfacePending []string - wroteSurfaces bool - warnings []string + pendingConfig string + wroteConfig bool + wroteSurfaces bool + warnings []string } // harnessConfigError marks a failure writing or re-reading agent_harnesses in @@ -222,7 +222,7 @@ func applyHarnessChoice(ctx context.Context, git *gitcli.Client, sc setupContext out.pendingConfig, out.wroteConfig = pending, wrote } var err error - if out.surfacePending, out.wroteSurfaces, err = installAuthorizedSurfaces(ctx, git, sc.repo.PrimaryWorktree); err != nil { + if _, out.wroteSurfaces, err = installAuthorizedSurfaces(ctx, git, sc.repo.PrimaryWorktree); err != nil { return out, err } if choice.warning != "" { diff --git a/internal/app/repository_init.go b/internal/app/repository_init.go index 7f6579d6a..6aa6efbd4 100644 --- a/internal/app/repository_init.go +++ b/internal/app/repository_init.go @@ -191,7 +191,14 @@ func RunRepositoryInit(ctx context.Context, d SetupDeps, o InitOptions) Reposito return harnessApplyFailure(OperationRepositoryInit, cls.State, herr) } pending = appendPending(pending, applied.pendingConfig) - pending = appendPending(pending, applied.surfacePending...) + // The surfaces pending review come from git status, as configure-harnesses + // reads them: a planned target that is gitignored (the Cursor rule) or + // unchanged is not something to commit. + surfacePending, serr := workingTreePendingPaths(ctx, d.Git, sc) + if serr != nil { + return repositoryExternalFailure(OperationRepositoryInit, cls.State, "listing the pending review paths", serr) + } + pending = appendPending(pending, surfacePending...) // Test policy: discover the suite from the primary worktree and write the // generated `.docket.yml` edit as another pending, UNSTAGED review path — From c65ad87aab6832e81837df3b6f7fac9fd3b86121 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 22:22:17 -0400 Subject: [PATCH 11/12] fix(repository): align configure-harnesses pending paths and guard with check; edit multi-line flow harness lists Review findings fixed: - Pending review paths: init and configure-harnesses now derive them through the same function repository check uses, so the .gitignore is pending only when its managed block is not yet committed. An unrelated uncommitted .gitignore edit no longer lists .gitignore or raises a healthy repository to needs-review. - configure-harnesses refuses a needs-review repository whose primary checkout is proven off the integration branch, as it already did for an otherwise-healthy one; an unknown branch probe still admits. - The agent_harnesses edit replaces a flow list through its closing ] line, so a multi-line flow list is rewritten instead of refused. --- .../reposetup_harnesses_integration_test.go | 17 ++++++ internal/app/repository_check.go | 34 ++++++----- .../app/repository_configure_harnesses.go | 43 +++++++------ .../repository_configure_harnesses_test.go | 13 +++- internal/app/repository_init.go | 2 +- internal/reposetup/harnessesedit.go | 60 ++++++++++++++++++- internal/reposetup/harnessesedit_test.go | 3 + 7 files changed, 132 insertions(+), 40 deletions(-) diff --git a/internal/app/reposetup_harnesses_integration_test.go b/internal/app/reposetup_harnesses_integration_test.go index 747cbcaee..0a1ea45f1 100644 --- a/internal/app/reposetup_harnesses_integration_test.go +++ b/internal/app/reposetup_harnesses_integration_test.go @@ -459,6 +459,23 @@ func TestIntegrationRepoSetupConfigureHarnessesTwiceInNeedsReview(t *testing.T) } } +// An uncommitted .gitignore edit unrelated to docket is not a pending review +// path once the managed block is committed: configure-harnesses decides the +// .gitignore from the committed-ignore fact, as repository check does, so a +// healthy repository stays healthy. +func TestIntegrationRepoSetupConfigureHarnessesIgnoresUnrelatedGitignoreEdit(t *testing.T) { + r := newHarnessRepo(t, "claude") + gi := filepath.Join(r.invocation, ".gitignore") + writeRepoFile(t, r.invocation, ".gitignore", string(mustReadFile(t, gi))+"unrelated-build-output/\n") + res := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("claude")}) + if res.Result != ResultNoOp { + t.Fatalf("Result = %q (%s), want no-op", res.Result, res.HumanText()) + } + if contains(res.PendingPaths, ".gitignore") || res.RepositoryState != string(reposetup.StateHealthy) { + t.Errorf("PendingPaths = %v state %q, want no .gitignore and healthy", res.PendingPaths, res.RepositoryState) + } +} + func TestIntegrationRepoSetupConfigureHarnessesLocalOverrideWarns(t *testing.T) { r := newHarnessRepo(t, "claude") writeRepoFile(t, r.invocation, ".docket.local.yml", "agent_harnesses: [cursor]\n") diff --git a/internal/app/repository_check.go b/internal/app/repository_check.go index 2076d340c..6e0e72f0a 100644 --- a/internal/app/repository_check.go +++ b/internal/app/repository_check.go @@ -440,6 +440,14 @@ func primaryOnIntegrationPresence(ctx context.Context, git *gitcli.Client, repo // A private repository writes neither .gitignore nor .docket.yml, so neither is // ever one of its pending paths. func pendingReviewPaths(ctx context.Context, git *gitcli.Client, repo gitcli.Repository, mode layout.Mode, committedIgnore reposetup.Presence) []string { + pending, _ := collectPendingReviewPaths(ctx, git, repo, mode, committedIgnore) + return pending +} + +// collectPendingReviewPaths is pendingReviewPaths that also returns the status +// read error; on that error the paths decided so far (the .gitignore) are still +// returned. +func collectPendingReviewPaths(ctx context.Context, git *gitcli.Client, repo gitcli.Repository, mode layout.Mode, committedIgnore reposetup.Presence) ([]string, error) { seen := map[string]bool{} var pending []string private := mode == layout.Private @@ -457,24 +465,22 @@ func pendingReviewPaths(ctx context.Context, git *gitcli.Client, repo gitcli.Rep // The parent-facing dispatch surfaces: pending when changed in the working // tree relative to HEAD. changes, err := git.ChangedPaths(ctx, repo.PrimaryWorktree) - if err == nil { - for _, ch := range changes { - rel := string(ch.Path) - if rel == gitignoreRel { - continue // the .gitignore is decided by the committed-ignore fact above - } - if private && rel == docketYMLRel { - continue // a private repository's config is .git/dckt/config.yml - } - if docketManagedWorktreePaths[rel] && !seen[rel] { - seen[rel] = true - pending = append(pending, rel) - } + for _, ch := range changes { + rel := string(ch.Path) + if rel == gitignoreRel { + continue // the .gitignore is decided by the committed-ignore fact above + } + if private && rel == docketYMLRel { + continue // a private repository's config is .git/dckt/config.yml + } + if docketManagedWorktreePaths[rel] && !seen[rel] { + seen[rel] = true + pending = append(pending, rel) } } sort.Strings(pending) - return pending + return pending, err } // checkCorpus is everything the report-only check reads from the pinned metadata diff --git a/internal/app/repository_configure_harnesses.go b/internal/app/repository_configure_harnesses.go index 07fb84be7..81d187d67 100644 --- a/internal/app/repository_configure_harnesses.go +++ b/internal/app/repository_configure_harnesses.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "sort" "strings" "github.com/danielhanold/docket/internal/gitcli" @@ -78,7 +77,7 @@ func RunRepositoryConfigureHarnesses(ctx context.Context, d SetupDeps, o Configu if herr != nil { return harnessApplyFailure(op, cls.State, herr) } - pending, perr := workingTreePendingPaths(ctx, d.Git, sc) + pending, perr := workingTreePendingPaths(ctx, d.Git, sc, facts.CommittedIgnoreBlock) if perr != nil { return repositoryExternalFailure(op, cls.State, "listing the pending review paths", perr) } @@ -109,8 +108,9 @@ func RunRepositoryConfigureHarnesses(ctx context.Context, d SetupDeps, o Configu return out } -// configureHarnessesGuard classifies once and admits healthy, needs-review, or -// either once init's clean-checkout and at-tip preconditions are set aside; +// configureHarnessesGuard classifies once and admits healthy, needs-review with +// the primary checkout on the integration branch, or either once init's +// clean-checkout and at-tip preconditions are set aside; // every other state is an invalid-state refusal whose remedy is valid there. // Pure over the gathered facts. func configureHarnessesGuard(facts reposetup.Facts) (reposetup.Classification, *RepositoryOpResult) { @@ -121,8 +121,16 @@ func configureHarnessesGuard(facts reposetup.Facts) (reposetup.Classification, * return cls, &out } switch cls.State { - case reposetup.StateHealthy, reposetup.StateNeedsReview: + case reposetup.StateHealthy: return cls, nil + case reposetup.StateNeedsReview: + // Needs-review is decided before the healthy postconditions, so it does not + // prove the primary checkout is on the integration branch; require that as + // the healthy path does (an unknown probe is not a proven absence). + if facts.PrimaryOnIntegration != reposetup.PresenceAbsent { + return cls, nil + } + return refuse("the primary checkout is not on the integration branch; run `docket repository check` and resolve the reported findings first") case reposetup.StateFresh: return refuse("repository is not initialized; run `docket repository init` (it takes --harnesses too)") case reposetup.StateLegacy: @@ -136,24 +144,15 @@ func configureHarnessesGuard(facts reposetup.Facts) (reposetup.Classification, * return refuse("repository is not in a healthy state; run `docket repository check` and resolve the reported findings first") } -// workingTreePendingPaths lists, sorted, every docket-managed working-tree path -// that differs from HEAD in the primary worktree — read from git status, so a -// surface removed this run counts as pending too. A private repository has -// none: its config and surfaces live outside the working tree. -func workingTreePendingPaths(ctx context.Context, git *gitcli.Client, sc setupContext) ([]string, error) { +// workingTreePendingPaths lists, sorted, the docket-managed working-tree paths +// pending review in the primary worktree — the set repository check names +// (collectPendingReviewPaths), so a surface removed this run counts as pending +// too and an unrelated .gitignore edit does not once the managed block is +// committed. Unlike check, a status read error is returned. A private +// repository has none: its config and surfaces live outside the working tree. +func workingTreePendingPaths(ctx context.Context, git *gitcli.Client, sc setupContext, committedIgnore reposetup.Presence) ([]string, error) { if sc.layout.Mode == layout.Private { return nil, nil } - changes, err := git.ChangedPaths(ctx, sc.repo.PrimaryWorktree) - if err != nil { - return nil, err - } - var pending []string - for _, ch := range changes { - if rel := string(ch.Path); docketManagedWorktreePaths[rel] { - pending = appendPending(pending, rel) - } - } - sort.Strings(pending) - return pending, nil + return collectPendingReviewPaths(ctx, git, sc.repo, sc.layout.Mode, committedIgnore) } diff --git a/internal/app/repository_configure_harnesses_test.go b/internal/app/repository_configure_harnesses_test.go index df6bc9a73..ba0f7dedb 100644 --- a/internal/app/repository_configure_harnesses_test.go +++ b/internal/app/repository_configure_harnesses_test.go @@ -14,8 +14,12 @@ import ( // valid there. func TestConfigureHarnessesGuard(t *testing.T) { admitted := map[string]func(*reposetup.Facts){ - "healthy": func(*reposetup.Facts) {}, - "needs-review": func(f *reposetup.Facts) { f.PendingReviewPaths = []string{".docket.yml"} }, + "healthy": func(*reposetup.Facts) {}, + "needs-review": func(f *reposetup.Facts) { f.PendingReviewPaths = []string{".docket.yml"} }, + "needs-review, branch probe unknown": func(f *reposetup.Facts) { + f.PendingReviewPaths = []string{".docket.yml"} + f.PrimaryOnIntegration = reposetup.PresenceUnknown + }, "dirty primary": func(f *reposetup.Facts) { f.PrimaryClean = reposetup.PresenceAbsent }, "behind the tip": func(f *reposetup.Facts) { f.PrimaryAtRemoteTip = reposetup.PresenceAbsent }, } @@ -44,6 +48,11 @@ func TestConfigureHarnessesGuard(t *testing.T) { }, "docket repository migrate"}, {"foreign worktree", func(f *reposetup.Facts) { f.DocketWorktree.Foreign = true }, "docket repository check"}, {"hooks on", func(f *reposetup.Facts) { f.DocketWorktree.HooksOff = reposetup.PresenceAbsent }, "docket repository check"}, + {"healthy off the integration branch", func(f *reposetup.Facts) { f.PrimaryOnIntegration = reposetup.PresenceAbsent }, "docket repository check"}, + {"needs-review off the integration branch", func(f *reposetup.Facts) { + f.PendingReviewPaths = []string{".docket.yml"} + f.PrimaryOnIntegration = reposetup.PresenceAbsent + }, "docket repository check"}, } for _, tc := range refused { t.Run("refuses "+tc.name, func(t *testing.T) { diff --git a/internal/app/repository_init.go b/internal/app/repository_init.go index 6aa6efbd4..7ca54de73 100644 --- a/internal/app/repository_init.go +++ b/internal/app/repository_init.go @@ -194,7 +194,7 @@ func RunRepositoryInit(ctx context.Context, d SetupDeps, o InitOptions) Reposito // The surfaces pending review come from git status, as configure-harnesses // reads them: a planned target that is gitignored (the Cursor rule) or // unchanged is not something to commit. - surfacePending, serr := workingTreePendingPaths(ctx, d.Git, sc) + surfacePending, serr := workingTreePendingPaths(ctx, d.Git, sc, facts.CommittedIgnoreBlock) if serr != nil { return repositoryExternalFailure(OperationRepositoryInit, cls.State, "listing the pending review paths", serr) } diff --git a/internal/reposetup/harnessesedit.go b/internal/reposetup/harnessesedit.go index 0c4080275..89f8aa74d 100644 --- a/internal/reposetup/harnessesedit.go +++ b/internal/reposetup/harnessesedit.go @@ -81,15 +81,22 @@ func RenderAgentHarnessesEdit(existing []byte, harnesses []string) (edited []byt if harnessSequenceEquals(val, harnesses) { return existing, false, nil } + starts := lineOffsets(existing) start, end := root.Content[keyIdx].Line, maxNodeLine(val) if end < start || val.Kind == yaml.ScalarNode && val.Tag == "!!null" && val.Value == "" { end = start // an empty value lives on the key's own line } + // A flow sequence's node stops at its last item; its closing ] may sit on + // a later line. + if val.Kind == yaml.SequenceNode && val.Style&yaml.FlowStyle != 0 { + if closing := flowSequenceEndLine(existing, starts, val); closing > end { + end = closing + } + } if keyIdx > 0 && maxNodeLine(root.Content[keyIdx-1]) >= start || keyIdx+2 < len(root.Content) && root.Content[keyIdx+2].Line <= end { return nil, false, fmt.Errorf("reposetup: %q shares a line with another setting%s", agentHarnessesKey, byHand) } - starts := lineOffsets(existing) from, to := starts[start-1], lineEndByte(existing, starts, end) text := line if !(to == len(existing) && (to == 0 || existing[to-1] != '\n')) { @@ -103,6 +110,57 @@ func RenderAgentHarnessesEdit(existing []byte, harnesses []string) (edited []byt return out, true, nil } +// flowSequenceEndLine returns the 1-based line holding the ] that closes the +// flow sequence n, whose [ is at n's own position in src, skipping quoted +// scalars and comments. It returns 0 when that bracket cannot be matched; the +// post-edit re-parse then refuses any splice that cut the value short. +func flowSequenceEndLine(src []byte, starts []int, n *yaml.Node) int { + if n.Line < 1 || n.Line > len(starts) || n.Column < 1 { + return 0 + } + i := starts[n.Line-1] + n.Column - 1 + if i >= len(src) || src[i] != '[' { + return 0 + } + line, depth := n.Line, 0 + for ; i < len(src); i++ { + switch c := src[i]; c { + case '\n': + line++ + case '[', '{': + depth++ + case ']', '}': + if depth--; depth == 0 { + return line + } + case '#': + if i > 0 && src[i-1] != ' ' && src[i-1] != '\t' && src[i-1] != '\n' { + continue + } + for i+1 < len(src) && src[i+1] != '\n' { + i++ + } + case '"', '\'': + for i++; i < len(src); i++ { + if src[i] == '\n' { + line++ + } else if c == '"' && src[i] == '\\' { + if i++; i < len(src) && src[i] == '\n' { + line++ + } + } else if src[i] == c { + if c == '\'' && i+1 < len(src) && src[i+1] == '\'' { + i++ + continue + } + break + } + } + } + } + return 0 +} + // harnessSequenceEquals reports whether n is a sequence of exactly the string // scalars want, in order. func harnessSequenceEquals(n *yaml.Node, want []string) bool { diff --git a/internal/reposetup/harnessesedit_test.go b/internal/reposetup/harnessesedit_test.go index e392d13f0..89991c7fe 100644 --- a/internal/reposetup/harnessesedit_test.go +++ b/internal/reposetup/harnessesedit_test.go @@ -20,6 +20,9 @@ func TestRenderAgentHarnessesEditRewrites(t *testing.T) { {"replace keeps CRLF", "a: 1\r\nagent_harnesses: [codex]\r\nb: 2\r\n", []string{"claude"}, "a: 1\r\nagent_harnesses: [claude]\r\nb: 2\r\n"}, {"append keeps CRLF", "a: 1\r\n", []string{}, "a: 1\r\nagent_harnesses: []\r\n"}, {"replace final line without newline", "a: 1\nagent_harnesses: [codex]", []string{}, "a: 1\nagent_harnesses: []"}, + {"replace multi-line flow", "a: 1\nagent_harnesses: [\n claude,\n codex\n]\nb: 2\n", []string{"cursor"}, "a: 1\nagent_harnesses: [cursor]\nb: 2\n"}, + {"replace multi-line flow, bracket after comment", "agent_harnesses: [ # mine\n codex, \"]\" ,\n # ] not here\n claude\n ] # end\nb: 2\n", []string{"cursor"}, "agent_harnesses: [cursor]\nb: 2\n"}, + {"replace multi-line empty flow", "agent_harnesses: [\n]\nb: 2\n", []string{"claude"}, "agent_harnesses: [claude]\nb: 2\n"}, {"replace empty value", "agent_harnesses:\nb: 2\n", []string{"cursor"}, "agent_harnesses: [cursor]\nb: 2\n"}, {"nested key is not top-level", "build:\n agent_harnesses: x\n", []string{"claude"}, "build:\n agent_harnesses: x\nagent_harnesses: [claude]\n"}, {"comments-only file", "# only a comment\n", []string{"claude"}, "# only a comment\nagent_harnesses: [claude]\n"}, From ee11acbc2bbeb312e794da339642fdf516dae7db Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 22:23:56 -0400 Subject: [PATCH 12/12] fix(harnesses): address review findings - refuse a repeated none in --harnesses as listed more than once - omit the review-and-commit clause when no paths are pending - cover configure-harnesses claude clearing harnesses-unset in a private repo - repeat the configure-harnesses follow-up after the final upgrade step - move configure-harnesses after configure-tests --command in the glossary --- docs/reference/glossary.md | 2 +- docs/release/upgrading-from-bash.md | 4 ++++ internal/app/repocheck_harnesses_integration_test.go | 6 ++++++ internal/app/repository_configure_harnesses.go | 2 ++ internal/reposetup/harnesses.go | 3 +++ internal/reposetup/harnesses_test.go | 1 + 6 files changed, 17 insertions(+), 1 deletion(-) diff --git a/docs/reference/glossary.md b/docs/reference/glossary.md index 6d22b3d26..88883f3ae 100644 --- a/docs/reference/glossary.md +++ b/docs/reference/glossary.md @@ -50,8 +50,8 @@ cd docket repository init # a repository that has never used docket (--harnesses picks the agents up front) docket repository migrate # a legacy single-branch repository (human-typed) docket repository configure-tests # set the build and finalize test commands -docket repository configure-harnesses # choose which coding agents get docket's instructions docket repository configure-tests --command "sh ./test.sh" # set both gates to local with this command +docket repository configure-harnesses # choose which coding agents get docket's instructions ``` ### Archived record diff --git a/docs/release/upgrading-from-bash.md b/docs/release/upgrading-from-bash.md index 94c73d65d..f418629c7 100644 --- a/docs/release/upgrading-from-bash.md +++ b/docs/release/upgrading-from-bash.md @@ -235,6 +235,10 @@ git commit -am "Remove the Bash docket block from CLAUDE.md" git push ``` +Last, choose your coding agents, which the configure-harnesses step left as "none yet": run +`docket repository configure-harnesses --harnesses ` (for example `claude`) and commit the +paths it lists. + ## 6. Settings that changed These are the Bash-era settings docket reports, in `.docket.yml` or in diff --git a/internal/app/repocheck_harnesses_integration_test.go b/internal/app/repocheck_harnesses_integration_test.go index 7e7402821..9759979bf 100644 --- a/internal/app/repocheck_harnesses_integration_test.go +++ b/internal/app/repocheck_harnesses_integration_test.go @@ -68,4 +68,10 @@ func TestIntegrationRepoCheckHarnessesUnsetPrivate(t *testing.T) { t.Fatalf("configure-harnesses none = %q (%s), want applied", none.Result, none.HumanText()) } assertHarnessesUnset(t, "none", checkIn(t, r.invocation), false, 0) + + claude := r.runConfigureHarnesses(t, ConfigureHarnessesOptions{Harnesses: harnessFlag("claude")}) + if claude.Result != ResultApplied { + t.Fatalf("configure-harnesses claude = %q (%s), want applied", claude.Result, claude.HumanText()) + } + assertHarnessesUnset(t, "claude", checkIn(t, r.invocation), false, 0) } diff --git a/internal/app/repository_configure_harnesses.go b/internal/app/repository_configure_harnesses.go index 81d187d67..b51eb71dd 100644 --- a/internal/app/repository_configure_harnesses.go +++ b/internal/app/repository_configure_harnesses.go @@ -101,6 +101,8 @@ func RunRepositoryConfigureHarnesses(ctx context.Context, d SetupDeps, o Configu out.human = fmt.Sprintf("%s: %s (%s): agent_harnesses is already %s and its instructions are current; nothing to write", op, ResultNoOp, state, list) case display == layout.PrivateConfigDisplay: out.human = fmt.Sprintf("agent harnesses set to %s (%s); wrote %s", list, state, layout.PrivateConfigDisplay) + case len(pending) == 0: + out.human = fmt.Sprintf("agent harnesses set to %s (%s)", list, state) default: out.human = fmt.Sprintf("agent harnesses set to %s (%s); review and commit the pending paths: %s", list, state, strings.Join(pending, ", ")) } diff --git a/internal/reposetup/harnesses.go b/internal/reposetup/harnesses.go index 2287a7066..affee0e63 100644 --- a/internal/reposetup/harnesses.go +++ b/internal/reposetup/harnesses.go @@ -36,6 +36,9 @@ func ParseHarnessSelection(tokens []string) ([]string, error) { case tok == "": empty = true case tok == HarnessesNone: + if none && !containsString(dups, tok) { + dups = append(dups, tok) + } none = true case !containsString(allowed, tok): unknown = append(unknown, tok) diff --git a/internal/reposetup/harnesses_test.go b/internal/reposetup/harnesses_test.go index de0565603..055d2114e 100644 --- a/internal/reposetup/harnesses_test.go +++ b/internal/reposetup/harnesses_test.go @@ -40,6 +40,7 @@ func TestParseHarnessSelection(t *testing.T) { {[]string{"claude", ""}, []string{"empty"}}, {[]string{"claude", "claude"}, []string{"claude", "more than once"}}, {[]string{"none", "claude"}, []string{"none"}}, + {[]string{"none", "none"}, []string{"none", "more than once"}}, {[]string{"bogus", "claude", "nope"}, []string{"bogus", "nope", "claude, codex, cursor, opencode"}}, {[]string{"Claude"}, []string{"Claude"}}, }