Skip to content

ci(shadcn): close the three declared alarm-channel gaps — timeout, analyze crash, consecutive unreachability - #4174

Merged
yinlianghui merged 1 commit into
mainfrom
claude/issue-3586-shadcn-check-alert-gaps
Aug 10, 2026
Merged

ci(shadcn): close the three declared alarm-channel gaps — timeout, analyze crash, consecutive unreachability#4174
yinlianghui merged 1 commit into
mainfrom
claude/issue-3586-shadcn-check-alert-gaps

Conversation

@yinlianghui

Copy link
Copy Markdown
Collaborator

Fixes#3586

Closes all three gaps PR #3497 declared and deliberately left open, per the PM claim comment's per-gap direction.

Premise check (against origin/main @ 0e67b53ff)

All three still live, re-verified in the tree rather than taken from the card:

  • Gap ②grep 'timeout-minutes' over .github/workflows/shadcn-check.yml: no match, on the job or any step.
  • Gap ③ — the Analyze components (offline) step still carried continue-on-error: true, and the classification block only ever read the check step's exit code.
  • Gap ① — nothing in the workflow reads any prior run.

One premise detail worth recording because it changes how gap ① reads: the registry is currently reachable from CI. Run 31374857502 (2026-08-10) logs Registry: 0 cached, 46 fetched with Errors: 0. So the escalation being added is a real backstop, not a workaround for a standing outage.

Gap ② — timeout-minutes: 20

The online step is 46 serial registry requests, and fetchUrl in scripts/shadcn-sync.js sets no socket timeout, so a black-holed connection was bounded by nothing but the job's default of 360 minutes.

Arithmetic, from measurement (stated on the job in the workflow too):

inputmeasured
whole job, all 30 runs in API history19-49 s; longest 49 s (2026-04-06); most recent 34 s
online step, run 31374857502~1.7 s for all 46 components (09:30:22.71 to 09:30:24.03), ~37 ms per serial request
pnpm install --frozen-lockfile7 s, on a pnpm store cache hit that all 30 observed runs got

Degraded-but-alive worst case at 10 s per request is 46 x 10 s = 7.7 min, plus ~35 s overhead; budgeting ~5 min for a cold store on a lockfile change gives ~13 min of compound worst case. 20 is ~1.5x that, ~24x the longest run ever observed, and turns a hang from six hours into twenty minutes.

Gap ③ — an analyze crash enters the same issue channel

component-analysis.js has exactly one non-zero exit: an uncaught crash. #3497 left the step tolerant because there was "no verdict here to swallow" — true of the drift verdict, false of the step itself, so a crash turned the job red on an unwatched weekly schedule and reached nobody.

  • continue-on-error is gone; the exit code is captured explicitly the way ci(shadcn): 按失败类别把 shadcn:check 的退出码路由进 issue 通道 #3497 captured the check step's, and the step still exits 0 by construction.
  • The class joins the existing classification as analyze: ok | broken, and broken routes into the same issue — same labels, same de-duplication, no second channel and no new continue-on-error.
  • The capture is now 2>&1 | tee. The old > analysis.txt sent only stdout to the file the issue body quotes, and a crash writes its stack to stderr — so the alarm would have reported a crash whose error text it had thrown away.

Gap ① — N consecutive unreachable runs escalate

Cost shape of every candidate mechanism (full version in readRegistryStreak's header):

mechanismcost at N=3verdict
previous run's conclusion1 callStructurally blind.#3497's tolerance makes an unreachable run exit 0, so its conclusion is success — identical to a clean run. Cannot express the distinction at any price.
previous runs' step conclusions3 calls, actions: read, state retained with the run (90 days ≈ 13 weekly runs)CHOSEN
previous runs' artifacts5 calls + downloads/unzip; 30-day retention ≈ 4 weekly runsmore expensive, no headroom above N=3
previous runs' annotations5 calls; state is ::warning:: prose re-parsed a week latermore expensive, weaker contract
comments on the alarm issue1-2 callsDisqualified on design. A tolerated run has no issue to comment on (label:shadcn-sync has matched 0 issues, ever), so it needs either opening the issue on run 1 — which is the alarm, contradicting the ruling this card leaves untouched — or a private state issue, i.e. the second channel the card forbids.
actions/cache~0 callsDisqualified. Entries evict 7 days after last access; the cron is exactly 7 days. A silently reset streak is an escalation that never fires — the failure shape being closed, rebuilt inside the fix.

Implementation: two no-op marker steps whose names and conclusions are the state the next run reads back. Three-valued by construction — exactly one succeeds in a run that reached a verdict, and neither appears in a run that died earlier, so "we don't know" stays distinguishable from "reachable" and only unreachable extends the streak. The walk stops at the first non-unreachable run, so it never costs more than N-1 job reads.

N = 3. Weekly cron, so N is weeks of blindness: N=2 (14 days) fires on any fortnight-long CDN or egress hiccup, which is the noise the single-run tolerance was ruled to avoid; N=4 (28 days) is a month of an early-warning system warning about nothing. 3 is past transient and inside every retention window the mechanism uses.

The single-run ruling is untouched for runs 1..N-1: exit 0, ::warning::, summary line, no issue. The only change to that path is that the warning now says which run of the three it is.

Two honest boundaries:

  • A failed cross-run read alarms rather than silently disabling the escalation — ci(shadcn): 按失败类别把 shadcn:check 的退出码路由进 issue 通道 #3497's "a check that cannot report is not a passing check", applied to the new moving part. It costs a false alarm on a hard API outage, which is the cheap direction to be wrong in.
  • On the first run after this merges, prior runs have no markers, so they read unknown and the streak starts at 1 even if the registry has been unreachable for weeks. Conservative by design.

Why the classification moved out of YAML

#3497 verified its logic by hand-running five fixtures once and writing the table into its PR body; nothing re-runs that between weekly cron fires. This card adds two more classified inputs to the same decision, so the classification now lives in scripts/shadcn-check-report.mjs, covered by scripts/__tests__/shadcn-check-report.test.ts — the same split scripts/render-budget-comment.mjs uses for performance-budget.yml.

The classifier exits 0 for every classified outcome, alarms included: the alarm is the issue, not the job colour. Its own crash still only turns the job red — the one failure this mechanism cannot route into its own channel, and the reason the logic was moved somewhere it can be unit tested.

Verification

vitest run --project unit scripts/__tests__31 files, 606 tests, all passing; the new file is 28 of them.

 Test Files 31 passed (31)
Tests 606 passed (606)

pnpm type-check:scripts exit 0 · node scripts/check-control-bytes.mjs OK (3852 files) · npx eslint on both new files exit 0 · yaml.safe_load parses the workflow, and the parsed step list confirms continue-on-error is now absent from every step.

node scripts/check-changeset-presence.mjs: "No source of a released package changed in this range, so no changeset is owed" — the presence script arbitrates, as the card directs. No skip-changeset label (decorative in this repo, #3724).

Reverse verification — direction predicted before each run

mutationpredictedobserved
gap ③: classifyAnalyze back to always ok + continue-on-error restored3 red3 red — the two classification tests and the YAML pin
gap ②: delete the timeout-minutes lineexactly 1 red1 red, the gap ② test
gap ①: rename ONE marker step in the YAML onlyexactly 1 red1 red, the marker-name pin

The gap ③ run is the load-bearing one, and the discriminating detail is what did not move: the test named "REVERSE: the identical run with the crash classified ok produces no alarm at all" stayed green through that mutation. It pins the pre-#3586 state — crash, job red on a weekly job, nothing in the triage queue — so if it had gone red with the others, the pair would have been keying off one switch instead of separating the two states.

The gap ① mutation is the one worth the pin: renaming a marker breaks nothing visible, resets the streak forever, and the escalation simply never fires again. That is why the names are asserted against the module's exported constants rather than re-spelled in the test.

Not in scope, per the card

Synthetically firing the token-creates-issue path (#3497 deliberately left it to the first real event) and the workflow_dispatch path (already verified 2026-08-07).


Generated by Claude Code

PR #3497 built the alarm channel and disclosed three gaps it left open.
This closes all three, and moves the classification out of YAML so the
new branching is testable:
- gap 2: `timeout-minutes: 20` on the job. The online step is 46 serial
registry requests and `fetchUrl` sets no socket timeout, so a hang was
bounded only by GitHub's 360-minute default. Sized from measurement:
49s longest of 30 observed runs, ~13min compound degraded worst case.
- gap 3: the `analyze` step's exit code is captured instead of hidden
behind `continue-on-error`, and an analyze crash enters the SAME issue
channel as check failures. Its output is now captured with `2>&1`, so
the crash the alarm reports is actually in the alarm's body.
- gap 1: N consecutive unreachable runs escalate into that same channel
(N=3). Cross-run state is carried by two marker steps whose names and
conclusions the next run reads via the Actions API — the cheapest
honest mechanism of the five costed in `readRegistryStreak`'s header.
The single-run tolerance ruling is untouched for runs 1..N-1.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
@vercel

vercelBot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectuiIgnoredIgnoredAug 10, 2026 4:23pm

Request Review

@yinlianghui
yinlianghui marked this pull request as ready for review August 10, 2026 16:33
@yinlianghui
yinlianghui added this pull request to the merge queueAug 10, 2026
Merged via the queue into main with commit e638531Aug 10, 2026
19 checks passed
@yinlianghui
yinlianghui deleted the claude/issue-3586-shadcn-check-alert-gaps branch August 10, 2026 16:33
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] shadcn-check 告警通道的三个已声明缺口:跨运行不可达告警 / 无 timeout-minutes / analyze 崩溃不入 issue 通道

2 participants

@yinlianghui@claude