Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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; 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).
6 changes: 6 additions & 0 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` or
`docs/한.md추가`) is not a disposition of the shorter path.

## Hourly NVIDIA NIM repair gate

```mermaid
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` 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.
Expand Down
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 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
Expand Down
76 changes: 76 additions & 0 deletions docs/doctoring/review-contract-per-file-disposition.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# 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`` 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
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.
Naming only ``docs/한.md추가`` or ``추가docs/한.md`` still leaves
``docs/한.md`` 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

Unicode Consortium. (2024). *The Unicode Standard* (Version 16.0.0).
https://www.unicode.org/versions/Unicode16.0.0/
84 changes: 83 additions & 1 deletion scripts/ci/opencode_review_normalize_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -854,13 +854,83 @@ 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_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:
"""Return True when ``text[index]`` extends a repository path token."""
if index < 0 or index >= len(text):
return False
char = text[index]
if _path_token_char(char):
return True
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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`` 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:
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:
Expand Down Expand Up @@ -1289,6 +1359,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):
Expand All @@ -1311,6 +1387,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):
Expand Down
Loading
Loading