Fix automerge terminal telemetry convergence#652
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 12:14 PM ET / 16:14 UTC. Summary Reproducibility: yes. The merge of #648 is a concrete current-main production case where durable command records show terminal completion but the metrics service retained only activation. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Operate the bounded reconciliation as an isolated telemetry repair and confirm in production that the existing #648 session converges exactly once without affecting automerge execution. Do we have a high-confidence way to reproduce the issue? Yes. The merge of #648 is a concrete current-main production case where durable command records show terminal completion but the metrics service retained only activation. Is this the best way to solve the issue? Yes, provisionally. Bounded reconciliation is narrower than introducing a second durable outbox and can repair both the existing missed event and future ambiguous delivery failures, while production convergence still needs observation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ea22ada84dd0. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Root cause evidence
2026-07-17T13:39:18Zthrough https://github.com/openclaw/clawsweeper/actions/runs/29584167178.mergemutation and terminal command completion. The same router batch contained the original automerge activation, so theterminal: mergedevent generation conditions were satisfied.What changed
merged_atalways takes precedence over genericclosed_at, and the existing first-terminal immutable projection remains unchanged.continue-on-error, so GitHub, status API, or ingest failures cannot affect automerge.Time-window coverage, show active-session count explicitly, and renderNo terminal samples yetinstead of zero-like KPI cards when the denominator is empty.Why reconciliation instead of an outbox
The durable active-session ledger already identifies sessions whose terminal delivery did not converge. Rechecking bounded authoritative PR status repairs both the existing missing terminal for #648 and future transport/timeout/response-loss cases. A new outbox would protect only failures after deployment and would still require a second backfill mechanism for already-active merged sessions.
This does not perform broad history backfill: candidates must have activity within 24 hours, the API response and GitHub reads are count-bounded, and open PRs remain active.
Validation
node --test test/automerge-metrics.test.ts— 14 passedpnpm run build:dashboard— passednode --test test/dashboard-worker.test.ts— 136 passedpnpm run check— passed on Nodev24.15.0$AUTOREVIEW --mode local --stream-engine-output— clean, no accepted/actionable findings;patch is correct(confidence 0.86)