diff --git a/.changeset/codex-install-picker-row.md b/.changeset/codex-install-picker-row.md new file mode 100644 index 00000000..6c02032a --- /dev/null +++ b/.changeset/codex-install-picker-row.md @@ -0,0 +1,29 @@ +--- +"@taskless/cli": patch +--- + +Name Codex in the install picker, so Codex users can see that it supports them. + +Codex has always been detected and installed into `.agents/`. The tool-selection +step just never said so: it read `Claude Code / Cursor / OpenCode / Agent +Skills`, and the entry that serves Codex is the one whose label only makes sense +if you already know `AGENTS.md` is the file Codex reads. A GPT-centric founder +looked at that list and concluded we did not support GPT. He was wrong, and the +list is why he thought it. Somebody who believes their harness is unsupported +does not file a bug, they leave. + +The picker now offers a `Codex` row alongside the generic `Agent Skills` row, +both pointing at `.agents/`. Two rows for one directory is deliberate: people +scan a list for the name of the tool they use, and `Agent Skills` still has to +be there for anyone on a harness the catalog does not enumerate. Neither label +is redundant, so neither one goes. + +That makes the catalog a list of rows rather than a list of directories, which +was an assumption the code held in three places. A single `.agents/` selection +used to match one row; it now matches two, and would have been pre-checked +twice, planned twice, written twice, and reported twice in the install summary. +The pre-checked set, `detectSelectedDirectories`, and the install plan all +collapse the catalog on `dir` first. The dedupe is on the directory rather than +on the Codex row specifically, so the next pair of rows that share a +destination inherits it instead of reintroducing the bug. Ticking either row +selects `.agents/` once, and ticking both is the same install as ticking one. diff --git a/openspec/changes/archive/2026-08-29-codex-install-picker-row/proposal.md b/openspec/changes/archive/2026-08-29-codex-install-picker-row/proposal.md new file mode 100644 index 00000000..c91d17d0 --- /dev/null +++ b/openspec/changes/archive/2026-08-29-codex-install-picker-row/proposal.md @@ -0,0 +1,50 @@ +## Why + +Codex is detected, routed to `.agents/`, and fully supported. The word +"Codex" never appears in the list the user actually reads. + +The install wizard keeps two lists. `TOOLS` drives detection and knows about +Codex. `SHIM_TARGETS` drives the "which tools do you want to enable Taskless +for?" multiselect and offers `Claude Code / Cursor / OpenCode / Agent Skills`. +A Codex user reads that list and concludes we do not support their harness, +because the support arrives under a label that only makes sense once you +already know `AGENTS.md` is the thing Codex reads. Someone who believes their +harness is unsupported does not file a bug, they leave. + +## What Changes + +- **The picker gains a `Codex` row pointing at `.agents/`.** Two rows now name + the same directory. That is the point: people scan the list for the name of + the tool they use. `Agent Skills` stays, because it is the entry that serves + anyone on a harness the catalog does not enumerate. +- **The catalog becomes a list of rows, not a list of directories.** Every + consumer that turns rows into directories or into install targets now goes + through a single deduplicating helper, so a shared directory is pre-checked + once, planned once, written once, and reported once. The dedupe is on `dir`, + not on the Codex row, so a future pair of rows sharing a destination + inherits the invariant. +- **The generic hint follows the generic row.** `generic agent skills` belongs + to the `Agent Skills` row; the `Codex` row hints `detected` or + `not detected` like any other named harness. + +**Delivery is a single PR.** One catalog row, one helper, three call sites, +tests, and a spec delta. No unit of this is meaningful on its own. + +## Capabilities + +### Modified Capabilities + +- `cli-init`: the tool-selection step names Codex, and a directory offered by + more than one row installs exactly once. + +## Impact + +- **Modified**: `packages/cli/src/install/install.ts` (the `Codex` row, + `uniqueShimTargets`, `detectSelectedDirectories`, `buildInstallPlan`), + `packages/cli/src/wizard/steps/locations.ts` (pre-checked set and hints), + `packages/cli/test/install.test.ts`, + `packages/cli/test/wizard-steps.test.ts`. +- **Unchanged**: detection, the canonical store, the stub format, and the + install manifest, which has always been keyed by directory. + +**Tracking:** taskless/cli#204 diff --git a/openspec/changes/archive/2026-08-29-codex-install-picker-row/specs/cli-init/spec.md b/openspec/changes/archive/2026-08-29-codex-install-picker-row/specs/cli-init/spec.md new file mode 100644 index 00000000..ef189a43 --- /dev/null +++ b/openspec/changes/archive/2026-08-29-codex-install-picker-row/specs/cli-init/spec.md @@ -0,0 +1,65 @@ +## MODIFIED Requirements + +### Requirement: Wizard prompts the user to choose install locations + +The wizard's location step SHALL be presented as a tool-selection step: "which tools do you want to enable Taskless for?". It SHALL offer a fixed catalog of rows: `Claude Code` (`.claude/`), `Codex` (`.agents/`), `Cursor` (`.cursor/`), `OpenCode` (`.opencode/`), and `Agent Skills` (`.agents/`). A row names a tool the user scans for, so more than one row MAY offer the same directory: Codex reads `.agents/`, and a user who arrived from Codex SHALL NOT have to know that before recognising their harness in the list. The generic `Agent Skills` row SHALL remain, since it serves harnesses this catalog does not enumerate. + +Every list of directories derived from the catalog SHALL name each directory at most once, however many rows offer it. The pre-checked set SHALL be the union of (a) every directory recorded as a target in the install manifest (`install.targets`) that matches an offered row, and (b) every detected tool's install directory, deduplicated by directory. When the manifest records no targets AND no tools are detected, `.agents/` SHALL be pre-checked as the first-run default. The canonical `.taskless/` store SHALL NOT appear as a selectable entry and SHALL NOT be pre-checked — it is always written and is never a manifest tool-directory target. + +Each offered entry SHALL carry an origin hint: `installed` when the entry's directory is recorded in the install manifest; otherwise `detected` when the entry's tool is detected on the filesystem; otherwise `not detected`. The generic `Agent Skills` row MAY instead carry a hint describing it as the generic agent-skills location; a row naming a specific harness SHALL NOT carry that hint. The `installed` hint SHALL take precedence over `detected` when both apply. + +Unchecking a pre-checked, manifest-recorded entry SHALL cause the resulting install plan to omit that target, so the existing manifest-diff removal path removes Taskless's reference stubs from that directory. The at-least-one-tool selection rule is unchanged: the wizard SHALL require at least one checked entry. + +Each selected directory SHALL produce exactly one `reference` stub target, even when several offered rows name it; the resulting install plan always contains the single `taskless` skill (and, for `.claude/` and `.cursor/`, the `tskl` command). A directory offered by more than one row SHALL be reported in the install summary under a single label. The function that maps detected tools and manifest targets to multiselect choices SHALL be pure — it SHALL receive both the detected tools and the manifest target list as arguments and SHALL perform no filesystem access — so the mapping is unit-testable. + +#### Scenario: Codex is named in the tool list + +- **WHEN** the wizard renders the tool-selection multiselect +- **THEN** a `Codex` entry SHALL be offered for `.agents/` +- **AND** a separate generic `Agent Skills` entry SHALL also be offered for `.agents/` + +#### Scenario: A directory offered by two rows is pre-checked once + +- **WHEN** the wizard reaches the tool-selection step and Codex is detected +- **THEN** `.agents/` SHALL appear exactly once in the pre-checked set + +#### Scenario: A directory offered by two rows installs once + +- **WHEN** the user's selection includes `.agents/`, whichever of its rows was checked +- **THEN** the install plan SHALL contain exactly one `.agents/` target +- **AND** the `.agents/` skill stub SHALL be written once + +#### Scenario: Detected tools are pre-checked + +- **WHEN** the wizard reaches the tool-selection step and `.claude/` is detected +- **THEN** `.claude/` SHALL be pre-checked in the multiselect +- **AND** `.claude/` SHALL carry the `detected` hint when it is not recorded in the install manifest + +#### Scenario: Manifest-recorded locations are pre-checked + +- **WHEN** the wizard reaches the tool-selection step and the install manifest records `.agents/` as a target +- **THEN** `.agents/` SHALL be pre-checked in the multiselect +- **AND** `.agents/` SHALL carry the `installed` hint + +#### Scenario: Installed hint takes precedence over detected + +- **WHEN** the wizard reaches the tool-selection step and `.claude/` is both detected on the filesystem and recorded in the install manifest +- **THEN** `.claude/` SHALL be pre-checked +- **AND** `.claude/` SHALL carry the `installed` hint, not the `detected` hint + +#### Scenario: Unchecking an installed location removes its stubs + +- **WHEN** the install manifest records `.claude/` and `.agents/` as targets and the user unchecks `.claude/` while leaving `.agents/` checked +- **THEN** the resulting install plan SHALL omit the `.claude/` target +- **AND** the wizard summary SHALL list the `.claude/` reference stubs as removals + +#### Scenario: Agents is the default when nothing is detected or installed + +- **WHEN** the wizard reaches the tool-selection step, no tools are detected, and the install manifest records no tool-directory targets +- **THEN** `.agents/` SHALL be pre-checked + +#### Scenario: Canonical store is not a selectable entry + +- **WHEN** the wizard renders the tool-selection multiselect +- **THEN** `.taskless/` SHALL NOT appear as a selectable option +- **AND** `.taskless/` SHALL NOT be pre-checked even though the manifest records it as a target diff --git a/openspec/changes/archive/2026-08-29-codex-install-picker-row/tasks.md b/openspec/changes/archive/2026-08-29-codex-install-picker-row/tasks.md new file mode 100644 index 00000000..ea97ef3c --- /dev/null +++ b/openspec/changes/archive/2026-08-29-codex-install-picker-row/tasks.md @@ -0,0 +1,31 @@ +Delivery shape: **single PR**. One catalog row, one deduplicating helper, its call sites, tests, and a spec delta. Landing the row without the dedupe would install `.agents/` twice, so the units are only correct together. + +## 1. Catalog + +- [x] 1.1 Add the `Codex` row on `.agents/`, ordered so named harnesses come before the generic `Agent Skills` fallback +- [x] 1.2 Add `uniqueShimTargets`, collapsing the catalog to one entry per directory in catalog order +- [x] 1.3 Say in the code why two rows share a directory, so it is not deleted as a duplicate later +- [x] 1.4 Update the `ShimTarget` doc comment, which read as promising one row per directory + +## 2. Call sites + +- [x] 2.1 `detectSelectedDirectories` maps the deduplicated catalog +- [x] 2.2 `buildInstallPlan` iterates the deduplicated catalog +- [x] 2.3 The wizard's pre-checked set is deduplicated; its options stay one per row +- [x] 2.4 The `generic agent skills` hint follows the generic row rather than the directory + +## 3. Tests + +- [x] 3.1 The picker lists Codex and Agent Skills as separate rows +- [x] 3.2 `detectSelectedDirectories` returns `.agents` once when Codex is detected +- [x] 3.3 `locationChoices` pre-checks `.agents` once when Codex is detected +- [x] 3.4 A selection naming `.agents` produces exactly one plan target, labelled `Agent Skills` +- [x] 3.5 An install of that plan writes each `.agents` skill stub once + +## 4. Verification + +- [x] 4.1 Confirm the tests fail with the dedupe removed +- [x] 4.2 Exercise the built CLI against a scratch directory outside the repo +- [x] 4.3 `pnpm build`, `pnpm typecheck`, `pnpm lint`, `pnpm test`, `pnpm cli check` +- [x] 4.4 `pnpm openspec validate --all --strict` +- [x] 4.5 Changeset diff --git a/openspec/specs/cli-init/spec.md b/openspec/specs/cli-init/spec.md index 724cbf4d..dcba767b 100644 --- a/openspec/specs/cli-init/spec.md +++ b/openspec/specs/cli-init/spec.md @@ -380,13 +380,32 @@ The wizard SHALL begin by rendering an ASCII rendition of the Taskless wordmark ### Requirement: Wizard prompts the user to choose install locations -The wizard's location step SHALL be presented as a tool-selection step: "which tools do you want to enable Taskless for?". It SHALL offer a fixed multiselect of `.claude/`, `.cursor/`, `.opencode/`, and `.agents/`. The pre-checked set SHALL be the union of (a) every directory recorded as a target in the install manifest (`install.targets`) that matches one of the four offered entries, and (b) every detected tool's install directory. When the manifest records no targets AND no tools are detected, `.agents/` SHALL be pre-checked as the first-run default. The canonical `.taskless/` store SHALL NOT appear as a selectable entry and SHALL NOT be pre-checked — it is always written and is never a manifest tool-directory target. +The wizard's location step SHALL be presented as a tool-selection step: "which tools do you want to enable Taskless for?". It SHALL offer a fixed catalog of rows: `Claude Code` (`.claude/`), `Codex` (`.agents/`), `Cursor` (`.cursor/`), `OpenCode` (`.opencode/`), and `Agent Skills` (`.agents/`). A row names a tool the user scans for, so more than one row MAY offer the same directory: Codex reads `.agents/`, and a user who arrived from Codex SHALL NOT have to know that before recognising their harness in the list. The generic `Agent Skills` row SHALL remain, since it serves harnesses this catalog does not enumerate. -Each offered entry SHALL carry an origin hint: `installed` when the entry's directory is recorded in the install manifest; otherwise `detected` when the entry's tool is detected on the filesystem; otherwise `not detected` (the `.agents/` first-run default MAY instead carry a hint describing it as the generic agent-skills location). The `installed` hint SHALL take precedence over `detected` when both apply. +Every list of directories derived from the catalog SHALL name each directory at most once, however many rows offer it. The pre-checked set SHALL be the union of (a) every directory recorded as a target in the install manifest (`install.targets`) that matches an offered row, and (b) every detected tool's install directory, deduplicated by directory. When the manifest records no targets AND no tools are detected, `.agents/` SHALL be pre-checked as the first-run default. The canonical `.taskless/` store SHALL NOT appear as a selectable entry and SHALL NOT be pre-checked — it is always written and is never a manifest tool-directory target. + +Each offered entry SHALL carry an origin hint: `installed` when the entry's directory is recorded in the install manifest; otherwise `detected` when the entry's tool is detected on the filesystem; otherwise `not detected`. The generic `Agent Skills` row MAY instead carry a hint describing it as the generic agent-skills location; a row naming a specific harness SHALL NOT carry that hint. The `installed` hint SHALL take precedence over `detected` when both apply. Unchecking a pre-checked, manifest-recorded entry SHALL cause the resulting install plan to omit that target, so the existing manifest-diff removal path removes Taskless's reference stubs from that directory. The at-least-one-tool selection rule is unchanged: the wizard SHALL require at least one checked entry. -Each checked entry SHALL produce one `reference` stub target; the resulting install plan always contains the single `taskless` skill (and, for `.claude/` and `.cursor/`, the `tskl` command). The function that maps detected tools and manifest targets to multiselect choices SHALL be pure — it SHALL receive both the detected tools and the manifest target list as arguments and SHALL perform no filesystem access — so the mapping is unit-testable. +Each selected directory SHALL produce exactly one `reference` stub target, even when several offered rows name it; the resulting install plan always contains the single `taskless` skill (and, for `.claude/` and `.cursor/`, the `tskl` command). A directory offered by more than one row SHALL be reported in the install summary under a single label. The function that maps detected tools and manifest targets to multiselect choices SHALL be pure — it SHALL receive both the detected tools and the manifest target list as arguments and SHALL perform no filesystem access — so the mapping is unit-testable. + +#### Scenario: Codex is named in the tool list + +- **WHEN** the wizard renders the tool-selection multiselect +- **THEN** a `Codex` entry SHALL be offered for `.agents/` +- **AND** a separate generic `Agent Skills` entry SHALL also be offered for `.agents/` + +#### Scenario: A directory offered by two rows is pre-checked once + +- **WHEN** the wizard reaches the tool-selection step and Codex is detected +- **THEN** `.agents/` SHALL appear exactly once in the pre-checked set + +#### Scenario: A directory offered by two rows installs once + +- **WHEN** the user's selection includes `.agents/`, whichever of its rows was checked +- **THEN** the install plan SHALL contain exactly one `.agents/` target +- **AND** the `.agents/` skill stub SHALL be written once #### Scenario: Detected tools are pre-checked diff --git a/packages/cli/src/install/install.ts b/packages/cli/src/install/install.ts index 1eda8aee..9c3915fa 100644 --- a/packages/cli/src/install/install.ts +++ b/packages/cli/src/install/install.ts @@ -126,7 +126,12 @@ export const TOOLS: ToolDescriptor[] = [ /** * A selectable stub destination. The wizard offers this fixed catalog as the * "which tools do you want to enable Taskless for?" multiselect; every entry - * is a peer — no directory is special-cased or routed onto another. + * is a peer, no directory is special-cased or routed onto another. + * + * The catalog is a list of *rows the user reads*, not a list of directories: + * more than one row may name the same `dir`. Anything downstream of the + * picker works in directories and must therefore collapse the rows first, + * which is what {@link uniqueShimTargets} is for. */ export interface ShimTarget { /** Directory the stub is written into, relative to the project root. */ @@ -135,15 +140,76 @@ export interface ShimTarget { label: string; /** Whether this directory receives the `tskl` command stub. */ commands: boolean; + /** + * Marks the catch-all row: the one offered to anyone whose harness this + * catalog does not name. At most one row carries it. + * + * It is a declared field rather than something derived, because every way + * of inferring it is a coincidence. "The last row for the directory" is + * declaration order, and "the object `uniqueShimTargets` happened to keep" + * is reference identity, so reordering the catalog or making the dedupe + * return copies would silently move the hint to a named harness with no + * type error anywhere. + */ + generic?: boolean; } +/** + * Codex and Agent Skills deliberately share `.agents/`. Both rows stay. + * People scan this list for the name of the tool they use, and a Codex user + * who sees only "Agent Skills" concludes Taskless does not support their + * harness, which is how we lost one. "Agent Skills" still serves anyone on a + * harness this catalog does not enumerate. Do not merge the two rows into one + * label: the duplicate directory is the point, not an oversight. + */ export const SHIM_TARGETS: readonly ShimTarget[] = [ { dir: ".claude", label: "Claude Code", commands: true }, + { dir: ".agents", label: "Codex", commands: false }, { dir: ".cursor", label: "Cursor", commands: true }, { dir: ".opencode", label: "OpenCode", commands: false }, - { dir: ".agents", label: "Agent Skills", commands: false }, + { dir: ".agents", label: "Agent Skills", commands: false, generic: true }, ]; +/** + * The catalog collapsed to one entry per directory, in catalog order. + * + * Every consumer that turns rows into directories or into install targets + * goes through here. Without it, a single `.agents/` selection matches both + * `.agents/` rows and the plan writes, reports, and records that directory + * twice. Deduping on `dir` (rather than special-casing Codex) keeps that + * one-entry-per-directory invariant true for any future pair of rows sharing + * a destination. + * + * What it does NOT do is reconcile the rows: the surviving entry is one whole + * row, so EVERY field comes from the last row for that directory, not just + * `label`. Today both `.agents/` rows agree on `commands`, so nothing is + * silently chosen. A future pair that disagreed on it would be resolved by + * catalog order rather than by anyone deciding, which is a reason to keep + * rows sharing a directory identical apart from `label` and `generic`. + * + * The LAST row for a directory wins both its position and its value. The + * catalog is ordered named-harness first, generic-fallback last, so `.agents/` + * is summarised as "Agent Skills" and sorts last: the selection carries only a + * directory, never which row was ticked, and the generic label is the one that + * is accurate either way. + * + * The `delete` before the `set` is what buys the position half. `Map.set` + * alone overwrites the value but keeps the FIRST insertion's position, which + * would leave `.agents/` holding the `Codex` row's slot (second) while + * displaying the `Agent Skills` label. That reorders writes and summary lines + * in every multi-tool install, for a change that is only supposed to add a + * label, and it quietly contradicts the generic-fallback-last ordering this + * comment relies on. + */ +export function uniqueShimTargets(): ShimTarget[] { + const byDirectory = new Map(); + for (const shim of SHIM_TARGETS) { + byDirectory.delete(shim.dir); + byDirectory.set(shim.dir, shim); + } + return [...byDirectory.values()]; +} + /** Directory selected by default when no tools are detected. */ export const DEFAULT_SHIM_DIR = ".agents"; @@ -186,7 +252,9 @@ export async function detectSelectedDirectories( const tools = await detectTools(cwd); if (tools.length === 0) return [DEFAULT_SHIM_DIR]; const directories = new Set(tools.map((t) => t.installDir)); - return SHIM_TARGETS.map((s) => s.dir).filter((d) => directories.has(d)); + return uniqueShimTargets() + .map((s) => s.dir) + .filter((d) => directories.has(d)); } // --- Embedded Skills --- @@ -270,7 +338,10 @@ export function buildInstallPlan( }); } - for (const shim of SHIM_TARGETS) { + // Iterate the deduplicated catalog: two rows share `.agents/`, and matching + // both would plan that directory twice, so it would be written twice and + // reported twice in the install summary. + for (const shim of uniqueShimTargets()) { if (!selectedDirectories.includes(shim.dir)) continue; targets.push({ dir: shim.dir, diff --git a/packages/cli/src/wizard/steps/locations.ts b/packages/cli/src/wizard/steps/locations.ts index d04bc45f..b35cc5b4 100644 --- a/packages/cli/src/wizard/steps/locations.ts +++ b/packages/cli/src/wizard/steps/locations.ts @@ -5,6 +5,7 @@ import { DEFAULT_SHIM_DIR, SHIM_TARGETS, detectTools, + uniqueShimTargets, type ToolDescriptor, } from "../../install/install"; import { readInstallState } from "../../install/state"; @@ -22,8 +23,10 @@ export interface LocationChoice { * the detected tools and the install manifest's recorded targets. Pure — no * prompt, no TTY, no filesystem access — so the mapping is unit-testable. * - * Every shim target is always offered (a peer list); the canonical - * `.taskless/` store is never an entry. The pre-checked set is the union of + * Every shim target row is always offered (a peer list); the canonical + * `.taskless/` store is never an entry. Rows may share a directory (Codex and + * Agent Skills both write `.agents/`), so options are one per row while the + * pre-checked set is one per directory. The pre-checked set is the union of * manifest-recorded shim directories and detected tools' directories, so a * location Taskless already installed into shows checked and can be * unchecked. `.agents/` is pre-checked as the default only when nothing is @@ -48,12 +51,29 @@ export function locationChoices( manifestDirectories.filter((d) => d !== CANONICAL_DIR) ); - const preChecked = SHIM_TARGETS.map((s) => s.dir).filter( - (directory) => - installedDirectories.has(directory) || detectedDirectories.has(directory) - ); + // Pre-checked entries are directories, not rows, so they come from the + // deduplicated catalog. Codex and Agent Skills both offer `.agents/`, and a + // pre-check list naming it twice would hand the multiselect a duplicate + // initial value and carry the duplicate into the install plan. + const preChecked = uniqueShimTargets() + .map((s) => s.dir) + .filter( + (directory) => + installedDirectories.has(directory) || + detectedDirectories.has(directory) + ); const initialValues = preChecked.length > 0 ? preChecked : [DEFAULT_SHIM_DIR]; + // Options, by contrast, are one per row: the whole point of a second + // `.agents/` row is that a Codex user sees the word "Codex". Rows sharing a + // directory share a value, so ticking either one selects that directory + // once, and the picker renders both as checked. + // + // The catch-all row is identified by its own `generic` flag rather than by + // object identity against the deduped catalog. That comparison was correct + // only while `uniqueShimTargets()` returned the original references AND the + // catalog happened to declare "Agent Skills" after "Codex"; either changing + // would have moved the hint onto a named harness with nothing to catch it. const options = SHIM_TARGETS.map((shim) => ({ value: shim.dir, label: `${shim.label} (${shim.dir}/)`, @@ -61,7 +81,7 @@ export function locationChoices( ? "installed" : detectedDirectories.has(shim.dir) ? "detected" - : shim.dir === DEFAULT_SHIM_DIR + : shim.generic === true ? "generic agent skills" : "not detected", })); diff --git a/packages/cli/test/install.test.ts b/packages/cli/test/install.test.ts index 932144c5..cb464f90 100644 --- a/packages/cli/test/install.test.ts +++ b/packages/cli/test/install.test.ts @@ -127,6 +127,72 @@ describe("detectSelectedDirectories", () => { await mkdir(join(cwd, ".codex"), { recursive: true }); expect(await detectSelectedDirectories(cwd)).toEqual([".agents"]); }); + + it("returns .agents once even though two catalog rows offer it", async () => { + // Codex and Agent Skills are separate rows in the picker pointing at the + // same directory. A duplicate here would flow into the install plan. + await mkdir(join(cwd, ".codex"), { recursive: true }); + await mkdir(join(cwd, ".claude"), { recursive: true }); + const directories = await detectSelectedDirectories(cwd); + expect(directories).toEqual([".claude", ".agents"]); + expect(new Set(directories).size).toBe(directories.length); + }); + + it("keeps .agents last, after a tool declared later in the catalog", async () => { + // The discriminating case for dedupe ORDER, which the test above cannot + // see: `.claude` sorts first either way. `.cursor` is declared after the + // `Codex` row and before the `Agent Skills` row, so it lands between them + // only if the LAST row for a directory wins its position. Deduping with a + // bare `Map.set` keeps the FIRST occurrence's slot, which would put + // `.agents` second and report `[".agents", ".cursor"]`. + await mkdir(join(cwd, ".codex"), { recursive: true }); + await mkdir(join(cwd, ".cursor"), { recursive: true }); + expect(await detectSelectedDirectories(cwd)).toEqual([ + ".cursor", + ".agents", + ]); + }); +}); + +describe("buildInstallPlan and duplicate shim rows", () => { + it("plans .agents exactly once when it is selected", () => { + const plan = buildInstallPlan( + [".agents"], + getEmbeddedSkills(), + getEmbeddedCommands() + ); + const agentsTargets = plan.targets.filter((t) => t.dir === ".agents"); + expect(agentsTargets).toHaveLength(1); + // The generic row supplies the shared directory's label: the selection + // carries a directory, never which of the two rows the user ticked. + expect(agentsTargets[0]!.label).toBe("Agent Skills"); + expect(agentsTargets[0]!.commands).toEqual([]); + }); + + it("plans .agents once when a caller passes it more than once", () => { + const plan = buildInstallPlan( + [".agents", ".agents", ".claude"], + getEmbeddedSkills(), + getEmbeddedCommands() + ); + expect(plan.targets.filter((t) => t.dir === ".agents")).toHaveLength(1); + expect(plan.targets.filter((t) => t.dir === ".claude")).toHaveLength(1); + }); + + it("writes .agents skill stubs once for a both-rows selection", async () => { + // What a user who ticks both `Codex` and `Agent Skills` gets: the + // multiselect collapses them onto one `.agents` value, and the plan must + // not turn that back into two targets writing the same files twice. + const plan = buildInstallPlan([".agents"], getEmbeddedSkills(), []); + const result = await applyInstallPlan(cwd, plan, { cliVersion: "0.7.0" }); + + const agentsWrites = result.writtenSkills.filter( + (entry) => entry.target === ".agents" + ); + expect(new Set(agentsWrites.map((entry) => entry.skill)).size).toBe( + agentsWrites.length + ); + }); }); describe("checkStaleness", () => { diff --git a/packages/cli/test/wizard-steps.test.ts b/packages/cli/test/wizard-steps.test.ts index 6e5b4e56..24f970c9 100644 --- a/packages/cli/test/wizard-steps.test.ts +++ b/packages/cli/test/wizard-steps.test.ts @@ -36,10 +36,39 @@ describe("locationChoices", () => { it("offers every shim target and never the canonical .taskless store", async () => { const { locationChoices } = await import("../src/wizard/steps/locations"); const values = locationChoices([]).options.map((o) => o.value); - expect(values).toEqual([".claude", ".cursor", ".opencode", ".agents"]); + expect(values).toEqual([ + ".claude", + ".agents", + ".cursor", + ".opencode", + ".agents", + ]); expect(values).not.toContain(".taskless"); }); + it("names Codex in the picker, alongside the generic .agents/ entry", async () => { + const { locationChoices } = await import("../src/wizard/steps/locations"); + const labels = locationChoices([]).options.map((o) => o.label); + // Both rows are offered on purpose: a Codex user scans for "Codex", and + // "Agent Skills" still serves harnesses the catalog does not enumerate. + expect(labels).toContain("Codex (.agents/)"); + expect(labels).toContain("Agent Skills (.agents/)"); + }); + + it("puts the generic hint on the catch-all row, not on Codex", async () => { + const { locationChoices } = await import("../src/wizard/steps/locations"); + const byLabel = new Map( + locationChoices([]).options.map((o) => [o.label, o.hint]) + ); + // Both rows write `.agents/`, so nothing about the DIRECTORY separates + // them. The hint has to follow the row's own `generic` flag. Deriving it + // from catalog order or from object identity against the deduped catalog + // put it on whichever row happened to win, which is Codex the moment the + // two rows are reordered. + expect(byLabel.get("Agent Skills (.agents/)")).toBe("generic agent skills"); + expect(byLabel.get("Codex (.agents/)")).toBe("not detected"); + }); + it("pre-checks .agents/ when no tools are detected", async () => { const { locationChoices } = await import("../src/wizard/steps/locations"); expect(locationChoices([]).initialValues).toEqual([".agents"]); @@ -103,9 +132,25 @@ describe("locationChoices", () => { // and the first-run .agents/ default applies. expect(initialValues).toEqual([".agents"]); expect(options.map((o) => o.value)).not.toContain(".taskless"); - expect(options.find((o) => o.value === ".agents")?.hint).toBe( + // Two rows share `.agents/`, so the hint is looked up by label: the + // generic hint belongs to the generic row, not to the Codex one. + expect(options.find((o) => o.label.startsWith("Agent Skills"))?.hint).toBe( "generic agent skills" ); + expect(options.find((o) => o.label.startsWith("Codex"))?.hint).toBe( + "not detected" + ); + }); + + it("pre-checks .agents/ exactly once when Codex is detected", async () => { + const { locationChoices } = await import("../src/wizard/steps/locations"); + const { TOOLS } = await import("../src/install/install"); + const codex = TOOLS.find((t) => t.name === "Codex")!; + + // Codex and Agent Skills both offer `.agents/`; the pre-checked set is + // directories, so it must name that directory once. + const { initialValues } = locationChoices([codex]); + expect(initialValues).toEqual([".agents"]); }); it("falls back to .agents/ when neither manifest nor detection has entries", async () => {