Skip to content

Fix: floor-wide delivery-pause toggle can silently misreport its own state - #530

Open
UsryAce wants to merge 1 commit into
HarnessMD:mainfrom
UsryAce:fix/floor-delivery-toggle-sync
Open

UsryAce wants to merge 1 commit into
HarnessMD:mainfrom
UsryAce:fix/floor-delivery-toggle-sync

Conversation

@UsryAce

@UsryAce UsryAce commented Sep 15, 2026 •

Copy link
Copy Markdown

Summary

`toggleFloorDelivery()` in `CommandCenterPanel.tsx` sets the displayed `floorDeliveryPaused` value optimistically, before any of the per-agent `controlAutoDelivery(a.id, next)` IPC calls resolve:

```ts
const toggleFloorDelivery = async () => {
const next = !floorDeliveryPaused;
setFloorDeliveryPaused(next); // shown immediately
const all = useStore.getState().agents;
await Promise.all(all.map((a) => window.cth.controlAutoDelivery(a.id, next).catch(() => null)));
};
```

Every per-agent call is wrapped in `.catch(() => null)`, and `controlAutoDelivery` actually resolves each agent's real resulting `AgentControlSnapshot` — but that return value, success or failure, is discarded entirely. So:

  • A failed call for any individual agent (main-process exception, an agent whose control record isn't ready yet, an IPC hiccup) is silently dropped.
  • There's no reconciliation afterward, and nothing surfaces the failure to the operator.

Why this matters

This is the ONE floor-wide switch for pausing auto-delivery to every agent. An operator could click "pause delivery" — often specifically because they're about to do something they don't want agents auto-acting during — see the switch confidently show "Delivery Paused" for the whole floor, while one or more agents (plausibly one mid-spawn or mid-teardown, which is exactly when a race like this bites) never actually got `autoDeliveryPaused` flipped in the main process's `ControlRegistry`.

Fix

Keeps the optimistic update for UI responsiveness, but now actually reads the per-agent results afterward. If any agent's returned snapshot is missing or doesn't match the requested state, it logs which agent(s) and re-syncs the displayed toggle from god's own live `controlSnapshot` instead of trusting the optimistic guess.

Test plan

  • `npm run typecheck` passes
  • `npm run build` succeeds
  • Maintainer/CI: the failure path (an agent not confirming) is timing-dependent and hard to force deterministically in a quick local check; the change itself only adds a read-and-reconcile step after the existing calls, so behavioral risk on the happy path is minimal.

🤖 Generated with Claude Code

Before

A minimal repro reproducing the exact control-flow shape of toggleFloorDelivery(), with a mocked controlAutoDelivery where one agent (simulating one mid-spawn, whose control record isn't ready yet) rejects:

$ node demo-floor-delivery.cjs

=== OLD toggleFloorDelivery ===
UI shows floorDeliveryPaused = true (no indication darryl-mid-spawn never actually paused)

The optimistic value is returned/displayed as success with zero indication that one agent's call actually failed - exactly what the real code does, since the per-agent result is discarded via .catch(() => null) and never inspected.

After

Same mock, same failing agent, using the new logic (read the per-agent results, detect the mismatch, log which agent(s) failed):

=== NEW toggleFloorDelivery ===
[demo] 1/5 agent(s) did not confirm: darryl-mid-spawn - re-syncing
{
  "displayedAsPaused": true,
  "actuallyConfirmedForAll": false,
  "failedAgents": [
    "darryl-mid-spawn"
  ]
}

The failure is now detected and surfaced instead of silently swallowed; the real code additionally re-syncs the displayed toggle from god's own live controlSnapshot at this point (not reproducible standalone without the real IPC layer, but the detection - the actual bug - is demonstrated here).

…state

toggleFloorDelivery() set the displayed floorDeliveryPaused value optimistically
before any of the per-agent controlAutoDelivery(a.id, next) IPC calls resolved,
then fired all of them with .catch(() => null) - so a failed call for any
individual agent (main-process exception, an agent whose control record isn't
ready yet, an IPC hiccup) was silently dropped, and the returned snapshots
(controlAutoDelivery actually resolves each agent's real resulting
AgentControlSnapshot) were discarded entirely, success or failure.

Net effect: the operator could click "pause delivery", see the floor-wide
switch confidently show "Delivery Paused", while one or more agents - most
likely ones mid-spawn or mid-teardown, exactly when an operator might want to
pause things - never actually had autoDeliveryPaused flipped in the main
process's ControlRegistry.

This keeps the optimistic update for responsiveness but now actually reads the
per-agent results: if any agent didn't confirm the requested state, it logs
which ones and re-syncs the displayed value from god's own live snapshot
instead of trusting the optimistic guess.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🚫 This PR is missing its before/after evidence

Every pull request here has to show its work. Screenshots or a short screen recording, before the change and after it.

  • Before — no image or video under that heading
  • After — no image or video under that heading

How to fix it: edit the description, keep the ### Before and ### After headings from the template, and drag an image or video under each. GitHub uploads it inline. This check re-runs the moment you save.

A bug fix with no visible surface still needs it: show the failing behaviour, then the same steps passing. A terminal recording is fine.

Genuinely nothing to show — a CI tweak, a typo, a dependency bump? A maintainer can apply the no-visual-change label. Please don't ask unless it truly has no observable effect.

📖 CONTRIBUTING.md → Evidence is mandatory

@chaitanyagiri

Copy link
Copy Markdown
Collaborator

Thank you, @UsryAce. We took this whole into 0.5.5, so it is already in the installers people are running. We want to merge it here too, so the credit sits on your pull request.

One thing stands in the way. The "Before / after evidence" check is red: the description has no picture or recording under the Before and After headings. For a fix with nothing to see on screen, a terminal screenshot works: the failing behaviour or test before, and the same steps passing after. Edit the description and drag one under each heading. The check runs again by itself.

The Build and Typecheck checks on this pull request are waiting on us, not on you. Once that is in and the checks are green, we merge it.

This branch has not been deployed

No deployments
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.

2 participants