fix(cli): survive an unwritable .codex/, inline skills when it happens, and ship the doctrine where the CLI can find it

.changeset/codex-skills-eperm.md

@@ -0,0 +1,18 @@
+---
+'stash': patch
+'@cipherstash/wizard': patch
+---
+
+Fix the Codex handoff installing zero skills — and losing `AGENTS.md` and `.cipherstash/` with them — when `.codex/` is not writable.
+
+Codex sandboxes deny writes under `.codex/`. `installSkills` created its destination with an unguarded `mkdirSync`, sitting directly above a per-skill copy loop that *was* guarded — so the failure threw past that fallback and past the caller, aborting the whole handoff step. Because the skills install runs first, nothing after it ran either: no `AGENTS.md`, no `.cipherstash/context.json`, no `.cipherstash/setup-prompt.md`. All five Codex runs of the rc.3 skilltester matrix landed here, and it was identified in that report as the primary driver of the Claude→Codex quality gap.
+
+The fix, hardened by a follow-up review of the first cut:
+
+- **`installSkills` never throws, and reports what happened.** It returns `{ copied, failed }` instead of a flat list, so callers can tell "unwritable destination" from "stripped build" from "partial copy" without re-deriving it — every filesystem failure degrades to a warning plus a `failed` entry.
+- **The Codex handoff inlines exactly the skills that failed.** Whatever could not be copied into `.codex/skills/` — all of them under a sandbox, or a subset after a partial failure — has its body inlined into `AGENTS.md` via the same `doctrine-plus-skills` path the editor-agent handoff uses. The launch prompt points at wherever each skill actually ended up, including both locations after a partial copy. A stripped build that ships no skills stays `doctrine-only` and says nothing.
+- **The doctrine now ships where the published CLI can find it.** The bundled AGENTS.md doctrine was copied to `dist/commands/init/doctrine`, but the compiled resolver probes ancestor directories of the chunk in `dist/bin/` — so every published build silently wrote the minimal `AGENTS.md` stub instead of the doctrine (and the inline fallback would have inlined nothing). It now lands at `dist/doctrine`, like the skills bundle. `buildAgentsMdBody` also honours `doctrine-plus-skills` even when the doctrine fragment is missing, so inlined skills are never dropped with it.
+- **The generated artifacts describe the fallback honestly.** `context.json` gains an `inlinedSkills` field, and `setup-prompt.md` distinguishes installed / inlined / failed skills instead of mislabelling an unwritable destination as a "stripped build". The Claude handoff now warns when skills exist but could not be installed, and the AGENTS.md handoff records what it inlined.
+- **The rest of the handoff is guarded too.** The `AGENTS.md` upsert (which refuses malformed sentinel pairs) and the bundled-file reads degrade to warnings instead of aborting the step before `.cipherstash/` is written.
+
+`@cipherstash/wizard` carries its own copy of `installSkills` with the same unguarded `mkdirSync` above the same guarded copy loop. It targets `.claude/skills` rather than `.codex/skills`, so the Codex sandbox case does not apply, but an unwritable destination crashed it identically — now guarded the same way, with a confirmed-then-failed install recorded in the wizard changelog instead of vanishing with the terminal output.

AGENTS.md

@@ -104,8 +104,10 @@ sentence in one of them the way you'd treat a wrong line of code:
 - `installSkills()` (`packages/cli/src/commands/init/lib/install-skills.ts`) copies the
   per-integration set into the user's `.claude/skills/` or `.codex/skills/` at handoff time.
 - `readBundledSkill()` inlines a skill's body into the user's `AGENTS.md` for editor
-  agents (Cursor / Windsurf / Cline). Only `SKILL.md` is inlined — content split into
-  sibling files is silently dropped on that path, so keep each `SKILL.md` self-sufficient.
+  agents (Cursor / Windsurf / Cline), and as the Codex fallback for skills that could
+  not be copied into `.codex/skills/` (#736). Only `SKILL.md` is inlined — content split
+  into sibling files is silently dropped on that path, so keep each `SKILL.md`
+  self-sufficient.
 
 **Every change to a package's public API, the CLI command surface, or a user-facing
 workflow must check the affected skills in the same PR.** These skills drift silently:

packages/cli/src/commands/impl/steps/__tests__/handoff-claude.test.ts

@@ -25,16 +25,37 @@ import { handoffClaudeStep } from '../handoff-claude.js'
 // assert on.
 const state = { integration: 'postgresql' } as unknown as InitState
 
+const warnings = () => vi.mocked(p.log.warn).mock.calls.map(String).join('\n')
+
 beforeEach(() => vi.clearAllMocks())
 
 it('launch prompt omits the skills dir when no skills were copied', async () => {
-  installSkills.mockReturnValue([])
+  installSkills.mockReturnValue({ copied: [], failed: [] })
   await handoffClaudeStep.run(state)
   expect(vi.mocked(p.note).mock.calls[0][0]).not.toContain('.claude/skills/')
 })
 
 it('launch prompt names the skills dir when skills were copied', async () => {
-  installSkills.mockReturnValue(['stash-encryption'])
+  installSkills.mockReturnValue({ copied: ['stash-encryption'], failed: [] })
   await handoffClaudeStep.run(state)
   expect(vi.mocked(p.note).mock.calls[0][0]).toContain('.claude/skills/')
 })
+
+// Unlike the Codex handoff there is no AGENTS.md on this path to inline
+// into — a failed install must be announced, not left looking complete
+// (#736 follow-up review).
+it('warns when skills exist but could not be installed', async () => {
+  installSkills.mockReturnValue({
+    copied: [],
+    failed: ['stash-encryption', 'stash-cli'],
+  })
+  await handoffClaudeStep.run(state)
+  expect(warnings()).toContain('could not be installed to .claude/skills/')
+  expect(warnings()).toContain('stash-encryption, stash-cli')
+})
+
+it('does not warn when this build simply ships no skills', async () => {
+  installSkills.mockReturnValue({ copied: [], failed: [] })
+  await handoffClaudeStep.run(state)
+  expect(p.log.warn).not.toHaveBeenCalled()
+})

packages/cli/src/commands/impl/steps/__tests__/handoff-codex.test.ts

@@ -1,26 +1,25 @@
-import { beforeEach, expect, it, vi } from 'vitest'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
 import type { InitState } from '../../../init/types.js'
 
 // Same seam as the handoff-claude test: the launch prompt's skills clause is
-// the unit under test. Mock the skill installer to control whether skills were
-// "copied". The Codex handoff also writes AGENTS.md to disk before printing
-// the prompt, so stub `node:fs` and the AGENTS.md builders to keep the test
-// hermetic (no file lands in the repo) — the assertion is purely on the
-// launch-prompt text.
+// the unit under test. Mock the skill installer to control the copied/failed
+// split, and the shared AGENTS.md writer to control whether the inline
+// fallback landed. The assertions are on the launch-prompt text, the
+// AGENTS.md mode chosen, and the delivery recorded into the artifacts.
 const installSkills = vi.hoisted(() => vi.fn())
 vi.mock('../../../init/lib/install-skills.js', () => ({ installSkills }))
+const writeAgentsMd = vi.hoisted(() => vi.fn())
 vi.mock('../../../init/lib/handoff-helpers.js', () => ({
+  AGENTS_MD_REL_PATH: 'AGENTS.md',
+  writeAgentsMd,
   writeArtifacts: vi.fn(),
   spawnAgent: vi.fn(async () => 0),
 }))
-vi.mock('../../../init/lib/build-agents-md.js', () => ({
-  buildAgentsMdBody: vi.fn(() => '# managed doctrine'),
-}))
-vi.mock('../../../init/lib/sentinel-upsert.js', () => ({
-  upsertManagedBlock: vi.fn(() => '# AGENTS.md'),
-}))
+const buildAgentsMdBody = vi.hoisted(() => vi.fn())
+vi.mock('../../../init/lib/build-agents-md.js', () => ({ buildAgentsMdBody }))
 vi.mock('node:fs', () => ({
   existsSync: vi.fn(() => false),
+  mkdirSync: vi.fn(),
   readFileSync: vi.fn(() => ''),
   writeFileSync: vi.fn(),
 }))
@@ -30,25 +29,147 @@ vi.mock('@clack/prompts', () => ({
 }))
 
 import * as p from '@clack/prompts'
+import { writeArtifacts } from '../../../init/lib/handoff-helpers.js'
 import { handoffCodexStep } from '../handoff-codex.js'
 
 // `agents` undefined → Codex "not installed" path, which routes the launch
 // prompt into a `p.note`.
 const state = { integration: 'postgresql' } as unknown as InitState
 
-beforeEach(() => vi.clearAllMocks())
+const promptBody = () => vi.mocked(p.note).mock.calls[0][0]
+const agentsMdMode = () => vi.mocked(buildAgentsMdBody).mock.calls[0][1]
+const inlinedList = () => vi.mocked(buildAgentsMdBody).mock.calls[0][2]
+const delivery = () => vi.mocked(writeArtifacts).mock.calls[0][3]
+const warnings = () => vi.mocked(p.log.warn).mock.calls.map(String).join('\n')
 
-it('launch prompt drops .codex/skills/ but keeps AGENTS.md when no skills copied', async () => {
-  installSkills.mockReturnValue([])
-  await handoffCodexStep.run(state)
-  const body = vi.mocked(p.note).mock.calls[0][0]
-  expect(body).not.toContain('.codex/skills/')
-  // The durable rules live in AGENTS.md regardless, so the prompt still names it.
-  expect(body).toContain('AGENTS.md')
+beforeEach(() => {
+  vi.clearAllMocks()
+  writeAgentsMd.mockReturnValue(true)
 })
 
-it('launch prompt names .codex/skills/ when skills were copied', async () => {
-  installSkills.mockReturnValue(['stash-encryption'])
+it('launch prompt names .codex/skills/ when all skills were copied', async () => {
+  installSkills.mockReturnValue({ copied: ['stash-encryption'], failed: [] })
   await handoffCodexStep.run(state)
-  expect(vi.mocked(p.note).mock.calls[0][0]).toContain('.codex/skills/')
+
+  expect(promptBody()).toContain('.codex/skills/')
+  // Skills are on disk, so AGENTS.md stays doctrine-only per Codex guidance.
+  expect(agentsMdMode()).toBe('doctrine-only')
+  expect(delivery()).toEqual({
+    installed: ['stash-encryption'],
+    inlined: [],
+    failed: [],
+  })
+})
+
+// #736: Codex sandboxes deny writes under `.codex/`. The skills cannot land,
+// but AGENTS.md (project root) can — so the guidance is inlined there rather
+// than lost, and Codex is pointed at it.
+describe('when .codex/skills could not be written at all', () => {
+  beforeEach(() => {
+    installSkills.mockReturnValue({
+      copied: [],
+      failed: ['stash-encryption', 'stash-cli'],
+    })
+  })
+
+  it('inlines the failed skills into AGENTS.md instead of shipping nothing', async () => {
+    await handoffCodexStep.run(state)
+    expect(agentsMdMode()).toBe('doctrine-plus-skills')
+    expect(inlinedList()).toEqual(['stash-encryption', 'stash-cli'])
+  })
+
+  it('points the launch prompt at AGENTS.md, not the directory that failed', async () => {
+    await handoffCodexStep.run(state)
+    const body = promptBody()
+    expect(body).not.toContain('.codex/skills/')
+    expect(body).toContain('inlined in AGENTS.md')
+  })
+
+  it('says so, rather than reporting a silent success', async () => {
+    await handoffCodexStep.run(state)
+    expect(warnings()).toContain('.codex/skills/')
+    expect(warnings()).toContain('AGENTS.md')
+  })
+
+  it('records the skills as inlined, not installed, in the artifacts', async () => {
+    await handoffCodexStep.run(state)
+    expect(delivery()).toEqual({
+      installed: [],
+      inlined: ['stash-encryption', 'stash-cli'],
+      failed: [],
+    })
+  })
+
+  it('records the skills as failed when AGENTS.md could not be written either', async () => {
+    writeAgentsMd.mockReturnValue(false)
+    await handoffCodexStep.run(state)
+    expect(delivery()).toEqual({
+      installed: [],
+      inlined: [],
+      failed: ['stash-encryption', 'stash-cli'],
+    })
+    // Nothing was inlined, so the prompt must not claim otherwise.
+    expect(promptBody()).not.toContain('inlined in AGENTS.md')
+  })
+})
+
+// A partial copy — mkdir succeeded, one skill's cpSync failed — must inline
+// exactly the missing skill, not declare success and drop it (#736 follow-up
+// review: the fallback used to be all-or-nothing on installed.length === 0).
+describe('when only some skills could be written', () => {
+  beforeEach(() => {
+    installSkills.mockReturnValue({
+      copied: ['stash-encryption'],
+      failed: ['stash-cli'],
+    })
+  })
+
+  it('inlines only the failed skill', async () => {
+    await handoffCodexStep.run(state)
+    expect(agentsMdMode()).toBe('doctrine-plus-skills')
+    expect(inlinedList()).toEqual(['stash-cli'])
+  })
+
+  it('points the launch prompt at both locations', async () => {
+    await handoffCodexStep.run(state)
+    const body = promptBody()
+    expect(body).toContain('.codex/skills/')
+    expect(body).toContain('inlined in AGENTS.md')
+  })
+
+  it('records the split delivery in the artifacts', async () => {
+    await handoffCodexStep.run(state)
+    expect(delivery()).toEqual({
+      installed: ['stash-encryption'],
+      inlined: ['stash-cli'],
+      failed: [],
+    })
+  })
+})
+
+// A stripped CLI build ships no skills at all. Nothing to copy AND nothing to
+// inline — claiming a fallback here would be a false success of the kind #714
+// and #687 removed elsewhere in init.
+describe('when this build ships no skills at all', () => {
+  beforeEach(() => {
+    installSkills.mockReturnValue({ copied: [], failed: [] })
+  })
+
+  it('stays doctrine-only — there is nothing to inline', async () => {
+    await handoffCodexStep.run(state)
+    expect(agentsMdMode()).toBe('doctrine-only')
+  })
+
+  it('drops the skills clause but still names AGENTS.md for the durable rules', async () => {
+    await handoffCodexStep.run(state)
+    const body = promptBody()
+    expect(body).not.toContain('.codex/skills/')
+    expect(body).not.toContain('inlined in AGENTS.md')
+    expect(body).toContain('AGENTS.md')
+  })
+
+  it('does not warn at all — there is no fallback to announce', async () => {
+    await handoffCodexStep.run(state)
+    expect(p.log.warn).not.toHaveBeenCalled()
+  })
 })

packages/cli/src/commands/impl/steps/handoff-agents-md.ts

@@ -1,17 +1,17 @@
-import { existsSync, readFileSync, writeFileSync } from 'node:fs'
-import { resolve } from 'node:path'
 import * as p from '@clack/prompts'
 import { buildAgentsMdBody } from '../../init/lib/build-agents-md.js'
-import { writeArtifacts } from '../../init/lib/handoff-helpers.js'
-import { upsertManagedBlock } from '../../init/lib/sentinel-upsert.js'
+import {
+  AGENTS_MD_REL_PATH,
+  writeAgentsMd,
+  writeArtifacts,
+} from '../../init/lib/handoff-helpers.js'
+import { availableSkills } from '../../init/lib/install-skills.js'
 import {
   CONTEXT_REL_PATH,
   SETUP_PROMPT_REL_PATH,
 } from '../../init/lib/write-context.js'
 import type { HandoffStep, InitState } from '../../init/types.js'
 
-const AGENTS_MD_REL_PATH = 'AGENTS.md'
-
 /**
  * Write `AGENTS.md`, `.cipherstash/context.json`, and
  * `.cipherstash/setup-prompt.md`, then stop.
@@ -32,19 +32,19 @@ export const handoffAgentsMdStep: HandoffStep = {
     const cwd = process.cwd()
     const integration = state.integration ?? 'postgresql'
 
-    const agentsMdAbs = resolve(cwd, AGENTS_MD_REL_PATH)
-    const managed = buildAgentsMdBody(integration, 'doctrine-plus-skills')
-    const existing = existsSync(agentsMdAbs)
-      ? readFileSync(agentsMdAbs, 'utf-8')
-      : undefined
-    writeFileSync(
-      agentsMdAbs,
-      upsertManagedBlock({ existing, managed }),
-      'utf-8',
+    const inlinable = availableSkills(integration)
+    const managed = buildAgentsMdBody(
+      integration,
+      'doctrine-plus-skills',
+      inlinable,
     )
-    p.log.success(`Wrote ${AGENTS_MD_REL_PATH}`)
+    const written = writeAgentsMd(cwd, managed)
 
-    writeArtifacts(cwd, state, 'agents-md', [])
+    writeArtifacts(cwd, state, 'agents-md', {
+      installed: [],
+      inlined: written ? inlinable : [],
+      failed: written ? [] : inlinable,
+    })
 
     p.note(
       [

packages/cli/src/commands/impl/steps/handoff-claude.ts

@@ -25,21 +25,37 @@ export const handoffClaudeStep: HandoffStep = {
     const cwd = process.cwd()
     const integration = state.integration ?? 'postgresql'
 
-    const installed = installSkills(cwd, CLAUDE_SKILLS_DIR, integration)
-    if (installed.length > 0) {
+    const { copied, failed } = installSkills(
+      cwd,
+      CLAUDE_SKILLS_DIR,
+      integration,
+    )
+    if (copied.length > 0) {
       p.log.success(
-        `Installed ${installed.length} skill${installed.length !== 1 ? 's' : ''} into ${CLAUDE_SKILLS_DIR}/: ${installed.join(', ')}`,
+        `Installed ${copied.length} skill${copied.length !== 1 ? 's' : ''} into ${CLAUDE_SKILLS_DIR}/: ${copied.join(', ')}`,
+      )
+    }
+    if (failed.length > 0) {
+      // Unlike the Codex handoff there is no AGENTS.md on this path to
+      // inline into, so the guidance is genuinely absent — say so rather
+      // than letting the run look complete (#736 follow-up review).
+      p.log.warn(
+        `${failed.length} skill${failed.length !== 1 ? 's' : ''} could not be installed to ${CLAUDE_SKILLS_DIR}/: ${failed.join(', ')}. The agent will run without them — https://cipherstash.com/docs covers the same ground.`,
       )
     }
 
-    writeArtifacts(cwd, state, 'claude-code', installed)
+    writeArtifacts(cwd, state, 'claude-code', {
+      installed: copied,
+      inlined: [],
+      failed,
+    })
 
     const mode = state.mode ?? 'implement'
     // Only point the agent at the skills dir when skills were actually copied.
-    // A stripped CLI build (no bundled skills) returns [] — claiming they're
-    // there would send the agent to read files that don't exist.
+    // A stripped CLI build (no bundled skills) copies nothing — claiming
+    // they're there would send the agent to read files that don't exist.
     const skillsClause =
-      installed.length > 0
+      copied.length > 0
         ? `The installed skills under ${CLAUDE_SKILLS_DIR}/ have the rules; `
         : ''
     const launchPrompt =