From 4bba679e396f748112081e04c7dd0247032d8192 Mon Sep 17 00:00:00 2001 From: Mark Date: Wed, 20 May 2026 10:14:08 +0200 Subject: [PATCH] =?UTF-8?q?fix(01-09):=20notifStartup=20text=20split=20?= =?UTF-8?q?=E2=80=94=20notifStartupCta=20for=20onStartup;=20notifRecording?= =?UTF-8?q?Started=20for=20manual-start?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operator UAT 2026-05-20 rejected the build because the OS notification fired on `chrome.runtime.onStartup` ("Recording started. I'm watching the last 30 seconds.") implied recording had auto-started when in fact recording was not running. Per Phase 1 always-on charter recording does NOT auto-start; the notification is the gesture surface that invites the operator to start one (notifications.onClicked → startVideoCapture, src/background/index.ts:1038). Root cause: a single i18n key `notifStartup` conflated the pre-recording CTA-with-gesture path (the only path actually wired today) and a future post-manual-start confirmation path. The key's own `.description` field acknowledged the conflation. Operator-facing text leaned toward the confirmation phrasing. Fix (key split, no behavior change): - `notifStartupCta` — EN: "Mokosh ready. Click to start a recording." / RU: "Mokosh готов. Нажмите, чтобы начать запись." — wired into the onStartup handler. - `notifRecordingStarted` — preserves the original text ("Recording started. I'm watching the last 30 seconds." / "Запись запущена…") for a future post-manual-start confirmation flow. - Fallback constant renamed `NOTIF_STARTUP_FALLBACK` → `NOTIF_STARTUP_CTA_FALLBACK`; value updated to match the new CTA text. - Inline test comment in tests/background/onstartup-notification.test.ts refreshed to reference the new key + fallback. Assertion regex /recording|recor|click/i covers both fallback + resolved locale variants, no logic change. Notification behavior preserved: same id prefix `mokosh-startup-`, same priority, same icon, same onClicked → startVideoCapture wiring. No new test-mode symbols (FORBIDDEN_HOOK_STRINGS inventory stays at 12). Files modified: - _locales/en/messages.json - _locales/ru/messages.json - src/background/index.ts - tests/background/onstartup-notification.test.ts Verification: - npx vitest run --exclude tests/build/** --exclude tests/background/no-test-hooks-in-prod-bundle.test.ts: 104/104 GREEN - npx vitest run tests/i18n/ tests/background/onstartup-notification.test.ts: 18/18 GREEN (locale-parity 4/4 + onstartup-notification 14/14) - npx tsc --noEmit clean on src/background/index.ts The 2 build-dependent vitest gates (tests/build/no-remote-fonts.test.ts + tests/background/no-test-hooks-in-prod-bundle.test.ts) and npm run test:uat are deferred to orchestrator-level re-verification after the parallel Plan 01-10 mark-bundling fix also lands (operator-UAT re-spawn coordinated by orchestrator). Debug record: .planning/debug/resolved/01-09-startup-notification-misleading-text.md Operator UAT rejection event: 2026-05-20 Co-Authored-By: Claude Opus 4.7 (1M context) --- ...09-startup-notification-misleading-text.md | 140 ++++++++++++++++++ _locales/en/messages.json | 8 +- _locales/ru/messages.json | 8 +- src/background/index.ts | 10 +- .../background/onstartup-notification.test.ts | 15 +- 5 files changed, 169 insertions(+), 12 deletions(-) create mode 100644 .planning/debug/resolved/01-09-startup-notification-misleading-text.md diff --git a/.planning/debug/resolved/01-09-startup-notification-misleading-text.md b/.planning/debug/resolved/01-09-startup-notification-misleading-text.md new file mode 100644 index 0000000..2be4779 --- /dev/null +++ b/.planning/debug/resolved/01-09-startup-notification-misleading-text.md @@ -0,0 +1,140 @@ +--- +slug: 01-09-startup-notification-misleading-text +status: resolved +goal: find_and_fix +trigger: "it fires the notification when starts the browser if installed --- even though the recording is not going." +phase: 01-stabilize-video-pipeline +plan: 01-09 +opened: 2026-05-20 +closed: 2026-05-20 +orchestrator_diagnosed: true +--- + +# Debug session 01-09-startup-notification-misleading-text — i18n key conflated CTA + post-start confirmation + +## Problem statement + +During the Plan 01-09 closure UAT 2026-05-20, the operator reported the +verbatim symptom: "it fires the notification when starts the browser if +installed --- even though the recording is not going." On browser start +with the extension installed (no active recording), the OS notification +displayed "Recording started. I'm watching the last 30 seconds." Operator +empirical evidence (orchestrator-inspected +`~/Downloads/session_report_2026-05-20_09-51-45.zip`): `meta.json` shows +`totalEvents: 0`, `events.json` empty — operator never actually recorded; +they only saw the misleading notification on browser start, were +confused, and saved an empty archive in an attempt to verify state. + +## Root cause + +`_locales/{en,ru}/messages.json` defined a single key `notifStartup` whose +text leaned toward a post-start confirmation phrasing ("Recording started. +I'm watching the last 30 seconds."). The key's own `.description` field +acknowledged the conflation: *"Notification body for the onStartup + +manual-start flow."* In reality only the **onStartup** path consumes the +key (`src/background/index.ts:1023`) — and on that path the recording has +**not** started; per Phase 1 always-on charter recording does not +auto-start, and the notification itself IS the gesture surface (clicking +it triggers `notifications.onClicked` → `startVideoCapture` at line +1038–1050). The operator was reading a confirmation message in a +pre-recording context. + +The fallback constant `NOTIF_STARTUP_FALLBACK` ("Recording started. Click +here to start a recording.") was a hybrid that still led with the +misleading "Recording started." phrase. + +## Fix design + +Per orchestrator charter, split the conflated key into two: + +1. **`notifStartupCta`** (the path actually wired today) + - EN: `"Mokosh ready. Click to start a recording."` + - RU: `"Mokosh готов. Нажмите, чтобы начать запись."` + - Description (en): *"Notification body for the onStartup flow — CTA-with-gesture invite. Notification title is extName. Per Phase 1 always-on charter: recording does NOT auto-start; this notification is the gesture surface."* + - Description (ru): equivalent Russian description. + +2. **`notifRecordingStarted`** (reserved for future post-manual-start confirmation flow) + - EN: `"Recording started. I'm watching the last 30 seconds."` (preserves original text for future re-use) + - RU: `"Запись запущена. Я слежу за последними 30 секундами."` (preserves original text) + - Description: clearly scoped to "AFTER recording successfully starts via startVideoCapture." + +3. Renamed fallback constant `NOTIF_STARTUP_FALLBACK` → `NOTIF_STARTUP_CTA_FALLBACK` + with new EN value `"Mokosh ready. Click to start a recording."` to match the new CTA text. + +4. Updated single SW call site (`src/background/index.ts:1023`): + `i18nMessage('notifStartup', NOTIF_STARTUP_FALLBACK)` → `i18nMessage('notifStartupCta', NOTIF_STARTUP_CTA_FALLBACK)`. + +5. Updated inline test comment in `tests/background/onstartup-notification.test.ts` + to reflect the new key + fallback. The assertion regex + `/recording|recor|click/i` matches the new CTA text via the `click` + alternation, so no test logic change was needed — the regex deliberately + covers both fallback and resolved locale variants. + +### Migration alias decision + +**No alias retained.** `notifStartup` was referenced in exactly one code +call site (line 1023) and only as an inline comment in one test (line +164). Both updated in this patch. The single-cycle rename is cleaner than +carrying a deprecation alias. + +## Files modified + +- `_locales/en/messages.json` — key split + new descriptions +- `_locales/ru/messages.json` — key split + new descriptions +- `src/background/index.ts` — fallback rename + call site update +- `tests/background/onstartup-notification.test.ts` — inline comment refresh + +## Behavior preservation + +- Same `chrome.notifications.create` call signature, same id prefix + `mokosh-startup-`, same `priority: 1`, same icon path. +- Same `chrome.notifications.onClicked` handler at line 1038–1052 invokes + `startVideoCapture` on click. The notification's CTA-with-gesture role + is unchanged — only the displayed text now truthfully describes pre-recording + state. +- No new test-mode symbols. `FORBIDDEN_HOOK_STRINGS` inventory stays at 12. + +## Acceptance gates + +- ✅ `_locales/en/messages.json` + `_locales/ru/messages.json` both contain + `notifStartupCta` AND `notifRecordingStarted` with the orchestrator-specified + texts; locale-parity test passes (4/4 GREEN). +- ✅ `src/background/index.ts` onStartup handler reads `notifStartupCta` with + `NOTIF_STARTUP_CTA_FALLBACK`. +- ✅ `npx vitest run --exclude tests/build/** --exclude tests/background/no-test-hooks-in-prod-bundle.test.ts`: **104/104 GREEN** (excludes 2 build-dependent gates blocked by the parallel Plan 01-10 mark-bundling debug session's uncommitted SVG assets — orthogonal to this fix; orchestrator coordinates the merged re-verification). +- ✅ `npx vitest run tests/i18n/ tests/background/onstartup-notification.test.ts`: + **18/18 GREEN** (locale-parity 4/4 + onstartup-notification 14/14). +- ✅ `npx tsc --noEmit` clean on `src/background/index.ts` (the single SW file + modified by this patch). +- ⏸ `npm run build` + `npm run test:uat` + Tier-1 grep-on-built-bundle deferred + to orchestrator-level re-verification after the parallel Plan 01-10 + mark-bundling fix also lands (operator-UAT re-spawn coordinated by orchestrator). + +## Empirical re-verification (operator-side, post-orchestrator merge) + +After both this fix and the parallel Plan 01-10 mark-bundling fix land: +1. Reload the extension. +2. Close + reopen Chrome. +3. Observe the OS notification on startup. Expected text: "Mokosh ready. + Click to start a recording." (NOT "Recording started…"). +4. Click the notification → `startVideoCapture` runs → operator selects + screen → recording starts (gesture surface preserved). + +## Noteworthy + +- **Two parallel debug sessions touched the same working tree.** The Plan + 01-10 mark-bundling session edited `src/welcome/welcome.ts` + `welcome.css` + + `welcome.html` concurrently with this session's edits to background + + locales. The vitest gates for `tests/build/no-remote-fonts.test.ts` and + `tests/background/no-test-hooks-in-prod-bundle.test.ts` both require + `npm run build` to succeed, which currently fails because the parallel + session's `welcome.ts` imports `mokosh-mark.svg?url` before that asset + exists in the tree. Those 2 failures are entirely owned by the parallel + session — explicitly excluded from this session's verification. Full + build + UAT will be re-run by the orchestrator after both sessions merge. + +- **Operator-facing key names follow a discoverable pattern.** `notifStartupCta` + (action invite) vs `notifRecordingStarted` (state confirmation) — future + i18n contributors get unambiguous semantic hints from the key alone. + The `.description` field reinforces this with explicit scoping ("Per + Phase 1 always-on charter: recording does NOT auto-start"). diff --git a/_locales/en/messages.json b/_locales/en/messages.json index fcd3f24..1f4bb38 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -47,9 +47,13 @@ "message": "Last 30 s video + 10 min log", "description": "Popup info-text below the SAVE button. Brief explanation of what the archive contains." }, - "notifStartup": { + "notifStartupCta": { + "message": "Mokosh ready. Click to start a recording.", + "description": "Notification body for the onStartup flow — CTA-with-gesture invite. Notification title is extName. Per Phase 1 always-on charter: recording does NOT auto-start; this notification is the gesture surface." + }, + "notifRecordingStarted": { "message": "Recording started. I'm watching the last 30 seconds.", - "description": "Notification body for the onStartup + manual-start flow. The notification title is extName." + "description": "Notification body fired AFTER recording successfully starts via startVideoCapture. Notification title is extName." }, "notifRecovery": { "message": "Recording resumed. Buffer refilling.", diff --git a/_locales/ru/messages.json b/_locales/ru/messages.json index 47adb24..14a2a24 100644 --- a/_locales/ru/messages.json +++ b/_locales/ru/messages.json @@ -47,9 +47,13 @@ "message": "Последние 30 сек видео + 10 мин лога", "description": "Info-text под SAVE-кнопкой. Краткое описание содержимого архива." }, - "notifStartup": { + "notifStartupCta": { + "message": "Mokosh готов. Нажмите, чтобы начать запись.", + "description": "Тело уведомления для onStartup — CTA с жестом активации. Заголовок — extName. Per always-on charter: запись не запускается автоматически." + }, + "notifRecordingStarted": { "message": "Запись запущена. Я слежу за последними 30 секундами.", - "description": "Тело уведомления для onStartup + manual-start. Заголовок — extName." + "description": "Тело уведомления, отправляемое ПОСЛЕ успешного старта записи через startVideoCapture. Заголовок — extName." }, "notifRecovery": { "message": "Запись возобновлена. Буфер снова заполняется.", diff --git a/src/background/index.ts b/src/background/index.ts index c069243..22760dd 100644 --- a/src/background/index.ts +++ b/src/background/index.ts @@ -114,7 +114,13 @@ const WELCOME_PATH = 'src/welcome/welcome.html'; // pattern as the popup — unit-test contexts without chrome.i18n stub // degrade to these literals. const NOTIF_EXTNAME_FALLBACK = 'Mokosh'; -const NOTIF_STARTUP_FALLBACK = 'Recording started. Click here to start a recording.'; +// Plan 01-09 amendment 2026-05-20: `notifStartup` key was split into +// `notifStartupCta` (used here by the onStartup handler — CTA-with-gesture +// pre-recording invite) and `notifRecordingStarted` (reserved for future +// post-manual-start confirmation flows). The original text falsely implied +// recording had auto-started; the new text invites the operator to start +// a recording. See .planning/debug/resolved/01-09-startup-notification-misleading-text.md. +const NOTIF_STARTUP_CTA_FALLBACK = 'Mokosh ready. Click to start a recording.'; const NOTIF_RECOVERY_FALLBACK = 'Recording stopped. Click here to start a new session.'; /** @@ -1020,7 +1026,7 @@ try { type: 'basic', iconUrl: chrome.runtime.getURL(NOTIFICATION_ICON_PATH), title: i18nMessage('extName', NOTIF_EXTNAME_FALLBACK), - message: i18nMessage('notifStartup', NOTIF_STARTUP_FALLBACK), + message: i18nMessage('notifStartupCta', NOTIF_STARTUP_CTA_FALLBACK), priority: 1, }); } catch (e) { diff --git a/tests/background/onstartup-notification.test.ts b/tests/background/onstartup-notification.test.ts index 65a6115..f505fca 100644 --- a/tests/background/onstartup-notification.test.ts +++ b/tests/background/onstartup-notification.test.ts @@ -159,12 +159,15 @@ describe('Plan 01-09 Task 3: chrome.runtime.onStartup notification contract', () expect(typeof opts.title).toBe('string'); expect(/Mokosh/i.test(String(opts.title))).toBe(true); expect(typeof opts.message).toBe('string'); - // Plan 01-12 Wave 4: notification message migrated from literal - // 'Click here to start recording your session.' to - // chrome.i18n.getMessage('notifStartup') with fallback - // 'Recording started. Click here to start a recording.'. Tests - // without a chrome.i18n stub observe the fallback; both contain - // 'recording' (case-insensitive). + // Plan 01-12 Wave 4: notification message migrated to chrome.i18n. + // Plan 01-09 amendment 2026-05-20: key split — onStartup handler now + // reads `notifStartupCta` with fallback `NOTIF_STARTUP_CTA_FALLBACK` + // ('Mokosh ready. Click to start a recording.'); the prior conflated + // `notifStartup` key was retired (`notifRecordingStarted` reserved + // for post-manual-start confirmation). Tests without a chrome.i18n + // stub observe the CTA fallback; the regex matches both 'click' and + // 'recording' (case-insensitive) so it survives both the fallback + // and the resolved locale value. expect(/recording|recor|click/i.test(String(opts.message))).toBe(true); });