diff --git a/.github/workflows/prerelease.yml b/.github/workflows/prerelease.yml index daa76fd0..9b30e8cb 100644 --- a/.github/workflows/prerelease.yml +++ b/.github/workflows/prerelease.yml @@ -32,9 +32,9 @@ jobs: node_modules stats/node_modules vendor/subminer-yomitan/node_modules - key: ${{ runner.os }}-bun-${{ hashFiles('bun.lock', 'stats/bun.lock', 'vendor/subminer-yomitan/package-lock.json') }} + key: ${{ runner.os }}-${{ runner.arch }}-bun-${{ hashFiles('bun.lock', 'stats/bun.lock', 'vendor/subminer-yomitan/package-lock.json') }} restore-keys: | - ${{ runner.os }}-bun- + ${{ runner.os }}-${{ runner.arch }}-bun- - name: Install dependencies run: | @@ -103,9 +103,9 @@ jobs: stats/node_modules vendor/texthooker-ui/node_modules vendor/subminer-yomitan/node_modules - key: ${{ runner.os }}-bun-${{ hashFiles('bun.lock', 'stats/bun.lock', 'vendor/texthooker-ui/package.json', 'vendor/subminer-yomitan/package-lock.json') }} + key: ${{ runner.os }}-${{ runner.arch }}-bun-${{ hashFiles('bun.lock', 'stats/bun.lock', 'vendor/texthooker-ui/package.json', 'vendor/subminer-yomitan/package-lock.json') }} restore-keys: | - ${{ runner.os }}-bun- + ${{ runner.os }}-${{ runner.arch }}-bun- - name: Install dependencies run: | @@ -162,9 +162,9 @@ jobs: stats/node_modules vendor/texthooker-ui/node_modules vendor/subminer-yomitan/node_modules - key: ${{ runner.os }}-bun-${{ hashFiles('bun.lock', 'stats/bun.lock', 'vendor/texthooker-ui/package.json', 'vendor/subminer-yomitan/package-lock.json') }} + key: ${{ runner.os }}-${{ runner.arch }}-bun-${{ hashFiles('bun.lock', 'stats/bun.lock', 'vendor/texthooker-ui/package.json', 'vendor/subminer-yomitan/package-lock.json') }} restore-keys: | - ${{ runner.os }}-bun- + ${{ runner.os }}-${{ runner.arch }}-bun- - name: Validate macOS signing/notarization secrets run: | @@ -238,9 +238,9 @@ jobs: stats/node_modules vendor/texthooker-ui/node_modules vendor/subminer-yomitan/node_modules - key: ${{ runner.os }}-bun-${{ hashFiles('bun.lock', 'stats/bun.lock', 'vendor/texthooker-ui/package.json', 'vendor/subminer-yomitan/package-lock.json') }} + key: ${{ runner.os }}-${{ runner.arch }}-bun-${{ hashFiles('bun.lock', 'stats/bun.lock', 'vendor/texthooker-ui/package.json', 'vendor/subminer-yomitan/package-lock.json') }} restore-keys: | - ${{ runner.os }}-bun- + ${{ runner.os }}-${{ runner.arch }}-bun- - name: Install dependencies run: | @@ -306,9 +306,9 @@ jobs: path: | ~/.bun/install/cache node_modules - key: ${{ runner.os }}-bun-${{ hashFiles('bun.lock') }} + key: ${{ runner.os }}-${{ runner.arch }}-bun-${{ hashFiles('bun.lock') }} restore-keys: | - ${{ runner.os }}-bun- + ${{ runner.os }}-${{ runner.arch }}-bun- - name: Install dependencies run: bun install --frozen-lockfile @@ -356,20 +356,6 @@ jobs: run: | set -euo pipefail - if gh release view "${{ steps.version.outputs.VERSION }}" >/dev/null 2>&1; then - gh release edit "${{ steps.version.outputs.VERSION }}" \ - --draft=false \ - --prerelease \ - --title "${{ steps.version.outputs.VERSION }}" \ - --notes-file release/prerelease-notes.md - else - gh release create "${{ steps.version.outputs.VERSION }}" \ - --latest=false \ - --prerelease \ - --title "${{ steps.version.outputs.VERSION }}" \ - --notes-file release/prerelease-notes.md - fi - shopt -s nullglob artifacts=( release/*.AppImage @@ -386,6 +372,27 @@ jobs: exit 1 fi + if gh release view "${{ steps.version.outputs.VERSION }}" >/dev/null 2>&1; then + gh release edit "${{ steps.version.outputs.VERSION }}" \ + --draft \ + --prerelease \ + --title "${{ steps.version.outputs.VERSION }}" \ + --notes-file release/prerelease-notes.md + else + gh release create "${{ steps.version.outputs.VERSION }}" \ + --draft \ + --latest=false \ + --prerelease \ + --title "${{ steps.version.outputs.VERSION }}" \ + --notes-file release/prerelease-notes.md + fi + for asset in "${artifacts[@]}"; do gh release upload "${{ steps.version.outputs.VERSION }}" "$asset" --clobber done + + gh release edit "${{ steps.version.outputs.VERSION }}" \ + --draft=false \ + --prerelease \ + --title "${{ steps.version.outputs.VERSION }}" \ + --notes-file release/prerelease-notes.md diff --git a/backlog/tasks/task-286.1 - Assess-and-address-PR-49-subsequent-CodeRabbit-review-round.md b/backlog/tasks/task-286.1 - Assess-and-address-PR-49-subsequent-CodeRabbit-review-round.md new file mode 100644 index 00000000..9b86647f --- /dev/null +++ b/backlog/tasks/task-286.1 - Assess-and-address-PR-49-subsequent-CodeRabbit-review-round.md @@ -0,0 +1,61 @@ +--- +id: TASK-286.1 +title: 'Assess and address PR #49 subsequent CodeRabbit review round' +status: Done +assignee: [] +created_date: '2026-04-11 23:14' +updated_date: '2026-04-11 23:16' +labels: + - bug + - code-review + - windows + - release +dependencies: [] +references: + - .github/workflows/prerelease.yml + - src/window-trackers/mpv-socket-match.ts + - src/window-trackers/win32.ts + - src/core/services/overlay-shortcut.ts +parent_task_id: TASK-286 +priority: high +--- + +## Description + + +Track the next unresolved CodeRabbit review threads on PR #49 after commit 9ce5de2f and resolve the still-valid follow-up issues without reopening already-assessed stale comments. + + +## Acceptance Criteria + +- [x] #1 All still-actionable CodeRabbit comments in the latest PR #49 round are fixed or explicitly shown stale with evidence. +- [x] #2 Regression coverage is added or updated for any behavior-sensitive fix in workflow or Windows socket matching. +- [x] #3 Relevant verification passes for the touched workflow, tracker, and shared matcher changes. + + +## Implementation Plan + + +1. Verify the five unresolved CodeRabbit threads against current branch state and separate still-valid bugs from stale comments. +2. Add or extend the narrowest failing tests for exact socket-path matching and prerelease workflow invariants before changing production code. +3. Implement minimal fixes in the prerelease workflow and Windows socket matching/cache path, leaving stale comments documented with evidence instead of forcing no-op changes. +4. Run targeted verification, then record the fixed-vs-stale assessment and close the subtask. + + +## Implementation Notes + + +Assessed five unresolved PR #49 threads after 9ce5de2f. Fixed prerelease workflow cache keys to include `runner.arch`, changed prerelease publishing to validate artifacts before release creation/edit and only undraft after uploads complete, tightened Windows socket matching to require exact argument boundaries, and stopped memoizing null command-line lookup misses in the Win32 cache path. + +Stale assessment: the `src/core/services/overlay-shortcut.ts` thread is still obsolete. Current code at `registerOverlayShortcuts()` returns `hasConfiguredOverlayShortcuts(shortcuts)`, not `false`, and the overlay-local handling remains intentionally driven by local fallback dispatch rather than global registration in this runtime path. + +Verification: `bun test src/prerelease-workflow.test.ts src/window-trackers/mpv-socket-match.test.ts`, `bun test src/window-trackers/windows-tracker.test.ts src/prerelease-workflow.test.ts src/window-trackers/mpv-socket-match.test.ts`, `bun run typecheck`. + + +## Final Summary + + +Handled the next CodeRabbit round on PR #49 by fixing the still-valid prerelease workflow and Windows socket-matching issues while documenting the stale overlay-shortcut comment instead of forcing a no-op code change. The prerelease workflow now scopes all dependency caches by `runner.arch`, validates the final artifact set before touching the GitHub release, creates/edits the prerelease as a draft during uploads, and only flips `--draft=false` after all assets succeed. On Windows, socket matching now requires an exact `--input-ipc-server` argument boundary so `subminer-1` no longer matches `subminer-10`, and transient PowerShell/CIM misses no longer get cached forever as null command lines. + +Regression coverage was added for the workflow invariants and exact socket matching. Verification passed with targeted prerelease workflow tests, Windows tracker tests, socket-matcher tests, and `bun run typecheck`. + diff --git a/src/prerelease-workflow.test.ts b/src/prerelease-workflow.test.ts index ce30aedd..050564c1 100644 --- a/src/prerelease-workflow.test.ts +++ b/src/prerelease-workflow.test.ts @@ -34,6 +34,17 @@ test('prerelease workflow publishes GitHub prereleases and keeps them off latest assert.match(prereleaseWorkflow, /gh release create[\s\S]*--latest=false/); }); +test('prerelease workflow scopes dependency caches by runner architecture', () => { + const archScopedCacheKeyMatches = prereleaseWorkflow.match( + /key:\s*\${{\s*runner\.os\s*}}-\${{\s*runner\.arch\s*}}-bun-/g, + ); + const archScopedRestoreKeyMatches = prereleaseWorkflow.match( + /\${{\s*runner\.os\s*}}-\${{\s*runner\.arch\s*}}-bun-/g, + ); + assert.equal(archScopedCacheKeyMatches?.length, 5); + assert.ok((archScopedRestoreKeyMatches?.length ?? 0) >= 10); +}); + test('prerelease workflow builds and uploads all release platforms', () => { assert.match(prereleaseWorkflow, /build-linux:/); assert.match(prereleaseWorkflow, /build-macos:/); @@ -54,6 +65,21 @@ test('prerelease workflow publishes the same release assets as the stable workfl ); }); +test('prerelease workflow validates artifacts before publishing the release and only undrafts after upload', () => { + const artifactsIndex = prereleaseWorkflow.indexOf('artifacts=('); + const createIndex = prereleaseWorkflow.indexOf('gh release create'); + const uploadIndex = prereleaseWorkflow.indexOf('gh release upload'); + const undraftIndex = prereleaseWorkflow.indexOf('--draft=false'); + + assert.notEqual(artifactsIndex, -1); + assert.notEqual(createIndex, -1); + assert.notEqual(uploadIndex, -1); + assert.notEqual(undraftIndex, -1); + assert.ok(artifactsIndex < createIndex); + assert.ok(uploadIndex < undraftIndex); + assert.match(prereleaseWorkflow, /gh release create[\s\S]*--draft[\s\S]*--prerelease/); +}); + test('prerelease workflow does not publish to AUR', () => { assert.doesNotMatch(prereleaseWorkflow, /aur-publish:/); assert.doesNotMatch(prereleaseWorkflow, /AUR_SSH_PRIVATE_KEY/); diff --git a/src/window-trackers/mpv-socket-match.test.ts b/src/window-trackers/mpv-socket-match.test.ts index 83b11e99..bec350d8 100644 --- a/src/window-trackers/mpv-socket-match.test.ts +++ b/src/window-trackers/mpv-socket-match.test.ts @@ -54,3 +54,13 @@ test('filterMpvPollResultBySocketPath keeps only matches for the requested socke assert.deepEqual(result.matches.map((match) => match.hwnd), [2]); assert.equal(result.windowState, 'visible'); }); + +test('matchesMpvSocketPathInCommandLine rejects socket-path prefix matches', () => { + assert.equal( + matchesMpvSocketPathInCommandLine( + 'mpv.exe --input-ipc-server=\\\\.\\pipe\\subminer-10 video.mkv', + '\\\\.\\pipe\\subminer-1', + ), + false, + ); +}); diff --git a/src/window-trackers/mpv-socket-match.ts b/src/window-trackers/mpv-socket-match.ts index 724b95c1..c8d893b9 100644 --- a/src/window-trackers/mpv-socket-match.ts +++ b/src/window-trackers/mpv-socket-match.ts @@ -13,9 +13,10 @@ export function matchesMpvSocketPathInCommandLine( } const escapedSocketPath = escapeRegex(targetSocketPath); - return new RegExp(`--input-ipc-server(?:=|\\s+)("?${escapedSocketPath}"?)`, 'i').test( - commandLine, - ); + return new RegExp( + `(?:^|\\s)--input-ipc-server(?:=|\\s+)(?:"${escapedSocketPath}"|${escapedSocketPath})(?=\\s|$)`, + 'i', + ).test(commandLine); } export function filterMpvPollResultBySocketPath( diff --git a/src/window-trackers/win32.ts b/src/window-trackers/win32.ts index bde62f0e..8767b55f 100644 --- a/src/window-trackers/win32.ts +++ b/src/window-trackers/win32.ts @@ -173,7 +173,7 @@ function getProcessNameByPid(pid: number): string | null { } } -const processCommandLineCache = new Map(); +const processCommandLineCache = new Map(); function getProcessCommandLineByPid(pid: number): string | null { if (processCommandLineCache.has(pid)) { @@ -204,7 +204,11 @@ function getProcessCommandLineByPid(pid: number): string | null { commandLine = null; } - processCommandLineCache.set(pid, commandLine); + if (commandLine !== null) { + processCommandLineCache.set(pid, commandLine); + } else { + processCommandLineCache.delete(pid); + } return commandLine; }