5210dd48b8
Three live findings from rc.4 staging, all on the relay-fronted Slack path, all with the failure observed in live logs before the fix: 1. Approval-send timeout is AMBIGUOUS, not failed (no re-ask). send_exec_approval through the connector can time out with the card already rendered — the connector may ack after the deadline (slow platform API call, transient backpressure, event-loop stall) — and the timeout-as-failure path re-sent and produced duplicate cards. The outcome is now tri-state: sent / failed / ambiguous. Ambiguous = no re-send, no text fallback; the prompt registration stays armed so a late tap still resolves. Only a definite send error falls back to text. 2. pending_approval tool results forbid re-issuing the command. With one card correctly armed, the agent could still mint a SECOND card by re-running a rephrased variant of the gated command after reading the pending_approval tool result (observed live: same command re-issued in a different form, two cards). The tool message now instructs: do not re-run/rephrase; wait or report pending. Applied to both the terminal and execute_code arms. 3. Draft interim AND seal frames carry format_hints. format_hints are stamped on send, edit, and send_for_platform, but both draft-frame builders (send_draft interim + _seal_open_draft seal) shipped bare metadata. A streamed final therefore arrived at the connector hintless and sealed as a plain code block while non-streamed sends rendered native markdown blocks (observed live: language-tagged block on send/edit, downgrade on streamed seal). Both sites now stamp _with_format_hints_for_chat (destination-resolved, same pattern as the existing lanes). Verified live after the fix against the platform's stored message payload: rich_text_preformatted with language field on a streamed seal. Tests: tri-state outcome unit tests (5), draft/seal hint stamping + knobs- off regression control (2, RED-first), existing format-hints suite intact (14/14). Mutation-verified: reverting the adapter hunk sends test_draft_interim_and_seal_frames_carry_hints red; restore -> green. Boundary sweep (text egress lanes crossing the frame contract): send ✓ (pre-existing) edit ✓ (pre-existing) send_for_platform ✓ (pre-existing) draft-interim ✓ (this PR) draft-seal ✓ (this PR); task_card lane carries no text content — exempt.
63 lines
2.3 KiB
Python
63 lines
2.3 KiB
Python
"""Approval prompt-send TIMEOUT must not trigger the re-ask/fallback lane.
|
|
|
|
Observed in live relay testing: `send_exec_approval`'s scheduling future can
|
|
hit its 15s `.result(timeout=...)` while the card HAS already posted to the
|
|
platform — the connector's ack simply arrives after the deadline (slow
|
|
platform API call, transient backpressure, event-loop stall). run.py treated
|
|
the timeout like a definitive send failure and ran the text fallback, so the
|
|
user saw the same approval multiple times; tapping an older card resolved a
|
|
prompt whose turn had already moved on ("/approve: nothing pending").
|
|
|
|
Contract under test (boundary rule — every prompt caller crossing the
|
|
send-timeout boundary): concurrent.futures.TimeoutError from the approval
|
|
send is AMBIGUOUS (possibly delivered). The gateway must NOT fall back /
|
|
re-send; the prompt registration stays live so the user's tap on the
|
|
(probably rendered) card still resolves. A definitive error (SendResult
|
|
success=False, or a non-timeout exception) keeps today's fallback.
|
|
"""
|
|
|
|
import concurrent.futures
|
|
from unittest.mock import MagicMock
|
|
|
|
import pytest
|
|
|
|
from gateway.run import _approval_send_outcome
|
|
|
|
|
|
class _Result:
|
|
def __init__(self, success, error=None):
|
|
self.success = success
|
|
self.error = error
|
|
|
|
|
|
def test_timeout_is_ambiguous_not_failure():
|
|
fut = MagicMock()
|
|
fut.result.side_effect = concurrent.futures.TimeoutError()
|
|
outcome = _approval_send_outcome(fut, timeout=0.01)
|
|
assert outcome == "ambiguous", (
|
|
"a send timeout re-ran the fallback — this is the duplicate-approval "
|
|
"re-pop (card posted, ack late); ambiguous must suppress the re-ask"
|
|
)
|
|
|
|
|
|
def test_success_is_sent():
|
|
fut = MagicMock()
|
|
fut.result.return_value = _Result(True)
|
|
assert _approval_send_outcome(fut, timeout=1) == "sent"
|
|
|
|
|
|
def test_definitive_error_result_is_failed():
|
|
fut = MagicMock()
|
|
fut.result.return_value = _Result(False, "relay prompt op unavailable")
|
|
assert _approval_send_outcome(fut, timeout=1) == "failed"
|
|
|
|
|
|
def test_non_timeout_exception_is_failed():
|
|
fut = MagicMock()
|
|
fut.result.side_effect = RuntimeError("loop unavailable")
|
|
assert _approval_send_outcome(fut, timeout=1) == "failed"
|
|
|
|
|
|
def test_missing_future_is_failed():
|
|
assert _approval_send_outcome(None, timeout=1) == "failed"
|