Smoke 2026-05-04 captured the cycle the prior 63d0adf fix didn't close:
- Spawn 1: i_will_plan succeeds, task pending → claimed → in_progress.
- Agent goes idle (LLM thinking, container exits, respawned).
- Spawn 2: i_will_plan called again. _i_will_plan_preflight rejects
'task in in_progress, expected pending'. Agent has no recovery path.
- Heartbeat eventually goes stale, reaper drops claim back to pending,
spawn 3 fires, loop repeats indefinitely.
Fix: when a respawned PM/dev re-enters the verb on a task they already
own in claimed/in_progress, return OK with current state and refresh
the heartbeat instead of rejecting. The verb is now genuinely
idempotent for the caller, which matches what 'I will plan' should
mean — record intent + advance state, regardless of how many times
the agent says it. Different-caller contention still rejects.
Refactored i_will_work_on's pending branch into _i_will_work_on_pending
helper to satisfy PLR0911 after the new branch raised return count.
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.
Smoke 2026-05-03 saw main-pm reach the git layer with a 'commit' call,
trying to author 'fix(gateway): allow claimed status in i_will_plan
preflight'. That should never have been possible — main_pm/cell_pm/
board/auditor/qa manifests all exclude commit. Reaching the verb body
means either the MCP manifest filter mis-routed, or the agent hit the
v2 do.py route directly.
Mirror Task 16 notify pattern: server-side role check in the verb body
rejects with not_authorized + 'PMs delegate, do not commit' remediate.
Defense-in-depth — manifest is still the primary gate.
pr_merge falls back to task.assigned_to for workspace resolution, but
that field is None at merge time (submit_qa/pass_qa cleared it during
prior transitions). When project.workspace_path is unset, the resolver
raises ValidationError 'no workspace configured and no agent_id
provided' — surfaces as 500 from cell_pm_complete.
Add actor_agent_id parameter (the PM doing the merge) and use it as
the primary workspace owner. Falls back to task.assigned_to, then
created_by, before raising. cell_pm_complete now threads pm_agent_id
through.
Real-DB integration test: pending pre-assigned task → claim() →
status=CLAIMED AND last_heartbeat_at populated. Without the seed
(reverted in unit testing), the reaper would interpret NULL as
stale and reap the freshly-claimed task on the next dispatch tick.
Smoke run agents picked invalid enums ('development', 'small') because
spawn prompts used 'code|documentation|...' format with literal '...'
that LLMs interpret as 'fill in something creative'. Replace with
explicit '<one of "a" / "b" / ...>' enumerations matching the
TaskType + Complexity enum members, plus an explicit reminder that
the gateway rejects invented values.
Smoke 2026-05-03: reaper fired ~12 times/sec against a freshly-claimed
task because _finalize_claim never seeded last_heartbeat_at. Reaper's
'NULL means stale' rule then reaped the claim within ~250ms of the
claim landing, agent re-claimed via i_will_plan, reaper reaped again.
Set last_heartbeat_at = claimed_at in _finalize_claim so the freshly-
claimed task carries an authentic heartbeat from second zero. Reaper
logic stays untouched.
Pin the smoke-2026-05-03 bugs that motivated 63d0adf:
- pre-assigned-and-pending: claim must still fire (was skipped when
task.assigned_to already matched pm_agent_id)
- start-returns-None: must surface invalid_state (was silently returning
Envelope.ok with fabricated status='in_progress')
Smoke run revealed CEO-pre-assigned root tasks stay stuck in pending
because both verbs skipped claim() when task.assigned_to already
matched the caller. Claim is what transitions pending → claimed; without
it, start() refused the claimed → in_progress transition and silently
returned None. i_will_plan then returned an OK envelope with a
fabricated 'in_progress' status string while the DB stayed pending,
causing the next delegate() call to correctly reject with
PARENT_NOT_CLAIMED. Agent looped on delegate retries, never figuring
out claim was the missing step.
Fix:
- Drive claim() on status == pending (idempotent for same assignee),
not on assigned_to mismatch.
- If start() returns None, surface invalid_state envelope instead of
fabricating success.
Audit followups before pushing the gateway-restoration batch:
- Add notify() row to cell_pm.md, main_pm.md, board.md verb tables.
Manifests + routes + MCP + tests all wired in 3a2498a but agents
had no prompt-level cue.
- Replace # type: ignore[attr-defined] in test_pm_respawn_reset.py
with cast('Any', orch) — matches project's no-suppress standard.
Task 22 added a runtime ValueError when entry_id is None, but the
dataclass still typed it Optional. Contract-vs-runtime split — callers
get no IDE/mypy hint about the required field. Tighten the type to
UUID (required) and remove the misleading "can be None for system
events" docstring note. Lifecycle events already use their synthetic
uuid5 path, so no real caller is broken.
Tiny race: dispatcher decided to spawn for closure between agent's
heartbeat and idle-pause. Spawn would land against an already-paused
parent. Gate spawn on (status != PAUSED OR last_heartbeat older than
cutoff).
State existed in the lifecycle table and the enum but no verb, route,
or service path ever set it. Removing dead state. If we need
problem-task isolation later we'll add it explicitly with a verb.
Two PMs completing different subtasks of the same parent could race
on gh API merge calls. GitHub returns 409 to one but local DB write
ordering wasn't guaranteed. Take row-level lock on parent task before
merge; retry once on 409.
Pre-gateway PMs/Board sent formal notifications (require ack); gateway
had say/dm only. Now PMs and Board can issue ack-required notifications
via NotificationService through the standard envelope path.
I1: _fetch_origin_best_effort runs as root and writes new pack files
+ ref updates that land root-owned, undoing _ensure_agent_owned that
ran before. Subsequent spawns hit Permission denied. Mirror the
fetch_branch_for_inspection pattern: re-run _ensure_agent_owned AFTER
the fetch.
I2: separate workspace_refresh_fetch_timeout_seconds (default 60s)
from workspace_clone_timeout (300s). Refresh transfers small deltas;
300s of blocking on every spawn against a hung remote is operationally
bad. 60s is enough for any sane refresh.
ensure_workspace short-circuited when the clone existed, so a
respawned PM/Doc could be reviewing arbitrarily stale diffs. Add
a best-effort 'git fetch origin' on every entry; checkout
unchanged.
X-Correlation-ID was bound to structlog but lost on the MCP→API hop.
MCP shims now forward it; Envelope carries it back to the agent;
audit_log row records it for forensic joins.
Task 13's has_recent_tracing_gap query used .astext on a generic JSON
column, which raises AttributeError at runtime. The choreographer's
exception handler swallowed it, leaving the strike-count reset
permanently inert in production. Migrate the column to JSONB (which
supports .astext + GIN indexing for future audit queries), update the
ORM, and add a real-DB integration test that would have caught this.
_pm_respawn_should_gate counted PARENT_NOT_CLAIMED rejections as
no-progress and killed PMs after 3 strikes — even when the new prompts
told them to call i_will_plan first. Reset counter when last response
was a tracing_gap (rule-following retry, not stuck).
ROBOCO_GATEWAY_ENABLED defaulted to False, leaving trigger_filter's
spawn cooldown / role-rate logic dormant. Flip to true and add the
gateway_triggers table migration if missing. Without this, respawn
rate has no server-side limit besides _pm_respawn_should_gate.
When a PM's i_will_plan and a child dev's spawn fire in the same tick,
the dev sometimes saw branch_name=None because the PM's transaction
hadn't committed yet. Auto-block fired and the dev sat blocked until
the next 30s tick. Add 3x250ms retries when parent is mid-claim.
I2: resume_for_agent did its own _validate_and_set_status call,
skipping resume()'s structlog 'Task resumed' event and the fire-and-
forget RAG lifecycle-event indexing. Gateway-driven resumes were
invisible to logs and the RAG corpus. Delegate to resume() after
the gateway-specific ownership pre-checks (mirrors pause_for_agent's
pattern).
I1: documented why Choreographer.unclaim does NOT call _touch on
its OK path - assigned_to is cleared, no claimant heartbeat to
refresh. Prevents future readers from 'fixing' the asymmetry.
i_am_idle auto-pauses owned in_progress tasks; lifecycle table allows
paused -> in_progress; no verb implemented it. Adds resume so an agent
respawned for a paused task can continue. Routes through
_validate_and_set_status mirroring the unclaim pattern (Task 9 fix).
unclaim_for_agent did direct attribute assignment, bypassing the
single point of truth for status transitions. Choreographer's pre-check
made the runtime correct today, but a future change to VALID_TRANSITIONS
or ROLE_RESTRICTED_TRANSITIONS would silently miss this path. Route
through _validate_and_set_status; assigned_to is still cleared
explicitly as the unclaim's specific side effect.
Choreographer remediate strings already pointed agents at unclaim;
the verb didn't exist. Now it does — claimed/in_progress -> pending,
clears assigned_to, available to dev/qa/doc/cell_pm/main_pm.
ContentActions.commit was stripping user-supplied prefixes but never
re-adding the canonical one. Dev prompt promised auto-prefix; code
delivered nothing. Now every gateway commit lands with [task-id-short].
Format choice: simple [task_id[:8]] (8-char), matching the dev prompt
("Auto-prefixes [task-id]") and CLAUDE.md's documented commit format.
The legacy templates/git/commit.py uses the richer
[root_short:task_short] for the commit_for_task API path with full
traceability metadata; the gateway commit path is intentionally simpler
and stays aligned with the prompt-level promise.
Previous regex used a greedy [^|]* and required at least one / before
the host, so 'curl roboco-orchestrator:8000/api' (no scheme, no slash)
slipped through. Split into two simpler checks: (a) line starts with
curl/wget/http/https/httpie, AND (b) line contains a forbidden host.
Probe-verified: scheme-ful, scheme-less, and protocol-relative forms
all denied; external URLs still allowed; GitHub-specific deny still
fires first.
Prompts told agents internal API calls were denied; the guard only
denied GitHub. Combined with task 4 (X-Agent-Role enforcement) this
closes the manifest-bypass loophole.
Choreographer takes an audit dep but never used it. Now every rejection
Envelope (invalid_state, not_authorized, tracing_gap, not_found) writes a
gateway.rejected row with verb + reason + missing fields. Forensic
signal for stuck flows; no behaviour change for happy path.
Adds AuditService.log_event() — generic free-form event_type write so
the gateway doesn't have to extend AuditEventType for every new surface.
Audit writes are best-effort: a failure logs a warning but never
propagates, since the agent's response Envelope is the contract.
I1: post_to_channel raised no error if get_agent_slug returned None
(unknown/deleted agent), and send_message's 'if agent_slug' skipped
the validate_channel_access call. Same fail-open class the prior
commit was fixing, just narrower window. Now post_to_channel raises
ChannelAccessDeniedError directly when the slug lookup fails — say()
already converts that to a clean not_authorized Envelope.
I2: 'writable channels for your role' wording was misleading because
get_agent_channels resolves by slug, not role. Replaced with
'channels you may write to' for clarity.
post_to_channel was calling send_message without agent_slug, which
disabled the validate_channel_access check. Forward the slug; convert
ChannelAccessDeniedError to a friendly not_authorized Envelope with
the agent's writable-channel list (pre-gateway behaviour).
Route layer now rejects 403 if the role doesn't match the router's
allowed set. Choreographer still re-checks role per verb where needed,
but defense in depth means a future verb that forgets the role check
doesn't leak. Auditor router also gated.
Gate E made i_am_done strict (requires pr_number set), but the only
verb that opened PRs was i_am_done_with_catchup which lives off the
dev manifest. Devs hit NO_PR with no escape. Adds submit_for_qa as
the explicit push+PR step, leaving i_am_done to do the strict submit.
i_will_work_on, i_have_committed, i_am_done, i_am_blocked, i_will_plan,
and pm_give_me_work now call task.heartbeat() so the reaper has fresh
data. Closes the loop opened by tasks 2a + 2b.
I1: claim_heartbeat_ttl_seconds (300s) overlapped semantically with the
pre-existing claim_stale_seconds (180s). Between 180-300s of silence,
trigger_filter queued duplicate spawns while the reaper hadn't yet
released the claim — exactly the dispatcher churn the reaper was
supposed to close. Collapse to one field (claim_stale_seconds, 180s);
reaper now consumes the same setting trigger_filter uses, so both
agree on 'stale' on the same tick and the reaper runs first.
I2: _task_svc injection slot on AgentOrchestrator.__init__ was
production-dead (always None) and only used by __new__-based test
instances. Drop the __init__ slot + the production branch in
_reap_stale_claims that read it. Tests still pre-bind on __new__
instances; the attribute exists per-instance, not per-class.
Dispatch loop now releases tasks whose holder has gone silent past
ROBOCO_CLAIM_HEARTBEAT_TTL_SECONDS (default 300s). Closes the
'dead container squats task forever' failure mode that the schema
hinted at but no code enforced.
Foundation for stale-claim recovery. Schema column existed since
migration 006; no writer until now. Idempotent UPDATE — no select
roundtrip — so callable from any hot verb without extra DB cost.
Service signatures are (task_id, agent_id, ...) but choreographer was
calling (agent_id, task_id). Production claim path silently returned
None; unit tests pinned the buggy order so the bug was invisible. Swap
all 7 call sites and update test assertions. Add a regression pin
that locks in the correct order.
cell_pm_complete, main_pm_complete, and submit_up already had subtask-
terminality gates inherited from the pre-gateway closure check at
roboco/services/task.py. This commit verifies the gate is preserved
and improves the remediation hint to actually NAME the non-terminal
subtasks instead of telling the PM to "call triage()".
The improvement uses a new private helper
``Choreographer._non_terminal_subtask_ids`` that queries get_subtasks
and filters to non-terminal statuses, returning a comma-separated
list of "<id> (<status>)" pairs. The PM now sees exactly which
subtasks are blocking the parent's completion.
Pre-gateway reference: roboco/services/task.py closure check
(documented in PRE_GATEWAY_LIFECYCLE.md §4.3).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
i_am_done now strictly enforces the four pre-gateway field-level gates
restored from roboco/api/routes/tasks.py:903-940 at commit 254cc93:
- NOT_SELF_VERIFIED: task.self_verified must be true.
- NO_COMMITS: task.commits must be non-empty.
- NO_PR: task.pr_number must be set.
- NO_PROGRESS: task.progress_updates must have at least one entry.
Each missing field surfaces as a tracing_gap with the matching pre-
gateway error code in the missing list, and a remediation hint that
tells the dev exactly what to do.
The previous silent-catch-up behavior is preserved as a separate
opt-in verb i_am_done_with_catchup. Existing tests that asserted the
catch-up behavior have been migrated to the new verb.
This fixes the failure mode where a dev could call i_am_done with no
commits and the gateway would silently try to push nothing, open an
empty PR, etc. — now the dev sees an explicit error.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a caller passes an explicit task_id to commit / note / say / dm /
evidence, ContentActions now verifies task.assigned_to == caller_agent_id
before allowing the side effect. Auto-fill from get_active_task_for_agent
is implicitly self-owned and does not need a re-check.
evidence() additionally allows assigned_to=None (post-handoff transient
state) so QA / documenter can inspect tasks between reassignments.
Pre-gateway, agents could not even see tasks they didn't own because the
MCP handlers resolved task from session context. The gateway exposes
task_id parameters across multiple verbs, so the explicit ownership
gate is required.
Exception: say() and dm() with NO task_id are exempt — used for channel
announcements and off-task A2A. The strict guard only applies when the
agent supplies a task_id parameter.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renn F