Skip to content

fix(models): acknowledge and cancel existing artifact verification - #7262

Closed
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/model-download-cancellable-verification
Closed

tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/model-download-cancellable-verification

Conversation

@tang-vu

@tang-vu tang-vu commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Move verification of existing GGUF artifacts into the owned, cancellable model-download worker. Accepted operations return started, publish verifying and settle as complete, cancelled or failed. Valid existing files are reused without network transfer; cancellation and hash-provider failure preserve them.

Why this matters

After a host-agent restart its in-memory integrity proof is gone. Clicking Download for an existing large GGUF synchronously hashes it before the host responds or enables Cancel. The dashboard download deadline is 15 seconds and its host transport allows 30 seconds; ordinary large-file verification can outlast both. The request appears failed while lifecycle ownership remains busy and Cancel reports no download. This change acknowledges and exposes verification through the existing polling/cancellation contract.

AI Assistance

Codex assisted investigation, implementation, tests and PR preparation. A separate Codex agent reviewed exact candidate cc8bb702e08d70dcfcb89cc8a04489d1b2666307, traced the actual UI contract, and independently passed 2 HTTP/FastAPI tests, 24 host regressions and 16 Vitest tests. Human review remains required before merging.

Release Lane

  • Mainline change targeting main

Changed Surface

  • Dashboard API / host agent

Host download control plus tests and API documentation. Dashboard production code is unchanged.

Risk And Validation

  • Risk level: Medium — model artifact lifecycle and cancellation.
  • git diff --check
  • Focused tests listed below

Candidate cc8bb702e08d70dcfcb89cc8a04489d1b2666307, upstream base 2234855567d898e040fadb035675e35223210a43, native Windows/Python 3.14 and Node/Vitest 4.1.7.

  • Unchanged-main regression: 2 failed because the public API start request cannot return while SHA256 runs.
  • Candidate: public authenticated FastAPI POST/status/cancel routes use actual host HTTP and actual SHA256 with controlled timing. Both cancellation and injected hash-provider failure preserve the existing file, release ownership and permit a successful retry. Duplicate operation returns 409; terminal Cancel returns no_download.
  • Focused API/host tests: 71 passed, including existing corrupt/empty artifact replacement, split reuse and download cancellation cases.
  • Dashboard action/deadline/polling tests: 63 passed, including verification-to-completion rendering.
  • New Python test Ruff, touched UI test ESLint, Python compile and diff checks passed.
# ods/extensions/services/dashboard-api
python -m pytest -q tests/test_host_download_verification_http.py tests/test_models.py tests/test_model_routes.py tests/test_model_recovery_api.py tests/test_host_agent.py -k 'download or Download or artifact or model_lifecycle or model_status'
# ods/extensions/services/dashboard
npm test -- src/hooks/__tests__/useDownloadProgress.test.js src/hooks/__tests__/useModels.test.js src/hooks/__tests__/useModelsOperationDeadline.test.js

Overlap check

Searched open and closed PRs for model verification/hash/download cancel, exact _handle_model_download, preexisting, already_downloaded, and host-agent verification; each query returned fewer than 100 results. Checked the open inventory and recent PR inventory. #7199 changes bootstrap verification performance/status; #7202 changes bootstrap phase display; neither changes this host request/cancellation boundary. #5453 binds multipart progress closure values in the same function; this change preserves those loop/progress semantics and does not replace its fix. #6838 handles split model deletion, and #6863 handles installer partial-file cleanup. HF token #7213 and benchmark #6990 are separate and untouched.

Operational Change Check

  • This is an operational change and validation is intentionally deferred for: full installation, live large-model/GPU, Docker/WSL and fleet checks. Local HTTP establishes acknowledgement, status, ownership, cancellation and recovery; controlled timing is not a performance benchmark.

Notes For Reviewers

The accepted existing-file response intentionally changes from synchronous already_downloaded to started followed by complete. Both actual dashboard API transport/polling and dashboard action/status consumers are covered. Existing checksums, URL policy, private metadata, split-file reuse and worker cleanup remain in place. Independent review found no blocking issues at the candidate SHA; local green results do not establish hosted CI or live model behavior. Cancellation is checked between digest chunks; it cannot interrupt a blocked filesystem read. Upstream advanced to a53f61ad8 before submission with no changes in the five affected files. Revert restores synchronous preexisting-file verification.

Combined integration validation

All five independent candidates (#7262, #7263, #7264, #7266 and #7270) were composed on upstream a53f61ad826e654cedc1cb7cfb0c76f939796775; combined head 19809ab5f9f627264484e6ba61e8932c10159a87.

Native combined results: 887 API tests passed, 12 skipped (Windows symlink privileges and POSIX-only modes/ownership/durability); 92 frontend tests passed across six suites; 19 ownership + 52 uninstall checks passed, plus native runtime recovery. API coverage includes actual host HTTP handlers and actual atomic performance-file publication. Docker outcomes and rendered browser API promises remain simulated where described above. Source candidates match the exact published heads; the registered QA worktree and logs are retained locally. This adds combination evidence, not a full install, live model/Portal or fleet claim. No dependency on merging the other four PRs is introduced.

Archived predecessor (2026-10-06)

Superseded by #7405, which reconstructs this coherent outcome on upstream main c5b5533. This PR remains closed at its original reviewed head cc8bb70 because GitHub REST rejects reopening after a closed branch was reconstructed/force-pushed. The original head and this original validation record are preserved; current-candidate validation is reported in #7405. The contributor and Codex collaborated on project direction, scope, environment setup, implementation, analysis and tests as applicable; maintenance commands were agent-executed. Human/maintainer release review remains separate.

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.

1 participant