diff --git a/docs/install/claude-code.md b/docs/install/claude-code.md index c774a53ec..2c2276e12 100644 --- a/docs/install/claude-code.md +++ b/docs/install/claude-code.md @@ -10,6 +10,13 @@ calling out, after a word on what the install writes. 17 agent wrappers are written to `~/.claude/agents/docket-*.md`, each carrying the model and effort resolved from your global config over the built-in table. Nothing is written per repository for these. +- **Two `SessionStart` hooks in `~/.claude/settings.json`.** These content-free hooks run + `dckt instructions --hook claude --section dispatch` and + `dckt instructions --hook claude --section lessons`, which print nothing outside a private + repository, so they are inert everywhere else. A `settings.json` docket cannot edit in place (a + symlink, or a file docket cannot parse, such as one with comments) is left untouched, and the + run warns about it. Uninstall removes the hooks + only while they are unchanged. - **The dispatch block in `CLAUDE.md`, on opt-in only.** When a repository's `.docket.yml` or `.docket.local.yml` lists `claude` in `agent_harnesses`, the install reconciles a marker-bounded `docket` dispatch block in that repository's `CLAUDE.md`. When Codex or opencode is diff --git a/docs/install/codex.md b/docs/install/codex.md index 34d330b66..90100573d 100644 --- a/docs/install/codex.md +++ b/docs/install/codex.md @@ -1,6 +1,6 @@ # Codex: running docket under Codex -Codex is a first-class docket harness. An install writes two kinds of Codex artifact: +Codex is a first-class docket harness. An install writes three kinds of Codex artifact: - **User-level skills and agent wrappers.** docket's skills are linked under the harness-neutral `~/.agents/skills/` root, which Codex reads, and its 17 agent wrappers are written to @@ -13,6 +13,11 @@ Codex is a first-class docket harness. An install writes two kinds of Codex arti `.toml` agent (Codex reads `AGENTS.md`; it has no analog of Cursor's `.mdc` rule). The block is **committed and machine-neutral**: it carries only agent names and routing instructions, never a model ID or effort value, so it is identical in every clone (ADR-0036). +- **A `dckt:private-instructions` pointer block in `~/.codex/AGENTS.md`.** This user-level, + content-free block asks Codex to run `dckt instructions` once at the start of a session and to + follow whatever it prints as the repository's own `AGENTS.md`. Outside a private repository the + command prints nothing, so the block is inert. It needs no opt-in, and uninstall removes it only + while it is unchanged. ### The opt-in you need diff --git a/docs/install/cursor.md b/docs/install/cursor.md index 342caf7cb..944639157 100644 --- a/docs/install/cursor.md +++ b/docs/install/cursor.md @@ -10,6 +10,11 @@ small, stable permission configuration, because docket must run **outside** Curs `description`, and a `model:` line wherever a model resolves, with any reasoning effort encoded inside the model value. Agent overrides come from your global config only. Nothing is written per repository for these. +- **A `sessionStart` hook in `~/.cursor/hooks.json`.** This content-free hook runs + `dckt instructions --hook cursor`, which prints nothing outside a private repository, so it is + inert everywhere else. A `hooks.json` docket cannot edit in place (a symlink, or a file docket + cannot parse, such as one with comments) is left untouched, and the run warns about it. Uninstall removes the hook only while it is + unchanged. - **The dispatch rule, on opt-in only.** When a repository's `.docket.yml` or `.docket.local.yml` lists `cursor` in `agent_harnesses`, the install writes `.cursor/rules/docket-dispatch.mdc` in that repository. It tells Cursor to dispatch a docket workflow to its matching agent instead of running diff --git a/docs/install/install.md b/docs/install/install.md index a19b2cf98..3c1e0c76a 100644 --- a/docs/install/install.md +++ b/docs/install/install.md @@ -46,6 +46,16 @@ harness, after editing a config file, and after every version update). A single global agent wrappers** — the model/effort-pinned subagent copies, resolved from your global config over docket's built-in defaults. Wrappers are user-level only; no repository carries its own copies. +- **Installs one content-free session trigger per harness**, so a private repository's agent + instructions load without any file in that repository: two Claude Code `SessionStart` hooks in + `~/.claude/settings.json` (`dckt instructions --hook claude --section dispatch` and + `dckt instructions --hook claude --section lessons`), a Cursor `sessionStart` hook in + `~/.cursor/hooks.json` (`dckt instructions --hook cursor`), the OpenCode plugin + `~/.config/opencode/plugins/dckt-instructions.js`, and a `dckt:private-instructions` pointer + block in `~/.codex/AGENTS.md`. Each one only runs `dckt instructions`, which prints nothing + outside a private repository, so a trigger is inert everywhere else. A hooks file docket cannot + edit in place (a symlink, or a file docket cannot parse, such as one with comments) is left + untouched, and the run warns about it. - **Retires the old global parent-facing dispatch blocks** that earlier docket versions wrote into your personal `~/.claude/CLAUDE.md` and the other harnesses' global instruction files, while keeping the global skills and agent wrappers. Removal is **proof-gated** — the engine deletes a @@ -75,7 +85,9 @@ Claude-Code-only user can stop here. To pin a model or change a default on this ## Uninstalling docket `docket uninstall` removes the harness integrations docket recorded for you — the -global `skills/` symlinks and `agents/` wrappers it wrote — and nothing else. With +global `skills/` symlinks, the `agents/` wrappers, and the session triggers it wrote +(the hook entries, the OpenCode plugin, and the Codex pointer block, each removed only +while it is unchanged) — and nothing else. With no flag it removes **every recorded harness**; a repeatable **`--harness `** limits the run to the harness(es) you name, and duplicate `--harness` values are de-duplicated before anything is touched. **`--dry-run`** reports exactly what a diff --git a/docs/install/opencode.md b/docs/install/opencode.md index 908a066a1..93962790b 100644 --- a/docs/install/opencode.md +++ b/docs/install/opencode.md @@ -1,6 +1,6 @@ # opencode: running docket under opencode -opencode is a first-class docket harness. An install writes two kinds of opencode artifact: +opencode is a first-class docket harness. An install writes three kinds of opencode artifact: - **User-level skills and agent definitions**, under `${XDG_CONFIG_HOME:-~/.config}/opencode/`: docket's skills are linked under `skills/`, and its 17 @@ -13,6 +13,10 @@ opencode is a first-class docket harness. An install writes two kinds of opencod block, **shared with Codex**: opencode reads the same committed project-root `AGENTS.md`, so one managed block serves both. A repo targeting either harness gets it; a repo targeting both gets it exactly once. It is **committed and machine-neutral** (ADR-0036). +- **The `plugins/dckt-instructions.js` plugin**, under the same user-level opencode directory. This + content-free plugin runs `dckt instructions` and adds whatever it prints to the system prompt. + Outside a private repository the command prints nothing, so the plugin is inert. It needs no + opt-in, and uninstall removes it only while it is unchanged. ### The opt-in you need diff --git a/docs/superpowers/specs/2026-10-06-load-a-private-repository-s-agent-instructions-without-repos-design.md b/docs/superpowers/specs/2026-10-06-load-a-private-repository-s-agent-instructions-without-repos-design.md new file mode 100644 index 000000000..4bf329b59 --- /dev/null +++ b/docs/superpowers/specs/2026-10-06-load-a-private-repository-s-agent-instructions-without-repos-design.md @@ -0,0 +1,207 @@ +# Load a private repository's agent instructions without repository files: design + +Change 0535 — Load a private repository's agent instructions without repository files + +Change #535, groomed interactively on 2026-10-05, split out of #532 in the private-visibility series. Depends on #531 (private mode, `.git/dckt/`) and #534 (the `dckt` alias). #533 depends on it. + +## Summary + +In shared repositories, the parent agent learns docket's dispatch and run-tracker rules from the repository's own CLAUDE.md or AGENTS.md. A private repository can't carry those files. Instead: + +- The rules live in a private instructions file under `.git/dckt/`. +- Each harness gets a trigger that loads that file. The triggers carry no rules and are installed once per machine. + +Each trigger loads the file mechanically wherever the harness offers a way to do so, and nothing is written into the private repository's working tree. Promoted lessons move into the same private file. + +## Evidence gathered at grooming + +- **The dispatch rules exist only in the repository's own instructions file.** Since changes 0334 and 0351, the dispatch block (dispatch the named agent; bracket each implement-next run with `run.start` / `run.verdict`) is written only to a repository's own CLAUDE.md or AGENTS.md. + - `docket install` writes no user-level copy and retires old ones (`GlobalDispatchTarget` in `internal/harness/claude/claude.go`; the codex and opencode adapters say the same). + - 0334 removed the user-level copy because a second copy of the rules drifted. + - At grooming, `~/.claude/CLAUDE.md`, `~/.codex/AGENTS.md`, and `~/.config/opencode/AGENTS.md` contained no docket text. +- **Repository-level surfaces.** The `agent_harnesses` repository phase (`reposeed.Plan`, `installAuthorizedSurfaces` in `internal/app/repository_init.go`, `internal/app/repophase.go`) writes: + - the AGENTS.md and CLAUDE.md dispatch blocks, with the interior from `harness.DispatchInterior` / `CodexDispatchInterior` in `internal/harness/dispatch.go`; + - `.cursor/rules/docket-dispatch.mdc`. +- **Learnings promotion** lands a graduated rule in the integration-branch AGENTS.md or CLAUDE.md by hand (`skills/docket-convention/references/learnings.md`, *Promotion*). + +### Delivery spike (2026-10-06) + +The first build attempt halted at its per-harness spike. The human then had each delivery mechanism re-tested before re-grooming. + +**How it was tested:** +- headless, fresh processes, with a stub `dckt`; +- temporary config only: Claude `--settings` / `--plugin-dir`, and `OPENCODE_CONFIG_DIR` for OpenCode. Cursor's user-level hooks file was created only for the test and removed afterwards; +- the payload was this repository's real 12.8KB dispatch block plus lessons, with a test rule as its last line, so truncation would show; +- each passing case was confirmed by having the model quote the run-tracker's step 2 and the first Shell lesson verbatim. + +**Results:** + +- **Claude Code 2.1.291.** + - Each hook's `additionalContext` (or plain stdout) is capped at 10,000 characters, and the cap is measured per hook. Over the cap, Claude Code saves the output to a file and shows the model only the path and a 2,000-character preview. + - With one hook carrying the whole file, the model's view ended in the middle of run-tracker step 1, so it lost step 2 (obey `run.verdict`) and every lesson. + - Two hooks, one for the dispatch block (6,152 characters) and one for the lessons (6,601), delivered everything: 4/4 runs, in the primary checkout and a linked worktree. They added nothing outside git. + - A plugin does not lift the cap: + - a plugin hook has the same limit; + - a plugin's `CLAUDE.md` is never loaded; + - MCP server `instructions` were cut at about 2,000 characters. +- **OpenCode 1.18.31.** + - The AGENTS.md pointer was ignored on a trivial prompt, 2/2. + - A plugin using `experimental.chat.system.transform` to add `dckt instructions` output to the system prompt delivered everything: 5/5 on `deepseek-v4-flash`, primary checkout and linked worktree. It added nothing outside git. +- **Cursor CLI 2026.10.01.** + - A user-level `~/.cursor/hooks.json` `sessionStart` hook returning `additional_context` delivered all 12.7KB: 5/5, primary checkout and linked worktree. It added nothing outside git. + - User-level hooks run with their working directory set to `~/.cursor`. The project arrives as `CURSOR_PROJECT_DIR` and as `workspace_roots` in the stdin payload. + - The excluded `.cursor/rules` file also worked in the primary checkout, but it does not exist in linked worktrees. +- **Codex 0.154.0.** The pointer was followed on a real task and skipped on a trivial prompt. This is accepted as best effort. + +## Decisions (settled with the human) + +1. **The private instructions file.** A private repository's parent-facing rules live in `/dckt/AGENTS.md`: the dispatch block plus promoted lessons. +2. **Mechanical delivery wherever the harness offers it**, re-decided after the delivery spike: + - **Claude Code:** two user-level `SessionStart` hook entries in `~/.claude/settings.json`, one for the dispatch block and one for the lessons. `docket install` adds them and `docket uninstall` removes them. The human chose settings entries over a docket-owned Claude plugin: a plugin would change only the packaging, not what the model sees. + - **Cursor:** one user-level `sessionStart` hook entry in `~/.cursor/hooks.json`. + - **OpenCode:** a docket-owned plugin file in `~/.config/opencode/plugins/`. + - **Codex:** a static pointer block in `~/.codex/AGENTS.md`. Whether the model follows it is up to the model; the human accepted that as best effort. +3. **No drift.** No user-level surface carries rule text. The rules have one source, so the drift that retired the user-level copy cannot recur. +4. **Neutral spelling.** Every trigger invokes `dckt` (#534). The pointer's managed markers use the `dckt:` prefix, and the plugin file is `dckt-instructions.js`, so nothing in the user's home directory reads "docket". +5. **Nothing in the private working tree.** This replaces the earlier excluded Cursor rule file. The user-level Cursor hook makes that file unnecessary, and an excluded file would be missing from every linked worktree. +6. **Split by meaning, not by size (Claude).** Each Claude hook carries a complete document: the dispatch block, or the lessons. That way the order in which they arrive doesn't matter. + - Docket generates the dispatch block, so it is kept under the 10,000-character cap and pinned by a test. + - Lessons that grow past the cap fall back to Claude Code's own path-plus-preview. That is visible to the model and never stops anything. +7. **Scope.** `docket install` writes the user-level triggers once per machine. The private file is written per repository. `agent_harnesses` stays a per-repository choice, exactly as in shared mode. + +## Design + +### The private instructions file + +In private repositories: + +- `init --private`, and `install`'s repository phase, write the managed dispatch block into `/dckt/AGENTS.md`. It is the same interior the repository-level block carries today, selected by `agent_harnesses`, Codex clause included. +- They write nothing in the worktree: no AGENTS.md or CLAUDE.md dispatch block and no Cursor rule file. They add nothing to `.git/info/exclude` either. +- The learnings promotion destination is this file, and the learnings reference says so. +- Blocks committed before the repository went private are #533's `--remove-shared-files` concern. + +### `docket instructions` + +A new read-only catalog operation. + +- **In a private repository:** prints the private instructions file from any worktree of the clone. It resolves through the git common directory, because a feature worktree's `.git` is a file. +- **Anywhere else:** prints **nothing** and exits 0. That covers shared repositories, non-docket repositories, and running outside git. It never fails noisily, because it runs in every session. +- **`--section dispatch|lessons`:** `dispatch` prints the managed dispatch block, markers included. `lessons` prints everything outside it. A section with no content prints nothing. +- **`--hook claude`:** wraps the selected content in Claude Code's `SessionStart` hook output (`hookSpecificOutput.additionalContext`). It prints nothing outside private repositories or when the content is empty. +- **`--hook cursor`:** wraps the content in Cursor's `sessionStart` hook output (`{"additional_context": …}`). + - Cursor runs user-level hooks from `~/.cursor`, so this mode finds the repository from `CURSOR_PROJECT_DIR`, or failing that from the first `workspace_roots` entry of the hook's stdin payload. + - It prints nothing when neither names a private repository. +- **Hook modes never fail a session:** any error means no output and exit 0. + +### User-level triggers + +`docket install` writes these when it installs the corresponding harness. They are installed even on machines with no private repository yet, and are no-ops everywhere else. + +| Harness | Surface | What it does | +|---|---|---| +| Claude Code | two `SessionStart` command entries in `~/.claude/settings.json`: `dckt instructions --hook claude --section dispatch` and `dckt instructions --hook claude --section lessons` | loads the dispatch block and the lessons at session start, each under Claude Code's per-hook cap; adds nothing outside private repositories | +| Cursor | one `sessionStart` command entry in `~/.cursor/hooks.json` (`"version": 1`): `dckt instructions --hook cursor` | loads the whole file at session start; adds nothing outside private repositories | +| OpenCode | plugin file `~/.config/opencode/plugins/dckt-instructions.js` | adds `dckt instructions` output to the system prompt on every model call | +| Codex | managed pointer block in `~/.codex/AGENTS.md` | asks the model to run `dckt instructions` once per session and follow its output (best effort) | + +The OpenCode plugin and the Codex pointer wording are final in the plan. Both must stay rule-free and free of the word "docket". The plugin, as proven in the spike: + +```js +// Adds a private repository's instructions to the system prompt. +// `dckt instructions` prints nothing outside a private repository. +export const DcktInstructions = async ({ $, directory }) => ({ + "experimental.chat.system.transform": async (_input, output) => { + const text = await $`dckt instructions`.cwd(directory).quiet().nothrow().text() + if (text.trim()) output.system.push(text) + }, +}) +``` + +The Codex pointer: + +```markdown + +## Private repository instructions + +At the start of a session inside a git repository, run `dckt instructions` once. +If it prints anything, treat that output as this repository's own AGENTS.md and +follow it for the rest of the session. If it prints nothing, ignore this section. + +``` + +**Safety requirements:** + +- **JSON hook entries** (Claude's `settings.json`, Cursor's `hooks.json`): + - Each entry is identified by its exact command string. + - Install adds an entry only when it is absent, and merges it without reformatting or dropping any other setting or hook. + - Install creates `~/.cursor/hooks.json` with `"version": 1` when the file is absent. + - Uninstall removes only an exact match, and reports a modified entry rather than touching it. +- **The OpenCode plugin file** is wholly docket-owned and identified by its exact bytes. + - Install writes it when absent and does nothing when it is identical. + - It reports a modified file and never overwrites it. + - Uninstall removes only an exact match. +- **The Codex pointer block** uses the managed-block machinery, extended to accept the `dckt` marker prefix with the same guarantees: closed-block guard, idempotence, outside bytes preserved, and ownership-proved retirement. +- **This is a deliberate, narrow return of user-level parent-facing writes**, which 0351 retired. It is acceptable because the surfaces carry only a trigger, never rules. +- **Without the alias** (#534 reports a foreign `dckt`), the triggers fail harmlessly: the hooks print nothing, the plugin gets empty output, and the pointer's command fails. `docket instructions` still works by hand. + +**Known limits, accepted:** +- OpenCode's hook is `experimental.`, so a future OpenCode release may rename it. +- Cursor documents `sessionStart` as fire-and-forget (it was delivered in every spike run). +- Only the Cursor CLI was exercised, not the desktop app. +- Lessons past 10,000 characters reach Claude as Claude Code's path-plus-preview. + +### Fresh-session acceptance (replaces the spike task) + +The grooming spike proved each mechanism with a stub. The build repeats the check against the real built `dckt`: + +- in fresh headless sessions, with temporary configuration only; +- Cursor's user-level hook is exercised through a project-level `hooks.json` in a temporary fixture, plus a run from an unrelated working directory with `CURSOR_PROJECT_DIR` set; +- results are recorded in the results file. + +If Claude Code, Cursor, or OpenCode fails, the build stops, because their mechanisms are proven. A Codex result is recorded and never stops the build. A harness that can't be run (missing CLI or login) is recorded as an Important human verification item, as is a real user-level Cursor `~/.cursor/hooks.json` check. + +### Documentation + +- `docket instructions` command help. +- The skills: + - convention: the private promotion destination; + - agent-layer reference: private delivery, listing the four triggers. +- No `docs/` pages. + +## Acceptance criteria + +1. **The private file.** In a private repository, `/dckt/AGENTS.md` carries the dispatch block (Codex clause when selected). Nothing is written in the worktree (no AGENTS.md, CLAUDE.md, or Cursor rule file), and nothing is added to `.git/info/exclude`. +2. **`docket instructions` output:** + - it prints the file from the primary worktree, a feature worktree, and the metadata worktree; + - it prints nothing and exits 0 in a shared repository, a non-docket repository, and outside git; + - `--section dispatch` prints exactly the managed block, and `--section lessons` prints everything else; + - `--hook claude` emits valid `SessionStart` output only in private repositories, and only for non-empty content; + - run from an unrelated working directory, `--hook cursor` finds the repository from `CURSOR_PROJECT_DIR`, or failing that from the stdin `workspace_roots`, and emits `additional_context` only for a private repository. +3. **Installing the triggers:** + - install adds the two Claude hook entries, the Cursor hook entry, the OpenCode plugin file, and the Codex pointer block; + - a second install is a no-op; + - an unrelated `settings.json` or `hooks.json` key and hook survive byte-for-byte; + - uninstall removes only docket's exact entries, file, and block, and reports a modified one without touching it. +4. **No rule text, no "docket".** The triggers contain no dispatch-block sentence and no word "docket", pinned by a test that fails if either appears. +5. **The dispatch block fits one Claude hook.** The dispatch block docket generates stays under 10,000 characters for every `agent_harnesses` selection, pinned by a test. +6. **Learnings promotion** in a private repository names the private file as its destination. +7. **Shared repositories are unchanged:** repository-level blocks, and no private file. +8. **Fresh-session acceptance per harness** is recorded in the results file, as *Fresh-session acceptance* describes. In a private repository with no repository instruction files, a Claude Code, Cursor, and OpenCode session each receives the complete dispatch rules. + +## ADRs expected + +A private repository's parent-facing rules live in `/dckt/AGENTS.md`. They are reached through content-free user-level triggers: + +- two Claude Code `SessionStart` hooks, split by meaning to fit the per-hook cap; +- a Cursor `sessionStart` hook; +- an OpenCode system-prompt plugin; +- a Codex pointer block (best effort). + +This narrowly revisits 0351's retirement of user-level parent-facing writes. Relates to ADR-0036 and ADR-0078. + +## Out of scope + +- Shared repositories. +- Writing rules and the leak check (#532). +- The `dckt` alias (#534). +- Moving instructions on a mode switch (#533). diff --git a/internal/app/finding_codes.go b/internal/app/finding_codes.go index d2955cb6d..9a97ab04e 100644 --- a/internal/app/finding_codes.go +++ b/internal/app/finding_codes.go @@ -155,6 +155,13 @@ const ( // unaffected. FCBinaryAliasForeign FindingCode = "binary-alias-foreign" FCBinaryAliasMissing FindingCode = "binary-alias-missing" + + // hook-file-not-editable is the warning code an install, development + // install, or install check surfaces for a hooks file docket cannot edit in + // place (a dotfiles manager's symlink, or a file it cannot parse): the file + // is left untouched and that harness's trigger is not installed. A warning + // only: the primary result is unaffected. + FCHookFileNotEditable FindingCode = "hook-file-not-editable" ) // AllFindingCodes is the authoritative, sorted, deduplicated vocabulary of every @@ -244,6 +251,7 @@ var AllFindingCodes = []FindingCode{ FCEmptyWhyDeferred, FCEmptyWhyKilled, FindingCode(ReasonStatusExternal), + FCHookFileNotEditable, FindingCode("illegal-source-status"), FindingCode(ReasonStatusInternalError), FindingCode(ReasonStatusInterrupted), diff --git a/internal/app/install.go b/internal/app/install.go index 628759571..6408c88fb 100644 --- a/internal/app/install.go +++ b/internal/app/install.go @@ -236,9 +236,25 @@ func NewInstallResult(operation string, out install.Outcome) InstallResult { r.Warnings = append(r.Warnings, collectionWarning(out.Collection)) } r.Warnings = append(r.Warnings, aliasWarnings(out.AliasFindings)...) + r.Warnings = append(r.Warnings, skippedWarnings(out.Skipped)...) return r } +// skippedWarnings renders the targets an operation left untouched because +// docket cannot edit them in place. Each is reported with its path and remedy +// and never reclassifies the operation: the rest of the installation proceeded. +func skippedWarnings(skipped []install.Inspection) []InstallWarning { + var out []InstallWarning + for _, s := range skipped { + out = append(out, InstallWarning{Diagnostic: config.Diagnostic{ + Code: string(FCHookFileNotEditable), Severity: config.SeverityWarning, Path: s.Target.Path, + Message: "hooks file left untouched; its trigger is not installed", + Remedy: s.Remedy, + }}) + } + return out +} + // aliasWarnings renders the dckt alias findings as warnings. The alias is a // convenience beside the binary, so its absence or a foreign occupant is // reported with its remedy and never reclassifies the operation. diff --git a/internal/app/install_test.go b/internal/app/install_test.go index 224bdf72c..687338509 100644 --- a/internal/app/install_test.go +++ b/internal/app/install_test.go @@ -475,6 +475,37 @@ func TestInstallResultNoWarningsOmitsField(t *testing.T) { } } +// A skipped target — a hooks file docket cannot edit in place — is a warning +// naming the path and its remedy: it never changes the result class, so an +// install that skipped one trigger still reports applied. +func TestInstallResultRendersSkippedTargetsAsWarnings(t *testing.T) { + out := install.Outcome{ + Mode: install.ModeRelease, + Applied: true, + Skipped: []install.Inspection{{ + Target: install.Target{Path: "/h/.claude/settings.json", Kind: install.KindHookEntries}, + Disposition: install.DispositionSkip, + Remedy: "make it a regular file, then re-run", + }}, + } + r := NewInstallResult(OperationInstall, out) + if r.Result != ResultApplied { + t.Fatalf("result = %q, want %q: a skipped target must not change the class", r.Result, ResultApplied) + } + if len(r.Warnings) != 1 { + t.Fatalf("warnings = %+v", r.Warnings) + } + got := r.Warnings[0] + if got.Code != string(FCHookFileNotEditable) || got.Path != "/h/.claude/settings.json" || + got.Remedy != "make it a regular file, then re-run" || got.Severity != config.SeverityWarning || got.Message == "" { + t.Errorf("warning = %+v", got) + } + human := r.HumanText() + if !strings.Contains(human, "warning: /h/.claude/settings.json") || !strings.Contains(human, "make it a regular file") { + t.Errorf("human text does not show the skipped hooks file with its remedy:\n%s", human) + } +} + // A dckt finding is a warning: it never changes the result class, so a check // that only finds an alias problem still exits clean. func TestInstallResultRendersAliasFindingsAsWarnings(t *testing.T) { diff --git a/internal/app/instructions.go b/internal/app/instructions.go new file mode 100644 index 000000000..10b851950 --- /dev/null +++ b/internal/app/instructions.go @@ -0,0 +1,268 @@ +package app + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "strings" + + "github.com/danielhanold/docket/internal/document" + "github.com/danielhanold/docket/internal/layout" +) + +// OperationInstructions is the protocol operation id of `docket instructions`. +const OperationInstructions = "instructions" + +// The sections `docket instructions --section` selects from a private +// repository's instructions file: the whole file, the managed dispatch block +// with its marker lines, or everything outside that block (the promoted +// lessons). +const ( + InstructionsSectionAll = "" + InstructionsSectionDispatch = "dispatch" + InstructionsSectionLessons = "lessons" +) + +// instructionsBlockName is the managed block the dispatch section selects — +// the block reposeed.PlanPrivate plans into the private file. +const instructionsBlockName = "dispatch" + +// cursorStdinLimit bounds the Cursor hook payload read from stdin. +const cursorStdinLimit = 1 << 20 + +// ReadPrivateInstructions reads a private repository's instructions file +// (/dckt/AGENTS.md) for any directory inside any of its +// worktrees. It walks up from dir to the first `.git` entry and resolves the +// common dir from the filesystem alone — no git process — so a session-start +// hook pays almost nothing. +// +// Outside git, or in a shared repository: (nil, false, nil). Private without +// the file: (nil, true, nil). Any other probe or read error is returned, never +// reported as "nothing". +func ReadPrivateInstructions(dir string) (content []byte, private bool, err error) { + abs, err := filepath.Abs(dir) + if err != nil { + return nil, false, err + } + root, found, err := findDotGit(abs) + if err != nil || !found { + return nil, false, err + } + common, ok, err := layout.CommonDirOf(root) + if err != nil || !ok { + return nil, false, err + } + mode, err := layout.Detect(common) + if err != nil { + return nil, false, err + } + if mode != layout.Private { + return nil, false, nil + } + content, err = os.ReadFile(layout.PrivateInstructionsPath(common)) + if errors.Is(err, fs.ErrNotExist) { + return nil, true, nil + } + if err != nil { + return nil, true, err + } + return content, true, nil +} + +// findDotGit returns the nearest directory at or above abs that holds a `.git` +// entry. Not-exist moves one level up (stopping at the filesystem root); any +// other probe error is returned. +func findDotGit(abs string) (string, bool, error) { + for d := abs; ; { + _, err := os.Lstat(filepath.Join(d, ".git")) + if err == nil { + return d, true, nil + } + if !errors.Is(err, fs.ErrNotExist) { + return "", false, fmt.Errorf("instructions: probing %s: %w", filepath.Join(d, ".git"), err) + } + parent := filepath.Dir(d) + if parent == d { + return "", false, nil + } + d = parent + } +} + +// SelectInstructionsSection selects one section of an instructions file. All: +// the content verbatim. Dispatch: the `dispatch` managed block with its marker +// lines. Lessons: everything outside that block. A whitespace-only dispatch or +// lessons result is nil. Malformed markers and an unknown section are errors. +func SelectInstructionsSection(content []byte, section string) ([]byte, error) { + switch section { + case InstructionsSectionAll: + return content, nil + case InstructionsSectionDispatch, InstructionsSectionLessons: + default: + return nil, fmt.Errorf("instructions: unknown section %q (want %q or %q)", + section, InstructionsSectionDispatch, InstructionsSectionLessons) + } + doc, err := document.Parse(content) + if err != nil { + return nil, fmt.Errorf("instructions: %w", err) + } + src := doc.Source() + var out []byte + block, ok := doc.Block(instructionsBlockName) + switch { + case section == InstructionsSectionDispatch && ok: + out = src[block.Start.Start:block.End.End] + case section == InstructionsSectionDispatch: + out = nil + case ok: + out = append(append([]byte(nil), src[:block.Start.Start]...), src[block.End.End:]...) + default: + out = src + } + if len(bytes.TrimSpace(out)) == 0 { + return nil, nil + } + return out, nil +} + +// claudeSessionStart is Claude Code's SessionStart hook output document. +type claudeSessionStart struct { + HookSpecificOutput struct { + HookEventName string `json:"hookEventName"` + AdditionalContext string `json:"additionalContext"` + } `json:"hookSpecificOutput"` +} + +// cursorSessionStart is Cursor's sessionStart hook output document. +type cursorSessionStart struct { + AdditionalContext string `json:"additional_context"` +} + +// ClaudeSessionStartOutput wraps content as one Claude Code SessionStart hook +// output line: +// {"hookSpecificOutput":{"hookEventName":"SessionStart","additionalContext":…}}. +// Blank content gives nil. +func ClaudeSessionStartOutput(content []byte) []byte { + if len(bytes.TrimSpace(content)) == 0 { + return nil + } + var doc claudeSessionStart + doc.HookSpecificOutput.HookEventName = "SessionStart" + doc.HookSpecificOutput.AdditionalContext = string(content) + return encodeHookLine(doc) +} + +// CursorSessionStartOutput wraps content as one Cursor sessionStart hook output +// line: {"additional_context":…}. Blank content gives nil. +func CursorSessionStartOutput(content []byte) []byte { + if len(bytes.TrimSpace(content)) == 0 { + return nil + } + return encodeHookLine(cursorSessionStart{AdditionalContext: string(content)}) +} + +// encodeHookLine renders v as one JSON line with HTML escaping off, so the +// managed block's `\nX\n\ntail\n" + +func TestSelectInstructionsSection(t *testing.T) { + cases := []struct { + name, content, section string + want []byte + }{ + {"all verbatim", sectionFixture, InstructionsSectionAll, []byte(sectionFixture)}, + {"dispatch block", sectionFixture, InstructionsSectionDispatch, + []byte("\nX\n\n")}, + {"lessons", sectionFixture, InstructionsSectionLessons, []byte("lead\ntail\n")}, + {"only the block: no lessons", "\nX\n\n", + InstructionsSectionLessons, nil}, + {"no block: no dispatch", "just lessons\n", InstructionsSectionDispatch, nil}, + {"no block: whole file is lessons", "just lessons\n", InstructionsSectionLessons, []byte("just lessons\n")}, + } + for _, c := range cases { + got, err := SelectInstructionsSection([]byte(c.content), c.section) + if err != nil { + t.Errorf("%s: error %v", c.name, err) + continue + } + if c.want == nil { + if got != nil { + t.Errorf("%s: got %q, want nil", c.name, got) + } + continue + } + if !bytes.Equal(got, c.want) { + t.Errorf("%s: got %q, want %q", c.name, got, c.want) + } + } + + unbalanced := "lead\n\nX\n" + for _, section := range []string{InstructionsSectionDispatch, InstructionsSectionLessons} { + if got, err := SelectInstructionsSection([]byte(unbalanced), section); err == nil { + t.Errorf("unbalanced markers, section %q: got %q, nil error; want an error", section, got) + } + } + if got, err := SelectInstructionsSection([]byte(sectionFixture), "bogus"); err == nil { + t.Errorf("unknown section: got %q, nil error; want an error", got) + } +} + +func TestClaudeSessionStartOutput(t *testing.T) { + const content = "a\"b\nX\n") + if res := Instructions(unbalanced, InstructionsSectionDispatch); res.Result != ResultInvalidState || + res.Reason != ReasonInstructionsMarkersInvalid || res.Message == "" || res.Failure != nil { + t.Errorf("Instructions over unbalanced markers = %+v, want invalid-state / %s with a message and no failure", + res, ReasonInstructionsMarkersInvalid) + } + if res := Instructions(r, "bogus"); res.Result != ResultInvalidInput || + res.Reason != ReasonInstructionsUnknownSection || res.Message == "" || res.Failure != nil { + t.Errorf("Instructions with an unknown section = %+v, want invalid-input / %s with a message and no failure", + res, ReasonInstructionsUnknownSection) + } +} + +// TestDispatchBlockFitsOneClaudeHook is acceptance 5: for every non-empty +// harness selection, the dispatch section as install renders it stays under +// Claude Code's 10,000-character per-hook cap. +func TestDispatchBlockFitsOneClaudeHook(t *testing.T) { + rt, err := buildRunTracker() + if err != nil { + t.Fatal(err) + } + all := []string{"claude", "codex", "cursor", "opencode"} + root := filepath.Join(testsupport.TempDir(t), "r") + largest, largestSel, sawCodex := 0, "", false + for mask := 1; mask < 1<= 10000 { + t.Errorf("%v: dispatch section is %d characters; Claude Code caps one hook at 10,000", sel, n) + } + if n > largest { + largest, largestSel = n, strings.Join(sel, ",") + } + } + if !sawCodex { + t.Fatal("no selection carried the Codex clause; the largest case was never measured") + } + t.Logf("largest dispatch section: %d characters ([%s])", largest, largestSel) +} diff --git a/internal/app/private_instructions_integration_test.go b/internal/app/private_instructions_integration_test.go new file mode 100644 index 000000000..d13aebb5b --- /dev/null +++ b/internal/app/private_instructions_integration_test.go @@ -0,0 +1,490 @@ +//go:build integration + +package app + +import ( + "context" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/config" + "github.com/danielhanold/docket/internal/install" + "github.com/danielhanold/docket/internal/layout" + "github.com/danielhanold/docket/internal/reposeed" + "github.com/danielhanold/docket/internal/testsupport" +) + +// This file is the real-Git shard (prefix TestIntegrationPrivateInstructions) +// for a private repository's instructions file: install's repository phase and +// private init write the dispatch block into /dckt/AGENTS.md and +// nothing into any working tree, retire the surfaces an earlier install wrote +// into the primary worktree, and leave shared repositories as they were. + +// pinInstructionsUserRoots moves HOME and XDG_CONFIG_HOME to fresh temp dirs +// before anything reaches the installer; the user-level roots it resolves must +// never be the real machine's. XDG_DATA_HOME is pinned by newPrivateInitRepo (or +// by the shared test itself). +func pinInstructionsUserRoots(t *testing.T) { + t.Helper() + t.Setenv("HOME", testsupport.TempDir(t)) + t.Setenv("XDG_CONFIG_HOME", testsupport.TempDir(t)) +} + +// newPrivateInstructionsRepo builds a private-initialized clone with no harness +// opted in and returns it with its canonical primary worktree and common dir. +func newPrivateInstructionsRepo(t *testing.T) (r *initRepo, primary, common string) { + t.Helper() + pinInstructionsUserRoots(t) + r, _ = newPrivateInitRepo(t, nil) + if res := r.runInitWith(t, InitOptions{Private: true}); res.Result != ResultApplied { + t.Fatalf("private init = %q (%s), want applied", res.Result, res.HumanText()) + } + var err error + if primary, err = filepath.EvalSymlinks(r.invocation); err != nil { + t.Fatal(err) + } + common = filepath.Join(primary, ".git") + return r, primary, common +} + +var agentHarnessesLine = regexp.MustCompile(`(?m)^agent_harnesses:.*\n`) + +// setPrivateHarnesses rewrites the agent_harnesses line of the private config +// (.git/dckt/config.yml) to the given flow list, e.g. "[claude, codex]". +func setPrivateHarnesses(t *testing.T, common, list string) { + t.Helper() + path := layout.PrivateConfigPath(common) + cfg := string(mustReadFile(t, path)) + cfg = agentHarnessesLine.ReplaceAllString(cfg, "") + if !strings.HasSuffix(cfg, "\n") { + cfg += "\n" + } + cfg += "agent_harnesses: " + list + "\n" + if err := os.WriteFile(path, []byte(cfg), 0o644); err != nil { + t.Fatal(err) + } +} + +// installSurfacesIn runs the init-time surface install against primary and +// fails the test on an error. +func installSurfacesIn(t *testing.T, primary string) []string { + t.Helper() + pending, _, err := installAuthorizedSurfaces(context.Background(), newGitClient(t), primary) + if err != nil { + t.Fatalf("installAuthorizedSurfaces(%s): %v", primary, err) + } + return pending +} + +// assertNoWorktreeSurfaces fails when any parent-facing surface exists in dir. +func assertNoWorktreeSurfaces(t *testing.T, dir string) { + t.Helper() + for _, name := range []string{"AGENTS.md", "CLAUDE.md", ".cursor"} { + if _, err := os.Lstat(filepath.Join(dir, name)); !os.IsNotExist(err) { + t.Errorf("%s exists in %s (err=%v); a private repository gets nothing in its working tree", name, dir, err) + } + } +} + +// recordedPaths is the surface paths of the ownership record at path. +func recordedPaths(t *testing.T, path string) []string { + t.Helper() + rec, err := reposeed.LoadRecord(path) + if err != nil || rec == nil { + t.Fatalf("LoadRecord(%s) = %v, %v; want a record", path, rec, err) + } + var out []string + for _, s := range rec.Surfaces { + out = append(out, s.Path) + } + return out +} + +const dispatchStartMarker = "\n## Rules\nroute it\n\n" + instrLesson = "\n## Lessons\nkeep it small\n" + instrPrivate = instrBlock + instrLesson +) + +// instrPrivateRepo fabricates a private primary worktree whose instructions +// file holds content, and returns the worktree root. +func instrPrivateRepo(t *testing.T, content string) string { + t.Helper() + r := filepath.Join(testsupport.TempDir(t), "r") + if err := os.MkdirAll(filepath.Join(r, ".git", "dckt"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(r, ".git", "dckt", "AGENTS.md"), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return r +} + +// instrSharedRepo fabricates a shared repository (a .git directory, no dckt). +func instrSharedRepo(t *testing.T) string { + t.Helper() + r := filepath.Join(testsupport.TempDir(t), "shared") + if err := os.MkdirAll(filepath.Join(r, ".git"), 0o755); err != nil { + t.Fatal(err) + } + return r +} + +// instrOutsideGit returns a directory with no .git at or above it, skipping +// when the temp dir lives inside a git work tree. +func instrOutsideGit(t *testing.T) string { + t.Helper() + dir := testsupport.TempDir(t) + for d := dir; ; d = filepath.Dir(d) { + if _, err := os.Lstat(filepath.Join(d, ".git")); err == nil { + t.Skipf("ancestor %s of the temp dir has a .git entry", d) + } + if filepath.Dir(d) == d { + return dir + } + } +} + +func TestInstructionsPrivatePlain(t *testing.T) { + t.Chdir(instrPrivateRepo(t, instrPrivate)) + cases := []struct { + args []string + want string + }{ + {[]string{"instructions"}, instrPrivate}, + {[]string{"instructions", "--section", "dispatch"}, instrBlock}, + {[]string{"instructions", "--section", "lessons"}, instrLesson}, + } + for _, c := range cases { + out, errS, code := runCLI(t, c.args...) + if out != c.want || errS != "" || code != 0 { + t.Errorf("%v: out=%q err=%q code=%d, want out=%q", c.args, out, errS, code, c.want) + } + } +} + +func TestInstructionsPrivateNoLessonsPrintsNothing(t *testing.T) { + t.Chdir(instrPrivateRepo(t, instrBlock)) + out, errS, code := runCLI(t, "instructions", "--section", "lessons") + if out != "" || errS != "" || code != 0 { + t.Errorf("out=%q err=%q code=%d, want silence and 0", out, errS, code) + } +} + +func TestInstructionsSharedAndOutsideGitPrintNothing(t *testing.T) { + for label, dir := range map[string]func(*testing.T) string{ + "shared": instrSharedRepo, + "outside git": instrOutsideGit, + } { + t.Run(label, func(t *testing.T) { + t.Chdir(dir(t)) + for _, args := range [][]string{ + {"instructions"}, + {"instructions", "--hook", "claude"}, + {"instructions", "--hook", "claude", "--section", "dispatch"}, + } { + out, errS, code := runCLI(t, args...) + if out != "" || errS != "" || code != 0 { + t.Errorf("%v: out=%q err=%q code=%d, want silence and 0", args, out, errS, code) + } + } + }) + } +} + +func TestInstructionsHookClaude(t *testing.T) { + t.Chdir(instrPrivateRepo(t, instrPrivate)) + out, errS, code := runCLI(t, "instructions", "--hook", "claude", "--section", "dispatch") + if errS != "" || code != 0 { + t.Fatalf("err=%q code=%d", errS, code) + } + var doc struct { + HookSpecificOutput struct { + HookEventName string `json:"hookEventName"` + AdditionalContext string `json:"additionalContext"` + } `json:"hookSpecificOutput"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("decode %q: %v", out, err) + } + if doc.HookSpecificOutput.HookEventName != "SessionStart" || doc.HookSpecificOutput.AdditionalContext != instrBlock { + t.Errorf("decoded %+v, want SessionStart / the dispatch block", doc) + } +} + +func decodeCursor(t *testing.T, out string) string { + t.Helper() + var doc struct { + AdditionalContext string `json:"additional_context"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("decode %q: %v", out, err) + } + return doc.AdditionalContext +} + +// TestInstructionsHookCursorLocatesRepoWithoutWorkingDirectory proves the +// Cursor hook finds the repository from CURSOR_PROJECT_DIR or the stdin +// workspace_roots, never from the working directory (here an unrelated one). +func TestInstructionsHookCursorLocatesRepoWithoutWorkingDirectory(t *testing.T) { + private := instrPrivateRepo(t, instrPrivate) + shared := instrSharedRepo(t) + t.Setenv("CURSOR_PROJECT_DIR", private) + t.Chdir(testsupport.TempDir(t)) + + out, errS, code := runCLI(t, "instructions", "--hook", "cursor") + if errS != "" || code != 0 || decodeCursor(t, out) != instrPrivate { + t.Errorf("env: out=%q err=%q code=%d", out, errS, code) + } + + t.Setenv("CURSOR_PROJECT_DIR", "") + out, errS, code = runCLIStdin(t, `{"workspace_roots":["`+private+`"]}`, "instructions", "--hook", "cursor") + if errS != "" || code != 0 || decodeCursor(t, out) != instrPrivate { + t.Errorf("stdin: out=%q err=%q code=%d", out, errS, code) + } + + // From inside the private repository, stdin that names nothing usable must + // still print nothing: the working directory is never the fallback. + t.Chdir(private) + for label, stdin := range map[string]string{ + "shared repo": `{"workspace_roots":["` + shared + `"]}`, + "garbage": "not json at all", + "empty": "", + } { + out, errS, code := runCLIStdin(t, stdin, "instructions", "--hook", "cursor") + if out != "" || errS != "" || code != 0 { + t.Errorf("%s: out=%q err=%q code=%d, want silence and 0", label, out, errS, code) + } + } +} + +func TestInstructionsUnreadableFile(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root reads a mode-000 file") + } + r := instrPrivateRepo(t, instrPrivate) + path := filepath.Join(r, ".git", "dckt", "AGENTS.md") + if err := os.Chmod(path, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(path, 0o644) }) + t.Chdir(r) + + out, errS, code := runCLI(t, "instructions") + if out != "" || code != 1 || !strings.Contains(errS, path) { + t.Errorf("plain: out=%q err=%q code=%d, want empty stdout, stderr naming %s, exit 1", out, errS, code, path) + } + t.Setenv("CURSOR_PROJECT_DIR", r) + for _, hook := range []string{"claude", "cursor"} { + out, errS, code := runCLI(t, "instructions", "--hook", hook) + if out != "" || errS != "" || code != 0 { + t.Errorf("--hook %s: out=%q err=%q code=%d, want silence and 0", hook, out, errS, code) + } + } +} + +func TestInstructionsJSON(t *testing.T) { + t.Chdir(instrPrivateRepo(t, instrPrivate)) + out, errS, code := runCLI(t, "--json", "instructions") + if errS != "" || code != 0 { + t.Fatalf("err=%q code=%d", errS, code) + } + for _, want := range []string{`"operation":"instructions"`, `"result":"applied"`, `"private":true`} { + if !strings.Contains(out, want) { + t.Errorf("--json output %q lacks %s", out, want) + } + } + for _, args := range [][]string{ + {"--json", "instructions", "--hook", "claude"}, + {"instructions", "--hook", "foo"}, + {"instructions", "--section", "foo"}, + {"instructions", "--hook", "claude", "--section", "foo"}, + } { + _, _, code := runCLI(t, args...) + if code != 2 { + t.Errorf("%v: exit %d, want 2 (invalid arguments)", args, code) + } + } + out, _, _ = runCLI(t, "--json", "instructions", "--hook", "claude") + if !strings.Contains(out, `"result":"invalid-input"`) { + t.Errorf("--json --hook claude: %q, want an invalid-input document", out) + } +} + +// TestTriggerCommandsParse runs every installed hook command, minus its +// program name, through the real CLI in a private fixture. +func TestTriggerCommandsParse(t *testing.T) { + r := instrPrivateRepo(t, instrPrivate) + t.Chdir(r) + for _, command := range []string{harness.ClaudeDispatchHookCommand, harness.ClaudeLessonsHookCommand, harness.CursorHookCommand} { + words := strings.Fields(command) + if len(words) < 2 || words[0] != "dckt" { + t.Fatalf("trigger %q does not start with the dckt program name", command) + } + if command == harness.CursorHookCommand { + t.Setenv("CURSOR_PROJECT_DIR", r) + } + out, errS, code := runCLI(t, words[1:]...) + if code != 0 || errS != "" || strings.Count(out, "\n") != 1 || !json.Valid([]byte(out)) { + t.Errorf("%q: out=%q err=%q code=%d, want one JSON line and exit 0", command, out, errS, code) + } + } +} + +func TestInstructionsRunsWithoutInstallation(t *testing.T) { + pinInstallEnv(t) + t.Chdir(instrPrivateRepo(t, instrPrivate)) + if out, errS, code := runCLI(t, "instructions"); code != 0 || out != instrPrivate { + t.Errorf("out=%q err=%q code=%d, want the file and 0 with no installation", out, errS, code) + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index eed1b3bee..d292f59dc 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -58,6 +58,10 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer, info buildinf // code is the verdict, so it is threaded straight to the process exit below, // bypassing the presenter (change 0318). var devTestCode *int + // rawOut carries `instructions` output in its plain and hook forms: raw + // bytes a harness consumes verbatim, never a result document, so it bypasses + // the presenter the way devTestCode does. + var rawOut *rawOutput root := &cobra.Command{ Use: "docket", @@ -416,11 +420,16 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer, info buildinf maintenanceCmd := newMaintenanceCommand(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, + func(r app.OperationResult) { result = r }, + func(out []byte, errText string, code int) { + rawOut = &rawOutput{out: out, errText: errText, code: code} + }) 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, diagnosticCmd, installCmd, uninstallCmd, developmentCmd) + 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(extra...) // The asset-dependence guard. Commands that do not read installed assets @@ -474,6 +483,14 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer, info buildinf // its exit code is the verdict; present nothing and return that code // directly, the way a relayed development-install child does (change 0318). return *devTestCode + case rawOut != nil: + // `instructions` in its plain or hook form: the bytes are the output, + // an error is one stderr line, and the code is the verdict. + _, _ = stdout.Write(rawOut.out) + if rawOut.errText != "" { + fmt.Fprintln(stderr, rawOut.errText) + } + return rawOut.code case result != nil: // A development-install parent that handed off to its candidate has no // document of its own: the candidate already wrote the one result to the @@ -507,6 +524,14 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer, info buildinf } } +// rawOutput is a command's verbatim output: bytes for stdout, an optional +// stderr line, and the exit code. +type rawOutput struct { + out []byte + errText string + code int +} + // rejectHiddenCompletionCommand refuses Cobra's hidden shell-completion // commands as ordinary unknown commands, returning nil for every other // invocation. diff --git a/internal/document/document.go b/internal/document/document.go index f9a0e5f96..30f4e3428 100644 --- a/internal/document/document.go +++ b/internal/document/document.go @@ -310,7 +310,9 @@ func EmptyValue(located Field, locatedOK bool, node *yaml.Node) bool { // mutating it cannot reach the document's own index. func (d Document) Blocks() []Block { return append([]Block(nil), d.blocks...) } -// Block returns the managed block named name, and whether it exists. +// Block returns the managed block named name, and whether it exists. A +// `docket:` block is named bare ("dispatch"); a block in the neutral namespace +// is named with NeutralMarkerPrefix ("dckt:private-instructions"). func (d Document) Block(name string) (Block, bool) { for _, b := range d.blocks { if b.Name == name { diff --git a/internal/document/fuzz_test.go b/internal/document/fuzz_test.go index 4c8bd661c..2646d224f 100644 --- a/internal/document/fuzz_test.go +++ b/internal/document/fuzz_test.go @@ -85,6 +85,9 @@ var markerAdversarialSeeds = []string{ "\nx\n\n", "---\nid: 1\n---\n\nx\n\n", "---\n\n---\nbody\n", // marker-shaped text inside frontmatter + "\nx\n\n\ny\n\n", + "\nx\n\n", // namespaces do not pair across + "\n", // malformed neutral name } // seedMarkerBodies seeds the marker surface with body-oriented slices: the @@ -172,18 +175,20 @@ func FuzzMarkers(f *testing.F) { if m == nil { t.Fatalf("block %q start line %q does not match the marker grammar", b.Name, startText) } - if string(m[1]) != b.Name || string(m[2]) != "start" || string(m[3]) != b.Annotation { + if spelled := string(m[1]) + ":" + string(m[2]); spelled != MarkerSpelling(b.Name) || + string(m[3]) != "start" || string(m[4]) != b.Annotation { t.Fatalf("block %q start line %q reports (%q, %q, %q), want (%q, \"start\", %q)", - b.Name, startText, m[1], m[2], m[3], b.Name, b.Annotation) + b.Name, startText, spelled, m[3], m[4], MarkerSpelling(b.Name), b.Annotation) } endText := trimTerminator(src[b.End.Start:b.End.End]) m = markerRE.FindSubmatch(endText) if m == nil { t.Fatalf("block %q end line %q does not match the marker grammar", b.Name, endText) } - if string(m[1]) != b.Name || string(m[2]) != "end" || string(m[3]) != "" { + if spelled := string(m[1]) + ":" + string(m[2]); spelled != MarkerSpelling(b.Name) || + string(m[3]) != "end" || string(m[4]) != "" { t.Fatalf("block %q end line %q reports (%q, %q, %q), want (%q, \"end\", \"\")", - b.Name, endText, m[1], m[2], m[3], b.Name) + b.Name, endText, spelled, m[3], m[4], MarkerSpelling(b.Name)) } // The canonical renderers agree with source whenever the source // line is already canonical — the end marker has exactly one diff --git a/internal/document/markers.go b/internal/document/markers.go index 42e04925d..fc67ae178 100644 --- a/internal/document/markers.go +++ b/internal/document/markers.go @@ -5,31 +5,57 @@ import ( "fmt" "regexp" "strconv" + "strings" "unicode/utf8" ) +// NeutralMarkerPrefix qualifies a block in the neutral `dckt:` marker namespace, +// used where nothing docket-named may appear. A bare name is in `docket:`. +const NeutralMarkerPrefix = "dckt:" + var ( - // markerPrefixRE matches a line that BEGINS like a Docket marker. It is what + // markerPrefixRE matches a line that BEGINS like a marker in either + // namespace (`docket:` or the neutral `dckt:`). It is what // separates "malformed marker" from ordinary authored prose: a line opening // with this prefix is claiming to be a marker, so failing the grammar is a // defect rather than content. The prefix is column-zero exact, so an // indented marker-shaped line stays prose. - markerPrefixRE = regexp.MustCompile(`^$`) + `^$`) // codeFenceRE matches a CommonMark fenced-code-block delimiter: up to three // leading spaces, then a run of at least three backticks or tildes. codeFenceRE = regexp.MustCompile("^ {0,3}(`{3,}|~{3,})") ) -// blockNameRE is markerRE's name group on its own, so a block a patch CREATES -// is held to exactly the grammar the scanner will later have to recognize. It -// is deliberately not validKey: marker names are hyphenated, field keys are -// underscored. -var blockNameRE = regexp.MustCompile(`^[a-z][a-z0-9-]*$`) +// blockNameRE is markerRE's name group on its own, optionally qualified by +// NeutralMarkerPrefix, so a block a patch CREATES is held to exactly the +// grammar the scanner will later have to recognize. It is deliberately not +// validKey: marker names are hyphenated, field keys are underscored. +var blockNameRE = regexp.MustCompile(`^(?:dckt:)?[a-z][a-z0-9-]*$`) + +// splitBlockName splits a block name into its marker namespace and its bare +// marker name: a NeutralMarkerPrefix-qualified name is in `dckt`, any other +// name in `docket`. +func splitBlockName(name string) (namespace, bare string) { + if rest, ok := strings.CutPrefix(name, NeutralMarkerPrefix); ok { + return "dckt", rest + } + return "docket", name +} + +// MarkerSpelling is how a block name appears inside its marker lines: +// "dispatch" is spelled "docket:dispatch", and a qualified +// "dckt:private-instructions" is spelled as given. +func MarkerSpelling(name string) string { + ns, bare := splitBlockName(name) + return ns + ":" + bare +} // validBlockName reports whether name may appear in a Docket marker line. func validBlockName(name string) bool { return blockNameRE.MatchString(name) } @@ -61,12 +87,12 @@ func validAnnotation(s string) error { // terminator; the caller supplies the document's own. func startMarkerLine(name, annotation string) string { if annotation == "" { - return "" + return "" } - return "" + return "" } -func endMarkerLine(name string) string { return "" } +func endMarkerLine(name string) string { return "" } // Block is one located managed marker block. type Block struct { @@ -130,17 +156,21 @@ func scanMarkers(src []byte, lines []sourceLine, firstLine int) ([]marker, error Offset: lines[i].span.Start, Line: i + 1, Column: 1, - Msg: "line opens as a docket marker but does not match the marker grammar", + Msg: "line opens as a marker but does not match the marker grammar", } } continue } - isStart := string(m[2]) == "start" - annotation := string(m[3]) + name := string(m[2]) + if string(m[1]) == "dckt" { + name = NeutralMarkerPrefix + name + } + isStart := string(m[3]) == "start" + annotation := string(m[4]) if !isStart && annotation != "" { return nil, &Error{ Kind: KindMalformedMarker, - Name: string(m[1]), + Name: name, Offset: lines[i].span.Start, Line: i + 1, Column: 1, @@ -148,7 +178,7 @@ func scanMarkers(src []byte, lines []sourceLine, firstLine int) ([]marker, error } } markers = append(markers, marker{ - name: string(m[1]), + name: name, isStart: isStart, annotation: annotation, span: lines[i].span, diff --git a/internal/document/markers_test.go b/internal/document/markers_test.go index 94b7a033a..7ef2a7440 100644 --- a/internal/document/markers_test.go +++ b/internal/document/markers_test.go @@ -1,6 +1,10 @@ package document -import "testing" +import ( + "errors" + "strings" + "testing" +) const artifactsBlock = "\n| a |\n\n" @@ -179,3 +183,94 @@ func TestCRLFMarkerLinesDiscovered(t *testing.T) { t.Fatalf("interior = %q", got) } } + +func TestNeutralMarkerBlockRoundTrip(t *testing.T) { + src := []byte("user line\n\nbody\n\n\nd\n\n") + doc, err := Parse(src) + if err != nil { + t.Fatal(err) + } + b, ok := doc.Block("dckt:private-instructions") + if !ok || string(src[b.Interior.Start:b.Interior.End]) != "body\n" || b.Annotation != "managed — do not hand-edit" { + t.Fatalf("neutral block not located: %+v ok=%v", b, ok) + } + if _, ok := doc.Block("private-instructions"); ok { + t.Fatal("a bare name must not find a dckt: block") + } + if _, ok := doc.Block("dispatch"); !ok { + t.Fatal("the docket: block must keep its bare name") + } + var p PatchSet + p.RemoveBlock("dckt:private-instructions") + out, err := doc.Apply(p) + if err != nil || string(out) != "user line\n\nd\n\n" { + t.Fatalf("remove: %q %v", out, err) + } +} + +func TestNeutralMarkerInsertRendersDcktPrefix(t *testing.T) { + doc := mustParse(t, "keep\n") + var p PatchSet + p.InsertBlock("dckt:private-instructions", "managed — do not hand-edit", "x", AtDocumentStart) + out, err := doc.Apply(p) + if err != nil { + t.Fatal(err) + } + want := "\nx\n\nkeep\n" + if string(out) != want { + t.Fatalf("insert = %q, want %q", out, want) + } + if strings.Contains(string(out), "docket") { + t.Fatalf("a dckt: block must not spell docket: %q", out) + } + again := mustParse(t, string(out)) + if _, ok := again.Block("dckt:private-instructions"); !ok { + t.Fatal("the inserted dckt: block does not re-parse under its qualified name") + } + var r PatchSet + r.ReplaceBlock("dckt:private-instructions", "y\n") + out2, err := again.Apply(r) + if err != nil || !strings.Contains(string(out2), "-->\ny\n") { + t.Fatalf("replace: %q %v", out2, err) + } +} + +func TestMalformedNeutralMarkerRefuses(t *testing.T) { + _, err := Parse([]byte("\nx\n")) + var de *Error + if !errors.As(err, &de) || de.Kind != KindMalformedMarker { + t.Fatalf("err = %v, want malformed marker", err) + } +} + +func TestSameNameInBothNamespacesIsTwoBlocks(t *testing.T) { + doc := mustParse(t, "\na\n\n\nb\n\n") + src := doc.Source() + bare, ok := doc.Block("x") + if !ok || string(src[bare.Interior.Start:bare.Interior.End]) != "a\n" { + t.Fatalf("docket:x not found: %+v ok=%v", bare, ok) + } + neutral, ok := doc.Block("dckt:x") + if !ok || string(src[neutral.Interior.Start:neutral.Interior.End]) != "b\n" { + t.Fatalf("dckt:x not found: %+v ok=%v", neutral, ok) + } +} + +func TestNeutralMarkerMismatchedNamespaceIsImbalance(t *testing.T) { + _, err := Parse([]byte("\na\n\n")) + var de *Error + if !errors.As(err, &de) || de.Kind != KindMarkerImbalance || de.Name != "x" { + t.Fatalf("err = %v, want imbalance naming the docket: end", err) + } +} + +func TestMarkerSpelling(t *testing.T) { + for name, want := range map[string]string{ + "dispatch": "docket:dispatch", + "dckt:private-instructions": "dckt:private-instructions", + } { + if got := MarkerSpelling(name); got != want { + t.Fatalf("MarkerSpelling(%q) = %q, want %q", name, got, want) + } + } +} diff --git a/internal/harness/claude/claude.go b/internal/harness/claude/claude.go index 680c5b193..420e9ad51 100644 --- a/internal/harness/claude/claude.go +++ b/internal/harness/claude/claude.go @@ -1,5 +1,7 @@ // Package claude renders Docket's installation for Claude Code: skill -// symlinks and one native agent definition per agent source. It no longer +// symlinks, one native agent definition per agent source, and one content-free +// trigger (the private-instructions SessionStart hooks in settings.json, +// harness.ClaudeHookTarget). It no longer // plans a user-global dispatch block (change 0351) — parent-facing routing // belongs to a repository's own CLAUDE.md, not a personal global one — but it // still exports GlobalDispatchTarget so the installer can retire a leftover a @@ -33,6 +35,7 @@ const ( skillsDir = "skills" agentsDir = "agents" dispatchFile = "CLAUDE.md" + settingsFile = "settings.json" // The managed block docket owns in the dispatch file. Both spellings are // part of the installed-state identity: install.InspectTarget finds the @@ -119,6 +122,8 @@ func (adapter) Plan(in harness.PlanInput) ([]install.Target, error) { }) } + targets = append(targets, harness.ClaudeHookTarget(filepath.Join(root, settingsFile))) + sort.Slice(targets, func(i, j int) bool { return targets[i].Path < targets[j].Path }) return targets, nil } diff --git a/internal/harness/claude/claude_test.go b/internal/harness/claude/claude_test.go index 5476fadec..40fb1dbd0 100644 --- a/internal/harness/claude/claude_test.go +++ b/internal/harness/claude/claude_test.go @@ -577,3 +577,21 @@ func TestClaudePlanRejectsUnusableInput(t *testing.T) { } }) } + +// The plan carries exactly one trigger: the two SessionStart hook entries in +// ~/.claude/settings.json, at the exact target harness.ClaudeHookTarget names. +func TestClaudePlanTrigger(t *testing.T) { + want := harness.ClaudeHookTarget(filepath.Join(fakeHome, ".claude", "settings.json")) + var got []install.Target + for _, tg := range planFixture(t) { + if tg.Role == harness.TriggerRole { + got = append(got, tg) + } + } + if len(got) != 1 { + t.Fatalf("plan carries %d trigger-role targets, want 1: %+v", len(got), got) + } + if !reflect.DeepEqual(got[0], want) { + t.Errorf("trigger = %+v, want %+v", got[0], want) + } +} diff --git a/internal/harness/codex/codex.go b/internal/harness/codex/codex.go index 7bff61955..735a2cb4e 100644 --- a/internal/harness/codex/codex.go +++ b/internal/harness/codex/codex.go @@ -1,6 +1,8 @@ // Package codex renders Docket's installation for Codex: skill symlinks under -// the harness-neutral agents root and one native TOML agent definition per -// agent source. It no longer plans a user-global dispatch block (change +// the harness-neutral agents root, one native TOML agent definition per agent +// source, and one content-free trigger (the private-instructions pointer block +// in the same AGENTS.md GlobalDispatchTarget names, harness.CodexPointerTarget). +// It no longer plans a user-global dispatch block (change // 0351) — parent-facing routing belongs to a repository's own AGENTS.md, not a // personal global one — but it still exports GlobalDispatchTarget so the // installer can retire a leftover a prior install owns. It plans only: nothing @@ -178,6 +180,8 @@ func (adapter) Plan(in harness.PlanInput) ([]install.Target, error) { }) } + targets = append(targets, harness.CodexPointerTarget(GlobalDispatchTarget(in.Roots).Path)) + sort.Slice(targets, func(i, j int) bool { return targets[i].Path < targets[j].Path }) return targets, nil } diff --git a/internal/harness/codex/codex_test.go b/internal/harness/codex/codex_test.go index d7b922f2e..801cfe90d 100644 --- a/internal/harness/codex/codex_test.go +++ b/internal/harness/codex/codex_test.go @@ -639,7 +639,9 @@ func TestCodexPlanHasNoGlobalDispatch(t *testing.T) { if tg.Role == "dispatch" { t.Errorf("plan still carries a dispatch-role target at %q", tg.Path) } - if tg.Kind == install.KindManagedBlock { + // The content-free pointer (TestCodexPlanTrigger pins it exactly) is + // the one managed block the plan may carry. + if tg.Kind == install.KindManagedBlock && tg.Role != harness.TriggerRole { t.Errorf("plan still carries a managed-block target at %q", tg.Path) } } @@ -827,7 +829,7 @@ func TestCodexInventoryAdditionPropagates(t *testing.T) { if tg.Path == wantPath && tg.Kind == install.KindFile { found = true } - if tg.Role == "dispatch" || tg.Kind == install.KindManagedBlock { + if tg.Role == "dispatch" || (tg.Kind == install.KindManagedBlock && tg.Role != harness.TriggerRole) { t.Errorf("the grown plan carries a global dispatch surface at %q", tg.Path) } } @@ -835,3 +837,26 @@ func TestCodexInventoryAdditionPropagates(t *testing.T) { t.Errorf("the grown plan carries no agent file at %s", wantPath) } } + +// The plan carries exactly one trigger: the pointer block in the same +// ~/.codex/AGENTS.md GlobalDispatchTarget names for retirement, at the exact +// target harness.CodexPointerTarget names. +func TestCodexPlanTrigger(t *testing.T) { + path := GlobalDispatchTarget(fixtureRoots()).Path + if want := filepath.Join(fakeHome, ".codex", "AGENTS.md"); path != want { + t.Fatalf("pointer path = %q, want %q", path, want) + } + want := harness.CodexPointerTarget(path) + var got []install.Target + for _, tg := range planFixture(t) { + if tg.Role == harness.TriggerRole { + got = append(got, tg) + } + } + if len(got) != 1 { + t.Fatalf("plan carries %d trigger-role targets, want 1: %+v", len(got), got) + } + if !reflect.DeepEqual(got[0], want) { + t.Errorf("trigger = %+v, want %+v", got[0], want) + } +} diff --git a/internal/harness/cross_harness_test.go b/internal/harness/cross_harness_test.go index 067b235d7..774b5ac75 100644 --- a/internal/harness/cross_harness_test.go +++ b/internal/harness/cross_harness_test.go @@ -207,6 +207,11 @@ func TestWrapperRecursionGuard(t *testing.T) { // re-added a dispatch block or rule reddens here regardless of what a golden // says. A parent surface is a dispatch-role target or a managed block — the two // shapes a global CLAUDE.md/AGENTS.md dispatch block or a .cursor rule took. +// +// One managed block is allowed: the content-free Codex pointer, and only in its +// exact form (trigger role, the pointer's block name, the pointer's interior). +// Beside that narrowing, every trigger-role target (the pointer included) must +// carry no line of rule text, and each adapter plans exactly one trigger. func TestNoGlobalParentSurface(t *testing.T) { in := crossPlanInput(t) adapters := map[string]harness.Adapter{ @@ -223,6 +228,7 @@ func TestNoGlobalParentSurface(t *testing.T) { if err != nil { t.Fatalf("ParseInventory: %v", err) } + population := rulePopulation(t) for _, name := range harness.Order { a, ok := adapters[name] @@ -233,12 +239,16 @@ func TestNoGlobalParentSurface(t *testing.T) { if err != nil { t.Fatalf("%s Plan: %v", name, err) } - agentOrSkill := 0 + agentOrSkill, triggers := 0, 0 for _, tg := range targets { + if tg.Role == harness.TriggerRole { + triggers++ + assertNoRuleText(t, name+" trigger "+tg.Path, string(tg.Content), population) + } if tg.Role == "dispatch" { t.Errorf("%s plans a dispatch-role target at %q", name, tg.Path) } - if tg.Kind == install.KindManagedBlock { + if tg.Kind == install.KindManagedBlock && !isCodexPointer(tg) { t.Errorf("%s plans a managed-block target at %q", name, tg.Path) } if tg.Role == "agent" || tg.Role == "skill" { @@ -250,9 +260,19 @@ func TestNoGlobalParentSurface(t *testing.T) { if agentOrSkill == 0 || len(targets) < len(sources) { t.Errorf("%s planned %d targets for %d agent sources; the gate would be vacuous", name, len(targets), len(sources)) } + if triggers != 1 { + t.Errorf("%s plans %d trigger-role targets, want 1", name, triggers) + } } } +// isCodexPointer is the one managed block TestNoGlobalParentSurface allows: +// the trigger-role pointer, carrying exactly the pointer's name and interior. +func isCodexPointer(tg install.Target) bool { + return tg.Role == harness.TriggerRole && tg.BlockName == harness.PointerBlockName && + string(tg.Content) == harness.PointerInterior +} + // TestNoCrossHarnessDelegation plans all four adapters under one shared input // and asserts the two ways an installation could bleed across harnesses: an // agent definition that instructs its harness to launch a sibling's runner, diff --git a/internal/harness/cursor/cursor.go b/internal/harness/cursor/cursor.go index 5eff0017a..b194f543b 100644 --- a/internal/harness/cursor/cursor.go +++ b/internal/harness/cursor/cursor.go @@ -1,5 +1,7 @@ -// Package cursor renders Docket's installation for Cursor: skill symlinks and -// one native custom-agent document per agent source. It no longer plans a +// Package cursor renders Docket's installation for Cursor: skill symlinks, one +// native custom-agent document per agent source, and one content-free trigger +// (the private-instructions sessionStart hook in hooks.json, +// harness.CursorHookTarget). It no longer plans a // user-global dispatch rule (change 0351) — parent-facing routing belongs to a // repository's own .cursor/rules, not a personal global one — but it still // exports GlobalDispatchTarget so the installer can retire a leftover a prior @@ -34,6 +36,7 @@ const ( agentsDir = "agents" rulesDir = "rules" dispatchFile = "docket-dispatch.mdc" + hooksFile = "hooks.json" // Target roles, as recorded in the installed state. roleSkill = "skill" @@ -116,6 +119,8 @@ func (adapter) Plan(in harness.PlanInput) ([]install.Target, error) { }) } + targets = append(targets, harness.CursorHookTarget(filepath.Join(root, hooksFile))) + sort.Slice(targets, func(i, j int) bool { return targets[i].Path < targets[j].Path }) return targets, nil } diff --git a/internal/harness/cursor/cursor_test.go b/internal/harness/cursor/cursor_test.go index 9890437f8..b31734370 100644 --- a/internal/harness/cursor/cursor_test.go +++ b/internal/harness/cursor/cursor_test.go @@ -491,3 +491,21 @@ func TestCursorInventoryAdditionPropagates(t *testing.T) { t.Errorf("the grown plan carries no agent file at %s", wantPath) } } + +// The plan carries exactly one trigger: the sessionStart hook entry in +// ~/.cursor/hooks.json, at the exact target harness.CursorHookTarget names. +func TestCursorPlanTrigger(t *testing.T) { + want := harness.CursorHookTarget(filepath.Join(fakeHome, ".cursor", "hooks.json")) + var got []install.Target + for _, tg := range planFixture(t) { + if tg.Role == harness.TriggerRole { + got = append(got, tg) + } + } + if len(got) != 1 { + t.Fatalf("plan carries %d trigger-role targets, want 1: %+v", len(got), got) + } + if !reflect.DeepEqual(got[0], want) { + t.Errorf("trigger = %+v, want %+v", got[0], want) + } +} diff --git a/internal/harness/export_test.go b/internal/harness/export_test.go new file mode 100644 index 000000000..9938ffa4f --- /dev/null +++ b/internal/harness/export_test.go @@ -0,0 +1,6 @@ +package harness + +// DispatchPreambleForTest exposes the unexported routing rule to the external +// test package, so the trigger guard's non-vacuity check can name a line the +// rule population must contain without spelling it a second time. +const DispatchPreambleForTest = dispatchPreamble diff --git a/internal/harness/opencode/opencode.go b/internal/harness/opencode/opencode.go index 30d355627..d3b8dcf01 100644 --- a/internal/harness/opencode/opencode.go +++ b/internal/harness/opencode/opencode.go @@ -1,6 +1,7 @@ // Package opencode renders Docket's installation for opencode: skill -// symlinks and one native agent definition per agent source, all under the XDG -// config root. It no longer plans a user-global dispatch block (change +// symlinks, one native agent definition per agent source, and one content-free +// trigger (the private-instructions system-prompt plugin, +// harness.OpenCodePluginTarget), all under the XDG config root. It no longer plans a user-global dispatch block (change // 0351) — parent-facing routing belongs to a repository's own AGENTS.md, not a // personal global one — but it still exports GlobalDispatchTarget so the // installer can retire a leftover a prior install owns. It plans only: nothing @@ -35,6 +36,7 @@ const ( skillsDir = "skills" agentsDir = "agents" dispatchFile = "AGENTS.md" + pluginsDir = "plugins" // The managed block docket owns in the dispatch file. Both spellings are // part of the installed-state identity: install.InspectTarget finds the @@ -133,6 +135,8 @@ func (adapter) Plan(in harness.PlanInput) ([]install.Target, error) { }) } + targets = append(targets, harness.OpenCodePluginTarget(filepath.Join(root, pluginsDir, harness.OpenCodePluginFile))) + sort.Slice(targets, func(i, j int) bool { return targets[i].Path < targets[j].Path }) return targets, nil } diff --git a/internal/harness/opencode/opencode_test.go b/internal/harness/opencode/opencode_test.go index a3795ad98..2b115165c 100644 --- a/internal/harness/opencode/opencode_test.go +++ b/internal/harness/opencode/opencode_test.go @@ -637,3 +637,21 @@ func TestOpencodeInventoryAdditionPropagates(t *testing.T) { t.Errorf("the grown plan carries no agent file at %s", wantPath) } } + +// The plan carries exactly one trigger: the system-prompt plugin under the XDG +// config root, at the exact target harness.OpenCodePluginTarget names. +func TestOpencodePlanTrigger(t *testing.T) { + want := harness.OpenCodePluginTarget(filepath.Join(fakeConfig, "opencode", "plugins", "dckt-instructions.js")) + var got []install.Target + for _, tg := range planFixture(t) { + if tg.Role == harness.TriggerRole { + got = append(got, tg) + } + } + if len(got) != 1 { + t.Fatalf("plan carries %d trigger-role targets, want 1: %+v", len(got), got) + } + if !reflect.DeepEqual(got[0], want) { + t.Errorf("trigger = %+v, want %+v", got[0], want) + } +} diff --git a/internal/harness/trigger.go b/internal/harness/trigger.go new file mode 100644 index 000000000..2575d5bd3 --- /dev/null +++ b/internal/harness/trigger.go @@ -0,0 +1,70 @@ +package harness + +import "github.com/danielhanold/docket/internal/install" + +// User-level triggers: content-free by design — no rule text, no "docket". +// +// A private repository keeps its parent-facing rules in its own git directory, +// and each harness reaches them through one trigger installed once per machine. +// A trigger only runs `dckt instructions`, which prints nothing outside a +// private repository, so a trigger can sit on every machine without carrying a +// rule that could drift or a name that could leak. +// TestTriggersCarryNoRuleTextAndNoDocket renders every byte below through its +// producer and enforces both properties. +const ( + // TriggerRole is the installed-state role every trigger target carries. + TriggerRole = "trigger" + + // The Claude Code SessionStart commands, split by section so each output + // fits Claude Code's per-hook character cap. + ClaudeDispatchHookCommand = "dckt instructions --hook claude --section dispatch" + ClaudeLessonsHookCommand = "dckt instructions --hook claude --section lessons" + + // CursorHookCommand is Cursor's single sessionStart command. + CursorHookCommand = "dckt instructions --hook cursor" + + // OpenCodePluginFile is the plugin's base name under opencode's plugins + // directory, and OpenCodePlugin its exact bytes. + OpenCodePluginFile = "dckt-instructions.js" + OpenCodePlugin = "// Adds a private repository's instructions to the system prompt.\n" + + "// `dckt instructions` prints nothing outside a private repository.\n" + + "export const DcktInstructions = async ({ $, directory }) => ({\n" + + " \"experimental.chat.system.transform\": async (_input, output) => {\n" + + " const text = await $`dckt instructions`.cwd(directory).quiet().nothrow().text()\n" + + " if (text.trim()) output.system.push(text)\n" + + " },\n" + + "})\n" + + // The Codex pointer: a managed block in the neutral dckt: namespace asking + // the model to run `dckt instructions` once per session (best effort). + PointerBlockName = "dckt:private-instructions" + PointerBlockAnnotation = "managed — do not hand-edit" + PointerInterior = "## Private repository instructions\n\n" + + "At the start of a session inside a git repository, run `dckt instructions` once.\n" + + "If it prints anything, treat that output as this repository's own AGENTS.md and\n" + + "follow it for the rest of the session. If it prints nothing, ignore this section.\n" +) + +// ClaudeHookTarget is the Claude Code trigger: both SessionStart commands as +// one hook-entries target in settings.json (one target per hooks file). +func ClaudeHookTarget(settingsPath string) install.Target { + return install.Target{Path: settingsPath, Kind: install.KindHookEntries, HookDialect: install.HookDialectClaude, + HookCommands: []string{ClaudeDispatchHookCommand, ClaudeLessonsHookCommand}, Role: TriggerRole} +} + +// CursorHookTarget is the Cursor trigger: the sessionStart command in hooks.json. +func CursorHookTarget(hooksPath string) install.Target { + return install.Target{Path: hooksPath, Kind: install.KindHookEntries, HookDialect: install.HookDialectCursor, + HookCommands: []string{CursorHookCommand}, Role: TriggerRole} +} + +// OpenCodePluginTarget is the OpenCode trigger: a wholly owned plugin file. +func OpenCodePluginTarget(pluginPath string) install.Target { + return install.Target{Path: pluginPath, Kind: install.KindFile, Content: []byte(OpenCodePlugin), Role: TriggerRole} +} + +// CodexPointerTarget is the Codex trigger: the pointer block in AGENTS.md. +func CodexPointerTarget(agentsPath string) install.Target { + return install.Target{Path: agentsPath, Kind: install.KindManagedBlock, BlockName: PointerBlockName, + Annotation: PointerBlockAnnotation, Content: []byte(PointerInterior), Role: TriggerRole} +} diff --git a/internal/harness/trigger_test.go b/internal/harness/trigger_test.go new file mode 100644 index 000000000..52de4b649 --- /dev/null +++ b/internal/harness/trigger_test.go @@ -0,0 +1,147 @@ +// This file is package harness_test so its rule-population helper is shared +// with cross_harness_test.go, which must live outside package harness (it +// plans with all four adapter packages, each of which imports harness). +package harness_test + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/assets" + "github.com/danielhanold/docket/internal/document" + "github.com/danielhanold/docket/internal/harness" + "github.com/danielhanold/docket/internal/install" +) + +// rulePopulation is every line of parent-facing rule text docket renders into a +// dispatch surface: the shared dispatch interior over the real run tracker, +// plus the Codex root-entry clause. A line is a trimmed line of at least 20 +// characters, so headings and prose both count while blank and bracket-only +// lines do not. The population is computed from the producers, never written +// down, so a rule added to either producer joins it automatically. +func rulePopulation(t *testing.T) []string { + t.Helper() + c, err := assets.EmbeddedCatalog() + if err != nil { + t.Fatalf("EmbeddedCatalog: %v", err) + } + rt, err := harness.RunTracker(c) + if err != nil { + t.Fatalf("RunTracker: %v", err) + } + var lines []string + for _, src := range []string{harness.DispatchInterior(rt), harness.CodexRootEntryClause} { + for _, line := range strings.Split(src, "\n") { + if line = strings.TrimSpace(line); len(line) >= 20 { + lines = append(lines, line) + } + } + } + // Non-vacuity: a population that shrank to nothing would let every + // rendering pass. It must be substantial and must include the routing + // rule's first line, named through the producer rather than respelled. + first := strings.TrimSpace(strings.SplitN(harness.DispatchPreambleForTest, "\n", 2)[0]) + if len(lines) < 10 { + t.Fatalf("the rule population holds %d lines; the guard would be vacuous", len(lines)) + } + found := false + for _, l := range lines { + if l == first { + found = true + } + } + if !found { + t.Fatalf("the rule population lacks the routing rule's first line %q", first) + } + return lines +} + +// assertNoRuleText fails when rendered carries any rule-population line. +func assertNoRuleText(t *testing.T, label, rendered string, population []string) { + t.Helper() + for _, line := range population { + if strings.Contains(rendered, line) { + t.Errorf("%s carries rule text %q", label, line) + } + } +} + +// renderedTriggers renders every trigger byte through the producer that writes +// it, keyed by a label: the three hook commands, the hooks file install creates +// for each dialect, the OpenCode plugin, and the Codex pointer block as the +// installer would insert it. +func renderedTriggers(t *testing.T) map[string]string { + t.Helper() + claudeT := harness.ClaudeHookTarget("/h/.claude/settings.json") + cursorT := harness.CursorHookTarget("/h/.cursor/hooks.json") + + doc, err := document.Parse(nil) + if err != nil { + t.Fatalf("Parse(nil): %v", err) + } + var p document.PatchSet + p.InsertBlock(harness.PointerBlockName, harness.PointerBlockAnnotation, harness.PointerInterior, document.AtDocumentStart) + pointer, err := doc.Apply(p) + if err != nil { + t.Fatalf("rendering the pointer block: %v", err) + } + if !strings.Contains(string(pointer), "## Private repository instructions") { + t.Fatalf("the rendered pointer block lost its interior:\n%s", pointer) + } + + return map[string]string{ + "claude dispatch hook command": harness.ClaudeDispatchHookCommand, + "claude lessons hook command": harness.ClaudeLessonsHookCommand, + "cursor hook command": harness.CursorHookCommand, + "claude hooks file": string(install.NewHookFileBytes(claudeT.HookDialect, claudeT.HookCommands)), + "cursor hooks file": string(install.NewHookFileBytes(cursorT.HookDialect, cursorT.HookCommands)), + "opencode plugin": string(harness.OpenCodePluginTarget("/h/.config/opencode/plugins/x.js").Content), + "codex pointer block": string(pointer), + } +} + +// TestTriggersCarryNoRuleTextAndNoDocket is the content-free property of every +// user-level trigger: each one only runs `dckt instructions`, so none may carry +// a line of the rules it delivers, and none may say "docket" (a trigger lands +// on every machine, private repository or not). +func TestTriggersCarryNoRuleTextAndNoDocket(t *testing.T) { + population := rulePopulation(t) + rendered := renderedTriggers(t) + for label, body := range rendered { + if body == "" { + t.Errorf("%s rendered empty; the guard would be vacuous", label) + } + if strings.Contains(strings.ToLower(body), "docket") { + t.Errorf("%s says docket:\n%s", label, body) + } + assertNoRuleText(t, label, body, population) + } +} + +// TestTriggerTargets pins each trigger target's shape: the kind, the dialect, +// the commands in order, the content, and the shared role. +func TestTriggerTargets(t *testing.T) { + c := harness.ClaudeHookTarget("/h/.claude/settings.json") + if c.Kind != install.KindHookEntries || c.HookDialect != install.HookDialectClaude || c.Role != harness.TriggerRole || + len(c.HookCommands) != 2 || c.HookCommands[0] != harness.ClaudeDispatchHookCommand || c.HookCommands[1] != harness.ClaudeLessonsHookCommand { + t.Errorf("claude hook target = %+v", c) + } + u := harness.CursorHookTarget("/h/.cursor/hooks.json") + if u.Kind != install.KindHookEntries || u.HookDialect != install.HookDialectCursor || u.Role != harness.TriggerRole || + len(u.HookCommands) != 1 || u.HookCommands[0] != harness.CursorHookCommand { + t.Errorf("cursor hook target = %+v", u) + } + o := harness.OpenCodePluginTarget("/h/p.js") + if o.Kind != install.KindFile || string(o.Content) != harness.OpenCodePlugin || o.Role != harness.TriggerRole { + t.Errorf("opencode plugin target = %+v", o) + } + x := harness.CodexPointerTarget("/h/.codex/AGENTS.md") + if x.Kind != install.KindManagedBlock || x.BlockName != harness.PointerBlockName || x.Annotation != harness.PointerBlockAnnotation || + string(x.Content) != harness.PointerInterior || x.Role != harness.TriggerRole { + t.Errorf("codex pointer target = %+v", x) + } + if filepath.Base(harness.OpenCodePluginFile) != harness.OpenCodePluginFile { + t.Errorf("OpenCodePluginFile %q is not a bare file name", harness.OpenCodePluginFile) + } +} diff --git a/internal/install/hook_entries.go b/internal/install/hook_entries.go new file mode 100644 index 000000000..9c56ec0ba --- /dev/null +++ b/internal/install/hook_entries.go @@ -0,0 +1,608 @@ +package install + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "reflect" + "strings" +) + +// A hook-entries target owns a fixed list of commands inside one JSON hooks +// file the user also owns: Claude Code's settings.json or Cursor's hooks.json. +// Each command is one entry under hooks., and an entry is identified by +// its exact command; every other byte in the file is the user's. +// +// One target covers every command in a file, never one target per command: +// the installed state holds one record per path (ValidateState), and a +// transaction can carry only one removal per path (applySteps re-verifies each +// later removal's captured pre-image, which an earlier removal on the same file +// has already rewritten). +// +// Every edit is a byte splice. Nothing in the file is decoded and re-encoded: +// an insert adds an entry after the last one in its container, and a removal +// cuts exactly the bytes an insert added, so install-then-uninstall returns the +// file the user had. encoding/json is used only to read the file, and only a +// file it reads as an object whose hooks value is an object and whose event +// value is an array is ever edited; anything else is a conflict, never +// "entries absent". + +// The hook dialects: which hooks-file shape a hook-entries target edits. +const ( + HookDialectClaude = "claude" + HookDialectCursor = "cursor" +) + +var errHookFileInvalid = errors.New("install: hooks file is not a JSON object whose hooks value is an object and whose event value is an array") + +// hookEvent is the hooks. key a dialect's session-start entries live +// under. +func hookEvent(dialect string) (string, bool) { + switch dialect { + case HookDialectClaude: + return "SessionStart", true + case HookDialectCursor: + return "sessionStart", true + default: + return "", false + } +} + +// claudeHookGroup is a Claude Code matcher group with no matcher, so every +// session source fires it. +type claudeHookGroup struct { + Hooks []claudeHookHandler `json:"hooks"` +} + +type claudeHookHandler struct { + Type string `json:"type"` + Command string `json:"command"` +} + +type cursorHookEntry struct { + Command string `json:"command"` +} + +// canonicalHookEntry is the entry install writes for command. +func canonicalHookEntry(dialect, command string) any { + if dialect == HookDialectCursor { + return cursorHookEntry{Command: command} + } + return claudeHookGroup{Hooks: []claudeHookHandler{{Type: "command", Command: command}}} +} + +// checkHookFields is the structural check a hook-entries target and its record +// share: a known dialect and a non-empty list of distinct, non-empty commands. +func checkHookFields(dialect string, commands []string) error { + if _, ok := hookEvent(dialect); !ok { + return fmt.Errorf("unknown hook dialect %q", dialect) + } + if len(commands) == 0 { + return errors.New("no hook commands") + } + seen := make(map[string]bool, len(commands)) + for _, c := range commands { + if c == "" { + return errors.New("an empty hook command") + } + if seen[c] { + return fmt.Errorf("duplicate hook command %q", c) + } + seen[c] = true + } + return nil +} + +// hookEntriesDigest is a hook-entries record's identity: the dialect and the +// ordered commands it owns. +func hookEntriesDigest(dialect string, commands []string) string { + data, err := json.Marshal(struct { + Dialect string `json:"dialect"` + Commands []string `json:"commands"` + }{dialect, commands}) + if err != nil { + // A struct of strings always encodes. + panic("install: encoding a hook-entries identity: " + err.Error()) + } + return hashBytes(data) +} + +// NewHookFileBytes is the file install writes when the hooks file is absent: +// one canonical entry per command, and for Cursor the "version": 1 its hooks +// file requires, ahead of "hooks". An unknown dialect has no file. +func NewHookFileBytes(dialect string, commands []string) []byte { + event, ok := hookEvent(dialect) + if !ok { + return nil + } + entries := make([]any, 0, len(commands)) + for _, c := range commands { + entries = append(entries, canonicalHookEntry(dialect, c)) + } + hooks := map[string][]any{event: entries} + var doc any = struct { + Hooks map[string][]any `json:"hooks"` + }{hooks} + if dialect == HookDialectCursor { + doc = struct { + Version int `json:"version"` + Hooks map[string][]any `json:"hooks"` + }{1, hooks} + } + out, err := json.MarshalIndent(doc, "", " ") + if err != nil { + panic("install: encoding a new hooks file: " + err.Error()) + } + return append(out, '\n') +} + +// readHookEntries returns the event entries. ok=false: not editable — never +// "absent". A whitespace-only file is editable and empty. +// +// It reads the way encoding/json does, so a duplicated key resolves to its +// last occurrence; findMember follows the same rule, which keeps an edit inside +// the value this function read. +func readHookEntries(src []byte, dialect string) (entries []json.RawMessage, ok bool) { + event, known := hookEvent(dialect) + if !known { + return nil, false + } + if len(bytes.TrimSpace(src)) == 0 { + return nil, true + } + var top map[string]json.RawMessage + if json.Unmarshal(src, &top) != nil || top == nil { + return nil, false + } + raw, has := top["hooks"] + if !has { + return nil, true + } + var hooks map[string]json.RawMessage + if json.Unmarshal(raw, &hooks) != nil || hooks == nil { + return nil, false + } + ev, has := hooks[event] + if !has { + return nil, true + } + if !bytes.HasPrefix(bytes.TrimSpace(ev), []byte("[")) || json.Unmarshal(ev, &entries) != nil { + return nil, false + } + return entries, true +} + +// entryHasCommand reports whether an entry runs command: for Claude, any +// handler in the group's hooks; for Cursor, the entry itself. A value that does +// not decode runs nothing. +func entryHasCommand(raw json.RawMessage, dialect, command string) bool { + switch dialect { + case HookDialectClaude: + var group map[string]json.RawMessage + if json.Unmarshal(raw, &group) != nil { + return false + } + var handlers []json.RawMessage + if json.Unmarshal(group["hooks"], &handlers) != nil { + return false + } + for _, h := range handlers { + if commandIs(h, command) { + return true + } + } + return false + case HookDialectCursor: + return commandIs(raw, command) + default: + return false + } +} + +// commandIs reports whether raw is an object whose command is exactly command. +func commandIs(raw json.RawMessage, command string) bool { + var obj map[string]json.RawMessage + if json.Unmarshal(raw, &obj) != nil { + return false + } + c, has := obj["command"] + if !has { + return false + } + var s string + return json.Unmarshal(c, &s) == nil && s == command +} + +// anyEntryRuns reports whether some entry runs command. +func anyEntryRuns(entries []json.RawMessage, dialect, command string) bool { + for _, e := range entries { + if entryHasCommand(e, dialect, command) { + return true + } + } + return false +} + +// isExactEntry reports whether raw is, as JSON, exactly the canonical entry for +// command — however it is spelled. An entry the user has added a field to is +// theirs now. +func isExactEntry(raw json.RawMessage, dialect, command string) bool { + var have any + if json.Unmarshal(raw, &have) != nil { + return false + } + canon, err := json.Marshal(canonicalHookEntry(dialect, command)) + if err != nil { + return false + } + var want any + if json.Unmarshal(canon, &want) != nil { + return false + } + return reflect.DeepEqual(have, want) +} + +// hookEntriesState is what a recorded hook-entries target finds in its file: +// whether the file is editable, whether any entry runs a recorded command, and +// whether every entry that does is exactly docket's. +func hookEntriesState(src []byte, dialect string, commands []string) (editable, anyPresent, allExact bool) { + entries, ok := readHookEntries(src, dialect) + if !ok { + return false, false, false + } + allExact = true + for _, e := range entries { + for _, c := range commands { + if entryHasCommand(e, dialect, c) { + anyPresent = true + if !isExactEntry(e, dialect, c) { + allExact = false + } + } + } + } + return true, anyPresent, allExact +} + +// insertHookEntries adds the canonical entry for every command no entry runs +// yet, in order, each as the last element of the event array. A file needing +// nothing comes back unchanged, and a blank one becomes NewHookFileBytes. +func insertHookEntries(src []byte, dialect string, commands []string) ([]byte, error) { + entries, ok := readHookEntries(src, dialect) + if !ok { + return nil, errHookFileInvalid + } + var missing []string + for _, c := range commands { + if !anyEntryRuns(entries, dialect, c) && !containsString(missing, c) { + missing = append(missing, c) + } + } + if len(missing) == 0 { + return src, nil + } + if len(bytes.TrimSpace(src)) == 0 { + return NewHookFileBytes(dialect, missing), nil + } + event, _ := hookEvent(dialect) + unit := indentUnit(src) + out := src + for _, c := range missing { + next, err := insertHookEntry(out, event, unit, canonicalHookEntry(dialect, c)) + if err != nil { + return nil, err + } + out = next + } + // The splices assume a valid input and keep it valid; a result that does not + // read back is a defect in them, and must never reach the user's file. + if _, ok := readHookEntries(out, dialect); !ok { + return nil, fmt.Errorf("install: adding hook entries produced an unreadable file") + } + return out, nil +} + +// removeHookEntries cuts every exact entry for commands, last command first — +// the reverse of insertion, so a round trip splices back the original bytes. +// An event array docket's removal empties is removed with its member, and a +// hooks object that empties is removed with its member too. An entry that runs +// a command but is not exact is left where it is. +func removeHookEntries(src []byte, dialect string, commands []string) ([]byte, error) { + if _, ok := readHookEntries(src, dialect); !ok { + return nil, errHookFileInvalid + } + event, _ := hookEvent(dialect) + out := src + for i := len(commands) - 1; i >= 0; i-- { + for { + next, removed := removeHookEntry(out, event, dialect, commands[i]) + if !removed { + break + } + out = next + } + } + if _, ok := readHookEntries(out, dialect); !ok { + return nil, fmt.Errorf("install: removing hook entries produced an unreadable file") + } + return out, nil +} + +// insertHookEntry splices one entry in as the last element of hooks., +// creating the event member or the hooks member when absent, as the last member +// of its parent. src must be a valid JSON object (readHookEntries said so). +func insertHookEntry(src []byte, event, unit string, entry any) ([]byte, error) { + top := skipSpace(src, 0) + members, topClose := objectMembers(src, top) + hi, ok := findMember(members, "hooks") + if !ok { + return spliceLast(src, top, topClose, memberSpans(members), 1, unit, func(pad string) ([]byte, error) { + return memberText("hooks", map[string][]any{event: {entry}}, pad, unit) + }) + } + hooks := members[hi] + hmembers, hooksClose := objectMembers(src, hooks.value.start) + ei, ok := findMember(hmembers, event) + if !ok { + return spliceLast(src, hooks.value.start, hooksClose, memberSpans(hmembers), 2, unit, func(pad string) ([]byte, error) { + return memberText(event, []any{entry}, pad, unit) + }) + } + ev := hmembers[ei] + elems, arrayClose := arrayElements(src, ev.value.start) + return spliceLast(src, ev.value.start, arrayClose, elems, 3, unit, func(pad string) ([]byte, error) { + return json.MarshalIndent(entry, pad, unit) + }) +} + +// removeHookEntry cuts the first exact entry for command. When it is the only +// element of its array, the event member goes instead, and when that member is +// the only one in hooks, the hooks member goes. +func removeHookEntry(src []byte, event, dialect, command string) ([]byte, bool) { + top := skipSpace(src, 0) + if top >= len(src) || src[top] != '{' { + return src, false + } + members, topClose := objectMembers(src, top) + hi, ok := findMember(members, "hooks") + if !ok { + return src, false + } + hooks := members[hi] + hmembers, hooksClose := objectMembers(src, hooks.value.start) + ei, ok := findMember(hmembers, event) + if !ok { + return src, false + } + ev := hmembers[ei] + elems, arrayClose := arrayElements(src, ev.value.start) + k := -1 + for i, e := range elems { + if isExactEntry(src[e.start:e.end], dialect, command) { + k = i + break + } + } + switch { + case k < 0: + return src, false + case len(elems) > 1: + return cutElement(src, ev.value.start, arrayClose, elems, k), true + case len(hmembers) > 1: + return cutElement(src, hooks.value.start, hooksClose, memberSpans(hmembers), ei), true + default: + return cutElement(src, top, topClose, memberSpans(members), hi), true + } +} + +// spliceLast adds rendered text as the last element of the container opening +// at open and closing at close, whose elements sit at depth indent units. +// After an existing element it writes ",\n" + pad + text; into an empty +// container it replaces the (whitespace-only) interior with "\n" + pad + text + +// "\n" + the container's own pad. +func spliceLast(src []byte, open, close int, elems []jsonSpan, depth int, unit string, render func(pad string) ([]byte, error)) ([]byte, error) { + pad := strings.Repeat(unit, depth) + text, err := render(pad) + if err != nil { + return nil, fmt.Errorf("install: encoding a hook entry: %w", err) + } + if len(elems) == 0 { + return concatBytes(src[:open+1], []byte("\n"+pad), text, []byte("\n"+strings.Repeat(unit, depth-1)), src[close:]), nil + } + at := elems[len(elems)-1].end + return concatBytes(src[:at], []byte(",\n"+pad), text, src[at:]), nil +} + +// cutElement removes element k of a container's elements. The only element +// empties the container; a later one goes with the separator before it; the +// first of several goes with the separator after it. +func cutElement(src []byte, open, close int, elems []jsonSpan, k int) []byte { + switch { + case len(elems) == 1: + return concatBytes(src[:open+1], src[close:]) + case k > 0: + return concatBytes(src[:elems[k-1].end], src[elems[k].end:]) + default: + return concatBytes(src[:elems[0].start], src[elems[1].start:]) + } +} + +// memberText renders `"key": value` with value indented for pad. +func memberText(key string, value any, pad, unit string) ([]byte, error) { + k, err := json.Marshal(key) + if err != nil { + return nil, err + } + v, err := json.MarshalIndent(value, pad, unit) + if err != nil { + return nil, err + } + return concatBytes(k, []byte(": "), v), nil +} + +// indentUnit is the file's indent unit: the leading whitespace of the first +// indented line after the first, else two spaces. +func indentUnit(src []byte) string { + lines := bytes.Split(src, []byte("\n")) + for _, line := range lines[1:] { + rest := bytes.TrimLeft(line, " \t") + if len(rest) == len(line) || len(bytes.TrimSpace(rest)) == 0 { + continue + } + return string(line[:len(line)-len(rest)]) + } + return " " +} + +func concatBytes(parts ...[]byte) []byte { + n := 0 + for _, p := range parts { + n += len(p) + } + out := make([]byte, 0, n) + for _, p := range parts { + out = append(out, p...) + } + return out +} + +func containsString(list []string, s string) bool { + for _, v := range list { + if v == s { + return true + } + } + return false +} + +// --- the byte-span scanner --------------------------------------------------- +// +// The scanner runs only over input readHookEntries accepted, so it is valid +// JSON with an object at the top; that is what lets it skip error handling. + +// jsonSpan is a half-open byte range. +type jsonSpan struct{ start, end int } + +// jsonMember is one object member: its decoded key, its whole span (the key's +// opening quote through the value's end), and its value's span. +type jsonMember struct { + key string + span jsonSpan + value jsonSpan +} + +func isJSONSpace(c byte) bool { return c == ' ' || c == '\t' || c == '\n' || c == '\r' } + +func skipSpace(src []byte, i int) int { + for i < len(src) && isJSONSpace(src[i]) { + i++ + } + return i +} + +// valueEnd is the index just past the value starting at i. +func valueEnd(src []byte, i int) int { + switch src[i] { + case '"': + return stringEnd(src, i) + case '{', '[': + depth := 0 + for j := i; j < len(src); j++ { + switch src[j] { + case '"': + j = stringEnd(src, j) - 1 + case '{', '[': + depth++ + case '}', ']': + depth-- + if depth == 0 { + return j + 1 + } + } + } + return len(src) + default: + j := i + for j < len(src) && strings.IndexByte(",}] \t\r\n", src[j]) < 0 { + j++ + } + return j + } +} + +// stringEnd is the index just past the string whose opening quote is at i. +func stringEnd(src []byte, i int) int { + for j := i + 1; j < len(src); j++ { + switch src[j] { + case '\\': + j++ + case '"': + return j + 1 + } + } + return len(src) +} + +// objectMembers lists the members of the object opening at open, and the index +// of its closing brace. +func objectMembers(src []byte, open int) ([]jsonMember, int) { + var members []jsonMember + i := skipSpace(src, open+1) + if i < len(src) && src[i] == '}' { + return nil, i + } + for i < len(src) { + keyEnd := valueEnd(src, i) + var key string + _ = json.Unmarshal(src[i:keyEnd], &key) + vs := skipSpace(src, skipSpace(src, keyEnd)+1) // past the colon + ve := valueEnd(src, vs) + members = append(members, jsonMember{key: key, span: jsonSpan{i, ve}, value: jsonSpan{vs, ve}}) + j := skipSpace(src, ve) + if j >= len(src) || src[j] == '}' { + return members, j + } + i = skipSpace(src, j+1) // past the comma + } + return members, len(src) +} + +// arrayElements lists the element spans of the array opening at open, and the +// index of its closing bracket. +func arrayElements(src []byte, open int) ([]jsonSpan, int) { + var elems []jsonSpan + i := skipSpace(src, open+1) + if i < len(src) && src[i] == ']' { + return nil, i + } + for i < len(src) { + end := valueEnd(src, i) + elems = append(elems, jsonSpan{i, end}) + j := skipSpace(src, end) + if j >= len(src) || src[j] == ']' { + return elems, j + } + i = skipSpace(src, j+1) + } + return elems, len(src) +} + +// findMember returns the index of the LAST member named key, matching +// encoding/json's last-duplicate-wins rule that readHookEntries used. +func findMember(members []jsonMember, key string) (int, bool) { + for i := len(members) - 1; i >= 0; i-- { + if members[i].key == key { + return i, true + } + } + return -1, false +} + +func memberSpans(members []jsonMember) []jsonSpan { + out := make([]jsonSpan, len(members)) + for i, m := range members { + out[i] = m.span + } + return out +} diff --git a/internal/install/hook_entries_test.go b/internal/install/hook_entries_test.go new file mode 100644 index 000000000..3c0413400 --- /dev/null +++ b/internal/install/hook_entries_test.go @@ -0,0 +1,971 @@ +package install + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/danielhanold/docket/internal/assets" + "github.com/danielhanold/docket/internal/buildinfo" + "github.com/danielhanold/docket/internal/config" + "github.com/danielhanold/docket/internal/testsupport" +) + +// The commands each dialect's tests install. Claude carries two, so a removal +// has to undo two splices in one array; Cursor carries one. +var ( + claudeHookCmds = []string{"a --x", "a --y"} + cursorHookCmds = []string{"a --z"} +) + +func hookCmdsFor(dialect string) []string { + if dialect == HookDialectCursor { + return cursorHookCmds + } + return claudeHookCmds +} + +// hookOriginal is one user hooks file docket did not write. +type hookOriginal struct { + name string + src string +} + +func hookOriginals(dialect string) []hookOriginal { + common := []hookOriginal{ + {"empty object", "{}\n"}, + {"unrelated key", "{\n \"model\": \"opus\"\n}\n"}, + {"tab indented", "{\n\t\"env\": {\n\t\t\"A\": \"1\"\n\t}\n}\n"}, + {"compact one line", `{"model":"opus"}`}, + // The scanner must step over quotes, brackets, and escapes inside + // strings, and over every scalar shape. + {"tricky values", "{\n \"s\": \"br]ack}e\\\"t\\\\\",\n \"n\": -1.5e3,\n \"t\": true,\n \"z\": null,\n \"a\": [[1], {\"k\": []}]\n}\n"}, + {"crlf line endings", "{\r\n \"model\": \"opus\"\r\n}\r\n"}, + // encoding/json reads an escaped key as its unescaped spelling, so the + // splice must find it under the same name. + {"escaped hooks key", "{\"ho\\u006bks\": {\"Stop\": []}}\n"}, + } + if dialect == HookDialectCursor { + return append(common, + hookOriginal{"another event", "{\n \"version\": 1,\n \"hooks\": {\n \"afterFileEdit\": [\n {\n \"command\": \"./fmt.sh\"\n }\n ]\n }\n}\n"}, + hookOriginal{"user entry in the event", "{\n \"version\": 1,\n \"hooks\": {\n \"sessionStart\": [\n {\"command\": \"./mine.sh\"}\n ]\n }\n}\n"}, + hookOriginal{"duplicate hooks keys", "{\n \"version\": 1,\n \"hooks\": {},\n \"model\": \"x\",\n \"hooks\": {\n \"afterFileEdit\": [{\"command\": \"./fmt.sh\"}]\n }\n}\n"}, + ) + } + return append(common, + hookOriginal{"another event", "{\n \"hooks\": {\n \"Stop\": [\n {\n \"hooks\": [\n {\n \"type\": \"command\",\n \"command\": \"./stop.sh\"\n }\n ]\n }\n ]\n }\n}\n"}, + hookOriginal{"user group with a matcher", "{\n \"hooks\": {\n \"SessionStart\": [\n {\n \"matcher\": \"startup\",\n \"hooks\": [{\"type\": \"command\", \"command\": \"./mine.sh\"}]\n }\n ]\n }\n}\n"}, + hookOriginal{"duplicate hooks keys", "{\n \"hooks\": {},\n \"model\": \"opus\",\n \"hooks\": {\n \"Stop\": [{\"hooks\": [{\"type\": \"command\", \"command\": \"./stop.sh\"}]}]\n }\n}\n"}, + ) +} + +// userEntrySeed is the original whose event array already holds a user entry. +func userEntrySeed(t *testing.T, dialect string) string { + t.Helper() + for _, o := range hookOriginals(dialect) { + if o.name == "user entry in the event" || o.name == "user group with a matcher" { + return o.src + } + } + t.Fatalf("no user-entry original for %s", dialect) + return "" +} + +// countEntries counts the entries satisfying pred. +func countEntries(entries []json.RawMessage, pred func(json.RawMessage) bool) int { + n := 0 + for _, e := range entries { + if pred(e) { + n++ + } + } + return n +} + +// decodedSet is every entry decoded, so two spellings of one entry compare equal. +func decoded(t *testing.T, entries []json.RawMessage) []any { + t.Helper() + out := make([]any, 0, len(entries)) + for _, e := range entries { + var v any + if err := json.Unmarshal(e, &v); err != nil { + t.Fatalf("decoding entry %s: %v", e, err) + } + out = append(out, v) + } + return out +} + +// otherSettings is the top-level document without its hooks value: every +// setting docket never owns. +func otherSettings(t *testing.T, src []byte) map[string]any { + t.Helper() + top := map[string]any{} + if len(bytes.TrimSpace(src)) == 0 { + return top + } + if err := json.Unmarshal(src, &top); err != nil { + t.Fatalf("decoding %q: %v", src, err) + } + delete(top, "hooks") + return top +} + +func TestInsertHookEntriesRoundTrips(t *testing.T) { + for _, dialect := range []string{HookDialectClaude, HookDialectCursor} { + cmds := hookCmdsFor(dialect) + for _, orig := range hookOriginals(dialect) { + t.Run(dialect+"/"+orig.name, func(t *testing.T) { + src := []byte(orig.src) + before, ok := readHookEntries(src, dialect) + if !ok { + t.Fatalf("the original is not editable: %q", src) + } + + ins, err := insertHookEntries(src, dialect, cmds) + if err != nil { + t.Fatalf("insertHookEntries: %v", err) + } + if !json.Valid(ins) { + t.Fatalf("inserted bytes are not valid JSON:\n%s", ins) + } + if !bytes.Equal(src, []byte(orig.src)) { + t.Fatalf("insertHookEntries mutated its input") + } + after, ok := readHookEntries(ins, dialect) + if !ok { + t.Fatalf("the inserted file is not editable:\n%s", ins) + } + for _, c := range cmds { + if n := countEntries(after, func(e json.RawMessage) bool { return isExactEntry(e, dialect, c) }); n != 1 { + t.Errorf("%d exact entries run %q, want 1:\n%s", n, c, ins) + } + } + if len(after) != len(before)+len(cmds) { + t.Errorf("%d entries after insert, want %d + %d:\n%s", len(after), len(before), len(cmds), ins) + } + have := decoded(t, after) + for _, want := range decoded(t, before) { + found := false + for _, h := range have { + if reflect.DeepEqual(h, want) { + found = true + break + } + } + if !found { + t.Errorf("the user's entry %v did not survive:\n%s", want, ins) + } + } + if got, want := otherSettings(t, ins), otherSettings(t, src); !reflect.DeepEqual(got, want) { + t.Errorf("other settings changed: got %v, want %v", got, want) + } + + again, err := insertHookEntries(ins, dialect, cmds) + if err != nil || !bytes.Equal(again, ins) { + t.Errorf("a second insert changed the file (err %v):\n%s", err, again) + } + + rem, err := removeHookEntries(ins, dialect, cmds) + if err != nil { + t.Fatalf("removeHookEntries: %v", err) + } + if string(rem) != orig.src { + t.Errorf("round trip =\n%q\nwant\n%q\n(inserted:\n%s)", rem, orig.src, ins) + } + }) + } + } + + t.Run("cursor pre-existing empty hooks object", func(t *testing.T) { + src := []byte("{\n \"version\": 1,\n \"hooks\": {}\n}\n") + ins, err := insertHookEntries(src, HookDialectCursor, cursorHookCmds) + if err != nil { + t.Fatal(err) + } + rem, err := removeHookEntries(ins, HookDialectCursor, cursorHookCmds) + if err != nil { + t.Fatal(err) + } + if want := "{\n \"version\": 1\n}\n"; string(rem) != want { + t.Errorf("round trip = %q, want %q", rem, want) + } + }) +} + +func TestInsertHookEntriesAddsOnlyMissing(t *testing.T) { + exact := "{\n \"hooks\": {\n \"SessionStart\": [\n {\n \"hooks\": [\n {\n \"type\": \"command\",\n \"command\": \"a --x\"\n }\n ]\n }\n ]\n }\n}\n" + edited := strings.Replace(exact, `"command": "a --x"`, `"command": "a --x", "timeout": 5`, 1) + for name, src := range map[string]string{"exact": exact, "timeout added": edited} { + t.Run(name, func(t *testing.T) { + ins, err := insertHookEntries([]byte(src), HookDialectClaude, claudeHookCmds) + if err != nil { + t.Fatal(err) + } + entries, ok := readHookEntries(ins, HookDialectClaude) + if !ok || len(entries) != 2 { + t.Fatalf("entries = %d (ok %v), want 2:\n%s", len(entries), ok, ins) + } + runsX := countEntries(entries, func(e json.RawMessage) bool { return entryHasCommand(e, HookDialectClaude, "a --x") }) + exactY := countEntries(entries, func(e json.RawMessage) bool { return isExactEntry(e, HookDialectClaude, "a --y") }) + if runsX != 1 || exactY != 1 { + t.Errorf("entries running a --x = %d, exact a --y = %d; want 1 and 1:\n%s", runsX, exactY, ins) + } + if !strings.HasPrefix(string(ins), strings.TrimSuffix(src, "\n ]\n }\n}\n")) { + t.Errorf("the existing entry's bytes were rewritten:\n%s", ins) + } + }) + } +} + +func TestNewHookFileBytes(t *testing.T) { + claude := NewHookFileBytes(HookDialectClaude, claudeHookCmds) + type handler struct { + Type string `json:"type"` + Command string `json:"command"` + } + type group struct { + Hooks []handler `json:"hooks"` + } + doc := struct { + Hooks map[string][]group `json:"hooks"` + }{map[string][]group{"SessionStart": { + {Hooks: []handler{{"command", "a --x"}}}, + {Hooks: []handler{{"command", "a --y"}}}, + }}} + want, err := json.MarshalIndent(doc, "", " ") + if err != nil { + t.Fatal(err) + } + if string(claude) != string(want)+"\n" { + t.Errorf("claude new file =\n%s\nwant\n%s", claude, want) + } + + cursor := NewHookFileBytes(HookDialectCursor, cursorHookCmds) + if !strings.HasPrefix(string(cursor), "{\n \"version\": 1,\n \"hooks\": {") || !strings.HasSuffix(string(cursor), "}\n") { + t.Errorf("cursor new file does not lead with its version:\n%s", cursor) + } + var got, wantCursor any + if err := json.Unmarshal(cursor, &got); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal([]byte(`{"version":1,"hooks":{"sessionStart":[{"command":"a --z"}]}}`), &wantCursor); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, wantCursor) { + t.Errorf("cursor new file decodes to %v, want %v", got, wantCursor) + } + + for _, dialect := range []string{HookDialectClaude, HookDialectCursor} { + for _, blank := range [][]byte{nil, []byte(" \n")} { + got, err := insertHookEntries(blank, dialect, hookCmdsFor(dialect)) + if err != nil || !bytes.Equal(got, NewHookFileBytes(dialect, hookCmdsFor(dialect))) { + t.Errorf("%s: insert into %q = %q (err %v), want the new-file bytes", dialect, blank, got, err) + } + } + } +} + +func hookTarget(path, dialect string) Target { + return Target{Path: path, Kind: KindHookEntries, HookDialect: dialect, + HookCommands: append([]string(nil), hookCmdsFor(dialect)...), Role: "trigger"} +} + +func TestHookEntriesInspect(t *testing.T) { + type tc struct { + name string + seed *string // nil: absent + want Disposition + reason string + remedy string + } + str := func(s string) *string { return &s } + invalid := func(dialect string) []tc { + wrongEvent, nullEvent := `{"hooks": {"SessionStart": {}}}`, `{"hooks": {"SessionStart": null}}` + if dialect == HookDialectCursor { + wrongEvent, nullEvent = `{"hooks": {"sessionStart": "x"}}`, `{"hooks": {"sessionStart": null}}` + } + var out []tc + // A file docket cannot parse is skipped, never a conflict: a conflict + // would fail the whole install over a trigger that is inert outside a + // private repository. + for _, s := range []string{`{"hooks": []}`, wrongEvent, nullEvent, `[1]`, `null`, `not json`, `{"hooks": null}`, `{} {}`, + "\ufeff{}", "{\n // a comment\n}\n"} { + out = append(out, tc{"invalid " + s, str(s), DispositionSkip, "", remedyHookFileInvalid}) + } + return out + } + + for _, dialect := range []string{HookDialectClaude, HookDialectCursor} { + full := string(NewHookFileBytes(dialect, hookCmdsFor(dialect))) + timed := strings.Replace(full, `"command": "`+hookCmdsFor(dialect)[0]+`"`, + `"command": "`+hookCmdsFor(dialect)[0]+`",`+"\n"+`"timeout": 5`, 1) + cases := []tc{ + {"absent", nil, DispositionCreate, "", ""}, + {"none of the commands", str("{\n \"model\": \"opus\"\n}\n"), DispositionUpdate, "", ""}, + {"all present exactly", str(full), DispositionNoop, "", ""}, + {"all present, one with a timeout", str(timed), DispositionNoop, "", ""}, + } + if dialect == HookDialectClaude { + cases = append(cases, tc{"one of two", str(string(NewHookFileBytes(dialect, claudeHookCmds[:1]))), DispositionUpdate, "", ""}) + } + cases = append(cases, invalid(dialect)...) + for _, c := range cases { + t.Run(dialect+"/"+c.name, func(t *testing.T) { + path := filepath.Join(testsupport.TempDir(t), "hooks.json") + if c.seed != nil { + writeFileOrDie(t, path, *c.seed) + } + insp, err := InspectTarget(hookTarget(path, dialect), nil, nil) + if err != nil { + t.Fatalf("InspectTarget: %v", err) + } + if insp.Disposition != c.want || insp.Reason != c.reason || insp.Remedy != c.remedy { + t.Errorf("inspection = %s/%q/%q, want %s/%q/%q", insp.Disposition, insp.Reason, insp.Remedy, c.want, c.reason, c.remedy) + } + if c.seed != nil { + if got := readOrDie(t, path); got != *c.seed { + t.Errorf("inspection changed the file: %q", got) + } + } + }) + } + + t.Run(dialect+"/symlinked file", func(t *testing.T) { + dir := testsupport.TempDir(t) + real := filepath.Join(dir, "dotfiles-hooks.json") + writeFileOrDie(t, real, "{}\n") + path := filepath.Join(dir, "hooks.json") + if err := os.Symlink(real, path); err != nil { + t.Fatal(err) + } + insp, err := InspectTarget(hookTarget(path, dialect), nil, nil) + if err != nil { + t.Fatal(err) + } + // A dotfiles manager's link is skipped, never a conflict: a conflict + // would fail the whole install over a trigger that is inert outside a + // private repository. + if insp.Disposition != DispositionSkip || insp.Reason != "" || insp.Remedy != remedyHookFileNotRegular { + t.Errorf("inspection = %+v, want a skip with the not-a-regular-file remedy", insp) + } + if got := readOrDie(t, real); got != "{}\n" { + t.Errorf("the link's destination changed: %q", got) + } + if fi, err := os.Lstat(path); err != nil || fi.Mode()&os.ModeSymlink == 0 { + t.Errorf("the link was replaced: %v %v", fi, err) + } + }) + + t.Run(dialect+"/directory at the hooks path", func(t *testing.T) { + path := filepath.Join(testsupport.TempDir(t), "hooks.json") + if err := os.Mkdir(path, 0o755); err != nil { + t.Fatal(err) + } + insp, err := InspectTarget(hookTarget(path, dialect), nil, nil) + if err != nil { + t.Fatal(err) + } + if insp.Disposition != DispositionSkip || insp.Remedy != remedyHookFileNotRegular { + t.Errorf("inspection = %+v, want a skip with the not-a-regular-file remedy", insp) + } + }) + } +} + +// hookWorld is a temp home whose single planner (named after the dialect) plans +// one hook-entries target, plus any extra targets, and the matching uninstall. +func hookWorld(t *testing.T, dialect string, extra func(UserRoots) []Target) (Options, UninstallOptions, string) { + t.Helper() + roots := versionRoots(t) + // Hermetic home: nothing below may resolve the real user's configuration. + t.Setenv("HOME", roots.Home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(roots.Home, ".config")) + t.Setenv("XDG_DATA_HOME", filepath.Join(roots.Home, ".local", "share")) + + path := filepath.Join(roots.Home, ".claude", "settings.json") + if dialect == HookDialectCursor { + path = filepath.Join(roots.Home, ".cursor", "hooks.json") + } + var more []Target + if extra != nil { + more = extra(roots) + } + payload := samplePayload() + manifest := sampleManifest(t, payload) + opts := Options{ + Roots: roots, FS: RealFS{}, Config: &config.Snapshot{}, + Catalog: assets.NewCatalog(manifest, openFrom(payload)), + Info: buildinfo.Info{Version: "hook-entries"}, + Harnesses: []string{dialect}, + Planners: []Planner{{ + Name: dialect, + Plan: func(Mode, string, assets.Catalog) ([]Target, error) { + return append([]Target{hookTarget(path, dialect)}, more...), nil + }, + }}, + } + uopts := UninstallOptions{Roots: roots, FS: RealFS{}, SupportedHarnesses: []string{HookDialectClaude, HookDialectCursor}} + return opts, uopts, path +} + +func hasConflictFor(out Outcome, path, detail string) bool { + for _, a := range out.Actions { + if a.Op == OpConflict && a.Path == path && (detail == "" || strings.HasPrefix(a.Detail, detail)) { + return true + } + } + return false +} + +func TestHookEntriesInstallUninstallLifecycle(t *testing.T) { + for _, dialect := range []string{HookDialectClaude, HookDialectCursor} { + seed := userEntrySeed(t, dialect) + + t.Run(dialect+"/install then uninstall restores the seed", func(t *testing.T) { + opts, uopts, path := hookWorld(t, dialect, nil) + writeFileOrDie(t, path, seed) + if out := Install(opts); out.Err != nil || !out.Applied { + t.Fatalf("Install: %v (applied %v, actions %+v)", out.Err, out.Applied, out.Actions) + } + installed := readOrDie(t, path) + entries, ok := readHookEntries([]byte(installed), dialect) + if !ok { + t.Fatalf("the installed file is not editable:\n%s", installed) + } + for _, c := range hookCmdsFor(dialect) { + if countEntries(entries, func(e json.RawMessage) bool { return isExactEntry(e, dialect, c) }) != 1 { + t.Errorf("no exact entry runs %q:\n%s", c, installed) + } + } + state, err := LoadState(opts.Roots.StatePath()) + if err != nil || state == nil { + t.Fatalf("LoadState = %v, %v", state, err) + } + var rec *TargetRecord + for i := range state.Targets { + if state.Targets[i].Path == path { + rec = &state.Targets[i] + } + } + if rec == nil || rec.Kind != KindHookEntries || rec.HookDialect != dialect || + !reflect.DeepEqual(rec.HookCommands, hookCmdsFor(dialect)) || + rec.SHA256 != hookEntriesDigest(dialect, hookCmdsFor(dialect)) { + t.Fatalf("state record = %+v", rec) + } + + if out := Install(opts); out.Err != nil || out.Applied { + t.Errorf("a second install: err %v, applied %v (want a no-op)", out.Err, out.Applied) + } + if got := readOrDie(t, path); got != installed { + t.Errorf("a second install changed the file:\n%s", got) + } + + if out := Uninstall(uopts); out.Err != nil { + t.Fatalf("Uninstall: %v (actions %+v)", out.Err, out.Actions) + } + if got := readOrDie(t, path); got != seed { + t.Errorf("after uninstall =\n%q\nwant the seed\n%q", got, seed) + } + assertNoStaging(t, filepath.Dir(path)) + }) + + t.Run(dialect+"/an edited entry is a conflict and is left alone", func(t *testing.T) { + opts, uopts, path := hookWorld(t, dialect, nil) + writeFileOrDie(t, path, seed) + if out := Install(opts); out.Err != nil { + t.Fatalf("Install: %v", out.Err) + } + first := hookCmdsFor(dialect)[0] + installed := readOrDie(t, path) + edited := strings.Replace(installed, `"command": "`+first+`"`, `"command": "`+first+`", "timeout": 5`, 1) + if edited == installed { + t.Fatalf("the test could not edit the entry:\n%s", installed) + } + writeFileOrDie(t, path, edited) + + if out := Install(opts); out.Err != nil || out.Applied { + t.Errorf("install over an edited entry: err %v, applied %v (want a no-op)", out.Err, out.Applied) + } + out := Uninstall(uopts) + if out.Err == nil || !hasConflictFor(out, path, ReasonOwnershipConflict) { + t.Errorf("Uninstall = err %v, actions %+v; want an ownership conflict for %s", out.Err, out.Actions, path) + } + if got := readOrDie(t, path); got != edited { + t.Errorf("uninstall changed an edited file:\n%s", got) + } + }) + + t.Run(dialect+"/entries removed by hand need no removal", func(t *testing.T) { + opts, uopts, path := hookWorld(t, dialect, nil) + writeFileOrDie(t, path, seed) + if out := Install(opts); out.Err != nil { + t.Fatalf("Install: %v", out.Err) + } + writeFileOrDie(t, path, seed) + if out := Uninstall(uopts); out.Err != nil { + t.Fatalf("Uninstall: %v (actions %+v)", out.Err, out.Actions) + } + if got := readOrDie(t, path); got != seed { + t.Errorf("uninstall changed a file holding no docket entry:\n%s", got) + } + }) + + t.Run(dialect+"/a file broken after install is a conflict", func(t *testing.T) { + opts, uopts, path := hookWorld(t, dialect, nil) + if out := Install(opts); out.Err != nil { + t.Fatalf("Install: %v", out.Err) + } + writeFileOrDie(t, path, "not json") + // Install and check skip the broken file; uninstall still refuses + // it, because removal of the recorded entries cannot be proven from + // a file docket cannot parse — and it is never edited. + if check := Check(opts); check.Err != nil || !skippedWith(check, path, remedyHookFileInvalid) { + t.Errorf("Check: err %v (actions %+v), skipped %+v; want the broken file skipped", check.Err, check.Actions, check.Skipped) + } + out := Uninstall(uopts) + if out.Err == nil || out.Reason != ReasonManagedBlockInvalid || !hasConflictFor(out, path, ReasonManagedBlockInvalid) { + t.Errorf("Uninstall = err %v, reason %s, actions %+v", out.Err, out.Reason, out.Actions) + } + if got := readOrDie(t, path); got != "not json" { + t.Errorf("uninstall changed a broken file: %q", got) + } + }) + + t.Run(dialect+"/a later failure rolls the file back", func(t *testing.T) { + var later string + opts, _, path := hookWorld(t, dialect, func(r UserRoots) []Target { + later = filepath.Join(r.Home, "."+dialect, "zz.txt") + return []Target{{Path: later, Kind: KindFile, Content: []byte("later\n"), Role: "agent"}} + }) + writeFileOrDie(t, path, seed) + hookPublishes := 0 + opts.FS = &injectFS{inner: RealFS{}, fail: func(op, p string) error { + if op != "Rename" { + return nil + } + switch p { + case path: + hookPublishes++ + case later: + return fmt.Errorf("injected failure publishing %s", p) + } + return nil + }} + out := Install(opts) + if out.Err == nil { + t.Fatalf("Install succeeded despite the injected failure: %+v", out.Actions) + } + if hookPublishes != 2 { + t.Errorf("the hooks file was published %d times, want the apply plus the restore", hookPublishes) + } + if got := readOrDie(t, path); got != seed { + t.Errorf("after rollback =\n%q\nwant\n%q", got, seed) + } + if _, err := os.Lstat(later); !os.IsNotExist(err) { + t.Errorf("%s survived the rollback: %v", later, err) + } + assertNoStaging(t, filepath.Dir(path)) + }) + + t.Run(dialect+"/no seed leaves the empty document", func(t *testing.T) { + opts, uopts, path := hookWorld(t, dialect, nil) + if out := Install(opts); out.Err != nil { + t.Fatalf("Install: %v", out.Err) + } + if got := readOrDie(t, path); got != string(NewHookFileBytes(dialect, hookCmdsFor(dialect))) { + t.Errorf("created file =\n%s", got) + } + if out := Uninstall(uopts); out.Err != nil { + t.Fatalf("Uninstall: %v (actions %+v)", out.Err, out.Actions) + } + want := "{}\n" + if dialect == HookDialectCursor { + want = "{\n \"version\": 1\n}\n" + } + if got := readOrDie(t, path); got != want { + t.Errorf("after uninstall = %q, want %q", got, want) + } + }) + } +} + +// A later release that changes a trigger command must retire the recorded +// command's entry in the same install, or both entries would fire at every +// session start. Only an exact entry is retired: one the user has edited is +// theirs and stays. +func TestHookEntriesChangedCommandRetiresTheRecordedEntry(t *testing.T) { + for _, dialect := range []string{HookDialectClaude, HookDialectCursor} { + for _, edited := range []bool{false, true} { + name := dialect + "/exact old entry" + if edited { + name = dialect + "/edited old entry" + } + t.Run(name, func(t *testing.T) { + seed := userEntrySeed(t, dialect) + opts, uopts, path := hookWorld(t, dialect, nil) + oldCmds := hookCmdsFor(dialect) + newCmds := []string{"b --new"} + cmds := oldCmds + opts.Planners[0].Plan = func(Mode, string, assets.Catalog) ([]Target, error) { + return []Target{{Path: path, Kind: KindHookEntries, HookDialect: dialect, + HookCommands: append([]string(nil), cmds...), Role: "trigger"}}, nil + } + writeFileOrDie(t, path, seed) + if out := Install(opts); out.Err != nil || !out.Applied { + t.Fatalf("first Install: %v (applied %v)", out.Err, out.Applied) + } + if edited { + installed := readOrDie(t, path) + changed := strings.Replace(installed, `"command": "`+oldCmds[0]+`"`, + `"command": "`+oldCmds[0]+`", "timeout": 5`, 1) + if changed == installed { + t.Fatalf("the test could not edit the entry:\n%s", installed) + } + writeFileOrDie(t, path, changed) + } + + cmds = newCmds + if out := Install(opts); out.Err != nil || !out.Applied { + t.Fatalf("second Install: %v (applied %v, actions %+v)", out.Err, out.Applied, out.Actions) + } + got := readOrDie(t, path) + entries, ok := readHookEntries([]byte(got), dialect) + if !ok { + t.Fatalf("the file is not editable:\n%s", got) + } + if countEntries(entries, func(e json.RawMessage) bool { return isExactEntry(e, dialect, newCmds[0]) }) != 1 { + t.Errorf("no exact entry runs the new command:\n%s", got) + } + for i, c := range oldCmds { + runs := countEntries(entries, func(e json.RawMessage) bool { return entryHasCommand(e, dialect, c) }) + want := 0 + if edited && i == 0 { + want = 1 // the user's edited entry is theirs + } + if runs != want { + t.Errorf("%d entries run the retired command %q, want %d:\n%s", runs, c, want, got) + } + } + rec := stateRecordAt(t, opts.Roots, path) + if rec == nil || !reflect.DeepEqual(rec.HookCommands, newCmds) { + t.Fatalf("state record = %+v, want the new commands only", rec) + } + if out := Install(opts); out.Err != nil || out.Applied { + t.Errorf("a third install: err %v, applied %v (want a no-op)", out.Err, out.Applied) + } + if !edited { + if out := Uninstall(uopts); out.Err != nil { + t.Fatalf("Uninstall: %v (actions %+v)", out.Err, out.Actions) + } + if got := readOrDie(t, path); got != seed { + t.Errorf("after uninstall =\n%q\nwant the seed\n%q", got, seed) + } + } + }) + } + } +} + +// skippedFor reports whether the outcome names path as a skipped target +// carrying the not-a-regular-file remedy. +func skippedFor(out Outcome, path string) bool { + return skippedWith(out, path, remedyHookFileNotRegular) +} + +// skippedWith reports whether the outcome names path as a skipped target +// carrying remedy. +func skippedWith(out Outcome, path, remedy string) bool { + for _, s := range out.Skipped { + if s.Target.Path == path && s.Disposition == DispositionSkip && s.Remedy == remedy { + return true + } + } + return false +} + +func stateRecordAt(t *testing.T, roots UserRoots, path string) *TargetRecord { + t.Helper() + state, err := LoadState(roots.StatePath()) + if err != nil { + t.Fatalf("LoadState: %v", err) + } + if state == nil { + return nil + } + for i := range state.Targets { + if state.Targets[i].Path == path { + return &state.Targets[i] + } + } + return nil +} + +// A hooks file that is a dotfiles manager's symlink cannot be edited in place +// (the transaction publishes by rename, which would replace the link). It must +// not fail the install: the link and its destination stay byte-identical, every +// other target installs, the outcome names the skipped trigger, no record is +// written for it, and check and uninstall stay clean. +func TestHookEntriesSymlinkedHooksFileIsSkipped(t *testing.T) { + for _, dialect := range []string{HookDialectClaude, HookDialectCursor} { + seed := userEntrySeed(t, dialect) + + t.Run(dialect+"/fresh install skips the link and installs the rest", func(t *testing.T) { + var other string + opts, uopts, path := hookWorld(t, dialect, func(r UserRoots) []Target { + other = filepath.Join(r.Home, "."+dialect, "agent.md") + return []Target{{Path: other, Kind: KindFile, Content: []byte("agent\n"), Role: "agent"}} + }) + real := filepath.Join(opts.Roots.Home, "dotfiles", filepath.Base(path)) + writeFileOrDie(t, real, seed) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(real, path); err != nil { + t.Fatal(err) + } + assertLinkUntouched := func(stage string) { + t.Helper() + if dest, err := os.Readlink(path); err != nil || dest != real { + t.Errorf("%s: the link now reads %q (%v), want %q", stage, dest, err, real) + } + if got := readOrDie(t, real); got != seed { + t.Errorf("%s: the link's destination changed:\n%s", stage, got) + } + } + + out := Install(opts) + if out.Err != nil || !out.Applied { + t.Fatalf("Install: err %v (reason %q, applied %v, actions %+v)", out.Err, out.Reason, out.Applied, out.Actions) + } + if !skippedFor(out, path) { + t.Errorf("the outcome does not name the skipped hooks file %s: %+v", path, out.Skipped) + } + for _, a := range out.Actions { + if a.Path == path { + t.Errorf("an action names the skipped hooks file: %+v", a) + } + } + if got := readOrDie(t, other); got != "agent\n" { + t.Errorf("the other target was not installed: %q", got) + } + assertLinkUntouched("install") + if rec := stateRecordAt(t, opts.Roots, path); rec != nil { + t.Errorf("a record was written for the skipped hooks file: %+v", rec) + } + if rec := stateRecordAt(t, opts.Roots, other); rec == nil { + t.Errorf("no record was written for the installed target %s", other) + } + + again := Install(opts) + if again.Err != nil || again.Applied || !skippedFor(again, path) { + t.Errorf("a second install: err %v, applied %v, skipped %+v (want a no-op that still names the skip)", + again.Err, again.Applied, again.Skipped) + } + assertLinkUntouched("second install") + + check := Check(opts) + if check.Err != nil || !skippedFor(check, path) { + t.Errorf("Check: err %v (reason %q, actions %+v), skipped %+v; want clean with the skip named", + check.Err, check.Reason, check.Actions, check.Skipped) + } + + if u := Uninstall(uopts); u.Err != nil { + t.Fatalf("Uninstall: %v (actions %+v)", u.Err, u.Actions) + } + assertLinkUntouched("uninstall") + if _, err := os.Lstat(other); !os.IsNotExist(err) { + t.Errorf("uninstall left the installed target %s: %v", other, err) + } + }) + + t.Run(dialect+"/a recorded hooks file later linked is skipped and its record dropped", func(t *testing.T) { + opts, uopts, path := hookWorld(t, dialect, nil) + writeFileOrDie(t, path, seed) + if out := Install(opts); out.Err != nil { + t.Fatalf("Install: %v", out.Err) + } + if stateRecordAt(t, opts.Roots, path) == nil { + t.Fatalf("the first install recorded no hook-entries target") + } + installed := readOrDie(t, path) + // The user moves the file into a dotfiles checkout and links it back. + real := filepath.Join(opts.Roots.Home, "dotfiles", filepath.Base(path)) + writeFileOrDie(t, real, installed) + if err := os.Remove(path); err != nil { + t.Fatal(err) + } + if err := os.Symlink(real, path); err != nil { + t.Fatal(err) + } + + out := Install(opts) + if out.Err != nil || !skippedFor(out, path) { + t.Fatalf("Install over the link: err %v (reason %q, actions %+v), skipped %+v", + out.Err, out.Reason, out.Actions, out.Skipped) + } + if rec := stateRecordAt(t, opts.Roots, path); rec != nil { + t.Errorf("the record for the now-linked hooks file survived: %+v", rec) + } + if u := Uninstall(uopts); u.Err != nil { + t.Fatalf("Uninstall: %v (actions %+v)", u.Err, u.Actions) + } + if dest, err := os.Readlink(path); err != nil || dest != real { + t.Errorf("the link now reads %q (%v), want %q", dest, err, real) + } + if got := readOrDie(t, real); got != installed { + t.Errorf("the link's destination changed:\n%s", got) + } + }) + } +} + +// A hooks file docket cannot parse (a JSONC comment, a byte-order mark, a +// non-object hooks value) must not fail the install either: it is left +// byte-identical, every other target installs, the outcome names the skipped +// trigger with its remedy, no record is written for it, and check and +// uninstall stay clean. +func TestHookEntriesUnparseableHooksFileIsSkipped(t *testing.T) { + for _, dialect := range []string{HookDialectClaude, HookDialectCursor} { + for _, broken := range []string{"{\n // my settings\n \"model\": \"opus\"\n}\n", "\ufeff{}\n", `{"hooks": null}`} { + t.Run(dialect+"/"+broken, func(t *testing.T) { + var other string + opts, uopts, path := hookWorld(t, dialect, func(r UserRoots) []Target { + other = filepath.Join(r.Home, "."+dialect, "agent.md") + return []Target{{Path: other, Kind: KindFile, Content: []byte("agent\n"), Role: "agent"}} + }) + writeFileOrDie(t, path, broken) + assertUntouched := func(stage string) { + t.Helper() + if got := readOrDie(t, path); got != broken { + t.Errorf("%s: the unparseable hooks file changed:\n%q", stage, got) + } + } + + out := Install(opts) + if out.Err != nil || !out.Applied { + t.Fatalf("Install: err %v (reason %q, applied %v, actions %+v)", out.Err, out.Reason, out.Applied, out.Actions) + } + if !skippedWith(out, path, remedyHookFileInvalid) { + t.Errorf("the outcome does not name the skipped hooks file %s: %+v", path, out.Skipped) + } + for _, a := range out.Actions { + if a.Path == path { + t.Errorf("an action names the skipped hooks file: %+v", a) + } + } + if got := readOrDie(t, other); got != "agent\n" { + t.Errorf("the other target was not installed: %q", got) + } + assertUntouched("install") + if rec := stateRecordAt(t, opts.Roots, path); rec != nil { + t.Errorf("a record was written for the skipped hooks file: %+v", rec) + } + + check := Check(opts) + if check.Err != nil || !skippedWith(check, path, remedyHookFileInvalid) { + t.Errorf("Check: err %v (reason %q, actions %+v), skipped %+v; want clean with the skip named", + check.Err, check.Reason, check.Actions, check.Skipped) + } + + if u := Uninstall(uopts); u.Err != nil { + t.Fatalf("Uninstall: %v (actions %+v)", u.Err, u.Actions) + } + assertUntouched("uninstall") + }) + } + } +} + +func TestValidateTargetHookEntries(t *testing.T) { + good := TargetRecord{Path: "/h/.claude/settings.json", Kind: KindHookEntries, HookDialect: HookDialectClaude, + HookCommands: []string{"a --x", "a --y"}, SHA256: hookEntriesDigest(HookDialectClaude, []string{"a --x", "a --y"}), Role: "trigger"} + if err := validateTarget(good); err != nil { + t.Fatalf("a well-formed record is invalid: %v", err) + } + bad := map[string]func(*TargetRecord){ + "no dialect": func(r *TargetRecord) { r.HookDialect = "" }, + "unknown dialect": func(r *TargetRecord) { r.HookDialect = "vim" }, + "no commands": func(r *TargetRecord) { r.HookCommands = nil }, + "empty command": func(r *TargetRecord) { r.HookCommands = []string{"a --x", ""} }, + "duplicate command": func(r *TargetRecord) { r.HookCommands = []string{"a --x", "a --x"} }, + "no sha256": func(r *TargetRecord) { r.SHA256 = "" }, + "a block name": func(r *TargetRecord) { r.BlockName = "dispatch" }, + "a link target": func(r *TargetRecord) { r.LinkTarget = "/x" }, + } + for name, mutate := range bad { + rec := good + rec.HookCommands = append([]string(nil), good.HookCommands...) + mutate(&rec) + if err := validateTarget(rec); err == nil { + t.Errorf("record with %s is valid", name) + } + } + for name, rec := range map[string]TargetRecord{ + "file with hook_commands": {Path: "/h/f", Kind: KindFile, SHA256: "ab", Role: "agent", HookCommands: []string{"a"}}, + "file with hook_dialect": {Path: "/h/f", Kind: KindFile, SHA256: "ab", Role: "agent", HookDialect: HookDialectClaude}, + "block with hook fields": {Path: "/h/f", Kind: KindManagedBlock, BlockName: "dispatch", SHA256: "ab", Role: "dispatch", HookDialect: HookDialectCursor, HookCommands: []string{"a"}}, + "link with hook_commands": {Path: "/h/f", Kind: KindSymlink, LinkTarget: "/x", Role: "agent", HookCommands: []string{"a"}}, + } { + if err := validateTarget(rec); err == nil { + t.Errorf("%s is valid", name) + } + } + + goodT := hookTarget("/h/.cursor/hooks.json", HookDialectCursor) + if err := goodT.validate(); err != nil { + t.Fatalf("a well-formed target is invalid: %v", err) + } + badT := map[string]func(*Target){ + "no dialect": func(t *Target) { t.HookDialect = "" }, + "unknown dialect": func(t *Target) { t.HookDialect = "vim" }, + "no commands": func(t *Target) { t.HookCommands = nil }, + "empty command": func(t *Target) { t.HookCommands = []string{""} }, + "duplicate command": func(t *Target) { t.HookCommands = []string{"a", "a"} }, + "a block name": func(t *Target) { t.BlockName = "dispatch" }, + "file with hooks": func(t *Target) { t.Kind = KindFile }, + } + for name, mutate := range badT { + tgt := goodT + tgt.HookCommands = append([]string(nil), goodT.HookCommands...) + mutate(&tgt) + if err := tgt.validate(); !errors.Is(err, ErrInvalidTarget) { + t.Errorf("target with %s: validate = %v, want ErrInvalidTarget", name, err) + } + } + + rec, err := RecordFor(goodT) + if err != nil { + t.Fatal(err) + } + if rec.Kind != KindHookEntries || rec.HookDialect != HookDialectCursor || !reflect.DeepEqual(rec.HookCommands, cursorHookCmds) || + rec.SHA256 != hookEntriesDigest(HookDialectCursor, cursorHookCmds) || rec.BlockName != "" { + t.Errorf("RecordFor = %+v", rec) + } + goodT.HookCommands[0] = "changed" + if rec.HookCommands[0] != cursorHookCmds[0] { + t.Errorf("RecordFor shares the target's command slice") + } + if err := validateTarget(rec); err != nil { + t.Errorf("RecordFor produced an invalid record: %v", err) + } + + // The record survives the strict state decoder. + roots := versionRoots(t) + state := &State{FormatVersion: StateFormatVersion, ProductVersion: "v", AssetProtocol: 1, AssetSetID: "x", + Mode: ModeRelease, Harnesses: []string{"cursor"}, AgentDigest: "d", + Targets: []TargetRecord{func() TargetRecord { r := rec; r.Harness = "cursor"; return r }()}} + if err := WriteStateAtomic(roots.StatePath(), state); err != nil { + t.Fatal(err) + } + loaded, err := LoadState(roots.StatePath()) + if err != nil || loaded == nil || !reflect.DeepEqual(loaded.Targets[0].HookCommands, cursorHookCmds) { + t.Errorf("LoadState = %+v, %v", loaded, err) + } +} diff --git a/internal/install/inspect.go b/internal/install/inspect.go index 7d4119f68..878e6d862 100644 --- a/internal/install/inspect.go +++ b/internal/install/inspect.go @@ -2,6 +2,7 @@ package install import ( "bytes" + "encoding/json" "errors" "fmt" "io/fs" @@ -17,12 +18,12 @@ type Inspection struct { Target Target Disposition Disposition Reason string // conflict detail: ReasonOwnershipConflict | ReasonManagedBlockInvalid - // Remedy is the target-specific way forward for a conflict: what docket - // found here, and the one action that clears it. There is deliberately no - // --force, so the only way past a conflict is for the user to change what - // is on disk and run again — a report naming the reason alone would leave - // them with nowhere to go. Every conflict carries one; no other - // disposition does. + // Remedy is the target-specific way forward for a conflict or a skip: what + // docket found here, and the one action that clears it. There is + // deliberately no --force, so the only way past a conflict is for the user + // to change what is on disk and run again — a report naming the reason + // alone would leave them with nowhere to go. Every conflict and every skip + // carries one; no other disposition does. Remedy string } @@ -84,6 +85,8 @@ func InspectTarget(t Target, prior *State, legacy LegacyReproducer) (Inspection, return inspectSymlink(t, info, rec, hasRec, legacy) case KindManagedBlock: return inspectManagedBlock(t, info, rec, hasRec, legacy) + case KindHookEntries: + return inspectHookEntries(t, info, rec, hasRec) default: // validate has already refused every other kind. return Inspection{}, fmt.Errorf("%w: %s has unknown kind %q", ErrInvalidTarget, t.Path, t.Kind) @@ -215,10 +218,85 @@ func inspectManagedBlock(t Target, info fs.FileInfo, rec TargetRecord, hasRec bo return conflict(t, ReasonOwnershipConflict, remedyForeignBlock(t.BlockName)), nil } +// inspectHookEntries classifies a hook-entries target inside a hooks file the +// user also owns. Adding an entry only splices one in and keeps every other +// byte, so — as for adding a managed block — writing into a file docket did not +// author needs no ownership proof; a file docket cannot read as an editable +// hooks document is reported, never treated as "entries absent". +// +// A prior record at the same path whose commands the plan no longer carries (a +// later release changed a trigger command) names entries to retire in the same +// apply: an exact entry for such a command is cut before the new ones are +// added, so the old and the new trigger never both fire. Only an exact entry is +// cut, as on uninstall; one the user has edited is theirs and stays. +func inspectHookEntries(t Target, info fs.FileInfo, rec TargetRecord, hasRec bool) (Inspection, error) { + if !info.Mode().IsRegular() { + // The transaction publishes by rename, which would replace a symlink (a + // dotfiles manager's link, say) with a regular file. A skip, not a + // conflict: these entries are only a trigger, inert outside a private + // repository, so a file docket cannot edit in place must not fail the + // rest of the install. + return skip(t, remedyHookFileNotRegular), nil + } + data, err := os.ReadFile(t.Path) + if err != nil { + return Inspection{}, fmt.Errorf("install: reading %s: %w", t.Path, err) + } + entries, ok := readHookEntries(data, t.HookDialect) + if !ok { + // Skipped for the same reason: a file docket cannot parse (a JSONC + // comment, a byte-order mark, a non-object hooks value) is the user's to + // repair, and refusing it would fail every other target over a trigger. + return skip(t, remedyHookFileInvalid), nil + } + t.retireHookCommands = staleHookCommands(t, entries, rec, hasRec) + if len(t.retireHookCommands) > 0 { + return Inspection{Target: t, Disposition: DispositionUpdate}, nil + } + for _, c := range t.HookCommands { + if !anyEntryRuns(entries, t.HookDialect, c) { + return Inspection{Target: t, Disposition: DispositionUpdate}, nil + } + } + // Every command already has an entry. One the user has since edited (a + // timeout added) still runs the command, so it is theirs and left alone. + return Inspection{Target: t, Disposition: DispositionNoop}, nil +} + +// staleHookCommands lists the prior record's commands the plan no longer +// carries and the file still holds an exact entry for. The record must be a +// consistent hook-entries record in the same dialect — its digest is its +// identity — or it proves nothing and nothing is retired. +func staleHookCommands(t Target, entries []json.RawMessage, rec TargetRecord, hasRec bool) []string { + if !hasRec || rec.Kind != KindHookEntries || rec.HookDialect != t.HookDialect || + rec.SHA256 == "" || rec.SHA256 != hookEntriesDigest(rec.HookDialect, rec.HookCommands) { + return nil + } + var stale []string + for _, c := range rec.HookCommands { + if containsString(t.HookCommands, c) { + continue + } + for _, e := range entries { + if isExactEntry(e, t.HookDialect, c) { + stale = append(stale, c) + break + } + } + } + return stale +} + func conflict(t Target, reason, remedy string) Inspection { return Inspection{Target: t, Disposition: DispositionConflict, Reason: reason, Remedy: remedy} } +// skip is a target left untouched and reported with its remedy: what is at the +// path cannot be edited in place, and the operation proceeds without it. +func skip(t Target, remedy string) Inspection { + return Inspection{Target: t, Disposition: DispositionSkip, Remedy: remedy} +} + // The remedies. Each names what docket found, then the one action that clears // it — the installer has no --force, so the only way past a conflict is for the // user to change what is on disk and run again. @@ -226,6 +304,10 @@ const ( remedyForeignPath = "docket did not write what is at this path; move or delete it, then re-run" remedyDriftedPath = "this path no longer matches the recorded install, so docket cannot prove it may overwrite it; " + "restore the recorded content, or move it aside, then re-run" + remedyHookFileNotRegular = "this hooks file is not a regular file (a symlink or a directory), so docket left it untouched " + + "and did not add its session-start entries; to install them, make it a regular file, then re-run" + remedyHookFileInvalid = "this hooks file is not a JSON object docket can edit (hooks must be an object and each event an array), " + + "so docket left it untouched and did not add its session-start entries; to install them, repair it by hand, then re-run" ) // remedyLinkDestination names the destination the plan wants, which is the one @@ -239,18 +321,22 @@ func remedyLinkDestination(want string) string { // remedyBlockMarkers is the only remedy docket cannot offer to perform itself: // the markers delimit somebody's own file, and guessing where a dangling range // was meant to end would eat their content. +// +// Each block remedy spells the block the way its own marker lines do +// (document.MarkerSpelling), so a neutral dckt: block is never misnamed as a +// docket: one. func remedyBlockMarkers(block string) string { - return "the docket:" + block + " markers in this file are malformed, unbalanced, or out of order; " + + return "the " + document.MarkerSpelling(block) + " markers in this file are malformed, unbalanced, or out of order; " + "repair them by hand, then re-run" } func remedyForeignBlock(block string) string { - return "the docket:" + block + " block in this file holds content docket did not write; " + + return "the " + document.MarkerSpelling(block) + " block in this file holds content docket did not write; " + "delete the block, or move the file aside, then re-run" } func remedyDriftedBlock(block string) string { - return "the docket:" + block + " block no longer matches the recorded install, " + + return "the " + document.MarkerSpelling(block) + " block no longer matches the recorded install, " + "so docket cannot prove it may rewrite it; restore its recorded content, or delete the block, then re-run" } @@ -353,6 +439,21 @@ func recordMatchesDisk(rec TargetRecord) (bool, error) { } return interiorDigest(doc.Source()[block.Interior.Start:block.Interior.End]) == rec.SHA256, nil + case KindHookEntries: + // Every entry that runs a recorded command must be exactly docket's. A + // file holding none of them is a match too: nothing of docket's is left + // to preserve, as for a vanished target. + if !info.Mode().IsRegular() || rec.SHA256 == "" || + rec.SHA256 != hookEntriesDigest(rec.HookDialect, rec.HookCommands) { + return false, nil + } + data, err := os.ReadFile(rec.Path) + if err != nil { + return false, fmt.Errorf("install: reading %s: %w", rec.Path, err) + } + editable, _, allExact := hookEntriesState(data, rec.HookDialect, rec.HookCommands) + return editable && allExact, nil + default: return false, nil } diff --git a/internal/install/repophase.go b/internal/install/repophase.go index a2a67f4b0..933144a62 100644 --- a/internal/install/repophase.go +++ b/internal/install/repophase.go @@ -59,6 +59,10 @@ type RepoPhase struct { // unauthorized. When authorized it is always non-nil — an authorized empty // list publishes the encoded empty record rather than deleting it. RecordBytes []byte + // ExtraRecords are further repository ownership records the commit + // publishes beside RecordPath: a private repository's linked-worktree record + // from an earlier install, rewritten once its surfaces (in Removals) retire. + ExtraRecords []StateDoc // Worktree is the selected working-tree root, carried only so the "not // authorized" no-op action can name where reconciliation would have happened. // It is not otherwise consulted, and is empty for a machine-only run outside diff --git a/internal/install/repophase_test.go b/internal/install/repophase_test.go index 677157197..1d6887f6b 100644 --- a/internal/install/repophase_test.go +++ b/internal/install/repophase_test.go @@ -285,6 +285,36 @@ func TestApplyPlanEmptyListRetiresAndPublishesEmptyRecord(t *testing.T) { } } +// TestApplyPlanPublishesExtraRecords proves a phase's further records publish +// beside its main one, and that a run whose extra record is already on disk is +// settled while one whose extra record differs is not. +func TestApplyPlanPublishesExtraRecords(t *testing.T) { + w := newRepoWorld(t) + p := plannedInstallation{mode: ModeRelease, harnesses: []string{"claude"}, assetSetID: "sha256:x", assetProtocol: 1} + recordPath := filepath.Join(w.base, "gitdir", "dckt", "install.json") + extraPath := filepath.Join(w.base, "gitdir", "worktrees", "f", "dckt", "install.json") + writeFileOrDie(t, extraPath, "the old record\n") + emptyRecord := []byte("{\"format_version\":1,\"surfaces\":[]}\n") + repo := &RepoPhase{ + Authorized: true, RecordPath: recordPath, RecordBytes: emptyRecord, + ExtraRecords: []StateDoc{{Path: extraPath, Bytes: emptyRecord}}, + Worktree: filepath.Join(w.base, "wt"), + } + writeFileOrDie(t, recordPath, string(emptyRecord)) + if settled, err := repoRecordSettled(true, repo); err != nil || settled { + t.Fatalf("repoRecordSettled with a differing extra record = %v, %v; want unsettled", settled, err) + } + if out := applyPlan(w.options(), p, repo, w.applyOut()); out.Err != nil { + t.Fatalf("applyPlan: %v (reason %q)", out.Err, out.Reason) + } + if got := readOrDie(t, extraPath); got != string(emptyRecord) { + t.Errorf("extra record = %q, want %q", got, emptyRecord) + } + if settled, err := repoRecordSettled(true, repo); err != nil || !settled { + t.Errorf("repoRecordSettled after the publish = %v, %v; want settled", settled, err) + } +} + func TestApplyPlanEditedOwnedSurfaceBlocksEverything(t *testing.T) { w := newRepoWorld(t) machineFile := filepath.Join(w.base, "machine", "agent.md") diff --git a/internal/install/retire_test.go b/internal/install/retire_test.go index 85de269bc..5d1d0ffb5 100644 --- a/internal/install/retire_test.go +++ b/internal/install/retire_test.go @@ -1,10 +1,16 @@ package install import ( + "fmt" "os" "path/filepath" + "strings" "testing" + "github.com/danielhanold/docket/internal/assets" + "github.com/danielhanold/docket/internal/buildinfo" + "github.com/danielhanold/docket/internal/config" + "github.com/danielhanold/docket/internal/document" "github.com/danielhanold/docket/internal/testsupport" ) @@ -486,3 +492,197 @@ func TestRetireCollectsAllConflicts(t *testing.T) { t.Errorf("expected both conflicts collected, got %d: %+v", len(conflicts), conflicts) } } + +// --- retiring one block and writing another in the same file ---------------- + +// pointerBlockName is the neutral-namespace block the codex planner writes into +// the same file that still carries a leftover docket:dispatch block. +const pointerBlockName = "dckt:private-instructions" + +// leftoverCodexWorld seeds /.codex/AGENTS.md with the user's prose around +// a docket:dispatch block whose interior the prior state records, and returns +// install options whose codex planner writes a pointer block into that same file +// (plus any extra targets). The planner's GlobalDispatchTarget names the +// leftover, so one install must retire it and add the pointer together. +func leftoverCodexWorld(t *testing.T, extra ...Target) (opts Options, path, original string) { + t.Helper() + return leftoverCodexWorldAt(t, versionRoots(t), extra...) +} + +// leftoverCodexWorldAt is leftoverCodexWorld over roots the caller already +// holds, so an extra target can be planned under the same home. +func leftoverCodexWorldAt(t *testing.T, roots UserRoots, extra ...Target) (opts Options, path, original string) { + t.Helper() + path = filepath.Join(roots.Home, ".codex", "AGENTS.md") + interior := "leftover dispatch rules\n" + original = "# mine\n\n" + + "\n" + interior + "\n" + + "tail\n" + writeFileOrDie(t, path, original) + + payload := samplePayload() + manifest := sampleManifest(t, payload) + prior := &State{FormatVersion: StateFormatVersion, ProductVersion: "v-prior", AssetProtocol: assets.AssetProtocol, + AssetSetID: manifest.AssetSetID, Mode: ModeRelease, Harnesses: []string{"codex"}, + AgentDigest: "sha256:agents", Targets: []TargetRecord{blockRecord(path, interior, "codex")}} + if err := WriteStateAtomic(roots.StatePath(), prior); err != nil { + t.Fatal(err) + } + + opts = Options{ + Roots: roots, FS: RealFS{}, Config: &config.Snapshot{}, + Catalog: assets.NewCatalog(manifest, openFrom(payload)), + Info: buildinfo.Info{Version: "retire-and-write"}, + Harnesses: []string{"codex"}, + Planners: []Planner{{ + Name: "codex", + Plan: func(Mode, string, assets.Catalog) ([]Target, error) { + return append([]Target{{ + Path: path, Kind: KindManagedBlock, BlockName: pointerBlockName, + Annotation: "managed — do not hand-edit", Content: []byte("pointer"), Role: "trigger", + }}, extra...), nil + }, + GlobalDispatchTarget: func(r UserRoots) Target { + return mbTarget(filepath.Join(r.Home, ".codex", "AGENTS.md")) + }, + }}, + } + return opts, path, original +} + +func TestInstallRetiresLeftoverBlockAndWritesPointerInOneFile(t *testing.T) { + opts, path, _ := leftoverCodexWorld(t) + + out := Install(opts) + if out.Err != nil { + t.Fatalf("Install: %v (reason %s, actions %+v)", out.Err, out.Reason, out.Actions) + } + + got := readOrDie(t, path) + doc, err := document.Parse([]byte(got)) + if err != nil { + t.Fatalf("parsing the installed file: %v\n%s", err, got) + } + if _, ok := doc.Block("dispatch"); ok || strings.Contains(got, "docket:dispatch") { + t.Errorf("the leftover docket:dispatch block survived:\n%s", got) + } + block, ok := doc.Block(pointerBlockName) + if !ok { + t.Fatalf("no %s block was written:\n%s", pointerBlockName, got) + } + if interior := got[block.Interior.Start:block.Interior.End]; interior != "pointer\n" { + t.Errorf("pointer interior = %q, want %q", interior, "pointer\n") + } + if n := strings.Count(got, "\npointer\n\n") { + t.Errorf("the pointer block was not written:\n%s", got) + } + for _, kept := range []string{"user prose above\n", "user prose below\n"} { + if !strings.Contains(got, kept) { + t.Errorf("the user's %q did not survive:\n%s", kept, got) + } + } + assertNoStaging(t, f.targets) +} + +func TestTxnRetiresOneBlockAndWritesAnotherInOneFileRollsBack(t *testing.T) { + // Every mutation the pair performs gets an injected failure, and every one + // must restore managed.md byte-for-byte: two steps on one path take two + // backups, both of the original bytes, and the rollback restores newest first. + var ops []string + func() { + f := newFixture(t) + insp, rem := retireAndWritePlan(f) + ifs := &injectFS{inner: RealFS{}} + txn, err := BeginTxnWithRemovals(ifs, f.roots, insp, rem) + if err != nil { + t.Fatalf("BeginTxnWithRemovals: %v", err) + } + ifs.fail = recordCalls(&ops) + if err := txn.Apply(); err != nil { + t.Fatalf("clean Apply: %v", err) + } + }() + if len(ops) == 0 { + t.Fatal("a clean apply performed no mutations; the table would be vacuous") + } + + for n := 1; n <= len(ops); n++ { + t.Run(fmt.Sprintf("fail-at-%02d-%s", n, ops[n-1]), func(t *testing.T) { + f := newFixture(t) + insp, rem := retireAndWritePlan(f) + before := snapshotWorld(t, f.targets) + + ifs := &injectFS{inner: RealFS{}} + txn, err := BeginTxnWithRemovals(ifs, f.roots, insp, rem) + if err != nil { + t.Fatalf("BeginTxnWithRemovals: %v", err) + } + ifs.fail = failAtCall(n) + + if err := txn.Apply(); err == nil { + t.Fatal("Apply succeeded despite an injected filesystem failure") + } + assertWorld(t, before, snapshotWorld(t, f.targets), "after rollback") + assertNoStaging(t, f.targets) + if _, found, err := DetectRecovery(f.roots); err != nil || found { + t.Errorf("DetectRecovery after rollback = (found %v, err %v), want (false, nil)", found, err) + } + }) + } +} + +func TestRejectDuplicateDestinationsRefusesRemovalAndWriteOfOneBlock(t *testing.T) { + f := newFixture(t) + // f.plan already writes the dispatch block in managed.md; retiring that same + // block in the same plan has an outcome that depends on step order. + _, err := BeginTxnWithRemovals(RealFS{}, f.roots, f.plan, []TargetRecord{{ + Path: f.path("dispatch", "managed.md"), Kind: KindManagedBlock, BlockName: "dispatch", Role: "dispatch", + }}) + if !errors.Is(err, ErrInvalidTarget) { + t.Fatalf("err = %v, want ErrInvalidTarget", err) + } + if journalCount(t, f.roots) != 0 { + t.Errorf("a refused plan left a journal behind") + } +} + +func TestRejectDuplicateDestinationsRefusesTwoBlockRemovalsInOneFile(t *testing.T) { + f := newFixture(t) + // The second removal would re-verify a pre-image the first one had already + // rewritten, so it could only ever fail as stale. + path := f.path("dispatch", "managed.md") + _, err := BeginTxnWithRemovals(RealFS{}, f.roots, nil, []TargetRecord{ + {Path: path, Kind: KindManagedBlock, BlockName: "dispatch", Role: "dispatch"}, + {Path: path, Kind: KindManagedBlock, BlockName: "dckt:pointer", Role: "trigger"}, + }) + if !errors.Is(err, ErrInvalidTarget) { + t.Fatalf("err = %v, want ErrInvalidTarget", err) + } + if journalCount(t, f.roots) != 0 { + t.Errorf("a refused plan left a journal behind") + } +} + +func TestRejectDuplicateDestinationsRefusesAWholeFileBesideABlock(t *testing.T) { + f := newFixture(t) + // A block removal only frees disjoint bytes; a whole-file write on the same + // path is still two writers of one file. + path := f.path("dispatch", "managed.md") + _, err := BeginTxnWithRemovals(RealFS{}, f.roots, []Inspection{{ + Target: Target{Path: path, Kind: KindFile, Content: []byte("whole\n"), Role: "agent"}, + Disposition: DispositionUpdate, + }}, []TargetRecord{{Path: path, Kind: KindManagedBlock, BlockName: "dispatch", Role: "dispatch"}}) + if !errors.Is(err, ErrInvalidTarget) { + t.Fatalf("err = %v, want ErrInvalidTarget", err) + } +} diff --git a/internal/install/uninstall.go b/internal/install/uninstall.go index c7eb1b523..352d6860a 100644 --- a/internal/install/uninstall.go +++ b/internal/install/uninstall.go @@ -325,6 +325,22 @@ func proveUninstallRemoval(rec TargetRecord) (bool, bool, string, error) { return true, false, "", nil } } + if rec.Kind == KindHookEntries && info.Mode().IsRegular() { + data, err := os.ReadFile(rec.Path) + if err != nil { + return false, false, "", fmt.Errorf("install: reading %s: %w", rec.Path, err) + } + editable, anyPresent, _ := hookEntriesState(data, rec.HookDialect, rec.HookCommands) + if !editable { + return false, false, ReasonManagedBlockInvalid, nil + } + if !anyPresent { + // No entry runs a recorded command: the user's file holds nothing of + // docket's, so there is nothing to remove. + return true, false, "", nil + } + // Otherwise recordMatchesDisk decides: a modified entry is a conflict. + } matches, err := recordMatchesDisk(rec) if err != nil { return false, false, "", err diff --git a/internal/layout/layout.go b/internal/layout/layout.go index 9913fdaf3..741059440 100644 --- a/internal/layout/layout.go +++ b/internal/layout/layout.go @@ -30,6 +30,12 @@ const ( SharedWorktreeDir = ".docket" PrivateConfigFile = "config.yml" PrivateConfigDisplay = ".git/dckt/config.yml" + + // PrivateInstructionsFile is a private repository's parent-facing rules + // file beneath /dckt; PrivateInstructionsDisplay is its + // user-facing spelling. + PrivateInstructionsFile = "AGENTS.md" + PrivateInstructionsDisplay = ".git/dckt/AGENTS.md" ) // Detect decides the mode from state; a probe error is returned, never guessed. @@ -67,6 +73,12 @@ func PrivateConfigPath(commonDir string) string { return filepath.Join(commonDir, PrivateName, PrivateConfigFile) } +// PrivateInstructionsPath is a private repository's parent-facing rules file, +// /dckt/AGENTS.md. +func PrivateInstructionsPath(commonDir string) string { + return filepath.Join(commonDir, PrivateName, PrivateInstructionsFile) +} + // CommonDirOf resolves a working-tree root's common dir from the filesystem // alone (no git process). ok=false when /.git does not exist. // diff --git a/internal/layout/layout_test.go b/internal/layout/layout_test.go index 0077013a2..e45b2bb2e 100644 --- a/internal/layout/layout_test.go +++ b/internal/layout/layout_test.go @@ -190,3 +190,12 @@ func TestCommonDirOf(t *testing.T) { } }) } + +func TestPrivateInstructionsPath(t *testing.T) { + if got, want := PrivateInstructionsPath("/r/.git"), "/r/.git/dckt/AGENTS.md"; got != want { + t.Fatalf("PrivateInstructionsPath = %q, want %q", got, want) + } + if got, want := PrivateInstructionsDisplay, ".git/dckt/AGENTS.md"; got != want { + t.Fatalf("PrivateInstructionsDisplay = %q, want %q", got, want) + } +} diff --git a/internal/process/recover_test.go b/internal/process/recover_test.go index 462cf8208..809eda2c3 100644 --- a/internal/process/recover_test.go +++ b/internal/process/recover_test.go @@ -263,12 +263,32 @@ func TestRecoverLeavesUnprovableGroupForInspection(t *testing.T) { svc := newTestService(t) root := testsupport.TempDir(t) out := launchHelper(t, svc, root, "sleep") - m, _ := readManifest(out.RunDir) - signalGroup(m.PGID, syscall.SIGKILL) + m, err := readManifest(out.RunDir) + if err != nil || m == nil || m.SupervisorPID <= 1 || m.PGID <= 1 { + t.Fatalf("manifest: %+v (err %v)", m, err) + } + // Kill the supervisor ALONE and wait until it is reaped, and only then end + // its command. One group-wide SIGKILL is not atomic across the group's + // processes: the command can die first while a supervisor thread, parked in + // cmd.Wait, returns and writes terminal.json before its own SIGKILL lands — + // a run Recover then correctly classifies "terminal", never reaching the + // group probe this test exists to exercise. A supervisor that is gone + // before its command dies can never record that death. + if err := syscall.Kill(m.SupervisorPID, syscall.SIGKILL); err != nil { + t.Fatalf("kill the supervisor: %v", err) + } + waitFor(t, "the killed supervisor to be reaped", 30*time.Second, func() bool { + return processAlive(m.SupervisorPID) == probeAbsent + }) + _ = signalGroup(m.PGID, syscall.SIGKILL) // end the orphaned command waitFor(t, "lock release", 30*time.Second, func() bool { held, _ := probeFlock(filepath.Join(out.RunDir, liveLockFile)) return !held }) + // Setup, not the code under test: the run must reach the group probe. + if term, terr := readTerminal(out.RunDir); terr != nil || term != nil { + t.Fatalf("setup: terminal record present (%+v, err %v); the group probe is unreachable", term, terr) + } // A probe error is not clean absence: the recorded group is unprovable, so // the run must be left for inspection and never marked abandoned. prev := recoverGroupProbe diff --git a/internal/repoguard/budgets_test.go b/internal/repoguard/budgets_test.go index e5b3752a3..ef168488f 100644 --- a/internal/repoguard/budgets_test.go +++ b/internal/repoguard/budgets_test.go @@ -199,9 +199,9 @@ var skillBudgets = []skillBudget{ {"docket-convention/SKILL.md", 361, 7263}, // 0532: the private-repository writing rule and visibility hand-off (359/7160 -> 361/7263); 0531 review: skills say "the metadata branch", naming `docket` once for shared mode (359/7155 -> 359/7160); 0531: the metadata worktree and remote come from the repository.prepare context (357/7062 -> 359/7155); 0530: plan, results, and build evidence live on the docket branch — the plan-writer's one metadata operation, the attach operations as file-backlink writers, relative same-branch links, and the new ## Build evidence record section (357/7026 -> 357/7062); 0529 review: the backlink block names its PR-body writers beside artifact.backlink, one shared renderer (357/7004 -> 357/7026); 0515: the repair sign-off is retired; a green repair merges (357/7007 -> 357/7004); 0502 rebase onto 0510: kept the backlog-match follow-up clause in the results-artifact paragraph (357/7004 -> 357/7007); 0502 review: the review-tier dispatch is the controller's own (357/6995 -> 357/7004); 0502: configuration read paths corrected — operations read .docket.yml from origin's default-branch tip, the repository setup operations from the primary worktree (357/6959 -> 357/6995); 0502: "loop" means /loop only (357/7036 -> 357/7037); 0502: one metadata layout — the docket branch and the .docket/ worktree (357/7035 -> 357/7036); 0502: agent layer names user-level wrappers and global-only pins (357/7024 -> 357/7035); 0502: typed operations replace the Bash-era script names; ## Run halted facts corrected (359/7092 -> 357/7024); 0502: terminal publish, the learnings cap, and the repository auto_groom default are deleted (362/7256 -> 359/7092); 0502: the auto-capture export paragraph is deleted (370/7334 -> 362/7256); 0502: the configuration contract rewritten from the schema (371/7488 -> 370/7334); 0502: fixed role skills; halt posture (372/7660 -> 371/7488); 0502: record blocks mirror the renderers (367/7619 -> 372/7660); 0502: the dummy-mode shared definition is deleted (390/7848 -> 367/7619); 0410: +required-results lifecycle prose; 0399: +schema request/result contract prose; 0388: +sync-integration prose (see note above); 0502: drop change citations and Go v1 narration (357/7037 -> 357/6958); 0502: the harness adapter is named without a docket source path (357/6958 -> 357/6959) // 0154: docket-convention/github-board-mirror.md removed — the GitHub mirror is // retired (unsupported, mutation-blocking); its budget row is deleted with it. - {"docket-convention/references/agent-layer.md", 148, 1419}, // 0502: rewritten around user-level wrappers and global-only pins (202/2346 -> 148/1419); 0502: the legacy agents-file migration clause is deleted (202/2349 -> 202/2346) + {"docket-convention/references/agent-layer.md", 164, 1553}, // 0535: a private repository's instructions file and its user-level triggers (148/1419 -> 164/1553); 0502: rewritten around user-level wrappers and global-only pins (202/2346 -> 148/1419); 0502: the legacy agents-file migration clause is deleted (202/2349 -> 202/2346) {"docket-convention/references/close-out.md", 141, 1347}, // 0531: the metadata worktree and remote come from the repository.prepare context (141/1349 -> 141/1347); 0502: "loop" means /loop only (144/1375 -> 144/1374); 0502: one metadata layout — the docket branch and the .docket/ worktree (144/1376 -> 144/1375); 0502: the scripts/.md contract pointer becomes the schema operation (144/1374 -> 144/1376); 0502: the terminal-publication step is deleted (240/2150 -> 144/1374); 0474: reference renamed (ADR-0129 row 59a); ceilings unchanged; 0502: drop change citations and Go v1 narration (144/1374 -> 144/1356); 0502: the docket-source test citations are deleted (144/1356 -> 141/1349) - {"docket-convention/references/learnings.md", 66, 457}, // 0502: one metadata layout — the docket branch and the .docket/ worktree (66/455 -> 66/457); 0502: learning.record/learning.update are the writers; the capacity section is deleted (84/580 -> 66/455) + {"docket-convention/references/learnings.md", 69, 487}, // 0535: promotion lands in the private instructions file in a private repository (66/457 -> 69/487); 0502: one metadata layout — the docket branch and the .docket/ worktree (66/455 -> 66/457); 0502: learning.record/learning.update are the writers; the capacity section is deleted (84/580 -> 66/455) {"docket-convention/references/stacked-changes.md", 214, 2149}, // 0502: typed operations and binary-emitted findings replace the Bash-era names (213/2125 -> 214/2149); 0502: the deferred descendant-publication sentence is deleted (215/2140 -> 213/2125); 0327: +carry-preservation contract prose (see note above) {"docket-finalize-change/SKILL.md", 241, 6033}, // 0532 review: both finalize gate dispatches carry the Visibility payload line (239/6020 -> 241/6033); 0532: the private-repository writing rule and visibility hand-off (239/6000 -> 239/6020); 0529 review: the PR-backlink repoint is a best-effort follow-up after the archive push, for root-archived descendants too (239/5984 -> 239/6000); 0529: close-out repoints the merged PR description's backlink; pr-backlink-pending is a retryable warning (239/5936 -> 239/5984); 0525 review: workspace-remnant rides any finished-workspace disposition and is named in the final report; branch-rules-unavailable comes only from the probing run (239/5910 -> 239/5936); 0525: plan-gated merges carry branch-rules-unavailable into closeout notes; cleanup reports workspace-remnant and names what blocked a workspace (239/5820 -> 239/5910); 0515 review: the refused-notes fallback names every dropped note in the final report (238/5809 -> 239/5820); 0515 review: a repair whose run halts before closeout is named in finalize.block and carried into a later closeout (238/5708 -> 238/5809); 0515: the repair sign-off is retired; a green repair merges (238/5808 -> 238/5708); 0517: the repaired-head evidence.record carries --owner finalize and records finalize.test_command (238/5790 -> 238/5808); 0502 review: clear-block re-reads the revision after the block lands (238/5771 -> 238/5790); 0502: repair sign-off records finalize.block before publishing the repaired head (238/5733 -> 238/5771); 0502: "loop" means /loop only (238/5729 -> 238/5733); 0502: repair sign-off is the human running finalize.clear-block (238/5666 -> 238/5729); 0502: the repaired head is re-gated through the gate driver (236/5568 -> 238/5666); 0502: the dummy-mode paragraph is deleted (238/5641 -> 236/5568); 0455: +record-invalid refusal (structural scope, findings remedy, merged-outside-docket precedence) in step 8 (word ceiling 5520 -> 5647); 0442: +post-publication base-advance guidance (word ceiling 5421 -> 5520); de-duplicated the shared forward-rebase mechanic against the 0438 unpublished-case paragraph (reclaimed 57 words), but the distinct published-refresh facts plus the retained 0438 guidance cannot fit the old ceiling without deleting required guidance; 0411: +reconciliation-write recovery exception paragraph in the resolver loop (ceilings 238/5232 -> 239/5421); 0413: +generated-bundle mixed-conflict handoff sentence in the resolver-loop block (word ceiling 5200 -> 5232); 0419: +repair-attempt budget payload line and rewired repair contract (line ceiling 236 -> 238); 0393: +exact payload, marker, and direct-dispatch lines atop 0349/0410 (see note above) {"docket-finalize-change/references/gate-failure.md", 162, 2111}, // 0532 review: the gate-agent payload names Visibility (161/2109 -> 162/2111); 0515 review: the refused-notes fallback names every dropped note in the final report (160/2098 -> 161/2109); 0515 review: a repair whose run halts before closeout is named in finalize.block and carried into a later closeout (153/2005 -> 160/2098); 0515: the repair sign-off is retired; a green repair merges (160/2086 -> 153/2005); 0517: the repaired-head evidence.record carries --owner finalize (159/2081 -> 160/2086); 0502 review: clear-block re-reads the revision after the block lands (157/2062 -> 159/2081); 0502: repair sign-off records finalize.block before publishing the repaired head (155/2028 -> 157/2062); 0502: the embedded-bundle rule is conditioned on docket's own repository (155/2024 -> 155/2028); 0502: "loop" means /loop only (155/2027 -> 155/2024); 0502: repair sign-off is the human running finalize.clear-block; the marker lifecycle names what removes it (154/2013 -> 155/2027); 0502: the repaired head is re-gated through the gate driver (148/1937 -> 154/2013); 0497: +wait-for-the-leftover-group sentence (146/1901 -> 148/1937); 0493: the gate relaunch is retired (145/1892 -> 146/1901); 0491: the run id is retired (145/1894 -> 145/1892); 0490: the shared-slot note became the shared worktree-lock note (147/1901 -> 145/1894); 0411: +reconciliation-write exception section and abort-set carve-out (ceilings 135/1472 -> 147/1901); 0413: +conflicted_paths-lists-authored-only rule in the resolver-report section (line ceiling 133 -> 135, word ceiling 1465 -> 1472); 0419: +repair-attempt budget payload and rewired repair contract prose (word ceiling 1450 -> 1465); 0349: +reserve-before-dispatch resolver protocol prose; 0375: +worktree-slot note for the scopeless finalize gate (120/1300 -> 133/1450) diff --git a/internal/repoguard/private_instructions_docs_test.go b/internal/repoguard/private_instructions_docs_test.go new file mode 100644 index 000000000..587ca7bb2 --- /dev/null +++ b/internal/repoguard/private_instructions_docs_test.go @@ -0,0 +1,81 @@ +package repoguard + +import ( + "fmt" + "regexp" + "strings" + "testing" +) + +// The convention references tell an agent where a private repository's +// parent-facing rules live and how they reach a session: promoted lessons land +// in `.git/dckt/AGENTS.md`, and user-level triggers (never a working-tree rule +// file) deliver that file. These rows keep both statements in their sections. +var privateInstructionsDocContracts = []docSectionContract{ + {change: "private_instructions_delivery", file: "skills/docket-convention/references/agent-layer.md", + section: "## Repository dispatch blocks: agent_harnesses", terminator: "## Launch posture", + present: []string{ + "dckt instructions --hook claude --section dispatch", + "dckt instructions --hook cursor", + "dckt-instructions.js", + "dckt:private-instructions", + }, + absent: []string{".cursor/rules/dckt-dispatch.mdc"}}, +} + +// privatePromotionSection bounds the learnings promotion rule. +var privatePromotionSection = docSectionContract{ + change: "private_instructions_promotion", file: "skills/docket-convention/references/learnings.md", + section: "## Promotion — the shrink valve", terminator: "## Off switch", +} + +var paragraphBreak = regexp.MustCompile(`\n[ \t]*\n`) + +// scanPrivatePromotion requires one paragraph of the promotion section to name +// both the private file and a private repository, so the destination stays +// bound to the repository kind it applies to. +func scanPrivatePromotion(content string, c docSectionContract) []string { + si := strings.Index(content, c.section) + if si < 0 { + return []string{fmt.Sprintf("%s: missing section heading %q", c.file, c.section)} + } + rest := content[si+len(c.section):] + ti := strings.Index(rest, c.terminator) + if ti < 0 { + return []string{fmt.Sprintf("%s: section %q missing terminator %q", c.file, c.section, c.terminator)} + } + for _, para := range paragraphBreak.Split(rest[:ti], -1) { + p := collapseWS(para) + if strings.Contains(p, ".git/dckt/AGENTS.md") && strings.Contains(p, "private repository") { + return nil + } + } + return []string{fmt.Sprintf("%s §%q: no paragraph binds `.git/dckt/AGENTS.md` to a private repository", c.file, c.section)} +} + +func TestPrivateInstructionsDocContracts(t *testing.T) { + root := guardRoot(t) + for _, c := range privateInstructionsDocContracts { + for _, v := range scanDocSection(readMaintained(t, root, c.file), c) { + t.Errorf("[%s] %s", c.change, v) + } + } + for _, v := range scanPrivatePromotion(readMaintained(t, root, privatePromotionSection.file), privatePromotionSection) { + t.Errorf("[%s] %s", privatePromotionSection.change, v) + } + + t.Run("non_vacuity", func(t *testing.T) { + c := privatePromotionSection + ok := c.section + "\nIn a private repository it lands in\n`.git/dckt/AGENTS.md`.\n" + c.terminator + if got := scanPrivatePromotion(ok, c); len(got) != 0 { + t.Errorf("a bound paragraph was rejected: %v", got) + } + split := c.section + "\nIn a private repository.\n\nIt lands in `.git/dckt/AGENTS.md`.\n" + c.terminator + if got := scanPrivatePromotion(split, c); len(got) != 1 { + t.Errorf("an unbound pair across paragraphs was accepted: %v", got) + } + if got := scanPrivatePromotion(c.section+"\nIn a private repository `.git/dckt/AGENTS.md`.\n", c); len(got) != 1 { + t.Errorf("a missing terminator was accepted: %v", got) + } + }) +} diff --git a/internal/reposeed/plan.go b/internal/reposeed/plan.go index 0d715c78c..ddb52a5db 100644 --- a/internal/reposeed/plan.go +++ b/internal/reposeed/plan.go @@ -17,6 +17,7 @@ import ( "github.com/danielhanold/docket/internal/harness" "github.com/danielhanold/docket/internal/harness/cursor" "github.com/danielhanold/docket/internal/install" + "github.com/danielhanold/docket/internal/layout" ) // The four harness tokens this planner answers to. They mirror the validated @@ -86,14 +87,9 @@ type PlanInput struct { // several. Every planned path must resolve inside WorktreeRoot after // filepath.Clean; a path that escapes is an error. func Plan(in PlanInput) ([]install.Target, map[string][]string, error) { - selected := map[string]bool{} - for _, h := range in.Harnesses { - switch h { - case harnessClaude, harnessCodex, harnessOpencode, harnessCursor: - selected[h] = true - default: - return nil, nil, fmt.Errorf("reposeed: unknown harness token %q", h) - } + selected, err := selectHarnesses(in.Harnesses) + if err != nil { + return nil, nil, err } if len(selected) == 0 { return nil, nil, nil @@ -201,6 +197,87 @@ func Plan(in PlanInput) ([]install.Target, map[string][]string, error) { return targets, owners, nil } +// selectHarnesses validates the opt-in tokens and returns them as a set. An +// unknown token is an error (defense in depth; see the harness constants). +func selectHarnesses(tokens []string) (map[string]bool, error) { + selected := map[string]bool{} + for _, h := range tokens { + switch h { + case harnessClaude, harnessCodex, harnessOpencode, harnessCursor: + selected[h] = true + default: + return nil, fmt.Errorf("reposeed: unknown harness token %q", h) + } + } + return selected, nil +} + +// PrivatePlanInput is the pure input to PlanPrivate. WorktreeRoot is the +// PRIMARY worktree, a canonical absolute path; CommonDir is its git common dir +// and must be WorktreeRoot/.git. Harnesses are the validated opt-in tokens this +// run plans for (a --harness scope narrows them); OptIns is the repository's +// full opt-in set, which picks the shared file's interior, and nil means +// Harnesses is already the full set. RunTracker is the run-tracker payload the +// interior carries. +type PrivatePlanInput struct { + WorktreeRoot string + CommonDir string + Harnesses []string + OptIns []string + RunTracker []byte +} + +// PlanPrivate renders a private repository's single parent-facing target: the +// dispatch managed block in /dckt/AGENTS.md, owned by every selected +// harness, carrying the Codex interior when codex is opted in. The interior +// follows the full opt-in set, not the selection: the file is shared, so a run +// scoped away from codex must not strip the clause codex still owns. It plans nothing +// in the working tree and nothing at all when no harness is selected. A +// CommonDir other than WorktreeRoot/.git (a separate git dir) is an error. +func PlanPrivate(in PrivatePlanInput) ([]install.Target, map[string][]string, error) { + selected, err := selectHarnesses(in.Harnesses) + if err != nil { + return nil, nil, err + } + root := filepath.Clean(in.WorktreeRoot) + common := filepath.Clean(in.CommonDir) + if common != filepath.Join(root, ".git") { + return nil, nil, fmt.Errorf("reposeed: unsupported separate-git-dir layout: git common dir %q is not %q", + common, filepath.Join(root, ".git")) + } + if len(selected) == 0 { + return nil, nil, nil + } + + interiorSet := selected + if in.OptIns != nil { + if interiorSet, err = selectHarnesses(in.OptIns); err != nil { + return nil, nil, err + } + } + content := harness.DispatchInterior(in.RunTracker) + if interiorSet[harnessCodex] { + content = harness.CodexDispatchInterior(in.RunTracker) + } + path := filepath.Clean(layout.PrivateInstructionsPath(common)) + if !contained(root, path) { + return nil, nil, fmt.Errorf("reposeed: planned path %q escapes worktree root %q", path, root) + } + owners := make([]string, 0, len(selected)) + for h := range selected { + owners = append(owners, h) + } + sort.Strings(owners) + return []install.Target{{ + Path: path, + Kind: install.KindManagedBlock, + Content: []byte(content), + BlockName: dispatchBlockName, + Annotation: dispatchAnnotation, + Role: roleDispatch, + }}, map[string][]string{path: owners}, nil +} + // contained reports whether cleaned path p lies at or under cleaned root. Both // are already filepath.Clean'd; the separator guard keeps "/repofoo" from // reading as inside "/repo". diff --git a/internal/reposeed/plan_test.go b/internal/reposeed/plan_test.go index f1023ec70..dde498dc3 100644 --- a/internal/reposeed/plan_test.go +++ b/internal/reposeed/plan_test.go @@ -11,6 +11,7 @@ import ( "github.com/danielhanold/docket/internal/harness" "github.com/danielhanold/docket/internal/harness/cursor" "github.com/danielhanold/docket/internal/install" + "github.com/danielhanold/docket/internal/layout" ) // runTracker is a stand-in run-tracker payload. Plan is pure and never parses it, so @@ -422,3 +423,121 @@ func TestPlanRunnerFreeAndByteStable(t *testing.T) { } } } + +func TestPlanPrivate(t *testing.T) { + commonDir := filepath.Join(worktreeRoot, ".git") + privateFile := layout.PrivateInstructionsPath(commonDir) + plain := []byte(harness.DispatchInterior(runTracker)) + codex := []byte(harness.CodexDispatchInterior(runTracker)) + + cases := []struct { + name string + harnesses []string + interior []byte + owners []string + }{ + {"claude", []string{"claude"}, plain, []string{"claude"}}, + {"claude+codex", []string{"claude", "codex"}, codex, []string{"claude", "codex"}}, + {"cursor", []string{"cursor"}, plain, []string{"cursor"}}, + {"opencode", []string{"opencode"}, plain, []string{"opencode"}}, + {"all four", []string{"opencode", "cursor", "codex", "claude"}, codex, []string{"claude", "codex", "cursor", "opencode"}}, + } + + var union []install.Target + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + targets, owners, err := PlanPrivate(PrivatePlanInput{ + WorktreeRoot: worktreeRoot, + CommonDir: commonDir, + Harnesses: tc.harnesses, + RunTracker: runTracker, + }) + if err != nil { + t.Fatalf("PlanPrivate: %v", err) + } + if len(targets) != 1 { + t.Fatalf("got %d targets, want exactly 1: %+v", len(targets), targets) + } + tg := targets[0] + if tg.Path != privateFile { + t.Fatalf("path = %q, want %q", tg.Path, privateFile) + } + if tg.Kind != install.KindManagedBlock || tg.BlockName != dispatchBlockName || + tg.Annotation != dispatchAnnotation || tg.Role != roleDispatch { + t.Fatalf("target identity = %+v, want the shared dispatch managed block", tg) + } + if !bytes.Equal(tg.Content, tc.interior) { + t.Fatalf("interior mismatch:\n got %q\nwant %q", tg.Content, tc.interior) + } + if len(owners) != 1 || !reflect.DeepEqual(owners[privateFile], tc.owners) { + t.Fatalf("owners = %v, want {%q: %v}", owners, privateFile, tc.owners) + } + union = append(union, targets...) + }) + } + + t.Run("no harnesses", func(t *testing.T) { + targets, owners, err := PlanPrivate(PrivatePlanInput{ + WorktreeRoot: worktreeRoot, CommonDir: commonDir, RunTracker: runTracker, + }) + if err != nil || len(targets) != 0 || len(owners) != 0 { + t.Fatalf("PlanPrivate([]) = (%v, %v, %v), want nothing", targets, owners, err) + } + }) + + t.Run("never a working-tree surface", func(t *testing.T) { + if len(union) == 0 { + t.Fatal("no targets collected") + } + for _, tg := range union { + if tg.Path != privateFile { + t.Errorf("target %q is not the private file", tg.Path) + } + if filepath.Base(tg.Path) == "CLAUDE.md" { + t.Errorf("target %q is a CLAUDE.md", tg.Path) + } + if strings.Contains(tg.Path, string(filepath.Separator)+".cursor"+string(filepath.Separator)) { + t.Errorf("target %q lies under .cursor/", tg.Path) + } + if tg.Path == filepath.Join(worktreeRoot, "AGENTS.md") { + t.Errorf("target %q is the working-tree AGENTS.md", tg.Path) + } + } + }) + + t.Run("scoped selection keeps the opted-in codex interior", func(t *testing.T) { + targets, owners, err := PlanPrivate(PrivatePlanInput{ + WorktreeRoot: worktreeRoot, CommonDir: commonDir, + Harnesses: []string{"claude"}, OptIns: []string{"claude", "codex"}, RunTracker: runTracker, + }) + if err != nil || len(targets) != 1 { + t.Fatalf("PlanPrivate = (%d targets, %v), want one target", len(targets), err) + } + if !bytes.Equal(targets[0].Content, codex) { + t.Fatalf("interior = %q, want the Codex interior codex still owns", targets[0].Content) + } + if !reflect.DeepEqual(owners[privateFile], []string{"claude"}) { + t.Fatalf("owners = %v, want only the in-scope claude", owners) + } + }) + + t.Run("separate git dir errors", func(t *testing.T) { + _, _, err := PlanPrivate(PrivatePlanInput{ + WorktreeRoot: worktreeRoot, CommonDir: "/elsewhere/repo.git", + Harnesses: []string{"claude"}, RunTracker: runTracker, + }) + if err == nil || !strings.Contains(err.Error(), "separate-git-dir") { + t.Fatalf("err = %v, want a separate-git-dir layout error", err) + } + }) + + t.Run("unknown token errors", func(t *testing.T) { + _, _, err := PlanPrivate(PrivatePlanInput{ + WorktreeRoot: worktreeRoot, CommonDir: commonDir, + Harnesses: []string{"claude", "vim"}, RunTracker: runTracker, + }) + if err == nil || !strings.Contains(err.Error(), "unknown harness token") { + t.Fatalf("err = %v, want an unknown-token error", err) + } + }) +} diff --git a/skills/docket-convention/references/agent-layer.md b/skills/docket-convention/references/agent-layer.md index 4687a1c08..37c1409dd 100644 --- a/skills/docket-convention/references/agent-layer.md +++ b/skills/docket-convention/references/agent-layer.md @@ -89,6 +89,22 @@ The install also retires the global dispatch blocks earlier docket versions wrot (`~/.claude/CLAUDE.md` and the other harnesses' globals). The removal is proof-gated: a block is removed only while it still matches docket's exact ownership marker; a modified or foreign block is left untouched and reported. +### In a private repository + +In a private repository, `agent_harnesses` writes the `docket:dispatch` block into the private instructions file +`.git/dckt/AGENTS.md` and nothing in the working tree. Promoted lessons sit in the same file, outside the block. +`docket install` delivers that file through user-level triggers that carry no rules themselves: + +- two Claude Code `SessionStart` hooks in `~/.claude/settings.json`, + `dckt instructions --hook claude --section dispatch` and `dckt instructions --hook claude --section lessons`, one + per section so each fits Claude Code's per-hook size limit; +- a Cursor `sessionStart` hook in `~/.cursor/hooks.json`, `dckt instructions --hook cursor`; +- the OpenCode plugin `~/.config/opencode/plugins/dckt-instructions.js`; +- a `dckt:private-instructions` pointer block in `~/.codex/AGENTS.md`, which Codex follows on a best-effort basis. + +Outside a private repository `docket instructions` prints nothing, so the triggers are inert there. Uninstall +removes each trigger only while it is unchanged. + ## Launch posture Agent-source frontmatter may declare `launch: root-coordinator`; absence means the closed default diff --git a/skills/docket-convention/references/learnings.md b/skills/docket-convention/references/learnings.md index 2de1ba019..24f2bcc77 100644 --- a/skills/docket-convention/references/learnings.md +++ b/skills/docket-convention/references/learnings.md @@ -55,7 +55,10 @@ Tiering criterion: *"will the agent know to search for this?"* A rule that must `promotion_state: candidate` on the metadata branch and **never touches the integration branch** (ADR-0005). A human lands the graduation in the integration-branch agent-instructions file (`AGENTS.md`/`CLAUDE.md`, symlink-aware; `AGENTS.md` is the neutral spelling when neither -exists) and flips `promoted` + `promoted_to:`. A promoted finding leaves the topic groups for a +exists) and flips `promoted` + `promoted_to:`. In a private repository the graduation lands +instead in the private instructions file `.git/dckt/AGENTS.md`, outside its managed +`docket:dispatch` block, since a private repository carries no agent-instructions file in its +tree. A promoted finding leaves the topic groups for a compressed `## Promoted` appendix — but its file is **kept**, never deleted: it is the graduated rule's receipt, the dedup memory against re-recording a duplicate, and a one-line-reversible demotion path. diff --git a/tests/runtime-budgets.tsv b/tests/runtime-budgets.tsv index 3ad56746b..43fc65386 100644 --- a/tests/runtime-budgets.tsv +++ b/tests/runtime-budgets.tsv @@ -43,6 +43,7 @@ tests/test_go_integration_app_reporecovery.sh 20 parallel tests/test_go_integration_app_reporepair.sh 30 parallel tests/test_go_integration_app_reposetup.sh 35 parallel tests/test_go_integration_app_reposetup_race.sh 25 parallel +tests/test_go_integration_app_privateinstructions.sh 30 parallel tests/test_go_integration_app_reposynccheck.sh 30 parallel tests/test_go_integration_app_state.sh 50 parallel tests/test_go_integration_app_workflow.sh 35 parallel diff --git a/tests/test_go_integration_app_privateinstructions.sh b/tests/test_go_integration_app_privateinstructions.sh new file mode 100755 index 000000000..88a4b0fda --- /dev/null +++ b/tests/test_go_integration_app_privateinstructions.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# docket-suite: go +# tests/test_go_integration_app_privateinstructions.sh — Go integration shard +# (change 0535): a private repository's instructions file — the repository phase +# and private init write only /dckt/AGENTS.md, retire surfaces an +# earlier install wrote into the working tree, and leave shared repositories +# unchanged — behind the `integration` build tag, prefix +# ^TestIntegrationPrivateInstructions. 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="TestIntegrationPrivateInstructions" +SHARD_MODE="normal" + +. "$REPO/tests/lib/go-integration-shard.sh" +shard_inspect_maybe +run_integration_shard +exit "$fail"