Skip to content

Provide explicit Cancel for Publish>Apps and make it modal while running (BL-16350) - #8133

Draft
StephenMcConnel wants to merge 4 commits into
masterfrom
BL-16350-ProvideCancelForPublishApps
Draft

Provide explicit Cancel for Publish>Apps and make it modal while running (BL-16350)#8133
StephenMcConnel wants to merge 4 commits into
masterfrom
BL-16350-ProvideCancelForPublishApps

Conversation

@StephenMcConnel

@StephenMcConnel StephenMcConnel commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

While a Reading App Builder prepare/build/try-on-phone action is running (its Cancel button showing), the Publish → Apps operation is now modal: the user's only option is the explicit Cancel button, and Bloom prevents navigating away rather than auto-cancelling. This implements the design agreed on the card (the earlier auto-cancel-on-navigate approach, PR #8119, was discarded).

Modal navigation

  • Workspace tabs (Collections / Edit / Publish) are disabled from C# for the duration of an action — RabPublishApi injects PublishView and calls WorkspaceView.SetTabsEnabled(false/true) around each action (same mechanism LibraryPublishApi uses during an upload).
  • Publish-tool switcher (BloomPUB / ePUB / Apps / …) is blocked in React — PublishTabPane vetoes onSelect and disables the other tools while the Apps screen reports busy via a new onBusyChange callback.

Responsive cancellation

  • Check for cancellation between building each BloomPUB in the export loop, so a Cancel during a multi-book export stops after the current book instead of grinding through all of them.
  • Thread a CancellationToken through the RAB installer download HTTP calls so a stalled download aborts promptly instead of waiting for the 30-minute HttpClient timeout.

No debris left behind on interrupt

  • Download: writes to a temp .part file and moves it into place only on success; a cancelled/failed download leaves no partial installer at the real name (which FindRabSetupInstallerPath would otherwise find and try to run).
  • Build: a cancelled/failed build deletes intermediate .apk files under BuildRoot so a partial/unsigned APK can't be mistaken for a finished app by FindLatestApkPath; the previously built signed APK in SafeApkRoot is preserved.
  • Install: the update-incompatible recovery (uninstall + reinstall) is made atomic via a scoped _protectCurrentProcessFromCancellation flag, so a cancel can't leave the phone with the old app removed and nothing reinstalled.

Tests

New unit tests cover cancel-before-retry and cancel-during-replace in install, failed-build APK cleanup, and the atomic-download cancel/success cases.

Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16350

Devin review


This change is Reviewable

StephenMcConnel and others added 2 commits July 30, 2026 15:22
…ing (BL-16350)

While a Reading App Builder prepare/build/try-on-phone action is running (its
Cancel button showing), the Apps operation is now modal: the user's only option
is the explicit Cancel button, and Bloom prevents navigating away rather than
auto-cancelling.

- Modal navigation: disable the main workspace tabs from C# for the duration of
  an action (RabPublishApi injects PublishView -> WorkspaceView.SetTabsEnabled),
  and block the publish-tool switcher in React (PublishTabPane vetoes onSelect
  and disables the other tools while the Apps screen reports busy via
  onBusyChange).
- Responsive cancellation: check for cancellation between building each BloomPUB
  in the export loop, and thread a CancellationToken through the RAB installer
  HTTP download so a stalled download aborts promptly instead of waiting for the
  timeout.
- No debris on interrupt:
  - Installer download writes to a temp .part file and is moved into place only
    on success; a cancelled/failed download leaves no partial installer at the
    real name (which FindRabSetupInstallerPath would otherwise try to run).
  - A cancelled/failed build deletes intermediate .apk files under BuildRoot so
    a partial/unsigned APK cannot be mistaken for a finished app; the previously
    built signed APK in SafeApkRoot is preserved.
  - The update-incompatible install recovery (uninstall + reinstall) is made
    atomic so a cancel cannot leave the phone with no app installed.
- Tests covering the new cancellation/cleanup paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
StephenMcConnel and others added 2 commits July 30, 2026 15:35
…BL-16350)

Devin review of PR #8133 caught that cancellation was only checked after a
subprocess exited, never before launching one. So a Cancel clicked during the
in-process work between two RAB runs (e.g. in Build() between the project-update
run and the Gradle build) let the next multi-minute build launch and run to
completion before the cancel was noticed -- and with navigation now locked, the
user was stuck watching it finish.

RunProcess/RunProcessCapturingOutput now throw OperationCanceledException before
starting a process when a cancel is already pending, and also kill the process if
a cancel landed in the narrow window between Start() and registering it as the
current process. Both checks are skipped during the protected uninstall+reinstall
recovery, which must still run to completion.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…user cancel (BL-16350)

Devin review of PR #8133 caught that the prepare/build/install handlers catch
OperationCanceledException and report it as a user cancellation. But an HttpClient
timeout during the RAB installer download throws TaskCanceledException, which also
derives from OperationCanceledException -- so a stalled download that timed out
was shown to the user as cancelled and never logged as the failure it is.

Gate the catch with `when (_rabProjectService.IsCancellationRequested)`. Every
user-initiated OperationCanceledException in this code is raised only while
_cancelRequested is set, so a timeout (with _cancelRequested false) now falls
through to the failure handler, which logs and surfaces the real error.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread src/BloomExe/Publish/Rab/RabPublishApi.cs
Comment thread src/BloomExe/Publish/Rab/RabProjectService.cs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Devin] Bug: Cancel does nothing while the Reading App Builder installer is running, and the user can't leave the screen

On first run, if RAB isn't installed, Prepare downloads and runs the installer. InstallRabFromSetup runs the setup via StartShellProcess + WaitForExit and does not register that process for cancellation, so RequestCancellation has nothing to kill and no cancel check runs until the next step. Because this change also disables the workspace tabs and the publish-tool switcher for the whole Prepare, a user who cancels during the install is left with an unresponsive Cancel and no way to navigate away until the installer finishes. (src/BloomExe/Publish/Rab/RabProjectService.cs — InstallRabFromSetup; lock at RabPublishApi.cs)

Sent to the developer as a decision (see the preflight report): what should Cancel do during a third-party installer run — kill it (a killed install is detected as not-installed and re-run), or leave the tabs unlocked during the install/download prerequisite phase?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Devin] Bug: Busy state is pushed to the parent screen from a useEffect, which the repo's React guidance lists as a "don't"

The Apps screen notifies its host that an action is running from inside a React.useEffect watching screenState.busyAction. src/BloomBrowserUI/AGENTS.md.github/skills/react-useeffect/SKILL.md lists "Notify parent of changes" as a DON'T for effects (DO: call in an event handler). (src/BloomBrowserUI/publish/Apps/AppPublisherScreen.tsx)

Sent to the developer as a decision (see the preflight report): busyAction here is derived from several asynchronous sources (the websocket action-complete event, status-poll recovery, tab-activation restore), not a single user event, so an effect that mirrors it up is arguably the right tool — but the fuller refactor (pass the callback into the hook, notify at each state change, keep an unmount-cleanup) is available if strict compliance is preferred.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Devin] Investigate: A cancelled build could leave a truncated APK at the deliverable name in SafeApkRoot

The interrupted-build cleanup deletes intermediate .apk files under BuildRoot, but deliberately does not touch SafeApkRoot (to preserve the previous good signed APK on a failed rebuild). RAB writes its finished APK to SafeApkRoot under a name derived from the project — the same name each build. If RAB/Gradle is killed while writing/signing into SafeApkRoot, a truncated file could be left at the real APK name, overwriting the previous good APK and then picked up by FindLatestApkPath; if the build signature didn't change, "Try on phone" would still be enabled for it. (src/BloomExe/Publish/Rab/RabProjectService.cs)

Whether this is reachable depends on whether RAB writes its output atomically (temp + move). Sent to the developer (see the preflight report) since it needs knowledge of RAB's output behavior.

@StephenMcConnel

Copy link
Copy Markdown
Contributor Author

[Claude Opus 4.8 (1M context)] Consulted Devin on 2026-07-30 16:04 MDT up to commit bf4a6e0c7.

  • 2 bugs fixed this run (download-timeout misreported as cancel; cancel ignored before launching the next subprocess) — threads replied to and resolved.
  • 2 bugs + 1 investigate flag sent to the developer as decisions (Cancel unresponsive/locked-out during the RAB installer run; busy-state useEffect vs. the repo's React convention; possible truncated APK left at the SafeApkRoot deliverable name) — threads left open; details in the preflight report.
  • 14 informational items not posted (low signal).

Other reviewers: CI green (pr-automation, track/set-waiting); CodeRabbit not run (.coderabbit.yml has auto_review.enabled: false).

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