-
Notifications
You must be signed in to change notification settings - Fork 0
fix(review): persist 422 inline failures as overview receipts #964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
ab69188
fix(review): cite trusted path:line in GitHub 422 inline fallback
seonghobae e099c28
fix(review): persist 422 inline failures as overview receipts
seonghobae 0683a16
test(review): pin 422 fallback sentence in the Python helper
seonghobae 8c3d863
fix(review): do not label a 422 substring as HTTP 422
seonghobae 9a90524
fix(review): escape overview 422 receipt phrases
seonghobae 92dc848
fix(review): omit HTML and backtick finding paths from 422 receipts
seonghobae 6054d78
fix(review): cite leftover GitHub 422 error codes
seonghobae cb8e896
fix(review): retry inline comments one at a time after batch 422
seonghobae 6f6f24b
fix(coverage): accept only bounded relative requirement includes
seonghobae 52685e6
fix(review): sanitize leftover overview receipt path and phrase
seonghobae 1bc7e68
fix(review): omit leftover overview paths with comment closers
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| # Architecture — ContextualWisdomLab `.github` | ||
|
|
||
| This repository is the organization control plane. It is not naruon and it | ||
| does not own product data. Sibling products remain standalone modules; this | ||
| repo publishes org profile assets, reusable required workflows, and the | ||
| review/merge schedulers those products consume. | ||
|
|
||
| ## System context | ||
|
|
||
| ```mermaid | ||
| flowchart LR | ||
| Buyer["Commercial buyer / reviewer"] | ||
| Agents["Agents on AGENTS.md"] | ||
| Project["GitHub Project #1"] | ||
| Hub["This repo: org .github"] | ||
| Products["Owned products<br/>naruon · orchestrator · engines"] | ||
| Runner["Required workflows in each repo context"] | ||
|
|
||
| Buyer --> Hub | ||
| Agents --> Project | ||
| Agents --> Hub | ||
| Project --> Hub | ||
| Hub --> Runner | ||
| Runner --> Products | ||
| Products -->|"standalone or as module"| Buyer | ||
| ``` | ||
|
|
||
| ## 422 receipt phrase gate | ||
|
|
||
| ```mermaid | ||
| flowchart TD | ||
| Err["gh api review write error"] | ||
| Kind{"HTTP 422 line or JSON errors[].message?"} | ||
| Phrase["Receipt: GitHub HTTP 422"] | ||
| Generic["Receipt: GitHub review write failed"] | ||
|
|
||
| Err --> Kind | ||
| Kind -->|"yes"| Phrase | ||
| Kind -->|"no"| Generic | ||
| ``` | ||
|
|
||
| CWE-1288: a bare `422` substring is not an HTTP status. | ||
|
|
||
| ## Control-plane data flow | ||
|
|
||
| ```mermaid | ||
| sequenceDiagram | ||
| participant PR as Pull request | ||
| participant RW as Required workflows | ||
| participant OC as OpenCode reviewer | ||
| participant SV as sandboxed_verify / web E2E | ||
| participant MS as Merge scheduler | ||
|
|
||
| PR->>RW: pull_request_target on trusted base | ||
| RW->>OC: bounded evidence + NVIDIA NIM / OpenCode | ||
| OC->>SV: PoC command in isolated copy | ||
| SV-->>OC: redacted stdout/stderr + command metadata | ||
| OC-->>PR: APPROVE or request changes | ||
| MS->>PR: merge only on current-head approval + green checks | ||
| ``` | ||
|
|
||
| ## Trust boundaries | ||
|
|
||
| - Required review workflows execute **base-branch** scripts. | ||
| - Reviewer agents stay `edit: deny`. | ||
| - Logs redact credential shapes. They do not mask operational PII. | ||
| - LLM and scheduled agents bind `NVIDIA_NIM_API_KEY`. They never use | ||
| `COPILOT_GITHUB_TOKEN`. | ||
| - Rust remains the psychometric arithmetic owner. | ||
|
|
||
| ## Quality gates | ||
|
|
||
| `scripts/ci/` ships with 100% statement/branch coverage and 100% | ||
| docstrings. | ||
|
|
||
| ## Related durable documents | ||
|
|
||
| - [`docs/CWL-MASTER-CONTEXT.md`](docs/CWL-MASTER-CONTEXT.md) | ||
| - [`docs/doctoring/review-inline-comment-422-fallback.md`](docs/doctoring/review-inline-comment-422-fallback.md) | ||
| - [`docs/agent-github-project-protocol.md`](docs/agent-github-project-protocol.md) |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| # GitHub 422 inline-comment fallback cites trusted path:line | ||
|
|
||
| 검토 기준일: **2026-08-13** | ||
|
|
||
| ## Incident | ||
|
|
||
| When GitHub rejects an OpenCode `REQUEST_CHANGES` review because one or more | ||
| inline comments cannot attach, the publisher already falls back to a PR-level | ||
| body and does not copy suggested diffs into that body. The fallback sentence | ||
| said only “the cited finding lines.” Authors then had to open the workflow log | ||
| or control JSON to learn *which* `path:line` GitHub refused (GitHub, n.d.-a, | ||
| n.d.-b). That is weaker than the line-anchored review artifact modern code | ||
| review expects (Bacchelli & Bird, 2013). | ||
|
|
||
| ## Decision | ||
|
|
||
| Leftover overview paths that contain `-->`, `<!--`, or a suggestion fence are omitted so a leftover cannot close the HTML comment. | ||
| Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include; a lone `--require-hashes` line is not lock evidence. | ||
|
|
||
| Leftover overview receipts sanitize path and phrase; a leftover cannot close the HTML comment or reopen a suggestion fence. | ||
|
|
||
| `scripts/ci/opencode_inline_comment_fallback.py` reads the trusted control | ||
| JSON, keeps first-seen safe relative `path` plus positive integer `line` | ||
| pairs, and appends them to the fallback body as `` `path:line` `` list | ||
| items. Unsafe paths (`..`, absolute, drive, backslash) and non-positive | ||
| lines are omitted. An empty location set is stated explicitly. | ||
|
|
||
| After a refused attach, the publisher first checks that the failure is | ||
| HTTP 422 (not a bare `422` substring; CWE-1288), splits the batch | ||
| `comments` array into at most 20 single-comment review payloads | ||
| (`OPENCODE_INLINE_COMMENT_RETRY_LIMIT`, default 20), and retries each | ||
| with the same write helper. Comments past that cap are recorded as not | ||
| retried. Remaining failures still rebuild the fallback from the | ||
| `gh api` error file and writes durable receipts into the OpenCode | ||
| overview comment (`<!-- opencode-review-overview -->`). Each receipt is | ||
| `` `path:line` — GitHub HTTP 422: <phrase> ``. The phrase prefers JSON | ||
| `errors[].message` (for example `pull_request_review_thread.path is | ||
| invalid`) plus leftover `errors[].code` when present, and otherwise | ||
| the first `HTTP 422` line. URLs are stripped and | ||
| the phrase is bounded to 240 characters. Backticks and HTML | ||
| metacharacters are escaped before the phrase is written into the | ||
| overview body. | ||
|
|
||
| The publisher calls this helper from `build_inline_comment_failure_body` | ||
| with the same control object used to build the inline `comments` array. | ||
| Suggested diffs stay out of the PR-level body. | ||
|
|
||
| CWE-1288: the receipt phrase is `GitHub HTTP 422` only from an | ||
| `HTTP 422` line or JSON `errors[].message`. A bare `422` substring | ||
| (commit SHA, issue number) must not be labeled as a GitHub 422. | ||
|
|
||
| ## Verification contract | ||
|
|
||
| - `tests/test_opencode_inline_comment_fallback.py` pins safe-pair extraction, | ||
| the exact location list, GitHub JSON `errors[].message` phrases, HTTP 422 | ||
| line fallback, empty-set sentence, CLI success with `--error-file`, and | ||
| fail-closed unreadable control or error input. | ||
| - `tests/test_opencode_agent_contract.py` and | ||
| `scripts/ci/test_strix_quick_gate.sh` pin the workflow call with | ||
| `$control_json`. | ||
|
|
||
| ## Rollback | ||
|
|
||
| If GitHub later accepts off-diff comments, keep citing the attempted | ||
| `path:line` in the fallback. Do not restore a location-free sentence. | ||
|
|
||
| ## References (APA 7th) | ||
|
|
||
| MITRE. (2026). *CWE-1288: Improper validation of syntactic correctness of | ||
| input*. https://cwe.mitre.org/data/definitions/1288.html | ||
|
|
||
| Bacchelli, A., & Bird, C. (2013). Expectations, outcomes, and challenges of | ||
| modern code review. In *Proceedings of the 35th International Conference on | ||
| Software Engineering* (pp. 712–721). IEEE. | ||
| https://doi.org/10.1109/ICSE.2013.6606617 | ||
|
|
||
| GitHub. (n.d.-a). *Create a review for a pull request*. GitHub Docs. Retrieved | ||
| August 13, 2026, from | ||
| https://docs.github.com/en/rest/pulls/reviews#create-a-review-for-a-pull-request | ||
|
|
||
| GitHub. (n.d.-b). *Create a review comment for a pull request*. GitHub Docs. | ||
| Retrieved August 13, 2026, from | ||
| https://docs.github.com/en/rest/pulls/comments#create-a-review-comment-for-a-pull-request |
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
모든 호출 경로에
control_json을 전달해야 합니다.Line 5775에서
control_json이 필수 인자가 되었습니다. 그러나run_failed_check_diagnosis()는 Line 6382에서 인자 두 개만 전달합니다. 이 호출은--control ""을 실행하고 helper를 실패시킵니다.그 결과 failed-check diagnosis 경로는 생성한 inline payload를 게시하지 못하고 일반 fallback으로 내려갑니다. 해당 호출에
"$control_json"을 전달하세요. 이후create_pull_review_with_payload호출에도 원본 body와 control JSON을 전달하여 422 error phrase를 다시 생성할 수 있게 하세요.🤖 Prompt for AI Agents