Skip to content

fix(hitl): a decision card asked from the owner's DM is posted in, and answered from, that DM - #6027

Merged
sonichi merged 1 commit into
mainfrom
fix/hitl-card-in-asking-room
Oct 30, 2026
Merged

sonichi merged 1 commit into
mainfrom
fix/hitl-card-in-asking-room

Conversation

@sonichi

@sonichi sonichi commented Oct 30, 2026

Copy link
Copy Markdown
Owner

Priority

Level: medium
Reason: a worker's decision card shows up in the wrong DM, and the owner's click never gets back to the worker that asked.

Closes

No issue.

Summary

A pool worker can call ask-owner.py --task-file <its task> --default/--option ... from a task that came from the owner's DM with that worker. The text ask already goes to the task's own conversation (destination_from_task). The decision card did not: it was stored with room: "", so the gateway's projector posted it in its default owner DM, which is the core's. The owner saw the card in the core's DM, and the click became a task for the core instead of the worker.

The fix is in the two places that own the policy:

  • pending_questions_ask: new card_room(dest). A card's room is the task's own conversation when destination_from_task names one. It only names one for an owner-tier task whose attested headers say it is an AG2 Space DM (channel_kind: dm). Any other origin keeps the empty room, which means the default owner DM, as today. That covers a shared room, no DM evidence, a non-owner tier, or no --task-file. A decision card is never posted in a shared room. hitl.cards.choice_request takes the room.
  • hitl.projector (and its byte-identical vendored copy in ag2_sparrow.hitl): each card is posted, edited and followed up in req.room when that is set, and in the default room_id otherwise. Only the primary gateway projects (owns_unaddressed), so one loop posts each card and nothing is projected twice. Claude-local cards already had req.room filled with the owner DM, so where they post does not change.

How the click gets back to the worker:

  1. The click arrives as a relay task whose channel_id is the card's room.
  2. validate_action already rejects a reply whose room is not the card's (if req.room and reply.room != req.room).
  3. An applied click with turn_on_action keeps going as the same task, now marked hitl_click: true (_handle_hitl_action → _write_task).
  4. The pool router's targets_for(roster, task["channel_id"]) sends it to the worker bound to that room.

None of the click path needed a change.

Checklist

  • Confirmed no other open PR closes the same issue (gh pr list --state open --search "hitl card room"; feat(pending-questions): ask-owner sends a decision card when --default or --option is given #5942 is the earlier draft that added the card and is a different change)
  • Git author email matches my CLA-signed email
  • Single concern per PR
  • Confirmed bug exists on main (see the before run below)
  • Test added
  • Before/after evidence pasted below
  • Every claim in this PR body is verifiable from the diff or the pasted output
  • Live path? Not done. Per the owner, no heavy local runs on this host. The projector and ask paths are covered by unit tests, but nobody has tried a live post to a worker DM and a click back yet.
  • Stacked PR? N/A
  • Scanned added lines for hardcoded paths (review-checks.sh clean)
  • No CI workflow change
  • No workspace/home resolution change

Before / after evidence

The new tests, run against the code on main (only the production file swapped back):

$ python3 tests/pending-questions-decision-card.test.py -k CardRoom
FAIL: test_a_click_from_the_cards_room_applies_and_turns_and_another_room_is_stale
FAIL: test_a_task_from_the_owners_dm_puts_the_card_in_that_dm
FAIL: test_source_room_id_names_the_dm_too
Ran 5 tests
FAILED (failures=3)

$ python3 tests/hitl-projector.test.py -k room
FAIL: test_a_card_with_a_room_is_created_and_edited_there_and_one_without_goes_to_the_default
FAIL: test_a_cards_follow_up_notice_goes_to_its_room
Ran 2 tests
FAILED (failures=2)

The same test files with this branch:

$ python3 tests/pending-questions-decision-card.test.py   Ran 66 tests  OK
$ python3 tests/hitl-projector.test.py                    Ran 65 tests  OK
$ python3 tests/ag2-sparrow-hitl-vendored.test.py         Ran 3 tests   OK
$ python3 tests/pending-questions-card-clicks.test.py     Ran 51 tests  OK
$ bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

The full suite is left to CI.

🤖 Generated with Claude Code

…d answered from, that DM

ask-owner routed the text ask to the asking task's own conversation, but stored the
decision card with an empty room, so the gateway posted it in its default owner DM
(the core's). A worker's card then landed in the core's DM and the owner's click
went to the core, never back to the worker that asked.

- pending_questions_ask: the card's room is the task's own conversation when
  destination_from_task names one (owner tier + attested AG2 Space DM); any other
  origin, or no --task-file, keeps the empty room (the default owner DM).
- hitl.cards.choice_request takes the room.
- hitl.projector (and its vendored copy) posts a card, its edits and its notices to
  req.room when set, else the default room. One primary gateway still projects.
- The existing reply gate (schema.validate_action) then rejects a click from any
  other room; an accepted click goes on as a hitl_click task whose channel_id is the
  card's room, which the pool router routes to the worker bound to that room.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 94%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • skills/pending-questions/scripts/pending_questions_ask.py (100%)
  • src/hitl/projector.py (100%)

Summary

  • Total: 5 lines
  • Missing: 0 lines
  • Coverage: 100%

@sonichi
sonichi requested a review from john-the-dev October 30, 2026 01:01

@liususan091219 liususan091219 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.

Not approving yet — the approval bar is not met. No code change requested.

Missing piece: there is no approval on this PR at all. It needs one expert approval (hitl / pending-questions area) before it can pass.

What I checked at 4847f35:

  • The fix sits in the two policy owners: card_room() in pending_questions_ask.py (room only for an owner-tier task with channel_kind: dm), and req.room or room_id in hitl/projector.py, with the vendored ag2-sparrow copy byte-identical.
  • Tests are behavioural: CardRoom asks through the real _ask and runs a click through the real HitlReplyHandler (rejected from another room, applied from the card's room); the projector tests check create, edit and follow-up notice all land in the card's room. The PR body shows them failing on main.
  • Shared room, no DM evidence and team tier all fall back to the default owner DM, so a card is never posted into a shared room.
  • No hardcoded paths; CI green; license/cla success; no unresolved threads.

Also owed (named, not blocking): a live round trip — a worker asks from its owner DM, the card appears in that DM, and the click comes back to that worker.

— Sutando (susans-MacBook-Pro)

@qingyun-wu qingyun-wu 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.

Approve at 4847f35. Ready to merge.

Verified:

  • Bug is real on main: build_card never passed a room, so every card had room: "" and the projector posted it in the gateway's default owner DM, while validate_action (schema.py:263) only checks room when one is set.
  • The fix sits where the policy lives. card_room() reuses destination_from_task, which names a channel only for an owner-tier task with attested DM evidence. A shared room, no DM evidence, a non-owner tier and no task file all keep "", so a card never goes into a shared room. The projector uses req.room or room_id for create, edit and follow-up notices. The vendored ag2-sparrow projector.py is byte-identical to src/hitl/projector.py.
  • Tests exercise the failure mode through the real code: _ask, then HitlReplyHandler. A click from another room is rejected and one from the card's room is applied and turns. The projector tests show the card created and edited in its room, and its notice posted there. The PR body shows these tests failing on main. Diff coverage is 100% and CI is green.
  • Worst case for existing installs: cards with no room behave exactly as before. Only owner-DM tasks on AG2 Space change destination.

Non-blocking:

  • If the projecting gateway's account is not a member of the asking DM, the send is rejected and retried on every pass. Because the card exists, no text fallback goes out, so the question stays invisible. This is fine while pool workers share the gateway's account. The live round trip the body lists as not done would confirm that.

@sonichi
sonichi merged commit feb0f16 into main Oct 30, 2026
41 checks passed
@sonichi
sonichi deleted the fix/hitl-card-in-asking-room branch October 30, 2026 07:17
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.

3 participants