Add 7 parallel VERIFY checks + SHRINK skill for codebase-health discipline#20
Open
silas-dsc wants to merge 3 commits into
Open
Add 7 parallel VERIFY checks + SHRINK skill for codebase-health discipline#20silas-dsc wants to merge 3 commits into
silas-dsc wants to merge 3 commits into
Conversation
…et/a11y) and SHRINK skill
Implements the next round of agent-workflow improvements: every check the
operator asked for (axe-core in Tester, pnpm audit, vitest --changed,
dependency-cruiser, Firestore rules tests, bundle-size budget, semgrep SAST)
plus a codebase-shrink skill for per-touch waste removal.
scripts/verify-changes.sh — restructured around a bounded-concurrency job
runner (wait -n, capped at VERIFY_PARALLELISM/nproc, max 8). Adds six new
parallel checks alongside the existing scoped lint/typecheck/test:
- audit pnpm audit --prod --audit-level high
- depcruise dependency-cruiser if .dependency-cruiser.{js,cjs,mjs,json}
- semgrep semgrep --config auto on touched source files
- knip knip --reporter json, filtered to diff-introduced orphans
- fb-rules runs the repo's firestore:test/rules:test/test:rules script
when firestore.rules was modified
- bundle-size compares built artefact sizes against .bundle-budget.json
Each check is **graceful**: exits 77 (skipped) when its tool or config isn't
present, so the script works against repos at any stage of tooling adoption
and lights up automatically as adoption happens. Existing
forbidden-token/secret/leftover scans remain synchronous.
Diff-aware tests: detects vitest vs jest from the package's package.json and
uses --changed / --changedSince respectively. Speeds up the test loop as the
suite grows.
New env knobs: VERIFY_PARALLELISM, VERIFY_SKIP (comma-separated check names),
VERIFY_LOG_DIR (default /tmp/symphony-verify-<sha>). Per-check full logs
persist; failing logs' last 30 lines surface inline in the script output.
prompts/SHRINK.md (new) — codebase-shrink-on-touch skill applied in Phase 3
alongside CODE_QUALITY. Four mandatory checks per commit: orphaned symbols
deleted, unused deps removed, duplicated blocks extracted (or refusal
justified), single-caller functions inlined where shorter & clearer. Out of
scope is unrelated waste — that's a Backlog ticket, not PR expansion.
Periodic full-repo audits use knip / depcheck / jscpd as operator-triggered
commands.
prompts/TESTER.md — added axe-core injection step per scenario via
browser_evaluate. Loads axe from CDN if not present, runs against WCAG 2 A/AA
+ 2.1 AA tags, fails the scenario on serious/critical violations.
prompts/CODE_REVIEW.md — codebase-shrink section: orphans/unused-deps/
duplication that the current diff created are Blocking; pre-existing waste in
untouched files is at most a Suggestion. Cross-references workpad's Shrink
pass note.
prompts/CODE_QUALITY.md — references SHRINK.md as the per-touch shrink gate.
prompts/VERIFY.md — documents all ten checks, parallelism behaviour,
graceful-skip semantics, per-check logs, and env knobs.
docs/AGENT_MEMORY.md — new "Codebase-health tooling — adoption status" table
showing which tools the workspace has wired up vs which the script skips
gracefully, with one-paragraph adoption guidance per tool. Periodic-audit
commands documented for operator use.
WORKFLOW.md — SHRINK added to Phase 3 skill loadout and the skill-reference
table; Phase 3 DoD requires a workpad "Shrink pass on <SHA>" entry; Phase
status checklist mentions a11y serious/critical clean for Phase 4.
README.md — Agent skills table updated with shrink + a11y entries, VERIFY
description rewritten to mention all ten checks and graceful skipping.
Existing test suite still 74/74 green. New script passes VERIFY: pass on its
own diff (with appropriate SKIPs for tools not yet adopted in this repo).
…IFY checks
When VERIFY reports `skipped` for a tool that isn't yet adopted in the target
workspace, the operator needs a clear path to wire it up. This script is that
path. It runs in any workspace (typically team-dsc) and handles every tool
VERIFY conditionally consults except `pnpm audit` (built into pnpm) and
`axe-core` (loaded from a CDN by the Tester at test time).
scripts/install-verify-tools.sh (new):
Modes:
--check (default) Detect and report. No writes. Safe for the agent
to run inside its workspace as part of "## Tooling gaps"
investigation.
--install `pnpm add -D -w` (or yarn / npm equivalents) for the npm-
installable tools: dependency-cruiser, knip, and
@firebase/rules-unit-testing (last one only when
firestore.rules is present in the workspace).
--scaffold Writes starter config files where absent:
- .dependency-cruiser.cjs (no-circular + no-orphans + an
example app→functions-internals rule when the monorepo
layout matches)
- knip.json (workspace-aware shape if pnpm-workspace.yaml
is present)
- firestore-tests/example.test.ts (only when
firestore.rules exists; uses @firebase/rules-unit-testing
+ vitest, demonstrates an unauth-fail and an auth-succeed
case the operator can extend)
- .bundle-budget.json (Remix paths if packages/app has
@remix-run/* in deps, generic otherwise)
Refuses to overwrite existing files — won't clobber operator
edits.
--all --install + --scaffold.
--dry-run Pairs with --install / --scaffold. Prints what would happen,
doesn't write. Counts in the verdict line reflect the would-
be-actioned set.
semgrep is a Python tool. The script does NOT auto-install — it prints the
install command for macOS (brew), Linux (pipx / pip --user), and Docker, and
leaves the choice to the operator (an automated `pip install` would silently
modify the host Python environment, which is intrusive).
Detection logic:
- Package manager: pnpm > yarn > npm based on lockfile presence.
- Monorepo: pnpm-workspace.yaml OR a packages/ directory.
- Firestore: firestore.rules at workspace root.
- Remix: @remix-run/* anywhere in packages/app/package.json.
Verdict line shape:
INSTALL-VERIFY-TOOLS: <mode> (...)
check: (present=X, missing=Y) + a stderr list of items still missing.
install/ (installed=X, scaffolded=Y, missing=Z, present=W) + a stderr
scaffold/ "review the modified files" reminder with a suggested commit
all: message.
prompts/VERIFY.md — "Adopting a missing tool" subsection explains the
operator-vs-agent boundary: the agent runs --check to surface gaps; only the
operator runs --install/--scaffold because adoption changes package.json and
adds gates the team has to live with.
docs/AGENT_MEMORY.md — "Adopting a missing tool — the supported path"
subsection in the Codebase-health tooling section, with the five canonical
invocations, the post-run review steps, and the updated adoption-ticket
shape that points at the script instead of bespoke install commands.
README.md — Agent skills table extended with one row for the installer.
Tested:
bash -n — syntax clean.
--help — prints the header block (sed extracts it from the
script itself, so the help stays in sync with the
code).
--check on Symphony — reports 3 PRESENT (axe-core, pnpm audit,
vitest/jest), 6 MISSING. Exits 0.
--all --dry-run — `installed=2, scaffolded=3, missing=6, present=3`
and zero files written (verified with git status).
scripts/verify- — still VERIFY: pass on the branch, no regression.
changes.sh
Existing orchestrator test suite still 74/74 green.
…, tracked_artefacts check
Two distinct concerns, both handled:
PREVENTION — agent-generated artefacts in workspace .gitignore.
Per-ticket state (.claude/, .symphony-figma/, .symphony-ports,
.symphony-app.pid, .symphony-proxy.pid) must never reach a PR. Extends
install-verify-tools.sh's --scaffold/--all mode to merge these patterns into
the workspace's .gitignore. The merge:
- Checks each pattern individually (exact line match) rather than the
wrapping comment, so it doesn't duplicate entries already covered under a
different heading. Verified by running --check against Symphony's own
.gitignore: recognises .claude/ is already covered, only flags the four
.symphony-* patterns as missing.
- Inserts a marker-delimited block (# >>> Symphony agent artefacts ... #
<<<) on the first scaffold so future runs can find and skip the section.
- Honours --dry-run (prints what would happen, no writes).
CLEANUP — historical and per-PR.
Historical (one-time, operator-triggered): new --audit-tracked mode in
install-verify-tools.sh scans `git ls-files` read-only for:
- Files in dist/, build/, coverage/, playwright-report/, test-results/,
out/, .next/, .nuxt/ — tracked output directories.
- OS / editor junk: .DS_Store, Thumbs.db, desktop.ini, *.swp, *.swo, *~.
- node_modules accidentally tracked.
- Image files >100kb outside asset directories (public/, assets/, images/,
static/, fonts/, media/, icons/, __image_snapshots__/, __screenshots__/).
- Other binaries >1MB (zip, exe, dmg, jar, pdf, sqlite, ...) outside asset
dirs.
Prints each candidate with the reason and the `git rm --cached <path>`
command to remove from the index without deleting locally. Operator
coordinates the cleanup — these removals affect everyone on pull, so they
batch into a deliberate PR. Verified --audit-tracked on Symphony repo
returns "candidates=0 — repo looks clean".
Per-PR (automated gate): new tracked_artefacts sync check in
verify-changes.sh applies the same heuristics to files the current diff
*adds* (or are untracked-not-ignored). Fails the build if anything matches.
Complements .gitignore — only ungignored additions can trigger it, so a
properly-ignored .DS_Store won't fire. Verified by temporarily creating
`some/weird/path/coverage/index.html` (not covered by Symphony's gitignore):
script correctly fails with `tracked_artefacts` and prints the remediation
hint. Verified that an ignored path (e.g. dist/foo.js, where dist/ is in
Symphony's .gitignore) does not trigger the check.
Doc updates:
- docs/AGENT_MEMORY.md — two new sections: "Agent artefacts in .gitignore"
(table of patterns + install-verify-tools invocation) and "Cleaning up
historically-committed artefacts" (--audit-tracked workflow + the per-PR
gate complementing it).
- prompts/VERIFY.md — adds check #13 to the synchronous-checks list.
- README.md — Install VERIFY tooling row in the skills table updated with
.gitignore merge + --audit-tracked.
Existing test suite still 74/74; VERIFY: pass on the branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the seven additional verification checks plus codebase-shrink discipline.
All seven new checks run in
VERIFY, six of them in parallel:pnpm audit --prod --audit-level highdependency-cruisersemgrep --config autodangerouslySetInnerHTML, eval, NoSQL/SQL injection, prototype pollutionknip --reporter json(diff-filtered)firestore:testscript whenfirestore.ruleschangedvitest --changedorjest --changedSince(auto-detected)axe-coreinjected viabrowser_evaluateper scenarioParallelisation: bounded-concurrency runner (
wait -n, capped atVERIFY_PARALLELISM/nproc, max 8). Six checks plus per-package scoped lint/typecheck/test run concurrently; cheap synchronous scanners (forbidden tokens, secrets, leftovers) run after.Graceful skip semantics: each parallel check exits 77 if its tool or config isn't present, and the script reports
SKIP: <name>rather than failing. The script works today against repos that haven't adopted every tool, and lights up automatically as they do. The verdict line tracks skips:Agents flag non-zero skip counts in
.claude/workpad.mdunder## Tooling gapsand (once per missing tool) file a Linear Backlog ticket to adopt them.New env knobs:
VERIFY_PARALLELISM,VERIFY_SKIP(comma-separated check names),VERIFY_LOG_DIR(default/tmp/symphony-verify-<sha>). Full per-check logs persist; last 30 lines of failing logs surface inline.Codebase-shrink skill (
prompts/SHRINK.md):Per-touch checks, mandatory on every Phase 3 commit:
package.jsonAdjacent unrelated waste → Backlog ticket; don't widen the PR. Code Reviewer treats orphans/unused-deps/duplication that this diff created as Blocking; pre-existing waste in untouched files is at most a Suggestion. Periodic full-repo audits documented as operator-triggered commands (
knip,depcheck,jscpd).Wiring updates:
WORKFLOW.md— SHRINK in Phase 3 skill loadout + skill-reference table + Phase 3 DoD checkbox + Phase status checklist mentions a11y.prompts/TESTER.md— axe-core injection step per scenario; serious/critical violations fail the scenario.prompts/CODE_QUALITY.md— references SHRINK as the per-touch shrink gate.prompts/CODE_REVIEW.md— codebase-shrink section distinguishing diff-introduced waste (Blocking) from pre-existing waste (Suggestion).prompts/VERIFY.md— full documentation of all ten checks, parallelism, graceful skips, env knobs, per-check logs.docs/AGENT_MEMORY.md— adoption-status table for the new tools with one-paragraph guidance per tool; periodic-audit commands.README.md— Agent skills table updated.Test plan
bash -n scripts/verify-changes.sh— syntax cleanbash scripts/verify-changes.shon this branch —VERIFY: pass (ran=12, skipped=7). Audit ran (and passed). The six new tools that aren't yet installed in the Symphony repo correctly reportSKIP, notFAIL.VERIFY_SKIP=audit,semgrep bash scripts/verify-changes.sh— confirms env-var skip pathnproc, and the## Tooling gapsworkpad section is populated for skipped tools.tdd/verify/memory/debug/shrinktags to confirm the new gates are catching the right class of issues (and not over-firing).Generated by Claude Code