From 1f2c5994dcbe3f6c7d9524f6fde2ea44b3f1a33a Mon Sep 17 00:00:00 2001 From: prode Date: Thu, 13 Aug 2026 12:55:16 -0300 Subject: [PATCH] feat(templates): deliver on a branch in the current checkout, not a worktree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The worktree existed for one case — several sessions at once on one repo — and that is the user's own setup, not a step every single-session run should pay for: a directory to create, one to switch into, one to remember to remove, and a stale checkout left behind whenever a run died before the last step. What survives is the line it was really carrying: leave the checkout back on `main` and clean, because that is where the next unit of work starts. delivery.md becomes `git switch -c /`, with the parallel-session worktree named once as the user's to set up. `plan-run` asks three kickoff questions instead of four, and `worktree:` stops being a frontmatter answer: it leaves loopValues, checkLoopAnswers and the `map brief` line. A plan that already recorded `worktree: per-group` keeps validating — the key is ignored, not rejected — and simply stops being displayed. Template set 14 -> 15. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01ECsTMymG9rUZWKdFvU6wMz --- design/orchestration.md | 57 +++++++++++-------- design/plan-format-v2.md | 2 +- design/plan.md | 2 +- design/research.md | 8 ++- internal/artifact/edit.go | 2 +- internal/artifact/edit_test.go | 8 +-- internal/assets/assets.go | 10 +++- .../assets/templates/agents/code-review.md | 2 +- .../templates/agents/security-review.md | 2 +- .../assets/templates/commands/scc-plan-run.md | 3 +- internal/assets/templates/rules/delivery.md | 24 ++++---- .../assets/templates/skills/plan-run/SKILL.md | 35 +++++------- internal/cli/map.go | 2 +- internal/validate/plan.go | 11 ++-- internal/validate/plan_test.go | 10 ++-- 15 files changed, 95 insertions(+), 83 deletions(-) diff --git a/design/orchestration.md b/design/orchestration.md index 741e533..31c4c25 100644 --- a/design/orchestration.md +++ b/design/orchestration.md @@ -481,7 +481,7 @@ rule there is *ask once, before doing anything, and record the answer*; the mist forbids is asking repeatedly, not asking at all. A plan's recorded `autonomy` and `ci` were given when the plan was **written**. Starting a loop that will open and merge pull requests for hours is a different and larger thing to agree to, and it raises a -question authoring never had — worktree per group, or the checkout you are in. +question authoring never had — one pull request per group, or one at the end. So `plan-run` asks three questions, once, **after reporting the groups**, because the answers are only meaningful to someone who can see what they are agreeing to. Anything @@ -492,16 +492,22 @@ them instead of asking again: | Key | Values | Decides | |---|---|---| | `autonomy` | `auto` · `gated` | straight through, or stop at each group boundary | -| `worktree` | `per-group` · `in-place` | a worktree per group, or a branch in the current checkout | +| `pr` | `per-group` · `per-plan` | one pull request per group, or one at the end | | `ci` | `wait` · `no-wait` | whether the checks have to settle before the merge | | `merge` | `auto` · `manual` | whether the loop merges, or stops at the PR for the developer | -`worktree` and `merge` are new, plan-only, and validated by `plan` on exactly the terms +`pr` and `merge` are new, plan-only, and validated by `plan` on exactly the terms `autonomy` and `ci` are — **only when present**, since a plan no loop has run over is not a plan with a defect. They are validated at all because the skill writes them and -every later session reads them, so `worktree: yes` would silently decide how the next +every later session reads them, so `merge: whenever` would silently decide how the next ten groups get built. +There was a fifth key, `worktree: per-group | in-place`, and it is gone. The worktree +existed for one case — several sessions at once on one repo — which is the user's own +setup, and charging every single-session run a directory to create, switch to and +remember to remove (and a stale checkout whenever the run died before the last step) +bought nothing back. §9 is a branch in the checkout you are already in. + The tempting alternative was for the loop to **override** `ci: no-wait`, on the argument that the next group branches from the merge and an unverified base poisons everything after it. Rejected: the argument is sound and the developer is still the one @@ -688,30 +694,35 @@ interference, edit conflicts, resource collisions, and cross-task breakage — a blind to the failure that partial context actually causes. Sequential execution has a second benefit worth naming: nothing needs isolating, so -worktrees, per-agent resource namespacing, merge-conflict resolution, and +separate checkouts, per-agent resource namespacing, merge-conflict resolution, and result attribution all stop being problems rather than being solved. None of this rules out parallelism as such — only *dispatched, task-level* parallelism. Feature-level parallelism is real and supported: the user runs several -Claude Code sessions, one per feature, each in its own worktree, and merges them into -`main`. See §9, which lays out why that version survives the objections above. +Claude Code sessions, one per feature, and merges them into `main`. See §9, which lays +out why that version survives the objections above. -## 9 · Delivery — branch, worktree, PR +## 9 · Delivery — branch, PR Work does not happen on `main` and does not end with a green test run. It ends with a pull request. -### Branch in a worktree — one per session +### Branch in the checkout you are in -Each unit of work — a spec, or a plan's leaf — gets its own branch, developed in its -own **git worktree**. +Each unit of work — a spec, or a plan's leaf — gets its own branch, cut from a green +`main` in the checkout the session is already sitting in, and the checkout goes back to +`main` and clean once the work has landed. That last part is the whole discipline: the +next unit of work starts where this one did. -The reason is that **the user may run several Claude Code sessions at once**, one per -feature, and merge them all into `main` when they land. The worktree is what makes -that possible: session A on `feat/billing` and session B on `feat/auth` each get their -own directory, neither disturbs the other, and neither touches the checkout the user -is actually sitting in. A shared tree with `git switch` cannot do this at all — two -sessions would fight over one working directory. +**A git worktree is not part of this, and used to be.** It was there for one case — +**the user running several Claude Code sessions at once**, one per feature, each +needing a directory of its own, since a shared tree with `git switch` would have two +sessions fighting over one working directory. That case is real and still supported. +But it is the user's setup to make, once, for the runs they actually parallelize, and +making it a step of the ordinary procedure charged every single-session run for it: a +directory to create, one to switch into, one to remember to remove, and a stale +checkout left behind whenever a run died before the last step. What survives is the +line the worktree was really carrying — leave the checkout clean, on `main`. So parallelism is back, and it is worth being precise about why this version is fine when §8's was not. **The user drives this one; the orchestrator drove that one.** Four @@ -732,13 +743,13 @@ within each session the accumulated context prevents it outright. The risk drops ### What running several sessions still costs -Worktrees isolate files. They do not isolate the world outside them, so two of these -concerns from §8 survive at feature granularity and should be said plainly: +Separate checkouts isolate files. They do not isolate the world outside them, so two of +these concerns from §8 survive at feature granularity and should be said plainly: - **Shared external resources.** Two sessions running the suite at the same time will fight over a fixed port, one test database, or a shared temp path. Either the suite - namespaces those per worktree, or the test runs have to be serialized. Nothing about - a worktree fixes this. + namespaces those per checkout, or the test runs have to be serialized. Nothing about + a separate directory fixes this. - **Cross-feature breakage.** Two features green on their own branches can be broken together. Only CI on `main` after the merge sees that — which is a good reason to care about the merge order and about `main` staying green. @@ -783,8 +794,8 @@ stays reproducible from the file, and nobody gets asked twice. - **No remote, or no `gh`** — commit on the branch and stop there, saying so. A branch the user can push themselves is a real deliverable; silently skipping the PR is not. -- **Worktrees left behind accumulate.** Remove the worktree once its branch is merged; - keep it if it still holds unmerged work, and say which. +- **A checkout left dirty or off `main`** is what this shape can leave behind. Say what + is still uncommitted rather than starting the next unit of work on top of it. ## 10 · The three spec artifacts diff --git a/design/plan-format-v2.md b/design/plan-format-v2.md index e9cf466..05b08e4 100644 --- a/design/plan-format-v2.md +++ b/design/plan-format-v2.md @@ -303,7 +303,7 @@ Em plano com `status: approved`: | `patch add` | exige `--reason`; **recusa `--number`** (o scc aloca, §7); exige `--group N` ou `--new-group` | | `patch rm` | **remoção lógica**: reescreve a task com `_Status removed_` + `_Reason`; nunca apaga linhas | | `patch check`/`uncheck` | permitido | -| `patch fm` | permitido (respostas do loop: `pr`, `worktree`, `merge`) | +| `patch fm` | permitido (respostas do loop: `pr`, `merge`) | | `patch task --text/--method/--number` | **recusado** — muda conteúdo funcional | | `patch append`/`prepend`/`replace` | **recusado em plano aprovado** | diff --git a/design/plan.md b/design/plan.md index b6c08ca..fd343a9 100644 --- a/design/plan.md +++ b/design/plan.md @@ -188,7 +188,7 @@ whole point is that a rule is read when it is relevant: | `methodology.md` | §3 — impact analysis first, then Unit or TDD; when TDD is mandatory. | | `tasks.md` | §4 — the grammar, and "independently verifiable" as the size rule. | | `verification.md` | §7 — build, scoped tests, lint, fix; scope not suite. | -| `delivery.md` | §9 — branch in a worktree, review before the PR, the CI question, degrading. | +| `delivery.md` | §9 — branch in the checkout, review before the PR, the CI question, degrading. | | `specs.md` | §10–§11 — EARS, deltas, design scaled by complexity, spec-anchored. | | `knowledge-base.md` | `docs/` — what belongs in the wiki, in an ADR, in the glossary, in the stack. | | `project.md` | The project's own test and lint commands. Shipped as a stub, owned by the user from first edit. This file is why scc needs no config file. | diff --git a/design/research.md b/design/research.md index 8293129..fa53072 100644 --- a/design/research.md +++ b/design/research.md @@ -184,9 +184,11 @@ multi-agent workflows run ~4–7× the tokens of a single-agent session, agent t ### 3.2 Worktree-per-session is the community pattern, caveats included -The published playbooks say what §9 says: one worktree per parallel session, each running -its own tests, diffs reviewed and merged in dependency order. They also independently hit -our §9 resource-collision caveat — recommending `.env.local` per worktree (gitignored) and +The published playbooks agree on the shape §9 supports for *parallel* work: one worktree +per parallel session, each running its own tests, diffs reviewed and merged in dependency +order. (§9 has since stopped making that a step of the ordinary single-session procedure — +the worktree is the user's setup for the runs they actually parallelize.) They also +independently hit our §9 resource-collision caveat — recommending `.env.local` per worktree (gitignored) and explicit port/database isolation, because a committed shared `.env` is read by every worktree. Our "worktrees isolate files, not the world outside them" is the same lesson. diff --git a/internal/artifact/edit.go b/internal/artifact/edit.go index 83287f7..bdeac46 100644 --- a/internal/artifact/edit.go +++ b/internal/artifact/edit.go @@ -406,7 +406,7 @@ func (e *Editor) Prepend(ref, text string) { } // SetFrontmatter writes one key in the leading block, adding the block if the file -// has none. It is how the plan-run loop records its answers — worktree, merge, pr — +// has none. It is how the plan-run loop records its answers — pr, merge, ci — // without the skill having to hold the file. func (e *Editor) SetFrontmatter(key, value string) { if e.err != nil { diff --git a/internal/artifact/edit_test.go b/internal/artifact/edit_test.go index f796276..e4b0c78 100644 --- a/internal/artifact/edit_test.go +++ b/internal/artifact/edit_test.go @@ -289,16 +289,16 @@ func writeAt(path, content string) error { func TestSetFrontmatterKeepsTheOrderAsked(t *testing.T) { _, content := edited(t, func(e *Editor) { e.SetFrontmatter("pr", "per-plan") - e.SetFrontmatter("worktree", "per-group") + e.SetFrontmatter("lang", "en") e.SetFrontmatter("merge", "auto") }) pr := strings.Index(content, "pr: per-plan") - wt := strings.Index(content, "worktree: per-group") + lg := strings.Index(content, "lang: en") mg := strings.Index(content, "merge: auto") - if pr < 0 || wt < 0 || mg < 0 { + if pr < 0 || lg < 0 || mg < 0 { t.Fatalf("a key is missing:\n%s", firstLines(content, 10)) } - if !(pr < wt && wt < mg) { + if !(pr < lg && lg < mg) { t.Errorf("keys came out reversed:\n%s", firstLines(content, 10)) } } diff --git a/internal/assets/assets.go b/internal/assets/assets.go index 8dc5d5b..1cabd84 100644 --- a/internal/assets/assets.go +++ b/internal/assets/assets.go @@ -103,7 +103,15 @@ import ( // file-order one. And the rules stop offering to read the plan at all: `map brief` // once plus `map tasks --next` per task is the whole reading surface, which is what // gives "never read the plan" the authority to be a rule. -const Version = "14" +// 15: delivery is a branch in the checkout you are in, and the worktree is gone from +// the procedure. It was there for one reason — several sessions at once on one repo — +// and that is the user's setup to make, not a step every single-session run pays for: +// a directory to create, one to switch to, and one to remember to remove, with a +// checkout left behind whenever the run dies before the last step. So `plan-run` asks +// three questions instead of four, `worktree:` stops being a frontmatter answer, and +// what survives is the one line the worktree was really carrying — leave the checkout +// back on `main` and clean, because that is where the next unit of work starts. +const Version = "15" // The embedded tree. "all:" so nothing is silently dropped for having a name the // default embed pattern skips. diff --git a/internal/assets/templates/agents/code-review.md b/internal/assets/templates/agents/code-review.md index 7e4cadc..0d2756a 100644 --- a/internal/assets/templates/agents/code-review.md +++ b/internal/assets/templates/agents/code-review.md @@ -24,7 +24,7 @@ git diff --stat main...HEAD line, so re-reading a file to look at them buys nothing. Open a file only when the diff is genuinely not enough to judge a change, only if the diff touches it, and only once — a review that fetches the same source three times spent its context on what it was -handed. When the work is in a worktree, read that path, never the main checkout's copy. +handed. Then read what the work was supposed to be, on the same terms: `scc map ` for its shape and `scc map show
` for the part you need. **The artifact diff --git a/internal/assets/templates/agents/security-review.md b/internal/assets/templates/agents/security-review.md index bed5d5b..3a3fc97 100644 --- a/internal/assets/templates/agents/security-review.md +++ b/internal/assets/templates/agents/security-review.md @@ -29,7 +29,7 @@ issues in untouched code are worth one line at the end, not the body of the revi That scope is also your read budget. The diff carries the changed lines already: open a file only to follow reachability the diff cannot show you, only if the diff touches it, -and once. In a worktree, read that path rather than the main checkout's copy. +and once. ## The method — four passes, in this order diff --git a/internal/assets/templates/commands/scc-plan-run.md b/internal/assets/templates/commands/scc-plan-run.md index 99e553c..e3531d6 100644 --- a/internal/assets/templates/commands/scc-plan-run.md +++ b/internal/assets/templates/commands/scc-plan-run.md @@ -21,8 +21,7 @@ Then take every answer the line above already gave and ask only for what is left most of it; re-asking what someone just typed is the friction that stops people using this at all. Restate what you took so a wrong reading is cheap to correct, then put the remaining questions in one exchange — automatic or gated, one PR at the end or one -per group, a worktree per group or the current checkout, and what happens once a PR is -open. **These are the developer's calls.** Anything the plan's frontmatter already +per group, and what happens once a PR is open. **These are the developer's calls.** Anything the plan's frontmatter already records is a proposed answer to confirm, not a decision already made. The plan is delivered when CI is green on its pull request — never on the strength of diff --git a/internal/assets/templates/rules/delivery.md b/internal/assets/templates/rules/delivery.md index 037089e..7d60a48 100644 --- a/internal/assets/templates/rules/delivery.md +++ b/internal/assets/templates/rules/delivery.md @@ -1,16 +1,15 @@ -# Delivery — branch, worktree, PR +# Delivery — branch, PR Work does not happen on `main` and does not end with a green test run. It ends with a -pull request. Each unit of work gets its own branch in its own worktree: +pull request. Each unit of work gets its own branch, in the checkout you are in: ``` -git worktree add ../- -b / +git switch -c / ``` -The user may run several sessions at once, one per feature, merging them as they land. -The worktree is what makes that possible: each session gets its own directory and none -touches the checkout the user is in. A shared tree with `git switch` cannot — two -sessions would fight over one working directory. +Branch from a green `main`, and leave the checkout back on `main` and clean once the +work lands. Nothing here needs a second directory: a user running several sessions +against one repo may give each its own `git worktree`, and that is theirs to set up. ## Implementation is sequential — you write the code @@ -27,9 +26,10 @@ other way first, and rejected: execution cannot produce this: the later task sees the earlier task's code. Feature-level parallelism has none of that and is supported — a *human* picks the split -and each session has full context. Worktrees isolate files, not the world: suites -fighting over a fixed port or one test database must be namespaced or serialized, and -two features green separately can still break together, which only CI on `main` sees. +and each session has full context. Separate sessions isolate files, not the world: +suites fighting over a fixed port or one test database must be namespaced or +serialized, and two features green separately can still break together, which only CI +on `main` sees. ## The delivery sequence @@ -58,5 +58,5 @@ PR is the finish line. **No remote, or no `gh`** — commit on the branch and stop there, saying so. A branch the user can push themselves is a real deliverable; silently skipping the PR is not. -**Worktrees accumulate** — remove one once its branch is merged, keep it if it holds -unmerged work, and say which you did. +**A checkout left dirty or off `main`** is what this shape can leave behind — say what +is still uncommitted rather than starting the next unit of work on top of it. diff --git a/internal/assets/templates/skills/plan-run/SKILL.md b/internal/assets/templates/skills/plan-run/SKILL.md index 13bc4ca..032b02e 100644 --- a/internal/assets/templates/skills/plan-run/SKILL.md +++ b/internal/assets/templates/skills/plan-run/SKILL.md @@ -53,7 +53,7 @@ once, rather than inventing a decomposition the author did not write. correct cheaply now and expensively after three merges. 3. **Take every answer the invocation already gave, and ask only for what is left.** A prompt like *"implement the whole plan, open one PR at the end, and if CI passes - it is delivered"* has answered three of the four questions in one sentence. + it is delivered"* has answered most of the table below in one sentence. Re-asking what somebody just typed is the friction that stops people using this skill at all. Restate what you took, so a wrong reading is cheap to correct, then ask for the remainder in a single exchange. @@ -62,7 +62,6 @@ once, rather than inventing a decomposition the author did not write. |---|---|---| | Run every group straight through, or stop at each group boundary for review? | automatic · gated | `autonomy: auto` · `gated` | | One PR at the end of the plan, or one per group? | at the end · per group | `pr: per-plan` · `per-group` | -| A git worktree per group, or a branch in the checkout you are already in? | worktree · in place | `worktree: per-group` · `in-place` | | Once a PR is open — wait for CI and merge, merge without waiting, or stop and let me merge? | wait and merge · merge now · stop at the PR | `ci` + `merge`, below | The plan's frontmatter may already carry `autonomy` and `ci` from when it was written. @@ -70,7 +69,7 @@ The plan's frontmatter may already carry `autonomy` and `ci` from when it was wr treat them as decided.** They were given for authoring the plan — this is a loop that will open and merge pull requests for hours, which is a larger thing to agree to. -Say what each answer costs, in one line each, because two of them have a consequence +Say what each answer costs, in one line each, because some of them have a consequence that only shows up later: - **wait and merge** (`ci: wait`, `merge: auto`) — the loop runs to the end unattended. @@ -78,9 +77,6 @@ that only shows up later: branches from a base CI never checked. Say this out loud before accepting it. - **stop at the PR** (`merge: manual`) — the loop pauses after every group until the merge lands. It is no longer unattended, and that is a legitimate thing to want. -- **in place** (`worktree: in-place`) — one directory, and this session cannot run - alongside another on the same repo. Right when the project's setup is expensive to - duplicate; wrong when the user is running several features at once. - **one PR at the end** (`pr: per-plan`) — the cheap shape, and measurably the fast one: the groups become sequential commits on a single branch, the review subagents run once over the whole diff instead of once per group, and CI settles once. What @@ -93,7 +89,7 @@ that only shows up later: again for this plan: ```bash -scc patch fm autonomy=auto ci=wait pr=per-plan worktree=per-group merge=auto +scc patch fm autonomy=auto ci=wait pr=per-plan merge=auto ``` ```yaml @@ -101,7 +97,6 @@ scc patch fm autonomy=auto ci=wait pr=per-plan worktree=per-group merge=a autonomy: auto ci: wait pr: per-plan -worktree: per-group merge: auto --- ``` @@ -128,9 +123,8 @@ you stop to deliver. 1. **Start green.** In the primary checkout, `git switch main && git pull --ff-only`. Every group branches from the previous group's merge, which is the whole reason this is a loop and not a fan-out. -2. **Branch**, in a worktree or in place, as `worktree:` was answered. The worktree - mechanics are `{{.Rules}}/delivery.md`'s; `in-place` means the same - branch without the worktree, and it means you must leave the checkout on `main` +2. **Branch** in the checkout you are in, as `{{.Rules}}/delivery.md` has + it. One directory for the whole run, which means the checkout goes back to `main` and clean when the group ends. 3. **Implement the group, one `--next` at a time.** `scc map tasks --next --group N --json` gives you the one task to do; do it, verify it, tick it, ask @@ -146,9 +140,9 @@ you stop to deliver. settle and fix what is red before merging. `merge: auto` means you merge once that answer is satisfied; `merge: manual` means you open the PR, say where it is, and stop — the loop resumes when the developer's merge is on `main`. -7. **Back to a green main** — `git switch main && git pull --ff-only`. -8. **Remove the worktree** now that its branch has landed, if there was one. -9. **Report the group in one line**, then start the next. +7. **Back to a green main** — `git switch main && git pull --ff-only`, with nothing + uncommitted left behind. The next group branches from there. +8. **Report the group in one line**, then start the next. ### `pr: per-plan` — one pull request at the end @@ -221,11 +215,12 @@ Resuming is a question about state, not about prose, so read it as state: `scc m whole plan to find one unticked box is the cost this loop would otherwise pay every time a session dies. `brief` again only if you have lost what the plan is for. -Which checkout you read that from depends on the shape: +Which branch you read that from depends on the shape: -- **`pr: per-group` — read `main`.** Pull it and map the plan there; the copy in an - old worktree is stale by construction. A group whose boxes are ticked on `main` is - done, as is a task naming a spec whose `tasks.md` is fully ticked there — `scc map +- **`pr: per-group` — read `main`.** Switch to it, pull, and map the plan there; a + half-finished group branch is stale by construction. A group whose boxes are ticked + on `main` is done, as is a task naming a spec whose `tasks.md` is fully ticked + there — `scc map trace specs//` answers that in one call, without opening either file. An open PR means that group is mid-flight; under `merge: manual` that is the expected resting state. Finish it before starting another — two open groups is the fan-out @@ -236,9 +231,7 @@ Which checkout you read that from depends on the shape: but no PR is open, the run died between the last group and the review pass: run the subagents and push. If the PR is open, the run died waiting on CI. -A leftover worktree whose branch is already merged is debris. Remove it. - -A plan whose frontmatter carries no `pr`, `worktree`, or `merge` is a plan no loop has +A plan whose frontmatter carries no `pr` or `merge` is a plan no loop has run over. Ask what the invocation did not already answer. ## Degrading diff --git a/internal/cli/map.go b/internal/cli/map.go index 03bb844..7476169 100644 --- a/internal/cli/map.go +++ b/internal/cli/map.go @@ -291,7 +291,7 @@ func frontmatterLine(a *artifact.Artifact) string { return frontmatterOf(a.Front func frontmatterOf(fm map[string]string) string { var parts []string - for _, k := range []string{"status", "autonomy", "ci", "lang", "pr", "worktree", "merge"} { + for _, k := range []string{"status", "autonomy", "ci", "lang", "pr", "merge"} { if v, ok := fm[k]; ok { parts = append(parts, k+":"+v) } diff --git a/internal/validate/plan.go b/internal/validate/plan.go index a2c0984..a103570 100644 --- a/internal/validate/plan.go +++ b/internal/validate/plan.go @@ -237,12 +237,11 @@ func sectionTitles() string { // Plan-only, and validated on exactly the terms the kickoff answers are: only when // present, because a plan nobody has ever run a loop over is not a plan with a defect. // They are checked at all for one reason — the skill writes them and every later -// session reads them, so `worktree: yes` instead of `per-group` would silently decide -// how the next ten groups get built. +// session reads them, so `merge: whenever` instead of `auto` would silently decide how +// the next ten groups get built. var loopValues = map[string]map[string]bool{ - "worktree": {"per-group": true, "in-place": true}, - "merge": {"auto": true, "manual": true}, - "pr": {"per-group": true, "per-plan": true}, + "merge": {"auto": true, "manual": true}, + "pr": {"per-group": true, "per-plan": true}, } // checkSeal validates the two keys `plan approve` writes. They are checked only when @@ -270,7 +269,7 @@ func checkSeal(set *finding.Set, file string, fm mdscan.Frontmatter) { } func checkLoopAnswers(set *finding.Set, file string, fm mdscan.Frontmatter) { - for _, key := range []string{"worktree", "merge", "pr"} { + for _, key := range []string{"merge", "pr"} { value, ok := fm.Get(key) if !ok { continue diff --git a/internal/validate/plan_test.go b/internal/validate/plan_test.go index 5fdea59..4a27b93 100644 --- a/internal/validate/plan_test.go +++ b/internal/validate/plan_test.go @@ -221,14 +221,14 @@ func TestPlanKickoffAnswers(t *testing.T) { // The answers `plan-run` writes back before it starts a loop. A wrong value is worth // a finding because the skill writes these and every later session reads them — -// `worktree: yes` would quietly decide how the rest of the plan gets built. +// `merge: whenever` would quietly decide how the rest of the plan gets built. func TestPlanLoopAnswers(t *testing.T) { root := t.TempDir() writePlan(t, root, "sweep", - plan("---\nworktree: yes\nmerge: whenever\npr: sometimes\n---\n\n", "- [ ] 1.1 (Unit) Do it\n")) + plan("---\nmerge: whenever\npr: sometimes\n---\n\n", "- [ ] 1.1 (Unit) Do it\n")) got := planFindings(t, root, "sweep") - if n := count(got, "plan.loop-invalid"); n != 3 { - t.Errorf("rules = %v, want three plan.loop-invalid findings, got %d", got, n) + if n := count(got, "plan.loop-invalid"); n != 2 { + t.Errorf("rules = %v, want two plan.loop-invalid findings, got %d", got, n) } } @@ -256,7 +256,7 @@ func TestPlanLoopAnswersAreOptional(t *testing.T) { t.Errorf("rules = %v, want no plan.loop-invalid", got) } writePlan(t, root, "run", - plan("---\nautonomy: auto\nci: wait\npr: per-plan\nworktree: per-group\nmerge: auto\n---\n\n", + plan("---\nautonomy: auto\nci: wait\npr: per-plan\nmerge: auto\n---\n\n", "- [ ] 1.1 (Unit) Do it\n")) if got := planFindings(t, root, "run"); len(got) != 0 { t.Errorf("a plan carrying every valid answer reported %v", got)