diff --git a/.github/workflows/noema-review.yml b/.github/workflows/noema-review.yml index 064c4e5aee..c0016afe4e 100644 --- a/.github/workflows/noema-review.yml +++ b/.github/workflows/noema-review.yml @@ -22,6 +22,10 @@ concurrency: noema-review-${{ github.event.pull_request.base.repo.full_name || github.event.client_payload.target_repository || github.repository }}-${{ + github.event_name == 'workflow_run' + && github.event.workflow_run.conclusion == 'cancelled' + && github.run_id + || github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number || github.event.client_payload.pr_number || github.run_id }} diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 2aa245e7f2..256228eb7f 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -1404,6 +1404,38 @@ jobs: writable_npm_cache_dir="" writable_pnpm_store_dir="" + npm_lock_project_dir() { + local candidate_dir="$PWD" + + case "$candidate_dir" in + "$COVERAGE_SOURCE_WORKDIR"|"$COVERAGE_SOURCE_WORKDIR"/*) ;; + *) + echo "::error::npm package directory escaped the validated coverage worktree." >&2 + return 1 + ;; + esac + + while true; do + if [ -f "$candidate_dir/npm-shrinkwrap.json" ] \ + && [ ! -L "$candidate_dir/npm-shrinkwrap.json" ]; then + printf '%s\n' "$candidate_dir" + return 0 + fi + if [ -f "$candidate_dir/package-lock.json" ] \ + && [ ! -L "$candidate_dir/package-lock.json" ]; then + printf '%s\n' "$candidate_dir" + return 0 + fi + if [ "$candidate_dir" = "$COVERAGE_SOURCE_WORKDIR" ]; then + break + fi + candidate_dir="$(dirname "$candidate_dir")" + done + + echo "::error::No regular non-symlink npm lock was found at the package or validated workspace root." >&2 + return 1 + } + trusted_npm_lock_is_materialized() { local relative_dir local lock_name @@ -1613,9 +1645,11 @@ jobs: install_package_dependencies() { local package_runner="$1" + local npm_project_dir case "$package_runner" in npm) - if [ -f package-lock.json ] || [ -f npm-shrinkwrap.json ]; then + if npm_project_dir="$(npm_lock_project_dir)"; then + pushd "$npm_project_dir" >/dev/null if ! trusted_npm_lock_is_materialized || ! prepare_writable_npm_cache; then append "### JavaScript/TypeScript dependencies (npm)" append "" @@ -1623,6 +1657,7 @@ jobs: append "- Reason: the current npm lock is not hash-bounded to the validated base or HEAD, or the trusted npm cache is unavailable." append "" failures=$((failures + 1)) + popd >/dev/null return 0 fi run_and_capture "JavaScript/TypeScript dependencies (npm offline ci, lifecycle hooks disabled)" \ @@ -1632,6 +1667,7 @@ jobs: --cache "$writable_npm_cache_dir" \ --no-audit \ --no-fund + popd >/dev/null else append "### JavaScript/TypeScript dependencies (npm)" append "" diff --git a/CHANGELOG.md b/CHANGELOG.md index 39c61c142b..56e6303dd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Run npm workspace coverage installs from the nearest validated ancestor lock + while keeping tests scoped to the changed package; regular non-symlink lock + files remain hash-bounded by the existing materialization manifest. - Harden the review sidecar's per-account catalog cap against silent drift: `contextual_orchestrator_review_launcher.py`'s two `build_zdr_prioritized_catalog` call sites now source their diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 4053f4fd53..271fd0c23b 100644 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -96,6 +96,13 @@ assert_file_not_contains() { fi } +required_workflow_bootstrap_has_if() { + local bootstrap_file="$1" + + awk '/^ required-workflow-bootstrap:$/{p=1; print; next} p && /^ [A-Za-z0-9_-]+:/{exit} p' "$bootstrap_file" | + grep '^ if:' >/dev/null +} + seal_opencode_test_artifacts() { local runner_temp="$1" local head_sha="$2" @@ -522,9 +529,23 @@ assert_opencode_review_uses_codegraph_and_contextual_orchestrator() { assert_file_not_contains "$workflow_file" "Wait for trusted OpenCode approval review" "opencode pull_request bridge was removed to avoid duplicate required-check resource use" assert_file_not_contains "$workflow_file" "Trusted OpenCode requested changes for head" "opencode pull_request bridge no longer reconsumes stale trusted review state" assert_file_not_contains "$workflow_file" "github.event.pull_request.number == 240" "opencode review workflow must not hard-code repository-specific PR bypasses" - if awk '/^ required-workflow-bootstrap:$/,/^[^ ]/' "$bootstrap_file" | grep -q '^[[:space:]]*if:'; then + if required_workflow_bootstrap_has_if "$bootstrap_file"; then record_failure "opencode required workflow bootstrap must not depend on required-workflow event payload fields" fi + local large_bootstrap_fixture + local fixture_line + large_bootstrap_fixture="$(mktemp)" + { + printf '%s\n' 'jobs:' ' required-workflow-bootstrap:' ' if: forbidden' + for ((fixture_line = 0; fixture_line < 20000; fixture_line++)); do + printf '%s\n' ' # padding forces the producer past the pipe buffer' + done + printf '%s\n' ' next-job:' ' runs-on: ubuntu-latest' + } >"$large_bootstrap_fixture" + if ! required_workflow_bootstrap_has_if "$large_bootstrap_fixture"; then + record_failure "opencode required workflow bootstrap condition detection must survive a job block larger than the pipe buffer" + fi + rm -f "$large_bootstrap_fixture" assert_file_contains "$workflow_file" 'github.event.client_payload.target_repository || github.repository' "opencode review scopes concurrency by target repository" assert_file_contains "$workflow_file" "format('pr-{0}', github.event.client_payload.pr_number)" "opencode review scopes repository_dispatch concurrency by current PR" assert_file_not_contains "$workflow_file" "format('pr-{0}-{1}'" "opencode review does not keep stale head-specific concurrency groups" @@ -1501,7 +1522,7 @@ assert_opencode_review_posts_suggested_diffs_inline() { assert_file_contains "$workflow_file" "publish_request_changes_from_control" "opencode review REQUEST_CHANGES path publishes findings from the control JSON" if awk '/format_request_changes_body\(\)/,/build_request_changes_review_payload\(\)/ { print }' "$workflow_file" | - grep -Fq '```diff'; then + grep -F '```diff' >/dev/null; then record_failure "opencode review PR-level REQUEST_CHANGES body must not contain fenced suggested diffs" fi } diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 79fdba39aa..b8d2de7f55 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -627,6 +627,9 @@ def test_opencode_target_coverage_materializes_only_after_authorized_dispatch(): assert "corepack pnpm fetch" in measure_step assert "--store-dir /opt/pnpm-store" in measure_step assert "chmod -R a+rX /opt/corepack /opt/npm-cache /opt/pnpm-store" in measure_step + assert "npm_lock_project_dir() {" in measure_step + assert 'pushd "$npm_project_dir" >/dev/null' in measure_step + assert "No regular non-symlink npm lock was found at the package or validated workspace root." in measure_step assert "trusted_npm_lock_is_materialized()" in measure_step assert ( 'head_blob="$(trusted_git rev-parse "${PR_HEAD_SHA}:${relative_lock}"' @@ -1540,6 +1543,64 @@ def test_opencode_coverage_discovers_changed_nested_javascript_package(tmp_path) assert result.stdout.splitlines() == ["ADFS 연동 라이브러리/Node.JS/Node App"] +def test_opencode_coverage_resolves_ancestor_npm_lock_for_workspace_package(tmp_path): + """Workspace coverage installs from the root lock while testing the nested package.""" + bash = shutil.which("bash") + if bash is None: + pytest.skip("bash is required for the extracted workflow function regression test") + + workflow = Path(".github/workflows/opencode-review-dispatch.yml").read_text( + encoding="utf-8" + ) + measure_start = workflow.index( + " - name: Measure test and docstring evidence\n" + ) + measure_end = workflow.index("\n - name:", measure_start + 1) + measure_step = workflow[measure_start:measure_end] + helper_start = measure_step.index(" npm_lock_project_dir() {\n") + helper_end = measure_step.index( + "\n\n trusted_npm_lock_is_materialized()", helper_start + ) + shell = "\n".join( + ( + "set -euo pipefail", + textwrap.dedent(measure_step[helper_start:helper_end]), + "npm_lock_project_dir", + ) + ) + + repo = tmp_path / "repo" + package = repo / "apps" / "desktop" + package.mkdir(parents=True) + (repo / "package-lock.json").write_text("{}\n", encoding="utf-8") + env = os.environ.copy() + env["COVERAGE_SOURCE_WORKDIR"] = str(repo) + result = subprocess.run( + [bash, "-c", shell], + cwd=package, + env=env, + capture_output=True, + text=True, + timeout=10, + ) + + assert result.returncode == 0, result.stderr + assert result.stdout.splitlines() == [str(repo)] + + outside = tmp_path / "outside" + outside.mkdir() + escaped = subprocess.run( + [bash, "-c", shell], + cwd=outside, + env=env, + capture_output=True, + text=True, + timeout=10, + ) + assert escaped.returncode != 0 + assert "escaped the validated coverage worktree" in escaped.stderr + + def test_opencode_runtime_pin_supports_reasoning_options(): """Keep OpenCode runtime new enough to apply model-level reasoning settings.""" review_workflow = Path(".github/workflows/opencode-review-dispatch.yml").read_text( diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 3dcfe2cdd8..33b1fd4b98 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "2aa245e7f2a053a4c0b7a9cc8bac0d5d44d38092" +REVIEW_DISPATCH_BLOB_SHA = "256228eb7fc8f0c1613fbb7ec48effc8d6322c1b" def _workflow_text(path: Path) -> str: diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index b3eac37fac..dbbdd79625 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -489,11 +489,14 @@ def test_required_workflow_trusted_source_refs_are_not_input_controlled() -> Non assert "GITHUB_CONTEXT_JSON: ${{ toJSON(github) }}" in workflow -def test_noema_triggers_serialize_one_review_per_pull_request() -> None: - """Serialize every Noema trigger type for one pull request.""" +def test_noema_triggers_serialize_actionable_reviews_per_pull_request() -> None: + """Serialize actionable Noema triggers without letting cancelled runs evict them.""" workflow = workflow_text("noema-review.yml") concurrency_contract = workflow.split("permissions:", 1)[0] + assert "github.event_name == 'workflow_run'" in concurrency_contract + assert "github.event.workflow_run.conclusion == 'cancelled'" in concurrency_contract + assert "&& github.run_id" in concurrency_contract assert "github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number" in concurrency_contract assert "github.event.client_payload.pr_number" in concurrency_contract assert "github.event_name }}" not in concurrency_contract