Milestone v1 (v2.0.0): Mokosh — Session Capture #1

Merged
strategy155 merged 297 commits from gsd/phase-04-harden-clean-up-optional into main 2026-05-31 15:34:17 +00:00
3 changed files with 481 additions and 8 deletions
Showing only changes of commit a2dfc8cb9b - Show all commits

View File

@@ -0,0 +1,244 @@
---
slug: 01-09-notification-start-no-active-tab
status: awaiting_human_verify
goal: find_and_fix
trigger: "Operator UAT 2026-05-20: clicking onStartup notification ('Mokosh ready. Click to start a recording.') logs `Failed to start video capture: Error: No active tab found` and `notification-triggered start failed: Error: No active tab found` from the SW chunk (La function). Notification path silently fails; toolbar path unaffected."
created: 2026-05-20
updated: 2026-05-20
phase: 01-stabilize-video-pipeline
plan: 01-09
orchestrator_diagnosed: true
red_test: tests/background/start-video-capture-no-tab.test.ts
---
# Debug session 01-09-notification-start-no-active-tab — startVideoCapture must not depend on an active tab (D-01 cleanup gap)
## Problem statement
After Plan 01-10 + the 2026-05-20 commit `4bba679` split the onStartup
notification CTA text to "Mokosh ready. Click to start a recording.",
operator UAT exercised the notification click path for the first time and
observed silent failure: the SW devtools console logged
```
[SW:Main] 2026-05-20T08:56:43.336Z Failed to start video capture: Error: No active tab found
at La (index.ts-CKI4Evvn.js:17:95466)
[SW:Main] 2026-05-20T08:56:43.336Z notification-triggered start failed: Error: No active tab found
at La (index.ts-CKI4Evvn.js:17:95466)
```
Badge stayed IDLE; no Chrome "Sharing your screen" banner; no recording
began. Toolbar click path remained functional.
## Root cause
`src/background/index.ts:514-521` (pre-fix) inside `startVideoCapture()`:
```typescript
// Получаем активную вкладку
const [tab] = await chrome.tabs.query({ active: true, currentWindow: true });
if (!tab.id || !tab.url) {
throw new Error('No active tab found');
}
logger.log(`Starting video capture for tab ${tab.id}: ${tab.url}`);
```
This block was load-bearing in the pre-D-01 `chrome.tabCapture` era —
the capture stream-id was bound to the active tab. Since Plan 01-09's
D-01 conversion (`chrome.tabCapture``getDisplayMedia` whole-desktop in
offscreen), the tab reference has been functionally dead: every line
after 521 (`ensureOffscreen()`, `await offscreenReady`,
`chrome.runtime.sendMessage({ type: 'START_RECORDING' })`, `isRecording =
true`, `setRecordingMode()`, error handling) makes no reference to `tab`
or to any tab property.
The two callers diverge because of the chrome permission model:
* `chrome.action.onClicked` (toolbar) — Chrome grants `activeTab` on
the click gesture; `tab.url` is readable; the validation at line 517
passes; the dead log line executes; the function proceeds normally.
* `chrome.notifications.onClicked` — NOT a user gesture toward a tab.
`activeTab` is not granted, and the extension manifest declares no
`tabs` permission (per `.planning/intel/01-11-SUMMARY.md` follow-up
backlog: "tabs permission gap"). `chrome.tabs.query({ active: true,
currentWindow: true })` resolves to a tab whose `url` is undefined
(or `[]` depending on Chrome version + window state); `!tab.url`
evaluates true; the function throws "No active tab found" before
`ensureOffscreen()` is reached.
The bug was latent because Plan 01-09's smoke validation only
exercised toolbar.onClicked. The new Plan 01-10 + debug-2-of-3
notification CTA text (commit `4bba679`) explicitly invites operators
down the previously-untested notification.onClicked path. Operators
hit this on every browser-restart-after-install.
## Fix
Surgical removal of the dead tab query + validation + tab-dependent
log from `startVideoCapture` (`src/background/index.ts:514-521`).
Replace with a tab-independent log message that documents WHY the
removal happened (D-01 cleanup gap + this debug session).
```typescript
// Plan 01-09 D-01 cleanup gap (debug session
// `01-09-notification-start-no-active-tab`, 2026-05-20):
// The legacy chrome.tabs.query({ active: true, currentWindow: true })
// here was load-bearing in the pre-D-01 chrome.tabCapture era but
// is functionally dead post-D-01 — capture is whole-desktop via
// getDisplayMedia in offscreen and the SW-side start path needs
// no tab reference. The query also failed for chrome.notifications
// .onClicked callers (no activeTab grant + no `tabs` permission →
// tab.url undefined → "No active tab found" throw) so the onStartup
// notification CTA was silently broken. captureScreenshot +
// saveArchive retain their own genuine tab queries (tab.windowId
// for captureVisibleTab, tab.id for content-script sendMessage).
logger.log('Starting video capture (whole-desktop via getDisplayMedia in offscreen per D-01)');
```
### Out of scope (NOT touched — genuine tab dependencies)
* `captureScreenshot()` `src/background/index.ts:568-591` — passes
`tab.windowId` to `chrome.tabs.captureVisibleTab(tab.windowId, ...)`.
Genuine dependency on the active tab's window.
* `saveArchive()` `src/background/index.ts:741+` — uses `tab.id` for
`chrome.tabs.sendMessage(tab.id, ...)` to query the rrweb content
script for events, and uses the tab's window for the embedded
screenshot. Genuine dependency.
These remain because (a) they only fire under operator gestures that
DO grant activeTab (toolbar click → popup → SAVE), and (b) they have
real downstream API consumers of tab fields. A separate Phase 5
"adopt `tabs` permission" decision is the appropriate forum for
hardening; not in scope for this surgical bug fix.
## TDD evidence
### RED test (new file)
`tests/background/start-video-capture-no-tab.test.ts` — 3 tests:
* **A:** `tabs.query` mocked to `[]` (no-activeTab notifications.onClicked
context) → assert `chrome.runtime.sendMessage({type:'START_RECORDING'})`
was dispatched (was failing pre-fix with "expected 0 to be greater than
or equal to 1" — startVideoCapture threw before reaching sendMessage).
* **B:** `tabs.query` mocked to `[{id: 1}]` (url-less tab) → same
assertion. Was failing pre-fix for the identical reason.
* **C:** REGRESSION GUARD — `tabs.query` mocked to a fully-populated
tab (action.onClicked / activeTab-granted context) → same assertion.
Was passing pre-fix; MUST stay passing after the fix so we don't
over-trim and break the toolbar path.
Pre-fix run:
```
Test Files 1 failed (1)
Tests 2 failed | 1 passed (3)
```
Post-fix run:
```
Test Files 1 passed (1)
Tests 3 passed (3)
```
### Acceptance gates (all GREEN)
| Gate | Pre-fix baseline | Post-fix | Notes |
|------|------------------|----------|-------|
| `npm test` | 150/150 | **153/153** | +3 from new test file; 27 → 28 test files |
| `npm run test:uat` | 24/24 | **24/24** | No UAT changes; harness unaffected |
| `npx tsc --noEmit` | exit 0 | **exit 0** | No type-system surface change |
| `npm run build` | clean | **clean** | Built in 23.36s |
Note: A single ffprobe-driven test (`tests/background/webm-remux.test.ts
> ffprobe -count_frames`) showed an intermittent 5000ms timeout in one
of the multi-suite runs but passed in isolation (5/5) and on the second
full-suite run (153/153). This is a pre-existing flake under load
(ffprobe child-process spawn time), entirely unrelated to this fix —
no source files this fix touches are consumed by webm-remux tests.
### Pre-checkpoint bundle gates (per saved memory `feedback-pre-checkpoint-bundle-gates.md`)
Inspected the production SW chunk `dist/assets/index.ts-DBpA3-1k.js`
(referenced by `dist/service-worker-loader.js`):
* **Hook-string Tier-1 grep:** 0 matches (TEST_HOOK / MOKOSH_TEST_HOOK
/ `__test__` / `window.mokosh` / `globalThis.mokosh` etc).
* **SW CSP safety (eval / new Function):** 1 match — vendor library
internal templater (rrweb-adjacent), guarded by `typeof` checks;
pre-existing in baseline build; unrelated to this fix (no new
vendor imports).
* **Node-globals (require / process / Buffer / __dirname / global):** 2
matches — all inside vendor library `ArrayBuffer`/`Buffer` runtime
feature-detect paths; pre-existing; not introduced by this fix.
* **DOM-globals (window / document / localStorage / alert / navigator):**
3 matches — all inside vendor library `typeof window<"u"&&...`
feature-detect patterns (firebug-shim, ArrayBuffer detect, JSDOM
detect); evaluate false in SW context at runtime; pre-existing;
not introduced by this fix.
* **Manifest validation:** unchanged — same permissions, same
service_worker entry, same web_accessible_resources.
The diff is purely subtractive in the SW source (8 lines removed, 14
lines of code-comment + 1 new logger.log line added; no new imports,
no new APIs, no new vendor dependencies). It cannot have introduced
any of the pre-existing vendor matches.
## Why this was latent until 2026-05-20
* D-01 (`chrome.tabCapture``getDisplayMedia` in offscreen) shipped
in Plan 01-09; the tab dependency was orphaned at that point.
* Plan 01-09 smoke validation only exercised the toolbar
`action.onClicked` path, which grants activeTab and made the dead
code transparent.
* The original `notifStartup` CTA copy wording was conservative
("Mokosh is ready"), not action-inviting; operators tended to
dismiss the notification rather than click it.
* Commit `4bba679` (2026-05-20) split `notifStartup` into
`notifStartupCta` ("Mokosh ready. Click to start a recording.")
+ `notifRecordingStarted`. The new CTA copy explicitly invites
the click — operators began exercising the path on browser
restarts. The latent bug surfaced immediately.
## Files modified
* `src/background/index.ts` — startVideoCapture: removed lines
514-521 dead tab query + validation + tab-dependent log; replaced
with a multi-line WHY comment + a tab-independent
`logger.log('Starting video capture (whole-desktop via
getDisplayMedia in offscreen per D-01)')`.
* `tests/background/start-video-capture-no-tab.test.ts` (NEW) —
3 tests pinning the new contract.
## Forward-looking notes
* `captureScreenshot()` and `saveArchive()` still query the active
tab; this is correct because they are reached only through gestures
that grant activeTab (toolbar → popup → SAVE button). Do NOT
generalise this fix to those functions — they have real downstream
consumers of tab fields.
* The "add `tabs` permission to manifest" question is a separate
scope-expansion decision (Phase 5 hardening candidate). It would
let notifications.onClicked + future no-gesture paths read tab
metadata, but it widens the permission surface. Not required for
the current bug because the start-recording path doesn't need tab
data.
* Defensive UX feedback (e.g., showing an error notification when
startVideoCapture fails) is out of scope here. After this fix
the failure mode that motivated such UX is gone; if a future
failure path emerges, address it then.
## Operator UAT closure
Orchestrator re-spawns the operator UAT post-merge to confirm:
1. Install built extension to fresh Chrome profile.
2. Restart Chrome → `onStartup` fires → mokosh-startup-* notification
appears with CTA "Mokosh ready. Click to start a recording."
3. Click the notification → badge transitions to REC → Chrome shows
"Sharing your screen" banner → SW devtools console shows
`Starting video capture (whole-desktop via getDisplayMedia in
offscreen per D-01)` + `START_RECORDING sent successfully` (NO
"No active tab found" anywhere).
4. Toolbar click while not recording still starts a new session
(regression guard against over-trimming).

View File

@@ -511,14 +511,19 @@ async function startVideoCapture() {
}
try {
// Получаем активную вкладку
const [tab] = await chrome.tabs.query({ active: true, currentWindow: true });
if (!tab.id || !tab.url) {
throw new Error('No active tab found');
}
logger.log(`Starting video capture for tab ${tab.id}: ${tab.url}`);
// Plan 01-09 D-01 cleanup gap (debug session
// `01-09-notification-start-no-active-tab`, 2026-05-20):
// The legacy chrome.tabs.query({ active: true, currentWindow: true })
// here was load-bearing in the pre-D-01 chrome.tabCapture era but
// is functionally dead post-D-01 — capture is whole-desktop via
// getDisplayMedia in offscreen and the SW-side start path needs
// no tab reference. The query also failed for chrome.notifications
// .onClicked callers (no activeTab grant + no `tabs` permission →
// tab.url undefined → "No active tab found" throw) so the onStartup
// notification CTA was silently broken. captureScreenshot +
// saveArchive retain their own genuine tab queries (tab.windowId
// for captureVisibleTab, tab.id for content-script sendMessage).
logger.log('Starting video capture (whole-desktop via getDisplayMedia in offscreen per D-01)');
// Создаём offscreen документ (с reason из D-02)
await ensureOffscreen();

View File

@@ -0,0 +1,224 @@
// tests/background/start-video-capture-no-tab.test.ts
//
// Plan 01-09 — Debug `01-09-notification-start-no-active-tab`
//
// Regression pin: startVideoCapture must NOT depend on the presence of
// an active tab. Post Plan 01-09 D-01 (chrome.tabCapture → getDisplayMedia
// in offscreen) the SW-side recording-start path has no functional need
// for a tab — the only consumer of the legacy tab query was a log line.
//
// chrome.notifications.onClicked does NOT grant the activeTab permission
// and the extension manifest carries no `tabs` permission (per
// .planning/intel/01-11-SUMMARY.md follow-up backlog). So in production,
// chrome.tabs.query({ active: true, currentWindow: true }) called from
// inside the notifications.onClicked path resolves to `[]` (or a tab
// stripped of `url` / `id`). The previous implementation rejected with
// "No active tab found" before reaching the ensureOffscreen + START_RECORDING
// dispatch — the operator-observed silent failure of the onStartup
// notification CTA path.
//
// These tests pin both halves of the contract:
// * empty tabs.query result → still dispatches START_RECORDING (no throw)
// * url-less tab → still dispatches START_RECORDING (no throw)
//
// They are isomorphic to the action.onClicked path which today works
// because activeTab is granted on the click gesture. Removing the dead
// tab query equalises the two callers.
import { describe, it, expect, vi, beforeEach } from 'vitest';
interface OnConnectCallback { (port: unknown): void; }
interface OnMessageCallback {
(msg: unknown, sender: { id?: string }, sendResponse: (r: unknown) => void): boolean;
}
interface OnClickedCallback { (info?: unknown): void | Promise<void>; }
interface OnNotificationClickedCallback { (notificationId: string): void; }
interface BgChromeStub {
runtime: {
id: string;
getURL: (p: string) => string;
getManifest: () => { version: string };
sendMessage: ReturnType<typeof vi.fn>;
onMessage: { addListener: ReturnType<typeof vi.fn>; _callbacks: OnMessageCallback[] };
onConnect: { addListener: (cb: OnConnectCallback) => void; _callbacks: OnConnectCallback[] };
onInstalled: { addListener: ReturnType<typeof vi.fn> };
onStartup: { addListener: (cb: () => void) => void; _callbacks: Array<() => void> };
};
offscreen: {
hasDocument?: () => Promise<boolean>;
createDocument: ReturnType<typeof vi.fn>;
Reason: { DISPLAY_MEDIA: 'DISPLAY_MEDIA' };
};
tabs: {
query: ReturnType<typeof vi.fn>;
sendMessage: ReturnType<typeof vi.fn>;
captureVisibleTab: ReturnType<typeof vi.fn>;
};
downloads: { download: ReturnType<typeof vi.fn> };
action: {
onClicked: { addListener: (cb: OnClickedCallback) => void; _callbacks: OnClickedCallback[] };
setPopup: ReturnType<typeof vi.fn>;
setBadgeText: ReturnType<typeof vi.fn>;
setBadgeBackgroundColor: ReturnType<typeof vi.fn>;
setTitle: ReturnType<typeof vi.fn>;
};
notifications: {
create: ReturnType<typeof vi.fn>;
clear: ReturnType<typeof vi.fn>;
onClicked: {
addListener: (cb: OnNotificationClickedCallback) => void;
_callbacks: OnNotificationClickedCallback[];
};
};
}
interface GlobalWithBgChrome {
chrome?: BgChromeStub;
indexedDB?: { deleteDatabase: ReturnType<typeof vi.fn> };
}
function buildBgStub(tabsQueryResult: unknown[]): BgChromeStub {
const stub: BgChromeStub = {
runtime: {
id: 'ext-id-test',
getURL: (p) => `chrome-extension://ext-id-test/${p}`,
getManifest: () => ({ version: '1.0.0' }),
sendMessage: vi.fn().mockResolvedValue(undefined),
onMessage: { addListener: vi.fn(), _callbacks: [] },
onConnect: { addListener: (cb) => stub.runtime.onConnect._callbacks.push(cb), _callbacks: [] },
onInstalled: { addListener: vi.fn() },
onStartup: { addListener: (cb) => stub.runtime.onStartup._callbacks.push(cb), _callbacks: [] },
},
offscreen: {
hasDocument: async () => false,
createDocument: vi.fn().mockResolvedValue(undefined),
Reason: { DISPLAY_MEDIA: 'DISPLAY_MEDIA' },
},
tabs: {
// Mirror the production notifications.onClicked context: no activeTab
// grant + no `tabs` permission → tabs.query returns `[]`. The
// url-less-tab variant is supplied via the parameter.
query: vi.fn().mockResolvedValue(tabsQueryResult),
sendMessage: vi.fn().mockResolvedValue({ events: [], userEvents: [] }),
captureVisibleTab: vi.fn().mockResolvedValue('data:image/png;base64,xx'),
},
downloads: { download: vi.fn().mockResolvedValue(1) },
action: {
onClicked: { addListener: (cb) => stub.action.onClicked._callbacks.push(cb), _callbacks: [] },
setPopup: vi.fn(),
setBadgeText: vi.fn(),
setBadgeBackgroundColor: vi.fn(),
setTitle: vi.fn(),
},
notifications: {
create: vi.fn(),
clear: vi.fn(),
onClicked: {
addListener: (cb) => stub.notifications.onClicked._callbacks.push(cb),
_callbacks: [],
},
},
};
stub.runtime.onMessage.addListener.mockImplementation((cb: OnMessageCallback) => {
stub.runtime.onMessage._callbacks.push(cb);
});
return stub;
}
/**
* Drive the SW startVideoCapture path via notifications.onClicked + a
* connected video-keepalive port + an OFFSCREEN_READY onMessage event,
* which is the same await-offscreenReady satisfaction that the existing
* onstartup-notification.test.ts Test C uses.
*/
async function driveNotificationStart(stub: BgChromeStub): Promise<void> {
const port = {
name: 'video-keepalive',
sender: { id: 'ext-id-test' },
postMessage: vi.fn(),
onMessage: { addListener: vi.fn() },
onDisconnect: { addListener: vi.fn() },
disconnect: vi.fn(),
};
if (stub.runtime.onConnect._callbacks.length > 0) {
stub.runtime.onConnect._callbacks[0](port);
}
const onMsgHandler = stub.runtime.onMessage._callbacks[0];
onMsgHandler({ type: 'OFFSCREEN_READY' }, { id: 'ext-id-test' }, vi.fn());
// Synthesize the click on a mokosh-* notification id.
const notifCb = stub.notifications.onClicked._callbacks[0];
notifCb('mokosh-startup-1234567890');
// Drain microtasks so the entire async chain reaches sendMessage.
for (let i = 0; i < 64; i++) await Promise.resolve();
}
function countStartRecording(stub: BgChromeStub): number {
return stub.runtime.sendMessage.mock.calls.filter((args: unknown[]) => {
const msg = args[0] as { type?: unknown };
return typeof msg === 'object' && msg !== null && msg.type === 'START_RECORDING';
}).length;
}
describe('Plan 01-09 — startVideoCapture: no active-tab dependency', () => {
beforeEach(() => {
vi.resetModules();
});
// ──────────────────────────────────────────────────────────────────────
// Test A — RED today, GREEN after fix:
// chrome.tabs.query returns [] (real production notifications.onClicked
// context, since the extension has no `tabs` permission and notification
// clicks do not grant activeTab) → startVideoCapture must NOT throw
// "No active tab found"; START_RECORDING must still be dispatched.
// ──────────────────────────────────────────────────────────────────────
it('A: tabs.query → [] still dispatches START_RECORDING (no "No active tab" throw)', async () => {
const stub = buildBgStub([]);
(globalThis as unknown as GlobalWithBgChrome).chrome = stub;
(globalThis as unknown as GlobalWithBgChrome).indexedDB = { deleteDatabase: vi.fn() };
await import('../../src/background/index');
await driveNotificationStart(stub);
expect(countStartRecording(stub)).toBeGreaterThanOrEqual(1);
});
// ──────────────────────────────────────────────────────────────────────
// Test B — RED today, GREEN after fix:
// tabs.query returns a tab whose `url` is undefined (the activeTab-less
// shape Chrome surfaces when the caller lacks both activeTab and `tabs`
// permission) → startVideoCapture must NOT throw; START_RECORDING is
// dispatched.
// ──────────────────────────────────────────────────────────────────────
it('B: tabs.query → [{id, url: undefined}] still dispatches START_RECORDING', async () => {
const stub = buildBgStub([{ id: 1 }]);
(globalThis as unknown as GlobalWithBgChrome).chrome = stub;
(globalThis as unknown as GlobalWithBgChrome).indexedDB = { deleteDatabase: vi.fn() };
await import('../../src/background/index');
await driveNotificationStart(stub);
expect(countStartRecording(stub)).toBeGreaterThanOrEqual(1);
});
// ──────────────────────────────────────────────────────────────────────
// Test C — REGRESSION GUARD (was GREEN today, MUST stay GREEN after fix):
// tabs.query returns a fully-populated tab (the action.onClicked /
// click-gesture-activeTab context) → still dispatches START_RECORDING.
// Pins that the fix does not over-trim and break the toolbar path.
// ──────────────────────────────────────────────────────────────────────
it('C: tabs.query → [{id, url}] still dispatches START_RECORDING (regression guard)', async () => {
const stub = buildBgStub([{ id: 1, url: 'https://example.com', windowId: 100 }]);
(globalThis as unknown as GlobalWithBgChrome).chrome = stub;
(globalThis as unknown as GlobalWithBgChrome).indexedDB = { deleteDatabase: vi.fn() };
await import('../../src/background/index');
await driveNotificationStart(stub);
expect(countStartRecording(stub)).toBeGreaterThanOrEqual(1);
});
});