qa-self-critique · git:20260806.146d8e8 · 2026-08-06 · sha256 11dc64c080b29860

qa-self-critique git:20260806.146d8e8A

Immutable. This exact content never changes and is served at /api/v1/blob/11dc64c080b29860.

---
name: qa-self-critique
description: "Find issues in recently modified files and fix them immediately."
---

## OBJECTIVE

**PRE-COMMIT SELF-VERIFICATION & AUTO-FIX.**
You are verifying and correcting your own recent changes before commit.
**Your Goal**: Find issues in recently modified files and fix them immediately.
**The Standard**: Verify the fix is correct before applying it. A wrong fix is worse than no fix.

## COMPUTE DIRECTIVE

Before emitting any fix, reason through each finding in your thinking block. Verify the fix is correct before applying it. Do not rush to output — a wrong fix is worse than no fix.

## INPUT

```text
qa-self-critique <target> [--fix | --dry-run] [--focus <domains>]
```

## PARAMETERS

| Parameter | Default | Description |
|-----------|---------|-------------|
| `<target>` | required | File(s) or directory just modified |
| `--fix` | default | Find issues and fix them immediately |
| `--dry-run` | off | Report issues without fixing (for review before commit) |
| `--focus <domains>` | all | Comma-separated: `completeness`, `logic`, `types`, `arch`, `tests` |

## WHEN TO RUN

After implementing a feature or fix, before committing. This is a self-correction pass on YOUR OWN recent output — not a review of someone else's code.

- This is a standalone manual audit, not a required workflow handoff stage.

## PROTOCOL

### Pass 1: Tool Scan (EXECUTE, DO NOT GUESS)

**If you have bash/tool access, run these commands first.** Ingest stdout. Fix based on tool output, not visual inspection.

```bash
# 1a. Incomplete work
grep -rnE 'TODO|FIXME|HACK|XXX|STUB|NotImplementedError|pass$|\.\.\.($| #)' {{target}}

# 1b. Type laziness
grep -rnE '\bAny\b|dict\[str,\s*Any\]|#\s*type:\s*ignore' {{target}}

# 1c. Hardcoded strings (potential prompt/config leaks)
# Multi-line string detection needs Python, not single-line grep
python3 -c "
import ast, sys, os
for root, _, files in os.walk('{{target}}'):
    for f in files:
        if not f.endswith('.py'): continue
        path = os.path.join(root, f)
        try:
            tree = ast.parse(open(path).read())
            for node in ast.walk(tree):
                if isinstance(node, ast.Constant) and isinstance(node.value, str) and len(node.value) > 200:
                    print(f'{path}:{node.lineno}: long string ({len(node.value)} chars)')
        except: pass
"

# 1d. Non-real substitute sweep. Names do not change the policy.
rg -n -i 'mock|fake|fixture|stub|spy|emulat|simulat|adapter|harness|in[-_ ]?memory|monkeypatch|recorded|synthetic|sandbox|test[-_ ]?(mode|implementation)' {{target}} 2>/dev/null
rg -n 'SUBSTITUTE_JUSTIFICATION' {{target}} 2>/dev/null

# 1e. Phantom imports (Python) — top-level module only; submodule imports may not resolve
#     out-of-context, so treat results as a hint, not a verdict.
python3 - <<'PY' "{{target}}"
import ast, importlib.util, os, sys
target = sys.argv[1]
for root, _, files in os.walk(target):
    for f in files:
        if not f.endswith('.py'):
            continue
        path = os.path.join(root, f)
        try:
            tree = ast.parse(open(path, encoding='utf-8').read())
        except Exception:
            continue
        for node in ast.walk(tree):
            if isinstance(node, ast.Import):
                names = [a.name for a in node.names]
            elif isinstance(node, ast.ImportFrom):
                names = [node.module] if node.module else []
            else:
                continue
            for mod in names:
                if not mod:
                    continue
                top = mod.split('.', 1)[0]
                if importlib.util.find_spec(top) is None:
                    print(f'PHANTOM: {path}:{node.lineno}: {mod}')
PY

# 1f. Secrets scan
grep -rnEi 'password\s*=|api_key\s*=|secret\s*=|token\s*=.*['\''\"]\w{8,}' {{target}}

# 1g. Formatter/lint drift on YOUR changes (editor-vs-gate). Run the repo's own
#     formatter/linter in --check mode on the changed files — an editor's format-on-save
#     can have re-touched them with an ungated style (wrong line length, reordered
#     imports, re-indented shell). Use the repo's config, not ad-hoc flags. Examples:
#       ruff format --check <files> && ruff check <files>
#       gofumpt -l <files> ; shfmt -d <files>
#       npm run lint ; npm run format:check  # only when defined by the repo
#     If anything is flagged, that is a drift finding — re-run the formatter and fix.
```

**If you do NOT have tool access:** Skip Pass 1. Proceed to Pass 2 with visual inspection only, and note that your coverage is reduced.

### Pass 2: Semantic Checks (ONE DOMAIN AT A TIME)

Process each domain sequentially against the files in scope. Do not attempt all at once.

#### 2a. Completeness

- Any placeholder logic? Stub returns? Empty except blocks?
- Any function that was declared but never fully implemented?
- Do all imports resolve to real modules/exports?
- **Action if found:** Implement immediately. Do not report and move on.

#### 2b. Logic & Error Handling

- Trace the primary input. What happens when it's null/empty/malformed?
- External I/O (network, file, DB): wrapped in error handling?
- Off-by-one: loop bounds, array indexing, range boundaries (`..` vs `..=`)
- State transitions: are all branches covered? Any unreachable code?
- **Action if found:** Fix immediately.

#### 2c. Type Strictness

- `Any` used out of laziness (not necessity)?
  → Replace with `TypedDict`, `Protocol`, bounded generics, or concrete types
- `# type: ignore` without explanation?
  → Fix the underlying type issue, not the symptom
- Return types match declared signatures?
- **Action if found:** Fix immediately.
- **Exception:** `# type: ignore  # <reason>` with genuine justification is acceptable.

#### 2d. Architecture

- Multi-line strings containing instructions/prompts/schemas embedded in code?
  → Extract to external file, load via utility
- `__init__` params referencing concrete classes instead of protocols?
  → Replace with protocol/interface
- Duplicate type/class definitions across files?
  → Consolidate to one canonical definition
- Config values hardcoded instead of read from environment/config file?
  → Extract
- **Action if found:** Fix immediately.

#### 2e. Kernel error-path quadrant (when `lint:kernel` mode applies)

Skip this domain when the target is not C kernel code. When it is:

- For each cleanup helper called under `out_*:` / `goto err_*`, did you read it end-to-end? List every state mutation it performs. If the helper has more side effects than "undo the previous step", you are likely calling the wrong helper.
- Enumerate `(input × concurrent-op)` quadrants for every selector in the failing function (`gbl_chg`, `map_chg`, `max_hpages`, ...). A quadrant with no documented cleanup is a bug.
- Grep the same file for existing `out_*` ladders on the same struct. The pattern you need is probably already there — mirror it instead of inventing.
- For paired charge / uncharge of two ownership domains (e.g., `h_cg` vs `h_cg_rsvd`), are they stored in **separate** locals? If a single local is reused, task migration / preemption can break ownership.
- Reservation / accounting deltas: does any cleanup unconditionally `++` a counter that has a global invariant (`resv_huge_pages <= free_huge_pages`)? A concurrent free can drop `free_huge_pages` between the failed step and the cleanup. Decrement under the owning lock; do not raw-bump shared counters.
- **Action if found:** Fix immediately by mirroring an existing `out_*` ladder in the same file. Do not invent new cleanup logic when precedent exists.

#### 2f. Test Quality

- Any mock, fake, fixture, stub, spy, emulator, in-memory implementation, monkeypatch, recorded/synthetic response, sandbox, test mode, or renamed non-real replacement?
  → Require a complete `SUBSTITUTE_JUSTIFICATION` for each use. If the real component or a disposable real instance can safely and deterministically serve the assertion, remove the substitute.
- Does the necessity say only "faster", "easier", "hermetic", "offline", "CI", "flaky service", or "missing credentials"?
  → Reject it. Missing real prerequisites block that test layer; they do not authorize fake evidence.
- Is a substitute-backed run counted as integration, E2E, acceptance, security, readiness, or completion evidence?
  → Remove it from the evidence set and mark the required real layer `BLOCKED` until it runs.
- `# type: ignore` in tests hiding production type bugs?
  → Fix the production types, not the test
- Tests asserting implementation details (mock call counts) instead of behavior?
  → Rewrite to assert outputs/state
- **Action if found:** Fix immediately.

### Pass 3: Apply Fixes

**`--fix` mode (default):**

Output ONLY the file modifications. For each fix, emit:

```text
# [one-line description of what was fixed]
[file edit / diff / tool call to apply the change]
```

No apologies. No "I noticed that...". No Markdown report sections. Just the fix with a one-line label.

After all fixes are applied:

```text
[AUDIT_COMPLETE]: {N} issues fixed across {M} files.
```

**`--dry-run` mode:**

Output a compact finding list (no fixes applied):

```text
[AUDIT_DRY_RUN]:
- {file}:{line}: {one-line description} [{domain}]
- {file}:{line}: {one-line description} [{domain}]
...
{N} issues found. Run /audit --fix to apply.
```

**If clean:**

```text
[AUDIT_PASS]: Clean. Ready to commit.
```

## CAPABILITY BOUNDARIES

**Can do:**

- Pattern-match incomplete code, bad types, structural smells
- Trace logic paths for null/error/edge cases
- Identify architecture violations by inspection
- Fix all of the above in-place

**Cannot do without tools:**

- Verify imports actually resolve (needs Python interpreter)
- Run tests (needs execution environment)
- Check dependency versions for CVEs (needs package audit tools)
- Measure actual complexity metrics (needs AST tooling)

If a check requires tools you don't have, skip it. Do not pretend.

## EXECUTION RULES

1. **FIX, don't report.** This is not `qa-review`. You are correcting your own work.
2. **Tool output overrides visual inspection.** If grep found it, it's real. Don't rationalize it away.
3. **One domain at a time.** Don't scan for type issues while thinking about error handling.
4. **No apologies, no narrative.** "Fixed X" not "I noticed I forgot X, sorry about that."
5. **Wrong fix > no fix? NO.** If you're unsure about a fix, flag it for human review instead of applying a bad patch.

## DISTRIBUTION

This canonical skill is emitted to each runtime's native Agent Skills root. It is not duplicated into command surfaces on hosts that already support skills.

## EXAMPLES

```text
qa-self-critique src/auth/
qa-self-critique . --dry-run
qa-self-critique src/ tests/ --focus types,arch
qa-self-critique src/api/router.py --fix
```