Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/codeboarding.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ name: CodeBoarding review

on:
pull_request:
# Generate once, when the PR becomes reviewable, not on every push, so we
# don't spend an LLM job per commit. Add `synchronize` to re-run on each
# push, or refresh anytime with /codeboarding. 'closed' only cancels an
# in-flight review (see concurrency), it doesn't start one.
# Generate once, when the PR becomes reviewable. Reusing this PR's previous
# analysis makes per-push runs affordable, so `synchronize` is a reasonable
# addition now; /codeboarding still refreshes on demand. 'closed' only
# cancels an in-flight review (see concurrency), it doesn't start one.
types: [opened, reopened, ready_for_review, closed]
issue_comment:
types: [created]
Expand Down
23 changes: 22 additions & 1 deletion AGENT.md → AGENTS.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# AGENT.md — CodeBoarding-action
# AGENTS.md — CodeBoarding-action

This repo is a GitHub Action with two modes, selected by the `mode` input:

Expand All @@ -15,6 +15,27 @@ pinned to a release in `action.yml`. `scripts/engine_adapter.py` is the CLI
adapter into it (no analysis logic lives there). Engine changes reach users only
when that pin is bumped *and* a new action release ships.

## Protected tests

Some tests encode a behavioural contract that is expensive to rediscover once
lost. They are marked with a `PROTECTED TEST` header naming what they protect.

**No agent, assistant, or automated tool may edit, weaken, skip, rename or
delete a protected test — not even to make a build pass. Only a human may
change one, and only after explicitly saying so in that conversation.** A
request to "fix the failing tests" is not that consent.

When a protected test fails, the behaviour it describes regressed. Fix the code.
If you believe the test itself is wrong, stop and say so, then wait for a human
to decide.

Protected tests:

- `tests/test_merge_base_contract.py` — a review compares a pull request against
its merge base, never against the base branch tip. Comparing against the tip
attributes other people's commits to the pull request and reports them
backwards, as removals.

## Releases

Consumers reference a moving major tag (`uses: CodeBoarding/CodeBoarding-action@v1`),
Expand Down
30 changes: 28 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

One GitHub Action with two modes:

- **`review`** (default) compares a pull request's exact base and head commits, posts an inline Mermaid architecture diff, and uploads the head `analysis.json` as a workflow artifact.
- **`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.

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.
Expand Down Expand Up @@ -44,10 +44,34 @@ jobs:

Automatic runs update one sticky **CodeBoarding review** comment. 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.

The action checks out and analyzes the exact PR head SHA, but compares it with the exact upstream base SHA from the event. It does not commit generated files to either branch.
| Command | What it does |
|---|---|
| `/codeboarding` | Analyzes the current head, reusing this PR's previous analysis when one is available. |
| `/codeboarding refresh` | Ignores that previous analysis and re-derives the head from the merge base. |
| `/codeboarding full` | Forces a from-scratch full analysis of the head. |

The action checks out and analyzes the exact PR head SHA, and compares it with the PR's **merge base** — the commit the branch forked from, which is what GitHub's own "Files changed" tab uses. Commits pushed to the base branch after the fork point are therefore not reported as this PR's changes; the comment notes how far behind the branch is instead. It does not commit generated files to either branch.

Automatic fork runs are skipped because the `pull_request` event does not receive hosted OIDC credentials. A trusted `/codeboarding` command runs the released action code from the base repository and checks the fork's source into a separate analysis directory; it never executes an action definition from the fork with privileged credentials.

The uploaded artifact holds both sides of the comparison, so a reader can reproduce it without resolving the merge base again:

| File | Contents |
|---|---|
| `analysis.json` | the head analysis, at `head_sha` |
| `base_analysis.json` | the analysis it was compared against, at `merge_base_sha` |
| `metadata.json` | both SHAs, `merge_base_resolved`, `seed_source`, `chain_depth`, PR number |

Artifacts are kept 30 days. Reading the default branch's committed baseline instead of `base_analysis.json` would drift from the merge base in exactly the way described above.

### Reused analysis

Each review seeds the head analysis from this pull request's own previous run, so a run only covers the commits pushed since it. With no previous run, it seeds from the merge base's analysis. `sync` mode publishes that entry on the base branch, where every pull request can restore it; an entry a review run computes for itself is scoped to that pull request. Both live in the GitHub Actions cache, and state is re-derived from the merge base whenever the pinned CodeBoarding version, `.codeboardingignore`, the configured analysis depth, or the merge base itself changes. State produced while analyzing a fork is namespaced separately and is never restored by a run on this repository's own code.

Caching is best-effort: a cache miss, an unavailable cache service, or a GitHub Enterprise Server without one falls back to analyzing the merge base directly, exactly as before.

Actions cache entries are scoped to the ref that wrote them. Automatic `pull_request` runs therefore reuse each other's analysis and the shared base entry, while a `/codeboarding` command — which runs on the default branch ref — reuses the base entry but not a chain built by automatic runs, so it costs one base-seeded incremental. The action also accepts `pull_request_target`, which runs on the base branch ref and lets both share one chain; that trigger has its own trade-offs (a PR that adds this workflow will not run it until merged, and the fork gate becomes load-bearing), so `pull_request` remains the recommended default.

## Authentication and providers

With no LLM inputs, the action uses CodeBoarding's hosted OpenRouter tier. It mints short-lived GitHub OIDC credentials per request, so the job needs `id-token: write` and no stored LLM secret.
Expand Down Expand Up @@ -197,6 +221,8 @@ The `/codeboarding` command, comment heading, Mermaid direction (`LR`), hosted w
| `n_changed` | review | Number of changed components. |
| `truncated` | review | Whether the graph was reduced to fit GitHub limits. |
| `review_artifact_url` | review | URL of the uploaded head analysis. |
| `seed_source` | review | `pr-chain` when the head grew from this PR's previous analysis, `base` otherwise. |
| `merge_base_sha` | review | Merge base used as the comparison baseline. |
| `analysis_mode` | sync | `incremental` or `full`. |
| `files_written` | sync | Number of persisted analysis artifacts produced. |
| `committed` | sync | Whether a baseline commit was delivered. |
Expand Down
113 changes: 112 additions & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ outputs:
review_artifact_url:
description: 'URL of the uploaded review analysis artifact.'
value: ${{ steps.upload_review_artifact_dotcom.outputs.artifact-url }}
seed_source:
description: 'Which state the review head analysis grew from: pr-chain or base.'
value: ${{ steps.review_analyze.outputs.seed_source }}
merge_base_sha:
description: 'Merge base used as the review comparison baseline.'
value: ${{ steps.guard.outputs.merge_base_sha }}
analysis_mode:
description: 'Whether sync used incremental or full analysis.'
value: ${{ steps.sync_analyze.outputs.analysis_mode }}
Expand Down Expand Up @@ -102,6 +108,7 @@ runs:
EVENT_PR_NUMBER: ${{ github.event.pull_request.number }}
PULL_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
PULL_BASE_SHA: ${{ github.event.pull_request.base.sha }}
PULL_BASE_REF: ${{ github.event.pull_request.base.ref }}
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 }}
Expand Down Expand Up @@ -166,6 +173,50 @@ runs:
LICENSE_KEY: ${{ inputs.license_key }}
run: "$GITHUB_ACTION_PATH/scripts/action/configure-auth.sh"

- name: Resolve analysis cache keys
id: cache_keys
if: steps.guard.outputs.skip != 'true'
continue-on-error: true
shell: bash
env:
CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target
MERGE_BASE_SHA: ${{ steps.guard.outputs.merge_base_sha }}
PR_NUMBER: ${{ steps.guard.outputs.pr_number }}
HEAD_SHA: ${{ steps.guard.outputs.head_sha }}
HEAD_REPO: ${{ steps.guard.outputs.head_repo }}
IS_FORK: ${{ steps.guard.outputs.is_fork }}
SEED_MODE: ${{ steps.guard.outputs.seed_mode }}
LLM_PROVIDER: ${{ inputs.llm_provider }}
MODEL: ${{ inputs.model }}
AGENT_MODEL_INPUT: ${{ inputs.agent_model }}
PARSING_MODEL_INPUT: ${{ inputs.parsing_model }}
run: "$GITHUB_ACTION_PATH/scripts/action/cache-keys.sh"

# An exact key is this merge base's own analysis; the prefix falls back to
# the newest baseline any run produced, which still warm-starts the catch-up.
- name: Restore base analysis
id: base_cache
if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.cache_keys.outputs.base_key != ''
continue-on-error: true
uses: actions/cache/restore@v4
with:
path: ${{ runner.temp }}/cb-cache/base
key: ${{ steps.cache_keys.outputs.base_key }}
restore-keys: ${{ steps.cache_keys.outputs.base_restore_keys }}

# Looked up by prefix alone, because what a run must continue from is this
# pull request's newest analysis. An exact head-sha key would outrank a
# newer generation saved for the same head, so the entry a forced refresh
# replaced would win and undo the refresh.
- name: Restore pull request analysis
if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.guard.outputs.seed_mode == 'chain' && steps.cache_keys.outputs.chain_key != ''
continue-on-error: true
uses: actions/cache/restore@v4
with:
path: ${{ runner.temp }}/cb-cache/chain
key: ${{ steps.cache_keys.outputs.chain_restore_keys }}
restore-keys: ${{ steps.cache_keys.outputs.chain_restore_keys }}

- name: Analyze baseline
id: sync_analyze
if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync'
Expand All @@ -174,6 +225,7 @@ runs:
ACTION_PATH: ${{ github.action_path }}
ANALYSIS_KIND: sync
CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target
CACHE_OUT_DIR: ${{ runner.temp }}/cb-cache
FORCE_FULL: ${{ inputs.force_full }}
MODEL: ${{ inputs.model }}
AGENT_MODEL_INPUT: ${{ inputs.agent_model }}
Expand All @@ -199,6 +251,26 @@ runs:
REPOSITORY: ${{ github.repository }}
run: "$GITHUB_ACTION_PATH/scripts/action/deliver-sync.sh"

# Keyed by the commit pull requests will branch from, so their merge base
# hits this entry exactly and they skip the base analysis entirely.
- name: Save baseline analysis
if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync' && steps.sync_commit.outputs.baseline_sha != '' && steps.cache_keys.outputs.base_key_prefix != ''
continue-on-error: true
uses: actions/cache/save@v4
with:
path: ${{ runner.temp }}/cb-cache/base
key: ${{ steps.cache_keys.outputs.base_key_prefix }}${{ steps.sync_commit.outputs.baseline_sha }}

# The same state under the commit that was analyzed. A pull request branched
# before this run's baseline commit landed has that commit as its merge base.
- name: Save baseline analysis for the analyzed commit
if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync' && steps.sync_commit.outputs.analyzed_sha != '' && steps.sync_commit.outputs.analyzed_sha != steps.sync_commit.outputs.baseline_sha && steps.cache_keys.outputs.base_key_prefix != ''
continue-on-error: true
uses: actions/cache/save@v4
with:
path: ${{ runner.temp }}/cb-cache/base
key: ${{ steps.cache_keys.outputs.base_key_prefix }}${{ steps.sync_commit.outputs.analyzed_sha }}

- name: Write sync summary
if: always() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync'
shell: bash
Expand All @@ -218,16 +290,44 @@ runs:
ACTION_PATH: ${{ github.action_path }}
ANALYSIS_KIND: review
CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target
REVIEW_BASE_SHA: ${{ steps.guard.outputs.base_sha }}
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 }}
SEED_MODE: ${{ steps.guard.outputs.seed_mode }}
CACHE_BASE_DIR: ${{ runner.temp }}/cb-cache/base
CACHE_BASE_HIT: ${{ steps.base_cache.outputs.cache-hit }}
CACHE_CHAIN_DIR: ${{ runner.temp }}/cb-cache/chain
CACHE_OUT_DIR: ${{ runner.temp }}/cb-cache
ENGINE_VERSION: ${{ steps.cache_keys.outputs.engine_version }}
CFG_HASH: ${{ steps.cache_keys.outputs.cfg_hash }}
GIT_TOKEN: ${{ inputs.github_token }}
GITHUB_SERVER_URL: ${{ github.server_url }}
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"'

# Saved before the review is rendered: the analysis is the expensive part, so
# a later rendering or posting failure must not throw it away.
- name: Save pull request analysis
if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.cache_keys.outputs.chain_key != ''
continue-on-error: true
uses: actions/cache/save@v4
with:
path: ${{ runner.temp }}/cb-cache/chain
key: ${{ steps.cache_keys.outputs.chain_key }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Version refreshed analyses with a new cache key

When /codeboarding refresh or /codeboarding full runs for a head SHA that already has a chain cache, it deliberately recomputes the analysis but then saves under the same key as the existing entry. Actions caches are immutable, so this save cannot replace the old chain; the next run after another push restores the pre-refresh analysis rather than the PR's actual last analysis. Give forced refreshes a distinct generation key, or otherwise ensure subsequent prefix restores select the newly computed state.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 6771689. Cache entries being immutable meant the save was dropped and the next run restored precisely the state the refresh was asked to discard.

/codeboarding refresh and /codeboarding full now save under <prefix><head_sha>-<mode><run_id>.<attempt>. The prefix is unchanged, so a later run still finds it, and since prefix restores select the most recently created entry, the refreshed analysis wins over the one it replaced. Covered by test_a_forced_refresh_does_not_save_under_the_key_it_replaces.


# Only trusted runs publish a base entry: it is restorable repository-wide,
# so a fork's run must never be able to place state there.
- name: Save base analysis
if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_analyze.outputs.save_base == 'true' && steps.guard.outputs.is_fork != 'true' && steps.cache_keys.outputs.base_key != ''
continue-on-error: true
uses: actions/cache/save@v4
with:
path: ${{ runner.temp }}/cb-cache/base
key: ${{ steps.cache_keys.outputs.base_key }}
Comment thread
Svilen-Stefanov marked this conversation as resolved.

- name: Render review diagram
id: review_render
if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review'
Expand All @@ -244,10 +344,15 @@ runs:
shell: bash
env:
ANALYSIS_PATH: ${{ steps.review_analyze.outputs.analysis_path }}
BASE_ANALYSIS_PATH: ${{ steps.review_analyze.outputs.base_analysis_path }}
ANALYSIS_MODE: ${{ steps.review_analyze.outputs.analysis_mode }}
BASE_SHA: ${{ steps.guard.outputs.base_sha }}
MERGE_BASE_SHA: ${{ steps.guard.outputs.merge_base_sha }}
MERGE_BASE_RESOLVED: ${{ steps.guard.outputs.merge_base_resolved }}
HEAD_SHA: ${{ steps.guard.outputs.head_sha }}
PR_NUMBER: ${{ steps.guard.outputs.pr_number }}
SEED_SOURCE: ${{ steps.review_analyze.outputs.seed_source }}
CHAIN_DEPTH: ${{ steps.review_analyze.outputs.chain_depth }}
run: "$GITHUB_ACTION_PATH/scripts/action/build-review-artifact.sh"

- name: Upload review artifact
Expand All @@ -258,6 +363,9 @@ runs:
name: codeboarding-review-${{ github.run_id }}-${{ github.run_attempt }}
path: ${{ steps.review_artifact.outputs.artifact_dir }}
if-no-files-found: error
# Each run carries its own copy of the base graph, and the webview only
# ever reads a pull request's latest artifact, so value drops off fast.
retention-days: 30

- name: Build review comment
id: review_body
Expand All @@ -268,6 +376,9 @@ runs:
N_CHANGED: ${{ steps.review_render.outputs.n_changed }}
ARTIFACT_URL: ${{ steps.upload_review_artifact_dotcom.outputs.artifact-url }}
PR_NUMBER: ${{ steps.guard.outputs.pr_number }}
BEHIND_BY: ${{ steps.guard.outputs.behind_by }}
BASE_REF: ${{ steps.guard.outputs.base_ref }}
MERGE_BASE_RESOLVED: ${{ steps.guard.outputs.merge_base_resolved }}
run: "$GITHUB_ACTION_PATH/scripts/action/build-review-comment.sh"

- name: Post review comment
Expand Down
Loading
Loading