mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-03-26 00:26:05 -07:00
69 lines
3.6 KiB
Markdown
69 lines
3.6 KiB
Markdown
---
|
|
id: TASK-234
|
|
title: 'Address PR #35 latest CodeRabbit review round'
|
|
status: Done
|
|
assignee:
|
|
- codex
|
|
created_date: '2026-03-26 03:59'
|
|
updated_date: '2026-03-26 04:01'
|
|
labels:
|
|
- review-comments
|
|
- coderabbit
|
|
dependencies: []
|
|
references:
|
|
- /Users/sudacode/projects/japanese/SubMiner/src/main.ts
|
|
- /Users/sudacode/projects/japanese/SubMiner/src/cli/args.test.ts
|
|
- >-
|
|
/Users/sudacode/projects/japanese/SubMiner/src/main/runtime/cli-command-prechecks.test.ts
|
|
- >-
|
|
/Users/sudacode/projects/japanese/SubMiner/src/main/runtime/youtube-playback-launch.ts
|
|
priority: medium
|
|
---
|
|
|
|
## Description
|
|
|
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
|
Assess and implement the latest actionable CodeRabbit feedback on PR #35 for the Windows YouTube playback flow. Scope includes fixing the overlapping youtubePlay cleanup race in main runtime state and any low-risk follow-up test/clarity comments from the same review round.
|
|
<!-- SECTION:DESCRIPTION:END -->
|
|
|
|
## Acceptance Criteria
|
|
<!-- AC:BEGIN -->
|
|
- [x] #1 Overlapping youtubePlay requests no longer let an older flow clear active quit-on-disconnect/app-owned-flow state for a newer flow.
|
|
- [x] #2 Latest low-risk CodeRabbit test and clarity follow-ups for this PR round are addressed or intentionally rejected based on code verification.
|
|
- [x] #3 Relevant tests covering the touched areas pass locally.
|
|
<!-- AC:END -->
|
|
|
|
## Implementation Plan
|
|
|
|
<!-- SECTION:PLAN:BEGIN -->
|
|
1. Update runYoutubePlaybackFlowMain in src/main.ts to use a per-request generation guard around shared YouTube flow state so overlapping requests cannot clear the active timer, armed flag, or app-owned-flow marker for a newer request.
|
|
2. Address verified low-risk latest-round follow-ups: add direct startup-prereq assertions in src/cli/args.test.ts, extend side-effect assertions in src/main/runtime/cli-command-prechecks.test.ts, and rename the youtube-playback-launch polling variable for clarity.
|
|
3. Run targeted Bun tests for the touched areas and record results in the task notes/final summary.
|
|
<!-- SECTION:PLAN:END -->
|
|
|
|
## Implementation Notes
|
|
|
|
<!-- SECTION:NOTES:BEGIN -->
|
|
Implemented per-request youtubePlaybackFlowGeneration guard in src/main.ts so superseded youtubePlay flows cannot clear the active arm timer, armed flag, or app-owned-flow state for a newer request.
|
|
|
|
Added explicit startup-prereq assertions in src/cli/args.test.ts and stronger warmup/log side-effect assertions in src/main/runtime/cli-command-prechecks.test.ts for the latest CodeRabbit follow-ups.
|
|
|
|
Renamed youtube-playback-launch polling variable from pathChanged to pathDiffersFromInitial for accuracy without behavior change.
|
|
|
|
Verification: bun test src/cli/args.test.ts; bun test src/main/runtime/cli-command-prechecks.test.ts; bun test src/main/runtime/youtube-playback-launch.test.ts; bun run typecheck.
|
|
<!-- SECTION:NOTES:END -->
|
|
|
|
## Final Summary
|
|
|
|
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
|
Addressed the latest PR #35 CodeRabbit round by making YouTube playback flow cleanup generation-safe in src/main.ts. Overlapping youtubePlay requests now isolate timer/armed/app-owned-flow cleanup to the currently active request so an older flow cannot clear state for its replacement.
|
|
|
|
Also folded in the latest low-risk follow-ups: args tests now assert that youtube playback requires overlay startup prerequisites, cli-command precheck tests now assert warmup/log side effects for the youtube transition, and youtube-playback-launch.ts uses a clearer variable name for the initial-path comparison.
|
|
|
|
Verification:
|
|
- bun test src/cli/args.test.ts
|
|
- bun test src/main/runtime/cli-command-prechecks.test.ts
|
|
- bun test src/main/runtime/youtube-playback-launch.test.ts
|
|
- bun run typecheck
|
|
<!-- SECTION:FINAL_SUMMARY:END -->
|