feat(pull): /chat on the durable queue for pull pilots (#3127)

[obasilakis]

Summary

The last push path on a pull-pilot agent. POST /api/agents/{name}/chat is called by MCP chat_with_agent, the agent-to-agent connector tool, and the trinity chat CLI. On a pilot it is now admitted onto the durable queue and claimed by a worker, so every interactive trigger is pull-owned on a pilot. The web UI chat panel already went through POST /task, which #3124 routes.

• Memory on a pilot is one Claude conversation per (agent, user) (operator decision on feat(pull): UI /chat on the durable queue for pull pilots #3127). Turns run through session_turn_service.run_resumable_turn with key session:chat:<chat_sessions.id>, resuming chat_sessions.cached_claude_session_id. That gives the ResumeLock, persist_session, and a cold retry on resume-not-found. Off a pilot /chat keeps the agent container's single shared session.
• Admission order is unchanged: depth guard, skill gate, idempotency begin, breaker read. A pilot then skips the slot acquire and audits a self-approved gate; the queued turn carries gate_checked=True, so the executor backstop does not gate a second time.
• Agent-to-agent /chat (trigger agent): run_resumable_turn opts into the claim phase (caller_waiting=True), the pull claim orders session:-keyed rows with interactive turns, and the collaboration activity rides the queue payload so the pull sink closes it. A full backlog answers 429 capacity.
• Post-turn work that /chat did inline runs in the caller after the terminal: assistant chat_messages row (secrets scrubbed), activity closes, the same response shape, idempotency complete, a timeout receipt + 504, and ResumeLockBusy (after the default 30s lock wait, which stays under the 60s no-session sweep) → FAILED + 429 capacity.
• CapacityFull from execute_task now carries error code capacity for every caller: a Session tab turn at capacity on push answers 429 (was 502), and fan-out reports capacity.
• GET /chat/history on a pilot serves the caller's session from the database. DELETE /chat/history closes the sessions /chat used and forgets their ids.
• New column chat_sessions.cached_claude_session_id (SQLite migration + Alembic 0091, re-parented by the 2026-10-06 merge train onto fix(operator-queue): platform alerts are conditions — one pending row per subject (#3246, part 1) #3255's 0090_platform_alert_subjects), which joins the session reaper's keep set. execute_task(chain_depth=) keeps the depth on a cold-retry row.
• Client disconnect: tested against the app's middleware stack, the /chat handler is not cancelled when the client goes away. The MCP 25s abort therefore leaves the queued turn running and its receipt valid.

Live verification (isolated sibling stack, real Claude turns, 3-worker pilot + non-pilot)

• /chat on the pilot: 3 turns from one user were pulled on one Claude session and recalled earlier numbers ("17, 29"). A second user got "NONE" on their own key and session. History returned each caller's own messages. A reset made the next turn start cold, and history is empty after it.
• Non-pilot: /chat was pushed with memory kept, as before.
• The same run covered feat(pull): route interactive producers onto the durable queue on pull pilots (#3114) #3124's Workspace paths: portal sync/async + memory, the stream hold, Stop while queued/running/right after claim, room serialization, and an unclaimed turn → FAILED "Agent at capacity". The issues it found are fixed on feat(pull): route interactive producers onto the durable queue on pull pilots (#3114) #3124 (a04d6e8ce) and here (883656752).

Known gaps

• Post-turn work (assistant message, cached Claude id) runs in the caller, so a 504 or a backend restart loses it. Converge with fix(pull): the pull sink applies post-turn delivery settings (#2329) #3227's sink-side delivery once it lands.
• cached_uuid is read before the resume lock, so two concurrent first turns both run cold and the second id wins.
• The Chat tab writes the same chat_sessions row: its "New chat" resets MCP/CLI memory, and the owner reset closes Chat-tab sessions.
• On a pilot a /chat turn gets the task-mode platform prompt, where push gives it the chat mode.
• output_tokens is not recorded on pulled /chat messages.
• GET /chat/session still reads the agent's shared session.
• After a cold retry, the MCP receipt lookup may point at the failed first row.

Test Plan

• cd tests && pytest unit/test_3127_pull_chat.py -v (21 tests; 11 key-behaviour mutations + the reset mutation go red)
• Flipped pins: test_1766, test_2391, test_3114_pull_route_interactive, test_2610
• Combined feat(pull): route interactive producers onto the durable queue on pull pilots (#3114) #3124 + feat(pull): UI /chat on the durable queue for pull pilots #3127 suites: 481 passed
• check_alembic_heads: one head (0090_chat_session_claude_id; now 0091_chat_session_claude_id after the merge-train re-parent)
• /cso --diff: 0 findings
• Review round 1: test_3127_pull_chat.py, test_3114_pull_route_interactive.py, test_2842_2843_pull_claim_order.py (72 passed); related guard suites show no new failures

Fixes #3127

🤖 Generated with Claude Code

[github-actions]

✅ Alembic head check clear — merging this PR into dev leaves one head (0090_platform_alert_subjects).

Previously flagged; resolved.

Advisory — this check does not block merge. · head_sha: 378318859e3c77a32b39be64490a06187982283f · run

[github-actions]

✅ Nightly unit-suite clean when this PR is merged into dev, all 3 seeds (head_sha: b8e24ee41b45792c313d1d948c021b785ddc8292).

[github-actions]

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.

[vybe]

merge-train (2026-10-04): not on this train. It rides the next one once fixed. This PR was not validated today because the diff will change with the rework below.

The branch is 45 commits behind dev and no longer merges:

• Conflicts in src/ against dev: db/migrations.py, routers/chat.py, routers/paid.py, services/capacity_manager.py, services/pull_pilot.py, services/task_execution_service.py, shared_sessions/service.py, and the portal files PortalConversation.vue, portalWork.js and portalWork.spec.js. These are a re-apply over code that moved, so they are yours to resolve rather than a train fix.
• add/add conflicts in tests/unit/test_3114_pull_route_interactive.py and test_3114_pull_route_workspace.py: feat(pull): route interactive producers onto the durable queue on pull pilots #3114 has since been squash-merged, so the stacked copies collide with dev's.
• Alembic fork (the red alembic-head-watch): 0086_chat_session_claude_id has down_revision = "0085_ent720_email_identity", the same parent as dev's 0086_metric_points_restatement. dev's head is now 0088_skill_gate_requests, so after merging dev this revision becomes 0089_chat_session_claude_id on top of it. python3 scripts/ci/check_alembic_heads.py src/backend/migrations/versions should report one head.
• feat(skills): gated skills — the approval comes before the agent sees the request (abilityai/trinity-enterprise#751) #3208 (gated skills) merged today and touches 17 of this PR's files, including chat_execution_service.py, dispatch_admission_service.py, public_chat_service.py, task_execution_service.py, message_router.py and routers/internal.py. Its gate reads request_text at the /chat and /task admission seams, so the new durable-queue /chat path needs to pass through them.

I've set status-needs-fix; your next push clears it.

[vybe]

merge-train: not on today's train. It rides the next one once fixed.

Merged with current dev, regression diff fails tests/unit/test_ent751_gate_guards.py::test_every_producer_passes_the_requesters_own_words_or_is_named_unwrapped. The guard landed on dev after this branch was cut, so the PR head alone passes.

producers that compose a message without `request_text=` and are not named unwrapped:
[('services/chat_execution_service.py', 'run_pulled_chat_turn')]

The new run_pulled_chat_turn calls session_turn_service.run_resumable_turn(...) (~chat_execution_service.py:1007) with neither request_text= nor gate_checked=True. Picking one is a skill-gate (ent#751) decision, which is why this went back to you instead of getting a mechanical fix:

• If the /chat router's skill_gate_service.enforce(...) (~:2374) always runs before the pull branch, pass gate_checked=True, as the push path does.
• Otherwise pass request_text= with the same joined message, user_message and system_prompt that the router gates on.

Adding it to _UNWRAPPED would be wrong. /chat carries the requester's own words, which is exactly what the gate must read.

Reproduce: merge origin/dev into the branch, then run pytest tests/unit/test_ent751_gate_guards.py.

[AndriiPasternak31]

Thanks for this. The pull routing is clean: I couldn't find a double-execution path (run_chat_turn branches only on capacity is None, and execute_task re-decides with the same env-only predicate). The only caller-side terminal write is on a never-enqueued RUNNING row, so it can't race the token-gated sink, and the slot and idempotency accounting balance. Tier 1 is green and check_alembic_heads passes. I ran test_3127 + the flipped pins (164 passed) and the 1804/1578/2806/2842/2889/ent751/3114 guards (297 passed).

Two things I'd like fixed before merge. Both are invisible to the suite because dispatch_and_await_terminal is mocked in every turn test:

1. Skill gate runs twice on a pilot /chat (ent#751). The comment at chat_execution_service.py:1011 says "/chat runs no admission-seam gate", but admit_chat_request calls skill_gate_service.enforce at dispatch_admission_service.py:326, before the pilot branch at :387. Without gate_checked=True, execute_task's backstop gates again with a requester rebuilt from the row (requester_for_dispatch), and that requester never has is_person=True. I checked this with the real enforce: the admission requester self-approves the owner, and the backstop requester does not. So on a pilot, an approver's own gated request turns into a pending approval: the row is SKIPPED, a second ask is raised, the caller gets a 202, and the user message has no reply. The pilot branch also returns before audit_self_approved (:431). Fix: pass gate_checked=True (keep request_text), call audit_self_approved in the pilot branch, and assert gate_checked in test_pilot_turn_dispatches_through_the_queue. This is the first branch of vybe's 10-05 note.

2. Agent-to-agent /chat (trigger agent) gets no claim priority and no claim budget. "agent" is not in INTERACTIVE_TRIGGERS, so _CLAIM_WAITING_TRIGGERS (task_execution_service.py:720/811) skips the phase-1 wait. The turn queues behind batch work, and the only bound is timeout + 120s from enqueue. With a deep queue that ends in the 504 queued_timeout receipt while the row is still queued. It then runs for nobody: no assistant chat_messages row and no cached Claude id. The collaboration activity also stays started until the 120-min backstop, because build_pull_queue_payload sends collaboration_activity_id=None. That's the #1804 symptom. The a2a note in pull_pilot.py states the rule: a blocked caller earns priority and the budget. Could the pulled /chat opt into claim semantics explicitly (rather than by trigger), and could the collaboration activity id ride the payload so the sink closes it? A test with the real adapter for trigger agent would pin it.

Smaller items, non-blocking:

• Post-turn work runs in the caller, so it's lost on a backend restart or a 504. Issue scope item 3 said "moves to the terminal", and #3227 puts post-turn delivery in the sink. No textual conflict between the two PRs, but worth converging once #3227 lands (carry chat_session_id and collaboration_activity_id in the metadata).
• Same-user concurrency: the ResumeLock waits 30s and then the call gets a 429 and the row is FAILED. Agent keys resolve to the owner, so every agent of one owner shares one session with the target, and parallel chat_with_agent calls fail where push serialized them. Consider a pilot lock wait of at least one turn, as rooms do.
• cached_uuid is read before the lock (:1005), so two concurrent first turns both run cold and the second overwrites the first's id.
• Lock-busy path: the update_execution_status bool is ignored, no agent.task.failed is emitted (MCP's receipt goes out at 25s and this FAILED lands at 30s), and the 429 has no X-Trinity-Error-Code: capacity.
• Backlog full on a pilot /chat maps to 503 with no code, because "Agent backlog full…" doesn't match "at capacity". Push answered 429/capacity.
• The Chat tab writes into the same chat_sessions row, so "New chat" in the UI resets MCP/CLI memory, and the owner reset closes Chat-tab sessions.
• Docs: the chat_with_agent description in mcp-server/src/tools/chat.ts:395-399 still says the session is shared by every caller and restarts on a model change. persistent-chat-tracking.md, mcp-orchestration.md and agent-to-agent-collaboration.md aren't updated. The PR body and the CSO report still say Alembic 0086 and "stacked on #3124".
• Alembic: #3255 and #3256 also add 0090_* off 0089_supersede_queue_flood_backlog. The tables are disjoint, so this is a mechanical re-parent for whoever lands second.

[vybe]

merge-train (2026-10-06): riding this train. Mechanical changes pushed to your branch:

• 2082b2f24 re-parents the Alembic revision: 0090_chat_session_claude_id → 0091_chat_session_claude_id, down_revision = "0090_platform_alert_subjects" (fix(operator-queue): platform alerts are conditions — one pending row per subject (#3246, part 1) #3255's). fix(operator-queue): platform alerts are conditions — one pending row per subject (#3246, part 1) #3255, this PR and fix(workspace): a sent message shows its attachments, and keeps them after a reload (#3265) #3269 each forked an 0090 off 0089 (the bug: enterprise submodule pin advance lands two Alembic heads — entitled instances silently degrade to OSS-only #2068 two-heads class). The tables are disjoint, so a re-parent was enough. The SQLite entry is keyed by name and is unchanged; the two doc references to the id are updated.
• PR body: Fixes abilityai/trinity#3127 → Fixes #3127, because the cross-repo form of a same-repo issue skips status-in-dev promotion (docs: architecture area files are read deliberately — drop the unwired PreToolUse hook (abilityai/trinity#2642) #2644). The Alembic numbers are also updated.

Expected until #3255 lands: schema-parity reads two heads on this branch alone, since the parent lives on #3255. It goes green on the dev merge pushed after #3255 merges. A local DB stamped 0090_chat_session_claude_id needs alembic downgrade 0089_supersede_queue_flood_backlog && alembic upgrade head.

The validation found both blocking review items resolved by 7902fe83a (mutation-checked: dropping gate_checked, caller_waiting or the payload's collaboration_activity_id each turns a test red). Two non-blocking items are still open as follow-ups: cached_uuid is read before the lock, and the Chat tab and MCP/CLI share one session row.

Merge train 2026-10-06: both blocking items verified resolved by 7902fe8 (mutation-checked: gate_checked, caller_waiting, collaboration_activity_id each red when removed). Dismissed per the operator's Gate 1 call; open items are posted on the PR.

[AndriiPasternak31]

Thanks for the quick turnaround. I re-checked against b8e24ee. The current head 2082b2f only renumbers the migration (0090 → 0091), so all of this still applies. @vybe, this is on today's train. Could you hold it until the item below is fixed?

Both blockers are fixed, and the tests have teeth:

• Skill gate runs once: gate_checked=True on the pulled turn, and the self-approval now rides admission.gate into the row's setup (ent#752). Reverting either one fails a test.
• Agent-to-agent /chat: caller_waiting=True gives it the claim phase, the session: prefix gives it interactive claim order, and the collaboration activity id reaches the queued row, where #3227's sink delivery closes it. Only the CAS winner closes it. The caller's later close is a no-op under the #1804 lattice. Session tab and Workspace already wait for a claim (session/public triggers), so caller_waiting doesn't change them.
• Lock-busy CAS/event/429-capacity and backlog-full → 429 are fixed too. Alembic is one head on the merged tree. The merge resolution with #3227 looks right: no double chat-message save.

One thing I'd like changed before merge:

• The longer resume-lock wait runs into the #106 no-session sweep. While a second turn of the same session waits, its admission row is running with no lease and no Claude session, so the 5-minute cleanup fails it after 60s ("Silent launch failure"). When the lock frees, the enqueue CAS refuses the row and the caller gets 429 "Agent backlog full" for a turn that never ran. I reproduced the sweep half against the real db layer. Before this round the same case got an honest 429 after 30s. Either keep the default 30s wait for /chat, or keep the waiting row out of the sweeps (create or enqueue it after the lock). The queue's per-conversation claim guard already serializes execution.

Smaller, non-blocking:

• The merge took dev's shorter chat_with_agent text and dropped your pilot note. chat.ts still says the session is shared by every caller and restarts on a model change. The parallel parameter description isn't under the 2,048-char cap, so the note can go there.
• execution.md still says admission audits the self-approval on the pilot branch. Since the merge, the row's setup does that.
• The CAPACITY code on CapacityFull now applies to every execute_task caller (e.g. the Session tab at capacity on push becomes 429 instead of 502, and fan-out reports capacity). Probably right, but worth a line in the PR body.
• Three links have no test that fails when they're reverted: execute_task forwarding collaboration_activity_id into the payload, caller_waiting on the cold retry, and the backlog-full producer setting the code. A test that pushes an agent /chat row through apply_task_result and checks the activity ends completed would cover the first.
• The assistant message and the Claude id are still lost on a 504 (documented). Fine as a follow-up.

[obasilakis]

@AndriiPasternak31 addressed in b829ce1:

• Lock wait vs bug: Cleanup service misses 'skipped' executions and slow to detect no-session failures #106 sweep: /chat on a pilot keeps the default 30s resume-lock wait. The lock_wait plumbing is gone. test_lock_wait_stays_under_the_no_session_sweep pins the wait to the default and LOCK_WAIT_TOTAL_SECONDS < NO_SESSION_TIMEOUT_SECONDS.
• chat_with_agent parallel description: notes per-user sessions on a pilot (description-budget test passes).
• execution.md: the self-approval audit rides admission.gate into the row's setup.
• PR body: line on CapacityFull → capacity for every execute_task caller.

Merged dev (clean). check_alembic_heads reports two heads on this branch until #3255 lands, as the train note says.

Not done here: tests for the three unpinned links (payload collaboration_activity_id, caller_waiting on cold retry, backlog-full code). Open to adding them in this PR if you want them before merge.

[AndriiPasternak31]

Re-checked at b829ce1, plus the merge-train merge 3783188. F1 is fixed. Approving.

F1 (lock wait vs the #106 sweep)

• /chat on a pilot is back to the default 30s resume-lock wait. The no-session sweep and the watchdog both leave rows younger than 60s alone, so a second turn on the same session now gets the honest "another turn is in progress" 429 at ~30s, before either of them can touch its row.
• I ran this against the real db layer: real ResumeLock (fakeredis), real mark_no_session_executions_failed every 2s, real enqueue CAS.
  • b829ce1: 32s, 0 rows swept, lock-busy 429.
  • With the fix reverted: 72s, the row is reaped as "Silent launch failure", then a capacity 429.
• Reverting the fix turns test_lock_wait_stays_under_the_no_session_sweep red. It checks the wiring plus a constant (30 < 60), which is enough for this fix.
• C1/C2 and the lock-busy CAS/event/429 path are unchanged. All 16 of my round-2 mutations give the same results as before.

Migrations

• #3255 is in, and 3783188 resolves the SQLite MIGRATIONS tail keeping both entries. On the Alembic dir at 3783188, check_alembic_heads gives 92 revisions and 1 head (0091_chat_session_claude_id). schema-parity, pg-migrations and regression diff were red only because 0090 was missing, so they should clear on this run. CI was still running when I checked.

Non-blocking, fine as a follow-up (no need to hold the train for them):

• Three links still have no test that fails when they're reverted: execute_task forwarding collaboration_activity_id into the payload, caller_waiting on the cold retry, and the backlog-full producer setting error_code=CAPACITY. There is also no test that runs a queued agent /chat row through apply_task_result and checks that the collaboration activity ends completed.
• chat.ts: the pilot note comes right after "The session restarts after a model change". On a pilot the cached Claude id survives a model change, so that sentence doesn't apply there. Something like "On a pull-pilot agent each calling user has their own session, which a model change does not restart."
• PR body: "Converge with #3227 … once it lands" is stale (#3227 is in).

[vybe]

merge-train: batch validated on train #3279 (green); dev re-merged after #3255, one Alembic head.