feat: add Penny agent config and worktree isolation instruction #119

Merged
forgejo_admin merged 2 commits from 118-phase-16-agent-model-completion-penny-co into main 2026-03-17 03:23:35 +00:00
Contributor

Summary

  • Add agents/penny.md to complete the 5-agent model (betty-sue, dev, qa, dottie, penny)
  • Add worktree isolation section to CLAUDE.md preventing agent artifacts from polluting the main checkout

Changes

  • agents/penny.md: New agent config for Penny (communications and scheduling agent). Frontmatter matches Dottie's format exactly. Only includes currently available mcpServers (pal-e-docs, notion). Future servers (gmail-mcp, gcal-mcp, linkedin-mcp-scheduler) documented but not listed in frontmatter.
  • CLAUDE.md: Added "Worktree Isolation" section instructing dev agents to clone to /tmp/claude-custom-{branch} instead of working in ~/claude-custom directly.

Test Plan

  • Verify all 5 agent configs exist: ls agents/*.md | grep -v deprecated shows betty-sue.md, dev.md, dottie.md, penny.md, qa.md
  • Verify penny.md frontmatter parses correctly (name, description, disallowedTools, mcpServers, model)
  • Verify CLAUDE.md worktree isolation section is placed before Platform Detection
  • Spawn Penny agent and confirm it loads with correct tool restrictions

Review Checklist

  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive
## Summary - Add `agents/penny.md` to complete the 5-agent model (betty-sue, dev, qa, dottie, penny) - Add worktree isolation section to `CLAUDE.md` preventing agent artifacts from polluting the main checkout ## Changes - `agents/penny.md`: New agent config for Penny (communications and scheduling agent). Frontmatter matches Dottie's format exactly. Only includes currently available mcpServers (pal-e-docs, notion). Future servers (gmail-mcp, gcal-mcp, linkedin-mcp-scheduler) documented but not listed in frontmatter. - `CLAUDE.md`: Added "Worktree Isolation" section instructing dev agents to clone to `/tmp/claude-custom-{branch}` instead of working in `~/claude-custom` directly. ## Test Plan - [ ] Verify all 5 agent configs exist: `ls agents/*.md | grep -v deprecated` shows betty-sue.md, dev.md, dottie.md, penny.md, qa.md - [ ] Verify penny.md frontmatter parses correctly (name, description, disallowedTools, mcpServers, model) - [ ] Verify CLAUDE.md worktree isolation section is placed before Platform Detection - [ ] Spawn Penny agent and confirm it loads with correct tool restrictions ## Review Checklist - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related - Closes forgejo_admin/claude-custom#118 - `plan-pal-e-agency` — Phase 16: Agent Model Completion
Complete the 5-agent model by adding agents/penny.md (communications
and scheduling agent) and add worktree isolation section to CLAUDE.md
to prevent agent artifacts from polluting the main checkout.

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

QA Review — PR #119

Checklist

  • Frontmatter matches specname, description, disallowedTools, mcpServers, model all match todo-penny-claude-config exactly
  • Only deployed mcpServers listedpal-e-docs and notion in frontmatter; gmail-mcp, gcal-mcp, linkedin-mcp-scheduler correctly deferred to "Future MCP Servers" table
  • disallowedTools complete — Write, Edit, Bash, NotebookEdit + all Forgejo mutation tools blocked
  • Body follows Dottie format — heading, source-of-truth callout, Role, Access table, MCP Tools table, Code Tools, Constraints, Output sections all present
  • Access table correct — pal-e-docs read-only, Notion full, Forgejo none, codebase none
  • Constraints match agent-penny note — approval required, no docs writes, no code, no Forgejo, log actions, read draft first
  • CLAUDE.md worktree isolation — clean section, correctly placed before Platform Detection, references /tmp/claude-custom-{branch} pattern
  • All 5 agents verified — betty-sue.md, dev.md, qa.md, dottie.md, penny.md (confirmed via ls agents/*.md | grep -v deprecated)
  • No secrets committed
  • No unnecessary file changes — exactly 2 files, 75 additions, 0 deletions

Findings

No issues found. Implementation matches the spec from todo-penny-claude-config and agent-penny precisely.

VERDICT: APPROVE

## QA Review — PR #119 ### Checklist - [x] **Frontmatter matches spec** — `name`, `description`, `disallowedTools`, `mcpServers`, `model` all match `todo-penny-claude-config` exactly - [x] **Only deployed mcpServers listed** — `pal-e-docs` and `notion` in frontmatter; gmail-mcp, gcal-mcp, linkedin-mcp-scheduler correctly deferred to "Future MCP Servers" table - [x] **disallowedTools complete** — Write, Edit, Bash, NotebookEdit + all Forgejo mutation tools blocked - [x] **Body follows Dottie format** — heading, source-of-truth callout, Role, Access table, MCP Tools table, Code Tools, Constraints, Output sections all present - [x] **Access table correct** — pal-e-docs read-only, Notion full, Forgejo none, codebase none - [x] **Constraints match `agent-penny` note** — approval required, no docs writes, no code, no Forgejo, log actions, read draft first - [x] **CLAUDE.md worktree isolation** — clean section, correctly placed before Platform Detection, references `/tmp/claude-custom-{branch}` pattern - [x] **All 5 agents verified** — betty-sue.md, dev.md, qa.md, dottie.md, penny.md (confirmed via `ls agents/*.md | grep -v deprecated`) - [x] **No secrets committed** - [x] **No unnecessary file changes** — exactly 2 files, 75 additions, 0 deletions ### Findings No issues found. Implementation matches the spec from `todo-penny-claude-config` and `agent-penny` precisely. **VERDICT: APPROVE**
Author
Contributor

PR #119 Review

DOMAIN REVIEW

Tech stack: Claude Code agent configuration (YAML frontmatter + Markdown)

This PR adds a new agent config (agents/penny.md) and a worktree isolation section to CLAUDE.md. Domain expertise: Claude Code agent architecture, specifically frontmatter schema (name, description, disallowedTools, mcpServers, model, hooks), tool access control patterns, and defense-in-depth enforcement.

Finding 1: pal-e-docs access control mismatch (agents/penny.md)

The Access table (line 24) states pal-e-docs is "Read-only" and the MCP Tools table only lists read tools (get_note, list_notes, search_notes, get_board, list_board_items). However, mcpServers includes pal-e-docs which exposes ALL pal-e-docs tools at runtime (including create_note, update_note, update_block, create_board, update_board_item, etc.). None of these write tools appear in disallowedTools.

Compare to Dottie, who intentionally has full read/write access to pal-e-docs -- her Access table says "Full read/write" and her MCP Tools lists mcp__pal-e-docs__* (all). The documentation and config are aligned.

For Penny, either:

  • Add all pal-e-docs write tools to disallowedTools to enforce read-only, OR
  • Change the Access table to reflect full read/write if that is the intent

This is a permission escalation gap -- the stated policy (read-only) does not match the enforced config (full access).

Finding 2: Missing PreToolUse hooks for defense-in-depth (agents/penny.md)

Both Dottie and QA have hooks: blocks in their frontmatter that enforce tool restrictions via shell scripts as a second layer beyond disallowedTools:

  • Dottie: block-dottie-code-writes.sh (catches edge cases where Bash might slip through)
  • QA: block-write-tools.sh (hard enforcement for Write/Edit/Bash)

Penny's config relies solely on disallowedTools with no hooks enforcement. Given that Penny's constraints explicitly say "No Write, Edit, Bash, or NotebookEdit" and she should never modify repos, a hooks block is needed for parity with the existing security pattern.

Finding 3: Worktree isolation uses git clone instead of git worktree add (CLAUDE.md)

The new Worktree Isolation section instructs:

git clone ~/claude-custom /tmp/claude-custom-{branch}

This creates a full independent clone rather than a git worktree. While functional, this has different semantics from the isolation: worktree setting in dev.md, which uses Claude Code's built-in worktree mechanism (git worktree add). A clone does not share the same git object store, so changes in the main checkout are not visible without a fetch/pull. This may be intentional for claude-custom specifically (full isolation from the config repo), but the section title "Worktree Isolation" is misleading since it is actually "Clone Isolation."

BLOCKERS

1. pal-e-docs write access not enforced as read-only

Penny's stated access is "Read-only" to pal-e-docs, but the configuration grants full read/write access through the pal-e-docs mcpServer without any write tools in disallowedTools. This is a DRY violation in an auth/security path -- the documented policy diverges from the enforced policy. An agent running as Penny could modify notes, boards, and blocks in pal-e-docs despite the documented constraint saying otherwise.

Fix: Add pal-e-docs write tools to disallowedTools:
mcp__pal-e-docs__create_note, mcp__pal-e-docs__create_note_from_template, mcp__pal-e-docs__update_note, mcp__pal-e-docs__delete_note, mcp__pal-e-docs__create_block, mcp__pal-e-docs__update_block, mcp__pal-e-docs__delete_block, mcp__pal-e-docs__create_board, mcp__pal-e-docs__update_board, mcp__pal-e-docs__delete_board, mcp__pal-e-docs__create_board_item, mcp__pal-e-docs__update_board_item, mcp__pal-e-docs__remove_board_item, mcp__pal-e-docs__bulk_move_board_items, mcp__pal-e-docs__sync_board, mcp__pal-e-docs__create_project, mcp__pal-e-docs__delete_project, mcp__pal-e-docs__update_note_links, mcp__pal-e-docs__create_repo, mcp__pal-e-docs__update_repo

2. Missing PreToolUse hooks -- security defense-in-depth gap

Both Dottie and QA use PreToolUse hooks as a second enforcement layer. Penny has the same access restrictions but no hooks. This is a divergence in the auth/security pattern across agents -- the existing convention is that disallowedTools + hooks together form the enforcement pair. Penny should have a hooks block matching the pattern, either reusing block-write-tools.sh (like QA) or a Penny-specific hook.

NITS

  1. "Worktree Isolation" heading is slightly misleading -- The technique is git clone, not git worktree add. Consider renaming to "Clone Isolation" or "Dev Agent Isolation" to avoid confusion with Claude Code's built-in isolation: worktree feature. Alternatively, if this should use actual worktrees, the commands would be git worktree add /tmp/claude-custom-{branch} {branch}.

  2. Redundant forgejo disallowedTools -- Since forgejo is not in Penny's mcpServers, the forgejo-related entries in disallowedTools will never match an available tool. They are harmless (defense-in-depth) but worth noting -- they only matter if someone later adds forgejo to mcpServers.

  3. betty-sue.md Related section is stale -- Not introduced by this PR, but the Related section of agents/betty-sue.md still references the old 3+3 agent model (agent-dev-frontend, agent-dev-backend, agent-devops, agent-frontend-qa, agent-dev-qa, agent-devops-qa) which was consolidated in PR #108. After this PR merges, the 5-agent model (betty-sue, dev, qa, dottie, penny) is complete -- updating betty-sue.md's Related section would be a good follow-up.

SOP COMPLIANCE

  • Branch named after issue: 118-phase-16-agent-model-completion-penny-co references #118
  • PR body has: Summary, Changes, Test Plan, Related
  • Related references plan slug: plan-pal-e-agency Phase 16
  • No secrets committed
  • No unnecessary file changes (2 files, both in scope)
  • Commit messages are descriptive

Note: PR body uses ## Review Checklist instead of ## Review but contains the same information. The template section naming is slightly non-standard but the content is complete.

PROCESS OBSERVATIONS

  • Change failure risk: MEDIUM -- The pal-e-docs write access gap could lead to unintended note modifications if Penny is spawned before fix. Since Penny is not yet deployed (MCP servers for gmail/gcal/linkedin are marked NOT DEPLOYED), the blast radius is limited to pal-e-docs writes.
  • Documentation quality: GOOD -- The Future MCP Servers table clearly marks undeployed capabilities. The constraints section is thorough.
  • 5-agent model completion -- With this PR, all 5 agents exist. The consistency of enforcements across agents is the remaining gap.

VERDICT: NOT APPROVED

Two blockers must be addressed before merge:

  1. pal-e-docs write tools must be added to disallowedTools to enforce the documented read-only access
  2. PreToolUse hooks must be added for defense-in-depth parity with Dottie and QA agents
## PR #119 Review ### DOMAIN REVIEW **Tech stack: Claude Code agent configuration (YAML frontmatter + Markdown)** This PR adds a new agent config (`agents/penny.md`) and a worktree isolation section to `CLAUDE.md`. Domain expertise: Claude Code agent architecture, specifically frontmatter schema (name, description, disallowedTools, mcpServers, model, hooks), tool access control patterns, and defense-in-depth enforcement. **Finding 1: pal-e-docs access control mismatch (agents/penny.md)** The Access table (line 24) states pal-e-docs is **"Read-only"** and the MCP Tools table only lists read tools (`get_note`, `list_notes`, `search_notes`, `get_board`, `list_board_items`). However, `mcpServers` includes `pal-e-docs` which exposes ALL pal-e-docs tools at runtime (including `create_note`, `update_note`, `update_block`, `create_board`, `update_board_item`, etc.). None of these write tools appear in `disallowedTools`. Compare to Dottie, who intentionally has full read/write access to pal-e-docs -- her Access table says "Full read/write" and her MCP Tools lists `mcp__pal-e-docs__* (all)`. The documentation and config are aligned. For Penny, either: - Add all pal-e-docs write tools to `disallowedTools` to enforce read-only, OR - Change the Access table to reflect full read/write if that is the intent This is a permission escalation gap -- the stated policy (read-only) does not match the enforced config (full access). **Finding 2: Missing PreToolUse hooks for defense-in-depth (agents/penny.md)** Both Dottie and QA have `hooks:` blocks in their frontmatter that enforce tool restrictions via shell scripts as a second layer beyond `disallowedTools`: - Dottie: `block-dottie-code-writes.sh` (catches edge cases where Bash might slip through) - QA: `block-write-tools.sh` (hard enforcement for Write/Edit/Bash) Penny's config relies solely on `disallowedTools` with no hooks enforcement. Given that Penny's constraints explicitly say "No Write, Edit, Bash, or NotebookEdit" and she should never modify repos, a hooks block is needed for parity with the existing security pattern. **Finding 3: Worktree isolation uses `git clone` instead of `git worktree add` (CLAUDE.md)** The new Worktree Isolation section instructs: ``` git clone ~/claude-custom /tmp/claude-custom-{branch} ``` This creates a full independent clone rather than a git worktree. While functional, this has different semantics from the `isolation: worktree` setting in `dev.md`, which uses Claude Code's built-in worktree mechanism (`git worktree add`). A clone does not share the same git object store, so changes in the main checkout are not visible without a fetch/pull. This may be intentional for claude-custom specifically (full isolation from the config repo), but the section title "Worktree Isolation" is misleading since it is actually "Clone Isolation." ### BLOCKERS **1. pal-e-docs write access not enforced as read-only** Penny's stated access is "Read-only" to pal-e-docs, but the configuration grants full read/write access through the `pal-e-docs` mcpServer without any write tools in `disallowedTools`. This is a DRY violation in an auth/security path -- the documented policy diverges from the enforced policy. An agent running as Penny could modify notes, boards, and blocks in pal-e-docs despite the documented constraint saying otherwise. Fix: Add pal-e-docs write tools to `disallowedTools`: `mcp__pal-e-docs__create_note, mcp__pal-e-docs__create_note_from_template, mcp__pal-e-docs__update_note, mcp__pal-e-docs__delete_note, mcp__pal-e-docs__create_block, mcp__pal-e-docs__update_block, mcp__pal-e-docs__delete_block, mcp__pal-e-docs__create_board, mcp__pal-e-docs__update_board, mcp__pal-e-docs__delete_board, mcp__pal-e-docs__create_board_item, mcp__pal-e-docs__update_board_item, mcp__pal-e-docs__remove_board_item, mcp__pal-e-docs__bulk_move_board_items, mcp__pal-e-docs__sync_board, mcp__pal-e-docs__create_project, mcp__pal-e-docs__delete_project, mcp__pal-e-docs__update_note_links, mcp__pal-e-docs__create_repo, mcp__pal-e-docs__update_repo` **2. Missing PreToolUse hooks -- security defense-in-depth gap** Both Dottie and QA use PreToolUse hooks as a second enforcement layer. Penny has the same access restrictions but no hooks. This is a divergence in the auth/security pattern across agents -- the existing convention is that `disallowedTools` + hooks together form the enforcement pair. Penny should have a hooks block matching the pattern, either reusing `block-write-tools.sh` (like QA) or a Penny-specific hook. ### NITS 1. **"Worktree Isolation" heading is slightly misleading** -- The technique is `git clone`, not `git worktree add`. Consider renaming to "Clone Isolation" or "Dev Agent Isolation" to avoid confusion with Claude Code's built-in `isolation: worktree` feature. Alternatively, if this should use actual worktrees, the commands would be `git worktree add /tmp/claude-custom-{branch} {branch}`. 2. **Redundant forgejo disallowedTools** -- Since `forgejo` is not in Penny's `mcpServers`, the forgejo-related entries in `disallowedTools` will never match an available tool. They are harmless (defense-in-depth) but worth noting -- they only matter if someone later adds `forgejo` to `mcpServers`. 3. **betty-sue.md Related section is stale** -- Not introduced by this PR, but the Related section of `agents/betty-sue.md` still references the old 3+3 agent model (`agent-dev-frontend`, `agent-dev-backend`, `agent-devops`, `agent-frontend-qa`, `agent-dev-qa`, `agent-devops-qa`) which was consolidated in PR #108. After this PR merges, the 5-agent model (betty-sue, dev, qa, dottie, penny) is complete -- updating betty-sue.md's Related section would be a good follow-up. ### SOP COMPLIANCE - [x] Branch named after issue: `118-phase-16-agent-model-completion-penny-co` references #118 - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Related references plan slug: `plan-pal-e-agency` Phase 16 - [x] No secrets committed - [x] No unnecessary file changes (2 files, both in scope) - [x] Commit messages are descriptive Note: PR body uses `## Review Checklist` instead of `## Review` but contains the same information. The template section naming is slightly non-standard but the content is complete. ### PROCESS OBSERVATIONS - **Change failure risk: MEDIUM** -- The pal-e-docs write access gap could lead to unintended note modifications if Penny is spawned before fix. Since Penny is not yet deployed (MCP servers for gmail/gcal/linkedin are marked NOT DEPLOYED), the blast radius is limited to pal-e-docs writes. - **Documentation quality: GOOD** -- The Future MCP Servers table clearly marks undeployed capabilities. The constraints section is thorough. - **5-agent model completion** -- With this PR, all 5 agents exist. The consistency of enforcements across agents is the remaining gap. ### VERDICT: NOT APPROVED Two blockers must be addressed before merge: 1. pal-e-docs write tools must be added to `disallowedTools` to enforce the documented read-only access 2. PreToolUse hooks must be added for defense-in-depth parity with Dottie and QA agents
Add all 20 pal-e-docs write tools to disallowedTools and add PreToolUse
hooks as defense-in-depth. The Access table declares read-only but the
frontmatter previously allowed write tools through.

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

QA Blocker Fixes (97dce67)

Both blockers from the QA review have been resolved:

Blocker 1: pal-e-docs write tools now in disallowedTools

Penny's Access table declares read-only for pal-e-docs, but the frontmatter previously only blocked Forgejo write tools — all 20 pal-e-docs write tools were allowed through. Fixed by adding every write tool to disallowedTools:

mcp__pal-e-docs__create_note, update_note, delete_note, create_block, update_block, delete_block, create_board, update_board, delete_board, create_board_item, update_board_item, remove_board_item, bulk_move_board_items, sync_board, create_project, delete_project, create_repo, update_repo, update_note_links, create_note_from_template

Blocker 2: PreToolUse hooks added as defense-in-depth

Added hooks section to penny.md frontmatter matching the pattern from dottie.md and qa.md. The matcher catches Write|Edit|NotebookEdit|Bash plus all pal-e-docs write tool prefixes (create_, update_, delete_, remove_, bulk_, sync_).

Created hooks/block-penny-writes.sh following the same structure as block-dottie-code-writes.sh — blocks file writes, Bash, and pal-e-docs write operations with clear error messages identifying Penny as a communications agent with read-only access.

## QA Blocker Fixes (97dce67) Both blockers from the QA review have been resolved: ### Blocker 1: pal-e-docs write tools now in disallowedTools Penny's Access table declares **read-only** for pal-e-docs, but the frontmatter previously only blocked Forgejo write tools — all 20 pal-e-docs write tools were allowed through. Fixed by adding every write tool to `disallowedTools`: `mcp__pal-e-docs__create_note`, `update_note`, `delete_note`, `create_block`, `update_block`, `delete_block`, `create_board`, `update_board`, `delete_board`, `create_board_item`, `update_board_item`, `remove_board_item`, `bulk_move_board_items`, `sync_board`, `create_project`, `delete_project`, `create_repo`, `update_repo`, `update_note_links`, `create_note_from_template` ### Blocker 2: PreToolUse hooks added as defense-in-depth Added `hooks` section to penny.md frontmatter matching the pattern from dottie.md and qa.md. The matcher catches `Write|Edit|NotebookEdit|Bash` plus all pal-e-docs write tool prefixes (`create_`, `update_`, `delete_`, `remove_`, `bulk_`, `sync_`). Created `hooks/block-penny-writes.sh` following the same structure as `block-dottie-code-writes.sh` — blocks file writes, Bash, and pal-e-docs write operations with clear error messages identifying Penny as a communications agent with read-only access.
Author
Contributor

PR #119 Re-Review

DOMAIN REVIEW

Tech stack: Claude Code agent configuration (markdown frontmatter + bash hooks). This is a config/IaC-adjacent domain -- correctness of tool restrictions and defense-in-depth enforcement are the primary concerns.

Files reviewed (full diff, 3 files, +114/-0):

  • agents/penny.md -- new agent config (71 lines)
  • hooks/block-penny-writes.sh -- PreToolUse defense-in-depth hook (33 lines)
  • CLAUDE.md -- Worktree Isolation section addition (10 lines)

Agent config analysis (agents/penny.md):

  1. Frontmatter format matches the established pattern from dottie.md exactly: name, description, disallowedTools, mcpServers, model, hooks. Good.

  2. disallowedTools completeness -- Cross-checked against the full set of 20 pal-e-docs write tools available in the MCP server. All 20 are explicitly listed:

    • create_note, create_note_from_template, create_block, create_board, create_board_item, create_project, create_repo
    • update_note, update_block, update_board, update_board_item, update_repo, update_note_links
    • delete_note, delete_block, delete_board, delete_project
    • remove_board_item, bulk_move_board_items, sync_board

    All Forgejo write tools are also blocked. Complete coverage. No gaps.

  3. mcpServers lists only pal-e-docs and notion -- correct per the agent's role. Future servers (gmail-mcp, gcal-mcp, linkedin-mcp-scheduler) are documented in the body but NOT listed in frontmatter. This is the right approach -- document intent, only enable what exists.

  4. Hook wiring -- The PreToolUse matcher regex covers all write tool prefixes: Write|Edit|NotebookEdit|Bash|mcp__pal-e-docs__create_|mcp__pal-e-docs__update_|mcp__pal-e-docs__delete_|mcp__pal-e-docs__remove_|mcp__pal-e-docs__bulk_|mcp__pal-e-docs__sync_. This correctly matches the disallowedTools list as defense-in-depth.

Hook script analysis (hooks/block-penny-writes.sh):

  1. Script follows the established pattern from block-dottie-code-writes.sh and block-write-tools.sh.
  2. Uses set -euo pipefail -- correct.
  3. Reads stdin JSON, extracts tool_name via jq -- correct pattern.
  4. Case statement covers Write|Edit|NotebookEdit, Bash, and mcp__pal-e-docs__create_*|update_*|delete_*|remove_*|bulk_*|sync_* -- all write paths blocked.
  5. Exit codes: exit 2 with stderr message for blocks, exit 0 for allow. Correct protocol.
  6. File is chmod +x (mode 100755 in the diff). Good.
  7. Error messages are agent-specific and descriptive ("Penny cannot write files", "Penny has read-only access to pal-e-docs"). Clean.

CLAUDE.md Worktree Isolation section:

  1. Placed correctly before Platform Detection (between personality intro and Platform Detection).
  2. Instructions are clear: clone to /tmp/claude-custom-{branch}, checkout branch.
  3. Purpose statement is accurate -- prevents agent artifacts from polluting main checkout.

BLOCKERS

None. Both previously identified blockers have been resolved:

  1. disallowedTools completeness -- All 20 pal-e-docs write tools are now explicitly listed in the disallowedTools frontmatter. No gaps remain.

  2. Defense-in-depth hook -- block-penny-writes.sh now exists as a proper PreToolUse hook with correct case matching for all write paths, following the established pattern from existing hooks (block-dottie-code-writes.sh, block-write-tools.sh).

NITS

  1. SubagentStart matcher missing penny -- settings.json line 232 has the SubagentStart matcher for inject-subagent-context.sh but does not include penny in the pipe-delimited list. When Penny is spawned as a subagent, she will not receive the injected context. This is outside the scope of this PR (settings.json is not modified), but should be addressed as a follow-up. File: /home/ldraney/claude-custom/settings.json, line 232.

  2. Stale Related section in betty-sue.md -- The betty-sue agent config references deprecated agent names (agent-dev-frontend, agent-dev-backend, agent-devops, agent-frontend-qa, agent-dev-qa, agent-devops-qa) in its Related section. Penny is not referenced. Not in scope for this PR, but worth a follow-up cleanup.

  3. Minor: PR body uses "Review Checklist" header -- The SOP template calls for ## Related as the final section. The PR body has both ## Review Checklist and ## Related, which is fine but the checklist is non-standard. Very minor.

SOP COMPLIANCE

  • Branch named after issue: 118-phase-16-agent-model-completion-penny-co references issue #118
  • PR body has: Summary, Changes, Test Plan, Related -- all present
  • Related references plan slug: plan-pal-e-agency -- Phase 16: Agent Model Completion
  • No secrets committed -- confirmed, no credentials or env files in diff
  • No unnecessary file changes -- all 3 files directly serve the issue scope (penny config, penny hook, worktree isolation instruction)
  • Commit messages are descriptive (from PR title: "feat: add Penny agent config and worktree isolation instruction")

PROCESS OBSERVATIONS

  • Deployment frequency: Config-only change, zero runtime risk. Can merge immediately.
  • Change failure risk: Low. Agent configs are isolated -- a bad penny.md only affects Penny agent spawns, not other agents or the platform.
  • Follow-up needed: The SubagentStart matcher in settings.json should be updated to include penny before first operational use. Without it, Penny will spawn but without injected context. Recommend creating a follow-up issue.
  • 5-agent model: With this PR, all 5 agents are present: betty-sue, dev, qa, dottie, penny. The agent model is complete per the plan.

VERDICT: APPROVED

## PR #119 Re-Review ### DOMAIN REVIEW **Tech stack:** Claude Code agent configuration (markdown frontmatter + bash hooks). This is a config/IaC-adjacent domain -- correctness of tool restrictions and defense-in-depth enforcement are the primary concerns. **Files reviewed (full diff, 3 files, +114/-0):** - `agents/penny.md` -- new agent config (71 lines) - `hooks/block-penny-writes.sh` -- PreToolUse defense-in-depth hook (33 lines) - `CLAUDE.md` -- Worktree Isolation section addition (10 lines) **Agent config analysis (`agents/penny.md`):** 1. **Frontmatter format** matches the established pattern from `dottie.md` exactly: `name`, `description`, `disallowedTools`, `mcpServers`, `model`, `hooks`. Good. 2. **disallowedTools completeness** -- Cross-checked against the full set of 20 pal-e-docs write tools available in the MCP server. All 20 are explicitly listed: - `create_note`, `create_note_from_template`, `create_block`, `create_board`, `create_board_item`, `create_project`, `create_repo` - `update_note`, `update_block`, `update_board`, `update_board_item`, `update_repo`, `update_note_links` - `delete_note`, `delete_block`, `delete_board`, `delete_project` - `remove_board_item`, `bulk_move_board_items`, `sync_board` All Forgejo write tools are also blocked. Complete coverage. No gaps. 3. **mcpServers** lists only `pal-e-docs` and `notion` -- correct per the agent's role. Future servers (gmail-mcp, gcal-mcp, linkedin-mcp-scheduler) are documented in the body but NOT listed in frontmatter. This is the right approach -- document intent, only enable what exists. 4. **Hook wiring** -- The PreToolUse matcher regex covers all write tool prefixes: `Write|Edit|NotebookEdit|Bash|mcp__pal-e-docs__create_|mcp__pal-e-docs__update_|mcp__pal-e-docs__delete_|mcp__pal-e-docs__remove_|mcp__pal-e-docs__bulk_|mcp__pal-e-docs__sync_`. This correctly matches the disallowedTools list as defense-in-depth. **Hook script analysis (`hooks/block-penny-writes.sh`):** 1. Script follows the established pattern from `block-dottie-code-writes.sh` and `block-write-tools.sh`. 2. Uses `set -euo pipefail` -- correct. 3. Reads stdin JSON, extracts `tool_name` via `jq` -- correct pattern. 4. Case statement covers `Write|Edit|NotebookEdit`, `Bash`, and `mcp__pal-e-docs__create_*|update_*|delete_*|remove_*|bulk_*|sync_*` -- all write paths blocked. 5. Exit codes: `exit 2` with stderr message for blocks, `exit 0` for allow. Correct protocol. 6. File is `chmod +x` (mode 100755 in the diff). Good. 7. Error messages are agent-specific and descriptive ("Penny cannot write files", "Penny has read-only access to pal-e-docs"). Clean. **CLAUDE.md Worktree Isolation section:** 1. Placed correctly before Platform Detection (between personality intro and Platform Detection). 2. Instructions are clear: clone to `/tmp/claude-custom-{branch}`, checkout branch. 3. Purpose statement is accurate -- prevents agent artifacts from polluting main checkout. ### BLOCKERS None. Both previously identified blockers have been resolved: 1. **disallowedTools completeness** -- All 20 pal-e-docs write tools are now explicitly listed in the disallowedTools frontmatter. No gaps remain. 2. **Defense-in-depth hook** -- `block-penny-writes.sh` now exists as a proper PreToolUse hook with correct case matching for all write paths, following the established pattern from existing hooks (`block-dottie-code-writes.sh`, `block-write-tools.sh`). ### NITS 1. **SubagentStart matcher missing `penny`** -- `settings.json` line 232 has the SubagentStart matcher for `inject-subagent-context.sh` but does not include `penny` in the pipe-delimited list. When Penny is spawned as a subagent, she will not receive the injected context. This is outside the scope of this PR (settings.json is not modified), but should be addressed as a follow-up. File: `/home/ldraney/claude-custom/settings.json`, line 232. 2. **Stale Related section in `betty-sue.md`** -- The betty-sue agent config references deprecated agent names (`agent-dev-frontend`, `agent-dev-backend`, `agent-devops`, `agent-frontend-qa`, `agent-dev-qa`, `agent-devops-qa`) in its Related section. Penny is not referenced. Not in scope for this PR, but worth a follow-up cleanup. 3. **Minor: PR body uses "Review Checklist" header** -- The SOP template calls for `## Related` as the final section. The PR body has both `## Review Checklist` and `## Related`, which is fine but the checklist is non-standard. Very minor. ### SOP COMPLIANCE - [x] Branch named after issue: `118-phase-16-agent-model-completion-penny-co` references issue #118 - [x] PR body has: Summary, Changes, Test Plan, Related -- all present - [x] Related references plan slug: `plan-pal-e-agency` -- Phase 16: Agent Model Completion - [x] No secrets committed -- confirmed, no credentials or env files in diff - [x] No unnecessary file changes -- all 3 files directly serve the issue scope (penny config, penny hook, worktree isolation instruction) - [x] Commit messages are descriptive (from PR title: "feat: add Penny agent config and worktree isolation instruction") ### PROCESS OBSERVATIONS - **Deployment frequency**: Config-only change, zero runtime risk. Can merge immediately. - **Change failure risk**: Low. Agent configs are isolated -- a bad penny.md only affects Penny agent spawns, not other agents or the platform. - **Follow-up needed**: The SubagentStart matcher in `settings.json` should be updated to include `penny` before first operational use. Without it, Penny will spawn but without injected context. Recommend creating a follow-up issue. - **5-agent model**: With this PR, all 5 agents are present: betty-sue, dev, qa, dottie, penny. The agent model is complete per the plan. ### VERDICT: APPROVED
forgejo_admin deleted branch 118-phase-16-agent-model-completion-penny-co 2026-03-17 03:23:36 +00:00
Sign in to join this conversation.
No description provided.