From f6b72b617ea6f8c0e5b820bf1ece6c7e651aaa97 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sun, 3 May 2026 17:38:55 +0200 Subject: [PATCH 1/5] chore: improve codebase-audit skill from 2026-05-03 lessons - Fix agent 09 (unwired-settings) FP rate (was 38%, drove 39 of 39 FPs). Recognise direct ConfigResolver.get_int/float/str calls, composed-config methods (get_coordination_config()), Pydantic config-model embedding, subscriber pattern, and bootstrap-only flags as legitimate consumption. - Hard-prohibit agent scratch scripts: ban writing helper *.py to project root, scripts/, c:/tmp/, /tmp/. Past run leaked 14+ scratch files that triggered Pyright diagnostic floods. - Add Phase 7: Cleanup (mandatory) to sweep agent-leaked scripts. - Phase 0: handle existing _audit/latest dir (Junction collision); ban Bash redirects (project hook blocks them); explicit no-cd reminder. - Phase 1: filter Renovate Dependency Dashboard from injected issue list. - Phase 4: MANDATORY enumerate actual finding files via Glob; previous run hallucinated agent filenames in INDEX. - Phase 5 DEFAULT: produce deduped ISSUES.md (one row per issue class) before triage prompt. User shouldn't have to wade through 300+ raw findings to make a decision. - Validation batching strategy made concrete (heavy/mid/small files). - Lessons-from-prior-runs section so each rule's rationale is traceable. - Rules expanded: no scratch scripts (#10), no Bash redirects (#11), surface FP-rate signals end-of-run (#12). --- .claude/skills/codebase-audit/SKILL.md | 193 ++++++++++++++++++++++--- 1 file changed, 174 insertions(+), 19 deletions(-) diff --git a/.claude/skills/codebase-audit/SKILL.md b/.claude/skills/codebase-audit/SKILL.md index 7c3866f487..8454da10f0 100644 --- a/.claude/skills/codebase-audit/SKILL.md +++ b/.claude/skills/codebase-audit/SKILL.md @@ -39,14 +39,17 @@ Flags: Use the run-history layout (see "Phase 0 setup: run-history layout" below for the full description): ```bash -RUN_DIR="_audit/runs/$(date +%Y-%m-%d-%H%M%S)" -mkdir -p "$RUN_DIR/findings" -ln -sfn "runs/$(basename "$RUN_DIR")" _audit/latest +RUN_DIR="_audit/runs/$(date +%Y-%m-%d-%H%M%S)" && mkdir -p "$RUN_DIR/findings" && rm -f _audit/latest && ln -sfn "runs/$(basename $RUN_DIR)" _audit/latest && echo "RUN_DIR=$RUN_DIR" ``` +Notes for the setup command: +- `rm -f _audit/latest` clears any prior symlink before re-linking. If `_audit/latest` exists as a real directory (Junction on Windows from a prior run), `rm -f` will refuse and you must run `rmdir _audit/latest` (Windows) or `rm -rf _audit/latest` (POSIX) instead. Detect by `test -d _audit/latest && ! test -L _audit/latest` before deciding. +- **DO NOT use `2>/dev/null`, `&>/dev/null`, or any `>` / `>>` redirect in this Bash call**. The project's PreToolUse Bash hook (`scripts/check_bash_no_write.sh`) blocks all redirects unconditionally. Chain with `&&` instead and let stderr surface. +- **DO NOT use `cd`** -- the working directory is already the project root. Use absolute or workspace-relative paths. + Never delete `_audit/runs/*`. The `_audit/latest` symlink always points at the most recent run; older runs accumulate. On Windows, the OpenCode adapter first attempts `New-Item -ItemType SymbolicLink` (requires Developer Mode or admin); on failure it falls back to `New-Item -ItemType Junction`, which needs no special privileges and still resolves as a directory so downstream writes to `_audit/latest/findings/` succeed. -Verify `_audit/` is in `.gitignore`. If not, add it. +Verify `_audit/` is in `.gitignore` via `grep -E "^_audit" .gitignore`. If not, add it. --- @@ -61,7 +64,7 @@ Read these files to build context injected into EVERY agent prompt: 5. `web/src/router/routes.ts`: frontend routing 6. `web/src/stores/`: list all stores 7. `docs/DESIGN_SPEC.md`: spec index -8. Existing open issues: `gh issue list --state open --limit 200 --json number,title,labels` +8. Existing open issues: `gh issue list --state open --limit 200 --json number,title,labels`. **Filter the result before injecting into agent prompts**: drop any issue authored by `app/renovate` or with title containing "Dependency Dashboard", "Renovate", "renovate-bot". Per memory rule `feedback_open_issues_exclude_renovate.md`, those are bot-managed dependency churn, not framework work, and they pollute every agent's "do not duplicate" guard with noise. Produce an **Architecture Brief** (~400 words) covering: - Logging: `get_logger(__name__)`, structlog, event constants in `observability/events/`, structured kwargs @@ -164,6 +167,16 @@ Rules: - If zero issues, still create the file and note what you checked - Do NOT fix anything -- audit only - Do NOT use Bash to write files -- use the Write tool +- **DO NOT write helper / analysis Python scripts to disk anywhere** (no `*.py` in + the project root, in `scripts/`, in `c:\tmp\`, in `/tmp`, anywhere on the + filesystem outside `_audit/latest/findings/`). Past runs leaked 14+ scratch + scripts (`find_missing_logging.py`, `parse_audit.py`, `audit_pydantic_models*.py`, + `validate_config_examples.py`, `audit_diff.py`, etc.) that triggered Pyright + diagnostics in the main thread and required user cleanup. Use Grep/Glob/Read + inline -- if you can't accomplish the audit with those, narrow your scope. + The Write tool exists ONLY to write your finding file. The audit Bash tool is + for read-only inspection (`git`, `gh`, `find`, `wc`); never for `python -c`, + `cat >`, `tee`, redirects, or heredocs. ``` ### Streaming Pool Execution @@ -384,11 +397,47 @@ route AND no parent import are unreachable. Severity: medium. File: `_audit/latest/findings/09-unwired-settings.md` ```text -Check settings/definitions/ for setting definitions. For each, check if it is: -(a) subscribed to via settings/subscribers/, AND -(b) exposed via an API endpoint in api/controllers/settings.py. -Settings defined but never consumed are dead config. Severity: medium. +Check settings/definitions/ for setting definitions. For each setting key, verify +it is consumed somewhere. Settings defined but never read are dead config. + +CRITICAL FALSE-POSITIVE GUARD: prior runs of this agent had ~38% FP rate (39 of +107 findings overturned in validation, 2026-05-03 run) because the prompt only +checked bridge config methods. Settings are LEGITIMATELY consumed via several +patterns -- DO NOT flag a setting as unwired unless you have ruled out ALL of: + +1. **Bridge config methods**: ConfigResolver methods like get_api_bridge_config(), + get_communication_bridge_config(), get_tools_bridge_config(), etc. that + compose multiple settings into one config payload. +2. **Composed-config methods**: ConfigResolver.get__config() methods that + return a Pydantic config model assembled from individual settings (e.g. + get_coordination_config(), get_budget_config(), get_api_config()). All + settings inside such a composed config ARE wired. +3. **Direct scalar accessors**: ConfigResolver.get_int(...), get_float(...), + get_str(...), get_bool(...), get_secret(...) called with the namespace.key + string. Grep for the literal "." string across src/synthorg/. +4. **Pydantic config-model embedding**: when a setting is a field on a Pydantic + config dataclass (e.g. ApiConfig.api_prefix, AuthConfig.jwt_expiry_minutes, + RateLimitConfig.*, BudgetConfig.*), it IS consumed via the normal config + chain -- not unwired. +5. **Subscriber pattern**: settings/subscribers/ registrations that listen for + changes to a key. +6. **Bootstrap-only / read-only-post-init**: settings registered with + read_only_post_init=True or marked bootstrap-only are intentionally not in + bridge configs (they're registered for /settings discoverability only). DO + NOT flag these. + +VERIFICATION REQUIREMENT: before flagging a setting as unwired, you MUST run at +least three searches to rule out all six patterns above: + - Grep for the literal "." string across src/synthorg/ + - Grep for the field name in src/synthorg/config/ (Pydantic config models) + - Grep for the namespace in ConfigResolver methods (any get__config / + get__bridge_config method) + +If any of those searches find a consumer, the setting is NOT unwired -- skip it. + +A setting is "dead" only if NONE of the six consumption patterns apply. +Severity: medium. ``` **Agent 10: unwired-tools** (sonnet) @@ -3072,6 +3121,19 @@ After all launched audit agents complete, launch validation agents to verify fin 4. Cluster findings by audit-agent file when possible (a single validator can process all findings from one finding file in one batch, sharing context). If a single file has more than ~25 findings, split into multiple batches of ~12-25 findings each. Otherwise one validator per finding file is the default 5. Launch **sonnet** validation agents in parallel (`run_in_background: true`). Maintain the same rolling-pool-of-10 cadence used in Phase 2 so no slot sits idle +### Batching strategy (concrete) + +Run `Bash for f in _audit/latest/findings/*.md; do count=$(grep -cE "^### (critical|high|medium|low|info)" "$f"); echo "$count $(basename $f)"; done | sort -rn` to produce a sorted list of (count, filename). Then build batches: + +- **Heavy files (>25 findings)**: dedicate 1 batch per ~25-finding chunk. Example: a 107-finding file splits into batches of 35 + 36 + 36. +- **Mid files (10-25 findings)**: 1-2 files per batch. +- **Small files (1-9 findings)**: pack 5-8 files per batch, total findings per batch ~20-30. +- **Zero-finding files**: skip entirely. + +Target: each validator processes ~20-30 findings. Going much higher risks the validator running out of focus; going much lower wastes overhead. + +For a typical full run (~400 findings across ~100 non-empty files), expect ~17-25 validation batches. Launch them with the same rolling-pool-of-10 cadence as Phase 2. + ### Validation Agent Prompt ```text @@ -3108,7 +3170,11 @@ Findings to validate: ## Phase 4: Build INDEX.md -After validation, read all finding files and build `_audit/latest/INDEX.md`: +After validation, read all finding files and build `_audit/latest/INDEX.md`. + +**MANDATORY for the agent that builds INDEX.md**: enumerate the actual files in `_audit/latest/findings/` via `Glob` or `Bash ls _audit/latest/findings/`. Use the REAL filenames. The 2026-05-03 run produced an INDEX with hallucinated filenames (`14-web-store-architecture-drift.md`, `19-benchmark-regression.md`, etc.) that did not match the agent roster. Before writing INDEX, list `_audit/latest/findings/*.md` and copy the names verbatim. Each entry's finding count must come from actually grepping the file (`grep -cE "^### (critical|high|medium|low|info)" `), not from an assumed wave grouping. + +Use this template: ```markdown # Codebase Audit Index @@ -3162,18 +3228,62 @@ may not match the codebase's conventions: ## Phase 5: Triage with User -Present INDEX.md to the user. Walk through: +Present INDEX.md to the user with a one-paragraph summary, then **always produce the deduped issue list (Phase 5 DEFAULT OUTPUT) before asking what to do next**. Without that list, the user is staring at 300+ raw findings and cannot make a decision. + +### Phase 5 DEFAULT: Deduped Issue List (MANDATORY before triage prompt) + +Group every confirmed finding by its **issue class** (= the agent-finding-file source -- e.g. all 107 raw findings from `09-unwired-settings.md` collapse into ONE row "Unwired settings"). Write `_audit/latest/ISSUES.md`: + +```markdown +# Deduped Issue List + +One row per issue class. Multi-file patterns collapse into a single planning unit. + +## Critical + +| # | Issue class | Confirmed | FP removed | Top affected paths | Recommended action | +|---|-------------|-----------|------------|--------------------|---------------------| +| 1 | | | | <2-3 representative paths> | | + +## High +... + +## Medium +... + +## Low +... + +## Architectural Reworks (from REWORK.md) + +| # | Recommendation | Effort | Related agents | Recommended action | +``` + +Build the list by: + +1. For each `_audit/latest/findings/-.md`, count CONFIRMED findings (subtract FALSE_POSITIVE / INTENTIONAL from validate-batch-*.md verdicts). +2. Skip files where `confirmed = 0`. +3. Sort within each severity bucket by confirmed-count descending. +4. For "Top affected paths" pick the 2-3 paths whose finding cluster appears most often. +5. For "Recommended action" pick: + - `single-PR` if confirmed <= 5 and one file pattern + - `batch-PR` if confirmed > 5 with shared root cause + - `RFC` if listed in REWORK.md + - `track-only` for low / info / large-and-unclear + - `dismiss` if the cluster is dominated by INTENTIONAL verdicts and the few CONFIRMED entries are noise. + +Print the table inline to the user (above the AskUserQuestion call), AND save `_audit/latest/ISSUES.md` to disk. The inline render is what the user actually triages from. + +### Triage prompt -1. Critical + high findings first -2. Zero-finding categories (suspicious?) -3. Group related findings into potential GitHub issues by code proximity +Use AskUserQuestion with these options (in order): -Use AskUserQuestion to confirm: -- Which findings to create issues for -- How to group them (by module? by wave? by severity?) -- Whether to create issues now or export report only +1. **Report-only** (default if user already saw INDEX once): all artifacts written to disk; user takes it from there. +2. **Walk top-20 critical with me**: per-item triage, AskUserQuestion per row. +3. **Create GitHub issues for top N issue classes**: one issue per row in the deduped list above a chosen severity floor. +4. **Open RFC issues for REWORK.md only**: skip per-finding work, file architectural recommendations only. -If `--report-only`, skip this phase entirely. +If `--report-only` flag was passed at command time, skip the AskUserQuestion entirely and finalise as report-only. --- @@ -3381,6 +3491,30 @@ Bootstrap: not all 155 agents need golden tests upfront. Start with the 25 agent If an agent fails its self-test, INDEX.md "Self-Test Status" section flags it. +### Phase 7: Cleanup (NEW, MANDATORY) + +Runs after triage / report-only output is finalized. Sweep agent-leaked scratch files from the working tree. + +The 2026-05-03 run leaked at least 14 helper scripts to disk despite the agent-prompt rule against it (e.g. `find_missing_logging.py`, `find_missing_logging_filtered.py`, `parse_audit.py`, `validate_config_examples.py`, `scripts/audit_pydantic_models.py`, `scripts/audit_pydantic_models_v2.py`, `scripts/audit_pydantic_models_v3.py`, `scripts/audit_phase35_synthesis.py`, plus `c:\tmp\*.py` files). These triggered Pyright diagnostics in the main thread on every file write, polluted git status, and required user cleanup. The skill is responsible for cleaning up after its own agents. + +Run this Bash sweep: + +```bash +rm -f find_missing_logging.py find_missing_logging_filtered.py parse_audit.py validate_config_examples.py audit_diff.py audit_parity.py check_docs.py check_rate_limits.py circular_dep_analyzer.py check_protocols.py debug_scanner.py detailed_check.py final_audit.py find_unwired.py test_regex.py validate_configs.py verify_final.py verify_protocols.py 2>/dev/null || true +``` + +Then list and remove anything else suspicious: + +```bash +find . -maxdepth 2 -name "*.py" -newer _audit/runs/$(ls -1 _audit/runs/ | tail -2 | head -1)/findings -not -path "./src/*" -not -path "./tests/*" -not -path "./web/*" -not -path "./cli/*" -not -path "./scripts/*" +``` + +Show the user the list before deleting anything that's not on the known leak list. Files in `c:\tmp\`, `/tmp\`, or any path outside the project root: leave them; the OS will reap them. Files inside `scripts/`: prompt the user before removal -- those may be intentional helpers, not leakage. + +If `git status` shows any new untracked `.py` file at project root or in `scripts/` that didn't exist before the audit run started, flag it as a candidate for removal. + +The cleanup phase is REQUIRED on every run. Skipping it means the next run's diagnostic stream is contaminated by the previous run's leakage. + --- ## Rules @@ -3398,3 +3532,24 @@ If an agent fails its self-test, INDEX.md "Self-Test Status" section flags it. 7. **Rerunnable**: never delete `_audit/runs/*`; always create a fresh `_audit/runs//` and repoint `_audit/latest` at it 8. **Never use em-dashes** in any output files (project convention) 9. **Report progress** after each batch completes +10. **No scratch scripts**: agents may NOT write helper Python / shell scripts to disk anywhere outside `_audit/latest/findings/.md`. Use Grep / Glob / Read inline. The Phase 7 cleanup will sweep any leakage but the prevention rule lives at the agent level. +11. **No Bash redirects**: this project's PreToolUse hook blocks `>`, `>>`, `2>`, `&>` in any Bash call. Use `||` chains and let stderr surface. Use the Write/Edit tools for file creation. +12. **Verify FP signals end-of-run**: after Phase 6, if `_audit/metrics/agent-quality.json` shows any agent at >30% rolling FP rate, list the agent and its FP rate in the final summary so the user can prioritize prompt revision before the next run. + +--- + +## Lessons from prior runs + +Document specific issues observed in named runs so future runs avoid repeating them. + +### 2026-05-03 run + +- **Agent 09 (`unwired-settings`) had 38% FP rate** (39/107 overturned). Root cause: prompt only checked bridge config methods; missed `ConfigResolver.get_int/float/str` direct calls, composed-config methods (`get_coordination_config()`), and Pydantic config-model embedding. Fixed in this skill version. +- **14+ scratch Python scripts leaked to project root and `c:\tmp\`** -- `find_missing_logging.py`, `find_missing_logging_filtered.py`, `parse_audit.py`, `validate_config_examples.py`, `scripts/audit_pydantic_models{,_v2,_v3}.py`, `scripts/audit_phase35_synthesis.py`, `c:\tmp\check_rate_limits.py`, `c:\tmp\circular_dep_analyzer.py`, `c:\tmp\audit_parity.py`, `c:\tmp\audit_diff.py`, `c:\tmp\check_docs.py`. These triggered Pyright diagnostic floods in the main thread on every write. Fixed via Rule #10 + Phase 7 cleanup. +- **Bash redirects (`2>/dev/null`) blocked by PreToolUse hook** during Phase 0 setup. Fixed via Rule #11 + updated Phase 0 example. +- **INDEX.md hallucinated agent filenames** -- the Phase 4 agent invented entries like `14-web-store-architecture-drift.md`, `19-benchmark-regression.md` that never existed. Fixed via "MANDATORY: enumerate actual files" instruction in Phase 4. +- **Renovate Dependency Dashboard #1730** appeared in `gh issue list` injected into agent prompts. Per memory rule `feedback_open_issues_exclude_renovate.md`, exclude any Renovate-managed issue from the issue list. +- **Phase 6 DIFF agent only validated 31% of agents** because it ran in parallel with synthesis instead of after, and assumed batches were complete. Sequence Phase 6 strictly after Phase 3 validation finishes. +- **`_audit/latest` symlink collision**: when re-running, `ln -sfn` requires `rm -f _audit/latest` first (Junction directories block re-link). Fixed in Phase 0 setup command. + +When updating the skill in response to a new run's lessons, add a new dated subsection here. Older subsections stay so the rationale for each rule is traceable. From 8585a58ed9888896e96b4f3f0ddef9e3aa6e05a5 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sun, 3 May 2026 18:00:44 +0200 Subject: [PATCH 2/5] fix: address PR #1732 review feedback - Phase 0 setup: handle Junction/real-directory case for _audit/latest via inline conditional (rm -rf if directory but not symlink, else rm -f). CodeRabbit flagged plain rm -f would abort the chain on directory targets. - Phase 0 setup: quote $RUN_DIR in basename for defensive habit. Gemini. - Phase 7 cleanup: drop the stderr-discard redirect (violates new Rule #11 banning redirects). rm -f is silent on missing paths, so the redirect was redundant. CodeRabbit + Gemini. - Phase 7 cleanup: drop -not -path './scripts/*' from find sweep so the scripts/audit_pydantic_models leakage detected last run actually surfaces. Gemini. - Phase 7 batching strategy: quote $f in basename for filename safety. Gemini. Pre-reviewed by 2 external agents (CodeRabbit, Gemini), all 4 findings addressed. --- .claude/skills/codebase-audit/SKILL.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/.claude/skills/codebase-audit/SKILL.md b/.claude/skills/codebase-audit/SKILL.md index 8454da10f0..b348f67a99 100644 --- a/.claude/skills/codebase-audit/SKILL.md +++ b/.claude/skills/codebase-audit/SKILL.md @@ -39,11 +39,12 @@ Flags: Use the run-history layout (see "Phase 0 setup: run-history layout" below for the full description): ```bash -RUN_DIR="_audit/runs/$(date +%Y-%m-%d-%H%M%S)" && mkdir -p "$RUN_DIR/findings" && rm -f _audit/latest && ln -sfn "runs/$(basename $RUN_DIR)" _audit/latest && echo "RUN_DIR=$RUN_DIR" +RUN_DIR="_audit/runs/$(date +%Y-%m-%d-%H%M%S)" && mkdir -p "$RUN_DIR/findings" && if test -d _audit/latest && ! test -L _audit/latest; then rm -rf _audit/latest; else rm -f _audit/latest; fi && ln -sfn "runs/$(basename "$RUN_DIR")" _audit/latest && echo "RUN_DIR=$RUN_DIR" ``` Notes for the setup command: -- `rm -f _audit/latest` clears any prior symlink before re-linking. If `_audit/latest` exists as a real directory (Junction on Windows from a prior run), `rm -f` will refuse and you must run `rmdir _audit/latest` (Windows) or `rm -rf _audit/latest` (POSIX) instead. Detect by `test -d _audit/latest && ! test -L _audit/latest` before deciding. +- The `if test -d _audit/latest && ! test -L _audit/latest` branch detects the case where `_audit/latest` is a real directory (Junction on Windows from a prior run, or an accidental `mkdir _audit/latest`); plain `rm -f` would refuse and break the relink chain. The conditional handles both cases inline. +- `$RUN_DIR` is quoted inside `basename "$RUN_DIR"` even though the timestamp format never contains spaces -- defensive habit, costs nothing. - **DO NOT use `2>/dev/null`, `&>/dev/null`, or any `>` / `>>` redirect in this Bash call**. The project's PreToolUse Bash hook (`scripts/check_bash_no_write.sh`) blocks all redirects unconditionally. Chain with `&&` instead and let stderr surface. - **DO NOT use `cd`** -- the working directory is already the project root. Use absolute or workspace-relative paths. @@ -3123,7 +3124,7 @@ After all launched audit agents complete, launch validation agents to verify fin ### Batching strategy (concrete) -Run `Bash for f in _audit/latest/findings/*.md; do count=$(grep -cE "^### (critical|high|medium|low|info)" "$f"); echo "$count $(basename $f)"; done | sort -rn` to produce a sorted list of (count, filename). Then build batches: +Run `Bash for f in _audit/latest/findings/*.md; do count=$(grep -cE "^### (critical|high|medium|low|info)" "$f"); echo "$count $(basename "$f")"; done | sort -rn` to produce a sorted list of (count, filename). Then build batches: - **Heavy files (>25 findings)**: dedicate 1 batch per ~25-finding chunk. Example: a 107-finding file splits into batches of 35 + 36 + 36. - **Mid files (10-25 findings)**: 1-2 files per batch. @@ -3500,13 +3501,15 @@ The 2026-05-03 run leaked at least 14 helper scripts to disk despite the agent-p Run this Bash sweep: ```bash -rm -f find_missing_logging.py find_missing_logging_filtered.py parse_audit.py validate_config_examples.py audit_diff.py audit_parity.py check_docs.py check_rate_limits.py circular_dep_analyzer.py check_protocols.py debug_scanner.py detailed_check.py final_audit.py find_unwired.py test_regex.py validate_configs.py verify_final.py verify_protocols.py 2>/dev/null || true +rm -f find_missing_logging.py find_missing_logging_filtered.py parse_audit.py validate_config_examples.py audit_diff.py audit_parity.py check_docs.py check_rate_limits.py circular_dep_analyzer.py check_protocols.py debug_scanner.py detailed_check.py final_audit.py find_unwired.py test_regex.py validate_configs.py verify_final.py verify_protocols.py || true ``` -Then list and remove anything else suspicious: +(`rm -f` is silent on missing paths, so no stderr redirect is needed; `|| true` keeps the chain from aborting on edge-case errors. Per Rule #11, the project's PreToolUse hook blocks `2>/dev/null` and other redirects unconditionally.) + +Then list and remove anything else suspicious. The find DOES include `scripts/` because the 2026-05-03 run leaked `scripts/audit_pydantic_models{,_v2,_v3}.py` and `scripts/audit_phase35_synthesis.py`; those need to surface for user prompt: ```bash -find . -maxdepth 2 -name "*.py" -newer _audit/runs/$(ls -1 _audit/runs/ | tail -2 | head -1)/findings -not -path "./src/*" -not -path "./tests/*" -not -path "./web/*" -not -path "./cli/*" -not -path "./scripts/*" +find . -maxdepth 2 -name "*.py" -newer _audit/runs/$(ls -1 _audit/runs/ | tail -2 | head -1)/findings -not -path "./src/*" -not -path "./tests/*" -not -path "./web/*" -not -path "./cli/*" ``` Show the user the list before deleting anything that's not on the known leak list. Files in `c:\tmp\`, `/tmp\`, or any path outside the project root: leave them; the OS will reap them. Files inside `scripts/`: prompt the user before removal -- those may be intentional helpers, not leakage. From dd746be5ac5a9c1c39d57a1672f7b0e68cdb7cee Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sun, 3 May 2026 18:07:44 +0200 Subject: [PATCH 3/5] fix: address PR #1732 round-2 review feedback - Phase 1 issue list: include author in --json fields so the Renovate filter (drops author.login == 'app/renovate') has metadata to match on. Without --json author the filter is a no-op. CodeRabbit. - Phase 7 batching strategy: drop the literal leading 'Bash' token from the example command (was 'Run `Bash for f in ...`'); shell-invalid as written. Reformatted as a proper fenced bash block. CodeRabbit. - Phase 7 batching strategy: align heavy-files example with stated ~25-finding chunk size. Was '107 splits into 35+36+36'; now '107 splits into 25+25+25+32' (4 batches). CodeRabbit. - Lessons section: rewrite the symlink-collision note to describe the actual conditional fix landed in round-1 (handles all 3 states: symlink, real-dir/Junction, missing) instead of the obsolete 'rm -f alone' claim. CodeRabbit. - .opencode mirror: bump '152' to '155' in description and adapter prose to match SKILL.md. CodeRabbit. --- .claude/skills/codebase-audit/SKILL.md | 16 +++++++++++----- .opencode/commands/codebase-audit.md | 4 ++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/.claude/skills/codebase-audit/SKILL.md b/.claude/skills/codebase-audit/SKILL.md index b348f67a99..9f25d64c6d 100644 --- a/.claude/skills/codebase-audit/SKILL.md +++ b/.claude/skills/codebase-audit/SKILL.md @@ -65,7 +65,7 @@ Read these files to build context injected into EVERY agent prompt: 5. `web/src/router/routes.ts`: frontend routing 6. `web/src/stores/`: list all stores 7. `docs/DESIGN_SPEC.md`: spec index -8. Existing open issues: `gh issue list --state open --limit 200 --json number,title,labels`. **Filter the result before injecting into agent prompts**: drop any issue authored by `app/renovate` or with title containing "Dependency Dashboard", "Renovate", "renovate-bot". Per memory rule `feedback_open_issues_exclude_renovate.md`, those are bot-managed dependency churn, not framework work, and they pollute every agent's "do not duplicate" guard with noise. +8. Existing open issues: `gh issue list --state open --limit 200 --json number,title,labels,author`. **Filter the result before injecting into agent prompts**: drop any issue where `author.login == "app/renovate"` (the bot's `app/` form), or with title containing "Dependency Dashboard", "Renovate", or "renovate-bot". The `--json` field list MUST include `author` -- without it the filter has nothing to match on and Renovate noise leaks into every agent prompt. Per memory rule `feedback_open_issues_exclude_renovate.md`, those are bot-managed dependency churn, not framework work, and they pollute every agent's "do not duplicate" guard. Produce an **Architecture Brief** (~400 words) covering: - Logging: `get_logger(__name__)`, structlog, event constants in `observability/events/`, structured kwargs @@ -3124,10 +3124,16 @@ After all launched audit agents complete, launch validation agents to verify fin ### Batching strategy (concrete) -Run `Bash for f in _audit/latest/findings/*.md; do count=$(grep -cE "^### (critical|high|medium|low|info)" "$f"); echo "$count $(basename "$f")"; done | sort -rn` to produce a sorted list of (count, filename). Then build batches: +Run this Bash to produce a sorted list of (count, filename): -- **Heavy files (>25 findings)**: dedicate 1 batch per ~25-finding chunk. Example: a 107-finding file splits into batches of 35 + 36 + 36. -- **Mid files (10-25 findings)**: 1-2 files per batch. +```bash +for f in _audit/latest/findings/*.md; do count=$(grep -cE "^### (critical|high|medium|low|info)" "$f"); echo "$count $(basename "$f")"; done | sort -rn +``` + +Then build batches: + +- **Heavy files (>25 findings)**: dedicate 1 batch per ~25-finding chunk. Example: a 107-finding file splits into 4 batches of 25 + 25 + 25 + 32. +- **Mid files (10-25 findings)**: 1-2 files per batch (target ~20-30 findings). - **Small files (1-9 findings)**: pack 5-8 files per batch, total findings per batch ~20-30. - **Zero-finding files**: skip entirely. @@ -3553,6 +3559,6 @@ Document specific issues observed in named runs so future runs avoid repeating t - **INDEX.md hallucinated agent filenames** -- the Phase 4 agent invented entries like `14-web-store-architecture-drift.md`, `19-benchmark-regression.md` that never existed. Fixed via "MANDATORY: enumerate actual files" instruction in Phase 4. - **Renovate Dependency Dashboard #1730** appeared in `gh issue list` injected into agent prompts. Per memory rule `feedback_open_issues_exclude_renovate.md`, exclude any Renovate-managed issue from the issue list. - **Phase 6 DIFF agent only validated 31% of agents** because it ran in parallel with synthesis instead of after, and assumed batches were complete. Sequence Phase 6 strictly after Phase 3 validation finishes. -- **`_audit/latest` symlink collision**: when re-running, `ln -sfn` requires `rm -f _audit/latest` first (Junction directories block re-link). Fixed in Phase 0 setup command. +- **`_audit/latest` re-link collision** when re-running: `ln -sfn` alone fails on a pre-existing real directory or Windows Junction; `rm -f` alone refuses on a directory. The Phase 0 setup command now handles all three states (symlink, real directory / Junction, missing) via `if test -d _audit/latest && ! test -L _audit/latest; then rm -rf _audit/latest; else rm -f _audit/latest; fi` before the relink. When updating the skill in response to a new run's lessons, add a new dated subsection here. Older subsections stay so the rationale for each rule is traceable. diff --git a/.opencode/commands/codebase-audit.md b/.opencode/commands/codebase-audit.md index 844b961c58..60d0fdf4da 100644 --- a/.opencode/commands/codebase-audit.md +++ b/.opencode/commands/codebase-audit.md @@ -1,5 +1,5 @@ --- -description: "Full codebase audit: launches 152 specialized agents to find issues across Python/React/Go/docs/website, writes findings to _audit/latest/findings/, then triages with user" +description: "Full codebase audit: launches 155 specialized agents to find issues across Python/React/Go/docs/website, writes findings to _audit/latest/findings/, then triages with user" --- # OpenCode Adapter (read this FIRST, before the skill below) @@ -8,7 +8,7 @@ You are running in **OpenCode**, not Claude Code. Apply these overrides: ### Subagent spawning -The rewritten skill launches 152 audit agents with custom embedded prompts via the `Agent` tool. These are NOT mapped to `.opencode/agents/`; they use inline prompts defined in the skill itself. Spawn each agent with its prompt from the skill's Agent Roster section. +The rewritten skill launches 155 audit agents with custom embedded prompts via the `Agent` tool. These are NOT mapped to `.opencode/agents/`; they use inline prompts defined in the skill itself. Spawn each agent with its prompt from the skill's Agent Roster section. ### Scope changes From 4a1d5e13a5a56df244622092819bb0b16781aeb2 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sun, 3 May 2026 18:15:18 +0200 Subject: [PATCH 4/5] fix: address PR #1732 round-3 review feedback - Phase 7: 'finalized' (US spelling) becomes 'finalised' (UK). Project ships in British English per docs/design/internationalization.md and the internal Agent 37 directive. CodeRabbit + LanguageTool flagged. - Phase 7: harden the find -newer command for first/second-run cases. Was 'tail -2 | head -1' which compares against itself on first run (no findings) and is fragile in general. Now 'ls -1t | sed -n 2p' picks the second-newest run by mtime, the '[ -n $prev_run ] &&' guard short-circuits the first-run case where no previous run exists, and the path is now quoted. CodeRabbit. --- .claude/skills/codebase-audit/SKILL.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.claude/skills/codebase-audit/SKILL.md b/.claude/skills/codebase-audit/SKILL.md index 9f25d64c6d..998953e409 100644 --- a/.claude/skills/codebase-audit/SKILL.md +++ b/.claude/skills/codebase-audit/SKILL.md @@ -3500,7 +3500,7 @@ If an agent fails its self-test, INDEX.md "Self-Test Status" section flags it. ### Phase 7: Cleanup (NEW, MANDATORY) -Runs after triage / report-only output is finalized. Sweep agent-leaked scratch files from the working tree. +Runs after triage / report-only output is finalised. Sweep agent-leaked scratch files from the working tree. The 2026-05-03 run leaked at least 14 helper scripts to disk despite the agent-prompt rule against it (e.g. `find_missing_logging.py`, `find_missing_logging_filtered.py`, `parse_audit.py`, `validate_config_examples.py`, `scripts/audit_pydantic_models.py`, `scripts/audit_pydantic_models_v2.py`, `scripts/audit_pydantic_models_v3.py`, `scripts/audit_phase35_synthesis.py`, plus `c:\tmp\*.py` files). These triggered Pyright diagnostics in the main thread on every file write, polluted git status, and required user cleanup. The skill is responsible for cleaning up after its own agents. @@ -3512,12 +3512,16 @@ rm -f find_missing_logging.py find_missing_logging_filtered.py parse_audit.py va (`rm -f` is silent on missing paths, so no stderr redirect is needed; `|| true` keeps the chain from aborting on edge-case errors. Per Rule #11, the project's PreToolUse hook blocks `2>/dev/null` and other redirects unconditionally.) -Then list and remove anything else suspicious. The find DOES include `scripts/` because the 2026-05-03 run leaked `scripts/audit_pydantic_models{,_v2,_v3}.py` and `scripts/audit_phase35_synthesis.py`; those need to surface for user prompt: +Then list anything else suspicious. The find DOES include `scripts/` because the 2026-05-03 run leaked `scripts/audit_pydantic_models{,_v2,_v3}.py` and `scripts/audit_phase35_synthesis.py`; those need to surface for user prompt. Use the second-newest run (the one before this run) as the `-newer` reference: ```bash -find . -maxdepth 2 -name "*.py" -newer _audit/runs/$(ls -1 _audit/runs/ | tail -2 | head -1)/findings -not -path "./src/*" -not -path "./tests/*" -not -path "./web/*" -not -path "./cli/*" +prev_run=$(ls -1t _audit/runs/ | sed -n '2p') && [ -n "$prev_run" ] && find . -maxdepth 2 -name "*.py" -newer "_audit/runs/$prev_run/findings" -not -path "./src/*" -not -path "./tests/*" -not -path "./web/*" -not -path "./cli/*" ``` +Notes: +- `ls -1t` lists runs newest-first by mtime; `sed -n '2p'` picks the second line (i.e. the run BEFORE the one we just created in Phase 0). On the very first run there is no previous run, so `prev_run` is empty and the `[ -n "$prev_run" ] &&` guard short-circuits the `find` -- which is correct, since the cleanup target is "files newer than the previous run", not "files newer than this run". On the first run the `git status` check at the bottom of this section is the only safety net, which is fine. +- The path is quoted (`"_audit/runs/$prev_run/findings"`) to defend against pathological `_audit/runs/` contents. + Show the user the list before deleting anything that's not on the known leak list. Files in `c:\tmp\`, `/tmp\`, or any path outside the project root: leave them; the OS will reap them. Files inside `scripts/`: prompt the user before removal -- those may be intentional helpers, not leakage. If `git status` shows any new untracked `.py` file at project root or in `scripts/` that didn't exist before the audit run started, flag it as a candidate for removal. From 76d97ac3a4d8cc27402a277906cbf75d6a46d845 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sun, 3 May 2026 18:24:19 +0200 Subject: [PATCH 5/5] fix: address PR #1732 round-4 review feedback - Phase 0 .gitignore check: broaden grep pattern from '^_audit' (which misses '/_audit/' style entries) to a regex that matches all four common forms ('_audit', '_audit/', '/_audit', '/_audit/'). Prevents false-negative duplicate-add actions. CodeRabbit. --- .claude/skills/codebase-audit/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/codebase-audit/SKILL.md b/.claude/skills/codebase-audit/SKILL.md index 998953e409..9ca67b3187 100644 --- a/.claude/skills/codebase-audit/SKILL.md +++ b/.claude/skills/codebase-audit/SKILL.md @@ -50,7 +50,7 @@ Notes for the setup command: Never delete `_audit/runs/*`. The `_audit/latest` symlink always points at the most recent run; older runs accumulate. On Windows, the OpenCode adapter first attempts `New-Item -ItemType SymbolicLink` (requires Developer Mode or admin); on failure it falls back to `New-Item -ItemType Junction`, which needs no special privileges and still resolves as a directory so downstream writes to `_audit/latest/findings/` succeed. -Verify `_audit/` is in `.gitignore` via `grep -E "^_audit" .gitignore`. If not, add it. +Verify `_audit/` is in `.gitignore` via `grep -Eq '^/?_audit(/|$)' .gitignore`. The pattern allows `_audit`, `_audit/`, `/_audit`, and `/_audit/`, so plain-anchor false negatives don't trigger duplicate "add it" actions. If grep exits non-zero, add the entry. ---