diff --git a/.ratchet/changes/issue-scope-reconciliation/.ratchet.yaml b/.ratchet/changes/issue-scope-reconciliation/.ratchet.yaml new file mode 100644 index 0000000..e254ad5 --- /dev/null +++ b/.ratchet/changes/issue-scope-reconciliation/.ratchet.yaml @@ -0,0 +1,10 @@ +schema: ratchet +created: 2026-08-26 +standards: + - security-remediation + - documentation + - testing + - multi-agent-support + - generalizable-defaults + - instruction-fed-config + - delegated-lifecycle diff --git a/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/close-claim-honesty.feature b/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/close-claim-honesty.feature new file mode 100644 index 0000000..a29d3f2 --- /dev/null +++ b/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/close-claim-honesty.feature @@ -0,0 +1,28 @@ +Feature: Honest close-claims in batch manifests and change authoring + As a maintainer reading a manifest or a plan + I want a "Closes #N" claim to be earned rather than assumed + So that GitHub never auto-closes an issue whose material requirements are still unimplemented + + Scenario: Propose-batch forbids hard-coding a close-claim for unscoped work + Given the propose-batch workflow body + When an author reads its manifest-authoring rules + Then it forbids writing "Closes #N" or "Fixes #N" into a phase goal, a phase success criterion, or a change-level done for work that has not yet been scoped and verified + And it states that a close-claim is an output of verification and never an input of planning + + Scenario: Phase contracts reference issues without claiming closure + Given the propose-batch workflow body + When an author writes a phase contract that addresses an issue + Then the body requires the phrasing "targets #N" or "addresses #N" + And it states that the "Closes #N" linkage is earned at pull-request-authoring time only after the issue's material requirements are confirmed implemented + + Scenario: Partial coverage is stated as partial + Given the propose-batch workflow body + When a change-level done criterion covers only part of an issue + Then the body requires that done criterion to say "partially addresses #N" + And it forbids that done criterion from saying "Fixes #N" or "Closes #N" + + Scenario: The close-claim rule is shared by all three change-authoring workflows + Given the propose, propose-batch, and decompose-phase workflow bodies + When each body is inspected for the close-claim rule + Then each contains the shared rule that a close-claim is permitted only when the issue's material requirements are actually implemented + And each contains the shared rule that partial work must use "partially addresses #N" diff --git a/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/deferral-carry-forward.feature b/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/deferral-carry-forward.feature new file mode 100644 index 0000000..f61f8f7 --- /dev/null +++ b/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/deferral-carry-forward.feature @@ -0,0 +1,36 @@ +Feature: Deferrals survive phase boundaries during decomposition + As a maintainer decomposing a later batch phase + I want deferred scope and unearned close-claims from prior phases to reach this decomposition + So that "revisit in the next phase" prose cannot evaporate at a phase boundary + + Scenario: Decomposition reads the prior phases' plans, not only their done criteria + Given the decompose-phase workflow body + When an author reads its grounding step + Then it requires reading each prior phase's shipped change plan file in addition to the injected done criteria + And it explains that the injected done criteria are a paraphrase in which deferrals recorded as plan prose are invisible + + Scenario: Every deferred item is extracted from the prior plans + Given the decompose-phase workflow body + When an author sweeps a prior phase's plan + Then it requires extracting every out-of-scope, deferred, revisit, or equivalent item recorded there + + Scenario: Each extracted deferral gets one of exactly three outcomes + Given the decompose-phase workflow body + And an item extracted from a prior phase's plan + When the author decides what to do with it + Then the body requires the item to be carried forward as a change intent in the phase being decomposed, or matched to an existing open tracking issue and reported as tracked, or surfaced to the user as an explicit drop decision + And it states that silently ignoring an extracted item is not an available outcome + + Scenario: A prior phase's close-claim is verified before the issue is treated as shipped + Given the decompose-phase workflow body + And a prior phase whose done criterion claims "Fixes #N" + When the author grounds this phase in that prior result + Then the body requires comparing the issue's material requirements against what the prior phase's done criterion and plan describe as implemented + And it requires surfacing an unearned close-claim and carrying the remaining scope forward + And it forbids inheriting the close-claim as fact + + Scenario: The decomposition rules reach every supported coding agent + Given the decompose-phase command template + When it is rendered through every registered tool command adapter + Then the rendered command file for every adapter contains the prior-plan deferral sweep + And the rendered command file for every adapter contains the earned-close verification diff --git a/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/issue-reconciliation.feature b/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/issue-reconciliation.feature new file mode 100644 index 0000000..83e1ef5 --- /dev/null +++ b/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/issue-reconciliation.feature @@ -0,0 +1,42 @@ +Feature: Originating-issue reconciliation in change authoring + As a maintainer whose work originates from tracked issues + I want the propose and propose-batch workflows to reconcile authored scope against every originating issue + So that a requirement the issue states can never be dropped without a human being asked + + Background: + Given the ratchet workflow templates are the single source of the propose and propose-batch skill bodies + And the generated skill and command artifacts for every supported coding agent render from those templates + + Scenario: Propose fetches every originating issue before authoring artifacts + Given the propose workflow body + When an author reads its steps + Then it instructs the author to identify every originating issue referenced by the user, the batch manifest, or the injected done criterion + And it instructs the author to fetch each originating issue through the project's issue tracker before writing any artifact + And it names "gh issue view " only as a GitHub example rather than a required command + And it instructs the author to ask the user to paste the issue text when no tracker client is available + + Scenario: Propose reconciles authored scope against the fetched issue + Given the propose workflow body + When an author reads its reconciliation step + Then it requires enumerating the issue's material requirements from both its explicit fix items and the problems named in its narrative + And it requires mapping every enumerated requirement to an authored feature scenario or plan task + And it requires listing every requirement that the authored scope does not cover + + Scenario: An omitted requirement is surfaced as an explicit decision point + Given the propose workflow body + When an author reads its rules for a requirement the authored scope omits + Then it requires surfacing the omission to the user as an enumerated "issue asks X, this proposal does not include X" decision point before artifacts are finalized + And it forbids self-approving the omission by writing it into plan prose + + Scenario: Propose-batch reconciles the manifest against the originating issues + Given the propose-batch workflow body + When an author reads its steps + Then it instructs the author to fetch every issue an objective or phase originates from + And it requires reconciling each phase goal, success criterion, and change-level done against those issues' material requirements + And it requires surfacing any material requirement the manifest omits to the user before the manifest is scaffolded + + Scenario: The reconciliation rules reach every supported coding agent + Given the propose and propose-batch command templates + When each is rendered through every registered tool command adapter + Then the rendered command file for every adapter contains the originating-issue reconciliation step + And no adapter's rendering drops the prohibition on self-approved omissions diff --git a/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/stop-and-surface-guardrail.feature b/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/stop-and-surface-guardrail.feature new file mode 100644 index 0000000..0afa8fa --- /dev/null +++ b/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/stop-and-surface-guardrail.feature @@ -0,0 +1,24 @@ +Feature: Stop-and-surface guardrail for security-relevant de-scopes + As a maintainer relying on agents to author changes unattended + I want any de-scope of security, permission, or integrity work to halt the run and ask + So that a security exposure can never be narrowed by momentum inside an agent run + + Scenario: The guardrail is defined exactly once + Given the workflow template layer + When the stop-and-surface guardrail is located + Then it is defined as a single shared constant that the propose, propose-batch, and decompose-phase bodies all embed + And no workflow body restates it as a hand-authored copy + + Scenario: All three change-authoring workflows carry the guardrail verbatim + Given the propose, propose-batch, and decompose-phase workflow bodies + When each body's guardrails section is inspected + Then each contains the shared guardrail text verbatim + And each states that a de-scope of security, permission, or integrity work is a stop-and-surface event in which the workflow halts and asks the user rather than proceeding on momentum + + Scenario: An approved security de-scope requires a filed, linked, owned tracking issue + Given a workflow body carrying the shared guardrail + And a user who approves deferring a security-relevant requirement + When the author records the deferral + Then the guardrail requires an explicitly filed tracking issue with a named owner + And it requires that issue to be linked from the change plan before the author proceeds + And it states that a prose bullet in a plan is not a deferral mechanism diff --git a/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/worked-example-check.feature b/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/worked-example-check.feature new file mode 100644 index 0000000..0de7f7e --- /dev/null +++ b/.ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/worked-example-check.feature @@ -0,0 +1,28 @@ +Feature: The reconciliation procedure is demonstrated on a worked example + As a maintainer who has seen a scope reduction escape review once already + I want a runnable check that replays the failure on the hardened workflows + So that the reconciliation rules are demonstrated to work rather than merely asserted + + Background: + Given an eval fixture holding the originating issue's text, the phase-one manifest excerpt that hard-codes the close-claim, and the phase-one change plan carrying the out-of-scope bullet + And an eval spec that binds this scenario to that fixture + + Scenario: Replaying the worked example flags all three escapes + Given the hardened propose and propose-batch reconciliation procedure + When the procedure is applied to the fixture's issue text and the fixture's manifest and plan + Then it flags the issue's gating requirement as present in the issue and absent from the authored scope + And it flags the permission-posture bypass named in the issue's narrative as absent from the authored scope + And it flags the manifest's hard-coded close-claim as premature because the issue's material requirements are not all implemented + And it reports each of the three as a decision point requiring the user rather than a self-approved omission + + Scenario: The hedged wording in the issue does not lower the bar + Given the fixture's issue text phrases its gating requirement as something to "consider" + When the reconciliation procedure enumerates that issue's material requirements + Then the hedged requirement is still enumerated as material because the exposure is security-relevant + And the procedure does not treat the hedge as permission to drop the requirement + + Scenario: The check is runnable and its verdict is recorded + Given the eval spec bound to this scenario + When the eval is run for this case + Then the run produces a recorded verdict for the case + And the verdict is available as evidence that the worked example was actually replayed diff --git a/.ratchet/changes/issue-scope-reconciliation/plan.md b/.ratchet/changes/issue-scope-reconciliation/plan.md new file mode 100644 index 0000000..319be0e --- /dev/null +++ b/.ratchet/changes/issue-scope-reconciliation/plan.md @@ -0,0 +1,330 @@ +# issue-scope-reconciliation + +## Why + +The change-authoring workflows (`propose`, `propose-batch`, `decompose-phase`) have no step +that reconciles authored scope against the originating GitHub issue, no rule governing +`Closes #N` claims, and no mechanism that carries a deferral across a phase boundary — so +issue #80 lost its security-relevant half to a self-approved plan-prose de-scope, was claimed +`Closes`d anyway, and the next phase never saw the deferral. This change hardens all three +workflows so that failure chain is structurally impossible, and closes out the #80 remainder +that the chain left untracked. + +## What Changes + +- **New shared fragment module** `src/core/templates/workflows/scope-reconciliation.ts` + exporting three constants that all three workflow bodies embed, so the rules are authored + once and cannot drift between workflows: + - `ISSUE_RECONCILIATION_STEP` — fetch every originating issue, enumerate its material + requirements, map each to an authored feature/task, surface every gap as an explicit + decision point. + - `CLOSE_CLAIM_RULES` — a `Fixes #N` / `Closes #N` claim is permitted only when the issue's + material requirements are actually implemented; partial work says "partially addresses #N". + - `STOP_AND_SURFACE_GUARDRAIL` — a de-scope of security-, permission-, or integrity-relevant + work halts the workflow and asks the user; an approved deferral requires a filed, linked + tracking issue with a named owner. +- **`src/core/templates/workflows/propose.ts`**: its two verbatim-duplicated bodies (skill + `instructions` and command `content`) collapse into one shared body builder, and that body + gains the originating-issue reconciliation step plus the close-claim and stop-and-surface + guardrails. Implements `features/scope-reconciliation/issue-reconciliation.feature`, + `close-claim-honesty.feature`, `stop-and-surface-guardrail.feature`. +- **`src/core/templates/workflows/propose-batch.ts`**: gains the reconciliation step, a + prohibition on hard-coding `Closes #N` in a phase `goal`/`success` or a change-level `done`, + the required "targets #N" / "addresses #N" / "partially addresses #N" phrasing, and the + shared guardrails. Implements `issue-reconciliation.feature`, `close-claim-honesty.feature`, + `stop-and-surface-guardrail.feature`. +- **`src/core/templates/workflows/decompose-phase.ts`**: gains a prior-phase plan sweep + (read each prior phase's `plan.md`, extract every out-of-scope / deferred / revisit item, + and resolve each as carried-forward | tracked | explicitly-dropped), an earned-close + verification before treating an issue as shipped, and the shared guardrails. Implements + `deferral-carry-forward.feature`, `close-claim-honesty.feature`, + `stop-and-surface-guardrail.feature`. +- **Tests**: new `test/core/templates/workflows/{scope-reconciliation,propose,decompose-phase}.test.ts`, + extensions to `test/core/templates/workflows/propose-batch.test.ts`, and an `init`-level + assertion that the hardened text lands in **both** `.claude/skills/…/SKILL.md` and + `.opencode/skills/…/SKILL.md`. +- **Worked-example check**: a new eval fixture (`.ratchet/evals/fixtures/issue-scope-reconciliation/`) + holding issue #80's text, the phase-one manifest excerpt, and the phase-one plan excerpt, plus an + `llm-judge` eval spec binding `features/scope-reconciliation/worked-example-check.feature`. + Implements `worked-example-check.feature`. +- **Docs**: `docs/configuration/generated-artifacts.md` and `README.md` describe the new + reconciliation step, the close-claim rules, and the deferral carry-forward trichotomy. +- **Issue-#80 remainder remediation** (not a code behavior): a tracking issue with a named owner + for the ungated override seam, the voided permission posture under override, and `batch + config`'s override-unaware enforcement display; and PR #97's body corrected from `Closes #80` + to a partial-address claim linking that tracking issue. +- No **BREAKING** changes: no CLI surface, flag, config key, or manifest schema changes. The + generated skill/command prose grows; regenerating via `ratchet init` / `ratchet update` picks + it up. + +## Design + +**Templates are the single source of truth; both trees come for free.** `.claude/` and +`.opencode/` are gitignored in this repo — they are *generated* into a consuming project by +`ratchet init`/`update` from `getSkillTemplates()` / `getCommandTemplates()` in +`src/core/shared/skill-generation.ts`, which read the workflow template modules. Editing the +three modules under `src/core/templates/workflows/` is therefore the only edit that satisfies +the issue's "both trees **and** the generator" requirement; hand-editing a generated tree would +be reverted on the next regeneration and is explicitly not what this change does. This is also +what `delegated-lifecycle` requires: exactly one author of lifecycle instruction text. + +**Why a shared fragment module rather than three hand-written copies.** The cross-cutting +requirement is that the stop-and-surface guardrail and the close-claim rule appear in all three +workflows *verbatim*. Three prose copies are exactly the drift mechanism this issue is about, so +the rules live in one module and each body interpolates the constant. That also makes the +"verbatim in all three" requirement a mechanical assertion — `expect(body).toContain(CONSTANT)` +— instead of a prose review. `multi-agent-support` requires shared content be defined once and +rendered per agent through the adapter registry; the fragment module sits inside that shared +layer, so every agent's rendering carries the identical rules. + +**Why `propose.ts` is deduplicated first.** Today `propose.ts` carries the same body twice — +once as the skill's `instructions`, once as the command's `content` — differing in exactly two +lines. Adding the reconciliation rules to only one of them would reproduce, inside this very +change, the divergence the issue is about. The body becomes a single builder parameterized by +the two known deltas: +1. the **Input** line — skill: "The user's request should include a change name (kebab-case) OR + a description of what they want to build."; command: "The argument after `/rct:propose` is + the change name (kebab-case), OR a description of what the user wants to build." +2. the closing **Prompt** line — skill: "Run `/rct:apply` or ask me to implement to start + working on the tasks."; command: "Run `/rct:apply` to start implementing." +Everything else must stay byte-identical to today's text apart from the new sections. This +mirrors the shape `propose-batch.ts` and `decompose-phase.ts` already use (one body constant +shared by skill and command). + +**Ecosystem-agnostic issue fetching (`generalizable-defaults`).** These skills ship into +arbitrary user repositories, so the reconciliation step must not hard-require GitHub or the +`gh` binary. The prose instructs the author to fetch each originating issue *through the +project's issue tracker*, names `gh issue view ` only as a GitHub example, and requires +asking the user to paste the issue text when no tracker client is available. No default command +string is baked in. + +**No config reads in skill prose (`instruction-fed-config`).** The reconciliation step consumes +the issue reference the user / manifest / injected `done` already provides plus the standards +array the `ratchet instructions` payload already delivers. It adds no config-file read and no +"if config says X" branching to any template body, and it introduces no new schema. + +**`security-remediation` is the policy this change operationalizes.** That standard already +states the rules (no partial remediation of severe exposures, severity governs over hedged +source wording, honest close-claims, no silent scope drops, no lying security controls). It had +no enforcement point in the authoring workflows — this change adds it: the reconciliation step +is where "enumerate the exposure's material requirements before writing tasks or close-claims" +actually happens, and the stop-and-surface guardrail is where "a de-scope is a stop-and-surface +event" actually halts a run. The hedged-wording rule is carried explicitly, because issue #80's +own "**Consider** requiring…" is what handed the original propose agent its de-scope +justification. + +**Why the worked-example check is an `llm-judge` eval, not a unit test.** The behavior being +demonstrated is an agent applying a procedure to prose — there is no pure function to assert +over, and a `deterministic` binding could only re-grep the skill text, which the vitest suite +already does. The repo's existing precedent for judging agent behavior over a fixture is +`.ratchet/evals/specs/eval-self.yaml` (`kind: llm-judge`, spawned agent, self-contained +fixture); this change follows it. Deterministic coverage of criteria 1–5 stays in vitest, so CI +never depends on a live agent spawn; the eval is the *demonstration* required by acceptance +criterion 6 and must be run once with its verdict recorded as evidence. Known limitation, not +solved here: eval case ids embed the feature file's location (issue #55), so this binding will +need re-keying when the change is archived. + +**Trade-off accepted.** The three skill bodies get materially longer. That is the intended cost: +the alternative — a short body plus a reviewer expected to remember the rule — is what failed on +#80. Length is bounded by putting the shared rules in one fragment each body interpolates once. + +## Tasks + +- [x] 1.1 Create `src/core/templates/workflows/scope-reconciliation.ts` exporting + `ISSUE_RECONCILIATION_STEP`, `CLOSE_CLAIM_RULES`, and `STOP_AND_SURFACE_GUARDRAIL` as + string constants, with a module docstring stating they are the single author of these + rules for all three change-authoring workflows. Keep the prose agent-neutral ("your + agent", optional `AskUserQuestion` with a plain-prose fallback) per `multi-agent-support`, + and tracker-neutral (`gh issue view ` named only as a GitHub example, with a + paste-the-issue-text fallback) per `generalizable-defaults`. +- [x] 1.2 `ISSUE_RECONCILIATION_STEP` must require: identifying every originating issue + (referenced by the user, the manifest, or the injected `done`); fetching each one; + enumerating its material requirements from **both** its explicit fix items **and** the + problems named in its narrative/Why; mapping each requirement to an authored feature + scenario or plan task; and listing every requirement the authored scope does not cover. + It must state that hedged source wording ("consider", "maybe", "optionally") does not + lower the bar for a security-relevant requirement, and that every uncovered requirement + is surfaced to the user as an enumerated "issue asks X, this proposal does not include X" + decision point before artifacts are finalized — never self-approved in plan prose. +- [x] 1.3 `CLOSE_CLAIM_RULES` must state that a `Fixes #N` / `Closes #N` claim — in a manifest + `done`, a plan, or a PR body — is permitted only when the issue's material requirements + are actually implemented; that partial work MUST use "partially addresses #N"; and that a + close-claim is an output of verification, never an input of planning. +- [x] 1.4 `STOP_AND_SURFACE_GUARDRAIL` must state that any de-scope of security-, permission-, + or integrity-relevant work is a stop-and-surface event — the workflow halts and asks the + user, never proceeding on momentum — and that an approved deferral requires an explicitly + filed tracking issue with a named owner, linked from the plan, before proceeding; a prose + bullet in `plan.md` is not a deferral mechanism. +- [x] 2.1 Refactor `src/core/templates/workflows/propose.ts` so the skill `instructions` and the + command `content` derive from ONE shared body builder parameterized by exactly the two + known deltas (the **Input** line and the closing **Prompt** line, both quoted verbatim in + this plan's Design section). Assert by inspection that no other text changed relative to + the current file apart from the new sections added in 2.2. +- [x] 2.2 Add to the propose body: the `ISSUE_RECONCILIATION_STEP` as an explicit numbered step + that runs BEFORE artifacts are authored, and `CLOSE_CLAIM_RULES` + + `STOP_AND_SURFACE_GUARDRAIL` embedded in its **Guardrails** section. +- [x] 3.1 Add to `src/core/templates/workflows/propose-batch.ts`: the + `ISSUE_RECONCILIATION_STEP` as an explicit step that runs before the manifest is + scaffolded, reconciling each phase `goal`/`success` and each change-level `done` against + the originating issues' material requirements. +- [x] 3.2 Add to `propose-batch.ts` the no-premature-close rule: the manifest MUST NOT hard-code + `Closes #N` / `Fixes #N` in a phase `goal`, a phase `success`, or a change-level `done` + for work not yet scoped and verified; phase contracts use "targets #N" / "addresses #N"; + a `done` covering only part of an issue says "partially addresses #N". Embed + `CLOSE_CLAIM_RULES` + `STOP_AND_SURFACE_GUARDRAIL` in its **Guardrails** section. +- [x] 3.3 Verify the new propose-batch text does not trip the existing assertions in + `test/core/templates/workflows/propose-batch.test.ts`: it must not introduce the literal + `/rct:propose ` (with trailing space) and must not introduce the phrase "per-change + success". +- [x] 4.1 Add to `src/core/templates/workflows/decompose-phase.ts` a grounding sub-step that + requires reading each prior phase's shipped change `plan.md` files in addition to the + injected `done` criteria, stating explicitly that the injected criteria are a paraphrase + in which plan-prose deferrals are invisible. +- [x] 4.2 Require extracting from those plans every `## Out of scope`, "deferred", "revisit", or + equivalent item, and resolving EACH extracted item as exactly one of: (a) carried forward + as a change intent in the phase being decomposed, (b) matched to an existing open tracking + issue and reported as tracked, or (c) surfaced to the user as an explicit drop decision. + State that silently ignoring an item is not an available outcome. +- [x] 4.3 Require earned-close verification: before treating an issue as shipped, compare the + issue's material requirements against what the prior phase's `done` and `plan.md` describe + as implemented; surface an unearned `Fixes/Closes #N` and carry the remaining scope + forward rather than inheriting the claim as fact. Embed `CLOSE_CLAIM_RULES` + + `STOP_AND_SURFACE_GUARDRAIL` in its **Guardrails** section. +- [x] 5.1 Add `test/core/templates/workflows/scope-reconciliation.test.ts`: assert each of the + three constants contains its required rules, and assert all three workflow bodies + (`getRctProposeSkillTemplate`, `getProposeBatchSkillTemplate`, + `getDecomposePhaseSkillTemplate`) `toContain` `STOP_AND_SURFACE_GUARDRAIL` and + `CLOSE_CLAIM_RULES` **verbatim** (acceptance criterion 5). +- [x] 5.2 Add `test/core/templates/workflows/propose.test.ts`: assert the skill and command + bodies are identical except for the two known deltas; assert the reconciliation step and + the no-self-approved-omission rule are present (criterion 1); assert the body stays + agent-neutral and tracker-neutral; and render the command through every adapter in + `CommandAdapterRegistry.getAll()` asserting the reconciliation text survives each + rendering (mirroring the existing propose-batch adapter test). +- [x] 5.3 Extend `test/core/templates/workflows/propose-batch.test.ts` with the reconciliation + step (criterion 1) and the no-premature-`Closes` / "targets #N" / "partially addresses #N" + rules (criterion 2), including an adapter-render assertion. +- [x] 5.4 Add `test/core/templates/workflows/decompose-phase.test.ts` covering the prior-plan + sweep and the carry-forward/tracked/explicit-drop trichotomy (criterion 3), the + earned-close verification (criterion 4), the shared guardrail (criterion 5), the + skill/command shared-body identity, and an adapter-render assertion. +- [x] 5.5 Extend `test/core/init.test.ts` (or add a sibling test using the same fixture pattern) + asserting that after `init`, the reconciliation step and the stop-and-surface guardrail + are present in BOTH `.claude/skills/ratchet-propose/SKILL.md` and + `.opencode/skills/ratchet-propose/SKILL.md`, and likewise for `ratchet-propose-batch` and + `ratchet-decompose-phase` — the acceptance criteria's "both trees" clause, proven through + the generator rather than by hand-editing a tree. +- [x] 6.1 Create the eval fixture `.ratchet/evals/fixtures/issue-scope-reconciliation/` as a + self-contained ratchet project containing: `issue-80.md` (issue #80's text, including its + hedged "Consider requiring an explicit opt-in pairing flag…" item and the permission-posture + problem named in its Why), `manifest-excerpt.yaml` (the phase-one contract that hard-codes + `Closes #80` plus the `gate-and-mark-agent-cmd-override` change `done` ending "Fixes #80."), + and `phase-1-plan.md` (the "Out of scope (kept thin per the vertical-slice strategy)…" + bullet). Fixture content is quoted from issue #100's worked example — do not invent + variants. +- [x] 6.2 Add `.ratchet/evals/specs/issue-scope-reconciliation.yaml` binding the + `worked-example-check` scenario "Replaying the worked example flags all three escapes" to + that fixture with `kind: llm-judge`. Its `success` prose must require the spawned judge to + PASS only if the reconciliation procedure flags ALL THREE: (a) the unimplemented gate, + (b) the unaddressed permission-posture bypass, (c) the premature `Closes #80` — failing + closed if any is missing. Confirm the bound case id against the ids the eval CLI actually + derives (format: `#`) rather than assuming it. +- [ ] 6.3 Run the eval once for that case (e.g. `ratchet eval run --only `) and record + the resulting run id and verdict in the change's session evidence. A headless agent spawn + needs its auth env present (for the default Claude agent, `CLAUDE_CODE_OAUTH_TOKEN`); if + the spawn cannot be authenticated in this environment, say so explicitly in the completion + report rather than marking this task done. + **Blocked — recorded evidence (independent verification pass).** The eval ran + twice against this binding: run `20260826T221610877Z-22f8f4` and run + `20260826T222224814Z-db1005`, both recording `fail` for + `…/worked-example-check#replaying-the-worked-example-flags-all-three-escapes`. + Root cause is a PRE-EXISTING defect in the judge harness, not in this change: + `extractVerdictJson` (`src/core/eval/judge.ts`, introduced in `0cb74c8`, present + on `main`) takes the LAST balanced top-level `[...]` in the agent's stdout. The + claude adapter spawns the judge with `--output-format stream-json` + (`src/core/batch/engine/agent.ts:190`), so the judge's real verdict array only + ever appears INSIDE a JSON string value (the assistant event's `text`, the result + event's `result`) — which `balancedBlocks` deliberately skips — while the result + envelope's own `permission_denials` array is picked up instead. This binding can + therefore never pass through the claude adapter regardless of how the judge + answers. A probe reproducing that stdout shape yields the recorded verdict's + evidence strings byte-for-byte. `git diff main...HEAD -- src/core/eval/ + src/core/batch/engine/` is empty, so this branch neither caused nor can fix it + here; the harness defect needs its own change. The task stays UNCHECKED. +- [x] 7.1 Documentation task (REQUIRED by the `documentation` standard — not optional): update + `docs/configuration/generated-artifacts.md` to describe the originating-issue + reconciliation step, the close-claim rules, and the deferral carry-forward trichotomy as + part of the generated propose / propose-batch / decompose-phase artifacts. +- [x] 7.2 Update `README.md` wherever it describes the propose / propose-batch / decompose-phase + workflows so the described behavior matches the hardened prose. +- [x] 8.1 File the tracking issue for issue #80's remainder (acceptance criterion 7): the + ungated `RATCHET_BATCH_AGENT_CMD` / `RATCHET_EVAL_AGENT_CMD` override seam, the voided + permission posture under an override (`buildAgentSpawnRequest`'s override branch returns a + bare `bash -c ` request with no adapter and no permission flags), and `batch + config` (`src/core/batch/config.ts`) reporting the posture as enforced while an override + voids it. Assign a named owner (`--assignee joctaTorres`) and link it to #80 and #100. + DONE: filed as **#112**, open, assigned to `joctaTorres`, linking both #80 and #100. Its + three material requirements were each verified against + `origin/erh/phase-1-spawn-env-plumbing` before filing: + `buildAgentSpawnRequest`'s override branch returns a bare `bash -c ` and never + calls `buildAdapterRequest()`, so the adapter's `resolvePermissionFlags(...)` block is + skipped entirely; `src/commands/batch/config.ts:118` prints `posture ` + with no override awareness. +- [ ] 8.2 Correct #80's close-claim in the open PR stack (acceptance criterion 7): edit PR #97's + body so `Closes #80` becomes "Partially addresses #80 — remainder tracked in #", + leaving `Closes #89` intact. + DROPPED BY EXPLICIT HUMAN DECISION — recorded here per the `security-remediation` + standard's requirement that a deferral carry recorded human sign-off, not just a linked + issue. The repository owner reviewed the proposed edit and declined it, stating they will + close PR #97 themselves rather than have it amended. This is a de-scope of a + security-relevant record-keeping item, so it was surfaced and halted rather than actioned + on momentum; the owner's decision is the approval. The security remainder itself remains + tracked with an owner in #112 and is linked from #80 independently of #97's body, so no + security scope is left untracked by this drop. Sign-off: repository owner + (`joctaTorres`), during the apply session for this change. +- [x] 9.1 Run the full test suite and the coverage gate; all tests pass and the enforced coverage + threshold is not lowered (`testing` standard). +- [x] 9.2 Run `ratchet validate issue-scope-reconciliation` and confirm the change validates. +- [x] 9.3 Reconcile this change against issue #100 itself before declaring it done: walk its + seven acceptance criteria and confirm each maps to a completed task. Any criterion that + cannot be completed is a stop-and-surface event — report it, do not silently drop it. + +## Reconciliation against issue #100 + +Performed per this change's own rules — the procedure it adds to `propose`, applied to the +issue that commissioned it. Every criterion is enumerated; nothing is dropped silently, and +nothing hedged is downgraded. + +| # | Criterion | Status | Where | +|---|---|---|---| +| 1 | Reconciliation step + no-self-approved-omission guardrail in propose & propose-batch, both trees | **Met** | `scope-reconciliation.ts:52-106` → `propose.ts:70-72`, `propose-batch.ts:110-125`; both trees proven by `test/core/init-scope-reconciliation.test.ts` | +| 2 | propose-batch forbids hard-coded `Closes #N`; requires "targets #N" / "partially addresses #N" | **Met** | `propose-batch.ts:127-140`, `:161-164` | +| 3 | decompose-phase reads prior `plan.md`; carry-forward / tracked / explicit-drop trichotomy | **Met** | `decompose-phase.ts:63-94` | +| 4 | decompose-phase verifies a prior `Fixes/Closes #N` was earned | **Met** | `decompose-phase.ts:95-110` | +| 5 | Stop-and-surface guardrail in all three skills, both trees | **Met** | `scope-reconciliation.ts:125-137`; verbatim-containment asserted on all three bodies | +| 6 | A test or checkable procedure **demonstrates** the behavior on the #80 worked example | **Partially met** | Fixture + binding authored and runnable (`.ratchet/evals/{fixtures,specs}/issue-scope-reconciliation*`), case resolves to `judged`; **no green demonstration recorded** — see below | +| 7 | #80's remainder tracked with an owner; #80's `Closes` linkage corrected in the open PR stack | **Partially met** | Tracking half done (**#112**, owned). PR-body half dropped by owner decision — see task 8.2 | + +**Criterion 6 — the open gap, surfaced rather than self-approved.** The checkable procedure +exists, is correctly bound, and runs; what cannot be produced here is a passing verdict. Cause +(independently reproduced during verification): `extractVerdictJson` +(`src/core/eval/judge.ts:211`, introduced by PR #40, on `main`) takes the last balanced +top-level `[...]` in agent stdout, so under any adapter that emits stream-json the real verdict +array — which lives inside a JSON *string* value — is skipped in favour of the envelope's own +`permission_denials` array. Of the five registered adapters, `claude` and `opencode` both set +`emitsStreamJson: true`; `codex`, `gemini`, and `cursor` do not, but none of the three is +installed in this environment. So the binding cannot be demonstrated green by switching +adapters either, and this branch touches neither `src/core/eval/` nor +`src/core/batch/engine/` (`git diff main...HEAD` over both paths is empty). + +Three options were considered and rejected as violating the very rules this change adds: +weakening the eval spec so it passes; using `ratchet eval record` to override the verdict to +green; and quietly marking 6.3 done. The gap is instead recorded here and in the pull request, +and the harness defect is called out as needing its own change — it is repo-wide (it silently +fails-closed **every** `llm-judge` binding, including the pre-existing `eval-self.yaml`), not +specific to this work. + +**Consequent close-claim.** Because criteria 6 and 7 are partial, the pull request for this +change says **"partially addresses #100"** and does **not** claim `Closes #100`. That is this +change's own honest-close-claim rule applied to itself: a close-claim is an output of +verification, and verification did not produce one for all seven criteria. diff --git a/.ratchet/evals/fixtures/issue-scope-reconciliation/.claude/settings.json b/.ratchet/evals/fixtures/issue-scope-reconciliation/.claude/settings.json new file mode 100644 index 0000000..976835d --- /dev/null +++ b/.ratchet/evals/fixtures/issue-scope-reconciliation/.claude/settings.json @@ -0,0 +1,11 @@ +{ + "permissions": { + "allow": [ + "Bash", + "Read", + "Glob", + "Grep", + "LS" + ] + } +} diff --git a/.ratchet/evals/fixtures/issue-scope-reconciliation/.ratchet/config.yaml b/.ratchet/evals/fixtures/issue-scope-reconciliation/.ratchet/config.yaml new file mode 100644 index 0000000..7b4972a --- /dev/null +++ b/.ratchet/evals/fixtures/issue-scope-reconciliation/.ratchet/config.yaml @@ -0,0 +1 @@ +schema: ratchet diff --git a/.ratchet/evals/fixtures/issue-scope-reconciliation/README.md b/.ratchet/evals/fixtures/issue-scope-reconciliation/README.md new file mode 100644 index 0000000..13de813 --- /dev/null +++ b/.ratchet/evals/fixtures/issue-scope-reconciliation/README.md @@ -0,0 +1,25 @@ +# Fixture: issue-scope-reconciliation + +The worked example from +[issue #100](https://github.com/joctaTorres/ratchet/issues/100), reassembled as +a self-contained ratchet project so the hardened reconciliation procedure can be +replayed against it. + +| File | What it is | +| --- | --- | +| `issue-80.md` | Issue #80's text, verbatim. Its Why names the permission-posture bypass; its fix proposal hedges the gating requirement as "**Consider** requiring an explicit opt-in pairing flag". | +| `manifest-excerpt.yaml` | The phase-one contract that hard-codes `Closes #80` in `goal` and `success`, and whose `gate-and-mark-agent-cmd-override` change `done` ends "Fixes #80." | +| `phase-1-plan.md` | The shipped phase-one plan carrying the self-approved `## Out of scope` de-scope bullet. | + +Every file here is an **input** — a record of what the unhardened workflows +produced. None of it is a model to copy. + +The three escapes the reconciliation procedure must flag: + +1. The issue's gating requirement (fix item 3) is present in the issue and + absent from the authored scope — and its hedged "Consider" wording does not + lower the bar, because the exposure is a permission bypass. +2. The permission-posture bypass named in the issue's Why (problem 2) is absent + from the authored scope and was never even noted as deferred. +3. The manifest's hard-coded `Closes #80` / "Fixes #80." is premature: the + issue's material requirements are not all implemented. diff --git a/.ratchet/evals/fixtures/issue-scope-reconciliation/issue-80.md b/.ratchet/evals/fixtures/issue-scope-reconciliation/issue-80.md new file mode 100644 index 0000000..9bf3012 --- /dev/null +++ b/.ratchet/evals/fixtures/issue-scope-reconciliation/issue-80.md @@ -0,0 +1,25 @@ +# Issue #80 — engine: RATCHET_BATCH_AGENT_CMD / RATCHET_EVAL_AGENT_CMD silently replace the agent in production — no gating, no notice, no journal marker, permissions bypassed + +> Source: https://github.com/joctaTorres/ratchet/issues/80 +> +> Quoted verbatim. This is the originating issue from issue #100's worked example; +> it is fixture input, not a task list for the reader. + +## Why + +The env override seams are read unconditionally at spawn time in production code: any value is executed as `bash -c ` (`src/core/batch/engine/engine.ts:609-616`; `src/core/eval/judge.ts:274-277`; `src/core/eval/mutation-harness.ts:146-154`). If the var is set in any environment where a user runs `batch apply` or `eval run` — leftover from an eval session, CI, a `.envrc` — the configured coding agent is silently replaced. Three compounding problems: + +1. **No signal**: no console notice, and journal entries produced under an override are indistinguishable from real agent work. A stub that appends a verify completion drives every change to `done` (see #78) with nothing marking the run synthetic. +2. **Permissions bypassed**: the override path never consults an adapter, so `resolvePermissionFlags` posture/deny output (`src/core/batch/runtime/agent-permissions.ts:252-262`) is moot while the var is set — `batch config`'s displayed posture is a lie. +3. **Triplication**: the same override+resolveAdapter logic exists in three copies (engine, judge, mutation harness), so any gating fix must land three times or be extracted first (overlaps the dedup in #67). + +## Fix proposal + +1. Print a loud, unmissable one-line notice on every spawn while an override is active (`⚠ agent overridden by RATCHET_BATCH_AGENT_CMD`), in both text and `--json` output (a `agentOverride: true` field). +2. Stamp override provenance into every journal entry/run record produced under it (e.g. `via: "env-override"`), so synthetic runs are auditable after the fact. +3. Consider requiring an explicit opt-in pairing flag (`--allow-agent-override`) outside of test environments, or gating the seam on `NODE_ENV`/an explicit `RATCHET_TEST=1`. +4. Extract the single `buildAgentSpawnRequest` helper (shared with #67's dedup) so the gating exists in exactly one place. + +## Implementation route + +Alters spawn semantics and journal/record shape — implement via `rct:propose` → `apply` → `verify`. diff --git a/.ratchet/evals/fixtures/issue-scope-reconciliation/manifest-excerpt.yaml b/.ratchet/evals/fixtures/issue-scope-reconciliation/manifest-excerpt.yaml new file mode 100644 index 0000000..066960a --- /dev/null +++ b/.ratchet/evals/fixtures/issue-scope-reconciliation/manifest-excerpt.yaml @@ -0,0 +1,39 @@ +# Phase-one excerpt of `.ratchet/batches/engine-runtime-hardening/batch.yaml`, +# as reported in issue #100's worked example. +# +# This is the manifest AS AUTHORED — the input to the check, not a model to copy. +# It is what a `propose-batch` run produced before the workflow carried any +# originating-issue reconciliation step or close-claim rule, and it is the +# artifact that locked `Closes #80` in before anyone had decided what would +# actually be built. + +name: engine-runtime-hardening + +phases: + - name: phase-1 + goal: >- + Make the agent-override seam auditable: a loud notice on every overridden + spawn, override provenance stamped into the journal, and the spawn-request + construction extracted into one shared helper. The phase PR MUST link + `Closes #89` and `Closes #80`. + success: >- + An overridden spawn prints the notice in text and sets `agentOverride: + true` in `--json`; every journal entry written under an override carries + `via: "env-override"`; the three duplicated override+resolveAdapter copies + are replaced by one `buildAgentSpawnRequest`. The phase PR links + `Closes #89` and `Closes #80`. + proofOfWork: + kind: integration + run: pnpm vitest run test/core/batch/engine/agent-spawn.test.ts + pass: exit-zero + + changes: + - name: gate-and-mark-agent-cmd-override + after: [] + done: >- + Every spawn made while `RATCHET_BATCH_AGENT_CMD` or + `RATCHET_EVAL_AGENT_CMD` is set prints a one-line override notice + (text and `--json`), stamps `via: "env-override"` provenance into the + journal entries and run records it produces, and builds its spawn + request through the single shared `buildAgentSpawnRequest` helper + rather than the three duplicated copies. Fixes #80. diff --git a/.ratchet/evals/fixtures/issue-scope-reconciliation/phase-1-plan.md b/.ratchet/evals/fixtures/issue-scope-reconciliation/phase-1-plan.md new file mode 100644 index 0000000..484ca30 --- /dev/null +++ b/.ratchet/evals/fixtures/issue-scope-reconciliation/phase-1-plan.md @@ -0,0 +1,35 @@ +# gate-and-mark-agent-cmd-override + +Phase-one change plan excerpt, as reported in issue #100's worked example. This +is the plan AS AUTHORED (shipped in PR #97) — the input to the check, not a +model to copy. The `## Out of scope` bullet below is the self-approved de-scope: +no tracking issue was filed, no owner was named, and no human was asked. + +## Why + +The env override seams are read unconditionally at spawn time in production +code, so a leftover `RATCHET_BATCH_AGENT_CMD` silently replaces the configured +coding agent with no signal in the console or the journal. + +## What Changes + +- A loud one-line notice on every spawn made while an override is active, in + both text and `--json` output (an `agentOverride: true` field). +- Override provenance (`via: "env-override"`) stamped into every journal entry + and run record produced under an override. +- A single shared `buildAgentSpawnRequest` helper replacing the three duplicated + override+resolveAdapter copies in the engine, the judge, and the mutation + harness. + +## Design + +Out of scope (kept thin per the vertical-slice strategy): the issue's "consider +an explicit opt-in flag / NODE_ENV gate" (proposal item 3) — … an opt-in gate +would break every existing e2e/eval harness invocation; revisit if #78's +corroboration work (phase 2) still needs it. + +## Tasks + +- [x] Print the override notice on every spawn (text and `--json`). +- [x] Stamp `via: "env-override"` into journal entries and run records. +- [x] Extract `buildAgentSpawnRequest` and route all three call sites through it. diff --git a/.ratchet/evals/specs/issue-scope-reconciliation.yaml b/.ratchet/evals/specs/issue-scope-reconciliation.yaml new file mode 100644 index 0000000..ba663b8 --- /dev/null +++ b/.ratchet/evals/specs/issue-scope-reconciliation.yaml @@ -0,0 +1,82 @@ +# Worked-example check: replay issue #100's failure on the HARDENED workflows. +# +# Issue #100's acceptance criterion 6 asks for a checkable procedure that +# demonstrates the reconciliation behavior rather than merely asserting it. +# Deterministic coverage of criteria 1-5 lives in the vitest suite +# (test/core/templates/workflows/*.test.ts), so CI never depends on a live agent +# spawn; THIS binding is the demonstration, and it is an `llm-judge` because the +# behavior under test is an agent applying a procedure to prose — there is no +# pure function to assert over, and a `deterministic` binding could only re-grep +# the skill text the vitest suite already greps. +# +# HOW THE JUDGE GETS THE PROCEDURE. The reconciliation step has exactly one +# author: `src/core/templates/workflows/scope-reconciliation.ts`. Rather than +# restate it here — the duplication this whole change exists to kill — `setup` +# runs `ratchet init` inside the fixture working copy, so the judge reads the +# procedure out of a REAL generated `SKILL.md`. The procedure therefore cannot +# drift from what ships. +# +# `${RATCHET_CLI:-ratchet}` resolves to the `ratchet` on PATH by default. When +# running from a worktree whose build is not the one on PATH, export +# RATCHET_CLI first, e.g. +# export RATCHET_CLI="node $PWD/bin/ratchet.js" +# The success criteria below FAIL CLOSED when the generated skill turns out not +# to carry the reconciliation step, so a stale CLI surfaces as an explicit +# failure rather than a vacuous pass. +# +# Run it with: +# node bin/ratchet.js eval run --change issue-scope-reconciliation \ +# --path .ratchet/changes/issue-scope-reconciliation/features/scope-reconciliation/worked-example-check.feature \ +# --only llm-judge --json +# +# KNOWN LIMITATION (issue #55): eval case ids embed the feature file's location, +# so this binding needs re-keying when the change is archived. +changes/issue-scope-reconciliation/features/scope-reconciliation/worked-example-check#replaying-the-worked-example-flags-all-three-escapes: + fixture: issue-scope-reconciliation + kind: llm-judge + setup: "${RATCHET_CLI:-ratchet} init --tools claude --force" + success: >- + You are replaying a real scope-reduction failure against the hardened + change-authoring workflows, inside a self-contained ratchet project (your + current working directory). Do the work; do not reason from memory. + + STEP 1 — read the procedure. Open + `.claude/skills/ratchet-propose/SKILL.md` and + `.claude/skills/ratchet-propose-batch/SKILL.md` and locate the + originating-issue reconciliation step. If neither file contains that step, + STOP and FAIL, reporting that the generated skills do not carry the + reconciliation procedure — do not substitute your own reconciliation method. + + STEP 2 — read the fixture. `issue-80.md` is the originating issue. + `manifest-excerpt.yaml` and `phase-1-plan.md` are the authored scope that a + prior, unhardened run produced. All three are inputs; none is a model to copy. + + STEP 3 — apply the procedure from STEP 1, exactly as written, to the issue in + `issue-80.md` against the authored scope in `manifest-excerpt.yaml` and + `phase-1-plan.md`. Enumerate the issue's material requirements, map each to + the authored scope, and list what is uncovered. + + PASS only if your reconciliation flags ALL THREE of the following, and + reports each one as a decision point requiring the user rather than an + omission you may approve yourself: + + (a) The issue's GATING requirement — fix-proposal item 3, the explicit + opt-in pairing flag / NODE_ENV gate on the override seam — is present in the + issue and absent from the authored scope. Its hedged "Consider" wording must + NOT be treated as permission to drop it, because the exposure is a + permission bypass. + + (b) The PERMISSION-POSTURE bypass named in the issue's Why (the override path + never consults an adapter, so the resolved permission posture is voided while + the displayed posture still reports it enforced) is absent from the authored + scope — it was never even recorded as deferred. + + (c) The manifest's hard-coded close-claim (`Closes #80` in the phase `goal` + and `success`, and the `gate-and-mark-agent-cmd-override` change `done` + ending "Fixes #80.") is PREMATURE, because the issue's material requirements + are not all implemented. + + Cite the concrete file and line/field for each of the three as evidence, and + quote the sentence of the generated SKILL.md that made you flag it. If ANY of + the three is missing from your reconciliation, or you cannot cite it, FAIL + CLOSED. A partial result is a failure, not a pass. diff --git a/README.md b/README.md index e7f1208..b49f1e8 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,10 @@ the full picture. objective, slices it into ordered **vertical-slice** phases, **hard-gates** every phase on a success criterion + an executable proof-of-work, and writes the manifest with a **shallow DAG** — only phase one is decomposed into change - intents. Its sole artifact is `batch.yaml`; it creates no change directories. + intents. Before scaffolding it **reconciles the manifest against every + originating issue** and refuses to hard-code a `Closes #N` claim into a phase + contract (see [Scope reconciliation](#scope-reconciliation)). Its sole artifact + is `batch.yaml`; it creates no change directories. - **`/rct:apply-batch`** is the autonomous orchestrator. It **loops** the single-step `ratchet batch apply` — read status → advance one transition (`propose → apply → verify` for one ready DAG step) → interpret the outcome — @@ -93,6 +96,42 @@ the full picture. answer via `ratchet batch report`, and resumes. Changes are created **lazily** as the loop reaches them. +### Scope reconciliation + +The three change-authoring workflows — `propose`, `propose-batch`, and +`decompose-phase` — share one set of rules that keeps authored scope honest against +the issues the work came from. The rules are authored once and rendered into every +tool's generated skill and command. + +- **Originating-issue reconciliation.** `propose` and `propose-batch` fetch every + originating issue through the project's issue tracker before authoring anything + (`gh issue view ` is a GitHub example, not a requirement — you can paste the + issue text instead), enumerate its material requirements from both its explicit + fix items and the problems named in its narrative, map each requirement to a + feature scenario, plan task, or manifest field, and surface every uncovered one + as an enumerated *"issue asks X, this proposal does not include X"* decision + point. Hedged wording in the issue (*"consider"*, *"optionally"*) does not lower + the bar for a security-relevant requirement, and an omission may never be + self-approved in plan prose. +- **Honest close-claims.** A `Fixes #N` / `Closes #N` claim is permitted only when + the issue's material requirements are actually implemented — it is an output of + verification, never an input of planning. Partial work says + *"partially addresses #N"*. Phase contracts reference issues as *"targets #N"* / + *"addresses #N"*; the closing linkage is earned at PR-authoring time. +- **Stop-and-surface guardrail.** A de-scope of security-, permission-, or + integrity-relevant work halts the workflow and asks you rather than proceeding on + momentum. An approved deferral requires a filed tracking issue with a named owner, + linked from the plan — a prose bullet in `plan.md` is not a deferral mechanism. +- **Deferrals survive phase boundaries.** `decompose-phase` reads each prior phase's + shipped `plan.md` (not only the injected `done` criteria, which are a paraphrase + that hides plan-prose deferrals), extracts every out-of-scope / deferred / revisit + item, and resolves each as **carried forward**, **tracked** against an open issue, + or **explicitly dropped** by you. It also verifies a prior phase's `Closes #N` was + earned before treating the issue as shipped. + +See [Generated artifacts](docs/configuration/generated-artifacts.md) for the full +Reference entry. + ### Standards Standards are project-level guidelines kept at `.ratchet/standards/*.md` — a sibling of the feature store, **not** a per-change artifact. A standard can cover any concern (testing, security, architecture, design, …). `ratchet init` creates the directory empty; author standards with `/rct:propose-standard`. @@ -296,8 +335,8 @@ from the phase's. A batch is intent you can revise before applying. | Workflow | Command | What it does | |---|---|---| -| **propose-batch** | `/rct:propose-batch ` | Guided, anti-waterfall authoring: explores the objective, slices it into ordered vertical-slice phases, **hard-gates** each phase on a success criterion + a proof-of-work (`integration` / `blackbox`), then scaffolds the manifest with a **shallow DAG** (only phase one decomposed). Its only artifact is the manifest — never change directories. Ends with a **gated hand-off into `/rct:apply-batch`** to drive the batch now (this session as orchestrator) or defer it to a later run. | -| **apply-batch** | `/rct:apply-batch ` | Autonomous orchestrator that drives the batch to completion. It **loops** `ratchet batch apply` (which stays single-step) until done, surfacing halts (blocked / awaiting-approval) and proof-of-work failures to you, recording your answers via `ratchet batch report`, then resuming. The orchestrator does **no coding itself** — it only runs `ratchet` CLI commands and talks to you; the coding happens inside the engine-spawned agent. When the next step is a reachable phase whose changes are still empty, `batch apply` decomposes it **natively** — spawning an agent that delegates to the canonical `decompose-phase` skill to author that phase's change intents from the prior phase's shipped results — then the loop continues into the new changes, with no manual stop/propose/resume detour. | +| **propose-batch** | `/rct:propose-batch ` | Guided, anti-waterfall authoring: explores the objective, slices it into ordered vertical-slice phases, **hard-gates** each phase on a success criterion + a proof-of-work (`integration` / `blackbox`), then scaffolds the manifest with a **shallow DAG** (only phase one decomposed). Its only artifact is the manifest — never change directories. Before scaffolding it reconciles the manifest against every originating issue and refuses a premature `Closes #N` (see [Scope reconciliation](#scope-reconciliation)). Ends with a **gated hand-off into `/rct:apply-batch`** to drive the batch now (this session as orchestrator) or defer it to a later run. | +| **apply-batch** | `/rct:apply-batch ` | Autonomous orchestrator that drives the batch to completion. It **loops** `ratchet batch apply` (which stays single-step) until done, surfacing halts (blocked / awaiting-approval) and proof-of-work failures to you, recording your answers via `ratchet batch report`, then resuming. The orchestrator does **no coding itself** — it only runs `ratchet` CLI commands and talks to you; the coding happens inside the engine-spawned agent. When the next step is a reachable phase whose changes are still empty, `batch apply` decomposes it **natively** — spawning an agent that delegates to the canonical `decompose-phase` skill to author that phase's change intents from the prior phase's shipped results, sweeping those phases' `plan.md` files for deferred scope and verifying any prior `Closes #N` was earned — then the loop continues into the new changes, with no manual stop/propose/resume detour. | ``` You: /rct:propose-batch ship a checkout flow @@ -639,12 +678,12 @@ deprecated alias mapped onto the gate. | Workflow | What it does | |---|---| | **brainstorm** | Front door for an open-ended idea: explores context, clarifies one question at a time, weighs 2–3 approaches, designs section-by-section, then recommends + gates a route into `propose` or `propose-batch` (does no implementation itself) | -| **propose** | Clarifies intent (explore-first when unclear), then generates `features/` + `plan.md` | +| **propose** | Clarifies intent (explore-first when unclear), [reconciles the authored scope against every originating issue](#scope-reconciliation), then generates `features/` + `plan.md` | | **apply** | Implements against each scenario's `Given/When/Then`, checking off plan tasks | | **verify** | Confirms the implementation satisfies every scenario and all tasks are done | | **archive** | Runs `ratchet archive` to ratchet features into the permanent store | | **propose-standard** | Authors a new standard into `.ratchet/standards/` for propose + verify to apply | -| **propose-batch** | Slices an objective into ordered vertical-slice phases with per-phase proofs-of-work and writes a batch manifest (not change directories) | +| **propose-batch** | Slices an objective into ordered vertical-slice phases with per-phase proofs-of-work and writes a batch manifest (not change directories), [reconciled against every originating issue](#scope-reconciliation) and free of premature `Closes #N` claims | | **apply-batch** | Autonomously drives a batch to completion — loops the single-step `ratchet batch apply`, surfaces halts/approvals + proof-of-work failures, records answers, resumes | | **eval** | Runs the engine-backed eval, surfaces regressions first, and guides authoring bindings for unjudged cases | diff --git a/docs/configuration/generated-artifacts.md b/docs/configuration/generated-artifacts.md index 95da890..d7037e6 100644 --- a/docs/configuration/generated-artifacts.md +++ b/docs/configuration/generated-artifacts.md @@ -120,6 +120,123 @@ generated for every tool above at the tool's command path (e.g. claude The active profile is read from the global ratchet config. Pass `--profile core` or `--profile custom` to `ratchet init` to override it for that run. +### Scope reconciliation in the change-authoring workflows + +The `propose`, `propose-batch`, and `decompose-phase` workflows carry a shared set of +scope-reconciliation rules. The rules are authored once in +`src/core/templates/workflows/scope-reconciliation.ts` and interpolated into each +workflow body, so every generated skill and command for every tool in the table above +renders the same text. + +#### Overview + +```mermaid +flowchart TD + A[📝 Authoring run starts
propose · propose-batch] + B{🔍 Work originates
from a tracked issue?} + C[🌐 Fetch each issue
through the project's tracker] + D[📋 Enumerate material requirements
explicit fix items · narrative problems] + E[🔗 Map each requirement to a
feature scenario · plan task · manifest field] + F{❓ Any requirement
uncovered?} + G[👤 Surface as an enumerated decision point
issue asks X · this proposal omits X] + H{🔐 Security · permission ·
integrity relevant?} + I[🛑 Stop and surface
halt the run · ask the user] + J[👤 User decides] + K[📌 Approved deferral needs a filed
tracking issue · named owner · linked from the plan] + Z[✅ Author the artifacts] + L{🏷️ All material requirements
implemented?} + M[✅ A closing claim may be made
at pull-request-authoring time] + N[⚠️ Say partially addresses
never a closing claim] + + A --> B + B -- no --> Z + B -- yes --> C + C --> D + D --> E + E --> F + F -- no --> Z + F -- yes --> G + G --> H + H -- yes --> I + H -- no --> J + I --> K + K --> Z + J --> Z + Z --> L + L -- yes --> M + L -- no --> N + + classDef entry fill:#90EE90,stroke:#333,stroke-width:2px,color:darkgreen + classDef step fill:#E6E6FA,stroke:#333,stroke-width:2px,color:darkblue + classDef gate fill:#FFD700,stroke:#333,stroke-width:2px,color:black + classDef halt fill:#FFB6C1,stroke:#DC143C,stroke-width:2px,color:black + classDef done fill:#ADD8E6,stroke:#333,stroke-width:2px,color:darkblue + + class A entry + class C,D,E,G,J,K step + class B,F,H,L gate + class I,N halt + class Z,M done +``` + +#### Originating-issue reconciliation + +`propose` and `propose-batch` each run an originating-issue reconciliation step before +any artifact is authored — in `propose` it is step 2, before the change directory is +created; in `propose-batch` it is step 4, before `ratchet new batch` scaffolds the +manifest. The step requires: + +| Sub-step | Requirement | +|---|---| +| Identify | Collect every originating issue — referenced by the user, by a manifest phase or change intent, or by an injected `done` criterion. | +| Fetch | Read each issue through the project's issue tracker. `gh issue view ` is named as a GitHub example; no tracker or command is required. When no tracker client is available, the user is asked to paste the issue text. | +| Enumerate | List the issue's material requirements from both its explicit fix items and the problems named in its narrative. Hedged wording (`consider`, `maybe`, `optionally`) does not lower the bar for a security-, permission-, or integrity-relevant requirement. | +| Map | Point every enumerated requirement at the feature scenario or plan task that covers it. In `propose-batch` the target is the phase `goal`, the phase `success` criterion, or the change-level `done`. | +| List | Write out every requirement the authored scope does not cover. | +| Surface | Present each uncovered requirement to the user as an enumerated `"issue asks X, this proposal does not include X"` decision point before artifacts are finalized. Writing an omission into plan prose as self-approval is prohibited. | + +#### Close-claim rules + +A `Fixes #N` / `Closes #N` claim — in a batch manifest `done`, in a plan, or in a +pull-request body — is permitted only when the issue's material requirements are +actually implemented. A close-claim is an output of verification, never an input of +planning. Work covering only part of an issue states `"partially addresses #N"` and +does not state `Fixes #N` or `Closes #N`. + +`propose-batch` applies this to the manifest specifically: `Closes #N` / `Fixes #N` +must not be hard-coded into a phase `goal`, a phase `success` criterion, or a +change-level `done` for work not yet scoped and verified. Phase contracts reference an +issue as `"targets #N"` or `"addresses #N"`; the closing linkage is earned at +pull-request-authoring time. + +#### Stop-and-surface guardrail + +All three workflows carry the guardrail in their **Guardrails** section. A de-scope of +security-, permission-, or integrity-relevant work halts the workflow and asks the +user rather than proceeding on momentum. An approved deferral requires a tracking +issue that is explicitly filed, carries a named owner, and is linked from the change +plan before the workflow proceeds; a prose bullet in `plan.md` is not a deferral +mechanism. + +#### Deferral carry-forward in `decompose-phase` + +`decompose-phase` grounds a phase in the prior phases' shipped results. Its grounding +step additionally requires: + +- Reading each prior phase's shipped change `plan.md`, not only the injected `done` + criteria. The injected criteria are a paraphrase in which deferrals recorded as plan + prose are invisible. +- Extracting every `## Out of scope`, `deferred`, `revisit`, `later phase`, + `follow-up`, or equivalent item recorded in those plans. +- Resolving each extracted item as exactly one of three outcomes: **carried forward** + as a change intent in the phase being decomposed, **tracked** against an existing + open tracking issue, or **explicitly dropped** by the user. Silently ignoring an + extracted item is not an available outcome. +- Verifying that a prior phase's `Fixes #N` / `Closes #N` claim was earned before the + issue is treated as shipped. An unearned claim is surfaced and its remaining scope + carried forward, never inherited as fact. + + ## Change directory artifacts `ratchet new change ` scaffolds a change directory at `.ratchet/changes//`. The propose workflow writes the two required artifacts into it. diff --git a/src/core/templates/workflows/decompose-phase.ts b/src/core/templates/workflows/decompose-phase.ts index 15cacec..82f8ac7 100644 --- a/src/core/templates/workflows/decompose-phase.ts +++ b/src/core/templates/workflows/decompose-phase.ts @@ -15,8 +15,19 @@ * Like the other batch templates, the skill and the command share one body * constant, and the lazy-decomposition guidance is the SAME guidance * `propose-batch` owns (one author of decomposition semantics). + * + * The close-claim and stop-and-surface guardrails are interpolated from + * `./scope-reconciliation.js`, the single author of those rules across + * `propose`, `propose-batch`, and `decompose-phase` — never restated here. This + * workflow is where a deferral either survives a phase boundary or evaporates, + * so its grounding step also carries the prior-plan sweep and the earned-close + * verification. */ import type { SkillTemplate, CommandTemplate } from '../types.js'; +import { + CLOSE_CLAIM_RULES, + STOP_AND_SURFACE_GUARDRAIL, +} from './scope-reconciliation.js'; const DECOMPOSE_PHASE_BODY = `Decompose ONE phase of an EXISTING batch — author that phase's concrete change intents into its \`changes\` list in \`.ratchet/batches//batch.yaml\`, lazily, @@ -49,6 +60,52 @@ context is the basis for decomposition; do not invent requirements beyond it. decompose toward this phase's \`goal\` using what now actually exists, not a guess made before the prior phase ran. + a. **Read each prior phase's shipped change \`plan.md\`, not only the injected + \`done\` criteria.** The injected criteria are a **paraphrase** of what each + change set out to do. A deferral recorded as plan prose — an + \`## Out of scope\` section, a "deferred", "revisit in the next phase", or + "not doing this yet" bullet — never appears in that paraphrase, so grounding + only in the injected \`done\` makes those deferrals **invisible** at exactly + the moment they were supposed to be picked up. Open the prior phases' + shipped change directories and read their \`plan.md\` files directly. + + b. **Extract every deferred item recorded in those plans.** Sweep each plan for + every \`## Out of scope\` entry and every "deferred", "revisit", "later + phase", "follow-up", or equivalent item, wherever it appears — including + inside \`## Why\`, \`## What Changes\`, and \`## Design\` prose, not only under a + heading that happens to be named "Out of scope". List what you extracted. + + c. **Resolve EACH extracted item as exactly one of three outcomes.** For every + item on that list, choose and record one: + - **(a) carried forward** — authored as a change intent in the phase you are + decomposing; + - **(b) tracked** — matched to an existing OPEN tracking issue, reported to + the user with that issue's number; or + - **(c) explicitly dropped** — surfaced to the user as a drop decision and + answered by them. + + **Silently ignoring an extracted item is not an available outcome.** Every + item leaves this step with (a), (b), or (c) written next to it. Report the + resolved list before you author intents. For a security-, permission-, or + integrity-relevant item, outcome (c) is a stop-and-surface event under the + guardrails below, and outcome (b) requires the tracking issue to actually + exist and to be open — an issue you intend to file is not a tracked item + until it is filed. + + d. **Verify a prior phase's close-claim was earned before treating an issue as + shipped.** When a prior phase's \`done\` criterion or plan claims + \`Fixes #N\` / \`Closes #N\`, do not inherit that claim as fact. Fetch the + issue (on GitHub, for example, \`gh issue view \`; other trackers have + their own client; if your agent has none, ask the user to paste the issue + text), enumerate its material requirements from both its explicit fix items + and the problems named in its narrative, and compare them against what that + phase's \`done\` and \`plan.md\` describe as actually implemented. + + A requirement the prior phase did not implement means the close-claim was + **unearned**. Surface the unearned claim to the user explicitly, and carry + the remaining scope forward into this phase's change intents — the claim is + never inherited as fact, and the issue is not treated as shipped. + 2. **Slice the phase into concrete change intents** Author one or more concrete change intents that, taken together, achieve this @@ -79,6 +136,11 @@ context is the basis for decomposition; do not invent requirements beyond it. After editing, summarize: - The phase you decomposed and the batch it belongs to. - The change intents you authored, each with its \`after\` edges and its \`done\`. +- The deferred items you extracted from the prior phases' plans, each with its + resolution: carried forward, tracked (with the issue number), or explicitly + dropped by the user. +- Any prior-phase close-claim you found unearned, and where its remaining scope + went. - A reminder that the changes are not yet created on disk — \`ratchet batch apply\` creates them lazily. @@ -88,7 +150,12 @@ After editing, summarize: guess. - Every authored change intent has a non-empty \`done\`, or the manifest fails validation. -- The only artifact is the manifest edit — never change directories.`; +- The only artifact is the manifest edit — never change directories. +- Ground in the prior phases' \`plan.md\` files, not only the injected \`done\` + criteria; every extracted deferral is carried forward, tracked, or explicitly + dropped by the user — never silently ignored. +${CLOSE_CLAIM_RULES} +${STOP_AND_SURFACE_GUARDRAIL}`; export function getDecomposePhaseSkillTemplate(): SkillTemplate { return { diff --git a/src/core/templates/workflows/propose-batch.ts b/src/core/templates/workflows/propose-batch.ts index 6c1a2cf..50de318 100644 --- a/src/core/templates/workflows/propose-batch.ts +++ b/src/core/templates/workflows/propose-batch.ts @@ -10,8 +10,18 @@ * `.ratchet/batches//batch.yaml` — never change directories. * * Like `batch.ts`, both the skill and the command share a single body constant. + * + * The originating-issue reconciliation step and the close-claim / + * stop-and-surface guardrails are interpolated from `./scope-reconciliation.js`, + * the single author of those rules across `propose`, `propose-batch`, and + * `decompose-phase` — never restated here. */ import type { SkillTemplate, CommandTemplate } from '../types.js'; +import { + ISSUE_RECONCILIATION_STEP, + CLOSE_CLAIM_RULES, + STOP_AND_SURFACE_GUARDRAIL, +} from './scope-reconciliation.js'; const PROPOSE_BATCH_BODY = `Propose a batch — a phased, anti-waterfall unit of work — by guiding the author to a batch manifest at \`.ratchet/batches//batch.yaml\`. @@ -97,7 +107,37 @@ derive one from the objective. that the exact runnable command is **refined at phase entry**. Do not demand an exact command for software that does not yet exist. -4. **Scaffold the manifest via existing machinery (shallow DAG)** +4. **Reconcile the manifest against every originating issue (before scaffolding)** + + When the objective, a phase, or a change intent originates from a tracked + issue, reconcile against that issue BEFORE \`ratchet new batch\` is run. The + "authored scope" being reconciled here is the manifest you are about to write: + each phase \`goal\`, each phase \`success\` criterion, and each change-level + \`done\`. + +${ISSUE_RECONCILIATION_STEP} + + Map each enumerated requirement onto the phase \`goal\`, phase \`success\`, or + change-level \`done\` that carries it, and surface every requirement the + manifest leaves uncovered to the user BEFORE the manifest is scaffolded. A + requirement no phase contract carries is not "handled in a later phase" unless + a later phase's \`goal\` actually names it — an unwritten intention is an + omission, and omissions are the user's decision to make, not yours. + + **No premature close-claims in the manifest.** The manifest MUST NOT hard-code + \`Closes #N\` or \`Fixes #N\` in a phase \`goal\`, in a phase \`success\` + criterion, or in a change-level \`done\` for work that has not yet been scoped + and verified. At manifest-authoring time nothing has been implemented, so no + close-claim can have been earned: + + - Phase contracts reference an issue as **"targets #N"** or + **"addresses #N"** — never as a closing claim. + - A change-level \`done\` covering only part of an issue says + **"partially addresses #N"**, and MUST NOT say \`Fixes #N\` or \`Closes #N\`. + - The \`Closes #N\` linkage is earned at pull-request-authoring time, only + after the issue's material requirements are confirmed implemented. + +5. **Scaffold the manifest via existing machinery (shallow DAG)** Once you have valid phases with proofs-of-work and a batch name: \`\`\`bash @@ -118,6 +158,10 @@ derive one from the objective. specifically (distinct from the phase \`success\`). Keep it to one line. It is **required** and must be non-empty — a change intent without a \`done\` fails validation. + - **Issue references**: phase contracts and change-level \`done\` criteria + reference originating issues as "targets #N" / "addresses #N", or + "partially addresses #N" for partial coverage. Never write \`Closes #N\` or + \`Fixes #N\` into the manifest — see step 4. - **Settings**: if the user wants a setting that differs from the project defaults, record it under the manifest \`settings\` block. Only these keys are accepted (the schema is strict — any other key fails validation): \`gate\`, @@ -128,7 +172,7 @@ derive one from the objective. generate any change directories under \`.ratchet/changes/\`, and do not produce any per-change planning artifacts at proposal time. -5. **Hand off to apply-batch to drive the batch now (gated)** +6. **Hand off to apply-batch to drive the batch now (gated)** After the manifest is written, present an **explicit gate** — never an automatic action — asking whether to drive the batch now by running the @@ -166,7 +210,11 @@ After scaffolding, summarize: - Only phase one is decomposed into change intents; later phases stay goal+proof. - The only artifact is the manifest — never change directories at proposal time. - The apply-batch hand-off is always gated: ask, never auto-drive the batch. -- \`propose-batch\` is only useful alongside the \`batch\` workflow; pair them.`; +- \`propose-batch\` is only useful alongside the \`batch\` workflow; pair them. +- Reconcile every originating issue before scaffolding; a material requirement + the manifest omits is surfaced to the user, never quietly left to a later phase. +${CLOSE_CLAIM_RULES} +${STOP_AND_SURFACE_GUARDRAIL}`; export function getProposeBatchSkillTemplate(): SkillTemplate { return { diff --git a/src/core/templates/workflows/propose.ts b/src/core/templates/workflows/propose.ts index 4060f60..699fb58 100644 --- a/src/core/templates/workflows/propose.ts +++ b/src/core/templates/workflows/propose.ts @@ -1,16 +1,45 @@ /** - * Skill Template Workflow Modules + * Propose guided workflow skill + command templates. * - * This file is generated by splitting the legacy monolithic - * templates file into workflow-focused modules. + * `/rct:propose` creates a change and authors its two artifacts in one step: + * the executable Gherkin features and `plan.md`. + * + * **One body, two surfaces.** The skill's `instructions` and the command's + * `content` are the SAME body, produced by {@link buildProposeBody} and + * differing in exactly two lines: the **Input** line (a skill is invoked by + * request, a command by its argument) and the closing **Prompt** line. Those two + * deltas are the builder's only parameters. The body used to be duplicated + * verbatim in both places, which meant every edit had to be made twice or the + * two surfaces silently diverged — the exact drift this workflow's own + * reconciliation rules exist to prevent, so the duplication is gone. + * + * The originating-issue reconciliation step and the close-claim / + * stop-and-surface guardrails are not authored here: they are interpolated from + * `./scope-reconciliation.js`, the single author of those rules across + * `propose`, `propose-batch`, and `decompose-phase`. */ import type { SkillTemplate, CommandTemplate } from '../types.js'; +import { + ISSUE_RECONCILIATION_STEP, + CLOSE_CLAIM_RULES, + STOP_AND_SURFACE_GUARDRAIL, +} from './scope-reconciliation.js'; + +/** The two lines that differ between the skill body and the command body. */ +interface ProposeBodyDeltas { + /** The **Input** line: how the invocation carries the change name. */ + inputLine: string; + /** The closing **Prompt** line: how the user is told to start implementing. */ + promptLine: string; +} -export function getRctProposeSkillTemplate(): SkillTemplate { - return { - name: 'ratchet-propose', - description: 'Propose a new change with all artifacts generated in one step. Use when the user wants to describe what they want to build and get executable Gherkin features plus a plan ready for implementation.', - instructions: `Propose a new change - create the change and generate all artifacts in one step. +/** + * Build the shared propose body, parameterized by the only two lines that differ + * between the skill and command surfaces. Everything else is identical by + * construction rather than by discipline. + */ +function buildProposeBody({ inputLine, promptLine }: ProposeBodyDeltas): string { + return `Propose a new change - create the change and generate all artifacts in one step. I'll create a change with two artifacts: - features//*.feature (executable Gherkin: Feature / Scenario / Given-When-Then) @@ -20,7 +49,7 @@ When ready to implement, run /rct:apply --- -**Input**: The user's request should include a change name (kebab-case) OR a description of what they want to build. +${inputLine} **Steps** @@ -38,13 +67,17 @@ When ready to implement, run /rct:apply **IMPORTANT**: Do NOT proceed without understanding the behavior to build. -2. **Create the change directory** +2. **Reconcile the authored scope against every originating issue** + +${ISSUE_RECONCILIATION_STEP} + +3. **Create the change directory** \`\`\`bash ratchet new change "" \`\`\` This creates a scaffolded change in the planning home resolved by the CLI with \`.ratchet.yaml\`. -3. **Get the artifact build order** +4. **Get the artifact build order** \`\`\`bash ratchet status --change "" --json \`\`\` @@ -53,7 +86,7 @@ When ready to implement, run /rct:apply - \`artifacts\`: list of all artifacts with their status and dependencies - \`planningHome\`, \`changeRoot\`, \`artifactPaths\`, and \`actionContext\`: path and scope context. Use these instead of assuming repo-local paths. -4. **Create artifacts in sequence until apply-ready** +5. **Create artifacts in sequence until apply-ready** Use the **TodoWrite tool** to track progress through the artifacts. @@ -85,6 +118,10 @@ When ready to implement, run /rct:apply particular standard). These tags are validated against \`.ratchet/standards/\` and are materialized into the feature store on archive. - Show brief progress: "Created " + - Complete the reconciliation map from step 2 against what you actually + authored: every material requirement of every originating issue must point + at a feature scenario or a plan task. Anything still uncovered is surfaced + to the user as a decision point before you call the artifacts done. b. **Continue until all \`applyRequires\` artifacts are complete** - After creating each artifact, re-run \`ratchet status --change "" --json\` @@ -95,7 +132,7 @@ When ready to implement, run /rct:apply - Ask the user to clarify (use a structured-question tool such as AskUserQuestion if your agent has one) - Then continue with creation -5. **Show final status** +6. **Show final status** \`\`\`bash ratchet status --change "" \`\`\` @@ -106,7 +143,7 @@ After completing all artifacts, summarize: - Change name and location - List of artifacts created with brief descriptions - What's ready: "All artifacts created! Ready for implementation." -- Prompt: "Run \`/rct:apply\` or ask me to implement to start working on the tasks." +${promptLine} **Artifact Creation Guidelines** @@ -123,7 +160,34 @@ After completing all artifacts, summarize: - Always read dependency artifacts before creating a new one - If context is critically unclear, ask the user - but prefer making reasonable decisions to keep momentum - If a change with that name already exists, ask if user wants to continue it or create a new one -- Verify each artifact file exists after writing before proceeding to next`, +- Verify each artifact file exists after writing before proceeding to next +${CLOSE_CLAIM_RULES} +${STOP_AND_SURFACE_GUARDRAIL}`; +} + +/** The skill surface's **Input** line: a skill is invoked by a user request. */ +const SKILL_INPUT_LINE = + "**Input**: The user's request should include a change name (kebab-case) OR a description of what they want to build."; + +/** The command surface's **Input** line: a command is invoked with an argument. */ +const COMMAND_INPUT_LINE = + '**Input**: The argument after `/rct:propose` is the change name (kebab-case), OR a description of what the user wants to build.'; + +/** The skill surface's closing **Prompt** line. */ +const SKILL_PROMPT_LINE = + '- Prompt: "Run `/rct:apply` or ask me to implement to start working on the tasks."'; + +/** The command surface's closing **Prompt** line. */ +const COMMAND_PROMPT_LINE = '- Prompt: "Run `/rct:apply` to start implementing."'; + +export function getRctProposeSkillTemplate(): SkillTemplate { + return { + name: 'ratchet-propose', + description: 'Propose a new change with all artifacts generated in one step. Use when the user wants to describe what they want to build and get executable Gherkin features plus a plan ready for implementation.', + instructions: buildProposeBody({ + inputLine: SKILL_INPUT_LINE, + promptLine: SKILL_PROMPT_LINE, + }), license: 'MIT', compatibility: 'Requires ratchet CLI.', metadata: { author: 'ratchet', version: '1.0' }, @@ -136,119 +200,9 @@ export function getRctProposeCommandTemplate(): CommandTemplate { description: 'Propose a new change - create it and generate all artifacts in one step', category: 'Workflow', tags: ['workflow', 'artifacts', 'experimental'], - content: `Propose a new change - create the change and generate all artifacts in one step. - -I'll create a change with two artifacts: -- features//*.feature (executable Gherkin: Feature / Scenario / Given-When-Then) -- plan.md (## Why, ## What Changes, ## Design, ## Tasks) - -When ready to implement, run /rct:apply - ---- - -**Input**: The argument after \`/rct:propose\` is the change name (kebab-case), OR a description of what the user wants to build. - -**Steps** - -1. **Explore first when the request is unclear, then derive a name** - - If the request is vague or you cannot confidently describe the behavior as - Gherkin scenarios, ask the user to clarify before scaffolding — use a - structured-question tool such as AskUserQuestion (open-ended, no preset - options) if your agent has one, otherwise ask in plain prose: - > "What change do you want to work on? Describe the behavior you want - who does what, and what should happen." - - Ask follow-ups until you can name the capability and sketch at least one - Given/When/Then scenario. From their description, derive a kebab-case change - name (e.g., "add user authentication" → \`add-user-auth\`). - - **IMPORTANT**: Do NOT proceed without understanding the behavior to build. - -2. **Create the change directory** - \`\`\`bash - ratchet new change "" - \`\`\` - This creates a scaffolded change in the planning home resolved by the CLI with \`.ratchet.yaml\`. - -3. **Get the artifact build order** - \`\`\`bash - ratchet status --change "" --json - \`\`\` - Parse the JSON to get: - - \`applyRequires\`: array of artifact IDs needed before implementation (e.g., \`["plan"]\`) - - \`artifacts\`: list of all artifacts with their status and dependencies - - \`planningHome\`, \`changeRoot\`, \`artifactPaths\`, and \`actionContext\`: path and scope context. Use these instead of assuming repo-local paths. - -4. **Create artifacts in sequence until apply-ready** - - Use the **TodoWrite tool** to track progress through the artifacts. - - Loop through artifacts in dependency order (artifacts with no pending dependencies first): - - a. **For each artifact that is \`ready\` (dependencies satisfied)**: - - Get instructions: - \`\`\`bash - ratchet instructions --change "" --json - \`\`\` - - The instructions JSON includes: - - \`context\`: Project background (constraints for you - do NOT include in output) - - \`rules\`: Artifact-specific rules (constraints for you - do NOT include in output) - - \`standards\`: Active project standards (array of \`{ name, tag, fileName, content }\`) the change must follow - - \`template\`: The structure to use for your output file - - \`instruction\`: Schema-specific guidance for this artifact type - - \`resolvedOutputPath\`: Resolved path or pattern to write the artifact - - \`dependencies\`: Completed artifacts to read for context - - Read any completed dependency files for context - - Create the artifact file using \`template\` as the structure and write it to \`resolvedOutputPath\` - - Apply \`context\` and \`rules\` as constraints - but do NOT copy them into the file - - Embed the applicable \`standards\` into the artifact: the plan's \`## Design\` and - \`## Tasks\` should reflect what each relevant standard requires, and the features - should cover behavior the standards demand. Apply (which never reads standards) - relies on the plan already encoding them, so make them concrete here. - - Record which standards the change follows: collect the \`tag\` of every standard - you embedded and write them to the change's \`.ratchet.yaml\` as a - \`standards: […]\` list (omit the field entirely when the change follows no - particular standard). These tags are validated against \`.ratchet/standards/\` and - are materialized into the feature store on archive. - - Show brief progress: "Created " - - b. **Continue until all \`applyRequires\` artifacts are complete** - - After creating each artifact, re-run \`ratchet status --change "" --json\` - - Check if every artifact ID in \`applyRequires\` has \`status: "done"\` in the artifacts array - - Stop when all \`applyRequires\` artifacts are done - - c. **If an artifact requires user input** (unclear context): - - Ask the user to clarify (use a structured-question tool such as AskUserQuestion if your agent has one) - - Then continue with creation - -5. **Show final status** - \`\`\`bash - ratchet status --change "" - \`\`\` - -**Output** - -After completing all artifacts, summarize: -- Change name and location -- List of artifacts created with brief descriptions -- What's ready: "All artifacts created! Ready for implementation." -- Prompt: "Run \`/rct:apply\` to start implementing." - -**Artifact Creation Guidelines** - -- Follow the \`instruction\` field from \`ratchet instructions\` for each artifact type -- The schema defines what each artifact should contain - follow it -- Read dependency artifacts for context before creating new ones -- Use \`template\` as the structure for your output file - fill in its sections -- **IMPORTANT**: \`context\` and \`rules\` are constraints for YOU, not content for the file - - Do NOT copy \`\`, \`\`, \`\` blocks into the artifact - - These guide what you write, but should never appear in the output - -**Guardrails** -- Create ALL artifacts needed for implementation (as defined by schema's \`apply.requires\`) -- Always read dependency artifacts before creating a new one -- If context is critically unclear, ask the user - but prefer making reasonable decisions to keep momentum -- If a change with that name already exists, ask if user wants to continue it or create a new one -- Verify each artifact file exists after writing before proceeding to next` + content: buildProposeBody({ + inputLine: COMMAND_INPUT_LINE, + promptLine: COMMAND_PROMPT_LINE, + }), }; } diff --git a/src/core/templates/workflows/scope-reconciliation.ts b/src/core/templates/workflows/scope-reconciliation.ts new file mode 100644 index 0000000..7935091 --- /dev/null +++ b/src/core/templates/workflows/scope-reconciliation.ts @@ -0,0 +1,135 @@ +/** + * Scope-reconciliation fragments shared by every change-authoring workflow. + * + * `propose`, `propose-batch`, and `decompose-phase` must all carry the same + * originating-issue reconciliation procedure, the same close-claim rule, and the + * same stop-and-surface guardrail. This module is the SINGLE AUTHOR of those + * rules: each workflow body interpolates the constants below rather than + * restating them, so the three workflows cannot drift apart. + * + * That is the point rather than a convenience. The failure this module exists to + * prevent is a scope reduction that survived because the rule governing it lived + * in one workflow's prose and not the next one's: a security-relevant half of an + * issue was dropped in a plan bullet, the issue was claimed closed anyway, and + * the phase that promised to "revisit" it never saw the deferral. Three + * hand-written copies of a rule are exactly that drift mechanism, so there is + * one copy and every body embeds it. It also makes "the rule is present in all + * three workflows, verbatim" a mechanical assertion (`toContain(CONSTANT)`) + * instead of a prose review. + * + * These fragments operationalize the `security-remediation` standard, which + * already states the policy (no partial remediation of severe exposures, + * severity governs over hedged source wording, honest close-claims, no silent + * scope drops) but had no enforcement point in the authoring workflows. + * + * Authoring constraints these constants must keep: + * + * - **Agent-neutral** (`multi-agent-support`): the prose says "your agent", and + * any agent-specific capability (Claude Code's `AskUserQuestion`) is phrased as + * optional with a plain-prose fallback that works in any agent. + * - **Tracker-neutral** (`generalizable-defaults`): these skills ship into + * arbitrary repositories, so no tracker is required and no command string is + * baked in. `gh issue view ` appears only as a named GitHub example, with a + * paste-the-issue-text fallback when no tracker client is available. + * - **No config reads** (`instruction-fed-config`): the procedure consumes only + * the issue references the user, the manifest, or the injected `done` already + * provides. It reads no config file and branches on no config key. + * - **Interpolated verbatim, never re-indented.** {@link ISSUE_RECONCILIATION_STEP} + * is indented three spaces so it drops in as the body of a numbered step (the + * indentation every workflow body already uses); the two guardrail constants + * are column-zero bullet lists so they drop into a **Guardrails** bullet list. + * Re-indenting either at a call site breaks the verbatim-containment tests. + */ + +/** + * The originating-issue reconciliation procedure, embedded by `propose` and + * `propose-batch` as the body of a numbered step that runs BEFORE any artifact + * is authored. + * + * Indented three spaces: it is the continuation body of a numbered list item, + * matching the step formatting all workflow bodies already use. + */ +export const ISSUE_RECONCILIATION_STEP = ` Work that originates from a tracked issue is reconciled against that issue + BEFORE any artifact is written. Nothing below is optional, and no step of it + may be replaced by your own reading of what the issue "really" wants. + + a. **Identify every originating issue.** An issue is originating when the user + references it, when a batch manifest phase or change intent references it, + or when an injected \`done\` criterion references it. Collect every such + reference — there is often more than one. + + b. **Fetch each originating issue through the project's issue tracker.** Read + the real issue text. Never work from a paraphrase, and never from an + injected \`done\` criterion alone — that criterion is itself a paraphrase, and + whatever it left out is invisible in it. On GitHub, for example, + \`gh issue view \` prints an issue; other trackers have their own client. + If your agent has no tracker client for this project, ask the user to paste + the issue text and wait for it. Do not proceed on a summary. + + c. **Enumerate the issue's material requirements.** Read the whole issue, not + only its checklist. A material requirement comes from either of two places: + - its **explicit fix items** — the numbered or bulleted things it asks for; + and + - the **problems named in its narrative** — its Why, background, or body + prose — that the fix items never restate. A problem described only in + prose is still a requirement. + + **Hedged source wording does not lower the bar.** "Consider", "maybe", + "optionally", "could", and "nice to have" describe the issue author's + confidence, not the requirement's weight. For anything security-, + permission-, or integrity-relevant, severity governs and a hedged item is + enumerated as material exactly like an imperative one. + + d. **Map every enumerated requirement to authored scope.** Build an explicit + requirement-to-artifact map: each requirement names the feature scenario or + the plan task that covers it. A requirement with nothing pointing at it is + uncovered. + + e. **List every requirement the authored scope does not cover.** Write the + uncovered set out explicitly. An empty list is a valid outcome, but it is + stated rather than assumed. + + f. **Surface every uncovered requirement to the user as a decision point.** + Before the artifacts are finalized, present the uncovered set as an + enumerated list in the form "issue asks X, this proposal does not include + X", and ask what to do about each one — use a structured-question tool such + as AskUserQuestion if your agent has one, otherwise ask in plain prose and + wait for an answer. + + You MUST NOT self-approve an omission by writing it into plan prose. An + "Out of scope", "deferred", or "revisit later" bullet in a plan records a + decision the user already made; it is never a substitute for asking them.`; + +/** + * The close-claim rule, embedded by all three change-authoring workflows in + * their **Guardrails** section. + * + * A column-zero bullet list: it appends to an existing `- `-item Guardrails list. + */ +export const CLOSE_CLAIM_RULES = `- **A close-claim is earned, never assumed.** A \`Fixes #N\` / \`Closes #N\` claim — + in a batch manifest \`done\`, in a plan, or in a pull-request body — is permitted + only when the issue's material requirements are actually implemented. A + close-claim is an output of verification, never an input of planning: it is + written once the requirements are confirmed implemented, never before the work + is scoped. +- **Partial work says it is partial.** Work covering only part of an issue MUST + say "partially addresses #N", and MUST NOT say "Fixes #N" or "Closes #N". + Naming what remains uncovered is part of that claim.`; + +/** + * The stop-and-surface guardrail for security-relevant de-scopes, embedded by + * all three change-authoring workflows in their **Guardrails** section. + * + * A column-zero bullet list: it appends to an existing `- `-item Guardrails list. + */ +export const STOP_AND_SURFACE_GUARDRAIL = `- **A security-relevant de-scope is a stop-and-surface event.** Any de-scope of + security-, permission-, or integrity-relevant work halts this workflow and asks + the user. It is never taken on momentum, never taken because the narrower scope + is easier to ship, and never settled inside the run without a human answering. + Halt, present the de-scope, and wait for the user. +- **An approved deferral requires a filed, linked, owned tracking issue.** When + the user approves deferring such a requirement, a tracking issue for the + deferred scope MUST be explicitly filed with a named owner, and MUST be linked + from the change plan, before you proceed. A prose bullet in \`plan.md\` is not a + deferral mechanism — an unlinked, unowned "revisit later" note is exactly how + deferred security scope disappears at the next phase boundary.`; diff --git a/test/core/init-scope-reconciliation.test.ts b/test/core/init-scope-reconciliation.test.ts new file mode 100644 index 0000000..5c252a7 --- /dev/null +++ b/test/core/init-scope-reconciliation.test.ts @@ -0,0 +1,139 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { InitCommand } from '../../src/core/init.js'; +import { + ISSUE_RECONCILIATION_STEP, + CLOSE_CLAIM_RULES, + STOP_AND_SURFACE_GUARDRAIL, +} from '../../src/core/templates/workflows/scope-reconciliation.js'; + +/** + * Issue #100's acceptance criteria require the hardened prose to be present in + * BOTH `.claude/skills//SKILL.md` and `.opencode/skills//SKILL.md`. + * + * Those trees are gitignored here — they are GENERATED into a consuming project + * by `ratchet init` from the workflow template modules. So the "both trees" + * clause is proven the only way it can be honestly proven: by running `init` and + * reading what it wrote, never by hand-editing a generated tree (which the next + * regeneration would revert). + */ + +const { confirmMock, selectMock, showWelcomeScreenMock, searchableMultiSelectMock, runDoctorAdvisoryMock } = + vi.hoisted(() => ({ + confirmMock: vi.fn(), + selectMock: vi.fn(), + showWelcomeScreenMock: vi.fn().mockResolvedValue(undefined), + searchableMultiSelectMock: vi.fn(), + runDoctorAdvisoryMock: vi.fn(), + })); + +vi.mock('../../src/commands/doctor.js', () => ({ + runDoctorAdvisory: runDoctorAdvisoryMock, +})); + +vi.mock('@inquirer/prompts', () => ({ + confirm: confirmMock, + select: selectMock, +})); + +vi.mock('../../src/ui/welcome-screen.js', () => ({ + showWelcomeScreen: showWelcomeScreenMock, +})); + +vi.mock('../../src/prompts/searchable-multi-select.js', () => ({ + searchableMultiSelect: searchableMultiSelectMock, +})); + +/** The three change-authoring skills the reconciliation hardening covers. */ +const HARDENED_SKILLS = ['ratchet-propose', 'ratchet-propose-batch', 'ratchet-decompose-phase'] as const; + +/** Every agent tree `init` must render the hardened prose into. */ +const TREES = ['.claude', '.opencode'] as const; + +describe('init renders the scope-reconciliation hardening into every agent tree', () => { + let testDir: string; + let configTempDir: string; + let originalEnv: NodeJS.ProcessEnv; + + beforeEach(async () => { + testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ratchet-init-recon-test-')); + originalEnv = { ...process.env }; + configTempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ratchet-config-init-recon-')); + process.env.XDG_CONFIG_HOME = configTempDir; + + vi.spyOn(console, 'log').mockImplementation(() => { }); + confirmMock.mockReset(); + confirmMock.mockResolvedValue(true); + selectMock.mockReset(); + selectMock.mockResolvedValue('repo-sandboxed-permissive'); + showWelcomeScreenMock.mockClear(); + searchableMultiSelectMock.mockReset(); + + await new InitCommand({ tools: 'claude,opencode', force: true }).execute(testDir); + }); + + afterEach(async () => { + process.env = originalEnv; + await fs.rm(testDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 }); + await fs.rm(configTempDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 }); + vi.restoreAllMocks(); + }); + + const read = (tree: string, skill: string): Promise => + fs.readFile(path.join(testDir, tree, 'skills', skill, 'SKILL.md'), 'utf-8'); + + const cases = TREES.flatMap((tree) => HARDENED_SKILLS.map((skill) => [tree, skill] as const)); + + it.each(cases)('%s/skills/%s/SKILL.md carries the stop-and-surface guardrail', async (tree, skill) => { + // Criterion 5: all three skills, both trees. + expect(await read(tree, skill)).toContain(STOP_AND_SURFACE_GUARDRAIL); + }); + + it.each(cases)('%s/skills/%s/SKILL.md carries the close-claim rule', async (tree, skill) => { + expect(await read(tree, skill)).toContain(CLOSE_CLAIM_RULES); + }); + + const reconciliationCases = TREES.flatMap((tree) => + (['ratchet-propose', 'ratchet-propose-batch'] as const).map((skill) => [tree, skill] as const) + ); + + it.each(reconciliationCases)( + '%s/skills/%s/SKILL.md carries the originating-issue reconciliation step', + async (tree, skill) => { + // Criterion 1: the reconciliation step in propose and propose-batch, both trees. + expect(await read(tree, skill)).toContain(ISSUE_RECONCILIATION_STEP); + } + ); + + it.each(TREES)('%s/skills/ratchet-propose-batch/SKILL.md forbids a premature close-claim', async (tree) => { + // Criterion 2. + const content = await read(tree, 'ratchet-propose-batch'); + expect(content).toMatch(/no premature close-claims in the manifest/i); + expect(content).toContain('**"targets #N"**'); + expect(content).toContain('**"partially addresses #N"**'); + }); + + it.each(TREES)('%s/skills/ratchet-decompose-phase/SKILL.md carries the deferral sweep', async (tree) => { + // Criteria 3 and 4. + const content = await read(tree, 'ratchet-decompose-phase'); + expect(content).toMatch(/extract every deferred item recorded in those plans/i); + expect(content).toMatch(/Silently ignoring an extracted item is not an available outcome/i); + expect(content).toMatch(/close-claim was earned before treating an issue as/i); + }); + + it('renders both trees content-identically apart from slash-command naming', async () => { + // The two trees are generated from the same template body, so they may + // differ ONLY in how each agent spells a slash command (`/rct:apply` vs + // `/rct-apply`). Normalising that one documented difference away, the files + // must be byte-identical — any other divergence is the drift this change + // exists to prevent. + const normalise = (content: string): string => content.replace(/\/rct[:-]/g, '/rct:'); + + for (const skill of HARDENED_SKILLS) { + const [claude, opencode] = await Promise.all([read('.claude', skill), read('.opencode', skill)]); + expect(normalise(claude), skill).toBe(normalise(opencode)); + } + }); +}); diff --git a/test/core/templates/workflows/decompose-phase.test.ts b/test/core/templates/workflows/decompose-phase.test.ts new file mode 100644 index 0000000..938e3e8 --- /dev/null +++ b/test/core/templates/workflows/decompose-phase.test.ts @@ -0,0 +1,143 @@ +import { describe, it, expect } from 'vitest'; +import { + getDecomposePhaseSkillTemplate, + getRctDecomposePhaseCommandTemplate, +} from '../../../../src/core/templates/workflows/decompose-phase.js'; +import { + CLOSE_CLAIM_RULES, + STOP_AND_SURFACE_GUARDRAIL, +} from '../../../../src/core/templates/workflows/scope-reconciliation.js'; +import { CommandAdapterRegistry } from '../../../../src/core/command-generation/registry.js'; +import { generateCommand } from '../../../../src/core/command-generation/generator.js'; +import type { CommandContent } from '../../../../src/core/command-generation/types.js'; + +describe('decompose-phase workflow templates', () => { + it('shares one body between the skill and the command surfaces', () => { + const skill = getDecomposePhaseSkillTemplate(); + const command = getRctDecomposePhaseCommandTemplate(); + + expect(skill.name).toBe('ratchet-decompose-phase'); + expect(command.category).toBe('Workflow'); + expect(command.tags).toEqual(['workflow', 'batch', 'experimental']); + expect(command.content).toBe(skill.instructions); + }); + + describe('prior-plan deferral sweep (issue #100 criterion 3)', () => { + const body = getDecomposePhaseSkillTemplate().instructions; + + it('requires reading each prior phase plan, not only the injected done criteria', () => { + expect(body).toMatch( + /read each prior phase's shipped change `plan\.md`, not only the injected\s+`done` criteria/i + ); + expect(body).toMatch(/open the prior phases'\s+shipped change directories and read their `plan\.md` files directly/i); + }); + + it('explains that the injected criteria are a paraphrase hiding plan-prose deferrals', () => { + expect(body).toMatch(/the injected criteria are a \*\*paraphrase\*\*/i); + expect(body).toMatch(/never appears in that paraphrase/i); + expect(body).toMatch(/makes those deferrals \*\*invisible\*\*/i); + }); + + it('requires extracting every out-of-scope, deferred, or revisit item', () => { + expect(body).toMatch(/extract every deferred item recorded in those plans/i); + expect(body).toContain('`## Out of scope`'); + expect(body).toMatch(/"deferred", "revisit", "later\s+phase", "follow-up", or equivalent item/i); + // Not only under a heading that happens to be named "Out of scope". + expect(body).toMatch(/wherever it appears/i); + }); + + it('mandates the carry-forward / tracked / explicit-drop trichotomy', () => { + expect(body).toMatch(/resolve EACH extracted item as exactly one of three outcomes/i); + expect(body).toMatch(/\*\*\(a\) carried forward\*\*/); + expect(body).toMatch(/\*\*\(b\) tracked\*\*/); + expect(body).toMatch(/\*\*\(c\) explicitly dropped\*\*/); + expect(body).toMatch(/matched to an existing OPEN tracking issue/i); + }); + + it('states that silently ignoring an extracted item is not an outcome', () => { + expect(body).toMatch(/\*\*Silently ignoring an extracted item is not an available outcome\.\*\*/); + }); + + it('reports the resolved sweep in its output summary', () => { + expect(body).toMatch(/the deferred items you extracted from the prior phases' plans, each with its\s+resolution/i); + }); + }); + + describe('earned-close verification (issue #100 criterion 4)', () => { + const body = getDecomposePhaseSkillTemplate().instructions; + + it('requires verifying a prior close-claim before treating an issue as shipped', () => { + expect(body).toMatch(/verify a prior phase's close-claim was earned before treating an issue as\s+shipped/i); + expect(body).toMatch(/do not inherit that claim as fact/i); + }); + + it('compares the issue requirements against what the prior done and plan describe', () => { + expect(body).toMatch(/enumerate its material requirements from both its explicit fix items/i); + expect(body).toMatch(/problems named in its narrative/i); + expect(body).toMatch( + /compare them against what that\s+phase's `done` and `plan\.md` describe as actually implemented/i + ); + }); + + it('surfaces an unearned claim and carries the remaining scope forward', () => { + expect(body).toMatch(/the close-claim was\s+\*\*unearned\*\*/i); + expect(body).toMatch(/surface the unearned claim to the user explicitly/i); + expect(body).toMatch(/carry\s+the remaining scope forward into this phase's change intents/i); + expect(body).toMatch(/the claim is\s+never inherited as fact/i); + }); + + it('stays tracker-neutral when fetching the issue', () => { + expect(body).toContain('gh issue view '); + expect(body).toMatch(/on github, for example/i); + expect(body).toMatch(/other trackers have\s+their own client/i); + expect(body).toMatch(/ask the user to paste the issue\s+text/i); + }); + }); + + describe('shared guardrails (issue #100 criterion 5)', () => { + const body = getDecomposePhaseSkillTemplate().instructions; + + it('embeds the close-claim rule verbatim', () => { + expect(body).toContain(CLOSE_CLAIM_RULES); + }); + + it('embeds the stop-and-surface guardrail verbatim', () => { + expect(body).toContain(STOP_AND_SURFACE_GUARDRAIL); + }); + + it('carries the sweep as a guardrail as well as a step', () => { + expect(body).toMatch(/never silently ignored/i); + }); + + it('stays agent-neutral (multi-agent-support)', () => { + expect(body.toLowerCase()).toContain('your agent'); + expect(body).not.toMatch(/\bClaude\b/); + }); + }); + + it('renders the decomposition rules into every registered tool command', () => { + const cmd = getRctDecomposePhaseCommandTemplate(); + const content: CommandContent = { + id: 'rct-decompose-phase', + name: cmd.name, + description: cmd.description, + category: cmd.category, + tags: cmd.tags, + body: cmd.content, + }; + + const adapters = CommandAdapterRegistry.getAll(); + expect(adapters.length).toBeGreaterThanOrEqual(5); + for (const adapter of adapters) { + const { fileContent } = generateCommand(content, adapter); + const label = `tool: ${adapter.toolId}`; + // The prior-plan deferral sweep survives every adapter's formatting. + expect(fileContent, label).toMatch(/extract every deferred item recorded in those plans/i); + expect(fileContent, label).toMatch(/Silently ignoring an extracted item is not an available outcome/i); + // …and so does the earned-close verification. + expect(fileContent, label).toMatch(/close-claim was earned before treating an issue as/i); + expect(fileContent, label).toMatch(/stop-and-surface event/i); + expect(fileContent, label).toContain('"partially addresses #N"'); + } + }); +}); diff --git a/test/core/templates/workflows/propose-batch.test.ts b/test/core/templates/workflows/propose-batch.test.ts index 73a78b1..0def90e 100644 --- a/test/core/templates/workflows/propose-batch.test.ts +++ b/test/core/templates/workflows/propose-batch.test.ts @@ -3,6 +3,11 @@ import { getProposeBatchSkillTemplate, getRctProposeBatchCommandTemplate, } from '../../../../src/core/templates/workflows/propose-batch.js'; +import { + ISSUE_RECONCILIATION_STEP, + CLOSE_CLAIM_RULES, + STOP_AND_SURFACE_GUARDRAIL, +} from '../../../../src/core/templates/workflows/scope-reconciliation.js'; import { CommandAdapterRegistry } from '../../../../src/core/command-generation/registry.js'; import { generateCommand } from '../../../../src/core/command-generation/generator.js'; import type { CommandContent } from '../../../../src/core/command-generation/types.js'; @@ -115,4 +120,101 @@ describe('propose-batch workflow templates', () => { expect(fileContent, `tool: ${adapter.toolId}`).toMatch(/ratchet batch apply/); } }); + + describe('originating-issue reconciliation (issue #100 criterion 1)', () => { + const body = getProposeBatchSkillTemplate().instructions; + + it('carries the shared reconciliation step verbatim', () => { + expect(body).toContain(ISSUE_RECONCILIATION_STEP); + }); + + it('reconciles before the manifest is scaffolded', () => { + const reconcileAt = body.indexOf( + '4. **Reconcile the manifest against every originating issue (before scaffolding)**' + ); + const scaffoldAt = body.indexOf('5. **Scaffold the manifest via existing machinery (shallow DAG)**'); + + expect(reconcileAt).toBeGreaterThan(-1); + expect(scaffoldAt).toBeGreaterThan(reconcileAt); + expect(body).toMatch(/BEFORE `ratchet new batch` is run/); + expect(body).toMatch(/surface every requirement the\s+manifest leaves uncovered to the user BEFORE the manifest is scaffolded/i); + }); + + it('fetches every issue the objective or a phase originates from', () => { + expect(body).toMatch(/identify every originating issue/i); + expect(body).toMatch(/fetch each originating issue through the project's issue tracker/i); + expect(body).toMatch(/when the objective, a phase, or a change intent originates from a tracked\s+issue/i); + }); + + it('reconciles each phase goal, success criterion, and change-level done', () => { + expect(body).toMatch(/each phase `goal`, each phase `success` criterion, and each change-level\s+`done`/i); + expect(body).toMatch(/map each enumerated requirement onto the phase `goal`, phase `success`, or\s+change-level `done`/i); + }); + + it('forbids self-approving an omission in plan prose', () => { + expect(body).toMatch(/MUST NOT self-approve an omission by writing it into plan prose/i); + }); + }); + + describe('no premature close-claims (issue #100 criterion 2)', () => { + const body = getProposeBatchSkillTemplate().instructions; + + it('forbids hard-coding a close-claim in a goal, a success criterion, or a done', () => { + expect(body).toMatch(/no premature close-claims in the manifest/i); + expect(body).toMatch( + /MUST NOT hard-code\s+`Closes #N` or `Fixes #N` in a phase `goal`, in a phase `success`\s+criterion, or in a change-level `done` for work that has not yet been scoped\s+and verified/i + ); + }); + + it('requires "targets #N" / "addresses #N" phrasing for phase contracts', () => { + expect(body).toContain('**"targets #N"**'); + expect(body).toContain('**"addresses #N"**'); + expect(body).toMatch(/never as a closing claim/i); + }); + + it('requires partial coverage to say "partially addresses #N"', () => { + expect(body).toContain('**"partially addresses #N"**'); + expect(body).toMatch(/MUST NOT say `Fixes #N` or `Closes #N`/); + }); + + it('earns the closing linkage at pull-request-authoring time', () => { + expect(body).toMatch( + /`Closes #N` linkage is earned at pull-request-authoring time, only\s+after the issue's material requirements are confirmed implemented/i + ); + }); + + it('repeats the phrasing rule where the manifest fields are written', () => { + expect(body).toMatch(/\*\*Issue references\*\*/); + expect(body).toMatch(/Never write `Closes #N` or\s+`Fixes #N` into the manifest/i); + }); + + it('embeds the shared close-claim and stop-and-surface guardrails verbatim', () => { + expect(body).toContain(CLOSE_CLAIM_RULES); + expect(body).toContain(STOP_AND_SURFACE_GUARDRAIL); + }); + }); + + it('renders the reconciliation and close-claim rules into every registered tool command', () => { + const cmd = getRctProposeBatchCommandTemplate(); + const content: CommandContent = { + id: 'rct-propose-batch', + name: cmd.name, + description: cmd.description, + category: cmd.category, + tags: cmd.tags, + body: cmd.content, + }; + + const adapters = CommandAdapterRegistry.getAll(); + expect(adapters.length).toBeGreaterThanOrEqual(5); + for (const adapter of adapters) { + const { fileContent } = generateCommand(content, adapter); + const label = `tool: ${adapter.toolId}`; + expect(fileContent, label).toMatch(/fetch each originating issue through the project's issue tracker/i); + expect(fileContent, label).toMatch(/MUST NOT self-approve an omission by writing it into plan prose/i); + expect(fileContent, label).toMatch(/no premature close-claims in the manifest/i); + expect(fileContent, label).toContain('"partially addresses #N"'); + expect(fileContent, label).toMatch(/stop-and-surface event/i); + } + }); }); diff --git a/test/core/templates/workflows/propose.test.ts b/test/core/templates/workflows/propose.test.ts new file mode 100644 index 0000000..2bffc84 --- /dev/null +++ b/test/core/templates/workflows/propose.test.ts @@ -0,0 +1,136 @@ +import { describe, it, expect } from 'vitest'; +import { + getRctProposeSkillTemplate, + getRctProposeCommandTemplate, +} from '../../../../src/core/templates/workflows/propose.js'; +import { ISSUE_RECONCILIATION_STEP } from '../../../../src/core/templates/workflows/scope-reconciliation.js'; +import { CommandAdapterRegistry } from '../../../../src/core/command-generation/registry.js'; +import { generateCommand } from '../../../../src/core/command-generation/generator.js'; +import type { CommandContent } from '../../../../src/core/command-generation/types.js'; + +/** The only two lines that may differ between the skill and command surfaces. */ +const SKILL_INPUT_LINE = + "**Input**: The user's request should include a change name (kebab-case) OR a description of what they want to build."; +const COMMAND_INPUT_LINE = + '**Input**: The argument after `/rct:propose` is the change name (kebab-case), OR a description of what the user wants to build.'; +const SKILL_PROMPT_LINE = + '- Prompt: "Run `/rct:apply` or ask me to implement to start working on the tasks."'; +const COMMAND_PROMPT_LINE = '- Prompt: "Run `/rct:apply` to start implementing."'; + +describe('propose workflow templates', () => { + describe('one shared body, two surfaces', () => { + it('renders the skill and command bodies identically apart from the two known deltas', () => { + // The body used to be duplicated verbatim in both places. It is now one + // builder parameterized by exactly these two lines — this test is what + // keeps a third divergence from creeping back in. + const skillLines = getRctProposeSkillTemplate().instructions.split('\n'); + const commandLines = getRctProposeCommandTemplate().content.split('\n'); + + expect(commandLines.length).toBe(skillLines.length); + + const differing = skillLines + .map((line, i) => ({ line, other: commandLines[i], i })) + .filter(({ line, other }) => line !== other); + + expect(differing.map(({ line }) => line)).toEqual([SKILL_INPUT_LINE, SKILL_PROMPT_LINE]); + expect(differing.map(({ other }) => other)).toEqual([COMMAND_INPUT_LINE, COMMAND_PROMPT_LINE]); + }); + + it('exposes the expected skill and command metadata', () => { + const skill = getRctProposeSkillTemplate(); + expect(skill.name).toBe('ratchet-propose'); + expect(skill.description).toBeTruthy(); + + const command = getRctProposeCommandTemplate(); + expect(command.category).toBe('Workflow'); + expect(command.tags).toEqual(['workflow', 'artifacts', 'experimental']); + }); + }); + + describe('originating-issue reconciliation', () => { + const body = getRctProposeSkillTemplate().instructions; + + it('carries the shared reconciliation step verbatim', () => { + expect(body).toContain(ISSUE_RECONCILIATION_STEP); + }); + + it('runs reconciliation as a numbered step before the change directory is created', () => { + const reconcileAt = body.indexOf('2. **Reconcile the authored scope against every originating issue**'); + const createAt = body.indexOf('3. **Create the change directory**'); + const authorAt = body.indexOf('5. **Create artifacts in sequence until apply-ready**'); + + expect(reconcileAt).toBeGreaterThan(-1); + expect(createAt).toBeGreaterThan(reconcileAt); + expect(authorAt).toBeGreaterThan(createAt); + }); + + it('requires fetching every originating issue before any artifact is written', () => { + expect(body).toMatch(/identify every originating issue/i); + expect(body).toMatch(/fetch each originating issue through the project's issue tracker/i); + expect(body).toMatch(/BEFORE any artifact is written/); + }); + + it('requires enumerating requirements and mapping them to authored artifacts', () => { + expect(body).toMatch(/explicit fix items/i); + expect(body).toMatch(/problems named in its narrative/i); + expect(body).toMatch(/map every enumerated requirement to authored scope/i); + expect(body).toMatch(/list every requirement the authored scope does not cover/i); + }); + + it('forbids self-approving an omission in plan prose', () => { + expect(body).toContain('"issue asks X, this proposal does not include'); + expect(body).toMatch(/MUST NOT self-approve an omission by writing it into plan prose/i); + }); + + it('closes the loop against what was actually authored', () => { + expect(body).toMatch(/complete the reconciliation map from step 2/i); + expect(body).toMatch(/before you call the artifacts done/i); + }); + }); + + describe('neutrality', () => { + const body = getRctProposeSkillTemplate().instructions; + + it('stays agent-neutral (multi-agent-support)', () => { + expect(body.toLowerCase()).toContain('your agent'); + expect(body).toContain('AskUserQuestion'); + expect(body).toMatch(/if your agent has one, otherwise ask in plain prose/i); + // No agent is named in shared template content. + expect(body).not.toMatch(/\bClaude\b/); + }); + + it('stays tracker-neutral (generalizable-defaults)', () => { + expect(body).toMatch(/on github, for example/i); + expect(body).toMatch(/other trackers have their own client/i); + expect(body).toMatch(/ask the user to paste\s+the issue text/i); + }); + }); + + it('renders the reconciliation rules into every registered tool command', () => { + // The command is the genuinely per-tool surface: the shared body is formatted + // into each registered tool's command file via its adapter. Assert the + // reconciliation text survives every adapter's formatting. + const cmd = getRctProposeCommandTemplate(); + const content: CommandContent = { + id: 'rct-propose', + name: cmd.name, + description: cmd.description, + category: cmd.category, + tags: cmd.tags, + body: cmd.content, + }; + + const adapters = CommandAdapterRegistry.getAll(); + expect(adapters.length).toBeGreaterThanOrEqual(5); + for (const adapter of adapters) { + const { fileContent } = generateCommand(content, adapter); + const label = `tool: ${adapter.toolId}`; + expect(fileContent, label).toMatch(/fetch each originating issue through the project's issue tracker/i); + expect(fileContent, label).toMatch(/enumerate the issue's material requirements/i); + // No adapter's rendering drops the prohibition on self-approved omissions. + expect(fileContent, label).toMatch(/MUST NOT self-approve an omission by writing it into plan prose/i); + expect(fileContent, label).toMatch(/stop-and-surface event/i); + expect(fileContent, label).toContain('"partially addresses #N"'); + } + }); +}); diff --git a/test/core/templates/workflows/scope-reconciliation.test.ts b/test/core/templates/workflows/scope-reconciliation.test.ts new file mode 100644 index 0000000..5d780d5 --- /dev/null +++ b/test/core/templates/workflows/scope-reconciliation.test.ts @@ -0,0 +1,149 @@ +import { describe, it, expect } from 'vitest'; +import { + ISSUE_RECONCILIATION_STEP, + CLOSE_CLAIM_RULES, + STOP_AND_SURFACE_GUARDRAIL, +} from '../../../../src/core/templates/workflows/scope-reconciliation.js'; +import { getRctProposeSkillTemplate } from '../../../../src/core/templates/workflows/propose.js'; +import { getProposeBatchSkillTemplate } from '../../../../src/core/templates/workflows/propose-batch.js'; +import { getDecomposePhaseSkillTemplate } from '../../../../src/core/templates/workflows/decompose-phase.js'; + +/** Count non-overlapping occurrences of `needle` in `haystack`. */ +function occurrences(haystack: string, needle: string): number { + return haystack.split(needle).length - 1; +} + +describe('scope-reconciliation shared fragments', () => { + describe('ISSUE_RECONCILIATION_STEP', () => { + const step = ISSUE_RECONCILIATION_STEP; + + it('requires identifying every originating issue from all three sources', () => { + expect(step).toMatch(/identify every originating issue/i); + // The user, the manifest, and the injected `done` each originate issues. + expect(step).toMatch(/the user\s+references it/i); + expect(step).toMatch(/manifest phase or change intent references it/i); + expect(step).toMatch(/injected `done` criterion references it/i); + }); + + it('requires fetching each issue through the project tracker before authoring', () => { + expect(step).toMatch(/fetch each originating issue through the project's issue tracker/i); + expect(step).toMatch(/BEFORE any artifact is written/); + expect(step).toMatch(/never work from a paraphrase/i); + }); + + it('names gh only as a GitHub example and offers a paste fallback', () => { + // generalizable-defaults: no tracker is required and no command is baked in. + expect(step).toContain('gh issue view '); + expect(step).toMatch(/on github, for example/i); + expect(step).toMatch(/other trackers have their own client/i); + expect(step).toMatch(/ask the user to paste\s+the issue text/i); + }); + + it('enumerates material requirements from both fix items and narrative', () => { + expect(step).toMatch(/enumerate the issue's material requirements/i); + expect(step).toMatch(/explicit fix items/i); + expect(step).toMatch(/problems named in its narrative/i); + expect(step).toMatch(/A problem described only in\s+prose is still a requirement/i); + }); + + it('states that hedged wording does not lower the bar', () => { + expect(step).toMatch(/hedged source wording does not lower the bar/i); + expect(step).toContain('"Consider"'); + expect(step).toContain('"maybe"'); + expect(step).toContain('"optionally"'); + expect(step).toMatch(/security-,\s+permission-, or integrity-relevant, severity governs/i); + }); + + it('requires mapping every requirement and listing what is uncovered', () => { + expect(step).toMatch(/map every enumerated requirement to authored scope/i); + expect(step).toMatch(/feature scenario or\s+the plan task that covers it/i); + expect(step).toMatch(/list every requirement the authored scope does not cover/i); + }); + + it('requires the enumerated decision point and forbids self-approval', () => { + expect(step).toMatch(/surface every uncovered requirement to the user as a decision point/i); + expect(step).toContain('"issue asks X, this proposal does not include'); + expect(step).toMatch(/before the artifacts are finalized/i); + expect(step).toMatch(/MUST NOT self-approve an omission by writing it into plan prose/i); + }); + + it('stays agent-neutral with a plain-prose fallback', () => { + // multi-agent-support: AskUserQuestion is optional, never assumed. + expect(step).toContain('AskUserQuestion'); + expect(step).toMatch(/if your agent has one, otherwise ask in plain prose/i); + expect(step).not.toMatch(/\bClaude\b/); + }); + }); + + describe('CLOSE_CLAIM_RULES', () => { + const rules = CLOSE_CLAIM_RULES; + + it('permits a close-claim only when material requirements are implemented', () => { + expect(rules).toContain('`Fixes #N`'); + expect(rules).toContain('`Closes #N`'); + expect(rules).toMatch(/only when the issue's material requirements are actually implemented/i); + }); + + it('covers a manifest done, a plan, and a pull-request body', () => { + expect(rules).toMatch(/batch manifest `done`/i); + expect(rules).toMatch(/in a plan/i); + expect(rules).toMatch(/pull-request body/i); + }); + + it('requires partial work to say "partially addresses #N"', () => { + expect(rules).toContain('"partially addresses #N"'); + expect(rules).toMatch(/MUST NOT say "Fixes #N" or "Closes #N"/); + }); + + it('states a close-claim is an output of verification, never an input of planning', () => { + expect(rules).toMatch(/output of verification, never an input of planning/i); + }); + }); + + describe('STOP_AND_SURFACE_GUARDRAIL', () => { + const guardrail = STOP_AND_SURFACE_GUARDRAIL; + + it('makes a security-relevant de-scope halt the workflow and ask', () => { + expect(guardrail).toMatch(/stop-and-surface event/i); + expect(guardrail).toMatch(/security-,\s+permission-, or integrity-relevant work halts this workflow and asks/i); + expect(guardrail).toMatch(/never taken on momentum/i); + }); + + it('requires an approved deferral to have a filed, owned, linked tracking issue', () => { + expect(guardrail).toMatch(/MUST be explicitly filed with a named owner/i); + expect(guardrail).toMatch(/MUST be linked\s+from the change plan, before you proceed/i); + }); + + it('states a prose bullet in a plan is not a deferral mechanism', () => { + expect(guardrail).toMatch(/prose bullet in `plan\.md` is not a\s+deferral\s+mechanism/i); + }); + }); + + describe('the shared rules are carried by every change-authoring workflow', () => { + const bodies: Array<[string, string]> = [ + ['propose', getRctProposeSkillTemplate().instructions], + ['propose-batch', getProposeBatchSkillTemplate().instructions], + ['decompose-phase', getDecomposePhaseSkillTemplate().instructions], + ]; + + it.each(bodies)('%s embeds the close-claim rule verbatim', (_name, body) => { + expect(body).toContain(CLOSE_CLAIM_RULES); + }); + + it.each(bodies)('%s embeds the stop-and-surface guardrail verbatim', (_name, body) => { + expect(body).toContain(STOP_AND_SURFACE_GUARDRAIL); + }); + + it.each(bodies)('%s embeds each shared rule exactly once, never as a hand-authored copy', (_name, body) => { + // "The guardrail is defined exactly once": a second occurrence would mean a + // body restated the rule instead of interpolating the shared constant. + expect(occurrences(body, CLOSE_CLAIM_RULES)).toBe(1); + expect(occurrences(body, STOP_AND_SURFACE_GUARDRAIL)).toBe(1); + }); + + it('embeds the reconciliation step verbatim in the two issue-reconciling workflows', () => { + expect(getRctProposeSkillTemplate().instructions).toContain(ISSUE_RECONCILIATION_STEP); + expect(getProposeBatchSkillTemplate().instructions).toContain(ISSUE_RECONCILIATION_STEP); + }); + }); +});