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

* fix(cli): survive an unwritable .codex/, and inline skills when it happens

Codex sandboxes deny writes under `.codex/`. `installSkills` created its
destination with an unguarded `mkdirSync` sitting directly ABOVE a
per-skill copy loop that was already 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 that
report named it the primary driver of the Claude→Codex quality gap (#736).

Two changes.

`installSkills` never throws. Every filesystem step degrades to a warning
and a shorter return, so the caller decides what an empty result means
rather than being skipped entirely.

The Codex handoff falls back to inlining. When the skills cannot be
written, their bodies go into AGENTS.md — project root, writable —
through the same `doctrine-plus-skills` path the editor-agent handoff
already uses for Cursor / Windsurf / Cline. No new mechanism; the
existing one just was not reachable from this branch. The launch prompt
points at wherever the guidance actually ended up.

The fallback is only claimed when there is something to inline. A
stripped build ships no skills at all, so it stays doctrine-only and says
nothing — reporting a fallback that did not happen would be the same
false-success bug #714 and #687 removed elsewhere in init. That is why
`availableSkills` is split out: an empty install result alone cannot
distinguish "could not write" from "nothing to write".

@cipherstash/wizard carries its own copy of `installSkills` with the
identical unguarded `mkdirSync` above the identical guarded copy loop. It
targets `.claude/skills`, so the Codex sandbox case does not apply, but an
unwritable destination crashed it the same way — guarded to match.

Tests: 2 for the mkdirSync guard (including that the caller can still
distinguish unwritable from empty), 6 for the handoff across all three
states — copied, unwritable-so-inlined, and nothing-to-ship. 645 unit +
68 pty E2E pass; typecheck and build clean; 0 biome errors.

Checked per packages/cli/AGENTS.md: no E2E asserts on these strings (the
only codex assertion is the target list, unchanged), and no skill
documents the install location, so neither needed updating.

* fix(cli): make the Codex skills fallback deliver what it claims

Address the 15 findings from the PR #745 review.

The structural change: installSkills returns { copied, failed } instead
of a flat list, so callers distinguish unwritable / stripped / partial
outcomes without a second probe. The Codex handoff inlines exactly the
failed skills — a partial copy inlines the missing ones and the launch
prompt points at both locations.

The production-blocking find: the bundled AGENTS.md doctrine shipped at
dist/commands/init/doctrine, but findBundledDir probes ancestors of the
compiled chunk in dist/bin/ — so every published build wrote the minimal
AGENTS.md stub, and the inline fallback would have inlined nothing while
claiming otherwise. The doctrine now lands at dist/doctrine, like the
skills bundle, and buildAgentsMdBody honours doctrine-plus-skills even
when the doctrine fragment is missing.

Honesty and hardening around the fallback:

- writeArtifacts takes a SkillsDelivery (installed/inlined/failed);
  context.json gains inlinedSkills and setup-prompt.md stops
  mislabelling an unwritable destination as a "stripped build" — on the
  Claude and AGENTS.md handoffs too
- availableSkills filters on SKILL.md, the predicate the inliner
  actually uses, so "inlining N skills" never overstates
- 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; skills only count as inlined when
  the write landed
- the fallback warning announces the observed recovery instead of
  asserting an unverified cause, and flags a stale .codex/skills/ it
  could not refresh
- a confirmed-then-failed wizard install is recorded in the wizard
  changelog instead of vanishing with the terminal output

Tests pin the degrade-to-warning contract in both packages (the wizard
suite is new), cover the partial-copy split end to end, and replace the
vacuously-true warning assertions. The doctrine's "Where to read more",
the build-agents-md docblock, and root AGENTS.md now describe the real
mode consumers.

* test(cli): make the partial-copy fixture deterministic across platforms

The read-only-directory trigger was platform-dependent: on macOS cpSync
unlinks the destination file first (EACCES from the 0o555 parent), but
on Linux it overwrites the writable SKILL.md in place and the copy
succeeds — so the test failed on CI while passing locally.

Block the skill with a FILE where its directory must go instead:
cpSync then throws ERR_FS_CP_DIR_TO_NON_DIR, a type-mismatch error
raised before any permission-dependent operation, so it fires on every
platform and as root. Same technique the neighbouring mkdir test
already uses. Drops the skipIf guard the chmod approach needed.

* style(cli): format the partial-copy fixture line

Author: coderdan

.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 =