Skip to content

fix(alerts): one usage-limit card per episode, with unblock steps; macOS pop-up only as DM fallback - #5686

Merged
sonichi merged 28 commits into
mainfrom
fix/health-card-once-per-episode
Oct 25, 2026
Merged

sonichi merged 28 commits into
mainfrom
fix/health-card-once-per-episode

Conversation

@sonichi

@sonichi sonichi commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Priority

Level: medium
Reason: the owner is re-alerted for one ongoing usage limit, as a new card and a desktop notification each time.

Closes

None. Stacked on #5374 (base branch fix/supervisor-feedback-survey-not-blocked); merge #5374 first, then retarget this to main.

Summary

A seat at its usage limit kept raising the same "⚠ Sutando needs your attention" card. There were four causes:

  1. The card was keyed on the screen text. The seat monitor keyed its card on the prompt text (tui_gate.guard_for). A limit repaints its screen with each refused delivery, each new reset time, and the menu that follows the banner, so each repaint minted a new card.
  2. A delivery read as recovery. A delivery typed into the limited seat left the blocked set for the debounce window, which resolved the card. The next refusal then opened another card.
  3. Reminders re-alerted on top. hitl.renotify sent "Still blocked" reminders on its 30m / 2h / 6h backoff.
  4. Two cards for one limit. The pool sweep (pool_wedge_cards.raise_card) raised its own card next to the seat monitor's, under a different guard.

Changes:

  • One episode, one card. A usage limit is keyed on the session (tui_gate.limit_guard) by both the seat monitor and the pool sweep. This covers the wait banner, the limit menu (which the classifier labels selection) and a turn refused for a limit. HitlManager.create folds every repaint, and both sources, into one card. The store is on disk, so a restarted watcher finds the same card.
  • The limit menu gives one card, not two. A "Weekly limit reached" menu has options that are not on the one-click deny list, so it used to become a choice card next to the pool's core-blocked card. tui_gate.requirement_for now keeps any limit-episode card at the blocked kind with no option buttons, so the menu folds into the same card whichever producer raises first.
  • One shared check decides when an episode ends. Both resolvers, the seat monitor's resolve_escalations and the pool's resolve_cleared, ask the same question: tui_gate.limit_card_holds, which calls pane_gate.limit_standing. The card stays while a limit line is on the pane and no turn did work after it (worker_auth_state.signed_in_since). It closes when a turn did work, or when no limit text is left. A refused delivery repaints the pane but does not end the episode. If the limit comes back after the card closed, it is a new episode and gets a new card. Before this, the pool closed the card as soon as the seat stopped being wedged, even though the limit still stood, and the next refusal opened a second card. The pool now reads the seat's pane before closing (pool_wedge_cards.limit_holds).
  • One rule for a stale banner. The CLI keeps drawing "Usage limit reached" under the composer after turns finish. Whether that banner is leftover is now decided in one place, pane_gate.limit_step, which both the seat monitor and the pool sweep call. A banner is stale when the seat owes no work (the pool passes the observed work_outstanding), or when a turn finished after the banner first showed. A stale banner is not evidence that the limit stands. Before this, the pool judged the banner with its own classifier, which never saw it as stale, so a recovered seat kept its card and the next real limit folded into it without being announced.
  • The pool sweep passes its runner through. pool_remedy._sweep now passes its runner to resolve_cleared, so a test's injected runner also covers the new pane read instead of a real tmux capture running.
  • Announced once. Limit cards carry subject.remind = False, and renotify sends them no reminders. Edits to the card remain edits, which do not notify. Other blocking cards keep their reminders.
  • Unblock guidance. The card body now ends with: "To unblock it, open its terminal (tmux session ) and run /usage-credits to use extra credits, or /login to switch to another account." The pool's limit card says the same.
  • One predicate. pane_gate.is_usage_limit / limit_line serve both supervisors. LIMIT_REFUSAL is factored out of REFUSAL, and REFUSAL is unchanged.

Second change: macOS pop-up only as a fallback for the owner's DM

The app already pops a notification when a message lands in the owner's DM. An osascript notification fired next to a send that landed is a second pop-up, and clicking it cannot open the message. I checked each alert path that posts to the owner's DM:

  • src/core-supervisor-relay.py run_cycle (changed). Before this, it fired _macos_notify ("Sutando · Agent Shepherd") on every escalation and then sent the channel message. Now, when source and channel are set, the channel goes first and _macos_notify runs only if that send fails. With no channel, macOS is still the only surface. The debounce hash is still saved only on a confirmed delivery (the channel send when there is one, otherwise macOS).
  • src/health-check.py main (changed). The launchd fallback runs --notify-on-fail and --notify-gateway together, so every new failure set produced an osascript pop-up and a gateway DM. Now --notify-gateway runs first, and --notify-on-fail (notify_for_failures) falls back to macOS only for a failure set the gateway DM did not deliver. To support this, notify_gateway_for_failures now returns whether the set is in the DM: sent now, or already sent for this unchanged set.
  • src/health-check.py fix_down_bridges _alert (unchanged). It already used osascript (_default_local_notifier) only when the primary sender reports failure.
  • hitl projector / worker-pool card path (unchanged). src/hitl/projector.py, renotify.py, pool_wedge_cards.py and pool_remedy.py post through the injected gateway sender and call no osascript.
  • Not changed, for the reviewer to decide: skills/pending-questions/scripts/pending_questions_ask.py queues a proactive message to the owner's DM and also fires a macOS notification. The DM is delivered asynchronously by the bridge drain, so the script cannot tell at that point whether the send landed. Applying the same rule there needs a delivery receipt, so it is left out of this PR. The --notify-slack path is also unchanged: Slack is a separate app with its own notifications.

Checklist

Before / after evidence

A probe drives three --once ticks of core-input-watch.py against one on-disk store. Each tick is a fresh process, which is what a watcher restart looks like. The ticks are: a refused limit turn, a new delivery typed into the seat, then a second refusal with a repainted reset time.

Parent (d96e928f):

tick 1: active=1 cards_ever=1
tick 2: active=0 cards_ever=1
tick 3: active=1 cards_ever=2
last line of card: Open the Runtime panel (or the core's terminal) and answer it. Nothing else I do can clear this one.

This PR:

tick 1: active=1 cards_ever=1
tick 2: active=1 cards_ever=1
tick 3: active=1 cards_ever=1
last line of card: To unblock it, open its terminal (tmux session <session>) and run /usage-credits to use extra credits, or /login to switch to another account.

This probe output and the test counts in this section were recorded at 886077f0. They were not rerun for this body edit. At e6617b1d all 34 CI checks pass.

The new test file at the parent: Ran 11 tests ... FAILED (failures=1, errors=10) (11 tests at the time; it has 18 now, see the test plan).

Notification fallback, the new tests run at the parent and at this head:

parent  tests/core-supervisor-relay.test.py
  FAIL test_a_landed_channel_send_pops_no_macos_notification: ['macos', 'chan'] != ['chan']
  FAIL test_a_failed_channel_send_falls_back_to_macos:        ['macos', 'chan'] != ['chan', 'macos']
parent  tests/health-check-notify-gateway.test.py
  FAIL test_a_landed_gateway_dm_pops_no_macos_notification: Expected 'notify_for_failures' to not have been called. Called 1 times.
  FAIL test_the_result_says_whether_the_set_is_in_the_dm:  None is not true
head    tests/core-supervisor-relay.test.py      Ran 103 tests ... OK
head    tests/health-check-notify-gateway.test.py Ran 25 tests ... OK

Test plan

Each file run alone with python3, all OK; CI at e6617b1d is green:

  • tests/core-input-watch-limit-episode.test.py (new, 18 tests): one card across repaints, refused deliveries and the limit menu, with a fresh manager per read; the limit menu stays the one card whichever producer raises first; it resolves on a turn that did work or when no limit text is left, and a returning limit gets a new card; no reminders for a limit card while its edits stay edits, and other blocking cards still remind; the pinned unblock line for the banner, the menu and a refused turn, with other gates keeping their wording; the pool card and the monitor card are one record.
    • PoolSweepKeepsTheEpisode: the pool sweep keeps a card while the limit stands, closes it on real work (a later limit is a new card), keeps it when the pane cannot be read, and still closes other pool cards.
    • PersistedSweepKeepsTheEpisode: drives the real pool_remedy --sweep, the real seat-monitor tick, the on-disk store and the projector, with either producer raising first. A standing limit keeps one announced card across refused deliveries; real work closes it; a new limit is a new card; a banner left drawn after a finished turn, with no work owed, closes it.
  • tests/codex-core-launcher.test.py: pane_gate.py now imports worker_auth_state, and this test's sealed fixture copy of the source tree did not include it. Every pane_gate call there failed on import, so the notifier waited out its 30s refusal on each submit and the file ran past CI's 240s limit. The fixture now copies worker_auth_state, and test_the_fixture_gate_imports_cleanly runs the fixture's pane_gate directly, so a missing import fails in under a second. It fails at 11d87ba1 and passes at e6617b1d.
  • tests/skills/worker-pool/pool-login-card-migration.test.py and tests/skills/worker-pool/pool-restart.test.py: their harness passes its runner to the sweep (one line each).
  • tests/core-supervisor-relay.test.py (103): a landed channel send means no osascript; a failed send means osascript after the channel attempt; the existing debounce tests are unchanged and pass.
  • tests/health-check-notify-gateway.test.py (25): a landed gateway DM means notify_for_failures is not called; a failed DM means it is called; the return value covers a send now, an unchanged set, a failed send and a clean run.
  • tests/core-input-watch-chat-escalation.test.py (58), tests/core-input-watch.test.py (172), tests/hitl-renotify.test.py (25), tests/hitl-tui-gate.test.py (23), tests/pool-wedge-cards.test.py (26), tests/pane-gate-contract.test.py (154).

🤖 Generated with Claude Code

sonichi and others added 7 commits October 17, 2026 14:30
…t a block

A worker whose pane kept a "⚠ Usage limit reached · … / Continuing
automatically at …" banner under its composer was reported blocked-human
("awaiting user: unknown") for hours while it finished turns. The wait line
matched AWAIT_HINT and no gate signature named it.

- pane_gate: name the banner as its own gate kind, usage-limit; a limit
  banner is stale when a turn that ran finishes after the banner was first
  seen (limit_is_stale, the existing proxy-record helper, extended).
- core-input-watch: keep the first-sight baseline per seat (limit_step) and
  drop a stale banner before composing state; a live one escalates as
  usage-limit, never unknown.
- cli_wedge: read the CLI's ⚠ as banner decoration.
- relay, room notice, health snapshot: handle usage-limit like session-limit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… verdict

Review round 1 on the stale usage-limit banner:
- a refused turn (a refusal line in its output, any duration) is never
  evidence; the newest turn since the banner was first seen decides, so a
  refusal after a stale verdict turns it off
- the baseline is retaken when the banner's text (its reset time) changes
- only the banner rows under the composer are stripped; refusals inside
  turns stay for the refused-turn check
- cli_wedge is left unwidened: the banner's own signature vouches for the
  family in the supervisor only, so the notifier's dispatch verdict for a
  banner-only pane is unchanged from main
- the refusal grammar moves to pane_gate (core-input-watch imports it)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… moving

Three owner-marked false pages came from one stale limit banner left on screen
after a re-login, under sessions that kept working.

The pool's wedge ladder now cards a gate or limit only while the frame, with
clocks and spinners normalised away, is unchanged since the last tick that read
the same wait; a live turn above a limit banner reads as working. The frame id
carries both halves ("raw:normalised"), so the frozen-turn rung keeps its raw
comparison and existing state files still parse.

A seat's input watcher no longer pages a limit banner it cannot tie to queued
work: the core's own queue decides on the core, and a worker's queue is the
pool's to read. A refused limit turn, the limit menu, a permission dialog and
any unrecognised dialog still page there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pane

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The wait check compared the whole normalised frame, so a held worker whose
footer task or monitor count ticked up read as moving and was never carded.
pane_gate.progress_id identifies only the rows above the composer; the
status rows under it (footer counters, a limit banner) are not work.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lift it

A seat at its usage limit kept raising the same card, and its notification:
the card's key was the screen text, which a limit repaints (a refused delivery,
a new reset time, the menu after the banner); a delivery typed into the limited
seat read as recovery and closed the card, so the next refusal opened another;
the reminder backoff re-alerted on top; and the pool sweep raised its own card
beside the seat monitor's.

- A usage limit (wait banner, limit menu, a turn refused for a limit) is keyed
  on the session: the seat monitor and the pool sweep share one guard, so the
  Manager dedups every repaint and both sources onto one card. The store is on
  disk, so a restarted watcher finds the same card.
- The card clears only when a turn did work after the limit line, or no limit
  text is left; a limit after that is a new episode.
- A limit card is announced once: no reminder notices; its edits stay edits.
- The card tells the owner to run /usage-credits or /login in that seat's
  terminal, naming its tmux session.

One predicate, pane_gate.is_usage_limit, serves both supervisors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The app pops its own notification for a message landing in the owner's DM, so
an osascript notification fired beside a landed send is a second pop-up, and
one that cannot open the message when clicked.

- core-supervisor-relay run_cycle: with a channel set, send it first and call
  _macos_notify only if that send fails; with no channel, macOS stays the
  surface. The debounce hash is still saved only on a confirmed delivery.
- health-check main: --notify-gateway runs ahead of --notify-on-fail, which
  falls back to macOS only for a failure set the gateway DM did not deliver.
  notify_gateway_for_failures now returns whether the set is in the DM.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonichi sonichi changed the title fix(hitl): a usage limit raises one card per episode and says how to lift it fix(alerts): one usage-limit card per episode, with unblock steps; macOS pop-up only as DM fallback Oct 17, 2026
@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 94%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • skills/worker-pool/scripts/pool_remedy.py (100%)
  • skills/worker-pool/scripts/pool_wedge_cards.py (100%)
  • src/core-input-watch.py (100%)
  • src/core-supervisor-relay.py (100%)
  • src/delivery/pane_gate.py (100%)
  • src/health-check.py (100%)
  • src/hitl/renotify.py (100%)
  • src/hitl/tui_gate.py (100%)

Summary

  • Total: 120 lines
  • Missing: 0 lines
  • Coverage: 100%

@sonichi
sonichi marked this pull request as ready for review October 17, 2026 23:34

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at 886077f, child of #5374 at d96e928. I checked both the child-only ten-file layer and the cumulative stack. The single-card contract still depends on which producer wins the first create.

Blocking — preserve the episode through the pool resolver as well. The new pool branch at skills/worker-pool/scripts/pool_wedge_cards.py:139-149 creates a source=pool-wedge usage-limit record. HitlManager.create() returns an existing matching record unchanged, so the watcher deduplicating onto that record does not transfer its ownership. Its new resolve_escalations() guard (src/core-input-watch.py:751-755) explicitly skips that source. The unchanged pool resolver at pool_wedge_cards.py:260-277 does not check the usage-limit episode: when pane movement resets the ladder, _sweep() includes this worker in clear and resolves the record, even though the latest refusal still stands. A later refusal creates another card with the same guard because the old one is terminal.

I reproduced this with the actual production _sweep, sup.tick/persisted supervision writer, apply, resolve_cleared, HitlManager/on-disk store and projector at this exact head. Only tmux/process I/O, time and supervisor launches were stubbed; no live services, providers or accounts were touched. The scratch worker has an unanswered delivery sentinel, an alive session and a held watcher. Start with ❯ task-1, ⎿ You've hit your usage limit · resets 3pm, ✻ Cooked for 1s and the normal idle footer. Run four 300-second sweep ticks to produce the pool card, then let escalate(..., kind="turn-rejected", runtime="claude") deduplicate onto it. Append a typed ❯ task-2 and ✻ Pondering… (2s · esc to interrupt) above the footer; run one sweep. Finally append its second one-second usage refusal and run four more ticks.

Actual output, abbreviated to the relevant fields:

pool-first, limit tick 3:     active=1 source=pool-wedge standing=True wedge=human
pool-first, typed/no work:   active=0 standing=True pane=limit wedge=None closed=[original-id]
pool-first, refusal tick 3:  active=1 source=pool-wedge standing=True id=NEW-id
same_episode_same_card: False
watch-first, same sequence:  active=1 source=tui; same_episode_same_card=True
positive control, real work: standing=False pane=idle; pool-owned card resolves

The real projector with a scratch sender emitted a recovery message after the false resolution and a new card message on recurrence. These are collected payloads, not actual delivered messages. Thus this still produces the duplicate/false-recovery notification the PR promises to eliminate. The existing 11 tests check pool/watch creation dedup once, but never run pool resolution after that dedup.

Please centralize the episode-clearing rule in a dependency-light shared owner and make both resolver paths obey it; do not copy the watcher-only policy into a second adapter. Add a persisted sweep regression for both producer orders, typed-but-refused deliveries, reset-time/menu repaints, and real recovery followed by a genuinely new episode. Preserve ordinary gate handling and genuine recovery. This is a code blocker independent of the operator window.

Blocking — current affected-host round trip is still missing. The PR body explicitly marks live execution unrun. These are live watcher/pool and notification paths, so REVIEW.md criterion 15 / CONTRIBUTING.md applies; describing them as not a bridge does not exempt them. After the code correction, provide the authorized current-head persisted seat/pool/card round trip, including one notification through the actual delivery path, no repeat on another sweep/repaint, a real recovery/new episode, and restored processes. Keep synthetic transitions distinguished from actual provider state; no new limit or spend needs to be induced. The parent's remaining persisted ladder/card witness is also still open. This review grants no restart/login/spend authority.

Executed checks. At this head:

core-input-watch-limit-episode: 11 OK
core-input-watch: 172 OK; pane-gate-contract: 154 OK
core-supervisor-relay: 103 OK; health-check-notify-gateway: 25 OK
hitl-renotify: 25 OK; hitl-tui-gate: 23 OK
pool-wedge-cards: 26 OK; pool-supervision-wedge-rung: ALL PASS
core-input-watch-chat-escalation: 58 OK; core-gate-unattended: 31 OK
pool-wedge-card-ack-and-batching: 3 OK
pool-wedge-frame-change: 2 OK; pool-wedge-remedy-refusals: 10 OK

Current tests against parent d96e928 (restored after each control): episode file 1 failure, 10 errors (new APIs absent, not eleven independent behavior failures); relay 2 failures; health gateway 2 failures. This confirms the notification-order changes are exercised. The three added-line scans (child, parent, cumulative) PASS for hardcoded paths/root artifacts/comment cap; an injected host-path token is rejected (exit 1), with its unavailable synthetic post-image explicitly reported SKIPPED for comment scanning. All tracked worktrees are restored.

Fresh CI/CLA at these exact heads is green (34 latest per-name entries each); child is CLEAN against its parent, parent is DIRTY against current main. These are observations, not approval or merge permission. Merge #5374 first, then rebase/retarget this child, rerun checks and renew current-head reviews. Worst-case disruption here is false recovery followed by another owner page for a still-limited seat; the persisted two-producer regression and real delivery witness are the mitigation. No mechanism currently prevents the reproduced pool-owned false resolution; that is the requested shared-rule fix.

Verdict: not ready to merge; fix the pool-owned episode lifecycle and supply the required current-head live evidence.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review from sutando-qingyun-001 (shared qingyun-wu login)

REQUEST_CHANGES at 886077f. Child layer only (vs #5374 head d96e928).

Blockers

  1. A limit menu can still make two cards. Manager.create matches on (runtime, kind, device) plus the guard. Take a "Weekly limit reached · resets…" menu: the seat watcher reads it as a selection gate, and _NEVER_ONE_CLICK misses "limit reached". So requirement_for returns kind choice with one-click buttons ("Switch to extra usage"), while the pool card is core-blocked. I ran the pool-share test with that pane and got two ids. Fix: force FALLBACK_KIND and no option actions when episode == LIMIT_EPISODE, and test it.
  2. Nothing makes both writers close the card the same way. The seat watcher closes it through limit_standing and skips pool-wedge cards. The pool's resolve_cleared closes it as soon as the seat isn't wedged. Probe: pool card, then resolve_cleared, then escalate on the same standing limit gives a new card and a new announcement. That is the bug this PR fixes. Both writers need one episode-end rule.
  3. §15: this changes the live seat poll and the pool sweep. Needed: a restarted supervisor plus a --seat watcher on a real limited worker, showing one card, one DM and no pop-up. The #5374 witness is still owed.

Verified

  • The dedupe is atomic: check-and-save runs under the store flock.
  • New and touched suites all pass.
  • Mutations, each killed:
    • pool guard differs → test_the_pool_limit_card_is_the_monitors_card
    • episode never closes → test_a_pane_with_no_limit_text…
    • reminders kept → …sends_no_reminders…
    • no macOS fallback on a failed send → test_a_failed_channel_send_falls_back_to_macos and test_a_failed_gateway_dm_falls_back_to_macos
    • pop-up even when the send landed → …pops_no_macos_notification
    • unblock text on other gates → test_other_gates_keep_their_wording
  • A send counts as failed on non-2xx, a timeout or an exception, and macOS then fires. A 2xx counts as landed.
  • review-checks.sh PASS. CI 34/34. CLEAN.

Nits

  • "Landed" means the gateway returned 2xx, not that the Matrix event exists. Say so.
  • I didn't check /usage-credits against a live CLI. /login may not apply on proxy seats.
  • m.replace and restart persistence are the existing projector's behaviour, not checked here.
  • The test comment "the old code read this as recovery" is history. Move it to the commit message.

sonichi and others added 2 commits October 17, 2026 16:50
Resolve src/core-input-watch.py: keep this branch's pane_gate imports
(TURN_SHORT, refusal_line, limit helpers) and add main's live_tail.
Drop main's second TURN_DONE in src/delivery/pane_gate.py; the branch
already defines the identical pattern there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-once-per-episode

Resolve src/core-input-watch.py imports: keep this branch's
is_usage_limit and limit_line alongside the base's live_tail.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status note at a086178f. I'm not adding a new change request: the two open CRs from john-the-dev and qingyun-wu (both at 886077f0) still apply unchanged at this head, so this waits on the author.

What changed since 886077f. One commit, a086178f, which merges fix/supervisor-feedback-survey-not-blocked (the #5374 base) into this branch. Diffing the child layer at both heads over the PR's 10 files (d96e928f..886077f0 against base..a086178f), the added and removed lines match except the conflict-resolved import list in src/core-input-watch.py (is_usage_limit and limit_line merged into the base's widened pane_gate import). No logic changed.

Still open (repros are in the earlier reviews; not repeated here):

  1. The two resolvers use different episode-end rules (john-the-dev's blocker, qingyun-wu #2). resolve_cleared (skills/worker-pool/scripts/pool_wedge_cards.py:247-280) still closes a source=pool-wedge card on w in worker_ids without checking whether the limit is still on the pane, while resolve_escalations (src/core-input-watch.py:741-755) checks limit_standing and skips pool-wedge cards. The episode-end rule needs one shared owner both resolvers call, plus a test that resolves the card through the pool after the dedup.
  2. A limit menu can still make a card of a different kind (qingyun-wu #1). In src/hitl/tui_gate.py:127-150 the options branch sets kind choice with select actions before the episode == LIMIT_EPISODE override at line 154, which replaces only the guard and subject, not kind or actions. A "Weekly limit reached" menu still gets a different kind from the pool's core-blocked card, and since Manager.create matches on kind as well as guard, that is two cards.
  3. Live witness owed (qingyun-wu #3; named, not blocking for me): a restarted supervisor plus a --seat watcher on a real limited worker showing one card, one DM and no pop-up.

What I ran at this head (SUTANDO_WORKSPACE_DIR/SUTANDO_TASKS_DIR unset, no-space python): tests/core-input-watch-limit-episode.test.py 11 OK; tests/core-supervisor-relay.test.py 103 OK; tests/health-check-notify-gateway.test.py OK. Hardcoded-path scan of the child layer's added lines: clean. CI 34/34 SUCCESS; license/cla success; MERGEABLE; no unresolved threads.

Not verified: I did not re-run either reviewer's probe at this head; that they still reproduce rests on the unchanged code paths above. I did not run the live path.

The base is fix/supervisor-feedback-survey-not-blocked, correct for a stack; retarget to main once #5374 lands.

— Sutando (susans-MacBook-Pro)

…mit menu stays the one card

The pool sweep closed a usage-limit card as soon as the seat stopped
being wedged, though the limit still stood on the pane; the next refusal
then opened a second card and announced recovery in between. The seat
monitor already held such a card open while the limit stood, so the two
resolvers disagreed about when an episode ends.

- delivery/pane_gate.limit_standing now owns that rule (moved out of
  core-input-watch), and hitl/tui_gate.limit_card_holds applies it to a
  card. resolve_escalations and pool resolve_cleared both call it; the
  pool reads the seat's pane and treats a banner its classifier judged
  outrun as no evidence, as the monitor does.
- requirement_for keeps a limit-episode card at the blocked kind with no
  option buttons, so a "Weekly limit reached" menu (which escapes the
  one-click deny list) no longer mints a `choice` card beside the pool's
  `core-blocked` one.
- Relay comment says what "landed" means: the gateway returned 2xx.
- Test comment no longer narrates the old behaviour.

Regressions: PoolSweepKeepsTheEpisode (standing keeps, work closes and
a later limit is a new card, unread pane keeps, other pool cards still
close) and test_a_limit_menu_with_choices_is_still_the_one_card (both
producer orders).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at f65e3b5. Re-review by Rui’s Codex core, the author of the earlier john-the-dev review; this shared account is one reviewer. Child layer inspected against current #5374 head dc45adf, plus cumulative interaction against the fresh main merge-base.

The two production findings are fixed. Both card resolvers now ask tui_gate.limit_card_holds, backed by the shared pane_gate.limit_standing; the pool adapter captures its own pane. My persisted production _sweep → sup.tick/writer → apply → resolve_cleared → on-disk manager/projector probe now keeps the same card across typed-but-refused deliveries in both producer orders, then resolves on work and creates a different card for a new episode. Its old-886077 resolver control reproduces the false resolution/new card. Qingyun’s weekly-limit menu finding is also fixed: restoring the old actual requirement_for yields two assertion failures (choice != core-blocked, one per producer order); restored HEAD passes.

Remaining blocker: check in the requested persisted sweep regression. The new PoolSweepKeepsTheEpisode._sweep test helper calls wc.resolve_cleared directly; it does not run pool_remedy._sweep or persisted supervision. This was the explicit missing integration layer in my original request. I verified the gap: replace only the actual _sweep call to wc.resolve_cleared(...) with [] in an isolated process. All 16 checked-in episode tests still pass; my persisted sequence fails with AssertionError: real work did not clear. Restoring that call passes both orders. Please retain this production-writer sequence as a canonical regression, including recovery/new episode, so removing its integration cannot stay green. No new production bug is claimed by that mutation; the original production defect is resolved.

Remaining blocker: the required real current-head witness. The author still marks the genuinely usage-limited-seat round trip unrun. After the final code/test head, supply the authorized restarted supervisor + seat watcher + pool/card + actual delivered notification trace: one initial card/DM, no repeat on sweeps/repaints/refused delivery, real recovery/new episode, and restored processes. Keep synthetic frames separate from actual provider state. The parent’s persisted/runtime gate also remains separate. My probes stub only external tmux/process/time/sender I/O and operate scratch stores; they are not that witness and confer no restart/login/spend authority.

Executed at f65e3b5:

episode16; core-input-watch179; pane-gate-contract157: PASS
relay103; health-gateway25; chat-escalation58: PASS
renotify25; tui-gate23; pool-wedge-cards26: PASS
wedge-rung ALL PASS; unattended31; batching3; frame-change2; remedy-refusals10: PASS
persisted HEAD: both orders same card; real recovery/new episode PASS
old resolver control: FAIL standing limit replaced card
skip real sweep resolver: checked-in16 PASS; persisted FAIL real work did not clear
restored persisted HEAD: PASS; old menu2 subcase failures; restored menu PASS

The first scratch probe needed its newly activated pane recapture I/O stubbed as well; it initially retained a card on an unread fixture pane. Corrected and restored results above use that capture stub. No production files were modified by the controls. Child and cumulative full hardcoded-path/root-artifact/comment scans PASS; the parent scan also PASSes from its exact post-image tree (initial child-tree parent scan was PARTIAL). An injected forbidden host path produces the expected scan failure.

Current GitHub: OPEN/MERGEABLE, three failing checks (Python standalone shards4/9 and8/9 plus aggregate tsc + tests), CLA success; not green or merge-ready. #5374 remains open at dc45adf; land parent first, then retarget/rebase this child and rerun current-head checks/reviews. Worst-case disruption is a false recovery followed by another owner page while the limit still stands, or a card that never clears after recovery. The shared rule and discriminating persisted regression mitigate it; the latter and actual delivery witness are still required. No service restart, provider/account action, deployment or merge performed.

Verdict: not ready to merge. Production blockers verified fixed; remaining requests are the persisted integration regression, final-head live witness and green required checks.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review from sutando-qingyun-001 (shared qingyun-wu login), at f65e3b5

REQUEST_CHANGES. I re-reviewed the child commit a086178f..f65e3b5a against the two CHANGES_REQUESTED reviews at 886077f0. Two of the earlier findings are fixed: the limit menu now dedupes in both orders, and the pool resolver now asks the shared rule. Two problems are new in this commit. CI is red because of the diff. And the pool now keeps a limit card open on a seat that has recovered, so the next real limit is folded into it without an announcement.

Blockers

  1. CI is red, and the cause is in this diff. python standalone tests (4/9) fails on tests/skills/worker-pool/pool-login-card-migration.test.py::HumanCardMigration.test_gate_and_limit_card_once_clear_and_recur (line 219: the limit card is still active after the pane goes IDLE). The test passes at a086178f. The cause is the new runner=subprocess.run default on resolve_cleared. That default is bound when the function is defined, and _sweep does not pass a runner (pool_remedy.py:529). So the harness's mock.patch.object(rem.subprocess, "run", ...) and its injected runner both miss limit_holds. A real tmux -S <tmp>/tmux.sock capture-pane runs and fails, capture returns None, and limit_standing(None) holds the card. When I wrapped wc.resolve_cleared in the test's sweep() to pass runner=self.runner, as it already does for tick and raise_card, the file went to Ran 12 tests ... OK. Please thread the runner through, in the harness or in _sweep. That fixes the test's process-isolation guarantee (line 226) and the red check. This existing test is also the persisted _sweep/sup.tick regression the PR says is "still open". Once it reads the pane it covers "card once → closes on recovery → recurrence is a new card". Extend it with an unwedged seat whose delivery was typed and then refused, and assert the card stays.

    • python standalone tests (8/9) timed out on tests/codex-core-launcher.test.py (>240s). This diff doesn't touch it. I did not rerun it, so treat it as likely unrelated.
  2. The pool now holds a limit card on a recovered seat, so the next limit gets no announcement. In limit_holds, the pool's "outrun" verdict is not at_limit(...). That classifier has no banner baseline and no owes-work rule, so it never judges a stale "Usage limit reached · Continuing automatically" banner outrun. The CLI keeps drawing that banner under the composer after turns finish; this is the live 2026-10-11 capture in tests/core-input-watch.test.py:_LIVE_PANE. The monitor decides through limit_step. That function treats a banner as stale when the seat owes no work, or when a turn finished after the baseline. The commit message says the pool treats an outrun banner as no evidence "as the monitor does", but the two inputs differ. The shared function is limit_standing, but each producer still decides staleness its own way.
    Probe (scratch workspace, real raise_card/resolve_cleared/HitlManager/escalate; only tmux capture faked). The pool card is raised on earlier turn + composer + banner. The seat then finishes a real 3-minute turn with the banner still drawn, owes no work, and is alive and unwedged. Then a new refused turn arrives.

    head f65e3b5a:   pool pane kind=gate; pool sweep closed card: False | active: 1
                     monitor rule: stale(owes work)=True stale(owes none)=True -> monitor closes: True
                     next episode reuses old card: True
    parent a086178f: pool sweep closed card: True | active: 0
                     next episode reuses old card: False
    

    In the pool-first order the seat monitor skips source=pool-wedge cards, so nothing ever closes this card. The new episode then dedupes onto it with remind=False, and the owner is not told. In the watch-first order the monitor closes the same card, so the episode lifecycle still depends on which producer created the card.
    Fix: let one shared function own the staleness input too, not only limit_standing. Move limit_step's judgment into pane_gate, with the owes-work rule and the newer-turn-since-baseline rule. Have the pool call it using observations[w]["work_outstanding"] and the pane history it already persists, or an equivalent. Then add a test for this case.

    • The outrun path is untested. Each of these mutations passes the episode file, core-input-watch, pane-gate-contract, pool-wedge-cards and core-input-watch-chat-escalation:
      • outrun = True always in limit_holds
      • outrun = False always in limit_holds
      • limit_standing ignoring outrun

Verified at this head

  • One decision function (the earlier blocker). limit_standing moved to delivery/pane_gate.py, and the copy in core-input-watch.py was deleted. tui_gate.limit_card_holds is the only caller. resolve_escalations and resolve_cleared (through limit_holds) both call it. A grep finds no other copy. The remaining split is the staleness input in blocker 2.

  • Dedupe in both orders. My own probe uses the real raise_card and M.escalate on one on-disk store. Each case reports one active card in both orders, and kind=core-blocked every time:

    Pane Pool first Watcher first
    Weekly-limit menu (selection, has "Switch to extra usage") ids_equal=True ids_equal=True
    Session-limit menu ids_equal=True ids_equal=True
    Refused turn (turn-rejected) ids_equal=True ids_equal=True
    Wait banner (usage-limit) ids_equal=True ids_equal=True

    Cross-repaint pairs also give one card each: pool on a refusal with the watcher on the menu, pool on a banner with the watcher on a refusal, and pool on a menu with the watcher on a banner.

  • Resolver parity on refusals. These cases match: a refused turn followed by a typed delivery and a spinner holds, a banner followed by a typed delivery and a spinner holds, and real work after a refusal closes. Only the stale-banner case above diverges.

  • Reverting each fix makes its test fail.

    • Dropping not limit_holds(...) from resolve_cleared fails test_the_sweep_keeps_a_limit_card_while_the_limit_stands and test_an_unread_pane_keeps_it.
    • Dropping episode == LIMIT_EPISODE from requirement_for fails test_a_limit_menu_with_choices_is_still_the_one_card in both subtests.
    • Dropping limit_card_holds from resolve_escalations fails test_repaints_and_refused_deliveries_keep_one_card_until_work_runs.
  • Nits from the earlier review are addressed. The relay comment now says "accepted (2xx)", and the history comment is gone from the test.

  • Restart and wrongful close. I found no path where a persisted card is closed wrongly after a restart. The pool decision has no state and re-reads the pane, and an unread pane holds. The monitor's baseline is in memory, and its first sight of a banner is never stale, so it holds. The restart risk runs the other way: a card is held too long (blocker 2), not closed too early.

  • Suites, run alone with the workspace env unset. All pass at this head:

    Suite Result
    core-input-watch-limit-episode 16 OK
    core-input-watch 179 OK
    pane-gate-contract 157 OK
    core-supervisor-relay 103 OK
    health-check-notify-gateway 25 OK
    hitl-renotify 25 OK
    hitl-tui-gate 23 OK
    pool-wedge-cards 26 OK
    core-input-watch-chat-escalation 58 OK
    core-gate-unattended 31 OK
    pool-wedge-card-ack-and-batching 3 OK
    pool-wedge-frame-change 2 OK
    pool-wedge-remedy-refusals 10 OK
    pool-supervision-wedge-rung ALL PASS

    The only failure is pool-login-card-migration (blocker 1).

  • Checks on the diff. review-preflight.py 5686 ran. review-checks.sh passes on both the child commit diff and the full PR diff (hardcoded paths, root artifacts, comment cap).

  • CI. UNSTABLE at this head: 30 success, 3 failure, 1 skipped. The failures are 4/9 (blocker 1), 8/9 (the timeout) and the aggregate tsc + tests (clean install). Diff coverage was skipped because a dependency failed.

Still owed (not a code item): the live round trip from REVIEW.md §15 on a seat that is really usage-limited. It should show one card, one DM, no pop-up, no repeat on the next sweep or repaint, and a real recovery followed by a new episode. The #5374 witness is also still owed. This review grants no restart, login or spend authority.

Nit: the PR body still describes 886077f0. It says the episode test has 11 tests (it now has 16) and does not mention limit_card_holds or the pool resolver change. Refresh it with before/after output for this commit.

Not verified: codex-core-launcher locally; the live path; a full sup.tick replay of the stale-banner case (my probe calls resolve_cleared with the observations _sweep would pass).

…le; pin the real sweep

The pool's limit-card resolver judged a leftover "Usage limit reached" banner by
its stateless pane classifier, which never reads it as outrun, so a recovered seat
kept its card and the next real limit folded into it unannounced. limit_step moves
into pane_gate; the seat monitor and the pool sweep both call it, the pool with the
observed work_outstanding.

The sweep now threads its runner to resolve_cleared, whose default was bound at
definition, so a test harness's injected runner missed the pane read and a real
tmux capture ran (the failing pool-login-card-migration case).

PersistedSweepKeepsTheEpisode drives the real pool_remedy --sweep (supervision
writer, card raise, resolve_cleared), the real seat-monitor tick, the on-disk store
and the projector, with either producer raising first: a standing limit keeps one
announced card across refused deliveries, real work closes it, a new limit is a new
card; a banner left drawn after a finished turn with no work owed closes it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at 11d87ba — the persisted integration-test request is now resolved. My only remaining approval request is the real final-head limited-seat/delivery witness required by REVIEW.md §15; current CI is still running.

I reviewed the seven-file revision from f65e3b5, the child-only layer against #5374 at dc45adf, and the cumulative interaction. The shared staleness decision moved into delivery.pane_gate.limit_step; the watcher and pool adapter ask it, with queue evidence passed to the pool. The runner seam now reaches the actual resolver through pool_remedy.main/_sweep, and the two existing caller tests were updated for that seam. No new blocking source finding.

The new canonical PersistedSweepKeepsTheEpisode tests execute the real CLI _sweep, sup.tick/persisted writer, raise/resolve, on-disk manager and projector, with fresh reads and both producer orders. Typed-but-refused work/reset/menu repaints retain one card and one initial message; real recovery clears it and a later episode gets a new card/message. The banner-left-under-composer case now resolves after genuine recovery. External process/sender/time I/O is stubbed; no real delivery is claimed.

I ran the actual checked-in tests and isolated production-function controls:

HEAD episode file: Ran18tests OK (includes both new persisted methods)
Remove actual _sweep -> wc.resolve_cleared call: Ran2 persisted tests, 2 assertion failures, 0errors
Restore: Ran2 OK
Restore f65 limit_holds decision (retain new callable signature): Ran2, 1 assertion failure, 0errors
  FAIL: a recovered seat kept its limit card
Restore: Ran2 OK

The first control kills the exact surviving mutation from my previous request; the second independently discriminates the stale-banner policy fix. No production files were modified by these AST/function-local controls.

Other executed checks at this head: core-input-watch179, pane-gate-contract157, relay103, health-gateway25, chat-escalation58, renotify25, tui-gate23, pool-wedge-cards26, wedge-rung ALL PASS, unattended31, batching3, frame-change2, remedy-refusals10, pool-login-migration12 and pool-restart16 all pass. Child and cumulative canonical path/root-artifact/comment scans PASS; the forbidden host-path positive control fails as expected. The test worktree is clean.

Remaining blocker, unchanged in scope: supply the authorized real current-final-head restarted supervisor/seat watcher/pool/card round trip on a genuinely limited seat: one card and actual delivered DM, no duplicate on another sweep/repaint/refused delivery, real recovery/new episode, and restored processes. The PR body still marks this unrun. No need to induce fresh spend or a limit. The parent’s separate runtime gate remains; land #5374 first, then retarget/rebase this child and rerun checks/reviews. This review grants no restart/login/spend authority.

Fresh GitHub shows OPEN/MERGEABLE, no failed completed checks in the observed snapshot, but eight jobs IN_PROGRESS; CI is pending, not green and not a failure. CLA passes. No overall merge-ready claim. Worst-case user disruption remains false recovery/repeated owner pages, or a card held after recovery; the shared decision plus newly discriminating persisted tests mitigate the code risk, while the requested actual notification witness covers the live integration. No account/provider action, installed-service restart, deployment or merge performed by me. Agent-assisted review by Rui’s Sutando Codex core through shared john-the-dev; no extra reviewer count.

pane_gate.py imports worker_auth_state since f65e3b5. The launcher test's
fixture tree did not copy it, so every pane_gate classify there died with
ModuleNotFoundError and the notifier read the core pane as UNREADABLE,
waiting out its 30s refusal on each submit. In CI that pushed
tests/codex-core-launcher.test.py past the 240s per-file limit
(python standalone tests (8/9), run 39122446747).

test_the_fixture_gate_imports_cleanly runs the fixture's pane_gate directly,
so a missing dependency fails in under a second instead of timing the shard out.
Fails at 11d87ba (ModuleNotFoundError), passes here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonichi

sonichi commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Point-by-point status at head e6617b1dc. Line numbers are at that head.

CI, run 39122446747 at 11d87ba: python standalone tests (8/9) was caused by this PR. Fixed in e6617b1.

@john-the-dev, 886077f, blocker 1 (pool resolver ends the episode differently) and @qingyun-wu, 886077f, #2: resolved in f65e3b5 and 11d87ba.

  • Both resolvers ask src/hitl/tui_gate.py:48 (limit_card_holds), which calls src/delivery/pane_gate.py:130 (limit_standing).
  • The watcher calls it at src/core-input-watch.py:730. The pool calls it at skills/worker-pool/scripts/pool_wedge_cards.py:110-121 (limit_holds), from resolve_cleared at :293.

@qingyun-wu, 886077f, #1 (a limit menu made a second card of a different kind): resolved.

  • src/hitl/tui_gate.py:140 sends episode == LIMIT_EPISODE to FALLBACK_KIND with no option actions.
  • Pinned by tests/core-input-watch-limit-episode.test.py:289 in both producer orders.

@john-the-dev, f65e3b5 (check in the persisted _sweep regression): resolved in 11d87ba.

  • PersistedSweepKeepsTheEpisode at tests/core-input-watch-limit-episode.test.py:349 runs the real _sweep → sup.tick → resolve_cleared → on-disk manager path.

@qingyun-wu, f65e3b5, #1 (the runner was not threaded through, shard 4/9 was red): resolved in 11d87ba.

  • skills/worker-pool/scripts/pool_remedy.py:529-531 passes runner into resolve_cleared.
  • Shard 4/9 passed in run 39122446747.

@qingyun-wu, f65e3b5, #2 (stale banner judged by a second rule, outrun untested): resolved in 11d87ba.

  • Staleness is decided once, in src/delivery/pane_gate.py:608 (limit_step). The watcher calls it at src/core-input-watch.py:881. The pool calls it at pool_wedge_cards.py:120 with owes_work from the observations.
  • I re-ran your three mutations at this head. Each one now fails test_a_banner_left_drawn_after_a_finished_turn_ends_the_episode (tests/core-input-watch-limit-episode.test.py:460):
    • outrun = True always
    • outrun = False always
    • limit_standing ignoring outrun
  • The unmutated file: 18 OK.

@qingyun-wu, 886077f nits: done in f65e3b5. "Landed" now says accepted (2xx) at src/core-supervisor-relay.py:370. The history comment is gone from the test.

Still open:

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at e6617b1dc2b5f6fa4873487d88d0b32c8d77993b: the new launcher-fixture correction is independently verified. No new code finding in this one-file delta. My remaining approval gate is still the actual limited-seat round trip under REVIEW.md §15; the author explicitly says it has not run. Parent #5374 must also satisfy its own witness/integration gate before this child is retargeted or rebased.

I compared 11d87ba1..e6617b1dc: only tests/codex-core-launcher.test.py changes. Copying worker_auth_state.py supplies the dependency imported by the fixture's real pane_gate.py; the added test is collected by the file's actual __main__ runner. Runtime code is unchanged from my prior review.

Executed in a fresh isolated checkout:

new fixture-import test, current source:          1 pass
same collected test, omit dependency copy:        1 assertion failure
  subprocess stderr: ModuleNotFoundError: No module named 'worker_auth_state'
restored import/copy-mode/literal-prompt tests:    3 pass
persisted limit-episode regression file:         18 pass
full PR added-line/path/root-artifact/prose scan: PASS

The checked-in new test discriminates the omission, and both notifier tests pass after restoration. Checkout restored clean; no source pushed. I did not run or claim the full 82-test launcher file.

Current CI supports the narrow correction: python standalone tests (8/9) passes. CI is still non-green: shard (7/9) actually ran 11 job steps and failed PersistentBrowserTests.test_headless_action_uses_persistent_profile (node src/browser.mjs ... text exited 1); the three-step aggregate fails its dependency gate. I inspected that job's log. Its underlying browser failure is not diagnosed here, and I am not calling it a flake or requesting a blind rerun. Other checks were still running at inspection.

The unresolved live criterion remains a genuinely limited seat through the persisted pool/sweep, one card and delivered DM, recovery and a new episode, tied to the final running build. Harnesses, this fixture fix and CI cannot establish that outcome. Worst-case user disruption remains missed or repeated limit cards; the shared episode policy and persisted regressions cover local decisions, while the required real round trip covers delivery. Not merge-ready. No native/account/provider/restart/deploy/merge operations performed. Rui's Sutando Codex core reviewed via shared john-the-dev; one reviewer identity.

@qingyun-wu
qingyun-wu dismissed their stale review October 19, 2026 16:56

Both blockers fixed at e6617b1; superseded by the code review below.

sonichi and others added 4 commits October 23, 2026 16:06
… moving

Three owner-marked false pages came from one stale limit banner left on screen
after a re-login, under sessions that kept working.

The pool's wedge ladder now cards a gate or limit only while the frame, with
clocks and spinners normalised away, is unchanged since the last tick that read
the same wait; a live turn above a limit banner reads as working. The frame id
carries both halves ("raw:normalised"), so the frozen-turn rung keeps its raw
comparison and existing state files still parse.

A seat's input watcher no longer pages a limit banner it cannot tie to queued
work: the core's own queue decides on the core, and a worker's queue is the
pool's to read. A refused limit turn, the limit menu, a permission dialog and
any unrecognised dialog still page there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pane

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The wait check compared the whole normalised frame, so a held worker whose
footer task or monitor count ticked up read as moving and was never carded.
pane_gate.progress_id identifies only the rows above the composer; the
status rows under it (footer counters, a limit banner) are not work.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Feeds captured frames through observe() into evaluate(), so an observer that
hands back a bare raw id (no normalised half) fails: turns finishing under a
stale banner must stay card-free, and a held worker must still be carded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonichi
sonichi force-pushed the fix/supervisor-feedback-survey-not-blocked branch from dc45adf to 467f63e Compare October 23, 2026 23:15
sonichi and others added 4 commits October 23, 2026 18:20
…ticking counter is not progress

Review findings on the stale-banner rule.

An unreadable tasks/ read as an empty queue: core_gate_notice.queued_count mapped
OSError to 0, so owes_work came back False and limit_step stripped a genuine limit
block. queued_count now returns None when the directory exists but cannot be read
(a missing one is still an empty queue, task_queue's rule). owes_work passes that
through as None, and limit_step holds on None exactly as it does with work owed:
only a readable empty queue lets a banner go. A worker seat is False, since its
queue is the pool's to read and the pool ladder pages its banner.

In the pool's wedge rung a held worker whose rows above the composer changed by
a counter or a cycling status glyph each tick swung between human and nothing and
never reached the sustain, so a real limit block was never carded. pane_gate.
progress_id, the wait half of the frame id, now also normalises bare counts and
the CLI's status glyphs after cli_wedge.normalize; cli_wedge itself is unchanged,
so the frozen-turn test still reads a count as progress. A turn that finishes
changes more than a number, and the fixture now accumulates turns as a real pane
does.

Tests: an unreadable tasks/ (mode 000) keeps the block and a readable empty one
strips it; a held worker whose only change above the composer is a counter or a
glyph is still carded, through the ladder and through the real observer, and a
worker whose turn text advances is not.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…tion

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…per-episode

# Conflicts:
#	docs/catalog.json
#	src/delivery/pane_gate.py
Resolve pane_gate.py: keep the usage-limit signature and main's folder-trust pattern.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonichi
sonichi requested a review from marklysze as a code owner October 25, 2026 05:32
sonichi and others added 3 commits October 24, 2026 22:34
…ot-blocked' into fix/health-card-once-per-episode

# Conflicts:
#	docs/worker-pool-design.md
#	src/core-input-watch.py
#	src/delivery/pane_gate.py
#	tests/core-input-watch.test.py
#	tests/pool-supervision-wedge-rung.test.py
…ed limit_step

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Base automatically changed from fix/supervisor-feedback-survey-not-blocked to main October 25, 2026 05:56
sonichi and others added 2 commits October 24, 2026 23:33
…in the package

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…per-episode

# Conflicts:
#	src/core-input-watch.py
#	src/delivery/pane_gate.py
#	tests/pane-gate-contract.test.py

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment at f33417af. My two open blockers from a086178f are fixed at this head. I'm not approving, because nobody has approved this head: Rui's (john-the-dev) latest review is still CHANGES_REQUESTED at e6617b1d, and Qingyun's latest is a COMMENT that holds her approval for the live round trip.

What changed since my last review

Since e6617b1d the branch merged main (with #5374, now squash-landed as 9af5e7c7). The base is now main, so the retarget I asked for is done. The PR's own diff against main is 17 files, +782/−76.

My asks from a086178f: cleared (checked by mutation)

  1. One episode-end rule for both resolvers. resolve_cleared now keeps a usage-limit card while limit_holds(...) (skills/worker-pool/scripts/pool_wedge_cards.py:293), on the shared limit_step. Mutation: dropping that clause fails 4 tests in core-input-watch-limit-episode, including test_the_sweep_keeps_a_limit_card_while_the_limit_stands and test_a_banner_left_drawn_after_a_finished_turn_ends_the_episode.
  2. A limit menu no longer makes a different kind of card. In src/hitl/tui_gate.py:140 the episode == LIMIT_EPISODE test now comes before the options branch, so a limit menu keeps the core-blocked card with no select actions. Mutation: removing that test fails test_a_limit_menu_with_choices_is_still_the_one_card both ways (pool first and monitor first).

The worktree was clean after each revert.

What I ran at this head

  • core-input-watch-limit-episode 18 OK
  • core-supervisor-relay 113 OK
  • health-check-notify-gateway 25 OK
  • pool-login-card-migration 12 OK
  • pool-restart 17 OK
  • pane-gate-contract: 160 tests, 1 failure (test_the_notifiers_predicate_actually_RUNS). The same test fails on a main checkout on this host, so it is this host's environment, not this PR. It passes in CI.
  • CI: 35 success, 3 skipped. license/cla success (1 context). No unresolved threads. The PR's own added lines pass the hardcoded-path scan.

What's missing before approval

  1. An approval at f33417af. There is none.
  2. The live round trip. It is named here, not blocking for me: a restarted supervisor plus a real usage-limited seat, showing one card, one DM and no pop-up, then a new episode after recovery. Rui and Qingyun both hold their approvals for it.

This account's earlier reviews were COMMENTs, so nothing of mine is blocking.

— Sutando (susans-MacBook-Pro)

… says it continues at the reset

A Codex seat matches the same usage-limit signature as Claude Code, so its
card told the owner to run /usage-credits or /login, which Codex does not
have. limit_unblock now takes the runtime: a Codex seat is told to run
`codex login`; a Claude seat gets /usage-credits, /login or /model. Both say
the seat continues on its own when the limit resets. The monitor and the
pool card both pass the seat's runtime through.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@sonichi

sonichi commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Text check at 690d64f (no restart): the usage-limit card body as core-input-watch.escalation_message renders it from this build, for a Claude seat and a Codex seat. Raising, dedupe, routing and resolve are unchanged since f33417a, whose live run covers them. Agent-assisted (Claude, Blue).

--- runtime=claude
I am blocked and cannot continue without you.

State: blocked-human — awaiting user: session-limit
Gate: session-limit

What the terminal is showing:
  You've hit your usage limit · resets 3pm

It continues when the limit resets. To unblock it sooner, open its terminal (tmux session sutando-core) and run /usage-credits to use extra credits, /login to switch to another account, or /model to switch to a model with usage left.

--- runtime=codex
I am blocked and cannot continue without you.

State: blocked-human — awaiting user: session-limit
Gate: session-limit

What the terminal is showing:
  You've hit your usage limit · resets 3pm

It continues when the limit resets. To unblock it sooner, open its terminal (tmux session sutando-core) and run `codex login` to switch to another account.

@sonichi

sonichi commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Live witness, owner-authorized, run on pro-comm at f33417a. The window was 2026-10-24 ~22:20 PT to 2026-10-25 02:33 PT, and the seat was restored inside the cap.

Phase Result
A1: no new limit card while the banner was stale PASS: 0 new cards over 116 ticks (~122 min). Before the swap, main's monitor raised 12 in ~3 h.
A2: supervisor state PASS: idle-ready on every tick
A3: the 12 old cards close with a single recovery DM PASS: all 12 were resolved 5 s after the swap, as in-place card edits (no notification) plus one recovery DM
A4: every PR sweep decides "nothing" PASS
B: the 02:00 PT reset PASS: the banner went from "continuing shortly" to "Usage limit reset · continuing automatically", then cleared, with no card raised
C: one limit card, delivered once, on a real refusal NOT OBSERVED: no refusal occurred on the seat during the window

Counter-evidence from the same night: from ~02:22 PT, seats still on main raised 9 cards during a session-limit episode. pro-comm, on this PR, raised none.

Head: the evidence is for f33417a. The current head 690d64f changes only the unblock wording: a runtime-aware "continues when the limit resets", a /model option, and the codex-login text. The raise, guard, resolve and re-notify logic is unchanged, so A and B carry over and only C's card text differs. C is still unwitnessed at either head.

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 690d64f7a3383b42cd46c180e6ef398a73fe86ba. No new code blocker in f33417a..690d64f: the delta threads the runtime into the shared unblock text and adds the Claude/model and Codex-login wording. The recorded 19 episode tests at this exact delta pass, including runtime-specific message coverage; I reused that evidence rather than rerunning unchanged tests. Raise/guard/resolve/re-notify policy is unchanged.

I accept the posted live A/B evidence: stale-banner suppression, idle-ready state, old-card resolution with one recovery DM, and the actual reset. These are meaningful live outcomes, and I am no longer holding them as missing. They can carry across this wording-only delta; no repeat A/B run is requested.

C is still required before my approval, unless there is an explicit repository-owner exception scoped to #5686. The witness expressly says no genuine refusal occurred. A3 demonstrates resolution/recovery delivery for existing cards; it does not show the changed positive path creating and delivering one actionable limit card while work is owed, without duplicate cards/delivery. Test-level C plus live A/B therefore does not close the particular live positive-round-trip gap underlying my §15 request. §15 does not mechanically require every test branch to be run live; this is the specific changed delivery transition still unobserved.

The remaining evidence is a real limited seat with work owed producing one new actionable card, delivery/readback once, and its relevant recovery/rearming behavior through the restarted supervisor/pool path. Keep the existing harness proof; add that live positive receipt when an authorized operator can observe it, or obtain an explicit scoped exception. The separate #5374 exception is not inherited by this child, and the argument that a wording-only change needs no new witness does not supply the missing C.

Changes requested remains solely this C acceptance condition. No request to manufacture a refusal, no live operation by this reviewer, no merge-readiness or deployment claim.

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at 690d64f7a3383b42cd46c180e6ef398a73fe86ba, replacing my C-only changes request.

I accept the live A1–A4/B receipt, including existing-card resolution and one actual recovery DM through the restarted path. Together with the retained positive-path harness evidence and the 19 tests for the runtime/wording delta, this is enough for my review. My earlier demand that the specific positive C transition also be witnessed before approval was too broad for the remaining bounded risk: missed or repeated usage-limit cards, not an authorization bypass or destructive operation. No new code defect is identified.

C remains unobserved, not passed. Track it as a post-merge follow-up: at the next genuine limit with work owed, verify one actionable card, one delivery/readback, persistence across sweep/restart without duplicates, recovery, and rearming for a new episode. Record the exact deployed revision and receipt here or in a linked follow-up. Do not manufacture a refusal just to close the checklist.

This is my evidence-based review disposition, not an assertion that a relayed preference is a direct repository-owner exception. No additional owner statement is needed for my approval on this basis. I did not run the live witness, rerun unchanged tests, merge, or assess every other merge gate.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve at 690d64f as code owner — agent-assisted review by Mars, Qingyun's Sutando seat, on Qingyun's instruction to review and approve Chi's PRs.

My e6617b1 review found the code approvable and held the approval for two things. Both are now in:

  1. #5374 landed first: it merged at 05:56Z.

  2. Live witness: owner-authorized, on pro-comm, over about 4 h:

    • no new limit card across 116 ticks while the banner was stale (main raised 12 in about 3 h)
    • the 12 old cards closed as in-place edits with one recovery DM
    • every sweep decided "nothing"
    • the 02:00 PT reset cleared with no card

    Phase C (exactly one card, delivered once, on a real refusal) wasn't observed, because no refusal happened during the window. That path is covered by core-input-watch-limit-episode's dedupe and the atomic store check-and-save I verified earlier, so I'm accepting it as tested rather than witnessed.

Since e6617b1: the main merges (#5374, #5862, #5855 and others), a re-vendor of hitl/renotify.py, and 690d64f, which only changes the card text: limit_unblock(session, runtime) gives a Codex seat codex login and a Claude seat /usage-credits, /login or /model, and both now say it continues at the reset. The monitor and the pool card pass the seat's runtime through. The shared limit_step rule is unchanged.

Tests: core-input-watch-limit-episode, pane-gate-contract, core-supervisor-relay and health-check-notify-gateway pass at this head under env -i. John approved at this head, and CI is green.

@sonichi
sonichi merged commit a0e25c5 into main Oct 25, 2026
39 checks passed
@sonichi
sonichi deleted the fix/health-card-once-per-episode branch October 25, 2026 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants