Repository navigation
fix(park): wake a session off the roster when its wait settles - #1844
Conversation
A queued merge, a CI wait that left its call, a park and a note answer now wake their waiter by its session id through agents wake, on the roster or off it. agents wake resolves a role holder or any session by its own id when no roster agent matches; a session that is gone gets no wake and one line, and the issues it served go back to the board. Parks of sessions off the roster live in state.Parks. A park on a pull request also settles on a review or comment by anyone but its author since the park.
…akes * origin/main: fix(agents): stop a parked background session once its turn ends (#1825) fix(capacity): treat an unopened five-hour window as unused (#1808) fix(supervisor): keep the loop's inner-loop states out of the person's decisions (#1828) feat(run): add opt-in worktree builder Jobs (#1811) # Conflicts: # cmd/agentpark.go
teemow
left a comment
There was a problem hiding this comment.
Review: changes requested
Head reviewed: 4b6c67bfdcbf8271f63efe4d3912fc6cac79d8ea
The PR does what #1818 asks. Gate exit 76 and CI-wait owners, parks, and note filers off the roster are now woken by session id through agents wake. A gone session gets one line, and its records go back to the board. Each wait kind has a unit test. At this head, go vet ./cmd ./internal/github ./internal/state is clean, go test ./internal/github ./internal/state passes, and the PR's go test ./cmd -run '…' selection (plus Register) passes. One blocker:
Blocking
-
An off-roster park outlives the session joining the roster, then wakes the agent with a stale "your park is over" (
cmd/agents.go:401registerAgent,cmd/agentpark.go:179,cmd/agentpark.go:665).
Only a newagents parkby the same session (agentpark.go:179) or a resume removes an entry fromstate.Parks.registerAgentnever touchesst.Parks, and neither does anything that ends a roster park (reportIdle, a new task). Here is the failure, reproduced at this head with a throwaway test built on the PR's own helpers (parkingWatch,replyingRefs,stubWakes):offSessionparks ono/r#1.- It registers (
registerAgent) and takes a new task.st.Parksstill has 1 entry. - A comment lands on
o/r#1. w.pendingwakes the now-busy roster agent with:Your park on o/r#1 (the desk review) is over: o/r#1 has a new comment by a-reviewer … Re-query the live state of your task and continue it.
The cause is that
wakePartyresolves the session id to the roster agent first, soresumeOffRosterdelivers to it. The same stale park also stays visible tooffRosterParty/offRosterParkafter the session registers.
Fix: registering takes the session's entry out ofst.Parks, either dropping it or carrying it over asreg.parkwhen the new entry has none. That gives the "a new task or agents idle ends the park" rule an off-roster equivalent. Add a regression test for: park off the roster → register → a settle wakes nobody.
Not blocking
- Reply settling and this desk's single login.
RefStatesleaves out replies by the PR author (internal/github/refs.go:210), as #1154 specifies. On this desk, every session posts as the same GitHub login as the PR author, including the loop's review of a worker's PR. So a park on our own PR settles on a teammate's or an outside review, never on the desk's own review. The recent PRs (#1825, #1828, #1808, #1811) only have reviews and comments by that one login. Worth one sentence in the park help, so nobody parks on a PR expecting the loop's review to wake them. - Bot comments count as replies (anyone but the author). That is fine for now, but a CI or bot comment on a parked PR will resume the parker.
- The PR description's proof names head
f2996c9. The head is4b6c67b, a gofmt-only commit on top, so the proof still holds. Update the description when you next push.
A session that parked off the roster and then registered kept its entry in state.Parks, so a later settle woke the busy agent with a stale end of park. registerAgent drops the entry, and a resume of a session that joined the roster by any other way wakes nobody. The park help says a review under the author's own login settles nothing.
* origin/main: fix(park): wake a session off the roster when its wait settles (#1844)
…lert-state * origin/main: fix(park): wake a session off the roster when its wait settles (#1844)
* origin/main: fix(watch): stop sandbox scopes adopted by bubblewrap (#1862) fix(park): wake a session off the roster when its wait settles (#1844) chore(deps): update gsoci.azurecr.io/giantswarm/architect docker tag to v8.6.32 (#1854) feat(capacity): read Claude plan windows from its local usage command (#1829) fix(agents): resolve the worker contract against the host installation (#1851) fix(relay): allow roles to start in their configured folders (#1835) fix(deps): update module github.com/giantswarm/go-selfupdate to v1.7.2 (#1816) fix(note): recover notes after a failed initial status write (#1836) # Conflicts: # internal/config/config_test.go
* origin/main: fix(watch): stop sandbox scopes adopted by bubblewrap (#1862) fix(park): wake a session off the roster when its wait settles (#1844) chore(deps): update gsoci.azurecr.io/giantswarm/architect docker tag to v8.6.32 (#1854) feat(capacity): read Claude plan windows from its local usage command (#1829) fix(agents): resolve the worker contract against the host installation (#1851) fix(relay): allow roles to start in their configured folders (#1835) fix(deps): update module github.com/giantswarm/go-selfupdate to v1.7.2 (#1816) fix(note): recover notes after a failed initial status write (#1836) # Conflicts: # CHANGELOG.md
…uard * origin/main: fix(park): wake a session off the roster when its wait settles (#1844)
Problem
A session waiting on something beekeeper settles is woken only if it is on the roster. Off the roster, nothing reaches it:
payOweddrops the outcome of a queued merge (the gate's exit 76) or of a CI wait that left its call ("is no registered agent, nobody is woken").agents parkrefuses the session, andagents wakeresolves roster names only. A park on a pull request also never settles on a review or comment, only on a merge or close.Change: one wake path, by session id
agents wakestays the only way to deliver a wake. When no roster agent matches, it now resolves a session off the roster (cmd/offroster.go):local_id, or anomp_/agy_/codex_id.Delivery then runs exactly as for a roster agent: by name to a running CLI, a desktop turn, a background
claude --bg --resume(so a stopped background session resumes from its transcript too), a headless resume, or the harness inbox. A session that no longer exists (no transcript to resume, no harness process reading its inbox) is refused with the newerrGone.Every waiter is recorded by its session (
state.Party: session id, desktop or harness id), so the wake needs no roster entry:tellOwner→payOwed): the owner is woken by its session id, on the roster or off it. If its session is gone, the outcome is no longer owed and the log gets onedevctl.unheardline. An off-roster owner's session records are cleared, so the issues it served go back to the board.agents parkfrom a session off the roster is kept in the newstate.Parks(one per session); joining the roster (agents register) ends it, as a new task ends an agent's park, and a resume of a session that joined the roster otherwise wakes nobody. It settles insettledParks/markResumablelike a roster agent's park and is resumed byautoResumeoragents resume <id|name>through the same wake. If the session is gone, it gets one line and its records go back to the board.RefStatesreads the author and the last 10 reviews and comments of each pull request in the same GraphQL request. Pending reviews are left out. A review under the author's own login settles nothing (the park help says so): where every session posts under one login, the desk's own review does not wake a park.tellFiler).I chose this path over registering the caller at its first wait. Registering would list a supervisor or guide session as spare worker capacity, and the loop could staff it. The roster stays the worker roster, and every waiter is still woken through
agents wake.What this settles
agents wakenow finds the role holder by name or session id withoutagents register(supervisor start: a hand-started supervisor is absent from the agents wake roster #1777).Proof
TestAQueuedMergeWakesItsOwnerOffTheRoster(exit 76),TestAnUnheardRunWakesItsOwner(CI wait, roster and off the roster),TestAReviewResumesASessionParkedOffTheRoster,TestAReviewMakesAnAgentsParkResumable,TestResumeOffTheRosterandTestAGoneOwnerGetsOneLine(gone session: no wake, one line, records back to the board),TestJoiningTheRosterEndsAParkOffIt,TestWakePartyReachesSessionsOffTheRoster,TestRefStatesReadsAPullRequestsNewestReply.6403fef(main, including the stop of parked background sessions from fix(agents): stop a parked background session once its turn ends #1825, which this wake resumes from its transcript), headf2996c9(the later heads add a gofmt fix and the review's fix, head86aa488, proven by CI):go vet ./cmd ./internal/github ./internal/stateclean;go test ./internal/github ./internal/stateok;go test ./cmd -run 'Park|Resum|Owner|Owed|OffTheRoster|WakeParty|Gone|Unheard|Waiter|Queued|Background|Gate|Wake|Note|Filer|Answer'ok;golangci-lint runon the changed packages reports only one errcheck, incmd/agentstartcodex_test.go:512, which is on main and untouched here.Closes #1818
Closes #1777
Closes #1154