fix(watchdog): use a scope the token actually has, and label watchdog faults - #2542
Merged
Conversation
… faults The first fix traded one false alarm for another. Checking the mirror's run status via the Actions API is the natural question to ask, but MONO_READ_TOKEN carries contents:read only and 403s on actions:read — so the step died mid-run under `bash -e` and surfaced as a bare red run, indistinguishable from a real content alert. I had dry-run it with my own token, which has both scopes. Answer the same question with contents:read: walk back through mono's recent `content/` commits and take the newest one that touched a path the mirror actually publishes (content/** minus _system/, plus _system/generated/), then compare that against the "mirror: sync from mono@<sha>" stamp. Verified against live history — it skips d9a33de and 539b81f, both content/_system/ only, and lands on 0b0740a, which matches the stamp. Silent, correctly. Two hardening changes from the same incident: Every API call is now `|| true`. The step shell is `bash -e`, so any unguarded non-zero aborts the script at that line and the operator sees a red run with no message explaining it. "Cannot evaluate" is now its own labelled state — WATCHDOG DEGRADED — rather than looking identical to "content is stuck". A watchdog that cannot watch is still worth waking someone for, but not for the wrong reason.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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. |
Contributor
📝 WalkthroughWalkthroughThe content pipeline watchdog now tolerates GitHub API failures, compares the latest publishable mono commit with the mirror tip, marks unevaluable checks as degraded, and hardens age, publish PR, and CI metadata handling. ChangesContent pipeline watchdog hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Watchdog
participant MonoRepository
participant PeanutContent
participant GitHubAPI
Watchdog->>MonoRepository: scan recent commits affecting published paths
MonoRepository-->>Watchdog: return expected mono commit
Watchdog->>PeanutContent: read mirror tip commit metadata
PeanutContent-->>Watchdog: return mirrored mono commit
Watchdog->>GitHubAPI: fetch publish PR and CI metadata
GitHubAPI-->>Watchdog: return available metadata or failure
Watchdog->>Watchdog: set staleness, degraded, and gate state
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code-analysis diffPainscore total: 6309.47 → 6309.47 (0) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/content-pipeline-watchdog.yml (1)
59-64: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsolidate 3 calls to the same commit into 1.
TIP,TIP_MSG, andTIP_ATall hitrepos/peanutprotocol/peanut-content/commits/mainseparately. Besides the extra round-trips, each is an independent|| truefailure point — e.g.TIPcan succeed whileTIP_MSGtransiently fails, pushing the run intoDEGRADEDeven though the data was actually available a moment earlier.♻️ Proposed consolidation
- TIP=$(GH_TOKEN=$CONTENT_TOKEN gh api repos/peanutprotocol/peanut-content/commits/main --jq '.sha' || true) - TIP_MSG=$(GH_TOKEN=$CONTENT_TOKEN gh api repos/peanutprotocol/peanut-content/commits/main --jq '.commit.message' 2>/dev/null | head -1 || true) - TIP_AT=$(GH_TOKEN=$CONTENT_TOKEN gh api repos/peanutprotocol/peanut-content/commits/main --jq '.commit.committer.date' || true) + TIP_JSON=$(GH_TOKEN=$CONTENT_TOKEN gh api repos/peanutprotocol/peanut-content/commits/main || true) + TIP=$(printf '%s' "$TIP_JSON" | jq -r '.sha // empty') + TIP_MSG=$(printf '%s' "$TIP_JSON" | jq -r '.commit.message // empty' | head -1) + TIP_AT=$(printf '%s' "$TIP_JSON" | jq -r '.commit.committer.date // empty')🤖 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/content-pipeline-watchdog.yml around lines 59 - 64, Consolidate the three gh api requests in the hop-2 watchdog block into one request for the main commit, storing its JSON response once and deriving TIP, TIP_MSG, and TIP_AT from that shared response. Preserve the existing empty-value and failure-tolerant behavior while eliminating the independent calls and failure points.
🤖 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 @.github/workflows/content-pipeline-watchdog.yml:
- Around line 81-88: Update the per-commit fetch logic in the SHA loop so a
failed `gh api` request for commit file names is distinguished from a successful
response containing no published paths. Track the request status separately, and
mark the watchdog as degraded or otherwise stop normal expected-SHA selection
when fetching fails instead of continuing to an older SHA; preserve the existing
behavior for successful commits with no published paths.
- Around line 131-134: Update the workflow permissions to grant checks: read
alongside the existing UI_TOKEN permissions, and guard the check-runs gh api
invocation in the CI assignment with || true so lookup failures do not abort the
watchdog step.
---
Nitpick comments:
In @.github/workflows/content-pipeline-watchdog.yml:
- Around line 59-64: Consolidate the three gh api requests in the hop-2 watchdog
block into one request for the main commit, storing its JSON response once and
deriving TIP, TIP_MSG, and TIP_AT from that shared response. Preserve the
existing empty-value and failure-tolerant behavior while eliminating the
independent calls and failure points.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e6c12a06-e932-4e40-822c-a75f0a95d75e
📒 Files selected for processing (1)
.github/workflows/content-pipeline-watchdog.yml
Comment on lines
+81
to
+88
| for SHA in $(GH_TOKEN=$MONO_TOKEN gh api \ | ||
| "repos/peanutprotocol/mono/commits?path=content&per_page=15" --jq '.[].sha' || true); do | ||
| FILES=$(GH_TOKEN=$MONO_TOKEN gh api "repos/peanutprotocol/mono/commits/$SHA" --jq '.files[].filename' || true) | ||
| # Published: content/** except content/_system/**, plus content/_system/generated/** | ||
| PUBLISHED=$( { printf '%s\n' "$FILES" | grep '^content/' | grep -v '^content/_system/' | ||
| printf '%s\n' "$FILES" | grep '^content/_system/generated/'; } | head -1 ) | ||
| if [ -n "$PUBLISHED" ]; then EXPECTED="$SHA"; break; fi | ||
| done |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Per-commit fetch failure inside the loop is misclassified as "not published".
If gh api .../commits/$SHA --jq '.files[].filename' fails transiently for the actual newest publish-touching commit, FILES comes back empty via || true and the loop just moves on to an older SHA, silently setting EXPECTED to a stale commit. If the mirror already synced past that point, this produces exactly the false "mirror is behind mono" alert this PR is trying to eliminate — just one layer deeper. The top-level DEGRADED check (line 98) can't catch this because EXPECTED is non-empty, just wrong.
🐛 Proposed fix — distinguish fetch failure from "no published paths"
EXPECTED=""
+ FETCH_FAILED=false
for SHA in $(GH_TOKEN=$MONO_TOKEN gh api \
"repos/peanutprotocol/mono/commits?path=content&per_page=15" --jq '.[].sha' || true); do
- FILES=$(GH_TOKEN=$MONO_TOKEN gh api "repos/peanutprotocol/mono/commits/$SHA" --jq '.files[].filename' || true)
+ if ! FILES=$(GH_TOKEN=$MONO_TOKEN gh api "repos/peanutprotocol/mono/commits/$SHA" --jq '.files[].filename'); then
+ FETCH_FAILED=true
+ break
+ fi
# Published: content/** except content/_system/**, plus content/_system/generated/**
PUBLISHED=$( { printf '%s\n' "$FILES" | grep '^content/' | grep -v '^content/_system/'
printf '%s\n' "$FILES" | grep '^content/_system/generated/'; } | head -1 )
if [ -n "$PUBLISHED" ]; then EXPECTED="$SHA"; break; fi
done
+ [ "$FETCH_FAILED" = true ] && EXPECTED=""📝 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.
Suggested change
| for SHA in $(GH_TOKEN=$MONO_TOKEN gh api \ | |
| "repos/peanutprotocol/mono/commits?path=content&per_page=15" --jq '.[].sha' || true); do | |
| FILES=$(GH_TOKEN=$MONO_TOKEN gh api "repos/peanutprotocol/mono/commits/$SHA" --jq '.files[].filename' || true) | |
| # Published: content/** except content/_system/**, plus content/_system/generated/** | |
| PUBLISHED=$( { printf '%s\n' "$FILES" | grep '^content/' | grep -v '^content/_system/' | |
| printf '%s\n' "$FILES" | grep '^content/_system/generated/'; } | head -1 ) | |
| if [ -n "$PUBLISHED" ]; then EXPECTED="$SHA"; break; fi | |
| done | |
| EXPECTED="" | |
| FETCH_FAILED=false | |
| for SHA in $(GH_TOKEN=$MONO_TOKEN gh api \ | |
| "repos/peanutprotocol/mono/commits?path=content&per_page=15" --jq '.[].sha' || true); do | |
| if ! FILES=$(GH_TOKEN=$MONO_TOKEN gh api "repos/peanutprotocol/mono/commits/$SHA" --jq '.files[].filename'); then | |
| FETCH_FAILED=true | |
| break | |
| fi | |
| # Published: content/** except content/_system/**, plus content/_system/generated/** | |
| PUBLISHED=$( { printf '%s\n' "$FILES" | grep '^content/' | grep -v '^content/_system/' | |
| printf '%s\n' "$FILES" | grep '^content/_system/generated/'; } | head -1 ) | |
| if [ -n "$PUBLISHED" ]; then EXPECTED="$SHA"; break; fi | |
| done | |
| [ "$FETCH_FAILED" = true ] && EXPECTED="" |
🤖 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/content-pipeline-watchdog.yml around lines 81 - 88, Update
the per-commit fetch logic in the SHA loop so a failed `gh api` request for
commit file names is distinguished from a successful response containing no
published paths. Track the request status separately, and mark the watchdog as
degraded or otherwise stop normal expected-SHA selection when fetching fails
instead of continuing to an older SHA; preserve the existing behavior for
successful commits with no published paths.
Comment on lines
131
to
134
| if [ -n "$NUM" ]; then | ||
| SHA=$(GH_TOKEN=$UI_TOKEN gh api "repos/$REPO/pulls/$NUM" --jq '.head.sha') | ||
| CI=$(GH_TOKEN=$UI_TOKEN gh api "repos/$REPO/commits/$SHA/check-runs?per_page=100" \ | ||
| PR_SHA=$(GH_TOKEN=$UI_TOKEN gh api "repos/$REPO/pulls/$NUM" --jq '.head.sha' || true) | ||
| CI=$(GH_TOKEN=$UI_TOKEN gh api "repos/$REPO/commits/$PR_SHA/check-runs?per_page=100" \ | ||
| --jq '[.check_runs[] | select(.name=="ci-success")] | if length == 0 then "pending" elif all(.conclusion == "success") then "green" else "RED" end') |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '--- workflow excerpt ---\n'
nl -ba .github/workflows/content-pipeline-watchdog.yml | sed -n '1,170p'
printf '\n--- search for checks permission / check-runs usage ---\n'
rg -n 'checks:|check-runs|ci-success|permissions:' .github/workflows/content-pipeline-watchdog.ymlRepository: peanutprotocol/peanut-ui
Length of output: 253
🏁 Script executed:
set -euo pipefail
echo '--- workflow excerpt ---'
nl -ba .github/workflows/content-pipeline-watchdog.yml | sed -n '1,170p'
echo
echo '--- search for checks permission / check-runs usage ---'
rg -n 'checks:|check-runs|ci-success|permissions:' .github/workflows/content-pipeline-watchdog.ymlRepository: peanutprotocol/peanut-ui
Length of output: 227
🏁 Script executed:
set -euo pipefail
echo '--- workflow excerpt with line numbers ---'
awk 'NR>=1 && NR<=170 { printf "%4d %s\n", NR, $0 }' .github/workflows/content-pipeline-watchdog.yml
echo
echo '--- search for checks permission / check-runs usage ---'
rg -n 'checks:|check-runs|ci-success|permissions:' .github/workflows/content-pipeline-watchdog.ymlRepository: peanutprotocol/peanut-ui
Length of output: 11401
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/content-pipeline-watchdog.yml')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 170:
print(f"{i:4d} {line}")
PYRepository: peanutprotocol/peanut-ui
Length of output: 10921
🏁 Script executed:
set -euo pipefail
echo '--- workflow excerpt with line numbers ---'
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/content-pipeline-watchdog.yml')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 170:
print(f"{i:4d} {line}")
PY
echo
echo '--- search for checks permission / check-runs usage ---'
rg -n 'checks:|check-runs|ci-success|permissions:' .github/workflows/content-pipeline-watchdog.ymlRepository: peanutprotocol/peanut-ui
Length of output: 11401
Guard the check-runs lookup and add checks: read.
UI_TOKEN only has contents: read and pull-requests: read, so this call needs checks: read; without it, the lookup can fail and, because it isn’t wrapped in || true, abort the watchdog mid-step.
🤖 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/content-pipeline-watchdog.yml around lines 131 - 134,
Update the workflow permissions to grant checks: read alongside the existing
UI_TOKEN permissions, and guard the check-runs gh api invocation in the CI
assignment with || true so lookup failures do not abort the watchdog step.
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second fix for the same watchdog. It is still false-alarming to Discord every 15 min — this stops it for real.
What went wrong with fix #1
#2541 replaced the SHA comparison with a check on the mirror workflow's last run status. Correct question, wrong API:
MONO_READ_TOKENhas contents:read only — it 403s onactions:read. The step shell isbash -e, so the unguarded failure aborted mid-script and surfaced as a bare red run, indistinguishable from a real content alert. I had dry-run it with my token, which has both scopes. Same class of mistake as verifying MDX through the wrong compiler.The fix
Answer the same question using only contents:read — walk back through mono's recent
content/commits and take the newest that touched a path the mirror actually publishes (content/**minus_system/, plus_system/generated/), then compare that to themirror: sync from mono@<sha>stamp.Verified against live history — note it skipping exactly the commits that caused the original false alarm:
Hardening from the same incident
|| true. Underbash -ean unguarded non-zero aborts the script at that line, and the operator gets a red run with no message explaining it.WATCHDOG DEGRADED— instead of looking identical to "content is stuck". A watchdog that can't watch is still worth waking someone for, but not for the wrong reason. This is the failure mode that just cost two debugging cycles.Honest residual risk
I still cannot execute this with
MONO_READ_TOKENitself. What I can say:repos/mono/commits?path=contentis proven working with that token (it succeeded in the first live run), andrepos/mono/commits/{sha}is the same contents:read family. If it 403s anyway, it now reportsWATCHDOG DEGRADEDrather than a misleading content alert.Summary by CodeRabbit