From 285672f3f8e0141d290b3a9c1304775b6f23f0ef Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 05:55:22 +0000 Subject: [PATCH 1/4] Remove permanently-unreachable PR #827 repair workflow .github/workflows/repair-pr827-coderabbit-comments.yml gated on github.event.pull_request.number == 827 plus an exact head ref/repo/actor check. PR #827 is closed (merged: false) and GitHub never reuses PR numbers, so the condition can never fire again. Its only real-world caller was this workflow. Delete the workflow and its driver script (scripts/ci/repair_pr827_coderabbit_comments.py, 460 lines), whose only remaining reference was a synthetic-fixture test in tests/test_materialize_base_python_requirements.py kept alive purely to satisfy the 100%-coverage gate rather than for any live purpose. Remove that fixture test and the ast/shutil imports it alone needed. Verified via `coverage run -m pytest tests` (2104 passed, 100% coverage) and `interrogate` (100% docstring coverage) after the change. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw --- .../repair-pr827-coderabbit-comments.yml | 105 ---- CHANGELOG.md | 8 + .../ci/repair_pr827_coderabbit_comments.py | 460 ------------------ ...st_materialize_base_python_requirements.py | 110 ----- 4 files changed, 8 insertions(+), 675 deletions(-) delete mode 100644 .github/workflows/repair-pr827-coderabbit-comments.yml delete mode 100644 scripts/ci/repair_pr827_coderabbit_comments.py diff --git a/.github/workflows/repair-pr827-coderabbit-comments.yml b/.github/workflows/repair-pr827-coderabbit-comments.yml deleted file mode 100644 index 7221c45650..0000000000 --- a/.github/workflows/repair-pr827-coderabbit-comments.yml +++ /dev/null @@ -1,105 +0,0 @@ -name: Repair PR 827 CodeRabbit comments - -on: - pull_request: - types: [synchronize, reopened, ready_for_review] - -permissions: - contents: read - -concurrency: - group: repair-pr827-coderabbit-comments - cancel-in-progress: true - -jobs: - repair: - if: >- - github.event.pull_request.number == 827 && - github.event.pull_request.head.repo.full_name == github.repository && - github.event.pull_request.head.ref == 'fix/opencode-rust-coverage-runtime-boundary-main' && - github.event.pull_request.head.user.login != 'github-actions[bot]' - runs-on: ubuntu-24.04 - timeout-minutes: 45 - permissions: - contents: write - env: - FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - steps: - - name: Harden runner - uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0 - with: - egress-policy: audit - - - name: Checkout exact PR branch - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - ref: fix/opencode-rust-coverage-runtime-boundary-main - fetch-depth: 0 - persist-credentials: true - - - name: Set up Python 3.14 - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - with: - python-version: '3.14' - cache: pip - cache-dependency-path: requirements-opencode-review-ci-hashes.txt - - - name: Install exact hash-locked test tooling - run: >- - python -m pip install --disable-pip-version-check --require-hashes - -r requirements-opencode-review-ci-hashes.txt - - - name: Apply bounded non-workflow repairs - run: | - set -euo pipefail - python - <<'PY' - from pathlib import Path - - repair = Path('scripts/ci/repair_pr827_coderabbit_comments.py') - repair_text = repair.read_text(encoding='utf-8') - old = ' destination = output_dir / include_directory / Path(*relative_target.parts)\n' - new = ' destination = output_dir / include_directory / pathlib.Path(*relative_target.parts)\n' - if repair_text.count(old) != 1: - raise SystemExit('expected one unqualified generated Path reference') - repair.write_text(repair_text.replace(old, new, 1), encoding='utf-8') - PY - python scripts/ci/repair_pr827_coderabbit_comments.py - # The ordinary Actions token cannot update workflow files. The license - # basis is already recorded in the doctoring document, so retain the - # reviewed workflow source and publish the non-workflow repair only. - git checkout -- .github/workflows/opencode-review-dispatch.yml - rm -f scripts/ci/repair_pr827_coderabbit_comments.py - - - name: Verify materialization, coverage, docs, and syntax - run: | - set -euo pipefail - python -m pytest -q \ - tests/test_materialize_base_python_requirements.py \ - tests/test_opencode_rust_coverage_toolchain_contract.py - python -m coverage erase - python -m coverage run -m pytest tests - python -m coverage report --show-missing --fail-under=100 - python -m compileall -q scripts tests - git diff --check - - - name: Commit verified non-workflow repair - run: | - set -euo pipefail - # Restore the temporary repair driver so this commit contains only - # the reviewed product/test/doctoring changes. It is removed through - # the connector immediately after the verified push. - git checkout -- scripts/ci/repair_pr827_coderabbit_comments.py - git config user.name 'github-actions[bot]' - git config user.email '41898282+github-actions[bot]@users.noreply.github.com' - git add \ - scripts/ci/materialize_base_python_requirements.py \ - tests/test_materialize_base_python_requirements.py \ - CHANGELOG.md \ - docs/doctoring/opencode-rust-coverage-runtime-boundary.md - git diff --cached --check - if git diff --cached --quiet; then - echo 'No non-workflow repair changes remain; the rerun is complete.' - exit 0 - fi - git commit -m 'fix(coverage): preserve bounded requirement includes' - git push origin HEAD:fix/opencode-rust-coverage-runtime-boundary-main diff --git a/CHANGELOG.md b/CHANGELOG.md index 256b0cdf94..b33d1267b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Remove the permanently-unreachable PR #827 repair workflow + (`.github/workflows/repair-pr827-coderabbit-comments.yml`) and its driver + script (`scripts/ci/repair_pr827_coderabbit_comments.py`): PR #827 is + closed and unmerged, GitHub never reuses PR numbers, so the workflow's + `github.event.pull_request.number == 827` gate could never fire again. + Its only caller was that workflow; the script's sole remaining reference + was a synthetic-fixture test kept alive purely to satisfy the 100% + coverage gate, which is removed along with it. - Fix a dangling reference #1468 left in `docs/product-goal-directive.md` (flagged by Devin Review on that PR): the standing operating directive still named the removed `free_family_diversity` evidence field instead of diff --git a/scripts/ci/repair_pr827_coderabbit_comments.py b/scripts/ci/repair_pr827_coderabbit_comments.py deleted file mode 100644 index 8b1df14cb1..0000000000 --- a/scripts/ci/repair_pr827_coderabbit_comments.py +++ /dev/null @@ -1,460 +0,0 @@ -#!/usr/bin/env python3 -"""Apply the verified CodeRabbit repairs for pull request 827.""" - -from __future__ import annotations - -from pathlib import Path - - -def replace_once( - path: str, old: str, new: str, *, allow_repeated: bool = False -) -> None: - """Replace one exact fragment, optionally tolerating repeated history markers.""" - file_path = Path(path) - text = file_path.read_text(encoding="utf-8") - count = text.count(old) - if count == 0 and ( - text.count(new) == 1 or (allow_repeated and text.count(new) > 0) - ): - return - if count == 0 or (count != 1 and not allow_repeated): - raise SystemExit(f"{path}: expected one replacement marker, found {count}") - file_path.write_text(text.replace(old, new, 1), encoding="utf-8") - - -def insert_before(path: str, anchor: str, addition: str) -> None: - """Insert an addition before one unique anchor, at most once.""" - file_path = Path(path) - text = file_path.read_text(encoding="utf-8") - if addition in text: - return - count = text.count(anchor) - if count != 1: - raise SystemExit(f"{path}: expected one insertion anchor, found {count}") - file_path.write_text(text.replace(anchor, addition + anchor, 1), encoding="utf-8") - - -def insert_after(path: str, anchor: str, addition: str) -> None: - """Insert an addition after one unique anchor, at most once.""" - file_path = Path(path) - text = file_path.read_text(encoding="utf-8") - if addition in text: - return - count = text.count(anchor) - if count != 1: - raise SystemExit(f"{path}: expected one insertion anchor, found {count}") - file_path.write_text(text.replace(anchor, anchor + addition, 1), encoding="utf-8") - - -def replace_between(path: str, start: str, end: str, replacement: str) -> None: - """Replace a uniquely delimited source section idempotently.""" - file_path = Path(path) - text = file_path.read_text(encoding="utf-8") - start_index = text.find(start) - if start_index < 0 and text.count(replacement) == 1: - return - if start_index < 0 or text.find(start, start_index + 1) >= 0: - raise SystemExit(f"{path}: start marker missing or ambiguous") - end_index = text.find(end, start_index) - if end_index < 0: - raise SystemExit(f"{path}: end marker missing") - file_path.write_text( - text[:start_index] + replacement + text[end_index:], encoding="utf-8" - ) - - -SCRIPT = "scripts/ci/materialize_base_python_requirements.py" -TEST = "tests/test_materialize_base_python_requirements.py" -DOC = "docs/doctoring/opencode-rust-coverage-runtime-boundary.md" -WORKFLOW = ".github/workflows/opencode-review-dispatch.yml" - -replace_between( - SCRIPT, - "def _is_bounded_requirement_include(line: str) -> bool:\n", - "def _requirement_lines(content: bytes) -> list[str]:\n", - '''def _bounded_requirement_include_target( - line: str, -) -> pathlib.PurePosixPath | None: - """Return the safe relative target of one bounded requirements include. - - The target may use any normalized relative ``.txt`` name, including names - such as ``other-hashes.txt``. Eligibility does not confer trust: the exact - base-tree target must later be a regular blob containing only exact - SHA-256-pinned package requirements. - """ - fields = line.split() - if len(fields) != 2 or fields[0] not in {"-r", "--requirement"}: - return None - target = fields[1] - if ( - target.startswith(("-", "~")) - or "\\\\" in target - or ":" in target - or "?" in target - or "#" in target - ): - return None - include_path = pathlib.PurePosixPath(target) - if ( - not include_path.parts - or target != include_path.as_posix() - or include_path.is_absolute() - or "." in include_path.parts - or ".." in include_path.parts - or include_path.suffix != ".txt" - ): - return None - return include_path - - -def _is_bounded_requirement_include(line: str) -> bool: - """Return whether one include has a safe relative ``.txt`` target.""" - return _bounded_requirement_include_target(line) is not None - - -''', -) - -replace_once( - SCRIPT, - " if _is_candidate_lock_name(candidate.name):\n", - " if _is_candidate_lock_path(candidate):\n", -) - -helpers = '''def _included_base_lock_blobs( - repo_root: pathlib.Path, - base_sha: str, - source_path: str, - content: bytes, - regular_paths: set[str], -) -> list[tuple[pathlib.PurePosixPath, bytes]]: - """Load direct bounded includes from the exact base as complete closures.""" - source_parent = pathlib.PurePosixPath(source_path).parent - included: dict[pathlib.PurePosixPath, bytes] = {} - for line in _requirement_lines(content): - target = _bounded_requirement_include_target(line) - if target is None: - continue - resolved = source_parent / target - resolved_path = resolved.as_posix() - if resolved_path not in regular_paths: - raise RuntimeError( - f"bounded include {target} from {source_path} is not a regular base blob" - ) - included_content = _git(repo_root, "show", f"{base_sha}:{resolved_path}") - if not _is_flat_materializable_lock(included_content): - raise RuntimeError( - f"bounded include {resolved_path} must contain only exact SHA-256 pins" - ) - included[target] = included_content - return sorted(included.items(), key=lambda item: item[0].as_posix()) - - -def _rewrite_materialized_includes( - content: bytes, include_directory: str, source_path: str = "" -) -> bytes: - """Rewrite root include targets to their preserved generated subtree.""" - try: - text = content.decode("utf-8", errors="strict") - except UnicodeDecodeError as exc: - raise RuntimeError(f"base lock {source_path} is not valid UTF-8") from exc - rewritten: list[str] = [] - for raw_line in text.splitlines(keepends=True): - body = raw_line.rstrip("\\r\\n") - ending = raw_line[len(body) :] - stripped = body.strip() - target = _bounded_requirement_include_target(stripped) - if target is None: - rewritten.append(raw_line) - continue - indentation = body[: len(body) - len(body.lstrip())] - option = stripped.split()[0] - rewritten.append( - f"{indentation}{option} {include_directory}/{target.as_posix()}{ending}" - ) - return "".join(rewritten).encode("utf-8") - - -''' -insert_before(SCRIPT, "def materialize(\n", helpers) - -replace_between( - SCRIPT, - "def materialize(\n", - "def main(argv: list[str] | None = None) -> int:\n", - '''def materialize( - repo_root: pathlib.Path, - base_sha: str, - output_dir: pathlib.Path, -) -> list[dict[str, str]]: - """Write base locks and resolvable bounded includes into a safe context.""" - if output_dir.exists() and output_dir.is_symlink(): - raise ValueError("output directory must not be a symlink") - output_dir.mkdir(parents=True, exist_ok=True) - - resolved_repo = repo_root.resolve() - entries = _git(resolved_repo, "ls-tree", "-r", "-z", "--full-tree", base_sha) - regular_paths = { - path for path, _candidate in _regular_base_blob_paths(entries) - } - manifest: list[dict[str, str]] = [] - for index, (source_path, content) in enumerate( - base_hash_locks(resolved_repo, base_sha) - ): - generated_name = f"requirements-{index:03d}.txt" - include_directory = f"includes-{index:03d}" - included = _included_base_lock_blobs( - resolved_repo, - base_sha, - source_path, - content, - regular_paths, - ) - for relative_target, included_content in included: - destination = output_dir / include_directory / Path(*relative_target.parts) - destination.parent.mkdir(parents=True, exist_ok=True) - destination.write_bytes(included_content) - destination = output_dir / generated_name - destination.write_bytes( - _rewrite_materialized_includes(content, include_directory, source_path) - ) - manifest.append({"file": generated_name, "source": source_path}) - - (output_dir / "manifest.json").write_text( - json.dumps(manifest, indent=2, sort_keys=True) + "\\n", - encoding="utf-8", - ) - (output_dir / "manifest.txt").write_text( - "".join(f"{entry['file']}\\n" for entry in manifest), - encoding="utf-8", - ) - return manifest - - -''', -) - -insert_before(TEST, "from pathlib import Path\n", "import zipfile\n") -replace_once( - TEST, - ' assert not materializer._is_hash_pinned(b"-r other-hashes.txt\\n")\n', - ' assert materializer._is_hash_pinned(b"-r other-hashes.txt\\n")\n', -) -insert_after( - TEST, - ' assert not materializer._is_candidate_lock_name("pyproject.toml")\n', - ' assert materializer._is_candidate_lock_path(\n' - ' materializer.pathlib.PurePosixPath("requirements/ci.txt")\n' - ' )\n' - ' assert materializer._is_candidate_lock_path(\n' - ' materializer.pathlib.PurePosixPath("service/requirements/package.txt")\n' - ' )\n' - ' assert not materializer._is_candidate_lock_path(\n' - ' materializer.pathlib.PurePosixPath("service/config/ci.txt")\n' - ' )\n', -) -insert_after( - TEST, - ' (repo / "requirements-test.txt").write_text(\n' - ' "hypothesis==6 --hash=sha256:" + ("b" * 64) + "\\n",\n' - ' encoding="utf-8",\n' - ' )\n', - ' requirements_dir = repo / "requirements"\n' - ' requirements_dir.mkdir()\n' - ' (requirements_dir / "ci.txt").write_text(\n' - ' "pytest==9 --hash=sha256:" + ("c" * 64) + "\\n",\n' - ' encoding="utf-8",\n' - ' )\n', -) -replace_once( - TEST, - ' "requirements-test.txt",\n' - ' "services/account_unification/requirements-dev.txt",\n', - ' "requirements-test.txt",\n' - ' "requirements/ci.txt",\n' - ' "services/account_unification/requirements-dev.txt",\n', -) -insert_before( - TEST, - ' between_file.write_text("START old", encoding="utf-8")\n', - ''' before_file = tmp_path / "before.txt" - before_file.write_text("ANCHOR", encoding="utf-8") - insert_before = namespace["insert_before"] - insert_before(str(before_file), "ANCHOR", "PREFIX ") # type: ignore[operator] - assert before_file.read_text(encoding="utf-8") == "PREFIX ANCHOR" - insert_before(str(before_file), "ANCHOR", "PREFIX ") # type: ignore[operator] - with pytest.raises(SystemExit, match="expected one insertion anchor"): - insert_before(str(before_file), "MISSING", "OTHER ") # type: ignore[operator] - after_file = tmp_path / "after.txt" - after_file.write_text("ANCHOR", encoding="utf-8") - insert_after = namespace["insert_after"] - insert_after(str(after_file), "ANCHOR", " SUFFIX") # type: ignore[operator] - assert after_file.read_text(encoding="utf-8") == "ANCHOR SUFFIX" - insert_after(str(after_file), "ANCHOR", " SUFFIX") # type: ignore[operator] - with pytest.raises(SystemExit, match="expected one insertion anchor"): - insert_after(str(after_file), "MISSING", " OTHER") # type: ignore[operator] -''', -) - -integration_test = '''def test_materialized_bounded_include_is_resolvable_by_pip(tmp_path: Path) -> None: - """A safe base-owned include survives flattening and pip hash preflight.""" - repo = tmp_path / "repo" - repo.mkdir() - git(repo, "init") - git(repo, "config", "user.name", "Test") - git(repo, "config", "user.email", "test@example.invalid") - - wheel_dir = tmp_path / "wheels" - wheel_dir.mkdir() - wheel = wheel_dir / "demo-1-py3-none-any.whl" - with zipfile.ZipFile(wheel, "w") as archive: - archive.writestr("demo/__init__.py", "__version__ = '1'\\n") - archive.writestr( - "demo-1.dist-info/METADATA", - "Metadata-Version: 2.1\\nName: demo\\nVersion: 1\\n", - ) - archive.writestr( - "demo-1.dist-info/WHEEL", - "Wheel-Version: 1.0\\nGenerator: TEPP-test\\n" - "Root-Is-Purelib: true\\nTag: py3-none-any\\n", - ) - archive.writestr("demo-1.dist-info/RECORD", "") - digest = hashlib.sha256(wheel.read_bytes()).hexdigest() - - (repo / "requirements.txt").write_text( - "-r other-hashes.txt\\n", encoding="utf-8" - ) - (repo / "other-hashes.txt").write_text( - f"--require-hashes\\ndemo==1 --hash=sha256:{digest}\\n", encoding="utf-8" - ) - git(repo, "add", ".") - git(repo, "commit", "-m", "base") - base_sha = git(repo, "rev-parse", "HEAD") - - output = tmp_path / "output" - manifest = materializer.materialize(repo, base_sha, output) - assert manifest == [{"file": "requirements-000.txt", "source": "requirements.txt"}] - assert (output / "requirements-000.txt").read_text(encoding="utf-8") == ( - "-r includes-000/other-hashes.txt\\n" - ) - assert (output / "includes-000" / "other-hashes.txt").is_file() - - completed = subprocess.run( - [ - sys.executable, - "-m", - "pip", - "install", - "--dry-run", - "--ignore-installed", - "--disable-pip-version-check", - "--no-index", - "--find-links", - str(wheel_dir), - "--require-hashes", - "-r", - str(output / "requirements-000.txt"), - ], - check=False, - capture_output=True, - text=True, - ) - assert completed.returncode == 0, completed.stdout + completed.stderr - - -def test_materialization_rejects_missing_or_nested_include(tmp_path: Path) -> None: - """Includes must resolve to direct complete hash closures in the exact base.""" - repo = tmp_path / "repo" - repo.mkdir() - git(repo, "init") - git(repo, "config", "user.name", "Test") - git(repo, "config", "user.email", "test@example.invalid") - (repo / "requirements.txt").write_text("-r child.txt\\n", encoding="utf-8") - git(repo, "add", ".") - git(repo, "commit", "-m", "missing") - missing_sha = git(repo, "rev-parse", "HEAD") - with pytest.raises(RuntimeError, match="not a regular base blob"): - materializer.materialize(repo, missing_sha, tmp_path / "missing-output") - - (repo / "child.txt").write_text("-r grandchild.txt\\n", encoding="utf-8") - (repo / "grandchild.txt").write_text( - "demo==1 --hash=sha256:" + ("d" * 64) + "\\n", encoding="utf-8" - ) - git(repo, "add", ".") - git(repo, "commit", "-m", "nested") - nested_sha = git(repo, "rev-parse", "HEAD") - with pytest.raises(RuntimeError, match="must contain only exact SHA-256 pins"): - materializer.materialize(repo, nested_sha, tmp_path / "nested-output") - - with pytest.raises(RuntimeError, match="base lock requirements.txt is not valid UTF-8"): - materializer._rewrite_materialized_includes( - b"\\xff", "includes-000", "requirements.txt" - ) - - -''' -insert_before(TEST, "def test_rejects_invalid_base_sha", integration_test) -replace_once( - TEST, - ' assert not materializer._is_bounded_requirement_include("-r /abs/requirements.txt")\n', - ' assert not materializer._is_bounded_requirement_include("-r /abs/requirements.txt")\n' - ' assert not materializer._is_bounded_requirement_include("-r pyproject.toml")\n', -) -replace_once( - "tests/test_opencode_rust_coverage_toolchain_contract.py", - """ assert f'"${{LLVM_COV:-}}" != "$LLVM_COV_PATH"' in helper - assert f'"${{LLVM_PROFDATA:-}}" != "$LLVM_PROFDATA_PATH"' in helper -""", - """ assert '"${LLVM_COV:-}" != "$LLVM_COV_PATH"' in helper - assert '"${LLVM_PROFDATA:-}" != "$LLVM_PROFDATA_PATH"' in helper -""", -) -insert_after( - "tests/test_opencode_rust_coverage_toolchain_contract.py", - " for relative_path in watched_paths:\n" - " assert (_REPOSITORY_ROOT / relative_path).is_file(), relative_path\n", - """ doctoring = ( - _REPOSITORY_ROOT - / "docs/doctoring/opencode-rust-coverage-runtime-boundary.md" - ).read_text(encoding="utf-8") - assert "/usr/bin/llvm-cov-19" in doctoring - assert "/usr/bin/llvm-profdata-19" in doctoring - assert "unversioned `llvm-cov`" in doctoring - assert "fails closed" in doctoring -""", -) - -replace_once( - "CHANGELOG.md", - "- 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.\n", - "- Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. Includes such as `-r other-hashes.txt` remain allowed when the exact base-tree target is a regular, complete SHA-256-pinned closure; a lone `--require-hashes` directive, dotted `./lock.txt`, traversal, absolute, URL, and option-like targets fail closed.\n", - allow_repeated=True, -) - -replace_once( - DOC, - "These are compatibility and trust-boundary constants, not caller-selectable\nconfiguration. The reviewed helper `scripts/ci/ensure_rust_llvm19.sh` binds both\nexact paths and fails closed unless the live `LLVM_COV` / `LLVM_PROFDATA`\nvalues match and are executable before Rust coverage evidence is admitted. The\nindependent OpenCode review-dispatch workflow stays byte-for-byte so the\nreview-agent key system is not rewritten to carry this runtime check.\n", - "These are compatibility and trust-boundary constants, not caller-selectable\nconfiguration. The reviewed helper `scripts/ci/ensure_rust_llvm19.sh` validates\nboth exact paths and fails closed unless the live `LLVM_COV` / `LLVM_PROFDATA`\nvalues match and are executable before Rust coverage evidence is admitted. The\nactual environment binding is owned by\n`.github/workflows/opencode-review-dispatch.yml`, through its Dockerfile `ENV`\ndeclarations and the isolated container's `docker run --env` arguments. If that\nworkflow changes, its `REVIEW_DISPATCH_BLOB_SHA` pin must change with it; this\ndoes not rewrite the review-agent key system.\n", -) -replace_once( - DOC, - "NIST SP 800-218 PW.4.1 requires third-party software to come from expected,\ntrusted sources with integrity verification (Souppaya et al., 2022). Binding\ncoverage to the reviewed `/usr/bin/llvm-cov-19` and\n`/usr/bin/llvm-profdata-19` executables is that verification; an ambient\n`PATH` lookup would treat a runner-image change as a new producer.\n", - "NIST SP 800-218 PW.4.1 covers acquiring and maintaining third-party software\nfrom expected, trusted sources and reviewing its provenance (Souppaya et al.,\n2022). PW.4.4 covers verifying the integrity of acquired components. The exact\n`/usr/bin/llvm-cov-19` and `/usr/bin/llvm-profdata-19` bindings are\nproducer-selection controls: they select reviewed paths and `test -x` verifies\nexecutability. They do not hash or signature-verify the Debian package or binary;\npackage/image hashes, signatures, repository metadata, and attestations remain\nseparate PW.4.4 integrity controls and must not be inferred from path equality.\n", -) -replace_once( - DOC, - "Debian bookworm currently publishes the versioned `llvm-19` package from\n`llvm-toolchain-19`; Debian package file inventories expose versioned LLVM 19\ntool entry points including `llvm-cov-19`. Pinning the reviewed executable names\ninside the image converts that mutable ambient dependency into an explicit\ncontract that can be checked before source execution.\n", - "Debian publishes `llvm-19` from the `llvm-toolchain-19` source package; its\nofficial copyright record states `Apache-2.0 WITH LLVM-exception`. Debian package\nfile inventories expose versioned LLVM 19 tool entry points including\n`llvm-cov-19`. Pinning those reviewed executable names inside the image converts\nambient path selection into an explicit, testable producer contract; the Debian\ncopyright record supplies the package license basis, not executable integrity.\n", -) -replace_once( - DOC, - "Debian Project. (2026). *File list of package llvm-19*. Debian Packages.\nRetrieved August 10, 2026, from\nhttps://packages.debian.org/bookworm/amd64/llvm-19/filelist\n\n", - "Debian Project. (2026). *File list of package llvm-19*. Debian Packages.\nRetrieved August 10, 2026, from\nhttps://packages.debian.org/bookworm/amd64/llvm-19/filelist\n\nDebian Project. (2026). *Copyright file for llvm-toolchain-19 19.1.7-20*.\nDebian FTP Masters. Retrieved August 15, 2026, from\nhttps://metadata.ftp-master.debian.org/changelogs/main/l/llvm-toolchain-19/llvm-toolchain-19_19.1.7-20_copyright\n\n", -) - -replace_once( - WORKFLOW, - " RUN apt-get update \\\n && apt-get install --no-install-recommends -y \\\n", - " # llvm-19 / llvm-toolchain-19: Apache-2.0 WITH LLVM-exception. See docs/doctoring/opencode-rust-coverage-runtime-boundary.md.\n" - " RUN apt-get update \\\n && apt-get install --no-install-recommends -y \\\n", -) diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 58ded37400..a2da04ae25 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -1,11 +1,9 @@ from __future__ import annotations -import ast import hashlib import io import json import runpy -import shutil import subprocess import sys import tarfile @@ -398,114 +396,6 @@ def test_materialization_rejects_missing_or_nested_include(tmp_path: Path) -> No ) -def test_bounded_repair_driver_runs_against_a_staged_fixture( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch -) -> None: - """The one-shot repair driver applies every guarded edit in isolation.""" - repository_root = Path(__file__).parents[1] - relative_files = ( - "scripts/ci/repair_pr827_coderabbit_comments.py", - "scripts/ci/materialize_base_python_requirements.py", - "tests/test_materialize_base_python_requirements.py", - "tests/test_opencode_rust_coverage_toolchain_contract.py", - "docs/doctoring/opencode-rust-coverage-runtime-boundary.md", - ".github/workflows/opencode-review-dispatch.yml", - "CHANGELOG.md", - ) - for relative_file in relative_files: - source = repository_root / relative_file - destination = tmp_path / relative_file - destination.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(source, destination) - - changelog = tmp_path / "CHANGELOG.md" - changelog.write_text( - changelog.read_text(encoding="utf-8").replace( - "## [Unreleased]\n", - "## [Unreleased]\n\n" - "- 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.\n", - 1, - ), - encoding="utf-8", - ) - - monkeypatch.chdir(tmp_path) - runpy.run_path( - str(repository_root / "scripts/ci/repair_pr827_coderabbit_comments.py"), - run_name="__main__", - ) - - materializer_source = ( - tmp_path / "scripts/ci/materialize_base_python_requirements.py" - ).read_text(encoding="utf-8") - assert "def _bounded_requirement_include_target(" in materializer_source - assert "def _included_base_lock_blobs(" in materializer_source - assert "includes-000/" in ( - tmp_path / "tests/test_materialize_base_python_requirements.py" - ).read_text(encoding="utf-8") - assert "Apache-2.0 WITH LLVM-exception" in ( - tmp_path / "docs/doctoring/opencode-rust-coverage-runtime-boundary.md" - ).read_text(encoding="utf-8") - - script_path = repository_root / "scripts/ci/repair_pr827_coderabbit_comments.py" - tree = ast.parse(script_path.read_text(encoding="utf-8"), filename=str(script_path)) - definitions = [node for node in tree.body if isinstance(node, ast.FunctionDef)] - namespace: dict[str, object] = {"Path": Path} - exec( - compile( - ast.fix_missing_locations(ast.Module(body=definitions, type_ignores=[])), - str(script_path), - "exec", - ), - namespace, - ) - replace_once = namespace["replace_once"] - replace_between = namespace["replace_between"] - once_file = tmp_path / "once.txt" - once_file.write_text("old", encoding="utf-8") - replace_once(str(once_file), "old", "new") # type: ignore[operator] - assert once_file.read_text(encoding="utf-8") == "new" - with pytest.raises(SystemExit, match="expected one replacement marker"): - replace_once(str(once_file), "missing", "other") # type: ignore[operator] - repeated_file = tmp_path / "repeated.txt" - repeated_file.write_text("oldold", encoding="utf-8") - replace_once( # type: ignore[operator] - str(repeated_file), "old", "new", allow_repeated=True - ) - assert repeated_file.read_text(encoding="utf-8") == "newold" - - between_file = tmp_path / "between.txt" - between_file.write_text("START old END", encoding="utf-8") - replace_between(str(between_file), "START", "END", "START new ") # type: ignore[operator] - assert between_file.read_text(encoding="utf-8") == "START new END" - replace_between(str(between_file), "MISSING", "END", "START new ") # type: ignore[operator] - before_file = tmp_path / "before.txt" - before_file.write_text("ANCHOR", encoding="utf-8") - insert_before = namespace["insert_before"] - insert_before(str(before_file), "ANCHOR", "PREFIX ") # type: ignore[operator] - assert before_file.read_text(encoding="utf-8") == "PREFIX ANCHOR" - insert_before(str(before_file), "ANCHOR", "PREFIX ") # type: ignore[operator] - with pytest.raises(SystemExit, match="expected one insertion anchor"): - insert_before(str(before_file), "MISSING", "OTHER ") # type: ignore[operator] - after_file = tmp_path / "after.txt" - after_file.write_text("ANCHOR", encoding="utf-8") - insert_after = namespace["insert_after"] - insert_after(str(after_file), "ANCHOR", " SUFFIX") # type: ignore[operator] - assert after_file.read_text(encoding="utf-8") == "ANCHOR SUFFIX" - insert_after(str(after_file), "ANCHOR", " SUFFIX") # type: ignore[operator] - with pytest.raises(SystemExit, match="expected one insertion anchor"): - insert_after(str(after_file), "MISSING", " OTHER") # type: ignore[operator] - between_file.write_text("START old START END", encoding="utf-8") - with pytest.raises(SystemExit, match="start marker missing or ambiguous"): - replace_between(str(between_file), "START", "END", "replacement") # type: ignore[operator] - between_file.write_text("START old", encoding="utf-8") - with pytest.raises(SystemExit, match="end marker missing"): - replace_between(str(between_file), "START", "END", "replacement") # type: ignore[operator] - - def test_rejects_invalid_base_sha(tmp_path: Path) -> None: """Git options and symbolic refs cannot cross the exact-SHA boundary.""" with pytest.raises(ValueError, match="40 hexadecimal"): From 63c210eaa8feb9c5092790e9f84b648b3c4112de Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 08:32:15 +0000 Subject: [PATCH 2/4] Revert "Remove permanently-unreachable PR #827 repair workflow" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 285672f3f8e0141d290b3a9c1304775b6f23f0ef. Devin Review flagged the removal's core safety argument as wrong: PR #827 is closed and merged: false, but that does not make it unreachable. "GitHub never reuses PR numbers" only prevents a *new* PR from claiming #827 — it says nothing about the *existing* closed PR being reopened, which keeps its original number. Independently re-verified against live GitHub state: - PR #827: state=closed, merged=false, locked=false — reopenable by anyone with write access, or by its author, with no lock blocking it. - Its head branch, fix/opencode-rust-coverage-runtime-boundary-main, still exists on origin at the same SHA (48603e869e755b441a1a1b0828c8131948d3b23d). Both preconditions Devin named for a reachable repair path are true, so restore the workflow, its driver script (scripts/ci/repair_pr827_coderabbit_comments.py), and the test_bounded_repair_driver_runs_against_a_staged_fixture coverage fixture exactly as they were, and reframe the CHANGELOG entry as 'kept', not 'removed'. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw --- .../repair-pr827-coderabbit-comments.yml | 105 ++++ CHANGELOG.md | 19 +- .../ci/repair_pr827_coderabbit_comments.py | 460 ++++++++++++++++++ ...st_materialize_base_python_requirements.py | 110 +++++ 4 files changed, 687 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/repair-pr827-coderabbit-comments.yml create mode 100644 scripts/ci/repair_pr827_coderabbit_comments.py diff --git a/.github/workflows/repair-pr827-coderabbit-comments.yml b/.github/workflows/repair-pr827-coderabbit-comments.yml new file mode 100644 index 0000000000..7221c45650 --- /dev/null +++ b/.github/workflows/repair-pr827-coderabbit-comments.yml @@ -0,0 +1,105 @@ +name: Repair PR 827 CodeRabbit comments + +on: + pull_request: + types: [synchronize, reopened, ready_for_review] + +permissions: + contents: read + +concurrency: + group: repair-pr827-coderabbit-comments + cancel-in-progress: true + +jobs: + repair: + if: >- + github.event.pull_request.number == 827 && + github.event.pull_request.head.repo.full_name == github.repository && + github.event.pull_request.head.ref == 'fix/opencode-rust-coverage-runtime-boundary-main' && + github.event.pull_request.head.user.login != 'github-actions[bot]' + runs-on: ubuntu-24.04 + timeout-minutes: 45 + permissions: + contents: write + env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + steps: + - name: Harden runner + uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0 + with: + egress-policy: audit + + - name: Checkout exact PR branch + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: fix/opencode-rust-coverage-runtime-boundary-main + fetch-depth: 0 + persist-credentials: true + + - name: Set up Python 3.14 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: '3.14' + cache: pip + cache-dependency-path: requirements-opencode-review-ci-hashes.txt + + - name: Install exact hash-locked test tooling + run: >- + python -m pip install --disable-pip-version-check --require-hashes + -r requirements-opencode-review-ci-hashes.txt + + - name: Apply bounded non-workflow repairs + run: | + set -euo pipefail + python - <<'PY' + from pathlib import Path + + repair = Path('scripts/ci/repair_pr827_coderabbit_comments.py') + repair_text = repair.read_text(encoding='utf-8') + old = ' destination = output_dir / include_directory / Path(*relative_target.parts)\n' + new = ' destination = output_dir / include_directory / pathlib.Path(*relative_target.parts)\n' + if repair_text.count(old) != 1: + raise SystemExit('expected one unqualified generated Path reference') + repair.write_text(repair_text.replace(old, new, 1), encoding='utf-8') + PY + python scripts/ci/repair_pr827_coderabbit_comments.py + # The ordinary Actions token cannot update workflow files. The license + # basis is already recorded in the doctoring document, so retain the + # reviewed workflow source and publish the non-workflow repair only. + git checkout -- .github/workflows/opencode-review-dispatch.yml + rm -f scripts/ci/repair_pr827_coderabbit_comments.py + + - name: Verify materialization, coverage, docs, and syntax + run: | + set -euo pipefail + python -m pytest -q \ + tests/test_materialize_base_python_requirements.py \ + tests/test_opencode_rust_coverage_toolchain_contract.py + python -m coverage erase + python -m coverage run -m pytest tests + python -m coverage report --show-missing --fail-under=100 + python -m compileall -q scripts tests + git diff --check + + - name: Commit verified non-workflow repair + run: | + set -euo pipefail + # Restore the temporary repair driver so this commit contains only + # the reviewed product/test/doctoring changes. It is removed through + # the connector immediately after the verified push. + git checkout -- scripts/ci/repair_pr827_coderabbit_comments.py + git config user.name 'github-actions[bot]' + git config user.email '41898282+github-actions[bot]@users.noreply.github.com' + git add \ + scripts/ci/materialize_base_python_requirements.py \ + tests/test_materialize_base_python_requirements.py \ + CHANGELOG.md \ + docs/doctoring/opencode-rust-coverage-runtime-boundary.md + git diff --cached --check + if git diff --cached --quiet; then + echo 'No non-workflow repair changes remain; the rerun is complete.' + exit 0 + fi + git commit -m 'fix(coverage): preserve bounded requirement includes' + git push origin HEAD:fix/opencode-rust-coverage-runtime-boundary-main diff --git a/CHANGELOG.md b/CHANGELOG.md index e433efc38e..d56d54aaf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,14 +5,19 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] -- Remove the permanently-unreachable PR #827 repair workflow +- Keep the PR #827 repair workflow (`.github/workflows/repair-pr827-coderabbit-comments.yml`) and its driver - script (`scripts/ci/repair_pr827_coderabbit_comments.py`): PR #827 is - closed and unmerged, GitHub never reuses PR numbers, so the workflow's - `github.event.pull_request.number == 827` gate could never fire again. - Its only caller was that workflow; the script's sole remaining reference - was a synthetic-fixture test kept alive purely to satisfy the 100% - coverage gate, which is removed along with it. + script (`scripts/ci/repair_pr827_coderabbit_comments.py`): a prior pass on + this same PR proposed removing them as dead code on the theory that + "GitHub never reuses PR numbers" makes `github.event.pull_request.number + == 827` permanently false. Devin Review correctly flagged that reasoning + as a red herring here — PR #827 is `closed`/`merged: false`/`locked: + false`, so it remains reopenable by anyone with write access (or its + author) and would keep its original number if reopened, and its exact + head branch (`fix/opencode-rust-coverage-runtime-boundary-main`) still + exists on the remote at the same SHA. The workflow and script are + restored unchanged; the synthetic-fixture test kept alive purely for the + 100% coverage gate is restored with them. - Harden the review sidecar's per-account catalog cap against silent drift: `contextual_orchestrator_review_launcher.py`'s two `build_zdr_prioritized_catalog` call sites now source their diff --git a/scripts/ci/repair_pr827_coderabbit_comments.py b/scripts/ci/repair_pr827_coderabbit_comments.py new file mode 100644 index 0000000000..8b1df14cb1 --- /dev/null +++ b/scripts/ci/repair_pr827_coderabbit_comments.py @@ -0,0 +1,460 @@ +#!/usr/bin/env python3 +"""Apply the verified CodeRabbit repairs for pull request 827.""" + +from __future__ import annotations + +from pathlib import Path + + +def replace_once( + path: str, old: str, new: str, *, allow_repeated: bool = False +) -> None: + """Replace one exact fragment, optionally tolerating repeated history markers.""" + file_path = Path(path) + text = file_path.read_text(encoding="utf-8") + count = text.count(old) + if count == 0 and ( + text.count(new) == 1 or (allow_repeated and text.count(new) > 0) + ): + return + if count == 0 or (count != 1 and not allow_repeated): + raise SystemExit(f"{path}: expected one replacement marker, found {count}") + file_path.write_text(text.replace(old, new, 1), encoding="utf-8") + + +def insert_before(path: str, anchor: str, addition: str) -> None: + """Insert an addition before one unique anchor, at most once.""" + file_path = Path(path) + text = file_path.read_text(encoding="utf-8") + if addition in text: + return + count = text.count(anchor) + if count != 1: + raise SystemExit(f"{path}: expected one insertion anchor, found {count}") + file_path.write_text(text.replace(anchor, addition + anchor, 1), encoding="utf-8") + + +def insert_after(path: str, anchor: str, addition: str) -> None: + """Insert an addition after one unique anchor, at most once.""" + file_path = Path(path) + text = file_path.read_text(encoding="utf-8") + if addition in text: + return + count = text.count(anchor) + if count != 1: + raise SystemExit(f"{path}: expected one insertion anchor, found {count}") + file_path.write_text(text.replace(anchor, anchor + addition, 1), encoding="utf-8") + + +def replace_between(path: str, start: str, end: str, replacement: str) -> None: + """Replace a uniquely delimited source section idempotently.""" + file_path = Path(path) + text = file_path.read_text(encoding="utf-8") + start_index = text.find(start) + if start_index < 0 and text.count(replacement) == 1: + return + if start_index < 0 or text.find(start, start_index + 1) >= 0: + raise SystemExit(f"{path}: start marker missing or ambiguous") + end_index = text.find(end, start_index) + if end_index < 0: + raise SystemExit(f"{path}: end marker missing") + file_path.write_text( + text[:start_index] + replacement + text[end_index:], encoding="utf-8" + ) + + +SCRIPT = "scripts/ci/materialize_base_python_requirements.py" +TEST = "tests/test_materialize_base_python_requirements.py" +DOC = "docs/doctoring/opencode-rust-coverage-runtime-boundary.md" +WORKFLOW = ".github/workflows/opencode-review-dispatch.yml" + +replace_between( + SCRIPT, + "def _is_bounded_requirement_include(line: str) -> bool:\n", + "def _requirement_lines(content: bytes) -> list[str]:\n", + '''def _bounded_requirement_include_target( + line: str, +) -> pathlib.PurePosixPath | None: + """Return the safe relative target of one bounded requirements include. + + The target may use any normalized relative ``.txt`` name, including names + such as ``other-hashes.txt``. Eligibility does not confer trust: the exact + base-tree target must later be a regular blob containing only exact + SHA-256-pinned package requirements. + """ + fields = line.split() + if len(fields) != 2 or fields[0] not in {"-r", "--requirement"}: + return None + target = fields[1] + if ( + target.startswith(("-", "~")) + or "\\\\" in target + or ":" in target + or "?" in target + or "#" in target + ): + return None + include_path = pathlib.PurePosixPath(target) + if ( + not include_path.parts + or target != include_path.as_posix() + or include_path.is_absolute() + or "." in include_path.parts + or ".." in include_path.parts + or include_path.suffix != ".txt" + ): + return None + return include_path + + +def _is_bounded_requirement_include(line: str) -> bool: + """Return whether one include has a safe relative ``.txt`` target.""" + return _bounded_requirement_include_target(line) is not None + + +''', +) + +replace_once( + SCRIPT, + " if _is_candidate_lock_name(candidate.name):\n", + " if _is_candidate_lock_path(candidate):\n", +) + +helpers = '''def _included_base_lock_blobs( + repo_root: pathlib.Path, + base_sha: str, + source_path: str, + content: bytes, + regular_paths: set[str], +) -> list[tuple[pathlib.PurePosixPath, bytes]]: + """Load direct bounded includes from the exact base as complete closures.""" + source_parent = pathlib.PurePosixPath(source_path).parent + included: dict[pathlib.PurePosixPath, bytes] = {} + for line in _requirement_lines(content): + target = _bounded_requirement_include_target(line) + if target is None: + continue + resolved = source_parent / target + resolved_path = resolved.as_posix() + if resolved_path not in regular_paths: + raise RuntimeError( + f"bounded include {target} from {source_path} is not a regular base blob" + ) + included_content = _git(repo_root, "show", f"{base_sha}:{resolved_path}") + if not _is_flat_materializable_lock(included_content): + raise RuntimeError( + f"bounded include {resolved_path} must contain only exact SHA-256 pins" + ) + included[target] = included_content + return sorted(included.items(), key=lambda item: item[0].as_posix()) + + +def _rewrite_materialized_includes( + content: bytes, include_directory: str, source_path: str = "" +) -> bytes: + """Rewrite root include targets to their preserved generated subtree.""" + try: + text = content.decode("utf-8", errors="strict") + except UnicodeDecodeError as exc: + raise RuntimeError(f"base lock {source_path} is not valid UTF-8") from exc + rewritten: list[str] = [] + for raw_line in text.splitlines(keepends=True): + body = raw_line.rstrip("\\r\\n") + ending = raw_line[len(body) :] + stripped = body.strip() + target = _bounded_requirement_include_target(stripped) + if target is None: + rewritten.append(raw_line) + continue + indentation = body[: len(body) - len(body.lstrip())] + option = stripped.split()[0] + rewritten.append( + f"{indentation}{option} {include_directory}/{target.as_posix()}{ending}" + ) + return "".join(rewritten).encode("utf-8") + + +''' +insert_before(SCRIPT, "def materialize(\n", helpers) + +replace_between( + SCRIPT, + "def materialize(\n", + "def main(argv: list[str] | None = None) -> int:\n", + '''def materialize( + repo_root: pathlib.Path, + base_sha: str, + output_dir: pathlib.Path, +) -> list[dict[str, str]]: + """Write base locks and resolvable bounded includes into a safe context.""" + if output_dir.exists() and output_dir.is_symlink(): + raise ValueError("output directory must not be a symlink") + output_dir.mkdir(parents=True, exist_ok=True) + + resolved_repo = repo_root.resolve() + entries = _git(resolved_repo, "ls-tree", "-r", "-z", "--full-tree", base_sha) + regular_paths = { + path for path, _candidate in _regular_base_blob_paths(entries) + } + manifest: list[dict[str, str]] = [] + for index, (source_path, content) in enumerate( + base_hash_locks(resolved_repo, base_sha) + ): + generated_name = f"requirements-{index:03d}.txt" + include_directory = f"includes-{index:03d}" + included = _included_base_lock_blobs( + resolved_repo, + base_sha, + source_path, + content, + regular_paths, + ) + for relative_target, included_content in included: + destination = output_dir / include_directory / Path(*relative_target.parts) + destination.parent.mkdir(parents=True, exist_ok=True) + destination.write_bytes(included_content) + destination = output_dir / generated_name + destination.write_bytes( + _rewrite_materialized_includes(content, include_directory, source_path) + ) + manifest.append({"file": generated_name, "source": source_path}) + + (output_dir / "manifest.json").write_text( + json.dumps(manifest, indent=2, sort_keys=True) + "\\n", + encoding="utf-8", + ) + (output_dir / "manifest.txt").write_text( + "".join(f"{entry['file']}\\n" for entry in manifest), + encoding="utf-8", + ) + return manifest + + +''', +) + +insert_before(TEST, "from pathlib import Path\n", "import zipfile\n") +replace_once( + TEST, + ' assert not materializer._is_hash_pinned(b"-r other-hashes.txt\\n")\n', + ' assert materializer._is_hash_pinned(b"-r other-hashes.txt\\n")\n', +) +insert_after( + TEST, + ' assert not materializer._is_candidate_lock_name("pyproject.toml")\n', + ' assert materializer._is_candidate_lock_path(\n' + ' materializer.pathlib.PurePosixPath("requirements/ci.txt")\n' + ' )\n' + ' assert materializer._is_candidate_lock_path(\n' + ' materializer.pathlib.PurePosixPath("service/requirements/package.txt")\n' + ' )\n' + ' assert not materializer._is_candidate_lock_path(\n' + ' materializer.pathlib.PurePosixPath("service/config/ci.txt")\n' + ' )\n', +) +insert_after( + TEST, + ' (repo / "requirements-test.txt").write_text(\n' + ' "hypothesis==6 --hash=sha256:" + ("b" * 64) + "\\n",\n' + ' encoding="utf-8",\n' + ' )\n', + ' requirements_dir = repo / "requirements"\n' + ' requirements_dir.mkdir()\n' + ' (requirements_dir / "ci.txt").write_text(\n' + ' "pytest==9 --hash=sha256:" + ("c" * 64) + "\\n",\n' + ' encoding="utf-8",\n' + ' )\n', +) +replace_once( + TEST, + ' "requirements-test.txt",\n' + ' "services/account_unification/requirements-dev.txt",\n', + ' "requirements-test.txt",\n' + ' "requirements/ci.txt",\n' + ' "services/account_unification/requirements-dev.txt",\n', +) +insert_before( + TEST, + ' between_file.write_text("START old", encoding="utf-8")\n', + ''' before_file = tmp_path / "before.txt" + before_file.write_text("ANCHOR", encoding="utf-8") + insert_before = namespace["insert_before"] + insert_before(str(before_file), "ANCHOR", "PREFIX ") # type: ignore[operator] + assert before_file.read_text(encoding="utf-8") == "PREFIX ANCHOR" + insert_before(str(before_file), "ANCHOR", "PREFIX ") # type: ignore[operator] + with pytest.raises(SystemExit, match="expected one insertion anchor"): + insert_before(str(before_file), "MISSING", "OTHER ") # type: ignore[operator] + after_file = tmp_path / "after.txt" + after_file.write_text("ANCHOR", encoding="utf-8") + insert_after = namespace["insert_after"] + insert_after(str(after_file), "ANCHOR", " SUFFIX") # type: ignore[operator] + assert after_file.read_text(encoding="utf-8") == "ANCHOR SUFFIX" + insert_after(str(after_file), "ANCHOR", " SUFFIX") # type: ignore[operator] + with pytest.raises(SystemExit, match="expected one insertion anchor"): + insert_after(str(after_file), "MISSING", " OTHER") # type: ignore[operator] +''', +) + +integration_test = '''def test_materialized_bounded_include_is_resolvable_by_pip(tmp_path: Path) -> None: + """A safe base-owned include survives flattening and pip hash preflight.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + + wheel_dir = tmp_path / "wheels" + wheel_dir.mkdir() + wheel = wheel_dir / "demo-1-py3-none-any.whl" + with zipfile.ZipFile(wheel, "w") as archive: + archive.writestr("demo/__init__.py", "__version__ = '1'\\n") + archive.writestr( + "demo-1.dist-info/METADATA", + "Metadata-Version: 2.1\\nName: demo\\nVersion: 1\\n", + ) + archive.writestr( + "demo-1.dist-info/WHEEL", + "Wheel-Version: 1.0\\nGenerator: TEPP-test\\n" + "Root-Is-Purelib: true\\nTag: py3-none-any\\n", + ) + archive.writestr("demo-1.dist-info/RECORD", "") + digest = hashlib.sha256(wheel.read_bytes()).hexdigest() + + (repo / "requirements.txt").write_text( + "-r other-hashes.txt\\n", encoding="utf-8" + ) + (repo / "other-hashes.txt").write_text( + f"--require-hashes\\ndemo==1 --hash=sha256:{digest}\\n", encoding="utf-8" + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "base") + base_sha = git(repo, "rev-parse", "HEAD") + + output = tmp_path / "output" + manifest = materializer.materialize(repo, base_sha, output) + assert manifest == [{"file": "requirements-000.txt", "source": "requirements.txt"}] + assert (output / "requirements-000.txt").read_text(encoding="utf-8") == ( + "-r includes-000/other-hashes.txt\\n" + ) + assert (output / "includes-000" / "other-hashes.txt").is_file() + + completed = subprocess.run( + [ + sys.executable, + "-m", + "pip", + "install", + "--dry-run", + "--ignore-installed", + "--disable-pip-version-check", + "--no-index", + "--find-links", + str(wheel_dir), + "--require-hashes", + "-r", + str(output / "requirements-000.txt"), + ], + check=False, + capture_output=True, + text=True, + ) + assert completed.returncode == 0, completed.stdout + completed.stderr + + +def test_materialization_rejects_missing_or_nested_include(tmp_path: Path) -> None: + """Includes must resolve to direct complete hash closures in the exact base.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "requirements.txt").write_text("-r child.txt\\n", encoding="utf-8") + git(repo, "add", ".") + git(repo, "commit", "-m", "missing") + missing_sha = git(repo, "rev-parse", "HEAD") + with pytest.raises(RuntimeError, match="not a regular base blob"): + materializer.materialize(repo, missing_sha, tmp_path / "missing-output") + + (repo / "child.txt").write_text("-r grandchild.txt\\n", encoding="utf-8") + (repo / "grandchild.txt").write_text( + "demo==1 --hash=sha256:" + ("d" * 64) + "\\n", encoding="utf-8" + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "nested") + nested_sha = git(repo, "rev-parse", "HEAD") + with pytest.raises(RuntimeError, match="must contain only exact SHA-256 pins"): + materializer.materialize(repo, nested_sha, tmp_path / "nested-output") + + with pytest.raises(RuntimeError, match="base lock requirements.txt is not valid UTF-8"): + materializer._rewrite_materialized_includes( + b"\\xff", "includes-000", "requirements.txt" + ) + + +''' +insert_before(TEST, "def test_rejects_invalid_base_sha", integration_test) +replace_once( + TEST, + ' assert not materializer._is_bounded_requirement_include("-r /abs/requirements.txt")\n', + ' assert not materializer._is_bounded_requirement_include("-r /abs/requirements.txt")\n' + ' assert not materializer._is_bounded_requirement_include("-r pyproject.toml")\n', +) +replace_once( + "tests/test_opencode_rust_coverage_toolchain_contract.py", + """ assert f'"${{LLVM_COV:-}}" != "$LLVM_COV_PATH"' in helper + assert f'"${{LLVM_PROFDATA:-}}" != "$LLVM_PROFDATA_PATH"' in helper +""", + """ assert '"${LLVM_COV:-}" != "$LLVM_COV_PATH"' in helper + assert '"${LLVM_PROFDATA:-}" != "$LLVM_PROFDATA_PATH"' in helper +""", +) +insert_after( + "tests/test_opencode_rust_coverage_toolchain_contract.py", + " for relative_path in watched_paths:\n" + " assert (_REPOSITORY_ROOT / relative_path).is_file(), relative_path\n", + """ doctoring = ( + _REPOSITORY_ROOT + / "docs/doctoring/opencode-rust-coverage-runtime-boundary.md" + ).read_text(encoding="utf-8") + assert "/usr/bin/llvm-cov-19" in doctoring + assert "/usr/bin/llvm-profdata-19" in doctoring + assert "unversioned `llvm-cov`" in doctoring + assert "fails closed" in doctoring +""", +) + +replace_once( + "CHANGELOG.md", + "- 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.\n", + "- Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. Includes such as `-r other-hashes.txt` remain allowed when the exact base-tree target is a regular, complete SHA-256-pinned closure; a lone `--require-hashes` directive, dotted `./lock.txt`, traversal, absolute, URL, and option-like targets fail closed.\n", + allow_repeated=True, +) + +replace_once( + DOC, + "These are compatibility and trust-boundary constants, not caller-selectable\nconfiguration. The reviewed helper `scripts/ci/ensure_rust_llvm19.sh` binds both\nexact paths and fails closed unless the live `LLVM_COV` / `LLVM_PROFDATA`\nvalues match and are executable before Rust coverage evidence is admitted. The\nindependent OpenCode review-dispatch workflow stays byte-for-byte so the\nreview-agent key system is not rewritten to carry this runtime check.\n", + "These are compatibility and trust-boundary constants, not caller-selectable\nconfiguration. The reviewed helper `scripts/ci/ensure_rust_llvm19.sh` validates\nboth exact paths and fails closed unless the live `LLVM_COV` / `LLVM_PROFDATA`\nvalues match and are executable before Rust coverage evidence is admitted. The\nactual environment binding is owned by\n`.github/workflows/opencode-review-dispatch.yml`, through its Dockerfile `ENV`\ndeclarations and the isolated container's `docker run --env` arguments. If that\nworkflow changes, its `REVIEW_DISPATCH_BLOB_SHA` pin must change with it; this\ndoes not rewrite the review-agent key system.\n", +) +replace_once( + DOC, + "NIST SP 800-218 PW.4.1 requires third-party software to come from expected,\ntrusted sources with integrity verification (Souppaya et al., 2022). Binding\ncoverage to the reviewed `/usr/bin/llvm-cov-19` and\n`/usr/bin/llvm-profdata-19` executables is that verification; an ambient\n`PATH` lookup would treat a runner-image change as a new producer.\n", + "NIST SP 800-218 PW.4.1 covers acquiring and maintaining third-party software\nfrom expected, trusted sources and reviewing its provenance (Souppaya et al.,\n2022). PW.4.4 covers verifying the integrity of acquired components. The exact\n`/usr/bin/llvm-cov-19` and `/usr/bin/llvm-profdata-19` bindings are\nproducer-selection controls: they select reviewed paths and `test -x` verifies\nexecutability. They do not hash or signature-verify the Debian package or binary;\npackage/image hashes, signatures, repository metadata, and attestations remain\nseparate PW.4.4 integrity controls and must not be inferred from path equality.\n", +) +replace_once( + DOC, + "Debian bookworm currently publishes the versioned `llvm-19` package from\n`llvm-toolchain-19`; Debian package file inventories expose versioned LLVM 19\ntool entry points including `llvm-cov-19`. Pinning the reviewed executable names\ninside the image converts that mutable ambient dependency into an explicit\ncontract that can be checked before source execution.\n", + "Debian publishes `llvm-19` from the `llvm-toolchain-19` source package; its\nofficial copyright record states `Apache-2.0 WITH LLVM-exception`. Debian package\nfile inventories expose versioned LLVM 19 tool entry points including\n`llvm-cov-19`. Pinning those reviewed executable names inside the image converts\nambient path selection into an explicit, testable producer contract; the Debian\ncopyright record supplies the package license basis, not executable integrity.\n", +) +replace_once( + DOC, + "Debian Project. (2026). *File list of package llvm-19*. Debian Packages.\nRetrieved August 10, 2026, from\nhttps://packages.debian.org/bookworm/amd64/llvm-19/filelist\n\n", + "Debian Project. (2026). *File list of package llvm-19*. Debian Packages.\nRetrieved August 10, 2026, from\nhttps://packages.debian.org/bookworm/amd64/llvm-19/filelist\n\nDebian Project. (2026). *Copyright file for llvm-toolchain-19 19.1.7-20*.\nDebian FTP Masters. Retrieved August 15, 2026, from\nhttps://metadata.ftp-master.debian.org/changelogs/main/l/llvm-toolchain-19/llvm-toolchain-19_19.1.7-20_copyright\n\n", +) + +replace_once( + WORKFLOW, + " RUN apt-get update \\\n && apt-get install --no-install-recommends -y \\\n", + " # llvm-19 / llvm-toolchain-19: Apache-2.0 WITH LLVM-exception. See docs/doctoring/opencode-rust-coverage-runtime-boundary.md.\n" + " RUN apt-get update \\\n && apt-get install --no-install-recommends -y \\\n", +) diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index a2da04ae25..58ded37400 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -1,9 +1,11 @@ from __future__ import annotations +import ast import hashlib import io import json import runpy +import shutil import subprocess import sys import tarfile @@ -396,6 +398,114 @@ def test_materialization_rejects_missing_or_nested_include(tmp_path: Path) -> No ) +def test_bounded_repair_driver_runs_against_a_staged_fixture( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The one-shot repair driver applies every guarded edit in isolation.""" + repository_root = Path(__file__).parents[1] + relative_files = ( + "scripts/ci/repair_pr827_coderabbit_comments.py", + "scripts/ci/materialize_base_python_requirements.py", + "tests/test_materialize_base_python_requirements.py", + "tests/test_opencode_rust_coverage_toolchain_contract.py", + "docs/doctoring/opencode-rust-coverage-runtime-boundary.md", + ".github/workflows/opencode-review-dispatch.yml", + "CHANGELOG.md", + ) + for relative_file in relative_files: + source = repository_root / relative_file + destination = tmp_path / relative_file + destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(source, destination) + + changelog = tmp_path / "CHANGELOG.md" + changelog.write_text( + changelog.read_text(encoding="utf-8").replace( + "## [Unreleased]\n", + "## [Unreleased]\n\n" + "- 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.\n", + 1, + ), + encoding="utf-8", + ) + + monkeypatch.chdir(tmp_path) + runpy.run_path( + str(repository_root / "scripts/ci/repair_pr827_coderabbit_comments.py"), + run_name="__main__", + ) + + materializer_source = ( + tmp_path / "scripts/ci/materialize_base_python_requirements.py" + ).read_text(encoding="utf-8") + assert "def _bounded_requirement_include_target(" in materializer_source + assert "def _included_base_lock_blobs(" in materializer_source + assert "includes-000/" in ( + tmp_path / "tests/test_materialize_base_python_requirements.py" + ).read_text(encoding="utf-8") + assert "Apache-2.0 WITH LLVM-exception" in ( + tmp_path / "docs/doctoring/opencode-rust-coverage-runtime-boundary.md" + ).read_text(encoding="utf-8") + + script_path = repository_root / "scripts/ci/repair_pr827_coderabbit_comments.py" + tree = ast.parse(script_path.read_text(encoding="utf-8"), filename=str(script_path)) + definitions = [node for node in tree.body if isinstance(node, ast.FunctionDef)] + namespace: dict[str, object] = {"Path": Path} + exec( + compile( + ast.fix_missing_locations(ast.Module(body=definitions, type_ignores=[])), + str(script_path), + "exec", + ), + namespace, + ) + replace_once = namespace["replace_once"] + replace_between = namespace["replace_between"] + once_file = tmp_path / "once.txt" + once_file.write_text("old", encoding="utf-8") + replace_once(str(once_file), "old", "new") # type: ignore[operator] + assert once_file.read_text(encoding="utf-8") == "new" + with pytest.raises(SystemExit, match="expected one replacement marker"): + replace_once(str(once_file), "missing", "other") # type: ignore[operator] + repeated_file = tmp_path / "repeated.txt" + repeated_file.write_text("oldold", encoding="utf-8") + replace_once( # type: ignore[operator] + str(repeated_file), "old", "new", allow_repeated=True + ) + assert repeated_file.read_text(encoding="utf-8") == "newold" + + between_file = tmp_path / "between.txt" + between_file.write_text("START old END", encoding="utf-8") + replace_between(str(between_file), "START", "END", "START new ") # type: ignore[operator] + assert between_file.read_text(encoding="utf-8") == "START new END" + replace_between(str(between_file), "MISSING", "END", "START new ") # type: ignore[operator] + before_file = tmp_path / "before.txt" + before_file.write_text("ANCHOR", encoding="utf-8") + insert_before = namespace["insert_before"] + insert_before(str(before_file), "ANCHOR", "PREFIX ") # type: ignore[operator] + assert before_file.read_text(encoding="utf-8") == "PREFIX ANCHOR" + insert_before(str(before_file), "ANCHOR", "PREFIX ") # type: ignore[operator] + with pytest.raises(SystemExit, match="expected one insertion anchor"): + insert_before(str(before_file), "MISSING", "OTHER ") # type: ignore[operator] + after_file = tmp_path / "after.txt" + after_file.write_text("ANCHOR", encoding="utf-8") + insert_after = namespace["insert_after"] + insert_after(str(after_file), "ANCHOR", " SUFFIX") # type: ignore[operator] + assert after_file.read_text(encoding="utf-8") == "ANCHOR SUFFIX" + insert_after(str(after_file), "ANCHOR", " SUFFIX") # type: ignore[operator] + with pytest.raises(SystemExit, match="expected one insertion anchor"): + insert_after(str(after_file), "MISSING", " OTHER") # type: ignore[operator] + between_file.write_text("START old START END", encoding="utf-8") + with pytest.raises(SystemExit, match="start marker missing or ambiguous"): + replace_between(str(between_file), "START", "END", "replacement") # type: ignore[operator] + between_file.write_text("START old", encoding="utf-8") + with pytest.raises(SystemExit, match="end marker missing"): + replace_between(str(between_file), "START", "END", "replacement") # type: ignore[operator] + + def test_rejects_invalid_base_sha(tmp_path: Path) -> None: """Git options and symbolic refs cannot cross the exact-SHA boundary.""" with pytest.raises(ValueError, match="40 hexadecimal"): From 4323e2706660c2404fbf57722920a9516f922c63 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 09:34:53 +0000 Subject: [PATCH 3/4] fix(ci): bound required-workflow-bootstrap awk extraction to its own job Ports the identical fix from #1506 into this branch. This PR's exact-head-path-policy check runs its own head-branch copy of scripts/ci/test_strix_quick_gate.sh (plain `pull_request` trigger in strix-changed-path-quality-ci.yml, not pull_request_target), so the pre-existing main-branch bug is not fixed here just by #1506 merging into main -- it needs porting into this branch directly. Root cause: assert_opencode_review_uses_codegraph_and_contextual_orchestrator extracted the required-workflow-bootstrap job block from opencode-review.yml with awk '/^ required-workflow-bootstrap:$/,/^[^ ]/'. Every job key in that workflow is indented 2 spaces (never column 0), so the end pattern never matched until EOF, sweeping an unrelated `if:` line from a later job (added by already-merged PR #1497) into the "block" and failing the assertion on unrelated content. Fixed by using an explicit state flag so the end pattern (`^ [A-Za-z0-9_-]+:`) is only tested starting on the line after the start match, correctly bounding the block to just its own lines. See ContextualWisdomLab/.github#1506 for the full root-cause writeup and validation against origin/main. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw --- scripts/ci/test_strix_quick_gate.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 4053f4fd53..1fc45a34b9 100644 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -522,7 +522,7 @@ assert_opencode_review_uses_codegraph_and_contextual_orchestrator() { assert_file_not_contains "$workflow_file" "Wait for trusted OpenCode approval review" "opencode pull_request bridge was removed to avoid duplicate required-check resource use" assert_file_not_contains "$workflow_file" "Trusted OpenCode requested changes for head" "opencode pull_request bridge no longer reconsumes stale trusted review state" assert_file_not_contains "$workflow_file" "github.event.pull_request.number == 240" "opencode review workflow must not hard-code repository-specific PR bypasses" - if awk '/^ required-workflow-bootstrap:$/,/^[^ ]/' "$bootstrap_file" | grep -q '^[[:space:]]*if:'; then + if awk '/^ required-workflow-bootstrap:$/{p=1; print; next} p && /^ [A-Za-z0-9_-]+:/{exit} p' "$bootstrap_file" | grep -q '^[[:space:]]*if:'; then record_failure "opencode required workflow bootstrap must not depend on required-workflow event payload fields" fi assert_file_contains "$workflow_file" 'github.event.client_payload.target_repository || github.repository' "opencode review scopes concurrency by target repository" From afa21c8987186c0e37eaa4a275183efcdc214ea4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 11:22:40 +0000 Subject: [PATCH 4/4] fix(ci): remove grep -q from test_strix_quick_gate.sh pipeline checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit grep -q exits on first match and closes its end of the pipe; if the upstream awk is still writing a large block, it gets SIGPIPE (141). Under `set -o pipefail` that non-zero awk status wins over grep's real 0, so `if pipeline; then` sees the pipeline as failed even though grep found a genuine match — silently missing e.g. a forbidden `if:` key or a fenced-diff marker that should have failed the check. Ports the same-file fix from PR #1506 to this branch's two call sites (required-workflow-bootstrap job-block check; opencode review REQUEST_CHANGES fenced-diff check). This branch already carried #1506's awk job-block-boundary correction, so only the grep -q removal was needed here. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw --- scripts/ci/test_strix_quick_gate.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 1fc45a34b9..a92871b7ce 100644 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -522,7 +522,7 @@ assert_opencode_review_uses_codegraph_and_contextual_orchestrator() { assert_file_not_contains "$workflow_file" "Wait for trusted OpenCode approval review" "opencode pull_request bridge was removed to avoid duplicate required-check resource use" assert_file_not_contains "$workflow_file" "Trusted OpenCode requested changes for head" "opencode pull_request bridge no longer reconsumes stale trusted review state" assert_file_not_contains "$workflow_file" "github.event.pull_request.number == 240" "opencode review workflow must not hard-code repository-specific PR bypasses" - if awk '/^ required-workflow-bootstrap:$/{p=1; print; next} p && /^ [A-Za-z0-9_-]+:/{exit} p' "$bootstrap_file" | grep -q '^[[:space:]]*if:'; then + if awk '/^ required-workflow-bootstrap:$/{p=1; print; next} p && /^ [A-Za-z0-9_-]+:/{exit} p' "$bootstrap_file" | grep '^[[:space:]]*if:' >/dev/null; then record_failure "opencode required workflow bootstrap must not depend on required-workflow event payload fields" fi assert_file_contains "$workflow_file" 'github.event.client_payload.target_repository || github.repository' "opencode review scopes concurrency by target repository" @@ -1501,7 +1501,7 @@ assert_opencode_review_posts_suggested_diffs_inline() { assert_file_contains "$workflow_file" "publish_request_changes_from_control" "opencode review REQUEST_CHANGES path publishes findings from the control JSON" if awk '/format_request_changes_body\(\)/,/build_request_changes_review_payload\(\)/ { print }' "$workflow_file" | - grep -Fq '```diff'; then + grep -F '```diff' >/dev/null; then record_failure "opencode review PR-level REQUEST_CHANGES body must not contain fenced suggested diffs" fi }