ldraney/claude-custom

Fork 0

Add agent spawn requirements schema + refactor hook #60

Merged

forgejo_admin merged 2 commits from 59-agent-spawn-requirements-schema into main

2026-03-07 17:39:58 +00:00

forgejo_admin commented

2026-03-07 17:28:42 +00:00

Contributor

Summary

Replaces hardcoded regex validation in check-agent-spawn.sh with a JSON-driven schema approach. Agent types and their required_patterns are now defined in schemas/agent-spawn-requirements.json. Adding a new agent type requires only a JSON entry change -- no bash editing needed.

Changes

schemas/agent-spawn-requirements.json (new) -- Schema defining required_patterns, description, produces, and isolation for dev, qa, general-purpose, and Explore agent types
hooks/check-agent-spawn.sh (modified) -- Refactored to read schema, validate ALL required_patterns for the given subagent_type, and deny unknown types. 45 lines, bash + jq only.

Test Plan

echo '{"tool_input":{"prompt":"Fix issue #100","subagent_type":"dev"}}' | bash hooks/check-agent-spawn.sh -- exits 0, no output (pass)
echo '{"tool_input":{"prompt":"Review PR #101. Issue #100","subagent_type":"qa"}}' | bash hooks/check-agent-spawn.sh -- exits 0, no output (pass)
echo '{"tool_input":{"prompt":"Update docs. plan-foo","subagent_type":"general-purpose"}}' | bash hooks/check-agent-spawn.sh -- exits 0, no output (pass)
echo '{"tool_input":{"prompt":"Explore the codebase","subagent_type":"dev"}}' | bash hooks/check-agent-spawn.sh -- outputs deny JSON (missing issue ref)
echo '{"tool_input":{"prompt":"Do stuff","subagent_type":"new-unknown-type"}}' | bash hooks/check-agent-spawn.sh -- outputs deny JSON (unknown type)

Review Checklist

Schema is valid JSON (verified with jq .)
Hook is under 50 lines (45 lines)
All 5 manual tests pass
Unknown types denied
Backward compatible with existing spawn patterns
No unrelated changes

phase-7f-2-agent-spawn-schema -- phase note
agent-spawn-conventions -- convention to update after merge

Plan: plan-2026-02-26-tf-modularize-postgres (Phase 7f-2)
Forgejo issue: #59

## Summary Replaces hardcoded regex validation in `check-agent-spawn.sh` with a JSON-driven schema approach. Agent types and their required_patterns are now defined in `schemas/agent-spawn-requirements.json`. Adding a new agent type requires only a JSON entry change -- no bash editing needed. ## Changes - **`schemas/agent-spawn-requirements.json`** (new) -- Schema defining required_patterns, description, produces, and isolation for dev, qa, general-purpose, and Explore agent types - **`hooks/check-agent-spawn.sh`** (modified) -- Refactored to read schema, validate ALL required_patterns for the given subagent_type, and deny unknown types. 45 lines, bash + jq only. ## Test Plan - `echo '{"tool_input":{"prompt":"Fix issue #100","subagent_type":"dev"}}' | bash hooks/check-agent-spawn.sh` -- exits 0, no output (pass) - `echo '{"tool_input":{"prompt":"Review PR #101. Issue #100","subagent_type":"qa"}}' | bash hooks/check-agent-spawn.sh` -- exits 0, no output (pass) - `echo '{"tool_input":{"prompt":"Update docs. plan-foo","subagent_type":"general-purpose"}}' | bash hooks/check-agent-spawn.sh` -- exits 0, no output (pass) - `echo '{"tool_input":{"prompt":"Explore the codebase","subagent_type":"dev"}}' | bash hooks/check-agent-spawn.sh` -- outputs deny JSON (missing issue ref) - `echo '{"tool_input":{"prompt":"Do stuff","subagent_type":"new-unknown-type"}}' | bash hooks/check-agent-spawn.sh` -- outputs deny JSON (unknown type) ## Review Checklist - [x] Schema is valid JSON (verified with `jq .`) - [x] Hook is under 50 lines (45 lines) - [x] All 5 manual tests pass - [x] Unknown types denied - [x] Backward compatible with existing spawn patterns - [x] No unrelated changes ## Related Notes - `phase-7f-2-agent-spawn-schema` -- phase note - `agent-spawn-conventions` -- convention to update after merge ## Related - Plan: `plan-2026-02-26-tf-modularize-postgres` (Phase 7f-2) - Forgejo issue: #59

forgejo_admin added 1 commit

2026-03-07 17:28:42 +00:00

Add agent spawn requirements schema + refactor hook to use it 81d086ad07

Replace hardcoded regex validation in check-agent-spawn.sh with a
JSON-driven approach. Agent types and their required_patterns are
now defined in schemas/agent-spawn-requirements.json. The hook reads
the schema, validates all patterns for the given subagent_type, and
denies unknown types. Adding a new agent type now requires only a
JSON entry change.

Closes #59

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin commented

2026-03-07 17:31:52 +00:00

Author

Contributor

PR #60 Review

BLOCKERS

1. QA required_pattern pr matches common English substrings

In schemas/agent-spawn-requirements.json, the QA type has:

"required_patterns": ["#[0-9]+|[Ii]ssue|Forgejo issue", "PR|pull request|pr"]

The lowercase pr alternative will match any prompt containing the substring "pr" anywhere -- "provide", "process", "prompt", "project", "production", etc. This makes the second pattern effectively always-pass, defeating the purpose of requiring a PR reference for QA agents.

Suggested fix: Use a more specific pattern such as PR #[0-9]+|[Pp]ull [Rr]equest|PR [0-9]+ or add word boundaries if grep supports them (\bPR\b|\bpr\b|pull request). Note that grep -E does support \b.

NITS

1. No set -euo pipefail

The original script lacked it too, so this is inherited, but adding set -euo pipefail would guard against silent failures in the jq or grep calls. Non-blocking.

2. Schema $comment field

Using $comment as a documentation key is fine but unconventional for a non-JSON-Schema file. A top-level "_comment" or just a README alongside the schema might be cleaner. Non-blocking.

3. produces and isolation fields are informational-only

The hook reads only required_patterns. The produces and isolation fields are not consumed by any code in this PR. This is fine for future use, but worth noting they are currently dead metadata. Non-blocking.

SOP COMPLIANCE

Branch named after issue (59-agent-spawn-requirements-schema references #59)
PR body has: Summary, Changes, Test Plan, Related
Related section references the plan slug (plan-2026-02-26-tf-modularize-postgres)
No secrets committed
No unnecessary file changes (2 files, tightly scoped)
Commit messages are descriptive (PR title is clear)
Tests exist and pass -- manual test plan documented in PR body; no automated tests (acceptable for a bash hook, but noted)

VERDICT: NOT APPROVED

One blocker: the QA pr pattern is too permissive and would effectively skip PR-reference validation for any prompt containing common English words. Fix the regex pattern specificity, and this is ready to merge.

## PR #60 Review ### BLOCKERS **1. QA required_pattern `pr` matches common English substrings** In `schemas/agent-spawn-requirements.json`, the QA type has: ```json "required_patterns": ["#[0-9]+|[Ii]ssue|Forgejo issue", "PR|pull request|pr"] ``` The lowercase `pr` alternative will match any prompt containing the substring "pr" anywhere -- "provide", "process", "prompt", "project", "production", etc. This makes the second pattern effectively always-pass, defeating the purpose of requiring a PR reference for QA agents. Suggested fix: Use a more specific pattern such as `PR #[0-9]+|[Pp]ull [Rr]equest|PR [0-9]+` or add word boundaries if grep supports them (`\bPR\b|\bpr\b|pull request`). Note that `grep -E` does support `\b`. ### NITS **1. No `set -euo pipefail`** The original script lacked it too, so this is inherited, but adding `set -euo pipefail` would guard against silent failures in the jq or grep calls. Non-blocking. **2. Schema `$comment` field** Using `$comment` as a documentation key is fine but unconventional for a non-JSON-Schema file. A top-level `"_comment"` or just a README alongside the schema might be cleaner. Non-blocking. **3. `produces` and `isolation` fields are informational-only** The hook reads only `required_patterns`. The `produces` and `isolation` fields are not consumed by any code in this PR. This is fine for future use, but worth noting they are currently dead metadata. Non-blocking. ### SOP COMPLIANCE - [x] Branch named after issue (`59-agent-spawn-requirements-schema` references #59) - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Related section references the plan slug (`plan-2026-02-26-tf-modularize-postgres`) - [x] No secrets committed - [x] No unnecessary file changes (2 files, tightly scoped) - [x] Commit messages are descriptive (PR title is clear) - [ ] Tests exist and pass -- manual test plan documented in PR body; no automated tests (acceptable for a bash hook, but noted) ### VERDICT: NOT APPROVED One blocker: the QA `pr` pattern is too permissive and would effectively skip PR-reference validation for any prompt containing common English words. Fix the regex pattern specificity, and this is ready to merge.

forgejo_admin added 1 commit

2026-03-07 17:38:01 +00:00

Fix QA blocker: tighten PR pattern + add pipefail + rename comment key d62577247c

- QA pattern: PR|pull request|pr → PR #N|PR N|Pull Request|pulls/N (prevents false matches on "provide", "prompt")
- Add set -euo pipefail to hook
- Rename $comment to description in schema

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin commented

2026-03-07 17:39:15 +00:00

Author

Contributor

PR #60 Review (Re-review after fix commit `d625772`)

BLOCKERS

None. The previous blocker (broad QA regex matching "pr" substring in words like "prompt", "project") is resolved. The QA required_patterns now uses PR #[0-9]+|PR [0-9]+|[Pp]ull [Rr]equest|pulls/[0-9]+, which requires explicit PR references.

NITS

Unused schema fields -- produces and isolation are defined in the schema but not read by the hook. Not harmful (they serve as documentation / future use), but worth noting they are dead data for now. Consider a comment in the schema indicating they are informational.
set -euo pipefail + jq pipeline -- With set -e, if jq fails to parse the input on lines 9-10 (e.g., non-JSON input piped to the hook), the script will exit immediately with a non-zero code rather than falling through to the allow path. This is the correct behavior (fail-closed), but worth being aware of -- a malformed hook input will block the spawn rather than silently allowing it.
Schema path assumption -- The schema resolution $(cd "$(dirname "$0")/.." && pwd)/schemas/ assumes the hook lives one directory below the repo root. This is correct for the current layout (hooks/check-agent-spawn.sh -> ../schemas/), but if hooks are ever relocated, this will break silently (denied with "schema not found"). The fail-closed fallback is good here.

SOP COMPLIANCE

Branch named after issue -- 59-agent-spawn-requirements-schema references issue #59
PR body follows template -- Summary, Changes, Test Plan, Related sections all present
Related references plan slug -- plan-2026-02-26-tf-modularize-postgres (Phase 7f-2) referenced
No secrets committed -- No .env files, credentials, or sensitive data
No scope creep -- 2 files changed, both directly related to the schema refactor
Commit messages -- Fix commit addresses specific review feedback
Test plan provided -- 5 manual test cases covering pass, deny, and unknown type scenarios

VERDICT: APPROVED

All three previous findings (1 blocker, 2 relevant nits) have been addressed:

Blocker: QA pattern tightened -- no longer matches bare "pr" substrings
Nit: set -euo pipefail added
Nit: $comment renamed to description in schema

The schema-driven approach is clean. The hook is 45 lines, reads required_patterns from JSON, validates all patterns, and denies unknown types. Fail-closed on missing schema is the right default. Good to merge.

## PR #60 Review (Re-review after fix commit d625772) ### BLOCKERS None. The previous blocker (broad QA regex matching "pr" substring in words like "prompt", "project") is resolved. The QA `required_patterns` now uses `PR #[0-9]+|PR [0-9]+|[Pp]ull [Rr]equest|pulls/[0-9]+`, which requires explicit PR references. ### NITS 1. **Unused schema fields** -- `produces` and `isolation` are defined in the schema but not read by the hook. Not harmful (they serve as documentation / future use), but worth noting they are dead data for now. Consider a comment in the schema indicating they are informational. 2. **`set -euo pipefail` + jq pipeline** -- With `set -e`, if `jq` fails to parse the input on lines 9-10 (e.g., non-JSON input piped to the hook), the script will exit immediately with a non-zero code rather than falling through to the allow path. This is the correct behavior (fail-closed), but worth being aware of -- a malformed hook input will block the spawn rather than silently allowing it. 3. **Schema path assumption** -- The schema resolution `$(cd "$(dirname "$0")/.." && pwd)/schemas/` assumes the hook lives one directory below the repo root. This is correct for the current layout (`hooks/check-agent-spawn.sh` -> `../schemas/`), but if hooks are ever relocated, this will break silently (denied with "schema not found"). The fail-closed fallback is good here. ### SOP COMPLIANCE - [x] **Branch named after issue** -- `59-agent-spawn-requirements-schema` references issue #59 - [x] **PR body follows template** -- Summary, Changes, Test Plan, Related sections all present - [x] **Related references plan slug** -- `plan-2026-02-26-tf-modularize-postgres` (Phase 7f-2) referenced - [x] **No secrets committed** -- No .env files, credentials, or sensitive data - [x] **No scope creep** -- 2 files changed, both directly related to the schema refactor - [x] **Commit messages** -- Fix commit addresses specific review feedback - [x] **Test plan provided** -- 5 manual test cases covering pass, deny, and unknown type scenarios ### VERDICT: APPROVED All three previous findings (1 blocker, 2 relevant nits) have been addressed: - Blocker: QA pattern tightened -- no longer matches bare "pr" substrings - Nit: `set -euo pipefail` added - Nit: `$comment` renamed to `description` in schema The schema-driven approach is clean. The hook is 45 lines, reads required_patterns from JSON, validates all patterns, and denies unknown types. Fail-closed on missing schema is the right default. Good to merge.