Skip to content

fix(model): --wait-for-hash must wait for a deployable version (STO-441) - #337

Merged
jebenexer merged 1 commit into
mainfrom
benjaminbrannaka/sto-441-runpodctl-wait-for-hash-must-wait-for-a-deployable-model
Sep 9, 2026
Merged

jebenexer merged 1 commit into
mainfrom
benjaminbrannaka/sto-441-runpodctl-wait-for-hash-must-wait-for-a-deployable-model

Conversation

@jebenexer

@jebenexer jebenexer commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

STO-441: runpodctl --wait-for-hash must wait for a deployable model version

https://linear.app/runpod/issue/STO-441

  • --wait-for-hash returned as soon as the version had any hash. the api assigns a ph-<32 hex> placeholder the moment the version exists, while its status is still NEEDS_HASH, and myModels (what the cli polls) does not filter those out — so the wait ended immediately and printed a model url that the endpoint validator rejects with "Model Repo reference is not ready for endpoint deployment".
  • --delete-my-model-files-after-upload deleted the local files on that false confirmation. it now only fires once the version is genuinely deployable.
  • readiness is now status in {POD_READY, READY} plus a non-ph- hash — the exact set the api's READY_MODEL_REPO_STATUSES gate accepts.
  • a version that failed server-side (FAILED/POD_REMOVED/DEPRECATED with no canonical pointer) now ends the wait immediately with code conflict, instead of burning the whole --hash-timeout budget and reporting timeout (which tells an agent the work is still running and to poll rather than re-upload).
  • a content-deduped upload now succeeds instead of hanging: when the hashed bytes already exist, the api deprecates this uuid, leaves its placeholder hash in place forever, and names the version holding the content in metadata.dedupedToCanonicalVersion. the wait follows that pointer and reports the canonical hash.
  • the timeout message now names the last status seen and the command to poll with; the progress line reads waiting for model to be deployable.
  • docs: regenerated docs/runpodctl_model_add.md, README conflict row, and an AGENTS.md pitfall for the placeholder-hash and dedupe-deprecation behavior (neither is discoverable from the cli code alone).
  • bumped golang.org/x/crypto to v0.56.0 to clear the govulncheck CI failure (GO-2026-6354 / GO-2026-6355, an ssh.Dial deadlock DoS reachable from cmd/project/ssh.go).

Test plan:

  • go test ./... (cmd/model coverage 74.2%), go vet ./...
  • govulncheck ./... — 0 vulnerabilities in called code (was 2, fixed by the x/crypto bump above)
  • go test -tags e2e ./e2e
  • live e2e — done, against a local runpod-in-a-box stack (real GraphQL server built from actual RunPod source, real MySQL, real LocalStack S3, no cloud credentials needed). Forced a freshly-uploaded version to a terminal POD_REMOVED status while its hash was still the ph--prefixed placeholder: the pre-fix binary printed model is ready to deploy with the placeholder-hash url (the exact bug), the fixed binary correctly failed fast with {"error":"...is pod_removed, so it will never become deployable...","code":"conflict"}. Happy path (real async hashing via the local model-repo-hasher poller) succeeds identically on both binaries with the real non-placeholder hash — no regression.
    • Note: live testing also found that node/graphql/schema/model.ts's shared Model.versions filter (visibleModelVersionsWhere) hides NEEDS_HASH, FAILED, and DEPRECATED versions from myModels on main today (confirmed byte-identical resolver code). POD_REMOVED is not filtered and was confirmed reachable live; FAILED and the dedupe-follow path are correct here but currently unreachable through myModels as it exists on main — not a regression (the pre-fix code has the same blind spot), but worth a follow-up if fast-detection for those two paths is wanted.

the two pre-existing wait tests now start from the real bug state (ph-… + NEEDS_HASH) rather than an empty hash, so they fail against the old code. new coverage: the 9-case readiness table, terminal fast-exit (asserts conflict and that it never polls again), dedupe→canonical resolution, timeout message content, and "local files survive when only a placeholder hash ever appears".


Note

Medium Risk
Changes model-upload wait and post-upload local deletion gates; wrong logic could delete files early or mislead agents on errors, though behavior is heavily unit-tested.

Overview
Fixes model add --wait-for-hash so it no longer treats “any nonempty hash” as success. The API assigns a ph- placeholder while status is NEEDS_HASH, which previously ended the wait immediately and could print a deploy URL the endpoint validator rejects; --delete-my-model-files-after-upload could delete locals on that false positive.

Readiness is now POD_READY or READY plus a non-placeholder hash, matching deploy-time rules. Deduped uploads follow metadata.dedupedToCanonicalVersion and succeed with the canonical hash even when the new UUID stays DEPRECATED. Terminal statuses (FAILED, POD_REMOVED, bare DEPRECATED) fail fast with code conflict instead of burning --hash-timeout and reporting timeout. Timeout messages name the last status and runpodctl model list --name …; stderr progress says “waiting for model to be deployable”.

Docs (README, AGENTS.md, generated model add help) and api.ModelVersionStatus constants were extended accordingly. Tests cover placeholder polling, terminal exit, dedupe resolution, and timeout copy. golang.org/x/crypto (and related x/ modules) were bumped for govulncheck CI.

Reviewed by Cursor Bugbot for commit 7d8962d. Configure here.

The api assigns a `ph-`-prefixed placeholder hash the moment a model
version exists, while its status is still NEEDS_HASH, and myModels does
not filter those out. The wait treated any nonempty hash as ready, so it
returned immediately with a model url no endpoint would accept -- and
--delete-my-model-files-after-upload deleted the local files on that
false confirmation.

Readiness is now status POD_READY/READY plus a non-placeholder hash, the
same set the api's endpoint validator accepts. A version that failed
server-side ends the wait with `conflict` instead of burning the whole
--hash-timeout budget, and a content-deduped upload resolves through
metadata.dedupedToCanonicalVersion to the canonical hash rather than
waiting forever on a uuid the api deprecated.

Also bumps golang.org/x/crypto to v0.56.0 to clear GO-2026-6354 /
GO-2026-6355 (ssh.Dial deadlock DoS) flagged by govulncheck in CI.
@jebenexer
jebenexer force-pushed the benjaminbrannaka/sto-441-runpodctl-wait-for-hash-must-wait-for-a-deployable-model branch from e1b7ec2 to 7d8962d Compare September 8, 2026 18:42
@rutvik-runpod

Copy link
Copy Markdown

bugbot run

@jebenexer

Copy link
Copy Markdown
Collaborator Author

@cursor review

@rutvik-runpod

Copy link
Copy Markdown

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7d8962d. Configure here.

@jebenexer
jebenexer merged commit 3560e35 into main Sep 9, 2026
2 checks passed
@jebenexer
jebenexer deleted the benjaminbrannaka/sto-441-runpodctl-wait-for-hash-must-wait-for-a-deployable-model branch September 9, 2026 18:35
@promptless

promptless Bot commented Sep 9, 2026

Copy link
Copy Markdown

Promptless documentation updates

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