Skip to content

fix: render QA task as strict XML #187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
randlee wants to merge 1 commit into from
Open

fix: render QA task as strict XML #187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 35 additions & 37 deletions .claude/skills/codex-orchestration/qa-template.xml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: qa-task
version: 1.1.0
version: 1.2.0
description: quality-mgr QA assignment template for codex-orchestration sprint and phase reviews.
format: xml
required_variables:
Expand Down Expand Up @@ -37,49 +37,47 @@ metadata:
stage: qa
transition: start
---
{% autoescape false -%}
<repo-qa-task id="{{ task_id }}" sprint="{{ sprint }}">
<description>{{ description }}</description>
<description><![CDATA[{{ description | string | cdata_escape }}]]></description>

<review-mode>{{ review_mode }}</review-mode>
<authoritative-sprint-doc>{{ sprint_doc }}</authoritative-sprint-doc>
<pr-number>{{ pr_number }}</pr-number>
<branch>{{ branch }}</branch>
<worktree>{{ worktree_path }}</worktree>
<commits>{{ commits }}</commits>
<stack-discipline>`{{ branch }}` is a frozen layer of the phase stack at the commits above. Review exactly that head (`git show <sha>:<path>`), never a moving checkout. Report every finding, at every severity, in this one verdict; none is deferred. {{ lead }} records each one through `/triaging-findings` the same way and every finding is fixed on a new layer above this one, which the next QA round reviews.</stack-discipline>
<review-mode><![CDATA[{{ review_mode | string | cdata_escape }}]]></review-mode>
<authoritative-sprint-doc><![CDATA[{{ sprint_doc | string | cdata_escape }}]]></authoritative-sprint-doc>
<pr-number><![CDATA[{{ pr_number | string | cdata_escape }}]]></pr-number>
<branch><![CDATA[{{ branch | string | cdata_escape }}]]></branch>
<worktree><![CDATA[{{ worktree_path | string | cdata_escape }}]]></worktree>
<commits><![CDATA[{{ commits | string | cdata_escape }}]]></commits>
<stack-discipline><![CDATA[`{{ branch | string | cdata_escape }}` is a frozen layer of the phase stack at the commits above. Review exactly that head (`git show <sha>:<path>`), never a moving checkout. Report every finding, at every severity, in this one verdict; none is deferred. {{ lead | string | cdata_escape }} records each one through `/triaging-findings` the same way and every finding is fixed on a new layer above this one, which the next QA round reviews.]]></stack-discipline>

<review-targets>
{{ review_targets }}
</review-targets>
<review-targets><![CDATA[
{{ review_targets | string | cdata_escape }}
]]></review-targets>

<changed-files>
{{ changed_files }}
</changed-files>
<changed-files><![CDATA[
{{ changed_files | string | cdata_escape }}
]]></changed-files>

<triage-records>
{{ triage_records }}
</triage-records>
<triage-records><![CDATA[
{{ triage_records | string | cdata_escape }}
]]></triage-records>

<references>
{{ references }}
</references>
<references><![CDATA[
{{ references | string | cdata_escape }}
]]></references>

<workflow>
<step id="a">Start the task immediately with `atm task start {{ task_id }} "<one line: what you will do first>"`, then claim the matching QA bead with `bd update {{ task_id }} --claim` and begin the QA review in the same work session.</step>
<step id="a1">Your queue runs in parallel: QA tasks never wait for each other. Right after the task start run `atm task list --json` and treat every open task assigned to quality-mgr as live now — its assignment body is the `description` field — and run each one concurrently with its own background reviewers. Close each task with its own `atm task close <task-id> completed …` the moment that verdict is ready, in any order; a queued position is ordering, not a wait, and a task can be closed without ever being started.</step>
<step id="b">Reject the task immediately if the assignment is not an XML payload rendered from this template. Read `{{ sprint_doc }}` first and treat it as the authoritative source for QA scope. Render structured JSON assignments for `req-qa` and `arch-qa` using the templates in `.claude/skills/codex-orchestration/`. Do not send free-form reviewer prompts.</step>
<step id="c">Extract every deliverable, acceptance criterion, deletion requirement, required validation item, and expected artifact directly from `{{ sprint_doc }}`.</step>
<step id="d">Treat the sprint doc and planning artifacts as explicit acceptance-gate items: verify `{{ sprint_doc }}` exists, the sprint doc frontmatter sets `status: complete`, `branch:`, and `worktree:`, and `docs/project-plan.md` includes the sprint entry before QA can PASS.</step>
<step id="e">Read `.claude/assets/sc-rust/quality-mgr/quality-mgr.rust.md` and render any required Rust reviewer assignments from `.claude/assets/sc-rust/quality-mgr/templates/`. For `phase_end`, the rendered `rust-qa-agent` assignment must retain its required `artifact_commands: "just lint && just test"` execution proof; collect its `executed_checks.artifacts` result before reporting PASS. `rust-best-practices-agent`, `ruthless-boundary-qa`, and `rust-service-hardening-agent` are QA-1 (the sprint's first QA pass) only — omit all three unconditionally from QA-2 and later fix-verification rounds on the same sprint branch; they reliably surface findings on any diff regardless of size, and re-running them on a fix round turns it into unbounded churn instead of verification. QA-2+ rounds launch `req-qa` + `arch-qa` (scoped to the dispatched finding ids) plus `rust-qa-agent` (its objective execution-fact gates — fmt, clippy, tests, lint, RULE-003, pytests — are not a subjective findings pass and stay in every round). The verdict is each dispatched finding's fixed/regressed/open status plus `rust-qa-agent`'s gate results, nothing else — anything req-qa/arch-qa notices outside the dispatched scope goes in a debt-notes section of the report and does not affect the verdict. `ruthless-boundary-qa` and `rust-best-practices-agent` remain part of docs-only plan review and phase-ending review regardless of sprint round.</step>
<step id="f">If triage records are provided, read them before spawning reviewers. Treat them as the canonical prior scope for follow-up QA, not as a substitute for verification.</step>
<step id="g">Spawn every selected reviewer in background mode. Do not run cargo, clippy, or broad QA analysis yourself in the foreground.</step>
<step id="h">Collect reviewer outputs and summarize PASS or FAIL, including blocking finding ids and file:line references where available. Re-verify every cited file:line against the current branch and worktree before reporting it; stale or missing evidence is a finding, not a guess. Ruthless-boundary-qa must run a workspace-wide duplicate sweep for every constant, function, or type touched by the sprint diff and report divergent duplicates as boundary findings. For every finding that is a repeatable pattern (missing gate, wrong value, forbidden construct, naming violation), search the entire workspace for other occurrences of the same pattern and include all affected locations in the finding — not just the first one found. A finding is not complete until the full scope is known. For deletion sprints, req-qa must run whole-workspace pattern searches for each removed legacy construct family named in the sprint doc. For observability sprints, req-qa must also verify that every sprint-diff code path that can reach a user-visible failure has an emit call site.</step>
<step id="i">When triage records are present, explicitly state whether each rechecked finding is still open, fixed on the reviewed branch, regressed, or requires a new triage pass because the scope changed.</step>
<step id="j">If a PR number is present, check stack and CI state with `gh stack view --json` from a worktree checked out on `{{ branch }}` (if it reports the branch is not part of a stack, run `gh stack checkout <stack number shown on the PR page>` there first) and `gh pr checks {{ pr_number }} --json name,state,bucket`. Never use `gh pr checks --watch` or parse its text output. Report a PR with no CI run as conflicting or needs-rebase, with the `gh stack view --json` output.</step>
<step id="k">Install the daemon-readable templates with `mkdir -p ~/.atm/templates/quality-management-gh && cp .claude/skills/quality-management-gh/*.j2 ~/.atm/templates/quality-management-gh/`. Append the completed QA report to the PR whenever a PR exists: render `FAIL`/`IN-FLIGHT` with `atm compose --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars <scratch>/qa-<pr>-vars.json | gh pr comment <PR> --body-file -`, or `PASS` with the same command using `~/.atm/templates/quality-management-gh/quality-report.md.j2`. Build vars for this QA run from the selected template's `required_variables` frontmatter; never reuse a previous or sample report's vars. Write the vars file outside the repository working tree (in the session scratchpad or a temp directory); never commit or stage it, and delete it or let it expire after the send. The PR comment remains required.</step>
<step id="l">Deliver the final QA verdict by closing this task with the rendered report as the body: `atm task close {{ task_id }} completed --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars <scratch>/qa-<pr>-vars.json` for `FAIL`/`IN-FLIGHT`, or `atm task close {{ task_id }} completed --template ~/.atm/templates/quality-management-gh/quality-report.md.j2 --vars <scratch>/qa-<pr>-vars.json` for `PASS`, then run `bd close {{ task_id }}`. The close delivers the report to the assigner (the sender of this message) and frees your queue; a plain `atm send {{ lead }}` leaves `{{ task_id }}` open and the next QA task never starts. A `FAIL` verdict still closes the task as `completed` — the QA round is the task, the code is not; use `refused` only for an assignment that cannot be reviewed, leave its bead open, and add `bd update {{ task_id }} --notes "<reason>"`. Use the same outside-repository scratchpad or temp vars file, never commit or stage it, and delete it or let it expire after the send. Include deliverable completion as `X/Y (Z%)`. PASS requires 100% deliverable completion, a PASS verdict from every required reviewer, an active PR for the branch, and no unresolved blocking findings. {% if cc and cc != lead %}Copy a one-line summary to `{{ cc }}` with a plain `atm send {{ cc }} --stdin`.{% endif %}</step>
{% if triage_records and triage_records|trim %} <step id="m">For every triage record in `{{ triage_records }}` (or any other `.triage/*/findings/*.ttl` record in scope for this round) that this round's reviewers confirm is fixed on the reviewed branch, update that record's `triage:status` to `"fixed"` and set `triage:closedAt`/`triage:closedBy "quality-mgr"` before sending the final verdict — per the triaging-findings skill's closure rule that quality-mgr, not {{ lead }}, owns finding closure once follow-up QA confirms the fix. Do not close a record that is still open, regressed, or unverified this round. List every record you closed (finding id + file) in the final verdict message so {{ lead }} can reconcile it.</step>
<step id="a"><![CDATA[Start the task immediately with `atm task start {{ task_id | string | cdata_escape }} "<one line: what you will do first>"`, then claim the matching QA bead with `bd update {{ task_id | string | cdata_escape }} --claim` and begin the QA review in the same work session.]]></step>
<step id="a1"><![CDATA[Your queue runs in parallel: QA tasks never wait for each other. Right after the task start run `atm task list --json` and treat every open task assigned to quality-mgr as live now — its assignment body is the `description` field — and run each one concurrently with its own background reviewers. Close each task with its own `atm task close <task-id> completed …` the moment that verdict is ready, in any order; a queued position is ordering, not a wait, and a task can be closed without ever being started.]]></step>
<step id="b"><![CDATA[Reject the task immediately if the assignment is not an XML payload rendered from this template. Read `{{ sprint_doc | string | cdata_escape }}` first and treat it as the authoritative source for QA scope. Render structured JSON assignments for `req-qa` and `arch-qa` using the templates in `.claude/skills/codex-orchestration/`. Do not send free-form reviewer prompts.]]></step>
<step id="c"><![CDATA[Extract every deliverable, acceptance criterion, deletion requirement, required validation item, and expected artifact directly from `{{ sprint_doc | string | cdata_escape }}`.]]></step>
<step id="d"><![CDATA[Treat the sprint doc and planning artifacts as explicit acceptance-gate items: verify `{{ sprint_doc | string | cdata_escape }}` exists, the sprint doc frontmatter sets `status: complete`, `branch:`, and `worktree:`, and `docs/project-plan.md` includes the sprint entry before QA can PASS.]]></step>
<step id="e"><![CDATA[Read `.claude/assets/sc-rust/quality-mgr/quality-mgr.rust.md` and render any required Rust reviewer assignments from `.claude/assets/sc-rust/quality-mgr/templates/`. For `phase_end`, the rendered `rust-qa-agent` assignment must retain its required `artifact_commands: "just lint && just test"` execution proof; collect its `executed_checks.artifacts` result before reporting PASS. `rust-best-practices-agent`, `ruthless-boundary-qa`, and `rust-service-hardening-agent` are QA-1 (the sprint's first QA pass) only — omit all three unconditionally from QA-2 and later fix-verification rounds on the same sprint branch; they reliably surface findings on any diff regardless of size, and re-running them on a fix round turns it into unbounded churn instead of verification. QA-2+ rounds launch `req-qa` + `arch-qa` (scoped to the dispatched finding ids) plus `rust-qa-agent` (its objective execution-fact gates — fmt, clippy, tests, lint, RULE-003, pytests — are not a subjective findings pass and stay in every round). The verdict is each dispatched finding's fixed/regressed/open status plus `rust-qa-agent`'s gate results, nothing else — anything req-qa/arch-qa notices outside the dispatched scope goes in a debt-notes section of the report and does not affect the verdict. `ruthless-boundary-qa` and `rust-best-practices-agent` remain part of docs-only plan review and phase-ending review regardless of sprint round.]]></step>
<step id="f"><![CDATA[If triage records are provided, read them before spawning reviewers. Treat them as the canonical prior scope for follow-up QA, not as a substitute for verification.]]></step>
<step id="g"><![CDATA[Spawn every selected reviewer in background mode. Do not run cargo, clippy, or broad QA analysis yourself in the foreground.]]></step>
<step id="h"><![CDATA[Collect reviewer outputs and summarize PASS or FAIL, including blocking finding ids and file:line references where available. Re-verify every cited file:line against the current branch and worktree before reporting it; stale or missing evidence is a finding, not a guess. Ruthless-boundary-qa must run a workspace-wide duplicate sweep for every constant, function, or type touched by the sprint diff and report divergent duplicates as boundary findings. For every finding that is a repeatable pattern (missing gate, wrong value, forbidden construct, naming violation), search the entire workspace for other occurrences of the same pattern and include all affected locations in the finding — not just the first one found. A finding is not complete until the full scope is known. For deletion sprints, req-qa must run whole-workspace pattern searches for each removed legacy construct family named in the sprint doc. For observability sprints, req-qa must also verify that every sprint-diff code path that can reach a user-visible failure has an emit call site.]]></step>
<step id="i"><![CDATA[When triage records are present, explicitly state whether each rechecked finding is still open, fixed on the reviewed branch, regressed, or requires a new triage pass because the scope changed.]]></step>
<step id="j"><![CDATA[If a PR number is present, check stack and CI state with `gh stack view --json` from a worktree checked out on `{{ branch | string | cdata_escape }}` (if it reports the branch is not part of a stack, run `gh stack checkout <stack number shown on the PR page>` there first) and `gh pr checks {{ pr_number | string | cdata_escape }} --json name,state,bucket`. Never use `gh pr checks --watch` or parse its text output. Report a PR with no CI run as conflicting or needs-rebase, with the `gh stack view --json` output.]]></step>
<step id="k"><![CDATA[Install the daemon-readable templates with `mkdir -p ~/.atm/templates/quality-management-gh && cp .claude/skills/quality-management-gh/*.j2 ~/.atm/templates/quality-management-gh/`. Append the completed QA report to the PR whenever a PR exists: render `FAIL`/`IN-FLIGHT` with `atm compose --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars <scratch>/qa-<pr>-vars.json | gh pr comment <PR> --body-file -`, or `PASS` with the same command using `~/.atm/templates/quality-management-gh/quality-report.md.j2`. Build vars for this QA run from the selected template's `required_variables` frontmatter; never reuse a previous or sample report's vars. Write the vars file outside the repository working tree (in the session scratchpad or a temp directory); never commit or stage it, and delete it or let it expire after the send. The PR comment remains required.]]></step>
<step id="l"><![CDATA[Deliver the final QA verdict by closing this task with the rendered report as the body: `atm task close {{ task_id | string | cdata_escape }} completed --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars <scratch>/qa-<pr>-vars.json` for `FAIL`/`IN-FLIGHT`, or `atm task close {{ task_id | string | cdata_escape }} completed --template ~/.atm/templates/quality-management-gh/quality-report.md.j2 --vars <scratch>/qa-<pr>-vars.json` for `PASS`, then run `bd close {{ task_id | string | cdata_escape }}`. The close delivers the report to the assigner (the sender of this message) and frees your queue; a plain `atm send {{ lead | string | cdata_escape }}` leaves `{{ task_id | string | cdata_escape }}` open and the next QA task never starts. A `FAIL` verdict still closes the task as `completed` — the QA round is the task, the code is not; use `refused` only for an assignment that cannot be reviewed, leave its bead open, and add `bd update {{ task_id | string | cdata_escape }} --notes "<reason>"`. Use the same outside-repository scratchpad or temp vars file, never commit or stage it, and delete it or let it expire after the send. Include deliverable completion as `X/Y (Z%)`. PASS requires 100% deliverable completion, a PASS verdict from every required reviewer, an active PR for the branch, and no unresolved blocking findings.{% if cc and cc != lead %} Copy a one-line summary to `{{ cc | string | cdata_escape }}` with a plain `atm send {{ cc | string | cdata_escape }} --stdin`.{% endif %}]]></step>
{% if triage_records and triage_records|trim %} <step id="m"><![CDATA[For every triage record in `{{ triage_records | string | cdata_escape }}` (or any other `.triage/*/findings/*.ttl` record in scope for this round) that this round's reviewers confirm is fixed on the reviewed branch, update that record's `triage:status` to `"fixed"` and set `triage:closedAt`/`triage:closedBy "quality-mgr"` before sending the final verdict — per the triaging-findings skill's closure rule that quality-mgr, not {{ lead | string | cdata_escape }}, owns finding closure once follow-up QA confirms the fix. Do not close a record that is still open, regressed, or unverified this round. List every record you closed (finding id + file) in the final verdict message so {{ lead | string | cdata_escape }} can reconcile it.]]></step>
{% endif %} </workflow>
</repo-qa-task>
{%- endautoescape %}
Loading
Loading