feat(ci): ワークフロー検査をリポジトリ横断で実行できるようにする - #1075
Conversation
check_gh_repo_context は gh コマンドと同じ行にある --repo しか見ていなかった。 シェルの行継続で書かれた正常なワークフローを誤検知する。 keito4/calendar_alerm の release-please.yml が実際にこの書き方で、直近6実行は すべて success。PR #1071 でこの検査は CI をブロックするようになったため、 誤検知は正常動作しているリポジトリの PR を止める。 行継続を1つの論理行へ結合してから判定する。継続行があってもリポジトリ指定が 無ければ検出することをテストで固定した。 複数リポジトリのワークフローに対して実行して発見した。偽陽性を消したうえで 既知の実バグ4件を引き続き検出できることを再実証済み。 Closes #1072 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
検査は bash と awk だけで動き、対象リポジトリのワークフロー YAML を読むだけなので、 各リポジトリへスクリプトを配布せず config から一括で走らせられる。検査を直せば 次回の実行から全リポジトリへ反映される。 script/fleet-workflow-guards.sh を追加し、直近 push されたリポジトリを自動検出して 4検査を実行する。1リポジトリで違反が出ても走査は打ち切らず、全違反を集計してから 落ちる。結果は Job Summary へリポジトリ別のテーブルで出す。 走査は読み取りのみ。対象リポジトリへ Issue や PR は作らない。他リポジトリを読むため GITHUB_TOKEN ではなく CLAUDE_PAT を使う。 実リポジトリ13件に対して実行し、全て clean であることを確認済み。 raycast-extensions は最終 push が90日超のため対象外（仕様どおり）。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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. |
📝 WalkthroughWalkthroughChangesFleet workflow guards
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
Claude finished @keito4's task in 2m 58s —— View job タスク進捗
概要
Security
Quality
Performance / Accessibility
AI Residuals
Test Coverage
その他確認事項
Branch: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 081e569163
ℹ️ 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".
|
|
||
| for guard in "${GUARDS[@]}"; do | ||
| # 1 つ落ちても残りの検査を続ける。まとめて直せるようにするため。 | ||
| findings+="$(cd "$dest" && "$SCRIPT_DIR/repo-maintenance.sh" "$guard" 2>&1 | grep -E '^(⚠|.*\[1;33m)' || true)"$'\n' |
There was a problem hiding this comment.
When a scanned workflow omits retention-days or sets it above 30, check_artifact_retention emits a plain file: ... diagnostic rather than an output::warning line. This grep therefore discards the diagnostic, while || true also discards the guard's nonzero status, causing the repository and the overall fleet scan to be reported as clean even though one of the four advertised guards failed.
Useful? React with 👍 / 👎.
| dest="$WORK_DIR/$repo" | ||
| rm -rf "$dest" |
There was a problem hiding this comment.
When --repos receives an invalid name containing traversal components, such as ../.., dest resolves outside WORK_DIR and this unconditional rm -rf can delete the repository checkout or other parent data before gh rejects the name. Because the script is documented for direct local use and the workflow exposes repos as free-form input, validate repository names before using them as paths or derive a path-safe destination.
Useful? React with 👍 / 👎.
|
Claude finished @keito4's task in 3m 37s —— View job タスク進捗
概要
Security / Correctness
Quality
Test Coverage
Performance / Accessibility
AI Residuals
その他確認事項
まとめ: blocking級は P1（ |
CI の shellcheck が SC2015 (A && B || C) を指摘した。単なる lint ではなく実害が あり、cd の失敗を || true が握り潰して「違反なし」と報告していた。実際この構造で テストが空振りしていた。 チェックアウトが無い場合は 2 を返して violation として集計し、サマリへ "not scanned" と残す。テストのスタブも既知リポジトリのときだけ dest を作るよう 直し、「空リポジトリ」と「走査できなかった」を区別できるようにした。 あわせて && を使わない形へ書き換え、CI の古い shellcheck でも通るようにした。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
script/README.md (1)
209-217: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the
--owneroption.The script accepts
--owner OWNERand theFLEET_OWNERenvironment variable. The usage block in the script lists--owner, but this section does not. Add it so the README matches the script surface.📝 Proposed change
./script/fleet-workflow-guards.sh # 直近90日に push されたリポジトリ ./script/fleet-workflow-guards.sh --days 30 ./script/fleet-workflow-guards.sh --repos "config ohana" +./script/fleet-workflow-guards.sh --owner keito4🤖 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 `@script/README.md` around lines 209 - 217, Update the “Usage” section in script/README.md to document the supported --owner OWNER option and its FLEET_OWNER environment-variable equivalent, alongside the existing fleet-workflow-guards.sh examples, so the README matches the script’s interface.script/fleet-workflow-guards.sh (1)
133-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueA finding that contains
|breaks the markdown table row.The
sedfilter strips ANSI codes and the leading⚠, but it does not escape|. Guard messages include workflow content, so a pipe character is possible. The job summary row then renders with extra columns.Escape
|in the samesedexpression.♻️ Proposed change
- summary+="| | $(printf '%s' "$line" | sed 's/\x1b\[[0-9;]*m//g; s/^[[:space:]]*⚠[[:space:]]*//') |"$'\n' + summary+="| | $(printf '%s' "$line" | sed 's/\x1b\[[0-9;]*m//g; s/^[[:space:]]*⚠[[:space:]]*//; s/|/\\\\|/g') |"$'\n'🤖 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 `@script/fleet-workflow-guards.sh` around lines 133 - 136, Update the `sed` filter in the `findings` loop to escape every `|` in each finding before appending it to `summary`, while preserving the existing ANSI-code and leading-warning-marker removal.script/lib/repo_maintenance_checks.sh (1)
202-218: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueLine-continuation state is not reset at a job boundary.
pendingsurvives across jobs and files. The job-header rule at line 204 is guarded bypending == "", so arunstep whose last line ends with\swallows the next job header intopending. The following job is then evaluated as part of the previous job. This is unlikely in valid YAML, but it is cheap to make the parser strict.Reset
pendinginsideflush().🤖 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 `@script/lib/repo_maintenance_checks.sh` around lines 202 - 218, Reset the line-continuation variable pending inside the flush() function so no continuation state carries into the next job or file. Keep the existing job-header parsing and eval_line behavior unchanged..github/workflows/fleet-workflow-guards.yml (1)
31-37: 🧹 Nitpick | 🔵 TrivialConsider the timeout and the owner default.
Two operational notes for the scheduled run:
timeout-minutes: 15covers up to 200 shallow clones plus four guards per repository. Watch the first scheduled runs and raise the value if the job is cancelled.- The owner defaults to the literal
keito4inside the script. PassingFLEET_OWNER: ${{ github.repository_owner }}keeps the workflow correct in a fork or after a rename.🤖 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 @.github/workflows/fleet-workflow-guards.yml around lines 31 - 37, Update the scan job configuration to pass FLEET_OWNER using the GitHub repository owner context, ensuring scheduled runs target the current owner in forks or renamed repositories; retain the existing timeout unless initial scheduled runs show that 15 minutes is insufficient.test/fleet-workflow-guards.test.js (1)
23-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
ghstub returns discovered names for anyrepo listinvocation.The stub ignores
--jqand--json, so it cannot detect a regression in thepushedAtcutoff filter or in the--dayshandling. The discovery test at line 141 therefore proves only thatmapfileconsumes stdout.Consider echoing JSON and letting the real
--jqpath run, or add a stub assertion that--daysreachedgh repo list. This is optional for this PR.🤖 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/fleet-workflow-guards.test.js` around lines 23 - 44, Strengthen buildGhStub so repo list handling validates the requested --days cutoff and supports the --json/--jq output path instead of always echoing discovered names. Update the discovery test using buildGhStub to verify the cutoff argument reaches gh repo list and that filtering is applied, while preserving existing clone behavior.
🤖 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.
Inline comments:
In `@script/fleet-workflow-guards.sh`:
- Around line 112-116: Increment the violations counter in the clone-failure
branch before continuing, matching the existing missing-checkout handling so
failed scans produce a nonzero exit status and are not reported as clean. Add
test coverage for a gh repo clone invocation that exits nonzero, asserting the
failure is summarized as a violation and the script returns failure.
- Around line 107-110: Add path-safety validation for the repo variable in the
loop immediately after the existing non-empty check and before assigning dest.
Reject any repo value containing path traversal characters such as `/`, `..`,
embedded nulls, or other dangerous characters by adding a validation check that
continues to the next iteration if the name fails. This ensures only safe
repository names reach the dest assignment and the rm -rf command.
- Around line 81-85: Update the guard loop around repo-maintenance.sh so it
preserves the command’s exit status instead of unconditionally swallowing it
with || true. Continue processing all guards when output contains violation
findings, but treat a non-zero status with no matching warning lines as a scan
error and report the repository as not clean.
- Around line 62-71: The discover_repos function does not validate the DAYS
variable before passing it to the date commands. When DAYS is not a positive
integer, both date invocations fail, cutoff becomes an empty string, and the jq
select filter matches all repositories instead of enforcing the intended time
window. Add validation to ensure DAYS is a positive integer before using it in
the date command invocations, and remove or restructure the || fallback so that
an invalid DAYS or failed date processing causes the function to exit with an
error rather than allow an empty cutoff value to reach the jq filter.
In `@script/lib/repo_maintenance_checks.sh`:
- Around line 193-201: Update eval_line to detect GH_REPO declarations before
entering a job, store that workflow-level state, and seed each job’s has_gh_repo
value from it when the job begins. Preserve the existing per-job detection and
command validation behavior.
In `@test/fleet-workflow-guards.test.js`:
- Around line 188-193: In the test 'does not report clean when the checkout is
missing', add an assertion to verify that the result object from runFleet has a
non-zero exit status or exit code. This ensures the script exits with failure
when the repository checkout is missing, not just that the output contains the
expected text. Place this assertion alongside the existing output expectations.
---
Nitpick comments:
In @.github/workflows/fleet-workflow-guards.yml:
- Around line 31-37: Update the scan job configuration to pass FLEET_OWNER using
the GitHub repository owner context, ensuring scheduled runs target the current
owner in forks or renamed repositories; retain the existing timeout unless
initial scheduled runs show that 15 minutes is insufficient.
In `@script/fleet-workflow-guards.sh`:
- Around line 133-136: Update the `sed` filter in the `findings` loop to escape
every `|` in each finding before appending it to `summary`, while preserving the
existing ANSI-code and leading-warning-marker removal.
In `@script/lib/repo_maintenance_checks.sh`:
- Around line 202-218: Reset the line-continuation variable pending inside the
flush() function so no continuation state carries into the next job or file.
Keep the existing job-header parsing and eval_line behavior unchanged.
In `@script/README.md`:
- Around line 209-217: Update the “Usage” section in script/README.md to
document the supported --owner OWNER option and its FLEET_OWNER
environment-variable equivalent, alongside the existing fleet-workflow-guards.sh
examples, so the README matches the script’s interface.
In `@test/fleet-workflow-guards.test.js`:
- Around line 23-44: Strengthen buildGhStub so repo list handling validates the
requested --days cutoff and supports the --json/--jq output path instead of
always echoing discovered names. Update the discovery test using buildGhStub to
verify the cutoff argument reaches gh repo list and that filtering is applied,
while preserving existing clone behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 309a921d-42e9-46c9-8842-9d7034b9ed88
📒 Files selected for processing (8)
.github/workflows/fleet-workflow-guards.ymlAGENTS.mdscript/README.mdscript/fleet-workflow-guards.shscript/lib/repo_maintenance_checks.shtest/fleet-workflow-guards.test.jstest/integration/workflows.batstest/repo-maintenance-gh-guards.test.js
| for repo in "${repos[@]}"; do | ||
| [[ -n "$repo" ]] || continue | ||
| dest="$WORK_DIR/$repo" | ||
| rm -rf "$dest" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching fleet-workflow-guards.sh:\n'
fd -a 'fleet-workflow-guards\.sh$' . || true
file="$(fd 'fleet-workflow-guards\.sh$' . | head -n 1 || true)"
if [[ -n "$file" ]]; then
printf '\nOutline:\n'
ast-grep outline "$file" || true
printf '\nRelevant lines:\n'
sed -n '1,170p' "$file" | cat -n
fi
printf '\nSearch for script/fleet-workflow-guards.sh references and repos input/argument usage:\n'
rg -n --hidden --glob '!*.lock' --glob '!node_modules/**' 'fleet-workflow-guards\.sh|\\-\\-repos|repos=|workflow_dispatch|inputs\.repos|output::fatal|rm -rf' .Repository: keito4/config
Length of output: 17642
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
for bad in ["foo/bar", "foo/..", "../etc", "/etc", "foo\\0bar"]:
dest = "/tmp/work/" + bad
print(f"{bad!r}: dest={dest!r}, starts_with={dest.startswith('/tmp/work/')}, contains_dotdot={bad.endswith('/..') or '/..' in bad}")
PYRepository: keito4/config
Length of output: 529
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Workflow relevant lines:\n'
sed -n '1,80p' .github/workflows/fleet-workflow-guards.yml | cat -n
printf '\nTest cases touching fleet-workflow-guards validation/repo names:\n'
sed -n '1,260p' test/fleet-workflow-guards.test.js | cat -nRepository: keito4/config
Length of output: 11410
Path Traversal (CWE-22): Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
Reject invalid repository names before constructing dest.
--repos comes from FLEET_REPOS, so values like ../evil, /etc/passwd, or embedded nulls are passed unchecked to rm -rf "$WORK_DIR/$repo" before the clone check. Gate repo before assigning dest, e.g. match an allowed repository-name pattern and reject names containing /, .., or other path-dangerous characters.
🤖 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 `@script/fleet-workflow-guards.sh` around lines 107 - 110, Add path-safety
validation for the repo variable in the loop immediately after the existing
non-empty check and before assigning dest. Reject any repo value containing path
traversal characters such as `/`, `..`, embedded nulls, or other dangerous
characters by adding a validation check that continues to the next iteration if
the name fails. This ensures only safe repository names reach the dest
assignment and the rm -rf command.
| if ! gh repo clone "$OWNER/$repo" "$dest" -- --depth 1 --no-tags >/dev/null 2>&1; then | ||
| output::warning "$repo: clone failed; skipped" | ||
| summary+="| \`$repo\` | ⚠️ clone failed |"$'\n' | ||
| continue | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A clone failure is reported as no violations and the script exits 0.
The continue at line 115 skips the violations increment. The summary row shows ⚠️ clone failed, but violations stays 0. If every repository fails to clone, line 143 prints "No workflow guard violations across N repositories" and line 154 returns success.
This contradicts the stated behavior that a repository which cannot be scanned must be aggregated as a violation. The missing-checkout path at line 121 already increments violations; make the clone-failure path consistent.
The current test suite does not cover this path. does not report clean when the checkout is missing exercises the scan_status -eq 2 branch, because the gh stub exits 0 for unknown repositories. Add a test where gh repo clone exits non-zero.
🐛 Proposed fix
if ! gh repo clone "$OWNER/$repo" "$dest" -- --depth 1 --no-tags >/dev/null 2>&1; then
+ violations=$((violations + 1))
output::warning "$repo: clone failed; skipped"
summary+="| \`$repo\` | ⚠️ clone failed |"$'\n'
continue
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ! gh repo clone "$OWNER/$repo" "$dest" -- --depth 1 --no-tags >/dev/null 2>&1; then | |
| output::warning "$repo: clone failed; skipped" | |
| summary+="| \`$repo\` | ⚠️ clone failed |"$'\n' | |
| continue | |
| fi | |
| if ! gh repo clone "$OWNER/$repo" "$dest" -- --depth 1 --no-tags >/dev/null 2>&1; then | |
| violations=$((violations + 1)) | |
| output::warning "$repo: clone failed; skipped" | |
| summary+="| \`$repo\` | ⚠️ clone failed |"$'\n' | |
| continue | |
| fi |
🤖 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 `@script/fleet-workflow-guards.sh` around lines 112 - 116, Increment the
violations counter in the clone-failure branch before continuing, matching the
existing missing-checkout handling so failed scans produce a nonzero exit status
and are not reported as clean. Add test coverage for a gh repo clone invocation
that exits nonzero, asserting the failure is summarized as a violation and the
script returns failure.
| function eval_line(l) { | ||
| if (!in_job) return | ||
| if (l ~ /actions\/checkout/) has_checkout = 1 | ||
| if (l ~ /^[[:space:]]*GH_REPO:/) has_gh_repo = 1 | ||
| if (l ~ /(^|[^A-Za-z0-9_-])gh[[:space:]]+((label|issue|release|run|workflow)|pr[[:space:]]+(create|list|status))([^A-Za-z0-9_-]|$)/) { | ||
| # gh は -R / --repo= も受け付ける。落とすと正当なワークフローを止める。 | ||
| if (l !~ /(--repo[[:space:]=]|-R[[:space:]])/) bad_cmd = 1 | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
GH_REPO declared at workflow level is not detected.
eval_line returns early when in_job is 0. A workflow that declares GH_REPO in the top-level env: block (before jobs:) satisfies every job, but the guard never records it. Such a workflow is reported as a violation.
If you want the guard to accept that pattern, track a workflow-level flag before jobs: and seed each job with it.
♻️ Proposed change
function flush() {
- if (in_job && bad_cmd && !has_checkout && !has_gh_repo) bad = 1
+ if (in_job && bad_cmd && !has_checkout && !has_gh_repo && !global_gh_repo) bad = 1
bad_cmd = 0; has_checkout = 0; has_gh_repo = 0
}
function eval_line(l) {
- if (!in_job) return
+ if (!in_jobs && l ~ /^[[:space:]]*GH_REPO:/) { global_gh_repo = 1; return }
+ if (!in_job) return📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function eval_line(l) { | |
| if (!in_job) return | |
| if (l ~ /actions\/checkout/) has_checkout = 1 | |
| if (l ~ /^[[:space:]]*GH_REPO:/) has_gh_repo = 1 | |
| if (l ~ /(^|[^A-Za-z0-9_-])gh[[:space:]]+((label|issue|release|run|workflow)|pr[[:space:]]+(create|list|status))([^A-Za-z0-9_-]|$)/) { | |
| # gh は -R / --repo= も受け付ける。落とすと正当なワークフローを止める。 | |
| if (l !~ /(--repo[[:space:]=]|-R[[:space:]])/) bad_cmd = 1 | |
| } | |
| } | |
| function flush() { | |
| if (in_job && bad_cmd && !has_checkout && !has_gh_repo && !global_gh_repo) bad = 1 | |
| bad_cmd = 0; has_checkout = 0; has_gh_repo = 0 | |
| } | |
| function eval_line(l) { | |
| if (!in_jobs && l ~ /^[[:space:]]*GH_REPO:/) { global_gh_repo = 1; return } | |
| if (!in_job) return | |
| if (l ~ /actions\/checkout/) has_checkout = 1 | |
| if (l ~ /^[[:space:]]*GH_REPO:/) has_gh_repo = 1 | |
| if (l ~ /(^|[^A-Za-z0-9_-])gh[[:space:]]+((label|issue|release|run|workflow)|pr[[:space:]]+(create|list|status))([^A-Za-z0-9_-]|$)/) { | |
| # gh は -R / --repo= も受け付ける。落とすと正当なワークフローを止める。 | |
| if (l !~ /(--repo[[:space:]=]|-R[[:space:]])/) bad_cmd = 1 | |
| } | |
| } |
🤖 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 `@script/lib/repo_maintenance_checks.sh` around lines 193 - 201, Update
eval_line to detect GH_REPO declarations before entering a job, store that
workflow-level state, and seed each job’s has_gh_repo value from it when the job
begins. Preserve the existing per-job detection and command validation behavior.
| test('does not report clean when the checkout is missing', () => { | ||
| const result = runFleet(['--repos', 'ghost'], { workflows: {} }); | ||
|
|
||
| expect(result.output).not.toContain('No workflow guard violations'); | ||
| expect(result.output).toContain('ghost'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the exit status for the missing-checkout case.
The PR objective requires a non-zero exit when a repository cannot be scanned. This test only checks the output text. It passes even if the script exits 0.
Add the status assertion so the exit contract is fixed by the test.
💚 Proposed change
const result = runFleet(['--repos', 'ghost'], { workflows: {} });
+ expect(result.status).not.toBe(0);
expect(result.output).not.toContain('No workflow guard violations');
expect(result.output).toContain('ghost');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('does not report clean when the checkout is missing', () => { | |
| const result = runFleet(['--repos', 'ghost'], { workflows: {} }); | |
| expect(result.output).not.toContain('No workflow guard violations'); | |
| expect(result.output).toContain('ghost'); | |
| }); | |
| test('does not report clean when the checkout is missing', () => { | |
| const result = runFleet(['--repos', 'ghost'], { workflows: {} }); | |
| expect(result.status).not.toBe(0); | |
| expect(result.output).not.toContain('No workflow guard violations'); | |
| expect(result.output).toContain('ghost'); | |
| }); |
🤖 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/fleet-workflow-guards.test.js` around lines 188 - 193, In the test 'does
not report clean when the checkout is missing', add an assertion to verify that
the result object from runFleet has a non-zero exit status or exit code. This
ensures the script exits with failure when the repository checkout is missing,
not just that the output contains the expected text. Place this assertion
alongside the existing output expectations.
CodeRabbit と Codex の指摘4件を修正した。いずれも横断走査が黙って 「違反なし」を報告する、または想定外の範囲へ広がる不具合。 1. 違反の判定を ⚠ 行の grep で行っていた check_artifact_retention は output::warning ではなく素の "file: message" を出すため、非ゼロ終了しているのに違反が消えて clean と報告されていた。 依存不足など違反以外の失敗も同様に握り潰していた。 → 終了ステータスを正とし、出力が空でもその旨を残す。 2. --days を検証していなかった date が失敗すると cutoff が空になり、jq の比較が常に真になって全リポジトリ へ黙って広がる。→ 正の整数のみ受け付ける。 3. リポジトリ名を検証していなかった ../.. のような名前は dest が作業ディレクトリの外を指し、rm -rf がそこへ 及ぶ。→ 英数字とドット・ハイフン・アンダースコアのみ受け付ける。 artifact-retention の消失は実際に再現して確認した。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.134.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Closes #1074
Note
PR #1073 の commit を含みます。行継続の誤検知を直さないまま横断実行すると正常な calendar_alerm が違反として出るため、この PR は #1073 の上に積んでいます。base はフル CI を通すため main にしています。#1073 を先にマージすれば、この PR の差分は fleet 分だけに縮みます。どちらの順でマージしても問題ありません。
Why
ワークフロー検査は config 内でしか動かない。下流リポジトリには
script/repo-maintenance.shもscheduled-maintenance.ymlも配布されておらず、他のリポジトリで同種の不具合が起きても検知できない。実際、検査を実リポジトリへ向けたところ config 内では見つからなかった誤検知（#1072）が判明した。横断実行は再発防止だけでなく検査自体の品質にも効く。
What
検査は bash と awk だけで動き、対象リポジトリのワークフロー YAML を読むだけなので、各リポジトリへスクリプトを配布せず config から一括で走らせられる。検査を直せば次回の実行から全リポジトリへ反映される。
script/fleet-workflow-guards.sh.github/workflows/fleet-workflow-guards.ymldays/reposを指定可能方針
GITHUB_TOKENではなくCLAUDE_PATを使う（GITHUB_TOKENは自リポジトリしか見えない）How to test
実リポジトリ13件に対して本番同等で実行:
raycast-extensionsは最終 push が 90 日超のため対象外（仕様どおり）。検出側: 行継続の修正前に同じ仕組みで走らせたところ、calendar_alerm の
release-please.ymlを名指しで報告した（それが #1072 の発見経緯）。レビュー対応
CodeRabbit / Codex / CI の shellcheck から計 5 件の指摘。いずれも横断走査が黙って「違反なし」を報告する、または想定外の範囲へ広がるもので、すべて修正した。
check_artifact_retentionはoutput::warningではなく素のfile: messageを出すため、非ゼロ終了しているのに違反が消えて clean と報告されていた（再現確認済み）。依存不足など違反以外の失敗も同様に握り潰していた → 終了ステータスを正とするA && B || C)cdの失敗を|| trueが握り潰し「違反なし」と報告。実際この構造でテストが空振りした → チェックアウトが無い場合は violation として集計しnot scannedと残す--days未検証dateが失敗するとcutoffが空になり jq の比較が常に真 → 全リポジトリへ黙って広がる → 正の整数のみ受け付ける../..のような名前はdestが作業ディレクトリの外を指し、rm -rfがそこへ及ぶ → 英数字とドット・ハイフン・アンダースコアのみ受け付けるテストのスタブも「空リポジトリ」と「走査できなかった」を区別できるよう直した。
Unit 951 / Integration 310 / lint / format / shellcheck / actionlint すべて pass。実リポジトリ 5 件での再実行も clean。
Risk
CLAUDE_PATが必要。未設定・失効時は clone が失敗し、該当リポジトリは⚠️ clone failedとして記録され走査は継続する（全体は落ちない）。repos入力で明示指定もできる。--depth 1 --no-tagsの浅いクローンで、ランナー上の一時領域にのみ展開する。🤖 Generated with Claude Code
Summary by CodeRabbit