snapshots upload --selective with an empty image dir creates no snapshot even when --all-image-file-names-file lists the full suite #1397

Description

@joshdholtz

Problem Statement

Summary

sentry-cli snapshots upload given an empty directory prints "No image files found" and returns early without creating a snapshot, even when --all-image-file-names-file is passed with the complete image list. This makes it impossible to record an "all unchanged" build for a selective upload.

Context

We use selective uploads for PR visual diffing: on each PR we diff against a baseline and upload only changed/added images with --selective --all-image-file-names-file <all names>. The names file is meant to let Sentry tell "skipped" (unchanged, still present) apart from "removed".

Problem

For any PR that changes no images, the selective staging directory is empty. snapshots upload early-returns: in src/commands/snapshots/upload.rs, the if images.is_empty() { ... return Ok(()) } guard runs before--all-image-file-names-file is parsed and before the manifest is POSTed. Result: the PR gets no snapshot build at all, so there is nothing for reviewers/checks to see, and the "everything unchanged" state cannot be represented.

Expected

When --all-image-file-names-file (or --all-image-file-names) is provided, a selective upload with zero image files should still create a snapshot/manifest that references the full name list as unchanged, instead of early-returning.

Version

sentry-cli 3.6.0

Workaround

We currently skip the upload entirely when nothing changed, accepting that all-unchanged PRs have no snapshot build.

Solution Brainstorm

When --all-image-file-names/--all-image-file-names-file is present, treat the name list (not the on-disk image count) as the source of truth for what the snapshot should contain, so an empty upload directory is a valid "everything unchanged" case rather than a no-op.

Concretely, in src/commands/snapshots/upload.rs::execute, the early if images.is_empty() { return Ok(()) } guard fires before --all-image-file-names-file is parsed and before the manifest is POSTed. A zero-image selective upload with a full name list could instead build and POST a manifest with no changed image entries, letting the server record every listed name as unchanged/skipped.

Options, roughly in order of preference:

  1. Skip the empty-dir early return when a full name list is provided, and POST an empty-images manifest (selective: true, full all_image_file_names). The server already knows how to reconcile skipped vs removed from that list.
  2. Add an explicit flag (e.g. --allow-empty) that opts into creating an all-unchanged snapshot.
  3. At minimum, exit non-zero (or warn loudly) when --selective + a names file is given but zero images are uploaded, so callers know no snapshot was created instead of it silently succeeding.

Our current workaround is to detect the zero-change case ourselves and skip the upload, which leaves those PRs without a snapshot build.

Metadata

Metadata

Assignees

Labels

jaredTrigger the Jared agent to work on stuff

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

snapshots upload --selective with an empty image dir creates no snapshot even when --all-image-file-names-file lists the full suite #1397

Description

@joshdholtz

Problem Statement

Summary

sentry-cli snapshots upload given an empty directory prints "No image files found" and returns early without creating a snapshot, even when --all-image-file-names-file is passed with the complete image list. This makes it impossible to record an "all unchanged" build for a selective upload.

Context

We use selective uploads for PR visual diffing: on each PR we diff against a baseline and upload only changed/added images with --selective --all-image-file-names-file <all names>. The names file is meant to let Sentry tell "skipped" (unchanged, still present) apart from "removed".

Problem

For any PR that changes no images, the selective staging directory is empty. snapshots upload early-returns: in src/commands/snapshots/upload.rs, the if images.is_empty() { ... return Ok(()) } guard runs before--all-image-file-names-file is parsed and before the manifest is POSTed. Result: the PR gets no snapshot build at all, so there is nothing for reviewers/checks to see, and the "everything unchanged" state cannot be represented.

Expected

When --all-image-file-names-file (or --all-image-file-names) is provided, a selective upload with zero image files should still create a snapshot/manifest that references the full name list as unchanged, instead of early-returning.

Version

sentry-cli 3.6.0

Workaround

We currently skip the upload entirely when nothing changed, accepting that all-unchanged PRs have no snapshot build.

Solution Brainstorm

When --all-image-file-names/--all-image-file-names-file is present, treat the name list (not the on-disk image count) as the source of truth for what the snapshot should contain, so an empty upload directory is a valid "everything unchanged" case rather than a no-op.

Concretely, in src/commands/snapshots/upload.rs::execute, the early if images.is_empty() { return Ok(()) } guard fires before --all-image-file-names-file is parsed and before the manifest is POSTed. A zero-image selective upload with a full name list could instead build and POST a manifest with no changed image entries, letting the server record every listed name as unchanged/skipped.

Options, roughly in order of preference:

  1. Skip the empty-dir early return when a full name list is provided, and POST an empty-images manifest (selective: true, full all_image_file_names). The server already knows how to reconcile skipped vs removed from that list.
  2. Add an explicit flag (e.g. --allow-empty) that opts into creating an all-unchanged snapshot.
  3. At minimum, exit non-zero (or warn loudly) when --selective + a names file is given but zero images are uploaded, so callers know no snapshot was created instead of it silently succeeding.

Our current workaround is to detect the zero-change case ourselves and skip the upload, which leaves those PRs without a snapshot build.

Metadata

Metadata

Assignees

Labels

jaredTrigger the Jared agent to work on stuff

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

snapshots upload --selective with an empty image dir creates no snapshot even when --all-image-file-names-file lists the full suite #1397

Description

@joshdholtz

Problem Statement

Summary

sentry-cli snapshots upload given an empty directory prints "No image files found" and returns early without creating a snapshot, even when --all-image-file-names-file is passed with the complete image list. This makes it impossible to record an "all unchanged" build for a selective upload.

Context

We use selective uploads for PR visual diffing: on each PR we diff against a baseline and upload only changed/added images with --selective --all-image-file-names-file <all names>. The names file is meant to let Sentry tell "skipped" (unchanged, still present) apart from "removed".

Problem

For any PR that changes no images, the selective staging directory is empty. snapshots upload early-returns: in src/commands/snapshots/upload.rs, the if images.is_empty() { ... return Ok(()) } guard runs before--all-image-file-names-file is parsed and before the manifest is POSTed. Result: the PR gets no snapshot build at all, so there is nothing for reviewers/checks to see, and the "everything unchanged" state cannot be represented.

Expected

When --all-image-file-names-file (or --all-image-file-names) is provided, a selective upload with zero image files should still create a snapshot/manifest that references the full name list as unchanged, instead of early-returning.

Version

sentry-cli 3.6.0

Workaround

We currently skip the upload entirely when nothing changed, accepting that all-unchanged PRs have no snapshot build.

Solution Brainstorm

When --all-image-file-names/--all-image-file-names-file is present, treat the name list (not the on-disk image count) as the source of truth for what the snapshot should contain, so an empty upload directory is a valid "everything unchanged" case rather than a no-op.

Concretely, in src/commands/snapshots/upload.rs::execute, the early if images.is_empty() { return Ok(()) } guard fires before --all-image-file-names-file is parsed and before the manifest is POSTed. A zero-image selective upload with a full name list could instead build and POST a manifest with no changed image entries, letting the server record every listed name as unchanged/skipped.

Options, roughly in order of preference:

  1. Skip the empty-dir early return when a full name list is provided, and POST an empty-images manifest (selective: true, full all_image_file_names). The server already knows how to reconcile skipped vs removed from that list.
  2. Add an explicit flag (e.g. --allow-empty) that opts into creating an all-unchanged snapshot.
  3. At minimum, exit non-zero (or warn loudly) when --selective + a names file is given but zero images are uploaded, so callers know no snapshot was created instead of it silently succeeding.

Our current workaround is to detect the zero-change case ourselves and skip the upload, which leaves those PRs without a snapshot build.

Metadata

Metadata

Assignees

Labels

jaredTrigger the Jared agent to work on stuff

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

snapshots upload --selective with an empty image dir creates no snapshot even when --all-image-file-names-file lists the full suite #1397

Description

@joshdholtz

Problem Statement

Summary

sentry-cli snapshots upload given an empty directory prints "No image files found" and returns early without creating a snapshot, even when --all-image-file-names-file is passed with the complete image list. This makes it impossible to record an "all unchanged" build for a selective upload.

Context

We use selective uploads for PR visual diffing: on each PR we diff against a baseline and upload only changed/added images with --selective --all-image-file-names-file <all names>. The names file is meant to let Sentry tell "skipped" (unchanged, still present) apart from "removed".

Problem

For any PR that changes no images, the selective staging directory is empty. snapshots upload early-returns: in src/commands/snapshots/upload.rs, the if images.is_empty() { ... return Ok(()) } guard runs before--all-image-file-names-file is parsed and before the manifest is POSTed. Result: the PR gets no snapshot build at all, so there is nothing for reviewers/checks to see, and the "everything unchanged" state cannot be represented.

Expected

When --all-image-file-names-file (or --all-image-file-names) is provided, a selective upload with zero image files should still create a snapshot/manifest that references the full name list as unchanged, instead of early-returning.

Version

sentry-cli 3.6.0

Workaround

We currently skip the upload entirely when nothing changed, accepting that all-unchanged PRs have no snapshot build.

Solution Brainstorm

When --all-image-file-names/--all-image-file-names-file is present, treat the name list (not the on-disk image count) as the source of truth for what the snapshot should contain, so an empty upload directory is a valid "everything unchanged" case rather than a no-op.

Concretely, in src/commands/snapshots/upload.rs::execute, the early if images.is_empty() { return Ok(()) } guard fires before --all-image-file-names-file is parsed and before the manifest is POSTed. A zero-image selective upload with a full name list could instead build and POST a manifest with no changed image entries, letting the server record every listed name as unchanged/skipped.

Options, roughly in order of preference:

  1. Skip the empty-dir early return when a full name list is provided, and POST an empty-images manifest (selective: true, full all_image_file_names). The server already knows how to reconcile skipped vs removed from that list.
  2. Add an explicit flag (e.g. --allow-empty) that opts into creating an all-unchanged snapshot.
  3. At minimum, exit non-zero (or warn loudly) when --selective + a names file is given but zero images are uploaded, so callers know no snapshot was created instead of it silently succeeding.

Our current workaround is to detect the zero-change case ourselves and skip the upload, which leaves those PRs without a snapshot build.

Metadata

Metadata

Assignees

Labels

jaredTrigger the Jared agent to work on stuff

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

snapshots upload --selective with an empty image dir creates no snapshot even when --all-image-file-names-file lists the full suite #1397

Description

@joshdholtz

Problem Statement

Summary

sentry-cli snapshots upload given an empty directory prints "No image files found" and returns early without creating a snapshot, even when --all-image-file-names-file is passed with the complete image list. This makes it impossible to record an "all unchanged" build for a selective upload.

Context

We use selective uploads for PR visual diffing: on each PR we diff against a baseline and upload only changed/added images with --selective --all-image-file-names-file <all names>. The names file is meant to let Sentry tell "skipped" (unchanged, still present) apart from "removed".

Problem

For any PR that changes no images, the selective staging directory is empty. snapshots upload early-returns: in src/commands/snapshots/upload.rs, the if images.is_empty() { ... return Ok(()) } guard runs before--all-image-file-names-file is parsed and before the manifest is POSTed. Result: the PR gets no snapshot build at all, so there is nothing for reviewers/checks to see, and the "everything unchanged" state cannot be represented.

Expected

When --all-image-file-names-file (or --all-image-file-names) is provided, a selective upload with zero image files should still create a snapshot/manifest that references the full name list as unchanged, instead of early-returning.

Version

sentry-cli 3.6.0

Workaround

We currently skip the upload entirely when nothing changed, accepting that all-unchanged PRs have no snapshot build.

Solution Brainstorm

When --all-image-file-names/--all-image-file-names-file is present, treat the name list (not the on-disk image count) as the source of truth for what the snapshot should contain, so an empty upload directory is a valid "everything unchanged" case rather than a no-op.

Concretely, in src/commands/snapshots/upload.rs::execute, the early if images.is_empty() { return Ok(()) } guard fires before --all-image-file-names-file is parsed and before the manifest is POSTed. A zero-image selective upload with a full name list could instead build and POST a manifest with no changed image entries, letting the server record every listed name as unchanged/skipped.

Options, roughly in order of preference:

  1. Skip the empty-dir early return when a full name list is provided, and POST an empty-images manifest (selective: true, full all_image_file_names). The server already knows how to reconcile skipped vs removed from that list.
  2. Add an explicit flag (e.g. --allow-empty) that opts into creating an all-unchanged snapshot.
  3. At minimum, exit non-zero (or warn loudly) when --selective + a names file is given but zero images are uploaded, so callers know no snapshot was created instead of it silently succeeding.

Our current workaround is to detect the zero-change case ourselves and skip the upload, which leaves those PRs without a snapshot build.

Metadata

Metadata

Assignees

Labels

jaredTrigger the Jared agent to work on stuff

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

snapshots upload --selective with an empty image dir creates no snapshot even when --all-image-file-names-file lists the full suite #1397

Description

@joshdholtz

Problem Statement

Summary

sentry-cli snapshots upload given an empty directory prints "No image files found" and returns early without creating a snapshot, even when --all-image-file-names-file is passed with the complete image list. This makes it impossible to record an "all unchanged" build for a selective upload.

Context

We use selective uploads for PR visual diffing: on each PR we diff against a baseline and upload only changed/added images with --selective --all-image-file-names-file <all names>. The names file is meant to let Sentry tell "skipped" (unchanged, still present) apart from "removed".

Problem

For any PR that changes no images, the selective staging directory is empty. snapshots upload early-returns: in src/commands/snapshots/upload.rs, the if images.is_empty() { ... return Ok(()) } guard runs before--all-image-file-names-file is parsed and before the manifest is POSTed. Result: the PR gets no snapshot build at all, so there is nothing for reviewers/checks to see, and the "everything unchanged" state cannot be represented.

Expected

When --all-image-file-names-file (or --all-image-file-names) is provided, a selective upload with zero image files should still create a snapshot/manifest that references the full name list as unchanged, instead of early-returning.

Version

sentry-cli 3.6.0

Workaround

We currently skip the upload entirely when nothing changed, accepting that all-unchanged PRs have no snapshot build.

Solution Brainstorm

When --all-image-file-names/--all-image-file-names-file is present, treat the name list (not the on-disk image count) as the source of truth for what the snapshot should contain, so an empty upload directory is a valid "everything unchanged" case rather than a no-op.

Concretely, in src/commands/snapshots/upload.rs::execute, the early if images.is_empty() { return Ok(()) } guard fires before --all-image-file-names-file is parsed and before the manifest is POSTed. A zero-image selective upload with a full name list could instead build and POST a manifest with no changed image entries, letting the server record every listed name as unchanged/skipped.

Options, roughly in order of preference:

  1. Skip the empty-dir early return when a full name list is provided, and POST an empty-images manifest (selective: true, full all_image_file_names). The server already knows how to reconcile skipped vs removed from that list.
  2. Add an explicit flag (e.g. --allow-empty) that opts into creating an all-unchanged snapshot.
  3. At minimum, exit non-zero (or warn loudly) when --selective + a names file is given but zero images are uploaded, so callers know no snapshot was created instead of it silently succeeding.

Our current workaround is to detect the zero-change case ourselves and skip the upload, which leaves those PRs without a snapshot build.

Metadata

Metadata

Assignees

Labels

jaredTrigger the Jared agent to work on stuff

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

snapshots upload --selective with an empty image dir creates no snapshot even when --all-image-file-names-file lists the full suite #1397

Description

@joshdholtz

Problem Statement

Summary

sentry-cli snapshots upload given an empty directory prints "No image files found" and returns early without creating a snapshot, even when --all-image-file-names-file is passed with the complete image list. This makes it impossible to record an "all unchanged" build for a selective upload.

Context

We use selective uploads for PR visual diffing: on each PR we diff against a baseline and upload only changed/added images with --selective --all-image-file-names-file <all names>. The names file is meant to let Sentry tell "skipped" (unchanged, still present) apart from "removed".

Problem

For any PR that changes no images, the selective staging directory is empty. snapshots upload early-returns: in src/commands/snapshots/upload.rs, the if images.is_empty() { ... return Ok(()) } guard runs before--all-image-file-names-file is parsed and before the manifest is POSTed. Result: the PR gets no snapshot build at all, so there is nothing for reviewers/checks to see, and the "everything unchanged" state cannot be represented.

Expected

When --all-image-file-names-file (or --all-image-file-names) is provided, a selective upload with zero image files should still create a snapshot/manifest that references the full name list as unchanged, instead of early-returning.

Version

sentry-cli 3.6.0

Workaround

We currently skip the upload entirely when nothing changed, accepting that all-unchanged PRs have no snapshot build.

Solution Brainstorm

When --all-image-file-names/--all-image-file-names-file is present, treat the name list (not the on-disk image count) as the source of truth for what the snapshot should contain, so an empty upload directory is a valid "everything unchanged" case rather than a no-op.

Concretely, in src/commands/snapshots/upload.rs::execute, the early if images.is_empty() { return Ok(()) } guard fires before --all-image-file-names-file is parsed and before the manifest is POSTed. A zero-image selective upload with a full name list could instead build and POST a manifest with no changed image entries, letting the server record every listed name as unchanged/skipped.

Options, roughly in order of preference:

  1. Skip the empty-dir early return when a full name list is provided, and POST an empty-images manifest (selective: true, full all_image_file_names). The server already knows how to reconcile skipped vs removed from that list.
  2. Add an explicit flag (e.g. --allow-empty) that opts into creating an all-unchanged snapshot.
  3. At minimum, exit non-zero (or warn loudly) when --selective + a names file is given but zero images are uploaded, so callers know no snapshot was created instead of it silently succeeding.

Our current workaround is to detect the zero-change case ourselves and skip the upload, which leaves those PRs without a snapshot build.

Metadata

Metadata

Assignees

Labels

jaredTrigger the Jared agent to work on stuff

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

snapshots upload --selective with an empty image dir creates no snapshot even when --all-image-file-names-file lists the full suite #1397

Description

@joshdholtz

Problem Statement

Summary

sentry-cli snapshots upload given an empty directory prints "No image files found" and returns early without creating a snapshot, even when --all-image-file-names-file is passed with the complete image list. This makes it impossible to record an "all unchanged" build for a selective upload.

Context

We use selective uploads for PR visual diffing: on each PR we diff against a baseline and upload only changed/added images with --selective --all-image-file-names-file <all names>. The names file is meant to let Sentry tell "skipped" (unchanged, still present) apart from "removed".

Problem

For any PR that changes no images, the selective staging directory is empty. snapshots upload early-returns: in src/commands/snapshots/upload.rs, the if images.is_empty() { ... return Ok(()) } guard runs before--all-image-file-names-file is parsed and before the manifest is POSTed. Result: the PR gets no snapshot build at all, so there is nothing for reviewers/checks to see, and the "everything unchanged" state cannot be represented.

Expected

When --all-image-file-names-file (or --all-image-file-names) is provided, a selective upload with zero image files should still create a snapshot/manifest that references the full name list as unchanged, instead of early-returning.

Version

sentry-cli 3.6.0

Workaround

We currently skip the upload entirely when nothing changed, accepting that all-unchanged PRs have no snapshot build.

Solution Brainstorm

When --all-image-file-names/--all-image-file-names-file is present, treat the name list (not the on-disk image count) as the source of truth for what the snapshot should contain, so an empty upload directory is a valid "everything unchanged" case rather than a no-op.

Concretely, in src/commands/snapshots/upload.rs::execute, the early if images.is_empty() { return Ok(()) } guard fires before --all-image-file-names-file is parsed and before the manifest is POSTed. A zero-image selective upload with a full name list could instead build and POST a manifest with no changed image entries, letting the server record every listed name as unchanged/skipped.

Options, roughly in order of preference:

  1. Skip the empty-dir early return when a full name list is provided, and POST an empty-images manifest (selective: true, full all_image_file_names). The server already knows how to reconcile skipped vs removed from that list.
  2. Add an explicit flag (e.g. --allow-empty) that opts into creating an all-unchanged snapshot.
  3. At minimum, exit non-zero (or warn loudly) when --selective + a names file is given but zero images are uploaded, so callers know no snapshot was created instead of it silently succeeding.

Our current workaround is to detect the zero-change case ourselves and skip the upload, which leaves those PRs without a snapshot build.

Metadata

Metadata

Assignees

Labels

jaredTrigger the Jared agent to work on stuff

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions