diff --git a/.gitignore b/.gitignore index 2e250f2..ad3b7ed 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -node_modules/ +node_modules convoy dist/ .*.bun-build diff --git a/prompts/over-engineering-auditor.md b/prompts/over-engineering-auditor.md new file mode 100644 index 0000000..05d8686 --- /dev/null +++ b/prompts/over-engineering-auditor.md @@ -0,0 +1,60 @@ +# Over-Engineering Auditor + +You are the **over-engineering-auditor** agent of Convoy's `review` and `refine` pipelines. This is an audit-only phase: do not modify the repository. + +## Review scope + +Default scope is the attached diff: this branch or pull request against the base ref, plus any uncommitted changes. Read the rest of the repository freely as *context* — to check whether an abstraction, wrapper, flag, or helper is already used elsewhere — but every finding you report must be about changed lines. + +Do not report pre-existing over-engineering in untouched code. The one exception is code the change makes newly reachable or newly wrong; report it, say so explicitly, and tie it to the changed line. Widen scope only when `prd.md` explicitly asks for a repository-wide audit. + +## Objective + +Audit whether the scoped change is proportionate to what `prd.md` asks for. Hunt speculative generality, premature abstraction, unnecessary indirection, excessive configurability, accidental complexity, over-processed code, and out-of-scope gold-plating in the scoped change. + +## Workflow + +1. Read `prd.md`, `reports/scope.md`, the attached diff, and nearby code. +2. Identify what `prd.md` actually requires. +3. Check every abstraction, base class, helper, parameter, flag, wrapper, data structure, and extension point added in the diff against that requirement list. +4. Judge whether the change is proportionate or disproportionate. Never invent a "better design" — the finding is over-engineering and lack of proportionality, not a redesign. + +## The over-engineering taxonomy + +Levels N1–N7: + +- **N1 — YAGNI / speculative generality**: functionality, abstractions, or parameters added for hypothetical future cases with no current consumer and nothing in `prd.md` that calls for them. The over-engineering by antonomasia. +- **N2 — Premature abstraction**: an interface, helper, base class, or generic component extracted for a single real usage. +- **N3 — Excessive configurability**: flags, options, env vars, or extension points for scenarios that do not exist yet (and no PRD requirement that names them). +- **N4 — Unnecessary indirection**: layers, wrappers, or delegation that add cognitive load without reducing real complexity elsewhere. +- **N5 — Accidental complexity**: data structures, concurrency, or error-handling more elaborate than the problem requires. +- **N6 — Over-processed code**: logging, validation, telemetry, or hardening that duplicates mechanisms the repository already provides. +- **N7 — Gold-plating / scope creep**: polish, refactors, or "improvements" the PRD did not ask for. + +Variants V1–V7 (severity within a level): + +- **V1 — Marginal**: adds a little complexity, no real maintenance cost today; worth noting, not worth a change. +- **V2 — Minor**: localized over-engineering; a small, safe simplification exists. +- **V3 — Moderate**: already makes the changed lines harder to read or change than they need to be. +- **V4 — Notable**: the abstraction/flag/wrapper has no consumer in the diff or the codebase. +- **V5 — High**: structural over-engineering (a layer, pattern, or framework) the PRD did not ask for. +- **V6 — Severe**: would likely be rewritten when the real use case appears; confidently YAGNI. +- **V7 — Blocking**: blocks merge — flagrant YAGNI or large maintenance/attack surface added for no current need. + +Findings are additionally tagged (mutually exclusive, one primary tag per finding): + +- `yagni` — N1 +- `premature-abstraction` — N2 +- `configurability` — N3 +- `indirection` — N4 +- `complexity` — N5 +- `over-processing` — N6 +- `scope-creep` — N7 + +## Report + +Return Markdown with: + +- **Findings**: `OE-1`, `OE-2`, ... each with its tag (`yagni`, `premature-abstraction`, `configurability`, `indirection`, `complexity`, `over-processing`, `scope-creep`), level (`N1`–`N7`), variant (`V1`–`V7`), severity (`high|medium|low`), file reference, evidence, why it is disproportionate, and the concrete simplification. Severity maps to the merge decision (`high` = N6/V6 or N7/V7, `medium` = N4–N5 at V4+ or any level at V5+, `low` = everything else). +- **Proportionate parts**: where the change is well-sized. +- **Deferred/non-blocking**: observations that are not worth changing in this PR. diff --git a/prompts/review-adversary.md b/prompts/review-adversary.md index 868eb7a..284d117 100644 --- a/prompts/review-adversary.md +++ b/prompts/review-adversary.md @@ -14,7 +14,7 @@ Act as a skeptical second reviewer over the audit reports. Validate which findin ## Workflow -1. Read `prd.md`, `reports/scope.md`, `reports/bugs.md`, `reports/clean-code.md`, `reports/security.md`, and the attached diff. +1. Read `prd.md`, `reports/scope.md`, `reports/bugs.md`, `reports/clean-code.md`, `reports/security.md`, `reports/over-engineering.md`, and the attached diff. 2. Challenge every finding: - Is the evidence present in the diff or adjacent code? - Is the severity justified? diff --git a/prompts/review-report.md b/prompts/review-report.md index ca9f9dc..f9a3ba8 100644 --- a/prompts/review-report.md +++ b/prompts/review-report.md @@ -10,11 +10,11 @@ Widen scope only when `prd.md` explicitly asks for a repository-wide review. ## Objective -Synthesize every audit that ran before you — clean-code/pattern, security, and bug audits, each produced by two different models — into a single, concise, prioritized findings report. Decide which findings are real and worth acting on, and rank them so a maintainer can act (or defer) without re-reading the raw audits. +Synthesize every audit that ran before you — clean-code/pattern, security, bug, and over-engineering audits, each produced by two different models — into a single, concise, prioritized findings report. Decide which findings are real and worth acting on, and rank them so a maintainer can act (or defer) without re-reading the raw audits. ## Workflow -1. Read `prd.md`, `reports/scope.md`, every attached audit report (both model variants of clean-code, security, and bugs), and the attached diff. +1. Read `prd.md`, `reports/scope.md`, every attached audit report (both model variants of clean-code, security, bugs, and over-engineering; reads `reports/over-engineering.md` when present), and the attached diff. 2. Cross-check the two models behind each audit: - Where both models raise the same finding, treat it as **high-confidence**. - Where they disagree, use your own judgment against the diff to keep or drop it. diff --git a/src/built-in-prompts.ts b/src/built-in-prompts.ts index 32c04af..a4096ec 100644 --- a/src/built-in-prompts.ts +++ b/src/built-in-prompts.ts @@ -21,6 +21,7 @@ import implementationFixer from "../prompts/implementation-fixer.md" with { type import implementationTriage from "../prompts/implementation-triage.md" with { type: "text" } import implementationValidator from "../prompts/implementation-validator.md" with { type: "text" } import implementer from "../prompts/implementer.md" with { type: "text" } +import overEngineeringAuditor from "../prompts/over-engineering-auditor.md" with { type: "text" } import patternAuditor from "../prompts/pattern-auditor.md" with { type: "text" } import reviewAdversary from "../prompts/review-adversary.md" with { type: "text" } import reviewFixer from "../prompts/review-fixer.md" with { type: "text" } @@ -62,6 +63,7 @@ export const builtInPrompts: Record = { "implementation-triage": implementationTriage, "implementation-validator": implementationValidator, implementer, + "over-engineering-auditor": overEngineeringAuditor, "pattern-auditor": patternAuditor, "review-adversary": reviewAdversary, "review-fixer": reviewFixer, diff --git a/src/pipeline.ts b/src/pipeline.ts index 436c5b2..3b5e15a 100644 --- a/src/pipeline.ts +++ b/src/pipeline.ts @@ -101,6 +101,14 @@ export const builtInAgents: readonly AgentSpec[] = [ readOnly: true, builtIn: true, }, + { + name: "over-engineering-auditor", + description: "Audit-only reviewer for over-engineering, speculative generality, and scope creep risks", + defaultModel: fallbackModel, + temperature: 0.1, + readOnly: true, + builtIn: true, + }, { name: "security-reviewer", description: "Audit-only reviewer for security, privacy, and operational risks", @@ -380,12 +388,13 @@ export const builtInPipelines: Record = { }, review: { description: - "Report-only PR review: scope, then parallel bug/clean-code/security audits across two models, then one prioritized findings report. Makes no changes.", + "Report-only PR review: scope, then parallel bug/clean-code/security/over-engineering audits across two models, then one prioritized findings report. Makes no changes.", steps: [ { agent: "review-scope", name: "scope", model: defaultOpusModel, reports: "none", diff: true }, { parallel: [ { agent: "clean-code-auditor", name: "clean-code", models: [fallbackModel, defaultOpusModel], reports: ["scope"] }, + { agent: "over-engineering-auditor", name: "over-engineering", models: [fallbackModel, defaultOpusModel], reports: ["scope"] }, { agent: "security-reviewer", name: "security", models: [fallbackModel, defaultOpusModel], reports: ["scope"] }, { agent: "bug-auditor", name: "bugs", models: [fallbackModel, defaultOpusModel], reports: ["scope"] }, ], @@ -401,6 +410,7 @@ export const builtInPipelines: Record = { { parallel: [ { agent: "clean-code-auditor", name: "clean-code", models: [glmModel, kimiModel], reports: ["scope"] }, + { agent: "over-engineering-auditor", name: "over-engineering", models: [glmModel, kimiModel], reports: ["scope"] }, { agent: "security-reviewer", name: "security", models: [glmModel, kimiModel], reports: ["scope"] }, { agent: "bug-auditor", name: "bugs", models: [glmModel, kimiModel], reports: ["scope"] }, ], @@ -415,7 +425,8 @@ export const builtInPipelines: Record = { { agent: "bug-auditor", name: "bugs", model: fallbackModel, reports: ["scope"] }, { agent: "clean-code-auditor", name: "clean-code", model: fallbackModel, reports: ["scope"] }, { agent: "security-reviewer", name: "security", model: fallbackModel, reports: ["scope"] }, - { agent: "review-adversary", name: "triage", model: defaultOpusModel, reports: ["scope", "bugs", "clean-code", "security"] }, + { agent: "over-engineering-auditor", name: "over-engineering", model: fallbackModel, reports: ["scope"] }, + { agent: "review-adversary", name: "triage", model: defaultOpusModel, reports: ["scope", "bugs", "clean-code", "security", "over-engineering"] }, { agent: "review-fixer", name: "fixes", model: fallbackModel, reports: ["triage"] }, { agent: "review-validator", name: "validator", model: fallbackModel, reports: "all" }, ], @@ -429,9 +440,10 @@ export const builtInPipelines: Record = { { agent: "bug-auditor", name: "bugs", models: [sonnetModel, fallbackModel], reports: ["scope"] }, { agent: "clean-code-auditor", name: "clean-code", models: [sonnetModel, fallbackModel], reports: ["scope"] }, { agent: "security-reviewer", name: "security", models: [sonnetModel, fallbackModel], reports: ["scope"] }, + { agent: "over-engineering-auditor", name: "over-engineering", models: [sonnetModel, fallbackModel], reports: ["scope"] }, ], }, - { agent: "review-adversary", name: "triage", model: defaultOpusModel, reports: ["scope", "bugs", "clean-code", "security"] }, + { agent: "review-adversary", name: "triage", model: defaultOpusModel, reports: ["scope", "bugs", "clean-code", "security", "over-engineering"] }, { agent: "review-fixer", name: "fixes", model: sonnetModel, reports: ["triage"] }, { agent: "review-validator", name: "validator", model: defaultOpusModel, reports: "all" }, ], diff --git a/test/agents.test.ts b/test/agents.test.ts index 64b64d0..ea51281 100644 --- a/test/agents.test.ts +++ b/test/agents.test.ts @@ -34,6 +34,16 @@ describe("opencode config", () => { expect(prompt).toContain("not replaceable") }) + test("loads over-engineering-auditor prompt with taxonomy and safety guard rails", () => { + const prompt = loadAgentPrompt("over-engineering-auditor", "/tmp/non-existent-convoy-target") + + expect(prompt).toContain("# Over-Engineering Auditor") + expect(prompt).toContain("N1 — YAGNI") + expect(prompt).toContain("V1 — Marginal") + expect(prompt).toContain("`yagni` — N1") + expect(prompt).toContain("# Convoy Runtime Safety") + }) + test("project agent prompts replace built-ins but keep runtime safety", async () => { const dir = await mkdtemp(join(tmpdir(), "convoy-agents-")) try { diff --git a/test/config.test.ts b/test/config.test.ts index 4b1b8c1..8ac3324 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -365,6 +365,7 @@ describe("agent registry", () => { "review-scope", "bug-auditor", "clean-code-auditor", + "over-engineering-auditor", "security-reviewer", "review-adversary", "review-fixer", @@ -926,7 +927,7 @@ describe("materializing built-in pipelines", () => { const originalMember = originalGroup.parallel[0] if (typeof originalMember === "string") throw new Error("expected a member object") expect(original.steps).toHaveLength(3) - expect(originalGroup.parallel).toHaveLength(3) + expect(originalGroup.parallel).toHaveLength(4) expect(originalMember.models).toHaveLength(2) expect(originalMember.name).toBe("clean-code") }) diff --git a/test/pipeline.test.ts b/test/pipeline.test.ts index 714267e..3cafbb8 100644 --- a/test/pipeline.test.ts +++ b/test/pipeline.test.ts @@ -212,6 +212,8 @@ describe("built-in review pipeline", () => { "scope", "clean-code__openai-gpt-5-6-terra-xhigh", "clean-code__anthropic-claude-opus-5", + "over-engineering__openai-gpt-5-6-terra-xhigh", + "over-engineering__anthropic-claude-opus-5", "security__openai-gpt-5-6-terra-xhigh", "security__anthropic-claude-opus-5", "bugs__openai-gpt-5-6-terra-xhigh", @@ -225,6 +227,8 @@ describe("built-in review pipeline", () => { "reports/scope.md", "reports/clean-code__openai-gpt-5-6-terra-xhigh.md", "reports/clean-code__anthropic-claude-opus-5.md", + "reports/over-engineering__openai-gpt-5-6-terra-xhigh.md", + "reports/over-engineering__anthropic-claude-opus-5.md", "reports/security__openai-gpt-5-6-terra-xhigh.md", "reports/security__anthropic-claude-opus-5.md", "reports/bugs__openai-gpt-5-6-terra-xhigh.md", @@ -250,6 +254,8 @@ describe("built-in review-lite pipeline", () => { "scope", "clean-code__openrouter-z-ai-glm-5-2", "clean-code__openrouter-moonshotai-kimi-k3", + "over-engineering__openrouter-z-ai-glm-5-2", + "over-engineering__openrouter-moonshotai-kimi-k3", "security__openrouter-z-ai-glm-5-2", "security__openrouter-moonshotai-kimi-k3", "bugs__openrouter-z-ai-glm-5-2", @@ -267,6 +273,8 @@ describe("built-in review-lite pipeline", () => { "reports/scope.md", "reports/clean-code__openrouter-z-ai-glm-5-2.md", "reports/clean-code__openrouter-moonshotai-kimi-k3.md", + "reports/over-engineering__openrouter-z-ai-glm-5-2.md", + "reports/over-engineering__openrouter-moonshotai-kimi-k3.md", "reports/security__openrouter-z-ai-glm-5-2.md", "reports/security__openrouter-moonshotai-kimi-k3.md", "reports/bugs__openrouter-z-ai-glm-5-2.md", @@ -291,12 +299,43 @@ describe("built-in refine pipeline", () => { expect(byName.bugs).toMatchObject({ model: "openai/gpt-5.6-terra", variant: "xhigh" }) expect(byName["clean-code"]).toMatchObject({ model: "openai/gpt-5.6-terra", variant: "xhigh" }) expect(byName.security).toMatchObject({ model: "openai/gpt-5.6-terra", variant: "xhigh" }) + expect(byName["over-engineering"]).toMatchObject({ model: "openai/gpt-5.6-terra", variant: "xhigh" }) expect(byName.triage).toMatchObject({ model: "anthropic/claude-opus-5" }) + expect(byName.triage.inputFiles).toEqual([ + "prd.md", + "reports/scope.md", + "reports/bugs.md", + "reports/clean-code.md", + "reports/security.md", + "reports/over-engineering.md", + ]) expect(byName.fixes).toMatchObject({ model: "openai/gpt-5.6-terra", variant: "xhigh" }) expect(byName.validator).toMatchObject({ model: "openai/gpt-5.6-terra", variant: "xhigh" }) }) }) +describe("built-in ultra-refine pipeline", () => { + test("fans out audits across models and feeds all report files including over-engineering to triage", () => { + const pipeline = resolvePipeline({ name: "ultra-refine", spec: builtInPipelines["ultra-refine"]!, agents: builtInAgents }) + const agents = pipeline.steps.filter((step): step is AgentStep => step.type === "agent") + + const triage = agents.find((step) => step.name === "triage") + expect(triage?.inputFiles).toEqual([ + "prd.md", + "reports/scope__openrouter-anthropic-claude-sonnet-5.md", + "reports/scope__openai-gpt-5-6-terra-xhigh.md", + "reports/bugs__openrouter-anthropic-claude-sonnet-5.md", + "reports/bugs__openai-gpt-5-6-terra-xhigh.md", + "reports/clean-code__openrouter-anthropic-claude-sonnet-5.md", + "reports/clean-code__openai-gpt-5-6-terra-xhigh.md", + "reports/security__openrouter-anthropic-claude-sonnet-5.md", + "reports/security__openai-gpt-5-6-terra-xhigh.md", + "reports/over-engineering__openrouter-anthropic-claude-sonnet-5.md", + "reports/over-engineering__openai-gpt-5-6-terra-xhigh.md", + ]) + }) +}) + describe("built-in fixer pipeline", () => { const fixer = () => resolvePipeline({ name: "fixer", spec: builtInPipelines.fixer!, agents: builtInAgents }).steps.filter(