Conversation
Cohorts are admin-managed groups gating which agents act on each other's activity during simulation, to conserve LLM calls at large roster sizes. Isolation (opt-in via cohort_isolation_enabled, default off): - New cohorts / cohort_memberships tables (migration 0019) + models. - SimulationEngine recomputes each agent's allowed_sender_ids on the roster sync cadence; the gate is applied at the MessageLog read boundary (get_new_top_level_posts / get_tags_for_agent / get_replies_to_agent_posts) so Phase 2 scan and Phase 3 activation skip non-cohort senders and Phase 4/5 Opus calls are suppressed downstream. Human PI messages always pass. Uncohorted agents are isolated when the flag is on. A Phase 5 cold-tag guard strips @tags toward non-cohort agents. - Admin CRUD at /admin/cohorts (list/create/detail/delete/add/remove) + nav. Reactive-priority scheduler (sequential; replaces the abandoned concurrent-turn proposal): _select_agent now selects owed-reply agents first (oldest-waiting, excluding the last LLM caller) so 1:1 threads conclude promptly instead of waiting on staleness-weighted random selection, with a fairness valve (max_consecutive_reactive_turns) to avoid starving new-conversation formation. The weighted-random proactive path and the back-to-back guard are unchanged. Tests: tests/test_cohort_isolation.py covers the MessageLog filter (incl. backward-compat when disabled), allowed_sender_ids recomputation, _owes_reply, and the reactive tier. Full suite green (282 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…+ rebuild) Historically Slack was the primary durable store of conversation content: the in-memory MessageLog was rebuilt from Slack history on every start, and agent_messages held only metadata (message_length, no body). With Slack off the system could not reconstruct any conversation and a restart lost all history. This lands the foundation from specs/local-db-conversations.md so the local DB is the single source of truth and the simulation can run with Slack fully off. Stage 1 — content persistence + DB rebuild: - Migration 0019 extends agent_messages with content, sender_name, is_bot, posted_at and nullable slack_ts/slack_channel_id/slack_thread_ts mirror columns; relaxes agent_id to nullable (NULL = human/PI, mirroring LogEntry.sender_agent_id); adds UNIQUE(simulation_run_id, message_ts) plus posted_at / channel / partial slack_ts indexes. - MessageLog.append is now idempotent (skips duplicate ts) and fires a persist callback; load_entry() appends restored rows without re-persisting. - SimulationEngine buffers every appended entry (bot, peer, and human) and batch-upserts them into agent_messages in _flush_persisted (ON CONFLICT on run+message_ts), drained each main-loop tick and on stop(). The per-post _log_message path is retired. - _rebuild_state_from_db() hydrates the log from agent_messages; the per-agent state reconstruction is extracted to _rebuild_agent_state() so it runs with Slack on or off; _rebuild_state_from_slack is demoted to a Slack-only reconcile that only adds messages missing from the DB. Stage 2 — local id minting: - mint_ts() yields monotonic, unique, ts-shaped ids seeded from the rebuild's max(posted_at); replaces the str(time.time()) fallbacks and removes the "mock_ts" constant (a latent collision that idempotent append would have turned into message loss). - Slack-off channels use stable local:<name> ids (can't collide with Slack C…/G…); seeded channels are persisted to agent_channels. Verification: full suite 308 passed (new tests cover idempotent append, the persist callback, load_entry bypass, and mint_ts monotonicity/seeding), plus a real-Postgres continuity check (post -> persist -> fresh rebuild with thread intact, duplicate dropped, human row stored with NULL agent_id). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Formalizes running with Slack fully off and adds the Slack-independent inbound path for PI messages. - New src/agent/transport.py: a Transport Protocol matching the exact surface the engine uses on AgentSlackClient (which conforms structurally, no change), plus NullTransport — a no-op used when Slack is disabled. NullTransport reports is_connected=False (so the engine's existing guards take the no-op path), returns None from outbound posts (engine mints a local id), and returns local: ids for channel creates and [] from all inbound polls. - config.slack_enabled (bool | None): None auto-detects (Slack on iff any agent has a usable token); SLACK_ENABLED=false forces DB-only mode. main.py resolves it (--mock forces off), builds AgentSlackClients when on and NullTransports when off, and passes the flag to the engine. - SimulationEngine.slack_enabled gates the roster hot-add: when off, newly-active agents are admitted with a NullTransport instead of being dropped for a missing token / failed connect. - New _poll_pi_inbox_from_db(): ingests human/PI rows the web app writes to agent_messages (is_bot=false), appends any unseen ones to the MessageLog, and routes them through PI handling (proposal-review clear, thread reopen, pi_context, @bot tags) derived from the thread's own participants rather than a Slack user→agent map. Runs every tick regardless of Slack, and is the convergence point for the future Slack mirror's inbound side. Cursor seeded past restored history in _rebuild_state_from_db. Verification: full suite 314 passed (new tests: NullTransport behavior + Protocol conformance for both NullTransport and AgentSlackClient), plus a real-Postgres smoke test (a web-written PI reply on a proposal thread is ingested and clears the pending-proposal block; re-poll is a no-op). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… poller The public→collab_private migration (PI reopen flow) now works with Slack off, and a running sim ingests migration handover posts through the DB. - private_channels.py: _slack_enabled_for_migration() resolves the mode (explicit SLACK_ENABLED wins; else auto-detect from whether both bots have usable tokens). When off, _migrate_offline() creates the collab_private AgentChannel with a local: id and the same PrivateChannelMember rows as the Slack path, but makes no Slack calls: the handover posts (authored by the creator bot) and the neutral ⏸️ origin-thread close marker are written straight to agent_messages (visibility=collab_private for the handover) and refined_in_channel is set. The other PI is not DM'd (no transport). - Generalized the engine's DB inbound poller (renamed _poll_pi_inbox_from_db -> _poll_inbound_from_db): it now ingests ANY unseen message for the run, not just human rows — so bot-authored handover posts written by the web process reach the live MessageLog. Human/PI rows still additionally go through PI handling; the sim's own messages are already in the log and are skipped. Verification: full suite 314 passed, plus a real-Postgres smoke test with SLACK_ENABLED=false (offline migration creates a local: private channel with 3 members, writes 3 handover posts + origin close marker + refined_in_channel, and a running sim ingests all 3 handover posts from the DB). Also confirmed the auto-detect path still uses Slack when tokens are present. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Gives PIs (and delegates) a Slack-independent way to see recent activity and
inject a message into their agent's workspace.
- src/services/pi_inbox.py: get_latest_run_id() and record_pi_message(), which
insert a human/PI row (is_bot=false, agent_id=null) into agent_messages with a
minted ts, resolving channel_id/visibility from agent_channels. The running
sim ingests it via _poll_inbound_from_db and routes it through PI handling.
- agent_page.py: GET /{agent_id}/conversations (a read view of recent messages —
content now lives in the DB — plus a post form) and POST /{agent_id}/message
(writes the inbound row; an optional "address my bot" toggle prepends the
@botName tag the engine already understands). Both gated by
get_agent_with_access (owner or delegate).
- templates/agent/conversations.html + a dashboard card linking to it.
Identity uses AgentRegistry.user_id / access checks rather than a Slack user id,
so this works with Slack fully off. DM-style directives depend on DM persistence
and are wired in Stage 7.
Verification: full suite 314 passed; a real-Postgres smoke test (record_pi_message
-> _poll_inbound_from_db ingests the PI row and the @bot tag sets has_pi_directive
on the target agent); app restarted cleanly and the new routes resolve.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Makes the non-engine Slack writers degrade to the DB when Slack is off, and records the Slack↔DB id mapping so mirroring and reconcile dedup are correct. Secondary posters (gated by slack_tokens.slack_globally_enabled — explicit SLACK_ENABLED wins, else auto-detect from token presence): - grantbot.py: when Slack is off, funding opportunities are written to agent_messages (authored by a "grantbot" identity, 💰 top-level post) via _post_funding_to_db instead of released — so funding threads still exist and agents scan them. - email_inbound.py + agent_page.py reopen: the legacy "post guidance to the origin thread" fallback now writes to the DB inbox via record_pi_message when Slack is off (the primary reopen path already routes through the Slack-aware migration). invite.py's users_lookupByEmail was already token-guarded. Slack-mirror mapping: - LogEntry gains slack_ts/slack_channel_id; _post_message records them when a connected client posted (in pure Slack-on mode slack_ts == message_ts), and _flush_persisted upserts slack_ts/slack_channel_id/slack_thread_ts. - _rebuild_state_from_db seeds _known_slack_ts; the Slack reconcile skips any message already represented in the DB by its slack_ts (dedup for a DB-origin message later mirrored to Slack) and stamps reconciled entries with their slack mapping. Verification: full suite 314 passed, plus a real-Postgres smoke test (a connected post persists message_ts == slack_ts + slack_channel_id, and a fresh rebuild seeds _known_slack_ts for reconcile dedup). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ackfill
Completes the DB-primary refactor: PI<->bot DMs are durable and Slack-optional,
and there's a one-time importer for existing Slack history.
DM persistence:
- Migration 0020 + PiDmMessage model: pi_dm_messages (run, agent_id, pi_user_id,
direction inbound/outbound, content, ts, slack_ts, posted_at). pi_user_id is a
Slack user id (Slack-on) or local:<users.id> (web).
- pi_inbox.record_pi_dm() / web_pi_user_id(); PIHandler._send_dm now records every
outbound DM (durable + visible in the web UI even with Slack off) and takes
simulation_run_id.
- DM processing is unified through the DB: _poll_pi_dms (Slack) now only RECORDS
inbound DMs as rows; the new _poll_pi_dms_from_db is the single processor that
runs each inbound row through PIHandler.handle_dm and flips has_pi_directive.
This handles Slack and web DMs identically with no double-processing.
_seed_pi_dm_cursor starts it past existing history on boot.
PI web interface (DMs): POST /agent/{id}/dm writes an inbound DM row; the
conversations page shows the recent DM thread and a send box.
Ops:
- --fresh now also wipes pi_dm_messages.
- scripts/backfill_slack_history_to_db.py: one-time importer that reuses the
engine's setup + Slack reconcile + flush to pull channel/thread history
(content + slack_ts) into agent_messages for a run, without running any turns.
Verification: full suite 314 passed; a real-Postgres smoke test (inbound web DM
recorded → processed exactly once → directive flag set → re-poll no-op; outbound
DM persisted); app/worker/grantbot restarted cleanly; backfill script imports.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…els)
_persist_seeded_channels referenced AgentChannel but it wasn't in the module
import, raising NameError ("Failed to persist seeded channels") on startup so
seeded channels were never recorded in agent_channels. Surfaced by the first
Slack-off agent run. Add AgentChannel to the src.models import.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The channel poller and proposal-thread PI-reply poller recorded human Slack messages with message_ts set to the Slack ts but left the slack_ts / slack_channel_id mirror-mapping columns null, so "origin = Slack" reporting (and the reconcile's slack_ts dedup set) didn't see them. Content and dedup were unaffected (dedup keys on message_ts, which already equals the Slack ts here), but the mapping is now recorded for consistency with agent posts and the bulk reconcile. Web-origin reopen guidance stays null (it is DB-origin, not Slack). Verification: full suite 314 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merged main (the test-suite-characterization overhaul: tests/unit|integration| characterization|contract split, testcontainers harness, fakes/factories, CI gate). No conflicts with the refactor's src/ changes. Alignment fixups: - Move tests/test_transport.py -> tests/unit/ to match the new layout (still collected before, but keeps the convention). My edits to the three relocated files (test_message_log, test_simulation_logic, test_private_channel_migration) came across cleanly via git's rename-follow. - test_simulation_logic: add strict=False to the mint_ts zip() (ruff B905; the test lint gate is enforced by scripts/ci.sh). - test_harness_smoke: bump the pinned alembic head 0018 -> 0020 (this branch adds migrations 0019 + 0020). Verified: scripts/ci.sh passes — 460 tests (unit+integration+characterization+ contract), 13 golden-master snapshots unchanged, coverage 35.62% >= 35% floor, ruff clean; migrations 0019/0020 apply via the real alembic chain in the integration harness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds tests/integration/test_pi_inbox.py (real migrated Postgres) covering src/services/pi_inbox.py — the Slack-independent path by which PI web messages and DMs enter the simulation: - get_latest_run_id picks the most recent run - record_pi_message writes a human row (is_bot=false, agent_id NULL), resolves channel_id/visibility from agent_channels, falls back to local: + public, and sets phase new_post vs thread_reply - record_pi_dm writes inbound/outbound rows; web_pi_user_id formatting Lifts src coverage 35.62% -> 35.86% (pi_inbox was 0%). Full gate: 464 passed, 13 golden-master snapshots unchanged, ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_flush_persisted detached the pending buffer before the try and only logged on failure, so any transient DB error at flush time silently dropped that batch. Because the DB is now the primary conversation store, a restart rebuilds from the DB and the lost messages are gone for good. Re-queue the batch on failure (prepend ahead of any entries enqueued during the failed commit, preserving chronological order) so the next tick retries. The no-DB path still clears the buffer to avoid unbounded growth. Adds regression tests covering re-queue, ordering vs. newly-arrived entries, and the no-DB clear path. Addresses H1 from the PR #19 review (issue #18). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mint_ts precision: f"{time.time():.6f}" lost monotonicity at the current
epoch magnitude — float64 ULP (~2.4e-7s) is coarser than the 1e-6s step,
so consecutive ids could format to the same string (breaking uniqueness)
or fail to increase once parsed back to float (breaking posted_at order).
Replace with a shared TsMinter (src/agent/ids.py) that carries a monotonic
integer-microsecond high-water mark and formats to a string only at the
end, never round-tripping the fraction through a float.
M1 (canonical-id collision across processes):
- Share one mint_local_ts() helper across all writers (PI web inbox,
GrantBot) so they inherit the engine minter's per-process monotonic,
unique guarantee instead of raw time.time() (also DRYs the id format).
- M1a: add a WHERE guard to the engine flush's ON CONFLICT DO UPDATE so a
bot message can never overwrite an existing human (PI) row on a
cross-process ts collision; legitimate bot re-flush / human re-flush /
slack-mirror updates still apply.
- M1b: guard the web post route against the IntegrityError a collision
raises — roll back and retry once with a fresh id, else return 409
instead of a raw 500.
Tests: TsMinter/mint_local_ts unit tests (monotonic, unique, float-ordered,
ts-shaped, seed_floor); rewrite the mint_ts tests as precision regression
guards; integration tests for the M1a upsert guard (block bot-over-human,
allow bot re-flush, allow human re-flush) run against real Postgres.
conftest gains an optional TEST_DATABASE_URL to point the suite at an
existing Postgres when a Docker socket for testcontainers isn't available.
Addresses M1 and the mint-precision issue from the PR #19 review (issue #18).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both DB inbox pollers filtered posted_at > cursor and advanced the cursor to the max posted_at seen. But posted_at is stamped at row *creation* (mint time), not commit — so a row written by another process (a PI web message) can become visible only after this process has already advanced its cursor past that timestamp. That row lands below the high-water mark and is never ingested: silently, permanently, at debug log level. With Slack off this is the only PI input path, so the message is simply lost. Fix: query a lookback window behind the cursor and dedup by identity, so a late-committing row is re-queried within the window and ingested exactly once. The channel poller already deduped via the message log; the DM poller gains a seen-set (ts -> posted_at), pruned to the lookback window each poll, and _seed_pi_dm_cursor seeds it so a restart's first re-scan doesn't replay history through handle_dm. Polls are LLM-paced, so the re-scan is cheap. Also raise the three Slack-off poll-failure logs from debug to warning so a persistently failing poll isn't invisible. Tests (real Postgres): channel + DM pollers ingest a row committed below the cursor; the DM lookback re-scan dedups (processed once); the re-scan is bounded (a row older than the window is not re-queried); restart seed prevents DM replay. Addresses H2 from the PR #19 review (issue #18). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
B1 — _flush_persisted ran a full COUNT(*) over the run's agent_messages on every flush (~every tick) just to refresh the cosmetic total_messages / total_api_calls shown in the admin UI. Throttle that refresh to at most once per RUN_STATS_UPDATE_INTERVAL (30s), forced on shutdown, while still upserting the message rows every flush. total_messages may lag by a few seconds between refreshes — fine for a display counter. B2 — _rebuild_state_from_db loaded *every* row of a run (with content) into memory at startup, so restart time and RAM grew with all-time history. Bound the load to a window: messages from the last REBUILD_WINDOW_S (14d) plus the full history of any thread with no ThreadDecision (still undecided). Active-thread reconstruction only needs undecided threads, and all other restored state (proposals, prior threads, api counts) comes from DB tables, not the log — so old closed-thread bodies are safe to leave in the DB. Add _hydrate_thread_from_db to pull a specific thread back on demand, and call it in the three reopen paths (PI web reply, PI DM/Slack reopen via _reopen_thread, and rating=0 proposal reopen) so reopening an old closed thread still resolves participants and the reply-budget offset. Tests (real Postgres): stats count is throttled and force-refreshes on shutdown; rebuild loads recent + undecided threads and windows out old closed ones; on-demand hydration restores a windowed-out thread and is idempotent. Addresses B1/B2 from the PR #19 review (issue #18). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The engine seeded the channel name->id lookup by poking each client's private `_channel_name_to_id` attribute directly (simulation.py). That attribute isn't part of the Transport Protocol, so a new backend that satisfies the *declared* contract would crash with AttributeError in _ensure_seeded_channels — NullTransport only carried the attribute "for parity", which was the tell. Add a public `cache_channel_ids(mapping)` to the Transport Protocol, implement it on NullTransport and AgentSlackClient, and switch the engine's two seeding sites to call it. The write path is now part of the contract. Also drop `set_visibility_lookup`, which had zero callers across the repo (the visibility lookup is set via the AgentSlackClient constructor): removed from NullTransport, AgentSlackClient, and the test fake (plus the fake's unused _visibility_lookup attribute). Tests: cache_channel_ids seeds the lookup; it's declared on the Protocol; a Protocol-only backend without _channel_name_to_id can be seeded; the dead setter is gone. Addresses M2 from the PR #19 review (issue #18). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the three residual findings in .notes/db-conversations-residual-2026-07-24.md
— the failure modes left after H1/H2/M1a, all at boundaries the Slack-primary
system got for free (a globally unique authoritative ts, durability the instant a
message was posted, one clock).
R2 — messages lost on a hard kill
Posts are buffered in MessageLog and written to Postgres once per turn, so the
documented `docker rm -f agent-run` (SIGKILL) discarded the in-flight turn
permanently: the DB, not Slack, is the durable store. Three parts, because the
doc change alone was not enough:
* restart recipe is now `docker stop -t 30 agent-run && docker rm agent-run`
(CLAUDE.md, README.md, provision_slack_bots.py), and the agent service
carries stop_grace_period: 30s so the container's StopTimeout matches;
* the graceful path was itself racy — the handler did
`asyncio.ensure_future(sim_engine.stop())`, so the main loop could return
first and asyncio.run cancel the orphan flush mid-await. The handler is now
the sync request_stop() (flag only) and `await sim_engine.stop()` runs in the
entry point's finally-block, on every exit path;
* SIGTERM could not be noticed in time — the idle backoff sleeps up to 30s,
longer than the old 10s grace. Those sleeps now go through _sleep(), which
races the sleep against a stop event.
An in-flight LLM call is still not interruptible, hence -t 30 over the default.
R1 — cross-process canonical-id collision silently dropped a message
TsMinter guaranteed uniqueness per process, but three processes mint into the
same run (engine, web app, GrantBot). Two minting in the same microsecond
produced the identical id; the uq_agent_messages_run_ts conflict handler then
resolved it by keeping one row and dropping the other. Each minter now owns a
residue class: ids are quantized to a 100us slot with the writer id in the low
microsecond digits, so a cross-writer collision is structurally impossible. Ids
stay ts-shaped, float-parseable and strictly ordered; the cost is resolution
(one id per writer per 100us), far above the real posting rate. Writer ids are
claimed at process entry via set_default_writer_id(); the engine process claims
two, since its DM writes use the module-default minter.
Chosen over re-mint-on-conflict, which for a thread root would have to rewrite
MessageLog._by_ts, every child's thread_ts, PostRef.post_id,
ThreadState.thread_id, ProposalRef.thread_id and the ThreadDecision rows; and
over a DB sequence, which mint_ts() cannot await. The M1a guard and the web
route's M1b retry stay as backstops.
R3 — inbound delivery depended on the writers' clocks agreeing
Both DB inbox pollers paged over posted_at, which is float(the writing process's
minted ts). A PI message stamped more than the 300s lookback below the engine's
cursor was skipped silently and forever — safe on one host, not across hosts.
They now page over created_at (server_default=now(), the single Postgres
server's clock), so the window depends on one clock. posted_at remains the
ordering key for conversation content; it is just no longer the delivery cursor.
Cursors are datetimes, seeded from MAX(created_at) over the whole run rather
than the windowed rebuild's loaded rows. An id-based cursor was rejected: the PK
is a random uuid4, so it would have needed a new sequence column and a backfill.
The lookback stays — it covers commit visibility, not skew (H2).
Migration 0021 adds ix_agent_messages_run_created and
ix_pi_dm_run_direction_created to back the new access path.
Tests: writer-slot disjointness at a frozen clock (unit) and against the real
unique constraint (integration); graceful-shutdown flag/sleep/flush behaviour;
delivery of a row from a writer three years of skew behind. The H2 tests now
derive their cursor from each row's own created_at, so they no longer depend on
the test process's clock either. Full suite green: 500 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two remaining "minor observations" from
.notes/db-conversations-residual-2026-07-24.md.
Thread history was not posted_at-ordered
MessageLog.get_thread_history returned root + replies in log-insertion order.
Single-process appends are roughly ordered, but the DB inbound poller and the
Slack reconcile append entries whose posted_at predates messages already in the
log, so the LLM could get a subtly scrambled thread — unlike the Slack-primary
rebuild, which fetched in ts order. Replies are now sorted by posted_at; the
sort is stable, so entries sharing a timestamp keep their insertion order. The
root stays pinned first: it is the thread's parent by definition even when a
writer running behind stamps a reply below it. This also re-orders the input to
get_thread_allowed_agents' first-two-participants rule, which is the intended
correction.
Enabling Slack mid-conversation corrupted the mirror
Slack threads on the *root's* ts, which equals the canonical thread_ts only when
the root was born on Slack. A thread started with Slack off has a minted root id
that Slack has never seen. The finding named the recorded slack_thread_ts
column, but the defect was wider: _post_message passed the canonical thread_ts
straight to client.post_message, so the Slack API call itself was wrong and the
reply would detach or error. Fixing only the column would have left that intact.
* new _slack_parent_ts() translates canonical -> Slack via the root entry's
slack_ts, falling back to the canonical id when the root is not in the log
(windowed out by the B2 rebuild bound), which preserves pure-Slack-on
behaviour where the two are the same value;
* a reply whose root has no Slack presence is kept DB-only with a warning
rather than posted against an unknown id — the message still drives the
simulation, only the mirror skips it. Mid-life toggling stays unsupported
by design, but now degrades safely instead of corrupting the thread;
* LogEntry gains slack_thread_ts, set at every Slack-origin construction site
and written by _flush_persisted in place of the canonical thread_ts.
Prerequisite, found while fixing the above: the rebuild and hydrate paths were
dropping slack_ts from restored entries entirely, so after any restart every
root looked DB-origin. Left as-is, the new check would have been a regression
that stopped mirroring *all* replies on the first restart — and a restart with
Slack newly enabled is precisely the mid-life-toggle scenario. Both paths now
restore the mirror mapping, inferring it for pre-Stage-6 rows: a real Slack
channel_id with a NULL slack_ts means the canonical id IS the Slack ts.
No migration — slack_thread_ts already exists on agent_messages; only what gets
written into it changed.
Left alone: MessageLog.latest_timestamp has the same insertion-order flaw but
has no callers in src/ or tests/, so it is noted rather than changed. Fix or
delete it before anything uses it as a cursor.
Full suite green: 512 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects in private_channels.py, both cases of the DB not actually being
the primary store yet.
R4 — the Slack-off migration minted its canonical ids by hand as
f"{time.time():.6f}", the one site the R1 writer-slot fix missed. Those ids
carry no writer slot, so they can coincide with an id minted by the engine or
GrantBot; because these rows go in through the ORM rather than the ON CONFLICT
upsert, the collision aborts the whole migration transaction (channel,
members, handover, close marker) rather than dropping one message. The format
also round-trips microseconds through a float, which cannot hold them at
current epoch magnitudes, and it ignored the minter's high-water mark.
R5 — the Slack-on migration posted the handover and the origin-thread close
marker to Slack and never wrote them to agent_messages. Nothing was lost while
Slack stayed on (the reconcile pass re-reads Slack history), but a Slack-off
restart rebuilt the refinement channel with no handover in it: two bots in a
channel whose purpose is stated only in messages the DB never saw. The marker
was also posted with thread_ts=thread_decision.thread_id — the *canonical* id,
which is not a valid Slack ts once a thread has started Slack-off. That is the
same bug a93d136 fixed in the engine, still live here because the web process
has no MessageLog to resolve the root against.
Both paths now write through one helper, _add_handover_message: canonical id =
Slack ts when the mirror post landed, else minted; slack_* columns only when it
landed. _slack_parent_ts_from_db is the DB-side twin of the engine's resolver,
with the same pre-Stage-6 inference, and the marker stays DB-only (with a
warning) when the root has no Slack presence.
Two incidentals in the lines being touched: _latest_simulation_run_id moved
above the Slack side-effects, so a run-less DB fails before creating an orphan
Slack channel instead of after; and ThreadNotFound on the marker post no longer
aborts a PI-initiated migration.
Tests: three integration tests, each verified to fail against the pre-fix code
(IntegrityError on the duplicate message_ts; no rows written; a minted id handed
to Slack as thread_ts). FakeSlackClient gains _resolve_channel_id.
Full gate green: 515 passed, coverage 38.63%.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The log is append-only in insertion order, which is not time order: the DB inbound poller and the Slack reconcile append entries whose posted_at predates messages already stored. a93d136 fixed get_thread_history; three sibling reads were still keying on list position. - latest_timestamp returned _entries[-1].posted_at, so a cursor taken from it could move backwards. It has no callers, which is why it was left open — it now reads an O(1) high-water mark maintained by a new _record(), the single add path behind append/load_entry. - get_agent_top_level_posts sliced posts[-limit:] in insertion order. Callers: the Phase 5 dedup context (limit=10) and the daily post cap (limit=100). A late append of older history pushes a genuinely recent post out of the slice, so the LLM loses its most relevant dedup context and the cap under-counts. Now sorted by posted_at (stable) before slicing. - get_last_bot_sender_in_channel scanned reversed(_entries). Callers: the collab_private turn-taking gate (three sites). A late-appended *older* message answered as "the last poster", handing the turn to the wrong bot — letting one post back-to-back, or blocking the one whose turn it was. Now a max-scan on posted_at, ties resolved to the later insertion so behaviour is unchanged when timestamps collide. Tests: TestOrderingIsByPostedAtNotInsertion, 7 cases. The three behavioural ones fail against the pre-fix code (backwards cursor; newest post dropped from the slice; 'wiseman' != 'su' for the last poster); the other four pin invariants the fix must not break. Full gate green: 522 passed, coverage 38.71%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The channel poller's bot branch built its LogEntry without slack_ts, slack_channel_id or slack_thread_ts, while the human branch 20 lines below sets all three from the same conversations.history response. The missing column is not the problem; _slack_parent_ts is. An entry with no slack_ts looks DB-origin, so any thread rooted at a polled bot post resolved to "no Slack root" and _post_message kept every reply DB-only. The roots this branch ingests are another workspace bot's posts — i.e. GrantBot's funding posts, whose threads are open to all agents — so the effect was that replies to a funding thread silently stopped being mirrored to Slack. Two things masked it: a restart repairs the mapping (_restored_slack_ts infers it from a real Slack channel_id), and it only bites for a root polled in during the current process's lifetime, so the window is "replies to today's funding post, before the next restart". Found by inspecting live rows after a restart — 60 of 625 rows in the current run carry slack_ts NULL, and the newest are GrantBot posts sitting in real Slack channels. Slack-origin means the canonical id IS the Slack ts, so the thread parent needs no translation, same as the reconcile and proposal-thread pollers already do. Forward-only: existing NULL rows still rely on the rebuild's inference, which handles them correctly; nothing backfills the column. Tests: test_polled_bot_message_keeps_its_slack_mapping drives the real poller with a canned GrantBot history entry and checks the in-memory mapping, _slack_parent_ts resolution, and that the mapping survives the flush. Fails against the pre-fix code (slack_ts is None). Full gate green: 523 passed, coverage 39.14%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_restored_slack_ts treated "a row in a real Slack channel with slack_ts NULL" as proof the message was born on Slack, and promoted its canonical id to a Slack ts. That covers pre-Stage-6 history, but it is unsound: a DB-origin message can carry a real Slack channel_id by two independent routes — a PI message from the web inbox (_resolve_channel returns the agent_channels row's id, Slack's when Slack is on) and an agent post whose mirror failed (_flush_persisted uses _channel_id_map, likewise Slack's id). Both mint a *local* id, so the inference fabricates a timestamp Slack never issued. _slack_parent_ts then hands it to chat.postMessage as a thread_ts: Slack drops the unknown parent, creates an orphan top-level post, post_message raises ThreadNotFound and the thread gets evicted. Nothing on the row separates the two cases — is_bot doesn't, since the failed-mirror case is a bot row — so the guess is removed rather than narrowed. slack_ts is now the only evidence of Slack presence; NULL means not on Slack. Legacy data is repaired instead of guessed at. scripts/backfill_slack_ts.py asks Slack via conversations.history whether each candidate ts actually exists and writes slack_ts only for confirmed ones; a failed lookup is reported as unverified rather than treated as absent, so an expired token cannot mark real Slack history as DB-origin. Dry-run by default, idempotent. Verified on the dev DB before this change landed: of 11 candidate rows Slack confirmed 10 (9 GrantBot posts, 1 polled human message) and denied 1 — a web-written PI row whose id the old inference would have promoted. The 10 are backfilled; the 1 correctly keeps NULL. _slack_parent_ts_from_db (added in 2a2e98c, which copied the inference) is fixed the same way. test_rebuild_infers_the_slack_ts_of_a_pre_stage6_row pinned the bug and is replaced by test_rebuild_never_infers_a_slack_ts_from_the_channel_id, built from the row shape that actually broke. DEPLOY ORDER: on a workspace with pre-Stage-6 history, run the backfill BEFORE deploying this — otherwise replies stop being mirrored into legacy Slack threads. Full gate green: 523 passed, coverage 39.08%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Database as primary conversations
Brings the cohort interaction gate + reactive-priority scheduler onto main's db-primary-conversations base so the v2 spec (.notes/cohort-system-v2.md) can be implemented against the DB-native conversation layer.
Builds out .notes/cohort-system-v2.md in full on top of the merge of cohort-agent-isolation into main's db-primary base. The v1 spec and its implementation both predate the DB-primary rewrite and disagreed with each other in eleven places; this makes the documented rule and the running code the same thing. Migration (§14) - Renumber 0019_add_cohorts -> 0022_add_cohorts (down_revision 0021). Two migrations sharing revision "0019" is invisible to git and pytest: the merge is clean, every test passes, and Alembic only warns — then `upgrade head` dies on multiple heads while a targeted `upgrade <rev>` silently applies whichever duplicate sorts last and stamps the DB as fully migrated. Revision ids are assigned at merge, never at branch. - All downgrades idempotent (if_exists) so a rollback cannot wedge on an object a partially-applied upgrade never created. - scripts/ci.sh gains an offline alembic gate (single head, no duplicate ids) ahead of lint and pytest. `alembic check` is not sufficient — it reports "target database is not up to date", the wrong diagnosis, and needs a live DB. - New cohort_audit_events table: append-only, denormalised cohort_name/actor_email and no FK on cohort_id so the trail outlives both the cohort and the user. Gate semantics (§5) - cohort_default_policy: "open" (default) | "isolated". Under "open" an uncohorted agent is unrestricted, restoring the contract v1 published and the code inverted — enabling isolation with no topology defined is now a no-op instead of roster-wide silence. - Preflight refuses to run isolation that would silence everyone: forced off with an ERROR when no live roster agent has any membership, or when there is no DB handle. Counts live members, not cohorts — an empty cohort silences the roster just as completely as no cohorts at all. - The human bypass keys on is_bot, not `sender_agent_id is None`. agent_messages.agent_id is nullable, so a bot row with a NULL agent_id ingested by _poll_inbound_from_db would otherwise pass the gate as a human. Unattributable bot traffic now fails closed. - src/services/cohorts.py holds the decision logic as pure functions, so the engine and the admin preview cannot drift. Enforcement (§6) - has_new_reply_from_other is gated — the one read of eleven that was missed, and the one that let the scheduler prioritise exactly what the gate had rejected. - Every public MessageLog read carries a COHORT-GATE: GATED|UNGATED marker, with a test that fails when a new reader appears without one. - Writes are never gated: the log is shared by every agent in the process, so filtering at ingest would filter for all of them. Stated in code and in the spec. - Banked interesting_posts from non-permitted senders are pruned on resync. Private channels (§7) - A PI-created collab_private channel is exempt: an explicit human pairing outranks an admin-level grouping. Driven off the persisted LogEntry.visibility rather than the engine's in-memory channel map, so it is correct for rows ingested from another process and after a restart. Grandfathering (§8) - ThreadState.grandfathered. Open threads still get Phase 4 replies so they can conclude, but lose reactive priority. This is the normal path, not an edge case: the DB state rebuild runs before the first gate recompute, so every resumed run reconstructs its open partnerships cohort-blind. - _owes_reply skips grandfathered threads and reads through the agent's gate, so a non-cohort third party in an open funding thread cannot manufacture priority. Outbound hygiene (§9) - Mention stripping moved into _post_message, covering every outbound path instead of only Phase 5. Whole mention removed rather than de-@'d; unknown bot names left alone and logged at WARNING; per-agent strip counters. Scheduler (§10) - max_consecutive_reactive_turns default 8 -> 3. At 8, a single live pair took 24 of 27 turns. - turn_delay_seconds enforced as per-agent selection eligibility; the global asyncio.sleep that stalled Slack polling, DB ingestion and every other agent for one agent's cooldown is removed. - Reactive:proactive selection ratio logged every 100 picks. Admin (§12) — granular topology control - New agent x cohort matrix at /admin/cohorts/topology: every pair is a checkbox, column toggles, one audited save. Diffed against the cells that were rendered, so a partial or stale form can never delete a membership it did not display. - Per-agent "acts on" preview computed with the engine's own compute_gates. - Delete refused server-side while members exist, not just a disabled button. - Audit log on the detail page; every mutation writes an event. - Shared status banner stating what is actually in force, including a preflight override, and that filtering is forward-only. Provenance (§13.1) - Topology snapshots written to cohort_audit_events at run start and on every mid-run change, carrying the applied gate plus the counters the web process cannot see. Terminology (§1) - main already used "cohort" for date-bounded graph slices. Renamed those to run window (CABO_WINDOW_START, window_start_bound, window_posts) and replaced the dangling pointers to non-existent memory files. Testing - tests/unit/test_cohort_isolation.py (92 tests) relocated into the CI-gated layout and rewritten against v2; the old file asserted the inverted semantics. - tests/integration/test_cohort_admin.py (25 tests): real ASGI + Postgres + templates. - Full suite: 642 passed across unit/integration/characterization/contract against a real migrated Postgres, 13 golden-master snapshots unchanged. - Live migration audit on clones of the running database: fresh install, upgrade from the live DB's actual 0018, single head, downgrade/re-upgrade round trip, and a downgrade from a stamped-but-unapplied 0022 (the exact case that crashed the old migration). The live copi database was not modified. - An adversarial pass against this implementation found two real defects, both fixed with regressions: the preflight counted cohorts instead of live members, and the outbound whitespace tidy-up flattened indentation across the whole message, mangling code blocks and nested lists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit's engine tests were unit-level with fake DB objects: the real engine had never run against a real database with cohorts on. That left the claim "the DB conversation interface works with cohorting" unverified, and it is the claim that matters most. tests/integration/test_cohort_engine_live.py (47 tests) — real SimulationEngine, real committed Postgres rows, NullTransport (Slack off, the configuration where the DB is the sole conversation store): - The topology matrix: 10 shapes x 2 policies x the disabled case, evaluated through the engine rather than the helper. Empty, one empty cohort, solo, one pair, two disjoint pairs, overlapping, one big cohort, a hub in every cohort, partial cohorting, and a cohort whose only member is off the roster. Plus a symmetry invariant over every shape — an asymmetric gate would let one side monologue. - _poll_inbound_from_db ingesting rows written as if by another process, proving the shared log stays complete while only the per-agent read is filtered, and that a bot row with a NULL agent_id fails closed. - The resumed-run path: _rebuild_state_from_db + _rebuild_agent_state reconstructing a thread cohort-blind, then the first recompute grandfathering it. - _record_topology_snapshot actually writing a row (it is wrapped in try/except, so a broken write would only have logged a warning), and a mid-run topology change leaving a second snapshot. - A real Phase 2 with a scripted LLM: the excluded agent's content never reaches the prompt, and when everything is filtered no LLM call is made at all — the actual saving, measured at the boundary where it either exists or doesn't. - Phase 3 not activating on a non-cohort tag but still activating for a mate. - Phase 4 still answering a grandfathered thread while it stays out of the reactive tier — §8's central promise, driven rather than asserted structurally. - _post_message stripping a cross-cohort mention, read back from the persisted row. - PI DMs reaching the agent under a maximally closed gate (that path bypasses MessageLog entirely). Verification of the tests themselves: seven mutants introduced into the gate were all killed — reverting the policy inversion (9 failures), restoring the NULL-agent_id hole (4), dropping the private-channel exemption (4), letting grandfathered threads keep reactive priority (2), weakening the preflight to a cohort count (5), ungating get_new_top_level_posts (4), and removing the outbound strip (1). Also verified live: the full suite passes with SLACK_ENABLED both false and true (689 each, 13 golden-master snapshots unchanged), and the 0022 downgrade drops cleanly with real cohort rows present (1 cohort, 2 memberships, 1 audit event) and re-upgrades. New finding, documented in the spec and the admin banner: get_settings() is lru_cached, so cohort_isolation_enabled and cohort_default_policy are read once per process. Topology edits are live within ~30s; changing the flag or the policy needs an agent-run restart. Without this an operator flips the flag, sees nothing change, and concludes the feature is broken. Two harness bugs found and fixed while writing these tests, both of which would have made a test pass vacuously: the live engine did not register the message-log persist callback (so a row-level assertion read an empty table), and a SimpleNamespace stood in for Settings (so driving a real phase hit AttributeError rather than exercising real defaults). The harness now copies the real Settings object. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes the gaps named in the previous report, and finds one more. Blocked, not skipped: ANTHROPIC_API_KEY is present in .env but EMPTY, so no real API call is possible. tests/integration/test_cohort_real_llm.py is written and wired instead — 4 tests, marked real_llm, skipped without a key. Verified both ways: 4 skipped with no key; with a probe key all 4 execute and reach the Anthropic API (401 invalid x-api-key), so the plumbing is live and only the key is missing. It checks what a fake cannot: that a real model's OUTPUT is unaffected by gated-out content (marker-string proof, with an ungated control), that the outbound strip works on genuine model prose, and that a real scan response can only name post ids that survived the gate. Capped at 300 max_tokens on Sonnet, ~4 calls total. Browser testing via Playwright, 20 agents, 4 cohorts, real app instance on a separately-migrated database: - The matrix renders 20 rows x 4 columns = 80 cells with exactly 80 `present` fields. - The JS column toggle — previously untested — works: one real click checked all 20 boxes in a column, a second unchecked them, adjacent columns untouched. - A 19-membership topology built in the DOM and saved with a real click on Save returned "19 added, 0 removed", landed correctly in the DB, and the per-agent "Acts on" preview computed the right unions, including the two agents in two cohorts each (8 mates apiece) and the three uncohorted agents showing "everyone (gate off)" under policy=open. - Restarting the instance with policy=isolated flipped those three to "humans + PI private channels only" and named them in the banner — which also demonstrates the lru_cache restart requirement from the other side. - Clearing the matrix through the UI produced the red "switched on but forced OFF" preflight banner with its reason and the all-vs-all reassurance. That is the exact state that would have silenced the roster under v1. Multiprocess concurrency: three real OS processes against one Postgres — one writer churning memberships, two engines recomputing gates. 598 topology rewrites against 5,813 recomputes: zero errors, zero asymmetries, zero torn reads, no hangs. p50 2.6 ms, p99 ~9 ms, max 156 ms at 20 agents. NEW FINDING from that test. The safety depends on the membership write being atomic, and the dependency is invisible. A hostile writer that commits the wipe separately from the re-insert put ~57% of concurrent recomputes into the preflight-refused state — the gate fully OPEN for those ticks — versus 0 of ~4,500 with the single-transaction writer the shipped route uses. Any future bulk importer or seeding script that truncates then inserts would silently un-gate the roster about half the time under policy=isolated, and no code review would show it. Now pinned by test_matrix_save_writes_memberships_atomically and documented as normative in v2 §6.3.1, with the note that the failure is fail-open, never fail-closed — which is the right direction and also why it is easy to miss. Scale measured (v2 §14.6): the every-30s recompute is 2.3 ms p50 at 4 cohorts / 20 memberships and still 11 ms at 400 cohorts / 2,000 memberships. Free at any plausible roster; no caching warranted. Adversarial pass 3, 14 checks, all passing: cohort membership is correctly NOT transitive (A-B and B-C must not give A-C); 20 singleton cohorts leave every agent seeing only itself and none un-gated; all-20-in-one and every-agent-in-every-cohort both collapse correctly; the DB rejects duplicate (cohort, agent) at the constraint; SQL-looking agent_ids are inert data; all 190 unordered agent pairs symmetric over a random topology; and the admin preview matches the engine gate agent-for-agent on the topology that was saved through the browser. .notes/cohort-long-run-plan.md — three phases, ~500 calls, budget-bounded rather than time-bounded: a gate-OFF baseline (without which a quieter simulation is not evidence the gate works), a gate-ON run under policy=open, and a mid-run topology change to observe grandfathering under real conversational load. Includes the five blocking prerequisites (the empty API key and the live DB being three migrations behind its own code are both on that list), measured per-turn call costs, abort criteria, and the SQL that finds any cross-cohort thread. Full suite: 691 passed, 4 skipped (the real-API tests), 13 golden-master snapshots unchanged. Live copi database untouched at 0018 throughout; all scratch databases and the UI test container removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A working Anthropic key made two claims checkable that were not before — and both of the tests written for them were passing while proving nothing. Capturing the actual model output is what revealed it. Verified, not assumed: the configured model ids `claude-opus-4-6` and `claude-sonnet-4-6` are both valid and each resolved on a live call. Pricing for the cost model in the run plan: $5/$25 and $3/$15 per MTok. Vacuity #1 — the real-API test. It passed. With no profile loaded on the agent, the scan selected NOTHING in both the gated and ungated legs, so the gated assertion ("the model did not select the excluded post") held for the wrong reason and the ungated control demonstrated no difference at all. Rewritten around a profile that makes the excluded post directly relevant and the surviving post clearly irrelevant, which produces a measured behavioural difference: gate off -> model selects ["1000.0002"], reasoning cites the match explicitly gate on -> model selects [], having considered only the survivor That is the claim the feature rests on: ungated, the agent would have opened a thread and spent Opus calls on the excluded partner. A fake LLM can only show the prompt lacks the text. Documented as v2 §15.1 with the general rule — a test whose passing condition is an ABSENCE is vacuous without a paired positive leg showing the thing would otherwise be present. Vacuity #2 — the same mistake at experiment scale, in the live multi-turn run. Phase B (gate on) reported zero cross-cohort threads; so did Phase A, the gate-OFF baseline. Each cohort pair had been given matching interests and the pairs made mutually irrelevant, so even ungated the model correctly declined to pair chemistry with imaging. Redesigned: all four labs complementary on one problem (targeted protein degradation), cohorts drawn to cut across the strongest natural affinities (alpha=[su, wiseman], beta=[cravatt, lotz]), so the gate must block exactly the pairings the model most wants. Corrected two-phase run, real Opus/Sonnet calls, Slack off, one database per phase: A gate OFF: 18 msgs, 5 threads, pairs cravatt<->su, lotz<->su, lotz<->wiseman — all 3 cross-cohort, 4 SQL violations (the query catches leaks) B gate ON: 21 msgs, 4 threads, pairs cravatt<->lotz, su<->wiseman — 0 cross-cohort, 0 violations Under the gate the agents talked MORE (21 vs 18 on the same budget) but only to cohort-mates, and the two conversing pairs are exactly the two cohorts — the gate redirected collaboration rather than suppressing it, which rules out "the gate just made the simulation quieter." Verified twice: by the run script and by an independent SQL join of agent_messages to cohort_memberships. Also found: the first harness shared one database across phases, so one phase's setup deleted rows the other's still-buffering engine was about to flush. The engine re-queued those messages rather than dropping them — the H1 fix from the original audit working under a real mid-flight failure I caused by accident. Full suite: 694 passed with a key present (the 3 real-API tests execute rather than skip); 691 + 3 skipped without one. Live copi database untouched at 0018 throughout; all scratch databases dropped. The key was never written to any repo file — verified against the working tree, the git index, and git history. Not closed: Slack-on mirroring under an active gate. No bot tokens exist here (zero agents carry one, no SLACK_* value set), so every automated test uses NullTransport. The §13.1 topology snapshot is also not exercised by the run harness, which drives _recompute_allowed_sender_ids() directly rather than start(); that path is covered by test_topology_snapshot_is_actually_written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both were invisible to the existing suite, and both broke conversation
rather than leaking it — the gate was over-restrictive, not under.
1. compute_gates: under policy "open", "unrestricted" only held in one
direction. The uncohorted agent's own gate is None so it could act on
anyone, but every cohorted agent's gate is the union of its co-members
and so never contained it. It could open threads and never be replied
to. Uncohorted agents are now added to each cohorted agent's mate set,
which is what spec §5.1 says ("A has no cohort memberships, policy =
open -> Yes") and makes the relation symmetric. policy "isolated" is
unchanged: there, uncohorted still means excluded.
2. _post_message: never stamped `visibility`, so every agent-authored
message persisted as "public" even inside a collab_private channel.
Two readers depend on that field — the gate's private-channel
exemption (§7), which is how a PI pairing outranks an admin cohort
grouping, and the G2 memory filter that keeps private content out of
the public segment. The exemption was dead code and private content
was eligible for public memory. Now stamped from the channel class,
defaulting to public for unregistered channels.
Two of my own unit tests had pinned defect 1 (asserting the asymmetric
gate as expected) and are corrected here. The symmetry test skipped the
None-vs-set case, which is exactly how it missed it; it no longer skips.
The §7 test could not catch defect 2 because it wrote the AgentMessage
row with visibility pre-set, exercising only the read path — the new
test posts through _post_message and reads back what landed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YE9WxUMYvwhvfumLaC9irh
generate_agent_response's max_tokens retry doubled the cap once and then trusted the retry's text unconditionally, never re-checking stop_reason. A retry that ALSO truncates now logs loudly (agent/phase/output-token context) instead of silently returning a still-incomplete response — this matters most for the phase-5 opportunity assessment, whose <assessment_json> verdict sidecar is emitted last and would otherwise vanish with no trace. Separately, the retry is a second real, billed API call for a turn the caller already booked as one via agent.record_api_call() before the generate_agent_response call — so a retried turn silently undercounted the sliding-window rate limiter by one call, for exactly the agents (assessment turns) most likely to hit it. Adds an optional on_retry hook, fired exactly when the retry actually happens, and wires agent.record_api_call into it at every simulation.py/pi_handler.py call site that already books one call per turn. Purely additive: omitting the hook changes nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A truncated response whose whole body was <slack_message> tags stripped to "". _post_message had no emptiness guard, so it still minted a ts and wrote a LogEntry with content="" and slack_ts=None — a DB row with no Slack message behind it — while the caller counted the turn as published. Bail out before any state changes, and log why. _post_message now returns bool: True exactly when a message was recorded. The next commit makes the callers check it; without a return value the guard above would suppress the post and leave every caller's bookkeeping claiming success. bool, not `str | None`: blackbird widened the return to carry the post's slack_ts so an opportunity_assessments row could link back to it. org1 has no such table, so the id has no consumer here. Ported-from: 21869e2, 29fc8f1 (partial) Dropped: the assessment-sidecar parsing, _strip_assessment_sidecar, the verdict/gating persistence, and the oversized-field handling — all Blackbird product. The suppression comment is reworded accordingly: blackbird's is written around a _strip_assessment_sidecar call this branch does not have. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VzCmp8btW9Y6RnaQC9aRa
…discussions
agent_messages.agent_id is nullable, and it really is NULL in production:
_rebuild_state_from_slack records a real Slack message whose sender maps to no
known bot as is_bot=True with agent_id=NULL. Measured on the live database: 7
such rows, every one of them from the same raw Slack user id.
admin_discussions collected those into `available_agents` and then sorted() the
set, so a single NULL took the entire page down with
TypeError: '<' not supported between instances of 'NoneType' and 'str'
The replier and decision adds were already None-guarded; the poster's add was
the one that was not. All four are guarded now, in one loop, so the next person
adding a fifth source cannot reintroduce the asymmetry.
Not a regression from the post-type work — the sorted() call dates to e63daa1.
What surfaced it was the --fresh restart: re-importing Slack history under a new
run pulled those unmappable senders into the current run's scope, which is what
this page reads.
The regression test builds the exact production shape — one normal bot post and
one with agent_id=NULL, so the set is genuinely mixed rather than all-None — and
fails with the identical TypeError before the fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ted vote-tamper hole, and a false assessment-persist failure - admin_activity_detail: guard channel_stats[...]["agents"].add(msg.agent_id) against NULL agent_id the same way 73a78c3 did for /admin/discussions — Jinja's `sort` on that set crashed with the same TypeError, and this run is first in the Activity table, making it the most likely click in production. - admin_llm_calls: bound `page` with Query(1, ge=1) so `?page=0` gets a clean 422 instead of a Postgres "OFFSET must not be negative" 500. - proposal-vote details endpoint: require the stored voter_token to match even when the caller omits one — the old check short-circuited to False on a missing token, letting anyone with a vote_id overwrite or erase another visitor's comment. - discussions.html / activity_detail.html: render an honest placeholder instead of the literal "NoneBot" for messages with no mappable agent_id. - _persist_assessment: log the computed score with %s instead of %.2f so a legitimately-None score (the "no scores supplied" path) doesn't raise inside the success log line and get misreported as "Failed to persist assessment" after the row was already committed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e defects generate_with_tools has two internal max_tokens retry sites. Only one re-checked stop_reason, and neither reported the extra call, so a retried turn booked as one call against a limiter that had already counted it once. It is the function phase-4 thread replies use, which on this deployment is the whole product: a reply truncated after doubling max_tokens lost its closing </slack_message> with no trace in the logs. Both sites now fire the caller's on_retry hook and log at ERROR with the model, agent, phase and output-token count. simulation.py's thread_reply call site passes record_api_call, so the sliding-window limiter paces on real API calls. Ported-from: f32a83e (partial) Dropped: B1/B2/B3 — the /admin/assessments triage-queue run scoping, the derisking_milestones column, and the assessments.html styling. All Blackbird product. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VzCmp8btW9Y6RnaQC9aRa
_post_message returns False when nothing reached Slack, and no caller checked it: the phase-4 reply site, both phase-5 reply branches (private-channel flat follow-up, thread-creating reply) and the phase-5 new top-level post branch counted the turn, cleared pending-reply and backoff state, and moved posts between interesting_posts and active_threads for a message nobody ever saw. (On blackbird the new-post branch was guarded by 29fc8f1's caller hunk; the previous commit here took only that commit's return contract.) All four now check, and skip every one of those side effects when suppressed, leaving state exactly as if the turn had not been attempted. Phase 4 is the site that matters most here: collaboration replies are this deployment's whole product. Ported-from: e116feb, 29fc8f1 (partial) Dropped: the _extract_assessment_json newest-first rework and the three-way sidecar outcome logging at the phase-5 call site. Both are Blackbird product. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VzCmp8btW9Y6RnaQC9aRa
action_data.get("action", "new_post") turned a malformed phase-5 response into
a top-level post carrying whatever post_type and channel happened to parse.
Refuse the turn and log it instead.
Ported-from: 1b44e1c (partial)
Dropped: the <assessment_json> fenced-sidecar hijack guard and the downstream
verdict-persistence fixes (Blackbird product), and the phase-5 max_tokens
1000 -> 2500 increase. That ceiling was sized for scout_hub's eleven-section
assessment artifact plus its JSON sidecar; it is unconditional across all
roles, and on this deployment it is a 2.5x output-token increase with nothing
to spend it on.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013VzCmp8btW9Y6RnaQC9aRa
The filter at _build_lab_directories has always been correct, but start() ran it before _recompute_allowed_sender_ids, when every gate is still None — so it no-opped, and on a stable roster it never ran again. Production evidence: a spoke's phase-5 prompt named 51 unreachable labs, omitted its one reachable partner entirely, and the directory was 69% of the prompt. The directory is derived from the gate, so it is now refreshed on the gate's own cadence, including the paths that disable gating. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A post type declares the counterparty roles it addresses; availability is computed against the acting agent's live cohort gate. Gate None means no filtering, which is what keeps a mesh deployment unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Layer 1 rejects any post type outside a role's declared set, and runs even when the cohort gate is off. The vocabulary retires `idea` in favor of `idea_crosslab`, so a mesh deployment whose bind-mounted prompts still emit the old name would otherwise have every such post silently rejected. resolve_post_type_name() maps retired names on input only -- never in CANONICAL, a role's declared list, or a rendered menu. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the existing tools key, with the same never-raises degradation: an unknown type or malformed entry is dropped with a WARNING and the rest kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
build_phase5_prompt renders the role's declared post types through
render_menu() and substitutes the {post_type_menu} token via str.replace —
inert on a prompt that carries no token, which is exactly org1's case: its
prompts/phase5-new-post.md is frozen and tokenless, so nothing renders and
nothing changes. The mechanism lands so the coming cohort flip can enable a
menu without another port.
Ported-from: f2cbfe9 (partial)
Dropped: the four menu-presence tests
(test_phase5_menu_defaults_to_the_unfiltered_pi_lab_set,
test_phase5_default_menu_is_the_agents_own_role_not_pi_lab,
test_phase5_menu_uses_the_caller_supplied_text_when_given,
test_phase5_menu_survives_funding_only_surgery) — they assert the menu
renders, which needs the {post_type_menu} token only the excluded 0e1ac52
adds. Kept the token-absence and empty-enumeration guards, the two that hold
on a frozen prompt.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013VzCmp8btW9Y6RnaQC9aRa
…te failure Two [[post_types]] entries for the same name produced two contradictory specs while any by-name lookup silently kept only the last. parse_post_types now dedupes by name (last wins, first-occurrence order preserved) with a WARNING. _recompute_allowed_sender_ids now refreshes the lab directories even when the membership query raises, so a stale-but-correct gate does not leave a directory absent rather than merely stale. Pairs with the directory-after-gate reordering. Ported-from: 0a57e41, 10d598f (partial) Dropped: the body-mention rejection, the skip-backoff pre-reset capture, the tagged_agent near-miss normalisation, the _post_type_rejections counter and its admin banner row, and every prompt hunk. All of those exist only to serve the post-type enforcement this branch deliberately does not enable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VzCmp8btW9Y6RnaQC9aRa
…t tree src/agent/post_types.py's module docstring cites this document. Ported at its final state (d6bf5d7 as amended by a187a1d, 31cb20c and 454fa86) so the citation resolves, with a preamble recording that this branch takes the machinery and not the enforcement. Deliberately excludes docs/specs/2026-08-06-post-type-gating-prompts-draft/, which is blackbird's full prompt set including the scout_hub persona. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VzCmp8btW9Y6RnaQC9aRa
…ailers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VzCmp8btW9Y6RnaQC9aRa
…erride The rehearsal of the 0018->0024 migration found production one commit AHEAD of origin/copi-prod: 3e3c379 tracks docker-compose.override.yml (json-file log driver — without it every container dies at start under docker-compose.prod.yml's awslogs driver, because the copi-ec2-ses-role instance role lacks logs:CreateLogStream) and documents the prod compose file set in CLAUDE.md. The commit was never pushed, so Task 1's merge of origin/copi-prod could not see it, and deploying this branch would have deleted the tracked override — taking the whole stack down at the next `up -d`. CLAUDE.md auto-merges: cohort's Testing/roster sections and prod's "Compose file set" + baked-image restart procedure land side by side. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VzCmp8btW9Y6RnaQC9aRa
llm_agent_model and llm_agent_model_opus move to claude-opus-5 (from
claude-sonnet-4-6 / claude-opus-4-6). The ancillary sonnet knob
(email/grantbot/PI-DM classify) and the profile-synthesis model are
unchanged, as are all sampling params (none were set — Opus 5 rejects them).
Opus 5 thinks by default, and max_tokens caps thinking + response text
together. The agents' per-phase caps (300/1000/1500/2000) are deliberately
tight and pinned by the characterization golden masters, so every agent-path
call site now pins thinking={"type": "disabled"} — valid at effort high or
below — keeping today's exact token, cost, and latency envelope. The known
disabled-thinking risks were probed live before landing: at these exact call
shapes Opus 5 returned clean text at max_tokens=300 with zero thinking
blocks, and emitted a structured tool_use block (not text) on the phase-4
tools path.
Follow-up recorded in config.py: move to adaptive thinking + larger caps +
an effort level once the prompt freeze lifts, per the model migration guide.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013VzCmp8btW9Y6RnaQC9aRa
…d in the prod investigation Investigation of the dead reply-to-review flow (2026-08-11) found latent defects that would break or undermine the pipeline even once its missing AWS/DNS infrastructure is provisioned: - The SEC-5 anti-spoofing gate merged verdicts across ALL Authentication-Results headers with "a pass wins", so a sender-forged pass header overrode SES's fail verdicts. Now only the topmost header (the one SES prepends on receipt) is trusted, and it must carry the amazonses.com authserv-id. - HTML-only replies (no text/plain part) extracted an empty body and were silently dropped. Now fall back to tag-stripped HTML, with structural quote removal (blockquote/gmail_quote). - Auto-submitted mail (RFC 3834, e.g. out-of-office) was processed and could be answered with a help email - a mail loop. Now ignored. - MAX_REPLIES_PER_TOKEN_PER_HOUR was declared but never enforced. Now a sliding one-hour in-memory window per token. - A poison S3 object was retried every poll forever. Now quarantined to failed/ after 3 attempts for manual inspection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Prod sent 129 review emails telling PIs to "reply to this email to rate it" while ENABLE_INBOUND_EMAIL was off and the reply infrastructure (MX record, S3 bucket, receipt rule) did not exist - every PI who replied got silence plus an eventual bounce, which is the reported failure. Gate the reply-soliciting copy and the Reply-To header on settings.enable_inbound_email in the proposal-review reminder, the new-proposal alert, and the welcome email. When the flag is off, all three direct PIs to the web dashboard only, so outbound email can be safely re-enabled before (or without) provisioning inbound. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 2026-08-11 investigation found every infrastructure layer of the reply-to-review flow missing on prod: no MX record on reply.copi.science, no copi-inbound-email S3 bucket, no SES receipt rule, send-only perms on the copi-ec2-ses-role instance role, and ENABLE_INBOUND_EMAIL unset. scripts/setup_inbound_email.py --check reports each layer; --provision (admin creds) creates the bucket/policy/receipt rule and prints the DNS records and the IAM policy that must be applied by hand. docs/inbound-email.md is the architecture + bring-up runbook, including the ordered re-enable steps and the end-to-end verification procedure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 11, 2026
Open
Open
d4ee439 moved the default llm_agent_model from claude-sonnet-4-6 to claude-opus-5 along with the opus knob, which silently repriced every default-model call — phase-2 scan, phase-2 prune, memory synthesis, and the make_decision helper — onto Opus ($5/$25 vs $3/$15 per MTok). Those are the highest-volume calls in the loop and were never meant to move. The default is now claude-sonnet-5, cost-matching the prior Sonnet tier (same sticker; Sonnet 5's tokenizer spends ~30% more tokens on the same text, partly offset by intro pricing through 2026-08-31). Phase 4/5 keep claude-opus-5 via llm_agent_model_opus. The thinking={"type":"disabled"} pin carries over unchanged — Sonnet 5 also thinks by default when the param is omitted, and the tight snapshot-pinned per-phase caps rely on thinking output not competing for max_tokens. tests/unit/test_model_tiering.py pins the tiering: the config values, the model a model-less generate_agent_response/make_decision call lands on, and the thinking pin surviving the change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
Fixes for the reported failure that PIs are prompted to review proposals by replying to the notification email, but replies do nothing. Stacked on #30 (
org1-parity): merge #30 first — until then this PR's diff includes #30's commits. Only the threeemail-fixcommits are new here.Root cause (read-only prod investigation, 2026-08-11)
Outbound review emails worked (129 sent, each soliciting a reply), but the inbound half was never provisioned — a PI's reply could not even reach AWS:
reply.copi.sciencehas no MX record (only an A record to the EC2 box, no SMTP listener) → replies bounce back to the PI after their mail server gives up.copi-inbound-emaildoes not exist.copi-ec2-ses-rolehas no S3 permissions for the polling loop.ENABLE_INBOUND_EMAILis unset in the prod.env, so the worker never polls regardless.Outbound emails were later quieted by bulk-disabling
email_notification_preferencesrows in the DB (2026-08-06).What this PR changes
ENABLE_INBOUND_EMAIL(the direct fix for the reported harm): the proposal-review reminder, new-proposal alert, and welcome email only say "reply to this email" — and only setReply-Toto the reply domain — when inbound is enabled. With the flag off they direct PIs to the web dashboard, so outbound email can be re-enabled safely before (or without) provisioning inbound.Authentication-Resultsheaders with "a pass wins", so a sender-forged pass header defeated it (reproduced). Now only the topmost, SES-stamped header is trusted.MAX_REPLIES_PER_TOKEN_PER_HOURlimit is now actually enforced.failed/after 3 attempts.scripts/setup_inbound_email.py --checkreports each layer,--provision(admin creds) creates the bucket/policy/receipt rule and prints the MX record + IAM policy to apply by hand.docs/inbound-email.mdis the architecture + ordered bring-up runbook (provision → DNS → IAM → checks green → flag on + worker recreate → live end-to-end reply test → re-enable DB notification prefs).What this PR does NOT do
ENABLE_INBOUND_EMAIL.Testing
All fixes were written test-first (
tests/unit/test_email_inbound_hardening.py,tests/unit/test_email_reply_solicitation.py, 19 new tests). Full./scripts/ci.shgate green: 1672 passed / 120 skipped, branch coverage 65.84% (floor 60%), ruff ratchet 256/260.🤖 Generated with Claude Code