mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
* [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>
1143 lines
42 KiB
Python
1143 lines
42 KiB
Python
"""Tier 1 — spec self-tests. Fast (no DB, no network)."""
|
|
|
|
from __future__ import annotations
|
|
|
|
from types import SimpleNamespace
|
|
from typing import Any, cast
|
|
from uuid import uuid4
|
|
|
|
import pytest
|
|
from roboco.foundation import _validate_lifecycle as _validate
|
|
from roboco.foundation._validate_lifecycle import reachable_from
|
|
from roboco.foundation.policy import lifecycle as spec
|
|
from roboco.foundation.policy.lifecycle import _INTENT_VERBS, IntentSpec
|
|
from roboco.models.base import TaskStatus as ModelTaskStatus
|
|
from roboco.models.base import TaskType as ModelTaskType
|
|
|
|
|
|
def test_role_enum_has_every_pre_gateway_role() -> None:
|
|
"""Every role from PERMISSIONS.md must be enumerated.
|
|
|
|
The canonical Role enum is now defined in `roboco.foundation.identity`
|
|
and re-exported here. It includes the 9 pre-gateway roles plus the
|
|
SYSTEM sentinel used for orchestrator-generated rows. The pre-gateway
|
|
PERMISSIONS.md is the historical canon — SYSTEM is the post-foundation
|
|
addition that doesn't appear in policy tables.
|
|
"""
|
|
expected = {
|
|
"developer",
|
|
"qa",
|
|
"documenter",
|
|
"cell_pm",
|
|
"main_pm",
|
|
"product_owner",
|
|
"head_marketing",
|
|
"auditor",
|
|
"pr_reviewer", # reviews inbound external/fork PRs (read-only)
|
|
"prompter", # post-gateway intake role (human-only, drafts tasks)
|
|
"secretary", # CEO's chief-of-staff (human-only, gated CEO authority)
|
|
"ceo",
|
|
"system",
|
|
}
|
|
actual = {r.value for r in spec.Role}
|
|
assert actual == expected, f"Role enum drift: {actual ^ expected}"
|
|
|
|
|
|
def test_status_enum_has_every_pre_gateway_status() -> None:
|
|
"""Every status from STATUS_TRANSITIONS.md must be enumerated."""
|
|
expected = {
|
|
"backlog",
|
|
"pending",
|
|
"claimed",
|
|
"in_progress",
|
|
"blocked",
|
|
"paused",
|
|
"verifying",
|
|
"awaiting_qa",
|
|
"needs_revision",
|
|
"awaiting_documentation",
|
|
"awaiting_pr_review",
|
|
"awaiting_pm_review",
|
|
"awaiting_ceo_approval",
|
|
"completed",
|
|
"cancelled",
|
|
}
|
|
actual = {s.value for s in spec.Status}
|
|
assert actual == expected, f"Status enum drift: {actual ^ expected}"
|
|
|
|
|
|
def test_task_type_enum_matches_models() -> None:
|
|
"""The spec's TaskType must match the existing models.base.TaskType.
|
|
|
|
If the existing model adds/removes a type, the spec must be updated
|
|
in lockstep — that's the entire point of this module.
|
|
"""
|
|
spec_values = {t.value for t in spec.TaskType}
|
|
model_values = {t.value for t in ModelTaskType}
|
|
assert spec_values == model_values, (
|
|
f"TaskType drift between lifecycle.spec and models.base: "
|
|
f"{spec_values ^ model_values}"
|
|
)
|
|
|
|
|
|
def test_status_enum_matches_models() -> None:
|
|
"""The spec's Status must match models.base.TaskStatus — the ORM column
|
|
type and the lifecycle map must not drift. TaskType has this guard; Status
|
|
did not, so adding/renaming a status in one enum only wedged silently."""
|
|
spec_values = {s.value for s in spec.Status}
|
|
model_values = {s.value for s in ModelTaskStatus}
|
|
assert spec_values == model_values, (
|
|
f"Status drift between lifecycle.spec and models.base: "
|
|
f"{spec_values ^ model_values}"
|
|
)
|
|
|
|
|
|
def test_status_enum_parity_validator_passes_on_real_spec() -> None:
|
|
"""The import-time parity validator must agree with the real enums."""
|
|
_validate._check_status_enum_parity() # no raise
|
|
|
|
|
|
def test_status_coverage_rejects_stray_string_target() -> None:
|
|
"""A transition referencing a non-Status target string must fail the
|
|
coverage validator — the old check was a tautology (STATUS_GRAPH keys every
|
|
Status by construction) and let stray-string targets through."""
|
|
fake = (
|
|
SimpleNamespace(
|
|
source=spec.Status.PENDING, target="bogus_state", triggered_by_action="x"
|
|
),
|
|
)
|
|
original = spec._STATUS_TRANSITIONS
|
|
spec._STATUS_TRANSITIONS = cast("Any", fake)
|
|
try:
|
|
with pytest.raises(_validate.LifecycleSpecError, match="non-Status"):
|
|
_validate._check_status_enum_coverage()
|
|
finally:
|
|
spec._STATUS_TRANSITIONS = original
|
|
|
|
|
|
def test_status_coverage_rejects_orphan_non_terminal_source() -> None:
|
|
"""A non-terminal status that is the source of no transition (an orphan
|
|
state) must fail the coverage validator — the cancel fan-out made the old
|
|
'is a key in STATUS_GRAPH' check structurally always-true."""
|
|
original = spec._STATUS_TRANSITIONS
|
|
spec._STATUS_TRANSITIONS = tuple(
|
|
t for t in original if t.source is not spec.Status.PAUSED
|
|
)
|
|
try:
|
|
with pytest.raises(
|
|
_validate.LifecycleSpecError, match="no outgoing transition"
|
|
):
|
|
_validate._check_status_enum_coverage()
|
|
finally:
|
|
spec._STATUS_TRANSITIONS = original
|
|
|
|
|
|
def test_terminal_exit_requires_a_completed_path() -> None:
|
|
"""Every non-terminal status must reach COMPLETED specifically — the cancel
|
|
fan-out made the old {COMPLETED, CANCELLED} check trivial, so a status whose
|
|
sole exit was cancel passed the guard with no real forward completion path."""
|
|
original = spec.STATUS_GRAPH
|
|
fake = dict(original)
|
|
fake[spec.Status.PAUSED] = frozenset({spec.Status.CANCELLED})
|
|
spec.STATUS_GRAPH = fake
|
|
try:
|
|
with pytest.raises(_validate.LifecycleSpecError, match="no path to COMPLETED"):
|
|
_validate._check_terminal_exits()
|
|
finally:
|
|
spec.STATUS_GRAPH = original
|
|
|
|
|
|
def test_decision_allow_has_no_rejection_kind() -> None:
|
|
d = spec.Decision.allow()
|
|
assert d.allowed is True
|
|
assert d.rejection_kind is None
|
|
assert d.message is None
|
|
assert d.missing == []
|
|
assert d.remediate is None
|
|
|
|
|
|
def test_decision_reject_requires_rejection_kind() -> None:
|
|
d = spec.Decision.reject(
|
|
kind="not_authorized",
|
|
message="role 'developer' may not call delegate",
|
|
remediate="only PMs delegate; call give_me_work() instead",
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "not_authorized"
|
|
assert d.message == "role 'developer' may not call delegate"
|
|
assert d.remediate == "only PMs delegate; call give_me_work() instead"
|
|
|
|
|
|
def test_decision_tracing_gap_carries_missing_list() -> None:
|
|
d = spec.Decision.tracing_gap(
|
|
missing=["plan", "journal:decision"],
|
|
remediate="provide plan and a journal:decision entry",
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "tracing_gap"
|
|
assert d.missing == ["plan", "journal:decision"]
|
|
assert d.remediate == "provide plan and a journal:decision entry"
|
|
|
|
|
|
def test_decision_tracing_gap_defensively_copies_missing() -> None:
|
|
"""tracing_gap must isolate the stored list from the caller's source."""
|
|
src = ["plan"]
|
|
d = spec.Decision.tracing_gap(missing=src, remediate="r")
|
|
src.append("mutated")
|
|
assert d.missing == ["plan"]
|
|
|
|
|
|
def test_decision_invariants_enforced_at_construction() -> None:
|
|
"""allowed=True ⇒ rejection_kind None; allowed=False ⇒ kind set."""
|
|
with pytest.raises(ValueError, match="allowed=True requires rejection_kind=None"):
|
|
spec.Decision(
|
|
allowed=True,
|
|
rejection_kind="not_authorized",
|
|
message="x",
|
|
missing=[],
|
|
remediate="x",
|
|
)
|
|
with pytest.raises(ValueError, match="allowed=False requires rejection_kind"):
|
|
spec.Decision(
|
|
allowed=False,
|
|
rejection_kind=None,
|
|
message="x",
|
|
missing=[],
|
|
remediate="x",
|
|
)
|
|
|
|
|
|
def test_decision_invariant_rejects_allowed_with_missing_or_remediate() -> None:
|
|
"""allowed=True with missing or remediate set raises (Fix 1 lock-in)."""
|
|
with pytest.raises(
|
|
ValueError, match="allowed=True requires missing=\\[\\] and remediate=None"
|
|
):
|
|
spec.Decision(
|
|
allowed=True,
|
|
rejection_kind=None,
|
|
message=None,
|
|
missing=["plan"],
|
|
remediate=None,
|
|
)
|
|
with pytest.raises(
|
|
ValueError,
|
|
match="allowed=True requires missing=\\[\\] and remediate=None",
|
|
):
|
|
spec.Decision(
|
|
allowed=True,
|
|
rejection_kind=None,
|
|
message=None,
|
|
missing=[],
|
|
remediate="oops",
|
|
)
|
|
|
|
|
|
def test_precondition_check_returns_bool() -> None:
|
|
"""A Precondition.check() is the gate-table evaluator."""
|
|
p = spec.Precondition(
|
|
key="commits>=1",
|
|
check=lambda task, _agent, _ctx: bool(getattr(task, "commits", None)),
|
|
remediate="commit at least once before opening a PR",
|
|
missing_token="commits>=1",
|
|
)
|
|
|
|
task_with = SimpleNamespace(commits=["abc"])
|
|
task_without = SimpleNamespace(commits=[])
|
|
assert p.check(task_with, None, None) is True
|
|
assert p.check(task_without, None, None) is False
|
|
|
|
|
|
def test_action_spec_holds_role_status_and_precondition_data() -> None:
|
|
a = spec.ActionSpec(
|
|
name="claim",
|
|
allowed_roles=frozenset({spec.Role.DEVELOPER}),
|
|
source_statuses=frozenset({spec.Status.PENDING, spec.Status.NEEDS_REVISION}),
|
|
target_status=spec.Status.CLAIMED,
|
|
allowed_task_types=None,
|
|
preconditions=(),
|
|
self_review_block=False,
|
|
needs_team_match=True,
|
|
)
|
|
assert a.name == "claim"
|
|
assert spec.Role.DEVELOPER in a.allowed_roles
|
|
assert a.target_status == spec.Status.CLAIMED
|
|
|
|
|
|
def test_intent_spec_composes_atomic_actions() -> None:
|
|
i = spec.IntentSpec(
|
|
name="i_will_work_on",
|
|
allowed_roles=frozenset({spec.Role.DEVELOPER}),
|
|
description="Claim a task and start work on it.",
|
|
composes=("claim", "set_plan", "start"),
|
|
extra_preconditions=(),
|
|
side_effects=(),
|
|
next_hint=lambda _t: "edit + commit, then open_pr",
|
|
)
|
|
assert i.composes == ("claim", "set_plan", "start")
|
|
assert i.next_hint(None) == "edit + commit, then open_pr"
|
|
|
|
|
|
def test_status_transition_carries_role_constraint_optional() -> None:
|
|
t = spec.StatusTransition(
|
|
source=spec.Status.AWAITING_QA,
|
|
target=spec.Status.AWAITING_DOCUMENTATION,
|
|
triggered_by_action="qa_pass",
|
|
role_constraint=frozenset({spec.Role.QA}),
|
|
)
|
|
assert t.source == spec.Status.AWAITING_QA
|
|
assert t.target == spec.Status.AWAITING_DOCUMENTATION
|
|
assert t.triggered_by_action == "qa_pass"
|
|
assert t.role_constraint == frozenset({spec.Role.QA})
|
|
|
|
|
|
def test_status_transitions_includes_dev_path() -> None:
|
|
"""The dev happy path: pending → claimed → in_progress → verifying → awaiting_qa."""
|
|
sources = {(t.source, t.target) for t in spec._STATUS_TRANSITIONS}
|
|
assert (spec.Status.PENDING, spec.Status.CLAIMED) in sources
|
|
assert (spec.Status.CLAIMED, spec.Status.IN_PROGRESS) in sources
|
|
assert (spec.Status.IN_PROGRESS, spec.Status.VERIFYING) in sources
|
|
assert (spec.Status.VERIFYING, spec.Status.AWAITING_QA) in sources
|
|
|
|
|
|
def test_status_transitions_includes_qa_paths() -> None:
|
|
sources = {(t.source, t.target) for t in spec._STATUS_TRANSITIONS}
|
|
assert (spec.Status.AWAITING_QA, spec.Status.CLAIMED) in sources # QA claims
|
|
assert (spec.Status.AWAITING_QA, spec.Status.AWAITING_DOCUMENTATION) in sources
|
|
assert (spec.Status.AWAITING_QA, spec.Status.NEEDS_REVISION) in sources
|
|
|
|
|
|
def test_status_transitions_includes_ceo_paths() -> None:
|
|
sources = {(t.source, t.target) for t in spec._STATUS_TRANSITIONS}
|
|
assert (spec.Status.AWAITING_PM_REVIEW, spec.Status.COMPLETED) in sources
|
|
assert (
|
|
spec.Status.AWAITING_PM_REVIEW,
|
|
spec.Status.AWAITING_CEO_APPROVAL,
|
|
) in sources
|
|
assert (spec.Status.AWAITING_CEO_APPROVAL, spec.Status.COMPLETED) in sources
|
|
assert (spec.Status.AWAITING_CEO_APPROVAL, spec.Status.NEEDS_REVISION) in sources
|
|
# #100: a branchless coordination root rejected by the CEO routes to PENDING
|
|
# (Main PM re-plans) — the edge is in the spec so the audited privileged
|
|
# override that applies it can't be wedged by future admin-override tightening.
|
|
assert (spec.Status.AWAITING_CEO_APPROVAL, spec.Status.PENDING) in sources
|
|
# A blocked task the PM cannot resolve can also be surfaced to the CEO.
|
|
assert (spec.Status.BLOCKED, spec.Status.AWAITING_CEO_APPROVAL) in sources
|
|
|
|
|
|
def test_status_transitions_includes_block_pause_paths() -> None:
|
|
sources = {(t.source, t.target) for t in spec._STATUS_TRANSITIONS}
|
|
assert (spec.Status.IN_PROGRESS, spec.Status.BLOCKED) in sources
|
|
assert (spec.Status.IN_PROGRESS, spec.Status.PAUSED) in sources
|
|
assert (spec.Status.BLOCKED, spec.Status.IN_PROGRESS) in sources
|
|
assert (spec.Status.PAUSED, spec.Status.IN_PROGRESS) in sources
|
|
|
|
|
|
def test_every_non_terminal_status_can_be_cancelled() -> None:
|
|
"""PERMISSIONS.md says PM/CEO can cancel from any state."""
|
|
cancellable = {
|
|
t.source for t in spec._STATUS_TRANSITIONS if t.target == spec.Status.CANCELLED
|
|
}
|
|
non_terminal = set(spec.Status) - {spec.Status.COMPLETED, spec.Status.CANCELLED}
|
|
assert non_terminal <= cancellable, (
|
|
f"Statuses missing a cancel transition: {non_terminal - cancellable}"
|
|
)
|
|
|
|
|
|
def test_status_graph_lookup_returns_targets() -> None:
|
|
"""STATUS_GRAPH is a quick `source -> {targets}` lookup."""
|
|
assert spec.Status.CLAIMED in spec.STATUS_GRAPH[spec.Status.PENDING]
|
|
assert spec.Status.AWAITING_QA in spec.STATUS_GRAPH[spec.Status.VERIFYING]
|
|
assert spec.STATUS_GRAPH[spec.Status.COMPLETED] == frozenset()
|
|
|
|
|
|
def test_status_transitions_role_constraints_match_canon() -> None:
|
|
"""role_constraint must encode the per-row role gates from
|
|
PERMISSIONS.md / STATUS_TRANSITIONS.md exactly. Tests that look only
|
|
at (source, target) pairs miss role-typo regressions; this test
|
|
pins the gates explicitly.
|
|
"""
|
|
by_pair = {
|
|
(t.source, t.target, t.triggered_by_action): t.role_constraint
|
|
for t in spec._STATUS_TRANSITIONS
|
|
}
|
|
# QA is the only role that can claim awaiting_qa
|
|
assert by_pair[
|
|
(spec.Status.AWAITING_QA, spec.Status.CLAIMED, "claim")
|
|
] == frozenset({spec.Role.QA})
|
|
# Documenter is the only role that can claim awaiting_documentation
|
|
assert by_pair[
|
|
(spec.Status.AWAITING_DOCUMENTATION, spec.Status.CLAIMED, "claim")
|
|
] == frozenset({spec.Role.DOCUMENTER})
|
|
# qa_pass / qa_fail: QA only
|
|
assert by_pair[
|
|
(spec.Status.AWAITING_QA, spec.Status.AWAITING_DOCUMENTATION, "qa_pass")
|
|
] == frozenset({spec.Role.QA})
|
|
assert by_pair[
|
|
(spec.Status.AWAITING_QA, spec.Status.NEEDS_REVISION, "qa_fail")
|
|
] == frozenset({spec.Role.QA})
|
|
# docs_complete: documenter only
|
|
assert by_pair[
|
|
(
|
|
spec.Status.AWAITING_DOCUMENTATION,
|
|
spec.Status.AWAITING_PM_REVIEW,
|
|
"docs_complete",
|
|
)
|
|
] == frozenset({spec.Role.DOCUMENTER})
|
|
# PM complete: cell + main PM (not board, not CEO)
|
|
assert by_pair[
|
|
(spec.Status.AWAITING_PM_REVIEW, spec.Status.COMPLETED, "complete")
|
|
] == frozenset({spec.Role.CELL_PM, spec.Role.MAIN_PM})
|
|
# escalate_to_ceo: main_pm + product_owner + head_marketing — from a
|
|
# completed review and from a blocked task, same role gate.
|
|
escalate_roles = frozenset(
|
|
{
|
|
spec.Role.MAIN_PM,
|
|
spec.Role.PRODUCT_OWNER,
|
|
spec.Role.HEAD_MARKETING,
|
|
}
|
|
)
|
|
assert (
|
|
by_pair[
|
|
(
|
|
spec.Status.AWAITING_PM_REVIEW,
|
|
spec.Status.AWAITING_CEO_APPROVAL,
|
|
"escalate_to_ceo",
|
|
)
|
|
]
|
|
== escalate_roles
|
|
)
|
|
assert (
|
|
by_pair[
|
|
(
|
|
spec.Status.BLOCKED,
|
|
spec.Status.AWAITING_CEO_APPROVAL,
|
|
"escalate_to_ceo",
|
|
)
|
|
]
|
|
== escalate_roles
|
|
)
|
|
# CEO actions: CEO only
|
|
assert by_pair[
|
|
(spec.Status.AWAITING_CEO_APPROVAL, spec.Status.COMPLETED, "ceo_approve")
|
|
] == frozenset({spec.Role.CEO})
|
|
assert by_pair[
|
|
(spec.Status.AWAITING_CEO_APPROVAL, spec.Status.NEEDS_REVISION, "ceo_reject")
|
|
] == frozenset({spec.Role.CEO})
|
|
# Cancel: PM + CEO from any non-terminal status EXCEPT the CEO approval
|
|
# queue — cancelling a task the CEO is reviewing is the CEO's call, so
|
|
# awaiting_ceo_approval -> cancelled is gated to CEO only (a PM cancelling
|
|
# it would bypass the human CEO gate).
|
|
cancel_constraint = frozenset({spec.Role.CELL_PM, spec.Role.MAIN_PM, spec.Role.CEO})
|
|
for src in spec.Status:
|
|
if src in (spec.Status.COMPLETED, spec.Status.CANCELLED):
|
|
continue
|
|
expected = (
|
|
frozenset({spec.Role.CEO})
|
|
if src is spec.Status.AWAITING_CEO_APPROVAL
|
|
else cancel_constraint
|
|
)
|
|
assert by_pair[(src, spec.Status.CANCELLED, "cancel")] == expected, (
|
|
f"cancel from {src.value} has wrong role_constraint"
|
|
)
|
|
|
|
|
|
def test_atomic_action_table_has_pre_gateway_actions() -> None:
|
|
"""Every task tool from PERMISSIONS.md must have an ActionSpec."""
|
|
expected = {
|
|
"activate",
|
|
"claim",
|
|
"start",
|
|
"set_plan",
|
|
"block",
|
|
"unblock",
|
|
"pause",
|
|
"resume",
|
|
"submit_verification",
|
|
"submit_qa",
|
|
"qa_pass",
|
|
"qa_fail",
|
|
"docs_complete",
|
|
"complete",
|
|
"submit_pm_review",
|
|
"escalate_to_ceo",
|
|
"ceo_approve",
|
|
"ceo_reject",
|
|
"cancel",
|
|
"create_subtask",
|
|
}
|
|
assert expected <= set(spec._ATOMIC_ACTIONS), (
|
|
f"Missing ActionSpec entries: {expected - set(spec._ATOMIC_ACTIONS)}"
|
|
)
|
|
|
|
|
|
def test_claim_action_allows_developer_from_pending() -> None:
|
|
a = spec._ATOMIC_ACTIONS["claim"]
|
|
assert spec.Role.DEVELOPER in a.allowed_roles
|
|
assert spec.Status.PENDING in a.source_statuses
|
|
assert a.target_status == spec.Status.CLAIMED
|
|
|
|
|
|
def test_qa_pass_self_review_blocks() -> None:
|
|
"""A QA cannot qa_pass a task they themselves committed to."""
|
|
assert spec._ATOMIC_ACTIONS["qa_pass"].self_review_block is True
|
|
assert spec._ATOMIC_ACTIONS["qa_fail"].self_review_block is True
|
|
assert spec._ATOMIC_ACTIONS["docs_complete"].self_review_block is True
|
|
|
|
|
|
def test_claim_rules_match_pre_gateway_table() -> None:
|
|
"""PERMISSIONS.md "What Each Role Can Claim From" — exact match.
|
|
|
|
PMs claim from PENDING and NEEDS_REVISION — the latter to recover a rejected
|
|
coordination task (pr_fail / qa_fail / ceo_reject) by re-planning and
|
|
re-delegating fixes (scoped by give_me_work routing, which offers only the
|
|
caller's own assigned tasks). BACKLOG → PENDING is a separate `activate`
|
|
action (strict transitions; no implicit activate-on-claim).
|
|
"""
|
|
assert spec.CLAIM_RULES[spec.Role.DEVELOPER] == frozenset(
|
|
{spec.Status.PENDING, spec.Status.NEEDS_REVISION}
|
|
)
|
|
assert spec.CLAIM_RULES[spec.Role.QA] == frozenset({spec.Status.AWAITING_QA})
|
|
assert spec.CLAIM_RULES[spec.Role.DOCUMENTER] == frozenset(
|
|
{spec.Status.PENDING, spec.Status.AWAITING_DOCUMENTATION}
|
|
)
|
|
assert spec.CLAIM_RULES[spec.Role.CELL_PM] == frozenset(
|
|
{spec.Status.PENDING, spec.Status.NEEDS_REVISION}
|
|
)
|
|
assert spec.CLAIM_RULES[spec.Role.MAIN_PM] == frozenset(
|
|
{spec.Status.PENDING, spec.Status.NEEDS_REVISION}
|
|
)
|
|
|
|
|
|
def test_team_rules_pin_team_for_seeded_agents() -> None:
|
|
assert spec.ROLE_TEAM_RULES["be-dev-1"] == "backend"
|
|
assert spec.ROLE_TEAM_RULES["be-pm"] == "backend"
|
|
assert spec.ROLE_TEAM_RULES["fe-qa"] == "frontend"
|
|
assert spec.ROLE_TEAM_RULES["main-pm"] is None # cross-cell
|
|
|
|
|
|
def test_intent_verbs_table_has_every_gateway_verb() -> None:
|
|
"""Every gateway intent verb must have an IntentSpec."""
|
|
expected = {
|
|
"give_me_work",
|
|
"i_will_work_on",
|
|
"i_will_plan",
|
|
"delegate",
|
|
"open_pr",
|
|
"i_am_done",
|
|
"i_am_blocked",
|
|
"unclaim",
|
|
"resume",
|
|
"i_am_idle",
|
|
"claim_review",
|
|
"pass_review",
|
|
"fail_review",
|
|
"claim_doc_task",
|
|
"i_documented",
|
|
"complete",
|
|
"escalate_up",
|
|
"escalate_to_ceo",
|
|
"submit_up",
|
|
"unblock",
|
|
"triage",
|
|
"triage_all",
|
|
}
|
|
assert expected <= set(spec._INTENT_VERBS), (
|
|
f"Missing IntentSpec entries: {expected - set(spec._INTENT_VERBS)}"
|
|
)
|
|
|
|
|
|
def test_i_will_work_on_composes_claim_set_plan_start() -> None:
|
|
iv = spec._INTENT_VERBS["i_will_work_on"]
|
|
assert iv.composes == ("claim", "set_plan", "start")
|
|
assert spec.Role.DEVELOPER in iv.allowed_roles
|
|
|
|
|
|
def test_i_will_plan_composes_claim_set_plan_start() -> None:
|
|
"""PMs use i_will_plan; the composition mirrors i_will_work_on."""
|
|
iv = spec._INTENT_VERBS["i_will_plan"]
|
|
assert iv.composes == ("claim", "set_plan", "start")
|
|
assert iv.allowed_roles == frozenset({spec.Role.CELL_PM, spec.Role.MAIN_PM})
|
|
|
|
|
|
def test_i_am_done_composes_submit_verification_then_submit_qa() -> None:
|
|
iv = spec._INTENT_VERBS["i_am_done"]
|
|
assert iv.composes == ("submit_verification", "submit_qa")
|
|
|
|
|
|
def test_open_pr_has_git_side_effects() -> None:
|
|
"""open_pr is a side-effect-only verb (no DB transition)."""
|
|
iv = spec._INTENT_VERBS["open_pr"]
|
|
assert "push_branch" in iv.side_effects
|
|
assert "create_pr" in iv.side_effects
|
|
assert iv.composes == () # pure side effect verb
|
|
|
|
|
|
def test_delegate_composes_create_subtask() -> None:
|
|
iv = spec._INTENT_VERBS["delegate"]
|
|
assert iv.composes == ("create_subtask",)
|
|
assert iv.allowed_roles == frozenset({spec.Role.CELL_PM, spec.Role.MAIN_PM})
|
|
|
|
|
|
_STUB_TASK_DEFAULTS: dict[str, Any] = {
|
|
"status": "pending",
|
|
"task_type": "code",
|
|
"commits": [],
|
|
"plan": None,
|
|
"assigned_to": None,
|
|
"pr_number": None,
|
|
}
|
|
|
|
|
|
def _stub_task(**overrides: Any) -> SimpleNamespace:
|
|
fields = {**_STUB_TASK_DEFAULTS, **overrides}
|
|
fields["commits"] = fields["commits"] or []
|
|
return SimpleNamespace(**fields)
|
|
|
|
|
|
def test_can_claim_developer_pending_allowed() -> None:
|
|
d = spec.can_claim(spec.Role.DEVELOPER, _stub_task(status="pending"))
|
|
assert d.allowed is True
|
|
|
|
|
|
def test_can_claim_developer_completed_rejected() -> None:
|
|
d = spec.can_claim(spec.Role.DEVELOPER, _stub_task(status="completed"))
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "invalid_state"
|
|
|
|
|
|
def test_can_claim_developer_awaiting_qa_rejected() -> None:
|
|
"""Devs cannot claim awaiting_qa - that's QA's path."""
|
|
d = spec.can_claim(spec.Role.DEVELOPER, _stub_task(status="awaiting_qa"))
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "not_authorized"
|
|
|
|
|
|
def test_can_invoke_intent_developer_can_call_i_will_work_on() -> None:
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.DEVELOPER,
|
|
"i_will_work_on",
|
|
_stub_task(status="pending"),
|
|
context=spec.Context(plan="my plan"),
|
|
)
|
|
assert d.allowed is True
|
|
|
|
|
|
def test_can_invoke_intent_pm_cannot_call_i_will_work_on() -> None:
|
|
"""PMs use i_will_plan; i_will_work_on is dev-only."""
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.CELL_PM,
|
|
"i_will_work_on",
|
|
_stub_task(status="pending"),
|
|
context=spec.Context(plan="x"),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "not_authorized"
|
|
|
|
|
|
def test_can_invoke_intent_developer_open_pr_no_commits_tracing_gap() -> None:
|
|
"""open_pr requires >=1 commit. Without one -> tracing_gap."""
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.DEVELOPER,
|
|
"open_pr",
|
|
_stub_task(status="in_progress", commits=[]),
|
|
context=spec.Context(),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "tracing_gap"
|
|
assert "commits>=1" in d.missing
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# open_pr must enforce the PR-open state gate (parity with the HTTP path)
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def _owned_task(**overrides: Any) -> SimpleNamespace:
|
|
"""A task owned by ``actor`` with commits and no prior PR — only the state
|
|
gate can fail, isolating the PR-open-state precondition."""
|
|
actor = overrides.pop("actor_id", uuid4())
|
|
return _stub_task(
|
|
assigned_to=actor,
|
|
commits=["abc123"],
|
|
pr_number=None,
|
|
**overrides,
|
|
)
|
|
|
|
|
|
def test_open_pr_rejected_on_claimed_task() -> None:
|
|
"""``open_pr`` must be rejected from ``claimed`` — only ``in_progress`` may
|
|
open a PR (mirrors the HTTP path's ``_assert_pr_create_allowed``)."""
|
|
actor = uuid4()
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.DEVELOPER,
|
|
"open_pr",
|
|
_owned_task(status="claimed", actor_id=actor),
|
|
context=spec.Context(actor_id=actor),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "invalid_state"
|
|
|
|
|
|
def test_open_pr_rejected_on_paused_task() -> None:
|
|
actor = uuid4()
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.DEVELOPER,
|
|
"open_pr",
|
|
_owned_task(status="paused", actor_id=actor),
|
|
context=spec.Context(actor_id=actor),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "invalid_state"
|
|
|
|
|
|
def test_open_pr_rejected_on_blocked_task() -> None:
|
|
actor = uuid4()
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.DEVELOPER,
|
|
"open_pr",
|
|
_owned_task(status="blocked", actor_id=actor),
|
|
context=spec.Context(actor_id=actor),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "invalid_state"
|
|
|
|
|
|
def test_open_pr_rejected_on_completed_task() -> None:
|
|
"""A completed task is terminal — opening a PR on it is nonsensical."""
|
|
actor = uuid4()
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.DEVELOPER,
|
|
"open_pr",
|
|
_owned_task(status="completed", actor_id=actor),
|
|
context=spec.Context(actor_id=actor),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "invalid_state"
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"status",
|
|
[
|
|
"in_progress",
|
|
"verifying",
|
|
"awaiting_qa",
|
|
"awaiting_documentation",
|
|
"needs_revision",
|
|
],
|
|
)
|
|
def test_open_pr_allowed_in_pr_open_states(status: str) -> None:
|
|
"""Regression guard: every PR-open-eligible state still lets the owner open
|
|
a PR — the new state gate must not over-restrict the legitimate path."""
|
|
actor = uuid4()
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.DEVELOPER,
|
|
"open_pr",
|
|
_owned_task(status=status, actor_id=actor),
|
|
context=spec.Context(actor_id=actor),
|
|
)
|
|
assert d.allowed is True, f"open_pr should be allowed from {status}"
|
|
|
|
|
|
def test_open_pr_state_gate_takes_priority_over_unowned() -> None:
|
|
"""A non-owner in a wrong state: ownership (not_authorized) is checked
|
|
before state, mirroring the HTTP path's assignee-first ordering."""
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.DEVELOPER,
|
|
"open_pr",
|
|
_owned_task(status="claimed", actor_id=uuid4()),
|
|
context=spec.Context(actor_id=uuid4()), # different actor -> not owner
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "not_authorized"
|
|
|
|
|
|
def test_escalate_up_rejected_on_completed_task() -> None:
|
|
"""A PM must not resurrect a COMPLETED task via ``escalate_up`` — the spec
|
|
gate rejects terminal tasks before the journal:decision write fires."""
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.CELL_PM,
|
|
"escalate_up",
|
|
_stub_task(status="completed"),
|
|
context=spec.Context(notes="stuck on something"),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "invalid_state"
|
|
|
|
|
|
def test_escalate_up_rejected_on_cancelled_task() -> None:
|
|
"""Cancelled is terminal — escalate_up must not resurrect it either."""
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.MAIN_PM,
|
|
"escalate_up",
|
|
_stub_task(status="cancelled"),
|
|
context=spec.Context(notes="stuck on something"),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "invalid_state"
|
|
|
|
|
|
def test_escalate_up_allowed_on_blocked_task() -> None:
|
|
"""The terminal guard must not over-restrict — BLOCKED is the natural
|
|
escalation source and must still be allowed."""
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.CELL_PM,
|
|
"escalate_up",
|
|
_stub_task(status="blocked"),
|
|
context=spec.Context(notes="stuck on something"),
|
|
)
|
|
assert d.allowed is True
|
|
|
|
|
|
def test_valid_next_verbs_developer_in_progress_includes_open_pr_and_i_am_done() -> (
|
|
None
|
|
):
|
|
verbs = spec.valid_next_verbs(spec.Role.DEVELOPER, _stub_task(status="in_progress"))
|
|
assert "open_pr" in verbs
|
|
assert "i_am_done" in verbs
|
|
assert "i_am_blocked" in verbs
|
|
|
|
|
|
def test_valid_next_verbs_pm_pending_includes_i_will_plan() -> None:
|
|
# A PM i_will_plan's a PLANNING task (coordination), not a code task — the
|
|
# PM/code claim carve-out (Fix 2) removes i_will_plan from a pending code
|
|
# task's verb set, so the legitimate path is exercised with task_type=planning.
|
|
verbs = spec.valid_next_verbs(
|
|
spec.Role.CELL_PM, _stub_task(status="pending", task_type="planning")
|
|
)
|
|
assert "i_will_plan" in verbs
|
|
|
|
|
|
def test_composed_actions_for_returns_intent_composition() -> None:
|
|
assert spec.composed_actions_for("i_will_work_on") == ("claim", "set_plan", "start")
|
|
assert spec.composed_actions_for("open_pr") == ()
|
|
|
|
|
|
def test_intents_for_role_returns_role_scoped_verbs() -> None:
|
|
dev_verbs = spec.intents_for_role(spec.Role.DEVELOPER)
|
|
assert "i_will_work_on" in dev_verbs
|
|
assert "open_pr" in dev_verbs
|
|
assert "i_am_done" in dev_verbs
|
|
assert "delegate" not in dev_verbs # PM only
|
|
assert "claim_review" not in dev_verbs # QA only
|
|
|
|
|
|
def test_status_after_returns_target_status() -> None:
|
|
assert spec.status_after("claim", spec.Status.PENDING) == spec.Status.CLAIMED
|
|
assert (
|
|
spec.status_after("submit_qa", spec.Status.VERIFYING) == spec.Status.AWAITING_QA
|
|
)
|
|
assert (
|
|
spec.status_after("set_plan", spec.Status.IN_PROGRESS) is None
|
|
) # no transition
|
|
|
|
|
|
def test_can_invoke_intent_open_pr_passes_when_owner_with_commits() -> None:
|
|
"""Green path for open_pr: owner + commits + no prior PR → allow."""
|
|
owner_id = uuid4()
|
|
task = _stub_task(
|
|
status="in_progress",
|
|
commits=["abc"],
|
|
pr_number=None,
|
|
assigned_to=owner_id,
|
|
)
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.DEVELOPER,
|
|
"open_pr",
|
|
task,
|
|
context=spec.Context(actor_id=owner_id),
|
|
)
|
|
assert d.allowed is True, f"expected allow, got {d}"
|
|
|
|
|
|
def test_can_invoke_intent_open_pr_rejects_non_owner() -> None:
|
|
"""Non-owner trying open_pr → not_authorized (PRECONDITION_OWNERSHIP)."""
|
|
owner_id = uuid4()
|
|
intruder_id = uuid4()
|
|
task = _stub_task(
|
|
status="in_progress",
|
|
commits=["abc"],
|
|
pr_number=None,
|
|
assigned_to=owner_id,
|
|
)
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.DEVELOPER,
|
|
"open_pr",
|
|
task,
|
|
context=spec.Context(actor_id=intruder_id),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "not_authorized"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Task 8 — self-consistency validators (`_validate.py`)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_validators_pass_on_real_spec() -> None:
|
|
"""Importing roboco.foundation.policy.lifecycle must not raise —
|
|
module-level import IS the test. We additionally call the runner
|
|
directly so a future refactor that detaches it from import doesn't
|
|
silently skip the gate.
|
|
"""
|
|
_validate.run_all_lifecycle_validators()
|
|
|
|
|
|
def test_every_status_reachable_from_pending() -> None:
|
|
"""Reachability — except CANCELLED is its own thing and BACKLOG predates pending."""
|
|
reachable = reachable_from(spec.Status.PENDING)
|
|
expected_reachable = set(spec.Status) - {spec.Status.BACKLOG, spec.Status.CANCELLED}
|
|
assert expected_reachable <= reachable, (
|
|
f"Unreachable from pending: {expected_reachable - reachable}"
|
|
)
|
|
|
|
|
|
def test_every_intent_verb_composes_known_actions() -> None:
|
|
"""Every IntentSpec.composes must reference declared atomic actions."""
|
|
for name, iv in spec._INTENT_VERBS.items():
|
|
for action_name in iv.composes:
|
|
assert action_name in spec._ATOMIC_ACTIONS, (
|
|
f"Intent '{name}' composes unknown action '{action_name}'"
|
|
)
|
|
|
|
|
|
def test_self_review_symmetry() -> None:
|
|
"""If qa_pass blocks, qa_fail and docs_complete must too."""
|
|
qp = spec._ATOMIC_ACTIONS["qa_pass"].self_review_block
|
|
qf = spec._ATOMIC_ACTIONS["qa_fail"].self_review_block
|
|
dc = spec._ATOMIC_ACTIONS["docs_complete"].self_review_block
|
|
assert qp == qf == dc, (
|
|
"self_review_block asymmetry between qa_pass/qa_fail/docs_complete"
|
|
)
|
|
|
|
|
|
def test_run_all_validators_raises_on_unknown_intent_action(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
"""If an IntentSpec.composes references a non-existent action, the
|
|
validator must raise LifecycleSpecError. Pins the gate's actual
|
|
behavior — without this test, refactors that move run_all_validators()
|
|
out of the import path could silently disable the gate.
|
|
"""
|
|
iv = _INTENT_VERBS["delegate"]
|
|
broken = IntentSpec(
|
|
name=iv.name,
|
|
allowed_roles=iv.allowed_roles,
|
|
description=iv.description,
|
|
composes=("create_subtask", "ZZZ_FAKE_ACTION_DOES_NOT_EXIST"),
|
|
extra_preconditions=iv.extra_preconditions,
|
|
side_effects=iv.side_effects,
|
|
next_hint=iv.next_hint,
|
|
)
|
|
patched_intents = dict(_INTENT_VERBS)
|
|
patched_intents["delegate"] = broken
|
|
monkeypatch.setattr(
|
|
"roboco.foundation.policy.lifecycle._INTENT_VERBS", patched_intents
|
|
)
|
|
with pytest.raises(_validate.LifecycleSpecError, match="ZZZ_FAKE_ACTION"):
|
|
_validate.run_all_lifecycle_validators()
|
|
|
|
|
|
def test_next_hint_pr_fail_main_pm_root_steers_to_redelegate() -> None:
|
|
"""A ``pr_fail`` on a Main-PM branch-bearing root must steer the Main PM to
|
|
re-delegate the fixes, NOT re-submit the unchanged root. The root is an
|
|
assembled cell→root / root→master PR — coordination, not the Main PM's own
|
|
code — so re-submitting it is the 2026-06-27 infinite ``pr_fail`` loop."""
|
|
t = SimpleNamespace(team=spec.Team.MAIN_PM, branch_name="feature/main_pm/c80e19ff")
|
|
hint = _INTENT_VERBS["pr_fail"].next_hint(t)
|
|
assert "re-delegate" in hint
|
|
assert "do NOT re-submit" in hint
|
|
|
|
|
|
def test_next_hint_pr_fail_cell_dev_keeps_dev_revise() -> None:
|
|
"""A cell / dev task is revised in place by its dev, so ``pr_fail`` keeps the
|
|
dev-revise hint (the cell→root PR carries that dev's own code)."""
|
|
t = SimpleNamespace(team=spec.Team.BACKEND, branch_name="feature/backend/abc12345")
|
|
hint = _INTENT_VERBS["pr_fail"].next_hint(t)
|
|
assert hint == "idle - dev will revise and re-submit"
|
|
|
|
|
|
def test_next_hint_pr_fail_branchless_main_pm_keeps_dev_revise() -> None:
|
|
"""A branchless Main-PM umbrella (no ``branch_name``) assembles no PR of its
|
|
own, so the gate never lands a ``pr_fail`` on it — but defensively it keeps
|
|
the dev-revise hint rather than the re-delegate steer."""
|
|
t = SimpleNamespace(team=spec.Team.MAIN_PM, branch_name=None)
|
|
hint = _INTENT_VERBS["pr_fail"].next_hint(t)
|
|
assert hint == "idle - dev will revise and re-submit"
|
|
|
|
|
|
def test_unmigrated_is_pinned() -> None:
|
|
"""The known-debt set; remove an entry once that consumer is migrated."""
|
|
assert (
|
|
frozenset(
|
|
{
|
|
"enforcement.task_lifecycle._LEGACY_OPERATIONAL_EDGES",
|
|
"enforcement.task_lifecycle._LEGACY_ROLE_GATES",
|
|
}
|
|
)
|
|
== spec.UNMIGRATED
|
|
)
|
|
|
|
|
|
# --- PM/code claim invariant (Fix 2 + bug-1): the claim gate does NOT block a
|
|
# PM claiming a code task. A PM's only claim verb is i_will_plan, and planning a
|
|
# code-typed PARENT (to decompose + delegate the code) is legitimate (bug-1:
|
|
# scoping pm_cannot_execute_code to i_will_plan deadlocked the slice). Execution
|
|
# is blocked at the intent level — i_will_work_on is _DEV_ROLES only. The
|
|
# create/delegate guards (pm_cannot_own_code) block a PM from being ASSIGNED a
|
|
# fresh code task; the needs_revision carve-out (a PM resolving review issues
|
|
# directly / recovering a rejected coordination task) is naturally allowed
|
|
# because PMs claim NEEDS_REVISION. These pin that the claim gate does not
|
|
# regress bug-1.
|
|
|
|
|
|
def _claim_task(*, status: str, task_type: str) -> Any:
|
|
return SimpleNamespace(status=status, task_type=task_type)
|
|
|
|
|
|
def test_claim_allows_cell_pm_claiming_code_from_pending() -> None:
|
|
"""bug-1: a cell PM i_will_plan-ing a code-typed parent (PENDING) to plan +
|
|
delegate the code MUST be allowed — rejecting it deadlocks the slice."""
|
|
t = _claim_task(status="pending", task_type="code")
|
|
d = spec.can_invoke_action(spec.Role.CELL_PM, "claim", t)
|
|
assert d.allowed, d.message
|
|
|
|
|
|
def test_claim_allows_cell_pm_claiming_code_from_needs_revision() -> None:
|
|
"""Carve-out: a PM may take a code task in needs_revision to resolve the
|
|
review/QA issues directly / recover a rejected coordination task."""
|
|
t = _claim_task(status="needs_revision", task_type="code")
|
|
d = spec.can_invoke_action(spec.Role.CELL_PM, "claim", t)
|
|
assert d.allowed, d.message
|
|
|
|
|
|
def test_claim_allows_main_pm_claiming_code_from_needs_revision() -> None:
|
|
"""The same carve-out holds for the Main PM (coordination-recovery path)."""
|
|
t = _claim_task(status="needs_revision", task_type="code")
|
|
d = spec.can_invoke_action(spec.Role.MAIN_PM, "claim", t)
|
|
assert d.allowed, d.message
|
|
|
|
|
|
def test_claim_allows_main_pm_claiming_code_from_pending() -> None:
|
|
"""bug-1 parity: a Main PM planning a code-typed parent (PENDING) is allowed
|
|
for the same reason as the cell PM — execution is blocked at i_will_work_on,
|
|
not at the claim gate."""
|
|
t = _claim_task(status="pending", task_type="code")
|
|
d = spec.can_invoke_action(spec.Role.MAIN_PM, "claim", t)
|
|
assert d.allowed, d.message
|
|
|
|
|
|
def test_claim_allows_pm_claiming_planning_from_pending() -> None:
|
|
"""A PM claiming a planning task is the legitimate coordination path."""
|
|
t = _claim_task(status="pending", task_type="planning")
|
|
assert spec.can_invoke_action(spec.Role.CELL_PM, "claim", t).allowed
|
|
assert spec.can_invoke_action(spec.Role.MAIN_PM, "claim", t).allowed
|
|
|
|
|
|
def test_claim_allows_developer_claiming_code_from_pending() -> None:
|
|
"""A developer claiming fresh code is unaffected (the PM invariant is
|
|
enforced at create/delegate + i_will_work_on, not the claim gate)."""
|
|
t = _claim_task(status="pending", task_type="code")
|
|
assert spec.can_invoke_action(spec.Role.DEVELOPER, "claim", t).allowed
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Edge cases — logical-gap element sweep (2026-06-30)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_claim_pr_review_rejected_on_gate_task_points_to_claim_gate_review() -> None:
|
|
"""claim_pr_review is for an inbound external-PR task in PENDING only. An
|
|
awaiting_pr_review gate task must be rejected (and remediation must point
|
|
the reviewer at claim_gate_review), not silently accepted by the spec gate."""
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.PR_REVIEWER,
|
|
"claim_pr_review",
|
|
_stub_task(status="awaiting_pr_review"),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "invalid_state"
|
|
assert "claim_gate_review" in (d.remediate or "")
|
|
|
|
|
|
def test_claim_pr_review_allowed_on_pending_external_review() -> None:
|
|
"""Green path: a pending external-PR review task is claimable."""
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.PR_REVIEWER,
|
|
"claim_pr_review",
|
|
_stub_task(status="pending"),
|
|
)
|
|
assert d.allowed is True
|
|
|
|
|
|
def test_needs_team_match_rejects_cross_team_claim_when_agent_team_supplied() -> None:
|
|
"""needs_team_match was a dead spec field; when the caller supplies the
|
|
agent's team via Context, the spec gate must enforce it (a backend dev
|
|
cannot claim a frontend task)."""
|
|
d = spec.can_invoke_action(
|
|
spec.Role.DEVELOPER,
|
|
"claim",
|
|
_stub_task(status="pending", team="frontend"),
|
|
context=spec.Context(agent_team="backend"),
|
|
)
|
|
assert d.allowed is False
|
|
assert d.rejection_kind == "not_authorized"
|
|
|
|
|
|
def test_needs_team_match_allows_same_team_claim() -> None:
|
|
d = spec.can_invoke_action(
|
|
spec.Role.DEVELOPER,
|
|
"claim",
|
|
_stub_task(status="pending", team="backend"),
|
|
context=spec.Context(agent_team="backend"),
|
|
)
|
|
assert d.allowed is True
|
|
|
|
|
|
def test_needs_team_match_defers_when_agent_team_absent() -> None:
|
|
"""Backward compat: without agent_team in Context, the spec gate stays
|
|
permissive (the service layer still enforces team-match)."""
|
|
d = spec.can_invoke_action(
|
|
spec.Role.DEVELOPER,
|
|
"claim",
|
|
_stub_task(status="pending", team="frontend"),
|
|
)
|
|
assert d.allowed is True
|
|
|
|
|
|
def test_valid_next_verbs_omits_claim_review_when_qa_not_in_awaiting_qa() -> None:
|
|
"""valid_next_verbs must apply claim-rule narrowing for empty-compose
|
|
claim verbs; a QA reviewer on a COMPLETED task must not be told
|
|
claim_review is callable."""
|
|
verbs = spec.valid_next_verbs(spec.Role.QA, _stub_task(status="completed"))
|
|
assert "claim_review" not in verbs
|
|
|
|
|
|
def test_valid_next_verbs_includes_claim_review_for_qa_in_awaiting_qa() -> None:
|
|
verbs = spec.valid_next_verbs(spec.Role.QA, _stub_task(status="awaiting_qa"))
|
|
assert "claim_review" in verbs
|
|
|
|
|
|
def test_pr_reviewer_has_unclaim_release_verb() -> None:
|
|
"""A PR reviewer who cannot finish a review must have a self-release
|
|
verb (unclaim), not wedge the lane until the stale-claim reaper."""
|
|
assert "unclaim" in spec.intents_for_role(spec.Role.PR_REVIEWER)
|
|
|
|
|
|
def test_unclaim_allowed_for_pr_reviewer() -> None:
|
|
d = spec.can_invoke_intent(
|
|
spec.Role.PR_REVIEWER,
|
|
"unclaim",
|
|
_stub_task(status="awaiting_pr_review"),
|
|
)
|
|
assert d.allowed is True
|
|
|
|
|
|
def test_complete_intent_declares_no_inverted_pr_merge_side_effect() -> None:
|
|
"""complete's IntentSpec must not declare a trailing pr_merge side_effect:
|
|
TaskService.complete asserts the PR is already merged, so the merge runs
|
|
FIRST (choreographer verb body owns the ordering). The spec must match
|
|
reality, not lie about a complete-then-merge composition."""
|
|
iv = spec._INTENT_VERBS["complete"]
|
|
assert iv.composes == ("complete",)
|
|
assert iv.side_effects == ()
|