[hivecommons-hive]

Closes #17433
Refs #17427 (hub side of ask 3; spoke side in #17434)

Why

When a pod cannot reach its GitHub host, the spoke now keeps beating and sets forge_unreachable. The hub still showed generic failing-check / stale-advisory badges and counted the resulting github_auth failure and stale stats against the hive, though the pod is healthy and only egress to the forge is broken.

What

• Hub HeartbeatPayload.ForgeUnreachable (forge_unreachable), persisted on the registry entry (forgeUnreachable).
• Hive rows get forgeUnreachableLabel: "forge unreachable (github_host)" (or "forge unreachable"), rendered in place of the failing-check and stale-advisory pills.
• Skipped for this state: stale-advisory flag, stale-stats / broken-App health verdict, github_auth in the health-check-failing alert.
• Old spokes omit the field, so behaviour is unchanged.
• Tests in forge_unreachable_test.go; changelog fragment; api-reference citations refreshed.

— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 requested_by=@clubanderson

[kubestellar-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign danathar for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[hivecommons-hive]

Hive status — updated in place; edits do not notify. Full history is in the Activity Monitor.

hive/awaiting-ci: CI pending — sweep will merge when green. (TestStyleRatchet: the new forge-unreachable pill now reuses the stale-advisory pill markup, so the inline-style and raw-colour counts are back at baseline.)

---

🐝 Hive Agent: scanner | Instance: hosted-available-oke-11-placeholder-r05x | SHA: f609107fc

— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88

Review findings from adjudicator (folded here because this head already carries its hive review):

docs-currency (head f609107)

src/docs/advisory-staleness.md not updated for the new advisoryStale gate. The PR adds an unconditional early return to advisoryStale (src/pkg/hub/advisory_staleness.go:196-198: if e.ForgeUnreachable { return false, "" }), but src/docs/advisory-staleness.md — which states at :4-5 that "The rule lives in src/pkg/hub/advisory_staleness.go" and enumerates its gates at :16-36 — still lists only the four gates (participation / reported error / expected quiet / age). The suppression matrix at :40-51 already carries the analogous statsStale row (:51, #12842) and has no row for forge_unreachable. An operator reading that doc to understand why a hive shows no stale-advisory pill will not find this suppression.

Fix: add a gate 0 (or a note under Participation) — "ForgeUnreachable beat: never stale; the row shows forge unreachable (github_host) instead (#17433)" — and a matrix row: Spoke reports forge_unreachable (pod healthy, forge egress down) | would flag stale from the ageing post time | no pill / no alert; labelled by host | advisoryStale returns false when ForgeUnreachable.

The hub-server.md citation refresh (+6 lines from the RegistryEntry field at server.go:341-347) checks out against the diff; changelog fragment is present and accurate.

Review findings from reviewer (folded here because this head already carries its hive review):

Read the diff against the tree at 85c296a plus the merged spoke side (#17434). Looks correct to me.

What I verified

• Wire compatibility: the spoke on v5 sends ForgeUnreachable bool \json:"forge_unreachable,omitempty"` (src/pkg/hub/spoke/heartbeat.go:859, from #17434) and this PR's hub HeartbeatPayload reads the same tag (src/pkg/hub/heartbeat.go:299), copied onto the registry entry per beat at server.go:2132. Old spokes omit it → false` → today's behaviour, as the body says.
• forge_unreachable.go:10-17 blanks the label once Online is false, so a hive that stops beating during a forge outage does not keep a "healthy apart from egress" pill; the UI deliberately does not re-gate on h.online for that reason, and the two cannot drift.
• health_verdict.go:86-88 only downgrades the App bucket to unknown, and StatsStale still forces HealthStateUnknown with the forge named as cause (:100-102) — so this never asserts health for a hive it cannot see, matching the "unknown gates first" rule already in that function.
• alerts.go:1148-1150 / withoutForgeChecks drops only github_auth; other failing checks (disk etc.) still alert, and the browser filter in dashboard.html mirrors that exactly (ck.name !== 'github_auth').
• advisory_staleness.go:196-198 exempts the stale-advisory flag server-side, so the !fuPill gate in the browser is belt-and-braces rather than the only guard.

Small observation, not a defect: the shared amberNudgePill switches the stale-advisory pill's title from esc() to escAttr(), which is the right escaper for an attribute — a quiet improvement that rides along.

Evidence bundle `#1744185c296a` sha256:eb87dc713fc0ccd8426f47e79647481710e925ea64d2dadf2db8304a4cedf733 — download from the dashboard or `hivectl review evidence #17441`.

[github-actions]

Open PR overlap check

Base branch: v5

Conflicts are checked with every PR merged onto the current tip of v5, so a PR that only needs a rebase is not reported as conflicting with its siblings.

No conflicting open PRs or same-file overlaps were found.

[hivecommons-hive]

Looks correct to me. What I verified at c678b2a:

• The hub reads forge_unreachable (src/pkg/hub/heartbeat.go:299) and the spoke side in #17434 emits the same key from src/pkg/hub/spoke/heartbeat.go (ForgeUnreachable ... json:"forge_unreachable,omitempty"). The two PRs add the field to different structs (spoke payload vs hub payload), so they are complementary, not a merge conflict.
• handleHeartbeat copies it onto the RegistryEntry on every beat (src/pkg/hub/server.go:2132), so it clears automatically once the spoke stops reporting it; old spokes omit it and take today's path.
• hiveHealthFor (src/pkg/hub/health_verdict.go:82-86) neutralises StatsStale and the broken-App bucket only when the flag is set; advisoryStale (advisory_staleness.go:196) and evaluateAlertsInFleet (alerts.go:1148, via withoutForgeChecks) each short-circuit on the same flag. The tests in forge_unreachable_test.go cover all three paths against the pre-change baseline.
• Dashboard: fuPill replaces fcPill only when the label is non-empty (dashboard.html:9544), so hives without the flag render exactly as before.

The test (rest 3/4) failure on this head also fails on #17442, which touches none of these files, so it does not look attributable to this change.

— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

The test (rest 3/4) failure on c678b2a is caused by this PR, contrary to the earlier note on this thread. The Go side reads correctly to me; the one defect is in the dashboard asset.

Finding (medium) — TestStyleRatchet fails on the new pill.
pkg/dashboard/webstatic/style_ratchet_test.go:88 reports, for the hub static pages: 926 style= attributes, above the baseline of 925 and 451 raw color literals/functions, above the baseline of 448. Those deltas (+1 style=, +3 raw colours) are exactly what forgeUnreachableSummary adds in src/pkg/hub/assets/dashboard.html (new lines in the hunk after the /* forgeUnreachableSummary … */ comment): one inline style="…" containing #d29922, rgba(210,153,34,0.12) and rgba(210,153,34,0.35). The job re-ran three times with the same result, so it is not flaky, and the ratchet is a merge-blocking test.

Fix is small: give the span a class and define it in the page's <style> block using the tokens the file already uses for amber pills — e.g. color: var(--amber); border: 1px solid var(--amber); background: color-mix(in srgb, var(--amber) 12%, transparent) (compare .cluster-health-message.warn and .access-typeahead-match in the same file). Do not bump styleRatchetBaselines; the test's own message says the count may only go down.

What I verified as correct against v5:

• withoutForgeChecks (src/pkg/hub/forge_unreachable.go:19-27) uses names[:0:0], so a nil input stays nil and the result never aliases the caller's slice.
• hiveHealthBase (src/pkg/hub/health_verdict.go:84-87) neutralises StatsStale and the App bucket only when ForgeUnreachable is set; advisoryStale (advisory_staleness.go:196) and evaluateAlertsInFleet (alerts.go:1148) gate on the same flag. Old spokes omit the field and take today's path.
• The hub HeartbeatPayload key forge_unreachable (src/pkg/hub/heartbeat.go:299) matches what the spoke emits in #17434.

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Review follow-ups (below review.backlog.min_severity: P2)

Non-blocking findings below the backlog filing line are listed here instead of as issues, one comment per PR, updated in place. Tick them off as they are fixed; set review.backlog.min_severity: P3 to file every finding.

• P3 info security — amberNudgePill uses esc() rather than escAttr() for the title attribute (src/pkg/hub/assets/dashboard.html:3894): src/pkg/hub/assets/dashboard.html:3894 interpolates esc(title) into title="...". esc() (line 1043) is textContent/innerHTML based and does not encode double quotes, while escAttr() (line 1067) exists…

[hivecommons-hive]

Read the hub tree at a7628e5 plus the spoke side already on v5 (#17434). Wire key matches: spoke src/pkg/hub/spoke/heartbeat.go:859 emits forge_unreachable, hub heartbeat.go:299 reads it, server.go:2132 persists it. The Go-side exemptions are narrow and correct: advisoryStale early-return (advisory_staleness.go:196), withoutForgeChecks drops only github_auth (forge_unreachable.go:19-28), and the health verdict override (health_verdict.go:82-87) is tested against a baseline that proves it changes the outcome.

One mismatch between the UI and the alert rule (src/pkg/hub/assets/dashboard.html:9546, the fcPill assignment):

var fuPill = forgeUnreachableSummary(h); if (fuPill) fcPill = fuPill;

This replaces the whole failing-check pill, not just the github_auth entry. failingCheckSummary (dashboard.html:3857-3879) lists every failing check in its tooltip — disk, inference, etc. — so a hive that is forge-unreachable and has an unrelated hard failure shows only the amber "forge unreachable (host)" pill, while evaluateAlertsInFleet (alerts.go:1148-1150) correctly keeps the non-auth checks and raises health_check_failing for them. Operator sees an amber nudge; the alert feed says a red check is failing. Severity medium: it masks exactly the failures this change says it wants to keep visible ("the pod is healthy"). Suggested shape: filter github_auth out of failingChecks(h) when h.forgeUnreachableLabel is set and render both pills, mirroring withoutForgeChecks.

Open question, not a defect: hiveHealthBase clears StatsStale when ForgeUnreachable is set (health_verdict.go:85). If a beat is both stats-stale (collect timed out) and forge-unreachable, the hub will now assert green/amber health from cached agent stats, which the comment at health_verdict.go:96-100 says it never does. Worth confirming the spoke cannot emit both flags on the same beat after #17434.

Docs/changelog fragment are consistent with the Go behaviour; the advisory-staleness.md table row describes the alert-side rule accurately.

— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Style/maintainability review at head def0269 (read the diff plus server.go, saas_hives_api.go, health_verdict.go, forge_unreachable.go, dashboard.html at head; gofmt-clean).

1. fuPill is the only state pill in the row not gated on h.online, and its label claims the hive is healthy — src/pkg/hub/assets/dashboard.html:9554 (var fuPill = forgeUnreachableSummary(h); vs. var fcPill = h.online ? … : '' and var advPill = h.online && !fuPill ? … : '' on the same line). markStaleHives (src/pkg/hub/server.go:4457-4473) flips Online off but leaves the last beat's ForgeUnreachable intact, and forgeUnreachableLabel (src/pkg/hub/forge_unreachable.go:7-14) does not look at Online, so a hive that stopped beating after reporting the flag keeps rendering "forge unreachable (host)" with the tooltip "the hive itself is healthy" (dashboard.html:3897) on an offline row. The sibling pills that are deliberately NOT online-gated (dlPill, iaPill) each carry a comment saying why; this one has no such rationale. Suggested fix (either): gate it like its siblings — var fuPill = h.online ? forgeUnreachableSummary(h) : ''; — or add || !e.Online to the early return in forgeUnreachableLabel so the gating lives in Go like the advisoryStale rule the surrounding code cites.

2. ForgeUnreachableLabel is inserted between the AdvisoryStale / AdvisoryStaleReason pair — src/pkg/hub/saas_hives_api.go:174-178. The comment directly above (:167-173) describes the two advisory fields together ("Both stay zero/empty…"), so the new field now sits inside a documented pair and reads as if the comment covers it. Move it below AdvisoryStaleReason.

Advisory, no change needed: the amberNudgePill extraction (dashboard.html:3900-3908) also switches the stale-advisory tooltip from esc to escAttr, which is the correct escaper for an attribute — worth a word in the PR body since it is a behavioural touch to an existing pill.

— hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Read this against the v5 tree (both ends of the wire) rather than just the diff; it looks correct to me.

Verified

• Wire name matches both sides: spoke src/pkg/hub/spoke/heartbeat.go:859 emits json:"forge_unreachable,omitempty", filled from b.dashSrv.ForgeUnreachable() at src/cmd/hive/main.go:4765 (merged in #17434); the hub HeartbeatPayload here reads the same key and copies it into RegistryEntry in handleHeartbeat (server.go). Because the entry is rebuilt per beat, a later beat without the flag clears it — no sticky state.
• forgeUnreachableLabel (forge_unreachable.go:10) requires e.Online, so once markStaleHives flips a hive offline the carried-over flag cannot make the row claim "healthy apart from forge egress". buildMyHivesResponse computes the label on read, same as the advisory pill.
• hiveHealthBase (health_verdict.go:86) only replaces app with ghAppBucketUnknown (defined github_app_health.go:14); the !e.Online and e.StatsStale unknown-gates still run before any green verdict, so this cannot assert health for a hive the hub can't see. The ghAppBucketBroken red at line ~123 is the only path the override suppresses, which is the stated intent.
• withoutForgeChecks drops only healthCheckGitHubAuth (alerts.go:876); other failing checks (disk, etc.) still raise the alert. The JS failingChecks filter in dashboard.html mirrors it by name.
• advisoryStale short-circuits to false, "" — consistent with the existing "unknown never alarms" rule documented above it.

Tests in forge_unreachable_test.go cover each of the five branches. Body matches the diff (Closes #17433, hub side of #17427 ask 3).

— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[github-actions]

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback