Chore: 141 Gaps fill-in (#283)

* Updated uv.lock

* Bunch of fixes we need to verify first..

* feat(megatask): per-cell project map root-subtasks (multi-project, multi-cell)

A MegaTask root-subtask can now target an ad-hoc per-cell project map — a
third targeting shape that mirrors the existing product fan-out root. In
RoboCo a project is per-cell (ProjectTable.assigned_cell); a monorepo is N
per-cell projects sharing one git_url. So 'multi-cell' IS 'multi-project',
and a task may mix per-cell projects across products or include OSS-library
projects not in any product.

Storage: migration 052 adds task_cell_projects (mirrors product_projects;
unique per (task, team)). TaskTable gains a cascade-delete cell_projects
relationship; TaskCreateRequest / TaskCreate / Task response carry the map.

Policy: batch.is_branchless_coordination + is_valid_batch_shape gain a
has_cell_projects param — a root-subtask targets exactly one of project /
product / cell-map; the umbrella still targets none. TaskService passes
has_cell_projects at every predicate call site and persists the rows in
create(). _ensure_branch_for_task cuts feature/main_pm/{root} per distinct
project in the map (via _distinct_projects_for_task); _require_target_or_umbrella
and _validate_batch_membership accept the map shape.

Fan-out: every distinct_project_ids site (task.py branch creation, routes
_project_for_complete + _resolve_project_for_merge, orchestrator
_ambient_projects_for_task, pr_review._project_slug_for, git._project_for_task)
generalizes to first-distinct-project-of-map-or-product. Choreographer
_resolve_subtask_project resolves a delegated subtask's cell from the parent's
cell map. The product-scoped _slugs_for_product intake helper is unchanged.

Intake: prompter._draft_cell_map extracts the per-cell map from the_work[].
_validate_batch_scope counts distinct projects across all drafts' cells
(>=2 min stays; one 2-cell draft satisfies it). create_task_from_draft
persists cell_projects for >=2-cell drafts (project_id/product_id None),
collapses a 1-cell map to the single-project shape, and leaves single-cell
top-level project_id drafts unchanged. _resolve_owning_team routes a
multi-cell map to Main PM (coordination root, like a product root — a cell
PM can't delegate cross-cell). propose_draft/propose_batch tool descriptions
declare the per-cell project_id (both Claude SDK + grok runtimes).

The umbrella stays branchless / pure-coordination / submit_root-rejected;
the CEO-escalation pr_number gate is not widened (the map root is
is_umbrella=False, mirroring a product root, so submit_root supplies it).
Single-cell root-subtasks and everything below them are byte-for-byte
unchanged. Un-run MegaTask waves (multi-cell drafts) become runnable.

* [feature] Panel per-cell project picker + pnpm format infra

MegaTask root-subtasks can fan out across cells (be+fe, fe+uxui). Since a
RoboCo project is per-cell (ProjectTable.assigned_cell), a monorepo is N
per-cell projects sharing one git_url — so multi-cell IS multi-project. The
batch-review card now shows one project Select per the_work entry, scoped to
that cell's repos, instead of one Select bound to a single top-level
project_id. confirmBatch validates each cell's project is in scope and the
batch still spans >=2 distinct projects.

- prompter.ts: CellWork gains optional project_id (the per-cell picker seam).
- batch-review-card.tsx: per-cell Selects (one per the_work entry), scoped to
  the cell's projects; legacy single-cell drafts keep the one-Select path.
- use-prompter.ts: updateBatchDraftProject edits per-cell (entryIndex);  confirmBatch validates every cell; batchFromEvent parses per-cell map.

Also adds the missing pnpm format infrastructure (the panel had no formatter
at all): prettier devDep + .prettierrc.json (default-style config: 80-col,
double-quote, semi, trailing-comma-all) + .prettierignore, plus format /
format:check scripts. Only the 3 changed files above were reformatted; the
~222 pre-existing non-compliant files are left untouched (a wholesale reformat
is a separate explicit decision, not bundled into this feature).

* [fix] MegaTask verification: migration 052 enum + async cell-map read

Two real bugs surfaced running the full gate against a containerized
Postgres (and the orchestrator boot log):

1. Migration 052 crashed a real orchestrator boot with
   'type "team" already exists'. The generic sa.Enum(create_type=False)
   does NOT set the postgres enum's create_type attribute, so op.create_table
   (checkfirst=False) emitted a redundant CREATE TYPE against the pre-existing
   team enum. Switched to postgresql.ENUM(create_type=False) — the postgres-
   native enum whose create_type _check_for_name_in_memos actually reads, so
   the CREATE TYPE is suppressed. Verified: 051->052 upgrade against a DB where
   the team enum pre-existed (the exact path that crashed) now succeeds;
   downgrade 052->051 drops the table and preserves the shared enum; fresh
   upgrade head clean. (Migration 016 has the same latent sa.Enum pattern but
   never re-runs in prod, so it's noted, not touched here.)

2. _ensure_branch_for_task read task.cell_projects (lazy=selectin to-many)
   directly, tripping MissingGreenlet on a freshly-created/unqueried task —
   which then poisoned the async session (PendingRollbackError). Replaced with
   _task_has_cell_map: peeks InstanceState.unloaded (no IO) and reads the
   already-loaded map, falling back to an awaited count query only when the
   relationship is genuinely unloaded. Non-ORM stubs route to the plain
   attribute. Fixes 2 integration tests; the 6 cell-map unit tests still pass.

Also: typed the self stub as Any in test_choreographer_subtask_project
(mypy tests/ wants Choreographer, not SimpleNamespace) — the codebase idiom.

Gate: ruff format/check clean; mypy roboco/ + tests/ clean; full pytest
10371 passed / 388 skipped against containerized pgvector:pg16; vulture clean.
Pre-existing xenon C-rank on reassign (from prior commit 19a474d3, not this
feature) still blocks make quality — surfaced separately.

* [refactor] Extract reassign board-advisory diversion helper (C→B complexity)

`reassign` in roboco/services/task.py hit xenon absolute complexity 11 (a
C-rank block), failing `make quality`'s --max-absolute B gate. The C-rank
originated in 19a474d3 (pre-existing, not this feature branch's work).

Extract the board/advisory → cell-task diversion into
`_maybe_divert_board_advisory_reassign` (complexity 4, A). reassign drops to
9 (B); behavior is byte-for-byte preserved — the helper runs the same
guard + pool diversion + log, returning the diverted task or None so the
caller falls through to the normal handoff. Whole-repo xenon exits 0; the 159
reassign / board-guard tests pass.

Unblocks `make quality` on feature/metrics-granularity.

* [fix] migration 016: postgresql.ENUM(create_type=False) for reused team enum

016_add_products_and_task_product_id used `sa.Enum(..., create_type=False)`
for the reused Postgres "team" enum — the same latent defect that crashed
052 on a real orchestrator boot. On the generic `sa.Enum` the
`create_type` kwarg is silently dropped, so `_check_for_name_in_memos`
never sees it and `op.create_table` (checkfirst=False) emits a redundant
`CREATE TYPE team` that fails with "type 'team' already exists" against a
DB where the enum pre-exists.

Switch to the postgres-native `postgresql.ENUM(..., create_type=False)` —
its `create_type` is a real attribute the guard reads, so the CREATE TYPE
is suppressed (and DROP TYPE on downgrade too). The member list is inert
under create_type=False (it never creates/alters the type), so it stays at
016's original six, reflecting the enum as it stood then, not the
later-widened set.

This never crashed in prod because 016 is never re-run (alembic_version is
past it), but it's the same defect class. Verified on the real boot path:
upgrade to 015 in process A (team enum created by 001), then `upgrade head`
in a fresh process B — 016 applied clean, no DuplicateObjectError; downgrade
016->015 clean, shared team enum preserved.

See project_migration_enum_create_type_gotcha.

* [chore] panel: prettier reformat across the codebase

Apply `pnpm format` (prettier 3.8.5, 80-col / double-quote / semi /
trailing-comma-all) to the 223 pre-existing panel files that predated the
prettier infra added in cb5365a4. Pure formatting — no semantic changes:
multi-line arrays/objects collapsed where they fit, trailing newlines added
(.prettierrc.json), import grouping unchanged.

Verified: `pnpm format:check` clean, `pnpm lint` clean, `pnpm typecheck`
clean, `pnpm test` 113/113 pass (7 files).

* Bunch of runtime fixes for MegaTask and other issues

* Fix different project same PR number collision problem

Fix (two layers):
1. Root cause — pr_merge and rebase_pr_for_task now take a required project_id and scope the lookup where(pr_number == X AND project_id == Y). Required so no caller can forget — the bug class can't recur. All 4 call sites updated (choreographer cell_pm_complete, the rebase-retry, the superseded close_pull_request now passes project_id, and _verb_runner._do_pr_merge).
2. Crash guard — _finalize_cell_complete None-checks the complete() return and returns a clean invalid_state envelope (with a remediate hint) instead of dereffing None → 500 → respawn loop.

* Fix: Make main_pm + task_type=code impossible

* Fix Main PM needs revision can't re delegate

* [chore] Bump local LLM glm-5→glm-5.2 + swap Ollama fleet defaults off minimax

- llm_catalog: OLLAMA_DEFAULT_MODEL minimax-m3:cloud → kimi-k2.7-code:cloud;
  role defaults kimi-k2.6→kimi-k2.7-code, developer minimax→kimi, product_owner/
  ceo kimi→glm-5.2, documenter glm→kimi; GLM 5.1→5.2 comment fix.
- config + .env.example + docker-compose{.yml,.yaml,.registry.yml} + docs +
  memory_distiller + optimal_brain: glm-5:cloud → glm-5.2:cloud.
- panel ai-routing-card: typed SelfHostedModel/boolean annotations; drop the
  stale "Minimax M3 default" string (default is now catalog-driven).
- tests: glm-5:cloud → glm-5.2:cloud in pricing + rate-limit-retry fixtures.

* [fix] submit_root: hard unchanged-PR gate stops the pr_fail re-submit loop

The 2026-06-27 infinite pr_fail loop: a Main-PM root (PR #139) was pr_fail'd,
routed to needs_revision, and re-submitted byte-identical → awaiting_pr_review
→ pr_fail again, forever. The prior hint/a2a steer was ignored by the weak
coordinator model — hints don't stop a model that won't read them. A HARD gate
refuses the re-submit when the assembled root PR's head SHA is unchanged since
the last pr_fail (no new cell work → identical diff); a different SHA ⇒ the
branch advanced ⇒ allow. Every ambiguous case fails open (no prior fail, no
recorded SHA, no pr_number, unresolvable slug, git error, closed PR) — only the
exact-unchanged case is hard-blocked.

- content/models: PrReviewContent.head_sha (optional; JSON col → no migration).
- git: get_pr_head_sha (GitHub pulls API; None on any failure → fail-open).
- pr_gate: pr_fail captures head_sha into the verdict record; pr_pass does not.
- _impl: submit_root runs _submit_root_unchanged_pr_guard after _submit_up_guard;
  _current_root_pr_head_sha resolves slug + current SHA (fail-open).
- pr_review: extract module-level resolve_task_project_slug, shared by the mixin
  and the gate helper (_LegacyChoreographer reaches it via cast to the
  ChoreographerHelpers typed view — it doesn't inherit the helpers mixin).
- tests: test_submit_root_unchanged_pr_guard (11 — refuse/allow/6 fail-open/3
  capture-side, mypy-clean via cc:Any spy idiom, zero type:ignore) +
  test_pr_gate_notifies_pm capture-path stub.

* [chore] mypy tests/: clear all 15 pre-existing type errors so make quality can go green

The branch tip had 15 mypy tests/ errors in files this bundle did not author,
which blocked CI's make quality mypy step (mypy roboco/ tests/) regardless of
the bundle's own commits. Pre-existing is still existing — fix every one:

- test_schemas_v1_flow.py (8): the StrList coercion tests intentionally pass
  SDK-nested list-of-strings input ([[['...']]], {'item':{'$text':'...'}}, int,
  dict). Annotate those literals as list[Any] locals so mypy accepts the
  coerce-able shape; the StrList BeforeValidator still flattens to list[str] at
  runtime. No type:ignore.
- test_pr_gate_records_verdict.py (3): notes_structured is dict|None; narrow
  with 'assert t.notes_structured is not None' before indexing (the existing
  pattern at line 90).
- test_pr_review_hand_format_guard.py (1 site, 2 errors): the _verb_runner()
  spy assertion — use the cc: Any = c alias idiom so assert_not_awaited
  resolves; drops the now-unused type:ignore[union-attr].
- test_pr_gate_notifies_pm.py (1): drop the unused type:ignore[method-assign]
  on the a2a.send reassignment.
- test_content_models.py (1): narrow coerced with isinstance(coerced,
  PrReviewContent) before reading .issues (the base _Content lacks the field).

Gates: rm -rf .mypy_cache && mypy roboco/ tests/ = Success (855 files);
ruff check + format clean; 5 affected suites = 40 passed.

* [fix] fail_qa routes needs_revision back to the dev, never the pool

A dev task in needs_revision must go back to the developer, never the
pool. The pool path let a cell PM re-claim the revision (PMs can claim
needs_revision) — the live 2026-06-27 'needs revision on a dev task sent to
the cell PM' bug.

fail_qa's original_developer marker is the fast path, but it is
unreliable in practice (live observation: never persisted), so the
unassign else-branch was the load-bearing path and it dropped the task
into the pool. Add a work-session fallback (_resolve_revision_dev) that
resolves the developer who actually worked the task — the most recent
work session whose agent is a developer, the QA's own session excluded
— and reassigns to that dev instead of unassigning. Only unassign when
no developer ever touched the task. Self-heals the marker so a
subsequent re-fail takes the fast path and the QA-review index
attributes the work correctly.

* [feature] delegate carries dev-task collision surface (sequencing S1)

The cell/main PM's delegate verb now carries the dev-task collision
surface (intends_to_touch / adds_migration / touches_shared) and an
explicit depends_on override through DelegateRequest -> DelegateInputs
-> _create_subtask_from_inputs -> create_subtask, and create_subtask
forwards sequence / dependency_ids / batch_id / surfaces into the
prepared TaskCreateRequest instead of dropping them (the base create
already persists them at task.py:878-884).

This is the plumbing for the multi-level sequencing model edge kind 3
(dev-task collision DAG). Previously a dev task delegated with a
collision surface or an explicit dependency lost it before persistence
— dependency_ids was always [], so the only dev-task ordering was the
weak assignee-keyed spawn barrier (the live 2026-06-27 out-of-order
break: 40842957 started before 9b3682b8's PR merged). Phase S2 runs
SequencingService over the surfaced siblings and wires the DAG via
add_dependency.

* [feature] wire dev-task collision DAG at cell-PM delegation (sequencing S2)

Pure dev_task_collision_edges in sequencing.py turns a parent's surfaced
siblings into (depends_on_id, task_id) pairs via SequencingService. TaskService.
wire_sibling_collision_dag wires them through add_dependency (idempotent). The
choreographer calls it after each dev-task delegate so the sibling collision DAG
is built incrementally as the cell PM decomposes — file-overlap serializes,
migration chains, shared-last; stable (priority, sequence) ordering keeps edges
from flipping into reverse cycles on re-runs.

* [feature] wire cell-task wave chain + by-osmosis edge (sequencing S3)

Kind 2 (cell-task wave chain): a new cell-task under root-subtask UT_n
depends on every cell-task under every root-subtask in UT_n.dependency_ids
(the kind-1 wave-chain edges), so its branch carries the previous wave's
merged cell work. Re-derived from the root-subtask's deps, not the cell-task's
own dependency_ids (which also carry UX/product-fanout edges the by-osmosis
edge must not pick up). A root may fan to several cell-tasks (different cells),
so the previous wave's cell-task is a SET.

Kind 4 (by-osmosis): the first dev task (sequence 0) under a cell-task depends
on each predecessor cell-task's tail (max-sequence) dev task, so the new wave's
first branch carries the previous wave's fully-merged tail. Subsequent dev
tasks inherit the tail via kind 3 or the merged base.

Both wired from _create_subtask_from_inputs, dispatched on parent.team
(MAIN_PM -> kind 2; cell team -> kind 4). Pure helpers
(cell_task_wave_chain_depends_on, by_osmosis_tail_dev_tasks) unit-tested in
test_sequencing.py; TaskService methods integration-tested. Idempotent +
best-effort throughout (add_dependency dedupes; missing predecessors are
no-ops). Also fixes a latent mypy-tests gap (estimated_complexity required on
direct TaskCreateRequest calls in the S2 tests).

* [feature] sync_branch dev verb — gate-level branch rebase (Phase B1)

Raw shell git is denied to agents (Bash(git:*) base deny), so a developer
whose branch fell behind its base had no gate-level rebase — only the
CEO/PM-only /rebase HTTP route. sync_branch is the dev verb that wraps the
rebase through the gate (traced + evidenced), so the 'everything goes through
the gates' invariant holds.

- lifecycle: IntentSpec sync_branch (dev-only, ownership-gated, composes=(),
  git-only — no DB transition); _next_hint_synced helper.
- GitService.sync_task_branch: rebase task.branch_name onto its resolved base
  via rebase_onto_base (fetch + rebase + force-with-lease push).
- Choreographer.sync_branch + _sync_branch_preflight_rejection: not_found /
  unknown-role / spec-gate / no-branch / protected-base guards, then the git
  op; conflicts abort (no force-push) and steer to resolve-by-hand; git failure
  steers to i_am_blocked.
- HTTP route /api/v1/flow/developer/sync_branch + SyncBranchRequest schema.
- MCP tool sync_branch(task_id) + _TOOLS registration (manifest auto-propagates
  via intents_for_role(Role.DEVELOPER)).

Tests: intent spec (5), choreographer handler (8: happy/conflicts/not_found/
not_authorized/no-branch/protected-base/git-failure/audit), route (1), MCP (1).
ruff + mypy roboco/ tests/ clean; unit suite green (DB-fixture errors env-only).

* [feature] i_am_done behind-base submit gate (Phase B2)

A sibling's PR merging into the parent branch while a dev worked leaves the
dev's branch behind its base — the assembled PR then can't merge cleanly and
the sibling's changes go missing (the 2026-06-27 out-of-order dev-task break).
The behind-base gate refuses i_am_done in that state and steers the dev to
sync_branch (the Phase B1 gate-level rebase verb).

- GitService.is_behind_base: rev-list --left-right --count across
  origin/{base}...origin/{head} → (behind, ahead); fetch-first so origin
  reflects the pushed head. Raises on git failure (consistent with
  rebase_onto_base); malformed stdout degrades to (0,0).
- Choreographer._behind_base_gate: wired into _i_am_done_gate after
  _ensure_branch_pushed. behind>0 → invalid_state remediate→sync_branch.
  Fail-open on git/base-resolution error (flaky fetch can't strand a task at
  the submit gate — the merge layer has its own behind checks). Skipped for
  branchless roots and protected bases (master/main/-prefixed).

Tests: gate (6: refuse+steer/up-to-date/branchless/protected/fail-open-base/
fail-open-git), is_behind_base (6: parse/up-to-date/malformed/argv-form/
requires-branch/missing-project). ruff + mypy roboco/ tests/ clean; unit green.

* [docs] sync_branch prompt + behind-base guidance (Phase B3)

Update every behind-base/rebase guidance surface to reflect the B1
sync_branch dev verb + B2 i_am_done behind-base gate: devs now self-rebase
through the gate instead of escalating a plain behind-base condition; PMs
still escalate cell/root integration branches (they have no rebase verb).

- developer.md: sync_branch in the verb table; 'When your branch is behind
  its base' rewritten — call sync_branch, do NOT i_am_blocked a plain
  behind-base; conflicts → resolve by hand, commit, sync_branch again.
- cell_pm.md: delegate signature gains intends_to_touch/adds_migration/
  touches_shared/depends_on + a 'Collision surface' section (fill it on every
  code subtask so sibling dev tasks that touch the same files sequence into a
  conflict-free order — the 2026-06-27 out-of-order break fix); behind-base
  section steers devs to sync_branch, PMs escalate only the integration branch.
- main_pm.md: behind-base section — dev leaf = dev's sync_branch; cell/root
  integration branch = escalate_up.
- RAG git-errors.md / blocked-tools.md: devs sync_branch, PMs escalate.
- docs/troubleshooting/common-issues.md: leaf self-rebases; integration branch
  still escalates to operator.
- CLAUDE.md verb surface: developer gains sync_branch.
- agents/prompts/_generated/*: regenerated via scripts/regenerate_verb_tables.py
  — adds sync_branch to the dev table AND catches the generated tables up to
  the S1/S2 delegate sequencing params + meltdown-fix note top-level params
  (the derived files had drifted stale vs the already-committed schemas).

Docs/prompts only — no code. ruff + mypy roboco/ tests/ clean.

* [chore] orchestrator: refuse to spawn human-only roles (CEO/prompter/secretary)

A live 2026-06-27 incident saw a CEO agent container spawned. Root cause:
_dispatch_a2a_work iterates every A2A/notification target and spawns it
with no human-role filter, and _is_agent_active('ceo') is always false
(the CEO is never a container), so the 'skip if active' check could never
protect the CEO. Any CEO-addressed notification (board handoff, escalation)
launched a CEO container — the system acting as the human CEO: a trust
violation. The CEO is the human operator; intake (prompter) and secretary
are human-driven chats launched through their own dedicated guarded paths
(_spawn_intake_container / _spawn_secretary_container), never spawn_agent.

Fix: a single chokepoint guard at the top of spawn_agent refuses
Role.CEO / PROMPTER / SECRETARY (raises AgentReadinessError + logs). This
structurally covers every dispatcher present and future, since they all go
through spawn_agent. Plus a defense-in-depth skip in _dispatch_a2a_work so
a human-role target never even calls in (avoids error-log spam; the
notification stays for the human to read in the panel).

Safe: the dedicated human-spawn paths do not route through spawn_agent.
Regression tests: spawn_agent refuses ceo/intake-1/secretary-1, does NOT
refuse a real agent; _dispatch_a2a_work skips CEO/intake/secretary targets
and still spawns real-agent + mixed-target cases.

* [chore] orchestrator: skip human-only assignees in claimed/pm-review dispatchers

Defense-in-depth for the spawn_agent human-role chokepoint (d31d6719).
The chokepoint structurally guarantees no CEO/prompter/secretary container
can ever spawn — every dispatcher goes through spawn_agent. But two
dispatchers resolve an arbitrary assigned_to and spawn it with only a
None/unknown-role filter, so a human-assigned task would reach the
chokepoint and RAISE: caught by the per-dispatcher try/except, but it
aborts that dispatcher's whole tick (stalling other respawns behind the
mis-assigned task) and error-logs every cycle. The other dispatchers are
already safe by whitelist/hardcoded slug (blocker_resolver_slug returns
None for non-PM/non-BOARD; escalation/approval use whitelists; marketing
and audit hardcode their non-human slug).

- _claimed_task_needs_agent: return None for a CEO/prompter/secretary
  assignee — no container to respawn, and do NOT release a human-owned
  task to pending (that would re-route it to a PM). Leave it for the human.
- _dispatch_pm_review_work (assigned branch): skip a human-only assignee
  so a CEO-assigned awaiting_pm_review task neither spawns nor aborts the
  dispatcher's tick.

Audited all target-iterating dispatchers; only these two lacked a filter.
Regression tests cover both skips.

* [F002] retype board-routed MegaTask root-subtasks code->planning on activation

_activate_batch_root_subtasks flipped a held root-subtask to team=MAIN_PM
but left task_type=code (intake only coerces main_pm-team drafts, so a
board-routed code root reached activation still code-typed). The
main_pm+code combo re-introduces the 2026-06-27 meltdown. Mirror
approve_and_start's own retype via main_pm_cannot_own_code so the
activated child is a planning-typed coordination root.

TDD: RED test_activate_batch_root_subtasks_retypes_code_to_planning
watched fail (task_type stayed CODE), then GREEN after the retype.
ruff+mypy clean; 125 batch/umbrella/approve tests green, no regressions.

* [F003,F004,F014] enforce HMAC agent-token gate on do routes + WebSocket streams

F003/F014: /api/v1/do/* only required X-Agent-ID (UUID) — no token check,
unlike the flow routers' role guards. A forged X-Agent-ID passed. Added
require_any_authenticated_agent (token-only; do router serves all roles)
and applied it as a router-level dependency. Binds X-Agent-ID to a verified
HMAC token when ROBOCO_AGENT_AUTH_REQUIRED=true; rejects a forged token
even in dev mode.

F004: /ws/* per-agent streams (channels/agents/sessions/notifications)
never read the nginx-injected X-Agent-Token, so in strict mode an agent on
the Docker network could subscribe to another agent's notifications with
no auth. Added _require_panel_token verifying the CEO panel token against
the CEO identity; wired into all four per-agent streams (system stream
stays operator-only per its docstring). Same strict/dev contract.

TDD: RED tests watched fail (no gate -> 200/accept), then GREEN. ruff+mypy
clean; 399 api/mcp + 29 WS tests green, no regressions.

* [F005,F006] grok auth: directory mount + atomic-write fallback

F005: the single-file bind mount of auth.json pinned the inode, so the
orchestrator's atomic refresh (tmp+rename within ~/.grok) never reached a
running grok container — a long-lived container hung at the login prompt
when the original ~6h token expired. Mount the host ~/.grok DIRECTORY (ro)
at /home/agent/.grok-auth-ro; the entrypoint symlinks ~/.grok/auth.json at
that RO mount so grok + the --check backstop read the live credential (the
directory mount sees the host-side rename) while grok's writable state
(config.toml, sessions/) stays in the image's ~/.grok.

F006: a rotated refresh_token is single-use — xAI invalidates the old one
the instant it issues the new one. If the atomic write failed after the
rotation, the file kept the now-dead old refresh_token and the credential
was permanently lost on the next refresh. _atomic_write now falls back to a
direct write when tmp+replace fails, so the rotated token always lands on
disk (losing the write is catastrophic; losing atomicity is not).

TDD: RED tests watched fail, then GREEN. ruff+mypy clean; 32 grok tests
green, no regressions.

* [F016,F017] choreographer: surface invalid_state instead of None.status 500 on submit_root / i_am_blocked

Both verbs compose a single atomic action whose None return (the verb's
own result) flowed out of run_intent and was dereferenced as t.status,
HTTP 500-ing with no actionable rejection:

- F016 submit_root: submit_for_review returns None when the root->master
  PR was already opened / the task raced out of in_progress. Post-runner
  None-guard extracted into _submit_root_finalize -> invalid_state
  (re-fetch; if awaiting_pr_review the PR is open, wait for reviewer;
  else re-delegate fixes and retry) instead of None.status.

- F017 i_am_blocked: escalate returns None in four cases (no task, no
  agent, no resolvable escalation-target slug, no target agent row) e.g.
  a developer whose role has no PM above it. _run_i_am_blocked_intent
  now guards updated is None -> (t, invalid_state rejection) with
  remediation (re-fetch + escalate to CEO directly / retry) instead of
  the caller deref'ing None.status -> 500 + respawn-loop.

TDD red->green; ruff + mypy clean; gateway suite green (58 passed).

* [F007] choreographer: cell-level unchanged-PR re-submit loop-stopper for submit_up

The root loop-stopper (F016) was root-only; a weak cell PM could re-submit
the unchanged cell->root PR after a pr_fail and loop awaiting_pr_review ->
pr_fail forever (the cell analogue of the 2026-06-27 root loop).

pr_fail stamps the assembled PR's head SHA into notes_structured.pr_review
.head_sha for cell AND root gate tasks alike (the capture is gate-verb-
level, not root-level), so the same structural refusal applies to submit_up:
if the cell PR's current head SHA equals the SHA the last pr_fail recorded,
no new dev work landed on the cell branch -> the diff is byte-identical ->
refuse, do not re-open the gate. Different SHA -> branch advanced -> allow.

- _submit_up_unchanged_pr_guard mirrors _submit_root_unchanged_pr_guard
  (cell-PM remediation: re-delegate to the dev + wait for re-assembly),
  wired into submit_up after _submit_up_guard passes.
- Renamed shared _current_root_pr_head_sha -> _current_pr_head_sha (both
  guards use it; the lookup was never root-specific).
- Every ambiguous case FAILS OPEN (no prior fail, no recorded sha, no
  pr_number, no resolvable project, git/closed-PR None) — only the exact-
  unchanged case is hard-blocked.

TDD red->green; ruff + mypy clean; F007+F016 guard suites green (15 passed).

* [F008] evidence_builder: surface persisted pr_review verdict+issues in the PM task_handoff

The pr_fail a2a steer to the owning PM is fire-and-forget; a PM respawned
into needs_revision later read none of it (build_task_handoff never looked
at notes_structured), saw a generic 'needs revision' with zero concrete
change-requests, and re-submitted the same PR (the 2026-06-27 infinite
pr_fail loop on 9980d0a0 / PR #138). The signal-gap was only partially
closed by the a2a.

build_task_handoff now extracts notes_structured.pr_review
(verdict/summary/issues/head_sha — the slot pr_fail authors on every fail)
into a pr_review field on the handoff, so every PM briefing for the task
carries the concrete change-requests. A prior pr_fail alone now counts as
prior-work-worth-resuming. Type-guarded + capped; absent => no key (no
misleading empty slot).

TDD red->green; ruff + mypy clean; evidence_builder suite green (14 passed).

* [F009] notification: derive requires_ack from ACK_REQUIRED_BY_TYPE, not the True default

NotificationService._create_notification built NotificationTable without
requires_ack, so the column default (True) applied to EVERY notification -
including informational REVIEW_REQUEST / DOCUMENTATION_REQUEST /
A2A_REQUEST / KNOWLEDGE_SHARE (ACK_REQUIRED_BY_TYPE -> False) and every
@mention from MessagingService._notify_mentions. Each false ack-required
inflated the recipient's unacked set and soft-blocked i_am_idle into
respawn churn.

- _create_notification: requires_ack=ACK_REQUIRED_BY_TYPE.get(type, True)
  (unmapped types default True - preserve the action-required bias).
- _notify_mentions: requires_ack=False explicit (MENTION is informational).

TDD red->green (identity is False/is True assertions - the mocked
flush doesn't apply SQLA's insert-time default, so pre-fix the attribute
was None); ruff + mypy clean; notification suite green (18 passed).

* [F010] notification: never dedup informational notifications (knowledge-share data loss)

The purpose-based dedup suppressed a same-purpose (same sender/type/task,
overlapping recipients) notification while a prior one was unacked. For
informational types (KNOWLEDGE_SHARE / MENTION / A2A_REQUEST / BROADCAST +
the pickup-proves-receipt triad) each send carries DISTINCT content (a new
learning, a new mention) and acking is voluntary, so a recipient who never
acks the prior one let the dedup permanently suppress every subsequent
same-sender broadcast - silent learning-broadcast data loss.

The dedup's anti-loop rationale (stop unacked-set inflation soft-blocking
i_am_idle) only holds for action-required signals. Gate the dedup on
ACK_REQUIRED_BY_TYPE.get(type, True): action-required types still dedup,
informational types always create. Unmapped types default True (dedup on).

TDD red->green; ruff + mypy clean; notification + dedup suites green (20).

* [F011] playbook: de-index rejected/archived playbooks from the PLAYBOOKS RAG index

* [F012] release_executor: fail-closed on git add/commit before push

* [F013] release_proposal: Redis SET NX mutex guards the ~40min execute against concurrent approves

* [F015] flow_qa/flow_doc: add i_am_blocked route (manifest-registered escape hatch was 404)

* [F018] claim_guards: treat blocked as active + broaden the guard lookup so a blocked dev can't double-claim

* [F019] git: clear orphaned .git/*.lock files after a timeout-SIGKILL'd mutation op

* [F031] identity: role_for_slug_or_none so defensive skip-guards don't crash the dispatcher tick on stale slugs

* [F032] test: unknown-assignee claim reaches release-to-pending path

F031's role_for_slug_or_none fix made the unknown-assignee release branch
in _dispatch_claimed_without_agent reachable (the human-only guard no
longer raises/short-circuits on a stale slug). Lock that reachability in:
a claimed task with an unknown-assignee UUID past grace returns the slug
(not None) so get_agent_role -> 'unknown' releases the claim to pending
for a role-matched reclaim.

* [F033] orchestrator: capture container_id at startup re-adoption

_readopt_running_agents registered re-adopted ACTIVE instances with
container_id=None. _check_health skips container_id-is-None instances, so
when a re-adopted container later exited the stopped-container handler
never ran and the task stranded under a phantom ACTIVE instance forever.

Add _resolve_container_id (docker inspect -f '{{.Id}}') and store the real
id on re-adopt. Best-effort: a probe failure degrades to None (still
ACTIVE; the reaper's Docker-liveness fallback covers it).

* [F034] orchestrator: re-stamp respawn last_check at restore

_pm_made_rule_following_retry bounds its tracing_gap audit lookup with
since = record.get('last_check'). A stale persisted last_check from before
the restart matched pre-restart tracing_gap rows, falsely resetting the
breaker on the very first post-restart spawn — exactly when a fresh strike
count should be evaluating current state.

_partition_respawn_rows now re-stamps last_check to the restore time on
every restorable entry, bounding the lookup to post-restart gaps only.

* [F035] orchestrator: probe-resume loop actually revives parked agents

_park_provider_unavailable parked the provider + offlined the instance but
never registered a WaitingRecord, so _on_probe_success -> _parked_agents_for
(always filtered on waiting_for=='rate_limit_lifted') returned [] and
resolve_wait revived nobody — recovery fell to the 600s stale-claim reaper
instead of the probe-success path the parking design relied on.

Register + persist a rate_limit_lifted WaitingRecord at park time (mirrors
mark_waiting_long, minus stop_agent — the container is already dead).

Companion reaper guard: _reap_with_service now skips provider-parked
assignees (_assignee_is_provider_parked) so the claim survives until the
probe revives the agent — otherwise the reaper releases the claim to pending
and probe-success respawns on a task the agent no longer owns.

* [F036] orchestrator: read transcript for overload detection too

The SDK server writes model-API errors (529/500/503) to /tmp/sdk-server.log,
not stdout, so an overload marker can appear only in the durable Claude
transcript — the same rationale already applied to the session-limit
detector. _provider_overload_park_target read only docker logs, so an
overload was missed and the agent crash-respawned straight back into it.

Now concatenates the transcript tail before matching, mirroring the
rate-limit path.

* [F037] orchestrator: drop bare error-NNN overload markers

The bare 'error 529'/'error 500'/'error 503' markers were broad enough to
false-match an agent that merely writes about an HTTP status code in its own
notes ('the endpoint returned error 500, retrying'), parking the whole
Anthropic fleet on a non-issue.

The SDK error formatter emits 'API Error: NNN' + a JSON error type, so the
remaining 'api error: 529/500/503' + 'overloaded_error' +
'internal_server_error' markers cover every real overload without that
false-match surface.

* [F038/F039] orchestrator: sign X-Agent-Token on self-API calls

The prior self-PATCH 401 fix only carried X-Agent-ID/X-Agent-Role. Arming
ROBOCO_AGENT_AUTH_REQUIRED=true made the middleware require a signed
X-Agent-Token, so every orchestrator self-call (auto-block / auto-resume /
auto-recover / SLA annotation) 401'd and silently no-op'd — wedging
paused/blocked parents.

Add _system_api_headers() that wraps the base headers with a signed token
for the system identity (issue_agent_token); switch all six self-call sites.
Dev fallback: no secret set => UNSIGNED sentinel + auth not required.

* [F040] orchestrator: finalize grok spawn session on cost-cap kill

_enforce_grok_cost_budget killed + evicted the container without calling
_finalize_spawn_session, so the open agent_spawn_sessions row stayed open
(ended_at IS NULL) and the burned usage/cost was never recorded in the
dashboard.

Call _finalize_spawn_session(exit_reason='cost_cap') BEFORE popping the
instance — it reads self._instances[agent_id] for the model +
usage_session_id, which the pop would lose.

* [F041] park grok exit-78 (auth missing/expired) instead of crash-retrying

A one-shot grok container whose entrypoint ran grok_auth --check and found
the token missing/expired exits 78 (EX_CONFIG). Crash-retrying 3x burns
tokens for zero progress — the agent cannot start without a valid token.
Park the provider with kind=auth_missing (same shape as the 429 exit-75
path) so the probe-resume loop revives the task once grok_auth.refresh_if_stale
mints a fresh token; if still expired, the next exit 78 re-parks (no burn).

Also fixes a latent F035 regression: _park_provider_unavailable now registers
a WaitingRecord, so the bare-__new__ rate-limit park test had to set
_waiting_records + stub _persist_waiting_record (mirrors the overload-test
fixture).

* [F042] isolate concurrent-duplicate conventions cache put in a savepoint

Two task creates for the same project/HEAD can race to populate the
conventions cache; the loser's INSERT fails the partial-unique index with
IntegrityError. A bare session.add + flush poisons the shared session (the
task-create transaction rides the same session), so every subsequent op
raises 'this session is in error state' and task creation crashes.

Run the INSERT in a savepoint (begin_nested) and swallow the IntegrityError:
only the savepoint rolls back, the outer transaction stays usable, and the
winner's row satisfies the next _cache_get.

* [F043] guard escalate_up against resurrecting terminal tasks

escalate_up had composes=() and no source-status guard, so a PM could
escalate a COMPLETED/CANCELLED task and apply_escalation set it back to
BLOCKED — bypassing the state machine's terminal-state invariant.

Defense in depth:
- spec: add PRECONDITION_NON_TERMINAL to escalate_up's extra_preconditions so
  the lifecycle gate rejects terminal tasks (invalid_state) before the
  journal:decision write fires; generalize _check_intent_preconditions to
  honor non-tracing rejection_kind (not_authorized / invalid_state).
- service: apply_escalation (the single write primitive) returns False and
  refuses to mutate a terminal task — covers the HTTP escalate route which
  bypasses the spec gate. escalate() / escalate_up_to_role() return None on
  refusal so the gateway emits a clean invalid_state envelope.
- route: the HTTP escalate route 409s a terminal task BEFORE sending the
  escalation notification (so a finished task isn't yanked back, PM not pinged).

* [F044] pr_pass gate remediation points the reviewer at pr_fail, not i_am_blocked

The pr_pass gate runs the toolchain + conventions guards on the REVIEWER's
workspace, but their remediation text said 'call i_am_blocked' — a verb the
PR reviewer does not have. The reviewer would chase a verb they cannot call
instead of rejecting the PR.

Make the guards reviewer-aware: a reviewer=True flag (passed by _pr_pass_blocked)
switches the remediation to pr_fail(issues=[...]) — the reviewer's reject
lever, sending the PR back to needs_revision for the dev to fix the
environment / validator. The dev (i_am_done) path keeps i_am_blocked, which a
dev does have. _conventions_guard (the pr_pass path) now passes reviewer=True
through to _conventions_rejection.

* [F045] rate-limit: loud activate-failure log + in-memory orphan-probe fallback

The in-verb i_am_blocked(rate_limited) path wrapped RateLimitStateTracker.activate
in a bare contextlib.suppress. A silent activate failure stranded the fleet:
agents were parked in _waiting_records but the provider never entered the tracker,
so the tracker-driven _sweep_rate_limit_probes never probed it and no
_on_probe_success ever resumed them — parked agents stuck in WAITING_LONG.

Fix: (1) replace the bare suppress with a try/except that logs an error event
naming the provider + affected agents; (2) in _sweep_rate_limit_probes, after
probing the tracker-listed set, scan _waiting_records for any rate_limit_lifted
provider the loop did NOT cover and probe it via the time-expiry fallback (empty
state -> probe now) so _on_probe_success resumes the parked agents. The fallback
reads only local memory, so it still resumes when Redis was down at park time
(list_rate_limited_providers failure now falls through to the orphan scan instead
of returning early).

* [F046] pr_gate: guard None runner result on concurrent transition (pr_pass/pr_fail)

_gate_decision dereferenced the verb-runner result without a None guard.
run_intent returns None when a concurrent transition (cancel or a racing
reviewer) moves the task out of awaiting_pr_review between the precondition
gate and the runner's final composed action (the verb runner's documented
last-action source-status contract). The subsequent t.assigned_to /
t.status / _post_gate_review_to_pr(t, ...) dereferences then crashed the
gate with a 500 AttributeError. Add a None guard that surfaces a clean
invalid_state rejection (re-fetch + re-issue) before any dereference; no
PR post or a2a runs against a None task. TDD test_pr_gate_notifies_pm.py (+2).

* [F047] conventions: reviewer-aware block-finding remediation on pr_pass gate

The pr_pass (reviewer) conventions guard reused the dev-path block-finding
remediation: 'add a waiver to .roboco/conventions.yml in your branch'. A
pr_reviewer does not own the assembled cell->root / root->master branch and
has no commit verb on it, so the waiver remediation is unreachable — a false
positive stranded the gate with no self-recovery (the reviewer could neither
commit a waiver nor pr_pass). The fail-open content path is documented
precision-over-recall and stays as-is; the actionable gap is the remediation.

Fix: _conventions_rejection now branches the block-finding remediation on
reviewer=True (mirroring the could_not_run branch from F044). The reviewer
path points at pr_fail carrying the findings as issues so the PR returns to
needs_revision and the DEV fixes the violation or commits the waiver (the dev
CAN commit to the branch); waiver authorship is framed as the dev's action,
not the reviewer's. Dev i_am_done path wording unchanged. TDD
test_conventions_gate_pr_pass.py (+1).

* [F048] notify: reject human-only recipients (prompter/secretary) — no agent ack path

notify() only checked the SENDER role. The recipient was resolved by
NotificationService._resolve_recipients, which drops only unresolvable slugs
— it does not exclude human-only roles. The prompter (intake-1) and secretary
(secretary-1) are seeded agent rows, so they resolved, and an ack-required
ALERT addressed to them sat permanently unacked (no agent auto-acks it),
polluted the panel's pending-ack view, and — via the dedup query's
~acked_by.contains — permanently suppressed any later same-purpose
notification from the same sender to that human role. The knowledge-share
path already excludes all three human-only roles; the general notify path
did not.

Fix: a recipient-role guard in notify() via _reject_disallowed_recipient
(folds the new check into the existing CEO-dependency-block return slot so
notify stays under the PLR0911 return limit). Rejects prompter/secretary
with not_authorized; the CEO is human too but acks via the panel, so it stays
an allowed recipient (its only disallowed case, a dependency-block page, is
preserved). TDD test_notify.py (+3: reject prompter, reject secretary, allow
CEO).

* [F049] merge_pull_request: idempotent on already-merged PR (mirror _merge_with_retry)

* [F050] merge_pr_for_task: verify caller pr_number matches task's recorded PR

* [F051] open_conventions_pr: refuse dirty tree + verify checkout-base landed

* [F052] pr_target: scope task lookup by project_id (mirror close_pull_request)

* [F053] _token_for_project: log decryption failure (key rotation) with project slug

* [F054] learnings index: enforce shareable on every shared retrieval path (private-leak fix)

* [F055] messaging: recover from concurrent channel auto-create race via savepoint + re-fetch

* [F056] messaging: lock group row before session check-then-create to prevent active-session orphan race

* [F057] playbook: index/unindex as a post-commit step so the RAG corpus never leads the status transaction

* [F058] release-readiness: non-empty bump plan on first release

_canonical_bump_files derived the bump set from the previous
chore(release): commit. On the first release there is no such commit,
so it returned [] -> assess set version_bump_plan=[] -> the executor
published a tag with no files bumped (a no-op masquerading as X.Y.Z).

Fall back to the version-reference scan when no prior release commit
exists: the files currently embedding the version are exactly the set a
first release must bump, and the set the first release commit then
records as canonical for subsequent releases. Read-only derivation; the
CEO-approval gate and fail-closed executor are untouched.

* [F059] self-heal: hold fix tasks for CEO Approve-&-Start (restore dispatch gate)

The module docstring promised self-heal fix tasks 'wait for the CEO's
Approve-&-Start', but _originate created them confirmed_by_human=True and the
orchestrator dispatched them at once — a self-heal fix that re-broke CI would
trigger another cycle, open another auto-dispatched fix, and loop with no CEO
gate on dispatch.

Restore the documented gate:
* _originate opens the task confirmed_by_human=False (held for the CEO).
* The orchestrator holds a self-heal task out of both the PM and dev dispatch
  paths until confirmed_by_human flips True.
* approve_and_start (the CEO's start gate) sets confirmed_by_human=True so the
  held task finally dispatches (idempotent for board/intake tasks already True).
* list_pending_for_agent scopes the give_me_work hold to self-heal
  (source != self_heal OR confirmed_by_human) so an already-alive PM can't grab
  it pre-approval — while ordinary delegated subtasks (confirmed_by_human=False
  by default, where the delegation IS the authorization) still dispatch.

The 'never self-deploys' guarantee (no merge) is unchanged.

* [F059] fix DB-integration test auth + retype self-heal root code→planning

conftest test-DB defaults matched the project's own running postgres
(roboco/roboco @ localhost:15432, the docker-compose roboco-postgres
service with CREATEDB) instead of the OS user on localhost:5432 which has
no such role — every db_session test failed with InvalidPasswordError
instead of running.

Once the DB connection worked, the self-heal origination DB test went RED
with MAIN_PM_NO_CODE: the self-heal root was task_type=CODE owned by
main_pm, the combo the main_pm_cannot_own_code guard rejects. The Main PM
coordinates the fix (delegates the code work to a cell dev); it has no
code verb. Retyped CODE→PLANNING and rewrote description/AC to
coordination-level.

* [F060] emit reversal audit row on claim-branch-failure rollback

The forward task.claimed audit row is flushed before the branch-creation
attempt, and AuditService commits on its own connection, so the rollback's
flush reverts the task row but not that audit row — the journey's last
event stayed task.claimed while the task reverted to its pre-claim status,
diverging from real state and corrupting downstream cycle-time/bottleneck
metrics. The rollback now emits a CLAIMED->original reversal audit row
(only when the forward transition was made) attributed to the claimant.

* Removing completely unnecessary files (for the repo they are unnecessary)

* [F061] audit status-transition rows now written in-session (F061/F073/F075)

_emit_status_transition_audit now writes AuditLogTable rows into
self.session synchronously (session.add) instead of dispatching
AuditService.log_task_event fire-and-forget on its own connection.

The audit row now commits/rolls back atomically with the status
transition in the caller's transaction, closing three facets at once:
- F061: audit commit no longer decoupled from the transition commit
- F073: a committed transition can no longer have NO audit row
  (the row rides the same transaction; a swallowed persist can't drop it)
- F075: a transition rolled back inside a verb savepoint no longer
  leaves a phantom audit row (the row is in the savepoint too)

log_task_event is now called only from this helper (narrow blast
radius verified); revision_count increment stays at this single
chokepoint. Cycle-time/bottleneck reconstruction from task.<status>
events is no longer silently corruptible.

Tests: test_emit_status_transition_audit_writes_in_session_atomically,
test_finalize_claim_rollback_emits_reversal_audit, escalation-audit
tests retargeted to in-session AuditLogTable rows.

Also: _canonical_bump_files grep-looseness follow-on (F058) -- filter
by subject, not body; git log --grep matches any message line, so a
non-release commit whose body references chore(release): shadowed the
real release commit. Test
test_canonical_bump_files_ignores_body_only_chore_release_match.

* [F061] drop type:ignore from audit-emit tests

Convention: no type:ignore/noqa. The F061 in-session audit-emit
tests used '# type: ignore[assignment]' to assign a MagicMock to
AsyncSession.add, and the F060 test assigned to .flush the same way.

Rewritten to hold a local 'session: MagicMock' variable (mypy sees
its auto-children as MagicMock, so .add.side_effect / .flush assign
cleanly with no suppression). Verified via 'mypy tests/' that both
files are now type-clean (the F060/F061 commits had skipped tests/
in mypy, masking two method-assign errors).

* [chore] clear all 64 pre-existing mypy errors in tests/ (no type:ignore)

Convention: no type:ignore/noqa, and pre-existing violations still
violate. The make-quality gate runs 'mypy roboco/ tests/', but the
prior commits' gates only ran mypy on production files, masking 64
type errors across 15 test files (method-assign, unused-ignore,
no-untyped-def, attr-defined, union-attr, has-type, index, misc).

Fixed without any type:ignore:
- method-assign (svc.session.X = / svc.method = AsyncMock()): hold a
  local 'session: MagicMock'/'AsyncMock' and assert on it, or stub via
  object.__setattr__ / monkeypatch / a typed '_bind' helper returning
  Any, or alias 'cc: Any = c' (the pattern the file already used).
- unused 'type: ignore[assignment]' (real code was method-assign):
  removed; replaced with the no-suppression patterns above.
- 'Callable[...] has no attribute assert_*': keep a typed local ref to
  the AsyncMock and assert on the local, not the method-typed attr.
- no-untyped-def: annotate helper params (Any / pytest.MonkeyPatch).
- attr-defined / index / union-attr: type the helper as Any, narrow
  with an 'is not None' assert, or add the missing attr to a fake.
- has-type / return-value: fix the declared return type to the tuple
  the function actually returns.
- PLC0415 inline imports: hoisted to top-level.

test_pr_gate_notifies_pm._stub_gate_path converted fully to the
'cc: Any = c' alias (it already used it for one attr) so its five
'# type: ignore[method-assign]' suppressions are gone.

mypy tests/: 64 errors -> 0 (538 files). ruff check tests/: clean.
All 84 tests in the touched files pass.

* [chore] remove all remaining type:ignore suppressions from tests/

Converts 115 `# type: ignore[...]` suppressions across 23 test files to
no-suppression patterns (helper-return widening to Any, local Any aliases,
cc:Any aliases, cast at narrow call sites, typed fixtures) so the hard
no-type:ignore convention holds across tests/. No test logic or assertions
changed — only mock-wiring mechanics and type annotations.

Gate: ruff check tests/ clean; mypy tests/ (538 files) clean; 176 changed-file
tests pass. Zero real suppressions remain (the 7 grep hits are 3 hygiene-
checker string-literal test inputs and 4 prose mentions in comments).

* [F062] work_session.merge_pr: idempotency + active-status guard

merge_pr unconditionally set pr_status=merged, pr_merged_at, merged_by,
status=COMPLETED on whatever session it loaded — the only session-terminal
transition in WorkSessionService lacking both the active-status guard
(complete/abandon) and the terminal-idempotency guard (close). Two failure
modes: (1) a retried merge after a successful-but-unconfirmed GitHub merge
overwrote merged_by/pr_merged_at with the retry's actor/timestamp, corrupting
the merge audit trail; (2) merge_pr on an ABANDONED session resurrected it to
COMPLETED, undoing the single-active abandonment. Mirrors close()'s guard:
if status != ACTIVE, return the session unchanged. Both git.py callers await
merge_pr and discard the return, so the no-op is safe. TDD: 3 tests
(happy-path + both modes).

* [F063] workspace._clone_repo: rmtree half-configured clone on failure

If _configure_git raised CalledProcessError before its `remote set-url`
scrub, .git/config kept the tokenized auth URL (the project PAT) and
_assert_no_pat_leak never ran. The except clauses raised WorkspaceError
without removing the workspace, so the next ensure_workspace's health
short-circuit (valid .git with HEAD + objects) skipped past the leak —
mounting the agent on a workspace whose .git/config let it read+exfiltrate
the PAT. Both clone-failure except clauses now rmtree the workspace before
raising, so a half-configured clone is destroyed and ensure_workspace
re-clones from scratch. TDD: 2 tests (configure-failure leak + timeout).

* [F067] flow_main_pm: add missing /triage route

main_pm's manifest advertises triage (lifecycle.intents_for_role(MAIN_PM)
includes it via _PM_ROLES, alongside triage_all) but flow_main_pm.py had no
POST /triage route, so a main_pm agent calling triage hit a raw 404 that
bypassed the per-verb circuit breaker. Added the route mirroring flow_cell_pm's
/triage — wires to the existing team-scoped choreographer.triage (uses pm.team,
works for any PM role; Main PM gets its own team's blocked/awaiting tasks).
Fix direction: add-route, NOT remove-from-manifest — the manifest is spec-correct
(intents_for_role by construction); removing triage would contradict the spec
and leave main_pm with only cross-team triage_all. TDD: test_triage_route_exists_and_dispatches.

* [F068][F069] mcp servers: classify all rejection shapes + envelope 404s

F068: the do/flow-server circuit breaker only counted rejections whose
`error` field was a STRING in _CIRCUIT_REJECTION_KINDS. A 422 validation
failure (no `error` field, a `detail` list) and a 500/HTTPException
(dict-shaped `error` from the exception handlers) both bypassed the breaker
→ unbounded retries on a storm of either. Added _classify_rejection(payload)
(shared, applied to both servers) mapping all three shapes to a counted kind:
string error (existing), dict error → substring-mapped code
(*DENIED*/*AUTHORIZED*/*FORBIDDEN*/*PERMISSION*→not_authorized,
INVALID_INPUT/*VALIDATION*→incomplete_input, *NOT_FOUND*→None parity, else
→invalid_state), 422 detail→incomplete_input. The dict TypeError defence lives
in the classifier (isinstance, never dict-in-frozenset).

F069: a manifest-registered verb whose HTTP route is missing got FastAPI's raw
`{"detail":"Not Found"}` 404 body — a non-envelope payload the breaker
couldn't classify, so a storm bypassed it. _post now synthesizes an
invalid_state Envelope rejection (with a remediate hint → i_am_blocked/i_am_idle)
for a 404 status, routed through _record_and_check_circuit so the breaker counts
it. A 404 that carries a real Envelope (error field present) is surfaced as-is,
preserving test_flow_post_returns_envelope_on_404. TDD: 422/dict/404 tests in
both server test files; updated test_dict_shaped_error_does_not_crash to assert
the SDK is now called with not_authorized (replacing the pass-through assertion
that encoded the bug).

* [F064][F065][F066] websocket: non-blocking fan-out, finally-disconnect, idle timeout

F064: the bridge forwarder awaited every conn.send_text in a gather with no
per-connection queue and no send timeout — one slow WS client back-pressured
ALL event delivery to ALL clients (head-of-line blocking on the listen loop).
Each connect_* now registers a _ClientConnection (bounded asyncio.Queue(256) +
sender task); broadcasts enqueue via put_nowait (drop + structlog warn on
QueueFull) and return immediately. The sender drains the queue with each send
wrapped in wait_for(SEND_TIMEOUT=10s). Unregistered legacy sockets (set
directly into a subscription set, bypassing connect_*) get a timeout-bounded
fallback send task held in _pending_sends (ruff RUF006). disconnect cancels +
drops the sender.

F065: route handlers caught only WebSocketDisconnect with no finally — a
non-clean exit (anyio closed-resource, CancelledError, transport error)
propagated without manager.disconnect, leaking the dead socket into every
subscription set forever. Added finally: manager.disconnect(websocket) to all
5 handlers (disconnect is idempotent).

F066: no server-side heartbeat/idle timeout — a half-open socket from a dead
container blocked receive_text forever and was never reaped. receive_text now
wraps in wait_for(IDLE_TIMEOUT_SECONDS=90s); on TimeoutError, log + fall
through to the F065 finally. Named module constants (no config.py precedent for
WS tuning; callers/tests patch them).

TDD: 22 new tests across 3 files (handler cleanup, idle timeout, send queue),
non-flaky across repeats; 1 existing test adapted with a yield for the new
async fan-out (assertion unchanged). ruff/mypy clean, 421 unit/api tests pass.
No type:ignore/noqa.

* [F022][F023][F024][F025][F026] api: scrub secrets from 422 log, gate a2a/dashboard/orchestrator routes, SSE session-per-query

- middleware: redact known credential fields (git_token/api_key/token/...)
  from the 422 request-validation log line; response body unchanged
- a2a: require_any_authenticated_agent on /message/send + /message/stream;
  subscribe_to_task opens a short-lived session per poll instead of holding
  one asyncpg connection for the full SSE lifetime (pool exhaustion) + auth
- dashboard: gate auditor flag/report mutating routes to Auditor or CEO
- orchestrator: router-level CEO gate on all control routes (spawn/stop/...)

TDD; ruff/mypy clean; 449 unit/api tests green; no type:ignore/noqa.

* [F030] conventions: typescript-scoped custom rules now apply to .tsx files

The validator tags a .tsx file as language 'tsx' (the JSX grammar needs
that tag, distinct from plain 'typescript'), but a custom rule scoped to
'typescript' — the language the scan reports for a React+TS repo — silently
skipped every .tsx file. The two suffix maps were NOT unified: the 'tsx'
tag is load-bearing (grammars.py picks the JSX grammar on it; hygiene.py
keys on it), so unifying would make .tsx fail to parse.

Fix is in check_custom: a one-directional dialect map _DIALECT_OF =
{'tsx': 'typescript'} — a typescript-scoped rule fires on a .tsx file,
but a tsx-scoped (JSX-only) rule still does not fire on plain .ts.

TDD; ruff/mypy clean; 80 unit + 38 integration conventions tests green.

* [F029] websocket: remove broken /api/permissions/check loopback from channel stream

channel_stream called validate_channel_access, which HTTP-loopbacked to
GET /api/permissions/check — a route that does not exist. Every call 404'd
-> False -> the channel stream closed with WS_1008_POLICY_VIOLATION for
EVERY client, so the real-time channel stream was dead. Removed the
function, its call site, and the now-unused httpx + settings imports.

Post-F004 the panel-token gate is the channel-stream authorization (the
CEO panel is the sole WS client and may view every channel), so the
broken loopback is removed rather than replaced with an in-process check
the CEO always passes. The legitimate enforcement.validate_channel_access
(slugs, in-process static ACL) is a different function and is untouched.

F027 is resolved-by-F004 (no code change): all three per-agent streams
gate on _require_panel_token first, so only the authorized CEO panel can
connect — 'any viewer subscribes to any target' is closed.

TDD; ruff/mypy clean; 530 unit/api+enforcement+RBAC tests green.

* [F078] release_executor: deadline every subprocess (git/make/gh/clone)

A hung git/make/gh/clone would block the CEO-gated release loop
indefinitely. Wrap each proc.communicate() in asyncio.wait_for via a
shared _await_proc helper; on expiry proc.kill() the child and return a
non-zero rc (124) so every caller's fail-closed branch fires. Mirrors the
quality-gate _run_one kill-on-timeout idiom.

Deadlines are generous (30min gate / 10min clone / 5min push+gh) so a
legitimate slow op is never wrongly aborted — floor-assertion tests pin
the floors to guard exactly that logical regression. Green path returns
the real rc unchanged.

* [F072] reaper: deadline docker inspect/exec + harden _check_health sweep

A hung Docker daemon (or a stuck container FS) froze the single asyncio
event loop: the reaper runs inline before every dispatch tick and shares
that loop with every background sweeper. Bound each docker subprocess
with asyncio.wait_for; on expiry proc.kill() the child and either raise
(inspect / resolve_container_id — callers apply their own fail-direction)
or return None (the gateway probe — inconclusive, caller declines to act,
matching its existing probe-failure contract). Deadlines generous
(10s inspect / 30s exec) so a legitimate slow docker call is never
wrongly aborted; floor-assertion tests pin the floors.

Also harden _check_health's per-agent loop so one agent's hung inspect
skips that agent, not the whole sweep — preserving the check-all-agents
invariant the timeout-then-raise would otherwise break (without this, a
hung daemon means no agent gets health-checked any tick).

* [F076] say/dm: handler guard rejects all 4 no-comms roles, not just auditor

The say()/dm() defence-in-depth guard only rejected auditor, but CLAUDE.md
mandates the same no-agent-comms invariant for pr_reviewer (posts findings
on the PR), prompter and secretary (human-only, note + evidence). For those
three the manifest was the only gate, so a call bypassing the manifest
(direct API POST, test harness, future routing change) would not be refused
at the handler — admission depended on the agent's slug happening to be
absent from the channel/a2a matrix. Extend the guard to a _NO_COMMS_ROLES
frozenset (auditor + pr_reviewer + prompter + secretary), matching the
explicit role-frozenset gates on commit/notify/pitch/playbook/open_session.
Role-appropriate remediation per role. The claimed defence-in-depth now
covers 4 of 4 silent roles, not 1 of 4.

* [F070] drain fire-and-forget _bg_tasks on shutdown (bounded, data-preserving)

Orchestrator.stop() cancelled only the named loop tasks + agents, then
returned, abandoning in-flight _schedule_bg work. An in-flight
_persist_respawn_record upsert dropped at shutdown meant the last few
gate-mutation strikes never reached the DB; restore_respawn_tracker() on
the next start repopulated a stale lower count and the dispatcher re-burned
the full 4-spawn strike threshold against a still-wedged task — the exact
re-burn the durable tracker exists to stop. Audit-log writes (load-bearing
for cycle-time/rework metrics) were similarly dropped.

Add _drain_bg_tasks(): bounded wait (5s default) lets short DB writes
commit before exit (data preserved), then cancels any stuck task past the
deadline so a hang can't wedge shutdown. return_exceptions=True so one
failing bg task doesn't crash the drain. Wrap the stop_agent loop in
try/except + logger.exception so one bad agent can't skip the drain
(re-introducing the data-loss tail). Floor test pins the deadline >= 3s
so a too-short change can't silently drop a legitimate slow write.

* [F071] abort non-blocking intake/secretary spawn on mid-spawn shutdown

The non-blocking spawn (start_intake_session / start_secretary_session)
schedules _spawn_intake_container_guarded / _spawn_secretary_container_guarded
via _schedule_bg. Those run docker run and only register in _instances at the
END. If shutdown arrived between docker run and the registration line, the
container was started but the orchestrator had no handle — stop() iterates
only _instances, so the container was orphaned (leaked, manual docker rm).
Worse, the F070 drain could let the spawn coroutine complete the
registration AFTER stop() already iterated _instances, landing a live
container into a shutting-down registry nothing tears down.

Add a post-docker-run shutdown guard in _spawn_intake_container and
_spawn_secretary_container: re-check self._running after _run_container_cmd
returns; if the orchestrator began shutting down, remove the just-started
container (by its deterministic name) and raise _SpawnAbortedDuringShutdown
WITHOUT registering. The guarded wrappers catch that BEFORE except Exception
and close the live relay silently (shutdown is not a user-facing failure,
no error pushed to the SSE stream). The F070 stop() drain awaits the bg
spawn coroutine, so the abort surfaces cleanly.

TOCTOU-safe: between the _running check and the _instances assignment there
is no await (config + instance construction are sync), so once the check
passes, registration completes before the event loop can interleave stop().
The normal running path is unchanged (sanity tests pin it).

* [F074] per-agent advisory lock closes claim TOCTOU

_run_claim_guards read the agent's other tasks via unlocked SELECTs
before claim() took its row lock, and claim()'s FOR UPDATE locked only
the TARGET row — so two concurrent i_will_work_on by the SAME agent on
TWO DIFFERENT pending tasks each locked their own row, each read an
empty in_progress set, each passed already_active, each claimed+started
→ the agent ended with two in_progress tasks (the in-process asyncio
Lock is lost on orchestrator-restart split-brain, so it wasn't a
DB-level guarantee).

Fix: TaskService.acquire_claim_lock takes a transaction-scoped
pg_advisory_xact_lock keyed by hashtextextended(agent_id). The gate
acquires it BEFORE the guard reads (for non-coordinator roles only) so
the second concurrent claim's read sees the first's committed
in_progress task and is rejected. Tx-scoped → auto-releases on
commit/rollback, can't outlive the request.

Coordinator exemption (the key logical-regression guard): cell_pm /
main_pm do NOT take the lock — the PM coordinator concurrency feature
lets a PM plan+delegate many roots in parallel, and a per-agent lock
would serialize those claims and regress it. Matches the existing
_COORDINATOR_ROLES already_active/paused guard exemption. A hash
collision only causes benign false serialization, never a false
negative.

Tests: unit (dev acquires lock before guard read; coordinator does
not) + real-PG integration (same-agent serializes, different-agent
does not, releases on rollback).

* [F021] handle SSE transport errors so the intake composer isn't stuck

openStream registered listeners for the server-sent event kinds but not
the EventSource's own transport-level error. The 'error' kind IS in
LIVE_EVENT_KINDS, so a server-sent event:error (JSON MessageEvent) was
handled — but a dropped connection / dead session fires a plain Event
with NO data, which JSON.parse(undefined) swallowed in the try/catch,
so the stream 'stayed open' (EventSource loop-reconnected a session that
no longer existed) and isSending stayed true — the composer was
permanently disabled.

Fix: route the 'error' event by payload. A MessageEvent with string
data is a server-sent error → handleEvent (unchanged). A no-data Event
is a transport error → handleTransportError: clear streamingId/activity,
set isSending false, add a 'connection lost' error message, keep a
draft/batch preview up (so the human can still act on a proposed card)
else land on 'chatting', and close the dead stream so EventSource stops
loop-reconnecting.

Tests: renderHook + a jsdom EventSource double that fires a transport
error (plain Event, no data) vs a server-sent error (MessageEvent +
JSON). RED: transport error left isSending true; GREEN: resets to
false, surfaces the message, closes the stream. The server-sent-JSON
path is unchanged. Full panel suite (129) green; eslint/typecheck/prettier clean.

* [F081] Approve dialog: label notes required (>=20 chars), not optional

The CEO Approve dialog's notes label fell into the default branch
('Notes (optional') for the approve action, but approve actually
requires substantive notes >= 20 chars — enforced client-side
(toast error on < 20) and server-side. So the CEO was told 'optional'
and only learned the real requirement from a toast after hitting
submit with empty notes.

approve and start both require >= 20 chars; reject only requires a
reason. Collapse the label to two branches: reject -> 'Reason for
rejection (required)'; everything else (approve + start) ->
'Approval notes (required, >= 20 characters)'. The approve
placeholder now also signals intent ('Why this is ready to ship...').

Tests: render the queue, click Approve, assert the notes label says
'required' + '20' and does NOT say 'optional'. RED: label read
'Notes (optional)'; GREEN: 'Approval notes (required, >= 20
characters)'. eslint/typecheck/prettier clean.

* [F082] surface release-proposal query failures instead of silent hide

The card collapsed any non-404 backend failure (500 / network drop) onto
`!proposal` and returned null, so the CEO had no idea the release-proposal
endpoint was unreachable. Distinguish the cases: isError + a Retry affordance
vs the 404 null empty state that stays hidden. Mirrors PrReviewQueue.

* [F083] clear stale usage snapshot when /ws/system leaves connected

The hook synced wsState into the store but never dropped usageData when the
stream dropped, so on reconnect wsState flipped to "connected" before any
fresh USAGE_SNAPSHOT arrived and UsageOverviewPanel rendered the prior
session's totals/cost as if they were live. Clear usageData whenever state
leaves "connected" so the panel falls back to the polling summary until a
new snapshot lands. Connected->connected is a no-op clear skip.

* [F084] scope per-control disable to the in-flight mutation, not all

FeatureFlagsCard disabled every switch while any one flag toggle was pending,
and PlaybookReviewQueue disabled every row's Approve while any one approve was
pending — so the operator couldn't act on an independent control during a
slow round-trip. Gate the disable on the in-flight mutation's variables
(matching key / id) so only the control being mutated locks; the others stay
usable. The same-flag double-tap protection is preserved.

* [F085] reject submitting both project_id and product_id

validate() only checked 'at least one of project/product', so the dialog let
both be submitted together. The server silently lets product_id win at routing
and drops project_id, recording a misleading, never-used repo. Add a validator
that refuses the ambiguous submit with a clear error. The at-least-one rule and
the single-pick submit paths are unchanged.

* [F020] kanban: confirm admin-override drags that skip lifecycle preconditions

A drag on the operator kanban routes the status move through the admin
status-override, which bypasses the in-band lifecycle validator entirely.
That override is intentional (it's how an operator recovers a wedged task)
but it also let a careless drag skip material preconditions silently —
completing a task with no open PR, QA-bypassing, finishing docs on a task
whose docs aren't complete.

Leave the override intact but make the bypass explicit: compute the
preconditions the dragged move would skip (open PR, docs complete,
self-verified + commits + progress for submit-qa, visible non-terminal
subtasks for coordination-root targets) and, when any are skipped, hold the
move behind a confirmation dialog that lists exactly what's being skipped.
Precision over recall — only warn on what the panel can verify from the
task and its in-list children; never fabricate a 'satisfied' claim, and
stay silent on benign transitions that gate on nothing we can check.

The admin status-override capability is preserved (Confirm still fires it);
this only surfaces the bypass instead of letting it happen silently. Does
not touch the master-merge invariant — the board's updateTask is the
operator override, not the Main-PM merge path.

* [F086] prompter: restore parked cell content on project toggle off/on

rebuildCellWork appended a blank {summary:'', items:[]} entry for a newly-
selected cell, so toggling a cell's project OFF then back ON in the MegaTask
review card discarded the agent-authored per-cell summary/items — the entry
was dropped on toggle-off and re-added blank on toggle-on.

Park each draft's last per-cell content in client-only BatchProposal state
(parkedCellWork, keyed by draft index — never sent to the backend; confirm
ships only title/drafts/project_ids/route, and it ride-alongs into the
localStorage persist slice so the restore survives a reload mid-review).
rebuildCellWork gains an optional priorByCell map: a re-added cell with no
live entry restores its parked summary/items (with the new project_id) in-
stead of blanking; a live entry still wins over a stale parked copy so an
in-place edit is never regressed. parkCellWork is the pure merge seam
(prevParked seeds, live work overwrites) the setBatchDraftProjects updater
calls — kept pure so the updater stays a thin caller.

Tests: rebuildCellWork restore/blank-fallback/live-wins + parkCellWork
retain/overwrite/merge (6 new), 19 GREEN. eslint/typecheck/prettier clean.
No wire-payload change, no regression to the fill/drop/one-repo-per-cell
invariants.

* Updated domain

* [F087,F088] enforce panel token on live-chat bridges (Phase 5)

Add a CEO-bound, header-token-only gate (require_panel_token) at the route
level of the prompter_live + secretary_live bridges, which were the only
panel-facing API surface that ran unauthenticated. It mirrors the WS
_require_panel_token and _check_agent_auth_token contracts: in dev
(ROBOCO_AGENT_AUTH_REQUIRED unset) a missing token is allowed; a
presented-but-forged token is rejected even in dev; in prod nginx already
injects the CEO-signed X-Agent-Token on /api/ for GET + POST, so the SSE
stream (EventSource can't set headers) and the POSTs are now checked instead
of anonymous. Applied to start/stream/status/messages/stop on both routers;
preview_live_batch switched from CurrentAgentContext+noqa to the route-level
gate (genuinely auth-only). confirm/confirm-batch/re-interview keep
CurrentAgentContext (they use agent.identity). The container->relay /events
callback is intentionally left ungated (internal Docker network, opaque
session id) — gated by a test sentinel so Option B (spawn+SDK token wiring)
is a deliberate future decision. No panel/nginx/spawn/SDK changes; master
merge invariant untouched. 22 new TDD auth tests, 492 api tests green.

* [F089] honest WorkSession agent_id nullability across the read path

The work_sessions.agent_id column is nullable=True with ondelete=SET
NULL — deleting an agent nulls the FK on every session it ever held. The
ORM annotation lied (Mapped[UUID] non-optional), the converter papered
over the lie (typing_cast to a non-optional UUID), and the response
model rejected None outright (WorkSessionResponse.agent_id: UUID). A
session whose agent had been deleted crashed the GET endpoint with a
pydantic ValidationError instead of serializing agent_id: null.

Make the read path honest end-to-end:
- WorkSessionTable.agent_id: Mapped[UUID | None] (matches the column).
- WorkSessionResponse.agent_id: UUID | None (serializes null, no crash).
- session_to_response passes agent_id via typing_cast('UUID | None', ...)
  to bridge SQLAlchemy's UUID[Any] to stdlib uuid.UUID while preserving
  None-ness (the cast stays for the same mypy-plugin reason every other
  field uses one; it no longer narrows away None).

WorkSessionCreate.agent_id stays UUID — at create time the claiming
agent is always known. The unused WorkSession pydantic read model is
left as-is (never materialized from a DB row). task.py:_needs_revision_dev
already None-guards ws.agent_id via to_python_uuid (returns None -> skip).

* [F090] drop auditor from write_roles on main-pm-board / board-private

The auditor is a silent, read-only observer on every channel, but the
channel catalog (roboco/foundation/policy/communications.py) listed it
in write_roles for main-pm-board and board-private 'for parity' with the
legacy CHANNEL_ACCESS table, while the actual silent-observer rule was
enforced only at the say/dm guard (content_actions._NO_COMMS_ROLES) and
PermissionService.can_write_channel's auditor short-circuit.

That left the catalog-only enforcement path — the HTTP messaging route
(messages.py send_message -> validate_channel_access) — authorizing an
auditor write that both the say/dm guard and PermissionService would
have blocked. A reader of the catalog also believed the auditor could
post to those channels, which is false.

Fix: remove Role.AUDITOR from write_roles on both channels (main-pm
+ board remain writers; ceo remains a writer on board-private). The
auditor stays in read_roles, so its silent read is unchanged. silent_roles
is left empty (matches the announcements precedent: auditor reads via
read_roles, not the silent bucket) — the DB seed and silent_observers
field are untouched.

Logical-regression check: the auditor's read access on both channels
is byte-for-byte preserved (still in read_roles, so validate_channel_access
read returns True via the direct list); the legitimate writers (main-pm,
product-owner, head-marketing, ceo) are untouched; CHANNEL_ACCESS is
derived from the spec so the foundation/seed drift tests self-adjust;
PermissionService.can_write_channel already short-circuited auditor to
False everywhere, so no behavior change there; AUDITOR_SILENT_ACCESS is
unchanged (auditor not added to silent_roles -> no DB silent_observers
change -> no group-access behavior change); the say/dm _NO_COMMS_ROLES
guard is unchanged. Tests: 3 new in test_channel_access.py — auditor
write on main-pm-board/board-private now raises ChannelAccessDeniedError
(RED before: returned True), auditor read still True, main-pm/ceo still
write.

* [F091] warn at spawn time when host grok auth.json is missing

GrokCliProvider._append_grok_auth_mount silently skipped the mount when
the host ~/.grok/auth.json was absent. The spawn still succeeded (docker
run returned 0 — the container was created), so the operator had no
spawn-time signal that the agent was doomed: the entrypoint's
`python -m roboco.llm.providers.grok_auth --check` backstop then
refused to start (exit 78) and the failure only surfaced later via the
container's log markers.

Fix: emit a spawn-time WARNING (module logger) naming the missing file
and the remediation (`grok login` on the host, or set
ROBOCO_HOST_GROK_DIR) when the mount is skipped. The spawn outcome is
unchanged — the container still starts and the existing exit-78 -> park
flow (F041) still catches it — but the operator now sees the missing
credential immediately instead of diagnosing a later exit-78.

Logical-regression check: the mount-present path is byte-for-byte
unchanged (auth.json exists -> the -v bind is appended, no warning); the
spawn still succeeds when auth is absent (no raise — the existing
test_grok_spawn_omits_auth_mount_when_absent still passes: no mount, no
crash); the exit-78 entrypoint backstop and the orchestrator's
exit-78-park handling (F041) are untouched; a module-level logger adds no
side effects. Tests: new test_grok_spawn_warns_when_auth_absent uses
caplog to assert a WARNING mentioning auth.json + `grok login` is
emitted on a missing-credential spawn (RED before: no warning; GREEN
after). 102 grok tests green; ruff/mypy clean.

* [F092] decode JWT exp when refresh omits expires_in

xAI's refresh-token response sometimes omits expires_in. Without it the
new access token kept the stale pre-refresh expires_at, so is_valid /
--check forever rejected a fresh token — and the refresh loop re-rotated
the single-use refresh token every tick, killing the credential (F006).

The access token is a JWT whose exp is the authoritative expiry: decode it
when expires_in is absent. Fallback to the documented ~6h TTL + a structlog
warning when the JWT exp is unreadable, so a fresh token is treated as live
instead of stale.

* [F093] serialize concurrent live-chat spawns under a per-agent lock

The intake and secretary agent ids are each a single fixed id, so two
concurrent start_intake_session / start_secretary_session calls raced on
the container name (docker run --name roboco-agent-<id>) and the
_instances[<id>] write: both passed the reap-prior check before either
registered, both ran docker run, and the last _instances write won,
orphaning the other container + its relay.

Add _intake_spawn_lock / _secretary_spawn_lock (asyncio.Lock) and wrap the
_spawn_intake_container / _spawn_secretary_container bodies so the second
start waits for the first to fully register before its own reap-prior check
runs. Distinct from self._lock (which stop_agent takes) to avoid a
reentrancy deadlock: the spawn body holds the spawn lock then calls
stop_agent (acquires self._lock) — lock order is always spawn_lock ->
self._lock, never the reverse.

* [F094] add a persistent-probe-failure escape hatch to provider parking

_on_probe_failure only incremented the failure counter and, at 10 failures,
sent a one-shot CEO notification. It never cleared the tracker, never gave
up, never fell back to time-expiry. _do_probe returns False for any non-2xx
AND any httpx error, so a permanently unreachable probe endpoint (removed
API key, network partition to the probe host, misconfigured base URL) kept
the provider parked forever — every agent on it gated by
_provider_spawn_parked, their tasks reaped to pending but the spawn gate
queuing every spawn, sitting pending forever. The only recovery was the
operator manually clearing the Redis key.

Past _PROBE_GIVE_UP_THRESHOLD (30) persistent failures, fall back to the
same time-expiry optimism the unprobeable-provider path uses (_do_probe
returns True when there is no probe URL): clear the park and resume parked
agents. If the provider is genuinely still down the real workload attempts
re-park via the 429/5xx path, so this is bounded burn — strictly better
than a silent forever-strand. Kept above the CEO-notify threshold (10) so
the operator still gets the notification first.

* [F095] orchestrator: parked-provider spawn short-circuits before expensive prepare

spawn_agent ran the full _prepare_agent_spawn (writes blueprint/settings/
briefing/MCP files, ensures the image, registers a STARTING instance) every
dispatcher tick only to bail at the after-prepare parked-provider check —
wasting all that file I/O while the provider stayed parked and leaving a
STARTING instance registered then downgraded to OFFLINE.

Move the parked check before _prepare_agent_spawn: resolve the route cheaply
via _resolve_agent_route (only provider_type is needed) and bail with a
minimal unregistered OFFLINE instance. The existing-running check stays
first (inside the lock) so a live agent is never replaced; a TOCTOU
re-check guards the unlocked window before prepare; the after-prepare
check is kept as a rare-race defense (a park landing during prepare).

* [F096] orchestrator: serialize fire-and-forget respawn persists per commit order

_persist_respawn_record is fire-and-forget per gate mutation; a respawn loop
fires count 1->2->3->4 in quick succession, scheduling one persist per
increment for the same (agent_slug, task_id). The ON CONFLICT DO UPDATE upsert
is row-level race-free, but the fire-and-forget tasks can still COMMIT out of
order: a slow stale persist (count=2) scheduled first can resolve AFTER a fast
fresh one (count=4) scheduled second, leaving the durable row at the stale low
count and re-burning the strike threshold on restart.

Fix: acquire self._respawn_persist_lock (new asyncio.Lock) as the FIRST await
in _persist_respawn_record, so acquisition order = task creation order (FIFO
ready queue) = logical schedule order, and commits land in that order. The
durable row always ends at the latest logical value. The lock lives in the bg
task, so the dispatcher hot path never blocks; persists are best-effort and
a slow one queuing the rest just delays the durable catch-up (in-memory record
stays authoritative).

* [F097] orchestrator: back off grok re-park retry_after within a rate-limit episode

_probe_target returns (None, {}) for grok — the grok CLI's xAI endpoint is
closed and the SuperGrok OIDC access token is not a valid bearer for the metered
api.x.ai, so a real probe would either no-op or strand grok parked forever.
_do_probe treats url-is-None as success (time-expiry optimism), so once the
60s retry_after passes the probe loop optimistically clears the grok park, a
cleared park dispatches a fresh grok agent that hits the still-active xAI 429,
exits 75, and re-parks — a flat ~90s crash-retry cycle for the whole xAI
rate-limit window (each cycle costs container startup + a rejected grok call).

Fix: track _grok_repark_count + _grok_last_park_at in _park_grok_rate_limited
and back the re-park retry_after off exponentially within one episode
(60 -> 120 -> 240 -> ... capped at 2**4 = ~16min cycle) so the churn dampens. A
gap past _GROK_REPARK_EPISODE_GAP_S (25min, > the capped cycle) means no re-park
for that long => the rate limit actually lifted => a fresh episode resets the
count to the base 60s, so recovery latency isn't penalized across episodes.
The first park in a fresh episode is unchanged at 60s.

* [F098] orchestrator: keep waiting record through a re-park during probe-success resume

resolve_wait deleted the waiting record (in-memory + durable) BEFORE calling
spawn_agent. A re-park in the window between the probe-success clear and the
spawn — the provider's rate limit lifts then immediately re-limits, or a second
provider limit lands — bails spawn with an OFFLINE instance (the parked-provider
short-circuit). Deleting the record first orphaned the agent: with no record
the probe-resume loop can never revive it and the spawn gate bails every tick,
so the agent is lost until the operator intervenes.

Fix: spawn first, then tear down the record only once a container actually
launched (instance.state == ACTIVE). On an OFFLINE bail the record stays so the
next probe-success re-attempts the resume. On a spawn EXCEPTION the record is
torn down + re-raised so the probe loop doesn't keep re-resuming a task that
moved to a different state (e.g. readiness refused -> task auto-blocked) —
matching the pre-fix behavior where the record was deleted before the spawn.

* [F099] wire pr_pass/pr_fail self_review block in the spec gate

The pr_pass/pr_fail ActionSpecs carry self_review_block=True, but
_gate_preflight never populated Context.original_developer_slug, and
actor_slug was read off agent.slug — which GatewayAgentView does not
carry, so it was always None in production. The block was structurally
dormant: a reviewer who was also the original developer of the
assembled PR could pass (or fail) their own work. The service-layer
_validate_not_self_review backstop only covers qa/documenter, not
pr_reviewer, so the spec gate is the only defense.

Set actor_slug=str(reviewer_agent_id) (GatewayAgentView has no slug,
so the UUID is the identity) and original_developer_slug from the
original_developer marker (a UUID stored as a string). Both resolve to
UUID strings, so the spec's string-equality comparison fires when the
reviewer IS the recorded original developer.

The marker is never set on assembled coordination tasks (only on
dev-leaf tasks at QA/doc claim), so the block stays dormant by design
in production — but the gate is now correctly wired to fire if the
marker were ever set to the reviewer. Zero production behavior change;
the dormant-in-production state is pinned by the no-marker test.

* [F100] atomic Redis probe-failure counter via server-side Lua

increment_probe_failures / reset_probe_failures did a non-atomic
get_state (GET) -> mutate -> set (SET) in Python. A concurrent
activate() re-park writes a FRESH episode blob (probe_failures: 0 +
fresh activated_at / retry_after / affected_agents / kind); if the
stale increment's SET landed after the fresh activate's SET, the stale
blob overwrote the fresh episode metadata AND un-reset the counter
(clobbering the new episode).

Redis single-threads a Lua EVAL, so a server-side read-modify-write
is indivisible: activate's SET is serialized entirely before or after
the script, never interleaved between the script's GET and SET. The
two scripts mutate ONLY probe_failures, so every other episode field
survives the bump. activate stays a single atomic SET (a fresh episode
resetting the counter to 0 is correct semantics).

* [F101] enforce PR-open state gate on gateway open_pr (parity with HTTP path)

* [F102] make project_id mandatory on pr_target (close cross-repo pr_number collision)

* [F103] make project_id mandatory on close_pull_request (close cross-repo collision)

* [F104] fail-closed on conventions resolution errors (block gate no longer silently disabled)

* [F106] compound (timestamp, id) keyset cursor for message pagination

get_messages used strict timestamp inequalities with a non-deterministic
order_by(timestamp.desc()), so equal-timestamp messages were cut by limit
on one page and excluded (strict < T / > T) from the next — they vanished
across pages. Bundled the (timestamp, id) pair into a MessageCursor dataclass
so the next page resumes exactly past the cursor's id at the shared
timestamp (or_: strictly-older OR same-timestamp-smaller-id for before; the
mirror for after), with a deterministic order_by(timestamp.desc(), id.desc())
so the last-item cursor is unambiguous. id is None for a legacy timestamp-
only cursor (strict inequality, prior behavior). The route builds cursors
from the flat before/before_id + after/after_id HTTP params; the schema now
carries the tie-breaker ids. Also clears PLR0913 (cursors replace the
before_id/after_id params).

* [F107] defer Redis bus publish until DB commit (no phantom notifications)

deliver() and _persist_and_deliver() ran inside the caller's open
transaction: the notification row was flushed but not committed, yet
NOTIFICATION_SENT was published to the Redis bus immediately. A commit
failure (DB hiccup, constraint, asyncpg error) rolled the row back while
connected WebSocket clients had already received a push for an id that
no longer existed — a phantom notification (notify_get -> NotFoundError).

Added a deferred-publish (transactional-outbox) helper: defer_bus_publish
enqueues the event on session.info and registers one-shot after_commit /
after_rollback listeners on session.sync_session the first time it is
called for that session. On commit, the after_commit listener schedules
the async drain via asyncio.create_task on the running loop (the listener
fires synchronously inside await AsyncSession.commit, so the loop is
active); the task handles are stashed on the session so callers/tests can
await them. On rollback, after_rollback drops the pending queue — a
rolled-back txn emits nothing. deliver() now builds the per-recipient
events up front (data materialized to strings, so deferral is safe even
if the ORM object later expires) and defers each; the delivered_at DB
marker stays in-tx (rolls back with the row). The bus block stays
best-effort (try/except + log) so a bus-init failure never propagates or
rolls back the notification row — matching the prior inline semantics.

This fixes every deliver/_persist_and_deliver caller at once (the two
cited in F107 plus the orchestrator + task.py deliver sites), since they
all commit the session afterward (the deferred publish fires on that
commit; the row is durable by the time the event goes out).

* [F108] atomic replace_chunks: single-txn delete+insert closes reindex race

* [F109] playbook curation status guards: approve/reject draft-only, archive approved-only

* [F110] draft slug TOCTOU: catch IntegrityError on flush -> ConflictError (no 500)

* [F113] collapse WorkSession creation to the validated service path

_create_work_session_if_needed constructed WorkSessionTable directly,
duplicating WorkSessionService.create's validation (existing-active
check, single-active-per-task supersede, project/task existence). The
two sites had drifted. Route through WorkSessionService.create instead,
mapping ConflictError to the idempotent 'if needed' None. Remove the
now-dead _supersede_other_active_sessions (create's
supersede_active_sessions_for_task replaces it).

Fix three pre-existing RED tests surfaced by the sweep (all confirmed
failing on the F110 commit before this change):
- test_fail_qa_work_session_fallback_excludes_qa_session: inserted two
  ACTIVE work_sessions per task, violating uq_work_sessions_one_active
  _per_task (migration 047). The QA session is now ABANDONED — still in
  the fallback query's result set (the query filters by task_id +
  agent_id, not status), so the exclude filter (agent_id != qa_id) is
  still exercised and the dev is resolved.
- test_ceo_reject_routes_coordination_task_to_main_pm /
  test_ceo_reject_routes_batch_umbrella_to_main_pm: ceo_reject emits an
  audit row keyed to CEO_AGENT_ID, but the tests never seeded the CEO
  agent row (fk_audit_log_agent_id_agents). Seed the CEO agent (get-or-
  create, mirroring test_ceo_reject_writes_handoff_journal).

* [F114] single-claimant guard on pr_gate_claim

pr_gate_claim delegated straight to _qa_or_doc_claim, which overwrites
claimed_by / active_claimant_id with no single-claimant check. Two
reviewers race-claiming the same awaiting_pr_review task would
last-write-wins overwrite the first claim, and the first reviewer's
subsequent pr_pass / pr_fail would actor-mismatch against the new owner
(wasting a review cycle). The orchestrator's gate dispatcher already
prevents double-reviewer-dispatch in normal flow (one task -> one team
-> one reviewer + is_agent_active + per-tick spawned set), so the race
is only reachable via direct concurrent API calls (defense-in-depth).

Add a role-aware single-claimant guard in pr_gate_claim: lock the row
FOR UPDATE (serialize concurrent claims, mirroring the dev claim path),
then refuse only when the task is already actively claimed by a
DIFFERENT PR-reviewer. The gate task is owned by the PM at entry
(submit_for_review does not clear ownership, unlike submit_for_qa), so
the guard must distinguish a PM/dev owner — which the first reviewer
legitimately overclaims — from a competing reviewer claim; checking the
existing claimant's role (pr_reviewer) does exactly that. A re-claim by
the same reviewer is idempotent (skipped by the != check). The gateway
claim_gate_review handler already maps a None return to a clean
invalid_state envelope ('it may already be claimed; give_me_work for
the next'), so no gateway change is needed.

TDD: 3 integration tests in test_task_service_basics.py — reject a second
reviewer race-claim (returns None, first claim intact), allow the first
reviewer when the PM owns the root (regression guard for the
PM-owns-at-entry model), idempotent re-claim by the same reviewer.
Confirmed the reject test RED first (race-claim succeeded, overwriting
reviewer1).

* [F115] sample monorepo per (repo,workflow)/(repo,command) not per repo

The CI-watch and dep-update loaders collapsed a monorepo's cell-projects
to one canonical entry per repo (slug-sorted-first), so a repo whose cells
each carry their OWN ci_watch_workflow / dep_update_command had only the
canonical cell's workflow/command sampled — a red on another cell's
workflow or drift on another cell's lockfile was missed (under-count).

Refactor the shared one-per-repo collapse into _projects_one_per_key, keyed
by repo identity for external-PR discovery (unchanged: one review per PR per
repo), by (repo, effective workflow) for CI-watch, and by (repo, command)
for dep-update. Each distinct workflow/command is now sampled once; the
engines' per-git_url fix-task dedup still prevents duplicate fix tasks for
the same repo. _projects_one_per_repo now delegates to _projects_one_per_key.

key_fn uses a string annotation (Callable lives under TYPE_CHECKING, like
the existing Coroutine/Iterable annotations at lines 4193/5279).

* [R115] originate ci_watch/dep_update fix tasks as PLANNING coordination roots

The Main-PM-code-impossibility guard (commit e202ce39, Thread 4 of this
audit) made team=MAIN_PM + task_type=CODE impossible — a Main PM coordinates,
it does not write code. But the ci_watch and dep_update engines still
originated their fix tasks as task_type=TaskType.CODE assigned to main-pm,
so task_svc.create raised MAIN_PM_NO_CODE and NO fix task was ever opened
— a regression introduced by the earlier audit fix (confirmed: the engine
tests pass at e202ce39~1 and fail at HEAD).

Mirror the hardened self_heal_engine precedent (self_heal_engine.py:197)
which already uses task_type=TaskType.PLANNING for its Main-PM coordination
root with an explicit 'decompose the fix and delegate the code work to a
cell dev — the Main PM does not write the fix itself' description. Both
engines now originate PLANNING coordination roots with matching delegation
guidance in the description + acceptance criteria. confirmed_by_human
stays True for both (they ride the normal delivery flow without the CEO
gate, unlike self-heal — intentional per the architecture).

The dedupe/open-cap queries (list_open_ci_watch_tasks /
list_open_dep_update_tasks) key on source + non-terminal status + git_url,
NOT task_type, so the type change does not break dedup (still one open fix
task per repo).

The two source-test fixtures (test_ci_watch_source / test_dep_update_source)
created CODE+MAIN_PM tasks directly to exercise the listing queries — same
guard violation; switched to PLANNING (the queries assert on source/status,
not task_type, so the fixture type matches the engines' corrected type).

* [F116] hold the read-clone lock across the dep-probe local clone

dry_upgrade_changes_lockfile called ensure_read_clone (which syncs the
read clone under the _meta-conventions lock then releases it) and ran
'git clone --local --no-hardlinks <read_clone>' OUTSIDE the lock. A
concurrent ensure_read_clone -> _sync_read_clone (fetch + hard-reset to
origin's default branch) could mutate the read clone's working tree /
object db mid-clone, racing the clone and producing an inconsistent or
failing probe.

Split _probe_lockfile_change into _clone_local_into (the local clone,
run under the read-clone lock) + _probe_lockfile_on_clone (the upgrade +
git status, run without the lock on the now-independent copy). The probe
acquires _ensure_lock_for(slug, '_meta-conventions') — the same lock
ensure_read_clone syncs under — and holds it only for the clone step; the
upgrade operates on the full --no-hardlinks copy and never touches the
read clone, so the lock is released before it to avoid blocking
conventions reads for the upgrade duration.

The tiny gap between ensure_read_clone releasing the lock and the probe
re-acquiring it is safe: any concurrent _sync_read_clone completes under
the lock before the probe acquires, so the clone reads a stable state.

* [F117] stop the orchestrator in lifespan shutdown BEFORE closing the DB

The lifespan shutdown closed OptimalService + the DB, and only THEN did
bootstrap's finally block call orchestrator.stop() — so stop() ran with
the DB already closed. stop() drains fire-and-forget _bg_tasks writes
(respawn_tracker upserts, audit-log rows) and stop_agent finalizes work
sessions / agent state, all needing the DB still open; closing it first
silently dropped those final writes (the durable PM-respawn counter's
last few strikes, the metrics-bearing audit trail tail).

Move orchestrator.stop() into the lifespan shutdown path, BEFORE
close_optimal_service + close_db, guarded by a new get_orchestrator_or_none()
safe accessor (no crash when no orchestrator is wired — tests,
skip_orchestrator). bootstrap's finally-block stop() becomes an idempotent
safety net: stop() gains a _stopped flag (getattr-guarded so __new__-
constructed test instances still stop) so the double-call is a clean no-op,
not a re-stop of already-stopped agents / re-drain of an empty bg set.

* [F118] coerce a lone-string where_to_look into a list

where_to_look is a list-typed handoff field like consequences/next_steps
but was the only one NOT in the _wrap_scalar_in_list field_validator. A
well-intentioned where_to_look='src/api/' 422'd at the route with no
remediation envelope, and the agent's retry loop tripped the do-server
circuit breaker — the exact failure mode the other list fields were
hardened against. Add it to the mode='before' validator so a lone string
is wrapped into a one-element list before type coercion.

* [F119] sender reaps dead sockets on send error instead of waiting for receive idle timeout

* [F120] release a stopped agent's claimed task immediately on budget-kill/shutdown

* [F122] name the already-open PR in submit_up's None-state remediate

submit_up's create_pr pre-side-effect opens the cell→root PR BEFORE
submit_for_review runs (its pr_created gate requires it — lifecycle.py:1338-1343).
When submit_for_review returns None (a concurrent state change raced the task
out of in_progress between the precondition gate and the composed action), the
old remediate ('check task state — must be in_progress with PR ready') hid
that the PR was already open on GitHub — an orphaned external artifact the PM
could not reconcile. Mirror submit_root's F016 None-envelope remediate: name
the open PR, point the PM at re-fetch + reconcile, and note create_pr is
idempotent so a re-issue re-attaches to the existing PR (no duplicate). Pure
message improvement — zero behavior change; reordering is off the table
(create_pr must precede the pr_created gate).

* [F124] re-check dependency state before releasing a dependency-blocked claim

The unmet_dependency guard read dependency state via an unlocked SELECT, then
fired release_dependency_blocked_claim (a state mutation: claimed/in_progress
-> pending, clears branch_name, abandons WorkSession) as a side-effect BEFORE
returning the rejection. An upstream dependency that reached a terminal state
(completed/cancelled) in the microseconds between the read and the release left
the task NEEDLESSLY released — its branch cleared + WorkSession abandoned +
assignee bounced, only to be re-dispatched + re-claimed when the dependency-
completion re-dispatch fired a moment later.

Re-check unmet_dependency_ids immediately before the release and skip it
(returning None — proceed) when the upstream just completed. Dependencies are
monotonic (unmet -> met only; terminal states never reopen), so a fresh read
that now finds them met stays met: safe to proceed without releasing. The
'still unmet' path is byte-for-byte the prior behavior (no regression). The
cross-task residual window (upstream completes between the re-check and the
release) is not closable by a row lock on the dependent, but the re-check
narrows the window from [first read -> release] to [re-check -> release], and
in the common case the first read already sees met (no guard fires). No
committed-work loss either way (a dependency-blocked task has none; the branch
ref + commits persist across the branch_name clear).

* [F125] serialize same-parent delegate via per-parent advisory lock

The delegate sibling-dedup guard read the parent's existing subtasks via an
unlocked get_subtasks SELECT (the dedup read) then created the subtask (the
write) with no DB serialization between them. Two concurrent delegate calls
for the same parent (PM re-delegating while a reaper re-dispatches, or two
orchestrator ticks racing) each read a duplicate-free sibling set, each passed
the dedup guard, and each created a subtask — the parent got the duplicate the
guard exists to prevent (the smoke-run runaway pattern).

Fix: a PostgreSQL transaction-scoped advisory lock keyed by the parent task
id (seed 1, disjoint from the per-agent claim lock's seed 0), acquired at the
top of the delegate body before the first get_subtasks read (the briefing
context read AND the dedup sibling read) and held through create_subtask's
flush + the outer request commit. The second concurrent same-parent delegate
blocks until the first commits, then its dedup read sees the committed
sibling and is rejected.

Per-PARENT (not per-agent): a coordinator PM legitimately delegates many
subtasks under one parent in quick succession and plans many roots in
parallel — a per-agent lock would serialize all of a PM's delegates and
regress the PM coordinator concurrency feature. The per-parent lock
serializes only same-parent delegates (the dedup invariant is per-parent)
and leaves different parents untouched.

TDD: red-first ordering test (lock acquired before first get_subtasks read
and before create_subtask) + no-regression test (create still runs).

* [F127] per-task advisory lock prevents open_pr milestone double-emit

open_pr's idempotent re-entry guard (pr_number is not None) read t.pr_number
from an unlocked fetch. Two concurrent same-task open_pr calls (the
alive-but-unresponsive respawn race) both fetched pr_number=None, both passed
the guard, both ran the runner (GitHub 422 ensures one PR), and both reached
_record_milestone_progress -> a double-emitted 70% 'opened PR #N' entry.

Fix: acquire_task_lock (pg_advisory_xact_lock, seed 2) before the fetch, held
through the runner + milestone + request commit. The second concurrent call
blocks until the first commits, then its fetch sees the committed pr_number
and the idempotent guard short-circuits without re-emitting. Per-task (single-
active-task guard means same-task concurrent open_pr is only the bug case).

* [F128] require active claim on explicit-task content posts

_verify_explicit_task_ownership checked assigned_to, which is stale
across a reap/handoff (persists until reassignment; active_claimant_id is
cleared on release). A reaped agent could keep posting say/dm/note to its
former task. Add the active-claimant check when assigned_to == caller;
assigned_to=None keep its existing allow (read-side inspection between
reassignments uses evidence, which has its own ownership path).

Existing 'active owner' test mocks passed assigned_to=agent_id without
active_claimant_id; production sets both together on claim, so the mocks
were incomplete. Updated to set both — realistic, not a behavior change.

* [F129,F130] harden quality gate _run_one exit status + timeout cleanup

F129: _run_one returned 'proc.returncode or 0', masking a None returncode
(communicate returned without a recorded exit code — process killed
out-of-band) as 0 / success. Treat None as a non-zero failure (fail-closed).

F130: on timeout, _run_one killed the subprocess but never awaited wait()
— communicate() was cancelled so it never closed the stdout/stderr pipes,
leaving a transient zombie + leaked FDs. Await wait() after kill() to reap
the process and close the transports.

* [F132] timeout the conventions validator + reap on hang

_run_conventions_validator awaited proc.communicate() with no timeout —
a hung subprocess (tree-sitter deadlock, huge repo) hung the
i_am_done/pr_pass gate forever and orphaned the python subprocess on
orchestrator restart. Wrap communicate() in wait_for(120s); on timeout
kill+wait the proc and fail closed (could_not_run=True → block gate
refuses the submit), matching the validator's own fail-loud philosophy.

* [F135] re-check activity before sweeper closes a session (TOCTOU)

sweep_timed_out_sessions read last_activity_at once at the candidate
SELECT, then closed. A message landing in that window refreshed
last_activity_at in the DB, but the sweeper closed on its stale in-memory
value — closing a just-used session. Re-read last_activity_at fresh right
before the close and skip if the session is no longer timed out.

* [F136] cancel startup indexing task on OptimalService.close()

close() cancelled only the periodic update task, then cleared the plugins.
The startup _indexing_task (background auto-index, slow Ollama / large repo)
could still be mid-flight at shutdown and write against closed/cleared
plugins. Cancel and await _indexing_task FIRST (its tail starts the periodic
task, so ordering also prevents a late periodic spawn), then the periodic
task, then clear plugins.

* [F139] scope active_task_owns_branch to the polled project

active_task_owns_branch did an unscoped WHERE branch_name = ? — a cross-project
branch_name collision (UUID-derived 8-char prefixes, theoretical) made the
internal-PR reviewer skip the WRONG project's PR (project A's leftover PR
skipped because project B happened to have an active task with the same
branch). Pass project_id (in scope at the orchestrator call site) and add
TaskTable.project_id == project_id to the WHERE. Correct for single-project
tasks and MegaTask multi-repo batches alike: each root-subtask carries its own
project_id matching its own repo, so a branch on project A's repo is owned
only by a task whose project_id == A.

* [sweep] strip Fxxx audit-ID tokens + trim bloated comments/docstrings + add behavior-change docs

Post-audit sweep over the 135 audit-fix commits since 19a474d3:

1. Stripped every # Fxxx: audit-ID token from comments AND every Fxxx token
   from docstring openings across 211 blocks / ~626 lines. The CEO flagged
   these twice: audit-issue IDs in code confuse future devs/agents. The
   descriptive text is preserved; only the Fxxx token is removed (and bloated
   narrative blocks trimmed to 1-3 lines keeping the one non-obvious invariant).
2. Trimmed bloated comments/docstrings to the concise standard (1-3 lines).
3. Added missing behavior-change docs for the audit-fix batch: prompts/roles
   (documenter, pr_reviewer, qa), user-facing docs (api auth, websockets,
   agent-gateway, megatask, merge-model, task-lifecycle, grok, resilience,
   conventions, panel, security, troubleshooting), and the RAG corpus (cell-pm,
   main-pm, pr-reviewer, qa roles; conventions; messaging-tools; escalation;
   megatask; task-claiming workflows).

Comment/docstring/prose ONLY — zero code-line edits (verified: the diff
contains no def/class/return/if/for/await/assignment/call lines). Gates green:
ruff format + ruff check clean, mypy clean on roboco/. The only pytest failures
are the pre-existing sync_branch tracing-decision gap (B1, 250be5c2) — not
sweep-caused and tracked separately.

* [fix] register sync_branch in VERBS_WITHOUT_TRACING

sync_branch (B1, 250be5c2) is a git-only rebase+force-push verb (composes=(),
no DB transition, side_effects=()) but was never registered in the tracing
parity tables, so test_every_intent_verb_has_a_tracing_decision failed.
Mirrors open_pr: a mechanical git op with inline preconditions (ownership),
no journal/plan rationale required.

* chore(release): 0.14.0

* [fix] resolve 16 mypy errors across 9 test files (make quality gate)

type-clean the test files so make quality (mypy roboco/ tests/) is green:
- Any-typed locals for the two TypeError-asserting scoping tests (bypass
  the required-arg check without getattr/ruff B009)
- Any-typed view for the shutdown-drain _drain_bg_tasks override (bypass
  mypy method-assign without setattr/ruff B010)
- cast("uuid.UUID", ...) / cast("UUID", ...) for SQLAlchemy UUID[Any]
  returns (TC006-quoted), config=None for AgentInstance stubs, None-narrowed
  await_args, Iterator return on a yielding fixture, UUID annotation on the
  _task helper. No type:ignore / noqa.

* [docs] regenerate lifecycle artifacts for sync_branch + branch-keyed submit_root gate

The committed artifacts were stale: lifecycle.py grew the sync_branch verb
(B1) and the branch-keyed submit_root gate description (B2/B3) but the
generated markdown/json were never regenerated. make foundation-check
enforces artifact==generator(lifecycle.py); regenerating restores that.
No source change — pure generator output.

* [refactor] reduce xenon C-rank blocks to A (behavior-preserving)

Extract helpers / flatten conditionals in 11 blocks that rated C(11)+
under xenon --max-absolute B, dropping pr_gate.py module rank B->A in
the process. Pure move-and-call refactors: each extracted helper holds
the original logic verbatim and the caller delegates to it; no control
flow, return values, or side effects changed.

Sites: validators._extract_strs, sequencing.dev_task_collision_edges,
evidence_builder.build_task_handoff, intake_driver._coerce_draft,
task.claim_task_for_agent (2 guards), prompter.create_task_from_draft
(validate+assignee), pr_gate._gate_decision (3 helpers),
orchestrator._handle_stopped_container + _reap_with_service,
_impl._create_subtask_from_inputs + complete.

_impl helper returns tuple[TaskNature, list[str]] to preserve mypy
narrowing of acceptance_criteria at the TaskCreateRequest site.

Also fix vulture: rename unused __aexit__ param tb->_tb in
test_conventions_cache_put.py (was hidden while xenon short-circuited
the gate).

* [security] bash-guard uv run --active deny + CodeQL path-traversal fixes

Fix 1 (be-dev-1 brick prevention): bash-guard now denies 'uv run --active'
and 'uv run'/'uvx' against /app targets. In the agent container
VIRTUAL_ENV=/app/.venv is baked globally, so 'uv run --active' always
resolves onto the image-baked MCP-gateway venv and uv rebuilds it,
deleting /app/.venv/bin and bricking every MCP server spawn. Bare
'uv run' (workspace .venv, cwd-relative) is untouched.

CodeQL fixes:
- docs.py: replace bypassable '..' substring guard with a
  resolve-and-contain helper (_resolve_contained_path). An absolute
  path made pathlib reset (base / '/etc/passwd' == '/etc/passwd'),
  letting read_doc/delete_doc reach arbitrary files. Applied to both
  sinks.
- orchestrator.py: _safe_agent_path_segment at the spawn_agent
  chokepoint (rejects traversal-shaped agent_id before any fs op) and
  inside _remove_container (slug guard before the log-dir mkdir,
  defense-in-depth).
- agent_sdk/server.py: /usage/sync transcript_path now resolved and
  contained under ROBOCO_TRANSCRIPT_DIR with a .jsonl suffix requirement
  (was Path(raw) — unauthenticated endpoint could stat arbitrary files).

TDD RED->GREEN across all four; make quality green (4890 passed).

* [fix] enum-parity gate: drop false-green mask, skip empty/unmigrated DB

The foundation-check gate ran the enum verifier behind
`|| echo "(skipped — postgres unreachable)"`, which swallows ANY
non-zero exit — including real drift — and prints 'All quality gates
passed'. On a host with a dockerized but empty/unmigrated `roboco` DB
(0 tables: the agentrole/team enum types don't exist), the verifier
connected, found every foundation value 'missing', exited 1, and the
mask relabeled it 'skipped' → false-green.

Fix:
- scripts/verify_postgres_enums.py: move skip semantics INTO the script.
  Distinguish unreachable (skip, exit 0), DB-not-migrated/both-enum-types-
  absent (skip, exit 0), real drift (exit 1), match (exit 0). Extract
  pure enum_drift + should_skip_for_unmigrated helpers + a type_exists
  probe so an empty DB is 'no migrated target', not drift.
- Makefile: drop the `|| echo` mask — real drift now fails the gate.

TDD RED->GREEN (10 tests); make quality green (10906 passed).

* [security] docs path guard: reject '.'/empty segments for clean 400

_resolve_contained_path used an '..' substring ban, which (a) left rel='.'
passing the guard — read_doc/delete_doc then got the base DIRECTORY itself
and raised IsADirectoryError (500) instead of a clean ValidationError, and
(b) false-rejected legit filenames containing '..' like 'v1..v2.md'.

Replace the substring ban with a raw-segment check (rel.split('/')) that
rejects any '.', '..', or empty segment. Path(rel).parts was the wrong tool
— pathlib collapses '.' and empty segments on 3.13, hiding them. The split
check catches '.' / 'a/./b' / 'a//b' / '..' / 'a/../b' while allowing
'v1..v2.md' ('..' inside a filename, no bad segment). The post-resolve
parents-containment check (the real defense) is unchanged.

TDD RED->GREEN (4 new tests); make quality green (10910 passed).

Follow-up to the CodeQL path-traversal review: the two CodeQL 'High' alerts
on this guard are false-positives-on-the-fix (resolve-and-contain already
contains the bypass); this hardening closes the one genuine low residual
(rel='.' -> 500) the review surfaced, which CodeQL did not flag.

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
Renzo F
2026-06-29 05:38:21 +02:00
committed by GitHub
co-authored by Renn F
parent fd10cc862c
commit 15effce014
559 changed files with 33018 additions and 4013 deletions
+97
View File
@@ -0,0 +1,97 @@
"""/api/v1/do/* must enforce the same HMAC agent-token gate as
the /api/v1/flow/* routers.
The do router serves every role (content tools are role-uniform), so it has
no single role to assert — but it must still bind the presented X-Agent-ID
to a verified token when ROBOCO_AGENT_AUTH_REQUIRED=true and reject a forged
token even in dev mode. Without this guard the content-tool endpoints were
the one agent-gateway path that accepted a forged X-Agent-ID with no token
check — a weaker gate than the flow routers' role guards.
"""
from __future__ import annotations
from typing import TYPE_CHECKING
from unittest.mock import AsyncMock, MagicMock
from fastapi import FastAPI
from fastapi.testclient import TestClient
from roboco.agents_config import issue_agent_token
from roboco.api.deps import get_content_actions
from roboco.api.routes.v1.do import router
from roboco.services.gateway.content_actions import ContentActions
if TYPE_CHECKING:
import pytest
_HTTP_200 = 200
_HTTP_401 = 401
_AGENT_ID = "00000000-0000-0000-0000-000000000001"
_SECRET = "test-secret-for-do-auth"
def _build_app() -> FastAPI:
app = FastAPI()
app.include_router(router)
mock_actions = MagicMock(spec=ContentActions)
mock_env = MagicMock()
mock_env.as_dict.return_value = {"status": "ok", "next": "continue"}
mock_actions.commit = AsyncMock(return_value=mock_env)
app.dependency_overrides[get_content_actions] = lambda: mock_actions
return app
def _commit_body() -> dict:
return {"message": "add user authentication endpoint"}
def test_do_route_401_when_auth_required_and_no_token(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Strict mode: a do endpoint must require the token, not just X-Agent-ID."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
client = TestClient(_build_app())
r = client.post(
"/api/v1/do/commit",
json=_commit_body(),
headers={"X-Agent-ID": _AGENT_ID, "X-Agent-Role": "developer"},
)
assert r.status_code == _HTTP_401
def test_do_route_rejects_forged_token_even_in_dev(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Even in header-trust mode, a presented-but-forged token is rejected."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.delenv("ROBOCO_AGENT_AUTH_REQUIRED", raising=False)
client = TestClient(_build_app())
r = client.post(
"/api/v1/do/commit",
json=_commit_body(),
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "developer",
"X-Agent-Token": "forged-not-a-real-hmac",
},
)
assert r.status_code == _HTTP_401
def test_do_route_accepts_valid_token(monkeypatch: pytest.MonkeyPatch) -> None:
"""The good path: a valid HMAC token passes the guard and reaches the handler."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
token = issue_agent_token(_AGENT_ID, "developer")
client = TestClient(_build_app())
r = client.post(
"/api/v1/do/commit",
json=_commit_body(),
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "developer",
"X-Agent-Token": token,
},
)
assert r.status_code == _HTTP_200
+45 -1
View File
@@ -7,7 +7,7 @@ No DB required — Choreographer is mocked.
from __future__ import annotations
from unittest.mock import AsyncMock, MagicMock
from uuid import uuid4
from uuid import UUID, uuid4
import pytest
from fastapi import FastAPI
@@ -296,6 +296,50 @@ async def test_delegate_dispatches_inputs_bundle() -> None:
assert inputs.task_type == "code"
@pytest.mark.asyncio
async def test_delegate_forwards_collision_surfaces_to_inputs() -> None:
"""The dev-task collision surface (intends_to_touch / adds_migration /
touches_shared) and an explicit depends_on override must round-trip from
the HTTP body through DelegateInputs so the cell PM can express the
dev-task collision DAG (the missing edge kind 3 — see the multi-level
sequencing design). Today these fields do not exist on DelegateRequest /
DelegateInputs, so the test would 422 / AttributeError — the red signal.
"""
mock_chore = MagicMock()
mock_chore.delegate = AsyncMock(
return_value=_make_envelope(status="created", task_id=_TASK_ID)
)
client = TestClient(_build_app(mock_chore))
dep_id = str(uuid4())
resp = client.post(
"/api/v1/flow/cell_pm/delegate",
json={
"parent_task_id": _TASK_ID,
"title": "Implement /v1/foo",
"description": "Add the foo endpoint with passing tests.",
"assigned_to": "be-dev-1",
"team": "backend",
"task_type": "code",
"nature": "technical",
"estimated_complexity": "medium",
"acceptance_criteria": ["GET /v1/foo returns 200 with body"],
"intends_to_touch": ["roboco/api/routes/v1/foo.py"],
"adds_migration": True,
"touches_shared": False,
"depends_on": [dep_id],
},
headers=_HEADERS,
)
assert resp.status_code == _HTTP_200, resp.text
inputs = mock_chore.delegate.await_args.args[2]
assert inputs.intends_to_touch == ["roboco/api/routes/v1/foo.py"]
assert inputs.adds_migration is True
assert inputs.touches_shared is False
assert inputs.depends_on == [UUID(dep_id)]
@pytest.mark.asyncio
async def test_submit_up_dispatches_notes() -> None:
"""POST /api/v1/flow/cell_pm/submit_up forwards task_id and notes."""
+19
View File
@@ -205,3 +205,22 @@ async def test_resume_dispatches_task_id() -> None:
)
assert resp.status_code == _HTTP_200
mock_chore.resume.assert_awaited_once()
@pytest.mark.asyncio
async def test_sync_branch_dispatches_task_id() -> None:
"""POST sync_branch forwards task_id to Choreographer.sync_branch (git-only)."""
mock_chore = MagicMock()
mock_chore.sync_branch = AsyncMock(
return_value=_make_envelope(status="ok", task_id=_TASK_ID)
)
client = TestClient(_build_app(mock_chore))
resp = client.post(
"/api/v1/flow/developer/sync_branch",
json={"task_id": _TASK_ID},
headers=_HEADERS,
)
assert resp.status_code == _HTTP_200
mock_chore.sync_branch.assert_awaited_once()
# the only positional arg beyond x_agent_id is task_id
assert str(mock_chore.sync_branch.call_args.args[1]) == _TASK_ID
+20
View File
@@ -190,3 +190,23 @@ async def test_resume_dispatches() -> None:
)
assert resp.status_code == _HTTP_200
mock_chore.resume.assert_awaited_once()
@pytest.mark.asyncio
async def test_i_am_blocked_dispatches_to_choreographer() -> None:
"""POST /api/v1/flow/documenter/i_am_blocked returns an envelope (the
documenter manifest registers i_am_blocked) rather than a raw 404."""
mock_chore = MagicMock()
mock_chore.i_am_blocked = AsyncMock(
return_value=_make_envelope(status="blocked", task_id=_TASK_ID)
)
client = TestClient(_build_app(mock_chore))
resp = client.post(
"/api/v1/flow/documenter/i_am_blocked",
json={"task_id": _TASK_ID, "reason": "PR diff unavailable"},
headers=_HEADERS,
)
assert resp.status_code == _HTTP_200
body = resp.json()
assert body["status"] == "blocked"
mock_chore.i_am_blocked.assert_awaited_once()
+63 -1
View File
@@ -7,7 +7,7 @@ No DB required — Choreographer is mocked.
from __future__ import annotations
from unittest.mock import AsyncMock, MagicMock
from uuid import uuid4
from uuid import UUID, uuid4
import pytest
from fastapi import FastAPI
@@ -273,6 +273,49 @@ async def test_delegate_to_cell_pm_dispatches_inputs_bundle() -> None:
assert inputs.task_type == "planning"
@pytest.mark.asyncio
async def test_delegate_forwards_collision_surfaces_to_inputs() -> None:
"""Main-PM delegate forwards the dev-task collision surface + depends_on
override through DelegateInputs (parity with the cell-PM route — the
main PM delegates cell-tasks/dev-tasks the same way)."""
mock_chore = MagicMock()
mock_chore.delegate = AsyncMock(
return_value=_make_envelope(status="created", task_id=_TASK_ID)
)
client = TestClient(_build_app(mock_chore))
dep_id = str(uuid4())
resp = client.post(
"/api/v1/flow/main_pm/delegate",
json={
"parent_task_id": _TASK_ID,
"title": "Backend slice",
"description": "Plan + drive backend work for feature X end to end.",
"assigned_to": "be-pm",
"team": "backend",
"task_type": "planning",
"nature": "technical",
"estimated_complexity": "high",
"acceptance_criteria": [
"all subtasks created with acceptance criteria",
"branch + PR opened against the slice",
],
"intends_to_touch": ["roboco/services/foo.py"],
"adds_migration": True,
"touches_shared": True,
"depends_on": [dep_id],
},
headers=_HEADERS,
)
assert resp.status_code == _HTTP_200, resp.text
inputs = mock_chore.delegate.await_args.args[2]
assert inputs.intends_to_touch == ["roboco/services/foo.py"]
assert inputs.adds_migration is True
assert inputs.touches_shared is True
assert inputs.depends_on == [UUID(dep_id)]
@pytest.mark.asyncio
async def test_escalate_to_ceo_dispatches() -> None:
mock_chore = MagicMock()
@@ -319,3 +362,22 @@ async def test_resume_dispatches() -> None:
)
assert resp.status_code == _HTTP_200
mock_chore.resume.assert_awaited_once()
@pytest.mark.asyncio
async def test_triage_route_exists_and_dispatches() -> None:
"""POST /api/v1/flow/main_pm/triage wires to choreographer.triage (the
main_pm manifest advertises `triage` alongside `triage_all`); the
team-scoped choreographer.triage impl works for any PM role."""
mock_chore = MagicMock()
mock_chore.triage = AsyncMock(return_value=_make_envelope(status="idle"))
client = TestClient(_build_app(mock_chore))
resp = client.post(
"/api/v1/flow/main_pm/triage",
json={},
headers=_HEADERS,
)
assert resp.status_code == _HTTP_200, resp.text
body = resp.json()
assert body["status"] == "idle"
mock_chore.triage.assert_awaited_once()
+20
View File
@@ -219,3 +219,23 @@ async def test_i_am_idle_dispatches_agent_id() -> None:
)
assert resp.status_code == _HTTP_200
mock_chore.i_am_idle.assert_awaited_once()
@pytest.mark.asyncio
async def test_i_am_blocked_dispatches_to_choreographer() -> None:
"""POST /api/v1/flow/qa/i_am_blocked returns an envelope (the QA manifest
registers i_am_blocked) rather than a raw 404."""
mock_chore = MagicMock()
mock_chore.i_am_blocked = AsyncMock(
return_value=_make_envelope(status="blocked", task_id=_TASK_ID)
)
client = TestClient(_build_app(mock_chore))
resp = client.post(
"/api/v1/flow/qa/i_am_blocked",
json={"task_id": _TASK_ID, "reason": "PR diff won't load"},
headers=_HEADERS,
)
assert resp.status_code == _HTTP_200
body = resp.json()
assert body["status"] == "blocked"
mock_chore.i_am_blocked.assert_awaited_once()
@@ -0,0 +1,65 @@
"""session_to_response / session_to_summary honesty for a null agent_id.
The ``work_sessions.agent_id`` column is ``nullable=True`` with
``ondelete="SET NULL"`` — if an agent row is deleted, the FK nulls out on every
session that agent ever held. The read path (``session_to_response`` ->
``WorkSessionResponse``) used to assume ``agent_id`` was always present: the
ORM ``Mapped[UUID]`` lied, the converter ``typing_cast("UUID", ...)``
papered over the lie, and ``WorkSessionResponse.agent_id: UUID`` rejected
``None`` outright. A session whose agent had been deleted therefore crashed
the GET endpoint with a pydantic ``ValidationError`` instead of serializing
``agent_id: null``. These tests pin the honest end-to-end read path.
"""
from __future__ import annotations
from datetime import UTC, datetime
from uuid import UUID, uuid4
from roboco.api.schemas.work_session import (
WorkSessionResponse,
session_to_response,
session_to_summary,
)
from roboco.db.tables import WorkSessionTable
from roboco.models.work_session import WorkSessionStatus
def _make_session(agent_id: UUID | None) -> WorkSessionTable:
"""Build a detached WorkSessionTable row with an explicit agent_id."""
now = datetime.now(UTC)
return WorkSessionTable(
id=uuid4(),
project_id=uuid4(),
task_id=uuid4(),
agent_id=agent_id,
branch_name="feature/x",
base_branch="main",
target_branch="main",
started_at=now,
status=WorkSessionStatus.ACTIVE,
commits=[],
files_modified=[],
created_at=now,
)
def test_session_to_response_serializes_null_agent_id() -> None:
"""A SET-NULL'd session (agent deleted) must serialize agent_id as None."""
session = _make_session(agent_id=None)
result = session_to_response(session)
assert isinstance(result, WorkSessionResponse)
assert result.agent_id is None
def test_session_to_response_preserves_present_agent_id() -> None:
"""A normal session still carries its agent_id through unchanged."""
agent_id = uuid4()
result = session_to_response(_make_session(agent_id=agent_id))
assert result.agent_id == agent_id
def test_session_to_summary_does_not_read_agent_id() -> None:
"""The summary view omits agent_id entirely, so a null agent must not raise."""
result = session_to_summary(_make_session(agent_id=None))
assert result.status == WorkSessionStatus.ACTIVE
@@ -116,6 +116,15 @@ def test_note_request_coerces_string_next_steps_to_list() -> None:
assert req.next_steps == ["wait for QA"]
def test_note_request_coerces_string_where_to_look_to_list() -> None:
"""A single string for where_to_look is wrapped into a one-element list,
mirroring consequences/next_steps, so a lone scalar does not 422 the route."""
req = NoteRequest.model_validate(
{"text": "x", "scope": "handoff", "where_to_look": "src/api/auth.py"}
)
assert req.where_to_look == ["src/api/auth.py"]
def test_note_request_coerces_single_option_dict_to_list() -> None:
"""A single option dict (not wrapped in a list) is wrapped into a list."""
req = NoteRequest.model_validate(
+182
View File
@@ -0,0 +1,182 @@
"""POST /api/a2a/message/send and /message/stream enforce the same HMAC
agent-token gate as the /api/v1/do/* router (``require_any_authenticated_agent``,
token-only, DB-free, no role assertion — the a2a router serves every role).
"""
from __future__ import annotations
from typing import TYPE_CHECKING
import pytest
from fastapi import FastAPI
from httpx import ASGITransport, AsyncClient
from roboco.agents_config import issue_agent_token
from roboco.api.routes.a2a import router as a2a_router
if TYPE_CHECKING:
from collections.abc import AsyncIterator
_SECRET = "test-secret-for-a2a-auth"
_AGENT_ID = "00000000-0000-0000-0000-000000000002"
_HTTP_200 = 200
_HTTP_400 = 400
_HTTP_401 = 401
def _message_body() -> dict:
"""A minimal valid SendMessageRequest body.
``message.task_id`` defaults to None, so the send route raises
TASK_ID_REQUIRED (400) AFTER the gate passes — proving the gate let the
request through without touching the DB. The stream route takes the
``else`` (new-task) branch and returns 200 with no DB access.
"""
return {"message": {"role": "user", "parts": [{"type": "text", "text": "x"}]}}
@pytest.fixture
async def a2a_client() -> AsyncIterator[AsyncClient]:
app = FastAPI()
app.include_router(a2a_router, prefix="/api/a2a")
async with AsyncClient(
transport=ASGITransport(app=app), base_url="http://test"
) as client:
yield client
app.dependency_overrides.clear()
# ---------------------------------------------------------------------------
# /message/send
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_send_rejects_missing_token_when_required(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Strict mode + no X-Agent-Token => 401, never reaches the handler."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
r = await a2a_client.post(
"/api/a2a/message/send",
json=_message_body(),
headers={"X-Agent-ID": _AGENT_ID, "X-Agent-Role": "developer"},
)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_send_rejects_forged_token_even_in_dev(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A presented-but-forged token is rejected even in header-trust mode."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.delenv("ROBOCO_AGENT_AUTH_REQUIRED", raising=False)
r = await a2a_client.post(
"/api/a2a/message/send",
json=_message_body(),
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "developer",
"X-Agent-Token": "forged-not-a-real-hmac",
},
)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_send_accepts_valid_token(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A valid token passes the gate; the route body then raises
TASK_ID_REQUIRED (400) because message.task_id is None — proving the
gate let the request through (401 would mean the gate rejected it)."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
token = issue_agent_token(_AGENT_ID, "developer")
r = await a2a_client.post(
"/api/a2a/message/send",
json=_message_body(),
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "developer",
"X-Agent-Token": token,
},
)
assert r.status_code == _HTTP_400 # TASK_ID_REQUIRED — gate passed
@pytest.mark.asyncio
async def test_send_dev_mode_missing_token_still_succeeds_gate(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Dev mode + no token => no-op, route body runs (400 TASK_ID_REQUIRED).
Preserves the agent/panel flow in dev exactly as F003/F004 did."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.delenv("ROBOCO_AGENT_AUTH_REQUIRED", raising=False)
r = await a2a_client.post(
"/api/a2a/message/send",
json=_message_body(),
headers={"X-Agent-ID": _AGENT_ID, "X-Agent-Role": "developer"},
)
assert r.status_code == _HTTP_400 # gate passed; route raised TASK_ID_REQUIRED
# ---------------------------------------------------------------------------
# /message/stream
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_stream_rejects_missing_token_when_required(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Strict mode + no X-Agent-Token => 401 on the stream route too."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
r = await a2a_client.post(
"/api/a2a/message/stream",
json=_message_body(),
headers={"X-Agent-ID": _AGENT_ID, "X-Agent-Role": "developer"},
)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_stream_rejects_forged_token_even_in_dev(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A presented-but-forged token is rejected even in header-trust mode."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.delenv("ROBOCO_AGENT_AUTH_REQUIRED", raising=False)
r = await a2a_client.post(
"/api/a2a/message/stream",
json=_message_body(),
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "developer",
"X-Agent-Token": "forged-not-a-real-hmac",
},
)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_stream_accepts_valid_token(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A valid token passes the gate; the stream route returns 200 (SSE) on
the new-task branch (message.task_id is None -> no DB access)."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
token = issue_agent_token(_AGENT_ID, "developer")
r = await a2a_client.post(
"/api/a2a/message/stream",
json=_message_body(),
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "developer",
"X-Agent-Token": token,
},
)
assert r.status_code == _HTTP_200
+219
View File
@@ -0,0 +1,219 @@
"""SSE ``subscribe_to_task`` is authenticated like the rest of the a2a
message surface and opens a SHORT-LIVED DB session per poll iteration
(via ``get_session_factory()``) instead of holding the request-scoped
``db: DbSession`` for the full SSE lifetime, which exhausted the asyncpg
pool one connection per connected client.
"""
from __future__ import annotations
from typing import TYPE_CHECKING, Any, cast
from unittest.mock import AsyncMock, MagicMock
import pytest
from fastapi import FastAPI
from httpx import ASGITransport, AsyncClient
from roboco.agents_config import issue_agent_token
from roboco.api.routes import a2a as a2a_module
from roboco.api.routes.a2a import router as a2a_router
from roboco.db.base import get_db
if TYPE_CHECKING:
from collections.abc import AsyncIterator
from fastapi.routing import APIRoute
_SECRET = "test-secret-for-a2a-subscribe"
_AGENT_ID = "00000000-0000-0000-0000-000000000003"
_HTTP_200 = 200
_HTTP_401 = 401
_HTTP_404 = 404
@pytest.fixture
async def a2a_client() -> AsyncIterator[AsyncClient]:
app = FastAPI()
app.include_router(a2a_router, prefix="/api/a2a")
async with AsyncClient(
transport=ASGITransport(app=app), base_url="http://test"
) as client:
yield client
app.dependency_overrides.clear()
# ---------------------------------------------------------------------------
# Auth gate (F023 parity)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_subscribe_rejects_missing_token_when_required(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Strict mode + no X-Agent-Token => 401, never reaches the generator."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
r = await a2a_client.get(
"/api/a2a/tasks/some-task/subscribe",
headers={"X-Agent-ID": _AGENT_ID, "X-Agent-Role": "developer"},
)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_subscribe_rejects_forged_token_even_in_dev(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A presented-but-forged token is rejected even in header-trust mode."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.delenv("ROBOCO_AGENT_AUTH_REQUIRED", raising=False)
r = await a2a_client.get(
"/api/a2a/tasks/some-task/subscribe",
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "developer",
"X-Agent-Token": "forged-not-a-real-hmac",
},
)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_subscribe_accepts_valid_token_then_404s_unknown_task(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A valid token passes the gate; the route then 404s on the initial
task-existence check (no DB seeded). 404 (not 401) proves the gate let
the request through."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
token = issue_agent_token(_AGENT_ID, "developer")
# get_task returns None -> 404. Patch A2AService.get_task to return None
# so the route doesn't need a real DB.
monkeypatch.setattr(a2a_module.A2AService, "get_task", AsyncMock(return_value=None))
r = await a2a_client.get(
"/api/a2a/tasks/some-task/subscribe",
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "developer",
"X-Agent-Token": token,
},
)
assert r.status_code == _HTTP_404
# ---------------------------------------------------------------------------
# Session-per-query: structural + behavioral
# ---------------------------------------------------------------------------
def test_subscribe_route_does_not_hold_request_scoped_db() -> None:
"""The route must NOT depend on ``get_db`` — the request-scoped session
would be held for the full SSE lifetime (up to 1 hour). Each poll opens
its own short-lived session via ``get_session_factory``.
"""
subscribe_route = cast(
"APIRoute",
next(
r
for r in a2a_router.routes
if getattr(r, "path", "") == "/tasks/{task_id}/subscribe"
),
)
# Walk the route's dependency tree; get_db must not appear anywhere.
deps = [subscribe_route.dependant]
seen: set[int] = set()
found_get_db = False
while deps:
d = deps.pop()
if id(d) in seen:
continue
seen.add(id(d))
if d.call is get_db:
found_get_db = True
deps.extend(d.dependencies)
assert not found_get_db, (
"subscribe_to_task still depends on get_db — the request-scoped "
"session is held for the full SSE lifetime (pool-exhaustion vector)."
)
@pytest.mark.asyncio
async def test_subscribe_opens_a_short_lived_session_per_poll(
a2a_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Each poll iteration opens its own session and closes it before the next
``asyncio.sleep`` — never holding one connection across the full SSE
lifetime. Asserts more than one session open (one per poll, not one for
the lifetime)."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
token = issue_agent_token(_AGENT_ID, "developer")
# Count session opens across the SSE lifetime.
open_count = {"n": 0}
def _factory() -> Any:
open_count["n"] += 1
class _Ctx:
async def __aenter__(self) -> MagicMock:
return MagicMock()
async def __aexit__(self, *exc: object) -> None:
return None
return _Ctx()
monkeypatch.setattr(a2a_module, "get_session_factory", lambda: _factory)
# Non-terminal fake task so the loop keeps polling.
fake_task = MagicMock()
fake_task.status.state = "in_progress"
fake_task.model_dump_json = MagicMock(return_value="{}")
monkeypatch.setattr(
a2a_module.A2AService, "get_task", AsyncMock(return_value=fake_task)
)
# No sleeping — drain the generator as fast as possible.
monkeypatch.setattr(a2a_module.asyncio, "sleep", AsyncMock(return_value=None))
# Disconnect after 3 polls so the stream terminates.
disconnect_after = {"remaining": 3}
async def _fake_is_disconnected() -> bool:
if disconnect_after["remaining"] <= 0:
return True
disconnect_after["remaining"] -= 1
return False
# The route reads request.is_disconnected(); patch it on the request via
# the Starlette request. We patch the Request.is_disconnected property.
monkeypatch.setattr(
"fastapi.Request.is_disconnected",
lambda _self: _fake_is_disconnected(),
)
r = await a2a_client.get(
"/api/a2a/tasks/some-task/subscribe",
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "developer",
"X-Agent-Token": token,
},
)
# Drain the SSE stream so the generator runs to completion.
assert r.status_code == _HTTP_200
# Consume the body (the SSE stream finishes once is_disconnected returns
# True on the 4th check).
_ = await r.aread()
# 3 polls + 1 initial validation = 4 session opens (one per query, none
# held across the lifetime). The key assertion: more than one session
# was opened — proving the request-scoped session is gone.
assert open_count["n"] > 1, (
f"only {open_count['n']} session open(s) — the route is holding a "
"single request-scoped session for the full SSE lifetime (pool "
"exhaustion vector)."
)
+55
View File
@@ -16,6 +16,7 @@ import pytest
from fastapi import FastAPI
from roboco.api.app import app as default_app
from roboco.api.app import create_app, lifespan
from roboco.api.deps import clear_orchestrator, set_orchestrator
if TYPE_CHECKING:
from collections.abc import AsyncIterator
@@ -144,6 +145,60 @@ async def test_lifespan_startup_and_shutdown_happy_path() -> None:
transcription_mock.stop.assert_awaited_once()
@pytest.mark.asyncio
async def test_lifespan_stops_orchestrator_before_closing_db_and_optimal() -> None:
"""orchestrator.stop() runs BEFORE close_optimal_service / close_db on
shutdown — stop() drains fire-and-forget DB writes (respawn_tracker
upserts, audit-log rows) and finalizes work sessions, all needing the
DB still open."""
order: list[str] = []
def _record(label: str) -> AsyncMock:
async def _fn() -> None:
order.append(label)
return AsyncMock(side_effect=_fn)
transcription_mock = MagicMock()
transcription_mock.start = AsyncMock()
transcription_mock.stop = AsyncMock()
orchestrator_mock = MagicMock()
orchestrator_mock.stop = _record("orchestrator.stop")
set_orchestrator(orchestrator_mock)
try:
with (
patch("roboco.api.app.init_db", new=AsyncMock()),
patch("roboco.api.app.close_db", new=_record("close_db")),
patch(
"roboco.api.app.close_optimal_service",
new=_record("close_optimal_service"),
),
patch(
"roboco.api.app.TranscriptionService", return_value=transcription_mock
),
patch("roboco.api.app.ExtractionService"),
patch("roboco.api.app.ExtractionPipeline"),
patch(
"roboco.api.app.get_optimal_service",
new=AsyncMock(return_value=MagicMock()),
),
):
app = create_app()
async with lifespan(app):
pass
finally:
# Clear the global so it doesn't leak into other tests.
clear_orchestrator()
# orchestrator.stop() ran, and it ran BEFORE close_optimal_service + close_db.
assert "orchestrator.stop" in order
assert order.index("orchestrator.stop") < order.index("close_optimal_service")
assert order.index("orchestrator.stop") < order.index("close_db")
# The DB is still the last thing closed (innermost resource).
assert order.index("close_optimal_service") < order.index("close_db")
@pytest.mark.asyncio
async def test_lifespan_handles_optimal_init_failure_gracefully() -> None:
"""Optimal-service init failure → app.state.optimal=None, no raise."""
@@ -0,0 +1,195 @@
"""Dashboard auditor flag/report mutating routes (``create_auditor_flag``,
``resolve_auditor_flag``, ``create_auditor_report``, ``send_auditor_report``)
are gated to AUDITOR or CEO via a ``CurrentAgentContext`` dependency plus a
coarse role gate, mirroring ``roboco/api/routes/playbooks.py::_require_curator``.
"""
from __future__ import annotations
from http import HTTPStatus
from typing import TYPE_CHECKING
from uuid import uuid4
import pytest
import pytest_asyncio
from fastapi import FastAPI
from httpx import ASGITransport, AsyncClient
from roboco.api.deps import get_agent_context, get_db
from roboco.api.routes.dashboard import router as dashboard_router
from roboco.models import AgentRole
from roboco.models.permissions import AgentContext
from roboco.services.dashboard import reset_storage
if TYPE_CHECKING:
from collections.abc import AsyncGenerator, AsyncIterator
from sqlalchemy.ext.asyncio import AsyncSession
def _override_agent(role: AgentRole) -> AgentContext:
return AgentContext(agent_id=uuid4(), role=role, team=None)
@pytest_asyncio.fixture
async def auditor_client(
db_session: AsyncSession,
) -> AsyncIterator[AsyncClient]:
"""A client authenticated as the Auditor (the legitimate caller)."""
reset_storage()
app = FastAPI()
app.include_router(dashboard_router, prefix="/api/dashboard")
async def _override_db() -> AsyncGenerator[AsyncSession]:
yield db_session
app.dependency_overrides[get_db] = _override_db
app.dependency_overrides[get_agent_context] = lambda: _override_agent(
AgentRole.AUDITOR
)
transport = ASGITransport(app=app)
async with AsyncClient(transport=transport, base_url="http://test") as client:
yield client
app.dependency_overrides.clear()
@pytest_asyncio.fixture
async def dev_client(
db_session: AsyncSession,
) -> AsyncIterator[AsyncClient]:
"""A client authenticated as a Developer — must NOT be able to mutate
auditor flags/reports."""
reset_storage()
app = FastAPI()
app.include_router(dashboard_router, prefix="/api/dashboard")
async def _override_db() -> AsyncGenerator[AsyncSession]:
yield db_session
app.dependency_overrides[get_db] = _override_db
app.dependency_overrides[get_agent_context] = lambda: _override_agent(
AgentRole.DEVELOPER
)
transport = ASGITransport(app=app)
async with AsyncClient(transport=transport, base_url="http://test") as client:
yield client
app.dependency_overrides.clear()
# ---------------------------------------------------------------------------
# Legitimate caller (Auditor) succeeds
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_auditor_can_create_flag(auditor_client: AsyncClient) -> None:
response = await auditor_client.post(
"/api/dashboard/auditor/flags",
json={
"severity": "warning",
"category": "quality",
"title": "Flag",
"description": "x",
},
)
assert response.status_code == HTTPStatus.CREATED
@pytest.mark.asyncio
async def test_auditor_can_create_report(auditor_client: AsyncClient) -> None:
response = await auditor_client.post(
"/api/dashboard/auditor/reports",
json={
"report_type": "weekly",
"title": "T",
"summary": "s",
"sections": [],
},
)
assert response.status_code == HTTPStatus.CREATED
@pytest.mark.asyncio
async def test_auditor_can_send_report(auditor_client: AsyncClient) -> None:
create = await auditor_client.post(
"/api/dashboard/auditor/reports",
json={
"report_type": "weekly",
"title": "T",
"summary": "s",
"sections": [],
},
)
rid = create.json()["id"]
response = await auditor_client.post(f"/api/dashboard/auditor/reports/{rid}/send")
assert response.status_code == HTTPStatus.OK
@pytest.mark.asyncio
async def test_auditor_can_resolve_flag(auditor_client: AsyncClient) -> None:
create = await auditor_client.post(
"/api/dashboard/auditor/flags",
json={
"severity": "warning",
"category": "quality",
"title": "F",
"description": "x",
},
)
flag_id = create.json()["id"]
response = await auditor_client.put(
f"/api/dashboard/auditor/flags/{flag_id}/resolve",
params={"notes": "fixed"},
)
assert response.status_code == HTTPStatus.OK
# ---------------------------------------------------------------------------
# Forged caller (Developer) is rejected with 403
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_developer_cannot_create_flag(dev_client: AsyncClient) -> None:
response = await dev_client.post(
"/api/dashboard/auditor/flags",
json={
"severity": "warning",
"category": "quality",
"title": "F",
"description": "x",
},
)
assert response.status_code == HTTPStatus.FORBIDDEN
@pytest.mark.asyncio
async def test_developer_cannot_resolve_flag(dev_client: AsyncClient) -> None:
# The role gate fires before the route checks flag existence, so a random
# UUID is enough to prove the dev is rejected at the gate.
response = await dev_client.put(
f"/api/dashboard/auditor/flags/{uuid4()}/resolve",
params={"notes": "fixed"},
)
assert response.status_code == HTTPStatus.FORBIDDEN
@pytest.mark.asyncio
async def test_developer_cannot_create_report(dev_client: AsyncClient) -> None:
response = await dev_client.post(
"/api/dashboard/auditor/reports",
json={
"report_type": "weekly",
"title": "T",
"summary": "s",
"sections": [],
},
)
assert response.status_code == HTTPStatus.FORBIDDEN
@pytest.mark.asyncio
async def test_developer_cannot_send_report(dev_client: AsyncClient) -> None:
response = await dev_client.post(
f"/api/dashboard/auditor/reports/{uuid4()}/send",
)
assert response.status_code == HTTPStatus.FORBIDDEN
+105
View File
@@ -38,6 +38,7 @@ from roboco.services.base import (
from roboco.services.base import (
ValidationError as ServiceValidationError,
)
from structlog.testing import capture_logs
# ---------------------------------------------------------------------------
# get_status_code
@@ -275,3 +276,107 @@ def test_request_validation_handler_returns_422_with_details() -> None:
body = response.json()
assert "detail" in body
assert "body" in body
# ---------------------------------------------------------------------------
# secret scrubbing in the 422 log line
# ---------------------------------------------------------------------------
class _SecretBody(BaseModel):
"""Module-level model so FastAPI can resolve the annotation under
`from __future__ import annotations` (function-local classes with complex
field types aren't resolvable from the function's module globals)."""
name: str
git_token: str | None = None
api_key: str | None = None
auth_token: str | None = None
nested: dict[str, Any] | None = None
def test_request_validation_handler_scrubs_secrets_from_log() -> None:
"""A 422 on a secret-bearing request must not dump the plaintext secret
into the log line — only the redacted placeholder. The 422 response body
is unchanged (the client sent those values; the server only redacts its
own log)."""
app = FastAPI()
setup_middleware(app)
@app.post("/project")
async def _create(_data: _SecretBody) -> Any:
return {"ok": True}
secret_pat = "ghp_livesecret_123456"
secret_key = "ollama-key-do-not-log"
secret_token = "bearer-should-not-leak"
payload = {
# Missing required `name` -> 422, but the secret fields are still
# parsed into rve.body and would be logged verbatim without the scrub.
"git_token": secret_pat,
"api_key": secret_key,
"auth_token": secret_token,
"nested": {"git_token": "nested-secret-abc", "safe": "keep"},
}
client = TestClient(app, raise_server_exceptions=False)
with capture_logs() as logs:
response = client.post("/project", json=payload)
assert response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY
# The response body is NOT scrubbed (the client sent these values).
resp_body = response.json()
assert resp_body["body"]["git_token"] == secret_pat
assert resp_body["body"]["api_key"] == secret_key
# Exactly one "Request validation failed" warning was emitted.
fails = [e for e in logs if e["event"] == "Request validation failed"]
assert len(fails) == 1
logged_body = fails[0]["body"]
# The log line must not contain any of the plaintext secrets.
assert secret_pat not in str(logged_body)
assert secret_key not in str(logged_body)
assert secret_token not in str(logged_body)
assert "nested-secret-abc" not in str(logged_body)
# The redaction placeholder appears for each secret field (so ops can see
# WHICH secret field was present), and the per-field errors are still
# logged (they don't carry secrets).
assert logged_body["git_token"] == "***REDACTED***"
assert logged_body["api_key"] == "***REDACTED***"
assert logged_body["auth_token"] == "***REDACTED***"
assert logged_body["nested"]["git_token"] == "***REDACTED***"
assert logged_body["nested"]["safe"] == "keep" # non-secret preserved
assert "errors" in fails[0]
def test_request_validation_handler_log_preserves_non_secret_fields() -> None:
"""Non-secret fields in the body are still logged in full — only the
known credential-looking field names are redacted."""
app = FastAPI()
setup_middleware(app)
@app.post("/project")
async def _create(_data: _SecretBody) -> Any:
return {"ok": True}
client = TestClient(app, raise_server_exceptions=False)
# `title` is not a field on _SecretBody -> 422, and `title` is non-secret
# so it should still appear in the log; `git_token` is secret and must be
# redacted.
with capture_logs() as logs:
response = client.post(
"/project",
json={"title": "visible-title", "git_token": "ghp_secret_xyz"},
)
assert response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY
fails = [e for e in logs if e["event"] == "Request validation failed"]
assert len(fails) == 1
logged_body = fails[0]["body"]
assert logged_body["title"] == "visible-title" # non-secret preserved
assert logged_body["git_token"] == "***REDACTED***" # secret redacted
assert "ghp_secret_xyz" not in str(logged_body)
+198
View File
@@ -0,0 +1,198 @@
"""Orchestrator control routes (/api/orchestrator/*) are gated to the
CEO/operator identity: the presented ``X-Agent-ID`` is bound to a verified
HMAC token (DB-free panel-token guard) and the role asserted as CEO. In dev
(header-trust) mode a missing token is a no-op; a presented-but-forged token
is still rejected.
"""
from __future__ import annotations
from typing import TYPE_CHECKING
from unittest.mock import AsyncMock, MagicMock
from uuid import uuid4
import pytest
import pytest_asyncio
from fastapi import FastAPI
from httpx import ASGITransport, AsyncClient
from roboco.agents_config import issue_agent_token
from roboco.api.deps import _ServiceHolder, set_orchestrator
from roboco.api.routes.orchestrator import router as orch_router
if TYPE_CHECKING:
from collections.abc import AsyncIterator
_SECRET = "test-secret-for-orch-auth"
_AGENT_ID = "00000000-0000-0000-0000-000000000001"
_HTTP_201 = 201
_HTTP_204 = 204
_HTTP_401 = 401
_HTTP_403 = 403
def _mock_orchestrator() -> MagicMock:
orch = MagicMock()
orch.spawn_agent = AsyncMock(
return_value=MagicMock(
agent_id=_AGENT_ID,
state=MagicMock(value="starting"),
current_task_id=None,
error_count=0,
started_at=None,
waiting_for=None,
)
)
orch.stop_agent = AsyncMock(return_value=None)
return orch
@pytest_asyncio.fixture
async def orch_client() -> AsyncIterator[tuple[AsyncClient, MagicMock]]:
app = FastAPI()
app.include_router(orch_router, prefix="/api/orchestrator")
orch = _mock_orchestrator()
set_orchestrator(orch)
async with AsyncClient(
transport=ASGITransport(app=app), base_url="http://test"
) as client:
yield client, orch
_ServiceHolder.orchestrator = None
app.dependency_overrides.clear()
# ---------------------------------------------------------------------------
# Strict mode: token required
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_spawn_rejects_missing_token_when_required(
orch_client: tuple[AsyncClient, MagicMock],
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Strict mode + no X-Agent-Token => 401, never reaches the orchestrator."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
client, orch = orch_client
r = await client.post(
f"/api/orchestrator/agents/{_AGENT_ID}/spawn",
headers={"X-Agent-ID": _AGENT_ID, "X-Agent-Role": "ceo"},
)
assert r.status_code == _HTTP_401
orch.spawn_agent.assert_not_awaited()
# ---------------------------------------------------------------------------
# Dev mode: forged token rejected, missing token is a no-op
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_spawn_rejects_forged_token_even_in_dev(
orch_client: tuple[AsyncClient, MagicMock],
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""A presented-but-forged token is rejected even in header-trust mode."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.delenv("ROBOCO_AGENT_AUTH_REQUIRED", raising=False)
client, orch = orch_client
r = await client.post(
f"/api/orchestrator/agents/{_AGENT_ID}/spawn",
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "ceo",
"X-Agent-Token": "forged-not-a-real-hmac",
},
)
assert r.status_code == _HTTP_401
orch.spawn_agent.assert_not_awaited()
@pytest.mark.asyncio
async def test_spawn_rejects_non_ceo_role(
orch_client: tuple[AsyncClient, MagicMock],
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""A developer (even with a validly-issued token) must not spawn/stop agents."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
client, orch = orch_client
dev_id = str(uuid4())
token = issue_agent_token(dev_id, "developer")
r = await client.post(
f"/api/orchestrator/agents/{_AGENT_ID}/spawn",
headers={
"X-Agent-ID": dev_id,
"X-Agent-Role": "developer",
"X-Agent-Token": token,
},
)
assert r.status_code == _HTTP_403
orch.spawn_agent.assert_not_awaited()
# ---------------------------------------------------------------------------
# Legitimate CEO caller succeeds
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_spawn_accepts_valid_ceo_token(
orch_client: tuple[AsyncClient, MagicMock],
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""A valid CEO token passes the gate and reaches the orchestrator."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
client, orch = orch_client
token = issue_agent_token(_AGENT_ID, "ceo")
r = await client.post(
f"/api/orchestrator/agents/{_AGENT_ID}/spawn",
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "ceo",
"X-Agent-Token": token,
},
)
assert r.status_code == _HTTP_201
orch.spawn_agent.assert_awaited_once()
@pytest.mark.asyncio
async def test_stop_accepts_valid_ceo_token(
orch_client: tuple[AsyncClient, MagicMock],
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""The gate is wired into stop_agent too."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
client, orch = orch_client
token = issue_agent_token(_AGENT_ID, "ceo")
r = await client.post(
f"/api/orchestrator/agents/{_AGENT_ID}/stop",
headers={
"X-Agent-ID": _AGENT_ID,
"X-Agent-Role": "ceo",
"X-Agent-Token": token,
},
)
assert r.status_code == _HTTP_204
orch.stop_agent.assert_awaited_once()
@pytest.mark.asyncio
async def test_dev_mode_missing_token_still_succeeds(
orch_client: tuple[AsyncClient, MagicMock],
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Dev mode (no ROBOCO_AGENT_AUTH_REQUIRED) + no token => no-op, route runs.
Preserves the panel/operator flow in dev exactly as F003/F004 did."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.delenv("ROBOCO_AGENT_AUTH_REQUIRED", raising=False)
client, orch = orch_client
r = await client.post(
f"/api/orchestrator/agents/{_AGENT_ID}/spawn",
headers={"X-Agent-ID": _AGENT_ID, "X-Agent-Role": "ceo"},
)
assert r.status_code == _HTTP_201
orch.spawn_agent.assert_awaited_once()
+276
View File
@@ -0,0 +1,276 @@
"""Token enforcement on the live intake chat (Phase 5).
The panel-facing ``prompter_live`` routes used to take no auth dependency — the
SSE stream carried no identity (browser ``EventSource`` can't set headers) and
``start``/``status``/``messages``/``stop`` accepted anonymous calls. The fix
adds a CEO-bound, header-token-only gate (``require_panel_token``) at the route
level: in prod nginx injects the CEO-signed ``X-Agent-Token`` on ``/api/``, and
in dev a missing token is allowed while a presented-but-forged one is still
rejected (matching ``_check_agent_auth_token`` and the WS gate).
These tests mount the router on a bare ``FastAPI()`` (no ``setup_middleware``),
so the gate must raise ``HTTPException(401)`` — the same shape as the a2a auth
tests.
"""
from __future__ import annotations
from http import HTTPStatus
from typing import TYPE_CHECKING, Any
from uuid import uuid4
import httpx
import pytest
import pytest_asyncio
from fastapi import FastAPI
from httpx import ASGITransport, AsyncClient
from roboco.agents_config import issue_panel_token
from roboco.api import deps
from roboco.api.routes.prompter_live import router as prompter_live_router
from roboco.db.base import get_db
from roboco.services import prompter_live
if TYPE_CHECKING:
from collections.abc import AsyncIterator
_SECRET = "test-secret-for-prompter-live-auth"
_HTTP_401 = HTTPStatus.UNAUTHORIZED
class _FakeOrchestrator:
"""Records spawn/reap calls; stands in for the real orchestrator singleton."""
def __init__(self) -> None:
self.spawned: list[dict[str, Any]] = []
self.reaped: list[str] = []
async def start_intake_session(
self,
session_id: str,
*,
project_slug: str | None = None,
product_id: str | None = None,
project_ids: list[str] | None = None,
initial_message: str | None = None,
) -> None:
self.spawned.append(
{
"session_id": session_id,
"project_slug": project_slug,
"product_id": product_id,
"project_ids": project_ids,
"initial_message": initial_message,
}
)
async def reap_intake_session(self, session_id: str) -> None:
self.reaped.append(session_id)
@pytest_asyncio.fixture
async def auth_client(
monkeypatch: pytest.MonkeyPatch,
) -> AsyncIterator[AsyncClient]:
"""Mounted router + fake orchestrator + empty registry; no auth env set.
Each test monkeypatches ``ROBOCO_AGENT_AUTH_SECRET`` and
``ROBOCO_AGENT_AUTH_REQUIRED`` to pick dev vs strict mode. The registry is
empty so the SSE stream over an unknown session yields nothing (200),
``status`` reports dead, ``messages`` 404s, and ``/events`` reports
``pushed: false`` — all non-401, which is what the "gate passed" assertions
need.
"""
orch = _FakeOrchestrator()
monkeypatch.setattr(deps._ServiceHolder, "orchestrator", orch)
def container_handler(_req: httpx.Request) -> httpx.Response:
return httpx.Response(200, json={"ok": True})
mock_client = httpx.AsyncClient(transport=httpx.MockTransport(container_handler))
registry = prompter_live.PrompterLiveRegistry(http_client=mock_client)
prompter_live._RegistryHolder.instance = registry
async def _fake_db() -> AsyncIterator[object]:
yield object()
app = FastAPI()
app.include_router(prompter_live_router, prefix="/api/prompter")
app.dependency_overrides[get_db] = _fake_db
transport = ASGITransport(app=app)
async with AsyncClient(transport=transport, base_url="http://test") as client:
yield client
prompter_live._RegistryHolder.instance = None
await mock_client.aclose()
app.dependency_overrides.clear()
def _strict(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
def _dev(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.delenv("ROBOCO_AGENT_AUTH_REQUIRED", raising=False)
def _start_body() -> dict[str, Any]:
return {"product_id": str(uuid4()), "initial_message": "build X"}
# ---------------------------------------------------------------------------
# /live/start
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_start_rejects_missing_token_when_required(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.post("/api/prompter/live/start", json=_start_body())
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_start_rejects_forged_token_when_required(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.post(
"/api/prompter/live/start",
json=_start_body(),
headers={"X-Agent-Token": "forged-not-a-real-hmac"},
)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_start_accepts_valid_panel_token(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.post(
"/api/prompter/live/start",
json=_start_body(),
headers={"X-Agent-Token": issue_panel_token()},
)
assert r.status_code == HTTPStatus.CREATED # gate passed -> 201
@pytest.mark.asyncio
async def test_start_rejects_forged_token_even_in_dev(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A presented-but-forged token is rejected even in header-trust mode."""
_dev(monkeypatch)
r = await auth_client.post(
"/api/prompter/live/start",
json=_start_body(),
headers={"X-Agent-Token": "forged-not-a-real-hmac"},
)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_start_dev_mode_missing_token_succeeds(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_dev(monkeypatch)
r = await auth_client.post("/api/prompter/live/start", json=_start_body())
assert r.status_code == HTTPStatus.CREATED # dev flow preserved
# ---------------------------------------------------------------------------
# /live/{id}/stream (SSE)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_stream_rejects_missing_token_when_required(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.get("/api/prompter/live/unknown/stream")
assert r.status_code == _HTTP_401 # 401 before the EventSourceResponse starts
@pytest.mark.asyncio
async def test_stream_accepts_valid_panel_token(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.get(
"/api/prompter/live/unknown/stream",
headers={"X-Agent-Token": issue_panel_token()},
)
assert r.status_code == HTTPStatus.OK # unknown session -> empty stream -> 200
# ---------------------------------------------------------------------------
# /live/{id}/status, /messages, /stop
# ---------------------------------------------------------------------------
@pytest.mark.parametrize(
("method", "path", "json"),
[
("GET", "/api/prompter/live/unknown/status", None),
("POST", "/api/prompter/live/unknown/messages", {"text": "hi"}),
("POST", "/api/prompter/live/sess/stop", None),
],
)
@pytest.mark.asyncio
async def test_status_send_stop_reject_missing_token_when_required(
auth_client: AsyncClient,
monkeypatch: pytest.MonkeyPatch,
method: str,
path: str,
json: dict[str, Any] | None,
) -> None:
_strict(monkeypatch)
if method == "GET":
r = await auth_client.get(path)
else:
r = await auth_client.post(path, json=json)
assert r.status_code == _HTTP_401
# ---------------------------------------------------------------------------
# /live/{id}/preview-batch — switched from CurrentAgentContext to the panel gate
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_preview_batch_passes_with_valid_token_in_strict_mode(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.post(
"/api/prompter/live/s1/preview-batch",
json={"drafts": [{"title": "A"}, {"title": "B"}]},
headers={"X-Agent-Token": issue_panel_token()},
)
# Gate passed -> 200 with waves (preview is pure compute; no session needed).
assert r.status_code == HTTPStatus.OK
# ---------------------------------------------------------------------------
# /live/{id}/events — container -> relay, intentionally UNGATED (scope sentinel)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_events_ungated_in_strict_mode(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""``/events`` is the container->relay callback on the internal Docker
network (opaque session id). Option A leaves it ungated; this test pins
that decision so a future gating change can't land silently."""
_strict(monkeypatch)
r = await auth_client.post(
"/api/prompter/live/unknown/events", json={"kind": "text"}
)
assert r.status_code == HTTPStatus.OK # ungated -> 200 (pushed: false)
assert r.json() == {"pushed": False}
+43 -8
View File
@@ -11,7 +11,7 @@ from __future__ import annotations
from datetime import UTC, datetime
from types import SimpleNamespace
from typing import Any
from typing import Any, cast
from unittest.mock import AsyncMock, MagicMock, patch
from uuid import UUID, uuid4
@@ -30,6 +30,7 @@ from roboco.api.schemas.tasks import (
transform_update_data,
)
from roboco.models.base import Complexity, TaskNature, TaskStatus, TaskType, Team
from roboco.models.product import ProductCellMapping
_ORDER_DEFAULT = 0
@@ -211,7 +212,8 @@ def test_parse_uuid_list_with_valid() -> None:
def test_parse_uuid_list_skips_empty_strings() -> None:
raw = uuid4()
out = _parse_uuid_list([str(raw), "", None]) # type: ignore[list-item]
vals: list[Any] = [str(raw), "", None]
out = _parse_uuid_list(cast("list[str]", vals))
assert raw in out
assert len(out) == 1
@@ -277,7 +279,7 @@ def test_task_update_sequence_rejects_negative() -> None:
# ---------------------------------------------------------------------------
def _stub_task(*, with_project: bool = False) -> SimpleNamespace:
def _stub_task(*, with_project: bool = False) -> Any:
"""Build a TaskTable stand-in that matches task_to_response's reads."""
return SimpleNamespace(
id=uuid4(),
@@ -291,6 +293,7 @@ def _stub_task(*, with_project: bool = False) -> SimpleNamespace:
task_type=TaskType.CODE,
project_id=uuid4(),
product_id=None,
cell_projects=[],
project=(SimpleNamespace(slug="proj-1") if with_project else None),
docs_complete=False,
pr_created=False,
@@ -335,7 +338,7 @@ def test_task_to_response_omits_slug_when_project_not_loaded() -> None:
fake_inspector = MagicMock()
fake_inspector.unloaded = {"project"}
with patch("roboco.api.schemas.tasks.sa_inspect", return_value=fake_inspector):
resp = task_to_response(stub) # type: ignore[arg-type]
resp = task_to_response(stub)
assert resp.project_slug is None
@@ -344,10 +347,42 @@ def test_task_to_response_includes_slug_when_project_loaded() -> None:
fake_inspector = MagicMock()
fake_inspector.unloaded = set() # project IS loaded
with patch("roboco.api.schemas.tasks.sa_inspect", return_value=fake_inspector):
resp = task_to_response(stub) # type: ignore[arg-type]
resp = task_to_response(stub)
assert resp.project_slug == "proj-1"
def test_task_to_response_serializes_cell_projects_when_loaded() -> None:
"""An ad-hoc per-cell map (a multi-cell MegaTask root-subtask) round-trips
into the response when the relationship is loaded."""
be_proj, fe_proj = uuid4(), uuid4()
stub = _stub_task()
stub.project_id = None
stub.product_id = None
stub.cell_projects = [
SimpleNamespace(team=Team.BACKEND, project_id=be_proj),
SimpleNamespace(team=Team.FRONTEND, project_id=fe_proj),
]
fake_inspector = MagicMock()
fake_inspector.unloaded = set() # cell_projects IS loaded
with patch("roboco.api.schemas.tasks.sa_inspect", return_value=fake_inspector):
resp = task_to_response(stub)
assert resp.cell_projects == [
ProductCellMapping(team=Team.BACKEND, project_id=be_proj),
ProductCellMapping(team=Team.FRONTEND, project_id=fe_proj),
]
def test_task_to_response_omits_cell_projects_when_unloaded() -> None:
"""A freshly-created task whose cell_projects relationship is unloaded
serializes to [] rather than triggering a lazy load."""
stub = _stub_task()
fake_inspector = MagicMock()
fake_inspector.unloaded = {"cell_projects"}
with patch("roboco.api.schemas.tasks.sa_inspect", return_value=fake_inspector):
resp = task_to_response(stub)
assert resp.cell_projects == []
def test_task_to_response_serializes_all_note_sections() -> None:
"""Regression: pr_reviewer_notes / doc_notes / notes_structured MUST be in the
response. The builder previously omitted them, so the panel showed them blank
@@ -359,7 +394,7 @@ def test_task_to_response_serializes_all_note_sections() -> None:
fake_inspector = MagicMock()
fake_inspector.unloaded = {"project"}
with patch("roboco.api.schemas.tasks.sa_inspect", return_value=fake_inspector):
resp = task_to_response(stub) # type: ignore[arg-type]
resp = task_to_response(stub)
assert resp.pr_reviewer_notes == "## Findings\n- looks good"
assert resp.doc_notes == "Updated the README"
assert resp.notes_structured == {"pr_review": {"verdict": "passed"}}
@@ -370,7 +405,7 @@ def test_task_list_to_response_returns_list() -> None:
fake_inspector = MagicMock()
fake_inspector.unloaded = {"project"}
with patch("roboco.api.schemas.tasks.sa_inspect", return_value=fake_inspector):
out = task_list_to_response(stubs) # type: ignore[arg-type]
out = task_list_to_response(stubs)
assert len(out) == len(stubs)
@@ -384,7 +419,7 @@ def _stub_response() -> Any:
fake_inspector = MagicMock()
fake_inspector.unloaded = {"project"}
with patch("roboco.api.schemas.tasks.sa_inspect", return_value=fake_inspector):
resp = task_to_response(_stub_task()) # type: ignore[arg-type]
resp = task_to_response(_stub_task())
return resp
+98 -1
View File
@@ -2,11 +2,16 @@
from __future__ import annotations
from typing import Any
from uuid import uuid4
import pytest
from pydantic import ValidationError
from roboco.api.schemas.v1.flow import DelegateRequest
from roboco.api.schemas.v1.flow import (
DelegateRequest,
IWillPlanRequest,
IWillWorkOnRequest,
)
def test_delegate_request_requires_task_type() -> None:
@@ -48,3 +53,95 @@ def test_delegate_request_accepts_explicit_task_type() -> None:
acceptance_criteria=["returns 200"],
)
assert req.task_type == "code"
# ---------------------------------------------------------------------------
# StrList — SDK-nested list-of-strings coercion (Bug A)
# ---------------------------------------------------------------------------
def test_i_will_plan_request_flattens_sdk_nested_technical_considerations() -> None:
"""The Claude SDK parses XML-ish ``<item>…</item>`` list-of-strings tool
input into nested arrays (``[[[""]]]``). A bare ``list[str]`` field
hard-rejects element 1 (a list, not a str) at validation time — the live
``i_will_plan`` crash: ``technical_considerations.1 Input should be a
valid string``. The ``StrList`` BeforeValidator must flatten it to a flat
``list[str]`` so the verb body receives clean strings.
"""
# The SDK nests list-of-strings tool input as nested arrays / dict-wrapped
# text (``[[["…"]]]``, ``{"item": {"$text": "…"}}``). Annotated ``list[Any]``
# so mypy accepts the coerce-able shape; the ``StrList`` BeforeValidator
# flattens it to ``list[str]`` at runtime (no ``type: ignore`` owed).
technical_considerations: list[Any] = [
[[["Empty state distinct from loaded state, coverage target 80%"]]],
[{"item": {"$text": "Use asyncpg prepared statements"}}],
]
req = IWillPlanRequest(
task_id=uuid4(),
plan="Plan narrative describing the approach in full sentences.",
approach=(
"Approach text long enough to clear the 150-character minimum "
"enforced on the plan's Approach field so the Plan tab is fully "
"populated for audit and tracing instead of rendering an empty view."
),
technical_considerations=technical_considerations,
)
assert req.technical_considerations == [
"Empty state distinct from loaded state, coverage target 80%",
"Use asyncpg prepared statements",
]
def test_i_will_work_on_request_flattens_dict_wrapped_technical_considerations() -> (
None
):
"""Same coercion on the developer planning verb — a dict-wrapped string
(``{"item": {"$text": ""}}``, the SDK's element-text marker) must reduce
to the bare string, not ``str(dict)``."""
technical_considerations: list[Any] = [
{"item": {"$text": "Cache the lookup result"}}
]
req = IWillWorkOnRequest(
task_id=uuid4(),
technical_considerations=technical_considerations,
)
assert req.technical_considerations == ["Cache the lookup result"]
def test_delegate_request_flattens_sdk_nested_acceptance_criteria() -> None:
"""``delegate``'s ``acceptance_criteria`` is the same list-of-strings shape
the SDK can nest (this is the ``delegate``-verb analogue of the MegaTask
Bug 3 crash). The ``StrList`` field must flatten the nested input so the
VARCHAR[] insert downstream never sees a dict/list element."""
acceptance_criteria: list[Any] = [
[[["returns 200 for valid input"]]],
[{"item": {"$text": "rejects malformed input with 400"}}],
]
req = DelegateRequest(
parent_task_id=uuid4(),
title="t",
description="add the new endpoint plus tests",
assigned_to="be-dev-1",
team="backend",
task_type="code",
nature="technical",
estimated_complexity="medium",
acceptance_criteria=acceptance_criteria,
)
assert req.acceptance_criteria == [
"returns 200 for valid input",
"rejects malformed input with 400",
]
def test_strlist_drops_non_string_junk_instead_of_crashing() -> None:
"""Non-string junk (a bare int, a dict with no string values, whitespace)
is dropped — the field never raises on garbage the SDK might emit; only
real strings survive. An all-junk payload yields an empty list (the
delegate min_length=1 gate then rejects it cleanly, not a 500)."""
technical_considerations: list[Any] = [42, {"foo": 123}, [[" "]], "real note"]
req = IWillWorkOnRequest(
task_id=uuid4(),
technical_considerations=technical_considerations,
)
assert req.technical_considerations == ["real note"]
+190
View File
@@ -0,0 +1,190 @@
"""Token enforcement on the live Secretary chat (Phase 5).
Mirror of ``test_prompter_live_auth`` for the ``secretary_live`` router, which
previously had zero auth on any endpoint. The same ``require_panel_token`` gate
applies at the route level. The Secretary's *authority* (directive execution)
is gated separately at ``/api/secretary/directives``; this only closes the
live-chat transport.
"""
from __future__ import annotations
from http import HTTPStatus
from typing import TYPE_CHECKING, Any
import httpx
import pytest
import pytest_asyncio
from fastapi import FastAPI
from httpx import ASGITransport, AsyncClient
from roboco.agents_config import issue_panel_token
from roboco.api import deps
from roboco.api.routes.secretary_live import router as secretary_live_router
from roboco.services import prompter_live
if TYPE_CHECKING:
from collections.abc import AsyncIterator
_SECRET = "test-secret-for-secretary-live-auth"
_HTTP_401 = HTTPStatus.UNAUTHORIZED
class _FakeOrchestrator:
def __init__(self) -> None:
self.spawned: list[dict[str, Any]] = []
self.reaped: list[str] = []
async def start_secretary_session(
self, session_id: str, *, initial_message: str | None = None
) -> None:
self.spawned.append(
{"session_id": session_id, "initial_message": initial_message}
)
async def reap_secretary_session(self, session_id: str) -> None:
self.reaped.append(session_id)
@pytest_asyncio.fixture
async def auth_client(
monkeypatch: pytest.MonkeyPatch,
) -> AsyncIterator[AsyncClient]:
orch = _FakeOrchestrator()
monkeypatch.setattr(deps._ServiceHolder, "orchestrator", orch)
def container_handler(_req: httpx.Request) -> httpx.Response:
return httpx.Response(200, json={"ok": True})
mock_client = httpx.AsyncClient(transport=httpx.MockTransport(container_handler))
registry = prompter_live.PrompterLiveRegistry(http_client=mock_client)
prompter_live._RegistryHolder.instance = registry
app = FastAPI()
app.include_router(secretary_live_router, prefix="/api/secretary")
transport = ASGITransport(app=app)
async with AsyncClient(transport=transport, base_url="http://test") as client:
yield client
prompter_live._RegistryHolder.instance = None
await mock_client.aclose()
def _strict(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
def _dev(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.delenv("ROBOCO_AGENT_AUTH_REQUIRED", raising=False)
def _start_body() -> dict[str, Any]:
return {"initial_message": "hi"}
@pytest.mark.asyncio
async def test_start_rejects_missing_token_when_required(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.post("/api/secretary/live/start", json=_start_body())
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_start_rejects_forged_token_when_required(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.post(
"/api/secretary/live/start",
json=_start_body(),
headers={"X-Agent-Token": "forged-not-a-real-hmac"},
)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_start_accepts_valid_panel_token(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.post(
"/api/secretary/live/start",
json=_start_body(),
headers={"X-Agent-Token": issue_panel_token()},
)
assert r.status_code == HTTPStatus.CREATED
@pytest.mark.asyncio
async def test_start_rejects_forged_token_even_in_dev(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_dev(monkeypatch)
r = await auth_client.post(
"/api/secretary/live/start",
json=_start_body(),
headers={"X-Agent-Token": "forged-not-a-real-hmac"},
)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_stream_rejects_missing_token_when_required(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.get("/api/secretary/live/unknown/stream")
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_stream_accepts_valid_panel_token(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
_strict(monkeypatch)
r = await auth_client.get(
"/api/secretary/live/unknown/stream",
headers={"X-Agent-Token": issue_panel_token()},
)
assert r.status_code == HTTPStatus.OK
@pytest.mark.parametrize(
("method", "path", "json"),
[
("GET", "/api/secretary/live/unknown/status", None),
("POST", "/api/secretary/live/unknown/messages", {"text": "hi"}),
("POST", "/api/secretary/live/sess/stop", None),
],
)
@pytest.mark.asyncio
async def test_status_send_stop_reject_missing_token_when_required(
auth_client: AsyncClient,
monkeypatch: pytest.MonkeyPatch,
method: str,
path: str,
json: dict[str, Any] | None,
) -> None:
_strict(monkeypatch)
if method == "GET":
r = await auth_client.get(path)
else:
r = await auth_client.post(path, json=json)
assert r.status_code == _HTTP_401
@pytest.mark.asyncio
async def test_events_ungated_in_strict_mode(
auth_client: AsyncClient, monkeypatch: pytest.MonkeyPatch
) -> None:
"""``/events`` is the container->relay callback; intentionally ungated
(scope sentinel mirroring the prompter test)."""
_strict(monkeypatch)
r = await auth_client.post(
"/api/secretary/live/unknown/events", json={"kind": "text"}
)
assert r.status_code == HTTPStatus.OK
assert r.json() == {"pushed": False}
+167
View File
@@ -0,0 +1,167 @@
"""WebSocket streams (/ws/*, operator-only — the panel is the sole WS client)
enforce the HMAC panel/CEO token gate when ROBOCO_AGENT_AUTH_REQUIRED=true:
each per-agent WS upgrade requires + verifies the CEO token in strict mode
and rejects a forged token even in dev mode (same contract as the HTTP role
gates).
"""
from __future__ import annotations
from typing import TYPE_CHECKING
from unittest.mock import AsyncMock, MagicMock
from uuid import uuid4
import pytest
from fastapi import WebSocketDisconnect, status
from roboco.agents_config import CEO_AGENT_ID, issue_agent_token
from roboco.api.websocket import (
ConnectionManager,
agent_stream,
channel_stream,
notification_stream,
)
if TYPE_CHECKING:
import pytest as _pytest # noqa: F401
_SECRET = "test-secret-for-ws-auth"
def _mock_ws(headers: dict[str, str] | None, query: dict[str, str] | None) -> MagicMock:
ws = MagicMock()
ws.accept = AsyncMock()
ws.close = AsyncMock()
ws.send_json = AsyncMock()
ws.send_text = AsyncMock()
# One pong then disconnect so the receive loop exits after a successful gate.
ws.receive_text = AsyncMock(side_effect=["ping", WebSocketDisconnect()])
ws.headers = headers or {}
ws.query_params = query or {}
return ws
@pytest.mark.asyncio
async def test_notification_stream_rejects_missing_token_when_required(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Strict mode + no X-Agent-Token => policy-violation close, never accepted."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
agent_id = uuid4()
ws = _mock_ws(headers={}, query={})
with pytest.MonkeyPatch.context() as mp:
mp.setattr(
"roboco.api.websocket.validate_agent_exists",
AsyncMock(return_value=True),
)
await notification_stream(ws, agent_id)
ws.close.assert_awaited_once()
assert ws.close.await_args.kwargs["code"] == status.WS_1008_POLICY_VIOLATION
ws.accept.assert_not_awaited()
@pytest.mark.asyncio
async def test_notification_stream_rejects_forged_token_even_in_dev(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Even in dev mode a presented-but-forged token is rejected."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.delenv("ROBOCO_AGENT_AUTH_REQUIRED", raising=False)
agent_id = uuid4()
ws = _mock_ws(headers={"x-agent-token": "forged-not-a-real-hmac"}, query={})
with pytest.MonkeyPatch.context() as mp:
mp.setattr(
"roboco.api.websocket.validate_agent_exists",
AsyncMock(return_value=True),
)
await notification_stream(ws, agent_id)
ws.close.assert_awaited_once()
assert ws.close.await_args.kwargs["code"] == status.WS_1008_POLICY_VIOLATION
ws.accept.assert_not_awaited()
@pytest.mark.asyncio
async def test_notification_stream_accepts_valid_panel_token(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""A valid CEO panel token passes the gate and the socket is accepted."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
token = issue_agent_token(CEO_AGENT_ID, "ceo", "")
agent_id = uuid4()
ws = _mock_ws(headers={"x-agent-token": token}, query={})
with pytest.MonkeyPatch.context() as mp:
mp.setattr(
"roboco.api.websocket.validate_agent_exists",
AsyncMock(return_value=True),
)
await notification_stream(ws, agent_id)
ws.accept.assert_awaited_once()
ws.close.assert_not_awaited()
@pytest.mark.asyncio
async def test_agent_stream_rejects_missing_token_when_required(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""The gate is wired into agent_stream too (viewer_id query param path)."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
target_id = uuid4()
viewer_id = uuid4()
ws = _mock_ws(headers={}, query={"viewer_id": str(viewer_id)})
with pytest.MonkeyPatch.context() as mp:
mp.setattr(
"roboco.api.websocket.validate_agent_exists",
AsyncMock(return_value=True),
)
await agent_stream(ws, target_id)
ws.close.assert_awaited_once()
assert ws.close.await_args.kwargs["code"] == status.WS_1008_POLICY_VIOLATION
ws.accept.assert_not_awaited()
# ---------------------------------------------------------------------------
# The channel stream must be usable by a panel-token holder. It previously
# called validate_channel_access, which HTTP-loopbacked to a non-existent
# /api/permissions/check endpoint — every connection 404'd → False → the stream
# closed with WS_1008_POLICY_VIOLATION for every client (the channel live-stream
# was dead). Post-F004 the panel-token gate IS the channel-stream authorization
# (the CEO panel is the sole WS client and may view every channel), so the
# broken loopback check is removed rather than replaced with theater.
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_channel_stream_accepts_panel_token_holder(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""A panel-token holder supplying an agent_id query param is accepted and
registered on the channel stream not fail-closed by a dead permission
check that 404s against a non-existent endpoint."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET)
monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true")
token = issue_agent_token(CEO_AGENT_ID, "ceo", "")
channel_id = uuid4()
viewer_id = uuid4()
mgr = ConnectionManager()
ws = _mock_ws(
headers={"x-agent-token": token},
query={"agent_id": str(viewer_id)},
)
monkeypatch.setattr("roboco.api.websocket.manager", mgr)
await channel_stream(ws, channel_id)
ws.accept.assert_awaited_once()
# Not fail-closed by a dead permission check.
ws.close.assert_not_awaited()
# The "connected" confirmation is sent immediately after connect_channel
# registers the socket, and its subscriber_count proves the socket was in
# the channel's subscription set at confirmation time (the mock then raises
# WebSocketDisconnect so the finally disconnects it — the normal clean
# exit, not a fail-close).
confirmation = ws.send_json.await_args.args[0]
assert confirmation["type"] == "connected"
assert confirmation["channel_id"] == str(channel_id)
assert confirmation["subscriber_count"] == 1
@@ -0,0 +1,184 @@
"""WS route handlers must disconnect on ANY exit path, not just
WebSocketDisconnect.
Each handler adds ``finally: manager.disconnect(websocket)``; ``disconnect``
is idempotent (``set.discard`` / ``dict.pop`` with default), so the
clean-disconnect path and the finally both calling it is safe. Tests use
mock sockets (no real app/Redis) and an isolated ``ConnectionManager``
patched in for the module-global ``manager``.
"""
from __future__ import annotations
import asyncio
from unittest.mock import AsyncMock, MagicMock
from uuid import uuid4
import pytest
from fastapi import WebSocketDisconnect
from roboco.api.websocket import (
ConnectionManager,
agent_stream,
channel_stream,
notification_stream,
session_stream,
system_stream,
)
def _mock_ws_for_receive(receive_side_effect: object) -> MagicMock:
"""A socket whose receive_text raises/returns per ``receive_side_effect``."""
ws = MagicMock()
ws.accept = AsyncMock()
ws.close = AsyncMock()
ws.send_json = AsyncMock()
ws.send_text = AsyncMock()
ws.receive_text = AsyncMock(side_effect=receive_side_effect)
ws.headers = {}
ws.query_params = {}
return ws
# ---------------------------------------------------------------------------
# system_stream (no per-agent keying)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_system_stream_disconnects_on_non_disconnect_exception() -> None:
"""A non-WebSocketDisconnect exception (e.g. anyio closed-resource during
shutdown) must still remove the socket from the manager the old code
only caught WebSocketDisconnect and leaked the dead socket."""
mgr = ConnectionManager()
ws = _mock_ws_for_receive(RuntimeError("connection closed during shutdown"))
await mgr.connect_system(ws)
assert ws in mgr.system_connections
with pytest.MonkeyPatch.context() as mp:
mp.setattr("roboco.api.websocket.manager", mgr)
with pytest.raises(RuntimeError):
await system_stream(ws)
assert ws not in mgr.system_connections
@pytest.mark.asyncio
async def test_system_stream_disconnects_on_cancelled_error() -> None:
"""asyncio.CancelledError during shutdown must also disconnect."""
mgr = ConnectionManager()
ws = _mock_ws_for_receive(asyncio.CancelledError())
await mgr.connect_system(ws)
with pytest.MonkeyPatch.context() as mp:
mp.setattr("roboco.api.websocket.manager", mgr)
with pytest.raises(asyncio.CancelledError):
await system_stream(ws)
assert ws not in mgr.system_connections
# ---------------------------------------------------------------------------
# notification_stream (representative per-agent handler)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_notification_stream_disconnects_on_non_disconnect_exception(
monkeypatch: pytest.MonkeyPatch,
) -> None:
agent_id = uuid4()
mgr = ConnectionManager()
ws = _mock_ws_for_receive(RuntimeError("transport reset"))
monkeypatch.setattr(
"roboco.api.websocket.validate_agent_exists", AsyncMock(return_value=True)
)
monkeypatch.setattr("roboco.api.websocket.manager", mgr)
with pytest.raises(RuntimeError):
await notification_stream(ws, agent_id)
assert ws not in mgr.notification_connections.get(agent_id, set())
assert ws not in mgr.connection_agents
# ---------------------------------------------------------------------------
# channel / agent / session handlers (same pattern, distinct subscription sets)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_channel_stream_disconnects_on_non_disconnect_exception(
monkeypatch: pytest.MonkeyPatch,
) -> None:
channel_id = uuid4()
agent_id = uuid4()
mgr = ConnectionManager()
ws = _mock_ws_for_receive(RuntimeError("anyio closed"))
ws.query_params = {"agent_id": str(agent_id)}
monkeypatch.setattr("roboco.api.websocket.manager", mgr)
with pytest.raises(RuntimeError):
await channel_stream(ws, channel_id)
assert ws not in mgr.channel_connections.get(channel_id, set())
@pytest.mark.asyncio
async def test_agent_stream_disconnects_on_non_disconnect_exception(
monkeypatch: pytest.MonkeyPatch,
) -> None:
target_id = uuid4()
viewer_id = uuid4()
mgr = ConnectionManager()
ws = _mock_ws_for_receive(RuntimeError("anyio closed"))
ws.query_params = {"viewer_id": str(viewer_id)}
monkeypatch.setattr(
"roboco.api.websocket.validate_agent_exists", AsyncMock(return_value=True)
)
monkeypatch.setattr("roboco.api.websocket.manager", mgr)
with pytest.raises(RuntimeError):
await agent_stream(ws, target_id)
assert ws not in mgr.agent_connections.get(target_id, set())
@pytest.mark.asyncio
async def test_session_stream_disconnects_on_non_disconnect_exception(
monkeypatch: pytest.MonkeyPatch,
) -> None:
session_id = uuid4()
agent_id = uuid4()
mgr = ConnectionManager()
ws = _mock_ws_for_receive(RuntimeError("anyio closed"))
ws.query_params = {"agent_id": str(agent_id)}
monkeypatch.setattr(
"roboco.api.websocket.validate_agent_exists", AsyncMock(return_value=True)
)
monkeypatch.setattr("roboco.api.websocket.manager", mgr)
with pytest.raises(RuntimeError):
await session_stream(ws, session_id)
assert ws not in mgr.session_connections.get(session_id, set())
@pytest.mark.asyncio
async def test_system_stream_clean_disconnect_still_works() -> None:
"""Regression: the clean WebSocketDisconnect path still disconnects (the
new finally must not break the happy path or double-disconnect)."""
mgr = ConnectionManager()
ws = _mock_ws_for_receive(["ping", WebSocketDisconnect()])
await mgr.connect_system(ws)
with pytest.MonkeyPatch.context() as mp:
mp.setattr("roboco.api.websocket.manager", mgr)
await system_stream(ws)
assert ws not in mgr.system_connections
# pong was answered before disconnect.
ws.send_text.assert_awaited_with("pong")
@@ -0,0 +1,193 @@
"""Server-side idle timeout reaps half-open WS sockets.
Each handler wraps ``receive_text()`` in
``asyncio.wait_for(..., timeout=IDLE_TIMEOUT_SECONDS)``; on timeout the
handler's ``finally`` disconnects the idle socket. ``IDLE_TIMEOUT_SECONDS``
is a named module constant (ruff PLR2004). Tests patch it to a tiny value
and use a never-resolved ``Future`` so assertions hold in well under a
second, never relying on real wall-clock timing.
"""
from __future__ import annotations
import asyncio
from unittest.mock import AsyncMock, MagicMock
from uuid import uuid4
import pytest
from fastapi import WebSocketDisconnect
from roboco.api.websocket import (
IDLE_TIMEOUT_SECONDS,
ConnectionManager,
agent_stream,
channel_stream,
notification_stream,
session_stream,
system_stream,
)
def _mock_ws_for_receive(receive_side_effect: object) -> MagicMock:
ws = MagicMock()
ws.accept = AsyncMock()
ws.close = AsyncMock()
ws.send_json = AsyncMock()
ws.send_text = AsyncMock()
if isinstance(receive_side_effect, asyncio.Future):
# A never-resolving Future means "hang forever" (half-open socket).
# AsyncMock treats a non-callable side_effect as an iterable, which a
# Future isn't — so install a real async receive_text that awaits it.
hang_future = receive_side_effect
async def _hang_forever() -> str:
await hang_future # never resolves; wait_for cancels it on timeout.
return ""
ws.receive_text = _hang_forever
else:
ws.receive_text = AsyncMock(side_effect=receive_side_effect)
ws.headers = {}
ws.query_params = {}
return ws
# ---------------------------------------------------------------------------
# Constant shape
# ---------------------------------------------------------------------------
def test_idle_timeout_seconds_is_a_named_module_constant() -> None:
"""IDLE_TIMEOUT_SECONDS must be a module-level constant (ruff PLR2004)."""
assert isinstance(IDLE_TIMEOUT_SECONDS, int | float)
assert IDLE_TIMEOUT_SECONDS > 0
# ---------------------------------------------------------------------------
# Half-open socket is reaped after the idle timeout (deterministic)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_system_stream_reaps_silent_socket_after_idle_timeout() -> None:
"""A silent half-open socket (receive_text never returns) is reaped after
the idle timeout the wait_for raises TimeoutError and the finally
disconnects. Deterministic: tiny patched timeout + never-resolving Future."""
mgr = ConnectionManager()
hang_future: asyncio.Future[str] = asyncio.Future()
ws = _mock_ws_for_receive(hang_future)
await mgr.connect_system(ws)
with pytest.MonkeyPatch.context() as mp:
mp.setattr("roboco.api.websocket.manager", mgr)
mp.setattr("roboco.api.websocket.IDLE_TIMEOUT_SECONDS", 0.05)
# Must return promptly (well under 2s), not block for the real default.
await asyncio.wait_for(system_stream(ws), timeout=2.0)
assert ws not in mgr.system_connections
@pytest.mark.asyncio
async def test_notification_stream_reaps_silent_socket_after_idle_timeout(
monkeypatch: pytest.MonkeyPatch,
) -> None:
agent_id = uuid4()
mgr = ConnectionManager()
hang_future: asyncio.Future[str] = asyncio.Future()
ws = _mock_ws_for_receive(hang_future)
monkeypatch.setattr(
"roboco.api.websocket.validate_agent_exists", AsyncMock(return_value=True)
)
monkeypatch.setattr("roboco.api.websocket.manager", mgr)
monkeypatch.setattr("roboco.api.websocket.IDLE_TIMEOUT_SECONDS", 0.05)
await asyncio.wait_for(notification_stream(ws, agent_id), timeout=2.0)
assert ws not in mgr.notification_connections.get(agent_id, set())
@pytest.mark.asyncio
async def test_channel_stream_reaps_silent_socket_after_idle_timeout(
monkeypatch: pytest.MonkeyPatch,
) -> None:
channel_id = uuid4()
agent_id = uuid4()
mgr = ConnectionManager()
hang_future: asyncio.Future[str] = asyncio.Future()
ws = _mock_ws_for_receive(hang_future)
ws.query_params = {"agent_id": str(agent_id)}
monkeypatch.setattr("roboco.api.websocket.manager", mgr)
monkeypatch.setattr("roboco.api.websocket.IDLE_TIMEOUT_SECONDS", 0.05)
await asyncio.wait_for(channel_stream(ws, channel_id), timeout=2.0)
assert ws not in mgr.channel_connections.get(channel_id, set())
@pytest.mark.asyncio
async def test_agent_stream_reaps_silent_socket_after_idle_timeout(
monkeypatch: pytest.MonkeyPatch,
) -> None:
target_id = uuid4()
viewer_id = uuid4()
mgr = ConnectionManager()
hang_future: asyncio.Future[str] = asyncio.Future()
ws = _mock_ws_for_receive(hang_future)
ws.query_params = {"viewer_id": str(viewer_id)}
monkeypatch.setattr(
"roboco.api.websocket.validate_agent_exists", AsyncMock(return_value=True)
)
monkeypatch.setattr("roboco.api.websocket.manager", mgr)
monkeypatch.setattr("roboco.api.websocket.IDLE_TIMEOUT_SECONDS", 0.05)
await asyncio.wait_for(agent_stream(ws, target_id), timeout=2.0)
assert ws not in mgr.agent_connections.get(target_id, set())
@pytest.mark.asyncio
async def test_session_stream_reaps_silent_socket_after_idle_timeout(
monkeypatch: pytest.MonkeyPatch,
) -> None:
session_id = uuid4()
agent_id = uuid4()
mgr = ConnectionManager()
hang_future: asyncio.Future[str] = asyncio.Future()
ws = _mock_ws_for_receive(hang_future)
ws.query_params = {"agent_id": str(agent_id)}
monkeypatch.setattr(
"roboco.api.websocket.validate_agent_exists", AsyncMock(return_value=True)
)
monkeypatch.setattr("roboco.api.websocket.manager", mgr)
monkeypatch.setattr("roboco.api.websocket.IDLE_TIMEOUT_SECONDS", 0.05)
await asyncio.wait_for(session_stream(ws, session_id), timeout=2.0)
assert ws not in mgr.session_connections.get(session_id, set())
# ---------------------------------------------------------------------------
# Regression: ping/pong within the idle window keeps the socket alive
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_ping_within_idle_window_does_not_disconnect() -> None:
"""A client that sends ping before the idle timeout elapses is NOT
disconnected the wait_for resets on each successful receive_text."""
mgr = ConnectionManager()
ws = _mock_ws_for_receive(["ping", WebSocketDisconnect()])
await mgr.connect_system(ws)
with pytest.MonkeyPatch.context() as mp:
mp.setattr("roboco.api.websocket.manager", mgr)
mp.setattr("roboco.api.websocket.IDLE_TIMEOUT_SECONDS", 30)
await system_stream(ws)
# Disconnected only because of the WebSocketDisconnect, not the timeout.
assert ws not in mgr.system_connections
ws.send_text.assert_awaited_with("pong")
+315
View File
@@ -0,0 +1,315 @@
"""Per-connection send queue + send timeout — one slow WS client must not
back-pressure ALL event delivery to ALL clients.
Each registered connection gets a bounded send queue + a sender coroutine
that drains it, with ``send_text`` behind
``asyncio.wait_for(..., timeout=SEND_TIMEOUT_SECONDS)``. Broadcasts become
fire-and-enqueue: a slow client's queue fills, then drops/overflows (logged)
instead of blocking the fan-out. Tests patch ``SEND_TIMEOUT_SECONDS`` to a
tiny value and use a never-resolved ``Future`` so assertions hold in well
under a second, never relying on real wall-clock timing.
"""
from __future__ import annotations
import asyncio
import contextlib
from unittest.mock import AsyncMock, MagicMock
import pytest
from roboco.api.websocket import (
MAX_SEND_QUEUE,
SEND_TIMEOUT_SECONDS,
ConnectionManager,
)
def _make_ws(*, send_side_effect: object | None = None) -> MagicMock:
ws = MagicMock()
ws.accept = AsyncMock()
ws.close = AsyncMock()
ws.send_json = AsyncMock()
if send_side_effect is None:
ws.send_text = AsyncMock()
elif isinstance(send_side_effect, asyncio.Future):
async def _hang(*_args: object) -> None:
await send_side_effect # never resolves
ws.send_text = _hang
else:
ws.send_text = AsyncMock(side_effect=send_side_effect)
return ws
# ---------------------------------------------------------------------------
# Constants shape
# ---------------------------------------------------------------------------
def test_send_constants_are_named_module_constants() -> None:
"""SEND_TIMEOUT_SECONDS + MAX_SEND_QUEUE must be module-level constants."""
assert isinstance(SEND_TIMEOUT_SECONDS, int | float)
assert SEND_TIMEOUT_SECONDS > 0
assert isinstance(MAX_SEND_QUEUE, int)
assert MAX_SEND_QUEUE > 0
# ---------------------------------------------------------------------------
# Broadcast is fire-and-enqueue: a slow registered client does NOT block it
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_broadcast_returns_promptly_with_slow_registered_client() -> None:
"""A registered connection whose send_text never returns must NOT block
broadcast broadcast enqueues (non-blocking) and returns immediately."""
mgr = ConnectionManager()
hang: asyncio.Future[None] = asyncio.Future()
slow_ws = _make_ws(send_side_effect=hang)
await mgr.connect_system(slow_ws)
with pytest.MonkeyPatch.context() as mp:
mp.setattr("roboco.api.websocket.SEND_TIMEOUT_SECONDS", 0.1)
# Must return in well under 1s — enqueue must not await the slow send.
await asyncio.wait_for(
mgr.broadcast_system({"type": "RATE_LIMIT_HIT"}), timeout=1.0
)
# Cleanup: disconnect cancels the stuck sender task.
mgr.disconnect(slow_ws)
@pytest.mark.asyncio
async def test_slow_client_does_not_block_fast_client() -> None:
"""Two registered connections — one slow (send_text hangs), one fast.
The fast client receives the message promptly; the slow client's send
does not delay the fast client's delivery nor the broadcast return."""
mgr = ConnectionManager()
hang: asyncio.Future[None] = asyncio.Future()
slow_ws = _make_ws(send_side_effect=hang)
fast_ws = _make_ws() # default AsyncMock send_text returns immediately.
await mgr.connect_system(slow_ws)
await mgr.connect_system(fast_ws)
with pytest.MonkeyPatch.context() as mp:
mp.setattr("roboco.api.websocket.SEND_TIMEOUT_SECONDS", 0.1)
# Broadcast returns promptly despite the slow client.
await asyncio.wait_for(
mgr.broadcast_system({"type": "USAGE_SNAPSHOT"}), timeout=1.0
)
# Let the fast sender drain its queue.
await asyncio.sleep(0.05)
# Fast client received the message; slow client's send was attempted but
# is still pending (the sender is blocked on the never-resolving send).
assert fast_ws.send_text.await_count >= 1
sent = fast_ws.send_text.await_args.args[0]
assert "USAGE_SNAPSHOT" in sent
mgr.disconnect(slow_ws)
mgr.disconnect(fast_ws)
hang.cancel()
# ---------------------------------------------------------------------------
# Queue-full drop + warning (deterministic: pre-fill the queue, no await)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_broadcast_drops_and_warns_when_queue_full() -> None:
"""When a slow client's bounded send queue is full, broadcast drops the
message (not enqueued) instead of blocking. Deterministic: pre-fill the
queue synchronously (no await so the sender can't drain), then broadcast
once the put_nowait raises QueueFull drop. Assert on the queue state
(still full, the new message was NOT enqueued) rather than log capture,
since structlog doesn't propagate to stdlib ``caplog`` in this config."""
mgr = ConnectionManager()
hang: asyncio.Future[None] = asyncio.Future()
slow_ws = _make_ws(send_side_effect=hang)
await mgr.connect_system(slow_ws)
conn = mgr.connection_senders[slow_ws]
# Pre-fill the queue synchronously — the sender task has not been
# scheduled yet (no await between put_nowait calls), so it can't drain.
for _ in range(conn.queue.maxsize):
conn.queue.put_nowait("pending")
assert conn.queue.full()
with pytest.MonkeyPatch.context() as mp:
mp.setattr("roboco.api.websocket.SEND_TIMEOUT_SECONDS", 0.1)
# Must not raise and must not block.
await asyncio.wait_for(
mgr.broadcast_system({"type": "RATE_LIMIT_HIT"}), timeout=1.0
)
# The broadcast was dropped: the queue still holds exactly maxsize items
# (the new message was NOT enqueued — put_nowait raised QueueFull).
assert conn.queue.full()
assert conn.queue.qsize() == conn.queue.maxsize
mgr.disconnect(slow_ws)
hang.cancel()
# ---------------------------------------------------------------------------
# Fast registered client receives the message (happy path)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_broadcast_delivers_to_registered_fast_client() -> None:
"""A registered connection with a fast send_text receives the message
via its sender task."""
mgr = ConnectionManager()
fast_ws = _make_ws()
await mgr.connect_system(fast_ws)
await mgr.broadcast_system({"type": "RATE_LIMIT_HIT", "provider": "anthropic"})
# Let the sender drain.
await asyncio.sleep(0.05)
assert fast_ws.send_text.await_count == 1
sent = fast_ws.send_text.await_args.args[0]
assert "RATE_LIMIT_HIT" in sent
assert "anthropic" in sent
mgr.disconnect(fast_ws)
# ---------------------------------------------------------------------------
# Legacy fallback: unregistered socket in a subscription set still gets a
# send timeout (so the OLD direct-send path is also protected).
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_broadcast_send_timeout_protects_legacy_unregistered_socket() -> None:
"""A socket present in a subscription set but NOT registered via connect_*
(the legacy test path) still must not block broadcast forever: the
fallback wraps send_text in wait_for(SEND_TIMEOUT_SECONDS)."""
mgr = ConnectionManager()
hang: asyncio.Future[None] = asyncio.Future()
async def _hang() -> None:
await hang
legacy_ws = MagicMock()
legacy_ws.send_text = _hang
# Put it straight into the set — bypasses connect_system (no sender).
mgr.system_connections.add(legacy_ws)
with pytest.MonkeyPatch.context() as mp:
mp.setattr("roboco.api.websocket.SEND_TIMEOUT_SECONDS", 0.1)
# Must return in well under 1s — the slow send is timed out, not
# awaited indefinitely.
await asyncio.wait_for(
mgr.broadcast_system({"type": "RATE_LIMIT_HIT"}), timeout=1.0
)
hang.cancel()
# ---------------------------------------------------------------------------
# Send-side failure proactively reaps the dead socket.
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_sender_triggers_disconnect_on_send_error() -> None:
"""When ``send_text`` raises (transport closed / dead socket), the sender
task must proactively disconnect the socket from every subscription set
rather than wait for the receive loop's idle timeout — otherwise a
send-side-detected dead socket lingers and broadcasts keep enqueuing into
a queue whose consumer has exited."""
mgr = ConnectionManager()
dead_ws = _make_ws(send_side_effect=ConnectionError("transport closed"))
await mgr.connect_system(dead_ws)
assert dead_ws in mgr.system_connections
assert dead_ws in mgr.connection_senders
await mgr.broadcast_system({"type": "RATE_LIMIT_HIT"})
# Let the sender drain the queue and hit the send error.
await asyncio.sleep(0.05)
# The send error triggered disconnect: the dead socket is gone from every
# subscription set + the sender registry, so future broadcasts skip it
# entirely instead of enqueueing into an un-drained queue.
assert dead_ws not in mgr.system_connections
assert dead_ws not in mgr.connection_senders
@pytest.mark.asyncio
async def test_sender_keeps_live_socket_on_send_timeout_only() -> None:
"""A send TIMEOUT alone (slow client, not a dead socket) must NOT disconnect
the socket only a hard send Exception (transport closed) does. A
slow-but-live client should keep receiving once it drains; timing it out
is the graceful-degradation path, not a reap trigger."""
mgr = ConnectionManager()
hang: asyncio.Future[None] = asyncio.Future()
slow_ws = _make_ws(send_side_effect=hang)
await mgr.connect_system(slow_ws)
with pytest.MonkeyPatch.context() as mp:
mp.setattr("roboco.api.websocket.SEND_TIMEOUT_SECONDS", 0.1)
await mgr.broadcast_system({"type": "RATE_LIMIT_HIT"})
await asyncio.sleep(0.2)
# Timed out, but the socket is still live (registered) — slow, not dead.
assert slow_ws in mgr.system_connections
assert slow_ws in mgr.connection_senders
mgr.disconnect(slow_ws)
hang.cancel()
# ---------------------------------------------------------------------------
# disconnect cancels the sender task (no leak)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_disconnect_cancels_sender_task() -> None:
"""disconnect() cancels the per-connection sender task so it doesn't
leak after the socket is removed."""
mgr = ConnectionManager()
ws = _make_ws()
await mgr.connect_system(ws)
conn = mgr.connection_senders[ws]
sender = conn.sender
assert sender is not None
assert not sender.cancelled()
mgr.disconnect(ws)
# Sender is removed + cancelled (or done). Give the loop a tick so the
# cancellation actually propagates (cancel() schedules, doesn't sync).
assert ws not in mgr.connection_senders
with contextlib.suppress(TimeoutError, asyncio.CancelledError):
await asyncio.wait_for(sender, timeout=1.0)
assert sender.cancelled() or sender.done()
# ---------------------------------------------------------------------------
# Existing direct-set subscription-set broadcast still works (backward compat)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_broadcast_to_legacy_direct_set_sends_to_each_socket() -> None:
"""Sockets added directly to a subscription set (not via connect_*) are
still sent to via the fallback path preserves the existing test contract."""
mgr = ConnectionManager()
ws1, ws2 = MagicMock(), MagicMock()
ws1.send_text = AsyncMock()
ws2.send_text = AsyncMock()
mgr.system_connections = {ws1, ws2}
await mgr.broadcast_system({"type": "x"})
# The fallback schedules a timeout-bounded send task per socket; let them
# run to completion before asserting.
await asyncio.sleep(0.05)
ws1.send_text.assert_awaited_once()
ws2.send_text.assert_awaited_once()
+6
View File
@@ -8,6 +8,7 @@ against mock sockets (no real app/lifespan/Redis).
from __future__ import annotations
import asyncio
from unittest.mock import AsyncMock, MagicMock
import pytest
@@ -33,9 +34,14 @@ async def test_broadcast_system_sends_to_every_connection() -> None:
ws1, ws2 = MagicMock(), MagicMock()
ws1.send_text = AsyncMock()
ws2.send_text = AsyncMock()
# These sockets are placed directly into the subscription set (bypassing
# connect_system), so they take the F064 legacy fallback path: broadcast
# schedules a timeout-bounded send task per socket instead of awaiting
# send_text inline. Yield once so those tasks run before asserting.
mgr.system_connections = {ws1, ws2}
await mgr.broadcast_system({"type": "RATE_LIMIT_HIT", "provider": "anthropic"})
await asyncio.sleep(0)
ws1.send_text.assert_awaited_once()
ws2.send_text.assert_awaited_once()