diff --git a/.docket.example.yml b/.docket.example.yml index 0a92a7ed7..c7ecd0496 100644 --- a/.docket.example.yml +++ b/.docket.example.yml @@ -174,6 +174,18 @@ leak_check: match_word: true +# ═══ open — where docket open takes you ═══════════════════════════════════════════════════ + +open: + # artifacts — github (default): in a shared repository, `docket open` shows the board, a + # change, and its spec, plan, and results as GitHub pages in your browser. local: open the + # file from your metadata checkout in your system's default app for Markdown instead — + # handy offline, or when you would rather read in an editor. A private repository always + # opens the local file, and `docket open pr` always opens the pull request on GitHub. + # scope: any layer + artifacts: github + + # ═══ learnings — the learnings ledger ═════════════════════════════════════════════════════ learnings: diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 679b37786..68c4ab722 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -43,6 +43,9 @@ Each line is a pointer: the command, what its verbs govern, and where to read th - **`docket learning`** — record and update learning findings. Verbs: `docket learning --help`. - **`docket maintenance`** — close out merged changes, retry close-out cleanup, and reclaim expired claims in one pass (`docket status` stays read-only). Verbs: `docket maintenance --help`. +- **`docket open`** — open the board, or a change's record, spec, plan, results, or pull request, + as a GitHub page or from the local metadata checkout. Without an id it opens the change whose + branch is checked out. Details: `docket open --help`. - **`docket pr`** — publish the ready-for-review pull request for an in-progress change's tested head. Verbs: `docket pr --help`. - **`docket repository`** — initialize, migrate, check, and repair the docket repository diff --git a/docs/reference/config-keys.md b/docs/reference/config-keys.md index c43765b00..1f769db7c 100644 --- a/docs/reference/config-keys.md +++ b/docs/reference/config-keys.md @@ -52,6 +52,7 @@ Each key has one scope: | `reclaim.lease_ttl` | `72` | any layer | hours a claim on an in-progress change lasts | | `reclaim.auto` | `false` | any layer | whether `docket maintenance sweep` reclaims an eligible change or reports it skipped | | `learnings.enabled` | `true` | any layer | whether learnings are written and read | +| `open.artifacts` | `github` | any layer | where `docket open` shows a shared repository's board and change artifacts: `github` pages, or the `local` metadata checkout's files | | `gate_observation_budget` | `30` | any layer | minutes docket waits for a test-suite run it started to finish | | `board_surfaces` | `[inline]` | any layer | which board views to render; `[]` renders none | | `board.section_order` | `[in-progress, built, blocked, groomed, proposed, deferred]` | any layer | the order of the board's sections | diff --git a/docs/superpowers/specs/2026-10-08-open-the-board-a-change-and-its-artifacts-from-the-terminal-design.md b/docs/superpowers/specs/2026-10-08-open-the-board-a-change-and-its-artifacts-from-the-terminal-design.md new file mode 100644 index 000000000..ecb1579d2 --- /dev/null +++ b/docs/superpowers/specs/2026-10-08-open-the-board-a-change-and-its-artifacts-from-the-terminal-design.md @@ -0,0 +1,168 @@ +# Open docket artifacts from the terminal: design + +Change 0542 — Open the board, a change, and its artifacts from the terminal + +Change #542, groomed interactively on 2026-10-08. Related: #136 (artifact backlinks, which introduced the GitHub link builder this reuses), #531 (private visibility, which decides where private metadata lives), #534 (the `dckt` alias, so `dckt open …` works the same way). ADRs: 0019 (config-key layer classification), 0141 (plan, results, and evidence live on the metadata branch), 0142 (private visibility layout). + +## Summary + +Getting from the terminal to a change's spec, plan, results, or PR, or to the board, takes several clicks today. You open GitHub, switch to the `docket` branch, find the record under `active/` or `archive/`, and follow its Artifacts table. A private repository has no web page at all: the files live in a checkout under `~/.local/share/dckt/…`. + +This change adds one command, `docket open [id]`, where `` is `board`, `change`, `spec`, `plan`, `results`, or `pr`. Run with no id inside a checkout whose branch belongs to a change, it opens that change's artifact. + +- In a shared repository, artifacts open as GitHub pages in the browser by default. A new layered config key, `open.artifacts: github | local`, lets a user open the file from the local metadata checkout in their default Markdown app instead. +- Private repositories always open locally. +- `pr` always opens the PR's URL. + +## Evidence gathered at grooming + +- **The GitHub link builder already exists.** + - `githubWebURL` (`internal/app/link_context.go`) turns origin into `https://github.com//` for `git@github.com:`, `https://github.com/`, and `ssh://git@github.com/` remotes only. Any other host gives `""`. + - `linkContextOf(pin)` is the only `render.LinkContext` constructor, which `link_context_guard_test.go` enforces. + - `LinkContext.BlobURL(path)` (`internal/render/link.go`) returns `RepoWebURL + "/blob//" + path`. It returns `""` when `RepoWebURL` is empty, or when `PrivateMetadata` is set and the branch is the metadata branch. The PR-body backlinks use it today. +- **Private metadata has no web view.** `layout.PrivateLayout` puts the bare remote at `/dckt//remote.git` and the metadata checkout at `…/checkouts/`, outside the clone. A private PR body carries no backlinks; `PRPublish` skips them. +- **Record resolution is an established path.** + - The path is `PinContext` (which fetches, via `loadOperationalContext`) → `ReadCorpus` → `parseCorpus` → `repository.BuildSnapshot` → `snap.Change(id)` (`LookupFound` / `LookupAbsent` / `LookupAmbiguous`). `resolvePRChange` and `loadWorkspaceContext` use it. + - `domain.Change` exposes `Path()` (the record's real `active/` or `archive/` path), `Spec()`, `Plan()`, `Results()`, `PR()`, `Branch()`, and `Trivial()`. + - No existing read operation returns spec, plan, results, and PR together. +- **Nothing infers a change from the current directory.** Every operation takes `--id`. The building blocks exist: + - `gitcli.(*Client).WorktreeCheckoutState` (`internal/gitcli/checkoutstate.go`) returns the checked-out branch's full ref, or `Detached`. + - `recordedBranch(c)` (`internal/app/branch_identity.go`) reads a record's branch. +- **No opener code exists.** Nothing in the Go tree runs `open`, `xdg-open`, or `gh browse`. +- **The metadata checkout can lag the metadata tip,** because operations read pinned blobs, not the checkout. `RunRepositoryPrepare` is the established sync: + - a clean, behind checkout is fast-forwarded + - a missing checkout is attached + - a dirty, ahead, or diverged one is refused with a finding +- **One command keeps the catalog small.** Every public executable leaf must be a catalog entry whose id equals its dotted command path (`TestProductionCapabilityCorrespondence`), so one `open` leaf with a positional `` costs one entry, not six. +- **The board path is fixed.** It is `boardCorpusPath(cfg)`, which is `/BOARD.md`. +- **A template for the new key exists.** The `visibility` key (`internal/config/schema.go`) is a string enum leaf with `merge: mergeScalar`, `scope: scopeAny`, `disp: dispSupported`, and `validate: enumLeaf(…)`. That is exactly the shape `open.artifacts` needs. +- **Browsers show Markdown as source.** Chrome, Safari, and Firefox don't render a local `.md` file. GitHub renders it on its servers. That is why local opens go to the operating system's default handler for the file (on the development machine, a Markdown viewer app) rather than to a browser. + +## Design + +### 1. Command + +``` +docket open [id] [--print] [--repo-dir ] [--json] +``` + +- `` is one of `board`, `change`, `spec`, `plan`, `results`, `pr`. Any other value is a usage error that lists the six. +- `board` takes no id. Passing one is a usage error. +- The id is a decimal change id with optional leading zeros (`541`, `0541`). `#541` is not accepted, because an interactive shell can treat a word that starts with `#` as a comment. +- With no id, the change is inferred from the checkout (section 3). +- On success the command prints the opened target on one line: the URL, or the absolute file path. +- `--print` prints the target without launching anything. It is for SSH sessions, scripts, and agents that want to hand a human a link. +- `--json` emits the protocol-v1 envelope with an `OpenResult`: + - `what` + - `change_id` (omitted for `board`) + - `target` + - `target_kind`: `url` or `file` + - `launched`: false under `--print` + - `notes`: a list of one-line notes, such as a fallback reason or a stale-checkout warning +- `dckt open …` works identically through the existing alias. + +### 2. What each target opens + +| `` | GitHub target | Local target (file opened with the OS default app) | +|---|---|---| +| `board` | `BlobURL(/BOARD.md)` | `//BOARD.md` | +| `change` | `BlobURL(change.Path())`, active or archived | `/` | +| `spec` / `plan` / `results` | `BlobURL()` | `/` | +| `pr` | the record's `pr:` value | the same `pr:` URL, because the PR lives on GitHub in both modes | + +**Choosing GitHub or local** (applies to every `` except `pr`): + +1. Private repository: local. `open.artifacts` is not consulted, and no note is added. +2. Otherwise `open.artifacts: local`: local. +3. Otherwise (`github`, the default): use `linkContextOf(pin).BlobURL(path)`. When it returns `""` (origin is not a GitHub remote), open locally and add the note `origin is not a GitHub remote — opened the local file instead`. + +No new visibility or host logic is added. The existing builder's empty result is the fallback trigger. + +**Spec copy.** `spec` always opens the copy on the metadata branch, which exists from grooming onward. The copy on the integration branch, present only after merge, is not used. + +**Board availability.** When the resolved `board_surfaces` has no `inline` surface, `open board` fails with `this repository has no board (board_surfaces has no inline surface)`, even if a stale `BOARD.md` is still present. + +**PR value.** The `pr:` value opens as-is when it is an `http://` or `https://` URL. Anything else fails with an error that names the value. There is no shorthand expansion. + +### 3. Inferring the change + +When `` is not `board` and no id is given: + +1. Read the checkout state of the resolved repo dir (`--repo-dir`, defaulting to the current directory) with `WorktreeCheckoutState`. +2. Detached HEAD fails: `HEAD is detached — pass a change id`. +3. Compare the branch's short name with every record's recorded branch (`recordedBranch`). Check active records first, and archived records only when no active record matches. +4. Exactly one match: that change. No match: `branch belongs to no change — pass a change id`. That covers the metadata checkout and the main checkout on `main`. More than one active match: `branch is recorded by changes , — pass a change id`. + +Matching is by branch, not by folder path, so it works in `.worktrees/` and in the main checkout when a feature branch is checked out there. + +### 4. Local opens and freshness + +Before any local open, the command runs the same work as `repository.prepare` (`RunRepositoryPrepare`) against the repository: + +- `applied` / `no-op`: the checkout is current, or was just fast-forwarded or attached. Open the file. +- `refused` / `error`: the checkout may be stale, for example because of uncommitted edits. Open the file on disk anyway and add a note naming prepare's finding code: `metadata checkout not synced () — file may be stale`. This never blocks. +- If the file does not exist on disk, fail and name both the path and prepare's finding. + +The path opened is `/`, using the prepare context's `metadata_worktree_path`. The command never assumes `.docket` or a private store path. + +### 5. The `open.artifacts` key + +- New schema entry modelled on `visibility`: `{path: "open.artifacts", kind: kindString, enum: []string{"github", "local"}, def: "github", merge: mergeScalar, scope: scopeAny, disp: dispSupported, validate: enumLeaf("github", "local")}`. +- New `Open.Artifacts Value[string]` on `config.Effective`, assigned in `resolve.go`. +- It is an ordinary layered key: repo-local > repo-committed (or `.git/dckt/config.yml` in a private repo) > global > built-in. It is not a repository-identity key, so under ADR-0019 it is honoured in every layer. +- In a private repository it resolves normally but has no effect, because private metadata has no GitHub copy. This is deliberately silent. A global `github` would otherwise produce a note on every open in every private repository. +- An unknown value fails resolution like every other enum key. + +### 6. Opener + +- A small seam injected into the app deps, never called directly. `open` on darwin, `xdg-open` on linux, and no opener on any other platform. +- The opener receives the URL or absolute path as a single argv element, with no shell. The command waits for the opener to exit and reports a non-zero exit as an error. +- With no opener available (an unsupported platform, or `xdg-open` not on `PATH`), the command prints the target and fails with `no opener available on — open it yourself, or use --print`. +- Tests always inject a fake. The suite never launches a real application. + +### 7. Catalog, schema, docs + +- CLI: `internal/cli/open.go`, registered in `root.go`. Annotation `capability("open", EffectRead, EffectLocalWrite, EffectProcessControl)`. `local-write` covers the prepare sync, and `process-control` covers launching the opener. +- Schema: `{ID: "open", Request: nil, Result: OpenResult{}}` in `operationBindings` (`internal/app/schema_registry.go`), the shape `status` and `run.verify` use. +- Update the asset-independent allowlist in `internal/cli/install.go` (`TestAssetIndependentSetExact`) and the catalog and schema production tests (`capability_production_test.go`, `schema_production_test.go`) for the new entry. +- Docs: + - `docs/reference/cli.md`: one noun line for `docket open`, naming the six targets and the no-id inference. That page is a pointer list and never lists flags; `docket open --help` owns those. + - `.docket.example.yml`: the `open.artifacts` key with its description. ADR-0048 makes this file canonical, and an existing test keeps it in step with the schema, so that test reddens until the key is added. + - `docs/reference/config-keys.md`: the key's row (default `github`, scope any layer). + +### 8. Errors + +Every error is one line with a non-zero exit: + +| Situation | Message | +|---|---| +| unknown ``, or an id with `board` | usage error listing the six targets | +| malformed id | `invalid change id ` | +| no such change | `no change ` | +| field not set | `change has no plan yet` (likewise `results`, `PR`, and `spec` on a non-trivial change) | +| trivial change, `spec` | `change is trivial — it has no spec` | +| board disabled | `this repository has no board (board_surfaces has no inline surface)` | +| `pr:` not a URL | `change pr: is not a URL` | +| inference failures | section 3 messages | +| local file missing | names the path and prepare's finding | +| no opener | section 6 message, after printing the target | + +## Tests + +- **Target table (unit, fake opener, fake reader).** Cover {shared + `github`, shared + `local`, shared with a non-GitHub origin, private} × {`board`, `change` (active and archived), `spec`, `plan`, `results`, `pr`}. Assert `target`, `target_kind`, and notes. Assert that private mode ignores `open.artifacts: github` silently, and that the non-GitHub fallback adds its note. +- **Inference.** Cover a match in a worktree, the main checkout on a feature branch, archived fallback when no active record matches, no match, detached HEAD, the metadata checkout, and two active records recording one branch. Each error message is asserted. +- **Errors.** Each row of section 8. +- **Output.** `--print` launches nothing and prints the target. `--json` carries the `OpenResult` fields. The success path prints exactly one line. +- **Opener selection** by GOOS, the missing-`xdg-open` path, and argv passed as one element. +- **Config.** `open.artifacts` resolves per layer with normal precedence, defaults to `github`, and refuses an unknown value. The registry-default parity tests pick it up. +- **Integration (real fixture repository, private layout).** A behind metadata checkout is fast-forwarded before a local open. A dirty one opens with the stale note. A missing file fails naming the path. +- **Catalog and schema production tests** updated for the `open` entry. +- **Mutation checks (AGENTS.md).** Strip the active-before-archived ordering, the `BlobURL`-empty fallback, and the private short-circuit in turn, and confirm a test reddens for each. + +## Out of scope + +- Web links for hosts other than GitHub (GitHub Enterprise, GitLab, and others). Those fall back to local. +- Rendering Markdown to HTML or serving a local preview site. +- Opening ADRs, learnings, or the integration-branch copy of a spec. +- Shell completion for change ids. +- Changing the board or the `## Artifacts` table to advertise the command. diff --git a/internal/app/config.go b/internal/app/config.go index 2ee9012f6..580ad3473 100644 --- a/internal/app/config.go +++ b/internal/app/config.go @@ -254,6 +254,7 @@ func effectiveLines(eff *config.Effective) []effectiveLine { leafLine("reclaim.lease_ttl", strconv.Itoa(eff.Reclaim.LeaseTTL.Value), eff.Reclaim.LeaseTTL.Provenance), leafLine("reclaim.auto", strconv.FormatBool(eff.Reclaim.Auto.Value), eff.Reclaim.Auto.Provenance), leafLine("leak_check.match_word", strconv.FormatBool(eff.LeakCheck.MatchWord.Value), eff.LeakCheck.MatchWord.Provenance), + leafLine("open.artifacts", textValue(eff.Open.Artifacts.Value), eff.Open.Artifacts.Provenance), leafLine("review.min_fix_severity", textValue(eff.Review.MinFixSeverity.Value), eff.Review.MinFixSeverity.Provenance), leafLine("review.max_fix_tasks", strconv.Itoa(eff.Review.MaxFixTasks.Value), eff.Review.MaxFixTasks.Provenance), leafLine("gate_observation_budget", strconv.Itoa(eff.GateObservation.Value), eff.GateObservation.Provenance), diff --git a/internal/app/config_test.go b/internal/app/config_test.go index df94918a7..2637558dc 100644 --- a/internal/app/config_test.go +++ b/internal/app/config_test.go @@ -839,3 +839,21 @@ func TestConfigDiagnosticsAttemptLimitsSurface(t *testing.T) { t.Errorf("human effective block lacks the resolved run attempt-limit line:\n%s", h) } } + +// TestConfigDiagnosticsOpenArtifactsSurface pins that the effective-config +// diagnostics surface reports open.artifacts on both halves — the +// auto-reflected `effective` JSON and the hand-maintained human block — and +// asserts the resolved value, not mere presence (learnings: +// defaulted-param-hides-caller-wiring). +func TestConfigDiagnosticsOpenArtifactsSurface(t *testing.T) { + src := []config.Source{{Layer: config.LayerRepository, Name: ".docket.yml", Data: []byte("open:\n artifacts: local\n")}} + for _, tc := range []struct { + sources []config.Source + want, line string + }{{sparseSources(), "github", "open.artifacts = github [built-in]"}, {src, "local", "open.artifacts = local"}} { + r := DiagnosticConfig(tc.sources, mainCtx(), false) + if r.Effective == nil || r.Effective.Open.Artifacts.Value != tc.want || !strings.Contains(r.HumanText(), tc.line) { + t.Errorf("want %q and line %q:\n%s", tc.want, tc.line, r.HumanText()) + } + } +} diff --git a/internal/app/open.go b/internal/app/open.go new file mode 100644 index 000000000..aa1ed9048 --- /dev/null +++ b/internal/app/open.go @@ -0,0 +1,384 @@ +package app + +// open.go holds `docket open`: its argument grammar, its result document (the +// six targets it accepts, the reasons a refusal carries, the notes a success +// may attach, and OpenResult, the one document every outcome renders), and +// Open, which resolves a target to a GitHub page or a file in the metadata +// checkout and hands it to the platform opener. + +import ( + "context" + "errors" + "fmt" + "io/fs" + "net/url" + "os" + "path/filepath" + "regexp" + "runtime" + "slices" + "strconv" + "strings" + + "github.com/danielhanold/docket/internal/domain" + "github.com/danielhanold/docket/internal/gitcli" + "github.com/danielhanold/docket/internal/layout" + "github.com/danielhanold/docket/internal/repository" +) + +// OperationOpen is the operation id of `docket open`. +const OperationOpen = "open" + +// OpenTargets lists what `docket open` can open, in usage order. Every target +// but board takes an optional change id. +var OpenTargets = []string{"board", "change", "spec", "plan", "results", "pr"} + +// Target kinds an OpenResult reports: a URL or a local file path. +const ( + OpenTargetKindURL = "url" + OpenTargetKindFile = "file" +) + +// Reasons an OpenResult carries when it refuses or fails. +const ( + ReasonOpenInvalidTarget = "invalid-target" + ReasonOpenUnknownChange = "unknown-change" + ReasonOpenAmbiguousChange = "ambiguous-change" + ReasonOpenArtifactUnset = "artifact-unset" + ReasonOpenTrivialNoSpec = "trivial-no-spec" + ReasonOpenBoardDisabled = "board-disabled" + ReasonOpenPRNotURL = "pr-not-url" + ReasonOpenPathInvalid = "artifact-path-invalid" + ReasonOpenHeadDetached = "head-detached" + ReasonOpenBranchUnmatched = "branch-unmatched" + ReasonOpenBranchAmbiguous = "branch-ambiguous" + ReasonOpenCheckoutUnreadable = "checkout-unreadable" + ReasonOpenFileMissing = "file-missing" + ReasonOpenNoOpener = "no-opener" + ReasonOpenOpenerFailed = "opener-failed" +) + +// openNoteNotGitHub is the note attached when a GitHub page was wanted but the +// origin is not a GitHub remote, so the local file was opened instead. +const openNoteNotGitHub = "origin is not a GitHub remote — opened the local file instead" + +// openNoteStale is the note attached when preparing the metadata checkout +// failed with code: the local file opened anyway, but may be out of date. +func openNoteStale(code string) string { + return "metadata checkout not synced (" + code + ") — file may be stale" +} + +// OpenOptions is one `docket open` request. +type OpenOptions struct { + RepoDir string + What string + ID int // 0 = infer from the checked-out branch + Print bool +} + +// OpenDeps is what Open reads and runs, injected so tests start no real git +// and launch no real application. +type OpenDeps struct { + Reader StatusReader + Checkout func(ctx context.Context, dir string) (gitcli.CheckoutState, error) + Prepare func(ctx context.Context, repoDir string) RepositoryPrepareResult + Opener Opener +} + +// OpenResult is the document `docket open` emits for every outcome. ChangeID is +// omitted for the board; Notes is always a list, possibly empty. +type OpenResult struct { + Envelope + What string `json:"what"` + ChangeID int `json:"change_id,omitempty"` + Target string `json:"target,omitempty"` + TargetKind string `json:"target_kind,omitempty"` + Launched bool `json:"launched"` + Notes []string `json:"notes"` + Reason string `json:"reason,omitempty"` + Message string `json:"message,omitempty"` +} + +// HumanText renders the result for a terminal: the target alone on success +// (so `--print` composes in command substitution), the resolved target then +// the failure message when a failure follows resolution, or the message alone. +func (r OpenResult) HumanText() string { + if r.Message == "" { + return r.Target + } + if r.Target != "" { + return r.Target + "\n" + r.Message + } + return r.Message +} + +// HumanNotes returns the notes a human renderer writes apart from HumanText. +func (r OpenResult) HumanNotes() []string { return r.Notes } + +// openFailure is a refusal or failure on the way to a target, carrying the +// result class, reason, and message the OpenResult reports. +type openFailure struct { + result Result + reason string + message string +} + +// openIDPattern admits plain decimal ids, leading zeros included; a sign, a +// "#", or a hex prefix is refused. +var openIDPattern = regexp.MustCompile(`^[0-9]+$`) + +// ParseOpenArgs parses `open [id]` into the target and the change id, +// 0 when none was given. Every error is a one-line usage message. +func ParseOpenArgs(args []string) (string, int, error) { + targets := strings.Join(OpenTargets, ", ") + switch { + case len(args) == 0: + return "", 0, fmt.Errorf("missing target — choose one of %s", targets) + case len(args) > 2: + return "", 0, fmt.Errorf("too many arguments — usage: open [id], where is one of %s", targets) + } + what := args[0] + if !slices.Contains(OpenTargets, what) { + return "", 0, fmt.Errorf("unknown target %q — choose one of %s", what, targets) + } + if len(args) == 1 { + return what, 0, nil + } + if what == "board" { + return "", 0, fmt.Errorf("board takes no change id — the targets are %s, and only the last five take an id", targets) + } + raw := args[1] + n, err := strconv.Atoi(raw) + if !openIDPattern.MatchString(raw) || err != nil || n <= 0 { + return "", 0, fmt.Errorf("invalid change id %s", raw) + } + return what, n, nil +} + +// Open resolves what (and the change id, or the change the checked-out branch +// records when the id is 0) to a target and opens it. A private repository, +// open.artifacts: local, or a non-GitHub origin opens the file from the +// metadata checkout after preparing it; otherwise the GitHub blob page opens. +// A change's PR always opens its URL. With Print set nothing is launched. +func Open(ctx context.Context, deps OpenDeps, o OpenOptions) OpenResult { + res := OpenResult{What: o.What, ChangeID: o.ID, Notes: []string{}} + fail := func(f openFailure) OpenResult { + res.Envelope = NewEnvelope(OperationOpen, f.result) + res.Reason, res.Message = f.reason, f.message + return res + } + if !slices.Contains(OpenTargets, o.What) || (o.What == "board" && o.ID != 0) { + args := []string{o.What} + if o.ID != 0 { + args = append(args, strconv.Itoa(o.ID)) + } + _, _, err := ParseOpenArgs(args) + return fail(openFailure{ResultInvalidInput, ReasonOpenInvalidTarget, err.Error()}) + } + if o.RepoDir == "" { + return fail(openFailure{ResultInvalidInput, ReasonOpenInvalidTarget, "a repository directory is required"}) + } + // The checkout probe refuses a relative path, so every seam gets an + // absolute one. + dir, err := filepath.Abs(o.RepoDir) + if err != nil { + return fail(openFailure{ResultInvalidInput, ReasonOpenInvalidTarget, fmt.Sprintf("cannot resolve %s: %v", o.RepoDir, err)}) + } + branch := "" + if o.What != "board" && o.ID == 0 { // before any fetch + b, f := checkoutBranch(ctx, deps.Checkout, dir) + if f != nil { + return fail(*f) + } + branch = b + } + pin, err := deps.Reader.PinContext(ctx, dir) + if err != nil { + result, reason := classifyStatusError(ctx, err) + return fail(openFailure{result, reason, err.Error()}) + } + eff := pin.Config.Effective + var rel string + if o.What == "board" { + if !slices.Contains(eff.BoardSurfaces.Value, boardSurfaceInline) { + return fail(openFailure{ResultInvalidState, ReasonOpenBoardDisabled, "this repository has no board (board_surfaces has no inline surface)"}) + } + rel = boardCorpusPath(eff) + } else { + c, f := resolveOpenChange(ctx, deps.Reader, pin, o.ID, branch) + if f != nil { + return fail(*f) + } + res.ChangeID = int(c.ID()) + if o.What == "pr" { + u, f := changePRURL(c) + if f != nil { + return fail(*f) + } + res.Target, res.TargetKind = u, OpenTargetKindURL + return finishOpen(ctx, deps, o, res) + } + if rel, f = changeArtifactPath(c, o.What); f != nil { + return fail(*f) + } + } + // A recorded path that escapes the repository is never opened, stat'ed, + // or linked. + if !filepath.IsLocal(filepath.FromSlash(rel)) { + return fail(openFailure{ResultInvalidState, ReasonOpenPathInvalid, fmt.Sprintf("%s path %q is not a path inside the repository", o.What, rel)}) + } + if pin.Layout.Mode != layout.Private && eff.Open.Artifacts.Value != "local" { + if u := linkContextOf(pin).BlobURL(rel); u != "" { + res.Target, res.TargetKind = u, OpenTargetKindURL + return finishOpen(ctx, deps, o, res) + } + res.Notes = append(res.Notes, openNoteNotGitHub) + } + path, notes, f := localArtifact(ctx, deps, dir, pin, rel) + res.Notes = append(res.Notes, notes...) + if f != nil { + return fail(*f) + } + res.Target, res.TargetKind = path, OpenTargetKindFile + return finishOpen(ctx, deps, o, res) +} + +// localArtifact prepares the metadata checkout and returns rel's path inside +// it. A prepare that does not succeed never blocks the open: it adds a +// staleness note, and the layout's metadata worktree stands in for prepare's +// own path. A file absent from the checkout is refused. +func localArtifact(ctx context.Context, deps OpenDeps, dir string, pin StatusPin, rel string) (string, []string, *openFailure) { + pr := deps.Prepare(ctx, dir) + root, code := pin.Layout.MetadataWorktree, "" + var notes []string + if pr.Disposition == PrepareDispositionApplied || pr.Disposition == PrepareDispositionNoOp { + if pr.Context != nil && pr.Context.MetadataWorktreePath != "" { + root = pr.Context.MetadataWorktreePath + } + } else { + code = prepareFindingCode(pr) + notes = append(notes, openNoteStale(code)) + } + path := filepath.Join(root, filepath.FromSlash(rel)) + if _, err := os.Stat(path); err != nil { + msg := fmt.Sprintf("%s is not in the metadata checkout", path) + if !errors.Is(err, fs.ErrNotExist) { + msg = fmt.Sprintf("could not read %s: %v", path, err) + } + if code != "" { + msg += fmt.Sprintf(" (metadata checkout not synced: %s)", code) + } + return "", notes, &openFailure{ResultInvalidState, ReasonOpenFileMissing, msg} + } + return path, notes, nil +} + +// resolveOpenChange reads the pinned corpus and returns change id, or, when id +// is 0, the change whose recorded branch is branch. +func resolveOpenChange(ctx context.Context, reader StatusReader, pin StatusPin, id int, branch string) (domain.Change, *openFailure) { + blobs, err := reader.ReadCorpus(ctx, pin) + if err != nil { + result, reason := classifyStatusError(ctx, err) + return domain.Change{}, &openFailure{result, reason, err.Error()} + } + inputs, _ := parseCorpus(blobs) + build, err := repository.BuildSnapshot(repository.BuildInput{Config: pin.Config.Effective, Documents: inputs}) + if err != nil { + return domain.Change{}, &openFailure{ResultInternalError, ReasonStatusInternalError, err.Error()} + } + if id == 0 { + return inferChangeFromBranch(build.Snapshot, branch) + } + c, out := build.Snapshot.Change(domain.ChangeID(id)) + switch out { + case domain.LookupFound: + return c, nil + case domain.LookupAmbiguous: + return domain.Change{}, &openFailure{ResultInvalidState, ReasonOpenAmbiguousChange, + fmt.Sprintf("more than one record claims change %d — refusing to choose", id)} + } + return domain.Change{}, &openFailure{ResultInvalidInput, ReasonOpenUnknownChange, fmt.Sprintf("no change %d", id)} +} + +// changeArtifactPath returns the repository-relative path what names for c: the +// record itself, or its spec, plan, or results. A trivial change has no spec; +// an unset or malformed field is refused as not there yet. +func changeArtifactPath(c domain.Change, what string) (string, *openFailure) { + var field domain.OptionalString + switch what { + case "change": + return c.Path(), nil + case "spec": + if c.Trivial() { + return "", &openFailure{ResultInvalidState, ReasonOpenTrivialNoSpec, + fmt.Sprintf("change %d is trivial — it has no spec", int(c.ID()))} + } + field = c.Spec() + case "plan": + field = c.Plan() + case "results": + field = c.Results() + } + if field.State != domain.FieldPresent || strings.TrimSpace(field.Value) == "" { + return "", &openFailure{ResultInvalidState, ReasonOpenArtifactUnset, + fmt.Sprintf("change %d has no %s yet", int(c.ID()), what)} + } + return field.Value, nil +} + +// changePRURL returns c's recorded pull-request URL, refusing an unset value +// and anything that is not an absolute http(s) URL. +func changePRURL(c domain.Change) (string, *openFailure) { + pr := c.PR() + if pr.State != domain.FieldPresent || strings.TrimSpace(pr.Value) == "" { + return "", &openFailure{ResultInvalidState, ReasonOpenArtifactUnset, fmt.Sprintf("change %d has no PR yet", int(c.ID()))} + } + u, err := url.Parse(pr.Value) + if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" { + return "", &openFailure{ResultInvalidState, ReasonOpenPRNotURL, + fmt.Sprintf("change %d pr: %s is not a URL", int(c.ID()), pr.Value)} + } + return pr.Value, nil +} + +// prepareFindingCode names why a prepare did not succeed: its first finding's +// code, else the repository state, else the disposition. +func prepareFindingCode(pr RepositoryPrepareResult) string { + if len(pr.Findings) > 0 && pr.Findings[0].Code != "" { + return pr.Findings[0].Code + } + if pr.RepositoryState != "" { + return pr.RepositoryState + } + return pr.Disposition +} + +// finishOpen launches the resolved target through the opener, or, with Print, +// only reports it. On failure the target stays set so HumanText prints it +// first. +func finishOpen(ctx context.Context, deps OpenDeps, o OpenOptions, res OpenResult) OpenResult { + if o.Print { + res.Envelope = NewEnvelope(OperationOpen, ResultApplied) + return res + } + opener := deps.Opener + var err error + if opener == nil { + err = &NoOpenerError{GOOS: runtime.GOOS} + } else { + err = opener.Open(ctx, res.Target) + } + var none *NoOpenerError + switch { + case errors.As(err, &none): + res.Envelope = NewEnvelope(OperationOpen, ResultExternalFailed) + res.Reason, res.Message = ReasonOpenNoOpener, none.Error() + case err != nil: + res.Envelope = NewEnvelope(OperationOpen, ResultExternalFailed) + res.Reason, res.Message = ReasonOpenOpenerFailed, fmt.Sprintf("could not open %s: %v", res.Target, err) + default: + res.Envelope = NewEnvelope(OperationOpen, ResultApplied) + res.Launched = true + } + return res +} diff --git a/internal/app/open_infer.go b/internal/app/open_infer.go new file mode 100644 index 000000000..f44909196 --- /dev/null +++ b/internal/app/open_infer.go @@ -0,0 +1,72 @@ +package app + +// open_infer.go infers which change `docket open` means when no id is given: +// it reads the branch checked out in the working directory and matches it +// against the branch each change records. + +import ( + "context" + "fmt" + "sort" + "strconv" + "strings" + + "github.com/danielhanold/docket/internal/domain" + "github.com/danielhanold/docket/internal/gitcli" +) + +// checkoutBranch returns the short name of the branch checked out at dir. A +// probe error is reported as checkout-unreadable, never as a branch that +// belongs to no change; a detached HEAD is its own refusal. +func checkoutBranch(ctx context.Context, probe func(context.Context, string) (gitcli.CheckoutState, error), dir string) (string, *openFailure) { + st, err := probe(ctx, dir) + if err != nil { + return "", &openFailure{ResultExternalFailed, ReasonOpenCheckoutUnreadable, + fmt.Sprintf("could not read the branch checked out at %s: %v — pass a change id", dir, err)} + } + if st.Detached || st.Branch == "" { + return "", &openFailure{ResultInvalidInput, ReasonOpenHeadDetached, "HEAD is detached — pass a change id"} + } + return strings.TrimPrefix(string(st.Branch), branchRefPrefix), nil +} + +// inferChangeFromBranch finds the change whose recorded branch is branch. +// Active records win; archived records are consulted only when no active one +// matches. No match, or more than one in the winning set, is refused with a +// message telling the user to pass a change id. +func inferChangeFromBranch(snap domain.Snapshot, branch string) (domain.Change, *openFailure) { + var active, archived []domain.Change + for _, c := range snap.Changes() { + b, err := recordedBranch(c) + if err != nil || b != branch { + continue + } + if c.Location() == domain.LocationArchive { + archived = append(archived, c) + } else { + active = append(active, c) + } + } + matches := active + if len(matches) == 0 { + matches = archived + } + switch len(matches) { + case 1: + return matches[0], nil + case 0: + return domain.Change{}, &openFailure{ResultInvalidInput, ReasonOpenBranchUnmatched, + fmt.Sprintf("branch %s belongs to no change — pass a change id", branch)} + } + ids := make([]int, 0, len(matches)) + for _, c := range matches { + ids = append(ids, int(c.ID())) + } + sort.Ints(ids) + parts := make([]string, len(ids)) + for i, id := range ids { + parts[i] = strconv.Itoa(id) + } + return domain.Change{}, &openFailure{ResultInvalidInput, ReasonOpenBranchAmbiguous, + fmt.Sprintf("branch %s is recorded by changes %s — pass a change id", branch, strings.Join(parts, ", "))} +} diff --git a/internal/app/open_infer_test.go b/internal/app/open_infer_test.go new file mode 100644 index 000000000..069827db2 --- /dev/null +++ b/internal/app/open_infer_test.go @@ -0,0 +1,87 @@ +package app + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/danielhanold/docket/internal/domain" + "github.com/danielhanold/docket/internal/gitcli" + "github.com/danielhanold/docket/internal/repository" +) + +// openChange builds a change blob: active = in-progress under active/, +// archived = done under archive/ with a date prefix. extra is raw frontmatter. +func openChange(id int, slug string, archived bool, extra string) StatusBlob { + status, loc := "in-progress", repository.LocationActive + path := fmt.Sprintf("docs/changes/active/%04d-%s.md", id, slug) + if archived { + status, loc = "done", repository.LocationArchive + path = fmt.Sprintf("docs/changes/archive/2026-01-03-%04d-%s.md", id, slug) + } + fm := fmt.Sprintf("---\nid: %d\nslug: %s\ntitle: Change %d\nstatus: %s\npriority: medium\ntype: feat\ncreated: 2026-01-02\n%s---\n\nBody.\n", + id, slug, id, status, extra) + return StatusBlob{Kind: repository.KindChange, Location: loc, Path: path, Revision: fmt.Sprintf("blob%04d%s", id, slug), Data: []byte(fm)} +} + +func openSnapshot(t *testing.T, blobs ...StatusBlob) domain.Snapshot { + t.Helper() + inputs, _ := parseCorpus(blobs) + build, err := repository.BuildSnapshot(repository.BuildInput{Config: testConfig(t).Effective, Documents: inputs}) + if err != nil { + t.Fatalf("BuildSnapshot: %v", err) + } + return build.Snapshot +} + +func TestInferChangeFromBranch(t *testing.T) { + w := openChange(7, "widget", false, "branch: feat/widget\n") + none := func(b string) string { return "branch " + b + " belongs to no change — pass a change id" } + cases := []struct { + branch string + blobs []StatusBlob + wantID int + reason, msg string + }{ + {"feat/widget", []StatusBlob{w}, 7, "", ""}, + {"feat/old", []StatusBlob{w, openChange(3, "old", true, "branch: feat/old\n")}, 3, "", ""}, // archived fallback + {"feat/widget", []StatusBlob{openChange(4, "widget-old", true, "branch: feat/widget\n"), w}, 7, "", ""}, // active wins over archived + {"main", []StatusBlob{w}, 0, ReasonOpenBranchUnmatched, none("main")}, + {"docket", []StatusBlob{w}, 0, ReasonOpenBranchUnmatched, none("docket")}, // metadata checkout + {"feat/fresh", []StatusBlob{openChange(9, "fresh", false, "")}, 0, ReasonOpenBranchUnmatched, none("feat/fresh")}, // no recorded branch + {"feat/widget", []StatusBlob{openChange(12, "widget-two", false, "branch: feat/widget\n"), w}, 0, + ReasonOpenBranchAmbiguous, "branch feat/widget is recorded by changes 7, 12 — pass a change id"}, + } + for i, tc := range cases { + c, f := inferChangeFromBranch(openSnapshot(t, tc.blobs...), tc.branch) + if tc.reason == "" && (f != nil || int(c.ID()) != tc.wantID) { + t.Errorf("case %d: got (%d, %+v), want change %d", i, c.ID(), f, tc.wantID) + } + if tc.reason != "" && (f == nil || f.reason != tc.reason || f.message != tc.msg || f.result != ResultInvalidInput) { + t.Errorf("case %d: failure = %+v, want %s %q", i, f, tc.reason, tc.msg) + } + } +} + +func probeReturning(st gitcli.CheckoutState, err error) func(context.Context, string) (gitcli.CheckoutState, error) { + return func(context.Context, string) (gitcli.CheckoutState, error) { return st, err } +} + +func TestCheckoutBranch(t *testing.T) { + if b, f := checkoutBranch(context.Background(), probeReturning(gitcli.CheckoutState{Branch: "refs/heads/feat/widget"}, nil), "/r"); f != nil || b != "feat/widget" { + t.Errorf("branch = (%q, %+v)", b, f) + } + if _, f := checkoutBranch(context.Background(), probeReturning(gitcli.CheckoutState{Detached: true}, nil), "/r"); f == nil || + f.reason != ReasonOpenHeadDetached || f.message != "HEAD is detached — pass a change id" { + t.Errorf("detached = %+v", f) + } +} + +// TestCheckoutBranchProbeErrorIsNotAbsence (Review Focus 5). +func TestCheckoutBranchProbeErrorIsNotAbsence(t *testing.T) { + _, f := checkoutBranch(context.Background(), probeReturning(gitcli.CheckoutState{}, errors.New("not a git repository")), "/r") + if f == nil || f.reason != ReasonOpenCheckoutUnreadable || f.result != ResultExternalFailed { + t.Fatalf("failure = %+v, want checkout-unreadable external-failed", f) + } +} diff --git a/internal/app/open_integration_test.go b/internal/app/open_integration_test.go new file mode 100644 index 000000000..0d6cc9017 --- /dev/null +++ b/internal/app/open_integration_test.go @@ -0,0 +1,120 @@ +//go:build integration + +package app + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/layout" +) + +// docket open's local opens over a real private repository (prefix +// TestIntegrationOpenArtifact): a private repository always opens from the +// metadata checkout, which Open prepares first and never blocks on. Not +// parallel: the helpers use t.Setenv. + +// openPrivateFixture is one private repository with a single groomed change: +// the invocation clone's real planning node, its resolved private layout, and +// the change id. +type openPrivateFixture struct { + node realNode + lay layout.Layout + id int +} + +// newOpenPrivateFixture runs init --private, then creates and grooms one +// change with a spec (the groom commit, the metadata tip, adds the spec). +func newOpenPrivateFixture(t *testing.T) *openPrivateFixture { + t.Helper() + r, _ := newPrivateInitRepo(t, nil) + if res := r.runInitWith(t, InitOptions{Private: true}); res.Result != ResultApplied { + t.Fatalf("init --private = %q (%s)", res.Result, res.HumanText()) + } + node := planningDepsFor(t, r.invocation) + id, _ := seedBuildReadyPrivate(t, node) + return &openPrivateFixture{node: node, lay: resolvedPrivateLayout(t, r.invocation), id: id} +} + +// open runs `docket open spec --print` through the production seams +// (real reader, checkout probe, and prepare) with a fake opener. +func (f *openPrivateFixture) open(t *testing.T) OpenResult { + t.Helper() + client := f.node.deps.Client + deps := OpenDeps{ + Reader: f.node.deps.Reader, + Checkout: client.WorktreeCheckoutState, + Prepare: func(ctx context.Context, dir string) RepositoryPrepareResult { + return RunRepositoryPrepare(ctx, SetupDeps{Git: client, RepoDir: dir}, PrepareOptions{}) + }, + Opener: &fakeOpener{}, + } + return Open(context.Background(), deps, OpenOptions{RepoDir: f.node.dir, What: "spec", ID: f.id, Print: true}) +} + +// rewind resets the checkout to the commit before the metadata tip (before +// the spec existed) and returns the tip. Both preconditions are asserted so +// neither can pass vacuously. +func (f *openPrivateFixture) rewind(t *testing.T) string { + t.Helper() + wt := f.lay.MetadataWorktree + runGit(t, wt, "fetch", "-q", f.lay.DefaultBareRemote, f.lay.MetadataBranch) + tip := runGit(t, wt, "rev-parse", "FETCH_HEAD") + if !strings.Contains(runGit(t, wt, "ls-tree", "-r", "--name-only", tip), "-design.md") { + t.Fatalf("precondition: the metadata tip %s carries no spec", tip) + } + runGit(t, wt, "reset", "-q", "--hard", tip+"~1") + if strings.Contains(runGit(t, wt, "ls-files"), "-design.md") { + t.Fatalf("precondition: the rewound checkout still has the spec") + } + return tip +} + +// dirty leaves an untracked file in the metadata checkout, which prepare +// refuses to sync over. +func (f *openPrivateFixture) dirty(t *testing.T) { + t.Helper() + if err := os.WriteFile(filepath.Join(f.lay.MetadataWorktree, "scratch.txt"), []byte("edit\n"), 0o644); err != nil { + t.Fatal(err) + } +} + +// One fixture, three phases: a stale checkout missing the file fails naming +// the path and prepare's finding; once clean, a behind checkout is +// fast-forwarded before the open; a dirty one opens with the stale note. +func TestIntegrationOpenArtifactLocalOpenSyncsFirst(t *testing.T) { + f := newOpenPrivateFixture(t) + wt := f.lay.MetadataWorktree + tip := f.rewind(t) + f.dirty(t) + res := f.open(t) + if res.Result != ResultInvalidState || res.Reason != ReasonOpenFileMissing || !strings.Contains(res.Message, wt) || + !strings.Contains(res.Message, "-design.md") || !strings.Contains(res.Message, "metadata-worktree-dirty") { + t.Fatalf("missing: (%s, %s, %q), want file-missing naming the spec path and the finding", res.Result, res.Reason, res.Message) + } + + if err := os.Remove(filepath.Join(wt, "scratch.txt")); err != nil { + t.Fatal(err) + } + res = f.open(t) + if res.Result != ResultApplied || res.TargetKind != OpenTargetKindFile || len(res.Notes) != 0 || + !strings.HasPrefix(res.Target, wt+string(filepath.Separator)) || !strings.HasSuffix(res.Target, "-design.md") { + t.Fatalf("behind: (%s, %q, %q, %q), want the spec inside %s, no note", res.Result, res.Target, res.Notes, res.Message, wt) + } + if _, err := os.Stat(res.Target); err != nil { + t.Errorf("target missing: %v", err) + } + if head := runGit(t, wt, "rev-parse", "HEAD"); head != tip { + t.Errorf("checkout HEAD = %s, want fast-forwarded to %s", head, tip) + } + + f.dirty(t) + res = f.open(t) + want := "metadata checkout not synced (metadata-worktree-dirty) — file may be stale" + if res.Result != ResultApplied || len(res.Notes) != 1 || res.Notes[0] != want { + t.Fatalf("dirty: (%s, %q, %q), want applied with %q", res.Result, res.Notes, res.Message, want) + } +} diff --git a/internal/app/open_opener.go b/internal/app/open_opener.go new file mode 100644 index 000000000..4686d8910 --- /dev/null +++ b/internal/app/open_opener.go @@ -0,0 +1,106 @@ +package app + +// open_opener.go is the seam through which `docket open` hands a resolved +// target (a URL or a local file path) to the platform's opener. Production +// wiring uses NewSystemOpener with exec.LookPath and RunOpenerProcess; tests +// inject fakes so no real application is ever launched. + +import ( + "bytes" + "context" + "errors" + "fmt" + "os/exec" + "path/filepath" + "strings" + "time" +) + +// openerWaitDelay bounds how long cmd.Wait keeps draining the opener's stderr +// pipe after the opener exits. xdg-open can hand that pipe to the long-lived +// application it launches; without the bound a pipe-holding grandchild keeps +// cmd.Run blocked until the user closes that application. +const openerWaitDelay = 2 * time.Second + +// Opener hands one target, a URL or an absolute file path, to whatever +// application the platform associates with it. +type Opener interface { + Open(ctx context.Context, target string) error +} + +// NoOpenerError reports that the platform has no supported opener, or that its +// opener command is not on PATH. The caller can still print the target. +type NoOpenerError struct { + GOOS string +} + +// Error renders the refusal with its remedy. +func (e *NoOpenerError) Error() string { + return "no opener available on " + e.GOOS + " — open it yourself, or use --print" +} + +// openerCommand names the opener command for goos: "open" on darwin, +// "xdg-open" on linux, and "" where docket supports none. +func openerCommand(goos string) string { + switch goos { + case "darwin": + return "open" + case "linux": + return "xdg-open" + default: + return "" + } +} + +// systemOpener resolves the platform opener through lookPath and launches it +// through run. +type systemOpener struct { + goos string + lookPath func(string) (string, error) + run func(ctx context.Context, bin, target string) error +} + +// NewSystemOpener returns the Opener for goos. lookPath resolves the opener +// command (exec.LookPath in production) and run launches it +// (RunOpenerProcess in production). +func NewSystemOpener(goos string, lookPath func(string) (string, error), run func(ctx context.Context, bin, target string) error) Opener { + return systemOpener{goos: goos, lookPath: lookPath, run: run} +} + +// Open launches the platform opener on target, or returns *NoOpenerError +// without launching anything when the platform has none or it is not on PATH. +func (o systemOpener) Open(ctx context.Context, target string) error { + name := openerCommand(o.goos) + if name == "" { + return &NoOpenerError{GOOS: o.goos} + } + bin, err := o.lookPath(name) + if err != nil { + return &NoOpenerError{GOOS: o.goos} + } + return o.run(ctx, bin, target) +} + +// RunOpenerProcess runs bin with target as its only argument (no shell) and +// waits for it to exit, bounding the stderr drain by openerWaitDelay. +// Stdin/stdout stay unset (the null device), so opener chatter never +// reaches docket's protocol stdout; a non-zero exit carries the opener's stderr. +func RunOpenerProcess(ctx context.Context, bin, target string) error { + cmd := exec.CommandContext(ctx, bin, target) + var stderr bytes.Buffer + cmd.Stderr = &stderr + cmd.WaitDelay = openerWaitDelay + err := cmd.Run() + // ErrWaitDelay means the opener exited 0 and only a grandchild still held + // stderr: the open itself succeeded. + if errors.Is(err, exec.ErrWaitDelay) { + return nil + } + if err != nil { + if detail := strings.Join(strings.Fields(stderr.String()), " "); detail != "" { + return fmt.Errorf("%s: %w: %s", filepath.Base(bin), err, detail) + } + return fmt.Errorf("%s: %w", filepath.Base(bin), err) + } + return nil +} diff --git a/internal/app/open_opener_test.go b/internal/app/open_opener_test.go new file mode 100644 index 000000000..23efab9cd --- /dev/null +++ b/internal/app/open_opener_test.go @@ -0,0 +1,149 @@ +package app + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + "time" + + "github.com/danielhanold/docket/internal/testsupport" +) + +func TestSystemOpenerSelectsByPlatform(t *testing.T) { + cases := []struct { + goos, lookup string + lookErr error + noOpener bool + }{ + {goos: "darwin", lookup: "open"}, + {goos: "linux", lookup: "xdg-open"}, + {goos: "linux", lookup: "xdg-open", lookErr: errors.New("not found"), noOpener: true}, + {goos: "windows", noOpener: true}, + {goos: "freebsd", noOpener: true}, + } + for _, tc := range cases { + var looked []string + var ran [][2]string + o := NewSystemOpener(tc.goos, + func(n string) (string, error) { looked = append(looked, n); return "/bin/" + n, tc.lookErr }, + func(_ context.Context, bin, target string) error { + ran = append(ran, [2]string{bin, target}) + return nil + }) + err := o.Open(context.Background(), "/tmp/a b/c.md") + if tc.noOpener { + var no *NoOpenerError + want := "no opener available on " + tc.goos + " — open it yourself, or use --print" + if !errors.As(err, &no) || err.Error() != want || len(ran) != 0 { + t.Errorf("%s: err=%v ran=%v, want %q and no launch", tc.goos, err, ran, want) + } + if tc.lookup == "" && len(looked) != 0 { + t.Errorf("%s: consulted PATH %v", tc.goos, looked) + } + continue + } + if err != nil || len(looked) != 1 || looked[0] != tc.lookup || len(ran) != 1 || ran[0] != [2]string{"/bin/" + tc.lookup, "/tmp/a b/c.md"} { + t.Errorf("%s: err=%v looked=%v ran=%v", tc.goos, err, looked, ran) + } + } +} + +// openerScript writes a stand-in opener recording "$#|$1", chattering on +// stdout and stderr, and exiting code. +func openerScript(t *testing.T, code int, errText string) (bin, argsFile string) { + t.Helper() + dir := testsupport.TempDir(t) + argsFile, bin = filepath.Join(dir, "args"), filepath.Join(dir, "fake-opener") + body := fmt.Sprintf("#!/bin/sh\nprintf '%%s|%%s' \"$#\" \"$1\" > '%s'\necho chatter\necho '%s' >&2\nexit %d\n", argsFile, errText, code) + if err := os.WriteFile(bin, []byte(body), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(bin, 0o755); err != nil { + t.Fatal(err) + } + return bin, argsFile +} + +func TestRunOpenerProcessPassesOneArgAndReportsFailure(t *testing.T) { + bin, args := openerScript(t, 0, "") + if err := RunOpenerProcess(context.Background(), bin, "/tmp/a b/c d.md"); err != nil { + t.Fatalf("RunOpenerProcess: %v", err) + } + if got, _ := os.ReadFile(args); string(got) != "1|/tmp/a b/c d.md" { + t.Errorf("opener saw %q, want one argument", got) + } + bin, _ = openerScript(t, 3, "cannot open display") + err := RunOpenerProcess(context.Background(), bin, "https://example.test/x") + if err == nil || !strings.Contains(err.Error(), "exit status 3") || !strings.Contains(err.Error(), "cannot open display") { + t.Fatalf("err = %v, want exit status and stderr", err) + } + bin, _ = openerScript(t, 4, "cannot open display\n no DISPLAY set") + err = RunOpenerProcess(context.Background(), bin, "https://example.test/x") + if err == nil || strings.ContainsAny(err.Error(), "\r\n") || !strings.Contains(err.Error(), "cannot open display no DISPLAY set") { + t.Fatalf("err = %q, want multi-line stderr collapsed onto one line", err) + } +} + +// TestRunOpenerProcessReturnsWhenGrandchildHoldsStderr models xdg-open handing +// its stderr to a long-lived application: the opener backgrounds a process +// that inherits stderr, then exits 0. RunOpenerProcess must return promptly +// with success instead of waiting for that application to exit. +func TestRunOpenerProcessReturnsWhenGrandchildHoldsStderr(t *testing.T) { + dir := testsupport.TempDir(t) + bin, pidFile := filepath.Join(dir, "fake-opener"), filepath.Join(dir, "pid") + body := fmt.Sprintf("#!/bin/sh\nsleep 30 &\necho $! > '%s'\nexit 0\n", pidFile) + if err := os.WriteFile(bin, []byte(body), 0o755); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if raw, err := os.ReadFile(pidFile); err == nil { + if pid, err := strconv.Atoi(strings.TrimSpace(string(raw))); err == nil { + if p, err := os.FindProcess(pid); err == nil { + _ = p.Kill() + } + } + } + }) + start := time.Now() + err := RunOpenerProcess(context.Background(), bin, "https://example.test/x") + elapsed := time.Since(start) + if err != nil { + t.Fatalf("RunOpenerProcess: %v, want success once the opener exits 0", err) + } + if elapsed > 15*time.Second { + t.Fatalf("RunOpenerProcess took %v, want it bounded by the pipe-drain delay", elapsed) + } +} + +// TestRunOpenerProcessKeepsOpenerStdoutOffDocketStdout guards the --json +// stream: a chattering opener must never write to docket's own stdout. It +// swaps the process-global os.Stdout for a file, so it must not run in +// parallel. +func TestRunOpenerProcessKeepsOpenerStdoutOffDocketStdout(t *testing.T) { + bin, _ := openerScript(t, 0, "") + captured, err := os.Create(filepath.Join(testsupport.TempDir(t), "stdout")) + if err != nil { + t.Fatal(err) + } + defer captured.Close() + saved := os.Stdout + os.Stdout = captured + t.Cleanup(func() { os.Stdout = saved }) + runErr := RunOpenerProcess(context.Background(), bin, "https://example.test/x") + os.Stdout = saved + if runErr != nil { + t.Fatalf("RunOpenerProcess: %v", runErr) + } + got, err := os.ReadFile(captured.Name()) + if err != nil { + t.Fatal(err) + } + if len(got) != 0 { + t.Fatalf("opener wrote %q to docket's stdout, want nothing", got) + } +} diff --git a/internal/app/open_test.go b/internal/app/open_test.go new file mode 100644 index 000000000..ddd2c6aad --- /dev/null +++ b/internal/app/open_test.go @@ -0,0 +1,400 @@ +package app + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/config" + "github.com/danielhanold/docket/internal/gitcli" + "github.com/danielhanold/docket/internal/layout" + "github.com/danielhanold/docket/internal/reposetup" + "github.com/danielhanold/docket/internal/testsupport" +) + +func TestParseOpenArgs(t *testing.T) { + ok := []struct { + args []string + what string + id int + }{ + {[]string{"board"}, "board", 0}, {[]string{"spec"}, "spec", 0}, + {[]string{"change", "541"}, "change", 541}, {[]string{"plan", "0541"}, "plan", 541}, + {[]string{"pr", "7"}, "pr", 7}, {[]string{"results", "000012"}, "results", 12}, + } + for _, tc := range ok { + if what, id, err := ParseOpenArgs(tc.args); err != nil || what != tc.what || id != tc.id { + t.Errorf("ParseOpenArgs(%q) = (%q, %d, %v)", tc.args, what, id, err) + } + } + bad := []struct { + args []string + want string // exact message; "" = must list all six targets + }{ + {nil, ""}, {[]string{"bogus"}, ""}, {[]string{"board", "5"}, ""}, {[]string{"spec", "5", "6"}, ""}, + {[]string{"spec", "#541"}, "invalid change id #541"}, {[]string{"spec", "+5"}, "invalid change id +5"}, + {[]string{"spec", "-5"}, "invalid change id -5"}, {[]string{"spec", "0"}, "invalid change id 0"}, + {[]string{"spec", "0x1F"}, "invalid change id 0x1F"}, + {[]string{"spec", "99999999999999999999999"}, "invalid change id 99999999999999999999999"}, + } + for _, tc := range bad { + _, _, err := ParseOpenArgs(tc.args) + if err == nil || strings.Contains(err.Error(), "\n") { + t.Errorf("ParseOpenArgs(%q) = %v, want a one-line usage error", tc.args, err) + continue + } + if tc.want != "" && err.Error() != tc.want { + t.Errorf("ParseOpenArgs(%q) = %q, want %q", tc.args, err, tc.want) + } + for _, w := range OpenTargets { + if tc.want == "" && !strings.Contains(err.Error(), w) { + t.Errorf("ParseOpenArgs(%q) = %q does not list %q", tc.args, err, w) + } + } + } +} + +func TestOpenResultRendering(t *testing.T) { + r := OpenResult{What: "board", Target: "/m/BOARD.md", Notes: []string{openNoteNotGitHub}} + if r.HumanText() != "/m/BOARD.md" || !reflect.DeepEqual(r.HumanNotes(), r.Notes) { + t.Errorf("success renders %q / %q", r.HumanText(), r.HumanNotes()) + } + if r.Message = "boom"; r.HumanText() != "/m/BOARD.md\nboom" { // resolved target, then the failure + t.Errorf("failure with target renders %q", r.HumanText()) + } + if r.Target = ""; r.HumanText() != "boom" { + t.Errorf("failure renders %q", r.HumanText()) + } + raw, _ := json.Marshal(OpenResult{What: "board", Notes: []string{}}) + if strings.Contains(string(raw), "change_id") || !strings.Contains(string(raw), `"notes":[]`) || !strings.Contains(string(raw), `"launched":false`) { + t.Errorf("board document = %s, want no change_id, notes [], launched false", raw) + } + full, _ := json.Marshal(OpenResult{What: "spec", ChangeID: 7, Target: "/m/spec.md", TargetKind: "file", Launched: true, Notes: []string{"n1"}}) + var doc map[string]any + if err := json.Unmarshal(full, &doc); err != nil { + t.Fatal(err) + } + for k, want := range map[string]any{"what": "spec", "change_id": float64(7), "target": "/m/spec.md", "target_kind": "file", "launched": true, "notes": []any{"n1"}} { + if !reflect.DeepEqual(doc[k], want) { + t.Errorf("success document %s = %#v, want %#v (doc %s)", k, doc[k], want, full) + } + } +} + +const ( + openWeb = "https://github.com/acme/widget" + widgetSpec = "docs/superpowers/specs/2026-01-02-widget-design.md" + widgetPlan = "docs/superpowers/plans/2026-01-02-widget.md" + widgetResults = "docs/results/2026-01-03-widget-results.md" + widgetPR = "https://github.com/acme/widget/pull/12" + oldSpec = "docs/superpowers/specs/2025-12-01-old-design.md" +) + +// 7 fully linked, 3 archived, 8 bare, 9 trivial, 10 shorthand pr. +func openCorpus() []StatusBlob { + return []StatusBlob{ + openChange(7, "widget", false, "branch: feat/widget\nspec: "+widgetSpec+"\nplan: "+widgetPlan+"\nresults: "+widgetResults+"\npr: '"+widgetPR+"'\n"), + openChange(3, "old", true, "branch: feat/old\nspec: "+oldSpec+"\npr: 'https://github.com/acme/widget/pull/2'\n"), + openChange(8, "bare", false, "branch: feat/bare\n"), + openChange(9, "tiny", false, "branch: feat/tiny\ntrivial: true\n"), + openChange(10, "short", false, "branch: feat/short\npr: 'acme/widget#12'\n"), + } +} + +func openConfig(t *testing.T, yml string) config.Snapshot { + t.Helper() + var src []config.Source + if yml != "" { + src = []config.Source{{Layer: config.LayerRepository, Name: ".docket.yml", Data: []byte(yml)}} + } + snap, _, err := config.Resolve(src, config.ResolveContext{DefaultBranch: "main"}) + if err != nil { + t.Fatalf("resolve %q: %v", yml, err) + } + return *snap +} + +type fakeOpener struct { + targets []string + err error +} + +func (o *fakeOpener) Open(_ context.Context, target string) error { + o.targets = append(o.targets, target) + return o.err +} + +type openFixture struct { + repoDir, root string + reader *fakeReader + prepare RepositoryPrepareResult + prepared, checkoutDirs []string + checkout gitcli.CheckoutState + checkoutErr error + opener *fakeOpener +} + +// newOpenFixture: a fake-reader harness whose metadata checkout (the layout's +// MetadataWorktree) is a real temp directory. +func newOpenFixture(t *testing.T, cfgYAML string, private bool, web string, blobs ...StatusBlob) *openFixture { + t.Helper() + base := testsupport.TempDir(t) + lay := layout.SharedLayout(filepath.Join(base, ".git"), base) + if private { + lay = layout.PrivateLayout(filepath.Join(base, ".git"), base, filepath.Join(base, "data"), "acme-widget") + } + pin := docketPin(t) + pin.Config, pin.Layout, pin.RepoWebURL = openConfig(t, cfgYAML), lay, web + return &openFixture{ + repoDir: base, root: lay.MetadataWorktree, reader: &fakeReader{pin: pin, corpus: blobs}, opener: &fakeOpener{}, + prepare: RepositoryPrepareResult{Envelope: NewEnvelope(OperationRepositoryPrepare, ResultNoOp), + Disposition: PrepareDispositionNoOp, Context: &PrepareContext{MetadataWorktreePath: lay.MetadataWorktree}}, + } +} + +func (f *openFixture) deps() OpenDeps { + return OpenDeps{ + Reader: f.reader, + Checkout: func(_ context.Context, dir string) (gitcli.CheckoutState, error) { + f.checkoutDirs = append(f.checkoutDirs, dir) + return f.checkout, f.checkoutErr + }, + Prepare: func(_ context.Context, dir string) RepositoryPrepareResult { + f.prepared = append(f.prepared, dir) + return f.prepare + }, + Opener: f.opener, + } +} + +func (f *openFixture) writeFile(t *testing.T, rel string) string { + t.Helper() + p := filepath.Join(f.root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte("x\n"), 0o644); err != nil { + t.Fatal(err) + } + return p +} + +func (f *openFixture) refusePrepare(code string) { + f.prepare = RepositoryPrepareResult{Envelope: NewEnvelope(OperationRepositoryPrepare, ResultInvalidState), + Disposition: PrepareDispositionRefused, Findings: []reposetup.Finding{{Code: code}}} +} + +func (f *openFixture) open(what string, id int) OpenResult { + return Open(context.Background(), f.deps(), OpenOptions{RepoDir: f.repoDir, What: what, ID: id}) +} + +// TestOpenTargetTable: {shared+github, shared+local, shared non-GitHub origin, +// private with open.artifacts: github} x {board, change active/archived, spec, +// plan, results, pr}. +func TestOpenTargetTable(t *testing.T) { + targets := []struct { + what string + id int + rel string // repo-relative path, or the PR URL + isURL bool + }{ + {"board", 0, "docs/changes/BOARD.md", false}, + {"change", 7, "docs/changes/active/0007-widget.md", false}, + {"change", 3, "docs/changes/archive/2026-01-03-0003-old.md", false}, + {"spec", 7, widgetSpec, false}, {"spec", 3, oldSpec, false}, + {"plan", 7, widgetPlan, false}, {"results", 7, widgetResults, false}, + {"pr", 7, widgetPR, true}, + } + modes := []struct { + name, cfg, web string + private, github bool + note string + }{ + {"shared github", "", openWeb, false, true, ""}, + {"shared local", "open:\n artifacts: local\n", openWeb, false, false, ""}, + {"shared non-github origin", "", "", false, false, openNoteNotGitHub}, + {"private ignores github", "open:\n artifacts: github\n", openWeb, true, false, ""}, + } + for _, m := range modes { + for _, tg := range targets { + t.Run(fmt.Sprintf("%s/%s-%d", m.name, tg.what, tg.id), func(t *testing.T) { + f := newOpenFixture(t, m.cfg, m.private, m.web, openCorpus()...) + kind, target, notes := OpenTargetKindURL, "", []string{} + switch { + case tg.isURL: + target = tg.rel + case m.github: + target = openWeb + "/blob/" + layout.SharedName + "/" + tg.rel + default: + kind, target = OpenTargetKindFile, f.writeFile(t, tg.rel) + if m.note != "" { + notes = []string{m.note} + } + } + res := f.open(tg.what, tg.id) + if res.Result != ResultApplied || res.Target != target || res.TargetKind != kind || !reflect.DeepEqual(res.Notes, notes) { + t.Fatalf("Open = (%s, %q, %s, %q, %q), want (applied, %q, %s, %q)", res.Result, res.Target, res.TargetKind, res.Notes, res.Message, target, kind, notes) + } + if !res.Launched || !reflect.DeepEqual(f.opener.targets, []string{target}) { + t.Errorf("launched=%v opener=%v", res.Launched, f.opener.targets) + } + if want := map[bool]int{true: 1, false: 0}[kind == OpenTargetKindFile]; len(f.prepared) != want { + t.Errorf("prepare ran %d times, want %d", len(f.prepared), want) + } + if (tg.what != "board" && res.ChangeID != tg.id) || len(f.checkoutDirs) != 0 { + t.Errorf("change_id %d, checkout probes %v", res.ChangeID, f.checkoutDirs) + } + }) + } + } +} + +func TestOpenErrors(t *testing.T) { + dup := append(openCorpus(), openChange(7, "widget-copy", false, "branch: feat/widget-copy\n")) + inv, st := ResultInvalidInput, ResultInvalidState + cases := []struct { + what string + id int + result Result + reason string + msg string // "" = not pinned here + }{ + {"change", 99, inv, ReasonOpenUnknownChange, "no change 99"}, + {"change", 7, st, ReasonOpenAmbiguousChange, "more than one record claims change 7 — refusing to choose"}, + {"spec", 8, st, ReasonOpenArtifactUnset, "change 8 has no spec yet"}, + {"plan", 8, st, ReasonOpenArtifactUnset, "change 8 has no plan yet"}, + {"results", 8, st, ReasonOpenArtifactUnset, "change 8 has no results yet"}, + {"pr", 8, st, ReasonOpenArtifactUnset, "change 8 has no PR yet"}, + {"spec", 9, st, ReasonOpenTrivialNoSpec, "change 9 is trivial — it has no spec"}, + {"board", 0, st, ReasonOpenBoardDisabled, "this repository has no board (board_surfaces has no inline surface)"}, + {"pr", 10, st, ReasonOpenPRNotURL, "change 10 pr: acme/widget#12 is not a URL"}, + {"bogus", 7, inv, ReasonOpenInvalidTarget, ""}, + {"board", 7, inv, ReasonOpenInvalidTarget, ""}, + } + for i, tc := range cases { + cfg, blobs := "", openCorpus() + if tc.reason == ReasonOpenBoardDisabled { + cfg = "board_surfaces: []\n" + } + if tc.reason == ReasonOpenAmbiguousChange { + blobs = dup + } + f := newOpenFixture(t, cfg, false, openWeb, blobs...) + f.writeFile(t, "docs/changes/BOARD.md") // a stale board must not rescue a disabled one + res := f.open(tc.what, tc.id) + if res.Result != tc.result || res.Reason != tc.reason || (tc.msg != "" && res.Message != tc.msg) || + strings.Contains(res.HumanText(), "\n") || len(f.opener.targets) != 0 { + t.Errorf("case %d: (%s, %s, %q) opener %v, want (%s, %s, %q)", i, res.Result, res.Reason, res.Message, f.opener.targets, tc.result, tc.reason, tc.msg) + } + } +} + +// TestOpenRefusesArtifactPathOutsideRepository (Review Focus 3). If the domain +// decoder already marks such a value malformed, make changeArtifactPath report +// a malformed field as ReasonOpenPathInvalid; do not loosen this assert. +func TestOpenRefusesArtifactPathOutsideRepository(t *testing.T) { + for _, bad := range []string{"../../etc/passwd", "/etc/passwd"} { + for _, cfg := range []string{"", "open:\n artifacts: local\n"} { + f := newOpenFixture(t, cfg, false, openWeb, openChange(11, "escape", false, "branch: feat/escape\nplan: '"+bad+"'\n")) + res := f.open("plan", 11) + if res.Result != ResultInvalidState || res.Reason != ReasonOpenPathInvalid || len(f.opener.targets) != 0 || len(f.prepared) != 0 { + t.Errorf("plan %q cfg %q: (%s, %s, %q) opener %v prepare %v", bad, cfg, res.Result, res.Reason, res.Message, f.opener.targets, f.prepared) + } + } + } +} + +func TestOpenLocalFreshness(t *testing.T) { + local := "open:\n artifacts: local\n" + + f := newOpenFixture(t, local, false, openWeb, openCorpus()...) + f.root = testsupport.TempDir(t) // prepare's own path wins over the layout's + f.prepare.Disposition, f.prepare.Context = PrepareDispositionApplied, &PrepareContext{MetadataWorktreePath: f.root} + want := f.writeFile(t, widgetPlan) + if res := f.open("plan", 7); res.Result != ResultApplied || res.Target != want || len(res.Notes) != 0 { + t.Errorf("context path: (%s, %q, %q), want %q", res.Result, res.Target, res.Notes, want) + } + + f = newOpenFixture(t, local, false, openWeb, openCorpus()...) + f.refusePrepare("metadata-worktree-dirty") + want = f.writeFile(t, widgetPlan) + res := f.open("plan", 7) + if res.Result != ResultApplied || res.Target != want || + !reflect.DeepEqual(res.Notes, []string{"metadata checkout not synced (metadata-worktree-dirty) — file may be stale"}) { + t.Errorf("refused prepare: (%s, %q, %q)", res.Result, res.Target, res.Notes) + } + + f = newOpenFixture(t, local, false, openWeb, openCorpus()...) + f.refusePrepare("metadata-worktree-dirty") + res = f.open("plan", 7) + path := filepath.Join(f.root, filepath.FromSlash(widgetPlan)) + if res.Result != ResultInvalidState || res.Reason != ReasonOpenFileMissing || !strings.Contains(res.Message, path) || + !strings.Contains(res.Message, "metadata-worktree-dirty") || len(f.opener.targets) != 0 { + t.Errorf("missing file: (%s, %s, %q)", res.Result, res.Reason, res.Message) + } + + f = newOpenFixture(t, local, false, openWeb, openCorpus()...) + if res := f.open("plan", 7); res.Reason != ReasonOpenFileMissing { + t.Errorf("missing after clean prepare: (%s, %s, %q)", res.Result, res.Reason, res.Message) + } +} + +func TestOpenPrintAndLaunchOutcomes(t *testing.T) { + f := newOpenFixture(t, "", false, openWeb, openCorpus()...) + res := Open(context.Background(), f.deps(), OpenOptions{RepoDir: f.repoDir, What: "pr", ID: 7, Print: true}) + if res.Result != ResultApplied || res.Launched || len(f.opener.targets) != 0 || res.Target != widgetPR { + t.Errorf("--print: (%s, %v, %v, %q)", res.Result, res.Launched, f.opener.targets, res.Target) + } + for _, tc := range []struct { + err error + reason string + }{{&NoOpenerError{GOOS: "plan9"}, ReasonOpenNoOpener}, {errors.New("exit status 4"), ReasonOpenOpenerFailed}} { + f := newOpenFixture(t, "", false, openWeb, openCorpus()...) + f.opener.err = tc.err + res := f.open("pr", 7) + if res.Result != ResultExternalFailed || res.Reason != tc.reason || res.Launched || !strings.HasPrefix(res.HumanText(), widgetPR+"\n") { + t.Errorf("%v: (%s, %s, %q)", tc.err, res.Result, res.Reason, res.HumanText()) + } + } +} + +// TestOpenInferenceViaOpen: no id -> the checked-out branch's change, from a +// worktree path and from a relative dir that every seam receives absolute +// (Review Focus 2); detached / probe error fail before any pin. +func TestOpenInferenceViaOpen(t *testing.T) { + for _, rel := range []bool{false, true} { + f := newOpenFixture(t, "open:\n artifacts: local\n", false, openWeb, openCorpus()...) + f.checkout = gitcli.CheckoutState{Branch: "refs/heads/feat/widget"} + want, dir := f.writeFile(t, widgetPlan), filepath.Join(f.repoDir, ".worktrees", "widget") + if rel { + dir = "." + } + res := Open(context.Background(), f.deps(), OpenOptions{RepoDir: dir, What: "plan"}) + if res.Result != ResultApplied || res.ChangeID != 7 || res.Target != want { + t.Errorf("%s: (%s, %d, %q, %q)", dir, res.Result, res.ChangeID, res.Target, res.Message) + } + if len(f.checkoutDirs) != 1 || len(f.prepared) != 1 || !filepath.IsAbs(f.checkoutDirs[0]) || !filepath.IsAbs(f.prepared[0]) { + t.Errorf("%s: probe %v prepare %v, want one absolute dir each", dir, f.checkoutDirs, f.prepared) + } + } + for _, tc := range []struct { + state gitcli.CheckoutState + err error + reason string + }{ + {gitcli.CheckoutState{Detached: true}, nil, ReasonOpenHeadDetached}, + {gitcli.CheckoutState{}, errors.New("not a git repository"), ReasonOpenCheckoutUnreadable}, + } { + f := newOpenFixture(t, "", false, openWeb, openCorpus()...) + f.checkout, f.checkoutErr = tc.state, tc.err + if res := f.open("plan", 0); res.Reason != tc.reason || f.reader.pinCount != 0 { + t.Errorf("reason %s pins %d, want %s and no pin", res.Reason, f.reader.pinCount, tc.reason) + } + } +} diff --git a/internal/app/schema_registry.go b/internal/app/schema_registry.go index f591ca0a7..ec2062b53 100644 --- a/internal/app/schema_registry.go +++ b/internal/app/schema_registry.go @@ -101,6 +101,7 @@ var operationBindings = []OperationBinding{ {ID: "learning.update", Request: LearningUpdateRequest{}, Result: LearningResult{}}, // LearningUpdate {ID: "maintenance.preflight", Request: nil, Result: MaintenancePreflightResult{}}, // MaintenancePreflight {ID: "maintenance.sweep", Request: nil, Result: MaintenanceResult{}}, // MaintenanceSweep + {ID: "open", Request: nil, Result: OpenResult{}}, // Open {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 diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index 71afca4d9..a5dccb772 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:05fb9b2fdf4bbd2c88734f81efa7fc7bdd1a7050c2bbb6e55e35733a53137f3c", + "asset_set_id": "sha256:d2437c7b1af99e29a452da850d3f479ff2e6aab9fd6f27b7d1e254133410d30d", "entries": [ { "path": ".docket.example.yml", "role": "config-schema", "mode": 420, - "size": 20596, - "sha256": "3937c05a796905b7dc5f2028811f661744b3ab671036ebfcb2acd2da0620ff91" + "size": 21305, + "sha256": "ff6d14059cd34e7bd5a32a4189af95c40eebd31d668353c0000ec7e663426d57" }, { "path": "agents/docket-adr.md", diff --git a/internal/assets/embedded/tree/.docket.example.yml b/internal/assets/embedded/tree/.docket.example.yml index 0a92a7ed7..c7ecd0496 100644 --- a/internal/assets/embedded/tree/.docket.example.yml +++ b/internal/assets/embedded/tree/.docket.example.yml @@ -174,6 +174,18 @@ leak_check: match_word: true +# ═══ open — where docket open takes you ═══════════════════════════════════════════════════ + +open: + # artifacts — github (default): in a shared repository, `docket open` shows the board, a + # change, and its spec, plan, and results as GitHub pages in your browser. local: open the + # file from your metadata checkout in your system's default app for Markdown instead — + # handy offline, or when you would rather read in an editor. A private repository always + # opens the local file, and `docket open pr` always opens the pull request on GitHub. + # scope: any layer + artifacts: github + + # ═══ learnings — the learnings ledger ═════════════════════════════════════════════════════ learnings: diff --git a/internal/cli/capability_production_test.go b/internal/cli/capability_production_test.go index 295b3f140..fe6d83d0f 100644 --- a/internal/cli/capability_production_test.go +++ b/internal/cli/capability_production_test.go @@ -345,6 +345,8 @@ func TestRepresentativeSignatures(t *testing.T) { "finalize.block": "--attempt --head --id --input --pr-number --reason --revision [--repo-dir ]", "finalize.merge": "--head --id --revision [--admin] [--repo-dir ]", "workspace.prepare": "--id --revision [--repo-dir ]", + // positional target and optional id lead, the optional flags trail sorted. + "open": " [] [--print] [--repo-dir ]", } for id, wantSig := range want { e, ok := entryByID(entries, id) @@ -361,6 +363,11 @@ func TestRepresentativeSignatures(t *testing.T) { if e, ok := entryByID(entries, "run.start"); !ok || strings.Join(e.Effects, " ") != "local-write process-control" { t.Errorf("run.start effects = %v, want [local-write process-control]", e.Effects) } + // open reads the corpus, prepares the metadata checkout, and launches the + // system opener. + if e, ok := entryByID(entries, "open"); !ok || strings.Join(e.Effects, " ") != "local-write process-control read" { + t.Errorf("open effects = %v, want [local-write process-control read]", e.Effects) + } // change 0472: no catalog operation's signature carries the retired // record-revision flags (the `version` op itself takes no flags). for _, e := range entries { diff --git a/internal/cli/install.go b/internal/cli/install.go index c4d8cf880..1ac455cb3 100644 --- a/internal/cli/install.go +++ b/internal/cli/install.go @@ -66,6 +66,7 @@ var assetIndependent = map[string]bool{ "evidence verify": true, "evidence recertify": true, // composes the local gate + GitHub PR edit; reads no installed assets + "open": true, // reads the corpus and launches the system opener; 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 diff --git a/internal/cli/open.go b/internal/cli/open.go new file mode 100644 index 000000000..b359e8246 --- /dev/null +++ b/internal/cli/open.go @@ -0,0 +1,74 @@ +package cli + +import ( + "context" + "os/exec" + "runtime" + + "github.com/spf13/cobra" + + "github.com/danielhanold/docket/internal/app" + "github.com/danielhanold/docket/internal/gitcli" +) + +var ( + // openRunner runs the open operation; tests replace it to observe the + // options the command parsed without resolving or launching anything. + openRunner = func(ctx context.Context, d app.OpenDeps, o app.OpenOptions) app.OperationResult { + return app.Open(ctx, d, o) + } + // openDepsFor wires the open operation to real git, the repository + // prepare operation, and the platform's system opener. Tests replace it. + openDepsFor = func() (app.OpenDeps, error) { + client, err := gitcli.NewClient() + if err != nil { + return app.OpenDeps{}, err + } + return app.OpenDeps{ + Reader: app.NewGitStatusReader(client), + Checkout: client.WorktreeCheckoutState, + Prepare: func(ctx context.Context, dir string) app.RepositoryPrepareResult { + return app.RunRepositoryPrepare(ctx, app.SetupDeps{Git: client, RepoDir: dir}, app.PrepareOptions{}) + }, + Opener: app.NewSystemOpener(runtime.GOOS, exec.LookPath, app.RunOpenerProcess), + }, nil + } +) + +// newOpenCommand builds `docket open []`: a thin adapter that parses +// the target and change id, then hands them to app.Open. setResult receives +// the operation's result document for the root to present. +func newOpenCommand(setResult func(app.OperationResult)) *cobra.Command { + cmd := &cobra.Command{ + Use: "open []", + Short: "Open the board, or a change's record, spec, plan, results, or pull request", + Long: `Without an id, opens the change whose recorded branch is checked out. Shared +repositories open GitHub pages unless open.artifacts is local; private ones open +the metadata checkout's file; pr opens the pull request.`, + Args: func(_ *cobra.Command, args []string) error { + _, _, err := app.ParseOpenArgs(args) + return err + }, + Annotations: capability("open", EffectRead, EffectLocalWrite, EffectProcessControl), + RunE: func(c *cobra.Command, args []string) error { + what, id, err := app.ParseOpenArgs(args) + if err != nil { + return err + } + repoDir, err := resolveRepoDir(c) + if err != nil { + return err + } + printOnly, _ := c.Flags().GetBool("print") + deps, err := openDepsFor() + if err != nil { + return err + } + setResult(openRunner(c.Context(), deps, app.OpenOptions{RepoDir: repoDir, What: what, ID: id, Print: printOnly})) + return nil + }, + } + cmd.Flags().Bool("print", false, "print the URL or path without opening it") + cmd.Flags().String("repo-dir", "", "repository `dir` to operate on (default: current directory)") + return cmd +} diff --git a/internal/cli/open_test.go b/internal/cli/open_test.go new file mode 100644 index 000000000..ede96b8cd --- /dev/null +++ b/internal/cli/open_test.go @@ -0,0 +1,78 @@ +package cli + +import ( + "context" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/app" +) + +// stubOpen replaces the open operation and its dependency wiring for one test, +// recording every OpenOptions the command passes and returning res. +func stubOpen(t *testing.T, res app.OpenResult) *[]app.OpenOptions { + t.Helper() + var got []app.OpenOptions + prevRunner, prevDeps := openRunner, openDepsFor + t.Cleanup(func() { openRunner, openDepsFor = prevRunner, prevDeps }) + openDepsFor = func() (app.OpenDeps, error) { return app.OpenDeps{}, nil } + openRunner = func(_ context.Context, _ app.OpenDeps, o app.OpenOptions) app.OperationResult { + got = append(got, o) + return res + } + return &got +} + +// openOK is a successful spec open of target carrying notes. +func openOK(target string, notes ...string) app.OpenResult { + if notes == nil { + notes = []string{} + } + return app.OpenResult{Envelope: app.NewEnvelope(app.OperationOpen, app.ResultApplied), What: "spec", ChangeID: 541, + Target: target, TargetKind: app.OpenTargetKindURL, Notes: notes} +} + +func TestOpenCLIPassesParsedOptions(t *testing.T) { + got := stubOpen(t, openOK("https://example.test/s")) + if _, errS, code := runCLI(t, "open", "spec", "0541", "--print", "--repo-dir", "/tmp/repo"); code != 0 { + t.Fatalf("code %d stderr %q", code, errS) + } + if _, _, code := runCLI(t, "open", "plan", "--repo-dir", "/tmp/repo"); code != 0 { + t.Fatalf("inference code %d", code) + } + want := []app.OpenOptions{{RepoDir: "/tmp/repo", What: "spec", ID: 541, Print: true}, {RepoDir: "/tmp/repo", What: "plan"}} + if len(*got) != 2 || (*got)[0] != want[0] || (*got)[1] != want[1] { + t.Fatalf("options = %+v, want %+v", *got, want) + } +} + +// TestOpenCLIPrintNotesGoToStderr pins that `--print` output composes in +// command substitution: stdout is exactly the target, notes go to stderr. +func TestOpenCLIPrintNotesGoToStderr(t *testing.T) { + stubOpen(t, openOK("/m/docs/spec.md", "origin is not a GitHub remote — opened the local file instead")) + out, errS, code := runCLI(t, "open", "spec", "7", "--print", "--repo-dir", "/tmp/repo") + if code != 0 || out != "/m/docs/spec.md\n" || errS != "note: origin is not a GitHub remote — opened the local file instead\n" { + t.Fatalf("code %d stdout %q stderr %q", code, out, errS) + } +} + +func TestOpenCLIUsageErrors(t *testing.T) { + got := stubOpen(t, openOK("x")) + for _, args := range [][]string{{"open"}, {"open", "bogus"}, {"open", "board", "5"}, {"open", "spec", "#541"}, {"open", "spec", "0"}, {"open", "spec", "1", "2"}} { + if _, errS, code := runCLI(t, args...); code != 2 || strings.Contains(strings.TrimRight(errS, "\n"), "\n") { + t.Errorf("%q: code %d stderr %q, want exit 2 and one line", args, code, errS) + } + } + _, errS, _ := runCLI(t, "open", "bogus") + for _, w := range app.OpenTargets { + if !strings.Contains(errS, w) { + t.Errorf("unknown-target error %q does not list %q", errS, w) + } + } + if _, errS, _ := runCLI(t, "open", "spec", "#541"); !strings.Contains(errS, "invalid change id #541") { + t.Errorf("stderr = %q", errS) + } + if len(*got) != 0 { + t.Errorf("usage errors reached the operation: %+v", *got) + } +} diff --git a/internal/cli/presenter.go b/internal/cli/presenter.go index 6f8feb1ea..477593867 100644 --- a/internal/cli/presenter.go +++ b/internal/cli/presenter.go @@ -33,6 +33,13 @@ func (p Presenter) Present(r app.OperationResult) int { return app.ExitCode(r.Env().Result) } fmt.Fprintln(p.Stdout, r.HumanText()) + // Notes go to stderr in human mode so stdout stays exactly the text line + // (`docket open --print` inside command substitution). JSON carries them. + if n, ok := r.(interface{ HumanNotes() []string }); ok { + for _, note := range n.HumanNotes() { + fmt.Fprintf(p.Stderr, "note: %s\n", note) + } + } return app.ExitCode(r.Env().Result) } diff --git a/internal/cli/presenter_test.go b/internal/cli/presenter_test.go index 96056c736..3c48a83b0 100644 --- a/internal/cli/presenter_test.go +++ b/internal/cli/presenter_test.go @@ -69,3 +69,26 @@ func TestPresentHumanErrorGoesToStderrOnly(t *testing.T) { t.Fatalf("stderr = %q", errBuf.String()) } } + +// notedResult is a result that carries human notes, the way `docket open` does. +type notedResult struct{ app.CLIErrorResult } + +func (notedResult) HumanText() string { return "https://example.test/x" } +func (notedResult) HumanNotes() []string { return []string{"first", "second"} } + +// TestPresentHumanNotesGoToStderr pins that human-mode notes go to stderr, one +// "note:" line each, so stdout stays exactly the text line; JSON mode writes +// nothing to stderr because the document carries the notes. +func TestPresentHumanNotesGoToStderr(t *testing.T) { + r := notedResult{app.CLIErrorResult{Envelope: app.NewEnvelope("open", app.ResultApplied)}} + var out, errBuf bytes.Buffer + Presenter{Stdout: &out, Stderr: &errBuf}.Present(r) + if out.String() != "https://example.test/x\n" || errBuf.String() != "note: first\nnote: second\n" { + t.Errorf("stdout %q stderr %q", out.String(), errBuf.String()) + } + out.Reset() + errBuf.Reset() + if (Presenter{Stdout: &out, Stderr: &errBuf, JSON: true}).Present(r); errBuf.Len() != 0 { + t.Errorf("JSON mode wrote stderr %q", errBuf.String()) + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index d292f59dc..b83dcaaa3 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -418,6 +418,7 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer, info buildinf gateCmd := newGateCommand(func(r app.OperationResult) { result = r }) finalizeCmd := newFinalizeCommand(func(r app.OperationResult) { result = r }) maintenanceCmd := newMaintenanceCommand(func(r app.OperationResult) { result = r }) + openCmd := newOpenCommand(func(r app.OperationResult) { result = r }) repositoryCmd := newRepositoryCommand(func(r app.OperationResult) { result = r }) agentCmd := newAgentCommand(info, func(r app.OperationResult) { result = r }) instructionsCmd := newInstructionsCommand(stdin, jsonMode, @@ -429,7 +430,7 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer, info buildinf installCmd.AddCommand(installCheckCmd, installCollectCmd) developmentCmd.AddCommand(developmentInstallCmd, developmentTestCmd) diagnosticCmd.AddCommand(runtimeCmd, configCmd) - root.AddCommand(capabilitiesCmd, schemaCmd, versionCmd, statusCmd, changeCmd, contextCmd, workspaceCmd, evidenceCmd, prCmd, runCmd, learningCmd, adrCmd, gateCmd, finalizeCmd, maintenanceCmd, repositoryCmd, agentCmd, instructionsCmd, diagnosticCmd, installCmd, uninstallCmd, developmentCmd) + root.AddCommand(capabilitiesCmd, schemaCmd, versionCmd, statusCmd, changeCmd, contextCmd, workspaceCmd, evidenceCmd, prCmd, runCmd, learningCmd, adrCmd, gateCmd, finalizeCmd, maintenanceCmd, openCmd, repositoryCmd, agentCmd, instructionsCmd, diagnosticCmd, installCmd, uninstallCmd, developmentCmd) root.AddCommand(extra...) // The asset-dependence guard. Commands that do not read installed assets diff --git a/internal/config/config.go b/internal/config/config.go index 200cc1154..a6efbefda 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -130,6 +130,7 @@ type Effective struct { Learnings Learnings `json:"learnings"` Reclaim Reclaim `json:"reclaim"` LeakCheck LeakCheck `json:"leak_check"` + Open Open `json:"open"` Review Review `json:"review"` GateObservation Value[int] `json:"gate_observation_budget"` // minutes BoardSurfaces Value[[]string] `json:"board_surfaces"` @@ -192,6 +193,12 @@ type LeakCheck struct { MatchWord Value[bool] `json:"match_word"` } +// Open is the `docket open` policy: where a shared repository's board and +// change artifacts open. +type Open struct { + Artifacts Value[string] `json:"artifacts"` // github|local +} + type Review struct { MinFixSeverity Value[string] `json:"min_fix_severity"` // minor|important|blocker MaxFixTasks Value[int] `json:"max_fix_tasks"` diff --git a/internal/config/decode_test.go b/internal/config/decode_test.go index fcb10f4a3..c3b17f398 100644 --- a/internal/config/decode_test.go +++ b/internal/config/decode_test.go @@ -91,6 +91,8 @@ func decodeAcceptanceCases() []decodeCase { block: "reclaim:\n auto: true\n", flow: "reclaim: {auto: true}\n", value: true}, {row: "leak_check.match_word", path: "leak_check.match_word", block: "leak_check:\n match_word: false\n", flow: "leak_check: {match_word: false}\n", value: false}, + {row: "open.artifacts", path: "open.artifacts", + block: "open:\n artifacts: local\n", flow: "open: {artifacts: local}\n", value: "local"}, {row: "build.checkpoint", path: "build.checkpoint", block: "build:\n checkpoint: true\n", flow: "build: {checkpoint: true}\n", value: true}, @@ -323,6 +325,7 @@ func TestDecodeRejections(t *testing.T) { {"bool quoted string", "auto_groom: \"true\"\n", CodeInvalidType, "auto_groom", SeverityError}, {"negative int", "learnings:\n cap: -1\n", CodeInvalidValue, "learnings.cap", SeverityError}, {"bad enum", "finalize:\n gate: sometimes\n", CodeInvalidValue, "finalize.gate", SeverityError}, + {"bad open.artifacts enum", "open:\n artifacts: browser\n", CodeInvalidValue, "open.artifacts", SeverityError}, {"metadata branch obsolete every layer", "metadata_branch: trunk\n", CodeObsoleteSetting, "metadata_branch", SeverityWarning}, {"absolute dir", "changes_dir: /etc/changes\n", CodeInvalidValue, "changes_dir", SeverityError}, {"unclean dir", "changes_dir: docs/../etc\n", CodeInvalidValue, "changes_dir", SeverityError}, diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 4764fff84..6a5b2d0b3 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -50,6 +50,7 @@ func builtinEffective() Effective { Auto: builtinValue(false), }, LeakCheck: LeakCheck{MatchWord: builtinValue(true)}, + Open: Open{Artifacts: builtinValue("github")}, Review: Review{ MinFixSeverity: builtinValue("minor"), MaxFixTasks: builtinValue(10), diff --git a/internal/config/defaults_test.go b/internal/config/defaults_test.go index dbbc95040..a9f3a6f7e 100644 --- a/internal/config/defaults_test.go +++ b/internal/config/defaults_test.go @@ -158,6 +158,7 @@ func TestBuiltinEffectiveMatchesRegistryDefaults(t *testing.T) { "reclaim.lease_ttl": eff.Reclaim.LeaseTTL.Value, "reclaim.auto": eff.Reclaim.Auto.Value, "leak_check.match_word": eff.LeakCheck.MatchWord.Value, + "open.artifacts": eff.Open.Artifacts.Value, "review.min_fix_severity": eff.Review.MinFixSeverity.Value, "review.max_fix_tasks": eff.Review.MaxFixTasks.Value, "gate_observation_budget": eff.GateObservation.Value, diff --git a/internal/config/fixtures_test.go b/internal/config/fixtures_test.go index 585c7e428..5dd893e1b 100644 --- a/internal/config/fixtures_test.go +++ b/internal/config/fixtures_test.go @@ -184,6 +184,7 @@ func TestFixtureSparseDefaults(t *testing.T) { {"reclaim.lease_ttl", eff.Reclaim.LeaseTTL.Explicit, eff.Reclaim.LeaseTTL.Provenance.Layer}, {"reclaim.auto", eff.Reclaim.Auto.Explicit, eff.Reclaim.Auto.Provenance.Layer}, {"leak_check.match_word", eff.LeakCheck.MatchWord.Explicit, eff.LeakCheck.MatchWord.Provenance.Layer}, + {"open.artifacts", eff.Open.Artifacts.Explicit, eff.Open.Artifacts.Provenance.Layer}, {"review.min_fix_severity", eff.Review.MinFixSeverity.Explicit, eff.Review.MinFixSeverity.Provenance.Layer}, {"review.max_fix_tasks", eff.Review.MaxFixTasks.Explicit, eff.Review.MaxFixTasks.Provenance.Layer}, {"gate_observation_budget", eff.GateObservation.Explicit, eff.GateObservation.Provenance.Layer}, diff --git a/internal/config/resolve.go b/internal/config/resolve.go index 076fe7cfc..d295d7d2e 100644 --- a/internal/config/resolve.go +++ b/internal/config/resolve.go @@ -318,6 +318,7 @@ func (r *resolution) assemble(byLayer map[LayerKind]map[string]leafDecl) (Effect set(assign(&eff.Reclaim.LeaseTTL, r.declared, "reclaim.lease_ttl")) set(assign(&eff.Reclaim.Auto, r.declared, "reclaim.auto")) set(assign(&eff.LeakCheck.MatchWord, r.declared, "leak_check.match_word")) + set(assign(&eff.Open.Artifacts, r.declared, "open.artifacts")) set(assign(&eff.Review.MinFixSeverity, r.declared, "review.min_fix_severity")) set(assign(&eff.Review.MaxFixTasks, r.declared, "review.max_fix_tasks")) set(assign(&eff.GateObservation, r.declared, "gate_observation_budget")) diff --git a/internal/config/resolve_test.go b/internal/config/resolve_test.go index 4bdc71594..417448867 100644 --- a/internal/config/resolve_test.go +++ b/internal/config/resolve_test.go @@ -92,6 +92,8 @@ func effectiveLeaf(t *testing.T, eff Effective, path string) (any, Provenance, b return eff.Reclaim.Auto.Value, eff.Reclaim.Auto.Provenance, eff.Reclaim.Auto.Explicit case "leak_check.match_word": return eff.LeakCheck.MatchWord.Value, eff.LeakCheck.MatchWord.Provenance, eff.LeakCheck.MatchWord.Explicit + case "open.artifacts": + return eff.Open.Artifacts.Value, eff.Open.Artifacts.Provenance, eff.Open.Artifacts.Explicit case "review.min_fix_severity": return eff.Review.MinFixSeverity.Value, eff.Review.MinFixSeverity.Provenance, eff.Review.MinFixSeverity.Explicit case "review.max_fix_tasks": @@ -1301,3 +1303,28 @@ func TestWarningsFilter(t *testing.T) { t.Fatalf("Warnings = %v, want only the warning-severity diagnostic", out) } } + +// TestOpenArtifactsIsAnOrdinaryLayeredKey pins open.artifacts as an ordinary +// layered key: a non-explicit built-in "github" default, and the +// highest-precedence layer that declares it wins. +func TestOpenArtifactsIsAnOrdinaryLayeredKey(t *testing.T) { + if v := mustResolve(t, nil, mainCtx).effective.Open.Artifacts; v.Value != "github" || v.Provenance.Layer != LayerBuiltIn || v.Explicit { + t.Fatalf("default = %+v, want non-explicit built-in github", v) + } + g, r, l := srcG("open:\n artifacts: local\n"), srcR("open:\n artifacts: github\n"), srcL("open:\n artifacts: local\n") + for _, tc := range []struct { + sources []Source + want string + layer LayerKind + }{{[]Source{g}, "local", LayerGlobal}, {[]Source{g, r}, "github", LayerRepository}, {[]Source{r, l}, "local", LayerRepositoryLocal}} { + res := mustResolve(t, tc.sources, mainCtx) + if v := res.effective.Open.Artifacts; v.Value != tc.want || v.Provenance.Layer != tc.layer || !v.Explicit { + t.Errorf("open.artifacts = %+v, want explicit %q from %q", v, tc.want, tc.layer) + } + for _, d := range res.diags { + if d.Severity == SeverityWarning || d.Severity == SeverityError { + t.Errorf("unexpected diagnostic %s/%s/%s", d.Severity, d.Code, d.Path) + } + } + } +} diff --git a/internal/config/schema.go b/internal/config/schema.go index 565f522e9..2373de79e 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -307,6 +307,12 @@ func buildRegistry() []pathSpec { {path: "leak_check.match_word", kind: kindBool, def: true, merge: mergeScalar, scope: scopeAny, disp: dispSupported, validate: boolLeaf()}, + // open.artifacts: where `docket open` sends a shared repository's + // artifacts. A private repository always opens locally. + {path: "open.artifacts", kind: kindString, enum: []string{"github", "local"}, def: "github", + merge: mergeScalar, scope: scopeAny, disp: dispSupported, + validate: enumLeaf("github", "local")}, + // 15: build. The build role's own gate policy (change 0374): build.gate // and build.test_command resolve independently of the finalize pair. {path: "build.checkpoint", kind: kindBool, def: false, diff --git a/internal/config/schema_test.go b/internal/config/schema_test.go index 0a27c380b..a67624320 100644 --- a/internal/config/schema_test.go +++ b/internal/config/schema_test.go @@ -23,6 +23,7 @@ func TestRegistryPathSetMatchesV092(t *testing.T) { "learnings.enabled", "learnings.cap", "reclaim.lease_ttl", "reclaim.auto", "leak_check.match_word", + "open.artifacts", "build.checkpoint", "build.gate", "build.test_command", "build.max_attempts", "run.max_attempts", "review.min_fix_severity", "review.max_fix_tasks", @@ -124,6 +125,7 @@ func TestRegistryDefaults(t *testing.T) { "reclaim.lease_ttl": 72, "reclaim.auto": false, "leak_check.match_word": true, + "open.artifacts": "github", "build.checkpoint": false, "build.gate": "local", "build.test_command": "", @@ -180,6 +182,7 @@ func TestRegistryEnumRows(t *testing.T) { "visibility": {"shared", "private"}, "finalize.gate": {"local", "ci", "both", "off"}, "build.gate": {"local", "off"}, + "open.artifacts": {"github", "local"}, "review.min_fix_severity": {"minor", "important", "blocker"}, "runners.codex.sandbox": {"workspace-write", "danger-full-access"}, "runners.opencode.permissions": {"ask", "auto-approve"}, diff --git a/internal/config/setting_paths_test.go b/internal/config/setting_paths_test.go index 8eb7a81f9..1251a9f91 100644 --- a/internal/config/setting_paths_test.go +++ b/internal/config/setting_paths_test.go @@ -48,6 +48,7 @@ func TestSettingPathsSupportSplit(t *testing.T) { "review.min_fix_severity", "review.max_fix_tasks", "reclaim.lease_ttl", "reclaim.auto", "leak_check.match_word", + "open.artifacts", "learnings.enabled", "gate_observation_budget", "board_surfaces", "board.section_order", diff --git a/tests/runtime-budgets.tsv b/tests/runtime-budgets.tsv index d8c7076b4..f48c4ed2a 100644 --- a/tests/runtime-budgets.tsv +++ b/tests/runtime-budgets.tsv @@ -59,6 +59,7 @@ tests/test_go_integration_app_runverdict.sh 20 parallel tests/test_go_integration_app_runfence.sh 20 parallel tests/test_go_integration_app_runcompletion.sh 15 parallel tests/test_go_integration_app_runrecord.sh 10 parallel +tests/test_go_integration_app_open.sh 15 parallel tests/test_go_integration_app_runstart.sh 15 parallel tests/test_go_integration_app_gatelifecycle.sh 10 parallel tests/test_go_integration_app_finalizeops.sh 30 parallel diff --git a/tests/test_go_integration_app_open.sh b/tests/test_go_integration_app_open.sh new file mode 100755 index 000000000..904fce20a --- /dev/null +++ b/tests/test_go_integration_app_open.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# docket-suite: go +# tests/test_go_integration_app_open.sh — Go integration shard: docket open's local opens over a +# real private repository (the metadata checkout is prepared first, a behind checkout is +# fast-forwarded, a dirty one opens with a staleness note) — real-git tests kept out of the +# default internal/app corpus, which must never start real git (the no-real-git guard in +# internal/app/nogit_guard_test.go) — behind the `integration` build tag, prefix +# ^TestIntegrationOpenArtifact. Declarations only — execution and inspection live in +# tests/lib/go-integration-shard.sh; the completeness contract is +# tests/test_go_integration_contract.sh. +set -uo pipefail +REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P)" +cd "$REPO" || exit 1 +fail=0 +assert(){ if eval "$2"; then printf 'ok - %s\n' "$1"; else printf 'NOT OK - %s\n' "$1"; fail=1; fi; } + +SHARD_PKG="./internal/app" +SHARD_PREFIX="TestIntegrationOpenArtifact" +SHARD_MODE="normal" + +. "$REPO/tests/lib/go-integration-shard.sh" +shard_inspect_maybe +run_integration_shard +exit "$fail"