mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
48d6e99fa46ba2c2372bcc38511ac75fbdb07a8f
343
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
207aaecd72 |
Feature: lifecycle canonical spec (#14)
* chore: clean make quality baseline on feature/lifecycle-canonical-spec
Three classes of pre-existing issues blocking `make quality`:
1. Alembic migrations 002/009/011 used runtime introspection
(op.get_bind() + inspect / bind.execute) without guarding for
offline (--sql) mode. `alembic upgrade head --sql` is part of
`make quality`; in offline mode `op.get_bind()` returns a
MockConnection with no inspection system, so the migrations
crashed before emitting their SQL stubs. Each migration now
short-circuits or simplifies in `context.is_offline_mode()` —
live-DB behavior is unchanged.
2. ruff format drift on three files left over from prior in-flight
edits (choreographer/_impl.py, content_actions.py, and one test
file). `ruff format` applied.
3. vulture flagged two unused `tb` parameters in async __aexit__
stubs in test_task_service_lifecycle_misc.py. The parameter is
protocol-required but unused by the body — renamed to `_tb`
(vulture treats underscore-prefixed names as intentionally unused).
`make quality` is now green from this branch's HEAD; subsequent
lifecycle-spec work can use it as the per-task gate.
* feat(lifecycle): canonical spec package + Role/Status/TaskType enums
Foundation for the canonical lifecycle/permissions module. Enums
mirror docs/internal/old/workflows/STATUS_TRANSITIONS.md +
PERMISSIONS.md. Tests pin enum membership against both the
predecessor canon and roboco.models.base.TaskType.
* feat(lifecycle): Decision dataclass with allow/reject/tracing_gap constructors
Single rejection shape every consumer maps to its native format
(Envelope, HTTP code, prompt hint). __post_init__ enforces the
allowed/rejection_kind invariants so a malformed Decision can't reach
a consumer.
* fix(lifecycle): tighten Decision invariants per Task 2 review
Two reviewer findings on the Task 2 Decision dataclass, addressed
in one commit:
1. The docstring promised `allowed=True ⇒ rejection_kind is None
AND missing == [] AND remediate is None`, but __post_init__ only
checked the rejection_kind half. A caller could construct an
allow-shaped Decision with stale missing/remediate fields and
sneak it past validation. Tighten __post_init__ to enforce the
full invariant. Add a regression test.
2. tracing_gap defensively copies the missing list (`list(missing)`)
to isolate the stored list from later caller-side mutation, but
no test pinned this. Add a regression test that mutates the source
list after construction and asserts the stored list is unchanged.
Issue 2 from the same review (mutable list vs tuple for `missing`)
is a broader design call deferred until consumers exist; the
defensive copy is sufficient until then.
* feat(lifecycle): Precondition/ActionSpec/IntentSpec/StatusTransition dataclasses
The four dataclasses that hold the canonical tables. ActionSpec and
StatusTransition are direct ports of pre-gateway PERMISSIONS.md +
STATUS_TRANSITIONS.md rows. IntentSpec is the gateway-only addition:
each gateway intent verb declares which atomic actions it composes.
* feat(lifecycle): _STATUS_TRANSITIONS table + STATUS_GRAPH view
Direct port of STATUS_TRANSITIONS.md. Every transition records its
trigger action and (optionally) a role constraint. STATUS_GRAPH is
the precomputed source→{targets} view callers use for reachability
checks.
* fix(lifecycle): pin role_constraint values + clarify Task-5 handoff
Two reviewer findings on Task 4 _STATUS_TRANSITIONS, addressed in
one commit:
1. The original Task-4 tests verified (source, target) pairs but
not role_constraint contents. A typo in a single role name (e.g.
forgetting MAIN_PM from escalate_to_ceo) would have slipped past
them silently. Add test_status_transitions_role_constraints_match_canon
pinning every non-None constraint and the cancel-block invariant.
2. role_constraint=None on the `claim` rows from PENDING and
NEEDS_REVISION was load-bearing — it is the explicit handoff
point between the StatusTransition table (state machine layer)
and CLAIM_RULES (per-role claim authority, lands in Task 5).
The original inline comment said this in passing; expand it so
the design choice is unmissable for a stranger reading just
spec.py.
* feat(lifecycle): _ATOMIC_ACTIONS + CLAIM_RULES + ROLE_TEAM_RULES tables
Direct port of PERMISSIONS.md. Every task management tool gets an
ActionSpec with allowed_roles, source_statuses, target_status,
self_review_block, and needs_team_match flags. CLAIM_RULES maps each
Role to the statuses they can claim from. ROLE_TEAM_RULES is the
per-slug team restriction.
* fix(lifecycle): tighten ActionSpec contracts per Task 5 review
Three reviewer findings on Task 5's _ATOMIC_ACTIONS table, addressed
in one commit:
1. set_plan.source_statuses widened to {CLAIMED, IN_PROGRESS} but
every existing caller (i_will_work_on / i_will_plan compositions)
runs set_plan while CLAIMED, between claim and start. Narrow to
{CLAIMED} only. If a future "edit plan mid-flight" feature lands,
widen explicitly with test coverage at that time.
2. needs_team_match was set True only on claim/qa_pass/qa_fail/
docs_complete. Defense-in-depth says every role-scoped task
action should re-assert team match (don't rely on the inheritance
chain through assigned_to alone). Flip to True on: start,
set_plan, block, pause, submit_verification, submit_qa,
submit_pm_review, complete, create_subtask. Leave False on
board/CEO actions and PM cross-cell interventions (unblock,
resume, cancel) where the cross-cell semantics are intentional.
3. claim.source_statuses is intentionally a SUPERSET of any single
role's CLAIM_RULES allowance (the table holds the union; CLAIM_RULES
holds the per-role authority). Add an inline comment above the
claim ActionSpec so a future reader doesn't conclude the two
tables disagree — they don't, they encode overlapping facts at
different grains.
* feat(lifecycle): _INTENT_VERBS table — every gateway verb declared
Each gateway intent verb is now a named composition of atomic actions
plus optional side effects. i_will_work_on = (claim, set_plan, start);
i_am_done = (submit_verification, submit_qa); open_pr is pure side
effects (push_branch, create_pr); etc.
* fix(lifecycle): widen block.allowed_roles to include QA + Documenter
Task 6 review caught a role-set inconsistency: i_am_blocked.allowed_roles
admits dev/QA/doc, but the underlying block.allowed_roles only allowed
dev+PM. Result: a QA or documenter calling i_am_blocked would pass the
IntentSpec gate and then be rejected by the composed ActionSpec gate
when Task 7 wires can_invoke_intent.
Widen block to include QA + Documenter. The semantic case is sound: a
QA reviewing a task can discover an external blocker; a documenter
writing docs may need PM intervention. Predecessor PERMISSIONS.md
restricted block to dev+PM, but with the gateway exposing i_am_blocked
to all worker roles, the underlying atomic must agree.
The deeper unclaim/escalate_up "imperative verb" concern from the same
review (composes=() but mutates state) is deferred to Task 8 where the
validator design lands.
* feat(lifecycle): public lookup functions + Context + preconditions
can_claim, can_invoke_action, can_invoke_intent, valid_next_verbs,
composed_actions_for, intents_for_role, status_after — the entire
public surface every consumer will use. Context carries the
caller-supplied state preconditions need (plan, journal-decision
flag, etc.). Preconditions for plan/commits/no_pr/ownership are
declared once and wired into the relevant IntentSpecs.
* fix(lifecycle): wire PRECONDITION_OWNERSHIP through Context.actor_id
Task 7 review found _p_owns_task reads agent.id but every call site
passes None for the agent arg. Result: getattr(None, "id", object())
returns a fresh sentinel, task.assigned_to == <sentinel> is always
False, and open_pr / i_am_done would reject every owner the moment
Task 9 wires consumers.
Fix: thread identity through Context.actor_id (new UUID field) and
rewrite _p_owns_task to read from the context. Both call sites already
pass the Context — no signature changes elsewhere. Add green-path
test exercising the owner-can-open-pr case the existing tests
missed (the Task 7 plan only tested precondition-failure paths,
which masked the bug).
Plus surface hygiene: STATUS_GRAPH, CLAIM_RULES, ROLE_TEAM_RULES,
and the four PRECONDITION_* constants are now in
roboco.lifecycle.__init__.__all__ so consumers in Tasks 8/9 don't
depend on the implicit `from roboco.lifecycle.spec import ...`
backdoor.
* feat(lifecycle): import-time self-consistency validators
10 validators run at module import; first failure raises
LifecycleSpecError and prevents the package from loading. Covers
status enum coverage, reachability, terminal exits, intent
compositions, status chain consistency, claim-rule role/status
coverage, self-review symmetry, team-rule slug existence, and
StatusTransition action references.
* fix(lifecycle): close validator gaps; resolve BACKLOG-claim and submit_qa IN_PROGRESS-shortcut ambiguity
Three reviewer follow-ups on Task 8's _validate.py, plus two real
data corrections the new action-target-reachability validator
surfaced.
1. Design spec §9 calls for "every ActionSpec.target_status, when
set, is reachable from each source_status via STATUS_GRAPH" —
missing from Task 8's 10 validators. Add
_check_action_target_reachable_from_source.
2. _check_role_team_rules_slugs verified slug existence in
AGENT_UUIDS but NOT that the cell team in ROLE_TEAM_RULES
matches the seed. Add _check_role_team_rules_team_match,
scoped to non-None entries only — None means "exempt from
team-match enforcement" (cross-cell roles), not "no team in
org chart".
3. test_validators_pass_on_real_spec was ceremonial. Add
test_run_all_validators_raises_on_unknown_intent_action,
a deliberate-break regression that monkeypatches _INTENT_VERBS
to inject a fake action and asserts LifecycleSpecError raises.
The new action-target-reachability validator caught two real
data inconsistencies between the predecessor canon docs and the
spec tables:
A. claim.source_statuses listed BACKLOG and CLAIM_RULES[*PM]
listed BACKLOG, but STATUS_GRAPH[BACKLOG] = {PENDING, CANCELLED}
only. Resolution: PMs use the explicit \`activate\` action to
move BACKLOG → PENDING, then claim from PENDING. Drop BACKLOG
from claim.source_statuses and CLAIM_RULES.
B. submit_qa.source_statuses listed IN_PROGRESS, but
STATUS_GRAPH[IN_PROGRESS] does NOT include AWAITING_QA. The
intent verb i_am_done composes (submit_verification, submit_qa)
which forces IN_PROGRESS → VERIFYING → AWAITING_QA — no
shortcut. Drop the stale IN_PROGRESS entry from
submit_qa.source_statuses.
Both corrections tighten the canonical state machine to a strict
no-skip transition graph. Pre-gateway PERMISSIONS.md/STATUS_TRANSITIONS.md
disagreements are resolved here; spec.py is the canon now.
* feat(gateway): Envelope.from_decision maps lifecycle Decisions to envelopes
Single shape adapter so verb bodies stop hand-composing rejection
envelopes. Each rejection_kind maps to a specific envelope flavor;
'self_review' folds into 'not_authorized' with a parenthetical hint;
constructing from an allow Decision raises (programmer error).
* feat(gateway): VerbRunner for atomic composed-action dispatch
Wraps spec.composed_actions_for(intent) in session.begin_nested()
so mid-sequence failures roll the DB back. Side effects run AFTER
the savepoint commits. Each atomic action name dispatches to a
TaskService method via a single, exhaustive _dispatch_atomic
mapping. New verbs slot in by adding an IntentSpec entry + a
_dispatch_atomic case if a new atomic is needed.
* refactor(gateway): i_will_work_on uses spec.can_invoke_intent + VerbRunner
Replace the bespoke status-branch dispatcher in i_will_work_on with the
spec-driven flow: load task -> load agent -> build spec.Context ->
spec.can_invoke_intent (and spec.can_claim for per-role status authority)
-> Envelope.from_decision on rejection -> VerbRunner.run_intent on success.
The _i_will_work_on_pending, _i_will_work_on_claimed,
_i_will_work_on_needs_revision, and _start_failed_envelope helpers are
removed; the runner replaces them. Two narrow verb-body re-entry blocks
remain for behaviors the spec does not yet model:
1. in_progress + same agent -> idempotent heartbeat-only return
2. claimed + same agent -> _resume_from_claimed (set_plan + start)
to recover from a stuck mid-claim crash without re-running claim
against a state the spec excludes.
The behavioral claim guards (already_active / paused / sibling_sequence)
also stay imperative for now -- they're not in the spec yet and migrate
into spec.extra_preconditions in a later task. Per-role claim authority
is enforced via spec.can_claim because the atomic claim action's
source_statuses are the union across roles; CLAIM_RULES narrows.
Parity test in tests/lifecycle/test_consumer_parity.py runs the verb
against every (role x status x task_type='code') combo (112 rows) and
asserts the envelope error matches the spec's Decision (or can_claim's
Decision when the intent gate passes but per-role claim authority does
not). This is the contract that makes spec/verb drift impossible.
Existing tests updated where rejection-message text changed (the spec
now produces the messages, e.g. "role 'cell_pm' may not call
'i_will_work_on'" instead of "PM cannot execute code") or where the
spec's stricter view ("invalid_state" -> "not_authorized" for a dev
trying to claim awaiting_qa) is more accurate. Test fixtures were
updated to wire task.session.begin_nested as a proper async context
manager (required by VerbRunner) and to set agent_for().id so runner-
driven calls line up with assert_awaited_with(task_id, agent_id).
* refactor(lifecycle): push CLAIM_RULES enforcement into can_invoke_action
Task 11's i_will_work_on migration had to call spec.can_claim()
separately after spec.can_invoke_intent() because the claim action's
source_statuses is the union across all claim-eligible roles —
can_invoke_intent alone would let a developer pass for claiming
awaiting_qa (a QA-only state).
The retrofit pattern would repeat in every claim-composing verb
(i_will_plan, claim_review, claim_doc_task). Push the per-role
narrowing inside can_invoke_action when the action is "claim",
using the same not_authorized vs invalid_state disambiguation
can_claim already implemented (status-reserved-for-another-role
returns not_authorized; status-no-role-can-claim returns
invalid_state). Extracted the body to _check_claim_rules_narrow
to keep can_invoke_action under xenon's complexity threshold.
Update _i_will_work_on_gate to drop the redundant spec.can_claim
call. Update test_consumer_parity.py to assert only against
can_invoke_intent's Decision.
Tasks 12-22 will inherit the cleaner pattern: spec.can_invoke_intent
is the single gate; verb bodies don't need per-action retrofits.
* refactor(gateway): i_will_plan uses spec.can_invoke_intent + VerbRunner
Migrates i_will_plan to the spec-driven pattern Task 11 set up for
i_will_work_on. The verb body now: (1) loads task + agent, (2) builds
Context, (3) checks idempotent/recovery re-entry, (4) calls
spec.can_invoke_intent, (5) returns Envelope.from_decision on
rejection, (6) delegates composition to VerbRunner. The
_i_will_plan_* helpers are removed — the runner replaces them.
Parity test in tests/lifecycle/test_consumer_parity.py runs the verb
against every (role × status × task_type) combo and asserts the
envelope matches spec.Decision.
* refactor(gateway): delegate uses spec.can_invoke_intent for role/state gate
Migrates delegate to the spec-driven role/state gate. The chain
validation (main_pm->cell_pm, cell_pm->its team's devs), the
assignee-vs-task_type rule (Cell PMs receive planning-typed only),
the enum coercion, and the parent-lifecycle/cap guards STAY in the
verb body — they encode delegate-specific semantics the spec
doesn't model.
Parity test in tests/lifecycle/test_consumer_parity.py asserts the
spec's role+state rejection is correctly surfaced. Chain/assignee
rejections continue to be tested in test_choreographer_pm_extras.
* refactor(gateway): open_pr uses spec.can_invoke_intent + VerbRunner
Migrates open_pr to spec-driven gating. The spec's
extra_preconditions (PRECONDITION_OWNERSHIP, PRECONDITION_COMMITS,
PRECONDITION_NO_PR) handle all three precondition checks; the verb
body delegates side-effect dispatch (push_branch, create_pr) to
VerbRunner.
Idempotent re-entry retained: an open_pr call against a task that
already has a PR (and the caller owns it) returns OK without
re-opening, rather than the tracing_gap the spec would otherwise
produce. This preserves agent ergonomics — two calls in a row
shouldn't surface a misleading "no_prior_pr" hint.
Parity test in tests/lifecycle/test_consumer_parity.py runs the verb
against representative (status x commits x pr_number) combos and
asserts the envelope matches spec.Decision.
* refactor(gateway): i_am_done uses spec.can_invoke_intent + VerbRunner
Migrates i_am_done to spec-driven gating. The spec's
extra_preconditions (PRECONDITION_OWNERSHIP, PRECONDITION_COMMITS)
handle ownership and commit-count checks; VerbRunner dispatches
the (submit_verification, submit_qa) atomic chain.
The tracing-gate preconditions (progress entry, journal:reflect,
acceptance criteria) and the field-level submit-qa gates stay in
the verb body — they model gates the spec doesn't yet cover.
Defense-in-depth: those gates run after the spec accepts the
ownership/commits checks.
Parity test in tests/lifecycle/test_consumer_parity.py runs the
verb against (role × status × ownership × commits) and asserts
the envelope matches spec.Decision.
* refactor(gateway): i_am_blocked uses spec.can_invoke_intent + VerbRunner
Migrates i_am_blocked to spec-driven gating. The journal:struggle
write stays in the verb body (it's a side effect outside the
lifecycle action). VerbRunner dispatches the `block` atomic action
via task_service.escalate.
Parity test in tests/lifecycle/test_consumer_parity.py.
* refactor(gateway): unclaim and resume use spec.can_invoke_intent
Migrates both verbs to the spec-driven gate. unclaim's verb body
keeps its dispatch (task.unclaim_for_agent) because composes=();
resume goes through VerbRunner with composes=("resume",).
The reassignment-rejection branch (introduced in
|
||
|
Renn F
|
091e4076a2 |
fix(gateway): reject Cell-PM-assigned subtasks that aren't task_type=planning
Bug B from the 2026-05-09 smoke run. main-pm called delegate(assigned_to='be-pm', task_type='code'). The chain validator let it through (be-pm IS in main-pm's allowed targets), the schema let it through (task_type='code' is a valid enum value), and the subtask got created mis-typed. Task 0 made it cosmetically work because PMs can now plan code-typed parents — but the model is wrong: a Cell PM owns the PLANNING of the slice; the code execution is what they delegate to devs. New gate in _delegate_static_guards: when assignee is a Cell PM (be-pm/fe-pm/ux-pm), task_type MUST be 'planning'. Returns invalid_state with a remediate hint pointing at the right type. Devs are unrestricted (could be code OR documentation, depending on the slice). Tests: 3139 passing (+ 2 regression tests pinning the rule), 100% coverage, ruff clean. |
||
|
Renn F
|
73e1e96851 | Many fixes and cleanups | ||
|
Renn F
|
d819c28893 |
fix(gateway): make i_will_work_on plan-precondition atomic; recover claimed-no-plan
Bug A from the 2026-05-09 smoke run. be-dev-1 called i_will_work_on without `plan` on a pending task; claim() ran first (transitioned to `claimed`), then the plan check failed → tracing_gap. The natural retry path then dispatched to `_i_will_work_on_claimed`, which had no plan-recovery logic and called start() against a still-plan-less task, returning `start failed` forever. The dev kept looping; the parent escalated up; the whole slice ended `blocked`. Two changes (Task-5 atomicity pattern applied to i_will_work_on): 1. `_i_will_work_on_pending`: move the plan precondition BEFORE `claim()`. A missing-plan first call now returns tracing_gap with the task untouched in `pending`, so the agent's retry-with-plan succeeds cleanly. 2. `_i_will_work_on_claimed`: now accepts `plan` and calls set_plan before start() if the task has no plan yet. Recovery path for any already-stuck task (e.g. left over from the earlier image, or an orchestrator restart that left a partial claim). Also wires `plan` through the dispatcher to the claimed branch. Tests: 3137 passing (3135 + 2 regression tests pinning the atomic invariant), 100% coverage, ruff clean. |
||
|
Renn F
|
b601441da7 |
fix(audit): record actor's actual role from agents.role at write time
The 2026-05-08 trace caught an audit row with actor=main-pm but
agent_role=cell_pm — the caller had supplied the verb's *expected*
role rather than the actor's actual role. Forensics work that joined
audit_log on agent_role would silently miscategorize the row.
Fix: AuditService now reads the actor's role directly from
agents.role at write time via the new _resolve_actor_role_from_db
helper. Wired into log_task_action_denial,
log_state_transition_denial, and log_notification_denial. The
caller-supplied role param is kept as a best-effort fallback for the
case where the DB lookup fails (singleton-without-DB paths,
permission errors, etc.) so audit writes never block the operation
being audited.
Coverage:
- 3 unit tests (test_audit.py) for the no-DB / invalid-id paths
- 1 unit-with-real-DB test (test_audit_real_query.py) verifying
the persisted row's agent_role is read from DB even when the
caller passes a deliberately-wrong role
- 1 unit-with-real-DB test for the no-row case
Tests: 3135 passing, 100% coverage, ruff clean.
|
||
|
Renn F
|
19f27b4f88 |
fix(gateway): improve unclaim/resume rejection when task was reassigned
Investigation finding for Task 6 of the gateway introspection plan:
the 2026-05-08 trace's "not your claim" rejection at 02:51:22 / 02:52:48
was NOT caused by a UUID-comparator bug. AGENT_UUIDS in
roboco/seeds/initial_data.py are static so identity is stable across
restarts, and SQLAlchemy + Pydantic both round-trip UUIDs cleanly
(pinned by two new regression tests in test_task_service_misc.py).
The actual cause: the task was REASSIGNED out from under main-pm by
an upstream verb between when main-pm last touched it and when it
tried to unclaim/resume. Common triggers:
- cell_pm_complete propagates up via _maybe_advance_parent_to_pm_review,
which reassigns the parent to the cell PM for the team
- main_pm_complete clears assigned_to to None (CEO acts via UI)
- unblock with restore=True flips assigned_to back to pre_block_state
Pre-fix the rejection said only "not your claim" — agents can't tell
whether they hit a transient race or whether the task was legitimately
moved on. Fix: surface the current_owner UUID and hint that an
upstream verb did this, telling the agent to call give_me_work() to
find its current work.
|
||
|
Renn F
|
6806516015 |
refactor(gateway): rename submit_for_qa to open_pr; pin atomic preconditions
Pre-fix, submit_for_qa opened a PR (side effect) and returned OK with next='call i_am_done' — agents read the verb name, assumed they were done with QA handoff, never called i_am_done, and PRs ended up orphaned (PR #12 in the 2026-05-08 trace). Two changes: 1. Rename submit_for_qa -> open_pr so the verb name matches the semantic. The PR opens here; the actual QA handoff happens at i_am_done. Renamed across: - choreographer/_impl.py (method) - mcp/flow_server.py (tool registration + _TOOLS dict) - api/routes/v2/flow_dev.py (route + handler) - api/schemas/v2/flow.py (OpenPrRequest) - services/gateway/verb_gates.py (_STATE_VERBS) - services/gateway/role_config.py (developer flow manifest) - services/gateway/content_actions.py (commit-success next= hint) - agent_sdk/server.py (post-tool guidance map) - runtime/orchestrator.py (developer prompt) - agents/prompts/{base,roles/developer,_generated/*}.md - tests/unit/gateway/test_submit_for_qa.py -> test_open_pr.py - tests/unit/api/routes/v2/test_flow_dev.py - tests/unit/gateway/test_verb_gates.py - tests/unit/api/test_correlation_id.py - tests/unit/mcp_servers/test_flow_server.py - tests/integration/test_full_lifecycle_real_db.py 2. New regression test (test_open_pr_does_not_create_pr_if_no_commits) pins the atomic invariant: preconditions (assignee, commits, no-prior-PR) must be checked BEFORE git.create_pr/push_branch run. Any future re-ordering breaks the test. Tests: 3128 passing (3127 + 1 new), 100% coverage, ruff clean. Note: TaskService.submit_for_qa() (the v1-layer service method) is INTENTIONALLY not renamed — it's a different layer used by the v1 routes. The rename here is only the gateway verb surface. |
||
|
Renn F
|
2eeefb2ee1 |
refactor(gateway): consolidate commit + notify role gates into verb_gates
Replaces hardcoded role-string-constants in content_actions.py (_COMMIT_ALLOWED_ROLES, _NOTIFY_ALLOWED_ROLES) with calls into verb_gates.is_verb_allowed against a synthetic in-progress task probe. Pre-fix the same role lists lived in both content_actions and verb_gates; if one drifted the other would mask it. Now there's one table. Adds `notify` to verb_gates._ALWAYS_AVAILABLE for cell_pm, main_pm, product_owner, head_marketing. Note: i_will_plan / delegate role checks INTENTIONALLY stay as explicit `role not in (cell_pm, main_pm)` checks, not is_verb_allowed. Their state checks must surface as `invalid_state` (a different agent-side error code) — conflating them with role-state combo checks breaks the rejection-code semantics agents rely on. Tests: 3127 passing, 100% coverage, ruff clean. |
||
|
Renn F
|
ebdbd7fc47 |
feat(gateway): wire envelope introspection into qa.py + doc.py role mixins
Closes the Task 3 wiring loop: every Envelope construction site in the QA and Documenter mixins now stamps current_state + valid_next_verbs. Refactored doc._check_i_documented_inputs to take the loaded task as a parameter so it can pass through to .with_introspection() without re-fetching. Task 3 of the 2026-05-08 gateway introspection plan is now complete across _impl.py, qa.py, and doc.py. |
||
|
Renn F
|
17630bf3fc |
feat(gateway): wire envelope introspection into i_am_blocked, unclaim, resume,
escalate_up, escalate_to_ceo, submit_up, unblock Continues Task 3 of the gateway introspection plan. Every developer- and PM-facing lifecycle verb in _impl.py now stamps current_state + valid_next_verbs on both the success path and (where the task was loaded successfully) on rejection paths. The qa.py / doc.py role mixins still need wiring; that's a follow-up since their structure mirrors what's already been done here. |
||
|
Renn F
|
6092df59b2 |
feat(gateway): wire envelope introspection into complete + main_pm/cell_pm complete
Trace-driven priority: the 2026-05-08 audit log showed PMs spamming `complete` against tasks in the wrong status (claimed/in_progress rather than awaiting_pm_review). Introspection on the rejection path now tells the PM the actual current_state and which verbs are valid right now (typically `delegate` / `escalate_up`), shrinking the trial-and-error loop. Continues Task 3 of the gateway introspection plan. |
||
|
Renn F
|
56fbb97a80 |
feat(gateway): wire envelope introspection into submit_for_qa, i_am_done
submit_for_qa: ok envelopes (PR-already-open + new-PR) and not-assigned / no-commits rejections now stamp current_state + valid_next_verbs. i_am_done: not-assigned rejection plus tracing-gap and field-gate rejections all stamp introspection. Success path inherits via the shared _build_i_am_done_ok helper. Continues Task 3 of the gateway introspection plan. |
||
|
Renn F
|
93c66fa07a |
feat(gateway): wire envelope introspection into give_me_work, i_will_plan, delegate
Continues Task 3 of the gateway introspection plan: every successful or rejected envelope from these three verbs now carries current_state + valid_next_verbs sourced from verb_gates.valid_next_verbs(role, task). Remaining verbs (submit_for_qa, i_am_done, i_am_blocked, unclaim, resume, unblock, complete, main_pm_complete, escalate_up, escalate_to_ceo, submit_up, plus the qa.py / doc.py role-mixins) to be wired in subsequent commits — the choke-point pattern at each call site is `.with_introspection(task=t, role=role)` on the constructed Envelope, so it stays mechanical from here. |
||
|
Renn F
|
fd344511a5 |
feat(gateway): add Envelope introspection (current_state + valid_next_verbs)
Pre-fix, agents had no way to introspect what verbs were valid from a
task's current state — the 2026-05-08 trace showed them spamming
escalate_to_ceo/complete/unblock/resume against a `claimed` task and
racking up rejections. Pre-gateway agents could ground reasoning in
VALID_TRANSITIONS[status] from a doc; the gateway hid that.
Now every Envelope carries:
- current_state: the task's status string (or None for tool-discovery
envelopes that aren't task-bound)
- valid_next_verbs: the verbs the caller can usefully call right now,
sourced from verb_gates.valid_next_verbs(role, task)
Wired into i_will_work_on (highest-traffic verb) for both the OK path
and the wrong-state rejection path. Remaining lifecycle verbs to be
wired in subsequent commits (Task 3.9).
|
||
|
Renn F
|
b4ec19ca9c |
feat(gateway): add verb_gates single source of truth for role x state
Pre-2026-05-08, role checks lived in three places that could disagree
silently:
1. roboco/services/gateway/role_config.py — verb allow-list per
role (used by spawn manifest)
2. roboco/services/gateway/claim_guards.py — pm_cannot_execute_code,
role_typed_claim
3. Choreographer string constants in _impl.py / qa.py / doc.py /
content_actions.py
verb_gates.valid_next_verbs(role, task) collapses them into one
declarative table mapping (role, task_status) -> tuple of valid verbs,
plus a per-role set of always-available verbs. Will be wired into
Envelope.valid_next_verbs in the next task so agents stop
trial-and-erroring against the gateway, and into the choreographer
guards in Task 4.
|
||
|
Renn F
|
01ff44b83f |
fix(gateway): unblock PM planning + drop magic delegate task_type
Two coupled fixes from the 2026-05-08 smoke-test trace: 1. pm_cannot_execute_code is now scoped to i_will_work_on (the EXECUTION verb) only. Pre-fix it also fired on i_will_plan, which deadlocked any code-typed parent: cell_pm couldn't plan, so couldn't transition parent to in_progress, so couldn't delegate. PMs PLAN code-typed parents and DELEGATE the work — that's exactly the verb we were blocking. 2. delegate.task_type is now REQUIRED at both the HTTP boundary (DelegateRequest) and the choreographer dataclass (DelegateInputs). The pre-fix default of 'code' silently changed semantics whenever a caller forgot the field — main-pm's call in the smoke trace omitted it, schema defaulted to 'code', and the cell PM downstream was wedged. Also drops the choreographer's task.task_type fallback (the DB column is NOT NULL anyway). Plus middleware coverage tests for the parallel ServiceError → 4xx handler hierarchy added in the prior session, restoring 100% coverage across the touched files. Tests: 3101 passing, 100% coverage, ruff clean. |
||
|
Renn F
|
f0eec854d1 | Code quality | ||
|
Renn F
|
9aa30fb945 | 100% Coverage | ||
|
Renn F
|
64c48356d0 |
test: lift coverage 41% → 76% (+1068 tests across 36 files)
Service-level tests now exercise provider, permissions, project, journal, messaging, work_session, metrics, kanban, extraction, learning, notification, dashboard, llm_routing, a2a, task, repository_base, audit, db_seed, branch_name, indexed_document, query_helpers, agent. API route tests cover provider, journal, project, sessions, dashboard, work_session, tasks, a2a, groups, notifications, agents, channels, messages, kanban, api_resources. Pure-function helpers covered: handlers, deps_helpers, middleware, middleware_docs, transcription, pr templates, agents_config, errors, logging, journal/notification/channel/a2a access, task_lifecycle, streaming, converters, crypto, schemas (common + websocket), events, permissions extras. pyproject ruff per-file-ignores extended for tests so PLR2004 (status code magic values), PLC0415 (lazy imports), PLR0913 (fixture params), ARG001 (unused fixture deps), SIM105, and E501 don't fight test idioms. |
||
|
Renn F
|
b6903490f1 | + tests | ||
|
Renn F
|
85ef124c8f | Quality Gates | ||
|
Renn F
|
4829f93a68 |
fix(gateway): unblock task claim; full Phase 0/1/2 remediation
Resolves the 100% claim-failure rate introduced by the gateway rewrite
(commit
|
||
|
Renn F
|
0a3c963923 |
fix(gateway): make i_will_plan / i_will_work_on idempotent on re-entry
Smoke 2026-05-04 captured the cycle the prior
|
||
|
Renn F
|
cfdd389c43 |
test(gateway): regression for ContentActions.commit role gate
Pin: PM, QA, Board cannot call commit; developer + documenter can. Smoke 2026-05-03 saw main-pm reach the git layer with a 'commit' call attempting to author a fix to the very gateway bug we were hitting. |
||
|
Renn F
|
c61a3bf81e |
fix(gateway): role-gate ContentActions.commit (developers + documenters only)
Smoke 2026-05-03 saw main-pm reach the git layer with a 'commit' call, trying to author 'fix(gateway): allow claimed status in i_will_plan preflight'. That should never have been possible — main_pm/cell_pm/ board/auditor/qa manifests all exclude commit. Reaching the verb body means either the MCP manifest filter mis-routed, or the agent hit the v2 do.py route directly. Mirror Task 16 notify pattern: server-side role check in the verb body rejects with not_authorized + 'PMs delegate, do not commit' remediate. Defense-in-depth — manifest is still the primary gate. |
||
|
Renn F
|
ee743ffc7a |
fix(git): pass actor_agent_id to pr_merge for workspace resolution
pr_merge falls back to task.assigned_to for workspace resolution, but that field is None at merge time (submit_qa/pass_qa cleared it during prior transitions). When project.workspace_path is unset, the resolver raises ValidationError 'no workspace configured and no agent_id provided' — surfaces as 500 from cell_pm_complete. Add actor_agent_id parameter (the PM doing the merge) and use it as the primary workspace owner. Falls back to task.assigned_to, then created_by, before raising. cell_pm_complete now threads pm_agent_id through. |
||
|
Renn F
|
24279fbee4 |
test(task): pin claim() seeds last_heartbeat_at (regression for 931eb0a)
Real-DB integration test: pending pre-assigned task → claim() → status=CLAIMED AND last_heartbeat_at populated. Without the seed (reverted in unit testing), the reaper would interpret NULL as stale and reap the freshly-claimed task on the next dispatch tick. |
||
|
Renn F
|
638e06910f |
fix(orchestrator/prompts): enumerate task_type/complexity values explicitly
Smoke run agents picked invalid enums ('development', 'small') because
spawn prompts used 'code|documentation|...' format with literal '...'
that LLMs interpret as 'fill in something creative'. Replace with
explicit '<one of "a" / "b" / ...>' enumerations matching the
TaskType + Complexity enum members, plus an explicit reminder that
the gateway rejects invented values.
|
||
|
Renn F
|
931eb0a40f |
fix(task): seed last_heartbeat_at on claim to stop reaper tight-loop
Smoke 2026-05-03: reaper fired ~12 times/sec against a freshly-claimed task because _finalize_claim never seeded last_heartbeat_at. Reaper's 'NULL means stale' rule then reaped the claim within ~250ms of the claim landing, agent re-claimed via i_will_plan, reaper reaped again. Set last_heartbeat_at = claimed_at in _finalize_claim so the freshly- claimed task carries an authentic heartbeat from second zero. Reaper logic stays untouched. |
||
|
Renn F
|
809f169bb0 |
test(gateway): regression tests for i_will_plan claim/start bugs
Pin the smoke-2026-05-03 bugs that motivated
|
||
|
Renn F
|
63d0adfaa5 |
fix(gateway): always claim() pending tasks in i_will_plan/i_will_work_on
Smoke run revealed CEO-pre-assigned root tasks stay stuck in pending because both verbs skipped claim() when task.assigned_to already matched the caller. Claim is what transitions pending → claimed; without it, start() refused the claimed → in_progress transition and silently returned None. i_will_plan then returned an OK envelope with a fabricated 'in_progress' status string while the DB stayed pending, causing the next delegate() call to correctly reject with PARENT_NOT_CLAIMED. Agent looped on delegate retries, never figuring out claim was the missing step. Fix: - Drive claim() on status == pending (idempotent for same assignee), not on assigned_to mismatch. - If start() returns None, surface invalid_state envelope instead of fabricating success. |
||
|
Renn F
|
9310d66508 |
chore(post-audit): document notify verb + remove # type: ignore
Audit followups before pushing the gateway-restoration batch:
- Add notify() row to cell_pm.md, main_pm.md, board.md verb tables.
Manifests + routes + MCP + tests all wired in
|
||
|
Renn F
|
9bae446cbe | Linting/Formatting | ||
|
Renn F
|
95c96d4ce3 |
chore(work_session): delete dead generate_branch_name helper
Used '/' separator while live build_branch_name uses '--'. Footgun for future contributors. No live callers; safe to remove. |
||
|
Renn F
|
f8e07d47e3 |
fix(optimal): make IndexJournalEntryParams.entry_id required
Task 22 added a runtime ValueError when entry_id is None, but the dataclass still typed it Optional. Contract-vs-runtime split — callers get no IDE/mypy hint about the required field. Tighten the type to UUID (required) and remove the misleading "can be None for system events" docstring note. Lifecycle events already use their synthetic uuid5 path, so no real caller is broken. |
||
|
Renn F
|
b0107f8c12 |
fix(optimal): raise instead of falling back to 'unknown' doc source
Silent fallback hid an upstream bug where journal_entry.id wasn't flushed before indexing. Raise so we see the regression. |
||
|
Renn F
|
279aceab3d |
chore(docker): set ROBOCO_PUBLIC_BASE_URL for commit-link rendering
Default 127.0.0.1 produced unusable links in commit message bodies. Set to the NAS LAN IP. |
||
|
Renn F
|
d6f64f4b9d |
fix(audit): populate agent_id on task.* and agent.* events
task.awaiting_qa fired after submit_qa cleared claimed_by; agent.* events stored slug-only. Capture claimed_by before mutation and add slug→UUID resolver in orchestrator audit path. |
||
|
Renn F
|
bf44d5aade |
fix(orchestrator): skip closure spawn if PM just paused via i_am_idle
Tiny race: dispatcher decided to spawn for closure between agent's heartbeat and idle-pause. Spawn would land against an already-paused parent. Gate spawn on (status != PAUSED OR last_heartbeat older than cutoff). |
||
|
Renn F
|
3cabee155e |
chore(lifecycle): remove quarantined state (phantom)
State existed in the lifecycle table and the enum but no verb, route, or service path ever set it. Removing dead state. If we need problem-task isolation later we'll add it explicitly with a verb. |
||
|
Renn F
|
5c742e6aec |
fix(git): serialize concurrent merges to the same parent branch
Two PMs completing different subtasks of the same parent could race on gh API merge calls. GitHub returns 409 to one but local DB write ordering wasn't guaranteed. Take row-level lock on parent task before merge; retry once on 409. |
||
|
Renn F
|
3a2498a609 |
feat(gateway): notify(target, text, scope?) for PMs and Board
Pre-gateway PMs/Board sent formal notifications (require ack); gateway had say/dm only. Now PMs and Board can issue ack-required notifications via NotificationService through the standard envelope path. |
||
|
Renn F
|
1ed1317a35 |
fix(workspace): re-apply agent ownership after refresh fetch
I1: _fetch_origin_best_effort runs as root and writes new pack files + ref updates that land root-owned, undoing _ensure_agent_owned that ran before. Subsequent spawns hit Permission denied. Mirror the fetch_branch_for_inspection pattern: re-run _ensure_agent_owned AFTER the fetch. I2: separate workspace_refresh_fetch_timeout_seconds (default 60s) from workspace_clone_timeout (300s). Refresh transfers small deltas; 300s of blocking on every spawn against a hung remote is operationally bad. 60s is enough for any sane refresh. |
||
|
Renn F
|
e21ecd000b |
feat(workspace): fetch refs on healthy-clone re-entry
ensure_workspace short-circuited when the clone existed, so a respawned PM/Doc could be reviewing arbitrarily stale diffs. Add a best-effort 'git fetch origin' on every entry; checkout unchanged. |
||
|
Renn F
|
074f47a2f9 |
feat(observability): propagate correlation_id end-to-end
X-Correlation-ID was bound to structlog but lost on the MCP→API hop. MCP shims now forward it; Envelope carries it back to the agent; audit_log row records it for forensic joins. |
||
|
Renn F
|
99eac69aab |
fix(audit): migrate audit_log.details to JSONB so .astext works
Task 13's has_recent_tracing_gap query used .astext on a generic JSON column, which raises AttributeError at runtime. The choreographer's exception handler swallowed it, leaving the strike-count reset permanently inert in production. Migrate the column to JSONB (which supports .astext + GIN indexing for future audit queries), update the ORM, and add a real-DB integration test that would have caught this. |
||
|
Renn F
|
44784293c7 |
fix(orchestrator): respect tracing-gap as forward progress
_pm_respawn_should_gate counted PARENT_NOT_CLAIMED rejections as no-progress and killed PMs after 3 strikes — even when the new prompts told them to call i_will_plan first. Reset counter when last response was a tracing_gap (rule-following retry, not stuck). |
||
|
Renn F
|
87ef42bf09 |
chore(orchestrator): enable gateway cooldown logic in production
ROBOCO_GATEWAY_ENABLED defaulted to False, leaving trigger_filter's spawn cooldown / role-rate logic dormant. Flip to true and add the gateway_triggers table migration if missing. Without this, respawn rate has no server-side limit besides _pm_respawn_should_gate. |
||
|
Renn F
|
00b385c019 |
fix(orchestrator): retry parent branch lookup to close PM race
When a PM's i_will_plan and a child dev's spawn fire in the same tick, the dev sometimes saw branch_name=None because the PM's transaction hadn't committed yet. Auto-block fired and the dev sat blocked until the next 30s tick. Add 3x250ms retries when parent is mid-claim. |
||
|
Renn F
|
41d7e8295e |
fix(task): resume_for_agent delegates to resume() for observability
I2: resume_for_agent did its own _validate_and_set_status call, skipping resume()'s structlog 'Task resumed' event and the fire-and- forget RAG lifecycle-event indexing. Gateway-driven resumes were invisible to logs and the RAG corpus. Delegate to resume() after the gateway-specific ownership pre-checks (mirrors pause_for_agent's pattern). I1: documented why Choreographer.unclaim does NOT call _touch on its OK path - assigned_to is cleared, no claimant heartbeat to refresh. Prevents future readers from 'fixing' the asymmetry. |