Repository navigation
Conversation
Store repeated structured image data and proven JSON mirrors through a reversible binary storage envelope so recovery files keep each image once without changing delivered messages. Reject corrupt references explicitly and require queued session appends to drain before ID-based history mutations.
Keep large retained image histories inside the Host during deletion and pending-prompt retraction instead of uploading an entire transcript. Verify durable turn ownership before retracting a stopped prompt and preserve revision archives and surviving message attribution. Allow revision activation to require its canonical Host prefix while retaining the existing explicit-prefix RPC behavior for older callers.
Normal transcript mutations were uploading retained tool images to Host, which could exceed the fixed request limit. Address durable rows by ID behind strict outbox drain and rewrite fencing while preserving legacy replacement and explicit revision-prefix contracts. Restore stopped drafts only after Host confirms retraction, and preserve newer prompts, session caches, and late callbacks across awaited results. Reject restoration failures before runtime admission so corrupt history cannot silently start a prompt with empty context.
The new local history commands must preserve the same read-only boundary\nas the snapshot operations they replace. Cover keyboard and service calls\nthat bypass disabled remote controls.
Host mutation serialization left renderer history reads free to overlap. A second delete could reload the pre-delete transcript and resurrect an already removed turn even when both durable deletions succeeded. Queue each session's full read, optimistic update, and result settlement before starting the next delete. Release the queue on every exit and recheck the active session so failures and session switches cannot hold or redirect queued work. Deferred regressions cover both first-request outcomes while retaining unrelated late transcript callbacks.
Protect pending target-session appends before awaiting an active flush so permission, foreign-key, and retry-exhaustion failures cannot masquerade as durable writes. Keep normal flush disposal unchanged and observe startup load rejection immediately while preserving fail-closed errors for later callers.
Nested result mirrors otherwise retain escaped copies of image payloads inside their enclosing JSON strings. Store proven canonical mirror tokens as text dependencies while preserving their original source spelling. Resolve dependencies before restoring business values, rejecting invalid indices, cycles and references outside proven mirror tokens explicitly.
Screenshot histories duplicate binary strings across structured results and exact JSON mirrors. Store them once per record and restore complete business values before consumers see them, including nested mirrors. Convert legacy files atomically at first write, retain storage markers, and keep structured paths and proven mirrors consistent during profile relocation. A backed-up version gate prevents older apps from silently misreading the new representation. Exercise real large histories, bounded mutations, restarts and relocation without raising the transport limit or hiding failed persistence. Fixes vastsa#1868
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical storage-validation and moderate persistence/state-consistency findings must be addressed before approval.
2 open issues
What changed in this PR
This pull request reduces screenshot-heavy session history size through lossless per-record image deduplication while preserving consumer-visible values, and replaces large history rewrites with bounded Host-owned mutations.
Changes:
- Adds synchronized Rust and TypeScript storage codecs for transcripts, revisions, checkpoints, compaction tails, outbox records, and relocated profiles.
- Adds schema v26 migration, legacy conversion, corruption handling, relocation support, and strict outbox recovery.
- Adds bounded delete, retract, and canonical revision operations with IPC updates, localization, documentation, and extensive unit, integration, and E2E coverage.
Review findings:
- Critical, 1 vote —
crates/host-core/src/transcripts/binary_migration.rs: Existing schema-2 records are not validated before a subsequent write, allowing corrupted storage to accept a mutation and fail only on later reads. - Moderate, 1 vote —
apps/desktop/electron/main/persistence-outbox.ts: Retained dead-letter entries are not considered when preparing a session mutation, allowing later replay to resurrect content. - Moderate, 1 vote —
apps/desktop/src/stores/slices/transcript-slice.ts: Optimistic deletion does not reconcile the renderer state when the Host reports that the target was already absent. - Moderate, 1 vote —
crates/host-core/src/transcripts/binary_storage.rs:MirrorPartaccepts unknown fields in Rust while the TypeScript decoder rejects them, creating inconsistent corruption handling. - Nit, 4 votes —
docs/spec/03-runtime/04-data-storage.md: The documented owner ofdiscovery_stateconflicts with the implemented migration history.
| File | Summary |
|---|---|
scripts/e2e-tool-result-persistence.mjs |
End-to-end coverage for deduplicated image persistence, mutations, revisions, forks, and relocation. |
packages/shared/src/protocol.ts |
Adds IPC channels for bounded delete and prompt-retraction operations. |
packages/i18n/src/locales/zh-TW/index.ts |
Adds Traditional Chinese prompt-retraction failure text. |
packages/i18n/src/locales/zh-CN/index.ts |
Adds Simplified Chinese prompt-retraction failure text. |
packages/i18n/src/locales/tr/index.ts |
Adds Turkish prompt-retraction failure text. |
packages/i18n/src/locales/pt-BR/index.ts |
Adds Brazilian Portuguese prompt-retraction failure text. |
packages/i18n/src/locales/ko/index.ts |
Adds Korean prompt-retraction failure text. |
packages/i18n/src/locales/fr/index.ts |
Adds French prompt-retraction failure text. |
packages/i18n/src/locales/es/index.ts |
Adds Spanish prompt-retraction failure text. |
packages/i18n/src/locales/en/index.ts |
Adds English prompt-retraction failure text. |
packages/i18n/src/locales/de/index.ts |
Adds German prompt-retraction failure text. |
packages/host-runtime/src/index.ts |
Exports the shared binary-storage codec. |
packages/host-runtime/src/host-request-limit.test.ts |
Tests oversized request diagnostics and no-write/no-retry behavior. |
packages/host-runtime/src/host-process.ts |
Adds request byte diagnostics and recovery guidance for the 64 MiB limit. |
packages/host-runtime/src/fixtures/binary-storage.json |
Provides the primary lossless storage golden fixture. |
packages/host-runtime/src/fixtures/binary-storage-nested.json |
Provides nested-mirror and exact-escaping fixtures. |
packages/host-runtime/src/binary-storage.ts |
Implements reversible image and JSON-mirror deduplication. |
packages/host-runtime/src/binary-storage.test.ts |
Tests codec round trips, corruption handling, and nested mirrors. |
packages/agent-runtime/src/sidecar.ts |
Makes persisted-history restoration failures explicit. |
package.json |
Adds the tool-result persistence E2E command. |
docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md |
Documents the persistence E2E scenario in Chinese. |
docs/zh-CN/spec/03-runtime/06-host-rpc-protocol.md |
Documents bounded history mutation behavior in Chinese. |
docs/zh-CN/spec/03-runtime/04-data-storage.md |
Documents schema v26 and lossless storage behavior in Chinese. |
docs/spec/06-delivery/04-e2e-test-plan.md |
Documents the persistence E2E scenario. |
docs/spec/03-runtime/06-host-rpc-protocol.md |
Documents bounded mutation RPC contracts. |
docs/spec/03-runtime/04-data-storage.md |
Documents the storage envelope and migration behavior. |
docs/adr/README.md |
Registers the lossless image-storage ADR. |
docs/adr/lossless-tool-image-storage.md |
Records the storage and bounded-mutation architecture decision. |
crates/host-core/src/turn_queue.rs |
Updates the schema-version assertion. |
crates/host-core/src/transcripts/binary_storage.rs |
Implements the Rust lossless storage codec and relocation support. |
crates/host-core/src/transcripts/binary_storage_tests.rs |
Tests Rust codec behavior, migration, and relocation cases. |
crates/host-core/src/transcripts/binary_migration.rs |
Converts legacy transcript files atomically on write. |
crates/host-core/src/transcripts.rs |
Integrates encoding, decoding, validation, and migration into persistence. |
crates/host-core/src/sessions/history_mutations/tests.rs |
Tests bounded delete, retract, and canonical revision behavior. |
crates/host-core/src/sessions/history_mutations.rs |
Adds Host-owned ID-based history mutations. |
crates/host-core/src/sessions.rs |
Exposes canonical revision activation and mutation operations. |
crates/host-core/src/rpc/mod.rs |
Adds bounded mutation RPC handlers and canonical-prefix support. |
crates/host-core/src/plugins/providers/tests.rs |
Updates the current schema-version assertion. |
crates/host-core/src/db/tool_image_storage_migration.rs |
Adds the version-only schema v25-to-v26 migration and backup tests. |
crates/host-core/src/db/repositories.rs |
Registers schema v26 migration sequencing. |
crates/host-core/src/db.rs |
Raises the database schema version and registers the migration module. |
crates/host-core/src/data_relocation/tests.rs |
Tests lossless relocation of encoded records and outbox data. |
crates/host-core/src/data_relocation.rs |
Relocates structured paths while preserving encoded mirrors and images. |
apps/desktop/test/transcript-style.test.mjs |
Updates source-level expectations for ID-based mutations. |
apps/desktop/test/sidecar-history-restore.test.mjs |
Tests that restore failures prevent clean prompt admission. |
apps/desktop/test/persistence-outbox.test.mjs |
Tests encoded outbox data, strict drains, retries, and corruption handling. |
apps/desktop/test/helpers/transcript-rewrite-harness.mjs |
Provides test wiring for mutation IPC handlers. |
apps/desktop/test/bounded-history-mutations.test.mjs |
Tests renderer, preload, IPC, mutation races, and bounded payloads. |
apps/desktop/src/stores/slices/transcript-slice.ts |
Uses serialized ID mutations and preserves drafts on failed retraction. |
apps/desktop/src/lib/remote-ipc-safety.ts |
Restricts new mutations to local sessions. |
apps/desktop/src/lib/api.ts |
Adds API wrappers and canonical revision parameters. |
apps/desktop/electron/main/persistence-outbox.ts |
Encodes outbox records and implements strict mutation draining. |
apps/desktop/electron/main/ipc/session-ipc.ts |
Registers bounded mutation handlers and rewrite-fence integration. |
Review effort: Lite
Comment on lines
+50
to
+51
| if current_header(&value)? { | ||
| return Ok(false); |
| bounded display list, not relational data. | ||
|
|
||
| Schema v25 adds nullable `discovery_state`, distinct from metadata deprecation. | ||
| Schema v26 adds nullable `discovery_state`, distinct from metadata deprecation. |
A missing durable message is an acknowledged no-op, so it must not commit the renderer's guessed removal of an entire user turn. Restore the affected projection and cache while retaining messages and terminal updates received during the request, including after a session switch. Verify quarantined recovery entries remain manual-only across restart and mutation preparation without blocking unrelated history mutations.
Reject malformed encoded records before append or update changes the transcript or derived index. Cache only decoder-verified file generations and advance the proof after an owned append is synced and closed. Keep historical malformed plain rows skippable during legacy conversion, invalidate proofs on rewrites and deletion, and retain constant-time normal appends with generation-aware regression coverage.
Prove that existing current-format corruption refuses both append and update without changing files, and that nested mirror validation agrees across codecs. Exercise metadata restoration with writable Windows test handles instead of skipping the identity check. Keep legacy conversion validation explicit without a new lint warning, and correct the unrelated schema number changed by the storage bump.
Snapshot rollback can resurrect a provisional native row after a terminal event gives it a durable ID. Preserve the event-processed projection until Host confirms deletion, then update its cache and retained panes from the current messages, including when the runtime LRU entry was evicted. Cover missing targets, failures, late re-keys, newer turns, session switches, retained panes, and cache eviction without uploading history.
Chromium background CONNECT requests can reach the test proxy and throw its localhost assertion outside the async runner, leaving Electron alive until the deadline. Deny non-fixture destinations with 502 and fail uncaught fixture assertions immediately instead of hiding the failure. Inject an unrelated CONNECT request as a deterministic regression while retaining provider authentication, certificate, redirect, cancellation, and proxy assertions with the original deadlines.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Screenshot-heavy sessions persist the same image in structured content, details,
and JSON mirrors. Store recognized binary strings once per record, reconstruct
all original values before consumers see them, and send IDs for normal history
mutations instead of uploading retained images.
Fixes #1868.
Changes
checkpoints, compaction tails and the Desktop outbox. Preserve original image
bytes, metadata and exact mirror text, including nested mirrors.
first write. Mark converted headerless/empty files once. Invalid storage and
failed writes remain explicit errors.
index. Cache only decoder-verified file generations; detect replacement files
and same-size rewrites, and invalidate proofs on rewrite/deletion. Consecutive
owned appends do not repeatedly scan the full transcript.
existing RPC forms, plugin inputs, rewrite fences, drafts and pending writes.
Serialize consecutive deletes and retain outbox entries when a drain fails.
Publish deletion only after Host acknowledgment, preserving native terminal
ID replacements, newer messages and retained panes across session switches
and runtime cache eviction.
memory identities consistent with the repository's canonical read paths.
with method/byte diagnostics and recovery guidance, without logging contents.
Validation
full Desktop 4,469 passed / 3 existing skips; Host-runtime 155 passed;
affected Desktop/outbox tests 102 passed. No failures in these runs.
and production IPC/outbox handlers through the existing test harness. This
data-path harness does not launch the full Desktop GUI.
rejected; the stored transcript is 24,843,724 bytes with one copy per image.
Restart restores every image, detail field and exact text.
regenerate, revision activation, fork and restart preserve retained values;
ID mutation requests are at most 182 bytes. Three-level nested results also
survive real offline profile relocation and Rust/TypeScript decoding.
targets, native terminal ID re-keys, session switches, retained panes and LRU
eviction have regression coverage. Quarantined outbox entries remain manual
recovery only; both codecs reject unknown mirror-part fields.
transcript/session checks also passed. Evidence:
https://github.com/Seraph-x/PI-Desktop/actions/runs/40367995584
traffic hitting the fixture's localhost assertion outside the async runner.
The test proxy now rejects non-fixture destinations with 502 and uncaught
fixture assertions fail immediately. A deterministic injected CONNECT guards
this case. Two Linux batches of 10 consecutive real Electron runs and three
local runs passed; authentication, certificate, redirect, proxy, cancellation
assertions and deadlines remain unchanged. Production proxy behavior and
upstream workflow configuration are unchanged.
documentation checks passed. Clippy retains three existing warnings.
Independent review findings were fixed and rechecked.
Compatibility and limits
New code reads old sessions/outboxes, and existing plugin data/RPC forms remain
supported. A backed-up, version-only DB migration from 25 to 26 prevents old
applications from misreading encoded files. Downgrade requires a complete
compatible backup. Deduplication is per record, not a global blob store.
Arbitrary full uploads and a genuinely oversized single result retain the
64 MiB limit.
Local validation used macOS and Node 24. Windows checks above cover the targeted
Host paths, not a Windows installer test. The incident did not capture its failing
RPC method; these deterministic regressions do not claim to identify that missing
method.