Remove deprecated agent types and exempt QA from spawn gate #125

Merged
forgejo_admin merged 1 commit from fix/qa-spawn-gate-exception into main 2026-03-18 17:58:38 +00:00
Contributor

Summary

  • Removed 6 deprecated specialized agent types from agent-spawn-requirements.json (Phase 12 consolidation)
  • Set QA required_patterns to [] — QA can't write code, so the scoping gate was pure friction
  • Zero changes to check-agent-spawn.sh — schema-driven design means the hook just works

Changes

  • schemas/agent-spawn-requirements.json: removed dev-backend, dev-frontend, devops, dev-qa, frontend-qa, devops-qa; updated qa to empty patterns with rationale in description

Test Plan

  • Spawn a QA agent without issue/PR references — should succeed
  • Spawn a Dev agent without issue reference — should still be blocked
  • Spawn an Explore agent — should still pass through
  • No regressions in Dottie/general-purpose gate (still requires plan-)

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive
  • Closes #124
  • todo-agent-spawn-qa-exception — the pal-e-docs TODO that identified this gap
  • plan-pal-e-agency — Phase 12 deprecated the specialized agents
## Summary - Removed 6 deprecated specialized agent types from `agent-spawn-requirements.json` (Phase 12 consolidation) - Set QA `required_patterns` to `[]` — QA can't write code, so the scoping gate was pure friction - Zero changes to `check-agent-spawn.sh` — schema-driven design means the hook just works ## Changes - `schemas/agent-spawn-requirements.json`: removed `dev-backend`, `dev-frontend`, `devops`, `dev-qa`, `frontend-qa`, `devops-qa`; updated `qa` to empty patterns with rationale in description ## Test Plan - [ ] Spawn a QA agent without issue/PR references — should succeed - [ ] Spawn a Dev agent without issue reference — should still be blocked - [ ] Spawn an Explore agent — should still pass through - [ ] No regressions in Dottie/general-purpose gate (still requires `plan-`) ## Review Checklist - [ ] Passed automated review-fix loop - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive ## Related - Closes #124 - `todo-agent-spawn-qa-exception` — the pal-e-docs TODO that identified this gap - `plan-pal-e-agency` — Phase 12 deprecated the specialized agents
Six specialized agent types (dev-backend, dev-frontend, devops, dev-qa,
frontend-qa, devops-qa) were deprecated in pal-e-agency Phase 12 but
still present in the schema. QA agents cannot write code (Write/Edit/Bash
excluded), so the scoping gate added friction without serving its purpose.

Closes forgejo_admin/claude-custom#124

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
Contributor

PR #125 Review

DOMAIN REVIEW

Tech stack: JSON schema consumed by a Bash hook (check-agent-spawn.sh). This is a configuration-only change -- no logic changes to the hook itself.

Schema correctness verified. The hook at hooks/check-agent-spawn.sh handles empty required_patterns arrays correctly (lines 44-48): PATTERN_COUNT evaluates to 0, the if [ "$PATTERN_COUNT" -eq 0 ] branch fires, and the hook exits with 0 (allow). This is the same code path used by Explore and claude-code-guide, both of which already have required_patterns: []. The QA exemption is consistent with existing patterns.

Removal of 6 deprecated types is clean. The types dev-backend, dev-frontend, devops, dev-qa, frontend-qa, devops-qa were already deprecated at the agent config level (.md.deprecated files exist in agents/). Removing them from the spawn schema means any attempt to spawn these types will now hit the "Unknown agent type" deny path (line 38-41 of the hook). This is correct behavior -- it enforces the consolidation from Phase 12 (PR #108).

No JSON syntax issues. The diff cleanly removes the 6 type blocks and updates the qa entry. The remaining schema is well-formed.

BLOCKERS

None.

  • No new functionality requiring tests -- this is schema configuration, and the consuming hook (check-agent-spawn.sh) is unchanged.
  • No user input vectors -- the schema is read by the hook at spawn time, not exposed to external input.
  • No secrets or credentials.
  • No DRY violations in auth/security paths.

NITS

  1. Dead code in hooks/inject-subagent-context.sh: Lines 22-30 and 34-41 still contain case branches for frontend-qa, dev-qa, devops-qa, dev-backend, dev-frontend, and devops. These are unreachable now -- the spawn gate will deny these types before SubagentStart fires. Not a bug (SubagentStart is guidance-only and cannot block), but dead code that should be cleaned up in a follow-up.

  2. Stale matcher in settings.json line 232: The SubagentStart matcher is "qa|dev|dev-backend|dev-frontend|devops|frontend-qa|dev-qa|devops-qa|general-purpose|dottie|penny". The deprecated types should be removed from this matcher. Again, not harmful (the hook just exits cleanly for unmatched types via the * case), but it is misleading documentation.

  3. devops type still in schema on main but removed in this PR: Confirming this is intentional. The devops type had "Same gate as dev" and was functionally identical to dev. The consolidated model routes all dev work through the single dev type. This is correct per Phase 12 consolidation.

Both nits 1 and 2 should be tracked as a follow-up cleanup issue rather than blocking this PR, since they are pre-existing dead code that this PR's change makes more obviously stale.

SOP COMPLIANCE

  • Branch named after issue number -- Branch is fix/qa-spawn-gate-exception, not 124-.... Convention expects the issue number in the branch name. However, the branch name is descriptive and the PR body explicitly references Closes #124. Minor deviation.
  • PR body follows template -- Has Summary, Changes, Test Plan, Review Checklist, Related sections.
  • Related references plan slug -- References plan-pal-e-agency (Phase 12).
  • No secrets committed
  • No unnecessary file changes -- 1 file changed, exactly scoped to the issue
  • Commit messages are descriptive (inferred from PR title)

PROCESS OBSERVATIONS

Deployment frequency: This is a low-risk config change that unblocks QA agent spawning. Merging quickly improves DF by removing friction from the review-fix loop.

Change failure risk: Minimal. The hook is schema-driven, the schema change is additive (removing entries = more restrictive for deprecated types, less restrictive for QA). The empty-array code path is already exercised by two other agent types in production.

Follow-up needed: The dead code in inject-subagent-context.sh and the stale settings.json matcher should be cleaned up in a dedicated issue. This is housekeeping, not a risk.

VERDICT: APPROVED

## PR #125 Review ### DOMAIN REVIEW **Tech stack:** JSON schema consumed by a Bash hook (`check-agent-spawn.sh`). This is a configuration-only change -- no logic changes to the hook itself. **Schema correctness verified.** The hook at `hooks/check-agent-spawn.sh` handles empty `required_patterns` arrays correctly (lines 44-48): `PATTERN_COUNT` evaluates to `0`, the `if [ "$PATTERN_COUNT" -eq 0 ]` branch fires, and the hook exits with `0` (allow). This is the same code path used by `Explore` and `claude-code-guide`, both of which already have `required_patterns: []`. The QA exemption is consistent with existing patterns. **Removal of 6 deprecated types is clean.** The types `dev-backend`, `dev-frontend`, `devops`, `dev-qa`, `frontend-qa`, `devops-qa` were already deprecated at the agent config level (`.md.deprecated` files exist in `agents/`). Removing them from the spawn schema means any attempt to spawn these types will now hit the "Unknown agent type" deny path (line 38-41 of the hook). This is correct behavior -- it enforces the consolidation from Phase 12 (PR #108). **No JSON syntax issues.** The diff cleanly removes the 6 type blocks and updates the `qa` entry. The remaining schema is well-formed. ### BLOCKERS None. - No new functionality requiring tests -- this is schema configuration, and the consuming hook (`check-agent-spawn.sh`) is unchanged. - No user input vectors -- the schema is read by the hook at spawn time, not exposed to external input. - No secrets or credentials. - No DRY violations in auth/security paths. ### NITS 1. **Dead code in `hooks/inject-subagent-context.sh`:** Lines 22-30 and 34-41 still contain `case` branches for `frontend-qa`, `dev-qa`, `devops-qa`, `dev-backend`, `dev-frontend`, and `devops`. These are unreachable now -- the spawn gate will deny these types before SubagentStart fires. Not a bug (SubagentStart is guidance-only and cannot block), but dead code that should be cleaned up in a follow-up. 2. **Stale matcher in `settings.json` line 232:** The SubagentStart matcher is `"qa|dev|dev-backend|dev-frontend|devops|frontend-qa|dev-qa|devops-qa|general-purpose|dottie|penny"`. The deprecated types should be removed from this matcher. Again, not harmful (the hook just exits cleanly for unmatched types via the `*` case), but it is misleading documentation. 3. **`devops` type still in schema on `main` but removed in this PR:** Confirming this is intentional. The `devops` type had `"Same gate as dev"` and was functionally identical to `dev`. The consolidated model routes all dev work through the single `dev` type. This is correct per Phase 12 consolidation. Both nits 1 and 2 should be tracked as a follow-up cleanup issue rather than blocking this PR, since they are pre-existing dead code that this PR's change makes more obviously stale. ### SOP COMPLIANCE - [ ] Branch named after issue number -- Branch is `fix/qa-spawn-gate-exception`, not `124-...`. Convention expects the issue number in the branch name. However, the branch name is descriptive and the PR body explicitly references `Closes #124`. Minor deviation. - [x] PR body follows template -- Has Summary, Changes, Test Plan, Review Checklist, Related sections. - [x] Related references plan slug -- References `plan-pal-e-agency` (Phase 12). - [x] No secrets committed - [x] No unnecessary file changes -- 1 file changed, exactly scoped to the issue - [x] Commit messages are descriptive (inferred from PR title) ### PROCESS OBSERVATIONS **Deployment frequency:** This is a low-risk config change that unblocks QA agent spawning. Merging quickly improves DF by removing friction from the review-fix loop. **Change failure risk:** Minimal. The hook is schema-driven, the schema change is additive (removing entries = more restrictive for deprecated types, less restrictive for QA). The empty-array code path is already exercised by two other agent types in production. **Follow-up needed:** The dead code in `inject-subagent-context.sh` and the stale `settings.json` matcher should be cleaned up in a dedicated issue. This is housekeeping, not a risk. ### VERDICT: APPROVED
forgejo_admin deleted branch fix/qa-spawn-gate-exception 2026-03-18 17:58:38 +00:00
Sign in to join this conversation.
No description provided.