fix(dialogs): wait for the confirmation before deleting or removing the key #465
No reviewers
Labels
No labels
autopilot:pending-human
source:analyste
source:defenseur
source:human
source:medic
status:approved
status:blocked
status:in-progress
status:needs-clarification
status:needs-fix
status:ready
status:review
status:triage
type:bug
type:feature
type:infra
type:refactor
type:schema
type:security
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: maximus/Simpl-Resultat#465
Loading…
Reference in a new issue
No description provided.
Delete branch "issue-421-confirm-dialog"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Refs #421
Cause
On desktop, tauri-plugin-dialog 2.6.0 runs an init script,
init-iife.js. Read in the crate source, it containswindow.confirm=async function(i){return await invoke("plugin:dialog|confirm",...)}.confirm(...)therefore returns a Promise, which is always truthy, so everyif (!confirm(...)) return;let the action run before the user answered. Deleting a secondary profile removed its database even on "Cancel". The plugin also replaceswindow.alertwith a function that does not block.Changes
confirmAction(message)(src/services/confirmService.ts) callsaskfrom@tauri-apps/plugin-dialogwithkind: "warning". The buttons arecommon.confirm/common.cancel(both keys already exist in FR and EN), taken from the shared i18next instance, so they follow the app language. A failed call resolvesfalseand goes throughlogWarn(fail closed). This is the only service that translates; its doc comment andarchitecture.mdsay so.The six sites await it:
ProfileFormModal,CategoryDetailPanel,AdjustmentDetailPanel,TemplateActionsandAccountsPage(handleDeleteCategorybecameasync, with the linked-accounts check kept before the dialog), plus both confirmations ofperformRemoveLicense.RemoveLicenseUi.confirmis now(message) => Promise<boolean>, andLicenseCardpassesconfirmAction.@tauri-apps/plugin-dialog2.6.0, the version of the crate, as for the other plugins (opener, process, sql and updater are each at the version of their crate). The lock gains one key,node_modules/@tauri-apps/plugin-dialog; the only other change in the lock is the root entry. Itsintegritymatchesnpm view. A clean-roomnpm ciwith npm 11 passes and does not rewrite the lock. The package's code (dist-js/index.js, 197 lines) only callsinvoke('plugin:dialog|…'): it has no install script and no network call. Its only dependency,@tauri-apps/api ^2.8.0, is already in the lock at 2.10.1. Thedialog:defaultpermission already includesallow-ask(crate'spermissions/default.toml): nothing changes on the Rust side.Guard
src/__integration__/native-dialog-guard.test.tsscans the non-test sources ofsrc/, with comments removed and line numbers kept. It fails on any nativeconfirm/alertcall, bare or throughwindow,globalThisorself. It also fails on anyconfirmAction(or<x>.confirm(call not directly preceded byawait. The first form is not covered by TypeScript, since the DOM type ofconfirmstaysboolean; the second is not either, since!promiseis not a TS error (mutation M1:tscgreen). Canaries:Run before the fix, the guard reported exactly the six sites (eight lines).
CHANGELOG FR/EN: a Fixed entry.
docs/architecture.md: the service (counts 26 to 27 and 20 to 21) and thetauri-plugin-dialogplugin row.prompt(...)(PIN entry inProfileFormModal, renaming inAccountsPage) is out of scope, as the issue says. The plugin does not replace it.Tests
useLicenseMachines.test.ts, the confirmations now resolve asynchronously, asaskdoes. On the old code, 6 tests are red: both refusals, the first and the second (the second once per retry code). They are green after the fix.confirmService.test.ts, 5 tests: true, false, rejection giving false pluslogWarn, French labels, English labels afterchangeLanguage("en");ProfileFormModal.delete.test.ts, 2 tests on the hook harness: a click on the trash can with a refused confirmation does not calldeleteProfile, and with an accepted one it is called with the profile's id;npm run buildis green.npm test: 137 files, 2285 tests (2256 before). Probe first: a deliberately red test exits 1.Mutations, one at a time on the committed tree, restored by git; the table counts red tests out of 113 (guard, service, license, profile):
awaitdropped inCategoryDetailPanelconfirmActionreplaced bywindow.confirminAccountsPageaskrejection resolvedtrueawaitdropped on the 1st license confirmationawaitdropped on the 2nd license confirmationawaitdropped inProfileFormModalLicenseCardback towindow.confirm(wrapped inasync)Post-merge checklist (hence
Refs, not a closing keyword).dbfile underapp_config_dir()are still there.prompt()while there. Open a separate issue if the entry fails.Review level
High per
REVIEW.md:package.jsonadds a dependency andpackage-lock.jsongains anode_modules/<name>key (zone Dépendances). The license files touched (useLicenseMachines.ts,LicenseCard.tsx) are not in the High table.🤖 Generated with Claude Code
Niveau : haut —
package.jsonetpackage-lock.json(Dépendances : ajout de@tauri-apps/plugin-dialog, nouvelle clénode_modules/@tauri-apps/plugin-dialog)Max doit lire avant le merge.
Verdict : APPROVE
Le correctif est juste et prouvé. Les six sites attendent
confirmAction, qui passe paraskdu plugin et échoue fermé. La cause annoncée se vérifie dans la source du crate : leinit-iife.jsdetauri-plugin-dialog2.6.0 remplace bienwindow.confirmpar une fonctionasyncetwindow.alertpar une fonction qui n'attend rien.Vérifications rejouées (tête
1557227)git archivede la tête,node_modulescopié) :npm run buildvert. Sonde : un test volontairement rouge sort en 1.npm test: 137 fichiers, 2285 tests verts, comme annoncé.check-frontendrun 566 vert. Son journal indique 137 fichiers, 2285 tests etfound 0 vulnerabilitiesaunpm ci.check-rustn'avait pas à tourner : aucun fichier soussrc-tauri/.~/.cargo/registry/.../tauri-plugin-dialog-2.6.0) :askconstruitOkCancelCustom(yes, no)avecbuilder.parent(&window)sous#[cfg(desktop)]. La boîte est donc modale pour la fenêtre, et un second clic pendant l'attente n'est pas possible.showrendtrueseulement si le bouton cliqué porte le libellé OK. Les libellés FR (Confirmer/Annuler) et EN (Confirm/Cancel) sont distincts, et un refus ou une fermeture donnefalse.permissions/default.tomlcontientallow-ask, etcapabilities/default.jsona déjàdialog:default.Cargo.lock. L'integritydu lock est identique ànpm view @tauri-apps/plugin-dialog@2.6.0 dist.integrity. Seul script :build: rollup -c, aucun script d'installation. Lesdist-jsne font que desinvoke('plugin:dialog|…'), sansfetchnirequire. Le diff du lock fait 10 lignes : la racine et la nouvelle clé.architecture.md: 27 services danssrc/services/, 21 lignes dans la section « Services TypeScript ». « Seul service qui traduit » est exact :licenseService.tsne cite i18n que dans des commentaires et des noms de champ.pull_refavecref_action: "none". Aucuncloses, ce qui est cohérent avecRefs #421et la fermeture manuelle après la liste post-merge.Mutations du relecteur
Une mutation à la fois, sur la copie. On compte les tests rouges parmi les 113 des quatre fichiers concernés (garde, service, licence, profil).
TemplateActions:if (confirmAction(…))sansawaitconfirmService:catchqui rendtrueconfirmService:return ask(…)sansawait, donc le rejet échappe autry"Confirmer")AWAITEDaccepte toutwindow.détecté (forme nue,globalThisetselfignorés)ProfileFormModal: aliasconst ask = confirmAction; if (!ask(…))AccountsPage: même aliasR4 mérite d'être nommée : c'est le
return awaitdans letryqui rend l'échec fermé réel, et le test du rejet l'épingle bien.Passe sécurité (niveau haut)
eval, pas d'HTML injecté. Le message passe au dialogue natif en texte.REVIEW.md:useLicenseMachines.tsetLicenseCard.tsxne sont pas dans le tableau Haut, et le diff ne touche ni jeton ni édition.common.confirmetcommon.cancelexistent dans les deux langues, et le test EN le prouve.Aucun constat.
Suggestions (non bloquantes)
c(m)oùcvautconfirmActionéchappe aux deux règles. Cette forme ne se rencontre que si on l'écrit exprès, mais une troisième règle fermerait le trou : toute référence àconfirmActionqui n'est ni un appel attendu, ni l'import, ni la déclaration, niconfirm: confirmActionserait refusée. Même logique pourwindow["confirm"](…),confirm?.(…)etconst { confirm } = window, queNATIVE_CALLne voit pas.AccountsPage.Actions faites
scratchpad/pr465).f75fcee12btoff9656d015Merged by fast-forward through the API on 2026-10-18 (
mainff9656d), on Max's request (« corrige 1 et /merge-stack »).Added after the review (comment 24953), not reviewed:
ff9656d(f75fceebefore the rebase), test-only. It applies suggestion 1: any mention ofconfirmActionother than the awaited call, its declaration, a plain named import orconfirm: confirmActionis refused, andwindow["confirm"](…),confirm?.(…)andconst { confirm } = windoware flagged. Guard 22 to 34 tests. The reviewer's surviving mutation (const c = confirmAction; if (!c(...))inAccountsPage) is now red (1 test), as are the three native forms (1 each). No production code changed.Rebase:
mainhad moved to5222329(PR #450,.claude/skills/release/SKILL.mdonly, no file in common). Both commits were rebased onto it; the full suite was rerun locally (2297 tests) and CI ran on the rebased head (check-frontendgreen, 2297 tests).Suggestion 2 (behavioural tests for the four other sites) goes to a follow-up issue. #421 stays open (
Refs) until Max's manual check (post-merge checklist of the PR body).