Skip to content

feat(meetings): capture system audio in Electron - #7196

Open
kaizawa97 wants to merge 2 commits into
kirodotdev:mainfrom
kaizawa97:pr/meetings-electron-system-audio
Open

feat(meetings): capture system audio in Electron#7196
kaizawa97 wants to merge 2 commits into
kirodotdev:mainfrom
kaizawa97:pr/meetings-electron-system-audio

Conversation

@kaizawa97

Copy link
Copy Markdown
Collaborator

Stacked on #5738 (pr/meetings-recording-core) and unmergeable before it: systemAudio.ts / the capture pipeline this extends land there. Until #5738 merges (or is rebased), this PR's diff shows both commits and it will report conflicts with main exactly as #5738 does; only the feat(meetings): capture system audio in Electron commit is for review here. I will rebase this branch as soon as #5738 lands.

Problem / Motivation

In a browser tab, system audio (the remote side of a Zoom/Chime call) is only reachable through a display-capture prompt, and on some platforms not at all. So in the desktop app — the place most likely to be running next to a call — a meeting records only what the microphone picks up.

What changed

  • Electron's display-media handler resolves a loopback audio track directly, no picker prompt: website/electron/display-media.js grants the request with system audio where the platform can provide it, and preload.js exposes the capability to the renderer behind the existing bridge.
  • The capture tier is probed, not assumed (audio/captureTier.ts): the frontend inspects the track it actually received and reports which tier it got. A platform that cannot provide system audio degrades to microphone-only instead of recording silence — the failure a hardcoded assumption would produce.
  • One catalog key added to all 13 locales; spec updated in the same commit.

Tests

  • website/src/test/MeetingsCaptureTier.test.ts (16 tests) — the tier probe and each degradation path.
  • website/electron/test/display-media.test.js (17 node:test cases) — the main-process handler's grant/refuse branches.

Both suites run green on the branch (Linux, node 22). tsc -b does not pass on this branch's checkout because the stack's base predates the main-branch repair in #5522 (useDialogFocusTrap.ts) — untouched by this change and resolved by the rebase onto #5738's post-rebase head.

Manual verification

Not yet performed on a real desktop build; loopback capture availability differs per OS and needs a hands-on pass (macOS in particular) before merge sign-off.

Related Issues

Part of the meetings feature stack split from the feat/meetnote branch; depends on #5738, independent of #5741/#2190/#7194.

no linked issue: feature work from the meetings stack; no tracked issue exists for it.

Adds a recording package that owns an audio session: a state machine
per session, a WAV writer, a transcript persister, and recovery for a
session whose process died mid-recording. The dashboard exposes it at
/api/ws/recording, and the Meetings app records what a session
produced through its own recording store.

The frontend captures both the microphone and the system output,
mixes them through the PCM worklet's two inputs, and shows input level
in a meter so a user can see the capture is live before the meeting
starts.
A browser tab can only offer system audio through a display-capture
prompt, and on some platforms not at all. Electron can grant it
directly, so the display-media handler resolves a loopback audio track
and the frontend probes which capture tier it actually received.

The tier is reported rather than assumed, so a platform that cannot
provide system audio degrades to microphone-only instead of recording
silence.
@kaizawa97
kaizawa97 requested a review from a team August 31, 2026 03:30
@kaizawa97
kaizawa97 requested a review from a team as a code owner August 31, 2026 03:30
@kaizawa97
kaizawa97 requested a review from Zedmor August 31, 2026 03:30
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: checking Automated validation is still running merge conflict Branch has merge conflicts with its base — author must resolve before merge labels Aug 31, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator

Audit note — this PR and #5738 are one branch stack, not duplicates

A duplicate-detection sweep flagged these two as near-identical because they share a large block of added lines. They are not duplicates: one branch contains the other's commits, so the shared lines are inherited, and the review diff overstates what this PR actually authored.

This is the textbook stacked-branch artifact, provable by SHA rather than by reading code: git rev-parse pr/5738 = 4bb2f16 and git rev-parse pr/7196^ is the SAME sha, off the same merge base abc8222. git log <mb>..pr/7196 is exactly two commits (4bb2f16 then 375f708) while git log <mb>..pr/5738 is exactly one (4bb2f16). So #5738's entire 41-file/+5212 diff appears verbatim inside #7196's three-dot diff, and the 41 shared files are a STRICT SUBSET of #7196's 47 (comm -23 on the two files.tsv is empty). Only #7196's own commit -- 22 files, +597/-15, none of them backend -- is its own work, and it is strictly additive: it never rewrites a line #5738 wrote, it extends #5738's systemAudio.ts, SYSTEM_AUDIO_KEY and meetings.md Recording section on top. Neither PR duplicates the other's behaviour: #5738 is the backend recording subsystem plus browser capture, #7196 is the Electron main-process loopback grant, and #5738 touches no file under website/electron/ at all.

What is genuinely each PR's own

If #5738 merges, #7196 still carries all of its own work and loses nothing: 6 files #5738 never touches (website/electron/display-media.js +102, main.js +35, preload.js +27, electron/test/display-media.test.js +95, website/src/apps/meetings/audio/captureTier.ts +95, website/src/test/MeetingsCaptureTier.test.ts +171), plus its 35-line extension of systemAudio.ts, the 3-line no-loopback row in SYSTEM_AUDIO_KEY, one catalog key in 13 locales and 36 lines of spec. That is the whole capability of granting real system audio in the desktop shell and degrading to mic-only where the platform cannot -- unlanded and unduplicated. The dependency runs one way only: #7196's own commit CANNOT apply to main alone, because website/src/apps/meetings/audio/ does not exist on origin/main (git ls-tree empty) and SYSTEM_AUDIO_KEY/sysAudioUnsupported have zero hits there, so the files it modifies are created by #5738. Correct sequencing is #5738 first, then #7196 rebases; #7196 merging as-is would simply carry #5738's commit in with it, which is not redundancy but the base of the stack.

Suggested action

Land #5738 first, then rebase #7196 onto main so its diff shrinks to its own work. Reviewing #7196 before that means reviewing #5738's code twice.


From a repository-wide duplicate/overlap audit of every pull request open against main (2026-09-02, 330 PRs, one reviewer per PR). Each PR was read as its full merge-base diff plus its description and every comment and review, then compared against each candidate PR's own diff and against origin/main at 1a765b88ceb7. This PR is not being closed — the note is informational. If the reading is wrong, please correct the reasoning rather than just the conclusion.

@bolichen97

Copy link
Copy Markdown
Collaborator

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

  • PR #5738 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #5738: CONTINUE_DEVELOPMENT. A branch stack, not a duplicate: PR #5738 is the base and PR #7196 is the dependent. Land PR #5738 first and have PR #7196 rebase onto it. Closing PR #5738 in favour of PR #7196 would be closing the base in favour of the tip and would silently make the whole 5212-line backend subsystem land under the wrong review. Files: src/kiro_crew/recording/ws.py, website/src/apps/meetings/audio/systemAudio.ts, website/src/apps/meetings/hooks/useMeetingSession.ts.
  • PR #5741 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #5741: KEEP. Materially different scope (live system-audio capture in the desktop shell); no shared behavior with the import route. Files: src/kiro_crew/recording/ws.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.
  • This PR is OVERLAPPING with PR #7194. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7196: REBASE. Complementary meetings features sharing the manifest, app.json permission list, MeetingView header and useMeetingSession surface. Whichever lands second resolves the highlight renumbering; no code needs to be dropped. Files: website/src/components/appstore/appManifest.ts, src/kiro_crew/apps/builtins/meetings/app.json, website/src/apps/meetings/MeetingView.tsx.
  • This PR is OVERLAPPING with PR #7532. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7196: REBASE. A merged refactor moved this PR's host code out of main.js into window-lifecycle.js, so the change must be re-applied there and its source-pinning test updated. Independent of the PR #5738 stack. Files: website/electron/main.js.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

@dwu96 dwu96 added the needs-pr-triage PR scanner: awaiting automated triage label Sep 7, 2026
@NicholasRBowers NicholasRBowers added needs-author-decision PR blocked on author input and removed needs-pr-triage PR scanner: awaiting automated triage labels Sep 7, 2026
@NicholasRBowers

Copy link
Copy Markdown
Contributor

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: This PR has been inactive for 7+ days. I reviewed the blockers but they require your input:

  • The PR is stacked on unmerged feat(recording): capture meeting audio over a WebSocket #5738 (itself conflicting with main); only you can sequence the rebase once the base lands.
  • A merged refactor moved this PR's website/electron/main.js host code into window-lifecycle.js — the change needs re-applying there and its source-pinning test updated (per the 2026-09-04 audit note).
  • Your body flags the outstanding hands-on pass: loopback system-audio capture differs per OS and needs real macOS/Windows verification before merge sign-off — a Linux automation host cannot provide that.
  • Coordination: feat(meetings): add a per-meeting note the user owns #7194 overlaps on manifest/app.json/MeetingView — whichever lands second rebases.

When you've addressed these, the pipeline will re-assess on its next cycle.

@bolichen97

Copy link
Copy Markdown
Collaborator

Attempted a rebase onto main 8534cbf7 as part of the 2026-09-08 open-PR audit. Aborted, branch untouched — the conflicts need your decisions, not a mechanical resolution.

They land in the parent commit (4bb2f165d, i.e. #5738), 21 files:

  • website/public/pcm-worklet.js — main replaced the decimator with a polyphase anti-aliased resampler; where the two-input sum belongs relative to the new filter history is your call.
  • website/src/apps/meetings/MeetingView.tsx — main moved the action bar into an overflow menu (it says five sibling buttons breached the two-per-row cap) and added translation; the recording/system-audio controls need placing in that new shape.
  • src/kiro_crew/config/loader.py — now a facade over config/sections.py; RecordingConfig needs a home there.
  • security_posture.py, config-baseline.json, useMeetingSession.ts, useMeetingTranscription.ts, meetings.md, 13 locale catalogs.

Also pending on this PR's own commit: merged #7532 moved setDisplayMediaRequestHandler and additionalArguments out of website/electron/main.js into window-lifecycle.js, so that hunk no longer applies and MeetingsCaptureTier.test.ts's three source-pinning assertions must be repointed.

Suggested order: land #5738 first, then rebase this branch so its diff is just its own 22 files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) merge conflict Branch has merge conflicts with its base — author must resolve before merge needs-author-decision PR blocked on author input readiness: checking Automated validation is still running

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants