feat(browser): Vivaldi app-mode single window #6
No reviewers
Labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
slaid098/voice_assistant!6
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/browser/vivaldi-app-mode"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Что сделано
webbrowser.openзаменён наsubprocess.Popen([vivaldi, "--app=<url>"])— открывает видео в app-режиме (одно окно без вкладок и панели управления)taskkill /IM vivaldi.exe— старое app-окно закрывается, копления вкладок нетvivaldi.exeна Windows (LOCALAPPDATA, Program Files, Program Files (x86))webbrowser.openна не-Windows (Linux для dev/CI)BROWSER_PATHвconfig.pyи.env.template(пусто = авто-detect)BrowserStateхранит флаг открытого окна (PID не сохраняется)test_browser.pyпереписаны: app-mode, taskkill перед новым окном, авто-detect, fallback, Vivaldi не найденПочему
Вкладки копились в браузере — ПК зависал от десятков открытых YouTube-вкладок. Мама слепая, не видит панель вкладок и не может их закрыть. App-режим Vivaldi открывает каждое видео в одном окне без интерфейса, а
taskkillперед новым запуском гарантирует одно активное окно всегда.Watch out
taskkill /IM vivaldi.exeзакрывает весь Vivaldi (все окна, не только app-окна). Приемлемо: пользователь работает с одной вкладкой всегда, обычных окон Vivaldi не держит. Отличить app-окно от обычного по image name нельзя — app-режим запускает отдельный процесс, но image name общий.BROWSER_PATHпусто по умолчанию — на Windows авто-detect ищет vivaldi.exe в типичных расположениях. Если Vivaldi установлен в нестандартное место — задаётся через env.webbrowser.openбез taskkill. В app-режиме открывается только на Windows.Pending
Closes #2
Closes #2
Code Review Summary
Summary
PR реализует app-режим Vivaldi для одного окна без вкладок — архитектурно чисто (Protocol-совместимый
BrowserState, fallback на не-Windows, моки subprocess в тестах). Но найдена критическая несогласованность: настройкаBROWSER_PATHдобавлена вconfig.py,.env.templateи README, ноbrowser.pyеё не читает — dead config, нарушение контракта PR.Critical (must fix before merge)
BROWSER_PATHне используется — dead config.PR body заявляет: «Новая настройка
BROWSER_PATHвconfig.pyи.env.template(пусто = авто-detect)». README: «Путь к Vivaldi (пусто = авто-detect на Windows)»..env.template: «Путь к Vivaldi (если пусто — авто-detect на Windows)». Ноbrowser.pyне импортируетsettingsи не читаетsettings.browser_path— вызывается только_detect_vivaldi_path(). Пользователь задаётBROWSER_PATH=C:\My\Vivaldi\vivaldi.exeв нестандартном расположении →config.pyзагружает значение →browser.pyигнорирует → авто-detect не находит (4 типичных пути) → лог «Vivaldi не найден — задайте BROWSER_PATH» (но задание ничего не даёт).Fix: в
open_urlпроверитьsettings.browser_pathперед авто-detect: Добавить тестtest_open_url_uses_browser_path_from_config— мокsettings.browser_path="C:\\Custom\\vivaldi.exe", assertPopenвызван с этим путём,_detect_vivaldi_pathне вызывается.Cross-file impact: missing paired update
PR #6 меняет writer
config.py:55,211(добавляетbrowser_path: strполе +BROWSER_PATHenv var).browser.py:open_url— предполагаемый readersettings.browser_path— НЕ обновлён:config.py:55пишетbrowser_pathвSettings(writer)browser.py:88читает только_detect_vivaldi_path()(reader не подключён)config.pyменяет контракт (новое поле) →browser.pyдолжен его потреблять → иначе dead configТЗ на fix:
open_urlдолжен использоватьsettings.browser_path(если задан) с fallback на_detect_vivaldi_path()src/voice_assistant/services/browser.py:88browser_path = settings.browser_path or _detect_vivaldi_path(); если оба None → лог errortests/test_browser.py— добавитьtest_open_url_uses_browser_path_from_configДобавьте fix в этот PR. ~3 строки в browser.py + ~15 строк тестов.
Warnings (should fix)
src/voice_assistant/services/browser.py:84 [error-handling]
except Exception— слишком broad для fallback.webbrowser.openбросаетwebbrowser.Error(илиOSError). Сузить доexcept (webbrowser.Error, OSError) as ex.Fix:
except (webbrowser.Error, OSError) as ex:src/voice_assistant/services/browser.py:102 [robustness]
subprocess.Popen([browser_path, f"--app={url}"])безstdout=DEVNULL, stderr=DEVNULL— Vivaldi может писать шум в консоль ассистента (chromium logs). Для end-user приложения это шум.Fix:
subprocess.Popen([browser_path, f"--app={url}"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)PR body vs code [docs] PR body: «Авто-detect пути к
vivaldi.exeна Windows (LOCALAPPDATA, Program Files, Program Files (x86))» — 3 пути. Код (browser.py:42-58) проверяет 4 пути: LOCALAPPDATA, USERPROFILE, Program Files, Program Files (x86). Расхождение body↔code — обновить body или README.Fix: уточнить в PR body «4 типичных расположения».
Positives
--app=<url>— корректное решение для одного окна без вкладок (Chrome/Vivaldi app mode).taskkillперед новым запуском — гарантирует одно активное окно, решает исходную проблему #2 (копление вкладок).webbrowser.openдля Linux/dev/CI — тестируемость сохранена.# noqa: S603обоснован: путь из_detect_vivaldi_pathпроверенos.path.isfile.# noqa: S607дляtaskkill— системная утилита Windows, ок._has_open_windowфлаг) — соответствует требованию, упрощает state.BROWSER_PROFILEне добавлен — корректно (uBlock не нужен, Vivaldi блокирует рекламу по умолчанию).subprocess.Popen/subprocess.run— реальный Vivaldi не запускается, CI-safe.platform.system()вместоsys.platform— корректнее для detect (работает и в CI-контейнерах).## Что сделано/## Почему/## Watch out/## Pending— все заполнены осмысленно,Watch outчестно описывает ограничение taskkill (закрывает весь Vivaldi).Verdict: REQUEST_CHANGES
Code Review Summary
Re-review после фиксов (
baf64c8,091bd77). Все 4 замечания из предыдущего review проверены — 3 исправлены, 1 (расхождение body/кода) осознанно проигнорировано как не-блокер.Проверка замечаний
BROWSER_PATH не читался — ✅ исправлено.
browser.py:89теперьsettings.browser_path or _detect_vivaldi_path(): если путь задан в settings, авто-detect не вызывается. Тестtest_browser_path_from_settingsверифицирует это через_fail_if_calledsentinel — авто-detect падает с AssertionError, если вызван, и тест подтверждаетdetect_called["yes"] is False.except Exceptionслишком broad — ✅ исправлено.browser.py:85сужен доexcept (webbrowser.Error, OSError)(fallback на не-Windows),browser.py:110—except OSError(запуск Vivaldi). Тестыtest_open_url_non_windows_webbrowser_errorиtest_open_url_non_windows_oserror_fallbackпокрывают обе ветки.subprocess.Popenбез DEVNULL — ✅ исправлено.browser.py:105-106добавленыstdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL. Тесты верифицируют черезassert_called_once_with(..., stdout=DEVNULL, stderr=DEVNULL).Расхождение body/кода 3 vs 4 пути — осознанно проигнорировано (не блокер), ок.
Регрессии и новые проблемы
_current_url→_has_open_window: callers (handlers.py,youtube_flow.py) используют только публичные функцииopen_browser_urlиget_current_title— не затронуты. Проверено черезrg.uv run pytest tests/test_browser.py).Positives
_detect_vivaldi_path()хорошо структурирован: 4 кандидата (LOCALAPPDATA, USERPROFILE-derived, Program Files, Program Files x86), проверка черезos.path.isfile, ранний return на не-Windows.dataclasses.replace(settings, browser_path=...)для изоляции — чистый подход без мутации глобального settings.noqa: S603/S607аннотированы осознанно (subprocess с динамическим путём / taskkill без полного пути).Suggestions (info, not blocking)
if self._has_open_window— taskkill вызывается только если ранее уже открывали окно. Если пользователь вручную закрыл Vivaldi,_has_open_windowостаётся True → taskkill будет вызван впустую при следующем open_url. Это безвредно (taskkill сcheck=False), но можно отметить как известный trade-off. Не блокер.Verdict: APPROVE