Refactor spawn gate to capability-based gating for Explore agents #90

Merged
forgejo_admin merged 1 commit from 89-refactor-spawn-gate-to-capability-based into main 2026-03-14 17:10:15 +00:00
Contributor

Summary

Removes the issue-reference requirement from Explore agents, which are read-only by architecture and cannot cause failures. The spawn gate now uses capability-based gating: agents with empty required_patterns pass through without any prompt validation.

Changes

  • schemas/agent-spawn-requirements.json -- Cleared required_patterns for Explore type (was requiring issue ref), updated description to reflect capability rationale
  • hooks/check-agent-spawn.sh -- Added explicit early-exit when required_patterns array is empty, preventing reliance on fragile empty-string loop behavior

Test Plan

  • Explore agent with no issue/plan ref: passes (verified)
  • Dev agent without issue ref: denied with clear message (verified)
  • Dev agent with issue ref: passes (verified)
  • QA agent without issue ref: denied (verified)
  • general-purpose with plan ref: passes (verified)
  • general-purpose without plan ref: denied (verified)
  • dottie with plan ref: passes via normalization (verified)

Review Checklist

  • Explore agents spawn without any issue/plan reference requirement
  • Dev agents still require issue reference
  • QA agents still require issue reference
  • general-purpose/dottie agents still require plan reference
  • Hook still outputs clear error messages for blocked agents
  • No unrelated changes
  • Plan: plan-pal-e-agency Phase 8a
  • Forgejo issue: #89

Closes #89

## Summary Removes the issue-reference requirement from Explore agents, which are read-only by architecture and cannot cause failures. The spawn gate now uses capability-based gating: agents with empty `required_patterns` pass through without any prompt validation. ## Changes - `schemas/agent-spawn-requirements.json` -- Cleared `required_patterns` for Explore type (was requiring issue ref), updated description to reflect capability rationale - `hooks/check-agent-spawn.sh` -- Added explicit early-exit when `required_patterns` array is empty, preventing reliance on fragile empty-string loop behavior ## Test Plan - Explore agent with no issue/plan ref: passes (verified) - Dev agent without issue ref: denied with clear message (verified) - Dev agent with issue ref: passes (verified) - QA agent without issue ref: denied (verified) - general-purpose with plan ref: passes (verified) - general-purpose without plan ref: denied (verified) - dottie with plan ref: passes via normalization (verified) ## Review Checklist - [x] Explore agents spawn without any issue/plan reference requirement - [x] Dev agents still require issue reference - [x] QA agents still require issue reference - [x] general-purpose/dottie agents still require plan reference - [x] Hook still outputs clear error messages for blocked agents - [x] No unrelated changes ## Related - Plan: `plan-pal-e-agency` Phase 8a - Forgejo issue: #89 Closes #89
Explore agents are read-only by architecture and cannot cause failures,
so gating them on issue references blocks discovery flow without reducing
change failure rate. Empty required_patterns now means pass-through.

Closes #89

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

PR #90 Review

BLOCKERS

None.

NITS

  1. Line 43 uses -r unnecessarily with length -- jq -r (raw output) is not needed when the output is a numeric value from length. It works fine, but jq --arg t "$SUBAGENT_TYPE" '.types[$t].required_patterns | length' without -r would be more precise. Non-blocking.

  2. Comment on line 2 still says "no issue, no agent" axiom -- The hook header comment (# PreToolUse hook: enforce "no issue, no agent" axiom via schema) is now slightly misleading since the hook explicitly allows agents with no issue reference (Explore). The axiom has shifted to "no issue, no agent -- unless capability-based pass-through." This is a minor doc drift. Non-blocking.

SOP COMPLIANCE

  • Branch named after issue (89-refactor-spawn-gate-to-capability-based references #89)
  • PR body has Summary, Changes, Test Plan, Related sections
  • Related references plan slug (plan-pal-e-agency Phase 8a)
  • Closes #89 present in PR body
  • No secrets, .env files, or credentials committed
  • No scope creep -- exactly 2 files changed, both directly related to the issue
  • Commit messages are descriptive (PR title is clear)

CODE CORRECTNESS

Traced all six agent-type scenarios through the modified hook:

Scenario Expected Actual Result
Explore, no issue ref PASS early-exit at line 46 Correct
dev, no issue ref DENY grep fails at line 53 Correct
dev, with issue ref PASS grep matches, exit 0 at line 60 Correct
QA, no issue ref DENY first pattern fails at line 53 Correct
general-purpose, no plan ref DENY grep fails at line 53 Correct
dottie, with plan ref PASS normalized to general-purpose, grep matches Correct

The early-exit approach is correct and preferable to relying on the fragile behavior where jq ... '.required_patterns[]' on an empty array produces empty output and grep -qE "" happens to match any non-empty string. Making the empty-patterns case explicit is the right defensive choice.

Schema change is minimal and correct: only Explore's required_patterns changed from ["#[0-9]+|[Ii]ssue|Forgejo issue"] to [], and description updated to reflect the rationale. All other agent types (dev, qa, general-purpose) retain their gates unchanged.

VERDICT: APPROVED

## PR #90 Review ### BLOCKERS None. ### NITS 1. **Line 43 uses `-r` unnecessarily with `length`** -- `jq -r` (raw output) is not needed when the output is a numeric value from `length`. It works fine, but `jq --arg t "$SUBAGENT_TYPE" '.types[$t].required_patterns | length'` without `-r` would be more precise. Non-blocking. 2. **Comment on line 2 still says "no issue, no agent" axiom** -- The hook header comment (`# PreToolUse hook: enforce "no issue, no agent" axiom via schema`) is now slightly misleading since the hook explicitly allows agents with no issue reference (Explore). The axiom has shifted to "no issue, no agent -- unless capability-based pass-through." This is a minor doc drift. Non-blocking. ### SOP COMPLIANCE - [x] Branch named after issue (`89-refactor-spawn-gate-to-capability-based` references #89) - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related references plan slug (`plan-pal-e-agency` Phase 8a) - [x] `Closes #89` present in PR body - [x] No secrets, .env files, or credentials committed - [x] No scope creep -- exactly 2 files changed, both directly related to the issue - [x] Commit messages are descriptive (PR title is clear) ### CODE CORRECTNESS Traced all six agent-type scenarios through the modified hook: | Scenario | Expected | Actual | Result | |----------|----------|--------|--------| | Explore, no issue ref | PASS | early-exit at line 46 | Correct | | dev, no issue ref | DENY | grep fails at line 53 | Correct | | dev, with issue ref | PASS | grep matches, exit 0 at line 60 | Correct | | QA, no issue ref | DENY | first pattern fails at line 53 | Correct | | general-purpose, no plan ref | DENY | grep fails at line 53 | Correct | | dottie, with plan ref | PASS | normalized to general-purpose, grep matches | Correct | The early-exit approach is correct and preferable to relying on the fragile behavior where `jq ... '.required_patterns[]'` on an empty array produces empty output and `grep -qE ""` happens to match any non-empty string. Making the empty-patterns case explicit is the right defensive choice. Schema change is minimal and correct: only Explore's `required_patterns` changed from `["#[0-9]+|[Ii]ssue|Forgejo issue"]` to `[]`, and description updated to reflect the rationale. All other agent types (dev, qa, general-purpose) retain their gates unchanged. ### VERDICT: APPROVED
forgejo_admin deleted branch 89-refactor-spawn-gate-to-capability-based 2026-03-14 17:10:15 +00:00
Sign in to join this conversation.
No description provided.