diff --git a/docs/rag/architecture/config-reference.md b/docs/rag/architecture/config-reference.md index 73a1f083..b507ab5d 100644 --- a/docs/rag/architecture/config-reference.md +++ b/docs/rag/architecture/config-reference.md @@ -189,6 +189,14 @@ Weekly, the Product Owner explores the company's projects and proposes a themed No dedicated migration — a cycle is marker-backed (`orchestration_markers` on the held exploration task), not a new table. +## Possibilities Matrix + +The work-already-done fast path on `i_am_done`. Default-off, transparent to the dev (no verb change, no opt-in call). See `docs/rag/architecture/possibilities-matrix.md`. + +| Variable | Default | Description | +|----------|---------|-------------| +| `ROBOCO_POSSIBILITIES_MATRIX_ENABLED` | `false` | Master switch. Off = `i_am_done` behaves exactly as it always has. On = when a task already has commits, an open PR, every AC addressed, and no open findings, `i_am_done` submits straight to `awaiting_qa`, trusting the PR's own CI-green signal (falling back to the local `make quality` gate when there's no CI signal, and refusing outright on a known-red CI). | + ## Cloud Auth **Not a panel feature flag** — unlike the flags above, `ROBOCO_CLOUD_AUTH_ENABLED` is env-only (deliberately absent from `roboco/services/settings.py`'s `FEATURE_FLAGS`, so it can't be flipped on for a deployment that isn't behind TLS). Lets the panel/API be exposed beyond localhost without changing the CEO's local no-login flow while off. See `docs/rag/architecture/cloud-auth.md`. diff --git a/docs/rag/architecture/possibilities-matrix.md b/docs/rag/architecture/possibilities-matrix.md new file mode 100644 index 00000000..a9201be9 --- /dev/null +++ b/docs/rag/architecture/possibilities-matrix.md @@ -0,0 +1,57 @@ +# Possibilities Matrix (work-already-done fast path) + +A fast path on `i_am_done`: when a dev's work already looks done, the choreographer submits straight to `awaiting_qa` in one call instead of walking the standard verify/journal turn. Gated by `ROBOCO_POSSIBILITIES_MATRIX_ENABLED` (default off — panel-toggleable, Settings → Feature Flags). Fully inert when off: `i_am_done` behaves exactly as it always has. + +## You don't opt in — it's transparent + +There is no separate verb and no argument that turns this on. A developer always just calls: + +```python +i_am_done(task_id="", notes="...", resolved_findings=None) +``` + +`_maybe_i_am_done_fast_path` runs on **every** `i_am_done` call when the flag is armed, checks whether the task already looks done, and silently takes the fast path when it does. Nothing about how you call `i_am_done` changes — you never need to know whether the fast path fired. + +The orchestrator's dev spawn prompt sometimes steers a freshly spawned dev straight to a `WORK_ALREADY_DONE` state (when the flag is on and the task already has an open PR + commits at spawn time) that tells you to call `i_am_done` directly rather than re-deriving what's already done — this is a turn-saving nudge for that specific spawn-timing case, not a requirement. A mid-session dev who never saw that prompt still gets the fast path the moment it calls plain `i_am_done`. + +## What "already looks done" means + +`_work_appears_done` — all of the following, checked against the live task: + +- Status is `claimed`, `in_progress`, or `verifying` +- At least one commit exists +- A PR is open (`pr_created` or `pr_number` set) +- Every acceptance criterion is addressed (each AC has a recorded artifact reference) +- No open finding remains on the revision-findings ledger for this task + +Ownership (`assigned_to == you`) is checked separately before the fast path is even considered. + +## What still runs — nothing is skipped that matters + +The fast path is not a bypass of the non-negotiable guards, only of the standard multi-turn derivation: + +1. Substantive-notes check +2. Video render-preview check, on a `source=video` task (`Requirement.RENDER_VERIFIED`) +3. `resolved_findings` applied, if given +4. Commits/PR field gates (`NO_COMMITS` / `NO_PR`) +5. Branch pushed +6. Not behind base +7. Architectural-conventions gate, if enabled +8. Every open finding re-checked — `FINDINGS_ADDRESSED` still blocks a resubmit that left one unnamed +9. The quality verdict (below) + +What IS skipped versus the standard path: the retroactive rich-plan derivation, and the journal progress/reflect tracing gates. + +## The quality verdict — CI trusted, local gate as fallback + +`_fast_path_quality_verdict` resolves the assembled PR's own CI status the same way `pr_pass` does: + +- **CI green** → trusted outright; no local gate runs at all. +- **CI red** → the fast path refuses outright: "fast path refused — PR CI is failing; QA reviews working code, not a red build." Fix CI (or route through the standard path) — the fast path will not ship a known-red build to QA. +- **No CI signal at all** (not configured, pending, or unresolvable) → falls back to the local `make quality`-style gate (plus the toolchain-match guard, when `ROBOCO_TOOLCHAIN_MATCH_ENABLED` is also armed). + +## See also + +- `docs/rag/roles/developer.md` — the fast path from the dev's seat +- `docs/rag/roles/pr-reviewer.md` / `docs/rag/architecture/review-findings.md` — the same CI-green trust `pr_pass` applies +- `CLAUDE.md` "Possibilities matrix" — the canonical feature summary diff --git a/docs/rag/architecture/review-findings.md b/docs/rag/architecture/review-findings.md index a1256d65..ba218433 100644 --- a/docs/rag/architecture/review-findings.md +++ b/docs/rag/architecture/review-findings.md @@ -63,7 +63,11 @@ i_am_done( ## Verification (the reviewer's side) -When the SAME origin's review passes on a later round, every `addressed` finding of that origin is bulk-promoted to `verified` in the same transaction — `pass` (QA) verifies `qa`-origin findings, `pr_pass` verifies `pr_gate`-origin, `complete` (PM) verifies `pm`-origin. `ceo_approve` does the same for `ceo`-origin findings, best-effort. A finding can also be `waived` (the repository supports it) but no verb currently calls that path — an unaddressed finding cannot yet be dismissed without actually resolving it. +When the SAME origin's review passes on a later round, every `addressed` finding of that origin is bulk-promoted to `verified` in the same transaction — `pass` (QA) verifies `qa`-origin findings, `pr_pass` verifies `pr_gate`-origin, `complete` (PM) verifies `pm`-origin. `ceo_approve` does the same for `ceo`-origin findings, best-effort. + +## Waiving a finding (Auditor only) + +A finding can also be `waived` instead of fixed — but only by the Auditor, and only for non-blocking severity. `waive_finding(finding_id, note)` is a flow verb on the Auditor's manifest, severity-scoped: `blocker`/`major` findings are refused outright ("must be fixed, never waived"); only `minor`/`nit` findings still `open` are eligible, and a non-empty `note` explaining why is required. The ledger row moves `open -> waived` (no task status change) and a `task.finding_waived` audit event records the decision. See `docs/rag/roles/auditor.md`. ## `ceo_reject` specifically @@ -80,6 +84,7 @@ The CEO acts through the panel, not a gateway verb — there is no agent-facing - `docs/rag/roles/cell-pm.md` / `docs/rag/roles/main-pm.md` — `request_changes` in practice - `docs/rag/roles/developer.md` — resolving a bounce with `resolved_findings` - `docs/rag/roles/ceo.md` — `ceo_reject` +- `docs/rag/roles/auditor.md` — `waive_finding` - `docs/rag/lifecycle/intent-verbs.md` — the canonical verb reference - `docs/rag/standards/conventions.md` — the unrelated `convention_findings` concept - `docs/map/review-findings.md` — the implementation map (code-facing, not agent-facing) diff --git a/docs/rag/architecture/video-engine.md b/docs/rag/architecture/video-engine.md index 0494442e..927b5f03 100644 --- a/docs/rag/architecture/video-engine.md +++ b/docs/rag/architecture/video-engine.md @@ -26,7 +26,7 @@ All three open a normal, **assigned** UX/UI authoring task (balanced across the ## Artifact verification (request_render) -Authoring is gated on the RENDERED artifact, not just its source: the `request_render` do-tool (developer/QA) renders the caller's actual composition through the sidecar and extracts evenly spaced keyframe PNGs to a container-shared `.previews/` path, returning their absolute paths in the envelope's `evidence.frames`. The agent must Read every frame and verify each scene/feature from the brief appears fully and legibly — a 14-second cut that only ever shows its first scene is exactly what this catches. A developer renders their own working tree (worktree-aware, `head_sha`/`dirty` provenance stamped); QA renders a read-only `git archive` export of the assembled branch — never a working tree. A successful render stamps the task's `render_preview` marker, and `i_am_done` on a video-authoring task refuses without it (`Requirement.RENDER_VERIFIED`, mirrored in the possibilities-matrix fast path), so no video task can complete on a source-only self-review. QA's `claim_review` evidence carries a `video_context` block (composition id + the dev's stamped preview + an instruction to re-render the branch state) so the reviewer checks output, not source. +Authoring is gated on the RENDERED artifact, not just its source: the `request_render` do-tool (developer/QA) renders the caller's actual composition through the sidecar and extracts evenly spaced keyframe PNGs to a container-shared `.previews/` path, returning their absolute paths in the envelope's `evidence.frames`. The agent must Read every frame and verify each scene/feature from the brief appears fully and legibly — a 14-second cut that only ever shows its first scene is exactly what this catches. A developer renders their own working tree (worktree-aware, `head_sha`/`dirty` provenance stamped); QA renders a read-only `git archive` export of the assembled branch — never a working tree. A successful render stamps the task's `render_preview` marker, and `i_am_done` on a video-authoring task refuses without it (`Requirement.RENDER_VERIFIED`, mirrored in the possibilities-matrix fast path — see `docs/rag/architecture/possibilities-matrix.md`), so no video task can complete on a source-only self-review. QA's `claim_review` evidence carries a `video_context` block (composition id + the dev's stamped preview + an instruction to re-render the branch state) so the reviewer checks output, not source. ## Render loop and the sidecar diff --git a/docs/rag/architecture/workspaces.md b/docs/rag/architecture/workspaces.md index 7c718faa..0b3f4b4c 100644 --- a/docs/rag/architecture/workspaces.md +++ b/docs/rag/architecture/workspaces.md @@ -103,7 +103,7 @@ If `auto_clone=True` and workspace doesn't exist, it's created on first access. ## Authentication -HTTPS repositories require a GitHub PAT configured on the project: +HTTPS repositories require a git token configured on the project — the field is historically named for GitHub PATs but works unchanged for a project registered against Gitea or GitLab (`projects.git_provider`): - **Token configured**: Auto-clone works, git operations succeed - **Token missing**: Error "Project requires a git token for HTTPS repositories" diff --git a/docs/rag/roles/auditor.md b/docs/rag/roles/auditor.md index 6d8f1df7..112c7a93 100644 --- a/docs/rag/roles/auditor.md +++ b/docs/rag/roles/auditor.md @@ -42,17 +42,32 @@ You still cannot claim tasks, message agents, or write code — the scheduled sw - Record private observations via `note(text="...", scope="reflect")` - Attach evidence via `evidence(task_id)` - Search the knowledge base via `roboco_ask_mentor` / `roboco_kb_search` +- Waive one open **minor/nit** revision-findings-ledger finding via `waive_finding(finding_id, note)` — see below +- Curate the KB's playbook queue via `approve_playbook` / `reject_playbook` / `archive_playbook` — a deliberate, bounded expansion of your read-only surface (KB curation, not agent comms, so the no-`dm` restriction still holds) - Curate the Obsidian vault's narrative for a just-completed root task-tree via `curate_vault(task_id, narrative)` — see below (only when `ROBOCO_OBSIDIAN_VAULT_ENABLED`) ## What You CANNOT Do - Claim, create, assign, complete, or cancel tasks - Pass or fail QA -- Escalate (`triage` is your only flow verb besides `i_am_idle`) +- Escalate (`triage` is your only flow verb besides `i_am_idle`/`waive_finding`) - DM agents (`dm`) or send `notify` - Acknowledge notifications (silent observer — `notify_ack` is not yours) - Write to project docs, write code, or run git write operations +## Waiving a review finding + +The Auditor is the **only** role that can close a revision-findings-ledger finding without a dev actually fixing it — and only for non-blocking severity: + +```python +waive_finding( + finding_id="a1b2c3d4", + note="Cosmetic — the naming nit doesn't affect behavior; not worth a rework cycle.", +) +``` + +`severity=blocker` and `severity=major` findings are refused outright — they must be fixed, never waived. Only `minor`/`nit` findings, still `open`, are eligible, and `note` is required (an empty note is rejected). No task status change: the ledger row moves `open -> waived` and a `task.finding_waived` audit event records the decision. See `docs/rag/architecture/review-findings.md`. + ## Silent Observer Mode The Auditor has **silent read access** across the org: @@ -89,8 +104,8 @@ When the Obsidian vault is armed, the orchestrator spawns you once per completed | MCP server | Verbs you can call | |-----------------------|--------------------| -| `roboco-flow` | `triage`, `i_am_idle` | -| `roboco-do` | `note` (scope=`reflect`), `evidence`, `notify_list`, `notify_get`, `curate_vault` | +| `roboco-flow` | `triage`, `waive_finding`, `i_am_idle` | +| `roboco-do` | `note` (scope=`reflect`), `evidence`, `notify_list`, `notify_get`, `approve_playbook`, `reject_playbook`, `archive_playbook`, `curate_vault` | | `roboco-git-readonly` | `roboco_git_status`, `roboco_git_log`, `roboco_git_diff`, `roboco_git_branch_list` | | `roboco-optimal` | `roboco_ask_mentor`, `roboco_kb_search` | diff --git a/docs/rag/roles/cell-pm.md b/docs/rag/roles/cell-pm.md index 25d5db44..ac02d519 100644 --- a/docs/rag/roles/cell-pm.md +++ b/docs/rag/roles/cell-pm.md @@ -18,7 +18,7 @@ ## What You CAN Do - Pull pending parent tasks via `give_me_work()` -- Plan and start a parent task via `i_will_plan(task_id, plan)` (this also auto-creates the parent branch) +- Plan and start a parent task via `i_will_plan(task_id, plan)` (this also auto-creates the parent branch); its planning briefing carries `collision_context` when same-parent siblings already declare overlapping file globs or migrations, so you can sequence subtasks before you delegate them - Create subtasks via `delegate(parent_task_id, title, description, body)` - Triage your cell's queue via `triage()` - Unblock blocked tasks via `unblock(task_id, reason, restore=True)` — `reason` (why the block is cleared) is recorded as your `journal:decision`, so no separate `note(scope='decision')` call is needed @@ -105,7 +105,7 @@ delegate( ) ``` -The args are **flat keywords** (not a nested `body=` dict). `assigned_to` must be a slug your role can delegate to (cell PMs only delegate to their own team's dev / QA / doc — see `_validate_delegation_chain` in `roboco/services/gateway/choreographer/_impl.py`). `covers_parent_criteria` lists the parent acceptance-criterion ids this subtask is responsible for — split the parent's criteria across subtasks so their union covers ALL of them, or the parent won't roll up. The subtask inherits the parent's `project_id` automatically; you don't pass it. +The args are **flat keywords** (not a nested `body=` dict). `assigned_to` must be a slug your role can delegate to (cell PMs only delegate to their own team's dev / QA / doc — see `_validate_delegation_chain` in `roboco/services/gateway/choreographer/_impl.py`). `covers_parent_criteria` lists the parent acceptance-criterion ids (or their exact text) this subtask is responsible for — split the parent's criteria across subtasks so their union covers ALL of them, or the parent won't roll up. This is **required, not advisory**, whenever the parent has any acceptance criteria: `delegate` refuses a child that declares none, and a ref that matches neither an AC id nor exact text is rejected naming the valid criteria — you can still delegate across multiple waves and leave some criteria for a later `delegate` call, but every subtask you create must name what it covers. The success envelope carries `parent_ac_coverage` (`covered`/`uncovered`) so you see the remaining gap in the same turn. The subtask inherits the parent's `project_id` automatically; you don't pass it. ## Completing Tasks diff --git a/docs/rag/roles/developer.md b/docs/rag/roles/developer.md index 78414223..3b5ab040 100644 --- a/docs/rag/roles/developer.md +++ b/docs/rag/roles/developer.md @@ -48,7 +48,10 @@ open_pr(task_id) → opens the PR, transitions to awaiting_qa └── QA fails → returns to needs_revision; fix + commit + open_pr again i_am_blocked(task_id, reason) → external dependency; cell PM unblocks -i_am_done(task_id, notes, resolved_findings?) → batched verify + open_pr shortcut +i_am_done(task_id, notes, resolved_findings?) → batched verify + open_pr shortcut; + silently fast-paths straight to QA when the + possibilities matrix is armed and your work already + looks done (see "The possibilities-matrix fast path") unclaim(task_id) → release a task back to the queue resume(task_id) → recover after compact / restart i_am_idle() → no work in your queue right now @@ -97,6 +100,14 @@ When toolchain matching is enabled, `i_am_done` is refused if the project's test When the architectural-conventions standard is enabled, `i_am_done` is refused on any block-level convention finding (e.g. a model defined in a router), reported with the offending `file:line` and a fix hint. A genuine false positive is cleared by committing a waiver in `.roboco/conventions.yml`. +## The possibilities-matrix fast path + +When `ROBOCO_POSSIBILITIES_MATRIX_ENABLED` is armed, `i_am_done` checks whether your work already looks done — commits exist, the PR is open, every acceptance criterion is addressed, and no revision finding is still open. If so, it takes a fast path straight to `awaiting_qa` in one call instead of the standard multi-turn verify/journal derivation. You don't call anything different or opt in — you always just call `i_am_done(task_id, notes, resolved_findings?)`, and the fast path silently applies when it applies. The non-negotiable guards still run either way: ownership, branch pushed and not behind base, conventions, and every open finding named via `resolved_findings`. The fast path trusts the PR's own CI-green signal as the quality gate; if there's no CI signal it falls back to the local `make quality` gate, and a known-red CI refuses the fast path outright (fix CI, don't route around it) rather than shipping a broken build to QA. + +## Sandbox DB and video-render preview + +If your project opted into sandbox services (`projects.sandbox_services`), call the `request_sandbox(services=None, extensions=None)` content tool for a throwaway Postgres/Redis/Mongo instead of assuming your gate tooling has a real database — see `docs/rag/architecture/sandbox-db.md`. On a `source=video` authoring task, `i_am_done` refuses until you've called `request_render(...)` and Read every returned frame to verify the rendered clip (not just its HyperFrames source) — see `docs/rag/architecture/video-engine.md`. + ## Recovering from a bounce (`needs_revision`) QA (`fail`), the in-path PR reviewer (`pr_fail`), your PM (`request_changes`), or the CEO (`ceo_reject`) can bounce your task back to `needs_revision` — and now the feedback is structured, not just a prose note. `evidence(task_id)` carries `revision_findings`: the OPEN entries from the revision-findings ledger, each with `file`/`line`/`severity`/`expected`/`actual`/`fix`. Read every one before you touch code — this is the actual code-level feedback, not a summary of it. diff --git a/docs/rag/roles/main-pm.md b/docs/rag/roles/main-pm.md index 9c24e27d..807f0a21 100644 --- a/docs/rag/roles/main-pm.md +++ b/docs/rag/roles/main-pm.md @@ -62,7 +62,7 @@ delegate( notify(target="be-pm", text="New initiative assigned — see task", task_id=subtask_id) ``` -`delegate` validates the delegation chain (main_pm → cell_pm) and the assignee-vs-task_type rule. Documentation is NOT delegatable — the lifecycle auto-creates the doc phase after the code subtask passes QA. +`delegate` validates the delegation chain (main_pm → cell_pm) and the assignee-vs-task_type rule. `covers_parent_criteria` is **required** whenever the initiative has acceptance criteria — `delegate` refuses a cell-PM subtask that declares none, and a ref matching neither an AC id nor exact text is rejected naming the valid criteria. Documentation is NOT delegatable — the lifecycle auto-creates the doc phase after the code subtask passes QA. ## Cross-Cell Coordination @@ -106,10 +106,12 @@ This is for *help while work is in flight*. Finished cell-scoped work arrives by You own the **root** task and the root→master PR. Each Cell PM assembles, gates, and merges its own cell→root PR into your integration branch (its `submit_up` enters the cell-level PR-review gate, not your queue) — so cell work lands on the root branch without you acting per-cell. ``` -master ← feature/main_pm/{root} ← feature/{cell}/{root}/{cell-pm} ← dev branches -(CEO) (you, via gate) (cell PM, via gate) (devs) +head rung ← feature/main_pm/{root} ← feature/{cell}/{root}/{cell-pm} ← dev branches +(CEO) (you, via gate) (cell PM, via gate) (devs) ``` +"head rung" is the project's env-ladder head (`roboco.models.env_branches.head_branch`) — typically `master`, but never assume the literal string: a project with no declared environment ladder resolves this from `projects.default_branch`, so this is unchanged for most projects. See `CLAUDE.md` "Env-branches ladder". + - A cell PM's `complete` merges a leaf PR into its cell branch; after the cell gate, its `complete` merges the cell→root PR into your root branch. You do not merge cell branches. - Once every cell's parent is terminal, **`submit_root(root_task_id, notes)`** opens the root→master PR and enters the in-path gate (`awaiting_pr_review`). The **main PR reviewer** checks the assembled root diff: `pr_pass` → `awaiting_pm_review`; `pr_fail` → `needs_revision` (owned by you, fix + re-`submit_root`). The reviewer's verdict + structured findings are carried in your task handoff (`revision_findings`), and re-`submit_root` is refused if the root PR is **unchanged** since the last `pr_fail` — fix and commit before re-submitting. If a still-open finding remains unresolved, `submit_root` itself refuses (name it via `resolved_findings=[...]` first — see `docs/rag/architecture/review-findings.md`). diff --git a/docs/rag/roles/pr-reviewer.md b/docs/rag/roles/pr-reviewer.md index ce01125f..9617961e 100644 --- a/docs/rag/roles/pr-reviewer.md +++ b/docs/rag/roles/pr-reviewer.md @@ -10,22 +10,24 @@ ## Core Responsibilities 1. Review **inbound** pull requests the org did not open — external / fork PRs (gated by an author allowlist), and, behind a second flag, internal org-repo PRs opened outside the agent task-flow. -2. Read the PR diff adversarially against the project's standards and post **one** complete change-request as a real GitHub review **on the PR itself** — no agent-to-agent chatter. +2. Read the PR diff adversarially against the project's standards and post **one** complete change-request as a real review **on the PR itself** (whichever forge the project uses — GitHub, Gitea, or GitLab) — no agent-to-agent chatter. 3. Journal evidence of what was checked. The org's own in-flight integration PRs are skipped by the **inbound** poll above — a live task already owns their branch. Re-review of inbound PRs is driven by the PR's head commit: an unchanged PR is skipped, new commits open a fresh review. ## In-path PR-review gate -The `pr_reviewer` role also runs the **in-path gate** on the org's OWN assembled delivery PRs — the merge-level review QA does not do. When a cell PM bubbles up its cell→root PR (`submit_up`) or the Main PM opens the root→master PR (`submit_root`), the task enters `awaiting_pr_review`. The cell reviewer (be/fe/ux-pr-reviewer) reviews its cell's assembled PR; pr-reviewer-1 reviews the root→master PR for the cross-cell integration seam (the bug class that lives where the FE and BE meet). Workflow: `claim_gate_review(task_id)` → review the assembled diff against the parent objective + every acceptance criterion + the FE↔BE contract → `note(scope="learning", ...)` → `pr_pass(task_id, notes)` (moves it on to the PM merge) or `pr_fail(task_id, findings=[{file?, line?, severity, criterion?, expected, actual, fix?, evidence?}])` (sends it back to `needs_revision`, like a QA fail — the old `issues=[...]` string form still works this release but is deprecated). Either verdict is also posted on the assembled PR itself as a GitHub review (server-side, via the bot account) so the decision is visible on the PR the PM merges: `pr_pass` posts an APPROVE and `pr_fail` a REQUEST_CHANGES — except on the root→master PR, which only ever gets a plain COMMENT because only the CEO acts on `master`. This gate gives the merge level the reject teeth the PM otherwise lacks. Leaf dev tasks and branchless coordination roots skip the gate. +The `pr_reviewer` role also runs the **in-path gate** on the org's OWN assembled delivery PRs — the merge-level review QA does not do. When a cell PM bubbles up its cell→root PR (`submit_up`) or the Main PM opens the root→master PR (`submit_root`), the task enters `awaiting_pr_review`. The cell reviewer (be/fe/ux-pr-reviewer) reviews its cell's assembled PR; pr-reviewer-1 reviews the root→master PR for the cross-cell integration seam (the bug class that lives where the FE and BE meet). Workflow: `claim_gate_review(task_id)` → review the assembled diff against the parent objective + every acceptance criterion + the FE↔BE contract → `note(scope="learning", ...)` → `pr_pass(task_id, notes)` (moves it on to the PM merge) or `pr_fail(task_id, findings=[{file?, line?, severity, criterion?, expected, actual, fix?, evidence?}])` (sends it back to `needs_revision`, like a QA fail — the old `issues=[...]` string form still works this release but is deprecated). Either verdict is also posted on the assembled PR itself as a review (server-side, via the bot account) so the decision is visible on the PR the PM merges: `pr_pass` posts an APPROVE and `pr_fail` a REQUEST_CHANGES — except on the root→master PR, which only ever gets a plain COMMENT because only the CEO acts on `master`. This is forge-agnostic on GitHub and Gitea, which both support a real "request changes" review; GitLab has no such primitive, so on a GitLab-backed project `pr_fail` posts as a plain MR note instead of a blocking review — the task still transitions to `needs_revision` normally regardless of forge, only the PR-visible signal differs. This gate gives the merge level the reject teeth the PM otherwise lacks. Leaf dev tasks and branchless coordination roots skip the gate. On a round ≥2 review, `claim_gate_review` also returns `prior_findings` — the FULL revision-findings ledger for this task, newest first. Your own prior verdict and every finding filed on it arrive in the briefing; check each one against the current diff before deciding, rather than re-deriving what you already found. `pr_fail`'s findings are capped the same way QA's are: a soft nudge above 5 in one call, a hard reject above 10. See `docs/rag/architecture/review-findings.md`. +`claim_gate_review` evidence also carries `collision_context` when the task under review has same-parent siblings that collide with it (overlapping declared file globs, or both adding a migration) — each entry names the sibling, the overlapping globs, and a drift flag when the diff's actual touched files stray from what was declared. `None` when there's no parent or no colliding sibling. Same collision map QA and the delegating PM see. + ### Gate enforcement When the architectural-conventions standard is enabled, `pr_pass` is refused on any block-level convention finding, the same way the developer's `i_am_done` is — the remediation hint points you at the offending `file:line` + the `pr_fail` verb (not `i_am_blocked`). When toolchain matching is enabled, `pr_pass` is likewise refused on a "broken" toolchain status. Your verdict note is a mandatory structured field (`pr_reviewer_notes`) written at `pr_pass` / `pr_fail`; it is persisted structured with a derived text mirror. -`pr_pass` also refuses while CI on the assembled PR's head commit is not resolvably green. Failing CI names the check(s) and points the remediation at `pr_fail` with a finding naming the failing check; pending / not-yet-scheduled / a transient GitHub API error are framed as retryable — wait and call `pr_pass` again once CI resolves, not a defect to route back to the dev via `pr_fail` unless the diff itself is also bad. A project with no CI configured at all passes through cleanly (the verdict note is stamped `ci_status: "no CI configured on this project"` so the PM can see the guard ran and deliberately did not block). Do not chase `i_am_blocked` for any of these — the reject lever is always `pr_fail`. +`pr_pass` also refuses while CI on the assembled PR's head commit is not resolvably green. Failing CI names the check(s) and points the remediation at `pr_fail` with a finding naming the failing check; pending / not-yet-scheduled / a transient forge-API error are framed as retryable — wait and call `pr_pass` again once CI resolves, not a defect to route back to the dev via `pr_fail` unless the diff itself is also bad. CI vocabulary differs per forge (GitHub check runs, GitLab pipelines, Gitea commit statuses) but is shaped into the same envelope before it reaches you — the green/red/pending read is identical regardless of which forge the project uses. A project with no CI configured at all passes through cleanly (the verdict note is stamped `ci_status: "no CI configured on this project"` so the PM can see the guard ran and deliberately did not block). Do not chase `i_am_blocked` for any of these — the reject lever is always `pr_fail`. **The per-AC evidence-walk is non-negotiable.** Do not assert "criteria met" from a skim. Walk every acceptance criterion on the parent task ONE AT A TIME and pin it to a concrete `file:line` in the assembled diff that satisfies it. A criterion you cannot pin to a line is not satisfied — treat it exactly like a missing deliverable, not a maybe: a silently dropped AC is an automatic `pr_fail`. @@ -36,7 +38,7 @@ You cannot `pr_pass` / `pr_fail` an assembled PR you authored (self-review guard ## What You CAN Do - Pull an inbound-PR review task via `give_me_work()` and claim it via `claim_pr_review(task_id)`. -- Post your verdict via `post_pr_review(task_id, ...)` — the change-request lands on the PR as a GitHub review (server-side; you never push to the contributor's fork). +- Post your verdict via `post_pr_review(task_id, ...)` — the change-request lands on the PR as a review, server-side, on whichever forge the project uses (you never push to the contributor's fork). - Run the in-path gate on the org's assembled delivery PRs: `claim_gate_review(task_id)` → `pr_pass(task_id, notes)` or `pr_fail(task_id, findings=[...])`. - Read-only inspect git via `roboco_git_status / _log / _diff / _branch_list`. - Search the knowledge base via `roboco_ask_mentor` / `roboco_kb_search`. diff --git a/docs/rag/roles/qa.md b/docs/rag/roles/qa.md index fe6d5f59..11b2ab63 100644 --- a/docs/rag/roles/qa.md +++ b/docs/rag/roles/qa.md @@ -17,7 +17,7 @@ ## What You CAN Do - Pull awaiting-QA tasks via `give_me_work()` / `claim_review(task_id)` -- Pass via `pass(task_id, notes)` (transitions to `awaiting_documentation`) +- Pass via `pass(task_id, notes, criteria_verified=[{criterion, evidence}, ...])` (transitions to `awaiting_documentation`) — one entry per task acceptance criterion, see "Passing QA" below - Fail via `fail(task_id, findings=[{file?, line?, severity, criterion?, expected, actual, fix?, evidence?}])` (returns to `needs_revision`) — see "Failing QA" below. The old `issues=[...]` (plain strings) form still works this release but is deprecated. - Read-only inspect git via `roboco_git_status / _log / _diff / _branch_list` - Search the knowledge base via `roboco_ask_mentor` / `roboco_kb_search` @@ -39,7 +39,9 @@ give_me_work() → returns an awaiting_qa task claim_review(task_id) → claim for review (auto-checks-out the dev's branch) -pass(task_id, notes) → moves to awaiting_documentation +pass(task_id, notes, criteria_verified=[{criterion, evidence}]) + → moves to awaiting_documentation; one + criteria_verified entry per task AC fail(task_id, findings=[...]) → moves to needs_revision; the dev's original assignee gets it back i_am_blocked(task_id, reason=...) → external blocker (broken env, can't @@ -81,10 +83,17 @@ pass( "Redis TTL matches, tests cover the boundary. ruff + mypy " "clean. Journal logged." ), + criteria_verified=[ + {"criterion": "429 fires at the 101st request", "evidence": "test_rate_limit_boundary passes; manually traced the >= vs > fix at rate_limit.py:88"}, + {"criterion": "Redis key TTL matches the configured window", "evidence": "verified TTL=60 in test_ttl_matches_window"}, + {"criterion": "AC #3 — Redis-down failover path", "evidence": "test_redis_down_failover covers the fallback branch"}, + ], ) ``` -`notes` must be substantive — the enforcement layer rejects empty or near-empty notes. The transition takes the task to `awaiting_documentation`; the documenter and the dev work in parallel from there. +`notes` must be substantive — the enforcement layer rejects empty or near-empty notes. `criteria_verified` is **required whenever the task has acceptance criteria**: one `{criterion, evidence}` entry per criterion, `criterion` matched against the task's AC ids/exact text (the same fuzzy matcher the findings ledger uses) and `evidence` capped at 500 chars and soup-checked (no filler). Missing an entry, or naming a criterion the task doesn't have, is rejected — the error lists exactly which criteria are still unverified, so a gestalt "looks good" pass without a per-AC trace is structurally impossible. Each entry renders deterministically into `qa_notes` as its own line: `[AC] — verified: `, appended after your `notes`. A zero-AC task imposes no `criteria_verified` requirement. + +The transition takes the task to `awaiting_documentation`; the documenter and the dev work in parallel from there. Your pass/fail note is a mandatory structured note (a QaNote) carrying substantive findings, not an empty string. It is persisted structured, and the legacy `qa_notes` text column is derived from it. @@ -94,6 +103,10 @@ When the architectural-conventions standard is enabled, the evidence returned on On a round ≥2 review (a task that has bounced before), `claim_review` also carries `prior_findings` — the FULL revision-findings ledger for this task, every round, newest first. Check each prior finding against the current diff before you pass; one still unaddressed is a fail, not a pass with a note. See `docs/rag/architecture/review-findings.md`. +## Collision Context in Review Evidence + +`claim_review` evidence also carries `collision_context` when this task has same-parent siblings that would collide with it — overlapping declared `intends_to_touch` globs, or both siblings adding a migration. Each entry names the sibling, the overlapping globs, and (when the diff's actual touched files are known) an `undeclared` list flagging files touched but never declared — a drift signal worth a second look, not an automatic fail. `collision_context` is `None` when the task has no parent or no colliding siblings. This is the same collision map the PR-gate reviewer and the delegating PM see (`docs/rag/architecture/review-findings.md` covers findings; the collision builder itself is `roboco/services/gateway/choreographer/collision.py`). + ## Failing QA ```python diff --git a/docs/rag/tools/task-tools.md b/docs/rag/tools/task-tools.md index cb492b28..30ce7ec6 100644 --- a/docs/rag/tools/task-tools.md +++ b/docs/rag/tools/task-tools.md @@ -23,21 +23,27 @@ resume(task_id) # recover a paused task after compact/restart i_am_idle() # no work in your queue right now ``` -There is no separate claim / start / pause verb — `i_will_work_on` composes claim + set-plan + start atomically, and `i_am_done` composes verify + submit-qa. Branches are auto-created on `i_will_work_on`; do not checkout by hand. +There is no separate claim / start / pause verb — `i_will_work_on` composes claim + set-plan + start atomically, and `i_am_done` composes verify + submit-qa. Branches are auto-created on `i_will_work_on`; do not checkout by hand — every root task branches from the project's env-ladder **head rung**, not a hardcoded `default_branch`/`master` string (see `CLAUDE.md` "Env-branches ladder"; a project with no declared ladder resolves this identically to its `default_branch`, so nothing changes unless the project opted in). + +Call `i_am_done` exactly the same way whether or not the **possibilities matrix** fast path (`ROBOCO_POSSIBILITIES_MATRIX_ENABLED`) is armed — when your task already has commits, an open PR, every AC addressed, and no open findings, `i_am_done` silently takes a fast path straight to QA instead of the standard verify→plan→journal turn. You never call anything different; there's nothing to opt into. ## QA flow ```python give_me_work() # returns an awaiting_qa task claim_review(task_id) # claim for review (auto-checks-out dev branch) -pass_review(task_id, notes) # awaiting_qa -> awaiting_documentation -fail_review(task_id, findings=[{file?, line?, severity, criterion?, expected, actual, fix?, evidence?}]) +pass(task_id, notes, criteria_verified=[{criterion, evidence}, ...]) + # awaiting_qa -> awaiting_documentation; one + # criteria_verified entry per task AC, required +fail(task_id, findings=[{file?, line?, severity, criterion?, expected, actual, fix?, evidence?}]) # awaiting_qa -> needs_revision (dev gets it back); # the deprecated issues=[str] shim still works this release unclaim(task_id) / resume(task_id) / i_am_idle() ``` -`notes` (on pass_review) and each `findings` entry (on fail_review) must be substantive — the enforcement layer rejects empty or near-empty content. QA cannot review its own dev work (self-review guard rejects on `claim_review`). Every `fail_review` finding is persisted to the append-only revision-findings ledger and rendered into `qa_notes`; a soft nudge fires above 5 findings, a hard reject above 10. On a round ≥2 review, `claim_review` also returns `prior_findings` (the full ledger) so you check what was filed before. See `docs/rag/architecture/review-findings.md`. +The callable MCP tool names are `pass` / `fail` (`pass`/`fail` are reserved words internally, so the IntentSpec/lifecycle layer calls them `pass_review`/`fail_review` — you call the short names). + +`notes` (on `pass`) and each `findings` entry (on `fail`) must be substantive — the enforcement layer rejects empty or near-empty content. `criteria_verified` is required whenever the task has acceptance criteria: name every one with concrete `evidence` (matched by AC id or exact text, capped 500 chars) or `pass` is rejected naming which criteria are still unverified; each entry renders into `qa_notes` as `[AC] — verified: `. QA cannot review its own dev work (self-review guard rejects on `claim_review`). Every `fail` finding is persisted to the append-only revision-findings ledger and rendered into `qa_notes`; a soft nudge fires above 5 findings, a hard reject above 10. On a round ≥2 review, `claim_review` also returns `prior_findings` (the full ledger) and `collision_context` (same-parent siblings that collide with this task's declared file globs or migrations, when any exist) so you check what was filed before and whether a sibling's work overlaps. See `docs/rag/architecture/review-findings.md`. ## Documenter flow @@ -61,7 +67,11 @@ delegate(parent_task_id, title, description, assigned_to, team, task_type, nature, estimated_complexity, acceptance_criteria, covers_parent_criteria=[...]) # create a subtask; covers_parent_criteria maps - # it to the parent ACs it is responsible for + # it to the parent ACs it is responsible for — + # REQUIRED whenever the parent has any acceptance + # criteria (a ref that matches neither an AC id + # nor exact text is rejected, naming the valid + # criteria); omit only when the parent has none reassign(task_id, assigned_to) # move a subtask to a different agent unblock(task_id, reason) # blocked -> in_progress (PM only); reason is # recorded as your journal:decision (no separate @@ -79,7 +89,7 @@ request_changes(task_id, findings=[...]) escalate_up(task_id, reason) # escalate to your escalation target ``` -After `i_will_plan` and each `delegate`, the envelope includes a coverage view of the parent — `parent_ac_coverage` (per-criterion `id` / `text` / `claimed` / `verified`) and `unclaimed_parent_acs` (criteria no subtask covers yet). A parent cannot idle with unclaimed criteria, nor `complete` / `submit_up` / `escalate_to_ceo` until every criterion traces to a child that passed QA. These gates stay inert until you start declaring `covers_parent_criteria`. See `docs/rag/workflows/task-planning.md`. +After `i_will_plan` and each `delegate`, the envelope includes a coverage view of the parent — `parent_ac_coverage` (per-criterion `id` / `text` / `claimed` / `verified`) and `unclaimed_parent_acs` (criteria no subtask covers yet). A parent cannot idle with unclaimed criteria, nor `complete` / `submit_up` / `escalate_to_ceo` until every criterion traces to a child that passed QA. `delegate` refusing a child with no `covers_parent_criteria` (above) is what puts every parent with acceptance criteria under this coverage discipline from its first subtask on — a decomposition can no longer opt out by never declaring. `i_will_plan`'s planning briefing also carries `collision_context` (in `context_briefing`, not `evidence`) surfacing any same-parent siblings that already collide on file globs or migrations, so you can sequence your delegation before you commit to it. See `docs/rag/workflows/task-planning.md`. **Delegation rules** (enforced): `main_pm -> cell_pm`; `cell_pm -> its team's devs`. Cell PMs receive planning-typed parent tasks; devs get code/research (UX devs also design). Always create subtasks via `delegate` with `parent_task_id` set — there is no standalone task-create verb for agents. @@ -138,14 +148,15 @@ The same role also runs the **in-path PR-review gate** on the org's own assemble ```python claim_gate_review(task_id) # claim an awaiting_pr_review task; returns the assembled - # diff + (on round >=2) prior_findings, the full ledger + # diff + collision_context (colliding siblings, if any) + + # (on round >=2) prior_findings, the full ledger pr_pass(task_id, notes) # assembled PR is correct -> awaiting_pm_review (the PM merges) pr_fail(task_id, findings=[...]) # send it back -> needs_revision, like a QA fail; # the deprecated issues=[str] shim still works this release ``` -Both verdicts are also posted on the assembled PR itself as a GitHub review (server-side, bot account) so the decision is visible on the PR the PM merges: `pr_pass` → APPROVE, `pr_fail` → REQUEST_CHANGES — except the root→master PR, which only ever gets a plain COMMENT (only the CEO acts on `master`). +Both verdicts are also posted on the assembled PR itself as a review (server-side, bot account) so the decision is visible on the PR the PM merges: `pr_pass` → APPROVE, `pr_fail` → REQUEST_CHANGES — except the root→master PR, which only ever gets a plain COMMENT (only the CEO acts on `master`). On a GitLab-backed project `pr_fail` posts as a plain MR note instead (GitLab has no request-changes review primitive) — the task still goes to `needs_revision` normally regardless of forge. A cell reviewer (be/fe/ux-pr-reviewer) reviews its cell's assembled cell→root PR; `pr-reviewer-1` reviews the root→master PR for the cross-cell integration seam, before the CEO sees it. diff --git a/docs/rag/troubleshooting/git-errors.md b/docs/rag/troubleshooting/git-errors.md index 66cf0ec0..5e606099 100644 --- a/docs/rag/troubleshooting/git-errors.md +++ b/docs/rag/troubleshooting/git-errors.md @@ -4,19 +4,20 @@ **Error:** `Project requires a git token for HTTPS repositories` (also surfaces as `WorkspaceError` during clone) -**Cause:** No encrypted GitHub PAT on `projects.git_token_encrypted` for this project. +**Cause:** No encrypted token on `projects.git_token_encrypted` for this project. **Fix:** 1. Open the project's settings tab in the panel -2. Paste a GitHub Personal Access Token with `repo` scope +2. Paste a Personal Access Token for the project's forge (`repo` scope on GitHub/Gitea; an equivalent `api`/`write_repository` scope on GitLab) — the Forge select on the project decides which provider `GitService` routes to 3. Save — the panel encrypts and stores it; the API never returns the plaintext Notes: - Each project has its own token (no global fallback) - Tokens are encrypted at rest with Fernet -- The token is injected only at the MCP layer (commit / clone / PR ops); `.git/config` is scrubbed post-clone so a leaked PAT from there is not a recovery path +- The token is injected only at the MCP layer (commit / clone / PR ops); `.git/config` is scrubbed post-clone so a leaked token from there is not a recovery path +- The field is historically named for GitHub PATs but works unchanged for a project registered against Gitea or GitLab (`projects.git_provider`) ## Workspace Not Found @@ -95,7 +96,7 @@ Don't checkout by hand — there is no `roboco_git_checkout` tool. 1. Nothing to push — no commits on the branch 2. Branch is on the workspace but not pushed yet (rare; the choreographer pushes during `commit`, but a stale workspace can drift) 3. Project has no git token configured -4. The GitHub repo doesn't allow PRs from your branch (rare; usually org-level branch protection) +4. The forge repo doesn't allow PRs from your branch (rare; usually org-level branch protection — applies on GitHub, Gitea, and GitLab alike) **Fix:** @@ -103,6 +104,10 @@ Don't checkout by hand — there is no `roboco_git_checkout` tool. - Verify the project has a git token (Missing Git Token, above) - If the task is in a stuck state, `unclaim(task_id)` and re-`claim` to rebuild the branch +## "GitHub API" wording in an error on a Gitea/GitLab project + +Some low-level git error messages still say "GitHub API" in their text regardless of which forge the project actually uses (a known wording gap, not a routing bug) — the underlying failure is real even when the vendor name in the message is wrong. Diagnose from the actual symptom (missing token, PR-not-found, merge conflict, etc.), not from the vendor name in the message. + ## FORCE_PUSH_FORBIDDEN **Cause:** Force-push is CEO-only. Anyone else attempting it (typically because their branch diverged) is denied. diff --git a/docs/rag/workflows/git-pr-types.md b/docs/rag/workflows/git-pr-types.md index e44e60a1..58a3b4bf 100644 --- a/docs/rag/workflows/git-pr-types.md +++ b/docs/rag/workflows/git-pr-types.md @@ -1,20 +1,40 @@ # Git PR Types -| `is_root_pr` | Target | Reviewer / Merger | Content | -|--------------|--------|-------------------|---------| -| `True` | `master` | CEO approves; Main PM opens + merges | Full task tree, all commits, all agent links | -| `False` | parent branch | Cell PM merges | Task commits only, scoped to the cell | +There is no `is_root_pr` field or shortcut — every assembled PR now passes through the in-path **PR-review gate** (`awaiting_pr_review`) before a PM merges it. This page is the quick-reference; `docs/rag/lifecycle/status-transitions.md` and `docs/rag/architecture/review-findings.md` cover the mechanics in depth. -## How PRs Are Created +## The three PR kinds + +| Kind | Opened by | Target | Gate reviewer | Merged by | +|------|-----------|--------|----------------|-----------| +| Leaf PR | Developer's `open_pr(task_id)` | The parent (cell) task's branch | none — QA reviews the diff directly, no PR-gate | Cell PM's `complete(task_id, notes)` | +| Cell→root PR | Cell PM's `submit_up(task_id, notes)` | The root task's branch | The cell's PR reviewer (be/fe/ux-pr-reviewer) via `pr_pass`/`pr_fail` | Cell PM's `complete(task_id, notes)`, after `pr_pass` | +| Root→master PR | Main PM's `submit_root(task_id, notes)` | The project's env-ladder **head rung** (`roboco.models.env_branches.head_branch`, typically `master` — never a literal string, always read through the shim) | The main PR reviewer (pr-reviewer-1) via `pr_pass`/`pr_fail` | The CEO, from the panel, after Main PM's `complete` escalates to `awaiting_ceo_approval` | + +A leaf dev task and a branchless coordination root (product fan-out, MegaTask umbrella) skip the PR-review gate entirely — there's no assembled PR for a reviewer to gate. + +## How PRs are created There is **no** `roboco_git_create_pr` MCP tool. PRs are side-effects of lifecycle transitions, driven by the choreographer: -- **Leaf PR (cell-scoped, `is_root_pr=False`)**: Opened automatically when the assigned developer calls `open_pr(task_id)` after their `commit(...)` calls. Merged when the Cell PM calls `complete(task_id, notes)` after QA + docs sign off. - -- **Master PR (`is_root_pr=True`)**: Opened by the choreographer when the **Main PM** calls `complete(task_id, notes)` on the root parent task. Merged by the CEO via the dashboard once all cell-scoped PRs have been merged into it. +- **Leaf PR**: opened automatically when the assigned developer calls `open_pr(task_id)` after their `commit(...)` calls (`verifying -> awaiting_qa`). +- **Cell→root PR**: opened by `submit_up(task_id, notes)` — enters `awaiting_pr_review`. +- **Root→master PR**: opened by `submit_root(task_id, notes)` — enters `awaiting_pr_review`. Targets the project's **head rung**, not literal `master` — a project with no declared environment ladder resolves this from `projects.default_branch` via the read-time shim, so nothing changes for a project that hasn't opted into a multi-rung ladder. See `CLAUDE.md` "Env-branches ladder". Title and body are generated from the task templates in `roboco/templates/git/pr_*.py`. Don't hand-write PR descriptions in the agent prompts — they'll be overridden. +## The PR-review gate (assembled PRs only) + +`submit_up` and `submit_root` land the task on `awaiting_pr_review`, not directly on `awaiting_pm_review`. A reviewer must `claim_gate_review(task_id)` then verdict: + +- `pr_pass(task_id, notes)` -> `awaiting_pm_review`, the PM merges via `complete`. +- `pr_fail(task_id, findings=[...])` -> `needs_revision`, routed back to the PM that submitted it, same as a QA fail. + +`pr_pass` additionally refuses while the PR's own CI is red or unresolvable; a repo with no CI configured passes through cleanly. On non-GitHub forges the verdict is posted differently: GitHub and Gitea both support a real "request changes" review, but GitLab has no such primitive, so a `pr_fail` verdict on a GitLab-backed project posts as a plain MR note rather than a blocking review — the task still transitions to `needs_revision` normally regardless of forge. See `docs/rag/roles/pr-reviewer.md`. + +## PR labels + +Every fleet-opened PR is best-effort labeled with the org-structure vocabulary (`derive_pr_labels`, `roboco/foundation/policy/pr_labels.py`): `to master` (today, only the root→master PR) vs `to slave`, `root` for an assembled root PR, `MegaTask` for a batch-carrying task, and a layer label (`main-pm` / `cell/{team}` / `subtask/{team}`) — so a human triaging the PR queue on the forge sees which tree and org layer a PR belongs to at a glance. + ## Auto-Checkout Branches and checkout are handled automatically: @@ -22,3 +42,7 @@ Branches and checkout are handled automatically: - `i_will_work_on(task_id)` (devs) creates the task's branch and checks it out in the agent's workspace. - `i_will_plan(task_id, plan)` (PMs) does the same for parent tasks. - Workspace dirty? The verb returns an error envelope; clean up first with `commit(...)` or escalate via `i_am_blocked(task_id, reason)`. + +## Forge-agnostic + +None of the above changes shape by forge — GitHub, Gitea, and GitLab (`projects.git_provider`) all route through the same `submit_up`/`submit_root`/`pr_pass`/`pr_fail` verbs and the same task states. Don't assume a PR lives at a `github.com` URL; `pr_url` always carries the real forge URL for whichever provider the project is registered against. diff --git a/docs/rag/workflows/pr-creation.md b/docs/rag/workflows/pr-creation.md index e69ea6ed..cc809d54 100644 --- a/docs/rag/workflows/pr-creation.md +++ b/docs/rag/workflows/pr-creation.md @@ -4,7 +4,7 @@ PRs are opened **before** QA review, not during `awaiting_documentation`. The choreographer creates the PR as a side-effect of the developer's `open_pr(task_id)` transition (`verifying → awaiting_qa`). -This is by design: QA reviews the real PR diff on GitHub, and the downstream PM/CEO approval chain operates on a PR that already exists. +This is by design: QA reviews the real PR diff on the project's forge, and the downstream PM/CEO approval chain operates on a PR that already exists. You do **not** call any tool to create a PR. There is no `roboco_git_create_pr` MCP tool. @@ -64,9 +64,13 @@ There is no `roboco_git_merge_pr` MCP tool. ## Prerequisites -- **Git token:** the project must have an encrypted GitHub PAT set on `projects.git_token_encrypted`. Without it, the workspace clone — and therefore everything downstream — fails with `WorkspaceError`. -- **Token scope:** `repo` (for branch push, PR create, PR merge). -- **Merge target:** the root→master PR targets the project's env-ladder **head** rung (`roboco.models.env_branches.head_branch`, typically `master`) — a project with no declared environment ladder resolves this straight from `projects.default_branch` via the read-time shim, so this is unchanged for every project that hasn't opted into a multi-rung ladder. +- **Git token:** the project must have an encrypted token set on `projects.git_token_encrypted`. Without it, the workspace clone — and therefore everything downstream — fails with `WorkspaceError`. The field is historically named for GitHub PATs but works for any forge a project is registered against (GitHub, Gitea, GitLab — `projects.git_provider`); see "Forge-agnostic git" below. +- **Token scope:** `repo` on GitHub/Gitea; an equivalent `api`/`write_repository` scope on GitLab (for branch push, PR/MR create, PR/MR merge). +- **Merge target:** every dev/cell/root PR — never just the root→master one — targets the project's env-ladder **head** rung (`roboco.models.env_branches.head_branch`, typically `master`) — a project with no declared environment ladder resolves this straight from `projects.default_branch` via the read-time shim, so this is unchanged for every project that hasn't opted into a multi-rung ladder. A middle ladder rung (e.g. `qa`/`stag`) is never a PR target for dev/cell/root work — the only thing that ever lands a PR on a middle or prod rung is the `EnvSyncEngine` cascade (see `CLAUDE.md` "Env-branches ladder"), which is a platform-authored sync, not something you open. `sync_branch` and `submit_up` resolve their target from the task's own parent hierarchy (`resolve_parent_branch`), not the ladder directly — the ladder only surfaces at the two edge cases where a task has no branched ancestor: a project-root task's branch cut, and `submit_root`'s PR target. + +## Forge-agnostic git + +The PR/CI/review surface is provider-routed (`roboco/services/forge/`) — GitHub, Gitea, and GitLab are all supported per-project (`projects.git_provider`), and `GitService` never branches on which one a project uses. From your side, `pr_number` and `pr_url` are always real values regardless of forge — `pr_url` is the actual forge URL (a Gitea/GitLab instance host, never assumed to be `github.com`). One real asymmetry: GitLab has no "request changes" review primitive, so a `pr_fail`/change-request verdict on a GitLab-backed project posts as a plain MR note rather than a blocking review — the task still moves to `needs_revision` normally either way, only the PR-visible signal differs. Don't hardcode `github.com` in any URL you construct or reason about. ## Troubleshooting diff --git a/docs/rag/workflows/qa-review.md b/docs/rag/workflows/qa-review.md index 10d5167f..865a9cfd 100644 --- a/docs/rag/workflows/qa-review.md +++ b/docs/rag/workflows/qa-review.md @@ -59,9 +59,16 @@ pass( "pytest 1635 passed; ruff and mypy clean. " "PR #123." ), + criteria_verified=[ + {"criterion": "429 on the 101st request in the window", "evidence": "test_rate_limit_boundary passes at rate_limit.py:88"}, + {"criterion": "Redis key TTL matches the configured window", "evidence": "test_ttl_matches_window asserts TTL=60"}, + {"criterion": "Tests cover happy path + boundary", "evidence": "3 new tests in test_rate_limit.py, all pass"}, + ], ) ``` +`criteria_verified` is required whenever the task has acceptance criteria — one entry per criterion, matched by AC id or exact text, `evidence` non-empty and capped at 500 chars. Missing a criterion, or naming one the task doesn't have, is rejected with the still-unverified criteria listed. Each entry renders deterministically into `qa_notes` as `[AC] — verified: `, so a gestalt "looks good" pass with no per-criterion trace is structurally impossible. A zero-AC task imposes no requirement. + Result: - Task advances to `awaiting_documentation` @@ -104,6 +111,8 @@ Result: If the task has failed before, `claim_review` returns `prior_findings` — the FULL ledger, every round, newest first — alongside the usual PR diff. Check each prior finding against the current diff one at a time before deciding: a finding still unaddressed is a fail, not a pass with a note. Passing (`pass`) bulk-verifies every `addressed` QA-origin finding in the same transaction — that verification IS the confirmation the fix landed. +`claim_review` also returns `collision_context` whenever this task has same-parent siblings that collide with it (overlapping declared file globs, or both adding a migration) — worth a glance before you pass, since an overlap you don't expect can explain an otherwise-mysterious diff hunk. + ## Reflect (recommended) After pass or fail, journal the review for future QA agents to learn from: diff --git a/docs/rag/workflows/task-planning.md b/docs/rag/workflows/task-planning.md index 1238faa2..8c294c7f 100644 --- a/docs/rag/workflows/task-planning.md +++ b/docs/rag/workflows/task-planning.md @@ -47,7 +47,9 @@ delegate( ## Acceptance-Criteria Coverage -When you decompose a parent task, declare which parent criteria each subtask is responsible for with **`covers_parent_criteria`** (a list of the parent's `acceptance_criteria_ids`). This is what lets the org prove a decomposition covers the parent's full intent — and it drives two gates and your coverage briefing. +When you decompose a parent task, declare which parent criteria each subtask is responsible for with **`covers_parent_criteria`** (a list of the parent's `acceptance_criteria_ids`, or the criterion's exact text). This is what lets the org prove a decomposition covers the parent's full intent — and it drives two gates and your coverage briefing. + +**`covers_parent_criteria` is required, not optional, whenever the parent has any acceptance criteria.** `delegate` refuses a child with no `covers_parent_criteria` declared — "'' declares no covers_parent_criteria, but the parent has acceptance criteria to decompose" — and a ref that resolves to neither an AC id nor exact text is rejected too, naming the valid criteria so you don't have to guess. It's only optional when the parent itself carries zero acceptance criteria. You don't have to cover everything in one `delegate` call — a wave may deliberately leave criteria for a later delegate — but every subtask you DO create must name what it covers. After `i_will_plan` and after each `delegate`, your envelope carries a coverage view of the parent so you can see what is still unmapped: @@ -59,7 +61,9 @@ Two gates build on the coverage link: - **Decomposition floor** — you cannot go `i_am_idle` on a parent while a criterion is still unclaimed. Delegate (or `reassign`) subtasks until every criterion is covered. - **Roll-up gate** — a parent cannot `complete`, `submit_up`, or `escalate_to_ceo` unless every criterion traces to a child that **passed QA** on it. -Both gates are **safe-by-construction**: they stay inert until you start declaring `covers_parent_criteria`, so a decomposition that never declares coverage is never blocked. Declaring coverage is how you opt your parent into the guarantee. +Since `delegate` now requires `covers_parent_criteria` on every child of a parent with acceptance criteria, both gates are live from the first subtask on for any such parent — there's no longer a way to decompose without opting in. A parent with zero acceptance criteria is exempt from both, since there's nothing to trace coverage to. + +The `verified` half of `parent_ac_coverage` isn't automatic — QA's `pass_review` (called via the `pass` tool) requires its own `criteria_verified` on the SUBTASK's own acceptance criteria before it can move a child to `awaiting_documentation`. Two distinct requirements on the same coverage chain: `covers_parent_criteria` (down, at delegate time — this subtask maps to those parent criteria) and `criteria_verified` (up, at QA pass time — each of THIS task's own criteria has concrete evidence). See `docs/rag/roles/qa.md` for the QA-side requirement. ## Delegating Code Work: Per-Dev Queues @@ -89,8 +93,8 @@ The task hierarchy is capped at `MAX_TASK_DEPTH = 4` levels (depths 0–3). The All code tasks follow the git workflow: - **Branches are auto-created when a developer claims the task** via `i_will_work_on` — no manual branch creation -- Root tasks: branch created from the default branch (main/master) -- Subtasks: branch forked from the parent's branch +- Root tasks: branch created from the project's env-ladder **head rung** (typically `master`/`main`) — a project with no declared environment ladder resolves this from `projects.default_branch` via the read-time shim, so nothing changes for a project that hasn't opted into a multi-rung ladder +- Subtasks: branch forked from the parent's branch (unaffected by the env ladder — this is pure task-hierarchy resolution) Coordination/parent tasks that only plan and delegate (no code) do not need a branch of their own.