feat: add /send-email skill wrapping sop-email-send

forgejo_admin commented

2026-04-03 19:07:08 +00:00

Author

Contributor

PR #233 Review

DOMAIN REVIEW

Tech stack: Claude Code skill file (Markdown with YAML frontmatter). Domain: agent configuration / skill authoring for the claude-custom repo.

Frontmatter structure: The skill has name and description fields. The description starts with "Use when" which matches the convention visible in the system-reminder skill listing. This is correct.

Missing optional frontmatter fields:

No argument-hint -- acceptable since /send-email takes no arguments (the SOP drives the workflow).
No disable-model-invocation -- acceptable since this skill needs the model to execute MCP calls and follow the SOP interactively.
No context: fork -- acceptable; the skill does not need session isolation.

Pattern compliance: The skill follows the delegation pattern established by /board-status and /board-sync -- a thin SKILL.md that points to a pal-e-docs SOP via MCP call. The heavy content lives in sop-email-send, not inline. This is the correct architecture.

MCP call format: The skill references mcp__pal-e-docs__get_note(slug="sop-email-send") and mcp__pal-e-docs__get_note(slug="arch-email") -- correct tool names and syntax.

Safety gates: The Red Flags section is well-constructed. It covers:

No send without test email
No blast without explicit approval
No reuse of stale approval
No inline layout/query creation (file a ticket instead)
Explicit rejection of "just send to a few" workaround

This aligns with feedback_no_email_without_five_approvals.md and feedback_never_email_without_approval.md from MEMORY.md.

Word count: The file is 32 lines / approximately 160 words of content (excluding frontmatter). Well under the 200-word budget stated in the PR review checklist.

BLOCKERS

None.

This is a 32-line skill file that delegates to an SOP. The BLOCKER criteria (test coverage for new functionality, unvalidated user input, secrets, DRY violations in auth paths) do not apply to a declarative Markdown skill definition. The skill itself does not execute code -- it instructs the agent to read and follow an SOP that contains the actual logic and safety gates.

NITS

Quick Reference section could drift from SOP. The inline quick reference (layouts, test email, endpoint) duplicates information that presumably lives in sop-email-send and arch-email. If the SOP changes the endpoint or adds a layout, this skill will be stale. Consider whether the Quick Reference adds enough value to justify the maintenance burden, or whether the SOP alone is sufficient. This is a judgment call, not a blocker -- the current content is accurate and useful as a fast reminder.
Test email address is hardcoded. draneylucas@gmail.com is hardcoded as the test email. If this ever changes, it needs updating in both the SOP and this skill. Not a secret (it is a personal email used for testing, not a credential), but it is a hardcoded value that could drift.
No step for loading the SOP references the basketball-api context. The description says "through basketball-api" but the instructions do not explicitly tell the agent to check basketball-api connection/availability. The SOP likely handles this, so this is informational only.

SOP COMPLIANCE

Branch named after issue (232-add-send-email-skill-wrapping-sop-email references #232)
PR body follows template (Summary, Changes, Test Plan, Review Checklist, Related Notes)
Related section references SOPs and architecture notes
No secrets committed (test email is not a secret)
No unnecessary file changes (single file, exactly on scope)
Commit message is descriptive (feat: add /send-email skill wrapping sop-email-send)
Closes #232 present in PR body

PROCESS OBSERVATIONS

Clean, minimal PR. The skill correctly follows the thin-wrapper pattern: SKILL.md is the trigger surface, pal-e-docs SOPs hold the logic. This keeps the claude-custom repo lightweight and ensures SOP updates propagate without needing skill file changes.

The test plan includes real-world validation (contract reminder email via basketball-api tickets #750-752), which is appropriate for a skill that cannot be unit-tested in isolation.

Deployment frequency: no risk. This is additive (new file only, no modifications to existing skills or hooks). Change failure risk: negligible.

VERDICT: APPROVED

## PR #233 Review ### DOMAIN REVIEW **Tech stack:** Claude Code skill file (Markdown with YAML frontmatter). Domain: agent configuration / skill authoring for the claude-custom repo. **Frontmatter structure:** The skill has `name` and `description` fields. The `description` starts with "Use when" which matches the convention visible in the system-reminder skill listing. This is correct. **Missing optional frontmatter fields:** - No `argument-hint` -- acceptable since `/send-email` takes no arguments (the SOP drives the workflow). - No `disable-model-invocation` -- acceptable since this skill needs the model to execute MCP calls and follow the SOP interactively. - No `context: fork` -- acceptable; the skill does not need session isolation. **Pattern compliance:** The skill follows the delegation pattern established by `/board-status` and `/board-sync` -- a thin SKILL.md that points to a pal-e-docs SOP via MCP call. The heavy content lives in `sop-email-send`, not inline. This is the correct architecture. **MCP call format:** The skill references `mcp__pal-e-docs__get_note(slug="sop-email-send")` and `mcp__pal-e-docs__get_note(slug="arch-email")` -- correct tool names and syntax. **Safety gates:** The Red Flags section is well-constructed. It covers: - No send without test email - No blast without explicit approval - No reuse of stale approval - No inline layout/query creation (file a ticket instead) - Explicit rejection of "just send to a few" workaround This aligns with `feedback_no_email_without_five_approvals.md` and `feedback_never_email_without_approval.md` from MEMORY.md. **Word count:** The file is 32 lines / approximately 160 words of content (excluding frontmatter). Well under the 200-word budget stated in the PR review checklist. ### BLOCKERS None. This is a 32-line skill file that delegates to an SOP. The BLOCKER criteria (test coverage for new functionality, unvalidated user input, secrets, DRY violations in auth paths) do not apply to a declarative Markdown skill definition. The skill itself does not execute code -- it instructs the agent to read and follow an SOP that contains the actual logic and safety gates. ### NITS 1. **Quick Reference section could drift from SOP.** The inline quick reference (layouts, test email, endpoint) duplicates information that presumably lives in `sop-email-send` and `arch-email`. If the SOP changes the endpoint or adds a layout, this skill will be stale. Consider whether the Quick Reference adds enough value to justify the maintenance burden, or whether the SOP alone is sufficient. This is a judgment call, not a blocker -- the current content is accurate and useful as a fast reminder. 2. **Test email address is hardcoded.** `draneylucas@gmail.com` is hardcoded as the test email. If this ever changes, it needs updating in both the SOP and this skill. Not a secret (it is a personal email used for testing, not a credential), but it is a hardcoded value that could drift. 3. **No step for loading the SOP references the basketball-api context.** The description says "through basketball-api" but the instructions do not explicitly tell the agent to check basketball-api connection/availability. The SOP likely handles this, so this is informational only. ### SOP COMPLIANCE - [x] Branch named after issue (`232-add-send-email-skill-wrapping-sop-email` references #232) - [x] PR body follows template (Summary, Changes, Test Plan, Review Checklist, Related Notes) - [x] Related section references SOPs and architecture notes - [x] No secrets committed (test email is not a secret) - [x] No unnecessary file changes (single file, exactly on scope) - [x] Commit message is descriptive (feat: add /send-email skill wrapping sop-email-send) - [x] `Closes #232` present in PR body ### PROCESS OBSERVATIONS Clean, minimal PR. The skill correctly follows the thin-wrapper pattern: SKILL.md is the trigger surface, pal-e-docs SOPs hold the logic. This keeps the claude-custom repo lightweight and ensures SOP updates propagate without needing skill file changes. The test plan includes real-world validation (contract reminder email via basketball-api tickets #750-752), which is appropriate for a skill that cannot be unit-tested in isolation. Deployment frequency: no risk. This is additive (new file only, no modifications to existing skills or hooks). Change failure risk: negligible. ### VERDICT: APPROVED

2026-04-03 19:05:30 +00:00

Summary

Adds /send-email skill that loads the email SOP from pal-e-docs and guides agents through the compose → test → approve → blast workflow.

Changes

skills/send-email/SKILL.md — new skill file with YAML frontmatter, instructions to read sop-email-send and arch-email, quick reference table (layouts, test email, endpoint), and red flags section for safety gate enforcement

Test Plan

Skill file has valid YAML frontmatter with name and description
Description starts with "Use when"
/send-email invocation loads the skill and directs to the SOP
Real-world validation: contract reminder email send (pending ticket #750-752 in basketball-api)

Review Checklist

No unrelated changes
Skill under 200 words (heavy content lives in SOP, not skill)
References sop-email-send and arch-email via MCP calls

sop-email-send — the SOP this skill wraps
arch-email — email system architecture reference
project-westside-basketball — parent project
Closes #232

## Summary Adds `/send-email` skill that loads the email SOP from pal-e-docs and guides agents through the compose → test → approve → blast workflow. ## Changes - `skills/send-email/SKILL.md` — new skill file with YAML frontmatter, instructions to read `sop-email-send` and `arch-email`, quick reference table (layouts, test email, endpoint), and red flags section for safety gate enforcement ## Test Plan - [ ] Skill file has valid YAML frontmatter with `name` and `description` - [ ] Description starts with "Use when" - [ ] `/send-email` invocation loads the skill and directs to the SOP - [ ] Real-world validation: contract reminder email send (pending ticket #750-752 in basketball-api) ## Review Checklist - [ ] No unrelated changes - [ ] Skill under 200 words (heavy content lives in SOP, not skill) - [ ] References `sop-email-send` and `arch-email` via MCP calls ## Related Notes - `sop-email-send` — the SOP this skill wraps - `arch-email` — email system architecture reference - `project-westside-basketball` — parent project - Closes #232

forgejo_admin added 1 commit

feat: add /send-email skill wrapping sop-email-send 0301b48967

Thin skill that loads the email SOP from pal-e-docs and walks agents
through the compose → test → approve → blast workflow. Safety gates
(test email, phone approval, double approval for contracts) live in
the SOP — the skill is the discovery trigger.

Closes #232

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