mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
197b1576c36a9b03662e8f0f4c370b16fef0ba66
98
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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
|
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
|
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
|
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
|
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.
|
||
|
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
|
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. |