[codex] Structure update manifest failures - #3293

Merged
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors
Jun 20, 2026
Merged

[codex] Structure update manifest failures#3293
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • replace generic update-manifest parse, merge, and serialization exceptions with structured Schema errors
  • retain platform, source location, offending input, and conflicting version/field/file metadata
  • expose known validation failures through the Effect error channel while leaving unexpected exceptions as defects
  • keep semantic validation failures cause-free

Validation

  • vp test run scripts/merge-update-manifests.test.ts (10 tests)
  • vp check
  • vp run typecheck

Sensitive-context follow-up\n- unsupported manifest lines are represented by line number and line length rather than raw contents\n- parse messages no longer echo potentially sensitive line text


Note

Medium Risk
Changes how merge-update-manifest failures are represented (typed Effect errors vs generic throws), which callers must handle; scope is release scripting, not runtime auth, with lower blast radius but a behavioral contract shift.

Overview
Replaces generic Error throws in update manifest parse, merge, and serialize with Effect Schema tagged errors (UpdateManifestParseError, version/extra/file conflict errors, serialization errors). Adds attemptUpdateManifest so the merge CLI surfaces known failures on the Effect error channel while unexpected exceptions still die as defects.

Failure payloads and messages are tightened so logs do not echo sensitive input: unsupported lines report line number and length (not the line text), file URL conflicts use getUrlDiagnostics (length, protocol, hostname only), and extra-field conflicts record types and string lengths instead of raw values. File merge conflicts also record which manifest (primary / secondary) and hash/size metadata without embedding full URLs or sha512 strings.

Adds shared getUrlDiagnostics (exported from @t3tools/shared/urlDiagnostics) and redactDpopRequestTarget in dpop.ts for the same redaction pattern; tests assert neither helper leaks credentials, paths, or query tokens.

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

Note

Replace generic errors with typed structured errors in update manifest parsing and merging

  • Introduces typed error classes (UpdateManifestParseError, UpdateManifestVersionConflictError, UpdateManifestExtraConflictError, UpdateManifestFileConflictError, UpdateManifestSerializationError) using effect/Schema in update-manifest.ts, replacing all generic Error throws.
  • Conflict errors expose only non-sensitive diagnostics (types, lengths, URL protocol/hostname) rather than raw values, using a new getUrlDiagnostics utility in urlDiagnostics.ts and redactDpopRequestTarget in dpop.ts.
  • Adds attemptUpdateManifest helper to wrap manifest operations as Effect failures instead of defects; merge-update-manifests.ts is updated to use it, surfacing errors on the Effect error channel.
  • Behavioral Change: callers of mergeUpdateManifestFiles now receive typed Effect failures instead of thrown exceptions for all parse, merge, and serialization errors.

Macroscope summarized 4bd4e6d.

@coderabbitai

coderabbitaiBot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3cf50f52-e7f8-43c7-a769-0be0fc7f89a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/update-manifest-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Parse message omits offending line
    • Added the offendingLine content to the message getter so it now produces messages like "unsupported line 'not yaml'." when offendingLine is present.
  • ✅ Fixed: File conflict labels wrong manifest
    • Separated the combined file iteration into two explicit loops (primary first, then secondary) so that primarySha512/primarySize are structurally guaranteed to come from the primary manifest's entries for cross-manifest conflicts.

Create PR

Or push these changes by commenting:

@cursor push 3d9418fa45
Preview (3d9418fa45)
diff --git a/scripts/lib/update-manifest.ts b/scripts/lib/update-manifest.ts--- a/scripts/lib/update-manifest.ts+++ b/scripts/lib/update-manifest.ts@@ -36,7 +36,8 @@
override get message(): string {
const location =
this.lineNumber === undefined ? this.sourcePath : `${this.sourcePath}:${this.lineNumber}`;
- return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}.`;+ const suffix = this.offendingLine !== undefined ? ` '${this.offendingLine}'` : "";+ return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}${suffix}.`;
}
}
@@ -361,7 +362,7 @@
}
const filesByUrl = new Map<string, UpdateManifestFile>();
- for (const file of [...primary.files, ...secondary.files]) {+ for (const file of primary.files) {
const existing = filesByUrl.get(file.url);
if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {
throw new UpdateManifestFileConflictError({
@@ -375,6 +376,20 @@
}
filesByUrl.set(file.url, file);
}
+ for (const file of secondary.files) {+ const existing = filesByUrl.get(file.url);+ if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {+ throw new UpdateManifestFileConflictError({+ platformLabel,+ url: file.url,+ primarySha512: existing.sha512,+ primarySize: existing.size,+ secondarySha512: file.sha512,+ secondarySize: file.size,+ });+ }+ filesByUrl.set(file.url, file);+ }
return {
version: primary.version,

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit ac92fa2. Configure here.

Comment threadscripts/lib/update-manifest.ts
Comment threadscripts/lib/update-manifest.ts Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeappBot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Build script refactor converting plain Error throws to structured tagged error classes. Same error conditions, same behavior - just better typed errors. Comprehensive tests verify the new structure and that sensitive data is not leaked.

You can customize Macroscope's approvability policy. Learn more.

@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 14:37

Dismissing prior approval to re-evaluate 9114601

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 16:10

Dismissing prior approval to re-evaluate c9aeab5

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
juliusmarmingeand others added 4 commits June 20, 2026 10:11
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge
juliusmarmingeforce-pushed the codex/update-manifest-errors branch from c9aeab5 to 4bd4e6dCompareJune 20, 2026 17:15
@juliusmarminge
juliusmarminge changed the base branch from main to codex/redact-dpop-request-targetJune 20, 2026 17:15
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 17:15

Dismissing prior approval to re-evaluate 4bd4e6d

@juliusmarminge
juliusmarminge merged this pull request into codex/redact-dpop-request-targetJun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/update-manifest-errors branch June 20, 2026 18:33
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

[codex] Structure update manifest failures - #3293

Merged
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors
Jun 20, 2026
Merged

[codex] Structure update manifest failures#3293
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • replace generic update-manifest parse, merge, and serialization exceptions with structured Schema errors
  • retain platform, source location, offending input, and conflicting version/field/file metadata
  • expose known validation failures through the Effect error channel while leaving unexpected exceptions as defects
  • keep semantic validation failures cause-free

Validation

  • vp test run scripts/merge-update-manifests.test.ts (10 tests)
  • vp check
  • vp run typecheck

Sensitive-context follow-up\n- unsupported manifest lines are represented by line number and line length rather than raw contents\n- parse messages no longer echo potentially sensitive line text


Note

Medium Risk
Changes how merge-update-manifest failures are represented (typed Effect errors vs generic throws), which callers must handle; scope is release scripting, not runtime auth, with lower blast radius but a behavioral contract shift.

Overview
Replaces generic Error throws in update manifest parse, merge, and serialize with Effect Schema tagged errors (UpdateManifestParseError, version/extra/file conflict errors, serialization errors). Adds attemptUpdateManifest so the merge CLI surfaces known failures on the Effect error channel while unexpected exceptions still die as defects.

Failure payloads and messages are tightened so logs do not echo sensitive input: unsupported lines report line number and length (not the line text), file URL conflicts use getUrlDiagnostics (length, protocol, hostname only), and extra-field conflicts record types and string lengths instead of raw values. File merge conflicts also record which manifest (primary / secondary) and hash/size metadata without embedding full URLs or sha512 strings.

Adds shared getUrlDiagnostics (exported from @t3tools/shared/urlDiagnostics) and redactDpopRequestTarget in dpop.ts for the same redaction pattern; tests assert neither helper leaks credentials, paths, or query tokens.

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

Note

Replace generic errors with typed structured errors in update manifest parsing and merging

  • Introduces typed error classes (UpdateManifestParseError, UpdateManifestVersionConflictError, UpdateManifestExtraConflictError, UpdateManifestFileConflictError, UpdateManifestSerializationError) using effect/Schema in update-manifest.ts, replacing all generic Error throws.
  • Conflict errors expose only non-sensitive diagnostics (types, lengths, URL protocol/hostname) rather than raw values, using a new getUrlDiagnostics utility in urlDiagnostics.ts and redactDpopRequestTarget in dpop.ts.
  • Adds attemptUpdateManifest helper to wrap manifest operations as Effect failures instead of defects; merge-update-manifests.ts is updated to use it, surfacing errors on the Effect error channel.
  • Behavioral Change: callers of mergeUpdateManifestFiles now receive typed Effect failures instead of thrown exceptions for all parse, merge, and serialization errors.

Macroscope summarized 4bd4e6d.

@coderabbitai

coderabbitaiBot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3cf50f52-e7f8-43c7-a769-0be0fc7f89a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/update-manifest-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Parse message omits offending line
    • Added the offendingLine content to the message getter so it now produces messages like "unsupported line 'not yaml'." when offendingLine is present.
  • ✅ Fixed: File conflict labels wrong manifest
    • Separated the combined file iteration into two explicit loops (primary first, then secondary) so that primarySha512/primarySize are structurally guaranteed to come from the primary manifest's entries for cross-manifest conflicts.

Create PR

Or push these changes by commenting:

@cursor push 3d9418fa45
Preview (3d9418fa45)
diff --git a/scripts/lib/update-manifest.ts b/scripts/lib/update-manifest.ts--- a/scripts/lib/update-manifest.ts+++ b/scripts/lib/update-manifest.ts@@ -36,7 +36,8 @@
override get message(): string {
const location =
this.lineNumber === undefined ? this.sourcePath : `${this.sourcePath}:${this.lineNumber}`;
- return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}.`;+ const suffix = this.offendingLine !== undefined ? ` '${this.offendingLine}'` : "";+ return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}${suffix}.`;
}
}
@@ -361,7 +362,7 @@
}
const filesByUrl = new Map<string, UpdateManifestFile>();
- for (const file of [...primary.files, ...secondary.files]) {+ for (const file of primary.files) {
const existing = filesByUrl.get(file.url);
if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {
throw new UpdateManifestFileConflictError({
@@ -375,6 +376,20 @@
}
filesByUrl.set(file.url, file);
}
+ for (const file of secondary.files) {+ const existing = filesByUrl.get(file.url);+ if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {+ throw new UpdateManifestFileConflictError({+ platformLabel,+ url: file.url,+ primarySha512: existing.sha512,+ primarySize: existing.size,+ secondarySha512: file.sha512,+ secondarySize: file.size,+ });+ }+ filesByUrl.set(file.url, file);+ }
return {
version: primary.version,

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit ac92fa2. Configure here.

Comment threadscripts/lib/update-manifest.ts
Comment threadscripts/lib/update-manifest.ts Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeappBot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Build script refactor converting plain Error throws to structured tagged error classes. Same error conditions, same behavior - just better typed errors. Comprehensive tests verify the new structure and that sensitive data is not leaked.

You can customize Macroscope's approvability policy. Learn more.

@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 14:37

Dismissing prior approval to re-evaluate 9114601

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 16:10

Dismissing prior approval to re-evaluate c9aeab5

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
juliusmarmingeand others added 4 commits June 20, 2026 10:11
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge
juliusmarmingeforce-pushed the codex/update-manifest-errors branch from c9aeab5 to 4bd4e6dCompareJune 20, 2026 17:15
@juliusmarminge
juliusmarminge changed the base branch from main to codex/redact-dpop-request-targetJune 20, 2026 17:15
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 17:15

Dismissing prior approval to re-evaluate 4bd4e6d

@juliusmarminge
juliusmarminge merged this pull request into codex/redact-dpop-request-targetJun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/update-manifest-errors branch June 20, 2026 18:33
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

[codex] Structure update manifest failures - #3293

Merged
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors
Jun 20, 2026
Merged

[codex] Structure update manifest failures#3293
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • replace generic update-manifest parse, merge, and serialization exceptions with structured Schema errors
  • retain platform, source location, offending input, and conflicting version/field/file metadata
  • expose known validation failures through the Effect error channel while leaving unexpected exceptions as defects
  • keep semantic validation failures cause-free

Validation

  • vp test run scripts/merge-update-manifests.test.ts (10 tests)
  • vp check
  • vp run typecheck

Sensitive-context follow-up\n- unsupported manifest lines are represented by line number and line length rather than raw contents\n- parse messages no longer echo potentially sensitive line text


Note

Medium Risk
Changes how merge-update-manifest failures are represented (typed Effect errors vs generic throws), which callers must handle; scope is release scripting, not runtime auth, with lower blast radius but a behavioral contract shift.

Overview
Replaces generic Error throws in update manifest parse, merge, and serialize with Effect Schema tagged errors (UpdateManifestParseError, version/extra/file conflict errors, serialization errors). Adds attemptUpdateManifest so the merge CLI surfaces known failures on the Effect error channel while unexpected exceptions still die as defects.

Failure payloads and messages are tightened so logs do not echo sensitive input: unsupported lines report line number and length (not the line text), file URL conflicts use getUrlDiagnostics (length, protocol, hostname only), and extra-field conflicts record types and string lengths instead of raw values. File merge conflicts also record which manifest (primary / secondary) and hash/size metadata without embedding full URLs or sha512 strings.

Adds shared getUrlDiagnostics (exported from @t3tools/shared/urlDiagnostics) and redactDpopRequestTarget in dpop.ts for the same redaction pattern; tests assert neither helper leaks credentials, paths, or query tokens.

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

Note

Replace generic errors with typed structured errors in update manifest parsing and merging

  • Introduces typed error classes (UpdateManifestParseError, UpdateManifestVersionConflictError, UpdateManifestExtraConflictError, UpdateManifestFileConflictError, UpdateManifestSerializationError) using effect/Schema in update-manifest.ts, replacing all generic Error throws.
  • Conflict errors expose only non-sensitive diagnostics (types, lengths, URL protocol/hostname) rather than raw values, using a new getUrlDiagnostics utility in urlDiagnostics.ts and redactDpopRequestTarget in dpop.ts.
  • Adds attemptUpdateManifest helper to wrap manifest operations as Effect failures instead of defects; merge-update-manifests.ts is updated to use it, surfacing errors on the Effect error channel.
  • Behavioral Change: callers of mergeUpdateManifestFiles now receive typed Effect failures instead of thrown exceptions for all parse, merge, and serialization errors.

Macroscope summarized 4bd4e6d.

@coderabbitai

coderabbitaiBot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3cf50f52-e7f8-43c7-a769-0be0fc7f89a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/update-manifest-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Parse message omits offending line
    • Added the offendingLine content to the message getter so it now produces messages like "unsupported line 'not yaml'." when offendingLine is present.
  • ✅ Fixed: File conflict labels wrong manifest
    • Separated the combined file iteration into two explicit loops (primary first, then secondary) so that primarySha512/primarySize are structurally guaranteed to come from the primary manifest's entries for cross-manifest conflicts.

Create PR

Or push these changes by commenting:

@cursor push 3d9418fa45
Preview (3d9418fa45)
diff --git a/scripts/lib/update-manifest.ts b/scripts/lib/update-manifest.ts--- a/scripts/lib/update-manifest.ts+++ b/scripts/lib/update-manifest.ts@@ -36,7 +36,8 @@
override get message(): string {
const location =
this.lineNumber === undefined ? this.sourcePath : `${this.sourcePath}:${this.lineNumber}`;
- return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}.`;+ const suffix = this.offendingLine !== undefined ? ` '${this.offendingLine}'` : "";+ return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}${suffix}.`;
}
}
@@ -361,7 +362,7 @@
}
const filesByUrl = new Map<string, UpdateManifestFile>();
- for (const file of [...primary.files, ...secondary.files]) {+ for (const file of primary.files) {
const existing = filesByUrl.get(file.url);
if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {
throw new UpdateManifestFileConflictError({
@@ -375,6 +376,20 @@
}
filesByUrl.set(file.url, file);
}
+ for (const file of secondary.files) {+ const existing = filesByUrl.get(file.url);+ if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {+ throw new UpdateManifestFileConflictError({+ platformLabel,+ url: file.url,+ primarySha512: existing.sha512,+ primarySize: existing.size,+ secondarySha512: file.sha512,+ secondarySize: file.size,+ });+ }+ filesByUrl.set(file.url, file);+ }
return {
version: primary.version,

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit ac92fa2. Configure here.

Comment threadscripts/lib/update-manifest.ts
Comment threadscripts/lib/update-manifest.ts Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeappBot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Build script refactor converting plain Error throws to structured tagged error classes. Same error conditions, same behavior - just better typed errors. Comprehensive tests verify the new structure and that sensitive data is not leaked.

You can customize Macroscope's approvability policy. Learn more.

@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 14:37

Dismissing prior approval to re-evaluate 9114601

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 16:10

Dismissing prior approval to re-evaluate c9aeab5

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
juliusmarmingeand others added 4 commits June 20, 2026 10:11
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge
juliusmarmingeforce-pushed the codex/update-manifest-errors branch from c9aeab5 to 4bd4e6dCompareJune 20, 2026 17:15
@juliusmarminge
juliusmarminge changed the base branch from main to codex/redact-dpop-request-targetJune 20, 2026 17:15
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 17:15

Dismissing prior approval to re-evaluate 4bd4e6d

@juliusmarminge
juliusmarminge merged this pull request into codex/redact-dpop-request-targetJun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/update-manifest-errors branch June 20, 2026 18:33
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

[codex] Structure update manifest failures - #3293

Merged
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors
Jun 20, 2026
Merged

[codex] Structure update manifest failures#3293
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • replace generic update-manifest parse, merge, and serialization exceptions with structured Schema errors
  • retain platform, source location, offending input, and conflicting version/field/file metadata
  • expose known validation failures through the Effect error channel while leaving unexpected exceptions as defects
  • keep semantic validation failures cause-free

Validation

  • vp test run scripts/merge-update-manifests.test.ts (10 tests)
  • vp check
  • vp run typecheck

Sensitive-context follow-up\n- unsupported manifest lines are represented by line number and line length rather than raw contents\n- parse messages no longer echo potentially sensitive line text


Note

Medium Risk
Changes how merge-update-manifest failures are represented (typed Effect errors vs generic throws), which callers must handle; scope is release scripting, not runtime auth, with lower blast radius but a behavioral contract shift.

Overview
Replaces generic Error throws in update manifest parse, merge, and serialize with Effect Schema tagged errors (UpdateManifestParseError, version/extra/file conflict errors, serialization errors). Adds attemptUpdateManifest so the merge CLI surfaces known failures on the Effect error channel while unexpected exceptions still die as defects.

Failure payloads and messages are tightened so logs do not echo sensitive input: unsupported lines report line number and length (not the line text), file URL conflicts use getUrlDiagnostics (length, protocol, hostname only), and extra-field conflicts record types and string lengths instead of raw values. File merge conflicts also record which manifest (primary / secondary) and hash/size metadata without embedding full URLs or sha512 strings.

Adds shared getUrlDiagnostics (exported from @t3tools/shared/urlDiagnostics) and redactDpopRequestTarget in dpop.ts for the same redaction pattern; tests assert neither helper leaks credentials, paths, or query tokens.

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

Note

Replace generic errors with typed structured errors in update manifest parsing and merging

  • Introduces typed error classes (UpdateManifestParseError, UpdateManifestVersionConflictError, UpdateManifestExtraConflictError, UpdateManifestFileConflictError, UpdateManifestSerializationError) using effect/Schema in update-manifest.ts, replacing all generic Error throws.
  • Conflict errors expose only non-sensitive diagnostics (types, lengths, URL protocol/hostname) rather than raw values, using a new getUrlDiagnostics utility in urlDiagnostics.ts and redactDpopRequestTarget in dpop.ts.
  • Adds attemptUpdateManifest helper to wrap manifest operations as Effect failures instead of defects; merge-update-manifests.ts is updated to use it, surfacing errors on the Effect error channel.
  • Behavioral Change: callers of mergeUpdateManifestFiles now receive typed Effect failures instead of thrown exceptions for all parse, merge, and serialization errors.

Macroscope summarized 4bd4e6d.

@coderabbitai

coderabbitaiBot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3cf50f52-e7f8-43c7-a769-0be0fc7f89a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/update-manifest-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Parse message omits offending line
    • Added the offendingLine content to the message getter so it now produces messages like "unsupported line 'not yaml'." when offendingLine is present.
  • ✅ Fixed: File conflict labels wrong manifest
    • Separated the combined file iteration into two explicit loops (primary first, then secondary) so that primarySha512/primarySize are structurally guaranteed to come from the primary manifest's entries for cross-manifest conflicts.

Create PR

Or push these changes by commenting:

@cursor push 3d9418fa45
Preview (3d9418fa45)
diff --git a/scripts/lib/update-manifest.ts b/scripts/lib/update-manifest.ts--- a/scripts/lib/update-manifest.ts+++ b/scripts/lib/update-manifest.ts@@ -36,7 +36,8 @@
override get message(): string {
const location =
this.lineNumber === undefined ? this.sourcePath : `${this.sourcePath}:${this.lineNumber}`;
- return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}.`;+ const suffix = this.offendingLine !== undefined ? ` '${this.offendingLine}'` : "";+ return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}${suffix}.`;
}
}
@@ -361,7 +362,7 @@
}
const filesByUrl = new Map<string, UpdateManifestFile>();
- for (const file of [...primary.files, ...secondary.files]) {+ for (const file of primary.files) {
const existing = filesByUrl.get(file.url);
if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {
throw new UpdateManifestFileConflictError({
@@ -375,6 +376,20 @@
}
filesByUrl.set(file.url, file);
}
+ for (const file of secondary.files) {+ const existing = filesByUrl.get(file.url);+ if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {+ throw new UpdateManifestFileConflictError({+ platformLabel,+ url: file.url,+ primarySha512: existing.sha512,+ primarySize: existing.size,+ secondarySha512: file.sha512,+ secondarySize: file.size,+ });+ }+ filesByUrl.set(file.url, file);+ }
return {
version: primary.version,

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit ac92fa2. Configure here.

Comment threadscripts/lib/update-manifest.ts
Comment threadscripts/lib/update-manifest.ts Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeappBot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Build script refactor converting plain Error throws to structured tagged error classes. Same error conditions, same behavior - just better typed errors. Comprehensive tests verify the new structure and that sensitive data is not leaked.

You can customize Macroscope's approvability policy. Learn more.

@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 14:37

Dismissing prior approval to re-evaluate 9114601

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 16:10

Dismissing prior approval to re-evaluate c9aeab5

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
juliusmarmingeand others added 4 commits June 20, 2026 10:11
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge
juliusmarmingeforce-pushed the codex/update-manifest-errors branch from c9aeab5 to 4bd4e6dCompareJune 20, 2026 17:15
@juliusmarminge
juliusmarminge changed the base branch from main to codex/redact-dpop-request-targetJune 20, 2026 17:15
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 17:15

Dismissing prior approval to re-evaluate 4bd4e6d

@juliusmarminge
juliusmarminge merged this pull request into codex/redact-dpop-request-targetJun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/update-manifest-errors branch June 20, 2026 18:33
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

[codex] Structure update manifest failures - #3293

Merged
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors
Jun 20, 2026
Merged

[codex] Structure update manifest failures#3293
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • replace generic update-manifest parse, merge, and serialization exceptions with structured Schema errors
  • retain platform, source location, offending input, and conflicting version/field/file metadata
  • expose known validation failures through the Effect error channel while leaving unexpected exceptions as defects
  • keep semantic validation failures cause-free

Validation

  • vp test run scripts/merge-update-manifests.test.ts (10 tests)
  • vp check
  • vp run typecheck

Sensitive-context follow-up\n- unsupported manifest lines are represented by line number and line length rather than raw contents\n- parse messages no longer echo potentially sensitive line text


Note

Medium Risk
Changes how merge-update-manifest failures are represented (typed Effect errors vs generic throws), which callers must handle; scope is release scripting, not runtime auth, with lower blast radius but a behavioral contract shift.

Overview
Replaces generic Error throws in update manifest parse, merge, and serialize with Effect Schema tagged errors (UpdateManifestParseError, version/extra/file conflict errors, serialization errors). Adds attemptUpdateManifest so the merge CLI surfaces known failures on the Effect error channel while unexpected exceptions still die as defects.

Failure payloads and messages are tightened so logs do not echo sensitive input: unsupported lines report line number and length (not the line text), file URL conflicts use getUrlDiagnostics (length, protocol, hostname only), and extra-field conflicts record types and string lengths instead of raw values. File merge conflicts also record which manifest (primary / secondary) and hash/size metadata without embedding full URLs or sha512 strings.

Adds shared getUrlDiagnostics (exported from @t3tools/shared/urlDiagnostics) and redactDpopRequestTarget in dpop.ts for the same redaction pattern; tests assert neither helper leaks credentials, paths, or query tokens.

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

Note

Replace generic errors with typed structured errors in update manifest parsing and merging

  • Introduces typed error classes (UpdateManifestParseError, UpdateManifestVersionConflictError, UpdateManifestExtraConflictError, UpdateManifestFileConflictError, UpdateManifestSerializationError) using effect/Schema in update-manifest.ts, replacing all generic Error throws.
  • Conflict errors expose only non-sensitive diagnostics (types, lengths, URL protocol/hostname) rather than raw values, using a new getUrlDiagnostics utility in urlDiagnostics.ts and redactDpopRequestTarget in dpop.ts.
  • Adds attemptUpdateManifest helper to wrap manifest operations as Effect failures instead of defects; merge-update-manifests.ts is updated to use it, surfacing errors on the Effect error channel.
  • Behavioral Change: callers of mergeUpdateManifestFiles now receive typed Effect failures instead of thrown exceptions for all parse, merge, and serialization errors.

Macroscope summarized 4bd4e6d.

@coderabbitai

coderabbitaiBot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3cf50f52-e7f8-43c7-a769-0be0fc7f89a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/update-manifest-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Parse message omits offending line
    • Added the offendingLine content to the message getter so it now produces messages like "unsupported line 'not yaml'." when offendingLine is present.
  • ✅ Fixed: File conflict labels wrong manifest
    • Separated the combined file iteration into two explicit loops (primary first, then secondary) so that primarySha512/primarySize are structurally guaranteed to come from the primary manifest's entries for cross-manifest conflicts.

Create PR

Or push these changes by commenting:

@cursor push 3d9418fa45
Preview (3d9418fa45)
diff --git a/scripts/lib/update-manifest.ts b/scripts/lib/update-manifest.ts--- a/scripts/lib/update-manifest.ts+++ b/scripts/lib/update-manifest.ts@@ -36,7 +36,8 @@
override get message(): string {
const location =
this.lineNumber === undefined ? this.sourcePath : `${this.sourcePath}:${this.lineNumber}`;
- return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}.`;+ const suffix = this.offendingLine !== undefined ? ` '${this.offendingLine}'` : "";+ return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}${suffix}.`;
}
}
@@ -361,7 +362,7 @@
}
const filesByUrl = new Map<string, UpdateManifestFile>();
- for (const file of [...primary.files, ...secondary.files]) {+ for (const file of primary.files) {
const existing = filesByUrl.get(file.url);
if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {
throw new UpdateManifestFileConflictError({
@@ -375,6 +376,20 @@
}
filesByUrl.set(file.url, file);
}
+ for (const file of secondary.files) {+ const existing = filesByUrl.get(file.url);+ if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {+ throw new UpdateManifestFileConflictError({+ platformLabel,+ url: file.url,+ primarySha512: existing.sha512,+ primarySize: existing.size,+ secondarySha512: file.sha512,+ secondarySize: file.size,+ });+ }+ filesByUrl.set(file.url, file);+ }
return {
version: primary.version,

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit ac92fa2. Configure here.

Comment threadscripts/lib/update-manifest.ts
Comment threadscripts/lib/update-manifest.ts Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeappBot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Build script refactor converting plain Error throws to structured tagged error classes. Same error conditions, same behavior - just better typed errors. Comprehensive tests verify the new structure and that sensitive data is not leaked.

You can customize Macroscope's approvability policy. Learn more.

@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 14:37

Dismissing prior approval to re-evaluate 9114601

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 16:10

Dismissing prior approval to re-evaluate c9aeab5

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
juliusmarmingeand others added 4 commits June 20, 2026 10:11
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge
juliusmarmingeforce-pushed the codex/update-manifest-errors branch from c9aeab5 to 4bd4e6dCompareJune 20, 2026 17:15
@juliusmarminge
juliusmarminge changed the base branch from main to codex/redact-dpop-request-targetJune 20, 2026 17:15
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 17:15

Dismissing prior approval to re-evaluate 4bd4e6d

@juliusmarminge
juliusmarminge merged this pull request into codex/redact-dpop-request-targetJun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/update-manifest-errors branch June 20, 2026 18:33
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

[codex] Structure update manifest failures - #3293

Merged
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors
Jun 20, 2026
Merged

[codex] Structure update manifest failures#3293
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • replace generic update-manifest parse, merge, and serialization exceptions with structured Schema errors
  • retain platform, source location, offending input, and conflicting version/field/file metadata
  • expose known validation failures through the Effect error channel while leaving unexpected exceptions as defects
  • keep semantic validation failures cause-free

Validation

  • vp test run scripts/merge-update-manifests.test.ts (10 tests)
  • vp check
  • vp run typecheck

Sensitive-context follow-up\n- unsupported manifest lines are represented by line number and line length rather than raw contents\n- parse messages no longer echo potentially sensitive line text


Note

Medium Risk
Changes how merge-update-manifest failures are represented (typed Effect errors vs generic throws), which callers must handle; scope is release scripting, not runtime auth, with lower blast radius but a behavioral contract shift.

Overview
Replaces generic Error throws in update manifest parse, merge, and serialize with Effect Schema tagged errors (UpdateManifestParseError, version/extra/file conflict errors, serialization errors). Adds attemptUpdateManifest so the merge CLI surfaces known failures on the Effect error channel while unexpected exceptions still die as defects.

Failure payloads and messages are tightened so logs do not echo sensitive input: unsupported lines report line number and length (not the line text), file URL conflicts use getUrlDiagnostics (length, protocol, hostname only), and extra-field conflicts record types and string lengths instead of raw values. File merge conflicts also record which manifest (primary / secondary) and hash/size metadata without embedding full URLs or sha512 strings.

Adds shared getUrlDiagnostics (exported from @t3tools/shared/urlDiagnostics) and redactDpopRequestTarget in dpop.ts for the same redaction pattern; tests assert neither helper leaks credentials, paths, or query tokens.

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

Note

Replace generic errors with typed structured errors in update manifest parsing and merging

  • Introduces typed error classes (UpdateManifestParseError, UpdateManifestVersionConflictError, UpdateManifestExtraConflictError, UpdateManifestFileConflictError, UpdateManifestSerializationError) using effect/Schema in update-manifest.ts, replacing all generic Error throws.
  • Conflict errors expose only non-sensitive diagnostics (types, lengths, URL protocol/hostname) rather than raw values, using a new getUrlDiagnostics utility in urlDiagnostics.ts and redactDpopRequestTarget in dpop.ts.
  • Adds attemptUpdateManifest helper to wrap manifest operations as Effect failures instead of defects; merge-update-manifests.ts is updated to use it, surfacing errors on the Effect error channel.
  • Behavioral Change: callers of mergeUpdateManifestFiles now receive typed Effect failures instead of thrown exceptions for all parse, merge, and serialization errors.

Macroscope summarized 4bd4e6d.

@coderabbitai

coderabbitaiBot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3cf50f52-e7f8-43c7-a769-0be0fc7f89a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/update-manifest-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Parse message omits offending line
    • Added the offendingLine content to the message getter so it now produces messages like "unsupported line 'not yaml'." when offendingLine is present.
  • ✅ Fixed: File conflict labels wrong manifest
    • Separated the combined file iteration into two explicit loops (primary first, then secondary) so that primarySha512/primarySize are structurally guaranteed to come from the primary manifest's entries for cross-manifest conflicts.

Create PR

Or push these changes by commenting:

@cursor push 3d9418fa45
Preview (3d9418fa45)
diff --git a/scripts/lib/update-manifest.ts b/scripts/lib/update-manifest.ts--- a/scripts/lib/update-manifest.ts+++ b/scripts/lib/update-manifest.ts@@ -36,7 +36,8 @@
override get message(): string {
const location =
this.lineNumber === undefined ? this.sourcePath : `${this.sourcePath}:${this.lineNumber}`;
- return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}.`;+ const suffix = this.offendingLine !== undefined ? ` '${this.offendingLine}'` : "";+ return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}${suffix}.`;
}
}
@@ -361,7 +362,7 @@
}
const filesByUrl = new Map<string, UpdateManifestFile>();
- for (const file of [...primary.files, ...secondary.files]) {+ for (const file of primary.files) {
const existing = filesByUrl.get(file.url);
if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {
throw new UpdateManifestFileConflictError({
@@ -375,6 +376,20 @@
}
filesByUrl.set(file.url, file);
}
+ for (const file of secondary.files) {+ const existing = filesByUrl.get(file.url);+ if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {+ throw new UpdateManifestFileConflictError({+ platformLabel,+ url: file.url,+ primarySha512: existing.sha512,+ primarySize: existing.size,+ secondarySha512: file.sha512,+ secondarySize: file.size,+ });+ }+ filesByUrl.set(file.url, file);+ }
return {
version: primary.version,

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit ac92fa2. Configure here.

Comment threadscripts/lib/update-manifest.ts
Comment threadscripts/lib/update-manifest.ts Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeappBot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Build script refactor converting plain Error throws to structured tagged error classes. Same error conditions, same behavior - just better typed errors. Comprehensive tests verify the new structure and that sensitive data is not leaked.

You can customize Macroscope's approvability policy. Learn more.

@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 14:37

Dismissing prior approval to re-evaluate 9114601

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 16:10

Dismissing prior approval to re-evaluate c9aeab5

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
juliusmarmingeand others added 4 commits June 20, 2026 10:11
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge
juliusmarmingeforce-pushed the codex/update-manifest-errors branch from c9aeab5 to 4bd4e6dCompareJune 20, 2026 17:15
@juliusmarminge
juliusmarminge changed the base branch from main to codex/redact-dpop-request-targetJune 20, 2026 17:15
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 17:15

Dismissing prior approval to re-evaluate 4bd4e6d

@juliusmarminge
juliusmarminge merged this pull request into codex/redact-dpop-request-targetJun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/update-manifest-errors branch June 20, 2026 18:33
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

[codex] Structure update manifest failures - #3293

Merged
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors
Jun 20, 2026
Merged

[codex] Structure update manifest failures#3293
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • replace generic update-manifest parse, merge, and serialization exceptions with structured Schema errors
  • retain platform, source location, offending input, and conflicting version/field/file metadata
  • expose known validation failures through the Effect error channel while leaving unexpected exceptions as defects
  • keep semantic validation failures cause-free

Validation

  • vp test run scripts/merge-update-manifests.test.ts (10 tests)
  • vp check
  • vp run typecheck

Sensitive-context follow-up\n- unsupported manifest lines are represented by line number and line length rather than raw contents\n- parse messages no longer echo potentially sensitive line text


Note

Medium Risk
Changes how merge-update-manifest failures are represented (typed Effect errors vs generic throws), which callers must handle; scope is release scripting, not runtime auth, with lower blast radius but a behavioral contract shift.

Overview
Replaces generic Error throws in update manifest parse, merge, and serialize with Effect Schema tagged errors (UpdateManifestParseError, version/extra/file conflict errors, serialization errors). Adds attemptUpdateManifest so the merge CLI surfaces known failures on the Effect error channel while unexpected exceptions still die as defects.

Failure payloads and messages are tightened so logs do not echo sensitive input: unsupported lines report line number and length (not the line text), file URL conflicts use getUrlDiagnostics (length, protocol, hostname only), and extra-field conflicts record types and string lengths instead of raw values. File merge conflicts also record which manifest (primary / secondary) and hash/size metadata without embedding full URLs or sha512 strings.

Adds shared getUrlDiagnostics (exported from @t3tools/shared/urlDiagnostics) and redactDpopRequestTarget in dpop.ts for the same redaction pattern; tests assert neither helper leaks credentials, paths, or query tokens.

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

Note

Replace generic errors with typed structured errors in update manifest parsing and merging

  • Introduces typed error classes (UpdateManifestParseError, UpdateManifestVersionConflictError, UpdateManifestExtraConflictError, UpdateManifestFileConflictError, UpdateManifestSerializationError) using effect/Schema in update-manifest.ts, replacing all generic Error throws.
  • Conflict errors expose only non-sensitive diagnostics (types, lengths, URL protocol/hostname) rather than raw values, using a new getUrlDiagnostics utility in urlDiagnostics.ts and redactDpopRequestTarget in dpop.ts.
  • Adds attemptUpdateManifest helper to wrap manifest operations as Effect failures instead of defects; merge-update-manifests.ts is updated to use it, surfacing errors on the Effect error channel.
  • Behavioral Change: callers of mergeUpdateManifestFiles now receive typed Effect failures instead of thrown exceptions for all parse, merge, and serialization errors.

Macroscope summarized 4bd4e6d.

@coderabbitai

coderabbitaiBot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3cf50f52-e7f8-43c7-a769-0be0fc7f89a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/update-manifest-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Parse message omits offending line
    • Added the offendingLine content to the message getter so it now produces messages like "unsupported line 'not yaml'." when offendingLine is present.
  • ✅ Fixed: File conflict labels wrong manifest
    • Separated the combined file iteration into two explicit loops (primary first, then secondary) so that primarySha512/primarySize are structurally guaranteed to come from the primary manifest's entries for cross-manifest conflicts.

Create PR

Or push these changes by commenting:

@cursor push 3d9418fa45
Preview (3d9418fa45)
diff --git a/scripts/lib/update-manifest.ts b/scripts/lib/update-manifest.ts--- a/scripts/lib/update-manifest.ts+++ b/scripts/lib/update-manifest.ts@@ -36,7 +36,8 @@
override get message(): string {
const location =
this.lineNumber === undefined ? this.sourcePath : `${this.sourcePath}:${this.lineNumber}`;
- return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}.`;+ const suffix = this.offendingLine !== undefined ? ` '${this.offendingLine}'` : "";+ return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}${suffix}.`;
}
}
@@ -361,7 +362,7 @@
}
const filesByUrl = new Map<string, UpdateManifestFile>();
- for (const file of [...primary.files, ...secondary.files]) {+ for (const file of primary.files) {
const existing = filesByUrl.get(file.url);
if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {
throw new UpdateManifestFileConflictError({
@@ -375,6 +376,20 @@
}
filesByUrl.set(file.url, file);
}
+ for (const file of secondary.files) {+ const existing = filesByUrl.get(file.url);+ if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {+ throw new UpdateManifestFileConflictError({+ platformLabel,+ url: file.url,+ primarySha512: existing.sha512,+ primarySize: existing.size,+ secondarySha512: file.sha512,+ secondarySize: file.size,+ });+ }+ filesByUrl.set(file.url, file);+ }
return {
version: primary.version,

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit ac92fa2. Configure here.

Comment threadscripts/lib/update-manifest.ts
Comment threadscripts/lib/update-manifest.ts Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeappBot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Build script refactor converting plain Error throws to structured tagged error classes. Same error conditions, same behavior - just better typed errors. Comprehensive tests verify the new structure and that sensitive data is not leaked.

You can customize Macroscope's approvability policy. Learn more.

@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 14:37

Dismissing prior approval to re-evaluate 9114601

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 16:10

Dismissing prior approval to re-evaluate c9aeab5

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
juliusmarmingeand others added 4 commits June 20, 2026 10:11
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge
juliusmarmingeforce-pushed the codex/update-manifest-errors branch from c9aeab5 to 4bd4e6dCompareJune 20, 2026 17:15
@juliusmarminge
juliusmarminge changed the base branch from main to codex/redact-dpop-request-targetJune 20, 2026 17:15
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 17:15

Dismissing prior approval to re-evaluate 4bd4e6d

@juliusmarminge
juliusmarminge merged this pull request into codex/redact-dpop-request-targetJun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/update-manifest-errors branch June 20, 2026 18:33
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

[codex] Structure update manifest failures - #3293

Merged
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors
Jun 20, 2026
Merged

[codex] Structure update manifest failures#3293
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/update-manifest-errors

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • replace generic update-manifest parse, merge, and serialization exceptions with structured Schema errors
  • retain platform, source location, offending input, and conflicting version/field/file metadata
  • expose known validation failures through the Effect error channel while leaving unexpected exceptions as defects
  • keep semantic validation failures cause-free

Validation

  • vp test run scripts/merge-update-manifests.test.ts (10 tests)
  • vp check
  • vp run typecheck

Sensitive-context follow-up\n- unsupported manifest lines are represented by line number and line length rather than raw contents\n- parse messages no longer echo potentially sensitive line text


Note

Medium Risk
Changes how merge-update-manifest failures are represented (typed Effect errors vs generic throws), which callers must handle; scope is release scripting, not runtime auth, with lower blast radius but a behavioral contract shift.

Overview
Replaces generic Error throws in update manifest parse, merge, and serialize with Effect Schema tagged errors (UpdateManifestParseError, version/extra/file conflict errors, serialization errors). Adds attemptUpdateManifest so the merge CLI surfaces known failures on the Effect error channel while unexpected exceptions still die as defects.

Failure payloads and messages are tightened so logs do not echo sensitive input: unsupported lines report line number and length (not the line text), file URL conflicts use getUrlDiagnostics (length, protocol, hostname only), and extra-field conflicts record types and string lengths instead of raw values. File merge conflicts also record which manifest (primary / secondary) and hash/size metadata without embedding full URLs or sha512 strings.

Adds shared getUrlDiagnostics (exported from @t3tools/shared/urlDiagnostics) and redactDpopRequestTarget in dpop.ts for the same redaction pattern; tests assert neither helper leaks credentials, paths, or query tokens.

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

Note

Replace generic errors with typed structured errors in update manifest parsing and merging

  • Introduces typed error classes (UpdateManifestParseError, UpdateManifestVersionConflictError, UpdateManifestExtraConflictError, UpdateManifestFileConflictError, UpdateManifestSerializationError) using effect/Schema in update-manifest.ts, replacing all generic Error throws.
  • Conflict errors expose only non-sensitive diagnostics (types, lengths, URL protocol/hostname) rather than raw values, using a new getUrlDiagnostics utility in urlDiagnostics.ts and redactDpopRequestTarget in dpop.ts.
  • Adds attemptUpdateManifest helper to wrap manifest operations as Effect failures instead of defects; merge-update-manifests.ts is updated to use it, surfacing errors on the Effect error channel.
  • Behavioral Change: callers of mergeUpdateManifestFiles now receive typed Effect failures instead of thrown exceptions for all parse, merge, and serialization errors.

Macroscope summarized 4bd4e6d.

@coderabbitai

coderabbitaiBot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3cf50f52-e7f8-43c7-a769-0be0fc7f89a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/update-manifest-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Parse message omits offending line
    • Added the offendingLine content to the message getter so it now produces messages like "unsupported line 'not yaml'." when offendingLine is present.
  • ✅ Fixed: File conflict labels wrong manifest
    • Separated the combined file iteration into two explicit loops (primary first, then secondary) so that primarySha512/primarySize are structurally guaranteed to come from the primary manifest's entries for cross-manifest conflicts.

Create PR

Or push these changes by commenting:

@cursor push 3d9418fa45
Preview (3d9418fa45)
diff --git a/scripts/lib/update-manifest.ts b/scripts/lib/update-manifest.ts--- a/scripts/lib/update-manifest.ts+++ b/scripts/lib/update-manifest.ts@@ -36,7 +36,8 @@
override get message(): string {
const location =
this.lineNumber === undefined ? this.sourcePath : `${this.sourcePath}:${this.lineNumber}`;
- return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}.`;+ const suffix = this.offendingLine !== undefined ? ` '${this.offendingLine}'` : "";+ return `Invalid ${this.platformLabel} update manifest at ${location}: ${this.reason}${suffix}.`;
}
}
@@ -361,7 +362,7 @@
}
const filesByUrl = new Map<string, UpdateManifestFile>();
- for (const file of [...primary.files, ...secondary.files]) {+ for (const file of primary.files) {
const existing = filesByUrl.get(file.url);
if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {
throw new UpdateManifestFileConflictError({
@@ -375,6 +376,20 @@
}
filesByUrl.set(file.url, file);
}
+ for (const file of secondary.files) {+ const existing = filesByUrl.get(file.url);+ if (existing && (existing.sha512 !== file.sha512 || existing.size !== file.size)) {+ throw new UpdateManifestFileConflictError({+ platformLabel,+ url: file.url,+ primarySha512: existing.sha512,+ primarySize: existing.size,+ secondarySha512: file.sha512,+ secondarySize: file.size,+ });+ }+ filesByUrl.set(file.url, file);+ }
return {
version: primary.version,

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit ac92fa2. Configure here.

Comment threadscripts/lib/update-manifest.ts
Comment threadscripts/lib/update-manifest.ts Outdated
macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeappBot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Build script refactor converting plain Error throws to structured tagged error classes. Same error conditions, same behavior - just better typed errors. Comprehensive tests verify the new structure and that sensitive data is not leaked.

You can customize Macroscope's approvability policy. Learn more.

@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 14:37

Dismissing prior approval to re-evaluate 9114601

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 16:10

Dismissing prior approval to re-evaluate c9aeab5

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jun 20, 2026
juliusmarmingeand others added 4 commits June 20, 2026 10:11
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge
juliusmarmingeforce-pushed the codex/update-manifest-errors branch from c9aeab5 to 4bd4e6dCompareJune 20, 2026 17:15
@juliusmarminge
juliusmarminge changed the base branch from main to codex/redact-dpop-request-targetJune 20, 2026 17:15
@macroscopeapp
macroscopeappBot dismissed their stale reviewJune 20, 2026 17:15

Dismissing prior approval to re-evaluate 4bd4e6d

@juliusmarminge
juliusmarminge merged this pull request into codex/redact-dpop-request-targetJun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/update-manifest-errors branch June 20, 2026 18:33
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge