Skip to content

fix(coverage): restore validated npm workspace lock owners - #748

Closed
seonghobae wants to merge 24 commits into
mainfrom
fix/npm-workspace-coverage-root-clean
Closed

fix(coverage): restore validated npm workspace lock owners#748
seonghobae wants to merge 24 commits into
mainfrom
fix/npm-workspace-coverage-root-clean

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Purpose

Restore the fail-closed npm workspace lock-owner resolver and its central OpenCode coverage integration as a focused current-main change.

A selected nested package such as BandScope apps/desktop must install from its nearest validated npm workspace owner instead of requiring an invalid duplicate lockfile beside every workspace package.

Rebuild state

The branch was reset to protected main at f070c504c1cb06891b800d7ab0cf6ac7d3cf8eae because the previous stacked base had diverged and accidentally removed the verified LLVM 19 coverage toolchain while applying the workspace resolver. A bounded exact-parent repair now reapplies only the reviewed seven-file npm workspace ownership contract and its tests while preserving the current central control-plane baseline.

The temporary branch-repair helper and workflow must delete themselves before the verified product-policy commit is published. The final diff must contain exactly:

  • .github/workflows/opencode-review-dispatch.yml
  • docs/doctoring/npm-workspace-lock-ownership.md
  • scripts/ci/npm_workspace_install_root.py
  • tests/npm_workspace_test_support.py
  • tests/test_npm_workspace_install_root.py
  • tests/test_npm_workspace_install_root_hardening.py
  • tests/test_opencode_agent_contract.py

Merge gate

This PR remains Draft until the rebuilt seven-file diff is published and exact-head focused/full tests, resolver statement/branch coverage 100%, production docstrings 100%, Python compilation, Ruff, workflow contracts, security workflows, OpenCode, Noema, CodeRabbit, qualifying independent approval, unresolved-thread policy, and branch protection all succeed.

No predecessor-head check or review result is reused.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d27fb207-f79f-4b58-a306-e73c45f7a3ae

📥 Commits

Reviewing files that changed from the base of the PR and between e6fc914 and d9542e3.

📒 Files selected for processing (1)
  • scripts/ci/apply_pr748_current_main.py

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

Copy link
Copy Markdown
Contributor Author

Stack correction: #731 was closed without merge on August 4, 2026, so it is not a valid remaining dependency and none of its checks or reviews are transferable. Keep this PR draft at exact head e280cd677ecb4e34a907574f3fcbee289aeab593. The current protected-baseline dependency is #743. After #743 merges, rebuild or reconcile this branch onto the resulting protected main, remove inherited CodeQL/Strix and historical control-plane overlap, reduce the final diff to the bounded workspace-lock-owner slice named in the description, and rerun all exact-head quality, security, supply-chain, OpenCode, Noema, independent-approval, unresolved-thread, and branch-protection gates. Do not mark ready before that reconciliation.

@seonghobae
seonghobae force-pushed the fix/npm-workspace-coverage-root-clean branch 2 times, most recently from 4d3bd2c to 785f2c2 Compare August 5, 2026 08:37
@seonghobae
seonghobae changed the base branch from main to fix/central-strix-security-closure August 5, 2026 08:37
@seonghobae
seonghobae force-pushed the fix/npm-workspace-coverage-root-clean branch from 785f2c2 to b715577 Compare August 5, 2026 08:50
@seonghobae
seonghobae changed the base branch from fix/central-strix-security-closure to fix/opencode-coverage-failure-diagnostics August 5, 2026 08:50

Copy link
Copy Markdown
Contributor Author

Exact-head downstream evidence from ContextualWisdomLab/bandscope#751 exposes an additional fail-closed npm-lock compatibility defect that should be covered before this workspace-owner slice is declared complete.

At BandScope head fa1dc9436a7cc0fc2aa0982159309e4c2b5502f4, central run 30992866042 rejects the valid npm v3 entry apps/desktop/node_modules/@types/react-dom because it has version/peer metadata but omits resolved and integrity. The exact protected base acdbea6344fe1231c39535b575f4de35e4c607c9 contains the same npm-generated entry, so this is not a Dependabot regression. validate_head_npm_lock() currently requires every non-link node_modules entry to carry its own registry tarball and SHA-512 value, which is stricter than real npm workspace lock serialization and blocks an otherwise fully green security update.

Please preserve the supply-chain boundary rather than broadly skipping incomplete entries. A reviewable acceptance rule is: permit a metadata-only nested entry only when the lock contains a canonical package entry for the same package identity/version whose resolved URL is exactly https://registry.npmjs.org/...tgz and whose integrity is one valid SHA-512 SRI; reject missing canonical metadata, version mismatch, ambiguous conflicting canonical entries, non-registry URLs, and invalid/missing integrity. Add positive BandScope-shaped workspace coverage plus negative mismatch/ambiguity/URL/integrity cases at 100% statement and branch coverage. The workspace-owner resolution and dependency-cache receipt checks should then consume the validated lock unchanged.

@seonghobae
seonghobae force-pushed the fix/opencode-coverage-failure-diagnostics branch from 7d01600 to dcbd777 Compare August 5, 2026 11:25
@seonghobae
seonghobae force-pushed the fix/npm-workspace-coverage-root-clean branch from b715577 to f070c50 Compare August 5, 2026 12:15
@seonghobae
seonghobae changed the base branch from fix/opencode-coverage-failure-diagnostics to main August 5, 2026 12:17

Copy link
Copy Markdown
Contributor Author

Closing this stale stacked draft. Its current head is the protected main commit f070c504c1cb06891b800d7ab0cf6ac7d3cf8eae, while its base is the unmerged #759 feature branch, so the reported 33-file/102-commit diff does not represent the stated focused npm workspace resolver and cannot be safely reviewed or merged. Recreate the resolver as a clean current-main PR only after the surviving coverage/review baseline is integrated, preserving the validated lock-owner and BandScope regression contracts without inherited history.

@seonghobae seonghobae closed this Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Correction after re-reading the live branch: the stale stack was already reset and retargeted to protected main, and the current two-file diff is a bounded self-removing rebuild helper for the declared seven-file resolver slice. Reopening so that repair can finish. The PR must remain Draft and unmergeable until both helper files delete themselves, exactly the seven permanent files remain, 100% resolver coverage/docstrings and full current-head checks/reviews pass.

@seonghobae seonghobae reopened this Aug 5, 2026
Comment thread .github/workflows/rebuild-pr748-current-main.yml Fixed
Comment thread scripts/ci/apply_pr748_current_main.py Fixed
Comment thread scripts/ci/apply_pr748_current_main.py Fixed
Comment thread scripts/ci/apply_pr748_current_main.py Fixed

Copy link
Copy Markdown
Contributor Author

Release blocker: current scope is still a branch-rebuilding workflow plus apply_pr748_current_main.py, not the permanent seven-file npm workspace ownership repair. Do not execute or merge this source-writing path.

@opencode-agent address

Delete .github/workflows/rebuild-pr748-current-main.yml and the patch/apply helper. Reimplement the validated npm workspace lock-owner resolver directly through ordinary commits in the authoritative workflow, resolver, tests, support code, and APA 7 doctoring. Preserve current protected-main LLVM and coverage infrastructure. The resolver must select the nearest validated npm workspace owner, bind exact package identity and workspace declaration, reject traversal/symlinks/malformed patterns/ambiguous owners/missing or untrusted lockfiles, and never require an invalid duplicate lock beside each nested package. The final diff must contain only the intended permanent files and no repair workflow, trigger, snapshot, or writer. Keep Draft until exact-head focused/full tests, resolver 100% statement/branch coverage, public docstrings, Python compilation/Ruff, coverage workflow contracts, security/supply-chain gates, current-head OpenCode/Noema/CodeRabbit, independent approval, and branch protection all pass without reuse of predecessor evidence.

Copy link
Copy Markdown
Contributor Author

@jules Complete the existing current-main rebuild on exact live head 567e2310a6531b336ed198b400795e38b12aa1c8 without adding another workflow or helper. Keep Draft.

The branch still exposes only .github/workflows/rebuild-pr748-current-main.yml and scripts/ci/apply_pr748_current_main.py; the intended seven-file permanent diff was not published. Use the committed helper/workflow logic as reviewed input, but apply the focused resolver slice directly in an ordinary descendant commit, run its focused and complete quality gates, then delete both temporary files in the same final product commit.

Final base-to-head changed paths must be exactly:

  • .github/workflows/opencode-review-dispatch.yml
  • docs/doctoring/npm-workspace-lock-ownership.md
  • scripts/ci/npm_workspace_install_root.py
  • tests/npm_workspace_test_support.py
  • tests/test_npm_workspace_install_root.py
  • tests/test_npm_workspace_install_root_hardening.py
  • tests/test_opencode_agent_contract.py

Preserve the protected-main LLVM 19 toolchain and all existing central security/reviewer boundaries. Do not absorb the separate nested metadata-pin problem from #807 into this PR. Run focused resolver statement/branch coverage 100%, interrogate 100%, compileall, Ruff, complete central tests, workflow contracts, and git diff --check. Commit normally; no amend, force-push, source-generating workflow, Ready transition, merge, release, or protection/credential change. Stop without mutation if the live head differs before writing.

@seonghobae
seonghobae marked this pull request as ready for review August 7, 2026 00:35
@seonghobae
seonghobae enabled auto-merge (squash) August 7, 2026 00:36

@opencode-agent opencode-agent 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.

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 66d155513d08bce07c3d9c789797d43e2fe7b919.

  • Head SHA: 66d155513d08bce07c3d9c789797d43e2fe7b919

  • Workflow run: 31135677183

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: rebuild-pr748-current-main.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: rebuild-pr748-current-main.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["CI script: apply_pr748_current_main.py"]
  S2 --> I2["review and security gate shell path"]
  I2 --> R2["Review risk: CI script: apply_pr748_current_main.py"]
  R2 --> V2["bash -n plus Strix self-test"]
Loading

@opencode-agent

opencode-agent Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 66d155513d08bce07c3d9c789797d43e2fe7b919
  • Workflow run: 31135677183
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 66d155513d08bce07c3d9c789797d43e2fe7b919.

  • Head SHA: 66d155513d08bce07c3d9c789797d43e2fe7b919

  • Workflow run: 31135677183

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: rebuild-pr748-current-main.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: rebuild-pr748-current-main.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["CI script: apply_pr748_current_main.py"]
  S2 --> I2["review and security gate shell path"]
  I2 --> R2["Review risk: CI script: apply_pr748_current_main.py"]
  R2 --> V2["bash -n plus Strix self-test"]
Loading

@opencode-agent
opencode-agent Bot disabled auto-merge August 7, 2026 00:48
Comment thread .github/workflows/pr748-trigger-rebuild-once.yml Fixed
Comment thread .github/workflows/rebuild-pr748-current-main.yml Fixed
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

자동 정리: base 대비 실제 변경(diff)이 0건이라 이 PR을 닫습니다. 변경을 추가한 뒤 reopen하세요.

@github-actions github-actions Bot closed this Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@opencode-agent

Finish the bounded current-main reconstruction through its existing self-removing materializer. Import only the reviewed seven-file npm workspace lock-owner contract while preserving the current LLVM 19 coverage baseline. Verify nearest validated npm workspace owner selection for nested packages, exact regular lockfile/package ownership, traversal/symlink/workspace-glob fail-closed behavior, immutable base provenance, resolver 100% statement/branch coverage and public docstrings, Python compilation, Ruff, full workflow contracts and security gates. The final diff must contain only the seven documented product files and no temporary repair workflow/helper. Do not approve, merge, release, use COPILOT_GITHUB_TOKEN, or alter protections.

Copy link
Copy Markdown
Contributor Author

The PR now has an empty permanent diff after removing the PR-controlled rebuild and patch-application path. Closing the no-op branch; the npm workspace lock-owner repair must be rebuilt through ordinary reviewed source/test/doc commits if still needed.

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.

2 participants