fix(cli): install agent skills in stash init, as its first step

.changeset/init-installs-agent-skills.md

@@ -0,0 +1,45 @@
+---
+'stash': patch
+---
+
+`stash init` installs the agent skills again, and does it first.
+
+Since 1.0.0-rc.4 the only callers of the skills installer were the `plan` and
+`impl` handoff steps, which `stash init` never reaches — so `stash@1.1.0`
+installed no `stash-*` skills for anyone, in any mode. The most common flow, a
+coding agent running `npx stash init --supabase` inside a project, completed
+with a green summary, a plausible-looking `.cipherstash/context.json`, and zero
+guidance: the skills sat unread in `node_modules/stash/dist/skills/` unless the
+agent thought to go digging. Fixes #923.
+
+Init now copies the per-integration skills into `.claude/skills/` (Claude Code
+detected via the `claude` binary or a `.claude/` directory) and `.codex/skills/`
+(Codex), installing to both when both are detected, and records them in
+`context.json`.
+
+It runs as init's **first** step, ahead of authentication. Installing skills
+needs no network, no credentials and no database, while authenticate,
+resolve-database and install-eql each need one and each can exit non-zero —
+so the guidance now survives a run that fails partway, which is when it is
+needed most. One behaviour change falls out of that: a run cancelled at the
+first prompt leaves the skills directory behind where previously it wrote
+nothing.
+
+Also:
+
+- **New optional `stash init --target <claude-code|codex>`** names the skills
+  destination and skips detection. Unlike `plan --target` / `impl --target` it
+  selects the destination only — `init` still performs no handoff. Existing
+  invocations are unaffected.
+- **The summary reports the outcome either way.** A run that installs nothing
+  now says so, and prints the command that will install them, instead of a
+  silent `installedSkills: []`.
+- **`--target` is validated properly on `init`, `plan` and `impl`.** A
+  trailing `--target` with no value, and `--target=`, were both treated as
+  "flag absent" — so the command silently did whatever it does with no flag at
+  all, rather than telling you the value was missing. All three commands share
+  one validator now.
+- **A later handoff no longer erases the record.** `stash plan --target
+  agents-md` installs no skill directories of its own and used to overwrite
+  `installedSkills` with an empty list, dropping skills that were on disk.
+  Deliveries are merged across hops now.

packages/cli/src/cli/registry.ts

@@ -111,6 +111,7 @@ export const registry: CommandGroup[] = [
           'init --supabase',
           'init --prisma',
           'init --region us-east-1',
+          'init --target claude-code',
         ],
         flags: [
           {
@@ -131,6 +132,12 @@ export const registry: CommandGroup[] = [
             description:
               'Region to authenticate against (e.g. us-east-1). Skips the interactive region picker. Required for non-interactive init when not already logged in.',
           },
+          {
+            name: '--target',
+            value: '<name>',
+            description:
+              'Which agent to install the bundled skills for: claude-code (.claude/skills) or codex (.codex/skills). Skips agent detection. Unlike `plan --target` and `impl --target`, this selects the skills destination only — init performs no handoff. agents-md, lovable and wizard install no skill directories (those handoffs inline the skills instead), so passing one here installs nothing.',
+          },
         ],
       },
       {

packages/cli/src/commands/impl/__tests__/how-to-proceed.test.ts

@@ -6,6 +6,7 @@ import {
   defaultChoice,
   HANDOFF_CHOICES,
   resolveTarget,
+  resolveTargetFlag,
 } from '../steps/how-to-proceed.js'
 
 function makeAgents(claudeCode: boolean, codex: boolean): AgentEnvironment {
@@ -15,6 +16,7 @@ function makeAgents(claudeCode: boolean, codex: boolean): AgentEnvironment {
       claudeDir: false,
       claudeMd: false,
       claudeSkillsDir: false,
+      codexDir: false,
       agentsMd: false,
     },
     editor: 'unknown',
@@ -100,3 +102,64 @@ describe('howToProceed — resolveTarget', () => {
     expect(resolveTarget(undefined)).toBeNull()
   })
 })
+
+/**
+ * `--target` is accepted by three commands, and the validation had been
+ * hand-copied into each — which is how `plan` and `impl` kept this bug after
+ * `init` was fixed. Testing the shared helper covers all three.
+ *
+ * The distinction that matters is "absent" versus "present but unusable".
+ * `parseArgs` files a trailing `--target` (nothing followed it) under `flags`
+ * as `true`, and `--target=` under `values` as an empty string. Testing the
+ * value for truthiness alone reads both as absent, so the command silently
+ * does whatever it does with no flag at all — for `init`, writing skills to an
+ * auto-detected directory the user had just declined by naming another.
+ */
+describe('howToProceed — resolveTargetFlag', () => {
+  it('passes a valid target through', () => {
+    expect(resolveTargetFlag({}, { target: 'codex' })).toEqual({
+      target: 'codex',
+      error: null,
+    })
+  })
+
+  it('treats an absent flag as neither a target nor an error', () => {
+    expect(resolveTargetFlag({}, {})).toEqual({ target: null, error: null })
+  })
+
+  it('rejects a trailing `--target`, which parseArgs files under flags', () => {
+    const { target, error } = resolveTargetFlag({ target: true }, {})
+    expect(target).toBeNull()
+    expect(error).toContain('needs a value')
+  })
+
+  it('rejects an empty `--target=`', () => {
+    const { target, error } = resolveTargetFlag({}, { target: '' })
+    expect(target).toBeNull()
+    expect(error).toContain('needs a value')
+  })
+
+  it('reports an unknown value differently from a missing one', () => {
+    const { target, error } = resolveTargetFlag({}, { target: 'emacs' })
+    expect(target).toBeNull()
+    expect(error).toContain('Unknown --target `emacs`')
+    expect(error).not.toContain('needs a value')
+  })
+
+  it.each([
+    ['a trailing flag', { target: true }, {}],
+    ['an empty value', {}, { target: '' }],
+    ['an unknown value', {}, { target: 'emacs' }],
+  ])('lists the valid values when rejecting %s', (_label, flags, values) => {
+    const { error } = resolveTargetFlag(flags, values)
+    for (const choice of HANDOFF_CHOICES) expect(error).toContain(choice)
+  })
+
+  // An unrelated boolean flag must not be mistaken for the target flag.
+  it('ignores other flags', () => {
+    expect(resolveTargetFlag({ yes: true }, {})).toEqual({
+      target: null,
+      error: null,
+    })
+  })
+})

packages/cli/src/commands/impl/index.ts

@@ -23,7 +23,7 @@ import { detectPackageManager, runnerCommand } from '../init/utils.js'
 import {
   HANDOFF_CHOICES,
   howToProceedStep,
-  resolveTarget,
+  resolveTargetFlag,
 } from './steps/how-to-proceed.js'
 
 function buildStateFromContext(
@@ -35,6 +35,15 @@ function buildStateFromContext(
     clientFilePath: ctx.encryptionClientPath,
     schemas: ctx.schemas,
     envKeys: ctx.envKeys,
+    // Carry the skills already on disk so the handoff's `writeArtifacts`
+    // merges into them instead of overwriting the record with just its own
+    // delivery — an `agents-md` handoff installs no directories, and used to
+    // reset `installedSkills` to `[]` on a project that had them (#923).
+    skills: {
+      installed: ctx.installedSkills ?? [],
+      inlined: ctx.inlinedSkills ?? [],
+      failed: [],
+    },
     stackInstalled: true,
     cliInstalled: true,
     eqlInstalled: true,
@@ -152,12 +161,9 @@ export async function implCommand(
 
   // Validate `--target` before printing the intro so the error sits at
   // the top of the output instead of after a half-rendered prompt frame.
-  const targetFlag = values.target
-  const target = resolveTarget(targetFlag)
-  if (targetFlag && !target) {
-    p.log.error(
-      `Unknown --target \`${targetFlag}\`. Valid values: ${HANDOFF_CHOICES.join(', ')}.`,
-    )
+  const { target, error: targetError } = resolveTargetFlag(flags, values)
+  if (targetError) {
+    p.log.error(targetError)
     process.exit(1)
   }
 

packages/cli/src/commands/impl/steps/how-to-proceed.ts

@@ -40,6 +40,44 @@ export function resolveTarget(
     : null
 }
 
+/**
+ * Resolve a `--target` from raw parsed argv, distinguishing "absent" from
+ * "present but unusable".
+ *
+ * The distinction is the whole point, and it needs both halves of `parseArgs`
+ * to see. A trailing `--target` (nothing followed it) lands in `flags` as
+ * `true`; `--target=` lands in `values` as an empty string. Each command used
+ * to test `values.target` for truthiness alone, so both forms read as "flag
+ * absent" and fell through to whatever the no-flag path does — for `init`,
+ * writing skills to an auto-detected directory the user had just declined to
+ * accept by naming a different one.
+ *
+ * Returns the validated target, or an `error` message the caller prints
+ * before exiting. Exit MECHANICS stay with the caller: `init` unwinds through
+ * `CliExit` so telemetry flushes, while `plan` and `impl` call `process.exit`
+ * directly.
+ *
+ * Lives here beside {@link HANDOFF_CHOICES} and {@link resolveTarget} because
+ * three commands accept this flag and the validation had already been
+ * hand-copied into each — which is exactly how two of them kept the bug after
+ * the third was fixed.
+ */
+export function resolveTargetFlag(
+  flags: Record<string, boolean>,
+  values: Record<string, string>,
+): { target: HandoffChoice | null; error: string | null } {
+  const provided = flags.target === true || Object.hasOwn(values, 'target')
+  const raw = values.target
+  const target = resolveTarget(raw)
+  if (!provided || target) return { target, error: null }
+  return {
+    target: null,
+    error: raw
+      ? `Unknown --target \`${raw}\`. Valid values: ${HANDOFF_CHOICES.join(', ')}.`
+      : `\`--target\` needs a value. Valid values: ${HANDOFF_CHOICES.join(', ')}.`,
+  }
+}
+
 /**
  * Pick the default option in the menu.
  *

packages/cli/src/commands/init/__tests__/init-command.test.ts

@@ -17,11 +17,24 @@ const authRun = vi.hoisted(() =>
   vi.fn(async (state: InitState, _provider: InitProvider) => state),
 )
 const passthrough = { run: async (s: InitState) => s }
+// Controllable so the skills-summary tests can vary what the first step
+// delivered. Mocked like every other step — the REAL one copies files into
+// `process.cwd()`, which in this suite is the package root, so leaving it
+// unmocked writes `.claude/skills/` into the repo on every test run.
+const skillsRun = vi.hoisted(() =>
+  vi.fn(async (s: InitState) => ({
+    ...s,
+    skills: { installed: ['stash-cli'], inlined: [], failed: [] },
+  })),
+)
 // Controllable so the honest-summary tests can vary whether EQL installed.
 const eqlRun = vi.hoisted(() =>
   vi.fn(async (s: InitState) => ({ ...s, eqlInstalled: true })),
 )
 
+vi.mock('../steps/install-skills.js', () => ({
+  installSkillsStep: { id: 'install-skills', name: 'Skills', run: skillsRun },
+}))
 vi.mock('../steps/authenticate.js', () => ({
   authenticateStep: { id: 'authenticate', name: 'Authenticate', run: authRun },
 }))
@@ -478,3 +491,79 @@ describe('initCommand — CI detection on the `stash plan` chain offer', () => {
     )
   })
 })
+
+describe('initCommand — skills summary and --target', () => {
+  const summaryBody = () =>
+    vi
+      .mocked(p.note)
+      .mock.calls.find(([, title]) => title === 'Setup complete')?.[0] as
+      | string
+      | undefined
+
+  it('reports how many skills were installed', async () => {
+    await initCommand({}, {})
+    expect(summaryBody()).toContain('✓ 1 agent skill installed')
+  })
+
+  /**
+   * The visible half of #923. Init printed an unqualified "Setup complete"
+   * while delivering no guidance at all, and `context.json` recorded an
+   * `installedSkills: []` that looked like a normal empty field. Three of
+   * four skilltester runs against 1.1.0 ended exactly here, with each agent
+   * left to find the bundled skills in `node_modules` on its own.
+   */
+  it('says so loudly, with a remedy, when nothing was installed', async () => {
+    skillsRun.mockImplementationOnce(async (s: InitState) => ({
+      ...s,
+      skills: { installed: [], inlined: [], failed: [] },
+    }))
+
+    await initCommand({}, {})
+
+    const body = summaryBody()
+    expect(body).toContain('No agent skills installed')
+    expect(body).toContain('plan --target claude-code')
+  })
+
+  it('threads a valid --target onto state for the skills step', async () => {
+    await initCommand({}, { target: 'codex' })
+    expect(skillsRun.mock.calls[0]?.[0].targetFlag).toBe('codex')
+  })
+
+  it('rejects an unknown --target before doing any work', async () => {
+    await expect(initCommand({}, { target: 'emacs' })).rejects.toBeInstanceOf(
+      CliExit,
+    )
+    expect(skillsRun).not.toHaveBeenCalled()
+  })
+
+  /**
+   * `parseArgs` files a trailing `--target` (nothing followed it) under
+   * `flags` and `--target=` under `values` as an empty string. A bare
+   * truthiness test on the value treats both as "flag absent", so init would
+   * fall through to auto-detection and could write skills to a directory the
+   * user never chose — silently, having been asked for something specific.
+   */
+  it('rejects a valueless `--target`', async () => {
+    await expect(initCommand({ target: true }, {})).rejects.toBeInstanceOf(
+      CliExit,
+    )
+    expect(skillsRun).not.toHaveBeenCalled()
+  })
+
+  it('rejects an empty `--target=`', async () => {
+    await expect(initCommand({}, { target: '' })).rejects.toBeInstanceOf(
+      CliExit,
+    )
+    expect(skillsRun).not.toHaveBeenCalled()
+  })
+
+  it('names the problem when the value is missing rather than unknown', async () => {
+    await expect(initCommand({ target: true }, {})).rejects.toBeInstanceOf(
+      CliExit,
+    )
+    const message = vi.mocked(p.log.error).mock.calls.map(String).join('\n')
+    expect(message).toContain('needs a value')
+    expect(message).not.toContain('Unknown')
+  })
+})