Skip to content

Fix auto-skip detection in build-staged.yml#81

Merged
toshi0806 merged 2 commits into
mainfrom
issue-80-fix-auto-skip-detection
May 12, 2026
Merged

Fix auto-skip detection in build-staged.yml#81
toshi0806 merged 2 commits into
mainfrom
issue-80-fix-auto-skip-detection

Conversation

@toshi0806
Copy link
Copy Markdown
Member

@toshi0806 toshi0806 commented May 11, 2026

resolve #80

Summary

build-staged.yml の auto-skip 検出が、PR の差分ではなく「PR 分岐後に base 側に追加された差分」を見ていたため、ほぼ全ての PR で DOCS_ONLY=true と誤判定され、Dockerfile / TOML / yaml の変更すらビルドがスキップされていました。

git diff の比較対象を base から merge commit (= 本 PR がテストする対象そのもの) に変更します。

Root cause

GitHub Actions が pull_request イベントでチェックアウトする synthetic merge commit では:

  • HEAD = merge commit (base と PR head を合成)
  • HEAD^1 = base (main)
  • HEAD^2 = PR head

旧コード git diff HEAD^2 HEADPR head から merge commit への差分、つまり「base 側が PR 分岐後に追加した変更」を返します。main が静止している間は空 (もしくは docs-only) になり、PR がどんな変更をしていても DOCS_ONLY=true に倒れていました。

Fix

git diff HEAD^1 HEAD (base → merge commit) に変更しました。これは「merge を取り込んだ後にレポジトリがどう変わるか」=「PR が base の上に追加する内容」を直接表します。

なぜ HEAD^1 HEAD^2 ではなく HEAD^1 HEAD なのか

初期実装では HEAD^1 HEAD^2 (base → PR head) を採用していましたが、レビュー過程で次のエッジケースが指摘されました:

  • PR 分岐後に main が new.cpp を追加
  • HEAD^1 (current main) には new.cpp あり
  • HEAD^2 (PR head, frozen) には new.cpp なし
  • git diff HEAD^1 HEAD^2new.cpp削除として 検出される
  • docs-only な PR が「.cpp の変更を含む」と誤判定 → 不要なビルド

HEAD^1 HEAD なら merge 結果に base のすべての変更が既に取り込まれているので、上記のノイズが入りません。コミット d060b0e で改善しました。

Changes

  • .github/workflows/build-staged.yml L77: diff の比較対象を HEAD^2 HEADHEAD^1 HEAD に修正
  • 同じ過ちが繰り返されないよう、2 つの失敗モード (HEAD^2..HEAD 旧バグ / HEAD^1..HEAD^2 のエッジケース) を説明するコメントを追加

fetch-depth: 2 のまま動作します (merge commit と 2 つの親はすべてフェッチ済み)。

Verification

CI run (commit d060b0e)

run 2570547320632m8s で success[build-all] タグなしで Lite ビルドが正常起動し、判定ログで以下を確認できました:

Build required due to: .github/workflows/build-staged.yml
Documentation-only change: false

過去の検証 (commit ee59bcc, 初期実装の HEAD^1 HEAD^2)

run 25669561988 が 52m49s で success。base が進んでいない単純ケースでは旧実装でも動作していたが、エッジケース耐性のため d060b0e で更に改善しました。

Test plan

  • CI run の determine-strategy ログで Documentation-only change: false を確認
  • Lite ビルドが正常に走る ([build-all] タグなしで)
  • PR build verification コメントが投稿される

影響範囲

  • 既に merge 済みの過去 PR には影響なし
  • 今後の PR では auto-skip が本来の意図通り動作 (docs-only のみスキップ、それ以外はビルド)
  • CLAUDE.md に記載されている auto-skip 仕様に実装が追従する

The previous diff direction (HEAD^2..HEAD) compared the PR head
against the synthetic merge commit, which only returns changes
that landed on base since the PR branched off — typically empty
or docs-only. As a result, DOCS_ONLY stayed true and every PR
got auto-skipped regardless of what it actually changed.

Use HEAD^1..HEAD^2 (base..PR head) instead, which is the PR's
actual diff. Add a comment explaining why, since the merge-commit
parent semantics are easy to get wrong.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the auto-skip (“docs-only”) detection in the staged container build workflow by correcting how changed files are computed for pull_request runs, aligning the workflow behavior with the intended CI policy (skip only when PR changes are documentation-only).

Changes:

  • Fixes the git diff invocation used to collect changed files for PRs.
  • Adds an inline comment explaining synthetic merge commit parent semantics to prevent regressions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/build-staged.yml Outdated
Comment on lines +77 to +81
# where HEAD^1 is the base (main) and HEAD^2 is the PR head. The PR's own
# changes are the diff from HEAD^1 to HEAD^2, NOT from HEAD^2 to HEAD
# (the latter only reflects changes that landed on base after the PR branched).
if [[ "$EVENT_NAME" == "pull_request" ]]; then
CHANGED_FILES=$(git diff --name-only HEAD^2 HEAD 2>/dev/null || git diff --name-only HEAD^ HEAD 2>/dev/null || echo "")
CHANGED_FILES=$(git diff --name-only HEAD^1 HEAD^2 2>/dev/null || git diff --name-only HEAD^ HEAD 2>/dev/null || echo "")
Per Copilot review: HEAD^1..HEAD^2 (base..PR head) can include
base-side changes since the PR branched, surfacing them as PR-side
deletions and misclassifying a docs-only PR as needing a build.

Use HEAD^1..HEAD (base..merge) instead — this represents exactly
what the PR contributes on top of the current base, and is what
the build will actually be testing. Expand the comment to document
both failure modes (the original HEAD^2..HEAD bug and the
HEAD^1..HEAD^2 edge case) so future maintainers see why we picked
this form.
@toshi0806
Copy link
Copy Markdown
Member Author

Copilot 指摘に対応しました (commit d060b0e)。

対応内容

git diff --name-only HEAD^1 HEAD^2git diff --name-only HEAD^1 HEAD に変更しました。

指摘の妥当性

HEAD^1..HEAD^2 (base → PR head) は、main が PR 分岐後に進んだ場合に base 側の追加ファイルを「PR 側の削除」として 検出してしまいます。例:

  • PR 分岐後に main に new.cpp が追加
  • HEAD^1 (current main) には new.cpp あり
  • HEAD^2 (PR head, frozen) には new.cpp なし
  • git diff HEAD^1 HEAD^2new.cpp が削除として現れる
  • docs-only な PR が「.cpp の変更を含む」と誤判定 → 不要なビルド

HEAD^1..HEAD (base → merge commit) なら merge 結果 = base + PR の変更 が既に統合されているので、その差分は「PR が base の上に追加した内容」だけになり、上記のノイズが入りません。

CI 検証

修正版 (ee59bcc) の CI run 2566956198852m49s で success[build-all] タグなしでビルドが正常起動 (Documentation-only change: false) することを実証済みです。新しいコミット (d060b0e) では振る舞いがより堅牢になっていますが、本 PR 自体は workflow を変更しているため引き続き build トリガーは正しく動作する見込みです。

コメントの拡充

将来の保守者が同じ罠を踏まないよう、2 つの失敗モード (HEAD^2..HEAD 旧バグと HEAD^1..HEAD^2 のエッジケース) をコメントで残しました。

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment on lines +75 to 89
# Detect changed files for auto-skip detection.
# For pull_request events, GitHub Actions checks out a synthetic merge
# commit where HEAD^1 is the base (main) and HEAD^2 is the PR head.
# Diffing HEAD^1 against HEAD (the merge result) gives exactly what this
# PR would introduce on top of the current base; this avoids two failure
# modes of the previous logic:
# - HEAD^2..HEAD returned base-side changes since the PR branched,
# so non-doc PRs were misclassified as docs-only.
# - HEAD^1..HEAD^2 would, if main has advanced since the PR branched,
# surface those base-only changes as PR-side deletions and
# misclassify a docs-only PR as needing a build.
if [[ "$EVENT_NAME" == "pull_request" ]]; then
CHANGED_FILES=$(git diff --name-only HEAD^2 HEAD 2>/dev/null || git diff --name-only HEAD^ HEAD 2>/dev/null || echo "")
CHANGED_FILES=$(git diff --name-only HEAD^1 HEAD 2>/dev/null || git diff --name-only HEAD^ HEAD 2>/dev/null || echo "")
else
CHANGED_FILES=$(git diff --name-only HEAD^ HEAD 2>/dev/null || echo "")
@toshi0806
Copy link
Copy Markdown
Member Author

Copilot 指摘 (description と実装の乖離) に対応しました。

対応内容

PR description を最新実装 (HEAD^1 HEAD) に同期。あわせて、初期 HEAD^1 HEAD^2 案から HEAD^1 HEAD への改善経緯 (base が進んだ場合のエッジケース対策) も明記しました。

なおこの修正は description のみで、ソースコードへの変更はありません。

CI 結果

最新コミット d060b0e の CI run 2570547320632m8s で success[build-all] タグなしで判定が Documentation-only change: false となり Lite ビルドが正常起動することを確認済みです。

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@toshi0806 toshi0806 merged commit 8545922 into main May 12, 2026
15 checks passed
@toshi0806 toshi0806 deleted the issue-80-fix-auto-skip-detection branch May 12, 2026 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build-staged.yml: auto-skip incorrectly classifies non-doc changes as docs-only

2 participants