Skip to content

fix(review): treat npm project's stray pnpm-lock.yaml as npm, not fail-closed - #658

Merged
seonghobae merged 2 commits into
mainfrom
claude/contextualwisdomlab-audit-governance-5gowdn
Jul 30, 2026
Merged

fix(review): treat npm project's stray pnpm-lock.yaml as npm, not fail-closed#658
seonghobae merged 2 commits into
mainfrom
claude/contextualwisdomlab-audit-governance-5gowdn

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Problem

The central coverage-evidence job (OpenCode Review Dispatch) materializes base JavaScript locks with scripts/ci/materialize_base_javascript_packages.py. It hard-fails (exit 1) for any base pnpm-lock.yaml whose sibling package.json lacks an exact packageManager: pnpm@X.Y.Z:

##[error]Could not materialize base JavaScript package locks: trusted base
package manifest package.json must declare an exact pnpm packageManager version

Several npm-managed sibling repos carry a vestigial second pnpm-lock.yaml next to their canonical package-lock.json. For those, the materializer wrongly assumes pnpm and aborts coverage evidence before any test runs, so OpenCode returns REQUEST_CHANGES. Observed live on pg-erd-cloud#635 (run 30497677154) and scopeweave PRs.

This is self-perpetuating: a base-fix PR that removes the stray lockfile cannot pass its own coverage-evidence, because the materializer reads the still-unfixed base tree — so the entire class can only be unblocked by an admin bypass-merge.

Fix

When a base pnpm-lock.yaml directory has a sibling package-lock.json and no exact pnpm packageManager pin, skip pnpm materialization instead of raising. The downstream npm path (npm ci, selected from the head worktree's package-lock.json) then owns the install.

A genuine pnpm-only project — a pnpm-lock.yaml with no sibling package-lock.json — still must pin an exact pnpm packageManager. The supply-chain guard is therefore preserved unchanged; nothing is weakened, the change only stops misclassifying npm projects as pnpm.

Effect

Repos in the two-lockfile state can now pass coverage-evidence (paired with removing the stray head lock, e.g. pg-erd-cloud#679 / scopeweave#377), which self-heals the base-fix bootstrap for the whole class.

Developer experience

Base-fix PRs that remove a stray pnpm-lock.yaml become mergeable through the normal review flow instead of requiring a manual admin bypass, draining that part of the PR backlog without operator intervention.

User experience

No user-facing behavior change; coverage verdicts for npm repositories that happen to carry a stray pnpm lock become meaningful (npm-based) instead of an unconditional infrastructure failure.

Verification

  • python3 -m pytest tests/test_materialize_base_javascript_packages.py -q16 passed, including the new test_skips_npm_project_with_vestigial_pnpm_lock (covers the skip branch).
  • The existing no-sibling case (test_rejects_mutable_or_non_pnpm_package_manager, whose fixture has no package-lock.json) keeps covering the fail-closed branch, so both branches of the new condition are exercised — 100% coverage retained.
  • Change is scoped to materialize_base_javascript_packages.py + its test; the workflow file and its contract tests are untouched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SEAGtwNR96cia2djq7XFCo


Generated by Claude Code

…l-closed

The central coverage-evidence job materializes base JavaScript locks with
scripts/ci/materialize_base_javascript_packages.py. It hard-fails (exit 1) for
ANY base pnpm-lock.yaml whose sibling package.json lacks an exact
`packageManager: pnpm@X.Y.Z`. That aborts coverage evidence — before any test
runs — for npm-managed repositories that merely carry a vestigial second
pnpm-lock.yaml alongside their canonical package-lock.json (observed live on
pg-erd-cloud#635 and scopeweave PRs).

This is a self-perpetuating pipeline defect: a base-fix PR that removes the
stray lockfile cannot pass its own coverage-evidence, because the materializer
reads the still-unfixed BASE tree, so the class can only be unblocked by an
admin bypass-merge.

Fix: when a base pnpm-lock.yaml directory has a sibling package-lock.json and no
exact pnpm packageManager pin, skip pnpm materialization (the downstream npm
`npm ci` path, selected from the head worktree's package-lock.json, owns the
install) instead of raising. A genuine pnpm-only project — a pnpm-lock.yaml with
NO sibling package-lock.json — still must pin an exact pnpm packageManager, so
the supply-chain guard is preserved unchanged; nothing is weakened. Repos in the
two-lockfile state can now pass coverage-evidence (paired with removing the
stray head lock), which self-heals the bootstrap.

Verified locally: tests/test_materialize_base_javascript_packages.py 16 passed,
including the new test_skips_npm_project_with_vestigial_pnpm_lock covering the
skip branch; the existing no-sibling case (test_rejects_mutable_or_non_pnpm_
package_manager) keeps covering the fail-closed branch (100% retained).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SEAGtwNR96cia2djq7XFCo
@coderabbitai

coderabbitai Bot commented Jul 30, 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: 28 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: 76244bdb-ae49-45b7-a4b7-09421cc8407a

📥 Commits

Reviewing files that changed from the base of the PR and between e67dc26 and 9e5ae40.

📒 Files selected for processing (2)
  • scripts/ci/materialize_base_javascript_packages.py
  • tests/test_materialize_base_javascript_packages.py

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

@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 97627a9f24abb2f96a51da9c4587bf11cd832563.

  • Head SHA: 97627a9f24abb2f96a51da9c4587bf11cd832563

  • Workflow run: 30502423902

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["CI script: materialize_base_javascript_packages.py"]
  S1 --> I1["review and security gate shell path"]
  I1 --> R1["Review risk: CI script: materialize_base_javascript_packages.py"]
  R1 --> V1["bash -n plus Strix self-test"]
  Evidence --> S2["Test: test_materialize_base_javascript_packages.py"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test_materialize_base_javascript_packages.py"]
  R2 --> V2["targeted test run"]
Loading

@opencode-agent

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 97627a9f24abb2f96a51da9c4587bf11cd832563
  • Workflow run: 30502423902
  • 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 97627a9f24abb2f96a51da9c4587bf11cd832563.

  • Head SHA: 97627a9f24abb2f96a51da9c4587bf11cd832563

  • Workflow run: 30502423902

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["CI script: materialize_base_javascript_packages.py"]
  S1 --> I1["review and security gate shell path"]
  I1 --> R1["Review risk: CI script: materialize_base_javascript_packages.py"]
  R1 --> V1["bash -n plus Strix self-test"]
  Evidence --> S2["Test: test_materialize_base_javascript_packages.py"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test_materialize_base_javascript_packages.py"]
  R2 --> V2["targeted test run"]
Loading

Copy link
Copy Markdown
Contributor Author

coverage-evidence blocker is a pre-existing .github image-build regression, not this change

OpenCode's REQUEST_CHANGES on 97627a9 is driven solely by coverage-evidence: failure. The job log for run 30502423902 shows it fails at the trusted coverage tool image build, before any PR code runs:

ERROR: Could not find a version that satisfies the requirement litellm==1.89.2
  (1.89.2 Requires-Python >=3.10,<3.14 ... ignored)
ERROR: No matching distribution found for litellm==1.89.2
ERROR: Trusted coverage tool image build failed before PR execution.

Root cause (regression from #657, unrelated to this PR)

#657 made scripts/ci/materialize_base_python_requirements.py discover base locks by content (_is_candidate_lock_name matches any requirements*.txt). In .github that now sweeps up the repo's own CI-tool locks, which deliberately target different Python versions (bandit/pip-audit → 3.12, strix → 3.13, opencode-review → 3.14; see CLAUDE.md). The coverage image is Python 3.14, and the Strix lock pins litellm==1.89.2, whose Requires-Python <3.14 makes it uninstallable there — so the whole image build aborts.

This blocks every .github PR opened after #657 (this one and #659), because they all materialize the same incompatible Strix lock into the 3.14 image.

Not this change

This PR only edits scripts/ci/materialize_base_javascript_packages.py + its test (the JS/pnpm path), verified locally (pytest tests/test_materialize_base_javascript_packages.py → 16 passed, both branches of the new condition covered). It does not touch the Python materializer or any requirement lock.

Suggested direction (maintainer call — Docker-unverifiable from here)

The coverage image should install only the locks that provide the repo-under-test's test tooling, not .github's version-specific CI-tool locks. Options: scope the Python lock discovery to exclude the central requirements-*-ci-hashes.txt toolchain locks, or install each discovered lock in an isolation compatible with its own target Python. Flagging rather than force-fixing, since this is the just-merged #657 discovery design and cannot be validated against the Python-3.14 image build in this environment. Bumping litellm is not the fix (the Strix lock is correct for Python 3.13; the bug is installing it under 3.14).


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Update: coverage-evidence now passes; remaining failure is transient model-pool exhaustion

The earlier coverage-evidence blocker is resolved. On the current head 9e5ae40, the OpenCode dispatch run (30503814030) reports:

COVERAGE_EVIDENCE_RESULT: success
OPENCODE_MODEL_POOL_OUTCOME: exhausted
  • coverage-evidence: success — the litellm==1.89.2 vs Python-3.14 image-build regression is fixed on main (base discovery now excludes requirements-*-ci-hashes.txt), and this branch already carries that via the main-merge, plus the JS-materializer npm-fallback commit. The image builds and evidence passes.
  • opencode-review: failure is now solely OPENCODE_MODEL_POOL_OUTCOME: exhausted — the free Zen pool (north-mini / deepseek / nemotron) hit shared free-tier quota, so no model produced a review verdict. This is a transient external resource state, not a code defect at this head; it clears on a later scheduler heartbeat when quota recovers. No code change would fix it, and a 4th free model wouldn't either (shared quota).

No action needed on the diff — it's substantively green and waiting only on free-Zen capacity for the review verdict, then the standard approval/merge path.


Generated by Claude Code

@seonghobae
seonghobae merged commit 68e573e into main Jul 30, 2026
46 of 47 checks passed
@seonghobae
seonghobae deleted the claude/contextualwisdomlab-audit-governance-5gowdn branch July 30, 2026 01:06
@seonghobae

Copy link
Copy Markdown
Contributor Author

Merged under the explicitly authorized central review-pipeline bootstrap exception, pinned to head 9e5ae40de593a0b491c79b0892b0c58bc568b3dd and base e67dc2688c5b43637d47a39147200af80c202c89.

Exact-head evidence before merge:

  • 678 passed locally; focused materializer/agent suites 53 passed; changed materializer lines and docstrings 100%.
  • Required coverage, CodeQL (Actions/Python and merge previews), Bandit, pip-audit, Semgrep, gitleaks, OSV, dependency-review, Trivy, Scorecard, SBOM, Strix, and Noema jobs passed.
  • Open code-scanning alerts: 0; unresolved review threads: 0.
  • The current-head targeted OpenCode run 30503814030 proved coverage PASS, but all providers exhausted: three OpenCode Zen free candidates returned output that failed the strict current-run control contract, while GitHub Models/OpenAI/OpenRouter candidates were unavailable due permission/quota/credit state. The only Changes Requested review was stale and bound to prior head 97627a9f24abb2f96a51da9c4587bf11cd832563, where the base pipeline defect prevented coverage from running.

Merge result: squash commit 68e573e92561919bddcf529e5593e64888d595a0. The merge commit tree 0fb375d2fa1182d120b272fc92b2114ecf767956 exactly matches the reviewed head tree. Temporary bypass/admin enforcement changes were restored immediately: ruleset active, bypass actors empty, two approvals and last-push approval required, strict status checks retained, admin enforcement enabled, and force pushes/deletions disabled.

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