Skip to content

fix(review): require line-anchored current-head REQUEST_CHANGES findings - #959

Draft
seonghobae wants to merge 5 commits into
mainfrom
feat/review-line-anchored-findings
Draft

fix(review): require line-anchored current-head REQUEST_CHANGES findings#959
seonghobae wants to merge 5 commits into
mainfrom
feat/review-line-anchored-findings

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Buyer-visible gap

OpenCode already emits inline REQUEST_CHANGES findings, but the trusted normalizer accepted arbitrary non-empty paths and positive line numbers. GitHub can then reject unchanged paths or past-EOF lines with HTTP 422, leaving blockers absent from Files changed.

This Draft proposes:

  • a non-empty path and positive non-boolean line contract;
  • exact membership in the trusted current-head changed-file set when available;
  • containment in the trusted current-head source root;
  • regular-file, bounded-size, and real-line verification;
  • matching prompt/model-pool instructions and focused doctoring.

Exact identity and bounded scope

  • protected base and live tip: main@6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba;
  • exact current head: a3b93b70b84a679f39faf32aa5e86adb4eaa97ac;
  • state: Draft and mechanically mergeable;
  • bounded current diff: normalizer, prompt/model-pool contract, focused tests/doctoring, and CHANGELOG.md.

Unrelated central AGENTS/CLAUDE/Architecture and trusted-lock materializer production/test changes entered the branch. The current ordinary forward commit restores those paths to protected-main content. No force-push, rebase, history rewrite, predecessor evidence transfer, or gate weakening was used.

Current blocking correctness findings

This Draft is not ready for integration.

  1. Canonical path mismatch: validation calls finding_location_error(str(finding["path"]).strip(), line) but appends the original finding object unchanged. A model path such as " scripts/ci/example.py " can be validated against the stripped current-head path and then published with whitespace, still risking GitHub HTTP 422. The normalized finding must carry the exact canonical path that was validated, or non-canonical path text must be rejected.
  2. Missing changed-file evidence is permissive: finding_location_error() checks membership only when current_changed_files() is non-empty. If the trusted changed-file artifact is absent/unsafe while the manifest and source root remain available, a REQUEST_CHANGES finding on an unchanged source path can pass the membership gate. For inline current-head findings, missing changed-file evidence should fail closed unless a separately reviewed contract proves an intentional no-diff mode.
  3. Path identity must remain byte/Unicode safe: any canonicalization must not Unicode-normalize two distinct Git paths into the same identity, resolve symlink aliases as changed-file identity, or silently rewrite path separators.

Required TDD repair

Before Ready:

  • commit RED regressions for whitespace-padded finding paths and absent/unsafe changed-file artifacts;
  • either reject non-canonical paths or replace finding["path"] with the exact validated canonical value before publication;
  • require a trusted changed-file artifact for non-empty REQUEST_CHANGES findings;
  • preserve source-root containment, real-line/EOF, regular-file, size, and symlink protections;
  • rerun focused and full tests with exact 100% owned production statement/branch/docstring evidence;
  • complete current-head security, SAST, dependency/SBOM, supply-chain, and OpenCode/Noema/CodeRabbit review.

Do not mark Ready or enable auto-merge while these blockers remain. No administrative bypass, self-approval, or relaxation of line anchoring is requested.

@seonghobae
fix(review): require line-anchored current-head REQUEST_CHANGES findings
ca04609 
GitHub rejects inline review comments on unchanged paths or past-EOF
lines with HTTP 422. Fail-close those findings in the trusted
normalizer so blockers attach on Files changed.
@seonghobae
seonghobae enabled auto-merge (squash) August 13, 2026 06:35
@seonghobae

seonghobae commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review please review this current head. REQUEST_CHANGES findings must now name an exact current-head changed file and a line that exists in that file so GitHub can attach the inline comment.

@coderabbitai

coderabbitai Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 71e52f32-481f-4b0f-bf87-fb8c0e703b70

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

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

@seonghobae
docs(review): cite CWE-1288 for line-anchored findings
972c420 
Record that a REQUEST_CHANGES path and line must be consistent with the
trusted current-head artifact. Force the trusted-uv installer tests onto
the linux x86_64 runner path and add the control-plane architecture
diagram.
@seonghobae
fix(review): reject line 0 and True as finding anchors
71cf52e 
finding_location_error deferred line-shape checks to the caller. Line 0
passes the EOF probe because 0 > line_count is false, so a current-head
path could be treated as anchored. Reject bool and non-positive lines
inside the helper.
@seonghobae
fix(coverage): accept only bounded relative requirement includes
6736734 
Materialize a base Python lock only when every package line is an exact
SHA-256 pin or a two-token relative -r/--requirement include of a
candidate lock path. A lone --require-hashes directive, ./dotted paths,
and -r other-hashes.txt no longer enter the trusted build context.
opencode-agent[bot]
opencode-agent Bot requested changes Aug 14, 2026
View reviewed changes

@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 could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

  • Problem: Failed same-head checks remain for 67367344cba0be95c81832187436f85c31d65c9a.
  • Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
  • Fix: Read and fix the failed check logs below, then rerun the current-head checks.
  • Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (4 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (4 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Docs: review-line-anchored-findings.md"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs: review-line-anchored-findings.md"]
  R2 --> V2["docs review"]
  Evidence --> S3["CI script (4 files)"]
  S3 --> I3["review and security gate shell path"]
  I3 --> R3["Review risk: CI script (4 files)"]
  R3 --> V3["bash -n plus Strix self-test"]
  Evidence --> S4["Test (3 files)"]
  S4 --> I4["regression suite"]
  I4 --> R4["Review risk: Test (3 files)"]
  R4 --> V4["targeted test run"]
Loading

@opencode-agent

opencode-agent Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 67367344cba0be95c81832187436f85c31d65c9a
  • Workflow run: 31754331454
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

  • Problem: Failed same-head checks remain for 67367344cba0be95c81832187436f85c31d65c9a.
  • Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
  • Fix: Read and fix the failed check logs below, then rerun the current-head checks.
  • Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (4 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (4 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Docs: review-line-anchored-findings.md"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs: review-line-anchored-findings.md"]
  R2 --> V2["docs review"]
  Evidence --> S3["CI script (4 files)"]
  S3 --> I3["review and security gate shell path"]
  I3 --> R3["Review risk: CI script (4 files)"]
  R3 --> V3["bash -n plus Strix self-test"]
  Evidence --> S4["Test (3 files)"]
  S4 --> I4["regression suite"]
  I4 --> R4["Review risk: Test (3 files)"]
  R4 --> V4["targeted test run"]
Loading

@opencode-agent
opencode-agent Bot disabled auto-merge August 14, 2026 01:29
@seonghobae
seonghobae marked this pull request as draft August 14, 2026 09:11
@seonghobae ChatGPT Codex Connector

seonghobae commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Returned to Draft because the exact current tree mixes the line-anchored REQUEST_CHANGES contract with an unrelated trusted-uv materializer branch.

The valid publication boundary is bounded: each blocking finding must target a non-empty exact current-head changed path and a line that exists in that current-head file; invalid locations must be rejected before GitHub receives the review. Head 67367344cba0be95c81832187436f85c31d65c9a additionally changes materialize_base_python_requirements.py and its tests plus broad governance files.

Rebuild from protected main retaining only the normalizer, prompt/model-pool contract text, focused current-head/path/EOF regressions, doctoring, directly attributable governance/changelog evidence, and no materializer changes. Coordinate with #946/#958/#998 so inline publication, per-file disposition, and bounded fallback have one explicit dependency order, then reacquire exact-head quality/security/supply-chain and independent-review evidence.

@seonghobae seonghobae mentioned this pull request Aug 14, 2026
Draft
2 tasks
@seonghobae ChatGPT Codex Connector

seonghobae commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Draft blockers at exact head a3b93b70b84a679f39faf32aa5e86adb4eaa97ac:

  1. finding_location_error(str(finding["path"]).strip(), line) validates a stripped path but normalized_findings.append(finding) publishes the original value. A whitespace-padded path can pass current-head/EOF validation and still be rejected by GitHub inline-review publication.
  2. Changed-file membership is checked only when current_changed_files() is non-empty. Missing or unsafe trusted changed-file evidence can therefore permit a finding on an unchanged source path.

Add RED regressions, then reject or persist the exact validated path and fail closed when changed-file evidence is unavailable for non-empty REQUEST_CHANGES findings. Preserve byte/Unicode Git path identity, source-root containment, symlink, regular-file, size, and EOF boundaries. Keep Draft until focused/full exact coverage and current-head reviews pass.

@opencode-agent review
@cwl-noema-review review
@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown

Rate Limit Exceeded

@seonghobae have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 50 seconds before sending another message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@opencode-agent opencode-agent[bot] opencode-agent[bot] requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@seonghobae