diff --git a/docs/hosted-groomer.md b/docs/hosted-groomer.md index 067f09ec..2df16059 100644 --- a/docs/hosted-groomer.md +++ b/docs/hosted-groomer.md @@ -173,9 +173,26 @@ The content excerpts are checked against is what the run's repository-context fe A close that fails any rule is a validation error and applies nothing. The applier re-checks the whole policy, grounding included, against the run's catalog before closing (see [What is applied, and in which order](#what-is-applied-and-in-which-order)); a close that fails there is withheld and the plan lands as backlog. +### Decomposition + +A plan may also split the issue into bounded children instead of (or alongside) promoting it. This is an independent decision from the close and ready promotions: a plan that decomposes is withheld only when its own decomposition policy fails, never because the close or ready policy did. The policy requires, all at once: + +- no close in the same plan — a decomposed parent is not closed; +- verdict confidence of at least `medium` (a low-confidence split is too speculative to fan out); +- no material uncertainty remaining (the same `verdict.uncertainties[].material` flag the close policy keys on); +- every child brief is a **complete bounded implementation brief**: its `problem`, `designDecision`, and `verifiedCurrentBehavior` are all non-blank, and its `relevantPaths`, `inScope`, `outOfScope`, `acceptanceCriteria`, and `tests` each name at least one entry (`dependencies` may be empty). A child brief that leaves any of those short — a design choice left open, no verified current behavior, no relevant paths, no out-of-scope, no acceptance criteria, no tests — is not bounded, so the whole split is withheld with the gap named in `withheld.decomposition` (`child brief[i] is not a complete bounded implementation brief (missing: …)`). This is an apply-time gate: the validator does not police child-brief completeness (a child brief is not an `ImplementationBrief`), so the gate lives in the decomposition policy the applier evaluates. + +When it holds, each bounded child brief in `decomposition.childBriefs` becomes its own GitHub issue, created with the child labels (`status/backlog`). Child creation is idempotent: every child is keyed by a stable `childBriefKey` (repository, parent issue number, and the child brief) recorded in the `GroomingChildClaim` table, so a retried attempt reuses a child an earlier attempt already created and opens only the ones still missing. Once every child exists or is reused, the children step records the parent's decomposition state — `decomposed`, `decomposedAt`, `decomposedBy: "hosted-groomer"`, the decomposition reason as the note, and the created child URLs as its `followUpUrls` — through the same `setDecompositionState` helper the operator `POST /api/issues/actions/decompose` route uses, so both paths write the state and its audit entry identically, and **only then** adds the umbrella label, as the step's final write (an additive `addLabel`, not part of the labels write). The ordering matters: the umbrella label removes the issue from groomer selection on every path (the selector excludes `umbrella`-labeled issues on every path, including a targeted re-groom), so it must be the step's last write; any earlier failure (a child create, the state write) keeps the parent re-selectable, so a partially applied decomposition converges on retry. Two convergence behaviors of the child-claim hold are accepted: a fresh null claim under a different application key is held as another in-flight attempt's claim, so if an LLM re-plan produces a new key shortly after a failed attempt, that child's convergence waits out the active-claim window (about 10 minutes) rather than failing forever; and if the final umbrella label add fails after the decomposition state was recorded, a later retry replays the created children but re-records the decomposition state, which appends another `issue_decomposed` audit entry — accepted as audit noise on an already-rare failure path. + +Alongside creating the children, the step writes a **managed decomposition section** into the parent's body between the `dispatch-groomer:decomposition:start` and `dispatch-groomer:decomposition:end` markers — a short heading, one line that says what the section is, and one `- #: ` line per created-or-reused child, in brief order. The section renders into the body as it stands after the content step, so it **coexists** with the managed enrichment section that step writes — each is parsed and rendered independently. The write only happens when the section content actually changed (no digests, no timestamps), so re-rendering the same children is a byte-for-byte no-op and a well-groomed parent is never churned; the step records the outcome in its detail (`written`, `unchanged`, or `refused`). A body whose markers a human edit broke (unpaired, repeated, or out of order) is **refused rather than guessed at**, and so is a rendered body that would exceed GitHub's body cap (a write that large would fail on every retry and the umbrella would never land) — in either case the step records the refusal reason in its detail, but the children, the state write, and the umbrella still land, so the decomposition converges on retry. + +Three consequences of the managed section are accepted. Re-planning is **last-write-wins**: a fresh plan with different child briefs replaces the section and the recorded `followUpUrls` with the new child set, and children a previous plan already created stay open as `status/backlog` issues, still traceable to the parent by the `Parent:` backlink in their body rather than by any link on the parent. A single well-formed marker pair in the body is treated as **Dispatch-owned** and replaced in place — only a broken or duplicated pair (unpaired, repeated, or out of order) is refused. And because the section's fixed prose makes the parent body permanently non-sparse, a decomposed parent is **never enriched afterwards**: the content step's sparsity gate sees the section's text and skips the write. + +A decomposition that fails any rule is withheld: no child is created and the parent is not decorated, and `withheld.decomposition` records why. + ### Compatibility -`GroomingRun.validatedOutput` stores the full plan. `mutationPlan` keeps its existing fields and adds `planSchemaVersion`, `evidenceDigest`, `readiness`, `closeRecommendation`, `applicationKey`, `preconditions` and, when a policy withheld something, `withheld`. The run path applies mutations through `toGroomerOutput`, the legacy view that `POST /api/groomer/run` still returns as `output` (with the plan alongside as `plan`). A rejected plan's raw output and `validationErrors` are kept on the run. Runs recorded before the plan contract still render on `/automation/groomer`, marked `legacy`, with no readiness claim. +`GroomingRun.validatedOutput` stores the full plan. `mutationPlan` keeps its existing fields and adds `planSchemaVersion`, `evidenceDigest`, `readiness`, `closeRecommendation`, `applicationKey`, `preconditions` and `willCreateChildren` (true when the plan's decomposition children will be created; forced false for in-flight plans, including their dry-run previews, which apply nothing) and, when a policy withheld something, `withheld`. The run path applies mutations through `toGroomerOutput`, the legacy view that `POST /api/groomer/run` still returns as `output` (with the plan alongside as `plan`). A rejected plan's raw output and `validationErrors` are kept on the run. Runs recorded before the plan contract still render on `/automation/groomer`, marked `legacy`, with no readiness claim. ## Applying a plan @@ -207,8 +224,9 @@ The diff is computed against the live issue, never Dispatch's cache, and only wh 1. **labels**: priority/type changes and the derived status. For an `already_done` plan the status stays as it was here. 2. **comment**: at most one, with `@` mentions neutralized and a hidden `` marker at its end. Any marker the model wrote into its own text is stripped, and a marker only counts on a comment by an automation author, so nobody else can forge one to suppress or impersonate a groomer comment. 3. **title/body**: one write. A title is rewritten only when the current one is bad (the existing guard). The body is never replaced: enrichment goes into one Dispatch-managed section between `` and `` markers, appended after the human text on first write and replaced in place afterwards. Text outside the section is kept byte for byte. Enrichment still applies only when the human-authored text is sparse, and a body whose markers are unpaired or repeated is left alone. -4. **close**, only for an `already_done` plan that still satisfies the close policy. -5. **status/done**, only once the close has landed, so a failed close leaves the issue open in its previous (groomable) status rather than open with `status/done`, which the selector would skip forever. +4. **children**, only for a plan that decomposes and still satisfies the decomposition policy (no close in the same plan, at least medium confidence, no material uncertainty, and every child brief a complete bounded implementation brief). Each bounded child brief becomes its own issue (created with the child labels), an already-created child is reused rather than re-opened, and once every child exists or is reused the parent is recorded as decomposed with the child URLs as its follow-ups; the step then writes a managed decomposition section into the parent body (one `- #: ` line per child) and, only then, adds the `umbrella` label — the step's final write (see [Decomposition](#decomposition)). +5. **close**, only for an `already_done` plan that still satisfies the close policy. +6. **status/done**, only once the close has landed, so a failed close leaves the issue open in its previous (groomable) status rather than open with `status/done`, which the selector would skip forever. Each step is recorded as `applied`, `replayed`, `noop`, `skipped` (comment cooldown), `failed` or `not_attempted` in `appliedMutations.steps`, alongside the existing `labelsUpdated`, `titleUpdated`, `bodyUpdated`, `commentUrl`, `commentSkippedReason`, `commentError`, `issueClosed` and `issueClosedError` fields. A run where a later step failed after earlier ones landed ends with `status: "partial"`, `retryable: true` and an `errorMessage` naming the failed step; the grooming fields and freshness baseline record only what actually landed. A run whose first needed write failed (nothing landed) fails as before. @@ -227,7 +245,7 @@ Every run captures its own snapshot, so the key only recurs when the issue, its Dry runs use the same preconditions, diff and policies without writing: `mutationPlan.applyOutcome` is `dry_run`, `stale` or `unverifiable` (with `preconditionFailures`), or `would_replay` when the key was already applied. A dry run never claims a key, so it never reports `busy`, and it never writes a backoff. -Out of scope here, and still to come: worker admission gating on these results (#1065), child issue creation (#1066), and UI exposure of the new history fields (#1067). +Out of scope here, and still to come: worker admission gating on these results (#1065), and UI exposure of the new history fields (#1067). ## History and Audit diff --git a/package-lock.json b/package-lock.json index 47f157ea..e0b5ecac 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1380,9 +1380,9 @@ } }, "node_modules/@img/sharp-darwin-arm64": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-darwin-arm64/-/sharp-darwin-arm64-0.35.4.tgz", - "integrity": "sha512-Uhfl4V4lhP2nbUVF9+hyH1+luj86f1gUFeo8ALYxFoULoU+G87D43BfeMP8XHsk9boxAnCY/bf2EHwhA7MuGsA==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-darwin-arm64/-/sharp-darwin-arm64-0.35.5.tgz", + "integrity": "sha512-QRUlFQ0WxvdWyqqG/WtI3iupfD5rBzmCHXSdPsY91sAtVtTo7Q4cb6zOccZ3gqEqkr0f1As1ehLqmEpDsRf+lg==", "cpu": [ "arm64" ], @@ -1398,13 +1398,13 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-libvips-darwin-arm64": "1.3.3" + "@img/sharp-libvips-darwin-arm64": "1.3.4" } }, "node_modules/@img/sharp-darwin-x64": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-darwin-x64/-/sharp-darwin-x64-0.35.4.tgz", - "integrity": "sha512-hWniXY3bG5qKpkKrAwPe4y+VTPmf086YQAnkxWh7uA1YrlRouWGa0M0Mxj3ZjnXFkv7/TD1bTy9lGUK26vRvWw==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-darwin-x64/-/sharp-darwin-x64-0.35.5.tgz", + "integrity": "sha512-+BR255RhDlpygUpOc/Jdt1nT6DQ3XG/ERo5wbcdOf5Q320dKtPCKPLR1LJs9VGXRaMa8l1uUa0tkCNOXiAxZUw==", "cpu": [ "x64" ], @@ -1420,20 +1420,20 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-libvips-darwin-x64": "1.3.3" + "@img/sharp-libvips-darwin-x64": "1.3.4" } }, "node_modules/@img/sharp-freebsd-wasm32": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-freebsd-wasm32/-/sharp-freebsd-wasm32-0.35.4.tgz", - "integrity": "sha512-lIsKw/BU+kjB4eZjxrYrZmwOJYi3Ajrv66iAlBmUPyKc3HpnloevB1g3wxGD9P/5BbQ1brBGl65VRRrCvQDEqA==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-freebsd-wasm32/-/sharp-freebsd-wasm32-0.35.5.tgz", + "integrity": "sha512-Y/z91nEZ4uIBX5X3nfTovjU9lHNKFYbL2lpHCLVNmXQK03VIZvXBBt0KxbPGp2SdGSF+2mQU4e+hQaWOt86iAw==", "license": "Apache-2.0", "optional": true, "os": [ "freebsd" ], "dependencies": { - "@img/sharp-wasm32": "0.35.4" + "@img/sharp-wasm32": "0.35.5" }, "engines": { "node": ">=20.9.0" @@ -1443,9 +1443,9 @@ } }, "node_modules/@img/sharp-libvips-darwin-arm64": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-darwin-arm64/-/sharp-libvips-darwin-arm64-1.3.3.tgz", - "integrity": "sha512-suTBPTDGrI9WodccaDdwZItTSaBYASlBk1NSfElSHrUfzu3szG6lvIF58+WiFvnfzuK8ZBFS5zE00PxqxnRiPg==", + "version": "1.3.4", + "resolved": "https://registry.npmjs.org/@img/sharp-libvips-darwin-arm64/-/sharp-libvips-darwin-arm64-1.3.4.tgz", + "integrity": "sha512-5R89nBYiRdUlSWJxPhO+GVtaXzXSxKnRu/xqMn3KTA3L9EB9Oy/P+Nn2f2vlhPuUdy/Zusb2DarbyTpGCfEDuw==", "cpu": [ "arm64" ], @@ -1459,9 +1459,9 @@ } }, "node_modules/@img/sharp-libvips-darwin-x64": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-darwin-x64/-/sharp-libvips-darwin-x64-1.3.3.tgz", - "integrity": "sha512-FVJZ5mITMobmXIz/hPDTw0EintTW5H3WfrxwLqEqjiIihlu+hVRyGrFQ60xl0Lxn7Bt3zdpevPaQi0HEzqz9fw==", + "version": "1.3.4", + "resolved": "https://registry.npmjs.org/@img/sharp-libvips-darwin-x64/-/sharp-libvips-darwin-x64-1.3.4.tgz", + "integrity": "sha512-iR2OKH80yi0U+dUplyh3/xdpFvps6YkCwsXenIJxqxR1v9o+xtKTGbS9H7cps+2Vxjc8B1j96p75NmTGjIhtpQ==", "cpu": [ "x64" ], @@ -1475,9 +1475,9 @@ } }, "node_modules/@img/sharp-libvips-linux-arm": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-arm/-/sharp-libvips-linux-arm-1.3.3.tgz", - "integrity": "sha512-3rbU4vqXXc3hY/OiXdl52xZvT0F1yEngWfvqudtPJg/KkyiaQw2DRsFrNzpmLvfavbwOq3qXn36GP8obHRULQA==", + "version": "1.3.4", + "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-arm/-/sharp-libvips-linux-arm-1.3.4.tgz", + "integrity": "sha512-LmRtTsOHuvM2+wlO2Db37dx5MiZhB0FvSunciw48YjdOkZz9KAiRbm8ujeMOA1INqmei5NapFxYEK1D1ZSidmw==", "cpu": [ "arm" ], @@ -1494,9 +1494,9 @@ } }, "node_modules/@img/sharp-libvips-linux-arm64": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-arm64/-/sharp-libvips-linux-arm64-1.3.3.tgz", - "integrity": "sha512-0DaL0A6Xu6sQSQFwe4iVCrKWU2cCTItnRsYsCdxAMm9NF6twAA9BKnoqy4hqz4+azQ0JHuA26qiUKsf1XJ/v5A==", + "version": "1.3.4", + "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-arm64/-/sharp-libvips-linux-arm64-1.3.4.tgz", + "integrity": "sha512-Y3dgX/6lE2QhQb+Gxy0WZxfg9MEm/JBjamZpS2IklP7xIQoKN4hzAm7KcMVGtaVDt3neE9OKBC7vAfonA/Lr1A==", "cpu": [ "arm64" ], @@ -1513,9 +1513,9 @@ } }, "node_modules/@img/sharp-libvips-linux-ppc64": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-ppc64/-/sharp-libvips-linux-ppc64-1.3.3.tgz", - "integrity": "sha512-cdn1OvUBwsXhbC0zSzJnNzf5MZ/mTrobawDvNXBTxe8VtqKAm0sRuEY2Evzovb/w9JMk4TvRxqt1mekSuJz64w==", + "version": "1.3.4", + "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-ppc64/-/sharp-libvips-linux-ppc64-1.3.4.tgz", + "integrity": "sha512-Le6boB8Tai0Nis+gIxIpKx68UDVVIqdR8Tin5Yf1z2LJJQLDJvCDRqRu+jC2qCoD+eIomonmOwB4smBRxfVpYQ==", "cpu": [ "ppc64" ], @@ -1532,9 +1532,9 @@ } }, "node_modules/@img/sharp-libvips-linux-riscv64": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-riscv64/-/sharp-libvips-linux-riscv64-1.3.3.tgz", - "integrity": "sha512-HjPVx7yKz+0lqdhDlTw1tt90wamBoxhiXpvl1XZpJLiHH4RCJ5yDTqH+VlYPv2fwFs89JFw4c1IexYOcQUi4IQ==", + "version": "1.3.4", + "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-riscv64/-/sharp-libvips-linux-riscv64-1.3.4.tgz", + "integrity": "sha512-aHkkIEHPRdQEegJN20MLmGtxYD9R2wQr3Cwpddnu5+YKMt6Uzax7S9h5gpZTo8wyrGuZSlfQ63OevL5mTyOC7Q==", "cpu": [ "riscv64" ], @@ -1551,9 +1551,9 @@ } }, "node_modules/@img/sharp-libvips-linux-s390x": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-s390x/-/sharp-libvips-linux-s390x-1.3.3.tgz", - "integrity": "sha512-neWLh+3yCNThxnfy3c4BbVBeGgt9aftno+XbT56iK28RgeDs3UOFWviLWlUu0bArYVYJaFDK+RRohbicUNCm8Q==", + "version": "1.3.4", + "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-s390x/-/sharp-libvips-linux-s390x-1.3.4.tgz", + "integrity": "sha512-ra/mB6MikESDUO7Yg+Mi95bFBb9GsObURuhnOv3OqknjGe9sZrG8tCe9q0xSIGrtLgvgw0gKnFWcK4blSgQOuQ==", "cpu": [ "s390x" ], @@ -1570,9 +1570,9 @@ } }, "node_modules/@img/sharp-libvips-linux-x64": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-x64/-/sharp-libvips-linux-x64-1.3.3.tgz", - "integrity": "sha512-4vKmvAst9nrowcqquKFAyZJUDolUaIp8uRiN0mWFguJ1IplC9/pitXtlnnlU4aa/eJw3J7i67V+pwUL+wZGdsA==", + "version": "1.3.4", + "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-x64/-/sharp-libvips-linux-x64-1.3.4.tgz", + "integrity": "sha512-GJ//SSXbnwSDes02umB3nDJLFcQzw8a18V8fyhqr6tV515tOEMdImjjxj1AoafMRz56F3PHgftnj1QEKSU1zkw==", "cpu": [ "x64" ], @@ -1589,9 +1589,9 @@ } }, "node_modules/@img/sharp-libvips-linuxmusl-arm64": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linuxmusl-arm64/-/sharp-libvips-linuxmusl-arm64-1.3.3.tgz", - "integrity": "sha512-Y9kQaLMuNoB0bPYOOdcZMaseNrFpPodIWWMrx+CZyydf2xn68j9WYc6sWWRrDwNkzCQjKYfc68L7jKjGlHMibw==", + "version": "1.3.4", + "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linuxmusl-arm64/-/sharp-libvips-linuxmusl-arm64-1.3.4.tgz", + "integrity": "sha512-hvulFwtjUcagsis6BBxHwGFwWoNZjgYmULGVrZcyfNbjA8hKILbRxGg15/7w5HDyXHXUos/j6baAWqnCyQ2DWA==", "cpu": [ "arm64" ], @@ -1608,9 +1608,9 @@ } }, "node_modules/@img/sharp-libvips-linuxmusl-x64": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linuxmusl-x64/-/sharp-libvips-linuxmusl-x64-1.3.3.tgz", - "integrity": "sha512-fj8Mv0HHfD1Rr+4I68+3agJynxDWtBFgicTbSOb9Bke6pIwzGcJ+RX/yHjmiEGFMCavY/dxvem7MyNaJF+wDiw==", + "version": "1.3.4", + "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linuxmusl-x64/-/sharp-libvips-linuxmusl-x64-1.3.4.tgz", + "integrity": "sha512-6zXKeE/p39I1AmA3cJG35eyBGNqNddLnUXjhwBnsGjFPWqf5VKkDBEqaEkPDoTEtkxwi2vv8Tcr2mDyP4So7Fg==", "cpu": [ "x64" ], @@ -1627,9 +1627,9 @@ } }, "node_modules/@img/sharp-linux-arm": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-linux-arm/-/sharp-linux-arm-0.35.4.tgz", - "integrity": "sha512-7OAS8gI0EReKGVN2HssHlM6umJgxF5VI3xN0p9FA91p/YO+ou5hiNghLdZ5BEHztwaaK5+bLKRf8x/o2L2nk9A==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-linux-arm/-/sharp-linux-arm-0.35.5.tgz", + "integrity": "sha512-LEaXK2WdXVK5ykcw0buWyPMsmLLL2vpHLD6yrNSW+JGEL3BZPA4tpKN6iaMc4AxTTAoaX/sU1rOL51lcIz48ZQ==", "cpu": [ "arm" ], @@ -1648,13 +1648,13 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-libvips-linux-arm": "1.3.3" + "@img/sharp-libvips-linux-arm": "1.3.4" } }, "node_modules/@img/sharp-linux-arm64": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-linux-arm64/-/sharp-linux-arm64-0.35.4.tgz", - "integrity": "sha512-De4jpEnAU8Hd5oT0j1G3uL4ZvTuipVMn7YC6vPaJhy6/7EwEae0SVAoBrUMYQbkLGDm85taVWwuPc1a44LTzCQ==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-linux-arm64/-/sharp-linux-arm64-0.35.5.tgz", + "integrity": "sha512-LYVx5JTsOM2CBzmxreh+nl64/3H6Xb09iSLknqH47z2T2DFFxDeFLP5y4dJwe6H7uGQlHPyEEtIqyo3DYsRwdQ==", "cpu": [ "arm64" ], @@ -1673,13 +1673,13 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-libvips-linux-arm64": "1.3.3" + "@img/sharp-libvips-linux-arm64": "1.3.4" } }, "node_modules/@img/sharp-linux-ppc64": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-linux-ppc64/-/sharp-linux-ppc64-0.35.4.tgz", - "integrity": "sha512-2oYZJeIl4kCcMGk4ouZVjnkCtFrpQFlNEtJ6GbxzhHQchwH0NH/qEb9ykmOl29dqwMq+JhFdZn+1ak2FKhI9fQ==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-linux-ppc64/-/sharp-linux-ppc64-0.35.5.tgz", + "integrity": "sha512-QVxAAq8evVRI9ia2vqgwrmWucn5Dfv+JdWzj75pD8omHLPSP7f8p20O8jxzjCcuCEQEOtYOZUmX1hkiZ0kdevA==", "cpu": [ "ppc64" ], @@ -1698,13 +1698,13 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-libvips-linux-ppc64": "1.3.3" + "@img/sharp-libvips-linux-ppc64": "1.3.4" } }, "node_modules/@img/sharp-linux-riscv64": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-linux-riscv64/-/sharp-linux-riscv64-0.35.4.tgz", - "integrity": "sha512-cPbNChoRURAWdebDIHSenxRpgEdy7JkPydSnUxRm9VvKD7m0/xVaR/8Fzlu81pk5nHEvHH87UZUA7cTtwnbJSA==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-linux-riscv64/-/sharp-linux-riscv64-0.35.5.tgz", + "integrity": "sha512-LtdreXguaavKODPIfzJ4kffx7UNt1omwtK0rch4EBbbSTXPnxWmYSayXdLJw0fJzQ97kHt1gL/yh4tvU+nCyRQ==", "cpu": [ "riscv64" ], @@ -1723,13 +1723,13 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-libvips-linux-riscv64": "1.3.3" + "@img/sharp-libvips-linux-riscv64": "1.3.4" } }, "node_modules/@img/sharp-linux-s390x": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-linux-s390x/-/sharp-linux-s390x-0.35.4.tgz", - "integrity": "sha512-RY0JFY8Fd6RonCBtHz+DvadaPkXDSI1AUn6yWL9TipqkZ1vY8w8evqdgyDFnkm4/K1ve1TvZiaePP5oSd4+WVQ==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-linux-s390x/-/sharp-linux-s390x-0.35.5.tgz", + "integrity": "sha512-UZasTOFiYzotTsGOCu42BfUzP6Tu6Do/947iRm1RsLKvlllxwGcn4RN27LibGWceix4Y+Pmw3jsnTcCQIgWjqA==", "cpu": [ "s390x" ], @@ -1748,13 +1748,13 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-libvips-linux-s390x": "1.3.3" + "@img/sharp-libvips-linux-s390x": "1.3.4" } }, "node_modules/@img/sharp-linux-x64": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-linux-x64/-/sharp-linux-x64-0.35.4.tgz", - "integrity": "sha512-9qvvEAuk8k89TfWUoX2htWjbAMX8p+NxCppjpcg5k6xMsjhBQPTsoIh36h9Qde4WRuGpJeYnOjdosDn/cnv+OA==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-linux-x64/-/sharp-linux-x64-0.35.5.tgz", + "integrity": "sha512-SxFtLTeJInhAA9Q836kux2vZNeOBQEx658qvbboZScr0wIARym3IcGmW7KpVD5sbVg0Ojy+udFQdayYIZyoNog==", "cpu": [ "x64" ], @@ -1773,13 +1773,13 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-libvips-linux-x64": "1.3.3" + "@img/sharp-libvips-linux-x64": "1.3.4" } }, "node_modules/@img/sharp-linuxmusl-arm64": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-linuxmusl-arm64/-/sharp-linuxmusl-arm64-0.35.4.tgz", - "integrity": "sha512-KB5jxpfWQTr0nc3xdHtWChdbifHrBGsd2SM62Eyxrl8afikm+f5qGBU75SJIZBT/S1MC8XyacdlXBMSWq6OURA==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-linuxmusl-arm64/-/sharp-linuxmusl-arm64-0.35.5.tgz", + "integrity": "sha512-9HbMclmI1zlNkFRs3z9/eBtDjfD0sGlrX1z6b1qwmiFY5ElDLh4BC0LPBdVp7z1DXFiKlIcznf+ZlsuZzLxQqg==", "cpu": [ "arm64" ], @@ -1798,13 +1798,13 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-libvips-linuxmusl-arm64": "1.3.3" + "@img/sharp-libvips-linuxmusl-arm64": "1.3.4" } }, "node_modules/@img/sharp-linuxmusl-x64": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-linuxmusl-x64/-/sharp-linuxmusl-x64-0.35.4.tgz", - "integrity": "sha512-f+eZJZIQNEEd26RPSW+76chwOf1XtA2Y/O+5ocVyLliHkeih3e+jhLVBdNTd2rS3IbNXK8+ug93Vf5ZXtF5Lxg==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-linuxmusl-x64/-/sharp-linuxmusl-x64-0.35.5.tgz", + "integrity": "sha512-4KOphqB035HrVdqLZfCgMzzERrQkkzOwRhl4OAkRO1YCldbaFjySXMaK534Mo0V+LndnlJk+sbUyLeU0ULyD1A==", "cpu": [ "x64" ], @@ -1823,13 +1823,13 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-libvips-linuxmusl-x64": "1.3.3" + "@img/sharp-libvips-linuxmusl-x64": "1.3.4" } }, "node_modules/@img/sharp-wasm32": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-wasm32/-/sharp-wasm32-0.35.4.tgz", - "integrity": "sha512-zQnl4Kwp7Q6NHsENtU2T/00Zi+w3AQNwz3+UaTyVBy2FpXrzXzGjndpK61onhZjRtRpQXxCTeqw19bVyXOh7jA==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-wasm32/-/sharp-wasm32-0.35.5.tgz", + "integrity": "sha512-Ptsga1su4tQx+LLF1ECS9U6nz5kmrXKo6XVbtR48Ke3ZRxxgaWBu7IDtEe1quo8hiupwm6WFqxVlXaSf7IINGQ==", "license": "Apache-2.0 AND LGPL-3.0-or-later AND MIT", "optional": true, "dependencies": { @@ -1843,16 +1843,16 @@ } }, "node_modules/@img/sharp-webcontainers-wasm32": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-webcontainers-wasm32/-/sharp-webcontainers-wasm32-0.35.4.tgz", - "integrity": "sha512-ESfNkywmCfPNyaZjxooddJQiQ+l/nTpGEOGthxiLnIHXC/CmcBixnfwUleX9mCz9ovrUUvKMap/pm8RYbzfwaA==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-webcontainers-wasm32/-/sharp-webcontainers-wasm32-0.35.5.tgz", + "integrity": "sha512-hfhF/FmoQyTUkA0bIKFOtw536BQSeBMe6BF6QyWlrPxT754+TFLaZ7sKKTfvvM0yJgKgaYTwnFCIZ/GuDw5SUA==", "cpu": [ "wasm32" ], "license": "Apache-2.0", "optional": true, "dependencies": { - "@img/sharp-wasm32": "0.35.4" + "@img/sharp-wasm32": "0.35.5" }, "engines": { "node": ">=20.9.0" @@ -1862,9 +1862,9 @@ } }, "node_modules/@img/sharp-win32-arm64": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-win32-arm64/-/sharp-win32-arm64-0.35.4.tgz", - "integrity": "sha512-iNdlBX9gLVvqe2I3uIJSIKTq6wckP/DYxZtcqxm09x5Gi24DnFBmPAWZmr60ZyYMG0xlzo6goG3670ar+RXvRw==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-win32-arm64/-/sharp-win32-arm64-0.35.5.tgz", + "integrity": "sha512-X4t7g+7ZA5DKblCBEXGjUqqemj4vczING/5viFwAL8h4N3qYeyjwdCvRLHi4EdOUI+2Z7UFlp1VM+p/AuEtm6Q==", "cpu": [ "arm64" ], @@ -1881,9 +1881,9 @@ } }, "node_modules/@img/sharp-win32-ia32": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-win32-ia32/-/sharp-win32-ia32-0.35.4.tgz", - "integrity": "sha512-kqRsbaa5CS6KHlpxnN7WhE6vAAugXyZButpRdvDWetlv6Qv4N9WTcrWzF7tXfB9T7MsoadqdI8hmwLq6UlLvtw==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-win32-ia32/-/sharp-win32-ia32-0.35.5.tgz", + "integrity": "sha512-5Zm82LoBc43nhwNybZlG7Y1KO//Zhsn306fQl29ZOuStHLGTo3BWL83q3cznX0poxSAMuYL1On/BHBxkBeKr6A==", "cpu": [ "ia32" ], @@ -1900,9 +1900,9 @@ } }, "node_modules/@img/sharp-win32-x64": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/@img/sharp-win32-x64/-/sharp-win32-x64-0.35.4.tgz", - "integrity": "sha512-XtmnYhBcrORsJ4XJngyzr/EWP0hRZLAZRFaApdKuviyqF78+ylxh2y06ZmtULAMOnObJ3ucpN0AcwSWnMowTRg==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/@img/sharp-win32-x64/-/sharp-win32-x64-0.35.5.tgz", + "integrity": "sha512-x76eH0vEiHlcMQu8Y8IenntaACtddpT6W0wmXtWrnKcnKI7ME5DdgqhAD6SEWOEl1v2zDvkZDhFA9KnURwpfqg==", "cpu": [ "x64" ], @@ -9440,9 +9440,9 @@ "license": "ISC" }, "node_modules/proxy-addr": { - "version": "2.0.7", - "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.7.tgz", - "integrity": "sha512-llQsMLSUDUPT44jdrU/O37qlnifitDP+ZwrmmZcoSKyLKvtZxpyV0n2/bD/N4tBAAZ/gJEdZU7KMraoK1+XYAg==", + "version": "2.0.8", + "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.8.tgz", + "integrity": "sha512-5nnx0yGyVUcY6t9RnWcARWtwT9F1D8O9rt08htPvnd49W1IgZtmLkhu9WfMzQj1cFxjHIO6connUNVW5k7AVyQ==", "license": "MIT", "dependencies": { "forwarded": "0.2.0", @@ -9450,6 +9450,10 @@ }, "engines": { "node": ">= 0.10" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" } }, "node_modules/punycode": { @@ -10048,9 +10052,9 @@ "license": "ISC" }, "node_modules/sharp": { - "version": "0.35.4", - "resolved": "https://registry.npmjs.org/sharp/-/sharp-0.35.4.tgz", - "integrity": "sha512-n++8XWcj+jCOr2IOl7h8LbKnGBDY4aPbmprMONBNFdn0ImXqpGVv5zliDs0V9HbmbCQLpbuo2ej9rAoOQTvMDA==", + "version": "0.35.5", + "resolved": "https://registry.npmjs.org/sharp/-/sharp-0.35.5.tgz", + "integrity": "sha512-Ywn4OnzGukp7CDMrp08RQ50YKmuwG47brZgIVPTvBaaAfQlRlygrRqSrxdCiL9M+LlzLBiJ68IR1QqvzHyjC7g==", "license": "Apache-2.0", "optional": true, "dependencies": { @@ -10065,31 +10069,31 @@ "url": "https://opencollective.com/libvips" }, "optionalDependencies": { - "@img/sharp-darwin-arm64": "0.35.4", - "@img/sharp-darwin-x64": "0.35.4", - "@img/sharp-freebsd-wasm32": "0.35.4", - "@img/sharp-libvips-darwin-arm64": "1.3.3", - "@img/sharp-libvips-darwin-x64": "1.3.3", - "@img/sharp-libvips-linux-arm": "1.3.3", - "@img/sharp-libvips-linux-arm64": "1.3.3", - "@img/sharp-libvips-linux-ppc64": "1.3.3", - "@img/sharp-libvips-linux-riscv64": "1.3.3", - "@img/sharp-libvips-linux-s390x": "1.3.3", - "@img/sharp-libvips-linux-x64": "1.3.3", - "@img/sharp-libvips-linuxmusl-arm64": "1.3.3", - "@img/sharp-libvips-linuxmusl-x64": "1.3.3", - "@img/sharp-linux-arm": "0.35.4", - "@img/sharp-linux-arm64": "0.35.4", - "@img/sharp-linux-ppc64": "0.35.4", - "@img/sharp-linux-riscv64": "0.35.4", - "@img/sharp-linux-s390x": "0.35.4", - "@img/sharp-linux-x64": "0.35.4", - "@img/sharp-linuxmusl-arm64": "0.35.4", - "@img/sharp-linuxmusl-x64": "0.35.4", - "@img/sharp-webcontainers-wasm32": "0.35.4", - "@img/sharp-win32-arm64": "0.35.4", - "@img/sharp-win32-ia32": "0.35.4", - "@img/sharp-win32-x64": "0.35.4" + "@img/sharp-darwin-arm64": "0.35.5", + "@img/sharp-darwin-x64": "0.35.5", + "@img/sharp-freebsd-wasm32": "0.35.5", + "@img/sharp-libvips-darwin-arm64": "1.3.4", + "@img/sharp-libvips-darwin-x64": "1.3.4", + "@img/sharp-libvips-linux-arm": "1.3.4", + "@img/sharp-libvips-linux-arm64": "1.3.4", + "@img/sharp-libvips-linux-ppc64": "1.3.4", + "@img/sharp-libvips-linux-riscv64": "1.3.4", + "@img/sharp-libvips-linux-s390x": "1.3.4", + "@img/sharp-libvips-linux-x64": "1.3.4", + "@img/sharp-libvips-linuxmusl-arm64": "1.3.4", + "@img/sharp-libvips-linuxmusl-x64": "1.3.4", + "@img/sharp-linux-arm": "0.35.5", + "@img/sharp-linux-arm64": "0.35.5", + "@img/sharp-linux-ppc64": "0.35.5", + "@img/sharp-linux-riscv64": "0.35.5", + "@img/sharp-linux-s390x": "0.35.5", + "@img/sharp-linux-x64": "0.35.5", + "@img/sharp-linuxmusl-arm64": "0.35.5", + "@img/sharp-linuxmusl-x64": "0.35.5", + "@img/sharp-webcontainers-wasm32": "0.35.5", + "@img/sharp-win32-arm64": "0.35.5", + "@img/sharp-win32-ia32": "0.35.5", + "@img/sharp-win32-x64": "0.35.5" }, "peerDependenciesMeta": { "@types/node": { @@ -10216,9 +10220,9 @@ } }, "node_modules/source-map-js": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/source-map-js/-/source-map-js-1.2.1.tgz", - "integrity": "sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA==", + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/source-map-js/-/source-map-js-1.2.2.tgz", + "integrity": "sha512-KGj/8Y43x35aZVDtt+J4mK1hoLGHULMYfSkODJNQjNDC3oW1PqPoxMwo0pLUsWM/UEGzON/NxeHywEfNXNP3Vw==", "license": "BSD-3-Clause", "engines": { "node": ">=0.10.0" diff --git a/prisma/migrations/20261007000000_add_grooming_child_claim/migration.sql b/prisma/migrations/20261007000000_add_grooming_child_claim/migration.sql new file mode 100644 index 00000000..76642768 --- /dev/null +++ b/prisma/migrations/20261007000000_add_grooming_child_claim/migration.sql @@ -0,0 +1,27 @@ +-- One row per child brief the hosted groomer intends for a decomposed parent +-- (#1066). The unique childKey (parent issue + normalized child brief) is +-- the claim: creation is idempotent, so a retry after partial failure reuses +-- children whose rows already carry their URLs and creates only the missing +-- ones. applicationKey records the claiming application: same-key retries are +-- serialized by the GroomingApplication resume CAS, so a fresh null claim +-- under the retry's own key is an abandoned create, not a concurrent holder. +CREATE TABLE "GroomingChildClaim" ( + "id" TEXT NOT NULL, + "childKey" TEXT NOT NULL, + "parentIssueId" TEXT NOT NULL, + "repoFullName" TEXT NOT NULL, + "parentNumber" INTEGER NOT NULL, + "title" TEXT NOT NULL, + "childNumber" INTEGER, + "childUrl" TEXT, + "applicationKey" TEXT, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updatedAt" TIMESTAMP(3) NOT NULL, + + CONSTRAINT "GroomingChildClaim_pkey" PRIMARY KEY ("id") +); + +ALTER TABLE "GroomingChildClaim" ADD CONSTRAINT "GroomingChildClaim_parentIssueId_fkey" FOREIGN KEY ("parentIssueId") REFERENCES "Issue"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +CREATE UNIQUE INDEX "GroomingChildClaim_childKey_key" ON "GroomingChildClaim"("childKey"); +CREATE INDEX "GroomingChildClaim_parentIssueId_idx" ON "GroomingChildClaim"("parentIssueId"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index d918ba58..b1ee22db 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -42,6 +42,7 @@ model Issue { leases Lease[] groomingRuns GroomingRun[] groomingApplications GroomingApplication[] + groomingChildClaims GroomingChildClaim[] // Escalated lane outcome tracking decomposed Boolean @default(false) @@ -604,6 +605,34 @@ model GroomingApplication { @@index([groomingRunId]) } +/// One row per child brief the hosted groomer intends for a decomposed parent +/// (#1066). The unique childKey (parent issue + normalized child brief) is +/// the claim: creation is idempotent, so a retry after partial failure reuses +/// children whose rows already carry their URLs and creates only the missing +/// ones. +model GroomingChildClaim { + id String @id @default(cuid()) + childKey String @unique + parentIssueId String + repoFullName String + parentNumber Int + title String @db.Text + childNumber Int? + childUrl String? @db.Text + /// The application that claimed this child. Same-key attempts are not + /// expected to run concurrently (the GroomingApplication resume CAS + /// serializes fresh claims, and the groomer is confined to a single + /// replica), so a null claim under my key is my own abandoned create. + /// No FK: the application row may be deleted later. + applicationKey String? @db.Text + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt + + parentIssue Issue @relation(fields: [parentIssueId], references: [id], onDelete: Cascade) + + @@index([parentIssueId]) +} + // --------------------------------------------------------------------------- // Issue sync run tracking (for scheduled sync endpoint) // --------------------------------------------------------------------------- diff --git a/src/app/api/issues/actions/decompose/route.test.ts b/src/app/api/issues/actions/decompose/route.test.ts index 57e52adc..9633ead6 100644 --- a/src/app/api/issues/actions/decompose/route.test.ts +++ b/src/app/api/issues/actions/decompose/route.test.ts @@ -4,6 +4,7 @@ import { makeDispatchEnvMock } from "@/test/route-helpers"; const { mocks } = vi.hoisted(() => ({ mocks: { findFirstIssue: vi.fn().mockResolvedValue(null), + findUniqueIssue: vi.fn().mockResolvedValue(null), updateIssue: vi.fn().mockResolvedValue(undefined), createAuditLog: vi.fn().mockResolvedValue({ id: "log-1" }), }, @@ -13,6 +14,7 @@ vi.mock("@/lib/prisma", () => ({ prisma: { issue: { findFirst: mocks.findFirstIssue, + findUnique: mocks.findUniqueIssue, update: mocks.updateIssue, }, auditLog: { @@ -50,14 +52,16 @@ describe("POST /api/issues/actions/decompose — actor attribution", () => { number: 66, labels: ["priority/p1"], }); - mocks.updateIssue.mockResolvedValue({ + const updated = { id: "issue-1", decomposed: true, decomposedAt: new Date(), decomposedBy: "example-agent", decomposedNote: null, followUpUrls: [], - }); + }; + mocks.updateIssue.mockResolvedValue(updated); + mocks.findUniqueIssue.mockResolvedValue(updated); mocks.createAuditLog.mockResolvedValue({ id: "log-1" }); }); @@ -213,14 +217,16 @@ describe("POST /api/issues/actions/decompose — reactivity", () => { number: 66, labels: ["priority/p1"], }); - mocks.updateIssue.mockResolvedValue({ + const updated = { id: "issue-1", decomposed: false, decomposedAt: null, decomposedBy: null, decomposedNote: null, followUpUrls: [], - }); + }; + mocks.updateIssue.mockResolvedValue(updated); + mocks.findUniqueIssue.mockResolvedValue(updated); mocks.createAuditLog.mockResolvedValue({ id: "log-1" }); }); diff --git a/src/app/api/issues/actions/decompose/route.ts b/src/app/api/issues/actions/decompose/route.ts index 1cb69048..d4008eb5 100644 --- a/src/app/api/issues/actions/decompose/route.ts +++ b/src/app/api/issues/actions/decompose/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; +import { setDecompositionState } from "@/lib/decomposition"; import { prisma } from "@/lib/prisma"; import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { resolveActor } from "@/lib/resolve-actor"; @@ -65,41 +66,28 @@ export async function POST(request: Request) { return errorResponse(`Issue #${issueNumber} not found in ${repo}`, 404); } - // Update decomposed state - const updated = await prisma.issue.update({ - where: { id: issue.id }, - data: { - decomposed, - decomposedAt: decomposed ? new Date() : null, - decomposedBy: decomposed ? actor : null, - decomposedNote: note ?? null, - followUpUrls: followUpUrls ?? [], - }, + // Persist the decomposition state and its audit entry through the shared + // helper, so the operator route and the hosted groomer write it identically + // (dispatch#1066). + await setDecompositionState(prisma, { + issue: { id: issue.id, labels: issue.labels }, + repoFullName: `${owner}/${name}`, + issueNumber, + actor, + decomposed, + note: note ?? null, + followUpUrls: followUpUrls ?? [], }); - // Log the action in audit trail - await prisma.auditLog.create({ - data: { - actor, - action: decomposed ? "issue_decomposed" : "issue_reactivated", - repoFullName: `${owner}/${name}`, - issueNumber, - issueId: issue.id, - beforeLabels: [...issue.labels], - afterLabels: [...issue.labels], - success: true, - notes: decomposed - ? `Issue marked as decomposed. Note: ${note ?? "none"}. Follow-up URLs: ${(followUpUrls ?? []).join(", ")}` - : `Issue reactivated (decomposed set to false)`, - }, - }); + // Re-read the issue so the response reflects the persisted state. + const updated = await prisma.issue.findUnique({ where: { id: issue.id } }); return NextResponse.json({ success: true, - issueId: updated.id, - decomposed: updated.decomposed, - decomposedAt: updated.decomposedAt, - followUpUrls: updated.followUpUrls, + issueId: updated?.id ?? issue.id, + decomposed: updated?.decomposed ?? decomposed, + decomposedAt: updated?.decomposedAt ?? null, + followUpUrls: updated?.followUpUrls ?? (followUpUrls ?? []), }, { status: 200 }); } catch (error) { return handleApiError("update decomposed state", error); diff --git a/src/lib/decomposition.test.ts b/src/lib/decomposition.test.ts new file mode 100644 index 00000000..5ea0ca69 --- /dev/null +++ b/src/lib/decomposition.test.ts @@ -0,0 +1,254 @@ +import { describe, expect, it } from "vitest"; +import { + CHILD_ISSUE_LABELS, + UMBRELLA_LABEL, + childBodyMarker, + childBriefKey, + renderChildIssueBody, + setDecompositionState, + type ChildIssueTarget, + type DecompositionStateClient, +} from "./decomposition"; +import type { ChildBrief } from "./groomer/plan"; + +const parent: ChildIssueTarget = { + repoFullName: "acme/storefront", + number: 260, + url: "https://github.com/acme/storefront/issues/260", +}; + +const fullChild: ChildBrief = { + title: "Add order search to the admin dashboard", + problem: "Admins cannot search orders from the dashboard.", + designDecision: "Search lives in a dedicated admin sub-route.", + verifiedCurrentBehavior: "Dashboard.tsx renders four unrelated legacy widgets.", + relevantPaths: ["src/admin/Dashboard.tsx", "src/admin/routes.ts"], + inScope: ["the order search query and results list"], + outOfScope: ["moving the dashboard to the new design system"], + dependencies: ["the refund approval queue child"], + acceptanceCriteria: ["an admin can search orders by id and see the match"], + tests: ["src/admin/orders-search.test.ts"], +}; + +const minimal: ChildBrief = { + title: "Fix redirect after reset", + problem: "Login drops the return URL after a password reset.", + designDecision: null, + verifiedCurrentBehavior: null, + relevantPaths: [], + inScope: [], + outOfScope: [], + dependencies: [], + acceptanceCriteria: ["a reset-then-login test lands on the return URL"], + tests: [], +}; + +describe("labels", () => { + it("uses the umbrella label and starts children in status/backlog", () => { + expect(UMBRELLA_LABEL).toBe("umbrella"); + expect(CHILD_ISSUE_LABELS).toEqual(["status/backlog"]); + }); +}); + +describe("childBriefKey", () => { + it("is stable for the same input", () => { + expect(childBriefKey(parent.repoFullName, parent.number, fullChild)).toBe( + childBriefKey(parent.repoFullName, parent.number, fullChild), + ); + }); + + it("is insensitive to repo case and surrounding whitespace", () => { + expect(childBriefKey(" ACME/Storefront ", 260, fullChild)).toBe(childBriefKey("acme/storefront", 260, fullChild)); + }); + + it("differs for a different parent issue", () => { + expect(childBriefKey(parent.repoFullName, 261, fullChild)).not.toBe(childBriefKey(parent.repoFullName, 260, fullChild)); + }); + + it("keeps the identity of an old-shape brief: missing fields hash as null/empty", () => { + const { + designDecision: _dd, + verifiedCurrentBehavior: _vcb, + relevantPaths: _rp, + inScope: _ins, + outOfScope: _oos, + dependencies: _deps, + tests: _tests, + ...oldShape + } = fullChild; + const oldShapeWithNulls: ChildBrief = { + ...oldShape, + designDecision: null, + verifiedCurrentBehavior: null, + relevantPaths: [], + inScope: [], + outOfScope: [], + dependencies: [], + tests: [], + }; + expect(childBriefKey(parent.repoFullName, parent.number, oldShape as unknown as ChildBrief)).toBe( + childBriefKey(parent.repoFullName, parent.number, oldShapeWithNulls), + ); + }); +}); + +describe("childBodyMarker", () => { + it("embeds the child key in a dispatch-groomer HTML comment", () => { + expect(childBodyMarker("abc123")).toBe(""); + }); +}); + +describe("renderChildIssueBody", () => { + it("puts the marker first, the parent link, and a status/backlog footer", () => { + const body = renderChildIssueBody({ brief: minimal, parent, decompositionReason: null, childKey: "k" }); + expect(body.split("\n")[0]).toBe(""); + expect(body).toContain(`Parent: ${parent.url}`); + expect(body).toContain("This child starts as `status/backlog`"); + }); + + it("omits sections whose content is null or empty", () => { + const body = renderChildIssueBody({ brief: minimal, parent, decompositionReason: null, childKey: "k" }); + for (const heading of [ + "## Verified current behavior", + "## Current relevant code paths", + "## Settled design decision", + "## In scope", + "## Out of scope", + "## Dependencies", + "## Tests", + "Decomposition reason:", + ]) { + expect(body).not.toContain(heading); + } + }); + + it("renders a full brief deterministically, with checklist acceptance criteria", () => { + const input = { brief: fullChild, parent, decompositionReason: "The dashboard bundles four independent features.", childKey: "abc" }; + const expected = ` + +Parent: ${parent.url} + +## Problem +Admins cannot search orders from the dashboard. + +## Verified current behavior +Dashboard.tsx renders four unrelated legacy widgets. + +## Current relevant code paths +- src/admin/Dashboard.tsx +- src/admin/routes.ts + +## Settled design decision +Search lives in a dedicated admin sub-route. + +## In scope +- the order search query and results list + +## Out of scope +- moving the dashboard to the new design system + +## Dependencies +- the refund approval queue child + +## Acceptance criteria +- [ ] an admin can search orders by id and see the match + +## Tests +- src/admin/orders-search.test.ts + +--- +Created by the Dispatch hosted groomer when it decomposed ${parent.url}. +Decomposition reason: The dashboard bundles four independent features. +This child starts as \`status/backlog\`: it needs its own evidence-backed grooming pass before it is worker-ready. Do not implement it directly from this brief.`; + expect(renderChildIssueBody(input)).toBe(expected); + expect(renderChildIssueBody(input)).toBe(expected); + }); + + it("neutralizes @-mentions so model text never carries a live mention", () => { + const brief: ChildBrief = { ...minimal, problem: "Login drops the return URL; asked @user for input." }; + const body = renderChildIssueBody({ brief, parent, decompositionReason: null, childKey: "k" }); + expect(body).toContain("asked `@user` for input"); + }); +}); + +describe("setDecompositionState", () => { + function fakeClient() { + const calls = { update: [] as Record[], create: [] as Record[] }; + const client: DecompositionStateClient = { + issue: { + update: (args) => { + calls.update.push(args.data); + return Promise.resolve({}); + }, + }, + auditLog: { + create: (args) => { + calls.create.push(args.data); + return Promise.resolve({}); + }, + }, + }; + return { client, calls }; + } + + it("marks the issue decomposed and records the audit entry", async () => { + const labels = ["status/backlog", "priority/p2"]; + const { client, calls } = fakeClient(); + await setDecompositionState(client, { + issue: { id: "issue-1", labels }, + repoFullName: "acme/storefront", + issueNumber: 260, + actor: "hosted-groomer", + decomposed: true, + note: "split into four children", + followUpUrls: ["https://github.com/acme/storefront/issues/261"], + }); + labels.push("mutated-after"); + + expect(calls.update).toHaveLength(1); + expect(calls.update[0]).toMatchObject({ + decomposed: true, + decomposedBy: "hosted-groomer", + decomposedNote: "split into four children", + followUpUrls: ["https://github.com/acme/storefront/issues/261"], + }); + expect(calls.update[0]!.decomposedAt).toBeInstanceOf(Date); + expect(calls.create).toHaveLength(1); + expect(calls.create[0]).toMatchObject({ + actor: "hosted-groomer", + action: "issue_decomposed", + repoFullName: "acme/storefront", + issueNumber: 260, + issueId: "issue-1", + beforeLabels: ["status/backlog", "priority/p2"], + afterLabels: ["status/backlog", "priority/p2"], + success: true, + }); + expect(calls.create[0]!.notes).toBe( + "Issue marked as decomposed. Note: split into four children. Follow-up URLs: https://github.com/acme/storefront/issues/261", + ); + }); + + it("reactivates with nulls and the reactivation note", async () => { + const { client, calls } = fakeClient(); + await setDecompositionState(client, { + issue: { id: "issue-1", labels: [] }, + repoFullName: "acme/storefront", + issueNumber: 260, + actor: "operator", + decomposed: false, + note: null, + followUpUrls: [], + }); + + expect(calls.update[0]).toMatchObject({ + decomposed: false, + decomposedAt: null, + decomposedBy: null, + decomposedNote: null, + followUpUrls: [], + }); + expect(calls.create[0]).toMatchObject({ action: "issue_reactivated" }); + expect(calls.create[0]!.notes).toBe("Issue reactivated (decomposed set to false)"); + }); +}); diff --git a/src/lib/decomposition.ts b/src/lib/decomposition.ts new file mode 100644 index 00000000..82b46a4b --- /dev/null +++ b/src/lib/decomposition.ts @@ -0,0 +1,174 @@ +/** + * Shared decomposition helpers for the hosted groomer (dispatch#1066). + * + * When a GroomingPlan requires decomposition, the applier creates one bounded + * child issue per ChildBrief. These helpers are the contract both the + * operator route and the groomer build on: a stable child identity (so child + * creation is idempotent across retries), the child issue body, and the + * parent's decomposition-state persistence with its audit entry. + */ + +import { createHash } from "crypto"; +import type { ChildBrief } from "./groomer/plan"; +import { neutralizeMentions } from "./groomer/sanitize"; + +/** Label that marks an issue as an audit/umbrella parent. */ +export const UMBRELLA_LABEL = "umbrella"; + +/** Labels a created child issue carries; it is not worker-ready until groomed. */ +export const CHILD_ISSUE_LABELS: readonly string[] = ["status/backlog"]; + +/** + * The brief in canonical hashing form: exactly the ten ChildBrief fields in + * interface order, strings trimmed, nulls and missing list items kept as + * null/empty. Missing optional fields therefore hash identically to explicit + * nulls/empty arrays, so a child stored before the brief was widened keeps + * its identity (dispatch#1066). + */ +function canonicalChildBrief(brief: ChildBrief): Record { + // A field stored before the brief was widened is `undefined`, not null; + // both canonicalize to null so old briefs keep their identity. + const text = (value: string | null | undefined): string | null => + value === null || value === undefined ? null : String(value).trim(); + const list = (value: readonly string[] | null | undefined): string[] => (value ?? []).map((item) => String(item).trim()); + return { + title: String(brief.title).trim(), + problem: String(brief.problem).trim(), + designDecision: text(brief.designDecision), + verifiedCurrentBehavior: text(brief.verifiedCurrentBehavior), + relevantPaths: list(brief.relevantPaths), + inScope: list(brief.inScope), + outOfScope: list(brief.outOfScope), + dependencies: list(brief.dependencies), + acceptanceCriteria: list(brief.acceptanceCriteria), + tests: list(brief.tests), + }; +} + +/** + * Stable identity for a child: sha256 over the normalized parent + * (repo lowercased/trimmed + issue number) and the canonical child brief. + * The GroomingChildClaim row is keyed by it, which is what makes child + * creation idempotent. + */ +export function childBriefKey(repoFullName: string, parentIssueNumber: number, brief: ChildBrief): string { + const payload = { + v: 1, + repo: repoFullName.trim().toLowerCase(), + parent: parentIssueNumber, + brief: canonicalChildBrief(brief), + }; + return createHash("sha256").update(JSON.stringify(payload)).digest("hex"); +} + +/** Machine-readable marker that identifies a groomer-created child issue. */ +export function childBodyMarker(childKey: string): string { + return ``; +} + +export interface ChildIssueTarget { + repoFullName: string; + number: number; + url: string; +} + +/** + * Deterministic Markdown body for a groomer-created child issue: the child + * marker first, then the bounded brief, then a footer that tells the next + * groomer this child is not yet worker-ready. Same input, byte-identical + * output. Model text never carries live @-mentions, so the result runs + * through neutralizeMentions. + */ +export function renderChildIssueBody(input: { + brief: ChildBrief; + parent: ChildIssueTarget; + decompositionReason: string | null; + childKey: string; +}): string { + const { brief, parent, decompositionReason, childKey } = input; + const lines: string[] = [childBodyMarker(childKey), "", `Parent: ${parent.url}`]; + + const section = (heading: string, bodyLines: string[]): void => { + if (bodyLines.length === 0) return; + lines.push("", `## ${heading}`, ...bodyLines); + }; + const bullets = (items: readonly string[]): string[] => items.map((item) => `- ${item}`); + + section("Problem", [brief.problem]); + section("Verified current behavior", brief.verifiedCurrentBehavior === null ? [] : [brief.verifiedCurrentBehavior]); + section("Current relevant code paths", bullets(brief.relevantPaths)); + section("Settled design decision", brief.designDecision === null ? [] : [brief.designDecision]); + section("In scope", bullets(brief.inScope)); + section("Out of scope", bullets(brief.outOfScope)); + section("Dependencies", bullets(brief.dependencies)); + section("Acceptance criteria", brief.acceptanceCriteria.map((criterion) => `- [ ] ${criterion}`)); + section("Tests", bullets(brief.tests)); + + const footer: string[] = ["", "---", `Created by the Dispatch hosted groomer when it decomposed ${parent.url}.`]; + if (decompositionReason !== null) footer.push(`Decomposition reason: ${decompositionReason}`); + footer.push( + "This child starts as `status/backlog`: it needs its own evidence-backed grooming pass before it is worker-ready. Do not implement it directly from this brief.", + ); + lines.push(...footer); + + // Trim trailing whitespace (per line and at the end) so the body is + // canonical regardless of how the brief strings were produced. + const rendered = lines.join("\n").replace(/[ \t]+$/gm, "").trim(); + return neutralizeMentions(rendered); +} + +/** + * The slice of the Prisma client the shared decomposition persistence needs; + * the real client satisfies it structurally. + */ +export interface DecompositionStateClient { + issue: { update(args: { where: { id: string }; data: Record }): Promise }; + auditLog: { create(args: { data: Record }): Promise }; +} + +/** + * Persist a parent's decomposition state and record the audit entry + * (dispatch#1066). Shared by the operator decompose route and the hosted + * groomer so decomposition persistence and audit stay in one place. + */ +export async function setDecompositionState( + client: DecompositionStateClient, + input: { + issue: { id: string; labels: readonly string[] }; + repoFullName: string; + issueNumber: number; + actor: string; + decomposed: boolean; + note: string | null; + followUpUrls: string[]; + }, +): Promise { + const { issue, repoFullName, issueNumber, actor, decomposed, note, followUpUrls } = input; + + await client.issue.update({ + where: { id: issue.id }, + data: { + decomposed, + decomposedAt: decomposed ? new Date() : null, + decomposedBy: decomposed ? actor : null, + decomposedNote: note ?? null, + followUpUrls, + }, + }); + + await client.auditLog.create({ + data: { + actor, + action: decomposed ? "issue_decomposed" : "issue_reactivated", + repoFullName, + issueNumber, + issueId: issue.id, + beforeLabels: [...issue.labels], + afterLabels: [...issue.labels], + success: true, + notes: decomposed + ? `Issue marked as decomposed. Note: ${note ?? "none"}. Follow-up URLs: ${followUpUrls.join(", ")}` + : "Issue reactivated (decomposed set to false)", + }, + }); +} diff --git a/src/lib/github-issues.ts b/src/lib/github-issues.ts index 6ade37cf..f8e051c7 100644 --- a/src/lib/github-issues.ts +++ b/src/lib/github-issues.ts @@ -335,8 +335,9 @@ export async function closeIssue( } /** - * Open a new issue. Used by the CI-failure ingester; every other issue in - * Dispatch arrives from GitHub rather than being created by it. + * Open a new issue. Used by the CI-failure ingester and the hosted groomer's + * decomposition children (dispatch#1066); every other issue in Dispatch arrives + * from GitHub rather than being created by it. */ export async function createIssue( repoFullName: string, diff --git a/src/lib/groomer/evals/cases/broad-needs-decomposition.ts b/src/lib/groomer/evals/cases/broad-needs-decomposition.ts index bebee43e..1ef65904 100644 --- a/src/lib/groomer/evals/cases/broad-needs-decomposition.ts +++ b/src/lib/groomer/evals/cases/broad-needs-decomposition.ts @@ -74,5 +74,5 @@ export const broadNeedsDecomposition: GroomingCase = { }, }, ], - pending: [{ name: "accepted child briefs are created once, idempotently", on: "#1066" }], + expectsChildCreation: true, }; diff --git a/src/lib/groomer/evals/corpus.test.ts b/src/lib/groomer/evals/corpus.test.ts index 00f5d8ef..d3c22d10 100644 --- a/src/lib/groomer/evals/corpus.test.ts +++ b/src/lib/groomer/evals/corpus.test.ts @@ -161,7 +161,7 @@ describe("grooming corpus", () => { expect(first.writes.closes).toBe(1); expect(first.writes.comments).toHaveLength(1); expect(replay.mutationPlan?.applicationKey).toBe(first.mutationPlan?.applicationKey); - expect(replay.writes).toEqual({ labels: [], titleBody: [], comments: [], closes: 0 }); + expect(replay.writes).toEqual({ labels: [], titleBody: [], comments: [], closes: 0, children: [] }); }); }); @@ -194,6 +194,26 @@ describe("grooming corpus", () => { }); } + if (c.expectsChildCreation) { + it("creates accepted child briefs once, idempotently (dispatch#1066)", async () => { + const candidate = c.candidates.find( + (x) => + x.expect.accepted === true && + (x.output as GroomingPlanDraft)?.decomposition?.required === true && + (((x.output as GroomingPlanDraft)?.decomposition?.childBriefs.length ?? 0) > 0), + ); + expect(candidate, `[${c.id}] no accepted decomposing candidate to check child creation`).toBeTruthy(); + const briefCount = (candidate!.output as GroomingPlanDraft).decomposition.childBriefs.length; + const applications = new Map(); + const childClaims = new Map(); + const first = await runCandidate(c, candidate!, { applications, childClaims, runId: "run-1" }); + const second = await runCandidate(c, candidate!, { applications, childClaims, runId: "run-2" }); + expect(first.writes.children, `[${c.id}] first run should create one child per brief`).toHaveLength(briefCount); + expect(second.writes.children, `[${c.id}] replay should create no new child`).toHaveLength(0); + expect(childClaims.size, `[${c.id}] one child claim per brief`).toBe(briefCount); + }); + } + for (const pending of c.pending ?? []) { it.todo(`${pending.name} [pending ${pending.on}]`); } diff --git a/src/lib/groomer/evals/drafts.ts b/src/lib/groomer/evals/drafts.ts index 4c7e3f55..ff409a94 100644 --- a/src/lib/groomer/evals/drafts.ts +++ b/src/lib/groomer/evals/drafts.ts @@ -141,11 +141,22 @@ export function alreadyDone( return draft; } -/** Child briefs for decomposition fixtures. */ +/** + * Child briefs for decomposition fixtures. Every field the apply-time + * decomposition policy requires a complete bounded brief to carry is filled in + * (a brief left incomplete would withhold the split, not create children). + */ export function children(titles: string[]): ChildBrief[] { return titles.map((title) => ({ title, problem: `${title}, as its own bounded change.`, + designDecision: `${title} follows the existing pattern; no design choice is left open.`, + verifiedCurrentBehavior: `${title} is not implemented yet; the current code does not cover it.`, + relevantPaths: ["src/admin/Dashboard.tsx"], + inScope: [title.toLowerCase()], + outOfScope: [`${title} does not change unrelated areas`], + dependencies: [], acceptanceCriteria: [`${title} works end to end`], + tests: [`${title} is covered by an automated test`], })); } diff --git a/src/lib/groomer/evals/harness.ts b/src/lib/groomer/evals/harness.ts index cbda4c44..ab3a2b39 100644 --- a/src/lib/groomer/evals/harness.ts +++ b/src/lib/groomer/evals/harness.ts @@ -35,6 +35,7 @@ export interface GitHubWrites { titleBody: Array<{ title?: string; body?: string | null }>; comments: string[]; closes: number; + children: Array<{ number: number; url: string }>; } export interface GroomingOutcome { @@ -112,8 +113,12 @@ function openTrackedIssues(c: GroomingCase, args: unknown) { /** GroomingApplication rows (#1063); share one between runs to exercise replay. */ export type ApplicationRows = Map & { applicationKey: string }>; +/** GroomingChildClaim rows (#1066); share one between runs to exercise child-creation idempotency. */ +export type ChildClaimRows = Map & { childKey: string }>; + export interface RunCandidateOptions { applications?: ApplicationRows; + childClaims?: ChildClaimRows; /** Distinguishes GroomingRun ids when one case is run more than once. */ runId?: string; } @@ -123,7 +128,7 @@ export async function runCandidate( candidate: CaseCandidate, options: RunCandidateOptions = {}, ): Promise { - const writes: GitHubWrites = { labels: [], titleBody: [], comments: [], closes: 0 }; + const writes: GitHubWrites = { labels: [], titleBody: [], comments: [], closes: 0, children: [] }; let validation: GroomingPlanValidationResult | null = null; let catalog: EvidenceCatalog | null = null; let context = ""; @@ -167,6 +172,7 @@ export async function runCandidate( }; const applications: ApplicationRows = options.applications ?? new Map(); + const childClaims: ChildClaimRows = options.childClaims ?? new Map(); const runId = options.runId ?? `run-${c.id}`; const prisma = { automationRepo: { findUnique: async () => ({ id: "repo-1", fullName: c.repoFullName, enabled: true }) }, @@ -209,6 +215,25 @@ export async function runCandidate( return { count: 1 }; }, }, + // Child-issue creation claims (#1066), in memory for this run. + groomingChildClaim: { + findUnique: async ({ where }: { where: { childKey: string } }) => childClaims.get(where.childKey) ?? null, + create: async ({ data }: { data: Record }) => { + const key = String(data.childKey); + // The unique childKey, as Postgres enforces it. + if (childClaims.has(key)) throw Object.assign(new Error("Unique constraint failed on childKey"), { code: "P2002" }); + // A created row has no childNumber/childUrl until the creation is + // recorded; updatedAt is stamped at create, mirroring @updatedAt. + const row = { ...structuredClone(data), childKey: key, childNumber: null, childUrl: null, updatedAt: new Date() }; + childClaims.set(key, row); + return row; + }, + update: async ({ where, data }: { where: { childKey: string }; data: Record }) => { + const row = childClaims.get(where.childKey)!; + Object.assign(row, structuredClone(data)); + return row; + }, + }, }; const deps: GroomerDeps = { @@ -241,6 +266,15 @@ export async function runCandidate( closeIssue: async () => { writes.closes++; }, + createIssue: async (repoFullName, input) => { + const number = 1000 + writes.children.length; + const url = `https://github.com/${repoFullName}/issues/${number}`; + writes.children.push({ number, url }); + return { number, html_url: url }; + }, + // The children step's umbrella: a separate additive write, not part of the + // labels-step writes.labels. + addLabel: async () => {}, findActiveLeases: async () => [], upsertLease: async () => ({ created: true, lease: { id: "lease-1" } }), releaseLease: async () => ({ id: "lease-1" }), diff --git a/src/lib/groomer/evals/types.ts b/src/lib/groomer/evals/types.ts index f655fc7f..44cf84ae 100644 --- a/src/lib/groomer/evals/types.ts +++ b/src/lib/groomer/evals/types.ts @@ -175,4 +175,10 @@ export interface GroomingCase { candidates: CaseCandidate[]; freshness?: FreshnessProbe[]; pending?: PendingBehavior[]; + /** + * Set when an accepted candidate decomposes the issue into bounded children. + * The runner then asserts the child-creation idempotency (dispatch#1066): + * the first run creates one child per brief, an exact replay creates none. + */ + expectsChildCreation?: boolean; } diff --git a/src/lib/groomer/mutation-applier.test.ts b/src/lib/groomer/mutation-applier.test.ts index 613374f1..cf2b83c0 100644 --- a/src/lib/groomer/mutation-applier.test.ts +++ b/src/lib/groomer/mutation-applier.test.ts @@ -1,19 +1,27 @@ import { describe, expect, it, vi } from "vitest"; +import { childBriefKey } from "@/lib/decomposition"; import type { GroomingEvidenceSnapshot } from "./evidence-snapshot"; import { buildEvidenceCatalog } from "./plan-evidence"; import { collectPinnedReadContent } from "./close-grounding"; import { validateGroomingPlan, type GroomingPlan, type GroomingPlanDraft } from "./plan"; import type { LiveComment, LiveIssueState } from "./mutation-validator"; import { + ACTIVE_CLAIM_MS, + DECOMPOSITION_BODY_END, + DECOMPOSITION_BODY_START, MANAGED_BODY_END, MANAGED_BODY_START, applyGroomingMutations, + parseDecompositionBody, + renderDecompositionBody, + renderDecompositionSection, commentMarker, commentMarkerKey, computeApplicationKey, groomerCommentKey, computeMutationDiff, makePrismaApplicationStore, + MAX_GITHUB_BODY_CHARS, parseManagedBody, renderManagedBody, type ApplicationRecord, @@ -21,6 +29,7 @@ import { type ApplierGitHub, type ApplyInput, type ApplySteps, + type ChildClaimRecord, type GroomingMutationDiff, } from "./mutation-applier"; @@ -127,6 +136,38 @@ function planFor(d: GroomingPlanDraft, snap = snapshot()): GroomingPlan { return result.plan!; } +/** A plan that splits the issue into `count` bounded children (dispatch#1066). */ +function decomposeDraft(count = 2): GroomingPlanDraft { + // A ready `implementation` plan may not decompose, so the split lands as a + // non-ready (backlog) verdict. + const letter = (n: number) => String.fromCharCode(64 + n); + return { + ...draft({ actionability: "backlog", lane: { id: "backlog", confidence: "high", reason: "bounded" } }), + implementationBrief: null, + decomposition: { + required: true, + reason: `splits into ${count} bounded children`, + childBriefs: Array.from({ length: count }, (_, i) => { + const l = letter(i + 1); + // Every field the apply-time decomposition policy requires a complete + // bounded brief to carry: an incomplete brief withholds the split. + return { + title: `Child issue ${l}`, + problem: `Child ${l} problem`, + designDecision: `Child ${l} follows the existing pattern; no design choice is left open.`, + verifiedCurrentBehavior: `Child ${l} is not implemented yet; login.ts drops the return URL.`, + relevantPaths: ["src/auth/login.ts"], + inScope: [l], + outOfScope: [`Child ${l} does not touch authentication`], + dependencies: [], + acceptanceCriteria: [`Child ${l} works`], + tests: [`Child ${l} is covered by an automated test`], + }; + }), + }, + }; +} + function live(snap = snapshot()): LiveIssueState { return { title: snap.issue.title, body: snap.issue.body, labels: snap.issue.labels, state: snap.issue.state }; } @@ -177,6 +218,15 @@ describe("managed body section", () => { }); }); +describe("parseDecompositionBody", () => { + it("refuses a section whose end marker precedes its start", () => { + expect(parseDecompositionBody(`${DECOMPOSITION_BODY_END}\nx\n${DECOMPOSITION_BODY_START}`)).toMatchObject({ + ok: false, + reason: "the decomposition section end marker precedes its start", + }); + }); +}); + describe("computeMutationDiff", () => { it("diffs labels from the live issue and leaves the derived status as the only status", () => { const snap = snapshot({ issue: { ...snapshot().issue, labels: ["priority/p1", "status/backlog", "status/needs-review"] } }); @@ -201,6 +251,17 @@ describe("computeMutationDiff", () => { expect(diff.labelsAfter.filter((l) => l.startsWith("status/"))).toEqual(["status/done"]); }); + it("keeps a non-status label in the labels step on the done (close) branch", () => { + // The umbrella is a non-status label; `umbrella` itself is not a model-usable + // label, so a representative allowed non-status label (type/bug) proves the + // done branch preserves non-status labels — the same derivation that keeps + // the umbrella once the decomposition adds it to labelsAfter. + const diff = diffFor(alreadyDone({ labelsToAdd: ["type/bug"] })); + expect(diff.close).toBe(true); + expect(diff.labelsAfter).toContain("type/bug"); + expect(diff.labelsStep).toContain("type/bug"); + }); + it("neutralizes @-mentions in a rewritten title and in the managed body section", () => { const snap = snapshot({ issue: { ...snapshot().issue, title: "P0" } }); const diff = diffFor(draft({}, { proposedTitle: "Ask @alice about the redirect", proposedBody: "cc @bob" }), snap); @@ -288,11 +349,19 @@ describe("computeApplicationKey", () => { // ─── applyGroomingMutations ────────────────────────────────────────────────── -function memoryStore(initial?: ApplicationRecord): ApplicationStore & { rows: Map } { +function memoryStore(initial?: ApplicationRecord): ApplicationStore & { + rows: Map; + children: Map; + decompositionStates: Array<{ labels: readonly string[]; followUpUrls: string[] }>; +} { const rows = new Map(); + const children = new Map(); + const decompositionStates: Array<{ labels: readonly string[]; followUpUrls: string[] }> = []; if (initial) rows.set(initial.applicationKey, initial); return { rows, + children, + decompositionStates, find: async (key) => rows.get(key) ?? null, claim: async (input) => { const existing = rows.get(input.applicationKey) ?? null; @@ -320,29 +389,79 @@ function memoryStore(initial?: ApplicationRecord): ApplicationStore & { rows: Ma row.updatedAt = new Date(); return true; }, + claimChild: async (input) => { + const existing = children.get(input.childKey) ?? null; + if (!existing) { + // Mirrors @updatedAt: the row is stamped at create, so a claim left by + // a crashed attempt is "fresh" until it ages out. + children.set(input.childKey, { + childKey: input.childKey, + childNumber: null, + childUrl: null, + applicationKey: input.applicationKey, + updatedAt: new Date(), + }); + } + return { existing: existing ? { ...existing } : null }; + }, + saveChild: async (childKey, data) => { + const row = children.get(childKey)!; + row.childNumber = data.childNumber; + row.childUrl = data.childUrl; + }, + // The shared helper's persistence is exercised by the route and + // integration tests; the in-memory fake records the call only. + setDecompositionState: async (input) => { + decompositionStates.push({ labels: input.issue.labels, followUpUrls: input.followUpUrls }); + }, }; } function fakeGitHub(overrides: Partial = {}) { const calls: string[] = []; + const bodies: string[] = []; const github: ApplierGitHub = { updateLabels: vi.fn(async (_r, _n, labels: string[]) => { calls.push(`labels:${labels.filter((l) => l.startsWith("status/")).join(",")}`); }), + addLabel: vi.fn(async (_r, _n, label: string) => { + calls.push(`addLabel:${label}`); + }), addComment: vi.fn(async () => { calls.push("comment"); return { url: "https://github.com/org/repo/issues/42#issuecomment-1" }; }), updateTitleAndBody: vi.fn(async (_r, _n, fields) => { calls.push(`content:${Object.keys(fields).join("+")}`); + if (typeof fields.body === "string") bodies.push(fields.body); }), closeIssue: vi.fn(async () => { calls.push("close"); }), + createIssue: vi.fn(async (_r, input) => { + calls.push(`child:${input.title}`); + return { number: 43, url: "https://github.com/org/repo/issues/43" }; + }), fetchRecentComments: vi.fn(async () => []), ...overrides, }; - return { github, calls }; + return { github, calls, bodies }; +} + +/** + * A fakeGitHub whose child creates get a distinct number per brief title + * (the plain fake returns 43 for every child), still recording + * `child:` in `calls` for ordering. + */ +function distinctChildGitHub(overrides: Partial<ApplierGitHub> = {}) { + const base = fakeGitHub(overrides); + const numbers: Record<string, number> = { "Child issue A": 43, "Child issue B": 44, "Child issue C": 45 }; + const createIssue = vi.fn(async (r: string, input: { title: string }) => { + base.calls.push(`child:${input.title}`); + const number = numbers[input.title] ?? 43; + return { number, url: `https://github.com/org/repo/issues/${number}` }; + }); + return { github: { ...base.github, createIssue } as ApplierGitHub, calls: base.calls, bodies: base.bodies, createIssue }; } function applyInput(diff: GroomingMutationDiff, overrides: Partial<ApplyInput> = {}): ApplyInput { @@ -350,6 +469,7 @@ function applyInput(diff: GroomingMutationDiff, overrides: Partial<ApplyInput> = repoFullName: "org/repo", issueNumber: 42, issueId: "issue-42", + parentUrl: "https://github.com/org/repo/issues/42", groomingRunId: "run-1", applicationKey: KEY, diff, @@ -604,6 +724,660 @@ describe("applyGroomingMutations", () => { }); }); +describe("applyGroomingMutations → decomposition", () => { + it("creates each child issue, adds the umbrella via addLabel after all children land, and records the child links", async () => { + const { github, calls } = fakeGitHub(); + const store = memoryStore(); + const diff = diffFor(decomposeDraft()); + expect(diff.children).not.toBeNull(); + // The umbrella is NOT part of the labels step: it is an additive write the + // children step makes via addLabel, so it never rides labelsStep/labelsAfter. + expect(diff.labelsAfter).not.toContain("umbrella"); + expect(diff.labelsStep).not.toContain("umbrella"); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(result.outcome).toBe("applied"); + expect(github.createIssue).toHaveBeenCalledTimes(2); + expect(store.children.size).toBe(2); + // The single labels write carries no umbrella; the children step makes it. + expect(github.updateLabels).toHaveBeenCalledTimes(1); + expect(github.updateLabels).toHaveBeenCalledWith("org/repo", 42, expect.not.arrayContaining(["umbrella"])); + // addLabel is called exactly once, with the umbrella, and only after every + // child was created or reused. + expect(github.addLabel).toHaveBeenCalledTimes(1); + expect(github.addLabel).toHaveBeenCalledWith("org/repo", 42, "umbrella"); + const addLabelIdx = calls.indexOf("addLabel:umbrella"); + const childAIdx = calls.indexOf("child:Child issue A"); + const childBIdx = calls.indexOf("child:Child issue B"); + expect(addLabelIdx).toBeGreaterThan(childAIdx); + expect(addLabelIdx).toBeGreaterThan(childBIdx); + // The children step carries the created child LINKS (key/number/url), not counts. + const step = result.steps.children!; + expect(step.status).toBe("applied"); + expect(step.children?.created).toHaveLength(2); + expect(step.children?.reused).toHaveLength(0); + for (const link of step.children!.created) { + expect(link.key).toBeTypeOf("string"); + expect(link.number).toBeTypeOf("number"); + expect(link.url).toBeTypeOf("string"); + } + // The parent's decomposition state is recorded with the label set at + // state-write time (labelsAfter — the umbrella add lands afterwards, so it + // is not claimed yet) and the child URLs as the follow-ups. + expect(store.decompositionStates).toHaveLength(1); + expect(store.decompositionStates[0].labels).toEqual(diff.labelsAfter); + expect(store.decompositionStates[0].labels).not.toContain("umbrella"); + expect(store.decompositionStates[0].followUpUrls).toHaveLength(2); + // ApplyResult.labels (freshness baseline / audit) includes the umbrella. + expect(result.labels).toContain("umbrella"); + // ApplyResult.children carries the created links in brief order. + expect(result.children).toHaveLength(2); + }); + + it("orders the children step's writes: child creates → decomposition state → body section → umbrella label", async () => { + const { github, calls } = fakeGitHub(); + const store = memoryStore(); + const recordState = store.setDecompositionState; + store.setDecompositionState = async (input) => { + calls.push("decompositionState"); + return recordState(input); + }; + const diff = diffFor(decomposeDraft()); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(result.outcome).toBe("applied"); + const childAIdx = calls.indexOf("child:Child issue A"); + const childBIdx = calls.indexOf("child:Child issue B"); + const stateIdx = calls.indexOf("decompositionState"); + const bodyIdx = calls.indexOf("content:body"); + const addLabelIdx = calls.indexOf("addLabel:umbrella"); + for (const idx of [childAIdx, childBIdx, stateIdx, bodyIdx, addLabelIdx]) expect(idx).toBeGreaterThan(-1); + // Both creates precede the state write; the state write precedes the + // section body write; the umbrella — which removes the parent from every + // selection path — is the step's final write. + expect(stateIdx).toBeGreaterThan(childAIdx); + expect(stateIdx).toBeGreaterThan(childBIdx); + expect(bodyIdx).toBeGreaterThan(stateIdx); + expect(addLabelIdx).toBeGreaterThan(bodyIdx); + }); + + it("reuses a child an earlier attempt already created, creating only the missing one", async () => { + const { github } = fakeGitHub(); + const store = memoryStore(); + const diff = diffFor(decomposeDraft()); + const briefs = diff.children!.briefs; + // Simulate the first child having been created by an earlier attempt. + const firstKey = childBriefKey("org/repo", 42, briefs[0]); + store.children.set(firstKey, { + childKey: firstKey, + childNumber: 99, + childUrl: "https://github.com/org/repo/issues/99", + applicationKey: KEY, + }); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(github.createIssue).toHaveBeenCalledTimes(1); + const step = result.steps.children!; + expect(step.children?.created).toHaveLength(1); + expect(step.children?.reused).toEqual([{ key: firstKey, number: 99, url: "https://github.com/org/repo/issues/99" }]); + // ApplyResult.children is in brief order: the reused first child, then the created one. + expect(result.children.map((l) => l.number)).toEqual([99, 43]); + }); + + it("surfaces the created child links again on a replay, without re-creating them", async () => { + const store = memoryStore(); + const diff = diffFor(decomposeDraft()); + const first = fakeGitHub(); + const applied = await applyGroomingMutations(applyInput(diff), first.github, store); + expect(applied.outcome).toBe("applied"); + expect(applied.children).toHaveLength(2); + // A replay of the same applied key re-surfaces the links and writes nothing. + const second = fakeGitHub(); + const replayed = await applyGroomingMutations(applyInput(diff), second.github, store); + expect(replayed.outcome).toBe("replayed"); + expect(second.github.createIssue).not.toHaveBeenCalled(); + expect(replayed.children).toEqual(applied.children); + }); + + it("creates no children when the decomposition is withheld by policy", async () => { + const { github } = fakeGitHub(); + const store = memoryStore(); + // A low-confidence split is withheld; no children land. + const base = decomposeDraft(); + const diff = diffFor({ ...base, verdict: { ...base.verdict, confidence: "low" } }); + expect(diff.children).toBeNull(); + expect(diff.withheld.decomposition).toBeDefined(); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(github.createIssue).not.toHaveBeenCalled(); + expect(github.addLabel).not.toHaveBeenCalled(); + expect(store.children.size).toBe(0); + expect(store.decompositionStates).toHaveLength(0); + expect(result.steps.children).toMatchObject({ status: "noop" }); + expect(result.labels).not.toContain("umbrella"); + }); + + it("adds no umbrella when the decomposition is withheld for material uncertainty", async () => { + const { github } = fakeGitHub(); + const store = memoryStore(); + // A split with a material uncertainty is withheld; no children and no umbrella. + const base = decomposeDraft(); + const diff = diffFor({ + ...base, + verdict: { ...base.verdict, uncertainties: [{ kind: "scope", question: "Which child owns the migration?", material: true }] }, + }); + expect(diff.children).toBeNull(); + expect(diff.withheld.decomposition).toBeDefined(); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(github.createIssue).not.toHaveBeenCalled(); + expect(github.addLabel).not.toHaveBeenCalled(); + expect(store.children.size).toBe(0); + expect(store.decompositionStates).toHaveLength(0); + expect(result.steps.children).toMatchObject({ status: "noop" }); + expect(result.labels).not.toContain("umbrella"); + }); + + it("does not create a child whose null claim is held by a fresh in-flight attempt", async () => { + const { github } = fakeGitHub(); + const store = memoryStore(); + const diff = diffFor(decomposeDraft()); + const briefs = diff.children!.briefs; + const firstKey = childBriefKey("org/repo", 42, briefs[0]); + // A claim with no recorded child under a DIFFERENT application key, + // written moments ago: another attempt is in flight on it. Do not create a + // duplicate on top of it. + store.children.set(firstKey, { + childKey: firstKey, + childNumber: null, + childUrl: null, + applicationKey: "b".repeat(64), + updatedAt: new Date(), + }); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(result.outcome).toBe("partial"); // the labels step landed, children failed + expect(result.failure).toMatchObject({ step: "children", error: expect.stringContaining(`child claim ${firstKey} is held by another in-flight attempt`) }); + expect(result.steps.children).toMatchObject({ status: "failed" }); + // No create for the held child, and no umbrella on a failed decomposition. + expect(github.createIssue).not.toHaveBeenCalled(); + expect(github.addLabel).not.toHaveBeenCalled(); + expect(result.labels).not.toContain("umbrella"); + }); + + it("creates a child whose fresh null claim is its own (same application key)", async () => { + const { github } = fakeGitHub(); + const store = memoryStore(); + const diff = diffFor(decomposeDraft()); + const briefs = diff.children!.briefs; + const firstKey = childBriefKey("org/repo", 42, briefs[0]); + // A claim with no recorded child under THIS application's own key, written + // moments ago: the GroomingApplication resume CAS already excludes a + // concurrent same-key attempt, so this is my own abandoned create from a + // crashed attempt — create on top of it (the immediate same-key retry). + store.children.set(firstKey, { + childKey: firstKey, + childNumber: null, + childUrl: null, + applicationKey: KEY, + updatedAt: new Date(), + }); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(result.outcome).toBe("applied"); + expect(github.createIssue).toHaveBeenCalledTimes(2); + // The abandoned claim is overwritten with the newly created child. + expect(store.children.get(firstKey)).toMatchObject({ childNumber: 43, childUrl: "https://github.com/org/repo/issues/43" }); + expect(github.addLabel).toHaveBeenCalledTimes(1); + }); + + it("creates a child whose null claim has aged out (no in-flight attempt)", async () => { + const { github } = fakeGitHub(); + const store = memoryStore(); + const diff = diffFor(decomposeDraft()); + const briefs = diff.children!.briefs; + const firstKey = childBriefKey("org/repo", 42, briefs[0]); + // A claim with no recorded child and no applicationKey (a row shape no + // deployed env has), written more than 2×ACTIVE_CLAIM_MS ago: the attempt + // that took it is long gone, so create proceeds. + store.children.set(firstKey, { + childKey: firstKey, + childNumber: null, + childUrl: null, + applicationKey: null, + updatedAt: new Date(Date.now() - 2 * ACTIVE_CLAIM_MS), + }); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(result.outcome).toBe("applied"); + expect(github.createIssue).toHaveBeenCalledTimes(2); + // The stale claim is overwritten with the newly created child. + expect(store.children.get(firstKey)).toMatchObject({ childNumber: 43, childUrl: "https://github.com/org/repo/issues/43" }); + expect(github.addLabel).toHaveBeenCalledTimes(1); + }); + + it("carries the partial children on a failed create, and converges on a retry of the same key", async () => { + const first = fakeGitHub({ + createIssue: vi.fn(async (_r: string, input: { title: string }) => { + if (input.title === "Child issue B") throw new Error("GitHub API error creating issue: 502"); + return { number: 43, url: "https://github.com/org/repo/issues/43" }; + }), + }); + const store = memoryStore(); + const diff = diffFor(decomposeDraft(3)); + const attempt = await applyGroomingMutations(applyInput(diff), first.github, store); + expect(attempt.outcome).toBe("partial"); // the labels step landed + const step = attempt.steps.children!; + expect(step.status).toBe("failed"); + expect(step.error).toContain("502"); + // The failed step record carries the child that landed before the failure. + expect(step.children?.created).toHaveLength(1); + expect(step.children?.created[0].number).toBe(43); + expect(attempt.failure).toMatchObject({ step: "children" }); + // A partial decomposition never lands the umbrella. + expect(first.github.addLabel).not.toHaveBeenCalled(); + + // A retry under the same application key (no hand-seeded rows): child A + // is reused; child B's fresh null claim carries THIS key, which the + // GroomingApplication resume CAS proves is not a concurrent holder, so the + // held-claim guard does not fire and only the missing children are created, + // and the umbrella lands at that point. + const retry = fakeGitHub(); + const converged = await applyGroomingMutations(applyInput(diff), retry.github, store); + expect(converged.outcome).toBe("applied"); + expect(retry.github.createIssue).toHaveBeenCalledTimes(2); // B and C, not A + expect(retry.github.createIssue).not.toHaveBeenCalledWith("org/repo", expect.objectContaining({ title: "Child issue A" })); + const retriedStep = converged.steps.children!; + expect(retriedStep.children?.created).toHaveLength(2); + expect(retriedStep.children?.reused).toHaveLength(1); + expect(retry.github.addLabel).toHaveBeenCalledTimes(1); + expect(retry.github.addLabel).toHaveBeenCalledWith("org/repo", 42, "umbrella"); + }); + + it("surfaces the child links again when a resumed partial application replays its landed children step", async () => { + const created = [{ key: "k-a", number: 43, url: "https://github.com/org/repo/issues/43" }]; + const reused = [{ key: "k-b", number: 44, url: "https://github.com/org/repo/issues/44" }]; + // The children step landed in the prior attempt (with its links recorded) + // but the application never finished: the resume replays the step instead + // of re-creating the children, and the result still carries the links. + const store = memoryStore({ + applicationKey: KEY, + groomingRunId: "run-0", + status: "partial", + steps: { children: { status: "applied", children: { created, reused } } }, + attempts: 1, + updatedAt: new Date(Date.now() - 11 * 60 * 1000), + }); + const { github } = fakeGitHub(); + const result = await applyGroomingMutations(applyInput(diffFor(decomposeDraft())), github, store); + expect(result.outcome).not.toBe("busy"); + expect(result.steps.children).toMatchObject({ status: "replayed" }); + expect(github.createIssue).not.toHaveBeenCalled(); + expect(result.children).toEqual([...created, ...reused]); + }); + + it("reuses every child under a different application key, creating no duplicates", async () => { + const first = fakeGitHub(); + const store = memoryStore(); + const diff = diffFor(decomposeDraft()); + const applied = await applyGroomingMutations(applyInput(diff), first.github, store); + expect(applied.outcome).toBe("applied"); + expect(first.github.createIssue).toHaveBeenCalledTimes(2); + + // A different application (a re-plan under a new key) that plans the same + // children: the child claims from the first run are reused, no duplicates. + const otherKey = "b".repeat(64); + const second = fakeGitHub(); + const reused = await applyGroomingMutations(applyInput(diff, { applicationKey: otherKey, groomingRunId: "run-2" }), second.github, store); + expect(reused.outcome).toBe("applied"); + expect(second.github.createIssue).not.toHaveBeenCalled(); + expect(reused.steps.children?.children?.reused).toHaveLength(2); + expect(reused.steps.children?.children?.created).toHaveLength(0); + expect(second.github.addLabel).toHaveBeenCalledTimes(1); + }); + + it("computes an identical application key for the same children in a different brief order", () => { + const base = decomposeDraft(); + const keyA = computeApplicationKey({ + repoFullName: "org/repo", + issueNumber: 42, + plan: planFor(base), + diff: diffFor(base), + }); + // The same set of children, reordered: the application key is stable. + const reordered: GroomingPlanDraft = { ...base, decomposition: { ...base.decomposition, childBriefs: [...base.decomposition.childBriefs].reverse() } }; + const keyB = computeApplicationKey({ + repoFullName: "org/repo", + issueNumber: 42, + plan: planFor(reordered), + diff: diffFor(reordered), + }); + expect(keyB).toBe(keyA); + // A changed brief (a new title) is a different application. + const changed: GroomingPlanDraft = { + ...base, + decomposition: { ...base.decomposition, childBriefs: base.decomposition.childBriefs.map((b, i) => (i === 0 ? { ...b, title: "A renamed child" } : b)) }, + }; + const keyC = computeApplicationKey({ + repoFullName: "org/repo", + issueNumber: 42, + plan: planFor(changed), + diff: diffFor(changed), + }); + expect(keyC).not.toBe(keyA); + }); +}); + +describe("applyGroomingMutations → decomposition body section", () => { + it("writes one parent body whose section lists every child, keeping the human text byte for byte", async () => { + const { github, bodies } = distinctChildGitHub(); + const store = memoryStore(); + const diff = diffFor(decomposeDraft()); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(result.outcome).toBe("applied"); + // One parent body write for the whole application: the children step's section. + expect(bodies).toHaveLength(1); + expect(github.updateTitleAndBody).toHaveBeenCalledTimes(1); + const body = bodies[0]; + // One managed section: the parse succeeding means exactly one of each marker. + const parsed = parseDecompositionBody(body); + expect(parsed.ok).toBe(true); + if (!parsed.ok) throw new Error("unreachable"); + expect(parsed.decomposition).toContain("## Decomposition"); + // One "- #<n>: <url>" line per child, in brief order. + expect(body).toContain("- #43: https://github.com/org/repo/issues/43"); + expect(body).toContain("- #44: https://github.com/org/repo/issues/44"); + for (const child of result.children) expect(body).toContain(`- #${child.number}: ${child.url}`); + // The original human text is preserved byte for byte before the appended section. + expect(body.startsWith("Broken.\n\n")).toBe(true); + expect(parsed.human).toBe("Broken."); + // Re-rendering the same children is a fixed point. + expect(renderDecompositionBody(parsed, renderDecompositionSection(result.children))).toBe(body); + // The step's detail records the counts and the section outcome; the result + // reports the real post-apply body. + expect(result.steps.children?.detail).toBe("2 created, 0 reused; decomposition section written"); + expect(result.body).toBe(body); + }); + + it("coexists with the managed enrichment section: the children step renders into the post-content body", async () => { + const base = decomposeDraft(); + const diff = diffFor({ ...base, mutations: { ...base.mutations, proposedBody: "Notes." } }); + expect(diff.body).not.toBeNull(); + const { github, calls, bodies } = distinctChildGitHub(); + const result = await applyGroomingMutations(applyInput(diff), github, memoryStore()); + expect(result.outcome).toBe("applied"); + // Two parent body writes: the content step's enrichment, then the children + // step's section rendered into the body as it stands after that write. + expect(bodies).toHaveLength(2); + const firstBodyIdx = calls.indexOf("content:body"); + const lastBodyIdx = calls.lastIndexOf("content:body"); + expect(lastBodyIdx).toBeGreaterThan(firstBodyIdx); + expect(calls.indexOf("addLabel:umbrella")).toBeGreaterThan(lastBodyIdx); + // The first is the managed enrichment section alone. + expect(bodies[0]).toBe(`Broken.\n\n${MANAGED_BODY_START}\nNotes.\n${MANAGED_BODY_END}`); + expect(bodies[0]).not.toContain(DECOMPOSITION_BODY_START); + // The last is both sections: the enrichment section kept, the decomposition + // section appended after it. + const section = + "## Decomposition\n\n" + + "Created from this parent's grooming decomposition plan. Each child starts `status/backlog` and needs its own grooming pass before pickup.\n\n" + + "- #43: https://github.com/org/repo/issues/43\n" + + "- #44: https://github.com/org/repo/issues/44"; + const expected = `${bodies[0]}\n\n${DECOMPOSITION_BODY_START}\n${section}\n${DECOMPOSITION_BODY_END}`; + expect(bodies[bodies.length - 1]).toBe(expected); + // The human text survives both managed sections. + const managed = parseManagedBody(bodies[bodies.length - 1]); + expect(managed.ok).toBe(true); + if (!managed.ok) throw new Error("unreachable"); + expect(managed.managed).toBe("Notes."); + const decomp = parseDecompositionBody(managed.human); + expect(decomp.ok).toBe(true); + if (!decomp.ok) throw new Error("unreachable"); + expect(decomp.human).toBe("Broken."); + expect(result.body).toBe(bodies[bodies.length - 1]); + expect(result.steps.children?.detail).toBe("2 created, 0 reused; decomposition section written"); + }); + + it("writes no body when the section already lists the same children (a fixed point), and still lands the umbrella", async () => { + const { github, bodies } = distinctChildGitHub(); + const store = memoryStore(); + const briefs = decomposeDraft().decomposition.childBriefs; + // Every child was already created by an earlier application. + const links = [ + { key: childBriefKey("org/repo", 42, briefs[0]), number: 99, url: "https://github.com/org/repo/issues/99" }, + { key: childBriefKey("org/repo", 42, briefs[1]), number: 100, url: "https://github.com/org/repo/issues/100" }, + ]; + for (const link of links) { + store.children.set(link.key, { + childKey: link.key, + childNumber: link.number, + childUrl: link.url, + applicationKey: KEY, + }); + } + // The live body already carries the section rendered from those same links. + const parsed = parseDecompositionBody("Broken."); + if (!parsed.ok) throw new Error("unreachable"); + const liveBody = renderDecompositionBody(parsed, renderDecompositionSection(links)); + const diff = diffFor(decomposeDraft(), snapshot({ issue: { ...snapshot().issue, body: liveBody } })); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(result.outcome).toBe("applied"); + // Re-rendering the same section is a fixed point: no body write at all. + expect(bodies).toHaveLength(0); + expect(github.updateTitleAndBody).not.toHaveBeenCalled(); + // The post-apply body is the (unchanged) live body. + expect(result.body).toBe(liveBody); + // The children are reused, the state is recorded, and the umbrella still lands. + expect(result.steps.children?.detail).toBe("0 created, 2 reused; decomposition section unchanged"); + expect(result.children.map((l) => l.number)).toEqual([99, 100]); + expect(store.decompositionStates).toHaveLength(1); + expect(store.decompositionStates[0].followUpUrls).toEqual(["https://github.com/org/repo/issues/99", "https://github.com/org/repo/issues/100"]); + expect(github.addLabel).toHaveBeenCalledWith("org/repo", 42, "umbrella"); + }); + + it("refuses a malformed section (two start markers): no body write, the umbrella still lands, and the detail says so", async () => { + const { github, bodies } = distinctChildGitHub(); + const store = memoryStore(); + // A human edit broke the section markers: two starts, one end. + const liveBody = + `Broken.\n\n` + + `${DECOMPOSITION_BODY_START}\n- #99: https://github.com/org/repo/issues/99\n${DECOMPOSITION_BODY_END}\n\n` + + `${DECOMPOSITION_BODY_START}\n- #100: https://github.com/org/repo/issues/100`; + const snap = snapshot({ issue: { ...snapshot().issue, body: liveBody } }); + const diff = diffFor(decomposeDraft(), snap); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(result.outcome).toBe("applied"); + // The section write is refused, not guessed at: the body is untouched. + expect(bodies).toHaveLength(0); + expect(github.updateTitleAndBody).not.toHaveBeenCalled(); + expect(result.body).toBe(liveBody); + // The children, the state and the umbrella still land. + expect(github.createIssue).toHaveBeenCalledTimes(2); + expect(store.decompositionStates).toHaveLength(1); + expect(github.addLabel).toHaveBeenCalledWith("org/repo", 42, "umbrella"); + expect(result.steps.children).toMatchObject({ status: "applied" }); + expect(result.steps.children?.detail).toBe( + "2 created, 0 reused; decomposition section write refused: the decomposition section markers are unpaired or repeated", + ); + }); + + it("records the created/reused counts and the section outcome on the step detail in all three outcomes", async () => { + // Written. + const written = await applyGroomingMutations( + applyInput(diffFor(decomposeDraft())), + distinctChildGitHub().github, + memoryStore(), + ); + expect(written.steps.children?.detail).toBe("2 created, 0 reused; decomposition section written"); + // Unchanged: every child reused, section already current. + const briefs = decomposeDraft().decomposition.childBriefs; + const store = memoryStore(); + const links = [ + { key: childBriefKey("org/repo", 42, briefs[0]), number: 99, url: "https://github.com/org/repo/issues/99" }, + { key: childBriefKey("org/repo", 42, briefs[1]), number: 100, url: "https://github.com/org/repo/issues/100" }, + ]; + for (const link of links) { + store.children.set(link.key, { + childKey: link.key, + childNumber: link.number, + childUrl: link.url, + applicationKey: KEY, + }); + } + const parsed = parseDecompositionBody("Broken."); + if (!parsed.ok) throw new Error("unreachable"); + const liveBody = renderDecompositionBody(parsed, renderDecompositionSection(links)); + const snap = snapshot({ issue: { ...snapshot().issue, body: liveBody } }); + const unchanged = await applyGroomingMutations( + applyInput(diffFor(decomposeDraft(), snap)), + distinctChildGitHub().github, + store, + ); + expect(unchanged.steps.children?.detail).toBe("0 created, 2 reused; decomposition section unchanged"); + // Refused: malformed markers in the live body. + const broken = `${DECOMPOSITION_BODY_START}\nx\n${DECOMPOSITION_BODY_START}\ny`; + const refused = await applyGroomingMutations( + applyInput(diffFor(decomposeDraft(), snapshot({ issue: { ...snapshot().issue, body: broken } }))), + distinctChildGitHub().github, + memoryStore(), + ); + expect(refused.steps.children?.detail).toBe( + "2 created, 0 reused; decomposition section write refused: the decomposition section markers are unpaired or repeated", + ); + }); + + it("strips decomposition markers the model embedded in its enrichment, so the children section lands exactly once", async () => { + // A full marker pair the model embedded in its enrichment, and an + // unpaired start marker: without stripping, the pair would be parsed as + // the children step's own section (its model text between the markers + // replaced) and the unpaired one would poison the body into a refusal. + const variants: Array<[proposedBody: string, modelText: string]> = [ + [`Before ${DECOMPOSITION_BODY_START}\nnested model text\n${DECOMPOSITION_BODY_END} After`, "nested model text"], + [`Before ${DECOMPOSITION_BODY_START} orphan marker text After`, "orphan marker text"], + ]; + for (const [proposedBody, modelText] of variants) { + const base = decomposeDraft(); + const diff = diffFor({ ...base, mutations: { ...base.mutations, proposedBody } }); + const { github, bodies } = distinctChildGitHub(); + const result = await applyGroomingMutations(applyInput(diff), github, memoryStore()); + expect(result.outcome).toBe("applied"); + // The content step's enrichment write carries no decomposition marker: + // the model's text survives, the markers do not. + expect(bodies).toHaveLength(2); + expect(bodies[0]).not.toContain(DECOMPOSITION_BODY_START); + expect(bodies[0]).not.toContain(DECOMPOSITION_BODY_END); + expect(bodies[0]).toContain(modelText); + // The children step's section lands exactly once, at the top level. + const finalBody = bodies[bodies.length - 1]; + expect(finalBody.split(DECOMPOSITION_BODY_START).length - 1).toBe(1); + expect(finalBody.split(DECOMPOSITION_BODY_END).length - 1).toBe(1); + const managed = parseManagedBody(finalBody); + expect(managed.ok).toBe(true); + if (!managed.ok) throw new Error("unreachable"); + expect(managed.managed).toContain(modelText); + expect(managed.managed).not.toContain(DECOMPOSITION_BODY_START); + expect(managed.managed).not.toContain(DECOMPOSITION_BODY_END); + const decomp = parseDecompositionBody(finalBody); + expect(decomp.ok).toBe(true); + if (!decomp.ok) throw new Error("unreachable"); + expect(decomp.decomposition).toContain("## Decomposition"); + // The section is not nested inside the managed one. + expect(decomp.decomposition).not.toContain(MANAGED_BODY_START); + // The human text is intact. + expect(decomp.human).toContain("Broken."); + expect(result.steps.children?.detail).toBe("2 created, 0 reused; decomposition section written"); + } + }); + + it("refuses a section write that would exceed GitHub's body cap, and still lands the umbrella", async () => { + const { github, bodies } = distinctChildGitHub(); + const store = memoryStore(); + // A body at the cap: appending the section would exceed it, so the write + // is refused rather than failing on every retry. + const huge = "A".repeat(MAX_GITHUB_BODY_CHARS); + const snap = snapshot({ issue: { ...snapshot().issue, body: huge } }); + const diff = diffFor(decomposeDraft(), snap); + const result = await applyGroomingMutations(applyInput(diff), github, store); + expect(result.outcome).toBe("applied"); + // No body write at all: the body is left exactly as it was. + expect(bodies).toHaveLength(0); + expect(github.updateTitleAndBody).not.toHaveBeenCalled(); + expect(result.body).toBe(huge); + // The children, the state and the umbrella still land. + expect(github.createIssue).toHaveBeenCalledTimes(2); + expect(store.decompositionStates).toHaveLength(1); + expect(github.addLabel).toHaveBeenCalledWith("org/repo", 42, "umbrella"); + expect(result.steps.children).toMatchObject({ status: "applied" }); + expect(result.steps.children?.detail).toBe( + `2 created, 0 reused; decomposition section write refused: body would exceed ${MAX_GITHUB_BODY_CHARS} characters`, + ); + }); + + it("keeps an existing decomposition section byte for byte when the content step replaces the managed section", async () => { + // Cross-application: the parent was enriched by one application and + // decomposed by another; the body carries both managed sections. A fresh + // plan that enriches again must replace only the managed section. + const origDecomp = `${DECOMPOSITION_BODY_START}\n- #99: https://github.com/org/repo/issues/99\n${DECOMPOSITION_BODY_END}`; + const baseBody = `Broken.\n\n${MANAGED_BODY_START}\nold enrichment\n${MANAGED_BODY_END}\n\n${origDecomp}`; + const snap = snapshot({ issue: { ...snapshot().issue, body: baseBody } }); + const base = decomposeDraft(); + const diff = diffFor({ ...base, mutations: { ...base.mutations, proposedBody: "Notes." } }, snap); + const { github, bodies } = distinctChildGitHub(); + const result = await applyGroomingMutations(applyInput(diff), github, memoryStore()); + expect(result.outcome).toBe("applied"); + expect(bodies).toHaveLength(2); + // The content step replaces only the managed section: the pre-existing + // decomposition section survives the enrichment write byte for byte. + expect(bodies[0]).toBe(`Broken.\n\n${MANAGED_BODY_START}\nNotes.\n${MANAGED_BODY_END}\n\n${origDecomp}`); + // The children step then re-renders the decomposition section in place: + // the old child lines are replaced, and both managed sections coexist in + // the final body, each holding its own. + const section = + "## Decomposition\n\n" + + "Created from this parent's grooming decomposition plan. Each child starts `status/backlog` and needs its own grooming pass before pickup.\n\n" + + "- #43: https://github.com/org/repo/issues/43\n" + + "- #44: https://github.com/org/repo/issues/44"; + const finalBody = `Broken.\n\n${MANAGED_BODY_START}\nNotes.\n${MANAGED_BODY_END}\n\n${DECOMPOSITION_BODY_START}\n${section}\n${DECOMPOSITION_BODY_END}`; + expect(bodies[bodies.length - 1]).toBe(finalBody); + const managed = parseManagedBody(bodies[bodies.length - 1]); + expect(managed.ok).toBe(true); + if (!managed.ok) throw new Error("unreachable"); + expect(managed.managed).toBe("Notes."); + const decomp = parseDecompositionBody(bodies[bodies.length - 1]); + expect(decomp.ok).toBe(true); + if (!decomp.ok) throw new Error("unreachable"); + expect(decomp.decomposition).toBe(section); + expect(decomp.human).toContain("Broken."); + expect(result.body).toBe(bodies[bodies.length - 1]); + }); +}); + +describe("applyGroomingMutations → ApplyResult.body", () => { + it("is the newly written section body when the children step wrote the section", async () => { + const { github, bodies } = distinctChildGitHub(); + const result = await applyGroomingMutations(applyInput(diffFor(decomposeDraft())), github, memoryStore()); + expect(result.outcome).toBe("applied"); + expect(result.body).not.toBeNull(); + expect(result.body).toBe(bodies[bodies.length - 1]); + expect(result.body).toContain(DECOMPOSITION_BODY_START); + expect(result.body).toContain(DECOMPOSITION_BODY_END); + }); + + it("is the content step's body when the content step wrote and the plan does not decompose", async () => { + const { github, bodies } = fakeGitHub(); + const diff = diffFor(draft({}, { proposedBody: "Notes." })); + const result = await applyGroomingMutations(applyInput(diff), github, memoryStore()); + expect(result.outcome).toBe("applied"); + expect(result.steps.content).toMatchObject({ status: "applied" }); + expect(result.steps.children).toMatchObject({ status: "noop" }); + expect(bodies).toHaveLength(1); + expect(result.body).toBe(diff.body); + expect(result.body).toContain(MANAGED_BODY_START); + expect(result.body).not.toContain(DECOMPOSITION_BODY_START); + }); + + it("is null when neither the content nor the children step wrote a body", async () => { + const snap = snapshot({ issue: { ...snapshot().issue, labels: ["priority/p1", "status/ready", "type/bug"] } }); + const { github, bodies } = fakeGitHub(); + const result = await applyGroomingMutations(applyInput(diffFor(draft(), snap)), github, memoryStore()); + expect(result.outcome).toBe("noop"); + expect(bodies).toHaveLength(0); + expect(result.body).toBeNull(); + }); +}); + function addCommentBody(github: ApplierGitHub): string { return (github.addComment as ReturnType<typeof vi.fn>).mock.calls[0][2] as string; } @@ -631,6 +1405,13 @@ describe("makePrismaApplicationStore", () => { updateMany: vi.fn(async () => ({ count: 1 })), }, groomingRun: { findFirst: vi.fn(async () => null) }, + groomingChildClaim: { + findUnique: vi.fn(async (): Promise<ChildClaimRecord | null> => null), + create: vi.fn(async ({ data }: { data: Record<string, unknown> }) => data), + update: vi.fn(async () => ({})), + }, + issue: { update: vi.fn(async () => ({})) }, + auditLog: { create: vi.fn(async () => ({})) }, }; } @@ -684,6 +1465,28 @@ describe("makePrismaApplicationStore", () => { expect(claimed.existing).toEqual(winner); }); + it("reads the winner when it loses a concurrent child claim (P2002)", async () => { + const c = client(); + const winner: ChildClaimRecord = { + childKey: "child-key", + childNumber: 43, + childUrl: "https://github.com/org/repo/issues/43", + applicationKey: KEY, + }; + c.groomingChildClaim.findUnique.mockResolvedValueOnce(null).mockResolvedValueOnce(winner); + c.groomingChildClaim.create.mockRejectedValueOnce(Object.assign(new Error("Unique constraint failed"), { code: "P2002" })); + const store = makePrismaApplicationStore(c); + const claimed = await store.claimChild({ + childKey: "child-key", + parentIssueId: "issue-42", + repoFullName: "org/repo", + parentNumber: 42, + title: "Child issue A", + applicationKey: KEY, + }); + expect(claimed.existing).toEqual(winner); + }); + it("propagates other database errors, so nothing is applied unclaimed", async () => { const c = client(); c.groomingApplication.create.mockRejectedValueOnce(new Error("connection refused")); @@ -692,4 +1495,21 @@ describe("makePrismaApplicationStore", () => { store.claim({ applicationKey: KEY, issueId: "i", groomingRunId: "r1", repoFullName: "org/repo", issueNumber: 42 }), ).rejects.toThrow("connection refused"); }); + + it("propagates other database errors from a child-claim create, so nothing is claimed unrecorded", async () => { + const c = client(); + c.groomingChildClaim.findUnique.mockResolvedValueOnce(null); + c.groomingChildClaim.create.mockRejectedValueOnce(new Error("connection refused")); + const store = makePrismaApplicationStore(c); + await expect( + store.claimChild({ + childKey: "child-key", + parentIssueId: "issue-42", + repoFullName: "org/repo", + parentNumber: 42, + title: "Child issue A", + applicationKey: KEY, + }), + ).rejects.toThrow("connection refused"); + }); }); diff --git a/src/lib/groomer/mutation-applier.ts b/src/lib/groomer/mutation-applier.ts index 9036689e..35ee400d 100644 --- a/src/lib/groomer/mutation-applier.ts +++ b/src/lib/groomer/mutation-applier.ts @@ -18,16 +18,38 @@ import { createHash } from "crypto"; import type { StatusLabel } from "@/types"; +import { + childBriefKey, + CHILD_ISSUE_LABELS, + renderChildIssueBody, + setDecompositionState, + UMBRELLA_LABEL, + type ChildIssueTarget, + type DecompositionStateClient, +} from "@/lib/decomposition"; import { getBacklogLane } from "@/lib/lane-config"; import { isAutomationAuthor } from "./context"; import { neutralizeMentions } from "./sanitize"; -import { inFlightStatus, toGroomerOutput, type GroomingPlan } from "./plan"; +import { inFlightStatus, toGroomerOutput, type ChildBrief, type GroomingPlan } from "./plan"; import type { EvidenceCatalog } from "./plan-evidence"; import type { GroomerOutput } from "./schema"; -import { evaluateClosePolicy, evaluateReadyPolicy, type LiveComment, type LiveIssueState } from "./mutation-validator"; +import { + evaluateClosePolicy, + evaluateDecompositionPolicy, + evaluateReadyPolicy, + type LiveComment, + type LiveIssueState, +} from "./mutation-validator"; export const APPLICATION_KEY_VERSION = 1; export const MAX_GITHUB_COMMENT_CHARS = 4096; +/** + * The largest body the applier will write to GitHub. GitHub's issue body cap + * is ~125k characters; a write at or over it fails, so a section render that + * would cross this cap is refused (like a malformed marker) rather than + * failing on every retry and never landing the umbrella. + */ +export const MAX_GITHUB_BODY_CHARS = 120_000; // ─── Label helpers (moved from run.ts) ─────────────────────────────────────── @@ -136,6 +158,77 @@ export function renderManagedBody(parsed: Extract<ManagedBody, { ok: true }>, co return `${human}${separator}${section}`; } +// ─── Managed decomposition section ───────────────────────────────────────────── + +export const DECOMPOSITION_BODY_START = "<!-- dispatch-groomer:decomposition:start -->"; +export const DECOMPOSITION_BODY_END = "<!-- dispatch-groomer:decomposition:end -->"; + +export type DecompositionBody = + | { ok: true; human: string; before: string; after: string; decomposition: string | null } + | { ok: false; reason: string }; + +/** + * Split an issue body around its one managed decomposition section, if + * present. The section the children step writes after a decomposition (the + * children the issue was split into): the same marker discipline as the + * managed section — malformed markers (unpaired, repeated, or out of order) + * are reported rather than guessed at, so a human edit that broke them never + * makes the groomer rewrite text it does not own. + */ +export function parseDecompositionBody(body: string | null): DecompositionBody { + const text = body ?? ""; + const starts = text.split(DECOMPOSITION_BODY_START).length - 1; + const ends = text.split(DECOMPOSITION_BODY_END).length - 1; + if (starts === 0 && ends === 0) return { ok: true, human: text, before: text, after: "", decomposition: null }; + if (starts !== 1 || ends !== 1) return { ok: false, reason: "the decomposition section markers are unpaired or repeated" }; + const start = text.indexOf(DECOMPOSITION_BODY_START); + const end = text.indexOf(DECOMPOSITION_BODY_END); + if (end < start) return { ok: false, reason: "the decomposition section end marker precedes its start" }; + const before = text.slice(0, start); + const after = text.slice(end + DECOMPOSITION_BODY_END.length); + const decomposition = text.slice(start + DECOMPOSITION_BODY_START.length, end).trim(); + const human = [before.trim(), after.trim()].filter((part) => part.length > 0).join("\n\n"); + return { ok: true, human, before, after, decomposition }; +} + +function decompositionSection(content: string): string { + const cleaned = content.split(DECOMPOSITION_BODY_START).join("").split(DECOMPOSITION_BODY_END).join("").trim(); + return `${DECOMPOSITION_BODY_START}\n${cleaned}\n${DECOMPOSITION_BODY_END}`; +} + +/** + * The body with `content` as its one managed decomposition section. An + * existing section is replaced in place; otherwise the section is appended + * after the text that precedes it. Everything outside the section — including + * the managed enrichment section — is kept byte for byte, so re-rendering the + * same content is a fixed point. + */ +export function renderDecompositionBody(parsed: Extract<DecompositionBody, { ok: true }>, content: string): string { + const section = decompositionSection(content); + if (parsed.decomposition !== null) return `${parsed.before}${section}${parsed.after}`; + const human = parsed.before; + if (human.trim().length === 0) return section; + const separator = human.endsWith("\n\n") ? "" : human.endsWith("\n") ? "\n" : "\n\n"; + return `${human}${separator}${section}`; +} + +/** + * The section content: a human-readable heading and the one sentence that + * says what the section is, then one line per created-or-reused child link, + * in brief order. Deterministic — no timestamps, no digests — so re-rendering + * the same children is a fixed point. + */ +export function renderDecompositionSection(children: ChildIssueLink[]): string { + const bullets = children.map((child) => `- #${child.number}: ${child.url}`).join("\n"); + return [ + "## Decomposition", + "", + "Created from this parent's grooming decomposition plan. Each child starts `status/backlog` and needs its own grooming pass before pickup.", + "", + bullets, + ].join("\n"); +} + // ─── Comment marker ─────────────────────────────────────────────────────────── /** Only the marker Dispatch appends counts: it must end the comment. */ @@ -163,11 +256,21 @@ export function groomerCommentKey(comment: Pick<LiveComment, "author" | "body">) // ─── Diff ───────────────────────────────────────────────────────────────────── +/** + * The plan's decomposition intent, when the plan splits the issue into + * bounded children (dispatch#1066). `null` when the plan does not decompose + * (or the decomposition was withheld at apply time). + */ +export interface DecompositionIntent { + briefs: ChildBrief[]; + reason: string | null; +} + export interface GroomingMutationDiff { /** The effective legacy view the diff was computed from (after any withholding). */ output: GroomerOutput; - /** Why an intended close or ready promotion was withheld at apply time. */ - withheld: { close?: string[]; ready?: string[] }; + /** Why an intended close, ready promotion or decomposition was withheld at apply time. */ + withheld: { close?: string[]; ready?: string[]; decomposition?: string[] }; labelsBefore: string[]; /** Final label set. */ labelsAfter: string[]; @@ -180,9 +283,21 @@ export interface GroomingMutationDiff { title: string | null; /** New full body; null when unchanged. */ body: string | null; + /** + * The live body the diff was computed from, before any of this diff's + * writes; null when the issue has no body. The children step renders its + * managed section into the body as it stands after the content step — the + * post-content body when that step has a write, this live body otherwise. + * The apply preconditions verify the live body equals the evidence + * snapshot's, so this is exactly the body the children step starts from + * when the content step writes nothing. + */ + bodyBefore: string | null; /** Why proposed body enrichment was not applied, when it was not. */ bodySkippedReason: string | null; close: boolean; + /** The plan's decomposition intent; null when the plan does not decompose. */ + children: DecompositionIntent | null; } export interface MutationDiffInput { @@ -254,6 +369,23 @@ export function computeMutationDiff(input: MutationDiffInput): GroomingMutationD } } + // The decomposition is a separate, independent decision from the close and + // ready promotions above: a plan that splits the issue into children is + // withheld only when its own decomposition policy fails (a close in the + // same plan, low confidence, or a material uncertainty), never because the + // close or ready policy did. + let children: DecompositionIntent | null = + plan.decomposition.required && plan.decomposition.childBriefs.length > 0 + ? { briefs: plan.decomposition.childBriefs, reason: plan.decomposition.reason } + : null; + if (children) { + const reasons = evaluateDecompositionPolicy(plan); + if (reasons.length > 0) { + withheld.decomposition = reasons; + children = null; + } + } + const output = toGroomerOutput(effective, live.labels); const done = effective.verdict.actionability === "already_done"; const labelsBefore = [...live.labels]; @@ -264,6 +396,15 @@ export function computeMutationDiff(input: MutationDiffInput): GroomingMutationD effective.mutations.status, ); + // The umbrella label is deliberately NOT part of labelsAfter (and so not of + // the labels step): it is added by the children step, and only after every + // child exists or is reused. The groomer selector excludes umbrella-labeled + // issues from selection on every path — including a targeted re-groom by + // issueNumber (which bypasses only the grooming-state exclusion, not the + // umbrella one) — so landing the umbrella first would make a decomposition + // that fails halfway un-reselectable, breaking "a partial create converges on + // retry". The label we write is still folded into ApplyResult.labels below so + // the freshness baseline and audit record it. // status/done lands only after the close succeeds, so a failed close // leaves the issue open with its previous status (still groomable), // never open with status/done (which the selector skips forever). An issue @@ -291,7 +432,13 @@ export function computeMutationDiff(input: MutationDiffInput): GroomingMutationD } else if (!shouldEnrichBody(parsed.human)) { bodySkippedReason = "the human-authored body is not sparse"; } else { - const rendered = renderManagedBody(parsed, neutralizeMentions(output.proposedBody)); + // A marker the model wrote into its enrichment is stripped before it + // lands (like the comment's, below): a decomposition marker in + // particular would otherwise be parsed as the children step's own + // section — an embedded pair would have the model's text between it + // replaced, an unpaired one would poison the body into a refusal. + const enrichment = neutralizeMentions(output.proposedBody.replace(ANY_GROOMER_MARKER, "")); + const rendered = renderManagedBody(parsed, enrichment); if (rendered !== (live.body ?? "")) body = rendered; else bodySkippedReason = "the managed section already holds this content"; } @@ -311,8 +458,10 @@ export function computeMutationDiff(input: MutationDiffInput): GroomingMutationD comment, title, body, + bodyBefore: live.body, bodySkippedReason, close: done && live.state === "open" && inFlightStatus(live.labels) === null, + children, }; } @@ -341,6 +490,13 @@ export function computeApplicationKey(input: { title: diff.title, body: diff.body, close: diff.close, + // The decomposition is part of the intent: its children are the stable + // childBriefKeys, so a change to the split (or its withholding) changes + // the application key. Sorted, because the brief order is not identity — + // the same set of children in a different order is the same application. + children: diff.children + ? diff.children.briefs.map((brief) => childBriefKey(input.repoFullName, input.issueNumber, brief)).sort() + : null, }, }; return createHash("sha256").update(JSON.stringify(canonical)).digest("hex"); @@ -348,7 +504,19 @@ export function computeApplicationKey(input: { // ─── Application ────────────────────────────────────────────────────────────── -export const APPLY_STEPS = ["labels", "comment", "content", "close", "done_label"] as const; +/** + * A step write that failed partway, carrying the partial result that did + * complete (e.g. the children created before a later child's create threw) so + * the failed step record — and a retry — can see and reuse it. + */ +export class PartialStepError extends Error { + constructor(message: string, readonly partial: Partial<ApplyStepResult>) { + super(message); + this.name = "PartialStepError"; + } +} + +export const APPLY_STEPS = ["labels", "comment", "content", "children", "close", "done_label"] as const; export type ApplyStep = (typeof APPLY_STEPS)[number]; /** @@ -362,12 +530,27 @@ export type ApplyStep = (typeof APPLY_STEPS)[number]; */ export type ApplyStepStatus = "applied" | "replayed" | "noop" | "skipped" | "failed" | "not_attempted"; +/** A created-or-reused child issue, auditable from the run and the parent. */ +export interface ChildIssueLink { + key: string; + number: number; + url: string; +} + +/** The child issues a decomposition step created and reused this attempt. */ +export interface AppliedChildIssues { + created: ChildIssueLink[]; + reused: ChildIssueLink[]; +} + export interface ApplyStepResult { status: ApplyStepStatus; detail?: string; error?: string; commentUrl?: string | null; at?: string; + /** Children step: the child issues created and reused this attempt. */ + children?: AppliedChildIssues; } export type ApplySteps = Partial<Record<ApplyStep, ApplyStepResult>>; @@ -399,6 +582,31 @@ export interface ApplicationRecord { updatedAt?: Date | string | null; } +/** + * A GroomingChildClaim row, as the applier reads it: the stable child key and + * the created child's number/URL once the creation has been recorded. A claim + * whose childNumber is null means the creation write has not been recorded, so + * re-creating it covers a crashed attempt — though a create whose response was + * lost after GitHub accepted it can still duplicate, which the child body + * marker exists to surface for manual discovery. + */ +export interface ChildClaimRecord { + childKey: string; + childNumber: number | null; + childUrl: string | null; + /** + * The application that claimed this child (dispatch#1066); null for rows + * written before the column existed (none in any deployed env). The resume + * CAS (which serializes attempts while the prior application claim is fresh) + * plus the single-replica confinement of the groomer make concurrent + * same-key application attempts not expected, so a null claim under MY key + * is my own abandoned create, not a concurrent holder. + */ + applicationKey: string | null; + /** When the claim row was last written; the prisma store returns it. */ + updatedAt?: Date | string | null; +} + export interface ApplicationStore { /** The record for a key, if one exists. Read-only (dry runs use it). */ find(applicationKey: string): Promise<ApplicationRecord | null>; @@ -419,6 +627,36 @@ export interface ApplicationStore { resume(applicationKey: string, seen: ApplicationRecord): Promise<boolean>; /** Whether a hosted-groomer comment was recorded on this issue since `since`. */ hasRecentComment(issueId: string, since: Date): Promise<boolean>; + /** + * Claim a child key, atomically (dispatch#1066). `existing` is the prior + * claim when the child was already created, so a retry reuses it rather than + * creating a second child. `applicationKey` is recorded on a new claim so a + * same-key retry can tell its own abandoned create apart from a fresh + * claim held by a DIFFERENT application. + */ + claimChild(input: { + childKey: string; + parentIssueId: string; + repoFullName: string; + parentNumber: number; + title: string; + applicationKey: string; + }): Promise<{ existing: ChildClaimRecord | null }>; + /** Record the created child's number and URL on its claim. */ + saveChild(childKey: string, data: { childNumber: number; childUrl: string }): Promise<void>; + /** + * Persist a parent's decomposition state and its audit entry (dispatch#1066), + * sharing the operator route's persistence path. + */ + setDecompositionState(input: { + issue: { id: string; labels: readonly string[] }; + repoFullName: string; + issueNumber: number; + actor: string; + decomposed: boolean; + note: string | null; + followUpUrls: string[]; + }): Promise<void>; } export interface ApplierGitHub { @@ -428,12 +666,18 @@ export interface ApplierGitHub { closeIssue(repoFullName: string, issueNumber: number): Promise<void>; /** Newest first; used to find a comment that landed before its write reported success. */ fetchRecentComments(repoFullName: string, issueNumber: number, max: number): Promise<LiveComment[]>; + /** Open a new issue (a decomposition child); returns its number and URL. */ + createIssue(repoFullName: string, input: { title: string; body: string; labels?: string[] }): Promise<{ number: number; url: string }>; + /** Add a single label to the issue (the children step's umbrella). */ + addLabel(repoFullName: string, issueNumber: number, label: string): Promise<void>; } export interface ApplyInput { repoFullName: string; issueNumber: number; issueId: string; + /** The parent issue's URL, so a child body can link back to it. */ + parentUrl: string; groomingRunId: string; applicationKey: string; diff: GroomingMutationDiff; @@ -452,6 +696,8 @@ export interface ApplyResult { commentUrl: string | null; /** Labels on GitHub after this attempt, as far as the recorded steps show. */ labels: string[]; + /** Child issues created or reused this attempt, in brief order. */ + children: ChildIssueLink[]; title: string | null; body: string | null; closed: boolean; @@ -507,7 +753,16 @@ export async function applyGroomingMutations( }); const prior = readSteps(existing?.steps); const claimedByRunId = existing && existing.groomingRunId !== input.groomingRunId ? existing.groomingRunId : null; - const nothing = { claimedByRunId, commentUrl: null, labels: diff.labelsBefore, title: null, body: null, closed: false, failure: null }; + const nothing = { + claimedByRunId, + commentUrl: null, + labels: diff.labelsBefore, + children: [] as ChildIssueLink[], + title: null, + body: null, + closed: false, + failure: null, + }; if (existing && existing.status !== "applied") { const updatedAt = existing.updatedAt ? new Date(existing.updatedAt).getTime() : NaN; @@ -521,6 +776,10 @@ export async function applyGroomingMutations( if (existing?.status === "applied") { const commentUrl = prior.comment?.commentUrl ?? null; + // A replay surfaces the children an earlier attempt created or reused. + const children = prior.children?.children + ? [...prior.children.children.created, ...prior.children.children.reused] + : []; return { outcome: "replayed", steps: Object.fromEntries( @@ -531,6 +790,7 @@ export async function applyGroomingMutations( ) as ApplySteps, ...nothing, commentUrl, + children, }; } @@ -538,6 +798,7 @@ export async function applyGroomingMutations( let failure: ApplyResult["failure"] = null; let commentUrl: string | null = null; let labels = diff.labelsBefore; + let appliedChildren: ChildIssueLink[] = []; const persist = async (status: string) => { try { @@ -556,6 +817,11 @@ export async function applyGroomingMutations( if (landed(prior[step])) { steps[step] = { ...prior[step]!, status: "replayed" }; if (step === "comment") commentUrl = prior.comment?.commentUrl ?? null; + // A replayed children step surfaces its links in the result, the same + // as the full-replay path. + if (step === "children" && prior.children?.children) { + appliedChildren = [...prior.children.children.created, ...prior.children.children.reused]; + } return; } if (failure) { @@ -571,7 +837,15 @@ export async function applyGroomingMutations( steps[step] = { status: "applied", ...(result ?? {}), at: now().toISOString() }; } catch (err) { const error = errorMessage(err); - steps[step] = { status: "failed", error, at: now().toISOString() }; + // A PartialStepError carries the partial result that completed (e.g. the + // children created before a later create threw), so the failed step + // record — and a retry — can see and reuse it. + steps[step] = { + status: "failed", + error, + ...(err instanceof PartialStepError ? err.partial : {}), + at: now().toISOString(), + }; failure = { step, error }; console.error(`[groomer] ${repoFullName}#${issueNumber}: ${step} failed; later steps not attempted:`, err); } @@ -652,12 +926,170 @@ export async function applyGroomingMutations( return { detail: Object.keys(fields).join("+") }; }, diff.bodySkippedReason ?? "title and body unchanged"); - // 4. Close, the highest-impact write, only after everything above landed. + // 4. Children (dispatch#1066): one bounded child issue per brief. Creation + // is idempotent through the GroomingChildClaim keyed by the childBriefKey + // (a retry reuses a created child and creates only the missing ones). Only + // after every child exists or is reused does this step record the parent's + // decomposition state with the child URLs as the follow-ups, and only then + // add the umbrella label (an additive write) last. The umbrella lands last + // — after the state write — so a decomposition that fails mid-create, or + // whose state write fails, leaves the parent still re-selectable (the + // selector excludes umbrella issues) and converges on retry. It lands + // before the close, so a close is never applied on top of a decomposition + // that failed to land. + const children = diff.children; + // The body the children step starts from: the post-content body when the + // content step has a write (the managed enrichment section, if it lands), + // the live body as diffed otherwise. `childrenFinalBody` records what the + // step's section write leaves the body as (the unchanged body when the + // write is refused or a no-op), so the result can report the real + // post-apply body for the freshness baseline. + const baseBody = diff.body !== null ? diff.body : (diff.bodyBefore ?? ""); + const liveUnchangedBody = diff.body !== null ? diff.body : diff.bodyBefore; + let childrenFinalBody: string | null = null; + await run( + "children", + children !== null, + async () => { + if (children === null) return; + const parentTarget: ChildIssueTarget = { repoFullName, number: issueNumber, url: input.parentUrl }; + const created: ChildIssueLink[] = []; + const reused: ChildIssueLink[] = []; + const links: ChildIssueLink[] = []; + try { + for (const brief of children.briefs) { + const childKey = childBriefKey(repoFullName, issueNumber, brief); + const { existing } = await store.claimChild({ + childKey, + parentIssueId: input.issueId, + repoFullName, + parentNumber: issueNumber, + title: brief.title, + applicationKey: input.applicationKey, + }); + if (existing && existing.childNumber !== null && existing.childUrl !== null) { + // An earlier attempt already created this child. + const link: ChildIssueLink = { key: childKey, number: existing.childNumber, url: existing.childUrl }; + reused.push(link); + links.push(link); + } else { + // A null claim under a DIFFERENT application key with a fresh + // updatedAt is held by another in-flight attempt claiming the same + // child; do not create a duplicate on top of it. A null claim under + // THIS key is my own abandoned create: the GroomingApplication + // resume CAS (which serializes attempts only while the prior + // application claim is fresh) plus the single-replica confinement + // of the groomer make concurrent same-key application attempts not + // expected, so creating on top of it is how an immediate same-key + // retry converges. A row with no applicationKey (none exists in any + // deployed env; the migration ships with the feature) is not mine, + // so a fresh one is a foreign holder. + const heldAt = existing?.updatedAt ? new Date(existing.updatedAt).getTime() : NaN; + if ( + existing && + existing.childNumber === null && + existing.applicationKey !== input.applicationKey && + Number.isFinite(heldAt) && + now().getTime() - heldAt < ACTIVE_CLAIM_MS + ) { + throw new Error(`child claim ${childKey} is held by another in-flight attempt`); + } + const body = renderChildIssueBody({ + brief, + parent: parentTarget, + decompositionReason: children.reason, + childKey, + }); + const issue = await github.createIssue(repoFullName, { + title: brief.title, + body, + labels: [...CHILD_ISSUE_LABELS], + }); + await store.saveChild(childKey, { childNumber: issue.number, childUrl: issue.url }); + const link: ChildIssueLink = { key: childKey, number: issue.number, url: issue.url }; + created.push(link); + links.push(link); + } + } + appliedChildren = links; + // Record the decomposition state with the parent's label set at the + // moment of the state write (labelsAfter — the umbrella genuinely is + // not on the issue yet) and the child URLs, BEFORE the umbrella add: + // a failure here — like a failed child create above — must leave the + // parent still re-selectable, so the umbrella, which removes it from + // every selection path, is the step's final write. The audit entry + // records labels at state-write time; the umbrella add lands + // afterwards and its own success/failure is visible on the children + // step record and the run's groom audit, so the entry never claims a + // label that has not landed. + await store.setDecompositionState({ + issue: { id: input.issueId, labels: diff.labelsAfter }, + repoFullName, + issueNumber, + actor: "hosted-groomer", + decomposed: true, + note: children.reason, + followUpUrls: links.map((child) => child.url), + }); + // The managed decomposition section: one section in the parent's body + // listing the children this decomposition created or reused. It + // renders into the body as it stands after the content step, so the + // managed enrichment section and this one coexist, each parsed and + // rendered independently. A failed write here throws like a failed + // child create above, so the umbrella never lands on a decomposition + // whose section write failed, and a retry converges; a body whose + // markers a human edit broke is refused, not guessed at, and a + // rendered body that would exceed GitHub's body cap is refused + // likewise (the write would fail on every retry, and the umbrella + // would never land) — in either case the children, the state and the + // umbrella still land. + // The step's detail carries the created/reused counts plus the + // section outcome (written, unchanged, or refused with the reason), + // so the run record shows what the body write did without re-reading + // the issue. + let detail = `${created.length} created, ${reused.length} reused`; + const parsedSection = parseDecompositionBody(baseBody); + if (!parsedSection.ok) { + detail += `; decomposition section write refused: ${parsedSection.reason}`; + childrenFinalBody = liveUnchangedBody; + } else { + const rendered = renderDecompositionBody(parsedSection, renderDecompositionSection(links)); + if (rendered !== baseBody) { + if (rendered.length > MAX_GITHUB_BODY_CHARS) { + detail += `; decomposition section write refused: body would exceed ${MAX_GITHUB_BODY_CHARS} characters`; + childrenFinalBody = liveUnchangedBody; + } else { + await github.updateTitleAndBody(repoFullName, issueNumber, { body: rendered }); + childrenFinalBody = rendered; + detail += "; decomposition section written"; + } + } else { + childrenFinalBody = liveUnchangedBody; + detail += "; decomposition section unchanged"; + } + } + // The umbrella lands last, now that every child exists or is reused, + // the decomposition state is recorded and the section is written or + // refused; a failed create, a failed state write or a failed section + // write throws before this line, so a partial decomposition never + // lands the umbrella. + await github.addLabel(repoFullName, issueNumber, UMBRELLA_LABEL); + return { children: { created, reused }, detail }; + } catch (err) { + // Carry what completed so far so the failed step record (and a retry) + // can see the children that did land. + throw new PartialStepError(errorMessage(err), { children: { created, reused } }); + } + }, + "no decomposition in the plan", + ); + + // 5. Close, the highest-impact write, only after everything above landed. await run("close", diff.close, async () => { await github.closeIssue(repoFullName, issueNumber); }, "no close in the plan"); - // 5. status/done, only once the issue is actually closed. + // 6. status/done, only once the issue is actually closed. const closed = landed(steps.close); await run( "done_label", @@ -670,19 +1102,38 @@ export async function applyGroomingMutations( if (landed(steps.labels)) labels = diff.labelsStep; if (landed(steps.done_label)) labels = diff.labelsAfter; + // The children step writes the umbrella additively (it is not part of + // diff.labelsAfter), so fold in the label we actually wrote. A union with the + // current set (not a reset to diff.labelsAfter) keeps whatever the labels / + // done-label step landed while adding the umbrella the children step wrote. + if (landed(steps.children)) labels = [...new Set([...labels, UMBRELLA_LABEL])]; const wrote = APPLY_STEPS.some((step) => steps[step]?.status === "applied" || steps[step]?.status === "replayed"); const outcome: ApplyOutcome = failure ? (wrote ? "partial" : "failed") : wrote ? "applied" : "noop"; await persist(failure ? (wrote ? "partial" : "failed") : "applied"); + // The real post-apply body, for the freshness baseline (run.ts): the + // children step's section write is the last word on the body, so what it + // left behind wins whenever it was reached (even if the step then failed at + // the umbrella — the write itself landed); otherwise the content step's + // write; otherwise null (nothing written, the baseline falls back to the + // snapshot). + const finalBody = + childrenFinalBody !== null + ? childrenFinalBody + : landed(steps.content) && diff.body !== null + ? diff.body + : null; + return { outcome, steps, claimedByRunId, commentUrl, labels, + children: appliedChildren, title: landed(steps.content) && diff.title !== null ? diff.title : null, - body: landed(steps.content) && diff.body !== null ? diff.body : null, + body: finalBody, closed, failure, }; @@ -697,9 +1148,20 @@ interface GroomingApplicationDelegateLike { updateMany(args: { where: Record<string, unknown>; data: Record<string, unknown> }): Promise<{ count: number }>; } -export interface ApplicationStoreClient { +/** + * The slice of the Prisma client the application store needs. Extends + * `DecompositionStateClient` so the store can also persist a parent's + * decomposition state through the shared helper (dispatch#1066). + */ +export interface ApplicationStoreClient extends DecompositionStateClient { groomingApplication: GroomingApplicationDelegateLike; groomingRun: { findFirst(args: unknown): Promise<unknown> }; + /** GroomingChildClaim rows keying a decomposition's children (dispatch#1066). */ + groomingChildClaim: { + findUnique(args: { where: { childKey: string } }): Promise<ChildClaimRecord | null>; + create(args: { data: Record<string, unknown> }): Promise<unknown>; + update(args: { where: { childKey: string }; data: Record<string, unknown> }): Promise<unknown>; + }; } function isUniqueViolation(err: unknown): boolean { @@ -760,5 +1222,38 @@ export function makePrismaApplicationStore(client: ApplicationStoreClient): Appl }); return recent !== null && recent !== undefined; }, + async claimChild(input) { + // The unique childKey makes the claim atomic: a concurrent creator for + // the same child loses with P2002 and reads the winner's row, so two + // attempts never create the same child twice. The create records + // applicationKey (and findUnique selects it back), so a same-key retry + // can tell its own abandoned create apart from a foreign in-flight + // holder. + const child = client.groomingChildClaim; + const existing = await child.findUnique({ where: { childKey: input.childKey } }); + if (existing) return { existing }; + try { + await child.create({ + data: { + childKey: input.childKey, + parentIssueId: input.parentIssueId, + repoFullName: input.repoFullName, + parentNumber: input.parentNumber, + title: input.title, + applicationKey: input.applicationKey, + }, + }); + return { existing: null }; + } catch (err) { + if (!isUniqueViolation(err)) throw err; + return { existing: await child.findUnique({ where: { childKey: input.childKey } }) }; + } + }, + async saveChild(childKey, data) { + await client.groomingChildClaim.update({ where: { childKey }, data }); + }, + async setDecompositionState(input) { + await setDecompositionState(client, input); + }, }; } diff --git a/src/lib/groomer/mutation-validator.test.ts b/src/lib/groomer/mutation-validator.test.ts index 3de3d3a2..dd719d92 100644 --- a/src/lib/groomer/mutation-validator.test.ts +++ b/src/lib/groomer/mutation-validator.test.ts @@ -2,9 +2,11 @@ import { describe, expect, it, vi } from "vitest"; import type { GroomingEvidenceSnapshot } from "./evidence-snapshot"; import { buildEvidenceCatalog } from "./plan-evidence"; import { collectPinnedReadContent } from "./close-grounding"; -import { validateGroomingPlan, type GroomingPlan, type GroomingPlanDraft } from "./plan"; +import { validateGroomingPlan, type ChildBrief, type GroomingPlan, type GroomingPlanDraft } from "./plan"; import { + childBriefCompletenessGaps, evaluateClosePolicy, + evaluateDecompositionPolicy, evaluateReadyPolicy, validateApplyPreconditions, type LiveComment, @@ -113,12 +115,52 @@ function alreadyDoneDraft(evidenceRefs: string[] = ["repo:src/auth/login.ts"]): }; } +/** A plan that splits the issue into one bounded child (dispatch#1066). */ +function decomposeDraft(): GroomingPlanDraft { + return { + ...readyDraft(), + // A ready `implementation` plan may not decompose, so the split lands as + // a non-ready (backlog) verdict. + verdict: { ...readyDraft().verdict, actionability: "backlog", lane: { id: "backlog", confidence: "high", reason: "bounded" } }, + implementationBrief: null, + decomposition: { + required: true, + reason: "splits into two bounded children", + childBriefs: [ + { + title: "Child issue A", + problem: "Child A problem", + designDecision: "No design choice; follow the existing returnTo handling", + verifiedCurrentBehavior: "redirectAfterLogin drops session.returnTo", + relevantPaths: ["src/auth/login.ts"], + inScope: ["A"], + outOfScope: ["B"], + dependencies: ["child B: the migration"], + acceptanceCriteria: ["A works"], + tests: ["reset-then-login test"], + }, + ], + }, + }; +} + function planFor(draft: GroomingPlanDraft, snap = snapshot()): GroomingPlan { const result = validateGroomingPlan(draft, { catalog: catalogFor(snap) }); if (!result.valid) throw new Error(result.errors!.join("; ")); return result.plan!; } +/** The plan with its first child brief overridden, built as if valid. */ +function withChildBrief(base: GroomingPlan, brief: Partial<ChildBrief>): GroomingPlan { + return { + ...base, + decomposition: { + ...base.decomposition, + childBriefs: base.decomposition.childBriefs.map((b, i) => (i === 0 ? { ...b, ...brief } : b)), + }, + }; +} + const WINDOW_START = new Date("2026-09-26T00:00:00.000Z"); // Fixed clock for the search-index grace window tests (#1116). const NOW = new Date("2026-09-26T02:00:00.000Z"); @@ -503,3 +545,149 @@ describe("evaluateReadyPolicy", () => { expect(evaluateReadyPolicy(forged, catalogFor(snapshot()))).toContain("the plan's derived readiness is not ready"); }); }); + +describe("evaluateDecompositionPolicy", () => { + const plan = planFor(decomposeDraft()); + + it("allows a complete bounded child brief with no close and no material uncertainty", () => { + expect(evaluateDecompositionPolicy(plan)).toEqual([]); + }); + + it("refuses a decomposition the plan also recommends closing for", () => { + const forged: GroomingPlan = { + ...plan, + mutations: { + ...plan.mutations, + close: { reason: "already_done", rationale: "done", evidenceRefs: ["repo:src/auth/login.ts"] }, + }, + }; + expect(evaluateDecompositionPolicy(forged)).toContain( + "the plan recommends closing the issue; a decomposed parent is not closed", + ); + }); + + it("refuses a decomposition whose confidence is low", () => { + const forged: GroomingPlan = { ...plan, verdict: { ...plan.verdict, confidence: "low" } }; + expect(evaluateDecompositionPolicy(forged)).toContain( + "verdict confidence is low; decomposition requires at least medium confidence", + ); + }); + + it("refuses a decomposition with a material uncertainty", () => { + const forged: GroomingPlan = { + ...plan, + verdict: { ...plan.verdict, uncertainties: [{ kind: "scope", question: "Which child owns the migration?", material: true }] }, + }; + expect(evaluateDecompositionPolicy(forged)).toContain( + "material uncertainty remains (verdict.uncertainties[0]): Which child owns the migration?", + ); + }); + + it("ignores a non-material uncertainty", () => { + const forged: GroomingPlan = { + ...plan, + verdict: { ...plan.verdict, uncertainties: [{ kind: "scope", question: "Nice to know", material: false }] }, + }; + expect(evaluateDecompositionPolicy(forged)).toEqual([]); + }); + + const fieldCases: Array<[string, Partial<ChildBrief>, string]> = [ + ["problem", { problem: " " }, "problem"], + ["designDecision", { designDecision: null }, "designDecision"], + ["verifiedCurrentBehavior", { verifiedCurrentBehavior: null }, "verifiedCurrentBehavior"], + ["relevantPaths", { relevantPaths: [] }, "relevantPaths"], + ["inScope", { inScope: [] }, "inScope"], + ["outOfScope", { outOfScope: [] }, "outOfScope"], + ["acceptanceCriteria", { acceptanceCriteria: [] }, "acceptanceCriteria"], + ["tests", { tests: [] }, "tests"], + ]; + + it.each(fieldCases)("rejects a child brief missing %s", (_field, patch, missing) => { + const forged = withChildBrief(plan, patch); + expect(evaluateDecompositionPolicy(forged)).toEqual([ + `child brief[0] is not a complete bounded implementation brief (missing: ${missing})`, + ]); + }); + + it("does not treat empty dependencies as a gap", () => { + const forged = withChildBrief(plan, { dependencies: [] }); + expect(evaluateDecompositionPolicy(forged)).toEqual([]); + }); + + it("treats whitespace-only entries as absent", () => { + const forged = withChildBrief(plan, { + designDecision: " ", + verifiedCurrentBehavior: " ", + relevantPaths: ["\t"], + inScope: [" "], + outOfScope: ["\n"], + acceptanceCriteria: [" "], + tests: [" "], + }); + expect(evaluateDecompositionPolicy(forged)).toEqual([ + "child brief[0] is not a complete bounded implementation brief (missing: designDecision, verifiedCurrentBehavior, relevantPaths, inScope, outOfScope, acceptanceCriteria, tests)", + ]); + }); + + it("reports every incomplete child in its own reason, in brief order", () => { + const full = plan.decomposition.childBriefs[0]; + const forged: GroomingPlan = { + ...plan, + decomposition: { + ...plan.decomposition, + childBriefs: [ + { ...full, designDecision: null, tests: [] }, + { ...full, verifiedCurrentBehavior: null, outOfScope: [] }, + ], + }, + }; + expect(evaluateDecompositionPolicy(forged)).toEqual([ + "child brief[0] is not a complete bounded implementation brief (missing: designDecision, tests)", + "child brief[1] is not a complete bounded implementation brief (missing: verifiedCurrentBehavior, outOfScope)", + ]); + }); +}); + +describe("childBriefCompletenessGaps", () => { + const full: ChildBrief = { + title: "Child", + problem: "Child problem", + designDecision: "follow the existing pattern", + verifiedCurrentBehavior: "redirectAfterLogin drops session.returnTo", + relevantPaths: ["src/auth/login.ts"], + inScope: ["A"], + outOfScope: ["B"], + dependencies: [], + acceptanceCriteria: ["A works"], + tests: ["reset-then-login test"], + }; + + it("lists no gaps for a complete brief, whatever its dependencies are", () => { + expect(childBriefCompletenessGaps(full)).toEqual([]); + }); + + it("lists the missing fields in field order and never reports dependencies", () => { + const brief: ChildBrief = { + ...full, + problem: " ", + designDecision: null, + verifiedCurrentBehavior: null, + relevantPaths: [], + inScope: [], + outOfScope: [], + dependencies: [], + acceptanceCriteria: [], + tests: [], + }; + expect(childBriefCompletenessGaps(brief)).toEqual([ + "problem", + "designDecision", + "verifiedCurrentBehavior", + "relevantPaths", + "inScope", + "outOfScope", + "acceptanceCriteria", + "tests", + ]); + }); +}); diff --git a/src/lib/groomer/mutation-validator.ts b/src/lib/groomer/mutation-validator.ts index cfed0a75..65ec4791 100644 --- a/src/lib/groomer/mutation-validator.ts +++ b/src/lib/groomer/mutation-validator.ts @@ -25,7 +25,7 @@ import { type ExplorationToolCallLike, type NegativeSearchRecheckProbe, } from "./freshness"; -import { evaluateReadiness, type GroomingPlan } from "./plan"; +import { evaluateReadiness, type ChildBrief, type GroomingPlan } from "./plan"; import type { EvidenceCatalog } from "./plan-evidence"; import { evaluateCloseGrounding } from "./close-grounding"; @@ -484,3 +484,82 @@ export function evaluateReadyPolicy(plan: GroomingPlan, catalog: EvidenceCatalog reasons.push(...evaluateReadiness(plan, catalog)); return reasons; } + +/** + * The child-brief fields that are missing from this brief, in field order + * (dispatch#1066). A field counts as present when it is non-blank after + * trimming; a list field needs at least one entry that trims non-empty + * (whitespace-only entries do not count). `dependencies` is deliberately + * never reported: a child may legitimately depend on nothing. + */ +export function childBriefCompletenessGaps(brief: ChildBrief): string[] { + const gaps: string[] = []; + if (brief.problem.trim().length === 0) gaps.push("problem"); + if (brief.designDecision === null || brief.designDecision.trim().length === 0) gaps.push("designDecision"); + if (brief.verifiedCurrentBehavior === null || brief.verifiedCurrentBehavior.trim().length === 0) { + gaps.push("verifiedCurrentBehavior"); + } + const lists: Array<[string, string[]]> = [ + ["relevantPaths", brief.relevantPaths], + ["inScope", brief.inScope], + ["outOfScope", brief.outOfScope], + ["acceptanceCriteria", brief.acceptanceCriteria], + ["tests", brief.tests], + ]; + for (const [name, entries] of lists) { + if (!entries.some((entry) => entry.trim().length > 0)) gaps.push(name); + } + return gaps; +} + +/** + * Decomposition re-checked at apply time (dispatch#1066). Returns every + * reason the plan's children may not be created; empty means they may. + * + * A decomposition splits one issue into bounded children, so it is only + * applied when the parent's analysis is decisive enough to trust the split: + * - the plan does not also recommend closing the issue (a closed parent has + * no children to carry its work); + * - the verdict confidence is not low; + * - no material uncertainty of any kind remains. + * The apply preconditions separately guarantee the issue is still open. + * + * Every child brief must also be a COMPLETE bounded implementation brief, + * because each one becomes a real child GitHub issue that a fresh worker + * has to implement with no other context: + * - `problem` is non-blank (the plan parser already guarantees this; it is + * enforced again so a forged plan cannot slip past); + * - `designDecision` is a non-blank string; null is rejected. Even the + * outcome "no design choice, follow existing pattern X" must be stated, + * so the worker knows nothing is left to decide; + * - `verifiedCurrentBehavior` is a non-blank string; null is rejected — + * the brief must carry the current behavior the parent's analysis + * verified; + * - `relevantPaths`, `inScope`, `outOfScope`, `acceptanceCriteria` and + * `tests` each contain at least one entry whose trimmed value is + * non-empty (whitespace-only entries do not count as present); + * - `dependencies` may legitimately be empty: a child may depend on + * nothing, so it is never a reason. + * Each incomplete brief is a reason of its own naming its index and + * missing fields (see childBriefCompletenessGaps), so the run records + * exactly which children and which fields are short. + */ +export function evaluateDecompositionPolicy(plan: GroomingPlan): string[] { + const reasons: string[] = []; + if (plan.mutations.close) { + reasons.push("the plan recommends closing the issue; a decomposed parent is not closed"); + } + if (plan.verdict.confidence === "low") { + reasons.push("verdict confidence is low; decomposition requires at least medium confidence"); + } + plan.verdict.uncertainties.forEach((u, i) => { + if (u.material) reasons.push(`material uncertainty remains (verdict.uncertainties[${i}]): ${u.question}`); + }); + plan.decomposition.childBriefs.forEach((brief, i) => { + const gaps = childBriefCompletenessGaps(brief); + if (gaps.length > 0) { + reasons.push(`child brief[${i}] is not a complete bounded implementation brief (missing: ${gaps.join(", ")})`); + } + }); + return reasons; +} diff --git a/src/lib/groomer/plan-schema.test.ts b/src/lib/groomer/plan-schema.test.ts index c82bff3e..891e26dd 100644 --- a/src/lib/groomer/plan-schema.test.ts +++ b/src/lib/groomer/plan-schema.test.ts @@ -83,6 +83,32 @@ describe("buildGroomingPlanResponseSchema", () => { expect(criteria.items.properties.excerpt).toMatchObject({ type: "string", minLength: 24, maxLength: 300 }); }); + it("describes the full child brief shape (dispatch#1066)", () => { + const childBriefs = schema.properties.decomposition.properties.childBriefs; + expect(childBriefs.maxItems).toBe(8); + expect(childBriefs.items.required).toEqual([ + "title", + "problem", + "designDecision", + "verifiedCurrentBehavior", + "relevantPaths", + "inScope", + "outOfScope", + "dependencies", + "acceptanceCriteria", + "tests", + ]); + const props = childBriefs.items.properties; + expect(props.title).toMatchObject({ type: "string", minLength: 10, maxLength: 200 }); + expect(props.problem).toMatchObject({ type: "string", maxLength: 1000 }); + expect(props.designDecision).toMatchObject({ anyOf: [{ type: "null" }, { type: "string", maxLength: 1000 }] }); + expect(props.verifiedCurrentBehavior).toMatchObject({ anyOf: [{ type: "null" }, { type: "string", maxLength: 1000 }] }); + for (const key of ["relevantPaths", "inScope", "outOfScope", "dependencies", "tests"]) { + expect(props[key], key).toMatchObject({ type: "array", maxItems: 12, items: { type: "string", maxLength: 300 } }); + } + expect(props.acceptanceCriteria).toMatchObject({ type: "array", maxItems: 8, items: { type: "string", maxLength: 300 } }); + }); + it("forces arrays empty when the catalog has no ids of the needed kind", () => { const bare = buildGroomingPlanResponseSchema(buildEvidenceCatalog({ ...snapshot, sources: [] })) as Node; expect(bare.properties.relatedWork.maxItems).toBe(0); diff --git a/src/lib/groomer/plan-schema.ts b/src/lib/groomer/plan-schema.ts index cf8dde57..6a88fe57 100644 --- a/src/lib/groomer/plan-schema.ts +++ b/src/lib/groomer/plan-schema.ts @@ -140,7 +140,14 @@ export function buildGroomingPlanResponseSchema(catalog?: EvidenceCatalog): Sche obj({ title: str(L.titleMax, L.titleMin), problem: str(L.text), + designDecision: nullable(str(L.text)), + verifiedCurrentBehavior: nullable(str(L.text)), + relevantPaths: list(str(L.shortText), L.listItems), + inScope: list(str(L.shortText), L.listItems), + outOfScope: list(str(L.shortText), L.listItems), + dependencies: list(str(L.shortText), L.listItems), acceptanceCriteria: list(str(L.shortText), L.childCriteria), + tests: list(str(L.shortText), L.listItems), }), L.childBriefs, ), diff --git a/src/lib/groomer/plan.test.ts b/src/lib/groomer/plan.test.ts index 6756dcf1..05da1f8e 100644 --- a/src/lib/groomer/plan.test.ts +++ b/src/lib/groomer/plan.test.ts @@ -325,7 +325,26 @@ describe("GroomingPlan readiness invariant", () => { it("rejects implementation-ready work that needs decomposition", () => { expectInvalid( - draft({ decomposition: { required: true, reason: "two changes", childBriefs: [{ title: "Fix redirect after reset", problem: "p", acceptanceCriteria: [] }] } }), + draft({ + decomposition: { + required: true, + reason: "two changes", + childBriefs: [ + { + title: "Fix redirect after reset", + problem: "p", + designDecision: null, + verifiedCurrentBehavior: null, + relevantPaths: [], + inScope: [], + outOfScope: [], + dependencies: [], + acceptanceCriteria: [], + tests: [], + }, + ], + }, + }), "readiness: decomposition.required is true", ); }); @@ -533,6 +552,67 @@ describe("GroomingPlan validation failures", () => { expectInvalid(notReady("backlog", { decomposition: { required: true, reason: "umbrella", childBriefs: [] } }), "decomposition.childBriefs"); }); + describe("child briefs (dispatch#1066)", () => { + const fullChild = { + title: "Add order search to the admin dashboard", + problem: "Admins cannot search orders from the dashboard.", + designDecision: "Search lives in a dedicated admin sub-route, not a dashboard widget.", + verifiedCurrentBehavior: "Dashboard.tsx renders four unrelated legacy widgets; routes.ts has no admin sub-routes.", + relevantPaths: ["src/admin/Dashboard.tsx", "src/admin/routes.ts"], + inScope: ["the order search query and results list"], + outOfScope: ["moving the dashboard to the new design system"], + dependencies: ["the refund approval queue child"], + acceptanceCriteria: ["an admin can search orders by id and see the match"], + tests: ["src/admin/orders-search.test.ts"], + }; + + it("round-trips every field of a full child brief", () => { + const plan = validPlan( + notReady("backlog", { decomposition: { required: true, reason: "umbrella", childBriefs: [fullChild] } }), + ); + expect(plan.decomposition.childBriefs).toEqual([fullChild]); + }); + + it("parses an old three-field brief with the new fields defaulted to null/empty", () => { + const old = { + ...notReady("backlog"), + decomposition: { + required: true, + reason: "umbrella", + childBriefs: [{ title: "Fix redirect after reset", problem: "p", acceptanceCriteria: ["a reset-then-login test passes"] }], + }, + } as unknown as Record<string, unknown>; + const plan = validPlan(old); + expect(plan.decomposition.childBriefs).toEqual([ + { + title: "Fix redirect after reset", + problem: "p", + designDecision: null, + verifiedCurrentBehavior: null, + relevantPaths: [], + inScope: [], + outOfScope: [], + dependencies: [], + acceptanceCriteria: ["a reset-then-login test passes"], + tests: [], + }, + ]); + }); + + it("bounds child brief fields with the field path", () => { + const overList = { ...fullChild, relevantPaths: Array.from({ length: PLAN_LIMITS.listItems + 1 }, (_, i) => `path/${i}.ts`) }; + expectInvalid( + notReady("backlog", { decomposition: { required: true, reason: "umbrella", childBriefs: [overList] } }), + `decomposition.childBriefs[0].relevantPaths: must have at most ${PLAN_LIMITS.listItems} items`, + ); + const overText = { ...fullChild, designDecision: "x".repeat(PLAN_LIMITS.text + 1) }; + expectInvalid( + notReady("backlog", { decomposition: { required: true, reason: "umbrella", childBriefs: [overText] } }), + `decomposition.childBriefs[0].designDecision: must be at most ${PLAN_LIMITS.text} characters`, + ); + }); + }); + describe("close decisions", () => { const done = (patch: DraftPatch = {}) => draft({ diff --git a/src/lib/groomer/plan.ts b/src/lib/groomer/plan.ts index 545a38c0..c9c9368e 100644 --- a/src/lib/groomer/plan.ts +++ b/src/lib/groomer/plan.ts @@ -170,10 +170,30 @@ export interface GroomingMutationIntent { close: CloseRecommendation | null; } +/** + * One bounded child brief (dispatch#1066). Rich enough that a fresh worker + * with no other context can implement the child: problem/motivation, the + * current behavior the parent's analysis verified, the relevant code paths, + * the settled design decision, explicit scope, dependencies, deterministic + * acceptance criteria and tests. The child still gets its own evidence-backed + * grooming pass before worker admission; the brief orients, it does not + * certify. + */ export interface ChildBrief { title: string; problem: string; + /** The settled design decision this child implements; null when there is no decision left. */ + designDecision: string | null; + /** Current behavior verified by the parent's analysis; null when not verified. */ + verifiedCurrentBehavior: string | null; + /** Current relevant code paths the child touches or reads. */ + relevantPaths: string[]; + inScope: string[]; + outOfScope: string[]; + /** Descriptive only: dependencies between siblings or on external work. */ + dependencies: string[]; acceptanceCriteria: string[]; + tests: string[]; } export interface GroomingDecomposition { @@ -448,7 +468,14 @@ function parseDraft(data: Obj, r: Reader): GroomingPlanDraft { return { title: r.text(c.title, `${path}.title`, L.titleMax, L.titleMin), problem: r.text(c.problem, `${path}.problem`, L.text), + designDecision: r.optionalText(c.designDecision, `${path}.designDecision`, L.text), + verifiedCurrentBehavior: r.optionalText(c.verifiedCurrentBehavior, `${path}.verifiedCurrentBehavior`, L.text), + relevantPaths: r.textList(c.relevantPaths, `${path}.relevantPaths`, L.listItems, L.shortText), + inScope: r.textList(c.inScope, `${path}.inScope`, L.listItems, L.shortText), + outOfScope: r.textList(c.outOfScope, `${path}.outOfScope`, L.listItems, L.shortText), + dependencies: r.textList(c.dependencies, `${path}.dependencies`, L.listItems, L.shortText), acceptanceCriteria: r.textList(c.acceptanceCriteria, `${path}.acceptanceCriteria`, L.childCriteria, L.shortText), + tests: r.textList(c.tests, `${path}.tests`, L.listItems, L.shortText), }; }), }; diff --git a/src/lib/groomer/prompts/system-prompt.test.ts b/src/lib/groomer/prompts/system-prompt.test.ts index f981a86f..9de4d303 100644 --- a/src/lib/groomer/prompts/system-prompt.test.ts +++ b/src/lib/groomer/prompts/system-prompt.test.ts @@ -136,6 +136,36 @@ describe("buildGroomerSystemPrompt", () => { expect(prompt).toContain("decomposition.required is false"); }); + it("describes the full child brief shape and when children may be emitted (dispatch#1066)", () => { + const prompt = buildGroomerSystemPrompt(baseParams); + for (const field of [ + "designDecision", + "verifiedCurrentBehavior", + "relevantPaths", + "inScope", + "outOfScope", + "dependencies", + "acceptanceCriteria", + "tests", + ]) { + expect(prompt, field).toContain(field); + } + expect(prompt).toContain("complete bounded implementation brief"); + expect(prompt).toContain("never emit child briefs while a material design question remains unresolved"); + expect(prompt).toContain("that work goes to the escalation lane instead"); + expect(prompt).toContain("start as `status/backlog`"); + }); + + it("makes every child brief field required, matching the apply-time completeness gate (dispatch#1066)", () => { + const prompt = buildGroomerSystemPrompt(baseParams); + expect(prompt).toContain("every field is required and non-blank"); + expect(prompt).toContain("every list except dependencies must hold at least one entry"); + expect(prompt).toContain("dependencies is the only one that may be empty"); + expect(prompt).toContain("designDecision and verifiedCurrentBehavior must not be null"); + expect(prompt).toContain("no design choice; follow the existing pattern"); + expect(prompt).toContain("withholds the whole split"); + }); + it("lets unknowns be recorded instead of guessed", () => { const prompt = buildGroomerSystemPrompt(baseParams); expect(prompt).toContain("Unknown is an answer."); diff --git a/src/lib/groomer/prompts/system-prompt.ts b/src/lib/groomer/prompts/system-prompt.ts index b6e8ec04..3c2e7d81 100644 --- a/src/lib/groomer/prompts/system-prompt.ts +++ b/src/lib/groomer/prompts/system-prompt.ts @@ -65,7 +65,7 @@ Return ONLY valid JSON: a grooming plan with this shape (the response schema set "decomposition": { "required": false, "reason": null, "childBriefs": [] }, "relatedWork": [] } -Use null for implementationBrief when you cannot write one honestly (for example needs_info or design work). "close" is null or { "reason": "already_done|duplicate|superseded", "rationale": "...", "evidenceRefs": [...], "criteria": [{ "criterion": "...", "evidenceRef": "repo:path/you/read.ts", "excerpt": "..." }] } (criteria is [] unless the reason is already_done). relatedWork entries are { "ref": "github:issue:owner/repo#12", "relation": "duplicate_of|superseded_by|related", "note": "..." }: the ref is always a "github:" id from the evidence list. childBriefs entries are { "title": "...", "problem": "...", "acceptanceCriteria": ["..."] }. +Use null for implementationBrief when you cannot write one honestly (for example needs_info or design work). "close" is null or { "reason": "already_done|duplicate|superseded", "rationale": "...", "evidenceRefs": [...], "criteria": [{ "criterion": "...", "evidenceRef": "repo:path/you/read.ts", "excerpt": "..." }] } (criteria is [] unless the reason is already_done). relatedWork entries are { "ref": "github:issue:owner/repo#12", "relation": "duplicate_of|superseded_by|related", "note": "..." }: the ref is always a "github:" id from the evidence list. childBriefs entries are { "title": "...", "problem": "what the child must fix, in repo terms", "designDecision": "the settled design decision the child implements", "verifiedCurrentBehavior": "what the parent's analysis verified the code does today", "relevantPaths": ["repo:path/the/child/touches.ts"], "inScope": ["what the child may do"], "outOfScope": ["what the child must not do"], "dependencies": ["sibling or external work this child depends on"], "acceptanceCriteria": ["deterministic, observable result"], "tests": ["test to add or update"] }; every field is required and non-blank, and every list except dependencies must hold at least one entry — dependencies is the only one that may be empty. designDecision and verifiedCurrentBehavior must not be null: state the settled decision even when it is simply "no design choice; follow the existing pattern", and ground the current behavior in what the code was verified to do. A child brief missing any of these withholds the whole split, so write every child complete. Evidence rules: - The user message ends with "Evidence you can cite": the only valid evidence ids for this run. Cite ids exactly as listed wherever the plan asks for evidenceRefs or a ref. Never invent an id; an unknown id rejects the whole plan. @@ -78,7 +78,7 @@ Readiness rules (Dispatch rejects a "ready" plan that breaks any of these): - verdict.evidenceRefs cites at least one "repo:" id read at the pinned head SHA, and confidence is not low. - No material uncertainty remains. - For implementation work: implementationBrief is present, verifiedCurrentBehavior cites "repo:" evidence, at least one relevant path or file to create is named, every relevantPaths entry with change "modify" is a "repo:" id read at the pinned head SHA (a path only surfaced by search may have moved), inScope is not empty, and every acceptance criterion is deterministic: its verification is automated_test, command or code_inspection, never subjective. -- decomposition.required is false. When an issue bundles several independently shippable changes, set decomposition.required with one childBrief per change; that issue is not implementation-ready. +- decomposition.required is false. When an issue bundles several independently shippable changes, set decomposition.required with one childBrief per change; that issue is not implementation-ready. Every childBrief must be a complete bounded implementation brief (problem, verified current behavior, relevant paths, the settled design decision, in/out of scope, dependencies, acceptance criteria, tests) written for a worker with no other context; never emit child briefs while a material design question remains unresolved — that work goes to the escalation lane instead. Created children start as \`status/backlog\` and get their own grooming pass. - mutations.close is null. If any rule fails, the issue is not ready: pick the actionability that says why and record what is missing. diff --git a/src/lib/groomer/run.test.ts b/src/lib/groomer/run.test.ts index c2195f07..704bac5a 100644 --- a/src/lib/groomer/run.test.ts +++ b/src/lib/groomer/run.test.ts @@ -26,6 +26,7 @@ const { mocks } = vi.hoisted(() => ({ addIssueComment: vi.fn(), updateIssueTitleAndBody: vi.fn(), closeIssue: vi.fn(), + createIssue: vi.fn(), findActiveLeasesForIssue: vi.fn(), upsertLease: vi.fn(), releaseLease: vi.fn(), @@ -39,10 +40,12 @@ const { mocks } = vi.hoisted(() => ({ releaseGroomerLock: vi.fn(), compareCommits: vi.fn(), applications: new Map<string, Record<string, any>>(), + childClaims: new Map<string, Record<string, any>>(), prisma: { automationRepo: { findUnique: vi.fn() }, groomingRun: { create: vi.fn(), update: vi.fn(), findFirst: vi.fn(), findMany: vi.fn() }, groomingApplication: { findUnique: vi.fn(), create: vi.fn(), update: vi.fn(), updateMany: vi.fn() }, + groomingChildClaim: { findUnique: vi.fn(), create: vi.fn(), update: vi.fn() }, issue: { update: vi.fn(), findMany: vi.fn() }, issueLane: { create: vi.fn() }, agentRun: { create: vi.fn() }, @@ -73,6 +76,7 @@ vi.mock("@/lib/github", () => ({ addIssueComment: mocks.addIssueComment, updateIssueTitleAndBody: mocks.updateIssueTitleAndBody, closeIssue: mocks.closeIssue, + createIssue: mocks.createIssue, addIssueLabel: mocks.addIssueLabel, removeIssueLabel: mocks.removeIssueLabel, })); @@ -287,6 +291,7 @@ describe("runHostedGroomer", () => { mocks.getHostedGroomerConfig.mockReturnValue(mockConfig); mocks.callGroomerLLM.mockResolvedValue(mockOutput); mocks.updateIssueLabels.mockResolvedValue(undefined); + mocks.addIssueLabel.mockResolvedValue(undefined); mocks.updateIssueTitleAndBody.mockResolvedValue(undefined); mocks.addIssueComment.mockResolvedValue({ url: null }); mocks.closeIssue.mockResolvedValue(undefined); @@ -354,6 +359,33 @@ describe("runHostedGroomer", () => { return row; }, ); + // In-memory GroomingChildClaim with the unique childKey claim (dispatch#1066). + // A created row has no childNumber/childUrl until the creation is recorded, + // matching the nullable columns the reuse check reads; updatedAt is stamped + // at create, mirroring @updatedAt, and applicationKey is carried from the + // claim input. + mocks.childClaims.clear(); + mocks.prisma.groomingChildClaim.findUnique.mockImplementation( + async ({ where }: { where: { childKey: string } }) => mocks.childClaims.get(where.childKey) ?? null, + ); + mocks.prisma.groomingChildClaim.create.mockImplementation(async ({ data }: { data: Record<string, any> }) => { + if (mocks.childClaims.has(data.childKey)) throw Object.assign(new Error("Unique constraint"), { code: "P2002" }); + const row = { ...data, childNumber: null, childUrl: null, updatedAt: new Date() }; + mocks.childClaims.set(data.childKey, row); + return row; + }); + mocks.prisma.groomingChildClaim.update.mockImplementation( + async ({ where, data }: { where: { childKey: string }; data: Record<string, any> }) => { + const row = mocks.childClaims.get(where.childKey)!; + Object.assign(row, JSON.parse(JSON.stringify(data))); + return row; + }, + ); + // Each created child gets its own number and URL, as GitHub would. + mocks.createIssue.mockImplementation(async (repoFullName: string) => { + const number = mocks.createIssue.mock.calls.length + 1001; + return { number, html_url: `https://github.com/${repoFullName}/issues/${number}` }; + }); }); it("returns null when no grooming candidate available", async () => { @@ -2601,6 +2633,199 @@ Investigate session handling in auth module.`; }); }); + describe("decomposition child creation (dispatch#1066)", () => { + const childBrief = (n: number) => ({ + title: `Bounded child ${n}`, + problem: `Child ${n} problem, as its own bounded change.`, + // Every field the apply-time decomposition policy requires a complete + // bounded brief to carry: an incomplete brief withholds the split. + designDecision: `Child ${n} follows the existing pattern; no design choice is left open.`, + verifiedCurrentBehavior: `Child ${n} is not implemented yet; login.ts drops the return URL.`, + relevantPaths: ["src/auth/login.ts"], + inScope: [`child ${n}`], + outOfScope: [`Child ${n} does not touch authentication`], + dependencies: [], + acceptanceCriteria: [`Child ${n} works end to end`], + tests: [`Child ${n} is covered by an automated test`], + }); + + /** A non-ready (backlog) verdict that splits the issue into `count` bounded children. */ + const decomposingDraft = (count: number) => + notReadyDraft("backlog", { + decomposition: { + required: true, + reason: "the issue spans several independent areas", + childBriefs: Array.from({ length: count }, (_, i) => childBrief(i + 1)), + }, + }); + + it("creates one bounded child per brief, decorates the parent as an umbrella, and records the decomposition", async () => { + mocks.callGroomerLLM.mockResolvedValue(decomposingDraft(2)); + const result = await runHostedGroomer(); + + expect(result!.appliedMutations).toMatchObject({ outcome: "applied" }); + expect(mocks.createIssue).toHaveBeenCalledTimes(2); + // Each child lands as a backlog issue (not worker-ready). + expect(mocks.createIssue).toHaveBeenCalledWith( + "org/repo", + expect.objectContaining({ labels: ["status/backlog"] }), + ); + // The umbrella is an additive write by the children step, not part of the labels write. + expect(mocks.updateIssueLabels).toHaveBeenCalledTimes(1); + expect(mocks.updateIssueLabels).toHaveBeenCalledWith("org/repo", 42, expect.not.arrayContaining(["umbrella"])); + expect(mocks.addIssueLabel).toHaveBeenCalledTimes(1); + expect(mocks.addIssueLabel).toHaveBeenCalledWith("org/repo", 42, "umbrella"); + // The parent's decomposition state is persisted with the child URLs as follow-ups. + expect(mocks.prisma.issue.update).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ decomposed: true, followUpUrls: expect.any(Array) }), + }), + ); + expect(mocks.prisma.auditLog.create).toHaveBeenCalledWith( + expect.objectContaining({ data: expect.objectContaining({ action: "issue_decomposed" }) }), + ); + // The run records the created child links, in brief order. + expect(result!.appliedMutations!.childrenCreated).toHaveLength(2); + // The children step writes a managed decomposition section into the + // parent body: the marker pair plus one `- #<n>: <url>` line per + // created child. The content step wrote nothing (no title/body in the + // plan), so this is the only body write. + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalledTimes(1); + const written = mocks.updateIssueTitleAndBody.mock.calls[0][2] as { body?: string | null }; + expect(written.body).toContain("<!-- dispatch-groomer:decomposition:start -->"); + expect(written.body).toContain("<!-- dispatch-groomer:decomposition:end -->"); + const created = result!.appliedMutations!.childrenCreated as { number: number; url: string }[]; + for (const child of created) { + expect(written.body).toContain(`- #${child.number}: ${child.url}`); + } + }); + + it("an exact retry is a replay: it creates no new child and re-surfaces the created links", async () => { + mocks.callGroomerLLM.mockResolvedValue(decomposingDraft(2)); + + const first = await runHostedGroomer(); + const second = await runHostedGroomer(); + + expect(first!.appliedMutations).toMatchObject({ outcome: "applied" }); + expect(second!.appliedMutations).toMatchObject({ outcome: "replayed" }); + // Only the first attempt created the children; the replay re-surfaces them. + expect(mocks.createIssue).toHaveBeenCalledTimes(2); + expect(first!.appliedMutations!.childrenCreated).toHaveLength(2); + expect(second!.appliedMutations!.childrenCreated).toHaveLength(2); + // The umbrella is added once, on the first attempt; the replay adds none. + expect(mocks.addIssueLabel).toHaveBeenCalledTimes(1); + expect(mocks.addIssueLabel).toHaveBeenCalledWith("org/repo", 42, "umbrella"); + // Same application key, so the replay is attributable to the first run. + expect(first!.mutationPlan!.applicationKey).toBe(second!.mutationPlan!.applicationKey); + }); + + it("a child-creation failure is a partial, retryable run; the retry reuses the landed child and creates only the missing ones", async () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + mocks.callGroomerLLM.mockResolvedValue(decomposingDraft(3)); + let createCalls = 0; + mocks.createIssue.mockImplementation(async () => { + createCalls += 1; + if (createCalls === 2) throw new Error("GitHub API error creating issue: 502"); + const number = createCalls + 1000; + return { number, html_url: `https://github.com/org/repo/issues/${number}` }; + }); + + const first = await runHostedGroomer(); + expect(first!.appliedMutations).toMatchObject({ outcome: "partial" }); + expect(first!.appliedMutations!.childrenError).toMatch(/502/); + // A partial decomposition never lands the umbrella. + expect(mocks.addIssueLabel).not.toHaveBeenCalled(); + + // The retry: the failing creation now succeeds. + mocks.createIssue.mockImplementation(async () => { + createCalls += 1; + const number = createCalls + 1000; + return { number, html_url: `https://github.com/org/repo/issues/${number}` }; + }); + const second = await runHostedGroomer(); + errSpy.mockRestore(); + + expect(second!.appliedMutations).toMatchObject({ outcome: "applied" }); + // The child that landed before the failure is reused; only the missing ones are created. + expect(second!.appliedMutations!.childrenCreated).toHaveLength(2); + expect(second!.appliedMutations!.childrenReused).toHaveLength(1); + // The umbrella lands once the decomposition fully converges. + expect(mocks.addIssueLabel).toHaveBeenCalledTimes(1); + expect(mocks.addIssueLabel).toHaveBeenCalledWith("org/repo", 42, "umbrella"); + // One claim per distinct child, across both attempts (idempotent child identity). + expect(mocks.prisma.groomingChildClaim.create).toHaveBeenCalledTimes(3); + }); + + it("creates no children when the split is withheld for low confidence", async () => { + mocks.callGroomerLLM.mockResolvedValue( + notReadyDraft("backlog", { + verdict: { confidence: "low" }, + decomposition: { required: true, reason: "split it", childBriefs: [childBrief(1)] }, + }), + ); + const result = await runHostedGroomer(); + + expect(result!.appliedMutations).toMatchObject({ outcome: "applied" }); + expect(result!.appliedMutations!.withheld).toMatchObject({ decomposition: expect.any(Array) }); + expect(mocks.createIssue).not.toHaveBeenCalled(); + expect(mocks.addIssueLabel).not.toHaveBeenCalled(); + expect(mocks.prisma.groomingChildClaim.create).not.toHaveBeenCalled(); + }); + + it("creates no children when a material uncertainty remains", async () => { + mocks.callGroomerLLM.mockResolvedValue( + notReadyDraft("backlog", { + verdict: { uncertainties: [{ kind: "scope", question: "Which child owns the migration?", material: true }] }, + decomposition: { required: true, reason: "split it", childBriefs: [childBrief(1)] }, + }), + ); + const result = await runHostedGroomer(); + + expect(result!.appliedMutations).toMatchObject({ outcome: "applied" }); + expect(result!.appliedMutations!.withheld).toMatchObject({ decomposition: expect.any(Array) }); + expect(mocks.createIssue).not.toHaveBeenCalled(); + expect(mocks.addIssueLabel).not.toHaveBeenCalled(); + expect(mocks.prisma.groomingChildClaim.create).not.toHaveBeenCalled(); + }); + + it("withholds the decomposition end to end when a child brief is not a complete bounded implementation brief", async () => { + // Everything complete except the settled design decision: a child that + // leaves a design choice open is not bounded, so the split is withheld. + mocks.callGroomerLLM.mockResolvedValue( + notReadyDraft("backlog", { + decomposition: { + required: true, + reason: "split it", + childBriefs: [{ ...childBrief(1), designDecision: null }], + }, + }), + ); + const result = await runHostedGroomer(); + + // The plan still lands (as backlog); only the decomposition is withheld. + expect(result!.appliedMutations).toMatchObject({ outcome: "applied" }); + // The run records exactly which child and which field is short. + const withheld = result!.appliedMutations!.withheld as { decomposition: string[] }; + expect(withheld.decomposition).toEqual([ + "child brief[0] is not a complete bounded implementation brief (missing: designDecision)", + ]); + // No child is created, no umbrella lands, and the parent is not decomposed. + expect(mocks.createIssue).not.toHaveBeenCalled(); + expect(mocks.addIssueLabel).not.toHaveBeenCalled(); + expect(mocks.prisma.groomingChildClaim.create).not.toHaveBeenCalled(); + expect( + (mocks.prisma.issue.update.mock.calls.map((c) => c[0].data as Record<string, unknown>) ?? []).some( + (data) => data.decomposed === true, + ), + ).toBe(false); + expect( + (mocks.prisma.auditLog.create.mock.calls.map((c) => c[0].data as Record<string, unknown>) ?? []).some( + (data) => data.action === "issue_decomposed", + ), + ).toBe(false); + }); + }); + it("strips a status the groomer does not own, so the derived status is the only one", async () => { mocks.selectGroomingCandidate.mockResolvedValue({ ...mockCandidate, labels: ["priority/p0", "status/needs-review"] }); mocks.callGroomerLLM.mockResolvedValue(notReadyDraft("backlog")); diff --git a/src/lib/groomer/run.ts b/src/lib/groomer/run.ts index d434772f..26549b65 100644 --- a/src/lib/groomer/run.ts +++ b/src/lib/groomer/run.ts @@ -1,5 +1,5 @@ import { prisma } from "@/lib/prisma"; -import { addIssueComment, closeIssue, updateIssueLabels, updateIssueTitleAndBody } from "@/lib/github"; +import { addIssueComment, addIssueLabel, closeIssue, createIssue, updateIssueLabels, updateIssueTitleAndBody } from "@/lib/github"; import { findActiveLeasesForIssue, releaseLease, upsertLease } from "@/lib/lease"; import { acquireGroomerLock, heartbeatGroomerLock, HEARTBEAT_MS, releaseGroomerLock } from "./groomer-lock"; import { selectGroomingCandidate } from "./selector"; @@ -107,6 +107,10 @@ export interface GroomerDeps { addComment: typeof addIssueComment; updateTitleAndBody: typeof updateIssueTitleAndBody; closeIssue: typeof closeIssue; + /** Open a decomposition child issue (dispatch#1066). */ + createIssue: typeof createIssue; + /** Add a single label (the children step's umbrella, dispatch#1066). */ + addLabel: typeof addIssueLabel; findActiveLeases: typeof findActiveLeasesForIssue; upsertLease: typeof upsertLease; releaseLease: typeof releaseLease; @@ -138,6 +142,8 @@ const defaultDeps: GroomerDeps = { addComment: addIssueComment, updateTitleAndBody: updateIssueTitleAndBody, closeIssue, + createIssue, + addLabel: addIssueLabel, findActiveLeases: findActiveLeasesForIssue, upsertLease, releaseLease, @@ -581,7 +587,9 @@ async function executeGroomerRun( } for (const [what, reasons] of Object.entries(diff.withheld)) { - contextWarnings.push(`apply: withheld ${what === "close" ? "the already_done close" : "the ready promotion"}: ${reasons!.join("; ")}`); + const withheldName = + what === "close" ? "the already_done close" : what === "ready" ? "the ready promotion" : "the decomposition"; + contextWarnings.push(`apply: withheld ${withheldName}: ${reasons!.join("; ")}`); } // Post-condition invariant (dispatch#941): exactly one status/* label. @@ -597,6 +605,7 @@ async function executeGroomerRun( notReadyReason: notReadyReason ?? null, willComment: diff.comment !== null, willCloseIssue: diff.close, + willCreateChildren: diff.children !== null, titleRewritten: diff.title !== null, originalTitle: diff.title !== null ? analyzed.title : undefined, proposedTitle: diff.title ?? undefined, @@ -624,6 +633,7 @@ async function executeGroomerRun( inFlightStatus: inFlight, willComment: false, willCloseIssue: false, + willCreateChildren: false, titleRewritten: false, bodyEnriched: false, }); @@ -834,6 +844,12 @@ async function executeGroomerRun( addComment: deps.addComment, updateTitleAndBody: deps.updateTitleAndBody, closeIssue: deps.closeIssue, + addLabel: deps.addLabel, + // The GitHub client returns `html_url`; the applier wants `url`. + createIssue: async (repoFullName, input) => { + const created = await deps.createIssue(repoFullName, input); + return { number: created.number, url: created.html_url }; + }, fetchRecentComments: (_repo, _number, max) => reader.fetchRecentComments(max), }; const applied = await applyGroomingMutations( @@ -841,6 +857,7 @@ async function executeGroomerRun( repoFullName: candidate.repoFullName, issueNumber: candidate.number, issueId: candidate.id, + parentUrl: candidate.url, groomingRunId: groomingRun.id, applicationKey, diff, @@ -1137,6 +1154,14 @@ function describeApplication(applied: ApplyResult, withheld: Record<string, stri out.titleUpdated = applied.title !== null; out.bodyUpdated = applied.body !== null; } + const childrenStep = steps.children; + if (childrenStep) { + if (childrenStep.children) { + if (childrenStep.children.created.length > 0) out.childrenCreated = childrenStep.children.created; + if (childrenStep.children.reused.length > 0) out.childrenReused = childrenStep.children.reused; + } + if (childrenStep.status === "failed") out.childrenError = childrenStep.error; + } const comment = steps.comment; if (comment && comment.status !== "noop") { if (applied.commentUrl) out.commentUrl = applied.commentUrl; diff --git a/src/lib/issue-filters.ts b/src/lib/issue-filters.ts index 7020ba75..7743904e 100644 --- a/src/lib/issue-filters.ts +++ b/src/lib/issue-filters.ts @@ -1,4 +1,5 @@ import { AGENT_PREFIX, isAgentLabel, isOwnerLabel, OWNER_PREFIX, STATUS_LABELS } from "@/types"; +import { UMBRELLA_LABEL } from "@/lib/decomposition"; /** * Single source of truth for Renovate issue detection. Both the in-memory @@ -141,7 +142,7 @@ export function buildUmbrellaIssueExclusionWhere() { return { NOT: { OR: [ - { labels: { has: "umbrella" } }, + { labels: { has: UMBRELLA_LABEL } }, { title: { startsWith: "Weekly tech debt audit:", mode: "insensitive" } }, ], },