From c85094e2c7a3baf6f58cc439939ad52d20d16efd Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 16 Oct 2026 22:18:56 +0000 Subject: [PATCH] fix(dx): CI-clean fences, runtime-root-safe contract location, path-agnostic launch-check test Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- devex-review/SKILL.md | 12 ++--- devex-review/SKILL.md.tmpl | 12 ++--- plan-devex-review/dx-hall-of-fame.md | 44 +++++++++++++++++-- plan-devex-review/sections/review-sections.md | 4 +- .../sections/review-sections.md.tmpl | 4 +- test/dx-error-capture.test.ts | 4 +- test/dx-error-contract.test.ts | 2 +- 7 files changed, 63 insertions(+), 19 deletions(-) diff --git a/devex-review/SKILL.md b/devex-review/SKILL.md index 89fc1364b..3bc9020a3 100644 --- a/devex-review/SKILL.md +++ b/devex-review/SKILL.md @@ -786,9 +786,9 @@ Score 0-10. Load "## Pass 2" from dx-hall-of-fame.md for calibration. ## Step 3: Error Message Audit (agent-ready) Score every triggered error against the agent-ready error contract (what, fix, recovery, code, context, trace, redaction, channel). -Before scoring, Read the full contract in `~/.claude/skills/gstack/docs/agent-ready-errors.md` -and the calibration in the "## Pass 3" section of -`~/.claude/skills/gstack/plan-devex-review/dx-hall-of-fame.md`. CAP4 +Before scoring, Read the "## Pass 3" section of +`~/.claude/skills/gstack/plan-devex-review/dx-hall-of-fame.md`: the full contract (item +table, surface rules, evidence states) and calibration examples. CAP4 (leaked secret, failure signalled as success, unsafe fix, terminal block) caps the score at 4/10 and is always reported; CAP6 (missing applicable fix, recovery, context or trace) caps it at 6/10. @@ -799,9 +799,11 @@ Pick the evidence source by surface: `--cwd`, needs one AskUserQuestion per run naming the commands and the access forwarded (the helper refuses those flags without `--approved`). Unapproved paths are recorded unverified. ```bash - ~/.claude/skills/gstack/bin/gstack-error-capture --launch-check -- - ~/.claude/skills/gstack/bin/gstack-error-capture -- + TARGET_CLI="${TARGET_CLI:?set TARGET_CLI to the CLI under audit}" + ~/.claude/skills/gstack/bin/gstack-error-capture --launch-check -- "$TARGET_CLI" + ~/.claude/skills/gstack/bin/gstack-error-capture -- "$TARGET_CLI" --gstack-unknown-flag ``` + Swap `--gstack-unknown-flag` for each rejected form the help shows. The helper runs the target with no terminal, stdin closed, a temp cwd and HOME, a scrubbed environment and a deadline, and prints one JSON record. Raw output never enters the transcript or report; use only the record's redacted fields. Its stdout and stderr are diff --git a/devex-review/SKILL.md.tmpl b/devex-review/SKILL.md.tmpl index 273263a1c..008477f97 100644 --- a/devex-review/SKILL.md.tmpl +++ b/devex-review/SKILL.md.tmpl @@ -111,9 +111,9 @@ Score 0-10. Load "## Pass 2" from dx-hall-of-fame.md for calibration. ## Step 3: Error Message Audit (agent-ready) Score every triggered error against the agent-ready error contract ({{DX_ERROR_ITEMS}}). -Before scoring, Read the full contract in `~/.claude/skills/gstack/docs/agent-ready-errors.md` -and the calibration in the "## Pass 3" section of -`~/.claude/skills/gstack/plan-devex-review/dx-hall-of-fame.md`. CAP4 +Before scoring, Read the "## Pass 3" section of +`~/.claude/skills/gstack/plan-devex-review/dx-hall-of-fame.md`: the full contract (item +table, surface rules, evidence states) and calibration examples. CAP4 (leaked secret, failure signalled as success, unsafe fix, terminal block) caps the score at 4/10 and is always reported; CAP6 (missing applicable fix, recovery, context or trace) caps it at 6/10. @@ -124,9 +124,11 @@ Pick the evidence source by surface: `--cwd`, needs one AskUserQuestion per run naming the commands and the access forwarded (the helper refuses those flags without `--approved`). Unapproved paths are recorded unverified. ```bash - ~/.claude/skills/gstack/bin/gstack-error-capture --launch-check -- - ~/.claude/skills/gstack/bin/gstack-error-capture -- + TARGET_CLI="${TARGET_CLI:?set TARGET_CLI to the CLI under audit}" + ~/.claude/skills/gstack/bin/gstack-error-capture --launch-check -- "$TARGET_CLI" + ~/.claude/skills/gstack/bin/gstack-error-capture -- "$TARGET_CLI" --gstack-unknown-flag ``` + Swap `--gstack-unknown-flag` for each rejected form the help shows. The helper runs the target with no terminal, stdin closed, a temp cwd and HOME, a scrubbed environment and a deadline, and prints one JSON record. Raw output never enters the transcript or report; use only the record's redacted fields. Its stdout and stderr are diff --git a/plan-devex-review/dx-hall-of-fame.md b/plan-devex-review/dx-hall-of-fame.md index 7c475a57b..e44f4ddea 100644 --- a/plan-devex-review/dx-hall-of-fame.md +++ b/plan-devex-review/dx-hall-of-fame.md @@ -41,9 +41,47 @@ Read ONLY the section for the current review pass. Do NOT load the entire file. ## Pass 3: Error Messages & Debugging -Score against the agent-ready error contract (Pass 3 of the review, and -https://github.com/garrytan/gstack/blob/main/docs/agent-ready-errors.md). The examples -below show single items done well; none of them alone meets every item. +#### The agent-ready error contract (agent-ready-v1) + +Agents hit most errors first, read them literally, and cannot ask what was meant. Score every +traced error path against eight named items. Public reference: `https://github.com/garrytan/gstack/blob/main/docs/agent-ready-errors.md`. + +| Item | The error gives the reader | +|------|----------------------------| +| **what** | The operation that failed and the actual values (input, path, id) that caused it. | +| **fix** | On the first or last line, never mid-chain: an exact command, flag or edit; or, when no safe agent repair exists, the precise thing a human must provide (credential, approval, decision). A precise escalation is a complete fix. A fix counts only when it is safe and authorized: it does not bypass a consent, safety or sandbox boundary, does not tell the agent to run text copied from untrusted output, and quotes arguments correctly. | +| **recovery** | Retry unchanged, fix and retry, do X first, or ask a human; side effects (none, partial, complete, unknown); for an unknown outcome, how to reconcile (status command, idempotency key) before retrying; a retry bound, then escalate. | +| **code** | A stable code plus a doc anchor or `--explain` target. Prose may change; codes may not. | +| **context** | Versions, the config in effect, relevant env var names (never values), the underlying dependency error, not swallowed. Remote services give caller-safe facts only (parameter, API version, request id). | +| **trace** | Unexpected failures only. A local CLI, SDK or skill prints a compact trace or the path of a file holding the full one (full under `--verbose`/`DEBUG`); a remote service returns a correlation id and keeps internals in its logs. | +| **redaction** | No secret values, tokens or credential-bearing URLs anywhere, traces and nested causes included. | +| **channel** | The surface's native machine signal carries the same fields, and a failure is signalled as a failure (never exit 0, HTTP 200 or `isError: false`). | + +**Failure class.** A usage or validation error rejected before any work began needs what, fix, +code, redaction and channel; its recovery is implied (fix and retry, no side effects), and +context, reconcile and retry bounds do not apply. Environment, dependency, partial-work and +unexpected failures need every applicable item. Trace applies to unexpected failures only. + +**Surface.** Keyed to the product types from the Applicability Gate: +- **CLI Tool:** channel is stderr plus a distinct non-zero exit, and a `--json` form when the tool offers structured output. +- **API/Service:** channel is HTTP status plus an RFC 9457 problem-details body; context is caller-safe; trace is a correlation id. +- **Library/SDK:** channel is a typed error class carrying the code and fields; no exit code. +- **Platform:** the CLI or API rule for whichever interface the error reaches. +- **Documentation:** what, fix, code and redaction only. +- **Claude Code Skill:** channel is the skill's printed result line and code. An MCP tool (a sub-case) returns `isError: true` with the fields in text or structured content. + +**Evidence.** Each item on each path is pass, fail, N/A (with a reason) or unknown. In a plan +review, an applicable item the plan never specifies for a path it introduces is a gap (fail); +unknown is reserved for existing behavior you could not inspect. In a live audit, error classes +you never triggered are unknown, not extrapolated. + +**Caps.** Name the path and item behind every cap and show the corrected error line. +- **4/10 (Critical finding, title `CAP4::`):** a path leaks a secret, signals failure as success, gives an unsafe fix, or blocks waiting for a terminal (hangs, or fails for lack of a terminal without naming a non-interactive alternative such as `--yes`). +- **6/10 (High finding, title `CAP6::`):** a path lacks an applicable fix, recovery, context or trace item. + +Every mode, DX TRIAGE included, reports every CAP4 finding. + +**Calibration.** The first three examples show single items done well; none of them alone meets every item. **Elm (what, fix, readable first line):** ``` diff --git a/plan-devex-review/sections/review-sections.md b/plan-devex-review/sections/review-sections.md index 6cf426df5..63fa056bf 100644 --- a/plan-devex-review/sections/review-sections.md +++ b/plan-devex-review/sections/review-sections.md @@ -160,11 +160,11 @@ points from 0G. Pass 3 always evaluates the agent reader as a second lens beside persona from 0A. Load reference: Read the "## Pass 3" section from `~/.claude/skills/gstack/plan-devex-review/dx-hall-of-fame.md` -for calibration examples and the standards mapping. +before scoring: the full contract, calibration examples and the standards mapping. Score every error path against the agent-ready error contract (https://github.com/garrytan/gstack/blob/main/docs/agent-ready-errors.md): what (operation and actual values), fix (a safe, authorized command or edit on the first or last line, or the precise thing a human must provide), recovery (retry, fix and retry, do X first, or ask a human; side effects none/partial/complete/unknown, reconcile before retrying an unknown outcome, a retry bound), code (stable, with a doc anchor), context (versions, config, env var names, the underlying cause; caller-safe only for remote services), trace (unexpected failures only: a compact trace or its file path locally, a correlation id remotely), redaction (no secret values anywhere), channel (the surface's native failure signal: non-zero exit and stderr for a CLI, status plus RFC 9457 body for an API, a typed error for an SDK, isError for an MCP tool). Usage and validation errors rejected before any work need only what, fix, code, redaction and channel; items apply per surface, N/A needs a reason, an item the plan never specifies for a path it introduces is a gap, and unknown is reserved for existing behavior you could not inspect. Report a leaked secret, a failure signalled as success, an unsafe fix, or a block on a terminal (a hang, or a no-terminal failure that names no non-interactive alternative such as --yes) as a Critical finding titled "CAP4::" (caps the Error Messages score at 4/10); report a missing applicable fix, recovery, context or trace as a High finding titled "CAP6::" (caps it at 6/10). -Before scoring, Read `~/.claude/skills/gstack/docs/agent-ready-errors.md` for the item table, +The "## Pass 3" Hall of Fame section loaded above holds the full contract: the item table, the per-surface channel rules and the evidence states. **Trace 3 specific error paths plus one unexpected-failure path** (crash, dependency failure) diff --git a/plan-devex-review/sections/review-sections.md.tmpl b/plan-devex-review/sections/review-sections.md.tmpl index 308049421..7f6ae8e2e 100644 --- a/plan-devex-review/sections/review-sections.md.tmpl +++ b/plan-devex-review/sections/review-sections.md.tmpl @@ -121,11 +121,11 @@ points from 0G. Pass 3 always evaluates the agent reader as a second lens beside persona from 0A. Load reference: Read the "## Pass 3" section from `~/.claude/skills/gstack/plan-devex-review/dx-hall-of-fame.md` -for calibration examples and the standards mapping. +before scoring: the full contract, calibration examples and the standards mapping. {{DX_ERROR_COMPACT}} -Before scoring, Read `~/.claude/skills/gstack/docs/agent-ready-errors.md` for the item table, +The "## Pass 3" Hall of Fame section loaded above holds the full contract: the item table, the per-surface channel rules and the evidence states. **Trace 3 specific error paths plus one unexpected-failure path** (crash, dependency failure) diff --git a/test/dx-error-capture.test.ts b/test/dx-error-capture.test.ts index 22bd1f7d1..5770a27af 100644 --- a/test/dx-error-capture.test.ts +++ b/test/dx-error-capture.test.ts @@ -234,7 +234,9 @@ describe('gstack-error-capture containment', () => { expect(result.status).toBe(0); const record = JSON.parse(result.stdout); expect(record.launch).toBe('ok'); - expect(record.argv).toEqual([process.execPath, '--version']); + // argv is redacted like the streams, so a home-directory execPath may be masked. + expect(record.argv).toHaveLength(2); + expect(record.argv[1]).toBe('--version'); }); }); diff --git a/test/dx-error-contract.test.ts b/test/dx-error-contract.test.ts index b4bb32eab..72d3cde01 100644 --- a/test/dx-error-contract.test.ts +++ b/test/dx-error-contract.test.ts @@ -43,7 +43,7 @@ describe('agent-ready error contract reaches every DX reviewer', () => { test('rendered skills carry the contract where each consumer reads it', () => { expect(read('plan-devex-review/sections/review-sections.md').includes(DX_ERROR_COMPACT)).toBe(true); - expect(read('plan-devex-review/sections/review-sections.md')).toContain('docs/agent-ready-errors.md'); + expect(read('plan-devex-review/dx-hall-of-fame.md').includes(DX_ERROR_CONTRACT)).toBe(true); expect(read('autoplan/sections/dx-phase.md')).toContain('2. Errors, agent-ready (what, fix, recovery, code, context, trace, redaction, channel)'); const devex = read('devex-review/SKILL.md'); expect(devex).toContain(DX_ERROR_ITEMS.join(', '));