[fix]. Harden fork PR fetch + config marker
This commit is contained in:
@@ -21,6 +21,8 @@ class PullRequestContext:
|
|||||||
clone_url: str
|
clone_url: str
|
||||||
html_url: str
|
html_url: str
|
||||||
is_fork: bool
|
is_fork: bool
|
||||||
|
base_clone_url: str | None = None
|
||||||
|
head_clone_url: str | None = None
|
||||||
|
|
||||||
|
|
||||||
class GiteaClient:
|
class GiteaClient:
|
||||||
@@ -56,6 +58,8 @@ class GiteaClient:
|
|||||||
encoded_owner = quote(owner, safe="")
|
encoded_owner = quote(owner, safe="")
|
||||||
encoded_name = quote(name, safe="")
|
encoded_name = quote(name, safe="")
|
||||||
payload = self._request("GET", f"/api/v1/repos/{encoded_owner}/{encoded_name}/pulls/{pr_number}")
|
payload = self._request("GET", f"/api/v1/repos/{encoded_owner}/{encoded_name}/pulls/{pr_number}")
|
||||||
|
base_clone_url = payload["base"]["repo"]["clone_url"]
|
||||||
|
head_clone_url = payload["head"]["repo"]["clone_url"]
|
||||||
return PullRequestContext(
|
return PullRequestContext(
|
||||||
repo=repo,
|
repo=repo,
|
||||||
pr_number=pr_number,
|
pr_number=pr_number,
|
||||||
@@ -63,7 +67,9 @@ class GiteaClient:
|
|||||||
base_sha=payload["base"]["sha"],
|
base_sha=payload["base"]["sha"],
|
||||||
head_ref=payload["head"]["ref"],
|
head_ref=payload["head"]["ref"],
|
||||||
head_sha=payload["head"]["sha"],
|
head_sha=payload["head"]["sha"],
|
||||||
clone_url=payload["head"]["repo"]["clone_url"],
|
clone_url=head_clone_url,
|
||||||
|
base_clone_url=base_clone_url,
|
||||||
|
head_clone_url=head_clone_url,
|
||||||
html_url=payload["html_url"],
|
html_url=payload["html_url"],
|
||||||
is_fork=bool(payload["head"]["repo"]["full_name"] != payload["base"]["repo"]["full_name"]),
|
is_fork=bool(payload["head"]["repo"]["full_name"] != payload["base"]["repo"]["full_name"]),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -41,6 +41,8 @@ def format_result_comment(head_sha: str, result: dict, *, repo_configured: bool
|
|||||||
body = markdown_comment.strip()
|
body = markdown_comment.strip()
|
||||||
if usage_note:
|
if usage_note:
|
||||||
body = f"{body}\n\n{usage_note}"
|
body = f"{body}\n\n{usage_note}"
|
||||||
|
if missing_config_note:
|
||||||
|
body = f"{body}\n\n{missing_config_note}"
|
||||||
return _inject_head_sha_marker(head_sha, body)
|
return _inject_head_sha_marker(head_sha, body)
|
||||||
|
|
||||||
verdict = result.get("verdict", "has_issues")
|
verdict = result.get("verdict", "has_issues")
|
||||||
|
|||||||
@@ -152,7 +152,17 @@ def _build_install_and_run_command(
|
|||||||
'auth_b64="$(printf "%s" "${GITEA_GIT_USERNAME}:${GITEA_TOKEN}" | base64 | tr -d \'\\n\')"',
|
'auth_b64="$(printf "%s" "${GITEA_GIT_USERNAME}:${GITEA_TOKEN}" | base64 | tr -d \'\\n\')"',
|
||||||
f'git -c http.extraHeader="Authorization: Basic $auth_b64" clone --no-tags --depth 80 {shlex.quote(pr.clone_url)} /work/repo',
|
f'git -c http.extraHeader="Authorization: Basic $auth_b64" clone --no-tags --depth 80 {shlex.quote(pr.clone_url)} /work/repo',
|
||||||
"cd /work/repo",
|
"cd /work/repo",
|
||||||
f'git -c http.extraHeader="Authorization: Basic $auth_b64" fetch --no-tags origin {shlex.quote(pr.base_ref)} {shlex.quote(pr.head_ref)}',
|
"fetch_required() { "
|
||||||
|
"remote=\"$1\"; ref=\"$2\"; sha=\"$3\"; label=\"$4\"; "
|
||||||
|
"if git -c http.extraHeader=\"Authorization: Basic $auth_b64\" fetch --no-tags \"$remote\" \"$ref\"; then return 0; fi; "
|
||||||
|
"if git -c http.extraHeader=\"Authorization: Basic $auth_b64\" fetch --no-tags \"$remote\" \"$sha\"; then return 0; fi; "
|
||||||
|
"echo \"Failed to fetch $label from remote '$remote' using ref '$ref' or sha '$sha'\" >&2; "
|
||||||
|
"return 7; "
|
||||||
|
"}",
|
||||||
|
f"base_remote={'upstream' if pr.base_clone_url and pr.base_clone_url != pr.clone_url else 'origin'}",
|
||||||
|
f"if [ \"$base_remote\" = \"upstream\" ]; then git remote add upstream {shlex.quote(pr.base_clone_url or '')}; fi",
|
||||||
|
f"fetch_required origin {shlex.quote(pr.head_ref)} {shlex.quote(pr.head_sha)} head",
|
||||||
|
f"fetch_required \"$base_remote\" {shlex.quote(pr.base_ref)} {shlex.quote(pr.base_sha)} base",
|
||||||
f"git checkout --detach {shlex.quote(pr.head_sha)}",
|
f"git checkout --detach {shlex.quote(pr.head_sha)}",
|
||||||
'resolved_head="$(git rev-parse HEAD)"',
|
'resolved_head="$(git rev-parse HEAD)"',
|
||||||
f'if [ "$resolved_head" != {shlex.quote(pr.head_sha)} ]; then echo "Checked out SHA mismatch: expected {pr.head_sha}, got $resolved_head" >&2; exit 9; fi',
|
f'if [ "$resolved_head" != {shlex.quote(pr.head_sha)} ]; then echo "Checked out SHA mismatch: expected {pr.head_sha}, got $resolved_head" >&2; exit 9; fi',
|
||||||
|
|||||||
@@ -36,6 +36,22 @@ def _sample_pr() -> PullRequestContext:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _sample_fork_pr() -> PullRequestContext:
|
||||||
|
return PullRequestContext(
|
||||||
|
repo="acme/repo",
|
||||||
|
pr_number=2,
|
||||||
|
base_ref="main",
|
||||||
|
base_sha="c" * 40,
|
||||||
|
head_ref="feature",
|
||||||
|
head_sha="d" * 40,
|
||||||
|
clone_url="https://gitea.test/fork/repo.git",
|
||||||
|
base_clone_url="https://gitea.test/acme/repo.git",
|
||||||
|
head_clone_url="https://gitea.test/fork/repo.git",
|
||||||
|
html_url="https://gitea.test/acme/repo/pulls/2",
|
||||||
|
is_fork=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_build_docker_command_api_key_mode_uses_openai_env() -> None:
|
def test_build_docker_command_api_key_mode_uses_openai_env() -> None:
|
||||||
settings = get_settings()
|
settings = get_settings()
|
||||||
|
|
||||||
@@ -84,7 +100,10 @@ def test_build_install_command_chatgpt_mode_sets_git_checkout_and_review(monkeyp
|
|||||||
assert 'printf "%s" "$CODEX_AUTH_JSON_B64" | base64 -d > /root/.codex/auth.json' in command
|
assert 'printf "%s" "$CODEX_AUTH_JSON_B64" | base64 -d > /root/.codex/auth.json' in command
|
||||||
assert "git -c http.extraHeader=" in command
|
assert "git -c http.extraHeader=" in command
|
||||||
assert f"clone --no-tags --depth 80 {pr.clone_url} /work/repo" in command
|
assert f"clone --no-tags --depth 80 {pr.clone_url} /work/repo" in command
|
||||||
assert f"fetch --no-tags origin {pr.base_ref} {pr.head_ref}" in command
|
assert "fetch_required() {" in command
|
||||||
|
assert f"fetch_required origin {pr.head_ref} {pr.head_sha} head" in command
|
||||||
|
assert f"fetch_required \"$base_remote\" {pr.base_ref} {pr.base_sha} base" in command
|
||||||
|
assert "base_remote=origin" in command
|
||||||
assert f"git checkout --detach {pr.head_sha}" in command
|
assert f"git checkout --detach {pr.head_sha}" in command
|
||||||
assert "resolved_head=\"$(git rev-parse HEAD)\"" in command
|
assert "resolved_head=\"$(git rev-parse HEAD)\"" in command
|
||||||
assert "unset GITEA_TOKEN auth_b64" in command
|
assert "unset GITEA_TOKEN auth_b64" in command
|
||||||
@@ -107,6 +126,18 @@ def test_build_install_command_does_not_include_reasoning_effort_flag() -> None:
|
|||||||
assert "--reasoning-effort" not in command
|
assert "--reasoning-effort" not in command
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_install_command_uses_upstream_remote_for_fork_pr_base_fetch() -> None:
|
||||||
|
settings = get_settings()
|
||||||
|
pr = _sample_fork_pr()
|
||||||
|
|
||||||
|
command = _build_install_and_run_command(settings, pr=pr)
|
||||||
|
|
||||||
|
assert "base_remote=upstream" in command
|
||||||
|
assert f"git remote add upstream {pr.base_clone_url}" in command
|
||||||
|
assert f"fetch_required origin {pr.head_ref} {pr.head_sha} head" in command
|
||||||
|
assert f"fetch_required \"$base_remote\" {pr.base_ref} {pr.base_sha} base" in command
|
||||||
|
|
||||||
|
|
||||||
def test_chatgpt_mode_requires_existing_auth_json(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
def test_chatgpt_mode_requires_existing_auth_json(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
||||||
missing = tmp_path / "missing-auth.json"
|
missing = tmp_path / "missing-auth.json"
|
||||||
monkeypatch.setenv("CODEX_AUTH_MODE", "chatgpt")
|
monkeypatch.setenv("CODEX_AUTH_MODE", "chatgpt")
|
||||||
|
|||||||
@@ -71,7 +71,7 @@ def test_format_result_comment_appends_missing_config_note_for_system_layout() -
|
|||||||
assert body.endswith("> ℹ️.codex-review.yml is not configured")
|
assert body.endswith("> ℹ️.codex-review.yml is not configured")
|
||||||
|
|
||||||
|
|
||||||
def test_format_result_comment_does_not_append_missing_config_note_to_agent_markdown() -> None:
|
def test_format_result_comment_appends_missing_config_note_to_agent_markdown() -> None:
|
||||||
body = format_result_comment(
|
body = format_result_comment(
|
||||||
"ff0011",
|
"ff0011",
|
||||||
{
|
{
|
||||||
@@ -79,4 +79,4 @@ def test_format_result_comment_does_not_append_missing_config_note_to_agent_mark
|
|||||||
},
|
},
|
||||||
repo_configured=False,
|
repo_configured=False,
|
||||||
)
|
)
|
||||||
assert "> ℹ️.codex-review.yml is not configured" not in body
|
assert body.endswith("> ℹ️.codex-review.yml is not configured")
|
||||||
|
|||||||
Reference in New Issue
Block a user