fix(review): prohibit isolation param in roster output and SKILL.md

packages/cli/src/commands/review/agent-prompt.test.ts

@@ -1253,6 +1253,50 @@ describe('--roster — every prompt the plan requires, in one call', () => {
       rmSync(dir, { recursive: true, force: true });
     }
   });
+
+  it('emits the working_dir parameter note when worktreePath is present', () => {
+    // A run that passed both `working_dir` and `isolation: "worktree"` failed
+    // all 11 agents (mutually exclusive). The roster is the last text the
+    // orchestrator reads before constructing agent calls — the parameter note
+    // must be there, not just 400 lines back in SKILL.md.
+    const dir = mkdtempSync(join(tmpdir(), 'ap-roster-wt-'));
+    try {
+      const wt = '.qwen/tmp/review-pr-9999';
+      const plan = join(dir, 'plan.json');
+      writeFileSync(
+        plan,
+        JSON.stringify({ ...PLAN, worktreePath: wt, prNumber: '9999' }),
+      );
+      (agentPromptCommand.handler as (a: unknown) => void)({
+        plan,
+        roster: true,
+      });
+      const printed = (writeStdoutLine as unknown as Mock).mock
+        .calls[0][0] as string;
+      expect(printed).toContain(`working_dir: "${wt}"`);
+      expect(printed).toContain('Do NOT set `isolation`');
+      expect(printed).toContain('mutually exclusive');
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('omits the parameter note when worktreePath is absent', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'ap-roster-nowt-'));
+    try {
+      const plan = join(dir, 'plan.json');
+      writeFileSync(plan, JSON.stringify(PLAN));
+      (agentPromptCommand.handler as (a: unknown) => void)({
+        plan,
+        roster: true,
+      });
+      const printed = (writeStdoutLine as unknown as Mock).mock
+        .calls[0][0] as string;
+      expect(printed).not.toContain('Do NOT set `isolation`');
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
 });
 
 // Dogfooded on a real 3A review: the orchestrator delivered Step 3 prompts verbatim

packages/cli/src/commands/review/agent-prompt.ts

@@ -1257,6 +1257,22 @@ function runRoster(report: PlanReport, planPath: string, rules?: string): void {
     recordPrompt(planPath, key, prompt);
     return `───── agent ${i + 1} of ${roster.length} — ${rosterLabel(req)} ─────\n\n${prompt}`;
   });
+  // Worktree-mode reviews: remind the orchestrator of the exact Agent tool
+  // parameters at the point of action. A run that passed both `working_dir`
+  // and `isolation: "worktree"` failed all 11 agents (mutually exclusive) and
+  // the review produced nothing. The roster is the last text the orchestrator
+  // reads before constructing agent calls — a reminder here is worth more than
+  // one 400 lines back in SKILL.md.
+  const wt = report.worktreePath;
+  const paramNote =
+    typeof wt === 'string' && wt
+      ? `\n\n**Agent tool parameters (worktree mode):** Set ` +
+        `\`working_dir: "${wt}"\` and ` +
+        `\`subagent_type: "general-purpose"\`, \`run_in_background: false\` ` +
+        `on EVERY agent call below. Do NOT set \`isolation\` — the worktree ` +
+        `already exists; \`isolation\` creates a new copy and is mutually ` +
+        `exclusive with \`working_dir\`.`
+      : '';
   writeStdoutLine(
     [
       `${roster.length} agents required. Launch one agent per block below, ` +
@@ -1268,7 +1284,8 @@ function runRoster(report: PlanReport, planPath: string, rules?: string): void {
         `either is missing, this output was truncated in transit: every prompt ` +
         `is also recorded on disk, so rebuild just the missing blocks with ` +
         `--chunk <id>, or --role <r> (--file <path> for an invariant agent), ` +
-        `plus the same --rules this call was given.`,
+        `plus the same --rules this call was given.` +
+        paramNote,
       ...blocks,
       `───── end of roster — ${roster.length} agents ─────`,
     ].join('\n\n'),

packages/core/src/skills/bundled/review/SKILL.md

@@ -382,7 +382,7 @@ A check you perform silently is a check you skip, and this one has been skipped:
 
 **Every agent MUST return inline: set `subagent_type: "general-purpose"` and `run_in_background: false` on every `agent` call.** Do NOT fork them — never set `subagent_type: "fork"`. A fork runs fire-and-forget and its findings never come back to you, so the review would stall in Step 4 with nothing to aggregate. You need every agent's findings returned to you inline.
 
-**For same-repo PR reviews (worktree mode), every `agent` call MUST also set `working_dir: "<worktreePath>"`** — the `worktreePath` from the Step 1 fetch report (a repo-relative path like `.qwen/tmp/review-pr-<n>`; pass it through as-is). This sets each agent's working directory to the PR worktree, so its `git diff`, `grep_search`, file reads, and Agent 7's build/test **resolve against the PR's code, not the user's main checkout**. It is a deterministic, harness-level cwd pin — it does NOT depend on the agent remembering to `cd`, and it is what makes reviewing multiple PRs concurrently safe. (It pins the working directory; it is not a hard filesystem sandbox — an absolute path could still reach elsewhere — but normal review operations stay inside the worktree.) This rule applies to **every** agent the review workflow launches — not just the Step 3 dimension agents, but also the Step 4 verification agent and the Step 5 reverse-audit agents (both restated below). Do NOT set `working_dir` for **local-diff, file-path, or cross-repo lightweight** reviews — those have no worktree, so the agents run in the main project directory.
+**For same-repo PR reviews (worktree mode), every `agent` call MUST also set `working_dir: "<worktreePath>"`** — the `worktreePath` from the Step 1 fetch report (a repo-relative path like `.qwen/tmp/review-pr-<n>`; pass it through as-is). This sets each agent's working directory to the PR worktree, so its `git diff`, `grep_search`, file reads, and Agent 7's build/test **resolve against the PR's code, not the user's main checkout**. It is a deterministic, harness-level cwd pin — it does NOT depend on the agent remembering to `cd`, and it is what makes reviewing multiple PRs concurrently safe. (It pins the working directory; it is not a hard filesystem sandbox — an absolute path could still reach elsewhere — but normal review operations stay inside the worktree.) This rule applies to **every** agent the review workflow launches — not just the Step 3 dimension agents, but also the Step 4 verification agent and the Step 5 reverse-audit agents (both restated below). Do NOT set `working_dir` for **local-diff, file-path, or cross-repo lightweight** reviews — those have no worktree, so the agents run in the main project directory. **Do NOT set `isolation` on review agents.** `isolation: "worktree"` creates a brand-new worktree copy of the repo; the review worktree already exists at `worktreePath`, and `isolation` is mutually exclusive with `working_dir` — passing both fails every agent call with a parameter error and the review produces nothing.
 
 **You no longer compose these prompts. `qwen review agent-prompt` does** — one `--roster` call builds every one of them, and each block it prints goes to its agent unedited. It already contains everything the list below used to ask you to remember: `diffPathAbsolute` and the exact `read_file` ranges for that role (its own `offset`/`limit` for a chunk agent; every chunk for a whole-diff or 3A agent; the post-change file plus `addedRanges[]` and its own `diffRange` for an invariant agent), the agent's focus areas, the severity definitions verbatim, the finding format, and the project rules. **Never give an agent a `git diff` command** — see "Diff capture and the review topology" in Step 1 for why. In worktree-mode PR reviews the agent's `working_dir` is the PR worktree, so `grep_search` and source-file reads resolve against the PR's code automatically — the agent must NOT `cd` into the worktree or prefix absolute paths for those.