diff --git a/README.md b/README.md index 42b6447..89c36be 100644 --- a/README.md +++ b/README.md @@ -59,11 +59,7 @@ Automatic runs update one sticky **CodeBoarding review** comment. A trusted repo Keep the `concurrency` block if you keep `synchronize`: it is what makes a push continue from the push before it, and what stops a slower run for an older commit from overwriting the review comment for a newer one. Set `cancel-in-progress: true` instead to abandon a superseded run rather than queue it, which costs less when branches are pushed to rapidly, at the price of no analysis for the commits in between. -| 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. | +`/codeboarding` analyzes the current head, reusing this pull request's previous analysis when there is one. It takes no arguments. 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. diff --git a/action.yml b/action.yml index 218599d..44eebc1 100644 --- a/action.yml +++ b/action.yml @@ -228,7 +228,7 @@ runs: run: "$GITHUB_ACTION_PATH/scripts/action/fetch-state.sh" - name: Fetch this pull request's last analysis - if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.guard.outputs.seed_mode == 'chain' && steps.state.outputs.warmstart_name != '' && github.server_url == 'https://github.com' + if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.state.outputs.warmstart_name != '' && github.server_url == 'https://github.com' continue-on-error: true shell: bash env: @@ -333,7 +333,6 @@ runs: 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 }} 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 }} diff --git a/docs/COMMIT_STRATEGY.md b/docs/COMMIT_STRATEGY.md index a7913f0..55745ae 100644 --- a/docs/COMMIT_STRATEGY.md +++ b/docs/COMMIT_STRATEGY.md @@ -101,7 +101,7 @@ forking from that commit gets the first row. | Source | Covers | |---|---| | this pull request's warm-start bundle | only the commits pushed since that run | -| nothing to fetch: first run, moved merge base, changed config, `refresh`, or a fork | the whole pull request | +| nothing to fetch: first run, moved merge base, changed config, or a fork | the whole pull request | A restored bundle is used only when it grew from the very base graph this run diffs against, recorded as a digest in `origin.json`. Two runs of the engine over diff --git a/scripts/action/analyze.sh b/scripts/action/analyze.sh index 74ad967..bd5a75b 100755 --- a/scripts/action/analyze.sh +++ b/scripts/action/analyze.sh @@ -142,7 +142,6 @@ fetch_commit() { # lineage are read from the bundle itself, so they are checked here. warmstart_usable() { local base_analysis="$1" bundle_cap base_cap - [ "${SEED_MODE:-chain}" = chain ] || return 1 [ -f "${WARMSTART_DIR:-}/analysis.json" ] || return 1 bundle_cap="$(depth_cap_from "$WARMSTART_DIR/analysis.json")" base_cap="$(depth_cap_from "$base_analysis")" @@ -208,13 +207,9 @@ analyze_review() { fi rm -f "$head_state/origin.json" - if [ "${SEED_MODE:-chain}" = full ]; then + incremental "$CHECKOUT_DIR" "$head_state" + if [ "$REQUIRES_FULL" = true ]; then full "$CHECKOUT_DIR" "$head_state" "$depth" - else - incremental "$CHECKOUT_DIR" "$head_state" - if [ "$REQUIRES_FULL" = true ]; then - full "$CHECKOUT_DIR" "$head_state" "$depth" - fi fi write_origin "$head_state" "$seed_source" "$chain_depth" "$(analysis_digest "$base_analysis")" diff --git a/scripts/action/guard.sh b/scripts/action/guard.sh index 5b48ddc..84d9261 100755 --- a/scripts/action/guard.sh +++ b/scripts/action/guard.sh @@ -40,7 +40,6 @@ if [ "$MODE" = sync ]; then exit 0 fi -seed_mode=chain case "$EVENT" in pull_request|pull_request_target) pr_number="$EVENT_PR_NUMBER" @@ -51,20 +50,13 @@ case "$EVENT" in base_ref="${PULL_BASE_REF:-}" ;; issue_comment) - read -r first_word second_word <<< "$(printf '%s' "$COMMENT_BODY" | tr -d '\r' | awk 'NR == 1 {print $1, $2}')" + first_word="$(printf '%s' "$COMMENT_BODY" | tr -d '\r' | awk 'NR == 1 {print $1}')" [ "$first_word" = /codeboarding ] || skip "Comment is not a /codeboarding command." case "$AUTHOR_ASSOCIATION" in OWNER|MEMBER|COLLABORATOR) ;; *) skip "Only trusted collaborators may run /codeboarding." ;; esac [ -n "$ISSUE_PR_URL" ] || skip "The command was not posted on a pull request." - # refresh ignores the pull request's own cached analysis and re-seeds from - # the base; full additionally forces a from-scratch head analysis. - case "$second_word" in - "") ;; - refresh|full) seed_mode="$second_word" ;; - *) echo "::warning::Unknown /codeboarding argument '$second_word'; running the default incremental review." ;; - esac pr_json="$(gh api "$ISSUE_PR_URL")" pr_number="$(jq -r '.number // empty' <<< "$pr_json")" base_sha="$(jq -r '.base.sha // empty' <<< "$pr_json")" @@ -136,6 +128,5 @@ is_fork=false echo "checkout_repo=$head_repo" echo "checkout_ref=$head_sha" echo "comment_id=$comment_id" - echo "seed_mode=$seed_mode" echo "is_fork=$is_fork" } >> "$GITHUB_OUTPUT" diff --git a/tests/test_action_state.py b/tests/test_action_state.py index 622a445..96ca757 100644 --- a/tests/test_action_state.py +++ b/tests/test_action_state.py @@ -181,7 +181,6 @@ def _analyze(self, **extra: str) -> dict[str, str]: "PR_NUMBER": "42", "ENGINE_VERSION": "0.13.8", "CFG_HASH": "cfg", - "SEED_MODE": "chain", "BASE_DIR": str(self.base_dir), "WARMSTART_DIR": str(self.warmstart_dir), "STAGE_DIR": str(self.stage_dir), @@ -227,15 +226,6 @@ def test_a_published_base_without_a_stored_head_seeds_from_the_base(self) -> Non self.assertEqual(values["chain_depth"], "1") self.assertEqual(len(self._engine_calls()), 1) - def test_refresh_ignores_the_stored_analysis(self) -> None: - _state(self.base_dir) - self._bind(chain_depth=3) - - values = self._analyze(SEED_MODE="refresh") - - self.assertEqual(values["seed_source"], "base") - self.assertEqual(values["chain_depth"], "1") - def test_depth_change_discards_the_stored_analysis(self) -> None: _state(self.base_dir, depth=2) _state(self.warmstart_dir, depth=1, chain_depth=3) @@ -273,17 +263,6 @@ def test_a_stored_analysis_with_no_recorded_base_is_discarded(self) -> None: self.assertEqual(values["seed_source"], "base") - def test_a_forced_full_rebuilds_at_the_configured_cap(self) -> None: - # The baseline stopped short of its cap. Rebuilding at the realized - # depth would ratchet the configured depth down for good. - _state(self.base_dir, depth=1, cap=2) - - self._analyze(SEED_MODE="full") - - calls = self._engine_calls() - self.assertEqual(calls[-1]["mode"], "full") - self.assertEqual(calls[-1]["depth"], "2") - def test_each_bundle_says_what_it_is(self) -> None: # A base bundle is otherwise an analysis.json and nothing else, which # unpacks exactly like a head artifact and would be rendered as one.