chore(ci): remove orphaned required-workflow-bootstrap job from dispatch workflow - #1489
chore(ci): remove orphaned required-workflow-bootstrap job from dispatch workflow#1489seonghobae wants to merge 17 commits into
Conversation
…tch workflow The required-workflow-bootstrap job in opencode-review-dispatch.yml only echoed a materialization message and was never referenced by any needs: clause in the file. It looks copy-pasted from the real trust-boundary bootstrap job in the sibling pull_request_target-triggered opencode-review.yml, but this workflow fires only on repository_dispatch (an already-trusted, non-PR context) and is not itself a path the org required-workflow ruleset targets, so that pattern's reason for existing does not apply here. Update the two byte-for-byte blob-hash pins on this workflow file (tests/test_pr_review_autofix_nvidia_nim_contract.py, tests/test_opencode_rust_coverage_toolchain_contract.py reads the same constant) and the contract test that had pinned the orphaned job's presence, replacing it with a regression guard against its reintroduction. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
|
Warning Review limit reachedNext included review available in 54 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (7)
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. Comment |
Resolve conflicts from concurrent hollow-path audit PRs that merged to main since this PR was opened. # Conflicts: # CHANGELOG.md
Ports the identical fix from #1506 into this branch. This PR's exact-head-path-policy check runs its own head-branch copy of scripts/ci/test_strix_quick_gate.sh (plain `pull_request` trigger in strix-changed-path-quality-ci.yml, not pull_request_target), so the pre-existing main-branch bug is not fixed here just by #1506 merging into main -- it needs porting into this branch directly. Root cause: assert_opencode_review_uses_codegraph_and_contextual_orchestrator extracted the required-workflow-bootstrap job block from opencode-review.yml with awk '/^ required-workflow-bootstrap:$/,/^[^ ]/'. Every job key in that workflow is indented 2 spaces (never column 0), so the end pattern never matched until EOF, sweeping an unrelated `if:` line from a later job (added by already-merged PR #1497) into the "block" and failing the assertion on unrelated content. Fixed by using an explicit state flag so the end pattern (`^ [A-Za-z0-9_-]+:`) is only tested starting on the line after the start match, correctly bounding the block to just its own lines. See #1506 for the full root-cause writeup and validation against origin/main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
|
grep -q exits on first match and closes its end of the pipe; if the upstream awk is still writing a large block, it gets SIGPIPE (141). Under `set -o pipefail` that non-zero awk status wins over grep's real 0, so `if pipeline; then` sees the pipeline as failed even though grep found a genuine match — silently missing e.g. a forbidden `if:` key or a fenced-diff marker that should have failed the check. Ports the same-file fix from PR #1506 to this branch's two call sites (required-workflow-bootstrap job-block check; opencode review REQUEST_CHANGES fenced-diff check). This branch already carried #1506's awk job-block-boundary correction, so only the grep -q removal was needed here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
|
Contextual-Orchestrator와 관계한 것들을 같이 손보든 어쩌든 해결하세요. Bypass merge 필요하면 가능 (chicken and eggs 상황이라면) + NVIDIA NIM 만 쓰는 건 허용하지 않아요. Contextual-Orchestrator를 쓰세요. Timeout은 적어도 3시간으로 잡으세요. 120초 같은 건 당황스럽군요. Opencode와 Noema 는 Coderabbitai 및 Devin 수준으로 실제로 리뷰를 하게 하시오. Strix도 보안 리뷰를 꼼꼼하게 하도록 하시오. 특히 보안 리뷰는 전체 코드로 수행하는 것입니다. Contextual-Orchestrator는 실시간으로 빠르면서 능력이 좋은 모델에 요청을 보내어 시간을 당기시오. |
Resolves the merge conflict in scripts/ci/test_strix_quick_gate.sh by keeping main's required_workflow_bootstrap_has_if() helper, which fixes both the awk job-block boundary bug and the grep -q/pipefail SIGPIPE false-negative bug (superseding this branch's inline awk|grep -q form). main's #1533 (repository_dispatch head_sha warn-and-proceed instead of hard-fail) and this PR's own required-workflow-bootstrap job removal both land cleanly in opencode-review-dispatch.yml. Two follow-on fixes were needed to match the merged file's actual content, since the repair already applied to main is not itself merged there yet: - REVIEW_DISPATCH_BLOB_SHA in test_pr_review_autofix_nvidia_nim_contract.py updated to the merged file's actual blob hash (git hash-object), which is unique to this branch (job removal + #1533 combined). - test_opencode_privileged_review_security_boundaries_are_fail_closed (test_opencode_agent_contract.py) updated to pin #1533's warn-and-proceed head_sha invariant instead of the pre-#1533 hard-fail assertion: base_ref/base_sha/head_ref stay hard-fail exact matches, head_sha is explicitly excluded from the mismatches set, and the warning path is asserted present. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
|
Current-main revalidation confirms this hollow path still exists on protected |
…ode-dispatch-bootstrap # Conflicts: # tests/test_opencode_agent_contract.py # tests/test_pr_review_autofix_nvidia_nim_contract.py
Protected main regressed to 99% scripts/ci coverage after #1546 added live_head_matches, a no-active/no-stale fall-through in prepare_autofix_slot, and an "already queued or running" wait branch to pr_review_fix_scheduler.py without covering them, while the pre-existing inspect_pr conflicted-draft/conflicted-unauthorized returns and pr_review_merge_scheduler.py's fetch_workflow_names_by_check_suite_rest pagination/filtering/ permission-denied paths stayed untested. Every PR rebasing onto main inherits this via the coverage-evidence required check regardless of its own diff. Test-only change; no production code touched.
Adds a dated traceability entry for the coverage gap this PR closes: root cause (#1546's uncovered additions plus the older #1547/#1551/ #1554 gap, neither of which merged or transfers evidence here), the fix and its verification, the resolved Devin false-positive on sub-clause coverage, and the known pre-existing SIGPIPE test flake left unremediated as out of scope.
Raise scoped docstring coverage for the newly added scheduler REST regression helpers to 100% without changing test behavior or production code.
|
Generated by Claude Code Generated by Claude Code |
|
This hollow-path removal is still required on protected |
…ode-dispatch-bootstrap # Conflicts: # tests/test_pr_review_autofix_nvidia_nim_contract.py
…rest-and-live-head' into fix/hollow-path-opencode-dispatch-bootstrap
|
Converged on current main ( Also merged Validated on the new head ( @opencode-agent please review the new exact head Generated by Claude Code Generated by Claude Code |
RCA: the #1567 exact-head Hourly NVIDIA NIM Review Repair run failed in test_scheduler_wake_reuses_trusted_receipt_predicate with exit 141. The production block pipes jq JSON into gh api --input -, while the test fake exited without reading stdin. Under pipefail that can SIGPIPE jq. Reuse the already RED/GREEN-verified #1569 fixture blob and drain stdin before recording the fake dispatch. This makes #1567 self-contained so the central 100% coverage repair no longer depends on a separate PR that itself inherits the coverage failure.
|
The repeated "Hourly cadence, immutable source, NIM credential, and conflict scope" failures on this head were the Stress-tested in isolation (20/20 clean) and validated on the new head ( Generated by Claude Code Generated by Claude Code |
…ode-dispatch-bootstrap # Conflicts: # CHANGELOG.md # docs/product-technical-gap-baseline.md
# Conflicts: # CHANGELOG.md
Merging current main pulled in #1587 (free-pool admission filtering) and #1564 (merge-base-anchored deleted-file review evidence), both of which left pre-existing tests referencing removed/renamed names or excluded fixtures broken. Port the same fix already opened as its own dedicated PR (#1598) rather than widening this PR's own scope: - tests/test_contextual_orchestrator_review_policy.py: swap the stale "openai" free-pool fixture to "bytez" in test_build_catalog_applies_account_cap and test_build_catalog_respects_limit. - tests/test_noema_review_gate.py: rename fetch_changed_file_paths call sites to fetch_changed_files with (path, status) tuples; accept the new changed_files parameter in build_review_context mocks; drop the two CodeGraph-only assertions/tests for the removed function. - tests/test_noema_removed_file_context.py: rewrite against the real run() JSON-per-line contract, fetch_merge_base_sha's SHA validation, and fetch_file_content_at_ref; add direct coverage for the malformed-input and empty-content branches #1564 introduced. Full suite: 2318 passed, 100% branch coverage, 100% docstrings.
…ee-pool fixture fix) # Conflicts: # tests/test_contextual_orchestrator_review_policy.py
Summary
Fixes one confirmed hollow-path finding from the org-wide audit ("빈 깡통 경로" sweep).
Finding:
required-workflow-bootstrapjob (lines 27-31) in.github/workflows/opencode-review-dispatch.ymlonly echoed"OpenCode repository-dispatch review run materialized."and was neverreferenced by any
needs:clause anywhere in the file (verified: the onlyother
needs:clauses in the file are[validate-pr-metadata],[validate-pr-metadata, coverage-source-tree], and[validate-pr-metadata, coverage-evidence]).Independent re-verification before touching anything:
repository_dispatch— analready-trusted, non-PR context — unlike its sibling
opencode-review.yml, which triggers onpull_request_targetand whosematching
required-workflow-bootstrapjob (still present, still needed)resolves and pins the trusted workflow source before any real work runs
(
needs: [required-workflow-bootstrap]at line 217 there).docs/org-required-workflow-rollout.md, the org ruleset's activerequired workflow paths list
opencode-review.yml, notopencode-review-dispatch.yml— the stable branch-protection job names(
required-workflow-bootstrap,coverage-source-tree,coverage-evidence,opencode-review) belong to that sibling file. Thedispatch workflow is invoked downstream of it via
repository_dispatchand is not itself a ruleset-gated entrypoint.
required-workflow-bootstrapstring also appears later in this file(in a jq
self_check_filterlist used to exclude the review pipeline'sown check-run names from "peer checks" polling) — that reference is to
the check-run name shared with the sibling workflow's job, not a
needs:dependency on this local job, and is untouched by this change.
(
tests/test_opencode_agent_contract.py::test_opencode_target_coverage_materializes_only_after_authorized_dispatch),which is itself part of the hollow-path artifact (a copy-paste that
codified dead code). Updated it into a regression guard against
reintroduction instead.
Confirmed real; removed the job.
Changes
.github/workflows/opencode-review-dispatch.yml: delete the orphanedrequired-workflow-bootstrapjob (holds no required-check slot, gatesnothing).
tests/test_opencode_agent_contract.py: replace the stalepositive-presence assertions/slicing for that job with
assert "required-workflow-bootstrap:" not in workflow.tests/test_pr_review_autofix_nvidia_nim_contract.py: update thebyte-for-byte
REVIEW_DISPATCH_BLOB_SHApin on this workflow file to itsnew git blob hash (also read by
tests/test_opencode_rust_coverage_toolchain_contract.py).CHANGELOG.md:[Unreleased]entry.Test plan
All run from a clean isolated clone against this branch:
coverage run -m pytest tests -q && coverage report --show-missing— 2105 passed, 1 skipped, 21 subtests passed;scripts/cistays at 100% coverage (untouched by this change).interrogate—RESULT: PASSED (minimum: 100.0%, actual: 100.0%).bash scripts/ci/test_strix_quick_gate.sh—test_strix_quick_gate: PASS.python3 -c "yaml.safe_load(...)"on the edited workflow — parses, job list no longer containsrequired-workflow-bootstrap.🤖 Generated with Claude Code
Generated by Claude Code