docs: specify skills-mode flag matrix

Author: lee-to

docs/architecture.md

@@ -185,7 +185,24 @@ Auto-review strategy is controlled globally by `AGENT_AUTO_REVIEW_STRATEGY`:
 
 Tasks also have a `skipReview` flag (default `false`). When `true`, the coordinator bypasses the review stage entirely — after successful implementation the task moves directly to `done`, skipping the `review-sidecar` and `security-sidecar` runs. This is useful for small changes or tasks where code review is unnecessary.
 
-Skills-mode tasks (`useSubagents=false`) also have two opt-in flags. `runPlanImprove` inserts `/aif-improve` after the initial plan and before `plan_ready`. `runPostVerify` inserts `/aif-verify` after implementation and before review; if `skipReview=true`, verification moves directly to `done`. Both flags default to `false` and are ignored for subagent tasks.
+Skills-mode tasks (`useSubagents=false`) also have two opt-in flags. `runPlanImprove` inserts `/aif-improve` after the initial plan and before `plan_ready`. This is plan refinement: it may replace the stored plan only when the improver returns a complete plan-shaped update. `runPostVerify` inserts `/aif-verify` after implementation and before review. This is an execution validation gate: it stores verification output, passes through to review/done on pass or warn, and moves to `blocked_external` for a blocking gate result. Both flags default to `false` and are ignored for subagent tasks.
+
+Flag interaction table:
+
+| `useSubagents` | `skipReview` | `runPlanImprove` | `runPostVerify` | Effective pipeline after planning starts                                |
+| -------------- | ------------ | ---------------- | --------------- | ----------------------------------------------------------------------- |
+| `true`         | `false`      | ignored          | ignored         | Planning → Plan Ready → Implementing → Review → Done                    |
+| `true`         | `true`       | ignored          | ignored         | Planning → Plan Ready → Implementing → Done                             |
+| `false`        | `false`      | `false`          | `false`         | Planning → Plan Ready → Implementing → Review → Done                    |
+| `false`        | `true`       | `false`          | `false`         | Planning → Plan Ready → Implementing → Done                             |
+| `false`        | `false`      | `true`           | `false`         | Planning → Improve → Plan Ready → Implementing → Review → Done          |
+| `false`        | `true`       | `true`           | `false`         | Planning → Improve → Plan Ready → Implementing → Done                   |
+| `false`        | `false`      | `false`          | `true`          | Planning → Plan Ready → Implementing → Verify → Review → Done           |
+| `false`        | `true`       | `false`          | `true`          | Planning → Plan Ready → Implementing → Verify → Done                    |
+| `false`        | `false`      | `true`           | `true`          | Planning → Improve → Plan Ready → Implementing → Verify → Review → Done |
+| `false`        | `true`       | `true`           | `true`          | Planning → Improve → Plan Ready → Implementing → Verify → Done          |
+
+`verify` remains a coordinator stage, not a human action. That keeps it covered by the same claim, timeout, watchdog, runtime-profile, and activity-log machinery as other autonomous work. The semantic contract is narrower than review: verify validates the implementation against the accepted plan, while review/security sidecars evaluate code quality and risk.
 
 ### QA Pipeline
 

packages/agent/src/__tests__/coordinator.test.ts

@@ -348,6 +348,179 @@ describe("coordinator", () => {
     expect(runVerifier).toHaveBeenCalledTimes(1);
   });
 
+  it.each([
+    {
+      name: "subagent mode ignores improve/verify and runs review",
+      useSubagents: true,
+      skipReview: false,
+      runPlanImprove: true,
+      runPostVerify: true,
+      expectImprover: false,
+      expectVerifier: false,
+      expectReviewer: true,
+    },
+    {
+      name: "subagent mode with skipReview ignores verify and goes done",
+      useSubagents: true,
+      skipReview: true,
+      runPlanImprove: true,
+      runPostVerify: true,
+      expectImprover: false,
+      expectVerifier: false,
+      expectReviewer: false,
+    },
+    {
+      name: "skills mode baseline runs review",
+      useSubagents: false,
+      skipReview: false,
+      runPlanImprove: false,
+      runPostVerify: false,
+      expectImprover: false,
+      expectVerifier: false,
+      expectReviewer: true,
+    },
+    {
+      name: "skills mode skipReview goes done",
+      useSubagents: false,
+      skipReview: true,
+      runPlanImprove: false,
+      runPostVerify: false,
+      expectImprover: false,
+      expectVerifier: false,
+      expectReviewer: false,
+    },
+    {
+      name: "skills mode improve then review",
+      useSubagents: false,
+      skipReview: false,
+      runPlanImprove: true,
+      runPostVerify: false,
+      expectImprover: true,
+      expectVerifier: false,
+      expectReviewer: true,
+    },
+    {
+      name: "skills mode improve with skipReview goes done",
+      useSubagents: false,
+      skipReview: true,
+      runPlanImprove: true,
+      runPostVerify: false,
+      expectImprover: true,
+      expectVerifier: false,
+      expectReviewer: false,
+    },
+    {
+      name: "skills mode verify then review",
+      useSubagents: false,
+      skipReview: false,
+      runPlanImprove: false,
+      runPostVerify: true,
+      expectImprover: false,
+      expectVerifier: true,
+      expectReviewer: true,
+    },
+    {
+      name: "skills mode verify with skipReview goes done",
+      useSubagents: false,
+      skipReview: true,
+      runPlanImprove: false,
+      runPostVerify: true,
+      expectImprover: false,
+      expectVerifier: true,
+      expectReviewer: false,
+    },
+    {
+      name: "skills mode improve and verify then review",
+      useSubagents: false,
+      skipReview: false,
+      runPlanImprove: true,
+      runPostVerify: true,
+      expectImprover: true,
+      expectVerifier: true,
+      expectReviewer: true,
+    },
+    {
+      name: "skills mode improve and verify with skipReview goes done",
+      useSubagents: false,
+      skipReview: true,
+      runPlanImprove: true,
+      runPostVerify: true,
+      expectImprover: true,
+      expectVerifier: true,
+      expectReviewer: false,
+    },
+  ])(
+    "should follow the skills-mode flag truth table: $name",
+    async ({
+      name,
+      useSubagents,
+      skipReview,
+      runPlanImprove,
+      runPostVerify,
+      expectImprover,
+      expectVerifier,
+      expectReviewer,
+    }) => {
+      const db = testDb.current;
+      const taskId = `task-flag-table-${name.replace(/[^a-zA-Z0-9]+/g, "-").toLowerCase()}`;
+      db.insert(tasks)
+        .values({
+          id: taskId,
+          projectId: "test-project",
+          title: name,
+          status: "planning",
+          autoMode: true,
+          useSubagents,
+          skipReview,
+          runPlanImprove,
+          runPostVerify,
+        })
+        .run();
+
+      await pollAndProcess();
+
+      expect(runPlanner).toHaveBeenCalledWith(taskId, "/tmp/test");
+      expect(runPlanChecker).toHaveBeenCalledWith(taskId, "/tmp/test");
+      expect(runImplementer).toHaveBeenCalledWith(taskId, "/tmp/test");
+
+      if (expectImprover) {
+        expect(runImprover).toHaveBeenCalledWith(taskId, "/tmp/test");
+        expect(vi.mocked(runPlanner).mock.invocationCallOrder[0]).toBeLessThan(
+          vi.mocked(runImprover).mock.invocationCallOrder[0] ?? 0,
+        );
+        expect(vi.mocked(runImprover).mock.invocationCallOrder[0]).toBeLessThan(
+          vi.mocked(runPlanChecker).mock.invocationCallOrder[0] ?? 0,
+        );
+      } else {
+        expect(runImprover).not.toHaveBeenCalled();
+      }
+
+      if (expectVerifier) {
+        expect(runVerifier).toHaveBeenCalledWith(taskId, "/tmp/test");
+        expect(vi.mocked(runImplementer).mock.invocationCallOrder[0]).toBeLessThan(
+          vi.mocked(runVerifier).mock.invocationCallOrder[0] ?? 0,
+        );
+      } else {
+        expect(runVerifier).not.toHaveBeenCalled();
+      }
+
+      if (expectReviewer) {
+        expect(runReviewer).toHaveBeenCalledWith(taskId, "/tmp/test");
+        const previousStageOrder = expectVerifier
+          ? (vi.mocked(runVerifier).mock.invocationCallOrder[0] ?? 0)
+          : (vi.mocked(runImplementer).mock.invocationCallOrder[0] ?? 0);
+        expect(previousStageOrder).toBeLessThan(
+          vi.mocked(runReviewer).mock.invocationCallOrder[0] ?? 0,
+        );
+      } else {
+        expect(runReviewer).not.toHaveBeenCalled();
+      }
+
+      const task = db.select().from(tasks).where(eq(tasks.id, taskId)).get();
+      expect(task!.status).toBe("done");
+    },
+  );
+
   it("should not auto-implement plan_ready tasks when autoMode=false", async () => {
     const db = testDb.current;
     db.insert(tasks)