Files
roboco/tests/unit/mcp_servers/test_flow_server_circuit_breaker.py
T
df87fcf059 Chore/logical gaps element sweep fixes (#287)
* [sweep] lifecycle: 6 confirmed gaps fixed (cancel-ceo-gate, claim_pr_review gate, needs_team_match, valid_next_verbs narrowing, pr_reviewer unclaim, complete side_effect ordering)

* [chore] logical-gaps: route-layer force gate + privileged-field gate + pre-task audit attribution

tasks.py (5 gaps):
- _HATCH_OVERRIDE_STATES expanded to 7: a privileged PATCH INTO a gate
  state (completed/cancelled/awaiting_{qa,documentation,pr_review,
  pm_review,ceo_approval}) now requires explicit force — the panel hatch
  is no longer a quiet click that drops a task into/out of a human gate.
- _RESURRECT_SOURCE_STATES: a privileged PATCH OUT of a terminal status
  (completed/cancelled) resurrects finished work and likewise requires
  force, audited as an override.
- _PRIVILEGED_UPDATE_FIELDS gate: a bare task owner (UPDATE_OWN, no
  ASSIGN) cannot self-reassign / re-team / re-parent / re-depend /
  re-block / rewrite-plan / re-project its task — those structural fields
  are PM-gated; the REST surface must not bypass the verb-layer's
  reassign/delegate/triage gate. A 403 names the touched fields + the
  verb to use instead.
- pre-task create denial: a role that cannot create tasks is now logged
  via log_task_creation_denial (distinct task_creation target_type +
  attempted payload) instead of a 'N/A' task_id that coerced to NULL and
  left the role-escalation attempt unattributable.

audit.py:
- split log_task_action_denial (5-param, under PLR0913) from
  log_task_creation_denial (4-param) — the create path has no task_id;
  the non-UUID sentinel (N/A) is preserved in details[target_id_raw]
  rather than dropped to a NULL target_id indistinguishable from any
  other NULL-target denial.

tests:
- test_tasks_routes.py: parametrized admin-override gate (force
  required for gate + terminal states, force succeeds).
- test_tasks_route_privileged_fields.py: dev owner 403 on
  assigned_to/team/parent_task_id, 200 on dev-facing description.
- test_audit.py: pre-task attribution via log_task_creation_denial +
  non-UUID sentinel preservation.

* [chore] logical-gaps: kanban board column coverage + status-class fixes (6 gaps)

models/kanban.py:
- DEV_COLUMNS: cover all 15 lifecycle statuses (was 7; dropped BACKLOG,
  PAUSED, VERIFYING, NEEDS_REVISION, AWAITING_PR_REVIEW, AWAITING_PM_REVIEW,
  AWAITING_CEO_APPROVAL, CANCELLED). A dev whose task bounced to
  needs_revision or sits in a gate used to see their own task vanish.
- PM_COLUMNS: add the gate/revision/paused/cancelled/backlog columns so the
  cell PM sees the QA->docs->PR-review->PM-review->CEO chain on its board.
- QA_COLUMNS: drop the 'In Review'->VERIFYING mapping. VERIFYING is the dev's
  self-verification (task still with the dev, not with QA); it misrepresented
  dev mid-verification as active QA work.

services/kanban.py:
- _build_flat_board: add an 'Other' fallback column for any task whose status
  matches no configured column, so total_cards == sum(card_count) and no card
  is built-then-silently-dropped (the vanished-card leak).
- get_qa_board: drop VERIFYING from qa_statuses (consistent with the column
  change).
- get_documenter_board: scope to task_type=documentation so a dev IN_PROGRESS
  code task sharing the cell team no longer appears under 'Gathering'.
- get_main_pm_board_flat: widen the status filter to include PENDING/CLAIMED/
  COMPLETED and route those to the incoming/distributed/done columns, which
  were structurally always empty under the in-flight-only filter.

tests/integration/test_kanban_service.py: parametrized coverage of every
dropped dev status, PM gate/revision states, QA excludes VERIFYING,
documenter excludes dev code tasks, flat Main PM incoming/distributed/done
populated, and the 'Other' fallback invariant.

* [chore] logical-gaps: lifecycle-enforcement validators + status-class fixes (5 gaps)

enforcement/task_lifecycle.py:
- drop the spurious VERIFYING->awaiting_documentation legacy edge. The
  canonical exit is submit_qa -> awaiting_qa -> (qa_pass) ->
  awaiting_documentation; the direct edge bypassed the entire QA review hop
  (ungated — no role gate existed for it).
- is_waiting_state: add awaiting_pr_review. The PR-review gate parks the PM on
  the reviewer; it is a waiting state. The hard-coded set was never updated
  when AWAITING_PR_REVIEW was added to the enum, so the gate status was
  miscategorized as active.

foundation/_validate_lifecycle.py:
- _check_status_enum_coverage: replace the tautology (STATUS_GRAPH keys every
  Status by construction) with a real bidirectional check — every non-terminal
  Status is the source of a transition (catches orphan states), and every
  source/target referenced is a real Status member (catches stray-string
  targets).
- _check_terminal_exits: split the {COMPLETED, CANCELLED} reachability into a
  COMPLETED-path requirement + a cancel-exit requirement. The cancel fan-out
  made the old check structurally trivial — a status whose sole exit was cancel
  passed with no real forward completion path.
- _check_status_enum_parity (new, registered): cross-check spec.Status against
  models.base.TaskStatus at import so the ORM column type and the lifecycle
  map cannot drift (TaskType had this guard; Status did not).

tests: verifying->awaiting_documentation rejected, self-fail preserved,
awaiting_pr_review is waiting, mutually-disjoint classification invariant,
status enum parity, stray-string-target / orphan-source / cancel-only-exit
validator rejections.

* [chore] logical-gaps: stream-bus poison-pill ACK + dead-letter, periodic reclaim, cancelled-handler marker cleanup (3 gaps)

stream_bus.py:
- _handle_message isolates Event.from_json in its own try/except; an
  undecodable payload (unknown EventType, bad UUID/timestamp, malformed
  JSON) is dead-lettered then ACKed instead of falling through to the
  broad except that only logged — a poison pill stayed pending forever
  and re-failed on every reclaim. (gap: stream-bus-malformed-event-poison-pill)
- _reclaim_loop spawned alongside _listen_loop in start_listening (cancelled
  in disconnect). XREADGROUP '>' delivers only NEW messages, so a runtime
  handler failure left its message pending and unretried until a restart;
  the loop re-runs recover_pending every 60s so the idempotency-guarded
  replay actually fires. (gap: stream-bus-no-runtime-reclaim-loop)
- _run_handler_guarded marker cleanup catches BaseException so a handler
  cancelled mid-flight (asyncio.CancelledError is BaseException-derived
  since 3.8) clears its SET-NX marker; otherwise the guard suppressed the
  very redelivery that would complete the work. (gap: stream-bus-cancelled-
  handler-keeps-idempotency-marker)

TDD: 4 red->green tests in tests/unit/events/test_bus.py.

* [chore] logical-gaps: verb_runner trailing-None side-effect guard + actor_agent_id threading (3 gaps)

_verb_runner.py:
- run_intent skips the side_effects loop when a TRAILING composed action
  returned None (its source-status check failed under a concurrent
  transition). Previously the loop ran unconditionally on the None task
  and _do_push_branch(None)/_do_pr_merge(None) crashed with a
  NoneType AttributeError, turning the clean INVALID_STATE the
  entry/intermediate guards give into a 500/respawn loop. The trailing
  None now flows to the caller's `if task is None` handler. Latent today
  (no shipped intent has both a None-capable compose and trailing
  side_effects) but the runner is generic. (gap: runner-side-effects-fire-
  on-trailing-none-task)
- _do_push_branch / _do_create_pr / _do_create_root_pr forward
  actor_agent_id=agent.id into git_service (push_branch / create_pr),
  matching _do_pr_merge. Without it, a verb on a task whose assigned_to
  was cleared before the side effect falls through to created_by and
  pushes from / opens a PR against the wrong workspace.
  (gap: side-effect-handlers-drop-actor-agent-id)
- _do_escalate_to_ceo forwards actor_agent_id=agent.id so the
  awaiting_ceo_approval audit row attributes to the specific PM/Board
  agent. (gap: do-escalate-to-ceo-drops-actor-agent-id)

task.py: escalate_to_ceo gains actor_agent_id param, passed as
audit_agent_id to _validate_and_set_status and recorded as
escalated_by_agent_id in the event payload + log. escalate_to_ceo_for_agent
forwards agent.agent_id.

_impl.py: the main_pm complete->escalate path forwards
actor_agent_id=main_pm_agent_id.

TDD: 5 red->green tests (synthetic trailing-None intent, actor forwarding
for push_branch/create_pr/create_root_pr/escalate_to_ceo) + real-DB audit
test asserting the awaiting_ceo_approval row carries the actor UUID.
Updated 3 board escalate_to_ceo tests to assert the forwarded actor.

* [B-REL] release executor: idempotent half-landed retry + commit-scoped CI + decoupled workflow

Three confirmed gaps in the release fail-closed pipeline (#87/#318/#402):

#87 publish_failed retry duplicates changelog: execute() only short-circuits
on an existing tag. A publish_failed outcome (commit pushed + CI green, no
tag) left no tag, so a retry re-ran apply_version_bumps + write_changelog_entry
(re-inserting the entry above the already-present heading -> duplicate) and
commit_and_push (a second chore(release) commit). Add ReleaseOps
.release_commit_sha(version) detecting a prior release commit on the branch
(clone already at the target version); when present, skip the bump/changelog/
gate/commit pipeline and rejoin the shared CI -> publish tail on the existing
commit. No second commit, no duplicate entry.

#318 wait_for_ci polls branch-latest, not the release commit: a later push to
master during the ~40min wait made the latest run's head_sha != the release
sha forever, exhausting _CI_MAX_POLLS -> false ci_failed on a release whose
own CI was green. Thread head_sha through get_latest_ci_conclusion /
_fetch_latest_ci_run (GitHub actions/runs?head_sha=) so the gate polls the
release commit's own run; a concurrent push can no longer mask it.

#402 release CI gate reuses self_heal_ci_workflow: that setting documents an
empty-string mode for single-workflow repos which, inherited here, degraded
the fail-closed gate to the all-workflows mode git.py itself flags as
unreliable. Add release_ci_workflow (default ci.yml) and _resolve_release_
ci_workflow(); the release gate always resolves a NAMED workflow, never None.

Refactor: bundle the CI-fetch per-project inputs into a _CiRunQuery dataclass
so _fetch_latest_ci_run stays under the arg-count gate; unify the half-landed
path into execute's shared tail (drops a separate _publish_existing, one
return path). TDD red->green; ruff/mypy clean.

* [chore] logical-gaps: a2a service hierarchy gate (typed, unconditional) + persist skill on message row (3 gaps)

create_a2a_notification gated A2A hierarchy only when both ends resolved
(`if from_agent and target_agent:`), so an unattributed (from_agent falsy)
or unresolvable-target request slipped past the hierarchy matrix and
dispatched with from_agent='unknown' / to_agent='' — and a denial came back
as a bare ValueError indistinguishable from the missing-task_id ValueError.
Require both ends present, then validate via the shared typed
validate_a2a_access path (A2AAccessDeniedError + route_hint) so the legacy
notification surface enforces the same who-may-talk-to-whom invariant as the
conversation path.

send() accepts skill= and the gateway callers (qa/doc/pr_gate) pass it
expecting the receiver to learn which capability the message is about, but
send_chat_message never read it from options — silently dropped. Persist a
nullable skill column (migration 054) on a2a_messages, wire it through
send_chat_message + _msg_to_model + the A2AChatMessage model, and fix the
send() docstring (it claimed 'recorded in message metadata').

TDD: 4 red→green (skill recorded on message + surfaces in inbox; permission
denied raises typed A2AAccessDeniedError with route_hint; self-A2A raises
typed; missing from_agent raises instead of silent dispatch). 103 a2a
integration tests green; ruff/mypy clean; migration 054 verified
upgrade/downgrade on throwaway PG.

* [chore] logical-gaps: release-proposal already_published closes proposal + heartbeat-lock-loss cancels execute (2 gaps)

approve() closed the proposal only on status=='published'. A retry that finds
the tag already shipped returns 'already_published' (is_already_published),
so if a prior publish's route commit failed / HTTP 504'd, the proposal stayed
non-terminal forever — every retry returned already_published and never
closed it; only a manual cancel unstuck it. Close on both published and
already_published: the release shipped either way.

_heartbeat_loop returned silently when the lock was no longer owned (a >TTL
Redis outage let the mutex expire mid-execute), leaving executor.execute
running UNGUARDED — a concurrent approve (once Redis returns) could then
acquire the lock and _prepare_release_clone rm -rf the in-flight shared
release clone while the first execute was still mid-run_gate, re-opening the
very rm -rf-clone race the mutex+heartbeat exist to prevent. Run execute as a
task; on lock-loss the heartbeat sets a flag and cancels it, and approve()
turns the CancelledError into a structured 'lock_lost' result (an external
cancellation of approve itself still propagates — distinguished by the flag).

TDD: 2 red→green (already_published → COMPLETED not wedged; heartbeat lock-loss
→ lock_lost + execute cancelled, proposal not completed). 8 concurrency tests
green; ruff/mypy clean.

* [chore] logical-gaps: release approve async dispatch (202) — kill the 40min synchronous HTTP 504

The approve route ran the whole fail-closed execute inline: clone(600s) +
gate(1800s) + CI poll(2400s) + publish(300s) ≈ up to 85min worst case. nginx
(the single :3000 entry point, ~60s read timeout) 504'd long before it
finished, so the CEO's approve always appeared to fail even when the release
succeeded server-side — the structured ReleaseResult was unreachable over the
wire. dispatch_approve spawns the execute in a background task with a fresh
session (built from the request session's engine) and the route returns 202
'accepted' immediately; _INFLIGHT_APPROVES tracks the dispatched task for
observability (self-cleans via done-callback; the Redis mutex still refuses a
double-execute on a second click). The panel already polls GET /proposal every
30s, so it observes the final status (COMPLETED on published/already_published,
else the proposal stays open for retry); the card's approve toast now treats
'accepted' as an info 'dispatched, running in the background' instead of the
old 'Release halted' warning.

TDD: 2 route tests red→green (approve returns 202 'accepted' + the proposal
transitions to COMPLETED / stays PENDING once the background faked execute
completes; the dispatched task is awaited while the executor patch is live).
83 release tests green; ruff/mypy clean; panel typecheck+lint+format+test
green.

* [chore] mcp-servers: normalize exception bodies to Envelope + lift task_id/correlation_id on circuit_open (#232 #359 #57)

flow_server/do_server: the non-404 JSON path returned exception-handler bodies
raw (dict `error` from roboco/generic/http exception handlers, or a 422
`detail` list) — neither is the Envelope wire format the agent is prompted to
trust (string error kind + message + remediate + missing), so on any
service/validation failure the agent got no remediate and flailed until the
breaker tripped. _normalize_exception_envelope lifts the body into a real
Envelope (code -> counted string kind via _classify_dict_error_code, NOT_FOUND
-> not_found, message lifted, remediate synthesized, missing=[]; 422 -> incomplete_input with the validation detail preserved). The synthesized
envelope still flows through the breaker so a 500/422 storm trips it.

_record_and_check_circuit: the circuit_open substitution dropped task_id /
correlation_id from the top level (the SDK's envelope omits them); lift them
from the original rejection so the agent's envelope contract and ops audit-join
of the trip event still work, not just nested in inner.

intake_server._post_event: capture the relay response body under `detail` on
non-success so the grok intake agent gets the real reason (e.g. 'session not in
MegaTask scope' on a 422) instead of an opaque http_422 token with no
remediation.

TDD red->green; ruff + mypy clean; 157 mcp/SDK-breaker tests pass.

* [chore] a2a-routes: authenticate send_message responder + gate cancel task (PM-only) (#116 #423)

send_message took the responder identity from a client-supplied
metadata.from_agent, so any caller could spoof anyone (e.g.
from_agent='ceo') in the task's notes and in the spawn/notification
routed back to the original requester. Stamp the authenticated caller's
slug as the responder instead (CurrentAgentContext).

cancel_task was ungated: no auth dependency and no role check, so any
agent (or any caller) could cancel a task the lifecycle rule reserves to
PM roles (Any -> cancelled: PM roles only) — and the cascade-cancel of
all non-terminal descendants ran with a hardcoded cell_pm role and no
recorded actor. Add require_any_authenticated_agent + a PM-or-above gate,
and thread the authenticated role (into the cascade role gate) and slug
(into the cancellation note) into A2AService.cancel_task.

Tests: send_message ignores a spoofed from_agent and records the
authenticated slug; cancel rejects a developer (403) and a missing auth
header; a PM cancel threads role + slug into the service; the pre-existing
cancel success/already-terminal/not-found tests now run under a PM context
(the success test's body was missing the A2A 'name' field and false-passed
on a 422 — now genuine).

* [chore] work-session-routes: ownership check on mutating routes + stamp merge_pr merged_by from auth (#158 #271)

Every mutating work-session route keyed off session_id alone after the
role gate, so any developer could commit into / abandon / complete a
peer's active session (breaking the single-active-WorkSession invariant
and stranding that task) and any PM could merge any cell's PR — the REST
surface bypassed the verb layer's active-claimant gate entirely. Add a
shared _assert_ownership guard: dev ops require session.agent_id to be
the caller; PM merge_pr requires a cell PM to own the session's task cell
(main PM / CEO / board coordinate every cell), 404 for a missing session.

merge_pr took merged_by from the request body, so any PM could record a
PR merge under another agent's id, corrupting the merge audit trail the
completion/CEO-approval chain and metrics rely on. Drop the body param
and stamp the authenticated caller's agent_id as merged_by (the
MergePRRequest schema is gone with it).

Tests: a second dev's token hitting a peer's /commits and /abandon -> 403
(session left active); a foreign-cell PM -> 403, same-cell PM -> 200; a
spoofed body merged_by is ignored and the persisted row records the PM.

* [chore] ci-watch/dep-update dedupe: normalize git_url + treat empty-string workflow as default (#148 #1267)

The per-repo open-task dedupe filtered ProjectTable.git_url == git_url
(exact), while the orchestrator collapses its poll set by repo_key
(lower / strip trailing '/' / drop '.git'). Two projects whose git_url
differs only by those accidentals (a monorepo's cell-projects, or a
re-registered canonical project) defeated the one-open-task-per-repo
invariant and opened duplicate fix / dep-update tasks. Extract
roboco.utils.converters.repo_key as the single source and match the
dedupe query on its SQL mirror (regexp_replace(rtrim(lower(...)))).

The ci_watch (git_url, workflow) dedupe used func.coalesce(ci_watch_workflow,
default), but SQL COALESCE only substitutes for NULL — a project saved with
ci_watch_workflow='' (reachable via panel/API) yielded coalesce('', default)
= '' != default, so the DB diverged from the engine/orchestrator (which
collapse '' to the default via Python truthiness) and opened a duplicate
fix task every red cycle. Wrap with func.nullif(..., '') so an empty string
collapses to the default too.

Tests: a ''-workflow + NULL-workflow project on one repo dedupe to one task;
git_url accidentals (.git suffix / trailing slash) dedupe across both
ci_watch and dep_update. The orchestrator _repo_key now delegates to repo_key.

* [chore] admin_set_status: attribute the blocked-restore to the admin actor + emit override row (#2176)

admin_set_status taking a BLOCKED task to pending/in_progress with a
pre-block snapshot returned early via _apply_pre_block_restore, which
emitted its audit row with agent_role=None and audit_agent_id=restored_owner
(the pre-block dev) — the admin actor_id/actor_role were dropped entirely.
Because this branch runs with force=false (pending/in_progress aren't hatch
destinations), the distinguishing task.admin_override row (written only on
the non-restore path, gated by force) was never written, so an operator
could silently re-own a blocked task with no trace of who triggered it.

Thread actor_id/actor_role into _apply_pre_block_restore (admin_set_status
passes them with admin_override=True) so the transition audit row attributes
the re-owning to the admin, and emit a task.admin_override row (forced=False,
restore=True) on this branch independent of the force flag. The in-band
unblock(restore=True) path passes no actor and keeps the legacy attribution
(restored owner) with no override row.

Test: admin PATCH status=pending on a BLOCKED task with a snapshot attributes
every audit row to the admin (not the restored dev) and emits the override
row.

* [chore] converters: typed InvalidIdentifierError from require_uuid + log the orchestrator drop (#25)

require_uuid raised a bare ValueError('UUID value cannot be None'), so a
malformed/None identifier propagated as an opaque error callers either let
500 or broad-catch-and-silently-swallow — the orchestrator reaper call site
wrapped it in a bare except-Exception return with NO log, dropping a bad
task_id_str invisibly. Introduce InvalidIdentifierError(ValueError) and
raise it from require_uuid for both None and unparseable input; it stays a
ValueError subclass so existing except-ValueError / except-Exception callers
are unaffected, but typed so a caller can handle a bad identifier distinctly.
The reaper now catches the typed error, logs at warning, and no-ops — the
drop is visible instead of swallowed.

Tests: None and an unparseable string both raise InvalidIdentifierError; it
subclasses ValueError (back-comat).

* [sweep] notification_delivery: list_system_notifications over-fetch-then-slice for pending_ack_only

The SQL limit was applied before the post-fetch 'not fully acked' Python
filter. A window of newer fully-acked ack-required rows filled the limit
and masked older unacked notifications the operator still needs to act on
(the pending-ACK queue silently under-reported; a CEO-approval notification
could be hidden by newer already-acked noise). pending_ack_only now drops
the SQL limit, filters in Python, then slices to limit; the non-pending
branch keeps the SQL limit unchanged.

* [sweep] proactive: drop vestigial code-patterns surface from context package

Code indexing was removed, so _find_code_patterns always returned [] yet
build_context_package still called it, ContextPackage.code_patterns stayed
a live field, _build_summary advertised 'Found N code patterns', and
_count_items counted it — a permanently-empty slot the system claimed to
populate. The dead method, its call, the summary line, and the count
reference are removed. The code_patterns field itself is retained
(always-empty, serialized in to_dict and the optimal route response) for
API/schema back-compat, marked deprecated in its docstring.

* [sweep] migration 052: integration-test the task_cell_projects unique constraint

The UNIQUE(task_id, team) 'one project per cell per task' invariant was
only exercised through SimpleNamespace stubs that never touch a DB
session, so the real Postgres constraint was unverified. If it were
mis-declared or dropped, two same-team rows could coexist and
_resolve_subtask_project would non-deterministically return one, cutting
a subtask's branch/PR against the wrong repo. Adds an integration test
that inserts two same-(task_id, team) rows and asserts IntegrityError on
uq_task_cell_projects_task_team, plus a positive different-teams case.

* [sweep] pr_gate: classify MegaTask root-subtask as root so its root->master PR gets COMMENT (#608)

_post_gate_review_to_pr identified a root->master PR by absence of a
parent_task_id. A MegaTask root-subtask opens its own root->master PR into
the project's master (submit_root, parent='master') but carries
parent_task_id=umbrella, so is_root was False and the gate posted APPROVE
(pr_pass) / REQUEST_CHANGES (pr_fail) instead of COMMENT. The APPROVE could
satisfy a single-approval master branch-protection rule and let a non-CEO
merge via the GitHub UI before the CEO, against the documented invariant
that only the CEO acts on master. is_root now also covers
is_batch_root_subtask (batch_id set + parented); a non-batch cell-PM
coordination root keeps batch_id=None so it stays a cell->root PR
(APPROVE/REQUEST_CHANGES). Extends the _task test helper with a batch_id
kwarg.

* [sweep] enforcement: complete the status-class partition + coverage invariant (#247)

is_waiting_state already covered awaiting_pr_review (the primary fix), but
the doc's coverage invariant was missing: backlog and pending fell through
ALL three predicates (terminal/active/waiting), so a future enum addition
could silently land in no category. is_waiting_state now also covers
pending (waiting for a claim) and backlog (waiting on PM activation), so
is_terminal_state / is_active_state / is_waiting_state partition the whole
Status enum. Adds test_status_classification_covers_every_enum_member
asserting every Status member is classified by exactly one predicate, so
an enum addition that drifts the partition fails the build.

* [chore] test-suite: unblock the quality gate (mypy + 2 behavior fixes)

12 mypy errors across 5 test files: drop banned type:ignore comments
(lifecycle_spec monkeypatch uses cast(Any, ...); the ignores were unused),
wrap SQLAlchemy-typed ids with cast(UUID, ...) for AgentContext / WorkSession
args (AgentTable.id is Mapped[sqla UUID], not uuid.UUID), annotate **kw: Any,
and cast(Any, svc) for a method-assignment mock.

test_cancel_descendants_cascades_for_authorized_pm: the child was parked in
awaiting_ceo_approval, which the spec gates to CEO-only cancel
(lifecycle.py:378-389) — a cell_pm cascade correctly refuses it (the #103
refuse path). Use a PM-cancelable in_progress child so the positive-cascade
assertion holds; the refuse case is already covered by its sibling test.

test_a2a_message_auth: /message/send now resolves the authenticated
responder slug via get_agent_context (a DB lookup, #116). This is a DB-free
unit test of the token gate + route body, so stub get_agent_context in the
fixture — the gate (require_any_authenticated_agent) still runs real and
401s on a missing/forged token before that dependency resolves.

* [chore] complexity: split 5 C-rank blocks to <=B for the xenon gate

No behavior change; each C-rank function factored into a helper so the
complexity gate (xenon --max-absolute B) holds.

- lifecycle.can_invoke_action: extract the team-match check into
  _check_team_match.
- a2a.cancel_task: extract _status_value_of + _apply_cancel_note.
- task._apply_pre_block_restore: extract _restore_block_ownership (status/
  owner restore + snapshot clear) and _emit_admin_override_audit (#2176).
- release_proposal.approve: extract _finalize_release_lock (heartbeat/
  execute cancel + mutex release) out of the finally.
- kanban.get_main_pm_board_flat: dict-dispatch the column routing instead
  of a 7-branch if/elif ladder (status wins over team; in-flight + no cell
  team falls through to Coordination, #196).

* [chore] lifecycle artifacts: regenerate to match the spec (foundation-check)

The rendered artifacts (docs/rag/lifecycle, panel/lib/lifecycle.json, the
_generated role-prompt fragments) had drifted from the spec — the prior
sweep commits (cancel-CEO gate, claim_pr_review preconditions, pr_reviewer
unclaim, complete merge-first ordering) changed spec data without
regenerating, and the foundation-check render+diff stage never ran because
mypy failed earlier in the gate. make foundation-check now passes.

* [fix] chat: wire live message delivery end-to-end (MESSAGE_SENT)

send_message persisted messages but never broadcast them, there was no
MESSAGE_SENT event type or bridge forwarder, and the panel session view
had no websocket subscription — the live chat path was dead end-to-end.

- add EventType.MESSAGE_SENT and publish it best-effort on every persisted
  send (a bus outage logs, never rolls back the durable row)
- bridge _handle_message_event forwards to /ws/sessions/{id} and
  /ws/channels/{id}; subscribe it in register_websocket_bridge_handlers
- panel useSessionStream subscribes the session view; the page invalidates
  the transcript + session-detail queries on each message.new so the held
  (staleTime Infinity) views refresh live without the manual Refresh

* [fix] chat: return session task_links in one read; drop panel N+1

GET /sessions/{id} ran a bare select and session_to_response omitted
task_links, so it always returned them empty — the panel worked around it
with a triple-fetch (get session, get-tasks-for-session which re-fetched
the same endpoint, then a task GET per link), and the links never showed.

- add get_session_with_links(_or_raise) that eager-loads task_links -> task
- add session_to_response_with_links; GET /sessions/{id} uses both
- panel useSession now relies on the single populated response; remove the
  dead getTasksForSession + per-task fetch and the unused tasksApi import

* [fix] chat: validate reply_to against the effective session; guard closed-session composer

Posting to a closed session transparently redirects the message to the
group's active session (intended for agents holding stale refs), but
reply_to was validated against the requested session, not the one the
message lands in — letting a cross-session reply slip through — and the
panel silently posted there too, so the message vanished from the view.

- validate reply_to against session.id (the effective, possibly-redirected
  session), not req.session_id
- panel: render a "session is closed" notice instead of the composer for a
  non-active session; if a send still lands elsewhere (stale status), toast
  that it went to the active session rather than letting it appear to vanish

* [fix] chat: close session/group/message read IDOR; fix doubled 404s

get_session and the messages-list took an agent id but never used it, and
get_group took none at all — any authenticated agent could read any private
channel's group, session, and message transcripts. Three NotFoundError sites
also passed a full sentence as resource_type, yielding "... not found not found".

- add require_group_read_access / require_session_read_access (channel
  member / silent observer / privileged, mirroring list_group_sessions_for_agent)
  and get_session_with_links_for_agent; enforce on GET /sessions/{id},
  GET /sessions/{id}/tasks, GET /messages, GET /groups/{id} (-> 403 on deny)
- fix the three doubled-404 sites to the NotFoundError(resource_type, resource_id) form

Also folds two gate fixes for the prior chat commits: cast session.id to UUID
for the reply_to validation, and ruff import/format touch-ups.

Note: POST /messages intentionally still skips the channel write-ACL on the
HTTP (human-CEO/panel) path — the CEO is not in writers for 8/11 channels, so
enforcing it there would block the panel; the gateway/agent path enforces it.

* [fix] secretary: harden live chat — stuck spinner, mid-reply clobber, reload

The Secretary live chat had three live-behaviour bugs: a dropped SSE
connection left a permanent "thinking…" spinner (openStream set no
transport-error handler, so the no-data error Event was swallowed by the
JSON-parse guard and streaming never reset); sending mid-reply wiped the
accumulation buffer and pushed a user message without guarding the in-flight
turn, abandoning/duplicating the reply; and the chat lived only in React
state, so a reload wiped it.

- route the dual-purpose `error` listener: server-sent JSON → handleEvent,
  transport error (no data) → reset streaming, surface a notice, close stream
- guard send while streaming (streamingRef); disable the composer Send/Enter
  while a reply is in flight
- persist sessionId + messages to localStorage (TTL'd) and, on mount, restore
  + re-attach the stream once the backend confirms the session is still alive
  (mirrors the intake/prompter durability)

* [chore] groups: extract group-read helper to keep module rank A

The get_group IDOR access-check added try/except branches that tipped the
module to xenon rank B. Extract the service-error→HTTP mapping into a small
helper so get_group stays lean and the module is rank A again (behaviour
unchanged; covered by the groups route tests).

* [fix] chat: correct panel session-task mutation endpoints

linkTask/unlinkTask posted to /add-task and /remove-task (with a body), but
the backend exposes POST /sessions/{id}/tasks and DELETE
/sessions/{id}/tasks/{task_id} (path param) — so every call 404'd. updateTaskLink
targeted /update-task, a route that does not exist at all. Point linkTask and
unlinkTask at the real routes and drop the phantom updateTaskLink. All three were
unused, so no behaviour changes today — this removes a latent 404 trap.

* [docs] chat: document live message delivery (MESSAGE_SENT / message.new)

Document the live transcript-update path the chat-subsystem fixes wired:
- docs/api/websockets.md: add the message.new event-types row (carried on
  /ws/sessions + /ws/channels from EventType.MESSAGE_SENT) and note the
  forwarder sets type:"message.new"
- docs/panel/communications-and-journals.md: the session transcript updates
  live; a closed session is read-only (composer disabled)
- CLAUDE.md: name message.new on the per-resource streams and make
  MESSAGE_SENT the worked example of the add-a-live-event recipe

The internal roboco_map slices (gitignored) were updated in place to match.

* [docs] reconcile published docs with code since v0.13.0

Drift caught by the doc-reconciliation pass (all verified against HEAD):
- CLAUDE.md + rag: pr_reviewer gained the unclaim verb (16b71be8)
- rag permissions/task-states/task-tools: awaiting_ceo_approval -> cancelled is
  CEO-only, not PM+CEO (16b71be8 cancel-ceo-gate; lifecycle.py:373-382)
- deploy/env-reference: ROBOCO_APP_VERSION default 0.9.0 -> 0.14.0 (config.py:31);
  add ROBOCO_RELEASE_CI_WORKFLOW row (2759edf7, config.py:454)
- deploy/data-and-migrations: 44->54 revisions, head 054_a2a_message_skill
- optional/autonomous-maintenance: CI-watch dedupe is per (repo, workflow) (d34bc1a7)

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
2026-07-01 01:11:34 +02:00

802 lines
29 KiB
Python

"""flow_server wires gateway rejections into the SDK per-verb circuit breaker.
Phase 3 Task 14 added the SDK-side tracker (POST /verb/attempted,
GET /verb/circuit_status, Envelope.circuit_open). This module verifies
the *missing wiring*: rejection envelopes from the gateway must be
forwarded to the SDK, and when the breaker opens the rejection must be
substituted with the circuit_open envelope before reaching the agent.
The tests stub the orchestrator's httpx.Client (path '/api/v1/flow/...')
and the SDK's httpx.Client (path '/verb/attempted') so the helper can
be exercised end-to-end without a real network. We pick which mock to
return by inspecting the URL the code under test is hitting.
"""
from __future__ import annotations
import importlib
import json
from typing import TYPE_CHECKING, Any
from unittest.mock import MagicMock, patch
import pytest
if TYPE_CHECKING:
import types
from pathlib import Path
_FULL_MANIFEST = {
"agent_id": "00000000-0000-0000-0000-000000000099",
"role": "developer",
"team": "backend",
"workspace_path": "/tmp/test",
"flow_tools": [
"give_me_work",
"i_will_work_on",
"open_pr",
"i_am_done",
"i_am_blocked",
"unclaim",
"resume",
"i_am_idle",
"claim_review",
"pass",
"fail",
"claim_doc_task",
"i_documented",
"triage",
"triage_all",
"unblock",
"complete",
"escalate_up",
"i_will_plan",
"delegate",
"submit_up",
"escalate_to_ceo",
],
"do_tools": ["commit", "note", "say", "dm", "evidence"],
"read_tools": ["Read", "Glob", "Grep"],
"write_tools": ["Edit", "Write"],
"bash_allowed": True,
"subagent_allowed": False,
"subagent_model": None,
"env": {},
}
@pytest.fixture()
def flow_module(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> types.ModuleType:
"""Import flow_server with a tmp manifest + known orchestrator/SDK URLs."""
manifest_path = tmp_path / "tool-manifest.json"
manifest_path.write_text(json.dumps(_FULL_MANIFEST))
monkeypatch.setenv("ROBOCO_AGENT_ID", "00000000-0000-0000-0000-000000000099")
monkeypatch.setenv("ROBOCO_AGENT_ROLE", "developer")
monkeypatch.setenv("ROBOCO_ORCHESTRATOR_URL", "http://test-orchestrator:8000")
monkeypatch.setenv("ROBOCO_SDK_URL", "http://test-sdk:9000")
monkeypatch.setenv("ROBOCO_TOOL_MANIFEST_PATH", str(manifest_path))
import roboco.mcp.flow_server as srv
importlib.reload(srv)
return srv
def _make_client(
orchestrator_response: dict[str, Any], sdk_response: dict[str, Any] | None
) -> Any:
"""Build an httpx.Client mock that dispatches by destination URL.
Calls hitting ``test-orchestrator`` return ``orchestrator_response``;
calls hitting ``test-sdk`` return ``sdk_response``. The mock also
records every URL+body it sees so assertions can verify the SDK was
(or wasn't) called.
"""
captured: list[tuple[str, dict[str, Any] | None]] = []
def _client_factory(*_args: Any, **_kwargs: Any) -> MagicMock:
client = MagicMock()
client.__enter__ = MagicMock(return_value=client)
client.__exit__ = MagicMock(return_value=False)
def _post(url: str, **kwargs: Any) -> MagicMock:
captured.append((url, kwargs.get("json")))
resp = MagicMock()
if "test-sdk" in url:
if sdk_response is None:
raise AssertionError("SDK called unexpectedly")
resp.json.return_value = sdk_response
else:
resp.json.return_value = orchestrator_response
return resp
client.post.side_effect = _post
return client
return _client_factory, captured
# ---------------------------------------------------------------------------
# Successful envelope — SDK must NOT be called
# ---------------------------------------------------------------------------
def test_ok_envelope_does_not_touch_sdk(flow_module: types.ModuleType) -> None:
"""An envelope with error=None never POSTs to /verb/attempted."""
factory, captured = _make_client(
orchestrator_response={
"status": "awaiting_qa",
"task_id": "task-A",
"next": "wait",
"error": None,
},
sdk_response=None, # blow up if SDK is called
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A", notes="done")
assert result["status"] == "awaiting_qa"
assert result["error"] is None
# Only the orchestrator was contacted.
assert all("test-sdk" not in url for url, _ in captured)
# ---------------------------------------------------------------------------
# Rejection envelope — SDK gets the verb + task_id + rejection_kind
# ---------------------------------------------------------------------------
def test_rejection_forwards_to_sdk(flow_module: types.ModuleType) -> None:
"""A tracing_gap envelope triggers POST /verb/attempted with the right keys."""
factory, captured = _make_client(
orchestrator_response={
"error": "tracing_gap",
"missing": ["pr_number"],
"remediate": "open the PR",
},
sdk_response={
"verb": "i_am_done",
"task_id": "task-A",
"attempts": 1,
"limit": 3,
"window_seconds": 60,
"open": False,
"circuit_envelope": None,
},
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A", notes="done")
# Original rejection survives — breaker is not yet open.
assert result["error"] == "tracing_gap"
# Check the SDK saw the right payload.
sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url]
assert len(sdk_calls) == 1
sdk_url, sdk_body = sdk_calls[0]
assert sdk_url.endswith("/verb/attempted")
assert sdk_body == {
"verb": "i_am_done",
"task_id": "task-A",
"rejection_kind": "tracing_gap",
}
@pytest.mark.parametrize(
"rejection_kind",
["tracing_gap", "invalid_state", "not_authorized", "incomplete_input"],
)
def test_all_counted_rejection_kinds_forwarded(
flow_module: types.ModuleType, rejection_kind: str
) -> None:
"""All four counted error kinds forward to the SDK."""
factory, captured = _make_client(
orchestrator_response={
"error": rejection_kind,
"message": "no",
"remediate": "fix",
},
sdk_response={
"verb": "i_am_done",
"task_id": "task-A",
"attempts": 1,
"limit": 3,
"window_seconds": 60,
"open": False,
"circuit_envelope": None,
},
)
with patch("httpx.Client", side_effect=factory):
flow_module.i_am_done("task-A")
sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url]
assert len(sdk_calls) == 1
_, sdk_body = sdk_calls[0]
assert sdk_body["rejection_kind"] == rejection_kind
def test_other_error_kinds_do_not_touch_sdk(flow_module: types.ModuleType) -> None:
"""not_found / transport_error aren't counted — SDK is not called."""
factory, captured = _make_client(
orchestrator_response={"error": "not_found", "message": "no task"},
sdk_response=None,
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A")
assert result["error"] == "not_found"
assert all("test-sdk" not in url for url, _ in captured)
# ---------------------------------------------------------------------------
# Breaker open — envelope substituted with circuit_open
# ---------------------------------------------------------------------------
def test_breaker_open_substitutes_envelope(flow_module: types.ModuleType) -> None:
"""When SDK reports open=true, the agent gets the circuit_open envelope."""
circuit_env: dict[str, Any] = {
"error": "circuit_open",
"message": ("verb 'i_am_done' rejected 3 times in last 60s — breaker open"),
"remediate": "call i_am_blocked or i_am_idle",
"context_briefing": {},
"status": None,
"task_id": None,
"next": None,
"evidence": {},
"correlation_id": None,
"current_state": None,
"valid_next_verbs": None,
}
factory, _ = _make_client(
orchestrator_response={"error": "tracing_gap", "remediate": "x"},
sdk_response={
"verb": "i_am_done",
"task_id": "task-A",
"attempts": 3,
"limit": 3,
"window_seconds": 60,
"open": True,
"circuit_envelope": circuit_env,
},
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A")
# The top-level envelope is the circuit_open substitution; the original
# fixable rejection is preserved nested as ``inner`` (#60).
assert result["error"] == "circuit_open"
assert "i_am_blocked" in result["remediate"]
assert result["inner"]["error"] == "tracing_gap"
assert result["inner"]["remediate"] == "x"
def test_breaker_open_preserves_original_as_inner(
flow_module: types.ModuleType,
) -> None:
"""#60: the breaker substitution must not erase the fixable rejection.
The agent still needs the original kind/message/remediate to understand
WHY the verb failed — the circuit_open envelope only says the breaker
tripped. The original rejection is nested under ``inner``.
"""
circuit_env: dict[str, Any] = {
"error": "circuit_open",
"message": "verb 'i_am_done' rejected 3 times in 60s — breaker open",
"remediate": "call i_am_blocked or i_am_idle",
}
original = {
"error": "not_authorized",
"message": "you may not merge that PR",
"remediate": "PMs delegate; use delegate(...)",
"missing": [],
}
factory, _ = _make_client(
orchestrator_response=original,
sdk_response={
"verb": "i_am_done",
"task_id": "task-A",
"attempts": 3,
"limit": 3,
"open": True,
"circuit_envelope": circuit_env,
},
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A")
assert result["error"] == "circuit_open"
# The original fixable rejection survives nested, not erased.
assert result["inner"] == original
# The SDK's envelope dict is not mutated in place (a fresh copy carries
# ``inner``) — important when the SDK reuses the envelope across calls.
assert "inner" not in circuit_env
def test_fourth_rejection_returns_circuit_open(flow_module: types.ModuleType) -> None:
"""Hitting the cap on the Nth call yields circuit_open on that same call.
The SDK records the attempt FIRST and reports open=true on the
response that just pushed it over the threshold — so the call that
trips the breaker is also the call that sees the substitution.
"""
# On the trip call the SDK reports open=true with the envelope.
circuit_env: dict[str, Any] = {
"error": "circuit_open",
"message": ("verb 'i_am_done' rejected 3 times in last 60s — breaker open"),
"remediate": "call i_am_blocked(reason='...') or i_am_idle()",
"context_briefing": {},
"status": None,
"task_id": None,
"next": None,
"evidence": {},
"correlation_id": None,
"current_state": None,
"valid_next_verbs": None,
}
factory, captured = _make_client(
orchestrator_response={"error": "tracing_gap", "remediate": "x"},
sdk_response={
"verb": "i_am_done",
"task_id": "task-A",
"attempts": 3,
"limit": 3,
"window_seconds": 60,
"open": True,
"circuit_envelope": circuit_env,
},
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A")
# Substituted envelope shown to the agent.
assert result["error"] == "circuit_open"
# Both calls happened: orchestrator (the verb) then SDK (record).
urls = [url for url, _ in captured]
assert any("test-orchestrator" in u for u in urls)
assert any("test-sdk" in u for u in urls)
# ---------------------------------------------------------------------------
# Fail-open behaviour — SDK down must not break the gateway path
# ---------------------------------------------------------------------------
def test_sdk_unreachable_fails_open(flow_module: types.ModuleType) -> None:
"""If the SDK raises, the agent still sees the original rejection."""
import httpx
def _client_factory(*_args: Any, **_kwargs: Any) -> MagicMock:
client = MagicMock()
client.__enter__ = MagicMock(return_value=client)
client.__exit__ = MagicMock(return_value=False)
def _post(url: str, **_kwargs: Any) -> MagicMock:
if "test-sdk" in url:
raise httpx.ConnectError("SDK down")
resp = MagicMock()
resp.json.return_value = {
"error": "tracing_gap",
"missing": ["pr_number"],
"remediate": "open the PR first",
}
return resp
client.post.side_effect = _post
return client
with patch("httpx.Client", side_effect=_client_factory):
result = flow_module.i_am_done("task-A")
# SDK was unreachable — original envelope passes through.
assert result["error"] == "tracing_gap"
assert result["remediate"] == "open the PR first"
def test_sdk_returns_malformed_json_fails_open(flow_module: types.ModuleType) -> None:
"""If the SDK responds with un-JSON-able body, the original envelope wins."""
def _client_factory(*_args: Any, **_kwargs: Any) -> MagicMock:
client = MagicMock()
client.__enter__ = MagicMock(return_value=client)
client.__exit__ = MagicMock(return_value=False)
def _post(url: str, **_kwargs: Any) -> MagicMock:
resp = MagicMock()
if "test-sdk" in url:
resp.json.side_effect = ValueError("not json")
else:
resp.json.return_value = {
"error": "invalid_state",
"message": "wrong state",
"remediate": "transition first",
}
return resp
client.post.side_effect = _post
return client
with patch("httpx.Client", side_effect=_client_factory):
result = flow_module.i_am_done("task-A")
assert result["error"] == "invalid_state"
assert result["message"] == "wrong state"
# ---------------------------------------------------------------------------
# Verb extraction
# ---------------------------------------------------------------------------
def test_verb_from_path_extracts_last_segment(flow_module: types.ModuleType) -> None:
"""_verb_from_path strips the role prefix and returns the verb token."""
assert (
flow_module._verb_from_path("/api/v1/flow/developer/i_am_done") == "i_am_done"
)
assert flow_module._verb_from_path("/api/v1/flow/qa/pass") == "pass"
assert (
flow_module._verb_from_path("/api/v1/flow/board/escalate_to_ceo")
== "escalate_to_ceo"
)
# ---------------------------------------------------------------------------
# task_id pass-through
# ---------------------------------------------------------------------------
def test_task_id_none_is_forwarded_as_null(flow_module: types.ModuleType) -> None:
"""Verbs without a task_id (e.g. give_me_work) post task_id=None to the SDK."""
factory, captured = _make_client(
orchestrator_response={"error": "tracing_gap", "remediate": "x"},
sdk_response={
"verb": "give_me_work",
"task_id": None,
"attempts": 1,
"limit": None, # unlimited verb
"window_seconds": 60,
"open": False,
"circuit_envelope": None,
},
)
with patch("httpx.Client", side_effect=factory):
flow_module.give_me_work()
sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url]
assert len(sdk_calls) == 1
_, sdk_body = sdk_calls[0]
assert sdk_body["task_id"] is None
assert sdk_body["verb"] == "give_me_work"
# ---------------------------------------------------------------------------
# Non-string-error / no-error-field rejection shapes are counted
# ---------------------------------------------------------------------------
def test_422_validation_failure_counts_as_incomplete_input(
flow_module: types.ModuleType,
) -> None:
"""A 422 validation-failure body (`{"detail": [...]}`, no `error`) must count
toward the breaker as `incomplete_input` — a storm of 422s is retry-storm-worthy.
Mirrors do_server's classifier (the two servers share the same breaker logic).
"""
factory, captured = _make_client(
orchestrator_response={
"detail": [
{
"loc": ["body", "task_id"],
"msg": "field required",
"type": "value_error.missing",
}
],
"body": None,
},
sdk_response={
"verb": "i_am_done",
"task_id": "task-A",
"attempts": 1,
"limit": 3,
"window_seconds": 60,
"open": False,
"circuit_envelope": None,
},
)
with patch("httpx.Client", side_effect=factory):
flow_module.i_am_done("task-A")
sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url]
assert len(sdk_calls) == 1
assert sdk_calls[0][1]["rejection_kind"] == "incomplete_input"
def test_dict_shaped_internal_error_counts_as_invalid_state(
flow_module: types.ModuleType,
) -> None:
"""A 500 INTERNAL_ERROR dict-shaped response (generic_exception_handler) counts
as `invalid_state` — a storm of 500s is retry-storm-worthy.
"""
factory, captured = _make_client(
orchestrator_response={
"error": {
"code": "INTERNAL_ERROR",
"message": "An internal error occurred",
"details": {"correlation_id": "abc"},
}
},
sdk_response={
"verb": "i_am_done",
"task_id": "task-A",
"attempts": 1,
"limit": 3,
"window_seconds": 60,
"open": False,
"circuit_envelope": None,
},
)
with patch("httpx.Client", side_effect=factory):
flow_module.i_am_done("task-A")
sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url]
assert len(sdk_calls) == 1
assert sdk_calls[0][1]["rejection_kind"] == "invalid_state"
def test_dict_shaped_not_found_normalized_to_envelope_and_not_counted(
flow_module: types.ModuleType,
) -> None:
"""A dict-shaped NOT_FOUND (404 family) is normalized to an Envelope
(`error="not_found"` string + lifted message + remediate) — the agent
never sees a dict `error`, which violates the Envelope wire format — and
the breaker is NOT touched (parity with the string `not_found` contract).
"""
factory, captured = _make_client(
orchestrator_response={
"error": {"code": "TASK_NOT_FOUND", "message": "no such task"}
},
sdk_response=None, # SDK must not be touched
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A")
# Normalized to a string-kind Envelope — the dict `error` never reaches
# the agent (#232).
assert result["error"] == "not_found"
assert result["message"] == "no such task"
assert isinstance(result["remediate"], str) and result["remediate"]
assert result["missing"] == []
# NOT_FOUND still does not count toward the breaker.
assert all("test-sdk" not in url for url, _ in captured)
# ---------------------------------------------------------------------------
# A manifest-registered verb whose route is missing must return an envelope
# rejection (not a raw 404 body) so the breaker counts it.
# ---------------------------------------------------------------------------
def _make_404_client() -> tuple[Any, list[tuple[str, dict[str, Any] | None]]]:
"""Build an httpx.Client mock whose orchestrator call returns FastAPI's
default 404 body (``{"detail": "Not Found"}``, status 404) — the shape a
manifest-registered verb sees when its route is missing. The SDK call
returns a not-yet-open breaker so the original envelope is preserved.
"""
captured: list[tuple[str, dict[str, Any] | None]] = []
def _client_factory(*_args: Any, **_kwargs: Any) -> MagicMock:
client = MagicMock()
client.__enter__ = MagicMock(return_value=client)
client.__exit__ = MagicMock(return_value=False)
def _post(url: str, **kwargs: Any) -> MagicMock:
captured.append((url, kwargs.get("json")))
resp = MagicMock()
if "test-sdk" in url:
resp.json.return_value = {
"verb": "triage",
"task_id": None,
"attempts": 1,
"limit": 3,
"window_seconds": 60,
"open": False,
"circuit_envelope": None,
}
else:
# FastAPI's default 404 for a missing route.
resp.status_code = 404
resp.json.return_value = {"detail": "Not Found"}
return resp
client.post.side_effect = _post
return client
return _client_factory, captured
def test_missing_route_404_returns_envelope_and_counts(
flow_module: types.ModuleType,
) -> None:
"""A 404 from the orchestrator (manifest-registered verb with no route) must
surface as a proper `invalid_state` Envelope rejection — not FastAPI's raw
``{"detail": "Not Found"}`` body — and the breaker must count it.
"""
factory, captured = _make_404_client()
with patch("httpx.Client", side_effect=factory):
result = flow_module.triage()
# Envelope rejection, not the raw 404 body.
assert result["error"] == "invalid_state"
assert "remediate" in result
assert "detail" not in result # the raw 404 body was not passed through
# Breaker was notified so a storm of these trips it.
sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url]
assert len(sdk_calls) == 1
_, sdk_body = sdk_calls[0]
assert sdk_body is not None
assert sdk_body["rejection_kind"] == "invalid_state"
# ---------------------------------------------------------------------------
# #232: a non-404 JSON exception-handler body (dict `error` / 422 `detail`)
# must be normalized to an Envelope wire format before reaching the agent.
# ---------------------------------------------------------------------------
def test_exception_dict_500_normalized_to_invalid_state_envelope(
flow_module: types.ModuleType,
) -> None:
"""A 500 with a dict-shaped error (generic_exception_handler) is normalized
to an Envelope: top-level `error='invalid_state'` (string kind), the dict's
`message` lifted to top-level `message`, a non-empty `remediate`, `missing=[]`.
The agent never receives a dict `error` (which violates the Envelope contract
it's prompted to trust). The breaker still counts it as `invalid_state`."""
factory, captured = _make_client(
orchestrator_response={
"error": {
"code": "INTERNAL_ERROR",
"message": "An internal error occurred",
"details": {"correlation_id": "abc"},
}
},
sdk_response={
"verb": "i_am_done",
"task_id": "task-A",
"attempts": 1,
"limit": 3,
"window_seconds": 60,
"open": False,
"circuit_envelope": None,
},
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A")
assert result["error"] == "invalid_state"
assert result["message"] == "An internal error occurred"
assert isinstance(result["remediate"], str) and result["remediate"]
assert result["missing"] == []
# The synthesized Envelope still flows through the breaker (string kind in
# the counted set) so a 500 storm trips it.
sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url]
assert len(sdk_calls) == 1
assert sdk_calls[0][1]["rejection_kind"] == "invalid_state"
def test_422_detail_normalized_to_incomplete_input_envelope(
flow_module: types.ModuleType,
) -> None:
"""A 422 request-validation body (`{"detail": [...]}`, no `error`) is
normalized to an Envelope with `error='incomplete_input'`, a non-empty
`remediate`, and `missing=[]`; the raw validation `detail` is preserved so
the agent can see WHICH fields failed. The breaker still counts it."""
detail_body = [
{"loc": ["body", "task_id"], "msg": "field required", "type": "missing"}
]
factory, captured = _make_client(
orchestrator_response={"detail": detail_body, "body": None},
sdk_response={
"verb": "i_am_done",
"task_id": "task-A",
"attempts": 1,
"limit": 3,
"window_seconds": 60,
"open": False,
"circuit_envelope": None,
},
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A")
assert result["error"] == "incomplete_input"
assert isinstance(result["remediate"], str) and result["remediate"]
assert result["missing"] == []
# The validation detail survives so the agent knows which fields to fix.
assert result["detail"] == detail_body
sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url]
assert len(sdk_calls) == 1
assert sdk_calls[0][1]["rejection_kind"] == "incomplete_input"
def test_dict_authorized_normalized_to_not_authorized_envelope(
flow_module: types.ModuleType,
) -> None:
"""A dict-shaped PERMISSION_DENIED maps to `error='not_authorized'` in the
normalized Envelope (exact-code map, not a substring accident — #161), with
the message lifted and a remediate. The breaker counts it as not_authorized."""
factory, captured = _make_client(
orchestrator_response={
"error": {
"code": "PERMISSION_DENIED",
"message": "you may not merge that PR",
"details": {},
}
},
sdk_response={
"verb": "i_am_done",
"task_id": "task-A",
"attempts": 1,
"limit": 3,
"window_seconds": 60,
"open": False,
"circuit_envelope": None,
},
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A")
assert result["error"] == "not_authorized"
assert result["message"] == "you may not merge that PR"
assert isinstance(result["remediate"], str) and result["remediate"]
assert result["missing"] == []
sdk_calls = [(url, body) for url, body in captured if "test-sdk" in url]
assert len(sdk_calls) == 1
assert sdk_calls[0][1]["rejection_kind"] == "not_authorized"
# ---------------------------------------------------------------------------
# #359: the circuit_open substitution lifts task_id/correlation_id from the
# original rejection to the top-level envelope (the SDK's envelope omits them).
# ---------------------------------------------------------------------------
def test_circuit_open_lifts_task_id_and_correlation_id(
flow_module: types.ModuleType,
) -> None:
"""When the breaker trips, the SDK's circuit_envelope carries no task_id /
correlation_id, but the original rejection payload does (the orchestrator
stamps them on every envelope). The substitution must lift them to the
top-level so the agent's envelope contract (read top-level task_id /
correlation_id) and ops audit-join still work — not leave them only nested
in `inner`."""
circuit_env: dict[str, Any] = {
"error": "circuit_open",
"message": "verb 'i_am_done' rejected 3 times in 60s — breaker open",
"remediate": "call i_am_blocked or i_am_idle",
}
factory, _ = _make_client(
orchestrator_response={
"error": "tracing_gap",
"remediate": "open the PR",
"task_id": "T1",
"correlation_id": "C1",
},
sdk_response={
"verb": "i_am_done",
"task_id": "T1",
"attempts": 3,
"limit": 3,
"window_seconds": 60,
"open": True,
"circuit_envelope": circuit_env,
},
)
with patch("httpx.Client", side_effect=factory):
result = flow_module.i_am_done("task-A")
assert result["error"] == "circuit_open"
# Lifted from the original rejection to the top level (not just in inner).
assert result["task_id"] == "T1"
assert result["correlation_id"] == "C1"
# The original rejection still survives nested.
assert result["inner"]["error"] == "tracing_gap"
assert result["inner"]["task_id"] == "T1"