fix(review): require REQUEST_CHANGES lines on current-head hunks - #962
fix(review): require REQUEST_CHANGES lines on current-head hunks#962seonghobae wants to merge 6 commits into
Conversation
GitHub 422s inline comments that sit outside every @@ hunk even when the file changed. The trusted normalizer now fail-closes those findings using a sealed path:line hunk manifest from git diff --unified=0.
|
@cwl-noema-review please review this current head. REQUEST_CHANGES findings must now cite a line that sits on a current-head changed hunk so GitHub can attach the inline comment. |
|
Warning Review limit reached
Next review available in: 15 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
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 |
Treat a present empty opencode-changed-hunk-lines artifact as evidence that no RIGHT-side line is attachable, instead of skipping the hunk gate the same way a missing local artifact does.
A hostile hunk-line row with backticks, HTML metacharacters, or backslashes must not become a trusted current-head hunk line.
A leftover start_line outside the sealed current-head hunk still 422s a multi-line comment even when line is attachable. Reject that leftover range before GitHub sees it.
Materialize treats a lone --require-hashes line and dotted or non-lock -r includes as untrusted so only exact SHA-256 pins or a bounded relative include enter the coverage image.
|
@cwl-noema-review exact current head |
Reject sealed hunk-line manifest rows whose path contains -->, <!--, or a suggestion fence so a leftover cannot close the overview HTML comment or reopen an applyable GitHub suggestion block.
OpenCode Review Overview
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: opencode-review-dispatch.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: opencode-review-dispatch.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (2 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (2 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Docs (2 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (2 files)"]
R3 --> V3["docs review"]
Evidence --> S4["CI script (4 files)"]
S4 --> I4["review and security gate shell path"]
I4 --> R4["Review risk: CI script (4 files)"]
R4 --> V4["bash -n plus Strix self-test"]
Evidence --> S5["Test (5 files)"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test (5 files)"]
R5 --> V5["targeted test run"]
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
APPROVE with evidence at head 68f5906. Changed files inspected as changed-file evidence: .github/workflows/opencode-review-dispatch.yml, scripts/ci/opencode_adversarial_receipts.py, scripts/ci/opencode_review_normalize_output.py, scripts/ci/materialize_base_python_requirements.py, tests/test_opencode_review_normalize_output.py, tests/test_opencode_adversarial_receipts.py, tests/test_materialize_base_python_requirements.py, tests/test_opencode_agent_contract.py, tests/test_required_workflow_queue_contract.py, docs/doctoring/review-hunk-anchored-findings.md. Approval sufficiency: positive evidence supports PR #962 intent (publish REQUEST_CHANGES only on current-head hunk lines) with fail-closed behavior, no failed checks and no unresolved threads at current head. Verification posture: Coverage execution evidence reports Result: PASS — supported repository test suites passed via python3 -m pytest tests; Failed GitHub Check evidence reports no completed failed checks at head 68f5906. Linter/static: no lint findings in evidence; lint contract (hadolint Dockerfile) does not apply to changed files. TDD/regression: hunk-anchoring tests added to tests/test_opencode_review_normalize_output.py (+109) and tests/test_opencode_adversarial_receipts.py (+57); workflow-contract assertions added to tests/test_opencode_agent_contract.py and tests/test_required_workflow_queue_contract.py (+2 each); bounded-relative-include tests extended in tests/test_materialize_base_python_requirements.py (+34). Coverage: Coverage execution evidence Result: PASS — supported repository test suites passed. Docstring coverage: configured repository docstring gates passed or docstring coverage was advisory per Coverage execution evidence. DAG: base-to-head flowchart 'opencode-review-dispatch evidence pipeline' in the review body maps hunk-line generation, sealing, model-pool enforcement, and normalizer rejection. PoC/execution: no OPENCODE_EXECUTION_RECEIPT lines exist in the bounded evidence, so no runtime-tool run is claimed; verification rests on trusted Coverage execution evidence, trusted diff hunks, and the CodeGraph verbatim source trace of materialize_base_python_requirements.py. DDD/domain: CI review-governance domain only; no entity, aggregate, or bounded-context surface changes outside the review pipeline. CDD/context: no context-map changes; the pipeline remains the single review/comment publisher. Similar issues: repo history shows iterative hardening commits (57379bb, d23a4d8, 2864612, 68f5906) for empty manifests, HTML/backtick paths, leftover start_line, and comment-closer paths; no similar open issue in the evidence. Claim/concept check: PR body claim (GitHub HTTP 422 for positive lines outside every @@ hunk; approve gate rejected while normalizer published) is consistent with the added hunk-lines artifact and normalizer enforcement and is documented in docs/doctoring/review-hunk-anchored-findings.md. Standards search: no external standard required; the constraint is the GitHub inline-review API hunk rule described in the PR body. Compatibility/convention: new identifiers OPENCODE_CHANGED_HUNK_LINES_FILE and opencode-changed-hunk-lines.txt are multi-word and follow the existing OPENCODE_* env convention; no reserved-word or single-word naming issues; no schema/API/config objects with externally meaningful names changed. Breaking-change/backcompat: changes are additive (new env var, new --hunk-lines-file arg, new sealed artifact); the normalizer now rejects off-hunk REQUEST_CHANGES, which is the intended strictness change and is regression-tested. Implementation completeness: no placeholder bodies; receipts generation, normalizer validation, materializer include bounds, and tests are concrete and documented. Performance: negligible — one small extra artifact per review run and one extra file digest in the seal step. Developer experience: reviewer REQUEST_CHANGES comments now attach to current-head hunks instead of failing with HTTP 422 (review-comment output surface, judged from workflow diff, docs, and tests). User experience: non-web surface — PR review gate behavior with fail-closed messages on receipt-generation failure. Visual/DOM: non-web CI change; reviewed interaction surface is CLI/workflow/review-comment output, not DOM/ARIA. Accessibility/i18n: no web UI change; review language evidence is English. Supply-chain/license: no new dependencies; uv remains the trusted pinned installer used by the materializer; no manifest or lockfile changes in this PR. Packaging: no new packages; scripts run under the existing pyproject.toml python3 contract (requires-python >=3.10). Security/privacy: no secret-handling changes (existing OPENCODE_APPROVE_TOKEN binding unchanged); materialize_base_python_requirements.py path handling rejects absolute paths and '..' parts per CodeGraph verbatim source, and the hunk-lines artifact is sealed with chmod 0600; no sequential user-facing identifiers introduced.
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including .github/workflows/opencode-review-dispatch.yml, AGENTS.md, CHANGELOG.md, docs/doctoring/review-hunk-anchored-findings.md, docs/doctoring/trusted-uv-lock-materialization.md, and 9 more.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects .github/workflows/opencode-review-dispatch.yml to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source claims require trusted bounded source evidence prepared outside the isolated model process; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: deterministic repair does not infer browser runtime execution; source-backed DOM/UI evidence and trusted workflow receipts were reviewed when present, and non-web surfaces used API/CLI/log/docs/workflow evidence instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
Adversarial validation
{"status":"passed","probes":[{"path":"tests/test_opencode_review_normalize_output.py","line":70,"hypothesis":"REQUEST_CHANGES findings anchored to lines outside every current-head @@ hunk would be published instead of rejected (the HTTP 422 inline-comment failure this PR removes).","attack_or_counterexample":"A finding cites a positive line that exists in a changed file but lies outside the base-to-head diff hunks, exactly the off-hunk case described in the PR body.","evidence":"Coverage execution evidence (trusted workflow, head 68f59063393c8ddf2f32fa2646b3cccdddfdb8a8) reports Result: PASS — supported repository test suites passed via python3 -m pytest tests; the suite includes the hunk-anchoring tests this PR added to tests/test_opencode_review_normalize_output.py (receipt lines 70 and 1774; file +109 lines per diff stat) and completed with zero failures, exercising the hunk-anchored validation with no regression; trusted source trace at tests/test_opencode_review_normalize_output.py:70; source-line-sha256=81b21b47e0e54a30e9ad529be5f2ca0ddc1427775fa011be11605bb78d96e096","outcome":"falsified"},{"path":".github/workflows/opencode-review-dispatch.yml","line":2543,"hypothesis":"Hunk-line receipt-generation failure or a zero-hunk state could seal a missing or empty opencode-changed-hunk-lines.txt and let off-hunk REQUEST_CHANGES publish, or crash the pipeline.","attack_or_counterexample":"opencode_adversarial_receipts.py exits nonzero, or a PR yields no parseable hunks so the hunk-lines file stays empty.","evidence":"Trusted diff (.github/workflows/opencode-review-dispatch.yml, base 6eb06cdd -> head 68f59063) at line 2543 adds the OPENCODE_CHANGED_HUNK_LINES_FILE env binding for the evidence-prep step; the same changed run block pre-seeds that file with the non-empty placeholder '# no current-head hunk lines' before invoking scripts/ci/opencode_adversarial_receipts.py --hunk-lines-file and prints 'Trusted current-head receipt generation failed; approval must fail closed.' on nonzero exit, and the Seal step raises SystemExit('trusted artifact is missing, empty, or outside runner temp') for missing or <=0-byte artifacts, so an empty hunk-lines file cannot be sealed; Coverage execution evidence Result: PASS — python3 -m pytest tests passed at head 68f59063 including workflow-contract tests extended by this PR (tests/test_opencode_agent_contract.py receipt lines 1739-1740, tests/test_required_workflow_queue_contract.py receipt lines 558 and 569), observed zero failures; trusted source trace at .github/workflows/opencode-review-dispatch.yml:2543; source-line-sha256=19a68081a0c8a016fdb58d504b5dab22f5e78200bd3ea990728c7c1d50e8e8f0","outcome":"falsified"}],"residual_risk":"Exotic Git path names (non-UTF8 bytes, very long paths, comment-closer or backtick/HTML names) are mitigated by dedicated head commits d23a4d8a and 68f59063 and their tests, but GitHub inline-comment 422 behavior for such edge paths is unit-tested only, not E2E-tested; a future exotic-path PR could still surface a publishing failure, bounded and fail-closed by the normalizer and seal guards."}- Result: APPROVE
- Reason: Hunk-anchored REQUEST_CHANGES enforcement is implemented end-to-end (hunk-lines artifact, sealed provenance, normalizer validation), covered by passing tests, and fails closed on receipt-generation failure; Coverage execution evidence PASS, no failed GitHub checks, no unresolved threads at head 68f5906.
- Head SHA:
68f59063393c8ddf2f32fa2646b3cccdddfdb8a8 - Workflow run: 31754854943
- Workflow attempt: 1
Pull request was converted to draft
|
Returned to Draft because the current-head hunk-location contract is mixed with the trusted-uv materializer branch and the duplicate #960 platform-test workaround. The valid review-publication slice is bounded: seal the exact RIGHT-side hunk-line artifact, reject off-hunk REQUEST_CHANGES locations before GitHub publication, and require the LLM prompt to cite a current-head changed hunk. Head Rebuild from protected |
1 participant
Summary
GitHub inline review comments attach to the pull-request diff. A positive line that merely exists in a changed file but sits outside every
@@hunk returns HTTP 422. The approve gate already rejected those findings; the trusted Python normalizer still published them.opencode-changed-hunk-lines.txt(path:linefor every RIGHT-side hunk line).finding_hunk_location_error()fail-closesREQUEST_CHANGESwhen that artifact is present and the line is off-hunk.Also includes the same Linux x86_64 installer-test pin as #960 so the local full suite stays at 100% on macOS until that PR lands.
Test plan
pytest tests— 979 passed, 100% statements/branches, interrogate 100%