Skip to content

fix(images): harden staged artifact recovery - #44

Merged
sambitcreate merged 3 commits into
mainfrom
feature/fix-image-artifact-review
Aug 25, 2026
Merged

sambitcreate merged 3 commits into
mainfrom
feature/fix-image-artifact-review

Conversation

@sambitcreate

Copy link
Copy Markdown
Owner

Summary

Addresses the remaining actionable review findings from #36.

  • degrades unreadable/unsafe artifact staging without aborting application startup
  • keeps chat mutations blocked when staging is unavailable
  • catches recovery-limit failures while retaining staged payloads for repair
  • computes pending usage from cached metadata without cloning base64 image payloads

Verification

  • npx tsx --test main/services/display-image-artifact-store.test.ts — 12 passed
  • npm run type-check
  • npm run lint
  • git diff --check

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important

The degradation logic is correct for its stated intent, but the fail-closed state has no recovery path and its block is app-wide with a message that promises a fix no restart can deliver.

Reviewed changes

  • Degrades staging failures instead of aborting startup — initialize() records an unavailableReason for corrupt or unsafe files rather than throwing, so a bad display-image-artifacts.json no longer crashes the app.
  • Fails closed on every mutator — stage/remove/deleteChat/pending/recover now call requireAvailable(); startup recovery is guarded by availability() and wrapped in a warn-level try/catch.
  • hasPending short-circuits to true when unavailable — keeps sends/copies/exports blocked without reading the file.
  • Stops cloning staged payloads for usage projection — usageByChat reads cached records directly instead of structuredClone-ing the whole store via pending().

⚠️ Degraded state has no recovery path

When the staging file is unreadable or has an unsupported shape, the app now stays up but blocks send/copy/export/delete for every chat, and nothing in the UI lets a user clear or repair display-image-artifacts.json. "Retains staged payloads for repair" describes the on-disk data, not an actual repair mechanism — a restart cannot clear a corrupt file, so a user who hits this is stranded until they locate and delete the file by hand.

Technical details
# No recovery path for the degraded staging state

## Affected sites
- main/services/display-image-artifact-store.ts:332 — `hasPending` returns `true` for all chats when unavailable
- main/handlers/chats.ts:247/332/357/466/572 — send/copy/export/delete all blocked with "Restart Aiden to recover…"

## Required outcome
- A user with a corrupt/unsafe staging file must have some way to recover, or the block messages must not promise a restart that cannot succeed.

## Open questions for the human
- Is a manual delete of `display-image-artifacts.json` the intended recovery path? If so, should the app surface it (e.g. a "Reset image staging" action), or at least log the exact path?
- Is the app-wide fail-closed scope (blocking chats that never had a staged image) the accepted tradeoff, or should the block be narrower?

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Pro | 𝕏

Comment thread main/services/display-image-artifact-store.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ No critical issues — the prior review's findings are addressed cleanly. One minor residual on the degraded-state messaging below.

Reviewed changes

  • Quarantines invalid staging instead of failing closed app-wide — initialize() renames a corrupt/unsafe display-image-artifacts.json aside to <file>.invalid-<stamp>-<uuid> and opens a clean store, so the unreadable-file case no longer blocks every chat.
  • Rewords block messages from "Restart Aiden" to "Delete this chat to discard it" — across chats.ts, llm-client.ts, and chat-pane.tsx, matching the now-truthful per-chat recovery path.
  • Logs the preserved path — main/index.ts warns with the exact .invalid-* path and reports degraded availability.
  • Adds quarantine coverage — corrupt and unsupported-shape tests assert availability, non-blocking hasPending, and preserved bytes.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Pro | 𝕏

Comment thread main/handlers/chats.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • Added a store-wide imageArtifactRecoveryUnavailable flag to the chat read contract — chats:get reports availability() and short-circuits hasPending to false when staging is unavailable, keeping per-chat pending distinct from store-wide unavailability.
  • Branched copy/export/send block messages on availability() — the unavailable state now directs the user to the developer log (${availability.reason} Open Aiden's developer log to locate the staging file that needs repair.) instead of prescribing "Delete this chat to discard it", which deleteChat refuses in that state.
  • Gated the display-image generation path and the renderer composer/copy on the unavailable flag — prepareGeneration throws the repair message, and ChatPane disables the composer and copy path with the same truthful copy.
  • Reworded the quarantine-failure reason to name the exact source and preserved paths — Aiden could not move ${source} to ${preserved} instead of the vague "could not preserve it".

The prior ℹ️ thread (quarantine-failure message prescribing an impossible deletion) is now resolved by this distinction, and the renderer/main block messages agree across all four mutation paths.

Pullfrog  | View workflow run | Using DeepSeek Pro | 𝕏

@sambitcreate
sambitcreate merged commit 51cda88 into main Aug 25, 2026
3 checks passed
@sambitcreate
sambitcreate deleted the feature/fix-image-artifact-review branch August 25, 2026 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant