feat: gate the implicit image pull behind --pull - #613
Merged
Merged
Conversation
`print_upload_logs` acted on one error string, "denied: Your authorization token has expired.". Every other error in the stream - a failed layer upload, a denied push, ECR throttling, a network reset - fell through and was recorded as progress. The caller then recorded the image and printed a green success message for an image that never reached the registry. Any error now ends the upload with exit 1 and prints the error verbatim. The same parser reads the pull stream, so a failed pull is no longer silent either. Move the cursor below the progress block before printing, so the error does not overwrite a progress line, and raise `click.exceptions.Exit` rather than calling `sys.exit` so click can clean up.
Add `DockerPushLogItem` beside `DockerBuildLogItem` so the push and pull streams are typed rather than `dict[str, Any]`, and widen the progress map key to `Optional[str]` to match what the stream actually carries. Cover the pull path, which shares the parser with the header suppressed.
Address msto review: the header-suppressed path asserted the header is absent, but nothing covered that it appears by default. Add a positive assertion so the print_header=True path is exercised. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`record_in_db` ran unconditionally after the push and issued a plain `createPrivateImage` create. A failure there exited 1, the same code as a failed push, so a caller could not tell "the image is not in the registry" from "the image is in the registry but Latch does not know about it". Retrying on that exit code can never succeed, because the tag is immutable. - `is_recorded_in_db` queries for one image and version. - `record_in_db` skips the mutation when the record is already present, so a retry of a partly completed upload no longer hits the uniqueness constraint. - `record_in_db_or_exit` reports a record failure as distinct from a push failure and exits 3, and both upload paths now use it. `latch image upload` documents the three exit codes.
`is_recorded_in_db` returned the truthiness of the node list rather than checking that a returned node was the image in question. The filter is only correct if the server honours all three fields; one that ignored an unrecognised filter field would report every image in the workspace as a match, `record_in_db` would skip the create, and the upload would exit 0 with the image published and no record of it - the one silent failure the exit codes exist to prevent. Verified against the live API that all three fields are honoured, so this is not reachable today. Matching client-side makes the check independent of filter semantics at no cost. Correct the recovery advice: re-running `upload` pushes again before it records, so it is not a retry of the record. State what is broken instead. Replace the filter test, which passed against a query that filtered on the workspace alone, with tests that feed a non-matching node.
Address msto review: the PrivateImageExists query reused PrivateImageNode, dropping creationTime and adding workspaceId. That broke ls()'s typing, which still reads node["creationTime"]. Split out PrivateImageExistsNode/PrivateImageExistsResult so each query has a type matching the fields it selects. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address nh13 review: is_recorded_in_db compared the BigInt workspaceId against a str workspace id. Postgraphile serializes BigInt as a string today, but if it ever returns a number the match silently fails, the create runs anyway, and the uniqueness constraint this guard prevents fires. Normalize with str(), and cover the click.Abort passthrough arm that was previously untested. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`latch register` takes `--workspace-id`, but `latch image upload` and `latch image ls` resolved the workspace from ambient config. A CI job could pin the workspace for registration but not for image publishing, and the two could disagree silently: the upload went to one workspace, and the `ls` used to confirm it read that same wrong workspace, so it looked like it worked. Add `--workspace-id` to both commands and thread the resolved value through all four consumers - the namespaced repository name, the credentials, the push target, and the database record. `get_credentials` resolved the workspace itself, so it now takes `ws_id` as a required keyword argument. `dbnp` and `remote_dbnp` carry it through, which covers the local and remote build paths. `resolve_workspace` checks an explicit id against the workspaces the user can reach and names the target, so a typo fails before the push rather than publishing into a namespace they did not intend.
- Rename `resolve_workspace` to `resolve_workspace_id`: it returns an id. - Share one `--workspace-id` option between the two commands. - Drive `upload_image` end to end in a test, rather than only the pieces. - Replace the `type: ignore[arg-type]` stub argument with a real instance. Validate only an explicit id, not the active workspace. `latch register` checks both, but `get_workspaces` measures ~230ms against ~120ms for the `ls` query it would precede, which is a poor trade for a value that comes from the user's own config. Recorded as a deliberate difference in the docstring and its test.
Address msto review: register_staging binds ws_id once so the repo name, build credentials, and DB record all agree on one workspace, but the record mutation still re-called current_workspace(). Use the bound ws_id so the record honors the single-binding intent the comment documents. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address nh13 review: the single-workspace binding sat below the LatestVersion duplicate-check query and its error message, which still re-read current_workspace(). Hoist the bind above the check so all four consumers - the query, the already-exists message, the repository name, and the record - share one value, making the invariant the comment documents fully true. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`upload_image` pulled any reference missing from the local daemon and pushed it
to the workspace registry. An unqualified reference resolves to Docker Hub, so a
build that silently failed to produce the tag would publish whatever a third
party had parked at that name, under our tag, in our workspace.
A missing image now fails before the confirmation prompt and before credentials
are minted, and names the reference it would have pulled:
No local image matches `team/tool:v1`.
That reference is unqualified, so Docker resolves it to
`docker.io/team/tool:v1` on Docker Hub. Check that you own that namespace.
Build the image first, or pass `--pull` to fetch `docker.io/team/tool:v1`.
The Docker Hub warning is omitted when the reference is already qualified, so it
does not cry wolf on the case it does not apply to.
`resolve_pull_reference` applies Docker's rule that the first path component is a
registry when it contains a `.` or `:`, is `localhost`, or is not all lowercase -
a repository path cannot contain uppercase, so such a component can only be a
host. `--pull` pulls that resolved reference rather than the original, so the
message cannot drift from the call it describes.
Address nh13 review: the parametrized table exercised a dot-host, localhost, uppercase host, digests, and official images, but no first component marked a host by a port alone. Add localhost:5000/tool:v1 so the ":" in head branch is covered. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ameynert
marked this pull request as ready for review
August 17, 2026 22:02
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
2 participants
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.
latch image uploadno longer pulls a missing local image implicitly. An unqualified reference resolves to Docker Hub, so the old behavior could publish a third party's image under your own tag. Pass--pullto fetch a missing image.Changes
--pullto fetch an image that is absent locally; otherwise exit 1.Tests: the reference-resolution table, including the port-host case.