Repository navigation
fix(native-chat): admit persisted Claude queued prompts and notices - #19428
brennanb2025 wants to merge 3 commits into
Conversation
brennanb2025
left a comment
There was a problem hiding this comment.
Independent review at e893a1c: FINDINGS — not CLEAN.
P2: Negotiate restored system-notice publication for released clients. The decoder now returns system text/tones unconditionally (src/main/native-chat/transcript-line-decoders-claude.ts:91–99). The existing runtime read/snapshot/replacement/append paths publish them without a notice capability (src/main/runtime/rpc/methods/native-chat.ts:125–130,187–194,217–234). The latest release, v1.4.197 (5ee4ace), classifies every non-user message as agent content in mobile/src/session/MobileNativeChatMessage.tsx:295–297,329–370, with agent controls and no notice label/tone.
A focused probe passed a current-decoder informational warning into that actual released message component: system role and warning tone survived, but the output had one agent copy control and no system label. Thus updating only the host makes login/system warnings appear as Claude prose on released mobile. This violates the required attribution distinction and remote-wire Rule 3 despite the unchanged schema/opcode. Keep classification in the shared parser; add capability-aware host publication with a defined legacy fallback and tests covering both version directions across read, initial, append, replacement and paging. The probe mocked native primitives and used checkout dependencies, so it is released-component evidence, not a complete paired-runtime test.
Functional correctness: 41 independent root tests and 34 mobile component tests pass, zero failures. Hidden Electron with an isolated synthetic folder workspace displayed the queued prompt once, both replies, distinct notices, safe code/link rendering, and no bookkeeping/task XML; a new JSONL append appeared through the live tailer. Existing timestamp ordering still places the queued prompt before the first reply, as explicitly excluded. Neither whole ticket is established complete.
Architectural fit: The shared Claude dispatcher removes the role-first omission at its owner; a renderer recovery classifier or cross-provider rewrite would be inferior. The remaining defect is publication compatibility.
Precedent/deviations: Inspected structured provider classification, readable extraction, full/tail readers, and existing notice/Markdown routing. Semantic/extraction/presentation reuse is concrete; structured journal delivery and legacy filesystem replay differ. No upstream provider-schema alignment is claimed.
Validation/gaps: Author 72-test, sequential root/mobile typecheck and 11/1/3 ablation evidence inspected; independently reran the affected 41/34 tests and rendered desktop path. Broad compilers were not repeated. Rendered mobile QA remains blocked (no Orca emulator running); full released-runtime pairing, real provider incident, external navigation and physical Windows/Linux/WSL/SSH remain unverified. No new polling, subprocess fanout or unbounded scan was introduced; large-record Markdown performance was not benchmarked.
brennanb2025
left a comment
There was a problem hiding this comment.
No issues found. CLEAN for the scoped admission/publication change at cf4f3a4; the prior P2 publication finding is cleared.
Functional correctness: strict per-request systemNotices support gates read/paging/snapshot/replacement/append publication; absent/false retain legacy omission and malformed support rejects. Host evidence, lifecycle and cursors remain intact. Actual released mobile keeps the empty-page Load earlier control.
Architectural fit: retain shared semantic admission and host-owned compatibility projection. The former role-first gate and independently versioned consumer misattribution are distinct root failures, both addressed at their owners. No renderer-specific recovery or global subscription capability state is needed.
Precedent/material deviations: inspected actual v1.4.197 host schema, decoder, mobile row/stream/paging mechanisms and current transcriptPending capability placement. Old hosts strip the optional key. Boolean content support differs from pending literal 1; no new opcode or dependent client action requires an echo. Structured journal delivery/lifecycle is not claimed equivalent to persisted JSONL replay.
Independent validation: 7 root suites/89 tests and 4 mobile suites/52 tests passed, zero assertion failures; all three sequential node/web/CLI typechecks passed. Inspected extracted-module harness rerun passed four pairings/28 observations plus 12 delivery observations while changing capability across subscriptions. Fresh hidden Electron CDP proved exact checkout, real chat toggle, queued prompt once, both replies, safe Markdown/link presentation, no bookkeeping/task noise, a notice appended after reader readiness, and toggle replay. Full-window screenshots directly inspected; task-owned process cleanup verified.
Setup errors were corrected and retained in local evidence: incomplete synthetic status metadata, two wrong mobile filename filters, and a wrong toggle label. Extracted modules use synthetic reader/watcher/native primitives and current transitive imports: this is not live mixed-version pairing. Rendered mobile is blocked without a task-owned emulator; full live pairing, physical Windows/Linux/WSL/SSH, real incident, external link navigation and alternate viewport/theme remain unverified. No full causal ordering, Codex output_text, ticket closure or overall merge-readiness claim.
|
Renderer-side companion to this: #20847. It does not touch That case survives admission by this PR's own scoping: records lacking Independent evidence for the admission case was posted on #20846 ( Happy to fold #20847 into this branch if you would rather land one change. |
ELI5
Claude can persist queued prompts and login notices outside ordinary user/assistant records. Native Chat discarded those records before examining their content. The shared decoder now admits real queued prompts and readable system notices, and the host publishes notices only to paired clients that can distinguish them from agent speech.
What changed and why
A Claude-specific semantic classifier admits ordinary messages, queued prompts and readable notices while excluding known bookkeeping and task-notification attachments. Full and bounded/live-tail readers share that owner. Existing UUIDs, timestamps, tool results, image companions, injected-context filtering and interruption handling retain their owners. Desktop notices reuse the existing system notice row and safe Markdown handling; mobile maps already-classified notices into labeled rows with no agent copy controls.
The runtime RPC publication boundary requires
capabilities.systemNotices: trueon reads and subscriptions before publishing toned system messages. Absent or false retains legacy omission; malformed values fail validation. Desktop runtime, web and mobile advertise support for reads, paging and subscriptions/reconnect. Initial snapshots, replacements and appends use the same host projection, preserving underlying record evidence, lifecycle and paging cursors. Same-build local Electron IPC keeps notices.This addresses content-publication skew under remote-wire Rule 3. Released v1.4.197 mobile otherwise treats non-user notices as agent prose. Old clients do not receive this notice fix until upgraded; new clients on old hosts still cannot recover records those hosts discard. There is no new opcode, and old hosts ignore the optional capability.
Scope
Full causal ordering, structured live paths and Codex output_text normalization remain excluded. Neither ticket is claimed complete.
Validation at cf4f3a4
config/vitest.config.ts(mobile with an absolute config path and mobile root).npx tsc --noEmitprojects node, tc.web and tc.cli all passed. Commit hooks passed oxlint, React Doctor and formatting./loginas code and enrollment as a link, without telemetry/task XML. Appending a new JSONL notice displayed a distinct warning row. Full-window screenshots are retained in local QA evidence; no public image upload.Initial failed attempts were corrected and retained: one outdated RPC-argument test expectation, a mobile config path error and incorrect component filename filters, three new-test curly-brace lint errors, and released-module probe setup errors. The first hidden launch exceeded macOS's Unix-socket path limit; a shorter fresh synthetic path resolved it. Both launches were cleaned up through recorded, command-verified process groups.
Review and remaining gaps
Draft. Independent architectural re-review pending at cf4f3a4; the previous P2 publication finding has an implementation fix and author validation, not independent clearance.
Rendered mobile QA is blocked: no task-owned Orca emulator is running. Full live released/current pairing, physical Windows/Linux/WSL/SSH, real provider incident reproduction and external link navigation remain unverified. No merge-readiness or ticket-closure claim.
Author: @BrennanKB5. No upstream skill-installer content copied.