diff --git a/.claude/agent-memory/archgate-developer/MEMORY.md b/.claude/agent-memory/archgate-developer/MEMORY.md index cc5d09a8..633a4d91 100644 --- a/.claude/agent-memory/archgate-developer/MEMORY.md +++ b/.claude/agent-memory/archgate-developer/MEMORY.md @@ -8,7 +8,7 @@ Every work loop MUST end with these steps — no exceptions, even for trivial ch 2. **`@reviewer` skill** — Invoke via `Skill tool` with skill `"archgate:reviewer"`. Validates structural ADR compliance beyond automated rules. 3. **`@lessons-learned` skill** — Invoke via `Skill tool` with skill `"archgate:lessons-learned"`. Captures learnings and governance gaps. -Skipping steps 2 or 3 is a workflow violation. The user should NEVER have to invoke these manually. (Note: `.claude/settings.json` still allowlists the older `archgate:architect`/`archgate:quality-manager` names — update it to match.) +Skipping steps 2 or 3 is a workflow violation. The user should NEVER have to invoke these manually. (`.claude/settings.json` permissions now correctly allowlist `archgate:reviewer`/`archgate:lessons-learned`/`archgate:adr-author` — the older `archgate:architect`/`archgate:quality-manager` naming issue noted here previously has been fixed; verified 2026-07-01.) ## Version References @@ -60,6 +60,11 @@ Non-enforceable lessons — environment/CI/platform quirks no static rule can re - **`Bun.Glob.scan()` silently fails for brace patterns with path separators** — `new Bun.Glob("svc/{src/env.ts,env.ts}").scan(...)` returns zero results (no error), while `.match()` works correctly for the same pattern. The match engine was rewritten in Bun 1.2.3 (PR #16824) to expand braces recursively, but the scanner (`GlobWalker.zig`) was not updated. Filed upstream as [oven-sh/bun#32596](https://github.com/oven-sh/bun/issues/32596). Workaround: `expandBracePattern()` in `src/engine/runner.ts` pre-expands brace groups containing `/` before scanning. Applied in `ctx.glob()`, `ctx.grepFiles()`, and `resolveScopedFiles()`. - **ARCH-020 `glob-scan-dot` rule triggers on `.scan()` in comments** — The rule uses regex `/\.scan\(([^)]*)\)/gu` which matches `.scan()` text in JSDoc/inline comments (e.g., `Bun.Glob.scan() silently...`). Workaround: rephrase comments to avoid the exact `.scan()` text — e.g., "Bun.Glob scanning silently..." instead of "Bun.Glob.scan() silently...". +## Claude Code Harness Config + +- [WorktreeCreate hook contract](project_worktree_create_hook_contract.md) — stdin JSON in, path-only stdout out; hook owns the entire worktree creation once configured, not just post-setup +- [Cursor Approval Agent is external, not in-repo](reference_cursor_approval_agent.md) — "Archgate CLI Approver" automation lives on cursor.com; no APPROVAL_POLICY.md/ROUTING.md exist in this repo + ## Translation Quality - **Docs have TWO locales: `nb/` AND `pt-br/`** — Editing any English docs page requires updating BOTH `docs/src/content/docs/nb/` and `docs/src/content/docs/pt-br/` in the same changeset (GEN-002 `i18n-translation-drift` is an error-severity rule). Don't stop at nb. Also: locale pages can silently lack whole sections present in English (e.g., a section might exist in English but be entirely absent from a locale page) — the drift rule only checks that the file was touched, not content parity, so compare section structure when editing. diff --git a/.claude/agent-memory/archgate-developer/project_worktree_create_hook_contract.md b/.claude/agent-memory/archgate-developer/project_worktree_create_hook_contract.md new file mode 100644 index 00000000..75e693fe --- /dev/null +++ b/.claude/agent-memory/archgate-developer/project_worktree_create_hook_contract.md @@ -0,0 +1,29 @@ +--- +name: project-worktree-create-hook-contract +description: How the Claude Code WorktreeCreate hook contract works — stdin JSON in, path-only stdout out — and how the repo's hook was broken/fixed +metadata: + type: project +--- + +`.claude/settings.json`'s `hooks.WorktreeCreate` hook was broken from ~Feb 2026 (or earlier) until fixed on 2026-07-01: it only ran `bun install`, which is not enough — once a `WorktreeCreate` hook is configured, the harness defers the **entire** worktree creation to it (no automatic git worktree creation happens in parallel). Symptom reported by the user: `WorktreeCreate hook failed: path contains control characters`; empirically also produced `ENOENT: ... chdir 'E:\archgate\cli\' -> ''`. + +**Why:** Diagnosed by dumping the hook's stdin/env/args to a side-channel file (`{ echo ...; cat; env; } > /tmp/debug.txt; pwd`) and by calling `EnterWorktree` directly to observe raw harness errors. Found: (1) the hook receives a JSON payload on **stdin** — `{"cwd", "name", "session_id", ...}` — same pattern as the existing `PostToolUse` hook that reads `.tool_input.file_path` via `jq`; (2) the harness error `hook succeeded but returned no worktree path (command: echo the path to stdout; ...)` reveals the hook **must** print only the resulting absolute worktree path as its final stdout line; (3) any other stdout (unsilenced `bun install`, `git worktree add` banner text) gets misread by the harness as the path, corrupting session state (`ExitWorktree` later reported removing a worktree "at Checked 177 installs... [28ms]" — the literal bun install stdout). + +**Evidence of prior breakage:** `.claude/worktrees/` had 21 stale, completely empty leftover directories (no `.git` file, no content) alongside orphaned local branches like `claude/wonderful-bose-0f7e45` — confirming `git worktree add` used to run (via some earlier, more complete hook version) but the directory was left behind when path corruption broke cleanup. + +**How to apply:** The fixed hook (see `CLAUDE.md` "Claude Code Harness Config" section) parses `.name` from stdin via `jq`, creates `git worktree add "$CLAUDE_PROJECT_DIR/.claude/worktrees/$name" -B "claude/$name"`, redirects ALL setup command output to stderr (`>&2`), and ends with `printf '%s\n' "$dir"` as the only real stdout. Verified end-to-end via `EnterWorktree`/`ExitWorktree` — worktree created with full file content + `node_modules`, session cwd matched, clean removal. Note: `git worktree remove` does not delete the branch — `claude/` branches accumulate locally over time; periodic `git branch -D` cleanup of merged/abandoned `claude/*` branches is a reasonable manual chore, not automated by anything. + +If this hook is ever "simplified" back to a bare `bun install`, the exact same bug returns. + +**Round 2 (same PR, caught by Cursor Bugbot on [PR #441](https://github.com/archgate/cli/pull/441)):** My first fix had two more bugs, both confirmed by direct reproduction before fixing: + +1. **Stale-dir skip** — `if [ ! -d "$dir" ]` skipped `git worktree add` whenever the target path already existed, even as an empty non-worktree leftover (exactly the kind of debris this same PR documents finding 21 of). Repro: `mkdir -p .claude/worktrees/x; echo '{"name":"x"}' | bash -c "$HOOK_CMD"` produced exit 0 and a printed path, but no real checkout. Fix: check for `"$dir/.git"` and `rm -rf "$dir"` first if it's a stale non-worktree directory, before the create-if-missing check. +2. **CRLF in `jq -r` output** — on Windows Git Bash, `jq -r` can emit CRLF; `$(...)` command substitution strips only the trailing `\n`, leaving a `\r` in `name`/`dir`/the final stdout path — reproducing the exact "path contains control characters" bug this PR set out to fix. This is the same pattern already documented under "jq on Windows Git Bash emits CRLF line endings" in the top-level MEMORY.md (bit `install.sh` before). Repro: wrapped `jq` in a PATH-shadowing stub that appends `\r` to its output, ran the hook end-to-end, verified via `cat -A`/byte-length comparison (not naive `grep '\r'` — that gave a false positive against `od -c` text output). Fix: `name=$(jq -r '.name // empty' | tr -d '\r')`. + +**Lesson: reproduce Bugbot/reviewer findings against the actual script before trusting or dismissing them.** Both were true positives, verified by running the real hook command through simulated failure conditions (pre-existing stale dir; CRLF-emitting jq stub) rather than just reasoning about the code. One dead end worth noting: `grep -q '\r'` against `od -c` output is NOT a reliable way to detect an embedded carriage return byte (od's own text formatting can produce a false match) — use `cat -A` (shows `^M`) or compare `wc -c` byte length to the expected clean string instead. + +**Round 3 (same PR, Bugbot re-ran on the round-2 fix commit):** Flagged that `bun install --silent` failures were silently swallowed — no `|| exit 1` check, so the hook always ended with `printf` and exit 0 regardless of install outcome. Repro: created a worktree, corrupted its `package.json` to force a parse error, re-ran the hook — confirmed exit 0 despite `bun install`'s own error printing to stderr. **Deliberately did not hard-fail** (`|| exit 1`) here, unlike the `git worktree add` step: by the time `bun install` runs, the git worktree already exists and may be a _reused_ existing worktree with real uncommitted work in it (not just a fresh one) — hard-failing without cleanup would orphan directories again (round-2's bug #1), and adding cleanup would risk deleting real work on a transient `bun install` failure (e.g., a network blip) if the worktree was being reused rather than freshly created. Instead added an explicit `warning: bun install failed in $dir -- ...` line to stderr on failure (`bun install --silent >&2 || echo "warning: ..." >&2`), verified it appears via the same corrupt-package.json repro, and verified the happy path produces no spurious warning. This is a case of fixing the underlying "silently swallowed" problem the finding correctly identified, while explicitly disagreeing with the literal "hook should fail" framing — explained the reasoning in the reply rather than just complying. + +**Round 4 (same PR, Bugbot re-ran again):** Flagged that `git worktree add` runs without a preceding `git worktree prune`, so if a worktree's directory was ever deleted without going through `git worktree remove` (crash, manual `rm -rf`, disk cleanup) git still has it registered internally (shows as `prunable` in `git worktree list`) and `git worktree add` at that path/branch fails outright with `fatal: '' is already used by worktree at ''` — even though `[ ! -d "$dir" ]` is true (directory is gone) and my round-2 stale-dir check doesn't catch it either (that check only fires when the directory _exists_ but lacks `.git`; here the directory doesn't exist at all, only git's internal metadata does). Repro: created a worktree, `rm -rf`'d just the directory (not `git worktree remove`), re-ran the hook with the same name — confirmed exit 1 with that exact fatal error. Fix: `git worktree prune >&2` immediately before `git worktree add`, cheap and safe to run unconditionally every time. Verified the repro now succeeds, plus regression-tested round-2's stale-dir case and the plain happy path still work. + +**Running tally: 4 rounds of Bugbot findings on one hook, all true positives, all found by actually reproducing before fixing.** This hook is deceptively easy to get subtly wrong — small shell script, but git worktree state has more failure modes (missing dir, stale dir, stale git registration, failed dep install) than are obvious up front. If touching this hook again, re-run all four repros above as a regression suite before pushing, not just the happy path. diff --git a/.claude/agent-memory/archgate-developer/reference_cursor_approval_agent.md b/.claude/agent-memory/archgate-developer/reference_cursor_approval_agent.md new file mode 100644 index 00000000..27d2b3a4 --- /dev/null +++ b/.claude/agent-memory/archgate-developer/reference_cursor_approval_agent.md @@ -0,0 +1,14 @@ +--- +name: reference-cursor-approval-agent +description: PRs go through an external Cursor.com "Archgate CLI Approver" automation not implemented anywhere in this repo — where to look when its approval fails +metadata: + type: reference +--- + +PRs in `archgate/cli` are evaluated by a custom **Cursor Automation** named "Archgate CLI Approver" (visible on PR checks as "Cursor Approval Agent: Archgate CLI Approver", linked to `https://cursor.com/automations/`). It is entirely external/hosted on cursor.com — **not** implemented as a GitHub Actions workflow or any file in this repository. Confirmed by exhaustive search: no `APPROVAL_POLICY.md`, no `cursor/approval-policies/ROUTING.md`, no `.cursor/` policy files, and no workflow under `.github/workflows/` references "cursor" or "bugbot" (checked 2026-07-01). + +**Why:** The agent's own PR comments say it looks for `APPROVAL_POLICY.md` / `cursor/approval-policies/ROUTING.md` in-repo to customize its behavior, and falls back to a default heuristic otherwise: block auto-approval unless it finds a `cursor[bot]` review comment containing the marker ``. Observed failure mode on [PR #439](https://github.com/archgate/cli/pull/439): Cursor Bugbot's check passed with a clean SUCCESS conclusion but posted **no** review comment at all (plausibly because it found zero issues to flag — nothing to say). The Approval Agent's fallback couldn't distinguish "Bugbot ran clean" from "Bugbot's comment hasn't posted yet," so it conservatively left a non-approving comment instead of approving. The PR still merged fine (author is sole CODEOWNER); this is a soft/non-blocking signal, not a broken merge. + +**Resolution (2026-07-01):** Confirmed via Cursor's own docs (`cursor.com/docs/bugbot`) that a `success` Bugbot check conclusion legitimately means "no issues found, no unresolved comments" — Bugbot does NOT always post a comment, so "check passed, no comment" is the documented happy path, not an ambiguous/unverified signal. Also confirmed via `cursor.com/docs/cloud-agent/automations` that Cursor Automations have no built-in/documented repo-policy-file mechanism — the `APPROVAL_POLICY.md` lookup is bespoke logic this specific automation's author coded into its own prompt (not a Cursor platform feature), so it's genuinely repo-controllable. Created [APPROVAL_POLICY.md](../../../APPROVAL_POLICY.md) at the repo root (after asking the user, per their explicit "read the docs and bring a solution" request) documenting that a `success` Bugbot check alone is sufficient for approval regardless of comment presence, while still requiring human review when the check is missing/pending/failed or Bugbot left unresolved comments. Not yet verified against a live automation run — the automation's prompt might not actually read this file (its "policy file" search was self-reported, not independently confirmed); watch the next PR's approval-agent comment to see if it picks this up, and if not, the fallback is editing the automation's prompt directly on cursor.com (only the account owner has access). + +**How to apply:** If a future "Cursor approval failing" report comes in for a DIFFERENT failure mode (not the missing-comment case above), don't assume `APPROVAL_POLICY.md` covers it — re-check what specific signal the automation says it's missing, since its policy-reading behavior is unverified/undocumented and may not generalize. diff --git a/.claude/settings.json b/.claude/settings.json index 4d07b5dd..95b86078 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -41,7 +41,7 @@ "hooks": [ { "type": "command", - "command": "bun install", + "command": "name=$(jq -r '.name // empty' | tr -d '\\r'); [ -z \"$name\" ] && name=\"wt-$$\"; dir=\"$CLAUDE_PROJECT_DIR/.claude/worktrees/$name\"; branch=\"claude/$name\"; if [ -d \"$dir\" ] && [ ! -e \"$dir/.git\" ]; then rm -rf \"$dir\"; fi; if [ ! -d \"$dir\" ]; then git worktree prune >&2; git worktree add \"$dir\" -B \"$branch\" >&2 || exit 1; fi; cd \"$dir\" || exit 1; bun install --silent >&2 || echo \"warning: bun install failed in $dir -- dependencies may be missing, run 'bun install' manually\" >&2; printf '%s\\n' \"$dir\"", "timeout": 60, "statusMessage": "Installing dependencies in worktree..." } diff --git a/APPROVAL_POLICY.md b/APPROVAL_POLICY.md new file mode 100644 index 00000000..f0ca60d5 --- /dev/null +++ b/APPROVAL_POLICY.md @@ -0,0 +1,34 @@ +# PR Approval Policy + +This file is read by the **Cursor Approval Agent ("Archgate CLI Approver")** automation that evaluates pull requests in this repository. It exists to resolve one specific ambiguity in that automation's default behavior; everything not covered here falls back to the automation's own judgment. + +## Bugbot signal: a passing check with no comment is a valid, complete signal + +Per [Cursor's own Bugbot documentation](https://cursor.com/docs/bugbot), the `success` conclusion on the "Cursor Bugbot" check means: + +> Bugbot found no issues, and there are no unresolved Bugbot comments from earlier runs. + +In other words, **Bugbot does not always post a PR review comment.** When it finds nothing to flag, it completes with a `success` check and posts no comment at all — that is the documented, expected happy path, not a sign that the review didn't run or that its output is unverified. + +**Do:** + +- Treat the "Cursor Bugbot" check's own conclusion (`success` / `failure` / etc.) as authoritative on its own. +- Treat `success` with zero new review comments as equivalent to "no issues found" — this alone is sufficient to approve, it does not require also finding a `cursor[bot]` comment containing a `` marker or any other comment-based confirmation. + +**Don't:** + +- Don't block approval solely because no Bugbot review comment appeared, if the Bugbot check itself reports `success`. +- Don't poll for a comment as a liveness check for Bugbot — use the check's status/conclusion instead. + +## When to still hold back for human review + +This policy narrows one specific false-negative case. All other conservative defaults still apply — continue requiring human review when: + +- The "Cursor Bugbot" check is missing, pending, `failure`, `error`, or otherwise not `success`. +- Bugbot posted one or more unresolved review comments on the PR. +- Any other required status check (lint, test, typecheck, build, etc.) has not passed. +- Anything else about the PR looks ambiguous or out of scope for automated approval — when in doubt, leave a non-blocking comment rather than approve. + +## Scope + +This policy covers only the Bugbot-signal-detection step of the approval decision. It does not change reviewer assignment, CODEOWNERS behavior, or any other part of the approval workflow. diff --git a/CLAUDE.md b/CLAUDE.md index 1776b08a..6afd5d25 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,6 +43,10 @@ git config --local include.path ../.githooks Opt out of a specific hook: `git config --local hook..enabled false`. Skip all hooks for a single commit: `git commit --no-verify`. +## Claude Code Harness Config (`.claude/settings.json`) + +The `hooks.WorktreeCreate` entry is **not** a post-creation setup step — once it's configured, the Claude Code harness defers the _entire_ worktree creation to it (it does not also create a git worktree on its own). The hook receives a JSON payload on stdin (`{ "cwd", "name", ... }`, same pattern as the `PostToolUse` hook reading `.tool_input.file_path` via `jq`) and **must** create the worktree itself and echo _only_ the resulting absolute path as its final stdout line — any other stdout (e.g. unsilenced `bun install` or `git worktree add` output) gets misread as the path and breaks `EnterWorktree`/`ExitWorktree` with errors like `path contains control characters` or `ENOENT: ... chdir`. Redirect all setup-command output to stderr (`>&2`) and keep the trailing `printf` as the only real stdout. Do not simplify this hook back down to a bare `bun install` — that regression is exactly what caused the worktree-creation bug fixed here. + ## Architecture ### Commands