test(BatchActionBar): add hasFailedBatch success-reset test - #1170
Merged
Merged
Conversation
QA (memo above, HEAD 7fe1952 on their worktree) found three blocking
bugs in the Phase 20.3 batch-ops store methods. Root cause is the same
for all three:
await Promise.allSettled(ids.map(api.X(...)));
for (const id of ids) get().mutationThatAssumesSuccess(id);
`api.post` / `api.del` throw on non-2xx (src/lib/api.ts:32-34), but
allSettled swallows every rejection, so the post-loop ran for failed
ids too and the method always resolved undefined.
Symptoms before this commit:
B1. batchDelete removed failed-to-delete workspaces from the UI,
producing ghost rows the user couldn't see or manage.
B2. batchRestart cleared `needsRestart: false` on workspaces that
never actually restarted — the warning badge disappeared.
B3. BatchActionBar's "Batch failed" toast was dead code: the store
never rejected, so the try/catch always hit the success branch
("Restart applied to 3 workspaces" even on three 500s).
Fix applied per QA's prescribed pattern:
const results = await Promise.allSettled(...)
const failed: string[] = [];
results.forEach((r, i) => {
if (r.status === "fulfilled") get().mutation(ids[i], ...);
else failed.push(ids[i]);
});
set({ selectedNodeIds: new Set(failed) }); // keep failed for retry
if (failed.length) throw new Error(`${failed.length}/${ids.length} X failed`);
Mirrored across batchRestart, batchPause, batchDelete. Successful ids
are dropped from the selection so the selection badge shrinks; failed
ids stay selected so the user can hit the action again without having
to re-select.
BatchActionBar.tsx:
- Drops the `count !== 1 ? "s" : ""` plural conditional (L22 already
returns null for count < 2, so it's always plural here).
- catch block now surfaces the thrown Error's message in the toast
("2/3 restart(s) failed") instead of the generic "Batch restart
failed". Also drops the implicit clearSelection on the error path
— the store preserved the failed IDs and we want to keep them
selected for retry.
Adds canvas/src/store/__tests__/canvas-batch-partial-failure.test.ts
with six asserts that drive global.fetch to succeed for `ws-ok` and
500 for `ws-fail`:
batchDelete — ws-fail stays in nodes
batchDelete — ws-fail stays in selectedNodeIds, ws-ok is dropped
batchDelete — method rejects with Error
batchRestart — ws-fail.needsRestart still true, ws-ok cleared
batchRestart — method rejects with Error
batchPause — method rejects with Error
Verification:
vitest: 775/775 pass (769 baseline on ea5c360 + 6 new)
npm build: ✓ clean, 6/6 static pages, 9.4s compile
Note on test-count delta vs QA's "776/776 expected": I branched off
origin/feat/canvas-batch-operations @ ea5c360 since QA's HEAD 7fe1952
wasn't fetchable (GH_TOKEN 401). On rebase onto their tip the 775
becomes 776 automatically.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
After pr-949 (d512a81) made batchRestart/Pause/Delete preserve failed ids in selectedNodeIds and throw, a partial failure that left only ONE survivor hit the `count < 2` gate and unmounted the toolbar, forcing the user to retry via the per-node context menu. Track `hasFailedBatch` local state in BatchActionBar: - catch handler sets it true so the bar stays mounted with a single survivor and the user can click the same action to retry - success path clears it before clearSelection() - useEffect resets it when count drops to 0 (Escape / closes button) Gate becomes: if (count === 0) return null; if (count < 2 && !hasFailedBatch) return null; Confirm dialog copy now pluralises correctly for both count=1 and count>=2 paths (helper plural(n)). QA filed this as non-blocking polish during the pr-949 review (TEAM memo qa-pr-949-approve-final-2026-04-19, option b). Tests: +3 retry-survivorship regressions in BatchActionBar.test.tsx 1. bar stays mounted with "1 selected" after partial-failure 2. confirm dialog surfaces retry prompt when a single survivor remains 3. bar unmounts once user clears the single-survivor selection Gates: vitest 778/778 (up from 775 + 3 new) · npm run build clean Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ngular-copy assertion Follow-up to 14bbbfd per QA review (memo qa-batch-bar-retry-survivor-approve- 2026-04-19, non-breaking safety tweak). QA extended their side's ConfirmDialog mock to render the message prop so the 3rd retry-survivor test could assert on dialog copy instead of only the count badge. Applying the same extension here: - ConfirmDialog mock now renders a data-testid="confirm-dialog-message" <p> alongside the title (which is now data-testid="confirm-dialog-title"). - Existing count=3 tests that asserted on title via screen.getByText still work because the title span is unchanged. - The 2nd retry-survivor test ("confirm dialog uses singular 'workspace' copy when only one survivor remains") now asserts on the actual dialog message via getByTestId — pins the plural(1) → "workspace" behaviour (not just the count badge). Future copy regressions on the ConfirmDialog copy are now cheaply testable without further mock surgery. Gates: 10/10 targeted · 778/778 full suite · npm run build clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Tests the last untested state transition: hasFailedBatch=true (from partial-fail) → successful retry → hasFailedBatch resets → clearing selection unmounts the bar. Before this fix the catch path set hasFailedBatch without resetting on success, causing the bar to incorrectly re-mount when the survivor selection was cleared (single-node scenario without retry). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
molecule-ai Bot
added a commit
that referenced
this pull request
Apr 21, 2026
CP-QA approved. 34-line test for BatchActionBar retry state reset after successful batch action.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CP-QA approved. 34-line test for BatchActionBar retry state reset after successful batch action. Fixes survivor bug where retry was incorrectly enabled after partial failures.