From c7039c4430989fdc5c6b5dcc432868b46e90c19a Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Sun, 17 May 2026 22:53:57 -0700 Subject: [PATCH 01/17] feat(pi-arc): rename builder executor to coder --- packages/pi-arc/README.md | 12 ++-- .../pi-arc/agents/{builder.md => coder.md} | 10 +-- packages/pi-arc/extensions/arc.ts | 14 ++-- .../extensions/arc/model-profiles-ui.ts | 4 +- .../pi-arc/extensions/arc/model-profiles.ts | 2 +- packages/pi-arc/extensions/arc/subagents.ts | 2 +- packages/pi-arc/skills/arc-build/SKILL.md | 64 +++++++++---------- .../{builder-prompt.md => coder-prompt.md} | 6 +- packages/pi-arc/skills/arc-plan/SKILL.md | 2 +- packages/pi-arc/skills/arc-review/SKILL.md | 16 ++--- .../arc-model-profiles-contract.test.mjs | 2 +- .../arc-model-profiles-integration.test.mjs | 6 +- .../tests/arc-model-profiles-ui.test.mjs | 4 +- ...rc-subagents-auto-materialization.test.mjs | 18 +++--- .../pi-arc/tests/arc-subagents-sync.test.mjs | 22 ++++++- 15 files changed, 103 insertions(+), 81 deletions(-) rename packages/pi-arc/agents/{builder.md => coder.md} (93%) rename packages/pi-arc/skills/arc-build/{builder-prompt.md => coder-prompt.md} (92%) diff --git a/packages/pi-arc/README.md b/packages/pi-arc/README.md index e6e7768..ff2bdd9 100644 --- a/packages/pi-arc/README.md +++ b/packages/pi-arc/README.md @@ -51,7 +51,7 @@ This package is a Pi-native port of the Claude Code Arc plugin at https://github - When Arc recommends an option, list it first, append `(Recommended)` to the label, and explain why in the description. - **`arc_agent` tool**: - Runs bundled Arc specialist prompts from `agents/*.md` in fresh Pi subprocesses. - - Supports `builder`, `code-reviewer`, `doc-writer`, `evaluator`, `issue-manager`, and `spec-reviewer`. + - Supports `coder`, `code-reviewer`, `doc-writer`, `evaluator`, `issue-manager`, and `spec-reviewer`. - Resolves Arc model tiers (`small`, `standard`, `large`) to concrete Pi models so orchestrators can right-size subagent dispatches. - Current limitation: `isolation: "worktree"` is recognized but not implemented yet. - **Optional `pi-subagents` companion support**: @@ -172,7 +172,7 @@ The brainstorm skill writes a first-line marker like `` — to know which CLI to call. + +**Surfaces:** + +| `kind` | Create command | URL pattern | Encrypted? | Best for | +|---|---|---|---|---| +| `legacy` | `arc plan create ` | `http://localhost:7432/planner/` | no | Solo, plain HTTP, simplest comment thread | +| `share-local` | `arc share create ` | `http://localhost:7432/share/#k=` | yes | Solo, but want annotations + accept-resolve UI | +| `share-remote` | `arc share create --remote` | `/share/#k=` (default `https://arcplanner.sentiolabs.io`) | yes | Reviewers on other machines | + +`arc share create --server ` overrides `--remote` to target an explicit server. + +For the encrypted surfaces, the author's edit tokens live in the arc-server's local keyring (a `shares` table in `~/.arc/data.db`) — multi-client accessible via `/api/v1/shares`, never written to disk as JSON. Legacy plans don't have edit tokens; the URL is just the planner path. + +### `arc share` commands (share-local, share-remote) + +| Command | Purpose | +|---------|---------| +| `arc share create [--remote]` | Encrypt a plan and create a share, returns share ID. Default is local; `--remote` targets the configured share server. Output prints a single URL: `Preview URL` (local) or `Author URL` (shared) — the reviewer URL is obtained from the in-page **Share link** button on the share page header, not the CLI. | +| `arc share show ` | Decrypt and print plan content (use `--author-url` to reprint the Author URL) | +| `arc share approve ` | Mark the share as approved | +| `arc share comments ` | All review comments + statuses | +| `arc share pull ` | Accepted-only comments (the agent-input form) | +| `arc share list` | List shares known to this machine (incl. `plan_file` mapping). Add `--json` for `[{id, kind, url, key_b64url, plan_file, created_at}]` — pipe to `jq` to look up a share's local file path | +| `arc share update ` | Replace the encrypted plan content (in-place; ID stays stable) | +| `arc share delete ` | Delete a share (`--force` cleans up local keyring entries when the server is already gone) | + +### `arc plan` commands (legacy) + +| Command | Purpose | +|---------|---------| +| `arc plan create ` | Register a plan on the legacy `/planner/` surface (plain HTTP, no encryption). There's no in-place update — re-running `create` produces a new ID. | +| `arc plan show ` | Print plan metadata + content (the metadata header includes `File: `, useful for plan-file lookups) | +| `arc plan approve ` | Mark the plan as approved | +| `arc plan comments ` | List comments on the plan (flat thread; no Accept/Resolve/Reject states) | + +### Review cycle + +create → reviewers leave annotations → author Accepts/Resolves/Rejects (encrypted surfaces) or replies inline (legacy) → `arc share pull` surfaces accepted comments to the implementation flow (legacy reads the comments thread inline since it has no accepted-only filter). Approved design content is written into the epic's description field when creating implementation tasks. Run `arc docs plans` for full details. + +The `` marker on line 1 of every registered design doc tells downstream skills which CLI table above to use. See `skills/brainstorm/SKILL.md` step 6 for the marker-write contract and `skills/plan/SKILL.md` step 1 for the read pattern. + +## Labels + +Labels are global (shared across all projects) and support colors and descriptions. Use labels for cross-cutting categorization like `security`, `performance`, `tech-debt`. + +## Session Protocol + +**At session start:** +```bash +arc onboard # Get context, recover project if needed +``` + +**Before ending any session:** +Invoke the `finish` skill — it handles capturing remaining work, quality gates, arc updates, commit, and push. Work is NOT done until `git push` succeeds. + +**Writing notes for resumability:** +```bash +arc update --stdin <<'EOF' +COMPLETED: X. IN PROGRESS: Y. NEXT: Z +EOF +``` + +**Deep dive**: Run `arc docs resumability` for templates. + +## Common Workflows + +### Starting Work +```bash +arc onboard # Get context (recovers project if needed) +arc ready # Find available work +arc show # View details +arc update --take # Claim work (sets session ID + in_progress) +``` + +### Creating Issues +```bash +arc create "Title" -t task # Create task +arc create "Epic title" -t epic # Create epic +arc create "Subtask" --parent # Create child issue +arc dep add child-id parent-id --type parent-child # Or link existing issue to epic + +# With multi-line description (use --stdin flag): +arc create "Title" -t task --stdin <<'EOF' +Description with context, acceptance criteria, etc. +EOF +``` + +### Completing Work +```bash +arc close --reason "done" # Complete issue +arc ready # See what unblocked +``` + +**Deep dive**: Run `arc docs workflows` for complete checklists. diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/arc/_branch-check.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/arc/_branch-check.md new file mode 100644 index 0000000..c1a4109 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/arc/_branch-check.md @@ -0,0 +1,52 @@ +# Protected-Branch Check + +Shared reference for arc workflow skills (brainstorm, build, finish). When a skill says "perform the protected-branch check per `skills/arc/_branch-check.md`", do exactly what's in this file. + +## Why this check exists + +Direct commits to trunk (`main` / `master` / `release` / `production`) bypass review, can't be undone without a force-push that destroys teammates' history, and are how releases get broken. The cost of asking the user one question is far smaller than the cost of an unintended trunk commit — especially after they've spent an hour brainstorming or building work that now has to be rebased onto a feature branch. + +## When to run the check + +| Skill | When | +|---|---| +| `brainstorm` | Pre-flight, before any design dialogue. Sets up the branch context for everything downstream. | +| `build` | Pre-flight, before dispatching any task. Subagents will commit to whatever branch you're on. | +| `finish` | Phase 4, immediately before staging/committing. Last line of defense. | + +Run it **every time the skill runs** — don't assume a previous answer carries forward across sessions. Branch state changes; cost of asking again is one click. + +## How to run the check + +1. Get the current branch: + + ```bash + git branch --show-current + ``` + +2. If the result is **not** in the protected list (`main`, `master`, `release`, `production`), you're done — proceed with the skill. + +3. If the result **is** protected, check the project's `CLAUDE.md` (or `AGENTS.md`) for an explicit opt-out — a line like *"This project commits directly to main; skip the protected-branch check."* If present, you're done — proceed without prompting. (The project owner has consciously chosen trunk-based development.) + +4. Otherwise, use the `AskUserQuestion` tool with this exact shape — the wording matters because Claude has to recognise the branching choice and act on it: + + - **question**: `"You're on ''. Continue here, or switch to a feature branch first?"` + - **options**: + - `Switch to a feature branch` — recommended; you should run `git checkout -b ` (suggest a name from the work context — e.g. `feat/` for brainstorm, the arc task slug for build, a summary of the diff for finish) and proceed on the new branch + - `Stay on ''` — the user has consciously chosen trunk-direct work for this session + - `Cancel` — abort the current skill; user wants to handle branching manually first + +5. Branch on the answer: + - **Switch** → create the branch, then continue the skill on it + - **Stay** → continue on trunk + - **Cancel** → stop the skill; do not commit, do not dispatch tasks, do not write design docs + +## Why no env-var or CLI flag opt-out + +Earlier drafts had `ARC_MAIN_GUARD=off` and a bypass-token prefix. Both removed: this is a skill-level prompt, not a hook. The opt-out lives in `CLAUDE.md` so it's discoverable, version-controlled, and applies project-wide. If the user is annoyed by the prompt, the right answer is to add the `CLAUDE.md` line — not to teach Claude to skip the check on its own initiative. + +## What this check is NOT + +- Not a substitute for branch protection rules on the remote (GitHub/GitLab) — those are the actual enforcement layer +- Not a check that the *target* of `git push` is main; only that the *current* branch is. Pushing a feature branch from a main checkout is rare and not covered. +- Not a hook — there's no harness-level enforcement. If Claude skips this check, the user will only notice at PR time. The pre-flight placement (brainstorm + build) is the mitigation. diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/arc/_formatting.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/arc/_formatting.md new file mode 100644 index 0000000..20a61a5 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/arc/_formatting.md @@ -0,0 +1,26 @@ +# Content Formatting Guide + +The arc frontend renders GitHub Flavored Markdown with syntax highlighting. Follow these rules when writing issue descriptions, plans, comments, and notes. + +## Use +- **Fenced code blocks** with language tags: ` ```go `, ` ```bash `, ` ```json `, ` ```typescript `, ` ```sql `, ` ```yaml `, ` ```python `, ` ```html `, ` ```css ` +- **Headings** (`##` and `###`) for section structure +- **Bullet lists** (`-`) for unordered items and file lists +- **Numbered lists** (`1.`) for sequential steps +- **Task lists** (`- [ ]` and `- [x]`) for checklists +- **Tables** (`| col | col |`) for structured comparisons +- **Inline code** (backticks) for file paths, function names, variable names, and CLI commands +- **Bold** (`**text**`) for emphasis on key terms +- **Blockquotes** (`>`) for important callouts or notes +- **Links** (`[text](url)`) for references + +## Avoid +- Raw HTML tags — DOMPurify strips most tags +- Code fences without language tags — always specify the language for syntax highlighting +- UPPERCASE section headers (use `##` Markdown headings instead) +- Very long single-line paragraphs — use line breaks for readability + +## Code Block Languages +Supported with syntax highlighting: go, typescript, javascript, json, bash, shell, sql, yaml, markdown, html, css, python, text + +For unsupported languages, use `text` as the language tag. diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/brainstorm/SKILL.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/brainstorm/SKILL.md new file mode 100644 index 0000000..80e4a24 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/brainstorm/SKILL.md @@ -0,0 +1,348 @@ +--- +name: brainstorm +description: You MUST use this skill for any design exploration, architecture decision, or trade-off analysis before implementation begins — especially when the user says "brainstorm", "explore the design", "think through", "what approach should we take", or describes a feature with multiple valid strategies. This is the arc-native brainstorming skill that writes designs to docs/plans/ and registers them on one of three review surfaces (legacy `arc plan`, encrypted local `arc share`, or encrypted remote `arc share --remote`), depending on who's reviewing and whether encryption is needed. Always prefer this over generic brainstorming when the project uses arc issue tracking. +--- + +# Brainstorm — Design Discovery + +Explore requirements through Socratic dialogue before any implementation begins. + +## Hard Gate + +**Do NOT write any implementation code, scaffold any project, or take any implementation action until the design is approved.** Brainstorming produces a design document — not code. + +## Pre-flight: Branch Setup + +Before starting the design dialogue, perform the protected-branch check per `skills/arc/_branch-check.md`. + +Brainstorm itself doesn't commit code, but the design doc, the planned tasks, the eventual implementation, and the final commits will all land on whatever branch you start from. Catching trunk *now* avoids "we built three hours of work and it's all on main" at finish time. If the user picks "switch to a feature branch", suggest a name based on the brief they just gave you (e.g. `feat/`). + +## Workflow + +Create a task for each step below using `TaskCreate`. Mark each as `in_progress` when starting and `completed` when done. This creates a visible progress list in the CLI that carries forward into the plan skill. Step 5.5 gets its own task whether or not the user opts into grilling — "No, proceed" still counts as completing the step. + +### 1. Explore Project Context + +- Check existing files, docs, recent commits +- Review existing arc issues (`arc list`) +- Understand what already exists and what constraints are in play + +**Scope check before proceeding:** Before asking detailed clarifying questions, assess whether the request describes multiple independent subsystems (e.g., "build a platform with chat, storage, billing, and analytics"). If so, help the user decompose into sub-projects first — each sub-project gets its own brainstorm → plan → implement cycle. Don't spend questions refining details of a project that needs to be split. A decomposition sketch (what are the independent pieces, how do they relate, what order should they be built) is more valuable than a half-specified monolith. + +### 2. Ask Clarifying Questions + +- Ask questions **one at a time** — don't dump a list +- **Use the AskUserQuestion tool** for multiple-choice decisions (2-4 options) +- Use open-ended text questions only when you need freeform feedback +- Understand: purpose, constraints, success criteria, target users +- Continue until you have enough to propose approaches + +**If the user forecloses clarifying questions up front** (e.g., "no clarifying questions, just proceed", "skip to the design", "don't ask, just build"), keep this step's questions to a minimum or skip them. Step 5.5 is the explicit recovery loop in that case — depth-first interrogation against a draft, which is harder to skip past than a soft Q&A. Default 5.5's recommendation to *"Yes, grill me"* whenever step 2 was foreclosed, regardless of scale. + +**Example AskUserQuestion usage:** +``` +Question: "How should we handle session persistence?" +Options: + - "In-memory only" (simplest, lost on restart) + - "SQLite" (persistent, single-node, matches existing storage) + - "Redis" (distributed, adds infrastructure dependency) +``` + +### 3. Propose 2-3 Approaches + +- Each approach: summary, trade-offs, estimated complexity +- Include a recommendation with reasoning +- **Use the AskUserQuestion tool** to present approaches as structured choices +- Apply YAGNI — remove features from all designs that aren't explicitly required + +**Example AskUserQuestion usage:** +``` +Question: "Which approach should we go with?" +Options: + - "Approach A: ..." (recommended — trade-offs...) + - "Approach B: ..." (trade-offs...) + - "Approach C: ..." (trade-offs...) +``` + +**Capability-aware hint:** When comparing approaches, surface which imply heavier subagent model tiers during implementation. Approaches with more cross-cutting concerns, more files touched, or tighter coupling between components will likely need `opus`-tier dispatches and more review cycles. Approaches that decompose cleanly into single-file, mechanical tasks will run on `haiku`/`sonnet` and iterate faster. This is a soft consideration, not a deciding factor — but the user should see it. + +### 4. Present Design Section by Section + +- Break the design into logical sections (data model, API, UI, etc.) +- Present each section and get user approval before moving to the next +- Iterate on sections as needed based on feedback + +**Design for isolation and clarity:** Break the system into smaller units that each have one clear purpose, communicate through well-defined interfaces, and can be understood and tested independently. For each unit, you should be able to answer three questions: what does it do, how do you use it, and what does it depend on. Smaller, well-bounded units are also easier for subagents to work with — they reason better about code they can hold in context at once, and their edits are more reliable when files are focused. If a file in the design is projected to grow large, that's often a signal that it's doing too much — consider splitting the responsibility at design time. + +**In existing codebases:** Follow existing patterns. Where existing code has problems that affect the work (e.g., a file that's grown too large, unclear boundaries, tangled responsibilities), include targeted improvements as part of the design — the way a good developer improves code they're working in. Don't propose unrelated refactoring. Stay focused on what serves the current goal. + +### 5. Identify Shared Contracts (Parallel Readiness) + +If the design will produce multiple implementation tasks that could run in parallel, explicitly identify the **shared contracts** — types, interfaces, config keys, constants, and function signatures that multiple tasks will reference. + +Contracts fall into two tiers: + +- **Shared contracts** (referenced by 2+ tasks): produce **exact, copy-pasteable code blocks** including the type definition AND a contract test assertion. The T0 foundation task will write these verbatim. +- **Task-internal types** (used within a single task): use typed pseudocode (e.g., `FeedbackRequest { memory_id: i64, rating: i8 }`) — the subagent adapts to language idioms during implementation. + +Present shared contracts to the user as a "foundation layer" with exact code: + +```go +// internal/types/config.go + +// SessionConfig holds session-related settings. +type SessionConfig struct { + Timeout time.Duration `json:"timeout"` + MaxIdle int `json:"max_idle"` + Secure bool `json:"secure"` +} +``` + +```go +// internal/storage/storage.go + +// GetSession retrieves a session by ID. +// Returns nil and no error if the session does not exist. +GetSession(ctx context.Context, id string) (*Session, error) +``` + +Contract test assertions verify that the shared types satisfy compile-time expectations. Place these **inline in each relevant test file** with a clear separator: + +```go +// internal/types/config_test.go + +// --- Contract assertions --- + +// Verify SessionConfig fields exist with expected types. +var _ time.Duration = SessionConfig{}.Timeout +var _ int = SessionConfig{}.MaxIdle +var _ bool = SessionConfig{}.Secure +``` + +```go +// internal/storage/sqlite/sqlite_test.go + +// --- Contract assertions --- + +// Verify SQLiteStore satisfies the Storage interface. +var _ storage.Storage = (*SQLiteStore)(nil) +``` + +These exact definitions and contract tests become the **T0 foundation task** during planning — implemented sequentially before any parallel work begins. The T0 task writes the shared type files and embeds contract test assertions inline in each relevant test file, so that parallel agents can import these types immediately and any drift is caught at compile time. + +**Skip this step** if the design maps to a single task or purely sequential work. + +### 5.5. Grill the Design (Optional Stress-Test) + +Before publishing the design for review, save the draft to disk and offer a stress-test pass. Both this step and step 6 need the design as a file on disk, so first: + +- **Write the design document** to `docs/plans/` using `YYYY-MM-DD-.md` naming. Do this whether or not the user opts into grilling — step 6 picks it up either way. + +Then run a relentless-interrogation pass that probes the drafted design for unresolved *internal* decisions before publishing. This is a distinct job from step 7's review loop: that one processes external reviewer feedback you receive back; this one finds gaps the design didn't fully resolve, which become expensive to fix once implementation starts — and prevents publishing a version that's already known to be incomplete. + +**When to recommend it.** This is opt-in. Mark "grill" as recommended when the design appears Medium/Large per the Scale Detection table (multiple work items, multiple layers crossed, or migrations/breaking changes). For Small-scale single-task work, default the recommendation to "skip" — the overhead isn't worth it. + +**Always recommend grilling when step 2 was foreclosed.** If the user shut down clarifying questions up front, this is the recovery loop — override the scale-based default and mark *"Yes, grill me"* as recommended regardless of how small the design looks. + +**Use the AskUserQuestion tool:** + +``` +Question: "Stress-test the design before publishing?" +Options: + - "Yes, grill me" — interrogate decisions one at a time until we converge + - "No, proceed" — skip to step 6 register for review +``` + +If "Yes", run the loop: + +**Loop rules:** + +- Walk the design's decision tree **depth-first, ordered by dependency**. Resolve decisions that constrain later answers first (e.g., "what storage layer?" before "how do we serialize sessions?"). When a resolution opens new branches, recurse into them before backtracking. +- **One question per turn** via `AskUserQuestion`. Mark the recommended option. When the choice is genuinely contested, offer 2-3 options; when one option is objectively dominant, a single recommendation is fine — but never rubber-stamp open questions just because you have an opinion. +- **Codebase-first rule.** Before each question, name the symbol, file, or pattern that would answer it. If you can name one, search first (Grep / Read / symbol search) and only ask when the codebase doesn't — or can't — answer. This is the single biggest difference from step 2's clarifying questions, where you don't yet have a draft to ground against. +- **Capture resolutions in-place.** Each resolved decision is an edit to `docs/plans/.md` — update the relevant section, don't maintain a separate Q&A log. The design doc is the artifact. + +**Stop when ANY of:** + +- The user says "done", "enough", or "stop" +- Two consecutive rounds surface no new unresolved branches (the tree is exhausted) +- The loop has run ~10 rounds (hard cap — if you still have open branches at this point, surface them as a "remaining open questions" note in the design doc instead of asking another) + +Then proceed to step 6. + +### 6. Register for Review + +The design doc already exists on disk from step 5.5. This step registers it for review on the surface the user picks. + +Arc supports three review surfaces. They differ along two axes — *who reviews* (just you vs. teammates on other machines) and *do you want encryption + the new annotation/accept-resolve UI* (legacy planner is plain HTTP and simpler; `arc share` is encrypted and richer). Pick based on how the design will actually be reviewed, not which command you happen to remember. + +**Use the AskUserQuestion tool:** + +``` +Question: "How would you like to review this design?" +Options: + - "Legacy planner (solo, plain HTTP, simplest)" — + `arc plan` surface at /planner/. No encryption, no accept-resolve; + just a comment thread on a markdown render. Best when you want quick + review notes without setting up the share UI. + - "Encrypted local share (solo, but want annotations/accept-resolve)" — + `arc share` on this machine. Plan content + comments are encrypted at + rest in ~/.arc/data.db. Reviewer URL only works from this machine. + - "Encrypted remote share (multiple reviewers)" — + `arc share` on the configured remote server (default arcplanner.sentiolabs.io). + Reviewers on other machines can open the link. + - "Save for later" — keep the saved file (from step 5.5) and stop. No + server registration; resume in a new session. **Terminates the + skill — skip steps 7 and 8.** +``` + +Route on the answer: + +| Choice | CLI to run | Marker `kind=` | URL printed | +|---|---|---|---| +| Legacy planner | `arc plan create docs/plans/.md` | `legacy` | `Review at: http://localhost:7432/planner/` | +| Encrypted local | `arc share create docs/plans/.md` | `share-local` | `Preview URL (local-only — not reachable by others):` | +| Encrypted remote | `arc share create docs/plans/.md --remote` | `share-remote` | `Author URL (keep private — open it, then use the in-page Share link button to copy a reviewer URL):` | +| Save for later | (no command) | (no marker) | n/a | + +**Capture the ID and write the review marker.** After the create call succeeds, prepend a single HTML-comment line to the design doc so `/arc:plan` (and any future skill that queries review state) knows which CLI to call. Today only `/arc:plan` reads it — `/arc:build` and the dispatched implementer/reviewer agents read design content from the parent epic's description, not from the share/plan CLIs — but the marker is the canonical record of which surface this doc lives on. Without it, downstream falls back to `arc share list --json | jq` which doesn't cover legacy plans. + +```bash +# Run the chosen CLI and capture stdout. +OUT=$(arc share create docs/plans/2026-05-01-foo.md --remote) +echo "$OUT" # ALWAYS print verbatim — the user needs to see the URL + +# Extract the ID: +# - share-local / share-remote: the URL fragment contains /share/#... +# - legacy: the first line is "Plan created: (file: ..., status: ...)" +ID=$(echo "$OUT" | grep -oE '/share/[^#]+' | head -1 | sed 's|/share/||') +# For legacy, instead: ID=$(echo "$OUT" | grep -oE 'Plan created: \S+' | awk '{print $3}') + +KIND="share-remote" # legacy | share-local | share-remote (matches the chosen branch) + +# Prepend the marker idempotently. If line 1 already starts with "|" "$FILE" && rm "$FILE.bak" +else + { echo ""; cat "$FILE"; } > "$FILE.tmp" && mv "$FILE.tmp" "$FILE" +fi +``` + +The marker format is fixed: ``. Always line 1, always exactly one space between fields. + +**URL handling rules — print exactly what the CLI printed, then add a kind-specific instruction:** + +- **Legacy** — print the `Review at:` line. Tell the user this URL is local-only (their browser must reach `http://localhost:7432`). +- **Encrypted local** — print the Preview URL line. Tell the user it's not reachable from other machines; if they need a reviewer on a different machine, re-create the share with `--remote` instead. +- **Encrypted remote** — print the Author URL line. Then tell the user: *"Open this URL yourself; that's the author view. To send a reviewer link, click the **Share link** button in the page header — it strips `&t=` and copies a reviewer URL to your clipboard. Don't paste the Author URL into chat or tickets — the `&t=` token gives the recipient your edit privileges."* + +The encrypted-share CLI persists the edit_token + key into the local arc keyring (a `shares` table in `~/.arc/data.db`, served by the local arc-server — never written to disk as JSON). If a share Author URL is lost, regenerate it with `arc share show --author-url`. Legacy plans don't have this — the URL is just `/planner/` and there are no edit tokens. + +### 7. Review Loop + +**Skip this step entirely if step 6's answer was "Save for later"** — no surface was registered, no URL exists, no marker was written. Step 6 already terminated the skill in that case. + +Otherwise, print the URL from step 6 again as a reminder. **Use the AskUserQuestion tool:** + +``` +Question: "Design ready for review at — how would you like to proceed?" +Options: + - "Approve" — mark the design approved and proceed to step 8 + routing analysis + - "I've finished review (pull comments now)" — fetch reviewer feedback, + apply edits, re-share if needed, repeat + - "Pause review" — design is saved; resume in a new session +``` + +Branch the CLI by the marker's `kind`: + +| kind | Approve | Pull comments | +|---|---|---| +| `legacy` | `arc plan approve ` | `arc plan comments ` (no accepted-only filter — review the thread inline) | +| `share-local` | `arc share approve ` | `arc share pull ` (accepted-only by default) | +| `share-remote` | `arc share approve ` | `arc share pull ` (accepted-only by default) | + +**Why the legacy path lacks `pull`:** legacy plan comments don't have an Accept/Resolve/Reject state — they're a flat thread. The trade-off was made when picking legacy in step 6; if the volume of comments grows, suggest re-creating the design as `share-local` so the user gets the accepted-only filter. + +**For `share-local` / `share-remote`** — only `accepted` comments flow into refinement when pulled. The author is the only one who can mark comments as `accepted` (verified by the plan's `author_name`). For `share-remote`, reviewers comment via the reviewer URL (the in-page Share link button; *not* the Author URL). + +After a refinement pass, if the design changed materially, update the review surface to match the new content. The CLI and marker handling differ by `kind`: + +| kind | Update CLI | ID stable? | Marker action | +|---|---|---|---| +| `share-local` / `share-remote` | `arc share update ` | yes | leave marker as-is | +| `legacy` | `arc plan create ` (no in-place update — re-creates with a new ID) | **no — new ID** | rewrite line 1 with the new ID | + +For legacy, after re-creating, replace the `id=` portion of line 1 with the new ID — the idempotent `sed` snippet from step 6 works as-is: set `KIND=legacy` and `ID=` and the "marker already present" branch overwrites line 1. Then loop back to step 7. + +### 8. Routing Analysis & Transition + +After the design is approved (step 7's Approve), **you MUST produce a routing analysis before presenting options**. This analysis helps the user make an informed decision about what to do next. + +#### Routing Analysis + +Evaluate the approved design against these criteria and present a summary: + +| Factor | Assessment | +|--------|------------| +| **Work items** | Count of distinct implementation tasks identified in the design | +| **Parallel readiness** | Were shared contracts identified in step 5? (yes = plan needed for T0 sequencing) | +| **Files touched** | Approximate number of files created or modified | +| **Layers crossed** | Which architecture layers are involved (storage, API, CLI, frontend, tests) | +| **Risk areas** | Any migrations, API changes, or breaking changes? | +| **Scale** | Small / Medium / Large (from Scale Detection table) | + +Then produce a **recommendation** with reasoning: + +``` +📊 Routing Analysis +─────────────────── +Work items: N tasks identified +Parallel ready: Yes/No (shared contracts in step 5) +Files touched: ~N files across N directories +Layers crossed: [storage, API, CLI, ...] +Risk areas: [migrations, breaking changes, none, ...] +Scale: Small / Medium / Large + +➤ Recommendation: /arc:plan | /arc:build + Reason: <1-2 sentence justification based on the factors above> +``` + +**Routing rules** (use these to drive the recommendation): +- **→ arc:plan** when ANY of: 2+ work items, shared contracts exist, multiple layers crossed, migrations or breaking changes present, medium/large scale +- **→ arc:build** when ALL of: single work item, no shared contracts, single layer, no risk areas, small scale +- When borderline, recommend `arc:plan` — the overhead of planning is low, but the cost of a disorganized multi-task implementation is high + +After the analysis, use the **AskUserQuestion tool** — mark the recommended option: +``` +Question: "Design approved! What's next?" +Options: + - "Break into tasks with /arc:plan" (recommended — ) + - "Implement directly with /arc:build" (for small, single-task work) + - "Done for now" (design is saved — continue in a new session) +``` + +If `/arc:build` is recommended instead, swap which option gets the "(recommended)" tag. + +- **Break into tasks**: invoke the `plan` skill, passing the review ID from the line-1 marker (the `id=…` value; whether it's a legacy plan ID or a share ID depends on `kind=…`) +- **Implement directly**: invoke the `implement` skill +- **Done for now**: tell the user the design is approved and they can run `/arc:plan` in a new session + +## Scale Detection + +| Indicator | Scale | Structure | +|-----------|-------|-----------| +| Multiple phases, weeks of work, cross-cutting concerns | Large | Meta epic → phase epics → tasks | +| Single feature, days of work, contained scope | Medium | Epic → tasks | +| One task, hours of work, obvious approach | Small | Single issue | + +## Rules + +- The ONLY next skill after brainstorm is `plan` (or `implement` for small work) +- Never invoke implementation skills from brainstorm +- Design documents go in `docs/plans/` and are registered via one of three review surfaces (`arc plan create` for legacy, `arc share create` for encrypted local, `arc share create … --remote` for encrypted remote). The skill writes a `` marker as line 1 of the doc so downstream skills can route their CLI calls. +- Arc issues track persistent work; TaskCreate/TaskUpdate tracks workflow progress in the CLI +- YAGNI: if the user didn't ask for it, don't design it +- Format all arc content (descriptions, plans, comments) per `skills/arc/_formatting.md` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/brainstorm/evals/evals.json b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/brainstorm/evals/evals.json new file mode 100644 index 0000000..d0d8db9 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/brainstorm/evals/evals.json @@ -0,0 +1,23 @@ +{ + "skill_name": "brainstorm", + "evals": [ + { + "id": 0, + "eval_name": "solo-refactor-prefers-legacy", + "prompt": "I'm brainstorming a small solo refactor: extract a 200-line transcoding function in `internal/media/transcode.go` into three smaller helpers. Just me, no remote reviewers. I don't need encryption or accept/resolve workflows — a plain comment thread is fine. Walk me through what you'd do at section 6 (Save Design and Register for Review): which AskUserQuestion option you'd surface as recommended, the exact CLI command you'd run, the exact marker line you'd write to line 1 of `docs/plans/2026-05-01-transcode-refactor.md` (showing both the kind= and id= placeholder), and what URL the CLI is expected to print. Don't actually run anything — describe the plan in detail.", + "expected_output": "Recommends 'Legacy planner', shows `arc plan create docs/plans/2026-05-01-transcode-refactor.md`, marker line ``, prints `Review at: http://localhost:7432/planner/`. No mention of `--local` or `--share` (dropped flags). No promise of `arc share pull`-style filtering." + }, + { + "id": 1, + "eval_name": "multi-machine-review-picks-share-remote", + "prompt": "I'm designing the new auth handler for our SaaS. I want my coworker Steve (on a different laptop in another city) and one other reviewer to comment before we commit to an approach. Walk me through what you'd do at section 6: which AskUserQuestion option you'd surface as recommended, the exact CLI command, the marker line for `docs/plans/2026-05-01-auth-handler.md`, and EXACTLY what URL guidance you'd give the user — including how the user obtains a reviewer URL (since the CLI no longer prints one). Don't run anything — describe the plan.", + "expected_output": "Recommends 'Encrypted remote share', shows `arc share create docs/plans/2026-05-01-auth-handler.md --remote` (uses --remote, NOT --share), marker ``, prints Author URL labeled 'Author URL (keep private — open it, then use the in-page Share link button to copy a reviewer URL)'. Explicitly tells the user to open the Author URL in a browser and click the in-page Share link button to get a reviewer URL (which strips &t=). Warns NOT to paste the Author URL into chat." + }, + { + "id": 2, + "eval_name": "solo-encrypted-picks-share-local", + "prompt": "Brainstorm the queue migration design for me. Just me — no other reviewers. But I want every plan I work on stored encrypted at rest, and I want to use the new annotation/Accept/Resolve UI even though I'm the only reviewer. Walk me through section 6: which AskUserQuestion option you'd recommend, the exact CLI, the marker line for `docs/plans/2026-05-01-queue-migration.md`, and the URL guidance. Don't run anything.", + "expected_output": "Recommends 'Encrypted local share', shows `arc share create docs/plans/2026-05-01-queue-migration.md` (NO flag — local is the default; do NOT pass --local since that flag was dropped), marker ``, prints 'Preview URL (local-only — not reachable by others):'. Notes the URL is not reachable by reviewers on other machines." + } + ] +} diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/SKILL.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/SKILL.md new file mode 100644 index 0000000..857bee8 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/SKILL.md @@ -0,0 +1,365 @@ +--- +name: build +description: You MUST use this skill to execute implementation tasks from a planning artifact (the design + breakdown produced by /arc:brainstorm and /arc:plan) — especially when the user says "implement this", "build this", "execute the plan", "start coding", or wants to dispatch subagents for TDD execution of arc issues. The main agent orchestrates; it never writes implementation code directly. Always prefer this over generic implementation when the project uses arc issue tracking. +--- + +# Implement — Subagent-Driven TDD Execution + +Orchestrate task implementation by dispatching fresh `builder` subagents per task. Each subagent gets a clean context window with just the task description. + +## Core Rule + +**The main agent NEVER writes implementation code.** It orchestrates, dispatches, and reviews. If you're tempted to "just quickly fix this" — dispatch a subagent instead. + +## Pre-flight: Branch Setup + +Before dispatching any task, perform the protected-branch check per `skills/arc/_branch-check.md`. + +This catches the case where build was invoked without going through `brainstorm` first. Subagents commit to whatever branch the main agent is on — and the parallel-dispatch checkpoint push (P1) goes there too. Discovering at finish time that an entire epic landed on trunk is not recoverable cheaply. Suggest a branch name from the epic/task title if the user picks "switch." + +## Model Selection + +Every Agent dispatch can override the subagent's frontmatter model via the `model:` parameter. Use this to match model tier to task complexity. The default floor per agent is set in frontmatter — use these overrides to downgrade for trivial tasks or escalate for complex ones. + +| Task signal | Dispatch `model:` | +|---|---| +| Mechanical: 1-2 files, spec unambiguous, no cross-cutting concerns | `haiku` (downgrade from sonnet floor) | +| Standard: integration work, multi-file but contained, unambiguous | omit `model:` (use agent default) | +| Complex: 3+ files, cross-layer, design judgment required, migrations, breaking changes | `opus` | +| Re-dispatch after `BLOCKED` | escalate one tier (haiku → sonnet → opus); stop at opus | +| Re-dispatch after `NEEDS_CONTEXT` | same tier, richer context | + +Examples: + +```text +Agent(subagent_type="arc:builder", model="haiku", prompt="...") # mechanical +Agent(subagent_type="arc:builder", prompt="...") # standard (sonnet) +Agent(subagent_type="arc:builder", model="opus", prompt="...") # complex +``` + +**When unsure, omit `model:`** — the agent's frontmatter floor is calibrated for the typical case. + +**Escalation rule:** If a subagent returns `BLOCKED` with a reasoning or capability complaint, re-dispatch with the next tier up before asking the human. Stop escalating at opus — if opus also returns `BLOCKED`, escalate to the human with the subagent's blocker summary. + +## Dispatch Modes + +### Sequential (default) + +Tasks are dispatched one at a time through the orchestration loop below. Use this for: +- Most workflows — it's the safe default +- Tasks with any file overlap +- Tasks with dependency ordering (`blocks`/`blockedBy`) +- When you're unsure whether tasks are independent + +### Parallel + +Multiple tasks dispatched simultaneously using `isolation: "worktree"`. Use this **only** when ALL of these are true: +- 3+ independent tasks remain +- No shared files between any tasks in the batch +- No `blocks`/`blockedBy` dependencies between tasks in the batch +- Each task's scope is clearly defined with no ambiguity + +**When NOT to use parallel**: overlapping files, task dependencies, uncertainty about scope, fewer than 3 tasks. Default to sequential — the cost of serial execution is time; the cost of a bad parallel merge is data loss. + +## Orchestration Loop + +By default, use sequential dispatch. For independent tasks, see [Parallel Dispatch Protocol](#parallel-dispatch-protocol) below. + +**Task tracking**: At the start of implementation, create a task list using `TaskCreate` with one entry per arc issue to implement. This provides a visible progress tracker in the CLI. Update each task as you work: +- `in_progress` when dispatching the subagent +- `completed` when the task is closed in arc + +```bash +# Get the list of tasks to implement +arc list --parent= --status=open --json +``` + +Create a `TaskCreate` entry for each, then work through this loop: + +### 1. Find Next Task + +```bash +arc ready +# or for a specific epic: +arc list --parent= --status=open +``` + +### 2. Claim Task + +```bash +arc update --take +``` + +### 3. Dispatch Agent + +Record the current HEAD before dispatching — needed for review if escalated: + +```bash +PRE_TASK_SHA=$(git rev-parse HEAD) +``` + +Check whether the task has a `docs-only` label: + +```bash +arc show --json | jq -e '.labels[] | select(. == "docs-only")' > /dev/null 2>&1 +``` + +**If `docs-only`** (exit code 0) — spawn an `doc-writer` subagent: + +Use the template at `./doc-writer-prompt.md`. Fill placeholder `{TASK_ID}`. For docs-only work, the agent default (`haiku`) is correct — omit `model:` unless the docs task is unusually complex. + +**Otherwise** — spawn an `builder` subagent: + +Use the template at `./builder-prompt.md`. Fill placeholders (`{TASK_ID}`, `{PRE_TASK_SHA}`, `{DESIGN_EXCERPT}`) and apply Model Selection guidance (see `## Model Selection` above) for the dispatch `model:`. + +### 4. Evaluate Result + +When the subagent reports back, check its **Status** (one of `DONE | DONE_WITH_CONCERNS | BLOCKED | NEEDS_CONTEXT`) and **Gate Results**. Follow the `## Handle Implementer Status` table below for the status-specific action. In all cases, run the project test command fresh yourself — do NOT trust the subagent's report alone. + +**On `DONE`:** +- Run the project tests. If they pass → proceed to step 5 (Spec Compliance Review). +- If tests fail despite a `DONE` report, treat as `BLOCKED`: re-dispatch with the failure output. + +**On `DONE_WITH_CONCERNS`:** +- Read the concerns carefully. +- If the concerns touch correctness or scope (e.g., "I think this edge case isn't handled", "I modified a file outside the spec") — address before review by re-dispatching with specific guidance, or tightening the review prompt. +- If the concerns are observations (e.g., "this file is getting large") — note them as arc comments on the task and proceed to step 5. + +**On `BLOCKED` or `NEEDS_CONTEXT`:** +- Do NOT proceed to review. Do NOT close the task. +- For `NEEDS_CONTEXT`: gather the requested information, re-dispatch with it. +- For `BLOCKED`: assess the blocker per the Handle Implementer Status table. Escalate one model tier (haiku → sonnet → opus) per the Model Selection escalation rule, or invoke the `debug` skill if the blocker is a persistent test failure, or split the task if too large, or escalate to the human. +- After 3 re-dispatches on the same task without clean `DONE`, invoke the `debug` skill. + +**If the subagent did not include a Status field** (malformed report): +- Treat as `BLOCKED`. Re-dispatch with an explicit reminder to use the four-status Report Format. + +When re-dispatching, include the previous report's concerns / blockers so the implementer knows exactly what to fix: + +``` +Continue implementing this task. A previous attempt reported with these concerns: + + + +Address each concern and re-report. +``` + +### 5. Spec Compliance Review + +After confirming tests pass, dispatch the `spec-reviewer` to independently verify the implementation matches the spec: + +```bash +BASE_SHA=$PRE_TASK_SHA +``` + +Dispatch `spec-reviewer`: + +Use the template at `./spec-reviewer-prompt.md`. Fill placeholders (`{TASK_ID}`, `{BASE_SHA}`, `{HEAD_SHA}`). Spec review is a focused comparison task — the agent default is appropriate; omit `model:` unless the spec is unusually large or ambiguous. + +Handle results: +- `COMPLIANT` → proceed to Step 6 +- `ISSUES (Missing)` → re-dispatch `builder` with specific gaps listed by the spec reviewer. Re-run spec compliance review after. +- `ISSUES (Extra)` → re-dispatch `builder` to remove the extras listed by the spec reviewer. Re-run spec compliance review after. +- `ISSUES (Misunderstood)` → re-dispatch `builder` with clarification from the spec reviewer's findings. Re-run spec compliance review after. +- Circuit breaker: 3 spec-review/fix cycles without resolution → escalate to user. + +> **Docs-only tasks**: Skip this step. The spec-reviewer is designed around code verification (file lists, function signatures, test coverage) and doesn't apply to documentation. For docs-only tasks, the orchestrator verifies formatting/completeness directly: check that all files in `## Files` were created/modified, links resolve, heading hierarchy is correct, code blocks have language tags. + +### 6. Code Quality Review + +Only dispatched after spec compliance passes. Use the `review` skill or dispatch `code-reviewer` directly: + +```bash +HEAD_SHA=$(git rev-parse HEAD) +``` + +Use the template at `../review/reviewer-prompt.md`. Fill placeholders (`{TASK_ID}`, `{BASE_SHA}` = PRE_TASK_SHA recorded earlier, `{HEAD_SHA}` = current HEAD, `{DESIGN_EXCERPT}` from parent epic or "none", `{EVALUATOR_STATUS}` = "active" if evaluator was dispatched, else "not dispatched"). Follow Model Selection above for the dispatch `model:` — sonnet default is appropriate for most reviews. + +**On `{EVALUATOR_STATUS}`:** Decide whether to dispatch the evaluator (step 6.5) BEFORE filling this placeholder. If you plan to run step 6.5 in parallel with step 6, set `{EVALUATOR_STATUS}="active"`. Otherwise set `"not dispatched"`. Step 6.5 has the decision criteria for when to dispatch the evaluator. + +Handle findings: + +| Finding | Action | +|---------|--------| +| **Critical/Important** | Re-dispatch `builder` with fixes. Re-review after. | +| **Minor** | Note in arc comment. Proceed. | +| **Deviation (fix)** | Re-dispatch `builder` to match the design. | +| **Deviation (accept)** | Log as arc comment: "Accepted deviation: \. Rationale: \." Proceed. | + +Circuit breaker: 3 review/fix cycles on the same finding → escalate to user. + +> **Docs-only tasks**: Skip code quality review. For substantial documentation changes (developer-facing API docs, architecture docs), optionally dispatch `code-reviewer` for a quality check. + +### 6.5. High-Risk Evaluation (Optional) + +The evaluator is **not dispatched by default**. Dispatch only when: +- Task has a `high-risk` label +- The orchestrator judges the task warrants independent verification (e.g., complex spec with multiple valid interpretations, security-sensitive code, tasks that modify shared contracts) + +When dispatched, use `isolation: "worktree"` and the existing `evaluator` agent. The evaluator can run **in parallel with Step 6** (code quality review) since they examine orthogonal concerns: + +```bash +PARENT=$(arc show --json | jq -r '.parent_id // empty') +``` + +Use the template at `./evaluator-prompt.md`. Fill placeholder `{TASK_ID}`. Because evaluation is adversarial verification on high-risk tasks, escalate one tier from the agent default (typically to `opus`) — set `model: "opus"` on the dispatch unless the task is narrow. + +When dispatching alongside the evaluator, update the code quality reviewer's `## Evaluator Status` to `active`. + +Triage evaluator findings: + +| Evaluator verdict | Orchestrator action | +|---|---| +| `PASS` | No action — evaluator confirms the spec intent is satisfied. | +| `CONCERNS` | Read the concerns. Re-dispatch `builder` if the concerns describe substantive behavior gaps. Otherwise note as arc comments and proceed. | +| `FAIL — Spec-Intent Gap` | Re-dispatch `builder` with the evaluator's quoted spec text and the failing behavior description. | +| `FAIL — Missing Behavior` | Re-dispatch `builder` — the spec requires behavior that wasn't built. | +| `FAIL — Edge Case` | Lower-severity. Re-dispatch if the spec clearly implies the edge case; otherwise record as a known limitation. | +| `ERROR — Cannot Test` | The public API is insufficient. Re-dispatch with a request to expose the needed surface. | +| `BLOCKED` | Evaluator itself is blocked. Escalate per the Model Selection rules or involve the human. | + +### 7. Close Task + +```bash +arc close -r "Implemented: " +``` + +### 8. Integration Checkpoint + +After closing 2-3 related tasks, or before switching to a new epic phase, run the full integration test suite: + +```bash +make test-integration +``` + +This catches cross-task regressions that individual implementer gate checks won't — each implementer only validates its own task's scope. Do not wait until all tasks are complete to discover integration failures. + +If integration tests fail: +- Identify which task's changes caused the failure +- Re-dispatch `builder` with the failing test details and the relevant task context +- If the failure spans multiple tasks, invoke the `debug` skill + +### 9. Repeat + +Go to step 1 for the next task. Continue until all tasks in the epic are closed. + +## Handle Implementer Status + +Every `builder` and `doc-writer` dispatch returns one of four terminal statuses. Handle each explicitly: + +| Status | Orchestrator action | +|---|---| +| `DONE` | Proceed to spec review, then code review. | +| `DONE_WITH_CONCERNS` | Read the concerns. If they're about correctness or scope, address before review (re-dispatch or tighten review prompt). If they're observations (file getting large, naming doubt), note them as arc comments on the task and proceed to review — close only after a later dispatch yields a clean `DONE`. | +| `BLOCKED` | Assess the blocker: (1) context problem → provide missing context, re-dispatch same tier; (2) reasoning limit → re-dispatch one tier up per the Model Selection escalation rule; (3) task too large → split and re-plan; (4) plan is wrong → escalate to human. Never retry the same dispatch unchanged. | +| `NEEDS_CONTEXT` | Gather the specific missing information. Re-dispatch with it in the prompt. | + +**Never close a task** whose last report was `BLOCKED`, `NEEDS_CONTEXT`, or `DONE_WITH_CONCERNS` unresolved. Re-dispatch until you have a clean `DONE` — then close. + +## Parallel Dispatch Protocol + +When you have identified a batch of truly independent tasks (see [Dispatch Modes](#dispatch-modes)), switch from the sequential loop to this protocol: + +### P1. Commit Checkpoint + +Before switching to parallel, ensure all sequential work is committed and pushed: + +```bash +git status # Must be clean — no unstaged or uncommitted changes +git log -3 # Verify recent sequential commits are present +git push # Establish a recovery point on the remote +``` + +**Hard gate**: Do NOT proceed if `git status` shows uncommitted changes. + +### P2. Record HEAD Anchor + +```bash +PARALLEL_BASE=$(git rev-parse HEAD) +echo "Parallel base: $PARALLEL_BASE" +``` + +This is the baseline all worktrees will branch from. Record it — you'll need it for verification after merge. + +### P3. Verify Independence + +For each task in the planned parallel batch: + +```bash +arc show +``` + +Confirm: +- No `blocks`/`blockedBy` relationships between tasks in this batch +- No overlapping file paths in task descriptions +- Each task has a clearly scoped, non-ambiguous specification + +If any task fails these checks, remove it from the parallel batch and handle it sequentially after. + +### P4. Dispatch in Single Turn + +All parallel Agent tool calls with `isolation: "worktree"` **must happen in the same orchestrator message**. This ensures they all branch from the same HEAD. + +``` +# In a single response, dispatch all parallel tasks: +Agent(subagent_type="arc:builder", isolation="worktree", prompt="Task 1...") +Agent(subagent_type="arc:builder", isolation="worktree", prompt="Task 2...") +Agent(subagent_type="arc:builder", isolation="worktree", prompt="Task 3...") +``` + +**Never** dispatch worktree agents across multiple turns — HEAD may move between turns, causing stale branches. + +### P5. Merge-Back Verification + +After all parallel agents report back, verify the merge did not lose work: + +```bash +# 1. Check HEAD against the recorded anchor +git log --oneline $PARALLEL_BASE..HEAD # Should show ONLY the parallel agents' commits + +# 2. Verify sequential commits are still in history +git log --oneline HEAD | head -20 # All prior sequential commits must be present + +# 3. Run full test suite +make test # or project-specific test command +``` + +**If sequential commits are missing** → STOP. Do not continue. Recover from reflog: + +```bash +git reflog # Find the pre-merge state +git log --oneline # Verify it has the missing commits +# Cherry-pick or reset as appropriate — ask user if unsure +``` + +### P6. Resume Sequential + +After successful verification, return to the normal orchestration loop (step 1) for any remaining tasks. + +## When to Invoke Debug + +- Subagent reports test failures it can't resolve after reasonable effort +- 3+ implementation attempts fail on the same issue +- A regression appears that isn't explained by the current task's changes + +## Arc Commands Used + +```bash +arc ready # Find next task +arc update --take # Claim task (sets session ID + in_progress) +arc show # Get task description for subagent +arc close -r "reason" # Close completed task +``` + +## Rules + +- Never write implementation code as the main agent — always dispatch +- Never close a task without confirming tests pass yourself (fresh run) +- Never close a task if the implementer reported `BLOCKED`, `NEEDS_CONTEXT`, or unresolved `DONE_WITH_CONCERNS` without re-dispatching +- When re-dispatching after `BLOCKED`, escalate one model tier per the Model Selection table — never retry the same dispatch unchanged +- If in doubt about the result, re-dispatch rather than fixing manually +- Never dispatch parallel agents without committing and pushing all sequential work first +- Never dispatch parallel agents on tasks that share files +- Never proceed after parallel merge without verifying commit history against the recorded HEAD anchor +- Never mix sequential and parallel dispatch in the same batch — finish one mode before switching to the other +- Format all arc content (descriptions, plans, comments) per `skills/arc/_formatting.md` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/builder-prompt.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/builder-prompt.md new file mode 100644 index 0000000..982285c --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/builder-prompt.md @@ -0,0 +1,53 @@ +# Implementer Prompt Template + +Use this template when dispatching `builder` for a task. + +**Placeholders:** +- `{TASK_ID}` — arc issue ID (e.g., `task.abc123`) +- `{PRE_TASK_SHA}` — git SHA before this task starts (recorded by orchestrator) +- `{DESIGN_EXCERPT}` — relevant design section from parent epic, or omit if none +- `{MODEL_TIER_NOTE}` — optional hint about expected complexity + +````text +You are implementing arc task {TASK_ID}. + +## Task Spec + + +## Design Context +{DESIGN_EXCERPT} +(Omit this section if no parent epic design applies.) + +## Pre-Task SHA +{PRE_TASK_SHA} + +## Your Job + +1. Read the task spec end-to-end before writing code +2. If anything is unclear or missing, STOP and report `NEEDS_CONTEXT` with the specific question +3. Follow TDD: write the failing test first, make it pass, refactor +4. Only modify files listed in the task's `## Files` section — respect the `## Scope Boundary` +5. Use shared contracts from the task's `## Design Contracts` verbatim; do NOT invent shared types +6. Commit your work with a conventional commit message +7. Self-review your changes before reporting + +## Self-Review Before Reporting + +- Completeness: did I cover every requirement in the spec? +- Scope: did I avoid modifying files outside `## Files`? +- Quality: names accurate, code readable, no leftover debug output? +- Tests: do they verify behavior (not mocks)? Did I run them and see green? +- Discipline: no over-engineering, no speculative features (YAGNI)? + +## Report Format + +Report back with one of: `DONE` | `DONE_WITH_CONCERNS` | `BLOCKED` | `NEEDS_CONTEXT`. + +Include: +1. Status +2. Summary (one paragraph) +3. Files changed +4. Tests run and their outcome +5. Self-review findings +6. Concerns / Blockers / Missing context (non-DONE only) +```` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/doc-writer-prompt.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/doc-writer-prompt.md new file mode 100644 index 0000000..c04795d --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/doc-writer-prompt.md @@ -0,0 +1,41 @@ +# Doc Writer Prompt Template + +Use this template when dispatching `doc-writer` for a task labeled `docs-only`. + +**Placeholders:** +- `{TASK_ID}` — arc issue ID + +````text +You are writing/updating documentation for arc task {TASK_ID}. + +## Task Spec + + +## Your Job + +1. Read the task spec end-to-end +2. Only modify files listed in the task's `## Files` section +3. Follow the project's existing markdown style (check neighboring docs) +4. Use fenced code blocks with language tags (per arc formatting rules) +5. Verify internal links resolve and heading hierarchy has no skipped levels +6. Commit with a conventional commit message prefixed `docs(...)` + +## Verification Before Reporting + +Run the checks listed in the task's `## Verification` section. If none are specified: +- All internal relative links point to existing files +- Heading hierarchy uses `##` → `###` with no skipped levels +- All code blocks have language tags +- No HTML tags (DOMPurify strips them) + +## Report Format + +Report back with one of: `DONE` | `DONE_WITH_CONCERNS` | `BLOCKED` | `NEEDS_CONTEXT`. + +Include: +1. Status +2. Summary (one paragraph) +3. Files changed +4. Verification checks run and their outcome +5. Concerns / Blockers / Missing context (non-DONE only) +```` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/evaluator-prompt.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/evaluator-prompt.md new file mode 100644 index 0000000..6b4cd3f --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/evaluator-prompt.md @@ -0,0 +1,88 @@ +# Evaluator Prompt Template + +Use this template when dispatching `evaluator` for adversarial verification of a high-risk task. + +**Placeholders:** +- `{TASK_ID}` — arc issue ID + +````text +You are the adversarial evaluator for arc task {TASK_ID}. + +## Task Spec + + +## Your Job + +You have NOT seen the diff or the implementer's tests. Your job is to: + +1. Derive acceptance tests purely from the spec +2. Write them as ephemeral test files (prefix with `_eval_` — will be deleted) +3. Run them against the current code +4. Report which pass, which fail, and what the gap between spec-intent and built-behavior looks like + +You are the devil's advocate. The implementer believes the task is done. Prove it, or find the gap. + +## Process + +1. Read the spec. Identify every behavior the spec claims. +2. For each behavior, write a test that would fail if the behavior were missing. +3. Place tests in a location appropriate to the codebase (e.g., `_eval__test.go`). +4. Run the tests. +5. Collect pass/fail outcomes with evidence. +6. Delete your ephemeral tests (leave the codebase as you found it). +7. Report. + +## Report Format + +```text +## Evaluation: PASS | CONCERNS | FAIL | BLOCKED + +### Implementation Health (pre-check) +- Project builds: PASS | FAIL +- Existing tests pass: PASS | FAIL +- Binary/API available: PASS | FAIL + +### Evaluator Setup (self-check) +- Acceptance test compilation: PASS | FAIL () +- Evaluator dependencies resolved: PASS | FAIL + +### Spec Coverage ( behaviors) +- [PASS] +- [PASS] +- [FAIL] — +- ... + +### Findings + +#### Spec-Intent Gaps (implementation differs from spec) +- **Behavior**: +- **Expected**: +- **Actual**: +- **Severity**: Critical | Important + +#### Missing Behaviors (spec requires, not implemented) +- **Behavior**: +- **Evidence**: +- **Severity**: Critical + +#### Edge Case Failures (implied by domain, not explicit in spec) +- **Case**: +- **Expected**: +- **Actual**: +- **Severity**: Important | Minor + +#### Untestable Requirements (spec requires, API doesn't expose) +- **Requirement**: +- **Issue**: +- **Severity**: Important + +### Summary +<2-3 sentence assessment: does the implementation faithfully satisfy the spec?> +``` + +**Verdicts**: +- `PASS` — all spec behaviors pass and no critical gaps found +- `CONCERNS` — edge cases fail or minor gaps exist but core behaviors work +- `FAIL` — spec-intent gaps or missing behaviors found (sub-kinds: Spec-Intent Gap / Missing Behavior / Edge Case) +- `BLOCKED` — infrastructure failure prevented evaluation (tests didn't compile, binary missing, dependencies unresolvable). This is an evaluator problem, NOT an implementation problem — the orchestrator should not re-dispatch the implementer for BLOCKED results +```` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/spec-reviewer-prompt.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/spec-reviewer-prompt.md new file mode 100644 index 0000000..4d1b058 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/build/spec-reviewer-prompt.md @@ -0,0 +1,45 @@ +# Spec Reviewer Prompt Template + +Use this template when dispatching `spec-reviewer` after an implementer reports `DONE`. + +**Placeholders:** +- `{TASK_ID}` — arc issue ID +- `{BASE_SHA}` — pre-task SHA (recorded before dispatching the implementer) +- `{HEAD_SHA}` — current HEAD after implementer's commit + +````text +You are verifying that arc task {TASK_ID}'s implementation matches its spec exactly. + +## Task Spec + + +## Changes + + +## Your Job + +Compare the diff against the spec. For each requirement in the spec: +- Is it implemented? If yes, cite the file and line. +- If no, flag the gap. + +For the diff: +- Is anything present that the spec did NOT ask for? Flag it. +- Are files modified outside the task's `## Files` section? Flag as scope violation. + +You do NOT write code. You do NOT run tests. You do NOT close issues. + +## Report Format + +```text +## Result: COMPLIANT | ISSUES + +### Missing (only if ISSUES) +- + +### Extra (only if ISSUES) +- + +### Misunderstood (only if ISSUES) +- +``` +```` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/debug/SKILL.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/debug/SKILL.md new file mode 100644 index 0000000..25b60aa --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/debug/SKILL.md @@ -0,0 +1,91 @@ +--- +name: debug +description: You MUST use this skill when encountering any bug, test failure, unexpected behavior, nil pointer, panic, or error that needs root cause investigation — especially when the user says "debug", "investigate", "why is this failing", "root cause", or pastes a stack trace or error log. This is the arc-native debugging skill that enforces systematic investigation before any fix attempt. Always prefer this over generic debugging when the project uses arc issue tracking. +--- + +# Debug — Systematic Root Cause Investigation + +Investigate bugs methodically before attempting fixes. No guessing, no shotgunning, no Stack Overflow copypasta. + +## Iron Law + +**NO FIXES WITHOUT ROOT CAUSE INVESTIGATION FIRST.** + +If you don't understand why something is broken, you cannot fix it. A "fix" without understanding is a coincidence. + +## 4-Phase Process + +Create a TodoWrite checklist with these phases and work through them: + +### Phase 1: Investigate Root Cause + +- Read error messages **carefully** — they often tell you exactly what's wrong +- Reproduce the failure consistently — if you can't reproduce it, you can't verify a fix +- Check recent changes: `git diff`, `git log --oneline -10` +- Gather evidence: stack traces, logs, test output, error codes +- In multi-component systems, trace the data flow end-to-end +- **Do not propose fixes yet.** You are gathering evidence. + +### Phase 2: Pattern Analysis + +- Find working examples of similar code in the codebase +- Compare working code against broken code — what's different? +- Check if this is a known pattern (dependency version, config issue, API change) +- Look for similar past issues: `arc list --type=bug` + +### Phase 3: Hypothesis Testing + +- Form a **single** hypothesis about the root cause +- Design a minimal test to confirm or reject it — one change, one test +- If the hypothesis is wrong, **revert** the test change and form a new hypothesis +- Do NOT stack fixes — each hypothesis gets tested in isolation +- Document what you've tried and what you've learned + +### Phase 4: Implement Fix + +- Write a failing test that **demonstrates the bug** (the test should fail before the fix and pass after) +- Fix the **root cause**, not the symptom +- Verify the fix makes the bug test pass +- Run the **full test suite** to check for regressions +- If the fix introduces new failures, you fixed the wrong thing — go back to Phase 1 + +## The 3-Fix Rule + +If you've tried 3 fixes and none worked, **STOP**. + +You don't understand the problem yet. Going for fix #4 is insanity. + +Instead: +- Go back to Phase 1 and investigate more deeply +- Question your assumptions — are you fixing the right thing? +- Consider whether the architecture is wrong, not just the code +- Read the error message again — you probably skimmed it the first time + +## Arc Integration + +If the bug turns out to be bigger than expected (not a quick fix within the current task): + +```bash +arc create "Bug: " --type=bug --priority= +``` + +Then decide: fix it now (if it blocks current work) or defer it (if current work can continue without it). + +## Red Flags + +You're doing it wrong if you: +- Fix symptoms instead of causes +- Apply fixes without understanding why they work +- Copy code from the internet without understanding it +- Make multiple changes at once ("let me try this AND this AND this") +- Skip the failing test that demonstrates the bug +- Say "it works now" without understanding what changed + +## Rules + +- Always investigate before fixing — Phase 1 is not optional +- Always write a bug-demonstrating test before the fix +- Always run the full test suite after fixing +- Revert failed fix attempts cleanly — don't leave debris +- After debugging, return to the calling skill — typically `implement` step 4 to re-verify the subagent's result, or `verify` to re-run the gate sequence +- Format all arc content (descriptions, plans, comments) per `skills/arc/_formatting.md` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/finish/SKILL.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/finish/SKILL.md new file mode 100644 index 0000000..dcf65a5 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/finish/SKILL.md @@ -0,0 +1,153 @@ +--- +name: finish +description: You MUST use this skill at the end of any session, when the user says "land the plane", "wrap up", "done for the day", "finish up", "session complete", "push and close", or indicates work is complete. This is the arc-native session completion protocol that captures remaining work as arc issues, runs quality gates, updates arc issue statuses, commits, and pushes. Always prefer this over generic branch-finishing when the project uses arc issue tracking. +--- + +# Finish — Unified Session Completion + +Complete the session: capture remaining work, pass quality gates, update arc, commit, push. One protocol for all contexts. + +## Iron Law + +**Work is NOT done until `git push` succeeds. No exceptions.** + +Uncommitted code doesn't exist. Unpushed commits are local fiction. The remote is the source of truth. + +## Protocol + +Create a TodoWrite checklist with all steps and work through them: + +### Phase 1: Capture Remaining Work + +1. Review what was planned vs what was completed +2. For any unfinished work or newly discovered tasks: + ```bash + arc create "Remaining: " --type=task + ``` +3. Add context notes to new issues so the next session can pick up: + ```bash + arc update --description "CONTEXT: " + ``` + +### Phase 2: Quality Gates + +*Skip this phase if no code was changed in this session.* + +4. Run project test suite: + ```bash + make test # or: go test ./..., npm test, etc. + ``` +5. Run linter/formatter if configured: + ```bash + make lint # or: golangci-lint run, eslint, etc. + ``` +6. Run build if applicable: + ```bash + make build + ``` +7. **Hard gate**: If tests fail, fix them. Do NOT skip to commit. Invoke `debug` if needed. + +### Phase 3: Update Arc Issues + +8. Close completed issues: + ```bash + arc close -r "Done: " + ``` +9. Update in-progress issues with progress notes: + ```bash + arc update --description "PROGRESS: . NEXT: " + ``` +10. Verify issue states match reality — don't leave stale statuses + +### Phase 4: Commit and Push + +11. Stage changed files (specific files, not `git add -A`): + ```bash + git add ... + ``` +12. **Protected-branch check** — perform the check per `skills/arc/_branch-check.md`. This is the *last* place to catch trunk-direct work; ideally `brainstorm` or `build` already established a feature branch earlier, but check anyway because some flows skip those skills. +13. Commit with conventional commit message: + ```bash + git commit -m "feat(scope): summary of changes" + ``` +14. Push: + ```bash + git push + ``` +15. Verify push succeeded: + ```bash + git status # Must show "up to date with origin" + ``` +16. If push fails → resolve the issue → retry → succeed. Do not leave unpushed commits. +17. Clean up worktrees: + ```bash + git worktree list + ``` + If only the main working tree is listed, skip ahead. Otherwise, for each extra worktree: + + **a. Check for uncommitted work:** + ```bash + git -C status + git -C stash list + ``` + If there are uncommitted changes or stashes → do NOT remove. Create an arc issue to track the unmerged work: + ```bash + arc create "Recover unmerged worktree work: " --type=task + ``` + + **b. Check if the branch was merged:** + ```bash + git branch --merged | grep + ``` + If merged (or if the worktree is clean with no unique commits), safe to remove: + ```bash + git worktree remove + git branch -d # Delete the merged branch + ``` + + **c. If the branch has unmerged commits but no uncommitted changes:** + Check whether the commits exist on a remote: + ```bash + git log origin/ 2>/dev/null + ``` + If pushed → safe to remove locally. If not pushed → do NOT remove; create an arc issue. + + **d. Prune stale worktree references:** + ```bash + git worktree prune + ``` + +### Phase 5: Verify and Hand Off + +18. Confirm the commit: + ```bash + git log -1 # Verify latest commit is visible + ``` +19. Output context for next session: + ```bash + arc prime + ``` + +## Context-Aware Behavior + +| Session Type | Behavior | +|-------------|----------| +| **Single-agent** | Full protocol above | +| **Team lead** | Verify teammate work → close arc issues → team cleanup → commit → push | +| **Teammate** | Commit → push (team lead handles arc close and coordination) | + +## What's NOT in This Protocol + +- `git stash clear`, `git remote prune origin` — housekeeping, not gates +- Worktree directory `.gitignore` verification — assumed to be configured at project setup +- Merge/PR/keep/discard choice — arc workflow always commits and pushes +- Performative session summaries — `arc prime` handles handoff context + +## Rules + +- Never skip Phase 2 (quality gates) when code has changed +- Never commit with `git add -A` — stage specific files +- Never leave unpushed commits +- Never close arc issues without completing the work +- Always run `arc prime` at the end for next-session context +- Format all arc content (descriptions, plans, comments) per `skills/arc/_formatting.md` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/plan/SKILL.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/plan/SKILL.md new file mode 100644 index 0000000..ff01e96 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/plan/SKILL.md @@ -0,0 +1,393 @@ +--- +name: plan +description: You MUST use this skill to break a design or feature into implementation tasks — especially after brainstorming, when the user says "plan this", "break this down", "create tasks", or wants to turn a design into actionable arc issues with exact file paths. Creates self-contained arc issues that subagents can implement with zero prior context. Always prefer this over generic planning when the project uses arc issue tracking. +--- + +# Plan — Implementation Task Breakdown + +Break an approved design into bite-sized, self-contained tasks with exact file paths and steps. + +## Review Commands + +Design docs live in `docs/plans/.md`. The brainstorm skill registers each doc on one of three review surfaces and writes a routing marker as line 1 of the doc itself: + +``` + +``` + +**Always read the marker before invoking any review CLI.** The plan skill's CLI calls branch on `kind`: + +| kind | Show content | List comments | Pull accepted | Approve | Update content | +|---|---|---|---|---|---| +| `legacy` | `arc plan show ` | `arc plan comments ` | n/a — review thread inline | `arc plan approve ` | re-create the plan (no in-place update) | +| `share-local` | `arc share show ` | `arc share comments ` | `arc share pull ` | `arc share approve ` | `arc share update ` | +| `share-remote` | `arc share show ` | `arc share comments ` | `arc share pull ` | `arc share approve ` | `arc share update ` | + +Read the marker with one shell call: + +```bash +MARKER=$(head -1 docs/plans/.md) +KIND=$(echo "$MARKER" | grep -oE 'kind=[a-z-]+' | cut -d= -f2) +ID=$(echo "$MARKER" | grep -oE 'id=\S+' | sed 's/id=//' | tr -d '>' | xargs) +# Now branch on $KIND for every review CLI call. +``` + +**Encrypted-share keyring.** For `share-local` and `share-remote`, the author's edit tokens live in the arc-server's local keyring (a `shares` table in `~/.arc/data.db`) — not in any JSON file. `arc share show --author-url` reprints the Author URL if it's lost. Legacy plans don't have edit tokens; the URL is just `/planner/`. + +**Fallback for unmarked design docs.** Older design docs created before the marker contract may not have line 1 set. If the marker is missing, fall back to: + +```bash +arc share list --json | jq -r '.[] | select(.plan_file=="docs/plans/.md") | .id' +``` + +This only covers `share-*` plans (legacy plans aren't in the share keyring). If the fallback returns no result, ask the user which review surface the plan was registered on. + +## Granularity Rule + +Each task step is **ONE action, 2-5 minutes**. Assume the implementer has **zero codebase context** and fresh context without codebase familiarity. If a step says "add validation" without showing the code, it's too vague. + +## No Placeholders + +Every step in a task description must contain the actual content an implementer needs. These are **plan failures** — never write them: + +- `"Add appropriate error handling"` / `"add validation"` / `"handle edge cases"` — show the actual code +- `"Write tests for the above"` without test code — include the test code +- `"Similar to Task N"` — repeat the content; the implementer has zero context of other tasks +- Steps that describe what to do without showing how — code blocks required for code steps +- References to types, functions, or methods not defined in any task or already on HEAD +- `"TBD"`, `"TODO"`, `"implement later"`, `"fill in details"` + +Code blocks represent the **intent, structure, and behavior** — not a character-for-character mandate. The implementer follows the code block's signatures, logic, and patterns but adapts naming, error handling, and scaffolding to match project conventions (consistent with the implementer's Gate Check 4: Idiomatic Code Quality). Task-internal Design Contracts remain pseudocode that the implementer adapts to language idioms. The anti-placeholder rule prevents *missing* guidance, not idiomatic adaptation. + +## Workflow + +Add tasks for each step below using `TaskCreate`. If continuing from the brainstorm skill, the brainstorm tasks will already be visible — add the planning tasks alongside them so the user sees the full brainstorm→plan progression. Mark each as `in_progress` when starting and `completed` when done. + +### 1. Read the Design + +You're handed a plan-file path (typically `docs/plans/.md`) by the brainstorm skill. Read line 1 to learn which review surface the plan lives on, then call the matching show command: + +```bash +MARKER=$(head -1 docs/plans/.md) +KIND=$(echo "$MARKER" | grep -oE 'kind=[a-z-]+' | cut -d= -f2) +ID=$(echo "$MARKER" | grep -oE 'id=\S+' | sed 's/id=//' | tr -d '>' | xargs) + +case "$KIND" in + legacy) arc plan show "$ID" ;; + share-local|share-remote) arc share show "$ID" ;; + *) echo "No review marker; reading file directly"; cat docs/plans/.md ;; +esac +``` + +The full content is what you'll break down in the next steps. If the file has no marker (an older design doc), reading the file directly is fine — but warn the user the review-state CLI calls (approve, pull) won't work without a registered review surface, and offer to register it via brainstorm step 6. + +### 2. Identify Shared Contracts (Foundation Task) + +Check the design for **shared contracts** — types, interfaces, config keys, constants, or function signatures referenced by multiple tasks. If the brainstorm design includes a shared contracts section, use it as input. + +If shared contracts exist and parallel execution is likely: + +1. Create a **T0: Foundation** task that establishes all shared contracts +2. Mark all parallelizable tasks as **blocked by T0** +3. T0 runs sequentially before any parallel batch begins + +This ensures parallel agents inherit shared definitions from HEAD rather than inventing them independently. + +**T0 task descriptions must be literal, not prose.** The description should contain: +- **Exact type/interface code** to write to specific files (sourced from the brainstorm design's shared contracts) +- **Inline contract test assertions** to write in each relevant test file, so downstream tasks can verify they are using the correct types +- Steps that say "write this exact code to this exact file" — not vague instructions like "define the memory type" + +Example T0 task description: + +```markdown +## Summary +Establish shared types and contract tests for the memory feature. + +## Files +- Create: `internal/types/memory.go` +- Create: `internal/memory/memory_test.go` + +## Scope Boundary +Do NOT create or modify any files outside the Files section above. + +## Steps +1. Create `internal/types/memory.go` with this exact content: + ```go + package types + + import "time" + + type Memory struct { + ID int64 `json:"id" db:"id"` + Content string `json:"content" db:"content"` + CreatedAt time.Time `json:"created_at" db:"created_at"` + } + ``` +2. Create contract assertions in `internal/memory/memory_test.go`: + ```go + package memory + + import ( + "testing" + "time" + + "yourmodule/internal/types" + ) + + // --- Contract assertions --- + // These verify the design spec. Do NOT modify + // without updating the approved plan. + + func TestMemoryContract(t *testing.T) { + m := types.Memory{} + var _ int64 = m.ID + var _ string = m.Content + var _ time.Time = m.CreatedAt + } + + // --- Behavior tests (added by implementer) --- + ``` +3. Run `go build ./internal/types/...` — confirm it compiles +4. Run `go test ./internal/memory/...` — confirm contract tests pass +5. Commit: `feat(types): add foundation types and contract tests` + +## Test Command +go test ./internal/memory/... + +## Expected Outcome +Shared types compile and contract assertions pass. Parallel tasks can now import these types from HEAD. +``` + +**Skip this step** if the work is purely sequential or no shared contracts were identified. + +### 3. Identify Tasks + +Break the design into self-contained implementation units. Each task should: +- Have a clear, testable outcome +- Be implementable without knowledge of other tasks +- Include exact file paths for all files to create or modify +- Follow a logical dependency order +- **Not overlap in file ownership with other parallelizable tasks** + +When identifying tasks, assign **file ownership** — each file should be owned by exactly one task. If two tasks need to modify the same file, either merge them into one task, serialize them with a dependency, or extract the shared file into the foundation task. + +### 4. Create Epic and Tasks via issue-manager + +**Model tier:** `issue-manager` defaults to `haiku` — the right tier for CLI formatting and bulk issue creation. For this dispatch, omit `model:`. See the Model Selection table in `../build/SKILL.md` for the full guidance. + +**Never run `arc create` directly** — always delegate to the `issue-manager` agent. This keeps bulk CLI output in a disposable subagent context. + +Read the full plan content first using the kind-aware case from step 1 (`arc plan show "$ID"` for legacy, `arc share show "$ID"` for share-local / share-remote). Then build a task manifest that includes: +1. **The epic** — its description will be populated by the agent from the plan file (see below) +2. **All child tasks** with self-contained descriptions + +**Critical**: Do NOT paste or summarize the plan content into the agent prompt. Instead, pass the plan file path and let the agent read it directly. This prevents content loss from summarization. + +You typically already have the plan file path from the brainstorm hand-off. If you only have the ID and need to find the file path, the lookup depends on `kind`: + +```bash +# share-local / share-remote: keyring includes the plan_file mapping +arc share list --json | jq -r '.[] | select(.id=="") | .plan_file' + +# legacy: arc plan show prints "File: " in its metadata header +arc plan show | grep -oE '^File: \S+' | awk '{print $2}' +``` + +The share keyring entries have `{id, kind, url, key_b64url, plan_file, created_at}` — edit tokens are intentionally redacted. Then dispatch the manifest: + +``` +Use the Agent tool with subagent_type="arc:issue-manager": + +Create the following epic and tasks. +After creation, set dependencies and labels as listed. +Return a summary table mapping task names to arc IDs. + +## Epic + +### +Type: epic +Plan file: + +IMPORTANT: Read the plan file at the path above using the Read tool. Use the COMPLETE +file contents as the epic description. Do NOT summarize, truncate, or paraphrase — +copy the full file content verbatim as the description. + +## Tasks + +### T1: +Type: task +Parent: <epic-id from above> +Description: +<full multi-line self-contained description> + +### T2: <title> +Type: task +Parent: <epic-id from above> +Description: +<full multi-line self-contained description> + +## Dependencies +- T2 blocked by T1 +- T4 blocked by T3 + +## Labels +- T3: docs-only + +## Required Output +| Task | Arc ID | Title | +|------|--------|-------| +| Epic | ... | ... | +| T1 | ... | ... | +``` + +**IMPORTANT**: The epic description MUST contain the complete approved design. The agent reads the plan file directly to avoid any summarization or content loss. The plan file is ephemeral; the epic description is the permanent record. + +For each task, check whether **all** files in its `## Files` section are documentation (`.md`, `.txt`, `README`, `CHANGELOG`, or anything under `docs/`). If so, include it in the `## Labels` section with `docs-only`. Doc-only tasks skip TDD — the `implement` skill routes them to `doc-writer` instead of `builder`. + +### 5. Validate Returned Results + +Before proceeding, verify the agent's output: + +1. **Count check**: The number of returned IDs must match the number of tasks in your manifest +2. **Spot-check**: Run `arc show <id>` on one returned task to confirm it exists and has the correct parent +3. **If mismatch**: Re-dispatch the agent for missing tasks only, or create them manually + +### 6. Append Task Breakdown to Epic Description + +The epic was created in step 4 with the full design content. Now append the task breakdown table (with actual arc IDs from step 5) to the epic's description: + +```bash +arc update <epic-id> --stdin <<'EOF' +<existing epic description — the full design content from step 4> + +--- + +## Implementation Tasks + +<task breakdown table with arc IDs, titles, statuses, and dependency info> +EOF +``` + +**IMPORTANT**: Preserve the full design content already in the description — do not replace it with a summary. The epic description is the permanent record of the design. Only append the task breakdown table at the end. + +### 6.5. Self-Review + +After writing all tasks, review the plan against the design before proceeding: + +1. **Spec coverage:** Skim each section/requirement in the design. Can you point to a task that implements it? If a gap exists, add the task. +2. **Placeholder scan:** Search all task descriptions for red flags from the No Placeholders list. Fix them. +3. **Type consistency:** Do the types, method signatures, and property names used in later tasks match what was defined in earlier tasks? A function called `clearLayers()` in T1 but `clearFullLayers()` in T3 is a bug. +4. **Step completeness:** Every code step has a code block. Every command step has the exact command and expected output. No exceptions. + +Fix issues inline. No need to re-review — just fix and move on. + +### 7. Choose Execution Path + +**Use the AskUserQuestion tool** to let the user choose: + +``` +Question: "Epic and tasks created. How should we proceed with implementation?" +Options: + - "Start implementing now" (invoke /arc:build in this session — subagents handle TDD per task) + - "Implement in a new session" (provides the exact prompt to use) + - "Done for now" (tasks are tracked in arc — implement manually or later) +``` + +After the user chooses: + +**Start implementing now**: Invoke the `implement` skill immediately with the epic ID. + +**Implement in a new session**: Output the exact command for the user to copy-paste: +``` +Run this in a new Claude Code session: + + /arc:build <epic-id> + +``` +Replace `<epic-id>` with the actual epic ID. + +**Done for now**: Confirm the epic and tasks are saved in arc. The user can run `/arc:build <epic-id>` whenever they're ready. + +## Task Description Format + +Each task's `--description` must be **self-contained** (~3-5k tokens). The task description IS the implementation context — the implementer loads `arc show <task-id>` and nothing else. + +Include in every task description: + +``` +## Files +- Create: `path/to/new_file.go` +- Modify: `path/to/existing_file.go` +- Test: `path/to/file_test.go` + +## Scope Boundary +Do NOT create or modify any files outside the Files section above. +If you need a type, interface, or constant that doesn't exist, do NOT create it — +the foundation task or a prior task is responsible for shared definitions. + +## Design Contracts + +### Shared (use verbatim — defined in T0: Foundation) +```go +type Memory struct { + ID int64 `json:"id" db:"id"` + Content string `json:"content" db:"content"` + CreatedAt time.Time `json:"created_at" db:"created_at"` +} +``` + +### Task-internal +- `FeedbackRequest { memory_id: i64, rating: i8, comment: String? }` +- `MemoryStore.InsertMemory(content string) → (int64, error)` + +## Steps +1. Write failing test for <specific behavior> in `path/to/file_test.go` +2. Run `go test ./path/to/...` — confirm it fails with <expected error> +3. Implement <specific function> in `path/to/new_file.go`: + ```go + func specificFunction(arg Type) (Result, error) { + // exact implementation code — not prose descriptions + } + ``` +4. Run `go test ./path/to/...` — confirm it passes +5. Commit: `feat(module): add <feature>` + +## Test Command +go test ./path/to/... + +## Expected Outcome +<what should work when this task is done> +``` + +**Hard rule:** Every code step requires a code block. Every command step requires the exact command and expected output. Steps without these are plan failures — see the No Placeholders section above. + +### Design Contracts guidance + +Include a `## Design Contracts` section in every non-T0 task description, placed after `## Scope Boundary` and before `## Steps`. This section has two subsections: + +- **Shared (use verbatim)**: Exact type definitions copied from the T0 foundation task. The subagent MUST use these types exactly as written — same field names, same tags, same package. These are the canonical contracts established by T0 and committed to HEAD. +- **Task-internal**: Pseudocode descriptions of types and signatures that are private to this task. The subagent adapts these to language idioms (naming conventions, error handling patterns, etc.) as appropriate. + +If a type the subagent needs is not listed in Design Contracts and is not already on HEAD from T0, the subagent must NOT create it. This rule complements the Scope Boundary section — Scope Boundary restricts file ownership, Design Contracts restricts type ownership. + +For `docs-only` tasks, omit `## Test Command` and use `## Verification` instead: + +``` +## Verification +- All internal links resolve to existing files +- Heading hierarchy has no skipped levels +- Code blocks have language tags +``` + +## Rules + +- Never reference external docs or the full plan in task descriptions — everything needed is in the description +- Design documents live in `docs/plans/` and are registered via one of `arc plan create` (legacy), `arc share create` (encrypted local default), or `arc share create … --remote` (encrypted remote). The brainstorm skill writes a `<!-- arc-review: kind=… id=… -->` marker as line 1 of the doc — always read the marker before invoking review CLIs to route correctly +- Task descriptions must include actual code guidance, not vague instructions +- Team preparation (teammate labels) is optional — only if user chooses team execution +- The plan skill creates tasks; it does not implement them +- The plan skill never runs `arc create` directly — always delegate to `issue-manager` +- Every task must include a `## Scope Boundary` section — no file modifications outside the `## Files` list +- No two parallelizable tasks may own the same file — resolve overlaps via foundation task, merging, or serialization +- Format all arc content (descriptions, plans, comments) per `skills/arc/_formatting.md` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/plan/evals/evals.json b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/plan/evals/evals.json new file mode 100644 index 0000000..c084da3 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/plan/evals/evals.json @@ -0,0 +1,26 @@ +{ + "skill_name": "plan", + "evals": [ + { + "id": 0, + "eval_name": "marker-legacy-routes-to-arc-plan", + "prompt": "You're given the design doc at `docs/plans/2026-05-01-transcode-refactor.md`. The first line of the file is exactly:\n\n <!-- arc-review: kind=legacy id=42 -->\n\nFollowing the plan skill, walk me through Step 1 (Read the Design) and Step 4 (Read the full plan content first). Show the EXACT shell commands you'd run to (a) read the marker, (b) read the plan content, (c) look up the plan file path if all you had was the ID. Don't actually execute anything — show the commands.", + "expected_output": "Step 1 reads the marker via `head -1`, extracts kind=legacy and id=42, then runs `arc plan show 42` (NOT `arc share show`). Step 4's read uses the same `arc plan show 42`. The file-path lookup uses `arc plan show 42 | grep -oE '^File: \\S+' | awk '{print $2}'` (NOT the `arc share list --json | jq` form, since legacy plans aren't in the share keyring).", + "files": [] + }, + { + "id": 1, + "eval_name": "marker-share-local-routes-to-arc-share", + "prompt": "You're given the design doc at `docs/plans/2026-05-01-queue-migration.md`. The first line is exactly:\n\n <!-- arc-review: kind=share-local id=01HX3K9VPKABCDEF -->\n\nFollowing the plan skill, walk me through Step 1 and Step 4. Show the EXACT shell commands you'd run for (a) reading the marker, (b) reading the plan content, (c) looking up the plan file path from the ID. Don't execute anything.", + "expected_output": "Step 1 reads the marker via `head -1`, extracts kind=share-local and id=01HX3K9VPKABCDEF, then runs `arc share show 01HX3K9VPKABCDEF`. Step 4's content read is the same. The file-path lookup uses `arc share list --json | jq -r '.[] | select(.id==\"01HX3K9VPKABCDEF\") | .plan_file'`. Does NOT call `arc plan show`.", + "files": [] + }, + { + "id": 2, + "eval_name": "no-marker-falls-back-to-share-list", + "prompt": "You're given the design doc at `docs/plans/2025-old-design.md`. The first line of the file is `# Old design` — there's no `<!-- arc-review: ... -->` marker (it predates the marker contract). The user remembers this design was a `share-local` plan but doesn't remember the share ID. Walk me through what you'd do at Step 1 (Read the Design) and how you'd derive the ID. Show exact commands. Don't execute.", + "expected_output": "Detects the missing marker. Falls back to `arc share list --json | jq -r '.[] | select(.plan_file==\"docs/plans/2025-old-design.md\") | .id'` to derive the ID. After getting the ID, runs `arc share show <id>` to read the plan. Notes that this fallback only covers share-local/share-remote plans — if it returns empty, asks the user which review surface the plan was registered on.", + "files": [] + } + ] +} diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/review/SKILL.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/review/SKILL.md new file mode 100644 index 0000000..9baadbc --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/review/SKILL.md @@ -0,0 +1,163 @@ +--- +name: review +description: You MUST use this skill after implementing a task to get code review — especially when the user says "review this", "check my code", "review the changes", or after any implementation task completes. Dispatches the code-reviewer agent with git diff and task spec, then triages feedback by severity. Always prefer this over generic code review when the project uses arc issue tracking. +--- + +# Review — Code Review Dispatch + +Dispatch the `code-reviewer` subagent to review implementation work, then triage findings. + +## Workflow + +Create a TodoWrite checklist with these steps: + +### 1. Get Git SHAs + +Use the `PRE_TASK_SHA` recorded by the implement skill before dispatching the implementer: + +```bash +BASE_SHA=$PRE_TASK_SHA +HEAD_SHA=$(git rev-parse HEAD) +``` + +If `PRE_TASK_SHA` is not available (e.g., standalone review), determine the range manually: + +```bash +# Check recent commits to identify where the task's work begins +git log --oneline -10 +# Set BASE_SHA to the commit before the task's first change +BASE_SHA=$(git rev-parse <commit-before-task>) +HEAD_SHA=$(git rev-parse HEAD) +``` + +### 2. Get Design Context + +If the review was invoked from the implement skill, a design excerpt should be available. Retrieve it: + +```bash +# Get the parent epic of this task +arc show <task-id> --json | jq -r '.parent_id // empty' +# If parent exists, get the epic's plan content +arc show <parent-epic-id> +``` + +Extract the design excerpt relevant to this task — typically the sections covering the types, interfaces, and architectural decisions this task implements. If no parent epic exists or no design is available, skip the design spec section in the dispatch prompt. + +### 3. Dispatch Reviewer + +Use the Agent tool to spawn an `code-reviewer` subagent. Fill the template at `./code-reviewer-prompt.md` with the gathered placeholders (`{TASK_ID}`, `{BASE_SHA}`, `{HEAD_SHA}`, `{DESIGN_EXCERPT}`, `{EVALUATOR_STATUS}`). + +**Model tier:** Follow the Model Selection table in `../build/SKILL.md`. For most reviews, omit `model:` (use the agent's sonnet default). Escalate to `opus` when the diff is large (10+ files), crosses multiple architectural layers, or involves security-sensitive changes. + +### 4. Triage Feedback + +When the reviewer reports back: + +| Severity | Action | +|----------|--------| +| **Critical** | Fix immediately — re-dispatch `builder` with the specific fix. Then re-review. | +| **Important** | Fix before moving to next task — re-dispatch `builder`. Then re-review. | +| **Minor** | Note in arc issue comment for later. Proceed. | +| **Deviation (fix)** | Re-dispatch `builder` with the specific deviation to correct. | +| **Deviation (accept)** | Note the deviation as an arc comment on the task for traceability. Proceed. | + +### 5. Handle Fixes + +If fixes are needed: +1. Re-dispatch `builder` with the specific findings to address +2. After the implementer reports back, re-review (go to step 1 with updated SHAs) +3. Continue until the review is clean (no Critical or Important findings) + +**Circuit breaker**: If 3 review/fix cycles on the same task haven't resolved all findings, STOP. Escalate to the user with a summary of what keeps recurring — the reviewer and implementer may disagree on the approach, or the task spec may be ambiguous. + +### 6. Proceed + +- If all tasks are done → invoke `finish` +- If more tasks remain → return to `implement` for the next task + +## Response Discipline + +Receiving review feedback requires technical evaluation, not emotional performance. Verify before implementing. Ask before assuming. + +### Forbidden Responses + +Never write: + +- "You're absolutely right!" +- "Great point!" +- "Excellent feedback!" +- "Let me implement that now" (before verification) + +These are performative and explicitly violate project discipline. They signal acceptance before understanding. + +### Instead + +- **Restate** the technical requirement in your own words +- **Ask** clarifying questions when the feedback is unclear +- **Push back** with technical reasoning when the feedback is wrong +- **Just start working** — actions beat performative agreement + +### The Verification Pattern + +Apply this pattern to every finding: + +1. **READ** — Read the complete feedback without reacting +2. **UNDERSTAND** — Restate the requirement in your own words (or ask for clarification) +3. **VERIFY** — Check the claim against the actual codebase +4. **EVALUATE** — Is the feedback technically sound for *this* codebase's conventions? +5. **RESPOND** — Technical acknowledgment ("Confirmed, file X line Y has the issue") OR reasoned pushback ("Disagree: file X line Y actually does handle this case — test Z covers it") +6. **IMPLEMENT** — Fix one finding at a time, verify each before moving to the next + +### Triage by Severity + +When the `code-reviewer` reports findings, triage by severity: + +| Severity | Action | +|----------|--------| +| **Critical** | Fix immediately — re-dispatch `builder` with the specific fix. Then re-review. | +| **Important** | Fix before moving to next task — re-dispatch `builder`. Then re-review. | +| **Minor** | Note in arc issue comment for later. Proceed. | +| **Deviation (fix)** | Re-dispatch `builder` with the specific deviation to correct. | +| **Deviation (accept)** | Note the deviation as an arc comment on the task for traceability. Proceed. | + +Never agree performatively to Critical or Important findings. Never dismiss them without technical reasoning. If a finding is wrong, show *why* with evidence from the codebase. + +## Relationship to the Evaluator + +The evaluator is **not always present**. Your dispatch prompt includes an `## Evaluator Status` line that tells you whether the evaluator is running for this task. + +**When Evaluator Status is `active`** (high-risk tasks): + +The evaluator runs in parallel with you. Your concerns are complementary: + +| | Reviewer (you) | Evaluator | +|---|---|---| +| **Focus** | Code quality, conventions, plan adherence | Spec-intent compliance via independent testing | +| **Input** | Git diff + spec | Spec only (no diff) | +| **Modifies code?** | No | Writes ephemeral acceptance tests, then deletes them | + +Focus on code quality, naming, structure, conventions, and plan adherence. Defer behavioral verification to the evaluator's actual tests. + +**When Evaluator Status is `not dispatched`** (default path): + +You are the only reviewer. In addition to code quality and plan adherence, **flag behavioral concerns** — code paths that look like they might not match the spec, edge cases that appear unhandled, logic that seems inconsistent with the task's `## Expected Outcome`. Describe the suspected behavior gap and the code path involved so the orchestrator can decide whether to escalate to the evaluator. + +You are not expected to write or run tests — that's still the evaluator's job if escalated. But you should flag what you see. + +## Contexts + +This skill works in both execution models: + +| Context | How review works | +|---------|-----------------| +| **Single-agent** | Main agent dispatches `code-reviewer` subagent | +| **Team mode** | Team lead dispatches QA teammate or `code-reviewer` subagent | + +## Rules + +- Always review after implementation — don't skip to close +- Re-review after fixes — don't assume fixes are correct +- The reviewer reports; you decide what to do with the findings +- Never make code changes in the review skill — dispatch the implementer for fixes +- Focus on code quality and conventions. Flag behavioral concerns when no evaluator is present. +- Format all arc content (descriptions, plans, comments) per `skills/arc/_formatting.md` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/review/code-reviewer-prompt.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/review/code-reviewer-prompt.md new file mode 100644 index 0000000..5aba811 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/review/code-reviewer-prompt.md @@ -0,0 +1,42 @@ +# Reviewer Prompt Template + +Use this template when dispatching `code-reviewer` for code review. + +**Placeholders:** +- `{TASK_ID}` — arc issue ID +- `{BASE_SHA}` — starting commit SHA +- `{HEAD_SHA}` — ending commit SHA +- `{DESIGN_EXCERPT}` — relevant design section from parent epic, or "none" if not applicable +- `{EVALUATOR_STATUS}` — `active` if evaluator was dispatched for this task, else `not dispatched` + +````text +Review these changes against the task spec and project conventions. + +## Task Spec +<paste output of: arc show {TASK_ID}> + +## Design Spec +{DESIGN_EXCERPT} +If "none", omit this section. + +## Changes +<paste output of: git diff {BASE_SHA}..{HEAD_SHA}> + +## Evaluator Status +{EVALUATOR_STATUS} + +## Report Format + +Report findings in three severities: + +- **Critical** (must fix): correctness bugs, security issues, scope violations, spec deviations +- **Important** (should fix): quality issues, pattern mismatches, naming problems, test gaps +- **Minor** (note for later): style nits, observations, future cleanup candidates + +If a design spec was provided, also report Plan Adherence: +- **ADHERENT** — implementation matches the design +- **DEVIATION (fix)** — implementation diverges from design; recommend fixing +- **DEVIATION (accept)** — implementation diverges from design; recommend accepting the divergence (with reasoning) + +When Evaluator Status is `not dispatched`, also flag behavioral concerns — code paths that might not match spec intent. You do not write or run tests; describe what you see and where. +```` diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/team-dispatch/SKILL.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/team-dispatch/SKILL.md new file mode 100644 index 0000000..6eb4a09 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/team-dispatch/SKILL.md @@ -0,0 +1,166 @@ +--- +name: team-dispatch +description: Deploy an agent team from arc's issue graph. Use when the user wants to parallelize work across multiple agents, parallelize epic tasks, says "deploy team", "spawn teammates", or wants to distribute arc epic tasks by role using teammate labels. +--- + +# Arc Team Deploy + +Deploy an agent team from arc's issue graph. Translates `teammate:*` labels, plans, and dependencies into a Claude Code team with tasks and role-filtered context. + +## When to Invoke + +- User says "deploy team", "create agent team from arc", "spawn teammates from arc" +- User runs `/arc team-deploy` +- User wants to parallelize work on an arc epic across multiple agents + +## Prerequisites + +- An arc project is active (resolved via server path registration or local config) +- An epic exists with child issues labeled `teammate:<role>` (e.g., `teammate:frontend`, `teammate:backend`) +- The arc server is running (`arc server status`) + +## Workflow + +### Step 1: Gather Team Context + +Run `arc team context <epic-id> --json` to get the issue graph grouped by role. + +```bash +arc team context <epic-id> --json +``` + +The JSON output has this structure: + +```json +{ + "epic": { "id": "PROJ-5", "title": "Auth System", "status": "open" }, + "roles": { + "frontend": [ + { "id": "PROJ-5.1", "title": "Login form", "status": "open", "priority": 2, "blocked_by": [] } + ], + "backend": [ + { "id": "PROJ-5.2", "title": "Auth API", "status": "open", "priority": 1, "blocked_by": [] }, + { "id": "PROJ-5.3", "title": "Session middleware", "status": "open", "priority": 2, "blocked_by": ["PROJ-5.2"] } + ] + } +} +``` + +Use the `roles` keys as teammate names and paste each role's issue array into the teammate's dispatch prompt. + +If the user hasn't specified an epic, help them find one: + +```bash +arc list --type=epic --status=open +``` + +### Step 2: Present Team Composition + +Parse the JSON output and present a summary for approval: + +``` +Team composition for "<epic-title>": + + frontend (2 issues): Login form, Signup page + backend (3 issues): Auth API, User model, Session middleware +Proceed with team deployment? [Y/n] +``` + +### Step 3: Create Team and Tasks + +After approval: + +1. **Create the team** via `TeamCreate`: + ``` + team_name: "<epic-title-slug>" + description: "Working on <epic-title>" + ``` + +2. **Create tasks** via `TaskCreate` for each arc issue: + - `subject`: The arc issue title + - `description`: Include the arc issue ID, plan (if any), dependencies, and priority + - `activeForm`: Present continuous of the subject (e.g., "Implementing login form") + +3. **Set task dependencies** via `TaskUpdate` with `addBlockedBy`: + - Map arc dependency IDs to the corresponding task IDs + - Only map dependencies between issues within the same team deployment + +### Step 4: Spawn Teammates + +For each role, spawn a teammate via the `Agent` tool: + +``` +subagent_type: "general-purpose" +team_name: "<team-name>" +name: "<role>" (e.g., "frontend", "backend") +``` + +**Prompt template** for each teammate: + +``` +You are the <role> teammate working on "<epic-title>". + +Your arc role label is teammate:<role>. Focus on your assigned tasks. + +Environment: ARC_TEAMMATE_ROLE=<role> + +Workflow: +1. Check TaskList for your assigned tasks +2. Work on tasks in ID order (lowest first) +3. For each task: + - If labeled `docs-only`: mark in_progress, write documentation, verify formatting, commit, mark completed + - Otherwise: mark in_progress, implement with tests (RED → GREEN → REFACTOR), run test suite, commit, mark completed +4. After completing a task, check TaskList for the next available one +5. Send a message to the team lead when all your tasks are done + +Arc context for your issues: +<paste role-specific issues from the team context JSON> +``` + +### Step 5: Assign Tasks + +Use `TaskUpdate` with `owner` to assign each task to the corresponding teammate role name. + +### Step 6: Monitor and Sync + +As team lead, follow the sync protocol: + +1. **Monitor progress** via `TaskList` — teammates send messages on completion +2. **Verify work** before closing arc issues: + ```bash + arc show <issue-id> # Review the issue + # Check the code changes made by the teammate + ``` +3. **Close verified issues**: + ```bash + arc close <issue-id> --reason "completed by <role>" + ``` +4. **Check for newly unblocked work**: + ```bash + arc ready + ``` +5. **Shutdown teammates** when all work is complete via `SendMessage` with `type: "shutdown_request"` + +## Error Handling + +- If `arc team context` returns empty roles, the epic may not have `teammate:*` labels on children. Suggest labeling first. +- If a teammate reports a blocker, update the arc issue status: `arc update <id> --status=blocked` +- If a task fails, investigate before reassigning — the arc issue may need plan revision. + +## Example Session + +``` +User: Deploy a team for epic PROJ-5 + +1. Run: arc team context PROJ-5 --json +2. Parse: 2 roles (frontend: 2 issues, backend: 3 issues) +3. Present composition → user approves +4. TeamCreate: "auth-system" +5. TaskCreate: 5 tasks (mapped from arc issues) +6. TaskUpdate: set dependencies between tasks +7. Agent spawn: "frontend" teammate (2 assigned tasks) +8. Agent spawn: "backend" teammate (3 assigned tasks) +9. Monitor via TaskList, verify completions +10. arc close verified issues +11. Shutdown teammates when done +``` \ No newline at end of file diff --git a/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/verify/SKILL.md b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/verify/SKILL.md new file mode 100644 index 0000000..2d1aaf1 --- /dev/null +++ b/packages/pi-arc/tests/fixtures/arc-plugin-source/skills/verify/SKILL.md @@ -0,0 +1,83 @@ +--- +name: verify +description: You MUST use this skill before claiming any work is complete, any test passes, or any fix works — especially before arc close, before telling the user "done", or when the user asks "does it work?", "did the tests pass?", "is it fixed?". Requires fresh verification evidence (not cached results). Always prefer this over ad-hoc verification when the project uses arc issue tracking. +--- + +# Verify — Evidence-Based Completion Gates + +Run proof commands and read their output before making any completion claim. + +## Iron Law + +**NO COMPLETION CLAIMS WITHOUT FRESH VERIFICATION EVIDENCE.** + +"It should work" is not evidence. "Tests pass" without output is not evidence. Satisfaction expressed before running the proof command is a red flag. + +## Gate Sequence + +Create a TodoWrite checklist with these steps: + +### 1. IDENTIFY + +What command proves the claim? Examples: +- "Tests pass" → `make test` or `go test ./...` +- "Build succeeds" → `make build` +- "Issue is resolved" → `arc show <id>` (check status) +- "No regressions" → full test suite, not a subset + +### 2. RUN + +Execute the **full** command. At minimum, run all tests affected by the change; running the full suite is safer. Not a subset from memory. Not "I ran it earlier." + +Fresh. Complete. Now. + +### 3. READ + +Read the **FULL** output. Not just the last line. Check: +- Exit code (0 = success) +- Failure count (must be 0, not "some passed") +- Warning count (investigate, don't ignore) +- Any skipped tests (why were they skipped?) + +### 4. VERIFY + +Does the output **actually confirm** the claim? +- "0 failures" confirms "tests pass" — "tests ran" does not +- "exit 0" confirms "build succeeds" — "compiling..." does not +- "status: closed" confirms "issue resolved" — "status: in_progress" does not + +### 5. ONLY THEN + +Make the claim. Reference the evidence: +- "Tests pass: `go test ./...` shows 47 passed, 0 failed" +- "Build succeeds: `make build` exits 0, binary at `./bin/arc`" + +## Red Flags + +You are skipping verification if you: +- Use "should work", "probably passes", "seems fine" +- Express satisfaction before running the proof command +- Run a subset of tests instead of the full suite +- Trust a subagent's report without running the proof command yourself +- Say "tests pass" without showing the output +- Claim "no regressions" without running the full suite +- Close an arc issue before verification + +## Arc Integration + +```bash +# ONLY after verification passes: +arc close <id> -r "Verified: <evidence summary>" +``` + +If verification **fails**, do NOT close the issue. Instead: +- Return to `implement` to fix the failure +- Or invoke `debug` if the failure is unexpected + +## Rules + +- Never close an arc issue without fresh verification evidence +- Never claim completion without running the proof command +- Never trust cached or remembered results — run it fresh +- After verification, proceed to `finish` (session end) or back to `implement` (next task) +- Format all arc content (descriptions, plans, comments) per `skills/arc/_formatting.md` From 31adb18c183e6e1ff06d5e468449f7319dd19147 Mon Sep 17 00:00:00 2001 From: Ben Firestone <ben.firestone@krypticlabs.com> Date: Mon, 18 May 2026 01:19:28 -0700 Subject: [PATCH 14/17] fix(pi-arc): preserve source migration sections narrowly --- packages/pi-arc/scripts/migrate-arc-plugin.py | 393 ++---------------- .../pi-arc/tests/arc-source-sync.test.mjs | 12 +- 2 files changed, 54 insertions(+), 351 deletions(-) diff --git a/packages/pi-arc/scripts/migrate-arc-plugin.py b/packages/pi-arc/scripts/migrate-arc-plugin.py index fa2d440..fac1c0d 100644 --- a/packages/pi-arc/scripts/migrate-arc-plugin.py +++ b/packages/pi-arc/scripts/migrate-arc-plugin.py @@ -77,6 +77,38 @@ def validate_source(src: Path) -> None: ) overlay_text_by_rel[rel] = overlay_path.read_text() +# Preserve only the generated sections that encode Pi's existing executor split. +# This keeps upstream-synced files flowing through normal transforms while +# preventing future syncs from erasing coder/devops routing contracts. +PRESERVED_SECTION_OVERLAYS = [ + ("skills/arc-build/SKILL.md", "## Dispatch Modes\n\n", "## Dispatch Modes\n\n", "\n### 1. Find Next Task"), + ("skills/arc-build/SKILL.md", "### 3. Dispatch Agent\n\n", "### 3. Dispatch Agent\n\n", "\n### 4. Evaluate Result"), + ("skills/arc-build/SKILL.md", "### 4. Evaluate Result\n\n", "### 4. Evaluate Result\n\n", "\n### 6.5. High-Risk Evaluation"), + ("skills/arc-build/SKILL.md", "### 6.5. High-Risk Evaluation (Optional)\n\n", "### 6.5. High-Risk Evaluation (Optional)\n\n", "\n### 7. Close Task"), + ("skills/arc-build/SKILL.md", "### 8. Integration Checkpoint\n\n", "### 8. Integration Checkpoint\n\n", "\n### 9. Repeat"), + ("skills/arc-build/SKILL.md", "## Handle Executor Status\n\n", "## Handle Implementer Status\n\n", "\n## Parallel Patch Protocol"), + ("skills/arc-plan/SKILL.md", "## Labels", "## Labels", "\n### 5. Validate Returned Results"), + ("agents/issue-manager.md", "## Processing Task Manifests\n\n", "## Processing Task Manifests\n\n", "\n## Bulk Operations"), +] + +def section_text(path: Path, start_marker: str, end_marker: str) -> str: + text = path.read_text() + start = text.index(start_marker) + end = text.index(end_marker, start) + return text[start:end] + +section_overlay_text_by_key: dict[tuple[str, str, str], str] = {} +for rel, source_start_marker, target_start_marker, end_marker in PRESERVED_SECTION_OVERLAYS: + overlay_path = ARC_ROOT / rel + if not overlay_path.exists(): + raise SystemExit( + f"Missing required Pi section overlay source: {rel}. " + "Restore tracked resources before running migration." + ) + section_overlay_text_by_key[(rel, target_start_marker, end_marker)] = section_text( + overlay_path, source_start_marker, end_marker + ) + # Clean generated Arc resource directories only. Keep package.json, README, # extension edits, and Pi-only maintainer skills that are not present upstream. for name in ["prompts", "agents"]: @@ -167,10 +199,8 @@ def transform_text(text: str) -> str: # Pi executor rename: builder -> coder (legacy upstream input may still mention builder). text = text.replace("arc-builder", "arc-coder") text = text.replace("builder-prompt.md", "coder-prompt.md") - text = text.replace("`builder`", "`coder`") text = text.replace("agent=\"builder\"", "agent=\"coder\"") text = text.replace("agent: \"builder\"", "agent: \"coder\"") - text = text.replace("dispatching `builder`", "dispatching `coder`") # Relative paths after skill directory renames. text = text.replace("../build/", "../arc-build/") @@ -267,6 +297,8 @@ def replace_section(rel: str, start_marker: str, end_marker: str, replacement: s "## Contexts\n\nThis skill works in both execution models:\n\n| Context | How review works |\n|---------|-----------------|\n| **Single-agent** | Main agent dispatches `code-reviewer` subagent |\n| **Team mode** | Team lead dispatches QA teammate or `code-reviewer` subagent |", "## Contexts\n\nThis skill works in orchestrated Arc execution:\n\n| Context | How review works |\n|---------|-----------------|\n| **Sequential build** | Main agent dispatches `code-reviewer` subagent after the coder reports completion |\n| **Parallel patch batch** | Main agent applies each accepted patch to the main worktree, then dispatches `code-reviewer` against the applied diff |", ), + ("re-dispatch `builder`", "re-dispatch `coder`"), + ("Re-dispatch `builder`", "Re-dispatch `coder`"), ]) patch_file("skills/arc-finish/SKILL.md", [ @@ -294,7 +326,7 @@ def replace_section(rel: str, start_marker: str, end_marker: str, replacement: s "By default, use sequential dispatch. For independent batches with `pi-subagents` available, see [Parallel Patch Protocol](#parallel-patch-protocol) below.", ), ( - "Use the template at `./doc-writer-prompt.md`. Fill placeholder `{TASK_ID}`. For docs-only work, the agent default (`haiku`) is correct — omit `model:` unless the docs task is unusually complex.\n\n**Otherwise** — spawn an `coder` subagent:\n\nUse the template at `./coder-prompt.md`. Fill placeholders (`{TASK_ID}`, `{PRE_TASK_SHA}`, `{DESIGN_EXCERPT}`) and apply Model Selection guidance (see `## Model Selection` above) for the dispatch `model:`.", + "Use the template at `./doc-writer-prompt.md`. Fill placeholder `{TASK_ID}`. For docs-only work, the agent default (`haiku`) is correct — omit `model:` unless the docs task is unusually complex.\n\n**Otherwise** — spawn an `builder` subagent:\n\nUse the template at `./coder-prompt.md`. Fill placeholders (`{TASK_ID}`, `{PRE_TASK_SHA}`, `{DESIGN_EXCERPT}`) and apply Model Selection guidance (see `## Model Selection` above) for the dispatch `model:`.", "Use the template at `./doc-writer-prompt.md`. Fill placeholder `{TASK_ID}`. For docs-only work, the agent default (`haiku`) is correct — omit `model:` unless the docs task is unusually complex.\n\nDispatch preference:\n- If `subagent` is available and `arc-doc-writer` is installed: `subagent({ agent: \"arc-doc-writer\", task: \"<filled prompt>\", context: \"fresh\" })`\n- If `subagent` is available but Arc specialists are missing: run `/arc-subagents-sync`, verify with `subagent({ action: \"list\" })`, then retry.\n- Otherwise: `arc_agent(agent=\"doc-writer\", task=\"<filled prompt>\")`\n\n**Otherwise** — spawn an `coder` subagent:\n\nUse the template at `./coder-prompt.md`. Fill placeholders (`{TASK_ID}`, `{PRE_TASK_SHA}`, `{DESIGN_EXCERPT}`) and apply Model Selection guidance (see `## Model Selection` above) for the dispatch `model:`.\n\nDispatch preference:\n- If `subagent` is available and `arc-coder` is installed: `subagent({ agent: \"arc-coder\", task: \"<filled prompt>\", model: \"<tier-if-needed>\", context: \"fresh\" })`\n- If `subagent` is available but Arc specialists are missing: run `/arc-subagents-sync`, verify with `subagent({ action: \"list\" })`, then retry.\n- Otherwise: `arc_agent(agent=\"coder\", task=\"<filled prompt>\", model=\"<tier-if-needed>\")`", ), ( @@ -830,134 +862,9 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st **Escalation rule:** If a subagent returns `BLOCKED` with a reasoning or capability complaint, re-dispatch with the next tier up before asking the human. Stop escalating at `large` — if `large` also returns `BLOCKED`, escalate to the human with the subagent's blocker summary. """) -replace_section("skills/arc-build/SKILL.md", "## Dispatch Modes\n\n", "\n### 1. Find Next Task", """## Dispatch Modes - -Choose the manifest-driven parallel path first; if the batch is not ready, fall back to sequential dispatch. - -### Parallel (plan-driven) - -If the plan includes a `### Parallel Batch Manifest`, read it first. Select a batch only when all prerequisites are complete and the gates below pass. When the batch is ready, use [Parallel Patch Protocol](#parallel-patch-protocol) below. - -### Sequential (default) - -Tasks are dispatched one at a time through the orchestration loop below. Use this for: -- Most workflows — it's the safe default -- Tasks with any file overlap -- Tasks with dependency ordering (`blocks`/`blockedBy`) -- When you're unsure whether tasks are independent - -### Parallel - -Parallel worktree dispatch is available **only** through an installed `pi-subagents` extension/tool, not through `arc_agent`. Use it only when ALL of these are true: -- `pi-subagents` loaded and the `subagent` tool is available -- Arc agent definitions such as `arc-coder` / `arc-devops` / `arc-doc-writer` are auto-materialized for `pi-subagents` -- 3+ independent tasks remain, or one high-risk evaluator needs a disposable worktree -- No shared files or operational targets between any coder/devops/doc-writer tasks in the batch -- No `blocks`/`blockedBy` dependencies between tasks in the batch -- Do not parallelize live devops operations; live operations require sequential orchestration even when labels/dependencies look independent -- Each task's scope is clearly defined with no ambiguity - -`pi-subagents` worktree mode returns per-task patch files and cleans up temporary worktrees. It does **not** automatically merge changes into the main working tree. The orchestrator must inspect, apply, verify, commit, and close each patch/task explicitly. - -**When NOT to use parallel**: missing `subagent` tool, missing Arc agent definitions, overlapping files, task dependencies, uncertainty about scope, or fewer than 3 implementation tasks. Default to sequential — the cost of serial execution is time; the cost of a bad parallel patch merge is data loss. - -## Orchestration Loop - -Start here by checking whether the plan's `Parallel Batch Manifest` can be dispatched in parallel. - -### 0. Choose Dispatch Mode - -Inspect the plan's `Parallel Batch Manifest` first. If it yields a ready batch and the gates below pass, dispatch that batch through [Parallel Patch Protocol](#parallel-patch-protocol). Otherwise, continue with sequential dispatch. - -**Task tracking**: At the start of implementation, create a task list using the bundled `todo` checklist (via `todo` tool / `/todos`) with one entry per arc issue to implement. This provides a visible progress tracker in the CLI. Update each task as you work: -- `in_progress` when dispatching the subagent -- `completed` when the task is closed in arc - -```bash -# Get the list of tasks to implement -arc list --parent=<epic-id> --status=open --json -``` - -Create a `todo` checklist entry for each, then work through this loop: -""") - -replace_section("skills/arc-build/SKILL.md", "### 3. Dispatch Agent\n\n", "\n### 4. Evaluate Result", """### 3. Dispatch Agent - -Record the current HEAD before dispatching — needed for review if escalated: - -```bash -PRE_TASK_SHA=$(git rev-parse HEAD) -``` - -Resolve executor from issue labels: - -```bash -arc show <task-id> --json | jq -r '.labels[]' | grep '^executor:' -``` - -Routing contract: -- exactly one `executor:coder` → `coder` -- exactly one `executor:devops` → `devops` -- exactly one `executor:docs` → `doc-writer` -- zero executor labels + old `docs-only` label → `doc-writer` (legacy fallback) -- zero executor labels + no `docs-only` label → `coder` (legacy fallback) -- multiple or unknown executor labels → stop and require issue correction before dispatch - -If executor resolution fails, do not dispatch. Add an arc comment describing the invalid labels and request correction. - -**If resolved executor is `doc-writer`** — spawn a `doc-writer` subagent: - -Use the template at `./doc-writer-prompt.md`. Fill placeholder `{TASK_ID}`. For docs work, the agent default (`small`) is correct — omit `model:` unless the docs task is unusually complex. - -Dispatch preference: -- If `subagent` is available and `arc-doc-writer` is installed: `subagent({ agent: "arc-doc-writer", task: "<filled prompt>", context: "fresh", async: true, clarify: false })` -- If `subagent` is available but Arc specialists are missing: Arc specialists should already be auto-materialized. First run `subagent({ action: "doctor" })` and inspect Arc's materialization warning. Use `/arc-subagents-sync` only as a deprecated repair command, then re-check with `subagent({ action: "list" })`. -- Otherwise: `arc_agent(agent="doc-writer", task="<filled prompt>")` - -For async `pi-subagents` dispatches, immediately capture the returned run ID, poll with `subagent({ action: "status", id: "<run-id>" })` or watch `/subagents-status` until terminal, then read the final output before evaluating the report or moving to validation. - -**If resolved executor is `devops`** — spawn a `devops` subagent: - -Use the task spec directly and include evidence expectations in the prompt (changed files, command output, logs/runbook/config proof, and rollback notes when applicable). - -Dispatch preference: -- If `subagent` is available and `arc-devops` is installed: `subagent({ agent: "arc-devops", task: "<task spec and evidence instructions>", context: "fresh", async: true, clarify: false })` -- If `subagent` is available but Arc specialists are missing: Arc specialists should already be auto-materialized. First run `subagent({ action: "doctor" })` and inspect Arc's materialization warning. Use `/arc-subagents-sync` only as a deprecated repair command, then re-check with `subagent({ action: "list" })`. -- Otherwise: `arc_agent(agent="devops", task="<task spec and evidence instructions>")` - -Devops tasks may legitimately finish as evidence-only with no commit when no repository files changed. Treat command output and operational evidence as the deliverable in that case. - -For async `pi-subagents` dispatches, immediately capture the returned run ID, poll with `subagent({ action: "status", id: "<run-id>" })` or watch `/subagents-status` until terminal, then read the final output before evaluating the report or moving to validation. - -**If resolved executor is `coder`** — spawn a `coder` subagent: - -Use the template at `./coder-prompt.md`. Fill placeholders (`{TASK_ID}`, `{PRE_TASK_SHA}`, `{DESIGN_EXCERPT}`) and apply Model Selection guidance (see `## Model Selection` above) for the dispatch `model:`. - -Dispatch preference: -- If `subagent` is available and `arc-coder` is installed: `subagent({ agent: "arc-coder", task: "<filled prompt>", model: "<concrete-model-if-needed>", context: "fresh", async: true, clarify: false })` -- If `subagent` is available but Arc specialists are missing: Arc specialists should already be auto-materialized. First run `subagent({ action: "doctor" })` and inspect Arc's materialization warning. Use `/arc-subagents-sync` only as a deprecated repair command, then re-check with `subagent({ action: "list" })`. -- Otherwise: `arc_agent(agent="coder", task="<filled prompt>", model="<tier-if-needed>")` - -For async `pi-subagents` dispatches, immediately capture the returned run ID, poll with `subagent({ action: "status", id: "<run-id>" })` or watch `/subagents-status` until terminal, then read the final output before evaluating the report or moving to validation. -""") - -replace_section("skills/arc-build/SKILL.md", "Dispatch `spec-reviewer`:\n\n", "\nHandle results:", """Dispatch `spec-reviewer`: - -Use the template at `./spec-reviewer-prompt.md`. Fill placeholders (`{TASK_ID}`, `{BASE_SHA}`, `{HEAD_SHA}`). Spec review is a focused comparison task — the Arc `standard` tier is appropriate unless the spec is unusually large or ambiguous. - -Dispatch preference: -- If `subagent` is available and `arc-spec-reviewer` is installed: `subagent({ agent: "arc-spec-reviewer", task: "<filled prompt>", model: "openai-codex/gpt-5.3-codex", context: "fresh", async: true, clarify: false })` -- If `subagent` is available but Arc specialists are missing: Arc specialists should already be auto-materialized. First run `subagent({ action: "doctor" })` and inspect Arc's materialization warning. Use `/arc-subagents-sync` only as a deprecated repair command, then re-check with `subagent({ action: "list" })`. -- Otherwise: `arc_agent(agent="spec-reviewer", task="<filled prompt>")` - -For async `pi-subagents` dispatches, immediately capture the returned run ID, poll with `subagent({ action: "status", id: "<run-id>" })` or watch `/subagents-status` until terminal, then read the final output before handling compliance results. - -Do **not** substitute the generic `worker` or `reviewer` agent for spec compliance gates. Generic `pi-subagents` agents are not Arc specialists, and manually passing an Anthropic model bypasses Arc's Pi-native model tier policy. If Arc `pi-subagents` definitions are unavailable, use the bundled sequential `arc_agent` fallback. -""") - patch_file("skills/arc-build/SKILL.md", [ ( - "Orchestrate task implementation by dispatching fresh `coder` subagents per task. Each subagent gets a clean context window with just the task description.", + "Orchestrate task implementation by dispatching fresh `builder` subagents per task. Each subagent gets a clean context window with just the task description.", "Orchestrate task implementation by dispatching fresh executor-specific subagents per task. Each subagent gets a clean context window with just the task description.", ), ( @@ -966,174 +873,6 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st ), ]) -replace_section("skills/arc-build/SKILL.md", "### 4. Evaluate Result\n\n", "\n### 6.5. High-Risk Evaluation", """### 4. Evaluate Result - -When the subagent reports back, check its **Status** (one of `DONE | DONE_WITH_CONCERNS | BLOCKED | NEEDS_CONTEXT`) and **Gate Results**. Follow the `## Handle Executor Status` table below for the status-specific action. In all cases, run the project test command fresh yourself — do NOT trust the subagent's report alone. - -For follow-up remediation in steps 5, 6, 6.5, and 8, re-dispatch the resolved executor (`coder`, `devops`, or `doc-writer`) with the specific findings and required fixes. - -**On `DONE`:** -- Run the project tests. If they pass → proceed to step 5 (Spec Compliance Review). -- If tests fail despite a `DONE` report, treat as `BLOCKED`: re-dispatch with the failure output. - -**On `DONE_WITH_CONCERNS`:** -- Read the concerns carefully. -- If the concerns touch correctness or scope (e.g., "I think this edge case isn't handled", "I modified a file outside the spec") — address before review by re-dispatching with specific guidance, or tightening the review prompt. -- If the concerns are observations (e.g., "this file is getting large") — note them as arc comments on the task and proceed to step 5. - -**On `BLOCKED` or `NEEDS_CONTEXT`:** -- Do NOT proceed to review. Do NOT close the task. -- For `NEEDS_CONTEXT`: gather the requested information, re-dispatch with it. -- For `BLOCKED`: assess the blocker per the Handle Executor Status table. Escalate one model tier (`nano` → `small` → `standard` → `large`) per the Model Selection escalation rule, or invoke the `debug` skill if the blocker is a persistent test failure, or split the task if too large, or escalate to the human. -- After 3 re-dispatches on the same task without clean `DONE`, invoke the `debug` skill. - -**If the subagent did not include a Status field** (malformed report): -- Treat as `BLOCKED`. Re-dispatch with an explicit reminder to use the four-status Report Format. - -When re-dispatching, include the previous report's concerns / blockers so the resolved executor knows exactly what to fix: - -``` -Continue implementing this task. A previous attempt reported <status> with these concerns: - -<paste concerns> - -Address each concern and re-report. -``` - -### 5. Spec Compliance Review - -After confirming tests pass, dispatch the `spec-reviewer` to independently verify the implementation matches the spec: - -```bash -BASE_SHA=$PRE_TASK_SHA -``` - -Dispatch `spec-reviewer`: - -Use the template at `./spec-reviewer-prompt.md`. Fill placeholders (`{TASK_ID}`, `{BASE_SHA}`, `{HEAD_SHA}`). Spec review is a focused comparison task — the Arc `standard` tier is appropriate unless the spec is unusually large or ambiguous. - -Dispatch preference: -- If `subagent` is available and `arc-spec-reviewer` is installed: `subagent({ agent: "arc-spec-reviewer", task: "<filled prompt>", model: "openai-codex/gpt-5.3-codex", context: "fresh", async: true, clarify: false })` -- If `subagent` is available but Arc specialists are missing: Arc specialists should already be auto-materialized. First run `subagent({ action: "doctor" })` and inspect Arc's materialization warning. Use `/arc-subagents-sync` only as a deprecated repair command, then re-check with `subagent({ action: "list" })`. -- Otherwise: `arc_agent(agent="spec-reviewer", task="<filled prompt>")` - -For async `pi-subagents` dispatches, immediately capture the returned run ID, poll with `subagent({ action: "status", id: "<run-id>" })` or watch `/subagents-status` until terminal, then read the final output before handling compliance results. - -Do **not** substitute the generic `worker` or `reviewer` agent for spec compliance gates. Generic `pi-subagents` agents are not Arc specialists, and manually passing an Anthropic model bypasses Arc's Pi-native model tier policy. If Arc `pi-subagents` definitions are unavailable, use the bundled sequential `arc_agent` fallback. - -Handle results: -- `COMPLIANT` → proceed to Step 6 -- `ISSUES (Missing)` → re-dispatch the resolved executor with specific gaps listed by the spec reviewer. Re-run spec compliance review after. -- `ISSUES (Extra)` → re-dispatch the resolved executor to remove the extras listed by the spec reviewer. Re-run spec compliance review after. -- `ISSUES (Misunderstood)` → re-dispatch the resolved executor with clarification from the spec reviewer's findings. Re-run spec compliance review after. -- Circuit breaker: 3 spec-review/fix cycles without resolution → escalate to user. - -> **Documentation-executor tasks**: Skip this step when the resolved executor is `doc-writer` (including legacy `docs-only`). The spec-reviewer is designed around code verification (file lists, function signatures, test coverage) and doesn't apply to documentation. For these tasks, the orchestrator verifies formatting/completeness directly: check that all files in `## Files` were created/modified, links resolve, heading hierarchy is correct, code blocks have language tags. -> -> **Devops evidence-only tasks**: If executor resolved to `devops` and no repo files changed, a missing diff/commit is acceptable. Verify the reported devops evidence (commands, outputs, config/runbook state, operational proof) against the task spec before proceeding. - -### 6. Code Quality Review - -Only dispatched after spec compliance passes. Use the `review` skill or dispatch `code-reviewer` directly: - -```bash -HEAD_SHA=$(git rev-parse HEAD) -``` - -Use the template at `../arc-review/reviewer-prompt.md`. Fill placeholders (`{TASK_ID}`, `{BASE_SHA}` = PRE_TASK_SHA recorded earlier, `{HEAD_SHA}` = current HEAD, `{DESIGN_EXCERPT}` from parent epic or "none", `{EVALUATOR_STATUS}` = "active" if evaluator was dispatched, else "not dispatched"). Follow Model Selection above for the dispatch `model:` — `standard` default is appropriate for most reviews. - -**On `{EVALUATOR_STATUS}`:** Decide whether to dispatch the evaluator (step 6.5) BEFORE filling this placeholder. If you plan to run step 6.5 in parallel with step 6, set `{EVALUATOR_STATUS}="active"`. Otherwise set `"not dispatched"`. Step 6.5 has the decision criteria for when to dispatch the evaluator. - -Handle findings: - -| Finding | Action | -|---------|--------| -| **Critical/Important** | Re-dispatch the resolved executor with fixes. Re-review after. | -| **Minor** | Note in arc comment. Proceed. | -| **Deviation (fix)** | Re-dispatch the resolved executor to match the design. | -| **Deviation (accept)** | Log as arc comment: "Accepted deviation: \\<description\\>. Rationale: \\<why\\>." Proceed. | - -Circuit breaker: 3 review/fix cycles on the same finding → escalate to user. - -> **Documentation-executor tasks**: Skip code quality review when the resolved executor is `doc-writer` (including legacy `docs-only`). For substantial documentation changes (developer-facing API docs, architecture docs), optionally dispatch `code-reviewer` for a quality check. -> -> **Devops evidence-only tasks**: Reviewers may receive no code diff when no repo files changed. In that case, review the devops evidence package for completeness and task alignment instead of requiring a commit. -""") - -replace_section("skills/arc-build/SKILL.md", "### 6.5. High-Risk Evaluation (Optional)\n\n", "\n### 7. Close Task", """### 6.5. High-Risk Evaluation (Optional) - -The evaluator is **not dispatched by default**. Dispatch only when: -- Task has a `high-risk` label -- The orchestrator judges the task warrants independent verification (e.g., complex spec with multiple valid interpretations, security-sensitive code, tasks that modify shared contracts) - -When `pi-subagents` is available, dispatch the evaluator through a one-task worktree-isolated parallel run. This gives it a disposable repository copy so it can write acceptance tests and add temporary dependencies without dirtying the main worktree: - -```ts -subagent({ - tasks: [ - { agent: "arc-evaluator", task: "<filled evaluator prompt>", model: "openai-codex/gpt-5.5" } - ], - worktree: true, - concurrency: 1, - context: "fresh", - async: true, - clarify: false -}) -``` - -If `pi-subagents` or `arc-evaluator` is not available, fall back to sequential `arc_agent(agent="evaluator", model="large", task="<filled evaluator prompt>")` and ensure the evaluator does not leave uncommitted artifacts in the main worktree. - -```bash -PARENT=$(arc show <task-id> --json | jq -r '.parent_id // empty') -``` - -Use the template at `./evaluator-prompt.md`. Fill placeholder `{TASK_ID}`. Because evaluation is adversarial verification on high-risk tasks, escalate one tier from the agent default (typically to `large`) — set `model: "large"` on `arc_agent` dispatches unless the task is narrow. For `pi-subagents`, pass the concrete configured large model. - -When you plan to run the evaluator, set the code quality reviewer's `## Evaluator Status` to `active`; otherwise set it to `not dispatched`. - -Triage evaluator findings (for devops tasks, evaluators should also inspect reported devops evidence and can pass evidence-only runs when no repo files changed): - -| Evaluator verdict | Orchestrator action | -|---|---| -| `PASS` | No action — evaluator confirms the spec intent is satisfied. | -| `CONCERNS` | Read the concerns. Re-dispatch the resolved executor if the concerns describe substantive behavior gaps. Otherwise note as arc comments and proceed. | -| `FAIL — Spec-Intent Gap` | Re-dispatch the resolved executor with the evaluator's quoted spec text and the failing behavior description. | -| `FAIL — Missing Behavior` | Re-dispatch the resolved executor — the spec requires behavior that wasn't built. | -| `FAIL — Edge Case` | Lower-severity. Re-dispatch the resolved executor if the spec clearly implies the edge case; otherwise record as a known limitation. | -| `ERROR — Cannot Test` | The public API is insufficient. Re-dispatch the resolved executor with a request to expose the needed surface. | -| `BLOCKED` | Evaluator itself is blocked. Escalate per the Model Selection rules or involve the human. | -""") - -replace_section("skills/arc-build/SKILL.md", "### 8. Integration Checkpoint\n\n", "\n### 9. Repeat", """### 8. Integration Checkpoint - -After closing 2-3 related tasks, or before switching to a new epic phase, run the full integration test suite: - -```bash -make test-integration -``` - -This catches cross-task regressions that individual executor gate checks won't — each executor subagent only validates its own task's scope. Do not wait until all tasks are complete to discover integration failures. - -If integration tests fail: -- Identify which task's changes caused the failure -- Re-dispatch the resolved executor with the failing test details and the relevant task context -- If the failure spans multiple tasks, invoke the `debug` skill -""") - -replace_section("skills/arc-build/SKILL.md", "## Handle Implementer Status\n\n", "\n## Parallel Patch Protocol", """## Handle Executor Status - -Every `coder`, `devops`, and `doc-writer` dispatch returns one of four terminal statuses. Handle each explicitly: - -| Status | Orchestrator action | -|---|---| -| `DONE` | Proceed to spec review, then code review. | -| `DONE_WITH_CONCERNS` | Read the concerns. If they're about correctness or scope, address before review (re-dispatch or tighten review prompt). If they're observations (file getting large, naming doubt), note them as arc comments on the task and proceed to review — close only after a later dispatch yields a clean `DONE`. | -| `BLOCKED` | Assess the blocker: (1) context problem → provide missing context, re-dispatch same tier; (2) reasoning limit → re-dispatch one tier up per the Model Selection escalation rule; (3) task too large → split and re-plan; (4) plan is wrong → escalate to human. Never retry the same dispatch unchanged. | -| `NEEDS_CONTEXT` | Gather the specific missing information. Re-dispatch with it in the prompt. | - -**Never close a task** whose last report was `BLOCKED`, `NEEDS_CONTEXT`, or `DONE_WITH_CONCERNS` unresolved. Re-dispatch until you have a clean `DONE` — then close. -""") - patch_file("agents/coder.md", [ ( "description: Use this agent for implementing a single task using TDD. Dispatched by the implement skill with a task description from arc. Receives task context, implements following RED → GREEN → REFACTOR → GATE, commits results, and reports back.", @@ -1153,6 +892,7 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st patch_file("skills/arc-build/coder-prompt.md", [ ("# Implementer Prompt Template", "# Coder Prompt Template"), + ("Use this template when dispatching `builder` for a task.", "Use this template when dispatching `coder` for a task."), ("You are implementing arc task {TASK_ID}.", "You are coding arc task {TASK_ID}."), ]) @@ -1164,21 +904,10 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st ]) patch_file("skills/arc-plan/SKILL.md", [ - ( - "## Labels\n- T3: docs-only", - "## Labels\n- T1: executor:coder\n- T2: executor:devops\n- T3: executor:docs", - ), - ( - "For each task, check whether **all** files in its `## Files` section are documentation (`.md`, `.txt`, `README`, `CHANGELOG`, or anything under `docs/`). If so, include it in the `## Labels` section with `docs-only`. Doc-only tasks skip TDD — the `implement` skill routes them to `doc-writer` instead of `coder`.", - "Every new implementation task must include exactly one executor label: `executor:coder`, `executor:devops`, or `executor:docs`. Multiple or missing executor labels are plan failures.\n\nUse `executor:docs` as the source of truth for new docs tasks. `docs-only` is a legacy input handled by arc-build only and must not be emitted for new tasks.\n\n### Executor Classification\n\n| Task content | Executor label |\n|---|---|\n| Application/library/CLI code changes | `executor:coder` |\n| Kubernetes, Terraform/OpenTofu, Helm, Kustomize, ArgoCD, CI/CD, cloud infra, runbooks, live operational checks | `executor:devops` |\n| Documentation-only changes | `executor:docs` |\n\nLive operations require the `live-ops-approved` label plus explicit task-body authorization.\n\n### DevOps Task Description Format\n\nUse this format for tasks labeled `executor:devops`:\n\n```markdown\n## Executor\nexecutor:devops\n\n## Live Operation Authorization\n- Explicit task-body authorization: <required statement>\n- Requires label: live-ops-approved (for live operations)\n\n## Target Environment\n- <environment>\n\n## Allowed Operations\n- <allowed commands/actions>\n\n## Scope Boundary\n- <in-scope systems>\n- <out-of-scope systems>\n\n## Preflight Checks\n1. <check>\n\n## Execution Steps\n1. <step>\n\n## Rollback Plan\n1. <rollback step>\n\n## Validation/Post-checks\n1. <validation command>\n\n## Evidence to Report\n- <logs/screenshots/command output>\n```", - ), ( "For `docs-only` tasks, omit `## Test Command` and use `## Verification` instead:", "For `executor:docs` tasks, omit `## Test Command` and use `## Verification` instead. `docs-only` is legacy input only:", ), -]) - -patch_file("skills/arc-plan/SKILL.md", [ ( "**Model tier:** `issue-manager` defaults to `nano` — the right tier for low-reasoning CLI formatting and bulk issue creation. For this dispatch, omit `model:`. See the Model Selection table in `../arc-build/SKILL.md` for the full guidance.", "**Model tier:** `issue-manager` defaults to `nano` — the right tier for low-reasoning CLI formatting and bulk issue creation. Model profile: issue creation uses the issueManager profile when configured via `/arc-models`; otherwise it falls back to the legacy tier/frontmatter behavior. This work is mostly CLI formatting, so the recommended profile uses gpt-5.4-mini with thinking off. For this dispatch, omit `model:`. See the Model Selection table in `../arc-build/SKILL.md` for the full guidance.", @@ -1302,46 +1031,6 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st """ insert_before_if_missing(rel, marker, insertion, "## Supervisor Escalation") -replace_section("agents/issue-manager.md", "## Processing Task Manifests\n\n", "\n## Bulk Operations", """## Processing Task Manifests - -When receiving a structured manifest from the `plan` or `brainstorm` skills, parse the `## Epic` and `## Tasks` sections to assemble the manifest, then process it in phases: - -1. **Create the epic first** and capture the epic ID. -2. **Create all child tasks** with the epic as parent before applying dependencies. - ```bash - arc create "Task title" --type=task --parent=<epic-id> --stdin <<'EOF' - Full multi-line description here. - EOF - ``` -3. **Capture the complete task-name-to-ID table**. -4. **Apply dependencies only after all child IDs exist**. - ```bash - arc dep add <real-later-id> <real-earlier-id> --type=blocks - ``` -5. **Validate executor labels before applying labels**: every child task must carry exactly one executor label. - - Allowed values: `executor:coder`, `executor:devops`, `executor:docs` - - Missing executor labels or multiple executor labels on one task are manifest failures. -6. **Apply labels after dependencies**, or in the same post-creation phase. - ```bash - # Labels are managed via the REST API (no CLI command exists) - # If CLI cannot apply labels directly, report the exact labels - # per task in the summary so the dispatcher can apply them. - ``` -7. **Return the final ID table, dependency summary, and `## Timing` summary**. - -Print `[arc-issue-manager] phase=<name> status=start|done elapsed_ms=<n>` progress lines around each phase (`epic`, `child_tasks`, `dependencies`, `labels`, and optional `verification`) so long-running issue creation is observable. - -**Concurrency note:** Concurrent child-task creation is future work pending Arc CLI/server concurrency verification. Do not claim true parallel CLI issue creation is safe today. - -**Handling partial failures**: If a task creation fails mid-manifest: -- Continue creating the remaining tasks in order — do not abort the manifest -- Report partial results clearly: "Created 4/5 tasks. T3 failed: `<error message>`" -- Include the ID mapping for all successfully created tasks so the dispatcher can act on what exists -- Do not attempt to clean up already-created tasks — the dispatcher will decide - -This is the primary interface used by the `plan` and `brainstorm` skills for bulk issue creation. -""") - patch_file("agents/issue-manager.md", [ ( "- Summarize any errors encountered\n- Provide next steps if applicable", @@ -1349,6 +1038,14 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st ), ]) +# Restore tracked executor-split sections after all upstream-derived patches. +for (rel, target_start_marker, end_marker), replacement in section_overlay_text_by_key.items(): + target = ARC_ROOT / rel + text = target.read_text() + start = text.index(target_start_marker) + end = text.index(end_marker, start) + target.write_text(text[:start] + replacement + text[end:]) + print(f"Migrated arc plugin resources from {SRC}") print(f"Package root: {ARC_ROOT}") diff --git a/packages/pi-arc/tests/arc-source-sync.test.mjs b/packages/pi-arc/tests/arc-source-sync.test.mjs index 1a59d23..91558a2 100644 --- a/packages/pi-arc/tests/arc-source-sync.test.mjs +++ b/packages/pi-arc/tests/arc-source-sync.test.mjs @@ -1,6 +1,6 @@ import { execFileSync } from 'node:child_process'; import { createHash } from 'node:crypto'; -import { cpSync, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync } from 'node:fs'; +import { cpSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, relative } from 'node:path'; import { test } from 'node:test'; @@ -131,7 +131,10 @@ test('migration script deterministically maps upstream builder resources to code const tempRoot = mkdtempSync(join(tmpdir(), 'pi-arc-source-sync-')); const packageCopy = join(tempRoot, 'pi-arc'); try { - cpSync('.', packageCopy, { recursive: true }); + mkdirSync(packageCopy, { recursive: true }); + for (const rel of ['agents', 'prompts', 'scripts', 'skills', 'tests/fixtures']) { + cpSync(rel, join(packageCopy, rel), { recursive: true }); + } const originalDevopsOverlay = read('agents/devops.md'); execFileSync('python3', ['scripts/migrate-arc-plugin.py', UPSTREAM_ARC_SOURCE], { @@ -147,7 +150,10 @@ test('migration script deterministically maps upstream builder resources to code assert.equal(existsSync(join(packageCopy, 'skills/arc-source-sync/SKILL.md')), false); assert.equal(readFileSync(join(packageCopy, 'agents/devops.md'), 'utf8'), originalDevopsOverlay); - const firstSnapshot = snapshotGeneratedResources(packageCopy); + const generatedText = snapshotGeneratedResources(packageCopy); + assert.doesNotMatch(generatedText, /arc-builder|builder-prompt\.md|agent=\"builder\"|agent: \"builder\"|\bbuilder\b/); + + const firstSnapshot = generatedText; execFileSync('python3', ['scripts/migrate-arc-plugin.py', UPSTREAM_ARC_SOURCE], { cwd: packageCopy, encoding: 'utf8', From c1d5fe754804b368aedd69852f2aa62d929daf85 Mon Sep 17 00:00:00 2001 From: Ben Firestone <ben.firestone@krypticlabs.com> Date: Mon, 18 May 2026 01:54:55 -0700 Subject: [PATCH 15/17] fix(pi-arc): make review contracts source-sync safe --- packages/pi-arc/agents/code-reviewer.md | 20 +- packages/pi-arc/agents/spec-reviewer.md | 14 +- packages/pi-arc/scripts/migrate-arc-plugin.py | 179 ++++++++++++++---- packages/pi-arc/skills/arc-build/SKILL.md | 2 +- packages/pi-arc/skills/arc-review/SKILL.md | 20 +- .../skills/arc-review/code-reviewer-prompt.md | 8 +- .../tests/arc-build-executor-routing.test.mjs | 2 + .../pi-arc/tests/arc-source-sync.test.mjs | 33 +++- ...rc-subagents-auto-materialization.test.mjs | 23 +++ .../pi-arc/tests/arc-subagents-sync.test.mjs | 8 + 10 files changed, 241 insertions(+), 68 deletions(-) diff --git a/packages/pi-arc/agents/code-reviewer.md b/packages/pi-arc/agents/code-reviewer.md index ab8d837..d4bf4e4 100644 --- a/packages/pi-arc/agents/code-reviewer.md +++ b/packages/pi-arc/agents/code-reviewer.md @@ -1,5 +1,5 @@ --- -description: Use this agent for reviewing code changes against a task spec and project conventions. Dispatched by the review skill with a git diff and task description. Reports findings categorized by severity. Read-only — never modifies code. +description: Use this agent for reviewing implementation changes (code, infrastructure/config, and runbook updates) against a task spec and project conventions. Dispatched by the review skill with a git diff and task description. Reports findings categorized by severity. Read-only — never modifies code. tools: - bash - read @@ -10,9 +10,11 @@ model: standard # Arc Reviewer Agent -You are a code review agent. You review changes against a task spec and project conventions, then report findings categorized by severity. +You are a review agent. You review implementation changes against a task spec and project conventions, then report findings categorized by severity. -You are read-only. You never make code changes or close issues. You report — the dispatching agent decides what to do with your findings. +You are read-only. You never make code changes or close issues. Review only; return findings only. Do not edit files. You report — the dispatching agent decides what to do with your findings. + +For `executor:devops` tasks, treat infrastructure/config/runbook updates and operational evidence as first-class review targets. ## Workflow @@ -20,13 +22,13 @@ You are read-only. You never make code changes or close issues. You report — t 2. **Read the design spec** if provided — this is the approved design that the task implements 3. **Read the git diff** provided or retrieve via `git diff <base>..<head>` 4. **Check spec compliance**: Does the implementation match what was requested? Missing features? Extra scope? -5. **Check code quality**: Naming consistency, structure, error handling, edge cases, SOLID principles -6. **Check test quality**: Coverage of happy path, edge cases, error conditions. Meaningful assertions. +5. **Check implementation/artifact quality**: Naming consistency, structure, error handling, edge cases, maintainability, and safety across code, infrastructure/config, and runbook changes. +6. **Check validation quality**: Tests, validation commands, preflight/post-checks, rollback proof, and evidence coverage for happy paths, edge cases, and error conditions. Assertions or evidence should be meaningful. 7. **Check plan adherence** (only if design spec is provided): Does the implementation match the approved design's decisions? - - Naming: Do types, functions, and variables match the names specified in the design? + - Naming: Do types, functions, variables, config keys, operational targets, and runbook steps match the names specified in the design? - File organization: Are files placed where the design specified? - - Architecture: Does the implementation follow the patterns and structures described in the design? - - Type choices: Are the correct types used as specified? (Contract tests catch most of these, but review catches indirect violations like unnecessary type conversions) + - Architecture/operations: Does the implementation follow the patterns, structures, target environment constraints, and operational safety requirements described in the design? + - Type/config choices: Are the correct types, schemas, settings, and operational parameters used as specified? (Contract tests catch many of these, but review catches indirect violations like unnecessary type conversions or unsafe config drift.) 8. **Report findings** using the output format below ## Output Format @@ -78,7 +80,7 @@ The dispatching agent decides whether to fix or accept each deviation. - **Technical evaluation, not performative agreement.** No "Great work!" or "Looks good!" without specific evidence. If code is clean, say "No issues found." - **Be specific.** "Error handling could be improved" is useless. "The `CreateUser` handler on line 45 swallows the database error and returns 200" is actionable. - **Check against the spec.** The task description says what should be built. If the implementation diverges, that's a Critical finding. -- **Check against conventions.** Read the project's CLAUDE.md if it exists. Scan 2-3 existing files in the same directory as the changed code to identify naming, structure, and error-handling patterns. Deviations from established patterns are Important findings. +- **Check against conventions.** Read the project's AGENTS.md or legacy CLAUDE.md if it exists. Scan 2-3 existing files in the same directory as the changed artifacts to identify naming, structure, error-handling, configuration, and runbook patterns. Deviations from established patterns are Important findings. - **Check against the design.** If a design spec is provided, the implementation must match its type definitions, naming choices, and architectural decisions. Deviations that are arguably improvements still get flagged — the orchestrator decides whether to accept them. ## Supervisor Escalation diff --git a/packages/pi-arc/agents/spec-reviewer.md b/packages/pi-arc/agents/spec-reviewer.md index 3d57b38..2010fb2 100644 --- a/packages/pi-arc/agents/spec-reviewer.md +++ b/packages/pi-arc/agents/spec-reviewer.md @@ -16,11 +16,13 @@ You have a fresh context window. Everything you need is in your dispatch prompt. ## Iron Law -**Do NOT trust the implementer's report.** The report may be incomplete, inaccurate, or optimistic. You MUST verify everything by reading actual code. +**Do NOT trust the implementer's report.** The report may be incomplete, inaccurate, or optimistic. You MUST verify everything by reading the actual changed artifacts and supplied evidence. + +For `executor:devops` tasks, verify target environment constraints, allowed operations, required preflight checks, rollback plan, validation steps, infrastructure/config/runbook changes, and operational evidence requirements from the spec while remaining review-only. ## Your Job -Read the implementation code and verify against the task spec: +Read the changed artifacts and any supplied operational evidence, then verify against the task spec: ### Missing requirements - Did they implement everything specified in `## Steps`? @@ -43,10 +45,10 @@ Read the implementation code and verify against the task spec: ## How to Verify 1. Read the task's `## Files` section — identify every file that should exist or be modified -2. Read each file. Compare actual code against what `## Steps` specified +2. Read each file and supplied evidence. Compare the actual changed artifacts, operational steps, and command output against what `## Steps` specified. 3. Check for files changed that aren't in `## Files` (use `git diff --name-only` if a base SHA is provided) -4. Check for extra functions/types/exports beyond what the spec describes -5. Check test coverage alignment: compare the task's `## Expected Outcome` against the implementer's test assertions. Do the tests verify the behaviors the spec describes, or do they only test implementation details? Flag gaps where a spec behavior has no corresponding test assertion. +4. Check for extra functions/types/exports, configuration keys, operational actions, or runbook steps beyond what the spec describes +5. Check validation coverage alignment: compare the task's `## Expected Outcome` against tests, validation commands, and reported evidence. Do they verify the behaviors or operational outcomes the spec describes, or do they only test implementation details? Flag gaps where a spec behavior has no corresponding validation assertion or evidence. ## Supervisor Escalation @@ -76,7 +78,7 @@ Use `COMPLIANT` only when the implementation matches the spec exactly — everyt ## Rules - Never modify code — you are read-only -- Never trust the implementer's report — read the actual code +- Never trust the implementer's report — read the actual changed artifacts and supplied evidence - Never interact with the user — report back to the dispatching agent - Never manage arc issues — the dispatcher handles arc state - Flag extras with the same severity as omissions — over-building is a spec violation diff --git a/packages/pi-arc/scripts/migrate-arc-plugin.py b/packages/pi-arc/scripts/migrate-arc-plugin.py index fac1c0d..fabaab5 100644 --- a/packages/pi-arc/scripts/migrate-arc-plugin.py +++ b/packages/pi-arc/scripts/migrate-arc-plugin.py @@ -77,37 +77,6 @@ def validate_source(src: Path) -> None: ) overlay_text_by_rel[rel] = overlay_path.read_text() -# Preserve only the generated sections that encode Pi's existing executor split. -# This keeps upstream-synced files flowing through normal transforms while -# preventing future syncs from erasing coder/devops routing contracts. -PRESERVED_SECTION_OVERLAYS = [ - ("skills/arc-build/SKILL.md", "## Dispatch Modes\n\n", "## Dispatch Modes\n\n", "\n### 1. Find Next Task"), - ("skills/arc-build/SKILL.md", "### 3. Dispatch Agent\n\n", "### 3. Dispatch Agent\n\n", "\n### 4. Evaluate Result"), - ("skills/arc-build/SKILL.md", "### 4. Evaluate Result\n\n", "### 4. Evaluate Result\n\n", "\n### 6.5. High-Risk Evaluation"), - ("skills/arc-build/SKILL.md", "### 6.5. High-Risk Evaluation (Optional)\n\n", "### 6.5. High-Risk Evaluation (Optional)\n\n", "\n### 7. Close Task"), - ("skills/arc-build/SKILL.md", "### 8. Integration Checkpoint\n\n", "### 8. Integration Checkpoint\n\n", "\n### 9. Repeat"), - ("skills/arc-build/SKILL.md", "## Handle Executor Status\n\n", "## Handle Implementer Status\n\n", "\n## Parallel Patch Protocol"), - ("skills/arc-plan/SKILL.md", "## Labels", "## Labels", "\n### 5. Validate Returned Results"), - ("agents/issue-manager.md", "## Processing Task Manifests\n\n", "## Processing Task Manifests\n\n", "\n## Bulk Operations"), -] - -def section_text(path: Path, start_marker: str, end_marker: str) -> str: - text = path.read_text() - start = text.index(start_marker) - end = text.index(end_marker, start) - return text[start:end] - -section_overlay_text_by_key: dict[tuple[str, str, str], str] = {} -for rel, source_start_marker, target_start_marker, end_marker in PRESERVED_SECTION_OVERLAYS: - overlay_path = ARC_ROOT / rel - if not overlay_path.exists(): - raise SystemExit( - f"Missing required Pi section overlay source: {rel}. " - "Restore tracked resources before running migration." - ) - section_overlay_text_by_key[(rel, target_start_marker, end_marker)] = section_text( - overlay_path, source_start_marker, end_marker - ) # Clean generated Arc resource directories only. Keep package.json, README, # extension edits, and Pi-only maintainer skills that are not present upstream. @@ -297,8 +266,8 @@ def replace_section(rel: str, start_marker: str, end_marker: str, replacement: s "## Contexts\n\nThis skill works in both execution models:\n\n| Context | How review works |\n|---------|-----------------|\n| **Single-agent** | Main agent dispatches `code-reviewer` subagent |\n| **Team mode** | Team lead dispatches QA teammate or `code-reviewer` subagent |", "## Contexts\n\nThis skill works in orchestrated Arc execution:\n\n| Context | How review works |\n|---------|-----------------|\n| **Sequential build** | Main agent dispatches `code-reviewer` subagent after the coder reports completion |\n| **Parallel patch batch** | Main agent applies each accepted patch to the main worktree, then dispatches `code-reviewer` against the applied diff |", ), - ("re-dispatch `builder`", "re-dispatch `coder`"), - ("Re-dispatch `builder`", "Re-dispatch `coder`"), + ("re-dispatch `builder`", "re-dispatch the resolved executor (`coder`, `devops`, or `doc-writer`)"), + ("Re-dispatch `builder`", "Re-dispatch the resolved executor"), ]) patch_file("skills/arc-finish/SKILL.md", [ @@ -871,6 +840,12 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st "- Never close a task if the implementer reported `BLOCKED`, `NEEDS_CONTEXT`, or unresolved `DONE_WITH_CONCERNS` without re-dispatching", "- Never close a task if the resolved executor reported `BLOCKED`, `NEEDS_CONTEXT`, or unresolved `DONE_WITH_CONCERNS` without re-dispatching", ), + ("re-dispatch `builder`", "re-dispatch `coder`"), + ("Re-dispatch `builder`", "Re-dispatch `coder`"), + ( + "Every `builder` and `doc-writer` dispatch returns one of four terminal statuses. Handle each explicitly:", + "Every `coder` and `doc-writer` dispatch returns one of four terminal statuses. Handle each explicitly:", + ), ]) patch_file("agents/coder.md", [ @@ -908,6 +883,10 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st "For `docs-only` tasks, omit `## Test Command` and use `## Verification` instead:", "For `executor:docs` tasks, omit `## Test Command` and use `## Verification` instead. `docs-only` is legacy input only:", ), + ( + "For each task, check whether **all** files in its `## Files` section are documentation (`.md`, `.txt`, `README`, `CHANGELOG`, or anything under `docs/`). If so, include it in the `## Labels` section with `docs-only`. Doc-only tasks skip TDD — the `implement` skill routes them to `doc-writer` instead of `builder`.", + "For each task, check whether **all** files in its `## Files` section are documentation (`.md`, `.txt`, `README`, `CHANGELOG`, or anything under `docs/`). If so, include it in the `## Labels` section with `docs-only`. Doc-only tasks skip TDD — the `implement` skill routes them to `doc-writer` instead of `coder`.", + ), ( "**Model tier:** `issue-manager` defaults to `nano` — the right tier for low-reasoning CLI formatting and bulk issue creation. For this dispatch, omit `model:`. See the Model Selection table in `../arc-build/SKILL.md` for the full guidance.", "**Model tier:** `issue-manager` defaults to `nano` — the right tier for low-reasoning CLI formatting and bulk issue creation. Model profile: issue creation uses the issueManager profile when configured via `/arc-models`; otherwise it falls back to the legacy tier/frontmatter behavior. This work is mostly CLI formatting, so the recommended profile uses gpt-5.4-mini with thinking off. For this dispatch, omit `model:`. See the Model Selection table in `../arc-build/SKILL.md` for the full guidance.", @@ -967,7 +946,11 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st replace_section("skills/arc-review/SKILL.md", "### 3. Dispatch Reviewer\n\n", "\n### 4. Triage Feedback", """### 3. Dispatch Reviewer -Fill the template at `./code-reviewer-prompt.md` with the gathered placeholders (`{TASK_ID}`, `{BASE_SHA}`, `{HEAD_SHA}`, `{DESIGN_EXCERPT}`, `{EVALUATOR_STATUS}`). Preserve the template's review-only instruction (`Review only; return findings only. Do not edit files.`) and avoid adding wording that asks the reviewer to apply fixes directly. Prefer true `pi-subagents` so longer reviews are visible in `/subagents-status`: +Fill the template at `./code-reviewer-prompt.md` with the gathered placeholders (`{TASK_ID}`, `{BASE_SHA}`, `{HEAD_SHA}`, `{DESIGN_EXCERPT}`, `{EVALUATOR_STATUS}`, `{EXECUTOR_CONTEXT}`). Preserve the template's review-only instruction (`Review only; return findings only. Do not edit files.`) and avoid adding wording that asks the reviewer to apply fixes directly. Prefer true `pi-subagents` so longer reviews are visible in `/subagents-status`. + +Set `{EXECUTOR_CONTEXT}` from the task label: +- If task label is `executor:devops`, include a short devops review context block that calls out target environment, allowed operations, preflight checks, rollback, validation, and required devops evidence/config/runbook review. +- Otherwise set `{EXECUTOR_CONTEXT}` to `none`. Dispatch preference (use **async** so longer reviews appear in `/subagents-status`): - Primary: `subagent({ agent: "arc-code-reviewer", task: "<filled prompt>", context: "fresh", async: true, clarify: false })` @@ -986,6 +969,14 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st "Review only; return findings only. Do not edit files.", ) patch_file("skills/arc-review/code-reviewer-prompt.md", [ + ( + "- `{EVALUATOR_STATUS}` — `active` if evaluator was dispatched for this task, else `not dispatched`", + "- `{EVALUATOR_STATUS}` — `active` if evaluator was dispatched for this task, else `not dispatched`\n- `{EXECUTOR_CONTEXT}` — `executor:devops` context block for devops tasks, else `none`", + ), + ( + "## Evaluator Status\n{EVALUATOR_STATUS}\n\n## Report Format", + "## Evaluator Status\n{EVALUATOR_STATUS}\n\n## Executor Context\n{EXECUTOR_CONTEXT}\nIf `executor:devops`, review infrastructure/config/runbook diffs and required operational evidence for target environment, allowed operations, preflight, rollback, and validation.\nIf `none`, omit this section.\n\n## Report Format", + ), ( "- **Critical** (must fix): correctness bugs, security issues, scope violations, spec deviations", "- **Critical** (blocking): correctness bugs, security issues, scope violations, spec deviations", @@ -994,6 +985,69 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st "- **Important** (should fix): quality issues, pattern mismatches, naming problems, test gaps", "- **Important** (address before proceeding): quality issues, pattern mismatches, naming problems, test gaps", ), + ( + "When Evaluator Status is `not dispatched`, also flag behavioral concerns — code paths that might not match spec intent. You do not write or run tests; describe what you see and where.", + "When Evaluator Status is `not dispatched`, also flag behavioral concerns — changed artifacts, operational steps, or code paths that might not match spec intent. You do not write or run tests; describe what you see and where.", + ), +]) + +patch_file("agents/code-reviewer.md", [ + ( + "description: Use this agent for reviewing code changes against a task spec and project conventions. Dispatched by the review skill with a git diff and task description. Reports findings categorized by severity. Read-only — never modifies code.", + "description: Use this agent for reviewing implementation changes (code, infrastructure/config, and runbook updates) against a task spec and project conventions. Dispatched by the review skill with a git diff and task description. Reports findings categorized by severity. Read-only — never modifies code.", + ), + ( + "You are read-only. You never make code changes or close issues. You report — the dispatching agent decides what to do with your findings.", + "You are read-only. You never make code changes or close issues. Review only; return findings only. Do not edit files. You report — the dispatching agent decides what to do with your findings.", + ), +]) + +insert_before_if_missing( + "agents/code-reviewer.md", + "## Workflow", + "For `executor:devops` tasks, treat infrastructure/config/runbook updates and operational evidence as first-class review targets.\n\n", + "For `executor:devops` tasks, treat infrastructure/config/runbook updates and operational evidence as first-class review targets.", +) + +insert_before_if_missing( + "agents/spec-reviewer.md", + "## Your Job", + "For `executor:devops` tasks, verify target environment constraints, allowed operations, required preflight checks, rollback plan, validation steps, infrastructure/config/runbook changes, and operational evidence requirements from the spec while remaining review-only.\n\n", + "For `executor:devops` tasks, verify target environment constraints, allowed operations, required preflight checks, rollback plan, validation steps, infrastructure/config/runbook changes, and operational evidence requirements from the spec while remaining review-only.", +) + +patch_file("agents/spec-reviewer.md", [ + ( + "**Do NOT trust the implementer's report.** The report may be incomplete, inaccurate, or optimistic. You MUST verify everything by reading actual code.", + "**Do NOT trust the implementer's report.** The report may be incomplete, inaccurate, or optimistic. You MUST verify everything by reading the actual changed artifacts and supplied evidence.", + ), + ( + "Read the implementation code and verify against the task spec:", + "Read the changed artifacts and any supplied operational evidence, then verify against the task spec:", + ), + ( + "2. Read each file. Compare actual code against what `## Steps` specified\n3. Check for files changed that aren't in `## Files` (use `git diff --name-only` if a base SHA is provided)\n4. Check for extra functions/types/exports beyond what the spec describes\n5. Check test coverage alignment: compare the task's `## Expected Outcome` against the implementer's test assertions. Do the tests verify the behaviors the spec describes, or do they only test implementation details? Flag gaps where a spec behavior has no corresponding test assertion.", + "2. Read each file and supplied evidence. Compare the actual changed artifacts, operational steps, and command output against what `## Steps` specified.\n3. Check for files changed that aren't in `## Files` (use `git diff --name-only` if a base SHA is provided)\n4. Check for extra functions/types/exports, configuration keys, operational actions, or runbook steps beyond what the spec describes\n5. Check validation coverage alignment: compare the task's `## Expected Outcome` against tests, validation commands, and reported evidence. Do they verify the behaviors or operational outcomes the spec describes, or do they only test implementation details? Flag gaps where a spec behavior has no corresponding validation assertion or evidence.", + ), + ( + "- Never trust the implementer's report — read the actual code", + "- Never trust the implementer's report — read the actual changed artifacts and supplied evidence", + ), +]) + +patch_file("agents/code-reviewer.md", [ + ( + "You are a code review agent. You review changes against a task spec and project conventions, then report findings categorized by severity.", + "You are a review agent. You review implementation changes against a task spec and project conventions, then report findings categorized by severity.", + ), + ( + "5. **Check code quality**: Naming consistency, structure, error handling, edge cases, SOLID principles\n6. **Check test quality**: Coverage of happy path, edge cases, error conditions. Meaningful assertions.\n7. **Check plan adherence** (only if design spec is provided): Does the implementation match the approved design's decisions?\n - Naming: Do types, functions, and variables match the names specified in the design?\n - File organization: Are files placed where the design specified?\n - Architecture: Does the implementation follow the patterns and structures described in the design?\n - Type choices: Are the correct types used as specified? (Contract tests catch most of these, but review catches indirect violations like unnecessary type conversions)\n8. **Report findings** using the output format below", + "5. **Check implementation/artifact quality**: Naming consistency, structure, error handling, edge cases, maintainability, and safety across code, infrastructure/config, and runbook changes.\n6. **Check validation quality**: Tests, validation commands, preflight/post-checks, rollback proof, and evidence coverage for happy paths, edge cases, and error conditions. Assertions or evidence should be meaningful.\n7. **Check plan adherence** (only if design spec is provided): Does the implementation match the approved design's decisions?\n - Naming: Do types, functions, variables, config keys, operational targets, and runbook steps match the names specified in the design?\n - File organization: Are files placed where the design specified?\n - Architecture/operations: Does the implementation follow the patterns, structures, target environment constraints, and operational safety requirements described in the design?\n - Type/config choices: Are the correct types, schemas, settings, and operational parameters used as specified? (Contract tests catch many of these, but review catches indirect violations like unnecessary type conversions or unsafe config drift.)\n8. **Report findings** using the output format below", + ), + ( + "- **Check against conventions.** Read the project's CLAUDE.md if it exists. Scan 2-3 existing files in the same directory as the changed code to identify naming, structure, and error-handling patterns. Deviations from established patterns are Important findings.", + "- **Check against conventions.** Read the project's AGENTS.md or legacy CLAUDE.md if it exists. Scan 2-3 existing files in the same directory as the changed artifacts to identify naming, structure, error-handling, configuration, and runbook patterns. Deviations from established patterns are Important findings.", + ), ]) SUPERVISOR_SECTIONS = { @@ -1038,14 +1092,55 @@ def insert_before_if_missing(rel: str, marker: str, insertion: str, sentinel: st ), ]) -# Restore tracked executor-split sections after all upstream-derived patches. -for (rel, target_start_marker, end_marker), replacement in section_overlay_text_by_key.items(): - target = ARC_ROOT / rel - text = target.read_text() - start = text.index(target_start_marker) - end = text.index(end_marker, start) - target.write_text(text[:start] + replacement + text[end:]) - +# Re-apply explicit Pi executor-split contract sections after upstream-derived patches. +# These are static, reviewed transformations (not snapshots of current files). +# Keep this list narrow: only sections where upstream lacks Pi executor/devops semantics. +EXPLICIT_PI_CONTRACT_SECTIONS = [ + ( + 'skills/arc-build/SKILL.md', + '## Dispatch Modes\n\n', + '\n### 1. Find Next Task', + "## Dispatch Modes\n\nChoose the manifest-driven parallel path first; if the batch is not ready, fall back to sequential dispatch.\n\n### Parallel (plan-driven)\n\nIf the plan includes a `### Parallel Batch Manifest`, read it first. Select a batch only when all prerequisites are complete and the gates below pass. When the batch is ready, use [Parallel Patch Protocol](#parallel-patch-protocol) below.\n\n### Sequential (default)\n\nTasks are dispatched one at a time through the orchestration loop below. Use this for:\n- Most workflows — it's the safe default\n- Tasks with any file overlap\n- Tasks with dependency ordering (`blocks`/`blockedBy`)\n- When you're unsure whether tasks are independent\n\n### Parallel\n\nParallel worktree dispatch is available **only** through an installed `pi-subagents` extension/tool, not through `arc_agent`. Use it only when ALL of these are true:\n- `pi-subagents` loaded and the `subagent` tool is available\n- Arc agent definitions such as `arc-coder` / `arc-devops` / `arc-doc-writer` are auto-materialized for `pi-subagents`\n- 3+ independent tasks remain, or one high-risk evaluator needs a disposable worktree\n- No shared files or operational targets between any coder/devops/doc-writer tasks in the batch\n- No `blocks`/`blockedBy` dependencies between tasks in the batch\n- Do not parallelize live devops operations; live operations require sequential orchestration even when labels/dependencies look independent\n- Each task's scope is clearly defined with no ambiguity\n\n`pi-subagents` worktree mode returns per-task patch files and cleans up temporary worktrees. It does **not** automatically merge changes into the main working tree. The orchestrator must inspect, apply, verify, commit, and close each patch/task explicitly.\n\n**When NOT to use parallel**: missing `subagent` tool, missing Arc agent definitions, overlapping files, task dependencies, uncertainty about scope, or fewer than 3 implementation tasks. Default to sequential — the cost of serial execution is time; the cost of a bad parallel patch merge is data loss.\n\n## Orchestration Loop\n\nStart here by checking whether the plan's `Parallel Batch Manifest` can be dispatched in parallel.\n\n### 0. Choose Dispatch Mode\n\nInspect the plan's `Parallel Batch Manifest` first. If it yields a ready batch and the gates below pass, dispatch that batch through [Parallel Patch Protocol](#parallel-patch-protocol). Otherwise, continue with sequential dispatch.\n\n**Task tracking**: At the start of implementation, create a task list using the bundled `todo` checklist (via `todo` tool / `/todos`) with one entry per arc issue to implement. This provides a visible progress tracker in the CLI. Update each task as you work:\n- `in_progress` when dispatching the subagent\n- `completed` when the task is closed in arc\n\n```bash\n# Get the list of tasks to implement\narc list --parent=<epic-id> --status=open --json\n```\n\nCreate a `todo` checklist entry for each, then work through this loop:\n", + ), + ( + 'skills/arc-build/SKILL.md', + '### 4. Evaluate Result\n\n', + '\n### 6.5. High-Risk Evaluation', + '### 4. Evaluate Result\n\nWhen the subagent reports back, check its **Status** (one of `DONE | DONE_WITH_CONCERNS | BLOCKED | NEEDS_CONTEXT`) and **Gate Results**. Follow the `## Handle Executor Status` table below for the status-specific action. In all cases, run the project test command fresh yourself — do NOT trust the subagent\'s report alone.\n\nFor follow-up remediation in steps 5, 6, 6.5, and 8, re-dispatch the resolved executor (`coder`, `devops`, or `doc-writer`) with the specific findings and required fixes.\n\n**On `DONE`:**\n- Run the project tests. If they pass → proceed to step 5 (Spec Compliance Review).\n- If tests fail despite a `DONE` report, treat as `BLOCKED`: re-dispatch with the failure output.\n\n**On `DONE_WITH_CONCERNS`:**\n- Read the concerns carefully.\n- If the concerns touch correctness or scope (e.g., "I think this edge case isn\'t handled", "I modified a file outside the spec") — address before review by re-dispatching with specific guidance, or tightening the review prompt.\n- If the concerns are observations (e.g., "this file is getting large") — note them as arc comments on the task and proceed to step 5.\n\n**On `BLOCKED` or `NEEDS_CONTEXT`:**\n- Do NOT proceed to review. Do NOT close the task.\n- For `NEEDS_CONTEXT`: gather the requested information, re-dispatch with it.\n- For `BLOCKED`: assess the blocker per the Handle Executor Status table. Escalate one model tier (`nano` → `small` → `standard` → `large`) per the Model Selection escalation rule, or invoke the `debug` skill if the blocker is a persistent test failure, or split the task if too large, or escalate to the human.\n- After 3 re-dispatches on the same task without clean `DONE`, invoke the `debug` skill.\n\n**If the subagent did not include a Status field** (malformed report):\n- Treat as `BLOCKED`. Re-dispatch with an explicit reminder to use the four-status Report Format.\n\nWhen re-dispatching, include the previous report\'s concerns / blockers so the resolved executor knows exactly what to fix:\n\n```\nContinue implementing this task. A previous attempt reported <status> with these concerns:\n\n<paste concerns>\n\nAddress each concern and re-report.\n```\n\n### 5. Spec Compliance Review\n\nAfter confirming tests pass, dispatch the `spec-reviewer` to independently verify the implementation matches the spec:\n\n```bash\nBASE_SHA=$PRE_TASK_SHA\n```\n\nDispatch `spec-reviewer`:\n\nUse the template at `./spec-reviewer-prompt.md`. Fill placeholders (`{TASK_ID}`, `{BASE_SHA}`, `{HEAD_SHA}`). Spec review is a focused comparison task — the Arc `standard` tier is appropriate unless the spec is unusually large or ambiguous.\n\nDispatch preference:\n- If `subagent` is available and `arc-spec-reviewer` is installed: `subagent({ agent: "arc-spec-reviewer", task: "<filled prompt>", model: "openai-codex/gpt-5.3-codex", context: "fresh", async: true, clarify: false })`\n- If `subagent` is available but Arc specialists are missing: Arc specialists should already be auto-materialized. First run `subagent({ action: "doctor" })` and inspect Arc\'s materialization warning. Use `/arc-subagents-sync` only as a deprecated repair command, then re-check with `subagent({ action: "list" })`.\n- Otherwise: `arc_agent(agent="spec-reviewer", task="<filled prompt>")`\n\nFor async `pi-subagents` dispatches, immediately capture the returned run ID, poll with `subagent({ action: "status", id: "<run-id>" })` or watch `/subagents-status` until terminal, then read the final output before handling compliance results.\n\nDo **not** substitute the generic `worker` or `reviewer` agent for spec compliance gates. Generic `pi-subagents` agents are not Arc specialists, and manually passing an Anthropic model bypasses Arc\'s Pi-native model tier policy. If Arc `pi-subagents` definitions are unavailable, use the bundled sequential `arc_agent` fallback.\n\nHandle results:\n- `COMPLIANT` → proceed to Step 6\n- `ISSUES (Missing)` → re-dispatch the resolved executor with specific gaps listed by the spec reviewer. Re-run spec compliance review after.\n- `ISSUES (Extra)` → re-dispatch the resolved executor to remove the extras listed by the spec reviewer. Re-run spec compliance review after.\n- `ISSUES (Misunderstood)` → re-dispatch the resolved executor with clarification from the spec reviewer\'s findings. Re-run spec compliance review after.\n- Circuit breaker: 3 spec-review/fix cycles without resolution → escalate to user.\n\n> **Documentation-executor tasks**: Skip this step when the resolved executor is `doc-writer` (including legacy `docs-only`). The spec-reviewer is designed around code verification (file lists, function signatures, test coverage) and doesn\'t apply to documentation. For these tasks, the orchestrator verifies formatting/completeness directly: check that all files in `## Files` were created/modified, links resolve, heading hierarchy is correct, code blocks have language tags.\n>\n> **Devops evidence-only tasks**: If executor resolved to `devops` and no repo files changed, a missing diff/commit is acceptable. Verify the reported devops evidence (commands, outputs, config/runbook state, operational proof) against the task spec before proceeding.\n\n### 6. Code Quality Review\n\nOnly dispatched after spec compliance passes. Use the `review` skill or dispatch `code-reviewer` directly:\n\n```bash\nHEAD_SHA=$(git rev-parse HEAD)\n```\n\nUse the template at `../arc-review/code-reviewer-prompt.md`. Fill placeholders (`{TASK_ID}`, `{BASE_SHA}` = PRE_TASK_SHA recorded earlier, `{HEAD_SHA}` = current HEAD, `{DESIGN_EXCERPT}` from parent epic or "none", `{EVALUATOR_STATUS}` = "active" if evaluator was dispatched, else "not dispatched", `{EXECUTOR_CONTEXT}` = devops context block when the resolved executor is `devops`, else "none"). Follow Model Selection above for the dispatch `model:` — `standard` default is appropriate for most reviews.\n\n**On `{EVALUATOR_STATUS}`:** Decide whether to dispatch the evaluator (step 6.5) BEFORE filling this placeholder. If you plan to run step 6.5 in parallel with step 6, set `{EVALUATOR_STATUS}="active"`. Otherwise set `"not dispatched"`. Step 6.5 has the decision criteria for when to dispatch the evaluator.\n\nHandle findings:\n\n| Finding | Action |\n|---------|--------|\n| **Critical/Important** | Re-dispatch the resolved executor with fixes. Re-review after. |\n| **Minor** | Note in arc comment. Proceed. |\n| **Deviation (fix)** | Re-dispatch the resolved executor to match the design. |\n| **Deviation (accept)** | Log as arc comment: "Accepted deviation: \\<description\\>. Rationale: \\<why\\>." Proceed. |\n\nCircuit breaker: 3 review/fix cycles on the same finding → escalate to user.\n\n> **Documentation-executor tasks**: Skip code quality review when the resolved executor is `doc-writer` (including legacy `docs-only`). For substantial documentation changes (developer-facing API docs, architecture docs), optionally dispatch `code-reviewer` for a quality check.\n>\n> **Devops evidence-only tasks**: Reviewers may receive no code diff when no repo files changed. In that case, review the devops evidence package for completeness and task alignment instead of requiring a commit.\n', + ), + ( + 'skills/arc-build/SKILL.md', + '### 6.5. High-Risk Evaluation (Optional)\n\n', + '\n### 7. Close Task', + '### 6.5. High-Risk Evaluation (Optional)\n\nThe evaluator is **not dispatched by default**. Dispatch only when:\n- Task has a `high-risk` label\n- The orchestrator judges the task warrants independent verification (e.g., complex spec with multiple valid interpretations, security-sensitive code, tasks that modify shared contracts)\n\nWhen `pi-subagents` is available, dispatch the evaluator through a one-task worktree-isolated parallel run. This gives it a disposable repository copy so it can write acceptance tests and add temporary dependencies without dirtying the main worktree:\n\n```ts\nsubagent({\n tasks: [\n { agent: "arc-evaluator", task: "<filled evaluator prompt>", model: "openai-codex/gpt-5.5" }\n ],\n worktree: true,\n concurrency: 1,\n context: "fresh",\n async: true,\n clarify: false\n})\n```\n\nIf `pi-subagents` or `arc-evaluator` is not available, fall back to sequential `arc_agent(agent="evaluator", model="large", task="<filled evaluator prompt>")` and ensure the evaluator does not leave uncommitted artifacts in the main worktree.\n\n```bash\nPARENT=$(arc show <task-id> --json | jq -r \'.parent_id // empty\')\n```\n\nUse the template at `./evaluator-prompt.md`. Fill placeholder `{TASK_ID}`. Because evaluation is adversarial verification on high-risk tasks, escalate one tier from the agent default (typically to `large`) — set `model: "large"` on `arc_agent` dispatches unless the task is narrow. For `pi-subagents`, pass the concrete configured large model.\n\nWhen you plan to run the evaluator, set the code quality reviewer\'s `## Evaluator Status` to `active`; otherwise set it to `not dispatched`.\n\nTriage evaluator findings (for devops tasks, evaluators should also inspect reported devops evidence and can pass evidence-only runs when no repo files changed):\n\n| Evaluator verdict | Orchestrator action |\n|---|---|\n| `PASS` | No action — evaluator confirms the spec intent is satisfied. |\n| `CONCERNS` | Read the concerns. Re-dispatch the resolved executor if the concerns describe substantive behavior gaps. Otherwise note as arc comments and proceed. |\n| `FAIL — Spec-Intent Gap` | Re-dispatch the resolved executor with the evaluator\'s quoted spec text and the failing behavior description. |\n| `FAIL — Missing Behavior` | Re-dispatch the resolved executor — the spec requires behavior that wasn\'t built. |\n| `FAIL — Edge Case` | Lower-severity. Re-dispatch the resolved executor if the spec clearly implies the edge case; otherwise record as a known limitation. |\n| `ERROR — Cannot Test` | The public API is insufficient. Re-dispatch the resolved executor with a request to expose the needed surface. |\n| `BLOCKED` | Evaluator itself is blocked. Escalate per the Model Selection rules or involve the human. |\n', + ), + ( + 'skills/arc-build/SKILL.md', + '### 8. Integration Checkpoint\n\n', + '\n### 9. Repeat', + "### 8. Integration Checkpoint\n\nAfter closing 2-3 related tasks, or before switching to a new epic phase, run the full integration test suite:\n\n```bash\nmake test-integration\n```\n\nThis catches cross-task regressions that individual executor gate checks won't — each executor subagent only validates its own task's scope. Do not wait until all tasks are complete to discover integration failures.\n\nIf integration tests fail:\n- Identify which task's changes caused the failure\n- Re-dispatch the resolved executor with the failing test details and the relevant task context\n- If the failure spans multiple tasks, invoke the `debug` skill\n", + ), + ( + 'skills/arc-build/SKILL.md', + '## Handle Implementer Status\n\n', + '\n## Parallel Patch Protocol', + "## Handle Executor Status\n\nEvery `coder`, `devops`, and `doc-writer` dispatch returns one of four terminal statuses. Handle each explicitly:\n\n| Status | Orchestrator action |\n|---|---|\n| `DONE` | Proceed to spec review, then code review. |\n| `DONE_WITH_CONCERNS` | Read the concerns. If they're about correctness or scope, address before review (re-dispatch or tighten review prompt). If they're observations (file getting large, naming doubt), note them as arc comments on the task and proceed to review — close only after a later dispatch yields a clean `DONE`. |\n| `BLOCKED` | Assess the blocker: (1) context problem → provide missing context, re-dispatch same tier; (2) reasoning limit → re-dispatch one tier up per the Model Selection escalation rule; (3) task too large → split and re-plan; (4) plan is wrong → escalate to human. Never retry the same dispatch unchanged. |\n| `NEEDS_CONTEXT` | Gather the specific missing information. Re-dispatch with it in the prompt. |\n\n**Never close a task** whose last report was `BLOCKED`, `NEEDS_CONTEXT`, or `DONE_WITH_CONCERNS` unresolved. Re-dispatch until you have a clean `DONE` — then close.\n", + ), + ( + 'skills/arc-plan/SKILL.md', + '## Labels', + '\n### 5. Validate Returned Results', + '## Labels\n- T1: executor:coder\n- T2: executor:devops\n- T3: executor:docs\n\n## Required Output\n| Task | Arc ID | Title |\n|------|--------|-------|\n| Epic | ... | ... |\n| T1 | ... | ... |\n\n## Timing\n| Phase | elapsed_ms |\n|-------|------------|\n| epic | ... |\n| child_tasks | ... |\n| dependencies | ... |\n| labels | ... |\n```\n\nThe `## Timing` section is required for bulk issue creation; use `unknown` for a phase only if the issue-manager could not capture a timestamp.\n\n**IMPORTANT**: The epic description MUST contain the complete approved design. The agent reads the plan file directly to avoid any summarization or content loss. The plan file is ephemeral; the epic description is the permanent record.\n\nEvery new implementation task must include exactly one executor label: `executor:coder`, `executor:devops`, or `executor:docs`. Multiple or missing executor labels are plan failures.\n\nUse `executor:docs` as the source of truth for new docs tasks. `docs-only` is a legacy input handled by arc-build only and must not be emitted for new tasks.\n\n### Executor Classification\n\n| Task content | Executor label |\n|---|---|\n| Application/library/CLI code changes | `executor:coder` |\n| Kubernetes, Terraform/OpenTofu, Helm, Kustomize, ArgoCD, CI/CD, cloud infra, runbooks, live operational checks | `executor:devops` |\n| Documentation-only changes | `executor:docs` |\n\nLive operations require the `live-ops-approved` label plus explicit task-body authorization.\n\n### DevOps Task Description Format\n\nUse this format for tasks labeled `executor:devops`:\n\n```markdown\n## Executor\nexecutor:devops\n\n## Live Operation Authorization\n- Explicit task-body authorization: <required statement>\n- Requires label: live-ops-approved (for live operations)\n\n## Target Environment\n- <environment>\n\n## Allowed Operations\n- <allowed commands/actions>\n\n## Scope Boundary\n- <in-scope systems>\n- <out-of-scope systems>\n\n## Preflight Checks\n1. <check>\n\n## Execution Steps\n1. <step>\n\n## Rollback Plan\n1. <rollback step>\n\n## Validation/Post-checks\n1. <validation command>\n\n## Evidence to Report\n- <logs/screenshots/command output>\n```\n', + ), + ( + 'agents/issue-manager.md', + '## Processing Task Manifests\n\n', + '\n## Bulk Operations', + '## Processing Task Manifests\n\nWhen receiving a structured manifest from the `plan` or `brainstorm` skills, parse the `## Epic` and `## Tasks` sections to assemble the manifest, then process it in phases:\n\n1. **Create the epic first** and capture the epic ID.\n2. **Create all child tasks** with the epic as parent before applying dependencies.\n ```bash\n arc create "Task title" --type=task --parent=<epic-id> --stdin <<\'EOF\'\n Full multi-line description here.\n EOF\n ```\n3. **Capture the complete task-name-to-ID table**.\n4. **Apply dependencies only after all child IDs exist**.\n ```bash\n arc dep add <real-later-id> <real-earlier-id> --type=blocks\n ```\n5. **Validate executor labels before applying labels**: every child task must carry exactly one executor label.\n - Allowed values: `executor:coder`, `executor:devops`, `executor:docs`\n - Missing executor labels or multiple executor labels on one task are manifest failures.\n6. **Apply labels after dependencies**, or in the same post-creation phase.\n ```bash\n # Labels are managed via the REST API (no CLI command exists)\n # If CLI cannot apply labels directly, report the exact labels\n # per task in the summary so the dispatcher can apply them.\n ```\n7. **Return the final ID table, dependency summary, and `## Timing` summary**.\n\nPrint `[arc-issue-manager] phase=<name> status=start|done elapsed_ms=<n>` progress lines around each phase (`epic`, `child_tasks`, `dependencies`, `labels`, and optional `verification`) so long-running issue creation is observable.\n\n**Concurrency note:** Concurrent child-task creation is future work pending Arc CLI/server concurrency verification. Do not claim true parallel CLI issue creation is safe today.\n\n**Handling partial failures**: If a task creation fails mid-manifest:\n- Continue creating the remaining tasks in order — do not abort the manifest\n- Report partial results clearly: "Created 4/5 tasks. T3 failed: `<error message>`"\n- Include the ID mapping for all successfully created tasks so the dispatcher can act on what exists\n- Do not attempt to clean up already-created tasks — the dispatcher will decide\n\nThis is the primary interface used by the `plan` and `brainstorm` skills for bulk issue creation.\n', + ), +] +for rel, start_marker, end_marker, replacement in EXPLICIT_PI_CONTRACT_SECTIONS: + replace_section(rel, start_marker, end_marker, replacement) print(f"Migrated arc plugin resources from {SRC}") print(f"Package root: {ARC_ROOT}") diff --git a/packages/pi-arc/skills/arc-build/SKILL.md b/packages/pi-arc/skills/arc-build/SKILL.md index b4e67a0..e02fb7b 100644 --- a/packages/pi-arc/skills/arc-build/SKILL.md +++ b/packages/pi-arc/skills/arc-build/SKILL.md @@ -284,7 +284,7 @@ Only dispatched after spec compliance passes. Use the `review` skill or dispatch HEAD_SHA=$(git rev-parse HEAD) ``` -Use the template at `../arc-review/reviewer-prompt.md`. Fill placeholders (`{TASK_ID}`, `{BASE_SHA}` = PRE_TASK_SHA recorded earlier, `{HEAD_SHA}` = current HEAD, `{DESIGN_EXCERPT}` from parent epic or "none", `{EVALUATOR_STATUS}` = "active" if evaluator was dispatched, else "not dispatched"). Follow Model Selection above for the dispatch `model:` — `standard` default is appropriate for most reviews. +Use the template at `../arc-review/code-reviewer-prompt.md`. Fill placeholders (`{TASK_ID}`, `{BASE_SHA}` = PRE_TASK_SHA recorded earlier, `{HEAD_SHA}` = current HEAD, `{DESIGN_EXCERPT}` from parent epic or "none", `{EVALUATOR_STATUS}` = "active" if evaluator was dispatched, else "not dispatched", `{EXECUTOR_CONTEXT}` = devops context block when the resolved executor is `devops`, else "none"). Follow Model Selection above for the dispatch `model:` — `standard` default is appropriate for most reviews. **On `{EVALUATOR_STATUS}`:** Decide whether to dispatch the evaluator (step 6.5) BEFORE filling this placeholder. If you plan to run step 6.5 in parallel with step 6, set `{EVALUATOR_STATUS}="active"`. Otherwise set `"not dispatched"`. Step 6.5 has the decision criteria for when to dispatch the evaluator. diff --git a/packages/pi-arc/skills/arc-review/SKILL.md b/packages/pi-arc/skills/arc-review/SKILL.md index 9c77398..5ab2509 100644 --- a/packages/pi-arc/skills/arc-review/SKILL.md +++ b/packages/pi-arc/skills/arc-review/SKILL.md @@ -45,7 +45,11 @@ Extract the design excerpt relevant to this task — typically the sections cove ### 3. Dispatch Reviewer -Fill the template at `./code-reviewer-prompt.md` with the gathered placeholders (`{TASK_ID}`, `{BASE_SHA}`, `{HEAD_SHA}`, `{DESIGN_EXCERPT}`, `{EVALUATOR_STATUS}`). Preserve the template's review-only instruction (`Review only; return findings only. Do not edit files.`) and avoid adding wording that asks the reviewer to apply fixes directly. Prefer true `pi-subagents` so longer reviews are visible in `/subagents-status`: +Fill the template at `./code-reviewer-prompt.md` with the gathered placeholders (`{TASK_ID}`, `{BASE_SHA}`, `{HEAD_SHA}`, `{DESIGN_EXCERPT}`, `{EVALUATOR_STATUS}`, `{EXECUTOR_CONTEXT}`). Preserve the template's review-only instruction (`Review only; return findings only. Do not edit files.`) and avoid adding wording that asks the reviewer to apply fixes directly. Prefer true `pi-subagents` so longer reviews are visible in `/subagents-status`. + +Set `{EXECUTOR_CONTEXT}` from the task label: +- If task label is `executor:devops`, include a short devops review context block that calls out target environment, allowed operations, preflight checks, rollback, validation, and required devops evidence/config/runbook review. +- Otherwise set `{EXECUTOR_CONTEXT}` to `none`. Dispatch preference (use **async** so longer reviews appear in `/subagents-status`): - Primary: `subagent({ agent: "arc-code-reviewer", task: "<filled prompt>", context: "fresh", async: true, clarify: false })` @@ -62,16 +66,16 @@ When the reviewer reports back: | Severity | Action | |----------|--------| -| **Critical** | Fix immediately — re-dispatch `coder` with the specific fix. Then re-review. | -| **Important** | Fix before moving to next task — re-dispatch `coder`. Then re-review. | +| **Critical** | Fix immediately — re-dispatch the resolved executor (`coder`, `devops`, or `doc-writer`) with the specific fix. Then re-review. | +| **Important** | Fix before moving to next task — re-dispatch the resolved executor (`coder`, `devops`, or `doc-writer`). Then re-review. | | **Minor** | Note in arc issue comment for later. Proceed. | -| **Deviation (fix)** | Re-dispatch `coder` with the specific deviation to correct. | +| **Deviation (fix)** | Re-dispatch the resolved executor with the specific deviation to correct. | | **Deviation (accept)** | Note the deviation as an arc comment on the task for traceability. Proceed. | ### 5. Handle Fixes If fixes are needed: -1. Re-dispatch `coder` with the specific findings to address +1. Re-dispatch the resolved executor with the specific findings to address 2. After the implementer reports back, re-review (go to step 1 with updated SHAs) 3. Continue until the review is clean (no Critical or Important findings) @@ -121,10 +125,10 @@ When the `code-reviewer` reports findings, triage by severity: | Severity | Action | |----------|--------| -| **Critical** | Fix immediately — re-dispatch `coder` with the specific fix. Then re-review. | -| **Important** | Fix before moving to next task — re-dispatch `coder`. Then re-review. | +| **Critical** | Fix immediately — re-dispatch the resolved executor (`coder`, `devops`, or `doc-writer`) with the specific fix. Then re-review. | +| **Important** | Fix before moving to next task — re-dispatch the resolved executor (`coder`, `devops`, or `doc-writer`). Then re-review. | | **Minor** | Note in arc issue comment for later. Proceed. | -| **Deviation (fix)** | Re-dispatch `coder` with the specific deviation to correct. | +| **Deviation (fix)** | Re-dispatch the resolved executor with the specific deviation to correct. | | **Deviation (accept)** | Note the deviation as an arc comment on the task for traceability. Proceed. | Never agree performatively to Critical or Important findings. Never dismiss them without technical reasoning. If a finding is wrong, show *why* with evidence from the codebase. diff --git a/packages/pi-arc/skills/arc-review/code-reviewer-prompt.md b/packages/pi-arc/skills/arc-review/code-reviewer-prompt.md index 7202898..a3e97a8 100644 --- a/packages/pi-arc/skills/arc-review/code-reviewer-prompt.md +++ b/packages/pi-arc/skills/arc-review/code-reviewer-prompt.md @@ -8,6 +8,7 @@ Use this template when dispatching `code-reviewer` for code review. - `{HEAD_SHA}` — ending commit SHA - `{DESIGN_EXCERPT}` — relevant design section from parent epic, or "none" if not applicable - `{EVALUATOR_STATUS}` — `active` if evaluator was dispatched for this task, else `not dispatched` +- `{EXECUTOR_CONTEXT}` — `executor:devops` context block for devops tasks, else `none` ````text Review these changes against the task spec and project conventions. @@ -27,6 +28,11 @@ If "none", omit this section. ## Evaluator Status {EVALUATOR_STATUS} +## Executor Context +{EXECUTOR_CONTEXT} +If `executor:devops`, review infrastructure/config/runbook diffs and required operational evidence for target environment, allowed operations, preflight, rollback, and validation. +If `none`, omit this section. + ## Report Format Report findings in three severities: @@ -40,5 +46,5 @@ If a design spec was provided, also report Plan Adherence: - **DEVIATION (fix)** — implementation diverges from design; recommend fixing - **DEVIATION (accept)** — implementation diverges from design; recommend accepting the divergence (with reasoning) -When Evaluator Status is `not dispatched`, also flag behavioral concerns — code paths that might not match spec intent. You do not write or run tests; describe what you see and where. +When Evaluator Status is `not dispatched`, also flag behavioral concerns — changed artifacts, operational steps, or code paths that might not match spec intent. You do not write or run tests; describe what you see and where. ```` diff --git a/packages/pi-arc/tests/arc-build-executor-routing.test.mjs b/packages/pi-arc/tests/arc-build-executor-routing.test.mjs index fc71d18..3c09483 100644 --- a/packages/pi-arc/tests/arc-build-executor-routing.test.mjs +++ b/packages/pi-arc/tests/arc-build-executor-routing.test.mjs @@ -27,6 +27,8 @@ test('arc-build documents devops dispatch and evidence-only completion', () => { assert.match(source, /evidence-only/i); assert.match(source, /no commit/i); assert.match(source, /devops evidence/i); + assert.match(source, /\{EXECUTOR_CONTEXT\}/); + assert.match(source, /resolved executor is `devops`/i); }); test('arc-build follow-up loops re-dispatch the resolved executor', () => { diff --git a/packages/pi-arc/tests/arc-source-sync.test.mjs b/packages/pi-arc/tests/arc-source-sync.test.mjs index 91558a2..e82ed70 100644 --- a/packages/pi-arc/tests/arc-source-sync.test.mjs +++ b/packages/pi-arc/tests/arc-source-sync.test.mjs @@ -119,6 +119,8 @@ test('migration script codifies coder/devops split overlays', () => { assert.match(source, /builder_prompt_path = ARC_ROOT \/ "skills" \/ "arc-build" \/ "builder-prompt\.md"/); assert.match(source, /builder_prompt_path\.unlink\(\)/); assert.doesNotMatch(source, /arc_agent\(agent=\\?"builder/); + assert.doesNotMatch(source, /PRESERVED_SECTION_OVERLAYS/); + assert.doesNotMatch(source, /section_overlay_text_by_key/); }); test('migration script deterministically maps upstream builder resources to coder/devops overlays', () => { @@ -151,7 +153,36 @@ test('migration script deterministically maps upstream builder resources to code assert.equal(readFileSync(join(packageCopy, 'agents/devops.md'), 'utf8'), originalDevopsOverlay); const generatedText = snapshotGeneratedResources(packageCopy); - assert.doesNotMatch(generatedText, /arc-builder|builder-prompt\.md|agent=\"builder\"|agent: \"builder\"|\bbuilder\b/); + assert.doesNotMatch(generatedText, /arc-builder|builder-prompt\.md|agent=\"builder\"|agent: \"builder\"|`builder`|\bbuilder\b/); + + const generatedFiles = []; + for (const rel of ['agents', 'prompts', 'skills']) { + for (const row of snapshotTree(join(packageCopy, rel)).split('\n')) { + const fileRel = row.split('\0')[0]; + if (fileRel.endsWith('.md') || fileRel.endsWith('.json') || fileRel.endsWith('.ts') || fileRel.endsWith('.js')) { + generatedFiles.push(join(packageCopy, rel, fileRel)); + } + } + } + for (const file of generatedFiles) { + const text = readFileSync(file, 'utf8'); + assert.doesNotMatch(text, /arc-builder|builder-prompt\.md|agent=\"builder\"|agent: \"builder\"|`builder`|\bbuilder\b/); + } + + const reviewSkill = readFileSync(join(packageCopy, 'skills/arc-review/SKILL.md'), 'utf8'); + const reviewPrompt = readFileSync(join(packageCopy, 'skills/arc-review/code-reviewer-prompt.md'), 'utf8'); + const codeReviewer = readFileSync(join(packageCopy, 'agents/code-reviewer.md'), 'utf8'); + const specReviewer = readFileSync(join(packageCopy, 'agents/spec-reviewer.md'), 'utf8'); + assert.match(reviewSkill, /\{EXECUTOR_CONTEXT\}/); + assert.match(reviewSkill, /executor:devops/); + assert.match(reviewPrompt, /\{EXECUTOR_CONTEXT\}/); + assert.match(reviewPrompt, /executor:devops/); + assert.match(reviewPrompt, /Review only; return findings only\. Do not edit files\./); + assert.match(reviewPrompt, /infrastructure\/config\/runbook/); + assert.match(codeReviewer, /infrastructure\/config, and runbook (changes|updates)/); + assert.match(codeReviewer, /executor:devops/); + assert.match(codeReviewer, /Review only; return findings only\. Do not edit files\./); + assert.match(specReviewer, /executor:devops/); const firstSnapshot = generatedText; execFileSync('python3', ['scripts/migrate-arc-plugin.py', UPSTREAM_ARC_SOURCE], { diff --git a/packages/pi-arc/tests/arc-subagents-auto-materialization.test.mjs b/packages/pi-arc/tests/arc-subagents-auto-materialization.test.mjs index dd09031..019bc72 100644 --- a/packages/pi-arc/tests/arc-subagents-auto-materialization.test.mjs +++ b/packages/pi-arc/tests/arc-subagents-auto-materialization.test.mjs @@ -264,6 +264,29 @@ test('Arc devops source agent documents ops gates and safety rules', () => { assert.match(source, /runbook/i); }); +test('Arc reviewer source agents support devops evidence and config review without edit instructions', () => { + const specReviewer = read('agents/spec-reviewer.md'); + assert.match(specReviewer, /target environment/i); + assert.match(specReviewer, /allowed operations/i); + assert.match(specReviewer, /preflight/i); + assert.match(specReviewer, /rollback/i); + assert.match(specReviewer, /validation/i); + assert.match(specReviewer, /evidence/i); + assert.match(specReviewer, /changed artifacts and supplied evidence/i); + assert.match(specReviewer, /read-only|never modify code/i); + assert.doesNotMatch(specReviewer, /reading actual code|Read the implementation code|Compare actual code/i); + + const codeReviewer = read('agents/code-reviewer.md'); + assert.match(codeReviewer, /infrastructure|config|runbook/i); + assert.match(codeReviewer, /executor:devops/i); + assert.match(codeReviewer, /evidence/i); + assert.match(codeReviewer, /implementation\/artifact quality/i); + assert.match(codeReviewer, /validation quality/i); + assert.match(codeReviewer, /operational safety/i); + assert.match(codeReviewer, /read-only|never make code changes/i); + assert.doesNotMatch(codeReviewer, /Check code quality|SOLID principles|test quality/i); +}); + test('Arc subagent markdown render runtime output matches expected structure', async () => { const mod = await import('../extensions/arc/subagents.ts'); const output = mod.buildArcSubagentMarkdown({ diff --git a/packages/pi-arc/tests/arc-subagents-sync.test.mjs b/packages/pi-arc/tests/arc-subagents-sync.test.mjs index bee62af..df6c424 100644 --- a/packages/pi-arc/tests/arc-subagents-sync.test.mjs +++ b/packages/pi-arc/tests/arc-subagents-sync.test.mjs @@ -105,6 +105,11 @@ test('arc-review prefers arc-code-reviewer via pi-subagents before arc_agent fal assert.match(source, /clarify: false/); assert.match(source, /subagent\(\{ action: "status", id: "<run-id>" \}\)/); assert.match(source, /arc_agent\(agent="code-reviewer"/); + assert.match(source, /executor:devops/); + assert.match(source, /devops evidence/i); + assert.match(source, /resolved executor \(`coder`, `devops`, or `doc-writer`\)/); + assert.doesNotMatch(source, /re-dispatch `coder`/i); + assert.doesNotMatch(source, /Re-dispatch `coder`/); }); test('arc-code-reviewer dispatch prompt stays review-only for pi-subagents completion guard', () => { @@ -112,6 +117,9 @@ test('arc-code-reviewer dispatch prompt stays review-only for pi-subagents compl assert.match(source, /Review only/i); assert.match(source, /return findings only/i); assert.match(source, /Do not edit files/i); + assert.match(source, /executor:devops/); + assert.match(source, /evidence/i); + assert.match(source, /config|runbook/i); assert.doesNotMatch(source, /\bmust\s+(?:edit|modify|change|fix|patch|apply)\b/i); assert.doesNotMatch(source, /\bapply\s+(?:the\s+)?fix(?:es)?\s+directly\b/i); assert.doesNotMatch(source, /\bmake\s+(?:the\s+)?code\s+changes\b/i); From ce2fe2ebad9338d692218d5b56641ba3fd7b45d4 Mon Sep 17 00:00:00 2001 From: Ben Firestone <ben.firestone@krypticlabs.com> Date: Mon, 18 May 2026 02:02:38 -0700 Subject: [PATCH 16/17] fix(pi-arc): clean stale arc-builder subagent --- packages/pi-arc/extensions/arc/subagents.ts | 62 +++++++++++++- ...rc-subagents-auto-materialization.test.mjs | 82 +++++++++++++++++++ 2 files changed, 143 insertions(+), 1 deletion(-) diff --git a/packages/pi-arc/extensions/arc/subagents.ts b/packages/pi-arc/extensions/arc/subagents.ts index a08437f..28d42cf 100644 --- a/packages/pi-arc/extensions/arc/subagents.ts +++ b/packages/pi-arc/extensions/arc/subagents.ts @@ -1,5 +1,5 @@ import { createHash } from "node:crypto"; -import { mkdir, readFile, stat, writeFile } from "node:fs/promises"; +import { mkdir, readFile, rm, stat, writeFile } from "node:fs/promises"; import path from "node:path"; import type { ArcModelProfileKey } from "./model-profiles.ts"; @@ -66,6 +66,8 @@ export interface ArcSubagentRenderInput { generatedAt: string; } +const ARC_STALE_GENERATED_SUBAGENTS = ["arc-builder"] as const; + export const ARC_PI_SUBAGENTS = [ { source: "coder", target: "arc-coder", profileKey: "coder" }, { source: "devops", target: "arc-devops", profileKey: "devops" }, @@ -220,6 +222,64 @@ async function materializeArcSubagentsOnce(input: Parameters<typeof materializeA const writes: ArcSubagentWriteResult[] = []; const shadows: ArcSubagentShadowWarning[] = []; + const staleDirs = [targetDir]; + if (input.scope === "user") { + const legacyTargetDir = resolveArcSubagentDir("user", input.cwd, input.homeDir, { legacyUserDir: true }); + if (path.resolve(legacyTargetDir) !== path.resolve(targetDir)) staleDirs.push(legacyTargetDir); + } + + for (const staleAgent of ARC_STALE_GENERATED_SUBAGENTS) { + for (const staleDir of staleDirs) { + const stalePath = path.join(staleDir, `${staleAgent}.md`); + let staleContent: string | undefined; + + try { + staleContent = await readFile(stalePath, "utf8"); + } catch (error) { + if (errorCode(error) !== "ENOENT") { + writes.push({ + agent: staleAgent, + source: stalePath, + target: stalePath, + status: "failed", + reason: `could not inspect stale subagent: ${error instanceof Error ? error.message : String(error)}`, + }); + } + continue; + } + + if (!isGeneratedArcSubagent(staleContent)) { + writes.push({ + agent: staleAgent, + source: stalePath, + target: stalePath, + status: "skipped", + reason: "preserving custom stale subagent; missing generated marker", + }); + continue; + } + + try { + await rm(stalePath, { force: true }); + writes.push({ + agent: staleAgent, + source: stalePath, + target: stalePath, + status: "written", + reason: "removed stale generated subagent", + }); + } catch (error) { + writes.push({ + agent: staleAgent, + source: stalePath, + target: stalePath, + status: "failed", + reason: `could not remove stale generated subagent: ${error instanceof Error ? error.message : String(error)}`, + }); + } + } + } + for (const { source, target } of ARC_PI_SUBAGENTS) { const sourcePath = path.join(input.agentsDir, `${source}.md`); const targetPath = path.join(targetDir, `${target}.md`); diff --git a/packages/pi-arc/tests/arc-subagents-auto-materialization.test.mjs b/packages/pi-arc/tests/arc-subagents-auto-materialization.test.mjs index 019bc72..8b5afad 100644 --- a/packages/pi-arc/tests/arc-subagents-auto-materialization.test.mjs +++ b/packages/pi-arc/tests/arc-subagents-auto-materialization.test.mjs @@ -146,6 +146,88 @@ test('Arc materializer preserves non-generated files and reports project shadows } }); +test('Arc materializer deletes stale generated arc-builder subagent during materialization', async () => { + const mod = await import('../extensions/arc/subagents.ts'); + const markers = [ + mod.ARC_SUBAGENT_GENERATED_MARKER, + '<!-- generated by @sentiolabs/pi-arc arc-subagents-sync -->', + ]; + + for (const marker of markers) { + const root = await mkdtemp(path.join(tmpdir(), 'arc-subagents-')); + try { + const cwd = path.join(root, 'project'); + const homeDir = path.join(root, 'home'); + const agentsDir = path.join(root, 'agents'); + await mkdir(cwd, { recursive: true }); + await mkdir(homeDir, { recursive: true }); + await mkdir(agentsDir, { recursive: true }); + + const targetDir = mod.resolveArcSubagentDir('user', cwd, homeDir); + await mkdir(targetDir, { recursive: true }); + await writeFile(path.join(targetDir, 'arc-builder.md'), `${marker}\nlegacy`, 'utf8'); + + const result = await mod.materializeArcSubagents({ + reason: 'manual_repair', + scope: 'user', + cwd, + homeDir, + agentsDir, + modelsConfigSha256: 'models-hash', + renderAgent: async (source, target) => renderTestAgent(mod, source, target), + }); + + await assert.rejects(readFile(path.join(targetDir, 'arc-builder.md'), 'utf8')); + const cleanup = result.writes.find((entry) => entry.agent === 'arc-builder'); + assert.equal(cleanup?.status, 'written'); + assert.match(cleanup?.reason ?? '', /removed stale generated subagent/i); + } finally { + await rm(root, { recursive: true, force: true }); + } + } +}); + +test('Arc materializer cleans stale arc-builder in modern and legacy user dirs without deleting custom files', async () => { + const mod = await import('../extensions/arc/subagents.ts'); + const root = await mkdtemp(path.join(tmpdir(), 'arc-subagents-')); + try { + const cwd = path.join(root, 'project'); + const homeDir = path.join(root, 'home'); + const agentsDir = path.join(root, 'agents'); + await mkdir(cwd, { recursive: true }); + await mkdir(homeDir, { recursive: true }); + await mkdir(agentsDir, { recursive: true }); + + const targetDir = mod.resolveArcSubagentDir('user', cwd, homeDir); + const legacyDir = mod.resolveArcSubagentDir('user', cwd, homeDir, { legacyUserDir: true }); + await mkdir(targetDir, { recursive: true }); + await mkdir(legacyDir, { recursive: true }); + await writeFile(path.join(targetDir, 'arc-builder.md'), '# my custom specialist', 'utf8'); + await writeFile(path.join(legacyDir, 'arc-builder.md'), `${mod.ARC_SUBAGENT_GENERATED_MARKER}\nlegacy`, 'utf8'); + + const result = await mod.materializeArcSubagents({ + reason: 'manual_repair', + scope: 'user', + cwd, + homeDir, + agentsDir, + modelsConfigSha256: 'models-hash', + renderAgent: async (source, target) => renderTestAgent(mod, source, target), + }); + + assert.equal(await readFile(path.join(targetDir, 'arc-builder.md'), 'utf8'), '# my custom specialist'); + await assert.rejects(readFile(path.join(legacyDir, 'arc-builder.md'), 'utf8')); + + const cleanups = result.writes.filter((entry) => entry.agent === 'arc-builder'); + assert.equal(cleanups.find((entry) => entry.target === path.join(targetDir, 'arc-builder.md'))?.status, 'skipped'); + assert.equal(cleanups.find((entry) => entry.target === path.join(legacyDir, 'arc-builder.md'))?.status, 'written'); + assert.match(cleanups.find((entry) => entry.target === path.join(targetDir, 'arc-builder.md'))?.reason ?? '', /preserving custom stale subagent/i); + assert.match(cleanups.find((entry) => entry.target === path.join(legacyDir, 'arc-builder.md'))?.reason ?? '', /removed stale generated subagent/i); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + test('Arc materializer falls back to legacy user directory when modern user directory is unavailable', async () => { const mod = await import('../extensions/arc/subagents.ts'); const root = await mkdtemp(path.join(tmpdir(), 'arc-subagents-')); From 2d7958543534140c59c9a413ceb5267d3dc5d837 Mon Sep 17 00:00:00 2001 From: Ben Firestone <ben.firestone@krypticlabs.com> Date: Mon, 18 May 2026 02:11:55 -0700 Subject: [PATCH 17/17] docs(pi-arc): document executor split --- packages/pi-arc/README.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/pi-arc/README.md b/packages/pi-arc/README.md index ff2bdd9..e0b2983 100644 --- a/packages/pi-arc/README.md +++ b/packages/pi-arc/README.md @@ -52,6 +52,7 @@ This package is a Pi-native port of the Claude Code Arc plugin at https://github - **`arc_agent` tool**: - Runs bundled Arc specialist prompts from `agents/*.md` in fresh Pi subprocesses. - Supports `coder`, `code-reviewer`, `doc-writer`, `evaluator`, `issue-manager`, and `spec-reviewer`. + - Also supports `devops` for evidence-driven infrastructure/config/runbook work via `arc_agent(agent="devops")`. - Resolves Arc model tiers (`small`, `standard`, `large`) to concrete Pi models so orchestrators can right-size subagent dispatches. - Current limitation: `isolation: "worktree"` is recognized but not implemented yet. - **Optional `pi-subagents` companion support**: @@ -172,7 +173,9 @@ The brainstorm skill writes a first-line marker like `<!-- arc-review: kind=shar Use `/arc-models` to configure Arc's recommended Pi model and thinking level per workflow role. Arc stores profile preferences at `${XDG_CONFIG_HOME:-~/.config}/pi-arc/models.json`, with top-level `modelProfiles`. -Profile keys map directly to the workflow roles: `brainstorm`, `plan`, `issueManager`, `coder`, `codeReviewer`, `docWriter`, `specReviewer`, and `evaluator`. +Profile keys map directly to the workflow roles: `brainstorm`, `plan`, `issueManager`, `coder`, `devops`, `codeReviewer`, `docWriter`, `specReviewer`, and `evaluator`. + +The former code executor names <code>builder</code> / <code>arc-builder</code> were renamed to `coder` / `arc-coder`. Legacy <code>builder</code> model profile entries migrate to `coder` automatically when `coder` is unset, so existing configs keep working while new configs should use `coder`. ```json { @@ -214,6 +217,7 @@ Arc writes generated specialists to `~/.agents/` by default. Legacy user scope ` Generated specialists include: - `arc-coder` +- `arc-devops` - `arc-doc-writer` - `arc-spec-reviewer` - `arc-code-reviewer` @@ -253,8 +257,18 @@ For Arc gates (especially spec compliance), use Arc specialists (`arc-spec-revie - Sequential Arc build: use when tasks overlap, dependencies are linear, or `pi-subagents` is unavailable. - Parallel Arc batch: use when `/arc-plan` provides a T0 foundation, file ownership matrix, parallel batch manifest, and validation matrix. +- Devops lane: use `arc-devops` / `arc_agent(agent="devops")` for evidence-only operational tasks. If no repository files change, the task may complete without a commit. - Ant Colony: future/optional lane for large exploratory work; not a replacement for Arc gates in this iteration. +### Executor labels and legacy routing + +- `executor:coder` routes implementation work to `arc-coder`. +- `executor:devops` routes operational work to `arc-devops`. +- `executor:docs` routes documentation work to `arc-doc-writer` in pi-subagents or `doc-writer` through `arc_agent`. +- `live-ops-approved` marks devops tasks that have explicit live-operation authorization. +- Old unlabeled issues route to `coder` as legacy input. +- Old `docs-only` issues route to docs as legacy input. + ## Naming differences from the Claude plugin Claude plugin commands used names like `/arc:create`. Pi prompt templates are filename-based, so this package uses hyphenated names: