Skip to content

feat: split apply health lane reporting#397

Merged
steipete merged 3 commits into
openclaw:mainfrom
brokemac79:codex/apply-lane-reporting
Jul 4, 2026
Merged

feat: split apply health lane reporting#397
steipete merged 3 commits into
openclaw:mainfrom
brokemac79:codex/apply-lane-reporting

Conversation

@brokemac79

Copy link
Copy Markdown
Contributor

Dependency: this PR is stacked on #391 (codex/problem-focused-apply-health). Please land #391 first, then rebase/review this follow-up. Until then, GitHub may show the #391 commits in the diff.

What Problem This Solves

Apply runs can close records and refresh durable review comments in the same applicator surface. Before this change, the status summary and dashboard counters made it harder to tell whether a run was doing closure work, comment-sync work, or both. A run that only refreshed comments could look like a close apply run with no pruning progress.

Why This Change Was Made

  • Adds lane-level apply health under lanes.closure and lanes.comment_sync while preserving the existing top-level counts for compatibility.
  • Classifies close actions, closure skip reasons, successful durable review comment syncs, stale comment-sync skips, and comment-write auth/locked failures into the correct lane.
  • Parses lane health in the dashboard status endpoint.
  • Keeps the visible dashboard low-noise by reusing the existing closed and comments synced pills, with clearer hover text showing lane-specific processed counts.
  • Documents that closure processing and durable review-comment sync are reported separately even when they share the applicator.

User Impact

Maintainers can now distinguish actual pruning throughput from durable comment maintenance. A zero-close run that is only doing comment sync should no longer look like failed pruning, and comment-write failures are attributed to the comment-sync lane instead of closure.

Evidence

  • pnpm run build:all passed after the accepted review fix.
  • node --test test\repair\workflow-utils.test.ts test\dashboard-worker.test.ts passed: 72 passed, 0 failed.
  • pnpm run format:check -- src/repair/workflow-utils.ts test/repair/workflow-utils.test.ts dashboard/worker.ts test/dashboard-worker.test.ts docs/live-dashboard.md passed.
  • PNPM_BIN=$env:LOCALAPPDATA\Corepack\shims\pnpm.CMD; pnpm run check passed after the review fix: 1135 passed, 1 skipped, coverage lane 100%, format clean.
  • Direct workflow-utility proof used pnpm run --silent workflow -- summarize-apply-report ... against a close-mode report with one close, one closure skip, one comment sync, and two comment-write skips. Output showed status=ok, processed=5, closed=1, comment_synced=1, closure lane {"processed":2,"closed":1,"comment_synced":0,"skipped":1,"skip_reasons":{"skipped_changed_since_review":1}}, and comment-sync lane {"processed":3,"closed":0,"comment_synced":1,"skipped":2,"skip_reasons":{"skipped_comment_auth":1,"skipped_locked_conversation":1}}.
  • Codex review initially found one accepted issue: comment-write auth/locked skips were classified as closure in close-mode runs. That was fixed and covered by regression assertions. The rerun codex.cmd -c service_tier='"'"'fast'"'"' review --uncommitted reported no actionable correctness issues.

Screenshot Proof

No new dashboard card or always-visible metric is added in this PR. The existing #391 pruning alert remains the visual surface; this follow-up changes the status JSON and the hover text on the existing closed and comments synced pills so the dashboard stays quiet unless #391 already decides the apply health needs attention.

Risk Notes

  • This does not loosen close/apply gates.
  • This does not dispatch repair, review, close, or comment actions by itself.
  • Existing top-level processed, closed, comment_synced, and skip_reasons fields remain unchanged for downstream consumers.

@brokemac79 brokemac79 requested a review from a team as a code owner July 3, 2026 22:35
@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 9:56 PM ET / 01:56 UTC.

Summary
The PR adds closure and comment-sync lane summaries to apply-health JSON, parses and displays those lane counts in the dashboard, and updates docs plus focused workflow/dashboard tests.

Reproducibility: not applicable. as a feature PR. Source review confirms current main lacks lane-level apply-health output while PR head adds and tests it.

Review metrics: 2 noteworthy metrics.

  • Changed surface: 5 files, +232/-8. The PR touches both the apply-health producer and dashboard consumer for live automation telemetry.
  • Lane buckets added: 2 lane summaries. The durable status JSON now separates closure and comment-sync accounting while retaining top-level compatibility fields.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Risk before merge

  • [P1] This changes live apply-health status JSON and dashboard hover telemetry; a lane classifier regression could mislead maintainers about whether closure or durable comment-sync work is blocked, even though the top-level fields remain compatible.

Maintainer options:

  1. Merge after normal automation review (recommended)
    Maintainers can merge after required checks and review, accepting the low but real telemetry risk because the PR preserves top-level fields and adds focused lane tests.
  2. Ask for a dashboard smoke if desired
    Maintainers who want extra confidence can request a quick dashboard/status JSON smoke, but the current PR-body terminal proof and tests are already sufficient for the changed producer path.

Next step before merge

  • No automated repair is needed; the previous lane-accounting issue is addressed and the remaining action is normal maintainer review/check gating.

Security
Cleared: No concrete security or supply-chain concern was found; the diff adds no dependencies, workflow permissions, secret handling, action refs, or third-party execution path.

Review details

Best possible solution:

Land the lane split after normal maintainer review and required checks, preserving the existing top-level fields and focused tests for close-mode durable-comment side effects.

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

Not applicable as a feature PR. Source review confirms current main lacks lane-level apply-health output while PR head adds and tests it.

Is this the best way to solve the issue?

Yes. The PR is a narrow follow-up to the merged apply-health surface, keeps existing top-level fields for compatibility, and now reuses the durable review-comment sync classifier for lane counts.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 279d3bd60f3b.

Label changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied live output from the changed workflow utility showing after-fix lane JSON, plus focused test, format, and full-check results.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This is a normal-priority automation observability improvement with limited blast radius and focused validation.
  • merge-risk: 🚨 automation: The diff changes apply-health JSON and dashboard telemetry used to monitor apply automation progress.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied live output from the changed workflow utility showing after-fix lane JSON, plus focused test, format, and full-check results.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live output from the changed workflow utility showing after-fix lane JSON, plus focused test, format, and full-check results.
Evidence reviewed

What I checked:

Likely related people:

  • brokemac79: Merged current-main apply-health dashboard/summarizer work and apply queue rotation work central to this PR's follow-up lane reporting. (role: feature owner and recent area contributor; confidence: high; commits: ead7d6461a6c, 5bdf6f31c904; files: src/repair/workflow-utils.ts, dashboard/worker.ts, .github/workflows/sweep.yml)
  • Vincent Koc: History for comment-write auth/locked skip handling points to adjacent apply-result behavior that this PR classifies into the comment-sync lane. (role: adjacent contributor; confidence: medium; commits: c759f49d3c20; files: src/clawsweeper.ts)
  • Mason Huang: Recent stale review-comment sync protection introduced skipped_stale_review_comment_sync, one of the comment-sync lane skip actions handled here. (role: recent adjacent contributor; confidence: medium; commits: f7e8b13d0eb3; files: src/clawsweeper.ts)
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.

@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 3, 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.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. 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 3, 2026
@brokemac79 brokemac79 force-pushed the codex/apply-lane-reporting branch from cad6163 to 59aae05 Compare July 4, 2026 01:18
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 4, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 4, 2026
@steipete steipete self-assigned this Jul 4, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 4, 2026
@steipete

steipete commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Maintainer review complete on exact head bd5dc2488cdbec8f850ede916763f73b7d77c198.

This cleanly separates closure and durable-comment-sync health without changing the existing aggregate status contract. The final fix also preserves comment refresh counts performed during closure processing and exposes that split in the dashboard tooltip.

Proof:

  • pnpm run build:all
  • GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=commit.gpgsign GIT_CONFIG_VALUE_0=false node --test test/repair/workflow-utils.test.ts test/dashboard-worker.test.ts — 76/76 passed
  • whole-branch autoreview — no accepted/actionable findings
  • exact-head CI: pnpm check, Windows Codex launcher, CodeQL, and Socket checks all green

No known proof gaps for this reporting-only change.

@steipete steipete merged commit 5f4af59 into openclaw:main Jul 4, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants