Skip to content

Plan lifecycle: closed-plan step repair and edit regression guard #40

Description

@DocksDocks

Goal

Give the shipped plan.mjs two narrow safety fixes observed failing in real runs: (1) a step-repair path for a closed-but-unarchived plan that preserves the original closing event, and (2) an edit guard that refuses a body write which would regress a Steps Status cell from terminal to non-terminal.

Mode: plan-and-implement

Research

  • Real failure 1 (plan Skill kit refresh: fix audit findings, add routing, remove okf-bundle #35): the issue closed via its PR merge while step hash_gate was still in-flight (a step-mark command aborted earlier in a && chain). step requires status ongoing (plan.mjs:905) and archive requires every step terminal (plan.mjs:1033-1034), a deadlock. The session recovered by reopen + hand-close + a second closing PR, which is exactly the destructive path: archivePullRequestReferences (plan.mjs:384-388) deliberately trusts only the LATEST closure event, so the reopen destroyed the PR-closure proof. A session advisory settled the design: never accept a displaced earlier closure; the latest-closure rule is load-bearing (an old merged PR must not authorize a later manual close). The repair must therefore mutate the step cell on the closed issue without touching GitHub state.
  • Real failure 2 (plan Reword security/refactor plan-record label blocks to creation state #39): edit re-applied a locally stale draft over a fresh export. The provenance guard (plan.mjs:833-842) compares the sidecar digest against the REMOTE body, so an intervening export refreshes the sidecar and a stale FILE passes, silently reverting both Steps Status cells from done to planned. A pre-merge advisory caught the would-be repeat of failure 1.
  • Fix 1 shape: relax plan.mjs:905 to also accept record.status === 'finished' when the TARGET status is terminal (done or skipped); every other status keeps the ongoing requirement. Body edits on closed GitHub issues are permitted by GitHub and do not create closure events. Dependency and transition legality checks (plan.mjs:911-915) stay as-is.
  • Fix 2 shape: in editPlan after checkPlan (plan.mjs:843), parse the Steps tables of issue.body and after (parseRows with the same 8-cell contract); for every step id present in both, fail when the current cell is done or skipped and the incoming cell differs and is non-terminal, naming the step id and instructing re-export. Renames/additions/removals of rows stay legal.
  • Test surfaces: scripts/tests/plan-cli.mjs drives the CLI against scripts/tests/fixtures/gh-stub/gh and asserts exit codes and messages; scripts/tests/plan-skill-phases.mjs pins contract wording across skills/references/template; plugins/plan-lifecycle/test/selftest.mjs checks manifest/contract coherence. New behavior needs plan-cli coverage for both fixes (allowed and refused paths).
  • Contract text: plan-manager/references/plan-contract.md (Issue writes section) and the step and edit rows of the CLI table in plan-manager/SKILL.md describe the current ongoing-only rule; both must state the narrow finished-plan repair and the regression refusal. docs/PLAN.md and the workspace template stay byte-identical to the contract's canonical sections per the phase tests - touch only if those sections quote the changed sentences.
  • plan-lifecycle is self-versioned; a release/version bump is a separate action outside this plan.

Steps

# Id Task Files Depends Effect Status Done when
1 step_repair Accept step <issue> <id> <done or skipped> on a finished plan; all other statuses keep requiring ongoing; never touch GitHub state or reopen plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs - local done Repairing a terminal step on a closed COMPLETED issue succeeds and archive then verifies against the ORIGINAL closing PR; non-terminal targets on finished plans still fail
2 edit_guard Refuse edit bodies that regress any matching step id from done or skipped to a different non-terminal status, naming the step and instructing re-export plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs - local done A stale draft reverting a done step fails loudly; legitimate row additions, removals, and renames still pass
3 tests Cover both fixes in the CLI suite: finished-plan terminal repair passes, finished-plan non-terminal mutation fails, regression edit fails, non-regressing edit passes scripts/tests/plan-cli.mjs, scripts/tests/fixtures/gh-stub/gh 1,2 local done node scripts/tests/plan-cli.mjs exit 0 with the new assertions
4 contract_docs State the finished-plan step repair and the edit regression refusal in the contract and the CLI table; sync any mirrored sentence plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md, plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md, docs/PLAN.md, plugins/plan-lifecycle/skills/productivity/plan-workspace/references/plan-md-template.md 1,2 local done node scripts/tests/plan-skill-phases.mjs exit 0; contract and CLI table name both behaviors
5 gate Re-sync touched skill hashes and run the plugin gate plugins/plan-lifecycle/ 1,2,3,4 local done node scripts/ci.mjs --plugin plan-lifecycle exit 0

Acceptance

ID Command Expected
A1 node scripts/tests/plan-cli.mjs Exit 0 including finished-plan repair and regression-guard assertions
A2 node scripts/tests/plan-skill-phases.mjs Exit 0
A3 node plugins/plan-lifecycle/test/selftest.mjs Exit 0
A4 node scripts/ci.mjs --plugin plan-lifecycle Exit 0

Do not touch

  • archivePullRequestReferences latest-closure semantics (plan.mjs:349-393) - the displaced-closure rejection is load-bearing and stays.
  • docs/plans/finished/ - frozen history.
  • Manifest versions and release tooling - release is a separate action.

Open questions

None

Review

No review yet.

Verification Results

  • A1 node scripts/tests/plan-cli.mjs - exit 0 with four new assertions: closed-COMPLETED terminal step repair (body edit only, no reopen/close), non-terminal target refusal (plan status is finished; expected ongoing), terminal-regression edit refusal with no body write, and a terminal-preserving edit that still passes.
  • A2 node scripts/tests/plan-skill-phases.mjs - exit 0 with contract, SKILL.md CLI table, docs/PLAN.md, and workspace template synchronized on both behaviors.
  • A3 node scripts/ci.mjs --plugin plan-lifecycle - exit 0 (content-hash backfill applied to plan-manager and plan-workspace).
  • Plugin selftest exit 0.

Metadata

Metadata

Assignees

Labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions