[sonichi]

Stack

#5910 merged (20f3042). It fixed the HITL click tier check: an owner id at a non-owner tier must not apply a click. origin/main, including #5910, is merged into this branch.

What changed, and why

Before this PR, scripts/ask-owner.py --default/--option sent only plain text. The default, the reason and the options were stored on the row and nowhere else. Now a question that has a default or options, and is bound for AG2 Space, is asked as a decision card.

1. One shared card builder, src/hitl/cards.py.

   • choice_request: one button per action; action ids must be unique; turn_on_action is on by default; an optional pre-allocated req_id.
   • create_choice: stores the card; the store dedupes by guard.
   • manager_for(workspace): the workspace's HITL store.
   • new_id: allocates a card id before the card is stored.

   report-feedback (register_ask and register_held) and all four worker-pool wedge cards now build through this helper. Neither constructs a HumanRequirement or a HitlStore itself any more.

2. The ask is ordered so that no card exists without a durable question.

   • Buttons: with a default, Approve comes first and carries it, followed by the other options. With options only, the buttons are exactly the options as given, with no Approve. The card stores {action id: consequence}.

   • Order: queue_question builds the card with a pre-allocated id and holds the question in the outbox, naming that id. The adapter then writes the row (**Sent:** card hitl_<id> for ag2space owner-dm (decision card) at <time>). Only after that does create_card store the card for the AG2 Space gateway to post in the owner's DM.

   • Report: the ask reports stored, not yet shown: decision card hitl_<id>, because storing a card is not showing it.

   • Card only: once a card exists, no text is sent for that question. There is no wait, no withdraw, and no text fallback after a card exists.

   • Three-way outcome: create_card returns stored, not stored or unknown.

     • A store that raised but whose card file is there (a save that failed after its rename) counts as stored.
     • A store that cannot be checked is unknown.
     • Both are kept as the card, with no text.

   • When text is used: only when the card cannot be built; when the question cannot be held (then no card is stored at all); when the card is provably not stored; and for any other bridge, since only the AG2 Space gateway posts cards.

   • Definite card failure: the text goes to the question's own destination with its link. Then the outbox entry and the row are rewritten to name that file; the row through RoomDbStore.redeliver, one body write. The next reminder therefore sees a drained text, not an absent card. If the process dies between the text and the rewrite, the record still names an absent card and the reminder reads the question as due: the owner may get the text twice, never not at all.

   • Known limit: the owner may not be asked again by this skill if the card is never posted. Nothing here can see the DM. The reminder never sends text while the question's card is live (pending or clicked), whether posted or not, nor while the store cannot say. Follow-ups for the card itself belong to the HITL renotify. Once the gateway has posted the card, the question's last-asked time is the gateway's notified_at.

     • Text pointer: a live card with no post recorded 30 minutes after it was stored (CARD_POINTER_GRACE_SEC) gets one text pointer when the reminder runs. It is sent once per card and recorded durably on the card as pointer_at, so a gateway that stays down is not repeated.
     • What the pointer says: the question is waiting as a card in the owner's AG2 Space DM and may not have arrived, quoting the title. It carries no options and no approve wording, and says that a reply does not answer it. The card stays the only place the owner chooses.

     Only a card that is gone (never stored, or settled without closing the row) leaves the question due; the reminder then texts it, when it runs. Nothing is scheduled. SKILL.md documents this.

3. Clicks come back to the row. The bridge owner-checks the click, records it, and forwards the relay task with hitl_click: true. The core then runs python3 src/pending_questions_reader.py clicks (pq.py clicks does the same).

   • Claim before close: a closing claim, a pid:nonce token, is stamped on the card before the row is closed. The row is then closed as Answered in one guarded write that also requires the owner's Status to be open.
   • Concurrent readers: clicks and ack run under one exclusive lock beside the HITL store (state/hitl/pending-questions-clicks.lock), so two readers never act on the same snapshot. Under that lock, any claim already on a card is replayed, never skipped. A pid can be reused, or unsignalable (EPERM), so a claim naming a live pid must not hide the owner's click.
   • Output: the command prints chose <action-id> '<label>' -> <consequence>.
   • Ack: the core acts, then runs ack <ask-id>. Until the ack, the card stays claimed and a re-run replays the choice. A run that dies between the close and its record finds the row closed by code, not by the owner, and replays chose.
   • Stale clicks: a click on a row closed by the owner's own Status, or by code without this click's claim, is reported as already answered — not a decision and its card is cancelled.
   • Retiring cards: resolve cancels the question's card whenever it closes the row or the room refuses the close.

   This is documented in CLAUDE.md/AGENTS.md (Pending decisions) and in the skill's SKILL.md.

Where to look first

1. src/hitl/cards.py
2. In pending_questions_ask.py: queue_question, create_card, card_posted and asked_recently
3. In pending_questions_room_db.py: ask_owner (the create_card call after the row, and the row rewrite), clicks with its lock and claim token, ack_click, and _close_row (CODE_CLOSED_PREFIX)
4. RoomDbStore.close(require_open=) and RoomDbStore.redeliver

User behavior this proves

Before and after, from one probe script run against an origin/main checkout and against this head. The call is queue_question("Merge #1?", default_action="merge it", options=[("Hold", "wait")]), with the owner last active on ag2space; at HEAD it is followed by create_card:

== origin/main
proactive files: 1
hitl cards: 0
sent line: **Sent:** queued owner-dm (last-active bridge) via proactive-ask-1793076208747-90933-5afc3a.txt
report: sent: queued owner-dm (last-active bridge) via results/proactive-ask-1793076208747-90933-5afc3a.txt (a bridge drain delivers it; the reminder re-raises an undrained file)
== HEAD
proactive files: 0
hitl cards: 1
sent line: **Sent:** card hitl_0905900e9841 for ag2space owner-dm (decision card)
report: stored, not yet shown: decision card hitl_0905900e9841 for ag2space owner-dm (decision card)

Crash boundaries. The CrashBoundaries tests use a real process death: a subprocess calls os._exit(17) at the injected point. They run against the production stores: the outbox, the HITL store, and the room database through the adapter's own serve over the fake room capability's on-disk state.

• Death while holding the question: no card exists.
• Death after the row, before the card: the row names the card id, no card exists, and the reminder treats the question as not sent.
• Death between the click's row close and its record: on restart the choice replays as chose option-1 'Hold' -> wait, not as stale. With the claim check disabled, the same test fails with (False, True, False) != (True, False, True).
• Two concurrent clicks() processes: both take their snapshot before either closes (a file barrier after the snapshot), and the second one's close lands after the first's. The card stays in_progress, no reader reports the choice as stale, and the choice can still be acked. With the previous pending_questions_room_db.py (6c4b365) the same test fails with 'cancelled' != 'in_progress'. Five consecutive runs of all the crash and concurrency tests passed.
• A claim naming a live pid (a stand-in for a reused pid): the click is replayed as chose option-1 'Hold'. With the previous pending_questions_room_db.py (5574af1) the same test errors, because the click is skipped and nothing is returned.
• Death between the fallback text and its record rewrite: the text went out, and the record still names the card that was never stored, so the reminder reads the question as due: the owner gets the text again, never silence.

Reproduced at 2f46f93 before this round, each by a real run:

1. A wait_shown exception queued text while the card stayed pending and projectable.
2. A failing Outbox.save plus a failing withdraw queued text beside a live, posted card.
3. os._exit in Outbox.save left a posted card with no outbox entry, no row and no text.
4. os._exit after the row close made the real click read as stale on restart.

Items 1 and 2 no longer have a code path: wait_shown, _require_shown and _card_to_text are gone, and a test asserts it. Item 3 is fixed by the ordering, item 4 by the claim.

Feature/documentation consistency

docs/catalog.json has no canonical document for pending questions. The behaviour is documented in skills/pending-questions/SKILL.md ("Decision cards"), in CLAUDE.md/AGENTS.md (Pending decisions), and in the scripts/ask-owner.py docstring. docs/src-map.md was regenerated.

Tests run (all pass at HEAD 83e6602)

python3 tests/<file> for each of these:

• pending-questions-decision-card: 68 tests, OK. This includes:
  • the crash and concurrency boundaries;
  • the three card outcomes;
  • the definite-failure fallback keeping its destination and link and rewriting the outbox and the row, after which the reminder does not re-ask;
  • the reminder never texting beside a live card, and taking recency from notified_at;
  • "stored, not yet shown";
  • the text pointer: it goes out after the grace period and not before; two reminder passes produce a single pointer, recorded on the card; it is never sent for a posted or settled card or a text ask; it carries no option or answer wording; and the reminder runs it;
  • the ordering, a long label, duplicate labels, a long question, and owner Status (Answered or Resolved) followed by a contradicting click.
• pending-questions-room-db: 51 tests, OK. Its source-order pin now checks "text: queue, then hold" and "card: hold, then store the card".
• hermetic-env-structural: 27 tests, OK.
• Every other pending-questions-* suite, tests/skills/pending-questions/pq.test.py, every report-feedback*, pool-wedge-* and hitl-* suite, agent-api-pending-questions, check-pending-questions-collapse, friction-pending-questions-collapse, src-map, ag2-sparrow-hitl-vendored, and sparrow-hitl-projection: all pass, after merging origin/main.

Changed-line coverage was measured locally with coverage.py over those suites, against the merge base with main. Before the final pointer refinement it was 99.5% (380 of 382 changed executable lines). The once-per-card test added since then also runs the remaining two lines, the default-store branch of send_card_pointers. The only file not measured is scripts/ask-owner.py, whose change is docstring-only. scripts/review-checks.sh passes, and agents-md-sync.sh --check and gen-src-map.py --check are up to date.

Edge cases and limits

• Card never posted: see the known limit in section 2; the question resurfaces as text only when the reminder runs.
• Card store fails definitely after the row is written: the text is sent and the row and the outbox are rewritten to name it. A death before the rewrite can mean a second text later, never silence.
• Room outage: in an outage a click's close is recorded locally and cannot check the owner's Status. The owner's Status still wins in the row's effective status.
• Status set by the owner on the row: this does not retire the card. A later click on it is reported stale.

Live path

No live round trip has been run; a restart for it needs the owner's word. The bridge is untouched, and the card goes through the existing _project_hitl and HitlReplyHandler path. A witness run would be:

1. one ask-owner.py --option from an AG2 Space owner-DM task;
2. the card appears and is clicked;
3. src/pending_questions_reader.py clicks, then ack.

🤖 Generated with Claude Code

[qingyun-wu]

Requesting changes at 18ecca1 (agent-assisted review by Mars, Qingyun's Sutando seat). The refactor is faithful: comparing the old inline requirements with choice_request for report-feedback's ask and held cards and all three wedge-card shapes, they match field for field. Report-feedback's exactly-once and stale handling is unchanged. Click handling is sound: only the owner's clicks are applied, and the ask id comes from the card's own subject/guard, so a payload can't name another row. Two things to fix.

Blocking

1. CI: python standalone tests (2/9) fails because of the new test. tests/hermetic-env-structural.test.py flags tests/pending-questions-decision-card.test.py:28: unisolated. Call isolate_os_environ() (from _helpers.hermetic_env) at module top, before the imports at :27, as tests/pending-questions-room-db.test.py:10-13 does. Shard 4/9 timed out in the Claude-adapter test, which this PR doesn't touch; I've rerun it.
2. Cards for already-answered questions stay live, and a late click reads as a decision. resolve (pending_questions_room_db.py, around :287) and reconcile never cancel the question's card, so after the owner answers by text or Status, the card stays clickable. A later click is correctly refused in clicks() (around :365-381, REFUSED_PREFIX), but it's still returned, and src/pending_questions_reader.py's clicks branch (around :268-280) prints - [id] chose '<label>', which CLAUDE.md tells the core to act on. A stale choice can therefore override the real answer. The same reprint repeats on every run when resolve returns a non-refused failure such as "nothing to close".
   Fix:
   • (a) When resolve closes or refuses an ask id, cancel any active runtime == "pending-questions" card with that guard.
   • (b) Mark a refused click stale: True, and print it as "already answered — not a decision", outside the exit status.
   • (c) Add a test for answered-as-text, then clicked.

Non-blocking

• Gateway down: with an AG2 Space destination and the gateway down, the card is stored and no text goes out, so "text is still sent as the fallback" holds only for the no-store case (tested). There's never a double send.
• Reminder: SENT_RE (pending_questions_ask.py:53) matches only queued … via proactive-…, so a card-asked row never counts as recently asked, and --notify re-asks it as text immediately.
• Length: card_message has no length cap, so an oversized question fails at projection after the ask reported success.
• Lost choice: the chosen label is only printed, while the row closes as Answered without recording which option, so a core crash after clicks loses it.
• Destination: asks from a non-DM AG2 Space room now go to the owner's DM as a card. That matches the docs; just noting the change.

Tests: I ran 48 files locally (the new test, the pending-questions, report-feedback, pool-wedge and hitl-* tests), and all pass.

[sonichi]

Thanks @qingyun-wu. Both blockers are fixed at 81e017a.

1. CI 2/9 (hermetic env). tests/pending-questions-decision-card.test.py now calls isolate_os_environ() before its imports, as pending-questions-room-db.test.py does. tests/hermetic-env-structural.test.py passes locally (27 tests).

2. A live card after the question is answered, and a late click read as a choice.

• (a) resolve (the adapter; core's src/pending_questions_reader.py resolve reaches it) now cancels every live pending-questions card for that ask id whenever it closes the row, in the room or locally, or the room refuses the close. An answer by text or Status leaves nothing clickable. The message names the retired card.

• (b) clicks closes rows through the row half (_close_row). If the room refuses the close because the row is already closed, the click is returned with stale: True and its card is cancelled. The reader prints it as already answered — not a decision, and it does not affect the exit status. If a close cannot be recorded at all, the card is marked reported, so the next run prints reported before, not a new decision instead of chose again. Only a fresh chose line is a decision; CLAUDE.md/AGENTS.md and SKILL.md now say so.

• (c) New regression tests:

  • answered by text, then a late click (the card is cancelled and the click is refused as stale);
  • core resolve retires a card that was already clicked;
  • a refused close retires the card;
  • a click on a closed row is stale and never printed as chose;
  • a click whose close fails is not reprinted as a new choice.

  All five fail at 18ecca1 (2 failures, 3 errors) and pass at HEAD.

Tests at HEAD: 22/22 in the decision-card suite. Every pending-questions-*, report-feedback*, pool-wedge-* and hitl-* suite passes, as do tests/skills/pending-questions/pq.test.py, agent-api-pending-questions, check-/friction-pending-questions-collapse, src-map and hermetic-env-structural. review-checks.sh passes.

The non-blocking points are not addressed in this push: the reminder's SENT_RE, the card length cap, recording the chosen option on the row, and the gateway-down fallback.

[liususan091219]

Review at 81e017aa09740d4c46a3823a8fdff7a758c1b0f9. Verdict: comment. No change requested from me. Not approving yet, because there is no approval at this head and qingyun-wu's CHANGES_REQUESTED (at 18ecca11) still stands until that reviewer looks again.

Both of that review's blockers are fixed. I checked each one at both heads.

1. Hermetic-env gate

• At 18ecca11, tests/hermetic-env-structural.test.py failed 2 of 27, on pending-questions-decision-card.test.py:28: unisolated.
• At this head it passes 27/27. isolate_os_environ() now runs at module top, before any assignment.

2. Stale card after a text answer

• Before the fix, at 18ecca11, using the real queue_question and clicks: after the row was closed by text, the card stayed pending. A later click came back as ('Approve', closed=False), which src/pending_questions_reader.py:278 printed as chose 'Approve'.
• At this head, with the real resolve (no room, so a local close):
  • The card is cancelled.
  • A late apply_action raises StaleRequirementError.
  • clicks() returns [].
• With a click already landed and the room refusing the close as "closed already":
  • The click comes back stale=True.
  • The card is cancelled.
  • click_line prints already answered — not a decision.
  • A second run returns [].
• Mutation: I put the three changed source files back to 18ecca11 and kept the new tests. The suite then gives FAILED (failures=2, errors=3), across the five new stale/retire tests. So the tests exercise the real fix.

Also checked

• 46 related test files pass at this head: pending-questions, agent-api-pending-questions, report-feedback, pool-wedge, hitl-*, and the hermetic gate. The new test runs 22/22.
• Commit status: license/cla: success (1 context). No unresolved review threads. No added hardcoded ~/.sutando path or SUTANDO_WORKSPACE default.
• CI: python standalone tests (4/9) fails on a 240 s timeout in tests/ag2-sparrow-claude-code-adapter.test.py. This PR does not touch that file, and the same job fails on main for its last three commits. The ag2-sparrow publish gate (python 3.11) check was still pending when I submitted.

Non-blocking

• test_the_pool_store_handle_is_the_helpers asserts src.count("choice_request(") == 4 on read_text(). That goes red when someone adds a fifth wedge card, and stays green when a call is dead code. The AST check in test_no_card_producer_builds_a_requirement_or_a_store_itself already covers delegation for that file.
• The open points from the earlier review still apply: the reminder SENT_RE not counting card asks, no length cap on card_message, and the chosen label not being recorded on the row.

Missing before approval: an APPROVED review at this head from a maintainer with write access. qingyun-wu's re-review would also clear the standing change request.

— Sutando (susans-MacBook-Pro)

[qingyun-wu]

Approved at 81e017a (agent-assisted review by Mars, Qingyun's Sutando seat). Both blockers are fixed.

1. Isolation: the decision-card test calls isolate_os_environ() before any import, and tests/hermetic-env-structural.test.py passes locally (27 tests).
2. Answered questions and late clicks
   • resolve now wraps _close_row and calls retire_cards on a close (in the room or recorded locally) and on a refused close. That cancels only active runtime == "pending-questions" cards whose ask id or guard matches; report-feedback cards are excluded. The core's reader.resolve takes the same path.
   • In clicks(), a refused close marks the click stale: True, cancels the card, prints "already answered — not a decision" (never "chose"), and keeps the exit status clean.
   • An unrecorded close sets subject.reported, so it isn't re-announced as a new choice.
   • CLAUDE.md and SKILL.md now say to act only on chose lines.

Tests: all 22 decision-card cases pass at the head. The 5 new ones each fail on 18ecca1 (2 failures, 3 errors), and the 48 related pending-questions, report-feedback, pool-wedge and hitl files all pass.

Minor

• SKILL.md wording: it says an answer "by Status leaves no live card". Only resolve retires cards, and reconcile has no hook, so a row whose Status the owner sets directly keeps a live card until it's clicked. That click is then handled safely as stale.
• For the follow-up: the earlier non-blocking points (no text on a gateway outage at ask time, SENT_RE not matching card sends, no length cap, the chosen option not recorded on the row) are unchanged.
• Shard 4/9: its known ag2-sparrow-claude-code-adapter timeout is #5943's subject and isn't affected by this PR.

CI: the only red is python standalone tests (4/9), the known ag2-sparrow-claude-code-adapter timeout that #5943 fixes, plus the tsc + tests (clean install) aggregator that fails because of it. Every other shard is green.

[sonichi]

@qingyun-wu, both new points and the wording fix are addressed at dbd983d.

1. Silent non-delivery. I went with the minimal honest version. The preferred fix would mean the outbox drain or the bridge reading HITL projection state before sending a proactive file. That couples the one outbound-delivery implementation to the HITL store, which crosses the boundary in CLAUDE.md's architecture rules.

• The ask now reports sent: STORED as decision card hitl_… — the gateway must project it; if it is not shown within 10 min, the next reconcile, ask or reminder sends it as text. The row's **Sent:** line reads card … stored for ag2space owner-dm (decision card, not yet shown).
• unshown_cards_as_text (pending_questions_ask.py) treats a card as shown only when the gateway has recorded a projection (store.projection(id).revision > 0). A live, unclicked card with no projection after CARD_SHOW_GRACE_SEC (600 s) is queued as text to the owner's DM, once per card. The card is stamped text_fallback and left live, so whichever he answers closes the row.
• It runs from the adapter's reconcile_pass, so it covers pq.py reconcile and the reminder, including when there is no room store. It also runs from every ask_owner. Each fallback is reported.
• Limit: nothing is scheduled, so the fallback fires on the next reconcile, ask or reminder, not on a timer. SKILL.md says so.

2. Approve on options-only questions. Approve leads only when a default is proposed. With options alone, the buttons are the option labels as given (ids option-<i>), and the card text lists the same choices with no "Why Approve". The row body still uses ordered_options() and is unchanged.

SKILL.md wording. It now says a question closed through resolve (after a text answer, say) leaves no live card. A Status set on the row by the owner does not retire the card, because reconcile does not touch cards. A later click on that card is reported as stale.

Tests.

• The decision-card suite is now 27/27. The five new tests fail at 81e017a (2 failures, 3 errors) and pass at HEAD:
  • an options-only card has no Approve;
  • a stored card is reported as not yet shown;
  • an unshown card is sent as text once, after the grace period and not inside it;
  • a shown card is never doubled as text;
  • reconcile runs the fallback with no room.
• These all pass: every pending-questions-*, report-feedback*, pool-wedge-* and hitl-* suite, tests/skills/pending-questions/pq.test.py, agent-api-pending-questions, check-/friction-pending-questions-collapse, src-map and hermetic-env-structural. review-checks.sh passes.

[github-actions]

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%)
• skills/pending-questions/scripts/pending_questions_remind.py (100%)
• skills/pending-questions/scripts/pending_questions_room_db.py (100%)
• skills/pending-questions/scripts/pending_questions_store.py (100%)
• skills/pending-questions/scripts/pq.py (100%)
• skills/report-feedback/report-feedback.py (100%)
• skills/worker-pool/scripts/pool_wedge_cards.py (100%)
• src/hitl/cards.py (100%)
• src/pending_questions_reader.py (100%)

Summary

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

[sonichi]

Both re-review points are addressed at a258b8a.

1. Reported flag set before the choice was printed.

• The bug: clicks() stamped reported on a click whose row close could not be recorded, before the CLI printed the choice. A crash in between turned that choice into "reported before" on the next run.
• The fix: clicks() now persists nothing in that case. The choice is printed as chose … on every run until the core runs ack <ask-id>, and only the ack marks it acknowledged. Later runs then just retry the close and print acknowledged before, not a new decision. Once the close lands, the card is settled.
• Tests: the new crash-window test makes click_line raise inside pending_questions_reader.py clicks, then checks that the next run still prints chose option-1 'Hold'. The updated ack test covers choice, ack, a retried close that still fails, then a late close settling the card. Both fail at 529a8d1 and pass here. The decision-card suite passes, 49/49.

2. CI shard 5, tests/skills/worker-pool/pool-restart.test.py. I don't believe this PR causes it.

• The failing case is RestartSessionContinuity.test_explicit_resume_then_restart_keeps_a_and_its_recorded_folder. It asserts that a real tmux pane process exits within 60 s of SIGTERM (pane pid … survived SIGTERM for 60 s).
• That path spawns and kills a worker session. This PR's only worker-pool change is in pool_wedge_cards.py, where the wedge cards' HumanRequirements are now built through hitl.cards.choice_request. The test does not reach that code; it only gets loaded transitively through pool_remedy. Nothing in the diff touches process spawning, signals or tmux.
• The suite passes locally on this branch (18 tests). As you noted, it passes on base too. I read it as a runner timing flake.
• Shard 4's failure is the known main timeout that test(ag2-sparrow): split the claude-code adapter suite so each file runs well under CI's 240 s cap #5943 fixes.

[sonichi]

CI shard 7/9 at 6c4b365 (run 40444354949, job 121992001034) failed on one test, which I believe is a flake on main and not caused by this PR: tests/ack-claim-lock-cleanup.test.py.

  ok   d) cleanup waits while a publisher is between open and acquire; the inode stays
  FAIL d) once the publisher is done, cleanup removes the lock ([[]], 1 open(s))
FAIL: 1 failure(s)
##[error]FAILED: tests/ack-claim-lock-cleanup.test.py (exit 1)

Evidence:

• This PR doesn't touch it. The PR does not modify src/ack_claim.py or the test (git diff origin/main...HEAD -- src/ack_claim.py tests/ack-claim-lock-cleanup.test.py is empty), and none of its files are imported by either.
• It is timing-dependent. Case (d) is a two-thread interleave (a publisher paused between open and acquire, plus a cleaner joined with a 0.5 s timeout), so it depends on thread scheduling.
• It fails on main with the same output. Running the test 80 times in a loop on an origin/main checkout at 5580db8, it failed once, with the identical message FAIL d) once the publisher is done, cleanup removes the lock ([[]], 1 open(s)). On this PR's head it failed 1 time in 40.
• No hits in other recent runs. I found no other recent CI run where this test failed; I checked the standalone shards of the last 21 failed CI runs on main and other PRs. So it is rare in CI. The local reproduction on main is the evidence that it is pre-existing.

I'll rerun the failed shard once the run completes.

[qingyun-wu]

Reviewed at 6c4b365 (agent review): COMMENT.

The click side is solid.

• The closing claim plus CODE_CLOSED_PREFIX replays a click whose run died mid-close.
• reported is set only by ack.
• resolve retires cards.

Reverting each fix fails its new tests. 51/51 pass here, and so do 50 neighbouring suites. CI is green.

Two ask-side gaps:

1. A card that is never posted is reported as sent. At pending_questions_ask.py:463, if the gateway never posts a stored card, ask-owner still prints sent: decision card… and exits 0. Only an on-demand reminder resurfaces it, so delivery can fail silently. 529a8d1's wait-and-fallback was removed without a replacement. Please restore a bounded check, or report the send as unconfirmed.
2. The text fallback leaves a stale record. At pending_questions_ask.py:417-429, when create_card falls back to text, the outbox and the row still say **Sent:** card hitl_… for a card that was never stored. So the reminder sends the text a second time. The fallback also drops the destination and the row link.

[liususan091219]

Re-reviewed at 6c4b365: REQUEST_CHANGES, for one incomplete fix on the ask side.

What I verified

• The click side holds. tests/pending-questions-decision-card.test.py passes 49 cases at the head. pending-questions-room-db and hermetic-env-structural pass too.
  • Mutation check: I set replay = False at skills/pending-questions/scripts/pending_questions_room_db.py:416. test_death_between_the_close_and_its_record_replays_the_choice then fails, so the new closing claim is pinned by a test.
• Repo checks: CI is green (37 pass, 1 skipped). license/cla is success with 1 context. There are no unresolved threads.

Blocking: the text fallback leaves a stale "card sent" record. This confirms qingyun-wu's point 2 at this head by running it.

• queue_question saves the outbox with **Sent:** card <id> before the card exists (pending_questions_ask.py:380-393). The room row is written with the same line (pending_questions_room_db.py:363). create_card runs only after that (pending_questions_room_db.py:375).
• When create fails, create_card falls back to _ask_by_text (pending_questions_ask.py:417-429). That rewrites only the in-memory out["sent_line"].
• Repro: I took the existing test_a_card_that_cannot_be_stored_falls_back_to_text setup (create raises OSError("disk full")) and read the outbox file afterwards. It holds **Sent:** card hitl_… for ag2space owner-dm (decision card), but no card was stored. The proactive text was the only send, and out["where"] became owner-dm (last-active bridge). The fallback passes Destination() and link=None, so the original destination and the row link are lost.
• Ask:
  • When the fallback fires, rewrite the outbox record's sent line and the row's Delivery line to the text send.
  • Keep the original destination and the link.
  • Extend that test to assert on the outbox record, not just on out.
• I did not check qingyun-wu's further claim that the reminder then sends the text a second time.

Also open (qingyun-wu's point 1, checked by reading only): at pending_questions_ask.py:463, a stored card that the gateway never posts still reports sent: decision card … and exits 0. The line does say the reminder counts it as not sent. But reminders run only on demand, so a failed delivery stays silent. Please either add a bounded post check or word that line as unconfirmed.

Approval state: there is no APPROVED review at this head. qingyun-wu's approval is at 81e017a, and their latest review here is a COMMENT.

— Sutando (susans-MacBook-Pro)

[qingyun-wu]

Re-reviewed at 5574af1 (agent review): COMMENT.

What's fixed. Gap B: the text sent on a definite failure keeps the destination and the link, and the outbox and row records are rewritten. The lock, the stored/not-stored/unknown outcome and the tests hold up: each of the 9 fixes fails its test when reverted. 62/62 plus 48 suites pass, and CI is green.

Still open:

1. An unposted card is never followed up. At pending_questions_ask.py:202 the reminder never texts while a card is live, and SKILL.md:109's "follow-ups are the HITL renotify's" isn't true: renotify.py:24 only reminds auth/core-blocked cards, and only ones already posted. So a card the gateway never posts is now never followed up; previously the reminder resurfaced it. Please keep it due while it's still unposted after a grace period.
2. A stale claim can swallow a click. At pending_questions_room_db.py:406-417,445, under the exclusive lock, a claim from a reused pid (or EPERM) makes the owner's click skip silently. The lock already covers that case, so drop the live-holder skip.

[github-actions]

@cla-assistant check

[sonichi]

Parking this PR as a draft. The owner set the 5574af1 round as the final bounded round, with "park if more turns up", and the final review at that head found five more correctness gaps:

1. a reminder path for choice cards;
2. claim handoff through ack;
3. liveness-probe portability;
4. fallback durability;
5. stored/unknown outcomes reporting text that wasn't sent.

83e6602 addresses parts of 1 and 3, but not 2, 4 or 5. Plain-text asks on main remain the working path. The review threads stay as the spec if this is resumed, preferably as smaller PRs (click handling first, then ask-side delivery).

[qingyun-wu]

Approve at 83e6602 (agent review). An unposted card gets exactly one pointer after 30 min, recorded as pointer_at. It names no options and says a reply doesn't answer it; posted and settled cards get none. Under the clicks lock, any existing claim is now replayed, so a reused pid no longer hides the owner's click. Each fix's test fails when reverted; 68/68 pass. Two CI checks were still running when I checked.

[sonichi]

Split into two smaller PRs, per the owner. This draft stays open for reference only.

• feat(pending-questions): hand each decision-card click to exactly one consumer #5946 (click side, based on main): each decision-card click is handed to exactly one consumer through ack, under one exclusive lock with no pid probe. It also covers owner Status winning, the exact choice id and consequence, and the reader's clicks and ack commands.
• feat(pending-questions): ask-owner asks by decision card when --default or --option is given #5947 (ask side, stacked on feat(pending-questions): hand each decision-card click to exactly one consumer #5946): card creation with the question saved first; the stored, not stored and unknown outcomes; the definite-failure text fallback, made durable through the outbox's redeliver and reconcile; and the reminder rules, including the once-per-card pointer.

[liususan091219]

Comment at 83e6602. Both items from our change request at 6c4b365 are fixed and pinned by tests, and no change-request condition fires. This is not an approval yet, for two reasons: the PR is still a draft, and GitHub reports it as conflicting with main (mergeStateStatus DIRTY). Our earlier CHANGES_REQUESTED therefore still stands formally; an approval once it is rebased and out of draft will clear it.

Our items, verified at this head

1. Text fallback left a stale "card sent" record: fixed. card_to_text sends the text to the question's own destination with its link, then re-saves the outbox record.
   • Revert check: replacing the Outbox(...).save(...) call in card_to_text with pass fails test_a_definite_failure_texts_the_same_destination_with_the_link_and_rewrites_the_record and test_an_outbox_that_cannot_be_rewritten_is_reported.
   • Revert check: sending the fallback to an empty Destination() with no link fails test_a_definite_failure_texts_the_same_destination_with_the_link_and_rewrites_the_record.
2. An unposted card was never followed up: fixed. A live card with no post after CARD_POINTER_GRACE_SEC (30 min) gets one text pointer, recorded as pointer_at.
   • Revert check: dropping the pointer_at once-only guard fails test_two_reminder_passes_send_a_single_pointer_recorded_on_the_card.

• tests/pending-questions-decision-card.test.py (68 tests) and tests/pending-questions-room-db.test.py pass at this head. CI is green (Windows job skipped); license/cla success with 1 context.

Approval state: qingyun-wu approved at this exact head after re-checking each fix by revert, which meets the expert route. The live round trip of a card through the gateway is still owed (the body's "Live path" section says none has run); that is named here, not blocking.

Missing: rebase onto main to clear the conflict, and take it out of draft. Then it is ready to approve.

— Sutando (susans-MacBook-Pro)