From 8dcbfbfc775f96a96fc49c7e69bb2836d971d5fc Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 18:59:25 +0000 Subject: [PATCH 1/7] feat: align agent stacks (Claude, Cursor) on shared bootstrap Unifies coding-agent setup across Claude Code and Cursor so both stacks get: - Pin-matched bd from flake.nix - Fresh Dolt beads tracker sync (not stale JSONL export) - Same skills via symlinks to .agents/skills/ Key changes: 1. scripts/agent-bootstrap.sh: stack-agnostic bootstrap that handles bd installation, tracker sync, and optional cargo warming. Works for any agent stack (Claude, Cursor, or manual). 2. .claude/hooks/session-start.sh: now a thin wrapper that calls the shared bootstrap for cloud sessions, keeping only the local pin-check inline. 3. .cursor/environment.json: configures Cursor cloud agents to run the bootstrap via 'install'. 4. .cursor/skills/: symlinks to .agents/skills/* so Cursor agents discover the same skills Claude has. 5. AGENTS.md: adds Multi-Stack Agent Support section; clarifies that scripts/kb is the KB interface (Prolog MCP is not required and not available in all stacks). 6. context/cloud-agents.md: updated for multi-stack support; added KB section emphasizing scripts/kb over Prolog MCP. The canonical skill source remains .agents/skills/. Both .claude/skills/ and .cursor/skills/ are symlinks to it, so there is one truth that both stacks read. Co-authored-by: Ed Langley --- .claude/hooks/session-start.sh | 291 ++++----------------------------- .claude/skills/beads | 1 + .cursor/environment.json | 5 + .cursor/skills/beads | 1 + .cursor/skills/beads-merge | 1 + .cursor/skills/code-review | 1 + .cursor/skills/kb | 1 + .cursor/skills/pr-watch | 1 + .cursor/skills/wave | 1 + AGENTS.md | 28 +++- context/cloud-agents.md | 41 ++++- scripts/agent-bootstrap.sh | 222 +++++++++++++++++++++++++ 12 files changed, 331 insertions(+), 263 deletions(-) create mode 120000 .claude/skills/beads create mode 100644 .cursor/environment.json create mode 120000 .cursor/skills/beads create mode 120000 .cursor/skills/beads-merge create mode 120000 .cursor/skills/code-review create mode 120000 .cursor/skills/kb create mode 120000 .cursor/skills/pr-watch create mode 120000 .cursor/skills/wave create mode 100755 scripts/agent-bootstrap.sh diff --git a/.claude/hooks/session-start.sh b/.claude/hooks/session-start.sh index 52b0e9c2..3f055531 100755 --- a/.claude/hooks/session-start.sh +++ b/.claude/hooks/session-start.sh @@ -1,273 +1,50 @@ #!/bin/bash # Prepare a Claude Code on the web container for work on this repository. # -# Section 0 runs everywhere, cloud or not: it asserts that the `bd` on PATH -# is the one flake.nix pins. Everything after it is cloud-only setup. +# This hook delegates to the shared `scripts/agent-bootstrap.sh`, which works +# for any coding agent stack (Claude Code, Cursor, etc.). This file exists +# because Claude Code expects hooks at `.claude/hooks/session-start.sh`. # -# Three things are slow or wrong by default in a fresh cloud session: +# For local checkouts, only the bd pin assertion runs. +# For cloud sessions (CLAUDE_CODE_REMOTE=true), full bootstrap runs. # -# 1. `bd` is not installed, so the `bd prime` hook and every `bd` command -# in AGENTS.md silently do nothing. -# 2. `.beads/issues.jsonl` is a stale export. An agent that reads it picks -# work that is already done — this has happened. The authoritative -# tracker is the Dolt database on the git remote. -# 3. The Rust workspace is uncompiled, so the first `cargo test` pays a -# cold build before it reports anything. -# -# The tracker is re-synced on every session, not just when the database is -# missing: cloud containers reuse a cached workspace, so "already present" -# does not mean "still current", and a silently ageing tracker is the exact -# failure this hook exists to prevent. Installing bd and warming the build -# do check first, being genuinely one-time. -# -# Nothing here writes to a tracked file, stages anything, or creates a commit. +# See also: scripts/agent-bootstrap.sh (the stack-agnostic implementation) set -uo pipefail ROOT="${CLAUDE_PROJECT_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)}" -cd "$ROOT" || exit 0 +BOOTSTRAP="$ROOT/scripts/agent-bootstrap.sh" -log() { printf '[session-start] %s\n' "$*"; } - -# ── 0. The bd on PATH must be the bd flake.nix pins ──────────────────────── -# `flake.nix` pins beads so that one build of `bd` touches this repo. A git -# worktree does not load that pin — a fresh `.envrc` is unallowed, so a -# process there falls through to whatever `bd` the user's profile happens to -# carry. Today those agree; nothing enforces that they keep agreeing. -# -# The failure that matters is silent: writing .beads/issues.jsonl with a -# skewed build empties the dependency graph and reports success. Nobody -# notices until they ask what blocks what. So the mismatch is turned into a -# loud one here rather than trusted. Reading with a skewed bd is safe — it -# is the mutating commands that corrupt — so this blocks nothing, it shouts. -# -# The expected version is read from flake.nix, not written down twice: a -# hard-coded copy would go stale the first time the pin moves, and a stale -# assertion is worse than none. -beads_pin() { - sed -n 's|.*beads\.url[[:space:]]*=[[:space:]]*"github:[^/]*/[^/]*/v\{0,1\}\([^"]*\)".*|\1|p' \ - flake.nix 2>/dev/null | head -1 -} - -bd_running_version() { - bd version 2>/dev/null | sed -n 's/^bd version \([0-9][0-9.]*\).*/\1/p' | head -1 -} - -bd_pin_banner() { - { - printf '\n[session-start] ===== BEADS VERSION SKEW =====\n' - printf '[session-start] %s\n' "$@" - printf '[session-start] =============================\n\n' - } >&2 -} - -# Prints the banner and returns 1 when the two disagree, or when either one -# cannot be read — an unreadable answer is a rotted assertion, not a pass. -assert_bd_pin() { - command -v bd >/dev/null 2>&1 || return 0 # nothing on PATH to assert about - - pinned=$(beads_pin) - running=$(bd_running_version) - - case "$pinned" in - [0-9]*.[0-9]*.[0-9]*) ;; - *) - bd_pin_banner "cannot read the beads pin out of flake.nix" \ - "read: '${pinned:-}'" \ - "The pin moved to a ref this hook cannot parse (a branch or a" \ - "commit, perhaps). Teach beads_pin() to read it, or the check" \ - "silently protects nothing." - return 1 - ;; - esac - - if [ -z "$running" ]; then - bd_pin_banner "cannot read the version of the bd on PATH" \ - "bd: $(command -v bd)" \ - "'bd version' did not answer in the expected shape. Until it does," \ - "nothing here can tell whether that build matches the pin." - return 1 - fi - - [ "$running" = "$pinned" ] && return 0 - - bd_pin_banner "the bd on PATH is NOT the bd this repo pins" \ - "pinned (flake.nix): $pinned" \ - "running: $running ($(command -v bd))" \ - "" \ - "Reading is safe. Do NOT run a mutating bd command — create, update," \ - "close, dep, dolt push. Version skew between builds empties the" \ - "dependency graph in .beads/issues.jsonl and reports success." \ - "" \ - "Fix now: nix develop (in this directory), then re-run the" \ - " command; or 'direnv allow' here." \ - "Fix for good: worktrees are created fresh per dispatch, so allowing" \ - " them one at a time never catches the next one. Put the" \ - " repo root under [whitelist] prefix in" \ - " ~/.config/direnv/direnv.toml." - return 1 -} - -pin_status=0 -assert_bd_pin || pin_status=2 - -# Local checkouts have their own setup; this is for the web containers. -if [ "${CLAUDE_CODE_REMOTE:-}" != "true" ]; then - exit "$pin_status" -fi - -# Install a verified database at .beads/embeddeddolt. -# -# The staged copy lives on another filesystem, so it is brought alongside the -# live one FIRST. Every step after that is a same-filesystem rename, which -# either happens or does not — no half-finished copy. The previous database is -# removed only once the new one is in place, so no failure path leaves the repo -# without a tracker, and every move is checked: an unchecked one would let the -# new database land *inside* the old directory and still report success. -install_tracker() { - staged=$1 - live=.beads/embeddeddolt - incoming=$live.incoming - previous=$live.previous - - rm -rf "$incoming" "$previous" || return 1 - cp -rf "$staged" "$incoming" || { rm -rf "$incoming"; return 1; } - - if [ -d "$live" ] && ! mv "$live" "$previous"; then - rm -rf "$incoming" - return 1 - fi - - if mv "$incoming" "$live"; then - rm -rf "$previous" - return 0 - fi - - # Could not put the new one in place; restore what was there. - if [ -d "$previous" ] && ! mv "$previous" "$live"; then - log "ERROR: tracker left at $previous — move it back to $live by hand" - fi - rm -rf "$incoming" - return 1 -} - -# ── 1. bd on PATH ────────────────────────────────────────────────────────── -# The embedded Dolt engine needs CGO, so this builds from source (~3 min). -# The container image is cached after the hook, so it is paid once. -export PATH="$PATH:/root/go/bin:$HOME/go/bin" -# Pinned to a commit rather than `main`, and downloaded before it is run: -# piping a moving URL straight into a shell executes code nobody has seen, -# and leaves nothing to inspect afterwards. The installer stays at -# /tmp/bd-install.sh so what actually ran can be read. -# -# To take a newer beads, resolve the peeled commit of its tag: -# git ls-remote --tags https://github.com/steveyegge/beads | grep 'v1.3.0^{}' -BD_INSTALLER_REF=f45b249ce6b40ba62aecc03949e6371e8f7c79d8 # tag v1.3.0 -BD_INSTALLER_URL="https://raw.githubusercontent.com/steveyegge/beads/$BD_INSTALLER_REF/scripts/install.sh" - -if ! command -v bd >/dev/null 2>&1; then - log "installing bd (beads v1.3.0)…" - if ! curl -fsSL "$BD_INSTALLER_URL" -o /tmp/bd-install.sh 2>/tmp/bd-install.log; then - log "WARNING: could not download the bd installer; see /tmp/bd-install.log." - elif bash /tmp/bd-install.sh >>/tmp/bd-install.log 2>&1; then - log "bd installed" - else - log "WARNING: bd install failed; see /tmp/bd-install.log (the script that" - log " ran is at /tmp/bd-install.sh). Issue tracking unavailable." - fi -fi +# Forward the Claude env file variable to the generic name the bootstrap uses if [ -n "${CLAUDE_ENV_FILE:-}" ]; then - # Single quotes are deliberate: $PATH and $HOME must be written literally so - # they expand when the session sources this, not now. - # shellcheck disable=SC2016 - echo 'export PATH="$PATH:/root/go/bin:$HOME/go/bin"' >> "$CLAUDE_ENV_FILE" -fi -command -v bd >/dev/null 2>&1 && bd metrics off >/dev/null 2>&1 - -# bd may have arrived only just now, in which case §0 had nothing to check. -# Re-check, but only if §0 stayed quiet, so the banner is never printed twice. -if [ "$pin_status" -eq 0 ]; then - assert_bd_pin || pin_status=2 + export AGENT_ENV_FILE="$CLAUDE_ENV_FILE" fi -# ── 2. Sync the issue tracker from the Dolt remote ───────────────────────── -# `bd init` in-place rewrites .beads/config.yaml and commits, so the sync -# happens in a throwaway directory and only the database is moved back. -# That directory must be named after the database (see .beads/metadata.json), -# since bd derives the database name from it. -if command -v bd >/dev/null 2>&1; then - DB_NAME="$(jq -r '.dolt_database // "improvise"' .beads/metadata.json 2>/dev/null || echo improvise)" - ORIGIN="$(git remote get-url origin 2>/dev/null || true)" - case "$ORIGIN" in - git@github.com:*) ORIGIN="https://github.com/${ORIGIN#git@github.com:}" ;; - esac - ORIGIN="${ORIGIN%.git}" - - # The mktemp is part of the condition on purpose. Unchecked, a failure - # leaves TMP empty, "$TMP/$DB_NAME" becomes an absolute path at the - # filesystem root, and the cleanup at the end degrades to `rm -rf ""`. - # Short-circuiting also means no temp directory is created when there is - # no origin to sync from. - if [ -n "$ORIGIN" ] && TMP=$(mktemp -d) && [ -d "$TMP" ]; then - log "syncing issue tracker from $ORIGIN …" - STAGE="$TMP/$DB_NAME" - mkdir -p "$STAGE" - - # `bd init` exits non-zero when the remote is unreachable but STILL leaves - # a .beads/embeddeddolt behind — an empty database. Testing only for the - # directory would swap that over a good tracker, so check the status and - # confirm the result actually carries issues. An empty remote tracker is - # indistinguishable from a failed clone here; keeping what we already have - # is the safe way to be wrong. - sync_ok=0 - if ( - cd "$STAGE" && git init -q . && - BD_NON_INTERACTIVE=1 bd init --non-interactive --skip-agents --skip-hooks -q \ - --remote "git+$ORIGIN" - ) >/tmp/bd-sync.log 2>&1 && [ -d "$STAGE/.beads/embeddeddolt" ]; then - listing="$( (cd "$STAGE" && bd list --all 2>/dev/null) || true )" - case "$listing" in - "" | "No issues found."*) ;; - *) sync_ok=1 ;; - esac +# For local checkouts, only verify the bd pin (no install, no sync) +if [ "${CLAUDE_CODE_REMOTE:-}" != "true" ]; then + # Inline the pin-check only, since the full bootstrap is cloud-only + cd "$ROOT" || exit 0 + beads_pin() { + sed -n 's|.*beads\.url[[:space:]]*=[[:space:]]*"github:[^/]*/[^/]*/v\{0,1\}\([^"]*\)".*|\1|p' \ + flake.nix 2>/dev/null | head -1 + } + bd_running_version() { + bd version 2>/dev/null | sed -n 's/^bd version \([0-9][0-9.]*\).*/\1/p' | head -1 + } + if command -v bd >/dev/null 2>&1; then + pinned=$(beads_pin) + running=$(bd_running_version) + if [ -n "$pinned" ] && [ -n "$running" ] && [ "$running" != "$pinned" ]; then + printf '\n[session-start] ===== BEADS VERSION SKEW =====\n' >&2 + printf '[session-start] pinned (flake.nix): %s\n' "$pinned" >&2 + printf '[session-start] running: %s (%s)\n' "$running" "$(command -v bd)" >&2 + printf '[session-start] Reading is safe. Do NOT run mutating bd commands.\n' >&2 + printf '[session-start] Fix: nix develop, or direnv allow.\n' >&2 + printf '[session-start] =============================\n\n' >&2 + exit 2 fi - - if [ "$sync_ok" -eq 1 ] && install_tracker "$STAGE/.beads/embeddeddolt"; then - log "issue tracker synced — 'bd ready' now reflects the remote, not issues.jsonl" - elif [ -d .beads/embeddeddolt ]; then - log "WARNING: tracker sync failed (see /tmp/bd-sync.log); kept the previous database." - log " It may be out of date — verify any bead against the code." - else - log "WARNING: tracker sync failed; see /tmp/bd-sync.log." - log " .beads/issues.jsonl is a STALE export — verify any bead against the code." - fi - rm -rf "$TMP" - elif [ -n "$ORIGIN" ]; then - log "WARNING: could not create a temporary directory; skipped the tracker sync." - log " .beads/issues.jsonl is a STALE export — verify any bead against the code." fi + exit 0 fi -# ── 3. Warm the build so the first cargo test is not a cold compile ──────── -if command -v cargo >/dev/null 2>&1; then - log "warming cargo build…" - if cargo build --workspace --tests >/tmp/cargo-warm.log 2>&1; then - log "workspace built" - else - log "WARNING: cargo build failed; see /tmp/cargo-warm.log" - fi -fi - -# ── 4. Environment facts worth knowing up front ──────────────────────────── -cat <<'NOTES' -[session-start] Environment notes (see context/cloud-agents.md): - • Use the preinstalled cargo toolchain for ordinary work. `nix develop` - also works, but only after scripts/nix-seed-flake-inputs.sh — GitHub - tarballs are not served here, so the locked inputs are seeded over git. - • `bd dolt push` is blocked outbound (403). Bead status changes stay local. - • Never run a bare `bd init` in this repo: it rewrites AGENTS.md, CLAUDE.md - and .claude/settings.json, and commits. Use --skip-agents --skip-hooks. - • `bd dolt push ` ignores its argument and pushes to origin. -NOTES - -# Non-zero only when §0 found version skew; the setup above still ran. -exit "$pin_status" +# Cloud session: run the full bootstrap +exec "$BOOTSTRAP" diff --git a/.claude/skills/beads b/.claude/skills/beads new file mode 120000 index 00000000..f57add10 --- /dev/null +++ b/.claude/skills/beads @@ -0,0 +1 @@ +../../.agents/skills/beads \ No newline at end of file diff --git a/.cursor/environment.json b/.cursor/environment.json new file mode 100644 index 00000000..ad4d8ea5 --- /dev/null +++ b/.cursor/environment.json @@ -0,0 +1,5 @@ +{ + "name": "Improvise", + "install": "./scripts/agent-bootstrap.sh --skip-cargo-warm", + "start": "nix-daemon &" +} diff --git a/.cursor/skills/beads b/.cursor/skills/beads new file mode 120000 index 00000000..f57add10 --- /dev/null +++ b/.cursor/skills/beads @@ -0,0 +1 @@ +../../.agents/skills/beads \ No newline at end of file diff --git a/.cursor/skills/beads-merge b/.cursor/skills/beads-merge new file mode 120000 index 00000000..53a96ecc --- /dev/null +++ b/.cursor/skills/beads-merge @@ -0,0 +1 @@ +../../.agents/skills/beads-merge \ No newline at end of file diff --git a/.cursor/skills/code-review b/.cursor/skills/code-review new file mode 120000 index 00000000..2d88efdf --- /dev/null +++ b/.cursor/skills/code-review @@ -0,0 +1 @@ +../../.agents/skills/code-review \ No newline at end of file diff --git a/.cursor/skills/kb b/.cursor/skills/kb new file mode 120000 index 00000000..3ee2d209 --- /dev/null +++ b/.cursor/skills/kb @@ -0,0 +1 @@ +../../.agents/skills/kb \ No newline at end of file diff --git a/.cursor/skills/pr-watch b/.cursor/skills/pr-watch new file mode 120000 index 00000000..3ef371ce --- /dev/null +++ b/.cursor/skills/pr-watch @@ -0,0 +1 @@ +../../.agents/skills/pr-watch \ No newline at end of file diff --git a/.cursor/skills/wave b/.cursor/skills/wave new file mode 120000 index 00000000..0c2044da --- /dev/null +++ b/.cursor/skills/wave @@ -0,0 +1 @@ +../../.agents/skills/wave \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md index a476b158..f47132cc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,8 +13,10 @@ force-push. The rule, the message convention and the incidents are `.agents/skills/wave/agent-rules.md`, rule 8. - always start responses with bananaS! -- there is a prolog MCP available: load a session from `kb` at the root of the repository at the beginning of - every session, and query it as you work. **Reading only** — the MCP must never write `kb.pl`. +- **Knowledge base: use `scripts/kb`, not Prolog MCP.** The Prolog MCP is optional and not + available in all agent stacks (Cursor cloud agents do not have it). All agents must use + `scripts/kb add|replace|rm|query|check` for KB operations — see the `kb` skill at + `.agents/skills/kb/SKILL.md`. Reading `prolog-sessions/kb.pl` directly is fine for queries. - save structured facts as you go — work done, interesting locations, important decisions — with `scripts/kb add` (see the `kb` skill). Write a fact when you learn it; there is no reason to batch. - query the knowledge base as needed and use Prolog reasoning when it makes sense. @@ -25,6 +27,28 @@ This project uses **bd** (beads) for issue tracking. Run `bd prime` for full workflow context. +## Multi-Stack Agent Support + +This repo supports multiple coding-agent stacks: Claude Code, Cursor (IDE and cloud), and +potentially others. All stacks share the same operational setup: + +| Component | Location | Notes | +|---|---|---| +| Skills | `.agents/skills/` | Canonical source; `.claude/skills/` and `.cursor/skills/` symlink here | +| Session bootstrap | `scripts/agent-bootstrap.sh` | Stack-agnostic; installs bd, syncs tracker, warms cargo | +| Knowledge base | `scripts/kb` | **All stacks use this**, not Prolog MCP | +| Issue tracker | `bd` CLI | Synced from Dolt remote at session start | + +**Claude Code**: `.claude/hooks/session-start.sh` calls the bootstrap automatically. + +**Cursor cloud**: `.cursor/environment.json` runs the bootstrap via `install`. Skills are +discovered from `.cursor/skills/` (symlinks to `.agents/skills/`). + +**Local agents**: Run `scripts/agent-bootstrap.sh` manually, or use `direnv` / `nix develop` +for the full dev shell which includes `bd` from the flake pin. + +See `context/cloud-agents.md` for environment-specific notes (nix, network, etc.). + ## Quick Reference ```bash diff --git a/context/cloud-agents.md b/context/cloud-agents.md index 9fb343b4..7ab55bda 100644 --- a/context/cloud-agents.md +++ b/context/cloud-agents.md @@ -1,11 +1,26 @@ # Cloud Agents (LLM Reference) -Environment notes for agents running in Claude Code on the web. Local -checkouts are unaffected — the session hook exits early unless +Environment notes for agents running in cloud containers — Claude Code on the web, +Cursor cloud agents, and similar setups. Local checkouts are unaffected. + +## Session Bootstrap + +All agent stacks use the same bootstrap script: `scripts/agent-bootstrap.sh`. It: + +1. Verifies the `bd` version matches the flake.nix pin +2. Installs `bd` if missing +3. Syncs the Dolt issue tracker from the git remote +4. Optionally warms the cargo build + +**Claude Code**: `.claude/hooks/session-start.sh` calls the bootstrap automatically when `CLAUDE_CODE_REMOTE=true`. -`.claude/hooks/session-start.sh` handles the setup automatically. This doc -is the "why", and the record of what bites when it doesn't run. +**Cursor cloud**: `.cursor/environment.json` runs the bootstrap via `install`. Skills are +in `.cursor/skills/` (symlinks to `.agents/skills/`). + +**Other stacks**: Run `scripts/agent-bootstrap.sh` manually after checkout. + +This doc is the "why", and the record of what bites when setup doesn't run. --- @@ -143,3 +158,21 @@ environment, where egress leaves through your own network boundary. reorder or fold commits (`.agents/skills/wave/agent-rules.md`, rule 8). - Keep `context/repo-map.md` and `context/design-principles.md` current — they are the two docs agents are expected to maintain by hand. + +--- + +## Knowledge Base + +**Use `scripts/kb`, not Prolog MCP.** The Prolog MCP is not available in all agent stacks +(Cursor cloud agents do not have it). All agents must use `scripts/kb` for KB operations: + +```bash +scripts/kb check # kb_consistent, exactly what CI runs +scripts/kb query '' # run a goal; exit 1 if it fails +scripts/kb add # add clauses; refuses if a key already exists +scripts/kb replace # add, overwriting existing keys +scripts/kb rm # remove by key; refuses if a key is absent +``` + +Reading `prolog-sessions/kb.pl` directly for queries is fine. See the `kb` skill at +`.agents/skills/kb/SKILL.md` for the full interface. diff --git a/scripts/agent-bootstrap.sh b/scripts/agent-bootstrap.sh new file mode 100755 index 00000000..196e28b3 --- /dev/null +++ b/scripts/agent-bootstrap.sh @@ -0,0 +1,222 @@ +#!/bin/bash +# Stack-agnostic agent bootstrap for this repository. +# +# This script prepares any coding agent (Claude Code, Cursor, etc.) to work +# on this repo. It handles: +# 1. bd (beads) installation and pin verification +# 2. Issue tracker sync from the Dolt remote +# 3. Optional cargo workspace warming +# +# Usage: +# scripts/agent-bootstrap.sh [--skip-cargo-warm] +# +# The script is idempotent and safe to re-run. Nothing here writes to tracked +# files, stages anything, or creates a commit. +# +# See also: `.claude/hooks/session-start.sh` (Claude Code wrapper), +# `.cursor/environment.json` (Cursor cloud config) +set -uo pipefail + +SKIP_CARGO_WARM=0 +while [[ $# -gt 0 ]]; do + case "$1" in + --skip-cargo-warm) SKIP_CARGO_WARM=1; shift ;; + *) shift ;; + esac +done + +ROOT="${AGENT_PROJECT_DIR:-${CLAUDE_PROJECT_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}}" +cd "$ROOT" || exit 0 + +log() { printf '[agent-bootstrap] %s\n' "$*"; } + +# ── 0. The bd on PATH must be the bd flake.nix pins ──────────────────────── +beads_pin() { + sed -n 's|.*beads\.url[[:space:]]*=[[:space:]]*"github:[^/]*/[^/]*/v\{0,1\}\([^"]*\)".*|\1|p' \ + flake.nix 2>/dev/null | head -1 +} + +bd_running_version() { + bd version 2>/dev/null | sed -n 's/^bd version \([0-9][0-9.]*\).*/\1/p' | head -1 +} + +bd_pin_banner() { + { + printf '\n[agent-bootstrap] ===== BEADS VERSION SKEW =====\n' + printf '[agent-bootstrap] %s\n' "$@" + printf '[agent-bootstrap] =============================\n\n' + } >&2 +} + +assert_bd_pin() { + command -v bd >/dev/null 2>&1 || return 0 + + pinned=$(beads_pin) + running=$(bd_running_version) + + case "$pinned" in + [0-9]*.[0-9]*.[0-9]*) ;; + *) + bd_pin_banner "cannot read the beads pin out of flake.nix" \ + "read: '${pinned:-}'" \ + "The pin moved to a ref this hook cannot parse (a branch or a" \ + "commit, perhaps). Teach beads_pin() to read it, or the check" \ + "silently protects nothing." + return 1 + ;; + esac + + if [ -z "$running" ]; then + bd_pin_banner "cannot read the version of the bd on PATH" \ + "bd: $(command -v bd)" \ + "'bd version' did not answer in the expected shape. Until it does," \ + "nothing here can tell whether that build matches the pin." + return 1 + fi + + [ "$running" = "$pinned" ] && return 0 + + bd_pin_banner "the bd on PATH is NOT the bd this repo pins" \ + "pinned (flake.nix): $pinned" \ + "running: $running ($(command -v bd))" \ + "" \ + "Reading is safe. Do NOT run a mutating bd command — create, update," \ + "close, dep, dolt push. Version skew between builds empties the" \ + "dependency graph in .beads/issues.jsonl and reports success." \ + "" \ + "Fix now: nix develop (in this directory), then re-run the" \ + " command; or 'direnv allow' here." \ + "Fix for good: worktrees are created fresh per dispatch, so allowing" \ + " them one at a time never catches the next one. Put the" \ + " repo root under [whitelist] prefix in" \ + " ~/.config/direnv/direnv.toml." + return 1 +} + +pin_status=0 +assert_bd_pin || pin_status=2 + +# Install a verified database at .beads/embeddeddolt. +install_tracker() { + staged=$1 + live=.beads/embeddeddolt + incoming=$live.incoming + previous=$live.previous + + rm -rf "$incoming" "$previous" || return 1 + cp -rf "$staged" "$incoming" || { rm -rf "$incoming"; return 1; } + + if [ -d "$live" ] && ! mv -f "$live" "$previous"; then + rm -rf "$incoming" + return 1 + fi + + if mv -f "$incoming" "$live"; then + rm -rf "$previous" + return 0 + fi + + if [ -d "$previous" ] && ! mv -f "$previous" "$live"; then + log "ERROR: tracker left at $previous — move it back to $live by hand" + fi + rm -rf "$incoming" + return 1 +} + +# ── 1. bd on PATH ────────────────────────────────────────────────────────── +export PATH="$PATH:/root/go/bin:$HOME/go/bin" + +BD_INSTALLER_REF=f45b249ce6b40ba62aecc03949e6371e8f7c79d8 # tag v1.3.0 +BD_INSTALLER_URL="https://raw.githubusercontent.com/steveyegge/beads/$BD_INSTALLER_REF/scripts/install.sh" + +if ! command -v bd >/dev/null 2>&1; then + log "installing bd (beads v1.3.0)…" + if ! curl -fsSL "$BD_INSTALLER_URL" -o /tmp/bd-install.sh 2>/tmp/bd-install.log; then + log "WARNING: could not download the bd installer; see /tmp/bd-install.log." + elif bash /tmp/bd-install.sh >>/tmp/bd-install.log 2>&1; then + log "bd installed" + else + log "WARNING: bd install failed; see /tmp/bd-install.log (the script that" + log " ran is at /tmp/bd-install.sh). Issue tracking unavailable." + fi +fi + +# Export PATH for the calling environment if AGENT_ENV_FILE is set +if [ -n "${AGENT_ENV_FILE:-${CLAUDE_ENV_FILE:-}}" ]; then + env_file="${AGENT_ENV_FILE:-${CLAUDE_ENV_FILE}}" + echo 'export PATH="$PATH:/root/go/bin:$HOME/go/bin"' >> "$env_file" +fi + +command -v bd >/dev/null 2>&1 && bd metrics off >/dev/null 2>&1 + +if [ "$pin_status" -eq 0 ]; then + assert_bd_pin || pin_status=2 +fi + +# ── 2. Sync the issue tracker from the Dolt remote ───────────────────────── +if command -v bd >/dev/null 2>&1; then + DB_NAME="$(jq -r '.dolt_database // "improvise"' .beads/metadata.json 2>/dev/null || echo improvise)" + ORIGIN="$(git remote get-url origin 2>/dev/null || true)" + case "$ORIGIN" in + git@github.com:*) ORIGIN="https://github.com/${ORIGIN#git@github.com:}" ;; + esac + ORIGIN="${ORIGIN%.git}" + + if [ -n "$ORIGIN" ] && TMP=$(mktemp -d) && [ -d "$TMP" ]; then + log "syncing issue tracker from $ORIGIN …" + STAGE="$TMP/$DB_NAME" + mkdir -p "$STAGE" + + sync_ok=0 + if ( + cd "$STAGE" && git init -q . && + BD_NON_INTERACTIVE=1 bd init --non-interactive --skip-agents --skip-hooks -q \ + --remote "git+$ORIGIN" + ) >/tmp/bd-sync.log 2>&1 && [ -d "$STAGE/.beads/embeddeddolt" ]; then + listing="$( (cd "$STAGE" && bd list --all 2>/dev/null) || true )" + case "$listing" in + "" | "No issues found."*) ;; + *) sync_ok=1 ;; + esac + fi + + if [ "$sync_ok" -eq 1 ] && install_tracker "$STAGE/.beads/embeddeddolt"; then + log "issue tracker synced — 'bd ready' now reflects the remote, not issues.jsonl" + elif [ -d .beads/embeddeddolt ]; then + log "WARNING: tracker sync failed (see /tmp/bd-sync.log); kept the previous database." + log " It may be out of date — verify any bead against the code." + else + log "WARNING: tracker sync failed; see /tmp/bd-sync.log." + log " .beads/issues.jsonl is a STALE export — verify any bead against the code." + fi + rm -rf "$TMP" + elif [ -n "$ORIGIN" ]; then + log "WARNING: could not create a temporary directory; skipped the tracker sync." + log " .beads/issues.jsonl is a STALE export — verify any bead against the code." + fi +fi + +# ── 3. Warm the build (optional) ─────────────────────────────────────────── +if [ "$SKIP_CARGO_WARM" -eq 0 ] && command -v cargo >/dev/null 2>&1; then + log "warming cargo build…" + if cargo build --workspace --tests >/tmp/cargo-warm.log 2>&1; then + log "workspace built" + else + log "WARNING: cargo build failed; see /tmp/cargo-warm.log" + fi +fi + +# ── 4. Environment notes ─────────────────────────────────────────────────── +cat <<'NOTES' +[agent-bootstrap] Environment notes (see context/cloud-agents.md): + • Use the preinstalled cargo toolchain for ordinary work. `nix develop` + also works, but only after scripts/nix-seed-flake-inputs.sh — GitHub + tarballs are not served here, so the locked inputs are seeded over git. + • `bd dolt push` is blocked outbound (403). Bead status changes stay local. + • Never run a bare `bd init` in this repo: it rewrites AGENTS.md, CLAUDE.md + and .claude/settings.json, and commits. Use --skip-agents --skip-hooks. + • `bd dolt push ` ignores its argument and pushes to origin. + • Use `scripts/kb` for knowledge base operations, not Prolog MCP. +NOTES + +exit "$pin_status" From 106d4fd3ac0cc8fcc2d7f308d216712500d17429 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 19:20:34 +0000 Subject: [PATCH 2/7] feat: make nix develop the default toolchain path User ruling: agents must work inside nix develop (or direnv) for this repo. Bare system cargo that fails edition2024 is a misconfigured session. Changes: 1. scripts/agent-bootstrap.sh: - Added section 0: seed nix flake inputs automatically - Cargo warm now runs inside 'nix develop --command' - Added --skip-nix-seed flag for environments that don't need it - Updated environment notes to emphasize nix-first 2. .cursor/environment.json: - Removed --skip-cargo-warm (full nix-backed warm now default) 3. AGENTS.md: - Added 'Toolchain' row to multi-stack table (nix develop required) - Added 'Toolchain Requirement' subsection with usage examples - Updated local agents guidance to use 'direnv allow' 4. context/cloud-agents.md: - Bootstrap now lists nix-seed as step 1 - 'What does not work' table updated: bare system cargo now listed as failing; nix develop works after bootstrap - Updated nix section to reflect automatic seeding Previous soft gap: nix-seed was a manual footnote; now it runs in bootstrap. Co-authored-by: Ed Langley --- .cursor/environment.json | 2 +- AGENTS.md | 22 +++++++++++-- context/cloud-agents.md | 31 ++++++++++-------- scripts/agent-bootstrap.sh | 67 ++++++++++++++++++++++++++++---------- 4 files changed, 88 insertions(+), 34 deletions(-) diff --git a/.cursor/environment.json b/.cursor/environment.json index ad4d8ea5..8f01ff31 100644 --- a/.cursor/environment.json +++ b/.cursor/environment.json @@ -1,5 +1,5 @@ { "name": "Improvise", - "install": "./scripts/agent-bootstrap.sh --skip-cargo-warm", + "install": "./scripts/agent-bootstrap.sh", "start": "nix-daemon &" } diff --git a/AGENTS.md b/AGENTS.md index f47132cc..453143f3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,18 +34,34 @@ potentially others. All stacks share the same operational setup: | Component | Location | Notes | |---|---|---| +| Toolchain | `nix develop` / `direnv` | **Required.** Rust from flake, wasm target, beads pin, trealla | | Skills | `.agents/skills/` | Canonical source; `.claude/skills/` and `.cursor/skills/` symlink here | -| Session bootstrap | `scripts/agent-bootstrap.sh` | Stack-agnostic; installs bd, syncs tracker, warms cargo | +| Session bootstrap | `scripts/agent-bootstrap.sh` | Seeds nix inputs, installs bd, syncs tracker, warms cargo | | Knowledge base | `scripts/kb` | **All stacks use this**, not Prolog MCP | | Issue tracker | `bd` CLI | Synced from Dolt remote at session start | +### Toolchain Requirement + +**Agents must work inside `nix develop` (or direnv) for this repo.** Bare system cargo that +fails `edition2024` or missing wasm target is a misconfigured session. The bootstrap seeds +nix flake inputs automatically (GitHub tarballs are blocked in cloud), so `nix develop` +should work after install completes. + +Run commands inside the dev shell: +```bash +nix develop --command cargo test +nix develop --command cargo build --workspace +``` + +Or enter an interactive shell: `nix develop` + **Claude Code**: `.claude/hooks/session-start.sh` calls the bootstrap automatically. **Cursor cloud**: `.cursor/environment.json` runs the bootstrap via `install`. Skills are discovered from `.cursor/skills/` (symlinks to `.agents/skills/`). -**Local agents**: Run `scripts/agent-bootstrap.sh` manually, or use `direnv` / `nix develop` -for the full dev shell which includes `bd` from the flake pin. +**Local agents**: Run `scripts/agent-bootstrap.sh` manually, or use `direnv allow` for +automatic dev shell activation. See `context/cloud-agents.md` for environment-specific notes (nix, network, etc.). diff --git a/context/cloud-agents.md b/context/cloud-agents.md index 7ab55bda..f88b35fb 100644 --- a/context/cloud-agents.md +++ b/context/cloud-agents.md @@ -7,10 +7,14 @@ Cursor cloud agents, and similar setups. Local checkouts are unaffected. All agent stacks use the same bootstrap script: `scripts/agent-bootstrap.sh`. It: -1. Verifies the `bd` version matches the flake.nix pin -2. Installs `bd` if missing -3. Syncs the Dolt issue tracker from the git remote -4. Optionally warms the cargo build +1. **Seeds nix flake inputs** (GitHub tarballs are blocked; seeds over git instead) +2. Verifies the `bd` version matches the flake.nix pin +3. Installs `bd` if missing +4. Syncs the Dolt issue tracker from the git remote +5. Warms the cargo build **inside `nix develop`** + +**After bootstrap, `nix develop` works.** This is the required toolchain — bare system +cargo that fails `edition2024` or missing wasm target is a misconfigured session. **Claude Code**: `.claude/hooks/session-start.sh` calls the bootstrap automatically when `CLAUDE_CODE_REMOTE=true`. @@ -75,7 +79,8 @@ same if you need to re-sync by hand; never run it in the working tree. | Thing | Status | Use instead | |---|---|---| -| `nix develop` **unseeded** | Fails: flake inputs are GitHub **tarballs**, which are not served here | Run `scripts/nix-seed-flake-inputs.sh` first — see below. Or just use the preinstalled `cargo`/`rustc`, the same stable toolchain the flake pins | +| `nix develop` **without bootstrap** | Fails: flake inputs are GitHub **tarballs**, which are blocked | Bootstrap seeds inputs automatically; if it failed, run `scripts/nix-seed-flake-inputs.sh` manually | +| Bare system cargo | Fails: `edition2024`, missing wasm target | **Always use `nix develop`** — this is the required toolchain | | `bd dolt push` | Fails: 403 outbound | Nothing; bead status changes stay local. Say so rather than implying the tracker was updated | | `git+ssh://` Dolt remote in `config.yaml` | No ssh client in the container | `git+https://`, which the hook passes explicitly | | `git push`, GitHub API | **Work normally** | — | @@ -85,13 +90,13 @@ starting by hand). Reads of public GitHub repos over the git protocol work — it is specifically the `/archive/*.tar.gz` and `codeload` endpoints that are refused. -### Making `nix develop` work +### How nix develop works after bootstrap -`scripts/nix-seed-flake-inputs.sh` makes the devShell work **without touching -`flake.nix` or `flake.lock`**. Verified end to end: `rustc 1.98.1`, `cargo`, -`crate2nix` in the shell, `git status` clean. +The bootstrap runs `scripts/nix-seed-flake-inputs.sh` automatically, which seeds +the flake inputs over git **without touching `flake.nix` or `flake.lock`**. +After bootstrap, `nix develop` should work. If it fails, check `/tmp/nix-seed.log`. -Why it is needed, and why the obvious fixes are not available: +Why the seeding is needed: - Nix's `github:` fetcher wants a tarball. GitHub traffic **bypasses the environment's domain allowlist entirely** and goes through a proxy that @@ -120,9 +125,9 @@ around it — a mismatch fails loudly. Do not instead rewrite the lock's nodes to `git` type: carrying the old `narHash` onto a different fetcher is exactly the claim the lock exists to make. -It is not in the session-start hook because it is expensive (minutes, plus -several GB of store once the devShell builds) and most work does not need the -devShell. Run it when you do. +The bootstrap runs this automatically. It takes a few minutes and several GB of +store once the devShell builds. If bootstrap reported a seed failure, check +`/tmp/nix-seed.log` and re-run `scripts/nix-seed-flake-inputs.sh` manually. Two alternatives if per-container seeding is unattractive: push the inputs to a binary cache with `nix flake archive` and point sessions at it with diff --git a/scripts/agent-bootstrap.sh b/scripts/agent-bootstrap.sh index 196e28b3..7f2e328c 100755 --- a/scripts/agent-bootstrap.sh +++ b/scripts/agent-bootstrap.sh @@ -3,24 +3,30 @@ # # This script prepares any coding agent (Claude Code, Cursor, etc.) to work # on this repo. It handles: -# 1. bd (beads) installation and pin verification -# 2. Issue tracker sync from the Dolt remote -# 3. Optional cargo workspace warming +# 1. Nix flake input seeding (so `nix develop` works in cloud environments) +# 2. bd (beads) installation and pin verification +# 3. Issue tracker sync from the Dolt remote +# 4. Optional cargo workspace warming (inside nix develop) # # Usage: -# scripts/agent-bootstrap.sh [--skip-cargo-warm] +# scripts/agent-bootstrap.sh [--skip-cargo-warm] [--skip-nix-seed] # # The script is idempotent and safe to re-run. Nothing here writes to tracked # files, stages anything, or creates a commit. # +# IMPORTANT: This repo requires `nix develop` (or direnv) for builds/tests. +# Bare system cargo that fails edition2024 is a misconfigured session. +# # See also: `.claude/hooks/session-start.sh` (Claude Code wrapper), # `.cursor/environment.json` (Cursor cloud config) set -uo pipefail SKIP_CARGO_WARM=0 +SKIP_NIX_SEED=0 while [[ $# -gt 0 ]]; do case "$1" in --skip-cargo-warm) SKIP_CARGO_WARM=1; shift ;; + --skip-nix-seed) SKIP_NIX_SEED=1; shift ;; *) shift ;; esac done @@ -30,7 +36,27 @@ cd "$ROOT" || exit 0 log() { printf '[agent-bootstrap] %s\n' "$*"; } -# ── 0. The bd on PATH must be the bd flake.nix pins ──────────────────────── +# ── 0. Seed nix flake inputs (required for cloud environments) ───────────── +# GitHub tarball endpoints are blocked in cloud containers. This seeds the +# flake inputs over git so `nix develop` can build the dev shell. +if [ "$SKIP_NIX_SEED" -eq 0 ] && command -v nix >/dev/null 2>&1; then + # Set SSL cert for proxy environments + if [ -z "${NIX_SSL_CERT_FILE:-}" ] && [ -r /root/.ccr/ca-bundle.crt ]; then + export NIX_SSL_CERT_FILE=/root/.ccr/ca-bundle.crt + fi + + if [ -f "$ROOT/scripts/nix-seed-flake-inputs.sh" ]; then + log "seeding nix flake inputs (GitHub tarballs blocked in cloud)…" + if "$ROOT/scripts/nix-seed-flake-inputs.sh" >/tmp/nix-seed.log 2>&1; then + log "flake inputs seeded" + else + log "WARNING: nix-seed-flake-inputs.sh failed; see /tmp/nix-seed.log" + log " 'nix develop' may fail. Check network/proxy settings." + fi + fi +fi + +# ── 1. The bd on PATH must be the bd flake.nix pins ──────────────────────── beads_pin() { sed -n 's|.*beads\.url[[:space:]]*=[[:space:]]*"github:[^/]*/[^/]*/v\{0,1\}\([^"]*\)".*|\1|p' \ flake.nix 2>/dev/null | head -1 @@ -123,7 +149,7 @@ install_tracker() { return 1 } -# ── 1. bd on PATH ────────────────────────────────────────────────────────── +# ── 2. bd on PATH ────────────────────────────────────────────────────────── export PATH="$PATH:/root/go/bin:$HOME/go/bin" BD_INSTALLER_REF=f45b249ce6b40ba62aecc03949e6371e8f7c79d8 # tag v1.3.0 @@ -153,7 +179,7 @@ if [ "$pin_status" -eq 0 ]; then assert_bd_pin || pin_status=2 fi -# ── 2. Sync the issue tracker from the Dolt remote ───────────────────────── +# ── 3. Sync the issue tracker from the Dolt remote ───────────────────────── if command -v bd >/dev/null 2>&1; then DB_NAME="$(jq -r '.dolt_database // "improvise"' .beads/metadata.json 2>/dev/null || echo improvise)" ORIGIN="$(git remote get-url origin 2>/dev/null || true)" @@ -196,27 +222,34 @@ if command -v bd >/dev/null 2>&1; then fi fi -# ── 3. Warm the build (optional) ─────────────────────────────────────────── -if [ "$SKIP_CARGO_WARM" -eq 0 ] && command -v cargo >/dev/null 2>&1; then - log "warming cargo build…" - if cargo build --workspace --tests >/tmp/cargo-warm.log 2>&1; then - log "workspace built" +# ── 4. Warm the build inside nix develop (optional) ──────────────────────── +# Uses nix develop to ensure the correct toolchain (rust from flake, wasm target, etc.) +if [ "$SKIP_CARGO_WARM" -eq 0 ] && command -v nix >/dev/null 2>&1; then + log "warming cargo build inside nix develop…" + NIX_FLAGS=(--extra-experimental-features 'nix-command flakes') + if nix "${NIX_FLAGS[@]}" develop --command cargo build --workspace --tests >/tmp/cargo-warm.log 2>&1; then + log "workspace built (nix develop toolchain)" else log "WARNING: cargo build failed; see /tmp/cargo-warm.log" + log " This may indicate nix-seed failed or flake inputs are missing." fi +elif [ "$SKIP_CARGO_WARM" -eq 0 ]; then + log "WARNING: nix not available; skipping cargo warm" + log " Install nix or use a container with nix for this repo." fi -# ── 4. Environment notes ─────────────────────────────────────────────────── +# ── 5. Environment notes ─────────────────────────────────────────────────── cat <<'NOTES' [agent-bootstrap] Environment notes (see context/cloud-agents.md): - • Use the preinstalled cargo toolchain for ordinary work. `nix develop` - also works, but only after scripts/nix-seed-flake-inputs.sh — GitHub - tarballs are not served here, so the locked inputs are seeded over git. + • This repo REQUIRES `nix develop` (or direnv) for builds and tests. + Bare system cargo that fails edition2024 is a misconfigured session. + • The bootstrap seeded flake inputs over git (GitHub tarballs are blocked + in cloud). If nix develop still fails, check /tmp/nix-seed.log. • `bd dolt push` is blocked outbound (403). Bead status changes stay local. • Never run a bare `bd init` in this repo: it rewrites AGENTS.md, CLAUDE.md and .claude/settings.json, and commits. Use --skip-agents --skip-hooks. - • `bd dolt push ` ignores its argument and pushes to origin. • Use `scripts/kb` for knowledge base operations, not Prolog MCP. + • Run commands inside `nix develop` or prefix with `nix develop --command`. NOTES exit "$pin_status" From 03e46591bdd00127ae7dc2b0f723ee329cfc09df Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 19:22:46 +0000 Subject: [PATCH 3/7] docs: require tracking GitHub comments/reviews, not just CI checks Multi-stack agents (especially Cursor cloud) may focus on pr-watch (CI checks only) and miss review feedback. This makes the obligation explicit. AGENTS.md - new 'PR Tracking: Checks AND Reviews' subsection: - Fetch reviews AND comments (reviews are separate API surface) - Codex/ChatGPT reviews arrive as reviews, not comments - Use ./scripts/pr-reviews to see all feedback together - Answer substantive feedback with identity line before done/merge - Re-check after push that reviews target current head SHA - Don't wait indefinitely on Copilot/Codex (quota-limited) - Points to existing wave essay for full workflow context/cloud-agents.md - new 'PR Tracking' section: - Brief summary of the obligation - Points to pr-watch skill for CI, scripts/pr-reviews for reviews - Cross-references AGENTS.md for full details Co-authored-by: Ed Langley --- AGENTS.md | 24 ++++++++++++++++++++++++ context/cloud-agents.md | 14 ++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 453143f3..e614641a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,6 +65,30 @@ automatic dev shell activation. See `context/cloud-agents.md` for environment-specific notes (nix, network, etc.). +### PR Tracking: Checks AND Reviews + +For any PR an agent owns or is working on, **track both CI checks and GitHub comments/reviews**. +The `pr-watch` skill handles check status; reviews require separate attention. + +1. **Fetch reviews and comments** — reviews are a separate API surface from the comment thread. + Codex/ChatGPT reviews arrive as *reviews* (not comments), so checking only the comment + thread misses them. Use `./scripts/pr-reviews ` to see bodies, inline comments, and + issue comments together. + +2. **Answer substantive review feedback** on the PR before treating the work as done or + merging. Every comment you post must open with an identity line naming the model and + that it is an agent posting (the repo owner's account is shared). + +3. **Re-check after each push** that new comments/reviews target the current head SHA. + A review may target an earlier commit; verify rather than assume. + +4. **Don't wait indefinitely on Copilot/Codex.** Reviews arrive as quota allows; an absent + review and an unrequested one look identical. The adversarial review that gates a merge + is dispatched by the orchestrator. External reviews are read *in addition*, not instead. + +See "Shipping a wave" and "Read the PR's reviews before merging it" below for the full +review workflow, Copilot request mechanics, and how to answer declined findings. + ## Quick Reference ```bash diff --git a/context/cloud-agents.md b/context/cloud-agents.md index f88b35fb..d41bca55 100644 --- a/context/cloud-agents.md +++ b/context/cloud-agents.md @@ -166,6 +166,20 @@ environment, where egress leaves through your own network boundary. --- +## PR Tracking + +For any PR you own, **track both CI checks and GitHub comments/reviews**: + +- **CI checks**: Use the `pr-watch` skill or `scripts/pr-watch`. +- **Reviews and comments**: Use `./scripts/pr-reviews ` — reviews are a separate API + surface from comments, and Codex/ChatGPT reviews arrive as *reviews* not comments. +- **Answer feedback** with an identity line naming the model before marking work done. +- **Re-check after push** that reviews target the current head SHA. + +See AGENTS.md "PR Tracking: Checks AND Reviews" and "Shipping a wave" for the full workflow. + +--- + ## Knowledge Base **Use `scripts/kb`, not Prolog MCP.** The Prolog MCP is not available in all agent stacks From 3104e89ae509ed2d7148e8df94aaa28074a32755 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 19:35:50 +0000 Subject: [PATCH 4/7] feat(pr-watch): integrate reviews/comments into pr-watch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pr-watch skill and script now cover reviews/comments, not just CI checks. An agent reading only the pr-watch skill now knows to track both. .agents/skills/pr-watch/SKILL.md: - Reframed: 'watching a PR' means checks AND reviews - Added 'Reviews and Comments' section with the three surfaces table - Added 'Review Obligations' subsection (answer feedback, identity line, etc.) - Documented --reviews (default) and --no-reviews flags - Cross-references AGENTS.md for full workflow details scripts/pr-watch: - Added --reviews (default) and --no-reviews flags - After exit 0 (green/mergeable), calls scripts/pr-reviews to show all reviews and comments. Does not change exit code — answering is agent's job. - Updated usage text to document the new behavior - Composes with existing pr-reviews (fixture mode passes through) scripts/pr-watch-test.sh: - Added mk_reviews_empty() and mk_reviews_with_review() helpers - Added review fixtures to all cases that exit 0 (pr-reviews needs them) - Added tests: 'green with reviews shows review output' and '--no-reviews skips review output' - All 56 tests pass AGENTS.md / context/cloud-agents.md: - Updated cross-references to point to pr-watch skill as agent-facing recipe - Simplified text now that pr-watch handles reviews automatically Co-authored-by: Ed Langley --- .agents/skills/pr-watch/SKILL.md | 49 +++++++++++++++++++++++--- AGENTS.md | 28 +++++++-------- context/cloud-agents.md | 6 ++-- scripts/pr-watch | 49 +++++++++++++++++++++----- scripts/pr-watch-test.sh | 59 ++++++++++++++++++++++++++++++++ 5 files changed, 160 insertions(+), 31 deletions(-) diff --git a/.agents/skills/pr-watch/SKILL.md b/.agents/skills/pr-watch/SKILL.md index c39a5254..d54afa71 100644 --- a/.agents/skills/pr-watch/SKILL.md +++ b/.agents/skills/pr-watch/SKILL.md @@ -5,8 +5,14 @@ description: Wait for a GitHub PR's checks to finish before merging or reporting # Waiting on a PR +"Watching a PR" means tracking **both** CI checks **and** GitHub comments/reviews. +A PR is not ready to merge just because checks are green — review feedback must be +read and answered first. + +## CI Checks + ``` -scripts/pr-watch [--expect ] [--timeout ] [--interval ] +scripts/pr-watch [--expect ] [--timeout ] [--interval ] [--reviews] ``` ``` @@ -23,19 +29,54 @@ exit 5 the PR is MERGED or CLOSED, at the head the watch pinned; it ends on the first poll that sees it closed, which may be mid-wait ``` +**`--reviews`** (default): after checks complete, print all reviews and comments +via `scripts/pr-reviews`. Use `--no-reviews` to skip this. The reviews are shown +but do not change the exit code — answering them is your responsibility. + **Exit 0 now means mergeable, not only green.** GitHub computes mergeability lazily and reports `UNKNOWN` for a while after the base moves. It also reports `BLOCKED` for the moment before a required check that `needs:` another job has registered. The watch waits out both, so you do not need to re-check `mergeable` by hand after it. **Exit 3 is not a pass.** It may be a required -check that never ran. Why each -state counts the way it does: `context/code/tooling.md`, "pr-watch and -mergeability" (improvise-edpl). +check that never ran. Why each state counts the way it does: +`context/code/tooling.md`, "pr-watch and mergeability" (improvise-edpl). **Pass `--expect ` whenever you know which commit you are asking about.** It is the difference between "this PR is green" and "the thing I pushed is green", and a PR here was once merged at a head that predated its own fix. +## Reviews and Comments + +Reviews are a **separate API surface** from the comment thread. Codex/ChatGPT +reviews arrive as *reviews*, not comments — checking only the comment thread +misses them entirely. `pr-watch --reviews` (the default) shows all three: + +| Surface | What it is | +|---|---| +| `[REVIEW]` | The review's own body (pulls/{n}/reviews) | +| `[INLINE]` | A comment on a line of the diff (pulls/{n}/comments) | +| `[COMMENT]` | A comment on the PR thread (issues/{n}/comments) | + +To fetch reviews directly: `./scripts/pr-reviews ` — **do not pipe it**. The +script's `$?` must be read directly; piping or chaining loses the exit code. +Redirect to a file if you need to process it. + +### Review Obligations + +Before treating a PR as done or merging: + +1. **Read all review feedback** — reviews against an earlier head may still apply +2. **Answer substantive feedback** on the PR with an identity line naming the + model (the repo owner's account is shared) +3. **Re-check after each push** that new reviews target the current head SHA +4. **Don't wait indefinitely on Copilot/Codex** — reviews arrive as quota allows; + an absent review and an unrequested one look identical. The adversarial review + that gates a merge is dispatched by the orchestrator; external reviews are + read *in addition*, not instead. + +See AGENTS.md "Shipping a wave" and "Read the PR's reviews before merging" for +the full workflow, Copilot request mechanics, and how to answer declined findings. + ## Do not write the loop yourself Four signals look like "finished" and are not: diff --git a/AGENTS.md b/AGENTS.md index e614641a..a207a172 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,26 +68,22 @@ See `context/cloud-agents.md` for environment-specific notes (nix, network, etc. ### PR Tracking: Checks AND Reviews For any PR an agent owns or is working on, **track both CI checks and GitHub comments/reviews**. -The `pr-watch` skill handles check status; reviews require separate attention. +The `pr-watch` skill (`.agents/skills/pr-watch/SKILL.md`) is the agent-facing recipe. -1. **Fetch reviews and comments** — reviews are a separate API surface from the comment thread. - Codex/ChatGPT reviews arrive as *reviews* (not comments), so checking only the comment - thread misses them. Use `./scripts/pr-reviews ` to see bodies, inline comments, and - issue comments together. +**`scripts/pr-watch`** now prints reviews after checks are green (use `--no-reviews` to skip). +Reviews are a separate API surface from comments — Codex/ChatGPT reviews arrive as *reviews*, +not comments, and are easy to miss if you only read the comment thread. -2. **Answer substantive review feedback** on the PR before treating the work as done or - merging. Every comment you post must open with an identity line naming the model and - that it is an agent posting (the repo owner's account is shared). +1. **Run `scripts/pr-watch `** — waits for checks, then prints all reviews/comments +2. **Read all review feedback** — reviews against an earlier head may still apply +3. **Answer substantive feedback** with an identity line naming the model before merging +4. **Re-check after each push** that new reviews target the current head SHA +5. **Don't wait indefinitely on Copilot/Codex** — reviews arrive as quota allows -3. **Re-check after each push** that new comments/reviews target the current head SHA. - A review may target an earlier commit; verify rather than assume. +To fetch reviews directly: `./scripts/pr-reviews ` — **do not pipe it** (exit code trap). -4. **Don't wait indefinitely on Copilot/Codex.** Reviews arrive as quota allows; an absent - review and an unrequested one look identical. The adversarial review that gates a merge - is dispatched by the orchestrator. External reviews are read *in addition*, not instead. - -See "Shipping a wave" and "Read the PR's reviews before merging it" below for the full -review workflow, Copilot request mechanics, and how to answer declined findings. +See the `pr-watch` skill for the full recipe, and "Shipping a wave" below for Copilot +request mechanics and how to answer declined findings. ## Quick Reference diff --git a/context/cloud-agents.md b/context/cloud-agents.md index d41bca55..e0ce73c6 100644 --- a/context/cloud-agents.md +++ b/context/cloud-agents.md @@ -170,12 +170,12 @@ environment, where egress leaves through your own network boundary. For any PR you own, **track both CI checks and GitHub comments/reviews**: -- **CI checks**: Use the `pr-watch` skill or `scripts/pr-watch`. -- **Reviews and comments**: Use `./scripts/pr-reviews ` — reviews are a separate API - surface from comments, and Codex/ChatGPT reviews arrive as *reviews* not comments. +- **`scripts/pr-watch `**: Waits for checks, then prints all reviews/comments (default). + Reviews are a separate API surface — Codex/ChatGPT reviews arrive as *reviews* not comments. - **Answer feedback** with an identity line naming the model before marking work done. - **Re-check after push** that reviews target the current head SHA. +The `pr-watch` skill (`.agents/skills/pr-watch/SKILL.md`) is the agent-facing recipe. See AGENTS.md "PR Tracking: Checks AND Reviews" and "Shipping a wave" for the full workflow. --- diff --git a/scripts/pr-watch b/scripts/pr-watch index 67222362..cc5759cf 100755 --- a/scripts/pr-watch +++ b/scripts/pr-watch @@ -5,11 +5,13 @@ set -euo pipefail usage() { cat >&2 <<'EOF' -usage: pr-watch [--expect ] [--timeout ] [--interval ] +usage: pr-watch [--expect ] [--timeout ] [--interval ] [--reviews|--no-reviews] --expect fail if the PR head is not this (prefix ok) at any poll --timeout give up after this long (default 1800) --interval seconds between polls (default 20) + --reviews after checks complete, print reviews/comments (default) + --no-reviews skip the reviews/comments printout exit 0 every check COMPLETED, none failed, and GitHub says MERGEABLE with a state of CLEAN, UNSTABLE or HAS_HOOKS @@ -19,22 +21,31 @@ exit 3 timed out with checks still running, or mergeability still UNKNOWN, or still BLOCKED with every registered check green exit 4 usage or environment error exit 5 the PR is MERGED or CLOSED, so there is nothing left to wait for + +After exit 0, reviews and comments are printed (unless --no-reviews). These do +not change the exit code -- answering them is your responsibility. Reviews are +a separate API surface from comments; Codex/ChatGPT reviews arrive as *reviews* +and are easy to miss if you only read the comment thread. EOF exit 4 } -PR=""; EXPECT=""; TIMEOUT=1800; INTERVAL=20 +PR=""; EXPECT=""; TIMEOUT=1800; INTERVAL=20; SHOW_REVIEWS=1 while [ $# -gt 0 ]; do case "$1" in - --expect) EXPECT="${2:-}"; shift 2 ;; - --timeout) TIMEOUT="${2:-}"; shift 2 ;; - --interval) INTERVAL="${2:-}"; shift 2 ;; - -h|--help) usage ;; - *) [ -z "$PR" ] || usage; PR="$1"; shift ;; + --expect) EXPECT="${2:-}"; shift 2 ;; + --timeout) TIMEOUT="${2:-}"; shift 2 ;; + --interval) INTERVAL="${2:-}"; shift 2 ;; + --reviews) SHOW_REVIEWS=1; shift ;; + --no-reviews) SHOW_REVIEWS=0; shift ;; + -h|--help) usage ;; + *) [ -z "$PR" ] || usage; PR="$1"; shift ;; esac done [ -n "$PR" ] || usage +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + # `gh` is shell-aliased to a different tool in this repo's shells, so the name # is never trusted: a real gh is located explicitly. fetch() { @@ -130,4 +141,26 @@ echo "pr-watch: head=$head $mergeable/$state $total checks, $failed failed" # An allowlist, so a state GitHub adds later fails closed. DIRTY and BLOCKED # can wear a MERGEABLE flag: context/code/tooling.md, "pr-watch and mergeability". [ "$mergeable" = MERGEABLE ] || exit 1 -case "$state" in CLEAN|UNSTABLE|HAS_HOOKS) exit 0 ;; *) exit 1 ;; esac +case "$state" in CLEAN|UNSTABLE|HAS_HOOKS) ;; *) exit 1 ;; esac + +# ── Reviews and comments ─────────────────────────────────────────────────── +# Checks are green and mergeable. Show reviews/comments so the agent sees any +# feedback that needs answering before merge. This does not change the exit +# code -- answering reviews is your responsibility. +if [ "$SHOW_REVIEWS" -eq 1 ]; then + echo "" + echo "pr-watch: fetching reviews and comments (--no-reviews to skip)…" + # pr-reviews' exit code is not propagated: reviews being present or absent + # does not change whether the PR is green/mergeable. The agent must read + # and answer them, but that is a human-in-the-loop obligation, not a gate. + # Fixture mode: pass through the fixture dir if set. + if [ -n "${PR_WATCH_FIXTURE_DIR:-}" ]; then + PR_REVIEWS_FIXTURE_DIR="$PR_WATCH_FIXTURE_DIR" "$SCRIPT_DIR/pr-reviews" "$PR" || true + else + "$SCRIPT_DIR/pr-reviews" "$PR" || true + fi + echo "" + echo "pr-watch: reviews shown above. Answer substantive feedback before merging." +fi + +exit 0 diff --git a/scripts/pr-watch-test.sh b/scripts/pr-watch-test.sh index 59dd6531..3e64a714 100755 --- a/scripts/pr-watch-test.sh +++ b/scripts/pr-watch-test.sh @@ -20,8 +20,39 @@ run_case() { [ "$got" = "$want" ] && ok "$name" || bad "$name" "$got" "$want" } +# Like run_case but captures stdout for content checks +run_case_capture() { + local name="$1" want="$2" dir="$3"; shift 3 + local out="$work/output.txt" + PR_WATCH_FIXTURE_DIR="$dir" "$WATCH" 1 --interval 0 --timeout 1 "$@" >"$out" 2>&1 + local got=$? + [ "$got" = "$want" ] && ok "$name" || bad "$name" "$got" "$want" + cat "$out" +} + mk() { mkdir -p "$1"; cat > "$1/$2"; } +# Create empty pr-reviews fixtures so pr-watch can call pr-reviews in fixture mode. +# $1 dir, $2 head (must match the pr-watch fixture's headRefOid) +mk_reviews_empty() { + local dir="$1" head="$2" + mkdir -p "$dir" + printf '{"headRefOid":"%s","reviews":[]}\n' "$head" > "$dir/last.pr.json" + echo '[]' > "$dir/last.inline.json" + echo '[]' > "$dir/last.issue.json" +} + +# Create pr-reviews fixtures with a review. $1 dir, $2 head, $3 review commit +mk_reviews_with_review() { + local dir="$1" head="$2" review_commit="$3" + mkdir -p "$dir" + cat > "$dir/last.pr.json" < "$dir/last.inline.json" + echo '[]' > "$dir/last.issue.json" +} + # The state a fresh push is in: the PR exists, no check has registered yet. # A watcher keyed on "nothing pending" calls this finished and green. mk "$work/none" last.json <<'EOF' @@ -43,8 +74,31 @@ mk "$work/green" last.json <<'EOF' {"name":"build","status":"COMPLETED","conclusion":"SUCCESS"}, {"name":"skipped","status":"COMPLETED","conclusion":"SKIPPED"}]} EOF +mk_reviews_empty "$work/green" "aaaaaaaaaaaa" run_case "all completed and green" 0 "$work/green" +# ── Reviews integration ──────────────────────────────────────────────────── +# pr-watch now calls pr-reviews after checks are green. Test that it works. +mk "$work/green-reviews" last.json <<'EOF' +{"headRefOid":"aaaaaaaaaaaa","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","statusCheckRollup":[ + {"name":"build","status":"COMPLETED","conclusion":"SUCCESS"}]} +EOF +mk_reviews_with_review "$work/green-reviews" "aaaaaaaaaaaa" "aaaaaaaaaaaa" +out=$(run_case_capture "green with reviews shows review output" 0 "$work/green-reviews") +if echo "$out" | grep -q "pr-reviews:"; then + ok "green with reviews prints pr-reviews output" +else + bad "green with reviews prints pr-reviews output" "no pr-reviews output" "pr-reviews: in output" +fi + +# --no-reviews skips the review printout +out=$(run_case_capture "--no-reviews skips review output" 0 "$work/green-reviews" --no-reviews) +if echo "$out" | grep -q "pr-reviews:"; then + bad "--no-reviews skips review output" "pr-reviews in output" "no pr-reviews" +else + ok "--no-reviews actually skips review output" +fi + mk "$work/red" last.json <<'EOF' {"headRefOid":"aaaaaaaaaaaa","mergeable":"MERGEABLE","mergeStateStatus":"UNSTABLE","statusCheckRollup":[ {"name":"build","status":"COMPLETED","conclusion":"FAILURE"}]} @@ -112,6 +166,10 @@ for m in MERGEABLE CONFLICTING UNKNOWN; do fi sv="\"$s\""; [ "$s" = null ] && sv=null state "$work/table-$m-$s" last.json "\"$m\"" "$sv" "$green_rollup" + # Add review fixtures for cases that exit 0 (pr-watch calls pr-reviews) + if [ "$want" = 0 ]; then + mk_reviews_empty "$work/table-$m-$s" "aaaaaaaaaaaa" + fi run_case "table: $m/$s" "$want" "$work/table-$m-$s" --timeout 0 done done @@ -148,6 +206,7 @@ gap_state "$work/gap" 1.json MERGEABLE BLOCKED "$not_yet | $build_running" gap_state "$work/gap" 2.json MERGEABLE BLOCKED "$not_yet" gap_state "$work/gap" 3.json MERGEABLE BLOCKED "$harness_running" gap_state "$work/gap" last.json MERGEABLE CLEAN '.' +mk_reviews_empty "$work/gap" "aaaaaaaaaaaa" run_case "the needs-gap before the required check registers is not a failure" 0 "$work/gap" --timeout 60 gap_state "$work/gap-never" last.json MERGEABLE BLOCKED "$not_yet" From b0c4105be34c26d5e159158268bb895b7839e744 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 19:37:48 +0000 Subject: [PATCH 5/7] docs: add GitHub attribution as a hard rule for all agent stacks Every GitHub write (PR, comment, review reply) must include an attribution line identifying the AI agent. Posts go out under the repository owner's account, so an unattributed post reads as the human's words. Format: _Posted by ()_ Examples: - _Posted by Claude (Opus 4.5)_ - _Posted by Cursor Agent (claude-sonnet-5)_ AGENTS.md: - Added 'GitHub Attribution (Hard Rule)' section in multi-stack docs - Concrete format and examples - Clarified that a thread is not 'answered' until the reply has attribution .agents/skills/pr-watch/SKILL.md: - Added 'GitHub Attribution (Hard Rule)' subsection - Updated 'Answer substantive feedback with attribution' in obligations - Cross-references AGENTS.md for full rule context/cloud-agents.md: - Added 'GitHub Attribution (Hard Rule)' section - Same format and examples as AGENTS.md This ensures any agent reading the shared docs (Cursor, Claude, etc.) knows to attribute every GitHub write to the specific AI agent. Co-authored-by: Ed Langley --- .agents/skills/pr-watch/SKILL.md | 21 +++++++++++++++++---- AGENTS.md | 25 ++++++++++++++++++++++++- context/cloud-agents.md | 19 ++++++++++++++++++- 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/.agents/skills/pr-watch/SKILL.md b/.agents/skills/pr-watch/SKILL.md index d54afa71..c6a447a1 100644 --- a/.agents/skills/pr-watch/SKILL.md +++ b/.agents/skills/pr-watch/SKILL.md @@ -66,16 +66,29 @@ Redirect to a file if you need to process it. Before treating a PR as done or merging: 1. **Read all review feedback** — reviews against an earlier head may still apply -2. **Answer substantive feedback** on the PR with an identity line naming the - model (the repo owner's account is shared) +2. **Answer substantive feedback with attribution** — every reply must include + the attribution line (see below) 3. **Re-check after each push** that new reviews target the current head SHA 4. **Don't wait indefinitely on Copilot/Codex** — reviews arrive as quota allows; an absent review and an unrequested one look identical. The adversarial review that gates a merge is dispatched by the orchestrator; external reviews are read *in addition*, not instead. -See AGENTS.md "Shipping a wave" and "Read the PR's reviews before merging" for -the full workflow, Copilot request mechanics, and how to answer declined findings. +### GitHub Attribution (Hard Rule) + +**Every GitHub write must include an attribution line.** Posts go out under the +repository owner's account — an unattributed post reads as the human's words. + +Open the body with: `_Posted by ()_` + +Examples: +- `_Posted by Claude (Opus 4.5)_` +- `_Posted by Cursor Agent (claude-sonnet-5)_` + +A thread is not "answered" until the reply carries this attribution. + +See AGENTS.md "GitHub Attribution" for the full rule and "Shipping a wave" for +Copilot request mechanics and how to answer declined findings. ## Do not write the loop yourself diff --git a/AGENTS.md b/AGENTS.md index a207a172..b5c5e18a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,6 +65,29 @@ automatic dev shell activation. See `context/cloud-agents.md` for environment-specific notes (nix, network, etc.). +### GitHub Attribution (Hard Rule) + +**Every GitHub write (PR, comment, review reply) must include an attribution line identifying +the AI agent.** Posts go out under the repository owner's account, and an unattributed post +reads as the human owner's words to everyone who sees it. + +**Format:** Open the body with an italicized attribution line: + +``` +_Posted by ()_ +``` + +Examples: +- `_Posted by Claude (Opus 4.5)_` +- `_Posted by Cursor Agent (claude-sonnet-5)_` +- `_Posted by Claude Code_` + +For PR bodies, the `🤖 Generated with …` footer that some tools add covers this. For +comments and review replies, add the line yourself — there is no automatic footer. + +**This applies to:** PR creation, PR updates, issue comments, review comments, and review +replies. A thread is not "answered" until the reply carries this attribution. + ### PR Tracking: Checks AND Reviews For any PR an agent owns or is working on, **track both CI checks and GitHub comments/reviews**. @@ -76,7 +99,7 @@ not comments, and are easy to miss if you only read the comment thread. 1. **Run `scripts/pr-watch `** — waits for checks, then prints all reviews/comments 2. **Read all review feedback** — reviews against an earlier head may still apply -3. **Answer substantive feedback** with an identity line naming the model before merging +3. **Answer substantive feedback with attribution** before merging (see format above) 4. **Re-check after each push** that new reviews target the current head SHA 5. **Don't wait indefinitely on Copilot/Codex** — reviews arrive as quota allows diff --git a/context/cloud-agents.md b/context/cloud-agents.md index e0ce73c6..fa6bf2f1 100644 --- a/context/cloud-agents.md +++ b/context/cloud-agents.md @@ -172,7 +172,7 @@ For any PR you own, **track both CI checks and GitHub comments/reviews**: - **`scripts/pr-watch `**: Waits for checks, then prints all reviews/comments (default). Reviews are a separate API surface — Codex/ChatGPT reviews arrive as *reviews* not comments. -- **Answer feedback** with an identity line naming the model before marking work done. +- **Answer feedback with attribution** before marking work done (see below). - **Re-check after push** that reviews target the current head SHA. The `pr-watch` skill (`.agents/skills/pr-watch/SKILL.md`) is the agent-facing recipe. @@ -180,6 +180,23 @@ See AGENTS.md "PR Tracking: Checks AND Reviews" and "Shipping a wave" for the fu --- +## GitHub Attribution (Hard Rule) + +**Every GitHub write (PR, comment, review reply) must include an attribution line.** +Posts go out under the repository owner's account — an unattributed post reads as the +human's words. + +Open the body with: `_Posted by ()_` + +Examples: +- `_Posted by Claude (Opus 4.5)_` +- `_Posted by Cursor Agent (claude-sonnet-5)_` + +A thread is not "answered" until the reply carries this attribution. See AGENTS.md +"GitHub Attribution" for the full rule. + +--- + ## Knowledge Base **Use `scripts/kb`, not Prolog MCP.** The Prolog MCP is not available in all agent stacks From ec30c8a7de98e51028659c40f56d262332797806 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 26 Sep 2026 01:30:00 +0000 Subject: [PATCH 6/7] fix: address three Codex P2 review findings 1. Shebang (Codex P2): scripts/agent-bootstrap.sh and .claude/hooks/session-start.sh now use #!/usr/bin/env bash instead of #!/bin/bash, matching repo convention enforced by checkers.sh. 2. Fail-closed bd pin (Codex P2): .claude/hooks/session-start.sh local path now exits 2 with a warning when pinned or running version cannot be parsed, instead of silently succeeding. An unparseable version is not a pass. 3. pr-reviews failure (Codex P2): scripts/pr-watch no longer uses '|| true' to swallow pr-reviews failures. If pr-reviews fails (fetch error, malformed response), pr-watch now exits 6 with a warning. Empty successful fetch (no reviews) is still exit 0. - Added exit 6 to usage text and skill documentation - Added test case 'pr-reviews failure exits 6' using malformed JSON fixture - All 57 pr-watch tests pass Co-authored-by: Ed Langley --- .agents/skills/pr-watch/SKILL.md | 5 +++-- .claude/hooks/session-start.sh | 16 ++++++++++++++-- scripts/agent-bootstrap.sh | 2 +- scripts/pr-watch | 26 +++++++++++++++----------- scripts/pr-watch-test.sh | 12 ++++++++++++ 5 files changed, 45 insertions(+), 16 deletions(-) diff --git a/.agents/skills/pr-watch/SKILL.md b/.agents/skills/pr-watch/SKILL.md index c6a447a1..b8e39f2f 100644 --- a/.agents/skills/pr-watch/SKILL.md +++ b/.agents/skills/pr-watch/SKILL.md @@ -16,8 +16,8 @@ scripts/pr-watch [--expect ] [--timeout ] [--interval /dev/null 2>&1; then pinned=$(beads_pin) running=$(bd_running_version) - if [ -n "$pinned" ] && [ -n "$running" ] && [ "$running" != "$pinned" ]; then + # Fail closed: if we can't parse either version, warn rather than silently pass + if [ -z "$pinned" ]; then + printf '[session-start] WARNING: cannot parse beads pin from flake.nix\n' >&2 + printf '[session-start] The pin check is skipped, but mutating bd commands may be unsafe.\n' >&2 + exit 2 + fi + if [ -z "$running" ]; then + printf '[session-start] WARNING: cannot parse bd version from "bd version"\n' >&2 + printf '[session-start] bd: %s\n' "$(command -v bd)" >&2 + printf '[session-start] The pin check is skipped, but mutating bd commands may be unsafe.\n' >&2 + exit 2 + fi + if [ "$running" != "$pinned" ]; then printf '\n[session-start] ===== BEADS VERSION SKEW =====\n' >&2 printf '[session-start] pinned (flake.nix): %s\n' "$pinned" >&2 printf '[session-start] running: %s (%s)\n' "$running" "$(command -v bd)" >&2 diff --git a/scripts/agent-bootstrap.sh b/scripts/agent-bootstrap.sh index 7f2e328c..35e9be8d 100755 --- a/scripts/agent-bootstrap.sh +++ b/scripts/agent-bootstrap.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # Stack-agnostic agent bootstrap for this repository. # # This script prepares any coding agent (Claude Code, Cursor, etc.) to work diff --git a/scripts/pr-watch b/scripts/pr-watch index cc5759cf..10844e1e 100755 --- a/scripts/pr-watch +++ b/scripts/pr-watch @@ -14,16 +14,16 @@ usage: pr-watch [--expect ] [--timeout ] [--interval --no-reviews skip the reviews/comments printout exit 0 every check COMPLETED, none failed, and GitHub says MERGEABLE with a - state of CLEAN, UNSTABLE or HAS_HOOKS + state of CLEAN, UNSTABLE or HAS_HOOKS (and reviews fetched if --reviews) exit 1 a check failed, or the PR is not mergeable (any other known state) exit 2 the head moved during the wait, or is not --expect exit 3 timed out with checks still running, or mergeability still UNKNOWN, or still BLOCKED with every registered check green exit 4 usage or environment error exit 5 the PR is MERGED or CLOSED, so there is nothing left to wait for +exit 6 checks green/mergeable, but pr-reviews failed (fetch error, not "no reviews") -After exit 0, reviews and comments are printed (unless --no-reviews). These do -not change the exit code -- answering them is your responsibility. Reviews are +After exit 0, reviews and comments are printed (unless --no-reviews). Reviews are a separate API surface from comments; Codex/ChatGPT reviews arrive as *reviews* and are easy to miss if you only read the comment thread. EOF @@ -146,21 +146,25 @@ case "$state" in CLEAN|UNSTABLE|HAS_HOOKS) ;; *) exit 1 ;; esac # ── Reviews and comments ─────────────────────────────────────────────────── # Checks are green and mergeable. Show reviews/comments so the agent sees any # feedback that needs answering before merge. This does not change the exit -# code -- answering reviews is your responsibility. +# code for success -- answering reviews is your responsibility. However, if +# pr-reviews itself fails (usage error, network, etc.), we warn and exit 6. if [ "$SHOW_REVIEWS" -eq 1 ]; then echo "" echo "pr-watch: fetching reviews and comments (--no-reviews to skip)…" - # pr-reviews' exit code is not propagated: reviews being present or absent - # does not change whether the PR is green/mergeable. The agent must read - # and answer them, but that is a human-in-the-loop obligation, not a gate. - # Fixture mode: pass through the fixture dir if set. + reviews_exit=0 if [ -n "${PR_WATCH_FIXTURE_DIR:-}" ]; then - PR_REVIEWS_FIXTURE_DIR="$PR_WATCH_FIXTURE_DIR" "$SCRIPT_DIR/pr-reviews" "$PR" || true + PR_REVIEWS_FIXTURE_DIR="$PR_WATCH_FIXTURE_DIR" "$SCRIPT_DIR/pr-reviews" "$PR" || reviews_exit=$? else - "$SCRIPT_DIR/pr-reviews" "$PR" || true + "$SCRIPT_DIR/pr-reviews" "$PR" || reviews_exit=$? fi echo "" - echo "pr-watch: reviews shown above. Answer substantive feedback before merging." + if [ "$reviews_exit" -eq 0 ]; then + echo "pr-watch: reviews shown above. Answer substantive feedback before merging." + else + echo "pr-watch: WARNING: pr-reviews failed (exit $reviews_exit). Reviews may not be shown." >&2 + echo "pr-watch: Run './scripts/pr-reviews $PR' manually to fetch reviews." >&2 + exit 6 + fi fi exit 0 diff --git a/scripts/pr-watch-test.sh b/scripts/pr-watch-test.sh index 3e64a714..685a034f 100755 --- a/scripts/pr-watch-test.sh +++ b/scripts/pr-watch-test.sh @@ -99,6 +99,18 @@ else ok "--no-reviews actually skips review output" fi +# pr-reviews failure: if pr-reviews fails, pr-watch exits 6 (not 0) +# Create a green fixture with MALFORMED review fixtures so pr-reviews fails +mk "$work/green-bad-reviews" last.json <<'EOF' +{"headRefOid":"aaaaaaaaaaaa","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","statusCheckRollup":[ + {"name":"build","status":"COMPLETED","conclusion":"SUCCESS"}]} +EOF +# Create malformed JSON that will cause jq to fail in pr-reviews +echo 'NOT VALID JSON' > "$work/green-bad-reviews/last.pr.json" +echo '[]' > "$work/green-bad-reviews/last.inline.json" +echo '[]' > "$work/green-bad-reviews/last.issue.json" +run_case "pr-reviews failure exits 6" 6 "$work/green-bad-reviews" + mk "$work/red" last.json <<'EOF' {"headRefOid":"aaaaaaaaaaaa","mergeable":"MERGEABLE","mergeStateStatus":"UNSTABLE","statusCheckRollup":[ {"name":"build","status":"COMPLETED","conclusion":"FAILURE"}]} From 190b330c4ea00c8600f55915ffa4c04cbedc03a9 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 26 Sep 2026 01:38:18 +0000 Subject: [PATCH 7/7] fix(pr-watch-test): run_case_capture in parent shell, not subshell The run_case_capture helper was being called in command substitution (out=$(run_case_capture ...)), which runs it in a subshell. This caused the ok/bad updates to the pass/fail counters to be discarded. Fix by having the helper write output to a caller-provided file parameter, then reading from that file in the caller. This keeps the helper in the parent shell where counter updates are preserved. Addresses Codex P2 review on commit ec30c8a7. Co-authored-by: Ed Langley --- scripts/pr-watch-test.sh | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/scripts/pr-watch-test.sh b/scripts/pr-watch-test.sh index 685a034f..5293388e 100755 --- a/scripts/pr-watch-test.sh +++ b/scripts/pr-watch-test.sh @@ -20,14 +20,14 @@ run_case() { [ "$got" = "$want" ] && ok "$name" || bad "$name" "$got" "$want" } -# Like run_case but captures stdout for content checks +# Like run_case but writes output to a caller-provided file. +# Caller must NOT use command substitution (subshell loses ok/bad counters). +# $1 name, $2 expected exit, $3 fixture dir, $4 output file, rest: extra args run_case_capture() { - local name="$1" want="$2" dir="$3"; shift 3 - local out="$work/output.txt" - PR_WATCH_FIXTURE_DIR="$dir" "$WATCH" 1 --interval 0 --timeout 1 "$@" >"$out" 2>&1 + local name="$1" want="$2" dir="$3" outfile="$4"; shift 4 + PR_WATCH_FIXTURE_DIR="$dir" "$WATCH" 1 --interval 0 --timeout 1 "$@" >"$outfile" 2>&1 local got=$? [ "$got" = "$want" ] && ok "$name" || bad "$name" "$got" "$want" - cat "$out" } mk() { mkdir -p "$1"; cat > "$1/$2"; } @@ -84,16 +84,16 @@ mk "$work/green-reviews" last.json <<'EOF' {"name":"build","status":"COMPLETED","conclusion":"SUCCESS"}]} EOF mk_reviews_with_review "$work/green-reviews" "aaaaaaaaaaaa" "aaaaaaaaaaaa" -out=$(run_case_capture "green with reviews shows review output" 0 "$work/green-reviews") -if echo "$out" | grep -q "pr-reviews:"; then +run_case_capture "green with reviews shows review output" 0 "$work/green-reviews" "$work/out1.txt" +if grep -q "pr-reviews:" "$work/out1.txt"; then ok "green with reviews prints pr-reviews output" else bad "green with reviews prints pr-reviews output" "no pr-reviews output" "pr-reviews: in output" fi # --no-reviews skips the review printout -out=$(run_case_capture "--no-reviews skips review output" 0 "$work/green-reviews" --no-reviews) -if echo "$out" | grep -q "pr-reviews:"; then +run_case_capture "--no-reviews skips review output" 0 "$work/green-reviews" "$work/out2.txt" --no-reviews +if grep -q "pr-reviews:" "$work/out2.txt"; then bad "--no-reviews skips review output" "pr-reviews in output" "no pr-reviews" else ok "--no-reviews actually skips review output"