From 980aea6c919b09fc3abd69b579584fe8febcae87 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Fri, 23 Oct 2026 10:21:32 -0400 Subject: [PATCH] fix(agents): name the permission fix and keep a value-free cause on input access refusals Target inspector follow-ups after #379 (HACK-1390). - Classify input presence faults through an exported seam, projectInputAccessFault(), so EPERM is covered by a test (a mode-000 directory only produces EACCES). - ProjectInputAccessError keeps a value-free class/code diagnostic (for example Error:ENOTDIR) as a non-enumerable property, read with projectInputAccessDiagnostic(). Envelopes, printed errors and JSON stay unchanged and carry no path, OS message or cause. - input_permission_denied now names the permission fix first. The contract doc no longer says an unsearchable .hack or .hack/.internal is adoption_state_unsafe; it is input_permission_denied even on a project that never used adoption. Refs HACK-1390 Co-Authored-By: Claude Opus 5.5 --- docs/reference/ax-target-result-contract.md | 24 ++++--- src/lib/project-input-selection.ts | 76 +++++++++++++++++++-- src/lib/target-inspect.ts | 3 +- tests/project-input-selection.test.ts | 69 +++++++++++++++++-- tests/project-target-inspect.test.ts | 44 ++++++++++++ 5 files changed, 195 insertions(+), 21 deletions(-) diff --git a/docs/reference/ax-target-result-contract.md b/docs/reference/ax-target-result-contract.md index 178aa312f..81a049992 100644 --- a/docs/reference/ax-target-result-contract.md +++ b/docs/reference/ax-target-result-contract.md @@ -166,13 +166,18 @@ absolute path. The shared project input guards that every project command runs are mapped too. Native or conflicting project files are `project_inputs_unsupported`. A -legacy adoption state that is unparseable, interrupted, or cannot be checked -(for example a project directory the caller cannot search) is -`adoption_state_unsafe`. A non-directory where the project directory belongs is -`input_unreadable`. When the guard cannot check whether a project input exists -(for example a `.dev` file, or a `.dev` directory the caller cannot search, -beside a valid `.hack`), the refusal is `input_permission_denied` (`EACCES`, -`EPERM`) or `input_unreadable` (any other fault), like a read fault. +`.hack` or `.hack/.internal` the caller cannot search (for example mode `000` +or `0644`) is a permission fault, not an adoption problem: it is +`input_permission_denied` even when the project has never used adoption, and +its message names the permission fix (make the directories readable and +searchable by your user, for example `chmod u+rwx .hack`) before anything +else. A legacy adoption state that is unparseable or interrupted is +`adoption_state_unsafe`, whose message points at explicit adoption recovery. +A non-directory where the project directory belongs is `input_unreadable`. +When the guard cannot check whether a project input exists (for example a +`.dev` file, or a `.dev` directory the caller cannot search, beside a valid +`.hack`), the refusal is `input_permission_denied` (`EACCES`, `EPERM`) or +`input_unreadable` (any other fault), like a read fault. `retry` is advice for the caller. Like suggested actions and application logs, it carries no execution authority. @@ -219,8 +224,9 @@ WU21). The inspector itself only produces snapshots. | Two registrations owning one checkout | `ambiguous_project` | same | paired fixture | | Malformed Hack config or Compose file, oversized input | `config_parse_failed`, `compose_parse_failed`, `generation_input_too_large` | same | paired fixture | | Unreadable input (directory in place of a file, no read permission) | `input_unreadable`, `input_permission_denied` | same | paired fixture (permission case skipped as root) | -| Input guard refusals (native/legacy conflict, unsafe adoption state, unsearchable or non-directory project dir) | `project_inputs_unsupported`, `adoption_state_unsafe`, `input_unreadable` | same | paired fixture (unsearchable case skipped as root) | -| Uncheckable input presence (`.dev` file or unsearchable `.dev` beside a valid `.hack`) | `input_unreadable`, `input_permission_denied` | same | paired fixture and domain test (unsearchable case skipped as root) | +| Input guard refusals (native/legacy conflict, unsafe adoption state, non-directory project dir) | `project_inputs_unsupported`, `adoption_state_unsafe`, `input_unreadable` | same | paired fixture | +| Unsearchable `.hack` or `.hack/.internal` on a never-adopted project | `input_permission_denied` (permission fix named first) | same | paired fixture and domain test (skipped as root) | +| Uncheckable input presence (`.dev` file or unsearchable `.dev` beside a valid `.hack`) | `input_unreadable`, `input_permission_denied` | same | paired fixture and domain test (unsearchable case skipped as root; `EPERM` through the classifier seam) | | Detached linked worktree, no project, missing path | closed refusals | same | paired fixture | | Unknown service | `service_not_found` | same | paired fixture | | Config changed during read | `generation_changed_during_read` | same code path | domain test | diff --git a/src/lib/project-input-selection.ts b/src/lib/project-input-selection.ts index 1ddea5f7e..9f99cb312 100644 --- a/src/lib/project-input-selection.ts +++ b/src/lib/project-input-selection.ts @@ -57,17 +57,33 @@ export class ProjectInputSelectionError extends HackCliError { } } +/** Non-enumerable property holding a {@link ProjectInputAccessError}'s diagnostic. */ +const PROJECT_INPUT_ACCESS_DIAGNOSTIC_PROPERTY = "diagnosticCause"; +const DIAGNOSTIC_NAME = /^[A-Za-z][A-Za-z0-9]{0,63}$/; +const DIAGNOSTIC_CODE = /^[A-Z][A-Z0-9_]{0,63}$/; +const DIAGNOSTIC = /^[A-Za-z][A-Za-z0-9]{0,63}(?::[A-Z][A-Z0-9_]{0,63})?$/; +const DEFAULT_DIAGNOSTIC = "ProjectInputAccessError"; + /** * Stable, value-free refusal when a project input's presence cannot be * checked (for example a file where `.dev` belongs, or a project directory the * caller cannot search). Only ENOENT means absence, so the guard refuses * rather than guessing. `access` classifies the fault without its OS message, * which carries the absolute path and is therefore never kept. + * + * The fault itself is kept only as a value-free diagnostic: the class name and + * errno code of the underlying error (for example `Error:ENOTDIR`), never its + * message or path. It is a non-enumerable own property rather than `cause`, + * so envelopes, printed errors and serializers stay unchanged. Read it with + * {@link projectInputAccessDiagnostic}. */ export class ProjectInputAccessError extends HackCliError { readonly access: "permission" | "unreadable"; - constructor(access: "permission" | "unreadable") { + constructor( + access: "permission" | "unreadable", + diagnostic: string = DEFAULT_DIAGNOSTIC + ) { super({ code: "E_CONFIG_INVALID", message: @@ -75,7 +91,58 @@ export class ProjectInputAccessError extends HackCliError { }); this.name = "ProjectInputAccessError"; this.access = access; + Object.defineProperty(this, PROJECT_INPUT_ACCESS_DIAGNOSTIC_PROPERTY, { + value: DIAGNOSTIC.test(diagnostic) ? diagnostic : DEFAULT_DIAGNOSTIC, + enumerable: false, + }); + } +} + +/** + * The value-free diagnostic of a {@link ProjectInputAccessError} (for example + * `Error:EACCES`), for tests and debugging. Undefined for any other value. + */ +export function projectInputAccessDiagnostic( + error: unknown +): string | undefined { + if (!(error instanceof ProjectInputAccessError)) { + return undefined; + } + const diagnostic: unknown = Reflect.get( + error, + PROJECT_INPUT_ACCESS_DIAGNOSTIC_PROPERTY + ); + return typeof diagnostic === "string" && DIAGNOSTIC.test(diagnostic) + ? diagnostic + : undefined; +} + +/** + * The closed refusal for a fault other than ENOENT while checking whether a + * project input exists: `permission` for `EACCES`/`EPERM`, `unreadable` for + * any other fault, keeping only the fault's class/code diagnostic. Exported + * as a seam so faults a fixture cannot produce (such as `EPERM`) stay tested. + */ +export function projectInputAccessFault( + error: unknown +): ProjectInputAccessError { + const code = isRecord(error) ? error.code : undefined; + return new ProjectInputAccessError( + code === "EACCES" || code === "EPERM" ? "permission" : "unreadable", + valueFreeErrorDiagnostic(error) + ); +} + +/** Class name and identifier-shaped code of `error`; never its message. */ +function valueFreeErrorDiagnostic(error: unknown): string { + if (!(error instanceof Error)) { + return error === null ? "null" : typeof error; } + const name = DIAGNOSTIC_NAME.test(error.name) ? error.name : "Error"; + const code: unknown = Reflect.get(error, "code"); + return typeof code === "string" && DIAGNOSTIC_CODE.test(code) + ? `${name}:${code}` + : name; } /** @@ -209,12 +276,9 @@ async function inputPresent(path: string): Promise { await lstat(path); return true; } catch (error: unknown) { - const code = isRecord(error) ? error.code : undefined; - if (code === "ENOENT") { + if (isRecord(error) && error.code === "ENOENT") { return false; } - throw new ProjectInputAccessError( - code === "EACCES" || code === "EPERM" ? "permission" : "unreadable" - ); + throw projectInputAccessFault(error); } } diff --git a/src/lib/target-inspect.ts b/src/lib/target-inspect.ts index 2dd3f547b..43d76d21d 100644 --- a/src/lib/target-inspect.ts +++ b/src/lib/target-inspect.ts @@ -231,7 +231,8 @@ export const TARGET_REFUSALS = Object.freeze({ code: "E_PERMISSION", stage: "project", retry: "fix_project", - message: "A project input could not be read: permission denied.", + message: + "A project input could not be read: permission denied. Fix the permissions first: make the project directory (.hack or .dev), the directories inside it and its input files readable by your user, and the directories searchable (for example chmod u+rwx .hack), then retry.", }, input_unreadable: { code: "E_CONFIG_INVALID", diff --git a/tests/project-input-selection.test.ts b/tests/project-input-selection.test.ts index b9544e5b3..32244ef66 100644 --- a/tests/project-input-selection.test.ts +++ b/tests/project-input-selection.test.ts @@ -29,6 +29,8 @@ import { inspectProjectInputsAtRoot, NATIVE_PROJECT_FILENAME, ProjectInputAccessError, + projectInputAccessDiagnostic, + projectInputAccessFault, } from "../src/lib/project-input-selection.ts"; const roots: string[] = []; @@ -207,6 +209,26 @@ async function inputAccessFault(root: string): Promise { throw new Error("unexpected input selection success"); } +/** + * The access refusal keeps its fault only as a non-enumerable, value-free + * class/code diagnostic: never a `cause`, an enumerable field, the path or the + * OS message. + */ +function expectValueFreeDiagnostic( + error: unknown, + root: string, + diagnostic: string +): void { + expect(projectInputAccessDiagnostic(error)).toBe(diagnostic); + expect((error as Error).cause).toBeUndefined(); + expect(Object.keys(error as object)).not.toContain("diagnosticCause"); + const code = diagnostic.split(":")[1] ?? diagnostic; + for (const rendered of [String(error), JSON.stringify(error)]) { + expect(rendered).not.toContain(root); + expect(rendered).not.toContain(code); + } +} + test("a .dev file beside valid .hack inputs is a typed, value-free access refusal", async () => { const root = await fixture(); await file(root, ".hack/docker-compose.yml"); @@ -215,9 +237,47 @@ test("a .dev file beside valid .hack inputs is a typed, value-free access refusa expect(error).toBeInstanceOf(ProjectInputAccessError); expect((error as ProjectInputAccessError).access).toBe("unreadable"); expect((error as ProjectInputAccessError).code).toBe("E_CONFIG_INVALID"); - expect((error as Error).cause).toBeUndefined(); - expect(String(error)).not.toContain(root); - expect(String(error)).not.toContain("ENOTDIR"); + // HACK-1390: the fault's class/code stays attached for tests and debugging. + expectValueFreeDiagnostic(error, root, "Error:ENOTDIR"); +}); + +test("EPERM and EACCES classify as permission; other faults as unreadable", () => { + // A fixture cannot produce EPERM (a mode-000 directory gives EACCES), so the + // classifier inputPresent() uses is exercised through its seam. + const fault = (code: string) => + Object.assign( + new Error(`${code}: operation failed, lstat '/private/fixture/.dev'`), + { code, syscall: "lstat", path: "/private/fixture/.dev" } + ); + for (const [code, access] of [ + ["EPERM", "permission"], + ["EACCES", "permission"], + ["ENOTDIR", "unreadable"], + ["EIO", "unreadable"], + ] as const) { + const error = projectInputAccessFault(fault(code)); + expect(error).toBeInstanceOf(ProjectInputAccessError); + expect({ code, access: error.access }).toEqual({ code, access }); + expect(error.code).toBe("E_CONFIG_INVALID"); + expectValueFreeDiagnostic(error, "/private/fixture", `Error:${code}`); + } + // Anything that is not an identifier-shaped class or code is dropped. + expect( + projectInputAccessDiagnostic( + projectInputAccessFault( + Object.assign(new Error("x"), { code: "/private/fixture/.dev" }) + ) + ) + ).toBe("Error"); + expect( + projectInputAccessDiagnostic(projectInputAccessFault("/private/fixture")) + ).toBe("string"); + expect( + projectInputAccessDiagnostic( + new ProjectInputAccessError("unreadable", "/private/fixture/.dev") + ) + ).toBe("ProjectInputAccessError"); + expect(projectInputAccessDiagnostic(new Error("other"))).toBeUndefined(); }); // Root can search a mode-000 directory, so this fault cannot be produced. @@ -233,8 +293,7 @@ test.skipIf(process.getuid?.() === 0)( const error = await inputAccessFault(root); expect(error).toBeInstanceOf(ProjectInputAccessError); expect((error as ProjectInputAccessError).access).toBe("permission"); - expect(String(error)).not.toContain(root); - expect(String(error)).not.toContain("EACCES"); + expectValueFreeDiagnostic(error, root, "Error:EACCES"); } finally { await chmod(dev, 0o700); } diff --git a/tests/project-target-inspect.test.ts b/tests/project-target-inspect.test.ts index b9442a415..9f90e5fb6 100644 --- a/tests/project-target-inspect.test.ts +++ b/tests/project-target-inspect.test.ts @@ -1009,6 +1009,50 @@ describe("target inspector domain", () => { ); }); + // Root can search a mode-000 or mode-0644 directory, so this fault cannot be + // produced. + test.skipIf(process.getuid?.() === 0)( + "an unsearchable .hack or .hack/.internal names the permission fix, not adoption recovery", + async () => { + // HACK-1390: a project that never used adoption must not be told to run + // adoption recovery when its project directories cannot be searched. + const message = TARGET_REFUSALS.input_permission_denied.message; + expect(message).toContain("permission denied. Fix the permissions first"); + expect(message).toContain("chmod u+rwx .hack"); + expect(message.toLowerCase()).not.toContain("adoption"); + for (const [dir, mode] of [ + [".hack", 0o000], + [".hack", 0o644], + [".hack/.internal", 0o000], + [".hack/.internal", 0o644], + ] as const) { + const repo = await mkdtemp(join(fixture.root, "never-adopted-")); + await writeProject({ repo, name: "never-adopted", services: ["api"] }); + await mkdir(join(repo, ".hack", ".internal"), { recursive: true }); + const locked = join(repo, dir); + await chmod(locked, mode); + try { + const result = await inspectTarget({ + cwd: fixture.root, + registryPath: join(fixture.state, "projects.json"), + selector: { path: repo }, + }); + expect({ dir, mode, result }).toEqual({ + dir, + mode, + result: targetRefusalEnvelope({ + reason: "input_permission_denied", + }), + }); + expect(JSON.stringify(result)).not.toContain(repo); + expect(JSON.stringify(result)).not.toContain("EACCES"); + } finally { + await chmod(locked, 0o755); + } + } + } + ); + test("the MCP argv carries every selector as one --name=value token", () => { // The paired cases build the direct CLI argv with this same helper, so a // dropped selector would change both sides alike. Pin the argv literally.