From 8c97beae1af426b22d95b10040bb48e6e765f643 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Sun, 27 Sep 2026 06:04:10 -0400 Subject: [PATCH 1/5] docs(plan): implementation plan for change 0455 Docket-Plan-Path: docs/superpowers/plans/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina.md --- ...ecord-invalid-reason-in-the-docket-fina.md | 203 ++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina.md diff --git a/docs/superpowers/plans/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina.md b/docs/superpowers/plans/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina.md new file mode 100644 index 000000000..08e12c1b9 --- /dev/null +++ b/docs/superpowers/plans/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina.md @@ -0,0 +1,203 @@ + +> ↩ **[Change 0455 — Document finalize's record-invalid reason in the docket-finalize-change skill](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0455-document-finalize-s-record-invalid-reason-in-the-docket-fina.md)** + +# Document the `record-invalid` refusal — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: execute with the `docket-build` build role (task-by-task via its profile agents, one full-suite gate at the end). Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Document the `record-invalid` refusal of `finalize.merge` (in `docket-finalize-change` step 8) and `pr.publish` (in `docket-implement-next` "Publish the PR"), and fix the unrelated `gofmt` drift in `internal/githubcli/comment_integration_test.go`. + +**Architecture:** Prose-only edits to two distributed skill bodies, appended inside the existing paragraphs so they add no lines. Each edit comes with the matching word-ceiling bump in `internal/repoguard/budgets_test.go` (both files are exactly at their word ceilings today) and a regenerated embedded asset bundle (`internal/assets/embedded/`). The gofmt fix is formatting only. + +**Tech Stack:** Markdown skill bodies, Go (`go generate` asset bundle, `TestSkillSizeBudgets`), the toolchain-pinned `gofmt`. + +**Spec:** `docs/superpowers/specs/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina-design.md` (on the `docket` branch; synchronized copy at `/Users/homer/dev/docket/.docket/docs/superpowers/specs/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina-design.md`) + +## Global Constraints + +- No behavior change to either operation, to the reason token `record-invalid`, or to the `record-invalid`-before-`already-merged` precedence. +- No new repoguard test that pins skill reason tokens (YAGNI, per the spec's non-goals). +- Skill bodies ship into other repos, so the added prose must not name this repo's source files (`internal/app/...`), line numbers, or local-only facts. Name the operation, the reason token, and the result fields. +- Cross-references anchor on symbol names or quoted clauses, never line numbers (ADR-0054). +- Budgets are currently at the ceiling: `docket-finalize-change/SKILL.md` is 238 lines / 5520 words (ceiling 239 / 5520), and `docket-implement-next/SKILL.md` is 214 lines / 8025 words (ceiling 214 / 8025). Add **no new lines**: append to the existing paragraph line. Raise only the word ceiling, and set it to the exact post-edit `wc -w` count. +- Stage explicit paths only. Never `git add -A`. +- Format with the toolchain gofmt, never PATH's: `"$(GOTOOLCHAIN="$(awk '$1=="toolchain"{print $2}' go.mod)" go env GOROOT)/bin/gofmt"`. + +## Review Focus + +1. **Scope wording.** The prose must say the check covers the change, its `depends_on` targets, and its stack ancestors, and never associative links (`related`, `discovered_from`, ADRs). Wording that suggests any broken record in the corpus can trigger the refusal is wrong. +2. **Remedy wording.** Repair exactly the records the result's `findings` name, then re-run. Nothing may suggest guessing at the culprit, editing unnamed records, or an override flag. None exists. +3. **Merged-outside-docket case.** It must read as expected behavior (closeout would refuse the same defect), and the text must say that after the repair the re-run takes the merged-recovery path. +4. **Budget honesty.** Each ceiling bump equals the measured count and carries a `0455:` comment. Neither file may cross its line ceiling. +5. **Bundle drift.** `go run ./cmd/genassets -check` passes after every skill edit. An edited `skills/` file without a regenerated `internal/assets/embedded/tree/skills/...` copy reddens the suite. + +--- + +### Task 1: Document `record-invalid` in `docket-finalize-change` step 8 + +**Files:** +- Modify: `skills/docket-finalize-change/SKILL.md`, section `### 8. Merge exactly once`, first paragraph (the one beginning "The `finalize.merge` operation with `--id --version --head `") +- Modify: `internal/repoguard/budgets_test.go`, the `{"docket-finalize-change/SKILL.md", 239, 5520}` row of `skillBudgets` +- Regenerate: `internal/assets/embedded/` (manifest + `tree/skills/docket-finalize-change/SKILL.md`) +- Decided, no edit: `skills/docket-finalize-change/references/gate-failure.md`. Its abort-and-report set already covers merge refusals generically ("a merge conjunct that fails at the fresh recheck … returns the conjunct's token"). It lists no individual pre-effect refusal token, and the sibling `merge-method-unavailable` is absent too. The spec says not to add a line only for symmetry. Its budget (145/147 lines, 1892/1901 words) also has no real headroom. + +**Interfaces:** +- Consumes: nothing. +- Produces: nothing code-facing. + +- [ ] **Step 1: Confirm the gap (the failing check)** + +Run from the worktree root: +```bash +grep -c -- 'record-invalid' skills/docket-finalize-change/SKILL.md +``` +Expected: `0` (exit 1). + +- [ ] **Step 2: Append the prose to the step-8 paragraph** + +Insert the following text into the same line, directly after the sentence that ends "… it is not `merge-denied` and is never retried with another method." and before "It merges at the exact expected head". Keep a single space on each side and add no newline: + +```text +Before any GitHub call it also validates the change and the records it structurally requires — its `depends_on` targets and stack ancestors, never `related`, `discovered_from`, or ADR links — and refuses `blocked` with reason `record-invalid` (`halted`) when any carries a validation error; the result's `findings` name each bad record's code and path. No merge call was made: repair exactly the records `findings` name — never guess, never edit an unnamed record; no override exists — then re-run finalize. Because this check precedes already-merged recovery, a PR merged outside docket whose scope holds a defective record reports `record-invalid`, not `already-merged`; that is expected (closeout would refuse the same defect), and once repaired the re-run takes the merged-recovery path. +``` + +- [ ] **Step 3: Verify the prose and measure** + +```bash +f=skills/docket-finalize-change/SKILL.md +flat=$(tr -s '[:space:]' ' ' < "$f") +grep -qF -- 'refuses `blocked` with reason `record-invalid`' <<<"$flat" && echo scope-ok +grep -qF -- 'repair exactly the records `findings` name' <<<"$flat" && echo remedy-ok +grep -qF -- 'reports `record-invalid`, not `already-merged`' <<<"$flat" && echo merged-ok +wc -l < "$f"; wc -w < "$f" +``` +Expected: `scope-ok`, `remedy-ok`, and `merged-ok` all print. The line count is still `238`. Record the word count as `W1` (about 5630). + +- [ ] **Step 4: Raise the word ceiling to exactly `W1`** + +In `internal/repoguard/budgets_test.go`, change the row `{"docket-finalize-change/SKILL.md", 239, 5520},` to `{"docket-finalize-change/SKILL.md", 239, },` and put `0455: +record-invalid refusal (structural scope, findings remedy, merged-outside-docket precedence) in step 8 (word ceiling 5520 -> ); ` at the start of that row's trailing comment. Leave the rest of the comment unchanged. Then run the toolchain gofmt on the file: +```bash +"$(GOTOOLCHAIN="$(awk '$1=="toolchain"{print $2}' go.mod)" go env GOROOT)/bin/gofmt" -w internal/repoguard/budgets_test.go +``` + +- [ ] **Step 5: Regenerate the embedded bundle** + +```bash +go generate ./internal/assets && go run ./cmd/genassets -check +``` +Expected: `-check` exits 0. `git status --porcelain` shows only the skill, the budgets file, `internal/assets/embedded/manifest.json`, and `internal/assets/embedded/tree/skills/docket-finalize-change/SKILL.md`. + +- [ ] **Step 6: Run the guards** + +```bash +go test -count=1 ./internal/repoguard/ ./internal/assets/ +``` +Expected: PASS. Mutation check: temporarily set the ceiling to `-1`, rerun `go test -count=1 -run TestSkillSizeBudgets ./internal/repoguard/`, see it fail, then restore it. + +- [ ] **Step 7: Commit** + +```bash +git add skills/docket-finalize-change/SKILL.md internal/repoguard/budgets_test.go internal/assets/embedded/manifest.json internal/assets/embedded/tree/skills/docket-finalize-change/SKILL.md +git commit -m "docs(skills): document finalize.merge record-invalid refusal in finalize step 8 (change 0455)" +``` + +### Task 2: Document `record-invalid` in `docket-implement-next` "Publish the PR" + +**Files:** +- Modify: `skills/docket-implement-next/SKILL.md`, the `**Publish the PR.**` paragraph (the one containing "the `pr.publish` operation with `--id --head `") +- Modify: `internal/repoguard/budgets_test.go`, the `{"docket-implement-next/SKILL.md", 214, 8025}` row +- Regenerate: `internal/assets/embedded/` (manifest + `tree/skills/docket-implement-next/SKILL.md`) + +**Interfaces:** +- Consumes: the Task 1 commit (the budgets file and manifest already carry its edits). +- Produces: nothing code-facing. + +- [ ] **Step 1: Confirm the gap** + +```bash +grep -c -- 'record-invalid' skills/docket-implement-next/SKILL.md +``` +Expected: `0`. + +- [ ] **Step 2: Append the clause to the end of the paragraph line** + +After the paragraph's last sentence ("… and the result redacts the body bytes."), on the same line after one space, append: + +```text +It refuses `record-invalid` (`invalid-state`) before any GitHub call when the change, a `depends_on` target, or a stack ancestor carries a validation error (associative links are never checked): no PR was created or edited — repair exactly the records the result's `findings` name, then re-publish; the run keeps its existing halt posture for a typed refusal. +``` + +- [ ] **Step 3: Verify and measure** + +```bash +f=skills/docket-implement-next/SKILL.md +flat=$(tr -s '[:space:]' ' ' < "$f") +grep -qF -- 'It refuses `record-invalid` (`invalid-state`) before any GitHub call' <<<"$flat" && echo scope-ok +grep -qF -- 'repair exactly the records the result'"'"'s `findings` name, then re-publish' <<<"$flat" && echo remedy-ok +wc -l < "$f"; wc -w < "$f" +``` +Expected: `scope-ok` and `remedy-ok` print. The line count is still `214`. Record the word count as `W2` (about 8080). + +- [ ] **Step 4: Raise the word ceiling to exactly `W2`** + +Change `{"docket-implement-next/SKILL.md", 214, 8025},` to `{"docket-implement-next/SKILL.md", 214, },` and put `0455: +pr.publish record-invalid refusal clause (word ceiling 8025 -> ); ` at the start of its trailing comment. Run the toolchain gofmt on the file, as in Task 1 Step 4. + +- [ ] **Step 5: Regenerate and check the bundle** + +```bash +go generate ./internal/assets && go run ./cmd/genassets -check +``` +Expected: exit 0. The changed paths are only the skill, the budgets file, the manifest, and `internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md`. + +- [ ] **Step 6: Run the guards** + +```bash +go test -count=1 ./internal/repoguard/ ./internal/assets/ +``` +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add skills/docket-implement-next/SKILL.md internal/repoguard/budgets_test.go internal/assets/embedded/manifest.json internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md +git commit -m "docs(skills): document pr.publish record-invalid refusal in implement-next (change 0455)" +``` + +### Task 3: Fix gofmt drift in `internal/githubcli/comment_integration_test.go` + +**Files:** +- Modify: `internal/githubcli/comment_integration_test.go` (formatting only) + +- [ ] **Step 1: Confirm the drift** + +```bash +G="$(GOTOOLCHAIN="$(awk '$1=="toolchain"{print $2}' go.mod)" go env GOROOT)/bin/gofmt" +"$G" -l internal/githubcli/ +``` +Expected: prints `internal/githubcli/comment_integration_test.go`. + +- [ ] **Step 2: Reformat** + +```bash +"$G" -w internal/githubcli/comment_integration_test.go +``` + +- [ ] **Step 3: Verify the diff is whitespace-only** + +```bash +"$G" -l internal/githubcli/ # expected: no output +git diff -w --stat -- internal/githubcli/comment_integration_test.go # expected: empty (whitespace-only change) +go vet -tags integration ./internal/githubcli/ +``` +Expected: `gofmt -l` prints nothing, `git diff -w` is empty, and vet passes. If `git diff -w` is not empty (for example, gofmt reordered or aligned something non-whitespace), inspect the diff and confirm it is gofmt's own canonical output before continuing. + +- [ ] **Step 4: Commit** + +```bash +git add internal/githubcli/comment_integration_test.go +git commit -m "style(githubcli): gofmt comment_integration_test.go (change 0455)" +``` + +### Build gate (docket-build, after all tasks) + +Run the full suite with the command `build.test_command` resolves to from config, entered from source. Read the budget report even when the run is green. From a6a68093d5cd33a879763c4fce9a7121159f3902 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Sun, 27 Sep 2026 06:05:55 -0400 Subject: [PATCH 2/5] docs(skills): document finalize.merge record-invalid refusal in finalize step 8 (change 0455) --- internal/assets/embedded/manifest.json | 6 +++--- .../embedded/tree/skills/docket-finalize-change/SKILL.md | 2 +- internal/repoguard/budgets_test.go | 2 +- skills/docket-finalize-change/SKILL.md | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index 1fa71528e..124df7173 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:ffac8db623e2f3660fca51a30267487ebfc39c86d071d8a0ad2fe57d9afeb3b4", + "asset_set_id": "sha256:9a08f83a285e0bc24bab857ee6ea4ea2559434076769ca8bda75b68b16ae185c", "entries": [ { "path": ".docket.example.yml", @@ -385,8 +385,8 @@ "path": "skills/docket-finalize-change/SKILL.md", "role": "skill", "mode": 420, - "size": 38448, - "sha256": "db2b2aee572b665cbfff43a1f0ccbff2b875680e725ae30bf02e5212ed3e3dda" + "size": 39248, + "sha256": "a5f68aed7715f187286004d5ea64827aa137e347c66897bb0980a362d70a1ebd" }, { "path": "skills/docket-finalize-change/references/gate-failure.md", diff --git a/internal/assets/embedded/tree/skills/docket-finalize-change/SKILL.md b/internal/assets/embedded/tree/skills/docket-finalize-change/SKILL.md index e6bebfe2d..c5a490708 100644 --- a/internal/assets/embedded/tree/skills/docket-finalize-change/SKILL.md +++ b/internal/assets/embedded/tree/skills/docket-finalize-change/SKILL.md @@ -140,7 +140,7 @@ The `finalize.publish` operation with `--id --attempt --head --version --head `. It reloads fresh authority and rechecks every merge conjunct immediately before the effect — implemented, PR identity, heads agree, base is the effective base, gate satisfied, approval satisfied, no open children, not superseded — refusing with that conjunct's closed token (`not-mergeable`, `pr-not-open`, `unresolved-base`, a child conjunct, …) and issuing **no** merge call when any fails. It then selects the best merge method the repository settings and the base branch's active rules permit, in the fixed order rebase → merge commit → squash, and attempts exactly that one; the document's `method` field reports it (absent on already-merged recovery). A cleanly observed empty permitted set refuses `blocked` with reason `merge-method-unavailable` before any merge — fix the repository or branch-rule merge settings; it is not `merge-denied` and is never retried with another method. It merges at the exact expected head, never requests a branch delete, and verifies the merge authoritatively: a reprobe returns the exact `mergedAt`/merge-commit facts and a Git fetch proves the merge commit reachable from the destination tip. An open PR on reprobe is not merged; a different head/base is `contended`; an unobservable result is `unknown` — none permits closeout. An already-merged exact PR is a verified no-op regardless of who merged it, never a second merge. +The `finalize.merge` operation with `--id --version --head `. It reloads fresh authority and rechecks every merge conjunct immediately before the effect — implemented, PR identity, heads agree, base is the effective base, gate satisfied, approval satisfied, no open children, not superseded — refusing with that conjunct's closed token (`not-mergeable`, `pr-not-open`, `unresolved-base`, a child conjunct, …) and issuing **no** merge call when any fails. It then selects the best merge method the repository settings and the base branch's active rules permit, in the fixed order rebase → merge commit → squash, and attempts exactly that one; the document's `method` field reports it (absent on already-merged recovery). A cleanly observed empty permitted set refuses `blocked` with reason `merge-method-unavailable` before any merge — fix the repository or branch-rule merge settings; it is not `merge-denied` and is never retried with another method. Before any GitHub call it also validates the change and the records it structurally requires — its `depends_on` targets and stack ancestors, never `related`, `discovered_from`, or ADR links — and refuses `blocked` with reason `record-invalid` (`halted`) when any carries a validation error; the result's `findings` name each bad record's code and path. No merge call was made: repair exactly the records `findings` name — never guess, never edit an unnamed record; no override exists — then re-run finalize. Because this check precedes already-merged recovery, a PR merged outside docket whose scope holds a defective record reports `record-invalid`, not `already-merged`; that is expected (closeout would refuse the same defect), and once repaired the re-run takes the merged-recovery path. It merges at the exact expected head, never requests a branch delete, and verifies the merge authoritatively: a reprobe returns the exact `mergedAt`/merge-commit facts and a Git fetch proves the merge commit reachable from the destination tip. An open PR on reprobe is not merged; a different head/base is `contended`; an unobservable result is `unknown` — none permits closeout. An already-merged exact PR is a verified no-op regardless of who merged it, never a second merge. `--admin` is honored **only** on an attended, explicitly-named run where a sole maintainer forces past an otherwise-unsatisfiable required review; it is never inferred from an approval absence or a permission error, and a `merge-denied` stays `denied` (`halted`). A named id overrides the `approval-required` and `finalize-blocked` skips (step 1); it never overrides malformed state, a wrong PR identity, an unsafe stack, or the repair sign-off. diff --git a/internal/repoguard/budgets_test.go b/internal/repoguard/budgets_test.go index 348c75de0..63044b95a 100644 --- a/internal/repoguard/budgets_test.go +++ b/internal/repoguard/budgets_test.go @@ -194,7 +194,7 @@ var skillBudgets = []skillBudget{ {"docket-convention/references/learnings.md", 84, 580}, {"docket-convention/references/stacked-changes.md", 215, 2140}, // 0327: +carry-preservation contract prose (see note above) {"docket-convention/references/terminal-close-out.md", 240, 2150}, - {"docket-finalize-change/SKILL.md", 239, 5520}, // 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/SKILL.md", 239, 5637}, // 0455: +record-invalid refusal (structural scope, findings remedy, merged-outside-docket precedence) in step 8 (word ceiling 5520 -> 5637); 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", 147, 1901}, // 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) {"docket-groom-next/SKILL.md", 77, 1889}, // 0445: +revise route for already-groomed explicit ids (Step 1) and the fifth Step-4 exit (word ceiling 1650 -> 1813); +spec_version pin for a spec-body revise (1813 -> 1849); +revise spec_markdown excludes the backlink block (1849 -> 1850); +revise in the description and the revise contended/board clauses (1850 -> 1889) {"docket-implement-next/SKILL.md", 214, 8025}, // 0448: +named-invocation branch; bounded own-dependency closeout moved to edge-paths.md (ceilings 210/7716 -> 214/8270 -> 214/8025); 0393: +exact payload, marker, and direct-dispatch lines atop 0410/0354/0376; 0375: +gate-epoch resume pointer (word ceiling 7530 -> 7547); 0440: reader-first results prose diff --git a/skills/docket-finalize-change/SKILL.md b/skills/docket-finalize-change/SKILL.md index e6bebfe2d..c5a490708 100644 --- a/skills/docket-finalize-change/SKILL.md +++ b/skills/docket-finalize-change/SKILL.md @@ -140,7 +140,7 @@ The `finalize.publish` operation with `--id --attempt --head --version --head `. It reloads fresh authority and rechecks every merge conjunct immediately before the effect — implemented, PR identity, heads agree, base is the effective base, gate satisfied, approval satisfied, no open children, not superseded — refusing with that conjunct's closed token (`not-mergeable`, `pr-not-open`, `unresolved-base`, a child conjunct, …) and issuing **no** merge call when any fails. It then selects the best merge method the repository settings and the base branch's active rules permit, in the fixed order rebase → merge commit → squash, and attempts exactly that one; the document's `method` field reports it (absent on already-merged recovery). A cleanly observed empty permitted set refuses `blocked` with reason `merge-method-unavailable` before any merge — fix the repository or branch-rule merge settings; it is not `merge-denied` and is never retried with another method. It merges at the exact expected head, never requests a branch delete, and verifies the merge authoritatively: a reprobe returns the exact `mergedAt`/merge-commit facts and a Git fetch proves the merge commit reachable from the destination tip. An open PR on reprobe is not merged; a different head/base is `contended`; an unobservable result is `unknown` — none permits closeout. An already-merged exact PR is a verified no-op regardless of who merged it, never a second merge. +The `finalize.merge` operation with `--id --version --head `. It reloads fresh authority and rechecks every merge conjunct immediately before the effect — implemented, PR identity, heads agree, base is the effective base, gate satisfied, approval satisfied, no open children, not superseded — refusing with that conjunct's closed token (`not-mergeable`, `pr-not-open`, `unresolved-base`, a child conjunct, …) and issuing **no** merge call when any fails. It then selects the best merge method the repository settings and the base branch's active rules permit, in the fixed order rebase → merge commit → squash, and attempts exactly that one; the document's `method` field reports it (absent on already-merged recovery). A cleanly observed empty permitted set refuses `blocked` with reason `merge-method-unavailable` before any merge — fix the repository or branch-rule merge settings; it is not `merge-denied` and is never retried with another method. Before any GitHub call it also validates the change and the records it structurally requires — its `depends_on` targets and stack ancestors, never `related`, `discovered_from`, or ADR links — and refuses `blocked` with reason `record-invalid` (`halted`) when any carries a validation error; the result's `findings` name each bad record's code and path. No merge call was made: repair exactly the records `findings` name — never guess, never edit an unnamed record; no override exists — then re-run finalize. Because this check precedes already-merged recovery, a PR merged outside docket whose scope holds a defective record reports `record-invalid`, not `already-merged`; that is expected (closeout would refuse the same defect), and once repaired the re-run takes the merged-recovery path. It merges at the exact expected head, never requests a branch delete, and verifies the merge authoritatively: a reprobe returns the exact `mergedAt`/merge-commit facts and a Git fetch proves the merge commit reachable from the destination tip. An open PR on reprobe is not merged; a different head/base is `contended`; an unobservable result is `unknown` — none permits closeout. An already-merged exact PR is a verified no-op regardless of who merged it, never a second merge. `--admin` is honored **only** on an attended, explicitly-named run where a sole maintainer forces past an otherwise-unsatisfiable required review; it is never inferred from an approval absence or a permission error, and a `merge-denied` stays `denied` (`halted`). A named id overrides the `approval-required` and `finalize-blocked` skips (step 1); it never overrides malformed state, a wrong PR identity, an unsafe stack, or the repair sign-off. From e4788ed35bae55bed592919846586aaa245ba6c0 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Sun, 27 Sep 2026 06:07:11 -0400 Subject: [PATCH 3/5] docs(skills): document pr.publish record-invalid refusal in implement-next (change 0455) --- internal/assets/embedded/manifest.json | 6 +++--- .../embedded/tree/skills/docket-implement-next/SKILL.md | 2 +- internal/repoguard/budgets_test.go | 2 +- skills/docket-implement-next/SKILL.md | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index 124df7173..a08f902a2 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:9a08f83a285e0bc24bab857ee6ea4ea2559434076769ca8bda75b68b16ae185c", + "asset_set_id": "sha256:71a5604daef959c55ce32a78bf652460ccb5065fac9a363d6431eefe80b42210", "entries": [ { "path": ".docket.example.yml", @@ -406,8 +406,8 @@ "path": "skills/docket-implement-next/SKILL.md", "role": "skill", "mode": 420, - "size": 55936, - "sha256": "eb9e1364cb75f32b51a9abb708486fab3e1864fdf2e0c79e254454b6e894ff67" + "size": 56298, + "sha256": "09add3ea3f253758bf8acc5e4b827f335ff9eb7459d6bdcd59785c01f1ce3f3e" }, { "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 5ccb1f6b3..c33b3f2d4 100644 --- a/internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md +++ b/internal/assets/embedded/tree/skills/docket-implement-next/SKILL.md @@ -139,7 +139,7 @@ Local commit, remote publication, and metadata attachment are **distinct facts** **PR-body assembly.** The authored PR body carries three docket elements: the best-effort `#` reference (never `Closes #N`), the **back-link line** (`↩ Change — `, change 0136), and the **build-evidence block** (change 0170) — the current evidence record written marker-bounded (`<!-- docket:build-evidence:start -->` / `<!-- docket:build-evidence:end -->`) alongside the review outcome, carrying its actual result — `green`, or `skipped` / `build-gate-off` under `build_gate: off` — never overstated, the block `docket-finalize-change` reads to decide whether its post-rebase suite run can be skipped. Under dummy mode the body carries an authored plain-language block alongside them — see *Terminal disposition*. Before assembling any of the three, **read `references/edge-paths.md` now (blocking)** — it owns the mechanics, including marker validation and the expected-staleness rule for a post-gate `head_sha`. -**Publish the PR.** With the authored title+body and the verified evidence bytes each in a request file, the `pr.publish` operation with `--id <id> --head <feature head> --body <request-file> --evidence <evidence request-file>` reparses the evidence (never a prior result) and verifies local HEAD == published remote head == evidence head == requested head, and that the feature branch, effective-base branch, and repository identity agree, BEFORE calling the landed GitHub adapter; it then inserts or replaces ONLY the Docket-owned backlink and build-evidence blocks (authored prose byte-preserved) and returns the canonical PR reference, URL, number, head, base, and disposition. `unknown` stays `unknown` — reprobe, never a duplicate PR — and the result redacts the body bytes. +**Publish the PR.** With the authored title+body and the verified evidence bytes each in a request file, the `pr.publish` operation with `--id <id> --head <feature head> --body <request-file> --evidence <evidence request-file>` reparses the evidence (never a prior result) and verifies local HEAD == published remote head == evidence head == requested head, and that the feature branch, effective-base branch, and repository identity agree, BEFORE calling the landed GitHub adapter; it then inserts or replaces ONLY the Docket-owned backlink and build-evidence blocks (authored prose byte-preserved) and returns the canonical PR reference, URL, number, head, base, and disposition. `unknown` stays `unknown` — reprobe, never a duplicate PR — and the result redacts the body bytes. It refuses `record-invalid` (`invalid-state`) before any GitHub call when the change, a `depends_on` target, or a stack ancestor carries a validation error (associative links are never checked): no PR was created or edited — repair exactly the records the result's `findings` name, then re-publish; the run keeps its existing halt posture for a typed refusal. **Mark implemented.** The `change.mark-implemented` operation with `--id <id> --version <entity-version> --head <feature head> --pr <reference> --evidence <evidence request-file>` is the final mutation in this scope. Before its transaction it reprobes Git and GitHub and proves: the change is still the exact `in-progress` version, `reconciled: true`, linked to the verified plan; local and remote feature heads equal the supplied head; valid evidence names that head and a passed gate; exactly one verified PR for the feature branch targets the resolved effective-base branch and names that head; and a results artifact is attached and satisfies the final content contract and its artifact identity (missing, invalid, or unattached results — trivial changes included — refuse the transition). It then applies the landed transition atomically — `status: implemented` + `pr:` + updated date + `## Artifacts` block + inline board + audit receipt (letting the sweep read `pr:`), rendering the board inside that transaction, so no separate Board pass runs. It does NOT clear the claim, delete the branch/workspace, merge, archive, or close descendants (0316). A retry against an already-`implemented` change with matching PR/head returns the prior applied outcome, never a duplicate transition. diff --git a/internal/repoguard/budgets_test.go b/internal/repoguard/budgets_test.go index 63044b95a..1b9cdba69 100644 --- a/internal/repoguard/budgets_test.go +++ b/internal/repoguard/budgets_test.go @@ -197,7 +197,7 @@ var skillBudgets = []skillBudget{ {"docket-finalize-change/SKILL.md", 239, 5637}, // 0455: +record-invalid refusal (structural scope, findings remedy, merged-outside-docket precedence) in step 8 (word ceiling 5520 -> 5637); 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", 147, 1901}, // 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) {"docket-groom-next/SKILL.md", 77, 1889}, // 0445: +revise route for already-groomed explicit ids (Step 1) and the fifth Step-4 exit (word ceiling 1650 -> 1813); +spec_version pin for a spec-body revise (1813 -> 1849); +revise spec_markdown excludes the backlink block (1849 -> 1850); +revise in the description and the revise contended/board clauses (1850 -> 1889) - {"docket-implement-next/SKILL.md", 214, 8025}, // 0448: +named-invocation branch; bounded own-dependency closeout moved to edge-paths.md (ceilings 210/7716 -> 214/8270 -> 214/8025); 0393: +exact payload, marker, and direct-dispatch lines atop 0410/0354/0376; 0375: +gate-epoch resume pointer (word ceiling 7530 -> 7547); 0440: reader-first results prose + {"docket-implement-next/SKILL.md", 214, 8080}, // 0455: +pr.publish record-invalid refusal clause (word ceiling 8025 -> 8080); 0448: +named-invocation branch; bounded own-dependency closeout moved to edge-paths.md (ceilings 210/7716 -> 214/8270 -> 214/8025); 0393: +exact payload, marker, and direct-dispatch lines atop 0410/0354/0376; 0375: +gate-epoch resume pointer (word ceiling 7530 -> 7547); 0440: reader-first results prose {"docket-implement-next/references/edge-paths.md", 118, 1554}, // 0410: +resume/recovery + required-results reconciliation; 0375: +gate-epoch resume refusals (78/1091 -> 93/1261); 0448: +named own-dependency closeout moved from SKILL.md (93/1261 -> 118/1554) {"docket-implement-next/references/fix-loop.md", 190, 1958}, // 0410: +findings-to-results checkpoint linkage (see note above) {"docket-implement-next/results-template.md", 64, 446}, // 0440: reader-first template — action statement + merged Known issues (see note above) diff --git a/skills/docket-implement-next/SKILL.md b/skills/docket-implement-next/SKILL.md index 5ccb1f6b3..c33b3f2d4 100644 --- a/skills/docket-implement-next/SKILL.md +++ b/skills/docket-implement-next/SKILL.md @@ -139,7 +139,7 @@ Local commit, remote publication, and metadata attachment are **distinct facts** **PR-body assembly.** The authored PR body carries three docket elements: the best-effort `#<issue>` reference (never `Closes #N`), the **back-link line** (`↩ Change <padded-id> — <title>`, change 0136), and the **build-evidence block** (change 0170) — the current evidence record written marker-bounded (`<!-- docket:build-evidence:start -->` / `<!-- docket:build-evidence:end -->`) alongside the review outcome, carrying its actual result — `green`, or `skipped` / `build-gate-off` under `build_gate: off` — never overstated, the block `docket-finalize-change` reads to decide whether its post-rebase suite run can be skipped. Under dummy mode the body carries an authored plain-language block alongside them — see *Terminal disposition*. Before assembling any of the three, **read `references/edge-paths.md` now (blocking)** — it owns the mechanics, including marker validation and the expected-staleness rule for a post-gate `head_sha`. -**Publish the PR.** With the authored title+body and the verified evidence bytes each in a request file, the `pr.publish` operation with `--id <id> --head <feature head> --body <request-file> --evidence <evidence request-file>` reparses the evidence (never a prior result) and verifies local HEAD == published remote head == evidence head == requested head, and that the feature branch, effective-base branch, and repository identity agree, BEFORE calling the landed GitHub adapter; it then inserts or replaces ONLY the Docket-owned backlink and build-evidence blocks (authored prose byte-preserved) and returns the canonical PR reference, URL, number, head, base, and disposition. `unknown` stays `unknown` — reprobe, never a duplicate PR — and the result redacts the body bytes. +**Publish the PR.** With the authored title+body and the verified evidence bytes each in a request file, the `pr.publish` operation with `--id <id> --head <feature head> --body <request-file> --evidence <evidence request-file>` reparses the evidence (never a prior result) and verifies local HEAD == published remote head == evidence head == requested head, and that the feature branch, effective-base branch, and repository identity agree, BEFORE calling the landed GitHub adapter; it then inserts or replaces ONLY the Docket-owned backlink and build-evidence blocks (authored prose byte-preserved) and returns the canonical PR reference, URL, number, head, base, and disposition. `unknown` stays `unknown` — reprobe, never a duplicate PR — and the result redacts the body bytes. It refuses `record-invalid` (`invalid-state`) before any GitHub call when the change, a `depends_on` target, or a stack ancestor carries a validation error (associative links are never checked): no PR was created or edited — repair exactly the records the result's `findings` name, then re-publish; the run keeps its existing halt posture for a typed refusal. **Mark implemented.** The `change.mark-implemented` operation with `--id <id> --version <entity-version> --head <feature head> --pr <reference> --evidence <evidence request-file>` is the final mutation in this scope. Before its transaction it reprobes Git and GitHub and proves: the change is still the exact `in-progress` version, `reconciled: true`, linked to the verified plan; local and remote feature heads equal the supplied head; valid evidence names that head and a passed gate; exactly one verified PR for the feature branch targets the resolved effective-base branch and names that head; and a results artifact is attached and satisfies the final content contract and its artifact identity (missing, invalid, or unattached results — trivial changes included — refuse the transition). It then applies the landed transition atomically — `status: implemented` + `pr:` + updated date + `## Artifacts` block + inline board + audit receipt (letting the sweep read `pr:`), rendering the board inside that transaction, so no separate Board pass runs. It does NOT clear the claim, delete the branch/workspace, merge, archive, or close descendants (0316). A retry against an already-`implemented` change with matching PR/head returns the prior applied outcome, never a duplicate transition. From 2f77444d7fa2731a3c391007308acc1bb8b66c6b Mon Sep 17 00:00:00 2001 From: Daniel Hanold <danny@danielhanold.com> Date: Sun, 27 Sep 2026 06:27:18 -0400 Subject: [PATCH 4/5] docs(finalize): record-invalid envelope result and duplicate-id scope (change 0455 review) --- internal/assets/embedded/manifest.json | 6 +++--- .../embedded/tree/skills/docket-finalize-change/SKILL.md | 2 +- internal/repoguard/budgets_test.go | 2 +- skills/docket-finalize-change/SKILL.md | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index a08f902a2..8902df6c7 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:71a5604daef959c55ce32a78bf652460ccb5065fac9a363d6431eefe80b42210", + "asset_set_id": "sha256:f707a7ef0b87623c80f942469aafc407317973b44ea64e52c5fffad18d5cfdb4", "entries": [ { "path": ".docket.example.yml", @@ -385,8 +385,8 @@ "path": "skills/docket-finalize-change/SKILL.md", "role": "skill", "mode": 420, - "size": 39248, - "sha256": "a5f68aed7715f187286004d5ea64827aa137e347c66897bb0980a362d70a1ebd" + "size": 39314, + "sha256": "949ca22d0a0cc0ed13e333654fe617566f875ca66d57a791808242cdcde99407" }, { "path": "skills/docket-finalize-change/references/gate-failure.md", diff --git a/internal/assets/embedded/tree/skills/docket-finalize-change/SKILL.md b/internal/assets/embedded/tree/skills/docket-finalize-change/SKILL.md index c5a490708..95257b676 100644 --- a/internal/assets/embedded/tree/skills/docket-finalize-change/SKILL.md +++ b/internal/assets/embedded/tree/skills/docket-finalize-change/SKILL.md @@ -140,7 +140,7 @@ The `finalize.publish` operation with `--id <id> --attempt <attempt> --head <hea ### 8. Merge exactly once -The `finalize.merge` operation with `--id <id> --version <version> --head <head>`. It reloads fresh authority and rechecks every merge conjunct immediately before the effect — implemented, PR identity, heads agree, base is the effective base, gate satisfied, approval satisfied, no open children, not superseded — refusing with that conjunct's closed token (`not-mergeable`, `pr-not-open`, `unresolved-base`, a child conjunct, …) and issuing **no** merge call when any fails. It then selects the best merge method the repository settings and the base branch's active rules permit, in the fixed order rebase → merge commit → squash, and attempts exactly that one; the document's `method` field reports it (absent on already-merged recovery). A cleanly observed empty permitted set refuses `blocked` with reason `merge-method-unavailable` before any merge — fix the repository or branch-rule merge settings; it is not `merge-denied` and is never retried with another method. Before any GitHub call it also validates the change and the records it structurally requires — its `depends_on` targets and stack ancestors, never `related`, `discovered_from`, or ADR links — and refuses `blocked` with reason `record-invalid` (`halted`) when any carries a validation error; the result's `findings` name each bad record's code and path. No merge call was made: repair exactly the records `findings` name — never guess, never edit an unnamed record; no override exists — then re-run finalize. Because this check precedes already-merged recovery, a PR merged outside docket whose scope holds a defective record reports `record-invalid`, not `already-merged`; that is expected (closeout would refuse the same defect), and once repaired the re-run takes the merged-recovery path. It merges at the exact expected head, never requests a branch delete, and verifies the merge authoritatively: a reprobe returns the exact `mergedAt`/merge-commit facts and a Git fetch proves the merge commit reachable from the destination tip. An open PR on reprobe is not merged; a different head/base is `contended`; an unobservable result is `unknown` — none permits closeout. An already-merged exact PR is a verified no-op regardless of who merged it, never a second merge. +The `finalize.merge` operation with `--id <id> --version <version> --head <head>`. It reloads fresh authority and rechecks every merge conjunct immediately before the effect — implemented, PR identity, heads agree, base is the effective base, gate satisfied, approval satisfied, no open children, not superseded — refusing with that conjunct's closed token (`not-mergeable`, `pr-not-open`, `unresolved-base`, a child conjunct, …) and issuing **no** merge call when any fails. It then selects the best merge method the repository settings and the base branch's active rules permit, in the fixed order rebase → merge commit → squash, and attempts exactly that one; the document's `method` field reports it (absent on already-merged recovery). A cleanly observed empty permitted set refuses `blocked` with reason `merge-method-unavailable` before any merge — fix the repository or branch-rule merge settings; it is not `merge-denied` and is never retried with another method. Before any GitHub call it also validates the change and the records it structurally requires — its `depends_on` targets and stack ancestors (and any other record carrying its id), never `related`, `discovered_from`, or ADR links — and refuses `invalid-state`/`blocked` with reason `record-invalid` (the run is `halted`) when any carries a validation error; the result's `findings` name each bad record's code and path. No merge call was made: repair exactly the records `findings` name — never guess, never edit an unnamed record; no override exists — then re-run finalize. Because this check precedes already-merged recovery, a PR merged outside docket whose scope holds a defective record reports `record-invalid`, not `already-merged`; that is expected (closeout would refuse the same defect), and once repaired the re-run takes the merged-recovery path. It merges at the exact expected head, never requests a branch delete, and verifies the merge authoritatively: a reprobe returns the exact `mergedAt`/merge-commit facts and a Git fetch proves the merge commit reachable from the destination tip. An open PR on reprobe is not merged; a different head/base is `contended`; an unobservable result is `unknown` — none permits closeout. An already-merged exact PR is a verified no-op regardless of who merged it, never a second merge. `--admin` is honored **only** on an attended, explicitly-named run where a sole maintainer forces past an otherwise-unsatisfiable required review; it is never inferred from an approval absence or a permission error, and a `merge-denied` stays `denied` (`halted`). A named id overrides the `approval-required` and `finalize-blocked` skips (step 1); it never overrides malformed state, a wrong PR identity, an unsafe stack, or the repair sign-off. diff --git a/internal/repoguard/budgets_test.go b/internal/repoguard/budgets_test.go index 1b9cdba69..396b9179a 100644 --- a/internal/repoguard/budgets_test.go +++ b/internal/repoguard/budgets_test.go @@ -194,7 +194,7 @@ var skillBudgets = []skillBudget{ {"docket-convention/references/learnings.md", 84, 580}, {"docket-convention/references/stacked-changes.md", 215, 2140}, // 0327: +carry-preservation contract prose (see note above) {"docket-convention/references/terminal-close-out.md", 240, 2150}, - {"docket-finalize-change/SKILL.md", 239, 5637}, // 0455: +record-invalid refusal (structural scope, findings remedy, merged-outside-docket precedence) in step 8 (word ceiling 5520 -> 5637); 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/SKILL.md", 239, 5647}, // 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", 147, 1901}, // 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) {"docket-groom-next/SKILL.md", 77, 1889}, // 0445: +revise route for already-groomed explicit ids (Step 1) and the fifth Step-4 exit (word ceiling 1650 -> 1813); +spec_version pin for a spec-body revise (1813 -> 1849); +revise spec_markdown excludes the backlink block (1849 -> 1850); +revise in the description and the revise contended/board clauses (1850 -> 1889) {"docket-implement-next/SKILL.md", 214, 8080}, // 0455: +pr.publish record-invalid refusal clause (word ceiling 8025 -> 8080); 0448: +named-invocation branch; bounded own-dependency closeout moved to edge-paths.md (ceilings 210/7716 -> 214/8270 -> 214/8025); 0393: +exact payload, marker, and direct-dispatch lines atop 0410/0354/0376; 0375: +gate-epoch resume pointer (word ceiling 7530 -> 7547); 0440: reader-first results prose diff --git a/skills/docket-finalize-change/SKILL.md b/skills/docket-finalize-change/SKILL.md index c5a490708..95257b676 100644 --- a/skills/docket-finalize-change/SKILL.md +++ b/skills/docket-finalize-change/SKILL.md @@ -140,7 +140,7 @@ The `finalize.publish` operation with `--id <id> --attempt <attempt> --head <hea ### 8. Merge exactly once -The `finalize.merge` operation with `--id <id> --version <version> --head <head>`. It reloads fresh authority and rechecks every merge conjunct immediately before the effect — implemented, PR identity, heads agree, base is the effective base, gate satisfied, approval satisfied, no open children, not superseded — refusing with that conjunct's closed token (`not-mergeable`, `pr-not-open`, `unresolved-base`, a child conjunct, …) and issuing **no** merge call when any fails. It then selects the best merge method the repository settings and the base branch's active rules permit, in the fixed order rebase → merge commit → squash, and attempts exactly that one; the document's `method` field reports it (absent on already-merged recovery). A cleanly observed empty permitted set refuses `blocked` with reason `merge-method-unavailable` before any merge — fix the repository or branch-rule merge settings; it is not `merge-denied` and is never retried with another method. Before any GitHub call it also validates the change and the records it structurally requires — its `depends_on` targets and stack ancestors, never `related`, `discovered_from`, or ADR links — and refuses `blocked` with reason `record-invalid` (`halted`) when any carries a validation error; the result's `findings` name each bad record's code and path. No merge call was made: repair exactly the records `findings` name — never guess, never edit an unnamed record; no override exists — then re-run finalize. Because this check precedes already-merged recovery, a PR merged outside docket whose scope holds a defective record reports `record-invalid`, not `already-merged`; that is expected (closeout would refuse the same defect), and once repaired the re-run takes the merged-recovery path. It merges at the exact expected head, never requests a branch delete, and verifies the merge authoritatively: a reprobe returns the exact `mergedAt`/merge-commit facts and a Git fetch proves the merge commit reachable from the destination tip. An open PR on reprobe is not merged; a different head/base is `contended`; an unobservable result is `unknown` — none permits closeout. An already-merged exact PR is a verified no-op regardless of who merged it, never a second merge. +The `finalize.merge` operation with `--id <id> --version <version> --head <head>`. It reloads fresh authority and rechecks every merge conjunct immediately before the effect — implemented, PR identity, heads agree, base is the effective base, gate satisfied, approval satisfied, no open children, not superseded — refusing with that conjunct's closed token (`not-mergeable`, `pr-not-open`, `unresolved-base`, a child conjunct, …) and issuing **no** merge call when any fails. It then selects the best merge method the repository settings and the base branch's active rules permit, in the fixed order rebase → merge commit → squash, and attempts exactly that one; the document's `method` field reports it (absent on already-merged recovery). A cleanly observed empty permitted set refuses `blocked` with reason `merge-method-unavailable` before any merge — fix the repository or branch-rule merge settings; it is not `merge-denied` and is never retried with another method. Before any GitHub call it also validates the change and the records it structurally requires — its `depends_on` targets and stack ancestors (and any other record carrying its id), never `related`, `discovered_from`, or ADR links — and refuses `invalid-state`/`blocked` with reason `record-invalid` (the run is `halted`) when any carries a validation error; the result's `findings` name each bad record's code and path. No merge call was made: repair exactly the records `findings` name — never guess, never edit an unnamed record; no override exists — then re-run finalize. Because this check precedes already-merged recovery, a PR merged outside docket whose scope holds a defective record reports `record-invalid`, not `already-merged`; that is expected (closeout would refuse the same defect), and once repaired the re-run takes the merged-recovery path. It merges at the exact expected head, never requests a branch delete, and verifies the merge authoritatively: a reprobe returns the exact `mergedAt`/merge-commit facts and a Git fetch proves the merge commit reachable from the destination tip. An open PR on reprobe is not merged; a different head/base is `contended`; an unobservable result is `unknown` — none permits closeout. An already-merged exact PR is a verified no-op regardless of who merged it, never a second merge. `--admin` is honored **only** on an attended, explicitly-named run where a sole maintainer forces past an otherwise-unsatisfiable required review; it is never inferred from an approval absence or a permission error, and a `merge-denied` stays `denied` (`halted`). A named id overrides the `approval-required` and `finalize-blocked` skips (step 1); it never overrides malformed state, a wrong PR identity, an unsafe stack, or the repair sign-off. From 56a7743fbfe83f655f6491de681d1b4f6aad5317 Mon Sep 17 00:00:00 2001 From: Daniel Hanold <danny@danielhanold.com> Date: Sun, 27 Sep 2026 06:28:12 -0400 Subject: [PATCH 5/5] docs(results): change 0455 results --- ...valid-reason-in-the-docket-fina-results.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 docs/results/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina-results.md diff --git a/docs/results/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina-results.md b/docs/results/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina-results.md new file mode 100644 index 000000000..ba564893a --- /dev/null +++ b/docs/results/2026-09-27-document-finalize-s-record-invalid-reason-in-the-docket-fina-results.md @@ -0,0 +1,28 @@ +<!-- docket:backlink:start (generated — do not hand-edit) --> +> ↩ **[Change 0455 — Document finalize's record-invalid reason in the docket-finalize-change skill](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0455-document-finalize-s-record-invalid-reason-in-the-docket-fina.md)** +<!-- docket:backlink:end --> +# Document finalize's record-invalid reason in the docket-finalize-change skill — Results + +**Human action:** None required beyond reading the two changed skill paragraphs in the PR diff. This is a documentation-only change. + +## Outcome + +Change 0449 made two operations that touch GitHub, `finalize.merge` and `pr.publish`, refuse with the reason `record-invalid` when the change, or a record it structurally depends on, fails validation. Neither skill mentioned this, so an agent that hit the refusal had no documented fix. + +Both skills now describe the refusal: + +- `docket-finalize-change` step 8 says the merge refuses with `invalid-state`/`blocked` and reason `record-invalid` before any merge call. The check covers the change, its `depends_on` targets, its stack ancestors, and any other record carrying its id. It never follows `related`, `discovered_from`, or ADR links. The fix is to repair exactly the records the result's `findings` name and then re-run. The step also explains that a PR merged outside docket, with a broken record in scope, now reports `record-invalid` rather than `already-merged`, and that this is expected. +- `docket-implement-next` ("Publish the PR") says `pr.publish` refuses `record-invalid` (`invalid-state`) under the same scope before any GitHub call. The fix is the same: repair the named records and re-publish. + +The skill word budgets in `internal/repoguard/budgets_test.go` were raised to the exact new word counts, and the embedded asset bundle was regenerated. + +One planned item was dropped. The spec also asked for a `gofmt` fix to `internal/githubcli/comment_integration_test.go`. That file is already formatted for the toolchain pinned in `go.mod` (commit 21f851142, change 0436). Only the older `gofmt` on PATH flags it, and reformatting with that one would undo 21f851142. No change was made. + +`skills/docket-finalize-change/references/gate-failure.md` was intentionally left unchanged. It covers merge refusals in general and names no individual refusal token. + +## Verification performed + +- The full suite (`go run ./cmd/docket development test`) passed through the build gate. The final certification head is recorded in the PR's build-evidence block. +- Focused `repoguard` and `assets` tests passed after each skill edit, and `go run ./cmd/genassets -check` matched. The word-budget guard was mutation-tested: setting the ceiling one below the real count made it fail. +- A lean whole-branch review compared the prose against `finalize_merge.go`, `pr_publish.go`, and `subject_scope.go` and confirmed it is accurate. It raised two minor wording findings. Both are fixed in commit 2f77444d7: finalize now states the `invalid-state` result, and the scope now mentions records that carry the same id. +- Only automated guards check that the phrases are present. Whether the wording is accurate was checked by review.