Skip to content

Handle delayed proof nudge schedules#249

Merged
Takhoffman merged 1 commit into
mainfrom
codex/proof-nudge-delayed-schedule
Jun 3, 2026
Merged

Handle delayed proof nudge schedules#249
Takhoffman merged 1 commit into
mainfrom
codex/proof-nudge-delayed-schedule

Conversation

@Takhoffman
Copy link
Copy Markdown
Contributor

Summary

  • make the scheduled proof-nudge guard tolerate delayed GitHub schedule starts
  • use the triggering cron string plus the current America/Chicago zone abbreviation to choose the seasonal 5 AM Central candidate
  • update docs and the workflow guard test for the delayed-schedule behavior

Root cause

The 5 AM Central guard added in #246 checked the runner's actual local hour. GitHub started today's scheduled runs late, so both candidates skipped even though the feature was enabled:

Real behavior proof

Local proof for the new decision rule, including today's delayed starts:

0 10 * * * started 2026-06-03T11:35:03Z -> 06:35 CDT should_run=true
0 11 * * * started 2026-06-03T12:32:58Z -> 07:32 CDT should_run=false
0 10 * * * started 2026-12-03T11:35:03Z -> 05:35 CST should_run=false
0 11 * * * started 2026-12-03T12:32:58Z -> 06:32 CST should_run=true

This keeps exactly one seasonal candidate live even if GitHub queues the run late.

Validation

  • node --test --test-name-pattern "proof nudge workflow" test/clawsweeper.test.ts
  • pnpm run format:check
  • node -e delayed schedule decision proof above

@Takhoffman Takhoffman requested a review from a team as a code owner June 3, 2026 13:27
@Takhoffman
Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Jun 3, 2026

🦞🧹
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
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Jun 3, 2026

Codex review: needs maintainer review before merge. Reviewed June 3, 2026, 9:32 AM ET / 13:32 UTC.

Summary
The branch changes the proof-nudges workflow guard to choose the live UTC cron candidate from github.event.schedule plus the Central timezone abbreviation, and updates the proof-nudge docs and workflow guard test.

Reproducibility: yes. Current main's workflow checks the actual Central hour, and the PR body's delayed 06:35/07:32 CDT starts would both fall outside 05, so the skip behavior is reproducible from source inspection.

Review metrics: 2 noteworthy metrics.

  • Automation surface: 1 workflow changed. The behavior change is in scheduled GitHub Actions gating, where green CI cannot fully prove the live schedule event context.
  • Patch scope: 3 files, +11/-6. The workflow change is narrow and paired with one docs update and one focused guard-test update.

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:

  • Verify the first post-merge scheduled dry-run report keeps exactly one seasonal cron candidate live.

Risk before merge

  • [P1] This changes live GitHub Actions schedule gating; CI and local proof cover the decision rule, but the first post-merge scheduled dry-run is still the only full end-to-end Actions confirmation.

Maintainer options:

  1. Merge and monitor scheduled dry-run (recommended)
    Accept the narrow automation risk after required checks pass, then verify the next scheduled dry-run keeps only the CDT or CST candidate live.
  2. Hold for live Actions evidence
    Pause if maintainers want actual scheduled-workflow evidence before merge, since the PR cannot fully prove GitHub's schedule context locally.

Next step before merge

  • [P2] No repair lane is needed; the patch is narrow, reviewed as correct, and the remaining action is maintainer merge/monitoring rather than an automated code change.

Security
Cleared: The diff changes a scheduled workflow guard, docs, and a static test without adding third-party actions, broadening permissions, or exposing secrets.

Review details

Best possible solution:

Land the narrow guard update with the paired docs/test change, then confirm the first scheduled dry-run report shows only the seasonal 5 AM Central candidate continues.

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

Yes. Current main's workflow checks the actual Central hour, and the PR body's delayed 06:35/07:32 CDT starts would both fall outside 05, so the skip behavior is reproducible from source inspection.

Is this the best way to solve the issue?

Yes. Using the triggering cron string plus the current Central zone abbreviation is a narrow fix that preserves the existing two-cron DST design while removing sensitivity to GitHub queue delay.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 9c50a95427c2.

Label changes

Label changes:

  • add P2: This fixes a concrete proof-nudge scheduling bug with limited blast radius in an optional automation lane.
  • add merge-risk: 🚨 automation: The diff changes a scheduled GitHub Actions guard, so an incorrect mapping could skip or run proof-nudge automation at the wrong seasonal time.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live-style local output for the new decision rule across delayed CDT and CST scenarios, plus the focused validation commands.
  • 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-style local output for the new decision rule across delayed CDT and CST scenarios, plus the focused validation commands.

Label justifications:

  • P2: This fixes a concrete proof-nudge scheduling bug with limited blast radius in an optional automation lane.
  • merge-risk: 🚨 automation: The diff changes a scheduled GitHub Actions guard, so an incorrect mapping could skip or run proof-nudge automation at the wrong seasonal time.
  • 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-style local output for the new decision rule across delayed CDT and CST scenarios, plus the focused validation commands.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live-style local output for the new decision rule across delayed CDT and CST scenarios, plus the focused validation commands.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md is 90 lines and was read in full; its conservative workflow-review and no-CHANGELOG-normal-PR guidance was applied. (AGENTS.md:1, 9c50a95427c2)
  • Current main root cause: Current main checks the runner's actual America/Chicago hour and only sets should_run=true when that hour is 05, so a delayed scheduled start at 06:35 or 07:32 Central skips. (.github/workflows/proof-nudges.yml:62, 9c50a95427c2)
  • PR workflow patch: The PR patch adds PROOF_NUDGES_EVENT_SCHEDULE, reads github.event.schedule, switches the guard to date +%Z, and maps 0 10 * * * to CDT and 0 11 * * * to CST. (.github/workflows/proof-nudges.yml:60, a734a88359b6)
  • PR docs/test patch: The same patch updates proof-nudge docs for delayed GitHub starts and replaces the workflow test's local_hour assertion with assertions for event schedule, zone abbreviation, and both cron candidates. (test/clawsweeper.test.ts:16088, a734a88359b6)
  • Real behavior proof: The PR body includes copied local output for June delayed starts and December delayed starts, showing exactly one seasonal candidate returns should_run=true under the new rule. (a734a88359b6)
  • Feature history: git blame ties the current local-hour guard to the merged 5 AM Central work in 9c50a95, while the proof-nudge workflow/test surface dates back to 9b08258. (.github/workflows/proof-nudges.yml:56, 9c50a95427c2)

Likely related people:

  • Takhoffman: Authored the merged 5 AM Central schedule guard in 9c50a95 and the adjacent 5 AM schedule setup in 71e4da1, which are the direct behavior this PR adjusts. (role: recent area contributor; confidence: high; commits: 9c50a95427c2, 71e4da1fa1a5; files: .github/workflows/proof-nudges.yml, docs/proof-nudges.md, test/clawsweeper.test.ts)
  • brokemac79: Introduced the proof-nudge workflow, docs, command surface, and tests in 9b08258, so they are a relevant routing candidate for proof-nudge automation behavior. (role: feature introducer; confidence: high; commits: 9b08258b9c64; files: .github/workflows/proof-nudges.yml, docs/proof-nudges.md, 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.

@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 Jun 3, 2026
@Takhoffman Takhoffman merged commit 74f63a0 into main Jun 3, 2026
4 of 6 checks passed
@Takhoffman Takhoffman deleted the codex/proof-nudge-delayed-schedule branch June 3, 2026 13:33
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.

1 participant