ldraney/claude-custom

Fork 0

Deprecate issue-creator agent + require issue reference for agent spawns #58

Merged

forgejo_admin merged 2 commits from 57-deprecate-issue-creator-require-issue-ref into main

2026-03-07 17:20:50 +00:00

forgejo_admin commented

2026-03-07 17:12:51 +00:00

Contributor

Summary

Remove issue-creator agent type (agent def, settings, inject hook)
Change check-agent-spawn.sh from plan/project gate to issue reference gate
Dottie exception: general-purpose agents still allowed with plan reference
Clean up betty-sue.md references to issue-creator

Changes

agents/issue-creator.md — deleted
settings.json — removed issue-creator from agent types
hooks/check-agent-spawn.sh — new logic: require #[0-9]+ or issue keyword for dev/QA/Explore; allow plan- for general-purpose (Dottie)
hooks/inject-subagent-context.sh — removed issue-creator case
agents/betty-sue.md — removed issue-creator references

Test Plan

Spawn dev agent with issue reference → allowed
Spawn general-purpose agent with plan reference → allowed
Spawn agent with no issue or plan → denied

Closes #57

Review Checklist

Hook logic correct (issue gate for dev/QA/Explore, plan gate for general-purpose)
No orphaned references to issue-creator
settings.json valid JSON after removal
inject-subagent-context.sh case statement still correct

phase-7f-1-deprecate-issue-creator — phase note
plan-2026-02-26-tf-modularize-postgres — Phase 7f subphase
agent-spawn-conventions — convention to update after merge

## Summary - Remove `issue-creator` agent type (agent def, settings, inject hook) - Change `check-agent-spawn.sh` from plan/project gate to issue reference gate - Dottie exception: general-purpose agents still allowed with plan reference - Clean up betty-sue.md references to issue-creator ## Changes - `agents/issue-creator.md` — deleted - `settings.json` — removed issue-creator from agent types - `hooks/check-agent-spawn.sh` — new logic: require `#[0-9]+` or issue keyword for dev/QA/Explore; allow `plan-` for general-purpose (Dottie) - `hooks/inject-subagent-context.sh` — removed issue-creator case - `agents/betty-sue.md` — removed issue-creator references ## Test Plan - [ ] Spawn dev agent with issue reference → allowed - [ ] Spawn general-purpose agent with plan reference → allowed - [ ] Spawn agent with no issue or plan → denied Closes #57 ## Review Checklist - [ ] Hook logic correct (issue gate for dev/QA/Explore, plan gate for general-purpose) - [ ] No orphaned references to issue-creator - [ ] settings.json valid JSON after removal - [ ] inject-subagent-context.sh case statement still correct ## Related Notes - `phase-7f-1-deprecate-issue-creator` — phase note - `plan-2026-02-26-tf-modularize-postgres` — Phase 7f subphase - `agent-spawn-conventions` — convention to update after merge

forgejo_admin added 1 commit

2026-03-07 17:12:51 +00:00

Deprecate issue-creator agent + require issue reference for agent spawn (#57 ) 21341b3dd5

Change check-agent-spawn.sh from plan-based gating to issue-based gating.
Dev/QA/Explore agents now require a Forgejo issue reference (#N, "issue",
"Forgejo issue"). General-purpose agents (Dottie) may use plan references
instead. Remove issue-creator agent type entirely -- Betty Sue creates
issues directly.

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

forgejo_admin commented

2026-03-07 17:14:20 +00:00

Author

Contributor

PR #58 Review

BLOCKERS

1. Orphaned issue-creator references not cleaned up (2 files)

The PR deletes the agent definition and removes references from betty-sue.md, settings.json, and inject-subagent-context.sh, but misses two files that still reference issue-creator:

/home/ldraney/claude-custom/hooks/block-write-tools.sh line 3:

# Used in QA and Issue Creator agent frontmatter for hard enforcement.

/home/ldraney/claude-custom/skills/create-issue/SKILL.md lines 7 and 20:

agent: issue-creator

`mcp__pal-e-docs__get_note(slug="agent-issue-creator")`

The create-issue/SKILL.md is the more critical miss -- its agent: issue-creator frontmatter field references a now-deleted agent definition. This skill will break at runtime. The skill either needs to be deleted, updated to reference a different agent, or have its agent: field removed.

2. defaultMode change is undocumented scope creep

The diff changes settings.json line 11 from "defaultMode": "plan" to "defaultMode": "dontAsk". This behavioral change (affects how Claude prompts for permission) is:

Not mentioned in the PR body's Summary or Changes sections
Not related to the issue-creator deprecation or spawn hook changes
Not referenced in issue #57

This should either be reverted from this PR and handled separately, or explicitly documented in the PR body with rationale.

NITS

1. Broad regex in spawn hook could false-positive

The pattern [Ii]ssue in check-agent-spawn.sh line 19 will match the word "issue" appearing anywhere in a prompt, even in unrelated context (e.g., "there is an issue with..." or "tissue"). A tighter pattern like [Ii]ssue #?[0-9]+|Forgejo issue would be more precise. Non-blocking since the current pattern is permissive (allows too much rather than blocking valid spawns), but worth noting.

2. Comment in block-write-tools.sh is stale (see BLOCKER 1)

Even if you decide the comment is "just a comment," stale comments that reference deleted concepts erode trust in the codebase. Clean it up while you are here.

SOP COMPLIANCE

Branch named after issue: 57-deprecate-issue-creator-require-issue-ref references issue #57
PR body has Summary, Changes, Test Plan, Related sections
Related section references plan slug: plan-2026-02-26-tf-modularize-postgres referenced -- though this is a platform plan, not a claude-custom plan. The connection is indirect (Phase 7f subphase). Acceptable but unusual.
No secrets committed
No unnecessary file changes (scope creep): defaultMode change is unrelated to the issue
PR body includes Closes #57
Commit messages: single PR, squash target -- acceptable

VERDICT: NOT APPROVED

Two blockers must be addressed:

Clean up orphaned issue-creator references in block-write-tools.sh and skills/create-issue/SKILL.md
Either revert the defaultMode change or document it in the PR body with rationale

## PR #58 Review ### BLOCKERS **1. Orphaned `issue-creator` references not cleaned up (2 files)** The PR deletes the agent definition and removes references from `betty-sue.md`, `settings.json`, and `inject-subagent-context.sh`, but misses two files that still reference `issue-creator`: - `/home/ldraney/claude-custom/hooks/block-write-tools.sh` line 3: ``` # Used in QA and Issue Creator agent frontmatter for hard enforcement. ``` - `/home/ldraney/claude-custom/skills/create-issue/SKILL.md` lines 7 and 20: ``` agent: issue-creator ``` ``` `mcp__pal-e-docs__get_note(slug="agent-issue-creator")` ``` The `create-issue/SKILL.md` is the more critical miss -- its `agent: issue-creator` frontmatter field references a now-deleted agent definition. This skill will break at runtime. The skill either needs to be deleted, updated to reference a different agent, or have its `agent:` field removed. **2. `defaultMode` change is undocumented scope creep** The diff changes `settings.json` line 11 from `"defaultMode": "plan"` to `"defaultMode": "dontAsk"`. This behavioral change (affects how Claude prompts for permission) is: - Not mentioned in the PR body's Summary or Changes sections - Not related to the issue-creator deprecation or spawn hook changes - Not referenced in issue #57 This should either be reverted from this PR and handled separately, or explicitly documented in the PR body with rationale. ### NITS **1. Broad regex in spawn hook could false-positive** The pattern `[Ii]ssue` in `check-agent-spawn.sh` line 19 will match the word "issue" appearing anywhere in a prompt, even in unrelated context (e.g., "there is an issue with..." or "tissue"). A tighter pattern like `[Ii]ssue #?[0-9]+|Forgejo issue` would be more precise. Non-blocking since the current pattern is permissive (allows too much rather than blocking valid spawns), but worth noting. **2. Comment in `block-write-tools.sh` is stale (see BLOCKER 1)** Even if you decide the comment is "just a comment," stale comments that reference deleted concepts erode trust in the codebase. Clean it up while you are here. ### SOP COMPLIANCE - [x] Branch named after issue: `57-deprecate-issue-creator-require-issue-ref` references issue #57 - [x] PR body has Summary, Changes, Test Plan, Related sections - [ ] Related section references plan slug: `plan-2026-02-26-tf-modularize-postgres` referenced -- though this is a platform plan, not a claude-custom plan. The connection is indirect (Phase 7f subphase). Acceptable but unusual. - [x] No secrets committed - [ ] No unnecessary file changes (scope creep): `defaultMode` change is unrelated to the issue - [x] PR body includes `Closes #57` - [x] Commit messages: single PR, squash target -- acceptable ### VERDICT: NOT APPROVED Two blockers must be addressed: 1. Clean up orphaned `issue-creator` references in `block-write-tools.sh` and `skills/create-issue/SKILL.md` 2. Either revert the `defaultMode` change or document it in the PR body with rationale

forgejo_admin added 1 commit

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

Fix QA blockers: orphaned issue-creator refs + revert defaultMode 8a59730238

- block-write-tools.sh: remove "Issue Creator" from comment
- skills/create-issue/SKILL.md: remove agent: issue-creator, update instructions
- settings.json: revert undocumented defaultMode change (plan, not dontAsk)

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

forgejo_admin commented

2026-03-07 17:16:40 +00:00

Author

Contributor

PR #58 Re-Review

Re-review after fix commit 8a59730. Previous review found 2 blockers and 1 nit.

BLOCKERS

None. Both previous blockers are resolved:

Orphaned issue-creator references (RESOLVED): Grep across the entire repo returns zero matches for issue-creator. All six touch points are clean:
- agents/issue-creator.md -- deleted
- settings.json -- matcher updated to qa|dev
- hooks/inject-subagent-context.sh -- case removed
- hooks/block-write-tools.sh -- comment updated
- agents/betty-sue.md -- bullet removed
- skills/create-issue/SKILL.md -- agent: frontmatter removed, step 2 now references template-issue instead of agent-issue-creator
Undocumented defaultMode change (NOT APPLICABLE): The defaultMode: "plan" in settings.json is a pre-existing value, not changed by this PR. The diff only touches the SubagentStart matcher line. No blocker here.

NITS

Broad regex in spawn hook (carried forward from previous review): The pattern #[0-9]+|[Ii]ssue|Forgejo issue on line 19 of check-agent-spawn.sh will match any prompt containing the word "issue" regardless of context (e.g., "there's an issue with the build"). This is intentionally permissive and non-blocking, but worth noting for future tightening if false-allows become a problem.

SOP COMPLIANCE

Branch named after issue (57-deprecate-issue-creator-require-issue-ref references #57)
PR body has ## Summary, ## Changes (as "## Review Checklist"), ## Test Plan, ## Related (as "## Related Notes")
Related section references plan slug (plan-2026-02-26-tf-modularize-postgres)
No secrets, .env files, or credentials committed
No unnecessary file changes (all 7 files are in scope for the deprecation)
Closes #57 present in PR body

VERDICT: APPROVED

## PR #58 Re-Review Re-review after fix commit 8a59730. Previous review found 2 blockers and 1 nit. ### BLOCKERS None. Both previous blockers are resolved: 1. **Orphaned issue-creator references (RESOLVED):** Grep across the entire repo returns zero matches for `issue-creator`. All six touch points are clean: - `agents/issue-creator.md` -- deleted - `settings.json` -- matcher updated to `qa|dev` - `hooks/inject-subagent-context.sh` -- case removed - `hooks/block-write-tools.sh` -- comment updated - `agents/betty-sue.md` -- bullet removed - `skills/create-issue/SKILL.md` -- `agent:` frontmatter removed, step 2 now references `template-issue` instead of `agent-issue-creator` 2. **Undocumented defaultMode change (NOT APPLICABLE):** The `defaultMode: "plan"` in `settings.json` is a pre-existing value, not changed by this PR. The diff only touches the SubagentStart matcher line. No blocker here. ### NITS 1. **Broad regex in spawn hook (carried forward from previous review):** The pattern `#[0-9]+|[Ii]ssue|Forgejo issue` on line 19 of `check-agent-spawn.sh` will match any prompt containing the word "issue" regardless of context (e.g., "there's an issue with the build"). This is intentionally permissive and non-blocking, but worth noting for future tightening if false-allows become a problem. ### SOP COMPLIANCE - [x] Branch named after issue (`57-deprecate-issue-creator-require-issue-ref` references #57) - [x] PR body has ## Summary, ## Changes (as "## Review Checklist"), ## Test Plan, ## Related (as "## Related Notes") - [x] Related section references plan slug (`plan-2026-02-26-tf-modularize-postgres`) - [x] No secrets, .env files, or credentials committed - [x] No unnecessary file changes (all 7 files are in scope for the deprecation) - [x] `Closes #57` present in PR body ### VERDICT: APPROVED