Skip to content

Commit a86601e

Browse files
Check conflicting pull requests, and never call an error safe
GitHub skips pull_request workflows while a pull request has merge conflicts, so the README's workflow now runs on pull_request_target, which also lets the Action comment on pull requests from forks. On that trigger the checkout is the base branch itself, which git refuses to fetch into, so the Action fetches the pull request's commits on their own and no longer hides a failed fetch. A pre-flight report that hit an error said "Safe" above it. It now says the command was not checked, in the text and the Markdown. Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
1 parent d3a7d66 commit a86601e

4 files changed

Lines changed: 17 additions & 9 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -926,7 +926,7 @@ Add this to your repo as `.github/workflows/git-sim.yml`:
926926
```yaml
927927
name: git-sim
928928
on:
929-
pull_request:
929+
pull_request_target:
930930
types: [opened, synchronize, reopened]
931931
permissions:
932932
contents: read
@@ -961,7 +961,7 @@ To change how it runs, add a `with:` block under the `uses:` line:
961961
| `package` | `git-sim` | What gets installed: the latest git-sim from PyPI, a pinned version like `git-sim==0.4.0`, or a Git URL |
962962
| `token` | the workflow's own token | The token used to post the comment, to post as another account or bot |
963963

964-
Pull requests from forks get a read-only token, which can't post comments, so set `comment: "false"`. The graph artifact will still attach to the run.
964+
The workflow runs on `pull_request_target` because GitHub skips `pull_request` workflows for pull requests with merge conflicts, which are the ones you most want checked. It also lets the Action comment on pull requests from forks. That's safe here because the Action never runs the pull request's code: it only reads its commits.
965965

966966
</details>
967967

‎integrations/github-action/action.yml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ runs:
6060
NUMBER: ${{ github.event.pull_request.number }}
6161
run: |
6262
set -euo pipefail
63-
git fetch -q origin "refs/pull/$NUMBER/head:refs/heads/pr-$NUMBER" "$BASE:refs/heads/$BASE" || true
63+
# The pull request's commits as a branch git-sim can name. The base is
64+
# fetched apart from it: on pull_request_target the checkout is the base
65+
# branch itself, which git refuses to fetch into.
66+
git fetch -q origin "+refs/pull/$NUMBER/head:refs/heads/pr-$NUMBER"
6467
git fetch -q origin "$BASE"
6568
git checkout -q -B "$BASE" "origin/$BASE"
6669

‎src/git_sim/preflight_cli.py‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,12 @@ def words_after_preflight(argv: List[str]) -> List[str]:
5656

5757
def render_text(report: PreflightReport) -> str:
5858
d = report.to_dict()
59+
# a command that couldn't be checked is never called safe
60+
if d.get("error"):
61+
return f"NOT CHECKED git {d['command']}".rstrip() + f"\nerror: {d['error']}"
5962
lines = [
6063
f"{RISK_LABELS.get(d['risk'], d['risk'].upper())} git {d['command']}".rstrip()
6164
]
62-
if d.get("error"):
63-
lines.append(f"error: {d['error']}")
64-
return "\n".join(lines)
6565
if d.get("summary"):
6666
lines += ["", d["summary"]]
6767
for title, key in (
@@ -82,12 +82,11 @@ def render_text(report: PreflightReport) -> str:
8282
def render_markdown(report: PreflightReport) -> str:
8383
"""The report as Markdown, for a pull request comment or a chat message."""
8484
d = report.to_dict()
85+
if d.get("error"):
86+
return f"### ⚪ Not checked · `git {d['command']}`\n\n**Error:** {d['error']}"
8587
lines = [
8688
f"### {RISK_BADGES.get(d['risk'], d['risk'])} · `git {d['command']}`".rstrip()
8789
]
88-
if d.get("error"):
89-
lines += ["", f"**Error:** {d['error']}"]
90-
return "\n".join(lines)
9190
if d.get("summary"):
9291
lines += ["", d["summary"]]
9392
for title, key in (

‎tests/unit_tests/test_preflight_cli.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,12 @@ def test_render_text_shows_an_error_only():
9696
report.error = "Not a git repository: /nowhere"
9797
text = render_text(report)
9898
assert text.splitlines()[-1] == "error: Not a git repository: /nowhere"
99+
# a command that couldn't be checked is never reported as safe
100+
assert text.startswith("NOT CHECKED") and "SAFE" not in text
101+
from git_sim.preflight_cli import render_markdown
102+
103+
md = render_markdown(report)
104+
assert md.startswith("### ⚪ Not checked") and "Safe" not in md
99105

100106

101107
def test_cli_prints_json_for_a_real_repository(repo):

0 commit comments

Comments
 (0)