Skip to content

fix(ci): ワークフロー検査を配布テンプレートまで広げ PR で強制する - #1071

Merged
keito4 merged 2 commits into
mainfrom
fix/1070-workflow-guards-scope-and-ci
Aug 4, 2026
Merged

fix(ci): ワークフロー検査を配布テンプレートまで広げ PR で強制する#1071
keito4 merged 2 commits into
mainfrom
fix/1070-workflow-guards-scope-and-ci

Conversation

@keito4

@keito4 keito4 commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Closes #1070

Why

PR #1068 で4つのワークフロー検査を追加したが、「今後も起きない」と言い切れない穴が2つ残っていた。

1. 走査対象が .github/workflows/ だけだった

config は templates/workflows/ を下流リポジトリへ配布するが、検査はそこを見ていなかった。

PR #1068 で修正した templates/workflows/claude-health-check.ymlgh バグを、同じ PR で追加した検査が検出できない状態だった。修正前のテンプレートを置いた実測:

--- templates/workflows/claude-health-check.yml(GH_REPO なし)---
✓ gh repository context ok
exit=0

2. CI で強制されていなかった

検査は scheduled-maintenance.yml(週次 TAKT 実行)からしか呼ばれず、退行を含む PR は緑のままマージできた。今回の dependabot-auto が繰り返し失敗し続けていたのと同じ構図。

What

対応 内容
走査範囲 actionlint と同じ3ディレクトリを走査する workflow_files() を共有し、4検査すべてを揃える
CI 強制 ci.yml の Workflow Lint ジョブで4検査を実行

paths-filterworkflows は既に templates/workflows/** を含むため、テンプレートのみの変更でも検査が起動する。Workflow Lint は quality-gateneeds に含まれるため、失敗がマージを止める。

最初の失敗で打ち切らず全違反を報告してから落ちる形にした(1件直すたびに次が出る往復を避けるため)。

How to test

走査範囲の拡大(修正前のテンプレートを検出できるか):

# 変更前
✓ gh repository context ok                                                    exit=0
# 変更後
⚠ claude-health-check.yml: gh has no repository to resolve without a checkout  exit=1

CI 強制(退行を注入して CI ステップを再現):

✓ Claude Actions credential precedence ok
✓ Workflow self-cancellation guards ok
⚠ claude-health-check.yml: gh has no repository to resolve without a checkout
✓ Artifact retention settings ok
guard exit=1

既存テンプレートへの影響: 現行の templates/workflows/ 9ファイルに対して4検査とも exit=0。範囲拡大で新たな違反は出ない。

  • Unit (Jest): 931 tests, 0 failures(新規 5 件の Red → Green 確認済み)
  • Integration (BATS): 310 tests, 0 failures
  • npm run workflow:sync:check / actionlint / lint / format / shellcheck: すべて pass

レビュー対応(54132bd)

Codex が偽陽性を2件指摘した。検査が CI をブロックするようになった以上、偽陽性は正当な PR を止めるため修正した。

指摘 内容
gh-R / --repo= 形式を拒否 gh-R, --repo [HOST/]OWNER/REPO を受け付ける。--repo + 空白しか認めておらず、正しく書かれた checkout 無しジョブが Workflow Lint を落としていた
concurrency groupsha / run_id を含む場合も自己キャンセル扱い group に github.sha / github.run_id が入ると、自分が起こす push は別グループに属するため自分自身をキャンセルできない。この構成を拒否すると正当な publish ワークフローが通らない

偽陽性を消したうえで、実バグ4件を引き続き検出できることを再実証した(config の資格情報・gh context・配布テンプレート、intent-gate-android の自己キャンセル、いずれも修正前の実ファイルで exit=1)。

あわせてテストが file-length ゲート(500行)に達したため、共有ヘルパーを test/helpers/workflow-guards.js へ切り出し、gh 関連の検査を test/repo-maintenance-gh-guards.test.js へ分けた。ゲートの回避(.filelengthignore への追加)はしていない。

Risk

  • 検査が CI をブロックするようになる。現行のワークフローとテンプレートはすべて pass 済みなので、既存の PR を止めることはない。
  • Workflow Lint ジョブは needs.changes.outputs.workflows == 'true' が条件。検査スクリプト自体(script/lib/repo_maintenance_checks.sh)のみを変更する PR ではこのジョブは走らないが、検査の挙動は Jest 側の 30 件で固定されており常時実行される。
  • workflow_files()find ベースのため、ワークフローが存在しないリポジトリでも空を返して正常終了する(下流リポジトリでの repo-maintenance 実行を壊さない)。

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests

    • Added contract and integration tests verifying workflow guard validation in CI.
  • Chores

    • Integrated repository maintenance checks into CI workflow validation.
    • Extended workflow scanning to include reusable and distributed template directories.

検査を追加しただけでは「今後も起きない」と言えない穴が2つあった。

1. 走査対象が .github/workflows/ だけだった
   config は templates/workflows/ を下流リポジトリへ配布するが、そこを見て
   いなかった。実際、直前に修正した templates/workflows/claude-health-check.yml
   の gh バグを自分の検査が検出できない状態だった。
   actionlint と同じ3ディレクトリを走査する workflow_files() を共有し、
   4検査すべてをそこへ揃える。

2. 週次メンテナンスの警告のみで CI が強制していなかった
   退行を含む PR が緑のままマージでき、気付くのは次の定期実行時だった。
   ci.yml の Workflow Lint ジョブで4検査を実行する。同ジョブは quality-gate
   の needs に含まれるため失敗がマージを止める。最初の失敗で打ち切らず
   全違反を報告してから落ちる。

paths-filter の workflows は templates/workflows/** を含むため、テンプレート
のみの変更でも検査が起動する。退行を注入して CI ステップが exit 1 になること、
既存テンプレートに違反が出ないことを実測済み。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 4, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change expands workflow guard scanning to repository and template directories. The CI Workflow Lint job now runs all four guards and reports failures through the quality gate. Tests cover distributed templates, reusable templates, and aggregate failure handling.

Changes

Workflow guard coverage

Layer / File(s) Summary
Shared workflow discovery and guard coverage
script/lib/repo_maintenance_checks.sh, test/repo-maintenance-workflow-guards.test.js
workflow_files() collects YAML workflows from three directories. All four guards scan the shared list. Tests cover distributed and reusable workflow templates.
CI Workflow Lint enforcement
.github/workflows/ci.yml, test/claude-workflow-contract.test.js, .claude/commands/repo-maintenance.md, script/README.md
The Workflow Lint job runs all four guards, aggregates violations, and participates in the quality gate. Documentation describes the scan locations and CI enforcement.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WorkflowLint
  participant repo_maintenance_checks
  participant workflow_templates
  participant quality_gate
  WorkflowLint->>repo_maintenance_checks: run four workflow guards
  repo_maintenance_checks->>workflow_templates: scan repository and template workflows
  workflow_templates-->>repo_maintenance_checks: return violations and status
  repo_maintenance_checks-->>WorkflowLint: report accumulated status
  WorkflowLint->>quality_gate: pass or fail the quality gate
Loading

Possibly related PRs

  • keito4/config#1068: Introduced the workflow guards that this PR extends to template directories and CI.
  • keito4/config#869: Shares repository-maintenance workflow guard logic and template validation.
  • keito4/config#603: Shares workflow discovery across repository and template directories.

Suggested labels: bug

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address all objectives in issue #1070, including three-directory scanning, CI enforcement, aggregated failures, and regression coverage.
Out of Scope Changes check ✅ Passed The documentation and test updates directly support the workflow guard expansion and CI enforcement objectives.
Title check ✅ Passed タイトルは、ワークフロー検査の対象拡大とCIでの強制という主要変更を簡潔に示しています。
Description check ✅ Passed 説明は変更理由、対応内容、テスト結果、リスク、関連Issueを具体的に記載していますが、SummaryとChecklistは省略されています。
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1070-workflow-guards-scope-and-ci

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@keito4 keito4 self-assigned this Aug 4, 2026
@claude

claude Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @keito4's task in 4m 40s —— View job


レビュー結果

このPRは #1068 の穴(走査範囲がテンプレート未対応/CIで未強制)を的確に埋めています。workflow_files() の共有化、4検査すべての while read 化、ci.yml の Workflow Lint ジョブへの組み込みは実測(テンプレート検出前後の比較)付きで説明されており、テストも新規5件がRed→Greenで確認済みとのことで、実装の方向性は妥当です。


🟡 major: リスク説明の「Jestが常時実行される」は現在のCI設定では成立していない

PR本文の Risk 節にこうあります:

Workflow Lint ジョブは needs.changes.outputs.workflows == 'true' が条件。検査スクリプト自体(script/lib/repo_maintenance_checks.sh)のみを変更する PR ではこのジョブは走らないが、検査の挙動は Jest 側の 30 件で固定されており常時実行される

.github/workflows/ci.yml を確認したところ、これは成立しません。

  • changes ジョブの paths-filter(ci.yml:82-94)で script/**scripts 出力のみに一致し、codeworkflows には一致しません。
  • test(Unit Tests / Jest)ジョブの起動条件(ci.yml:127)は needs.changes.outputs.code == 'true' || needs.changes.outputs.dependencies == 'true' || needs.changes.outputs.workflows == 'true' で、scripts を含みません。
  • quality-gate ジョブ(ci.yml:330-333)は skipped を成功扱いします。

つまり script/lib/repo_maintenance_checks.sh だけを変更し、対応する test/*.test.js を触らない PR では、Jest(この30件を含む)はCI上で実行されず、quality-gate は緑のままマージできます。これは本PRが解決しようとしている「退行がCIをすり抜ける」問題そのもので、今回の変更で追加された新規5件を含む安全網が将来の変更で機能しない可能性があります。

対応案(どちらか):

  1. test ジョブの ifneeds.changes.outputs.scripts == 'true' を追加する
  2. あるいはPR本文の当該記述を「Jestは scripts-only 変更ではCIで実行されない」と訂正する

Fix this →


🟢 minor: 3ディレクトリ横断後、警告メッセージが basename だけだと同名ファイルを区別できない

script/lib/repo_maintenance_checks.sh:130,160,199,218output::warning "$(basename "$workflow"): ..." は、走査範囲が .github/workflows/ 単体だった頃は一意でしたが、workflow_files() により .github/workflows/.github/workflows/templates/templates/workflows/ を横断するようになった現在、claude.yml / label-sync.yml / scheduled-maintenance.yml / quality-gate-fallback.yml / dependabot-auto-merge.yml は実際に両方のディレクトリに同名で存在します(確認済み)。どちらのファイルに問題があるか、警告メッセージからは判別できません。

basename の代わりに相対パス($workflow そのもの)を出力に使うと、「1件直すたびに次が出る往復を避ける」という本PRの狙いにより合致すると思います。


🟢 minor: workflow_files()ci.yml の Collect workflow files ステップでロジックが重複

script/lib/repo_maintenance_checks.sh:63-69workflow_files().github/workflows/ci.yml:225-234(actionlint用の収集ステップ)は同じ3ディレクトリ・同じ find パターンをほぼそのまま重複させています。実害はありませんが(YAML側は GITHUB_OUTPUT 用に整形が要るため単純共有は難しい)、将来どちらか一方だけ変更してスキャン範囲がズレる可能性はあります。対応必須ではありません。


その他確認した点(問題なし)

  • ループを glob (for workflow in .github/workflows/*.yml ...) から while IFS= read -r workflow; do ... done < <(workflow_files) に変えた点は、スペースを含むファイル名にも安全で、find -type f の結果のみを扱うため [[ -f ]][[ -n ]] への変更も妥当です。
  • 新しい ci.ymlRun workflow guards ステップ(ci.yml:252-262)は同じ actionlint ジョブ内で actions/checkout 済み(line 223)なので相対パスの find は問題なく動きます。4検査を||で短絡させず全部回してから exit "$guard_status" する実装は、PR本文の「全違反を報告してから落ちる」と一致しています。
  • quality-gateneedsactionlint が含まれているため、Workflow Lint失敗がマージをブロックする導線も確認できました。
  • test/repo-maintenance-workflow-guards.test.js の一時ディレクトリは .context/ 配下(CLAUDE.mdの規約通り)に作られており、finally で確実に削除されています。
  • test/claude-workflow-contract.test.js の新規テストは ci.yml の文字列一致に依存する契約テストですが、実ファイルと完全に一致しており、CI強制の退行防止として機能します。
  • ドキュメント(script/README.md, .claude/commands/repo-maintenance.md)の追記は実装と整合しています。

総じて実装は堅実で、致命的なバグは見つかりませんでした。最初の指摘(Jestの安全網が本当に常時実行されるか)は本PRのリスク評価の前提に関わるため確認をお願いします。

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
test/repo-maintenance-workflow-guards.test.js (1)

381-464: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add template coverage for --check-artifact-retention.

This block covers three of the four workflow guards. It does not verify artifact retention scanning in either template directory. Add a template fixture with actions/upload-artifact and missing or invalid retention-days, then assert a nonzero status. This protects the changed loop at Lines 216-253.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/repo-maintenance-workflow-guards.test.js` around lines 381 - 464, Add a
new test case within the workflow guards describe block that verifies the
--check-artifact-retention guard works on distributed templates. Create a
workflow fixture that uses the actions/upload-artifact action without a
retention-days parameter or with an invalid retention-days value, then call
runCheck with the --check-artifact-retention flag pointing to the fixture in
either templates/workflows/ or .github/workflows/templates/, and assert that the
result.status is not zero. This mirrors the pattern of the existing test cases
in this describe block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/repo-maintenance-workflow-guards.test.js`:
- Around line 381-464: Add a new test case within the workflow guards describe
block that verifies the --check-artifact-retention guard works on distributed
templates. Create a workflow fixture that uses the actions/upload-artifact
action without a retention-days parameter or with an invalid retention-days
value, then call runCheck with the --check-artifact-retention flag pointing to
the fixture in either templates/workflows/ or .github/workflows/templates/, and
assert that the result.status is not zero. This mirrors the pattern of the
existing test cases in this describe block.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7e47e4c8-ef9b-49e9-b84b-c69e75bf3004

📥 Commits

Reviewing files that changed from the base of the PR and between e392838 and 6eeb656.

📒 Files selected for processing (6)
  • .claude/commands/repo-maintenance.md
  • .github/workflows/ci.yml
  • script/README.md
  • script/lib/repo_maintenance_checks.sh
  • test/claude-workflow-contract.test.js
  • test/repo-maintenance-workflow-guards.test.js

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6eeb6567fb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/ci.yml
for check in \
--check-claude-action-credentials \
--check-self-cancelling-workflows \
--check-gh-repo-context \

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 Honor the documented -R flag before enforcing this guard

For a checkout-less job that correctly runs gh issue create -R "$GITHUB_REPOSITORY" ..., this newly blocking check fails the Workflow Lint job: gh issue create --help documents -R, --repo [HOST/]OWNER/REPO, but check_gh_repo_context accepts only a literal --repo followed by whitespace. Thus a valid workflow or distributed template using the documented short form cannot pass CI; recognize -R (and the accepted --repo=... form) before making this guard mandatory.

Useful? React with 👍 / 👎.

Comment thread .github/workflows/ci.yml
guard_status=0
for check in \
--check-claude-action-credentials \
--check-self-cancelling-workflows \

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 Check the concurrency group before blocking release workflows

For a push-triggered publishing workflow whose concurrency group includes ${{ github.sha }} or ${{ github.run_id }}, a push created by the workflow belongs to a different group and therefore cannot cancel the current run. Nevertheless, check_self_cancelling_workflows considers only the literal cancel-in-progress: true, the push trigger, and a publishing command, so this newly mandatory invocation rejects that valid configuration and fails every workflow-related PR containing it. Inspect the group expression before treating this warning-oriented heuristic as a CI failure.

Useful? React with 👍 / 👎.

検査が CI をブロックするようになったため、偽陽性は正当な PR を止める。
Codex の指摘2件を修正した。

1. gh の -R / --repo= 形式を拒否していた
   gh は -R, --repo [HOST/]OWNER/REPO を受け付ける。--repo に空白が続く
   形式しか認めていなかったため、正しく書かれた checkout 無しジョブが
   Workflow Lint を落としていた。

2. concurrency group が sha / run_id を含む場合も自己キャンセル扱いしていた
   group に github.sha や github.run_id が入っていると、自分が起こす push は
   別グループに属するため自分自身をキャンセルできない。この構成を拒否すると
   正当な publish ワークフローが通らない。

偽陽性を消したうえで、実バグ4件(config の資格情報・gh context・配布テンプレート、
intent-gate-android の自己キャンセル)を引き続き検出できることを再実証済み。

あわせてテストが file-length ゲート(500行)に達したため、共有ヘルパーを
test/helpers/workflow-guards.js へ切り出し、gh 関連の検査を別ファイルへ分けた。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@keito4
keito4 merged commit 4a4303c into main Aug 4, 2026
17 checks passed
@keito4
keito4 deleted the fix/1070-workflow-guards-scope-and-ci branch August 4, 2026 12:07
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.133.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released リリース済み

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ワークフロー検査が配布テンプレートを走査せず CI でも強制されていない

1 participant