Skip to content

Agent engine: turn & thread state-machine correctness (7 PRs) #20

Description

@ahueb

Seven verified defects in the agent engine's turn and thread state machine. All live in src/agent/simulation.py (plus state.py, message_log.py, agent.py), all run in the agent-run container, so they share one reviewer and one test surface.

Line numbers are current as of origin/main @ b7edcbc (2026-07-30). Every item was re-verified at that commit; none is fixed.

Suggested order: E4 → E1 → E2 → E7 → E5 → E3 → E6 — robustness first (E4 stops the sim dying), then the state machine, then the two medium items.

PR E1 — Thread-outcome state-machine integrity (small)

  • COR-1_post_message (:2645) is -> None and returns early on the ThreadNotFound-evict path (:2676-2687) without appending a LogEntry; _reply_to_thread still bumps message_count (:1078) and runs _check_thread_outcome (:1084) on text that was never posted, and _evict_dead_thread (:1361) purges agent state, poll cursors and _closed_thread_ids but not the message log → a phantom ProposalRef/ThreadDecision/PI-DM. The swallowed-SlackApiError path (slack_client.py:411-418 returns None) now differs: _post_message mints a local id and does persist the entry (:2703-2714), so the message is durable but silently absent from Slack. Fix: return LogEntry|None; branch on None; purge the log on evict.
  • COR-3_check_thread_outcome (:1092-1124) finalizes on any against the first prior other-agent :memo: anywhere in reversed history — no temporal/adjacency check. Fix: require the memo be the most-recent other-agent message / newer than our last.
  • COR-4 — public path matches only raw (:1100); private path matches and :white_check_mark: (:1247), so a shortcode confirmation in a public thread is silently missed. Fix: one shared marker check for both paths.
  • COR-7_close_thread's _prior_threads…append (:1158) lacks the dedup its pending_proposals sibling has (:1115/:1177); reopen→reclose (and per-ThreadDecision rebuild, :3321) inflate every future Phase-5 prompt. Fix: dedup the append + idempotency-guard _close_thread.

PR E2 — Scan-cursor semantics + Phase-3 activation offset (small-medium)

  • COR-6_run_turn sets last_seen_cursor = time.time() (:648, wall clock) while MessageLog since-filters use posted_at <= since (message_log.py:133/229/250/321); a Phase-2 top-level post or Phase-3 activation arriving mid-turn is filtered out next turn and never processed. Fix: advance from the max processed posted_at / message_log.latest_timestamp.
  • COR-2 — Phase-3 activation builds ThreadState with no message_count_offset (:830-836 tag path, :869-875 reply path), so _reply_to_thread closes an activated ≥12-message thread as "timeout" (:964) before replying. The reopen paths do set the offset (:2356/2367/3837/3848) — proving the omission. Fix: set the offset on Phase-3 activation.

PR E3 — Durable + authorized PI reviews and reopens (medium)

  • COR-5_check_pi_proposal_review (:2308-2321) matches only thread_id, flips reviewed=True for all agents with no sender check, and persists nothing → dashboard/email keep showing "unreviewed" and _rebuild_agent_state (:3437) re-blocks on restart; any human (or the other lab's PI) clears the block. Fix: require the sender be the owning PI; insert a ProposalReview.
  • COR-13 — two DB→memory readers use different keys — rebuild (thread_decision_id, agent_id) (:3437) vs per-tick (agent_id, thread_id) (:3712) — because ProposalRef carries only thread_id (state.py:40-49); the rating-0 web reopen is deduped only by the in-memory _db_reopened_thread_ids (initialised empty at :253, never persisted), so it re-fires each restart and re-appends a persisted duplicate PI-guidance row (:3814-3824 mints a fresh id, so the idempotent append does not suppress it); Slack-native _reopen_thread (:2323) writes nothing durable. Fix: add thread_decision_id to ProposalRef; unify the review key; persist reopen/dedup state.

PR E4 — Poller & LLM-log-flush robustness (trivial)

  • COR-10_poll_pi_dms (:2372) calls poll_dm_messages unguarded (:2395); _call_with_retry catches only SlackApiError (slack_client.py:152-167) and the poller runs outside the _run_turn try (poller block :427-437, try at :481), so a transient socket/DNS/SSL error kills the whole sim (one-off container → no restart). It is also the only unthrottled poller — CHANNEL_POLL_INTERVAL/PROPOSAL_POLL_INTERVAL/ROSTER_POLL_INTERVAL exist (:132-134) but there is no DM equivalent. The _poll_inbound_from_db handler call (:2266) is likewise unguarded after the cursor advanced (:2244-2245), unlike its DM twin _poll_pi_dms_from_db, which does wrap handle_dm. Fix: wrap both like the sibling pollers; add a DM poll interval.
  • COR-11_flush_llm_logs (:3524) clears the buffer before the write (:3529-3530); the except only logs (:3550-3551) → up to 10 LLM-log rows lost per failed flush (also undercounts restart-rebuilt api_call_count). PR Database as primary conversations #19's H1 fixed the conversation flush but not this twin. Fix: mirror the H1 re-queue.

PR E5 — Phase-5 output hygiene + Slack mention resolution (small)

  • COR-9 — the persist path writes the visibility column (:3089), but _post_message builds the LogEntry without visibility= (:2703-2714), so private-channel bot posts default to "public" (message_log.py:27) and are persisted mislabeled in the authoritative store; the reply channel comes from LLM output action_data.get("channel", "general") (:1825) not the target post; _update_agent_memory (:3968) writes the raw LLM response to the memory file (:4035-4037) with no <slack_message>/tag strip. Fix: source visibility from the target channel; derive the reply channel from the target entry; strip synthesis output.
  • COR-8 (remainder) — real Slack <@Uxxx> mentions are undetected (_extract_tagged_agent regex @(\w+[Bb]ot)\b, message_log.py:303-309; PI-tag check literal f"@{bot_name.lower()}", :2199) and the regex is case-blind. Reproduced 2026-07-30: @WisemanBot → resolves; @WISEMANBOTNone; <@U04ABCDEF>None. Fix: translate <@Uxxx> via the bot-user-id→agent-id map; add IGNORECASE.

PR E6 — Budget accounting (medium)

budget_cap is checked only at selection (_agent_within_budget, :338, called at :453 and :577), never across the 5 mid-turn api_call_count += 1 sites (:713, :759, :1015, :1789, :4025); roster re-add builds a fresh Agent with api_call_count=0 (_sync_roster_from_db, :3589; agent.py:76), resetting the cap and dropping in-flight state on an inactive→active flip; total_api_calls recomputed from the live roster (:3158) is non-monotonic. Fix: one authoritative mid-turn _charge() check; seed re-added agents from llm_call_logs; accumulate totals monotonically.

PR E7 — Liveness / cost loops (small)

The daily-cap gate returns before the PI-priority/private/funding bypasses (:1615-1618, bypasses evaluated from :1621); has_pi_directive is cleared unconditionally (:645); thread.pi_context is set (:2193, :2298, :2355, :3836, :3953) but never cleared anywhere → re-injected as "authoritative" into every future Phase-4 prompt (agent.py:450-456); interesting_posts swap/restore both sit before the try (:1707 vs :1787, try at :1790) so an exception in the prompt-build window permanently narrows state. Fix: move the cap gate after bypass evaluation; clear pi_context after consumption; clear has_pi_directive only when acted on; wrap the swap in try/finally.

Definition of done: each PR ships a test that covers its defect line and fails against the pre-fix code. simulation.py measured 30 % coverage on the 2026-07-30 gate run, so this issue carries a large share of the coverage ratchet tracked in the issue #27.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions