Skip to content

feat(dataset export): add no-strict-asset-verification flag#1135

Merged
dcilke merged 3 commits into
mainfrom
cldx-5406
May 28, 2026
Merged

feat(dataset export): add no-strict-asset-verification flag#1135
dcilke merged 3 commits into
mainfrom
cldx-5406

Conversation

@dcilke
Copy link
Copy Markdown
Member

@dcilke dcilke commented May 27, 2026

Adds a new flag --no-strict-asset-verification to sanity dataset export to disable strict validation of asset hashes.


Note

Low Risk
CLI flag plumbing and dependency bump only; default export behavior remains strict verification unless the flag is set.

Overview
Adds --no-strict-asset-verification to sanity dataset export so exports can continue when downloaded assets fail hash or content-length checks. By default, strictAssetVerification stays true and is passed through to @sanity/export (bumped to ^6.2.0).

The command wires the new oclif flag, documents it in examples, and extends tests to assert the option is forwarded to exportDataset.

Reviewed by Cursor Bugbot for commit 477c236. Bugbot is set up for automated code reviews on this repo. Configure here.

@dcilke dcilke requested a review from a team as a code owner May 27, 2026 13:22
@claude
Copy link
Copy Markdown

claude Bot commented May 27, 2026

Claude finished @dcilke's task in 1m 36s —— View job


Review

  • Read PR diff
  • Review export.ts changes
  • Review test changes
  • Check for changeset
  • Post review

Issues

1. Missing changeset (required)

This PR adds a new user-facing CLI flag — a runtime behavior change — but no .changeset/*.md file is included. Per repo convention (and visible in prior PRs like pr-1063.md, pr-1114.md), a patch changeset is required.

Add .changeset/<slug>.md:

---
"@sanity/cli": patch
---

Add `--no-strict-asset-verification` flag to `sanity dataset export` to continue exports when an asset fails hash/content-length verification.

Fix this →

2. Flag description has awkward double-negativepackages/@sanity/cli/src/commands/datasets/export.ts:85-86

"Skip aborting the export when an asset fails…" is hard to parse (skip-abort = continue). The parenthetical also doesn't match the style of other flag descriptions in this file (none use parentheticals).

Suggested rewording:

description: "Continue the export when an asset fails hash or content-length verification (logs a warning instead of aborting)",

Or, even tighter and consistent with sibling flags:

description: "Do not abort the export when an asset fails hash or content-length verification",

3. New flag not surfaced in examplespackages/@sanity/cli/src/commands/datasets/export.ts:37-54

Other notable flags (--no-assets, --raw, --types) each have an example entry. Consider adding one for --no-strict-asset-verification so users discover it via sanity dataset export --help. Minor / optional.


Notes (no action needed)

  • Flag logic (strictAssetVerification: !flags['no-strict-asset-verification']) is correct given oclif's behavior with explicitly-named no-* flags, and matches the existing pattern for no-assets / no-compress / no-drafts.
  • Test at line 469-481 covers the --no-strict-asset-verification case; the default-case assertion at line 179 (strictAssetVerification: true) is also good. An additional explicit test that omitting the flag still passes strictAssetVerification: true is already effectively covered by that update.
  • @sanity/export bumped from ^6.1.0^6.2.0 in lockfile and package.json — consistent with introducing the new option.
    • Branch: cldx-5406

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

📦 Bundle Stats — @sanity/cli

Compared against main (dafed5c3)

@sanity/cli

Metric Value vs main (dafed5c)
Internal (raw) 2.1 KB -
Internal (gzip) 799 B -
Bundled (raw) 10.97 MB -
Bundled (gzip) 2.06 MB -
Import time 850ms +3ms, +0.4%

bin:sanity

Metric Value vs main (dafed5c)
Internal (raw) 1023 B -
Internal (gzip) 486 B -
Bundled (raw) 9.84 MB -
Bundled (gzip) 1.77 MB -
Import time 1.98s +11ms, +0.6%

🗺️ View treemap · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

📦 Bundle Stats — @sanity/cli-core

Compared against main (dafed5c3)

Metric Value vs main (dafed5c)
Internal (raw) 96.3 KB -
Internal (gzip) 22.7 KB -
Bundled (raw) 21.61 MB -
Bundled (gzip) 3.42 MB -
Import time 791ms -13ms, -1.6%

🗺️ View treemap · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

📦 Bundle Stats — create-sanity

Compared against main (dafed5c3)

Metric Value vs main (dafed5c)
Internal (raw) 908 B -
Internal (gzip) 483 B -
Bundled (raw) 931 B -
Bundled (gzip) 491 B -
Import time ❌ ChildProcess denied: node -
Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

Coverage Delta

File Statements
packages/@sanity/cli/src/commands/datasets/export.ts 93.5% (±0%)

Comparing 1 changed file against main @ dafed5c321960164da255ff024dda49667cc5ec3

Overall Coverage

Metric Coverage
Statements 84.3% (±0%)
Branches 74.3% (±0%)
Functions 84.2% (±0%)
Lines 84.8% (±0%)

@runeb
Copy link
Copy Markdown
Member

runeb commented May 27, 2026

@claude address the 3 review findings in a new commit

@squiggler-app
Copy link
Copy Markdown
Contributor

squiggler-app Bot commented May 27, 2026

Claude finished @runeb's task in 2m 5s —— View job


Addressed review findings in 477c236

  • Finding 1 — Missing changeset: Already addressed by auto-generated .changeset/pr-1135.md.
  • Finding 2 — Double-negative flag description: Reworded to "Do not abort the export when an asset fails hash or content-length verification" (matches the second tighter suggestion).
  • Finding 3 — Surface flag in examples: Added a new entry under examples demonstrating --no-strict-asset-verification.

Diff

+    {
+      command:
+        '<%= config.bin %> <%= command.id %> moviedb moviedb.tar.gz --no-strict-asset-verification',
+      description: 'Export dataset without aborting on asset verification failures',
+    },
...
     'no-strict-asset-verification': Flags.boolean({
       default: false,
       description:
-        'Skip aborting the export when an asset fails hash/content-length verification (keeps the asset with a warning)',
+        'Do not abort the export when an asset fails hash or content-length verification',
     }),

Notes

  • I was unable to run pnpm check:lint / pnpm test from this environment (commands required approval that isn't granted to this action). The change is a string/example update with no logic impact, but CI should be the source of truth here — please re-run if you'd like me to address any failures.
    · Branch: cldx-5406

…n and add example

Co-authored-by: Rune Botten <runeb@users.noreply.github.com>
Copy link
Copy Markdown
Member

@runeb runeb left a comment

Choose a reason for hiding this comment

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

LGTM

@dcilke dcilke merged commit 96f0646 into main May 28, 2026
85 of 89 checks passed
@dcilke dcilke deleted the cldx-5406 branch May 28, 2026 06:08
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