62 lines
5.8 KiB
Markdown
62 lines
5.8 KiB
Markdown
---
|
|
name: audit-subagent-deliverables
|
|
description: "Integrate subagent-written code safely: line-by-line audit, mechanical gates (bash test/lint), adversarial-review-report triage, resource-safety pass (fd/memory), and cutover with running-process restarts."
|
|
---
|
|
|
|
# Audit subagent deliverables
|
|
|
|
A subagent that wrote code is a source of plausible output, not a verified change. Integrate it the way you'd integrate an untrusted PR — with extra suspicion, because the author is an agent that shares your blind spots and its prose is more confident than its evidence.
|
|
|
|
Order of operations: read everything → gates → tests (as evidence, not proof) → resource-safety pass → fix in-place → cutover.
|
|
|
|
## 1. Read every produced file, line by line
|
|
Full file, not a diff skim. You are the reviewer who must catch the line the author never thought about.
|
|
|
|
## 2. Mechanical gates before logic review
|
|
```bash
|
|
bash -n <script> # syntax (scripts)
|
|
bunx tsc --noEmit # types (TS)
|
|
bun test # suite
|
|
```
|
|
Run the suite for information, not as the stopping point — it only proves the author's own scenarios.
|
|
|
|
## 3. Hunt the three classic agent defects
|
|
- **Quoting bugs:** `set -e` + `"${@/$p/$r}"` = substitution regex containing user input (bash can't escape `/` in a `${var/re/pat}` pattern: `/` reads as the pattern/replacement separator). Fix with a separator absent from inputs (e.g. `%`) and `printf '%q'` for shell-interpolated values.
|
|
- **`cd` assumptions:** parallel `runShell` calls share one CWD (pool mode). Any command sequence that assumes CWD (e.g. `git -C .` after a sibling's `cd`) must use explicit absolute paths.
|
|
- **Unbounded resources:** every fd opened must be closed on ALL paths (success, throw, validation fail) — `using` for fs.sinks (closes on throw; sinks are `AsyncDisposable`, not `fs.FileHandle`, no `.close()`); bounded `Map` for caches (`MAX` + FIFO eviction); explicit caps (lines read, file sizes).
|
|
|
|
## 4. Check the test file is real
|
|
Tests that assert internal state (`expect(wcwd).toBe(...)`) over output contracts; tests of mock echo; tautologies. Delete them per house rules, keep behavior contracts.
|
|
|
|
## 5. Cutover audit (repo-wide)
|
|
`grep -r` the symbol namespace the subagent touched — `wcwd`-style globals are the classic blast radius. Grep every `cd`/`process.chdir` site and verify each paired command sequence uses explicit paths. Stale comments describing the old design must go — comments that lie are time bombs for the next reader.
|
|
|
|
## 6. Cutover restarts running processes
|
|
A code fix on disk is NOT live in any process that loaded the file at spawn (bun/node load-at-spawn). After shipping fixes: restart every long-running supervisor/daemon/UI-server that carries the affected code path; prefer takeover protocols (stale-pid detection) that make restarts cheap. Symptom of forgetting: "fixed" code producing old-code symptoms in production (observed: 812 poisoned ledger rows).
|
|
|
|
## 7. Triaging an adversarial REVIEW report (not code)
|
|
When a subagent produced findings, treat each finding as a hypothesis with a base rate ~40% (measured over 3 rounds, 2026-10):
|
|
1. **Walk every finding end-to-end against the CURRENT tree** before accepting — cite file:line yourself. Reviewers frequently read a mid-edit snapshot; many findings are stale by construction.
|
|
2. **Classification that kills most findings:**
|
|
- *Stale read* — code changed during review; verify against live lines.
|
|
- *Inverted logic* — "failed rows are skipped" when the filter makes failed rows RETRY. Re-read the filter.
|
|
- *Unreachable* — trigger requires breaking a stated design law (single-worker, append-only). Not a defect.
|
|
- *By-design* — the reviewer labels a spec law a bug (serial execution, non-stream client). Check the spec, not the reviewer's confidence.
|
|
- *Constant-trust* — "XSS via manifest field" where the field is git-pinned config; "path traversal via REMOTE_TMP" where it's a hardcoded const. Ask: what untrusted input reaches this sink?
|
|
- *Self-refuting* — the reviewer's own probe output disproves its claim (raw==dedup probe in a "dedup" finding). Quote it back.
|
|
3. **Acceptance bar:** concrete trigger with real inputs + full path walked. Real accepts cluster in: resource caps (pids/timeout/awaited-cleanup), honest labels (default signatures), escaping discipline, self-healing probes.
|
|
4. **One-line verdicts + evidence per finding**, shipped in the commit message; calibration section ("genuinely solid") accepted as free verification of past fixes.
|
|
|
|
## Verification (the non-negotiable)
|
|
Build/run the real artifact with its real inputs. Observe behavior at the seams:
|
|
- concurrent calls → no CWD races;
|
|
- resource ceilings → caps enforced;
|
|
- cleanup → no fd/pid/dir leaks;
|
|
- security → injection probes inert (quotes, slashes, metachars);
|
|
- normal case → output identical to pre-audit.
|
|
|
|
## Evidence (2026-10-07 + 2026-10-10/11 sessions)
|
|
- Shell-engine subagent: 426 LOC, `bun test` 39/39 green. Line-audit found: 5 leaked fds per call (`fd` sink never closed), CWD-race in `git -C .`, `set -e` + `${@/\/g\//}` slash-escaping bug, unbounded 10k-entry cache, 45s timeout on git-status.
|
|
- Review-triage census over 3 blind adversarial rounds (same repo, ~41 findings): accepted 9, rejected ~32 — rejects dominated by stale reads (protocol rewritten mid-review), inverted filter logic, unreachable-under-law triggers, base64-alphabet "quote injection" (charset has no quotes), SIGPIPE non-leaks, "no memory cap" quoting the `--memory 4g` line above the claim. Real accepts: fork-bomb pids cap, stalled-ssh upload cap, stale-flag self-heal probe, honest census label, freshness gate on pid reuse.
|
|
- Poison-law incident: sandbox fix shipped while supervisor ran old build → 812 rows graded by the dead protocol; takeover made recovery cheap. The restart step (#6) exists because of this.
|