mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(tg): cockpit data correctness — real GLM pricing, display timezone, agent activity tracking (#666)
* fix(tg): cockpit data correctness — real GLM pricing, display timezone, agent activity tracking
Three root causes behind the Mini App/bot showing wrong numbers:
Pricing: glm-5.2 gets a grounded per-token rate (z.ai published pricing,
$1.40/$4.40/$0.26 per 1M, source+date in the table comment) so a GLM
fleet day stops reporting $0.00 for half a million tokens; ungrounded
Ollama-Cloud models render "subscription (untracked)" instead of a bare
zero (is_ollama_cloud_model, consumed directly by the cockpit). Side
effect, intended and documented: honestly-priced GLM now trips the
downgrade-only comparator for new qa/documenter complexity pins.
Display timezone: the cockpit bucketed days in UTC for a GMT+2 operator.
New pure foundation module display_time (resolve_zone/local_date/
trailing_dates/day_bounds_utc, DST-correct with tests for the 23h/25h
days) + ROBOCO_DISPLAY_TIMEZONE (IANA-validated, default UTC); the
cockpit's spend/velocity series bucket raw session/completion rows by
the display zone. The UTC-keyed rollup table and the main dashboard are
deliberately untouched.
Agent activity: AgentTable.status was never set to ACTIVE and
current_task_id was never written anywhere — "active: 0, working: []"
was structurally permanent. Every claim path now marks the claimant
ACTIVE with rollback symmetry (_finalize_claim for dev/PM claims,
_qa_or_doc_claim for QA/doc/PR-gate claims, pr_review_claim for external
review) and every release path clears it (pass/fail QA, pr_pass/pr_fail,
complete_review, advance-to-PM-review, reaper unclaim, voluntary
unclaim, reassign retarget, pool divert, admin transitions, unblock
restore-to-in-progress). The bot's /status shares the cockpit's fleet
derivation so the two surfaces can't disagree. Known ceiling, commented:
one current_task_id column shows a multi-root coordinator PM's most
recent claim only.
Drill: sonnet develop -> sonnet adversarial (refuted the original
chokepoint coverage claim; QA/doc/reviewer paths were unwired) ->
correction round (wired them all + restored a dropped assertion, deleted
a dead helper and the dead subscription_billed field) -> review.
* fix(db): post_update on AgentTable.current_task breaks the flush cycle
agents.current_task_id and tasks.assigned_to reference each other, so a
flush touching both rows — every claim now marks its agent ACTIVE — is
an instance-level circular dependency SQLAlchemy cannot topologically
sort. The e2e smoke's full verb paths (12 tests) hit it; the unit and
integration suites never flush both dirty rows with relationships
loaded. post_update emits the FK as a second UPDATE, the canonical fix
for mutually-referencing rows.
* fix(budgets): enforce only explicitly-set budgets — no per-TaskType defaults
The per-TaskType default cap table blocked an unbudgeted coordination
root one opus planning turn in ($1.50 PLANNING default vs. real
coordination spend) — a false positive by design the moment the fleet
runs a priced model. Budgets are now explicit-input only:
effective_task_budget_usd returns None for an unset budget_usd, the
budget sweep skips enforcement (and never prices spend) on None, and
the unblock re-check passes on None so clearing the budget field is
itself a valid resolution. The project monthly cap stays as the
explicit-input fleet-wide backstop. Panel copy tells the truth
("No cap" placeholder; empty = uncapped), and the TaskType default
table plus its resolver are deleted.
---------
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -567,12 +567,15 @@ async def test_wire_sibling_collision_dag_notifies_only_for_new_edges() -> None:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mark_agent_idle_sets_status_idle() -> None:
|
||||
agent = MagicMock(id=uuid4(), status=AgentStatus.ACTIVE)
|
||||
agent = MagicMock(id=uuid4(), status=AgentStatus.ACTIVE, current_task_id=uuid4())
|
||||
result = MagicMock()
|
||||
result.scalar_one_or_none.return_value = agent
|
||||
svc = _service_with(result)
|
||||
await svc.mark_agent_idle(agent.id)
|
||||
assert agent.status == AgentStatus.IDLE
|
||||
# Otherwise the agent keeps reporting its last task as "currently
|
||||
# working" forever — nothing else ever clears this column.
|
||||
assert agent.current_task_id is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -587,6 +590,10 @@ async def test_qa_claim_sets_assignment_on_awaiting_qa() -> None:
|
||||
result.scalar_one_or_none.return_value = task
|
||||
session = MagicMock(flush=AsyncMock())
|
||||
session.execute = AsyncMock(return_value=result)
|
||||
# _qa_or_doc_claim now looks up the claiming agent via session.get to
|
||||
# flip its ACTIVE marker — default to "no matching row" for a test that
|
||||
# doesn't care about that side effect.
|
||||
session.get = AsyncMock(return_value=None)
|
||||
svc = TaskService(session)
|
||||
qa_id = uuid4()
|
||||
out = await svc.qa_claim(qa_id, task.id)
|
||||
@@ -617,6 +624,7 @@ async def test_doc_claim_sets_assignment_on_awaiting_documentation() -> None:
|
||||
result.scalar_one_or_none.return_value = task
|
||||
session = MagicMock(flush=AsyncMock())
|
||||
session.execute = AsyncMock(return_value=result)
|
||||
session.get = AsyncMock(return_value=None)
|
||||
svc = TaskService(session)
|
||||
doc_id = uuid4()
|
||||
out = await svc.doc_claim(doc_id, task.id)
|
||||
@@ -677,7 +685,10 @@ async def test_unblock_with_restore_returns_to_pre_block_state() -> None:
|
||||
blocker_resolver_type=BlockerResolverType.AGENT,
|
||||
blocker_raised_by=pre_assignee,
|
||||
)
|
||||
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||
# An IN_PROGRESS restore now looks up the restored owner via session.get
|
||||
# to flip its ACTIVE marker — default to "no matching row".
|
||||
session = MagicMock(flush=AsyncMock(), get=AsyncMock(return_value=None))
|
||||
svc = TaskService(session)
|
||||
_bind(svc, "get", AsyncMock(return_value=task))
|
||||
out = await svc.unblock_with_restore(uuid4(), task.id, restore=True)
|
||||
assert out is task
|
||||
@@ -721,13 +732,17 @@ async def test_unblock_no_branch_returns_to_pending() -> None:
|
||||
task = _build_task(
|
||||
status=TaskStatus.BLOCKED, branch_name=None, blocker_raised_by=raiser
|
||||
)
|
||||
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||
session = MagicMock(flush=AsyncMock())
|
||||
svc = TaskService(session)
|
||||
_bind(svc, "get", AsyncMock(return_value=task))
|
||||
_bind(svc, "_index_lifecycle_event_background", AsyncMock())
|
||||
out = await svc.unblock(task.id)
|
||||
assert out is task
|
||||
assert task.status == TaskStatus.PENDING
|
||||
assert task.assigned_to == raiser
|
||||
# A PENDING restore is NOT a resume — the owner isn't marked active here;
|
||||
# a fresh claim() is what actually resumes it, so no agent lookup runs.
|
||||
session.get.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -747,7 +762,10 @@ async def test_admin_set_status_out_of_blocked_restores_pre_block_owner() -> Non
|
||||
pre_block_state="in_progress",
|
||||
pre_block_assignee=dev,
|
||||
)
|
||||
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||
# An IN_PROGRESS restore now looks up the restored owner (dev) via
|
||||
# session.get to flip its ACTIVE marker.
|
||||
session = MagicMock(flush=AsyncMock(), get=AsyncMock(return_value=None))
|
||||
svc = TaskService(session)
|
||||
_bind(svc, "get", AsyncMock(return_value=task))
|
||||
out = await svc.admin_set_status(task.id, TaskStatus.IN_PROGRESS)
|
||||
assert out is task
|
||||
@@ -791,7 +809,10 @@ async def test_admin_set_status_into_review_queue_clears_active_claimant() -> No
|
||||
claimed_by=dev,
|
||||
active_claimant_id=dev,
|
||||
)
|
||||
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||
# Clearing the stale claimant now looks it up via session.get to release
|
||||
# its ACTIVE marker too.
|
||||
session = MagicMock(flush=AsyncMock(), get=AsyncMock(return_value=None))
|
||||
svc = TaskService(session)
|
||||
_bind(svc, "get", AsyncMock(return_value=task))
|
||||
out = await svc.admin_set_status(task.id, TaskStatus.AWAITING_QA)
|
||||
assert out is task
|
||||
@@ -943,7 +964,10 @@ async def test_admin_set_status_blocked_to_review_state_clears_claim() -> None:
|
||||
pre_block_state="awaiting_pm_review",
|
||||
pre_block_assignee=pm,
|
||||
)
|
||||
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||
# Clearing the stale claim now looks it up via session.get to release
|
||||
# its ACTIVE marker too.
|
||||
session = MagicMock(flush=AsyncMock(), get=AsyncMock(return_value=None))
|
||||
svc = TaskService(session)
|
||||
_bind(svc, "get", AsyncMock(return_value=task))
|
||||
out = await svc.admin_set_status(task.id, TaskStatus.AWAITING_PM_REVIEW)
|
||||
assert out is task
|
||||
@@ -970,7 +994,10 @@ async def test_admin_set_status_blocked_to_needs_revision_clears_claim() -> None
|
||||
pre_block_state="awaiting_pm_review",
|
||||
pre_block_assignee=pm,
|
||||
)
|
||||
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||
# Clearing the stale claim now looks it up via session.get to release
|
||||
# its ACTIVE marker too.
|
||||
session = MagicMock(flush=AsyncMock(), get=AsyncMock(return_value=None))
|
||||
svc = TaskService(session)
|
||||
_bind(svc, "get", AsyncMock(return_value=task))
|
||||
out = await svc.admin_set_status(task.id, TaskStatus.NEEDS_REVISION)
|
||||
assert out is task
|
||||
@@ -1085,7 +1112,10 @@ async def test_admin_set_status_pre_block_restore_syncs_active_claimant() -> Non
|
||||
pre_block_state="in_progress",
|
||||
pre_block_assignee=dev,
|
||||
)
|
||||
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||
# An IN_PROGRESS restore now looks up the restored owner (dev) via
|
||||
# session.get to flip its ACTIVE marker.
|
||||
session = MagicMock(flush=AsyncMock(), get=AsyncMock(return_value=None))
|
||||
svc = TaskService(session)
|
||||
_bind(svc, "get", AsyncMock(return_value=task))
|
||||
out = await svc.admin_set_status(task.id, TaskStatus.IN_PROGRESS)
|
||||
assert out is task
|
||||
@@ -1566,7 +1596,10 @@ async def test_unblock_with_branch_resumes_in_progress() -> None:
|
||||
branch_name="feature/backend/abc12345",
|
||||
blocker_raised_by=uuid4(),
|
||||
)
|
||||
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||
# Resuming IN_PROGRESS now looks up the restored owner via session.get
|
||||
# to flip its ACTIVE marker.
|
||||
session = MagicMock(flush=AsyncMock(), get=AsyncMock(return_value=None))
|
||||
svc = TaskService(session)
|
||||
_bind(svc, "get", AsyncMock(return_value=task))
|
||||
_bind(svc, "_index_lifecycle_event_background", AsyncMock())
|
||||
out = await svc.unblock(task.id)
|
||||
@@ -1809,6 +1842,56 @@ async def test_finalize_claim_rollback_emits_reversal_audit() -> None:
|
||||
assert {"from": "claimed", "to": "pending"} in audit_calls
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_finalize_claim_sets_agent_active_then_rolls_back_on_failure() -> None:
|
||||
"""_finalize_claim must flip agent.status/current_task_id to ACTIVE/this
|
||||
task BEFORE the branch step runs (previously nothing ever wrote these
|
||||
fields at all — the fleet-status bug), and roll them back to their
|
||||
pre-claim values on a branch-creation failure, same as the task fields.
|
||||
"""
|
||||
session = MagicMock()
|
||||
session.flush = AsyncMock()
|
||||
svc = TaskService(session)
|
||||
|
||||
task = _build_task(
|
||||
status=TaskStatus.PENDING,
|
||||
branch_name=None,
|
||||
project_id=uuid4(),
|
||||
product_id=None,
|
||||
batch_id=None,
|
||||
parent_task_id=None,
|
||||
cell_projects=[],
|
||||
pr_created=False,
|
||||
pr_number=None,
|
||||
)
|
||||
agent = MagicMock(
|
||||
id=uuid4(),
|
||||
role=AgentRole.DEVELOPER,
|
||||
status=AgentStatus.IDLE,
|
||||
current_task_id=None,
|
||||
)
|
||||
_bind(svc, "_emit_status_transition_audit", MagicMock())
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
async def _boom(_task: object, _agent_id: object) -> str:
|
||||
captured["status"] = agent.status
|
||||
captured["current_task_id"] = agent.current_task_id
|
||||
raise RuntimeError("branch boom")
|
||||
|
||||
_bind(svc, "_ensure_branch_for_task", _boom)
|
||||
|
||||
with pytest.raises(RuntimeError, match="branch boom"):
|
||||
await svc._finalize_claim(task, agent, agent.id)
|
||||
|
||||
# Set to ACTIVE/this-task before the branch step ran...
|
||||
assert captured["status"] == AgentStatus.ACTIVE
|
||||
assert captured["current_task_id"] == task.id
|
||||
# ...and rolled back to the pre-claim values once branch creation failed.
|
||||
assert agent.status == AgentStatus.IDLE
|
||||
assert agent.current_task_id is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_emit_status_transition_audit_writes_in_session_atomically() -> None:
|
||||
"""The status-transition audit row is written into the CALLER's session (same
|
||||
|
||||
Reference in New Issue
Block a user