live: heartbeat sync 2026-10-21T02:32:51-07:00 (Refs #11)

agent/managed-skills/audit-subagent-deliverables/SKILL.md

@@ -1,61 +1,41 @@
 ---
 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."
+description: "Integrate subagent-written code safely: line-by-line audit, mechanical gates (bash test/lint), merge shared-function collisions, fix scope creep, verify env var propagation."
 ---
 
-# Audit subagent deliverables
+Integrating code written by subagents (fan-out implementation). The code is usually correct; the failures are structural — collisions between agents, scope creep, and assumptions that never reach the runtime.
 
-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.
+## Workflow
 
-Order of operations: read everything → gates → tests (as evidence, not proof) → resource-safety pass → fix in-place → cutover.
+1. **Read every deliverable line by line** before integrating — not the summary.
+2. **Run the mechanical gates immediately**, in parallel: `bash -n` on every script; `bats tests/ 2>&1 | grep "not ok"` for the failure list, never `tail` (tail shows a passing tail and hides the red in the middle).
+3. **Merge shared-function collisions.** When two agents add different versions of the same function, pick the superset and re-point the other's callers at it. (Real case: one agent's `fleet_base()` carried `-b`, the other's `resolve_fleet_base()` carried `--base-url`; resolve to the one flag set the tests use.)
+4. **Delete scope creep.** Code added for a goal nobody asked for — a new env var, a new CLI flag, a new output file — goes, with its docs, unless the operator asked.
+5. **Verify env var propagation.** A var the orchestrator computes must be exported before the `bash -c` / subshell that reads it, or the child silently takes a default. Check the export site, not the doc.
 
-## 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.
+## Audit checks
 
-## 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.
+| Check | How |
+|---|---|
+| Flag names real | `bash script.sh --help` |
+| Paths exist | `test -e` each literal in the diff |
+| Env reaches the runtime | grep the export/inline-assignment site, then the read |
+| Tests assert behaviour | not `[[ -n "$X" ]]`, not `test -f <new file>` |
+| Tests not weakened | `git diff HEAD~ -- tests/` — a deleted assertion is a red flag |
 
-## 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).
+## Red flags
 
-## 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.
+- Test asserting a new file exists rather than behaviour.
+- Doc claiming a flag the script never defines.
+- Function added with no caller.
+- Doc example that contradicts the code it documents.
+- Same helper defined twice with different flag spelling.
+- New env var read but never exported.
 
-## 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.
+## Re-verify after the operator's own edits
 
-## 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).
+Run the gates again — their edit may undo an integration (a re-typed helper, a reverted flag).
 
-## 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.
+## Report
 
-## 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.
+Per deliverable: correct / changed / removed, with file:line, plus the gate output that proves it (`bats` failure list before and after).