Skip to content
Open
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
11 changes: 10 additions & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ outputs:
description: 'Which state the review head analysis grew from: pr-chain or base.'
value: ${{ steps.review_analyze.outputs.seed_source }}
base_source:
description: 'How the review base was obtained: saved, committed, or computed.'
description: 'How the review base was obtained: saved, ancestor, committed, or computed.'
value: ${{ steps.review_analyze.outputs.base_source }}
merge_base_sha:
description: 'Merge base used as the review comparison baseline.'
Expand Down Expand Up @@ -451,6 +451,13 @@ runs:
CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target
STAGE_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/out
FORCE_FULL: ${{ inputs.force_full }}
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 }}
Expand Down Expand Up @@ -539,6 +546,8 @@ runs:
BASE_FETCH_SECONDS: ${{ steps.fetch_base.outputs.seconds }}
WARMSTART_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/warmstart
RENEW_BASE: ${{ steps.fetch_base.outputs.renew }}
# 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 != '' }}
# For rewriting the progress comment while a base is built from scratch.
PROGRESS_HEADER: ${{ steps.guard.outputs.comment_id }}
BASE_REF: ${{ steps.guard.outputs.base_ref }}
Expand Down
24 changes: 19 additions & 5 deletions docs/COMMIT_STRATEGY.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ never fires.
| `base_sha` | string | the base branch tip when the event fired — *not* what was compared against |
| `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 |
| `base_source` | string | how this run obtained the base graph: `saved` (the artifact for the merge base), `committed` (`.codeboarding/` committed at the merge base, caught up), or `computed` (full analysis in this run) |
| `base_reason` | string | only with `computed`: `no_baseline` (nothing to seed from) or `incompatible` (a candidate existed but its depth cap differed, or the engine demanded a full run); empty otherwise |
| `base_source` | string | how this run obtained the base graph: `saved` (the artifact for the merge base), `committed` (`.codeboarding/` committed at the merge base, caught up), `ancestor` (the artifact of an older base-branch commit, caught up), or `computed` (full analysis in this run) |
| `base_reason` | string | only with `computed`: `no_baseline` (nothing to seed from), `incompatible` (a candidate existed but its depth cap or configuration differed, or the engine demanded a full run) or `too_far_behind` (the nearest saved ancestor is beyond the catch-up bound); empty otherwise |
| `base_from_sha` | string | the commit whose saved analysis seeded the base; the merge base for `saved`, empty for `computed` or when it lies beyond the fetched history |
| `catchup_commits` | string | first-parent commits from `base_from_sha` to the merge base that change anything outside `.codeboarding/`; `0` when exact, empty when unknown |
| `base_seconds` | string | wall time spent obtaining the base, the artifact lookup included |
Expand Down Expand Up @@ -109,14 +109,28 @@ them:
|---|---|
| the published `codeboarding-base-<cfg>-<merge_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` |
| no compatible committed baseline either: the nearest `codeboarding-base-<cfg>-<sha>` artifact among the merge base's last 100 first-parent ancestors | one incremental from that commit to the merge base |
| none within 100 commits either | full analysis directly, at the configured `depth_cap` |

A trusted run that computed the base publishes it, so the next pull request
forking from that commit gets the first row. The rows are `base_source` `saved`,
`committed` and `computed` in the review metadata, and the review comment says
forking from that commit gets the first row; that includes a base caught up from
an ancestor. The rows are `base_source` `saved`, `committed`, `ancestor` and
`computed` in the review metadata, and the review comment says
which one ran, with measured times. While a base is computed, the progress
comment says so in two steps, with the elapsed time and the reason.

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. `too_far_behind` is reported only when the walk reached its bound and a
saved analysis is confirmed to be an ancestor beyond it. Sync uses the same lookup when the branch has
no usable committed baseline, 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.

Expand Down
84 changes: 75 additions & 9 deletions scripts/action/analyze.sh
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,23 @@ analyze_sync() {
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"
REQUIRES_FULL=true
if [ "$(printf '%s' "${FORCE_FULL:-false}" | tr '[:upper:]' '[:lower:]')" != true ]; then
if [ "$(depth_cap_from "$state/analysis.json")" = "$DEPTH_CAP" ]; then
incremental "$CHECKOUT_DIR" "$state"
fi
# A branch with no usable committed baseline, such as the first sync after the
# setup pull request merged, catches up from that pull request's saved base.
if [ "$REQUIRES_FULL" = true ] &&
seed_from_ancestor "${REPOSITORY:-}" "$(git -C "$CHECKOUT_DIR" rev-parse HEAD 2>/dev/null || true)" "$state" true "$CHECKOUT_DIR"; then
incremental "$CHECKOUT_DIR" "$state"
[ "$REQUIRES_FULL" = true ] ||
echo "::notice::Caught up from the saved analysis of $ANCESTOR_SHA instead of analyzing from scratch."
fi
fi
unset GIT_TOKEN
if [ "$REQUIRES_FULL" = true ]; then
full "$CHECKOUT_DIR" "$state" "$DEPTH_CAP"
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.
Expand All @@ -140,8 +150,9 @@ analyze_sync() {
}

# How far below the merge base this run looks for the commit a saved analysis
# describes. Past it, a catch-up count is reported as unknown.
CATCHUP_BOUND=100
# describes, and for a saved ancestor to catch up from. Past it, a catch-up count
# is unknown and an ancestor counts as too far behind. It is also the fetch depth.
CATCHUP_BOUND="${CATCHUP_BOUND:-100}"

# A depth above 1 also deepens a commit the shallow checkout already holds.
fetch_commit() {
Expand Down Expand Up @@ -209,6 +220,50 @@ catchup_count() {
echo "$count"
}

# Replaces $3 with the saved analysis of the nearest first-parent ancestor of $2
# (or of $2 itself when $4 is true), keeping the user-authored configuration of
# checkout $5. Sets ANCESTOR_SHA, or when there is none to use, ANCESTOR_REASON:
# too_far_behind or incompatible.
ANCESTOR_SHA="" ANCESTOR_REASON=""
seed_from_ancestor() {
local repository="$1" tip="$2" state="$3" include_tip="$4" config_from="$5" found dest="$RUNNER_TEMP/codeboarding-ancestor"
ANCESTOR_SHA="" ANCESTOR_REASON=""
[ "${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" \
"$ACTION_PATH/scripts/action/find-ancestor-base.sh" || true)"
ANCESTOR_SHA="$(awk -F= '$1 == "ancestor_sha" {print $2; exit}' <<< "$found")"
if [ -z "$ANCESTOR_SHA" ]; then
if grep -qx 'other_cfg_at_tip=true' <<< "$found"; then
ANCESTOR_REASON=incompatible
elif grep -qx 'too_far_behind=true' <<< "$found"; then
ANCESTOR_REASON=too_far_behind
fi
return 1
fi
if [ "$(depth_cap_from "$dest/analysis.json")" != "$DEPTH_CAP" ]; then
ANCESTOR_SHA="" ANCESTOR_REASON=incompatible
return 1
fi
rm -rf "$state"
cp -a "$dest" "$state"
keep_user_config "$config_from" "$state"
}
# 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
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
}

# Rewrites the sticky progress comment while the base is built from scratch. A
# fork's read-only token makes every call fail, which costs nothing.
PROGRESS_PID=""
Expand Down Expand Up @@ -289,9 +344,20 @@ analyze_review() {
elif [ -f "$base_state/analysis.json" ]; then
base_reason=incompatible
fi
# Nothing at the merge base to grow from: catch up from the nearest saved
# ancestor, and publish the result under the merge base's own name below.
if [ "$REQUIRES_FULL" = true ] && seed_from_ancestor "$REVIEW_BASE_REPO" "$REVIEW_BASE_SHA" "$base_state" false "$base_checkout"; then
incremental "$base_checkout" "$base_state"
if [ "$REQUIRES_FULL" = true ]; then
base_reason=incompatible
else
base_source=ancestor base_from_sha="$ANCESTOR_SHA"
catchup_commits="$(catchup_count "$ANCESTOR_SHA" "$REVIEW_BASE_SHA")"
fi
fi
if [ "$REQUIRES_FULL" = true ]; then
base_source=computed
base_reason="${base_reason:-no_baseline}"
base_reason="${base_reason:-${ANCESTOR_REASON:-no_baseline}}"
trap progress_stop EXIT
progress_start "$base_started"
full "$base_checkout" "$base_state" "$DEPTH_CAP"
Expand Down
96 changes: 96 additions & 0 deletions scripts/action/find-ancestor-base.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
#!/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 key=value lines on stdout and everything else on stderr:
# ancestor_sha the commit whose analysis is now in DEST, or empty
# too_far_behind true when a compatible ancestor exists only beyond the bound
# other_cfg_at_tip true when TIP_SHA has a saved analysis under another configuration
#
# 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="" too_far=false other_cfg=false
report() {
printf 'ancestor_sha=%s\ntoo_far_behind=%s\nother_cfg_at_tip=%s\n' "$ancestor" "$too_far" "$other_cfg"
}
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")"
walked="$(grep -c . <<< "$walk" || true)"
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'
if grep "^codeboarding-base-.*-$TIP_SHA\$" <<< "$names" | grep -vq "^$prefix"; then
other_cfg=true
fi
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

if [ -z "$ancestor" ]; then
# Too far behind only when the walk reached the bound, so a short or failed
# deepen is never mistaken for distance, and a saved analysis is confirmed to
# be an ancestor: others may belong to other branches.
if [ "$walked" -ge "$BOUND" ]; then
for candidate in $(grep . <<< "$saved" | head -n 5); do
if [ "$(api "repos/$REPOSITORY/compare/$candidate...$TIP_SHA" --jq .status 2>/dev/null || true)" = ahead ]; then
too_far=true
break
fi
done
fi
exit 0
fi

# 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
Loading
Loading