Skip to content

fix(dashboard): batch delete no longer toasts failure after success - #12711

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.51from
HouMinXi:fix/batch-delete-click-event
Sep 7, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.51from
HouMinXi:fix/batch-delete-click-event

Conversation

@HouMinXi

@HouMinXi HouMinXi commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Bug

On the provider detail page, bulk-deleting API keys (moonshot-native in this case) shows "deleted N connections" and then immediately "network error during batch delete". The keys are actually gone. The failure toast is a lie.

Fixes: dashboard bulk-delete success+failure toast pair.

Root Cause

ConfirmModal binds onConfirm to <Button onClick={onConfirm}>. A native click passes a MouseEvent.

handleBatchDeleteConfirm(onAfter?) treated any truthy first argument as a post-delete callback. After DELETE 200 it toasted success, then await onAfter() invoked the MouseEvent object, which throws, and the catch toasted batchDeleteNetworkError.

API side is fine: DELETE /api/providers with { ids } returns 200 and deletes the rows. The second toast is UI-only.

Measured 2026-09-04 against handleBatchDeleteConfirm + ConfirmModal wiring. Reproduction: pass a { type: "click" } stand-in as the first argument after a 200; success fires, then the network-error toast.

Fix

  • Run onAfter only when typeof onAfter === "function".
  • ConfirmModal confirm click: onClick={() => void onConfirm()} so the event never reaches the handler.

How to Verify

  1. Open a provider with several API keys (moonshot-native / any openai-compatible node).
  2. Select more than one key. Confirm bulk delete.
  3. Expect one success toast. No follow-up failure toast. Keys gone from the list.

Test Plan

  • tests/unit/ui/batch-delete-click-event.test.tsx — click-event stand-in after 200 does not toast error; a real onAfter still runs
  • Injection: revert typeof check → the click-event case goes red; restore → green
  • tests/unit/ui/confirm-delete-connection-7361.test.tsx still 3/3
  • tests/unit/providers-route-managed-catalog.test.ts still 6/6 (includes batch DELETE)

Risk

Low. ConfirmModal still calls onConfirm() with no args, which matches its type (() => void | Promise<void>). onAfter is unused in production callers today; the typeof guard is the belt, the ConfirmModal wrap is the suspenders.

Not in this PR

No chunking of DELETE past the existing 100-id API cap. Activate/deactivate already chunk; delete does not. Separate if you bulk-delete >100 at once.

Also in this commit (base hygiene that blocked CI)

  • Prefix unused useProviderConnections arg as _isSearchProvider (eslint @typescript-eslint/no-unused-vars on the file this PR already touches).
  • changelog.d/fixes/reset-aware-model-family.md now starts with - (fragment gate).
  • Register tests/unit/reset-aware-request-scope-12600.test.ts in stryker.conf.json tap.testFiles (mutation-test-coverage --strict).
  • open-sse/executors/glm.ts: drop the extra 16th arg to createSSETransformStreamWithLogger (TS2554; the 65536 never reached TransformStream).

Not in this PR: HuggingChat/stream public-error unit failures on release/v3.8.51, and local-only forge daemon_state (.code-forge/ is gitignored).

@HouMinXi

HouMinXi commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Technical review (LOCAL, review-default, baseline c3945a724) on 6aac03b83.

HOLD, 0 CONFIRMED, 2 UNCERTAIN. Not restarting rounds.

Adjudication:

  • typeof onAfter === "function" in handleBatchDeleteConfirm: this is the bug. ConfirmModal used to bind onClick={onConfirm}, so React passed a MouseEvent into onAfter. The truthiness check then called event(), which threw and the catch toasted batchDeleteNetworkError after a successful DELETE. tests/unit/ui/batch-delete-click-event.test.tsx covers that path. Other ConfirmModal callers either wrap with () => void handler() or pass a zero-arg confirm; only this handler accepted an optional callback and treated a truthy argument as callable.
  • onClick={() => void onConfirm()} on ConfirmModal: not cosmetic. ConfirmModalProps.onConfirm is () => void | Promise<void> — no event. The wrapper stops React from injecting the MouseEvent. Together with the typeof guard, the click event never reaches the batch-delete handler.

glm.ts dropping the extra positional is the TS2554 fix the other open PRs inherit from this branch. Unused KeyHealth / translateJsonResponse were already on the tip.

CI remaining: merge-integrity (cli-tunnel skill dry-run generate + tip changelog fragment without - ), unit shards. Same fingerprint as #12732. Typecheck / FQG on this PR are green.

Contributor only — not merging.

@HouMinXi

HouMinXi commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto upstream/release/v3.8.51 @ 9d1a896c6 (#12670).

range-diff every unique commit =. All rewritten commits GPG G. behind=0 vs tip.

Own tests re-run on the new base. --force-with-lease to the previous fork SHA.

⚠️ base-red inherited: #12732 (FQG / unit shards / changelog-integrity / glm arity until #12770 lands). Files this PR does not touch.

@HouMinXi

HouMinXi commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto release/v3.8.51 tip f9a1cc8a9 (#12682 / #12691 / #12834).

cb6a70c37 → 45c645244. range-diff 2 commits =. GPG G. No file overlap with the three landed commits.

ConfirmModal binds onConfirm to Button onClick, so the click MouseEvent
was forwarded into handleBatchDeleteConfirm's optional onAfter. After a
200, the success toast fired and then `await onAfter()` threw, which the
catch treated as a network error.

Treat onAfter as a callback only when it is a function, and stop
forwarding the click event from ConfirmModal.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
Cherry-pick onto the deploy recut conflicted with cheaper-inference#12770 on
open-sse/executors/glm.ts. This PR only fixes ConfirmModal forwarding
the click event into onAfter. Restore glm.ts to the release tip so
the arity fix stays on cheaper-inference#12770.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
@HouMinXi
HouMinXi force-pushed the fix/batch-delete-click-event branch from 45c6452 to b3503bc Compare September 7, 2026 02:35
@HouMinXi

HouMinXi commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto upstream/release/v3.8.51 b345c7f6c (#12870 OpenCode v2 plugin).

  • 45c645244b3 → b3503bc4194
  • range-diff identity (eq=2); all commits GPG G
  • 2 unique commit(s) ahead, 0 behind
  • --force-with-lease to HouMinXi:fix/batch-delete-click-event

diegosouzapw pushed a commit that referenced this pull request Sep 7, 2026
Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

O `65536` era o 16º posicional de um helper com 15 parâmetros — `TS2554` vivo no tip (`open-sse/executors/glm.ts:244`, confirmado aqui antes do board). O teste de guarda de aridade é o que impede a reincidência: ele checa a assinatura do helper e o call site, não o comportamento, que é exatamente onde o erro morava.

Obrigado por isolar isso do #12711 em vez de deixar o `glm.ts` viajar junto com pin/combo-split/moonshot.
@diegosouzapw
diegosouzapw merged commit 25bc16d into cheaper-inference:release/v3.8.51 Sep 7, 2026
8 of 16 checks passed
@HouMinXi
HouMinXi deleted the fix/batch-delete-click-event branch September 7, 2026 13:27
diegosouzapw added a commit that referenced this pull request Sep 10, 2026
…agment (#13200)

Um caractere. O fragmento do #13097 subiu sem o `- ` inicial e derrubou o `Merge integrity` para todo mundo que veio depois.

Terceira ocorrência da mesma causa nesta release; a anterior foi o `reset-aware-model-family.md`, que o #12711 consertou de carona.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…nce#12770)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

O `65536` era o 16º posicional de um helper com 15 parâmetros — `TS2554` vivo no tip (`open-sse/executors/glm.ts:244`, confirmado aqui antes do board). O teste de guarda de aridade é o que impede a reincidência: ele checa a assinatura do helper e o call site, não o comportamento, que é exatamente onde o erro morava.

Obrigado por isolar isso do cheaper-inference#12711 em vez de deixar o `glm.ts` viajar junto com pin/combo-split/moonshot.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…heaper-inference#12711)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

Além do bug do toast, esta PR foi a que derrubou os três base-reds vivos do tip: o fragmento `changelog.d/fixes/reset-aware-model-family.md` sem o `- ` inicial, o registro do `tests/unit/reset-aware-request-scope-12600.test.ts` no `stryker.conf.json` e o `TS2554` do glm. O `check-changelog-integrity` voltou a passar aqui por causa dela.

O diagnóstico do MouseEvent é o que dá o valor: `onConfirm` chegava como handler de clique nativo e `handleBatchDeleteConfirm` tratava qualquer primeiro argumento truthy como callback. O cinto (`typeof`) e o suspensório (o wrap no ConfirmModal) juntos estão certos — só um dos dois deixaria a porta aberta para o próximo caller.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…agment (cheaper-inference#13200)

Um caractere. O fragmento do cheaper-inference#13097 subiu sem o `- ` inicial e derrubou o `Merge integrity` para todo mundo que veio depois.

Terceira ocorrência da mesma causa nesta release; a anterior foi o `reset-aware-model-family.md`, que o cheaper-inference#12711 consertou de carona.
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