Repository navigation
Preserve the inbox and recover browser sessions safely - #158
Merged
Merged
Conversation
…service - Schema 2: sources.project_id may be NULL. A source without a project is an inbox entry. The migration rebuilds the table in one transaction and keeps every row, position, and timestamp. - A transfer to a project is one PUT that sets projectId, so a repeated transfer cannot copy the source. PUT with "If-None-Match: *" creates a source only if the ID is new (412 otherwise). - Backup format version 2 includes inbox entries and their files. Version 1 backups still restore. Counts and restore results report the inbox. - GET/PUT /v1/preferences/<key>: small JSON documents for client settings. Not part of a backup. - Session responses carry X-Knowledge-Session-TTL. - The legacy import reads "ingest-queue" and imports each inbox entry only if no source has its ID. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Inbox (review finding 2): - The inbox loads from the storage service and saves each change there. An entry leaves the inbox list only after the transfer to a project is saved. A transfer that the service rejects stays visible on screen only where the service has it. - An inbox that an earlier version kept in localStorage moves to the service once with create-only writes. The key is removed only after every entry is saved. - Browser settings are a preference document in the service. The app loads them before startup, so no page saves defaults over them. - The launcher derives a stable port from the library path, so the browser address (and its UI preferences) stays the same. --port and --session-idle-minutes are new options. Session (review finding 3): - BackendService tracks the connection state (active, none, expired, signedOut, unreachable) from the session lifetime header and from 401, 403, and network failures. - StorageService writes run in one ordered queue of idempotent PUT and DELETE requests. A write refused for the session stays queued with all later writes and runs once after reconnect. Uploads repeat only after 401 or 403. The page warns before closing with queued writes. - A blocking session dialog explains recovery: paste a new launch link, or open it in another tab and check again. No reload is needed. - Log out in the title bar saves queued writes first (or asks), ends the session, clears library caches, and reloads without library data. - "Saved" in source details shows only after the service saved. Tests: Chrome tests for the inbox across a port change, an interrupted transfer, expiry and recovery, and logout, with console and page errors checked at each stage; a desktop test for the local inbox migration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review finding 4 and the same problem in other visible actions: - Save as PDF goes through a WebsitePdf capability. The desktop uses the hidden-window printToPDF path; the browser disables the button and says why. The loading dialog no longer waits for a reply that never comes. - Website Preview opens a new tab in the browser (no embedded browser). - File Preview shows the library copy through ManagedFiles on both platforms (PDF, images, text) instead of the original local path. - Opening a file without a library copy explains that it needs the desktop app instead of waiting for IPC forever. - Chat is disabled in the context menu and hidden on cards in the browser; "Show in files" appears only where it works. - The browser makes no request to the external favicon service. - Website links open with noopener. - Fixed console errors: a missing icon cache entry logged an error, the fresh-start route had an undefined segment, and /favicon.ico was missing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…NTS.md - KC_HIDDEN_WINDOW=1 (set by e2e/lib.mjs) shows the window inactive, transparent, and without mouse input, and runs the app without a Dock icon on macOS. Tests no longer take the screen or focus. Chromium background throttling stays on: turning it off stopped painting after a reload. KC_E2E_SHOW=1 shows the window. - The embedded browser test follows a link to a second page, then uses the app's Back and Forward buttons, then checks that the view stays inside the window after a resize. - The packaged test uses backup version 3 as the unsupported example. - Removed screenshots that only served manual inspection. - AGENTS.md holds the agent guidance (commands, architecture, rules, including the rule on test rendering). CLAUDE.md points to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RobRoyce
commented
Oct 8, 2026
RobRoyce
left a comment
RobRoyce
left a comment
Owner
Author
There was a problem hiding this comment.
Focused integration review of PR #158 at 4d8d74b4a504a0f0b35d8839eb59aebbd4075988.
No confirmed blocking finding in the combined revival stack. The independent code review checked storage writes, schema migration, managed files, backup/restore, local authentication, session recovery, Electron startup, and dependency removal. Main's conflicts were resolved without downgrading the stack's dependencies. All original commits are preserved.
Validation on the combined stack at 6ee979b2615db242ca6525bb7d0ccc36d563479f, using Node.js 24.21.0 in an isolated checkout:
- Clean immutable dependency install: passed.
- Development build and storage type check: passed.
- Unit tests: 8 passed.
- Storage tests: 35 passed.
- Hidden-window desktop tests: 4 passed.
- Headless Chrome tests: 6 passed.
- Unsigned production package and packaged workflow: passed (1 test).
Each historical intermediate branch was not rebuilt independently. Signing, live AI credentials, other operating systems, and broad dependency security auditing are outside this review.
This is a review record submitted through the PR author's account, not the independent approving review required by main's branch protection. CodeQL status is reported separately by GitHub.
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
1 participant
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.
Move inbox entries and browser settings into the storage service. Add backup format version 2 with inbox records and retain version 1 restore support. Keep writes queued during session loss, provide recovery and logout, and disable unsupported browser actions. Add browser regression tests and embedded-browser Back and Forward checks. Keep automated desktop windows hidden.
The published snapshot still has the documented resize-history and expired-dialog logout limits. Later local fixes are outside this review.
Depends on #157. Merge the stack in order with merge commits, retargeting each next PR to main after its predecessor lands. Do not squash or delete intermediate branches during integration.
Validation on the combined stack at
6ee979b2615db242ca6525bb7d0ccc36d563479f, using Node.js 24.21.0 in an isolated checkout:Each historical intermediate branch was not rebuilt independently. Signing, live AI credentials, other operating systems, and broad dependency security auditing are outside this review.
Focused review found no confirmed blocker in the combined stack. The review covers basic workflows, data preservation, authentication, and the dependency changes. Broader modernization remains separate work.