From fc223ea19f912aa72ede4a5b08a74ccfc3a89b17 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 17 Aug 2026 07:24:35 +0900 Subject: [PATCH 1/2] fix(review): require every current-head file in OpenCode APPROVE Treat sibling prefixes such as example.py.bak as a different path token so an approval cannot skip unnamed current-head files. --- AGENTS.md | 1 + ARCHITECTURE.md | 6 + CHANGELOG.md | 1 + CLAUDE.md | 2 + .../review-contract-per-file-disposition.md | 68 +++++++ .../ci/opencode_review_normalize_output.py | 78 +++++++- .../test_opencode_review_normalize_output.py | 180 +++++++++++++++++- 7 files changed, 331 insertions(+), 5 deletions(-) create mode 100644 docs/doctoring/review-contract-per-file-disposition.md diff --git a/AGENTS.md b/AGENTS.md index 16f0981c0..494cfa773 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,3 +5,4 @@ Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include (no `.`/`..`); a lone `--require-hashes` directive is not trust evidence. See [`docs/doctoring/hourly-nvidia-nim-autofix.md`](docs/doctoring/hourly-nvidia-nim-autofix.md). Conflict-scope roots fail closed when the immediate parent directory is a symbolic link. +OpenCode APPROVE must name every current-head changed file as a whole path token. See [`docs/doctoring/review-contract-per-file-disposition.md`](docs/doctoring/review-contract-per-file-disposition.md). diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 6fe6621b6..3deaa6218 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -25,6 +25,12 @@ flowchart LR Products -->|"standalone or as module"| Buyer ``` +## Per-file OpenCode APPROVE + +`valid_control` rejects APPROVE unless every current-head changed path +appears as a whole token. A longer sibling (`example.py.bak`) is not a +disposition of `example.py`. + ## Hourly NVIDIA NIM repair gate ```mermaid diff --git a/CHANGELOG.md b/CHANGELOG.md index 1de9130a5..6920ba441 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Require OpenCode APPROVE to name every current-head changed file as a whole path token so a sibling such as `example.py.bak` cannot stand in for `example.py`. - Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. A lone `--require-hashes` directive, a dotted include such as `./lock.txt`, or `-r other-hashes.txt` no longer enters the trusted build context. - Refused a conflict-scope repository root whose immediate parent is a symbolic link, so a swapped parent cannot redirect the canonical worktree after the last-component check (CWE-367). - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. diff --git a/CLAUDE.md b/CLAUDE.md index 7127d3c1c..1fdfd49a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -114,6 +114,8 @@ repeatable compile command. without running the test suite will break CI. - **100% coverage and 100% docstrings on `scripts/ci/`** are hard gates, not aspirations. New helper code needs matching tests and docstrings. +- **OpenCode APPROVE** must name every current-head changed file as a whole path token. + Prefix siblings such as `example.py.bak` do not count. - **`pull_request_target` trust boundary.** The required review workflows run the *base branch's* trusted scripts. A PR that edits the trusted review workflows can fail its own checks until the base branch catches up; a same-head manual `workflow_dispatch` Strix run may supply review evidence diff --git a/docs/doctoring/review-contract-per-file-disposition.md b/docs/doctoring/review-contract-per-file-disposition.md new file mode 100644 index 000000000..f73b84061 --- /dev/null +++ b/docs/doctoring/review-contract-per-file-disposition.md @@ -0,0 +1,68 @@ +# OpenCode approval must name every current-head changed file + +검토 기준일: **2026-08-13** + +## Incident + +The OpenCode normalizer accepted an APPROVE when the reason/summary named +*one* current-head path. CodeRabbit-style reviews walk every changed file. +Buyers comparing the two saw OpenCode approve after a single path citation +while leaving the rest of the diff unnamed. That is the “less detailed than +CodeRabbit” gap. + +## Decision + +`unnamed_changed_files(reason, summary)` returns every path from +`current_changed_files()` that is not named as a whole path token. +Materialize accepts only exact SHA-256 pins or a bounded relative `-r` +include; a lone `--require-hashes` line is not lock evidence. +A longer sibling such as ``example.py.bak`` contains ``example.py`` as a +prefix substring; that is not a disposition of the shorter file +(CWE-1288; MITRE, 2026). ``path:line`` still counts. `valid_control` +rejects APPROVE when that tuple is non-empty, both before and after +bounded-evidence repair. Developer experience / User experience +section labels were already required; this change only closes the file-walk +hole. + +An empty changed-file set remains a no-op (`()`), so identity-only PRs are +unchanged. + +IEEE 1028 requires every software product in the review package to +receive a recorded disposition (IEEE, 2008). Naming one path therefore +cannot authorize APPROVE when the trusted artifact lists more files. + +## Trust boundary + +Changed-file identity still comes from the trusted workflow artifact, not +from model prose. The model cannot invent a smaller file set to pass the +walk. + +## Verification contract + +`test_approval_must_name_every_current_head_changed_file` uses a two-file +current-head list (`scripts/ci/example.py` and `.github/workflows/strix.yml`). +Naming only the first file is rejected. Naming both is accepted. A post-repair +reason that drops the second path is still rejected. Naming only +``scripts/ci/example.py.bak`` still leaves ``scripts/ci/example.py`` unnamed. + +## Rollback + +Remove the `unnamed_changed_files` checks from the two APPROVE blocks and +the helper. Existing “at least one file” detection remains. + +## References (APA 7th) + +MITRE. (2026). *CWE-1288: Improper validation of consistency within input*. +https://cwe.mitre.org/data/definitions/1288.html + +IEEE. (2008). *IEEE standard for software reviews and audits* (IEEE Std +1028-2008). https://doi.org/10.1109/IEEESTD.2008.4601584 + +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.). *About pull request reviews*. GitHub Docs. Retrieved +August 13, 2026, from +https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/about-pull-request-reviews diff --git a/scripts/ci/opencode_review_normalize_output.py b/scripts/ci/opencode_review_normalize_output.py index 4045d457c..1bee7ef11 100755 --- a/scripts/ci/opencode_review_normalize_output.py +++ b/scripts/ci/opencode_review_normalize_output.py @@ -854,13 +854,77 @@ def contradicts_material_changed_file_scope(reason: str, summary: str) -> bool: return any(phrase in combined for phrase in MATERIAL_CHANGE_FALSE_PHRASES) +_PATH_TOKEN_BODY = frozenset( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789._-+/" +) + + +def _path_token_continues(text: str, index: int) -> bool: + """Return True when ``text[index]`` extends a repository path token.""" + if index < 0 or index >= len(text): + return False + char = text[index] + if char in "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-+/": + return True + if char == "." and index + 1 < len(text) and text[index + 1] in _PATH_TOKEN_BODY: + return True + return False + + +def changed_file_named_in_text(text: str, path: str) -> bool: + """Return True when ``path`` appears as a whole path token in ``text``. + + A longer sibling such as ``example.py.bak`` contains ``example.py`` as a + prefix substring. That is not a disposition of the shorter file. A + sentence period after ``.yml`` is not a continuation. + """ + + if not path or not text: + return False + start = 0 + while True: + index = text.find(path, start) + if index < 0: + return False + after = index + len(path) + if not _path_token_continues(text, after) and not _path_token_continues( + text, index - 1 + ): + return True + start = index + 1 + + def mentions_actual_changed_file(reason: str, summary: str) -> bool: """Return whether an approval names an exact current-head changed file.""" changed_files = current_changed_files() if not changed_files: return False combined = f"{reason}\n{summary}" - return any(changed_file in combined for changed_file in changed_files) + return any( + changed_file_named_in_text(combined, changed_file) + for changed_file in changed_files + ) + + +def unnamed_changed_files(reason: str, summary: str) -> tuple[str, ...]: + """Return current-head changed files the approval never names. + + Naming one file is not a file-by-file walk. Live OpenCode approvals that + cited a single path while leaving the rest unnamed were thinner than the + CodeRabbit per-file contract the buyer asked for. + """ + + changed_files = current_changed_files() + if not changed_files: + return () + combined = f"{reason}\n{summary}" + return tuple( + sorted( + path + for path in changed_files + if not changed_file_named_in_text(combined, path) + ) + ) def mentions_verification_posture(reason: str, summary: str) -> bool: @@ -1289,6 +1353,12 @@ def reject(reason: str) -> None: return reject("approval admits missing structural review") if not mentions_actual_changed_file(reason, summary): return reject("approval does not cite changed-file evidence") + unnamed = unnamed_changed_files(reason, summary) + if unnamed: + return reject( + "approval does not name every current-head changed file: " + + ", ".join(unnamed) + ) if not mentions_verification_posture(reason, summary): return reject("approval does not include the required verification posture") if not mentions_full_coverage(reason, summary): @@ -1311,6 +1381,12 @@ def reject(reason: str) -> None: return reject("review prose does not follow the preferred PR language") if not mentions_actual_changed_file(reason, summary): return reject("approval does not cite changed-file evidence") + unnamed = unnamed_changed_files(reason, summary) + if unnamed: + return reject( + "approval does not name every current-head changed file: " + + ", ".join(unnamed) + ) if not mentions_verification_posture(reason, summary): return reject("approval does not include the required verification posture") if not mentions_full_coverage(reason, summary): diff --git a/tests/test_opencode_review_normalize_output.py b/tests/test_opencode_review_normalize_output.py index 590fb3e53..c9606a35e 100644 --- a/tests/test_opencode_review_normalize_output.py +++ b/tests/test_opencode_review_normalize_output.py @@ -856,6 +856,173 @@ def test_changed_file_and_verification_posture_detection(): ) +def test_approval_must_name_every_current_head_changed_file(tmp_path, monkeypatch): + """A CodeRabbit-thin approval that names one of two files cannot pass.""" + + changed_files = tmp_path / "opencode-changed-files.txt" + changed_files.write_text( + "scripts/ci/example.py\n.github/workflows/strix.yml\n", + encoding="utf-8", + ) + monkeypatch.setenv("OPENCODE_CHANGED_FILES_FILE", str(changed_files)) + seal_artifacts(tmp_path, changed_files) + norm.current_changed_files.cache_clear() + + assert norm.unnamed_changed_files( + "Reviewed scripts/ci/example.py.", + FULL_SUMMARY, + ) == (".github/workflows/strix.yml",) + assert ( + norm.unnamed_changed_files( + "Reviewed scripts/ci/example.py and .github/workflows/strix.yml.", + FULL_SUMMARY, + ) + == () + ) + assert ( + norm.unnamed_changed_files( + "Reviewed scripts/ci/example.py.bak only.", + "Coverage: 100%.", + ) + == (".github/workflows/strix.yml", "scripts/ci/example.py") + ) + assert norm.changed_file_named_in_text( + "Reviewed scripts/ci/example.py:7.", "scripts/ci/example.py" + ) + assert not norm.changed_file_named_in_text( + "Reviewed scripts/ci/example.py.bak.", "scripts/ci/example.py" + ) + assert not norm.changed_file_named_in_text("", "scripts/ci/example.py") + assert not norm.changed_file_named_in_text("scripts/ci/example.py", "") + assert norm._path_token_continues("scripts/ci/example.py.bak", 22) + assert not norm._path_token_continues("scripts/ci/example.py.", 22) + assert not norm._path_token_continues("x", -1) + assert not norm._path_token_continues("x", 1) + + reasons: list[str] = [] + assert ( + norm.valid_control( + control(reason="Reviewed scripts/ci/example.py only."), + expected_head_sha="head", + expected_run_id="run", + expected_run_attempt="attempt", + rejection_reasons=reasons, + ) + is None + ) + assert any( + "does not name every current-head changed file" in reason + and ".github/workflows/strix.yml" in reason + for reason in reasons + ) + + reasons.clear() + accepted = norm.valid_control( + control( + reason=( + "Reviewed scripts/ci/example.py and .github/workflows/strix.yml " + "on the current head." + ) + ), + expected_head_sha="head", + expected_run_id="run", + expected_run_attempt="attempt", + rejection_reasons=reasons, + ) + assert accepted is not None + assert accepted["result"] == "APPROVE" + + monkeypatch.delenv("OPENCODE_CHANGED_FILES_FILE", raising=False) + norm.current_changed_files.cache_clear() + assert norm.unnamed_changed_files("any reason", "any summary") == () + + monkeypatch.setenv("OPENCODE_CHANGED_FILES_FILE", str(changed_files)) + seal_artifacts(tmp_path, changed_files) + norm.current_changed_files.cache_clear() + monkeypatch.setattr(norm, "repair_approval_summary", lambda reason, summary: summary) + monkeypatch.setattr( + norm, + "repair_approval_reason", + lambda reason, summary: reason.replace(".github/workflows/strix.yml", ""), + ) + reasons.clear() + assert ( + norm.valid_control( + control( + reason=( + "Reviewed scripts/ci/example.py and .github/workflows/strix.yml " + "on the current head." + ) + ), + expected_head_sha="head", + expected_run_id="run", + expected_run_attempt="attempt", + rejection_reasons=reasons, + ) + is None + ) + assert any( + "does not name every current-head changed file" in reason + for reason in reasons + ) + + +def test_valid_control_rejects_kind_contradiction_after_naming_every_file( + tmp_path, monkeypatch +): + """Naming every path still fails when the approval denies source-file kinds.""" + + changed_files = tmp_path / "opencode-changed-files.txt" + changed_files.write_text("scripts/ci/example.py\n", encoding="utf-8") + monkeypatch.setenv("OPENCODE_CHANGED_FILES_FILE", str(changed_files)) + seal_artifacts(tmp_path, changed_files) + norm.current_changed_files.cache_clear() + + reasons: list[str] = [] + assert ( + norm.valid_control( + control( + reason="Reviewed scripts/ci/example.py with no source changes.", + summary=FULL_SUMMARY, + ), + expected_head_sha="head", + expected_run_id="run", + expected_run_attempt="attempt", + rejection_reasons=reasons, + ) + is None + ) + assert any("contradicts changed file kinds" in reason for reason in reasons) + + +def test_valid_control_rejects_material_trivialization_after_naming_every_file( + tmp_path, monkeypatch +): + """Naming every path still fails when the approval calls the change a string typo.""" + + changed_files = tmp_path / "opencode-changed-files.txt" + changed_files.write_text("scripts/ci/example.py\n", encoding="utf-8") + monkeypatch.setenv("OPENCODE_CHANGED_FILES_FILE", str(changed_files)) + seal_artifacts(tmp_path, changed_files) + norm.current_changed_files.cache_clear() + + reasons: list[str] = [] + assert ( + norm.valid_control( + control( + reason="Reviewed scripts/ci/example.py; no tests are needed.", + summary=FULL_SUMMARY, + ), + expected_head_sha="head", + expected_run_id="run", + expected_run_attempt="attempt", + rejection_reasons=reasons, + ) + is None + ) + assert any("trivializes material changed files" in reason for reason in reasons) + + def test_actual_changed_file_detection_prefers_current_head_file_list( tmp_path, monkeypatch ): @@ -1819,8 +1986,8 @@ def test_valid_control_accepts_model_confirms_with_bounded_current_head_receipt( candidate = control( reason=( - "src/main/java/example/LogSanitizer.java hardens log input and adds " - "a regression test." + "src/main/java/example/LogSanitizer.java hardens log input and " + "src/test/java/example/LogSanitizerTest.java adds a regression test." ), summary=FULL_SUMMARY.replace( "scripts/ci/example.py", @@ -1959,7 +2126,9 @@ def test_valid_control_rejects_fragile_approval_reason_before_evidence_repair( repaired = norm.valid_control( control( - reason="Dependency version bump with no source changes", + reason=( + "Dependency version bump in .github/workflows/r.yml with no source changes" + ), summary=( "Approval sufficiency: Dependency version bump with no source changes. " "Verification posture: No verification needed for workflow-only updates. " @@ -2157,7 +2326,10 @@ def test_valid_control_rejects_material_trivialization(tmp_path, monkeypatch): repaired = norm.valid_control( control( - reason="Current-head evidence was reviewed.", + reason=( + "Current-head .github/workflows/strix.yml and " + "scripts/ci/test_strix_quick_gate.sh were reviewed." + ), summary=( "The change is a simple typo fix in a string with no functional impact. " "No tests are needed for a string change." From 337f6313790e85aab47e70589e54a4afe6be4272 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 17 Aug 2026 08:06:54 +0900 Subject: [PATCH 2/2] fix(review): treat Unicode siblings as OpenCode APPROVE path tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ASCII-only token continuation accepted docs/한.md추가 as a disposition of docs/한.md. Count Unicode letters and digits as path-token body so prefix and suffix siblings cannot satisfy the per-file APPROVE walk. --- AGENTS.md | 2 +- ARCHITECTURE.md | 4 ++-- CHANGELOG.md | 2 +- CLAUDE.md | 2 +- .../review-contract-per-file-disposition.md | 14 ++++++++--- .../ci/opencode_review_normalize_output.py | 24 ++++++++++++------- .../test_opencode_review_normalize_output.py | 17 +++++++++++++ 7 files changed, 48 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 494cfa773..df06b7f4a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,4 +5,4 @@ Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include (no `.`/`..`); a lone `--require-hashes` directive is not trust evidence. See [`docs/doctoring/hourly-nvidia-nim-autofix.md`](docs/doctoring/hourly-nvidia-nim-autofix.md). Conflict-scope roots fail closed when the immediate parent directory is a symbolic link. -OpenCode APPROVE must name every current-head changed file as a whole path token. See [`docs/doctoring/review-contract-per-file-disposition.md`](docs/doctoring/review-contract-per-file-disposition.md). +OpenCode APPROVE must name every current-head changed file as a whole path token; Unicode prefix/suffix siblings are not a disposition. See [`docs/doctoring/review-contract-per-file-disposition.md`](docs/doctoring/review-contract-per-file-disposition.md). diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3deaa6218..b12215fe6 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -28,8 +28,8 @@ flowchart LR ## Per-file OpenCode APPROVE `valid_control` rejects APPROVE unless every current-head changed path -appears as a whole token. A longer sibling (`example.py.bak`) is not a -disposition of `example.py`. +appears as a whole token. A longer sibling (`example.py.bak` or +`docs/한.md추가`) is not a disposition of the shorter path. ## Hourly NVIDIA NIM repair gate diff --git a/CHANGELOG.md b/CHANGELOG.md index 6920ba441..f7c43b40d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed -- Require OpenCode APPROVE to name every current-head changed file as a whole path token so a sibling such as `example.py.bak` cannot stand in for `example.py`. +- Require OpenCode APPROVE to name every current-head changed file as a whole path token so a sibling such as `example.py.bak` or `docs/한.md추가` cannot stand in for the shorter path. - Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. A lone `--require-hashes` directive, a dotted include such as `./lock.txt`, or `-r other-hashes.txt` no longer enters the trusted build context. - Refused a conflict-scope repository root whose immediate parent is a symbolic link, so a swapped parent cannot redirect the canonical worktree after the last-component check (CWE-367). - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. diff --git a/CLAUDE.md b/CLAUDE.md index 1fdfd49a4..0d874c210 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -115,7 +115,7 @@ repeatable compile command. - **100% coverage and 100% docstrings on `scripts/ci/`** are hard gates, not aspirations. New helper code needs matching tests and docstrings. - **OpenCode APPROVE** must name every current-head changed file as a whole path token. - Prefix siblings such as `example.py.bak` do not count. + Prefix or suffix siblings such as `example.py.bak` or `docs/한.md추가` do not count. - **`pull_request_target` trust boundary.** The required review workflows run the *base branch's* trusted scripts. A PR that edits the trusted review workflows can fail its own checks until the base branch catches up; a same-head manual `workflow_dispatch` Strix run may supply review evidence diff --git a/docs/doctoring/review-contract-per-file-disposition.md b/docs/doctoring/review-contract-per-file-disposition.md index f73b84061..084250d2c 100644 --- a/docs/doctoring/review-contract-per-file-disposition.md +++ b/docs/doctoring/review-contract-per-file-disposition.md @@ -16,9 +16,12 @@ CodeRabbit” gap. `current_changed_files()` that is not named as a whole path token. Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include; a lone `--require-hashes` line is not lock evidence. -A longer sibling such as ``example.py.bak`` contains ``example.py`` as a -prefix substring; that is not a disposition of the shorter file -(CWE-1288; MITRE, 2026). ``path:line`` still counts. `valid_control` +A longer sibling such as ``example.py.bak`` or ``docs/한.md추가`` contains +the shorter path as a prefix substring; that is not a disposition of the +shorter file (CWE-1288; MITRE, 2026). Path-token continuation uses +Unicode letters and digits, not an ASCII-only class, so a Hangul suffix +or prefix sibling cannot stand in for the shorter path (Unicode +Consortium, 2024). ``path:line`` still counts. `valid_control` rejects APPROVE when that tuple is non-empty, both before and after bounded-evidence repair. Developer experience / User experience section labels were already required; this change only closes the file-walk @@ -44,6 +47,8 @@ current-head list (`scripts/ci/example.py` and `.github/workflows/strix.yml`). Naming only the first file is rejected. Naming both is accepted. A post-repair reason that drops the second path is still rejected. Naming only ``scripts/ci/example.py.bak`` still leaves ``scripts/ci/example.py`` unnamed. +Naming only ``docs/한.md추가`` or ``추가docs/한.md`` still leaves +``docs/한.md`` unnamed. ## Rollback @@ -66,3 +71,6 @@ https://doi.org/10.1109/ICSE.2013.6606617 GitHub. (n.d.). *About pull request reviews*. GitHub Docs. Retrieved August 13, 2026, from https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/about-pull-request-reviews + +Unicode Consortium. (2024). *The Unicode Standard* (Version 16.0.0). +https://www.unicode.org/versions/Unicode16.0.0/ diff --git a/scripts/ci/opencode_review_normalize_output.py b/scripts/ci/opencode_review_normalize_output.py index 1bee7ef11..900e7d8fc 100755 --- a/scripts/ci/opencode_review_normalize_output.py +++ b/scripts/ci/opencode_review_normalize_output.py @@ -854,9 +854,12 @@ def contradicts_material_changed_file_scope(reason: str, summary: str) -> bool: return any(phrase in combined for phrase in MATERIAL_CHANGE_FALSE_PHRASES) -_PATH_TOKEN_BODY = frozenset( - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789._-+/" -) +_PATH_TOKEN_SEPARATORS = frozenset("_-+/") + + +def _path_token_char(char: str) -> bool: + """Return True when ``char`` can appear inside a repository path token.""" + return char in _PATH_TOKEN_SEPARATORS or char.isalnum() def _path_token_continues(text: str, index: int) -> bool: @@ -864,19 +867,22 @@ def _path_token_continues(text: str, index: int) -> bool: if index < 0 or index >= len(text): return False char = text[index] - if char in "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-+/": - return True - if char == "." and index + 1 < len(text) and text[index + 1] in _PATH_TOKEN_BODY: + if _path_token_char(char): return True + if char == "." and index + 1 < len(text): + nxt = text[index + 1] + if nxt == "." or _path_token_char(nxt): + return True return False def changed_file_named_in_text(text: str, path: str) -> bool: """Return True when ``path`` appears as a whole path token in ``text``. - A longer sibling such as ``example.py.bak`` contains ``example.py`` as a - prefix substring. That is not a disposition of the shorter file. A - sentence period after ``.yml`` is not a continuation. + A longer sibling such as ``example.py.bak`` or ``docs/한.md추가`` + contains the shorter path as a prefix substring. That is not a + disposition of the shorter file. A sentence period after ``.yml`` is + not a continuation. """ if not path or not text: diff --git a/tests/test_opencode_review_normalize_output.py b/tests/test_opencode_review_normalize_output.py index c9606a35e..acc1bcd95 100644 --- a/tests/test_opencode_review_normalize_output.py +++ b/tests/test_opencode_review_normalize_output.py @@ -892,10 +892,27 @@ def test_approval_must_name_every_current_head_changed_file(tmp_path, monkeypatc assert not norm.changed_file_named_in_text( "Reviewed scripts/ci/example.py.bak.", "scripts/ci/example.py" ) + assert not norm.changed_file_named_in_text( + "Reviewed docs/한.md추가.", "docs/한.md" + ) + assert not norm.changed_file_named_in_text( + "Reviewed 추가docs/한.md.", "docs/한.md" + ) + assert not norm.changed_file_named_in_text( + "Reviewed docs/한.md.추가.", "docs/한.md" + ) + assert norm.changed_file_named_in_text("Reviewed docs/한.md.", "docs/한.md") assert not norm.changed_file_named_in_text("", "scripts/ci/example.py") assert not norm.changed_file_named_in_text("scripts/ci/example.py", "") + assert norm._path_token_char("한") + assert norm._path_token_char("_") + assert not norm._path_token_char(" ") assert norm._path_token_continues("scripts/ci/example.py.bak", 22) + assert norm._path_token_continues("docs/한.md추가", 9) + assert norm._path_token_continues("docs/한.md.추가", 9) + assert norm._path_token_continues("file.py..bak", 8) assert not norm._path_token_continues("scripts/ci/example.py.", 22) + assert not norm._path_token_continues("docs/한.md. Next", 9) assert not norm._path_token_continues("x", -1) assert not norm._path_token_continues("x", 1)