Skip to content

feat(gateway): an agent asked a room question answers it once, and the gate reads only its own task [AgentActivity] - #5937

Closed
marklysze wants to merge 156 commits into
sonichi:mainfrom
marklysze:feat/agent-answers-room-question
Closed

marklysze wants to merge 156 commits into
sonichi:mainfrom
marklysze:feat/agent-answers-room-question

Conversation

@marklysze

@marklysze marklysze commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #5913 (which is stacked on #5885 and #5883). Merge order: #5883 → #5885 → #5913 → this. This PR's own change is 04f524a8a plus its changelog line ef73d3413. After #5913 lands, I'll merge main into this PR and rerun its full checks.

Through its parents it carries #5885's wake-turn-only Stop-hook handling and #5913's SUTANDO_STOP_INTERRUPT switch (off by default). This PR's own answer path is behind REMOTE_TASK_TURN_CONTEXT=1, and today's broker sends no room_question.

Why

AgentActivity ruling O8 lets agents answer room questions. The broker now adds task["room_question"], and a [AG2 Space room question; …] block inside the body, when an agent is asked. Before this PR:

  • the structured field was dropped by _write_task, because it is not in _TASK_FIELDS;
  • nothing told the agent how to answer, or which question_id to answer;
  • the turn gate read applied_execution_phase and active_blocker_count from the latest outcome whatever task it described. An answer outcome that ever named the asker's task (for example input_ready) would have parked the answering agent's own reply as waiting_input + no_send. Backend ruling R2 says an answer outcome describes only the caller's own task; this guard is defensive.

What

  • Bridge. room_question joins _TASK_FIELDS and is written as one line of quoted room data, carrying only the pinned keys: question_id, expected_revision, prompt, options, allow_free_text, response_mode, target_members, expires_at. It uses JSON with escapes and stays one line. A missing or unsafe question_id writes nothing.
  • Answer rule. This applies under REMOTE_TASK_TURN_CONTEXT and only when can_answer is not false. It is a writer-owned line beside the ask-in-room rule. It tells the agent to:
    • take expected_revision and can_answer from room.question.inspect;
    • call room.action.execute_safe with action room.question.answer, top-level task_id, and arguments question_id, expected_revision and response ({"kind":"choice","option_id":…} or {"kind":"text","text":…}), once;
    • then write a [no-send] one-line result, so the answer is not also posted as a chat reply.
  • Gate. classify_turn and result_file_decision take the task id. An outcome or reconciled record naming another task_id neither suspends nor settles this task. An outcome without a task_id still counts as the turn's own.
  • Wake. A resolved blocker that carries an answers list renders each answer instead of answered "". The single answer (B13) is unchanged.

Evidence

python3 tests/… at the parent a20f02d93 (#5885) and at HEAD:

# before
$ python3 tests/sparrow-task-attempts.test.py
ERROR: test_a_room_question_renders_as_one_line_of_quoted_room_data
ERROR: test_the_answer_rule_needs_a_question_this_agent_may_answer
FAIL: test_render_wake_renders_an_answers_list_answer_by_answer
Ran 32 tests in 0.049s
FAILED (failures=1, errors=2)
$ python3 tests/agent-turn.test.py
ERROR: test_another_tasks_input_ready_does_not_suspend_this_one
ERROR: test_another_tasks_terminal_or_reconciled_outcome_does_not_settle_this_one
ERROR: test_its_own_input_ready_still_suspends_it
Ran 33 tests in 0.068s
FAILED (errors=3)
$ python3 tests/gateway-agent-activity-hooks.test.py
FAIL: test_a_room_question_reaches_the_task_file_with_its_answer_rule
Ran 45 tests in 0.731s
FAILED (failures=1)

# after
$ python3 tests/sparrow-task-attempts.test.py
Ran 32 tests in 0.051s
OK
$ python3 tests/agent-turn.test.py
Ran 33 tests in 0.085s
OK
$ python3 tests/gateway-agent-activity-hooks.test.py
Ran 45 tests in 0.722s
OK

The new cases:

  • another task's input_ready does not suspend this one, and the reply posts as completed;
  • this task's own input_ready, tagged or untagged, still suspends;
  • another task's terminal or reconciled outcome does not settle this one;
  • the room question renders as one line with only the pinned keys, escapes newlines and quotes, and an unsafe id renders nothing;
  • the answer rule names execute_safe, room.question.answer, the task id and the question id; it is absent when can_answer is false;
  • an answers list renders answer by answer;
  • _write_task writes the data line and the rule, only the data when can_answer is false, and neither for an ordinary task.

The bridge suites pass, with the same two local-only Discord failures as main. ruff, scripts/review-checks.sh and the vendor sync check are clean. All new bridge lines are covered.

Live: the agent-answers path is not yet exercised on the rig; covered by unit tests and the MCP replay of captured question tasks.

Adds src/agent_turn.py (vendored into ag2-sparrow): the 0400 turn file at
state/turns/<task_id>_g<gen>_<ulid>.json, its .closed fence, the journal
reader for dispatch_intent/dispatch_outcome records, and classify_turn.
A plain-text answer may be sent only when the turn has no bound intent, no
unbound intent since it began, and core-api confirms zero operations.
…gate on results

Behind REMOTE_TASK_LIFECYCLE_V2 (default off) the bridge sends one lease-holder
worker_id and protocol_version 2 on ack, records the execution_generation the
ack reports, beats each live attempt on POST /v1/tasks/heartbeat, and puts the
generation and worker_id on every result. A fenced attempt (task_cancelled,
stale_execution_generation) stops the core through a CANCEL_INSTRUCTION task and
is cancel-acked; its late result is settled, never posted. Every five minutes
GET /v1/tasks/active is reconciled with the in-flight set.

With REMOTE_TASK_TURN_CONTEXT too, each attempt's turn file is written on ack and
closed when its result settles, and the result is gated on the turn's journal:
skipped when the task closed terminally through MCP, posted as waiting_input +
no_send while suspended, and posted with no_send when the same text was already
sent as a room message. A result 409 that ends the task, or a 400
waiting_input_without_blocker, settles the item instead of quarantining it. A
re-served task carrying `wake` is left unacked rather than closed as a
redelivery.
…ask names

The coverage gate measures tests/**/*.test.py in process: the package test moves
to tests/sparrow-task-attempts.test.py, the bridge hooks get an in-process suite,
and the vendored agent_turn.py copy is omitted like the other src/ copies. Two
cancels in one millisecond no longer share a control-task name, and a recorded
acked_at of 0 is read as itself.
…er turn context

With REMOTE_TASK_TURN_CONTEXT, a task re-served with `wake` after its question was
answered runs once per blocker_episode_id: it is queued again under the same id
with each answer appended to the task's line as quoted room data, acked at the
same generation into a new turn, and its result rides its own outbox item
`<id>.wake-<episode>`. The earlier attempt's archived copies no longer settle it.
Each task file also carries one writer-owned line telling the agent to ask a
question that blocks the task with room.question.ask on its exact task_id.
…urn-gate

# Conflicts:
#	packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py
…re, a corrupt journal is unreadable

Under REMOTE_TASK_LIFECYCLE_V2 a result written before its task's ack named the
lease waits for that ack (at most 120 s), and records the proxy could not bind but
that name the task count as the attempt's turn from publication on, so neither
the lease fields nor the journal gate can be skipped by a fast core. Every fencing
error_code on either ack path (404 or 409) fences the attempt, retires its pending
ack and, for a published task, stops the core, except a stale older lease whose
newer serve is pending here. A journal line that does not parse makes the journal
unreadable instead of absent; an unterminated last line younger than 30 s defers
the result. A served task carrying `wake` is left unacked whether or not this node
saw its first attempt. A late ack for a task no longer queued opens no turn.
…pt to settle and always runs with its answer

Merges feat/agent-activity-turn-gate. A served `wake` is decided before any local
shortcut: run with its answer under turn context, or left unacked (turn context
off, ack_first, an episode already run, or no usable episode), whether or not
this node saw the first attempt. A wake served while an earlier attempt of the
same id is still settling here is left unacked until it is served again, under a
lock the settle also holds, so that settle can never retire the wake. Finishing
an attempt checks it is still the one the result belonged to, and opening a wake
starts a fresh turn.
…edelivery keeps the journal boundary

On a broker that has ever named a lease, a result whose ack never answered keeps
waiting until the ack lands, the attempt is fenced, or the server closes the task.
On a broker never seen to name one, the 120 s fallback is gated on the journal
records that name the task, so an early send, terminal action or blocker still
rules. A task served again before its ack keeps its first publication time, so
its earlier unbound room actions stay inside the turn's journal window.
…on every settle branch

The normal, skip-marker, dedup and orphan-recovery branches archive a result and
then retire its id (in-flight removal, room and media sidecars). Each now runs that
whole transition under the wake handoff lock and marks the id retiring, so a wake
served in that window, on another thread or this one, waits to be served again
instead of being published and then losing its tracking.
…-wake

# Conflicts:
#	packages/ag2-sparrow/ag2_sparrow/task_attempts.py
…r reconcile from its first publication

A result held because its ack never named the lease on a broker that names leases
is now released by the reconcile too: once the complete active listing omits the
task past the grace, the attempt is fenced, the core stopped, and the result
settled without a post.
…nd a settle never archives an open wake

The no-live-result recovery branch now judges disposal inside the same locked
retirement step as its settle, so a wake admitted after the judgement waits to be
served again. A settle also re-checks inside the lock: while a wake for the id is
open here and its own result is not delivered, nothing is archived or retired,
whatever the caller judged before it took the lock.
… every held task beats

A task_cancelled in a per-task beat no longer fences the attempt. The core gets one
CANCEL_INSTRUCTION task; once it has handled it, a result it wrote for the task is
posted normally with no cancel-ack, and with no result the stop is confirmed on
cancel-ack and a later result is settled. A task_cancelled on the ack still means
the task is not started and is cancel-acked at once. The per-task beat now runs on
its own 60 s cadence ahead of the client heartbeat's backoff, so a Stop reaches
every task the bridge holds for as long as it holds it.
…-wake

# Conflicts:
#	tests/gateway-agent-activity-hooks.test.py
… stopped, not no_action_needed

While a Stop is pending, a core result that is no_send or empty confirms the
Stop with a cancel-ack and is settled locally; it is never posted as
no_action_needed. A failed cancel-ack holds the result for the next pass. A
result with a body is still posted (a finished reply is never dropped).

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed 5c90bb0 (agent-assisted, Blue for Chi's seat).

  • (a) Fixed: the answer-outcome filter falls back to the intent's action, so an action-less answer outcome no longer suspends the answering turn, while your own ask still does.
  • (c) Fixed: the census anchor re-points to remote_gateway_bridge.py. census-d1-anchors and --verify both pass.

Still open:

  1. Inherits #5885's gap: a reconciled-failed ask posts the "I asked X" note as completed.
  2. "Answered once" is still partial. answered_in_journal is keyed by task_id, so a different task carrying the same question still gets the full answer rule after a crash. A single task offered a second question would wrongly get the already-answered note. Key it on question_id (or both). Nothing in the engine refuses a second room.question.answer; if the server's expected_revision is the real guarantee, say so in the rule's docstring.

agent-turn (47) and gateway-agent-activity-hooks (67) pass locally.

An ask whose outcome is a failure, in the journal or from reconciliation, means
no question exists, so the result posts failed with no_send instead of the note
as the completed reply. An ask whose outcome is still unknown keeps posting
waiting_input.
… on the question too

Before any settle, a question counts as answered here only when this task was
offered that question and its journal holds a succeeded answer. The docstring
states that the server's expected_revision guards only a concurrent answer.
@marklysze

Copy link
Copy Markdown
Collaborator Author

Fixed in e3552db (head e3552db, which merges #5885's failed-ask fix up through #5913).

  • (1) Inherited from feat(gateway): resume a woken task as a new turn and ask in the room [AgentActivity] #5885 via the merge.
  • (2) answered_in_journal is now keyed on both: before any settle, a question counts as answered only when this task was offered that question (answer_offered == question_id) and the task's journal holds a succeeded room.question.answer. After a settle, the record is by question_id. A different question on the same task still gets the rule (test).
  • The answer_rule docstring says that expected_revision guards only a concurrent answer: the rule re-reads it from room.question.inspect before answering, so it does not stop a second answer; the local record is the guard.

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at e3552db (agent-assisted, Blue for Chi's seat). The journal check now also requires answer_offered == qid, so a second question on the same task gets the full rule (the test covers it), and the docstring now says plainly that this record, not expected_revision, is what stops a second answer. It inherits #5885's failed-ask fix.

One known limit, not blocking: before the first settle, a different task carrying the same question still relies on the per-task journal, so a crash in that window can repeat an answer. agent-turn and sparrow-task-attempts pass locally.

@keweichen keweichen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at current exact head e3552dbf4073cec13bf6acb57381e86d13370006 — not ready to merge. The requested 5c90bb09… snapshot moved during review; this verdict covers the current head.

The action fallback, failed-ask inheritance, and same-task/different-question correction are verified. Focused suites pass: agent-turn 47, task-attempts 41, gateway hooks 67.

Blocker 1 — “answered once” still has a cross-task crash window. Before the first settle writes the answered-ID record, the journal check requires the current task’s answer_offered == question_id. After task A successfully answers q1 in the journal, a different task B carrying q1 still receives the full answer rule; meanwhile A carrying q2 is now correctly allowed. No documented backend contract forbids rematerializing q1 under B, and the engine itself does not refuse the second call. Either make the pre-settle lookup follow q1 to the task that offered it (or record qid in the durable journal), or document and pin the backend invariant. Worst case is a duplicate room answer after a crash.

Blocker 2 — the real restarted gateway/MCP witness is still absent. Please show current HEAD receiving the one-line room data, inspecting and executing one answer at the expected revision, writing local [no-send], emitting no echo, and replay/redelivery adding no second answer.

This head also inherits #5913’s canonical-socket defect and #5885’s wake-mapping recovery defect. CI was still running with no completed failures at review time; CLA passes.

…ves flight

finish() forgot the wake item as soon as the wake's result was confirmed, before
the caller archived the still-live result. A restart (or a failed archive) then
re-offered that result through the first attempt's delivered item, which
quarantined it to undelivered. The item is now kept in relay-wake-items until
the startup sweep sees the task out of flight, and a re-offer through a settled
wake's own item is recognised as delivered rather than re-sent.
…legacy name

_socket() read the legacy SUTANDO_TMUX_SOCK first. The launchers and
sutando-config.sh tmux-socket use SUTANDO_TMUX_SOCKET, so it now wins when both
are set, then the legacy name, then the config helper.
…every task carrying it

The journal names tasks, not questions, so answer_rule now keeps which tasks
were offered each question (relay-answer-offers, bounded). A task carrying a
question gets the already-answered note when any task offered that question has
a succeeded answer in its journal, before any settle; after a settle the record
by question id still rules.

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving at 6b4182b (agent-assisted, Blue for Chi's seat). Each question now records which tasks were offered it (under _MUTEX, capped at 8 per question), and a succeeded answer in any of those tasks' journals counts as answered. That closes the cross-task crash window I noted. A question no answering task was offered still gets the rule. One nit: answered_in_journal is keyed by task, so if an offered task also answered a different question, that would count too. That's fine while a task carries one question. sparrow-task-attempts passes locally.

@marklysze

Copy link
Copy Markdown
Collaborator Author

Cross-task answered-once: fixed in 6b4182b (head 6b4182b).

  • The proxy's journal names tasks, not questions, and records written by desktop can't carry question_id from this repo. So answer_rule keeps which tasks were offered each question (relay-answer-offers, bounded).
  • Task B carrying q1 gets the already-answered note as soon as any task offered q1 (say A) has a succeeded room.question.answer in its journal, before any settle. After a settle, the record by question id still rules.
  • Test (test_a_redelivery_after_a_crash_sees_the_answer_in_the_journal): A offered q1 and answered, with no settle. A redelivered → note; B with q1 → note; B with a question no answering task was offered → the rule.
  • Recording question_id in the journal itself would be a desktop-proxy change.

@bassilkhilo-ag2 bassilkhilo-ag2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at 6b4182b (delta from e3552db: 6b4182b plus the merge of #5913, reviewed there). The cross-task claim holds. One non-blocking regression note below. Verdict: not approving yet, because the REVIEW.md lesson 15 live witness is still missing.

Verified:

  • task_attempts.py:190-210 answer_rule keeps relay-answer-offers (qid -> up to 8 offered tasks, under _MUTEX). Task B carrying q1 gets ANSWERED_HERE_NOTE once any task offered q1 has a succeeded room.question.answer in its journal, before any settle.
  • test_a_redelivery_after_a_crash_sees_the_answer_in_the_journal fails on e3552db's task_attempts.py (rule returned instead of the note) and passes at head. sparrow-task-attempts 41, agent-turn, gateway-agent-activity-hooks and stop-interrupt all pass.
  • CI: 39 pass, 3 skipped. Mergeable.

Non-blocking (a regression from e3552db, reachable only if one task is ever offered two questions):

  • answered_in_journal (agent_turn.py:346) is per task, not per question. Once task R has answered q1 and is then offered q9, R joins offers[q9], and every later task carrying q9 gets the already-answered note even though q9 was never answered. Probe: R answers q1, then R offered q9 -> rule, then B offered q9 -> NOTE (at e3552db it was RULE). The assertion that covered "a different question on the same task" was dropped from the test (tests/sparrow-task-attempts.test.py:438-443), so nothing pins it now. If a task is guaranteed to carry one question for its whole life, say so in the answer_rule docstring. Otherwise, add R to offers[qid] only when R's journal holds no answer yet, and restore the dropped assertion. Worst case: a room question goes unanswered with a [no-send] result.

To approve: the restarted gateway/MCP witness keweichen asked for (one-line room data, one answer at the expected revision, local [no-send], no echo, replay adds no second answer).

(agent-assisted, Sutando for bassilkhilo-ag2)

@keweichen keweichen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at current exact head 6b4182b608da691ec38f9407d7dfb9d49b7a7632 — not ready to merge.

The narrow requested fix works: after task A is durably recorded as offered q1 and its answer succeeds in the journal, task B/q1 gets ANSWERED_HERE_NOTE; the same control returns the full rule at e3552dbf. The focused cumulative suites pass (task-attempts 41, agent-turn 47, gateway hooks 67, Stop 38).

Blocking — failure to persist an offer still publishes the irreversible answer instruction. In packages/ag2-sparrow/ag2_sparrow/task_attempts.py:203-210, _save_json() can return False, but answer_rule() ignores it, persists answer_offered, and returns the full rule. With only relay-answer-offers.json writes failed, A successfully answered q1 in the journal; after restart B/q1 still received the full answer rule because no durable association led the reader to A. This path must fail closed before publishing the rule, and needs a production-writer failure regression.

Blocking — the cap contradicts “any offered task.” ANSWER_OFFERS_PER_Q = 8 evicts a still-actionable association. After offering q1 to T0...T8, the file retained T1...T8. A succeeded answer by T0 was present in the journal, but T9/q1 still received the full rule. Preserve every association that can still answer, or pin and enforce a smaller upstream cardinality before relying on this bound; add the ninth-offer control.

Inherited blocker. The #5885 finish() transition in this head ignores its three durable-write results. Fault injection reproduces delivered-wake repost, episode rerun, and live-result quarantine states; see my current-head review on #5885.

Blocking live gate. The PR body still says this gateway/MCP answer path was not exercised on the rig, and no exact-head post-restart witness has since been posted. It needs one-line ingress, exactly one answer, local [no-send], no room echo, and replay/cross-task suppression, plus the inherited current-head witnesses required by the stack.

All current GitHub checks are green, including CLA. These are reproducible durability/cardinality gaps and missing live evidence, not CI failures.

finish() ignored each write. It now saves the wake's episode as seen, then its
outbox item, and only then removes the attempt; any failed write returns False
with the attempt kept, so the identity is never lost to a half-done finish.
…e the key

interrupt() decided from a snapshot taken before capture-pane. A row or result
written while the pane was read (T finishing, a successor turn starting) now
aborts the Escape before the at-most-once claim, so a late Stop cannot land in
the turn that followed.
…the answer instruction

answer_rule ignored a failed save of the offer or of the attempt's
answer_offered. An answer cannot be undone, so either failure now returns a
note telling the agent not to answer and to end with [no-send].
@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@marklysze

Copy link
Copy Markdown
Collaborator Author

Fixed in 7a8e5e6 (head 7a8e5e6). answer_rule now checks both writes: the question's offer record and the attempt's answer_offered. If either fails, it returns a note instead of the rule: the agent is told not to answer and to end with [no-send], because an answer cannot be undone and an unrecorded offer could not be guarded. Test: test_an_offer_that_cannot_be_recorded_never_carries_the_answer_instruction covers each failing write, then the rule once both are recorded.

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving at 7a8e5e6 (agent-assisted, Blue for Chi's seat). If the offer record (or answer_offered) can't be saved, the task gets ANSWER_UNRECORDED_NOTE (write [no-send], don't answer) instead of the answer rule. Since an answer can't be undone, that's the right side to fail on. sparrow-task-attempts passes locally.

@marklysze

marklysze commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator Author

Superseded by #6092.

marklysze added a commit that referenced this pull request Nov 2, 2026
…e gate reads only its own task [AgentActivity] (#6092)

* feat(gateway): an agent asked a room question answers it once, and the gate reads only its own task [AgentActivity]

Rebuilt on main from #5937, behind REMOTE_TASK_TURN_CONTEXT (off by default). The offers and the
answered record live in the attempt ledger; the rule is given only when the offer is recorded and
nothing says the question was, or may be, answered.

* test(gateway): the journal check's every outcome on the src copy; damaged question entries

* fix(gateway): a room answer is recorded at every settle, and a task its own answer closed is never asked to stop

* fix(gateway): an answer that cannot be told withholds the question for good; only owner and collaborator tasks answer

* fix(gateway): room questions are redacted, answers are matched by question, and nothing safe-to-forget is ever an answer

* fix(gateway): a room question is free, held by one task, or closed; owner tasks alone may answer

* test(gateway): the named-instance end-to-end checks the held shape

* fix(gateway): a non-owner task told a room question is context only, never to answer it

* fix(gateway): a swept attempt frees or closes the question it held; a settle waits for its turn's fence

* fix(gateway): an unbound attempt never frees its question, and an answer that may have rotated away is unknown

* fix(gateway): a settle closes its question and never frees it, and a newer generation never inherits one

* fix(gateway): at the bound closed questions are forgotten oldest first; only live holds can fill it

* fix(gateway): a live hold is never forgotten on local time; the earlier schedules pinned as they ran

* fix(gateway): an untrusted number is read through one finite() helper; a huge one never makes a task unqueueable

* docs: the source map lists safe_number

* fix(gateway): a room question is sanitized before any durable writer, vault commands included

The held serve's saved copy took the raw broker task, so a room question's secret
was written to disk before the render-time redaction, with turn context on or off.
A question string now gets the vault grammar's redaction (never interception), then
the signature scanner, at the top of _write_task; off, the question is dropped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants