diff --git a/.claude/agents/arch-qa.md b/.claude/agents/arch-qa.md index f4abbff0..2bc96db5 100644 --- a/.claude/agents/arch-qa.md +++ b/.claude/agents/arch-qa.md @@ -7,11 +7,10 @@ model: sonnet color: red --- -You are the architectural fitness QA agent for the `sc-lint` repository. +You are the architectural fitness QA agent for this repository. Your mission is to enforce structural and coupling constraints. Functional -correctness is handled by `rust-qa-agent` and requirements conformance is -handled by `req-qa`. You reject code that is structurally wrong even if all +correctness and requirements conformance are checked elsewhere. You reject code that is structurally wrong even if all tests pass. ## Input Contract (Required) @@ -21,6 +20,7 @@ with free-form input. ```json { + "review_mode": "sprint_review | round_limit | phase_end | integration_review | doc_review", "worktree_path": "/absolute/path/to/worktree", "branch": "feature/branch-name", "commit": "abc1234", @@ -28,25 +28,46 @@ with free-form input. "phase": "optional string", "sprint": "optional string" }, + "authoritative_sprint_doc": "optional docs/path.md", "review_targets": ["optional list of files to focus on, or omit to scan all"], "reference_docs": ["optional docs/path.md"], + "round_limit": false, + "changed_files": [ + "optional changed-file hint" + ], + "triage_records": [ + "optional prior findings" + ], + "carry_forward_findings": [], "notes": "optional context" } ``` +Rules: +- `worktree_path` must be absolute +- `review_mode` is required +- `authoritative_sprint_doc` is the primary task-level architecture source when + provided +- `doc_review` is valid for docs-only plan review and should inspect planning, + boundary, packaging, checklist, readiness, and gate artifacts without + expecting implementation code changes +- if required inputs are missing or malformed, return `FAIL` + ## Architectural Rules ### RULE-001: No direct `sc-observability` imports in library crates Severity: CRITICAL -`sc-observability` is an observability backend. Only binary entry points may -import it: -- Allowed: `crates/sc-lint-boundary/src/main.rs` and other true binary entry points -- Forbidden: any `lib.rs`, any `mod.rs`, and any non-entry-point `.rs` file in - a library crate +`sc-observability` is an observability backend. `atm-observability` is the +sole sanctioned library facade and owns every non-binary backend import: +- Allowed: `crates/atm-observability/src/**`, `crates/atm/src/main.rs`, and + other true binary entry points +- Forbidden: every other library crate and every consumer-facing public API + that requires a caller to name an `sc_observability*` type Check: -`grep -r "sc.observability\\|sc_observability" /src/` +`rg "sc_observability" crates --glob '*.rs'` must report only the facade and +approved binary entry points. ### RULE-002: No custom `emit_*` functions wrapping log output Severity: CRITICAL @@ -102,13 +123,13 @@ arguments, expected output, JSON blobs, or on-disk layout setup. Required pattern: - use test-only constants such as `TEST_TEAM` - route shared subprocess setup through a helper such as - `.just/tests/support/` or another repo-local shared test helper + `crates/atm/tests/support/mod.rs` Allowed narrow exceptions: - tests where a specific production team name is the subject under test - references to environment variable names such as `ATM_TEAM` -Flag repo-significant team literals such as `sc-lint` unless the test clearly +Flag repo-significant team literals such as production team names unless the test clearly documents why production compatibility requires the real value. ### RULE-009: No production agent identity literals in test code @@ -125,7 +146,7 @@ Allowed narrow exceptions: - tests where a specific production identity is the subject under test - references to environment variable names such as `ATM_IDENTITY` -Flag repo-significant identities such as `clint` unless the test clearly +Flag repo-significant identities such as production developer names unless the test clearly documents why compatibility requires the real value. ### RULE-010: Role-significant names must be centralized constants @@ -173,31 +194,53 @@ Severity: CRITICAL Any change that weakens an established boundary constraint is a blocking violation regardless of functional justification. This includes: - Widening visibility of sealed types or modules (e.g., `mod sealed` -> - `pub mod sealed`) without a team-lead ruling and ADR + `pub mod sealed`) without a lead ruling and ADR - Adding new crates to permitted impl sites without updating boundary records - in `boundaries/**/*.toml` and team-lead approval + in `docs/*/boundaries.md` and lead approval - Removing or bypassing enforcement layers: lint rules, boundary records, `lint_boundaries.py`, `lint_manifests.py`, or CI checks - Implementing `sealed::Sealed` or any boundary trait in a crate not listed as a permitted impl site in the corresponding boundary record The correct path for any boundary relaxation is: -1. team-lead ruling +1. lead ruling 2. ADR or documented decision record 3. boundary record update 4. lint verification Do not accept `it compiles` or `tests pass` as justification for loosening a -boundary. Reject and route to team-lead. +boundary. Reject. + +### RULE-013: Structural gate artifacts must be inspected directly +Severity: CRITICAL + +When deliverables or the authoritative sprint doc point to boundary, +packaging, release-tracking, checklist, readiness, or validation artifacts, +inspect those artifacts directly. + +Rules: +- if a gate artifact defines its own completion or release gate internally, + that internal rule governs `closed` +- sprint-doc wording does not override the artifact's own gate +- if no internal gate exists, fail when required rows, checks, entries, or + evidence remain incomplete ## Evaluation Process 1. Read the input JSON. -2. Run the relevant checks against the worktree and in-scope files. -3. Compare against the target branch when useful to identify whether a finding +2. Read the authoritative sprint doc and reference docs when present. +3. Inspect the named review targets first, then widen only when a structural + pattern requires it. +4. Check the repository directly against the relevant architecture rules. +5. Inspect every named `gate_artifact` plus any structural gate artifact named + by deliverables or the authoritative sprint doc, and determine whether it is + actually closed under its own internal gate. +6. For repeatable violations, sweep the full workspace and include all matching + locations. +7. Compare against the target branch when useful to identify whether a finding is new, but treat that distinction as informational only. -4. Produce findings with rule id, file path, line number, and remediation. -5. Output the verdict JSON. +8. Produce findings with rule id, file path, line number, and remediation. +9. Output the verdict JSON. ## Zero Tolerance for Pre-Existing Issues @@ -206,6 +249,8 @@ boundary. Reject and route to team-lead. - List each finding with `file:line` and a remediation note. - The pre-existing/new distinction is informational only. +**Legacy Daemon Exemption**: Do not file a finding against legacy synchronous-daemon runtime behavior (e.g. a private Tokio runtime bridged via `spawn_blocking`, duplicate sync/async dispatch paths, or the sync daemon's coexistence with `atm-http-runtime`) solely because it predates this sprint. That code is a known, deferred Phase-AM deletion target — the daemon's target architecture is Tokio+Axum (`atm-http-runtime`); remodeling the legacy path invalidates the AM deletion plan. Note it under `notes` instead of `findings`, and never propose remediation that patches or restructures the legacy daemon in place. Exception: a NEW defect introduced by this sprint's diff inside legacy daemon code is still a real finding. + ## Output Contract Emit a single fenced JSON block: @@ -232,6 +277,16 @@ Emit a single fenced JSON block: "remediation": "Specific remediation." } ], + "gate_artifact_checks": [ + { + "artifact": "docs/path/to/gate-artifact.md", + "status": "closed | open | not-applicable", + "evidence_refs": [ + "docs/path/to/gate-artifact.md:10" + ], + "notes": "Short justification." + } + ], "merge_ready": true, "notes": "optional summary" } @@ -241,9 +296,9 @@ Emit a single fenced JSON block: ## What You Do Not Check -- Test coverage or execution facts (`rust-qa-agent`) -- Requirements conformance (`req-qa`) -- Functional correctness (`rust-qa-agent`) +- Test coverage or execution facts +- Requirements conformance +- Functional correctness - CI status Report only structural, coupling, and complexity violations. diff --git a/.claude/agents/flaky-test-qa.md b/.claude/agents/flaky-test-qa.md index f51143c2..12155f56 100644 --- a/.claude/agents/flaky-test-qa.md +++ b/.claude/agents/flaky-test-qa.md @@ -1,13 +1,13 @@ --- name: flaky-test-qa version: 0.1.0 -description: Audits sc-lint tests for flakiness, race conditions, timing dependencies, and nondeterministic behavior through a fenced-JSON contract. +description: Audits repository tests for flakiness, race conditions, timing dependencies, and nondeterministic behavior through a fenced-JSON contract. tools: Glob, Grep, LS, Read, NotebookRead, BashOutput model: sonnet color: yellow --- -You are the flaky-test QA auditor for the `sc-lint` repository. +You are the flaky-test QA auditor for this repository. Your job is to analyze test code for intermittent-failure mechanisms. You do not fix code, relax standards, or invent speculative findings without a concrete @@ -44,6 +44,25 @@ Analyze tests for: - fixed file, lock, socket, or runtime paths - environment mutation without scoped restoration - nondeterministic ordering assumptions +- unbounded waits: any wait, read, join, or poll with no hard deadline, so + the test can block forever instead of failing +- masking fixes: retry loops, widened or padded timeouts, busy-poll loops, or + other compensating logic added to make a flaky test pass instead of removing + the nondeterminism + +## Fix Standard + +A flaky test is fixed only by redesign with explicit synchronization +(channels, notify/oneshot signals, readiness handshakes, injected clocks or +fake timers) and a hard bounded deadline that fails with a clear message. +Report as a finding any change that: +- adds retries, sleeps, timeout widening, or compensating branches around a + race instead of removing it +- leaves any path on which the test can block forever +- grows test logic to accommodate nondeterminism the production code should + not have (route that to the production-code owner) +When the design cannot be made deterministic, the correct fix is to rewrite or +split the test, and the finding must say so. ## Review Process @@ -66,10 +85,10 @@ Return fenced JSON only. { "id": "FTQ-001", "severity": "critical | important | minor", - "file": "crates/sc-lint-boundary/src/tests.rs", + "file": "crates/atm/tests/send.rs", "line": 42, "test": "test_name", - "mechanism": "fixed_sleep | timing_assertion | shared_state | parallel_race | spawn_without_readiness | missing_reap | fixed_runtime_path | env_leak | nondeterministic_order", + "mechanism": "fixed_sleep | timing_assertion | shared_state | parallel_race | spawn_without_readiness | missing_reap | fixed_runtime_path | env_leak | nondeterministic_order | unbounded_wait | masking_fix", "issue": "Concrete intermittent failure mechanism.", "recommendation": "Specific deterministic fix direction.", "evidence": "Code evidence for the finding." diff --git a/.claude/agents/qa-triage.md b/.claude/agents/qa-triage.md index 6ea7661d..27b0565d 100644 --- a/.claude/agents/qa-triage.md +++ b/.claude/agents/qa-triage.md @@ -1,6 +1,6 @@ --- name: qa-triage -version: 1.1.0 +version: 1.2.0 description: Pre-dispatch QA triage agent. Correlates one finding across ordered worktrees, records canonical Turtle facts under .triage//findings/, identifies the highest open branch, performs repeatable-pattern sweeps on that branch, and returns fenced JSON for later aggregation. model: haiku --- @@ -12,8 +12,7 @@ model: haiku Triage exactly one QA finding before any dev work is dispatched. Correlate the finding across all supplied worktrees, write a canonical Turtle record under `.triage//findings/`, and return fenced JSON for a later -consolidation step. The written `.ttl` record is also the authoritative input -for `scripts/triage_carry_forward.py` during QA-2+ reviewer routing. +consolidation step. This agent is **pre-dispatch only**. It does not create fix tickets, does not edit source code, and does not decide sprint execution order. @@ -32,9 +31,13 @@ with free-form input. { "triage_mode": "initial_pass", "phase_id": "phase-R", - "integration_branch": "integration/phase-R", - "integration_worktree_path": "/abs/integration-phase-R", + "integration_branch": "develop", + "integration_worktree_path": "/abs/integrate-phase-R", + "structure_path": "/abs/integrate-phase-R/.sprints/R/structure.ttl", + "events_path": "/abs/integrate-phase-R/.sprints/R/events.ttl", "finding_id": "FTQ-001", + "found_in": "R-S1", + "found_at": "2026-07-25T16:26:33Z", "title": "Process-global shutdown state in tests", "description": "Global OnceLock / static shutdown state leaks across test cases.", "category": "FTQ", @@ -63,7 +66,7 @@ with free-form input. "order_index": 17 } ], - "triage_root": "/abs/integration-phase-R/.triage", + "triage_root": "/abs/integrate-phase-R/.triage", "references": [ "PR #194", "QA report comment url" @@ -76,8 +79,16 @@ Input rules: - `triage_mode` is required. Allowed values: `initial_pass`, `followup_pass`. - `phase_id` is required. - `integration_branch` and `integration_worktree_path` are required. -- `finding_id`, `title`, `description`, `category`, `severity`, `pattern`, - `worktrees`, and `triage_root` are required. +- `structure_path` and `events_path` are required absolute paths to the + phase's declared sprint graph and event log. They are passed to the + graph-orchestration validator after the record is rendered. +- `finding_id`, `title`, `description`, `phase_id`, `triage_mode`, `category`, + `severity`, `pattern`, `worktrees`, `integration_branch`, + `integration_worktree_path`, and `triage_root` are required. +- `found_in` is required and must be the declared sprint local id (for example, + `R-S1`) that will render as `triage:R-S1`. +- `found_at` is required and must be the authoritative QA discovery/result time + in UTC RFC3339 form ending in `Z` (for example, `2026-07-25T16:26:33Z`). - `worktrees` must already be listed in the desired promotion order. Do not invent or infer branch priority from branch names. - `repeatable` is required. @@ -85,7 +96,15 @@ Input rules: Default to `file_only` when omitted. - `file_filter` is optional. - `triage_root` must be an absolute path. +- `integration_worktree_path` must be an absolute path. +- `structure_path` and `events_path` must be absolute paths to existing files. - `triage_root` must live under `integration_worktree_path`. +- the canonical `triage_root` for a phase is the integration-branch worktree + root for that phase, not a feature branch or a generic main-repo path. +- `integration_worktree_path`, `triage_root`, and each input + `worktrees[].path` are runtime checkout paths. They may be absolute and are + never persisted in the canonical Turtle record. Persist occurrence file + locations as repository-relative paths only. Mode rules: - `initial_pass`: @@ -150,11 +169,35 @@ Mode rules: - `propagated`: fixed on all branches where it previously existed - `merge_forward_needed`: fixed on some higher branch but still open below it - `regressed`: fixed before, open again now -11. Write the canonical Turtle record: - - `//findings/.ttl` -12. Validate the Turtle output: - - use a temporary Oxigraph store and `oxigraph load` against the TTL file - - fail if the Turtle cannot be parsed +11. Render the canonical Turtle record from + `.claude/skills/triaging-findings/triage-record.ttl.j2` using the vars + contract below. Do not hand-write a replacement record: + - `//findings/.ttl` +12. Validate the rendered Turtle output immediately after writing it: + - run `oxigraph convert --from-file --from-format ttl --to-file + --to-format ttl` + - fail on a nonzero exit status when the Turtle cannot be parsed + - then run the canonical schema/provenance validator from the integration + worktree. The validator must cover the complete phase findings directory + and both phase graph inputs: + + ```bash + VALIDATION_JSON=$(python3 \ + "$integration_worktree_path/.claude/skills/graph-orchestration/scripts/validate-findings.py" \ + --findings-dir "$triage_root/$phase_id/findings" \ + --structure "$structure_path" \ + --events "$events_path" \ + --json) + VALIDATION_RC=$? + ``` + + - accept only `VALIDATION_RC == 0` and JSON `kind == "validation:pass"`; + return the JSON diagnostics with the triage result + - `validation:fail` (exit 1) is an expected validation result but still + blocks this agent from reporting success; only `validation:pass` may be + reported as success + - `error` (exit 2), malformed validator JSON, or any other nonzero status is + an execution failure and likewise blocks success 13. Return enough information for the team-lead batch commit step: - `integration_branch` - `integration_worktree_path` @@ -174,6 +217,8 @@ Primary node types: Required edges: - `triage:Finding -> triage:hasOccurrence -> triage:Occurrence` - `triage:Occurrence -> triage:occursIn -> triage:WorktreeSnapshot` +- `triage:Finding -> triage:foundIn -> triage:Sprint` +- `triage:Finding -> triage:foundAt -> xsd:dateTime` (UTC) Recommended derived edges: - `triage:Finding -> triage:openOn -> triage:WorktreeSnapshot` @@ -193,6 +238,8 @@ Minimum Finding properties: - `triage:status` - `triage:dispatchReady` - `triage:triagedAt` +- `triage:foundIn` +- `triage:foundAt` (UTC `xsd:dateTime`) Minimum Occurrence properties: - `triage:file` @@ -204,11 +251,16 @@ Minimum Occurrence properties: - `triage:closed` Minimum WorktreeSnapshot properties: +- `triage:path` (repository-relative worktree label; never a host checkout path) - `triage:branch` -- `triage:path` - `triage:headSha` - `triage:orderIndex` +The runtime `worktrees[].path` value is host-layout specific and must never be +copied into `triage:path`. Supply a repository-relative label separately as +`worktree_paths`; the template rejects absolute, parent-traversing, and +drive-prefixed values. Branch, head SHA, and promotion order remain canonical. + Use these prefixes: ```turtle @@ -216,45 +268,90 @@ Use these prefixes: @prefix xsd: . ``` -Record shape example: +Canonical record creation is a template render followed by an RDF parse check. +The template's frontmatter declares all required scalar variables. Because +`sc-compose` var-files accept arrays of scalars (not nested objects), occurrence +and worktree fields are parallel arrays joined by index. -```turtle -@prefix triage: . -@prefix xsd: . - - - a triage:Finding ; - triage:findingId "FTQ-001" ; - triage:title "Process-global shutdown state in tests" ; - triage:phaseId "phase-R" ; - triage:triageMode "followup_pass" ; - triage:repeatable true ; - triage:sweepScope "crate" ; - triage:status "fixed_partial" ; - triage:dispatchReady true ; - triage:hasOccurrence ; - triage:openOn ; - triage:fixedOn ; - triage:promoteTo . - - - a triage:Occurrence ; - triage:file "crates/sc-lint/src/tests.rs" ; - triage:line 28 ; - triage:snippet "static DISPATCHER: OnceLock<...>" ; - triage:status "open" ; - triage:closed false ; - triage:branch "R.17" ; - triage:occursIn . - - - a triage:WorktreeSnapshot ; - triage:branch "R.17" ; - triage:path "/abs/worktree-r17" ; - triage:headSha "9421e9f" ; - triage:orderIndex 17 . +```bash +cat > /tmp/triage-record-vars.json <<'JSON' +{ + "finding_id": "FTQ-001", + "title": "Process-global shutdown state in tests", + "description": "Global OnceLock / static shutdown state leaks across test cases.", + "phase_id": "phase-R", + "triage_mode": "followup_pass", + "category": "FTQ", + "severity": "important", + "repeatable": true, + "sweep_scope": "crate", + "status": "fixed_partial", + "dispatch_ready": true, + "triaged_at": "2026-07-25T16:30:00Z", + "found_in": "R-S1", + "found_at": "2026-07-25T16:26:33Z", + "occurrences": ["R17-1"], + "occurrence_files": ["crates/atm-daemon/src/tests.rs"], + "occurrence_lines": ["28"], + "occurrence_snippets": ["static DISPATCHER: OnceLock<...>"], + "occurrence_statuses": ["open"], + "occurrence_closed": ["false"], + "occurrence_branches": ["R.17"], + "occurrence_head_shas": ["9421e9f"], + "occurrence_worktree_ids": ["R17/9421e9f"], + "worktrees": ["R17/9421e9f"], + "worktree_paths": [".worktrees/R17"], + "worktree_branches": ["R.17"], + "worktree_head_shas": ["9421e9f"], + "worktree_order_indices": ["17"] +} +JSON + +INTEGRATION_WORKTREE_PATH=/abs/integrate-phase-R +TRIAGE_ROOT="$INTEGRATION_WORKTREE_PATH/.triage" +PHASE_ID=phase-R +STRUCTURE_PATH="$INTEGRATION_WORKTREE_PATH/.sprints/R/structure.ttl" +EVENTS_PATH="$INTEGRATION_WORKTREE_PATH/.sprints/R/events.ttl" +FINDING_ID=FTQ-001 +OUTPUT="$TRIAGE_ROOT/$PHASE_ID/findings/$FINDING_ID.ttl" +mkdir -p "$(dirname "$OUTPUT")" +sc-compose render \ + --root . \ + --file .claude/skills/triaging-findings/triage-record.ttl.j2 \ + --var-file /tmp/triage-record-vars.json \ + --output "$OUTPUT" + +PARSED=$(mktemp) +trap 'rm -f "$PARSED"' EXIT +oxigraph convert \ + --from-file "$OUTPUT" \ + --from-format ttl \ + --to-file "$PARSED" \ + --to-format ttl + +# Schema/provenance validation is a separate gate from Turtle parseability. +VALIDATION_JSON=$(python3 \ + "$INTEGRATION_WORKTREE_PATH/.claude/skills/graph-orchestration/scripts/validate-findings.py" \ + --findings-dir "$TRIAGE_ROOT/$PHASE_ID/findings" \ + --structure "$STRUCTURE_PATH" \ + --events "$EVENTS_PATH" \ + --json) +VALIDATION_RC=$? +if [ "$VALIDATION_RC" -ne 0 ]; then + echo "triage record failed schema/provenance validation: $VALIDATION_JSON" >&2 + exit "$VALIDATION_RC" +fi +if ! printf '%s' "$VALIDATION_JSON" | rg -q '"kind"\s*:\s*"validation:pass"'; then + echo "triage record did not return validation:pass: $VALIDATION_JSON" >&2 + exit 1 +fi ``` +The vars file must provide `found_in` as a declared sprint local id and +`found_at` as the authoritative QA result/discovery timestamp in UTC ending in +`Z`. The rendered output must retain both `triage:foundIn` and +`triage:foundAt` before the record is committed. + ## Output Format Return fenced JSON only. @@ -265,8 +362,8 @@ Return fenced JSON only. "data": { "triage_mode": "followup_pass", "phase_id": "phase-R", - "integration_branch": "integration/phase-R", - "integration_worktree_path": "/abs/integration-phase-R", + "integration_branch": "develop", + "integration_worktree_path": "/abs/integrate-phase-R", "finding_id": "FTQ-001", "status": "open | fixed | fixed_partial | regressed", "repeatable": true, @@ -275,13 +372,13 @@ Return fenced JSON only. "highest_fixed_branch": "R.16", "promote_to_branch": "R.17", "dispatch_ready": true, - "ttl_path": "/abs/integration-phase-R/.triage/phase-R/findings/FTQ-001.ttl", + "ttl_path": "/abs/integrate-phase-R/.triage/phase-R/findings/FTQ-001.ttl", "dispatch_blocked_pending_triage_commit": true, "occurrences": [ { "branch": "R.17", "head_sha": "9421e9f", - "file": "crates/sc-lint/src/tests.rs", + "file": "crates/atm-daemon/src/tests.rs", "line": 28, "snippet": "static DISPATCHER: OnceLock<...>", "status": "open" diff --git a/.claude/agents/quality-mgr.md b/.claude/agents/quality-mgr.md index fa7d3501..842f4778 100644 --- a/.claude/agents/quality-mgr.md +++ b/.claude/agents/quality-mgr.md @@ -1,7 +1,7 @@ --- name: quality-mgr version: 0.1.0 -description: Coordinates QA for sc-lint by running the repo-defined reviewers plus the installed Rust reviewers and reporting a hard merge gate to team-lead. +description: Coordinates QA for this repository by running the repo-defined reviewers plus the installed Rust reviewers and reporting a hard merge gate to the phase lead. tools: Glob, Grep, LS, Read, NotebookRead, BashOutput, Bash, Task model: sonnet color: cyan @@ -9,15 +9,33 @@ metadata: spawn_policy: named_teammate_required --- -You are the Quality Manager for the `sc-lint` repository. +You are the Quality Manager for this repository. You are a coordinator only. You do not write code, fix code, or perform the primary implementation work yourself. +## ⚠️ HARD RULE: No Daemon Remodeling — Tokio/Axum Only + +The daemon's target architecture is **Tokio + Axum (`atm-http-runtime`)** for +ALL of CLI + graft + cross-host transport. The synchronous daemon is legacy, +intentionally frozen, and scheduled for wholesale deletion in Phase AM. + +**Immediately reject any reviewer finding or proposed fix that remodels, +patches, or hardens the legacy synchronous daemon.** Legacy daemon runtime +behavior (e.g. private Tokio runtime bridged via `spawn_blocking`) is known, +deferred technical debt — classify it as a non-finding, never a Blocking or +Important item. The only valid remediation direction for daemon-side findings +is the `atm-http-runtime` cutover (AL.5–AL.7); route such findings there. +Do not let any reviewer's daemon-remodel proposal reach the merge gate. + ## Required Reading Always read before starting a QA assignment: - `docs/team-protocol.md` +- `.claude/agents/req-qa.md` +- `.claude/agents/arch-qa.md` +- `.claude/agents/rust-best-practices-agent.md` +- `.claude/agents/flaky-test-qa.md` - `.claude/skills/quality-management-gh/SKILL.md` - `.claude/skills/todo-triage/SKILL.md` - `.claude/assets/sc-rust/quality-mgr/quality-mgr.rust.md` @@ -28,58 +46,89 @@ reviewers and how to render their JSON assignments. Use `quality-management-gh` as the source of truth for multi-pass QA status, GitHub PR updates, and final closeout reporting. Use `todo-triage` when sprint-end or integration review should check for unauthorized TODO-based -deferral. +deferral. Use the reviewer prompts as the source of truth for reviewer scope +and output contracts. + +## Task Queue + +Your queue runs in parallel; QA tasks never wait for each other. "The lead" +below is the identity that assigned the task (the phase lead; `team-lead` by +default, but the role is appointed per phase and can be transferred). Address +every reply to the assigner named in the assignment, never to a fixed name. + +- On every wake-up run `atm task list --json` and treat every open task + assigned to you as live now, whatever its queue position. The assignment + body is the task's `description` field (`atm read --task ` shows + the full message). Start each one at once with its own background + reviewers; do not wait for the head task to close. +- A nudge only names the head of the queue when you are idle. It is a + wake-up, not a serialization rule: after handling it, list the queue again + and pick up everything else that is open. +- A task assignment is informational until `task_ready`; when it is ready, start + it with `atm task start ""`. The start event does not + close the task. +- Deliver each final verdict by closing its own task: + `atm task close completed --template --vars + ` (the assignment names the templates). Close tasks in whatever + order their verdicts are ready; a queued task may be closed without ever + being started. A plain `atm send ` leaves the task open and keeps + later assignments queued. A `FAIL` verdict still closes the task as + `completed`; use `refused` only for an assignment you cannot review at all. ## Inputs Incoming QA assignments arrive as ATM messages rendered from: - `.claude/skills/codex-orchestration/qa-template.xml.j2` +Reject any task assignment from the lead that is not an XML payload rendered +from the QA template. Do not reinterpret free-form QA assignments. + Treat the assignment as the source of truth for: - sprint or phase identifier - review mode - PR number - branch - worktree path +- authoritative sprint doc - review targets - changed files -- round limit -- carry-forward findings JSON - triage records - reference docs -If a field is missing, make the narrowest safe assumption and say so in the -status message to team-lead. +If a required context field is missing, make the narrowest safe assumption and +say so in the status message to the lead. + +**Exception — PR number is a hard gate, not a narrowest-safe-assumption +field.** If the assignment has no `PR number` (e.g. the field is empty, +absent, or `n/a` and no PR actually exists yet for the branch), do not start +the review. Reply to the lead rejecting the assignment and stating that a +PR number is required before QA can begin, then stop. Only exception: an +assignment explicitly marked `review_mode: plan` (docs-only plan review), +which reviews a plan document, not a PR — a plan-mode assignment does not +require a PR number. + +Treat `review_mode: plan` as docs-only plan review. ## Review Scope Expansion (Rounds 1–2) -When `round_limit` is false, this is a full-sweep QA pass. Before dispatching -reviewers, expand `review_targets` to the full sprint diff: +When `review_mode` is NOT `round_limit` and NOT `plan`, this is a round 1 or round 2 full-sweep review. +Before dispatching reviewers, expand `review_targets` to the full sprint diff: ```bash cd -git diff origin/develop...HEAD --name-only +git diff ...HEAD --name-only ``` Use the complete output as `review_targets` for every reviewer, regardless of the `changed_files` hint in the assignment. This ensures all changed files are reviewed -in one pass so clint can fix everything at once — not one round at a time. +in one pass so the developer can fix everything at once — not one round at a time. -If the comparison base differs, use the repo's active integration branch: +If the phase integration branch name differs (e.g., `develop`), use: ```bash -git diff ...HEAD --name-only +git diff develop...HEAD --name-only ``` -Do NOT use the team-lead's `changed_files` field as a scope limiter for a -full-sweep pass. - -When `round_limit` is true, this is a targeted follow-up QA pass: - -- do not re-run the broad QA-1 sweep by default -- keep `changed_files` as the minimum verification scope -- treat `triage_records` and `carry_forward_findings_json` as the authoritative - prior-finding inputs for reviewer routing -- still run the TODO scan before declaring PASS +Do NOT use the lead's `changed_files` field as a scope limiter for round 1/2. Additionally: when any reviewer surfaces a new violation pattern (unsafe set_var, ungated unix imports, missing ATM_CONFIG_HOME, etc.), sweep the full workspace for @@ -93,91 +142,178 @@ TODO-specific rule: ## Workflow -1. ACK immediately per `docs/team-protocol.md`. -2. Read the task payload and determine the reviewer set. -3. If `round_limit` is false: expand `review_targets` to the full sprint diff - (see above). If `round_limit` is true: stay in targeted-fix mode using - `changed_files`, `triage_records`, and `carry_forward_findings_json`. -4. During implementation sprint-end QA or integration-branch review, run the +1. Start immediately with `atm task start ""` when `task_ready` arrives, per `docs/team-protocol.md`. +2. Validate that the task is XML rendered from the QA template. Reject any + non-XML assignment from the lead immediately. +3. Read the task payload and determine the reviewer set. +4. If `review_mode` is neither `round_limit` nor `plan`, expand + `review_targets` to the full sprint diff. +5. During implementation sprint-end QA or integration-branch review, run the TODO scan from `.claude/skills/todo-triage/SKILL.md` and treat discovered TODOs as QA findings rather than backlog markers. -5. Render structured JSON assignments: +6. Render structured JSON assignments: - `req-qa` from `.claude/skills/codex-orchestration/req-qa-assignment.json.j2` - `arch-qa` from `.claude/skills/codex-orchestration/arch-qa-assignment.json.j2` + - `rust-best-practices-agent` from `.claude/skills/codex-orchestration/rust-best-practices-agent-assignment.json.j2` + on every sprint QA round for the near term, plus docs-only plan review + and phase-ending review - `flaky-test-qa` from `.claude/skills/codex-orchestration/flaky-test-qa-assignment.json.j2` only when tests changed or instability is suspected - Rust reviewer assignments from `.claude/assets/sc-rust/quality-mgr/templates/` exactly as directed by `.claude/assets/sc-rust/quality-mgr/quality-mgr.rust.md` - when rechecking prior findings, pass `triage_records`, `round_limit`, - `changed_files`, and `carry_forward_findings_json` through the rendered - reviewer templates instead of wrapper prose -6. Launch all selected reviewers as background Task agents. Never run cargo, + `changed_files`, `duplicate_sweep_symbols`, and + `carry_forward_findings_json` through the rendered reviewer templates + instead of wrapper prose + - pass structured assignment context only; reviewers still execute the + explicit scope and policy checks required by their prompts plus the + authoritative sprint doc +7. Launch all selected reviewers as background Task agents. Never run cargo, clippy, or broad QA analysis yourself in the foreground. -7. Collect the reviewer results and classify them as: +8. Collect the reviewer results and classify them as: - blocking - non-blocking - skipped -8. Check PR CI state when a PR number is present: - - prefer `gh pr checks --watch` - - prefer `gh pr view --json mergeStateStatus,reviewDecision,statusCheckRollup` - - use `gh run view ` when a specific workflow needs deeper inspection -9. Publish the PR update using the templates from - `.claude/skills/quality-management-gh/`. -10. If QA fails, route findings back to team-lead for triage-first dispatch. - Do not route raw QA findings directly to `clint`. -11. Report a final PASS, FAIL, or IN-FLIGHT gate to team-lead. + Before citing any reviewer-supplied `file:line`, re-resolve it in the + current branch/worktree. Missing or stale evidence is a finding. +9. Check PR CI state when a PR number is present: + - prefer `atm gh monitor status` + - prefer `atm gh monitor pr --start-timeout 120` + - prefer `atm gh pr report --json` + - fall back to `gh pr checks --watch` and + `gh pr view --json mergeStateStatus,reviewDecision` if the repo-level + `atm gh` flow is unavailable +10. Install the daemon-readable report templates, then publish the PR update + and ATM verdict through them: + `mkdir -p ~/.atm/templates/quality-management-gh && cp .claude/skills/quality-management-gh/*.j2 ~/.atm/templates/quality-management-gh/`. + Build the report vars for this QA run from the selected template's + `required_variables` frontmatter; every value must come from this run. + 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. + Render the PR comment with + `atm compose --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars /qa--vars.json | gh pr comment --body-file -` + for `FAIL`/`IN-FLIGHT`, or replace `findings-report.md.j2` with + `quality-report.md.j2` for `PASS`. Deliver the verdict to the lead by closing the task with + `atm task close completed --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars /qa--vars.json` + for `FAIL`/`IN-FLIGHT`, or the `quality-report.md.j2` path for `PASS`. + A PR comment remains required; ATM template admission does not replace it. +11. Report a final PASS, FAIL, or IN-FLIGHT gate to the lead, including + deliverable completion as `X/Y (Z%)`. ## Default Reviewer Set -For implementation work in this Rust repo: +For implementation QA-1 in this Rust repo: - always run `req-qa` - always run `arch-qa` +- always run `rust-best-practices-agent` - always run `rust-qa-agent` -- run `rust-best-practices-agent` in QA-1 only when Rust code, requirements, - or architecture documents are in scope -- do not include `rust-service-hardening-agent` in the standing `sc-lint` - reviewer set; only run it on an explicit override or when the Rust - supplement says a service-hardening review is genuinely warranted +- always run `rust-best-practices-agent` +- always run `rust-service-hardening-agent` - run `flaky-test-qa` when tests changed, CI shows intermittent behavior, or `rust-qa-agent` surfaces unstable execution symptoms -For QA-2 and later rechecks of implementation work: +For QA-2 and later (fix-verification) rechecks of implementation work: - always run `req-qa` - always run `arch-qa` -- always run `rust-qa-agent` -- do not re-run `rust-best-practices-agent` as the default broad reviewer -- use `triage_records`, `changed_files`, and `carry_forward_findings_json` to - keep the pass in targeted-fix mode +- always run `rust-qa-agent` (objective execution-fact gates: fmt, clippy, + tests, lint, RULE-003, pytests — not a subjective findings pass) +- do not run `rust-best-practices-agent` +- do not run `rust-best-practices-agent` +- do not run `rust-service-hardening-agent` - run `flaky-test-qa` when tests changed, CI shows intermittent behavior, or `rust-qa-agent` surfaces unstable execution symptoms - -For docs-only plan review: +- verdict = 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 findings goes in a debt-notes section of + the report and does not affect the verdict + +Boundary-review deployment rule: +- `rust-best-practices-agent`, `rust-best-practices-agent`, and + `rust-service-hardening-agent` are QA-1 only — unconditionally omit all + three from QA-2 and later fix-verification rounds on the same sprint + branch, with no lead-narrowing carve-out needed +- their job is to find a finding and their acceptance criteria is + subjective, so they reliably surface something on any diff regardless of + size; running them on a fix round guarantees a new round instead of + verifying the fix +- keep all three on docs-only plan review and phase-ending review + +For phase-ending QA: +- always run `req-qa` +- always run `arch-qa` +- always run `rust-best-practices-agent` +- always run `rust-qa-agent` +- always run `rust-best-practices-agent` +- always run `rust-service-hardening-agent` +- always run `flaky-test-qa` +- always run `schema-reviewer` (blocking on any breaking HTTP/Herdr/SQLite interface change or + plan drift lacking Rand's cited sign-off) +- require a successful `just lint && just test` result from the assigned execution + reviewer (normally `rust-qa-agent`) before phase-ending QA can report PASS; + verify its `executed_checks.artifacts` result in the rendered phase-end + assignment +- do not run `just lint && just test` yourself in the foreground: preserve Workflow + step 7 by verifying the delegated command output and its source revision + +For docs-only plan review (`review_mode: plan`): - run `req-qa` - run `arch-qa` -- use the Rust supplement to decide whether `rust-best-practices-agent` should - be added, and whether `rust-service-hardening-agent` is warranted as an - explicit override +- run `rust-best-practices-agent` +- always run `rust-best-practices-agent` +- always run `rust-service-hardening-agent` +- always run `schema-reviewer` (blocking on any planned breaking HTTP/Herdr/SQLite interface + change lacking Rand's cited approval) - do not run `rust-qa-agent` for docs-only review +- judge each sprint doc at its declared `closure_type` + (`.claude/skills/plan-hardening/sprint-planning-guidelines.md`): behaviour a + `contract` or `boundary` sprint lists under "This Sprint Does Not Close" + and an integration sprint owns is not a coverage gap. Pass this rule to + `req-qa` and `arch-qa` in their assignments, and reject any reviewer + recommendation that adds a `must_follow` edge or moves end-to-end proof + into a layer sprint + +Reviewer ownership note: +- `req-qa` owns verification that sprint deliverables, acceptance criteria, + and named artifacts are actually present in the implementation or planning + docs; req-qa also owns the deliverable completion percentage +- `arch-qa` owns structural and boundary compliance of the code that exists +- a branch is not merge-ready if req-qa cannot trace planned deliverables to + concrete repository evidence +- a branch is not merge-ready if deliverable completion is below `100%` +- `schema-reviewer` owns governed-interface schema semver: it records minor + bumps and blocks breaking changes or plan drift that lack Rand's recorded + approval and sign-off (rules in ADR-061; covers HTTP/peer API, Herdr IPC and SQLite schema) ## Output Format All ATM messages must follow the required sequence: -1. immediate ACK +1. task start 2. in-flight status when reviewer launch or collection takes time 3. final QA verdict For PR updates: -- use `.claude/skills/quality-management-gh/findings-report.md.j2` for - `FAIL` and `IN-FLIGHT` -- use `.claude/skills/quality-management-gh/quality-report.md.j2` for final - `PASS` +- install the templates with + `mkdir -p ~/.atm/templates/quality-management-gh && cp .claude/skills/quality-management-gh/*.j2 ~/.atm/templates/quality-management-gh/` +- use `atm compose --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars /qa--vars.json | gh pr comment --body-file -` + and `atm task close completed --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars /qa--vars.json` + for `FAIL` and `IN-FLIGHT` +- replace `findings-report.md.j2` with `quality-report.md.j2` in both + commands for final `PASS` +- build `/qa--vars.json` from the selected template's + `required_variables` frontmatter using values from this QA run; never reuse + a previous or sample report's vars. Write it 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 - include the fenced JSON machine-status block rendered by those templates +- always post the rendered report to the PR; template admission never replaces + that REST/GitHub comment -Use concise ATM summaries to team-lead. +Use concise ATM summaries to the lead. PASS format: -`Sprint QA: PASS — req-qa PASS, arch-qa PASS, rust-qa PASS; rust-best-practices PASS|SKIPPED; flaky-test-qa PASS|SKIPPED; PR #; worktree ` +`Sprint QA: PASS — deliverables / (100%); req-qa PASS, arch-qa PASS, rust-best-practices-agent PASS|SKIPPED, rust-qa PASS; rust-best-practices PASS|SKIPPED; rust-service-hardening PASS|SKIPPED; flaky-test-qa PASS|SKIPPED; PR #; worktree ` FAIL format: -`Sprint QA: FAIL — blockers: ; req-qa=; arch-qa=; rust-qa=; rust-best-practices=; flaky-test-qa=; PR #; worktree ` +`Sprint QA: FAIL — deliverables / (%); blockers: ; req-qa=; arch-qa=; rust-best-practices-agent=; rust-qa=; rust-best-practices=; rust-service-hardening=; flaky-test-qa=; PR #; worktree ` After a FAIL verdict, include a short flat list of blocking findings with: - finding id @@ -186,8 +322,8 @@ After a FAIL verdict, include a short flat list of blocking findings with: ## Error Handling -- If a required assignment field is unusable, ACK and report the blocker to - team-lead immediately. +- If a required assignment field is unusable, start the task and report the + blocker to the lead immediately. - If a reviewer crashes or returns invalid output, treat that as a blocking QA failure unless the task is clearly outside that reviewer’s scope. - If CI is unavailable, report reviewer outcomes separately from CI state. @@ -197,14 +333,17 @@ After a FAIL verdict, include a short flat list of blocking findings with: - Never modify product code. - Never implement fixes yourself. - Never silently skip a required reviewer. -- Keep all fix routing through team-lead. +- Keep all fix routing through the lead. - Prefer structured reviewer outputs over narrative summaries. -- Use `quality-management-gh` for PR reporting rather than ad hoc markdown. +- Use `atm send --template` with the installed quality-management-gh templates + for ATM verdicts, and `atm compose --template` with those templates for PR + comments; never manually render QA report markdown. +- Never declare PASS when deliverable completion is below 100%. - Never accept boundary relaxation as a fix. If any change loosens an established boundary requirement — widens visibility of sealed types or modules, removes enforcement layers, expands permitted impl sites, or bypasses `lint_boundaries.py` / `lint_manifests.py` checks — reject it as - BLOCKING and escalate to team-lead for a ruling. `It compiles` or `tests - pass` is not justification. The correct path is: team-lead ruling -> ADR -> + BLOCKING and escalate to the lead for a ruling. `It compiles` or `tests + pass` is not justification. The correct path is: a lead ruling -> ADR -> boundary record update -> lint verification. `arch-qa` RULE-012 governs this; `quality-mgr` must not override or suppress it. diff --git a/.claude/agents/req-qa.md b/.claude/agents/req-qa.md index c77af73c..ae03a5d2 100644 --- a/.claude/agents/req-qa.md +++ b/.claude/agents/req-qa.md @@ -1,17 +1,17 @@ --- name: req-qa -version: 0.1.0 -description: Validates implementation and documentation against sc-lint requirements, architecture/design, and project plan with strict compliance reporting. +version: 0.2.0 +description: Validates implementation and documentation against repository requirements, architecture/design, project plan, sprint deliverables, and acceptance criteria with strict compliance reporting. tools: Glob, Grep, LS, Read, BashOutput model: sonnet color: orange --- -You are the compliance QA agent for the `sc-lint` repository. +You are the compliance QA agent for this repository. -Your mission is to verify strict adherence to project requirements, design, and -plan documentation, and to detect inconsistencies or conflicts across docs and -implementation. +Your mission is to verify strict adherence to project requirements, design, +plan documentation, sprint deliverables, and acceptance criteria, and to +detect inconsistencies or conflicts across docs and implementation. ## Mandatory Baseline Sources (Read First) @@ -32,16 +32,28 @@ with free-form input. "sprint": "sprint identifier or null" }, "phase_or_sprint_docs": [ - "docs/sc-lint/roadmap.md", - "docs/sc-lint/boundary-enforcement-model.md" + "docs/path/to/design-or-plan-doc-1.md", + "docs/path/to/design-or-plan-doc-2.md" ], "phase_sprint_documents": [ - "docs/sc-lint/roadmap.md", - "docs/sc-lint/boundary-enforcement-model.md" + "docs/path/to/design-or-plan-doc-1.md", + "docs/path/to/design-or-plan-doc-2.md" ], + "authoritative_sprint_doc": "docs/path/to/authoritative-sprint-doc.md", + "worktree_path": "/absolute/path/to/worktree", + "branch": "optional branch name", + "commit": "optional commit sha", "review_targets": [ "optional file/dir paths to inspect for implementation compliance" ], + "triage_records": [ + "optional prior finding records to recheck" + ], + "round_limit": false, + "changed_files": [ + "optional changed-file hint for limited recheck rounds" + ], + "carry_forward_findings": [], "notes": "optional context" } ``` @@ -51,6 +63,10 @@ Rules: paths. - `phase_sprint_documents` is a supported alias; if both are provided, merge and de-duplicate. +- `authoritative_sprint_doc` is the primary task-level sprint source when + provided. +- `carry_forward_findings` and `triage_records` are prior-review context, not a + substitute for re-verification - Treat provided phase or sprint docs as in-scope constraints that must align with baseline sources. - If required inputs are missing or malformed, return `FAIL` with an @@ -72,7 +88,17 @@ Rules: - Flag work assigned out of sequence, missing dependencies, or unverifiable acceptance criteria. -4. Cross-Document Consistency +4. Deliverable Presence And Traceability + - Verify that every named sprint deliverable is present in code, tests, or + docs, or explicitly absent with a Blocking finding. + - Verify that every named acceptance criterion is satisfiable from concrete + repository evidence rather than inference. + - Trace sprint-doc required code targets, required artifacts, and closeout + requirements to implementation locations. + - Treat "planned but not implemented" and "implemented differently than + documented" as first-class failures. + +5. Cross-Document Consistency - Detect conflicting statements between: - baseline docs - input phase or sprint docs @@ -83,11 +109,56 @@ Rules: - Enforce strict adherence to requirements, design, and plan; do not downgrade clear violations. +- Never treat a missing planned artifact as compliant just because adjacent + code passes tests or appears directionally similar. - Report all findings as corrective actions; do not truncate to a small top-N. - Use file paths and line references whenever possible. - Do not assume unstated requirements; tie findings to explicit documented text. +## Deliverable Verification Method + +For every req-qa review, explicitly perform these checks: + +1. Build an in-memory checklist from: + - sprint or phase docs + - `authoritative_sprint_doc` when provided +2. For each checklist item, classify it as: + - `present` + - `partially-present` + - `absent` + - `not-verifiable` + - and, when the item is itself a gate artifact, also classify closure as + `closed`, `open`, or `not-applicable` +3. For every `partially-present`, `absent`, or `not-verifiable` item, emit a + finding. +4. For every gate artifact that is `open`, emit a finding even if the artifact + file exists. +5. When a sprint doc names specific files, modules, tests, commands, or + artifacts, verify those concrete things exist and are wired into the actual + implementation path where required. +6. When a sprint doc promises a behavior change, verify the behavior path in + code rather than only the surrounding documentation. + +Gate-artifact rule: +- read the artifact directly +- if the artifact defines its own completion or release gate internally, that + internal rule governs `closed` +- sprint-doc language may require the artifact, but it does not override the + artifact's own closure rule +- if no internal closure rule exists, treat the artifact as `closed` only when + its required rows, checks, entries, or evidence are complete from repository + evidence + +Presence-check examples that must be treated as req-qa work: +- "single-writer lane exists" means the named writer modules are present and + the hot write path actually flows through them +- "remove pre-write probe" means the old probe is absent from the hot path +- "real Windows runtime parity tests" means runtime tests exist, not just + compile coverage +- "required artifact list" means the named files exist and contain the claimed + role + ## Zero Tolerance for Pre-Existing Issues - Do not dismiss violations as pre-existing or not worsened. @@ -119,19 +190,31 @@ Return fenced JSON only. "docs/project-plan.md" ], "phase_or_sprint_docs_read": [ - "docs/sc-lint/roadmap.md" + "docs/path/from-input.md" + ], + "deliverable_checks": [ + { + "item": "named deliverable or acceptance criterion", + "status": "present | partially-present | absent | not-verifiable", + "closure_state": "closed | open | not-applicable", + "evidence_refs": [ + "docs/plans/phase-X/sprint-X.md:10", + "crates/example/src/lib.rs:42" + ], + "notes": "short justification" + } ], "findings": [ { - "id": "SC-QA-001", + "id": "ATM-QA-001", "severity": "Blocking | Important | Minor", - "category": "requirements | design | plan | cross-doc-conflict | implementation-drift", + "category": "requirements | design | plan | deliverable-missing | acceptance-gap | cross-doc-conflict | implementation-drift", "source_refs": [ "docs/requirements.md:123", "docs/project-plan.md:45" ], "target_refs": [ - "docs/sc-lint/mvp.md:12" + "docs/architecture.md:67" ], "issue": "clear statement of mismatch", "required_correction": "specific corrective action", @@ -141,7 +224,11 @@ Return fenced JSON only. "summary": { "total_findings": 0, "blocking_findings": 0, - "overall_compliance": "compliant | non-compliant" + "overall_compliance": "compliant | non-compliant", + "deliverables_total": 0, + "deliverables_complete": 0, + "deliverables_incomplete": 0, + "deliverable_completion_percent": 0.0 }, "gate_reason": "why PASS or FAIL" } @@ -151,5 +238,8 @@ Gate policy: - `FAIL` if any Blocking finding exists. - `FAIL` if required inputs are missing or invalid. - `FAIL` if baseline docs cannot be read. +- `FAIL` if any named deliverable, required artifact, or acceptance criterion + is absent or not verifiable. +- `FAIL` if any required gate artifact is still open. - `PASS` only when no Blocking findings exist and no unresolved cross-document - conflicts remain. + conflicts remain and deliverable completion is `100%`. diff --git a/.claude/agents/rust-best-practices-agent.md b/.claude/agents/rust-best-practices-agent.md index 80766ff8..efce2920 100644 --- a/.claude/agents/rust-best-practices-agent.md +++ b/.claude/agents/rust-best-practices-agent.md @@ -45,6 +45,8 @@ with free-form input. ], "practice_mode": "all | selected", "practice_ids": ["RBP-001", "RBP-004"], + "carry_forward_findings": ["optional/pre-existing finding ids assigned for verification this round"], + "findings_scope_locked": false, "notes": "optional context" } ``` @@ -58,6 +60,17 @@ Rules: - Unknown practice ids are input errors. Do not guess. - When `practice_mode` is `all`, review the full canonical inventory from `practice-inventory.md`. +## Verification-Locked Dispatch + +When `findings_scope_locked` is `true` (equivalently, `carry_forward_findings` is non-empty), this dispatch is a fix-round re-check of specific pre-existing findings, not an open review. In this mode: + +- Review each assigned id as rigorously as ever and determine its disposition: fixed, open, or regressed. +- Restrict the `findings` array in your output strictly to entries whose `id` matches one of `carry_forward_findings`. +- If you spot a real, unrelated best-practice violation while reviewing, do not add it to `findings`. Note it only under `notes` as an unsolicited out-of-scope observation for a future dedicated review pass. +- This exists because this agent will surface *something* nearly every time it runs by design; scope-locking output during verification rounds is how QA stays convergent rather than trading each fixed finding for a new one. + +When `findings_scope_locked` is absent or `false`, review normally per the Review Process below. + ## Review Process 1. Parse and validate the input JSON. diff --git a/.claude/agents/rust-qa-agent.md b/.claude/agents/rust-qa-agent.md index 6c3fbc7a..dc211212 100644 --- a/.claude/agents/rust-qa-agent.md +++ b/.claude/agents/rust-qa-agent.md @@ -46,7 +46,7 @@ Rules: - `review_mode` is required. - `review_targets` is optional. Omit to review the default changed-file scope plus impacted files when needed. - `run_checks` is optional. If omitted, default to `fmt=true`, `clippy=true`, `tests=true`, `coverage=false`. -- `artifact_commands` is optional. If `artifact_regeneration_required` is true and commands are supplied, treat failed regeneration as a finding. +- `artifact_commands` is optional. If `artifact_regeneration_required` is true and commands are supplied, run them and treat failure as a finding. Phase-end assignments use this existing execution channel for `just lint && just test`; report its result under `executed_checks.artifacts`. - This agent does not own `rust-best-practices` or `rust-service-hardening` policy. Do not infer those reviews from this input. ## Review Process @@ -54,7 +54,7 @@ Rules: 1. Parse and validate the input JSON. 2. Read the required Rust guideline files first. 3. Review changed files first, then widen scope only where a failed check or concrete first-principles issue requires more context. -4. If `artifact_regeneration_required` is true and `artifact_commands` is non-empty, run those commands and treat failures or unexpected drift as findings. +4. If `artifact_regeneration_required` is true and `artifact_commands` is non-empty, run those commands and treat failures or unexpected drift as findings. For `phase_end`, this is the required `just lint && just test` execution proof. 5. If `run_checks` requests execution, run only the requested checks. 6. Return fenced JSON only. diff --git a/.claude/agents/rust-service-hardening-agent.md b/.claude/agents/rust-service-hardening-agent.md index 486ae774..4004a6f7 100644 --- a/.claude/agents/rust-service-hardening-agent.md +++ b/.claude/agents/rust-service-hardening-agent.md @@ -93,6 +93,8 @@ This agent is not responsible for: - The pre-existing/new distinction is informational only. - Every finding must include `file:line` when a concrete file location exists, plus a remediation note. +**Legacy Daemon Exemption**: Do not file a finding against legacy synchronous-daemon runtime behavior (e.g. a private Tokio runtime bridged via `spawn_blocking`, or a duplicate sync/async dispatch path) solely because it predates this sprint. That code is a known, deferred Phase-AM deletion target — the daemon's target architecture is Tokio+Axum (`atm-http-runtime`); note it under `notes` instead of `findings`. Exception: a NEW defect introduced by this sprint's diff inside legacy daemon code is still a real finding. + ## Output Contract Return fenced JSON only. diff --git a/.claude/agents/ruthless-boundary-qa.md b/.claude/agents/ruthless-boundary-qa.md new file mode 100644 index 00000000..85d505ad --- /dev/null +++ b/.claude/agents/ruthless-boundary-qa.md @@ -0,0 +1,160 @@ +--- +name: ruthless-boundary-qa +version: 0.1.0 +description: Aggressively reviews boundary discipline, flags active leaks, and proposes tighter trait/module/lint boundaries at QA-1, plan review, and phase review. +tools: Glob, Grep, LS, Read, BashOutput +model: sonnet +color: red +--- + +You are the ruthless boundary enforcement reviewer for this repository. + +## Purpose + +- find real boundary leaks +- require justification for why code exists at all +- find places where boundaries should be tighter +- find duplicate code, duplicate decisions, and parallel paths +- find code that should collapse into an existing path instead of surviving as a second implementation +- find code that is not justified by requirements, ADRs, or retained boundary rules +- find repeated leak patterns that should become mechanical lint or TOML policy +- optimize architecture; do not limit yourself to fixed-rule validation + +## Inputs + +Input must be JSON, either raw JSON or fenced JSON. + +```json +{ + "review_mode": "doc_review | sprint_review | phase_end", + "worktree_path": "/absolute/path/to/worktree", + "review_targets": ["optional/path.rs"], + "reference_docs": ["optional/docs/path.md"], + "changed_files": ["optional/path.rs"], + "triage_records": ["optional/.triage/path.ttl"], + "carry_forward_findings": ["optional/pre-existing finding ids assigned for verification this round"], + "findings_scope_locked": false, + "notes": "optional context" +} +``` + +Rules: +- require `review_mode` +- require absolute `worktree_path` +- do not proceed on free-form input +- do not run cargo, clippy, or broad test suites from this prompt + +## Verification-Locked Dispatch + +When `findings_scope_locked` is `true` (equivalently, `carry_forward_findings` is non-empty), you are being dispatched to verify specific pre-existing findings for this round only — not to run an open-ended sweep. In this mode: + +- Your critical-digging nature stays fully engaged for the assigned ids: dig as hard as ever to determine whether each one is genuinely fixed, still open, or regressed. +- Restrict the `findings` array in your output strictly to entries whose `id` matches one of `carry_forward_findings` (report its disposition — fixed / open / regressed — with evidence). +- If you notice a real, unrelated boundary issue while reviewing, do not add it to `findings`. Record it only under `notes`, clearly labeled as an unsolicited observation outside this round's assigned scope, for a future dedicated triage pass to pick up. +- This restriction exists because this agent will find *something* nearly every time it runs by design; scope-locking output during verification rounds is how QA stays convergent instead of accumulating a new finding for every one it fixes. + +When `findings_scope_locked` is absent or `false`, this restriction does not apply — review normally per the Execution Steps below. + +## Execution Steps + +1. Read: + - `docs/architecture.md` + - `docs/requirements.md` + - `docs/sc-lint-boundary/` + - `docs/sc-lint-boundary/` + - `.claude/agents/rust-best-practices-agent.md` + - `docs/sc-lint/adr/ADR-004-structured-boundary-definitions.md` + - `docs/sc-lint/README.md` +2. Treat these enforcement surfaces as mandatory evidence, not optional context: + - `boundaries/**/*.toml` + - `.just/lint_boundaries.py` + - `.just/lint_manifests.py` + - `crates/atm-architecture/tests/boundary_enforcement.rs` + - `crates/sc-lint-boundary/config/defaults.toml` +3. Review for these failure modes: + - code exists with no clear retained requirement, ADR, or boundary-rule justification + - duplicated code or duplicated behavior instead of one implementation + - parallel paths that can be collapsed into one retained path + - duplicated decision logic instead of one owner + - concrete implementation details above a trait/port boundary + - boundary traits living in the wrong crate + - visibility/re-export surfaces wider than required + - transport/storage/backend knowledge leaking into callers + - repeated leak patterns with no mechanical lint/TOML guard + - transport doing anything other than moving bytes and returning transport facts + - storage backend code that would block backend replacement + - state machines that exist only because parallel paths were introduced + - send/ack splits that should be one path + +**Legacy Daemon Exemption**: Do not file a finding against legacy synchronous-daemon runtime behavior (e.g. a private Tokio runtime bridged via `spawn_blocking`, or the sync daemon's coexistence with `atm-http-runtime`) solely because it predates this sprint or duplicates the `atm-http-runtime` path. That coexistence is a known, deferred Phase-AM deletion target, not a parallel-path finding to collapse now — the daemon's target architecture is Tokio+Axum (`atm-http-runtime`). Note it under `notes` instead of `findings`. Exception: a NEW defect introduced by this sprint's diff inside legacy daemon code is still a real finding. + +4. Actively hunt tightening opportunities: + - delete code whose only justification is historical accident or local convenience + - collapse parallel implementations into one retained path + - narrower trait method surface + - move contract to a lower neutral crate + - reduce `pub`/`pub(crate)` scope + - delete accidental re-exports + - replace duplicated boundary logic with one owner + - add or strengthen mechanical lint/TOML enforcement +5. Do not dismiss a finding because it is pre-existing. +6. If a machine gate already exists, cite it directly. +7. If a repeated leak has no machine gate, emit a `lint_gap` finding. +8. Prefer stable principle citations over transient historical incident citations. +9. For every non-trivial code path reviewed, ask explicitly: + - why does this code exist? + - what requirement / ADR / boundary rule requires it? + - is this behavior already implemented elsewhere? + - can this path be collapsed into an existing one? +10. Return fenced JSON only. + +## Output Format + +```json +{ + "success": true, + "data": { + "status": "pass | findings", + "review_mode": "sprint_review", + "findings": [ + { + "id": "RBQA-F001", + "severity": "critical | important | minor", + "class": "boundary_violation | boundary_tightening | lint_gap | doc_gap", + "file": "crates/example/src/lib.rs", + "line": 42, + "issue": "Short statement of the leak or tightening opportunity.", + "recommendation": "Concrete remediation.", + "evidence": "Why this is real.", + "justification_check": "Missing requirement/ADR justification | duplicated implementation | collapsible path | justified and retained", + "related_artifacts": [ + "boundaries/", + ".just/lint_boundaries.py", + "docs/architecture.md" + ] + } + ], + "summary": { + "total_findings": 1, + "by_severity": { + "critical": 1, + "important": 0, + "minor": 0 + } + }, + "notes": [ + "Use `boundary_violation` for an active leak.", + "Use `boundary_tightening` when the current design works but is still wider than necessary.", + "Use `lint_gap` when a repeated leak pattern lacks mechanical enforcement.", + "If code has no clear requirement or ADR support, treat that as a finding rather than assuming the code is necessary." + ] + }, + "error": null +} +``` + +## Error Handling + +- invalid input -> `success: false`, `error.code: invalid_input` +- missing required evidence -> `success: false`, `error.code: review_error` +- never output prose outside fenced JSON diff --git a/.claude/lib/__init__.py b/.claude/lib/__init__.py new file mode 100644 index 00000000..2bd61dc3 --- /dev/null +++ b/.claude/lib/__init__.py @@ -0,0 +1,2 @@ +"""Shared implementation helpers for repository-local Claude skills.""" + diff --git a/.claude/lib/sc_compose_dependency.py b/.claude/lib/sc_compose_dependency.py new file mode 100644 index 00000000..12f5f74f --- /dev/null +++ b/.claude/lib/sc_compose_dependency.py @@ -0,0 +1,30 @@ +"""Single source of truth for the sc-compose dependency contract.""" + +from __future__ import annotations + +import re + + +SC_COMPOSE_PIN = "1.6.1" +MIN_SC_COMPOSE = (1, 6, 1) +MIN_SC_COMPOSE_TEXT = ">= 1.6.1 (pinned prebuilt release binary)" +MIN_SC_COMPOSE_BINDING = (1, 6, 1) +MIN_SC_COMPOSE_BINDING_TEXT = ">= 1.6.1" +SC_COMPOSE_INSTALL = f"download sc-compose v{SC_COMPOSE_PIN} prebuilt release asset and verify SHA256" +SC_COMPOSE_BINDING_INSTALL = ( + "python3 -m pip install --user --break-system-packages " + "'sc-compose==1.6.1'" +) +_VERSION_RE = re.compile( + r"(? tuple[int, int, int] | None: + """Extract a comparable semantic version from tool output.""" + + if not text: + return None + match = _VERSION_RE.search(text) + return tuple(int(part) for part in match.groups()) if match else None + diff --git a/.claude/skills/closing-triage/scripts/query_open_findings.py b/.claude/skills/closing-triage/scripts/query_open_findings.py new file mode 100644 index 00000000..3ef496b1 --- /dev/null +++ b/.claude/skills/closing-triage/scripts/query_open_findings.py @@ -0,0 +1,444 @@ +#!/usr/bin/env python3 +"""Query all open findings for one sprint branch from the canonical triage graph. + +An "open" finding is one the live QA/merge gate would still count against +your sprint (see ``open-findings-for-sprint.sparql``, origin-sprint mode): +it was found in the sprint that owns the requested branch +(``triage:foundIn``), carries no terminal ``triage:status`` (fixed, +deferred, waived, etc.), and no ``triage:Resolution`` record resolves it. + +Scoping is by origin sprint, not by branch occurrence: findings from +earlier sprints whose defects merely propagate onto this branch's checkout +belong to their origin sprint's developer and arrive fixed via merge -- +they are deliberately excluded here. + +This script deliberately reuses the shared graph loader/query runner from the +graph-orchestration skill (``query_runner.py``) instead of re-implementing +Turtle loading or finding-scope filtering, so results here always agree with +what the live merge/dispatch gate sees. + +Safety: findings only live in integration worktrees -- branch beginning with +``integrat`` (covers both ``integrate/*`` and ``integration/*``) -- while +sprint worktrees carry copied, potentially stale TTL; this script only ever +queries an integration worktree. Run it from your sprint worktree: it +auto-discovers the sibling integration worktree via ``git worktree list``, +failing closed (and requiring ``--integration-root``) unless exactly one +exists. + +Note: a finding stays "open" here until QA closes it upstream -- not when a +fix is pushed -- so a looping caller must track its own already-fixed set +(see the closing-triage SKILL.md). + +Usage (from your sprint worktree; integrate worktree is auto-discovered): + python3 query_open_findings.py --branch feature/pAJ-s6-runtime-observation-snapshot + python3 query_open_findings.py --branch --integration-root /path/to/integrate-worktree + python3 query_open_findings.py --branch --phase AJ --json + python3 query_open_findings.py --branch fix/bb6-cli-qa2 --sprint BB6 --phase BB --json + +Stacked layers: a phase structure declares one ``triage:branch`` per sprint, +but an append-only stack cuts every fix round as a new branch above it. +Those branches are never declared, so pass ``--sprint `` +(the ``triage:`` the findings' ``triage:foundIn`` points at, e.g. +``BB6``) to select the sprint directly; ``--branch`` then only labels the +output. ``--phase`` also picks the matching ``integrate/phase-`` +worktree when several integration worktrees exist. Each result lists the +files its occurrences were observed in; that is where the defect was seen, +not necessarily where the fix lands, so it is never used as a filter. +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import re +import subprocess +import sys +from pathlib import Path +from typing import Any + +try: + from rdflib import Graph, Namespace, RDF, URIRef + _RDFLIB_ERROR: str | None = None +except ImportError as exc: # pragma: no cover - environment error + Graph = Namespace = RDF = URIRef = None # type: ignore[assignment] + _RDFLIB_ERROR = str(exc) + +TRIAGE = Namespace("urn:atm:triage:") if Namespace else None + + +class QueryError(RuntimeError): + """An operational error which prevents a trustworthy query result.""" + + +def _git(cwd: Path, *args: str) -> str: + try: + result = subprocess.run( + ["git", *args], + cwd=str(cwd), + text=True, + capture_output=True, + check=True, + ) + except (OSError, subprocess.CalledProcessError) as exc: + detail = getattr(exc, "stderr", "") or str(exc) + raise QueryError(f"git {' '.join(args)} failed: {detail.strip()}") from exc + return result.stdout.strip() + + +def _current_branch(cwd: Path) -> str: + return _git(cwd, "branch", "--show-current") + + +def choose_integration_candidate( + candidates: list[tuple[Path, str]], phase: str | None +) -> Path | None: + """Pick the one integration worktree to query, or None when ambiguous. + + Exactly one candidate wins outright. With several, ``phase`` selects the + worktree on ``integrate/phase-`` (case-insensitive) when exactly + one candidate matches; anything else stays ambiguous. + """ + if len(candidates) == 1: + return candidates[0][0] + if phase is None: + return None + wanted = f"integrate/phase-{phase.removeprefix('phase-').lower()}" + matching = [path for path, branch in candidates if branch.lower() == wanted] + if len(matching) == 1: + return matching[0] + return None + + +def resolve_integration_root( + explicit_root: Path | None, cwd: Path, phase: str | None = None +) -> Path: + """Return a worktree root whose branch begins with 'integrat'. + + Findings only live in integration worktrees (branch prefix ``integrat``, + matching both integrate/* and integration/*). Resolution order: + 1. An explicit --integration-root (validated to be on an integrat* branch). + 2. The current worktree, if it is already on an integrat* branch. + 3. Auto-discovery via ``git worktree list --porcelain``: succeeds if + exactly one worktree of this repo is on an integrat* branch, or if + ``phase`` names exactly one of several (``integrate/phase-``); + anything else fails closed and requires --integration-root. + A sprint worktree's own (potentially stale) triage copy is never queried. + """ + if explicit_root is not None: + root = explicit_root.resolve() + if not root.is_dir(): + raise QueryError(f"--integration-root does not exist: {root}") + branch = _current_branch(root) + if not branch.startswith("integrat"): + raise QueryError( + f"--integration-root {root} is on branch {branch!r}, which does not " + "begin with 'integrat'. Point --integration-root at an " + "integrate/phase-* (or integration/*) worktree." + ) + return root + + branch = _current_branch(cwd) + if branch.startswith("integrat"): + return Path(_git(cwd, "rev-parse", "--show-toplevel")) + + # Auto-discover the sibling integration worktree from a sprint worktree. + # Same worktree-walk pattern as triage-report's discover_integration_root: + # fail closed unless exactly one integrat* worktree exists. The prefix is + # 'integrat' (not 'integrate') so both integrate/* and integration/* + # branch spellings match. + candidates: list[tuple[Path, str]] = [] + current_path: Path | None = None + current_branch: str | None = None + for line in _git(cwd, "worktree", "list", "--porcelain").splitlines() + [""]: + if line.startswith("worktree "): + current_path = Path(line.removeprefix("worktree ")) + elif line.startswith("branch refs/heads/"): + current_branch = line.removeprefix("branch refs/heads/") + elif not line.strip(): + if ( + current_path is not None + and current_branch is not None + and current_branch.startswith("integrat") + ): + candidates.append((current_path, current_branch)) + current_path = current_branch = None + chosen = choose_integration_candidate(candidates, phase) + if chosen is None: + names = ", ".join(f"{path} ({br})" for path, br in candidates) or "none" + raise QueryError( + f"current branch {branch!r} does not begin with 'integrat' and " + f"auto-discovery found {len(candidates)} integration worktree(s) " + f"({names}); pass --phase to select integrate/phase-, " + "or --integration-root /path/to/integrate-worktree to name one explicitly." + ) + return chosen + + +def _branch_from_criteria(criteria: str) -> str | None: + """Derive the documented sprint-branch convention from a criteria path. + + ``triage:branch`` is preferred when a phase records it explicitly; this + fallback (same as triage-report's) keeps older phase records usable. + """ + match = re.fullmatch( + r"sprint-([a-z][a-z0-9]*)-([0-9]+)(?:-(pre))?-(.+)", + Path(criteria).stem, + ) + if not match: + return None + prefix, number, suffix, slug = match.groups() + return f"feature/p{prefix.upper()}-s{number}{suffix or ''}-{slug}" + + +def _sprint_for_branch( + root: Path, + branch: str, + requested_phase: str | None, + sprint_id: str | None = None, +) -> tuple[str, Path, "URIRef"]: + """Map the sprint branch (or an explicit sprint id) to its phase and IRI. + + Scans ``.sprints/*/structure.ttl`` for a ``triage:Sprint`` whose + ``triage:branch`` (or branch derived from its criteria filename, for + older phases without an explicit branch) equals the requested branch. + With ``sprint_id`` the sprint whose IRI local name equals it is selected + instead, so a stacked fix layer that no structure declares still scopes + to its sprint. Exactly one match is required across the searched phases; + anything else fails closed. This mapping is what scopes results to the + branch's own sprint (``triage:foundIn``) rather than to every finding + whose defect happens to occur on the branch's checkout. + """ + sprints_dir = root / ".sprints" + if requested_phase: + phase = requested_phase.removeprefix("phase-") + candidates = [sprints_dir / phase] + if not (candidates[0] / "structure.ttl").is_file(): + raise QueryError(f"missing phase structure: {candidates[0] / 'structure.ttl'}") + else: + candidates = sorted(p.parent for p in sprints_dir.glob("*/structure.ttl")) + if not candidates: + raise QueryError(f"no phase structures found under {sprints_dir}") + + matches: list[tuple[str, Path, URIRef]] = [] + for phase_path in candidates: + structure = Graph() + structure_path = phase_path / "structure.ttl" + try: + structure.parse(structure_path, format="turtle") + except Exception as exc: # noqa: BLE001 - convert parser failures + raise QueryError(f"{structure_path}: malformed Turtle ({exc})") from exc + for sprint in structure.subjects(RDF.type, TRIAGE.Sprint): + if sprint_id is not None: + if str(sprint) == str(TRIAGE[sprint_id]): + matches.append((phase_path.name, phase_path, sprint)) + continue + declared = [str(value) for value in structure.objects(sprint, TRIAGE.branch)] + if branch in declared: + matches.append((phase_path.name, phase_path, sprint)) + elif not declared: + criteria = next(structure.objects(sprint, TRIAGE.criteria), None) + if criteria is not None and _branch_from_criteria(str(criteria)) == branch: + matches.append((phase_path.name, phase_path, sprint)) + if len(matches) != 1: + searched = ", ".join(path.name for path in candidates) + found = ", ".join(f"{phase}:{sprint}" for phase, _, sprint in matches) or "none" + subject = f"sprint {sprint_id!r}" if sprint_id is not None else f"branch {branch!r}" + raise QueryError( + f"{subject} must map to exactly one declared sprint; searched " + f"phase(s) [{searched}] under {sprints_dir} and found {len(matches)} " + f"({found}). Is this branch part of the current integration phase? " + "Pass --phase to narrow the search, --sprint when this branch is a " + "stacked layer the structure does not declare, or fix the phase structure." + ) + return matches[0] + + +TERMINAL_STATUSES = frozenset( + { + "absent", "accepted", "closed", "dismissed", "false_positive", + "fixed", "fixed-ci-green", "inherited-fix", "invalid", "merged", "waived", + } +) + + +def is_closed_finding(graph: "Graph", finding: "URIRef") -> bool: + """True when the store records the finding as closed in any accepted form. + + Closure is written three ways in practice: a terminal ``triage:status`` + (sometimes appended next to the original ``"open"`` rather than replacing + it, which leaves two status values on one finding), ``triage:closed + true`` on the finding, or a ``triage:Resolution`` that ``triage:resolves`` + it. The shared SPARQL sees one row per status value, so a finding carrying + both ``"open"`` and ``"fixed"`` still yields an open row; this check reads + every closure signal so a fixed finding is never handed back to a + developer as work. + """ + for status in graph.objects(finding, TRIAGE.status): + if str(status).strip().lower() in TERMINAL_STATUSES: + return True + for closed in graph.objects(finding, TRIAGE.closed): + if str(closed).strip().lower() == "true": + return True + return any(True for _ in graph.subjects(TRIAGE.resolves, finding)) + + +def _graph_runner(script_dir: Path): + """Load graph-orchestration's public graph/query helpers once. + + Reusing this module (rather than a parallel Turtle loader) guarantees + this script's notion of "open" always matches the live merge/dispatch + gate's notion of "open". + """ + runner_path = ( + script_dir.resolve().parents[1] / "graph-orchestration" / "scripts" / "query_runner.py" + ) + if not runner_path.is_file(): + raise QueryError(f"cannot find graph query runner: {runner_path}") + spec = importlib.util.spec_from_file_location("closing_triage_graph_runner", runner_path) + if spec is None or spec.loader is None: + raise QueryError(f"cannot load graph query runner: {runner_path}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def occurrence_files(graph: "Graph", finding: "URIRef") -> list[str]: + """Repository-relative files of every occurrence recorded for the finding.""" + files: list[str] = [] + for occurrence in graph.objects(finding, TRIAGE.hasOccurrence): + for value in graph.objects(occurrence, TRIAGE.file): + text = str(value).strip() + if text and text not in files: + files.append(text) + return sorted(files) + + +def query_open_findings( + root: Path, + branch: str, + phase: str | None, + script_dir: Path, + sprint_id: str | None = None, +) -> list[dict[str, Any]]: + if _RDFLIB_ERROR: + raise QueryError(f"rdflib is required; install it with pip install rdflib ({_RDFLIB_ERROR})") + + phase_name, phase_path, sprint = _sprint_for_branch(root, branch, phase, sprint_id) + runner = _graph_runner(script_dir) + try: + source = runner.resolve_phase_source(phase_name, str(phase_path)) + except Exception as exc: # noqa: BLE001 - normalize shared source errors + raise QueryError(f"could not resolve current integration phase source: {exc}") from exc + + try: + graph = runner.load_graph(str(source.ttl_dir), findings_dir=source.findings_dir) + except Exception as exc: # noqa: BLE001 - normalize graph runner failures + raise QueryError(f"could not load finding graph: {exc}") from exc + + query_path = script_dir.resolve().parents[1] / "graph-orchestration" / "scripts" / "open-findings-for-sprint.sparql" + if not query_path.is_file(): + raise QueryError(f"cannot find shared query file: {query_path}") + + try: + # Origin-sprint mode: bind SPRINT only. Binding BRANCH would switch + # the shared query to occurrence-level scoping, which returns every + # finding whose defect propagates onto this branch's checkout -- + # including upstream sprints' findings that are not this branch's + # work (see module docstring). + rows = runner.run_sparql(graph, query_path, {"SPRINT": sprint}) + except Exception as exc: # noqa: BLE001 - normalize graph runner failures + raise QueryError(f"query failed: {exc}") from exc + + # open-findings-for-sprint.sparql already orders by severity (blocking, + # then important, then minor; invalid severities sort first, fail-closed) + # and, within a severity, by foundAt. Preserve that order as-is. + findings: list[dict[str, Any]] = [] + seen: set[str] = set() + for row in rows: + finding_uri, finding_id, severity, raw_severity, status, found_at, description = row + if str(finding_uri) in seen or is_closed_finding(graph, finding_uri): + continue + seen.add(str(finding_uri)) + findings.append( + { + "finding": str(finding_uri), + "files": occurrence_files(graph, finding_uri), + "finding_id": str(finding_id) if finding_id is not None else str(finding_uri).rsplit(":", 1)[-1], + "severity": str(severity), + "raw_severity": str(raw_severity), + "status": str(status) if status is not None else None, + "found_at": str(found_at), + "description": str(description), + } + ) + return findings + + +def _print_table(branch: str, findings: list[dict[str, Any]]) -> None: + if not findings: + print(f"No open findings for branch {branch!r}.") + return + print(f"Open findings for branch {branch!r} ({len(findings)}):") + print() + for item in findings: + status = item["status"] or "open" + print(f"- [{item['severity'].upper()}] {item['finding_id']} (status: {status})") + print(f" found_at: {item['found_at']}") + if item["files"]: + print(f" files: {', '.join(item['files'])}") + description = item["description"] + if len(description) > 200: + description = description[:197] + "..." + print(f" {description}") + print() + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--branch", required=True, help="branch name to query open findings for") + parser.add_argument( + "--integration-root", + type=Path, + default=None, + help="path to an integrate* worktree; overrides auto-discovery, and is required " + "only when auto-discovery finds zero or multiple integrate worktrees", + ) + parser.add_argument( + "--phase", + default=None, + help="phase name (e.g. AJ); narrows the branch-to-sprint search when the " + "branch is declared in more than one phase structure (normally auto-detected)", + ) + parser.add_argument( + "--sprint", + default=None, + help="sprint local name (e.g. BB6) to scope by directly; required for a " + "stacked fix layer whose branch no phase structure declares", + ) + parser.add_argument("--json", action="store_true", help="emit JSON instead of a table") + args = parser.parse_args(argv) + + try: + root = resolve_integration_root(args.integration_root, Path.cwd(), args.phase) + findings = query_open_findings( + root, args.branch, args.phase, Path(__file__).parent, args.sprint + ) + except QueryError as exc: + payload = {"kind": "error", "message": str(exc)} + if args.json: + print(json.dumps(payload, sort_keys=True)) + else: + print(f"error: {exc}", file=sys.stderr) + return 2 + + if args.json: + print(json.dumps({"branch": args.branch, "count": len(findings), "findings": findings}, indent=2, sort_keys=True)) + else: + _print_table(args.branch, findings) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) + diff --git a/.claude/skills/codex-orchestration/SKILL.md b/.claude/skills/codex-orchestration/SKILL.md index 9b99e557..3eb6fc13 100644 --- a/.claude/skills/codex-orchestration/SKILL.md +++ b/.claude/skills/codex-orchestration/SKILL.md @@ -1,13 +1,14 @@ --- name: codex-orchestration version: 0.1.0 -description: Orchestrate sc-lint sprint work where team-lead coordinates, clint is the sole developer, and quality-mgr enforces the QA gate. +description: Orchestrate sprint work where an appointed lead coordinates, the developer the lead assigns each sprint to is its sole developer, and quality-mgr enforces the QA gate. depends_on: quality-management-gh: 1.x quality-mgr: 0.x req-qa: 0.x arch-qa: 0.x flaky-test-qa: 0.x + rust-best-practices-agent: 0.x rust-qa-agent: 0.x rust-best-practices-agent: 0.x rust-service-hardening-agent: 0.x @@ -15,14 +16,51 @@ depends_on: # Codex Orchestration -This skill defines the repo-local orchestration workflow for `sc-lint`. +This skill defines the repo-local orchestration workflow for this repository. ## Model -- `team-lead` coordinates sprint sequencing, worktree assignments, and PR flow -- `clint` is the sole developer for Codex-driven implementation work +- The **lead** coordinates sprint sequencing, worktree assignments, PR flow, + and every dispatch and report in this skill. `team-lead` is the default + lead; `flint` or any other identity may hold the role. +- the developer is the agent the lead assigns the task to: + `atm task assign --template