feat(diary): modernize the diary with Tiptap — three-pane workspace, autosave, attachment-backed images by G9000 · Pull Request #139 · G9000/animaOS

G9000 · 2026-08-03T03:37:46Z

Rebuilds the desktop diary at /journal as a three-pane workspace on a properly architected Tiptap 3.29 editor.

Desktop-only. No server, schema, migration, or packages/api-client changes. HTML stays the wire format, so the separate page/block spec can still land later against a cleaner codebase.

Design: 2026-08-02-diary-tiptap-modernization-design.md · Plan: 2026-08-02-diary-tiptap-modernization.md

What changed

Structure — Journal.tsx (2,199 lines, everything in one component) → 23 focused modules under features/diary/. DiaryWorkspace.tsx is now 658 lines of composition. The boundary rule: editor/ never calls the API, hooks/ never touch ProseMirror.
Slash menu — the hand-rolled regex trigger, manual keydown interception, hand-computed popup coordinates, and five synchronised refs are replaced by @tiptap/suggestion.
New blocks — task lists, tables, toggles, callouts, lowlight code blocks, highlight tones.
Bubble menu — bold/italic/strike/code/link plus three highlight tones.
Drag handle — grip to reorder, with duplicate / delete / turn-into.
Always-editable canvas — the read/edit mode split is gone. Debounced autosave with honest Saving… / Saved / Needs attention states. No diary content ever touches browser storage.
Inline images — no longer base64-embedded in the entry body; they now go through the existing encrypted attachment store. Existing base64 entries still render and are not migrated.

Deliberate trade-offs

No text color. It requires allowing inline style through the sanitizer, which isn't worth it for a cosmetic feature. Highlight uses a fixed data-tone set instead.
Table column widths don't persist. Tiptap always serializes them as inline style, and resizable: false doesn't suppress it. Structure, content, colspan and rowspan all persist; resizing is view-only.
Last-write-wins saves. There's no revision column server-side and adding one was out of scope.
Upload retry doesn't survive an entry switch. A failed image's retry chip lives in the editor instance. Previously the placeholder was persisted and then silently vanished on reload — the loss is now immediate and honest.

Bugs found and fixed along the way

Several of these were latent in the plan itself, not introduced by the implementation:

prosemirror-model duplication introduced by the Tiptap upgrade — @tiptap/pm hoisted 1.25.11 while seven sibling packages stayed nested on 1.25.9. Cross-copy instanceof failures threw RangeError from toggleBulletList and toggleBlockquote, both reachable from the slash menu. Fixed with a root overrides pin.
Autosave flush() was not a real barrier — it could resolve while the newest edit was still mid-save, and flush() is what runs on unmount. A follow-up fix then introduced a worse bug (ensureLoop poisoning its own promise on any no-op call, permanently killing the scheduler); both are fixed and covered.
Emptying an entry never saved. canSaveDiaryEntry was gating persistence, so clearing a titled entry sent no PATCH and the old text resurrected on reopen — you couldn't erase what you'd written.
editorRef was nulled after every entry switch. React 19 + Tiptap order create(new) before destroy(old), so the outgoing editor's teardown clobbered a live ref: titles typed after a switch saved nothing, and an image-only page could be silently deleted. Teardown is now identity-aware.
Inline images re-downloaded in an unbounded loop — an unstable onError in the fetch effect's deps caused a decrypt+download per round-trip per image for as long as the entry was open (16k fetches in 300ms in a reproduction).
Turn-into destroyed content. Table/Divider replaced the selection; now they confirm first. Toggle-family entries were non-idempotent, and container blocks nested instead of converting.

Verification

bun test apps/desktop/tests — 222 pass, 0 fail (from a 111 baseline)
bun run build:desktop — exit 0
bun run lint:desktop — clean
Headless jsdom mount of the real editor, menus, and drag handle: mounts clean, every block type survives a real mounted round-trip, no console errors, keyed-remount handshake holds

Not verified: the interactive live pass. /journal is behind an authenticated unlocked session, which needs a passphrase. So paste-upload, pointer drag-and-drop, and menu click-through have not been exercised against a running backend. Worth a manual pass before merge, particularly: switch entries then type a title; clear an entry's body and reload; paste an image and confirm no base64 in the PATCH body and no repeat downloads in the network panel.

Known minors (deferred, not blocking)

Drag-handle grip is keyboard-inaccessible; the block menu can't be reached by keyboard
Drag-handle gutter overhangs the editor's left edge by ~18px
BubbleMenu uses an as unknown as cast for toggleHighlight; toggleMark("highlight", { tone }) is a cast-free equivalent
A cleared entry reloads as <p>U+200B</p>, so the placeholder doesn't render and the next keystroke saves a zero-width prefix
resolveBodyForSave writes the literal "Attachment-only diary entry." into the body, which round-trips back as visible prose the user never wrote
Task 12's editor-lifecycle tests cover the pure helpers, not the wiring — reverting the wiring leaves the suite green
Pre-existing: sanitized bodies allow class, so pasted Tailwind utilities could overlay the app. The dangerouslySetInnerHTML sink this affected was deleted with read mode; a class-prefix allowlist is worth separate work.

Desktop-only rebuild of /journal as a three-pane workspace with a properly architected Tiptap editor. Keeps the flat entry model and HTML body wire format, so the separate page/block spec can land later against a cleaner desktop codebase.

14 tasks across extraction, editor, shell, and attachments. Spec updated with spike findings: sanitizer needs colgroup/col, and table column widths cannot persist because Tiptap always serializes them as inline style.

…s and highlight

Deletes the regex trigger, manual keydown interception, coordinate math, and four synchronised refs from DiaryWorkspace.tsx (2199 -> 1862 lines) in favor of @tiptap/suggestion wired through the Task 3 extension factory. - editor/slashCommands.ts: SLASH_COMMANDS + filterSlashCommands, covering the new task-list/table/toggle block types. The "callout" command is intentionally omitted — setCallout doesn't exist until Task 8. - editor/SlashMenu.tsx: createSlashRenderer mounts a small React root into an imperatively-positioned DOM node, since Suggestion's onStart/onUpdate/ onKeyDown/onExit lifecycle is imperative by design. - editor/glyphIcons.tsx: the slash-menu glyph icons (plus the shared Glyph base) moved out of DiaryWorkspace.tsx so both it and the new menu can import them without duplication. - extensions.ts: DiaryExtensionOptions gains onImageRequest, invoked by the "image" command instead of running an editor command directly (inserting needs an async file read). - DiaryWorkspace.tsx now builds its editor via createDiaryExtensions(...) instead of its old inline StarterKit/Image/Placeholder array, and wires onImageRequest to the existing hidden file input. Tests: 130 passing (125 baseline + 5 new).

…tones

Task 3's Tiptap upgrade left the workspace resolving two copies of prosemirror-model (1.25.11 hoisted for @tiptap/[email protected], 1.25.9 nested under prosemirror-commands/-gapcursor/-schema-list/-state/-tables/ -transform/-view). Cross-copy instanceof checks between those two copies threw a RangeError the moment a wrap-style ProseMirror command (toggleBulletList, toggleBlockquote) ran through a live Editor — a real, user-facing break in diary block conversions, not a test artifact. Pin prosemirror-model to the version @tiptap/[email protected] wants via a root-level npm-style override so bun resolves exactly one copy.

…tructive conversions Round 1 of the Task 7 review: - Finding 1: insertTable/setHorizontalRule replace the whole selection, so Turn-into -> Table/Divider silently discarded text. Per human ruling, Table and Divider stay in the submenu; a window.confirm now gates the conversion, only when the source block actually has content, worded to name what's lost, and a no-op on cancel. - Finding 2: toggle-based entries (bullet/ordered/task/quote/code) could toggle a block already of that type back to a paragraph instead of staying put. Turn-into now checks the hovered node's actual type first and no-ops when it already matches the target. - Finding 3: DragHandle reports the outer container (e.g. blockquote) as the hovered node, so converting quoted text left the result nested inside the blockquote. The wrapper's content is now lifted to the top level before the conversion runs, so "Turn into Heading 1" on a blockquote produces a plain top-level <h1>.

Review found two Critical concurrency bugs in the brief's sample implementation and one Important issue: - flush() was not a real barrier: its post-drain re-check of `pending` raced the owning drain loop's own continuation, so flush() could resolve while a newer edit was still being saved. - retry() could start a save concurrently with an already-running drain loop by overwriting the shared in-flight promise directly. - dispose() did not guard run()'s continuation, so a save already in flight at dispose time could still fire onStatusChange afterward. Replace the inFlight/drain() pair with a single shared loop promise that schedule(), flush(), and retry() all join, and gate setStatus() on the disposed flag. Public interface unchanged. Adds 3 regression tests (flush barrier under mid-flight edit, retry concurrent with a scheduled save, dispose during an in-flight save) using controllable deferred promises rather than timer races.

…atus Round-1's fix for the flush/retry races introduced a new Critical: an async IIFE that never awaits (because pending was already null) runs to completion synchronously, so its own `loopPromise = null` reset ran BEFORE the outer `loopPromise = (...)()` assignment could complete, permanently overwriting it with a stray non-null resolved promise. Every later schedule()/flush()/retry() would then silently no-op forever. Reachable via an ordinary no-op flush() or a debounce timer left armed by a schedule() call during another operation's drain. Fix: force the loop body to suspend (`await Promise.resolve()`) before touching any shared state, so the loopPromise assignment structurally always completes first regardless of how many iterations the loop ends up running. Also fixes a pre-existing (not round-1-introduced) issue: status stayed "saved" for the whole debounce window after a new edit was scheduled, falsely claiming unsent work was safe. schedule() now transitions status to the already-exported "idle" state when this happens; no interface change. Adds 3 regression tests (no-op flush survival, dangling-timer-after- drain survival, status never "saved" while pending) that fail against the prior code and pass after the fix.

Removes the read/edit mode split from DiaryWorkspace.tsx: the canvas is now always editable, backed by a per-entry autosave scheduler (useAutosave) and a new PageHeader panel (cover, title, date/mood/folder, save-status chip with retry, drawer toggle). New entries POST immediately so attachments always have a real entry id to attach to; attachments and cover images now upload immediately instead of staging for a Save click that no longer exists. Closes the data-loss hazard flagged in the Task 10 review: isDiscardablePage now requires an explicit hasNonTextContent signal (image/table/divider/ callout/details/task-item), computed by walking the actual ProseMirror doc, so a page whose only content is a pasted image or an empty table is never deleted even though its plain-text content is empty. Verified against the real Tiptap schema in diary-editor-schema.test.ts.

…onest flush ordering Fix round 1 for the Task 11 review (1 Critical, 2 Important, 1 bundled minor): - CRITICAL: onUpdate/handleTitleChange gated autosave scheduling on canSaveDiaryEntry, which returns false for a structurally-empty editor with no attachments. Selecting all and deleting a titled entry's body therefore never scheduled a save, so the old text silently survived server-side and resurrected on reopen. Replaced with isSignificantEdit (schedule whenever content actually differs from what was last loaded/scheduled, regardless of emptiness) and resolveBodyForSave (computes the body to persist, including an explicit blank marker for a genuine clear). Also fixed an adjacent bug found while implementing this: the loaded-content reference now advances on every scheduled edit, not just on initial load, so reverting back to the entry's original pristine state after an intermediate save is still persisted. - Important: the blank-entry marker (a zero-width space, needed to satisfy the server's body min-length validation) never trimmed to empty under JS's String.trim(), so isDiscardablePage never saw a freshly-created, untouched entry as discardable — "+ New entry" then navigating away left it behind forever. isDiscardablePage now normalizes zero-width characters out of bodyPlainText before checking emptiness. - Important: useAutosave's cleanup nulled its scheduler ref before flush() started awaiting, so the untitled-page cleanup's "flush the outgoing entry before deciding whether to discard it" step was a silent no-op on the true-unmount path. Removed the premature null-out; the scheduler's own internal disposed guard is the correct place for that safety. - Bundled minor: NON_TEXT_NODE_TYPES gained codeBlock and blockquote (same class of hazard as the already-covered image/table/callout cases); hardBreak deliberately excluded as incidental noise, not deliberate content. 165 -> 180 passing tests.

…awer Task 12: extracts the data layer into useDiaryEntries, the sidebar into LibrarySidebar (with a collapse rail and empty/no-match CTAs), the interim Task 11 drawer into a complete DetailsDrawer (date/mood/folder/ cover, attachments with upload and retry-on-preview-failure, the voice recorder, word/char counts, timestamps, delete), and the editor itself into editor/DiaryEditor.tsx, keyed by entry id so switching pages builds a fresh editor. Recording logic also moved into hooks/useVoiceRecorder. DiaryWorkspace.tsx drops from 1,606 to 658 lines; all delicate autosave invariants from Task 11 (scheduler lifecycle, flush-before-select, lastLoadedBodyRef advance, isSignificantEdit gate, discard-on-snapshot) are preserved unchanged in behavior.

…apshot Task 12 review fix round 1. Finding 1 (CRITICAL): a keyed DiaryEditor remount can fire the new instance's create before the old instance's destroy (confirmed against @tiptap/react's EditorInstanceManager), so an unconditional null-out on teardown was dropping the live editor ref after every entry switch - silently breaking title autosave, inline-image embed, voice-transcript insertion, and (fail-open) letting image-only pages look discardable. Fixed with an identity-checked teardown (lib/editorHandoff.ts) plus a fail-safe true default for unavailable non-text-content data. Finding 2 (Important): the content snapshot used by the untitled-page cleanup is now tagged with the entry id it was captured from, and evaluateAndMaybeDiscard refuses to evaluate a mismatched tag - closes a race where a fast-enough entry switch could judge one entry's discardability against a different entry's content.

Task 13: adds a diaryImage node (attachmentId/caption/uploadState, uploadState never persisted) that coexists with the legacy base64 image node so existing entries keep rendering. Paste/drop/slash-image insertion now uploads via the attachment store instead of embedding base64. Adds diaryImage to NON_TEXT_NODE_TYPES so an image-only page is never mistakenly discardable. Removes the now-unmotivated MAX_INLINE_IMAGE_BYTES cap and fileToDataUrl path.

…, parse precedence Fix round 1 on Task 13 review: - useAttachmentBlobUrl held onError in its fetch effect's deps; every caller passes an inline arrow, so a successful fetch's own setUrl re-rendered the caller, producing a new onError identity, re-running the effect, revoking the URL, and re-fetching forever. Hold onError in a ref instead; fetch effect now depends only on attachment id + retryToken. - A diaryImage node with no attachmentId yet (uploading/failed) rendered to a bare <img alt> that matched no parse rule and silently vanished on reload if it reached the saved body. Added stripUnresolvedAttachmentImages and call it at both places the persisted body is computed, so a pending or failed image is never saved in a form that later disappears; the retry affordance and its cached File are explicitly scoped to the current editor instance and do not survive an entry switch. - diaryImage's parseHTML relied on an incorrect "more specific than img[src]" rationale; ProseMirror actually resolves ties by registration order, and TiptapImage is registered first. Added priority: 100 and corrected the comment.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a43e7b9a4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…r entry mismatch, stale mood, dropped attachments) - useAutosave: teardown now retries a failed save once before disposing the scheduler, instead of only flushing (which never touches the failed payload); surfaces via onUnsavedOnTeardown if the retry also fails. - useVoiceRecorder: captures the entry id when recording starts and threads it through onRecordingComplete/onFinalTranscript instead of letting callers re-derive "whatever is selected now" asynchronously; DiaryWorkspace drops a late transcript for a non-current entry (visible error) but still attaches a late recording to its originating entry (safe, since uploadAttachment addresses by id). - DetailsDrawer: keyed by entry.id to stop stale mood state leaking across entries, and mood now commits on a debounce (plus unmount flush) instead of only on blur. - DiaryEditor: a mixed image+file drop now forwards the non-image subset to the normal attachment flow instead of silently dropping it. Adds regression tests for all four findings (diary-autosave-teardown, diary-voice-recorder-entry-capture, diary-details-drawer-mood, diary-file-drop), each verified to fail against the pre-fix code.

G9000 · 2026-08-03T06:43:52Z

@codex review

Pushed 17cea30 addressing all four findings — both P1s and both P2s. All four threads replied to and resolved.

Summary:

P1 failed-autosave loss — teardown now attempts the failed payload once via the scheduler's retry() (re-queues into pending and joins the single drain loop, so the one-save-in-flight invariant holds), then flushes again. Bounded to one attempt. If it still fails, a workspace banner reports the loss instead of swallowing it.
P1 recording misattribution — the entry id is captured at start() and threaded through both completion callbacks. A late recording still attaches to its originating entry; a late transcript is dropped with a visible error rather than inserted into an unrelated entry.
P2 stale mood (rated higher — it overwrote real data) — drawer is keyed by entry id; mood also now commits on debounce + unmount flush rather than depending on a blur.
P2 mixed drop — new pure partitionDroppedFiles; the non-image subset is forwarded to the attachment flow before propagation stops.

231 tests pass (222 + 9 new regression tests, each verified to fail before the fix), build:desktop exit 0, lint:desktop clean.

Please re-review scoped to P1/important only. One thing worth your attention that is not in the diff: the interactive live pass has not been run, since /journal sits behind an authenticated unlocked session. Paste-upload, pointer drag-and-drop, and menu click-through are unverified against a running backend, so treat any finding in those paths as untested rather than working. I would recommend merging once this pass is clean.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17cea30a6c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…-modernization # Conflicts: # apps/desktop/src/pages/Journal.tsx

…folder/date to discard predicate, apply design tokens to moved diary markup Round 2 review findings on PR #139: - DetailsDrawer's onUpdate now carries the originating entry id, and DiaryWorkspace's dispatch targets that id instead of re-deriving "the current entry" from selectedEntryRef, which had already advanced by the time a deferred commit (mood's debounce/unmount flush) arrived. Audited all five dispatched fields (folder, entryDate, mood, cover clear/set); all now bind the same way via lib/drawerUpdate.ts's resolveDrawerUpdateEntryId. - isDiscardablePage now checks mood (bare presence — no ambient default exists) and folderId/entryDate (changed-from-selection-baseline — both always carry an ambient default, so bare presence isn't a safe signal). New initialFolderIdRef/initialEntryDateRef in DiaryWorkspace capture that baseline per entry selection. - Reapplied main's PR #138 design-token mapping (border-hairline[-strong], text-micro/label/caption, tracking-caps-*, pt-hud) across features/diary/** and index.css's diary rules, which the merge from origin/main missed because this branch had moved that markup out of the old Journal.tsx first. Visual change verified by build + typecheck only.

G9000 · 2026-08-03T07:53:19Z

@codex review

Pushed 0da9502. Merge conflict with main also resolved — the PR is now MERGEABLE.

Both P1s fixed, threads replied to and resolved.

Mood teardown flush targeting the wrong entry — this was a regression from my own previous round: keying the drawer fixed the stale display and created a stale write. Updates now carry their originating entry id via a pure resolveDrawerUpdateEntryId. I audited all five fields handleDrawerUpdate dispatches, not just mood; only mood had a live deferred path, but all five now bind uniformly so a future deferred commit cannot reintroduce it.
Metadata-only entry silently deleted — isDiscardablePage now takes mood, plus folderId and entryDate compared against a baseline captured at load (both have ambient defaults at creation, so presence alone proves nothing). All three are threaded from the real call sites, not just added to the type.

Conflict resolution, worth a look since it is not visible as a code change: main's #138 ("make the design system real") retokenized the old pages/Journal.tsx, but this branch had already moved that markup into features/diary/. I resolved the conflict keeping this branch's 5-line route shell, then applied #138's token mapping by hand to where the markup actually lives — 140 occurrences across 6 files (DetailsDrawer 55, LibrarySidebar 47, DiaryWorkspace 18, AttachmentImage 8, index.css diary rules 8, PageHeader 4). Every target token was verified to exist in packages/standard-templates/src/tokens.css first, since a nonexistent token silently produces no styling. This part is verified by build and typecheck only and has no test coverage — it is the highest-value thing for a human to spot-check visually.

283 tests pass (up from 273 after the merge; 10 new regression tests, each verified to fail before its fix), build:desktop exit 0, lint:desktop clean.

Please re-review scoped to P1/important only. Same standing caveat: the interactive live pass has not been run, so paste-upload, pointer drag-and-drop, and menu click-through are unverified against a running backend — treat findings there as untested rather than working. I would recommend merging once this pass is clean.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0da9502518

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…-mood race Round 3 review, Finding 2 (P1): evaluateAndMaybeDiscard judged only content, never origin, so a pre-existing (server-loaded) blank entry was silently deleted just by being opened and left. Gate discard to entries this workspace session itself created (sessionCreatedEntryIdsRef, in-memory only); once a session-created entry receives any real edit it graduates out of eligibility permanently, even if later cleared back to blank. Finding 1 (P1): the discard check read entry.mood (server-committed) directly, so a typed-but-undebounced mood didn't block discard, and the drawer's own unmount flush then PATCHed the deleted entry. DetailsDrawer now reports live mood keystrokes via onMoodDraftChange; the discard check resolves the live draft instead of the stale committed value. New isSessionDiscardable / resolveLiveMood / graduateSessionEntry pure functions in lib/pageLifecycle.ts, unit + integration tested in the new diary-session-discard.test.tsx (13 tests, BEFORE verified failing against the pre-fix module).

G9000 · 2026-08-03T09:27:02Z

@codex review

Pushed 67c7a80. Both P1s fixed, threads replied to and resolved.

Any blank entry deleted by opening and leaving it — agreed this was the most serious defect in the PR, and worse than it reads alone: it compounded commit 7d2a092 (which fixed a Critical where intentional clears never saved), producing clear → reopen → leave → silently destroyed. The workspace now tracks the ids it POSTed in memory and a pure isSessionDiscardable short-circuits before isDiscardablePage, so a server-loaded entry is never silently deleted regardless of how blank it is. A session-created entry graduates permanently out of eligibility on any real edit, which breaks the create→type→save→clear→leave chain too.
Stale mood read / PATCH-after-DELETE — incomplete fix on my side from round 2: mood was in the predicate but read from server state, not the live draft. The drawer now reports draft changes per keystroke and a pure resolveLiveMood is consulted instead. Audited the sibling fields: mood is the only debounced draft (entryDate/folderId commit synchronously, cover/attachments are immediate).

296 tests pass (283 → 296; 13 new, each verified failing first), build:desktop exit 0, lint:desktop clean.

Please re-review scoped to P1/important only. Two standing caveats, both unchanged and both worth weighting in your read:

The interactive live pass still has not been run — /journal is behind an authenticated unlocked session. Paste-upload, pointer drag-and-drop, and menu click-through are unverified against a running backend. Treat findings there as untested rather than working.
The design-token migration from the main merge has no test coverage — 140 occurrences across 6 files, verified by build and typecheck only. It is the best candidate for a human visual spot-check.

Worth noting the pattern across these three rounds: every P1 so far has been on the metadata/discard path, and two of them were regressions introduced by fixes to that same path. If anything else on that path looks suspect to you, I would rather hear it now than after merge. Recommend merging once this pass is clean.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 67c7a8060b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…tion, not completion PR #139 round 4, two P1 findings sharing one root cause: discard eligibility was evaluated against committed state while an attachment upload or voice recording was still in flight, so a session-created entry could be deleted mid-upload or mid-recording. Structural fix: push graduation into the three hook functions that actually perform the async work (useDiaryEntries.uploadAttachment, useAttachmentUpload, useVoiceRecorder.start), firing synchronously before any await. This closes all six audited content-producing call sites (Attach button, drag-drop, cover image, completed voice recording, record-start, inline editor images) through three chokepoints instead of scattered call-site edits, and cannot be forgotten by future callers.

G9000 · 2026-08-03T09:43:35Z

@codex review

Pushed a29cfe1. Both P1s fixed, threads replied to and resolved. 300 tests pass, build exit 0, lint clean.

Fixed as one root cause, not two call sites. Both findings this round — and the mood/debounce P1 from round 3 — are the same defect: discard eligibility was evaluated against committed state while user-initiated work was still in flight. Patching the reported sites would have left the class open, so instead: an entry now graduates out of eligibility the moment the user INITIATES a content-producing action, synchronously before any await.

Audited all 6 initiation points (recording-complete upload, Attach button, drag-drop, cover, voiceRecorder.start, inline editor image), now covered by 3 chokepoints — useDiaryEntries.uploadAttachment, useAttachmentUpload, useVoiceRecorder.start. Not one chokepoint because these are three independent async subsystems with no shared function, but each is the single point for its category, so future paths are covered by default. Notably onRecordingInitiated fires before getUserMedia, which covers the pending-permission half of the second finding too.

Deliberately not added: a wait-for-in-flight mechanism. Graduation alone covers all six and avoids a new hang class. Deliberately accepted: a failed upload leaves the entry permanently non-discardable — fail-toward-keep is this file's established default, and reverting graduation on failure reopens the race.

On the pattern I flagged last round: that is now 8 P1s across 4 rounds, every one on the metadata/discard path. Rounds 3 and 4 were both consequences of round 2's own fix. I think the structural change above is the first one that addresses the class rather than an instance, but I would rather you push on that judgement than take my word for it — if you can still construct any sequence where a user-initiated edit is in flight and evaluateAndMaybeDiscard deletes the entry, that is the finding I most want.

Two standing caveats, unchanged:

Interactive live pass still not run — /journal is behind an authenticated unlocked session, so paste-upload, pointer drag-and-drop, and menu click-through are unverified against a running backend. Treat findings there as untested rather than working.
The design-token migration has no test coverage — 140 occurrences across 6 files, build/typecheck only. Best candidate for a human visual spot-check.

Recommend merging once this pass is clean.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a29cfe1b1a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…ight PR #139 round 5 P1: a successful inline-image upload was silently discarded if the user navigated away before it resolved. The upload effect's `cancelled` flag skipped writing attachmentId/uploadState on completion, and skipped cleaning up the pendingFiles cache entry too — a silent partial success, since the server had already created the attachment row and Task 13's stripUnresolvedAttachmentImages had already dropped the placeholder from the persisted body. Moves completion ownership into the extension's storage (a new liveHandlers registry, populated by a NodeView while mounted) and a pure, unit-testable handleUploadResolution/isEditorAvailable pair that decides at resolution time whether it's safe to update the node inline, or whether to clean up and report the upload as an orphaned attachment on its originating entry via a new onUploadOrphaned callback (DiaryImageOptions -> extensions.ts -> DiaryEditor -> DiaryWorkspace's handleInlineUploadOrphaned, which surfaces a notice and silently refreshes entries so the attachment appears in that entry's details drawer). Audited features/diary/ for the same teardown-cancellation pattern; useAttachmentBlobUrl's own `cancelled` flag only discards a read (blob fetch for display), not a write with a server-side effect, so it was left unchanged.

…5 upload fix The BEFORE test served its one-time purpose (verified bug reproduction, recorded in the PR thread) but froze 295 lines of production source into the test tree permanently and could never fail again going forward. Removed the fixture directory and the single test that imported it; kept all AFTER tests (unmounted-orphan reconciliation, unchanged mounted happy path, and the four pure isEditorAvailable/handleUploadResolution unit tests), which exercise the real shipped code and are the ongoing regression guard. Verified by temporarily reintroducing the reverted `cancelled`-flag early-return in the live AttachmentImage.tsx: the unmounted-orphan test failed as expected, confirming the remaining suite is load-bearing.

G9000 · 2026-08-03T10:40:30Z

@codex review

Pushed eec8b70. The P1 is fixed, thread replied to and resolved. 306 tests pass, build exit 0, lint clean.

In-flight inline image lost across teardown — completion ownership moved out of the disposable NodeView into the extension's storage, so "is the originating editor still available" is answered at resolution time rather than captured by a cancelled flag at effect time. Mounted → unchanged inline write. Unmounted + success → cache cleaned, the file surfaces in the originating entry's attachment list, and the user is told: "An image finished uploading after you left the entry. It was saved as an attachment instead of appearing inline." No editor is touched when unmounted, since writing into whatever entry is now open would corrupt an unrelated one. Also audited the pattern feature-wide — one other let cancelled exists but discards only a display read with no server-side effect, so it is correctly left alone.

Two process notes, both self-inflicted and worth stating rather than hiding:

My first attempt at this proved the bug by vendoring 295 lines of pre-fix production source into tests/fixtures/. I had that removed in eec8b70. A test asserting frozen old code misbehaves can never fail again — it reads as coverage while guarding nothing, and the stale copy would rot beside the real component. The remaining tests exercise the shipped code, and I verified they are load-bearing by reverting the real fix and confirming the orphan test fails.
On the pattern I raised in round 4: this round found no new discard-eligibility hole, which is the first evidence that graduating-at-initiation closed the class rather than another instance. That was the specific thing I asked you to attack, so I am reading its absence as meaningful rather than as luck — but if you deliberately did not probe there, say so and I will not draw that conclusion.

Standing caveats, unchanged and still the two weakest points in this PR:

Interactive live pass never run — /journal is behind an authenticated unlocked session. Paste-upload, pointer drag-and-drop, and menu click-through are unverified against a running backend. Notably that includes the inline-upload path this round touched, so my fix here is verified by unit tests and source reasoning only.
Design-token migration has no test coverage — 140 occurrences across 6 files, build/typecheck only. Best candidate for a human visual spot-check.

Recommend merging once this pass is clean.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eec8b70e44

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…d notice, Escape not closing slash menu - AttachmentImage.tsx: add an in-flight guard (uploadsInFlight, keyed by localId) so a re-run of the upload effect (Strict Mode's mount->cleanup ->mount, or any future re-run) can't start a second uploadImage call for the same node. The mounted NodeView still receives the one upload's result via the existing liveHandlers registration. - orphanUploadNotices.ts (new) + DiaryWorkspace.tsx: hold pending orphan-upload notices in a module-scoped in-memory queue that outlives a single DiaryWorkspace mount (never browser storage), and drain it on the next mount / via a live subscriber — fixes the notice going silently missing when the user leaves /journal entirely before an orphaned upload resolves. Multiple queued notices coalesce into one counted message. - SlashMenu.tsx: Escape now actually tears the popup down (same as onExit) instead of returning true with no effect, and onUpdate refuses to recreate it for the rest of the same trigger session. 306 -> 318 tests passing (+12). build:desktop and lint:desktop clean.

G9000 · 2026-08-03T11:04:20Z

@codex review

Pushed 544fc43. All three P2s fixed, threads replied to and resolved. 318 tests pass, build exit 0, lint clean.

No P1s this round — and combined with round 5 also finding none on the discard path, I am now fairly confident round 4's graduate-at-initiation change closed that class rather than another instance.

Duplicate upload under Strict Mode — the real defect was the missing in-flight guard (round 5 removed the cancelled flag and left nothing behind); Strict Mode was just the dev-time reproduction. Added uploadsInFlight claimed by a pure claimUploadSlot, cleared on both success and failure. Handler registration stays in a separate unconditional effect, so the guard suppresses the redundant network call and never the outcome.
Orphan notice lost on leaving /journal — a regression from round 5, and worse than it reads: the notice worked when switching entries but not when leaving the route, which is the likelier case, so round 5's user-visible half was effectively dead. Now a module-scoped queue drained idempotently on next mount. Not browser storage, per this repo's absolute rule. Multiple queued notices coalesce into one counted message, since the error slot is a single string.
Escape not closing the slash menu — dismisses properly now, still returns true, and does not resurrect for the same trigger while a fresh / reopens normally.

Two process notes:

Per your last round, I did not vendor pre-fix source into the test tree this time. Each before/after was demonstrated by temporarily reverting the real code, confirming the new test fails, then restoring — results reported per finding on each thread.
Three of the last four findings have been regressions from my own fixes (round 3 and 4 from round 2; two of this round's three from round 5). That is the signal I am watching most closely now: the fixes are converging, but each one has been touching live async lifecycle code, which is exactly where a fix creates its successor. If you see any remaining place where round 5 or 6 traded one defect for another, that is the highest-value thing you can tell me.

Standing caveats, unchanged and still the two weakest points:

Interactive live pass never run — /journal sits behind an authenticated unlocked session, so paste-upload, pointer drag-and-drop, and menu click-through are unverified against a running backend. That now includes the inline-upload and Escape paths touched in rounds 5 and 6.
Design-token migration has no test coverage — 140 occurrences across 6 files, build/typecheck only.

Recommend merging once this pass is clean.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 544fc43a7c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…d, Escape leaves slash menu keys captured Round 6's drain effect set the notice then fired reload without awaiting it; reload's own setError(null) on success wiped the notice the instant the request resolved. Extracted drainAndShowOrphanUploadNotice, which now awaits reload first and sets the notice only once it settles. Round 6's Escape handler tore down the popup's root/container but never cleared `current`, so onKeyDown kept consuming ArrowUp/ArrowDown/Enter for the rest of the trigger session — Enter would silently invoke the last-selected, now-invisible command instead of inserting a newline. onKeyDown now treats root/container being null (the same signal onUpdate already uses) as a full stop.

G9000 · 2026-08-03T11:54:50Z

@codex review

Pushed 356f268. Both P2s fixed, threads replied to and resolved. 321 tests pass, build exit 0, lint clean.

Both were regressions from round 6, and both were the same shape: a fix that made something look correct while leaving the other half wrong.

Notice wiped by its own reload — the notice now lands after the reload settles rather than before. Fixed at the call site rather than making reload's setError(null) conditional, since its other two callers legitimately want stale errors cleared.
Escape leaving keys captured — onKeyDown now returns false once the popup is gone, reusing the same signal onUpdate already trusts instead of a second flag that could drift. Worth naming the harm precisely: round 6 gave you a menu that looked closed and behaved open, so Escape-then-Enter silently inserted a block instead of a newline. That is why I treated a P2 as worth a full round.

On convergence. Severity across the seven rounds: 2 P1+2 P2 → 2 P1 → 2 P1 → 2 P1 → 1 P1 → 3 P2 → 2 P2. No P1 in the last three rounds, and the last two rounds found only follow-on halves of my own fixes rather than new defect classes. That reads like genuine convergence rather than luck, but I would rather you contradict me than agree by default: if you think the remaining findings are tapering because the code is settling, say so; if you think they are tapering because your later passes are covering less ground than the early ones, say that instead. I will keep iterating either way, but which of those is true changes whether this is close to mergeable.

Standing caveats, unchanged, and both still the weakest points in this PR:

Interactive live pass never run — /journal sits behind an authenticated unlocked session, so paste-upload, pointer drag-and-drop, and menu click-through are unverified against a running backend. That now covers the inline-upload, orphan-notice, and Escape paths touched in rounds 5-7, i.e. most of what recent rounds changed.
Design-token migration has no test coverage — 140 occurrences across 6 files, build/typecheck only.

Recommend merging once this pass is clean.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 356f26861c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

… failure, atom turn-into hazard, orphaned entry on unmount - useAutosave: gate onStatusChange by scheduler identity so an outgoing generation's teardown retry can never publish status into the entry the user switched to. - DetailsDrawer: thread a real success/failure outcome back through onUpdate so a failed mood PATCH rolls back lastCommittedMoodRef instead of being treated as already committed. - BlockDragHandle: refuse "Turn into" on an atom (image/divider) — the collapsed-selection trick's `pos + 1` lands past the node, not inside it, and can relocate the pending selection onto the following block. - DiaryWorkspace: track mount state across startNewEntry's createEntry await; if the workspace unmounted before the POST resolved, delete the untouched entry (reusing isSessionDiscardable) instead of calling setSelectedId on a gone component or stranding it on the server. 330 tests passing (321 baseline + 9 new regression tests), build and lint clean. Full before/after verification per finding in pr-139-round8-report.md.

G9000 · 2026-08-03T12:36:14Z

@codex review

Pushed 4a0bc08. All four P2s fixed, threads replied to and resolved. 330 tests pass, build exit 0, lint clean.

Fourth consecutive round with no P1. Two of these four were again follow-ons from my own fixes (the stale-status leak came from round 1's teardown retry; the mood-commit marker from the round 6/7 mood work), and two were genuinely new ground — the atom hazard and the stranded-create leak.

Stale scheduler status — gated on scheduler identity, implemented to work with the deliberate no-null-on-cleanup decision rather than against it. Your last clause was the sharpest part: A's retry labelling B "Saved" mid-debounce would have violated the scheduler's own no-false-Saved invariant, and that is what the test pins. I also checked whether a user still learns about A's edit being lost if the retry fails after they move on — that path was already correct and goes through a different channel.
Unretried mood failure — committed marker now rolls back on failure so the draft stays retryable.
Atom turn-into — fixed by refusing the operation for atoms. One correction to the record: I verified against this exact Tiptap/ProseMirror version and the pre-fix code fails safely for the document — it relocates the live selection onto the following block rather than transforming it. Real bug, worth fixing, but "can transform the following paragraph" overstates this version's behavior. Recording that rather than accepting the stronger reading.
Stranded create after unmount — deletes via the existing isSessionDiscardable predicate rather than a second ad-hoc rule, so the never-delete-what-the-session-didn't-create invariant still holds. Its test caught a real bug in the fix's own first draft (a field-name mismatch making attachmentCount read undefined).

I asked last round whether findings are tapering because the code is settling or because later passes cover less ground, and I do not think you answered it. This round found two defects on ground no earlier round had touched — atoms in turn-into, and the pending-POST teardown window — which is evidence for "still finding new ground" rather than "settling". So I am reading convergence as unproven. A direct answer would genuinely change my merge recommendation.

Standing caveats, unchanged:

Interactive live pass never run — /journal is behind an authenticated unlocked session. Everything rounds 5-8 touched (inline upload, orphan notice, Escape, drag-handle turn-into) is unverified against a running backend.
Design-token migration has no test coverage — 140 occurrences across 6 files, build/typecheck only.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a0bc0806f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-03T12:41:34Z

+    return sanitizeDiaryHtml(
+      isHtmlBody(selectedEntry.body) ? selectedEntry.body : escapeHtmlForEditor(selectedEntry.body),


Strip the blank marker before initializing Tiptap

When a newly created or intentionally cleared entry has body === BLANK_BODY_MARKER, this passes U+200B into Tiptap as real text. JavaScript does not trim that character, so Tiptap treats the otherwise blank document as nonempty: the placeholder is suppressed, the details drawer reports one character and one word, and subsequent edits persist the marker alongside the user's text. Normalize the marker to an empty editor document before constructing initialHtml.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-03T12:41:34Z

+      void result.then((success) => {
+        if (!success && lastCommittedMoodRef.current === trimmed) {
+          lastCommittedMoodRef.current = previous;
+        }


Retain failed mood commits after drawer teardown

When the debounce fires immediately before an entry switch, or the unmount cleanup itself starts the PATCH, a failed request reaches this rollback only after the keyed drawer has been destroyed. Resetting a ref owned by that destroyed component cannot schedule another attempt, so reopening the entry loads the old mood and the draft is lost. Fresh evidence after the resolved mood-retry thread is that the teardown path calls commitMoodValue exactly once and has no owner for the rolled-back value; preserve that draft or retry it outside the disposable drawer.

Useful? React with 👍 / 👎.

G9000 added 22 commits August 2, 2026 14:40

docs: design for diary Tiptap modernization

1ef606e

Desktop-only rebuild of /journal as a three-pane workspace with a properly architected Tiptap editor. Keeps the flat entry model and HTML body wire format, so the separate page/block spec can land later against a cleaner desktop codebase.

docs: implementation plan for diary Tiptap modernization

6f69112

14 tasks across extraction, editor, shell, and attachments. Spec updated with spike findings: sanitizer needs colgroup/col, and table column widths cannot persist because Tiptap always serializes them as inline style.

refactor(diary): move journal helpers into features/diary/lib

f79546c

refactor(diary): extract DiaryWorkspace from the Journal route

a48bab1

feat(diary): add Tiptap extension set with task lists, tables, toggle…

8ca9d8f

…s and highlight

feat(diary): widen sanitizer for task lists, tables and toggles

d691b9b

feat(diary): add selection bubble menu with formatting and highlight …

0329458

…tones

feat(diary): add block drag handle with duplicate, delete and turn-into

00dc685

feat(diary): add callout block

a2918ab

feat(diary): add framework-free autosave scheduler

a446d34

feat(diary): add untitled page cleanup predicate

6894f24

chatgpt-codex-connector Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread apps/desktop/src/features/diary/panels/DetailsDrawer.tsx

Comment thread apps/desktop/src/features/diary/lib/pageLifecycle.ts Outdated

G9000 added 2 commits August 3, 2026 15:34

Merge remote-tracking branch 'origin/main' into worktree-diary-tiptap…

9c75003

…-modernization # Conflicts: # apps/desktop/src/pages/Journal.tsx

chatgpt-codex-connector Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread apps/desktop/src/features/diary/DiaryWorkspace.tsx Outdated

Comment thread apps/desktop/src/features/diary/DiaryWorkspace.tsx

chatgpt-codex-connector Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread apps/desktop/src/features/diary/DiaryWorkspace.tsx

Comment thread apps/desktop/src/features/diary/DiaryWorkspace.tsx

chatgpt-codex-connector Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread apps/desktop/src/features/diary/editor/nodes/AttachmentImage.tsx Outdated

G9000 added 2 commits August 3, 2026 18:36

chatgpt-codex-connector Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread apps/desktop/src/features/diary/editor/SlashMenu.tsx

Comment thread apps/desktop/src/features/diary/DiaryWorkspace.tsx Outdated

Comment thread apps/desktop/src/features/diary/editor/nodes/AttachmentImage.tsx

chatgpt-codex-connector Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread apps/desktop/src/features/diary/DiaryWorkspace.tsx Outdated

Comment thread apps/desktop/src/features/diary/editor/SlashMenu.tsx

chatgpt-codex-connector Bot reviewed Aug 3, 2026

View reviewed changes

G9000 merged commit 4f302f7 into main Aug 4, 2026

G9000 deleted the worktree-diary-tiptap-modernization branch August 4, 2026 10:01

		return sanitizeDiaryHtml(
		isHtmlBody(selectedEntry.body) ? selectedEntry.body : escapeHtmlForEditor(selectedEntry.body),

Conversation

G9000 commented Aug 3, 2026

What changed

Deliberate trade-offs

Bugs found and fixed along the way

Verification

Known minors (deferred, not blocking)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

G9000 commented Aug 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

G9000 commented Aug 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

G9000 commented Aug 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

G9000 commented Aug 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

G9000 commented Aug 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

G9000 commented Aug 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

G9000 commented Aug 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

G9000 commented Aug 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!