feat(f2): plugin subagents pos-code-reviewer + pos-architect#26
Conversation
Fase 0 + Fase 1 RED-first de feat/f2-agents-subagents. Scope ratificado en Fase -1: - 2 plugin subagents en agents/: pos-code-reviewer + pos-architect. - auditor diferido (no consumer real hoy; regla #7 CLAUDE.md). - Namespace pos-* para evitar colisión con built-in/user/project agents. - NO tocar policy.yaml (sin agents_allowed; deferred sin consumer). - NO tocar hooks/_lib/policy.py. - NO extender audit-session. Shape primitive (oficial Claude Code subagent): - frontmatter: name + description + tools (comma-separated string) + model. - body Markdown como system prompt. - model: sonnet para ambos. - Sin campos inventados. Files RED en este commit: - agents/tests/test_agent_frontmatter.py (NEW) — 24 RED tests: * Structure (2): file exists + parses * Frontmatter (14): keys subset, required, name match filename, namespace pos-*, description non-empty, tools comma-separated string, model valid * Body (2): substantive (>100 chars) * Capabilities (4): pos-code-reviewer menciona bug/security/scope/invariant; pos-architect menciona pattern/design/cross-file consistency. - .claude/skills/tests/test_skill_frontmatter.py (MODIFIED): * TestPreCommitReviewBehavior::test_delegates_to_code_reviewer renombrado a test_delegates_to_pos_code_reviewer + asserta general-purpose fallback. * TestCompoundBehavior::test_body_declares_fallback renombrado a test_body_delegates_to_pos_architect_with_fallback (asserta pos-architect literal + general-purpose fallback). * pattern-audit + audit-session negation lists incluyen pos-architect / pos-code-reviewer (forward-compat: main-strict skills nunca deben referenciar plugin subagents). Confirmado RED: 26 fail (24 agents + 2 skills flips) + 791 pass + 1 skip. Regression intacta sobre E1a/E1b/E2a/E2b/E3a/E3b/F1. Próximo: Fase 2 GREEN — escribir agents/pos-code-reviewer.md + agents/pos-architect.md + actualizar bodies de pre-commit-review/SKILL.md + compound/SKILL.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Fase 2 implementación que satisface los 24 RED tests + 2 skill flips. agents/pos-code-reviewer.md — plugin subagent para branch-diff review. Frontmatter primitive-correct (name + description + tools comma-separated + model: sonnet, sin campos inventados). Body declara las 5 capacidades explícitas que pre-commit-review delega: 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. Frontmatter primitive-correct. Body declara 3 dimensiones: pattern extraction (≥2 repeticiones, regla #7), 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/<slug>.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. subagent_type / agent tool refs intactos. .claude/skills/compound/SKILL.md — flip code-architect → pos-architect (description + body + steps + failure modes). Fallback a general-purpose intacto. Pattern format + STOP signal + writer-scoped strict intactos. Resultado: 817 passed + 1 skipped (baseline 791 + 26 nuevos: 24 agents contract + 2 skill flips). Zero regression sobre E1a/E1b/E2a/E2b/E3a/ E3b/F1. Próximo: Fase N+1 simplify pre-PR + Fase N+3 docs-sync (ROADMAP/HANDOFF/ MASTER_PLAN/skills.md/skills-map.md/ARCHITECTURE.md). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…bagents Cierre de Fase N+3 docs-sync de F2: documenta los plugin subagents `pos-code-reviewer` + `pos-architect` (entregados en commits previos e63f438 RED + 2a5396b GREEN) en todas las superficies de docs del meta-repo. Cero cambios de código en este commit — sólo prosa. ROADMAP.md - Fila F status flippea a `🔄 (F1 ✅, F2 ✅, F3..F4 ⏳)`. - Fila F2 `⏳` → `✅ | — (PR pendiente)` con scope completo. - Sección `### feat/f2-agents-subagents — ✅ (PR pendiente)` nueva antes de "## Convenciones" — entregables, suite stats (817+1), contrato fijado, 6 decisiones Fase -1 ratificadas, exit criterion. HANDOFF.md - §1 snapshot: rama actual F2 ✅ PR pendiente, anterior F1 ✅ #25, siguiente F3 — `feat/f3-selftest-end-to-end`. - §8 reframe: built-in subagents → plugin agents `pos-*` críticos (consumed by pre-commit-review + compound), built-in residuales sólo para branch-plan reads. - §9 Próxima rama: F2 → F3 (selftest end-to-end). - §20 Estado F2 NEW: entregables + contrato + decisiones + ajustes + resultado 817+1. MASTER_PLAN.md - § Rama F2 expandida: scope 2 agents pos-*, archivos a crear/ modificar, 6 decisiones cerradas, ajustes v1→v2 (recorte de 3→2 + namespace + no policy), contexto a leer, criterio de salida, carry-overs F3/F4. .claude/rules/skills.md - § Fork / delegación: precedentes flippean a `pos-code-reviewer` + `pos-architect` (post-F2) con disclaimer de fallback `general- purpose`. Plugin agent primitive box explícito (shape distinto al skill: `tools` comma-separated string vs YAML list `allowed- tools`). Forward-compat negation list extendida con `pos-*`. - Behavior tests row: refleja flips F2 + negation lists extendidas. .claude/rules/skills-map.md - Filas `pre-commit-review` y `compound`: subagent_type flippea a `pos-code-reviewer` / `pos-architect` (post-F2), fallback `general-purpose` literal. - Sección "## Subagents del plugin (entregado en F2)" NEW: tabla con los 2 agents + contract tests + fallback + diferidos (`pos-auditor`, `agents_allowed`) + forward-compat negation. Resultado: 817 passed + 1 skipped (sin cambios al code; suite intacta). E1a..F1 + D1..D6 regression intacta. `docs/ARCHITECTURE.md` NO modificado (decisión Fase -1 ratificada: F2 no toca `generator/` ni `hooks/`; pre-pr-gate conditional no exige sync). Próximo: simplify pre-PR + pre-commit-review (dogfooding F2 sobre sí mismo: pre-commit-review delega a pos-code-reviewer) + gh pr create. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…aimer El disclaimer de Delegation rezaba "The string `pos-code-reviewer` here reflects the Claude Code default shipped today" — wording correcto cuando apuntaba a `code-reviewer` (built-in de Claude Code) pero stale post-flip F2: `pos-code-reviewer` es plugin agent, no default de Claude Code. Reescrito para reflejar la realidad post-F2: "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." Sin cambios de comportamiento (la fallback rule a `general-purpose` queda literal). Sin cambios al contrato de tests. Suite 817 passed + 1 skipped. Writer-scoped strict: archivo dentro del diff `git diff --name-only main..HEAD`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds first-class, plugin-owned Claude Code subagents under agents/ (namespaced pos-*) and updates the existing consumer skills/docs/tests to delegate to them (with the existing general-purpose fallback intact).
Changes:
- Introduces
agents/pos-code-reviewer.mdandagents/pos-architect.md(frontmatter + system-prompt contracts). - Flips delegation in
pre-commit-reviewandcompoundfrom Claude Code defaults to the newpos-*agents, and updates skill behavior tests/negation lists accordingly. - Adds contract tests for the agent primitive and updates docs-sync artifacts to describe the new agent surface.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
agents/pos-code-reviewer.md |
New plugin subagent definition for branch diff review. |
agents/pos-architect.md |
New plugin subagent definition for pattern/design analysis. |
agents/tests/test_agent_frontmatter.py |
New contract tests for agent frontmatter/body capability surfaces. |
.claude/skills/pre-commit-review/SKILL.md |
Delegation updated to pos-code-reviewer + documents fallback. |
.claude/skills/compound/SKILL.md |
Delegation updated to pos-architect + keeps fallback. |
.claude/skills/tests/test_skill_frontmatter.py |
Behavior tests updated for pos-* delegation + main-strict negation extended. |
.claude/rules/skills.md |
Documents plugin-owned subagents + delegation conventions post-F2. |
.claude/rules/skills-map.md |
Updates skill map entries and adds a new “Subagents del plugin (F2)” section. |
ROADMAP.md |
Marks F2 as complete and documents deliverables/contract. |
MASTER_PLAN.md |
Updates F2 branch scope/decisions and deliverables to match implementation. |
HANDOFF.md |
Updates snapshot/next-branch guidance and records F2 state/details. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --- | ||
| 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 |
There was a problem hiding this comment.
The agent is granted the Bash tool in frontmatter. If the subagent is intended to work purely from the diff + provided context (and not execute commands), consider removing Bash (or scoping it down if supported) to keep the tool surface minimal and avoid unnecessary command execution capability.
| tools: Read, Grep, Glob, Bash | |
| tools: Read, Grep, Glob |
There was a problem hiding this comment.
No aplicado en este PR — mismo razonamiento que en pos-code-reviewer.md:4. La decisión Fase -1 (1) ratificada fija el tool surface; reducirlo es un cambio de contrato fuera del scope de esta PR ("No cambiar nada más de scope"). El body declara hard limits explícitos (no refactor, no Edit, no overwriting de patterns). Bash se mantiene para introspección git read-only del merged diff cuando el subagent extraiga patrones. Reabrir en rama separada si una review futura aporta señal de uso indebido.
| - (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). |
There was a problem hiding this comment.
This docs-sync bullet lists docs/ARCHITECTURE.md as part of the F2 docs deliverables, but this PR doesn’t modify that file and earlier in the same section it’s described as not required. Please either (a) remove docs/ARCHITECTURE.md from this docs-sync list, or (b) actually update docs/ARCHITECTURE.md in this PR so the documentation matches what’s claimed here.
| - (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). | |
| - (6) Docs-sync: ROADMAP + HANDOFF + MASTER_PLAN § F2 + `.claude/rules/skills.md` + `.claude/rules/skills-map.md`. |
There was a problem hiding this comment.
Applied as option (b) in commit f230320: docs/ARCHITECTURE.md § 6 Agents reescrita end-to-end (estaba stale — listaba code-reviewer/architect/auditor sin namespace, usaba allowed-tools: que es shape de skill, no de agent). Decisión Fase -1 (6) ahora consistente entre ROADMAP y MASTER_PLAN.
| **Contrato fijado por la suite** (extiende E1..F1 sin reabrirlos): | ||
|
|
||
| - **Plugin agent primitive**: `agents/<slug>.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_is_pos_prefixed` lockea esto. |
There was a problem hiding this comment.
This references a test name test_name_is_pos_prefixed, but in agents/tests/test_agent_frontmatter.py the namespace check is implemented as test_name_uses_pos_namespace. Please update this doc reference so it matches the actual test name (or rename the test if the doc name is intended to be canonical).
| - **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_is_pos_prefixed` lockea esto. | |
| - **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. |
There was a problem hiding this comment.
Fixed in f230320: doc reference actualizada a test_name_uses_pos_namespace (que es el nombre real del test en agents/tests/test_agent_frontmatter.py).
| @pytest.mark.parametrize("slug", ALLOWED_AGENTS) | ||
| def test_tools_is_comma_separated_string(self, slug: str): | ||
| fm, _ = read_agent(slug) | ||
| if "tools" not in fm: | ||
| return # tools is optional | ||
| 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, when present, must be non-empty" | ||
| ) | ||
|
|
||
| @pytest.mark.parametrize("slug", ALLOWED_AGENTS) | ||
| def test_model_valid_if_present(self, slug: str): | ||
| fm, _ = read_agent(slug) | ||
| if "model" not in fm: | ||
| return | ||
| model = fm["model"] | ||
| assert model in VALID_MODELS, ( | ||
| f"Agent {slug} model={model!r} must be one of " | ||
| f"{sorted(VALID_MODELS)}" | ||
| ) |
There was a problem hiding this comment.
test_tools_is_comma_separated_string currently accepts any non-empty string and doesn’t actually validate the comma-separated format (e.g., a space-separated string would pass). Also, the file-level contract text says the official primitive is name + description + tools + model, but the tests treat tools/model as optional via early-return; please either enforce them as required (add to REQUIRED_FRONTMATTER_KEYS) or update the contract wording/tests to be consistent about optionality.
There was a problem hiding this comment.
Both points fixed in f230320:
tools+modelmovidos aREQUIRED_FRONTMATTER_KEYS(no más early-return optional). Contract wording en docstring actualizado para reflejar required.test_tools_is_comma_separated_stringahora valida tokens: split por,, asserts non-empty + sin whitespace en el nombre antes de(...)— un space-separated string ahora falla.- NUEVO
test_model_is_sonnet_for_f2lockea decisión Fase -1 (1)model: sonnet. Bumpear a opus/haiku exige rama nueva + ratificación. Suite +2 → 819 passed.
| --- | ||
| 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 |
There was a problem hiding this comment.
The agent is granted the Bash tool in frontmatter. Given this subagent’s contract says its input is the prepared context + diff and it must not run tests/other commands, consider removing Bash (or scoping it down if the primitive supports scoping) to keep the agent tool surface minimal and reduce unnecessary execution capability.
| tools: Read, Grep, Glob, Bash | |
| tools: Read, Grep, Glob |
There was a problem hiding this comment.
No aplicado en este PR. La decisión Fase -1 (1) ratificada por el usuario fija el set de tools (Read, Grep, Glob, Bash); reducir el surface es un cambio de contrato que requiere ratificación explícita y no entra en la directiva del usuario para esta PR ("No cambiar nada más de scope"). El subagent body ya declara hard limits explícitos (no Edit, no Write, no PR, no invocación de otras skills/subagents) — Bash queda disponible para introspección read-only del diff vía git que el subagent puede necesitar al revisar. Reabrir en rama separada si una review futura aporta señal de uso indebido.
…md § 6 Atiende los 2 ajustes solicitados por el usuario sobre PR #26 (más un fix trivial de un test name reference flaggeado por Copilot): 1. Hardening del contract de agent primitive `agents/tests/test_agent_frontmatter.py`: - `tools` y `model` movidos a `REQUIRED_FRONTMATTER_KEYS` (antes opcionales vía early-return — dejaba el contrato flojo para drift). - `test_tools_is_comma_separated_string` ahora valida tokens (split por `,`, asserts non-empty + sin whitespace en el nombre antes de `(...)` — antes un space-separated string pasaba el test). - `test_model_valid_if_present` → `test_model_valid` (sin early-return). - NUEVO `test_model_is_sonnet_for_f2` parametrizado por agent: lockea decisión Fase -1 (1) `model: sonnet`. Bumpear a opus/haiku exige rama nueva + ratificación. 2. `docs/ARCHITECTURE.md § 6 Agents` reescrita La sección era stale (listaba `code-reviewer` / `architect` / `auditor` sin namespace, usaba `allowed-tools:` que es shape de skill, no de agent). Ahora documenta: - top-level superficie `agents/<slug>.md` con primitive oficial (`name` / `description` / `tools` comma-separated / `model`); - tabla shape skill vs agent (divergencia esencial post-F2); - namespace `pos-*` obligatorio + test que lo lockea; - 2 agents entregados + diferidos (auditor + agents_allowed) + forward-compat negation. 3. Trivial: `HANDOFF.md` test name reference `test_name_is_pos_prefixed` → `test_name_uses_pos_namespace` (matcheaba el doc al test real; el doc estaba desincronizado). Docs-sync sincronizado: ROADMAP + HANDOFF §20 + MASTER_PLAN § F2 ahora reportan 819 passed + 1 skipped (vs 817; +2 por el nuevo `test_model_is_sonnet_for_f2` parametrizado por 2 agents) y reconcilian la decisión (6) que ahora SÍ incluye `docs/ARCHITECTURE.md § 6 Agents` (antes "no requerido" — contradicción cerrada). No cambios de scope: sin `agents_allowed`, sin `pos-auditor`, sin modificar `policy.yaml`, sin tocar `audit-session`. Suite: 819 passed + 1 skipped. Sin regresión D1..D6 / E1a..E3b / F1. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - `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 + 24 agents contract + 2 skill flips + 2 hardening tests añadidos en revisión PR: `tools`/`model` requeridos + `model == "sonnet"` lockeado). 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). |
There was a problem hiding this comment.
The exit-criteria math in this sentence is inconsistent with the stated suite total (819) and the actual new tests: 793→819 is +26, which matches the new agents/tests/test_agent_frontmatter.py cases. The text currently references “24 agents contract + 2 skill flips + 2 hardening tests”, which doesn’t add up; please reconcile these numbers (or rephrase to avoid implying test-count deltas from modified tests).
| **Criterio de salida**: **819 passed + 1 skipped** (baseline F1 793 + 24 agents contract + 2 skill flips + 2 hardening tests añadidos en revisión PR: `tools`/`model` requeridos + `model == "sonnet"` lockeado). 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). | |
| **Criterio de salida**: **819 passed + 1 skipped** (baseline F1 793 → 819, +26 netos, correspondientes a la batería nueva de `agents/tests/test_agent_frontmatter.py`; además, en revisión PR se endurece la cobertura sobre `tools`/`model` requeridos y `model == "sonnet"` lockeado sin implicar delta adicional del total). 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). |
There was a problem hiding this comment.
Fixed in 6b3266f: exit criteria reescrito a "+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]". Las behavior flips de test_skill_frontmatter.py actualizan assertions de tests existentes — sin delta de count.
| - `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 agents + 2 skill flips). Sin regresión D1..D6 / E1a..E3b / F1. |
There was a problem hiding this comment.
This suite summary says “819 … (793; +26 agents + 2 skill flips)”, but 793→819 is +26 total. Since the skill behavior tests were updated/renamed (not added), the delta appears to come entirely from the 26 new parameterized agent contract cases. Please adjust the parenthetical so the arithmetic and attribution match the actual test counts.
| Suite global post-F2: **819 passed + 1 skipped** (baseline F1: 793; +26 agents + 2 skill flips). Sin regresión D1..D6 / E1a..E3b / F1. | |
| Suite global post-F2: **819 passed + 1 skipped** (baseline F1: 793; +26 agent contract cases; 2 skill flips/renames sin cambiar el total). Sin regresión D1..D6 / E1a..E3b / F1. |
There was a problem hiding this comment.
Fixed in 6b3266f: misma reconciliación en ROADMAP.md:621. "+26 contract cases" del nuevo archivo parametrizado; las flips de skill no añaden count.
| 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 |
There was a problem hiding this comment.
The behavior-contract header comment just above this class still states that pre-commit-review delegates to code-reviewer, which is now stale after the F2 flip to pos-code-reviewer. Please update that nearby comment (and any adjacent docstrings that still mention main..HEAD) so the test file’s narrative contract matches the assertions and the updated skill body.
There was a problem hiding this comment.
Fixed in 6b3266f: comentario actualizado a pos-code-reviewer (plugin subagent shipped en F2). Adicionalmente flippeados dos typos main..HEAD → main...HEAD en docstrings (líneas 351 + 419) — el three-dot es lo que ejecutan los skill bodies.
| - 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.<gate>.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; 24 contract tests + 2 behavior flips. **No** toca `policy.yaml` (`agents_allowed` diferido). `auditor` diferido (sin consumer real, regla #7). |
There was a problem hiding this comment.
This snapshot line says “24 contract tests + 2 behavior flips”, but agents/tests/test_agent_frontmatter.py currently expands to 26 parameterized test cases (and the overall suite delta 793→819 is +26). Please update the count wording here to match the actual test suite so the handoff summary stays trustworthy.
| - 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; 24 contract tests + 2 behavior flips. **No** toca `policy.yaml` (`agents_allowed` diferido). `auditor` diferido (sin consumer real, regla #7). | |
| - 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. **No** toca `policy.yaml` (`agents_allowed` diferido). `auditor` diferido (sin consumer real, regla #7). |
There was a problem hiding this comment.
Fixed in 6b3266f: snapshot reescrito a "26 contract tests parametrizados (agents/tests/test_agent_frontmatter.py). Behavior tests del consumer flippean assertions (sin delta de count)".
|
|
||
| - `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) — 24 contract tests parametrizados por `ALLOWED_AGENTS = ["pos-code-reviewer", "pos-architect"]`. 4 clases: structure, frontmatter, body, capability surfaces. |
There was a problem hiding this comment.
This bullet says the new agent contract suite is “24 contract tests”, but agents/tests/test_agent_frontmatter.py currently expands to 26 parameterized test cases (2 slugs × multiple assertions + capability checks). Please update this count so the plan matches the executable contract.
| - `agents/tests/test_agent_frontmatter.py` (NEW) — 24 contract tests parametrizados por `ALLOWED_AGENTS = ["pos-code-reviewer", "pos-architect"]`. 4 clases: structure, frontmatter, body, capability surfaces. | |
| - `agents/tests/test_agent_frontmatter.py` (NEW) — 26 contract tests parametrizados por `ALLOWED_AGENTS = ["pos-code-reviewer", "pos-architect"]`. 4 clases: structure, frontmatter, body, capability surfaces. |
There was a problem hiding this comment.
Fixed in 6b3266f: "24 contract tests" → "26 contract tests parametrizados por ALLOWED_AGENTS = [\"pos-code-reviewer\", \"pos-architect\"] (13 métodos × 2 slugs)".
Five tweaks surfaced by Copilot's second pass on PR #26 (commit f230320): 1. .claude/skills/tests/test_skill_frontmatter.py:316 — stale comment said `code-reviewer` (built-in default) when the test now asserts `pos-code-reviewer` (plugin subagent shipped by F2). Reworded. 2. test_skill_frontmatter.py:351 + 419 — `main..HEAD` (two-dot, ancestor-only) in docstrings flipped to `main...HEAD` (three-dot, merge-base relative) matching what the skill bodies actually run. 3. HANDOFF.md:9 + §20:425 — count framing said "+24 contract + 2 skill flips" or "+28 nuevos". Suite delta is +26 net (793 → 819) from the new parametrized file (2 slugs × 13 methods). Behavior flips edit assertions in existing tests — they don't add count. Reframed everywhere. 4. ROADMAP.md:621 + MASTER_PLAN.md:694 + 722 — same reconciliation as (3). 5. MASTER_PLAN.md decision (6) line 706 + ROADMAP exit criteria 640 already updated in f230320 to include `docs/ARCHITECTURE.md § 6 Agents`; now the count math in the same paragraphs matches. No code change. No test added or removed. Suite still 819 passed + 1 skipped. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
agents/con namespacepos-*—pos-code-reviewer(consumido porpre-commit-review, E2a) +pos-architect(consumido porcompound, E3a). Cierra la asimetría heredada de Fase E (skills propias del plugin pero subagents prestados de Claude Code defaults).pre-commit-review/compound) a los nuevos nombres con fallbackgeneral-purposeliteral intacto. Sin cambios apolicy.yaml,hooks/_lib/policy.py, ni extensión deaudit-session.ALLOWED_AGENTS+ 2 behavior flips). Cero regresión sobre D1..D6 / E1a..E3b / F1.Closed Fase -1 decisions (v2 ratified by user, recut from v1)
name+description+toolscomma-separated string +model: sonnet. Body Markdown como system prompt. Shape distinto al skill primitive (skill usa YAML listallowed-tools, agent usa stringtools). Sin campos inventados; precedente E1afeedback_skill_primitive_minimal.mdaplicado.pos-auditordiferido por falta de consumer real (regla feat(c3): tests harness renderer — 13-file output per profile, deferred frameworks testeable #7 CLAUDE.md: ≥2 repeticiones documentadas antes de abstraer).pos-*obligatorio para evitar colisión con built-in defaults de Claude Code (code-reviewer,code-architect,Plan,Explore,general-purpose) y user/project agents externos.agents_allowedNO añadido. Sin enforcement consumer (Stop hook leeskills.jsonl, no hay log de invocaciones de subagents).policy.yaml+hooks/_lib/policy.pyintactos.ALLOWED_AGENTS(4 clases: structure / frontmatter / body / capability surfaces) + behavior flips de skills consumidoras + forward-compat negation lists en main-strict skills (pattern-audit+audit-sessionjamás referencian plugin subagents).### feat/f2-agents-subagents — ✅nuevo; HANDOFF §1+§8+§9+§20; MASTER_PLAN § Rama F2 expandida con decisiones;.claude/rules/skills.md § Fork / delegación(precedentes a plugin agents);.claude/rules/skills-map.md(sección "Subagents del plugin (entregado en F2)");docs/ARCHITECTURE.md § 6 Agentsreescrita post-revisión PR (commitf230320— el nuevo top-levelagents/es superficie arquitectónica del plugin aunque no esté enforced por elpre-pr-gateconditional; antes erróneamente declarada "no requerida").Files
NEW (3):
agents/pos-code-reviewer.md— frontmatter primitive-correct + body declara 5 capacidades (bugs, logic errors, security, scope adherence, invariant violations) + output contract (severidad blocker/high/medium/nit, confidence-filtered, file:line refs) + hard limits.agents/pos-architect.md— frontmatter primitive-correct + body declara 3 dimensiones (pattern extraction ≥2 reps, architectural design, cross-file consistency) + output contract pattern proposals canonical-format (Name/Context/Signal/Rule/Examples/Rationale) + hard limits.agents/tests/test_agent_frontmatter.py— 26 contract tests parametrizados porALLOWED_AGENTS = ["pos-code-reviewer", "pos-architect"].ALLOWED_FRONTMATTER_KEYS = REQUIRED_FRONTMATTER_KEYS = {name, description, tools, model}(todos requeridos post-revisión PRf230320);VALID_MODELS = {sonnet, opus, haiku};F2_REQUIRED_MODEL = "sonnet".MODIFIED (9):
.claude/skills/pre-commit-review/SKILL.md— flipcode-reviewer→pos-code-reviewer(description + body + steps + failure modes); simplify pass reword del delegation disclaimer (7b60ebb); fallbackgeneral-purposeliteral intacto..claude/skills/compound/SKILL.md— flipcode-architect→pos-architect; fallbackgeneral-purposeintacto..claude/skills/tests/test_skill_frontmatter.py—TestPreCommitReviewBehavior::test_delegates_to_pos_code_reviewer+TestCompoundBehavior::test_body_delegates_to_pos_architect_with_fallbackflippean nombres + asertan fallback. Negation lists depattern-audit+audit-sessionextendidas conpos-*.ROADMAP.md,HANDOFF.md,MASTER_PLAN.md,.claude/rules/skills.md,.claude/rules/skills-map.md— docs-sync (commits33dde3e+f230320).docs/ARCHITECTURE.md§ 6 Agents — reescrita en commitf230320: tabla shape skill vs agent, namespacepos-*lock, 2 agents entregados + diferidos + forward-compat negation. Antes la sección era stale (listabacode-reviewer/architect/auditorsin namespace, usabaallowed-tools:que es shape de skill).Pre-PR pipeline (Fase N+1 → N+4)
7b60ebb). 1 cambio: stale framing del delegation disclaimer enpre-commit-review/SKILL.md(decía "reflects the Claude Code default shipped today" — true paracode-reviewerbuilt-in, false post-flip parapos-code-reviewerplugin agent). Reescrito a "is the plugin subagent shipped byposin F2".pos-code-reviewer; runtime Agent enum no expone aún el plugin agent → fallbackgeneral-purposedeclarado, dogfooding del propio contrato F2).clean to PR | findings advisory only. 0 blocker / 0 high / 0 medium / 1 nit.agents/tests/test_agent_frontmatter.py:133-158—toolsymodeltratados como optional vía early-return.33dde3e).f230320) — atendidos los 2 ajustes solicitados por el usuario después del review de Copilot:tools+modelmovidos aREQUIRED_FRONTMATTER_KEYS;test_tools_is_comma_separated_stringahora valida tokens (split + non-empty + sin whitespace antes de(...)); NUEVOtest_model_is_sonnet_for_f2lockea decisión Fase -1 (1).docs/ARCHITECTURE.md § 6 Agentsreescrita (era stale + uso erróneo deallowed-tools:); decisión Fase -1 (6) reconciliada.HANDOFF.mdtest name referencetest_name_is_pos_prefixed→test_name_uses_pos_namespace(matchea el doc al test real).Carry-overs a F3..F4
pre-commit-reviewycompoundreales sobre el repo sintético; ejercita la resolutionpos-code-reviewer/pos-architectcon fallbackgeneral-purposecuando el runtime sintético no expone agents del plugin.Test plan
agents/tests/test_agent_frontmatter.py— 26 contract tests pasan (24 RED-first kickoff e63f438 + GREEN 2a5396b; +2 hardening enf230320:test_model_is_sonnet_for_f2parametrizado por agent)..claude/skills/tests/test_skill_frontmatter.py— 2 behavior flips actualizados (delegation a pos-* + fallback general-purpose); negation lists extendidas con plugin agent names.TestIntegrationDiffUnavailable.pre-pr-gate.py(D4) verifica docs-sync mandatorio (ROADMAP + HANDOFF) + conditionalskills/**→.claude/rules/skills-map.md. Todos los archivos requeridos están en el diff..claude/branch-approvals/feat_f2-agents-subagents.approvedexiste (gitignored).🤖 Generated with Claude Code