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.
This commit is contained in:
Afloat authored and GitHub committed 2026-10-16 23:59:15 +08:00
1 parent d62d3082c2
commit c012015ae8
3 files changed
+147 -5

No files matched your search

+57
View File
@@ -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();
+9 -5
View File
@@ -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 ??
@@ -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);
},
);