Skip to content

fix(coverage): install npm workspaces from root lock - #1411

Open
seonghobae wants to merge 8 commits into
mainfrom
fix/opencode-npm-workspace-coverage
Open

fix(coverage): install npm workspaces from root lock#1411
seonghobae wants to merge 8 commits into
mainfrom
fix/opencode-npm-workspace-coverage

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Resolve the nearest regular, non-symlink npm lock at or below the validated coverage worktree root.
  • Run trusted offline npm ci from that lock project while keeping coverage scripts scoped to the changed nested package.
  • Add a workspace regression test and update the reviewed workflow blob pin.

Root cause

Central coverage discovered apps/desktop as the JavaScript package for ContextualWisdomLab/bandscope#1057, then looked for package-lock.json only in that directory. BandScope owns the lock at the repository root, so coverage failed before vitest could resolve from the workspace install.

Verification

  • python3 -m pytest -q — 1874 passed, 1 skipped, 17 subtests passed.
  • python3 -m pytest -q tests/test_opencode_rust_coverage_toolchain_contract.py tests/test_pr_review_autofix_nvidia_nim_contract.py — 31 passed, 1 skipped.
  • bash -n scripts/ci/test_strix_quick_gate.sh — passed.
  • actionlint was attempted with a 30-second bound but did not complete; it is not claimed as passing evidence.

Security Notes

  • The workflow still requires a regular non-symlink lock, exact current-head blob equality, and the existing trusted materialization manifest before offline installation.
  • Dependency installation remains offline with lifecycle scripts disabled and the unprivileged sandbox boundary unchanged.
  • The new ancestor search is bounded to COVERAGE_SOURCE_WORKDIR and fails closed if the package path escapes it or no lock is present.
  • No runtime product, URL, subprocess, IPC, model, or export boundary changed.

Devin Review

Summary by CodeRabbit

  • 개선 사항

    • 중첩된 npm 워크스페이스에서도 상위 프로젝트의 검증된 lock 파일을 자동으로 찾아 의존성을 설치합니다.
    • 오프라인 npm 설치 및 캐시 검증이 올바른 워크스페이스 범위에서 수행됩니다.
  • 버그 수정

    • 패키지 디렉터리에 lock 파일이 없는 npm 워크스페이스의 커버리지 설치 문제를 해결했습니다.
  • 테스트

    • 상위 lock 파일 탐색과 워크스페이스 커버리지 설치에 대한 회귀 테스트를 추가했습니다.
  • 문서

    • npm 워크스페이스 커버리지 설치 동작 변경 사항을 변경 로그에 기록했습니다.

@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 25 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4247c6b6-793c-4c3f-8f9e-934707398ffa

📥 Commits

Reviewing files that changed from the base of the PR and between 17f3cf1 and e09206b.

📒 Files selected for processing (3)
  • .github/workflows/noema-review.yml
  • scripts/ci/test_strix_quick_gate.sh
  • tests/test_required_workflow_queue_contract.py
📝 Walkthrough

Walkthrough

중첩된 npm 워크스페이스 패키지가 가장 가까운 검증된 조상 lock 파일을 사용하도록 설치 경로를 변경했습니다. 워크플로 검사 헬퍼와 관련 계약 검사를 갱신했습니다.

Changes

npm 워크스페이스 커버리지 설치

Layer / File(s) Summary
상위 npm lock 파일 탐색
.github/workflows/opencode-review-dispatch.yml, tests/test_opencode_agent_contract.py
npm_lock_project_dir가 검증된 커버리지 워크트리 안에서 일반 npm-shrinkwrap.json 또는 package-lock.json을 찾습니다. 중첩 패키지와 워크트리 밖 실행의 동작을 검증합니다.
해석된 디렉터리에서 의존성 설치
.github/workflows/opencode-review-dispatch.yml, CHANGELOG.md, tests/test_pr_review_autofix_nvidia_nim_contract.py
npm 설치가 lock 디렉터리로 이동한 뒤 lock 검증, 캐시 준비, 오프라인 npm ci를 실행하고 원래 디렉터리로 복귀합니다. 변경 로그와 워크플로 blob SHA를 갱신했습니다.
워크플로 검사 검증
scripts/ci/test_strix_quick_gate.sh
required_workflow_bootstrap_has_if가 큰 required-workflow-bootstrap: 잡 블록에서도 if: 조건을 검사합니다. 관련 quick gate 검사가 새 헬퍼를 사용합니다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 17f3c

The PR fixes workspace-root npm installation while preserving the existing trust and offline-installation safeguards. A quick-gate pattern may still falsely reject valid workflows by matching nested step conditions, so the change is mergeable with explicit follow-up to narrow that match.

Sequence Diagram(s)

sequenceDiagram
  participant CoverageStep
  participant npm_lock_project_dir
  participant LockProject
  participant npm
  CoverageStep->>npm_lock_project_dir: npm lock 프로젝트 디렉터리 해석
  npm_lock_project_dir->>LockProject: 일반 lock 파일 확인
  LockProject-->>CoverageStep: 프로젝트 디렉터리 반환
  CoverageStep->>npm: 해당 디렉터리에서 오프라인 npm ci 실행
  npm-->>CoverageStep: 설치 완료 후 원래 디렉터리 복귀
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 중첩된 npm 워크스페이스가 루트 lock 파일을 사용하도록 의존성 설치 경로를 수정하는 PR의 주요 변경 사항을 정확하고 간결하게 설명합니다.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (3 skipped: 2 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (3 skipped: 2 unsupported, 1 too large.)

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/opencode-npm-workspace-coverage

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

claude added 2 commits August 30, 2026 09:52
…-workspace-coverage

# Conflicts:
#	CHANGELOG.md
…space-coverage

# Conflicts:
#	CHANGELOG.md
#	tests/test_pr_review_autofix_nvidia_nim_contract.py

Copy link
Copy Markdown
Contributor Author

Resolved the stale merge conflict against current main and pushed a merge commit (4d5559b7).

Conflicts, both mechanical (no design decision involved):

  1. CHANGELOG.md — both this PR and main (independently, via fix(sidecar): give the gateway preflight probe a real reasoning budget #1436/fix(ci): gate Strix's orchestrator/free access on live diversity evidence #1437-class fixes landing after this PR was opened) appended distinct ## [Unreleased] bullets with no textual overlap. Resolved by keeping both entries, this PR's npm-workspace-lock entry first, followed by main's sidecar/Strix/ZDR entries unchanged.
  2. tests/test_pr_review_autofix_nvidia_nim_contract.pyREVIEW_DISPATCH_BLOB_SHA is a pinned git blob SHA of .github/workflows/opencode-review-dispatch.yml. Both sides had bumped it independently (this PR's npm_lock_project_dir() helper vs. main's unrelated changes to the same workflow file), so the constant necessarily diverged. Recomputed the correct post-merge value directly: git hash-object .github/workflows/opencode-review-dispatch.yml256228eb7fc8f0c1613fbb7ec48effc8d6322c1b, and used that (no other file references the stale SHAs).

The workflow file itself (.github/workflows/opencode-review-dispatch.yml) auto-merged cleanly — this PR's npm_lock_project_dir() ancestor-lock-search helper and main's unrelated edits didn't textually overlap.

Test evidence (post-merge, in an isolated worktree):

  • PYTHONPATH=. python3 -m pytest tests/test_opencode_agent_contract.py tests/test_pr_review_autofix_nvidia_nim_contract.py -q → 78 passed
  • PYTHONPATH=. python3 -m pytest tests -q (full suite) → 1898 passed, 1 skipped, 21 subtests passed
  • coverage run -m pytest tests -q && coverage report --show-missing → 100% (10072/10072 not applicable here; scripts/ci is untouched by this PR's diff, full-repo scripts/ci coverage remains 100%)
  • interrogate (repo-configured, tests/ excluded) → PASSED, 100%
  • git diff --check origin/main...HEAD → clean
  • Python ast.parse on both touched test files → syntax OK

No scripts/ci/*.py files are touched by this PR's own diff, so the 100% coverage/docstring gates are satisfied trivially by the unrelated files remaining at 100% as before.


Generated by Claude Code

@seonghobae seonghobae moved this from Todo to In Progress in naruon Platform Roadmap Aug 31, 2026
@seonghobae

Copy link
Copy Markdown
Contributor Author

Codex is reconciling this PR with current main and revalidating its exact-head coverage/review gates. Started 2026-08-31 23:05 KST; working in an isolated worktree and preserving the existing checkout.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Reconciled current main at exact head 17f3cf10f42ae831abfc025f80458a54b106b069. The PR is now GitHub-mergeable. Preserved the npm-workspace fix and current-main changelog entries; also ported the same source-branch quick-gate correction proven in #1506 because exact-head-path-policy checks out each PR head. Fresh local focused evidence: PYTHONPATH=. python3 -m pytest -q tests/test_opencode_agent_contract.py tests/test_pr_review_autofix_nvidia_nim_contract.py → 78 passed; workflow blob pin remains exact. The full quick-gate run was interrupted after 15 minutes and is not claimed as passing evidence; hosted exact-head checks are authoritative and now queued.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current exact-head verification for 17f3cf10f42ae831abfc025f80458a54b106b069: PYTHONPATH=. python3 -m pytest -q completed successfully — 2127 passed, 1 skipped, 21 subtests passed in 170.24s. Hosted Strix Changed Path Quality CI / exact-head-path-policy also succeeded on this head.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Addressed CodeRabbit’s exact-head finding in bc965eae: the shell contract now matches only exactly four-space-indented job-level if: keys, consistent with the Python contract, so nested step-level conditions cannot cause a false positive. Revalidated: git diff --check; focused contracts 78 passed.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Fresh full-suite verification on current exact HEAD bc965eae: PYTHONPATH=. python3 -m pytest -q2127 passed, 1 skipped, 21 subtests passed in 184.18s. The prior-head evidence is superseded by this run.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

Devin Review

Comment thread .github/workflows/noema-review.yml
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current exact-head evidence for e09206b1: fixed the Noema concurrency root cause where a cancelled OpenCode/Strix workflow_run could evict an actionable review before its job-level guard skipped the cancelled event. Cancelled workflow-run events now use their unique run ID; actionable triggers still serialize by PR. Validation: focused Noema/required-workflow contracts 114 passed; full suite 2127 passed, 1 skipped, 21 subtests passed in 210.36s.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent Review exact current head e09206b15f245c1053bbfbed6135be1edf1e7475 only. Recompute the formal verdict from current hosted evidence; do not reuse predecessor reviews. The previous required watcher timed out without an authenticated current-head verdict.

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

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants