Skip to content

fix(dashboard): clean up upload temp file on client disconnect + pin python-multipart (NS-501) - #48259

Merged
kshitijk4poor merged 1 commit into
mainfrom
fix/ns501-multipart-upload-salvage
Jun 19, 2026
Merged

fix(dashboard): clean up upload temp file on client disconnect + pin python-multipart (NS-501)#48259
kshitijk4poor merged 1 commit into
mainfrom
fix/ns501-multipart-upload-salvage

Conversation

@kshitijk4poor

@kshitijk4poor kshitijk4poor commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #47663 (NS-501)

#47663 (streaming multipart upload for the dashboard file manager) merged as c66163453. This is a tight follow-up fixing two issues that landed with it. No feature/frontend changes — only the two fixes below + a regression test.

# Issue on main Fix
1 Temp file leaks on client disconnect. upload_managed_file_stream's except chain catches only HTTPException / PermissionError / OSError — all Exception subclasses. asyncio.CancelledError (raised when a browser aborts a large upload mid-stream — the exact NS-501 scenario) is a BaseException, so it bypasses every except and reaches a finally that only closes the file handle and never unlinks the temp file. Every aborted large upload orphans a partial .{name}.*.upload file (up to ~100 MB) in the target directory. Cleanup moved into finally, keyed on a renamed success flag, so the temp file is removed on every non-success exit including BaseException. (os.replace clears tmp_path on success, so we only unlink when the rename didn't happen.)
2 python-multipart needlessly downgraded. The package was already resolved at 0.0.27 transitively (via daytona) before #47663; the explicit ==0.0.20 pin in the [web] extra + tool.dashboard lazy-install set downgraded it. Bumped both pins to ==0.0.27 and regenerated via uv lock (Updated python-multipart v0.0.20 -> v0.0.27). Base dep stays >=0.0.9,<1. uv lock --check passes. (0.0.20 was past CVE-2024-53981 too, so this is lockfile hygiene, not a security fix.)

Validation

  • New test test_stream_upload_cleans_temp_on_cancellationmutation-tested against current main: it fails on the merged code (leaks .aborted.bin.*.upload) and passes with the fix. It invokes the endpoint coroutine directly so the BaseException propagates instead of being swallowed by the test client.
  • 77 tests green across the file-server + lazy-deps suites; ruff check clean.
  • uv lock --check passes; python-multipart resolves to 0.0.27 (no downgrade).

Diff is scoped to 5 files (+67/-9): hermes_cli/web_server.py, pyproject.toml, tools/lazy_deps.py, uv.lock, tests/hermes_cli/test_web_server_files.py.

Reviewer notes

Touches pyproject.toml / tools/lazy_deps.py / hermes_cli/web_server.py (a dependency pin + dashboard API surface), so this is out of the pure-Docker lane → flagging for Teknium review.

Follow-up to #47663. Surfaced during review of #47663.

@kshitijk4poor
kshitijk4poor requested a review from a team June 18, 2026 05:53
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: fix/ns501-multipart-upload-salvage vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 10997 on HEAD, 10997 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5763 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related: #47663 (original by @benbarclay, this is the cherry-pick onto current main with authorship preserved via Co-authored-by), #43772 (sibling dashboard multipart-upload work). Salvage/cherry-pick — authoritative current version, not a duplicate.

…python-multipart (NS-501)

Follow-up to #47663 (streaming multipart upload), fixing two issues that
landed with it.

1. Temp file leaked on client disconnect. The streaming upload endpoint's
   except chain caught only HTTPException / PermissionError / OSError — all
   Exception subclasses. asyncio.CancelledError, raised when a browser aborts
   a large upload mid-stream (the exact NS-501 scenario), is a BaseException,
   so it bypassed every except clause and reached a finally that only closed
   the file handle and never unlinked the temp file. Every aborted large
   upload orphaned a partial `.{name}.*.upload` file (up to ~100 MB) in the
   target directory. Cleanup now lives in finally, keyed on a `renamed`
   success flag, so the temp file is removed on every non-success exit
   including BaseException paths. Added test_stream_upload_cleans_temp_on_cancellation,
   which fails on the pre-fix code (leaks the temp file) and passes with the fix.

2. python-multipart pinned to ==0.0.27 instead of ==0.0.20. The package was
   already resolved at 0.0.27 transitively (via daytona) before #47663; the
   explicit ==0.0.20 pin in the [web] extra and the tool.dashboard lazy-install
   set downgraded it. Bumped both to ==0.0.27 and regenerated with `uv lock`,
   keeping the lockfile coherent. The base dependency stays >=0.0.9,<1.
@kshitijk4poor
kshitijk4poor force-pushed the fix/ns501-multipart-upload-salvage branch from edd68fd to 6752da9 Compare June 18, 2026 06:02
@kshitijk4poor kshitijk4poor changed the title fix(dashboard): stream file uploads via multipart instead of base64 JSON (NS-501) fix(dashboard): clean up upload temp file on client disconnect + pin python-multipart (NS-501) Jun 18, 2026
@kshitijk4poor
kshitijk4poor enabled auto-merge June 18, 2026 06:07
@kshitijk4poor
kshitijk4poor merged commit 6278bca into main Jun 19, 2026
35 checks passed
@kshitijk4poor
kshitijk4poor deleted the fix/ns501-multipart-upload-salvage branch June 19, 2026 06:34
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…tipart-upload-salvage

fix(dashboard): clean up upload temp file on client disconnect + pin python-multipart (NS-501)
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…tipart-upload-salvage

fix(dashboard): clean up upload temp file on client disconnect + pin python-multipart (NS-501)
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…tipart-upload-salvage

fix(dashboard): clean up upload temp file on client disconnect + pin python-multipart (NS-501)
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…tipart-upload-salvage

fix(dashboard): clean up upload temp file on client disconnect + pin python-multipart (NS-501)
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…tipart-upload-salvage

fix(dashboard): clean up upload temp file on client disconnect + pin python-multipart (NS-501)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants