fix: restore unloaded and missing PR-review agents#1875
Merged
Conversation
…es + 13 .opencode ports + skill dispatch cleanup)
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (18)
WalkthroughThis PR establishes a specialized agent review system for SynthOrg. It introduces eleven new agent specifications (api-contract-drift, async-concurrency-reviewer, comment-quality-rot, conventions-enforcer, docs-consistency, frontend-reviewer, go-conventions-enforcer, go-security-reviewer, infra-reviewer, issue-resolution-verifier, logging-audit, resilience-audit, and test-quality-reviewer) and updates four existing agents (comment-analyzer, pr-test-analyzer, silent-failure-hunter) to use explicit model configuration. Each agent defines specific code-review checklists, severity levels, and structured report formats. The PR then updates two skill orchestration files to route review categories from generic fallbacks to their dedicated subagent types, ensuring each review function uses its specialized agent configuration. Comment |
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of specialized review agents defined in .claude/agents/ and updates the aurelio-review-pr and pre-pr-review skills to dispatch these agents directly by name. The changes standardize the subagent_type mapping to ensure consistent execution across different environments. Feedback focuses on three remaining inconsistencies in the pre-pr-review/SKILL.md table where design-token-audit, tool-parity-checker, and diagram-syntax-validator still use descriptive prompt paths instead of simple agent identifiers, which would cause task dispatch failures.
| | **security-reviewer** | Files in `src/synthorg/api/`, `src/synthorg/security/`, `src/synthorg/tools/`, `src/synthorg/config/`, `src/synthorg/persistence/`, `src/synthorg/engine/` changed, OR any `web_src` changed, OR diff contains `subprocess`, `eval`, `exec`, `pickle`, `yaml.load`, `sql`, auth/credential patterns | `security-reviewer` | | ||
| | **frontend-reviewer** | Any `web_src` or `web_test` | `code-reviewer` (custom prompt below) | | ||
| | **frontend-reviewer** | Any `web_src` or `web_test` | `frontend-reviewer` | | ||
| | **design-token-audit** | Any `web_src` | `.claude/agents/design-token-audit.md` prompt (scans for density, animation, spacing token violations) | |
Contributor
There was a problem hiding this comment.
The subagent_type for design-token-audit is inconsistent with the rest of the table. It contains a descriptive prompt path instead of the simple agent identifier. This will cause the Task tool to fail as it expects a valid agent name. Please update it to match the identifier used in .claude/agents/ and the corresponding table in aurelio-review-pr/SKILL.md.
Suggested change
| | **design-token-audit** | Any `web_src` | `.claude/agents/design-token-audit.md` prompt (scans for density, animation, spacing token violations) | | |
| | **design-token-audit** | Any web_src | design-token-audit | |
| | **go-security-reviewer** | Any `cli_go` whose diff contains `exec.Command`, `os/exec`, `http`, `os.Remove`, `os.WriteFile`, `filepath`, user-supplied paths | `go-security-reviewer` | | ||
| | **go-conventions-enforcer** | Any `cli_go` | `go-conventions-enforcer` | | ||
| | **issue-resolution-verifier** | Issue context was found in Phase 0 step 6 | `issue-resolution-verifier` | | ||
| | **tool-parity-checker** | Any `.claude/` or `.opencode/` or `opencode.json` or `AGENTS.md` or `CLAUDE.md` file changed | `.claude/agents/tool-parity-checker.md` prompt (verifies Claude Code <-> OpenCode config parity) | |
Contributor
There was a problem hiding this comment.
The subagent_type for tool-parity-checker is inconsistent and contains a long description instead of the agent identifier. This violates the dual-tool parity invariant mentioned in the PR description and will lead to tool execution failures. It should be updated to the simple agent name as seen in the aurelio-review-pr skill.
Suggested change
| | **tool-parity-checker** | Any `.claude/` or `.opencode/` or `opencode.json` or `AGENTS.md` or `CLAUDE.md` file changed | `.claude/agents/tool-parity-checker.md` prompt (verifies Claude Code <-> OpenCode config parity) | | |
| | **tool-parity-checker** | Any .claude/ or .opencode/ or opencode.json or AGENTS.md or CLAUDE.md file changed | tool-parity-checker | |
| | **go-conventions-enforcer** | Any `cli_go` | `go-conventions-enforcer` | | ||
| | **issue-resolution-verifier** | Issue context was found in Phase 0 step 6 | `issue-resolution-verifier` | | ||
| | **tool-parity-checker** | Any `.claude/` or `.opencode/` or `opencode.json` or `AGENTS.md` or `CLAUDE.md` file changed | `.claude/agents/tool-parity-checker.md` prompt (verifies Claude Code <-> OpenCode config parity) | | ||
| | **diagram-syntax-validator** | Any `docs` files changed that contain ` ```d2 ` or ` ```mermaid ` blocks | `.claude/agents/diagram-syntax-validator.md` prompt (validates diagram syntax, conventions, fence types) | |
Contributor
There was a problem hiding this comment.
The subagent_type for diagram-syntax-validator is still using a custom prompt description instead of the direct agent call. To maintain consistency with the other restored agents and ensure correct dispatch via the Task tool, this should be updated to the simple identifier diagram-syntax-validator.
Aureliolo
pushed a commit
that referenced
this pull request
May 12, 2026
<!-- HIGHLIGHTS_START --> ## Highlights > _AI-generated summary (model: `openai/gpt-4.1-mini` via GitHub Models). Commit-based changelog below._ ### What you'll notice - Password and secret fields now include an eye-toggle for easier visibility control. - Containers running without probes are shown as healthy in the doctor command. - Unloaded and missing PR-review agents are restored and available again. ### What's new - Gate baseline protection is enhanced to block em-dashes during writing. ### Under the hood - Replaced Atlas with yoyo-migrations for persistence management. - Refactored codebase extensively, including context-bound user authentication and registry pattern for enums. - Improved linting by draining magic number usages and tightening mock and constant checks. - Updated CI to retry Docker pushes on network timeout errors. - Updated apko lockfiles for dependency management. <!-- HIGHLIGHTS_END --> :robot: I have created a release *beep* *boop* --- ## [0.8.3](v0.8.2...v0.8.3) (2026-05-12) ### Features * harden gate baseline protection + block em-dashes at write time ([#1860](#1860)) ([b41f151](b41f151)) * **web:** eye-toggle on every password / secret field ([#1873](#1873)) ([9070387](9070387)) ### Bug Fixes * **ci:** retry Docker push on Go net/http deadline + cancellation errors ([#1877](#1877)) ([23a0bfa](23a0bfa)) * **cli:** render running-no-probe containers as healthy in doctor ([#1870](#1870)) ([6263795](6263795)) * restore unloaded and missing PR-review agents ([#1875](#1875)) ([db004fd](db004fd)), closes [#1871](#1871) ### Refactoring * bind authenticated user via ContextVar ([#1858](#1858)) ([57ed0b4](57ed0b4)) * code-structure cleanup (sub-tasks D + F + G + H + I) ([#1859](#1859)) ([362e5c8](362e5c8)) * convert enum dispatch to registry pattern ([#1854](#1854)) ([e90550e](e90550e)) * drain no_magic_numbers baseline to zero via Final hoists ([#1856](#1856) phase 2) ([#1872](#1872)) ([ec8109e](ec8109e)) * drain pagination + loop-init + kill-switch baselines ([#1857](#1857)) ([#1868](#1868)) ([115c3c2](115c3c2)) * **persistence:** replace Atlas with yoyo-migrations ([#1876](#1876)) ([1b7e975](1b7e975)), closes [#1874](#1874) * protocols audit follow-up (REVIEW + fold pass) ([#1869](#1869)) ([af33ddb](af33ddb)) * protocols audit follow-up REMOVE pass ([#1867](#1867)) ([dd1eebc](dd1eebc)) * tighten check_mock_spec gate, add mock_of[T], drain baseline ([#1862](#1862)) ([240a253](240a253)) * tighten check_no_magic_numbers for named module constants ([#1856](#1856)) ([#1866](#1866)) ([90c933b](90c933b)) ### CI/CD * update apko lockfiles ([#1863](#1863)) ([2bd32e6](2bd32e6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: synthorg-repo-bot[bot] <279117679+synthorg-repo-bot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Aureliolo
added a commit
that referenced
this pull request
May 18, 2026
…ents (#2000) ## Summary Builds on the anti-ghost-wiring toolset (gate + audit agent 14 + pre-PR mini-pass) with a hardening pass and the resolution of a long-standing review-agent loading bug surfaced during this PR's own `/pre-pr-review`. ### Anti-ghost-wiring gate (`scripts/check_no_ghost_wiring.py`) - **Fail-closed, clean UX**: malformed manifest, unreadable/non-UTF8 source, and syntax errors now print a clear gate message and `return 1` instead of an uncaught traceback (was inconsistent with the manifest-missing branch). - **Closes a fail-open**: now excludes a symbol's own defining module when counting construction sites, properly enforcing the manifest contract ("outside its own defining module"). A symbol only ever referenced inside its own file is correctly still a ghost. - `state` is `Literal["ENFORCED","PENDING"]`; `RUNTIME_PREFIXES` annotated `Final[tuple[str, ...]]`; nudge cap is a named constant; docstring uses `uv run`; generator simplified. - **New test suite** `tests/unit/scripts/test_check_no_ghost_wiring.py` (12 cases): happy path, regression, defining-module exclusion, attribute-call, non-runtime-prefix, PENDING nudge, manifest-missing, malformed manifest, syntax-error fail-closed, non-UTF8 fail-closed, CLI `--repo-root`. Matches the sibling `tests/unit/scripts/test_check_*.py` pattern. ### Review-agent loading bug (RCA + fix) `/pre-pr-review` could dispatch only 16 of 26 `.claude/agents/*.md`. **Root cause (26/26 correlation)**: the agent loader parses YAML frontmatter; an unquoted `description:` scalar containing an internal `: ` (colon-space) fails YAML's block-mapping parse, so Claude Code silently drops the agent. All 16 working agents had no internal `: `; all 10 failing ones had exactly one. This also explains why the earlier `model: inherit -> sonnet` workaround (#1875) never worked. **Fix**: double-quoted the `description:` value in the 10 affected agent files (behaviour-preserving; bodies untouched). They register on the next Claude Code restart. ### Skill hardening `pre-pr-review/SKILL.md` Phase 4 gains a MANDATORY agent-registry preflight: rostered `subagent_type`s are checked against the live list and missing agents surface the RCA + remediation, then run via `general-purpose` seeded with the verbatim agent body, instead of silently degrading. ### Docs - `audit-category-gate-coverage.md`: new classification row for the runtime-component ghost-wiring category; `~155 -> ~159` agents. - `pre-pr-review/SKILL.md`: `~155 -> ~159`; mini-pass skip-condition "five -> six". ## Test plan - `uv run ruff check` + `ruff format` clean on the gate and test. - `uv run mypy` clean. - `uv run python -m pytest tests/unit/scripts/test_check_no_ghost_wiring.py` -> 12 passed. - `uv run python scripts/check_no_ghost_wiring.py` -> exit 0 on the real tree. - Pre-push hooks all passed (incl. the new `no-ghost-wiring` gate, mypy-affected, pytest-affected, convention-gate inventory, no-review-origin, no-migration-framing). - Agent fix verified: zero unquoted `description:` with an internal `: ` remain across all 26 files. ## Review coverage `/pre-pr-review` ran: docs-consistency, tool-parity-checker, code-reviewer, python-reviewer, type-design-analyzer (the other 4 were blocked by the very bug fixed here). All valid findings (16) implemented; no deferrals. type-design optional extras intentionally skipped per the finding's own "do not over-engineer a small gate" guidance. Relates to EPIC #1955 (this is the regression gate protecting that work; it does not wire the runtime).
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
1 participant
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
The pr-review-toolkit removal (PR #1833) left the agent inventory in a broken state. Two distinct problems, one fix:
.claude/agents/*.mdfiles but the Claude Code harness silently drops them:pr-test-analyzer,silent-failure-hunter,comment-analyzer. Reproduced live during a/pre-pr-reviewrun onworktree-password-toggle, documented in pre-pr-review skill references agents not loaded by Claude Code harness #1871..opencode/agents/, with no.claude/agents/counterpart, breaking the dual-tool parity invariant and forcing/pre-pr-reviewand/aurelio-review-prto fall back tocode-reviewerwith inline prompts (losing the specialised system prompts).What changed
model: inheritwithmodel: sonneton the 3 broken agents. Caveat: this is the workaround suggested in pre-pr-review skill references agents not loaded by Claude Code harness #1871; the root cause is not fully understood.type-design-analyzeralso declaresmodel: inheritand IS loaded by the harness, which contradicts the issue's hypothesis. The fix matches the loaded-agent baseline (code-reviewer,python-reviewer,security-reviewer,persistence-reviewer,go-reviewerall declare an explicit model) and should make the agents load; a separate diagnostic issue may be worth filing to understand whyinheritworks for one file and not the others..opencode/agents/to.claude/agents/:api-contract-drift,async-concurrency-reviewer,comment-quality-rot,conventions-enforcer,docs-consistency,frontend-reviewer,go-conventions-enforcer,go-security-reviewer,infra-reviewer,issue-resolution-verifier,logging-audit,resilience-audit,test-quality-reviewermode: subagent, cloud-provider model,permission:map) to Claude Code (name:,model: sonnet,color:,tools:list). Bodies preserved verbatim..claude/skills/pre-pr-review/SKILL.mdand.claude/skills/aurelio-review-pr/SKILL.mddispatch tables to call each specialist directly (e.g.subagent_type: docs-consistency) instead ofcode-reviewerwith a custom prompt.Test plan
The Claude Code agent registry is built at session start, so the restored agents are NOT visible in the session that created this PR. Verification requires a fresh session.
After review:
subagent_typelist when a new agent invocation is attempted.pr-test-analyzer,frontend-reviewer,docs-consistency,logging-audit. Each must accept the call (notAgent type 'X' not found)./pre-pr-reviewon a small change and confirm Phase 3 dispatches each specialist by name without falling back tocode-reviewer-with-custom-prompt.pr-test-analyzer/silent-failure-hunter/comment-analyzerstill fails to load after theinherit -> sonnetchange, fall back to the canary-morph diagnostic (copy the workingtype-design-analyzer.md, progressively morph fields, find the actual differentiator).Review coverage
Local agent review skipped: all 18 changed files are
.mdunder.claude/(nosrc_py,test_py,web_src,cli_go, etc.), so the/pre-pr-reviewskill's auto-skip rule applied. Phase 2 automated checks had no scope (no Python/web/Go files). The change is meta-tooling-only.Closes #1871