fix(web): FileManager cancel actually aborts the in-flight transfer (#2396) - #2404
Merged
Conversation
…2396) Cancel on a FileManager transfer previously only removed the row from local React state — the in-flight browser fetch kept running to completion, so the UI claimed a cancel that never happened. - Wire an AbortController per in-flight transfer (upload + download); Cancel aborts the browser-side request. The upload watchdog timeout shares the same controller. - Add a distinct 'cancelled' transfer status (muted label, dismiss affordance) instead of silently dropping the row; an intentional cancel no longer renders as a generic failure. - Honest UX copy: an upload cancelled after the write command was dispatched notes the device may still save the file — there is no device-side cancellation on the single-shot file_read/file_write path (en + pt-BR). - Tests: cancel aborts the request signal, cancelled state renders, AbortError is not shown as a failure (download + upload). Client-side scope only (option 1 from the issue investigation); the dead /remote/transfers chunked path is untouched pending the deprecate-vs-revive decision. Refs #2396 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying breeze with
|
| Latest commit: |
c3de22a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2dbc555f.breeze-9te.pages.dev |
| Branch Preview URL: | https://fix-2396-filemanager-cancel.breeze-9te.pages.dev |
…ace guards (#2396) - Map a non-user-cancel AbortError on upload (the 120s watchdog) to 'unverified' with explicit timeout copy instead of the browser's misleading "The user aborted a request." boilerplate (en + pt-BR). - Match AbortError by name, not instanceof Error — DOMException is not an Error subclass in every runtime. - Log real failures that race a user cancel so the diagnostic isn't swallowed by the cancelled-row suppression. - Guard cancelTransfer against stale clicks: never relabel a terminal row as cancelled, and only record a cancel while a live controller exists (no ref leak). - Test pinning that the watchdog timeout renders as unverified/timeout, not as "Cancelled" (fake timers). Refs #2396 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Review run: /pr-review-toolkit:review-pr (code-reviewer, silent-failure-hunter, pr-test-analyzer) Findings: 4 raised → all addressed in c3de22a; 0 outstanding.
Tests: Status: review-clean, awaiting maintainer merge. |
ToddHebebrand
added a commit
that referenced
this pull request
Jul 13, 2026
…2413) Closes #2396 Executes **option 2** from the investigation on #2396: remove the dead chunked file-transfer subsystem end-to-end. The path has **never fired in production** — no server code has ever dispatched `file_transfer` / `cancel_transfer` commands to the agent, and nothing has called `POST /remote/transfers` since the web FileManager moved to the system-tools path (9379f17, Feb 2026); even before that, the create route only inserted DB rows. The live transfer mechanism (single-shot `file_read`/`file_write` via `systemTools/fileBrowser.ts`, plus PR #2404's client-side abort) is untouched. ## Removed **API** - `routes/remote/transfers.ts` (all `/remote/transfers*` routes) + `transfers.test.ts` - `routes/remote/internal.ts` (its only route was the agent-facing transfer-progress PATCH) and both mounts in `routes/remote/index.ts` - `workers/transferCleanup.ts` + its init/stop wiring in `index.ts` - `services/fileStorage.ts` (chunk save/assemble/stream helpers — no remaining consumers) - `createTransferSchema` / `listTransfersSchema` in `routes/remote/schemas.ts` - `getTransferWithOrgCheck` + `MAX_ACTIVE_TRANSFERS_PER_*` in `routes/remote/helpers.ts`; `hasSessionOrTransferOwnership` renamed to `hasSessionOwnership` (session paths keep using it) - openapi: `FileTransfer` component schema + `/remote/transfers*` paths - `EVENT_TYPES.REMOTE_FILE_TRANSFERRED` (`remote.file.transferred` — never emitted) and the transfer chunk-upload carve-out in `middleware/bodyLimit.ts` - Stale "use file transfer for larger files" copy in `systemTools/schemas.ts`, `aiToolsFilesystem.ts`, and `bodyLimit.ts` — that alternative no longer exists (and never worked) **Agent** - `agent/internal/filetransfer/` package (incl. its tests; PR #2391's fix is deleted with it — expected) - `handleFileTransfer` / `handleCancelTransfer` + registry entries in `heartbeat/handlers_desktop.go` - `CmdFileTransfer` / `CmdCancelTransfer` in `remote/tools/types.go` - `fileTransferMgr` field, `ftConfig` plumbing, and failover `SetServerURL` propagation in `heartbeat/heartbeat.go` (token init kept for the helper manager, renamed `ftToken` → `secToken`) **DB** - Migration `2026-07-12-drop-file-transfers.sql`: idempotent `DROP TABLE IF EXISTS file_transfers` + drops the two orphaned enum types. Per the forensic-trail rule it logs `RAISE WARNING 'dropping file_transfers with % rows'` before the drop. **The drop is destructive, but the data is dead**: rows (if any) are inert metadata from the never-functional feature — nothing could ever read or act on them. - `fileTransfers` table + `file_transfer_direction`/`file_transfer_status` enums removed from `db/schema/remote.ts` **Registration lists** (`file_transfers` removed from all three) - `DEVICE_ID_JOIN_POLICY_TABLES` in `rls-coverage.integration.test.ts` - device cascade-delete list in `routes/devices/core.ts` + `INTENTIONALLY_NO_ORG_ID` in `moveOrg.coverage.test.ts` - site-scope exempt lists in `site-scope-coverage.integration.test.ts` **Config/docs** - `TRANSFER_STORAGE_PATH` / `MAX_TRANSFER_SIZE_MB` / `MAX_ACTIVE_TRANSFERS_PER_*` removed from `.env.example`, `docker-compose.yml`, `deploy/docker-compose.prod.yml`, and the env docs (droplets may keep a stray empty `data/transfers` dir; harmless) - Docs: transfer API sections removed from `features/remote-access.mdx`, `reference/api.mdx`, `agents/commands.mdx`, `features/webhooks.mdx`, `docs/architecture.md` - `docsIndex.json` regenerated via `scripts/build-docs-index.ts` — note the checked-in index was stale, so the refresh also picks up docs pages added since it was last built (generated file) ## Intentionally untouched The `remote_session_type` enum value `'file_transfer'` and everything typed on it (`remote_sessions`, `SessionHistory.tsx`, `aiToolsRemote.ts`, `REMOTE_SESSION_TYPES`) — remote *sessions* typed `file_transfer` are a separate, live concept, and PG enum values can't be dropped safely anyway. `systemTools/fileBrowser.ts` and the FileManager UI are also untouched. ## Verification - Migration applied to a fresh scratch DB: table + enum types gone, ledger row recorded, `db:check-drift` green (396 files match); the forensic `WARNING: dropping file_transfers with 0 rows` fired - RLS coverage contract test run against a real Postgres **with the table actually dropped**: 53 passed - Site-scope coverage contract: 7 passed; moveOrg coverage + remote/sessions/bodyLimit/systemTools suites: green - `cd agent && go test -race ./...` green; `GOOS=windows` and `GOOS=linux` builds green - `tsc --noEmit` clean for api, web, shared; `@breeze/shared` tests 1078 passed 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Refs #2396— implements option 1 (client-side abort, the immediate honest fix) from the investigation comment on the issue. Options 2 (deprecate the dead/remote/transferschunked path) and 3 (revive it as the FileManager backend) remain open for a maintainer decision, so this PR intentionally does not close the issue.Previously
cancelTransferinFileManager.tsxonly didsetTransfers(prev => prev.filter(...))— it removed the row from React state while the in-flight browser fetch kept running to completion. The UI claimed a cancel that never happened.Changes
.abort()on the transfer's controller; the existing 2-minute upload watchdog now shares the same controller.cancelledstatus onTransferItem— a muted "Cancelled" label plus the dismiss affordance, instead of silently dropping the row. The catch paths in the upload/download flows recognize a user cancel (via a cancelled-ids ref) and skip the generic failure state, so an intentional abort never renders as "Failed to download/upload". The watchdog-timeout abort still surfaces as a failure.file_read/file_writepath. An upload cancelled after the write command was dispatched shows a note that the file may still be saved on the device. Strings added in bothenandpt-BRlocales./remote/transfers*,fileTransfers, oragent/internal/filetransfer/(which open PR fix(agent): remove filetransfer map entries when HandleTransfer returns (#2388) #2391 is editing).Verification
pnpm exec vitest run src/components/remote/FileManager.test.tsx— 3 passed (new: cancel aborts the request signal + renders cancelled state for download; cancelled upload shows the device-side caveat and no failure toast)pnpm exec vitest run src/lib/i18n/localeParity.test.ts src/lib/i18n/keyUsage.test.ts— 7 passednpx tsc --noEmit— clean🤖 Generated with Claude Code