From 4ef2826bd8239f7ebc506138ceff777b203b788e Mon Sep 17 00:00:00 2001 From: Simrandeep Singh Date: Thu, 9 Jul 2026 12:35:27 -0700 Subject: [PATCH] Split /spec-submit into /verify-submit and /create-runbook Replace the single /spec-submit command with two focused flows and factor their shared logic into skills: - /verify-submit: you write the code; capture intent, key decisions, and acceptance criteria, no implementation steps. - /create-runbook: Aviator's agent writes the code from a spec with full implementation detail. - spec-submission skill: shared submission mechanics (message, AC review loop, specSubmit call, PR directive, error handling) both commands load. - acceptance-criteria skill: the shared AC quality rulebook. - /spec-submit becomes a deprecation stub pointing at the two commands. Both commands pass submission_type to specSubmit (verify vs runbook). --- .claude-plugin/marketplace.json | 4 +- README.md | 14 +- aviator/.claude-plugin/plugin.json | 4 +- aviator/commands/create-runbook.md | 60 +++++ aviator/commands/spec-submit.md | 241 +------------------- aviator/commands/verify-submit.md | 89 ++++++++ aviator/skills/acceptance-criteria/SKILL.md | 108 +++++++++ aviator/skills/spec-submission/SKILL.md | 74 ++++++ 8 files changed, 350 insertions(+), 244 deletions(-) create mode 100644 aviator/commands/create-runbook.md create mode 100644 aviator/commands/verify-submit.md create mode 100644 aviator/skills/acceptance-criteria/SKILL.md create mode 100644 aviator/skills/spec-submission/SKILL.md diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index e5b285a..472f0ef 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -17,8 +17,8 @@ { "name": "aviator", "source": "./aviator", - "description": "Runbooks and workflow automation with Aviator MCP", - "version": "1.1.8" + "description": "Verify and Runbooks workflow automation with Aviator MCP", + "version": "1.2.0" } ] } diff --git a/README.md b/README.md index b6c4750..2005124 100644 --- a/README.md +++ b/README.md @@ -72,20 +72,24 @@ run `av pr --all` to create PRs for the entire stack. ### aviator -Connects Claude Code to [Aviator Runbooks](https://aviator.co/runbooks) via MCP for workflow automation. +Connects Claude Code to Aviator via MCP for spec submission to [Runbooks](https://aviator.co/runbooks) and Verify, to automate development workflows. -**What are Runbooks?** Aviator Runbooks let you capture and replay complex development workflows. Create a runbook from your Claude session to save your exploration, decisions, and implementation approach for future use or to share with your team. +**Verify vs Runbooks.** Aviator has two ways to hand a spec off from your Claude session: + +- **Verify** — *you* write the code and Aviator verifies it against your intent. You submit an intent, a free-form spec of the key decisions, and acceptance criteria; Aviator checks the PR you open against those criteria. +- **Runbooks** — *Aviator's agent* writes the code from your spec. The spec carries full implementation detail so the runbook can capture and replay the workflow. **What this plugin does:** -- Connects to the Aviator MCP server for runbook operations -- Creates runbooks from your current Claude session context +- Connects to the Aviator MCP server for spec submission and runbook operations +- Submits Verify specs and creates runbooks from your current Claude session context - Handles OAuth authentication automatically - Provides access to Aviator's workflow automation tools **Usage:** -Use `/spec-submit` to submit a spec and create a runbook from your current session. +- `/verify-submit` — submit a Verify spec (intent + acceptance criteria) for code you're writing yourself. +- `/create-runbook` — have Aviator's agent write the code from a spec with provided implementation detail. **Requirements:** diff --git a/aviator/.claude-plugin/plugin.json b/aviator/.claude-plugin/plugin.json index 634d12a..6a9ea60 100644 --- a/aviator/.claude-plugin/plugin.json +++ b/aviator/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "aviator", - "description": "Create Aviator Runbooks from Claude Code sessions to capture and relay development workflows", + "description": "Submit specs to Aviator from Claude Code sessions — Verify (intent + acceptance criteria over your own code) and Runbooks (agentic code generation)", "author": { "name": "Aviator", "email": "support@aviator.co" @@ -8,5 +8,5 @@ "homepage": "https://aviator.co", "repository": "https://github.com/aviator-co/agent-plugins", "license": "MIT", - "keywords": ["runbooks", "aviator", "spec-driven", "spec-driven-development"] + "keywords": ["verify", "runbooks", "aviator", "spec-driven", "behavior-driven-development"] } diff --git a/aviator/commands/create-runbook.md b/aviator/commands/create-runbook.md new file mode 100644 index 0000000..7a858f5 --- /dev/null +++ b/aviator/commands/create-runbook.md @@ -0,0 +1,60 @@ +--- +description: Create an Aviator Runbook — Aviator's agent writes the code from your spec +--- + +# Create a Runbook + +Create an Aviator Runbook from the current Claude Code session context. **Aviator's agent writes the code** from the spec you submit, so this flow carries full implementation detail — intent, scope, ordered steps, and acceptance criteria — and includes an implementation discussion with you before kicking off. + +> Writing the code yourself and just want Aviator to verify it against intent + acceptance criteria? Use `/verify-submit` instead — it captures intent and AC with no implementation steps. + +**Load the `spec-submission` skill** (Skill tool → `aviator:spec-submission`) before you start — it carries the shared mechanics this flow relies on: how the message reads, the Acceptance Criteria review loop, the `specSubmit` call, and the PR directive. This command file only covers what's specific to a Runbook. + +## Arguments + +$ARGUMENTS - Optional additional context or instructions for the runbook. + +## Step 1: Generate Message + Spec + +Write the message per the `spec-submission` skill, and the Acceptance Criteria per the `acceptance-criteria` skill — **AC are the primary output of this step.** What's specific to a Runbook is that the spec **carries implementation detail** — the agent uses it to write the code. + +### Spec file + +The spec provides the supporting context the AC needs to be unambiguous, plus the implementation detail the agent works from. Don't pad. + +If a plan file exists from plan mode (check the plan file path mentioned in the system prompt), read it and check whether its content is relevant to the user's current intent. If it is, use it as-is — do not restructure, reformat, or rewrite it. Pass its content through directly as the spec. If the plan file is unrelated to the current task, ignore it and generate a new spec instead. + +Similarly, if a spec file already exists in the conversation — one the user wrote, one generated earlier, or one provided via `$ARGUMENTS` — use it as-is. Do not restructure, reformat, or rewrite an existing spec. When the spec comes from a file, preserve the original filename. + +If no existing spec is available, generate one. Keep it **free-form** — there's no required structure or fixed set of sections. Write whatever best conveys the change to the agent that will implement it: the intent and the implementation approach or steps, shaped to the task rather than forced into headings. (The acceptance criteria are passed separately as the `acceptance_criteria` argument — you don't hand-embed them; the backend folds them into the spec the agent works from.) + +## Step 2: Review with the user + +Before submitting, show the user three things and get their sign-off: + +1. **The intent** — the short intent message, for grounding. +2. **The Acceptance Criteria** — run the review loop from the `spec-submission` skill, iterating until the user explicitly confirms. +3. **Any pertinent questions or callouts** — while writing the free-form spec, notice anything the user should weigh in on before submitting: a consequential choice (a new dependency, a data migration, a public API change, an area to leave untouched), an ambiguity in the approach, or a decision you made that they haven't seen. Raise only what's genuinely open — if the approach was already settled earlier in this session, don't re-litigate it. + +A simple "yes" or "go ahead" is enough to submit. + +## Step 3: Create Runbook + +Submit via the `specSubmit` call described in the `spec-submission` skill. For a Runbook: + +- `submission_type`: **`"runbook"`** — the agent writes the code from the spec. This is the default, but pass it explicitly. +- `acceptance_criteria`: the confirmed AC as a JSON array of strings (optional but recommended — they're folded into the spec the agent works from). +- `spec_files`: include only if a spec was generated; always a single file. +- `target_branch`: **optional** — the base branch the runbook builds on and checks out; the generated PR opens against it. Omit for the repo default (trunk). (Runbook mode generates its own PR, so there's no `working_branch` to connect here.) + +Then return the Runbook URL and set the PR directive, both per the `spec-submission` skill. The expected PR body shape: + +``` +Runbook: + +## Summary +… + +## Test plan +… +``` diff --git a/aviator/commands/spec-submit.md b/aviator/commands/spec-submit.md index d66ce3b..69a1e30 100644 --- a/aviator/commands/spec-submit.md +++ b/aviator/commands/spec-submit.md @@ -1,241 +1,12 @@ --- -description: Submit a spec to Aviator to create a Runbook +description: (Deprecated) Renamed — use /verify-submit for Verify, or /create-runbook to have Aviator's agent write the code --- -# Submit Spec to Aviator +# Spec Submit has been split -Submit a spec to Aviator to create a Runbook from the current Claude Code session context. +`/spec-submit` has been replaced by two focused commands. Pick the one that matches what you want, tell the user which you're running, and follow that command's instructions: -## Arguments +- **`/verify-submit`** — **you** are writing the code and want Aviator to verify it against your intent. Captures **Intent + a free-form record of key decisions/architecture + Acceptance Criteria**, with no implementation steps. This is the primary flow and the closest match to the old `/spec-submit` behavior. +- **`/create-runbook`** — you want **Aviator's agent to write the code** from a spec. Carries full implementation detail (scope, ordered steps) and includes an implementation discussion before kicking off. -$ARGUMENTS - Optional additional context or instructions for the runbook - -## Steps - -### Step 1: Generate Message + Spec - -Generate the artifacts from the session context. **Acceptance Criteria is the primary output of this step** — prioritize its quality over the length or polish of any other section. A spec with sharp AC and a thin Intent is better than a spec with a lush Intent and generic AC. - -#### Message - -A short, human-friendly description of what this runbook should do — written the way a person would describe the task to a colleague. Think of it as the task detail someone would type when filing a ticket. A few sentences at most. No markdown structure, no file paths, no code details. - -If the user provided $ARGUMENTS, lean on their words — they're telling you what they want, so echo their intent rather than rephrasing it technically. - -Good message example: -> Fix the 3 intentional bugs in calculator.py and add new math helper and string utility modules with proper tests so CI exercises real code instead of the automated-failure workflow. - -Bad message example (too technical, belongs in spec): -> Fix `calculator.py:19` multiply bug (`return a + b` → `return a * b`), remove unused `import os` on line 3, fix `power()` return type on line 32. Add tests covering edge cases... - -#### Acceptance Criteria (the focus) - -Acceptance Criteria (AC) are the concrete checks that prove a change works correctly and fits the codebase. Think of them as the test plan a reviewer would actually run — each item is a specific behavior, input/output pair, command, invariant, or observable experience. Some AC are programmatically testable (an endpoint returns 401, a command exits 0); others are behavioral, qualitative, or UX expectations — both are valid as long as two reviewers would agree on whether the AC is met. AC are the contract between intent and implementation, and the highest-value artifact in this spec. - -##### The goal of AC — observable outcomes, not implementation - -Acceptance criteria define what must be true for the work to be acceptable. Each criterion is a gate. The work is not done until every criterion passes; if any one fails, the work is not yet acceptable. They are the contract for "done." - -Acceptance criteria are NOT an implementation checklist. Internal details (which file was touched, which function was added, which private structure was used) belong in the implementation steps, not in the criteria. - -Favor fewer, sharper criteria over many shallow ones. A handful of strong outcome criteria is better than a long checklist of weak ones. - -##### Two readers, both must be served - -Every AC has two audiences and both must accept it: - -- **AI verifier** who later judges pass/fail. The criterion must reduce to a deterministic check the verifier can run against the system, code, or output — DOM/CSS inspection, API calls, file checks, test runs, state queries. If the only way to evaluate it is human judgment, it is not a gate. -- **The human** The criterion must read at a glance. They shouldn't have to mentally filter past implementation noise — file paths, hex codes, pixel values, internal structures, internal type names, function or handler names, internal route paths, queue/task names, middleware steps, infrastructure component names (Redis, Celery, Postgres, Kafka, etc.) — to extract the actual gate. Name the user-visible product, customer-facing surface, or externally observable outcome that's affected; mention internal infrastructure only when it IS the failure mode being tested (e.g. "if the cache is unavailable, requests still succeed"). - -If a candidate AC fails either reader, it doesn't belong on the list. - -##### The north-star test — governs every other rule below - -Before writing or keeping any AC, both must hold: - -1. **This change can break it.** Trace the AC to the diff. If nothing in this change could flip it from pass to fail — it names a code path the diff doesn't touch, it guards behavior outside this change's scope — it is out of scope. Drop it, however important the behavior is in the abstract. -2. **A violation has a named victim.** Ask "if this were violated, what gets worse, and for whom?" A specific impact on a specific party — a user hits a bug, the next maintainer trips on an inconsistent pattern, the prod operator can't debug a silent failure — means keep it. A vague answer ("things would just be less good", "code wouldn't be as clean") means filler, delete it. - -Test 2 alone is a trap: "if the auth cron broke, logins would fail" is severe and real, yet if this change never touches the auth cron, test 1 already ruled it out. Severity is not a license to gate what this change can't affect. - -**Both functional issues *and* codebase-health issues** (broken conventions, duplicated logic, missing observability) count — anyone downstream of this change (user, reviewer, maintainer, operator) is a valid party. The build pipeline is not on this list: a green type-check/lint/format/CI run is never the impact an AC defends (see the anti-pattern below). - -##### AC covers behavior first, fit second - -- **Functional correctness (the core of every list):** the change does the right thing — golden path, edge cases, failure modes, invariants. *"`divide(1, 0)` returns `Err(DivByZero)`"*. -- **Codebase fit (only when it's a genuine gate):** the change sits well with existing code — reuses existing helpers instead of duplicating logic, matches established patterns, doesn't quietly change a public API, doesn't pull in a new dependency. *"reuses `internal/retry.Backoff` instead of a new loop"*, *"no new entries in `package.json` `dependencies`"*. Skip this axis when the change has no such gate — don't manufacture one to fill a quota. - - -##### Sources to draw AC from — prioritize code over plan - -Do not generate from imagination. Before writing any AC, go read what's actually there — and treat the sources in this order of priority: - -- **Code changes made in this session (primary source).** Implementation could drifts from the plan as the session goes on, so the final code — not the original plan — is the ground truth for what this change actually *does*. Read the modified files end-to-end, not just the diff hunks, and understand what the code is trying to do: what behavior each new/changed function introduces, what invariants it preserves, what public surface it exposes, what failure modes it handles, what it replaces or removes. Every behavior present in the code must map to an AC, and the current code must pass every AC you write. -- **Existing spec, plan, or `$ARGUMENTS` (secondary source — cross-check, don't copy blindly).** If the user wrote a spec, ran plan mode, or supplied content via `$ARGUMENTS`, mine it for must-haves, constraints, and explicit success criteria the user already endorsed — preserve those, don't drop them. Use the plan to catch behaviors the code *should* have but doesn't (a gap, not a pass). **When the plan and the code disagree, trust the code** and surface the divergence to the user so they can confirm it was intentional — don't silently write AC for a behavior the code no longer implements. - -If the code would fail one of your AC, that's a signal: either the AC is wrong, or the change is incomplete. Flag the gap to the user rather than papering over it. - -##### Rules for valuable AC - - -- Describe an observable outcome or user-visible behavior that determines acceptance. -- Declarative, outcome-stating phrasing ("Users can log in with email and password", "API returns 401 for missing auth token"). Describe the resulting state of the system, not an action to take — actions belong in the runbook steps. -- Specific enough to judge pass or fail by inspection. -- Should be human readable — a natural-language sentence, not a code snippet or annotation. - -### Coverage - -Cover the meaningful behavior changes the runbook delivers. Each criterion is a gate that genuinely affects whether the work is done. Few and sharp beats many and shallow. A long list of overlapping restatements is worse than a short well-chosen set. - -When the runbook's deliverable is preserved behavior — refactors, restyles, migrations, dependency upgrades, performance work — "the existing X still works" is a meaningful gate, not a cop-out. Examples: "All existing article actions remain functional and resolve to their previous routes." "Existing tests continue to pass after the change." Do not drop these from preserve-behavior runbooks just because they sound generic. But this only applies to behavior the change actually touches or could regress. Code the diff never goes near is unrelated code, not preserved behavior; "X still works" is not a gate when the change couldn't have broken X. - -- **Verifiable, not necessarily runnable.** Runnable checks (a command exits 0, an endpoint returns 401) are the gold standard, but behavioral, qualitative, and UX criteria are equally valid as long as the criterion is sharp enough that different reviewers would reach the same verdict. The disqualifying test is ambiguity, not un-runnable-ness — rewrite or delete any AC whose pass/fail depends on interpretation. - -- **Outcome, not work done.** Do not restate the task as an AC. "The column is declared", "the model change is committed in code", "a new test module exists", "the migration is generated" narrate the work, not a testable outcome. Translate into observable effects: querying the schema returns the new field; `just dbmigrate` produces a clean migration file; `just pytest ` runs the new cases. If the only thing "verifying" an AC is that the developer did the work, delete it. - -- **Cover what matters.** Golden path, important edge cases, failure modes, and invariants that must still hold after the change. Skip what doesn't meaningfully change. - -- **No redundancy.** Before finalizing, read the list end to end. If two items would be satisfied by the same test, merge or delete. If one item is already implied by another, drop it. Each criterion must probe a distinct behavior. - - -##### Anti-patterns — do not produce these - -**Code blocks or snippets inside an AC — strict no.** Every AC is a one-line natural-language gate. Do not embed fenced code blocks, JSON/YAML payloads, SQL statements, request/response bodies, function signatures, or stack traces inside an AC bullet. If the behavior seems to need code to be clear, the criterion is doing too much — split it, move the example into the spec's Steps section, or rewrite the AC at a higher level. - - Bad: an AC bullet followed by a fenced ```json``` block showing the expected response. - - Good: "The articles list response includes a published timestamp in ISO-8601 format for every article." - -**Subjective taste words.** Words like "readable," "comfortable," "airy," "clean," "modern," "intuitive," "polished," "elegant" name a feeling, not a gate — two reviewers can disagree and both be right, so the verifier has no deterministic check. If the spec used these words, translate the underlying intent into a checkable structural property — an observable layout assertion, not a measurement — or drop the criterion. -- Bad: "The article body has comfortable line height and airy paragraph spacing." -- Good (if the intent is layout constraint): "The article body is constrained to a centered column rather than spanning the full viewport width." -- Or: drop the criterion entirely if other gates already capture the intent. Taste is not a gate. - -**Internal identifiers — exact paths, function/class/type names, line numbers, infra component names — are noise, and never the subject of an AC.** Absolute paths (`src//.py`), dotted paths (`package.module.helper`), GraphQL resolver/schema-type names, function/handler/class/component/hook/prop names, internal route paths, queue/task names, table/column names, and infrastructure names (Redis, Postgres, Celery, Kafka) describe implementation, not outcome. When one becomes the *subject* of a criterion, the AC reads like a code annotation rather than a behavioral gate. Reframe so the subject is the user, the customer-visible surface, or an externally observable outcome — what the caller *sees*, not which internal step produced it. - - Bad: "Function `validateJwt` exists in `src/auth/jwt.py`." - - Good: "JWT tokens are validated before any request reaches a protected route." - - Bad (mechanism-led): "Inbound SMS for unpaid customers is dropped at `/api/sms/inbound` before `process_sms` is enqueued." - - Good (outcome-led): "Inbound SMS for unpaid customers is accepted with HTTP 200 but produces no auto-replies, inbox entries, or downstream automation." - - If a value or identifier IS the externally observable contract — an HTTP status code, a public API field name, a customer-facing CLI flag, a documented config key — keep it. The rule targets internal mechanism leaking into AC, not all technical specifics. - -**Internal data shapes or private structures.** A reader cannot see private state without the code; the verifier checking it conflates implementation with outcome. -- Bad: "The `User` class has a `roles` list attribute." -- Good: "Authenticated users receive the roles assigned to their account in API responses." - -**Implementation-detail numerics that are noise to the human reader.** Pixel breakpoints, exact rem values, hex/rgba colors, exact font weights. The verifier could check them, but a human scanning the list has to mentally filter them to extract the gate. Two layers to this rule: - -- **Don't invent specifics the spec did not supply.** Framework defaults, library conventions, or your own reasoning are not the spec. The implementation STEPS own implementation detail; the AC owns the gate. - - Spec said "service retries on transient failures." Bad AC: "Service retries up to 3 times with 100ms backoff." Good AC: "Service retries on transient failures." -- **Prefer the abstract level even when the spec supplied a value.** Lift the gate to what a reader can absorb at a glance — but stop before "abstract" becomes "subjective taste." If the abstract version reduces to a taste word, you have gone too far; restructure or drop. - - When the value IS the contract, keep it verbatim. Spec said "API returns 429 when rate-limited." Keep "API returns 429 when rate-limited." - - When the value is incidental, name its role. Spec gave a specific color hex for badges. Better: "Badges use the brand accent color." - -**Build / lint / type-check / format / CI gates — never an AC.** "Typecheck passes", "lint is clean", "`prettier` reports no changes", "the code compiles", "CI is green", "all tests pass" are never acceptance criteria — they are the pipeline's job and tell a reviewer nothing about whether the feature works. Do not add them, not even as a secondary item and never as a catch-all at the end of the list. The one adjacent case that IS valid is a deliberate regression guard on behavior *this change could plausibly break* ("existing X still works after the refactor"), and even then phrase it as the behavior, not as "the test suite passes". - - Don't cite the verification mechanism as the acceptance criterion. Test commands, runner invocations, CI job names, and test file paths describe *how* the behavior is checked, not *what* the behavior is — name the outcome the check defends instead. Bad: "All test cases pass." Good: "Requests to protected routes without a valid token return 401." - -**Implementation choices and tradeoffs we made — not AC.** Decisions reached while building (an icon instead of a text label, a fallback format kept for older targets, building markup via the DOM rather than string concatenation) are *how* the feature was built, not gates on *what* it does. An AC states the user-visible outcome, not the option picked to reach it. If a candidate criterion would only make sense to someone who watched the session — "uses an icon, not text", "keeps a markdown fallback" — either reframe it as the behavior it produces ("the stack pastes as clickable links in chat") or drop it. The current observable behavior is the contract, not the menu of choices behind it. - -### When the value IS the deliverable - -When the runbook's purpose is to change a specific value — a version bump, a color token swap, a config introduction — that value belongs in the AC because it IS the gate. - -- React 17 → 18 upgrade: "Application runs on React 18 with no deprecation warnings in the browser console" is a strong gate. A separate "package.json declares react at ^18" is acceptable because the version IS the deliverable. -- Color token introduction: "$color-accent is defined and used wherever the legacy yellow appeared" is acceptable because the swap IS the deliverable. - -This is the only case where implementation-level specifics earn a place. Do not extend the exception to incidental values. - -#### Spec file - -The spec provides the supporting context the AC needs to be unambiguous — no more. Don't pad. - -If a plan file exists from plan mode (check the plan file path mentioned in the system prompt), read it and check whether its content is relevant to the user's current intent. If it is, use it as-is — do not restructure, reformat, or rewrite it. Pass its content through directly as the spec. If the plan file is unrelated to the current task, ignore it and generate a new spec instead. - -Similarly, if a spec file already exists in the conversation — either one the user wrote, one generated earlier in the session, or one provided via $ARGUMENTS — use it as-is. Do not restructure, reformat, or rewrite an existing spec. Pass it through directly. When the spec comes from a file, preserve the original filename — do not rename it. - -If no existing spec is available, generate one. Use these sections: - -``` -## Intent -What this change accomplishes and why. Keep it brief — enough context to make the AC make sense. - -## Scope -* **Modify:** files to change -* **Create:** new files to add -* **Forbid:** files/areas that should NOT be touched (if relevant) - -## Steps -Ordered implementation steps or phases. - -## Acceptance Criteria -- [ ] Concrete, testable, observable criteria (see rules above) -- [ ] Each one probes a distinct behavior -``` - -Adapt sections to fit the task — not every section is needed. Intent and Acceptance Criteria are the ones that almost always belong. Scope and Steps are optional supporting detail. - -### Step 2: Review Acceptance Criteria with User — Iterate Until Aligned - -Before submitting, show the user **only the Acceptance Criteria** for review. Do not dump the full spec body (Intent / Scope / Steps) into the chat — the spec is generated and will be submitted, but it's supporting context, not what the user is being asked to confirm. You may include the one-line message above the AC for grounding, but nothing more. If the user wants to see the spec body, they'll ask — show it then. Otherwise, keep the review focused on AC alone. - -**On the first showing of AC in this flow, preface it with a one-line primer** so users unfamiliar with the term know what they're reviewing — something like: *"Acceptance Criteria are the code-anchored behaviors this change must satisfy — each one will be verified independently against the codebase after the work is done. Please review whether these are the right ones."* Adjust the wording to feel natural, but always include a primer the first time. Skip it on subsequent re-shows after edits. - -Ask the user a single, direct question — something like: *"Do these AC cover what you care about — anything to add, remove, or tighten?"* Keep it to one question; don't bombard the user with a checklist of separate prompts. - -Apply the user's feedback: add missing criteria, remove redundant ones, tighten vague ones, split bundled ones. Re-show the updated AC list (call out what changed since the previous round so the user isn't re-reading from scratch) and ask again. Repeat this loop until the user **explicitly** confirms the AC is aligned with what they want. - -**Get a clear sign-off from the user before moving to Step 3.** A simple "yes" or "go ahead" is enough. - -### Step 3: Create Runbook - -**Only run this step after the user has explicitly confirmed alignment in Step 2.** - -**Lock the confirmed AC into the spec before submitting.** The AC list the user signed off on in Step 2 is the final AC — the downstream runbook must not regenerate, rephrase, extend, or prune them. Before calling `specSubmit`: - -- Replace the spec's `## Acceptance Criteria` section with the exact bullets the user confirmed in Step 2 — nothing added, nothing dropped, in the same order. - -Then use the `specSubmit` MCP tool from the Aviator server with: - -- `repo_name`: The repository in `owner/repo` format -- `message`: The confirmed message -- `spec_files`: `[{"filename": "", "content": "..."}]` (only if a spec was generated; always a single file — use the original filename if the spec came from a file) -- `working_branch` (optional): the existing branch where this work lives, passed by name. It auto-connects the PR you open later from that branch to the runbook this submission creates. You do NOT need to push the branch before submitting — submit the spec now and open the PR afterward; the connection is made by branch name, not by what's on the remote. (If the branch happens to be pushed already, the runbook will also read its latest code, but unpushed is completely fine.) Omit when the runbook should author the work from scratch — it will create and use its own branch. -- `target_branch` (optional): the branch this work is built on top of — omit for the repo default (trunk); pass the parent branch when this work is stacked on another in-flight branch - -The tool will return the runbook URL. Treat the returned URL as the canonical **Runbook URL** for this session — hold it for any PR opened later in the same session. - -### Step 4: Return Link and Set PR Directive - -#### 4a. Return to user - -Provide the user with: -- The Runbook URL from the tool response -- A brief summary of what was submitted - -#### 4b. PR description directive (session-scoped) - -When opening a PR for this runbook's work later in the same session, the PR body **MUST** begin with `Runbook: ` on its own line, followed by a blank line, then the rest of the description. This applies to `gh pr create`, `av pr`, or any equivalent PR-opening command. - -Sub-rules: - -- **Prepend, don't replace.** The `Runbook:` line goes *above* any template, summary, or drafted body — not in place of it. Existing PR-description content (Summary, Test plan, etc.) is preserved unchanged below it. -- **Exact format.** `Runbook: ` on its own line. Plain text — no markdown link, no emoji, no prefix. Keep it greppable so tooling can parse it. -- **Body only, not title.** Never put the URL in the PR title. Never add it to commit messages or branch names. -- **Scope.** Applies only to PRs that implement *this* runbook's work in *this* session. -- **New PRs only.** If a PR for this work already exists when `/spec-submit` runs, leave it alone — do not retroactively edit existing PR descriptions. - -Expected PR body shape: - -``` -Runbook: - -## Summary -… - -## Test plan -… -``` - -## Error Handling - -- If authentication is required, Claude Code will automatically open a browser for OAuth login -- If the repository is not found in Aviator, suggest connecting it in the Aviator dashboard under GitHub settings -- If the API returns an error about credits, inform the user they may need to add runbook credits in their Aviator dashboard +**Default:** unless the session is clearly about handing the work off to Aviator's agent to implement, treat the request as a Verify submission and run **`/verify-submit`**. diff --git a/aviator/commands/verify-submit.md b/aviator/commands/verify-submit.md new file mode 100644 index 0000000..a2d4e5c --- /dev/null +++ b/aviator/commands/verify-submit.md @@ -0,0 +1,89 @@ +--- +description: Submit a Verify spec to Aviator — intent + acceptance criteria over the code you're writing +--- + +# Submit for Verify + +Submit a Verify spec to Aviator from the current Claude Code session. Aviator Verify checks whether the intent has been accomplished, using a mix of code scans and behavior observations; the implementation is left to the author/agent. The first verification run happens when the PR is marked ready for review. This flow captures three things and nothing more: + +- **Intent** — what this change accomplishes and why. +- **Key decisions & architecture** — a free-form record of the decisions made and the shape of the change, written so a reviewer can understand the PR without reading every line. +- **Acceptance Criteria** — the concrete, observable behaviors the change must satisfy, verified independently against the code. + +**Load the `spec-submission` skill** (Skill tool → `aviator:spec-submission`) before you start — it carries the shared mechanics this flow relies on: how the message reads, the Acceptance Criteria review loop, the `specSubmit` call, and the PR directive. This command file only covers what's specific to Verify. + +## Arguments + +$ARGUMENTS - Optional additional context or instructions for the Verify submission. + +## Step 1: Read the current work + +The code is the ground truth for a Verify submission. Before generating anything: + +- Identify the **working branch** — the branch the in-flight work lives on (typically the current git branch). You'll pass this as `working_branch` so Verify tracks the PR opened from it. +- Identify the **repository** in `owner/repo` form (e.g. from `git remote get-url origin`) — you'll pass this as `repo_name`. +- Read the **actual current changes** end-to-end (the diff against the base branch, and the modified files in full — not just the hunks). Understand what the code does: what behavior each change introduces, what invariants it preserves, what it exposes, what failure modes it handles, what it replaces. + +Everything below is drawn from what the code actually does, cross-checked against `$ARGUMENTS` and any spec/plan already in the session — never from imagination. + +## Step 2: Generate Message + Spec + +Write the message per the `spec-submission` skill, and the Acceptance Criteria per the `acceptance-criteria` skill — **AC are the primary output of this step.** What's specific to Verify is the spec body: a **Key Decisions & Architecture** section. + +### Key Decisions & Architecture (free-form) + +This is the section a reviewer reads to *get* the change. Write it as free-form prose (short paragraphs or a few bullets — whatever reads best), capturing: + +- **The decisions that were made and why.** The fork in the road and the branch taken — "chose a token bucket over a fixed window because bursts are expected," "kept the old response shape to avoid breaking existing clients." The *why* is the point; a decision without its reason is noise. +- **Architectural & data-model changes.** Call out data-model or schema changes explicitly (they're easy to miss and aren't "components or data flows"), plus new components, changed responsibilities, new boundaries or data flows — anything that moves where logic lives or how parts talk to each other. +- **Anything that would surprise a reviewer.** A non-obvious tradeoff, a deliberate scope cut, a constraint that shaped the design, a follow-up left for later. + +What this section is **not**: + +- **Not a file-by-file walkthrough.** "Edited `foo.py`, then `bar.py`, added a helper in `baz.py`" is a changelog, not a decision record. The reviewer can read the diff for that. +- **Not implementation minutiae.** Exact function signatures, variable names, line-level logic, framework boilerplate — leave it out. Name the shape of the change, not its transcript. + +Aim for the altitude of "what a thoughtful reviewer needs to not be surprised, and to trust the change" — the reasoning behind the diff, not the diff. + +### Assembling the spec file + +Generate a single spec file (name it `spec.md`, or preserve the original filename if a spec already exists in the session — use it as-is, don't restructure it). The spec body is **intent + key decisions** — the acceptance criteria are **not** in the spec; they're passed as the `acceptance_criteria` argument at submit. Use these sections: + +``` +## Intent +What this change accomplishes and why. Keep it brief — enough context to make the rest make sense. + +## Key Decisions & Architecture +Free-form prose: the decisions made and why, architectural changes, anything that would surprise a reviewer. Not a file-by-file walkthrough, not implementation minutiae. +``` + +Intent always belongs. Include Key Decisions & Architecture whenever the change has any non-trivial reasoning behind it (nearly always). + +## Step 3: Review Acceptance Criteria with the user + +Run the Acceptance Criteria review loop from the `spec-submission` skill — iterate until the user explicitly confirms. + +One thing specific to Verify: show the user the **intent** line and the **Acceptance Criteria** — not the full spec body (Key Decisions & Architecture is submitted as supporting context, not what the user confirms). If they want to see the rest, they'll ask. + +## Step 4: Submit for Verify + +Submit via the `specSubmit` call described in the `spec-submission` skill. For Verify: + +- `submission_type`: **`"verify"`** — this is the argument that makes it a Verify submission (intent + AC over human-authored code, no step generation). Do not omit it; the default is `"runbook"`. +- `acceptance_criteria`: **required** — the confirmed AC as a JSON array of strings. +- `working_branch`: **required** — the branch the in-flight work lives on (from Step 1), passed by name, so Verify tracks the PR you open from that branch. + +Then return the Runbook URL and set the PR directive, both per the `spec-submission` skill. + +## Step 5: Keep Acceptance Criteria fresh as the PR evolves + +Verify AC are a living contract, not a one-time snapshot. As you keep pushing commits to the connected PR, the code drifts from the AC the user originally signed off on — new behavior appears, scope shifts, an edge case gets handled differently. **Stale AC verify the wrong thing.** + +So, after a meaningful push to the connected PR in this session (a new behavior, a changed contract, a dropped or added piece of scope — not a typo fix): + +1. Re-read the current AC and the runbook's version: `getRunbook(url, fields=['acceptance_criteria'])` — note the returned `runbook_version` (an int). +2. Compare the AC against the **current** diff. If the code now does something the AC don't cover, or an AC no longer matches what the code does, the AC are stale. +3. Refresh them: `editRunbook(runbook_url, expected_version=, payload={"acceptance_criteria": []})`. The payload is the COMPLETE new list — including unchanged items — and expresses add/update/remove/reorder in one atomic edit. If the edit fails with a stale-version error, someone else moved the runbook; re-read and retry. +4. Keep the same quality bar as Step 2 — observable outcomes, no implementation detail — and keep the user in the loop on non-trivial AC changes rather than silently rewriting their signed-off list. + +Do not re-run `specSubmit` to refresh AC — that creates a new runbook. Use `editRunbook` to update the existing one. diff --git a/aviator/skills/acceptance-criteria/SKILL.md b/aviator/skills/acceptance-criteria/SKILL.md new file mode 100644 index 0000000..9799915 --- /dev/null +++ b/aviator/skills/acceptance-criteria/SKILL.md @@ -0,0 +1,108 @@ +--- +name: acceptance-criteria +description: Acceptance Criteria quality rulebook for Aviator spec submissions. Load when generating or reviewing acceptance criteria for an Aviator Verify (/verify-submit) or Runbook (/create-runbook) submission — it defines what makes an AC valid. +--- + +# Acceptance Criteria — quality rulebook + +This is the shared quality bar for Acceptance Criteria (AC) in an Aviator spec submission — it applies identically whether you're submitting for **Verify** (`/verify-submit`) or creating a **Runbook** (`/create-runbook`). AC are the concrete checks that prove a change works correctly and fits the codebase: the test plan a reviewer would actually run. Some AC are programmatically testable (an endpoint returns 401, a command exits 0); others are behavioral, qualitative, or UX expectations — both are valid as long as two reviewers would agree on whether the AC is met. AC are the contract between intent and implementation, and the highest-value artifact in the spec. + +Throughout, "the spec's supporting sections" means the implementation Steps for a Runbook submission, or the Key Decisions & Architecture section for a Verify submission — wherever implementation detail lives for the flow you're in. + +## The goal of AC — observable outcomes, not implementation + +Acceptance criteria define what must be true for the work to be acceptable. Each criterion is a gate. The work is not done until every criterion passes; if any one fails, the work is not yet acceptable. They are the contract for "done." + +Acceptance criteria are NOT an implementation checklist. Internal details (which file was touched, which function was added, which private structure was used) belong in the spec's supporting sections, not in the criteria. + +Favor fewer, sharper criteria over many shallow ones. A handful of strong outcome criteria is better than a long checklist of weak ones. + +## Two readers, both must be served + +Every AC has two audiences and both must accept it: + +- **AI verifier** who later judges pass/fail. The criterion must reduce to a deterministic check the verifier can run against the system, code, or output — DOM/CSS inspection, API calls, file checks, test runs, state queries. If the only way to evaluate it is human judgment, it is not a gate. +- **The human** The criterion must read at a glance. They shouldn't have to mentally filter past implementation noise — file paths, hex codes, pixel values, internal structures, internal type names, function or handler names, internal route paths, queue/task names, middleware steps, infrastructure component names (Redis, Celery, Postgres, Kafka, etc.) — to extract the actual gate. Name the user-visible product, customer-facing surface, or externally observable outcome that's affected; mention internal infrastructure only when it IS the failure mode being tested (e.g. "if the cache is unavailable, requests still succeed"). + +If a candidate AC fails either reader, it doesn't belong on the list. + +## The north-star test — governs every other rule below + +Before writing or keeping any AC, both must hold: + +1. **This change can break it.** Trace the AC to the diff. If nothing in this change could flip it from pass to fail — it names a code path the diff doesn't touch, it guards behavior outside this change's scope — it is out of scope. Drop it, however important the behavior is in the abstract. +2. **A violation has a named victim.** Ask "if this were violated, what gets worse, and for whom?" A specific impact on a specific party — a user hits a bug, the next maintainer trips on an inconsistent pattern, the prod operator can't debug a silent failure — means keep it. A vague answer ("things would just be less good", "code wouldn't be as clean") means filler, delete it. + +Test 2 alone is a trap: "if the auth cron broke, logins would fail" is severe and real, yet if this change never touches the auth cron, test 1 already ruled it out. Severity is not a license to gate what this change can't affect. + +**Both functional issues *and* codebase-health issues** (broken conventions, duplicated logic, missing observability) count — anyone downstream of this change (user, reviewer, maintainer, operator) is a valid party. The build pipeline is not on this list: a green type-check/lint/format/CI run is never the impact an AC defends (see the anti-pattern below). + +## AC covers behavior first, fit second + +- **Functional correctness (the core of every list):** the change does the right thing — golden path, edge cases, failure modes, invariants. *"`divide(1, 0)` returns `Err(DivByZero)`"*. +- **Codebase fit (only when it's a genuine gate):** the change sits well with existing code — reuses existing helpers instead of duplicating logic, matches established patterns, doesn't quietly change a public API, doesn't pull in a new dependency. *"reuses `internal/retry.Backoff` instead of a new loop"*, *"no new entries in `package.json` `dependencies`"*. Skip this axis when the change has no such gate — don't manufacture one to fill a quota. + +## Sources to draw AC from — prioritize code over plan + +Do not generate from imagination. Before writing any AC, go read what's actually there — and treat the sources in this order of priority: + +- **The code in this session (primary source).** The final code — not any original plan — is the ground truth for what this change actually *does*. Read the modified files end-to-end, not just the diff hunks, and understand what the code is trying to do: what behavior each new/changed function introduces, what invariants it preserves, what public surface it exposes, what failure modes it handles, what it replaces or removes. Every behavior present in the code must map to an AC, and the current code must pass every AC you write. +- **Existing spec, plan, or `$ARGUMENTS` (secondary source — cross-check, don't copy blindly).** If the user wrote a spec, ran plan mode, or supplied content via `$ARGUMENTS`, mine it for must-haves, constraints, and explicit success criteria the user already endorsed — preserve those, don't drop them. Use the plan to catch behaviors the code *should* have but doesn't (a gap, not a pass). **When the plan and the code disagree, trust the code** and surface the divergence to the user so they can confirm it was intentional — don't silently write AC for a behavior the code no longer implements. + +If the code would fail one of your AC, that's a signal: either the AC is wrong, or the change is incomplete. Flag the gap to the user rather than papering over it. + +## Rules for valuable AC + +- Describe an observable outcome or user-visible behavior that determines acceptance. +- Declarative, outcome-stating phrasing ("Users can log in with email and password", "API returns 401 for missing auth token"). Describe the resulting state of the system, not an action to take — actions belong in the spec's supporting sections. +- Specific enough to judge pass or fail by inspection. +- Should be human readable — a natural-language sentence, not a code snippet or annotation. + +## Coverage + +Cover the meaningful behavior changes the change delivers. Each criterion is a gate that genuinely affects whether the work is done. Few and sharp beats many and shallow. A long list of overlapping restatements is worse than a short well-chosen set. + +When the deliverable is preserved behavior — refactors, restyles, migrations, dependency upgrades, performance work — "the existing X still works" is a meaningful gate, not a cop-out. Examples: "All existing article actions remain functional and resolve to their previous routes." "Existing tests continue to pass after the change." Do not drop these from preserve-behavior changes just because they sound generic. But this only applies to behavior the change actually touches or could regress. Code the diff never goes near is unrelated code, not preserved behavior. + +- **Verifiable, not necessarily runnable.** Runnable checks (a command exits 0, an endpoint returns 401) are the gold standard, but behavioral, qualitative, and UX criteria are equally valid as long as the criterion is sharp enough that different reviewers would reach the same verdict. The disqualifying test is ambiguity, not un-runnable-ness. +- **Outcome, not work done.** Do not restate the task as an AC. "The column is declared", "the model change is committed", "a new test module exists", "the migration is generated" narrate the work, not a testable outcome. Translate into observable effects: querying the schema returns the new field; the new test cases run and pass. +- **Cover what matters.** Golden path, important edge cases, failure modes, and invariants that must still hold after the change. Skip what doesn't meaningfully change. +- **No redundancy.** Before finalizing, read the list end to end. If two items would be satisfied by the same test, merge or delete. Each criterion must probe a distinct behavior. + +## Anti-patterns — do not produce these + +**Code blocks or snippets inside an AC — strict no.** Every AC is a one-line natural-language gate. Do not embed fenced code blocks, JSON/YAML payloads, SQL, request/response bodies, function signatures, or stack traces inside an AC bullet. If the behavior seems to need code to be clear, the criterion is doing too much — split it, move the example into the spec's supporting sections, or rewrite it at a higher level. +- Bad: an AC bullet followed by a fenced json block showing the expected response. +- Good: "The articles list response includes a published timestamp in ISO-8601 format for every article." + +**Subjective taste words.** Words like "readable," "comfortable," "clean," "modern," "intuitive," "polished," "elegant" name a feeling, not a gate — two reviewers can disagree and both be right. Translate the underlying intent into a checkable structural property, or drop the criterion. +- Bad: "The article body has comfortable line height and airy paragraph spacing." +- Good (if the intent is layout constraint): "The article body is constrained to a centered column rather than spanning the full viewport width." + +**Internal identifiers — exact paths, function/class/type names, line numbers, infra component names — are noise, and never the subject of an AC.** Absolute paths, dotted module paths, resolver/schema-type names, function/handler/class/component/hook/prop names, internal route paths, queue/task names, table/column names, and infrastructure names (Redis, Postgres, Celery, Kafka) describe implementation, not outcome. Reframe so the subject is the user, the customer-visible surface, or an externally observable outcome. +- Bad: "Function `validateJwt` exists in `src/auth/jwt.py`." +- Good: "JWT tokens are validated before any request reaches a protected route." +- Bad (mechanism-led): "Inbound SMS for unpaid customers is dropped at `/api/sms/inbound` before `process_sms` is enqueued." +- Good (outcome-led): "Inbound SMS for unpaid customers is accepted with HTTP 200 but produces no auto-replies, inbox entries, or downstream automation." + + If a value or identifier IS the externally observable contract — an HTTP status code, a public API field name, a customer-facing CLI flag, a documented config key — keep it. The rule targets internal mechanism leaking into AC, not all technical specifics. + +**Internal data shapes or private structures.** +- Bad: "The `User` class has a `roles` list attribute." +- Good: "Authenticated users receive the roles assigned to their account in API responses." + +**Implementation-detail numerics that are noise to the human reader.** Pixel breakpoints, exact rem values, hex/rgba colors, exact font weights. +- **Don't invent specifics the change did not require.** Spec said "service retries on transient failures." Bad AC: "Service retries up to 3 times with 100ms backoff." Good AC: "Service retries on transient failures." +- **Prefer the abstract level even when a value exists** — but stop before "abstract" becomes "subjective taste." When the value IS the contract, keep it verbatim ("API returns 429 when rate-limited"). When it's incidental, name its role ("Badges use the brand accent color"). + +**Build / lint / type-check / format / CI gates — never an AC.** "Typecheck passes", "lint is clean", "CI is green", "all tests pass" are the pipeline's job and tell a reviewer nothing about whether the feature works. The one adjacent case that IS valid is a deliberate regression guard on behavior *this change could plausibly break* ("existing X still works after the refactor"), phrased as the behavior, not as "the test suite passes." + +**Implementation choices and tradeoffs we made — not AC.** Decisions reached while building (an icon instead of a text label, a fallback format kept for older targets) are *how* the feature was built, not gates on *what* it does. Those belong in the spec's supporting sections. An AC states the user-visible outcome, not the option picked to reach it. + +### When the value IS the deliverable + +When the change's purpose is to change a specific value — a version bump, a color token swap, a config introduction — that value belongs in the AC because it IS the gate. +- React 17 → 18 upgrade: "Application runs on React 18 with no deprecation warnings in the browser console" is a strong gate. +- Color token introduction: "$color-accent is defined and used wherever the legacy yellow appeared" is acceptable because the swap IS the deliverable. + +This is the only case where implementation-level specifics earn a place. Do not extend the exception to incidental values. diff --git a/aviator/skills/spec-submission/SKILL.md b/aviator/skills/spec-submission/SKILL.md new file mode 100644 index 0000000..224b2b4 --- /dev/null +++ b/aviator/skills/spec-submission/SKILL.md @@ -0,0 +1,74 @@ +--- +name: spec-submission +description: Shared submission mechanics for Aviator spec commands — how to write the intent, run the acceptance-criteria review loop with the user, call the specSubmit MCP tool, set the Runbook-URL PR directive, and handle errors. Load when running /verify-submit or /create-runbook; the command supplies the flow-specific spec shape and submission_type, this skill supplies everything both flows do identically. +--- + +# Spec submission — shared mechanics + +This is the machinery shared by `/verify-submit` and `/create-runbook`. Your command file provides what differs between the two flows — the spec sections to write and the `submission_type` to send. Everything below is identical across both flows: how the intent reads, how you align the Acceptance Criteria with the user, how you call `specSubmit`, and what happens to any PR opened afterward. + +## The intent + +A short, human-friendly description of what this change accomplishes and why — written the way a person would describe it to a colleague filing a ticket. A few sentences at most. No markdown structure, no file paths, no code details. This is the `intent` argument to `specSubmit`. + +If the user provided `$ARGUMENTS`, lean on their words — echo their intent rather than rephrasing it technically. + +Good: +> Add rate limiting to the public API so a single client can't exhaust capacity, returning 429 with a retry hint once the per-client budget is spent. + +Bad (too technical — that belongs in the spec): +> Add `RateLimiter` middleware in `api/middleware.py`, wire a Redis token bucket keyed by client ID, decrement in `before_request`... + +## Acceptance Criteria are the primary output + +The Acceptance Criteria (AC) are the highest-value artifact of the submission — prioritize their quality over the length or polish of the spec body. Sharp AC with a thin spec beat a lush spec with generic AC. AC are submitted as their own `acceptance_criteria` argument (below), not embedded in the spec. + +**Before writing or reviewing any AC, load the `acceptance-criteria` skill** (Skill tool → `aviator:acceptance-criteria`) and apply its rulebook in full — it defines what makes an AC valid, the two readers each AC must serve, the north-star test, which sources to draw from, and the anti-patterns to avoid. This is a blocking step. + +If that skill is unavailable, fall back to the core rule: every AC is a single-line, human-readable, observable outcome that *this change could break* — no implementation detail, no internal identifiers, no build/lint/CI gates. + +## Reviewing Acceptance Criteria with the user — iterate until aligned + +Before submitting, get the user aligned on the AC. (Your command says what else, if anything, to show or discuss alongside them.) + +- **On the first showing in this flow, preface the AC with a one-line primer** so a user unfamiliar with the term knows what they're reviewing — something like: *"Acceptance Criteria are the code-anchored behaviors this change must satisfy — each one is verified independently. Please review whether these are the right ones."* Adjust the wording to feel natural; always include a primer the first time, skip it on re-shows. +- **Ask one direct question** — something like: *"Do these AC cover what you care about — anything to add, remove, or tighten?"* Keep it to a single question. +- **Apply the feedback** — add missing criteria, remove redundant ones, tighten vague ones, split bundled ones. Re-show the updated list, calling out what changed since the previous round, and ask again. +- **Repeat until the user explicitly confirms.** A simple "yes" or "go ahead" is enough. Do not submit on silence or an implied yes. +- **If invoked non-interactively** (no user available to confirm — e.g. an automated or orchestrated run), treat the generated AC as pre-confirmed and note in your output that the confirmation step was skipped. + +## Locking and submitting + +**Only submit after the user has explicitly confirmed in the review step.** + +**Pass the confirmed AC as the `acceptance_criteria` argument below — do not embed them in the spec markdown.** They're a first-class input; the spec carries intent and supporting context, not the AC. + +Then call the `specSubmit` MCP tool from the Aviator server with: + +- `repo_name`: the repository in `owner/repo` format (derive it from the git remote, e.g. `git remote get-url origin`). +- `submission_type`: **provided by your command** — `"verify"` for `/verify-submit`, `"runbook"` for `/create-runbook`. Pass it explicitly. +- `intent`: the confirmed intent (see "The intent" above). +- `acceptance_criteria`: the exact AC the user signed off on, as a JSON array of strings, e.g. `["First criterion","Second criterion"]`. **Required for Verify** (seeds the structured criteria set); optional for a Runbook (folded into the spec the agent works from). +- `spec_files`: `[{"filename": "", "content": "..."}]` — always a single file. +- `working_branch`: **Verify only** — an existing branch, passed by name; a PR opened from it auto-connects back to this submission (no need to push first — the link is by name). Runbook mode omits it and uses `target_branch`, since the runbook generates its own PR. +- `target_branch` (optional): the branch this work is built on top of — omit for the repo default (trunk); pass the parent branch when this work is stacked on another in-flight branch. + +The tool returns the runbook URL. Treat it as the canonical **Runbook URL** for this session — hold it for any PR opened later in the same session. + +## Return the link and set the PR directive + +Give the user the Runbook URL from the tool response and a brief summary of what was submitted. + +Then, **when opening a PR for this work later in the same session**, the PR body **MUST** begin with `Runbook: ` on its own line, followed by a blank line, then the rest of the description. This applies to `gh pr create`, `av pr`, or any equivalent. + +- **Prepend, don't replace.** The `Runbook:` line goes *above* any template, summary, or drafted body. +- **Exact format.** `Runbook: ` on its own line. Plain text — no markdown link, no emoji. Keep it greppable. +- **Body only, not title.** Never put the URL in the PR title, commit messages, or branch names. +- **Scope.** Applies only to PRs that implement *this* submission's work in *this* session. +- **New PRs only.** If a PR for this work already exists when the command runs, leave it alone. + +## Error handling + +- If authentication is required, Claude Code will automatically open a browser for OAuth login. +- If the repository is not found in Aviator, suggest connecting it in the Aviator dashboard under GitHub settings. +- If the API returns an error about credits, inform the user they may need to add runbook credits in their Aviator dashboard.