From ee34fea1a2e78f741a246d196bd4a23a5fbe1e0b Mon Sep 17 00:00:00 2001 From: svtter Date: Thu, 4 Jun 2026 11:09:37 +0800 Subject: [PATCH] fix: review action no longer fails on opencode session-share push denial (#129) opencode's built-in 'share session' step runs `git add && git commit && git push` to the PR branch after the agent finishes. When the workflow declares `contents: read` (the documented read-only mode for review), the push fails with 403, causing the entire job to exit 1 even though the review comment was already posted successfully via the API. This was the issue reported in #129: the action advertises read-only operation but the job fails when the runner scope matches that contract. The wrapper now detects the specific push-denied markers in the captured log ("Write access to repository not granted", "Command failed with code 128: git push", or "fatal: unable to access ... error: 403") and converts the non-zero exit to 0 with a ::warning::. This is consistent with the existing cleanup-error-comments logic, which already auto-deletes the error comment that opencode posts in this scenario. Also: the fake opencode fixture gains FAKE_OPENCODE_PUSH_DENIED_MODELS so the test suite can exercise the new path. --- CHANGELOG.md | 5 +++ github-run-opencode/run-github-opencode.py | 52 +++++++++++++++++++++- tests/fixtures/fake-installer.sh | 8 ++++ tests/test_all.py | 37 +++++++++++++++ 4 files changed, 100 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d186ee8..38798ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ All notable changes to this project will be documented in this file. +## [Unreleased] + +### Fixed +- fix: review action no longer fails when opencode's internal session-share `git push` is denied (e.g. `contents: read`). Detects the 403 push pattern and exits 0, since the review comment was already posted via API. (#129) + ## [3.4.0] - 2026-06-04 ### Added diff --git a/github-run-opencode/run-github-opencode.py b/github-run-opencode/run-github-opencode.py index df310d7..6f1c080 100755 --- a/github-run-opencode/run-github-opencode.py +++ b/github-run-opencode/run-github-opencode.py @@ -160,19 +160,67 @@ def run_model(model: str, log_file: str, effective_timeout: int, run_script: Pat sys.stdout.buffer.write(result.stdout) sys.stdout.buffer.flush() + if result.returncode != 0 and _is_push_denied_failure(result.stdout.decode("utf-8", errors="replace")): + print( + "::warning::opencode session-share 'git push' was denied (e.g. contents:read). " + "The review comment was already posted via API; treating this as success.", + file=sys.stderr, + ) + return 0 + return result.returncode def run_single(run_script: Path, timeout_sec: int) -> int: if timeout_sec > 0: result = subprocess.run( - ["timeout", "--foreground", f"{timeout_sec}s", str(run_script)] + ["timeout", "--foreground", f"{timeout_sec}s", str(run_script)], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, ) else: - result = subprocess.run([str(run_script)]) + result = subprocess.run( + [str(run_script)], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + ) + sys.stdout.write(result.stdout) + sys.stdout.flush() + + if result.returncode != 0 and _is_push_denied_failure(result.stdout): + print( + "::warning::opencode session-share 'git push' was denied (e.g. contents:read). " + "The review comment was already posted via API; treating this as success.", + file=sys.stderr, + ) + return 0 + return result.returncode +# Specific markers emitted by opencode when its built-in session-share step +# tries to push the working tree back to the PR branch but the runner only +# has contents:read. The review itself succeeds (comment posted via the API) +# before this push runs, so the job should not fail. +PUSH_DENIED_PATTERNS = re.compile( + r"(Write access to repository not granted" + r"|Command failed with code 128: git push" + r"|fatal: unable to access ['\"][^'\"]+['\"]?: The requested URL returned error: 403)", + re.IGNORECASE, +) + + +def _is_push_denied_failure(content: str) -> bool: + """Return True if the only meaningful failure in the log is opencode's + built-in session-share ``git push`` being denied by the runner's token + scope (typically ``contents: read``). The review comment itself was + already posted via the API, so the job should exit 0 in this case. + """ + return bool(PUSH_DENIED_PATTERNS.search(content)) + + def compute_effective_timeout( model_timeout: int, global_timeout: int, start_time: float ) -> int: diff --git a/tests/fixtures/fake-installer.sh b/tests/fixtures/fake-installer.sh index a879700..70852e1 100644 --- a/tests/fixtures/fake-installer.sh +++ b/tests/fixtures/fake-installer.sh @@ -44,6 +44,14 @@ if [[ -n "${FAKE_OPENCODE_ERROR_MODELS:-}" ]] && contains_model "$FAKE_OPENCODE_ exit 23 fi +if [[ -n "${FAKE_OPENCODE_PUSH_DENIED_MODELS:-}" ]] && contains_model "$FAKE_OPENCODE_PUSH_DENIED_MODELS" "${MODEL:-}"; then + printf 'Pushing to local branch...\n' + printf 'Command failed with code 128: git push\n' + printf 'remote: Write access to repository not granted.\n' + printf 'fatal: unable to access '"'"'https://github.com/example/repo/'"'"': The requested URL returned error: 403\n' + exit 128 +fi + printf 'fake opencode %s\n' "$*" printf 'MODEL=%s\n' "${MODEL:-}" printf 'PROMPT=%s\n' "${PROMPT:-}" diff --git a/tests/test_all.py b/tests/test_all.py index d11dd4b..ff45c83 100644 --- a/tests/test_all.py +++ b/tests/test_all.py @@ -569,6 +569,7 @@ def tearDown(self): "FAKE_OPENCODE_TIMEOUT_MODELS", "FAKE_OPENCODE_TIMEOUT_SLEEP_SECONDS", "FAKE_OPENCODE_ERROR_MODELS", + "FAKE_OPENCODE_PUSH_DENIED_MODELS", "MODEL_NAME", ]: os.environ.pop(key, None) @@ -582,6 +583,7 @@ def reset_env(self): "FAKE_OPENCODE_TIMEOUT_MODELS", "FAKE_OPENCODE_TIMEOUT_SLEEP_SECONDS", "FAKE_OPENCODE_ERROR_MODELS", + "FAKE_OPENCODE_PUSH_DENIED_MODELS", "MODEL_NAME", ]: self.env.pop(key, None) @@ -783,6 +785,41 @@ def test_global_timeout_zero_disables_timeout(self): self.assertEqual(result.returncode, 0, f"stderr: {result.stderr}") self.assertNotIn("TIMEOUT_DURATION", result.stdout) + def test_push_denied_treated_as_success(self): + """opencode's session-share 'git push' failing with 403 (contents:read) + should NOT fail the job — the review comment was already posted.""" + self.reset_env() + result = self.run_wrapper( + FAKE_OPENCODE_PUSH_DENIED_MODELS="wrapper-model", + ) + self.assertEqual(result.returncode, 0, f"stderr: {result.stderr}") + self.assertIn("Command failed with code 128: git push", result.stdout) + self.assertIn("Write access to repository not granted", result.stdout) + self.assertIn("session-share 'git push' was denied", result.stderr) + + def test_push_denied_with_fallback_skips_fallback(self): + """If primary model hits the push-denied pattern, treat as success and + do NOT rotate to the fallback model.""" + self.reset_env() + result = self.run_wrapper( + GITHUB_RUN_OPENCODE_MODEL="wrapper-model", + GITHUB_RUN_OPENCODE_FALLBACK_MODELS="opencode-go/gemini-2.5-pro", + FAKE_OPENCODE_PUSH_DENIED_MODELS="wrapper-model", + ) + self.assertEqual(result.returncode, 0, f"stderr: {result.stderr}") + self.assertIn("session-share 'git push' was denied", result.stderr) + # The fallback model should not have been run + self.assertNotIn("MODEL=opencode-go/gemini-2.5-pro", result.stdout) + + def test_unrelated_error_not_treated_as_push_denied(self): + """A genuine non-push error must still propagate as a failure.""" + self.reset_env() + result = self.run_wrapper( + FAKE_OPENCODE_ERROR_MODELS="wrapper-model", + ) + self.assertNotEqual(result.returncode, 0) + self.assertIn("deadline exceeded", result.stdout) + class TestReviewAction(unittest.TestCase): """Tests for review action metadata."""