fix(setup): preserve explicit clients with all-detected selection (#5508)

* fix(setup): preserve explicit clients with all-detected selection

* test(filesystem): align staged cancellation cleanup assertions

---------

Co-authored-by: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com>

docs/installation.md

@@ -104,9 +104,11 @@ your investigation. Failed steps and diagnostics remain in terminal history.
 
 Select exact clients in scripts with repeatable `--client` flags. Each explicit
 client skips interactive selection. Use `--all-detected` only when you intend
-to configure every detected supported client. `--skill=false` omits the
-workflow, and `--dry-run` returns a read-only plan with status `planned` and
-exit code 0:
+to configure every detected supported client. Combining `--client` with
+`--all-detected` configures both the explicitly selected and detected clients,
+even when an explicit client is not detected. Each client is configured once.
+`--skill=false` omits the workflow, and `--dry-run` returns a read-only plan
+with status `planned` and exit code 0:
 
 ```bash
 rea setup --client codex --client cursor --skill=false --dry-run

src/application/Setup.planning.test.ts

@@ -224,6 +224,61 @@ describe("setup workflow", () => {
 });
 
 describe("setup client selection", () => {
+  it.each([
+    {
+      name: "explicit clients before detected clients without duplicates",
+      selection: { clientIds: ["cursor", "codex", "cursor"] },
+      expected: ["cursor", "codex"],
+    },
+    {
+      name: "readiness-scope clients when no explicit list is supplied",
+      selection: { readinessScope: { clients: ["cursor"] } },
+      expected: ["cursor", "codex"],
+    },
+    {
+      name: "only detected clients without an explicit selection",
+      selection: {},
+      expected: ["codex"],
+    },
+  ])("configures and verifies $name", async ({ selection, expected }) => {
+    const host = new FakeSetupHost();
+    host.hopper = "/Applications/Hopper";
+    host.skill = "unchanged";
+    const codex = {
+      name: "codex",
+      configPath: "/codex.toml",
+      format: "toml" as const,
+    };
+    host.clients = [codex];
+    host.availableClients = [
+      { name: "cursor", configPath: "/cursor.json" },
+      codex,
+    ];
+
+    const result = await runSetup(
+      { ...options(true), ...selection, allDetectedClients: true },
+      host,
+    );
+
+    expect(result.status).toBe("ready");
+    expect(result.plannedActions.map(({ id }) => id)).toEqual(
+      expected.map((id) => `configure_client:${id}`),
+    );
+    expect(result.appliedActions).toEqual(
+      expected.map((id) => `configured_${id}`),
+    );
+    expect(host.configurations).toBe(expected.length);
+    expect(host.skillDirectoryValidations).toEqual([expected, expected]);
+    expect(host.doctorScopes.at(-1)).toEqual({
+      clients: expected,
+      providers: [],
+      skill: true,
+      skillClients: expected,
+    });
+    expect(host.hopperInstalls).toBe(0);
+    expect(host.skillInstalls).toBe(0);
+  });
+
   it.each([true, false])(
     "handles repeated clients once in first-selected order (dry run: %s)",
     async (dryRun) => {

src/application/Setup.ts

@@ -128,13 +128,13 @@ const runSetupWorkflow = async (
     options.installSkill === undefined;
   const interactiveSelection =
     confirm !== undefined && !options.structured && clientSelectionAllowed;
+  const requestedClientIds =
+    options.clientIds ?? options.readinessScope?.clients;
   let selectedClientIds = [
     ...new Set(
       options.allDetectedClients
-        ? discovery.detectedClientIds
-        : (options.clientIds ??
-            options.readinessScope?.clients ??
-            discovery.defaultClientIds),
+        ? [...(requestedClientIds ?? []), ...discovery.detectedClientIds]
+        : (requestedClientIds ?? discovery.defaultClientIds),
     ),
   ];
   let selectedSkillClientIds =

src/cli/setupCommands.ts

@@ -255,7 +255,6 @@ const setupRunOptions = (
     options.skillClient.length > 0;
   const hasSelectedClients =
     hasExplicitScope &&
-    !options.allDetected &&
     (options.client.length > 0 || options.skillClient.length > 0);
   return {
     approved: options.yes && !options.dryRun,
@@ -272,7 +271,7 @@ const setupRunOptions = (
       ? { installSkill: options.skill ?? agentIntegrationSelected }
       : {}),
     ...(interactive ? { onProgress: renderSetupProgress } : {}),
-    ...(hasSelectedClients
+    ...(hasSelectedClients && !options.allDetected
       ? {
           readinessScope: {
             clients: options.client,

tests/acceptance/setup/repeatedClientSetup.test.ts

@@ -81,6 +81,84 @@ cliTest(
   },
 );
 
+cliTest(
+  "combines explicitly selected undetected clients with all detected clients",
+  async ({ cli }) => {
+    const home = await createTestTempDirectory(
+      "rea-explicit-detected-clients-",
+    );
+    await mkdir(join(home, ".codex"));
+    const run = (selection: readonly string[], mode: string) =>
+      cli.run({
+        arguments: ["setup", ...selection, "--skill=false", "--json", mode],
+        cwd: home,
+        environment: {
+          HOME: home,
+          USERPROFILE: home,
+          TMPDIR: home,
+          XDG_CONFIG_HOME: join(home, ".config"),
+          XDG_DATA_HOME: join(home, ".local", "share"),
+          XDG_CACHE_HOME: join(home, ".cache"),
+          XDG_STATE_HOME: join(home, ".local", "state"),
+        },
+      });
+    const configurations = [
+      join(home, ".cursor", "mcp.json"),
+      join(home, ".codex", "config.toml"),
+    ];
+    const combined = [
+      "--client",
+      "cursor",
+      "--client",
+      "cursor",
+      "--all-detected",
+    ];
+    for (const [selection, expected] of [
+      [["--client", "cursor"], ["cursor"]],
+      [["--all-detected"], ["codex"]],
+      [combined, ["cursor", "codex"]],
+    ] as const) {
+      const preview = await run(selection, "--dry-run");
+      expect(preview.exitCode, preview.stdout).toBe(0);
+      expect(preview.json).toMatchObject({
+        status: "planned",
+        plannedActions: expected.map((id) => ({
+          id: `configure_client:${id}`,
+        })),
+        appliedActions: [],
+      });
+      for (const path of configurations)
+        await expect(access(path)).rejects.toMatchObject({ code: "ENOENT" });
+    }
+
+    const applied = await run(combined, "--yes");
+    expect(applied.exitCode, applied.stdout).toBe(0);
+    expect(applied.json).toMatchObject({
+      status: "ready",
+      plannedActions: [
+        { id: "configure_client:cursor" },
+        { id: "configure_client:codex" },
+      ],
+      appliedActions: ["configured_cursor", "configured_codex"],
+      clients: {
+        cursor: { status: "configured" },
+        codex: { status: "configured" },
+      },
+      doctor: {
+        healthy: true,
+        scope: {
+          mode: "explicit",
+          clients: ["codex", "cursor"],
+          providers: [],
+          skill: false,
+        },
+      },
+    });
+    for (const path of configurations)
+      expect(await readFile(path, "utf8")).toContain("rea");
+  },
+);
+
 cliTest(
   "uninstall preserves user edits made after an earlier setup",
   async ({ cli }) => {

tests/boundary/filesystem/jsonOutputOwnership.test.ts

@@ -200,12 +200,14 @@ it("preserves cancellation when staging cleanup also fails", async () => {
   const root = await createTestTempDirectory("rea-json-cancel-");
   const resources = new ArtifactResourceScope();
   const controller = new AbortController();
+  let unexpected: string | undefined;
   try {
     function* cancelledParts(): Generator<string> {
       const directories = requireStages(root);
       if (directories.length !== 1)
         throw new Error("Expected one staging directory");
-      writeFileSync(join(root, directories[0]!, "unexpected"), "retain this");
+      unexpected = join(root, directories[0]!, "unexpected");
+      writeFileSync(unexpected, "retain this");
       controller.abort();
       yield "must not publish";
     }
@@ -223,12 +225,16 @@ it("preserves cancellation when staging cleanup also fails", async () => {
     // directory is reported as cleanup state, not as the cancellation cause.
     expect(result.error.cause).toBe(controller.signal.reason);
     expect(result.error.cleanupPrimary).toBeInstanceOf(AnalysisCancelledError);
-    expect(result.error.cleanupResources).toEqual([
-      expect.stringContaining(".rea-write-"),
-    ]);
+    expect(result.error.cleanupPrimary).toMatchObject({
+      operation: "export_evidence_bundle",
+    });
+    expect(result.error.cleanupPrimary?.cause).toBe(controller.signal.reason);
+    const stages = requireStages(root);
+    expect(stages).toHaveLength(1);
+    expect(result.error.cleanupResources).toEqual([join(root, stages[0]!)]);
+    expect(await fs.readFile(unexpected!, "utf8")).toBe("retain this");
     await expect(fs.access(join(root, "cancelled.json"))).rejects.toThrow();
-    const stage = requireStages(root)[0];
-    if (stage !== undefined) await fs.rm(join(root, stage, "unexpected"));
+    await fs.rm(unexpected!);
     await resources.close();
     expect(requireStages(root)).toEqual([]);
     expect(result.error.cleanupIncomplete).toBe(false);
@@ -239,7 +245,9 @@ it("preserves cancellation when staging cleanup also fails", async () => {
     });
     expect(released.details).not.toHaveProperty("resources");
   } finally {
+    if (unexpected !== undefined) await fs.rm(unexpected, { force: true });
     await resources.close();
+    expect(requireStages(root)).toEqual([]);
     await rm(root, { recursive: true, force: true });
   }
 });