diff --git a/.claude/rules/skills-map.md b/.claude/rules/skills-map.md index 93f4af3..2b4fec6 100644 --- a/.claude/rules/skills-map.md +++ b/.claude/rules/skills-map.md @@ -44,7 +44,7 @@ Canonical order pre-PR: **`simplify → pre-commit-review`**. Reduce primero, re | Skill | Lifecycle | Modelo | Context | Qué hace | |---|---|---|---|---| | `simplify` | Fase N+1 (pre-PR) | sonnet | main (writer-scoped) | Reduce redundancia / ruido / complejidad accidental / abstracción prematura en archivos ya presentes en `git diff --name-only main..HEAD`. **Writer-scoped strict**: edita vía `Edit` sólo archivos del diff; no crea archivos nuevos; no toca archivos fuera del diff; no cambia comportamiento; no busca bugs; no hace refactor mayor. Cierra con reporte "qué simplificó / what was simplified" + "qué decidió no tocar / what it chose not to touch". | -| `pre-commit-review` | Fase N+2 (pre-PR) | sonnet | main + Agent-tool hybrid delegation (`code-reviewer`) | Main prepara context (branch kickoff + invariantes de `.claude/rules/*.md` aplicables al diff); delega vía `Agent(subagent_type="code-reviewer", ...)` con `git diff main..HEAD` completo + asks explícitos (bugs + logic + security + scope adherence + invariant violations); subagent corre en fork real y devuelve summary confidence-filtered; main folds (dedup + file:line + severity order + veredicto de una línea). Fallback a `general-purpose` si el runtime enum no expone `code-reviewer`. **Nunca edita, nunca abre PR, nunca sustituye a `simplify`**. | +| `pre-commit-review` | Fase N+2 (pre-PR) | sonnet | main + Agent-tool hybrid delegation (`pos-code-reviewer` plugin agent post-F2) | Main prepara context (branch kickoff + invariantes de `.claude/rules/*.md` aplicables al diff); delega vía `Agent(subagent_type="pos-code-reviewer", ...)` con `git diff main...HEAD` completo + asks explícitos (bugs + logic + security + scope adherence + invariant violations); subagent corre en fork real y devuelve summary confidence-filtered; main folds (dedup + file:line + severity order + veredicto de una línea). Fallback a `general-purpose` si el runtime no expone agents del plugin. **Nunca edita, nunca abre PR, nunca sustituye a `simplify`**. | | `compress` | on-demand (contexto >120k) | sonnet | main | Read-only advisory planner: propone qué `.claude/logs/*.jsonl` comprimir (edad, tamaño, importancia). No edita ni elimina archivos; usuario decide. E2b advisory-only. | | `audit-plugin` | antes de instalar community tool (MCP/plugin) | sonnet | main | Read-only advisory gate: audita contra SAFETY_POLICY.md 6-checklist, retorna GO/NO-GO/NEEDS_MORE_INFO. No instala, no enforza, no modifica policy.yaml. E2b advisory-only; enforcement diferido. | @@ -52,7 +52,7 @@ Canonical order pre-PR: **`simplify → pre-commit-review`**. Reduce primero, re | Skill | Lifecycle | Modelo | Context | Qué hace | |---|---|---|---|---| -| `compound` | post-merge (trigger by touched_paths, policy-driven) | opus | main (writer-scoped strict) | Lee merged diff; delega análisis a `code-architect` subagent (fallback `general-purpose`); escribe patrones a `.claude/patterns/` (formato canónico: `# Pattern:` + secciones `##`). No refactoring de código, solo propuestas de patrones. **STOP**: usuario revisa + aprueba merge de patrón. | +| `compound` | post-merge (trigger by touched_paths, policy-driven) | opus | main (writer-scoped strict) | Lee merged diff; delega análisis a `pos-architect` plugin agent post-F2 (fallback `general-purpose`); escribe patrones a `.claude/patterns/` (formato canónico: `# Pattern:` + secciones `##`). No refactoring de código, solo propuestas de patrones. **STOP**: usuario revisa + aprueba merge de patrón. | | `pattern-audit` | Fase N+3 post-merge (manual invoke) | sonnet | main (main-strict, no delegation) | Lee entradas `.claude/patterns/`; busca signals en codebase (Grep/Bash); detecta drift (signal/examples/rule inconsistency). Emite reporte diagnóstico sin mutar archivos. **STOP**: usuario revisa y decide acción (actualizar patrón / invocar compound). | | `test-scaffold` | al crear archivo sin test pair | sonnet | main (writer-scoped strict) | Detecta convención de tests (co-located vs `tests/`) sobre el repo; si ≥80% claro → genera **solo** el test pair skeleton del archivo source que el usuario provee; si ambigua → STOP + propone opciones. **NO** modifica source code, **NO** ejecuta pytest/vitest, **NO** modifica thresholds/config. Allowed-tools: `Glob`, `Grep`, `Read`, `Write`, `Bash(find:*)`, `Bash(git grep:*)`, logger. | | `test-audit` | on-demand / pre-release | sonnet | main (main-strict, no delegation) | Read-only advisory: declara **candidate signals** (no "detecta") via static analysis sobre archivos test — flaky risk (asserts en loops/conditionals), orphan tests (imports a paths que no existen), trivial assertions (`assert True`/`False`/empty). **NO** ejecuta pytest/vitest, **NO** modifica archivos. Cierra con disclaim de no-exhaustividad. Allowed-tools: `Glob`, `Grep`, `Read`, `Bash(find:*)`, `Bash(git grep:*)`, `Bash(wc:*)`, logger. | @@ -66,6 +66,26 @@ Canonical order pre-PR: **`simplify → pre-commit-review`**. Reduce primero, re | `/pos:pr-description` | pre-PR | sonnet | main | Genera descripción del PR desde commits + kickoff | | `/pos:release` | en tag | sonnet | main | Valida versión + publica + notifica | +## Subagents del plugin (entregado en F2) + +Plugin-owned subagents en `agents/.md`. **Shape distinto al skill primitive**: frontmatter `{name, description, tools, model}`, donde `tools` es comma-separated string (oficial Claude Code subagent format), no YAML list `allowed-tools`. Sin campos inventados. Namespace `pos-*` obligatorio para evitar colisión con built-ins de Claude Code (`code-reviewer`, `code-architect`, `Plan`, `Explore`, `general-purpose`) y con user/project agents externos. + +| Agent | Consumido por | Modelo | Capacidad | Hard limits | +|---|---|---|---|---| +| `pos-code-reviewer` | `pre-commit-review` (E2a) | sonnet | Branch-diff review: bugs / logic errors / security vulnerabilities / scope adherence / invariant violations. Output findings agrupados por severidad (blocker/high/medium/nit), confidence-filtered, `file:line` refs. | No `Edit`, no `Write`, no PR, no invocación de otras skills/subagents. | +| `pos-architect` | `compound` (E3a) | sonnet | Pattern extraction (≥2 repeticiones, regla #7) + architectural design + cross-file consistency. Output patterns en formato canónico (Name/Context/Signal/Rule/Examples/Rationale) listo para fold en `.claude/patterns/.md`. | No refactor, no `Edit`, no overwriting de patterns existentes. | + +**Contract tests** en `agents/tests/test_agent_frontmatter.py` parametrizados por `ALLOWED_AGENTS = ["pos-code-reviewer", "pos-architect"]`: 4 clases (structure, frontmatter keys ⊆ `{name, description, tools, model}` + namespace `pos-*` + tools comma-separated string + model ∈ `{sonnet, opus, haiku}`, body substantive >100 chars, capability surfaces). + +**Fallback `general-purpose`** explícito en bodies de skills consumidoras — protege contra runtimes que no exponen agents del plugin (Claude Code antes de discovery del directorio `agents/`, entornos minimal). + +**Diferidos en F2 (regla #7 CLAUDE.md)**: + +- `pos-auditor` — sin consumer real hoy. Reabrir cuando una skill futura lo requiera. +- `policy.yaml.agents_allowed` — sin enforcement consumer (Stop hook lee `skills.jsonl`, no hay log de invocaciones de subagents). Reabrir cuando un hook futuro requiera enforcement. + +**Forward-compat negation**: main-strict skills (`pattern-audit` E3a, `audit-session` F1) **nunca** deben referenciar plugin subagents. Tests negation list incluye `pos-architect` / `pos-code-reviewer` desde F2. + ## Disabled until earned Ninguna skill adicional se registra en `policy.yaml` hasta que pasa `/pos:audit-plugin --self`. diff --git a/.claude/rules/skills.md b/.claude/rules/skills.md index 4e856de..59fefe9 100644 --- a/.claude/rules/skills.md +++ b/.claude/rules/skills.md @@ -40,22 +40,26 @@ Ver [skills-map.md § Shape canónico](skills-map.md) para el shape autoritativo El primitive no soporta `context: fork` declarativamente. Si una skill requiere cross-file analysis pesado (≥3 archivos no triviales), **delegar inline vía Agent tool** desde el body de la SKILL.md es el patrón correcto. -La skill debe escoger el `subagent_type` **por capacidad**, no por nombre hardcoded — los nombres concretos son defaults de Claude Code y pueden variar entre releases/entornos. Regla operativa: inspeccionar el enum `subagent_type` de la tool `Agent` en runtime y matchear por capacidad. Los nombres abajo reflejan los defaults de Claude Code hoy: +La skill debe escoger el `subagent_type` **por capacidad**, no por nombre hardcoded — los nombres concretos pueden variar entre releases/entornos. Regla operativa: inspeccionar el enum `subagent_type` de la tool `Agent` en runtime y matchear por capacidad. Desde F2 los subagents críticos para review + arquitectura son **propiedad del plugin** (definidos en `agents/.md` con namespace obligatorio `pos-*`); el resto reusan defaults built-in de Claude Code: -- **Planning independiente** (cross-check contra el plan de la skill) — Claude Code hoy: `Plan`. -- **Segunda opinión arquitectónica** (crítica de diseño) — Claude Code hoy: `code-architect`. -- **Context gathering cross-files** (explora y devuelve digest) — Claude Code hoy: `Explore`. -- **Review de diff de rama** (confidence-filtered review) — Claude Code hoy: `code-reviewer`. **Primera consumidora: `pre-commit-review` (E2a)**. Patrón: main prepara context ligero (branch kickoff + invariantes de `.claude/rules/*.md` aplicables al diff); delega via `Agent(subagent_type="code-reviewer", ...)` con `git diff main..HEAD` completo + asks explícitos; subagent corre en fork real, devuelve summary confidence-filtered; main folds (dedup + file:line + severity). Hardcode del nombre `code-reviewer` en el body **con disclaimer** + fallback a `general-purpose` si el runtime enum no lo expone — decisión A5 de E2a: una sola consumidora hoy no justifica helper runtime (regla #7 CLAUDE.md). Reabrir si una segunda skill aporta la repetición. -- **Fallback general** si ninguna capacidad específica encaja — Claude Code hoy: `general-purpose` con task prompt explícito. +- **Planning independiente** (cross-check contra el plan de la skill) — Claude Code hoy: `Plan` (built-in). +- **Segunda opinión arquitectónica genérica** (crítica de diseño no-pattern-extraction) — Claude Code hoy: `code-architect` (built-in). Para pattern extraction post-merge usar `pos-architect` (ver abajo). +- **Context gathering cross-files** (explora y devuelve digest) — Claude Code hoy: `Explore` (built-in). +- **Review de diff de rama** (confidence-filtered review) — **plugin agent: `pos-code-reviewer`** (entregado en F2; ver `agents/pos-code-reviewer.md`). **Primera consumidora: `pre-commit-review` (E2a)**. Patrón: main prepara context ligero (branch kickoff + invariantes de `.claude/rules/*.md` aplicables al diff); delega via `Agent(subagent_type="pos-code-reviewer", ...)` con `git diff main...HEAD` completo + asks explícitos; subagent corre en fork real, devuelve summary confidence-filtered; main folds (dedup + file:line + severity). Hardcode del nombre `pos-code-reviewer` en el body **con disclaimer** + fallback a `general-purpose` si el runtime no expone agents del plugin — decisión A5 de E2a (una sola consumidora hoy no justifica helper runtime, regla #7 CLAUDE.md) ratificada en F2 con namespace `pos-*` para evitar colisión con built-ins. +- **Pattern extraction + cross-file design** (post-merge) — **plugin agent: `pos-architect`** (entregado en F2; ver `agents/pos-architect.md`). **Primera consumidora: `compound` (E3a)**. Patrón: main pasa diff merged + lista de patterns existentes; subagent identifica 1–3 patrones que repiten ≥2 veces y devuelve propuestas en formato canónico (Name/Context/Signal/Rule/Examples/Rationale); main folds en `.claude/patterns/.md`. Hardcode `pos-architect` con fallback `general-purpose`. +- **Fallback general** si ninguna capacidad específica encaja o el runtime no expone el agent del plugin — Claude Code hoy: `general-purpose` con task prompt explícito. El subagent corre en fork real; la skill recibe el summary al tool result y lo folds en su output — no paste-through. +**Plugin agent primitive (entregado en F2)**: `agents/.md` con frontmatter `{name, description, tools, model}`. **Shape distinto al skill primitive**: `tools` es comma-separated string (oficial Claude Code subagent format), no YAML list `allowed-tools`. Sin campos inventados. Namespace `pos-*` obligatorio. Contract tests en `agents/tests/test_agent_frontmatter.py` parametrizados por `ALLOWED_AGENTS`. No hay `agents_allowed` en `policy.yaml` hoy (sin enforcement consumer; el Stop hook lee `skills.jsonl`, no `agents.jsonl`). + **Precedentes**: -- `branch-plan` (E1b) delega cuando el plan es heavy (pick `Plan` / `code-architect` / `Explore` por capacidad). +- `branch-plan` (E1b) delega cuando el plan es heavy (pick `Plan` / `code-architect` / `Explore` built-in por capacidad). - `deep-interview` (E1b) nunca delega — su coste está en el dialog, no en reading. -- `pre-commit-review` (E2a) delega siempre a `code-reviewer` como hybrid pattern (main prepara context, subagent analiza, main folds) — primer uso del subagent `code-reviewer` en el repo. -- `project-kickoff` y `writing-handoff` (E1a), `simplify` (E2a) son main-strict por scope acotado. +- `pre-commit-review` (E2a) delega siempre a `pos-code-reviewer` como hybrid pattern (main prepara context, subagent analiza, main folds) — **primera consumidora del plugin agent post-F2** (antes de F2 apuntaba a `code-reviewer` built-in). +- `compound` (E3a) delega a `pos-architect` post-merge para pattern extraction — **primera consumidora del plugin agent post-F2** (antes de F2 apuntaba a `code-architect` built-in). +- `project-kickoff` y `writing-handoff` (E1a), `simplify` (E2a) son main-strict por scope acotado. `pattern-audit` (E3a) y `audit-session` (F1) son main-strict por design — **forward-compat negación**: nunca deben referenciar plugin subagents (negation list de tests incluye `pos-architect` / `pos-code-reviewer` desde F2). **Precedentes de writer-scope**: `writing-handoff` (E1a) escribe **sólo** en HANDOFF.md con scope por sección declarado en el body; `simplify` (E2a) es la **segunda** writer-scoped del repo — edita **sólo** archivos presentes en `git diff --name-only main..HEAD`, con scope check en cada `Edit` call y reclasificación a `skip (out of scope)` si el `file_path` no está en la lista derivada. Ambas cierran con reporte de qué cambiaron y qué decidieron no tocar. @@ -66,7 +70,7 @@ El primitive no soporta `model:` en el frontmatter — la skill corre en el mode ## Tests - **Contract tests** del primitive en `.claude/skills/tests/test_skill_frontmatter.py` — parametrizados por `ALLOWED_SKILLS` (renombrado desde `E1_SKILLS_KNOWN` en E2a cuando la allowlist cruzó el límite de fase; contract-bound al `skills_allowed` del meta-repo, no era-bound: extender = añadir entrada aquí + en `policy.yaml`, no renombrar). Validan: dir + SKILL.md existe; NO `skill.json`; frontmatter keys ⊆ `{name, description, allowed-tools}`; `name` == dir name; description case-insensitive `startswith "use when"`; `allowed-tools` es `list[str]` si presente; `name` sin prefijo `pos:`; body referencia `.claude/skills/_shared/log-invocation.sh`; shared logger existe y es ejecutable. -- **Behavior tests** del body — añadir una `TestXxxBehavior` class por skill cuando el framing del body sea parte del contrato. Existentes: `TestBranchPlanBehavior` + `TestDeepInterviewBehavior` (E1b — disclaim de marker, opt-in gating, no silent mutation); `TestPreCommitReviewBehavior` + `TestSimplifyBehavior` (E2a — delegation a `code-reviewer`, scope `git diff main..HEAD`, disclaim de escritura / reemplazo de `simplify`, writer-scoped-al-diff, reducer-not-bug-finder, reporte de qué simplificó + qué decidió no tocar). +- **Behavior tests** del body — añadir una `TestXxxBehavior` class por skill cuando el framing del body sea parte del contrato. Existentes: `TestBranchPlanBehavior` + `TestDeepInterviewBehavior` (E1b — disclaim de marker, opt-in gating, no silent mutation); `TestPreCommitReviewBehavior` + `TestSimplifyBehavior` (E2a — delegation a `pos-code-reviewer` post-F2, scope `git diff main...HEAD`, disclaim de escritura / reemplazo de `simplify`, writer-scoped-al-diff, reducer-not-bug-finder, reporte de qué simplificó + qué decidió no tocar); `TestCompoundBehavior` (E3a, flippeada en F2 — delegation a `pos-architect` con fallback `general-purpose`); negation lists de `TestPatternAuditBehavior` (E3a) y `TestAuditSessionBehavior` (F1) extendidas con `pos-*` desde F2 (forward-compat: main-strict skills nunca referencian plugin subagents). - **Integration contract** logger ↔ Stop hook en `hooks/tests/test_skills_log_contract.py`. Añadir un caso `test_all_N_skills_end_to_end` (o renombrar el existente) cuando la allowlist crezca. Precedente: `test_all_four_e1_skills_end_to_end` (E1b) → `test_all_six_e1_e2a_skills_end_to_end` (E2a). ## Logging — best-effort via helper diff --git a/.claude/skills/compound/SKILL.md b/.claude/skills/compound/SKILL.md index 43f189c..9788d70 100644 --- a/.claude/skills/compound/SKILL.md +++ b/.claude/skills/compound/SKILL.md @@ -21,7 +21,7 @@ Extract and register reusable patterns when a merged branch exhibits extractable You MAY: - Read the merged diff (`main...HEAD`). -- Delegate deep pattern analysis to `code-architect` subagent via Agent tool (hybrid pattern: main prepares context, subagent analyzes, main writes). +- Delegate deep pattern analysis to `pos-architect` subagent via Agent tool (hybrid pattern: main prepares context, subagent analyzes, main writes). - Create `.claude/patterns/.md` with pattern entries (write-only, strict scope). You MUST NOT: @@ -67,11 +67,11 @@ Mandatory fields: Context, Signal, Rule, Examples. Last observed: timestamped by - List touched top-level dirs + file count. - Collect `.claude/patterns/*.md` existing entries (to avoid duplicates). -3. **Delegate to `code-architect`** via Agent tool (with fallback): +3. **Delegate to `pos-architect`** via Agent tool (with fallback): - Full diff: `git diff main...HEAD`. - Existing patterns (so subagent can avoid re-proposing). - Explicit task: "identify 1–3 patterns that repeat ≥2 times across the diff and would reduce future duplication. Output: pattern proposals (no code refactoring, no file changes)." - - Preferred subagent: `code-architect` (pattern extraction via architecture lens). + - Preferred subagent: `pos-architect` (pattern extraction via architecture lens). - Fallback if unavailable: `general-purpose` with same task (generic analysis acceptable). - Subagent returns a summary (not raw code). diff --git a/.claude/skills/pre-commit-review/SKILL.md b/.claude/skills/pre-commit-review/SKILL.md index 66ad173..c46a054 100644 --- a/.claude/skills/pre-commit-review/SKILL.md +++ b/.claude/skills/pre-commit-review/SKILL.md @@ -1,6 +1,6 @@ --- name: pre-commit-review -description: Use when the user asks to "revisa la rama", "pre-commit review", "review antes del PR", "chequea el diff", or right before running `gh pr create`. Prepares the review context in the main thread (branch kickoff, scope, applicable invariants), delegates the analysis to the `code-reviewer` subagent via the Agent tool over `git diff main...HEAD`, and folds the subagent summary into prioritized findings. Does NOT rewrite code. Does NOT apply fixes. Does NOT replace `simplify`. +description: Use when the user asks to "revisa la rama", "pre-commit review", "review antes del PR", "chequea el diff", or right before running `gh pr create`. Prepares the review context in the main thread (branch kickoff, scope, applicable invariants), delegates the analysis to the `pos-code-reviewer` subagent via the Agent tool over `git diff main...HEAD`, and folds the subagent summary into prioritized findings. Does NOT rewrite code. Does NOT apply fixes. Does NOT replace `simplify`. allowed-tools: - Read - Grep @@ -14,7 +14,7 @@ allowed-tools: # pre-commit-review -Produce a prioritized review of the current branch's diff (`main...HEAD`) before the user opens a PR with `gh pr create`. Delegate the heavy analysis to the `code-reviewer` subagent; the main thread only prepares context and folds the summary. +Produce a prioritized review of the current branch's diff (`main...HEAD`) before the user opens a PR with `gh pr create`. Delegate the heavy analysis to the `pos-code-reviewer` subagent; the main thread only prepares context and folds the summary. Framing: this is an eligibility hint, not a guaranteed auto-trigger. Claude Code decides whether to activate you based on `description`. Output is a review proposal — the user decides which findings to act on. @@ -24,7 +24,7 @@ You MAY: - Read the kickoff commit and recent branch commits with `git log`. - Inspect the diff with `git diff main...HEAD`. - Read `.claude/rules/*.md`, `CLAUDE.md`, and any file cited in the branch's `MASTER_PLAN.md` entry to seed invariants into the review prompt. -- Delegate the analysis to the `code-reviewer` subagent via the Agent tool (see "Delegation" below). +- Delegate the analysis to the `pos-code-reviewer` subagent via the Agent tool (see "Delegation" below). - Emit prioritized findings in this conversation (confidence-filtered, bucketed by severity). You MUST NOT: @@ -36,17 +36,17 @@ You MUST NOT: ## Delegation (hybrid: main prepares context, subagent analyzes) -Main thread stays cheap; heavy analysis goes to the `code-reviewer` subagent via the `Agent` tool: +Main thread stays cheap; heavy analysis goes to the `pos-code-reviewer` subagent via the `Agent` tool: 1. Main gathers context — kickoff commit, branch scope, invariants cited in `.claude/rules/*.md` that touch paths in the diff. -2. Main invokes `Agent(subagent_type="code-reviewer", prompt=...)` with: +2. Main invokes `Agent(subagent_type="pos-code-reviewer", prompt=...)` with: - The prepared context (branch scope + invariants applicable to touched paths). - The full diff (`git diff main...HEAD`). - Explicit asks: bugs, logic errors, security vulnerabilities, adherence to branch scope, adherence to repo invariants. 3. Subagent runs in its own fork, returns a summary with confidence-filtered findings. 4. Main folds the summary — does NOT paste-through. Prioritizes by severity, attaches `file:line` references, groups by theme if useful. -The string `code-reviewer` here reflects the Claude Code default shipped today. It is hardcoded with a disclaimer: default `subagent_type` names can vary between releases/environments. If the Agent tool's `subagent_type` enum at runtime does NOT include `code-reviewer`, fall back to `general-purpose` with a task prompt that names the same capability (bugs + logic + security + convention adherence). See `.claude/rules/skills.md § Fork / delegación`. +The string `pos-code-reviewer` is the plugin subagent shipped by `pos` in F2 (defined in `agents/pos-code-reviewer.md`). It is hardcoded with a disclaimer: runtimes that do not load plugin agents (or load them under different names) may not expose it. If the Agent tool's `subagent_type` enum at runtime does NOT include `pos-code-reviewer`, fall back to `general-purpose` with a task prompt that names the same capability (bugs + logic + security + convention adherence). See `.claude/rules/skills.md § Fork / delegación`. ## Steps @@ -66,13 +66,13 @@ The string `code-reviewer` here reflects the Claude Code default shipped today. - Collect applicable invariants from `CLAUDE.md` (regla #3 tests-first, regla #7 patrones antes de abstraer, etc.). - Summarize branch scope from `MASTER_PLAN.md § Rama ` if the section exists. -3. **Delegate to `code-reviewer`** via the Agent tool. The prompt must include: +3. **Delegate to `pos-code-reviewer`** via the Agent tool. The prompt must include: - Branch name, base, and kickoff scope. - Invariants applicable to the touched paths (grep output, not full rule files). - The full diff (`git diff main...HEAD`). - Explicit asks: "bugs, logic errors, security vulnerabilities, places where the diff leaves the branch scope, and violations of the cited invariants. Group findings by severity (blocker / high / medium / nit). Confidence-filter: omit findings below medium confidence." - If the runtime Agent tool does NOT list `code-reviewer`, fall back to `general-purpose` with a prompt that names the capability; note the fallback in the user-facing summary. + If the runtime Agent tool does NOT list `pos-code-reviewer`, fall back to `general-purpose` with a prompt that names the capability; note the fallback in the user-facing summary. 4. **Fold the subagent summary** into this conversation. Do NOT paste-through: - Dedup findings that say the same thing twice. @@ -97,7 +97,7 @@ The string `code-reviewer` here reflects the Claude Code default shipped today. - HEAD is `main` / `master` → no branch to review; stop and tell the user. Log `status: partial`. - `git diff main...HEAD` returns empty → no changes to review; stop. Log `status: partial`. -- `code-reviewer` subagent unavailable at runtime → fall back to `general-purpose` with an equivalent task prompt; note the fallback in the user-facing summary. Log `status: degraded`. +- `pos-code-reviewer` subagent unavailable at runtime → fall back to `general-purpose` with an equivalent task prompt; note the fallback in the user-facing summary. Log `status: degraded`. - Subagent returns an empty / unusable summary → report it to the user; do NOT invent findings. Log `status: ambiguous`. ## Explicitly out of scope diff --git a/.claude/skills/tests/test_skill_frontmatter.py b/.claude/skills/tests/test_skill_frontmatter.py index 15dc6c7..906ea4f 100644 --- a/.claude/skills/tests/test_skill_frontmatter.py +++ b/.claude/skills/tests/test_skill_frontmatter.py @@ -313,9 +313,11 @@ def test_body_guards_against_silent_mutation(self): # Behavior-specific contracts (E2a — pre-commit-review + simplify) # # Lock down the Fase -1 decisions ratified by the user: -# - `pre-commit-review` delegates to the `code-reviewer` subagent over the +# - `pre-commit-review` delegates to the `pos-code-reviewer` plugin subagent +# (post-F2; pre-F2 was the `code-reviewer` built-in default) over the # branch diff, produces prioritized findings, never rewrites code, never -# applies fixes, never replaces `simplify`. +# applies fixes, never replaces `simplify`. Fallback to `general-purpose` +# mandatory if the runtime does not expose plugin agents. # - `simplify` is a writer scoped strictly to files already in the branch # diff: can Edit, cannot create new files, cannot touch files outside the # diff. Frames itself as a reducer (not a bug finder) and reports what it @@ -324,18 +326,22 @@ def test_body_guards_against_silent_mutation(self): class TestPreCommitReviewBehavior: - def test_delegates_to_code_reviewer(self): - """The skill must delegate to the `code-reviewer` subagent via the - Agent tool (same pattern branch-plan established in E1b). The string - `code-reviewer` is hardcoded with a disclaimer about default-name - fragility (see .claude/rules/skills.md § Fork / delegación).""" + def test_delegates_to_pos_code_reviewer(self): + """F2: pre-commit-review must delegate to the `pos-code-reviewer` + subagent (plugin-owned, namespaced to avoid collision with built-in + defaults). Fallback to `general-purpose` is mandatory if the runtime + does not expose plugin agents.""" _, body = read_skill("pre-commit-review") low = body.lower() - assert "code-reviewer" in low, ( - "pre-commit-review body must name the `code-reviewer` subagent " - "(the canonical default in Claude Code today) so the delegation " + assert "pos-code-reviewer" in low, ( + "pre-commit-review body must name the `pos-code-reviewer` " + "subagent (F2 plugin agent, namespaced) so the delegation " "contract is explicit and greppable." ) + assert "general-purpose" in low, ( + "pre-commit-review body must document the fallback to " + "`general-purpose` when `pos-code-reviewer` is unavailable." + ) assert "subagent_type" in low or "agent tool" in low, ( "pre-commit-review body must reference the Agent-tool delegation " "mechanism (e.g., `subagent_type`) so the pattern matches E1b " @@ -344,7 +350,8 @@ def test_delegates_to_code_reviewer(self): def test_scope_is_branch_diff(self): """Review operates on the branch diff, not the full tree. Body must - show the exact git invocation (`git diff ... main..HEAD`).""" + show the exact git invocation (`git diff ... main...HEAD` — + triple-dot, merge-base relative).""" _, body = read_skill("pre-commit-review") low = body.lower() assert "git diff" in low, ( @@ -412,8 +419,8 @@ def test_allowed_tools_includes_edit(self): def test_scope_limited_to_branch_diff_no_new_files(self): """Writer scope is strict: files already in the diff only, no new files, nothing outside the diff. Body must show the exact command - that derives the scope (`git diff --name-only main..HEAD`) and the - three explicit disclaims.""" + that derives the scope (`git diff --name-only main...HEAD` — + triple-dot, merge-base relative) and the three explicit disclaims.""" _, body = read_skill("simplify") low = body.lower() assert "git diff --name-only" in low and "main...head" in low, ( @@ -680,14 +687,21 @@ def test_body_contains_stop_signal(self): "(marks writer-scoped limit: writes patterns, stops)." ) - def test_body_declares_fallback(self): - """compound documents fallback to general-purpose if code-architect unavailable.""" + def test_body_delegates_to_pos_architect_with_fallback(self): + """F2: compound must delegate to the `pos-architect` plugin subagent + (namespaced to avoid collision with built-in defaults). Fallback to + `general-purpose` is mandatory if the runtime does not expose plugin + agents.""" _, body = read_skill("compound") low = body.lower() - # Verify fallback is documented in the body - assert ("fallback" in low and "general-purpose" in low) or "general-purpose" in low, ( - "compound body must document fallback to general-purpose subagent " - "when code-architect is unavailable." + assert "pos-architect" in low, ( + "compound body must name the `pos-architect` subagent " + "(F2 plugin agent, namespaced) so the delegation contract is " + "explicit and greppable." + ) + assert "general-purpose" in low, ( + "compound body must document the fallback to `general-purpose` " + "when `pos-architect` is unavailable." ) @@ -727,7 +741,14 @@ def test_body_declares_main_strict_no_delegation(self): """pattern-audit is main-strict: no Agent tool delegation in E3a.""" _, body = read_skill("pattern-audit") low = body.lower() - delegation_tokens = ("delegate", "subagent", "code-architect", "agent(") + delegation_tokens = ( + "delegate", + "subagent", + "code-architect", + "pos-architect", + "pos-code-reviewer", + "agent(", + ) assert not any(tok in low for tok in delegation_tokens), ( "pattern-audit body must NOT mention Agent delegation or subagents; " "analysis must be main-strict (local Grep/Bash only)." @@ -799,7 +820,13 @@ def test_body_declares_main_strict_no_delegation(self): needed.""" _, body = read_skill("audit-session") low = body.lower() - delegation_tokens = ("subagent", "code-architect", "agent(") + delegation_tokens = ( + "subagent", + "code-architect", + "pos-architect", + "pos-code-reviewer", + "agent(", + ) assert not any(tok in low for tok in delegation_tokens), ( "audit-session body must NOT mention Agent delegation or subagents; " "comparison must be main-strict (local Read/Glob/Grep only)." diff --git a/HANDOFF.md b/HANDOFF.md index 0ef3b30..f5ab2d8 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -5,8 +5,9 @@ ## 1. Snapshot - Repo: `project-operating-system` (plugin `pos`). -- Rama actual: **F1 ✅ PR pendiente** (`feat/f1-skill-audit-session`, en revisión docs-sync). Anterior: **E3b ✅ PR #24** (mergeada). Siguiente: **F2 — `feat/f2-agents-subagents`** (3 subagents: `code-reviewer`, `architect`, `auditor`). -- F1 entregó: `/pos:audit-session` (read-only advisory main-strict) — compara 3 superficies explícitas de `policy.yaml` (`skills_allowed`, `lifecycle..hooks_required`, `audit.required_logs`) vs `.claude/logs/` reales; reporta drift candidates por bucket sin auto-fix. Policy: `skills_allowed` 13→14. Fase F abierta (1/4 ramas). +- Rama actual: **F2 ✅ PR pendiente** (`feat/f2-agents-subagents`, en revisión docs-sync). Anterior: **F1 ✅ PR #25** (mergeada). Siguiente: **F3 — `feat/f3-selftest-end-to-end`** (`bin/pos-selftest.sh` + escenarios end-to-end). +- F2 entregó: `agents/pos-code-reviewer.md` + `agents/pos-architect.md` (plugin subagents primitive-correct con namespace `pos-*`); flips de `pre-commit-review` y `compound` a los nuevos consumidores; 26 contract tests parametrizados (`agents/tests/test_agent_frontmatter.py`). Behavior tests del consumer flippean assertions (sin delta de count). **No** toca `policy.yaml` (`agents_allowed` diferido). `auditor` diferido (sin consumer real, regla #7). +- F1 entregó: `/pos:audit-session` (read-only advisory main-strict) — compara 3 superficies explícitas de `policy.yaml` (`skills_allowed`, `lifecycle..hooks_required`, `audit.required_logs`) vs `.claude/logs/` reales; reporta drift candidates por bucket sin auto-fix. Policy: `skills_allowed` 13→14. Fase F abierta (2/4 ramas). - Fuente de verdad ejecutable: [MASTER_PLAN.md](MASTER_PLAN.md). - Estado vivo: [ROADMAP.md](ROADMAP.md). - Arquitectura canonical: [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md). @@ -123,21 +124,22 @@ Ver detalle: `.claude/rules/generator.md` (Deferrals), `MASTER_PLAN § Rama C3` ## 8. Skills / subagents del entorno Claude Code (no plugin) -Hasta que `pos` tenga sus propias skills: +Hasta F1 el plugin reusaba subagents built-in; desde F2 los críticos son propios: -- `Explore` (>3 queries de búsqueda cross-archivo). -- `code-reviewer`, `code-architect`, `Plan` — subagents built-in. +- `pos-code-reviewer` (plugin, F2) — consumido por `pre-commit-review`. Fallback `general-purpose` si runtime no expone agents del plugin. +- `pos-architect` (plugin, F2) — consumido por `compound`. Fallback `general-purpose`. +- Built-in residuales (sólo si `branch-plan` requiere lectura ≥3 archivos): `Plan`, `Explore`, `code-architect`. Mismos fallback rules. - Skills globales del usuario (si las tiene en `~/.claude/skills/`). ## 9. Próxima rama -**F2 — `feat/f2-agents-subagents`** (tras merge de F1 — segundo bloque de Fase F: subagents oficiales del plugin). +**F3 — `feat/f3-selftest-end-to-end`** (tras merge de F2 — tercer bloque de Fase F: selftest end-to-end del plugin). Scope: -- 3 subagent definitions en `agents/`: `code-reviewer.md`, `architect.md`, `auditor.md`. Hoy las skills E2a (`pre-commit-review`) y E3a (`compound`) referencian `subagent_type="code-reviewer"` y `code-architect` con disclaimer de fallback a `general-purpose`; F2 los entrega como contratos canónicos del plugin (no defaults de Claude Code). -- Decisión Fase -1: ¿shape de subagent definition primitive-correct (Markdown body + frontmatter `name`/`description`/`tools`)?, ¿cómo se descubren desde el plugin (`agents/` directory en plugin root vs registro en `policy.yaml`)?, ¿naming conflict con `code-reviewer` default de Claude Code (resolver vía namespace o aceptar override)? -- Sin cambios a `skills_allowed` (subagents no son skills); evaluar si `policy.yaml.agents_allowed` aparece como nuevo top-level (precedente: `skills_allowed`). +- `bin/pos-selftest.sh` + escenarios. Valida los gates principales (kickoff, Fase -1 marker, pre-write, pre-pr-gate, stop-policy-check, post-action) sobre un proyecto sintético generado por `npx tsx generator/run.ts`. +- Decisión Fase -1: ¿qué subset de gates entran en F3 vs diferimos?, ¿cómo aislar el repo sintético (tmpdir vs fixture committeado)?, ¿qué profile canónico se usa para el smoke (`nextjs-app` / `cli-tool` / `agent-sdk`)?, ¿cómo se ejecuta el selftest en CI (mismo workflow que vitest+pytest o workflow propio)? +- F3 puede dogfooding F2: el selftest invoca `pre-commit-review` y `compound` reales, lo que ejercita el resolution `pos-code-reviewer` / `pos-architect` con fallback `general-purpose` cuando el runtime sintético no expone los agents. ## 10. Estado E2b (✅ merged PR #22) @@ -384,3 +386,42 @@ Entregables completados: **Resultado**: suite conjunta `hooks/tests` + `.claude/skills/tests` arroja **793 passed + 1 skipped** (sin regresión D1..D6 + E1a..E3b; +5 behavior + 22 parametrizados via `ALLOWED_SKILLS` 13→14 + 2 renames integration). El skip es el D5 intencional `TestIntegrationDiffUnavailable` por subprocess-no-cover. CI verde tras docs-sync (`ROADMAP.md` Fase F abierta + § F1 detallado, `HANDOFF.md` §1+§9+§19 nuevos, `MASTER_PLAN.md § Rama F1` expandida + cierre `✅ PR pendiente`, `.claude/rules/skills-map.md` fila `/pos:audit-session` populada). **Detalle + deferrals + ajustes**: ver [ROADMAP.md § feat/f1-skill-audit-session](ROADMAP.md), [MASTER_PLAN.md § Rama F1](MASTER_PLAN.md), [.claude/rules/skills-map.md](.claude/rules/skills-map.md). + +## 20. Estado F2 (cerrada en rama, docs-sync en curso) + +`feat/f2-agents-subagents` — **segunda rama de Fase F**. Cierra la asimetría heredada de E2a/E3a: hasta F1 las skills `pre-commit-review` y `compound` delegaban a `code-reviewer` / `code-architect` (defaults de Claude Code, no propiedad del plugin). F2 entrega los subagents como contratos del plugin con namespace `pos-*` y flippea las skills consumidoras al nuevo nombre, sin tocar el shape ni el flujo. Tras F2, el plugin tiene primitive-correct ownership de toda su superficie (skills + hooks + agents + policy). + +**Entregables**: + +- `agents/pos-code-reviewer.md` — plugin subagent para branch-diff review. Consumido por `pre-commit-review` (E2a). Frontmatter primitive-correct (`name` + `description` + `tools` comma-separated string + `model: sonnet`). Body declara las 5 capacidades: bugs / logic errors / security vulnerabilities / scope adherence / invariant violations. Output contract: findings agrupados por severidad (blocker/high/medium/nit), confidence-filtered, `file:line` refs. Hard limits: no `Edit`, no `Write`, no PR, no invocación de otras skills/subagents. +- `agents/pos-architect.md` — plugin subagent para pattern extraction + cross-file design. Consumido por `compound` (E3a). Frontmatter primitive-correct. Body declara 3 dimensiones: pattern extraction (≥2 repeticiones, regla #7 CLAUDE.md), architectural design (cohesión + tradeoffs), cross-file consistency. Output contract: patterns en formato canónico (Name/Context/Signal/Rule/Examples/Rationale) listo para que `compound` los folde en `.claude/patterns/.md`. Hard limits: no refactor, no `Edit`, no overwriting de patterns existentes. +- `.claude/skills/pre-commit-review/SKILL.md` — flip `code-reviewer` → `pos-code-reviewer` (description + body + steps + failure modes). Fallback a `general-purpose` se mantiene literal — protege contra runtimes que no expongan agents del plugin. +- `.claude/skills/compound/SKILL.md` — flip `code-architect` → `pos-architect` (mismo patrón). Fallback `general-purpose` intacto. +- `agents/tests/test_agent_frontmatter.py` (NEW) — 26 contract tests parametrizados por `ALLOWED_AGENTS = ["pos-code-reviewer", "pos-architect"]`: 4 clases (`TestStructure`, `TestFrontmatter`, `TestBody`, `TestCodeReviewerCapability` + `TestArchitectCapability`). `ALLOWED_FRONTMATTER_KEYS = REQUIRED_FRONTMATTER_KEYS = {name, description, tools, model}` (todos requeridos post-revisión PR); `VALID_MODELS = {sonnet, opus, haiku}`; `F2_REQUIRED_MODEL = "sonnet"`. Validan: file exists + parses; keys ⊆ allowed; todos los 4 keys requeridos; `name` == filename slug; namespace `pos-*`; `tools` comma-separated string (NO YAML list — diferencia con skill primitive — con validación de tokens no vacíos y sin whitespace en el nombre antes de `(...)`); `model` válido + `model == "sonnet"` lockeado por F2 Fase -1 (1); body >100 chars; capability surfaces (bug/security/scope/invariant para `pos-code-reviewer`; pattern/design/cross-file consistency para `pos-architect`). +- `.claude/skills/tests/test_skill_frontmatter.py` — `TestPreCommitReviewBehavior::test_delegates_to_pos_code_reviewer` + `TestCompoundBehavior::test_body_delegates_to_pos_architect_with_fallback` flippean a los nuevos nombres + asertan literalmente fallback `general-purpose`. `pattern-audit` + `audit-session` negation lists incluyen ahora `pos-architect` / `pos-code-reviewer` (forward-compat: main-strict skills nunca deben referenciar plugin subagents). + +**Contrato fijado por la suite** (extiende E1..F1 sin reabrirlos): + +- **Plugin agent primitive**: `agents/.md` + frontmatter `{name, description, tools, model}`. **Shape distinto al skill primitive** — `tools` es comma-separated string (oficial Claude Code subagent format), no YAML list `allowed-tools`. Sin campos inventados (precedente E1a `feedback_skill_primitive_minimal.md` aplicado a primitive paralelo). +- **Namespace `pos-*` obligatorio** para evitar colisión con built-in defaults de Claude Code (`code-reviewer`, `code-architect`, `Plan`, `Explore`, `general-purpose`) y con user/project agents externos. Test `test_name_uses_pos_namespace` lockea esto. +- **Fallback `general-purpose` explícito** en bodies de skills consumidoras — protege contra runtimes que no exponen agents del plugin (Claude Code antes de discovery del directorio `agents/`, entornos minimal, etc.). +- **`auditor` diferido**: no tiene consumer real hoy; entregarlo violaría regla #7 CLAUDE.md (≥2 repeticiones documentadas). Reabrir en rama dedicada (F2b o post-F4) si una skill futura lo requiere. +- **`policy.yaml.agents_allowed` diferido**: no hay enforcement consumer (Stop hook lee `skills.jsonl`, no `agents.jsonl`; no hay log de invocaciones de subagents). Documentado como deferred — reabrir cuando un hook futuro requiera enforcement. + +**Decisiones cerradas en Fase -1 (ratificadas por el usuario)**: + +- (1) **Shape primitive**: oficial Claude Code subagent format (`name` + `description` + `tools` comma-separated + `model: sonnet`); body Markdown como system prompt. Sin campos inventados. +- (2) **Scope**: 2 agents (no 3). `auditor` diferido por falta de consumer. +- (3) **Naming**: namespace `pos-*` (no override silencioso de built-ins, no nombres a secas). +- (4) **Policy**: `agents_allowed` no añadido en F2 (no enforcement consumer hoy). Sin tocar `policy.yaml`, `hooks/_lib/policy.py`, ni extender `audit-session`. +- (5) **Tests**: contract tests parametrizados por `ALLOWED_AGENTS` + behavior flips de skills consumidoras + forward-compat negation en main-strict skills (`pattern-audit` + `audit-session` no deben mencionar plugin subagents). +- (6) **Docs-sync**: ROADMAP fila F2 + sección F + nuevo bloque ✅; HANDOFF §1+§8+§9+§20; MASTER_PLAN § Rama F2 expandida; `.claude/rules/skills.md § Fork / delegación` actualiza precedentes a plugin agents; `.claude/rules/skills-map.md` añade sección "Subagents del plugin"; `docs/ARCHITECTURE.md § 6 Agents` reescrita post-revisión PR (nuevo top-level `agents/` es superficie arquitectónica del plugin aunque no esté enforced por el `pre-pr-gate` conditional). + +**Ajustes vs plan original**: + +- **Recorte v1 → v2 (decisión usuario)**: la primera Fase -1 listaba 3 agents (`code-reviewer`, `architect`, `auditor`) sin namespace + extensión de `policy.yaml.agents_allowed` + cambios en `hooks/_lib/policy.py`. El usuario rechazó: scope a 2 agents (regla #7 sobre `auditor`), namespace `pos-*` obligatorio, **no** tocar `policy.yaml` ni hooks (sin enforcement consumer hoy). El v2 aprobado es el que se implementó. +- **Hardcode literal con disclaimer** (precedente E2a A5): los bodies hardcodean `pos-code-reviewer` / `pos-architect` con fallback `general-purpose`. Una sola consumidora por agent hoy → no justifica helper runtime (regla #7). Reabrir si una segunda skill aporta repetición. + +**Resultado**: **819 passed + 1 skipped** (vs baseline F1 de 793; +26 netos del nuevo `agents/tests/test_agent_frontmatter.py` parametrizado por 2 slugs × 13 métodos. Las behavior flips de `test_skill_frontmatter.py` actualizan assertions de tests existentes — no añaden count). Sin regresión D1..D6 / E1a..E3b / F1. `stop-policy-check.py` sigue en enforcement live con `ALLOWED_SKILLS = 14` (F2 no añade skills, solo agents). El skip es el D5 intencional `TestIntegrationDiffUnavailable` por subprocess-no-cover. + +**Detalle + deferrals + ajustes**: ver [ROADMAP.md § feat/f2-agents-subagents](ROADMAP.md), [MASTER_PLAN.md § Rama F2](MASTER_PLAN.md), [.claude/rules/skills-map.md § Subagents del plugin](.claude/rules/skills-map.md), [.claude/rules/skills.md § Fork / delegación](.claude/rules/skills.md). diff --git a/MASTER_PLAN.md b/MASTER_PLAN.md index e1805bf..485c744 100644 --- a/MASTER_PLAN.md +++ b/MASTER_PLAN.md @@ -685,7 +685,46 @@ Esperar aprobación explícita del usuario. Con OK → crear marker + rama. ### Rama F2 — `feat/f2-agents-subagents` -**Scope**: `agents/code-reviewer.md`, `agents/architect.md`, `agents/auditor.md` — subagent definitions. +**Scope**: 2 plugin subagent definitions con namespace `pos-*` en `agents/` — `pos-code-reviewer.md` (consumido por `pre-commit-review`, E2a) + `pos-architect.md` (consumido por `compound`, E3a). Cierra la asimetría heredada: hasta F1 las skills consumían defaults de Claude Code (`code-reviewer`, `code-architect`); F2 los entrega como contratos canónicos del plugin con namespace `pos-*` para evitar colisión y flippea las skills consumidoras. + +**Archivos a crear/modificar**: + +- `agents/pos-code-reviewer.md` (NEW) — frontmatter primitive-correct (`name` + `description` + `tools` comma-separated string + `model: sonnet`); body declara 5 capacidades (bugs, logic, security, scope, invariants); output contract findings agrupados por severidad. Hard limits explícitos. +- `agents/pos-architect.md` (NEW) — mismo shape; body declara 3 dimensiones (pattern extraction, architectural design, cross-file consistency); output contract pattern proposals canonical-format. +- `agents/tests/test_agent_frontmatter.py` (NEW) — 26 contract tests parametrizados por `ALLOWED_AGENTS = ["pos-code-reviewer", "pos-architect"]` (13 métodos × 2 slugs). 4 clases: structure, frontmatter, body, capability surfaces. +- `.claude/skills/pre-commit-review/SKILL.md` — flip `code-reviewer` → `pos-code-reviewer` (description + body + steps + failure modes). Fallback `general-purpose` literal intacto. +- `.claude/skills/compound/SKILL.md` — flip `code-architect` → `pos-architect`. Fallback `general-purpose` intacto. +- `.claude/skills/tests/test_skill_frontmatter.py` — `TestPreCommitReviewBehavior::test_delegates_to_pos_code_reviewer` + `TestCompoundBehavior::test_body_delegates_to_pos_architect_with_fallback` flippean nombres + asertan fallback. Negation lists de `pattern-audit` + `audit-session` extendidas con `pos-*` (forward-compat). + +**Decisiones cerradas en Fase -1 (ratificadas por el usuario, v2 tras recorte de v1)**: + +- (1) **Shape primitive** — oficial Claude Code subagent format: `name` + `description` + `tools` comma-separated string + `model: sonnet`; body Markdown como system prompt. **Shape distinto al skill primitive** (skill usa YAML list `allowed-tools`; agent usa string `tools`). Sin campos inventados; precedente E1a `feedback_skill_primitive_minimal.md` aplicado. +- (2) **Scope** — 2 agents, no 3. `auditor` diferido por falta de consumer real (regla #7 CLAUDE.md: ≥2 repeticiones documentadas antes de abstraer). Reabrir en rama dedicada si una skill futura lo requiere. +- (3) **Naming** — namespace `pos-*` obligatorio. Evita colisión con built-in defaults de Claude Code (`code-reviewer`, `code-architect`, `Plan`, `Explore`, `general-purpose`) y con user/project agents externos. NO override silencioso, NO nombres a secas. +- (4) **Policy** — `agents_allowed` NO añadido en F2. Sin enforcement consumer hoy (`stop-policy-check.py` lee `skills.jsonl`, no hay log de invocaciones de subagents). Sin tocar `policy.yaml`, `hooks/_lib/policy.py`, ni extender `audit-session`. Reabrir cuando un hook futuro requiera enforcement. +- (5) **Tests** — contract tests parametrizados por `ALLOWED_AGENTS` + behavior flips de skills consumidoras + forward-compat negation en main-strict skills. +- (6) **Docs-sync** — ROADMAP, HANDOFF (§1 + §8 + §9 + §20 nuevo), MASTER_PLAN § Rama F2 (este bloque), `.claude/rules/skills.md § Fork / delegación` (precedentes a plugin agents), `.claude/rules/skills-map.md` (sección "Subagents del plugin"), `docs/ARCHITECTURE.md § 6 Agents` (reescrita post-revisión PR — el nuevo top-level `agents/` es superficie arquitectónica del plugin, aunque el `pre-pr-gate` conditional no la exija para esta rama). + +**Ajustes vs plan v1 (rechazado por el usuario)**: + +- v1 listaba 3 agents (`code-reviewer`, `architect`, `auditor`) sin namespace + extensión de `policy.yaml.agents_allowed` + cambios en `hooks/_lib/policy.py`. **Rechazado**: scope a 2 agents (regla #7 sobre `auditor`), namespace `pos-*` obligatorio, no tocar `policy.yaml` ni hooks. v2 aprobado e implementado. +- **Hardcode literal con disclaimer** (precedente E2a A5): bodies hardcodean `pos-code-reviewer` / `pos-architect` con fallback `general-purpose`. Una sola consumidora por agent hoy → no justifica helper runtime (regla #7). Reabrir si segunda repetición. + +**Contexto a leer**: + +- `MASTER_PLAN.md § Rama F1` (precedente patrón Fase F open + read-only advisory). +- `.claude/rules/skills.md § Fork / delegación` (precedentes de hardcode + fallback). +- `.claude/rules/skills-map.md` (filas `pre-commit-review` E2a + `compound` E3a — consumers actuales). +- `.claude/skills/pre-commit-review/SKILL.md` + `.claude/skills/compound/SKILL.md` (bodies a flippear). +- `policy.yaml § skills_allowed` (líneas 263-296 — no se toca, pero confirmar para argumentar deferral de `agents_allowed`). +- `hooks/_lib/policy.py` (no se toca; confirmar superficie del loader para argumentar no-extensión). + +**Criterio de salida**: **819 passed + 1 skipped** (baseline F1 793 + 26 netos del nuevo `agents/tests/test_agent_frontmatter.py` parametrizado [2 slugs × 13 métodos = 26 casos, incluyendo los 2 hardening añadidos en revisión PR: `tools`/`model` requeridos + `model == "sonnet"` lockeado]. Las behavior flips de `test_skill_frontmatter.py` actualizan assertions de tests existentes — sin delta de count.). E1a..F1 + D1..D6 regression intacta. `stop-policy-check.py` sigue en enforcement live con `ALLOWED_SKILLS = 14` (F2 no añade skills, solo agents). Docs-sync dentro del PR incluye `docs/ARCHITECTURE.md § 6 Agents` (reescrita post-revisión). El hook `pre-pr-gate.py` aprueba este mismo PR (segundo dogfooding D4 sobre Fase F). + +**Carry-overs a F3..F4**: + +- F3 (selftest): el smoke end-to-end puede dogfooding F2 invocando `pre-commit-review` y `compound` reales sobre el repo sintético; ejercita el resolution `pos-code-reviewer` / `pos-architect` con fallback `general-purpose` cuando el runtime sintético no expone agents del plugin. +- F4 (marketplace): los agents del plugin entran en el set canónico publicado; sin cambios en su shape. ### Rama F3 — `feat/f3-selftest-end-to-end` diff --git a/ROADMAP.md b/ROADMAP.md index 645a24e..9136a78 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -13,7 +13,7 @@ Estado vivo. Cada fila refleja una rama de [MASTER_PLAN.md](MASTER_PLAN.md). | E1 | Skills orquestación | ✅ (E1a + E1b) | | E2 | Skills calidad | ✅ (E2a + E2b) | | E3 | Skills patterns + tests | ✅ (E3a ✅, E3b ✅) | -| F | Audit + selftest + marketplace | 🔄 (F1 ✅, F2..F4 ⏳) | +| F | Audit + selftest + marketplace | 🔄 (F1 ✅, F2 ✅, F3..F4 ⏳) | | G | Knowledge Plane (opcional) | ⏳ solo planificación (scope cerrado, sin implementación) | ## Ramas @@ -43,7 +43,7 @@ Estado vivo. Cada fila refleja una rama de [MASTER_PLAN.md](MASTER_PLAN.md). | `feat/e3a-skill-compound-pattern-audit` | `/pos:compound` (writer-scoped pattern extraction, Agent delegation with fallback), `/pos:pattern-audit` (read-only advisory, main-strict analysis) | ✅ | #23 | | `feat/e3b-skill-test-scaffold-audit-coverage` | `/pos:test-scaffold` (writer-scoped), `/pos:test-audit` (read-only advisory), `/pos:coverage-explain` (read-only advisory); `skills_allowed` 10→13 | ✅ | — (PR pendiente) | | `feat/f1-skill-audit-session` | `/pos:audit-session` (read-only advisory main-strict) — compara 3 superficies de `policy.yaml` (`skills_allowed`, `lifecycle.*.hooks_required`, `audit.required_logs`) vs `.claude/logs/`; `skills_allowed` 13→14 | ✅ | — (PR pendiente) | -| `feat/f2-agents-subagents` | 3 subagents | ⏳ | — | +| `feat/f2-agents-subagents` | 2 plugin subagents en `agents/` con namespace `pos-*`: `pos-code-reviewer` (consumido por `pre-commit-review`), `pos-architect` (consumido por `compound`); `auditor` diferido (sin consumer real); `agents_allowed` diferido (sin enforcement consumer) | ✅ | — (PR pendiente) | | `feat/f3-selftest-end-to-end` | `bin/pos-selftest.sh` + escenarios | ⏳ | — | | `feat/f4-marketplace-public-repo` | `javiAI/pos-marketplace` + release flow | ⏳ | — | | `feat/fx-knowledge-plane-plan` | Docs-only: abre FASE G en MASTER_PLAN (capa opcional knowledge plane) | ⏳ | — | @@ -605,6 +605,40 @@ Contrato fijado por la suite (extiende E1..E3b sin reabrirlos): **Criterio de salida**: 793 verdes + 1 skip intencional. E1a..E3b + D1..D6 regression intacta. `test_real_skills_allowed_populated_by_f1` flippa el pinpoint de la tupla 13→14. `stop-policy-check.py` sigue en enforcement live sin cambio de código — sólo con allowlist ampliada. Docs-sync dentro del PR (ROADMAP § F1 + HANDOFF §1/§9 + §19 nuevo + MASTER_PLAN § Rama F1 expandida + `.claude/rules/skills-map.md` fila `/pos:audit-session` populada). `docs/ARCHITECTURE.md` **no** requerido (F1 no toca `generator/` ni `hooks/`). El hook `pre-pr-gate.py` aprueba este mismo PR (dogfooding D4 sobre F1 — primer enforcement docs-sync sobre Fase F). +### `feat/f2-agents-subagents` — ✅ (PR pendiente) + +Segunda rama de Fase F — entrega los **subagents oficiales del plugin** que cierran la asimetría "skills propias del plugin pero subagents prestados de Claude Code defaults". Tras F2, el plugin tiene primitive-correct ownership de toda su superficie (skills + hooks + agents + policy). + +Entregables: + +- `agents/pos-code-reviewer.md` — plugin subagent para branch-diff review. Consumido por `pre-commit-review` (E2a). Frontmatter primitive-correct (`name` + `description` + `tools` comma-separated + `model: sonnet`). Body declara las 5 capacidades explícitas: bugs / logic errors / security vulnerabilities / scope adherence / invariant violations. Output contract: findings agrupados por severidad (blocker/high/medium/nit), confidence-filtered, `file:line` refs. Hard limits: no `Edit`, no `Write`, no PR, no invocación de otras skills/subagents. +- `agents/pos-architect.md` — plugin subagent para pattern extraction + cross-file design. Consumido por `compound` (E3a). Frontmatter primitive-correct. Body declara 3 dimensiones: pattern extraction (≥2 repeticiones, regla #7 CLAUDE.md), architectural design (cohesión + tradeoffs), cross-file consistency. Output contract: patterns en formato canónico (Name/Context/Signal/Rule/Examples/Rationale) listo para fold en `.claude/patterns/.md`. Hard limits: no refactor, no `Edit`, no overwriting de patterns existentes. +- `.claude/skills/pre-commit-review/SKILL.md` — flip `code-reviewer` → `pos-code-reviewer` (description + body + steps + failure modes). Fallback a `general-purpose` se mantiene literal. +- `.claude/skills/compound/SKILL.md` — flip `code-architect` → `pos-architect` (description + body + steps + failure modes). Fallback a `general-purpose` intacto. +- `agents/tests/test_agent_frontmatter.py` (NEW) — 26 contract tests parametrizados por `ALLOWED_AGENTS = ["pos-code-reviewer", "pos-architect"]`: structure (file exists + parses) + frontmatter (todos los 4 keys requeridos `{name, description, tools, model}`, name match filename, namespace `pos-*`, description non-empty, tools comma-separated string con validación de tokens no vacíos y sin whitespace antes de `(...)`, model valid `{sonnet, opus, haiku}` + `model == "sonnet"` lockeado por F2 Fase -1 (1)) + body substantive (>100 chars) + capability surfaces (bug/security/scope/invariant para `pos-code-reviewer`; pattern/design/cross-file consistency para `pos-architect`). +- `.claude/skills/tests/test_skill_frontmatter.py` — `TestPreCommitReviewBehavior::test_delegates_to_pos_code_reviewer` + `TestCompoundBehavior::test_body_delegates_to_pos_architect_with_fallback` flippean a los nuevos nombres + asertan literalmente fallback `general-purpose`. `pattern-audit` + `audit-session` negation lists incluyen ahora `pos-architect` / `pos-code-reviewer` (forward-compat: main-strict skills nunca deben referenciar plugin subagents). + +Suite global post-F2: **819 passed + 1 skipped** (baseline F1: 793; +26 contract cases del nuevo `agents/tests/test_agent_frontmatter.py`. Las behavior flips de `test_skill_frontmatter.py` son renames/updates de tests existentes — sin delta de count). Sin regresión D1..D6 / E1a..E3b / F1. + +Contrato fijado por la suite (extiende E1..F1 sin reabrirlos): + +- Plugin agent primitive: `agents/.md` + frontmatter `{name, description, tools, model}` (oficial Claude Code subagent shape — `tools` es comma-separated string, **shape distinto al skill primitive** que usa YAML list `allowed-tools`). **No campos inventados**. +- Namespace obligatorio `pos-*` para evitar colisión con built-in defaults de Claude Code (`code-reviewer`, `code-architect`, `Plan`, `Explore`, `general-purpose`) y con user/project agents externos. +- Hardcode del nombre `pos-code-reviewer` / `pos-architect` en los bodies de las skills consumidoras **con fallback explícito a `general-purpose`** — protege contra runtimes que no exponen agents del plugin (p.ej. Claude Code antes de discovery del directorio `agents/`). +- `auditor` **diferido**: no tiene consumer real hoy; entregarlo violaría regla #7 CLAUDE.md (≥2 repeticiones documentadas). Reabrir en rama dedicada (F2b o post-F4) si una skill futura lo requiere. +- `policy.yaml.agents_allowed` **diferido**: no hay enforcement consumer (Stop hook lee `skills.jsonl`, no agents.jsonl; no hay log de invocaciones de subagents). Documentado como deferred — reabrir cuando un hook futuro requiera enforcement. + +**Decisiones cerradas en Fase -1 (ratificadas por el usuario)**: + +- (1) Shape primitive: oficial Claude Code subagent format (`name`/`description`/`tools` comma-separated/`model`); body Markdown como system prompt; `model: sonnet` para ambos. Sin campos inventados (precedente E1a `feedback_skill_primitive_minimal.md` aplicado a primitive paralelo). +- (2) Scope: 2 agents (no 3). `auditor` diferido por falta de consumer (regla #7). +- (3) Naming: namespace `pos-*` (no override silencioso de built-ins, no nombres a secas). +- (4) Policy: `agents_allowed` no añadido en F2 (no enforcement consumer hoy). +- (5) Tests: contract tests parametrizados + behavior flips de skills consumidoras + forward-compat negation en main-strict skills. +- (6) Docs-sync: ROADMAP + HANDOFF + MASTER_PLAN § F2 + `.claude/rules/skills.md` + `.claude/rules/skills-map.md` + `docs/ARCHITECTURE.md` (nuevo top-level `agents/` justifica sub-sección aunque no esté enforced por el pre-PR gate). + +**Criterio de salida**: 819 verdes + 1 skip intencional. Sin regresión sobre F1. Docs-sync completo dentro del PR (incluye `docs/ARCHITECTURE.md § 6 Agents` reescrita post-revisión). `pre-pr-gate.py` aprueba este mismo PR — el conditional `skills/**` (porque tocamos dos `SKILL.md`) exige `skills-map.md`, satisfecho. `agents/**` no está en `policy.yaml.lifecycle.pre_pr.docs_sync_conditional` hoy (drift abierto deliberadamente — reabrir cuando un consumer enforcement justifique extender el gate). + ## Convenciones de este archivo - Una fila por rama. `⏳` pendiente, `🔄` en vuelo, `✅` completada, `❌` abandonada, `🚫` bloqueada. diff --git a/agents/pos-architect.md b/agents/pos-architect.md new file mode 100644 index 0000000..51dea24 --- /dev/null +++ b/agents/pos-architect.md @@ -0,0 +1,45 @@ +--- +name: pos-architect +description: Plugin subagent for architectural pattern extraction and cross-file design analysis. Invoked by compound (E3a) over a merged branch diff to identify reusable patterns, design repetitions, and cross-file consistency issues. Returns pattern proposals; does not refactor, edit, or open PRs. +tools: Read, Grep, Glob, Bash +model: sonnet +--- + +# pos-architect + +You are an architecture-focused subagent owned by the `pos` plugin. The skill `compound` invokes you with a merged branch diff (`git diff main...HEAD`), the list of existing patterns in `.claude/patterns/`, and a request to propose 1–3 reusable patterns. You analyze through an architectural lens; you do not refactor code, do not edit files, do not open PRs. + +## Capability surface + +You analyze diffs across three explicit dimensions: + +1. **Pattern extraction** — identify code or design repetitions that occur ≥2 times in the diff and would reduce future duplication if captured as a reusable pattern. Each pattern proposal should be concrete enough that a future contributor can recognize the trigger ("Signal") and apply the rule ("Rule") without re-deriving the design. +2. **Architectural design** — evaluate whether the diff coheres around a deliberate architectural decision (layering, dependency direction, separation of concerns) or whether it accretes ad-hoc choices that future maintenance will pay for. Surface design tradeoffs the orchestrating skill should flag. +3. **Cross-file consistency** — check that related changes across multiple files agree on naming, contracts, error handling, and invariants. A subtle inconsistency between a producer file and a consumer file is the kind of issue this dimension exists to catch. + +## Output contract + +For each pattern you propose, return a structured object that the orchestrating skill can fold directly into `.claude/patterns/.md` using its canonical format: + +- **Name** — short kebab-case identifier (the filename slug). +- **Context** — when this pattern applies; preconditions. +- **Signal** — observable markers that indicate the pattern should be considered. +- **Rule** — the actual idiom or abstraction (cite code verbatim from the diff, with `file:line` references). +- **Examples** — `file:line` pointers in the diff that demonstrate the repetition. +- **Rationale** — a one-paragraph explanation of why this pattern is worth capturing now (the regla #7 evidence: ≥2 repetitions, with reasoning on why a third occurrence would benefit). + +If you find no extractable pattern in this diff, say so explicitly. Silence is ambiguous; an explicit "no patterns" output lets the skill log `status: partial` and stop cleanly. + +## Hard limits + +- You do not refactor code. Pattern extraction is documentation, not modification. +- You do not edit files. You do not call `Edit` or `Write`. The orchestrating skill writes `.claude/patterns/*.md`. +- You do not modify existing patterns. Proposed patterns that overlap with existing entries are flagged for the orchestrator to handle (merge vs new), not silently overwritten. +- You do not open or describe PRs. +- You do not invoke other skills or subagents. + +## Failure modes + +- Diff empty → no patterns; say so and stop. +- Diff exhibits no clear repetition (≥2 occurrences) → explicitly return no patterns; do not invent forced abstractions just to fill the slot. Premature abstraction is worse than duplication (regla #7 CLAUDE.md). +- A candidate pattern conflicts with an existing entry in `.claude/patterns/` → flag the conflict, identify both candidates, and let the orchestrator decide merge vs new. diff --git a/agents/pos-code-reviewer.md b/agents/pos-code-reviewer.md new file mode 100644 index 0000000..eb94125 --- /dev/null +++ b/agents/pos-code-reviewer.md @@ -0,0 +1,45 @@ +--- +name: pos-code-reviewer +description: Plugin subagent for branch-diff code review. Invoked by pre-commit-review (E2a) over git diff main...HEAD to surface bugs, logic errors, security issues, scope drift, and invariant violations. Returns confidence-filtered findings; does not edit, write, or open PRs. +tools: Read, Grep, Glob, Bash +model: sonnet +--- + +# pos-code-reviewer + +You are a code reviewer subagent owned by the `pos` plugin. The skill `pre-commit-review` invokes you with a prepared context (branch kickoff, applicable invariants from `.claude/rules/*.md`) and the full branch diff (`git diff main...HEAD`). You return prioritized findings; you never edit files, never apply fixes, never open PRs. + +## Capability surface + +You analyze a branch diff against five explicit asks: + +1. **Bugs** — incorrect behavior, off-by-one errors, null/undefined handling, race conditions, resource leaks, error-path defects. +2. **Logic errors** — control-flow mistakes, wrong conditions, mishandled edge cases, dead branches, contradictory invariants. +3. **Security vulnerabilities** — injection (SQL / shell / XSS), unsafe deserialization, secret leakage, missing authn/authz checks, unsafe defaults, OWASP Top 10 patterns relevant to the touched paths. +4. **Scope adherence** — does the diff stay inside the branch's declared scope (per the kickoff commit + `MASTER_PLAN.md § Rama `)? Flag drift, opportunistic refactors, unrelated changes. +5. **Invariant violations** — does any change violate a path-scoped rule cited in `.claude/rules/*.md`, or a non-negotiable in `CLAUDE.md` (e.g., regla #3 tests-first, regla #7 patrones antes de abstraer)? + +## Output contract + +Group findings by severity: **blocker / high / medium / nit**. Confidence-filter: omit findings below medium confidence — false positives waste reviewer attention. Each finding includes: + +- `file:line` reference (so the orchestrator skill can fold them into the conversation). +- One-sentence statement of the issue. +- One-sentence rationale (why it matters / which invariant or capability is at stake). +- Optional: a one-line suggested fix as **suggestion only** — never a patch. + +If you find nothing in a bucket, say so explicitly (e.g., "Security: no findings"). Silence is ambiguous; explicit empty buckets aren't. + +## Hard limits + +- You do not edit files. You do not call `Edit` or `Write`. +- You do not open or describe PRs. The `gh` CLI and PR body are the user's call (or `/pos:pr-description` in Fase F). +- You do not create branch approval markers. +- You do not invoke other skills or other subagents. +- You do not run tests or coverage tools — your input is the diff + the prepared context, nothing else. + +## Failure modes + +- Diff empty → say so and stop. The orchestrator skill will short-circuit. +- Diff so large it exceeds your reasoning budget → triage by file (highest-risk first) and declare what you did not review. +- A finding requires reading more code than the diff to confirm → fetch with `Read` / `Grep` if the path is in the repo, otherwise mark the finding `low confidence` and explain. diff --git a/agents/tests/test_agent_frontmatter.py b/agents/tests/test_agent_frontmatter.py new file mode 100644 index 0000000..941f5e9 --- /dev/null +++ b/agents/tests/test_agent_frontmatter.py @@ -0,0 +1,237 @@ +"""Contract tests for the Claude Code Plugin Agent primitive used by pos (F2). + +Scope: + - Frontmatter shape (official primitive: name + description + tools + model). + - No invented fields beyond {name, description, tools, model}. + - `name` matches the filename without `.md`. + - description non-empty. + - tools is REQUIRED and is a comma-separated string (per official subagent docs). + - model is REQUIRED and one of the supported values; F2 agents declare `sonnet`. + - Body (system prompt) is substantive. + +F2 Fase -1 ratified decisions: + - 2 agents only: pos-code-reviewer + pos-architect (auditor deferred — no + consumer hoy, regla #7 CLAUDE.md). + - Namespace `pos-*` to avoid collision with built-in / user / project agents. + - No `agents_allowed` in policy.yaml in F2 (no enforcement consumer yet). + +Capability surfaces locked here are mirrored by the skills that delegate to +each agent: + - pre-commit-review (E2a) → pos-code-reviewer (bugs / security / scope / invariants). + - compound (E3a) → pos-architect (patterns / design / cross-file consistency). +""" +from __future__ import annotations + +from pathlib import Path + +import pytest +import yaml + +REPO_ROOT = Path(__file__).resolve().parents[2] +AGENTS_DIR = REPO_ROOT / "agents" + +ALLOWED_AGENTS = ["pos-code-reviewer", "pos-architect"] + +ALLOWED_FRONTMATTER_KEYS = {"name", "description", "tools", "model"} +REQUIRED_FRONTMATTER_KEYS = {"name", "description", "tools", "model"} +VALID_MODELS = {"sonnet", "opus", "haiku"} +F2_REQUIRED_MODEL = "sonnet" + + +def read_agent(slug: str) -> tuple[dict, str]: + """Return `(frontmatter, body)` for `agents/.md`. + + Raises `AssertionError` with a clear reason if the shape is wrong — this + is a contract test, explicit failure beats KeyError. + """ + path = AGENTS_DIR / f"{slug}.md" + assert path.exists(), f"Agent file missing: {path}" + raw = path.read_text(encoding="utf-8") + parts = raw.split("---\n", 2) + assert len(parts) >= 3 and parts[0] == "", ( + f"Agent {slug}.md does not start with `---\\n\\n---\\n` " + f"frontmatter (got: {parts[0]!r}...)" + ) + frontmatter = yaml.safe_load(parts[1]) or {} + body = parts[2] + assert isinstance(frontmatter, dict), ( + f"Agent {slug} frontmatter must parse as a YAML mapping, " + f"got {type(frontmatter).__name__}" + ) + return frontmatter, body + + +# ──────────────────────────────────────────────────────────────────────────── +# Structure: file exists and parses as frontmatter + body +# ──────────────────────────────────────────────────────────────────────────── + + +class TestStructure: + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_agent_file_exists(self, slug: str): + assert (AGENTS_DIR / f"{slug}.md").is_file(), ( + f"Agent file `agents/{slug}.md` missing — F2 declares 2 agents " + f"in ALLOWED_AGENTS." + ) + + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_agent_md_parses(self, slug: str): + fm, body = read_agent(slug) + assert fm + assert body.strip() + + +# ──────────────────────────────────────────────────────────────────────────── +# Frontmatter contract: name / description / tools / model only +# ──────────────────────────────────────────────────────────────────────────── + + +class TestFrontmatter: + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_frontmatter_keys_subset(self, slug: str): + fm, _ = read_agent(slug) + extra = set(fm.keys()) - ALLOWED_FRONTMATTER_KEYS + assert not extra, ( + f"Agent {slug} frontmatter has unexpected keys: {sorted(extra)}. " + f"Allowed (per official Claude Code subagent docs): " + f"{sorted(ALLOWED_FRONTMATTER_KEYS)}. F2 Fase -1 ratified: no " + f"invented fields." + ) + + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_required_keys_present(self, slug: str): + fm, _ = read_agent(slug) + missing = REQUIRED_FRONTMATTER_KEYS - set(fm.keys()) + assert not missing, ( + f"Agent {slug} missing required frontmatter keys: {sorted(missing)}" + ) + + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_name_matches_filename(self, slug: str): + fm, _ = read_agent(slug) + assert fm.get("name") == slug, ( + f"Agent {slug} frontmatter name={fm.get('name')!r} must match " + f"filename `{slug}.md` (convention)." + ) + + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_name_uses_pos_namespace(self, slug: str): + fm, _ = read_agent(slug) + name = fm.get("name", "") + assert isinstance(name, str) and name.startswith("pos-"), ( + f"Agent {slug} name={name!r} must use `pos-*` namespace to avoid " + f"collision with built-in / user / project agents (F2 decision)." + ) + + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_description_non_empty_string(self, slug: str): + fm, _ = read_agent(slug) + desc = fm.get("description", "") + assert isinstance(desc, str) and desc.strip(), ( + f"Agent {slug} description must be non-empty string" + ) + + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_tools_is_comma_separated_string(self, slug: str): + fm, _ = read_agent(slug) + tools = fm["tools"] + assert isinstance(tools, str), ( + f"Agent {slug} tools must be a comma-separated string per " + f"official Claude Code subagent docs, got " + f"{type(tools).__name__}. (Skill primitive uses YAML list, but " + f"agent primitive uses comma-separated string — different shapes.)" + ) + assert tools.strip(), f"Agent {slug} tools must be non-empty" + tokens = [t.strip() for t in tools.split(",")] + assert all(tokens), ( + f"Agent {slug} tools={tools!r} has empty token after split — " + f"comma-separated format requires non-empty tokens between commas." + ) + assert all(" " not in t.split("(", 1)[0] for t in tokens), ( + f"Agent {slug} tools={tools!r} contains a token with whitespace " + f"in its tool name (before any `(...)` scope). Format must be " + f"`Tool1, Tool2, Bash(cmd:*)` — comma-separated, not space-separated." + ) + + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_model_valid(self, slug: str): + fm, _ = read_agent(slug) + model = fm["model"] + assert model in VALID_MODELS, ( + f"Agent {slug} model={model!r} must be one of " + f"{sorted(VALID_MODELS)}" + ) + + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_model_is_sonnet_for_f2(self, slug: str): + """F2 Fase -1 decision (1) ratified `model: sonnet` for both agents. + Lock that decision here so a future drift to opus/haiku is explicit.""" + fm, _ = read_agent(slug) + assert fm["model"] == F2_REQUIRED_MODEL, ( + f"Agent {slug} model={fm['model']!r} must be " + f"{F2_REQUIRED_MODEL!r} per F2 Fase -1 decision (1). Bumping to " + f"opus/haiku requires a new branch + ratification." + ) + + +# ──────────────────────────────────────────────────────────────────────────── +# Body: system prompt is substantive +# ──────────────────────────────────────────────────────────────────────────── + + +class TestBody: + @pytest.mark.parametrize("slug", ALLOWED_AGENTS) + def test_body_substantive(self, slug: str): + _, body = read_agent(slug) + assert len(body.strip()) > 100, ( + f"Agent {slug} body must be substantive (>100 chars); body acts " + f"as the system prompt for the subagent." + ) + + +# ──────────────────────────────────────────────────────────────────────────── +# Capability contracts: lock down what each agent declares it can do +# ──────────────────────────────────────────────────────────────────────────── + + +class TestCodeReviewerCapability: + """pos-code-reviewer is consumed by pre-commit-review (E2a). Body must + document the capability surface the skill delegates to.""" + + def test_body_mentions_review_capabilities(self): + _, body = read_agent("pos-code-reviewer") + low = body.lower() + for keyword in ("bug", "security", "scope", "invariant"): + assert keyword in low, ( + f"pos-code-reviewer body must mention `{keyword}` as part " + f"of its review capability surface (consumed by " + f"pre-commit-review)." + ) + + +class TestArchitectCapability: + """pos-architect is consumed by compound (E3a). Body must document the + architecture/pattern analysis surface the skill delegates to.""" + + def test_body_mentions_pattern_capability(self): + _, body = read_agent("pos-architect") + assert "pattern" in body.lower(), ( + "pos-architect body must mention `pattern` (consumed by compound " + "for code pattern extraction)." + ) + + def test_body_mentions_design_or_architecture(self): + _, body = read_agent("pos-architect") + low = body.lower() + assert "design" in low or "architectur" in low, ( + "pos-architect body must mention `design` or `architectur` " + "(architectural lens)." + ) + + def test_body_mentions_cross_file_consistency(self): + _, body = read_agent("pos-architect") + low = body.lower() + assert "cross-file" in low or "cross file" in low or "consistency" in low, ( + "pos-architect body must reference cross-file consistency " + "analysis (reviewing design decisions across files)." + ) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9532432..cf8ab8b 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -391,13 +391,31 @@ E1a flippa de "Ausente" a "Lista poblada" sin tocar código del hook — puro ca `$ARGUMENTS`, `$1..$N`, `${CLAUDE_SESSION_ID}`, `${CLAUDE_SKILL_DIR}`, `` !`command` `` (shell injection, requiere `shell: true` en frontmatter). -## 6. Agents (subagents) +## 6. Agents (plugin-owned subagents) -- `agents/code-reviewer.md` — review de diffs. -- `agents/architect.md` — decisiones arquitectónicas (branch-plan, deep-interview, compound). -- `agents/auditor.md` — audit de plugins, policy check, session audit. +Top-level superficie del plugin: `agents/.md`. Cada archivo es un subagent definido en el primitive oficial de Claude Code (`name` + `description` + `tools` + `model`); el body Markdown actúa como system prompt. Los skills consumidoras los invocan vía `Agent(subagent_type="", ...)` con fallback a `general-purpose` si el runtime no los expone. -Cada agent declara sus tools permitidas (`allowed-tools:`) — subset mínimo. Un agente reviewer no debe tener `Write`. Un auditor no debe tener `Bash(git push)`. +**Shape distinto al skill primitive** (Fase F2): + +| Aspecto | Skill (`.claude/skills//SKILL.md`) | Agent (`agents/.md`) | +|---|---|---| +| Tools | `allowed-tools:` YAML list | `tools:` comma-separated string | +| Modelo | No campo en frontmatter (corre en orchestrator) | `model:` requerido (sonnet por defecto en F2) | +| Body | Body es la skill instructions | Body es el system prompt del subagent | + +**Namespace `pos-*` obligatorio** para evitar colisión con built-in defaults de Claude Code (`code-reviewer`, `code-architect`, `Plan`, `Explore`, `general-purpose`) y con user/project agents externos. Validado por `agents/tests/test_agent_frontmatter.py::TestFrontmatter::test_name_uses_pos_namespace`. + +**Agents entregados (F2)**: + +- `agents/pos-code-reviewer.md` — branch-diff review (bugs / logic / security / scope / invariants). Consumido por `pre-commit-review` (E2a). `tools: Read, Grep, Glob, Bash`. Hard limits: no `Edit`, no `Write`, no PR. +- `agents/pos-architect.md` — pattern extraction + cross-file design. Consumido por `compound` (E3a). `tools: Read, Grep, Glob, Bash`. Hard limits: no refactor, no overwriting de patterns existentes. + +**Diferidos en F2 (regla #7 CLAUDE.md — ≥2 repeticiones antes de abstraer)**: + +- `pos-auditor` — sin consumer real hoy. Reabrir cuando una skill futura lo requiera. +- `policy.yaml.agents_allowed` — sin enforcement consumer (Stop hook lee `skills.jsonl`, no hay log de invocaciones de subagents). Reabrir cuando un hook futuro requiera enforcement. + +**Forward-compat negation**: skills main-strict (`pattern-audit` E3a, `audit-session` F1) **nunca** referencian plugin subagents — los tests `test_skill_frontmatter.py` lockean esto vía negation lists. ## 7. Determinismo — tres capas