chore: reconcile main release line into develop#185
Merged
Conversation
- Add RehearsalWorkspace and SongRehearsalPack - Setup role-aware stem playback and section maps - Update UI to support low-confidence fallback buckets - Apply strict JobQueueManager with concurrency limit
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
v0.1.1 feat: implement rehearsal workspace design
* fix(ci): add --repo flag to gh release upload to fix git error * trigger ci
* test: update test_release_packaging to match installer packaging logic * trigger ci * style: fix ruff line length warnings in test_release_packaging.py * style: run ruff format on test_release_packaging.py * fix: update package_desktop_artifact.py to handle installers and extensions
* fix(release): build actual desktop installers instead of raw binaries in zip This updates the CI to build Tauri properly so users get DMG/EXE/MSI files instead of zip files containing just the executable and raw frontend files. * trigger ci * fix(ci): pass --bundles to ensure Tauri builds installers
* feat(ui): redesign workspace with shadcn and tailwind v4 This refactors the frontend to use Tailwind CSS v4 and Shadcn UI components. It improves the layout and typography of the App, Workspace, RoleSwitcher, and SectionRoadmap. * trigger ci * fix(ci): update package-lock.json to sync with package.json * fix(ci): silence tsconfig deprecation warning
* fix(ui): update generic app title and subtitle to product copy * fix(ui): update generic baseline text to clearer functionality text * fix(desktop): stabilize browser YouTube import fallback
fix(release): publish immutable release assets before publication
Prepare v0.1.3 as the recovery release for the immutable v0.1.2 asset upload failure.
Give the desktop workspace a rehearsal-first cockpit while tightening section timing and URL/project boundaries across the app stack.
…esign feat(ui): finalize rehearsal console redesign
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit릴리스 노트
WalkthroughCI·릴리스가 태그-기반 draft→검증→게시로 재구성되고 Bandit 스캔이 추가되었습니다. 데스크톱 앱은 명시적 분석 작업(start/poll/result) 라이프사이클로 리팩토링되었고 YouTube·로컬 파일 검증과 Section.timeRange가 엄격화되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "사용자"
participant App as "데스크톱 앱\n(App.tsx)"
participant Tauri as "Tauri 브리지\n(window.__TAURI_INTERNALS__ / __TAURI_INVOKE__)"
participant Engine as "분석 엔진\n(service)"
User->>App: 로컬 오디오 또는 YouTube 선택
App->>App: 입력 검증 및 selectedBootstrap 설정
User->>App: 분석 시작 (startAnalysisJob)
App->>Tauri: invoke start_analysis_job (request)
Tauri->>Engine: 작업 생성 요청
Engine-->>Tauri: jobId 반환
Tauri-->>App: jobId 전달
loop 폴링
App->>Tauri: getAnalysisJobStatus(jobId)
Tauri->>Engine: 상태 조회
Engine-->>Tauri: queued|running|succeeded|failed
Tauri-->>App: 상태 응답
App->>App: jobStatus/jobResult/jobError 업데이트
end
Engine-->>Tauri: 완료 결과 전송
Tauri-->>App: jobResult 전달
App->>User: 결과 렌더링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 분 Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared-types/src/index.ts (1)
1006-1023:⚠️ Potential issue | 🟠 Major
timeRange를 바로 필수화하면 기존 저장 프로젝트가 로드되지 않습니다.
validateRehearsalSection()가 마이그레이션 없이timeRange누락을 즉시 에러로 바꾸면서, 이전 버전에서 저장된sections[*]payload는 업그레이드 후parseRehearsalSong()에서 전부 거부됩니다. 이미loadProject()경로가 있는 만큼, 스키마 버전별 변환이나 레거시 fallback이 같이 없으면 기존 로컬 프로젝트를 여는 순간 깨집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared-types/src/index.ts` around lines 1006 - 1023, validateRehearsalSection currently treats a missing timeRange as an immediate error (via validateSectionTimeRange) which breaks loading older saved projects; modify validateRehearsalSection so it does not error when value.timeRange is undefined—either accept a missing timeRange (no error) or attempt a lightweight migration/fallback (e.g., call a migrateSectionTimeRange or fill a default timeRange) before returning an error; update callers like parseRehearsalSong/loadProject expectations accordingly so legacy sections[*] without timeRange are accepted and upgraded rather than rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-baseline.yml:
- Around line 321-356: The release step trusts artifacts/* and will publish any
stray files; update the "Validate release asset set" and the "Create draft
release with complete assets, then publish" steps so you build an explicit
allowlist of expected asset names (e.g. the platform arrays windows_amd64,
windows_arm64, macos_amd64, macos_arm64, checksum list, plus
bandscope-sbom.cdx.json and supply-chain/supplemental-component-inventory.json),
reject the job if any unexpected files exist under artifacts/ (fail when
extra/unrecognized files are present), and replace artifacts/* in the gh release
create invocation with the explicit allowlist variables (and pass
GH_TOKEN/RELEASE_TAG as before) so only whitelisted files are attached to the
release.
In @.github/workflows/ossf-scorecard.yml:
- Around line 25-29: The publish_results condition is mismatched with the
action's run condition: the step runs only when github.ref ==
format('refs/heads/{0}', github.event.repository.default_branch) but
publish_results is still checking github.ref == 'refs/heads/develop'; update the
publish_results expression to use the same branch check as the step (e.g.,
github.ref == format('refs/heads/{0}', github.event.repository.default_branch))
so publish_results toggles consistently with the if condition; adjust the
publish_results value near results_file/results_format to reference the same
github.event.repository.default_branch expression.
In `@apps/desktop/src/App.test.tsx`:
- Around line 9-38: The test mock in App.test.tsx replaces startAnalysisJob,
getAnalysisJobStatus, selectLocalAudioSource, and importYoutubeUrl entirely,
bypassing production validators like isAnalysisJobStatus,
parseProjectBootstrapSummary, and isSupportedYoutubeUrl; instead revert to using
the real implementations from ./lib/analysis and only stub the underlying bridge
call (window.__TAURI_INVOKE__ or the tauriInvoke wrapper) to return controlled
responses so the real validation/path logic still runs (keep
createDefaultAnalysisRequest, loadProject, saveProject overrides if needed, but
remove/restore the full-function overrides for
startAnalysisJob/getAnalysisJobStatus/selectLocalAudioSource/importYoutubeUrl
and replace them by stubbing tauriInvoke/window.__TAURI_INVOKE__ to return
sanitized responses that trigger desired branches).
In `@apps/desktop/src/App.tsx`:
- Around line 171-189: The catch block currently clears job tracking by calling
setJobStatus(null) when getAnalysisJobStatus(jobStatus.jobId) fails; instead
preserve the existing jobStatus on transient fetch errors and only set a
non-blocking error (e.g., setJobError) or schedule a retry so the UI continues
polling the same job; update the effect that uses getAnalysisJobStatus,
setJobStatus, setJobError and ANALYSIS_POLL_INTERVAL_MS to remove
setJobStatus(null) from the catch and replace it with setting an error message
(or a transientError flag) so the next timer iteration will attempt
getAnalysisJobStatus again for the current jobStatus.jobId.
In `@apps/desktop/src/components/ui/button.tsx`:
- Line 11: The default variant's hover rule targets child anchors instead of the
button itself, so update the default variant in the variant map used by the
Button component (the entry with key "default" and value "bg-primary
text-primary-foreground [a]:hover:bg-primary/80") to use the same hover selector
style as the other variants by replacing the `[a]:hover:bg-primary/80` fragment
with `hover:bg-primary/80` so the button background changes on hover.
In `@apps/desktop/src/components/ui/progress.tsx`:
- Around line 21-24: The component currently always renders the default
ProgressTrack and ProgressIndicator alongside any passed children, causing
duplicate UI; change the render logic in the Progress component so the default
Track/Indicator are only rendered when no children are provided (e.g., wrap the
default <ProgressTrack><ProgressIndicator/></ProgressTrack> in a conditional
that checks children or React.Children.count(children) === 0), leaving custom
children untouched when supplied.
In `@apps/desktop/src/components/ui/tabs.tsx`:
- Around line 14-18: The data-orientation attribute set on the Tabs root
(data-slot="tabs", data-orientation={orientation}) doesn't match the Tailwind
selectors used; update all Tailwind data-variant selectors in this component
(the className string for the Tabs root and any related classes for triggers,
content, indicator) to use Tailwind v4's arbitrary attribute syntax like
data-[orientation=horizontal]:... and data-[orientation=vertical]:... (and group
selectors as group-[data-orientation=horizontal]:... /
group-[data-orientation=vertical]:...) so the orientation-specific styles
actually match the rendered data-orientation value.
In `@apps/desktop/src/features/workspace/RoleSwitcher.tsx`:
- Around line 22-23: The component currently uses the literal string "all" as a
UI sentinel which can collide with a real role id; update RoleSwitcher to use a
private, collision-safe sentinel (e.g., a prefixed token like "__ALL__" or a
Symbol) for all internal radio/select values and convert to/from the public
representation (activeRole === null means “all”) in the event handlers.
Concretely, replace the literal "all" usage in the value and onValueChange
handlers (the occurrences that reference activeRole and call onRoleChange) so
the value prop maps activeRole null -> INTERNAL_ALL and onValueChange maps
INTERNAL_ALL -> null (and otherwise passes through real role ids), and apply the
same mapping wherever the component renders the “all” option and processes
incoming UI values.
- Around line 29-30: The Tab trigger classNames in RoleSwitcher.tsx use the
wrong attribute selector — replace any occurrences of "data-[state=active]:"
with the project-standard "data-active:" so the active styles defined in
tabs.tsx (TabsTrigger) apply; update the className strings in RoleSwitcher (both
the occurrences around the current lines and the similar one at lines ~37-38) to
use data-active: for the active background, text color, and shadow selectors.
In `@apps/desktop/src/features/workspace/SectionRoadmap.tsx`:
- Around line 117-140: Remove the redundant onKeyDown handler from the button in
SectionRoadmap.tsx: the button element already triggers onClick for Enter/Space,
so delete the onKeyDown prop (the block that checks onSongUpdate and e.key and
calls handleChordEdit(section.id, role)) and rely on the existing onClick={( )
=> handleChordEdit(section.id, role)} and disabled={!onSongUpdate} to handle
keyboard activation and focus behavior; leave aria-label, title, className, and
handleChordEdit references intact.
In `@apps/desktop/src/features/workspace/Workspace.tsx`:
- Around line 34-40: The grid template uses repeat(${sections.length}, ...)
which produces invalid CSS when sections.length is 0; in the Workspace component
update the gridTemplateColumns logic (the div with className "grid
min-w-[720px]") to use a fallback like repeat(${Math.max(1, sections.length)},
minmax(8rem, 1fr)) or conditionally render a dedicated empty-state column/markup
when sections.length === 0 so the timeline layout remains valid across browsers.
In `@CHANGELOG.md`:
- Around line 7-32: Several Markdown headings in the changelog (e.g., "###
Fixed", "## [0.1.2] - 2026-04-29", "### Changed", "### Added") are missing the
required blank line below them per MD022; fix this by inserting a single empty
line immediately after each heading line (for every section header such as the
top-level release headings and the "###" subsection headings) so each heading is
followed by one blank line and the content lines remain unchanged, ensuring the
file passes Markdown linting.
In `@docs/plans/2026-04-28-pr-159-rollout.md`:
- Around line 4-172: The markdown linter flags (MD022, MD058, MD047) come from
heading/table surrounding blank-line rules and a missing final newline in this
document (look for headings like "## Problem Statement" and "## Proposed
Solution", the decision tables, and the TODOs section); fix by ensuring a single
blank line before and after every ATX heading and before/after every markdown
table, remove any extraneous consecutive blank lines, and add a single trailing
newline at EOF; run markdownlint (or markdownlint-cli) with --fix or apply these
adjustments manually across the file to silence MD022/MD058 and add the final
newline to satisfy MD047.
- Around line 57-58: The Deployment vs Security criteria conflict needs
resolving: either make signing/notarization mandatory before passing the release
gate (update the "Deployment" cell to require automated signing/notarization and
adjust the acceptance test to verify Gatekeeper/SmartScreen-free execution) or
explicitly split acceptance criteria into "internal/draft" (DMG/EXE only) and
"public release" (signed/notarized + security checks) in the table; apply the
cross-platform build security rules referenced in the plan to CI,
release-validation, protected-branch checks, packaging, and artifact handling,
and mirror the same change for the related plan entries 158-164 so the document
consistently enforces the chosen rollout policy.
In `@packages/shared-types/test/index.test.ts`:
- Around line 680-690: Tests use hard-coded u32 boundary magic numbers
(4_294_967_296 / 4_294_967_297) in the createInvalidSong calls for
sections[0].timeRange.start/end; replace those literals with expressions derived
from the shared MAX_SECTION_TIME_SECONDS constant (e.g.,
MAX_SECTION_TIME_SECONDS + 1 and + 2 as needed) so the test asserts out-of-range
values relative to MAX_SECTION_TIME_SECONDS and avoids drift when the constant
changes; update the payloads passed to createInvalidSong and any assertions that
reference sections[0].timeRange.{start,end} accordingly.
In `@scripts/checks/verify_supply_chain.py`:
- Around line 159-163: The current guard checks for both "main" and
"ossf/scorecard-action" in the scorecard before validating the default-branch
token; remove the dependency on the literal "main" so the validation runs
whenever "ossf/scorecard-action" is present. Update the conditional that
references the scorecard (the block that currently checks if "main" in scorecard
and "ossf/scorecard-action" in scorecard) to only check for
"ossf/scorecard-action" in scorecard, and then keep the existing check that
appends the missing message if "github.event.repository.default_branch" is not
in scorecard. Ensure you modify the same scope where variables scorecard and
missing are used.
In `@scripts/release/package_desktop_artifact.py`:
- Around line 88-93: The current recursive rglob calls on bundle_dir collect
installers from any nested folder (variables: bundle_dir, installers) and can
pick up unintended files; restrict the search to the expected Tauri output
subdirectories instead of using bundle_dir.rglob(...). For example, only look
inside the dmg, nsis (or bundle/dmg, bundle/nsis, bundle/msi) subfolders by
checking each subdir exists and using non-recursive glob for the expected
extensions (e.g., search only *.dmg in the dmg subdir, *.exe in nsis, *.msi in
msi) and append results to installers; update the three rglob calls to these
targeted checks so unexpected .exe/.msi files outside the intended bundles are
not included.
- Around line 114-117: The current duplicate-handling branch using
archive_name.replace(ext, f"{ext}") does nothing; update the block that checks
if len(installers) > 1 to actually make archive_name unique per installer (use
the installer_path and ext variables). For example, inside the if
len(installers) > 1 branch, derive ext = installer_path.suffix and then append a
distinguishing token (such as ext or installer_path.stem or installer_path.name)
to archive_name so each installer produces a different archive_name (use the
archive_name, installers, installer_path and ext variables to construct the new
unique name).
In `@services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py`:
- Around line 44-46: The FileNotFoundError raised when validating audio_path is
currently inside the broad try/except that re-wraps any Exception into a
ValueError, losing the original error type; move the Path(audio_path).is_file()
check (and raise FileNotFoundError(path_str) if missing) outside the analysis
try block so the existence check is a single pre-condition and FileNotFoundError
is preserved, leaving the existing except Exception -> ValueError behavior to
only cover runtime analysis errors.
In `@services/analysis-engine/tests/test_chord_recognizer.py`:
- Line 40: The assertion currently checks only for the exact substring
"n_fft=1024" in the warnings list (caught), which is too narrow; update the
check in the test (the assertion using caught) to match a broader pattern such
as any mention of "n_fft" or a regex like r"n_fft\s*=" so that any related
warning (different values or spacing) is caught—replace the current list
comprehension/assert with a search using re.search or "any('n_fft' in
str(w.message) for w in caught)" (or similar) to ensure the test fails for any
n_fft-related warning.
In `@services/analysis-engine/tests/test_supply_chain_policy.py`:
- Around line 203-206: The test assertion hardcodes a line number (":6 ->")
making it flaky; update the assertion that currently checks the exact string
containing ".github/workflows/build-baseline.yml:6 -> workflow npx package
execution must use npm exec or npx --no-install: `@tauri-apps/cli`" to instead
assert the core message is present in violations (e.g., check that any item in
the violations iterable contains "workflow npx package execution must use npm
exec or npx --no-install: `@tauri-apps/cli`" or assert the substring is in
violations), using the existing violations variable to perform the
containment/any(...) check so the test no longer depends on a fixed line number.
In `@VERSION`:
- Line 1: The VERSION file currently contains "0.1.1" which is inconsistent with
the package.json and apps/desktop/src-tauri/tauri.conf.json values (0.1.3);
update the VERSION file to "0.1.3" so all release sources match, ensuring the
single source of truth for release automation is consistent across VERSION,
package.json, and apps/desktop/src-tauri/tauri.conf.json.
---
Outside diff comments:
In `@packages/shared-types/src/index.ts`:
- Around line 1006-1023: validateRehearsalSection currently treats a missing
timeRange as an immediate error (via validateSectionTimeRange) which breaks
loading older saved projects; modify validateRehearsalSection so it does not
error when value.timeRange is undefined—either accept a missing timeRange (no
error) or attempt a lightweight migration/fallback (e.g., call a
migrateSectionTimeRange or fill a default timeRange) before returning an error;
update callers like parseRehearsalSong/loadProject expectations accordingly so
legacy sections[*] without timeRange are accepted and upgraded rather than
rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1111eadf-6748-402f-85ed-8e13d3fbf593
⛔ Files ignored due to path filters (3)
apps/desktop/public/favicon.svgis excluded by!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.jsonservices/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (68)
.github/workflows/bandit.yml.github/workflows/build-baseline.yml.github/workflows/ossf-scorecard.yml.github/workflows/sbom.ymlCHANGELOG.mdVERSIONapps/desktop/components.jsonapps/desktop/index.htmlapps/desktop/package.jsonapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/src/main.rsapps/desktop/src-tauri/tauri.conf.jsonapps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/components/ui/badge.tsxapps/desktop/src/components/ui/button.tsxapps/desktop/src/components/ui/card.tsxapps/desktop/src/components/ui/input.tsxapps/desktop/src/components/ui/progress.tsxapps/desktop/src/components/ui/scroll-area.tsxapps/desktop/src/components/ui/separator.tsxapps/desktop/src/components/ui/tabs.tsxapps/desktop/src/features/workspace/ConfidenceBadge.tsxapps/desktop/src/features/workspace/RoleSwitcher.tsxapps/desktop/src/features/workspace/SectionRoadmap.tsxapps/desktop/src/features/workspace/Workspace.tsxapps/desktop/src/features/workspace/WorkspaceStates.tsxapps/desktop/src/index.cssapps/desktop/src/lib/analysis.test.tsapps/desktop/src/lib/analysis.tsapps/desktop/src/lib/utils.tsapps/desktop/src/locales/en/common.jsonapps/desktop/src/locales/ko/common.jsonapps/desktop/src/main.tsxapps/desktop/tsconfig.jsonapps/desktop/vite.config.tsdocs/architecture/overview.mddocs/operations/deploy-runbook.mddocs/plans/2026-04-28-pr-159-rollout.mddocs/release/release-policy.mddocs/security/github-required-checks.mddocs/security/sbom-policy.mdpackage.jsonpackages/shared-types/src/index.tspackages/shared-types/test/index.test.tsscripts/checks/security_gates.pyscripts/checks/verify_docs.pyscripts/checks/verify_github_bootstrap_policy.pyscripts/checks/verify_security_notes.pyscripts/checks/verify_supply_chain.pyscripts/fix-version-format.shscripts/release/package_desktop_artifact.pyservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/api.pyservices/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.pyservices/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyservices/analysis-engine/tests/test_api.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_chords.pyservices/analysis-engine/tests/test_cli.pyservices/analysis-engine/tests/test_priority.pyservices/analysis-engine/tests/test_release_metadata.pyservices/analysis-engine/tests/test_release_packaging.pyservices/analysis-engine/tests/test_roles_ml.pyservices/analysis-engine/tests/test_sections.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_temporal.pyservices/analysis-engine/tests/test_tuning.py
💤 Files with no reviewable changes (4)
- scripts/checks/verify_security_notes.py
- scripts/checks/verify_docs.py
- scripts/checks/verify_github_bootstrap_policy.py
- .github/workflows/sbom.yml
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/checks/verify_supply_chain.py`:
- Around line 31-37: The NPX_PACKAGE regex misses single/double-quoted package
arguments; replace the regex-only detection in verify_supply_chain.py with shell
tokenization using shlex.split() on the command string, then iterate tokens
after identifying the initial "npx" token to skip known options (e.g., -y,
--yes, --ignore-existing, --quiet, --no-install, --package/-p and their values)
and treat the first non-option token as the package spec (this naturally
preserves tokens wrapped in single or double quotes); update the logic that
previously used NPX_PACKAGE to use this tokenized approach and add unit tests
covering unquoted, backtick, single-quoted and double-quoted package forms
(e.g., npx `@org/pkg`, npx `@org/pkg`, npx '@org/pkg', npx "@org/pkg") to ensure
detection works.
In `@scripts/release/select_release_assets.py`:
- Around line 23-31: The allowlist regex in _artifact_pattern currently permits
any dash-suffix via the group (?:-[A-Za-z0-9._-]+)?, which allows unexpected
names like bandscope-windows-amd64-<sha>-debug.exe to be accepted; update the
_artifact_pattern function to remove or replace that open-ended optional suffix
with an explicit, closed set of allowed suffix tokens (or drop it entirely) and
ensure the installer_suffix and sidecar groups remain anchored after the SHA so
only known installer extensions (e.g., .exe|.msi|.dmg) and explicit sidecar
names (e.g., .sha256|.manifest.txt) are matched; adjust the regex in
_artifact_pattern accordingly so only sanctioned artifact filenames pass the
allowlist.
- Around line 42-45: The _ensure_file function currently allows symlinks because
it only checks path.is_file(); update _ensure_file (the function named
_ensure_file) to reject symlinks as well as non-files by checking
path.is_symlink() and raising a ValueError if the path is a symlink or not a
regular file (so only regular files are accepted for release assets/metadata
such as SBOMs and supplemental inventory).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9a4af7d0-6071-4c89-9f0f-9f577974b710
📒 Files selected for processing (4)
scripts/checks/verify_supply_chain.pyscripts/release/select_release_assets.pyservices/analysis-engine/tests/test_release_asset_selection.pyservices/analysis-engine/tests/test_supply_chain_policy.py
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src-tauri/Cargo.toml`:
- Around line 7-13: The dependencies currently set with empty feature lists
(tauri-build and tauri) do not disable their crate default features; if the
intent is to turn off defaults, update the dependency declarations for
tauri-build and tauri to include default-features = false (e.g., tauri-build = {
version = "2", default-features = false, features = [] } and similarly for
tauri) so default features are actually disabled while keeping the explicit
features list.
In `@apps/desktop/src/components/ui/scroll-area.tsx`:
- Around line 37-43: The Tailwind variant selectors using data-horizontal: and
data-vertical: in the className passed to ScrollAreaPrimitive.Scrollbar (which
sets data-orientation={orientation}) are incorrect; update those variants to the
bracketed attribute syntax data-[orientation=horizontal]: and
data-[orientation=vertical]: inside the cn(...) string so the
orientation-dependent size and border styles apply correctly for the
ScrollAreaPrimitive.Scrollbar component that uses data-orientation.
In `@apps/desktop/src/features/workspace/WorkspaceStates.tsx`:
- Around line 14-15: Hardcoded English titles ("Ready to Analyze" and "Analyzing
Audio") bypass i18n; replace those literal strings in WorkspaceStates.tsx with
calls to the existing translation function (t) using new keys (e.g.
t("readyToAnalyze") and t("analyzingAudio")) where those headings are rendered
(the <h3> elements around those titles, including the occurrences at the other
location noted), and add corresponding entries to your locale resource files so
non-English locales show translated titles; ensure you import/use the same
t/detectPreferredLocale pattern used elsewhere in this file/component.
In `@apps/desktop/src/lib/analysis.ts`:
- Around line 57-59: The legacy invoke shim should verify the injected value is
actually a function before returning it: change the branch that currently
returns window.__TAURI_INVOKE__ to instead check typeof window.__TAURI_INVOKE__
=== "function" and return it only in that case; otherwise return undefined (or a
safe noop) so callers don’t attempt to call a non-function value (reference the
window.__TAURI_INVOKE__ symbol in your change).
- Around line 82-90: The YouTube URL checks currently validate domain and path
but allow arbitrary strings as video IDs; update the validation to enforce a
standard YouTube video ID pattern (e.g. 11 characters using A-Z a-z 0-9 - _) by
adding a regex test against the ID in both branches: for the youtu.be branch
validate pathSegments[0] with the regex, and for the youtube.com branch validate
videoIds[0] (from parsedUrl.searchParams.getAll("v")) with the same regex before
returning true; use the existing parsedUrl, host, pathSegments, and videoIds
symbols so the logic is applied in the same locations.
In `@scripts/checks/verify_supply_chain.py`:
- Around line 409-422: The nested function record_step_violation currently
closes over loop variables (step_working_directory,
current_job_default_directory, workflow_default_directory,
step_uses_workspace_exec, path) which triggers Ruff B023; change
record_step_violation to accept these as explicit parameters (e.g.,
record_step_violation(step_working_directory, current_job_default_directory,
workflow_default_directory, step_uses_workspace_exec, path)) and update its
invocation to pass the current loop values instead of relying on closure capture
so the function no longer depends on external loop-scoped variables.
In `@services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py`:
- Around line 48-49: The global suppression of DeprecationWarning and
FutureWarning via warnings.simplefilter is too broad; restrict warning
suppression to the specific warnings produced by librosa/numba around the
librosa.load call instead, e.g., use warnings.filterwarnings with module and
message regex scoped immediately around the librosa.load invocation (or wrap the
call in a context manager that applies the narrower filter), add a brief comment
referencing the known librosa/numba warning being silenced, and remove or avoid
global simplefilter calls; look for the warnings.simplefilter lines and the
librosa.load usage to implement this targeted change.
In `@services/analysis-engine/tests/test_api.py`:
- Around line 218-225: Add test cases that pass Python bool values to
build_section_time_range to assert the implementation rejects booleans;
specifically extend the existing cases list in the test loop to include (True,
30) and (10, False) (or similar combinations) and verify they raise ValueError
whose message contains "timeRange" the same way other invalid inputs are
checked, referencing the build_section_time_range call within the test to ensure
the bool-path defensive branch is covered.
In `@services/analysis-engine/tests/test_cli.py`:
- Around line 221-223: The test currently removes the module with del
sys.modules["bandscope_analysis.cli"], which can leak state across tests;
replace that direct deletion with the pytest fixture call
monkeypatch.delitem(sys.modules, "bandscope_analysis.cli", raising=False) so the
module entry is removed for the test and automatically restored/isolated
afterward before calling runpy.run_module("bandscope_analysis.cli",
run_name="__main__"); update the test that references sys.modules and
runpy.run_module accordingly (use the existing monkeypatch fixture used
elsewhere in this test file).
In `@services/analysis-engine/tests/test_release_packaging.py`:
- Around line 23-29: The test asserts that
packaging.artifact_identity("installer.dmg") returns a Windows artifact but
still expects a macOS .dmg extension; update the test to use the correct Windows
installer extension (e.g., ".exe" or ".msi") so the expected keys in the
artifact dict match Windows outputs. Locate the assertion in
test_release_packaging.py around the call to packaging.artifact_identity and
change the input filename and expected "archive_name" / "manifest_name" values
to use the proper Windows extension (for example replace "installer.dmg" and any
".dmg" occurrences in expected values with "installer.exe" and corresponding
"bandscope-windows-arm64-... .exe" names).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3005b4a3-45a1-4c7b-951c-9370badf54e8
⛔ Files ignored due to path filters (3)
apps/desktop/public/favicon.svgis excluded by!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.jsonservices/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (74)
.github/workflows/bandit.yml.github/workflows/build-baseline.yml.github/workflows/ossf-scorecard.yml.github/workflows/sbom.ymlCHANGELOG.mdVERSIONapps/desktop/components.jsonapps/desktop/index.htmlapps/desktop/package.jsonapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/src/main.rsapps/desktop/src-tauri/tauri.conf.jsonapps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/components/ui/badge.tsxapps/desktop/src/components/ui/button.tsxapps/desktop/src/components/ui/card.tsxapps/desktop/src/components/ui/input.tsxapps/desktop/src/components/ui/progress.tsxapps/desktop/src/components/ui/scroll-area.tsxapps/desktop/src/components/ui/separator.tsxapps/desktop/src/components/ui/tabs.tsxapps/desktop/src/components/ui/ui-primitives.test.tsxapps/desktop/src/features/workspace/ConfidenceBadge.tsxapps/desktop/src/features/workspace/RoleSwitcher.test.tsxapps/desktop/src/features/workspace/RoleSwitcher.tsxapps/desktop/src/features/workspace/SectionRoadmap.test.tsxapps/desktop/src/features/workspace/SectionRoadmap.tsxapps/desktop/src/features/workspace/Workspace.test.tsxapps/desktop/src/features/workspace/Workspace.tsxapps/desktop/src/features/workspace/WorkspaceStates.tsxapps/desktop/src/index.cssapps/desktop/src/lib/analysis.test.tsapps/desktop/src/lib/analysis.tsapps/desktop/src/lib/utils.tsapps/desktop/src/locales/en/common.jsonapps/desktop/src/locales/ko/common.jsonapps/desktop/src/main.tsxapps/desktop/tsconfig.jsonapps/desktop/vite.config.tsdocs/architecture/overview.mddocs/operations/deploy-runbook.mddocs/plans/2026-04-28-pr-159-rollout.mddocs/release/release-policy.mddocs/security/github-required-checks.mddocs/security/sbom-policy.mdpackage.jsonpackages/shared-types/src/index.tspackages/shared-types/test/index.test.tsscripts/checks/security_gates.pyscripts/checks/verify_docs.pyscripts/checks/verify_github_bootstrap_policy.pyscripts/checks/verify_security_notes.pyscripts/checks/verify_supply_chain.pyscripts/fix-version-format.shscripts/release/package_desktop_artifact.pyscripts/release/select_release_assets.pyservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/api.pyservices/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.pyservices/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyservices/analysis-engine/tests/test_api.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_chords.pyservices/analysis-engine/tests/test_cli.pyservices/analysis-engine/tests/test_priority.pyservices/analysis-engine/tests/test_release_asset_selection.pyservices/analysis-engine/tests/test_release_metadata.pyservices/analysis-engine/tests/test_release_packaging.pyservices/analysis-engine/tests/test_roles_ml.pyservices/analysis-engine/tests/test_sections.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_temporal.pyservices/analysis-engine/tests/test_tuning.py
💤 Files with no reviewable changes (4)
- scripts/checks/verify_github_bootstrap_policy.py
- scripts/checks/verify_security_notes.py
- scripts/checks/verify_docs.py
- .github/workflows/sbom.yml
Owner
Author
|
Current head aeb856f addresses the latest CodeRabbit actionable findings, including matching frontend/Rust YouTube ID validation and locked Rust tests. Local harness and PR checks are green. @coderabbitai resolve |
|
Tip For best results, initiate chat on the files or code changes.
[resolve] |
Owner
Author
|
Current head aeb856f includes fixes for all prior actionable findings and the CodeRabbit comments have been resolved. Please run a current-head review. @coderabbitai full review |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src-tauri/src/main.rs (2)
919-923:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win오류 메시지에 원본 응답 JSON을 그대로 노출하지 마세요.
이 분기는 importer가 돌려준 전체 payload를 그대로 사용자 오류 문자열에 포함합니다.
filepath같은 로컬 경로나 예상치 못한 메타데이터가 그대로 새어 나갈 수 있으니, 여기서는 고정된 안전한 메시지만 반환하고 원본 payload가 필요하면 디버그 로그로만 남기는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/main.rs` around lines 919 - 923, The current else branch returns the entire parsed payload (parsed.to_string()) in the error string; change it to return a fixed, safe message (e.g., "YouTube import reported ok but missing metadata") instead of embedding parsed, and emit the original parsed payload only to a debug-level log (using the project's logging macro, e.g., log::debug! or tracing::debug!) for diagnostics; update the return in the branch that currently uses parsed.to_string() and add a debug log call referencing parsed so sensitive fields like filepath are not exposed to users.
885-899:⚠️ Potential issue | 🟠 MajorYouTube import 타임아웃이 하위 프로세스를 정리하지 못합니다.
여기서는
Command::output()를spawn_blocking안에 넣고tokio::time::timeout으로만 감싸고 있습니다. Tokio 문서상spawn_blocking작업은 abort되지 않으므로, 타임아웃 후에도 블로킹 클로저와 그 안의 하위 프로세스는 계속 실행될 수 있습니다. 이 경로는 반복 실패 시 다운로드 프로세스를 고아 상태로 남길 수 있으니,spawn()으로 child를 직접 잡고 만료 시kill()과wait()까지 보장해야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/main.rs` around lines 885 - 899, Current implementation uses tauri::async_runtime::spawn_blocking with Command::output inside a tokio::time::timeout, which doesn't abort the blocking task or its child process on timeout; change the logic in the YouTube import block (where YOUTUBE_IMPORT_TIMEOUT, spawn_blocking, and Command::output are used) to spawn the child process explicitly via Command::spawn to get a Child, run the child monitor on an async task (e.g., tokio::spawn) so you can await with timeout, and on timeout call child.kill() and then child.wait() (handling errors) to ensure the subprocess is terminated and reaped; propagate appropriate error strings instead of the current chained map_err messages.
♻️ Duplicate comments (2)
scripts/checks/verify_supply_chain.py (2)
523-526:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
npm exec --workspace검사가 줄바꿈된 셸 명령을 놓칩니다.
step_uses_workspace_exec를 단일 줄의"npm exec --workspace"부분 문자열로만 판정해서,npm exec \다음 줄에--workspace ...가 오는run: |블록은 그대로 우회됩니다. 같은 정책을 계속 강제하려면 step의run본문도logical_workflow_lines수준으로 정규화한 뒤 검사해야 합니다.Based on learnings: Apply cross-platform build security rules from
docs/security/cross-platform-build-policy.mdto CI, release validation, protected-branch checks, packaging, and artifact handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/checks/verify_supply_chain.py` around lines 523 - 526, The current detection for workspace exec only checks a single line substring and misses multiline run blocks; update the logic that sets step_uses_workspace_exec to normalize the step's run body into the same logical_workflow_lines form (reuse the existing logical_workflow_lines normalization) or join/normalize whitespace for the run scalar (the code that currently calls yaml_scalar_value and sets step_working_directory), then search the normalized string with a pattern that requires both "npm exec" and "--workspace" (e.g., regex allowing intervening whitespace/newlines) so multiline forms like "npm exec \\n --workspace ..." are caught; keep references to step_uses_workspace_exec, step_working_directory, yaml_scalar_value, and logical_workflow_lines to locate where to change.
31-34:⚠️ Potential issue | 🟠 Major
npx패키지 버전 스펙이 공급망 정책 검사를 우회합니다.현재
PACKAGE_SPEC정규식은@tauri-apps/cli@2.10.1,package@latest같은 버전 지정 spec을 거절합니다. 이 경우npx_package_from_command()는None을 반환하여 검사를 통과하게 되는데, npm 문서에 따르면npx는<pkg>[@<version>]형식을 표준으로 지원합니다. 따라서 버전이 명시된 원격 패키지 실행이 정책을 우회하는 결과가 됩니다.이를 해결하려면
PACKAGE_SPEC정규식 검증을 제거하고, 첫 번째 비옵션 토큰이 존재하면 정책 위반으로 처리해야 합니다. 정책의 의도는 모든npx <package>실행을 제어하는 것이므로, 버전 스펙 포함 여부와 관계없이 원격 패키지 실행을 감지해야 합니다.수정 예시
package = clean_package_token(current) - if PACKAGE_SPEC.fullmatch(package) is None: - return None - return None if no_install else package + if not package or package == "--": + return None + return None if no_install else package또한 92-137줄의 워크플로우 exec 정책 검사(525줄)도 줄바꿈 명령을 통한 우회를 막기 위해 검토가 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/checks/verify_supply_chain.py` around lines 31 - 34, The PACKAGE_SPEC regex allows versioned specs to bypass detection; remove the strict PACKAGE_SPEC validation and change npx_package_from_command() to treat any first non-option token (after NPX_FLAG_OPTIONS, NPX_PACKAGE_OPTIONS, NPX_VALUE_OPTIONS handling) as a remote package invocation and return a policy violation rather than None; update logic that currently filters tokens using PACKAGE_SPEC to accept anything as a package name when it's the first non-option token. Also review and harden the workflow exec policy check that parses command tokens (the exec-policy parsing block) to ensure newline/line-break splitting cannot be used to bypass npx detection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-baseline.yml:
- Around line 310-316: The "Upload release SBOM artifact" GitHub Action step
("Upload release SBOM artifact") is using actions/upload-artifact@bbbca2dd...
(v7.0.0) while other upload steps use v7.0.1; update this step to use the same
actions/upload-artifact version/commit hash used in the other upload steps (the
v7.0.1 commit) so all upload-artifact usages are consistent across the workflow.
In `@apps/desktop/src/App.test.tsx`:
- Around line 685-694: The tests that assert "no error shown" (e.g., the
"ignores cancellation when loading a project" case using mockLoadProject and the
saveProject variants using mockSaveProject) currently call
queryByText(...).toBeNull() immediately and can pass before the mocked rejection
is processed; change each test to first await the relevant mock being called or
the subsequent state transition (for example await waitFor(() =>
expect(mockLoadProject).toHaveBeenCalled()) or await waitFor(() =>
expect(screen.getByRole("button", { name: /open project/i })).toBeDisabled() /
other visible state change)), and only after that assert
expect(screen.queryByText(/Failed to load project/i)).toBeNull() (and
analogously for save tests using mockSaveProject) so the assertion runs after
the async rejection handling completes.
In `@apps/desktop/src/components/ui/tabs.tsx`:
- Around line 7-20: The Tabs component destructures orientation but only sets
data-orientation manually, so TabsPrimitive.Root still uses its default
orientation; update the Tabs component to pass orientation={orientation} into
TabsPrimitive.Root and remove the manual data-orientation attribute (keep any
needed data-slot/className logic), ensuring TabsPrimitive.Root receives the
actual orientation for correct layout and keyboard navigation; refer to the Tabs
function and TabsPrimitive.Root usage to locate the change.
In `@apps/desktop/src/features/workspace/RoleSwitcher.test.tsx`:
- Around line 20-24: Tests in RoleSwitcher.test.tsx are failing on non-English
environments because screen.getByRole queries use locale-dependent visible text
("All Roles", "Alloy Synth"); instead mock the i18n translation used by the
component (e.g., mock useTranslation or i18next.t used by RoleSwitcher) to
return deterministic English strings during the test, then update the test to
assert using those mocked strings (keep using fireEvent.click and
screen.getByRole but with the mocked labels) so onRoleChange expectations remain
stable across locales.
In `@apps/desktop/src/features/workspace/RoleSwitcher.tsx`:
- Around line 20-30: The function tabValueToRoleId currently returns unexpected
tab values directly; change it to validate against the known role set before
returning a role id: in tabValueToRoleId, if value === ALL_ROLES_VALUE return
null; if value.startsWith(ROLE_VALUE_PREFIX) extract id and check it exists in
the allowlist (e.g., a Set derived from the component's roles prop or
allowedRoles constant) and only then return the id; otherwise return null (or an
explicit sentinel) and avoid passing unvalidated values into onRoleChange —
update the caller (where onRoleChange is invoked) to handle the null/invalid
sentinel case and do not propagate invalid ids to state or routes, optionally
logging a warning for debugging.
In `@apps/desktop/src/features/workspace/SectionRoadmap.tsx`:
- Around line 94-96: The "Groove" and "Chord" labels in SectionRoadmap.tsx are
hardcoded and bypass i18n; update the JSX that renders the labels (the span
containing "Groove" and the label for "Chord") to use the existing i18n
translator (t) like other strings (e.g., t("sectionRoadmapTitle")), add/new
translation keys (e.g., "grooveLabel", "chordLabel") to your locale files, and
ensure the component uses the same useTranslation/t instance already used
elsewhere in the file so the labels localize with the rest of the UI.
- Around line 155-156: The "Cue" label in SectionRoadmap is hardcoded; replace
the literal "Cue" string with the same i18n translation call/pattern used for
the other labels ("Groove", "Chord") so the UI uses localization. Locate the
label rendering near role.cue.value in the SectionRoadmap component and swap the
hardcoded text for the corresponding t(...) lookup using the same key
namespace/format as the other labels (e.g., the existing keys used for
"Groove"/"Chord") so all labels are translated consistently.
In `@apps/desktop/src/features/workspace/Workspace.tsx`:
- Around line 128-176: The Workspace component contains multiple hardcoded
English UI strings ("Tonight's rehearsal map", "Song Timeline", "Stems",
"Rehearsal Priorities", etc.); replace them with i18n keys using the same
translator approach as SectionRoadmap: import/create the translator
(createTranslator) at the top of Workspace, instantiate t =
createTranslator(...), and replace each literal with t('namespace.key') calls
inside the JSX (e.g., heading, section titles, fallback texts like "Rehearsal
Workspace" and "first pass"); then add the corresponding keys into your locale
resource files and ensure fallback behavior is preserved.
In `@apps/desktop/vite.config.ts`:
- Line 4: The config currently relies on __dirname (and imports path) which
fails under ESM; update the alias resolution to compute the project root using
import.meta.url instead of __dirname—use fileURLToPath(import.meta.url) and
path.dirname(...) to derive the base and replace the alias target so Vite's
resolve.alias no longer references __dirname; ensure the path import remains and
that any occurrences of __dirname in the vite config (alias entries) are
replaced with the computed dirname from import.meta.url.
In `@services/analysis-engine/tests/test_api.py`:
- Around line 218-225: 현재 테스트 only checks end exceeding upper bound; add a case
where start exceeds the allowed max to catch regressions in
build_section_time_range. Update the test's cases list used in the loop
(variable cases) to include a tuple like (4_294_967_296, 30) (or another value >
2**32-1) so the loop will call build_section_time_range with an out-of-range
start and assert it raises a ValueError containing "timeRange".
In `@services/analysis-engine/tests/test_release_metadata.py`:
- Around line 9-11: The repo_root() helper currently assumes a fixed directory
depth via Path(__file__).resolve().parents[3], which is brittle; change
repo_root() to climb parent directories until it finds a repository marker
(e.g., pyproject.toml, setup.cfg, or .git) or a known top-level directory name
(e.g., "services") and return that path—replace the hardcoded parents[3] logic
in repo_root() with a loop that checks for these marker files/directories and
raises a clear error if none are found.
---
Outside diff comments:
In `@apps/desktop/src-tauri/src/main.rs`:
- Around line 919-923: The current else branch returns the entire parsed payload
(parsed.to_string()) in the error string; change it to return a fixed, safe
message (e.g., "YouTube import reported ok but missing metadata") instead of
embedding parsed, and emit the original parsed payload only to a debug-level log
(using the project's logging macro, e.g., log::debug! or tracing::debug!) for
diagnostics; update the return in the branch that currently uses
parsed.to_string() and add a debug log call referencing parsed so sensitive
fields like filepath are not exposed to users.
- Around line 885-899: Current implementation uses
tauri::async_runtime::spawn_blocking with Command::output inside a
tokio::time::timeout, which doesn't abort the blocking task or its child process
on timeout; change the logic in the YouTube import block (where
YOUTUBE_IMPORT_TIMEOUT, spawn_blocking, and Command::output are used) to spawn
the child process explicitly via Command::spawn to get a Child, run the child
monitor on an async task (e.g., tokio::spawn) so you can await with timeout, and
on timeout call child.kill() and then child.wait() (handling errors) to ensure
the subprocess is terminated and reaped; propagate appropriate error strings
instead of the current chained map_err messages.
---
Duplicate comments:
In `@scripts/checks/verify_supply_chain.py`:
- Around line 523-526: The current detection for workspace exec only checks a
single line substring and misses multiline run blocks; update the logic that
sets step_uses_workspace_exec to normalize the step's run body into the same
logical_workflow_lines form (reuse the existing logical_workflow_lines
normalization) or join/normalize whitespace for the run scalar (the code that
currently calls yaml_scalar_value and sets step_working_directory), then search
the normalized string with a pattern that requires both "npm exec" and
"--workspace" (e.g., regex allowing intervening whitespace/newlines) so
multiline forms like "npm exec \\n --workspace ..." are caught; keep references
to step_uses_workspace_exec, step_working_directory, yaml_scalar_value, and
logical_workflow_lines to locate where to change.
- Around line 31-34: The PACKAGE_SPEC regex allows versioned specs to bypass
detection; remove the strict PACKAGE_SPEC validation and change
npx_package_from_command() to treat any first non-option token (after
NPX_FLAG_OPTIONS, NPX_PACKAGE_OPTIONS, NPX_VALUE_OPTIONS handling) as a remote
package invocation and return a policy violation rather than None; update logic
that currently filters tokens using PACKAGE_SPEC to accept anything as a package
name when it's the first non-option token. Also review and harden the workflow
exec policy check that parses command tokens (the exec-policy parsing block) to
ensure newline/line-break splitting cannot be used to bypass npx detection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7d34ea99-5223-453e-9d2f-a1ca5440d159
⛔ Files ignored due to path filters (4)
apps/desktop/public/favicon.svgis excluded by!**/*.svgapps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.jsonservices/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (74)
.github/workflows/bandit.yml.github/workflows/build-baseline.yml.github/workflows/ossf-scorecard.yml.github/workflows/sbom.ymlCHANGELOG.mdVERSIONapps/desktop/components.jsonapps/desktop/index.htmlapps/desktop/package.jsonapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/src/main.rsapps/desktop/src-tauri/tauri.conf.jsonapps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/components/ui/badge.tsxapps/desktop/src/components/ui/button.tsxapps/desktop/src/components/ui/card.tsxapps/desktop/src/components/ui/input.tsxapps/desktop/src/components/ui/progress.tsxapps/desktop/src/components/ui/scroll-area.tsxapps/desktop/src/components/ui/separator.tsxapps/desktop/src/components/ui/tabs.tsxapps/desktop/src/components/ui/ui-primitives.test.tsxapps/desktop/src/features/workspace/ConfidenceBadge.tsxapps/desktop/src/features/workspace/RoleSwitcher.test.tsxapps/desktop/src/features/workspace/RoleSwitcher.tsxapps/desktop/src/features/workspace/SectionRoadmap.test.tsxapps/desktop/src/features/workspace/SectionRoadmap.tsxapps/desktop/src/features/workspace/Workspace.test.tsxapps/desktop/src/features/workspace/Workspace.tsxapps/desktop/src/features/workspace/WorkspaceStates.tsxapps/desktop/src/index.cssapps/desktop/src/lib/analysis.test.tsapps/desktop/src/lib/analysis.tsapps/desktop/src/lib/utils.tsapps/desktop/src/locales/en/common.jsonapps/desktop/src/locales/ko/common.jsonapps/desktop/src/main.tsxapps/desktop/tsconfig.jsonapps/desktop/vite.config.tsdocs/architecture/overview.mddocs/operations/deploy-runbook.mddocs/plans/2026-04-28-pr-159-rollout.mddocs/release/release-policy.mddocs/security/github-required-checks.mddocs/security/sbom-policy.mdpackage.jsonpackages/shared-types/src/index.tspackages/shared-types/test/index.test.tsscripts/checks/security_gates.pyscripts/checks/verify_docs.pyscripts/checks/verify_github_bootstrap_policy.pyscripts/checks/verify_security_notes.pyscripts/checks/verify_supply_chain.pyscripts/fix-version-format.shscripts/release/package_desktop_artifact.pyscripts/release/select_release_assets.pyservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/api.pyservices/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.pyservices/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyservices/analysis-engine/tests/test_api.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_chords.pyservices/analysis-engine/tests/test_cli.pyservices/analysis-engine/tests/test_priority.pyservices/analysis-engine/tests/test_release_asset_selection.pyservices/analysis-engine/tests/test_release_metadata.pyservices/analysis-engine/tests/test_release_packaging.pyservices/analysis-engine/tests/test_roles_ml.pyservices/analysis-engine/tests/test_sections.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_temporal.pyservices/analysis-engine/tests/test_tuning.py
💤 Files with no reviewable changes (4)
- scripts/checks/verify_docs.py
- scripts/checks/verify_security_notes.py
- scripts/checks/verify_github_bootstrap_policy.py
- .github/workflows/sbom.yml
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared-types/src/index.ts (1)
1088-1152:⚠️ Potential issue | 🟠 Major | ⚡ Quick win레거시
timeRange마이그레이션이 최상위 song 파서에만 적용됩니다.지금은
parseRehearsalSong()만migrateLegacySectionTimeRanges()를 거치고,AnalysisJobStatus.result/SongRehearsalPack.song/RehearsalWorkspace.songs[*].song경로는 여전히validateRehearsalSong()를 직접 타서 예전 저장 데이터가 그대로 로드 실패합니다. 이번 변경의 하위호환 의도가 맞다면 마이그레이션을validateRehearsalSong()쪽으로 끌어올리거나, 중첩 song 검증 경로들도 동일한 정규화를 먼저 거치게 해야 합니다.🔧 한 곳에서 정규화하는 방향 예시
function validateRehearsalSong(value: unknown): string | null { - if (!isRecord(value)) { + const normalized = migrateLegacySectionTimeRanges(value); + if (!isRecord(normalized)) { return invalidField("root"); } - const extraKey = unexpectedKey(value, ["id", "title", "sections", "exportSummary"], ""); + const extraKey = unexpectedKey(normalized, ["id", "title", "sections", "exportSummary"], ""); if (extraKey) { return extraKey; } - if (typeof value.id !== "string") { + if (typeof normalized.id !== "string") { return invalidField("id"); } - if (typeof value.title !== "string") { + if (typeof normalized.title !== "string") { return invalidField("title"); } - if (!isDenseArray(value.sections)) { + if (!isDenseArray(normalized.sections)) { return invalidField("sections"); } - for (const [index, section] of value.sections.entries()) { + for (const [index, section] of normalized.sections.entries()) { const sectionError = validateRehearsalSection(section, `sections[${index}]`); if (sectionError) { return sectionError; } } - - return validateExportSummary(value.exportSummary, "exportSummary"); + return validateExportSummary(normalized.exportSummary, "exportSummary"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared-types/src/index.ts` around lines 1088 - 1152, The migration of legacy section timeRange is only applied in parseRehearsalSong (via migrateLegacySectionTimeRanges) but not when other code paths call validateRehearsalSong (e.g., AnalysisJobStatus.result, SongRehearsalPack.song, RehearsalWorkspace.songs[*].song), causing older data to fail validation; update the validation flow by invoking migrateLegacySectionTimeRanges at the start of validateRehearsalSong (or ensure all callers run the migration before calling validateRehearsalSong) so the function validates the normalized object, keeping the rest of validateRehearsalSong logic and return semantics unchanged.
♻️ Duplicate comments (3)
apps/desktop/vite.config.ts (1)
8-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winESM 설정에서
__dirname사용은 설정 로드 실패를 유발할 수 있습니다.Line 10의 alias 계산은 ESM 컨텍스트에서
ReferenceError: __dirname is not defined를 유발할 수 있습니다.import.meta.url기반으로 바꾸는 게 안전합니다.권장 수정 패치
import react from "@vitejs/plugin-react"; import { defineConfig } from "vitest/config"; import tailwindcss from "@tailwindcss/vite"; +import { fileURLToPath } from "node:url"; import path from "path"; export default defineConfig({ plugins: [react(), tailwindcss()], resolve: { alias: { - "@": path.resolve(__dirname, "./src"), + "@": path.resolve(path.dirname(fileURLToPath(import.meta.url)), "./src"), }, },#!/bin/bash set -e echo "[1] ESM 설정 확인" rg -n '"type"\s*:\s*"module"' --glob 'package.json' echo "[2] vite.config.ts의 __dirname 사용 확인" rg -n '__dirname|fileURLToPath|import\.meta\.url' --glob 'apps/desktop/vite.config.ts'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/vite.config.ts` around lines 8 - 11, The Vite ESM config uses __dirname inside the resolve.alias calculation which causes ReferenceError in ESM; replace the __dirname usage in apps/desktop/vite.config.ts by deriving the directory from import.meta.url (via fileURLToPath and new URL) and use that value when building the alias path for "@" so that the resolve.alias entry uses a proper runtime-safe path (update any top-level imports or helper variables used by the resolve: { alias: { "@": ... } } block to obtain the directory from import.meta.url).CHANGELOG.md (1)
7-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win헤더 아래 빈 줄 누락으로 Markdown lint(MD022) 경고가 재발합니다.
각
###헤더 다음에 빈 줄 1줄을 넣어 lint 경고를 제거해 주세요.수정 제안
### Fixed + - Published release assets through a tag-driven draft release flow so immutable GitHub Releases include desktop installers, checksums, SBOM, and supplemental inventory before publication. - Added a supply-chain regression guard that rejects post-publication release asset uploads. @@ ### Changed + - Aligned the packaged desktop app version with the release package metadata. @@ ### Fixed + - Stabilized YouTube import fallback behavior in browser and desktop dev paths. - Guarded OSSF Scorecard execution so release-branch pushes skip unsupported non-default branch runs cleanly. @@ ### Added + - Implemented rehearsal workspace design (Issue `#107`) - Add capo and tuning detection heuristics (Issue `#103`) - Add bandit security scan workflow @@ ### Fixed + - Upgrade pytest to 9.0.3 to fix GHSA-6w46-j5rx-g56g - Resolve npm audit vulnerabilities - Fix ruff import sorting and formatting errors🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 7 - 27, The Markdown lint MD022 is triggered because there is no blank line after level-3 headers (e.g., the occurrences of "### Fixed", "### Changed", and "### Added" in CHANGELOG.md); update the file so each "### ..." header is followed by a single empty line before the next content paragraph or list item to satisfy the linter (ensure you modify all instances like the "### Fixed" blocks under the release sections and the "### Changed"/"### Added" headers accordingly)..github/workflows/build-baseline.yml (1)
310-316: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
upload-artifact액션 버전 불일치가 남아있습니다.이 단계에서는
actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f(v7.0.0)을 사용하지만, 다른 모든 업로드 단계(라인 91, 169, 224, 269)에서는 v7.0.1 (043fb46d...)을 사용합니다.♻️ 버전 통일 제안
- name: Upload release SBOM artifact - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: bandscope-release-sbom-${{ github.sha }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-baseline.yml around lines 310 - 316, The "Upload release SBOM artifact" workflow step uses actions/upload-artifact@bbbca2dd... (v7.0.0) which is inconsistent with the other upload steps; update the action reference in that step to the same v7.0.1 commit used elsewhere (actions/upload-artifact@043fb46d...) so all upload-artifact usages match, ensuring consistency across the workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src-tauri/src/main.rs`:
- Around line 973-1005: The function wait_for_process_output currently polls
child.try_wait() while leaving stdout/stderr as piped, which can deadlock if the
child fills OS pipe buffers; fix by spawning reader threads (or async tasks)
that continuously drain child.stdout and child.stderr as soon as the child is
spawned (before entering the try_wait() loop) and collect the output into
buffers so that when try_wait() returns Some(_) you can return the combined
Output without risking blocked writes; update references in the function (child
creation, .stdout/.stderr handling, and the try_wait() loop) to start and join
those reader threads and use their buffers to construct the Result instead of
blocking on wait_with_output().
In `@apps/desktop/src/components/ui/badge.tsx`:
- Around line 12-18: The badge variants default, secondary, destructive, and
outline currently use child-targeted selectors like "[a]:hover:..." so the
parent <span> badge doesn't get hover styles; update those variant strings in
the Badge component (symbols: default, secondary, destructive, outline in
apps/desktop/src/components/ui/badge.tsx) to use parent hover selectors (e.g.,
"hover:...") instead of "[a]:hover:..." so hover behavior matches ghost and link
variants (follow the same fix applied in button.tsx).
In `@apps/desktop/src/components/ui/progress.tsx`:
- Around line 15-29: The current hasCustomChildren uses Children.count which
treats boolean false/true as a child and hides the default UI; change it to
count only actually renderable children by replacing Children.count(children)
with React.Children.toArray(children).filter(c => c !== null && c !== undefined
&& c !== false && c !== true).length (or equivalent Boolean-safe filter) so
hasCustomChildren correctly reflects presence of real children and allows
ProgressTrack/ProgressIndicator to render when children are only conditional
booleans; update the hasCustomChildren definition in
apps/desktop/src/components/ui/progress.tsx (the block using Children.count,
ProgressPrimitive.Root, ProgressTrack, ProgressIndicator).
In `@apps/desktop/src/components/ui/ui-primitives.test.tsx`:
- Around line 49-67: The tests currently call the component functions directly
(Tabs(...) and ScrollBar(...)), which bypasses React rendering and will break if
hooks/contexts are introduced; change these tests to mount the components with a
real render() (e.g., from `@testing-library/react`) and assert against the
rendered DOM: render <Tabs orientation="vertical" /> and query the DOM to check
the orientation is applied, and render <ScrollBar orientation="horizontal" />
then assert the element has data-orientation="horizontal" and that its class
attribute contains the expected selector strings (e.g.,
"data-[orientation=horizontal]:h-2.5" and "data-[orientation=vertical]:h-full")
and does not contain the legacy "data-horizontal:..." strings; this ensures
tests use React’s render path and verify the actual DOM contract instead of
inspecting component props.
In `@apps/desktop/src/features/workspace/Workspace.test.tsx`:
- Around line 33-43: The test is locale-dependent and can fail on non-English
CI; before rendering the Workspace in the "falls back to safe timeline text for
malformed section times" test, call setNavigatorLanguage("en-US") to force the
navigator locale for this test so the English string "verse · 0:00–0:00" is
consistently produced; place the setNavigatorLanguage call at the start of the
test (near where createDemoRehearsalSong() is used) so the subsequent
render(<Workspace song={song} />) and screen.getByText(...) assertions run under
the en-US locale.
In `@apps/desktop/src/features/workspace/Workspace.tsx`:
- Around line 206-213: The Transcribe Bass button is currently enabled/disabled
by checking the string `activeRole` for "bass" (likely the role id), which is
incorrect; update the logic in the Button props (the disabled and title
expressions around the Transcribe Bass button) to use the actual role metadata
such as `activeRoleDetails?.name` or an explicit instrument/type field (e.g.,
`activeRoleDetails?.instrument` or `activeRoleDetails?.type`) instead of
`activeRole.toLowerCase()`, and adjust the title text accordingly to reflect the
selected role name when enabling/disabling the button.
In `@apps/desktop/src/index.css`:
- Around line 5-7: stylelint가 Tailwind v4의 새로운 at-rule들(`@custom-variant`, `@theme`,
`@apply`)을 scss/at-rule-no-unknown으로 오탐지해 빌드가 실패하므로, apps/desktop/src/index.css를
제외하거나 .stylelintrc.json의 scss/at-rule-no-unknown 규칙에 ignoreAtRules 옵션으로
"@custom-variant", "@theme", "@apply"를 추가하거나 대신 Tailwind v4를 지원하는 설정(e.g.
stylelint-config-tailwindcss)으로 전환해 문제를 해결하세요; 구체적으로는 scss/at-rule-no-unknown의
ignoreAtRules 배열에 해당 문자열들을 추가하거나 stylelint의 ignoreFiles에 index.css를 추가하거나 설정
패키지를 교체하는 방식으로 변경하세요.
In `@apps/desktop/src/lib/analysis.ts`:
- Around line 82-90: The host check currently allows any subdomain via
host.endsWith(".youtube.com"), which is too permissive; change the conditional
in the YouTube URL branch to an explicit allowlist (e.g., host === "youtube.com"
|| host === "www.youtube.com") instead of using endsWith, keeping the rest of
the logic (parsedUrl.pathname === "/watch", parsedUrl.searchParams.getAll("v"),
and YOUTUBE_VIDEO_ID_PATTERN) unchanged so only standard YouTube hosts pass
validation.
In `@docs/plans/2026-04-28-pr-159-rollout.md`:
- Around line 153-160: Update the "Trust Boundary" and "Mitigations" wording so
that "GitHub Releases" is described as a distribution channel, not a trusted
source: in the "Trust Boundary" section replace the current bullet "GitHub
Releases serves as the trusted source of artifacts" with a statement that
Releases is a distribution channel only and that artifacts must be treated as
untrusted until verified; in the "Mitigations" section ensure the
trusted-acceptance criteria (code signing, notarization, cryptographic hashes,
provenance verification) are listed as the mechanisms that establish trust and
explicitly state that verification must occur before treating artifacts from
GitHub Releases as trusted.
In `@scripts/release/package_desktop_artifact.py`:
- Around line 114-123: The loop currently uses shutil.copy2(installer_path, ...)
which follows symlinks and can copy external target files into artifacts; update
the logic in the packaging loop (the block using installers,
artifact_identity(), archive_safe_stem(), output_dir, and shutil.copy2) to first
validate installer_path is a regular file and not a symlink (e.g.,
installer_path.is_symlink() should be false and installer_path.is_file() true),
and if it fails validation log/raise or skip the entry rather than copying; this
ensures only regular files are packaged and prevents symlink targets from being
pulled into the release.
In `@scripts/release/select_release_assets.py`:
- Around line 142-146: The error message is currently printed to stdout which
can corrupt machine-readable asset output; change the except block that catches
ValueError around the call to select_release_assets in
scripts/release/select_release_assets.py to write the failure to stderr instead
(e.g., use print(..., file=sys.stderr) or logger.error) and ensure sys is
imported if using sys.stderr; keep the return 1 behavior unchanged so the
process still exits with failure.
---
Outside diff comments:
In `@packages/shared-types/src/index.ts`:
- Around line 1088-1152: The migration of legacy section timeRange is only
applied in parseRehearsalSong (via migrateLegacySectionTimeRanges) but not when
other code paths call validateRehearsalSong (e.g., AnalysisJobStatus.result,
SongRehearsalPack.song, RehearsalWorkspace.songs[*].song), causing older data to
fail validation; update the validation flow by invoking
migrateLegacySectionTimeRanges at the start of validateRehearsalSong (or ensure
all callers run the migration before calling validateRehearsalSong) so the
function validates the normalized object, keeping the rest of
validateRehearsalSong logic and return semantics unchanged.
---
Duplicate comments:
In @.github/workflows/build-baseline.yml:
- Around line 310-316: The "Upload release SBOM artifact" workflow step uses
actions/upload-artifact@bbbca2dd... (v7.0.0) which is inconsistent with the
other upload steps; update the action reference in that step to the same v7.0.1
commit used elsewhere (actions/upload-artifact@043fb46d...) so all
upload-artifact usages match, ensuring consistency across the workflow.
In `@apps/desktop/vite.config.ts`:
- Around line 8-11: The Vite ESM config uses __dirname inside the resolve.alias
calculation which causes ReferenceError in ESM; replace the __dirname usage in
apps/desktop/vite.config.ts by deriving the directory from import.meta.url (via
fileURLToPath and new URL) and use that value when building the alias path for
"@" so that the resolve.alias entry uses a proper runtime-safe path (update any
top-level imports or helper variables used by the resolve: { alias: { "@": ... }
} block to obtain the directory from import.meta.url).
In `@CHANGELOG.md`:
- Around line 7-27: The Markdown lint MD022 is triggered because there is no
blank line after level-3 headers (e.g., the occurrences of "### Fixed", "###
Changed", and "### Added" in CHANGELOG.md); update the file so each "### ..."
header is followed by a single empty line before the next content paragraph or
list item to satisfy the linter (ensure you modify all instances like the "###
Fixed" blocks under the release sections and the "### Changed"/"### Added"
headers accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0f7a1603-e83a-449d-813f-c04977cf9f37
⛔ Files ignored due to path filters (4)
apps/desktop/public/favicon.svgis excluded by!**/*.svgapps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.jsonservices/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (74)
.github/workflows/bandit.yml.github/workflows/build-baseline.yml.github/workflows/ossf-scorecard.yml.github/workflows/sbom.ymlCHANGELOG.mdVERSIONapps/desktop/components.jsonapps/desktop/index.htmlapps/desktop/package.jsonapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/src/main.rsapps/desktop/src-tauri/tauri.conf.jsonapps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/components/ui/badge.tsxapps/desktop/src/components/ui/button.tsxapps/desktop/src/components/ui/card.tsxapps/desktop/src/components/ui/input.tsxapps/desktop/src/components/ui/progress.tsxapps/desktop/src/components/ui/scroll-area.tsxapps/desktop/src/components/ui/separator.tsxapps/desktop/src/components/ui/tabs.tsxapps/desktop/src/components/ui/ui-primitives.test.tsxapps/desktop/src/features/workspace/ConfidenceBadge.tsxapps/desktop/src/features/workspace/RoleSwitcher.test.tsxapps/desktop/src/features/workspace/RoleSwitcher.tsxapps/desktop/src/features/workspace/SectionRoadmap.test.tsxapps/desktop/src/features/workspace/SectionRoadmap.tsxapps/desktop/src/features/workspace/Workspace.test.tsxapps/desktop/src/features/workspace/Workspace.tsxapps/desktop/src/features/workspace/WorkspaceStates.tsxapps/desktop/src/index.cssapps/desktop/src/lib/analysis.test.tsapps/desktop/src/lib/analysis.tsapps/desktop/src/lib/utils.tsapps/desktop/src/locales/en/common.jsonapps/desktop/src/locales/ko/common.jsonapps/desktop/src/main.tsxapps/desktop/tsconfig.jsonapps/desktop/vite.config.tsdocs/architecture/overview.mddocs/operations/deploy-runbook.mddocs/plans/2026-04-28-pr-159-rollout.mddocs/release/release-policy.mddocs/security/github-required-checks.mddocs/security/sbom-policy.mdpackage.jsonpackages/shared-types/src/index.tspackages/shared-types/test/index.test.tsscripts/checks/security_gates.pyscripts/checks/verify_docs.pyscripts/checks/verify_github_bootstrap_policy.pyscripts/checks/verify_security_notes.pyscripts/checks/verify_supply_chain.pyscripts/fix-version-format.shscripts/release/package_desktop_artifact.pyscripts/release/select_release_assets.pyservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/api.pyservices/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.pyservices/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyservices/analysis-engine/tests/test_api.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_chords.pyservices/analysis-engine/tests/test_cli.pyservices/analysis-engine/tests/test_priority.pyservices/analysis-engine/tests/test_release_asset_selection.pyservices/analysis-engine/tests/test_release_metadata.pyservices/analysis-engine/tests/test_release_packaging.pyservices/analysis-engine/tests/test_roles_ml.pyservices/analysis-engine/tests/test_sections.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_temporal.pyservices/analysis-engine/tests/test_tuning.py
💤 Files with no reviewable changes (4)
- scripts/checks/verify_docs.py
- scripts/checks/verify_github_bootstrap_policy.py
- scripts/checks/verify_security_notes.py
- .github/workflows/sbom.yml
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Owner
Author
|
@coderabbitai resolve |
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
apps/desktop/src/index.css (1)
6-9:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBiome 파서 오류가 남아 CI 린트 게이트를 막을 가능성이 있습니다.
Line 6, Line 9, Line 124/128/132의 Tailwind v4 at-rule은
stylelint-disable로는 해결되지만, 현재 힌트 기준으로 Biome 파서 오류가 계속 발생합니다. Biome가 필수 체크면 병합이 막힙니다.다음 스크립트로 Biome 게이트 영향 여부를 먼저 확인해 주세요.
#!/bin/bash set -e echo "[1] Biome 설정 파일 확인" fd -HI 'biome.json|biome.jsonc|biome.*.json' -x sh -c 'echo "---- $1"; cat "$1"' sh {} echo "[2] 린트 스크립트에서 Biome/CSS 대상 확인" rg -n '"lint"|biome|check|css' package.json apps/desktop/package.json echo "[3] 문제 파일 단독 검사" npm exec -- `@biomejs/biome` check apps/desktop/src/index.css || trueAlso applies to: 124-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/index.css` around lines 6 - 9, The Biome parser errors are caused by Tailwind v4 custom at-rules (e.g., the `@custom-variant` dark (&:is(.dark *)); and `@theme` inline { blocks) which Biome doesn't understand; move these Tailwind-specific at-rules out of apps/desktop/src/index.css into a separate file (e.g., tailwind-variants.css) and exclude that file from Biome checks by adding its path to your Biome ignore/exclude configuration (or .biomeignore), or alternatively add that file to the lint scripts' skip list; ensure you remove the moved at-rules from index.css and keep only Biome-parsable CSS there so the Biome gate no longer fails.apps/desktop/src/features/workspace/Workspace.test.tsx (1)
46-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win영문 라벨을 검사하는 테스트는 locale을 명시적으로 고정하세요.
Line 46 테스트는
"Bass Guitar","Transcribe Bass","Transcribe part"를 고정 문자열로 검사하지만navigator.language를 설정하지 않아 CI 기본 locale이 영어가 아니면 쉽게 깨집니다. 바로 앞의 타임라인 테스트처럼 시작 부분에서setNavigatorLanguage("en-US")를 호출해 이 케이스도 결정적으로 만드는 편이 안전합니다.수정 예시
it("enables bass transcription from selected role metadata rather than role id text", () => { + setNavigatorLanguage("en-US"); const song = createDemoRehearsalSong(); song.sections[0]!.roles[0] = { ...song.sections[0]!.roles[0]!, id: "low-end",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/features/workspace/Workspace.test.tsx` around lines 46 - 60, This test relies on English UI labels but doesn't fix the locale, so make it deterministic by calling setNavigatorLanguage("en-US") before rendering; update the test that uses createDemoRehearsalSong(), render(<Workspace song={song} />), fireEvent, and screen.getByRole(...) to invoke setNavigatorLanguage("en-US") at the start (same approach as the adjacent timeline test) so the "Bass Guitar", "Transcribe Bass", and "Transcribe part" strings are stable in CI.scripts/checks/verify_supply_chain.py (1)
552-558:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftallowlist 준비 검증이 실제 실행 경로에 묶여 있지 않습니다.
RELEASE_ASSET_VALIDATOR와RELEASE_ASSET_MAPFILE를 파일 전체 문자열 검색으로만 확인하면, 주석이나 무관한 step에 같은 리터럴이 있어도gh release createstep가 검증된 자산 목록을 만들지 않은 채 통과할 수 있습니다. 이 가드는 최소한 같은 실행 블록에서 validator 실행 →mapfile로드 →gh release create "${release_assets[@]}"사용까지 묶어서 확인해야 합니다.Based on learnings: Apply dependency, SBOM, and supply-chain rules from
docs/security/dependency-policy.mdto dependency additions, GitHub Actions, releases, bundled binaries, and model artifacts.Also applies to: 570-577
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/checks/verify_supply_chain.py` around lines 552 - 558, The current check only scans the whole file for RELEASE_ASSET_VALIDATOR and RELEASE_ASSET_MAPFILE which can match comments or unrelated steps; update the verifier in scripts/checks/verify_supply_chain.py to ensure RELEASE_ASSET_VALIDATOR, RELEASE_ASSET_MAPFILE and the release invocation string gh release create "${release_assets[@]}" occur together within the same run/job step block (i.e., parse the workflow step boundaries and require the sequence: validator run → mapfile load → gh release create using release_assets), and apply the same stricter logic for the other occurrence noted around the 570-577 region.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-baseline.yml:
- Around line 297-300: The download pattern used in the
actions/download-artifact step (pattern: bandscope-*-${{ github.sha }}) is too
broad and also matches SBOM artifacts, causing prior-run SBOMs to be downloaded
into artifacts/ on reruns; narrow the pattern to only match platform binaries
(e.g., use an explicit prefix like bandscope-release-bin-${{ github.sha }} or
similar) or change the SBOM upload name to a distinct prefix so that the step
with pattern: bandscope-*-${{ github.sha }} and path: artifacts only pulls the
intended platform artifacts and not SBOMs.
In `@apps/desktop/src-tauri/src/main.rs`:
- Around line 973-1043: The analysis runner can deadlock because
run_analysis_engine still spawns child processes with piped stdout/stderr and
polls with try_wait() then calls wait_with_output(), which can block if the
child's stdout/stderr buffer fills; reuse the existing wait_for_process_output()
helper instead of using try_wait()/wait_with_output() in run_analysis_engine
(and remove the piped-wait_with_output pattern). Locate run_analysis_engine and
replace its child spawn/wait logic to call wait_for_process_output(command,
timeout, poll_interval, timeout_message) so stdout/stderr are drained in
background threads the same way as the YouTube import path.
In `@apps/desktop/src/App.tsx`:
- Around line 240-242: When a new source/bootstrap is selected (the branch where
selection.ok is true and setSelectedBootstrap(selection.bootstrap) is called),
also reset the previous analysis state by clearing jobResult and jobStatus so
the UI no longer shows results from the prior workspace; update the same logic
paths where selection.ok is handled (including the similar block around the
other selection handling) to call the state reset functions (e.g.,
setJobResult(...) and setJobStatus(...)) immediately after setting the new
bootstrap.
- Around line 316-324: The renderWorkspaceState function currently prioritizes
jobError and returns <ErrorState> which hides an existing workspace when
save/load non-fatal errors occur; change render logic so that if jobResult
exists you render <Workspace song={jobResult} onSongUpdate={handleSongUpdate}/>
first, and only render a blocking <ErrorState> when there is no jobResult (or
when the error is fatal/unrecoverable). Keep analysisInFlight/isStarting
handling as-is, and ensure handleSaveProject/handleLoadProject surface
non-blocking errors via a banner/toast system instead of setting jobError to a
value that forces ErrorState to render; use a separate transient error state
(e.g., saveLoadError) or change jobError handling so it does not hide jobResult.
In `@apps/desktop/src/lib/analysis.ts`:
- Around line 147-165: The code currently uses a fixed projectId
"browser-youtube-project" for the import_youtube_url branch which causes
collisions; change the projectId to be unique per invocation (e.g., use
crypto.randomUUID(), Date.now(), or nanoid) and propagate that generated id into
projectRoot, cacheRoot, tempRoot and the source path passed to
createProjectBootstrapSummary so each browser fallback import creates an
isolated project; update the assignment of projectId in the import_youtube_url
block and ensure createProjectBootstrapSummary is called with the derived unique
projectRoot/cacheRoot/tempRoot and source.sourcePath.
In `@packages/shared-types/src/index.ts`:
- Around line 1110-1137: The migration currently fabricates a numeric timeRange
via legacySectionTimeRange(index) which creates synthetic seconds; instead
modify legacySectionTimeRange and migrateLegacySectionTimeRanges so they do NOT
emit a real {start,end} range — e.g. produce an explicit legacy marker (like
timeRange: { legacy: true, legacyIndex: index } or set timeRange: null and add
timeRangeSource: 'legacy-unknown') so callers can detect “unknown/legacy” and
trigger reanalysis/UI handling rather than treating it as real timecode; update
both legacySectionTimeRange and migrateLegacySectionTimeRanges (and the
analogous code at the other occurrence) to set that marker instead of numeric
ranges.
In `@services/analysis-engine/tests/test_release_metadata.py`:
- Around line 74-81: The test
test_rollout_plan_treats_github_releases_as_distribution_channel is brittle
because it hardcodes docs/plans/2026-04-28-pr-159-rollout.md; update it to
discover the rollout plan dynamically (e.g., use
repo_root().glob("docs/plans/*rollout*.md") or scan docs/plans for a file whose
text contains the rollout-related lines) and then assert the two strings against
the discovered file content; modify the test function and the use of repo_root()
accordingly so it fails only if no matching rollout plan exists or the policy
text is missing rather than because of a dated filename.
---
Duplicate comments:
In `@apps/desktop/src/features/workspace/Workspace.test.tsx`:
- Around line 46-60: This test relies on English UI labels but doesn't fix the
locale, so make it deterministic by calling setNavigatorLanguage("en-US") before
rendering; update the test that uses createDemoRehearsalSong(),
render(<Workspace song={song} />), fireEvent, and screen.getByRole(...) to
invoke setNavigatorLanguage("en-US") at the start (same approach as the adjacent
timeline test) so the "Bass Guitar", "Transcribe Bass", and "Transcribe part"
strings are stable in CI.
In `@apps/desktop/src/index.css`:
- Around line 6-9: The Biome parser errors are caused by Tailwind v4 custom
at-rules (e.g., the `@custom-variant` dark (&:is(.dark *)); and `@theme` inline {
blocks) which Biome doesn't understand; move these Tailwind-specific at-rules
out of apps/desktop/src/index.css into a separate file (e.g.,
tailwind-variants.css) and exclude that file from Biome checks by adding its
path to your Biome ignore/exclude configuration (or .biomeignore), or
alternatively add that file to the lint scripts' skip list; ensure you remove
the moved at-rules from index.css and keep only Biome-parsable CSS there so the
Biome gate no longer fails.
In `@scripts/checks/verify_supply_chain.py`:
- Around line 552-558: The current check only scans the whole file for
RELEASE_ASSET_VALIDATOR and RELEASE_ASSET_MAPFILE which can match comments or
unrelated steps; update the verifier in scripts/checks/verify_supply_chain.py to
ensure RELEASE_ASSET_VALIDATOR, RELEASE_ASSET_MAPFILE and the release invocation
string gh release create "${release_assets[@]}" occur together within the same
run/job step block (i.e., parse the workflow step boundaries and require the
sequence: validator run → mapfile load → gh release create using
release_assets), and apply the same stricter logic for the other occurrence
noted around the 570-577 region.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 191036d6-4b6c-427e-b95d-cf87a9856f09
⛔ Files ignored due to path filters (4)
apps/desktop/public/favicon.svgis excluded by!**/*.svgapps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.jsonservices/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (74)
.github/workflows/bandit.yml.github/workflows/build-baseline.yml.github/workflows/ossf-scorecard.yml.github/workflows/sbom.ymlCHANGELOG.mdVERSIONapps/desktop/components.jsonapps/desktop/index.htmlapps/desktop/package.jsonapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/src/main.rsapps/desktop/src-tauri/tauri.conf.jsonapps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/components/ui/badge.tsxapps/desktop/src/components/ui/button.tsxapps/desktop/src/components/ui/card.tsxapps/desktop/src/components/ui/input.tsxapps/desktop/src/components/ui/progress.tsxapps/desktop/src/components/ui/scroll-area.tsxapps/desktop/src/components/ui/separator.tsxapps/desktop/src/components/ui/tabs.tsxapps/desktop/src/components/ui/ui-primitives.test.tsxapps/desktop/src/features/workspace/ConfidenceBadge.tsxapps/desktop/src/features/workspace/RoleSwitcher.test.tsxapps/desktop/src/features/workspace/RoleSwitcher.tsxapps/desktop/src/features/workspace/SectionRoadmap.test.tsxapps/desktop/src/features/workspace/SectionRoadmap.tsxapps/desktop/src/features/workspace/Workspace.test.tsxapps/desktop/src/features/workspace/Workspace.tsxapps/desktop/src/features/workspace/WorkspaceStates.tsxapps/desktop/src/index.cssapps/desktop/src/lib/analysis.test.tsapps/desktop/src/lib/analysis.tsapps/desktop/src/lib/utils.tsapps/desktop/src/locales/en/common.jsonapps/desktop/src/locales/ko/common.jsonapps/desktop/src/main.tsxapps/desktop/tsconfig.jsonapps/desktop/vite.config.tsdocs/architecture/overview.mddocs/operations/deploy-runbook.mddocs/plans/2026-04-28-pr-159-rollout.mddocs/release/release-policy.mddocs/security/github-required-checks.mddocs/security/sbom-policy.mdpackage.jsonpackages/shared-types/src/index.tspackages/shared-types/test/index.test.tsscripts/checks/security_gates.pyscripts/checks/verify_docs.pyscripts/checks/verify_github_bootstrap_policy.pyscripts/checks/verify_security_notes.pyscripts/checks/verify_supply_chain.pyscripts/fix-version-format.shscripts/release/package_desktop_artifact.pyscripts/release/select_release_assets.pyservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/api.pyservices/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.pyservices/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyservices/analysis-engine/tests/test_api.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_chords.pyservices/analysis-engine/tests/test_cli.pyservices/analysis-engine/tests/test_priority.pyservices/analysis-engine/tests/test_release_asset_selection.pyservices/analysis-engine/tests/test_release_metadata.pyservices/analysis-engine/tests/test_release_packaging.pyservices/analysis-engine/tests/test_roles_ml.pyservices/analysis-engine/tests/test_sections.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_temporal.pyservices/analysis-engine/tests/test_tuning.py
💤 Files with no reviewable changes (4)
- scripts/checks/verify_github_bootstrap_policy.py
- scripts/checks/verify_security_notes.py
- .github/workflows/sbom.yml
- scripts/checks/verify_docs.py
✅ Actions performedComments resolved and changes approved. |
This was referenced Apr 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
develop, including the rehearsal workspace redesign, v0.1.3 release metadata, desktop packaging gates, and warning-clean analysis fixes.npxcalls with lockfile-backednpm exec, removingshadcnfrom runtime dependencies, and adding a regression guard for bare workflownpxpackage execution.Closes #178.
Closes #179.
Closes #180.
Closes #183.
Closes #184.
Related: #153, #154, #155, #177.
Verification
./scripts/harness/quickcheck.shnpm audit --workspaces --audit-level=high→found 0 vulnerabilitiespython3 scripts/checks/verify_supply_chain.py→Supply-chain verification passeduv run --project services/analysis-engine pytest services/analysis-engine/tests/test_supply_chain_policy.py -q→6 passednpm exec --workspace @bandscope/desktop -- tauri --version→tauri-cli 2.10.1[]Security Notes
Attack surface
Trust boundary
Mitigations
npm exec --workspace @bandscope/desktop -- tauri ...afternpm ci, backed by@tauri-apps/cliindevDependenciesandpackage-lock.json.scripts/checks/verify_supply_chain.pyrejects bare workflownpx <package>unless--no-installis used.shadcnwas removed from production dependencies and the stale CSS package import was removed.sbom.ymlno longer mutates immutablerelease: publishedassets; release assets remain tied to draft/tag publication flow.Test points
npxregression, including the lockfile-backed recurrence case.Dependency and Supply Chain
New direct dependency:
@tauri-apps/cliinapps/desktopdevDependenciesonly. It is needed so Windows/macOS native shell build jobs can execute Tauri from the lockfile afternpm ci. Alternative considered and rejected: barenpx @tauri-apps/cli, because it can fetch mutable package executables. Maintainer/source trust: official Tauri project package matching the existing Tauri desktop stack. License fit: Tauri packages are compatible with the existing open-source desktop stack. Known security issues:npm audit --workspaces --audit-level=highreports zero vulnerabilities. Transitive footprint: one CLI package plus platform-specific optional packages inpackage-lock.json; build-time only, not runtime. BandScope release risk: reduced, because native shell packaging now uses a reproducible lockfile-backed CLI path.i18n impact
Reviewer checklist
develop)