Agent Loops

Agent Workflow Loops

This skill defines the operational loops that implementer agents follow when making code changes and writing tests. Each loop has explicit entry criteria, exit criteria, and escalation rules. If you are an agent, follow these loops exactly.

You do not review your own work. All reviews are performed by an independent reviewer. Prefer Claude via the bundled scripts. If Claude is unavailable, use a different model before asking your own model family to review. Same-model shell-outs are the last resort. You never grade your own homework.

Bundled references:

• references/testing-standards.md — Test quality standards (how to write tests)
• references/audit-workflow.md — Test gap discovery (how to find what's missing)
• references/perspective-catalog.md — Review perspective selection (used by primary and fallback code review)
• references/review-prompt.md — Code review prompt template for fallback reviewers
• references/audit-prompt.md — Test audit prompt template for module-scope (full-contract) audits
• references/diff-audit-prompt.md — Test audit prompt template for diff-scope (per-commit) audits; used by diff-test-audit.sh when --git is active

Bundled scripts:

• $SKILL_DIR/scripts/specialist-review.sh — Provider-aware Claude/Gemini/Codex CLI path for code review
• $SKILL_DIR/scripts/diff-test-audit.sh — Provider-aware Claude/Gemini/Codex CLI path for test audit

Locate Scripts

The bundled scripts live inside the installed skill directory, not the project tree. Before invoking any script, resolve SKILL_DIR so paths work regardless of install scope:

SKILL_DIR="$(ls -d ~/.codex/skills/agent-loops 2>/dev/null || ls -d .codex/skills/agent-loops 2>/dev/null)"

All script invocations below use "$SKILL_DIR/scripts/...". Run the snippet above once at the start of your session and reuse the variable.

---

Architecture: Who Does What

Critical rule: Codex and Gemini NEVER self-review unless every independent provider path has already failed. Every review step must be performed by an independent reviewer using this selection order: 1. Bundled script with automatic Claude-first, self-last provider selection 2. A fresh-context Codex reviewer that did not implement the change

If neither path is available, stop and escalate to the user.

Why Shell-Based Review (Even for Claude)

The bundled scripts aren't a Codex/Gemini accommodation — they exist to enable cross-model independent review, which every agent benefits from. The provider rotation explicitly keeps the current agent's own model family last, so a Claude agent invoking specialist-review.sh gets its review from Gemini or Codex first, not another Claude instance.

Two kinds of reviewer independence are in play:

• Cross-model independence (shell scripts): reviewer is a *different model

family* with different training data and alignment. Catches blind spots inherent to the current model. Requires shelling out to a different provider.

• Fresh-context independence (sub-agents): reviewer is the *same model

family* but with no prior context. Catches local anchoring bias. Cheap to obtain via Task-tool sub-agents in Claude Code.

Agent-loops uses the first mechanism as its baseline because cross-model is a stronger guarantee than fresh-context alone. Claude-native sub-agent flows (like multi-specialist-review) add within-model multi-perspective diversity on top of the shell-based baseline when warranted — they don't replace it.

Reviewer Selection Order

When a review or audit is required, use this exact fallback chain:

1. Bundled script first. Let the bundled script try Claude, then another model family, and keep the current model family last. The script validates the artifact contract before accepting the generated artifact. 2. Fresh-context Codex next. Spawn a reviewer agent with fresh context. That agent must:

• not have authored or edited the implementation under review
• receive only the task spec, relevant diff/module/tests, and the bundled references
• act only as reviewer/auditor, not as implementer
• write its result to a markdown artifact under .agents/reviews/

3. Escalate if no independent reviewer is available.

Treat fallback artifacts exactly like script-generated REVIEW_FILE or REPORT_FILE outputs in the loops below. Call out fallback usage in the handoff so humans know whether the review came from Claude, Gemini, Codex, or fresh-context Codex.

---

Skill Invocation Reference

Pre-Review: Impact Analysis with Codanna (Optional)

Before requesting code review, you can use codanna to understand the blast radius of your changes. This provides grounded structural data that helps scope the review and catch issues the diff alone won't reveal.

# What calls the functions you changed?
codanna mcp find_callers process_request --watch

# What's the full impact if this symbol changes?
codanna mcp analyze_impact DatabaseConnection --watch --json

# Feed impact data into review context
IMPACT=$(codanna mcp analyze_impact "$CHANGED_SYMBOL" --watch --json 2>/dev/null)

This is optional — agent-loops works without codanna. But when available, impact data makes reviews more precise and catches downstream breakage the diff doesn't show.

specialist-review — Request Code Review

When: After completing implementation, after each remediation cycle. What you get back: Findings with severity levels (P0-P3) and a verdict (BLOCKED / PASS WITH ISSUES / CLEAN).

IMPORTANT: Source Files Only

Scope specialist-review to source files only. Do NOT include test files (*.test.*, *.spec.*, __tests__/) in the path filter — tests are reviewed separately in Loop 2 via diff-test-audit.

IMPORTANT: Do Not Review the Code Yourself

Your ONLY job is to invoke an independent reviewer and read the output artifact. Do NOT analyze the diff as the reviewer. Do NOT write review comments yourself. Do NOT adopt perspectives yourself. Route the review to Claude first, then fallback if needed.

Claude is still the preferred reviewer because it can load domain-specific skills such as owasp-top-10, secure-coding-practices, and python-testing-patterns. The bundled script now tries Claude first, then a different model family, and keeps same-model shell-outs for last resort. Codex and Gemini are both available as explicit providers. If the automated providers are unavailable or fail, continue with a fresh-context Codex reviewer instead of reviewing the code yourself.

Automated Path: Provider-Aware Script

LONG-RUNNING CALL — USE THE POLLING PATTERN BELOW.

This script invokes an external LLM and takes 3-5 minutes for larger diffs.

Do NOT start remediation, tests, or commits while the review is in progress.

When calling from a Bash tool, set timeout: 600000 (10 min) — the default

120-second timeout will kill the subprocess before the reviewer finishes.

The review is only done when you have a REVIEW_FILE path in hand.

Polling invocation (REQUIRED) — run the review in the background and poll so the Bash tool receives periodic output and does not time out:

# Start review in background, capture stdout (the result file path) separately
REVIEW_TMP=$(mktemp /tmp/review-out.XXXXXX)
"$SKILL_DIR/scripts/specialist-review.sh" --git -- src/parser/ src/auth.rs \
  >"$REVIEW_TMP" &
REVIEW_PID=$!

# Poll every 15s — each echo keeps the Bash tool connection alive
while kill -0 "$REVIEW_PID" 2>/dev/null; do
  sleep 15
  echo "[poll] Review still running (pid $REVIEW_PID)..."
done

# Collect exit code and read result
wait "$REVIEW_PID"
REVIEW_EXIT=$?
if [[ $REVIEW_EXIT -eq 0 ]]; then
  REVIEW_FILE=$(cat "$REVIEW_TMP")
  echo "Review complete: $REVIEW_FILE"
  cat "$REVIEW_FILE"
else
  echo "Review failed (exit $REVIEW_EXIT):"
  cat "$REVIEW_TMP"   # contains failure summary with stderr paths and debug info
fi
rm -f "$REVIEW_TMP"

The script emits heartbeat lines to stderr every 15 seconds during provider execution. Combined with the poll loop above, this ensures continuous output.

Additional invocation forms (wrap any of these in the polling pattern above):

# Review changes since a specific ref, scoped to a directory
"$SKILL_DIR/scripts/specialist-review.sh" --git origin/main -- claude_ctx_py/

# Review all changes vs last commit (use sparingly in monorepos)
"$SKILL_DIR/scripts/specialist-review.sh" --git

# Pipe in a pre-filtered diff
git diff HEAD~3..HEAD -- src/ | "$SKILL_DIR/scripts/specialist-review.sh" -

# Review a diff file
"$SKILL_DIR/scripts/specialist-review.sh" /path/to/changes.diff

# Custom output directory
"$SKILL_DIR/scripts/specialist-review.sh" --git --output ./my-reviews -- src/

# Force a specific provider
"$SKILL_DIR/scripts/specialist-review.sh" --provider gemini --git -- src/parser/
"$SKILL_DIR/scripts/specialist-review.sh" --provider codex --git -- src/parser/

Always scope to the files you touched. In a monorepo, an unscoped --git sends the entire repo diff to the reviewer, wasting tokens and risking timeouts.

Provider selection:

• Default is auto, which tries Claude first, keeps the current agent's own model

family last, and uses the remaining provider in between.

• Override per run with --provider auto|claude|gemini|codex.
• Override by environment with AGENT_LOOPS_LLM_PROVIDER or SPECIALIST_REVIEW_PROVIDER.
• Override provider models with CLAUDE_MODEL, GEMINI_MODEL, or CODEX_MODEL.
• Enable a second reviewer with AGENT_LOOPS_SECONDARY_PROVIDER=claude|gemini|codex

and optionally AGENT_LOOPS_SECONDARY_MODEL=<model>. Use SPECIALIST_REVIEW_SECONDARY_PROVIDER / SPECIALIST_REVIEW_SECONDARY_MODEL for code-review only, or TEST_REVIEW_SECONDARY_PROVIDER / TEST_REVIEW_SECONDARY_MODEL for test-audit only. The script preserves both raw artifacts and emits one synthesized final artifact.

• To conserve Claude usage while keeping a second signal, use:

CLAUDE_MODEL=sonnet AGENT_LOOPS_SECONDARY_PROVIDER=codex or CLAUDE_MODEL=sonnet AGENT_LOOPS_SECONDARY_PROVIDER=gemini.

• Set AGENT_LOOPS_SELF_PROVIDER=claude|gemini|codex when the current agent is not

auto-detected. Codex, Gemini, and Claude sessions auto-detect themselves when their standard session markers are present.

• The script validates the review artifact shape before accepting it; invalid provider output is rejected and the next fallback is tried.
• If both CLIs are unavailable or fail, use the fresh-context Codex fallback below.

Manual Fallback: Fresh-Context Codex

If the bundled script cannot get a usable artifact from any scripted provider:

1. Generate the same scoped diff you would have sent to the script. 2. Provide the reviewer:

• references/review-prompt.md
• references/perspective-catalog.md
• the scoped diff
• the prior review artifact, if this is a remediation cycle

3. Require the reviewer to emit markdown that follows the Review Output Format documented later in this skill. 4. Save that output under .agents/reviews/review-<timestamp>-fallback.md and treat the saved path as REVIEW_FILE.

Size Guards and --jumbo

Both specialist-review.sh and diff-test-audit.sh enforce a soft size guard before sending content to the reviewer:

The guard behaves as a two-step decision gate, not a hard cap:

1. First run aborts on oversize. The abort is deliberate: it forces you to consider whether the change can be split before sending a large payload to the reviewer. The error message lists concrete split strategies (by path filter, by ref range, by sub-module, by logical scope). Try those first.

2. If splitting doesn't make sense, rerun with `--jumbo`. Use this flag when the change is genuinely cohesive and decomposing it would fragment a single logical unit — a refactor that only reads correctly as a whole, generated code, or a single-commit feature whose parts don't stand alone. --jumbo sends the full content to the reviewer (no truncation).

   # First run aborts with splitting guidance
   "$SKILL_DIR/scripts/specialist-review.sh" --git -- src/big-refactor/

   # After deciding the change can't be split, retry with --jumbo
   "$SKILL_DIR/scripts/specialist-review.sh" --jumbo --git -- src/big-refactor/

--jumbo is a considered override, not a default. Use it only after the abort made you think about splitting. Modern provider context windows (Claude Opus/Sonnet 4.x, Gemini 2.5 Pro, GPT-5) can absorb the full payload, but reviewers do their best work on focused diffs — so prefer splitting when it's natural, and reach for --jumbo when it isn't.

AGENT_LOOPS_ALLOW_TRUNCATION=1 still exists for backward compatibility but silently truncates, which produces partial reviews. Prefer --jumbo over truncation in nearly every case.

Anti-Patterns

• Performing the review yourself — Use an independent reviewer, never the implementer.
• Summarizing the diff before invoking the script — Unnecessary. The script reads the diff directly.
• Ignoring the output artifact — The review is written to a file. Read it.
• Using a same-context Codex agent as reviewer — If Codex is the fallback reviewer, it must have fresh context and no implementation authorship.
• Stopping at the first Claude failure — Let the script try the non-self provider before the same-model last resort and fresh-context Codex.
• Skipping the polling pattern — Use the polling invocation from the section above. The review takes 3-5 minutes; without the poll loop the Bash tool will time out or the agent will lose track of the process. Set timeout: 600000 on the Bash call.
• Moving on before you have `REVIEW_FILE` — The review is a gate. Do not proceed to findings triage, remediation, tests, or commit until the poll loop exits and you have a file path.
• Reaching for `--jumbo` before considering a split — The first-run abort is a forcing function. Use --jumbo only after you've decided the change is cohesive; don't paper over a legitimately splittable diff.

diff-test-audit — Request Test Audit

When: Per-commit test coverage check — verifies the files your diff touched have adequate tests. What you get back: A gap report covering both missing coverage AND test quality issues (mirror tests, flaky assertions, etc.) scoped to your changed files, with P0/P1/P2 severity.

Scope: Default to Diff, Not Module

This audit is a per-commit completeness check — did your change get tested — not a module-level survey. Default to scoping by files the diff touched (--git filters the audit to changed files; add -- <paths> to narrow further):

"$SKILL_DIR/scripts/diff-test-audit.sh" <module> --git
"$SKILL_DIR/scripts/diff-test-audit.sh" <module> --git -- <touched-files>

When --git is active, the script uses a diff-focused prompt (references/diff-audit-prompt.md) that explicitly scopes findings to behaviors introduced or modified by the diff. Pre-existing untested code in touched files is out of scope — those gaps are not reported. This prevents the "audit surfaces noise in neighboring code" problem a full-contract prompt would produce even at file scope.

For full module audits (mapping a module's public contract and surveying coverage across unmodified code), use the `test-review` skill instead. It uses parallel Haiku sub-agents with synthesis — better suited to exhaustive module work. The full-contract prompt (audit-prompt.md) is still used by this script when invoked without --git, but that path is legacy — module audits belong in test-review.

IMPORTANT: Do Not Audit the Tests Yourself

Your ONLY job is to invoke an independent auditor and read the output artifact. Do NOT map behaviors yourself. Do NOT classify test coverage yourself. Do NOT produce the gap report yourself. Route the audit to Claude first, then fallback if needed.

Claude is still the preferred auditor because it can apply the testing standards and audit workflow with project-aware skill support. The bundled script now tries Claude first, then a different model family, and keeps same-model shell-outs for last resort. Codex and Gemini are both available as explicit providers. If the automated providers are unavailable or fail, continue with a fresh-context Codex auditor instead of skipping the audit.

Automated Path: Provider-Aware Script

LONG-RUNNING CALL — USE THE POLLING PATTERN BELOW.

This script invokes an external LLM and takes 3-5 minutes for larger modules.

Do NOT start writing tests or commits while the audit is in progress.

When calling from a Bash tool, set timeout: 600000 (10 min) — the default

120-second timeout will kill the subprocess before the auditor finishes.

The audit is only done when you have a REPORT_FILE path in hand.

Polling invocation (REQUIRED) — run the audit in the background and poll so the Bash tool receives periodic output and does not time out:

# Start audit in background, capture stdout (the result file path) separately
# Default scope: --git filters to files the current diff touched
REPORT_TMP=$(mktemp /tmp/audit-out.XXXXXX)
"$SKILL_DIR/scripts/diff-test-audit.sh" /path/to/module --git \
  >"$REPORT_TMP" &
AUDIT_PID=$!

# Poll every 15s — each echo keeps the Bash tool connection alive
while kill -0 "$AUDIT_PID" 2>/dev/null; do
  sleep 15
  echo "[poll] Audit still running (pid $AUDIT_PID)..."
done

# Collect exit code and read result
wait "$AUDIT_PID"
AUDIT_EXIT=$?
if [[ $AUDIT_EXIT -eq 0 ]]; then
  REPORT_FILE=$(cat "$REPORT_TMP")
  echo "Audit complete: $REPORT_FILE"
  cat "$REPORT_FILE"
else
  echo "Audit failed (exit $AUDIT_EXIT):"
  cat "$REPORT_TMP"   # contains failure summary with stderr paths and debug info
fi
rm -f "$REPORT_TMP"

The script emits heartbeat lines to stderr every 15 seconds during provider execution. Combined with the poll loop above, this ensures continuous output.

Additional invocation forms (wrap any of these in the polling pattern above). All default to diff scope via --git; for full module audits use the test-review skill, not this one.

# Default: audit changed files since main
"$SKILL_DIR/scripts/diff-test-audit.sh" /path/to/module --git

# Narrow further with explicit file filter
"$SKILL_DIR/scripts/diff-test-audit.sh" /path/to/module --git -- src/parser/ src/auth.py

# Audit changed files since a specific ref
"$SKILL_DIR/scripts/diff-test-audit.sh" /path/to/module --git origin/main

# Quick review of specific test files only
"$SKILL_DIR/scripts/diff-test-audit.sh" --quick /path/to/test_file.py

# Force a specific provider (still diff-scoped via --git)
"$SKILL_DIR/scripts/diff-test-audit.sh" --provider gemini /path/to/module --git
"$SKILL_DIR/scripts/diff-test-audit.sh" --provider codex /path/to/module --git

Provider selection:

• Default is auto, which tries Claude first, keeps the current agent's own model

family last, and uses the remaining provider in between.

• Override per run with --provider auto|claude|gemini|codex.
• Override by environment with AGENT_LOOPS_LLM_PROVIDER or TEST_REVIEW_PROVIDER.
• Set AGENT_LOOPS_SELF_PROVIDER=claude|gemini|codex when the current agent is not

auto-detected. Codex sessions auto-detect themselves already.

• The script validates the audit artifact shape before accepting it; invalid provider output is rejected and the next fallback is tried.
• If both CLIs are unavailable or fail, use the fresh-context Codex fallback below.

Manual Fallback: Fresh-Context Codex

If the bundled script cannot get a usable artifact from any scripted provider:

1. Gather the same source, tests, and bundled references the script would have used. 2. Provide the auditor:

• references/audit-prompt.md
• references/testing-standards.md
• references/audit-workflow.md
• the scoped module and test content

3. Require the auditor to emit markdown that follows the same gap-report contract used by this skill's audit loop. 4. Save that output under .agents/reviews/test-audit-<timestamp>-fallback.md and treat the saved path as REPORT_FILE.

Act on findings:

• P0 (Security/Correctness Critical): Fix before merge.
• P1 (Reliability/Edge Cases): Fix in current sprint.
• P2 (Completeness/Confidence): Backlog.

Size Guards and --jumbo

diff-test-audit.sh enforces the same two-step size guard as specialist-review.sh (limit: 500 KB of source + tests combined). On the first oversized run the script aborts with splitting suggestions; pass --jumbo on retry if the module is cohesive and cannot be decomposed. See the full explanation under specialist-review → Size Guards and `--jumbo` above — the decision framework is identical.

# First run aborts when source+tests exceed 500 KB
"$SKILL_DIR/scripts/diff-test-audit.sh" /path/to/big/module

# After deciding the module can't be split, retry with --jumbo
"$SKILL_DIR/scripts/diff-test-audit.sh" --jumbo /path/to/big/module

Anti-Patterns

• Performing the audit yourself — Use an independent auditor, never the implementer.
• Pre-reading source before invoking the script — Unnecessary. The script passes the module path to the provider flow.
• Ignoring the output artifact — The gap report is written to a file. Read it.
• Using a same-context Codex agent as auditor — If Codex is the fallback auditor, it must have fresh context and no authorship of the tested change.
• Proceeding without any audit artifact — Let the script try the non-self provider before the same-model last resort and fresh-context Codex.
• Skipping the polling pattern — Use the polling invocation from the section above. The audit takes 3-5 minutes; without the poll loop the Bash tool will time out or the agent will lose track of the process. Set timeout: 600000 on the Bash call.
• Moving on before you have `REPORT_FILE` — The audit is a gate. Do not proceed to gap analysis, test writing, or commit until the poll loop exits and you have a file path.
• Reaching for `--jumbo` before considering a split — The first-run abort is a forcing function. Use --jumbo only after you've decided the module is cohesive; don't paper over a legitimately splittable audit.

---

Team Review: User-Triggered Only

Multi-specialist team review is not an agent tool. It is a user-triggered operation — the cost profile (~$2-3 per review) and authority profile (PR-level / security-sensitive changes) warrant explicit user invocation.

The orchestration lives in a dedicated skill: `multi-specialist-review`. Implementers using this skill do not invoke it themselves.

When to flag for team review in your handoff

Recommend team review to the user when the change scope matches any of:

• PR-level changes (5+ modified files)
• Multi-commit ranges (main..feature-branch) rather than atomic commits
• Security-sensitive paths (auth, crypto, payments, input validation)
• Single-turn specialist-review flagged quality concerns you could not resolve

Your handoff should name which criterion triggered the recommendation, the scope a team review should cover, and any findings from specialist-review.sh that warrant a second look. Do not block on the user running team review — continue with single-turn specialist-review.sh in the meantime.

See skills/multi-specialist-review/SKILL.md for the orchestration details (Claude Code only — requires the team API).

---

Atomic Commits: The Unit of Work

Every loop operates on atomic commits, not features. An atomic commit is the smallest change that is complete, correct, and reviewable in isolation.

What Makes a Commit Atomic

An atomic commit:

• Does one thing. One bug fix, one new behavior, one refactor — not all three.
• Is self-consistent. The codebase compiles, tests pass, and lint is clean at this commit. You could revert it without breaking other commits.
• Is reviewable in isolation. A reviewer can understand the intent and evaluate correctness without seeing what comes next.
• Groups related files. If adding a function requires updating a test and a type — that's one commit, not three.

An atomic commit is NOT:

• An entire feature with multiple independent components.
• A single file (if the logical change spans several files).
• "Everything I did today."

How This Drives the Loops

For a multi-component feature, decompose into a sequence of atomic commits. Each commit gets its own pass through the relevant loop:

Feature: "Add rate limiting to API"

  Commit 1: rate limiter data model       → Code Change Loop → commit
  Commit 2: rate limiter business logic    → Code Change Loop → commit
  Commit 3: integrate into request handler → Code Change Loop → commit
  Commit 4: tests for rate limiter         → Test Writing Loop → commit
  Commit 5: lint fixes                     → Lint Gate → commit

The question at every step is: "What is my next atomic commit?" — not "How do I implement this feature?"

Commit Rules

1. Use `cortex git commit`, not `git commit`. Unless you have explicit user approval to commit another way, always use cortex git commit. It stages files and commits atomically, preventing common mistakes (staging ., empty messages, committing directories). Use cortex git patch when you need to stage individual diff hunks instead of whole files.

   cortex git commit "fix(parser): reject CONNECT requests with missing port" src/parser.rs tests/test_parser.rs
   # For partial staging (previously: committer --patch):
   cortex git patch --diff changes.diff "fix(parser): reject CONNECT requests" src/parser.rs

2. Dirty files are OK — use `cortex git patch`. If a file you need to commit already has uncommitted changes from other work, use cortex git patch to commit only your hunks. You do not need to escalate unless your changes overlap with the already-modified lines. If they overlap, stop and coordinate with the user. 3. Commit at the end of every loop. Each loop exit (code change, test writing, lint gate) produces a commit. Do not batch multiple loop exits into one commit. 4. Run existing tests before committing. After implementation, after lint fixes, after any code change — verify the existing test suite passes. A commit that breaks existing tests is not atomic. 5. Commit messages follow Conventional Commits. <type>(scope): <summary>. The summary should describe what changed, not what you were asked to do.

When to Split

If you find yourself in any of these situations, you need to split:

• Your diff touches more than one module for unrelated reasons.
• You're fixing a bug AND adding a feature in the same change.
• The review would need to evaluate two independent design decisions.
• You can describe your change only with "and" — "adds X and changes Y."

---

Overview

There are three primary loops. They run sequentially — code loop, then test loop, then lint gate.

┌─────────────────────────────────────────────────────────────────┐
│                        CODE CHANGE LOOP                         │
│  Implement → specialist-review → Remediate → specialist-review  │
│  Exit: all P0/P1 findings resolved                              │
│  Output: clean code + issues filed for P2+                      │
├─────────────────────────────────────────────────────────────────┤
│                        TEST WRITING LOOP                        │
│  Audit → Write Tests → Verify → Re-audit → Remediate → ...    │
│  Exit: all P0/P1 gaps covered, no bad tests                    │
│  Output: tests passing + issues filed for P2+                   │
├─────────────────────────────────────────────────────────────────┤
│                        LINT GATE                                │
│  Discover Linter → Auto-fix → Check → Remediate → Re-check    │
│  Exit: zero errors, no new warnings                             │
│  Output: lint-clean code                                        │
└─────────────────────────────────────────────────────────────────┘

---

Loop 1: Code Change Loop

Severity Levels

The Loop

ENTRY: Next atomic commit from your decomposition plan.
       (One logical change — see "Atomic Commits" above.)

┌──────────────────┐
│  IMPLEMENT       │ ← You (Codex/Gemini): write ONE atomic commit's worth of change
└──────┬───────────┘
       │
       ▼
┌──────────────────┐
│ specialist-review│ ← BLOCKING: run the script, wait for REVIEW_FILE, read it.
└──────┬───────────┘   Do NOT proceed until you have the artifact in hand.
       │                Scope to SOURCE files only — tests are reviewed in Loop 2.
       │
       ├── Findings? ──► Yes ──► Any P0 or P1? ──► Yes ──┐
       │                                                   │
       │                        No ──► File P2/P3 issues   │
       │                               Run tests ──► Pass? │
       │                               Commit (cortex git commit)  │
       │                               Exit loop           │
       │                                                   │
       │   No findings ──► Run tests ──► Pass?             │
       │                   Commit (cortex git commit)               │
       │                   Exit loop                        │
       │                                                   │
       ▼                                                   ▼
                                          ┌──────────────────┐
                                          │  REMEDIATE       │ ← You: fix ONLY P0/P1
                                          └──────┬───────────┘   findings cited by the review artifact
                                                 │
                                                 ▼
                                          ┌──────────────────┐
                                          │ specialist-review│ ← Run with --prior-review and only remediated files:
                                          └──────┬───────────┘   "$SKILL_DIR/scripts/specialist-review.sh" --git \
                                                 │                 --prior-review "$REVIEW_FILE" -- <remediated-files>
                                                 └── Loop back to findings check

Deferred Findings Are Mandatory Backlog Items

No loop closes with an unfixed finding unless there is a fixing commit, a backlog item ID, or explicit user waiver. Specifically:

• P0 / P1: Do not close the loop. Fix it or escalate to the user for approval.
• P2 / P3: Create a backlog item before committing and exiting the loop.

Include in the backlog item:

• Review/audit artifact path
• Finding ID and title (e.g., P2-003: Missing input validation)
• Affected file(s)
• Why it was deferred
• In the final handoff message, list every deferred finding with its backlog item ID.

Do not silently carry deferred findings forward. Do not defer filing to "later" — the backlog item must exist before the loop exit commit.

Circuit Breaker

Maximum iterations: 3 `specialist-review` cycles.

If P0/P1 findings remain after 3 cycles: 1. Stop. Do not attempt a 4th remediation. 2. Produce a summary of unresolved findings with context on why they persist. 3. Escalate to human reviewer with the summary and the latest review artifact.

This prevents infinite loops when you keep introducing new issues while fixing old ones, or when a finding requires a design-level change you can't make in remediation scope.

Code Review Checklist (Reviewer Criteria)

Any reviewer evaluates against these criteria. Use them to anticipate and prevent issues before review, and to interpret findings during remediation.

Correctness:

• [ ] Does the code do what the spec says? Not more, not less.
• [ ] Are all error cases handled? No unwrap() on fallible operations in non-test code (Rust). No unhandled promise rejections (TS).
• [ ] Are numeric operations safe from overflow/underflow?
• [ ] Are string operations safe with unicode input?
• [ ] Does the code handle empty/zero/None inputs?

Security (when applicable):

• [ ] Is untrusted input validated before use?
• [ ] Are auth checks present on protected paths?
• [ ] Is sensitive data (keys, tokens) excluded from logs and error messages?
• [ ] Are timing-safe comparisons used for secrets?

Patterns and conventions:

• [ ] Does the code follow existing patterns in the codebase? (highest precedence)
• [ ] Are new types/functions placed in the right modules?
• [ ] Do public APIs have doc comments?
• [ ] Are error types specific and actionable?

Resource management:

• [ ] Are connections, file handles, and locks properly released?
• [ ] Are timeouts set on all I/O operations?
• [ ] Do caches/pools have bounded capacity?
• [ ] Is cleanup handled on error paths, not just success paths?

Concurrency (when applicable):

• [ ] Is shared state accessed through appropriate synchronization?
• [ ] Are lock scopes minimized?
• [ ] Could this deadlock? (nested locks, await while holding lock)

Review Output Format (What the Reviewer Returns)

All review paths must produce this format. Parse it to determine your next action.

## Code Review: [change description]

**Files reviewed:** [list]
**Iteration:** N of 3

### Findings

#### P0-001: [title]
**File:** `src/tunnel.rs:45-52`
**Issue:** [what's wrong]
**Impact:** [what happens if not fixed]
**Suggested fix:** [specific guidance, not just "fix this"]

#### P1-001: [title]
**File:** `src/auth.rs:23`
**Issue:** [what's wrong]
**Impact:** [what happens if not fixed]
**Suggested fix:** [specific guidance]

#### P2-001: [title]
**File:** `src/config.rs:100`
**Issue:** [what's wrong]
**Recommendation:** [what to improve]

### Summary
- P0: N findings (MUST fix)
- P1: N findings (MUST fix)
- P2: N findings (file issues)
- P3: N findings (file issues)
- **Verdict:** BLOCKED / PASS WITH ISSUES / CLEAN

Remediation Rules

When fixing findings from the review artifact:

• Fix ONLY the cited findings. Do not refactor adjacent code.
• Do not introduce new functionality while remediating.
• If a fix requires changing the approach significantly, note this in the remediation and let the next reviewer evaluate the full new approach on the next review cycle.
• Each remediated finding should be annotated: Fixed P0-001: [what was changed]
• Scope remediation reviews to only the files you touched. Pass --prior-review "$REVIEW_FILE" when using Claude, or provide the prior artifact when using a fallback reviewer, so the next review can verify fixes and check for regressions without re-reviewing already-approved code:

  # Initial review — full scope
  REVIEW_FILE=$("$SKILL_DIR/scripts/specialist-review.sh" --git -- src/parser/ src/auth.rs)

  # Remediation review — only the files you fixed, with prior review for continuity
  REVIEW_FILE=$("$SKILL_DIR/scripts/specialist-review.sh" --git --prior-review "$REVIEW_FILE" -- src/auth.rs)

• If you disagree with a finding, see "Disagreeing with Review Findings" below — do not silently skip it.

---

Loop 2: Test Writing Loop

This loop runs after the code change loop exits cleanly. It ensures the new (and existing) code has adequate test coverage.

The audit does double duty: it finds missing coverage AND flags bad tests (mirror tests, flaky assertions, etc.). A bad test doesn't close a gap, so a single audit pass catches both problems. No separate quality review step needed.

Roles

Pragmatic Enforcement Policy

Use a hybrid gate to avoid unnecessary friction while preserving confidence:

• Non-trivial code changes (default): The test-writing loop is required. Do not close the loop without audit evidence from diff-test-audit or a fallback audit artifact, and a re-audit state with no unresolved P0/P1 gaps.
• Trivial changes (explicit exception): Docs-only, comments-only, formatting-only, or rename-only edits may skip the audit loop.
• Mandatory note for skips: Every skip must include audit skipped: trivial with a one-line reason in the loop summary.
• Uncertainty rule: If it is not clearly trivial, run the audit.

Practical close criteria for implementer loops: 1. Tests added or updated for the change (when behavior changed). 2. Local verification commands run and reported. 3. Audit evidence present, or explicit trivial skip note present.

The Loop

ENTRY: Code change loop has exited cleanly.

┌──────────────────────┐
│  AUDIT               │ ← BLOCKING: run the script, wait for REPORT_FILE, read it.
└──────┬───────────────┘   Do NOT proceed until you have the artifact in hand.
       │                   Claude script first; fallback auditor uses the same scoped materials.
       ▼
┌──────────────────────┐
│  SCOPE APPROVAL      │ ← Human reviews gap report
└──────┬───────────────┘   P0/P1 auto-approved. P2+ at human discretion.
       │                   Approved gaps become your work list.
       ▼
┌──────────────────────┐
│  WRITE TESTS         │ ← You (Codex/Gemini): write tests for P0 first,
└──────┬───────────────┘   then P1. Follow testing-standards.md.
       │
       ▼
┌──────────────────────┐
│  VERIFY              │ ← You: run the tests locally. They must:
└──────┬───────────────┘   1. Compile / pass lint (test files — full lint in Loop 3)
       │                   2. All pass (no test is born failing)
       │                   3. Actually exercise the code (not no-ops)
       ▼
┌──────────────────────┐
│  RE-AUDIT            │ ← BLOCKING: run the script, wait for REPORT_FILE, read it.
└──────┬───────────────┘   Do NOT proceed until you have the artifact in hand.
       │                   Same module path — script re-reads source + tests.
       │                   Reviewer checks: gaps closed? new tests good?
       │
       ├── All P0/P1 resolved? ──► Yes ──► File P2/P3 issues
       │                                   Run full test suite
       │                                   Commit (cortex git commit)
       │                                   Exit loop ✅
       │
       └── No ──► Any P0/P1 remaining?
                  │
                  ▼
           ┌──────────────────┐
           │  REMEDIATE       │ ← You: fix/rewrite the flagged tests
           └──────┬───────────┘   or write tests for remaining gaps
                  │
                  └── Back to VERIFY

What the Audit Catches

The audit report covers both gap analysis and quality in a single pass:

Coverage gaps (missing tests):

• Behaviors in the public contract with no corresponding test
• Error paths with no failure test
• Edge cases (empty input, boundary values, unicode) not exercised

Test quality issues (bad tests):

• Mirror tests — expected values computed from implementation, not hardcoded
• Trivial assertions — assert(true), assertions that can never fail
• Happy-path-only — tested behavior has no edge case or failure test
• Over-mocking — mocks at internal boundaries, not just external (network, fs, time)
• Flaky patterns — timing-dependent assertions, hardcoded ports, shared state

A bad test shows up as an unclosed gap. A mirror test for behavior X means X is still "Missing" in the gap report, not "Covered". This is why one audit pass is sufficient.

Audit Severity

Circuit Breaker

Maximum iterations: 3 audit cycles (initial audit + 2 re-audits).

If P0/P1 gaps remain after 3 cycles, escalate to human with a summary of what's proving difficult to test and why. This usually indicates the code needs refactoring to be testable — that's a design problem, not a test problem.

If Claude was unavailable for one or more audit cycles, include that fact in the summary so humans know the audit path used.

---

Loop 3: Lint Gate

After the test writing loop exits cleanly, run the project linter against ALL files touched across both loops. Lint fixes are code changes — they may produce deferred findings or break tests — so the lint gate runs before issue filing.

Linter Discovery

Discover the project's linter using this cascade. Stop at the first match:

1. Project docs — Check CLAUDE.md, README.md, and CONTRIBUTING.md for lint commands or conventions. 2. Task runner targets — Look for standard targets: make lint, npm run lint, cargo clippy, ./gradlew lint, bundle exec rubocop, etc. 3. Config file inference — Match config files to linters:

• .eslintrc* / eslint.config.* → npx eslint
• pyproject.toml [tool.ruff] → ruff check
• pyproject.toml [tool.black] → black --check
• .prettierrc* → npx prettier --check
• Cargo.toml → cargo clippy
• .rubocop.yml → bundle exec rubocop
• .golangci.yml → golangci-lint run
• biome.json → npx biome check

4. Fallback — If no linter is discoverable, escalate to the user. Never guess.

The Loop

ENTRY: Test writing loop has exited cleanly.

┌──────────────────────┐
│  DISCOVER LINTER     │ ← You: use the 4-step cascade above
└──────┬───────────────┘
       │
       ▼
┌──────────────────────┐
│  AUTO-FIX            │ ← You: run linter's auto-fix command (see table below)
└──────┬───────────────┘   Eliminates 80%+ of issues without iteration cost
       │
       ▼
┌──────────────────────┐
│  LINT CHECK          │ ← You: run lint check command
└──────┬───────────────┘
       │
       ├── Clean? ──► Yes ──► Run full test suite ──► Pass?
       │                      Commit (cortex git commit)
       │                      Exit loop ✅
       │
       └── No ──► Errors or new warnings remain
                  │
                  ▼
           ┌──────────────────┐
           │  REMEDIATE       │ ← You: manually fix remaining lint issues
           └──────┬───────────┘
                  │
                  ▼
           ┌──────────────────┐
           │  VERIFY TESTS    │ ← You: re-run tests to confirm lint fixes
           └──────┬───────────┘   didn't break anything
                  │
                  └── Back to LINT CHECK

What "Clean" Means

• Zero errors from the project linter.
• No new warnings introduced by your changes.
• Pre-existing warnings in unmodified files are noted but not blocking.

Auto-fix Reference

Auto-fix Rules

1. Always auto-fix before manual remediation. Auto-fix eliminates the mechanical majority. 2. Re-run tests after auto-fix. If auto-fix breaks tests, revert the auto-fix and remediate manually. 3. Revert if auto-fix breaks tests. A passing test suite takes priority over lint cleanliness.

Circuit Breaker

Maximum iterations: 2 lint-check cycles (initial check + 1 re-check after remediation).

Lint is mechanical, not semantic. If you can't get clean in 2 cycles, the issue is either a misconfigured rule or a design problem: 1. Stop. Do not attempt a 3rd remediation. 2. Summarize unresolved lint errors with context on why they persist. 3. Escalate to human reviewer with the summary.

Severity Model

Lint findings use a binary pass/fail model — no P0/P1/P2 triage. Lint rules are team policy: the agent complies, it doesn't judge. If a rule seems wrong, escalate to the user rather than disabling or ignoring it.

---

Loop 4: Issue Filing Verification

Deferred findings should already have backlog items from their respective loops (see "Deferred Findings Are Mandatory Backlog Items" above). This step verifies completeness: check that every deferred P2/P3 from all review and audit artifacts has a corresponding backlog item. File any that were missed.

Backlog-First Policy

When filing deferred findings in this repository:

• If a backlog/ folder exists at repo root and Backlog tooling is available (Backlog MCP tools and/or Backlog CLI), use Backlog to create tracked issues/tasks.
• Include enough context for another agent to execute without re-auditing:
• source review/audit artifact
• affected files/modules
• severity and user impact
• suggested fix direction
• acceptance criteria
• If Backlog is not available, create a markdown handoff under .agents/fixes/.
• Prefer naming by replacing review with fix from the source artifact name (s/review/fix/).
• If no review artifact name exists, use a descriptive *-fix.md filename.
• Implementers may also choose to fix deferred findings immediately instead of filing, when appropriate.

Issue Template

## [P2/P3] [Module]: [Brief description]

**Source:** [Code Review / Test Audit] iteration N
**Severity:** P2 | P3
**Module:** [file path]

### Description
[What's missing or what could be improved]

### Context
[Why this was deferred — not blocking but worth addressing]

### Suggested approach
[Brief guidance on how to address]

### Acceptance criteria
[How to verify this is done]

Filing Rules

• One issue per finding. Do not batch unrelated findings.
• P2 issues get the label quality or test-gap as appropriate.
• P3 issues get the label improvement.
• Issues from test audits reference the specific behavior from the gap report.
• Issues include enough context that a different agent (or human) can pick them up without re-auditing.

---

Operational Notes

Agent Responsibilities Summary

You (Codex / Gemini) are responsible for:

• Reading and understanding the task spec
• Decomposing the task into atomic commits (one logical change each)
• Writing implementation code — one atomic commit at a time
• Writing test code
• Running the full test suite before every commit
• Using cortex git commit for all commits (not git commit) unless explicitly approved otherwise
• Committing at the end of every loop exit (code change, test writing, lint gate)
• Running "$SKILL_DIR/scripts/specialist-review.sh" --git -- <your-files> after implementation; on remediation cycles, scope to remediated files and pass --prior-review "$REVIEW_FILE"
• Running "$SKILL_DIR/scripts/diff-test-audit.sh" <module> --git (diff-scoped) for initial audit and each re-audit; reach for the test-review skill if you need a full module audit instead
• Preserving fallback review/audit artifacts in .agents/reviews/ when Claude is unavailable
• Falling back to another model family before the same-model last resort when Claude is unavailable
• Fixing ONLY the findings the independent review/audit artifact identifies (no scope creep during remediation)
• Discovering and running the project linter after both loops exit
• Running auto-fix first, then manually fixing remaining lint issues
• Verifying tests still pass after lint fixes
• Escalating if lint cannot be resolved in 2 cycles
• Filing P2/P3 issues when loop exits
• Escalating when circuit breaker triggers

You are NOT responsible for:

• Reviewing your own code (an independent reviewer does this)
• Judging your own test coverage or quality (an independent auditor does this)
• Deciding whether a finding is valid (if you disagree, note it in the remediation and let the next review re-evaluate — do not silently skip findings)

Disagreeing with Review Findings

If the reviewer flags something you believe is incorrect: 1. Do NOT silently ignore the finding. 2. In your remediation response, explicitly state: Disputed P1-003: [your reasoning] 3. Run the next review pass with the dispute noted in your commit/changes or fallback review materials. 4. The next review pass will either accept your reasoning or reaffirm the finding with additional context. 5. If still disputed after 2 cycles, escalate to human — do not loop forever on a disagreement.

What "Escalate to Human" Means

When the circuit breaker triggers: 1. Stop all loops. 2. Produce a structured summary:

• What was attempted
• What findings remain unresolved
• Why they persist (agent's best assessment)
• Recommended human action

3. Do not attempt creative workarounds to avoid escalation.

Metrics (Track If Tooling Supports)

Per loop run:

• Number of iterations before exit
• Findings by severity per iteration
• Number of findings that regressed (fixed then reappeared)
• Time per iteration
• Circuit breaker activations

These metrics help tune the loop — if you're consistently hitting 3 iterations, either the review checklist is too strict or the implementer instructions need work.

---

Quick Reference: The Full Pipeline

1. TASK SPEC arrives
       │
       ▼
2. DECOMPOSE into atomic commits
   └── Each commit = one logical change, reviewable in isolation
       │
       ▼
3. FOR EACH atomic commit:
   │
   ├── CODE CHANGE LOOP
   │   ├── You: implement ONE commit's worth of change
   │   ├── specialist-review → Claude reviews diff, else non-self provider, else same-model last resort, else fresh-context Codex (max 3 cycles)
   │   ├── You: remediate P0/P1 → re-review with prior artifact
   │   ├── File issues for P2/P3
   │   ├── Run tests → Pass?
   │   └── cortex git commit "type(scope): summary" <files>
   │       │
   │   ├── TEST WRITING LOOP
   │   │   ├── diff-test-audit → Claude audits, else non-self provider, else same-model last resort, else fresh-context Codex
   │   │   ├── Human: scope approval (P0/P1 auto-approved)
   │   │   ├── You: write tests (testing-standards.md)
   │   │   ├── You: verify tests pass locally
   │   │   ├── diff-test-audit → re-audit using the same reviewer chain (max 3 cycles)
   │   │   ├── You: remediate P0/P1 gaps and bad tests
   │   │   ├── File issues for P2/P3
   │   │   ├── Run full test suite → Pass?
   │   │   └── cortex git commit "test(scope): summary" <files>
   │   │       │
   │   └── LINT GATE
   │       ├── You: discover project linter
   │       ├── You: run auto-fix if available
   │       ├── You: run lint check (max 2 cycles)
   │       ├── You: remediate remaining issues
   │       ├── Run full test suite → Pass?
   │       └── cortex git commit "style(scope): summary" <files>
   │
   └── Next atomic commit (back to step 3)
       │
       ▼
4. ISSUE FILING
   └── P2/P3 findings → tracked issues
       │
       ▼
5. PR READY FOR HUMAN REVIEW

You are performing a test coverage audit. All source code, test code, and reference standards are provided below. Do NOT read any files — everything you need is in this prompt.

Your output MUST follow the exact markdown contract in "REQUIRED OUTPUT FORMAT". Do not invent alternative section names.

Output the COMPLETE report as a single markdown document to stdout. The very first non-whitespace characters of your output must be ## Test Gap Report:.

CONSTRAINTS

1. No tools. Do not use Read, Write, Bash, or any other tools. Output the report directly. 2. Do NOT spawn sub-agents. 3. Stay focused. Only audit the provided module and tests. 4. Use only these coverage labels: Covered, Shallow, Missing. 5. Use only these severities for gaps: P0, P1, P2. 6. Do not output analysis notes outside the required format. 7. Do not prepend status text. Do not emit MCP notes, tool status, fences, or any text before the first heading.

TESTING STANDARDS

{{TESTING_STANDARDS}}

AUDIT WORKFLOW

{{AUDIT_WORKFLOW}}

SOURCE CODE

Module: {{MODULE_PATH}}

{{SOURCE_CONTENT}}

TEST CODE

Tests: {{TEST_PATH}}

{{TEST_CONTENT}}

YOUR TASK

1. Map the public contract from the source code above — list every public function/method, its error conditions, edge cases, state transitions, and integration points.

2. Map existing test coverage from the test code above — mark each behavior as:

• Covered — a test exercises it with meaningful assertions
• Shallow — a test touches it but assertions are weak (mirror test, trivial assert, no edge case)
• Missing — no test exercises it

3. Analyze and prioritize each Missing or Shallow behavior:

• P0: Security flaw or silent incorrect behavior if untested
• P1: Reliability risk, missing error handling, edge cases
• P2: Completeness improvement, nice-to-have coverage

4. Output the report directly to stdout using the exact format below.

REQUIRED OUTPUT FORMAT

## Test Gap Report: [module path or module name]

**Module:** `[module path]`
**Tests:** `[test path or "(none)"]`
**Mode:** [full|quick]

### Behavior Inventory

| Behavior | Coverage | Evidence |
|----------|----------|----------|
| [behavior description] | Covered / Shallow / Missing | [test name, file, or reason] |

### Prioritized Gaps

#### P0-001: [title]
**Behavior:** [behavior description]
**Status:** Missing / Shallow
**Why it matters:** [risk]
**Suggested test approach:** [how to test it]

#### P1-001: [title]
**Behavior:** [behavior description]
**Status:** Missing / Shallow
**Why it matters:** [risk]
**Suggested test approach:** [how to test it]

#### P2-001: [title]
**Behavior:** [behavior description]
**Status:** Missing / Shallow
**Why it matters:** [risk]
**Suggested test approach:** [how to test it]

### Summary
- Covered: [count]
- Shallow: [count]
- Missing: [count]
- P0: [count]
- P1: [count]
- P2: [count]

Additional rules:

• Include every public behavior in ### Behavior Inventory
• If there are no gaps, still include ### Prioritized Gaps, then write _No prioritized gaps._
• Number gaps separately within each severity (P1-001, P1-002, etc.)
• A shallow test must still appear in ### Prioritized Gaps
• Do not include any sections other than ## Test Gap Report, ### Behavior Inventory, ### Prioritized Gaps, and ### Summary
• Do not include overall prose introductions before the required sections
• Copy the section headings exactly as written above

Mode: {{MODE}}

Test Audit Workflow

This document defines the process for discovering untested or under-tested behavior in a codebase. The output is a prioritized gap report — not code. Do not write test implementations until the report is reviewed and approved.

See testing-standards.md for test quality standards that apply when tests are eventually written.

---

When to Run This Audit

• Before starting a new feature in an existing module
• After a significant refactor
• When preparing a module for production deployment
• On request ("audit tests for module X")
• Periodically as a codebase health check

---

Audit Process

Step 1: Map the Public Contract

Read the module's source code and produce a list of every public behavior it promises. This is NOT a list of functions — it's a list of things the module does.

Sources of contract information (check all of these):

• Public function/method signatures and their doc comments
• Type definitions and their invariants (e.g., "port must be 1-65535")
• Error types and when each variant should occur
• Trait implementations and what they promise
• Configuration options and their effects
• State transitions (if stateful)
• Concurrency guarantees (Send, Sync, thread-safe claims)
• Performance claims (timeouts, capacity limits, eviction policies)

Format each behavior as a plain-English statement:

BEHAVIORS for connect_parser:
- Parses valid CONNECT requests into (host, port) pairs
- Rejects CONNECT requests with missing port
- Rejects CONNECT requests with non-numeric port
- Rejects CONNECT requests with port outside 1-65535
- Rejects non-CONNECT HTTP methods
- Handles IPv6 addresses in bracket notation
- Handles IDN / punycode hostnames
- Preserves original Host header if present
- Returns specific error variants for each failure mode

Step 2: Map Existing Test Coverage

Read every test file that covers the module. For each test, record:

• What behavior it exercises (map to Step 1 list)
• Whether it tests the happy path, edge case, or failure mode
• Whether assertions are meaningful (see testing-standards.md anti-patterns)

Mark each behavior from Step 1:

• ✅ Covered — at least one test verifies this with meaningful assertions
• ⚠️ Shallow — a test touches this but doesn't properly verify it (e.g., only happy path, trivial assertions, mirror testing)
• ❌ Missing — no test exercises this behavior

Step 3: Adversarial Analysis

For each module, ask these questions. If you can't confidently answer "yes, a test catches that," it's a gap.

Input boundaries:

• What happens at zero / empty / None for every input?
• What happens at maximum values (u32::MAX, max capacity, longest valid string)?
• What happens one past maximum?
• What happens with malformed input that's almost-valid?
• What happens with unicode / special characters where strings are accepted?

Error handling:

• Does every Result::Err variant have a test that triggers it?
• Does every Option::None path have a test?
• If a function calls a fallible dependency, is the failure case tested?
• Are error messages/types specific enough to diagnose issues?

State and concurrency:

• If the module has state, is every transition tested?
• If state has limits (LRU capacity, TTL), are eviction/expiry tested?
• If shared across threads, is concurrent access tested?
• Is cleanup on drop/shutdown tested?

Integration seams:

• Where this module connects to another, is the contract at the boundary tested?
• If this module consumes config, are invalid/missing config values tested?
• If this module emits metrics, are metric values verified (not just "didn't panic")?

The "subtle bug" test: For each critical function, imagine these mutations and ask if any test would catch them:

• Off-by-one on a comparison (< vs <=)
• Swapped arguments in a function call
• Wrong default value
• Missing .await on an async call (for Rust: this is a compile error, skip)
• Short-circuit evaluation hiding a bug (early return before important side effect)
• Integer overflow / underflow on arithmetic

Step 4: Produce the Gap Report

---

Gap Report Format

# Test Gap Report: [Module Name]

**Audit date:** YYYY-MM-DD
**Source files audited:** [list paths]
**Test files audited:** [list paths]
**Current test count:** N tests
**Estimated coverage:** X% (if tooling available, otherwise "manual audit")

---

## Summary

[2-3 sentences: overall assessment of test health for this module]

Total behaviors identified: N
- ✅ Covered: N
- ⚠️ Shallow: N  
- ❌ Missing: N

---

## Gaps by Priority

### 🔴 P0 — Security / Correctness Critical

Tests that, if missing, could allow security vulnerabilities, data corruption,
or silent incorrect behavior in production.

| # | Behavior | Current State | Why P0 | Suggested Test |
|---|----------|---------------|--------|----------------|
| 1 | [behavior] | ❌ Missing | [what breaks] | [one-line description] |
| 2 | [behavior] | ⚠️ Shallow | [what's wrong with current test] | [what to fix] |

### 🟠 P1 — Reliability / Edge Cases

Tests for error handling, boundary conditions, and graceful degradation.
Missing these won't cause security issues but will cause operational pain.

| # | Behavior | Current State | Why P1 | Suggested Test |
|---|----------|---------------|--------|----------------|
| 1 | [behavior] | ❌ Missing | [what breaks] | [one-line description] |

### 🟡 P2 — Completeness / Confidence

Tests that round out coverage and prevent regressions during future changes.
Nice to have, lower urgency.

| # | Behavior | Current State | Why P2 | Suggested Test |
|---|----------|---------------|--------|----------------|
| 1 | [behavior] | ❌ Missing | [context] | [one-line description] |

### ✅ Well-Tested (No Action Needed)

[List behaviors that are adequately covered, briefly, so the report shows
the full picture and not just the gaps]

---

## Shallow Test Details

For each ⚠️ Shallow entry, explain what's wrong with the existing test:

### [Test name / behavior]
**Current test:** [what it does]
**Problem:** [mirror testing / happy path only / trivial assertion / over-mocked]
**Recommended fix:** [specific change needed]

---

## Notes

[Any observations about test infrastructure, missing test utilities,
or patterns that would make future testing easier]

---

Priority Assignment Criteria

🔴 P0 — Security / Correctness Critical

Assign P0 when the untested behavior:

• Involves authentication, authorization, or access control
• Parses untrusted input (HTTP requests, TLS ClientHello, config from external sources)
• Makes a security decision (block/allow, risk scoring threshold)
• Handles sensitive data (API keys, credentials, PII in transit)
• Has correctness requirements where "wrong answer silently" is worse than "crash"
• Involves cryptographic operations or certificate validation

Examples:

• CONNECT parser doesn't test request smuggling patterns → P0
• Domain allowlist doesn't test bypass via case sensitivity → P0
• Risk score calculation doesn't test overflow past 100 → P0
• Auth token validation doesn't test expired tokens → P0

🟠 P1 — Reliability / Edge Cases

Assign P1 when the untested behavior:

• Handles a failure mode (network timeout, disk full, OOM)
• Manages resource limits (connection pools, LRU caches, rate limits)
• Involves state transitions that could get stuck or leak
• Deals with concurrency (shared state, race conditions)
• Processes boundary values (empty input, max capacity)
• Affects operational visibility (metrics, logging, health checks)

Examples:

• LRU cache doesn't test eviction under memory pressure → P1
• Tunnel relay doesn't test what happens when origin hangs → P1
• Metric counters don't test overflow after long uptime → P1
• Graceful shutdown doesn't test mid-tunnel disconnect → P1

🟡 P2 — Completeness / Confidence

Assign P2 when the untested behavior:

• Has a working happy-path test but no edge case coverage
• Is configuration-driven with reasonable defaults
• Involves display / formatting logic (not security-relevant)
• Is covered indirectly by integration tests but lacks unit verification
• Would catch regressions during refactors but isn't currently at risk

Examples:

• Config parser doesn't test YAML with extra unknown fields → P2
• Log formatting doesn't test very long message truncation → P2
• Metric labels don't test special character escaping → P2
• CLI help text doesn't test all flag combinations → P2

---

What This Audit is NOT

• Not a coverage report. Line coverage misses semantic gaps. A line can be "covered" by a test that asserts nothing meaningful.
• Not a test plan. This report identifies WHAT is missing. How to implement the tests is governed by testing-standards.md.
• Not busywork. If a module is genuinely well-tested, the report should say so clearly and move on. Don't manufacture gaps to look thorough.

You are performing a per-commit test coverage check on a DIFF. All relevant content (the diff, touched source files, existing tests, testing standards) is provided below. Do NOT read any files — everything you need is in this prompt.

Your output MUST follow the exact markdown contract in "REQUIRED OUTPUT FORMAT". Do not invent alternative section names.

Output the COMPLETE report as a single markdown document to stdout. The very first non-whitespace characters of your output must be ## Test Gap Report:.

SCOPE: DIFF-INTRODUCED BEHAVIORS ONLY

This is a per-commit test completeness check, not a module audit. You are assessing whether the diff's changes are adequately tested — NOT whether the containing files are fully tested.

In scope (report gaps for these):

• New public functions, methods, or classes introduced by the diff
• Modified behaviors in existing functions: new branches, new error paths,

new return values, new side effects, new edge cases the diff introduces

• Newly-added error types or error conditions
• New integration points or contract changes

Out of scope (do NOT report gaps for these):

• Pre-existing functions in touched files that the diff did not modify
• Untested code in neighboring files not in the diff
• Refactors that preserve behavior (no semantic change → no new tests needed)
• Renames, formatting, comment-only changes, dead-code removal
• Documentation or configuration changes

If a function existed before and the diff did not change its behavior, it is out of scope regardless of whether it has tests. Gaps in pre-existing untested code belong in a dedicated module audit via the test-review skill, not here.

If the diff is entirely non-behavioral (formatting, renames, comments), your report should have an empty ### Prioritized Gaps section with _No behavioral changes in diff._ and a Behavior Inventory reflecting that.

CONSTRAINTS

1. No tools. Do not use Read, Write, Bash, or any other tools. Output the report directly. 2. Do NOT spawn sub-agents. 3. Stay focused. Only audit behaviors from the diff. 4. Use only these coverage labels: Covered, Shallow, Missing. 5. Use only these severities for gaps: P0, P1, P2. 6. Do not output analysis notes outside the required format. 7. Do not prepend status text. Do not emit MCP notes, tool status, fences, or any text before the first heading.

TESTING STANDARDS

{{TESTING_STANDARDS}}

THE DIFF

This is the change under audit. Every gap you report must trace to a behavior introduced or modified by this diff. If a behavior is visible in the source below but not in this diff, it is out of scope.

{{DIFF_CONTENT}}

SOURCE FILES (CURRENT STATE)

The files touched by the diff, at their current (post-diff) state. Use these for context on how diff-introduced behaviors interact with surrounding code, but remember: only behaviors from "THE DIFF" are in scope.

{{SOURCE_CONTENT}}

EXISTING TESTS

{{TEST_CONTENT}}

YOUR TASK

1. Identify diff-introduced behaviors. Read "THE DIFF" carefully. List every new or modified public behavior. If the diff contains only non-behavioral changes (formatting, renames, comments), note that and return an empty gap report.

2. Map each diff-introduced behavior to its test coverage. Use the tests above. Mark each as:

• Covered — a test exercises this behavior with meaningful assertions
• Shallow — a test touches it but weakly (mirror test, trivial assert, happy-path-only, missing error case)
• Missing — no test exercises this behavior

3. Report gaps for Shallow and Missing behaviors only.

• P0: Security flaw or silent incorrect behavior if untested
• P1: Reliability risk, missing error handling, meaningful edge case introduced by the diff
• P2: Completeness improvement for this diff's changes

4. Do not report gaps for pre-existing untested code. Those are out of scope for this audit mode. A module audit (via the test-review skill) is the right tool for that.

5. Output using the format below.

REQUIRED OUTPUT FORMAT

## Test Gap Report: [module path or module name]

**Module:** `[module path]`
**Tests:** `[test path or "(none)"]`
**Mode:** diff

### Behavior Inventory

Behaviors introduced or modified by this diff:

| Behavior | Coverage | Evidence |
|----------|----------|----------|
| [diff-introduced behavior] | Covered / Shallow / Missing | [test name, file, or reason] |

### Prioritized Gaps

#### P0-001: [title]
**Behavior:** [diff-introduced behavior description]
**Status:** Missing / Shallow
**Why it matters:** [risk specific to this change]
**Suggested test approach:** [how to test it]

#### P1-001: [title]
**Behavior:** [diff-introduced behavior description]
**Status:** Missing / Shallow
**Why it matters:** [risk specific to this change]
**Suggested test approach:** [how to test it]

#### P2-001: [title]
**Behavior:** [diff-introduced behavior description]
**Status:** Missing / Shallow
**Why it matters:** [risk specific to this change]
**Suggested test approach:** [how to test it]

### Summary
- Covered: [count]
- Shallow: [count]
- Missing: [count]
- P0: [count]
- P1: [count]
- P2: [count]

Additional rules:

• The Behavior Inventory lists only diff-introduced behaviors, not every public behavior in the touched files
• If the diff introduced no behaviors (pure formatting/renames), the inventory may have one row noting that
• If there are no gaps, still include ### Prioritized Gaps, then write _No prioritized gaps._
• Number gaps separately within each severity (P1-001, P1-002, etc.)
• A shallow test must still appear in ### Prioritized Gaps
• Do not include any sections other than ## Test Gap Report, ### Behavior Inventory, ### Prioritized Gaps, and ### Summary
• Do not include overall prose introductions before the required sections
• Copy the section headings exactly as written above

Mode: diff

Perspective Catalog

Maps review perspectives to trigger signals and relevant skills. Use this catalog to determine which perspectives apply to a given diff and which skills to load for each.

Perspectives

Correctness

Trigger signals: Any code change — logic, control flow, data transformation, state mutation.

Focus: Logic errors, off-by-one, null/undefined handling, edge cases, race conditions, contract violations, type mismatches, incomplete error propagation.

Relevant skills: (none required — this is a foundational perspective that applies universally)

---

Security

Trigger signals: Auth/authz code, user input handling, API endpoints, database queries, file operations, secrets/env vars, crypto, HTTP headers, CORS, cookie handling, dependencies.

Relevant skills:

• owasp-top-10 — OWASP Top 10 vulnerability detection and remediation
• secure-coding-practices — Defensive programming, input validation, output encoding
• threat-modeling-techniques — STRIDE analysis, attack surface evaluation
• security-testing-patterns — SAST/DAST patterns, vulnerability assessment

---

Performance

Trigger signals: Loops over collections, database queries, network calls, file I/O, memory allocation, caching logic, batch operations, algorithmic changes, rendering paths.

Relevant skills:

• python-performance-optimization — Profiling, vectorization, memory optimization (Python files)
• react-performance-optimization — Memoization, code splitting, render optimization (React files)
• workflow-performance — Systematic performance analysis methodology
• database-design-patterns — Query optimization, indexing (when DB queries involved)

---

Maintainability

Trigger signals: Any code change — naming, structure, coupling, cohesion, duplication, complexity, readability, abstraction levels.

Focus: Naming clarity, single responsibility, DRY violations, cognitive complexity, dead code introduction, unclear intent, missing context, over-abstraction, under-abstraction.

Relevant skills:

• code-quality-workflow — Quality assessment methodology and improvement patterns

---

Testing

Trigger signals: Test file changes, testable logic additions, public API changes, bug fixes (regression test needed), new error paths.

Focus: Test coverage gaps, assertion quality, edge case coverage, test isolation, mock appropriateness, flaky test introduction, missing negative tests.

Relevant skills:

• python-testing-patterns — pytest patterns, mocking, property-based testing (Python)
• test-generation — Coverage-driven test creation methodology
• test-driven-development — TDD red-green-refactor discipline
• testing-anti-patterns — Test smells, bad mocking, test-only production code

---

Architecture

Trigger signals: New modules/packages, cross-boundary imports, interface changes, dependency additions, service communication, data flow changes, configuration structure.

Focus: Layer violations, coupling direction, dependency inversion, interface segregation, domain boundary integrity, circular dependencies, abstraction leaks.

Relevant skills:

• system-design — Component design, data modeling, interface contracts
• api-design-patterns — REST/GraphQL patterns, versioning, error contracts
• microservices-patterns — Service decomposition, communication patterns (if distributed)
• event-driven-architecture — Event sourcing, CQRS (if event-based patterns present)

---

Infrastructure

Trigger signals: Terraform/HCL files, Kubernetes manifests, Helm charts, Dockerfiles, CI/CD configs, deployment scripts, cloud resource definitions.

Focus: Resource misconfiguration, missing limits/quotas, insecure defaults, state management, drift potential, blast radius.

Relevant skills:

• terraform-best-practices — IaC patterns, state management, module design
• kubernetes-deployment-patterns — Deployment strategies, workload patterns
• kubernetes-security-policies — RBAC, pod security, network policies
• helm-chart-patterns — Chart templates, values management
• gitops-workflows — ArgoCD/Flux declarative deployment

---

API Contract

Trigger signals: Endpoint additions/modifications, request/response schema changes, status code changes, header modifications, serialization changes, versioning.

Focus: Breaking changes, backwards compatibility, error response consistency, pagination patterns, authentication/authorization headers, rate limiting.

Relevant skills:

• api-design-patterns — REST/GraphQL design, versioning, HATEOAS
• api-gateway-patterns — Routing, rate limiting, BFF patterns

---

Accessibility

Trigger signals: HTML/JSX/TSX changes, CSS changes, component props for aria-*, role attributes, focus management, color values, form elements, image tags.

Focus: WCAG 2.2 AA compliance, keyboard navigation, screen reader compatibility, color contrast, semantic HTML, focus management, form labeling.

Relevant skills:

• accessibility-audit — WCAG 2.2 AA triage for pages and components

---

UX / Design

Trigger signals: UI component changes, layout modifications, user flow changes, error message text, loading states, empty states, form interactions.

Focus: User flow coherence, error messaging clarity, loading/empty states, progressive disclosure, interaction feedback, visual consistency.

Relevant skills:

• ux-review — Multi-perspective UX review (usability, accessibility, interaction)
• ui-design-aesthetics — Visual quality, progressive disclosure, design patterns

Perspective Selection Rules

1. Always include: Correctness, Maintainability (these apply to every code change) 2. Include by file type:

• .py files → consider Performance (Python), Testing (Python)
• .tsx/.jsx/.html/.css files → consider Accessibility, UX/Design, Performance (React)
• .tf/.hcl files → Infrastructure
• Dockerfile, *.yaml (k8s) → Infrastructure
• *test*/*spec* files → Testing

3. Include by content signals: Scan diff hunks for trigger signals listed above 4. Limit scope: Select 3-5 perspectives maximum to maintain review depth and quality 5. Prioritize: If more than 5 perspectives seem relevant, select the 5 most impactful based on the volume and nature of changes

You are performing a multi-perspective specialist code review. Output the COMPLETE review as a single markdown document to stdout.

Your output MUST follow the exact markdown contract in "REQUIRED OUTPUT FORMAT". Do not invent alternative headings, severity labels, or verdict labels. The very first non-whitespace characters of your output must be ## Code Review:.

CONSTRAINTS

1. No tools. Do not use Read, Write, Bash, or any other tools. Output the review directly. 2. Fresh perspective. When switching perspectives, mentally reset. Each perspective is independent. 3. Use only these severity labels: P0, P1, P2, P3. 4. Use only these verdict labels: BLOCKED, PASS WITH ISSUES, CLEAN. 5. Do not output phase headings. Perform the perspective thinking internally, then emit only the required final review. 6. Do not prepend status text. Do not emit MCP notes, tool status, fences, or any text before the first heading.

PERSPECTIVE CATALOG

{{PERSPECTIVE_CATALOG}}

PROCEDURE

1. Determine which 3-5 perspectives are most relevant using the catalog below. 2. Review the diff through those perspectives internally. 3. Synthesize all findings into the exact output format below.

When triaging perspectives, consider:

• File types and extensions in the diff
• Content signals (auth code, DB queries, UI components, etc.)
• Always include Correctness and Maintainability

SEVERITY MAPPING

Map findings into agent-loops severity labels:

• P0 — Security flaw, data loss, crash, silent incorrect behavior, or merge-blocking correctness issue
• P1 — Error handling gap, reliability issue, missing validation, meaningful edge case, or other must-fix issue
• P2 — Code quality, maintainability, documentation, minor edge case, or non-blocking improvement
• P3 — Style preference, optional polish, or future optimization

REQUIRED OUTPUT FORMAT

Output EXACTLY this structure:

## Code Review: [brief change description]

**Files reviewed:** [comma-separated list]
**Iteration:** [N of 3, or "1 of 3" if unknown]

### Findings

#### P0-001: [title]
**File:** `path/to/file.ext:line` or `path/to/file.ext:start-end`
**Perspective:** [perspective name]
**Issue:** [what is wrong]
**Impact:** [what happens if not fixed]
**Suggested fix:** [specific fix guidance]

#### P1-001: [title]
**File:** `path/to/file.ext:line` or `path/to/file.ext:start-end`
**Perspective:** [perspective name]
**Issue:** [what is wrong]
**Impact:** [what happens if not fixed]
**Suggested fix:** [specific fix guidance]

#### P2-001: [title]
**File:** `path/to/file.ext:line` or `path/to/file.ext:start-end`
**Perspective:** [perspective name]
**Issue:** [what is wrong]
**Recommendation:** [what to improve]

### Summary
- P0: [count] findings (MUST fix)
- P1: [count] findings (MUST fix)
- P2: [count] findings (file issues)
- P3: [count] findings (file issues)
- **Verdict:** BLOCKED / PASS WITH ISSUES / CLEAN

Additional rules:

• If there are no findings, still include ### Findings, then write _No findings._
• Number findings separately within each severity (P1-001, P1-002, etc.)
• Use Suggested fix only for P0 and P1
• Use Recommendation only for P2 and P3
• Do not include any sections other than ## Code Review, ### Findings, and ### Summary
• Do not include triage notes, reasoning traces, phase descriptions, cross-cutting sections, or extra commentary
• Copy the section headings exactly as written above
• Set verdict to:
• BLOCKED if any P0 or P1 findings exist
• PASS WITH ISSUES if only P2/P3 findings exist
• CLEAN if there are no findings

PRIOR REVIEW FINDINGS

The following is the output from the previous review cycle. Use it to:

• Verify that cited findings have been addressed in the current diff
• Check for regressions introduced by remediation
• Maintain continuity — do not re-report findings that were already fixed

{{PRIOR_REVIEW}}

DIFF TO REVIEW

{{DIFF_CONTENT}}

Testing Standards

This document defines testing expectations for all code in this repository. Read this before writing any test code. These are not suggestions — PRs that violate these standards will be rejected.

---

Core Principle: Tests Prove Behavior, Not Implementation

A test's job is to answer: "If someone rewrote the internals completely, would this test still catch a broken contract?"

If the answer is no, the test is worthless.

---

Anti-Patterns (Do NOT Do These)

1. Mirror Testing

BAD: The test restates the implementation logic.

// Implementation
fn calculate_risk(score: u32, multiplier: f64) -> u32 {
    (score as f64 * multiplier).round() as u32
}

// BAD TEST — just re-implements the function
#[test]
fn test_calculate_risk() {
    let score = 50;
    let multiplier = 1.5;
    let expected = (score as f64 * multiplier).round() as u32; // <- copying the impl
    assert_eq!(calculate_risk(score, multiplier), expected);
}

GOOD: The test uses independently-derived expected values.

#[test]
fn test_calculate_risk() {
    assert_eq!(calculate_risk(50, 1.5), 75);   // I know 50 * 1.5 = 75
    assert_eq!(calculate_risk(100, 0.5), 50);  // I know 100 * 0.5 = 50
    assert_eq!(calculate_risk(0, 999.0), 0);   // Zero stays zero
    assert_eq!(calculate_risk(1, 0.4), 0);     // Rounds down to 0
}

Rule: Never copy logic from the implementation into the test. Use hardcoded expected values you computed independently.

2. Happy Path Only

BAD: Only testing the success case.

#[test]
fn test_parse_connect_request() {
    let req = "CONNECT api.openai.com:443 HTTP/1.1\r\nHost: api.openai.com\r\n\r\n";
    let result = parse_connect(req).unwrap();
    assert_eq!(result.host, "api.openai.com");
    assert_eq!(result.port, 443);
}

GOOD: Testing the boundaries and failure modes.

#[test]
fn test_parse_connect_valid() {
    let req = "CONNECT api.openai.com:443 HTTP/1.1\r\nHost: api.openai.com\r\n\r\n";
    let result = parse_connect(req).unwrap();
    assert_eq!(result.host, "api.openai.com");
    assert_eq!(result.port, 443);
}

#[test]
fn test_parse_connect_missing_port() {
    let req = "CONNECT api.openai.com HTTP/1.1\r\n\r\n";
    assert!(parse_connect(req).is_err());
}

#[test]
fn test_parse_connect_invalid_port() {
    let req = "CONNECT api.openai.com:99999 HTTP/1.1\r\n\r\n";
    assert!(parse_connect(req).is_err());
}

#[test]
fn test_parse_connect_port_zero() {
    let req = "CONNECT api.openai.com:0 HTTP/1.1\r\n\r\n";
    assert!(parse_connect(req).is_err());
}

#[test]
fn test_parse_connect_empty_host() {
    let req = "CONNECT :443 HTTP/1.1\r\n\r\n";
    assert!(parse_connect(req).is_err());
}

#[test]
fn test_parse_connect_not_connect_method() {
    let req = "GET / HTTP/1.1\r\nHost: example.com\r\n\r\n";
    assert!(parse_connect(req).is_err());
}

#[test]
fn test_parse_connect_ipv6_host() {
    let req = "CONNECT [::1]:443 HTTP/1.1\r\n\r\n";
    let result = parse_connect(req).unwrap();
    assert_eq!(result.host, "::1");
    assert_eq!(result.port, 443);
}

3. Mock Everything

BAD: Mocking so aggressively that you're testing the mock, not the code.

// BAD — mocks the entire database, HTTP layer, and logger
// What is this even testing?
jest.mock('../db');
jest.mock('../http');
jest.mock('../logger');

test('processRequest works', async () => {
    db.query.mockResolvedValue([{id: 1}]);
    http.fetch.mockResolvedValue({status: 200});
    const result = await processRequest({id: 1});
    expect(db.query).toHaveBeenCalled();
    expect(result.status).toBe('ok');
});

GOOD: Mock only external boundaries (network, filesystem, time). Test real logic.

// GOOD — real logic, only mock the network call
test('processRequest blocks high-risk entities', async () => {
    const engine = new DetectionEngine(testRules);
    // Feed it a real request with known-bad patterns
    const request = buildRequest({
        path: '/api/v1/users',
        headers: { 'user-agent': 'sqlmap/1.0' },
        body: "' OR 1=1 --",
    });
    const decision = engine.analyze(request);
    expect(decision.blocked).toBe(true);
    expect(decision.riskScore).toBeGreaterThan(70);
    expect(decision.matchedRules).toContain('SQL_INJECTION');
});

Rule: If your test has more mock setup than assertions, something is wrong. Mock at the boundary, test real logic.

4. Trivial Assertions

BAD: Asserting things that can't possibly fail, or that test language features.

#[test]
fn test_config_creation() {
    let config = Config::default();
    assert!(config.is_some()); // Config::default() always returns Some. What are we testing?
}

#[test]
fn test_vec_push() {
    let mut v = vec![1, 2, 3];
    v.push(4);
    assert_eq!(v.len(), 4); // Testing Vec::push works? That's stdlib's job.
}

Rule: Every assertion must test a meaningful contract of YOUR code. Ask: "What bug would this catch?"

5. Testing Framework Plumbing

BAD: The test is mostly boilerplate to set up the test framework, with a trivial check at the end.

Rule: If setup is >60% of the test, extract a test helper/fixture. The test body should be readable in <10 lines.

---

Required Test Categories

Every non-trivial module must have tests in these categories:

1. Contract Tests (Required)

Test the public API contract. These answer: "Does the function do what its signature and docs promise?"

• Every public function has at least one contract test
• Test with representative inputs, not just defaults
• Assert on return values, not just "didn't panic"

2. Boundary Tests (Required)

Test edges and limits. For every input parameter, consider:

• Zero / empty: 0, "", [], None
• One: Single element, minimum valid input
• Maximum: At or near limits (u32::MAX, max string length, LRU capacity)
• Just past maximum: One over the limit — verify graceful handling
• Negative / invalid type: Wrong types, negative where unsigned expected
• Unicode / special chars: For any string inputs (paths, hostnames, headers)

3. Failure Mode Tests (Required)

Test that errors are handled correctly. For every Result<T, E> or Option<T>:

• Test the Err / None case explicitly
• Verify error types and messages are meaningful
• Test cascading failures (if A fails, does B handle it?)
• Test timeout behavior where applicable

4. State Transition Tests (When Applicable)

For stateful components (actors, sessions, campaigns, connection pools):

• Test state after each meaningful operation
• Test invalid transitions are rejected
• Test concurrent access if shared across threads
• Test cleanup / expiry behavior

5. Integration Tests (When Applicable)

For components that cross module boundaries:

• Test with real dependencies where practical (real rule engine, real state manager)
• Test the full pipeline for at least one realistic scenario
• For network code: test with a real TCP listener/connection on localhost

---

Self-Check Before Submitting

Before marking tests as complete, verify each test against this checklist:

For EACH test function, confirm:

[ ] The test name describes the BEHAVIOR being tested, not the function name
    BAD:  test_process_request
    GOOD: test_process_request_blocks_sqli_in_body

[ ] Expected values are hardcoded, not computed from the implementation

[ ] The test would FAIL if the behavior regressed, not just if the code changed

[ ] The test covers at least one edge case, not just the happy path

[ ] Assertions check MEANINGFUL outcomes (return values, state changes, side effects)
    not just "it didn't panic" or "a function was called"

[ ] If mocks are used, they mock EXTERNAL boundaries only (network, fs, time)
    not internal modules

[ ] Error messages in assertions are descriptive enough to diagnose failures
    BAD:  assert!(result.is_ok())
    GOOD: assert!(result.is_ok(), "CONNECT parse failed for valid IPv6 target: {result:?}")

[ ] The test is independent — it doesn't depend on other tests running first
    or on global mutable state

---

Language-Specific Standards

Rust

Test organization:

• Unit tests: #[cfg(test)] mod tests at the bottom of each source file
• Integration tests: tests/ directory at crate root
• Use #[test] for sync tests, #[tokio::test] for async
• Prefer assert_eq! / assert_ne! over assert! for better error messages

Error testing:

// GOOD — test specific error variants
#[test]
fn test_invalid_port_returns_parse_error() {
    let result = parse_connect("CONNECT host:abc HTTP/1.1\r\n\r\n");
    assert!(matches!(result, Err(ConnectError::InvalidPort(_))));
}

// GOOD — test error messages when they matter
#[test]
fn test_auth_failure_includes_identity() {
    let result = authenticate(&bad_key);
    let err = result.unwrap_err();
    assert!(err.to_string().contains("invalid API key"), "Error should mention invalid key: {err}");
}

Async / network testing:

// For anything involving TCP, use real localhost connections
#[tokio::test]
async fn test_forward_proxy_accepts_connect() {
    let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
    let addr = listener.local_addr().unwrap();
    
    // Start proxy in background
    let proxy = tokio::spawn(async move { run_proxy(listener).await });
    
    // Connect as client
    let mut client = TcpStream::connect(addr).await.unwrap();
    client.write_all(b"CONNECT example.com:443 HTTP/1.1\r\n\r\n").await.unwrap();
    
    let mut buf = [0u8; 256];
    let n = client.read(&mut buf).await.unwrap();
    let response = std::str::from_utf8(&buf[..n]).unwrap();
    assert!(response.starts_with("HTTP/1.1 200"), "Expected 200, got: {response}");
}

Property-based testing (use proptest crate when inputs have wide valid ranges):

proptest! {
    #[test]
    fn risk_score_never_exceeds_100(
        base in 0u32..=100,
        multiplier in 0.0f64..=10.0
    ) {
        let result = calculate_risk(base, multiplier);
        prop_assert!(result <= 100, "Risk score exceeded 100: {result}");
    }
}

What to test in Synapse specifically:

• Rule matching: known-bad inputs → expected rule IDs fire
• Risk scoring: cumulative score math with known inputs → exact expected scores
• LRU eviction: insert more than capacity → oldest evicted, newest retained
• JA4 fingerprinting: known ClientHello bytes → expected fingerprint string
• CONNECT parsing: valid/invalid request lines → correct parse/reject
• Domain matching: wildcard patterns against various hostnames → correct match/no-match
• Tunnel byte counting: send known data through relay → byte counts match

TypeScript / React

Test organization:

• Unit tests: *.test.ts / *.test.tsx co-located with source files
• Use describe blocks to group by behavior, not by function
• Use it with behavior descriptions: it('rejects expired sessions')

React component testing:

// GOOD — test user-visible behavior
test('sensor status shows "offline" when last heartbeat > 5 minutes ago', () => {
    const sensor = buildSensor({ lastHeartbeat: fiveMinutesAgo() });
    render(<SensorStatus sensor={sensor} />);
    expect(screen.getByText('Offline')).toBeInTheDocument();
    expect(screen.getByTestId('status-indicator')).toHaveClass('status-offline');
});

// BAD — testing implementation details
test('sensor status calls formatDate', () => {
    const spy = jest.spyOn(utils, 'formatDate');
    render(<SensorStatus sensor={sensor} />);
    expect(spy).toHaveBeenCalled(); // Who cares if it called formatDate?
});

API / async testing:

// GOOD — test real request/response cycle
test('tunnel events API returns events filtered by domain', async () => {
    // Seed test data
    await seedTunnelEvents([
        { domain: 'api.openai.com', bytes: 1024 },
        { domain: 'api.anthropic.com', bytes: 2048 },
        { domain: 'api.openai.com', bytes: 512 },
    ]);
    
    const response = await request(app)
        .get('/api/tunnel-events?domain=api.openai.com')
        .expect(200);
    
    expect(response.body.events).toHaveLength(2);
    expect(response.body.events.every(e => e.domain === 'api.openai.com')).toBe(true);
    expect(response.body.totalBytes).toBe(1536);
});

What to test in Signal Horizon specifically:

• WebSocket tunnel lifecycle: connect → authenticate → heartbeat → disconnect
• Tenant isolation: tenant A's request never returns tenant B's data
• Fleet aggregations: N sensors with known states → expected aggregate counts
• Dashboard data transformations: API response → chart-ready data
• Error boundaries: API failures → user-visible error states, not blank screens
• Time-based displays: "5 minutes ago", "2 hours ago" with known timestamps → expected strings

---

Coverage Expectations

Coverage is a floor, not a goal. Hitting the number with bad tests is worse than missing it with good ones.

---

Test Naming Convention

Test names must describe the scenario and expected outcome.

Pattern: test_<unit>_<scenario>_<expected_outcome>

GOOD:
  test_connect_parser_ipv6_host_parses_correctly
  test_domain_matcher_wildcard_matches_subdomain
  test_risk_score_caps_at_100_when_multiplier_overflows
  test_tunnel_auth_rejects_expired_api_key
  test_lru_cache_evicts_oldest_when_full

BAD:
  test_parser
  test_it_works
  test_domain
  test_risk_score_1
  test_happy_path

---

When You're Done

Run this mental exercise for every test file:

1. Delete the implementation. Would these tests serve as a specification someone could re-implement from? If not, the tests don't capture the behavior.

2. Introduce a subtle bug (off-by-one, wrong comparison operator, swapped arguments). Would at least one test catch it? If not, the tests are too shallow.

3. Read just the test names. Do they tell the story of what the module does? If not, rename them.