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
21 changes: 20 additions & 1 deletion .github/workflows/prevent-draft-merge.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
---
# Reusable Workflow: Prevent Draft Branch Merge
#
# This workflow prevents accidental merging of draft branches.
Expand Down Expand Up @@ -44,6 +43,7 @@ jobs:
id: check-draft
env:
HEAD_REF: ${{ github.head_ref }}
BASE_REF: ${{ github.base_ref }}
run: |
set -e

Expand Down Expand Up @@ -73,6 +73,21 @@ jobs:
fi
fi

# Allow draft-to-draft sync PRs (e.g. created by
# sync-next-draft.yml to carry accepted review suggestions
# into the next draft branch). Both head and base must match
# a draft pattern.
echo "::debug::Base branch: $BASE_REF"
if { [[ "$BRANCH_NAME" =~ ^[0-9]+(st|nd|rd|th)-draft$ ]] || \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ [LOW] draft-to-draft 同期PRの許可チェックが、ブランチ名の空チェック(line 77付近)より前に実行されています。BRANCH_NAME が空の場合でも正規表現 ^[0-9]+... はマッチしないため実害はありませんが、ロジックの順序として空チェックを先に行う方が防御的です。

[[ "$BRANCH_NAME" =~ ^abstract-[0-9]+(st|nd|rd|th)$ ]]; } && \
{ [[ "$BASE_REF" =~ ^[0-9]+(st|nd|rd|th)-draft$ ]] || \
[[ "$BASE_REF" =~ ^abstract-[0-9]+(st|nd|rd|th)$ ]]; }; then
{ echo "is_draft=false"; echo "draft_type=sync"; } >> "$GITHUB_OUTPUT"
echo "Draft-to-draft sync PR - merge allowed"
echo "::notice::Head and base are both draft branches - sync PR"
exit 0
fi

# Branch name validation
if [[ -z "$BRANCH_NAME" ]]; then
echo "::error::Branch name is empty"
Expand Down Expand Up @@ -132,13 +147,17 @@ jobs:
HEAD_REF: ${{ github.head_ref }}
IS_FINAL: ${{ steps.check-draft.outputs.is_final_submission }}
FINAL_TAG: ${{ steps.check-draft.outputs.final_tag }}
DRAFT_TYPE: ${{ steps.check-draft.outputs.draft_type }}
run: |
BRANCH_NAME="$HEAD_REF"

if [[ "$IS_FINAL" == "true" ]]; then
echo "Final submission branch - merge allowed"
echo "Branch name: $BRANCH_NAME"
echo "Final submission tag: $FINAL_TAG"
elif [[ "$DRAFT_TYPE" == "sync" ]]; then
echo "Draft-to-draft sync PR - merge allowed"
echo "Branch name: $BRANCH_NAME"
else
echo "Regular branch - merge allowed"
echo "Branch name: $BRANCH_NAME"
Expand Down
211 changes: 211 additions & 0 deletions .github/workflows/sync-next-draft.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
# Reusable Workflow: Sync Next Draft Branch
#
# When commits are pushed to a draft branch (e.g. a student accepts
# review suggestions on its PR), this workflow merges them into the
# following draft branch(es) so students always continue writing on
# top of the reviewed text.
#
# Pushes made with GITHUB_TOKEN do not trigger new workflow runs, so a
# single run walks the whole chain (1st-draft -> 2nd-draft -> ...).
# On merge conflict, it opens a draft-to-draft sync PR (resolvable in
# the browser) and notifies the review PR of the pushed branch.
#
# Use this from other repositories with:
#
# on:
# push:
# branches:
# - '*-draft'
# - 'abstract-*'
# jobs:
# sync:
# permissions:
# contents: write
# pull-requests: write
# uses: smkwlab/.github/.github/workflows/sync-next-draft.yml@v1
#
name: Sync Next Draft Branch

on:
workflow_call:

permissions:
contents: write
pull-requests: write

jobs:
sync-next-branch:
runs-on: ubuntu-latest

# Defensive guard against bot loops (GITHUB_TOKEN pushes do not
# trigger workflows in the first place)
if: github.actor != 'github-actions[bot]'

concurrency:
group: sync-next-draft-${{ github.repository }}
cancel-in-progress: false

steps:
- name: Checkout repository
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚨 [HIGH] actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 のコメントに # v7.0.0 とありますが、actions/checkout の最新安定版は v4 系です。v7.0.0 は存在しないバージョンの可能性があります。コミットハッシュが正しければ動作しますが、コメントのバージョン表記を確認・修正してください。

with:
fetch-depth: 0
token: ${{ github.token }}

- name: Configure Git
run: |
git config --global user.name "github-actions[bot]"
git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com"

- name: Sync pushed changes into following draft branches
env:
GH_TOKEN: ${{ github.token }}
PUSHED_BRANCH: ${{ github.ref_name }}
run: |
set -eu

# Ordinal suffix for a number (1 -> 1st, 2 -> 2nd, ...)
ordinal() {
local n="$1"
local last_two=$((n % 100))
local last_one=$((n % 10))
if [[ $last_two -ge 11 && $last_two -le 13 ]]; then
echo "${n}th"
elif [[ $last_one -eq 1 ]]; then
echo "${n}st"
elif [[ $last_one -eq 2 ]]; then
echo "${n}nd"
elif [[ $last_one -eq 3 ]]; then
echo "${n}rd"
else
echo "${n}th"
fi
}

# Draft branch following the given one (empty if no draft pattern)
next_branch() {
local branch="$1"
if [[ "$branch" =~ ^([0-9]+)(st|nd|rd|th)-draft$ ]]; then
# Also covers the 0th-draft -> 1st-draft special case

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ [LOW] コメントに「Also covers the 0th-draft -> 1st-draft special case」とありますが、0th-draft は英語の序数として不正確(0 に対する序数は通常 0th)であり、ordinal(0+1) = ordinal(1) = 1st となるため 0th-draft1st-draft の変換は正しく動作します。ただし、0th-draft というブランチ名が実際に使用されるかどうかをドキュメントに明記しておくと良いでしょう。現在の README には 0th-draft → 1st-draft の記載がありますが、create-next-draft.yml との整合性確認を推奨します。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

据え置きとします。0th-draft は既存運用で実際に使われるブランチ名で (create-next-draft.yml にも 0th-draft → 1st-draft の特例が実装済み)、ご指摘の通り ordinal(0+1)=1st により本実装でも正しく変換されます。README の記載 (0th-draft → 1st-draft) とも整合しているため、変更は不要と判断しました。

echo "$(ordinal $((BASH_REMATCH[1] + 1)))-draft"
elif [[ "$branch" =~ ^abstract-([0-9]+)(st|nd|rd|th)$ ]]; then
echo "abstract-$(ordinal $((BASH_REMATCH[1] + 1)))"
fi
}

# Merge "from" into "into" and push; retry once on push race.
# Returns 1 on merge conflict, 2 when the push keeps failing.
sync_pair() {
local from="$1" into="$2"
for _ in 1 2; do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ [LOW] リトライループが for _ in 1 2 の2回固定になっています。2回目の push も失敗した場合、ループを抜けて return 2 に到達しますが、ループ内で return 2 に到達するパスがないため、実際には2回目の push 失敗後にループが終了して return 2 が実行されます。これは意図通りですが、コードの意図が少し読みにくいです。

より重要な問題として、sync_pair 内で git checkout -B "$into" "origin/$into" を実行していますが、この関数はメインスクリプトと同じシェルプロセスで実行されます。リトライ時に git fetch で最新状態を取得し直してから checkout -B でリセットする設計は正しいですが、git merge --abort || true の後に continuebreak がなく、そのまま return 1 しているため、ループの2回目には入りません。つまり、コンフリクト時は1回しか試行されません(これは正しい動作ですが、コメントの「retry once on push race」との整合性のため、コンフリクト時は即 return 1 することを明示するとよいでしょう)。

# (Re)start from the freshly fetched remote state; on a
# retry after a rejected push, checkout -B discards the
# previous local merge and redoes it on the new tip
git fetch origin \
"+refs/heads/$from:refs/remotes/origin/$from" \
"+refs/heads/$into:refs/remotes/origin/$into"
git checkout -B "$into" "origin/$into"
if ! git merge --no-edit -m "Merge $from into $into (sync review suggestions)" "origin/$from"; then
git merge --abort || true
return 1
fi
if git push origin "$into"; then
return 0
fi
echo "Push of $into was rejected (concurrent update), retrying..."
done
return 2
}

conflict_pr_body() {
local from="$1" into="$2"
cat <<EOF

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚨 [HIGH] conflict_pr_body 関数内のヒアドキュメントのインデントに注意が必要です。cat <<EOF の後の本文が8スペースインデントされており、PRボディにそのままインデントが含まれてしまいます。cat <<-EOF を使用してタブによるインデント除去を行うか、ヒアドキュメントの内容をインデントしないようにする必要があります(現状ではGitHub PRのMarkdownレンダリングに影響する可能性があります)。

## 前稿の変更を自動で取り込めませんでした

「$from」で受け入れた suggestion(レビューでの修正)を「$into」へ自動 merge しようとしましたが、同じ箇所が両方のブランチで編集されているため、自動では取り込めませんでした。

### 対応手順(ブラウザだけで完結します)

1. この PR の下部にある **Resolve conflicts** ボタンを押す
2. エディタで \`<<<<<<<\` から \`>>>>>>>\` までの箇所を探し、残したい文になるように編集する
3. **Mark as resolved** を押してから **Commit merge** を押す
4. この PR を **Merge pull request** で merge する(この同期 PR は merge してかまいません)

merge が完了すると、以降の suggestion は再び自動で「$into」へ取り込まれます。
EOF
}

# On conflict: open a draft-to-draft sync PR and notify the
# review PR of the pushed branch.
handle_conflict() {
local from="$1" into="$2"
echo "::warning::Merge conflict while syncing $from into $into"

local sync_pr
sync_pr=$(gh pr list --head "$from" --base "$into" --state open \
--json number --jq '.[0].number // empty')

if [ -z "$sync_pr" ]; then
local pr_url
pr_url=$(gh pr create --base "$into" --head "$from" \
--title "Sync review suggestions from $from into $into" \
--body "$(conflict_pr_body "$from" "$into")")
sync_pr="${pr_url##*/}"
fi

# The review PR of "from" is the open PR whose base is not a
# draft branch (draft-based PRs are sync PRs, also mid-chain)
local review_pr
review_pr=$(gh pr list --head "$from" --state open \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [MEDIUM] review_pr を検索する際、--head "$from" で open な PR を検索し、base が $into でないものを選んでいます。しかし、$from ブランチを head とする PR が複数存在する場合(例: 同一ブランチから複数の PR が open)、意図しない PR にコメントする可能性があります。また、$fromPUSHED_BRANCH ではなくチェーン途中のブランチになった場合(コンフリクトが2段目以降で発生した場合)、そのブランチのレビュー PR を正しく特定できるか確認が必要です。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

対応しました (4e1b4ea)。「base が $into でない」ではなく「base が draft パターンに一致しない」PR を選ぶように強化しました。これにより同期 PR (base が draft) はチェーン途中のものも含めて除外され、レビュー PR (base=main 等) だけが対象になります。チェーン途中のブランチでコンフリクトした場合にそのブランチのレビュー PR が存在しないケースは、-n ガードによりコメントをスキップします (同期 PR 自体は作成されるため学生への導線は確保されます)。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ [LOW] handle_conflict 関数内で review_pr を検索する際の jq フィルタが複雑です。test("^[0-9]+(st|nd|rd|th)-draft$|^abstract-[0-9]+(st|nd|rd|th)$") は正規表現として正しいですが、gh pr list --head "$from" は同名 head ブランチを持つ全リポジトリのPRを返す可能性があります。--repo オプションを明示的に指定することを検討してください(GH_TOKENGITHUB_REPOSITORY 環境変数で自動解決されるため実害はないかもしれませんが、明示的な方が安全です)。

--json number,baseRefName \
--jq '[.[] | select(.baseRefName | test("^[0-9]+(st|nd|rd|th)-draft$|^abstract-[0-9]+(st|nd|rd|th)$") | not)][0].number // empty')
if [ -n "$review_pr" ]; then
gh pr comment "$review_pr" --body "受け入れた suggestion を「$into」へ自動 merge できませんでした(コンフリクト)。同期 PR #$sync_pr を開き、「Resolve conflicts」で解決してから merge してください。"
fi

{
echo "## Sync stopped by merge conflict"
echo ""
echo "- $from -> $into: conflict, see sync PR #$sync_pr"
} >> "$GITHUB_STEP_SUMMARY"
}

current="$PUSHED_BRANCH"
synced=""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ [LOW] synced 変数にブランチ名をスペース区切りで追記し、後で for b in $synced でイテレートしています。ブランチ名にスペースが含まれる場合(通常はないですが)に問題が生じます。また、先頭にスペースが付く形(synced="$synced $next")になるため、配列を使う方がより堅牢です:

synced=()
# ...
synced+=("$next")
# ...
for b in "${synced[@]}"; do


while true; do
next="$(next_branch "$current")"
if [ -z "$next" ]; then
echo "::notice::No next draft branch pattern for $current; done"
break
fi
if ! git ls-remote --exit-code --heads origin "$next" > /dev/null; then
echo "::notice::Next draft branch $next does not exist; done"
break
fi

status=0
sync_pair "$current" "$next" || status=$?

if [ "$status" -eq 1 ]; then
handle_conflict "$current" "$next"
exit 0
elif [ "$status" -ne 0 ]; then
echo "::error::Could not push $next (kept failing due to concurrent updates)"
exit 1
fi

echo "Synced $current -> $next"
synced="$synced $next"
current="$next"
done

if [ -n "$synced" ]; then
{
echo "## Synced draft branches"
echo ""
for b in $synced; do
echo "- $b"
done
} >> "$GITHUB_STEP_SUMMARY"
fi
34 changes: 33 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ smkwlab organization の共通設定および Reusable Workflows を管理する
| Workflow | 説明 | 用途 |
|----------|------|------|
| `create-next-draft.yml` | PR 作成時に次の draft ブランチを自動作成 | 卒論・ISE レポート |
| `sync-next-draft.yml` | draft ブランチへの push(suggestion 受け入れ等)を後続の draft ブランチへ自動 merge | 卒論・ISE レポート |
| `prevent-draft-merge.yml` | draft ブランチの誤マージを防止 | 卒論・ISE レポート |
| `auto-final-merge.yml` | final-* タグ push 時に承認済み PR を自動マージ | 卒論テンプレート |
| `ai-review.yml` | ワンショット LLM(Claude/Gemini)による PR 自動レビュー(CODE / ACADEMIC) | 全テンプレート |
Expand Down Expand Up @@ -384,9 +385,40 @@ draft ブランチからの PR 作成時に、次の draft ブランチを自動
- `abstract-1st` → `abstract-2nd` → ...
- `0th-draft` → `1st-draft`

### sync-next-draft.yml

draft ブランチへ push されたコミット(レビュー PR で suggestion を受け入れた場合を含む)を、後続の draft ブランチへ連鎖的に自動 merge します。PR 作成時に次稿ブランチが先行作成されるため、その後に受け入れた suggestion が次稿に反映されない問題(sotsuron-template#110)への対応です。

**動作:**

- `1st-draft` への push → `2nd-draft` が存在すれば merge → `3rd-draft` … と存在する限り伝播(GITHUB_TOKEN による push は新しい workflow run を発火しないため、1 回の実行でチェーン全体を処理)
- コンフリクト時: 前稿→次稿の同期 PR(例: head `1st-draft` / base `2nd-draft`)を自動作成し、前稿のレビュー PR へコメントで通知。学生はブラウザの「Resolve conflicts」で解決して同期 PR を merge する(`prevent-draft-merge.yml` は draft→draft の同期 PR を許可)
- 成功時は PR コメントしない(Actions の step summary にのみ記録)

**caller 例:**

```yaml
name: Sync Suggestions to Next Draft
on:
push:
branches:
- '*-draft'
- 'abstract-*'

jobs:
sync:
permissions:
contents: write
pull-requests: write
uses: smkwlab/.github/.github/workflows/sync-next-draft.yml@v1
```

> [!NOTE]
> `push` トリガーは **push されたブランチ上の**ワークフローファイルを参照します。既存リポジトリへ配布する際は default branch だけでなく、既存の draft ブランチにも caller を配置してください(`registry-manager propagate-workflow` 等で伝播)。

### prevent-draft-merge.yml

draft ブランチの誤マージを防止します。`final-*` タグが付いている場合のみマージを許可。
draft ブランチの誤マージを防止します。`final-*` タグが付いている場合のみマージを許可。また、base も draft ブランチである同期 PR(`sync-next-draft.yml` がコンフリクト時に作成)はマージを許可します。

### auto-final-merge.yml

Expand Down
Loading