Skip to content

feat: add local analysis orchestration#49

Closed
seonghobae wants to merge 6 commits into
developfrom
feat/issue-32-analysis-orchestration-v4
Closed

feat: add local analysis orchestration#49
seonghobae wants to merge 6 commits into
developfrom
feat/issue-32-analysis-orchestration-v4

Conversation

@seonghobae
Copy link
Copy Markdown
Owner

Summary

  • add typed analysis job request and status contracts across shared types, the desktop shell, Tauri commands, and the Python engine CLI
  • expose a narrow allowlisted Tauri IPC boundary for starting and polling local analysis jobs without opening a loopback HTTP listener
  • harden the orchestration boundary with runtime engine discovery, strict schema validation, localized UI states, in-flight limiting, timeout handling, and pinned SBOM tooling

Verification

  • ./scripts/harness/quickcheck.sh
  • cargo check --manifest-path apps/desktop/src-tauri/Cargo.toml

@seonghobae
Copy link
Copy Markdown
Owner Author

@coderabbitai review

@seonghobae seonghobae enabled auto-merge (squash) March 12, 2026 04:07
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 12, 2026

📝 Walkthrough

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능

    • 분석 작업 워크플로우가 추가되었습니다. 사용자는 분석을 시작하고 진행 상황(대기, 실행, 완료, 실패)을 추적할 수 있습니다.
    • 향상된 오류 처리와 상세한 상태 메시지가 분석 프로세스 전체에 통합되었습니다.
  • 로컬라이제이션

    • 한국어 언어 지원이 추가되었습니다.

Walkthrough

분석 오케스트레이션 시스템을 구현합니다. Tauri IPC 명령(start_analysis_job, get_analysis_job_status)을 통해 데스크톱 프론트엔드를 Rust 백엔드에 연결하고, Python 서브프로세스를 실행하여 작업을 처리합니다. 공유 타입, CLI 구현, UI 통합 포함.

Changes

Cohort / File(s) Summary
CI/CD 워크플로우
.github/workflows/sbom.yml
Syft 바이너리 설치 단계 추가 및 CycloneDX SBOM 생성 액션을 직접 Syft CLI 호출로 교체
아키텍처 문서
ARCHITECTURE.md, docs/architecture/overview.md, docs/security/app-security.md
로컬 분석 오케스트레이션이 Tauri IPC 명령과 Python 서브프로세스(stdin/stdout)를 사용하며 HTTP 리스너를 사용하지 않음을 명시
구현 계획
docs/plans/2026-03-12-issue-32-analysis-orchestration-design.md, docs/plans/2026-03-12-issue-*.md
분석 오케스트레이션 설계 및 5단계 구현 계획 추가, 보안 고려사항 및 테스트 전략 포함
Tauri 설정 및 권한
apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/build.rs, apps/desktop/src-tauri/tauri.conf.json, apps/desktop/src-tauri/capabilities/main.json, apps/desktop/src-tauri/permissions/autogenerated/*
serde, serde_json, time 의존성 추가; 타우리 빌드 설정 업데이트; start_analysis_jobget_analysis_job_status 명령 권한 및 capabilities 구성
Tauri 백엔드 구현
apps/desktop/src-tauri/src/main.rs
AppState, 작업 큐, 동시성 제어를 포함한 완전한 분석 작업 시스템 구현 (548줄 추가)
데스크톱 프론트엔드
apps/desktop/package.json, apps/desktop/src/App.tsx, apps/desktop/src/App.test.tsx, apps/desktop/src/lib/analysis.ts, apps/desktop/src/i18n/index.ts
@tauri-apps/api 의존성 추가; 비동기 분석 워크플로우 구현; 작업 상태 폴링; Tauri/브라우저 폴백 포함 분석 브리지 추가; 포괄적인 테스트 커버리지
로컬라이제이션
apps/desktop/src/locales/en/common.json, apps/desktop/src/locales/ko/common.json
분석 워크플로우 UI 텍스트 추가: 시작, 상태(대기 중/실행 중/완료/실패) 및 오류 메시지
공유 타입
packages/shared-types/src/index.ts, packages/shared-types/test/index.test.ts
AnalysisJob 도메인 타입 추가 (요청, 상태, 오류, 스냅샷); 검증 함수 및 헬퍼 추가; RehearsalSong 검증 강화 (264줄 추가)
분석 엔진 API
services/analysis-engine/src/bandscope_analysis/api.py
TypedDict 페이로드 정의 및 공개 API 함수 추가: validate_analysis_job_request(), build_demo_rehearsal_song(), run_analysis_job() (279줄)
분석 엔진 CLI
services/analysis-engine/src/bandscope_analysis/cli.py, services/analysis-engine/tests/test_cli.py, services/analysis-engine/tests/test_api.py
stdin 기반 JSON 입력을 처리하고 stdout으로 상태를 반환하는 CLI 엔트포인트 구현; 포괄적인 테스트 추가

Sequence Diagram

sequenceDiagram
    participant Frontend as 프론트엔드
    participant Tauri as Tauri 명령
    participant RustOrch as Rust 오케스트레이션
    participant PythonCLI as Python CLI
    participant JobStore as 작업 저장소

    Frontend->>Tauri: start_analysis_job(요청)
    Tauri->>RustOrch: 요청 검증
    RustOrch->>JobStore: 작업 ID 할당, 상태=대기 중
    RustOrch->>PythonCLI: JSON stdin으로 서브프로세스 시작
    PythonCLI->>PythonCLI: 요청 검증, 분석 실행
    PythonCLI-->>RustOrch: JSON 결과 stdout
    RustOrch->>JobStore: 상태 업데이트 (완료/실패)
    Tauri-->>Frontend: AnalysisJobStatus 반환 (작업 ID)
    
    Frontend->>Tauri: get_analysis_job_status(작업ID) [폴링 반복]
    Tauri->>RustOrch: 작업 ID 조회
    RustOrch->>JobStore: 저장된 상태 검색
    Tauri-->>Frontend: AnalysisJobStatus (현재 상태)
    
    Frontend->>Frontend: 상태 렌더링 (대기/실행/완료/실패)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • PR #5: .github/workflows/sbom.yml 워크플로우 수정 — CycloneDX 액션을 Syft CLI로 교체
  • PR #37: packages/shared-types/src/index.ts 확장 — RehearsalSong 파싱/검증기와 분석 계약 타입 추가
  • PR #1: .github/workflows/sbom.yml SBOM 생성 단계 수정 — anchore 액션을 Syft CLI로 교체

Poem

🐰 분석의 길을 다지며,
IPC와 Python이 손을 맞잡고,
작업은 차곡차곡 쌓이고,
상태는 신속히 날아오르네.
로컬 우선, 안전하게,
새로운 오케스트라가 울린다! 🎵

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.82% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목은 'feat: add local analysis orchestration'으로 주요 변경사항인 로컬 분석 오케스트레이션 추가를 명확하고 간결하게 요약하고 있습니다.
Description check ✅ Passed PR 설명은 공유 타입, Tauri IPC 경계, 스키마 검증, 로컬라이제이션 등 실제 변경사항과 관련된 구체적인 내용을 포함하고 있습니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/issue-32-analysis-orchestration-v4

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Comment @coderabbitai help to get the list of available commands and usage tips.

@seonghobae
Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 12, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

1 similar comment
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 12, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 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/sbom.yml:
- Around line 41-44: The "Install Syft" workflow step currently downloads
syft_1.20.0_linux_amd64.tar.gz without verifying integrity; update the step to
also download syft_1.20.0_checksums.txt (from the same GitHub release), compute
the SHA-256 of "$RUNNER_TEMP/syft.tar.gz" (e.g., via sha256sum) and compare it
against the expected value from the checksums file, and exit non‑zero / fail the
job if the checksum does not match; ensure references in the step to
"$RUNNER_TEMP/syft.tar.gz" and the downloaded checksums file are used for the
verification, and consider updating the version string (syft_1.20.0_... and
syft_1.20.0_checksums.txt) to a newer release when changing the URLs.

In `@apps/desktop/src-tauri/src/main.rs`:
- Around line 458-469: Validate parsed AnalysisJobStatus before persisting:
after serde_json::from_slice::<AnalysisJobStatus>(&output.stdout) (and before
calling store_status or using status.job_id), check that status.job_id matches
payload["jobId"] (fall back to failed_status if mismatch), ensure terminal
states (e.g., "succeeded" or "failed") have the expected fields (succeeded must
include result, failed must include error) and that non-terminal states aren't
treated as terminal; if any invariant fails, map to failed_status(...) with
AnalysisJobErrorCode::EngineUnavailable and a clear message rather than
accepting the payload. Update the code paths around failed_status,
AnalysisJobStatus, and store_status to perform these checks and only call
store_status when invariants hold.
- Around line 380-393: The failure path after stdin.write_all currently only
calls process.kill() and returns, leaving the child unreaped; after killing the
child you must also wait on it to reap the process before returning. Update the
branch where stdin.write_all(...) fails (referencing stdin.write_all,
process.kill, failed_status and AnalysisJobErrorCode::EngineUnavailable) to call
process.wait() (or an appropriate wait method) after kill(), ignore/log any wait
errors, then return the failed_status as before so the child is properly reaped.

In `@apps/desktop/src/App.tsx`:
- Around line 100-123: The polling useEffect currently clears jobStatus on any
fetch error which discards jobId and stops polling; instead preserve jobId and
only set an error state so polling can continue/retry. In the catch block of the
useEffect that calls getAnalysisJobStatus, remove setJobStatus(null) and call
setJobError(t("analysisCouldNotStart")) (or similar) without wiping jobStatus;
also consider changing the effect dependency from jobStatus to jobStatus?.jobId
(or include jobStatus.jobId) so the poll loop is keyed to the jobId and not
reset when transient state changes. Ensure setJobResult and setJobError handling
stays the same when nextStatus arrives.

In `@apps/desktop/src/lib/analysis.ts`:
- Around line 22-28: The current getInvoke() returns the imported invoke when
window exists which can call into missing Tauri internals; change its runtime
detection to use isTauri() from "@tauri-apps/api/core" instead of checking
window presence. Update getInvoke() to import and call isTauri() and only return
window.__TAURI_INVOKE__ ?? invoke when isTauri() is true; otherwise return null,
ensuring you reference getInvoke, isTauri, invoke, and window.__TAURI_INVOKE__
when making the change.

In `@docs/architecture/overview.md`:
- Line 38: 문서의 "route orchestration through typed Tauri IPC and a narrow Python
subprocess bridge" 설명에 대해 별도의 "Security Notes" 섹션을 추가하세요; 이 섹션에 IPC와 Python
subprocess의 trust boundary를 명확히 정의하고(예: Tauri IPC, Python subprocess bridge,
loopback HTTP), 각 경계별 공격 표면·실제 위협 시나리오·권한 요구사항(권한 격리·권한 하향 최소화)·권한 검증 및 입력
검증/escaping 같은 완화책·잔여 위험·테스트 포인트(예: 권한 상승, 타임아웃/리소스 고갈, stderr/stdout 정보 유출, 악성
파일 실행) 등을 간단 항목화하여 포함시키세요; 해당 섹션은 문서 내 "Security Notes" 제목으로 추가하고 참조할 수 있게 route
orchestration 문단에 링크나 레퍼런스를 남기세요.

In `@packages/shared-types/src/index.ts`:
- Around line 373-389: The validateAnalysisJobError (and similarly
validateAnalysisJobStatus) uses invalidField which returns the generic "Invalid
rehearsal song contract" message; change this so analysis validators produce an
analysis-specific message by either adding an optional contractName parameter to
invalidField (defaulting to existing text) and passing "analysis job contract"
from validateAnalysisJobError/validateAnalysisJobStatus, or introduce a new
helper like invalidAnalysisField and use it in these validators; update all
calls in validateAnalysisJobError/validateAnalysisJobStatus to call the
new/parameterized helper so error messages are specific to analysis payloads
rather than the rehearsal contract.

In `@services/analysis-engine/src/bandscope_analysis/cli.py`:
- Around line 12-24: Wrap the top-level body of run_analysis_job with a broad
try/except that catches all unexpected exceptions, logs the error (but does not
let it crash the process), and returns a typed failure envelope using the
existing failed_cli_response pattern but with an "error.code" like
"engine_unavailable"; ensure the returned object preserves the original jobId
and requestedAt values from the incoming payload (use the same jobId/requestedAt
variables used in run_analysis_job) and sets state to "failed" and updatedAt to
the preserved timestamp so the CLI always emits a safe, contract-compliant
failure instead of crashing.
🪄 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: f1353d83-66a5-4ae7-a09b-294a1b96a280

📥 Commits

Reviewing files that changed from the base of the PR and between c28e363 and f708c01.

⛔ Files ignored due to path filters (6)
  • apps/desktop/src-tauri/Cargo.lock is excluded by !**/*.lock
  • apps/desktop/src-tauri/gen/schemas/acl-manifests.json is excluded by !**/gen/**
  • apps/desktop/src-tauri/gen/schemas/capabilities.json is excluded by !**/gen/**
  • apps/desktop/src-tauri/gen/schemas/desktop-schema.json is excluded by !**/gen/**
  • apps/desktop/src-tauri/gen/schemas/macOS-schema.json is excluded by !**/gen/**
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (26)
  • .github/workflows/sbom.yml
  • ARCHITECTURE.md
  • apps/desktop/package.json
  • apps/desktop/src-tauri/Cargo.toml
  • apps/desktop/src-tauri/build.rs
  • apps/desktop/src-tauri/capabilities/main.json
  • apps/desktop/src-tauri/permissions/autogenerated/get_analysis_job_status.toml
  • apps/desktop/src-tauri/permissions/autogenerated/start_analysis_job.toml
  • apps/desktop/src-tauri/src/main.rs
  • apps/desktop/src-tauri/tauri.conf.json
  • apps/desktop/src/App.test.tsx
  • apps/desktop/src/App.tsx
  • apps/desktop/src/i18n/index.ts
  • apps/desktop/src/lib/analysis.ts
  • apps/desktop/src/locales/en/common.json
  • apps/desktop/src/locales/ko/common.json
  • docs/architecture/overview.md
  • docs/plans/2026-03-12-issue-32-analysis-orchestration-design.md
  • docs/plans/2026-03-12-issue-32-analysis-orchestration.md
  • docs/security/app-security.md
  • packages/shared-types/src/index.ts
  • packages/shared-types/test/index.test.ts
  • services/analysis-engine/src/bandscope_analysis/api.py
  • services/analysis-engine/src/bandscope_analysis/cli.py
  • services/analysis-engine/tests/test_api.py
  • services/analysis-engine/tests/test_cli.py

Comment on lines +41 to +44
- name: Install Syft
run: |
curl -sSfL -o "$RUNNER_TEMP/syft.tar.gz" "https://github.com/anchore/syft/releases/download/v1.20.0/syft_1.20.0_linux_amd64.tar.gz"
tar -xzf "$RUNNER_TEMP/syft.tar.gz" -C "$RUNNER_TEMP" syft
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Syft v1.20.0 release checksum sha256

💡 Result:

Anchore Syft v1.20.0 publishes per-artifact SHA256 sums in the GitHub release asset syft_1.20.0_checksums.txt on the v1.20.0 release page. [1]

To get the SHA256 for the specific binary you downloaded (linux/windows/darwin + arch), download that checksums file and match the filename:

curl -sSfL -O https://github.com/anchore/syft/releases/download/v1.20.0/syft_1.20.0_checksums.txt
grep 'syft_1.20.0_' syft_1.20.0_checksums.txt
# then verify, e.g.:
sha256sum -c syft_1.20.0_checksums.txt --ignore-missing

(Anchore’s install docs also point to downloading artifacts from the GitHub releases page.) [2]

If you tell me which exact asset filename you need (e.g., syft_1.20.0_linux_amd64.tar.gz vs syft_1.20.0_windows_amd64.zip), I can help you extract the right line format to look for.

References: [1] (github.com) [2] (oss.anchore.com)

Citations:


🌐 Web query:

Syft latest version 2024 2025 current release

💡 Result:

  • Current/latest Syft release (as of Mar 12, 2026): v1.42.2 (released Mar 9, 2026) [1]
  • Latest release in 2025: v1.39.0 (released Dec 22, 2025) [2][3]
  • Latest release in 2024: v1.18.0 (released Dec 9, 2024) [4]

Sources: [1] anchore/syft GitHub repo (latest release badge), [2] anchore/syft GitHub release v1.39.0 page, [3] Chocolatey bot test log showing syft 1.39.0 on 2025-12-23, [4] Anchore Community announcement for v1.18.0.


🌐 Web query:

Syft v1.20.0 security vulnerability CVE

💡 Result:

Syft v1.20.0 does not appear to have any Syft-specific CVEs affecting that release line; the only CVE currently listed in the anchore/syft GitHub Security Advisories is:

  • CVE-2023-24827 (GHSA-jp7v-3587-2956): credential/password disclosure when SYFT_ATTEST_PASSWORD is set; affected: >= v0.69.0 and < v0.70.0; patched in: v0.70.0—so Syft v1.20.0 is not in the affected range. [1][2][3]

Sources: [1] GitHub advisory, [2] GitHub security overview, [3] NVD entry.


🏁 Script executed:

# Let me also check if there's any way to retrieve the actual checksum from the GitHub release
# by constructing the URL to the checksums file
curl -s "https://github.com/anchore/syft/releases/download/v1.20.0/syft_1.20.0_checksums.txt" | head -20

Repository: seonghobae/bandscope

Length of output: 2064


보안 위험: 다운로드된 바이너리에 대한 체크섬 검증 누락

외부 바이너리를 직접 다운로드하여 실행하는 경우, 반드시 SHA-256 체크섬을 검증해야 합니다. 현재 구현은 다운로드가 변조되었을 경우 악성 코드가 CI에서 실행될 수 있는 공급망 공격에 취약합니다.

Syft는 GitHub 릴리스 페이지에서 syft_1.20.0_checksums.txt 파일로 각 아티팩트의 SHA256 체크섬을 제공합니다. 다음과 같이 검증을 추가하세요:

🛡️ 체크섬 검증 추가
       - name: Install Syft
         run: |
           curl -sSfL -o "$RUNNER_TEMP/syft.tar.gz" "https://github.com/anchore/syft/releases/download/v1.20.0/syft_1.20.0_linux_amd64.tar.gz"
+          curl -sSfL -o "$RUNNER_TEMP/syft_checksums.txt" "https://github.com/anchore/syft/releases/download/v1.20.0/syft_1.20.0_checksums.txt"
+          grep "syft_1.20.0_linux_amd64.tar.gz" "$RUNNER_TEMP/syft_checksums.txt" | sha256sum -c -
           tar -xzf "$RUNNER_TEMP/syft.tar.gz" -C "$RUNNER_TEMP" syft

추가로, Syft v1.20.0은 2024년 버전으로 현재(2026년 3월) 최신 버전 v1.42.2에서 크게 뒤떨어져 있습니다. 최신 버전으로의 업그레이드도 검토하세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/sbom.yml around lines 41 - 44, The "Install Syft" workflow
step currently downloads syft_1.20.0_linux_amd64.tar.gz without verifying
integrity; update the step to also download syft_1.20.0_checksums.txt (from the
same GitHub release), compute the SHA-256 of "$RUNNER_TEMP/syft.tar.gz" (e.g.,
via sha256sum) and compare it against the expected value from the checksums
file, and exit non‑zero / fail the job if the checksum does not match; ensure
references in the step to "$RUNNER_TEMP/syft.tar.gz" and the downloaded
checksums file are used for the verification, and consider updating the version
string (syft_1.20.0_... and syft_1.20.0_checksums.txt) to a newer release when
changing the URLs.

Comment on lines +380 to +393
if let Some(mut stdin) = process.stdin.take() {
if stdin.write_all(payload.to_string().as_bytes()).is_err() {
let _ = process.kill();
return failed_status(
payload["jobId"]
.as_str()
.unwrap_or("unknown-job")
.to_string(),
requested_at,
AnalysisJobErrorCode::EngineUnavailable,
"Analysis engine is unavailable.",
);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

stdin.write_all 실패 경로에서 자식 프로세스를 reap 하지 않습니다.

Line 382에서 kill()만 호출하고 바로 반환하면, 아직 살아 있는 자식이 wait() 없이 drop되어 unreaped process로 남을 수 있습니다. 다른 실패 분기처럼 종료를 회수한 뒤 반환해야 합니다.

🔧 제안 수정
     if let Some(mut stdin) = process.stdin.take() {
         if stdin.write_all(payload.to_string().as_bytes()).is_err() {
             let _ = process.kill();
+            let _ = process.wait();
             return failed_status(
                 payload["jobId"]
                     .as_str()
                     .unwrap_or("unknown-job")
                     .to_string(),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if let Some(mut stdin) = process.stdin.take() {
if stdin.write_all(payload.to_string().as_bytes()).is_err() {
let _ = process.kill();
return failed_status(
payload["jobId"]
.as_str()
.unwrap_or("unknown-job")
.to_string(),
requested_at,
AnalysisJobErrorCode::EngineUnavailable,
"Analysis engine is unavailable.",
);
}
}
if let Some(mut stdin) = process.stdin.take() {
if stdin.write_all(payload.to_string().as_bytes()).is_err() {
let _ = process.kill();
let _ = process.wait();
return failed_status(
payload["jobId"]
.as_str()
.unwrap_or("unknown-job")
.to_string(),
requested_at,
AnalysisJobErrorCode::EngineUnavailable,
"Analysis engine is unavailable.",
);
}
}
🤖 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 380 - 393, The failure path
after stdin.write_all currently only calls process.kill() and returns, leaving
the child unreaped; after killing the child you must also wait on it to reap the
process before returning. Update the branch where stdin.write_all(...) fails
(referencing stdin.write_all, process.kill, failed_status and
AnalysisJobErrorCode::EngineUnavailable) to call process.wait() (or an
appropriate wait method) after kill(), ignore/log any wait errors, then return
the failed_status as before so the child is properly reaped.

Comment on lines +458 to +469
serde_json::from_slice::<AnalysisJobStatus>(&output.stdout).unwrap_or_else(|_| {
failed_status(
payload["jobId"]
.as_str()
.unwrap_or("unknown-job")
.to_string(),
requested_at,
AnalysisJobErrorCode::EngineUnavailable,
"Analysis engine returned an invalid response.",
)
})
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

엔진 응답의 불변식을 확인하지 않아 상태 저장소가 오염될 수 있습니다.

여기는 JSON shape만 맞으면 그대로 받아들이고, 이후 store_status()status.job_id를 key로 씁니다. 엔진이 잘못된 jobId, 비종료 상태, 혹은 succeeded인데 result가 없는 응답을 보내면 원래 작업이 영구히 running에 남거나 프런트엔드가 매번 generic error로 떨어질 수 있습니다. 최소한 현재 job_id와의 일치, terminal state 여부, stateresult/error의 조합을 확인한 뒤 아니면 안전한 실패로 매핑하세요.

🤖 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 458 - 469, Validate parsed
AnalysisJobStatus before persisting: after
serde_json::from_slice::<AnalysisJobStatus>(&output.stdout) (and before calling
store_status or using status.job_id), check that status.job_id matches
payload["jobId"] (fall back to failed_status if mismatch), ensure terminal
states (e.g., "succeeded" or "failed") have the expected fields (succeeded must
include result, failed must include error) and that non-terminal states aren't
treated as terminal; if any invariant fails, map to failed_status(...) with
AnalysisJobErrorCode::EngineUnavailable and a clear message rather than
accepting the payload. Update the code paths around failed_status,
AnalysisJobStatus, and store_status to perform these checks and only call
store_status when invariants hold.

Comment thread apps/desktop/src/App.tsx
Comment on lines +100 to +123
useEffect(() => {
if (!jobStatus || (jobStatus.state !== "queued" && jobStatus.state !== "running")) {
return;
}

const timer = window.setTimeout(async () => {
try {
const nextStatus = await getAnalysisJobStatus(jobStatus.jobId);
setJobStatus(nextStatus);
if (nextStatus.state === "succeeded" && nextStatus.result) {
setJobResult(nextStatus.result);
setJobError(null);
}
if (nextStatus.state === "failed") {
setJobError(nextStatus.error?.message ?? t("analysisCouldNotStart"));
}
} catch {
setJobStatus(null);
setJobError(t("analysisCouldNotStart"));
}
}, ANALYSIS_POLL_INTERVAL_MS);

return () => window.clearTimeout(timer);
}, [jobStatus]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

폴링 예외에서 작업 ID를 버리면 완료 결과를 다시 회수할 수 없습니다.

Line 117에서 jobStatusnull로 지우면 현재 작업의 jobId를 잃고 폴링도 바로 끊깁니다. IPC 오류가 일시적이어도 백엔드 작업은 계속 실행될 수 있으니, 이 경로는 완료 결과를 영구히 놓치고 버튼을 다시 열어 중복 시작까지 허용합니다. 에러는 별도 상태로 노출하고, 재시도 루프는 jobId를 유지한 채 계속 돌도록 분리해야 합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/App.tsx` around lines 100 - 123, The polling useEffect
currently clears jobStatus on any fetch error which discards jobId and stops
polling; instead preserve jobId and only set an error state so polling can
continue/retry. In the catch block of the useEffect that calls
getAnalysisJobStatus, remove setJobStatus(null) and call
setJobError(t("analysisCouldNotStart")) (or similar) without wiping jobStatus;
also consider changing the effect dependency from jobStatus to jobStatus?.jobId
(or include jobStatus.jobId) so the poll loop is keyed to the jobId and not
reset when transient state changes. Ensure setJobResult and setJobError handling
stays the same when nextStatus arrives.

Comment on lines +22 to +28
function getInvoke(): TauriInvoke | null {
if (typeof window === "undefined") {
return null;
}

return window.__TAURI_INVOKE__ ?? invoke;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Tauri v2, what is the documented way to detect that the app is running inside a Tauri runtime before calling invokefrom@tauri-apps/api/core, and does imported invoke work in a plain browser environment?

💡 Result:

  • Documented runtime check (Tauri v2): use isTauri() from @tauri-apps/api/core and only call invoke when it returns true. The core namespace documents isTauri(): boolean alongside invoke(). [1]

    import { isTauri, invoke } from "@tauri-apps/api/core";
    
    if (isTauri()) {
      await invoke("my_command");
    }
  • Does imported invoke work in a plain browser? The function can be imported (it’s just JS), but it won’t work outside a Tauri runtime: it calls window.__TAURI_INTERNALS__.invoke(...), which doesn’t exist in a normal browser page, so you’ll get a runtime error if you call it. [2]

  • Related: the global window.__TAURI__ API is only available when running under Tauri (and when app.withGlobalTauri is enabled), reinforcing that these APIs aren’t meant to run in a plain browser. [3]

Sources: [1] [2] [3]


🏁 Script executed:

# Check the full file to understand context
cat -n apps/desktop/src/lib/analysis.ts | head -50

Repository: seonghobae/bandscope

Length of output: 2064


🏁 Script executed:

# Search for __TAURI_INVOKE__ definition in the codebase
rg "__TAURI_INVOKE__" --type ts --type tsx -B 2 -A 2

Repository: seonghobae/bandscope

Length of output: 2064


🏁 Script executed:

# Search for how getInvoke is used in the codebase
rg "getInvoke" --type ts --type tsx -B 1 -A 1

Repository: seonghobae/bandscope

Length of output: 2064


🏁 Script executed:

# Search for isTauri usage in the codebase to see if it's already used
rg "isTauri" --type ts --type tsx -B 1 -A 1

Repository: seonghobae/bandscope

Length of output: 2064


Tauri 런타임 감지 방식을 isTauri()로 변경하세요.

현재 코드는 window 존재 여부로만 확인하기 때문에 일반 브라우저에서도 imported invoke를 반환합니다. 이 함수를 호출하면 window.__TAURI_INTERNALS__.invoke(...)에 접근하려다 런타임 에러가 발생합니다.

Tauri v2 문서의 표준 방식은 @tauri-apps/api/core에서 isTauri()를 사용해서 런타임을 확인한 후에만 invoke를 호출하는 것입니다:

import { isTauri, invoke } from "@tauri-apps/api/core";

if (isTauri()) {
  await invoke("command");
}

window.__TAURI_INVOKE__ 존재 여부는 신뢰성이 낮으므로 대신 isTauri()로 확인하세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/analysis.ts` around lines 22 - 28, The current
getInvoke() returns the imported invoke when window exists which can call into
missing Tauri internals; change its runtime detection to use isTauri() from
"@tauri-apps/api/core" instead of checking window presence. Update getInvoke()
to import and call isTauri() and only return window.__TAURI_INVOKE__ ?? invoke
when isTauri() is true; otherwise return null, ensuring you reference getInvoke,
isTauri, invoke, and window.__TAURI_INVOKE__ when making the change.

- prefer local processing for audio and analysis
- keep risky capabilities narrow, allowlisted, and explicit
- treat files, URLs, models, caches, and release artifacts as untrusted inputs
- route orchestration through typed Tauri IPC and a narrow Python subprocess bridge before considering any loopback HTTP surface
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Security Notes 섹션이 아직 없습니다.

Line 38은 IPC/서브프로세스 trust boundary 변경을 문서화하고 있는데, 이런 작업은 설계 문서나 구현 요약에 Security Notes를 반드시 남겨야 합니다. 공격 표면, 경계, 현실적인 위협, 완화책, 잔여 위험, 테스트 포인트가 빠지면 timeout·권한 범위·오류 노출 기준이 문서 밖으로 흩어집니다.

📝 문서 보강 예시
 ## Local-first principle
 
 - prefer local processing for audio and analysis
 - keep risky capabilities narrow, allowlisted, and explicit
 - treat files, URLs, models, caches, and release artifacts as untrusted inputs
 - route orchestration through typed Tauri IPC and a narrow Python subprocess bridge before considering any loopback HTTP surface
+
+## Security Notes
+
+- attack surface: Tauri IPC commands, Python subprocess stdin/stdout contract, job-status polling
+- trust boundary touched: UI -> native shell -> Python engine
+- realistic threats: malformed payloads, widened command exposure, hung subprocesses, sensitive stderr propagation
+- mitigations: allowlisted capabilities, strict schema validation, in-flight job limits, timeouts, redacted surfaced errors
+- remaining risk: Python/runtime dependency behavior and local toolchain variance
+- test points: unknown-field rejection, permission denial, timeout mapping, concurrent start throttling

Based on learnings: If a task touches files, URLs, subprocesses, ffmpeg or native tools, WebView, local backend or IPC, updates, model downloads, project formats, logs, telemetry, or exports, the result must include Security Notes

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/architecture/overview.md` at line 38, 문서의 "route orchestration through
typed Tauri IPC and a narrow Python subprocess bridge" 설명에 대해 별도의 "Security
Notes" 섹션을 추가하세요; 이 섹션에 IPC와 Python subprocess의 trust boundary를 명확히 정의하고(예:
Tauri IPC, Python subprocess bridge, loopback HTTP), 각 경계별 공격 표면·실제 위협 시나리오·권한
요구사항(권한 격리·권한 하향 최소화)·권한 검증 및 입력 검증/escaping 같은 완화책·잔여 위험·테스트 포인트(예: 권한 상승,
타임아웃/리소스 고갈, stderr/stdout 정보 유출, 악성 파일 실행) 등을 간단 항목화하여 포함시키세요; 해당 섹션은 문서 내
"Security Notes" 제목으로 추가하고 참조할 수 있게 route orchestration 문단에 링크나 레퍼런스를 남기세요.

Comment on lines +373 to +389
function validateAnalysisJobError(value: unknown, path: string): string | null {
if (!isRecord(value)) {
return invalidField(path);
}
const extraKey = unexpectedKey(value, ["code", "message"], path);
if (extraKey) {
return extraKey;
}
if (!isOneOf(ANALYSIS_JOB_ERROR_CODES, value.code)) {
return invalidField(`${path}.code`);
}
if (typeof value.message !== "string") {
return invalidField(`${path}.message`);
}

return null;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

분석 payload 오류가 rehearsal song contract로 보고됩니다.

Line 375-Line 385와 Line 393-Line 437은 invalidField()를 재사용해서, 잘못된 AnalysisJobError/AnalysisJobStatus"Invalid rehearsal song contract"로 반환합니다. 이 파일의 요청 파서는 이미 분석 전용 문구를 쓰고 있으니, 분석용 formatter를 분리하거나 계약명을 파라미터화해서 메시지를 맞춰 주세요.

Also applies to: 391-440

🤖 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 373 - 389, The
validateAnalysisJobError (and similarly validateAnalysisJobStatus) uses
invalidField which returns the generic "Invalid rehearsal song contract"
message; change this so analysis validators produce an analysis-specific message
by either adding an optional contractName parameter to invalidField (defaulting
to existing text) and passing "analysis job contract" from
validateAnalysisJobError/validateAnalysisJobStatus, or introduce a new helper
like invalidAnalysisField and use it in these validators; update all calls in
validateAnalysisJobError/validateAnalysisJobStatus to call the new/parameterized
helper so error messages are specific to analysis payloads rather than the
rehearsal contract.

Comment on lines +12 to +24
def failed_cli_response(message: str) -> dict[str, object]:
"""Return a typed CLI failure envelope for malformed stdin payloads."""
requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z")
return {
"jobId": "unknown-job",
"state": "failed",
"requestedAt": requested_at,
"updatedAt": requested_at,
"error": {
"code": "invalid_request",
"message": message,
},
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

예상치 못한 엔진 예외도 typed 실패 응답으로 변환해 주세요.

지금은 run_analysis_job(...)에서 예외가 나면 CLI가 traceback과 함께 비정상 종료됩니다. 그러면 상위 Tauri IPC 경계에서 안전한 실패 envelope 대신 프로세스 실패로 보이게 되고, 내부 오류 정보가 stderr로 새어 나갈 수 있습니다. 여기서 예외를 잡아 jobIdrequestedAt를 유지한 채 안전한 engine_unavailable류의 실패 응답으로 매핑해야 계약이 유지됩니다.

🔧 제안 수정안
-def failed_cli_response(message: str) -> dict[str, object]:
+def failed_cli_response(
+    message: str,
+    *,
+    code: str = "invalid_request",
+    job_id: str = "unknown-job",
+    requested_at: str | None = None,
+) -> dict[str, object]:
     """Return a typed CLI failure envelope for malformed stdin payloads."""
-    requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z")
+    requested_at = requested_at or datetime.now(UTC).isoformat().replace("+00:00", "Z")
     return {
-        "jobId": "unknown-job",
+        "jobId": job_id,
         "state": "failed",
         "requestedAt": requested_at,
         "updatedAt": requested_at,
         "error": {
-            "code": "invalid_request",
+            "code": code,
             "message": message,
         },
     }
@@
-    response = run_analysis_job(job_id, request, requested_at)
+    try:
+        response = run_analysis_job(job_id, request, requested_at)
+    except Exception:
+        response = failed_cli_response(
+            "Analysis engine is unavailable right now. Please try again.",
+            code="engine_unavailable",
+            job_id=job_id,
+            requested_at=requested_at,
+        )
     json.dump(response, sys.stdout)
     return 0

Also applies to: 27-49

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/analysis-engine/src/bandscope_analysis/cli.py` around lines 12 - 24,
Wrap the top-level body of run_analysis_job with a broad try/except that catches
all unexpected exceptions, logs the error (but does not let it crash the
process), and returns a typed failure envelope using the existing
failed_cli_response pattern but with an "error.code" like "engine_unavailable";
ensure the returned object preserves the original jobId and requestedAt values
from the incoming payload (use the same jobId/requestedAt variables used in
run_analysis_job) and sets state to "failed" and updatedAt to the preserved
timestamp so the CLI always emits a safe, contract-compliant failure instead of
crashing.

@seonghobae
Copy link
Copy Markdown
Owner Author

Superseded by #50 to clear the remaining stale CodeRabbit changes-requested review after all required checks passed and no new actionable review remained.

@seonghobae seonghobae closed this Mar 12, 2026
auto-merge was automatically disabled March 12, 2026 04:22

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant