Files
SubMiner/backlog/tasks/task-286.1 - Assess-and-address-PR-49-subsequent-CodeRabbit-review-round.md
2026-04-11 21:45:52 -07:00

3.8 KiB

id, title, status, assignee, created_date, updated_date, labels, dependencies, references, parent_task_id, priority
id title status assignee created_date updated_date labels dependencies references parent_task_id priority
TASK-286.1 Assess and address PR #49 subsequent CodeRabbit review round Done
2026-04-11 23:14 2026-04-11 23:16
bug
code-review
windows
release
.github/workflows/prerelease.yml
src/window-trackers/mpv-socket-match.ts
src/window-trackers/win32.ts
src/core/services/overlay-shortcut.ts
TASK-286 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

  • #1 All still-actionable CodeRabbit comments in the latest PR #49 round are fixed or explicitly shown stale with evidence.
  • #2 Regression coverage is added or updated for any behavior-sensitive fix in workflow or Windows socket matching.
  • #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.