Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion agents/ast-grep-runner.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,31 @@ If `ast-grep` itself fails on a YAML (syntax error, missing file), include the e

## Process

### 0. Preflight: verify ast-grep binary (fail-fast)

**Always run this first, before any scan or inventory step.** Without the binary, every subsequent `ast-grep scan` call returns empty `tool_result`, the LLM loops on the verification check, and the job hits `activeDeadlineSeconds` 30 min later with no review posted. Observed end-to-end on bborbe/coding#34 (admin-merged after the loop).

```bash
if ! command -v ast-grep >/dev/null 2>&1 && ! command -v sg >/dev/null 2>&1; then
cat <<'EOF' >&2
ERROR: ast-grep / sg binary not found in PATH.
This funnel cannot run without it.

Install (host): npm install -g @ast-grep/cli
Install (alpine): apk --no-cache add ast-grep
Install (debian): curl -fsSL https://github.com/ast-grep/ast-grep/releases/latest/download/ast-grep-linux-x86_64.tar.gz | tar xz -C /usr/local/bin
Container fix: ensure pr-reviewer image has ast-grep — see
bborbe/maintainer agent/pr-reviewer/Dockerfile (commit 1de083f
added 'ast-grep' to the alpine apk line).
Runbook: [[Agent - Debug Missing PR Reviewer Review]] gate 5.
EOF
printf '{"stats":{"yamls_run":0,"findings_count":0,"elapsed_ms":0},"findings_by_owner":{},"errors":[{"kind":"missing-tool","tool":"ast-grep","detail":"binary not in PATH"}]}\n'
exit 1
fi
```

**Why exit 1, not "continue with errors":** the dispatcher's per-Owner adjudication phase reads from `findings_by_owner` only. A missing-tool error in `errors[]` would be invisible to the LLM tier — the review would post as APPROVED with no comments, masking that the mechanical layer never ran. Exit 1 surfaces the gap to the dispatcher (which aborts the review with an actionable error) instead of producing a silently-empty review.

### 1. Resolve rule inventory

```bash
Expand Down Expand Up @@ -117,7 +142,7 @@ Write the structured output to stdout. The dispatcher reads stdout, slices `find

| Symptom | Cause | Handling |
|---|---|---|
| ast-grep binary missing | toolchain gap | emit `{"errors": [{"kind": "missing-tool", "tool": "ast-grep"}]}`, exit non-zero |
| ast-grep binary missing | toolchain gap (e.g. pr-reviewer image regression — see bborbe/coding#34) | preflight Step 0 catches this BEFORE any scan; emit the missing-tool JSON skeleton, exit 1, dispatcher surfaces actionable error rather than posting an empty review |
| YAML rejected by ast-grep | rule syntax bug (PR #4 / #8 trap) | append to `errors[]`, continue |
| Rule ID not in index | stale walker output | append to `errors[]`, continue |
| Owner from index missing in `agents/` | stale rule entry | append to `errors[]` with `kind: missing-owner-agent`, drop finding |
Expand Down
11 changes: 11 additions & 0 deletions commands/code-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ Mirrors `commands/pr-review.md` Step 4 (PR #27). Mechanical pre-filter via `codi
- "Missing LICENSE file"
- "README missing license section" (check with Grep for `## License` in README.md)

#### 4.0: Toolchain preflight (fail-fast)

Mirror of `commands/pr-review.md` Step 4.0. Verify `ast-grep` is in PATH before invoking the runner; the runner fail-fasts on the same check (`agents/ast-grep-runner.md` Step 0), but doing it here too keeps the failure surface close to the dispatcher:

```bash
cd <directory> && (command -v ast-grep >/dev/null 2>&1 || command -v sg >/dev/null 2>&1) \
|| { echo "ERROR: ast-grep/sg not in PATH. Install via 'npm install -g @ast-grep/cli' (or 'apk add ast-grep' in alpine). pr-reviewer container fix: bborbe/maintainer agent/pr-reviewer/Dockerfile commit 1de083f." >&2; exit 1; }
```

If preflight fails, report the toolchain gap in Step 5 (Must Fix) and skip Step 4 entirely.

#### 4a: Mechanical funnel

```
Expand Down
11 changes: 11 additions & 0 deletions commands/pr-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@ The dispatcher replaces the previous hardcoded "load conventions + invoke fixed

**Short Mode**: No agents — skip to Step 5.

#### 4.0: Toolchain preflight (fail-fast)

Before invoking the runner, verify `ast-grep` is available in PATH. The runner itself fail-fasts on the same check (`agents/ast-grep-runner.md` Step 0), but doing it here too keeps the failure surface close to the dispatcher so the user sees a single clear error rather than the runner's JSON envelope:

```bash
cd <REVIEW_DIR> && (command -v ast-grep >/dev/null 2>&1 || command -v sg >/dev/null 2>&1) \
|| { echo "ERROR: ast-grep/sg not in PATH. Install via 'npm install -g @ast-grep/cli' (or 'apk add ast-grep' in alpine). pr-reviewer container fix: bborbe/maintainer agent/pr-reviewer/Dockerfile commit 1de083f." >&2; exit 1; }
```

If preflight fails, report the toolchain gap in Step 5 (Must Fix) and skip Step 4 entirely — a review without the mechanical funnel would silently miss every MUST-tier YAML finding.

#### 4a: Mechanical funnel

```
Expand Down
Loading