ship: prepare lane for review

Author: arul28

apps/ade-cli/src/tuiClient/__tests__/ApprovalPrompt.test.tsx

@@ -66,4 +66,51 @@ describe("ApprovalPrompt", () => {
     expect(frame).toContain("enter");
     expect(frame).toContain("next/send");
   });
+
+  it("renders orchestration model-selection briefing metadata", () => {
+    const approval: PendingApproval = {
+      itemId: "model-1",
+      description: "Build the orchestration roster.",
+      highStakes: false,
+      mode: "question",
+      request: {
+        requestId: "model-1",
+        source: "ade",
+        kind: "model_selection",
+        title: "Pick a model for the web-ui worker",
+        description: "Build the orchestration roster.",
+        allowsFreeform: true,
+        blocking: true,
+        canProceedWithoutAnswer: false,
+        providerMetadata: {
+          role: "worker",
+          tag: "web-ui",
+          workDescription: "Build the orchestration roster.",
+          filesHint: [" OrchestrationPanel.tsx ", "TaskCard.tsx"],
+          dependsOn: [" planning-rounds ", "model-routing"],
+        },
+        questions: [
+          {
+            id: "model",
+            header: "Model",
+            question: "Which model should the web-ui worker use?",
+          },
+        ],
+      },
+    };
+
+    const frame = stripAnsi(render(
+      <ApprovalPrompt
+        approval={approval}
+        questionState={createPendingQuestionSelectionState(approval)}
+        width={100}
+      />,
+    ).lastFrame() ?? "");
+
+    expect(frame).toContain("MODEL SELECTION");
+    expect(frame).toContain("Description: Build the orchestration roster.");
+    expect(frame).toContain("Files: OrchestrationPanel.tsx, TaskCard.tsx");
+    expect(frame).toContain("Runs after: planning-rounds, model-routing");
+    expect(frame).toContain("Which model should the web-ui worker use?");
+  });
 });

apps/ade-cli/src/tuiClient/components/ApprovalPrompt.tsx

@@ -46,6 +46,38 @@ function pendingInputAccent(source: string | null | undefined): string {
   return theme.provider(normalized as AdeCodeProvider).color;
 }
 
+function stringValue(value: unknown): string | null {
+  return typeof value === "string" && value.trim() ? value.trim() : null;
+}
+
+function stringListValue(value: unknown): string[] {
+  return Array.isArray(value)
+    ? value.flatMap((entry) => {
+        if (typeof entry !== "string") return [];
+        const trimmed = entry.trim();
+        return trimmed.length ? [trimmed] : [];
+      })
+    : [];
+}
+
+function compactList(values: string[], limit = 3): string {
+  const visible = values.slice(0, limit);
+  const suffix = values.length > limit ? `, +${values.length - limit} more` : "";
+  return `${visible.join(", ")}${suffix}`;
+}
+
+function modelSelectionBriefing(metadata: Record<string, unknown> | undefined): Array<[string, string]> {
+  if (!metadata) return [];
+  const rows: Array<[string, string]> = [];
+  const description = stringValue(metadata.workDescription);
+  const files = stringListValue(metadata.filesHint);
+  const dependsOn = stringListValue(metadata.dependsOn);
+  if (description) rows.push(["Description", description]);
+  if (files.length) rows.push(["Files", compactList(files)]);
+  if (dependsOn.length) rows.push(["Runs after", compactList(dependsOn)]);
+  return rows;
+}
+
 /**
  * An access-key prefix shown immediately before an action's pill, e.g. the `a`
  * in `a [ approve ]`. Accentuated when the action is highlighted.
@@ -174,6 +206,9 @@ export function ApprovalPrompt({
   const answeredCount = isQuestion
     ? pendingQuestionAnsweredCount(approval.request, questionState?.answers ?? {})
     : 0;
+  const briefingRows = kind === "model_selection"
+    ? modelSelectionBriefing(approval.request?.providerMetadata)
+    : [];
 
   const card = (
     <Box
@@ -220,6 +255,16 @@ export function ApprovalPrompt({
         </Text>
       ) : null}
 
+      {briefingRows.length ? (
+        <Box flexDirection="column" marginTop={1}>
+          {briefingRows.map(([label, value]) => (
+            <Text key={label} color={theme.color.t3} wrap="truncate-end">
+              {truncateEnd(`${label}: ${value}`, textWidth)}
+            </Text>
+          ))}
+        </Box>
+      ) : null}
+
       {isQuestion && questions.length ? (
         <Box flexDirection="column" marginTop={1}>
           {questions.map((entry, questionIndex) => {

apps/desktop/resources/agent-skills/ade-orchestrator/SKILL.md

@@ -20,7 +20,7 @@ The orchestration **bundle** at `<bundlePath>/manifest.json` + `<bundlePath>/pla
 Every rule below is a default. The user is authoritative. If the user directly instructs a deviation ("skip validation for this run", "no audit gate", "no asking, use Opus for everything", "only plan, I'll spawn workers myself"), comply with the instruction.
 
 When you accept an override:
-1. Log a `UserOverrideEntry` to `manifest.userOverrides` with the literal user instruction and an indication of which default rule it waives.
+1. Log the literal user instruction and which default rule it waives. For planning-round or validation-sequence overrides, do this with `recordPlanningOverride({ skippedRounds, skipReason })` so the service writes the matching `UserOverrideEntry` atomically. For other overrides, append `UserOverrideEntry` through the manifest patch path.
 2. Surface the material risk **once** in chat (one short paragraph). Do not re-prompt the default later in the same scope.
 3. Apply the override consistently — if the user says "no validation", do not propose validation steps in *this run*.
 
@@ -33,41 +33,33 @@ When you accept an override:
 
 ## §3 — Planning protocol (lead only)
 
-**Planning is interactive. You MUST ask the user questions at each step — do not silently plan and present a finished plan. The steps below are sequential; complete each one (with user confirmation) before moving to the next.**
+**Planning is a deterministic, gated sequence — server-enforced, not a suggestion.** It mirrors the user's dev loop: **context intake → three deliberation rounds (functional → UI → extras) → validation derivation → model picks → approval**. The gates physically block you: `askUserForModelSelection` is locked until all three rounds are recorded, and `requestPlanApproval` is locked until intake + rounds + validation steps exist. You cannot skip ahead by writing prose. Do not silently plan and present a finished plan — every round asks the user real questions through the question card.
 
-1. Read `goal.md` if present in the lane worktree; otherwise `askUser` for a one-line goal. Persist it to `manifest.goalSummary`.
+The planning state lives in `manifest.leadState.planning.stage` and advances `intake → round_functional → round_ui → round_extras → rounds_complete → ready`. Each transition is written only through the tools below (you are denied raw patch access to `/leadState/planning` and `/planSpec`).
 
-2. **Codebase intake — inspect-first, ask-on-uncertainty.** Read `CLAUDE.md`, `README.md`, package manifests (`package.json` / `pyproject.toml` / `Cargo.toml` / `go.mod` / etc.), CI config (`.github/workflows/`, `.circleci/`, `.gitlab-ci.yml`), top-level directory listing, recent `git log --oneline -50`. Infer: project shape, test stack, ancillary surfaces (docs/, mobile apps, SDKs, OpenAPI specs), available CI gates, doc structure.
+1. **Goal.** Read `goal.md` if present in the lane worktree; otherwise `askUser` for a one-line goal. Persist it to `manifest.goalSummary`.
 
-3. Propose a **tag taxonomy** (3–6 tags) and confirm via `askUser`. Tags are project-specific, not preset. Examples by shape:
-   - Fullstack web → `web-ui` / `backend` / `docs` / `tests`
-   - Graphics → `render-pipeline` / `shaders` / `assets`
-   - Mobile → `swiftui` / `storekit` / `share-extension`
-   - Library → `core-api` / `examples` / `docs`
+2. **Codebase intake (the `/context` step) — REQUIRED FIRST.** Read `CLAUDE.md`/`README.md`, package manifests (`package.json` / `pyproject.toml` / `Cargo.toml` / `go.mod`), CI config (`.github/workflows/` etc.), the top-level directory listing, and recent `git log`/`git diff main`. `planAppend` a human-readable **"Codebase intake"** section, then call **`recordCodebaseIntake({ projectShape, testStack, inFlightWork, ancillarySurfaces, docMap, ciGates })`**. This advances the stage to `round_functional`; nothing else unlocks until it is recorded.
 
-4. Propose **tasks** per phase. For Developing tasks, include `filesHint` derived from the intake (files most likely to be touched).
+3. **Three deliberation rounds (the `/plan` step).** Run each with **`askPlanningRound`**, in order — the tool enforces it:
+   - **Round 1 — functional** (`kind: "functional"`): resolve the real functional ambiguities. Offer concrete `options` with tradeoffs in `description`; never ask the user to write prose.
+   - **Round 2 — UI** (`kind: "ui"`): put an **ASCII wireframe in each option's `preview`** (rendered as a monospace box). If the change has no UI, offer a single "N/A — no UI" option.
+   - **Round 3 — extras** (`kind: "extras"`, usually `multiSelect: true`): delightful extras the user didn't ask for but might want.
+   Always pass `lockedSummary` (your one-line locked outcome). **Cascade rule:** if the user introduces new functional scope mid-plan, run a focused mini-round for just that piece (`askPlanningRound({ cascadedFrom: <round id>, ... })`) and merge it — do not redesign locked decisions.
 
-5. **Plan quality minimum.** The plan may include any extra detail that helps the user or workers, but before approval it must include at least:
-   - Goal, assumptions, and locked user decisions.
-   - In-scope work.
-   - Clear out-of-scope / non-goals.
-   - Alternatives, options, or tradeoffs considered for major choices.
-   - UI / UX / user-facing decisions when applicable, or an explicit "not applicable" note.
-   - Planned implementation order, dependencies, and what can run in parallel.
-   - Agent plan: worker / validator tags to spawn, model-routing status, and what each owns.
-   - Coordination/logging plan: how `plan.md` and the manifest stay updated as agents start, fail, discover gaps, finish, and replan.
-   - Validation / proof plan with concrete checks or evidence derived from the repo.
-   - Plan presentation details for the plan pane. Use GFM tables, mermaid fences, images, and links to `artifacts/ui/*.html` for design specs. Do not embed raw iframes; ADE renders `artifacts/ui/*.html` links as sandboxed previews with a full-design action.
+4. **Tag taxonomy + tasks.** Propose a project-specific tag taxonomy (3–6 tags, e.g. `web-ui` / `backend` / `docs`). Create tasks per phase via `manifestPatch`; for Developing tasks include `filesHint` derived from the intake.
 
-6. **Validation step derivation.** See §6. Detect which `ValidationConcern`s apply by inspecting the repo; ask the user where uncertain; write codebase-specific `prompt` text into each `validationStrategy.steps[]` entry. Do not assume vitest / pytest / specific CI commands unless the inspection confirmed them.
+5. **Validation derivation (the `/quality` + `/test` step).** See §6. Detect which `ValidationConcern`s apply by inspecting the repo and write codebase-specific `prompt` text into each `validationStrategy.steps[]` entry. At least one validation step is required before approval (or log a skip-validation override — see §1). Per-worker tasks get `reverify_changes`; the heavier `/quality` dual-review + `/test` stewardship + parity run as the `validating` phase panel.
 
-7. **Model picks.** For every `(role, tag)` pair (where role ∈ `worker`, `validator`), call `askUserForModelSelection(role, tag, workDescription)`. Always include a short `workDescription` (one sentence) explaining what this agent will do — e.g. "Implement the login form component and auth route" not just "renderer worker". The picker UI is ADE's in-house `ModelPicker` — never present a flat option list. Model selection must happen during planning, before `requestPlanApproval`. The tool will reject calls after the plan is approved.
+6. **Model picks.** Now unlocked. For every `(role, tag)` pair, call **`askUserForModelSelection({ role, tag, workDescription, filesHint, dependsOn })`**. Always include a one-sentence `workDescription` and, when known, `filesHint` (files it will touch) and `dependsOn` — the picker renders these as an agent briefing so the user can choose a fitting model. Never present a flat option list.
 
-8. Append a `DecisionLogEntry` per lock-in (tags, validation strategy, model routing, etc.). Each entry carries `source: "lead"`, `at`, and a short `summary`.
+7. **plan.md is the single source of truth.** Author the plan narrative incrementally as each round locks — `planAppend` the required sections so the user watches the plan grow live on the sidebar. The required sections (checked structurally at approval): **Goal · In scope · Out of scope · Alternatives · Implementation order · Agent plan · Validation plan · UI decisions (or N/A) · Coordination.** Use GFM tables, mermaid fences, and links to `artifacts/ui/*.html` for specs (rendered as sandboxed previews). There is no separate "approval summary" — the user approves the live plan.md.
 
-9. **Plan-ready gate.** Once Planning is complete, append a final plan-ready note and tell the user they can keep planning in chat or review the plan pane. Then call `requestPlanApproval` / present a `kind: "plan_approval"` pending input that summarises the proposed plan. This surfaces the plan-pane **Implement** button. The approval summary must pass the plan quality minimum above. **Until the user clicks Implement or otherwise approves, do not call `spawnAgent`.**
+8. **Approval.** Call **`requestPlanApproval`** (no summary argument — it reads the live plan.md). It marks planning ready, runs the structural readiness check over plan.md + manifest state, and surfaces the **Implement** button on the plan narrative. On approval the run advances to `developing`; on decline it records `changes_requested` so the panel can show a re-approval diff. **Until the user approves, `spawnAgent` is blocked.**
 
-10. **Live plan sync.** During Developing and Validating, keep `plan.md` synchronized as the shared operations log. Append worker starts, ownership changes, failures, material discoveries, re-plans, validation evidence, and final handoff notes so every agent can understand the live run without reading private chat transcripts.
+9. **User override (§1).** If the user explicitly waives a round ("no UI here, skip it") or validation, call **`recordPlanningOverride({ skippedRounds, skipReason })`** with the literal instruction as `skipReason`. The service logs the matching `UserOverrideEntry`; skipped rounds are only treated as satisfied when those entries exist.
+
+10. **Live plan sync.** During Developing and Validating, keep `plan.md` synchronized as the shared operations log — worker starts, ownership changes, failures, material discoveries, re-plans, validation evidence, and final handoff notes.
 
 ## §4 — Developing protocol (worker only)
 
@@ -105,7 +97,23 @@ When the planner writes a `validationStrategy.steps[]` entry, pick a `Validation
 
 **Planner derivation.** Write the prompt naming the file types the worker is touching and the relevant edge-case categories for *this* codebase. No vitest / React / specific tooling unless the inspection confirmed it exists.
 
-### `test_suite_truthfulness` (automate principle, only when codebase has tests)
+### The validation panel (how the heavy pass runs)
+
+The Validating phase runs as a **lean perspective-diverse panel**: spawn a small set of validators, each with a distinct lens, then synthesize. Call `proposeValidationSteps` to get codebase-aware suggestions seeded from the intake; review, edit, and write the ones you want via `manifestPatch`. Validators emit **structured findings** through `recordValidationRun({ findings: [{ severity, locus, title, fix, regressionTestTarget }] })` — the panel rolls these into a Blocker/High/Medium/Low table, and every Blocker/High must carry a `regressionTestTarget` (the named test that pins it). Keep the panel small (one validator per lens, not a fan-out).
+
+### `dual_review_correctness_security` (the /quality correctness + security track)
+
+**Principle.** Review the whole diff for bugs, broken existing features (trace cross-app/IPC side effects), unhandled error branches, and the security surface (secrets, permission/allowlist gaps, data-integrity). Emit structured findings with honest severity; never pad.
+
+### `dual_review_maintainability` (the /quality maintainability track)
+
+**Principle.** Review the diff for structural simplification, dead code, spaghetti conditionals, unnecessary optionality/casts, and feature logic leaking into shared/canonical layers. Each finding names the smallest behavior-preserving fix.
+
+### `regression_pinning` (ties /quality → /test)
+
+**Principle.** Turn every Blocker/High from the dual-review into a named regression test that fails on the bug and passes once fixed. A finding is not handled until a test pins it. Only meaningful when the codebase has tests.
+
+### `test_suite_truthfulness` / `test_stewardship` (automate principle, only when codebase has tests)
 
 **Principle.** "Leave the suite more truthful and smaller, not just larger." Three passes in order:
 - **PRUNE** — orphaned tests, `skip` / `only` / `todo`, anti-pattern tests like `expect(true)` or zero-assertion bodies, over-mocked fixtures, render-only UI tests.