feat: add Penny agent config and worktree isolation instruction #119
No reviewers
Labels
No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
ldraney/claude-custom!119
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "118-phase-16-agent-model-completion-penny-co"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
agents/penny.mdto complete the 5-agent model (betty-sue, dev, qa, dottie, penny)CLAUDE.mdpreventing agent artifacts from polluting the main checkoutChanges
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-customdirectly.Test Plan
ls agents/*.md | grep -v deprecatedshows betty-sue.md, dev.md, dottie.md, penny.md, qa.mdReview Checklist
Related
plan-pal-e-agency— Phase 16: Agent Model CompletionQA Review — PR #119
Checklist
name,description,disallowedTools,mcpServers,modelall matchtodo-penny-claude-configexactlypal-e-docsandnotionin frontmatter; gmail-mcp, gcal-mcp, linkedin-mcp-scheduler correctly deferred to "Future MCP Servers" tableagent-pennynote — approval required, no docs writes, no code, no Forgejo, log actions, read draft first/tmp/claude-custom-{branch}patternls agents/*.md | grep -v deprecated)Findings
No issues found. Implementation matches the spec from
todo-penny-claude-configandagent-pennyprecisely.VERDICT: APPROVE
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 toCLAUDE.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,mcpServersincludespal-e-docswhich exposes ALL pal-e-docs tools at runtime (includingcreate_note,update_note,update_block,create_board,update_board_item, etc.). None of these write tools appear indisallowedTools.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:
disallowedToolsto enforce read-only, ORThis 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 beyonddisallowedTools:block-dottie-code-writes.sh(catches edge cases where Bash might slip through)block-write-tools.sh(hard enforcement for Write/Edit/Bash)Penny's config relies solely on
disallowedToolswith 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 cloneinstead ofgit worktree add(CLAUDE.md)The new Worktree Isolation section instructs:
This creates a full independent clone rather than a git worktree. While functional, this has different semantics from the
isolation: worktreesetting indev.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-docsmcpServer without any write tools indisallowedTools. 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_repo2. 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 reusingblock-write-tools.sh(like QA) or a Penny-specific hook.NITS
"Worktree Isolation" heading is slightly misleading -- The technique is
git clone, notgit worktree add. Consider renaming to "Clone Isolation" or "Dev Agent Isolation" to avoid confusion with Claude Code's built-inisolation: worktreefeature. Alternatively, if this should use actual worktrees, the commands would begit worktree add /tmp/claude-custom-{branch} {branch}.Redundant forgejo disallowedTools -- Since
forgejois not in Penny'smcpServers, the forgejo-related entries indisallowedToolswill never match an available tool. They are harmless (defense-in-depth) but worth noting -- they only matter if someone later addsforgejotomcpServers.betty-sue.md Related section is stale -- Not introduced by this PR, but the Related section of
agents/betty-sue.mdstill 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
118-phase-16-agent-model-completion-penny-coreferences #118plan-pal-e-agencyPhase 16Note: PR body uses
## Review Checklistinstead of## Reviewbut contains the same information. The template section naming is slightly non-standard but the content is complete.PROCESS OBSERVATIONS
VERDICT: NOT APPROVED
Two blockers must be addressed before merge:
disallowedToolsto enforce the documented read-only accessQA 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_templateBlocker 2: PreToolUse hooks added as defense-in-depth
Added
hookssection to penny.md frontmatter matching the pattern from dottie.md and qa.md. The matcher catchesWrite|Edit|NotebookEdit|Bashplus all pal-e-docs write tool prefixes (create_,update_,delete_,remove_,bulk_,sync_).Created
hooks/block-penny-writes.shfollowing the same structure asblock-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.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):Frontmatter format matches the established pattern from
dottie.mdexactly:name,description,disallowedTools,mcpServers,model,hooks. Good.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_repoupdate_note,update_block,update_board,update_board_item,update_repo,update_note_linksdelete_note,delete_block,delete_board,delete_projectremove_board_item,bulk_move_board_items,sync_boardAll Forgejo write tools are also blocked. Complete coverage. No gaps.
mcpServers lists only
pal-e-docsandnotion-- 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.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):block-dottie-code-writes.shandblock-write-tools.sh.set -euo pipefail-- correct.tool_nameviajq-- correct pattern.Write|Edit|NotebookEdit,Bash, andmcp__pal-e-docs__create_*|update_*|delete_*|remove_*|bulk_*|sync_*-- all write paths blocked.exit 2with stderr message for blocks,exit 0for allow. Correct protocol.chmod +x(mode 100755 in the diff). Good.CLAUDE.md Worktree Isolation section:
/tmp/claude-custom-{branch}, checkout branch.BLOCKERS
None. Both previously identified blockers have been resolved:
disallowedTools completeness -- All 20 pal-e-docs write tools are now explicitly listed in the disallowedTools frontmatter. No gaps remain.
Defense-in-depth hook --
block-penny-writes.shnow 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
SubagentStart matcher missing
penny--settings.jsonline 232 has the SubagentStart matcher forinject-subagent-context.shbut does not includepennyin 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.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.Minor: PR body uses "Review Checklist" header -- The SOP template calls for
## Relatedas the final section. The PR body has both## Review Checklistand## Related, which is fine but the checklist is non-standard. Very minor.SOP COMPLIANCE
118-phase-16-agent-model-completion-penny-coreferences issue #118plan-pal-e-agency-- Phase 16: Agent Model CompletionPROCESS OBSERVATIONS
settings.jsonshould be updated to includepennybefore first operational use. Without it, Penny will spawn but without injected context. Recommend creating a follow-up issue.VERDICT: APPROVED