diff --git a/.github/actions/app-token/action.yml b/.github/actions/app-token/action.yml new file mode 100644 index 0000000..3a99288 --- /dev/null +++ b/.github/actions/app-token/action.yml @@ -0,0 +1,83 @@ +name: 'CodeBoarding App token' +description: >- + Mints a CodeBoarding GitHub App token so comments and baseline commits appear as + codeboarding-review[bot] (the App avatar is the CodeBoarding logo) instead of + github-actions[bot]. Fails open: with App credentials absent or invalid, `token` + is empty and the caller falls back to github.token. End-user repositories cannot + reuse this, since the App private key is never shipped to their runners. +inputs: + client_id: + description: 'vars.CODEBOARDING_APP_CLIENT_ID' + required: false + default: '' + app_id: + description: 'vars.CODEBOARDING_APP_ID' + required: false + default: '' + private_key: + description: 'secrets.CODEBOARDING_APP_PRIVATE_KEY' + required: false + default: '' +outputs: + token: + description: 'The App installation token, or empty.' + value: ${{ steps.client.outputs.token || steps.app.outputs.token }} +runs: + using: 'composite' + steps: + - name: Detect CodeBoarding GitHub App credentials + id: config + shell: bash + env: + CLIENT_ID: ${{ inputs.client_id }} + APP_ID: ${{ inputs.app_id }} + PRIVATE_KEY: ${{ inputs.private_key }} + run: | + client_id="${CLIENT_ID:-}" + app_id="${APP_ID:-}" + + # GitHub App client IDs start with "Iv". If that value was stored in + # CODEBOARDING_APP_ID, use it as a client ID to avoid the deprecated + # app-id input path. + if [ -z "$client_id" ] && [ "${app_id#Iv}" != "$app_id" ]; then + client_id="$app_id" + app_id="" + fi + + private_key_valid=false + if [ -n "$PRIVATE_KEY" ]; then + if printf '%s' "$PRIVATE_KEY" | openssl pkey -noout >/dev/null 2>&1; then + private_key_valid=true + else + echo "::warning::CODEBOARDING_APP_PRIVATE_KEY is not a valid PEM private key, so CodeBoarding will fall back to github-actions[bot]." + if printf '%b' "$PRIVATE_KEY" | openssl pkey -noout >/dev/null 2>&1; then + printf '%s\n' "::warning::CODEBOARDING_APP_PRIVATE_KEY looks like it contains literal \\n escapes. Store the downloaded PEM as multi-line secret text instead." + fi + fi + fi + + { + [ -n "$client_id" ] && echo "has_client_id=true" || echo "has_client_id=false" + [ -n "$app_id" ] && echo "has_app_id=true" || echo "has_app_id=false" + echo "client_id=$client_id" + echo "private_key_valid=$private_key_valid" + } >> "$GITHUB_OUTPUT" + - uses: actions/create-github-app-token@v3 + id: client + if: steps.config.outputs.has_client_id == 'true' && steps.config.outputs.private_key_valid == 'true' + continue-on-error: true + with: + client-id: ${{ steps.config.outputs.client_id }} + private-key: ${{ inputs.private_key }} + - uses: actions/create-github-app-token@v3 + id: app + if: steps.config.outputs.has_client_id != 'true' && steps.config.outputs.has_app_id == 'true' && steps.config.outputs.private_key_valid == 'true' + continue-on-error: true + with: + app-id: ${{ inputs.app_id }} + private-key: ${{ inputs.private_key }} + - name: Warn when CodeBoarding App token is unavailable + if: steps.client.outputs.token == '' && steps.app.outputs.token == '' + shell: bash + run: | + echo "::warning::CodeBoarding GitHub App token is unavailable; falling back to github-actions[bot]. Check CODEBOARDING_APP_PRIVATE_KEY formatting if app credentials are configured." diff --git a/.github/workflows/codeboarding-sync.yml b/.github/workflows/codeboarding-sync.yml deleted file mode 100644 index 2660515..0000000 --- a/.github/workflows/codeboarding-sync.yml +++ /dev/null @@ -1,152 +0,0 @@ -# Dogfood sync mode: keep THIS repo's committed architecture baseline -# (.codeboarding/analysis.json + rendered docs) current on every push to main, -# so the PR review workflow always diffs against an up-to-date baseline. -# -# This is the ONLY baseline writer for this repo. The manual "rebuild from -# scratch" path (previously a separate refresh-baseline.yml) is now the -# workflow_dispatch + force_full input below, which runs the same tested action -# instead of a hand-rolled copy of its pipeline. - -name: CodeBoarding sync - -on: - push: - branches: [main] - # Loop guard: don't re-trigger on the files this workflow itself commits. - # This list must name EVERY generated artifact (an un-ignored one leaks the - # bot's own commit through and re-triggers), and must name ONLY generated - # artifacts — never a user-authored input, whose edit changes analysis scope - # and must regenerate the baseline. So: deliberately NOT '.codeboarding/**' - # (would swallow .codeboarding/.codeboardingignore) and NOT - # '.codeboarding/health/**' (would swallow the user-authored health/.healthignore - # and health/health_config.json) — only the generated health/health_report.json - # is listed. In push mode the action also skips re-analyzing its own commit by - # author email as a backstop; in pull_request mode this list is the loop guard - # (a merged baseline PR touches only these files), with the commit step's - # "nothing to commit" gate stopping any stray re-run. The bot commit uses no - # [skip ci] (that would leak through squash-merges and skip real merges). - paths-ignore: - - '.codeboarding/*.md' - - '.codeboarding/analysis.json' - - '.codeboarding/fingerprint.json' - - '.codeboarding/static_analysis.pkl' - - '.codeboarding/static_analysis.sha' - - '.codeboarding/codeboarding_version.json' - - '.codeboarding/health/health_report.json' - - 'docs/development/architecture.md' - workflow_dispatch: - inputs: - force_full: - description: 'Ignore the committed baseline and rebuild it from scratch (full analysis).' - type: boolean - required: false - default: false - sync_strategy: - description: 'Deliver directly to the target branch or open/update a rolling baseline PR.' - type: choice - options: [push, pull_request] - required: false - default: push - -# No workflow-level permissions: the single job below requests only what it -# needs (least privilege), so the default token starts with none. -permissions: {} - -concurrency: - # Serialize this workflow against itself: a push landing while a manual - # dispatch is mid-run must not produce two concurrent commits to main. - group: codeboarding-baseline-writers - cancel-in-progress: false - -jobs: - sync: - runs-on: ubuntu-latest - timeout-minutes: 60 - permissions: - contents: write # push the generated baseline branch - pull-requests: write # workflow_dispatch may exercise pull_request delivery - id-token: write # mint per-request OIDC credentials for the relay - steps: - # Dogfood: run the action from the checked-out repo (uses: ./) so pushes to - # main exercise the action code on main, not the last published release. - # The action reads its scripts via github.action_path and checks the engine - # and target repo into subdirectories, so this local checkout is untouched. - - uses: actions/checkout@v4 - # Mint the CodeBoarding GitHub App token so the baseline commit lands as - # codeboarding-review[bot] (App avatar = the CodeBoarding logo) instead of - # the generic github-actions[bot]. Mirrors the review workflow's token flow. - # Fails open: when App credentials are absent/invalid we fall back to - # github.token below, and the commit shows the default Actions identity. - # End-user repos can't reuse this — the App private key is never shipped to - # their runners; they authenticate the free tier via OIDC (see README). - - name: Detect CodeBoarding GitHub App credentials - id: codeboarding-app-config - shell: bash - env: - CLIENT_ID: ${{ vars.CODEBOARDING_APP_CLIENT_ID }} - APP_ID: ${{ vars.CODEBOARDING_APP_ID }} - PRIVATE_KEY: ${{ secrets.CODEBOARDING_APP_PRIVATE_KEY }} - run: | - client_id="${CLIENT_ID:-}" - app_id="${APP_ID:-}" - - # GitHub App client IDs start with "Iv". If that value was stored in - # CODEBOARDING_APP_ID, use it as a client ID to avoid the deprecated - # app-id input path. - if [ -z "$client_id" ] && [ "${app_id#Iv}" != "$app_id" ]; then - client_id="$app_id" - app_id="" - fi - - has_private_key=false - private_key_valid=false - if [ -n "$PRIVATE_KEY" ]; then - has_private_key=true - if printf '%s' "$PRIVATE_KEY" | openssl pkey -noout >/dev/null 2>&1; then - private_key_valid=true - else - echo "::warning::CODEBOARDING_APP_PRIVATE_KEY is not a valid PEM private key, so the sync commit will fall back to github-actions[bot]." - if printf '%b' "$PRIVATE_KEY" | openssl pkey -noout >/dev/null 2>&1; then - printf '%s\n' "::warning::CODEBOARDING_APP_PRIVATE_KEY looks like it contains literal \\n escapes. Store the downloaded PEM as multi-line secret text instead." - fi - fi - fi - - { - [ -n "$client_id" ] && echo "has_client_id=true" || echo "has_client_id=false" - [ -n "$app_id" ] && echo "has_app_id=true" || echo "has_app_id=false" - echo "client_id=$client_id" - echo "has_private_key=$has_private_key" - echo "private_key_valid=$private_key_valid" - } >> "$GITHUB_OUTPUT" - - uses: actions/create-github-app-token@v3 - id: codeboarding-app-token-client - if: steps.codeboarding-app-config.outputs.has_client_id == 'true' && steps.codeboarding-app-config.outputs.private_key_valid == 'true' - continue-on-error: true - with: - client-id: ${{ steps.codeboarding-app-config.outputs.client_id }} - private-key: ${{ secrets.CODEBOARDING_APP_PRIVATE_KEY }} - - uses: actions/create-github-app-token@v3 - id: codeboarding-app-token-app - if: steps.codeboarding-app-config.outputs.has_client_id != 'true' && steps.codeboarding-app-config.outputs.has_app_id == 'true' && steps.codeboarding-app-config.outputs.private_key_valid == 'true' - continue-on-error: true - with: - app-id: ${{ vars.CODEBOARDING_APP_ID }} - private-key: ${{ secrets.CODEBOARDING_APP_PRIVATE_KEY }} - - name: Warn when CodeBoarding App token is unavailable - if: steps.codeboarding-app-token-client.outputs.token == '' && steps.codeboarding-app-token-app.outputs.token == '' - shell: bash - run: | - echo "::warning::CodeBoarding GitHub App token is unavailable; the sync commit falls back to github-actions[bot]. Check CODEBOARDING_APP_PRIVATE_KEY formatting if app credentials are configured." - - uses: ./ - with: - mode: sync - # CodeBoarding's own repositories run on the CodeBoarding plan. - llm: license - license_key: ${{ secrets.CODEBOARDING_LICENSE }} - sync_strategy: ${{ inputs.sync_strategy || 'push' }} - force_full: ${{ inputs.force_full || false }} - # App token authenticates the baseline push so the commit is attributed - # to the CodeBoarding App (logo avatar). Falls back to the workflow token, - # which can push because this job grants contents: write. - github_token: ${{ steps.codeboarding-app-token-client.outputs.token || steps.codeboarding-app-token-app.outputs.token || github.token }} diff --git a/.github/workflows/codeboarding.yml b/.github/workflows/codeboarding.yml index 3495800..342d5bd 100644 --- a/.github/workflows/codeboarding.yml +++ b/.github/workflows/codeboarding.yml @@ -1,4 +1,11 @@ -name: CodeBoarding review +# Dogfood: this repository reviews its pull requests and keeps its own baseline +# current with the action checked out here (uses: ./), so every change exercises +# the action code on that ref rather than the last published release. +# +# review pull requests and /codeboarding comments +# sync pushes to main + +name: CodeBoarding on: pull_request: @@ -8,15 +15,40 @@ on: types: [opened, reopened, closed, synchronize] issue_comment: types: [created] + push: + branches: [main] + # Loop guard: don't re-trigger on the files sync itself commits to main. + # This list must name EVERY generated artifact (an un-ignored one leaks the + # bot's own commit through and re-triggers), and must name ONLY generated + # artifacts — never a user-authored input, whose edit changes analysis scope + # and must regenerate the baseline. So: deliberately NOT '.codeboarding/**' + # (would swallow .codeboarding/.codeboardingignore) and NOT + # '.codeboarding/health/**' (would swallow the user-authored health/.healthignore + # and health/health_config.json) — only the generated health/health_report.json + # is listed. The action also skips re-analyzing its own commit by author email + # as a backstop. The bot commit uses no [skip ci] (that would leak through + # squash-merges and skip real merges). + paths-ignore: + - '.codeboarding/analysis.json' + - '.codeboarding/fingerprint.json' + - '.codeboarding/static_analysis.pkl' + - '.codeboarding/static_analysis.sha' + - '.codeboarding/codeboarding_version.json' + - '.codeboarding/health/health_report.json' -# No workflow-level permissions: the single job below requests only what it -# needs (least privilege), so the default token starts with none. +# No workflow-level permissions: each job requests only what it needs (least +# privilege), so the default token starts with none. permissions: {} +# Workflow-level, not per job: a 'closed' run skips the review job, and only a run +# that joins the group can cancel the review still in flight. +# review one group per pull request; queue, and cancel only when the PR closes — +# bot comments (issue_comment) and re-triggers must not cancel a running +# review, they queue behind it instead. +# sync one group for every baseline writer: a push landing while another +# sync is mid-run must not produce two concurrent commits to main. concurrency: - group: codeboarding-${{ github.event.pull_request.number || github.event.issue.number }} - # Cancel only when the PR closes — bot comments (issue_comment) and re-triggers - # must not cancel a running review; they queue behind it instead. + group: codeboarding-${{ github.event.pull_request.number || github.event.issue.number || 'baseline-writers' }} cancel-in-progress: ${{ github.event_name == 'pull_request' && github.event.action == 'closed' }} jobs: @@ -25,20 +57,16 @@ jobs: timeout-minutes: 60 permissions: # Review mode reads the repo + committed baseline and posts a PR comment; - # it does NOT commit generated files back to the branch (that is sync mode - # only — see action.yml `mode` input). So contents stays read-only. + # it does NOT commit generated files back to the branch (that is the sync + # job below). So contents stays read-only. contents: read actions: read # download the analysis an earlier run published pull-requests: write # post / update the architecture-diff PR comment issues: write # the /codeboarding issue_comment trigger + comment API id-token: write # mint per-request OIDC credentials for the relay - # Never auto-review the fixed machine-owned 'codeboarding/sync' PR: it only - # changes generated files, so a diff comment would be noise. Scope this to - # this repository so a fork using the same branch name is still reviewed. if: > (github.event_name == 'pull_request' && github.event.action != 'closed' && - github.event.pull_request.head.repo.full_name == github.repository && - !(github.head_ref == 'codeboarding/sync' && github.event.pull_request.head.repo.full_name == github.repository)) || + github.event.pull_request.head.repo.full_name == github.repository) || (github.event_name == 'issue_comment' && github.event.issue.pull_request != null && startsWith(github.event.comment.body, '/codeboarding') && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)) @@ -49,65 +77,12 @@ jobs: - uses: actions/checkout@v4 with: ref: ${{ github.event_name == 'issue_comment' && github.event.repository.default_branch || '' }} - - name: Detect CodeBoarding GitHub App credentials - id: codeboarding-app-config - shell: bash - env: - CLIENT_ID: ${{ vars.CODEBOARDING_APP_CLIENT_ID }} - APP_ID: ${{ vars.CODEBOARDING_APP_ID }} - PRIVATE_KEY: ${{ secrets.CODEBOARDING_APP_PRIVATE_KEY }} - run: | - client_id="${CLIENT_ID:-}" - app_id="${APP_ID:-}" - - # GitHub App client IDs start with "Iv". If that value was stored in - # CODEBOARDING_APP_ID, use it as a client ID to avoid the deprecated - # app-id input path. - if [ -z "$client_id" ] && [ "${app_id#Iv}" != "$app_id" ]; then - client_id="$app_id" - app_id="" - fi - - has_private_key=false - private_key_valid=false - if [ -n "$PRIVATE_KEY" ]; then - has_private_key=true - if printf '%s' "$PRIVATE_KEY" | openssl pkey -noout >/dev/null 2>&1; then - private_key_valid=true - else - echo "::warning::CODEBOARDING_APP_PRIVATE_KEY is not a valid PEM private key, so CodeBoarding will fall back to github-actions[bot]." - if printf '%b' "$PRIVATE_KEY" | openssl pkey -noout >/dev/null 2>&1; then - printf '%s\n' "::warning::CODEBOARDING_APP_PRIVATE_KEY looks like it contains literal \\n escapes. Store the downloaded PEM as multi-line secret text instead." - fi - fi - fi - - { - [ -n "$client_id" ] && echo "has_client_id=true" || echo "has_client_id=false" - [ -n "$app_id" ] && echo "has_app_id=true" || echo "has_app_id=false" - echo "client_id=$client_id" - echo "has_private_key=$has_private_key" - echo "private_key_valid=$private_key_valid" - } >> "$GITHUB_OUTPUT" - - uses: actions/create-github-app-token@v3 - id: codeboarding-app-token-client - if: steps.codeboarding-app-config.outputs.has_client_id == 'true' && steps.codeboarding-app-config.outputs.private_key_valid == 'true' - continue-on-error: true - with: - client-id: ${{ steps.codeboarding-app-config.outputs.client_id }} - private-key: ${{ secrets.CODEBOARDING_APP_PRIVATE_KEY }} - - uses: actions/create-github-app-token@v3 - id: codeboarding-app-token-app - if: steps.codeboarding-app-config.outputs.has_client_id != 'true' && steps.codeboarding-app-config.outputs.has_app_id == 'true' && steps.codeboarding-app-config.outputs.private_key_valid == 'true' - continue-on-error: true + - uses: ./.github/actions/app-token + id: app with: - app-id: ${{ vars.CODEBOARDING_APP_ID }} - private-key: ${{ secrets.CODEBOARDING_APP_PRIVATE_KEY }} - - name: Warn when CodeBoarding App token is unavailable - if: steps.codeboarding-app-token-client.outputs.token == '' && steps.codeboarding-app-token-app.outputs.token == '' - shell: bash - run: | - echo "::warning::CodeBoarding GitHub App token is unavailable; falling back to github-actions[bot]. Check CODEBOARDING_APP_PRIVATE_KEY formatting if app credentials are configured." + client_id: ${{ vars.CODEBOARDING_APP_CLIENT_ID }} + app_id: ${{ vars.CODEBOARDING_APP_ID }} + private_key: ${{ secrets.CODEBOARDING_APP_PRIVATE_KEY }} - uses: ./ with: # Named, not inferred: the action refuses to guess where credentials come @@ -115,4 +90,35 @@ jobs: # wants. CodeBoarding's own repositories run on the CodeBoarding plan. llm: license license_key: ${{ secrets.CODEBOARDING_LICENSE }} - github_token: ${{ steps.codeboarding-app-token-client.outputs.token || steps.codeboarding-app-token-app.outputs.token || github.token }} + # This repository commits its analysis to main, so reviews read it there. + codeboarding_analysis_location: in_place + github_token: ${{ steps.app.outputs.token || github.token }} + + # The ONLY baseline writer for this repository, started by pushes to main only: + # the push filter above is what decides which branch is synced. + sync: + if: github.event_name == 'push' + runs-on: ubuntu-latest + timeout-minutes: 60 + permissions: + contents: write # push the generated baseline + actions: read # catch up from an analysis a review saved + id-token: write # mint per-request OIDC credentials for the relay + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/app-token + id: app + with: + client_id: ${{ vars.CODEBOARDING_APP_CLIENT_ID }} + app_id: ${{ vars.CODEBOARDING_APP_ID }} + private_key: ${{ secrets.CODEBOARDING_APP_PRIVATE_KEY }} + - uses: ./ + with: + mode: sync + llm: license + license_key: ${{ secrets.CODEBOARDING_LICENSE }} + # Must match the review job above, which reads it. + codeboarding_analysis_location: in_place + # The App token attributes the baseline commit to the CodeBoarding App. + # The workflow token can push too, since this job grants contents: write. + github_token: ${{ steps.app.outputs.token || github.token }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e910cb9..280e3be 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -74,4 +74,4 @@ jobs: - name: Run actionlint run: actionlint - name: Run shellcheck - run: shellcheck scripts/run_local.sh + run: shellcheck -x scripts/run_local.sh scripts/action/*.sh diff --git a/AGENTS.md b/AGENTS.md index fd9fbbd..7492a2f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,9 +5,11 @@ This repo is a GitHub Action with two modes, selected by the `mode` input: - **`mode: review`** (default): analyzes a PR with the CodeBoarding engine and posts a Mermaid diagram of the added, modified, and removed components as a PR comment (runs on `pull_request` / `issue_comment`). -- **`mode: sync`**: on push to a branch, regenerates the architecture and commits - the versioned baseline (`.codeboarding/analysis.json` + rendered markdown) back - to the branch, so review mode always diffs against a current baseline. +- **`mode: sync`**: on push to the one branch its trigger lists, regenerates the + analysis and saves the `.codeboarding/` state where + `codeboarding_analysis_location` says (an orphan `codeboarding/baseline` branch + by default, or `in_place` on that branch itself), so review mode always diffs + against a current baseline. The action is a thin orchestration wrapper, not the analysis engine: the engine (`CodeBoarding/CodeBoarding`) is a separate repo checked out at runtime and @@ -82,6 +84,8 @@ bump moves adopters to `v2` and freezes `v1`, which would have left every existing workflow on the old silent-fallback behaviour forever — the opposite of the intent. Shipping it as a minor bump on the moving `v1` tag is what makes adopters actually receive it. Do not "correct" this to `feat!:` after the fact. +The analysis-location change (#143) is a second such exception: sync's default +moved from committing to the synced branch to `codeboarding/baseline`. - `chore:` / `docs:` / `ci:` / `refactor:` / `test:` → ride along in the next release but do not trigger one. diff --git a/README.md b/README.md index 922b666..922386e 100644 --- a/README.md +++ b/README.md @@ -3,40 +3,37 @@ One GitHub Action with two modes: - **`review`** (default) compares a pull request's head with its merge base, posts an inline Mermaid architecture diff, and uploads both analyses as a workflow artifact. -- **`sync`** updates the versioned analysis state used by future incremental runs. It can push directly or open one rolling PR for protected branches. +- **`sync`** updates the versioned analysis state used by future incremental runs. By default it saves to a branch of its own, so code branches are never written. The action is a thin wrapper around the [CodeBoarding](https://github.com/CodeBoarding/CodeBoarding) CLI. Analysis logic and provider defaults live in Core, not in this repository. [CodeBoarding](https://github.com/CodeBoarding/CodeBoarding) · [Website](https://codeboarding.org) · [Examples](https://codeboarding.org/diagrams) · [VS Code extension](https://marketplace.visualstudio.com/items?itemName=Codeboarding.codeboarding) · [Discord](https://discord.gg/T5zHTJYFuy) -## Review pull requests +## Set up -Create `.github/workflows/codeboarding.yml`: +CodeBoarding runs from one workflow, `.github/workflows/codeboarding.yml`, with a `review` job and a `sync` job. Each job asks only for the permissions it needs. Replace `main` with your default branch: ```yaml -name: CodeBoarding review +name: CodeBoarding on: pull_request: types: [opened, reopened, synchronize] issue_comment: types: [created] + push: + branches: [main] + +permissions: {} -permissions: - contents: read - actions: read # download the analysis an earlier run published - pull-requests: write - issues: write - id-token: write - -# One review at a time per pull request. Two pushes in quick succession would -# otherwise analyze concurrently, and both would start from the same older -# analysis instead of the newer one continuing from its predecessor. They also -# share one sticky comment, so whichever finishes last wins — which can be the -# run for the older commit. Queue rather than cancel, so a /codeboarding command -# waits for a running review instead of killing it. +# One review at a time per pull request, and one sync at a time per branch. Two +# pushes in quick succession would otherwise analyze concurrently, both starting +# from the same older analysis instead of the newer one continuing from its +# predecessor, and both writing the one sticky comment, so the run for the older +# commit could win. Queue rather than cancel, so a /codeboarding command waits for +# a running review instead of killing it. concurrency: - group: codeboarding-${{ github.event.pull_request.number || github.event.issue.number }} + group: codeboarding-${{ github.event.pull_request.number || github.event.issue.number || github.ref_name }} cancel-in-progress: false jobs: @@ -49,13 +46,37 @@ jobs: contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)) runs-on: ubuntu-latest timeout-minutes: 60 + permissions: + contents: read + actions: read # download the analysis an earlier run published + pull-requests: write + issues: write + id-token: write steps: - uses: CodeBoarding/CodeBoarding-action@v1 with: llm: hosted # or license, or a provider name -- see Authentication + + sync: + if: github.event_name == 'push' + runs-on: ubuntu-latest + timeout-minutes: 60 + permissions: + contents: write + actions: read # catch up from an analysis a review saved + id-token: write + steps: + - uses: CodeBoarding/CodeBoarding-action@v1 + with: + mode: sync + llm: hosted ``` -Automatic runs review both draft and non-draft pull requests and update one sticky **CodeBoarding review** comment. Opening, reopening, or pushing a commit runs analysis; changing only the draft state does not. A trusted repository owner, member, or collaborator can comment `/codeboarding` to analyze the current PR head again, including on fork PRs; every command creates a new result comment. +Both jobs use the default `codeboarding_analysis_location`, so review always reads the analysis where sync saves it. If you set it, set it in both jobs. Keep both jobs in this one file: a repository running CodeBoarding from more than one workflow file is an [older setup](#moving-an-existing-setup), which the CodeBoarding dashboard flags and offers to fold into one. + +## Review pull requests + +The `review` job's automatic runs review both draft and non-draft pull requests and update one sticky **CodeBoarding review** comment. Opening, reopening, or pushing a commit runs analysis; changing only the draft state does not. A trusted repository owner, member, or collaborator can comment `/codeboarding` to analyze the current PR head again, including on fork PRs; every command creates a new result comment. `synchronize` re-runs the review on every push to the branch. Each of those runs covers only the commits pushed since the previous one, so a push costs a fraction of a first analysis — and a pushed commit is the only thing that builds the reusable analysis, since GitHub gives comment-triggered runs a read-only cache. Drop `synchronize` from the list if you would rather spend one analysis per pull request than one per push. @@ -223,7 +244,7 @@ Set only `model` when both jobs should use the same model. Set either specialize ## Keep the baseline current -Sync mode commits only Core's persisted incremental-analysis state under `.codeboarding/`: +Sync mode saves Core's persisted incremental-analysis state under `.codeboarding/`: - `analysis.json` - `fingerprint.json` @@ -232,13 +253,31 @@ Sync mode commits only Core's persisted incremental-analysis state under `.codeb - `codeboarding_version.json` when emitted by Core - `health/health_report.json` -It does **not** render or commit architecture Markdown. Existing v1-generated `.codeboarding/*.md` and `docs/development/architecture.md` files carrying CodeBoarding's generated badge are removed on the first v2 sync. Hand-written Markdown and user-authored CodeBoarding configuration, including `health/health_config.json` and `health/.healthignore`, are preserved. +It does **not** render or commit architecture Markdown, and leaves every other file alone, including user-authored CodeBoarding configuration such as `health/health_config.json` and `health/.healthignore`. + +Sync keeps the analysis of your repository's default branch current. `codeboarding_analysis_location` decides where that analysis is stored, `codeboarding/baseline` by default. The `sync` job does this, on pushes to the default branch: a sync that runs on any other branch, from any event, fails in seconds, before anything is installed. + +`main` is only read. Each sync adds one commit to `codeboarding/baseline` in the same repository, an orphan branch that shares no history with `main`. It holds the `.codeboarding/` files listed above, plus `.codeboarding/source.json` naming the commit they describe and the configuration that made them; the commit message carries both as `CodeBoarding-Source:` and `CodeBoarding-Config:` trailers. Pushes only ever fast-forward, and no code branch is ever written, so the `push` trigger needs no `paths-ignore` and a protected `main` needs no exception. Reviews read their base from the branch, and the web platform reads the latest diagram from it. The [analysis branch section](docs/COMMIT_STRATEGY.md#the-baseline-branch) covers what happens if the branch is deleted, and a ruleset you should import to protect it: sync and review load a pickle from it. + +The branch push itself needs nothing beyond `contents: write`: the first sync creates the branch with an ordinary push. If you import the ruleset that protects it, import the [GitHub Actions variant](docs/baseline-branch-ruleset-actions.json): the example pushes with the default `github.token`, which the CodeBoarding app ruleset refuses, so the sync would fail at the push after a full analysis. -Create `.github/workflows/codeboarding-sync.yml`: +If sync pushes with an App token or a personal access token rather than the default `github.token`, its push to `codeboarding/baseline` starts your own `on: push` workflows, which then run on a branch with no code and fail. Exclude it from them: ```yaml -name: CodeBoarding sync +on: + push: + branches-ignore: ['codeboarding/**'] +``` + +Entries are keyed by the commit they describe, so renaming the default branch, say `master` to `main`, just continues incrementally. + +The first run, or one without a compatible saved analysis to continue from, is a full analysis. Otherwise sync asks Core for an incremental update. If the generated state is unchanged, no commit is created. If the synced branch advances while analysis is running, the result is still saved for the commit it analyzed, unless the analysis branch already holds a newer commit's. + +### Keep the analysis on the code branch +Set `codeboarding_analysis_location: in_place` to commit `.codeboarding/` to the synced branch itself instead, next to the code. Set it on the review job as well, so reviews read it from there, and add a `paths-ignore` so sync's own commit does not trigger another sync: + +```yaml on: push: branches: [main] @@ -248,58 +287,42 @@ on: - '.codeboarding/static_analysis.pkl' - '.codeboarding/static_analysis.sha' - '.codeboarding/codeboarding_version.json' - workflow_dispatch: - inputs: - force_full: - description: Rebuild without the committed baseline - type: boolean - default: false - -permissions: - contents: write - id-token: write - -concurrency: - group: codeboarding-sync - cancel-in-progress: false - -jobs: - sync: - runs-on: ubuntu-latest - timeout-minutes: 60 - steps: - - uses: CodeBoarding/CodeBoarding-action@v1 - with: - mode: sync - llm: hosted - target_branch: main - force_full: ${{ inputs.force_full || false }} -``` - -The first run, `force_full: true`, or an incompatible baseline causes a full analysis. Otherwise sync asks Core for an incremental update. If the generated state is unchanged, no commit is created. If the target advances while analysis is running, the stale result is not rebased onto code it did not analyze; the newer push run is allowed to produce the current baseline. - -### Protected branches - -Set `sync_strategy: pull_request` and grant `pull-requests: write`: - -```yaml -permissions: - contents: write - pull-requests: write - id-token: write + - '.codeboarding/health/health_report.json' # ... - uses: CodeBoarding/CodeBoarding-action@v1 with: mode: sync llm: hosted - target_branch: main - sync_strategy: pull_request + codeboarding_analysis_location: in_place ``` -Generation is identical to direct push. Only delivery changes: the same commit is force-with-lease pushed to the machine-owned `codeboarding/sync` branch and one rolling PR is opened into `target_branch`. When there is no longer a generated diff, an obsolete rolling PR is closed. +The list names only generated files on purpose: an edit to `.codeboarding/.codeboardingignore` or the health configuration changes what is analyzed, and must still trigger a sync. Sync pushes to `main` directly, so a branch rule protecting `main` needs the identity sync pushes with (the CodeBoarding app, or GitHub Actions for the default token) as a bypass actor, or a `github_token` that may push there. + +### Moving an existing setup + +Before this release sync committed `.codeboarding/` to the synced branch by default; it now saves to `codeboarding/baseline`, and runs on the default branch only. A workflow that set `sync_strategy: push` keeps committing to the code branch, and one that set `sync_strategy: pull_request` moves to the analysis branch, each with a warning. A `target_branch` naming a branch other than the one sync runs on fails, since sync now analyzes the branch it runs on. A workflow that set none of them moves to the analysis branch; the `.codeboarding/` committed on the code branch is then no longer updated. Separate review and sync workflow files still run, but belong in [one file](#set-up). The CodeBoarding dashboard flags all of these and opens the pull request that moves them; to make the move yourself, paste this into your coding agent: + +```text +Move this repository's CodeBoarding sync to the codeboarding/baseline analysis branch. +1. Put the review and sync jobs that run CodeBoarding/CodeBoarding-action in one + file, .github/workflows/codeboarding.yml, as the action README's "Set up" shows, + and delete the workflow files they came from once nothing else is left in them. +2. In those jobs, remove the sync_strategy, target_branch, force_full and + codeboarding_analysis_location inputs if set, and any workflow_dispatch trigger + that starts the sync job. Keep every other input. List only the default branch + under the push trigger. +3. Delete the files sync generated from the default branch: + .codeboarding/analysis.json, fingerprint.json, static_analysis.pkl, + static_analysis.sha, codeboarding_version.json and health/health_report.json. + Leave everything else under .codeboarding/. +4. Remove the paths-ignore entries for those files from the push trigger, and any + .gitattributes lines that mark them as linguist-generated. +5. Open a pull request with these changes. After it merges, close any open + pull request from the codeboarding/sync branch and delete that branch. +``` -With the default `github.token`, the repository or organization must allow GitHub Actions to create pull requests. A GitHub App token or PAT can instead be passed as `github_token`. The same input is used for review comments and sync delivery. +The first sync after the merge creates `codeboarding/baseline`, catching up from a saved analysis when there is one. To keep committing to the code branch instead, set `codeboarding_analysis_location: in_place`, as [above](#keep-the-analysis-on-the-code-branch). ## Inputs @@ -316,14 +339,14 @@ With the default `github.token`, the repository or organization must allow GitHu | `parsing_model` | both | empty | Parsing-only override for `model`. | | `depth_cap` | both | `2` | Positive integer maximum analysis depth, including full-analysis fallbacks. Changing it rebuilds incompatible state. | | `github_token` | both | `${{ github.token }}` | Token for comments and sync delivery. | -| `sync_strategy` | sync | `push` | `push` or `pull_request`. | -| `target_branch` | sync | event branch | Branch receiving the baseline or rolling PR. | -| `force_full` | sync | `false` | Ignore the committed baseline for this run. | +| `codeboarding_analysis_location` | both | `codeboarding_branch` | Where the synced branch's analysis is stored: `codeboarding_branch` (on `codeboarding/baseline`, for the default branch; the code is never written) or `in_place` (committed as `.codeboarding/` on the synced branch). Sync saves it there and review reads it from there. | | `warmstart_retention_days` | review | `1` | Days to keep the reusable analysis. Only the next run reads it. | -The `/codeboarding` command, comment heading, Mermaid direction (`LR`), hosted webview URL, rolling sync branch, commit message, and CodeBoarding 0.14.5 version are intentionally fixed rather than exposed as configuration. +`target_branch` and `sync_strategy` are deprecated, and read as the `codeboarding_analysis_location` they stand for, with a warning: `sync_strategy: push` as `in_place`, `sync_strategy: pull_request` as `codeboarding_branch`. `target_branch` is accepted only when it names the branch the run is on: the push trigger picks the branch. `force_full: true` is ignored with a warning. Anything the new input cannot express fails, linking to [moving an existing setup](#moving-an-existing-setup). + +The `/codeboarding` command, comment heading, Mermaid direction (`LR`), hosted webview URL, commit message, and CodeBoarding 0.14.5 version are intentionally fixed rather than exposed as configuration. -Review mode needs no sync workflow or committed `.codeboarding` directory. If no +Review mode needs no sync job or committed `.codeboarding` directory. If no usable merge-base analysis exists, it runs full analysis there directly, then seeds an incremental analysis of the PR head and publishes both states. Later runs prefer compatible prior PR state for incremental updates, while the review @@ -361,12 +384,10 @@ breaking CLI migration. | `analysis_mode` | sync | `incremental` or `full`. | | `files_written` | sync | Number of persisted analysis artifacts produced. | | `committed` | sync | Whether a baseline commit was delivered. | -| `sync_pr_url` | sync | Rolling PR URL for PR delivery. | -| `sync_pr_number` | sync | Rolling PR number for PR delivery. | ## GitHub Enterprise Server -Repository fetches, comments, pushes, and rolling-PR API calls use `github.server_url`; GitHub.com is not hardcoded for repository operations. The hosted CodeBoarding webview and LLM proxy remain CodeBoarding-operated production services. +Repository fetches, comments, and pushes use `github.server_url`; GitHub.com is not hardcoded for repository operations. The hosted CodeBoarding webview and LLM proxy remain CodeBoarding-operated production services. ## Local test harness diff --git a/action.yml b/action.yml index ab7f483..33ef7d0 100644 --- a/action.yml +++ b/action.yml @@ -142,22 +142,37 @@ inputs: description: 'Token used for comments and sync delivery.' required: false default: ${{ github.token }} - sync_strategy: - description: 'Sync delivery method: push or pull_request.' + codeboarding_analysis_location: + description: >- + Where the analysis of the synced branch (the one branch the sync workflow's + push trigger lists) is stored. Sync saves it there and review reads its base + from there, so set the same value for both. + codeboarding_branch (default): on codeboarding/baseline, an orphan branch of its + own with one commit per sync; the synced branch is never written. Sync then runs + on the repository's default branch only. + in_place: committed as .codeboarding/ on the synced branch itself. + required: false + default: 'codeboarding_branch' + warmstart_retention_days: + description: 'Days to keep the reusable analysis. Only the next run reads it.' required: false - default: 'push' + default: '1' + # Deprecated inputs. scripts/action/old_api_migrator.sh is the only reader. target_branch: - description: 'Branch updated by sync mode. Defaults to the event branch.' + description: 'Deprecated: sync analyzes the branch the run is on. Accepted only when it names that branch.' + deprecationMessage: 'target_branch is deprecated; sync analyzes the branch the run is on.' required: false default: '' - force_full: - description: 'Run a full sync analysis instead of reusing the committed baseline.' + sync_strategy: + description: 'Deprecated: use codeboarding_analysis_location (push is in_place, pull_request is codeboarding_branch).' + deprecationMessage: 'sync_strategy is deprecated; use codeboarding_analysis_location.' required: false - default: 'false' - warmstart_retention_days: - description: 'Days to keep the reusable analysis. Only the next run reads it.' + default: '' + force_full: + description: 'Deprecated and ignored: sync always continues from the nearest saved analysis.' + deprecationMessage: 'force_full is deprecated and ignored.' required: false - default: '1' + default: '' outputs: llm_tier: description: 'Resolved credential tier: hosted, license, byok, or byok+license.' @@ -195,16 +210,22 @@ outputs: committed: description: 'Whether sync delivered a baseline commit.' value: ${{ steps.sync_commit.outputs.committed }} - sync_pr_url: - description: 'Rolling sync PR URL when sync_strategy is pull_request.' - value: ${{ steps.sync_commit.outputs.sync_pr_url }} - sync_pr_number: - description: 'Rolling sync PR number when sync_strategy is pull_request.' - value: ${{ steps.sync_commit.outputs.sync_pr_number }} runs: using: 'composite' steps: + - name: Translate deprecated inputs + id: inputs + shell: bash + env: + MODE: ${{ inputs.mode }} + REF_NAME: ${{ github.ref_name }} + ANALYSIS_LOCATION: ${{ inputs.codeboarding_analysis_location }} + OLD_TARGET_BRANCH: ${{ inputs.target_branch }} + OLD_SYNC_STRATEGY: ${{ inputs.sync_strategy }} + OLD_FORCE_FULL: ${{ inputs.force_full }} + run: "$GITHUB_ACTION_PATH/scripts/action/old_api_migrator.sh" + - name: Resolve event id: guard shell: bash @@ -216,10 +237,8 @@ runs: EVENT: ${{ github.event_name }} REF_NAME: ${{ github.ref_name }} REF_TYPE: ${{ github.ref_type }} - EVENT_SHA: ${{ github.sha }} HEAD_AUTHOR_EMAIL: ${{ github.event.head_commit.author.email }} - TARGET_BRANCH_INPUT: ${{ inputs.target_branch }} - SYNC_STRATEGY: ${{ inputs.sync_strategy }} + ANALYSIS_LOCATION: ${{ steps.inputs.outputs.analysis_location }} COMMENT_BODY: ${{ github.event.comment.body }} AUTHOR_ASSOCIATION: ${{ github.event.comment.author_association }} ISSUE_PR_URL: ${{ github.event.issue.pull_request.url }} @@ -230,7 +249,8 @@ runs: PULL_HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} PULL_BASE_REPO: ${{ github.event.pull_request.base.repo.full_name }} REPOSITORY: ${{ github.repository }} - run: "$GITHUB_ACTION_PATH/scripts/action/guard.sh" + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + run: "$GITHUB_ACTION_PATH/scripts/action/resolve-github-event.sh" - name: Acknowledge review command if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.event == 'issue_comment' @@ -331,7 +351,21 @@ runs: fi exit 1 - - name: Post review progress + # Before the checkout and the engine install, like the credential check: a sync + # that could not save its result fails in seconds, not after a full analysis. + - name: Check analysis branch + id: analysis_check + if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync' + shell: bash + env: + BASELINE_BRANCH: ${{ steps.guard.outputs.analysis_branch }} + SYNCED_BRANCH: ${{ steps.guard.outputs.synced_branch }} + GIT_TOKEN: ${{ inputs.github_token }} + GITHUB_SERVER_URL: ${{ github.server_url }} + REPOSITORY: ${{ github.repository }} + run: "$GITHUB_ACTION_PATH/scripts/action/check-analysis-branch.sh" + + - name: Update review progress if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' continue-on-error: true uses: marocchino/sticky-pull-request-comment@v2 @@ -387,10 +421,9 @@ runs: ACTION_PATH: ${{ github.action_path }} run: "$GITHUB_ACTION_PATH/scripts/action/configure-auth.sh" - - name: Resolve analysis identity + - name: Resolve analysis cache keys id: state if: steps.guard.outputs.skip != 'true' - continue-on-error: true shell: bash env: CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target @@ -403,7 +436,7 @@ runs: MODEL: ${{ inputs.model }} AGENT_MODEL_INPUT: ${{ inputs.agent_model }} PARSING_MODEL_INPUT: ${{ inputs.parsing_model }} - run: "$GITHUB_ACTION_PATH/scripts/action/state-names.sh" + run: "$GITHUB_ACTION_PATH/scripts/action/resolve-cache-keys.sh" # Both lookups are best effort. Without them the run derives the base from # the committed baseline and the head from the base, which is what every run @@ -438,21 +471,27 @@ runs: DEST: ${{ runner.temp }}/cb-state/${{ github.action }}/warmstart run: "$GITHUB_ACTION_PATH/scripts/action/fetch-state.sh" - - name: Analyze baseline + - name: Generate sync baseline id: sync_analyze if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync' shell: bash env: ACTION_PATH: ${{ github.action_path }} - ANALYSIS_KIND: sync CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target STAGE_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/out - FORCE_FULL: ${{ inputs.force_full }} + BASELINE_BRANCH: ${{ steps.guard.outputs.analysis_branch }} + CFG_HASH: ${{ steps.state.outputs.cfg_hash }} + # Lets a branch without a usable committed baseline catch up from a saved analysis. + ANCESTOR_LOOKUP: ${{ github.server_url == 'https://github.com' && steps.state.outputs.cfg_hash != '' }} + GIT_TOKEN: ${{ inputs.github_token }} + REPOSITORY: ${{ github.repository }} + GH_HOST: ${{ github.server_url }} + GITHUB_SERVER_URL: ${{ github.server_url }} 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/analyze.sh"' + run: '"$GITHUB_ACTION_PATH/scripts/action/with-auth.sh" "$GITHUB_ACTION_PATH/scripts/action/sync-generate-baseline.sh"' - name: Deliver baseline id: sync_commit @@ -462,13 +501,12 @@ runs: ACTION_PATH: ${{ github.action_path }} ANALYSIS_DIR: ${{ steps.sync_analyze.outputs.analysis_dir }} CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target - TARGET_BRANCH: ${{ steps.guard.outputs.target_branch }} - SYNC_BRANCH_START_SHA: ${{ steps.guard.outputs.sync_branch_start_sha }} - SYNC_STRATEGY: ${{ inputs.sync_strategy }} + SYNCED_BRANCH: ${{ steps.guard.outputs.synced_branch }} + BASELINE_BRANCH: ${{ steps.guard.outputs.analysis_branch }} + ENGINE_VERSION: ${{ steps.state.outputs.engine_version }} + # Recorded with each baseline-branch commit, so a reader can tell which configuration made it. + CFG_HASH: ${{ steps.state.outputs.cfg_hash }} GITHUB_TOKEN: ${{ inputs.github_token }} - GH_TOKEN: ${{ inputs.github_token }} - GH_ENTERPRISE_TOKEN: ${{ inputs.github_token }} - GH_HOST: ${{ github.server_url }} GITHUB_SERVER_URL: ${{ github.server_url }} REPOSITORY: ${{ github.repository }} run: "$GITHUB_ACTION_PATH/scripts/action/deliver-sync.sh" @@ -516,25 +554,31 @@ runs: MODE: ${{ steps.sync_analyze.outputs.analysis_mode }} COMMITTED: ${{ steps.sync_commit.outputs.committed }} FILES: ${{ steps.sync_commit.outputs.files_written }} - STRATEGY: ${{ inputs.sync_strategy }} - PR_URL: ${{ steps.sync_commit.outputs.sync_pr_url }} + SAVED_TO: ${{ steps.guard.outputs.analysis_branch || steps.guard.outputs.synced_branch }} + # A failed sync has no pull request to comment on; the summary is where it says why. + FAILURE_REASON: ${{ steps.guard.outputs.failure_reason || steps.analysis_check.outputs.failure_reason || steps.state.outputs.failure_reason || steps.sync_analyze.outputs.failure_reason || steps.sync_commit.outputs.failure_reason }} run: "$GITHUB_ACTION_PATH/scripts/action/sync-summary.sh" - - name: Analyze pull request - id: review_analyze + - name: Generate review baseline + id: review_baseline if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' shell: bash env: ACTION_PATH: ${{ github.action_path }} - ANALYSIS_KIND: review CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target REVIEW_BASE_SHA: ${{ steps.guard.outputs.merge_base_sha }} - REVIEW_HEAD_SHA: ${{ steps.guard.outputs.head_sha }} REVIEW_BASE_REPO: ${{ steps.guard.outputs.base_repo }} - PR_NUMBER: ${{ steps.guard.outputs.pr_number }} BASE_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/base - WARMSTART_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/warmstart RENEW_BASE: ${{ steps.fetch_base.outputs.renew }} + BASELINE_BRANCH: ${{ steps.guard.outputs.analysis_branch }} + # Lets a merge base without a saved analysis catch up from an ancestor's. + ANCESTOR_LOOKUP: ${{ github.server_url == 'https://github.com' && steps.state.outputs.cfg_hash != '' }} + # For the progress comment while a base is built from scratch. + PROGRESS_HEADER: ${{ steps.guard.outputs.comment_id }} + PR_NUMBER: ${{ steps.guard.outputs.pr_number }} + BASE_REF: ${{ steps.guard.outputs.base_ref }} + REPOSITORY: ${{ github.repository }} + GH_HOST: ${{ github.server_url }} STAGE_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/out ENGINE_VERSION: ${{ steps.state.outputs.engine_version }} CFG_HASH: ${{ steps.state.outputs.cfg_hash }} @@ -544,7 +588,36 @@ runs: 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/analyze.sh"' + # The head step below analyzes with the same credentials, and removes them. + KEEP_AUTH: 'true' + run: '"$GITHUB_ACTION_PATH/scripts/action/with-auth.sh" "$GITHUB_ACTION_PATH/scripts/action/review-generate-baseline.sh"' + + - name: Analyze pull request + id: review_analyze + if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' + shell: bash + env: + ACTION_PATH: ${{ github.action_path }} + CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target + BASE_ANALYSIS_PATH: ${{ steps.review_baseline.outputs.base_analysis_path }} + WARMSTART_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/warmstart + REVIEW_BASE_SHA: ${{ steps.guard.outputs.merge_base_sha }} + REVIEW_HEAD_SHA: ${{ steps.guard.outputs.head_sha }} + PR_NUMBER: ${{ steps.guard.outputs.pr_number }} + STAGE_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/out + 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-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 @@ -564,7 +637,7 @@ runs: # never be what they read. - name: Publish base analysis id: publish_base - if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_analyze.outputs.publish_base == 'true' && steps.guard.outputs.is_fork != 'true' && steps.state.outputs.base_name != '' && github.server_url == 'https://github.com' + if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_baseline.outputs.publish_base == 'true' && steps.guard.outputs.is_fork != 'true' && steps.state.outputs.base_name != '' && github.server_url == 'https://github.com' continue-on-error: true uses: actions/upload-artifact@v4 with: @@ -585,7 +658,7 @@ runs: shell: bash env: ACTION_PATH: ${{ github.action_path }} - BASE_ANALYSIS_PATH: ${{ steps.review_analyze.outputs.base_analysis_path }} + BASE_ANALYSIS_PATH: ${{ steps.review_baseline.outputs.base_analysis_path }} HEAD_ANALYSIS_PATH: ${{ steps.review_analyze.outputs.analysis_path }} run: "$GITHUB_ACTION_PATH/scripts/action/render-review.sh" @@ -597,7 +670,7 @@ runs: ANALYSIS_PATH: ${{ steps.review_analyze.outputs.analysis_path }} BASE_ARTIFACT_NAME: ${{ steps.state.outputs.base_name }} BASE_ARTIFACT_ID: ${{ steps.publish_base.outputs.artifact-id || steps.fetch_base.outputs.artifact_id }} - BASE_ANALYSIS_PATH: ${{ steps.review_analyze.outputs.base_analysis_path }} + BASE_ANALYSIS_PATH: ${{ steps.review_baseline.outputs.base_analysis_path }} INLINE_BASE: ${{ steps.publish_base.outputs.artifact-id == '' && steps.fetch_base.outputs.artifact_id == '' }} ANALYSIS_MODE: ${{ steps.review_analyze.outputs.analysis_mode }} BASE_SHA: ${{ steps.guard.outputs.base_sha }} @@ -679,6 +752,6 @@ runs: message: | ### CodeBoarding review · failed - ${{ steps.review_analyze.outputs.failure_reason }} + ${{ steps.review_baseline.outputs.failure_reason || steps.review_analyze.outputs.failure_reason }} See the [workflow logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}). diff --git a/docs/COMMIT_STRATEGY.md b/docs/COMMIT_STRATEGY.md index 2849dfb..49d5712 100644 --- a/docs/COMMIT_STRATEGY.md +++ b/docs/COMMIT_STRATEGY.md @@ -3,11 +3,12 @@ Where CodeBoarding keeps analysis state, and how a review run finds the two graphs it compares. -## Three stores +## Where analysis is stored | Store | Holds | Lifetime | Who can read it | |---|---|---|---| -| **Git** (`sync` commits) | `.codeboarding/` on the target branch | forever | anyone with repo read | +| **Git, analysis branch** (`codeboarding_analysis_location: codeboarding_branch`, the default) | one commit per sync on `codeboarding/baseline` | forever | anyone with repo read | +| **Git, code branch** (`codeboarding_analysis_location: in_place`) | `.codeboarding/` on the synced branch | forever | anyone with repo read | | **Workflow artifacts** | every analysis this action reuses or publishes | a retention window | any run with `actions: read`, plus humans | | ~~Actions cache~~ | — | — | not used | @@ -70,7 +71,9 @@ never fires. | `base_artifact_id` | string | **which one**, since two artifacts can share that name and disagree: the engine is not deterministic, and a sync run publishes bases for the same commit | | `merge_base_resolved` | **boolean** | `false` means the merge base could not be resolved, so the comparison is against `base_sha` | | `base_sha` | string | the base branch tip when the event fired — *not* what was compared against | -| `pr_number`, `mode`, `seed_source`, `chain_depth` | string | provenance; nothing rendering a diagram needs them | +| `kind` | string | always `review`, so a reader can tell this artifact from a base or warm-start bundle | +| `analysed_files_changed` | string | analysed files whose content hash differs between base and head; `unknown` when the analyses cannot say | +| `pr_number`, `mode`, `seed_source`, `chain_depth` | string | provenance; nothing rendering a diagram needs them. `mode` is the head's engine mode | **A sync run** publishes the base graph under both the commit it analyzed and the baseline commit it writes on top, because a pull request opened either side of @@ -95,16 +98,45 @@ them: ## How a review resolves its two graphs -**Base**, first match wins: +**Base**, first match wins. This is `review-generate-baseline.sh`, in this order: | Source | Engine cost | |---|---| | the published `codeboarding-base--` artifact with a compatible depth cap | none | -| no usable artifact — check out the merge base, seed from a compatible baseline committed there, catch up | one incremental, full if Core requires it | -| no compatible committed baseline either | full analysis directly, at the configured `depth_cap` | +| the baseline branch: its entry for the merge base, else for the nearest of the merge base's last 100 first-parent ancestors, made under this configuration | none for the merge base itself, one incremental otherwise | +| the baseline committed on the branch at the merge base, with a compatible depth cap | one incremental | +| the nearest `codeboarding-base--` artifact among the merge base's last 100 first-parent ancestors | one incremental | +| nothing | full analysis, at the configured `depth_cap` | + +The baseline branch comes before the committed baseline because its entries are +pinned to this configuration and a committed baseline is not: one left behind +after a repository moved to the baseline branch must not win. A seed is only +checked before the run (depth cap, and configuration where the source records +it); if Core then asks for a full analysis, the base is analyzed in full rather +than retried from the next source. In practice only a committed baseline can be +refused that way, as it is the one source not pinned to the engine version. A trusted run that computed the base publishes it, so the next pull request -forking from that commit gets the first row. +forking from that commit gets the first row; that includes a base caught up from +an ancestor. Which source won is logged as a notice and not recorded anywhere +else. While a base is built from scratch, the progress comment says so in two +steps: building the base, then analysing the pull request. + +**Telemetry.** Every engine run carries `CODEBOARDING_RUN_ID=gh---`, +the role being `base`, `head` or `sync`, so the engine's own `analysis_started` / +`analysis_completed` events (mode, duration, tokens) can be read per run. A review +with no `base` events reused its base; `base` events say whether it was caught up +or built in full. + +The ancestor lookup walks the merge base's first-parent history, deepening the +shallow checkout to 101 commits, then pages through the repository's artifacts +newest first, keeping those named for this configuration and produced by a run on +the repository's own code. It stops at the first page holding one of the walked +commits (usually the first; at most 50 pages) and takes the nearest commit seen. +The merge base's own `.codeboardingignore` and health configuration replace the +seed's. Sync uses the same sources, in the same order, without the exact artifact, so the first sync after the setup pull request +merges catches up from the base that pull request's review saved, instead of +analyzing from scratch. The configuration hash includes `depth_cap`. The workflow input controls depth for both fresh and fallback analyses; stored legacy depth values never override it. @@ -121,6 +153,88 @@ diffs against, recorded as a digest in `origin.json`. Two runs of the engine ove one commit need not name components identically, so a head descended from one base and a diagram drawn against another would report changes nobody made. +## The baseline branch + +By default sync saves the analysis to a branch of its own in the same +repository, `codeboarding/baseline`, which the webview also reads. It holds the +default branch's analysis: a sync on any other branch fails before anything is +installed, so a manual run elsewhere cannot make another branch's analysis the +latest diagram. Entries are keyed by commit, so a renamed default branch just +continues. The synced branch is then only read. +`codeboarding_analysis_location: in_place` commits the analysis to the synced +branch instead, and skips everything below. + +**What lives where.** The branch is an orphan: it shares no history with the code. +Each sync adds one commit holding the same `.codeboarding/` files +sync would otherwise commit to the synced branch, plus +`.codeboarding/source.json`: + +```json +{"schema": 1, "synced_branch": "main", "source_sha": "", "generated_at": "", "engine_version": "", "config": ""} +``` + +The commit is `chore(codeboarding): diagram of main @` with three trailers: +`CodeBoarding-Source: `, `CodeBoarding-Branch: main` and +`CodeBoarding-Config: `, the same +configuration hash that names the base artifacts (engine version, provider, model, +depth cap). Engine output is never edited; which commit it describes and how it was +made live only in `source.json` and the trailers. The synced branch is never +written, not even `.gitattributes`. The base artifacts are still published, named +for the analysed commit. + +**How a sync writes it.** It seeds from the branch tip when the tip was made under +this configuration, and runs incrementally. The generated files are replaced +wholesale; only the checkout's own `.codeboardingignore` and health configuration +are kept. The push is a fast-forward onto the tip it fetched, never forced. Sync +refuses to write to an existing branch that is not a baseline branch (its tip has +no `CodeBoarding-Source` trailer, or holds anything besides `.codeboarding/`), so +a code branch that happens to be named `codeboarding/baseline` fails, before anything is installed, instead of being emptied, as does a branch named +`codeboarding`, next to which git cannot create it. If the synced branch moved +during the analysis, the result is still saved for the commit it analyzed: +entries are keyed by commit, and a branch that merges faster than sync runs would +otherwise never be saved. Only an analysis of an older commit than the tip's is +dropped, so the tip stays the latest diagram. If another sync moved the baseline branch, it builds on that tip once. A push the +remote refuses while the tip did not move is a branch rule, and the run fails +saying so. + +The tip is compared just before the push, not in the same transaction: if a sync +of a newer commit lands in that window, the push is no longer a fast-forward and +is retried on top of it once. + +**How a review reads it.** Right after an exact artifact, a review lists the newest 100 commits of the branch (fetched without +file contents, so the listing costs commit messages only) and matches their +trailers against the merge base's first-parent history, up to 100 commits deep. +Only entries made under this run's configuration count: an entry for the merge +base itself is reused as is, so nothing else would catch a different engine or +model. An entry for an ancestor is caught up incrementally. Only without a usable +entry does it fall back to a committed baseline, then to ancestor artifacts. + +**If the branch is deleted**, the next sync creates it again as a new orphan, +seeding from a saved ancestor artifact when there is one and analyzing in full +otherwise. The history is lost; the current diagram is not. + +**Protecting it.** Sync and review load `static_analysis.pkl` from this branch, and +a pickle runs code when loaded, so whoever can write the branch can run code in +the sync and review workflows. Import +[`baseline-branch-ruleset.json`](baseline-branch-ruleset.json) under Settings, +Rules, Rulesets, New ruleset, Import a ruleset. It blocks creating, updating, +deleting and force-pushing `codeboarding/baseline` for everyone except the +CodeBoarding Review app (`4021464`), so only the workflows you give the app's key +can write it; sync must then push with the app's token as `github_token`. That +token is only available to CodeBoarding's own workflows: the app's private key is +never shipped to other repositories. + +If sync uses the default `github.token`, as the README's examples do, import +[`baseline-branch-ruleset-actions.json`](baseline-branch-ruleset-actions.json) +instead, whose bypass actor is GitHub Actions (integration `15368`). **It only stops +people pushing by hand:** any workflow with `contents: write`, including one run +from a same-repository pull request branch, can still push to the branch and plant +the pickle sync loads. + +Creation is covered too, so the bypass actor must be the identity sync pushes as +before the first sync, or that sync fails on the rule. Rulesets on a private repository need a paid GitHub plan +(Pro, Team or Enterprise); on Free they apply to public repositories only. + ## Trust boundary `static_analysis.pkl` is a Python pickle, so state derived from code the diff --git a/docs/baseline-branch-ruleset-actions.json b/docs/baseline-branch-ruleset-actions.json new file mode 100644 index 0000000..19eab2d --- /dev/null +++ b/docs/baseline-branch-ruleset-actions.json @@ -0,0 +1,37 @@ +{ + "name": "CodeBoarding baseline branch (GitHub Actions: stops manual pushes only)", + "target": "branch", + "enforcement": "active", + "conditions": { + "ref_name": { + "include": [ + "refs/heads/codeboarding/baseline" + ], + "exclude": [] + } + }, + "rules": [ + { + "type": "creation" + }, + { + "type": "update", + "parameters": { + "update_allows_fetch_and_merge": false + } + }, + { + "type": "deletion" + }, + { + "type": "non_fast_forward" + } + ], + "bypass_actors": [ + { + "actor_id": 15368, + "actor_type": "Integration", + "bypass_mode": "always" + } + ] +} diff --git a/docs/baseline-branch-ruleset.json b/docs/baseline-branch-ruleset.json new file mode 100644 index 0000000..a3e46b2 --- /dev/null +++ b/docs/baseline-branch-ruleset.json @@ -0,0 +1,37 @@ +{ + "name": "CodeBoarding baseline branch", + "target": "branch", + "enforcement": "active", + "conditions": { + "ref_name": { + "include": [ + "refs/heads/codeboarding/baseline" + ], + "exclude": [] + } + }, + "rules": [ + { + "type": "creation" + }, + { + "type": "update", + "parameters": { + "update_allows_fetch_and_merge": false + } + }, + { + "type": "deletion" + }, + { + "type": "non_fast_forward" + } + ], + "bypass_actors": [ + { + "actor_id": 4021464, + "actor_type": "Integration", + "bypass_mode": "always" + } + ] +} diff --git a/scripts/action/analysis-state.sh b/scripts/action/analysis-state.sh new file mode 100644 index 0000000..0e10b05 --- /dev/null +++ b/scripts/action/analysis-state.sh @@ -0,0 +1,128 @@ +# shellcheck shell=bash +# Helpers for the directories that hold analysis state. Sourced, not run. + +DEPTH_CAP="${DEPTH_CAP:-2}" +if [[ ! "$DEPTH_CAP" =~ ^[1-9][0-9]*$ ]]; then + echo "::error::depth_cap must be a positive integer." + exit 1 +fi + +# Metadata is only a compatibility check, never the source of configuration. +# Legacy baselines without a configured cap are rebuilt at the requested depth. +depth_cap_from() { + local analysis="$1" + [ -f "$analysis" ] || return 0 + python3 -c 'import json,sys +metadata = json.load(open(sys.argv[1])).get("metadata", {}) +print(metadata.get("depth_cap", ""))' "$analysis" 2>/dev/null || true +} +# Whether $1 holds an analysis this run can continue from. +compatible_state() { + [ "$(depth_cap_from "$1/analysis.json")" = "$DEPTH_CAP" ] +} + +# What the user writes under .codeboarding/ belongs to the commit being analysed, +# not to whichever analysis seeded it. +USER_CONFIG=(.codeboardingignore health/health_config.json health/.healthignore) +keep_user_config() { + local checkout="$1" state="$2" file + mkdir -p "$state" + for file in "${USER_CONFIG[@]}"; do + rm -f "${state:?}/$file" + [ ! -f "$checkout/.codeboarding/$file" ] || { + mkdir -p "$(dirname "$state/$file")" + cp "$checkout/.codeboarding/$file" "$state/$file" + } + done +} +# Replaces $2 with $1, keeping checkout $3's user configuration. +replace_state() { + local from="$1" state="$2" config_from="$3" + rm -rf "$state" + mkdir -p "$(dirname "$state")" + cp -a "$from" "$state" + keep_user_config "$config_from" "$state" +} + +# A depth above 1 also deepens a commit the shallow checkout already holds. +fetch_commit() { + local repository="$1" sha="$2" depth="${3:-1}" + if git -C "$CHECKOUT_DIR" cat-file -e "$sha^{commit}" 2>/dev/null; then + [ "$depth" -gt 1 ] && [ "$(git -C "$CHECKOUT_DIR" rev-parse --is-shallow-repository)" = true ] || return 0 + fi + local auth + auth="$(printf 'x-access-token:%s' "${GIT_TOKEN:-}" | base64 -w0)" + git -C "$CHECKOUT_DIR" -c "http.extraheader=AUTHORIZATION: basic $auth" fetch \ + "${GITHUB_SERVER_URL%/}/${repository}.git" "$sha" --depth="$depth" +} + +# Two independently generated analyses of the same commit need not name the same +# components, so a head that grew from one base cannot be diffed against another. +analysis_digest() { + local analysis="$1" + [ -f "$analysis" ] || return 0 + if command -v sha256sum >/dev/null 2>&1; then + sha256sum "$analysis" | cut -c1-16 + else + shasum -a 256 "$analysis" | cut -c1-16 + fi +} + +# Records which state this analysis grew from, so a later run can tell whether +# it may continue from it. +origin_field() { + local state="$1" field="$2" + [ -f "$state/origin.json" ] || return 0 + python3 -c 'import json,sys; print(json.load(open(sys.argv[1])).get(sys.argv[2], ""))' \ + "$state/origin.json" "$field" 2>/dev/null || true +} +write_origin() { + local state="$1" + python3 -c 'import json,os,sys +json.dump({ + "schema": 1, + "pr_number": os.environ.get("PR_NUMBER", ""), + "merge_base_sha": os.environ.get("REVIEW_BASE_SHA", ""), + "head_sha": os.environ.get("REVIEW_HEAD_SHA", ""), + "engine_version": os.environ.get("ENGINE_VERSION", ""), + "cfg_hash": os.environ.get("CFG_HASH", ""), + "seed_source": sys.argv[2], + "chain_depth": int(sys.argv[3]), + "base_digest": sys.argv[4], +}, open(sys.argv[1], "w"), indent=2)' "$state/origin.json" "$2" "$3" "$4" +} + +# Lays out what the upload steps publish. The warm-start bundle is a complete +# working directory rather than the pickle alone, so restoring it needs one +# lookup and no correlating of two artifacts. +stage() { + local state="$1" kind="$2" + [ -n "${STAGE_DIR:-}" ] || return 0 + rm -rf "${STAGE_DIR:?}/$kind" + mkdir -p "$STAGE_DIR" + cp -a "$state" "$STAGE_DIR/$kind" + # Run logs and lock files are the engine's scratch, not analysis state. Nobody + # inflates them, and every fetch pays for them: they were a fifth of a bundle. + # Health config stays, because a run seeded from this bundle reads it. + rm -rf "$STAGE_DIR/$kind/logs" + find "$STAGE_DIR/$kind" -name '*.lock' -delete + # Say what this bundle is. Without it a base bundle is an analysis.json and + # nothing else, which unpacks exactly like a head artifact and would be + # rendered as one by a reader that fetched the wrong name. Written at staging + # time rather than carried in the state directory, so a bundle can never + # inherit the label of the one it was seeded from. + python3 -c 'import json,os,sys +kind = sys.argv[2] +marker = { + "kind": kind, + "engine_version": os.environ.get("ENGINE_VERSION", ""), + "cfg_hash": os.environ.get("CFG_HASH", ""), + "merge_base_sha": os.environ.get("REVIEW_BASE_SHA", ""), +} +# A base describes one commit and is shared by every pull request that forks +# there, so the run that happened to compute it is not part of its identity. +if kind == "warmstart": + marker["pr_number"] = os.environ.get("PR_NUMBER", "") + marker["head_sha"] = os.environ.get("REVIEW_HEAD_SHA", "") +json.dump(marker, open(sys.argv[1], "w"), indent=2)' "$STAGE_DIR/$kind/metadata.json" "$kind" +} diff --git a/scripts/action/analyze.sh b/scripts/action/analyze.sh index 5f246a0..bfff9fd 100755 --- a/scripts/action/analyze.sh +++ b/scripts/action/analyze.sh @@ -1,242 +1,45 @@ #!/usr/bin/env bash -# Runs incremental/full Core analysis and outputs the selected analysis paths and mode. +# Analyzes one checkout into a state directory: incrementally when the state holds +# a compatible analysis to continue from, in full otherwise or when Core asks for it. +# +# Usage: analyze.sh base|head|sync +# Prints analysis_mode= and analysis_path= on stdout; everything else goes to stderr. set -euo pipefail -DEPTH_CAP="${DEPTH_CAP:-2}" -if [[ ! "$DEPTH_CAP" =~ ^[1-9][0-9]*$ ]]; then - echo "::error::depth_cap must be a positive integer." - exit 1 -fi -parse_output() { - local output="$1" +# shellcheck source=scripts/action/analysis-state.sh +source "$(dirname "$0")/analysis-state.sh" +role="$1" checkout="$2" state="$3" +# Stdout is the result, so the error goes to stderr; the reason goes to the step's +# outputs for the review comment and the sync summary. +fail() { echo "::error::$1" >&2; echo "failure_reason=$1" >> "${GITHUB_OUTPUT:-/dev/null}"; exit 1; } + +# Tags Core's own telemetry with this run and the analysis it belongs to, so its +# events can be read per run: a review with no base events reused its base, one +# with base events caught it up or built it in full. +run_id="" +[ -z "${GITHUB_RUN_ID:-}" ] || run_id="gh-$GITHUB_RUN_ID-${GITHUB_RUN_ATTEMPT:-1}-$role" + +ANALYSIS_MODE="" REQUIRES_FULL="" ANALYSIS_PATH="" +run_core() { + local output + output="$(CODEBOARDING_RUN_ID="$run_id" python3 "$ACTION_PATH/scripts/analyze_repository.py" "$@" \ + --checkout "$checkout" --output-dir "$state")" ANALYSIS_MODE="$(awk -F= '$1 == "analysis_mode" {print $2; exit}' <<< "$output")" REQUIRES_FULL="$(awk -F= '$1 == "requires_full_analysis" {print $2; exit}' <<< "$output")" ANALYSIS_PATH="$(awk -F= '$1 == "analysis_path" {print substr($0, index($0, "=") + 1); exit}' <<< "$output")" } -incremental() { - local checkout="$1" output_dir="$2" output - output="$(python3 "$ACTION_PATH/scripts/analyze_repository.py" incremental \ - --checkout "$checkout" --output-dir "$output_dir")" - parse_output "$output" + +mkdir -p "$state" +if compatible_state "$state"; then + run_core incremental if [ "$ANALYSIS_MODE" != incremental ] || { [ "$REQUIRES_FULL" != true ] && [ ! -f "$ANALYSIS_PATH" ]; }; then - echo "::error::Invalid incremental-analysis result." - exit 1 + fail "Core returned an invalid incremental-analysis result." fi -} -full() { - local checkout="$1" output_dir="$2" depth="$3" output - output="$(python3 "$ACTION_PATH/scripts/analyze_repository.py" full \ - --checkout "$checkout" --output-dir "$output_dir" --depth-cap "$depth")" - parse_output "$output" + [ "$REQUIRES_FULL" != true ] || echo "::notice::Core cannot update the $role analysis incrementally; analyzing in full." >&2 +fi +if [ "$ANALYSIS_MODE" != incremental ] || [ "$REQUIRES_FULL" = true ]; then + run_core full --depth-cap "$DEPTH_CAP" if [ "$ANALYSIS_MODE" != full ] || [ ! -f "$ANALYSIS_PATH" ]; then - echo "::error::Invalid full-analysis result." - exit 1 + fail "Core returned an invalid full-analysis result." fi -} -# Metadata is only a compatibility check, never the source of configuration. -# Legacy baselines without a configured cap are rebuilt at the requested depth. -depth_cap_from() { - local analysis="$1" - [ -f "$analysis" ] || return 0 - python3 -c 'import json,sys -metadata = json.load(open(sys.argv[1])).get("metadata", {}) -print(metadata.get("depth_cap", ""))' "$analysis" 2>/dev/null || true -} - -seed_state() { - local checkout="$1" state="$2" - mkdir -p "$state" - [ ! -d "$checkout/.codeboarding" ] || cp -a "$checkout/.codeboarding/." "$state/" -} - -# Records which state this analysis grew from, so a later run can report its -# provenance and the review artifact can answer "which base did we use". -origin_field() { - local state="$1" field="$2" - [ -f "$state/origin.json" ] || return 0 - python3 -c 'import json,sys; print(json.load(open(sys.argv[1])).get(sys.argv[2], ""))' \ - "$state/origin.json" "$field" 2>/dev/null || true -} -write_origin() { - local state="$1" - python3 -c 'import json,os,sys -json.dump({ - "schema": 1, - "pr_number": os.environ.get("PR_NUMBER", ""), - "merge_base_sha": os.environ.get("REVIEW_BASE_SHA", ""), - "head_sha": os.environ.get("REVIEW_HEAD_SHA", ""), - "engine_version": os.environ.get("ENGINE_VERSION", ""), - "cfg_hash": os.environ.get("CFG_HASH", ""), - "seed_source": sys.argv[2], - "chain_depth": int(sys.argv[3]), - "base_digest": sys.argv[4], -}, open(sys.argv[1], "w"), indent=2)' "$state/origin.json" "$2" "$3" "$4" -} - -# Two independently generated analyses of the same commit need not name the same -# components, so a head that grew from one base cannot be diffed against another. -analysis_digest() { - local analysis="$1" - [ -f "$analysis" ] || return 0 - if command -v sha256sum >/dev/null 2>&1; then - sha256sum "$analysis" | cut -c1-16 - else - shasum -a 256 "$analysis" | cut -c1-16 - fi -} - -# Lays out what the upload steps publish. The warm-start bundle is a complete -# working directory rather than the pickle alone, so restoring it needs one -# lookup and no correlating of two artifacts. -stage() { - local state="$1" kind="$2" - [ -n "${STAGE_DIR:-}" ] || return 0 - rm -rf "${STAGE_DIR:?}/$kind" - mkdir -p "$STAGE_DIR" - cp -a "$state" "$STAGE_DIR/$kind" - # Run logs and lock files are the engine's scratch, not analysis state. Nobody - # inflates them, and every fetch pays for them: they were a fifth of a bundle. - # Health config stays, because a run seeded from this bundle reads it. - rm -rf "$STAGE_DIR/$kind/logs" - find "$STAGE_DIR/$kind" -name '*.lock' -delete - # Say what this bundle is. Without it a base bundle is an analysis.json and - # nothing else, which unpacks exactly like a head artifact and would be - # rendered as one by a reader that fetched the wrong name. Written at staging - # time rather than carried in the state directory, so a bundle can never - # inherit the label of the one it was seeded from. - python3 -c 'import json,os,sys -kind = sys.argv[2] -marker = { - "kind": kind, - "engine_version": os.environ.get("ENGINE_VERSION", ""), - "cfg_hash": os.environ.get("CFG_HASH", ""), - "merge_base_sha": os.environ.get("REVIEW_BASE_SHA", ""), -} -# A base describes one commit and is shared by every pull request that forks -# there, so the run that happened to compute it is not part of its identity. -if kind == "warmstart": - marker["pr_number"] = os.environ.get("PR_NUMBER", "") - marker["head_sha"] = os.environ.get("REVIEW_HEAD_SHA", "") -json.dump(marker, open(sys.argv[1], "w"), indent=2)' "$STAGE_DIR/$kind/metadata.json" "$kind" -} - -analyze_sync() { - local work="$RUNNER_TEMP/codeboarding-sync" state="$RUNNER_TEMP/codeboarding-sync/analysis" - rm -rf "$work" - seed_state "$CHECKOUT_DIR" "$state" - - if [ "${FORCE_FULL,,}" = true ] || [ "$(depth_cap_from "$state/analysis.json")" != "$DEPTH_CAP" ]; then - full "$CHECKOUT_DIR" "$state" "$DEPTH_CAP" - else - incremental "$CHECKOUT_DIR" "$state" - if [ "$REQUIRES_FULL" = true ]; then - full "$CHECKOUT_DIR" "$state" "$DEPTH_CAP" - fi - fi - # Sync already computes the graph every review of this branch compares against, - # so publish it instead of making the first pull request recompute it. - stage "$state" base - printf 'analysis_mode=%s\nanalysis_path=%s\nanalysis_dir=%s\n' \ - "$ANALYSIS_MODE" "$ANALYSIS_PATH" "$state" >> "$GITHUB_OUTPUT" -} - -fetch_commit() { - local repository="$1" sha="$2" - git -C "$CHECKOUT_DIR" cat-file -e "$sha^{commit}" 2>/dev/null && return 0 - local auth - auth="$(printf 'x-access-token:%s' "$GIT_TOKEN" | base64 -w0)" - git -C "$CHECKOUT_DIR" -c "http.extraheader=AUTHORIZATION: basic $auth" fetch \ - "${GITHUB_SERVER_URL%/}/${repository}.git" "$sha" --depth=1 -} - -# The artifact name pins configuration; verify the stored cap and lineage too. -warmstart_usable() { - local base_analysis="$1" bundle_cap - [ -f "${WARMSTART_DIR:-}/analysis.json" ] || return 1 - bundle_cap="$(depth_cap_from "$WARMSTART_DIR/analysis.json")" - if [ "$bundle_cap" != "$DEPTH_CAP" ]; then - echo "::notice::Analysis depth changed since the last run; re-seeding from the base analysis." - return 1 - fi - # A head that grew from one base graph cannot be diffed against another: two - # runs of the engine over one commit need not name components identically, so - # keeping it would report additions and removals for code nobody touched. - if [ "$(origin_field "$WARMSTART_DIR" base_digest)" != "$(analysis_digest "$base_analysis")" ]; then - echo "::notice::The base analysis is not the one this pull request's stored analysis grew from; re-seeding from it." - return 1 - fi -} - -analyze_review() { - local work="$RUNNER_TEMP/codeboarding-review" - local base_checkout="$work/base" base_state="$work/base-state" head_state="$work/head-state" - rm -rf "$work" - mkdir -p "$work" - - # A published base graph is this merge base's own analysis, named for it, so it - # needs no engine run at all. Without one, the merge base is checked out and - # analyzed from whatever baseline the repository committed there. - local base_source=published - if [ "$(depth_cap_from "${BASE_DIR:-}/analysis.json")" = "$DEPTH_CAP" ]; then - mkdir -p "$base_state" - cp -a "$BASE_DIR/." "$base_state/" - else - base_source=computed - fetch_commit "$REVIEW_BASE_REPO" "$REVIEW_BASE_SHA" - git -C "$CHECKOUT_DIR" worktree add --detach "$base_checkout" "$REVIEW_BASE_SHA" >/dev/null - seed_state "$base_checkout" "$base_state" - REQUIRES_FULL=true - if [ "$(depth_cap_from "$base_state/analysis.json")" = "$DEPTH_CAP" ]; then - incremental "$base_checkout" "$base_state" - fi - if [ "$REQUIRES_FULL" = true ]; then - full "$base_checkout" "$base_state" "$DEPTH_CAP" - fi - fi - unset GIT_TOKEN - - local base_analysis="$base_state/analysis.json" - [ -f "$base_analysis" ] || { echo "::error::Review baseline analysis is missing."; exit 1; } - - # Seed the head from this pull request's own last analysis when there is one, - # so the run only covers commits pushed since it. - local seed_source=base chain_depth=1 previous_depth - if warmstart_usable "$base_analysis"; then - cp -a "$WARMSTART_DIR" "$head_state" - seed_source=pr-chain - previous_depth="$(origin_field "$head_state" chain_depth)" - case "$previous_depth" in - ''|*[!0-9]*) previous_depth=0 ;; - esac - chain_depth=$(( previous_depth + 1 )) - else - cp -a "$base_state" "$head_state" - fi - rm -f "$head_state/origin.json" - - incremental "$CHECKOUT_DIR" "$head_state" - if [ "$REQUIRES_FULL" = true ]; then - full "$CHECKOUT_DIR" "$head_state" "$DEPTH_CAP" - fi - - write_origin "$head_state" "$seed_source" "$chain_depth" "$(analysis_digest "$base_analysis")" - stage "$head_state" warmstart - # Republishing a base graph that was only read back would store the same bytes - # under the same name every run, so normally only a run that produced one - # publishes it. The exception is lifetime: a review artifact references a base - # by id for its whole retention, so one about to expire is renewed rather than - # left dangling under a review that outlives it. - local publish_base=false - if [ "$base_source" = computed ] || [ "${RENEW_BASE:-false}" = true ]; then - stage "$base_state" base - publish_base=true - fi - - printf 'analysis_mode=%s\nanalysis_path=%s\nbase_analysis_path=%s\nseed_source=%s\nchain_depth=%s\npublish_base=%s\n' \ - "$ANALYSIS_MODE" "$ANALYSIS_PATH" "$base_analysis" "$seed_source" "$chain_depth" "$publish_base" >> "$GITHUB_OUTPUT" -} - -case "$ANALYSIS_KIND" in - sync) analyze_sync ;; - review) analyze_review ;; - *) echo "::error::Unknown analysis kind: $ANALYSIS_KIND"; exit 1 ;; -esac +fi +printf 'analysis_mode=%s\nanalysis_path=%s\n' "$ANALYSIS_MODE" "$ANALYSIS_PATH" diff --git a/scripts/action/check-analysis-branch.sh b/scripts/action/check-analysis-branch.sh new file mode 100755 index 0000000..2adf053 --- /dev/null +++ b/scripts/action/check-analysis-branch.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash +# An analysis branch name git cannot create, or one already used by code, fails +# here, before the engine is installed, instead of after a full analysis. +set -euo pipefail +# shellcheck source=scripts/action/codeboarding-baseline.sh +source "$(dirname "$0")/codeboarding-baseline.sh" +[ -n "${BASELINE_BRANCH:-}" ] || exit 0 + +status=0 +# shellcheck disable=SC2153 # REPOSITORY comes from the step's env. +baseline_branch_exists "$REPOSITORY" || status=$? +if [ "$status" = 2 ]; then + # The first sync creates it, which git refuses next to a branch named like its parent directory. + blocking="${BASELINE_BRANCH%/*}" + [ "$blocking" = "$BASELINE_BRANCH" ] || { + status=0 + baseline_branch_exists "$REPOSITORY" "$blocking" || status=$? + [ "$status" = 2 ] || + baseline_fail "$REPOSITORY has a branch named $blocking, so git cannot create $BASELINE_BRANCH next to it. Rename $blocking, or set codeboarding_analysis_location: in_place." + } + exit 0 +fi + +# Commit messages and the tree listing only: no analysis files are downloaded. +scratch="$RUNNER_TEMP/codeboarding-analysis-branch-check.git" +rm -rf "$scratch" +git init -q --bare "$scratch" +auth="$(printf 'x-access-token:%s' "${GIT_TOKEN:-}" | base64 -w0)" +git -C "$scratch" -c "http.extraheader=AUTHORIZATION: basic $auth" fetch -q --filter=blob:none --depth=1 \ + "${GITHUB_SERVER_URL%/}/${REPOSITORY}.git" "refs/heads/$BASELINE_BRANCH" || + baseline_fail "Could not fetch $BASELINE_BRANCH from $REPOSITORY." +source_trailer="$(git -C "$scratch" log -1 --format='%(trailers:key=CodeBoarding-Source,valueonly)' FETCH_HEAD | tr -d '[:space:]')" +if [ -z "$source_trailer" ] || [ "$(git -C "$scratch" ls-tree --name-only FETCH_HEAD)" != .codeboarding ]; then + baseline_fail "$BASELINE_BRANCH already exists and is not a CodeBoarding analysis branch, so sync will not write to it. Rename that branch, or set codeboarding_analysis_location: in_place." +fi diff --git a/scripts/action/clear-auth.sh b/scripts/action/clear-auth.sh new file mode 100755 index 0000000..b14eabd --- /dev/null +++ b/scripts/action/clear-auth.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash +# Stops the credential relay and deletes the resolved credentials. with-auth.sh +# calls it after its command; the action also runs it once at the end, so a step +# that kept credentials for the next one never leaves them behind. +AUTH_DIR="${RUNNER_TEMP}/codeboarding-auth" +if [ -s "$AUTH_DIR/relay.pid" ]; then + kill "$(cat "$AUTH_DIR/relay.pid")" 2>/dev/null || true +fi +rm -rf "$AUTH_DIR" diff --git a/scripts/action/codeboarding-baseline.sh b/scripts/action/codeboarding-baseline.sh new file mode 100644 index 0000000..be19508 --- /dev/null +++ b/scripts/action/codeboarding-baseline.sh @@ -0,0 +1,186 @@ +# shellcheck shell=bash +# The analysis branch (codeboarding_analysis_location: codeboarding_branch, BASELINE_BRANCH +# here, codeboarding/baseline): an orphan branch holding one commit per sync, each with trailers naming the +# commit it analysed (CodeBoarding-Source), the branch that commit is on +# (CodeBoarding-Branch) and the configuration it ran under (CodeBoarding-Config). +# Sync writes it; sync and review read it. Sourced, not run; needs analysis-state.sh. + +# How far below a commit a reader looks for an entry describing it or an ancestor. +CATCHUP_BOUND="${CATCHUP_BOUND:-100}" +BASELINE_BRANCH_DEPTH="${BASELINE_BRANCH_DEPTH:-100}" + +# Fails the run with a reason the review comment and the sync summary repeat. +baseline_fail() { + echo "::error::$1" + echo "failure_reason=$1" >> "$GITHUB_OUTPUT" + exit 1 +} + +# Returns 0 when branch $2 (BASELINE_BRANCH by default) exists in repository $1 and +# 2 when it does not. Any other failure is retried once, so one network blip does +# not fail the run. +baseline_branch_exists() { + local repository="$1" branch="${2:-$BASELINE_BRANCH}" auth status + auth="$(printf 'x-access-token:%s' "${GIT_TOKEN:-}" | base64 -w0)" + for attempt in 1 2; do + status=0 + git -c "http.extraheader=AUTHORIZATION: basic $auth" ls-remote --exit-code \ + "${GITHUB_SERVER_URL%/}/${repository}.git" "refs/heads/$branch" >/dev/null || status=$? + case "$status" in + 0 | 2) return "$status" ;; + esac + [ "$attempt" = 2 ] || sleep "${BASELINE_RETRY_DELAY:-2}" + done + baseline_fail "Could not read $branch from $repository. Check that github_token can read the repository." +} + +# ---- Reading ---- + +# Lists the branch's entries as " ", newest +# first; an entry without a config has none to compare. Fetched without blobs +# into a scratch repository: the lookup needs messages, and a hundred pickles +# would cost more than it saves. +codeboarding_baseline_index() { + local repository="$1" scratch="$RUNNER_TEMP/codeboarding-baseline-index.git" auth + rm -rf "$scratch" + git init -q --bare "$scratch" + auth="$(printf 'x-access-token:%s' "${GIT_TOKEN:-}" | base64 -w0)" + git -C "$scratch" -c "http.extraheader=AUTHORIZATION: basic $auth" fetch -q --filter=blob:none \ + --depth="$BASELINE_BRANCH_DEPTH" "${GITHUB_SERVER_URL%/}/${repository}.git" "refs/heads/$BASELINE_BRANCH" || return 1 + git -C "$scratch" log --format='%H %(trailers:key=CodeBoarding-Source,valueonly,separator=%x20) %(trailers:key=CodeBoarding-Config,valueonly,separator=%x20)' FETCH_HEAD | + awk 'NF >= 2 {print $1, $2, (NF >= 3 ? $3 : "-")}' +} + +# Seeds $3 from the branch's entry for $2, or for its nearest first-parent +# ancestor, made under this run's configuration: the trailer pins it the way the +# name pins a saved artifact, and without a configuration hash no entry is +# trusted. Keeps checkout $4's user configuration. Sets SEED_SHA to the commit +# the seed describes. +from_codeboarding_baseline() { + local repository="$1" tip="$2" state="$3" config_from="$4" index commit entry="" scratch="$RUNNER_TEMP/codeboarding-baseline-restore" + local status=0 + [ -n "${BASELINE_BRANCH:-}" ] || return 1 + [ -n "${CFG_HASH:-}" ] || return 1 + # A branch that does not exist yet is a miss: the first sync creates it. Any other + # failure to read it is an error, not a reason to quietly analyze from scratch. + baseline_branch_exists "$repository" || status=$? + if [ "$status" = 2 ]; then + echo "::notice::$BASELINE_BRANCH does not exist yet; the first sync creates it." + return 1 + fi + index="$(codeboarding_baseline_index "$repository")" || + baseline_fail "Could not fetch $BASELINE_BRANCH from $repository." + if [ -z "$index" ]; then + # A code branch, as when a review into another branch names the one that holds the analysis. + echo "::warning::$BASELINE_BRANCH is not a CodeBoarding analysis branch, so its analysis is not read here." + return 1 + fi + fetch_commit "$repository" "$tip" "$(( CATCHUP_BOUND + 1 ))" || + baseline_fail "Could not fetch the history of ${tip:0:7} to look for its analysis." + for commit in $(git -C "$CHECKOUT_DIR" rev-list --first-parent --max-count=$(( CATCHUP_BOUND + 1 )) "$tip" 2>/dev/null); do + entry="$(awk -v source="$commit" -v cfg="$CFG_HASH" '$2 == source && $3 == cfg {print $1; exit}' <<< "$index")" + [ -z "$entry" ] || break + done + if [ -z "$entry" ]; then + echo "::notice::$BASELINE_BRANCH has no analysis of ${tip:0:7} or its last $CATCHUP_BOUND ancestors made with this configuration." + return 1 + fi + # The entry is known to exist from here on, so failing to read it is an error. + fetch_commit "$repository" "$entry" || + baseline_fail "Could not fetch the analysis ${entry:0:7} from $BASELINE_BRANCH." + rm -rf "$scratch" + mkdir -p "$scratch" + git -C "$CHECKOUT_DIR" archive "$entry" .codeboarding | tar -x -C "$scratch" || + baseline_fail "Could not extract the analysis ${entry:0:7} from $BASELINE_BRANCH." + # source.json is provenance, not engine state. + rm -f "$scratch/.codeboarding/source.json" + compatible_state "$scratch/.codeboarding" || return 1 + replace_state "$scratch/.codeboarding" "$state" "$config_from" + # shellcheck disable=SC2034 # read by the scripts that source this file + SEED_SHA="$commit" + echo "::notice::Starting from the analysis of ${commit:0:7} saved on $BASELINE_BRANCH." +} + +# ---- Writing ---- + +# Saves the analysis installed by install-sync.sh as one fast-forward commit on +# BASELINE_BRANCH, never writing to the synced branch. Called by deliver-sync.sh, +# whose REMOTE, BASE_SHA and emit_result it uses; always exits. +save_to_codeboarding_baseline() { + local branch="$BASELINE_BRANCH" tree="$RUNNER_TEMP/codeboarding-baseline-tree" + local index="$RUNNER_TEMP/codeboarding-baseline-index" git_dir files new_tree tip tip_source parent commit now + git_dir="$(git rev-parse --absolute-git-dir)" + rm -rf "$tree" "$index" + mkdir -p "$tree" + CHECKOUT_DIR="$tree" "$ACTION_PATH/scripts/action/install-sync.sh" > /dev/null + files="$(find "$tree/.codeboarding" -maxdepth 1 -type f | wc -l | tr -d ' ')" + # Engine output is never edited; which commit it describes, and under which + # configuration, lives here and in the commit's trailers only. + python3 -c 'import datetime,json,os,sys +json.dump({ + "schema": 1, + "synced_branch": os.environ["SYNCED_BRANCH"], + "source_sha": sys.argv[2], + "generated_at": datetime.datetime.now(datetime.timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ"), + "engine_version": os.environ.get("ENGINE_VERSION", ""), + "config": os.environ.get("CFG_HASH", ""), +}, open(sys.argv[1], "w"), indent=2)' "$tree/.codeboarding/source.json" "$BASE_SHA" + GIT_INDEX_FILE="$index" git --git-dir="$git_dir" --work-tree="$tree" -C "$tree" add -A -f .codeboarding + new_tree="$(GIT_INDEX_FILE="$index" git --git-dir="$git_dir" write-tree)" + git config user.name 'codeboarding-review[bot]' + git config user.email 'codeboarding-review[bot]@users.noreply.github.com' + # Readers trust only entries made under their own configuration. + [ -n "${CFG_HASH:-}" ] || baseline_fail "No configuration hash to record with the analysis." + local trailers=(-m "CodeBoarding-Source: $BASE_SHA +CodeBoarding-Branch: $SYNCED_BRANCH +CodeBoarding-Config: $CFG_HASH") + + # Two tries: a concurrent sync that moved the branch for an older commit is + # built on top of once. A second move means a newer run is handling it. + # The synced branch moving on during the analysis is no reason to drop it: + # entries are keyed by the commit they describe, and a branch that merges + # faster than sync runs would otherwise never be saved. + for _ in 1 2; do + tip="" parent=() + if [ -n "$(git ls-remote "$REMOTE" "refs/heads/$branch")" ]; then + git fetch -q --depth=1 "$REMOTE" "refs/heads/$branch" + # The parent is what was fetched, not what ls-remote saw: the branch may move in between. + tip="$(git rev-parse FETCH_HEAD)" + tip_source="$(git log -1 --format='%(trailers:key=CodeBoarding-Source,valueonly)' "$tip" | tr -d '[:space:]')" + # Building on any other branch would leave it holding nothing but .codeboarding/. + if [ -z "$tip_source" ] || [ "$(git ls-tree --name-only "$tip")" != .codeboarding ]; then + baseline_fail "$branch already exists and is not a CodeBoarding baseline branch, so sync will not write to it. Rename that branch, or set codeboarding_analysis_location: in_place." + fi + parent=(-p "$tip") + if [ "$tip_source" = "$BASE_SHA" ] && + git diff --quiet -I '"generated_at"' -I '"timestamp"' "$tip" "$new_tree"; then + emit_result "$files" false "$BASE_SHA" + echo "::notice::$branch already holds this analysis of $SYNCED_BRANCH @${BASE_SHA:0:7}." + exit 0 + fi + # The tip is the latest diagram, so an older analysis never replaces a newer one. + if [ "$tip_source" != "$BASE_SHA" ]; then + git fetch -q --depth="$(( CATCHUP_BOUND + 1 ))" "$REMOTE" "$SYNCED_BRANCH" + if git merge-base --is-ancestor "$BASE_SHA" "$tip_source" 2>/dev/null; then + emit_result "$files" false "$BASE_SHA" + echo "::notice::$branch already holds the analysis of a newer commit, ${tip_source:0:7}; leaving it." + exit 0 + fi + fi + fi + commit="$(git commit-tree "$new_tree" ${parent[@]+"${parent[@]}"} \ + -m "chore(codeboarding): diagram of $SYNCED_BRANCH @${BASE_SHA:0:7}" "${trailers[@]}")" + # Never forced: the parent is the tip just read, so this only ever fast-forwards. + if git push -q "$REMOTE" "$commit:refs/heads/$branch"; then + emit_result "$files" true "$BASE_SHA" + exit 0 + fi + now="$(git ls-remote "$REMOTE" "refs/heads/$branch" | awk '{print $1; exit}')" + if [ "$now" = "$tip" ]; then + baseline_fail "GitHub refused the push to $branch, most likely because a branch rule protects it. Add the identity sync pushes with (the CodeBoarding app, or GitHub Actions for the default token) as a bypass actor for $branch in the repository's rulesets, and check that github_token may push to it." + fi + done + emit_result "$files" false "$BASE_SHA" + echo "::notice::Another sync keeps updating $branch; leaving it to that run." + exit 0 +} diff --git a/scripts/action/deliver-sync.sh b/scripts/action/deliver-sync.sh index bbc454c..98100a3 100755 --- a/scripts/action/deliver-sync.sh +++ b/scripts/action/deliver-sync.sh @@ -1,8 +1,9 @@ #!/usr/bin/env bash -# Installs generated state and delivers it by direct push or a rolling sync PR. +# Installs generated state and delivers it to the analysis branch, or, when there +# is none, commits it to the synced branch itself. set -euo pipefail cd "$CHECKOUT_DIR" -SYNC_BRANCH=codeboarding/sync +# shellcheck disable=SC2153 # REPOSITORY comes from the step's env. REMOTE="${GITHUB_SERVER_URL%/}/${REPOSITORY}.git" ASKPASS="$RUNNER_TEMP/codeboarding-git-askpass.sh" GENERATED_PATHS="$RUNNER_TEMP/codeboarding-sync-paths" @@ -16,7 +17,6 @@ esac SH chmod 700 "$ASKPASS" export GIT_ASKPASS="$ASKPASS" GIT_TERMINAL_PROMPT=0 -export GH_HOST="${GH_HOST#*://}" trap 'rm -f "$ASKPASS"' EXIT # A pull request's merge base is whichever commit its author branched from, and # this run's analysis is valid for two of them: the commit it analyzed, and the @@ -27,29 +27,6 @@ emit_result() { printf 'files_written=%s\ncommitted=%s\nbaseline_sha=%s\nanalyzed_sha=%s\n' \ "$1" "$2" "$3" "$BASE_SHA" >> "$GITHUB_OUTPUT" } -close_stale_pr() { - [ "$SYNC_STRATEGY" = pull_request ] || return 0 - git fetch "$REMOTE" "$TARGET_BRANCH" - [ "$(git rev-parse FETCH_HEAD)" = "$BASE_SHA" ] || return 0 - local number sync_sha - number="$(gh api --method GET "repos/$REPOSITORY/pulls" \ - -f state=open -f base="$TARGET_BRANCH" \ - -f head="${REPOSITORY%%/*}:$SYNC_BRANCH" --jq '.[0].number // empty')" - if [ -n "$number" ]; then - sync_sha="$(git ls-remote "$REMOTE" "refs/heads/$SYNC_BRANCH" | awk '{print $1; exit}')" - if [ "$sync_sha" != "${SYNC_BRANCH_START_SHA:-}" ]; then - echo "::notice::A newer run updated $SYNC_BRANCH; leaving its PR open." - return 0 - fi - if [ -n "$sync_sha" ] && ! git push \ - "--force-with-lease=refs/heads/$SYNC_BRANCH:$sync_sha" "$REMOTE" ":refs/heads/$SYNC_BRANCH"; then - echo "::notice::A newer run updated $SYNC_BRANCH; leaving its PR open." - return 0 - fi - gh pr close "$number" --repo "$REPOSITORY" - echo "::notice::Closed obsolete CodeBoarding sync PR #$number." - fi -} classify_push_failure() { local expected="$1" branch="$2" current current="$(git ls-remote "$REMOTE" "refs/heads/$branch" | awk '{print $1; exit}')" @@ -59,10 +36,19 @@ classify_push_failure() { echo "::notice::A newer run updated $branch; leaving it untouched." exit 0 fi - echo "::error::Could not push the CodeBoarding baseline to $branch." - exit 1 + baseline_fail "Could not push the CodeBoarding baseline to $branch." } +# shellcheck source=scripts/action/codeboarding-baseline.sh +source "$ACTION_PATH/scripts/action/codeboarding-baseline.sh" +if [ -n "${BASELINE_BRANCH:-}" ]; then + # The synced branch is never written from here, so its old copy can only be pointed out. + if git ls-files --error-unmatch .codeboarding/analysis.json >/dev/null 2>&1; then + echo "::warning::$SYNCED_BRANCH still has an old analysis in .codeboarding/ that sync no longer updates; it is saved to $BASELINE_BRANCH now. You can delete it from $SYNCED_BRANCH, keeping .codeboarding/.codeboardingignore and the health configuration." + fi + save_to_codeboarding_baseline +fi + "$ACTION_PATH/scripts/action/install-sync.sh" > "$GENERATED_PATHS" stage_paths=() while IFS= read -r path; do @@ -78,11 +64,11 @@ files_written="$(find "$CHECKOUT_DIR/.codeboarding" -maxdepth 1 -type f \ \( -name analysis.json -o -name fingerprint.json -o -name static_analysis.pkl -o -name static_analysis.sha \ -o -name codeboarding_version.json \) | wc -l)" -git fetch "$REMOTE" "$TARGET_BRANCH" +git fetch "$REMOTE" "$SYNCED_BRANCH" remote_sha="$(git rev-parse FETCH_HEAD)" if [ "$remote_sha" != "$BASE_SHA" ]; then emit_result "$files_written" false "$BASE_SHA" - echo "::notice::$TARGET_BRANCH advanced during analysis; a newer run should update its baseline." + echo "::notice::$SYNCED_BRANCH advanced during analysis; a newer run should update its baseline." exit 0 fi @@ -91,7 +77,6 @@ fi # every run. Dropping either filter turns every sync into a commit. if git diff --cached --quiet || git diff --cached --quiet -I '"generated_at"' -I '"timestamp"'; then git reset -q - close_stale_pr emit_result "$files_written" false "$BASE_SHA" echo "::notice::The CodeBoarding baseline is unchanged." exit 0 @@ -99,40 +84,7 @@ fi git commit -m 'chore(codeboarding): sync analysis baseline' >/dev/null -if [ "$SYNC_STRATEGY" = push ]; then - if ! git push "$REMOTE" "HEAD:refs/heads/$TARGET_BRANCH"; then - classify_push_failure "$BASE_SHA" "$TARGET_BRANCH" - fi - emit_result "$files_written" true "$(git rev-parse HEAD)" - exit 0 -fi - -pr_json="$(gh api --method GET "repos/$REPOSITORY/pulls" \ - -f state=open -f head="${REPOSITORY%%/*}:$SYNC_BRANCH" --jq '.[0] // empty')" -if [ -n "$pr_json" ] && [ "$(jq -r .base.ref <<< "$pr_json")" != "$TARGET_BRANCH" ]; then - echo "::error::$SYNC_BRANCH already has an open PR into $(jq -r .base.ref <<< "$pr_json"); close it before changing target_branch." - exit 1 -fi - -old_sync_sha="$(git ls-remote "$REMOTE" "refs/heads/$SYNC_BRANCH" | awk '{print $1; exit}')" -if ! git push "--force-with-lease=refs/heads/$SYNC_BRANCH:$old_sync_sha" \ - "$REMOTE" "HEAD:refs/heads/$SYNC_BRANCH"; then - classify_push_failure "$old_sync_sha" "$SYNC_BRANCH" +if ! git push "$REMOTE" "HEAD:refs/heads/$SYNCED_BRANCH"; then + classify_push_failure "$BASE_SHA" "$SYNCED_BRANCH" fi - -if [ -z "$pr_json" ]; then - gh pr create --repo "$REPOSITORY" --base "$TARGET_BRANCH" --head "$SYNC_BRANCH" \ - --title 'chore(codeboarding): sync analysis baseline' \ - --body "Updates the versioned CodeBoarding analysis for \`$TARGET_BRANCH\`." - pr_json="$(gh api --method GET "repos/$REPOSITORY/pulls" \ - -f state=open -f base="$TARGET_BRANCH" \ - -f head="${REPOSITORY%%/*}:$SYNC_BRANCH" --jq '.[0]')" -fi - -pr_url="$(jq -r .html_url <<< "$pr_json")" -pr_number="$(jq -r .number <<< "$pr_json")" -emit_result "$files_written" true "$BASE_SHA" -{ - echo "sync_pr_url=$pr_url" - echo "sync_pr_number=$pr_number" -} >> "$GITHUB_OUTPUT" +emit_result "$files_written" true "$(git rev-parse HEAD)" diff --git a/scripts/action/find-ancestor-base.sh b/scripts/action/find-ancestor-base.sh new file mode 100755 index 0000000..1038796 --- /dev/null +++ b/scripts/action/find-ancestor-base.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# Finds the nearest first-parent ancestor of TIP_SHA with a saved base analysis +# under this configuration and downloads it into DEST, so a run without an exact +# base can catch up from it instead of analyzing from scratch. Best effort. +# +# Prints ancestor_sha= on +# stdout and everything else on stderr. +# +# Needs the tip's first-parent history in CHECKOUT_DIR, at least CATCHUP_BOUND deep. +set -euo pipefail +: "${REPOSITORY:?}" "${CFG_HASH:?}" "${TIP_SHA:?}" "${CHECKOUT_DIR:?}" "${DEST:?}" +BOUND="${CATCHUP_BOUND:-100}" +rm -rf "$DEST" + +api() { gh api -H 'Accept: application/vnd.github+json' "$@"; } +export GH_HOST="${GH_HOST:-github.com}" +GH_HOST="${GH_HOST#*://}" + +ancestor="" +report() { echo "ancestor_sha=$ancestor"; } +trap report EXIT + +# The merge base's first-parent history, nearest first. Distance 0 is the tip +# itself, which a review already looked up by its exact name; sync has no such +# lookup and passes INCLUDE_TIP=true. +walk="$(git -C "$CHECKOUT_DIR" rev-list --first-parent --max-count=$(( BOUND + 1 )) "$TIP_SHA" 2>/dev/null || true)" +[ "${INCLUDE_TIP:-false}" = true ] || walk="$(tail -n +2 <<< "$walk")" +nearest() { + local commit + for commit in $walk; do + if grep -qx "$commit" <<< "$saved"; then + echo "$commit" + return 0 + fi + done +} + +# The artifact listing, newest first. Its name filter is exact, so a prefix needs +# the pages themselves; paging stops at the first page holding a saved ancestor. +# Bases are published as their commits are synced or reviewed, so the nearest one +# is nearly always the newest: one or two calls in practice, MAX_PAGES (50, 5,000 +# artifacts) at worst, against GITHUB_TOKEN's 1,000 requests an hour per +# repository, and only on a run that would otherwise analyze from scratch. The +# provenance rule is fetch-state.sh's: only artifacts from a run on this +# repository's own code, since a fork's workflow can upload under any name and +# its bytes would reach a pickle loader. +trusted='.artifacts[]? + | select(.expired == false) + | select(.workflow_run != null) + | select(.workflow_run.head_repository_id == .workflow_run.repository_id) + | select(.name | startswith("codeboarding-base-")) + | .name' +prefix="codeboarding-base-$CFG_HASH-" +saved="" page=1 +while [ "$page" -le "${MAX_PAGES:-50}" ]; do + if ! listing="$(api "repos/$REPOSITORY/actions/artifacts?per_page=100&page=$page" 2>/dev/null)"; then + echo "::warning::Could not list artifacts in $REPOSITORY; not looking for an older saved analysis." >&2 + exit 0 + fi + names="$(jq -r "$trusted" <<< "$listing" 2>/dev/null || true)" + saved="$saved$(grep "^$prefix" <<< "$names" | cut -c$(( ${#prefix} + 1 ))- || true)"$'\n' + ancestor="$(nearest)" + [ -z "$ancestor" ] || break + returned="$(jq -r '.artifacts | length' <<< "$listing" 2>/dev/null || echo 0)" + [ "${returned:-0}" -eq 100 ] || break + page=$(( page + 1 )) +done + +[ -n "$ancestor" ] || exit 0 + +# Downloaded by its exact name, through the same checks as every other lookup. +if ! GITHUB_OUTPUT="" ARTIFACT_NAME="$prefix$ancestor" DEST="$DEST" \ + "$(dirname "$0")/fetch-state.sh" >&2 || [ ! -f "$DEST/analysis.json" ]; then + ancestor="" +fi diff --git a/scripts/action/install-sync.sh b/scripts/action/install-sync.sh index eb0f748..90cca50 100755 --- a/scripts/action/install-sync.sh +++ b/scripts/action/install-sync.sh @@ -1,22 +1,20 @@ #!/usr/bin/env bash -# Copies Core's persisted artifacts into .codeboarding and removes generated v1 files. +# Copies Core's persisted artifacts into .codeboarding. set -euo pipefail +# Stdout is captured by the caller, so the error goes to stderr; the reason goes +# to the step's outputs for the job summary. +fail() { echo "::error::$1" >&2; echo "failure_reason=$1" >> "${GITHUB_OUTPUT:-/dev/null}"; exit 1; } output="$CHECKOUT_DIR/.codeboarding" mkdir -p "$output" -# Core exposes one canonical manifest. The compatibility imports keep the local -# harness usable with older Core checkouts without duplicating filenames. +# Core exposes one canonical manifest. Read from the action's directory, never the +# analyzed repository's, which may have a constants.py of its own. manifest="$(cd "$ACTION_PATH" && python3 - <<'PY' -try: - from constants import ANALYSIS_FILENAME, PERSISTED_ANALYSIS_ARTIFACT_FILENAMES as artifacts -except ImportError: - from static_analyzer.analysis_cache import STATIC_ANALYSIS_PKL, STATIC_ANALYSIS_SHA - from utils import ANALYSIS_FILENAME, FINGERPRINT_FILENAME - artifacts = (ANALYSIS_FILENAME, FINGERPRINT_FILENAME, STATIC_ANALYSIS_PKL, STATIC_ANALYSIS_SHA, "codeboarding_version.json") +from constants import ANALYSIS_FILENAME, PERSISTED_ANALYSIS_ARTIFACT_FILENAMES as artifacts print(ANALYSIS_FILENAME) print(*artifacts, sep="\n") PY -)" || { echo "::error::Could not read Core's persisted artifact manifest."; exit 1; } -[ -n "$manifest" ] || { echo "::error::Core's persisted artifact manifest is empty."; exit 1; } +)" || fail "Could not read Core's persisted artifact manifest." +[ -n "$manifest" ] || fail "Core's persisted artifact manifest is empty." # Read into an array without mapfile, which needs bash 4: macOS ships bash 3.2, # so mapfile made this script, and its tests, unrunnable for local development. manifest_entries=() @@ -25,7 +23,7 @@ while IFS= read -r entry; do done <<< "$manifest" required="${manifest_entries[0]}" artifacts=("${manifest_entries[@]:1}") -[ -f "$ANALYSIS_DIR/$required" ] || { echo "::error::Core did not produce $required."; exit 1; } +[ -f "$ANALYSIS_DIR/$required" ] || fail "Core did not produce $required." installed=0 for name in "${artifacts[@]}"; do @@ -52,13 +50,4 @@ elif [ -e "$health_target" ]; then rm -f "$health_target" fi printf '%s\n' "$health_target" - -# Remove identifiable v1 Markdown. -marker='https://img.shields.io/badge/Generated%20by-CodeBoarding' -for legacy in "$output"/*.md "$CHECKOUT_DIR/docs/development/architecture.md"; do - [ -f "$legacy" ] || continue - grep -Fq "$marker" "$legacy" || continue - rm -f "$legacy" - printf '%s\n' "$legacy" -done echo "installed=$installed" >> "$GITHUB_OUTPUT" diff --git a/scripts/action/old_api_migrator.sh b/scripts/action/old_api_migrator.sh new file mode 100755 index 0000000..b5be9e1 --- /dev/null +++ b/scripts/action/old_api_migrator.sh @@ -0,0 +1,53 @@ +#!/usr/bin/env bash +# Rewrites deprecated inputs as codeboarding_analysis_location, the way a person +# editing the workflow would, and fails on any it cannot express. It is the only +# place that knows they exist: every later step reads its output, never the inputs. +# +# sync_strategy: push -> codeboarding_analysis_location: in_place +# sync_strategy: pull_request -> codeboarding_analysis_location: codeboarding_branch +# target_branch: -> nothing +# target_branch: -> fails: the push trigger picks the branch +# force_full: -> nothing, with a warning +# +# Delete a mapping, and its input in action.yml, once no supported workflow sets it. +set -euo pipefail +MIGRATE_URL=https://github.com/CodeBoarding/CodeBoarding-action#moving-an-existing-setup +fail() { echo "::error title=CodeBoarding inputs::$1 See $MIGRATE_URL for the change, or a prompt that makes it for you."; exit 1; } +warn() { echo "::warning title=CodeBoarding inputs::$1 See $MIGRATE_URL."; } +DEFAULT_LOCATION=codeboarding_branch + +location="${ANALYSIS_LOCATION:-}" +case "$location" in + codeboarding_branch | in_place) ;; + *) fail "codeboarding_analysis_location must be codeboarding_branch or in_place, not '$location'." ;; +esac + +if [ -n "${OLD_SYNC_STRATEGY:-}" ]; then + case "$OLD_SYNC_STRATEGY" in + push) mapped=in_place ;; + pull_request) mapped=codeboarding_branch ;; + *) fail "sync_strategy is no longer supported; use codeboarding_analysis_location." ;; + esac + # codeboarding_analysis_location has a default, so only a value other than it can + # conflict; one that agrees is a migration half done. + [ "$location" = "$DEFAULT_LOCATION" ] || [ "$location" = "$mapped" ] || + fail "sync_strategy: $OLD_SYNC_STRATEGY and codeboarding_analysis_location: $location disagree. Set codeboarding_analysis_location only; sync_strategy is the input it replaces." + location="$mapped" + warn "sync_strategy is deprecated; replace sync_strategy: $OLD_SYNC_STRATEGY with codeboarding_analysis_location: $location, in the review workflow too." + [ "$OLD_SYNC_STRATEGY" != pull_request ] || + warn "Rolling sync pull requests are gone; close any open codeboarding/sync pull request." +fi + +if [ -n "${OLD_TARGET_BRANCH:-}" ]; then + # Only sync read it; the branch a sync runs on is the one its push trigger lists. + [ "$MODE" != sync ] || [ "$OLD_TARGET_BRANCH" = "$REF_NAME" ] || + fail "target_branch is no longer supported: sync analyzes the branch its push trigger lists, and this run is on $REF_NAME. List $OLD_TARGET_BRANCH under on: push: branches: instead, and remove target_branch." + warn "target_branch is deprecated and has no effect; remove it." +fi + +# Old templates pass `force_full: ${{ inputs.force_full || false }}`, so false is +# what nearly every one of them sends; only true asked for something. +[ "${OLD_FORCE_FULL:-false}" = false ] || + warn "force_full is no longer supported and has no effect; remove it." + +echo "analysis_location=$location" >> "$GITHUB_OUTPUT" diff --git a/scripts/action/state-names.sh b/scripts/action/resolve-cache-keys.sh similarity index 87% rename from scripts/action/state-names.sh rename to scripts/action/resolve-cache-keys.sh index 3c7dec9..b658ba6 100755 --- a/scripts/action/state-names.sh +++ b/scripts/action/resolve-cache-keys.sh @@ -18,11 +18,9 @@ digest() { # where the package is not importable, such as tests. engine_version="${ENGINE_VERSION:-}" if [ -z "$engine_version" ]; then - engine_version="$(python3 -c 'from importlib.metadata import version; print(version("codeboarding"))' 2>/dev/null || true)" -fi -if [ -z "$engine_version" ]; then - echo "::notice::Could not resolve the installed CodeBoarding version; this run will not reuse or publish analysis state." - exit 0 + # The engine was just installed and started, so an unreadable version is a broken install. + engine_version="$(python3 -c 'from importlib.metadata import version; print(version("codeboarding"))')" || + { reason="Could not read the installed CodeBoarding version."; echo "::error::$reason"; echo "failure_reason=$reason" >> "$GITHUB_OUTPUT"; exit 1; } fi # The name pins everything that decides what an analysis says, so a bundle is diff --git a/scripts/action/guard.sh b/scripts/action/resolve-github-event.sh similarity index 77% rename from scripts/action/guard.sh rename to scripts/action/resolve-github-event.sh index 84d9261..4f968ba 100755 --- a/scripts/action/guard.sh +++ b/scripts/action/resolve-github-event.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # Validates the event and outputs the exact refs and metadata used by later steps. set -euo pipefail -fail() { echo "::error::$1"; exit 1; } +fail() { echo "::error::$1"; echo "failure_reason=$1" >> "$GITHUB_OUTPUT"; exit 1; } skip() { echo "::notice::$1"; echo "skip=true" >> "$GITHUB_OUTPUT"; exit 0; } [ -z "${GH_HOST:-}" ] || export GH_HOST="${GH_HOST#*://}" case "$MODE" in @@ -9,33 +9,38 @@ case "$MODE" in *) fail "mode must be review or sync." ;; esac printf 'mode=%s\nskip=false\nevent=%s\n' "$MODE" "$EVENT" >> "$GITHUB_OUTPUT" +# The branch the analysis is kept on, or empty when it is committed in place, on +# the code branch itself. The webview reads the latest diagram from this name. +analysis_branch="" +[ "${ANALYSIS_LOCATION:-}" != codeboarding_branch ] || analysis_branch=codeboarding/baseline if [ "$MODE" = sync ]; then case "$EVENT" in push|workflow_dispatch|schedule) ;; *) skip "Sync mode ignores $EVENT events." ;; esac [ "$REF_TYPE" != tag ] || skip "Sync mode ignores tag pushes." - case "$SYNC_STRATEGY" in - push|pull_request) ;; - *) fail "sync_strategy must be push or pull_request." ;; - esac case "$HEAD_AUTHOR_EMAIL" in codeboarding-review\[bot\]@users.noreply.github.com|codeboarding\[bot\]@users.noreply.github.com) [ "$EVENT" != push ] || skip "Ignoring CodeBoarding's own baseline commit." ;; esac - target_branch="${TARGET_BRANCH_INPUT:-$REF_NAME}" - [ -n "$target_branch" ] || fail "target_branch is required for this event." - [ "$SYNC_STRATEGY" != pull_request ] || [ "$target_branch" != codeboarding/sync ] || fail "target_branch must differ from codeboarding/sync." - sync_branch_start_sha="" - if [ "$SYNC_STRATEGY" = pull_request ]; then - sync_branch_start_sha="$(gh api "repos/$REPOSITORY/branches/codeboarding%2Fsync" --jq '.commit.sha' 2>/dev/null || true)" + synced_branch="$REF_NAME" + [ -n "$synced_branch" ] || fail "Sync needs a branch to analyze; run it on one." + # One analysis branch holds one code branch's analysis: the default branch's. + # Checked here, in seconds, rather than after a full analysis. + if [ -n "$analysis_branch" ]; then + default_branch="${DEFAULT_BRANCH:-}" + [ -n "$default_branch" ] || + default_branch="$(gh api "repos/$REPOSITORY" --jq .default_branch 2>/dev/null || true)" + [ -n "$default_branch" ] || fail "Could not read the default branch of $REPOSITORY." + [ "$synced_branch" = "$default_branch" ] || + fail "Sync keeps the analysis of the default branch, $default_branch, and this run is on $synced_branch. Run sync on pushes to $default_branch only, or set codeboarding_analysis_location: in_place." fi { - echo "target_branch=$target_branch" - echo "sync_branch_start_sha=$sync_branch_start_sha" + echo "synced_branch=$synced_branch" + echo "analysis_branch=$analysis_branch" echo "checkout_repo=$REPOSITORY" - echo "checkout_ref=$target_branch" + echo "checkout_ref=$synced_branch" } >> "$GITHUB_OUTPUT" exit 0 fi @@ -91,7 +96,9 @@ for attempt in 1 2; do compare_behind="${compare#*$'\t'}" # A transient failure of one API call should not decide how this pull request # is measured, so try once more before giving up on the merge base. - [ -z "$compare_merge_base" ] && [ "$attempt" = 1 ] || break + if [ -n "$compare_merge_base" ] || [ "$attempt" != 1 ]; then + break + fi sleep 2 done case "$compare_behind" in @@ -122,6 +129,7 @@ is_fork=false echo "merge_base_resolved=$merge_base_resolved" echo "behind_by=$behind_by" echo "base_ref=$base_ref" + echo "analysis_branch=$analysis_branch" echo "head_sha=$head_sha" echo "base_repo=$base_repo" echo "head_repo=$head_repo" diff --git a/scripts/action/review-analyze-head.sh b/scripts/action/review-analyze-head.sh new file mode 100755 index 0000000..511790f --- /dev/null +++ b/scripts/action/review-analyze-head.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# Analyzes the pull request head. It continues from this pull request's last +# analysis when that grew from the same base graph, so a push only costs the +# commits pushed since; otherwise it starts from the base. +set -euo pipefail +# shellcheck source=scripts/action/analysis-state.sh +source "$(dirname "$0")/analysis-state.sh" +base_analysis="$BASE_ANALYSIS_PATH" +state="$RUNNER_TEMP/codeboarding-review/head-state" +rm -rf "$state" + +# The artifact name pins configuration; verify the stored cap and lineage too. +warmstart_usable() { + [ -f "${WARMSTART_DIR:-}/analysis.json" ] || return 1 + if ! compatible_state "$WARMSTART_DIR"; then + echo "::notice::Analysis depth changed since the last run; re-seeding from the base analysis." + return 1 + fi + # A head that grew from one base graph cannot be diffed against another: two + # runs of the engine over one commit need not name components identically, so + # keeping it would report additions and removals for code nobody touched. + if [ "$(origin_field "$WARMSTART_DIR" base_digest)" != "$(analysis_digest "$base_analysis")" ]; then + echo "::notice::The base analysis is not the one this pull request's stored analysis grew from; re-seeding from it." + return 1 + fi +} + +seed_source=base chain_depth=1 +if warmstart_usable; then + cp -a "$WARMSTART_DIR" "$state" + seed_source=pr-chain + previous_depth="$(origin_field "$state" chain_depth)" + case "$previous_depth" in + '' | *[!0-9]*) previous_depth=0 ;; + esac + chain_depth=$(( previous_depth + 1 )) +else + cp -a "$(dirname "$base_analysis")" "$state" +fi +rm -f "$state/origin.json" + +unset GIT_TOKEN +result="$("$ACTION_PATH/scripts/action/analyze.sh" head "$CHECKOUT_DIR" "$state")" +write_origin "$state" "$seed_source" "$chain_depth" "$(analysis_digest "$base_analysis")" +stage "$state" warmstart + +{ + echo "$result" + printf 'seed_source=%s\nchain_depth=%s\n' "$seed_source" "$chain_depth" +} >> "$GITHUB_OUTPUT" diff --git a/scripts/action/review-generate-baseline.sh b/scripts/action/review-generate-baseline.sh new file mode 100755 index 0000000..bfe664e --- /dev/null +++ b/scripts/action/review-generate-baseline.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# Produces the analysis of the review's merge base, the graph the pull request is +# compared against. First match wins: +# +# 1. the published analysis of the merge base (fetched by the step before) used as is +# 2. an entry on the baseline branch for the merge base or an ancestor \ +# 3. the analysis committed on the branch at the merge base > caught up to the merge base +# 4. the saved analysis of the nearest ancestor / +# 5. nothing built from scratch +# +# Outputs base_analysis_path, and publish_base when this run produced a base +# other pull requests forking there should reuse. +set -euo pipefail +# shellcheck source=scripts/action/seed-sources.sh +source "$(dirname "$0")/seed-sources.sh" +work="$RUNNER_TEMP/codeboarding-review" +checkout="$work/base" state="$work/base-state" +rm -rf "$work" +mkdir -p "$work" +token="${GIT_TOKEN:-}" + +progress() { + GH_TOKEN="$token" GH_ENTERPRISE_TOKEN="$token" \ + "$ACTION_PATH/scripts/action/update-review-progress.sh" "$1" >/dev/null 2>&1 || true +} + +publish_base=true +if compatible_state "${BASE_DIR:-}"; then + cp -a "$BASE_DIR" "$state" + echo "::notice::Using the published analysis of ${REVIEW_BASE_SHA:0:7}." + # Only a base about to expire is published again: a review artifact references + # it by id for its whole retention. + publish_base="${RENEW_BASE:-false}" +else + fetch_commit "$REVIEW_BASE_REPO" "$REVIEW_BASE_SHA" + git -C "$CHECKOUT_DIR" worktree add --detach "$checkout" "$REVIEW_BASE_SHA" >/dev/null + if from_codeboarding_baseline "$REVIEW_BASE_REPO" "$REVIEW_BASE_SHA" "$state" "$checkout" || + from_committed_baseline "$checkout" "$state" || + from_ancestor_artifact "$REVIEW_BASE_REPO" "$REVIEW_BASE_SHA" "$state" false "$checkout"; then + seeded=true + else + seeded=false + keep_user_config "$checkout" "$state" + fi + # Core never sees the token. + unset GIT_TOKEN + if [ "$SEED_SHA" = "$REVIEW_BASE_SHA" ]; then + echo "::notice::That analysis describes the merge base itself; nothing to catch up." + elif [ "$seeded" = true ]; then + "$ACTION_PATH/scripts/action/analyze.sh" base "$checkout" "$state" >/dev/null + else + echo "::notice::No saved analysis to start from; analyzing ${REVIEW_BASE_SHA:0:7} from scratch." + progress base + "$ACTION_PATH/scripts/action/analyze.sh" base "$checkout" "$state" >/dev/null + progress head + fi +fi +[ -f "$state/analysis.json" ] || { echo "::error::Review baseline analysis is missing."; exit 1; } +[ "$publish_base" != true ] || stage "$state" base +printf 'base_analysis_path=%s\npublish_base=%s\n' "$state/analysis.json" "$publish_base" >> "$GITHUB_OUTPUT" diff --git a/scripts/action/seed-sources.sh b/scripts/action/seed-sources.sh new file mode 100644 index 0000000..6922d10 --- /dev/null +++ b/scripts/action/seed-sources.sh @@ -0,0 +1,68 @@ +# shellcheck shell=bash +# Where an analysis to continue from can come from. Sourced, not run. +# +# Each from_* function fills a state directory with a compatible analysis of a +# commit or one of its ancestors and returns 0, or leaves the directory alone and +# returns 1. SEED_SHA is the commit the seed describes, when the source knows it; +# a seed of the very commit being analyzed needs no engine run at all. +HERE="$(dirname "${BASH_SOURCE[0]}")" +# shellcheck source=scripts/action/analysis-state.sh +source "$HERE/analysis-state.sh" +# shellcheck source=scripts/action/codeboarding-baseline.sh +source "$HERE/codeboarding-baseline.sh" +SEED_SHA="" + +# The analysis sync committed under .codeboarding/ on the branch itself. Which +# commit it describes is not recorded, so it is always caught up. +from_committed_baseline() { + local checkout="$1" state="$2" + [ -f "$checkout/.codeboarding/analysis.json" ] || return 1 + if ! compatible_state "$checkout/.codeboarding"; then + echo "::notice::The analysis committed on this branch was made with another depth; not starting from it." + return 1 + fi + replace_state "$checkout/.codeboarding" "$state" "$checkout" + echo "::notice::Starting from the analysis committed on this branch." +} + +# The analysis committed under .codeboarding/ on the parent of $2, when $2 deleted +# it: the commit that moves a repository to the analysis branch does, and would +# otherwise cost the first sync a full analysis. +from_parent_committed_baseline() { + local repository="$1" tip="$2" state="$3" checkout="$4" parent dest="$RUNNER_TEMP/codeboarding-parent-baseline" + [ ! -e "$checkout/.codeboarding/analysis.json" ] && [ -n "$repository" ] || return 1 + fetch_commit "$repository" "$tip" 2 >&2 || return 1 + parent="$(git -C "$checkout" rev-parse -q --verify "$tip^")" || return 1 + git -C "$checkout" cat-file -e "$parent:.codeboarding/analysis.json" 2>/dev/null || return 1 + rm -rf "$dest" + mkdir -p "$dest" + git -C "$checkout" archive "$parent" .codeboarding | tar -x -C "$dest" || return 1 + if ! compatible_state "$dest/.codeboarding"; then + echo "::notice::The analysis committed on ${parent:0:7} was made with another depth; not starting from it." + return 1 + fi + replace_state "$dest/.codeboarding" "$state" "$checkout" + echo "::notice::Starting from the analysis committed on ${parent:0:7}, which ${tip:0:7} removed." +} + +# A codeboarding-base-- artifact for the nearest first-parent ancestor +# of $2, or of $2 itself when $4 is true. A review already looked its tip up by +# exact name; sync has no such lookup. +from_ancestor_artifact() { + local repository="$1" tip="$2" state="$3" include_tip="$4" config_from="$5" found ancestor dest="$RUNNER_TEMP/codeboarding-ancestor" + [ "${ANCESTOR_LOOKUP:-false}" = true ] && [ -n "${CFG_HASH:-}" ] && [ -n "${REPOSITORY:-}" ] && [ -n "$tip" ] || return 1 + fetch_commit "$repository" "$tip" "$(( CATCHUP_BOUND + 1 ))" || true + found="$(GH_TOKEN="${GIT_TOKEN:-}" GH_ENTERPRISE_TOKEN="${GIT_TOKEN:-}" TIP_SHA="$tip" DEST="$dest" \ + INCLUDE_TIP="$include_tip" CATCHUP_BOUND="$CATCHUP_BOUND" \ + "$HERE/find-ancestor-base.sh" || true)" + ancestor="$(awk -F= '$1 == "ancestor_sha" {print $2; exit}' <<< "$found")" + [ -n "$ancestor" ] || return 1 + if ! compatible_state "$dest"; then + echo "::notice::The saved analysis of ${ancestor:0:7} was made with another depth; not starting from it." + return 1 + fi + replace_state "$dest" "$state" "$config_from" + # shellcheck disable=SC2034 # read by the scripts that source this file + SEED_SHA="$ancestor" + echo "::notice::Starting from the saved analysis of ${ancestor:0:7}." +} diff --git a/scripts/action/sync-generate-baseline.sh b/scripts/action/sync-generate-baseline.sh new file mode 100755 index 0000000..88daf72 --- /dev/null +++ b/scripts/action/sync-generate-baseline.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# Produces the analysis of the synced branch's head, continuing from the nearest +# saved analysis. First match wins: +# +# 1. an entry on the baseline branch for this commit or an ancestor +# 2. the analysis committed on the branch, or on the parent of a commit that removed it +# 3. the saved analysis of this commit or the nearest ancestor +# 4. nothing: analyzed from scratch +set -euo pipefail +# shellcheck source=scripts/action/seed-sources.sh +source "$(dirname "$0")/seed-sources.sh" +state="$RUNNER_TEMP/codeboarding-sync/analysis" +rm -rf "$RUNNER_TEMP/codeboarding-sync" +head_sha="$(git -C "$CHECKOUT_DIR" rev-parse HEAD)" +# An analysis branch holds only analysis: analyzing it, and committing the result +# onto it, would leave it unreadable as one. +if [ -n "$(git -C "$CHECKOUT_DIR" log -1 --format='%(trailers:key=CodeBoarding-Source,valueonly)' | tr -d '[:space:]')" ]; then + reason="This run is on a CodeBoarding analysis branch. Run sync on the code branch it analyzes." + echo "::error::$reason" + echo "failure_reason=$reason" >> "$GITHUB_OUTPUT" + exit 1 +fi + +if ! from_codeboarding_baseline "${REPOSITORY:-}" "$head_sha" "$state" "$CHECKOUT_DIR" && + ! from_committed_baseline "$CHECKOUT_DIR" "$state" && + ! from_parent_committed_baseline "${REPOSITORY:-}" "$head_sha" "$state" "$CHECKOUT_DIR" && + ! from_ancestor_artifact "${REPOSITORY:-}" "$head_sha" "$state" true "$CHECKOUT_DIR"; then + keep_user_config "$CHECKOUT_DIR" "$state" +fi + +# Core never sees the token. +unset GIT_TOKEN +result="$("$ACTION_PATH/scripts/action/analyze.sh" sync "$CHECKOUT_DIR" "$state")" +# Sync already computes the graph every review of this branch compares against, +# so publish it instead of making the first pull request recompute it. +stage "$state" base +{ + echo "$result" + echo "analysis_dir=$state" +} >> "$GITHUB_OUTPUT" diff --git a/scripts/action/sync-summary.sh b/scripts/action/sync-summary.sh index 287b9af..eae914f 100755 --- a/scripts/action/sync-summary.sh +++ b/scripts/action/sync-summary.sh @@ -3,11 +3,11 @@ set -euo pipefail { echo "### CodeBoarding Sync" - echo "- Analysis: ${MODE}" - echo "- Analysis artifacts: ${FILES:-0}" - echo "- Delivered: ${COMMITTED:-false}" - echo "- Strategy: ${STRATEGY}" - if [ -n "${PR_URL:-}" ]; then - echo "- Sync PR: ${PR_URL}" + [ -z "${FAILURE_REASON:-}" ] || echo "- **Failed:** ${FAILURE_REASON}" + if [ -n "${MODE:-}" ]; then + echo "- Analysis: ${MODE}" + echo "- Analysis artifacts: ${FILES:-0}" + echo "- Delivered: ${COMMITTED:-false}" + echo "- Saved to: ${SAVED_TO}" fi } >> "$GITHUB_STEP_SUMMARY" diff --git a/scripts/action/update-review-progress.sh b/scripts/action/update-review-progress.sh new file mode 100755 index 0000000..ec5be1d --- /dev/null +++ b/scripts/action/update-review-progress.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# Rewrites the review's sticky progress comment into two steps when the base is +# built from scratch: that is the slow path, and the reader should know why. +# Usage: update-review-progress.sh base|head, once as the base build starts and +# once as it ends. Best effort throughout. +set -euo pipefail +step="$1" +[ -n "${PROGRESS_HEADER:-}" ] && [ -n "${PR_NUMBER:-}" ] && [ -n "${REPOSITORY:-}" ] || exit 0 +export GH_HOST="${GH_HOST:-github.com}" +GH_HOST="${GH_HOST#*://}" + +# The sticky-comment action finds its comment by this line, so it must survive the edit. +marker="" +id_file="${RUNNER_TEMP:?}/codeboarding-progress-comment-${PROGRESS_HEADER}" +id="$(cat "$id_file" 2>/dev/null || true)" +if [ -z "$id" ]; then + id="$(gh api --paginate "repos/$REPOSITORY/issues/$PR_NUMBER/comments?per_page=100" \ + --jq ".[] | select((.body // \"\") | startswith(\"### CodeBoarding review\") and contains(\"$marker\")) | .id" | + tail -n 1)" + [ -n "$id" ] || exit 0 + echo "$id" > "$id_file" +fi + +if [ -n "${BASE_REF:-}" ]; then + branch="\`$BASE_REF\`" +else + branch="the base branch" +fi +sha7="${REVIEW_BASE_SHA:0:7}" +why="$branch has no saved diagram this review can start from, so it builds one first. Once a diagram of $branch is saved, reviews start from it and skip this step." +if [ "$step" = base ]; then + first="1. ⏳ Building the diagram of $branch @$sha7 from scratch" + second="2. Analysing this PR's changes" +else + first="1. ✅ Built the diagram of $branch @$sha7 from scratch" + second="2. ⏳ Analysing this PR's changes" +fi + +platform="https://app.codeboarding.org/$REPOSITORY/pull/$PR_NUMBER?utm_source=github&utm_medium=pr_comment&utm_campaign=gh_action" +run_url="${GITHUB_SERVER_URL:-https://github.com}/$REPOSITORY/actions/runs/${GITHUB_RUN_ID:-}" +body="$(printf '%s\n\n%s\n %s\n%s\n\n%s\n\n%s\n%s' \ + '### CodeBoarding review · analyzing…' \ + "$first" "$why" "$second" \ + "Open it in [CodeBoarding]($platform) meanwhile: the files, comments and review are there already, and the diff appears when the run finishes." \ + "run [${GITHUB_RUN_ID:-}]($run_url) · attempt ${GITHUB_RUN_ATTEMPT:-1}" \ + "$marker")" +gh api -X PATCH "repos/$REPOSITORY/issues/comments/$id" -f body="$body" >/dev/null diff --git a/scripts/action/with-auth.sh b/scripts/action/with-auth.sh index 1b4f1ad..57a8b00 100755 --- a/scripts/action/with-auth.sh +++ b/scripts/action/with-auth.sh @@ -1,14 +1,10 @@ #!/usr/bin/env bash # Runs one command with the resolved provider credentials, then removes them. +# KEEP_AUTH=true leaves them for a following step that analyzes too; the action's +# final clear-auth.sh step removes them then. set -euo pipefail AUTH_DIR="${RUNNER_TEMP}/codeboarding-auth" -cleanup() { - if [ -s "$AUTH_DIR/relay.pid" ]; then - kill "$(cat "$AUTH_DIR/relay.pid")" 2>/dev/null || true - fi - rm -rf "$AUTH_DIR" -} -trap cleanup EXIT +[ "${KEEP_AUTH:-false}" = true ] || trap '"$(dirname "$0")/clear-auth.sh"' EXIT if [ ! -s "$AUTH_DIR/tier" ] || [ ! -s "$AUTH_DIR/provider-name" ]; then echo "::error::CodeBoarding analysis credentials are unavailable." diff --git a/tests/test_action_inputs.py b/tests/test_action_inputs.py index 4757bc7..5fc96e7 100644 --- a/tests/test_action_inputs.py +++ b/tests/test_action_inputs.py @@ -86,6 +86,24 @@ def test_the_inferred_credential_inputs_are_gone(self) -> None: self.assertNotIn(stale, self.inputs) self.assertNotIn(f"inputs.{stale}", ACTION) + def test_every_sync_setup_can_read_saved_analyses(self) -> None: + """Sync catches up from an analysis a review saved; without actions: read it silently analyzes in full.""" + readme = (ROOT / "README.md").read_text(encoding="utf-8") + sync_setups = [block for block in readme.split("```yaml")[1:] if "mode: sync" in block] + sync_setups.append(DOGFOOD[DOGFOOD.index("\n sync:\n") :]) + for block in sync_setups: + if "permissions:" in block: + self.assertIn("actions: read", block, block[:200]) + + def test_deprecated_inputs_are_read_only_by_the_migrator(self) -> None: + """Backward compatibility lives in one script; every other step reads its outputs.""" + for old in ("target_branch", "sync_strategy"): + self.assertIn("deprecationMessage:", self.inputs[old]) + self.assertEqual(ACTION.count(f"inputs.{old} }}}}"), 1, f"only the migrator may read {old}") + new = "codeboarding_analysis_location" + self.assertEqual(ACTION.count(f"inputs.{new} }}}}"), 1, f"only the migrator may read {new}") + self.assertLess(ACTION.index("- name: Translate deprecated inputs"), ACTION.index("- name: Resolve event")) + def test_credentials_resolve_before_the_checkout_and_the_engine_install(self) -> None: """Fail-fast is positional: preflight is worth little after a minute of setup.""" preflight = ACTION.index("- name: Check LLM configuration") diff --git a/tests/test_action_state.py b/tests/test_action_state.py index 7b5a8aa..0a02ea4 100644 --- a/tests/test_action_state.py +++ b/tests/test_action_state.py @@ -11,8 +11,46 @@ ROOT = Path(__file__).resolve().parent.parent -STATE_NAMES = ROOT / "scripts" / "action" / "state-names.sh" -ANALYZE = ROOT / "scripts" / "action" / "analyze.sh" +SCRIPTS = ROOT / "scripts" / "action" +STATE_NAMES = SCRIPTS / "resolve-cache-keys.sh" +# The analysis steps of each mode, in the order action.yml runs them. +STEPS = { + "review": (SCRIPTS / "review-generate-baseline.sh", SCRIPTS / "review-analyze-head.sh"), + "sync": (SCRIPTS / "sync-generate-baseline.sh",), +} + + +def read_outputs(output: Path) -> dict[str, str]: + values: dict[str, str] = {} + for line in output.read_text(encoding="utf-8").splitlines(): + key, _, value = line.partition("=") + values[key] = value + return values + + +def run_steps(env: dict[str, str], output: Path) -> tuple[int, str, dict[str, str]]: + """Runs one mode's analysis steps as action.yml does, each seeing the outputs before it. + + Returns the first failing exit code (or 0), everything the steps logged, and their outputs. + """ + env = dict(env) + kind = env.pop("ANALYSIS_KIND", "review") + output.write_text("", encoding="utf-8") + log = "" + for script in STEPS[kind]: + base = read_outputs(output).get("base_analysis_path", "") + result = subprocess.run( + [str(script)], + env={**env, "GITHUB_OUTPUT": str(output), "BASE_ANALYSIS_PATH": base}, + capture_output=True, + text=True, + check=False, + ) + log += result.stdout + result.stderr + if result.returncode != 0: + return result.returncode, log, read_outputs(output) + return 0, log, read_outputs(output) + ENGINE_STUB = '''#!/usr/bin/env python3 """CodeBoarding CLI stand-in: records each call and writes a minimal analysis.""" @@ -26,6 +64,7 @@ "mode": argv[0], "checkout": argv[argv.index("--local") + 1], "depth": argv[argv.index("--depth-cap") + 1] if "--depth-cap" in argv else None, + "run_id": os.environ.get("CODEBOARDING_RUN_ID", ""), }) + "\\n") analysis = os.path.join(output, "analysis.json") metadata = json.load(open(analysis))["metadata"] if os.path.isfile(analysis) else {} @@ -140,16 +179,28 @@ def test_model_selection_changes_the_identity(self) -> None: self.assertNotEqual(baseline["cfg_hash"], self._run(PARSING_MODEL_INPUT="gpt-5")["cfg_hash"]) self.assertNotEqual(baseline["cfg_hash"], self._run(LLM_PROVIDER="anthropic")["cfg_hash"]) - def test_unresolvable_engine_version_disables_reuse_instead_of_failing(self) -> None: + def test_an_unresolvable_engine_version_fails_instead_of_disabling_reuse(self) -> None: stub_bin = self.root / "bin" stub_bin.mkdir() python_stub = stub_bin / "python3" python_stub.write_text("#!/bin/sh\nexit 1\n", encoding="utf-8") python_stub.chmod(0o755) - values = self._run(ENGINE_VERSION="", PATH=f"{stub_bin}:{os.environ['PATH']}") + result = subprocess.run( + [str(STATE_NAMES)], + env={ + "PATH": f"{stub_bin}:{os.environ['PATH']}", + "GITHUB_OUTPUT": str(self.output), + "CHECKOUT_DIR": str(self.checkout), + "ENGINE_VERSION": "", + }, + capture_output=True, + text=True, + check=False, + ) - self.assertEqual(values, {}) + self.assertNotEqual(result.returncode, 0) + self.assertIn("Could not read the installed CodeBoarding version", result.stdout) class ReviewChainTests(unittest.TestCase): @@ -176,16 +227,13 @@ def tearDown(self) -> None: self.temp_dir.cleanup() def _analyze(self, **extra: str) -> dict[str, str]: - self.output.write_text("", encoding="utf-8") - result = subprocess.run( - [str(ANALYZE)], - env={ + code, self.log, values = run_steps( + { "PATH": f"{self.bin_dir}:{os.environ['PATH']}", - "GITHUB_OUTPUT": str(self.output), "RUNNER_TEMP": str(self.runner_temp), "CB_ENGINE_LOG": str(self.engine_log), "ACTION_PATH": str(ROOT), - "ANALYSIS_KIND": "review", + "GITHUB_RUN_ID": "1", "CHECKOUT_DIR": str(self.checkout), "REVIEW_BASE_SHA": "merge-base-sha", "REVIEW_HEAD_SHA": "head-sha", @@ -199,20 +247,18 @@ def _analyze(self, **extra: str) -> dict[str, str]: "STAGE_DIR": str(self.stage_dir), **extra, }, - capture_output=True, - text=True, - check=False, + self.output, ) - self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - values: dict[str, str] = {} - for line in self.output.read_text(encoding="utf-8").splitlines(): - key, _, value = line.partition("=") - values[key] = value + self.assertEqual(code, 0, self.log) return values def _engine_calls(self) -> list[dict[str, str]]: return [json.loads(line) for line in self.engine_log.read_text(encoding="utf-8").splitlines()] + def _base_modes(self) -> list[str]: + """How the base was obtained, read the way telemetry reads it: no base run means reused.""" + return [c["mode"] for c in self._engine_calls() if c["run_id"].endswith("-base")] + def _bind(self, **origin: object) -> None: """Chain fixture bound to the base it was derived from.""" _state(self.warmstart_dir, base_digest=_digest(self.base_dir / "analysis.json"), **origin) @@ -286,6 +332,16 @@ def test_compatible_committed_baseline_runs_incrementally(self) -> None: self._analyze(REVIEW_BASE_SHA=sha, DEPTH_CAP="4") self.assertEqual([c["mode"] for c in self._engine_calls()], ["incremental", "incremental"]) + def test_engine_runs_are_tagged_with_the_run_and_the_analysis_they_belong_to(self) -> None: + sha = self._commit_base(cap=4) + self._analyze(REVIEW_BASE_SHA=sha, DEPTH_CAP="4", GITHUB_RUN_ID="991", GITHUB_RUN_ATTEMPT="2") + self.assertEqual([c["run_id"] for c in self._engine_calls()], ["gh-991-2-base", "gh-991-2-head"]) + + def test_a_reused_base_leaves_only_head_engine_runs(self) -> None: + _state(self.base_dir) + self._analyze(GITHUB_RUN_ID="991", GITHUB_RUN_ATTEMPT="1") + self.assertEqual([c["run_id"] for c in self._engine_calls()], ["gh-991-1-head"]) + def test_legacy_committed_depth_is_not_inherited(self) -> None: sha = self._commit_base(legacy=True) self._analyze(REVIEW_BASE_SHA=sha, DEPTH_CAP="4") @@ -312,9 +368,115 @@ def test_base_and_head_fallbacks_keep_configured_depth(self) -> None: self.assertEqual([c["mode"] for c in calls], ["incremental", "full", "incremental", "full"]) self.assertEqual([c["depth"] for c in calls if c["mode"] == "full"], ["4", "4"]) + # How the base was obtained is reported, not just used: the review comment and + # the webview explain a slow run by it. + + def _git(self, *args: str) -> str: + return subprocess.run( + [ + "git", + "-C", + str(self.checkout), + "-c", + "user.name=Test", + "-c", + "user.email=test@example.com", + "-c", + "commit.gpgsign=false", + *args, + ], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + def _commit(self, message: str, files: dict[str, str], *, bot: bool = False) -> str: + for name, content in files.items(): + (self.checkout / name).parent.mkdir(parents=True, exist_ok=True) + (self.checkout / name).write_text(content, encoding="utf-8") + self._git("add", "-A") + committer = ("-c", "user.email=codeboarding-review[bot]@users.noreply.github.com") if bot else () + self._git(*committer, "commit", "-q", "-m", message) + return self._git("rev-parse", "HEAD") + + def test_a_saved_base_is_reused(self) -> None: + _state(self.base_dir) + + self._analyze() + + self.assertEqual(self._base_modes(), []) + self.assertIn("Using the published analysis of merge-b.", self.log) + + def test_a_base_with_nothing_to_seed_it_is_a_full_analysis(self) -> None: + sha = self._commit_base() + + self._analyze(REVIEW_BASE_SHA=sha) + + self.assertEqual(self._base_modes(), ["full"]) + + def _assert_incompatible(self, _values: dict[str, str]) -> None: + self.assertEqual(self._base_modes()[-1:], ["full"]) + + def test_a_committed_baseline_with_another_depth_is_incompatible(self) -> None: + sha = self._commit_base(legacy=True) + + self._assert_incompatible(self._analyze(REVIEW_BASE_SHA=sha)) + + def test_a_saved_base_with_another_depth_is_incompatible(self) -> None: + sha = self._commit_base() + _state(self.base_dir, cap=1) + + self._assert_incompatible(self._analyze(REVIEW_BASE_SHA=sha)) + + def test_an_engine_that_demands_a_full_run_makes_the_baseline_incompatible(self) -> None: + sha = self._commit_base(cap=2) + + self._assert_incompatible(self._analyze(REVIEW_BASE_SHA=sha, CB_REQUIRE_FULL="true")) + + def _progress_stub(self) -> Path: + calls = self.root / "gh-calls" + gh = self.bin_dir / "gh" + gh.write_text( + "#!/usr/bin/env bash\n" + f'printf "%s\\n----\\n" "$*" >> "{calls}"\n' + 'case "$*" in *"/comments?per_page"*) echo 77 ;; esac\n', + encoding="utf-8", + ) + gh.chmod(0o755) + return calls + + def test_building_the_base_from_scratch_rewrites_the_progress_comment(self) -> None: + calls = self._progress_stub() + sha = self._commit_base() + + self._analyze( + REVIEW_BASE_SHA=sha, + PROGRESS_HEADER="codeboarding-review", + REPOSITORY="owner/repo", + BASE_REF="develop", + GIT_TOKEN="token", + ) + + patches = [call for call in calls.read_text().split("\n----\n") if "PATCH" in call] + self.assertEqual(len(patches), 2, calls.read_text()) + self.assertIn("repos/owner/repo/issues/comments/77", patches[0]) + self.assertIn(f"Building the diagram of `develop` @{sha[:7]} from scratch", patches[0]) + self.assertIn("`develop` has no saved diagram this review can start from", patches[0]) + self.assertIn("2. ⏳ Analysing this PR's changes", patches[-1]) + # The sticky-comment action finds its comment by this line on the final write. + self.assertIn("", patches[-1]) + + def test_a_saved_base_leaves_the_progress_comment_alone(self) -> None: + calls = self._progress_stub() + _state(self.base_dir) + + self._analyze(PROGRESS_HEADER="codeboarding-review", REPOSITORY="owner/repo", GIT_TOKEN="token") + + self.assertFalse(calls.exists()) + def test_invalid_depth_fails_before_analysis(self) -> None: result = subprocess.run( - [str(ANALYZE)], + [str(STEPS["review"][0])], env={"PATH": os.environ["PATH"], "DEPTH_CAP": "-1"}, capture_output=True, text=True, @@ -324,7 +486,10 @@ def test_invalid_depth_fails_before_analysis(self) -> None: self.assertEqual(self._engine_calls(), []) def test_sync_without_baseline_uses_configured_depth_directly(self) -> None: - self._analyze(ANALYSIS_KIND="sync", FORCE_FULL="false", DEPTH_CAP="4") + # Sync always runs on a checkout of the synced branch. + self._git("init", "-q") + self._commit("code", {"app.py": "pass\n"}) + self._analyze(ANALYSIS_KIND="sync", DEPTH_CAP="4") self.assertEqual([c["mode"] for c in self._engine_calls()], ["full"]) self.assertEqual(self._engine_calls()[0]["depth"], "4") @@ -419,6 +584,275 @@ def test_analysis_is_staged_for_publication(self) -> None: self.assertFalse((self.stage_dir / "base").exists(), "a published base needs no republishing") +GH_STUB = """#!/usr/bin/env python3 +\"\"\"gh stand-in: serves an artifact listing and one zip from a JSON config.\"\"\" +import json, os, sys +from urllib.parse import parse_qs, urlparse + +config = json.load(open(os.environ["CB_GH_CONFIG"])) +with open(os.environ["CB_GH_LOG"], "a") as log: + log.write(" ".join(sys.argv[1:]) + "\\n") +path = next(a for a in sys.argv[1:] if a.startswith("repos/")) +url = urlparse(path) +query = parse_qs(url.query) +if url.path.endswith("/zip"): + sys.stdout.buffer.write(open(config["zip"], "rb").read()) +elif url.path.endswith("/actions/artifacts"): + artifacts = config["artifacts"] + if "name" in query: + artifacts = [a for a in artifacts if a["name"] == query["name"][0]] + page = int(query.get("page", ["1"])[0]) + if "name" not in query and "pages" in config: + pages = config["pages"] + print(json.dumps({"artifacts": pages[page - 1] if page <= len(pages) else []})) + else: + print(json.dumps({"artifacts": artifacts if page == 1 else []})) +""" + + +class AncestorSeedTests(unittest.TestCase): + """With no saved base for the merge base and nothing committed there, a review + catches up from the nearest saved ancestor on the base branch's first-parent + history instead of analyzing the merge base from scratch.""" + + def setUp(self) -> None: + import zipfile + + self.temp_dir = tempfile.TemporaryDirectory() + self.root = Path(self.temp_dir.name) + self.bin_dir = self.root / "bin" + self.bin_dir.mkdir() + for name, body in (("codeboarding", ENGINE_STUB), ("gh", GH_STUB)): + (self.bin_dir / name).write_text(body, encoding="utf-8") + (self.bin_dir / name).chmod(0o755) + self.engine_log = self.root / "engine.log" + self.engine_log.write_text("", encoding="utf-8") + self.gh_log = self.root / "gh.log" + self.gh_log.write_text("", encoding="utf-8") + self.gh_config = self.root / "gh.json" + self.output = self.root / "github-output" + self.runner_temp = self.root / "runner" + self.runner_temp.mkdir() + self.stage_dir = self.root / "state" / "out" + self.origin = self.root / "origin" + self.origin.mkdir() + self.bundle = self.root / "bundle.zip" + with zipfile.ZipFile(self.bundle, "w") as archive: + archive.writestr("analysis.json", json.dumps({"metadata": {"depth_cap": 2}, "components": []})) + archive.writestr("static_analysis.pkl", "pickle") + archive.writestr("metadata.json", json.dumps({"kind": "base", "merge_base_sha": "ancestor"})) + + def tearDown(self) -> None: + self.temp_dir.cleanup() + + def _history(self, commits: int) -> list[str]: + """A base branch of `commits` code commits, oldest first, in origin/ (the checkout).""" + git = ["git", "-C", str(self.origin), "-c", "user.name=T", "-c", "user.email=t@example.com"] + subprocess.run([*git, "init", "-q"], check=True) + shas = [] + for index in range(commits): + (self.origin / f"file{index}.py").write_text("pass\n", encoding="utf-8") + subprocess.run([*git, "add", "-A"], check=True) + subprocess.run([*git, "-c", "commit.gpgsign=false", "commit", "-q", "-m", f"c{index}"], check=True) + shas.append(subprocess.check_output([*git, "rev-parse", "HEAD"], text=True).strip()) + return shas + + @staticmethod + def _artifact(name: str, *, fork: bool = False) -> dict: + return { + "id": abs(hash(name)) % 100000, + "name": name, + "expired": False, + "created_at": "2026-10-01T00:00:00Z", + "expires_at": "2027-01-01T00:00:00Z", + "workflow_run": {"id": 1, "repository_id": 1, "head_repository_id": 2 if fork else 1}, + } + + def _serve(self, artifacts: list[dict], pages: list | None = None) -> None: + config = {"artifacts": artifacts, "zip": str(self.bundle)} + if pages is not None: + config["pages"] = pages + self.gh_config.write_text(json.dumps(config), encoding="utf-8") + + def _analyze(self, checkout: Path, **extra: str) -> dict[str, str]: + code, self.log, values = run_steps( + { + "PATH": f"{self.bin_dir}:{os.environ['PATH']}", + "RUNNER_TEMP": str(self.runner_temp), + "CB_ENGINE_LOG": str(self.engine_log), + "CB_GH_CONFIG": str(self.gh_config), + "CB_GH_LOG": str(self.gh_log), + "ACTION_PATH": str(ROOT), + "GITHUB_RUN_ID": "1", + "CHECKOUT_DIR": str(checkout), + "REVIEW_HEAD_SHA": "head-sha", + "REVIEW_BASE_REPO": "origin", + "REPOSITORY": "owner/repo", + "GITHUB_SERVER_URL": f"file://{self.root}", + "PR_NUMBER": "42", + "ENGINE_VERSION": "0.14.5", + "CFG_HASH": "cfg", + "ANCESTOR_LOOKUP": "true", + "BASE_DIR": str(self.root / "state" / "base"), + "WARMSTART_DIR": str(self.root / "state" / "warmstart"), + "STAGE_DIR": str(self.stage_dir), + "DEPTH_CAP": "2", + **extra, + }, + self.output, + ) + self.assertEqual(code, 0, self.log) + return values + + def _modes(self) -> list[str]: + return [json.loads(line)["mode"] for line in self.engine_log.read_text().splitlines()] + + def _base_modes(self) -> list[str]: + calls = [json.loads(line) for line in self.engine_log.read_text().splitlines()] + return [c["mode"] for c in calls if c["run_id"].endswith("-base")] + + def _assert_full(self, _values: dict[str, str]) -> None: + self.assertEqual(self._base_modes(), ["full"]) + + def test_it_catches_up_from_the_nearest_saved_ancestor(self) -> None: + shas = self._history(5) + merge_base = shas[-1] + # Two saved ancestors: the nearer one wins. + self._serve( + [self._artifact(f"codeboarding-base-cfg-{shas[0]}"), self._artifact(f"codeboarding-base-cfg-{shas[2]}")] + ) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=merge_base) + + self.assertIn(f"Starting from the saved analysis of {shas[2][:7]}.", self.log) + self.assertEqual(self._modes(), ["incremental", "incremental"]) + # Published under the merge base's own name, so the next review hits it exactly. + self.assertEqual(values["publish_base"], "true") + staged = json.loads((self.stage_dir / "base" / "metadata.json").read_text()) + self.assertEqual(staged["merge_base_sha"], merge_base) + + def test_an_ancestor_beyond_the_bound_is_not_used(self) -> None: + shas = self._history(5) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1], CATCHUP_BOUND="2") + + self._assert_full(values) + self.assertEqual(self._modes(), ["full", "incremental"]) + self.assertNotIn("/zip", self.gh_log.read_text()) + + def test_a_saved_analysis_off_this_history_is_not_used(self) -> None: + shas = self._history(2) + self._serve([self._artifact("codeboarding-base-cfg-" + "e" * 40)]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1]) + + self._assert_full(values) + + def test_a_busy_artifact_store_is_paged_until_a_saved_ancestor_appears(self) -> None: + shas = self._history(3) + noise = [self._artifact(f"codeboarding-review-{i}-1") for i in range(100)] + found = self._artifact(f"codeboarding-base-cfg-{shas[0]}") + later = [self._artifact(f"codeboarding-warmstart-cfg-pr{i}") for i in range(100)] + self._serve([found], pages=[noise] * 11 + [noise[:99] + [found], later]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1]) + + self.assertIn(f"Starting from the saved analysis of {shas[0][:7]}.", self.log) + listings = [ + line + for line in self.gh_log.read_text().splitlines() + if "per_page=100&page=" in line and "name=" not in line + ] + self.assertEqual(len(listings), 12, "paging stops at the page holding the ancestor") + + def test_a_failed_deepen_falls_back_to_a_full_analysis(self) -> None: + # The deepen fails, so the shallow checkout walks no ancestor at all. + shas = self._history(4) + bare = self.root / "origin.git" + subprocess.run(["git", "clone", "-q", "--bare", str(self.origin), str(bare)], check=True) + checkout = self.root / "shallow" + subprocess.run(["git", "clone", "-q", "--depth=1", f"file://{bare}", str(checkout)], check=True) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + values = self._analyze(checkout, REVIEW_BASE_SHA=shas[-1], GITHUB_SERVER_URL=f"file://{self.root}/missing") + + self._assert_full(values) + + def test_the_merge_bases_own_configuration_survives_the_seed(self) -> None: + shas = self._history(2) + (self.origin / ".codeboarding").mkdir() + (self.origin / ".codeboarding" / ".codeboardingignore").write_text("docs/\n", encoding="utf-8") + git = ["git", "-C", str(self.origin), "-c", "user.name=T", "-c", "user.email=t@example.com"] + subprocess.run([*git, "add", "-A"], check=True) + subprocess.run([*git, "-c", "commit.gpgsign=false", "commit", "-q", "-m", "ignore docs"], check=True) + merge_base = subprocess.check_output([*git, "rev-parse", "HEAD"], text=True).strip() + import zipfile + + with zipfile.ZipFile(self.bundle, "a") as archive: + archive.writestr(".codeboardingignore", "stale/\n") + archive.writestr("health/health_config.json", "{}") + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + self._analyze(self.origin, REVIEW_BASE_SHA=merge_base) + + self.assertEqual(self._base_modes(), ["incremental"]) + staged = self.stage_dir / "base" + self.assertEqual((staged / ".codeboardingignore").read_text(), "docs/\n") + self.assertFalse((staged / "health" / "health_config.json").exists(), "the merge base has none") + + def test_an_ancestor_saved_by_a_run_on_forked_code_is_never_read(self) -> None: + shas = self._history(3) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}", fork=True)]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1]) + + self._assert_full(values) + self.assertNotIn("/zip", self.gh_log.read_text(), "a fork's bundle was downloaded") + + def test_another_configuration_saved_at_the_merge_base_is_incompatible(self) -> None: + shas = self._history(2) + self._serve([self._artifact(f"codeboarding-base-othercfg-{shas[-1]}")]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1]) + + self._assert_full(values) + + def test_a_shallow_checkout_is_deepened_to_find_the_ancestor(self) -> None: + shas = self._history(4) + bare = self.root / "origin.git" + subprocess.run(["git", "clone", "-q", "--bare", str(self.origin), str(bare)], check=True) + subprocess.run(["git", "-C", str(bare), "config", "uploadpack.allowAnySHA1InWant", "true"], check=True) + checkout = self.root / "shallow" + subprocess.run(["git", "clone", "-q", "--depth=1", f"file://{bare}", str(checkout)], check=True) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + self._analyze(checkout, REVIEW_BASE_SHA=shas[-1]) + + self.assertIn(f"Starting from the saved analysis of {shas[0][:7]}.", self.log) + self.assertEqual(self._base_modes(), ["incremental"]) + + def test_without_the_lookup_nothing_is_listed(self) -> None: + # GHES has no artifact store, so the action turns the lookup off there. + shas = self._history(2) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1], ANCESTOR_LOOKUP="false") + + self.assertEqual(self._base_modes(), ["full"]) + self.assertEqual(self.gh_log.read_text(), "") + + def test_a_first_sync_catches_up_from_the_setup_reviews_base(self) -> None: + # Merging the setup pull request leaves no committed baseline, but its + # preview review saved the base at its merge base, the new tip's parent. + shas = self._history(3) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[1]}")]) + + self._analyze(self.origin, ANALYSIS_KIND="sync") + + self.assertEqual(self._modes(), ["incremental"]) + + class ReviewArtifactTests(unittest.TestCase): """The artifact is the only channel a reader outside the run can use: cache entries have no download API, so whatever the webview needs must ship here.""" @@ -563,7 +997,7 @@ def test_a_fork_publishes_nothing_another_run_would_read(self) -> None: for s in self._uploads() if "outputs.base_name" in s.get("path", "") + s.get("name", "") or "base_name" in s.get("if", "") ] - published_by_review = [s for s in self._uploads() if "review_analyze.outputs.publish_base" in s.get("if", "")] + published_by_review = [s for s in self._uploads() if "review_baseline.outputs.publish_base" in s.get("if", "")] self.assertTrue(published_by_review, "no base publication step found") for step in published_by_review: self.assertIn("is_fork != 'true'", step["if"], f"{step['name']} would let a fork publish a shared base") diff --git a/tests/test_action_sync.py b/tests/test_action_sync.py index b0b87aa..0aab1e2 100644 --- a/tests/test_action_sync.py +++ b/tests/test_action_sync.py @@ -12,7 +12,20 @@ ROOT = Path(__file__).resolve().parent.parent INSTALL_SYNC = ROOT / "scripts" / "action" / "install-sync.sh" RENDER_REVIEW = ROOT / "scripts" / "action" / "render-review.sh" -GUARD = ROOT / "scripts" / "action" / "guard.sh" +GUARD = ROOT / "scripts" / "action" / "resolve-github-event.sh" + + +def write_core(path: Path) -> None: + """The part of Core's constants.py (0.14.5) that names the persisted artifacts.""" + path.mkdir(parents=True, exist_ok=True) + (path / "constants.py").write_text( + "ANALYSIS_FILENAME = 'analysis.json'\n" + "PERSISTED_ANALYSIS_ARTIFACT_FILENAMES = (\n" + " ANALYSIS_FILENAME, 'fingerprint.json', 'static_analysis.pkl', 'static_analysis.sha',\n" + " 'codeboarding_version.json',\n" + ")\n", + encoding="utf-8", + ) class ActionSyncTests(unittest.TestCase): @@ -154,22 +167,14 @@ def test_installs_core_manifest_and_preserves_user_configuration(self) -> None: output = checkout / ".codeboarding" analysis = root / "analysis" fake_core = root / "core" - static_analyzer = fake_core / "static_analyzer" - for directory in (output / "health", analysis, static_analyzer, checkout / "docs" / "development"): + for directory in (output / "health", analysis): directory.mkdir(parents=True) + # The analyzed repository's own constants.py must never be what is read. (checkout / "constants.py").write_text( "raise RuntimeError('imported target constants')\n", encoding="utf-8" ) - (fake_core / "utils.py").write_text( - "ANALYSIS_FILENAME = 'analysis.json'\nFINGERPRINT_FILENAME = 'fingerprint.json'\n", - encoding="utf-8", - ) - (static_analyzer / "__init__.py").touch() - (static_analyzer / "analysis_cache.py").write_text( - "STATIC_ANALYSIS_PKL = 'static_analysis.pkl'\nSTATIC_ANALYSIS_SHA = 'static_analysis.sha'\n", - encoding="utf-8", - ) + write_core(fake_core) produced = ("analysis.json", "fingerprint.json", "static_analysis.pkl") for name in produced: (analysis / name).write_text(f"new {name}\n", encoding="utf-8") @@ -185,16 +190,9 @@ def test_installs_core_manifest_and_preserves_user_configuration(self) -> None: } for path, content in preserved.items(): path.write_text(content, encoding="utf-8") - legacy = ( - output / "overview.md", - output / "health" / "health_report.json", - checkout / "docs" / "development" / "architecture.md", - ) - for path in legacy: - path.write_text( - "old generated content\nhttps://img.shields.io/badge/Generated%20by-CodeBoarding\n", - encoding="utf-8", - ) + # A health report this run did not produce is stale. + stale_report = output / "health" / "health_report.json" + stale_report.write_text("{}\n", encoding="utf-8") github_output = root / "github-output" result = subprocess.run( @@ -220,48 +218,18 @@ def test_installs_core_manifest_and_preserves_user_configuration(self) -> None: self.assertFalse((output / name).exists()) for path, content in preserved.items(): self.assertEqual(path.read_text(encoding="utf-8"), content) - for path in legacy: - self.assertFalse(path.exists()) + self.assertFalse(stale_report.exists()) self.assertEqual(github_output.read_text(encoding="utf-8"), "installed=3\n") - architecture = checkout / "docs" / "development" / "architecture.md" - architecture.write_text("hand-written architecture\n", encoding="utf-8") - result = subprocess.run( - [str(INSTALL_SYNC)], - cwd=checkout, - env={ - "PATH": os.environ["PATH"], - "PYTHONPATH": str(fake_core), - "ACTION_PATH": str(ROOT), - "ANALYSIS_DIR": str(analysis), - "CHECKOUT_DIR": str(checkout), - "GITHUB_OUTPUT": str(github_output), - }, - capture_output=True, - text=True, - check=False, - ) - self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - self.assertEqual(architecture.read_text(encoding="utf-8"), "hand-written architecture\n") - def test_installs_the_health_report_the_engine_produced(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) checkout = root / "checkout" analysis = root / "analysis" fake_core = root / "core" - static_analyzer = fake_core / "static_analyzer" - for directory in (checkout / ".codeboarding", analysis / "health", static_analyzer): + for directory in (checkout / ".codeboarding", analysis / "health"): directory.mkdir(parents=True) - (fake_core / "utils.py").write_text( - "ANALYSIS_FILENAME = 'analysis.json'\nFINGERPRINT_FILENAME = 'fingerprint.json'\n", - encoding="utf-8", - ) - (static_analyzer / "__init__.py").touch() - (static_analyzer / "analysis_cache.py").write_text( - "STATIC_ANALYSIS_PKL = 'static_analysis.pkl'\nSTATIC_ANALYSIS_SHA = 'static_analysis.sha'\n", - encoding="utf-8", - ) + write_core(fake_core) (analysis / "analysis.json").write_text("{}\n", encoding="utf-8") (analysis / "health" / "health_report.json").write_text('{"overall_score": 1.0}', encoding="utf-8") @@ -288,6 +256,53 @@ def test_installs_the_health_report_the_engine_produced(self) -> None: # file that is never printed is silently left out of the commit. self.assertIn(str(installed), result.stdout.splitlines()) + def test_a_missing_analysis_is_logged_and_given_as_the_failure_reason(self) -> None: + # Stdout is the list of installed paths, so an error printed there never reached the log. + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "checkout").mkdir() + (root / "analysis").mkdir() + write_core(root / "core") + output = root / "github-output" + output.write_text("", encoding="utf-8") + + result = subprocess.run( + [str(INSTALL_SYNC)], + env={ + "PATH": os.environ["PATH"], + "PYTHONPATH": str(root / "core"), + "ACTION_PATH": str(ROOT), + "ANALYSIS_DIR": str(root / "analysis"), + "CHECKOUT_DIR": str(root / "checkout"), + "GITHUB_OUTPUT": str(output), + }, + capture_output=True, + text=True, + check=False, + ) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("::error::Core did not produce analysis.json.", result.stderr) + self.assertIn("failure_reason=Core did not produce analysis.json.", output.read_text(encoding="utf-8")) + + def test_a_failed_sync_says_why_in_the_job_summary(self) -> None: + # A push-triggered sync has no pull request to comment on. + with tempfile.TemporaryDirectory() as tmp: + summary = Path(tmp) / "summary" + subprocess.run( + [str(ROOT / "scripts" / "action" / "sync-summary.sh")], + env={ + "PATH": os.environ["PATH"], + "GITHUB_STEP_SUMMARY": str(summary), + "FAILURE_REASON": "codeboarding/baseline already exists and is not a CodeBoarding analysis branch.", + "SAVED_TO": "codeboarding/baseline", + }, + check=True, + ) + text = summary.read_text(encoding="utf-8") + self.assertIn("- **Failed:** codeboarding/baseline already exists", text) + self.assertNotIn("- Analysis:", text, "nothing was analyzed") + def test_empty_review_is_successful(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) @@ -359,16 +374,7 @@ def setUp(self) -> None: (self.analysis / name).write_text(f"fresh {name}\n", encoding="utf-8") self.core = self.root / "core" - (self.core / "static_analyzer").mkdir(parents=True) - (self.core / "utils.py").write_text( - "ANALYSIS_FILENAME = 'analysis.json'\nFINGERPRINT_FILENAME = 'fingerprint.json'\n", - encoding="utf-8", - ) - (self.core / "static_analyzer" / "__init__.py").touch() - (self.core / "static_analyzer" / "analysis_cache.py").write_text( - "STATIC_ANALYSIS_PKL = 'static_analysis.pkl'\nSTATIC_ANALYSIS_SHA = 'static_analysis.sha'\n", - encoding="utf-8", - ) + write_core(self.core) def tearDown(self) -> None: self.temp_dir.cleanup() @@ -390,10 +396,8 @@ def test_it_publishes_the_analysis_under_both_commits(self) -> None: "RUNNER_TEMP": str(self.root), "GITHUB_SERVER_URL": str(self.root), "GITHUB_TOKEN": "unused", - "GH_HOST": "github.com", "REPOSITORY": "owner/repo", - "TARGET_BRANCH": "main", - "SYNC_STRATEGY": "push", + "SYNCED_BRANCH": "main", }, capture_output=True, text=True, diff --git a/tests/test_baseline_branch.py b/tests/test_baseline_branch.py new file mode 100644 index 0000000..eebc855 --- /dev/null +++ b/tests/test_baseline_branch.py @@ -0,0 +1,631 @@ +"""codeboarding_analysis_location: codeboarding_branch saves the analysis to an orphan branch, and reviews read +their base from it.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +from test_action_state import ENGINE_STUB, run_steps +from test_action_sync import write_core + +ROOT = Path(__file__).resolve().parent.parent +DELIVER = ROOT / "scripts" / "action" / "deliver-sync.sh" +# The webview reads the latest diagram from this branch, so the name is part of the contract. +BRANCH = "codeboarding/baseline" + + +def git(cwd: Path, *args: str) -> str: + return subprocess.run( + ["git", "-c", "user.name=T", "-c", "user.email=t@example.com", "-c", "commit.gpgsign=false", *args], + cwd=str(cwd), + capture_output=True, + text=True, + check=True, + ).stdout.strip() + + +class BaselineBranchDeliveryTests(unittest.TestCase): + """deliver-sync.sh against a real local remote.""" + + def setUp(self) -> None: + self.temp_dir = tempfile.TemporaryDirectory() + self.root = Path(self.temp_dir.name) + self.remote = self.root / "owner" / "repo.git" + self.remote.mkdir(parents=True) + git(self.remote, "init", "-q", "--bare", "-b", "main") + self.checkout = self.root / "checkout" + git(self.root, "clone", "-q", str(self.remote), str(self.checkout)) + (self.checkout / "app.py").write_text("print('hi')\n", encoding="utf-8") + self._push_code("initial") + self.analysis = self.root / "analysis" + self.analysis.mkdir() + self._analysis("first") + self.core = self.root / "core" + write_core(self.core) + + def tearDown(self) -> None: + self.temp_dir.cleanup() + + def _push_code(self, message: str) -> str: + (self.checkout / f"{message}.py").write_text("pass\n", encoding="utf-8") + git(self.checkout, "add", "-A") + git(self.checkout, "commit", "-q", "-m", message) + git(self.checkout, "push", "-q", "origin", "HEAD:main") + return git(self.checkout, "rev-parse", "HEAD") + + def _analysis(self, content: str) -> None: + for name in ("analysis.json", "fingerprint.json", "static_analysis.pkl"): + (self.analysis / name).write_text(f"{content} {name}\n", encoding="utf-8") + + def _deliver(self, expect_ok: bool = True) -> tuple[subprocess.CompletedProcess, dict[str, str]]: + output = self.root / "github-output" + output.write_text("", encoding="utf-8") + result = subprocess.run( + [str(DELIVER)], + env={ + "PATH": os.environ["PATH"], + "PYTHONPATH": str(self.core), + "ACTION_PATH": str(ROOT), + "ANALYSIS_DIR": str(self.analysis), + "CHECKOUT_DIR": str(self.checkout), + "GITHUB_OUTPUT": str(output), + "RUNNER_TEMP": str(self.root), + "GITHUB_SERVER_URL": str(self.root), + "GITHUB_TOKEN": "unused", + "GH_HOST": "github.com", + "REPOSITORY": "owner/repo", + "SYNCED_BRANCH": "main", + "BASELINE_BRANCH": BRANCH, + "ENGINE_VERSION": "0.14.5", + "CFG_HASH": "cfg", + }, + capture_output=True, + text=True, + check=False, + ) + if expect_ok: + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + values = dict(line.split("=", 1) for line in output.read_text(encoding="utf-8").splitlines() if "=" in line) + return result, values + + def _branch_log(self) -> list[str]: + return git(self.remote, "log", "--format=%H %P", BRANCH).splitlines() + + def test_the_first_sync_creates_an_orphan_branch_with_its_provenance(self) -> None: + main_before = git(self.remote, "rev-parse", "main") + + _result, values = self._deliver() + + (only,) = self._branch_log() + self.assertEqual(only.split(), [git(self.remote, "rev-parse", BRANCH)], "the branch must have no parent") + self.assertEqual(values["committed"], "true") + self.assertEqual(values["baseline_sha"], main_before, "artifacts are named for the analysed commit") + files = set(git(self.remote, "ls-tree", "-r", "--name-only", BRANCH).splitlines()) + self.assertEqual( + files, + { + ".codeboarding/analysis.json", + ".codeboarding/fingerprint.json", + ".codeboarding/static_analysis.pkl", + ".codeboarding/source.json", + }, + ) + source = json.loads(git(self.remote, "show", f"{BRANCH}:.codeboarding/source.json")) + self.assertEqual(source["schema"], 1) + self.assertEqual(source["synced_branch"], "main") + self.assertEqual(source["source_sha"], main_before) + self.assertEqual(source["engine_version"], "0.14.5") + self.assertEqual(source["config"], "cfg") + self.assertRegex(source["generated_at"], r"^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\dZ$") + message = git(self.remote, "log", "-1", "--format=%B", BRANCH) + self.assertEqual( + message, + f"chore(codeboarding): diagram of main @{main_before[:7]}\n\n" + f"CodeBoarding-Source: {main_before}\nCodeBoarding-Branch: main\nCodeBoarding-Config: cfg", + ) + # The synced branch is never written when the baseline has a branch of its own. + self.assertEqual(git(self.remote, "rev-parse", "main"), main_before) + + def test_the_next_sync_appends_a_fast_forward_commit(self) -> None: + self._deliver() + first = git(self.remote, "rev-parse", BRANCH) + new_main = self._push_code("feature") + self._analysis("second") + + _result, values = self._deliver() + + log = self._branch_log() + self.assertEqual(len(log), 2) + self.assertEqual(log[0].split()[1:], [first]) + message = git(self.remote, "log", "-1", "--format=%B", BRANCH) + self.assertIn(f"CodeBoarding-Source: {new_main}", message) + self.assertIn("CodeBoarding-Branch: main", message, "the branch it keeps the analysis of") + + def test_a_rerun_on_the_same_commit_adds_nothing(self) -> None: + self._deliver() + + _result, values = self._deliver() + + self.assertEqual(values["committed"], "false") + self.assertEqual(len(self._branch_log()), 1) + + def test_a_push_refused_by_a_branch_rule_says_how_to_fix_it(self) -> None: + hook = self.remote / "hooks" / "pre-receive" + hook.write_text( + "#!/bin/sh\nwhile read old new ref; do\n" + f' [ "$ref" != refs/heads/{BRANCH} ] || {{ echo "GH013: Repository rule violations found"; exit 1; }}\n' + "done\n", + encoding="utf-8", + ) + hook.chmod(0o755) + + result, _values = self._deliver(expect_ok=False) + + self.assertNotEqual(result.returncode, 0) + self.assertIn(f"GitHub refused the push to {BRANCH}", result.stdout) + self.assertIn("bypass actor", result.stdout) + + def _check(self, synced: str) -> subprocess.CompletedProcess: + (self.root / "check-output").write_text("", encoding="utf-8") + return subprocess.run( + [str(ROOT / "scripts" / "action" / "check-analysis-branch.sh")], + env={ + "PATH": os.environ["PATH"], + "GITHUB_OUTPUT": str(self.root / "check-output"), + "RUNNER_TEMP": str(self.root), + "GITHUB_SERVER_URL": str(self.root), + "GIT_TOKEN": "unused", + "REPOSITORY": "owner/repo", + "BASELINE_BRANCH": BRANCH, + "SYNCED_BRANCH": synced, + }, + capture_output=True, + text=True, + check=False, + ) + + def test_the_check_before_the_install_lets_the_first_and_the_same_branch_through(self) -> None: + self.assertEqual(self._check("main").returncode, 0, "no analysis branch yet: the first sync creates it") + self._deliver() + result = self._check("main") + self.assertEqual(result.returncode, 0, result.stdout) + + def test_the_check_before_the_install_refuses_a_branch_that_blocks_creating_it(self) -> None: + # Otherwise the full analysis runs and the push fails, blaming a branch rule. + git(self.checkout, "push", "-q", "origin", "main:refs/heads/codeboarding") + + result = self._check("main") + + self.assertNotEqual(result.returncode, 0) + self.assertIn("has a branch named codeboarding, so git cannot create codeboarding/baseline", result.stdout) + + def test_the_check_before_the_install_refuses_a_code_branch_with_the_name(self) -> None: + git(self.checkout, "push", "-q", "origin", f"main:refs/heads/{BRANCH}") + + result = self._check("main") + + self.assertNotEqual(result.returncode, 0) + self.assertIn(f"{BRANCH} already exists and is not a CodeBoarding analysis branch", result.stdout) + reason = (self.root / "check-output").read_text(encoding="utf-8") + self.assertIn(f"failure_reason={BRANCH} already exists", reason, "for the job summary") + + def test_an_old_analysis_left_on_the_synced_branch_is_pointed_out(self) -> None: + # The synced branch is never written from here, so its old copy can only be pointed out. + board = self.checkout / ".codeboarding" + board.mkdir() + (board / "analysis.json").write_text("{}\n", encoding="utf-8") + self._push_code("committed-baseline") + + result, values = self._deliver() + + self.assertEqual(values["committed"], "true") + self.assertIn("main still has an old analysis in .codeboarding/", result.stdout) + self.assertIn("keeping .codeboarding/.codeboardingignore", result.stdout) + + def test_a_deleted_branch_is_recreated_as_a_new_orphan(self) -> None: + self._deliver() + git(self.remote, "branch", "-D", BRANCH) + self._push_code("later") + + self._deliver() + + (only,) = self._branch_log() + self.assertEqual(len(only.split()), 1, "a recreated branch starts a new history") + + def test_an_existing_branch_that_is_not_a_baseline_branch_is_never_written(self) -> None: + # A code branch that happens to have the analysis branch's name: building on it would + # leave it holding nothing but .codeboarding/. + git(self.checkout, "push", "-q", "origin", f"main:refs/heads/{BRANCH}") + before = git(self.remote, "rev-parse", BRANCH) + + result, values = self._deliver(expect_ok=False) + + self.assertNotEqual(result.returncode, 0) + self.assertIn(f"{BRANCH} already exists and is not a CodeBoarding baseline branch", result.stdout) + self.assertIn("codeboarding_analysis_location: in_place", result.stdout) + self.assertEqual(git(self.remote, "rev-parse", BRANCH), before) + self.assertEqual(values.get("committed", "false"), "false") + + def _push_elsewhere(self) -> None: + other = self.root / "other" + git(self.root, "clone", "-q", str(self.remote), str(other)) + (other / "x.py").write_text("pass\n", encoding="utf-8") + git(other, "add", "-A") + git(other, "commit", "-q", "-m", "x") + git(other, "push", "-q", "origin", "HEAD:main") + + def test_a_synced_branch_that_moved_during_analysis_still_saves_it(self) -> None: + # A branch that merges faster than sync runs would otherwise never be saved. + self._deliver() + analyzed = self._push_code("feature") + self._push_elsewhere() + self._analysis("second") + + _result, values = self._deliver() + + self.assertEqual(values["committed"], "true") + self.assertIn(f"CodeBoarding-Source: {analyzed}", git(self.remote, "log", "-1", "--format=%B", BRANCH)) + + def test_an_older_analysis_never_replaces_a_newer_one(self) -> None: + first = git(self.checkout, "rev-parse", "HEAD") + self._push_code("feature") + self._deliver() + before = git(self.remote, "rev-parse", BRANCH) + git(self.checkout, "checkout", "-q", first) + self._analysis("older") + + result, values = self._deliver() + + self.assertEqual(values["committed"], "false") + self.assertIn("already holds the analysis of a newer commit", result.stdout) + self.assertEqual(git(self.remote, "rev-parse", BRANCH), before) + + +class BaselineBranchReadTests(unittest.TestCase): + """Reviews and sync read the branch: commits c0..c4 on main, branch entries for c1 and c3 + made under configuration `cfg`.""" + + def setUp(self) -> None: + self.temp_dir = tempfile.TemporaryDirectory() + self.root = Path(self.temp_dir.name) + work = self.root / "work" + work.mkdir() + git(work, "init", "-q", "-b", "main") + self.shas = [] + for index in range(5): + (work / f"f{index}.py").write_text("pass\n", encoding="utf-8") + git(work, "add", "-A") + git(work, "commit", "-q", "-m", f"c{index}") + self.shas.append(git(work, "rev-parse", "HEAD")) + git(work, "checkout", "-q", "--orphan", BRANCH) + git(work, "rm", "-rq", "--cached", ".") + for path in work.glob("f*.py"): + path.unlink() + board = work / ".codeboarding" + board.mkdir() + for source in (self.shas[1], self.shas[3]): + (board / "analysis.json").write_text( + json.dumps({"metadata": {"depth_cap": 2}, "components": [source]}), encoding="utf-8" + ) + (board / "static_analysis.pkl").write_text("pickle", encoding="utf-8") + (board / "source.json").write_text(json.dumps({"schema": 1, "source_sha": source}), encoding="utf-8") + git(work, "add", "-A") + git( + work, + "commit", + "-q", + "-m", + f"chore(codeboarding): diagram of main @{source[:7]}", + "-m", + f"CodeBoarding-Source: {source}\nCodeBoarding-Config: cfg", + ) + git(work, "checkout", "-q", "main") + bare = self.root / "origin.git" + git(self.root, "clone", "-q", "--bare", str(work), str(bare)) + git(bare, "config", "uploadpack.allowAnySHA1InWant", "true") + git(bare, "config", "uploadpack.allowFilter", "true") + self.checkout = self.root / "checkout" + git(self.root, "clone", "-q", "--depth=1", "--branch", "main", f"file://{bare}", str(self.checkout)) + + self.bin_dir = self.root / "bin" + self.bin_dir.mkdir() + (self.bin_dir / "codeboarding").write_text(ENGINE_STUB, encoding="utf-8") + (self.bin_dir / "codeboarding").chmod(0o755) + self.engine_log = self.root / "engine.log" + self.engine_log.write_text("", encoding="utf-8") + self.runner = self.root / "runner" + self.runner.mkdir() + self.output = self.root / "github-output" + + def tearDown(self) -> None: + self.temp_dir.cleanup() + + def _analyze(self, expect_ok: bool = True, **extra: str) -> dict[str, str]: + code, self.log, values = run_steps( + { + "PATH": f"{self.bin_dir}:{os.environ['PATH']}", + "RUNNER_TEMP": str(self.runner), + "CB_ENGINE_LOG": str(self.engine_log), + "ACTION_PATH": str(ROOT), + "GITHUB_RUN_ID": "1", + "CHECKOUT_DIR": str(self.checkout), + "REVIEW_HEAD_SHA": "head-sha", + "REVIEW_BASE_REPO": "origin", + "REPOSITORY": "origin", + "GITHUB_SERVER_URL": f"file://{self.root}", + "PR_NUMBER": "42", + "ENGINE_VERSION": "0.14.5", + "CFG_HASH": "cfg", + "BASELINE_BRANCH": BRANCH, + "BASELINE_RETRY_DELAY": "0", + "BASE_DIR": str(self.root / "state" / "base"), + "WARMSTART_DIR": str(self.root / "state" / "warmstart"), + "STAGE_DIR": str(self.root / "state" / "out"), + "DEPTH_CAP": "2", + **extra, + }, + self.output, + ) + self.assertEqual(code == 0, expect_ok, self.log) + return values + + def _outputs(self) -> dict[str, str]: + return dict(line.split("=", 1) for line in self.output.read_text().splitlines() if "=" in line) + + def _modes(self) -> list[str]: + return [json.loads(line)["mode"] for line in self.engine_log.read_text().splitlines()] + + def _base_modes(self) -> list[str]: + calls = [json.loads(line) for line in self.engine_log.read_text().splitlines()] + return [c["mode"] for c in calls if c["run_id"].endswith("-base")] + + def test_a_branch_entry_for_the_merge_base_is_reused(self) -> None: + values = self._analyze(REVIEW_BASE_SHA=self.shas[3]) + + self.assertEqual(self._base_modes(), [], "only the head is analyzed") + self.assertEqual(self._modes(), ["incremental"]) + base = json.loads(Path(values["base_analysis_path"]).read_text()) + self.assertEqual(base["components"], [self.shas[3]]) + self.assertFalse(Path(values["base_analysis_path"]).with_name("source.json").exists()) + # No artifact holds it yet, so it is published under the merge base's name. + self.assertEqual(values["publish_base"], "true") + + def test_a_branch_that_cannot_be_read_fails_and_the_review_comment_says_why(self) -> None: + # A token without access looks like this too: it must not read as a missing branch. + self._analyze(False, REVIEW_BASE_SHA=self.shas[4], GITHUB_SERVER_URL=f"file://{self.root}/missing") + + self.assertIn(f"Could not read {BRANCH}", self._outputs().get("failure_reason", "")) + self.assertEqual(self._modes(), []) + + def test_a_single_failed_lookup_is_retried(self) -> None: + real_git = shutil.which("git") + marker = self.root / "blipped" + wrapper = self.bin_dir / "git" + wrapper.write_text( + "#!/bin/sh\n" + f'for a in "$@"; do [ "$a" = ls-remote ] && [ ! -e {marker} ] && touch {marker} && exit 128; done\n' + f'exec {real_git} "$@"\n', + encoding="utf-8", + ) + wrapper.chmod(0o755) + + self._analyze(REVIEW_BASE_SHA=self.shas[3]) + + self.assertTrue(marker.exists(), "the first lookup failed") + self.assertEqual(self._base_modes(), [], "the retry found the entry") + + def test_the_nearest_branch_entry_below_the_merge_base_is_caught_up(self) -> None: + self._analyze(REVIEW_BASE_SHA=self.shas[4]) + + self.assertIn(f"Starting from the analysis of {self.shas[3][:7]} saved on {BRANCH}.", self.log) + self.assertEqual(self._base_modes(), ["incremental"]) + self.assertEqual(self._modes(), ["incremental", "incremental"]) + + def test_an_entry_made_under_another_configuration_is_never_reused(self) -> None: + # Another engine version or model: reusing it as is would diff an old + # configuration's base against a new head. + self._analyze(REVIEW_BASE_SHA=self.shas[3], CFG_HASH="othercfg") + + self.assertEqual(self._modes(), ["full", "incremental"]) + + def test_without_a_configuration_hash_no_entry_is_trusted(self) -> None: + self._analyze(REVIEW_BASE_SHA=self.shas[3], CFG_HASH="") + + self.assertEqual(self._base_modes(), ["full"]) + + def test_without_the_branch_the_base_is_a_full_analysis(self) -> None: + self._analyze(REVIEW_BASE_SHA=self.shas[4], BASELINE_BRANCH="codeboarding/none") + + self.assertIn("codeboarding/none does not exist yet", self.log) + self.assertEqual(self._modes(), ["full", "incremental"]) + + def test_a_branch_that_cannot_be_read_fails_instead_of_analyzing_from_scratch(self) -> None: + # A token without access looks like this too: it must not read as a missing branch. + self._analyze(False, REVIEW_BASE_SHA=self.shas[4], GITHUB_SERVER_URL=f"file://{self.root}/missing") + + self.assertIn(f"Could not read {BRANCH}", self.log) + self.assertEqual(self._modes(), []) + + def test_a_code_branch_named_as_the_analysis_branch_is_said_and_skipped(self) -> None: + # A code branch read as the analysis branch, as a review into another branch could. + self._analyze(REVIEW_BASE_SHA=self.shas[4], BASELINE_BRANCH="main") + + self.assertIn("main is not a CodeBoarding analysis branch", self.log) + self.assertEqual(self._modes(), ["full", "incremental"]) + + def test_the_branch_outranks_a_baseline_committed_at_the_merge_base(self) -> None: + # Its entries are pinned to this configuration; a committed baseline is not, + # so one left behind after switching to the baseline branch must not win. + board = self.checkout / ".codeboarding" + board.mkdir() + (board / "analysis.json").write_text(json.dumps({"metadata": {"depth_cap": 2}, "components": ["stale"]})) + git(self.checkout, "add", "-A") + git(self.checkout, "commit", "-q", "-m", "stale baseline") + merge_base = git(self.checkout, "rev-parse", "HEAD") + git(self.checkout, "push", "-q", "origin", "HEAD:main") + + values = self._analyze(REVIEW_BASE_SHA=merge_base) + + self.assertIn(f"saved on {BRANCH}.", self.log) + base = json.loads(Path(values["base_analysis_path"]).read_text()) + self.assertNotEqual(base["components"], ["stale"]) + + def test_sync_continues_from_the_branch_tip(self) -> None: + self._analyze(ANALYSIS_KIND="sync") + + self.assertEqual(self._modes(), ["incremental"]) + + def test_sync_replaces_the_committed_state_with_the_branch_tip(self) -> None: + # Switching from push: the checkout still holds the old committed baseline. + # Only its user configuration may survive; generated files come from the tip. + board = self.checkout / ".codeboarding" + board.mkdir() + (board / "analysis.json").write_text(json.dumps({"metadata": {"depth_cap": 2}, "old": True})) + (board / "static_analysis.pkl").write_text("old pickle") + (board / "static_analysis.sha").write_text("stale\n") + (board / ".codeboardingignore").write_text("docs/\n") + + values = self._analyze(ANALYSIS_KIND="sync") + + state = Path(values["analysis_dir"]) + self.assertEqual((state / "static_analysis.pkl").read_text(), "pickle", "the tip's engine state") + self.assertFalse((state / "static_analysis.sha").exists(), "a generated file from the old baseline") + self.assertFalse((state / "source.json").exists()) + self.assertEqual((state / ".codeboardingignore").read_text(), "docs/\n") + + def test_sync_does_not_continue_from_a_tip_made_under_another_configuration(self) -> None: + self._analyze(ANALYSIS_KIND="sync", CFG_HASH="othercfg") + + self.assertEqual(self._modes(), ["full"]) + + def test_sync_refuses_to_run_on_the_analysis_branch(self) -> None: + # Committing an analysis onto it, code-branch style, would make it unreadable as one. + analysis_checkout = self.root / "analysis-checkout" + git(self.root, "clone", "-q", "--branch", BRANCH, f"file://{self.root}/origin.git", str(analysis_checkout)) + + self._analyze(False, ANALYSIS_KIND="sync", CHECKOUT_DIR=str(analysis_checkout)) + + self.assertIn("This run is on a CodeBoarding analysis branch", self.log) + self.assertIn("This run is on a CodeBoarding analysis branch", self._outputs().get("failure_reason", "")) + self.assertEqual(self._modes(), []) + + def test_sync_continues_from_the_analysis_the_head_commit_removed(self) -> None: + # The commit that moves a repository to the analysis branch deletes .codeboarding/. + work = self.root / "move" + git(self.root, "clone", "-q", f"file://{self.root}/origin.git", str(work)) + (work / ".codeboarding").mkdir() + (work / ".codeboarding" / "analysis.json").write_text(json.dumps({"metadata": {"depth_cap": 2}})) + git(work, "add", "-A") + git(work, "commit", "-q", "-m", "committed analysis") + git(work, "rm", "-rq", ".codeboarding") + git(work, "commit", "-q", "-m", "move to the analysis branch") + git(work, "push", "-q", "origin", "HEAD:main") + shutil.rmtree(self.checkout) + git( + self.root, + "clone", + "-q", + "--depth=1", + "--branch", + "main", + f"file://{self.root}/origin.git", + str(self.checkout), + ) + + self._analyze(ANALYSIS_KIND="sync", BASELINE_BRANCH="codeboarding/none") + + self.assertEqual(self._modes(), ["incremental"]) + self.assertRegex(self.log, r"Starting from the analysis committed on \w{7}, which \w{7} removed") + + def test_sync_without_the_branch_analyzes_from_scratch(self) -> None: + self._analyze( + ANALYSIS_KIND="sync", + BASELINE_BRANCH="codeboarding/none", + ) + + self.assertEqual(self._modes(), ["full"]) + + +class BaselineBranchGuardTests(unittest.TestCase): + def _guard(self, **extra: str) -> tuple[subprocess.CompletedProcess, str]: + with tempfile.TemporaryDirectory() as tmp: + output = Path(tmp) / "github-output" + output.write_text("", encoding="utf-8") + result = subprocess.run( + [str(ROOT / "scripts" / "action" / "resolve-github-event.sh")], + env={ + "PATH": os.environ["PATH"], + "GITHUB_OUTPUT": str(output), + "MODE": "sync", + "EVENT": "push", + "REF_NAME": "main", + "REF_TYPE": "branch", + "HEAD_AUTHOR_EMAIL": "dev@example.com", + "ANALYSIS_LOCATION": "codeboarding_branch", + "REPOSITORY": "owner/repo", + "DEFAULT_BRANCH": "main", + **extra, + }, + capture_output=True, + text=True, + check=False, + ) + return result, output.read_text(encoding="utf-8") + + def test_sync_saves_to_the_analysis_branch_by_default(self) -> None: + result, values = self._guard() + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn("synced_branch=main\n", values) + self.assertIn(f"analysis_branch={BRANCH}\n", values) + + def test_sync_from_any_branch_but_the_default_fails_before_the_install(self) -> None: + # A manual run or a second push branch would otherwise make another branch's + # analysis the latest diagram. + result, values = self._guard(REF_NAME="feature-x") + self.assertNotEqual(result.returncode, 0) + self.assertIn("default branch, main, and this run is on feature-x", result.stdout) + self.assertIn("failure_reason=Sync keeps the analysis of the default branch", values) + + def test_a_renamed_default_branch_keeps_syncing(self) -> None: + # Entries are keyed by commit, so a master -> main rename just continues. + result, values = self._guard(REF_NAME="trunk", DEFAULT_BRANCH="trunk") + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn(f"analysis_branch={BRANCH}\n", values) + + def test_in_place_syncs_any_branch(self) -> None: + result, values = self._guard(REF_NAME="dev", ANALYSIS_LOCATION="in_place") + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn("synced_branch=dev\n", values) + + def test_in_place_commits_the_analysis_to_the_code_branch_in_both_modes(self) -> None: + with tempfile.TemporaryDirectory() as bin_dir: + gh = Path(bin_dir) / "gh" + gh.write_text("#!/bin/sh\nprintf 'base-sha\\t0\\n'\n", encoding="utf-8") + gh.chmod(0o755) + review = { + "EVENT": "pull_request", + "EVENT_PR_NUMBER": "42", + "PULL_BASE_SHA": "base-sha", + "PULL_HEAD_SHA": "head-sha", + "PULL_BASE_REPO": "owner/repo", + "PULL_HEAD_REPO": "owner/repo", + "PULL_BASE_REF": "develop", + "PATH": f"{bin_dir}:{os.environ['PATH']}", + } + for mode, extra in (("sync", {}), ("review", review)): + with self.subTest(mode=mode): + result, values = self._guard(MODE=mode, ANALYSIS_LOCATION="in_place", **extra) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn("analysis_branch=\n", values, result.stdout) + result, values = self._guard(MODE="review", **review) + self.assertIn(f"analysis_branch={BRANCH}\n", values, "a review reads the analysis branch") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_merge_base_contract.py b/tests/test_merge_base_contract.py index 938de62..67c0f81 100644 --- a/tests/test_merge_base_contract.py +++ b/tests/test_merge_base_contract.py @@ -33,8 +33,9 @@ ROOT = Path(__file__).resolve().parent.parent -GUARD = ROOT / "scripts" / "action" / "guard.sh" -ANALYZE = ROOT / "scripts" / "action" / "analyze.sh" +GUARD = ROOT / "scripts" / "action" / "resolve-github-event.sh" +GENERATE_BASELINE = ROOT / "scripts" / "action" / "review-generate-baseline.sh" +ANALYZE_HEAD = ROOT / "scripts" / "action" / "review-analyze-head.sh" GH_STUB = '''#!/usr/bin/env python3 """Minimal `gh` stand-in answering from a real git repository.""" @@ -218,28 +219,31 @@ def test_review_analyzes_the_merge_base_tree_not_the_base_branch_tip(self) -> No check=True, ) - result = subprocess.run( - [str(ANALYZE)], - env=self._env( - ACTION_PATH=str(ROOT), - ANALYSIS_KIND="review", - CHECKOUT_DIR=str(checkout), - REVIEW_BASE_SHA=self.fork_point, - REVIEW_HEAD_SHA=self.head_sha, - REVIEW_BASE_REPO="owner/repo", - GIT_TOKEN="unused", - GITHUB_SERVER_URL="https://github.com", - PR_NUMBER="7", - SEED_MODE="chain", - CACHE_OUT_DIR=str(Path(self.temp_dir.name) / "cache-out"), - ENGINE_VERSION="test", - CFG_HASH="test", - ), - capture_output=True, - text=True, - check=False, + env = self._env( + ACTION_PATH=str(ROOT), + CHECKOUT_DIR=str(checkout), + REVIEW_BASE_SHA=self.fork_point, + REVIEW_HEAD_SHA=self.head_sha, + REVIEW_BASE_REPO="owner/repo", + GIT_TOKEN="unused", + GITHUB_SERVER_URL="https://github.com", + PR_NUMBER="7", + SEED_MODE="chain", + CACHE_OUT_DIR=str(Path(self.temp_dir.name) / "cache-out"), + ENGINE_VERSION="test", + CFG_HASH="test", ) - self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + # The review runs as two steps: the merge base's analysis, then the head's. + self.output.write_text("", encoding="utf-8") + for script in (GENERATE_BASELINE, ANALYZE_HEAD): + result = subprocess.run( + [str(script)], + env={**env, "BASE_ANALYSIS_PATH": self._outputs().get("base_analysis_path", "")}, + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) runs = [json.loads(line) for line in self.engine_log.read_text(encoding="utf-8").splitlines()] self.assertEqual(len(runs), 2, f"expected a base and a head analysis, got {runs}") diff --git a/tests/test_old_api_migrator.py b/tests/test_old_api_migrator.py new file mode 100644 index 0000000..c64289c --- /dev/null +++ b/tests/test_old_api_migrator.py @@ -0,0 +1,130 @@ +"""Deprecated inputs keep working, translated in one place, and never silently lose to the new ones.""" + +from __future__ import annotations + +import os +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +MIGRATOR = ROOT / "scripts" / "action" / "old_api_migrator.sh" + + +def migrate(**inputs: str) -> tuple[subprocess.CompletedProcess, str]: + """Runs the migrator for a sync of main with action.yml's defaults, overridden by `inputs`.""" + with tempfile.TemporaryDirectory() as tmp: + output = Path(tmp) / "github-output" + output.write_text("", encoding="utf-8") + result = subprocess.run( + [str(MIGRATOR)], + env={ + "PATH": os.environ["PATH"], + "GITHUB_OUTPUT": str(output), + "MODE": "sync", + "REF_NAME": "main", + "ANALYSIS_LOCATION": "codeboarding_branch", + "OLD_TARGET_BRANCH": "", + "OLD_SYNC_STRATEGY": "", + **inputs, + }, + capture_output=True, + text=True, + check=False, + ) + values = dict(line.split("=", 1) for line in output.read_text(encoding="utf-8").splitlines()) + return result, values.get("analysis_location", "") + + +class OldApiMigratorTests(unittest.TestCase): + """Each deprecated input becomes the codeboarding_analysis_location a person would write, or fails.""" + + def test_current_inputs_pass_through_without_warnings(self) -> None: + for location in ("codeboarding_branch", "in_place"): + with self.subTest(location=location): + result, value = migrate(ANALYSIS_LOCATION=location) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertEqual(value, location) + self.assertNotIn("::warning", result.stdout) + + def test_a_location_that_is_not_one_of_the_two_fails(self) -> None: + for location in ("", "main", "codeboarding/baseline"): + with self.subTest(location=location): + result, value = migrate(ANALYSIS_LOCATION=location) + self.assertNotEqual(result.returncode, 0) + self.assertEqual(value, "") + self.assertIn("must be codeboarding_branch or in_place", result.stdout) + + def test_sync_strategy_maps_onto_the_location_in_both_modes(self) -> None: + # Review reads where sync wrote, so a shared with-block keeps both in step. + for mode in ("sync", "review"): + for old, new in (("push", "in_place"), ("pull_request", "codeboarding_branch")): + with self.subTest(mode=mode, old=old): + result, value = migrate(MODE=mode, OLD_SYNC_STRATEGY=old) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertEqual(value, new) + self.assertIn(f"codeboarding_analysis_location: {new}", result.stdout) + + def test_sync_strategy_pull_request_says_to_close_the_rolling_pull_request(self) -> None: + result, _value = migrate(OLD_SYNC_STRATEGY="pull_request") + self.assertIn("codeboarding/sync", result.stdout) + + def test_target_branch_naming_the_branch_the_run_is_on_changes_nothing(self) -> None: + result, value = migrate(OLD_TARGET_BRANCH="main") + self.assertEqual(result.returncode, 0, result.stdout) + self.assertEqual(value, "codeboarding_branch") + self.assertIn("target_branch is deprecated", result.stdout) + + def test_force_full_true_is_ignored_with_a_warning(self) -> None: + # It is released, so a workflow that still sets it must keep running. + result, value = migrate(OLD_FORCE_FULL="true") + self.assertEqual(result.returncode, 0, result.stdout) + self.assertEqual(value, "codeboarding_branch") + self.assertIn("::warning title=CodeBoarding inputs::force_full is no longer supported", result.stdout) + + def test_force_full_false_is_silent(self) -> None: + # Old templates pass `${{ inputs.force_full || false }}` on every push. + result, _value = migrate(OLD_FORCE_FULL="false") + self.assertEqual(result.returncode, 0, result.stdout) + self.assertNotIn("::warning", result.stdout) + + def test_sync_strategy_agreeing_with_the_location_only_warns(self) -> None: + # Halfway through a migration both are set, saying the same thing. + result, value = migrate(OLD_SYNC_STRATEGY="push", ANALYSIS_LOCATION="in_place") + self.assertEqual(result.returncode, 0, result.stdout) + self.assertEqual(value, "in_place") + self.assertIn("sync_strategy is deprecated", result.stdout) + + def test_sync_strategy_disagreeing_with_the_location_fails(self) -> None: + result, value = migrate(OLD_SYNC_STRATEGY="pull_request", ANALYSIS_LOCATION="in_place") + self.assertNotEqual(result.returncode, 0) + self.assertEqual(value, "") + self.assertIn("disagree", result.stdout) + + def test_a_target_branch_elsewhere_points_at_the_push_trigger(self) -> None: + # It must not tell anyone to analyze a branch they did not choose. + result, _value = migrate(REF_NAME="master", OLD_TARGET_BRANCH="develop") + self.assertNotEqual(result.returncode, 0) + self.assertIn("this run is on master. List develop under on: push: branches:", result.stdout) + + def test_what_the_new_input_cannot_express_fails_with_the_way_to_migrate(self) -> None: + for inputs in ( + {"OLD_TARGET_BRANCH": "develop"}, + {"OLD_SYNC_STRATEGY": "branch"}, + {"ANALYSIS_LOCATION": "in_place", "OLD_SYNC_STRATEGY": "pull_request"}, + ): + with self.subTest(inputs=inputs): + result, value = migrate(**inputs) + self.assertNotEqual(result.returncode, 0, result.stdout) + self.assertEqual(value, "") + self.assertIn("#moving-an-existing-setup", result.stdout) + + def test_review_never_fails_on_target_branch(self) -> None: + result, value = migrate(MODE="review", REF_NAME="42/merge", OLD_TARGET_BRANCH="develop") + self.assertEqual(result.returncode, 0, result.stdout) + self.assertEqual(value, "codeboarding_branch") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_progress_comment.py b/tests/test_progress_comment.py new file mode 100644 index 0000000..3ae0bfe --- /dev/null +++ b/tests/test_progress_comment.py @@ -0,0 +1,75 @@ +"""The progress comment while a base is built from scratch: two steps, base then head.""" + +import os +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +UPDATE_PROGRESS = ROOT / "scripts" / "action" / "update-review-progress.sh" + + +class ProgressCommentTests(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 _post(self, step: str, comment_id: str = "77", **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"*) echo "{comment_id}" ;; esac\n', + encoding="utf-8", + ) + gh.chmod(0o755) + subprocess.run( + [str(UPDATE_PROGRESS), step], + env={ + "PATH": f"{self.bin_dir}:{os.environ['PATH']}", + "RUNNER_TEMP": str(self.root), + "PROGRESS_HEADER": "codeboarding-review", + "PR_NUMBER": "9", + "REPOSITORY": "owner/repo", + "BASE_REF": "main", + "REVIEW_BASE_SHA": "a1b2c3d4e5f6", + "GITHUB_RUN_ID": "55", + **extra, + }, + check=True, + capture_output=True, + text=True, + ) + return [c for c in self.calls.read_text().split("\n----\n") if "PATCH" in c] if self.calls.exists() else [] + + def test_the_base_step_says_the_base_is_being_built_and_why(self) -> None: + (patch,) = self._post("base") + self.assertIn("1. ⏳ Building the diagram of `main` @a1b2c3d from scratch\n", patch) + self.assertIn(" `main` has no saved diagram this review can start from, so it builds one first.", patch) + self.assertIn("\n2. Analysing this PR's changes\n", patch) + + def test_the_head_step_marks_the_base_done(self) -> None: + (patch,) = self._post("head") + self.assertIn("1. ✅ Built the diagram of `main` @a1b2c3d from scratch\n", patch) + self.assertIn("2. ⏳ Analysing this PR's changes", patch) + self.assertTrue(patch.rstrip().endswith("")) + + def test_the_comment_is_looked_up_once(self) -> None: + self._post("base") + self._post("head") + lookups = [c for c in self.calls.read_text().split("\n----\n") if "/comments?per_page" in c] + self.assertEqual(len(lookups), 1) + + def test_no_progress_comment_means_no_edit(self) -> None: + self.assertEqual(self._post("base", comment_id=""), []) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_with_auth.py b/tests/test_with_auth.py new file mode 100644 index 0000000..49cfdbc --- /dev/null +++ b/tests/test_with_auth.py @@ -0,0 +1,71 @@ +"""Credentials last exactly as long as the analysis steps that need them.""" + +from __future__ import annotations + +import os +import re +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +WITH_AUTH = ROOT / "scripts" / "action" / "with-auth.sh" +ACTION = (ROOT / "action.yml").read_text(encoding="utf-8") + + +class WithAuthTests(unittest.TestCase): + def setUp(self) -> None: + self.temp_dir = tempfile.TemporaryDirectory() + self.addCleanup(self.temp_dir.cleanup) + self.runner_temp = Path(self.temp_dir.name) + self.auth = self.runner_temp / "codeboarding-auth" + self.auth.mkdir() + (self.auth / "tier").write_text("byok", encoding="utf-8") + (self.auth / "provider-name").write_text("openai", encoding="utf-8") + + def _run(self, **extra: str) -> subprocess.CompletedProcess: + return subprocess.run( + [str(WITH_AUTH), "true"], + env={"PATH": os.environ["PATH"], "RUNNER_TEMP": str(self.runner_temp), **extra}, + capture_output=True, + text=True, + check=False, + ) + + def test_credentials_are_removed_after_the_command(self) -> None: + self.assertEqual(self._run().returncode, 0) + self.assertFalse(self.auth.exists()) + + def test_a_step_that_keeps_them_leaves_them_for_the_next(self) -> None: + self.assertEqual(self._run(KEEP_AUTH="true").returncode, 0) + self.assertTrue((self.auth / "tier").exists()) + # The next step analyzes, then removes them. + self.assertEqual(self._run().returncode, 0, "the second analysis step found no credentials") + self.assertFalse(self.auth.exists()) + + +class ActionWiringTests(unittest.TestCase): + def _steps(self) -> list[str]: + runs = ACTION[ACTION.index("\nruns:\n") :] + return re.split(r"\n - name: ", runs)[1:] + + def test_every_step_but_the_last_of_a_mode_keeps_credentials(self) -> None: + """with-auth.sh consumes credentials; a second analysis step in one run needs them kept.""" + for mode in ("review", "sync"): + 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") + self.assertNotIn("KEEP_AUTH", steps[-1], f"{mode}: the last analysis step must remove them") + + def test_kept_credentials_are_always_removed(self) -> None: + names = [s.splitlines()[0] for s in self._steps()] + cleanup = self._steps()[names.index("Remove analysis credentials")] + self.assertIn("clear-auth.sh", cleanup) + self.assertIn("always()", cleanup) + self.assertGreater(names.index("Remove analysis credentials"), names.index("Analyze pull request")) + + +if __name__ == "__main__": + unittest.main()