diff --git a/action.yml b/action.yml index 3de0b22..f0f458f 100644 --- a/action.yml +++ b/action.yml @@ -611,14 +611,11 @@ runs: MODEL: ${{ inputs.model }} AGENT_MODEL_INPUT: ${{ inputs.agent_model }} PARSING_MODEL_INPUT: ${{ inputs.parsing_model }} + # A setup PR's run goes on to preview other pull requests; the credentials stay until + # those finish and the step after them removes them. + KEEP_AUTH: ${{ github.event_name == 'pull_request' && github.head_ref == 'codeboarding/setup-action' }} run: '"$GITHUB_ACTION_PATH/scripts/action/with-auth.sh" "$GITHUB_ACTION_PATH/scripts/action/review-analyze-head.sh"' - # The head step removes the credentials; this covers a run that failed before it. - - name: Remove analysis credentials - if: always() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' - shell: bash - run: '"$GITHUB_ACTION_PATH/scripts/action/clear-auth.sh"' - # Published before the review is rendered: the analysis is the expensive part, # so a later rendering or posting failure must not throw it away. Only the # next run reads this, hence the short retention. @@ -724,6 +721,160 @@ runs: GITHUB_TOKEN: ${{ inputs.github_token }} path: ${{ steps.review_body.outputs.path }} + # Before a setup PR merges, its own run is the only one GitHub starts for us, so this is + # where a new user's open pull requests get their first review: the ones the app listed + # in the setup PR's description, each applied to the commit this run just analysed and + # analysed incrementally from it (see review-preview-pr.sh). Nothing is posted on them. + # The app finds each as codeboarding-preview---pr-. + # Not codeboarding-review--...: the app takes the newest artifact with that prefix as + # the setup PR's own review, so a preview named that way would replace the setup PR's map. + # All best effort: the setup PR's map is already uploaded and must not fail because of them. + - name: Resolve pull requests to preview + id: preview_list + if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_comment.outcome == 'success' && github.event_name == 'pull_request' && github.head_ref == 'codeboarding/setup-action' + continue-on-error: true + shell: bash + env: + GH_TOKEN: ${{ inputs.github_token }} + GH_HOST: ${{ github.server_url }} + REPOSITORY: ${{ github.repository }} + PR_NUMBER: ${{ steps.guard.outputs.pr_number }} + HEAD_REF: ${{ github.head_ref }} + BASE_REF: ${{ steps.guard.outputs.base_ref }} + MAX_PREVIEWS: '3' + run: "$GITHUB_ACTION_PATH/scripts/action/resolve-preview-prs.sh" + + - name: Preview pull request 1 for the setup PR + id: preview_1 + if: steps.guard.outputs.mode == 'review' && steps.preview_list.outputs.count >= 1 + continue-on-error: true + shell: bash + env: + ACTION_PATH: ${{ github.action_path }} + CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target + GIT_TOKEN: ${{ inputs.github_token }} + REPOSITORY: ${{ github.repository }} + SETUP_PR: ${{ steps.guard.outputs.pr_number }} + REVIEW_BASE_SHA: ${{ steps.guard.outputs.merge_base_sha }} + BASE_ANALYSIS_PATH: ${{ steps.review_baseline.outputs.base_analysis_path }} + # The setup PR's own review artifact already records where the base graph lives. + REVIEW_ARTIFACT_DIR: ${{ steps.review_artifact.outputs.artifact_dir }} + ENGINE_VERSION: ${{ steps.state.outputs.engine_version }} + CFG_HASH: ${{ steps.state.outputs.cfg_hash }} + DEPTH_CAP: ${{ inputs.depth_cap }} + MODEL: ${{ inputs.model }} + AGENT_MODEL_INPUT: ${{ inputs.agent_model }} + PARSING_MODEL_INPUT: ${{ inputs.parsing_model }} + KEEP_AUTH: 'true' + run: '"$GITHUB_ACTION_PATH/scripts/action/with-auth.sh" "$GITHUB_ACTION_PATH/scripts/action/review-preview-pr.sh" 1' + + - name: Upload preview 1 + id: upload_preview_1 + if: steps.preview_1.outputs.artifact_dir != '' && github.server_url == 'https://github.com' + continue-on-error: true + uses: actions/upload-artifact@v4 + with: + name: codeboarding-preview-${{ github.run_id }}-${{ github.run_attempt }}-pr${{ steps.preview_1.outputs.pr_number }}-${{ steps.preview_1.outputs.head_sha }} + path: ${{ steps.preview_1.outputs.artifact_dir }} + if-no-files-found: error + retention-days: 14 + + # The sticky review comment is an edit by now, and edits send no notification. One new + # comment on a setup PR emails its author: after the first preview when there is one, + # so the email links a coloured pull request, otherwise as soon as the map is posted. + - name: Tell the setup PR's author the map is ready + if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_comment.outcome == 'success' && github.event_name == 'pull_request' && github.head_ref == 'codeboarding/setup-action' + continue-on-error: true + shell: bash + env: + GH_TOKEN: ${{ inputs.github_token }} + GH_HOST: ${{ github.server_url }} + REPOSITORY: ${{ github.repository }} + PR_NUMBER: ${{ steps.guard.outputs.pr_number }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + HEAD_REF: ${{ github.head_ref }} + BASE_REF: ${{ steps.guard.outputs.base_ref }} + FIRST_PR: ${{ steps.upload_preview_1.outcome == 'success' && steps.preview_1.outputs.pr_number || '' }} + FIRST_TITLE: ${{ steps.preview_1.outputs.title }} + PREVIEW_PRS: ${{ steps.preview_list.outputs.prs }} + run: "$GITHUB_ACTION_PATH/scripts/action/notify-setup-ready.sh" + + - name: Preview pull request 2 for the setup PR + id: preview_2 + if: steps.guard.outputs.mode == 'review' && steps.preview_list.outputs.count >= 2 + continue-on-error: true + shell: bash + env: + ACTION_PATH: ${{ github.action_path }} + CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target + GIT_TOKEN: ${{ inputs.github_token }} + REPOSITORY: ${{ github.repository }} + SETUP_PR: ${{ steps.guard.outputs.pr_number }} + REVIEW_BASE_SHA: ${{ steps.guard.outputs.merge_base_sha }} + BASE_ANALYSIS_PATH: ${{ steps.review_baseline.outputs.base_analysis_path }} + # The setup PR's own review artifact already records where the base graph lives. + REVIEW_ARTIFACT_DIR: ${{ steps.review_artifact.outputs.artifact_dir }} + ENGINE_VERSION: ${{ steps.state.outputs.engine_version }} + CFG_HASH: ${{ steps.state.outputs.cfg_hash }} + DEPTH_CAP: ${{ inputs.depth_cap }} + MODEL: ${{ inputs.model }} + AGENT_MODEL_INPUT: ${{ inputs.agent_model }} + PARSING_MODEL_INPUT: ${{ inputs.parsing_model }} + KEEP_AUTH: 'true' + run: '"$GITHUB_ACTION_PATH/scripts/action/with-auth.sh" "$GITHUB_ACTION_PATH/scripts/action/review-preview-pr.sh" 2' + + - name: Upload preview 2 + id: upload_preview_2 + if: steps.preview_2.outputs.artifact_dir != '' && github.server_url == 'https://github.com' + continue-on-error: true + uses: actions/upload-artifact@v4 + with: + name: codeboarding-preview-${{ github.run_id }}-${{ github.run_attempt }}-pr${{ steps.preview_2.outputs.pr_number }}-${{ steps.preview_2.outputs.head_sha }} + path: ${{ steps.preview_2.outputs.artifact_dir }} + if-no-files-found: error + retention-days: 14 + + - name: Preview pull request 3 for the setup PR + id: preview_3 + if: steps.guard.outputs.mode == 'review' && steps.preview_list.outputs.count >= 3 + continue-on-error: true + shell: bash + env: + ACTION_PATH: ${{ github.action_path }} + CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target + GIT_TOKEN: ${{ inputs.github_token }} + REPOSITORY: ${{ github.repository }} + SETUP_PR: ${{ steps.guard.outputs.pr_number }} + REVIEW_BASE_SHA: ${{ steps.guard.outputs.merge_base_sha }} + BASE_ANALYSIS_PATH: ${{ steps.review_baseline.outputs.base_analysis_path }} + # The setup PR's own review artifact already records where the base graph lives. + REVIEW_ARTIFACT_DIR: ${{ steps.review_artifact.outputs.artifact_dir }} + ENGINE_VERSION: ${{ steps.state.outputs.engine_version }} + CFG_HASH: ${{ steps.state.outputs.cfg_hash }} + DEPTH_CAP: ${{ inputs.depth_cap }} + MODEL: ${{ inputs.model }} + AGENT_MODEL_INPUT: ${{ inputs.agent_model }} + PARSING_MODEL_INPUT: ${{ inputs.parsing_model }} + run: '"$GITHUB_ACTION_PATH/scripts/action/with-auth.sh" "$GITHUB_ACTION_PATH/scripts/action/review-preview-pr.sh" 3' + + - name: Upload preview 3 + id: upload_preview_3 + if: steps.preview_3.outputs.artifact_dir != '' && github.server_url == 'https://github.com' + continue-on-error: true + uses: actions/upload-artifact@v4 + with: + name: codeboarding-preview-${{ github.run_id }}-${{ github.run_attempt }}-pr${{ steps.preview_3.outputs.pr_number }}-${{ steps.preview_3.outputs.head_sha }} + path: ${{ steps.preview_3.outputs.artifact_dir }} + if-no-files-found: error + retention-days: 14 + + # The head and preview steps remove the credentials themselves; this covers a run that + # failed before them, and a setup PR's run, which keeps them for its previews. + - name: Remove analysis credentials + if: always() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' + shell: bash + run: '"$GITHUB_ACTION_PATH/scripts/action/clear-auth.sh"' + # The same body, on the run page. Job summaries and logs are explicitly excluded from # the artifact storage allowance, and the cap is 1MiB per step against a diagram # measured in single-digit kilobytes, so this is free in both senses. It earns its diff --git a/scripts/action/build-review-artifact.sh b/scripts/action/build-review-artifact.sh index acdf52a..c53118b 100755 --- a/scripts/action/build-review-artifact.sh +++ b/scripts/action/build-review-artifact.sh @@ -1,8 +1,10 @@ #!/usr/bin/env bash # Packages both analyses and PR metadata, then outputs the artifact upload directory. set -euo pipefail -mkdir -p "${RUNNER_TEMP}/cb-review-artifact" -cp "$ANALYSIS_PATH" "${RUNNER_TEMP}/cb-review-artifact/analysis.json" +# A setup PR's previews of other pull requests are packaged the same way, each into its own directory. +dir="${ARTIFACT_DIR:-${RUNNER_TEMP}/cb-review-artifact}" +mkdir -p "$dir" +cp "$ANALYSIS_PATH" "$dir/analysis.json" # The base graph is normally published separately, named for the commit it # describes, so ten runs on one pull request store it once rather than ten times. # A fork review publishes nothing another run could read, so when it had to @@ -10,14 +12,16 @@ cp "$ANALYSIS_PATH" "${RUNNER_TEMP}/cb-review-artifact/analysis.json" # artifact would name a base that does not exist and no reader could reproduce # the comparison. if [ "${INLINE_BASE:-false}" = true ]; then - cp "$BASE_ANALYSIS_PATH" "${RUNNER_TEMP}/cb-review-artifact/base_analysis.json" + cp "$BASE_ANALYSIS_PATH" "$dir/base_analysis.json" BASE_ARTIFACT_NAME="" fi # The engine writes this next to the analysis it came from. Ship it when present # so a pull request's warnings are readable without rerunning the analysis. HEALTH_REPORT="$(dirname "$ANALYSIS_PATH")/health/health_report.json" -[ ! -f "$HEALTH_REPORT" ] || cp "$HEALTH_REPORT" "${RUNNER_TEMP}/cb-review-artifact/health_report.json" +[ ! -f "$HEALTH_REPORT" ] || cp "$HEALTH_REPORT" "$dir/health_report.json" +# basis=preview marks a review made inside a setup PR's run: the pull request's own changes +# laid over the commit that run analysed, not over its real merge base. setup_pr names that run's PR. # base_sha stays the event's base branch tip for consumers that key on it; # merge_base_sha records the commit the diagram actually compared against. # pr_base_sha carries the same value under the name the webview already reads: @@ -37,10 +41,13 @@ jq -n \ --arg base_artifact "$BASE_ARTIFACT_NAME" \ --arg base_artifact_id "$BASE_ARTIFACT_ID" \ --arg analysed_files_changed "${ANALYSED_FILES_CHANGED:-unknown}" \ + --arg basis "${BASIS:-}" \ + --arg setup_pr "${SETUP_PR:-}" \ '{kind: $kind, mode: $mode, base_sha: $base_sha, merge_base_sha: $merge_base_sha, pr_base_sha: $merge_base_sha, merge_base_resolved: $merge_base_resolved, head_sha: $head_sha, pr_number: $pr_number, seed_source: $seed_source, chain_depth: $chain_depth, base_artifact: $base_artifact, base_artifact_id: $base_artifact_id, - analysed_files_changed: $analysed_files_changed}' \ - > "${RUNNER_TEMP}/cb-review-artifact/metadata.json" -echo "artifact_dir=${RUNNER_TEMP}/cb-review-artifact" >> "$GITHUB_OUTPUT" + analysed_files_changed: $analysed_files_changed} + + (if $basis == "" then {} else {basis: $basis, setup_pr: $setup_pr} end)' \ + > "$dir/metadata.json" +echo "artifact_dir=$dir" >> "$GITHUB_OUTPUT" diff --git a/scripts/action/notify-setup-ready.sh b/scripts/action/notify-setup-ready.sh new file mode 100755 index 0000000..de20101 --- /dev/null +++ b/scripts/action/notify-setup-ready.sh @@ -0,0 +1,56 @@ +#!/usr/bin/env bash +# Tells the author of a CodeBoarding setup PR that its preview map is ready. +# +# The review comment is sticky: it is created when the run starts and edited when it +# ends, and GitHub sends no notification for an edit. A new comment does notify the +# PR's author, who for a setup PR is the person who asked for the map (the app opens +# it with their token), so this one comment is what turns "you can close this tab" +# into an email from GitHub. Failed runs need nothing here: GitHub already emails the +# person who triggered a failed run. +# +# When the setup PR asked for previews of other pull requests, this runs after the first +# one is ready (FIRST_PR) and links it, since a coloured pull request is the point; the +# rest of PREVIEW_PRS are named as still running. Posted once per PR, found again by its +# marker, so pushing new settings to the setup PR does not email again. Best effort throughout. +set -euo pipefail +[ "${HEAD_REF:-}" = "codeboarding/setup-action" ] || exit 0 +[ -n "${PR_NUMBER:-}" ] && [ -n "${REPOSITORY:-}" ] || exit 0 +export GH_HOST="${GH_HOST:-github.com}" +GH_HOST="${GH_HOST#*://}" + +marker="" +existing="$(gh api --paginate "repos/$REPOSITORY/issues/$PR_NUMBER/comments?per_page=100" \ + --jq ".[] | select((.body // \"\") | contains(\"$marker\")) | .id")" +[ -z "$existing" ] || exit 0 + +# A mention notifies even an author who unsubscribed from the thread. Bots are not mentioned. +greeting="Your" +case "${PR_AUTHOR:-}" in + "" | *"[bot]") ;; + *) greeting="@$PR_AUTHOR your" ;; +esac +if [ -n "${BASE_REF:-}" ]; then + branch="\`$BASE_REF\`" +else + branch="your default branch" +fi +link() { printf 'https://app.codeboarding.org/%s/pull/%s?utm_source=github&utm_medium=setup_ready&utm_campaign=gh_action' "$REPOSITORY" "$1"; } +if [ -n "${FIRST_PR:-}" ]; then + headline="$greeting CodeBoarding map of $branch is ready, and #$FIRST_PR is reviewed on it." + cta="**[Open #$FIRST_PR in CodeBoarding]($(link "$FIRST_PR"))**${FIRST_TITLE:+ ยท $FIRST_TITLE}" + rest="" + IFS=',' read -r -a all <<< "${PREVIEW_PRS:-}" + for pr in "${all[@]}"; do + if [ -n "$pr" ] && [ "$pr" != "$FIRST_PR" ]; then rest+="${rest:+, }#$pr"; fi + done + [ -z "$rest" ] || cta+=$'\n\n'"$rest follow in a few minutes. [Your map]($(link "$PR_NUMBER")) is open already." +else + headline="$greeting CodeBoarding map of $branch is ready." + cta="**[Open it in CodeBoarding]($(link "$PR_NUMBER"))**" +fi +body="$(printf '%s\n\n%s\n\n%s\n\n%s' \ + "$headline" \ + "$cta" \ + "This is a preview. Merge this PR to keep the map up to date and review every pull request." \ + "$marker")" +gh api -X POST "repos/$REPOSITORY/issues/$PR_NUMBER/comments" -f body="$body" >/dev/null diff --git a/scripts/action/resolve-preview-prs.sh b/scripts/action/resolve-preview-prs.sh new file mode 100755 index 0000000..b247aef --- /dev/null +++ b/scripts/action/resolve-preview-prs.sh @@ -0,0 +1,57 @@ +#!/usr/bin/env bash +# Reads which pull requests a CodeBoarding setup PR should preview, from a line the app +# writes into its description: +# +# +# +# Before the setup PR merges, its own run is the only one GitHub starts for us, so this is +# where a new user's open pull requests get their first review. The description is read +# now, after the map is built, not from the event payload, so a list changed while the map +# was building still counts. Each entry must be an open pull request from this repository +# into the same branch; anything else is skipped with a notice. At most MAX_PREVIEWS. +# +# Writes one JSON file per preview to $RUNNER_TEMP/codeboarding-preview/.json and +# outputs count= and prs= (comma-separated, in order). Best effort throughout. +set -euo pipefail +out="${RUNNER_TEMP:?}/codeboarding-preview" +rm -rf "$out" +mkdir -p "$out" +count=0 prs="" +finish() { printf 'count=%s\nprs=%s\n' "$count" "$prs" >> "${GITHUB_OUTPUT:-/dev/null}"; exit 0; } +[ "${HEAD_REF:-}" = "codeboarding/setup-action" ] || finish +[ -n "${PR_NUMBER:-}" ] && [ -n "${REPOSITORY:-}" ] && [ -n "${BASE_REF:-}" ] || finish +export GH_HOST="${GH_HOST:-github.com}" +GH_HOST="${GH_HOST#*://}" +max="${MAX_PREVIEWS:-3}" + +body="$(gh api "repos/$REPOSITORY/pulls/$PR_NUMBER" --jq '.body // ""' 2>/dev/null || true)" +line="$(grep -oE '' <<< "$body" | head -n 1 || true)" +[ -n "$line" ] || finish + +skip() { echo "::notice::Not previewing #$1: $2"; } +seen=" " +for pr in $(grep -oE '[0-9]+' <<< "${line#*preview}"); do + [ "$count" -lt "$max" ] || { skip "$pr" "at most $max pull requests are previewed."; continue; } + [ "$pr" != "$PR_NUMBER" ] || continue + case "$seen" in *" $pr "*) continue ;; esac + seen+="$pr " + if ! json="$(gh api "repos/$REPOSITORY/pulls/$pr" 2>/dev/null)"; then + skip "$pr" "it could not be read." + continue + fi + IFS=$'\t' read -r state head_repo base_ref base_sha head_sha < <(jq -r \ + '[.state, (.head.repo.full_name // ""), .base.ref, .base.sha, .head.sha] | @tsv' <<< "$json") + [ "$state" = open ] || { skip "$pr" "it is not open."; continue; } + [ "$head_repo" = "$REPOSITORY" ] || { skip "$pr" "it comes from a fork."; continue; } + [ "$base_ref" = "$BASE_REF" ] || { skip "$pr" "it targets $base_ref, not $BASE_REF."; continue; } + # The pull request's own changes are what it adds since its merge base, the same + # comparison GitHub's Files tab makes. + merge_base="$(gh api "repos/$REPOSITORY/compare/$base_sha...$head_sha" --jq '.merge_base_commit.sha // ""' 2>/dev/null || true)" + [ -n "$merge_base" ] || { skip "$pr" "its merge base could not be resolved."; continue; } + count=$(( count + 1 )) + jq -n --arg pr "$pr" --arg head_sha "$head_sha" --arg merge_base "$merge_base" \ + --arg title "$(jq -r '.title // ""' <<< "$json")" \ + '{pr_number: $pr, head_sha: $head_sha, merge_base_sha: $merge_base, title: $title}' > "$out/$count.json" + prs="${prs:+$prs,}$pr" +done +finish diff --git a/scripts/action/review-preview-pr.sh b/scripts/action/review-preview-pr.sh new file mode 100755 index 0000000..62f640a --- /dev/null +++ b/scripts/action/review-preview-pr.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +# Reviews the n-th pull request a setup PR asked to preview (see resolve-preview-prs.sh). +# +# The run has one analysis to start from: the commit the setup PR is compared against. +# Reviewing a pull request at its own merge base would need a second full analysis of an +# older commit, so instead its own changes (merge base to head, as GitHub's Files tab shows +# them) are applied to that commit and analysed incrementally from the analysis in hand. +# Only the pull request's own work differs between the two, so nobody else's commits are +# reported as its changes. A pull request that does not apply cleanly there is skipped. +# +# Usage: review-preview-pr.sh . Outputs pr_number=, title= and artifact_dir= when a +# preview was built, plus head_sha=; nothing when there is no n-th entry or it was skipped. +set -euo pipefail +# shellcheck source=scripts/action/analysis-state.sh +source "$(dirname "$0")/analysis-state.sh" +n="$1" +dir="${RUNNER_TEMP:?}/codeboarding-preview" +entry="$dir/$n.json" +[ -f "$entry" ] || exit 0 +IFS=$'\t' read -r pr head_sha pr_merge_base < <(jq -r '[.pr_number, .head_sha, .merge_base_sha] | @tsv' "$entry") +title="$(jq -r '.title' "$entry")" +base="$REVIEW_BASE_SHA" + +fetch_commit "$REPOSITORY" "$base" >&2 +fetch_commit "$REPOSITORY" "$pr_merge_base" >&2 +fetch_commit "$REPOSITORY" "$head_sha" >&2 +tree="$dir/tree-$n" +rm -rf "$tree" +git -C "$CHECKOUT_DIR" worktree add --detach "$tree" "$base" >/dev/null 2>&1 +if ! git -C "$CHECKOUT_DIR" diff --binary "$pr_merge_base" "$head_sha" | + git -C "$tree" apply --3way --whitespace=nowarn >/dev/null 2>&1; then + echo "::notice::#$pr does not apply cleanly to ${base:0:7}, the commit this map describes; not previewing it." + exit 0 +fi + +state="$dir/state-$n" +rm -rf "$state" +cp -a "$(dirname "$BASE_ANALYSIS_PATH")" "$state" +rm -f "$state/origin.json" +unset GIT_TOKEN +result="$("$ACTION_PATH/scripts/action/analyze.sh" head "$tree" "$state")" +analysis_path="$(awk -F= '$1 == "analysis_path" {print substr($0, index($0, "=") + 1); exit}' <<< "$result")" +analysis_mode="$(awk -F= '$1 == "analysis_mode" {print $2; exit}' <<< "$result")" + +meta="$(python3 "$ACTION_PATH/scripts/diff_to_mermaid.py" --base "$BASE_ANALYSIS_PATH" --head "$analysis_path" \ + --out "$dir/diagram-$n.md" --direction LR --render-depth 1)" +analysed="$(jq -r '.analysed_files_changed // "unknown"' <<< "$meta")" + +# Point at the same base graph the setup PR's review does, inlined when it does. +review_meta="$REVIEW_ARTIFACT_DIR/metadata.json" +inline=false +[ ! -f "$REVIEW_ARTIFACT_DIR/base_analysis.json" ] || inline=true +# The packaging step writes artifact_dir= to this step's outputs. +BASE_ARTIFACT_NAME="$(jq -r '.base_artifact // ""' "$review_meta")" \ + BASE_ARTIFACT_ID="$(jq -r '.base_artifact_id // ""' "$review_meta")" INLINE_BASE="$inline" \ + ARTIFACT_DIR="$dir/artifact-$n" ANALYSIS_PATH="$analysis_path" ANALYSIS_MODE="$analysis_mode" \ + BASE_SHA="$base" MERGE_BASE_SHA="$base" MERGE_BASE_RESOLVED=true HEAD_SHA="$head_sha" \ + PR_NUMBER="$pr" SEED_SOURCE=base CHAIN_DEPTH=1 ANALYSED_FILES_CHANGED="$analysed" \ + BASIS=preview SETUP_PR="$SETUP_PR" \ + "$ACTION_PATH/scripts/action/build-review-artifact.sh" +printf 'pr_number=%s\nhead_sha=%s\ntitle=%s\n' "$pr" "$head_sha" "${title//$'\n'/ }" >> "$GITHUB_OUTPUT" diff --git a/tests/test_setup_previews.py b/tests/test_setup_previews.py new file mode 100644 index 0000000..aaa705d --- /dev/null +++ b/tests/test_setup_previews.py @@ -0,0 +1,252 @@ +"""A setup PR's run previews the pull requests the app listed in its description.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +RESOLVE = ROOT / "scripts" / "action" / "resolve-preview-prs.sh" +PREVIEW = ROOT / "scripts" / "action" / "review-preview-pr.sh" +NOTIFY = ROOT / "scripts" / "action" / "notify-setup-ready.sh" + +PULLS = { + "412": {"state": "open", "head": {"repo": {"full_name": "owner/repo"}, "sha": "h412"}, "base": {"ref": "main", "sha": "t"}, "title": "Retry failed webhooks"}, + "409": {"state": "open", "head": {"repo": {"full_name": "owner/repo"}, "sha": "h409"}, "base": {"ref": "main", "sha": "t"}, "title": "Split ledger"}, + "401": {"state": "open", "head": {"repo": {"full_name": "owner/repo"}, "sha": "h401"}, "base": {"ref": "main", "sha": "t"}, "title": "Bump SDK"}, + "400": {"state": "open", "head": {"repo": {"full_name": "owner/repo"}, "sha": "h400"}, "base": {"ref": "main", "sha": "t"}, "title": "Fourth"}, + "377": {"state": "open", "head": {"repo": {"full_name": "someone/fork"}, "sha": "h377"}, "base": {"ref": "main", "sha": "t"}, "title": "From a fork"}, + "350": {"state": "closed", "head": {"repo": {"full_name": "owner/repo"}, "sha": "h350"}, "base": {"ref": "main", "sha": "t"}, "title": "Closed"}, + "340": {"state": "open", "head": {"repo": {"full_name": "owner/repo"}, "sha": "h340"}, "base": {"ref": "release", "sha": "t"}, "title": "Other branch"}, +} + + +def git(cwd: Path, *args: str) -> str: + return subprocess.run( + ["git", "-c", "user.name=t", "-c", "user.email=t@t", *args], cwd=cwd, check=True, capture_output=True, text=True + ).stdout.strip() + + +class ResolvePreviewTests(unittest.TestCase): + def setUp(self) -> None: + self.temp_dir = tempfile.TemporaryDirectory() + self.root = Path(self.temp_dir.name) + (self.root / "bin").mkdir() + (self.root / "pulls.json").write_text(json.dumps(PULLS)) + self.calls = self.root / "calls" + gh = self.root / "bin" / "gh" + # pulls/418 is the setup PR; its body carries the list. compare returns a merge base per head. + gh.write_text( + "#!/usr/bin/env python3\n" + "import json, sys\n" + f"open({str(self.calls)!r}, 'a').write(' '.join(sys.argv[1:]) + '\\n')\n" + f"pulls = json.load(open({str(self.root / 'pulls.json')!r}))\n" + "path = sys.argv[2]\n" + "if path.endswith('/pulls/418'):\n" + f" print(open({str(self.root / 'body')!r}).read())\n" + "elif '/compare/' in path:\n" + " print('mb-' + path.rsplit('...', 1)[1])\n" + "else:\n" + " n = path.rsplit('/', 1)[1]\n" + " if n not in pulls: sys.exit(1)\n" + " print(json.dumps(pulls[n]))\n", + encoding="utf-8", + ) + gh.chmod(0o755) + + def tearDown(self) -> None: + self.temp_dir.cleanup() + + def _run(self, body: str, **extra: str) -> dict[str, str]: + (self.root / "body").write_text(body) + output = self.root / "output" + output.write_text("") + subprocess.run( + [str(RESOLVE)], + env={ + "PATH": f"{self.root / 'bin'}:{os.environ['PATH']}", + "RUNNER_TEMP": str(self.root), + "GITHUB_OUTPUT": str(output), + "REPOSITORY": "owner/repo", + "PR_NUMBER": "418", + "HEAD_REF": "codeboarding/setup-action", + "BASE_REF": "main", + **extra, + }, + check=True, + capture_output=True, + text=True, + ) + return dict(line.split("=", 1) for line in output.read_text().splitlines() if line) + + def _entry(self, n: int) -> dict: + return json.loads((self.root / "codeboarding-preview" / f"{n}.json").read_text()) + + def test_the_listed_pull_requests_are_previewed_in_order(self) -> None: + out = self._run("Adds CodeBoarding.\n\n\n") + self.assertEqual(out, {"count": "2", "prs": "412,409"}) + self.assertEqual(self._entry(1), {"pr_number": "412", "head_sha": "h412", "merge_base_sha": "mb-h412", "title": "Retry failed webhooks"}) + self.assertEqual(self._entry(2)["pr_number"], "409") + + def test_forks_closed_and_other_branches_are_skipped_and_the_list_is_capped(self) -> None: + out = self._run("") + self.assertEqual(out, {"count": "3", "prs": "412,409,401"}) + + def test_no_list_means_no_previews(self) -> None: + self.assertEqual(self._run("Adds CodeBoarding."), {"count": "0", "prs": ""}) + + def test_other_pull_requests_read_nothing(self) -> None: + self.assertEqual(self._run("", HEAD_REF="feat/x"), {"count": "0", "prs": ""}) + self.assertFalse(self.calls.exists()) + + +class ReviewPreviewTests(unittest.TestCase): + """Real git history: main moved on after the PR forked, and the preview applies only the PR's own work.""" + + def setUp(self) -> None: + self.temp_dir = tempfile.TemporaryDirectory() + self.root = Path(self.temp_dir.name) + repo = self.repo = self.root / "repo" + repo.mkdir() + git(repo, "init", "-q", "-b", "main") + (repo / "shared.txt").write_text("one\ntwo\nthree\n") + git(repo, "add", ".") + git(repo, "commit", "-qm", "X") + self.fork_point = git(repo, "rev-parse", "HEAD") + git(repo, "checkout", "-qb", "pr") + (repo / "webhooks.txt").write_text("retry\n") + git(repo, "add", ".") + git(repo, "commit", "-qm", "P") + self.pr_head = git(repo, "rev-parse", "HEAD") + git(repo, "checkout", "-q", "main") + (repo / "ledger.txt").write_text("landed after the fork\n") + git(repo, "add", ".") + git(repo, "commit", "-qm", "Z") + self.base = git(repo, "rev-parse", "HEAD") + + # The base analysis this run built, and the setup PR's own review artifact. + self.base_dir = self.root / "base-state" + self.base_dir.mkdir() + (self.base_dir / "analysis.json").write_text('{"components": []}') + self.review_dir = self.root / "review-artifact" + self.review_dir.mkdir() + (self.review_dir / "metadata.json").write_text(json.dumps({"base_artifact": "cb-base-Z", "base_artifact_id": "77"})) + + # The action tree with the engine and renderer stubbed; the packager is the real one. + action = self.action = self.root / "action" + (action / "scripts" / "action").mkdir(parents=True) + shutil.copy(ROOT / "scripts" / "action" / "build-review-artifact.sh", action / "scripts" / "action") + analyze = action / "scripts" / "action" / "analyze.sh" + analyze.write_text( + "#!/usr/bin/env bash\n" + 'ls "$2" | sort > "$3/seen-files"\n' + 'echo \'{"components": ["Webhooks"]}\' > "$3/analysis.json"\n' + 'printf "analysis_mode=incremental\\nanalysis_path=%s\\n" "$3/analysis.json"\n', + encoding="utf-8", + ) + analyze.chmod(0o755) + (action / "scripts" / "diff_to_mermaid.py").write_text('print(\'{"n_changed": 1, "analysed_files_changed": 1}\')\n') + + self.preview_dir = self.root / "codeboarding-preview" + self.preview_dir.mkdir() + + def tearDown(self) -> None: + self.temp_dir.cleanup() + + def _run(self, pr_head: str | None = None) -> dict[str, str]: + (self.preview_dir / "1.json").write_text(json.dumps( + {"pr_number": "412", "head_sha": pr_head or self.pr_head, "merge_base_sha": self.fork_point, "title": "Retry failed webhooks"})) + output = self.root / "output" + output.write_text("") + subprocess.run( + [str(PREVIEW), "1"], + env={ + "PATH": os.environ["PATH"], + "RUNNER_TEMP": str(self.root), + "GITHUB_OUTPUT": str(output), + "ACTION_PATH": str(self.action), + "CHECKOUT_DIR": str(self.repo), + "REPOSITORY": "owner/repo", + "SETUP_PR": "418", + "REVIEW_BASE_SHA": self.base, + "BASE_ANALYSIS_PATH": str(self.base_dir / "analysis.json"), + "REVIEW_ARTIFACT_DIR": str(self.review_dir), + }, + check=True, + capture_output=True, + text=True, + ) + return dict(line.split("=", 1) for line in output.read_text().splitlines() if line) + + def test_the_pull_request_is_applied_to_the_analysed_commit(self) -> None: + out = self._run() + self.assertEqual(out["pr_number"], "412") + self.assertEqual(out["head_sha"], self.pr_head, "the artifact name carries it, so the app needs no download") + seen = (self.preview_dir / "state-1" / "seen-files").read_text().split() + # Z's file is there because the base is Z; the PR's file because it was applied; nothing else. + self.assertIn("ledger.txt", seen) + self.assertIn("webhooks.txt", seen) + + def test_the_artifact_names_the_pull_request_and_the_comparison(self) -> None: + out = self._run() + meta = json.loads((Path(out["artifact_dir"]) / "metadata.json").read_text()) + self.assertEqual(meta["pr_number"], "412") + self.assertEqual(meta["head_sha"], self.pr_head) + self.assertEqual(meta["merge_base_sha"], self.base, "the commit the diagram compared against") + self.assertEqual(meta["pr_base_sha"], self.base) + self.assertEqual(meta["basis"], "preview") + self.assertEqual(meta["setup_pr"], "418") + self.assertEqual(meta["base_artifact"], "cb-base-Z") + self.assertEqual(meta["base_artifact_id"], "77") + self.assertFalse((Path(out["artifact_dir"]) / "base_analysis.json").exists()) + + def test_the_base_is_inlined_when_the_setup_review_inlined_it(self) -> None: + (self.review_dir / "base_analysis.json").write_text("{}") + out = self._run() + self.assertTrue((Path(out["artifact_dir"]) / "base_analysis.json").exists()) + + def test_a_pull_request_that_conflicts_with_the_analysed_commit_is_skipped(self) -> None: + git(self.repo, "checkout", "-qb", "clash", self.fork_point) + (self.repo / "ledger.txt").write_text("a different ledger\n") + git(self.repo, "add", ".") + git(self.repo, "commit", "-qm", "C") + clash = git(self.repo, "rev-parse", "HEAD") + git(self.repo, "checkout", "-q", "main") + self.assertEqual(self._run(pr_head=clash), {}) + + +class FirstPreviewNoticeTests(unittest.TestCase): + def test_the_email_links_the_first_reviewed_pull_request(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "bin").mkdir() + calls = root / "calls" + gh = root / "bin" / "gh" + gh.write_text(f'#!/usr/bin/env bash\nprintf "%s\\n----\\n" "$*" >> "{calls}"\n', encoding="utf-8") + gh.chmod(0o755) + subprocess.run( + [str(NOTIFY)], + env={ + "PATH": f"{root / 'bin'}:{os.environ['PATH']}", + "REPOSITORY": "owner/repo", "PR_NUMBER": "418", "PR_AUTHOR": "maria-k", + "HEAD_REF": "codeboarding/setup-action", "BASE_REF": "main", + "FIRST_PR": "412", "FIRST_TITLE": "Retry failed webhooks", "PREVIEW_PRS": "412,409,401", + }, + check=True, + capture_output=True, + text=True, + ) + (post,) = [c for c in calls.read_text().split("\n----\n") if c.startswith("api -X POST")] + self.assertIn("@maria-k your CodeBoarding map of `main` is ready, and #412 is reviewed on it.", post) + self.assertIn("[Open #412 in CodeBoarding](https://app.codeboarding.org/owner/repo/pull/412?", post) + self.assertIn("#409, #401 follow in a few minutes.", post) + self.assertIn("(https://app.codeboarding.org/owner/repo/pull/418?", post) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_setup_ready_notice.py b/tests/test_setup_ready_notice.py new file mode 100644 index 0000000..47fee63 --- /dev/null +++ b/tests/test_setup_ready_notice.py @@ -0,0 +1,82 @@ +"""The one new comment on a setup PR that makes GitHub email its author the map is ready.""" + +import os +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +NOTIFY = ROOT / "scripts" / "action" / "notify-setup-ready.sh" +MARKER = "" + + +class SetupReadyNoticeTests(unittest.TestCase): + def setUp(self) -> None: + self.temp_dir = tempfile.TemporaryDirectory() + self.root = Path(self.temp_dir.name) + self.bin_dir = self.root / "bin" + self.bin_dir.mkdir() + self.calls = self.root / "calls" + + def tearDown(self) -> None: + self.temp_dir.cleanup() + + def _run(self, existing: str = "", **extra: str) -> list[str]: + gh = self.bin_dir / "gh" + gh.write_text( + "#!/usr/bin/env bash\n" + f'printf "%s\\n----\\n" "$*" >> "{self.calls}"\n' + f'case "$*" in *"/comments?per_page"*) printf "%s" "{existing}" ;; esac\n', + encoding="utf-8", + ) + gh.chmod(0o755) + env = { + "PATH": f"{self.bin_dir}:{os.environ['PATH']}", + "REPOSITORY": "owner/repo", + "PR_NUMBER": "418", + "PR_AUTHOR": "maria-k", + "HEAD_REF": "codeboarding/setup-action", + "BASE_REF": "main", + **extra, + } + subprocess.run([str(NOTIFY)], env=env, check=True, capture_output=True, text=True) + if not self.calls.exists(): + return [] + return [c for c in self.calls.read_text().split("\n----\n") if c.startswith("api -X POST")] + + def test_a_setup_pr_gets_one_new_comment_that_mentions_its_author(self) -> None: + (post,) = self._run() + self.assertIn("repos/owner/repo/issues/418/comments", post) + self.assertIn("@maria-k your CodeBoarding map of `main` is ready.", post) + self.assertIn("https://app.codeboarding.org/owner/repo/pull/418?", post) + self.assertNotIn("run=", post) + self.assertIn(MARKER, post) + + def test_it_is_posted_once_per_pull_request(self) -> None: + self.assertEqual(self._run(existing="991"), []) + + def test_other_pull_requests_get_nothing(self) -> None: + self.assertEqual(self._run(HEAD_REF="feat/retry-webhooks"), []) + self.assertFalse(self.calls.exists(), "no API call at all outside the setup branch") + + def test_a_bot_author_is_not_mentioned(self) -> None: + (post,) = self._run(PR_AUTHOR="codeboarding-app[bot]") + self.assertIn("Your CodeBoarding map of `main` is ready.", post) + self.assertNotIn("@", post.split("body=", 1)[1].split("\n", 1)[0]) + + def test_the_step_runs_only_after_a_posted_review_on_the_setup_branch(self) -> None: + action = (ROOT / "action.yml").read_text(encoding="utf-8") + start = action.index(" - name: Tell the setup PR's author the map is ready") + step = action[start:action.index("\n - name:", start + 1)] + self.assertGreater(start, action.index(" - name: Post review comment")) + condition = next(line for line in step.splitlines() if line.strip().startswith("if:")) + self.assertIn("steps.review_comment.outcome == 'success'", condition) + self.assertIn("github.event_name == 'pull_request'", condition) + self.assertIn("github.head_ref == 'codeboarding/setup-action'", condition) + self.assertIn("continue-on-error: true", step, "a notification must never fail the review") + self.assertIn("notify-setup-ready.sh", step) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_with_auth.py b/tests/test_with_auth.py index 49cfdbc..5e9afb4 100644 --- a/tests/test_with_auth.py +++ b/tests/test_with_auth.py @@ -56,7 +56,13 @@ def test_every_step_but_the_last_of_a_mode_keeps_credentials(self) -> None: steps = [s for s in self._steps() if "with-auth.sh" in s and f"mode == '{mode}'" in s] self.assertTrue(steps, mode) for step in steps[:-1]: - self.assertIn("KEEP_AUTH: 'true'", step, f"{mode}: {step.splitlines()[0]} consumes the credentials") + # The head analysis keeps them only on a setup PR's run, which goes on to preview + # other pull requests; elsewhere it is the last analysis and removes them. + keeps = "KEEP_AUTH: 'true'" in step or ( + step.startswith("Analyze pull request") + and "KEEP_AUTH: ${{ github.event_name == 'pull_request' && github.head_ref == 'codeboarding/setup-action' }}" in step + ) + self.assertTrue(keeps, f"{mode}: {step.splitlines()[0]} consumes the credentials") self.assertNotIn("KEEP_AUTH", steps[-1], f"{mode}: the last analysis step must remove them") def test_kept_credentials_are_always_removed(self) -> None: