Conversation
|
Claude finished @runeb's task in 1m 18s —— View job Code Review
1. Non-standard flag names ( The codebase has shared flag utilities in // upload.ts L41-50 — current
static override flags = {
dataset: Flags.string({...env: 'SANITY_DATASET'}),
project: Flags.string({...env: 'SANITY_PROJECT_ID'}),
}Should use the shared helpers instead: import {getDatasetFlag, getProjectIdFlag} from '../../util/sharedFlags.js'
static override flags = {
...getProjectIdFlag({env: 'SANITY_PROJECT_ID'}),
...getDatasetFlag({env: 'SANITY_DATASET'}),
}Then update line 70 to use 2. Lines 22–26 define 3. No test for multi-file upload The multi-file upload path (the 4. Silent error suppression on // L64
const cliConfig = await this.getCliConfig().catch(() => {})The empty catch silently swallows any error, including ones unrelated to "not in a Sanity project directory" (e.g. malformed config, I/O errors). Consider only suppressing known error types or at least logging to debug output so errors aren't completely invisible. |
Coverage Delta
Comparing 1 changed file against main @ Overall Coverage
|
f6cf019 to
6eeb5c4
Compare
Adds a new `assets` topic with an `upload` command that uploads files to the Sanity CDN and prints public URLs to stdout. - Supports multiple files in a single invocation - Auto-detects content type from file extension - Routes images to /assets/images/ and other files to /assets/files/ - Resolves project ID from --project flag, env, or sanity.config.ts - Resolves dataset from --dataset flag, env, config, or "production" - Auth handled via getProjectCliClient with requireUser validation Includes tests using vitest + nock + @sanity/cli-test utilities. Co-authored-by: freud <freud@miriad.systems>
Use client.getUrl() to construct the assets API URL, which respects SANITY_INTERNAL_ENV for staging environments (api.sanity.work vs api.sanity.io). Previously the host was hardcoded to api.sanity.io. Co-authored-by: freud <freud@miriad.systems>
1. Consolidate getCliConfig — call once, use for both project ID and
dataset resolution. Fixes bug where running outside a Sanity project
dir with --project flag would throw from findProjectRoot().
2. Add file-not-found error handling — wrap readFile in try/catch with
friendly "File not found: <path>" message.
3. Remove unused nock import and cleanup from tests — we mock fetch
globally with vi.stubGlobal, not nock interceptors.
4. Add {exit: 1} to all this.error() calls — matches codebase convention
(without it, oclif defaults to exit code 2).
Co-authored-by: freud <freud@miriad.systems>
The project uses ES2023 lib without DOM types, so Buffer and
Uint8Array don't satisfy the BodyInit type for fetch(). Using
Blob([buffer], {type}) which is in the ES2023 spec and passes
typecheck.
Co-authored-by: freud <freud@miriad.systems>
Major simplification addressing all 4 review findings:
1. Switch to client.assets.upload() — handles auth, host resolution,
and returns SanityAssetDocument with .url. No more raw fetch, no
manual URL construction, no token extraction. Eliminates the
Bearer undefined edge case entirely.
2. Remove required:true from arg — keep manual files.length check for
controlled error message. Fixes dead code (oclif would error before
run() with required:true + strict:false).
3. Fix test mocking — properly mock getProjectCliClient via
vi.mock('@sanity/cli-core') pattern (matches backup/disable.test.ts).
Tests now exercise the actual client.assets.upload() call path.
Added 5th test for file read errors.
4. Preserve original error in catch — "Cannot read file: ENOENT..."
instead of misleading "File not found" for permission errors etc.
Co-authored-by: freud <freud@miriad.systems>
1. Add test for missing URL in upload response (!doc.url branch) 2. Guard against SANITY_PROJECT_ID env var leaking into 'no project ID' test — save/restore pattern ensures test isolation 3. Add explanatory comment for why we use cliConfig?.api?.projectId instead of this.getProjectId() — the latter throws from findProjectRoot() when run outside a project directory
75621d1 to
116df93
Compare
1. Bring back CONTENT_TYPES map and pass contentType explicitly to client.assets.upload(). The client does not infer content type from the filename option when given a Buffer — only browser File objects get auto-detection. Without this, SVGs and other files would be uploaded as application/octet-stream. 2. Remove duplicate 'no URL in response' test that was accidentally added in both the review fixes commit and the Claude findings commit.
Summary
Adds a new
sanity assets uploadcommand that uploads one or more files to the Sanity CDN and prints public URLs to stdout.Motivation
There's no CLI command to upload files to Sanity CDN and get back public URLs. This is useful for embedding images in GitHub PRs, Linear tickets, docs, or any context where you need a hosted URL.
What's included
packages/@sanity/cli/src/commands/assets/upload.tsassetstopic (separate from existingmediatopic which handles media library operations)SanityCommandwithstrict = falsefor variadic file args (same pattern asexec.ts)--projectand--datasetflags with env var support (SANITY_PROJECT_ID,SANITY_DATASET).png,.jpg,.gif,.webp,.svg,.pdf)/assets/images/and other files to/assets/files/getProjectCliClient({ requireUser: true })— token extracted withclient.config().tokenpackages/@sanity/cli/src/commands/assets/__tests__/upload.test.ts@sanity/cli-test(testCommand,mockApi)Resolution chains
--projectflag →SANITY_PROJECT_IDenv →this.getProjectId()(readssanity.config.ts)--datasetflag →SANITY_DATASETenv →cliConfig.api.dataset→"production"fallbackSANITY_AUTH_TOKENenv →~/.config/sanity/config.json(viasanity login)Build & test