FN-5928: enforce surface enumeration for bug-fix invariants
Require bug-fix specs and reviews to enumerate affected surfaces and reject repro-only regression coverage. - add a required `## Surface Enumeration` section to triage prompt templates and bug-fix planning guidance - tighten reviewer guidance to block missing surface enumeration and repro-only regression tests - document the canonical surface checklist in `docs/testing.md` and cover the new wording with prompt/reviewer tests Files changed: AGENTS.md | 6 ++-- docs/testing.md | 9 +++++ packages/core/src/__tests__/agent-prompts.test.ts | 22 ++++++++++++ packages/core/src/agent-prompts.ts | 16 +++++++++ packages/engine/src/__tests__/reviewer.test.ts | 17 ++++++++++ packages/engine/src/__tests__/triage.test.ts | 41 ++++++++++++++++++++--- packages/engine/src/reviewer.ts | 3 ++ packages/engine/src/triage.ts | 24 +++++++++++++ 8 files changed, 131 insertions(+), 7 deletions(-) Fusion-Task-Id: FN-5928 Fusion-Task-Lineage: 717ddcbe-f3a6-4589-ad90-4e640f7a9ff2
This commit is contained in:
gsxdsm
@@ -287,6 +287,28 @@ describe("resolveAgentPrompt", () => {
|
||||
expect(coreBlock).toBe(engineBlock);
|
||||
});
|
||||
|
||||
it("built-in triage prompt requires surface enumeration for bug-fix specs", () => {
|
||||
const triagePrompt = resolveAgentPrompt("triage");
|
||||
expect(triagePrompt).toContain("## Surface Enumeration");
|
||||
expect(triagePrompt).toContain("spec MUST include a `## Surface Enumeration` section");
|
||||
expect(triagePrompt).toContain("blocking REVISE");
|
||||
});
|
||||
|
||||
it("built-in reviewer prompts reject missing surface enumeration and repro-only bug-fix tests", () => {
|
||||
const defaultReviewer = resolveAgentPrompt("reviewer");
|
||||
const strictReviewer = resolveAgentPrompt("reviewer", {
|
||||
roleAssignments: { reviewer: "strict-reviewer" },
|
||||
});
|
||||
|
||||
for (const prompt of [defaultReviewer, strictReviewer]) {
|
||||
expect(prompt).toContain("**Surface enumeration:**");
|
||||
expect(prompt).toContain("Missing or incomplete coverage is a blocking REVISE");
|
||||
expect(prompt).toContain("repro-only regression test");
|
||||
expect(prompt).toContain("spanning the `## Surface Enumeration` checklist");
|
||||
expect(prompt).toContain("FN-5797/FN-5875/FN-5919");
|
||||
}
|
||||
});
|
||||
|
||||
it("default role prompts include explicit heartbeat run guidance", () => {
|
||||
expect(resolveAgentPrompt("executor")).toContain("## Heartbeat Run Behavior");
|
||||
expect(resolveAgentPrompt("triage")).toContain("## Heartbeat Run Behavior");
|
||||
|
||||
@@ -233,6 +233,10 @@ Follow this structure exactly:
|
||||
|
||||
{One paragraph: what you're building and why it matters}
|
||||
|
||||
## Surface Enumeration
|
||||
|
||||
{Required for bug-fix tasks: a checklist enumerating every surface the fixed invariant must hold across. Include every provider/bridge for streaming and agent paths; desktop AND mobile breakpoints; empty/undefined/duplicate/populated data states; and every hook/component/module that shares the affected logic. Use the canonical checklist in docs/testing.md as the starting point.}
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **None**
|
||||
@@ -263,6 +267,12 @@ Follow this structure exactly:
|
||||
- [ ] {Specific, verifiable outcome}
|
||||
- [ ] Run targeted tests for changed files, asserting the invariant across all known surfaces (enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states)
|
||||
|
||||
For bug-fix tasks, paste and fill in this checklist in the \`## Surface Enumeration\` section:
|
||||
- [ ] Providers / bridges / execution paths touched by the invariant
|
||||
- [ ] Desktop + mobile breakpoints / platforms that exercise the behavior
|
||||
- [ ] Empty / undefined / duplicate / populated data states
|
||||
- [ ] Shared hooks / components / modules / helpers reusing the logic
|
||||
|
||||
**Artifacts:**
|
||||
- \`path/to/file\` (new | modified)
|
||||
|
||||
@@ -330,6 +340,8 @@ files with assertions that run via a test runner. Typechecks and builds are NOT
|
||||
tests. Manual verification is NOT a test.
|
||||
|
||||
- Each implementation step should include writing tests for the code being changed
|
||||
- For bug fixes, the spec MUST include a \`## Surface Enumeration\` section. During self-review via \`fn_review_spec()\`, treat a missing section on a bug-fix spec as a blocking REVISE.
|
||||
- For bug fixes, populate \`## Surface Enumeration\` with this checklist from \`docs/testing.md\`: providers/bridges/execution paths; desktop + mobile breakpoints/platforms; empty/undefined/duplicate/populated data states; shared hooks/components/modules/helpers.
|
||||
- For bug fixes, regression tests must assert the invariant across all known surfaces — enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states — not just the reported repro (see FN-5787/FN-5789/FN-5803 and FN-5751)
|
||||
- The final Testing step runs lint, the FULL test suite, and project typecheck when the repo exposes one
|
||||
- Specs must instruct executors to fix lint failures and quality-gate failures directly, even when the required edits extend beyond the original File Scope
|
||||
@@ -521,6 +533,7 @@ access to the codebase and can run commands to inspect code.
|
||||
|
||||
### Test Gaps
|
||||
- [Missing test scenarios]
|
||||
- [For bug fixes, call out any repro-only regression test that does not assert the invariant across the enumerated surfaces. Issue REVISE when coverage stops at the single reported case instead of spanning the \`## Surface Enumeration\` checklist (FN-5893; see FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751).]
|
||||
|
||||
### Suggestions
|
||||
- [Optional improvements, not blocking]
|
||||
@@ -545,6 +558,7 @@ access to the codebase and can run commands to inspect code.
|
||||
- **File scope accuracy:** [All affected files listed? No extras?]
|
||||
- **Dependency correctness:** [Dependencies exist and are appropriate?]
|
||||
- **Testing requirements:** [Real automated tests required, not just typechecks?]
|
||||
- **Surface enumeration:** [For bug-fix specs, is \`## Surface Enumeration\` present and does it enumerate the relevant providers/bridges/execution paths, desktop + mobile breakpoints/platforms, empty/undefined/duplicate/populated states, and shared hooks/components/modules/helpers? Missing or incomplete coverage is a blocking REVISE.]
|
||||
- **Documentation completeness:** [Must Update / Check If Affected sections present?]
|
||||
- **Sizing & review level:** [Size and review level appropriate for the work?]
|
||||
- **Subtask breakdown:** [Were complex tasks appropriately split into 2-5 child tasks? A task with 8+ implementation steps, affecting 3+ packages, should have been divided]
|
||||
@@ -795,6 +809,7 @@ submissions to a high bar for correctness, security, and maintainability.
|
||||
|
||||
### Test Gaps
|
||||
- [Missing test scenarios including edge cases]
|
||||
- [For bug fixes, call out any repro-only regression test that does not assert the invariant across the enumerated surfaces. Issue REVISE when coverage stops at the single reported case instead of spanning the \`## Surface Enumeration\` checklist (FN-5893; see FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751).]
|
||||
|
||||
### Backward Compatibility
|
||||
- [Any breaking changes or migration needs]
|
||||
@@ -822,6 +837,7 @@ submissions to a high bar for correctness, security, and maintainability.
|
||||
- **File scope accuracy:** [All affected files listed? No extras?]
|
||||
- **Dependency correctness:** [Dependencies exist and are appropriate?]
|
||||
- **Testing requirements:** [Real automated tests required, not just typechecks?]
|
||||
- **Surface enumeration:** [For bug-fix specs, is \`## Surface Enumeration\` present and does it enumerate the relevant providers/bridges/execution paths, desktop + mobile breakpoints/platforms, empty/undefined/duplicate/populated states, and shared hooks/components/modules/helpers? Missing or incomplete coverage is a blocking REVISE.]
|
||||
- **Documentation completeness:** [Must Update / Check If Affected sections present?]
|
||||
- **Sizing & review level:** [Size and review level appropriate for the work?]
|
||||
- **Subtask breakdown:** [Were complex tasks appropriately split into 2-5 child tasks?]
|
||||
|
||||
@@ -291,6 +291,23 @@ describe("reviewStep — spec review type", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("FN-5928 surface-enumeration review-gate wording", () => {
|
||||
it("requires spec reviews to block missing or incomplete surface enumeration for bug-fix specs", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("**Surface enumeration:**");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("Missing or incomplete coverage is a blocking REVISE");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("desktop + mobile breakpoints/platforms");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("shared hooks/components/modules/helpers");
|
||||
});
|
||||
|
||||
it("requires code reviews to reject repro-only regression tests for bug fixes", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("repro-only regression test");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("spanning the `## Surface Enumeration` checklist");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-5787/FN-5789/FN-5803");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-5797/FN-5875/FN-5919");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-5751");
|
||||
});
|
||||
});
|
||||
|
||||
describe("reviewStep — context-limit retry", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
@@ -648,12 +648,12 @@ describe("TRIAGE_SYSTEM_PROMPT", () => {
|
||||
});
|
||||
|
||||
describe("FN-5893 invariant regression wording", () => {
|
||||
it("requires invariant-level regression coverage in standard, fast, and core triage prompts", () => {
|
||||
const corePromptSource = readFileSync(
|
||||
fileURLToPath(new URL("../../../core/src/agent-prompts.ts", import.meta.url)),
|
||||
"utf8",
|
||||
);
|
||||
const corePromptSource = readFileSync(
|
||||
fileURLToPath(new URL("../../../core/src/agent-prompts.ts", import.meta.url)),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
it("requires invariant-level regression coverage in standard, fast, and core triage prompts", () => {
|
||||
for (const prompt of [
|
||||
TRIAGE_SYSTEM_PROMPT,
|
||||
FAST_TRIAGE_SYSTEM_PROMPT,
|
||||
@@ -668,13 +668,44 @@ describe("FN-5893 invariant regression wording", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("requires a Surface Enumeration section and blocking REVISE guidance for bug-fix specs", () => {
|
||||
for (const prompt of [TRIAGE_SYSTEM_PROMPT, FAST_TRIAGE_SYSTEM_PROMPT]) {
|
||||
expect(prompt).toContain("## Surface Enumeration");
|
||||
expect(prompt).toContain("spec MUST include a `## Surface Enumeration` section");
|
||||
expect(prompt).toContain("blocking REVISE");
|
||||
expect(prompt).toContain("docs/testing.md");
|
||||
expect(prompt).toContain("duplicate / populated data states");
|
||||
expect(prompt).toContain("shared hooks/components/modules/helpers");
|
||||
}
|
||||
|
||||
expect(corePromptSource).toContain("## Surface Enumeration");
|
||||
expect(corePromptSource).toContain("spec MUST include a \\`## Surface Enumeration\\` section");
|
||||
expect(corePromptSource).toContain("blocking REVISE");
|
||||
expect(corePromptSource).toContain("docs/testing.md");
|
||||
expect(corePromptSource).toContain("duplicate / populated data states");
|
||||
expect(corePromptSource).toContain("shared hooks/components/modules/helpers");
|
||||
});
|
||||
|
||||
it("requires implementation-step testing guidance to enumerate invariant surfaces in standard and fast prompts", () => {
|
||||
for (const prompt of [TRIAGE_SYSTEM_PROMPT, FAST_TRIAGE_SYSTEM_PROMPT]) {
|
||||
expect(prompt).toContain(
|
||||
"Run targeted tests for changed files, asserting the invariant across all known surfaces",
|
||||
);
|
||||
expect(prompt).toContain(
|
||||
"For bug-fix tasks, paste and fill in this checklist in the `## Surface Enumeration` section",
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("pins the canonical docs checklist heading", () => {
|
||||
const docsTestingSource = readFileSync(
|
||||
fileURLToPath(new URL("../../../../docs/testing.md", import.meta.url)),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
expect(docsTestingSource).toContain("### Surface Enumeration checklist");
|
||||
expect(docsTestingSource).toContain("Providers / bridges / execution paths touched by the invariant");
|
||||
});
|
||||
});
|
||||
|
||||
describe("fast-mode triage", () => {
|
||||
|
||||
@@ -120,6 +120,7 @@ Concrete examples:
|
||||
|
||||
### Test Gaps
|
||||
- [Missing test scenarios]
|
||||
- [For bug fixes, call out any repro-only regression test that does not assert the invariant across the enumerated surfaces. Issue REVISE when coverage stops at the single reported case instead of spanning the \`## Surface Enumeration\` checklist (FN-5893; see FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751).]
|
||||
|
||||
### Suggestions
|
||||
- [Optional improvements, not blocking]
|
||||
@@ -144,6 +145,7 @@ Concrete examples:
|
||||
- **File scope accuracy:** [All affected files listed? No extras?]
|
||||
- **Dependency correctness:** [Dependencies exist and are appropriate?]
|
||||
- **Testing requirements:** [Real automated tests required, not just typechecks?]
|
||||
- **Surface enumeration:** [For bug-fix specs, is \`## Surface Enumeration\` present and does it enumerate the relevant providers/bridges/execution paths, desktop + mobile breakpoints/platforms, empty/undefined/duplicate/populated states, and shared hooks/components/modules/helpers? Missing or incomplete coverage is a blocking REVISE.]
|
||||
- **Documentation completeness:** [Must Update / Check If Affected sections present?]
|
||||
- **Dangling task-document references:** [No \`.fusion/tasks/<id>/<file>\` path is cited in Context, Steps, or File Scope unless the file exists or is explicitly created as a \`(new)\` artifact in this spec. References to nonexistent task-local artifacts are a blocking REVISE.]
|
||||
- **Sizing & review level:** [Size and review level appropriate for the work?]
|
||||
@@ -198,6 +200,7 @@ Do NOT demand function-level implementation checklists.
|
||||
|
||||
When reviewing tests, check that they verify observable behavior and regression risk (not only implementation trivia).
|
||||
Flag REVISE when key edge cases or failure modes for changed behavior are untested.
|
||||
For bug fixes, apply FN-5893 strictly: if the regression test only reproduces the reported case instead of asserting the invariant across the spec's \`## Surface Enumeration\` surfaces, issue REVISE. Use the motivating recurrences (FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751) as concrete examples of why repro-only coverage is insufficient.
|
||||
|
||||
## Worktree Boundary Review
|
||||
|
||||
|
||||
@@ -116,6 +116,10 @@ Follow this structure exactly:
|
||||
|
||||
{One paragraph: what you're building and why it matters}
|
||||
|
||||
## Surface Enumeration
|
||||
|
||||
{Required for bug-fix tasks: a checklist enumerating every surface the fixed invariant must hold across. Include every provider/bridge for streaming and agent paths; desktop AND mobile breakpoints; empty/undefined/duplicate/populated data states; and every hook/component/module that shares the affected logic. Use the canonical checklist in docs/testing.md as the starting point.}
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **None**
|
||||
@@ -146,6 +150,12 @@ Follow this structure exactly:
|
||||
- [ ] {Specific, verifiable outcome}
|
||||
- [ ] Run targeted tests for changed files, asserting the invariant across all known surfaces (enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states)
|
||||
|
||||
For bug-fix tasks, paste and fill in this checklist in the \`## Surface Enumeration\` section:
|
||||
- [ ] Providers / bridges / execution paths touched by the invariant
|
||||
- [ ] Desktop + mobile breakpoints / platforms that exercise the behavior
|
||||
- [ ] Empty / undefined / duplicate / populated data states
|
||||
- [ ] Shared hooks / components / modules / helpers reusing the logic
|
||||
|
||||
**Artifacts:**
|
||||
- \`path/to/file\` (new | modified)
|
||||
|
||||
@@ -220,6 +230,8 @@ files with assertions that run via a test runner. Typechecks and builds are NOT
|
||||
tests. Manual verification is NOT a test.
|
||||
|
||||
- Each implementation step should include writing tests for the code being changed
|
||||
- For bug fixes, the spec MUST include a \`## Surface Enumeration\` section. During self-review via \`fn_review_spec()\`, treat a missing section on a bug-fix spec as a blocking REVISE.
|
||||
- For bug fixes, populate \`## Surface Enumeration\` with this checklist from \`docs/testing.md\`: providers/bridges/execution paths; desktop + mobile breakpoints/platforms; empty/undefined/duplicate/populated data states; shared hooks/components/modules/helpers.
|
||||
- For bug fixes, regression tests must assert the invariant across all known surfaces — enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states — not just the reported repro (see FN-5787/FN-5789/FN-5803 and FN-5751)
|
||||
- The final Testing step runs lint, the FULL test suite, and project typecheck when the repo exposes one
|
||||
- Specs must instruct executors to fix lint failures and quality-gate failures directly, even when the required edits extend beyond the original File Scope
|
||||
@@ -402,6 +414,10 @@ Follow this structure exactly:
|
||||
|
||||
{One paragraph: what to build and why it matters}
|
||||
|
||||
## Surface Enumeration
|
||||
|
||||
{Required for bug-fix tasks: a checklist enumerating every surface the fixed invariant must hold across. Include every provider/bridge for streaming and agent paths; desktop AND mobile breakpoints; empty/undefined/duplicate/populated data states; and every hook/component/module that shares the affected logic. Use the canonical checklist in docs/testing.md as the starting point.}
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **None**
|
||||
@@ -432,6 +448,12 @@ Follow this structure exactly:
|
||||
- [ ] {Specific, verifiable outcome}
|
||||
- [ ] Run targeted tests for changed files, asserting the invariant across all known surfaces (enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states)
|
||||
|
||||
For bug-fix tasks, paste and fill in this checklist in the \`## Surface Enumeration\` section:
|
||||
- [ ] Providers / bridges / execution paths touched by the invariant
|
||||
- [ ] Desktop + mobile breakpoints / platforms that exercise the behavior
|
||||
- [ ] Empty / undefined / duplicate / populated data states
|
||||
- [ ] Shared hooks / components / modules / helpers reusing the logic
|
||||
|
||||
**Artifacts:**
|
||||
- \`path/to/file\` (new | modified)
|
||||
|
||||
@@ -501,6 +523,8 @@ If this task REMOVES existing functionality (deleting modules, settings, API end
|
||||
## Testing requirements
|
||||
- Require real automated tests with assertions that run in the project's test runner
|
||||
- Typecheck/build/manual checks are not tests and cannot replace tests
|
||||
- For bug fixes, the spec MUST include a \`## Surface Enumeration\` section. During self-review via \`fn_review_spec()\`, treat a missing section on a bug-fix spec as a blocking REVISE.
|
||||
- For bug fixes, populate \`## Surface Enumeration\` with this checklist from \`docs/testing.md\`: providers/bridges/execution paths; desktop + mobile breakpoints/platforms; empty/undefined/duplicate/populated data states; shared hooks/components/modules/helpers.
|
||||
- For bug fixes, regression tests must assert the invariant across all known surfaces — enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states — not just the reported repro (see FN-5787/FN-5789/FN-5803 and FN-5751)
|
||||
- Include targeted tests in implementation steps and full quality-gate runs in final verification
|
||||
|
||||
|
||||
Reference in New Issue
Block a user