mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
5936c2bdea8a1f2d25bd523c2d64bbc9f9e5aa4d
362
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Renn F
|
5da909d9d7 |
fix(gateway): cross-team planning fanout + complete tracing-gap hints
Task #157 — spine-cap allows planning fanout across cells:
main-pm's pattern is to delegate planning to be-pm / fe-pm / ux-pm
in parallel — each on a different team. The previous spine-cap
rejected all planning siblings under one parent as
over-decomposition. New helper _is_cross_team_planning skips the
cap for planning when both teams are non-empty and distinct. Code
/ documentation stay capped regardless (single repo on one branch
shouldn't have two simultaneous code subtasks).
Task #159 — tracing-gap remediate hints every requirement:
journal:during_work>=1, journal:struggle, commits>=1, pr_open,
and self_verified had no entries in _hint_for_missing_key, so when
they were missing the agent saw the token in `missing[]` but the
`remediate` text had no instruction for how to satisfy them.
Smoke-10's be-dev-1 burned multiple turns retrying i_am_done not
knowing scope='reflect' doesn't count toward during_work. Now
every token has a hint, the during_work hint warns that reflect
doesn't satisfy it, and multi-hint remediate uses a numbered list
so the model treats each requirement as a distinct step instead
of a semicolon-blob.
Also coerce convert_plan._coerce_risk formatting (ruff-format follow-up
to
|
||
|
Renn F
|
2c838c2a9e |
feat(gateway): propagate sessions to subtasks + auto-emit milestone progress
Task #156 (sessions): pre-gateway flow created a session for the whole task tree at once, so subtasks were visible in the PM's group chat the moment they existed. The gateway creates subtasks one-by-one via delegate(), losing that wiring. Added MessagingService.propagate_sessions_to_subtask and threaded it through the choreographer's _create_subtask_from_inputs. ChoreographerDeps grew an optional `messaging` field so existing test wirings keep working. Task #155 (progress): smoke-9 had zero progress entries because the dev never called progress() explicitly. Added _record_milestone_progress and fire it server-side from two natural milestones — open_pr ("opened PR #N", 70%) and i_am_done ("submitted for QA review", 90%). Best-effort write (contextlib.suppress) so a progress failure cannot break the verb path. Extracted _open_pr_success_envelope to keep cyclomatic rank ≤ B. |
||
|
Renn F
|
4fdde2b082 |
fix(gateway): evidence/QA/doc paths populate files_changed from git
Bug:
ContentActions.evidence() hard-coded files_changed=[] and diffed
against HEAD~1 instead of the branch's parent. QA's _build_qa_claim_
evidence (and doc/_impl mirrors) sourced files_changed from
work_session.files_modified, which the gateway commit() never
populates (no add_files_modified plumbing). Result: QA / docs / PM
reviewers saw an empty change list on real PRs and only the latest
commit's delta — flagged in smoke-9 when PR #20 showed the README
change on GitHub but evidence() reported empty.
Fix:
Added GitService.list_changed_files (git diff --name-only against
parent branch). evidence(), _build_qa_claim_evidence,
_claim_doc_evidence, and _build_i_am_done_ok all source files_changed
from this — git is the authoritative source. evidence() also drops
the HEAD~1 base so the diff is the full PR.
Wired EvidenceRepo into ContentActionsDeps so evidence() returns
journal_highlights too, matching the QA/doc shape.
|
||
|
Renn F
|
d5ff8c7b13 |
fix(gateway): i_will_plan persists rich plan dict, not raw string
Bug:
Choreographer.i_will_plan() built spec_ctx with the raw plan string
while ctx (_ClaimPlanStartContext) got the resolved (panel-shaped)
dict. The verb runner uses spec_ctx — so the rich shape never reached
TaskService.set_plan. The panel's Plan tab stayed empty even when
PMs supplied approach / sub_tasks / risks.
Fix:
Pass the resolved (possibly-dict) plan into spec_ctx too. Widened
lifecycle.Context.plan to `str | dict[str, Any] | None` to match.
Tightened _resolve_effective_plan to require a non-empty narrative
paragraph — rich structure layers on top of prose, not in place of it.
|
||
|
Renn F
|
e3570b444f |
fix(orchestrator): briefing renders ToolSearch directive + current verb names
Smoke-8 follow-up. Two issues in _write_agent_briefing: 1. _build_tool_load_block was scraping role prompts for a "## Load on spawn" section that doesn't exist in any role file. Returned "" for every role → no ToolSearch directive in the briefing. Combined with weak models skipping the system-prompt-layer directive (#144), the agent's first action was Edit → "not enabled in this context." Fix: per-role tool list lives in the orchestrator (mirrors factories._base.py). Pre-renders the directive directly. developer and documenter get Edit + Write; QA/PMs/board get the common read-only set. 7 tests pin the contract. 2. The briefing's "Terminal tools (how to exit cleanly)" section still listed pre-gateway verb names: roboco_agent_idle, roboco_task_substitute, roboco_task_escalate, roboco_task_submit_qa, _qa_pass/fail, _docs_complete, _complete. Same rename pattern as #145's _TERMINAL_TOOLS set. Updated to: i_am_idle, i_am_blocked, unclaim, i_am_done, pass, fail, i_documented, complete, submit_up, escalate_up, escalate_to_ceo. The agent now reads the same directive in two places (system prompt + session briefing) — the second touch point catches weak models that skip the first. |
||
|
Renn F
|
cfefe85f87 |
fix(orchestrator): don't auto-restart on graceful exit; tighten role-status
Smoke-8 surfaced a tight respawn loop: QA failed a PR cleanly, container exited 0, then _check_health bumped error_count and respawned QA with the same task_id. But by then the task was in needs_revision (dev's state), so QA's claim_review was rejected — and the cycle repeated on the next health tick. Token-burning loop. Two layers: 1. _check_health now reads docker's exit code. exit_code == 0 → graceful (intentional handoff via i_am_idle / clean shutdown) → reset error_count, do NOT auto-restart. Non-zero → keep the existing crash-retry behavior. Refactored into _inspect_container_state + _handle_stopped_container to keep xenon's complexity check happy. 2. _readiness_check_role_for_status now includes the dev-owned states (needs_revision, verifying) so a misrouted spawn for QA / PM / board on these statuses fails the readiness gate before the gateway has to reject it. Defense in depth — the right path is #1 (don't respawn on clean exit at all), but if some other code path tries to spawn QA on needs_revision the gate now catches it. Tests: 12 new (5 for _check_health graceful/crash matrix + 7 for the expanded role-status table). Pre-gateway names (none of which were needed here) untouched. |
||
|
Renn F
|
87b18bc64f |
fix(gateway): spine-cap remediate forbids task_type workaround
Smoke-7: be-pm got the expected spine-cap rejection on a second delegate. The remediate said "drive the existing sibling to completion / cancel it, OR split this parent into two sibling parents". The model read that, decided neither applied, and "adapted" by re-delegating with task_type='documentation' as a "verification subtask". The gateway accepted it (different type = no cap collision) but the orphan subtask had no claimant — it blocked submit_up forever with "subtasks not all terminal". Two-layer fix: 1. _spine_type_dup_envelope remediate now explicitly forbids the workaround: "DO NOT work around this by delegating again with a different task_type (e.g. 'documentation' or 'research' as a 'verification' subtask). The lifecycle handles QA, documentation, and PM-review automatically after the code subtask finishes — you do not create auxiliary subtasks for those roles. Call i_am_idle() now and wait for the existing child to come back." 2. cell_pm.md workflow step 6 strengthened to name the anti-pattern explicitly: no verification subtask (QA is the verification step); never re-delegate with a different task_type as a workaround. 3 new tests pin the remediate text: forbids workaround, names the verification anti-pattern, retains invalid_state error kind. |
||
|
Renn F
|
d4126ffb7f |
fix(sdk): _TERMINAL_TOOLS uses current gateway verb names
Smoke-7 evidence: every agent's first successful i_am_idle was followed
by a stop-hook error "you stopped without calling a terminal tool"
even though i_am_idle had just succeeded.
Root cause: agent_sdk._TERMINAL_TOOLS still held pre-gateway names
(roboco_agent_idle, roboco_task_submit_qa, ...). /terminal/tool_recorded
strips the `mcp__roboco-flow__` prefix and stores 'i_am_idle' — the
membership check against {'roboco_agent_idle', ...} never matched, so
had_terminal_recently() always returned False, and the stop hook nagged
every clean exit. Wasted ~2-3 turns per agent + burned stop_allowance.
Fix: rebuilt _TERMINAL_TOOLS with the current gateway verb names:
i_am_idle, i_am_done, i_am_blocked, i_documented, unclaim, pass, fail,
complete, submit_up, escalate_up, escalate_to_ceo.
7 new tests pin:
- every role's terminal verbs are recognized
- pre-gateway names are NOT in the set
- _SessionState.had_terminal_recently() returns True after i_am_idle
|
||
|
Renn F
|
197b1576c3 |
fix(prompts): hoist ToolSearch activation to top of system prompt
Smoke-7: be-dev-1 hit "Edit exists but is not enabled in this context." Claude Code v2.1.69+ defers built-in tools (Edit, Write, Read, etc.) behind a ToolSearch call. Weak models (minimax-m2.7) skip soft directives buried in the briefing. Also: 4 role prompts (developer, cell_pm, main_pm, board) claimed "no ToolSearch needed" — a lie that compounds the problem. The manifest registers MCP tools; built-in tools are still deferred. Fix: compose_prompt now prepends a tool-load directive layer as the FIRST block in the system prompt. It names the exact ToolSearch call the role needs: - developer/documenter: Read, Bash, Grep, Glob, Task, TodoWrite, Edit, Write - qa/pm/board: Read, Bash, Grep, Glob, Task, TodoWrite (no Edit/Write) The directive includes the failure mode it prevents so the model understands what skipping the call causes. Updated role-prompt lines that lied about ToolSearch. 7 new tests pin: directive is the first block; developer/documenter get Edit/Write; qa/pm don't; failure-mode message is present. |
||
|
Renn F
|
417b8c5f29 |
fix(gateway): dm catches A2AAccessDeniedError; circuit breakers handle dict errors
Smoke-7 surfaced: be-qa called dm(recipient='qa-all', ...) — 'qa-all'
is a channel slug, not an agent. A2A enforcement raised
A2AAccessDeniedError. It propagated past dm(), past content_actions,
got caught by FastAPI middleware which renders RobocoError.to_dict()
as {'error': {'code': ..., 'message': ..., 'details': ...}} — a
DICT-shaped 'error' field.
do_server's circuit-breaker check (and flow_server's mirror) did
`payload.get('error') in _CIRCUIT_REJECTION_KINDS` — trying to hash
a dict against a frozenset → `TypeError: unhashable type: 'dict'`.
The agent saw "Error executing tool dm: unhashable type: 'dict'"
and got stuck calling dm in a loop.
Two-layer fix:
1. content_actions.dm now catches A2AAccessDeniedError and returns
Envelope.not_authorized with the original reason + route_hint as
remediate. This is the right shape — content tools always emit
Envelopes; RobocoErrors escaping to the middleware is a bug.
2. Defense-in-depth: do_server._record_and_check_circuit and
flow_server._record_and_check_circuit now guard against non-string
error fields. Any future RobocoError-leak that bypasses (1) will
pass through untouched instead of crashing the tool call.
3 new tests pin the contracts:
- dm A2A denial returns Envelope.not_authorized (not propagated)
- do_server circuit-breaker doesn't crash on dict-shaped errors
|
||
|
Renn F
|
b90ce83946 |
fix(mcp): expose pass/fail to QA via IntentSpec→public name mapping
Smoke-7 surfaced this: QA spawned, claim_review succeeded, but every
attempt to call `pass()` fell through to dm/say workarounds. The MCP
tool 'pass' never existed.
Root cause: foundation.policy.lifecycle declares the intent verbs as
`pass_review`/`fail_review` (Python-friendly names — `pass`/`fail` are
keywords). intents_for_role(Role.QA) returns those names, the spawn
manifest carries them, and flow_server reads them. But flow_server's
_TOOLS dict has keys 'pass'/'fail' — the manifest's pass_review keys
didn't match and got silently dropped from the registration.
Fix: add _INTENT_TO_PUBLIC = {'pass_review': 'pass', 'fail_review':
'fail'} in flow_server. _register_tools transforms manifest names
through it before _TOOLS lookup. Manifest entries map to the public
MCP tool names the prompts advertise.
Also fixed _VERB_RETRY_LIMITS keys in foundation.agent_loop — they
used the IntentSpec names too, but the SDK receives the public name
from /verb/attempted (derived from the flow URL path), so the limit
entries never matched real rejections. Renamed to 'pass'/'fail'.
3 regression tests pin: pass/fail register under public names;
IntentSpec names don't leak through; the registered tool POSTs to
the correct orchestrator path.
|
||
|
Renn F
|
3bbaf0d645 |
fix(gateway): reject null + DO-NOT-PASS-NULL remediate for decision/reflect
Smoke-6 found the agent calling note(scope='decision', context=null, chosen=null, rationale=null) eight times in a row. Root cause split across two surfaces: 1. The MCP tool schema declared these fields as `str | None = None`, producing a JSON schema of `anyOf [string, null]`. minimax-m2.7 read that and decided null was a valid value — passed it on every retry. 2. The remediate text used `<placeholder>` syntax for the example without telling the agent "don't pass null" explicitly. Fixes: - roboco/api/schemas/v2/do.py NoteRequest: context, chosen, rationale, what_done, what_learned, what_struggled now typed `str = ""` (no None). Pydantic on the route rejects literal null with 422 BEFORE the gateway sees it. Empty string still counts as missing at the gate. - roboco/mcp/do_server.py note(): matching signature changes so the MCP tool schema declares the fields as `string` not `anyOf[string,null]`. - roboco/services/gateway/content_actions.py: remediate text now opens with "DO NOT pass null" and the example uses concrete values (redis vs postgres) instead of <angle bracket> placeholders. Reflect remediate also gets the don't-pass-null intro and concrete values. 8 new tests pin "schema rejects null for each of the 6 string fields" plus "empty defaults work for unscoped notes". |
||
|
Renn F
|
21007e122f |
fix(mcp): do_server per-verb circuit breaker mirrors flow_server
Smoke-6 surfaced the gap. main-pm called note(scope='decision') with
context: null 8 times in a row — every one returned incomplete_input
and the agent kept retrying. The flow_server had a breaker (C1) but
do_server didn't, so content-tool rejections went uncapped.
Mirror the flow_server pattern:
- _CIRCUIT_REJECTION_KINDS = {tracing_gap, invalid_state,
not_authorized, incomplete_input} (same set)
- _record_and_check_circuit posts to the SDK's /verb/attempted on each
counted rejection
- When the SDK reports open=true, the original rejection envelope is
REPLACED with the circuit_open envelope so the agent stops retrying
The SDK side (agent_sdk/server.py) already accepts arbitrary verb
names; no changes there. note hits the default cap of 3 retries / 60s
from foundation.agent_loop. After the third incomplete_input the
agent gets circuit_open and the loop ends.
9 new tests pinning the contract.
|
||
|
Renn F
|
74b7c39612 |
feat(gateway): wire pr_update verb — ContentActions + route + MCP
ContentActions.pr_update enforces: - task.pr_number is set (else invalid_state, remediate 'call open_pr') - at least one of title/body/reviewers is non-None (else invalid_state) - caller is task assignee OR PM on team (cell_pm same-team / main_pm cross-team), else not_authorized - GitError raised by the underlying service maps to invalid_state with the upstream message preserved The route at POST /api/v2/do/pr_update binds PRUpdateRequest, whose model_validator returns 422 on all-None bodies so the verb layer never sees them. The MCP tool registry adds 'pr_update' so manifest- scoped do-servers can expose it to roles that opt in (next commit). |
||
|
Renn F
|
b5d3d13346 |
feat(git): add update_pr_for_task + PRUpdateRequest schema
Smoke-5 surfaced that be-dev-1 had no gateway-native way to fix a
PR's title/body or request a reviewer after open_pr; `gh pr edit`
is bash-shimmed and the dev correctly escalated rather than bypass
the guard. This adds the GitService primitive: PATCH /pulls/{n}
for title/body and POST /pulls/{n}/requested_reviewers for the
reviewer list, with NotFound + 422 mapped to typed GitError. The
PRUpdateRequest schema enforces 'at least one field' via a
model_validator so the route returns 422 before reaching the verb.
|
||
|
Renn F
|
1bd6eb3372 |
fix(gateway): journal task_id auto-injection works from blocked/paused
Smoke-5 root cause. Agents wrote 5 decisions / 8 reflections / 1 struggle
during the run — every single entry persisted with task_id=NULL. The C8
tracing gate then never saw them and PMs spiraled forever on
'missing: journal:decision' while their decisions sat orphaned.
Cause: ContentActions.note/say/dm/notify called
TaskService.get_active_task_for_agent for task_id auto-injection. That
helper filters to _DEV_ACTIVE_STATUSES = {claimed, in_progress,
verifying, awaiting_qa, awaiting_documentation}. BLOCKED, PAUSED, and
NEEDS_REVISION fall outside that set — so the moment an agent gets
stuck (which is exactly when they journal), auto-injection returns None
and the entry persists without task_id.
Fix:
- New TaskService.get_journal_context_task_for_agent — same shape as
get_active_task_for_agent but the status set
_JOURNAL_CONTEXT_STATUSES adds BLOCKED, PAUSED, NEEDS_REVISION.
- ContentActions.note/say/dm/notify use the new lookup.
- ContentActions.commit keeps the narrow get_active_task_for_agent —
can't commit from blocked, so the dev-active set is correct there.
Tests:
- tests/unit/services/test_journal_context_lookup.py — 5 tests pinning
the two queries: journal-context INCLUDES blocked/paused/needs_revision,
dev-active EXCLUDES them.
- Existing content-actions tests updated to stub the new method
alongside the old one.
This alone may be 70% of what was killing smoke runs end-to-end.
|
||
|
Renn F
|
7430c88f63 |
fix(gateway): open_session passes model schema, not API schema
Smoke run 4 crashed with AttributeError: 'SessionForTasksCreateRequest' object has no attribute 'config' when main-pm called open_session. The service _build_session_request reads req.config.max_message_count (model has nested config). The gateway was passing the API schema SessionForTasksCreateRequest (flat fields, no config attribute at all) — so `req.config` blew up with AttributeError, not the safer None. Fix: gateway now constructs SessionForTasksCreate (the model) with the enum-typed relationship_type, matching what the route at roboco/api/routes/sessions.py:230 does. Unknown relationship strings fall back to DISCUSSION. Two regression tests pin the contract: service receives the model; invalid relationship_type defaults to DISCUSSION. |
||
|
Renn F
|
4dfd1daf1e |
style: ruff format leftovers from Wave A-D sessions
Pure whitespace / line-wrap reformats accumulated when ruff format ran during earlier waves but weren't included in their commits. No semantic changes — collection literals reflowed, with-statement context managers regrouped via PEP 617 parens. |
||
|
Renn F
|
4f7dd7a336 |
docs(prompts): E4 clarify TodoWrite vs progress() distinction
TodoWrite is Anthropic's private session-local scratchpad — agents use it to track their own immediate next steps. It does NOT surface to the panel's Progress tab and is NOT a substitute for progress(task_id, message, percentage). Smoke run 3 didn't show this conflation yet, but Wave D's new progress() directive risks it. - base.md gets the canonical "TodoWrite vs progress()" callout - developer.md + documenter.md (the two roles with progress()) get inline reminders in their verb tables: "NOT TodoWrite" Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section E4. |
||
|
Renn F
|
f2551c0bdc |
fix(orchestrator): E3 disable builtin Claude.ai MCP connectors via --strict-mcp-config
Smoke run 3 showed agents loading builtin Anthropic connectors (mcp__claude_ai_Gmail__authenticate, Google Calendar, Notion, Drive) alongside our 5 roboco MCP servers. The connectors bloat the tool surface and give the LLM 'discover' targets it shouldn't have. The Claude Code CLI's --strict-mcp-config flag tells it to load ONLY the servers from --mcp-config, ignoring all builtin defaults. Added to _append_image_and_claude_args next to --mcp-config. Note: the existing --tools allowlist (Read,Write,Edit,Bash,Grep,Glob, Task,TodoWrite) only filters builtin tools, not MCP-prefixed ones — that's why the connectors slipped through. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section E3. |
||
|
Renn F
|
db4ac0c1c8 |
test(integration): E2 anti-regression — agents.role binds to 'agentrole' enum
Pin the SQLAlchemy enum-naming invariant: every column typed Role (aliased as AgentRole) MUST bind to the postgres enum named 'agentrole', not 'role'. Wave A5's smoke regression came from the inferred 'role' type colliding with migration 001's 'agentrole'. The fix already shipped (roboco/db/tables.py::_PG_ENUM_NAME_OVERRIDES); this test locks it in. Two assertions: agents.role.udt_name == agentrole; no stray 'role' enum type exists in pg_type. |
||
|
Renn F
|
41ef7f6b4e |
feat(gateway): C8 PM-decision gate windowed satisfaction
_check_pm_decision_required now requires the latest journal:decision within pm_decision_window_seconds (default 300). Older decisions no longer satisfy the gate. Adds JournalService.latest_decision_at. Future-tighten (out of scope): per-verb-group consumption tracking would need persistent state — Choreographer is per-request today. |
||
|
Renn F
|
89eacf028e |
feat(gateway): C7 synthetic checkpoint on auto-pause
Smoke run 3 showed agents auto-pausing on i_am_idle (correct behavior for non-terminal tasks) but capturing no checkpoint — panel's Checkpoints column stayed empty. Pre-gateway parity: the auto-pause path now writes a synthetic checkpoint summarizing state at pause-time so the panel reflects reality. Manual i_will_pause (G8a, deferred) will eventually let agents pass their own checkpoint_summary; for now this synthetic write covers the bare i_am_idle case which is what all current agents do. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section C7. |
||
|
Renn F
|
1ab9ccabd8 |
feat(events): C6 spawn auditor on escalation/block/cancel events
The auditor's role is 'silent observer' — read every channel and emit a reflect note when something notable happens. Smoke run 3 never spawned auditor because no event-subscription registered it. Added handler handle_auditor_spawn() wired to: - task.blocked (EventType.TASK_BLOCKED) - task.cancelled (EventType.TASK_CANCELLED) - task.awaiting_ceo_approval (EventType.TASK_AWAITING_CEO_APPROVAL) Routine events (task.claimed, task.started, task.created) deliberately do NOT trigger auditor — those are progress, not exceptions. The auditor's container is one-shot: i_am_idle() exits after logging its reflect note. Auditor spawn failures are swallowed into a WARNING log so they cannot block the underlying event's processing chain. The auditor is a silent observer — its absence must have no side effects on the lifecycle. |
||
|
Renn F
|
f38c15b966 |
feat(gateway): C5 write acceptance_criteria_status on i_am_done
Pre-gateway parity. evidence(task_id).acceptance_criteria_status was
always [] because the gateway's i_am_done gate validated each
criterion against the dev's journal:reflect but didn't persist the
per-criterion verdict. The panel + audit log couldn't show
per-criterion checkmarks.
Now the gate writes a list of {criterion, addressed, artifact_ref,
checked_at} entries to task.acceptance_criteria_status. The existing
matching logic surfaces which artifact (commit sha / reflect-note)
addressed each criterion; entries that aren't addressed get
addressed=False so the panel can flag them.
Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md
section C5.
|
||
|
Renn F
|
b53d8fe194 |
feat(gateway): C4 auto-create WorkSession on claim
Pre-gateway parity. Smoke run 3 showed task.work_session_id null on every task — the choreographer's claim/plan/start path didn't create the row that downstream subsystems (panel, PR tracking, merge chain) need to track agent-per-task git activity. Add TaskService.ensure_work_session(task_id, agent_id) as a public wrapper around the existing _create_work_session_if_needed logic. Role restriction lifted to None so both developers and PMs get a session (pre-gateway always created sessions for all claimants). Built-in re-entry guard prevents duplicate rows on re-claim. Wire the call into both _claim_plan_start_run and _resume_from_claimed immediately before _touch, so every successful in_progress transition (including the stuck-claimed recovery path) creates the row. Spec ref: Wave C task C4 (2026-05-12). |
||
|
Renn F
|
a47237416e |
feat(runtime): C3 tunable reaper threshold + heartbeat on every verb dispatch
Smoke run 3 showed agents reaped at the 3-min stale-claim window while they were actively retrying rejected verbs. Two causes: 1. The reaper threshold was hardcoded at 180s via claim_stale_seconds. LLM inference + retry loops routinely take longer than that between verb-successes. Added settings.stale_claim_reap_seconds (default 600s); override via ROBOCO_STALE_CLAIM_REAP_SECONDS env var. claim_stale_seconds (spawn-filter cutoff) is unchanged at 180s. 2. last_heartbeat_at only refreshed on verb SUCCESS. A verb stuck in a rejection loop (e.g. tracing_gap missing journal:decision) showed no heartbeat updates even though the agent was alive. Added a best-effort heartbeat refresh inside _emit_rejection so EVERY verb dispatch — success or rejection — counts as activity. Heartbeat approach: option (b) — touch inside _emit_rejection (single centralized rejection path). Requires no middleware layer, no HTTP body parsing, and no new files. The _touch guard for task_id=None means agent-level rejections (no task context) are a safe no-op. Net effect: agents stop being reaped mid-retry. Genuinely-stuck containers (no verb dispatch at all) still reap normally at 600s. Spec ref: Wave C Task C3. |
||
|
Renn F
|
eb9cd93e09 |
fix(workspace): C2 cache refresh fetch for 30s per workspace path
Smoke run 3 fired 'ensure_workspace: refresh fetch returned non-zero' 9 times per run because each evidence(task_id) call triggered ensure_workspace -> fetch. The workspace doesn't change in subseconds. Added a 30s TTL cache keyed by workspace path. ensure_workspace(force=True) bypasses the cache for callers that genuinely need a fresh fetch. Net effect: log noise drops from 9 entries to 1-2 per run; orchestrator spends less time waiting on redundant git fetches. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section C2. |
||
|
Renn F
|
cdb4a6edeb |
fix(mcp): C1 per-verb circuit breaker trips on incomplete_input too
Smoke run 3 showed Main PM hitting 7 incomplete_input rejections on the decision-note required-fields gate before finally succeeding. The per-verb breaker tracks repeated rejections of the same verb in a 60s window and returns circuit_open after the 3rd strike — but its classification set only included tracing_gap. incomplete_input was added in Wave 1 (pre-gateway parity for decision/reflect structured fields) and should have been added to the breaker at the same time. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section C1. |
||
|
Renn F
|
d73e86044b |
fix(gateway): B6 give_me_work returns pre-assigned pending tasks first
Smoke run 3 showed Main PM's first give_me_work() returning
{status: idle, next: 'no Main PM work'} even though c7935d2c was
pending and assigned to Main PM. The filter only walked
list_assigned_for_agent (ordered by priority/updated_at — pending
could rank behind in_progress rows) and the PM path fell through
to idle because the pre-assigned pending case was not checked first.
Pre-pended a list_pending_for_agent check in both give_me_work and
pm_give_me_work: tasks where assigned_to=agent_id AND status=pending
take priority over all other lookups. Added TaskService.list_pending_for_agent
for the query (ordered by sequence, priority, created_at).
Updated existing tests in test_choreographer_dev, test_choreographer_pm_extras,
and test_heartbeat_wired to set list_pending_for_agent.return_value=[]
where they were not testing the pre-assigned path.
Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md
section B6.
|
||
|
Renn F
|
85e20e6a2a |
fix(docker): B5 tighten bash-guard denial message to 2 lines
Smoke run 3 showed the bash-guard hook emitting 8+ lines on every blocked shell-git op — enumerating every alternative MCP verb across roboco-flow / roboco-do / roboco-git-readonly. That's repeated token spend on every refused retry; the LLM doesn't need the full alt-list inline, it has the role prompt + the MCP tool schema for that. Trimmed to 2 lines: denial reason + a one-line pointer to the role's State→Verb table. Test asserts <= 3 echo lines in any denial block. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section B5. |
||
|
Renn F
|
6550d69b75 |
fix(gateway): B4 decision/reflect remediate includes literal call example
Smoke run 3 showed Main PM taking 7 attempts to satisfy the decision-note required-fields contract — the remediate listed which fields were missing but didn't show what a fully-formed call looks like. The LLM pattern-matches examples better than field-list prose; each retry it dropped a different field. Added a literal note(scope='decision', ...) / note(scope='reflect', ...) call template to the rejection remediate so the agent sees the canonical shape with named-keyword args and example values. The missing-fields list stays — both pieces of information are useful, but the example is what actually drives convergence. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section B4. |
||
|
Renn F
|
ce92829385 |
fix: clear 55 pre-existing test failures uncovered after Wave A landed
Three classes of failure, all surfaced once Wave A's plan-required gate and the migration 013 went in. Per project standing rule: pre-existing errors are not a free pass — fix them. 1. Wave A1 ordering (32 lifecycle parity failures + 1 full-pipe test). _pm_sub_tasks_gate fired BEFORE _claim_plan_start_gate, so wrong-state PMs got `incomplete_input` (the gate's verdict) when the spec's lifecycle gate should have returned `invalid_state` first. Swapped: re-entry check → spec lifecycle gate → sub_tasks gate → claim_plan_run. Parity test now sees the spec's verdict as expected. 2. E2 enum naming (2 migration_013 failures + ripple). _str_enum in roboco/db/tables.py didn't pass name=… to SQLAlchemy Enum(...), so Base.metadata.create_all in test setup inferred `role` from the Python class `Role` while the alembic migrations declare `agentrole`. Tests saw two enums for the same class and hit `agentrole = role` operator errors. Fixed: default name to lower(class_name) (matches every migration), override `Role` → `agentrole`. One dict entry; no class-by-class registration needed. 3. _MockContentActions.note() signature drift. Wave 2 G4 added `structured` kwarg to ContentActions.note(). The integration mock at tests/integration/v2/test_full_pending_to_completed.py didn't accept the new kwarg → 1 test failed on the very first call from the v2 do/note route. Added `structured: object = None` and left it unused (the test asserts lifecycle, not journal rendering). Plus three ruff E501 line-length fixes in the test files I touched. Quality: ruff + mypy clean. pytest 6690 passed / 0 failed / 274 skipped. |
||
|
Renn F
|
f680db34c6 |
fix(gateway): B3 canonical say() return status — always 'posted'
Smoke run 3 showed inconsistent return strings — main-pm got status='sent', be-pm got status='posted' for the same verb. Confirmed say() already returns 'posted' at its sole success exit. Added test_say_status.py to pin the canonical past-tense pattern (note->'noted', say->'posted', notify_ack->'acked') and prevent regression. dm() and notify() retain 'sent' — different verbs, different semantics. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section B3. |
||
|
Renn F
|
eed4551497 |
feat(alembic): B2 drop unused pm_approvals Task column
Smoke run analysis initially flagged three Task fields as unused (pm_approvals, quick_context, proactive_context). A follow-up audit found quick_context (stores original_developer marker + doc notes + PR creator + escalation notes) and proactive_context (RAG injection) are actively used. Only pm_approvals is truly orphaned. Migration 014 drops pm_approvals; downgrade() recreates it if ever needed. The two false-positive fields stay untouched. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section B2 (re-scoped 2026-05-12). |
||
|
Renn F
|
7254ceee50 |
fix(docker): B1 update shell hooks to gateway verb names
Smoke run 3 showed stop-hook.sh complaining 'Denied: you stopped without calling a terminal tool' AFTER agents successfully called i_am_idle() — because the hook listed 9 pre-gateway verb names (roboco_agent_idle, roboco_task_substitute, etc.) that no longer exist. Same staleness in bash-guard-hook.sh. Both hooks now reference current gateway verbs only. stop-hook branches its suggestion by ROBOCO_AGENT_ROLE so devs see i_am_done/i_am_blocked, QAs see pass/fail, PMs see complete/escalate_up. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section B1. |
||
|
Renn F
|
fadc05966e |
feat(alembic): A5 migration 013 drops stray role postgres enum
Smoke run 2 (2026-05-11) produced 'UndefinedFunctionError: operator does not exist: agentrole = role' because postgres had two enums (role, agentrole) for the same Python class. Information_schema check confirms no column uses role; migration drops it. Upgrade() raises if that ever stops being true. Downgrade() recreates the enum with the foundation's Role values. Investigation of WHY a second enum got created is tracked in spec E2; this migration handles the symptom. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section A5. |
||
|
Renn F
|
d5a40086f4 |
fix(workspace): A4 downgrade expected refresh-fetch auth-fail to DEBUG
Smoke run 3 fired the same workspace.py warning ~9x per run: 'ensure_workspace: refresh fetch returned non-zero' stderr: 'fatal: could not read Username for https://github.com' This is EXPECTED behavior, not a bug. The docstring on _fetch_origin_best_effort explains that credentials are deliberately scrubbed from .git/config after the initial clone (part of the secret- exfiltration mitigation) and refresh fetches are best-effort. For private repos the auth-fail is the documented outcome. The original A4 spec proposed re-injecting the PAT -- that would have violated _assert_no_pat_leak and the URL-scrub mitigation. Re-scoped to: silence the known-benign signature at DEBUG, keep WARNING for genuine failures (network errors, broken remotes, repo-not-found). No behavior change. No security boundary touched. Just log level. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md A4 (re-scoped 2026-05-12 after investigation showed the original spec proposed reintroducing a documented security regression). |
||
|
Renn F
|
10be97fd5a |
refactor(orchestrator): A2+A3 follow-ups — extract workspace-path helpers
Fixes 2 important + 1 minor issue from the code-quality review of
|
||
|
Renn F
|
5adb4ff272 |
fix(orchestrator): A2+A3 set agent container cwd to workspace path
Smoke run 3 surfaced two bugs that share a root cause:
- Edit(/app/README.md) → 'Edit exists but is not enabled in this context'
- commit(files=['/app/README.md']) → 'outside repository at <workspace>'
Both happened because the container's WORKDIR is /app (roboco package
source) while the agent's task workspace is bind-mounted at
/data/workspaces/<project>/<team>/<agent>/. The Dev role's
Edit/Write permission allowlist scopes to the workspace, so any Edit
call from /app fails the path match.
Adds '-w {workspace_path}' to the docker run command so the container
starts with cwd = task workspace. Edit(README.md) and git add README.md
now resolve inside the workspace clone.
Mirrors _get_role_permissions path selection exactly:
- developer / product_owner / head_marketing: per-agent workspace
- documenter: cell workspace (matches its Write/Edit allowlist)
- qa / cell_pm / main_pm / auditor: omit -w, fall back to /app
Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md
sections A2 + A3 (re-scoped per investigation 2026-05-12).
|
||
|
Renn F
|
cfb7424c80 |
fix(gateway): A1 review-fixes — re-entry ordering, gate unit-coverage, approach check
Three fixes from the code-quality review of
|
||
|
Renn F
|
a1009c05e8 |
feat(gateway): A1 plan-required-at-claim gate
i_will_plan now requires approach (min_length=20) at the schema and non-empty sub_tasks at the gateway when the caller is a PM role. Restores pre-gateway parity for _validate_claimed_start — agents could not transition claimed -> in_progress without filling the rich plan. Smoke run 3 (2026-05-11) showed PMs calling i_will_plan with just plan='paragraph' and the gateway accepting it; Plan tab stayed empty because no agent filled approach/sub_tasks/risks/open_questions. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md section A1. |
||
|
Renn F
|
62d1084a0c |
fix(gateway): notify_list/get/ack call NotificationDeliveryService (not Service)
Wave 1 wired notify_list/get/ack into ContentActions but pointed them at `self.notifications` (which is NotificationService — sender side, with send_blocker_notification / send_qa_ready_notification / etc.). The read methods (list_for_agent, get_for_recipient_and_mark_read, acknowledge) live on `NotificationDeliveryService` instead. Smoke run 2026-05-11 surfaced this immediately: AttributeError: 'NotificationService' object has no attribute 'list_for_agent' Fixes: - roboco/api/deps.py — import NotificationDeliveryService and wire it in as a new ContentActionsDeps field `notification_delivery`. - roboco/services/gateway/content_actions.py — add notification_delivery to ContentActionsDeps (Optional with `None` default for back-compat with any tests that don't supply it). Point notify_list, notify_get, notify_ack at self._deps.notification_delivery. - tests/unit/gateway/test_content_actions.py — _make_deps adds a default AsyncMock for notification_delivery so existing tests continue to pass. Quality: ruff + mypy clean. 505 unit tests pass. |
||
|
Renn F
|
dc9c49e1e4 |
feat(gateway): G8 part b — typed blocker_type + what_needed on i_am_blocked
Pre-gateway parity (G8 part b of the 2026-05-11 design). The pre-gateway TaskBlockInput at 254cc93:roboco/mcp/schemas/__init__.py required blocker_type (external|internal|question|dependency) and what_needed so PMs could triage their inbox by class. Current i_am_blocked dropped both fields — every blocked task looked the same to the PM. Now i_am_blocked accepts both as optional kwargs: - Back-compat: callers that omit them still work (blocker_type defaults to None → rendered as flat reason in the struggle entry). - New: when supplied, the struggle journal entry body is structured markdown (## Blocker Type / ## What Needed sections) so the panel's journal view renders named blocks instead of one flat sentence. Validator on blocker_type enforces the enum at the Pydantic boundary with a clear "must be one of: ..." error if the agent invents a value (same pattern as the Wave 3 G7 validators). G8 part a — typed `pause(checkpoint_summary, remaining_work)` — defers. That gap needs a new IntentSpec in foundation/policy/lifecycle.py (currently pause is an ActionSpec only; agents auto-pause via i_am_idle) plus checkpoint wiring through TaskService.add_checkpoint. Material work, deferred until after the user has deployed and verified G7 + G8b lands cleanly. Wired: - roboco/api/schemas/v2/flow.py — IAmBlockedRequest gains optional blocker_type + what_needed; @field_validator enforces the enum - roboco/api/routes/v2/flow_dev.py — passes the new fields through - roboco/services/gateway/choreographer/_impl.py — i_am_blocked signature + structured struggle-entry rendering - roboco/mcp/flow_server.py — typed wrapper with the kwargs - agents/prompts/roles/developer.md — updated verb table - tests/unit/mcp_servers/test_flow_server.py — updated to expect the new optional kwargs as None when omitted Quality: ruff + mypy clean. 505 tests pass. |
||
|
Renn F
|
bd52e3d0c3 |
fix(schemas): pre-gateway-style cross-field validators on DelegateRequest
Pre-gateway parity for G7 of the 2026-05-11 design. The pre-gateway
TaskCreateInput at 254cc93:roboco/mcp/schemas/__init__.py:210-235 had
@field_validator hooks that caught the most common LLM-vs-schema
confusions with helpful "did you mean X?" hints. Those validators were
lost in the gateway refactor.
Three validators added to DelegateRequest:
- estimated_complexity: rejects ints (some agents send 1/2/3 thinking
it's a priority), enforces enum {low|medium|high|critical}. Hint
steers them to drop priority (which isn't a delegate parameter).
- nature: rejects invented values like the 2026-05-11 'standard'
regression. Enum is {technical|non_technical}. Hint explicitly cites
the regression so the LLM knows why this is enforced.
- task_type: rejects invented task_type values. Enum is {code,
documentation, research, planning, design, administrative}.
Fail-fast at the Pydantic boundary returns a 422 with the structured
hint inline, so the agent loops a single retry instead of leaking a
TaskCompletenessError up the stack.
Existing tests in tests/unit/api/routes/v2/test_flow_*.py used
"nature": "feature" — a value that the gateway's TaskNature enum
never accepted, so it would have been rejected at completeness check
anyway. Updated both to "technical".
Spec ref: docs/superpowers/specs/2026-05-11-pre-gateway-parity-design.md
|
||
|
Renn F
|
72e01a7f13 |
feat(gateway): Wave 2 pre-gateway parity — structured note, sub_tasks, channels
Three Wave 2 gaps from the 2026-05-11 pre-gateway parity design:
G4 — note() decision/reflect now require structured fields at the gateway
(pre-gateway `Field(...)` parity). Returns `incomplete_input` envelope
with field-by-field hints when any required field is missing.
- decision: context (str), options (list[{name,pros,cons}] min len 2),
chosen (str), rationale (str). `consequences` and `next_steps` are
now list[str] (was str). Renderer emits each option as a "### Name
+ Pros / Cons" block instead of a bullet — matches the pre-gateway
DecisionOption sub-shape exposed in `roboco/mcp/schemas/__init__.py`
at `254cc93`.
- reflect: what_done, what_learned, what_struggled (each non-empty
str). next_steps stays optional.
- Bumped tests/unit/gateway/test_content_actions.py with explicit
pass-with-N-options coverage (≥2 floor; 3-option case green).
G5 — i_will_plan now persists sub_tasks alongside approach / risks /
open_questions / technical_considerations. The Plan tab's Sub-Tasks
section was empty because the verb didn't accept the field. Choreographer
server-assigns id + order to each sub_task (pre-gateway build_plan_data
parity) and normalizes every list entry to the EXACT shape
`panel/src/types/index.ts::TaskPlan` consumes:
- SubTask: {id, title, description, completed:false, order,
estimated_hours:null, notes:null}
- Risk: {description, mitigation, severity:null} — accepts the
{risk, mitigation} pre-gateway shape too
- OpenQuestion: {question, answer:null, answered_by:null,
answered_at:null} — accepts a bare string fallback
The normalization lives in three small module-level helpers
(_normalize_sub_task / _normalize_risk / _normalize_open_question)
called from _build_panel_shaped_plan, keeping i_will_plan's branch
count under PLR0912.
G6 — new `channels()` verb returns the agent's readable + writable
channel slugs from foundation.policy.communications. Stops invented
slugs ("backend-dev", "backend") that we kept seeing in smoke runs.
Added to every role's manifest including auditor (read-only access).
Wired through:
- roboco/api/schemas/v2/do.py — list-typed consequences/next_steps,
dict-typed options, ChannelsRequest
- roboco/api/schemas/v2/flow.py — IWillPlanRequest.sub_tasks
- roboco/api/routes/v2/do.py — /channels endpoint
- roboco/api/routes/v2/flow_*.py — pass sub_tasks through
- roboco/services/gateway/content_actions.py — channels() method;
_check_scope_required_fields enforces decision/reflect structure;
_render_option_block emits per-option markdown blocks
- roboco/services/gateway/choreographer/_impl.py — _build_panel_shaped_plan
helper used by i_will_plan
- roboco/services/gateway/role_config.py — _CHANNEL_DISCOVERY tuple
on every role
- roboco/mcp/do_server.py — channels() tool + note() signature with
options as list[dict[str,str]]
- roboco/mcp/flow_server.py — i_will_plan signature with sub_tasks
Frontend: no code change. panel/src/types/index.ts already declares
the exact shape we now write; panel/src/components/tasks/task-detail/
{tab-plan,tab-progress,tab-sessions,tab-notes}.tsx already reads it.
The empty panels we observed were a backend write-side problem, not
a frontend read-side problem — Wave 1 + Wave 2 close it.
Quality: ruff + mypy clean. 505 unit tests pass (added 2 new tests on
decision-scope requirements, updated 3 existing tests to fit the
pre-gateway-parity contract).
Spec ref: docs/superpowers/specs/2026-05-11-pre-gateway-parity-design.md
|
||
|
|
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
|
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.
|