fix(review): harden offline coverage sandbox - #687
Merged
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughJavaScript lockfile을 base와 검증된 HEAD 기준으로 materialize합니다. npm 설치는 검증된 lockfile과 오프라인 writable cache를 사용합니다. R coverage gate는 Changes오프라인 커버리지 샌드박스 및 OpenCode 리뷰 검증
Estimated code review effort: 5 (Critical) | ~100 minutes Sequence Diagram(s)sequenceDiagram
participant OpenCodeWorkflow
participant Materializer
participant TrustedManifest
participant NpmCache
participant npm
OpenCodeWorkflow->>Materializer: base_sha와 PR_HEAD_SHA 전달
Materializer->>TrustedManifest: lock blob SHA와 revision SHA 기록
TrustedManifest-->>OpenCodeWorkflow: 검증된 JavaScript 입력 반환
OpenCodeWorkflow->>NpmCache: trusted cache를 writable 경로에 복사
OpenCodeWorkflow->>npm: npm ci --ignore-scripts --offline 실행
sequenceDiagram
participant OpenCodeWorkflow
participant ReceiptCLI
participant OpenCodeModel
participant DispatchStatus
OpenCodeWorkflow->>ReceiptCLI: merge base, HEAD, 변경 파일 전달
ReceiptCLI-->>OpenCodeWorkflow: 현재 HEAD source-line receipt 반환
OpenCodeWorkflow->>OpenCodeModel: bounded evidence와 trusted receipt 전달
OpenCodeModel-->>OpenCodeWorkflow: 검증된 승인 결과 반환
OpenCodeWorkflow->>DispatchStatus: 승인, coverage, HEAD 상태 전달
DispatchStatus-->>OpenCodeWorkflow: 승인 상태 반환
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/ci/materialize_base_javascript_packages.py (1)
353-374: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
base_npm_projects(repo_root, base_sha)를 두 번 호출합니다.Line 355와 Line 371에서 같은 base 리비전에 대해 동일한 수집을 반복합니다. 각 호출은
git ls-tree와 프로젝트별git show를 다시 실행합니다. Line 356의 루프에서 이미 base lock blob을 계산하므로, base npm 결과와 blob 값을 재사용하십시오.♻️ 제안 리팩터
manifest: list[dict[str, str]] = [] projects: list[tuple[str, str, dict[str, bytes], str, str]] = [] - for source_path, package_manager, base_inputs in base_pnpm_projects( - repo_root, base_sha - ) + base_npm_projects(repo_root, base_sha): + base_npm = base_npm_projects(repo_root, base_sha) + base_npm_blobs: dict[str, str] = {} + for source_path, package_manager, base_inputs in ( + base_pnpm_projects(repo_root, base_sha) + base_npm + ): + lock_blob = _lock_blob_sha(repo_root, base_sha, source_path) projects.append( ( source_path, package_manager, base_inputs, base_sha.lower(), - _lock_blob_sha(repo_root, base_sha, source_path), + lock_blob, ) ) + for source_path, _package_manager, _base_inputs in base_npm: + base_npm_blobs[source_path] = _lock_blob_sha( + repo_root, base_sha, source_path + ) if head_sha is not None: if not SHA_RE.fullmatch(head_sha): raise ValueError("head SHA must be exactly 40 hexadecimal characters") - base_npm_blobs = { - source_path: _lock_blob_sha(repo_root, base_sha, source_path) - for source_path, _package_manager, _base_inputs in base_npm_projects( - repo_root, base_sha - ) - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/materialize_base_javascript_packages.py` around lines 353 - 374, Update the loop over base_pnpm_projects and base_npm_projects to retain the base npm project data and computed lock blob values while building projects. Use that retained data to construct base_npm_blobs instead of calling base_npm_projects(repo_root, base_sha) again inside the head_sha block, preserving the existing source-path-to-blob mapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/opencode-review-dispatch.yml:
- Around line 1393-1406: Update the npm dependency branch around
trusted_npm_lock_is_materialized, prepare_writable_npm_cache, and the npm ci
fallback so lock or preparation failures are captured as an aggregated
validation failure instead of terminating the step under set -euo pipefail.
Ensure the failure path continues through the summary and GITHUB_OUTPUT
publication logic, including replacing the direct return 1 at the missing-lock
branch with the existing failure-recording mechanism.
In `@scripts/ci/materialize_base_javascript_packages.py`:
- Around line 161-168: Update the ownership-exclusion condition in
base_pnpm_projects to check whether the project contains any lockfile named in
NPM_LOCK_NAMES, rather than checking only package-lock.json. Keep
base_npm_projects using the same shared criterion so projects with
npm-shrinkwrap.json are excluded from pnpm collection and avoid the ValueError
path.
In `@tests/test_materialize_base_javascript_packages.py`:
- Line 559: 이스케이프되지 않은 정규식 메타문자를 포함한 pytest의 match 패턴을 수정하십시오. 해당 pytest.raises
호출에서 리터럴 마침표가 정규식 와일드카드로 해석되지 않도록 raw 문자열 이스케이프 또는 re.escape를 사용하고, 오류 메시지 매칭
동작은 유지하십시오.
---
Nitpick comments:
In `@scripts/ci/materialize_base_javascript_packages.py`:
- Around line 353-374: Update the loop over base_pnpm_projects and
base_npm_projects to retain the base npm project data and computed lock blob
values while building projects. Use that retained data to construct
base_npm_blobs instead of calling base_npm_projects(repo_root, base_sha) again
inside the head_sha block, preserving the existing source-path-to-blob mapping.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 9595e6c4-d27c-4fde-85a3-b5b2836c2434
📒 Files selected for processing (8)
.github/workflows/opencode-review-dispatch.ymlscripts/ci/materialize_base_javascript_packages.pyscripts/ci/r_coverage_peer_gate.pyscripts/ci/test_strix_quick_gate.shtests/test_materialize_base_javascript_packages.pytests/test_opencode_agent_contract.pytests/test_opencode_model_pool_runner.pytests/test_r_coverage_peer_gate.py
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/test_opencode_security_boundaries.py (1)
275-280: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winfixture 반환 타입 주석을 generator 타입으로 고치십시오.
trusted_dispatch_status_artifacts는 Line 310에서yield를 사용합니다. 따라서 실제 반환 타입은Iterator[None]입니다. 현재 주석-> None은 부정확하며 정적 타입 검사가 오류를 보고할 수 있습니다.♻️ 제안 수정
+from collections.abc import Iterator + `@pytest.fixture` def trusted_dispatch_status_artifacts( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, -) -> None: +) -> Iterator[None]: """Seal the source and changed-file evidence used by dispatch-status review validation."""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_opencode_security_boundaries.py` around lines 275 - 280, Update the return annotation of the trusted_dispatch_status_artifacts pytest fixture to Iterator[None], matching its yield-based implementation. Add or reuse the appropriate typing import for Iterator if needed, without changing the fixture behavior.tests/test_opencode_adversarial_receipts.py (1)
26-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win임시 Git 저장소에 격리된 Git 환경을 사용하십시오.
로컬 identity 설정만으로는 전역 및 시스템 Git 설정을 차단하지 못합니다. 모든 Git 서브프로세스에
GIT_CONFIG_GLOBAL=/dev/null과GIT_CONFIG_SYSTEM=/dev/null을 적용하고,commit.gpgsign=false및core.hooksPath=/dev/null을 설정하십시오.git init전에 격리 환경을 적용하여init.templateDir도 차단하십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_opencode_adversarial_receipts.py` around lines 26 - 39, Update initialized_repo to run every Git subprocess with an isolated environment setting GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM to /dev/null before git init, preventing inherited global, system, and init-template configuration; also configure commit.gpgsign=false and core.hooksPath=/dev/null in the temporary repository while preserving the deterministic local identity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/opencode-review-dispatch.yml:
- Around line 3294-3296: Update the review instructions so missing or
contradictory trusted evidence produces a schema-valid fail-closed result using
REQUEST_CHANGES, not NEEDS_INFO or a bare status substitution. Require that
result to include a finding and a confirmed adversarial probe, with all cited
path, line, and source-line-sha256 values copied only from the Adversarial probe
source-line receipts section.
In `@scripts/ci/opencode_review_prompt_template.md`:
- Line 49: Update the receipt-copying instruction near the example JSON in the
prompt template to include the example probe’s line value, preserving line: 1 as
a JSON number and requiring it to be replaced with the exact positive integer
from the same trusted receipt entry as path and source-line-sha256. Ensure the
copied path:line and hash remain consistent.
---
Nitpick comments:
In `@tests/test_opencode_adversarial_receipts.py`:
- Around line 26-39: Update initialized_repo to run every Git subprocess with an
isolated environment setting GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM to
/dev/null before git init, preventing inherited global, system, and
init-template configuration; also configure commit.gpgsign=false and
core.hooksPath=/dev/null in the temporary repository while preserving the
deterministic local identity.
In `@tests/test_opencode_security_boundaries.py`:
- Around line 275-280: Update the return annotation of the
trusted_dispatch_status_artifacts pytest fixture to Iterator[None], matching its
yield-based implementation. Add or reuse the appropriate typing import for
Iterator if needed, without changing the fixture behavior.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 2d6a365b-ae55-499c-8383-52574409ca0b
📒 Files selected for processing (10)
.github/workflows/opencode-review-dispatch.ymlscripts/ci/opencode_adversarial_receipts.pyscripts/ci/opencode_dispatch_status.pyscripts/ci/opencode_review_prompt_template.mdscripts/ci/run_opencode_review_model_pool.shscripts/ci/test_strix_quick_gate.shtests/test_opencode_adversarial_receipts.pytests/test_opencode_agent_contract.pytests/test_opencode_model_pool_runner.pytests/test_opencode_security_boundaries.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_opencode_adversarial_receipts.py`:
- Around line 15-27: Update isolated_git_environment() to remove repository,
index, object-database, author, committer, and EMAIL environment variables that
can leak host state, including GIT_DIR, GIT_WORK_TREE, GIT_COMMON_DIR,
GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY, GIT_ALTERNATE_OBJECT_DIRECTORIES, all
GIT_AUTHOR_* and GIT_COMMITTER_* variables. Set a fixed test author and
committer identity and dates, and set GIT_TERMINAL_PROMPT to 0 while preserving
the existing Git configuration isolation.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 7d226c5a-321c-46d5-81bd-0f85a3a0a8c0
📒 Files selected for processing (5)
.github/workflows/opencode-review-dispatch.ymlscripts/ci/opencode_review_prompt_template.mdtests/test_opencode_adversarial_receipts.pytests/test_opencode_agent_contract.pytests/test_opencode_security_boundaries.py
🚧 Files skipped from review as they are similar to previous changes (4)
- scripts/ci/opencode_review_prompt_template.md
- tests/test_opencode_security_boundaries.py
- tests/test_opencode_agent_contract.py
- .github/workflows/opencode-review-dispatch.yml
github-project-automation
Bot
moved this from In Progress
to Done
in naruon Platform Roadmap
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
Suggests, using a root-owned immutable DESCRIPTION snapshot plus exact-head successful R CMD check evidenceProduction evidence
EAI_AGAINand missingc8; this branch materializes the trusted base lock before the sandbox goes offlineaFIPCand declared helpermockerywere absent from the central image, while exact-head R CMD check run 30654991325 succeededr-cran-mockery, so the solution preserves offline isolation and uses bounded peer-check evidence instead of adding an unavailable packageVerification
Current head
ea99bd7:coverage run -m pytest -q && coverage report— 780 passed; 6,267/6,267 central Python statements covered (100%)interrogate -c pyproject.toml scripts/ci— 100% docstring coveragegit diff --check, and post-edit CodeGraph exploration passedCloses #686
Unblocks ContextualWisdomLab/scopeweave#386
Unblocks ContextualWisdomLab/aFIPC#193
Summary by CodeRabbit
개선 사항
버그 수정
테스트