OpsDeck/core
Watch
2
1
Fork
You've already forked core
 0
Code Issues 12 Pull requests Packages 1 Activity Actions

fix: a button that starts work sends one request and shows the run it started #73

Merged
thisilike merged 3 commits from fix/double-submit into main 2026-10-19 10:38:50 +02:00
Conversation 2 Commits 3 Files changed 17 +418 -82
thisilike commented 2026-09-23 15:42:07 +02:00
Owner

Clicking "Free 231 GiB" in the prune dialog gave no feedback beyond a background activity appearing in Activity, and the button could be spammed. The backend was already safe: prune holds the storage lock with onSelfConflict: drop, so duplicates became 409s or skipped rows. The frontend was not, and an audit found the same pattern in about 20 places.

Why the prune dialog showed nothing

It subscribed to core:task:<id>, which is a retained topic, but read only onData. The run has usually reported running before the subscription connects, so that news arrives only in the snapshot, which was dropped. The form, with its button live, stayed up until the sweep ended. Each extra click also leaked a subscription. The dialog then followed the skipped duplicate, which it rendered as running forever.

Commits

fix(docker): prune dialog. Adds a busy flag, held until the snapshot lands. The dialog adopts the snapshot and folds progress and step deltas into the run. It stops the previous subscription before opening another. skipped and interrupted now count as terminal. The reducer lives in run_model.ts, with tests, so the other pages that follow a single run can share it.

fix(shell): generic actions and forms. Handlers set busy but never checked it, and a disabled button lands a frame after a fast double click. Where the server queues rather than drops, this did double work: filebrowser copy/move/delete are wait actions, so a second Retry on RunPage queued them again, and minecraft's discard-latest popped two edits.

  • ConfirmDialog holds itself busy while an async onconfirm is in flight, on click and on Enter. This covers callers that pass no busy.
  • ActionsBlock, FormBlock, RunPage retry/cancel, schedules, Activity's run button and the module-storage clear all return early while a request is in flight.
  • ActionsBlock stops its previous progress subscription before opening a new one. For a plain POST that succeeds, it says "done" instead of showing nothing.

fix(docker): the other pages.

  • Volume import and update-all adopt the snapshot. Update-all could sit disabled until a reload when every stack was skipped.
  • Stack and container update buttons stay busy until the runs feed names the run, with a 15 s fallback. Before, a double click was reported as "already running" under an update that was going fine.
  • Container start/stop/restart are guarded. They are not tasks, so the engine received every click.
  • On stack detail and image pull, a 409 no longer drops the follow of the run that caused it.
  • "Measure now" and log truncate now report their failures instead of rejecting unhandled.

Verification

  • deno fmt --check, deno lint, deno task check: clean. check:svelte: 0 errors, no new warnings in the touched files. deno task test: 1019 passed, 0 failed.
  • The branch sits on 1c76917 (#66's head). A test-merge onto current main is clean, but the suite has not been re-run on the merged tree.
  • Tested with Playwright against OPSDECK_FAKE_DATA=1:
    • 10 clicks on "Free" in one frame: one run.
    • 10 clicks on "start container": one POST.
    • 10 clicks on "Recreate it": one run, no skipped duplicate.
    • A prune already running when the dialog confirms: the 409 is shown and the button works again.
  • Not tested in the browser: ActionsBlock's "done" label, FormBlock, and volume import.

Not in here

The truncate route ignores ?wait=1, which StoragePage sends, so the page reloads the log size from before the truncate. That is a backend change.

Downstream

The same guards went into module_filebrowser, module_minecraft and module_healthchecks. In filebrowser, the create/rename prompt's Cancel was a submit button (Button has no type), so Create fired twice per click, and Cancel with a name typed created the item.

🤖 Generated with Claude Code

Clicking "Free 231 GiB" in the prune dialog gave no feedback beyond a background activity appearing in Activity, and the button could be spammed. The backend was already safe: prune holds the `storage` lock with `onSelfConflict: drop`, so duplicates became 409s or `skipped` rows. The frontend was not, and an audit found the same pattern in about 20 places. ### Why the prune dialog showed nothing It subscribed to `core:task:<id>`, which is a retained topic, but read only `onData`. The run has usually reported `running` before the subscription connects, so that news arrives only in the snapshot, which was dropped. The form, with its button live, stayed up until the sweep ended. Each extra click also leaked a subscription. The dialog then followed the `skipped` duplicate, which it rendered as running forever. ### Commits **`fix(docker)`: prune dialog.** Adds a busy flag, held until the snapshot lands. The dialog adopts the snapshot and folds progress and step deltas into the run. It stops the previous subscription before opening another. `skipped` and `interrupted` now count as terminal. The reducer lives in `run_model.ts`, with tests, so the other pages that follow a single run can share it. **`fix(shell)`: generic actions and forms.** Handlers set busy but never checked it, and a disabled button lands a frame after a fast double click. Where the server queues rather than drops, this did double work: filebrowser copy/move/delete are `wait` actions, so a second Retry on RunPage queued them again, and minecraft's `discard-latest` popped two edits. - `ConfirmDialog` holds itself busy while an async `onconfirm` is in flight, on click and on Enter. This covers callers that pass no `busy`. - `ActionsBlock`, `FormBlock`, RunPage retry/cancel, schedules, Activity's run button and the module-storage clear all return early while a request is in flight. - `ActionsBlock` stops its previous progress subscription before opening a new one. For a plain POST that succeeds, it says "done" instead of showing nothing. **`fix(docker)`: the other pages.** - Volume import and update-all adopt the snapshot. Update-all could sit disabled until a reload when every stack was skipped. - Stack and container update buttons stay busy until the runs feed names the run, with a 15 s fallback. Before, a double click was reported as "already running" under an update that was going fine. - Container start/stop/restart are guarded. They are not tasks, so the engine received every click. - On stack detail and image pull, a 409 no longer drops the follow of the run that caused it. - "Measure now" and log truncate now report their failures instead of rejecting unhandled. ### Verification - `deno fmt --check`, `deno lint`, `deno task check`: clean. `check:svelte`: 0 errors, no new warnings in the touched files. `deno task test`: 1019 passed, 0 failed. - The branch sits on `1c76917` (#66's head). A test-merge onto current `main` is clean, but the suite has not been re-run on the merged tree. - Tested with Playwright against `OPSDECK_FAKE_DATA=1`: - 10 clicks on "Free" in one frame: one run. - 10 clicks on "start container": one POST. - 10 clicks on "Recreate it": one run, no skipped duplicate. - A prune already running when the dialog confirms: the 409 is shown and the button works again. - Not tested in the browser: `ActionsBlock`'s "done" label, `FormBlock`, and volume import. ### Not in here The truncate route ignores `?wait=1`, which `StoragePage` sends, so the page reloads the log size from before the truncate. That is a backend change. ### Downstream The same guards went into module_filebrowser, module_minecraft and module_healthchecks. In filebrowser, the create/rename prompt's Cancel was a submit button (`Button` has no `type`), so Create fired twice per click, and Cancel with a name typed created the item. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
The dialog subscribed to the run's retained topic but read only deltas, so
the snapshot carrying "running" was dropped and the form stayed up, button
live, until the sweep ended. Spamming it produced 409s or skipped runs in
Activity with nothing on screen.

- guard the confirm with a busy flag held until the snapshot lands
- adopt the snapshot; fold progress and step deltas into the run
  (run_model.ts, shared with the other pages that follow one run)
- stop the previous subscription before opening another
- skipped and interrupted are terminal, not an endless spinner
Handlers set their busy flag but never checked it, and a disabled button
lands a frame after a fast double click. Where the server queues rather
than drops (filebrowser copy/move/delete are "wait"), a second Retry did
the work twice; minecraft's discard-latest popped two edits.

- ConfirmDialog holds itself busy while an async onconfirm is in flight,
  covering callers that pass no busy prop, on click and on Enter
- ActionsBlock, FormBlock, RunPage retry/cancel, schedules, activity run
  and module-storage clear return early while in flight
- ActionsBlock stops a previous progress subscription before a new one,
  and says "done" for a plain POST that answered instead of nothing
- schedules no longer leave busy set when the request throws
fix(docker): every button that starts work waits for it before listening again
All checks were successful
Build and Deploy / verify (pull_request) Successful in 2m30s
Details
Build and Deploy / build (pull_request) Has been skipped
Details
Dependency Check / dependencies (pull_request) Successful in 1m9s
Details
7dfb82bed8
 
The same gap as the prune dialog, page by page: busy cleared when the POST
answered but before the run was on screen, handlers without an early
return, and follows that read only deltas.

- volume import and update-all adopt the retained snapshot, stop their
  previous subscription, and treat every terminal status as the end
- stack and container updates hold the row busy until the runs feed
  names the run, so a double click is not reported as "already running"
- start/stop/restart are guarded: they are not tasks, and the engine
  would restart twice
- stack detail, image pull: a refusal no longer drops the follow of the
  run that caused it
- measure now and log truncate report failures instead of rejecting
julian approved these changes 2026-09-23 16:07:17 +02:00
julian left a comment

Reviewed the three commits against 1c76917. I found no blocking bugs. Every busy flag resets on success, error and throw. ConfirmDialog's new pending resets before it rethrows, so callers that pass their own busy are unaffected. Each follow closes its previous subscription before it opens a new one.

Nits, none blocking:

  • VolumePage.svelte:119-134: uploading is now reset only on error or a null snapshot. When the run is adopted it stays true. Nobody can see that today, because a set importRun replaces the DropZone for the life of the page. But if anyone later clears importRun to allow a second import, the zone will come back disabled. A comment there, or a reset in adopt, would keep that from surprising someone.
  • ActionsBlock.svelte:190: doneTimer is not cleared on unmount. It is harmless in Svelte 5. $effect(() => () => clearTimeout(doneTimer)) would tidy it up.

As the description says, the suite has not been run on the tree merged with current main. Please run it before merging.

Reviewed the three commits against `1c76917`. I found no blocking bugs. Every busy flag resets on success, error and throw. ConfirmDialog's new `pending` resets before it rethrows, so callers that pass their own `busy` are unaffected. Each follow closes its previous subscription before it opens a new one. Nits, none blocking: - `VolumePage.svelte:119-134`: `uploading` is now reset only on error or a null snapshot. When the run is adopted it stays `true`. Nobody can see that today, because a set `importRun` replaces the DropZone for the life of the page. But if anyone later clears `importRun` to allow a second import, the zone will come back disabled. A comment there, or a reset in `adopt`, would keep that from surprising someone. - `ActionsBlock.svelte:190`: `doneTimer` is not cleared on unmount. It is harmless in Svelte 5. `$effect(() => () => clearTimeout(doneTimer))` would tidy it up. As the description says, the suite has not been run on the tree merged with current `main`. Please run it before merging.
thisilike commented 2026-10-19 10:38:49 +02:00
Author
Owner

Ran the full suite on this branch merged with current main (323e746), as the approval asked. The merge was clean.

  • deno task test: 1051 passed, 0 failed, 1 ignored
  • deno task check, deno lint and deno fmt --check: clean
  • check:svelte: 0 errors

The two nits (uploading in VolumePage and doneTimer in ActionsBlock) are non-blocking and not addressed here. The re-request for review on this PR was a mistake on my side, so I'm merging on the existing approval.

Ran the full suite on this branch merged with current `main` (`323e746`), as the approval asked. The merge was clean. - `deno task test`: 1051 passed, 0 failed, 1 ignored - `deno task check`, `deno lint` and `deno fmt --check`: clean - `check:svelte`: 0 errors The two nits (`uploading` in `VolumePage` and `doneTimer` in `ActionsBlock`) are non-blocking and not addressed here. The re-request for review on this PR was a mistake on my side, so I'm merging on the existing approval.
thisilike deleted branch fix/double-submit 2026-10-19 10:38:50 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
julian
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
OpsDeck/core!73
No description provided.