feat(dynamic)!: dynamic-build streamlining — delta reviews, honest findings/coverage, wave hardening, skill re-entrancy fixes

.devflow/features/index.md

@@ -1,3 +1,4 @@
 - **feature-knowledge-system** — commands/_partials, src/cli/commands/knowledge, shared/skills/feature-knowledge, shared/skills/apply-feature-knowledge, shared/agents/knowledge.md, scripts/build-mds.ts — Use when adding a new knowledge base entry, modifying how knowledge is loaded into agents, changing the write-through save model, extending the CLI knowledge commands, or understanding the MDS knowledge module.
 - **dream-capture-system** — scripts/hooks, shared/agents/dream.md, src/cli/commands/capture.ts, src/cli/commands/dream.ts, src/cli/commands/memory.ts, src/cli/commands/decisions.ts, src/cli/utils/decisions-config.ts, src/cli/utils/dream-cleanup.ts, src/cli/utils/project-paths.ts, src/cli/hud/components/decisions-counts.ts — Use when modifying capture hooks (capture-prompt/capture-turn/capture-question), the memory or dream pending-turns queues, the background-memory-update detached worker, the Dream agent (shared/agents/dream.md), the session-start-context dream directive, or the dream/decisions config toggles. Keywords: capture-prompt, capture-turn, capture-question, queue-append, pending-turns, memory-worker, background-memory-update, Dream agent, dream directive, DREAM MAINTENANCE, DEVFLOW_BG_UPDATER, dream config, dream-lock, json_extract_cwd_field, dream-cleanup, DREAM_MODEL allowlist.
 - **ambient-orchestrator** — scripts/hooks, src/cli/commands/ambient.ts, plugins/devflow-ambient — Use when modifying the ambient mode hooks (preamble, session-start-orchestrator), the orchestrator charter file, the git-marker helper, the ambient CLI toggle, or the plan-handoff fast-path. Keywords: ambient, preamble, orchestrator, charter, plan-handoff, session-start-orchestrator, git-marker, DEVFLOW_BG_UPDATER, devflow ambient, UserPromptSubmit, SessionStart.
+- **dynamic-workflow-engine** — commands/dynamic-build.mds, commands/dynamic-plan.mds, commands/dynamic-tickets.mds, commands/dynamic-wave.mds, commands/dynamic-profile.mds, commands/_partials/_engine.mds, commands/_partials/_wave.mds, plugins/devflow-dynamic/commands, tests/build-mds.test.ts — Use when authoring or modifying the dynamic-* commands (dynamic-build, dynamic-plan, dynamic-tickets, dynamic-wave, dynamic-profile), the shared engine/wave/preamble/factory MDS partials, or the build-mds test suite that pins doctrine literals. Keywords: dynamic-build, dynamic-plan, dynamic-tickets, dynamic-wave, dynamic-profile, Workflow tool, agentType, Gate 1, Gate 2, review loop, wave, tickets→plan→build, MDS, _engine.mds, _wave.mds.

commands/_partials/_engine.mds

@@ -31,11 +31,11 @@ Gate 2 inputs are produced by `/devflow:dynamic-plan`'s plan-challenge step —
 
 **Evaluator panel** (only if a plan exists):
 - Run `evaluator_panel()` — see that block for the panel composition
-- If any critical lens returns MISALIGNED: Coder fix (max 2 retries) → Gate 1 → re-evaluate (Gate 2 loops within itself on its own demanded change)
+- If any critical lens returns MISALIGNED: fix-and-continue — the demanded fixes are applied by a Coder that self-verifies its own build (batched per the review-loop batching doctrine if numerous). The recorded verdict becomes `FAIL-FIXED` (issues found, fixes applied, not re-evaluated by design); Gate 2 then proceeds.
 
 **Tester** (only if acceptance criteria exist):
 - Scenario-based acceptance tests covering functionality, API contracts, performance
-- FAIL → Coder fix (max 2 retries) → Gate 1 → re-test
+- FAIL → fix-and-continue — a Coder applies the demanded fixes and self-verifies its own build. The recorded verdict becomes `FAIL-FIXED`; Gate 2 then proceeds.
 
 **When Gate 2 inputs are absent:**
 - No plan → skip Evaluator panel silently (note in output: "Gate 2 Evaluator skipped — no plan available")
@@ -82,6 +82,8 @@ For each cycle:
 
 **Step 1 — Spawn reviewers in parallel (one agent() per focus)**
 
+**Review scope:** Cycle 1 reviews the entire branch diff (base SHA → HEAD). Cycles 2+ perform a DELTA REVIEW — review only the fix commits from `reviewBaseSha..HEAD` and verify those fixes are sound; do not re-review unchanged code. `reviewBaseSha` is updated after each fix phase (see Step 3). When the pre-fix SHA from the prior cycle is missing or was not recorded, the review scope falls back to the wider prior base (the un-advanced scope from the previous cycle) rather than a narrower delta — scope never regresses to a narrower window.
+
 Core reviewers (always, 8 total): security, architecture, performance, complexity, consistency, regression, testing, reliability
 
 Conditional by file type detected in the diff (add these when relevant):
@@ -96,6 +98,10 @@ Conditional by file type detected in the diff (add these when relevant):
 - `package.json` / `go.mod` / `Cargo.toml` / etc. → dependencies
 - Markdown / doc files → documentation
 
+**Spawn pacing:** launch reviewers in staggered chunks of 4–6 (sequential groups of parallel spawns) — the full roster is preserved, paced to avoid 429 batch death.
+
+**Reviewer result contract:** every reviewer MUST return a structured result with fields `focus`, `reviewed: true`, `filesExamined`, and `findings`. Each item in `findings` must carry: `file` (path of the primary file the finding relates to, or `null` if scope is unknown), `description` (one-sentence issue statement), and `severity` (`critical | high | medium | low`). A result is DEAD if: the agent returned null, threw, returned a guard string, or `reviewed !== true`. A DEAD reviewer is NEVER treated as a clean reviewer. Retry once sequentially. If still dead after retry: record the focus in `coverageGaps`. A cycle with any coverage gaps can never early-exit as clean, and the run verdict can NEVER be PASS; escalate with message "review coverage incomplete: <focus>".
+
 Each Reviewer gets `agentType: "Reviewer"` with its focus baked into the prompt. Total: 8–19 reviewers per cycle.
 
 **Step 2 — Adversarial finding verification**
@@ -109,9 +115,9 @@ Majority-survives: a finding needs >50% of verification lenses to confirm it. St
 
 **Step 3 — Early exit or fix**
 
-If no surviving findings: break (early exit — do not run unnecessary cycles).
+If no surviving findings and `coverageGaps` is empty: break (early exit — do not run unnecessary cycles).
 
-If survivors remain: Coder fixes them (batched per concurrency doctrine — see `concurrency_doctrine()`). The fixing Coder **self-verifies its own fix builds** (build/typecheck per the Coder's "Long-running commands" discipline). Do **NOT** run Gate 1 between cycles, and do NOT run Gate 2 (no Validator, no Simplifier, no Scrutinizer, no Evaluator, no Tester between cycles). The engine runs ONE final Gate 1 after the loop exits — see the `gate1_postcode()` cadence (Gate 1 #2).
+If survivors remain: before the fix phase, record `preFixSha` (run `git rev-parse HEAD` via a haiku Git agent — pin the return to `\{"sha": "<40-hex>"\}` and validate the shape before using it; fall back to the wider prior base if the SHA is absent or malformed, never narrow) — this SHA defines the next cycle's delta-review scope. Then batch the confirmed findings for fixing: group findings by file — one file per set of sub-batches, chunked at max 5 findings per sub-batch; never mix two files in one batch. A finding with no `file` field is its own singleton batch. Sub-batches for the SAME file run sequentially (never two Coders editing the same file concurrently — same-file edits in `parallel()` cause index contention and lost fixes); sub-batches for DISTINCT files run via `parallel()` (different code areas — safe per concurrency doctrine). Each Coder's prompt pins a return contract: `\{"status": "fixed"|"blocked", "commitShas": [...], "unresolved": [...]\}` — check `result.status === "fixed"` to decide disposition, never an absent `failed` flag. After fixes: update `reviewBaseSha = preFixSha`. Findings addressed by the fix Coders are reported FIXED (with `fixedInCycle` + commit SHAs), trusted by design; note that cycles 2+ DELTA REVIEW re-checks earlier fixes for free. `survivingFindings` = only findings NOT addressed (fix Coder dead/failed/blocked or deferred). The fixing Coder **self-verifies its own fix builds** (build/typecheck per the Coder's "Long-running commands" discipline). Do **NOT** run Gate 1 between cycles, and do NOT run Gate 2 (no Validator, no Simplifier, no Scrutinizer, no Evaluator, no Tester between cycles). The engine runs ONE final Gate 1 after the loop exits — see the `gate1_postcode()` cadence (Gate 1 #2).
 @end
 
 @define concurrency_doctrine():
@@ -137,22 +143,33 @@ The Workflow runtime KILLS any sub-agent that emits no output for 180 seconds. A
 
 **RULE: any agent (Validator, Coder, Tester) running a build / test / compile / install that may run silent for more than ~120s MUST run it in the BACKGROUND and POLL — never as a single silent foreground command.**
 
+Build commands are NEVER wrapped in `sh -c`, `bash -c`, or inline interpreters (`python3 -c`, `node -e`). Invoke commands directly with the step-1 redirect form — permission systems deny wrapper-invoked commands that would be allowed directly.
+
 Mechanical procedure (spike-verified — a workflow sub-agent survived a 253s job this way):
 
+0. **Pre-load Monitor:** before launching any background task, load the `Monitor` tool via ToolSearch (`select:Monitor`).
 1. Choose ONE unique base path for this run and reuse it verbatim in steps 1–3, e.g. `BASE=/tmp/df-build-<ticket-slug>`. Launch the command with the Bash tool using `run_in_background: true`:
    ```
    <build/test command> > <BASE>.log 2>&1; echo "EXIT=$?" > <BASE>.done
    ```
    This returns immediately with a background task id — do NOT block on it.
-2. Poll with the `Monitor` tool (load it first via ToolSearch `select:Monitor` if it is not already available). Arm ONE monitor that emits a heartbeat well under 180s AND exits when the job finishes:
+2. Arm ONE Monitor that emits a heartbeat well under 180s AND exits when the job finishes:
    - description: short, e.g. `await <build cmd>`
    - persistent: false
    - timeout_ms: comfortably ABOVE the expected job time (e.g. 600000)
    - command: `until [ -f <BASE>.done ]; do echo building; sleep 25; done; echo BUILD_DONE; cat <BASE>.done`
 
    The `building` heartbeat every 25s (≪ 180s) is delivered as a notification that re-invokes you, so the watchdog never sees a >180s gap. `BUILD_DONE` + the `EXIT=` line signal completion.
+
+   **Exit-code honesty:** the background task's own exit status is meaningless (the trailing `echo` always exits 0). ALWAYS read the `EXIT=` value written inside `<BASE>.done` — that is the authoritative result.
+
+   **Bounded polling:** arm ONE Monitor then stop acting. On Monitor timeout, re-arm at most 2× (never more than 3 total Monitor calls per build). If the build has not finished after 3 Monitor calls: record the state and escalate — never babysit. A Coder burned 241k tokens polling one build.
 3. When the monitor reports `BUILD_DONE`: the job PASSES iff `<BASE>.done` contains `EXIT=0`. Read `<BASE>.log` for output/failure detail.
 
+**Cheapest-sufficient validation:** iterate with the fastest check that proves the change — `tsc --noEmit`, `cargo check`, `go vet`, a single test file — the ecosystem's cheapest sufficient signal. Reserve expensive full/optimized builds and whole-suite runs for the final gates only.
+
+**One build gate per phase:** batch related fixes, validate once. A fix-pass Coder runs ONE light check over its whole batch — never several invocations per small fix. Do NOT validate after every individual mutation; validate once after the batch is complete.
+
 **Scope commands to stay short.** During the engine, PREFER crate/package-scoped builds and tests — `cargo build -p <crate>`, `cargo test -p <crate>`, `npm test -- <path>`, `go test ./pkg/...` — over the whole workspace. The full-workspace regression is the human's job after the wave (the wave already hands the integrated branch back to the user). Scoping keeps most commands under the watchdog window and under budget.
 
 **Invariants:** heartbeat interval MUST stay well under 180s (25–30s is the tested value); Monitor `timeout_ms` MUST exceed the expected job duration (a too-short timeout kills the poll, not the build). Never substitute a single silent long command for this procedure.
@@ -172,20 +189,37 @@ Each ticket engine run returns a structured result. The Synthesizer or the wave
   "survivingFindings": [
     {
       "focus": "string — reviewer focus area",
-      "finding": "string — description",
-      "severity": "critical | high | medium | low"
+      "file": "string — primary file the finding relates to, or null if scope unknown",
+      "description": "string — one-sentence issue statement",
+      "severity": "critical | high | medium | low",
+      "status": "surviving"
+    }
+  ],
+  "fixedFindings": [
+    {
+      "focus": "string — reviewer focus area",
+      "file": "string — primary file the finding relates to, or null if scope unknown",
+      "description": "string — one-sentence issue statement",
+      "severity": "critical | high | medium | low",
+      "status": "fixed",
+      "fixedInCycle": "number — cycle when fixed",
+      "commitShas": ["string — commit SHAs from the fix Coder"]
     }
   ],
+  "reviewCoverage": {
+    "failedFocuses": ["string — focus areas where reviewer died after retry"],
+    "complete": "boolean — true iff no coverage gaps"
+  },
   "escalations": [
     {
-      "type": "merge-conflict | gate2-fail | validation-exhausted | ambiguous-resolution",
+      "type": "merge-conflict | gate2-fail | validation-exhausted | ambiguous-resolution | review-coverage-incomplete | dependency-blocked | engine-crash",
       "description": "string"
     }
   ],
   "filesChanged": ["string — list of modified file paths"],
   "gate2": {
-    "evaluatorVerdict": "PASS | FAIL | SKIPPED",
-    "testerVerdict": "PASS | FAIL | SKIPPED",
+    "evaluatorVerdict": "PASS | FAIL | FAIL-FIXED | SKIPPED",
+    "testerVerdict": "PASS | FAIL | FAIL-FIXED | SKIPPED",
     "skipReasons": ["string — why a gate was skipped, if applicable"]
   }
 }
@@ -200,6 +234,7 @@ Each ticket engine run returns a structured result. The Synthesizer or the wave
 3. **All written code passes Gate 1.** No code merge, commit, or handoff before Validator + Simplifier + Scrutinizer (in that order).
 4. **Gate 2 runs once, at implementation acceptance.** It does not re-run after review-fixes.
 5. **NEVER auto-merge to main or master.** All merges target the integration branch. The user merges to main themselves.
+6. **No unauthorized GitHub side-effects.** Sub-agents NEVER create GitHub issues/PRs, comment, or push beyond the ticket-authorized branch unless the ticket, plan, or user explicitly authorizes that exact action. Proposed follow-ups go in the run report.
 @end
 
 @export gate1_postcode

commands/_partials/_preamble.mds

@@ -51,14 +51,10 @@ Two distinct LLM-authored-script bugs have crashed whole runs in the field: `Typ
 - **`meta` is a pure literal** — `name` and `description` present; NO variables, function calls, spreads, or template interpolation anywhere inside `meta`.
 - **Every `pipeline(x, …)` / `parallel(x)` first argument is a real array** — never a function, object, or possibly-undefined value. If it derives from a prior agent result, coerce it: `parallel((maybeList || []).map(...))`.
 - **No reference to a possibly-undefined field** — guard every cross-result field access with optional chaining (e.g. `result?.findings`, `seal?.num`). This is the `SEAL.num` crash class: an agent returned a shape without `num`, so `SEAL.num` threw.
-- **`.filter(Boolean)` before mapping over `agent()` / `parallel()` results** — an agent can return `null` (skipped, or died after retries); strip nulls before `.map` or field access.
+- **`.filter(Boolean)` before mapping over `agent()` / `parallel()` results** — an agent can return `null` (skipped, or died after retries); strip nulls before `.map` or field access. filter(Boolean) is crash-safety only; it must never convert missing required coverage into success.
 - **`phase()` titles match the phases declared in `meta`** — every `phase("…")` call has a matching declared phase, and vice-versa.
 
-Then run a cheap syntax gate: write the authored script to a scratch temp file (e.g. `/tmp/df-wf-check.js`) purely to validate it, run `node --check /tmp/df-wf-check.js`, and only then pass the script text to the `Workflow` tool as usual. `node --check` catches SYNTAX errors only — it does NOT catch the runtime type errors above, so the checklist is the real safeguard.
-
-### --dry-run affordance
-
-If the user's input includes `--dry-run`, PRINT the authored workflow script (inside a fenced code block) for inspection, then STOP — do not invoke the Workflow tool. This lets the user review the script before execution. For a large or novel script, prefer this dry-run path by default — eyeball the script once before the first live run.
+Then run a cheap syntax gate: write the authored script to a fresh, run-unique scratch file `/tmp/df-wf-check-<meta.name>-<epoch-seconds>.js` (a NEW filename every run — never reuse a prior run's path, as rewriting an existing file trips write guards), run `node --check` on that file, and only then pass the script text to the `Workflow` tool as usual. `node --check` catches SYNTAX errors only — it does NOT catch the runtime type errors above, so the checklist is the real safeguard.
 
 ### Budget scaling