From 6e83c7592c9cd9410653f78583f741a085fc4995 Mon Sep 17 00:00:00 2001 From: Simon Qi Date: Wed, 19 Aug 2026 18:10:09 +1200 Subject: [PATCH 1/3] fix: sync the vendored review bridge to the upstream that now carries it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The vendoring check has failed on every push since v0.7.0. `825da1a` added the review lane's session-cost harvest to the vendored copy of `tui_review_bridge.py` — 210 lines of it — without the change ever reaching the repository the file is vendored from, so `sync-bridge.sh` overwrote them and the diff that follows it never came back empty. ADR-0009 says this bridge is not owned here, and the check exists to hold that. Nothing was wrong with the check: the change was in the wrong repository. It now lives upstream, in review-switch#4, tested there at the seam that repository tests it at — the writer belongs to whichever consumer configured `--machine-log`, so what upstream pins is the argv the lane hands over, while the assertions here stay on the machine-log lines this repository's own writer produces. Upstream CI ran it on 3.11 through 3.14. So the pin moves to that merge commit, and the vendored copy is what `sync-bridge.sh` fetches from it. The 210 lines are byte-identical either way: the whole of this diff is the pin, in the script that holds it and in the comment header the script writes. The check has been red for four releases without stopping one, because it runs only on the 3.11 job and fails in four seconds while the three other jobs go green — worth knowing before trusting the tick beside a future release. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/sync-bridge.sh | 2 +- skills/crew/assets/review/scripts/tui_review_bridge.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/sync-bridge.sh b/scripts/sync-bridge.sh index 4dbd7c5..50918b2 100755 --- a/scripts/sync-bridge.sh +++ b/scripts/sync-bridge.sh @@ -3,7 +3,7 @@ set -euo pipefail UPSTREAM_REPOSITORY="https://github.com/okqixiaobao727-design/review-switch" UPSTREAM_RAW_BASE="https://raw.githubusercontent.com/okqixiaobao727-design/review-switch" -UPSTREAM_COMMIT="03f89b2400580c8998b2404bed12187effb94abe" +UPSTREAM_COMMIT="a5fdabada7eb628a476683b7e885f1478209159a" UPSTREAM_PATH="skills/review-switch-codex/scripts/tui_review_bridge.py" VENDORED_PATH="skills/crew/assets/review/scripts/tui_review_bridge.py" diff --git a/skills/crew/assets/review/scripts/tui_review_bridge.py b/skills/crew/assets/review/scripts/tui_review_bridge.py index af17643..82892c1 100644 --- a/skills/crew/assets/review/scripts/tui_review_bridge.py +++ b/skills/crew/assets/review/scripts/tui_review_bridge.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # Vendored from https://github.com/okqixiaobao727-design/review-switch -# Pinned upstream commit: 03f89b2400580c8998b2404bed12187effb94abe +# Pinned upstream commit: a5fdabada7eb628a476683b7e885f1478209159a # Changes belong upstream; update scripts/sync-bridge.sh when upgrading. """Interactive Codex TUI code-review channel for a Claude session. From ebcbc74253ecf3d69d0bbe2de7268659896db11a Mon Sep 17 00:00:00 2001 From: Simon Qi Date: Wed, 19 Aug 2026 18:12:14 +1200 Subject: [PATCH 2/3] docs(changelog): record the vendoring fix Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23eb8cd..46e0890 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ and this project uses [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Fixed +- The vendored review bridge is back in step with the repository it is vendored + from, so the check that holds ADR-0009 passes again. The review lane's + session-cost harvest was written straight into the vendored copy of + `tui_review_bridge.py` and never reached upstream, so `sync-bridge.sh` + overwrote those 210 lines on every run and the diff behind it never came back + empty — red on every push since 0.7.0. The check was right and the change was + in the wrong repository: it now lives upstream (review-switch#4), tested there + at the seam that repository tests, since the machine-log writer belongs to + whichever consumer configures `--machine-log` and what the lane owns is the + argv it hands over. The pin moves to that merge commit and the vendored copy + is what the script fetches from it, byte-identical to what was already here. + Worth knowing for the next release: this check runs only on the 3.11 job and + fails in four seconds, so it went red for four releases while the three other + jobs went green beside it. + ## [0.8.2] - 2026-08-19 ### Added From 3bbbb0b680f2a2f0c2cc12c17cc74a32faac9a10 Mon Sep 17 00:00:00 2001 From: Simon Qi Date: Wed, 19 Aug 2026 18:26:41 +1200 Subject: [PATCH 3/3] test: read the vendoring pin from the script that declares it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite kept its own copy of the pinned sha, so moving the pin turned a green suite red at a test that had found nothing: the script and the test disagreed about a constant only one of them owns. A pin is a fact about the script, and a second place to edit on every upgrade is a place that goes stale — silently, because a test asserting a sha nobody fetches proves nothing about the fetch. The four constants are now read out of `sync-bridge.sh`, which leaves the tests saying what they were always for: the sha the script fetches and the sha it writes into the vendored header are the same one, whatever it is. A script that stops declaring one fails the read rather than skipping the assertion. Verified by breaking it on purpose — a script whose header names a different sha than the one it fetches turns the suite red, so the assertion still bites. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_sync_bridge.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/tests/test_sync_bridge.py b/tests/test_sync_bridge.py index 1355bf3..c2a9e60 100644 --- a/tests/test_sync_bridge.py +++ b/tests/test_sync_bridge.py @@ -14,10 +14,28 @@ VENDORED_BRIDGE = pathlib.Path( "skills/crew/assets/review/scripts/tui_review_bridge.py" ) -UPSTREAM_REPOSITORY = "https://github.com/okqixiaobao727-design/review-switch" -UPSTREAM_RAW_BASE = "https://raw.githubusercontent.com/okqixiaobao727-design/review-switch" -UPSTREAM_COMMIT = "03f89b2400580c8998b2404bed12187effb94abe" -UPSTREAM_PATH = "skills/review-switch-codex/scripts/tui_review_bridge.py" + + +def pinned(name): + """One `NAME="value"` assignment, read from the script that owns it. + + The pin is the script's to declare, and a copy of it here would be a second + place to edit on every upgrade — one that goes stale silently, since a test + asserting a sha nobody fetches proves nothing. What these tests are for is + that the sha the script fetches and the sha it writes into the header are the + same one, whatever it is. + """ + for line in SYNC_SCRIPT.read_text(encoding="utf-8").splitlines(): + key, separator, value = line.partition("=") + if separator and key == name: + return value.strip().strip('"') + raise AssertionError(f"{SYNC_SCRIPT} declares no {name}") + + +UPSTREAM_REPOSITORY = pinned("UPSTREAM_REPOSITORY") +UPSTREAM_RAW_BASE = pinned("UPSTREAM_RAW_BASE") +UPSTREAM_COMMIT = pinned("UPSTREAM_COMMIT") +UPSTREAM_PATH = pinned("UPSTREAM_PATH") EXPECTED_SOURCE_URL = f"{UPSTREAM_RAW_BASE}/{UPSTREAM_COMMIT}/{UPSTREAM_PATH}" EXPECTED_HEADER = ( f"# Vendored from {UPSTREAM_REPOSITORY}\n"