fix(workflows): reference alert-failure action by remote ref, not local path (#61)#62
fix(workflows): reference alert-failure action by remote ref, not local path (#61)#62
Conversation
…al path Using `./.github/actions/alert-failure` requires the working tree to be populated, which fails precisely when we most need the alert — when checkout itself has failed. Switching to `elizaOS/knowledge/...@main` makes GitHub fetch the action via the Actions API, independent of the workflow's own checkout step. Fixes #61 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughUpdated references to the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/sync.yml (1)
141-146: Consider pinning to a commit SHA instead of@main(belt-and-braces for your alerting lifeline).Pinning a third-party-style
uses:to a mutable branch is the classic GitHub Actions supply-chain footgun — any push tomaininstantly changes what runs in every workflow, including ones triggered bypull_requestfrom forks. Since this is your own repo the blast radius is smaller, but it also means:
- A bad commit to
.github/actions/alert-failure/action.ymlonmainsilently breaks all failure alerts (the very thing this PR is trying to harden).- The PR's own trade-off note ("edits to alert-failure aren't testable on PRs that modify it") is a direct consequence of the
@mainpin — a SHA pin + Dependabot would give you both testability and immutability.Not a blocker given it's a same-repo internal action, just worth a conscious decision. Applies identically to all 9 call sites in this PR.
# Option A: pin to SHA (recommended; pairs well with Dependabot for actions) - uses: elizaOS/knowledge/.github/actions/alert-failure@<commit-sha> # Option B: pin to a release tag you bump intentionally - uses: elizaOS/knowledge/.github/actions/alert-failure@v1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sync.yml around lines 141 - 146, The workflow step currently references the internal action with a mutable ref "elizaOS/knowledge/.github/actions/alert-failure@main"; change that to an immutable ref by pinning to a specific commit SHA or a release tag (e.g., replace "@main" with "@<commit-sha>" or "@v1") for this step and the other eight call sites so the Alert on failure action cannot change unexpectedly—update each occurrence of the uses: elizaOS/knowledge/.github/actions/alert-failure@main to a fixed SHA or version and ensure you keep the chosen SHA/tag updated via Dependabot or manual bumping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/sync.yml:
- Around line 141-146: The workflow step currently references the internal
action with a mutable ref
"elizaOS/knowledge/.github/actions/alert-failure@main"; change that to an
immutable ref by pinning to a specific commit SHA or a release tag (e.g.,
replace "@main" with "@<commit-sha>" or "@v1") for this step and the other eight
call sites so the Alert on failure action cannot change unexpectedly—update each
occurrence of the uses: elizaOS/knowledge/.github/actions/alert-failure@main to
a fixed SHA or version and ensure you keep the chosen SHA/tag updated via
Dependabot or manual bumping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e348ba1f-75a6-4f99-bdb9-0525d24295f6
📒 Files selected for processing (7)
.github/workflows/README.md.github/workflows/aggregate-daily-sources.yml.github/workflows/extract_daily_facts.yml.github/workflows/generate-council-briefing.yml.github/workflows/help-reports.yml.github/workflows/retro.yml.github/workflows/sync.yml
1 participant
Fixes #61.
Summary
Replace
uses: ./.github/actions/alert-failurewithuses: elizaOS/knowledge/.github/actions/alert-failure@mainacross all 9 call sites (plus one doc example inworkflows/README.md).Why
Local-path action references require the working tree to be populated. When
actions/checkoutfails — which is exactly the worst-case silent-outage scenario — theAlert on failurestep also fails to loadaction.ymlfrom disk. Remote-refuses:bypasses the working tree entirely; GitHub fetches the action via the Actions API.This is the failure mode that hid the 6-day sync.yml outage (issue #57) from all monitoring.
Test plan
uses: \./\.github/actions/alert-failurereferencestoken: \${{ secrets.DOES_NOT_EXIST }}into a workflow's checkout step, confirm the Discord alert fires despite checkout failureTrade-off
PR changes to the
alert-failureaction itself will not be self-testable on the PR that introduces them — the@mainpin still points at merged main. Acceptable given the action changes rarely.🤖 Generated with Claude Code
Summary by CodeRabbit