From b492d731a29cd36ea134aac9f5b9018df0a941ba Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Sat, 26 Sep 2026 08:19:13 -0400 Subject: [PATCH 1/5] docs(plan): implementation plan for change 0460 Docket-Plan-Path: docs/superpowers/plans/2026-09-26-artifact-backlink-refuses-an-absolute-change-path-with-unkno.md --- ...uses-an-absolute-change-path-with-unkno.md | 347 ++++++++++++++++++ 1 file changed, 347 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-26-artifact-backlink-refuses-an-absolute-change-path-with-unkno.md diff --git a/docs/superpowers/plans/2026-09-26-artifact-backlink-refuses-an-absolute-change-path-with-unkno.md b/docs/superpowers/plans/2026-09-26-artifact-backlink-refuses-an-absolute-change-path-with-unkno.md new file mode 100644 index 000000000..0c43d0aba --- /dev/null +++ b/docs/superpowers/plans/2026-09-26-artifact-backlink-refuses-an-absolute-change-path-with-unkno.md @@ -0,0 +1,347 @@ + +> ↩ **[Change 0460 — artifact.backlink refuses an absolute --change path with unknown-change](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0460-artifact-backlink-refuses-an-absolute-change-path-with-unkno.md)** + +# artifact.backlink `--change` Path Validation Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make `artifact.backlink` refuse a malformed `--change` path with a typed, form-naming message instead of the misleading `unknown-change`, and fix the caller prose that produced the absolute path. + +**Architecture:** Add a purely lexical validation of `req.ChangePath` inside `ArtifactBacklink` (`internal/app/artifact_backlink.go`), mirroring the four checks `verifyAttachPath` (`internal/app/change_attach.go`) already enforces — copied inline as a small private helper, so the attach operations' observable reasons and messages are untouched. The check runs after the artifact read/parse and before the corpus read, so every refusal still predates any write and a malformed artifact is still reported first. Reuse the existing `ReasonBacklinkAbsolutePath` / `ReasonBacklinkPathEscape` constants — no new reason codes. Separately, state the repo-relative path form in the two `skills/docket-implement-next/SKILL.md` sentences that pass `artifact.backlink` flags with the form unstated, and regenerate the embedded asset tree that byte-copies that skill. + +**Tech Stack:** Go (stdlib `path`, `path/filepath`), Go tests in `internal/app`; `go generate ./internal/assets/` for the embedded asset bundle. + +**Spec:** `docs/superpowers/specs/2026-09-25-artifact-backlink-refuses-an-absolute-change-path-with-unkno-design.md` (on the `docket` metadata branch; synchronized copy at `.docket/docs/superpowers/specs/…` from the primary checkout) + +## Global Constraints + +- Accepting absolute paths on any flag is out of scope; the fix is a clear refusal, never a second accepted path form. +- Reuse `ReasonBacklinkAbsolutePath` (`"absolute-path"`) and `ReasonBacklinkPathEscape` (`"path-escape"`); mint no new reason codes. +- Every refusal must happen before any write: a refused call leaves the artifact byte-identical. +- The attach operations' (`change.attach-plan` / `change.attach-results`) observable reasons and messages must not change — `verifyAttachPath` is a reference, not an edit target. +- Each new message names the flag (`--change`) and the expected form (canonical repository-relative), e.g. `docs/changes/active/-.md`. +- A well-formed path that matches no record still returns `unknown-change`, with its existing message. +- Path handling in other operations, and the gate-drive `scope-closed` issue from the 0458 run, are out of scope. +- Point-in-time records (archived changes, results files, specs, old plans) keep their existing wording — only maintained caller prose is edited (repo rule: rewriting point-in-time records falsifies history). + +## Review Focus + +Checked the spec's input space against the tasks below; each line's test is pinned into Task 1 (test table) as noted: + +1. An **absolute path that names the real change file** (the exact 0458 shape, `/Users/…/docs/changes/active/-.md` where the repo-relative tail would match a record) must refuse `absolute-path`, never resolve — Task 1 table case `absolute`. +2. A **whitespace-only** `--change` value (`" "`) must refuse `path-escape` like empty, not panic or reach the corpus read — Task 1 table case `whitespace-only`. +3. An **interior `..` that does not escape** (`docs/changes/active/../active/0315-claim.md`) is lexically local (`filepath.IsLocal` accepts it) but non-canonical — it must be caught by the `path.Clean` check as `path-escape`, not fall through to `unknown-change` — Task 1 table case `interior-dotdot`. +4. A **trailing slash** (`docs/changes/active/0315-claim.md/`) is a non-canonical spelling of a real record's path and must refuse `path-escape`, not resolve and not report `unknown-change` — Task 1 table case `trailing-slash`. +5. **Refusal-before-write on the new branch**: a `--change` refusal must leave an artifact that already carries a stale backlink block byte-identical (the validation sits after the parse; a reorder during review could move it after the write) — Task 1's test seeds the artifact and asserts byte-identity on every table case. + +--- + +### Task 1: Lexical `--change` validation in `ArtifactBacklink` + +**Files:** +- Modify: `internal/app/artifact_backlink.go` (new helper + one call site between the document parse and `resolveBacklinkChange`) +- Test: `internal/app/artifact_backlink_test.go` (new `TestArtifactBacklinkChangePathValidation`) + +**Interfaces:** +- Consumes: existing test fixtures in `internal/app/artifact_backlink_test.go` — `docketPin(t)`, `backlinkCorpus()`, `backlinkDeps(&fakeReader{pin: pin, corpus: corpus})`, `testsupport.TempDir(t)`, and the constants `ReasonBacklinkAbsolutePath`, `ReasonBacklinkPathEscape`, `ResultInvalidInput`. +- Produces: `validateBacklinkChangePath(changePath string) (reason, message string)` — unexported, `("", "")` on a well-formed path; used only inside this file. No later task consumes it. + +- [ ] **Step 1: Write the failing test** + +Append to `internal/app/artifact_backlink_test.go`: + +```go +// TestArtifactBacklinkChangePathValidation: --change is validated as a +// canonical repository-relative path before the corpus read — the same rule +// --artifact and the attach operations enforce. A malformed spelling is a +// typed refusal naming the flag and the expected form, never unknown-change, +// and the artifact is left byte-identical. +func TestArtifactBacklinkChangePathValidation(t *testing.T) { + pin := docketPin(t) + corpus := backlinkCorpus() + + cases := []struct { + name string + changePath string + reason string + }{ + // The 0458 shape: an absolute spelling of a path whose repo-relative + // tail names a real record must refuse, never resolve. + {"absolute", "/work/repo/" + backlinkChangePath, ReasonBacklinkAbsolutePath}, + {"dotdot-escape", "../" + backlinkChangePath, ReasonBacklinkPathEscape}, + {"non-canonical-dot", "./" + backlinkChangePath, ReasonBacklinkPathEscape}, + {"interior-dotdot", "docs/changes/active/../active/0315-claim.md", ReasonBacklinkPathEscape}, + {"trailing-slash", backlinkChangePath + "/", ReasonBacklinkPathEscape}, + {"empty", "", ReasonBacklinkPathEscape}, + {"whitespace-only", " ", ReasonBacklinkPathEscape}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := testsupport.TempDir(t) + artifact := filepath.Join(root, "plan.md") + original := []byte("# Plan\n\nAuthored body.\n") + if err := os.WriteFile(artifact, original, 0o644); err != nil { + t.Fatalf("seed artifact: %v", err) + } + + got := ArtifactBacklink(context.Background(), backlinkDeps(&fakeReader{pin: pin, corpus: corpus}), root, + ArtifactBacklinkRequest{ArtifactPath: "plan.md", ChangePath: tc.changePath}) + + if got.Result != ResultInvalidInput { + t.Fatalf("result=%q, want %q (reason=%q message=%q)", got.Result, ResultInvalidInput, got.Reason, got.Message) + } + if got.Reason != tc.reason { + t.Fatalf("reason=%q, want %q (message=%q)", got.Reason, tc.reason, got.Message) + } + // The message must name the flag and the expected form — the 0458 + // failure was precisely a message that named neither. + if !strings.Contains(got.Message, "--change") { + t.Fatalf("message does not name the --change flag: %q", got.Message) + } + if !strings.Contains(got.Message, "repository-relative") { + t.Fatalf("message does not name the expected form: %q", got.Message) + } + // Refusal predates any write. + out, err := os.ReadFile(artifact) + if err != nil { + t.Fatalf("read back: %v", err) + } + if string(out) != string(original) { + t.Fatalf("file mutated on refusal:\n got %q\nwant %q", out, original) + } + }) + } +} +``` + +No new imports are needed: `context`, `os`, `filepath`, `strings`, `testing`, and `testsupport` are already imported by this file. + +- [ ] **Step 2: Run the new test to verify it fails** + +Run: `go test ./internal/app/ -run TestArtifactBacklinkChangePathValidation -count=1 -v` +Expected: FAIL. Every case currently falls through to `resolveBacklinkChange` and reports `reason="unknown-change"` (want `absolute-path` / `path-escape`). + +Also confirm the pre-change baseline is green so Step 4's diff is attributable: +Run: `go test ./internal/app/ -run 'TestArtifactBacklink' -count=1` +Expected: FAIL only in `TestArtifactBacklinkChangePathValidation`; every other `TestArtifactBacklink*` test passes. + +- [ ] **Step 3: Implement the validation** + +In `internal/app/artifact_backlink.go`: + +(a) Add `"path"` to the imports (alongside the existing `"path/filepath"`). + +(b) Add the helper next to `containedArtifactPath` (bottom of the file): + +```go +// validateBacklinkChangePath proves the --change value is a canonical +// repository-relative path — the one rule every path flag crossing the CLI +// follows (--artifact above, verifyAttachPath in change_attach.go). The check +// is purely lexical: the change record lives in the pinned git corpus, not on +// the feature worktree's filesystem, so there is no containment root to +// resolve against and no symlink to canonicalise. It mirrors verifyAttachPath +// deliberately (learning duplicated-gate-copies-the-whole-predicate: all four +// checks, not just the absolute-path threshold) without factoring it out, so +// the attach operations' observable reasons and messages stay untouched. It +// returns a stable refusal reason and a message naming the flag and the +// expected form, or ("", "") for a well-formed path. +func validateBacklinkChangePath(changePath string) (string, string) { + const form = "pass the canonical repository-relative change path (e.g. docs/changes/active/-.md)" + if strings.TrimSpace(changePath) == "" { + return ReasonBacklinkPathEscape, + fmt.Sprintf("--change path is empty; %s", form) + } + if filepath.IsAbs(changePath) { + return ReasonBacklinkAbsolutePath, + fmt.Sprintf("--change path %q is absolute; %s", changePath, form) + } + if !filepath.IsLocal(filepath.FromSlash(changePath)) { + return ReasonBacklinkPathEscape, + fmt.Sprintf("--change path %q escapes the repository root; %s", changePath, form) + } + // Clean is a no-op for a canonical path; an input that changes under Clean + // is non-canonical (a `./`, `//`, interior `..`, or trailing-slash + // spelling) and is refused as an escape, matching verifyAttachPath. + if clean := path.Clean(changePath); clean != changePath { + return ReasonBacklinkPathEscape, + fmt.Sprintf("--change path %q is not in canonical repository-relative form; %s", changePath, form) + } + return "", "" +} +``` + +(c) Call it in `ArtifactBacklink`, between the numbered step-3 document parse and the step-4 corpus read — i.e. immediately after the `document.Parse` refusal block and before the `resolveBacklinkChange(...)` line — keeping the spec's ordering (artifact containment and read, parse, `--change` validation, corpus read; every refusal before any write): + +```go + // 4. Validate --change lexically before the corpus read: a malformed + // spelling is refused by form, so unknown-change is reached only by a + // well-formed path that names no record. + if reason, msg := validateBacklinkChangePath(req.ChangePath); reason != "" { + return backlinkRefusal(ResultInvalidInput, reason, msg) + } +``` + +Renumber the existing step comments 4–7 in `ArtifactBacklink` to 5–8 so the numbered narration stays consecutive. + +(d) Update the two reason-constant doc comments so they cover both flags — change + +```go + // ReasonBacklinkAbsolutePath: the artifact path is absolute; paths crossing + // the CLI are canonical repository-relative (Global Constraints). +``` + +to + +```go + // ReasonBacklinkAbsolutePath: the artifact or change path is absolute; + // paths crossing the CLI are canonical repository-relative (Global + // Constraints). +``` + +and + +```go + // ReasonBacklinkPathEscape: the artifact path escapes the worktree with a + // `..` traversal. +``` + +to + +```go + // ReasonBacklinkPathEscape: the artifact path escapes the worktree with a + // `..` traversal, or the change path is empty, escaping, or a + // non-canonical spelling. +``` + +Also update `ReasonBacklinkUnknownChange`'s comment from "the --change path names no record in the corpus" to "the well-formed --change path names no record in the corpus" — the constant's contract narrowed. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `go test ./internal/app/ -run 'TestArtifactBacklink' -count=1 -v` +Expected: PASS — all of `TestArtifactBacklinkChangePathValidation`, plus the pre-existing tests that pin the neighbors this change must not move: +- `TestArtifactBacklinkPathContainment/absolute` — the spec's required regression that absolute `--artifact` still refuses `absolute-path` (already exists; it must stay green). +- `TestArtifactBacklinkUnknownChange` — a well-formed absent path still refuses `unknown-change` with its existing message. +- `TestArtifactBacklinkRendersBlock` / `TestArtifactBacklinkIdempotent` — the repo-relative happy path still renders. + +Then `gofmt -l internal/app/` — expected: no output. + +- [ ] **Step 5: Commit** + +```bash +git add internal/app/artifact_backlink.go internal/app/artifact_backlink_test.go +git commit -m "fix(app): artifact.backlink refuses a malformed --change path by form (change 0460)" +``` + +- [ ] **Step 6: Mutation-test the guard (after the commit, so restore is safe)** + +The restore below is `git checkout -- `, which restores to HEAD — that is only safe because Step 5 committed the finished implementation first (learning mutation-restore-needs-a-backup-copy). + +1. In `validateBacklinkChangePath`, delete (or comment out) the entire `filepath.IsAbs` branch. +2. Run: `go test ./internal/app/ -run TestArtifactBacklinkChangePathValidation -count=1` (`-count=1` defeats the result cache — a cached PASS against the mutated tree is the known trap). + Expected: FAIL — the `absolute` case now reports `path-escape` (the absolute path fails `filepath.IsLocal`), not `absolute-path`. If it stays green, the guard is decoration: stop and fix the test. +3. Restore: `git checkout -- internal/app/artifact_backlink.go` +4. Re-run: `go test ./internal/app/ -run TestArtifactBacklinkChangePathValidation -count=1` — expected: PASS. + +--- + +### Task 2: State the repo-relative form in the caller prose, and regenerate the embedded assets + +The absolute path in the 0458 run came from `skills/docket-implement-next/SKILL.md` leaving the path form unstated. The skill tree is byte-copied into the embedded asset bundle, and `TestEmbeddedMatchesAuthored` (`internal/assets/embedded_test.go`) is a two-directional drift guard — so the prose edit and the `go generate` regeneration must land in the same commit. + +**Files:** +- Modify: `skills/docket-implement-next/SKILL.md` (two sentences: the Step 4 dispatch summary and per-checkpoint mechanics item 3) +- Regenerate: `internal/assets/embedded/tree/**` and `internal/assets/manifest.json` via `go generate ./internal/assets/` (never hand-edited) +- Test: existing `internal/assets` drift guard (no new test) + +**Interfaces:** +- Consumes: nothing from Task 1 (independent; either order works, but keep plan order). +- Produces: nothing consumed later. + +- [ ] **Step 1: Derive the full edit list from a whole-repo grep (never a hand-made list)** + +Run from the worktree root: + +```bash +hits=$(grep -rn -e "artifact.backlink" -e "artifact backlink" --include='*.md' . | grep -v -e '^\./docs/changes/archive/' -e '^\./docs/results/' -e '^\./docs/superpowers/' -e '^\./docs/adrs/' -e '^\./internal/assets/embedded/' -e '^\./internal/harness/' -e '^\./internal/render/testdata/' -e '^\./\.docket/' -e '^\./\.worktrees/') +printf '%s\n' "$hits" +``` + +(Capture into a variable first — never pipe the producer into an early-exiting consumer under pipefail.) + +Sort the hits: point-in-time records and generated/embedded copies are excluded above by construction; what remains is maintained prose. Expected maintained hits: `skills/docket-implement-next/SKILL.md` (lines quoted in Step 2, plus flagless mentions), `skills/docket-implement-next/results-template.md` (marker-ownership note, passes no flags — leave), `skills/docket-convention/SKILL.md` (operation-roster and block-ownership mentions, pass no flags — leave), `agents/docket-plan-writer.md` (already says "repo-relative" — leave), `docs/comparison/ai-native-sdlc-playbook.md` (historical script name, passes no flags — leave). Only sites that **pass `artifact.backlink` flags with the path form unstated** are edited. If the grep surfaces a flag-passing site not listed here, fix it the same way as Step 2. + +- [ ] **Step 2: Make the two edits in `skills/docket-implement-next/SKILL.md`** + +Edit 1 — per-checkpoint mechanics item 3 (the sentence the spec names; currently line 123). Replace: + +``` +3. Write the update, then stamp and validate the back-link home with the `artifact.backlink` operation (`--artifact --change `). +``` + +with: + +``` +3. Write the update, then stamp and validate the back-link home with the `artifact.backlink` operation (`--artifact --change `). +``` + +Edit 2 — the Step 4 dispatch summary (currently line 84) has the same unstated form. In the sentence beginning `The child invokes the resolved plan skill`, replace the fragment: + +``` +stamps the backlink with the `artifact.backlink` operation (`--artifact --change `) +``` + +with: + +``` +stamps the backlink with the `artifact.backlink` operation (`--artifact --change `) +``` + +This matches the wording already used in `agents/docket-plan-writer.md` ("`--artifact --change `"). + +- [ ] **Step 3: Regenerate the embedded asset bundle** + +Run: `go generate ./internal/assets/` +Expected: `internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md` and `internal/assets/manifest.json` now reflect the edit (`git status` shows them modified; nothing else). + +- [ ] **Step 4: Run the drift guard and prose-adjacent suites** + +Run: `go test ./internal/assets/ -count=1` +Expected: PASS (`TestEmbeddedMatchesAuthored` green in both directions). + +Run: `go test ./internal/harness/... -count=1` +Expected: PASS — the harness golden wrappers are thin (they do not embed the skill body), so nothing reddens; this run proves that assumption. + +- [ ] **Step 5: Commit** + +```bash +git add skills/docket-implement-next/SKILL.md internal/assets/embedded/tree internal/assets/manifest.json +git commit -m "docs(skills): name the repo-relative form for artifact.backlink flags (change 0460)" +``` + +--- + +### Task 3: Whole-package verification + +**Files:** none (verification only). + +**Interfaces:** +- Consumes: Tasks 1–2 committed. +- Produces: a clean tree for the build gate. + +- [ ] **Step 1: Run the touched packages together, cache-defeated** + +Run: `go test ./internal/app/ ./internal/assets/ ./internal/harness/... -count=1` +Expected: PASS. + +- [ ] **Step 2: Confirm the tree is clean** + +Run: `git status --porcelain` +Expected: empty. (The full repository suite is the build gate's job — the gate runs whatever `build.test_command` resolves to, from source; do not substitute a partial package run for it.) + +--- + +## Self-review notes + +- Spec coverage: §1 validation table → Task 1 (all five rows: empty/whitespace, absolute, `..` escape, non-canonical, well-formed-absent unchanged); §1 message rule → Task 1 Step 1 message asserts + Step 3 wording; §1 ordering rule → Task 1 Step 3(c); §1 helper choice (inline copy, attach ops untouched) → Task 1 Step 3(b); §2 prose fix + grep-derived caller list → Task 2; Testing section rows map to Task 1's table, the existing `TestArtifactBacklinkUnknownChange` / happy-path / `--artifact`-absolute tests (pinned green in Task 1 Step 4), and the mutation check → Task 1 Step 6. +- The `--artifact` absolute regression the spec asks for already exists (`TestArtifactBacklinkPathContainment/absolute`); Task 1 Step 4 names it as a must-stay-green rather than duplicating it. +- Learnings consulted: duplicated-gate-copies-the-whole-predicate (copy all four checks, cited in the helper comment), fix-reintroduces-its-own-defect-class (the new helper is itself a path validator — its own table is the audit), mutation-restore-needs-a-backup-copy (Task 1 Step 6 commits first), cached-runner-serves-a-mutated-tree (`-count=1` throughout), consolidation-flattens-caller-variance (Task 2 edits name each site's own artifact kind rather than templating one sentence). From 6b3b96d61356be124fdc577732c15c5d0062674d Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Sat, 26 Sep 2026 08:21:22 -0400 Subject: [PATCH 2/5] fix(app): artifact.backlink refuses a malformed --change path by form (change 0460) --- internal/app/artifact_backlink.go | 63 ++++++++++++++++++++++---- internal/app/artifact_backlink_test.go | 62 +++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 9 deletions(-) diff --git a/internal/app/artifact_backlink.go b/internal/app/artifact_backlink.go index 0adb925bd..1bf9f135b 100644 --- a/internal/app/artifact_backlink.go +++ b/internal/app/artifact_backlink.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "path" "path/filepath" "strings" @@ -42,11 +43,13 @@ const ( // The stable machine reasons `artifact backlink` reports for its typed refusals. // Message text is explanatory and must not be parsed. const ( - // ReasonBacklinkAbsolutePath: the artifact path is absolute; paths crossing - // the CLI are canonical repository-relative (Global Constraints). + // ReasonBacklinkAbsolutePath: the artifact or change path is absolute; + // paths crossing the CLI are canonical repository-relative (Global + // Constraints). ReasonBacklinkAbsolutePath = "absolute-path" // ReasonBacklinkPathEscape: the artifact path escapes the worktree with a - // `..` traversal. + // `..` traversal, or the change path is empty, escaping, or a + // non-canonical spelling. ReasonBacklinkPathEscape = "path-escape" // ReasonBacklinkSymlinkEscape: a symlink hop on the artifact path resolves to // a physical location outside the worktree. @@ -58,8 +61,8 @@ const ( // malformed (dangling/out-of-order/nested markers); the block is not rewritten // and the file is left untouched. ReasonBacklinkMalformedMarkers = "malformed-markers" - // ReasonBacklinkUnknownChange: the --change path names no record in the - // corpus, so no backlink can be rendered. + // ReasonBacklinkUnknownChange: the well-formed --change path names no + // record in the corpus, so no backlink can be rendered. ReasonBacklinkUnknownChange = "unknown-change" // ReasonBacklinkRepoUnreadable: the worktree root cannot be canonicalised. ReasonBacklinkRepoUnreadable = "repo-unreadable" @@ -168,13 +171,20 @@ func ArtifactBacklink(ctx context.Context, deps PlanningDeps, repoDir string, re fmt.Sprintf("artifact %q has a malformed managed-block population: %v", req.ArtifactPath, err)) } - // 4. Resolve the target change from one pinned corpus read. + // 4. Validate --change lexically before the corpus read: a malformed + // spelling is refused by form, so unknown-change is reached only by a + // well-formed path that names no record. + if reason, msg := validateBacklinkChangePath(req.ChangePath); reason != "" { + return backlinkRefusal(ResultInvalidInput, reason, msg) + } + + // 5. Resolve the target change from one pinned corpus read. change, refusal := resolveBacklinkChange(ctx, deps, repoDir, req.ChangePath) if refusal != nil { return *refusal } - // 5. Render the deterministic backlink block and reduce it to the interior the + // 6. Render the deterministic backlink block and reduce it to the interior the // document layer manages between the markers it owns. block, err := render.BacklinkContent(change.change, change.link) if err != nil { @@ -182,7 +192,7 @@ func ArtifactBacklink(ctx context.Context, deps PlanningDeps, repoDir string, re } interior := backlinkInterior(block) - // 6. Rewrite (or insert) the managed block. + // 7. Rewrite (or insert) the managed block. var ps document.PatchSet if _, ok := doc.Block(backlinkBlockName); ok { ps.ReplaceBlock(backlinkBlockName, interior) @@ -201,7 +211,7 @@ func ArtifactBacklink(ctx context.Context, deps PlanningDeps, repoDir string, re applied := ArtifactBacklinkResult{Artifact: req.ArtifactPath, Change: change.change.Path()} - // 7. Idempotent write: unchanged bytes are a no-op, so a re-run yields a + // 8. Idempotent write: unchanged bytes are a no-op, so a re-run yields a // byte-identical file and no needless mtime churn. if string(updated) == string(original) { applied.Disposition = backlinkDispositionUnchanged @@ -332,3 +342,38 @@ func resolveEveryHop(p string) (string, error) { } return filepath.Join(parentReal, filepath.Base(p)), nil } + +// validateBacklinkChangePath proves the --change value is a canonical +// repository-relative path — the one rule every path flag crossing the CLI +// follows (--artifact above, verifyAttachPath in change_attach.go). The check +// is purely lexical: the change record lives in the pinned git corpus, not on +// the feature worktree's filesystem, so there is no containment root to +// resolve against and no symlink to canonicalise. It mirrors verifyAttachPath +// deliberately (learning duplicated-gate-copies-the-whole-predicate: all four +// checks, not just the absolute-path threshold) without factoring it out, so +// the attach operations' observable reasons and messages stay untouched. It +// returns a stable refusal reason and a message naming the flag and the +// expected form, or ("", "") for a well-formed path. +func validateBacklinkChangePath(changePath string) (string, string) { + const form = "pass the canonical repository-relative change path (e.g. docs/changes/active/-.md)" + if strings.TrimSpace(changePath) == "" { + return ReasonBacklinkPathEscape, + fmt.Sprintf("--change path is empty; %s", form) + } + if filepath.IsAbs(changePath) { + return ReasonBacklinkAbsolutePath, + fmt.Sprintf("--change path %q is absolute; %s", changePath, form) + } + if !filepath.IsLocal(filepath.FromSlash(changePath)) { + return ReasonBacklinkPathEscape, + fmt.Sprintf("--change path %q escapes the repository root; %s", changePath, form) + } + // Clean is a no-op for a canonical path; an input that changes under Clean + // is non-canonical (a `./`, `//`, interior `..`, or trailing-slash + // spelling) and is refused as an escape, matching verifyAttachPath. + if clean := path.Clean(changePath); clean != changePath { + return ReasonBacklinkPathEscape, + fmt.Sprintf("--change path %q is not in canonical repository-relative form; %s", changePath, form) + } + return "", "" +} diff --git a/internal/app/artifact_backlink_test.go b/internal/app/artifact_backlink_test.go index 3862b186e..1cf1d08cd 100644 --- a/internal/app/artifact_backlink_test.go +++ b/internal/app/artifact_backlink_test.go @@ -235,3 +235,65 @@ func TestArtifactBacklinkUnknownChange(t *testing.T) { t.Fatalf("file mutated for an unknown change: %q", out) } } + +// TestArtifactBacklinkChangePathValidation: --change is validated as a +// canonical repository-relative path before the corpus read — the same rule +// --artifact and the attach operations enforce. A malformed spelling is a +// typed refusal naming the flag and the expected form, never unknown-change, +// and the artifact is left byte-identical. +func TestArtifactBacklinkChangePathValidation(t *testing.T) { + pin := docketPin(t) + corpus := backlinkCorpus() + + cases := []struct { + name string + changePath string + reason string + }{ + // The 0458 shape: an absolute spelling of a path whose repo-relative + // tail names a real record must refuse, never resolve. + {"absolute", "/work/repo/" + backlinkChangePath, ReasonBacklinkAbsolutePath}, + {"dotdot-escape", "../" + backlinkChangePath, ReasonBacklinkPathEscape}, + {"non-canonical-dot", "./" + backlinkChangePath, ReasonBacklinkPathEscape}, + {"interior-dotdot", "docs/changes/active/../active/0315-claim.md", ReasonBacklinkPathEscape}, + {"trailing-slash", backlinkChangePath + "/", ReasonBacklinkPathEscape}, + {"empty", "", ReasonBacklinkPathEscape}, + {"whitespace-only", " ", ReasonBacklinkPathEscape}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := testsupport.TempDir(t) + artifact := filepath.Join(root, "plan.md") + original := []byte("# Plan\n\nAuthored body.\n") + if err := os.WriteFile(artifact, original, 0o644); err != nil { + t.Fatalf("seed artifact: %v", err) + } + + got := ArtifactBacklink(context.Background(), backlinkDeps(&fakeReader{pin: pin, corpus: corpus}), root, + ArtifactBacklinkRequest{ArtifactPath: "plan.md", ChangePath: tc.changePath}) + + if got.Result != ResultInvalidInput { + t.Fatalf("result=%q, want %q (reason=%q message=%q)", got.Result, ResultInvalidInput, got.Reason, got.Message) + } + if got.Reason != tc.reason { + t.Fatalf("reason=%q, want %q (message=%q)", got.Reason, tc.reason, got.Message) + } + // The message must name the flag and the expected form — the 0458 + // failure was precisely a message that named neither. + if !strings.Contains(got.Message, "--change") { + t.Fatalf("message does not name the --change flag: %q", got.Message) + } + if !strings.Contains(got.Message, "repository-relative") { + t.Fatalf("message does not name the expected form: %q", got.Message) + } + // Refusal predates any write. + out, err := os.ReadFile(artifact) + if err != nil { + t.Fatalf("read back: %v", err) + } + if string(out) != string(original) { + t.Fatalf("file mutated on refusal:\n got %q\nwant %q", out, original) + } + }) + } +} From 379e40960a3e0789db095f62fd89b4c6a0fc039d Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Sat, 26 Sep 2026 08:23:12 -0400 Subject: [PATCH 3/5] docs(skills): name the repo-relative form for artifact.backlink flags (change 0460) --- internal/assets/embedded/manifest.json | 6 +++--- .../embedded/tree/skills/docket-implement-next/SKILL.md | 4 ++-- skills/docket-implement-next/SKILL.md | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index fc4628ba9..9a093b36e 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:731403057cb43dbe23441329450003b511900f190a85c685fc103544bbfb7cda", + "asset_set_id": "sha256:42cade1bb3666833e5b1d8111f77e5285e46f404f98093f324e0adf0f4988211", "entries": [ { "path": ".docket.example.yml", @@ -406,8 +406,8 @@ "path": "skills/docket-implement-next/SKILL.md", "role": "skill", "mode": 420, - "size": 55911, - "sha256": "ea1ffb367fab8bae4adf5ae115834f5e037468f559be2a06bd744a5c27c018bd" + "size": 55967, + "sha256": "133e27896fe1b63070f8c8f5b820fadede3c479d1e5a25b3257fe56435027bda" }, { "path": "skills/docket-implement-next/references/edge-paths.md", diff --git a/internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md b/internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md index d06f13049..488389583 100644 --- a/internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md +++ b/internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md @@ -81,7 +81,7 @@ The `workspace.prepare` operation reloads the authoritative snapshot, resolves t **Plan authoring (dispatched).** The parent stays the orchestrator. Dispatch `docket-plan-writer` foreground at the model/effort its wrapper resolves. Its payload supplies, so the child rediscovers nothing: Feature worktree: -It also supplies the change id, title, synchronized change-file and spec paths, pre-dispatch HEAD, resolved `$SKILL_PLAN` and `$SKILL_BUILD`, learnings enablement and (when enabled) `/learnings/README.md`, and the synchronized change-file path its plan backlink targets. The child invokes the resolved plan skill **DIRECTED to:** write the plan file and stop there (on `auto` or a missing skill it authors the fallback artifact itself, warning prominently — carry that warning into the run report and PR body), stamps the backlink with the `artifact.backlink` operation (`--artifact --change `), stages only the plan path, commits on `/` with the exact git trailer `Docket-Plan-Path: `, and returns the single success line `PLAN_PATH=`. +It also supplies the change id, title, synchronized change-file and spec paths, pre-dispatch HEAD, resolved `$SKILL_PLAN` and `$SKILL_BUILD`, learnings enablement and (when enabled) `/learnings/README.md`, and the synchronized change-file path its plan backlink targets. The child invokes the resolved plan skill **DIRECTED to:** write the plan file and stop there (on `auto` or a missing skill it authors the fallback artifact itself, warning prominently — carry that warning into the run report and PR body), stamps the backlink with the `artifact.backlink` operation (`--artifact --change `), stages only the plan path, commits on `/` with the exact git trailer `Docket-Plan-Path: `, and returns the single success line `PLAN_PATH=`. **Verification (parent).** The returned path is a **claim, not proof**. Before attaching the plan, verify from git that: the path is a safe repo-relative path contained by the feature worktree; the file exists, is tracked, and changed after the pre-dispatch HEAD; the worktree is clean; the full branch delta since the pre-dispatch HEAD contains **only the returned plan file**; the plan commit carries **exactly one** `Docket-Plan-Path:` trailer whose value equals the returned path; and the artifact's managed backlink markers are ordered, balanced, and point home to this change. There is deliberately **no directory allowlist** — `docs/superpowers/plans/` belongs to `superpowers:writing-plans` and the fallback, and a custom `skills.plan` binding owns its own location; containment, single-artifact scope, committed state, and backlink identity are the stable properties. On a malformed return, unsafe path, unexpected delta, dirty worktree, missing commit, or invalid backlink: **halt** (`## Run halted` + the `halted` disposition) — **never adopt**, repair, or commit the child's uncommitted output, and never retry with the parent or a weaker model. Once verified, attach it: the `change.attach-plan` operation with `--id --version --path --commit `. Before its transaction `attach-plan` re-verifies the same facts from Git — never from the child return — (head + descends-from-base, a canonical tracked path in the allowed planning directory, the single-artifact delta with `--no-renames` and the `Docket-Plan-Path:` trailer, a balanced backlink targeting this change, no unresolved placeholder), rechecks the exact version, stores `plan:`, and renders the `## Artifacts` block and inline board atomically. A retry verifies the same plan identity (path + blob at the verified commit) and returns its prior applied outcome, never attaching whatever now occupies the path. The plan **file** merges with the code, so the `plan:` link resolves on the integration branch only after the PR merges (why `docket-status` ignores a missing `plan:` on an `implemented` change). @@ -120,7 +120,7 @@ The results artifact is **required for every change, trivial included** — ther 1. Verify ownership and quiescence — no live gate, running worker, or transferred drive. 2. Read and **preserve** the prior results content; a checkpoint updates it, never truncates it. -3. Write the update, then stamp and validate the back-link home with the `artifact.backlink` operation (`--artifact --change `). +3. Write the update, then stamp and validate the back-link home with the `artifact.backlink` operation (`--artifact --change `). 4. Commit **ONLY the results path** on `/` — stage by explicit path. 5. Publish through the existing feature-branch transport, **never forced**. 6. On first creation, attach via the `change.attach-results` operation with `--id --version --path --commit ` (the same verification as `attach-plan`), landing the `results:` FIELD on `metadata_branch` (the file rides the feature branch, the field lands via the transaction — the same split as `plan:`); later updates keep the **same path** and revalidate/reattach before completion. diff --git a/skills/docket-implement-next/SKILL.md b/skills/docket-implement-next/SKILL.md index d06f13049..488389583 100644 --- a/skills/docket-implement-next/SKILL.md +++ b/skills/docket-implement-next/SKILL.md @@ -81,7 +81,7 @@ The `workspace.prepare` operation reloads the authoritative snapshot, resolves t **Plan authoring (dispatched).** The parent stays the orchestrator. Dispatch `docket-plan-writer` foreground at the model/effort its wrapper resolves. Its payload supplies, so the child rediscovers nothing: Feature worktree: -It also supplies the change id, title, synchronized change-file and spec paths, pre-dispatch HEAD, resolved `$SKILL_PLAN` and `$SKILL_BUILD`, learnings enablement and (when enabled) `/learnings/README.md`, and the synchronized change-file path its plan backlink targets. The child invokes the resolved plan skill **DIRECTED to:** write the plan file and stop there (on `auto` or a missing skill it authors the fallback artifact itself, warning prominently — carry that warning into the run report and PR body), stamps the backlink with the `artifact.backlink` operation (`--artifact --change `), stages only the plan path, commits on `/` with the exact git trailer `Docket-Plan-Path: `, and returns the single success line `PLAN_PATH=`. +It also supplies the change id, title, synchronized change-file and spec paths, pre-dispatch HEAD, resolved `$SKILL_PLAN` and `$SKILL_BUILD`, learnings enablement and (when enabled) `/learnings/README.md`, and the synchronized change-file path its plan backlink targets. The child invokes the resolved plan skill **DIRECTED to:** write the plan file and stop there (on `auto` or a missing skill it authors the fallback artifact itself, warning prominently — carry that warning into the run report and PR body), stamps the backlink with the `artifact.backlink` operation (`--artifact --change `), stages only the plan path, commits on `/` with the exact git trailer `Docket-Plan-Path: `, and returns the single success line `PLAN_PATH=`. **Verification (parent).** The returned path is a **claim, not proof**. Before attaching the plan, verify from git that: the path is a safe repo-relative path contained by the feature worktree; the file exists, is tracked, and changed after the pre-dispatch HEAD; the worktree is clean; the full branch delta since the pre-dispatch HEAD contains **only the returned plan file**; the plan commit carries **exactly one** `Docket-Plan-Path:` trailer whose value equals the returned path; and the artifact's managed backlink markers are ordered, balanced, and point home to this change. There is deliberately **no directory allowlist** — `docs/superpowers/plans/` belongs to `superpowers:writing-plans` and the fallback, and a custom `skills.plan` binding owns its own location; containment, single-artifact scope, committed state, and backlink identity are the stable properties. On a malformed return, unsafe path, unexpected delta, dirty worktree, missing commit, or invalid backlink: **halt** (`## Run halted` + the `halted` disposition) — **never adopt**, repair, or commit the child's uncommitted output, and never retry with the parent or a weaker model. Once verified, attach it: the `change.attach-plan` operation with `--id --version --path --commit `. Before its transaction `attach-plan` re-verifies the same facts from Git — never from the child return — (head + descends-from-base, a canonical tracked path in the allowed planning directory, the single-artifact delta with `--no-renames` and the `Docket-Plan-Path:` trailer, a balanced backlink targeting this change, no unresolved placeholder), rechecks the exact version, stores `plan:`, and renders the `## Artifacts` block and inline board atomically. A retry verifies the same plan identity (path + blob at the verified commit) and returns its prior applied outcome, never attaching whatever now occupies the path. The plan **file** merges with the code, so the `plan:` link resolves on the integration branch only after the PR merges (why `docket-status` ignores a missing `plan:` on an `implemented` change). @@ -120,7 +120,7 @@ The results artifact is **required for every change, trivial included** — ther 1. Verify ownership and quiescence — no live gate, running worker, or transferred drive. 2. Read and **preserve** the prior results content; a checkpoint updates it, never truncates it. -3. Write the update, then stamp and validate the back-link home with the `artifact.backlink` operation (`--artifact --change `). +3. Write the update, then stamp and validate the back-link home with the `artifact.backlink` operation (`--artifact --change `). 4. Commit **ONLY the results path** on `/` — stage by explicit path. 5. Publish through the existing feature-branch transport, **never forced**. 6. On first creation, attach via the `change.attach-results` operation with `--id --version --path --commit ` (the same verification as `attach-plan`), landing the `results:` FIELD on `metadata_branch` (the file rides the feature branch, the field lands via the transaction — the same split as `plan:`); later updates keep the **same path** and revalidate/reattach before completion. From a1db7f58eac0f1c80eedf6d500f596790c3e7802 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Sat, 26 Sep 2026 08:23:39 -0400 Subject: [PATCH 4/5] docs(results): change 0460 results --- ...absolute-change-path-with-unkno-results.md | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 docs/results/2026-09-26-artifact-backlink-refuses-an-absolute-change-path-with-unkno-results.md diff --git a/docs/results/2026-09-26-artifact-backlink-refuses-an-absolute-change-path-with-unkno-results.md b/docs/results/2026-09-26-artifact-backlink-refuses-an-absolute-change-path-with-unkno-results.md new file mode 100644 index 000000000..035fd0af9 --- /dev/null +++ b/docs/results/2026-09-26-artifact-backlink-refuses-an-absolute-change-path-with-unkno-results.md @@ -0,0 +1,25 @@ + +> ↩ **[Change 0460 — artifact.backlink refuses an absolute --change path with unknown-change](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0460-artifact-backlink-refuses-an-absolute-change-path-with-unkno.md)** + +# artifact.backlink refuses an absolute --change path with unknown-change — Results + +**Human action:** None required. The fix is covered by automated tests; a reviewer only needs to read the PR diff. + +## Outcome + +`docket artifact backlink --change ` used to answer `unknown-change` when given an absolute path or an oddly spelled one (`./docs/...`, `a//b`, a `..` escape). That made it look like the change record was missing, when the real problem was the path's form. The command now checks `--change` the same way `--artifact` and the plan/results attach commands already check their paths: + +- an absolute path is refused with reason `absolute-path`; +- an empty value, a `..` escape, or a non-canonical spelling is refused with reason `path-escape`; +- each message names the `--change` flag and shows the expected repo-relative form. + +Only a well-formed repo-relative path that matches no record still returns `unknown-change`. Nothing is written when any of these refusals fires. Absolute paths are still not accepted, as the spec decided. + +The `docket-implement-next` skill text that led an agent to pass an absolute path now says "repo-relative" at both places it passes `artifact.backlink` flags, and the embedded copy of that skill was regenerated. + +## Verification performed + +- New table test `TestArtifactBacklinkChangePathValidation` covers absolute, `..` escape, `./` spelling, interior `..`, trailing slash, empty and whitespace-only values. It failed before the fix (all reported `unknown-change`) and passes after it. The artifact file is checked byte-identical after each refusal. +- Mutation check: removing the absolute-path branch made the `absolute` case fail, as intended. +- The embedded-asset drift guard and harness golden tests pass after regeneration. +- The full suite runs at the build gate; its result is recorded in the PR's build-evidence block. From 2bf58578d16419104b0dd3551b1368aff4b7f153 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Sat, 26 Sep 2026 08:32:34 -0400 Subject: [PATCH 5/5] docs(skills): fold the redundant backlink-target payload clause back under the word budget (change 0460) Commit 379e4096 named the repo-relative form on both artifact.backlink flag placeholders, pushing skills/docket-implement-next/SKILL.md to 8029 words against its 8025-word TestSkillSizeBudgets ceiling. The plan-writer payload sentence listed the synchronized change-file path twice; fold the second mention into the first as the plan backlink's target. No meaning lost, repo-relative wording kept, ceiling unchanged; embedded copy regenerated. --- internal/assets/embedded/manifest.json | 6 +++--- .../embedded/tree/skills/docket-implement-next/SKILL.md | 2 +- skills/docket-implement-next/SKILL.md | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index 9a093b36e..e58f7fec1 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:42cade1bb3666833e5b1d8111f77e5285e46f404f98093f324e0adf0f4988211", + "asset_set_id": "sha256:99499348930cd7288941f1c5f8207d27e814d01aec0773189fd01268fb903a23", "entries": [ { "path": ".docket.example.yml", @@ -406,8 +406,8 @@ "path": "skills/docket-implement-next/SKILL.md", "role": "skill", "mode": 420, - "size": 55967, - "sha256": "133e27896fe1b63070f8c8f5b820fadede3c479d1e5a25b3257fe56435027bda" + "size": 55936, + "sha256": "eb9e1364cb75f32b51a9abb708486fab3e1864fdf2e0c79e254454b6e894ff67" }, { "path": "skills/docket-implement-next/references/edge-paths.md", diff --git a/internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md b/internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md index 488389583..5ccb1f6b3 100644 --- a/internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md +++ b/internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md @@ -81,7 +81,7 @@ The `workspace.prepare` operation reloads the authoritative snapshot, resolves t **Plan authoring (dispatched).** The parent stays the orchestrator. Dispatch `docket-plan-writer` foreground at the model/effort its wrapper resolves. Its payload supplies, so the child rediscovers nothing: Feature worktree: -It also supplies the change id, title, synchronized change-file and spec paths, pre-dispatch HEAD, resolved `$SKILL_PLAN` and `$SKILL_BUILD`, learnings enablement and (when enabled) `/learnings/README.md`, and the synchronized change-file path its plan backlink targets. The child invokes the resolved plan skill **DIRECTED to:** write the plan file and stop there (on `auto` or a missing skill it authors the fallback artifact itself, warning prominently — carry that warning into the run report and PR body), stamps the backlink with the `artifact.backlink` operation (`--artifact --change `), stages only the plan path, commits on `/` with the exact git trailer `Docket-Plan-Path: `, and returns the single success line `PLAN_PATH=`. +It also supplies the change id, title, synchronized change-file (the plan backlink's target) and spec paths, pre-dispatch HEAD, resolved `$SKILL_PLAN` and `$SKILL_BUILD`, and learnings enablement plus (when enabled) `/learnings/README.md`. The child invokes the resolved plan skill **DIRECTED to:** write the plan file and stop there (on `auto` or a missing skill it authors the fallback artifact itself, warning prominently — carry that warning into the run report and PR body), stamps the backlink with the `artifact.backlink` operation (`--artifact --change `), stages only the plan path, commits on `/` with the exact git trailer `Docket-Plan-Path: `, and returns the single success line `PLAN_PATH=`. **Verification (parent).** The returned path is a **claim, not proof**. Before attaching the plan, verify from git that: the path is a safe repo-relative path contained by the feature worktree; the file exists, is tracked, and changed after the pre-dispatch HEAD; the worktree is clean; the full branch delta since the pre-dispatch HEAD contains **only the returned plan file**; the plan commit carries **exactly one** `Docket-Plan-Path:` trailer whose value equals the returned path; and the artifact's managed backlink markers are ordered, balanced, and point home to this change. There is deliberately **no directory allowlist** — `docs/superpowers/plans/` belongs to `superpowers:writing-plans` and the fallback, and a custom `skills.plan` binding owns its own location; containment, single-artifact scope, committed state, and backlink identity are the stable properties. On a malformed return, unsafe path, unexpected delta, dirty worktree, missing commit, or invalid backlink: **halt** (`## Run halted` + the `halted` disposition) — **never adopt**, repair, or commit the child's uncommitted output, and never retry with the parent or a weaker model. Once verified, attach it: the `change.attach-plan` operation with `--id --version --path --commit `. Before its transaction `attach-plan` re-verifies the same facts from Git — never from the child return — (head + descends-from-base, a canonical tracked path in the allowed planning directory, the single-artifact delta with `--no-renames` and the `Docket-Plan-Path:` trailer, a balanced backlink targeting this change, no unresolved placeholder), rechecks the exact version, stores `plan:`, and renders the `## Artifacts` block and inline board atomically. A retry verifies the same plan identity (path + blob at the verified commit) and returns its prior applied outcome, never attaching whatever now occupies the path. The plan **file** merges with the code, so the `plan:` link resolves on the integration branch only after the PR merges (why `docket-status` ignores a missing `plan:` on an `implemented` change). diff --git a/skills/docket-implement-next/SKILL.md b/skills/docket-implement-next/SKILL.md index 488389583..5ccb1f6b3 100644 --- a/skills/docket-implement-next/SKILL.md +++ b/skills/docket-implement-next/SKILL.md @@ -81,7 +81,7 @@ The `workspace.prepare` operation reloads the authoritative snapshot, resolves t **Plan authoring (dispatched).** The parent stays the orchestrator. Dispatch `docket-plan-writer` foreground at the model/effort its wrapper resolves. Its payload supplies, so the child rediscovers nothing: Feature worktree: -It also supplies the change id, title, synchronized change-file and spec paths, pre-dispatch HEAD, resolved `$SKILL_PLAN` and `$SKILL_BUILD`, learnings enablement and (when enabled) `/learnings/README.md`, and the synchronized change-file path its plan backlink targets. The child invokes the resolved plan skill **DIRECTED to:** write the plan file and stop there (on `auto` or a missing skill it authors the fallback artifact itself, warning prominently — carry that warning into the run report and PR body), stamps the backlink with the `artifact.backlink` operation (`--artifact --change `), stages only the plan path, commits on `/` with the exact git trailer `Docket-Plan-Path: `, and returns the single success line `PLAN_PATH=`. +It also supplies the change id, title, synchronized change-file (the plan backlink's target) and spec paths, pre-dispatch HEAD, resolved `$SKILL_PLAN` and `$SKILL_BUILD`, and learnings enablement plus (when enabled) `/learnings/README.md`. The child invokes the resolved plan skill **DIRECTED to:** write the plan file and stop there (on `auto` or a missing skill it authors the fallback artifact itself, warning prominently — carry that warning into the run report and PR body), stamps the backlink with the `artifact.backlink` operation (`--artifact --change `), stages only the plan path, commits on `/` with the exact git trailer `Docket-Plan-Path: `, and returns the single success line `PLAN_PATH=`. **Verification (parent).** The returned path is a **claim, not proof**. Before attaching the plan, verify from git that: the path is a safe repo-relative path contained by the feature worktree; the file exists, is tracked, and changed after the pre-dispatch HEAD; the worktree is clean; the full branch delta since the pre-dispatch HEAD contains **only the returned plan file**; the plan commit carries **exactly one** `Docket-Plan-Path:` trailer whose value equals the returned path; and the artifact's managed backlink markers are ordered, balanced, and point home to this change. There is deliberately **no directory allowlist** — `docs/superpowers/plans/` belongs to `superpowers:writing-plans` and the fallback, and a custom `skills.plan` binding owns its own location; containment, single-artifact scope, committed state, and backlink identity are the stable properties. On a malformed return, unsafe path, unexpected delta, dirty worktree, missing commit, or invalid backlink: **halt** (`## Run halted` + the `halted` disposition) — **never adopt**, repair, or commit the child's uncommitted output, and never retry with the parent or a weaker model. Once verified, attach it: the `change.attach-plan` operation with `--id --version --path --commit `. Before its transaction `attach-plan` re-verifies the same facts from Git — never from the child return — (head + descends-from-base, a canonical tracked path in the allowed planning directory, the single-artifact delta with `--no-renames` and the `Docket-Plan-Path:` trailer, a balanced backlink targeting this change, no unresolved placeholder), rechecks the exact version, stores `plan:`, and renders the `## Artifacts` block and inline board atomically. A retry verifies the same plan identity (path + blob at the verified commit) and returns its prior applied outcome, never attaching whatever now occupies the path. The plan **file** merges with the code, so the `plan:` link resolves on the integration branch only after the PR merges (why `docket-status` ignores a missing `plan:` on an `implemented` change).