fix(01-review): sweep #1 stopRecording nulls mediaStream first to prevent rotation race

src/offscreen/recorder.ts

@@ -359,16 +359,40 @@ function onUserStoppedSharing(): void {
 }
 
 function stopRecording(): void {
-  // Отменяем будущую ротацию, чтобы rotateSegment не дёрнул stop() на
-  // уже остановленном рекордере.
+  // Sweep #1 fix: race between rotateSegment / onSegmentStopped and a
+  // STOP_RECORDING that does NOT null the mediaStream. The previous
+  // implementation cleared the rotation timer and called .stop(), which
+  // triggers onSegmentStopped async; onSegmentStopped then unconditionally
+  // calls startNewSegment() whenever `mediaStream !== null`. Result: after
+  // a manual STOP_RECORDING, a new segment would spawn anyway and the
+  // recorder kept rolling silently. The fix mirrors onUserStoppedSharing:
+  // null the mediaStream FIRST so onSegmentStopped's gate sees the stopped
+  // state, then stop the recorder and release the tracks. This also fixes
+  // the secondary issue of leaked MediaStreamTracks (the picker indicator
+  // would have stayed up until the operator manually clicked "Stop
+  // sharing" in Chrome's UI).
   if (rotationTimerId !== null) {
     clearTimeout(rotationTimerId);
     rotationTimerId = null;
   }
+  // Capture stream before nulling so we can release its tracks.
+  const streamToStop = mediaStream;
+  mediaStream = null;
   if (videoRecorder !== null && videoRecorder.state !== 'inactive') {
+    try {
       videoRecorder.stop();
+    } catch (err) {
+      logger.warn('stop() during stopRecording failed:', err);
+    }
     logger.log('Recording stopped manually');
   }
+  if (streamToStop !== null) {
+    streamToStop.getTracks().forEach((t) => t.stop());
+  }
+  // Note: we do NOT call resetBuffer() here — the operator may want to
+  // STOP and then SAVE the buffered footage. resetBuffer happens on
+  // onUserStoppedSharing (operator dismissed Chrome's sharing indicator)
+  // and at the start of the next startRecording().
 }
 
 // ─── Bootstrap: handshake + port lifecycle (D-17, RESEARCH.md Patterns 4 & 5) ──

tests/offscreen/segment-rotation.test.ts

@@ -111,6 +111,25 @@ describe('segment rotation (D-13 restart-segments)', () => {
     expect(getSegments()).toEqual([]);
   });
 
+  it('resetBuffer preserves segments do NOT survive (sweep #1 stop-race adjacent)', () => {
+    // Sweep #1 documented that stopRecording() previously raced
+    // onSegmentStopped: the timer was cleared but onSegmentStopped's
+    // mediaStream != null branch would still spawn a new segment. The
+    // fix nulls mediaStream FIRST. resetBuffer is the orthogonal
+    // mechanism for clearing the buffer (used by onUserStoppedSharing
+    // AND between tests). This test pins the contract that resetBuffer
+    // never leaves a partial segment behind.
+    pushSegmentForTest(makeSegmentBlob(0xa));
+    pushSegmentForTest(makeSegmentBlob(0xb));
+    pushSegmentForTest(makeSegmentBlob(0xc));
+    expect(getSegments()).toHaveLength(MAX_SEGMENTS);
+    resetBuffer();
+    expect(getSegments()).toHaveLength(0);
+    // Subsequent pushes start from an empty buffer — no eviction needed.
+    pushSegmentForTest(makeSegmentBlob(0xd));
+    expect(getSegments()).toHaveLength(1);
+  });
+
   it('getSegments returns a snapshot — mutating the result does not corrupt internal state', () => {
     pushSegmentForTest(makeSegmentBlob(0x1));
     pushSegmentForTest(makeSegmentBlob(0x2));