Skip to content

fix(contracts): tolerate an unreadable snap-shot source on persisted image attachments - #10795

Closed
Bil0000 wants to merge 1 commit into
pingdotgg:mainfrom
Bil0000:fix/persisted-snap-shot-source
Closed

Bil0000 wants to merge 1 commit into
pingdotgg:mainfrom
Bil0000:fix/persisted-snap-shot-source

Conversation

@Bil0000

@Bil0000 Bil0000 commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

What Changed

ChatImageAttachment.source now decodes tolerantly on persisted events. When a stored snap-shot source does not match the current SnapShotSource schema, the decoder drops the source and keeps the image attachment instead of failing the whole message. Upload schemas keep the strict source validation.

One regression test added in packages/contracts/src/orchestration.test.ts.

Why

Starting the desktop app (pnpm dev:desktop) against an existing database crashed the backend in a restart loop:

PersistenceDecodeError: Decode error in OrchestrationEventStore.readFromSequence:rowToEvent
  Expected { readonly "kind": "snap-shot", ... } | undefined
    at ["payload"]["attachments"][0]["source"]

An image attachment written by an earlier build carried a source that today's SnapShotSource rejects. Because ChatAttachment is a union and the unknown-attachment member deliberately excludes type: "image", there was no fallback, so one old row made the whole event store unreadable and locked the user out of all their threads.

The source is provenance for the image, not the image. Losing it on an old row is harmless; refusing to start is not.

Verification

  • vp test run packages/contracts/src/orchestration.test.ts: 57 passed, including the new case.
  • tsc --noEmit clean for packages/contracts, apps/web, and apps/desktop.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes (n/a, no UI change)
  • I included a video for animation/interaction changes (n/a)

Written by Claude Fable 5.1 in Claude Code.

Summary by CodeRabbit

  • Bug Fixes
    • Improved compatibility when loading chat messages containing snapshot image sources the current version cannot read.
    • Unreadable snapshot sources are now omitted while the rest of the message remains available.

…image attachments

A persisted image attachment whose `source` does not match the current
SnapShotSource schema (written by an older or newer build, or by a field
that later gained tighter bounds) failed the whole event decode. The
desktop server then crashed on start with a PersistenceDecodeError in
OrchestrationEventStore.readFromSequence and restarted in a loop, locking
the user out of their data.

The source is provenance for the image, not the image itself, so the
decoder now drops an unreadable source and keeps the attachment. Uploads
keep the strict schema so a new capture is still validated in full.

Written by Claude Fable 5.1 in Claude Code.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 8, 2026

@macroscopeapp macroscopeapp Bot 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.

All clear

Posted via Macroscope — Effect Service Conventions

@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at b43c240

Macroscope's review found this PR approvable — This is a narrowly scoped compatibility fix that preserves valid image attachments while discarding only unreadable optional snapshot provenance. Required attachment validation and strict upload handling remain unchanged, with focused regression coverage added.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c4ab0283-87fd-4807-a098-f34f4c1604cf

📥 Commits

Reviewing files that changed from the base of the PR and between 061543e and b43c240.

📒 Files selected for processing (2)
  • packages/contracts/src/orchestration.test.ts
  • packages/contracts/src/orchestration.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Persisted image attachments now tolerate unreadable snapshot sources. The decoder drops an unsupported source instead of rejecting the orchestration message. A test covers an unknown window-capture source.

Changes

Snapshot source decoding

Layer / File(s) Summary
Persisted source fallback
packages/contracts/src/orchestration.ts, packages/contracts/src/orchestration.test.ts
PersistedSnapShotSource catches snapshot source decoding failures and produces an absent source. ChatImageAttachment.source uses this schema. The test verifies that an unknown source does not prevent message decoding.

Priority: ➖ Normal — Schedule the persisted attachment decoding change because unreadable snapshot sources can otherwise trigger message decode crashes and desktop backend restart loops.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to b43c2

Persisted image attachments with obsolete or invalid snapshot sources now remain readable with the source omitted, preventing message decode failures while preserving strict validation for uploads. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: persisted image attachments now tolerate unreadable snap-shot sources.
Description check ✅ Passed The description explains what changed, why it was needed, and how it was verified. It includes the required focused-change and UI checklist items. The UI Changes section is omitted appropriately becau…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR 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