Skip to content

feat(dashboard): expose apply lane health telemetry#732

Open
brokemac79 wants to merge 1 commit into
mainfrom
codex/csw-053-apply-lane-telemetry
Open

feat(dashboard): expose apply lane health telemetry#732
brokemac79 wants to merge 1 commit into
mainfrom
codex/csw-053-apply-lane-telemetry

Conversation

@brokemac79

Copy link
Copy Markdown
Contributor

Summary

Adds a third, disjoint Apply / close health surface to the ClawSweeper Overview. Review admission and result publication remain unchanged.

Problem

The apply/close lane had no independently observable Overview health surface during the incident. PR #730 added its 900-second state-lease acquisition override, while #731 restored the normal review/result fallback to 480 seconds; neither owns durable apply-lane dashboard telemetry.

Implementation

  • Persists signed, seven-day apply-observability v1 events in the dashboard StatusStore and exposes GET /api/apply-observability?range=6h|24h|7d.
  • Adds an Overview card with terminal queue snapshot, 15m/60m flow, range totals, retry amplification, lease/failure signals, last failure, and range controls.
  • Uses an explicit APPLY_TARGET_REPOS contract; all-target aggregates exclude non-apply targets and remain unknown until every configured apply target reports.
  • The producer runs after apply action-ledger/state publication, records terminal active/capacity truthfully, and publishes failures for workflow, action-ledger, state publication, and safe-close data only when observed.
  • Legacy state has no durable arrival/retry/dead-letter/state-lease timing ledger. Those values remain unknown; no null/fallback value is presented as healthy zero.

Accounting boundaries and data sources

Apply telemetry is a separate signed per-run record keyed by target repo, workflow run ID, and attempt. It reads the final durable apply-health report (or durable idle report), workflow terminal outcomes, and action-ledger/state-publication outcomes. It does not reuse review-admission or result-publication counters.

Validation

  • node --test test/apply-observability.test.ts test/dashboard-worker.test.ts — 161 passing on final rerun.
  • pnpm run build:all
  • pnpm run lint:dashboard
  • pnpm run lint:scripts
  • pnpm exec oxfmt --check .github/workflows/sweep.yml dashboard/worker.ts dashboard/wrangler.toml dashboard/apply-observability.ts scripts/publish-apply-observability.mjs test/apply-observability.test.ts test/dashboard-worker.test.ts
  • git diff --check
  • node --test test/sweep-workflow.test.ts — structural assertions passed; five existing helper cases remain blocked on this host because its WSL relay has no /bin/bash.

Rendered/UI proof

Dashboard worker/render tests exercise populated, empty/unknown, degraded, signed-storage, API, range, and card rendering paths. Docker Desktop was healthy (29.5.2) and local Crabbox sync completed, but browser-container proof was concretely blocked by the Windows PowerShell --script-stdin BOM behavior plus the Playwright image's pnpm layout. No external Testbox was used.

Risks and rollout

The card starts as Awaiting producer until the first apply event. Unknown legacy fields intentionally remain unknown, so rollout may reveal telemetry gaps rather than a false healthy state. No queue capacity, retry, dispatch, workflow rerun, gate, or incident-control behavior changes.

Review closeout

  • Dirty-patch review: codex review --uncommitted — clean; no actionable correctness issues.
  • Branch review: git fetch origin main && codex review --base origin/main — clean; telemetry pipeline, durable storage API, aggregation, rendering, and workflow integration consistent.

@brokemac79
brokemac79 requested a review from a team as a code owner July 21, 2026 02:33
@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jul 21, 2026
@clawsweeper

clawsweeper Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 20, 2026, 11:17 PM ET / July 21, 2026, 03:17 UTC.

Summary
Adds signed apply-lane observability events, a dashboard API/card, and sweep workflow publication wiring for apply-lane health telemetry.

Reproducibility: yes. from source: provide ACTION_LEDGER_OUTCOME=success or a successful state-publication outcome to the producer and inspect the emitted observed_failure_kinds; it incorrectly contains the matching failure kind.

Review metrics: 1 noteworthy metric.

  • Telemetry surface: 7 files affected; 858 added, 0 removed. The new workflow producer, durable storage API, aggregation, and UI all need consistent failure semantics.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Fix the inverted outcome checks and add a focused healthy-success regression test.
  • Attach redacted proof from a real apply workflow run showing the signed event and its rendered API/card result.
  • After updating the PR body, wait for the automatic fresh review or ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR provides tests but no attached after-fix dashboard or workflow evidence; the body says browser-container proof was blocked. Please attach redacted live event publication plus resulting API/card output, omitting private endpoints, tokens, IPs, and other sensitive details. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Merging this branch would mark normal successful ledger and state-publication outcomes as failures, creating persistent false degraded telemetry and undermining the card’s incident-detection purpose.
  • [P1] The PR lacks after-fix real behavior proof from a dashboard/workflow run; tests alone do not demonstrate that signed events publish and render correctly in the deployed path.

Maintainer options:

  1. Correct health classification before merge (recommended)
    Emit action-ledger and state-publication failure kinds only for actual failed outcomes, cover healthy-success events, and provide redacted live publication/render proof.
  2. Pause telemetry rollout
    Do not merge the dashboard telemetry until its failure semantics and deployed behavior are demonstrated reliably.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Fix the producer's inverted success/failure checks, add regression coverage for healthy successful outcomes, and attach redacted live apply-event plus dashboard/API proof.

Next step before merge

  • [P1] A narrow mechanical producer fix and regression test can resolve the blocking correctness defect; contributor-provided real behavior proof remains required before merge.

Security
Cleared: The patch uses the existing signed webhook pattern and introduces no concrete new permission, secret-exposure, dependency, or third-party execution concern.

Review findings

  • [P1] Emit failures only when the observed step failed — scripts/publish-apply-observability.mjs:39-44
Review details

Best possible solution:

Correct the producer so each failure kind is emitted only when its corresponding observed step failed, add focused regression coverage for successful outcomes, then attach a redacted real apply run and resulting dashboard/API output.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: provide ACTION_LEDGER_OUTCOME=success or a successful state-publication outcome to the producer and inspect the emitted observed_failure_kinds; it incorrectly contains the matching failure kind.

Is this the best way to solve the issue?

No. The proposed telemetry architecture may be viable, but the producer must classify only failed observed steps as failures and show the deployed publish/render path before this is merge-ready.

Full review comments:

  • [P1] Emit failures only when the observed step failed — scripts/publish-apply-observability.mjs:39-44
    This condition treats both success and failure as action_ledger_failure; the following state-publication condition repeats the inversion. Consequently, every healthy run with completed publication is stored and rendered as degraded telemetry. Restrict these branches to === "failure" and add a success-case producer regression test.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8c3f96952284.

Label changes

Label justifications:

  • P2: A faulty operational-health feature can mislead maintainers during incidents, but it does not change apply-lane execution itself.
  • merge-risk: 🚨 other: The inverted outcome checks would create false failure telemetry that green CI does not catch in the deployed workflow path.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR provides tests but no attached after-fix dashboard or workflow evidence; the body says browser-container proof was blocked. Please attach redacted live event publication plus resulting API/card output, omitting private endpoints, tokens, IPs, and other sensitive details. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

Acceptance criteria:

  • [P1] node --test test/apply-observability.test.ts test/dashboard-worker.test.ts.
  • [P1] pnpm run check.

What I checked:

Likely related people:

What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-07-21T02:36:10.974Z sha b306079 :: needs real behavior proof before merge. :: [P1] Emit failure kinds only for failed producer inputs

@clawsweeper clawsweeper Bot added merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. and removed merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant