From c012015ae812242eebac2527f819a458cb35aa8c Mon Sep 17 00:00:00 2001 From: Afloat <132809511+Afloat16@users.noreply.github.com> Date: Fri, 16 Oct 2026 23:59:15 +0800 Subject: [PATCH] fix(setup): deduplicate selected clients before planning (#3919) Normalize client identities once in the shared workflow while preserving first-selected order. Keep planning, readiness, default skill selection and configuration application on the same unique list. Cover duplicate selections in direct workflow tests and an isolated compiled CLI preview/apply/rerun journey. Refs morluto/rea#3847. --- src/application/Setup.planning.test.ts | 57 +++++++++++++ src/application/Setup.ts | 14 ++-- .../setup/repeatedClientSetup.test.ts | 81 +++++++++++++++++++ 3 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 tests/acceptance/setup/repeatedClientSetup.test.ts diff --git a/src/application/Setup.planning.test.ts b/src/application/Setup.planning.test.ts index 22e9310e4e..51917ba6a8 100644 --- a/src/application/Setup.planning.test.ts +++ b/src/application/Setup.planning.test.ts @@ -223,6 +223,63 @@ describe("setup workflow", () => { }); }); +describe("setup client selection", () => { + it.each([true, false])( + "handles repeated clients once in first-selected order (dry run: %s)", + async (dryRun) => { + const host = new FakeSetupHost(); + host.hopper = "/Applications/Hopper"; + host.skill = "unchanged"; + host.clients = [ + { name: "cursor", configPath: "/cursor.json" }, + { name: "codex", configPath: "/codex.toml", format: "toml" }, + ]; + const inspected: string[] = []; + const configured: string[] = []; + const inspect = host.inspectClientConfiguration; + const configure = host.configureClient; + host.inspectClientConfiguration = (client) => { + inspected.push(client.name); + return inspect(client); + }; + host.configureClient = (client, environment, command) => { + configured.push(client.name); + return configure(client, environment, command); + }; + + const result = await runSetup( + { + ...options(true), + clientIds: ["codex", "cursor", "codex", "cursor"], + installSkill: false, + dryRun, + }, + host, + ); + + expect(result.status).toBe(dryRun ? "planned" : "ready"); + expect(result.plannedActions.map(({ id }) => id)).toEqual([ + "configure_client:codex", + "configure_client:cursor", + ]); + expect(inspected).toEqual(["codex", "cursor"]); + expect(configured).toEqual(dryRun ? [] : ["codex", "cursor"]); + expect(result.appliedActions).toEqual( + dryRun ? [] : ["configured_codex", "configured_cursor"], + ); + if (!dryRun) { + expect(result.clients).toEqual({ + codex: { status: "configured" }, + cursor: { status: "configured" }, + }); + expect(host.doctorScopes.at(-1)?.clients).toEqual(["codex", "cursor"]); + } + expect(host.hopperInstalls).toBe(0); + expect(host.skillInstalls).toBe(0); + }, + ); +}); + describe("setup plan application", () => { it("returns a complete plan without mutation", async () => { const host = new FakeSetupHost(); diff --git a/src/application/Setup.ts b/src/application/Setup.ts index bec8711e6d..117238fa2a 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -128,11 +128,15 @@ const runSetupWorkflow = async ( options.installSkill === undefined; const interactiveSelection = confirm !== undefined && !options.structured && clientSelectionAllowed; - let selectedClientIds = options.allDetectedClients - ? discovery.detectedClientIds - : (options.clientIds ?? - options.readinessScope?.clients ?? - discovery.defaultClientIds); + let selectedClientIds = [ + ...new Set( + options.allDetectedClients + ? discovery.detectedClientIds + : (options.clientIds ?? + options.readinessScope?.clients ?? + discovery.defaultClientIds), + ), + ]; let selectedSkillClientIds = options.skillClientIds ?? options.readinessScope?.skillClients ?? diff --git a/tests/acceptance/setup/repeatedClientSetup.test.ts b/tests/acceptance/setup/repeatedClientSetup.test.ts new file mode 100644 index 0000000000..bacfb6e580 --- /dev/null +++ b/tests/acceptance/setup/repeatedClientSetup.test.ts @@ -0,0 +1,81 @@ +import { access, readFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect } from "vitest"; + +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest( + "repeated client flags plan and configure each selected client once", + async ({ cli }) => { + const directory = await createTestTempDirectory("rea-repeated-clients-"); + const arguments_ = [ + "setup", + "--client", + "cursor", + "--client", + "codex", + "--client", + "cursor", + "--skill=false", + "--json", + ]; + const run = (mode: string) => + cli.run({ + arguments: [...arguments_, mode], + cwd: directory, + environment: { + HOME: directory, + USERPROFILE: directory, + TMPDIR: directory, + }, + }); + const configurations = [ + join(directory, ".cursor/mcp.json"), + join(directory, ".codex/config.toml"), + ]; + + const preview = await run("--dry-run"); + expect(preview.exitCode, preview.stdout).toBe(0); + expect(preview.json).toMatchObject({ + status: "planned", + plannedActions: [ + { id: "configure_client:cursor" }, + { id: "configure_client:codex" }, + ], + appliedActions: [], + }); + for (const path of configurations) + await expect(access(path)).rejects.toMatchObject({ code: "ENOENT" }); + + const applied = await run("--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" }, + }, + }); + const configured = await Promise.all( + configurations.map((path) => readFile(path, "utf8")), + ); + + const repeated = await run("--yes"); + expect(repeated.exitCode, repeated.stdout).toBe(0); + expect(repeated.json).toMatchObject({ + status: "ready", + plannedActions: [], + appliedActions: [], + }); + expect( + await Promise.all(configurations.map((path) => readFile(path, "utf8"))), + ).toEqual(configured); + }, +);