Skip to content

Pin the system-audio stop tail handoff in tests - #1749

Merged
r3dbars merged 10 commits into
mainfrom
claude/fix-recording-tail-drop-260yya
Sep 22, 2026
Merged

r3dbars merged 10 commits into
mainfrom
claude/fix-recording-tail-drop-260yya

Conversation

@r3dbars

@r3dbars r3dbars commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Requested by Justin · project thread

Why

Before: #1747's release-hold checklist still lists the recording-end tail drop as an open bug — "PCM queued in the new backend ring can be dropped when host writer admission closes at stop" — and experiments/audio-only-probe/README.md still carries the matching "Deferred P2" bullet. A reader of either concludes the last slice of every meeting is still being lost.

After: the checklist item's first half is done and the docs say so. The bug was fixed in 4765e68 on that same branch; what was missing is anything pinning the fix, and the independent review the checklist also asks for.

I traced the whole stop path before writing anything, because the checklist wording was the only description I had. The tail survives because three things line up in Audio.stop():

  1. finishingCapture?.beginFinishing() runs before beginRecordingSessionGeneration(). After the generation advances, every buffer the backend's 10 ms consumer timer delivers arrives at the host with a stale generation, and the finishing handoff is the only path that still writes them. If admission were armed later — inside stopSystem, which is dispatched asynchronously — everything delivered in that window would be dropped with no diagnostic.
  2. stopSystem calls the attempt's finishAndDrain(), not cancel(). Cancellation deliberately discards queued PCM, which is correct for a superseded attempt and wrong for a normal stop.
  3. AudioStopCleanup closes the system writer on the same serial file queue the tail writes were enqueued on, after stopSystem returns. Reverse those and the drained tail is written to a closed file.

Each is a one- or two-line ordering inside a 3.5k-line file. CoreAudioSystemAudioCaptureTests covers the backend drain and the admission window in isolation, but nothing covered the wiring, and no CI job records real audio, so a reorder would silently truncate every saved meeting.

Product Impact

  • Affects: meetings
  • Lane: meeting reliability
  • Why this matters: losing recorded audio is the worst failure this app has, and the failure is invisible until someone plays back a call. This change does not alter behavior; it makes the existing fix hard to undo by accident and clears one half of a release-hold item.

What changed

  • Added Tests/TranscriptedCoreTests/AudioTests/SystemAudioStopTailHandoffTests.swift:
    • testStopCleanupWritesTheDrainedRingTailBeforeClosingTheWriter — drives a real SystemAudioCaptureStartAttempt over a hooked CoreAudioSystemAudioCapture, with a callback shaped like AudioFileManager's, through the real AudioStopCleanup.schedule. Two buffers sit in the ring when Stop is pressed; both must be in the WAV once cleanup completes.
    • testCancelledAttemptDiscardsItsRingTail — the counterpart, so a future "fix" for the tail can't start letting a superseded attempt append to a file a successor has moved on from.
    • testStopArmsTheTailHandoffBeforeAdvancingTheRecordingGeneration — a source-order contract over Audio.stop(), since no unit test below that level can observe the order of statements in it.
  • Corrected the stale "Deferred P2" bullet in experiments/audio-only-probe/README.md and noted the new pin in the hardening section.

No production code changes.

How I checked it

This is the part that needs saying plainly: none of the Swift checks ran. These sessions are Linux containers with no Swift toolchain, and this repo is macOS 26 + Apple Silicon only, so the new test has never been compiled anywhere. CI on macos-26 is the first real compile — the spm-tests job runs swift test, which picks the new file up automatically (AudioTests is a directory-based SPM target).

  • scripts/dev/agent-preflight.sh
  • python3 scripts/dev/check-build-source-lists.py — passed
  • bash build.sh --no-open — cannot run here
  • bash run-tests.sh — cannot run here
  • Performance budget — not applicable, no production code changed
  • bash run-integration-smoke.sh — cannot run here
  • swift test — cannot run here; this is the job that matters, left to CI
  • Manual check: none. The tail behavior itself was live-verified once on the base branch (a 56-second recording kept its final words); this PR does not re-verify that.

Risk Review

  • Privacy / local-first behavior reviewed — no payloads, no logging changes
  • Storage path or migration impact reviewed — tests write to a per-test temporary directory and clean up
  • Public-facing copy stays concrete and matches current product scope — no user-facing copy
  • Release/update impact reviewed — none
  • Agent PRs link the issue/workpad and stay draft until human review
  • UI changes include sanitized .agent-review/visuals/ evidence — no UI changes
  • No private transcripts, audio, tokens, personal paths, or customer data are included

Notes

Targets codex/system-audio-only-permission rather than adding a commit to it, so #1747's own history stays yours.

Two adjacent gaps I found while tracing and deliberately did not touch, since the brief was to keep this narrow:

  1. The mic side has the same tail drop, and always has. micAudioWriteBackpressure.close(...) runs at the top of Audio.stop(), but the mic tap is torn down later on a global queue. Mic buffers arriving in that window hit case .closed: in AudioFileManager and return — no finishing handoff, no diagnostic. This is pre-existing on main, not introduced by Use audio-only system capture and add Meetings plus menu #1747, and the window is however long the dispatch hop takes. Worth its own issue.
  2. cleanupAbandonedSetup() can race a normal stop. If Stop lands immediately after startIfNotCancelled returns, the setup path's sessionIsCurrent() check fails and calls captureAttempt.cancel(), which closes tail admission and stopSync()s the backend — discarding the ring that Audio.stop() had just armed for draining. Only reachable in the first moments of a recording, so the tail at stake is tiny, but it is a real ordering hole.

Neither blocks the remaining checklist items, which are the fresh-install, old-grant-upgrade, denial/recovery, route/sleep-wake and packaging checks that need a human at a Mac.

Agent handoff

COORD_DONE: BRIEF | https://github.com/r3dbars/transcripted/pull/1747 | tail-drop confirmed already fixed in 4765e68, added a host-wiring regression pin + source-order contract, corrected stale docs | none | whether to file the two adjacent gaps above as issues | agent-preflight + check-build-source-lists only; no Swift toolchain in session, swift test left to CI | read CI on this branch, then take the remaining #1747 checklist items to a Mac

🤖 Generated with Claude Code

https://claude.ai/code/session_01NwYbJHm2aXJogYcYKEps2s


Generated by Claude Code

r3dbars and others added 4 commits September 19, 2026 05:57
The recording-end tail drop in the Core Audio tap change is fixed in
4765e68: stop arms the attempt's tail admission before the recording
generation advances, finishes and drains the backend instead of
cancelling it, and AudioStopCleanup closes the system writer on the same
serial file queue behind the drained writes.

Nothing pinned that wiring. CoreAudioSystemAudioCaptureTests covers the
backend drain and the admission window in isolation, but no test
exercised Audio.stop's ordering, and no CI job records real audio, so
moving either line would silently truncate the end of every saved
meeting.

Add SystemAudioStopTailHandoffTests: an end-to-end pass through the real
AudioStopCleanup scheduler asserting a queued ring tail reaches the WAV
before the writer closes, the cancellation counterpart asserting a
superseded attempt still discards its tail, and a source-order contract
for the two orderings inside Audio.stop that no unit test can observe.

Also correct the stale "Deferred P2" bullet in the probe README, which
still described the tail drop as unresolved.

No production code changes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwYbJHm2aXJogYcYKEps2s
@r3dbars r3dbars self-assigned this Sep 19, 2026

r3dbars commented Sep 19, 2026 •

Copy link
Copy Markdown
Owner Author

CI note on 4b9cb0c: spm-tests failed, and it is not this PR's failure.

The job aborted with signal 6 out of the XCTest stall detector, in AudioTests.AudioLevelPublishGateTests.testMicLevelPublishesAgainAfterInterval — its drainMainQueue() helper waited on a main-queue expectation that never landed:

A stall was detected while waiting on expectations in
AudioTests.AudioLevelPublishGateTests.testMicLevelPublishesAgainAfterInterval()
at Tests/TranscriptedCoreTests/AudioTests/AudioLevelPublishGateTests.swift:56.
There's no IDE/Xcode connection to restart the process, so aborting to break
the apparent deadlock.

Why it isn't mine:

No fix exists to port: the abort is a main-queue stall under load in a time-based test, not an assertion failure, and I can't reproduce it — these sessions have no Swift toolchain. I'd rather not push a speculative change to an unrelated test I can't run.

I don't have permission to re-run the job (rerun-failed-jobs returns 403), so the one re-run these cases get is unspent and needs a human click. Re-running spm-tests on this run should be enough. I'm still watching the PR.

Final state of this commit: checks, app-build and repo-hygiene all green. build-and-test is red only as the rollup gate — its log is checks=success spm-tests=failure app-build=success, so it clears the moment spm-tests does.


Generated by Claude Code

@r3dbars

r3dbars commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

macOS verification follow-up — September 21

Pushed test-only hardening in eed87b76591f5d16a56fd1509739f56a0213dd3a; original authorship is preserved.

  • The original CI run stalled in the pre-existing metering test's synchronous XCTest wait. All four metering tests passed locally on both the original head and follow-up. Its intermittent CI root cause is not proven fixed; production metering and its tests are unchanged.
  • A separate original-head failure reproduced: the source-order fixture resolved Tests/Sources instead of Sources. Fixed that path.
  • Strengthened tail coverage with stereo PCM markers, explicit early delivery before backend teardown, normal-versus-finishing callback counts, and cancellation callback rejection. These are hooked-backend/scheduler and source-order tests, not full native Audio.stop() integration proof.
  • Final focused run: 32 passed. Forced dependency rebuild, app build, integration, and full automated QA passed: 14 passing steps, one non-blocking existing-local-log warning, zero failures. Fast suite: 14,094 assertions. Core: 1,117 passed, 13 skipped. QA package: 67 passed.
  • Independent review of the full PR diff and two-file follow-up found no remaining actionable findings; whitespace check passed.

Fresh hosted CI is running on the pushed head. Hardware skips are not passes. Fresh TCC, signed upgrade, real routes/calls and final release packaging remain separate gates. Release-health/product-task checks above are deterministic fixtures, not live fleet evidence. No merge or release performed.

@r3dbars

r3dbars commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Updated this test-only branch with its permission-branch base via normal merge facd0bf (no force push). The PR remains test/doc-only relative to its updated base. All eight focused stop-tail and metering tests pass locally on the new head; fresh hosted checks are pending. This does not establish hardware or release readiness.

@r3dbars

r3dbars commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Reconciled this companion PR with the updated audio-permission base at d73e93f using a normal merge (4fab65f). The remaining PR diff is still the stop-tail tests and their documentation. Focused Core Audio plus stop-tail tests pass: 27/27. The standalone base permission suite also passes 287/287. New exact-head hosted CI is pending; fresh permission, denial/re-enable and old-grant upgrade tests remain open. No merge or release performed.

@r3dbars
r3dbars changed the base branch from codex/system-audio-only-permission to main September 22, 2026 11:35
@r3dbars
r3dbars merged commit 3957b88 into main Sep 22, 2026
5 of 7 checks passed
@r3dbars
r3dbars deleted the claude/fix-recording-tail-drop-260yya branch September 22, 2026 11:59
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.

2 participants