feat: [HIMMEL-654] leg-3 batch — GLM startup watchdog, lane readiness, session telemetry, alert rules, review-freshness gate, PHI fence hardening (8 PRs) - #556
Conversation
…, session telemetry, alert rules, review-freshness gate, PHI fence hardening (8 PRs) Batch propagation of the HIMMEL-654 dispatch-session leg-3 merges (private #1612, #1614-#1620): - HIMMEL-1575: GLM startup-hang fail-fast — transcript-growth watchdog + failure_class accounting (default 10-min window, GLM_STARTUP_FAILFAST_MINS). - HIMMEL-1626: guard lane-READINESS, measured from the verify-return flow-runs ledger — lane-readiness.mjs probe, lanes.json readiness.passesRequired gates (glm 10, claudex 5), lane_ready() in guard-implementor-dispatch (fail-open on probe failure; a clean `down` skips the lane like a spent bank). - HIMMEL-1629: cli-proxy-api pin 7.2.115 -> 7.2.122 (fork-drift). - HIMMEL-1052: live session + subagent telemetry — session-runs ledger + hook, six exporter metric families, /sessions.json, war-room Sessions row (passive-reader charter intact). - HIMMEL-845: cross-drive X:tail fail-open closed FAIL-CLOSED in both PHI fence twins (lesson-write-fence, graphify-fence) + regression tests. - HIMMEL-924: nine Grafana/Prometheus alert rules + Telegram delivery (D18); HIMMEL-918 coverage invariant; promtool tests + provisioning. - HIMMEL-1181: review-freshness gate — cr-review-freshness.sh; a stale bot review anchor hard-blocks both check-ci modes (exit 4) and the merge gate (rc 2); body findings surfaced, never blocking. - HIMMEL-1631: already public via the #554 reship (test-worker-lifecycle ARM_TEMP_CWD_OK shield) — carried as a verified skip-path. All eight passed the private gate (critic panel + suites + check-ci) and merged to private main 2026-08-07.
📝 WalkthroughWalkthroughThe pull request adds CodeRabbit freshness gates, fail-closed Windows path handling, lane readiness checks, session observability and alerting, a GLM startup watchdog, and a CLIProxyAPI update. ChangesCodeRabbit review gates
Windows path guardrails
Lane readiness routing
Session observability and alerting
GLM startup watchdog
CLIProxyAPI version
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
scripts/observability/flow-exporter.test.ts (1)
1192-1219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a route-level test for
GET /sessions.json.The tests call
renderSessionsJsondirectly. The new branch instartFlowExporter'sfetchhandler (flow-exporter.tslines 1417-1428) stays untested. That branch also changed the 404 condition and sets theapplication/jsoncontent type. A regression there would ship undetected.Add one test that starts the exporter, fetches
/sessions.json, and asserts the status, the content type, and a parsed body. Assert that an unknown path still returns 404.🤖 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 `@scripts/observability/flow-exporter.test.ts` around lines 1192 - 1219, Add a route-level test that starts the exporter and requests /sessions.json through its fetch handler, asserting a successful status, application/json content type, and parsed session response body. Also request an unknown path and assert it returns 404, covering the routing changes in startFlowExporter.
🤖 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 `@scripts/check-ci.sh`:
- Around line 49-55: Update the comment above the freshness requirement to point
to the local review_freshness_gate function below, and identify
cr_review_freshness from scripts/lib/cr-review-freshness.sh as the library
reader.
In `@scripts/lanes/lane-readiness.mjs`:
- Around line 77-81: Update the validation in the readiness mapping around
passesRequired to require a positive integer, treating fractional values as
ungated and returning ready before trailingPasses is evaluated; preserve the
existing handling for non-numeric, non-finite, and non-positive values. Add a
lane-readiness test covering a fractional passesRequired value.
In `@scripts/observability/alerts.rules.yml`:
- Around line 45-55: The HimmelFlowRunStalled alert currently fires on
cumulative stalled outcomes and can remain active after the stalled run is no
longer current. Update the alert expression or the metric emitted by
foldFlowLedger so it represents only currently stalled runs, ensuring the alert
clears when no run remains stalled while preserving detection of active stalled
runs.
In `@scripts/observability/prometheus.yml`:
- Around line 10-11: Update the installation flow in install-stack.ps1 to copy
alerts.rules.yml alongside prometheus.yml into $stateRoot, preserving the
configured rule_files reference so the installed Prometheus process can load the
rules.
In `@scripts/observability/provisioning/alerting/rules.yaml`:
- Line 33: Replace the rolling flow_run_outcome_total increase() expressions for
truncated and error alerts in
scripts/observability/provisioning/alerting/rules.yaml at lines 33 and 61 with
an event-time metric or truly monotonic persisted counter, and apply the
identical corrected expression in scripts/observability/alerts.rules.yml. In
scripts/observability/alerts.rules.test.yml lines 16-50, add an aging-out
sequence such as 2 to 1 and assert it does not trigger a new alert.
In `@scripts/observability/session-run-ledger.ts`:
- Around line 117-124: Update jsonStr to escape every C0 control character below
U+0020, including the existing newline, carriage-return, and tab cases, using
valid JSON escape sequences or Unicode escapes. Preserve the current escaping of
backslashes and quotes so parseSessionLedgerRows can parse strings containing
any control byte.
In `@scripts/setup/cli-proxy-lane.ps1`:
- Around line 65-66: Update the synchronized CLIProxyAPI version and release URL
in scripts/setup/cli-proxy-lane.ps1 lines 65-66 to reference an artifact-backed
release, and update the matching pin in scripts/upstreams.json line 153. Ensure
both locations use the same version and that the Windows amd64 ZIP exists at the
generated release URL.
In `@scripts/telegram/spawn-glm.ts`:
- Around line 1019-1034: The watchdog logic around watchTimer and baseline must
not treat the first observed transcript size as the baseline. Detect assistant
activity already present on the first observation and disarm via the existing
healthy-response path, or initialize reliable size tracking before runSession
can write; add a regression test covering transcript creation and growth before
the first poll followed by inactivity past the fail-fast window.
In `@scripts/upstreams.json`:
- Around line 153-156: Update the note associated with the CLIProxyAPI upstream
row so its version-normalization example matches the current synced_base value
7.2.122, or rewrite the example to avoid hardcoded versions while preserving the
described leading-“v” comparison behavior.
---
Nitpick comments:
In `@scripts/observability/flow-exporter.test.ts`:
- Around line 1192-1219: Add a route-level test that starts the exporter and
requests /sessions.json through its fetch handler, asserting a successful
status, application/json content type, and parsed session response body. Also
request an unknown path and assert it returns 404, covering the routing changes
in startFlowExporter.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e150dd6-ca11-4cfe-afbe-c5d15de84b09
📒 Files selected for processing (37)
.claude/commands/check-ci.md.claude/commands/pr-check.mddocs/internals/enforcement.mdscripts/check-ci.shscripts/guardrails/graphify-fence.shscripts/guardrails/lesson-write-fence.shscripts/guardrails/test-graphify-fence.shscripts/guardrails/test-lesson-write-fence.shscripts/hooks/guard-implementor-dispatch.shscripts/hooks/test-block-unresolved-cr-merge.shscripts/hooks/test-guard-implementor-dispatch.shscripts/lanes/lane-readiness.mjsscripts/lanes/lanes.jsonscripts/lanes/tests/lane-readiness.test.mjsscripts/lib/cr-merge-gate.shscripts/lib/cr-review-freshness.shscripts/lib/test-cr-merge-gate.shscripts/observability/README.mdscripts/observability/alerts.rules.test.ymlscripts/observability/alerts.rules.ymlscripts/observability/dashboards/war-room-system.jsonscripts/observability/flow-exporter.test.tsscripts/observability/flow-exporter.tsscripts/observability/install-stack.ps1scripts/observability/prometheus.ymlscripts/observability/provisioning/alerting/contact-points.yamlscripts/observability/provisioning/alerting/policies.yamlscripts/observability/provisioning/alerting/rules.yamlscripts/observability/provisioning/datasources/prometheus.yamlscripts/observability/session-run-hook.tsscripts/observability/session-run-ledger.test.tsscripts/observability/session-run-ledger.tsscripts/setup/cli-proxy-lane.ps1scripts/telegram/spawn-glm.test.tsscripts/telegram/spawn-glm.tsscripts/test-check-ci.shscripts/upstreams.json
…ometheus.yml Addresses the CodeRabbit finding on this PR: prometheus.yml's rule_files entry is relative to the config location, so the rule file must travel with the config copy — without it the installed Prometheus refuses its config and the HIMMEL-924 native rules are inert after install. Same idempotent -Force Copy-Item pattern as the config copy. Private: PR #1621, merged (d0a428b2).
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/observability/install-stack.ps1 (1)
766-775: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRemove obsolete provisioning files on reinstall.
Copy-Item -Recurse -Forcemerges into$grafanaProvisioning. It does not remove files deleted or renamed inscripts/observability/provisioning. Grafana can continue loading obsolete alert rules or contact points after a rerun.If this directory is installer-owned, replace it from a clean staging directory or remove stale files before copying.
🤖 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 `@scripts/observability/install-stack.ps1` around lines 766 - 775, Update the provisioning installation block using $grafanaProvisioning so each reinstall starts from a clean directory, removing existing installer-owned contents before recursively copying the current provisioning contents from $scriptDir. Preserve the existing directory creation and content-copy behavior while ensuring deleted or renamed source files are not retained.
🤖 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.
Nitpick comments:
In `@scripts/observability/install-stack.ps1`:
- Around line 766-775: Update the provisioning installation block using
$grafanaProvisioning so each reinstall starts from a clean directory, removing
existing installer-owned contents before recursively copying the current
provisioning contents from $scriptDir. Preserve the existing directory creation
and content-copy behavior while ensuring deleted or renamed source files are not
retained.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e96c33b-3471-4223-8d91-cd0ceac316d3
📒 Files selected for processing (1)
scripts/observability/install-stack.ps1
HIMMEL-654 leg-3 batch (8 private PRs)
Propagates the 2026-08-07 dispatch-session leg-3 merges (private #1612, #1614–#1620) onto the freshly-merged wave-2k base:
failure_class="startup-hang"accounting (GLM_STARTUP_FAILFAST_MINS, 0=off)scripts/lanes/lane-readiness.mjs,lanes.jsonreadiness.passesRequiredgates (glm 10 / claudex 5),lane_ready()in the dispatch guard — a ruled-down lane is skipped like a spent bank; fail-open on every probe failure/sessions.json, war-room Sessions row (exporter stays a pure reader)X:tailfail-open closed FAIL-CLOSED in both PHI fence twins + lockstep regression tests--skip-pathEvery item passed the private gate (critic panel, per-suite runs, check-ci) and merged to private main today. Public notes: the HIMMEL-1632 hardening batch (from #554's public CR pass) is tracked privately and will follow the same private-first path.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation