.claude/spec/gitea-axi.md

@@ -491,7 +491,7 @@ The dashboard's empty states are `prs: 0 open` / `issues: 0 open` (raw strings,
 Empty output is never silent.
 
 **Principle 6 — Structured errors, exit codes, idempotent mutations, no prompts.**
-Errors are represented as a typed `AxiError` with one of ten named codes: `REPO_NOT_FOUND`, `ISSUE_NOT_FOUND`, `PR_NOT_FOUND`, `AUTH_REQUIRED`, `FORBIDDEN`, `RATE_LIMITED`, `TEA_NOT_INSTALLED`, `VALIDATION_ERROR`, `GIT_ERROR`, `UNKNOWN`.
+Errors are represented as a typed `AxiError` with one of eleven named codes: `REPO_NOT_FOUND`, `ISSUE_NOT_FOUND`, `PR_NOT_FOUND`, `AUTH_REQUIRED`, `FORBIDDEN`, `RATE_LIMITED`, `TEA_NOT_INSTALLED`, `VALIDATION_ERROR`, `GIT_ERROR`, `TARGET_NOT_WRITABLE`, `UNKNOWN`.
 The `ISSUE_NOT_FOUND`/`PR_NOT_FOUND` split (vs gh-axi's single `NOT_FOUND`) is a deliberate divergence enabled by path-based 404 classification.
 API error responses are classified by HTTP status code and calling context:
 
@@ -516,6 +516,8 @@ tea has logins but none match the detected hostname → `REPO_NOT_FOUND` (the re
 HTTP 401 from the API → `AUTH_REQUIRED` (token invalid or revoked), per the status table.
 A `--login` value naming a nonexistent profile is `VALIDATION_ERROR`, listing the available profile names.
 `GIT_ERROR` classifies non-zero git subprocess exits (currently only `pr checkout`), carrying git's first stderr line.
+`TARGET_NOT_WRITABLE` classifies a `setup` target the filesystem refuses (`EACCES`, `EPERM`, `EROFS`), naming the file and pointing at the general remedy: it appears to be managed by another tool, so the skill or hook belongs in that tool's configuration.
+It never names or infers a particular configuration manager — read-only is not diagnostic of one.
 Error output is TOON-encoded to stdout (not stderr): `error: <message>`, `code: <CODE>`, and optionally `help[N]:` with suggestion lines.
 The suggestions field is named `help`, not `hint`.
 Exit codes: 0 success, 1 error, 2 for `VALIDATION_ERROR` — covering unknown flags, missing required inputs, and server-side 422 rejections alike (the `axi-sdk-js` `exitCodeForError` mapping; see ADR 0004).

.claude/tasks/0044-setup-fails-clean-on-unwritable-targets.md

@@ -21,9 +21,32 @@ The failure follows the CLI's existing error convention rather than inventing a
 
 ## Acceptance criteria
 
-- [ ] An unwritable skill target produces a structured CLI error rather than an unhandled filesystem exception.
+- [x] An unwritable skill target produces a structured CLI error rather than an unhandled filesystem exception.
-- [ ] An unwritable hook target produces the same class of error, with the same guidance.
+- [x] An unwritable hook target produces the same class of error, with the same guidance.
-- [ ] Both errors name the file that could not be written and state that it appears to be managed by another tool.
+- [x] Both errors name the file that could not be written and state that it appears to be managed by another tool.
-- [ ] Neither error names or infers a specific configuration manager.
+- [x] Neither error names or infers a specific configuration manager.
-- [ ] A skill target that is unwritable but already byte-identical to the bundled copy succeeds rather than failing, since nothing needs to be written.
+- [x] A skill target that is unwritable but already byte-identical to the bundled copy succeeds rather than failing, since nothing needs to be written.
-- [ ] The errors carry a code and help lines consistent with the rest of the CLI's error surface.
+- [x] The errors carry a code and help lines consistent with the rest of the CLI's error surface.
+
+## Implementation Notes
+
+The condition is `EACCES`, `EPERM`, or `EROFS` — the three ways a filesystem refuses a write for a reason the user has to settle outside this tool.
+The new `TARGET_NOT_WRITABLE` code joins the spec's enumerated list, alongside a paragraph describing it.
+
+Two things came out of review and go slightly beyond the literal criteria.
+
+The skill half now guards its comparison read as well as its write.
+A target the filesystem will not let us read is one it will not let us replace either — the same condition reached one call earlier — so a mode-`000` file reports the same error rather than the raw exception the criteria were written against.
+
+The hook half collects its failures as `{path, detail}` records rather than the agent SDK's flattened `<path>: <message>` text.
+The SDK reports through a string, so the two halves are separated once at that boundary and judged apart.
+This matters for correctness, not just shape: testing the whole formatted string for an errno would misclassify an unrelated failure whose *path* happened to contain `EACCES`.
+
+Two known limits, both judged acceptable rather than fixed.
+
+The hook error names the target the SDK was writing, which is the intended path rather than necessarily the blocking one — if `~/.claude` were unwritable and `settings.json` absent, it would name the file rather than the directory.
+The SDK discards the error object, so its `path` is not recoverable; the skill half, which catches its own errors, does report the blocking path and is tested for it.
+
+An unwritable `~/.claude/settings.json` still leaves the Codex and OpenCode integrations installed, because the SDK writes them before the failure surfaces.
+The command exits 1 having done part of its work.
+Making the hook install transactional across three integrations owned by the SDK is a larger change than this task, and re-running after fixing the permission converges correctly.

src/commands/setup.ts

@@ -4,7 +4,12 @@ import { join } from "node:path";
 import { fileURLToPath } from "node:url";
 import { installSessionStartHooks } from "axi-sdk-js";
 import type { CliDeps } from "../deps.js";
-import { axiError } from "../errors.js";
+import {
+  axiError,
+  isNotWritableError,
+  isNotWritableMessage,
+  unwritableTargetError,
+} from "../errors.js";
 import { pruneDuplicateManagedHooks, resolveEntrypointOnPath } from "../hooks.js";
 import { renderDetail } from "../render.js";
 
@@ -65,16 +70,31 @@ function installSkill(home: string): { skill: string; path: string; status: Skil
   const targetPath = join(targetDir, "SKILL.md");
 
   let status: SkillStatus;
+  try {
     if (!existsSync(targetPath)) {
       status = "installed";
     } else {
       status = readFileSync(targetPath, "utf8") === source ? "unchanged" : "updated";
     }
 
+    // A target that already holds these bytes needs no write, so its being
+    // read-only is beside the point and the run succeeds.
     if (status !== "unchanged") {
       mkdirSync(targetDir, { recursive: true });
       writeFileSync(targetPath, source, "utf8");
     }
+  } catch (error) {
+    // The comparison read is inside the guard because a target the filesystem
+    // will not let us read is one it will not let us replace either — the same
+    // condition, reached one call earlier.
+    if (isNotWritableError(error)) {
+      // The path the filesystem names is the directory when it is the directory
+      // that is read-only, so report that rather than assuming the file.
+      const blocked = (error as { path?: string }).path ?? targetPath;
+      throw unwritableTargetError(blocked, `the ${SKILL_NAME} skill`);
+    }
+    throw error;
+  }
 
   return { skill: SKILL_NAME, path: collapseHome(targetPath, home), status };
 }
@@ -99,6 +119,38 @@ const HOOK_SETTINGS_FILES = [
   [".codex", "hooks.json"],
 ];
 
+/** One target the hook install could not write, and why. */
+interface TargetFailure {
+  /** The file being written when it failed. */
+  path: string;
+  /** The underlying failure, with no path spliced into it. */
+  detail: string;
+}
+
+/**
+ * Recover a {@link TargetFailure} from the agent SDK's reporting.
+ *
+ * It hands its failures to `onError` as `<path>: <message>` text rather than as
+ * errors, so the two halves have to be separated again before either can be
+ * judged on its own. Text in that shape is all it ever emits; anything else is
+ * returned whole as the detail, with no path to attribute it to.
+ */
+function parseReportedFailure(reported: string): TargetFailure {
+  const separator = reported.indexOf(": ");
+  if (separator === -1) {
+    return { path: "", detail: reported };
+  }
+  return {
+    path: reported.slice(0, separator),
+    detail: reported.slice(separator + 2),
+  };
+}
+
+/** A failure rendered back as the `<path>: <detail>` line a reader sees. */
+function formatFailure({ path, detail }: TargetFailure): string {
+  return path === "" ? detail : `${path}: ${detail}`;
+}
+
 /**
  * Collapse any duplicate managed entry the SDK's own recognition missed.
  *
@@ -109,7 +161,11 @@ const HOOK_SETTINGS_FILES = [
  * command's shape — and cannot mistake another tool's hook for ours the way a
  * substring test can.
  */
-function pruneHookSettingsFiles(home: string, command: string, errors: string[]): void {
+function pruneHookSettingsFiles(
+  home: string,
+  command: string,
+  errors: TargetFailure[],
+): void {
   const isManaged = (recorded: string) => recorded === command;
 
   for (const segments of HOOK_SETTINGS_FILES) {
@@ -124,14 +180,17 @@ function pruneHookSettingsFiles(home: string, command: string, errors: string[])
         writeFileSync(target, `${JSON.stringify(settings, null, 2)}\n`, "utf8");
       }
     } catch (error) {
-      errors.push(`${target}: ${error instanceof Error ? error.message : String(error)}`);
+      errors.push({
+        path: target,
+        detail: error instanceof Error ? error.message : String(error),
+      });
     }
   }
 }
 
 async function setupHooks(deps: CliDeps): Promise<string> {
   const home = resolveHome(deps);
-  const errors: string[] = [];
+  const errors: TargetFailure[] = [];
 
   // ADR 0019: record a search-path name, not an install-tree path. Handing the
   // SDK where the binary resolves on PATH — rather than the module-relative
@@ -156,13 +215,25 @@ async function setupHooks(deps: CliDeps): Promise<string> {
     // deferring to the SDK's auto-install safety gate (which is tuned for the
     // inferred dist/bin/<name>.js entrypoint layout gitea-axi does not use).
     shouldInstall: () => true,
-    onError: (message) => errors.push(message),
+    onError: (message) => errors.push(parseReportedFailure(message)),
   });
 
   pruneHookSettingsFiles(home, command, errors);
 
   if (errors.length > 0) {
-    throw axiError(`Failed to install session hooks: ${errors.join("; ")}`, "UNKNOWN");
+    // A read-only target is the most actionable thing that can be in here — it
+    // names something the user must settle elsewhere rather than a bug — so it
+    // is reported ahead of whatever else was collected.
+    const unwritable = errors.find(
+      (failure) => failure.path !== "" && isNotWritableMessage(failure.detail),
+    );
+    if (unwritable) {
+      throw unwritableTargetError(unwritable.path, `the ${SKILL_NAME} session hook`);
+    }
+    throw axiError(
+      `Failed to install session hooks: ${errors.map(formatFailure).join("; ")}`,
+      "UNKNOWN",
+    );
   }
 
   return renderDetail({

src/errors.ts

@@ -10,6 +10,7 @@ export type AxiErrorCode =
   | "TEA_NOT_INSTALLED"
   | "VALIDATION_ERROR"
   | "GIT_ERROR"
+  | "TARGET_NOT_WRITABLE"
   | "UNKNOWN";
 
 export function axiError(
@@ -20,6 +21,50 @@ export function axiError(
   return new AxiError(message, code, suggestions);
 }
 
+// The three ways a filesystem refuses a write for reasons the user must settle
+// outside this tool: the permission bits deny it, the file is flagged immutable
+// or otherwise protected, or the filesystem itself is mounted read-only.
+const NOT_WRITABLE_ERRNOS = ["EACCES", "EPERM", "EROFS"];
+
+/** Whether a caught filesystem error means the target cannot be written. */
+export function isNotWritableError(error: unknown): boolean {
+  const errno = (error as { code?: unknown } | null)?.code;
+  return typeof errno === "string" && NOT_WRITABLE_ERRNOS.includes(errno);
+}
+
+/**
+ * The same judgement made from an error's *message*, for a caller handed the
+ * formatted text rather than the error object.
+ *
+ * This takes the message alone, never a string the target's path has been
+ * spliced into: a path is the user's to name, and one that happened to contain
+ * `EACCES` would otherwise misreport an unrelated failure as a read-only target.
+ */
+export function isNotWritableMessage(message: string): boolean {
+  return NOT_WRITABLE_ERRNOS.some((errno) => message.includes(errno));
+}
+
+/**
+ * A read-only target reported as something the user can act on.
+ *
+ * The remedy is deliberately general. Read-only is not diagnostic of any
+ * particular configuration manager, and every plausible cause — a declarative
+ * home manager, an immutable flag, a root-owned path — has the same answer:
+ * whatever renders the file read-only is where this belongs, not here.
+ *
+ * `subject` names what the caller was installing, for the remedy line.
+ */
+export function unwritableTargetError(path: string, subject: string): AxiError {
+  return axiError(
+    `Cannot write ${path}: it is not writable, so it appears to be managed by another tool`,
+    "TARGET_NOT_WRITABLE",
+    [
+      `Declare ${subject} through that tool's configuration rather than installing it with this command`,
+      `Or make ${path} writable and re-run`,
+    ],
+  );
+}
+
 interface HttpResponseLike {
   status: number;
   url: string;

test/setup.test.ts

@@ -1,4 +1,13 @@
-import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import {
+  chmodSync,
+  existsSync,
+  mkdirSync,
+  mkdtempSync,
+  readdirSync,
+  readFileSync,
+  rmSync,
+  writeFileSync,
+} from "node:fs";
 import { tmpdir } from "node:os";
 import { delimiter, isAbsolute, join } from "node:path";
 import { fileURLToPath } from "node:url";
@@ -7,12 +16,49 @@ import { type CliResult, runCliTest } from "./harness.js";
 
 let tempHome: string;
 
+/** Restore write permission everywhere under `dir` so the tree can be removed. */
+function restorePermissions(dir: string): void {
+  for (const entry of readdirSync(dir, { withFileTypes: true })) {
+    const path = join(dir, entry.name);
+    chmodSync(path, entry.isDirectory() ? 0o700 : 0o600);
+    if (entry.isDirectory()) {
+      restorePermissions(path);
+    }
+  }
+}
+
 afterEach(() => {
-  if (tempHome) {
+  // Not every test creates a HOME, so this may be a directory an earlier test
+  // already removed.
+  if (tempHome && existsSync(tempHome)) {
+    // The read-only-target tests leave files and directories unwritable, and
+    // an unwritable directory cannot have its entries unlinked.
+    chmodSync(tempHome, 0o700);
+    restorePermissions(tempHome);
     rmSync(tempHome, { recursive: true, force: true });
   }
+  tempHome = "";
 });
 
+/**
+ * Permission bits are not enforced for root, so the read-only-target tests
+ * cannot express their premise there and are skipped rather than passing
+ * vacuously.
+ */
+const itUnlessRoot = process.getuid?.() === 0 ? it.skip : it;
+
+/**
+ * Assert the error's wording infers no particular configuration manager.
+ *
+ * Read-only is not diagnostic of one, so naming one would be wrong for most
+ * readers who hit this. The paths the error quotes are exempt — they are the
+ * user's own, and here the temp directory sits under a `nix-shell` TMPDIR.
+ */
+function expectNamesNoManager(stdout: string, home: string): void {
+  const wording = stdout.split(home).join("<home>");
+  expect(wording).not.toMatch(/\b(nix|home-manager|nixos|chezmoi|ansible|stow|guix)\b/i);
+}
+
 /**
  * Run `body` with `process.env.PATH` replaced. `setup hooks` reads PATH from the
  * process rather than the injected environment, because it has to agree with
@@ -78,6 +124,60 @@ describe("setup", () => {
     expect(third.stdout).toContain("status: updated");
     expect(readFileSync(installedPath, "utf8")).not.toBe("tampered");
   });
+
+  itUnlessRoot("reports a read-only skill target as a structured error", async () => {
+    tempHome = mkdtempSync(join(tmpdir(), "gitea-axi-setup-"));
+    const installedPath = join(tempHome, ".claude", "skills", "gitea-axi", "SKILL.md");
+
+    // A declaratively managed install in miniature: the file is present, its
+    // content differs from the bundled copy, and it cannot be written.
+    mkdirSync(join(tempHome, ".claude", "skills", "gitea-axi"), { recursive: true });
+    writeFileSync(installedPath, "managed elsewhere\n");
+    chmodSync(installedPath, 0o444);
+
+    const { stdout, exitCode } = await runCliTest(["setup"], { env: { HOME: tempHome } });
+
+    expect(exitCode).toBe(1);
+    expect(stdout).toContain("code: TARGET_NOT_WRITABLE");
+    expect(stdout).toContain(installedPath);
+    expect(stdout).toContain("managed by another tool");
+    expectNamesNoManager(stdout, tempHome);
+    // The bundled copy is untouched by a failed run.
+    expect(readFileSync(installedPath, "utf8")).toBe("managed elsewhere\n");
+  });
+
+  itUnlessRoot("names the directory when it is the directory that is read-only", async () => {
+    tempHome = mkdtempSync(join(tmpdir(), "gitea-axi-setup-"));
+    const skillsDir = join(tempHome, ".claude", "skills");
+
+    // Nothing installed yet, and no new entry can be created here — so the
+    // blocked path is the directory, not the file that would have gone in it.
+    mkdirSync(skillsDir, { recursive: true });
+    chmodSync(skillsDir, 0o555);
+
+    const { stdout, exitCode } = await runCliTest(["setup"], { env: { HOME: tempHome } });
+
+    expect(exitCode).toBe(1);
+    expect(stdout).toContain("code: TARGET_NOT_WRITABLE");
+    expect(stdout).toContain(join(skillsDir, "gitea-axi"));
+    expectNamesNoManager(stdout, tempHome);
+  });
+
+  itUnlessRoot("succeeds on a read-only skill target that is already up to date", async () => {
+    tempHome = mkdtempSync(join(tmpdir(), "gitea-axi-setup-"));
+    const installedPath = join(tempHome, ".claude", "skills", "gitea-axi", "SKILL.md");
+
+    const first = await runCliTest(["setup"], { env: { HOME: tempHome } });
+    expect(first.exitCode).toBe(0);
+
+    // Same bytes the command would write, so there is nothing to write and the
+    // target's being read-only is beside the point.
+    chmodSync(installedPath, 0o444);
+
+    const { stdout, exitCode } = await runCliTest(["setup"], { env: { HOME: tempHome } });
+    expect(exitCode).toBe(0);
+    expect(stdout).toContain("status: unchanged");
+  });
 });
 
 describe("setup hooks", () => {
@@ -179,6 +279,25 @@ describe("setup hooks", () => {
 
     expect(recordedHookCommand(tempHome)).toBe(entrypointPath());
   });
+
+  itUnlessRoot("reports a read-only hook target as the same structured error", async () => {
+    tempHome = mkdtempSync(join(tmpdir(), "gitea-axi-setup-"));
+    const settingsPath = join(tempHome, ".claude", "settings.json");
+
+    mkdirSync(join(tempHome, ".claude"), { recursive: true });
+    writeFileSync(settingsPath, "{}\n");
+    chmodSync(settingsPath, 0o444);
+
+    const { stdout, exitCode } = await runCliTest(["setup", "hooks"], {
+      env: { HOME: tempHome },
+    });
+
+    expect(exitCode).toBe(1);
+    expect(stdout).toContain("code: TARGET_NOT_WRITABLE");
+    expect(stdout).toContain(settingsPath);
+    expect(stdout).toContain("managed by another tool");
+    expectNamesNoManager(stdout, tempHome);
+  });
 });
 
 describe("setup dispatch", () => {