From d073bf1bdf36933b9b94df6f59364a10a33f42a6 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 24 May 2026 15:48:59 +0900 Subject: [PATCH] feat(ultrawork): enforce TDD, scenario contract, durable notepad, reviewer gate Closes five gaps in the ultrawork prompt versus codex-plugins' parallel directive, applied to all three model variants (default/Claude, GPT, Gemini) with prompt-engineering entropy gate (each addition encodes a distinct binding boolean, not narrative reinforcement): 1. TDD-MANDATORY (was conditional "when test infrastructure exists"): every production change follows RED -> GREEN -> SURFACE. Failing test first, capture assertion msg, smallest change to flip green, exercise real surface, capture artifact. Exemption whitelist: formatting / comment-only / version bump / rename-only, each must be justified in writing; unjustified exemption = rejection. 2. Scenario contract (was free-form Test Plan Template): require 3+ scenarios upfront covering happy path, edge (boundary / empty / malformed / concurrent), adjacent-surface regression. Each scenario binds a binary pass condition, a real-surface artifact source, and a test file + test id written test-first. 3. RED->GREEN evidence capture (was "all tests pass"): every scenario requires TWO captured artifacts -- RED assertion msg before the change AND GREEN assertion msg after -- alongside the real-surface artifact (tmux / curl / browser / Playwright / computer-use / CLI stdout / parsed config / DB diff). Tests are the floor (always required); surface artifact is the ceiling (also required). 4. Durable notepad: mktemp -t ulw-*.md with append-only sections (Plan, Scenarios, Now, Todo, Findings, Learnings). Survives context loss; resume by re-reading. 5. Reviewer gate: trigger when user said strictly / rigorously / "deeply", or task touches 3+ files / 20+ turns / 30+ min, or it is refactor / migration / perf / security work. Reviewer verdict is binding ("looks good but..." = rejection). Loop until unconditional approval. Plus: TODO format upgraded from vague "track every step" to atomic `path: for -- verify by ` with a GOOD test-first / impl pair example and a BAD list including "production code before its failing test". Per-variant adaptation: - default.ts (Claude): full structured sections. - gpt.ts (GPT-5.x): outcome-first prose, shorter prose per gpt-5.5 guide. - gemini.ts: explicit enforcement framing + anti-optimism checkpoint upgraded with a TDD-violation question (#7). Verified by: - bun test src/hooks/keyword-detector/ (119 pass / 0 fail). - lsp_diagnostics clean on all three files. - Module-load smoke test confirms each exported message string parses and contains the new section anchors (TDD MANDATORY, SCENARIO CONTRACT, DURABLE NOTEPAD, REVIEWER GATE). Char deltas (directive body only): - default 13646 -> 17144 (+26%) - gpt 6740 -> 9215 (+37%, was the leanest start) - gemini 14196 -> 16136 (+14%) Existing tests only assert presence of "ULTRAWORK MODE ENABLED!" which is preserved verbatim in every variant. --- .../keyword-detector/ultrawork/default.ts | 109 ++++++++++++------ .../keyword-detector/ultrawork/gemini.ts | 63 +++++++--- src/hooks/keyword-detector/ultrawork/gpt.ts | 70 ++++++----- 3 files changed, 165 insertions(+), 77 deletions(-) diff --git a/src/hooks/keyword-detector/ultrawork/default.ts b/src/hooks/keyword-detector/ultrawork/default.ts index c083b1946..394b0aaca 100644 --- a/src/hooks/keyword-detector/ultrawork/default.ts +++ b/src/hooks/keyword-detector/ultrawork/default.ts @@ -176,11 +176,13 @@ task(category="quick", load_skills=["git-master"], run_in_background=true) --- ## EXECUTION RULES -- **TODO**: Track EVERY step. Mark complete IMMEDIATELY after each. -- **PARALLEL**: Fire independent agent calls simultaneously via task(run_in_background=true) - NEVER wait sequentially. +- **TODO format**: \`path: for — verify by \` encoding WHERE / WHY (which scenario it advances) / HOW / VERIFY. Exactly ONE in_progress at a time. Mark completed IMMEDIATELY — never batch. + - GOOD pair (test-first, ordered): \`foo.test.ts: Write FAILING case invalid-email→ValidationError for S2 — verify by RED with assertion msg\` → \`src/foo/bar.ts: Implement validateEmail() for S2 — verify by foo.test.ts GREEN + curl 400 body\` + - BAD: "Implement feature" / "Fix bug" / "Add tests later" / production code before its failing test → rewrite. +- **PARALLEL**: Fire independent agent calls simultaneously via task(run_in_background=true) — NEVER wait sequentially. But NEVER parallelise RED and GREEN of the same scenario. - **BACKGROUND FIRST**: Use task for exploration/research agents (10+ concurrent if needed). -- **VERIFY**: Re-read request after completion. Check ALL requirements met before reporting done. -- **DELEGATE**: Don't do everything yourself - orchestrate specialized agents for their strengths. +- **VERIFY**: Re-read request after completion. Check every scenario PASS with both artifacts captured. +- **DELEGATE**: Don't do everything yourself — orchestrate specialized agents for their strengths. ## WORKFLOW 1. Analyze the request and identify required capabilities @@ -192,41 +194,53 @@ task(category="quick", load_skills=["git-master"], run_in_background=true) **NOTHING is "done" without PROOF it works.** -### Pre-Implementation: Define Success Criteria +### Pre-Implementation: Scenario Contract (BINDING) -BEFORE writing ANY code, you MUST define: +BEFORE writing ANY code, define **3+ realistic scenarios** covering: -| Criteria Type | Description | Example | -|---------------|-------------|---------| -| **Functional** | What specific behavior must work | "Button click triggers API call" | -| **Observable** | What can be measured/seen | "Console shows 'success', no errors" | -| **Pass/Fail** | Binary, no ambiguity | "Returns 200 OK" not "should work" | +| Class | Required | Example | +|-------|----------|---------| +| **Happy path** | yes | Valid input → 200 OK with expected body | +| **Edge** (boundary / empty / malformed / concurrent) | yes | Empty list, max-length input, two writers race | +| **Adjacent-surface regression** | yes | Caller X still works, sibling endpoint Y unchanged | -Write these criteria explicitly. **Record them in your TODO/Task items.** Each task MUST include a "QA: [how to verify]" field. These criteria are your CONTRACT - work toward them, verify against them. +Each scenario MUST specify, upfront: +- Pass condition as a binary observable ("returns 200 + body matches schema"), not "should work". +- The REAL surface that proves it: tmux transcript, curl status+body, browser/Playwright assertion, computer-use action log, CLI stdout, parsed config dump, DB state diff. Asserting "tests pass" alone is NOT evidence. +- The automated test file + test id that exercises this scenario (written test-first — see TDD below). -### Test Plan Template (MANDATORY for non-trivial tasks) +**These scenarios are the CONTRACT.** Record them in your TODO/notepad. You are not done until every one PASSES with both pieces of evidence captured (RED→GREEN proof + real-surface artifact). + +### Durable Notepad (survives context loss) + +Run once at start: \`NOTE=$(mktemp -t ulw-$(date +%Y%m%d-%H%M%S).XXXXXX.md)\`. Echo the path. Initialise with these sections and APPEND (never rewrite) as you work: \`\`\` -## Test Plan -### Objective: [What we're verifying] -### Prerequisites: [Setup needed] -### Test Cases: -1. [Test Name]: [Input] → [Expected Output] → [How to verify] -2. ... -### Success Criteria: ALL test cases pass -### How to Execute: [Exact commands/steps] +# Ultrawork Notepad — +Started: + +## Plan (exhaustive, atomic) +## Scenarios (the contract) +## Now (single step in progress) +## Todo (remaining, ordered) +## Findings (non-obvious facts with file:line refs) +## Learnings (patterns / pitfalls for next turn) \`\`\` +If context is lost, you re-read the notepad and resume. Do not skip this — it is the only durable memory across turns. + ### Execution & Evidence Requirements -| Phase | Action | Required Evidence | -|-------|--------|-------------------| -| **Build** | Run build command | Exit code 0, no errors | -| **Test** | Execute test suite | All tests pass (screenshot/output) | -| **Manual Verify** | Test the actual feature | Demonstrate it works (describe what you observed) | -| **Regression** | Ensure nothing broke | Existing tests still pass | +Every scenario requires TWO captured artifacts — both mandatory: -**WITHOUT evidence = NOT verified = NOT done.** +| Artifact | Source | Captures | +|----------|--------|----------| +| **RED→GREEN proof** | Test runner output before AND after the change | Test id + assertion message in both states | +| **Real-surface artifact** | tmux / curl / browser / Playwright / computer-use / CLI / DB | What the user actually sees | + +Supporting (necessary, not sufficient): build exit 0, full suite green, lsp_diagnostics clean on changed files, regression scenarios still PASS. + +Tests are the FLOOR (always required). Surface artifact is the CEILING (also required). "tests pass" alone is NOT done. ### YOU MUST EXECUTE MANUAL QA YOURSELF. THIS IS NOT OPTIONAL. @@ -254,27 +268,46 @@ Write these criteria explicitly. **Record them in your TODO/Task items.** Each t **Manual QA is the FINAL gate before reporting completion. Skip it and your work is INCOMPLETE.** -### TDD Workflow (when test infrastructure exists) +### TDD Workflow (MANDATORY on every production change) -1. **SPEC**: Define what "working" means (success criteria above) -2. **RED**: Write failing test → Run it → Confirm it FAILS -3. **GREEN**: Write minimal code → Run test → Confirm it PASSES -4. **REFACTOR**: Clean up → Tests MUST stay green -5. **VERIFY**: Run full test suite, confirm no regressions -6. **EVIDENCE**: Report what you ran and what output you saw +Test-first is not optional. Every behavior change — features, fixes, refactors, perf, glue, config-with-logic — follows RED → GREEN → SURFACE. + +1. **RED**: Write the failing test FIRST. Run it. Capture the assertion message proving it fails for the RIGHT reason (not syntax, not import). Paste RED output into the notepad. No production code yet. +2. **GREEN**: Write the SMALLEST change that flips RED→GREEN. Re-run. Capture GREEN output. If GREEN required ~20+ lines, your test was too coarse — split it. +3. **SURFACE**: Exercise the real user-facing surface named by the scenario. Capture artifact path into the notepad. +4. **REFACTOR**: Optional, only if needed. Tests MUST stay green throughout. +5. **REGRESSION**: Re-run the FULL scenario list. Record PASS/FAIL inline with both evidence paths. + +**Refactor exception**: Write characterization tests pinning current observable behavior FIRST, watch them go GREEN against old code, THEN refactor. They remain green throughout. + +**Exemption whitelist** (no new test required): pure formatting, comment-only edits, dependency version bumps with no behavior delta, rename-only moves. Each exemption MUST be justified in \`## Findings\` with the exact reason. Unjustified exemption is rejection. + +**If you typed production code without a failing test preceding it in the notepad: STOP, revert, write the test, watch it fail, then redo.** ### Verification Anti-Patterns (BLOCKING) | Violation | Why It Fails | |-----------|--------------| | "It should work now" | No evidence. Run it. | -| "I added the tests" | Did they pass? Show output. | -| "Fixed the bug" | How do you know? What did you test? | -| "Implementation complete" | Did you verify against success criteria? | +| "I added the tests" | Did they go RED first, then GREEN? Show both. | +| "Fixed the bug" | What scenario proves it? Where's the artifact? | +| "Implementation complete" | Every scenario PASS with both artifacts captured? | | Skipping test execution | Tests exist to be RUN, not just written | +| Writing code before its failing test | TDD floor violated — revert, write test, redo | **CLAIM NOTHING WITHOUT PROOF. EXECUTE. VERIFY. SHOW EVIDENCE.** +### Reviewer Gate (triggered, not optional) + +Trigger when ANY apply: user said "엄밀" / "strictly" / "rigorously" / "properly review"; task touches 3+ files OR ran 20+ turns OR 30+ minutes; refactor / migration / perf / security work; user called it "깊게" / "deeply". + +Procedure (non-negotiable): +1. Spawn a reviewer via \`task(category="ultrabrain", subagent_type="plan", load_skills=[...], run_in_background=false, prompt="")\` — or any high-rigor reviewer agent available. +2. Reviewer verdict is BINDING. There is no "false positive". Do not argue, minimise, or explain away. +3. Fix every concern. Re-run the FULL scenario QA. Capture fresh evidence. Update notepad. +4. Re-submit to the SAME reviewer. Loop until UNCONDITIONAL approval. "looks good but..." = REJECTION. +5. Only on unconditional approval may you declare done. + ## ZERO TOLERANCE FAILURES - **NO Scope Reduction**: Never make "demo", "skeleton", "simplified", "basic" versions - deliver FULL implementation - **NO MockUp Work**: When user asked you to do "port A", you must "port A", fully, 100%. No Extra feature, No reduced feature, no mock data, fully working 100% port. diff --git a/src/hooks/keyword-detector/ultrawork/gemini.ts b/src/hooks/keyword-detector/ultrawork/gemini.ts index 522e2df3d..a03ad6666 100644 --- a/src/hooks/keyword-detector/ultrawork/gemini.ts +++ b/src/hooks/keyword-detector/ultrawork/gemini.ts @@ -214,28 +214,65 @@ task(subagent_type="plan", load_skills=[], run_in_background=false, prompt=" ## BEFORE YOU CLAIM DONE, ANSWER HONESTLY: -1. Did I run \`lsp_diagnostics\` and see ZERO errors? (not "I'm sure there are none") -2. Did I run the tests and see them PASS? (not "they should pass") -3. Did I read the actual output of every command? (not skim) -4. Is EVERY requirement from the request actually implemented? (re-read the request NOW) -5. Did I classify intent at the start? (if not, my entire approach may be wrong) +1. Did EVERY scenario reach RED captured → GREEN captured → surface artifact captured? (paths in notepad) +2. Did I run \`lsp_diagnostics\` and see ZERO errors on changed files? (not "I'm sure") +3. Did I run the FULL suite and see it PASS? (not "they should pass") +4. Did I read the actual output of every command? (not skim) +5. Is EVERY requirement from the request actually implemented? (re-read the request NOW) +6. Did I classify intent at the start? (if not, my entire approach may be wrong) +7. Did I write code BEFORE its failing test, anywhere? (if yes, REVERT and redo via TDD) If ANY answer is no → GO BACK AND DO IT. Do not claim completion. +### REVIEWER GATE (triggered, not optional) + +Trigger if user said "엄밀"/"strictly"/"rigorously"/"properly review", or task touches 3+ files OR ran 20+ turns OR 30+ min, or refactor/migration/perf/security. Spawn a high-rigor reviewer via \`task\` with: goal, scenarios, evidence paths, full diff, notepad path. Verdict is BINDING. "looks good but..." = REJECTION. Fix every concern, re-run full scenario QA, capture fresh evidence, resubmit. Loop until UNCONDITIONAL approval. + ### YOU MUST EXECUTE MANUAL QA. THIS IS NOT OPTIONAL. DO NOT SKIP THIS. diff --git a/src/hooks/keyword-detector/ultrawork/gpt.ts b/src/hooks/keyword-detector/ultrawork/gpt.ts index 7a4b4a0b9..6495598ac 100644 --- a/src/hooks/keyword-detector/ultrawork/gpt.ts +++ b/src/hooks/keyword-detector/ultrawork/gpt.ts @@ -114,53 +114,71 @@ deep_context = background_output(task_id=...) - Surgical, minimal changes matching existing patterns - If delegating: provide exhaustive context and success criteria -**Verify:** -- \`lsp_diagnostics\` on modified files -- Run tests if available +**Verify (per-scenario, not just "at the end"):** +- RED→GREEN proof captured (test id + assertion msg in both states) +- Real-surface artifact (tmux / curl / browser / Playwright / computer-use / CLI / DB diff) +- \`lsp_diagnostics\` clean on modified files +- Full suite green, regression scenarios still PASS -## ACCEPTANCE CRITERIA WORKFLOW +## DURABLE NOTEPAD -**BEFORE implementation**, define what "done" means in concrete, binary terms: +At start, run \`NOTE=$(mktemp -t ulw-$(date +%Y%m%d-%H%M%S).XXXXXX.md)\` and echo the path. APPEND (never rewrite) to sections: Plan, Scenarios, Now, Todo, Findings (file:line refs), Learnings. If context is lost, re-read and resume. -1. Write acceptance criteria as pass/fail conditions (not "should work" - specific observable outcomes) -2. Record them in your TODO/Task items with a "QA: [how to verify]" field -3. Work toward those criteria, not just "finishing code" +## SCENARIO CONTRACT (binding, defined BEFORE coding) + +Define 3+ scenarios covering: **happy path**, **edge** (boundary / empty / malformed / concurrent), **adjacent-surface regression**. For each, write: +- Binary pass condition ("returns 200 with schema-matching body"), not "should work". +- The real surface that proves it. +- The test file + test id (written test-first; see TDD). + +Scenarios are the contract. Done = every scenario PASSES with RED→GREEN proof AND real-surface artifact captured. + +## TDD (MANDATORY on every production change) + +Features, fixes, refactors, perf, glue, config-with-logic — all follow RED→GREEN→SURFACE. Write the failing test FIRST; capture the assertion proving it fails for the right reason; write the SMALLEST change to flip it green; exercise the real surface; capture both artifacts. **If you wrote production code without a failing test preceding it: STOP, revert, write the test, redo.** + +Refactors: write characterization tests pinning current behavior FIRST, watch them GREEN against old code, THEN refactor. They stay green throughout. + +Exemption whitelist (no new test required): formatting, comment-only, version bumps with no behavior delta, rename-only. Each must be justified in writing. Unjustified exemption is rejection. ## QUALITY STANDARDS | Phase | Action | Required Evidence | |-------|--------|-------------------| +| RED | Run new test before impl | Failing assertion with msg | +| GREEN | Re-run after smallest change | Passing assertion | +| Surface | Exercise real user path | Artifact path (tmux/curl/browser/...) | | Build | Run build command | Exit code 0 | -| Test | Execute test suite | All tests pass | -| Lint | Run lsp_diagnostics | Zero new errors | -| **Manual QA** | **Execute the feature yourself** | **Actual output shown** | +| Suite | Full test run | All green; no skip/.only/xfail added | +| Lint | lsp_diagnostics on changed files | Zero new errors | ### MANUAL QA IS MANDATORY. lsp_diagnostics IS NOT ENOUGH. -lsp_diagnostics catches type errors. It does NOT catch logic bugs, missing behavior, or broken features. After EVERY implementation, you MUST manually test the actual feature. - -**Execute ALL that apply:** +lsp_diagnostics catches type errors only. Logic bugs, missing behavior, broken features survive a clean LSP. After every change, exercise the real surface: | If your change... | YOU MUST... | |---|---| -| Adds/modifies a CLI command | Run the command with Bash. Show the output. | -| Changes build output | Run the build. Verify output files. | -| Modifies API behavior | Call the endpoint. Show the response. | -| Adds a new tool/hook/feature | Test it end-to-end in a real scenario. | -| Modifies config handling | Load the config. Verify it parses correctly. | +| Adds/modifies a CLI command | Run it with Bash. Show output. | +| Changes build output | Run build. Verify output files. | +| Modifies API behavior | Call the endpoint. Show response. | +| Adds tool/hook/feature | Test end-to-end in a real scenario. | +| Modifies config handling | Load config. Verify parsed shape. | -**"This should work" is NOT evidence. RUN IT. Show what happened. That is evidence.** +"This should work" / "tests pass" / "lsp clean" are NOT evidence on their own — the surface artifact is. +## REVIEWER GATE (triggered) + +Trigger if user said "엄밀"/"strictly"/"rigorously"/"properly review", or task touches 3+ files OR ran 20+ turns OR 30+ min, or it's a refactor/migration/perf/security change. Spawn a high-rigor reviewer via \`task\` with goal + scenarios + evidence + diff. Reviewer verdict is BINDING; "looks good but..." = rejection. Re-submit until UNCONDITIONAL approval before declaring done. + ## COMPLETION CRITERIA -A task is complete when: -1. Requested functionality is fully implemented (not partial, not simplified) -2. lsp_diagnostics shows zero errors on modified files -3. Tests pass (or pre-existing failures documented) -4. Code matches existing codebase patterns -5. **Manual QA executed - actual feature tested, output observed and reported** +Done when ALL of: +1. Every scenario PASSES with RED→GREEN proof AND real-surface artifact captured. +2. Full test suite green; lsp_diagnostics clean on changed files. +3. Code matches existing patterns; no scope creep. +4. Reviewer gate (if triggered) returned unconditional approval. **Deliver exactly what was asked. No more, no less.**