fix: address PR review follow-ups

This commit is contained in:
2026-04-25 20:34:18 -07:00
parent ba48db6255
commit ac93a5bd2e
32 changed files with 345 additions and 40 deletions

View File

@@ -0,0 +1,51 @@
---
id: TASK-300
title: Fix transparent subtitle hover background config
status: Done
assignee: []
created_date: '2026-04-26 03:23'
updated_date: '2026-04-26 03:26'
labels:
- bug
- overlay
- config
dependencies: []
priority: medium
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
User reports setting subtitleStyle.hoverTokenBackgroundColor to transparent still renders default hover background in overlay subtitles.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 Transparent hoverTokenBackgroundColor is accepted by config resolution.
- [x] #2 Renderer applies transparent hover token background instead of falling back to default.
<!-- AC:END -->
## Implementation Plan
<!-- SECTION:PLAN:BEGIN -->
1. Reproduce config alias behavior with a failing config test.
2. Map subtitleStyle.hoverBackground to hoverTokenBackgroundColor in config resolution while keeping canonical key precedence.
3. Add renderer regression for transparent hover token background CSS variable.
4. Update docs and changelog fragment; run focused verification.
<!-- SECTION:PLAN:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
Local user config used subtitleStyle.hoverBackground, which was ignored because only subtitleStyle.hoverTokenBackgroundColor was recognized. Canonical key still takes precedence when both are present.
Verification passed: bun run test:config:src; bun test src/renderer/subtitle-render.test.ts; bun run changelog:lint; bun run docs:test; bun run docs:build.
<!-- SECTION:NOTES:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Implemented config compatibility for transparent hover token backgrounds. `subtitleStyle.hoverBackground` now maps to the canonical `subtitleStyle.hoverTokenBackgroundColor` during resolution, preserving canonical key precedence. Added regression coverage for the alias and renderer handling of `transparent`, documented the alias, and added a changelog fragment.
Verification: `bun run test:config:src`; `bun test src/renderer/subtitle-render.test.ts`; `bun run changelog:lint`; `bun run docs:test`; `bun run docs:build`.
<!-- SECTION:FINAL_SUMMARY:END -->

View File

@@ -0,0 +1,53 @@
---
id: TASK-301
title: Fix launcher-managed video close leaving background app alive
status: Done
assignee:
- Codex
created_date: '2026-04-26 03:29'
updated_date: '2026-04-26 03:33'
labels:
- bug
- launcher
- mpv
dependencies: []
priority: high
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Launcher/plugin-managed video playback should not leave the SubMiner background app or tray icon running after the video closes unless the user explicitly launched SubMiner in background mode via --background or by starting with no app arguments. This is a regression after crash-avoidance work that added background startup for launcher-managed playback.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 Closing a launcher-managed video exits the launcher-started SubMiner app/tray instead of leaving it alive.
- [x] #2 Explicit background launches still keep SubMiner alive after windows close.
- [x] #3 No-argument app startup behavior remains unchanged.
- [x] #4 Regression coverage exercises the launcher-managed playback shutdown lifecycle.
<!-- AC:END -->
## Implementation Plan
<!-- SECTION:PLAN:BEGIN -->
1. Add regression coverage first: plugin auto-start should tag launcher-managed playback, and app mpv shutdown handling should quit only when started in that managed playback mode.
2. Add a narrow CLI flag/state field for launcher-managed playback, separate from explicit persistent background mode.
3. Have plugin pass the new flag with its background start command.
4. On mpv shutdown/disconnect, request app quit only when managed playback mode is active; preserve explicit --background and no-arg startup persistence.
5. Run focused plugin/app tests, then relevant launcher/core gates if feasible.
<!-- SECTION:PLAN:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
Implemented managed playback shutdown by adding a `--managed-playback` app flag that the mpv plugin passes only for launcher-managed starts. The main mpv shutdown path now quits the app when initial args indicate managed playback, while explicit background/no-arg startup remains persistent. Added plugin start-gate and mpv protocol regression coverage.
<!-- SECTION:NOTES:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Launcher-managed video playback now exits the SubMiner background app/tray when mpv shuts down, avoiding a leftover background process after closing a launcher-started video. Explicit `--background` and no-argument app startup remain persistent because the quit path is gated on the new `--managed-playback` flag. The mpv plugin passes that flag for auto-started playback, and mpv protocol shutdown requests app quit only in that managed mode.
Verification: plugin start-gate regression coverage, mpv protocol shutdown regression coverage, CLI managed-playback parse coverage, plus broader local gates run before handoff.
<!-- SECTION:FINAL_SUMMARY:END -->