Repository navigation
fix(stt): end dictation when Transcribe drops the stream, and keep its credentials - #19550
Conversation
Amazon Transcribe can end the result stream mid-dictation (a refused event signature, 15 s without audio). The gateway only awaited the result stream at teardown, so the socket stayed open and the dashboard kept showing Listening while every later word was lost. A watcher task now waits on the result stream. If it ends before the client stops, it sends an stt_session_failed error frame and closes the socket, the way the apple relay already does. The duration cap and the watcher share one first-claimant record, so only one frame goes out and stt_stream_end records the cause that came first. The new tests bound their reads with default_timeout() from kiro_crew.testing.wait, which replaced _AUDIT_WAIT_TIMEOUT_SECS on main. Supersedes #17925. Co-authored-by: Amad Salmon <amad.salmon@icloud.com>
The streaming SDK asks the credential resolver again for every audio event, and each event's signature chains from the opening request's. _ProfileCredentialResolver asked the boto3 session every time, so a profile whose credentials refreshed mid-stream signed the next event with a different key and Transcribe refused it with "signature does not match". The resolver, built once per stream, now resolves once and returns the same credentials on every later call. botocore refreshes credentials with under ten minutes left before handing them out, so the pinned ones outlive the five-minute stream cap. Supersedes #17924. Co-authored-by: Amad Salmon <amad.salmon@icloud.com>
github-actions
Bot
added
the
readiness: checking
UX Review (Opus 5.5) — ⏭️ skippedRevision |
Design Review (Opus 5.5) — ✅ PASSDesign-level review of Design-Verdict: PASS Check against a live profile whose credentials refresh that the signature refusals stop. Only the watcher was run against real Transcribe (in #17925); the cause behind credential pinning has not been reproduced. [DESIGN-REVIEWED] 7e12549 Description read[DESCRIPTION-READ] 9bad42d661a17b650ed5091fcc69a6c4a4b2c23dec8dbb162453cd941e66dac7 That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with |
First Principles Review (Opus 5.5) — ✅ PASSPremise-level review of First-Principles-Verdict: PASS Check one claim before merge: botocore's refresh window (under 10 minutes left) must outlast the 5-minute cap, or the pinned credentials expire mid-stream. What this change shipsInventory (6 items) — 6 justifiedIntent: when Amazon Transcribe drops the stream, dictation on the
[FIRST-PRINCIPLES-REVIEWED] 7e12549 Description read[DESCRIPTION-READ] f866c6c8ab00e8ced4dac4c5091b31bd0940b29ebb58e1a3904821943a0e2a2e That sha256 names the PR description this verdict read. Recompute it with |
GPT 6.1 Review — ✅ no blocking findingsGPT 6.1 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Opus 5.5 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
github-actions
Bot
added
readiness: passed
iamwhatever
left a comment
iamwhatever
left a comment
There was a problem hiding this comment.
This fixes both causes behind #17842. First, a new _watch_results task in dashboard/stt_stream.py waits on the Transcribe result stream. If the stream ends before the client stops, it sends one stt_session_failed frame and closes the socket, so the dashboard no longer shows Listening over a dead stream. Second, _ProfileCredentialResolver in transcribe.py now resolves once per stream, so a mid-stream refresh can no longer change the signing key that each audio event's signature chains from. I read the whole diff and tried to break the teardown. The watcher is cancelled before end_stream(), so the normal end after a stop is not reported as a failure, and test_a_clean_stop_is_not_reported_as_an_early_end pins that. The cap and the watcher share one first-claimant fatal_outcome, and _enforce_deadline now returns when it is already set, so only one frame goes out and the audit records whichever cause came first. Cancelling the watcher mid-send still leaves error claimed. The credential pin is safe against expiry: botocore must refresh credentials with under 10 minutes left before it hands them out, which outlives the 5-minute cap, and every new dictation builds a new resolver. The tests use real botocore RefreshableCredentials, the stubs now end the result stream only on end_stream() like the real SDK, and all four AI lanes PASS on 7e12549. — crew-pr-reviewer (deep re-check)
github-actions
Bot
removed
the
readiness: passed
Problem / Motivation
Goal: Transcribe dictation keeps going until the user stops it or the 5-minute cap ends it, and when Amazon Transcribe ends the stream on its own, the dashboard stops showing Listening and says why.
With the
transcribeprovider, Amazon Transcribe sometimes ends the result stream mid-dictation: it refuses an audio event ("The request signature we calculated does not match the signature you provided") or hears no audio for 15 seconds. The gateway only looked at the result stream at teardown, so the socket stayed open. The composer kept the red microphone and the Listening pill, and everything said after that point was lost until the user stopped.Why it matters
Anyone dictating on the
transcribeprovider can lose minutes of speech with no sign that anything went wrong. The reporter saw it in 6 of 53 sessions longer than 30 seconds over two days, 4 of them signature refusals.Not a goal
stt.transcribe_profileis set. The SDK's own resolver signs those streams and is not changed here.What changed (motivation → approach → change)
Two causes, one commit each.
dashboard/stt_stream.py, Transcribe branch ofapi_ws_stt). The read loop only wakes on client data, so the stream's end went unnoticed. A watcher task now waits on the result stream. If it ends before the client stops, it sends anerrorframe withstt_session_failedand closes the socket, as theapplerelay already does. The dashboard already handles that frame: it ends capture, keeps the text so far and shows "The dictation session failed. Try recording again." The watcher is cancelled beforeend_stream()at teardown, so the normal end after a stop is not reported. The cap and the watcher share one first-claimant record (fatal_outcome), so only one frame goes out andstt_stream_endrecordserrorortimeoutfor whichever came first._ProfileCredentialResolver(transcribe.py) asked the boto3 session every time, so a profile whose credentials refreshed mid-stream signed the next event with a new key. It now resolves once and reuses the result. One resolver is built per stream (the only caller isstt_stream.py), so a new dictation still gets fresh credentials. botocore refreshes credentials with under 10 minutes left before handing them out, so the pinned ones outlive the 5-minute cap.Supersedes #17924 and #17925 by @amadsalmon (co-author). Review verdict: both are sound; #17925 fixes the silent Listening state and #17924 removes the only code path found that changes the signing key mid-stream, and neither alone closes the issue. Changes from their heads: rebased onto current main, and the new stt tests use
default_timeout()fromkiro_crew.testing.wait, because_AUDIT_WAIT_TIMEOUT_SECSwas removed from main by #19339 and the tests failed withNameError.Backwards compatibility
Compatible: the wire contract is unchanged. The new frame uses an existing code the dashboard already renders, and a session the user stops ends exactly as before. The resolver's interface and callers are unchanged.
Removes nothing: the diff adds a watcher, an error path and a cached credential; no field, frame, code or reader goes away.
Tests
test_a_result_stream_that_ends_early_ends_the_session[raises|returns]: a result stream that fails or simply ends while the client is connected produces onestt_session_failedframe, a close,stt_stream_end=error, andend_stream()still runs.test_a_clean_stop_is_not_reported_as_an_early_end: after a stop, the stream's normal end sends no frame and auditsok.test_a_cap_landing_during_an_early_end_sends_no_second_frame: holds the early-end frame until the cap has run; the cap sends nothing and the audit stayserror.test_a_stream_keeps_the_credentials_it_opened_with: real botocoreRefreshableCredentialsinside the refresh window, where every read refreshes to new keys; the resolver keeps the opening keys and asks the session once.handle_eventsnow waits forend_stream()like the real stream.Run locally at
-n0against this worktree'ssrc:test/test_stt_stream.py102 passed,test/test_transcribe.py249 passed, 2 skipped. Mutations, each restored after: removing the watcher task fails 3 tests; dropping the first-claimant check in the cap fails the cap test; removing the credential cache fails the credential test.Manual verification
Not re-run on this branch: no AWS profile with Transcribe consent on this host. #17925 records a live Amazon Transcribe run (us-east-1) of the same watcher code: a healthy 50 s session with silent gaps ended
okwith no error frame, and a forced 15 s no-audio refusal reached the client asstt_session_failedwithin 18 s.Related Issues
Closes #17842
Pattern harvest
Rule candidate: review-prompt
Pattern: a background stream or task whose failure is awaited only in teardown, so the session looks alive after the stream has died
Checklist
feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)Contribution License Agreement
Contributions are accepted under the project's Apache-2.0 license; see the Licensing section of CONTRIBUTING.md.
Decision for the owner
The decision is whether #17842 is closed by both fixes together: ending the session visibly when Transcribe drops the stream, and pinning a stream's credentials so a refresh cannot cause the drop. This draft ships both as two commits taken over from @amadsalmon's idle #17925 and #17924, because either alone leaves half of the reported failure in place.