feat(app): revise a groomed change's spec and owned sections through change.groom (change 0445) - #332
Merged
danielhanold merged 14 commits intoSep 25, 2026
Conversation
danielhanold
added a commit
that referenced
this pull request
Sep 25, 2026
… groom (change 0445) Human review of PR #332: spec_path could only ever repeat the record's spec: value (Plan refused anything else), and a wrong value surfaced as a generic duplicate-expectation error. The linked spec path now comes from the record. A spec-body revise still requires spec_version, but it is checked in Plan against the blob at the record's linked spec path on the attempt's base tree (the engine checks expectations before the record is read; Plan runs on the same fetched base, so the check is equally exact). A stale spec_version refuses with spec-version-mismatch, mapped onto contended like the engine's own pin mismatch. spec_version on any other request is refused (invalid-spec_version) rather than silently ignored. empty-spec_path and spec-path-mismatch are gone.
Docket-Plan-Path: docs/superpowers/plans/2026-09-24-0445-revise-groomed-change-typed-operation.md
…lary (change 0445)
…text (change 0445)
…ts (change 0445) go generate ./internal/assets/ to refresh the embedded manifest and the docket-groom-next / docket-new-change SKILL.md copies that 0b88f3a left stale, restoring TestEmbeddedMatchesAuthored and TestDevelopmentInstallFreshRenderHandoff.
…ange 0445) Review blocker: changeGroomOp.Plan unconditionally declared the change record (and, on a spec-body revise, the linked spec) as MutationReplace. A spec-only revise over a record whose updated: already equals today and whose docket:artifacts block is already rendered produced record bytes identical to the source, and an identical spec body or identical section text did the same for the spec/record. The engine's verifyActualDelta rejects a declared path that is not an actual change, so these revises ended in a failed disposition. Declare the record only when finalBytes differ from the source, and the spec replace only when the assembled bytes differ from the existing spec blob (read once via the new treeBlob helper). When nothing differs the plan is empty, which the engine already treats as its clean no-op path - the same skip includeBoard makes for the board (change 0335). Regression tests: a spec-only revise over a settled record declares only the spec; identical spec+section, spec-only, and section-only revises plan zero files. Mutation-tested: defeating either bytes.Equal guard reddens the new tests.
…ise (change 0445) Review finding (important): ChangeGroom pinned only the change record, so a revise carrying spec_markdown whole-body-replaced the spec file unpinned. Two revises pinned to the same record version (a same-day spec-only revise leaves the record bytes unchanged) both applied and the second clobbered the first; a concurrent spec edit was silently lost too. The request gains spec_path + spec_version (the linked spec's blob id). They travel as a pair on any outcome and are required on a revise carrying spec_markdown (empty-spec_path / empty-spec_version shape refusals, registered in the finding-code vocabulary). A supplied pin becomes a second exact-blob entity expectation, so drift yields the engine's standard contended outcome, and Plan refuses spec-path-mismatch when the pin names anything but the change's linked spec. docket-groom-next's revise exit documents reading the spec's blob id; embedded assets regenerated.
… block (change 0445) Review finding (important): a spec or revise spec_markdown that still held the docket:backlink block passed shape validation, and assembleSpecFile prepended a second block, committing duplicate marker pairs. Shape validation now refuses it with invalid-spec_markdown for both outcomes (refusal, never silent stripping). docket-groom-next's revise exit states spec_markdown excludes the backlink block; embedded assets regenerated.
…sal/receipt coverage (change 0445) Review minors 4-6: - Minor 4: docket-groom-next Step 5 now adapts the contended-retry and board clauses for the revise exit: a revise re-reads the spec too (a fresh spec_version) and stops only if the change is no longer an already-groomed proposed change; a revise keeps the row build-ready. - Minor 5: the skill's frontmatter description names the revise route (revising an already-groomed proposed change by explicit id). - Minor 6: TestChangeGroomPlanReviseRefusals adds not-revisable rows for in-progress, deferred, implemented, done, and killed (spec acceptance item 7); the spec-only and sections-only revise tests decode the plan receipt and pin spec_path to the existing linked path / empty. Mutation-checked in both directions. Word ceiling for docket-groom-next/SKILL.md raised 1850 -> 1889 with the house annotation; embedded assets regenerated.
… groom (change 0445) Human review of PR #332: spec_path could only ever repeat the record's spec: value (Plan refused anything else), and a wrong value surfaced as a generic duplicate-expectation error. The linked spec path now comes from the record. A spec-body revise still requires spec_version, but it is checked in Plan against the blob at the record's linked spec path on the attempt's base tree (the engine checks expectations before the record is read; Plan runs on the same fetched base, so the check is equally exact). A stale spec_version refuses with spec-version-mismatch, mapped onto contended like the engine's own pin mismatch. spec_version on any other request is refused (invalid-spec_version) rather than silently ignored. empty-spec_path and spec-path-mismatch are gone.
danielhanold
force-pushed
the
feat/revise-a-groomed-change-s-spec-and-owned-sections-through-a
branch
from
September 25, 2026 06:43
5bf6014 to
c7cbf91
Compare
danielhanold
deleted the
feat/revise-a-groomed-change-s-spec-and-owned-sections-through-a
branch
September 25, 2026 06:43
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
reviseoutcome tochange.groom. You can now adjust an already-groomedproposedchange (its linked spec body, its owned proposal sections, or both) in one transaction pinned to the exact version. Before this, the only route was a hand edit plus a plain git commit.What changed
change.groomacceptsoutcome: revise, but only for aproposedchange that already has a spec or a trivial verdict.spec_markdownreplaces the whole body of the existing spec file, keeping its path and backlink.sectionsedits the owned proposal sections. It never writesspec:ortrivial:.not-revisable,spec-not-linked,spec-file-missing,empty-revise,empty-spec_version, andinvalid-spec_version. A spec-body revise must sendspec_version, the blob id of the spec the record'sspec:links; the plan step checks it, and a stale one returnscontended(spec-version-mismatch). A revise that would change nothing returnsno-op.spec_markdownthat contains adocket:backlinkblock is refused (invalid-spec_markdown) on bothspecandrevise.outcomeand printschange NNNN revised — ….docket-groom-nextsends an explicit id for an already-groomed change to a revise flow. This replaces the "clearspec:by hand" workaround.docket-new-changepoints to that flow. The skill word budgets were raised to fit, and the embedded assets were regenerated.Review (docket-review-deep)
spec_markdowncarrying a backlink block produced a duplicate backlinkspec_pathassertionBuild notes: the first full-suite gate failed on stale embedded skill assets. They were regenerated in 1668bd9, and the gate then passed. The final full suite passed on the head below: 54/54 files.
Results:
docs/results/2026-09-24-revise-a-groomed-change-s-spec-and-owned-sections-through-a-results.mdcommand: go run ./cmd/docket development test
result: green
head_sha: c7cbf91
ran_at: 2026-09-25T06:42:56Z
Post-review revision (5bf6014): dropped the
spec_pathrequest field. It could only ever repeat the record'sspec:value, so the spec path now comes from the record and onlyspec_versionis sent.