Repository navigation
pass: Keep an idle pass's early re-plan to the backoff after an empty plan - #554
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
VERDICT: APPROVE: the idle early re-plan is now gated on the last applied plan having had actions; the wait itself and its bound are unchanged.
Reviewed head: e6834a2 (merge-base f1dd2c7).
The change only removes a path; it adds none. decide (crates/board-forge/src/pass/epics.rs:767-781) still computes the same backoff(DAY_SECONDS, empty, cap) capped by triageFollowUpDays. It now also sets early = empty == 0. The scheduler (epics.rs:1352-1355) puts early && ahead of the existing waited/busy/turn.planned conditions. This can only make the pass start fewer runs, never more, and it changes no writes, permissions or bounds. Because early is checked first, an epic that is not eligible does not take the single waited slot, so a later epic that is eligible can still go early. That is correct. Operator direction and changed direction still return Next::Plan before the wait is reached, so the operator can still bypass it. The Forgejo scheduler matches Next::Wait { hold, .. } (forgejo/epics.rs:635 on main) and never re-plans early, so the new field compiles there and changes nothing.
The worst merged outcome is an epic that sits idle until its doubled backoff ends (at most triageFollowUpDays) even though slots are free. That is the behaviour this PR intends.
Follow-ups
- nit: docs/pass.md:2041-2044: the added sentence leaves a line well past the paragraph's wrap width (
...go early again. An epic whose children are all closed...). Rewrap it. - nit: crates/board-forge/src/pass/epics/tests.rs:2267: the replay test checks
NOT_EARLYby matching on the row name prefix (name.starts_with("its last plan had nothing")). Renaming the row would silently skip that check. A field in the row table would be sturdier. - nit: the branch is based on f1dd2c7, not on main (2ccc28d). Main has since changed docs/pass.md and CHANGELOG.md. Expect a textual rebase there; I found no semantic conflict.
|
Fix round 1 of 2 for Generated-by: https://github.com/cgwalters/#llms |
|
❌ dispatch failed (agent, check, apply: success/success/failure; the agent's exit state: 0). |
cgwalters-bot
added
the
needs-human
|
The poll stopped following this pull request: fix round 1 is blocked: its run https://github.com/cgwalters-forge/tracker/actions/runs/41230086288 ended (failure) and pushed no new head for e6834a2 (conflicts with main). It is labelled Generated-by: https://github.com/cgwalters/#llms |
… plan An idle pass re-plans an epic ahead of its wait (agent-board#499) as soon as a child closes, since empty slots are cheap. That let tracker#786 be planned 5 times on 31 October, two of those runs ending with no plan changes, and an empty plan is exactly what the wait doubles for: the early start skipped it. So after a plan with no actions the change waits out the doubled interval, saying so in its hold, and then plans the epic as usual. The cap is the existing `triageFollowUpDays`. A plan that acts ends this, and the operator's word still plans at once, so nothing is held longer than the backoff already holds it where slots are full. Note an early re-plan always follows a change since the last receipt, so an exemption for "a change after the empty plan" would exempt every one of them; the change is not lost, it plans when the wait ends. Assisted-by: https://github.com/cgwalters/cgwalters#llms
e6834a2 to
ad50a11
Compare
There was a problem hiding this comment.
VERDICT: APPROVE: gates the idle-pass early re-plan on the existing empty-plan count; no new write path, bound or permission
Reviewed admitted head ad50a11c7cc159cc7749948af726d6847d79db1b against base 89f4546 (merge base).
The change only narrows behaviour. decide (crates/board-forge/src/pass/epics.rs:773) sets early = empty == 0, where empty is the existing count of consecutive applied receipts with no actions, which already drives backoff. The GitHub scheduler (epics.rs:1358) now arms self.waited only when early is true. The worst a merged bug here could do is plan an epic too early (as before this PR) or too late. Too late is still bounded: the epic plans at until, capped by triageFollowUpDays, and operator direction and a changed direction hash return Next::Plan before this point. It adds no dispatch, write or credential path, and it does not touch workflows, manifests or the lockfile. The Forgejo arm (forgejo/epics.rs:635) ignores early with .., and the change cannot affect otherwise() (epics.rs:524).
Follow-ups
- important:
crates/board-forge/src/pass/epics.rs:1358: when the highest-Priority waiting epic is notearly,self.waitedstaysNone, so a lower-Priority epic that is held only by a wait withearlytrue now gets the idle slot. Before this PR it would have lost to the higher one. That outcome is probably what we want, butdocs/pass.md:564("the highest-Priority live epic held only by that wait ... is planned at once, unless its last plan had nothing to do") reads as if nothing is planned early in that case. The replay table inidle_pass_plans_a_waiting_epic_early(epics/tests.rs:2222) has only one epic, so no test fixes either behaviour. Add a two-epic row and say in the doc which behaviour is intended. - nit:
crates/board-forge/src/pass/epics.rs:775:NOT_EARLYis also appended to holds on Forgejo, which never plans early (forgejo/epics.rs:635). There, "; not early, as its last plan had nothing to do" suggests an early re-plan that Forgejo does not have. - nit:
docs/pass.md:2051-2054: the new sentence leavesAn epic whose children are all closed...running on past the wrapped line width. Rewrap the paragraph.
agent-board#499's early re-plan on an idle pass skips the wait after each child close. tracker#786 was re-planned 5 times on 31 October, and two of its runs (receipts of 30 October 17:33 and 31 October 13:10) ended with no actions.
Changed: after an applied plan with no actions an idle pass no longer cuts the wait short. The change waits out the doubled interval, which the hold says (
…; not early, as its last plan had nothing to do), and then plans the epic as usual. A plan with actions lets the next change go early again, and the operator's word still plans at once. Bounded by the existingtriageFollowUpDays.An early re-plan always follows a change since the last receipt, so "allow it after a change" cannot be an exemption here without exempting everything: the change plans when the wait ends.
Effect on tracker#786: its last receipt has no actions and
triageFollowUpDaysis 7, so its next re-plan waits 2 days from its last request unless the operator comments.Verified:
just check, exit 0 (cargo test -p board-forge: 630 passed, 0 failed), with a replay row inidle_pass_plans_a_waiting_epic_earlyand a table in thedecidetest. Not verified: a live pass.Generated-by: https://github.com/cgwalters/#llms