From c8dfa8bb8336980a1a004ed07823c6fe874e156b Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 13:39:59 -0400 Subject: [PATCH 01/17] Add design spec: Load a private repository's agent instructions without repository files --- ...agent-instructions-without-repos-design.md | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-06-load-a-private-repository-s-agent-instructions-without-repos-design.md 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..57603d5a7 --- /dev/null +++ b/docs/superpowers/specs/2026-10-06-load-a-private-repository-s-agent-instructions-without-repos-design.md @@ -0,0 +1,125 @@ +# 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, it gets a private instructions file under `.git/dckt/`, plus one static, rule-free trigger per harness, installed once per machine, that loads it. 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*). +- **Claude Code `SessionStart` hooks** configured in user settings inject their output as session context. This session itself received plugin context that way. + +## 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. **Delivery is a hybrid, chosen over a pointer for every harness:** + - Claude Code loads the file mechanically through a user-level `SessionStart` hook. + - Codex and OpenCode get a static pointer in their user-level AGENTS.md. + + The hook is a guarantee where the harness offers one, and the pointer is the best available elsewhere. +3. **No drift.** Neither 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.** Both surfaces invoke `dckt` (#534), and the pointer's managed markers use the `dckt:` prefix, so nothing in the user's home directory reads "docket". +5. **Cursor.** It reads rules only from the project, so a private repository gets `.cursor/rules/dckt-dispatch.mdc`, excluded through `.git/info/exclude`. The human accepted this as the one visible trace in a private working tree. +6. **Scope.** The user-level triggers are written once per machine by `docket install`. 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 no AGENTS.md or CLAUDE.md dispatch block and no `.cursor/rules/docket-dispatch.mdc` in the worktree. +- 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. +- **`--hook`:** wraps the same content in Claude Code's `SessionStart` hook output shape (additional context), and prints nothing outside private repositories. + +### 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 | `SessionStart` hook entry in `~/.claude/settings.json` running `dckt instructions --hook` | loads the rules automatically at session start; adds nothing outside private repositories | +| Codex | managed pointer block in `~/.codex/AGENTS.md` | tells the model to run `dckt instructions` once per session and follow its output | +| OpenCode | managed pointer block in `~/.config/opencode/AGENTS.md` | same as Codex | +| Cursor | none at user level | per-repository excluded rule `.cursor/rules/dckt-dispatch.mdc` | + +The pointer wording is final in the plan, but it must stay rule-free and free of the word "docket": + +```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:** + +- **The hook entry** is identified by its exact command string. Install adds it only when absent and merges without reformatting or dropping any other setting. Uninstall removes only an exact match, and reports, rather than touches, a modified entry. +- **Pointer blocks** use 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, and `docket instructions` still works by hand. + +### First plan task (spike) + +In a fresh session per harness, confirm that: + +- Claude Code's user-level `SessionStart` hook delivers the output as context; +- Codex and OpenCode follow the pointer and run the command; +- Cursor reads an excluded rule file. + +Record the results in the results file before building on them. If a harness fails, stop and report rather than inventing another mechanism. + +### Documentation + +`docket instructions` command help, and the skills (convention: private promotion destination; agent-layer reference: private delivery). No `docs/` pages. + +## Acceptance criteria + +1. **The private file.** In a private repository, `/dckt/AGENTS.md` carries the dispatch block (Codex clause when selected). No AGENTS.md, CLAUDE.md, or docket-named Cursor rule is written in the worktree, and `.cursor/rules/dckt-dispatch.mdc` is excluded and ignored by git. +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; + - `--hook` emits valid `SessionStart` output in private repositories only. +3. **Installing the triggers:** + - install adds the hook entry and the two pointer blocks; + - a second install is a no-op; + - an unrelated `settings.json` key and hook survive byte-for-byte; + - uninstall removes only docket's exact entries. +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. **Learnings promotion** in a private repository names the private file as its destination. +6. **Shared repositories are unchanged:** repository-level blocks, and no private file. +7. **Fresh-session acceptance per harness** (the spike) is recorded in the results file. A private repository's Claude session receives the dispatch rules without any repository file. + +## ADRs expected + +A private repository's parent-facing rules live in `/dckt/AGENTS.md`. They are reached through content-free user-level triggers: a Claude `SessionStart` hook, and pointer blocks for Codex and OpenCode. 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). From c306d18a19b0a653ea29a7409dd5eecf846f9615 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 16:39:32 -0400 Subject: [PATCH 02/17] Update design spec: Load a private repository's agent instructions without repository files --- ...agent-instructions-without-repos-design.md | 156 +++++++++++++----- 1 file changed, 119 insertions(+), 37 deletions(-) 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 index 57603d5a7..4bf329b59 100644 --- 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 @@ -6,7 +6,12 @@ Change #535, groomed interactively on 2026-10-05, split out of #532 in the priva ## 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, it gets a private instructions file under `.git/dckt/`, plus one static, rule-free trigger per harness, installed once per machine, that loads it. Promoted lessons move into the same private file. +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 @@ -18,20 +23,51 @@ In shared repositories, the parent agent learns docket's dispatch and run-tracke - 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*). -- **Claude Code `SessionStart` hooks** configured in user settings inject their output as session context. This session itself received plugin context that way. + +### 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. **Delivery is a hybrid, chosen over a pointer for every harness:** - - Claude Code loads the file mechanically through a user-level `SessionStart` hook. - - Codex and OpenCode get a static pointer in their user-level AGENTS.md. - - The hook is a guarantee where the harness offers one, and the pointer is the best available elsewhere. -3. **No drift.** Neither 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.** Both surfaces invoke `dckt` (#534), and the pointer's managed markers use the `dckt:` prefix, so nothing in the user's home directory reads "docket". -5. **Cursor.** It reads rules only from the project, so a private repository gets `.cursor/rules/dckt-dispatch.mdc`, excluded through `.git/info/exclude`. The human accepted this as the one visible trace in a private working tree. -6. **Scope.** The user-level triggers are written once per machine by `docket install`. The private file is written per repository. `agent_harnesses` stays a per-repository choice, exactly as in shared mode. +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 @@ -40,7 +76,7 @@ In shared repositories, the parent agent learns docket's dispatch and run-tracke 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 no AGENTS.md or CLAUDE.md dispatch block and no `.cursor/rules/docket-dispatch.mdc` in the worktree. +- 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. @@ -50,7 +86,12 @@ 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. -- **`--hook`:** wraps the same content in Claude Code's `SessionStart` hook output shape (additional context), and prints nothing outside private repositories. +- **`--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 @@ -58,12 +99,25 @@ A new read-only catalog operation. | Harness | Surface | What it does | |---|---|---| -| Claude Code | `SessionStart` hook entry in `~/.claude/settings.json` running `dckt instructions --hook` | loads the rules automatically at session start; adds nothing outside private repositories | -| Codex | managed pointer block in `~/.codex/AGENTS.md` | tells the model to run `dckt instructions` once per session and follow its output | -| OpenCode | managed pointer block in `~/.config/opencode/AGENTS.md` | same as Codex | -| Cursor | none at user level | per-repository excluded rule `.cursor/rules/dckt-dispatch.mdc` | +| 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 pointer wording is final in the plan, but it must stay rule-free and free of the word "docket": +The Codex pointer: ```markdown @@ -77,45 +131,73 @@ follow it for the rest of the session. If it prints nothing, ignore this section **Safety requirements:** -- **The hook entry** is identified by its exact command string. Install adds it only when absent and merges without reformatting or dropping any other setting. Uninstall removes only an exact match, and reports, rather than touches, a modified entry. -- **Pointer blocks** use 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. +- **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, and `docket instructions` still works by hand. +- **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. -### First plan task (spike) +### Fresh-session acceptance (replaces the spike task) -In a fresh session per harness, confirm that: +The grooming spike proved each mechanism with a stub. The build repeats the check against the real built `dckt`: -- Claude Code's user-level `SessionStart` hook delivers the output as context; -- Codex and OpenCode follow the pointer and run the command; -- Cursor reads an excluded rule file. +- 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. -Record the results in the results file before building on them. If a harness fails, stop and report rather than inventing another mechanism. +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, and the skills (convention: private promotion destination; agent-layer reference: private delivery). No `docs/` pages. +- `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). No AGENTS.md, CLAUDE.md, or docket-named Cursor rule is written in the worktree, and `.cursor/rules/dckt-dispatch.mdc` is excluded and ignored by git. +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; - - `--hook` emits valid `SessionStart` output in private repositories only. + - `--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 hook entry and the two pointer blocks; + - 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` key and hook survive byte-for-byte; - - uninstall removes only docket's exact entries. + - 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. **Learnings promotion** in a private repository names the private file as its destination. -6. **Shared repositories are unchanged:** repository-level blocks, and no private file. -7. **Fresh-session acceptance per harness** (the spike) is recorded in the results file. A private repository's Claude session receives the dispatch rules without any repository file. +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: a Claude `SessionStart` hook, and pointer blocks for Codex and OpenCode. This narrowly revisits 0351's retirement of user-level parent-facing writes. Relates to ADR-0036 and ADR-0078. +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 From 6b3e64f4c07cc93498fdd745112623ed40aad971 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 16:43:31 -0400 Subject: [PATCH 03/17] feat(document): neutral dckt: marker namespace for managed blocks --- internal/document/document.go | 4 +- internal/document/fuzz_test.go | 13 +++-- internal/document/markers.go | 68 ++++++++++++++++------ internal/document/markers_test.go | 97 ++++++++++++++++++++++++++++++- 4 files changed, 157 insertions(+), 25 deletions(-) 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) + } + } +} From 4e80ab832f08e881b1be3a2e33bf53ef75a6708a Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 16:59:26 -0400 Subject: [PATCH 04/17] feat(install): retire one managed block and write another in the same file --- internal/install/inspect.go | 10 +- internal/install/retire_test.go | 200 ++++++++++++++++++++++++++++++++ internal/install/txn.go | 50 ++++++-- internal/install/txn_test.go | 167 ++++++++++++++++++++++++++ 4 files changed, 413 insertions(+), 14 deletions(-) diff --git a/internal/install/inspect.go b/internal/install/inspect.go index 7d4119f68..cafce1f3b 100644 --- a/internal/install/inspect.go +++ b/internal/install/inspect.go @@ -239,18 +239,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" } 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) + } +} From 26160addeeba9d508c399c79802763aaab357311 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 17:10:18 -0400 Subject: [PATCH 05/17] feat(install): hook-entries target kind for Claude Code and Cursor hooks files --- internal/install/hook_entries.go | 608 ++++++++++++++++++++++++ internal/install/hook_entries_test.go | 657 ++++++++++++++++++++++++++ internal/install/inspect.go | 50 ++ internal/install/state.go | 17 + internal/install/target.go | 22 +- internal/install/txn.go | 65 +++ internal/install/uninstall.go | 16 + 7 files changed, 1434 insertions(+), 1 deletion(-) create mode 100644 internal/install/hook_entries.go create mode 100644 internal/install/hook_entries_test.go 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..866e99ff4 --- /dev/null +++ b/internal/install/hook_entries_test.go @@ -0,0 +1,657 @@ +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 + for _, s := range []string{`{"hooks": []}`, wrongEvent, nullEvent, `[1]`, `null`, `not json`, `{"hooks": null}`, `{} {}`} { + out = append(out, tc{"invalid " + s, str(s), DispositionConflict, ReasonManagedBlockInvalid, 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) + } + if insp.Disposition != DispositionConflict || insp.Reason != ReasonOwnershipConflict || insp.Remedy != remedyHookFileNotRegular { + t.Errorf("inspection = %+v, want an ownership conflict 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) + } + }) + } +} + +// 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") + 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) + } + }) + } +} + +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 cafce1f3b..3937a3b17 100644 --- a/internal/install/inspect.go +++ b/internal/install/inspect.go @@ -84,6 +84,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) default: // validate has already refused every other kind. return Inspection{}, fmt.Errorf("%w: %s has unknown kind %q", ErrInvalidTarget, t.Path, t.Kind) @@ -215,6 +217,35 @@ 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". +func inspectHookEntries(t Target, info fs.FileInfo) (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. + return conflict(t, ReasonOwnershipConflict, 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 { + return conflict(t, ReasonManagedBlockInvalid, remedyHookFileInvalid), 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 +} + func conflict(t Target, reason, remedy string) Inspection { return Inspection{Target: t, Disposition: DispositionConflict, Reason: reason, Remedy: remedy} } @@ -226,6 +257,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 cannot add its entries in place; " + + "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); " + + "repair it by hand, then re-run" ) // remedyLinkDestination names the destination the plan wants, which is the one @@ -357,6 +392,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/state.go b/internal/install/state.go index d592b0a8d..b585fc09f 100644 --- a/internal/install/state.go +++ b/internal/install/state.go @@ -37,6 +37,9 @@ const ( KindFile TargetKind = "file" KindSymlink TargetKind = "symlink" KindManagedBlock TargetKind = "managed-block" + // KindHookEntries is a fixed list of hook commands inside a JSON hooks file + // the user also owns (see hook_entries.go). + KindHookEntries TargetKind = "hook-entries" ) // TargetRecord is the ownership proof for one installed target: enough to @@ -48,6 +51,10 @@ type TargetRecord struct { SHA256 string `json:"sha256,omitempty"` // file: whole file; managed-block: block interior BlockName string `json:"block_name,omitempty"` // managed-block only, e.g. "dispatch" Role string `json:"role"` // + // HookDialect and HookCommands are hook-entries only: the hooks-file shape + // and the exact commands docket's entries run, in install order. + HookDialect string `json:"hook_dialect,omitempty"` + HookCommands []string `json:"hook_commands,omitempty"` // Harness attributes the target to the harness whose planner produced it. // It is what makes a scoped run a scope rather than an uninstall: a run // that plans for one harness prunes only that harness's stale targets and @@ -181,6 +188,9 @@ func ValidateState(s *State) error { } func validateTarget(target TargetRecord) error { + if target.Kind != KindHookEntries && (target.HookDialect != "" || len(target.HookCommands) > 0) { + return fmt.Errorf("a %s target carries hook-entries fields", target.Kind) + } switch target.Kind { case KindFile: if target.SHA256 == "" || target.LinkTarget != "" || target.BlockName != "" { @@ -194,6 +204,13 @@ func validateTarget(target TargetRecord) error { if target.SHA256 == "" || target.BlockName == "" || target.LinkTarget != "" { return errors.New("invalid managed-block fields") } + case KindHookEntries: + if target.SHA256 == "" || target.BlockName != "" || target.LinkTarget != "" { + return errors.New("invalid hook-entries fields") + } + if err := checkHookFields(target.HookDialect, target.HookCommands); err != nil { + return fmt.Errorf("invalid hook-entries fields: %w", err) + } default: return fmt.Errorf("unsupported target kind %q", target.Kind) } diff --git a/internal/install/target.go b/internal/install/target.go index a2b130bc8..c5a2385eb 100644 --- a/internal/install/target.go +++ b/internal/install/target.go @@ -19,7 +19,12 @@ type Target struct { LinkTarget string // KindSymlink: desired destination (canonicalised by the planner) BlockName string // KindManagedBlock: marker name ("dispatch") Annotation string // KindManagedBlock: start-marker annotation - Role string + // HookDialect and HookCommands are KindHookEntries only: the hooks-file + // shape (HookDialectClaude, HookDialectCursor) and the exact commands, in + // install order, whose entries the target owns in that one file. + HookDialect string + HookCommands []string + Role string // Mode overrides the permissions an applied KindFile target is published // with. Zero means "the installer's policy": the mode an updated file // already had, else 0o644. It exists for the one target whose usefulness @@ -60,6 +65,9 @@ func (t Target) validate() error { case !filepath.IsAbs(t.Path): return fmt.Errorf("%w: path %q is not absolute", ErrInvalidTarget, t.Path) } + if t.Kind != KindHookEntries && (t.HookDialect != "" || len(t.HookCommands) > 0) { + return fmt.Errorf("%w: %s is a %s target carrying hook-entries fields", ErrInvalidTarget, t.Path, t.Kind) + } switch t.Kind { case KindFile: return nil @@ -76,6 +84,14 @@ func (t Target) validate() error { return fmt.Errorf("%w: %s is a managed block with no block name", ErrInvalidTarget, t.Path) } return nil + case KindHookEntries: + if t.BlockName != "" || t.LinkTarget != "" { + return fmt.Errorf("%w: %s is a hook-entries target carrying a block name or link target", ErrInvalidTarget, t.Path) + } + if err := checkHookFields(t.HookDialect, t.HookCommands); err != nil { + return fmt.Errorf("%w: %s: %v", ErrInvalidTarget, t.Path, err) + } + return nil default: return fmt.Errorf("%w: %s has unknown kind %q", ErrInvalidTarget, t.Path, t.Kind) } @@ -105,6 +121,10 @@ func RecordFor(t Target) (TargetRecord, error) { case KindManagedBlock: rec.BlockName = t.BlockName rec.SHA256 = interiorDigest(t.Content) + case KindHookEntries: + rec.HookDialect = t.HookDialect + rec.HookCommands = append([]string(nil), t.HookCommands...) + rec.SHA256 = hookEntriesDigest(t.HookDialect, t.HookCommands) } return rec, nil } diff --git a/internal/install/txn.go b/internal/install/txn.go index 3d732cfde..4d9f217b9 100644 --- a/internal/install/txn.go +++ b/internal/install/txn.go @@ -1,6 +1,7 @@ package install import ( + "bytes" "crypto/rand" "encoding/hex" "encoding/json" @@ -430,6 +431,11 @@ func (t *Txn) applyStep(i int) error { // deletion. return t.removeManagedBlock(step) } + if step.Kind == KindHookEntries { + // Likewise a hook-entries removal cuts only docket's entries out of a + // hooks file the user also owns. + return t.removeHookEntriesStep(step, target) + } // Nothing is created on the way to a deletion, so the destination's // directory is left exactly as it was found — including absent. return removeIfPresent(t.fs, step.Path) @@ -459,6 +465,24 @@ func (t *Txn) applyStep(i int) error { } return t.writeThroughStaging(step, data, target.Mode) + case KindHookEntries: + // The same reasoning as a managed block: publishing by rename would + // replace a link with a regular file, so a link found here appeared after + // the inspection and the transaction refuses. + if step.PreImage.State == preSymlink { + return fmt.Errorf("%w: %s is a symlink, which docket never edits a hooks file through", + ErrInvalidTarget, step.Path) + } + existing, err := os.ReadFile(step.Path) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("reading %s: %w", step.Path, err) + } + out, err := insertHookEntries(existing, target.HookDialect, target.HookCommands) + if err != nil { + return fmt.Errorf("adding hook entries to %s: %w", step.Path, err) + } + return t.writeThroughStaging(step, out, target.Mode) + case KindSymlink: // Symlink refuses an occupied path, so an update clears the old one // first. A destination that has appeared since the plan was made is left @@ -587,6 +611,33 @@ func (t *Txn) removeManagedBlock(step *journalStep) error { return t.writeThroughStaging(step, out, 0) } +// removeHookEntriesStep retires docket's hook entries from a hooks file the +// user also owns, cutting exactly those entries and keeping every other byte. +// It mirrors removeManagedBlock, and rewrites the file only when a cut was made. +func (t *Txn) removeHookEntriesStep(step *journalStep, target Target) error { + switch step.PreImage.State { + case preAbsent: + // The hooks file is already gone: nothing is left to retire. + return nil + case preSymlink: + return fmt.Errorf("%w: %s is a symlink, which docket never edits a hooks file through", + ErrInvalidTarget, step.Path) + } + existing, err := os.ReadFile(step.Path) + if err != nil { + return fmt.Errorf("reading %s: %w", step.Path, err) + } + out, err := removeHookEntries(existing, target.HookDialect, target.HookCommands) + if err != nil { + return fmt.Errorf("removing hook entries from %s: %w", step.Path, err) + } + if bytes.Equal(out, existing) { + return nil + } + // want == 0 keeps the file's pre-image mode: the file is the user's. + return t.writeThroughStaging(step, out, 0) +} + // Rollback restores every journaled destination, newest first, and removes the // journal once the world is back as it was found. A rollback that cannot finish // leaves the journal in place on purpose: it is the only remaining record of @@ -1038,6 +1089,20 @@ func removalTarget(rec TargetRecord) (Target, error) { BlockName: rec.BlockName, Role: rec.Role, }, nil + case KindHookEntries: + // The commands are what the removal cuts, so a record without them + // describes no deletable thing. + if len(rec.HookCommands) == 0 { + return Target{}, fmt.Errorf("%w: %s is recorded as hook entries with no commands", + ErrInvalidTarget, rec.Path) + } + return Target{ + Path: rec.Path, + Kind: rec.Kind, + HookDialect: rec.HookDialect, + HookCommands: append([]string(nil), rec.HookCommands...), + Role: rec.Role, + }, nil default: return Target{}, fmt.Errorf("%w: %s is recorded as %q, which a transaction never deletes", ErrInvalidTarget, rec.Path, rec.Kind) 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 From 9e062f75d8299e6743f197621476214b589cfe05 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 17:19:45 -0400 Subject: [PATCH 06/17] feat(harness): plan the private-instructions triggers for all four harnesses --- .../repository_private_integration_test.go | 20 +++ internal/harness/claude/claude.go | 7 +- internal/harness/claude/claude_test.go | 18 +++ internal/harness/codex/codex.go | 8 +- internal/harness/codex/codex_test.go | 29 +++- internal/harness/cross_harness_test.go | 24 ++- internal/harness/cursor/cursor.go | 9 +- internal/harness/cursor/cursor_test.go | 18 +++ internal/harness/export_test.go | 6 + internal/harness/opencode/opencode.go | 8 +- internal/harness/opencode/opencode_test.go | 18 +++ internal/harness/trigger.go | 70 +++++++++ internal/harness/trigger_test.go | 147 ++++++++++++++++++ internal/install/service_test.go | 24 ++- 14 files changed, 394 insertions(+), 12 deletions(-) create mode 100644 internal/harness/export_test.go create mode 100644 internal/harness/trigger.go create mode 100644 internal/harness/trigger_test.go diff --git a/internal/app/repository_private_integration_test.go b/internal/app/repository_private_integration_test.go index 00bc8808d..cae8fcc21 100644 --- a/internal/app/repository_private_integration_test.go +++ b/internal/app/repository_private_integration_test.go @@ -54,9 +54,28 @@ func newPrivateInitRepo(t *testing.T, files map[string]string) (*initRepo, strin return r, data } +// inheritedXDGDataHome is XDG_DATA_HOME as the test process inherited it, +// read at package initialization, before any test pins it. +var inheritedXDGDataHome = os.Getenv("XDG_DATA_HOME") + +// pinInitUserRoots keeps an init run off the real user's machine roots, since +// init reaches the installer (installAuthorizedSurfaces resolves its roots from +// the environment). HOME always moves to a fresh temp dir; XDG_CONFIG_HOME is +// pinned by newGitClient (or runInitWithGlobal); XDG_DATA_HOME moves to a fresh +// temp dir unless the test already pinned it, because a private store the test +// inspects (newPrivateInitRepo's data home) lives there. +func pinInitUserRoots(t *testing.T) { + t.Helper() + t.Setenv("HOME", testsupport.TempDir(t)) + if v := os.Getenv("XDG_DATA_HOME"); v == "" || v == inheritedXDGDataHome { + t.Setenv("XDG_DATA_HOME", testsupport.TempDir(t)) + } +} + // runInitWith runs RunRepositoryInit against the invocation clone with o. func (r *initRepo) runInitWith(t *testing.T, o InitOptions) RepositoryOpResult { t.Helper() + pinInitUserRoots(t) client := newGitClient(t) return RunRepositoryInit(context.Background(), SetupDeps{Git: client, RepoDir: r.invocation}, o) } @@ -66,6 +85,7 @@ func (r *initRepo) runInitWith(t *testing.T, o InitOptions) RepositoryOpResult { // the client is built. func (r *initRepo) runInitWithGlobal(t *testing.T, globalYML string, o InitOptions) RepositoryOpResult { t.Helper() + pinInitUserRoots(t) client := newGitClient(t) cfgHome := testsupport.TempDir(t) if err := os.MkdirAll(filepath.Join(cfgHome, "docket"), 0o755); err != nil { 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/service_test.go b/internal/install/service_test.go index 2e03d1a15..d6f4a652f 100644 --- a/internal/install/service_test.go +++ b/internal/install/service_test.go @@ -619,7 +619,6 @@ func TestInstallFreshApplies(t *testing.T) { // global destinations are created. for _, p := range [][]string{ {".claude", "CLAUDE.md"}, - {".codex", "AGENTS.md"}, {".cursor", "rules", "docket-dispatch.mdc"}, {".config", "opencode", "AGENTS.md"}, } { @@ -627,6 +626,29 @@ func TestInstallFreshApplies(t *testing.T) { t.Errorf("fresh install created a global dispatch surface at %v (err=%v)", p, err) } } + // ~/.codex/AGENTS.md does exist now, but it holds only the content-free + // private-instructions pointer, never a dispatch block. + codexAgents := readFile(t, w.path(".codex", "AGENTS.md")) + if strings.Contains(codexAgents, "docket:dispatch") || strings.Contains(codexAgents, harness.DispatchHeading) { + t.Errorf("fresh install wrote a dispatch block into ~/.codex/AGENTS.md:\n%s", codexAgents) + } + if !strings.Contains(codexAgents, "\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\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. From 3b0dcb2b190fad8575285fd47de578b09c669a92 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 17:41:29 -0400 Subject: [PATCH 10/17] docs(skills): private-repository instructions file and its user-level triggers --- internal/assets/embedded/manifest.json | 10 +-- .../references/agent-layer.md | 16 ++++ .../docket-convention/references/learnings.md | 5 +- internal/repoguard/budgets_test.go | 4 +- .../private_instructions_docs_test.go | 81 +++++++++++++++++++ .../references/agent-layer.md | 16 ++++ .../docket-convention/references/learnings.md | 5 +- 7 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 internal/repoguard/private_instructions_docs_test.go diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index 24001efc5..988979af5 100644 --- a/internal/assets/embedded/manifest.json +++ b/internal/assets/embedded/manifest.json @@ -1,7 +1,7 @@ { "format_version": 1, "asset_protocol": 1, - "asset_set_id": "sha256:54fb90cc80ae3642203dd0b106f9f4671b5fd393bbcce731b1fede565391724d", + "asset_set_id": "sha256:c7cbcdc5291b858312ab91b9c886add0dcaf40f6ab2415d09fe040cf7d3a6b80", "entries": [ { "path": ".docket.example.yml", @@ -329,8 +329,8 @@ "path": "skills/docket-convention/references/agent-layer.md", "role": "skill", "mode": 420, - "size": 10261, - "sha256": "a082581b56864c0edb43aa907657b68777b58baddfbbaf8dde994a10d7b8cce5" + "size": 11302, + "sha256": "1584ba36c700ddf4cb2ea82fdcbc79e8d1325692ee4ed99ecdb68528d82d9e52" }, { "path": "skills/docket-convention/references/close-out.md", @@ -343,8 +343,8 @@ "path": "skills/docket-convention/references/learnings.md", "role": "skill", "mode": 420, - "size": 3331, - "sha256": "6d030f9d7be9a0dda61e2d890cad18fbdea10b37e1e0c21380a7531524da2cc1" + "size": 3560, + "sha256": "4429f9afd8cd1f268304136648f763de094560e540cf691d69da345622ad4eb8" }, { "path": "skills/docket-convention/references/stacked-changes.md", diff --git a/internal/assets/embedded/tree/skills/docket-convention/references/agent-layer.md b/internal/assets/embedded/tree/skills/docket-convention/references/agent-layer.md index 4687a1c08..37c1409dd 100644 --- a/internal/assets/embedded/tree/skills/docket-convention/references/agent-layer.md +++ b/internal/assets/embedded/tree/skills/docket-convention/references/agent-layer.md @@ -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/internal/assets/embedded/tree/skills/docket-convention/references/learnings.md b/internal/assets/embedded/tree/skills/docket-convention/references/learnings.md index 2de1ba019..24f2bcc77 100644 --- a/internal/assets/embedded/tree/skills/docket-convention/references/learnings.md +++ b/internal/assets/embedded/tree/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/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/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. From 2f31778f76733169dd468a92619966aa16b6c1fe Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 17:47:47 -0400 Subject: [PATCH 11/17] test: fresh-session acceptance of private-instructions delivery Fixture: private repository (repository init --private, agent_harnesses [claude, codex, cursor, opencode]) plus a linked worktree; triggers installed by `docket development install --harness ` into a temporary HOME, XDG_CONFIG_HOME, XDG_DATA_HOME and bin dir. Lesson token PRIVTOKEN-4cee33d3 appended to .git/dckt/AGENTS.md. `git status --porcelain` empty after init. `dckt instructions --section dispatch`: 7773 characters (< 10000). Second install run: no trigger changes (only the rebuilt binary updated). Markers: A = reply contains `run.verdict` and `Obey the resulting`; B = reply ends with the lesson token. The plan's literal prompt asks for the sentence starting "After the run returns", which ends before "Obey the resulting", so it cannot carry marker A in any harness; each harness was also given "Quote verbatim, in full, the numbered step 2 that starts with 'After the run returns' (every sentence of that step). Then say hello." | Harness | Version | Mode and flags | Location | Verdict | Evidence | |---|---|---|---|---|---| | Claude Code | 2.1.291 | claude -p --no-session-persistence --setting-sources project --strict-mcp-config --settings /.claude/settings.json | private repo; feature worktree | PASS | full-step prompt: "...run `run.verdict` ... Obey the resulting `run-*` report line exactly..." then "PRIVTOKEN-4cee33d3" in both locations; literal prompt (2 tries): sentence + token | | Claude Code | 2.1.291 | same | non-git dir (control) | PASS (control) | "I can't find any sentence starting with..."; no token | | Cursor | cursor-agent 2026.10.01-e373342 | cursor-agent -p --trust --output-format text; hooks.json copied to /.cursor/hooks.json | private repo | PASS | "...run `run.verdict` ... Obey the resulting `run-*` report line exactly..." then "PRIVTOKEN-4cee33d3."; without the project hooks file: no token, older wording from elsewhere | | Cursor hook | fixture binary | dckt instructions --hook cursor | CURSOR_PROJECT_DIR=; and empty + stdin workspace_roots=[] | PASS | both print {"additional_context": ...} carrying the token; garbage stdin: no output, exit 0 | | OpenCode | 1.18.31 | OPENCODE_CONFIG_DIR=/opencode opencode run (no permission flag); model deepseek/deepseek-v4-flash-0731, agent build | private repo; feature worktree | PASS | "Step 2: ...run `run.verdict` ... Obey the resulting `run-*` report line exactly..." then "PRIVTOKEN-4cee33d3"; control in non-git dir: no quote, no token | | Codex | codex-cli 0.154.0 | codex exec --sandbox workspace-write; model gpt-6-astra, approval never; pointer copied to /AGENTS.md (project-level stand-in) | private repo | PASS (recorded, non-blocking) | ran `dckt instructions` itself; reply carried both markers and the token; user-level ~/.codex/AGENTS.md loading not exercised | From 64cb9c7792b5210a8be5ed23692a8f1dbf91237e Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 17:59:28 -0400 Subject: [PATCH 12/17] test(process): kill the supervisor before its command in the unprovable-group recover test A single group-wide SIGKILL is not atomic across the group: the command could die first while a supervisor thread returned from cmd.Wait and wrote terminal.json, so Recover classified the run terminal and never reached the group probe the test exercises. Kill the supervisor alone, wait until it is reaped, then end the orphaned command, and assert the setup left no terminal record. --- internal/process/recover_test.go | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) 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 From e38c1349f3fade778d7d1eb0e92a7ec0f66ebd3a Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 18:16:51 -0400 Subject: [PATCH 13/17] fix(reposeed): keep the Codex clause in the private file on a scoped install Review finding: a scoped install in a private repository stripped the Codex clause from the shared private instructions file. With agent_harnesses [claude, codex], `docket install --harness claude` rewrote .git/dckt/AGENTS.md with the plain interior while codex stayed a recorded owner, because PlanPrivate chose the interior from the in-scope harnesses only. The private file is one block shared by every opted-in harness, so its interior must follow the full opt-in set. PrivatePlanInput gains OptIns (the repository's full opt-in set; nil means Harnesses already is), which picks the interior; the in-scope Harnesses still decide whether the file is planned and which owners this run contributes. ResolveRepoPhase passes the opt-ins. Regression tests: a claude-scoped run over [claude, codex] keeps the Codex clause (integration, through ResolveRepoPhase and the apply), and a PlanPrivate unit case pins the interior/owner split. --- .../private_instructions_integration_test.go | 39 +++++++++++++++++++ internal/app/repophase.go | 1 + internal/reposeed/plan.go | 20 ++++++++-- internal/reposeed/plan_test.go | 16 ++++++++ 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/internal/app/private_instructions_integration_test.go b/internal/app/private_instructions_integration_test.go index 7975bd2b7..32f21bc65 100644 --- a/internal/app/private_instructions_integration_test.go +++ b/internal/app/private_instructions_integration_test.go @@ -298,6 +298,45 @@ func TestIntegrationPrivateInstructionsOwnerRemovalKeepsSharedFile(t *testing.T) assertNoWorktreeSurfaces(t, primary) } +// TestIntegrationPrivateInstructionsScopedRunKeepsCodexClause proves a run +// scoped to one harness picks the shared file's interior from every opted-in +// harness: with claude and codex opted in, a claude-only run rewrites the block +// with the Codex clause still in it, because codex still owns the file. +func TestIntegrationPrivateInstructionsScopedRunKeepsCodexClause(t *testing.T) { + _, primary, common := newPrivateInstructionsRepo(t) + privatePath := layout.PrivateInstructionsPath(common) + const codexClause = "### Codex root-coordinator entry" + + setPrivateHarnesses(t, common, "[claude, codex]") + installSurfacesIn(t, primary) + if got := string(mustReadFile(t, privatePath)); !strings.Contains(got, codexClause) { + t.Fatalf("precondition: %s = %q, want the Codex clause", layout.PrivateInstructionsDisplay, got) + } + + rt, err := buildRunTracker() + if err != nil { + t.Fatal(err) + } + scoped, _, _, err := ResolveRepoPhase(context.Background(), newGitClient(t), primary, []string{"claude"}, rt, nil, config.ResolveContext{DefaultBranch: "main"}) + if err != nil { + t.Fatalf("claude-scoped ResolveRepoPhase: %v", err) + } + if len(scoped.Targets) != 1 || !strings.Contains(string(scoped.Targets[0].Content), codexClause) { + t.Fatalf("claude-scoped plan: %d targets, want one private target carrying the Codex clause", len(scoped.Targets)) + } + roots, err := install.ResolveRoots(os.UserHomeDir, os.Getenv) + if err != nil { + t.Fatal(err) + } + if _, err := applyRepoPhaseSurfaces(scoped, roots); err != nil { + t.Fatalf("applying the claude-scoped phase: %v", err) + } + if got := string(mustReadFile(t, privatePath)); !strings.Contains(got, dispatchStartMarker) || !strings.Contains(got, codexClause) { + t.Errorf("after a claude-scoped run: %s = %q, want the block with the Codex clause kept", layout.PrivateInstructionsDisplay, got) + } + assertNoWorktreeSurfaces(t, primary) +} + // TestIntegrationPrivateInstructionsSharedRepositoryUnchanged is acceptance 7: // a shared init writes the repository-level AGENTS.md block and the CLAUDE.md // link, and no private file (no .git/dckt at all). diff --git a/internal/app/repophase.go b/internal/app/repophase.go index f00db9946..dca958a81 100644 --- a/internal/app/repophase.go +++ b/internal/app/repophase.go @@ -152,6 +152,7 @@ func ResolveRepoPhase(ctx context.Context, git *gitcli.Client, repoDir string, h WorktreeRoot: root, CommonDir: commonDir, Harnesses: effective, + OptIns: optIns, RunTracker: runTracker, }) if err != nil { diff --git a/internal/reposeed/plan.go b/internal/reposeed/plan.go index 193365c6c..ddb52a5db 100644 --- a/internal/reposeed/plan.go +++ b/internal/reposeed/plan.go @@ -214,18 +214,24 @@ func selectHarnesses(tokens []string) (map[string]bool, error) { // 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 repository's validated -// opt-in tokens; RunTracker is the run-tracker payload the interior carries. +// 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 selected. It plans nothing +// 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) { @@ -243,8 +249,14 @@ func PlanPrivate(in PrivatePlanInput) ([]install.Target, map[string][]string, er 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 selected[harnessCodex] { + if interiorSet[harnessCodex] { content = harness.CodexDispatchInterior(in.RunTracker) } path := filepath.Clean(layout.PrivateInstructionsPath(common)) diff --git a/internal/reposeed/plan_test.go b/internal/reposeed/plan_test.go index fbf1cd241..dde498dc3 100644 --- a/internal/reposeed/plan_test.go +++ b/internal/reposeed/plan_test.go @@ -505,6 +505,22 @@ func TestPlanPrivate(t *testing.T) { } }) + 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", From 4593b6dfe2b4bef206ddabbeb42e04375edbefdc Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 18:23:33 -0400 Subject: [PATCH 14/17] fix(install): skip a hooks file that is not a regular file instead of failing the install A symlinked ~/.claude/settings.json or ~/.cursor/hooks.json (a dotfiles manager's link) made inspection report an ownership conflict, and any conflict failed the whole install. The hook entries are only a trigger, inert outside a private repository, so that must not block anything. Inspection now returns a new skip disposition for a non-regular hooks file. Install and development install leave the path byte-untouched, plan no step and publish no record for it (a prior record is dropped, and a harness left with no target is not listed), and every other target proceeds. Install, development install, and install check report the skip as a hook-file-not-editable warning naming the path and the remedy; check no longer counts it as drift. --- internal/app/finding_codes.go | 8 ++ internal/app/install.go | 16 +++ internal/app/install_test.go | 31 +++++ internal/install/hook_entries_test.go | 168 +++++++++++++++++++++++++- internal/install/inspect.go | 29 +++-- internal/install/service.go | 46 ++++++- internal/install/target.go | 5 + 7 files changed, 287 insertions(+), 16 deletions(-) diff --git a/internal/app/finding_codes.go b/internal/app/finding_codes.go index d2955cb6d..63b19667d 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, say): 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/install/hook_entries_test.go b/internal/install/hook_entries_test.go index 866e99ff4..380a374a0 100644 --- a/internal/install/hook_entries_test.go +++ b/internal/install/hook_entries_test.go @@ -345,8 +345,11 @@ func TestHookEntriesInspect(t *testing.T) { if err != nil { t.Fatal(err) } - if insp.Disposition != DispositionConflict || insp.Reason != ReasonOwnershipConflict || insp.Remedy != remedyHookFileNotRegular { - t.Errorf("inspection = %+v, want an ownership conflict with the not-a-regular-file remedy", insp) + // 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) @@ -355,6 +358,20 @@ func TestHookEntriesInspect(t *testing.T) { 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) + } + }) } } @@ -569,6 +586,153 @@ func TestHookEntriesInstallUninstallLifecycle(t *testing.T) { } } +// 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 { + for _, s := range out.Skipped { + if s.Target.Path == path && s.Disposition == DispositionSkip && s.Remedy == remedyHookFileNotRegular { + 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) + } + }) + } +} + 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"} diff --git a/internal/install/inspect.go b/internal/install/inspect.go index 3937a3b17..9f49d053b 100644 --- a/internal/install/inspect.go +++ b/internal/install/inspect.go @@ -17,12 +17,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 } @@ -225,8 +225,11 @@ func inspectManagedBlock(t Target, info fs.FileInfo, rec TargetRecord, hasRec bo func inspectHookEntries(t Target, info fs.FileInfo) (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. - return conflict(t, ReasonOwnershipConflict, remedyHookFileNotRegular), nil + // 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 { @@ -250,6 +253,12 @@ 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. @@ -257,8 +266,8 @@ 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 cannot add its entries in place; " + - "make it a regular file, 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); " + "repair it by hand, then re-run" ) diff --git a/internal/install/service.go b/internal/install/service.go index 4592ecccb..7e3458ca6 100644 --- a/internal/install/service.go +++ b/internal/install/service.go @@ -180,8 +180,14 @@ type Outcome struct { // They are findings, never failures: they leave Reason and Err alone, and // the app layer renders them as warnings. AliasFindings []AliasFinding - Reason string - Err error + // Skipped are the planned targets an operation left untouched because + // docket cannot edit them in place (DispositionSkip), each with its + // remedy. Like AliasFindings they are findings, never failures: they leave + // Reason and Err alone, no record is published for them, and the app layer + // renders them as warnings. + Skipped []Inspection + Reason string + Err error // Relayed marks a development-install PARENT outcome whose candidate has // already printed the sole result document to the shared stdout. The parent // itself planned nothing and wrote nothing; the CLI presenter emits no @@ -399,6 +405,12 @@ func Check(o Options) Outcome { if inspection.Disposition == DispositionNoop { continue } + if inspection.Disposition == DispositionSkip { + // What install leaves untouched and unrecorded is a finding here + // too, never drift: install itself proceeds past it. + out.Skipped = append(out.Skipped, inspection) + continue + } detail := string(inspection.Disposition) if inspection.Reason != "" { // A conflict seen from `check` is the same dead end it is from @@ -518,6 +530,11 @@ type plannedInstallation struct { // docket cannot prove it owns. A prior roleBinaryAlias record for one of // them grants nothing, so it is not carried into the published state. foreignAliases []string + // skipped are the cleaned paths of planned targets applyPlan found it + // cannot edit in place (DispositionSkip). Neither a record for them nor a + // prior one is published: docket left them untouched, so it owns nothing + // there now. + skipped map[string]bool } // applyPlan is the shared tail of Install and DevelopmentInstall: classify, @@ -559,13 +576,23 @@ func applyPlan(o Options, p plannedInstallation, repo *RepoPhase, out Outcome) O inspections := make([]Inspection, 0, len(p.targets)) var conflicts []Inspection + p.skipped = map[string]bool{} for _, t := range p.targets { inspection, err := InspectTarget(t, prior, legacy) if err != nil { return fail(out, ReasonFilesystemFailed, err) } - if inspection.Disposition == DispositionConflict { + switch inspection.Disposition { + case DispositionConflict: conflicts = append(conflicts, inspection) + case DispositionSkip: + // Left untouched and reported, never refused: it becomes no + // transaction step and no record (desiredState), while every other + // target proceeds. It stays in p.targets so the prune scan still + // sees the path as planned and never tries to remove it. + out.Skipped = append(out.Skipped, inspection) + p.skipped[filepath.Clean(t.Path)] = true + continue } inspections = append(inspections, inspection) } @@ -805,7 +832,13 @@ func notAuthorizedAction(repo *RepoPhase) Action { func desiredState(o Options, p plannedInstallation, prior *State) (*State, error) { records := make([]TargetRecord, 0, len(p.targets)) claimed := make(map[string]bool, len(p.targets)) + skippedOwner := map[string]bool{} for _, t := range p.targets { + if p.skipped[filepath.Clean(t.Path)] { + claimed[filepath.Clean(t.Path)] = true // and so never carried from prior either. + skippedOwner[p.owner[filepath.Clean(t.Path)]] = true + continue + } rec, err := RecordFor(t) if err != nil { return nil, err @@ -845,7 +878,12 @@ func desiredState(o Options, p plannedInstallation, prior *State) (*State, error harnesses := map[string]bool{} for _, name := range p.harnesses { - harnesses[name] = true + // A harness that owns a skipped target is listed only through a record + // below: when every target it planned was skipped, nothing of it is + // installed, and a listed harness with no target is invalid state. + if !skippedOwner[name] { + harnesses[name] = true + } } for _, rec := range records { if rec.Harness != "" { diff --git a/internal/install/target.go b/internal/install/target.go index c5a2385eb..c3da9f8db 100644 --- a/internal/install/target.go +++ b/internal/install/target.go @@ -42,6 +42,11 @@ const ( DispositionNoop Disposition = "no-op" // present, already desired DispositionUpdate Disposition = "update" // present, owned, differs DispositionConflict Disposition = "conflict" // present, not provably ours + // DispositionSkip is a target docket cannot edit in place — a hooks file + // that is a symlink or a directory. Unlike a conflict it refuses nothing: + // the path is left byte-untouched, no step or record is planned for it, + // every other target proceeds, and the outcome reports it with its remedy. + DispositionSkip Disposition = "skip" ) // The conflict reasons are the spec's stable machine reasons; the operation From 0839691c782087e10b3358b8c46e703b2ab4ef01 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 18:24:58 -0400 Subject: [PATCH 15/17] docs(install): name the session triggers an install writes and uninstall removes --- docs/install/claude-code.md | 6 ++++++ docs/install/codex.md | 7 ++++++- docs/install/cursor.md | 5 +++++ docs/install/install.md | 13 ++++++++++++- docs/install/opencode.md | 6 +++++- 5 files changed, 34 insertions(+), 3 deletions(-) diff --git a/docs/install/claude-code.md b/docs/install/claude-code.md index c774a53ec..9346ce546 100644 --- a/docs/install/claude-code.md +++ b/docs/install/claude-code.md @@ -10,6 +10,12 @@ 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, for example) 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..22ddac822 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, for example) 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..8361fe09c 100644 --- a/docs/install/install.md +++ b/docs/install/install.md @@ -46,6 +46,15 @@ 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, for example) 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 +84,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 From 219c8981778acdc3e1bfaf13d356cdee06446895 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 18:26:50 -0400 Subject: [PATCH 16/17] fix(install): skip a hooks file docket cannot parse instead of failing the install A settings.json or hooks.json docket cannot read as an editable hooks document (a JSONC comment, a byte-order mark, a null or non-object hooks value, a non-array event) was a managed-block-invalid conflict, and any conflict failed the install for every harness. Inspection now skips it like a non-regular hooks file: the file stays byte-identical, no step or record is planned for it, every other target installs, and install and check report it as a hook-file-not-editable warning whose remedy says docket left the file untouched and did not add its entries. Uninstall still refuses a recorded hooks file that became unparseable, since removal of the recorded entries cannot be proven, and never edits it. --- docs/install/claude-code.md | 3 +- docs/install/cursor.md | 4 +- docs/install/install.md | 3 +- internal/app/finding_codes.go | 6 +- internal/install/hook_entries_test.go | 79 ++++++++++++++++++++++++++- internal/install/inspect.go | 9 ++- internal/install/target.go | 2 +- 7 files changed, 92 insertions(+), 14 deletions(-) diff --git a/docs/install/claude-code.md b/docs/install/claude-code.md index 9346ce546..2c2276e12 100644 --- a/docs/install/claude-code.md +++ b/docs/install/claude-code.md @@ -14,7 +14,8 @@ calling out, after a word on what the install writes. `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, for example) is left untouched, and the run warns about it. Uninstall removes the hooks + 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 diff --git a/docs/install/cursor.md b/docs/install/cursor.md index 22ddac822..944639157 100644 --- a/docs/install/cursor.md +++ b/docs/install/cursor.md @@ -12,8 +12,8 @@ small, stable permission configuration, because docket must run **outside** Curs 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, for example) is - left untouched, and the run warns about it. Uninstall removes the hook only while 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 diff --git a/docs/install/install.md b/docs/install/install.md index 8361fe09c..3c1e0c76a 100644 --- a/docs/install/install.md +++ b/docs/install/install.md @@ -54,7 +54,8 @@ harness, after editing a config file, and after every version update). A single `~/.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, for example) is left untouched, and the run warns about it. + 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 diff --git a/internal/app/finding_codes.go b/internal/app/finding_codes.go index 63b19667d..9a97ab04e 100644 --- a/internal/app/finding_codes.go +++ b/internal/app/finding_codes.go @@ -158,9 +158,9 @@ const ( // 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, say): the file is left untouched and - // that harness's trigger is not installed. A warning only: the primary - // result is unaffected. + // 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" ) diff --git a/internal/install/hook_entries_test.go b/internal/install/hook_entries_test.go index 380a374a0..73f083328 100644 --- a/internal/install/hook_entries_test.go +++ b/internal/install/hook_entries_test.go @@ -292,8 +292,12 @@ func TestHookEntriesInspect(t *testing.T) { wrongEvent, nullEvent = `{"hooks": {"sessionStart": "x"}}`, `{"hooks": {"sessionStart": null}}` } var out []tc - for _, s := range []string{`{"hooks": []}`, wrongEvent, nullEvent, `[1]`, `null`, `not json`, `{"hooks": null}`, `{} {}`} { - out = append(out, tc{"invalid " + s, str(s), DispositionConflict, ReasonManagedBlockInvalid, remedyHookFileInvalid}) + // 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 } @@ -519,6 +523,12 @@ func TestHookEntriesInstallUninstallLifecycle(t *testing.T) { 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) @@ -589,8 +599,14 @@ func TestHookEntriesInstallUninstallLifecycle(t *testing.T) { // 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 == remedyHookFileNotRegular { + if s.Target.Path == path && s.Disposition == DispositionSkip && s.Remedy == remedy { return true } } @@ -733,6 +749,63 @@ func TestHookEntriesSymlinkedHooksFileIsSkipped(t *testing.T) { } } +// 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"} diff --git a/internal/install/inspect.go b/internal/install/inspect.go index 9f49d053b..b7b465ba1 100644 --- a/internal/install/inspect.go +++ b/internal/install/inspect.go @@ -237,7 +237,10 @@ func inspectHookEntries(t Target, info fs.FileInfo) (Inspection, error) { } entries, ok := readHookEntries(data, t.HookDialect) if !ok { - return conflict(t, ReasonManagedBlockInvalid, remedyHookFileInvalid), nil + // 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 } for _, c := range t.HookCommands { if !anyEntryRuns(entries, t.HookDialect, c) { @@ -268,8 +271,8 @@ const ( "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); " + - "repair it by hand, 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 diff --git a/internal/install/target.go b/internal/install/target.go index c3da9f8db..74414c8f7 100644 --- a/internal/install/target.go +++ b/internal/install/target.go @@ -43,7 +43,7 @@ const ( DispositionUpdate Disposition = "update" // present, owned, differs DispositionConflict Disposition = "conflict" // present, not provably ours // DispositionSkip is a target docket cannot edit in place — a hooks file - // that is a symlink or a directory. Unlike a conflict it refuses nothing: + // that is a symlink or a directory, or one docket cannot parse. Unlike a conflict it refuses nothing: // the path is left byte-untouched, no step or record is planned for it, // every other target proceeds, and the outcome reports it with its remedy. DispositionSkip Disposition = "skip" From 6d50d0a4592233ce36f9f251833bee325a910143 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Tue, 6 Oct 2026 18:34:31 -0400 Subject: [PATCH 17/17] fix(install): retire changed trigger commands and linked-worktree surfaces; type instructions read failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings fixed: - Stale hook commands: when a prior hook-entries record at the same path owns commands the plan no longer carries, inspection marks the target for update and the apply cuts those commands' exact entries in the same rewrite before adding the new ones. An edited entry is the user's and stays. - Linked-worktree surfaces: a private install run from a linked worktree now loads that worktree's own earlier record, retires its surfaces (proof-gated against that record, as for the primary), and rewrites the record through a new RepoPhase.ExtraRecords commit document. - docket instructions --json: a failure no longer sets Envelope.Failure; it carries a typed reason and message (instructions-read-failed, instructions-markers-invalid, instructions-unknown-section). Not fixed: an empty working-tree AGENTS.md after block retirement — the ownership record cannot prove docket created the file. --- internal/app/instructions.go | 46 +++++++--- internal/app/instructions_test.go | 19 ++++- .../private_instructions_integration_test.go | 85 +++++++++++++++++++ internal/app/repophase.go | 55 ++++++++++-- internal/app/repository_init.go | 9 ++ internal/install/hook_entries_test.go | 77 +++++++++++++++++ internal/install/inspect.go | 39 ++++++++- internal/install/repophase.go | 4 + internal/install/repophase_test.go | 30 +++++++ internal/install/service.go | 17 ++-- internal/install/target.go | 6 +- internal/install/txn.go | 8 ++ 12 files changed, 365 insertions(+), 30 deletions(-) diff --git a/internal/app/instructions.go b/internal/app/instructions.go index 37d7636f0..10b851950 100644 --- a/internal/app/instructions.go +++ b/internal/app/instructions.go @@ -209,33 +209,55 @@ func CursorProjectDir(getenv func(string) string, stdin io.Reader) string { return "" } +// Stable machine reasons for the instructions operation's failure results. +// Message is explanatory prose and must not be parsed. +const ( + ReasonInstructionsReadFailed = "instructions-read-failed" + ReasonInstructionsMarkersInvalid = "instructions-markers-invalid" + ReasonInstructionsUnknownSection = "instructions-unknown-section" +) + // InstructionsResult is the --json form of `docket instructions`. type InstructionsResult struct { Envelope Private bool `json:"private"` Section string `json:"section,omitempty"` Content string `json:"content"` + Reason string `json:"reason,omitempty"` + Message string `json:"message,omitempty"` } // HumanText is the selected content itself. func (r InstructionsResult) HumanText() string { return r.Content } -// Instructions is the --json form: applied with the selected content, or -// external-failed (carrying the error as its failure detail) on a read or -// section error. +// Instructions is the --json form: applied with the selected content, or a +// failure result carrying a stable reason and a message — invalid-input for an +// unknown section, external-failed when the file cannot be read, invalid-state +// when its markers are malformed. It is a read: Failure, which diagnoses a +// failed transaction, is never set. func Instructions(dir, section string) InstructionsResult { - content, private, err := ReadPrivateInstructions(dir) - if err == nil { - content, err = SelectInstructionsSection(content, section) - } - if err != nil { - out := InstructionsResult{ - Envelope: NewEnvelope(OperationInstructions, ResultExternalFailed), + fail := func(result Result, private bool, reason string, err error) InstructionsResult { + return InstructionsResult{ + Envelope: NewEnvelope(OperationInstructions, result), Private: private, Section: section, + Reason: reason, + Message: err.Error(), } - out.Failure = &FailureStatus{Kind: string(ResultExternalFailed), Detail: err.Error()} - return out + } + switch section { + case InstructionsSectionAll, InstructionsSectionDispatch, InstructionsSectionLessons: + default: + _, err := SelectInstructionsSection(nil, section) + return fail(ResultInvalidInput, false, ReasonInstructionsUnknownSection, err) + } + content, private, err := ReadPrivateInstructions(dir) + if err != nil { + return fail(ResultExternalFailed, private, ReasonInstructionsReadFailed, err) + } + content, err = SelectInstructionsSection(content, section) + if err != nil { + return fail(ResultInvalidState, private, ReasonInstructionsMarkersInvalid, err) } return InstructionsResult{ Envelope: NewEnvelope(OperationInstructions, ResultApplied), diff --git a/internal/app/instructions_test.go b/internal/app/instructions_test.go index c154e5bf4..411b6fec8 100644 --- a/internal/app/instructions_test.go +++ b/internal/app/instructions_test.go @@ -293,8 +293,23 @@ func TestInstructionsJSONResult(t *testing.T) { } bad := filepath.Join(testsupport.TempDir(t), "r") writeFixture(t, filepath.Join(bad, ".git", "dckt"), "file\n") - if res := Instructions(bad, InstructionsSectionAll); res.Result != ResultExternalFailed || res.Failure == nil { - t.Errorf("Instructions over a broken layout = %+v, want external-failed with a failure", res) + // A read op's failure is a typed reason and a message; Failure is reserved + // for a failed transaction and stays unset. + if res := Instructions(bad, InstructionsSectionAll); res.Result != ResultExternalFailed || + res.Reason != ReasonInstructionsReadFailed || res.Message == "" || res.Failure != nil { + t.Errorf("Instructions over a broken layout = %+v, want external-failed / %s with a message and no failure", + res, ReasonInstructionsReadFailed) + } + unbalanced := privateFixture(t, testsupport.TempDir(t), "lead\n\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) } } diff --git a/internal/app/private_instructions_integration_test.go b/internal/app/private_instructions_integration_test.go index 32f21bc65..d13aebb5b 100644 --- a/internal/app/private_instructions_integration_test.go +++ b/internal/app/private_instructions_integration_test.go @@ -257,6 +257,91 @@ func TestIntegrationPrivateInstructionsRetiresBuggyWorktreeSurfaces(t *testing.T } } +// TestIntegrationPrivateInstructionsRetiresLinkedWorktreeSurfaces proves an +// install run from a linked worktree also retires the working-tree surfaces an +// earlier install planned into THAT worktree and recorded under its own git +// dir (/worktrees//dckt/install.json), and empties that record so +// a second run is a no-op. +func TestIntegrationPrivateInstructionsRetiresLinkedWorktreeSurfaces(t *testing.T) { + r, primary, common := newPrivateInstructionsRepo(t) + setPrivateHarnesses(t, common, "[claude, codex, cursor]") + feature := filepath.Join(r.root, "feature") + runGit(t, r.invocation, "worktree", "add", "-q", "-b", "feat/x", feature) + feature, err := filepath.EvalSymlinks(feature) + if err != nil { + t.Fatal(err) + } + featureGitDir := filepath.Join(common, "worktrees", filepath.Base(feature)) + if _, err := os.Stat(featureGitDir); err != nil { + t.Fatalf("precondition: the linked worktree's git dir %s: %v", featureGitDir, err) + } + + rt, err := buildRunTracker() + if err != nil { + t.Fatal(err) + } + targets, owners, err := reposeed.Plan(reposeed.PlanInput{ + WorktreeRoot: feature, Harnesses: []string{"claude", "codex", "cursor"}, RunTracker: rt, + ClaudeMDState: reposeed.ClaudeMDAbsent, + }) + if err != nil { + t.Fatal(err) + } + desired, err := reposeed.DesiredRecord(targets, owners, feature) + if err != nil { + t.Fatal(err) + } + recordBytes, err := reposeed.EncodeRecord(desired) + if err != nil { + t.Fatal(err) + } + roots, err := install.ResolveRoots(os.UserHomeDir, os.Getenv) + if err != nil { + t.Fatal(err) + } + legacyRecord := reposeed.RecordPath(featureGitDir, layout.PrivateName) + if _, err := applyRepoPhaseSurfaces(&install.RepoPhase{ + Authorized: true, Targets: targets, Owners: owners, + RecordPath: legacyRecord, RecordBytes: recordBytes, Worktree: feature, + }, roots); err != nil { + t.Fatalf("seeding the old linked-worktree surfaces: %v", err) + } + for _, rel := range []string{"CLAUDE.md", filepath.Join(".cursor", "rules", "docket-dispatch.mdc")} { + if _, err := os.Lstat(filepath.Join(feature, rel)); err != nil { + t.Fatalf("precondition: %s not seeded: %v", rel, err) + } + } + + installSurfacesIn(t, feature) + + for _, rel := range []string{"CLAUDE.md", filepath.Join(".cursor", "rules", "docket-dispatch.mdc")} { + if _, err := os.Lstat(filepath.Join(feature, rel)); !os.IsNotExist(err) { + t.Errorf("%s survived in the linked worktree (err=%v); want it retired", rel, err) + } + } + if agents, err := os.ReadFile(filepath.Join(feature, "AGENTS.md")); err == nil && strings.Contains(string(agents), dispatchStartMarker) { + t.Errorf("the linked worktree's AGENTS.md still carries the dispatch block: %q", agents) + } + if private := string(mustReadFile(t, layout.PrivateInstructionsPath(common))); !strings.Contains(private, dispatchStartMarker) { + t.Errorf("%s = %q, want the dispatch block", layout.PrivateInstructionsDisplay, private) + } + assertNoWorktreeSurfaces(t, primary) + if rec, err := reposeed.LoadRecord(legacyRecord); err != nil || rec == nil || len(rec.Surfaces) != 0 { + t.Errorf("the linked worktree's record = %+v (%v), want an empty record", rec, err) + } + + phase, _, _, err := ResolveRepoPhase(context.Background(), newGitClient(t), feature, nil, rt, nil, config.ResolveContext{DefaultBranch: "main"}) + if err != nil { + t.Fatalf("second ResolveRepoPhase: %v", err) + } + if len(phase.Removals) != 0 { + t.Errorf("second run removals = %v, want none", phase.Removals) + } + if wrote, err := applyRepoPhaseSurfaces(phase, roots); err != nil || wrote { + t.Errorf("second run: wrote %v, err %v; want a no-op", wrote, err) + } +} + // TestIntegrationPrivateInstructionsOwnerRemovalKeepsSharedFile proves the // private file is shared by every opted-in harness: dropping codex rewrites the // block with the plain interior, dropping the last harness retires the block, diff --git a/internal/app/repophase.go b/internal/app/repophase.go index dca958a81..3dfd1d7ed 100644 --- a/internal/app/repophase.go +++ b/internal/app/repophase.go @@ -83,6 +83,7 @@ func ResolveRepoPhase(ctx context.Context, git *gitcli.Client, repoDir string, h return nil, "", nil, nil } root, gitDir := wt.Root, wt.GitDir + invokedRoot := root common, ok, err := layout.CommonDirOf(root) if err != nil || !ok { if err == nil { @@ -193,18 +194,56 @@ func ResolveRepoPhase(ctx context.Context, git *gitcli.Client, repoDir string, h return nil, "", nil, err } + // An earlier private install run from a linked worktree planned the shared + // surfaces into THAT worktree and recorded them under its own git dir. They + // are retired here exactly as the primary's are — proof-gated against that + // record — and the record is rewritten with whatever this run left in it. + var extraRecords []install.StateDoc + if private && !sameDir(gitDir, commonDir) { + legacyPath := reposeed.RecordPath(gitDir, layout.PrivateName) + legacyRec, err := reposeed.LoadRecord(legacyPath) + if err != nil { + return nil, "", nil, &RepoResolutionError{Reason: install.ReasonStateInvalid, Err: err} + } + if legacyRec != nil { + legacyState := legacyRec.ToState(invokedRoot) + legacyRemovals, err := computeRemovals(legacyRec, invokedRoot, optIns, inScope, planned, legacyState, legacy) + if err != nil { + return nil, "", nil, err + } + legacyBytes, err := composeRecordBytes(nil, nil, legacyRec, invokedRoot, optIns, planned, inScope) + if err != nil { + return nil, "", nil, &RepoResolutionError{Reason: install.ReasonInternal, Err: err} + } + removals = append(removals, legacyRemovals...) + extraRecords = append(extraRecords, install.StateDoc{Path: legacyPath, Bytes: legacyBytes}) + } + } + return &install.RepoPhase{ - Authorized: true, - Targets: targets, - Owners: owners, - PriorState: priorState, - Removals: removals, - RecordPath: recordPath, - RecordBytes: recordBytes, - Worktree: root, + Authorized: true, + Targets: targets, + Owners: owners, + PriorState: priorState, + Removals: removals, + RecordPath: recordPath, + RecordBytes: recordBytes, + ExtraRecords: extraRecords, + Worktree: root, }, root, warnings, nil } +// sameDir reports whether a and b name one directory, by identity when both +// stat and by cleaned spelling otherwise. +func sameDir(a, b string) bool { + ai, aerr := os.Stat(a) + bi, berr := os.Stat(b) + if aerr == nil && berr == nil { + return os.SameFile(ai, bi) + } + return filepath.Clean(a) == filepath.Clean(b) +} + // isRepositoryLayer mirrors config's own write-authority predicate: only the // repository and repository-local layers grant the installer authority to touch // repository surfaces. diff --git a/internal/app/repository_init.go b/internal/app/repository_init.go index b6b975360..54aff96bf 100644 --- a/internal/app/repository_init.go +++ b/internal/app/repository_init.go @@ -565,6 +565,14 @@ func applyRepoPhaseSurfaces(phase *install.RepoPhase, roots install.UserRoots) ( if err != nil { return false, err } + for _, d := range phase.ExtraRecords { + if !recordSettled { + break + } + if recordSettled, err = repoRecordAlreadyOnDisk(d.Path, d.Bytes); err != nil { + return false, err + } + } if steps == 0 && len(phase.Removals) == 0 && recordSettled { return false, nil } @@ -579,6 +587,7 @@ func applyRepoPhaseSurfaces(phase *install.RepoPhase, roots install.UserRoots) ( var docs []install.StateDoc if phase.RecordBytes != nil { docs = append(docs, install.StateDoc{Path: phase.RecordPath, Bytes: phase.RecordBytes}) + docs = append(docs, phase.ExtraRecords...) } if err := txn.CommitDocs(docs); err != nil { return false, err diff --git a/internal/install/hook_entries_test.go b/internal/install/hook_entries_test.go index 73f083328..3c0413400 100644 --- a/internal/install/hook_entries_test.go +++ b/internal/install/hook_entries_test.go @@ -596,6 +596,83 @@ func TestHookEntriesInstallUninstallLifecycle(t *testing.T) { } } +// 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 { diff --git a/internal/install/inspect.go b/internal/install/inspect.go index b7b465ba1..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" @@ -85,7 +86,7 @@ func InspectTarget(t Target, prior *State, legacy LegacyReproducer) (Inspection, case KindManagedBlock: return inspectManagedBlock(t, info, rec, hasRec, legacy) case KindHookEntries: - return inspectHookEntries(t, info) + 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) @@ -222,7 +223,13 @@ func inspectManagedBlock(t Target, info fs.FileInfo, rec TargetRecord, hasRec bo // 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". -func inspectHookEntries(t Target, info fs.FileInfo) (Inspection, error) { +// +// 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 @@ -242,6 +249,10 @@ func inspectHookEntries(t Target, info fs.FileInfo) (Inspection, error) { // 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 @@ -252,6 +263,30 @@ func inspectHookEntries(t Target, info fs.FileInfo) (Inspection, error) { 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} } 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/service.go b/internal/install/service.go index 7e3458ca6..2142e7488 100644 --- a/internal/install/service.go +++ b/internal/install/service.go @@ -716,6 +716,7 @@ func applyPlan(o Options, p plannedInstallation, repo *RepoPhase, out Outcome) O docs := []StateDoc{{Path: o.Roots.StatePath(), Bytes: desiredBytes}} if repoActive && repo.RecordBytes != nil { docs = append(docs, StateDoc{Path: repo.RecordPath, Bytes: repo.RecordBytes}) + docs = append(docs, repo.ExtraRecords...) } steps := nonNoopCount(inspections) + nonNoopCount(repoInspections) @@ -800,14 +801,20 @@ func repoRecordSettled(active bool, repo *RepoPhase) (bool, error) { if !active || repo.RecordBytes == nil { return true, nil } - existing, err := os.ReadFile(repo.RecordPath) - if err != nil { - if errors.Is(err, fs.ErrNotExist) { + docs := append([]StateDoc{{Path: repo.RecordPath, Bytes: repo.RecordBytes}}, repo.ExtraRecords...) + for _, d := range docs { + existing, err := os.ReadFile(d.Path) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return false, nil + } + return false, fmt.Errorf("install: reading %s: %w", d.Path, err) + } + if string(existing) != string(d.Bytes) { return false, nil } - return false, fmt.Errorf("install: reading %s: %w", repo.RecordPath, err) } - return string(existing) == string(repo.RecordBytes), nil + return true, nil } // notAuthorizedAction names the repository no-op so a run that touched no diff --git a/internal/install/target.go b/internal/install/target.go index 74414c8f7..5125629de 100644 --- a/internal/install/target.go +++ b/internal/install/target.go @@ -24,7 +24,11 @@ type Target struct { // install order, whose entries the target owns in that one file. HookDialect string HookCommands []string - Role string + // retireHookCommands is set by inspection, never by a planner: commands a + // prior record owned that this target no longer carries, whose exact + // entries the apply cuts before adding HookCommands. + retireHookCommands []string + Role string // Mode overrides the permissions an applied KindFile target is published // with. Zero means "the installer's policy": the mode an updated file // already had, else 0o644. It exists for the one target whose usefulness diff --git a/internal/install/txn.go b/internal/install/txn.go index 4d9f217b9..595152fcb 100644 --- a/internal/install/txn.go +++ b/internal/install/txn.go @@ -477,6 +477,14 @@ func (t *Txn) applyStep(i int) error { if err != nil && !errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("reading %s: %w", step.Path, err) } + if len(target.retireHookCommands) > 0 { + // A changed trigger command: the recorded command's exact entry goes + // in the same rewrite, so the old and the new never both fire. + existing, err = removeHookEntries(existing, target.HookDialect, target.retireHookCommands) + if err != nil { + return fmt.Errorf("retiring stale hook entries from %s: %w", step.Path, err) + } + } out, err := insertHookEntries(existing, target.HookDialect, target.HookCommands) if err != nil { return fmt.Errorf("adding hook entries to %s: %w", step.Path, err)