Repository navigation
Preserve Solo queued messages across restarts - #1704
Conversation
Restore saved messages and resume them in order after interrupted work completes. Keep declined recovery paused and protect the durable Outbox handoff. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d02ab14ea6
ℹ️ 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".
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account. One change requested: preserve ordering when recovering an existing Outbox receipt; details inline.
Reviewed head d02ab14 against base b6e1941, with independent Rocket, Mantis and Groot lanes. Source-only: no tests or app execution; the hosted CI snapshot passed, with Windows validation skipped. Native restart and human acceptance remain unverified.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f88a83416b
ℹ️ 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".
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account. No further changes requested: the recovered-receipt ordering defect is repaired by registering the existing per-message wait before recovery, with a focused two-row regression.
Reviewed head f88a834 against base b6e1941, focusing on changes since d02ab14 with independent Rocket, Mantis and Groot lanes. Source-only: tests, CI, native restart and human acceptance were not verified.
kazemek21
left a comment
There was a problem hiding this comment.
Posted by Codex (gpt-6.1-sol) on behalf of kylez.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes’s GitHub account.
Verdict: changes requested. There are four blocking P2s. Three are replies on the existing threads at follow-up-queue.ts:338, follow-up-queue.ts:413 and follow-up-message.ts:35, each confirmed with a source trace or a failing probe. The fourth is a new inline comment at runner.ts:575. Three non-blocking UX notes are inline too.
Reviewed head f88a83416b4f74b8a413c76fd338cee46bdc180b against base b6e1941e6e29f2f688ba8e4920c5aaa175397221. Royal Court lanes covered UX/product (Princess Donut), persistence/cross-window (Mongo) and queue/runner integration (Mordecai), plus an independent challenge pass. Validation: hosted CI is green at this head. We added focused Vitest probes for (a) and (b), using the PR's own fixture. They failed as expected and were then discarded. We did not run a real two-window browser session or a native restart.
Merge criteria: fix the four blockers, each with a regression test in follow-up-persistence.test.ts. That covers Remove during the Outbox handoff, a cross-window predecessor, partial agent inventory, and a resume the runner forgets. Native restart and human acceptance remain outstanding, per the PR description.
| if (blockers(channelId)) continue; | ||
| const first = entries.find((entry) => entry.channelId === channelId); | ||
| if (first && !first.error) release(first); | ||
| if (first && !first.error && !first.restored) void release(first); |
There was a problem hiding this comment.
🤖 Optional (UX): a paused restored row silently holds back new messages to an idle agent. After Don't resume, or a restart with no interrupted turn, a freshly typed message queues behind the paused row and never sends. Its own row has no caption explaining why. Keeping the order seems right; the gap is the explanation. Consider a caption on rows stuck behind a paused one, e.g. "Waiting for the saved message above. Send or remove it first." Whether a fresh message should wait at all is a product call.
| ) | ||
| throw new Error("This chat is no longer available for sending."); | ||
| if (entry.message) restore(); | ||
| if (entry.message && (!storage || storageError)) |
There was a problem hiding this comment.
🤖 Optional (UX/recovery): one unreadable saved record (follow-up-storage.ts:54) sets storageError. That error blocks every queued-mode Solo send in the scope, even to an idle agent, and the alert appears in every Solo chat with no in-app way to clear it. Consider quarantining or skipping invalid records while still loading the rest, or offering a "Discard unreadable saved message" action. The test at follow-up-persistence.test.ts:386 currently asserts the hard failure.
| /> | ||
| {entry.restored && !entry.error && ( | ||
| <p className="text-caption"> | ||
| Saved before restart. Send when ready. |
There was a problem hiding this comment.
🤖 Optional (copy): a row restored with sending: true but no Outbox receipt still shows "Saved before restart. Send when ready." Pressing Send then reports "may already be sending in another window. Reload…" That's wrong with only one window open, and reloading just shows it again. Consider showing the "may already have been sent, check the conversation" state as soon as the row loads with sending set.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74d6b2f43a
ℹ️ 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".
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account. Two additional fix regressions remain: a P2 that leaves abandoned handoffs unremovable in another window, and a P3 that leaves a sent row stuck after runner abandonment; details inline. I also confirm the existing manual-Send finding.
Reviewed head 74d6b2f against base a3a6223 with independent Rocket, Mantis and Groot lanes. Source-only: no tests or app execution; CI, real two-window behavior, native restart and human acceptance were not verified.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage. The previous Send-deferral and pause-cleanup findings are repaired; the abandoned-handoff fix has one remaining P3 described inline. Reviewed head 0d742cbb41f46d6d525add3578a68b721979205b against base a3a6223d7eb47d38d6c41b2ed6e6d688619cd100, focusing on the changes since 74d6b2f43. Source-only: no tests or app execution; native restart, rendered/focus behavior, performance and human acceptance remain unverified.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad368ce081
ℹ️ 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".
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage. No further changes requested: the prior Remove-recovery finding is repaired by refreshing lock ownership after every restore, while preserving the durable sending marker and removal guard. Reviewed head ad368ce0814ef66ec2fbc163d9a116c5b9fc1809 against base a3a6223d7eb47d38d6c41b2ed6e6d688619cd100, focusing on the fix since 0d742cbb41. Source-only: no tests or app execution; hosted CI required passed, but native restart, rendered/focus behavior, performance and human acceptance remain unverified.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes’s GitHub account.
Verdict: one P2 remains, plus one P3. All four blockers from our last review are fixed at this head: the Remove handoff guard, the cross-window predecessor pause, waits on dropped resumes, and partial-inventory deferral. The P2 is a regression introduced by the predecessor-loss fix: removing a later row stalls the chat in other windows. The P3 is a narrow Remove race outside the lock. Both confirm the existing Codex threads.
Reviewed head ad368ce0814ef66ec2fbc163d9a116c5b9fc1809 against base a3a6223d7eb47d38d6c41b2ed6e6d688619cd100, focused on f88a834..ad368ce. Royal Court lanes: persistence/cross-window (Mongo), queue/runner integration (Mordecai), UX (Princess Donut). Validation: hosted CI is green at this head. We ran focused Vitest probes for both findings. Each failed at this head and was then discarded. The lock race is modeled in jsdom, not run in two real windows. No native restart was run.
Our three earlier optional UX notes are still open and remain optional: a paused row holding back new messages, an unreadable record blocking the scope, and the "another window" copy.
Merge criteria: pause only the rows after a removed predecessor, with a two-window regression test. The P3 is a one-line change and recommended. Native restart and human acceptance remain outstanding, per the PR description.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
Retain locally owned delivery on failed saves while discarding receipts removed by another window. Relinquish incremental journal ownership across in-flight writes so stale windows cannot delete renewed operations. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6831c3fe6
ℹ️ 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".
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage. The ordinary stale-receipt case is repaired, but one P2 remains: dismissal after the fresh lookup can still let Send consume a queued message without a durable receipt. Details inline corroborate the existing report and narrow its failure conditions.
Reviewed head b6831c3fe6c0b61dff90b35a3c8e4979d6f61cbc against base d58bb14a1bf3c577081ffd3e7c42830f43ee1016, focusing on changes since 899196ae22696c9f3d97a376b92001282faacf26. Source-only: no tests or app execution. The hosted required checks passed; Windows native validation was skipped. Test counts/timings were inspected, but no comparable baseline establishes a test-cost regression conclusion. Native restart, actual cross-window recovery, rendered focus and human acceptance remain unverified. This COMMENT review is not approval.
Hold receipt ownership through durable recovery and queued-row consumption. Preserve matching shared receipts without rewriting newer delivery status, and force persistence only for missing locally owned intent. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e15988fa1
ℹ️ 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".
| .find((row) => row.channelId === entry.channelId); | ||
| if ( | ||
| !requested.has(entry.id) && | ||
| (first?.id !== entry.id || |
There was a problem hiding this comment.
When two windows have stale, disjoint snapshots of the same chat, the window holding a later row can acquire the chat lock while the earlier row still exists; first?.id !== entry.id then marks the later row resumeCancelled with a sticky error. Even after the earlier row completes and is removed, pump() will never retry that successor, so the documented automatic FIFO drain in docs/agents.md:423-436 stalls until the user manually clicks Send. Treat an extant canonical predecessor as a temporary wait and reserve the permanent pause for a removed predecessor whose completion cannot be established.
AGENTS.md reference: AGENTS.md:L90-L93
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Non-blocking from our side. This behavior was already present at 0b554fb83, our last review. docs/agents.md documents that a window which cannot establish its predecessor's completion pauses competing rows for an explicit Send, and the test at follow-up-persistence.test.ts:1293 expects that. Retrying automatically once a known predecessor row still in storage advances would be nicer, but that is a product call for the author and Wes rather than a defect against the stated contract.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage. No new actionable findings in this receipt-adoption follow-up. The previous lookup-to-consumption data-loss finding is addressed: adoption and dismissal share the receipt lock, missing hydrated receipts retain the queue row, and missing locally owned receipts force a durable write before consumption.
Reviewed head 3e15988fa16803956ad18179724b9a1ce724f535 against base d58bb14a1bf3c577081ffd3e7c42830f43ee1016, focusing on changes since b6831c3fe6c0b61dff90b35a3c8e4979d6f61cbc. The competing-window explicit-Send behavior remains the documented, tested policy; this review does not request changing it.
Source-only: no tests, builds or app execution. The three added browser scenarios target real IndexedDB/Web Locks; their source and the author’s reported validation were inspected, not independently executed. Hosted CI was still running in the snapshot; Windows native validation was skipped. Native restart, actual multiwindow recovery, rendered focus and human acceptance remain unverified. This COMMENT review is not approval.
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Posted by Will's AI agent, from a source review of 3e15988fa.
Requesting changes. We found no path that loses a queued message or sends it twice. Three new ordering/pause defects block approval. Each was reproduced with a targeted test against the unchanged code at this head, and each has an inline comment:
- Automatic release can publish a row that was returned to paused while the predecessor liveness check was pending (
follow-up-queue.ts:497). - A recovery rejected because another window dismissed the receipt leaves behind a wait that never resolves, so the successor can't drain (
follow-up-queue.ts:453). - A fresh queue-mode submission skips the shared handoff guard and can publish ahead of another window's unfinished message (
follow-up-queue.ts:820).
These existing threads still hold at this head, and we'd treat them as blocking too:
- The stale-snapshot pause at
follow-up-queue.ts:492. The later row is permanently paused (restored,resumeCancelled,error) and never retries automatically, even after the earlier row completes. Suggest separating "a known earlier queued row", which should retry once it advances, from "an uncertain handed-off predecessor", which should pause for a decision. - The
setTimeout(0)before the negative assertions atfollow-up-persistence.test.ts:827. Please await the actual validation/release boundary instead.
Non-blocking, still worth fixing in this PR:
docs/buzz-help.md(bundled into the app's help tool) doesn't cover saved follow-ups. A short section on Resume / Don't resume / Send / Remove and their per-account/device scope would cover it.- The two existing UX threads still apply. One unreadable saved record blocks every queued-mode send, with no in-app way to resolve it. The stale pause error suppresses "Sending…" during a manual Send.
Scope. Most of the diff is test coverage that protects real concurrency cases, and it's worth keeping. Two pieces look independent of queue persistence and would be easier to review and land separately:
- The timeline anchoring change in
ChannelTimeline.tsx, its restoration tests, the extended Virtua patch, and the lockfile hash. This is a separate shared scrolling contract with its own native-acceptance gap. - The fixture repairs in
live-status.spec.mjs(manual Retry),solo-rename.spec.mjs(clock), andattachment-upload.test.mjs.
Structure. The three records (saved row, handoff marker, Outbox receipt) each earn their place. The defects above come from their transitions being spread out. Fresh submits and queued releases use different admission paths, recovery waits are installed outside a successful adoption, and the final guards come before another await. Consolidating those three points, plus a short note documenting lock order (chat, then row, then receipt), would make the invariants much easier to keep. A rewrite isn't needed.
Native restart and real multi-window behavior are still unverified on our side as well.
| ); | ||
| watch(); | ||
| if ( | ||
| (validated || !entry.beforeRelease) && |
There was a problem hiding this comment.
🤖 Fresh submissions skip the shared handoff guard. This fast path calls sent(entry) directly at L824, outside the chat lock. It only checks local blockers() and local entries, and never looks at the durable handoff marker that the release path now respects.
Scenario: window 1 hands off A and removes its saved row, and A hasn't completed. Window 2 hasn't received the activity signal yet, so its storage reload shows no queued rows. A fresh message typed in window 2 publishes immediately, overlapping A's live, incomplete handoff. Automatic release explicitly prevents that overlap, and the user chose no override.
Suggested fix: in queue mode, send fresh submissions through the same chat-scoped admission/handoff decision as queued releases, and persist a row when immediate release isn't safe. Lack of local activity shouldn't count as shared completion. Behavior outside queue mode can stay as is.
There was a problem hiding this comment.
🤖 Non-blocking from our side. The path is reachable: a normal no-attachment composer send passes validated=true (MessageComposer.tsx:2697). But the same fast path exists on base d58bb14a1, which had no cross-window ordering at all, and docs/agents.md scopes the new guard to competing saved rows. This is worth a follow-up if fresh queue-mode sends should respect the shared marker, but this PR doesn't introduce the regression.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account.
Verdict: three P2s remain at 3e15988fa. Both findings from our last review are fixed, and each has a regression that matches our repro: the stale handoff marker (r4432242348) and the cross-window Remove (r4432242355).
Blocking (details are in each thread):
- r4444713677: release publishes a row that was paused while the liveness check was pending. A runtime probe reproduces it.
- r4444713685: a rejected recovery leaves a wait that strands the successor. Reproduced. The thread includes a caution about the obvious fix.
- r4434870116:
docs/buzz-help.mddoes not cover saved follow-ups, which the repository standard requires.
Non-blocking:
- r4444713693: inherited from base.
- r4434870113: matches the documented conservative pause, so this is a product call.
- r4432543513: P3 test barrier.
- Timeline and Virtua: no regressions since
0b554fb83. The new restore tests fail against the old hold logic, which confirms r4428097424 is fixed. - Optional: we agree with Will that the timeline anchoring and the unrelated fixture repairs (
solo-rename,attachment-upload) would be easier to land separately.
Validation:
- Focused Vitest probes on the real queue and Outbox fixture at this head.
ChannelTimeline.restoreandvirtua-compensationsuites: 112/112 pass.- Hosted CI is green, and Windows native validation was skipped.
- Native restart and real multi-window behavior remain unverified.
Exit criteria: fix both queue races with regressions gated at those boundaries, and add the buzz-help section.
Register recovered delivery waits only after durable adoption and retire dismissed event waits before replacement. Document saved Solo follow-up recovery. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab01356653
ℹ️ 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".
| ); | ||
| // Exceed the real configured budget in one chunk, before spooling to disk. | ||
| // The meter's cumulative behavior is covered separately with a small limit. | ||
| const req = Readable.from([Buffer.alloc(UPLOAD_MAX_BYTES + 1)]); |
There was a problem hiding this comment.
When this browser-host test runs, UPLOAD_MAX_BYTES is 500 MiB, so each loop allocates a contiguous 500 MiB buffer inside the test worker; that creates substantial peak memory pressure and can OOM constrained CI runners. Keep the declared-length case body small and stream the undeclared case through bounded chunks, as the previous generator did, while retaining the new small-limit cumulative-meter test.
AGENTS.md reference: AGENTS.md:L17-L18
Useful? React with 👍 / 👎.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account.
Verdict: no remaining blockers from our side at ab0135665. All three P2s from our 3e15988fa review are fixed. Each thread has the evidence: r4444713677, r4444713685, r4434870116. Independent passes by Mongo and Mordecai found no new defect in this commit.
Still open and not blocking: r4444713693 (inherited from base), r4434870113 (product call on the documented pause) and r4432543513 (P3 test barrier). Will's review still requests changes on his own threads.
Validation at this head: gated runtime probes for both races; follow-up-persistence, MessageComposer and src/features/relay/ (128 files, 3,088 tests passed, 4 skipped); hosted CI green, with Windows native skipped. Native restart, real multi-window behavior and human acceptance remain unverified.
This COMMENT review is not approval.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source follow-up via Wes’s account, with independent Rocket, Mantis and Groot coverage. No new actionable findings in the merge integration since ab01356. The timeline resolution preserves correction attribution and fractional anchor position; the queue persistence and Outbox changes remain intact. Existing reviewer threads remain separate from this follow-up.
Reviewed head 4e8847e against base 3e94f2a. The public description has no attachments and its validation claims identify earlier revisions. Source-only: no local tests, builds or app execution. Hosted CI required passed; Windows native validation was skipped. Native restart, real multi-window recovery, rendered focus/scroll behavior, performance and human acceptance remain unverified. This COMMENT review is not approval.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69fe40726d
ℹ️ 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".
| if (!receipt && entry.deliveryId) { | ||
| entry.dismissedDelivery = entry.deliveryId; | ||
| await clearDismissed(entry); | ||
| return; | ||
| } | ||
| entry.deliveryId = receipt?.event.id; | ||
| entry.sending = !!entry.deliveryId; |
There was a problem hiding this comment.
When dismissal cleanup fails and the app restarts, the saved row still has sending: true, but the dismissed Outbox receipt and the in-memory deliveryId are both gone. Fresh evidence beyond the earlier dismissal fix is this restart path: refreshSending() sets only entry.sending to false when no receipt is found, leaving the durable flag intact; the next manual Send then hits storage.isSending(entry.id) in release() and can never retry, although Remove remains possible. Once the row lock and shared-journal lookup establish that the former owner is gone, clear or otherwise retire the stale persisted sending marker so the retained message can be sent again.
Useful? React with 👍 / 👎.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source follow-up via Wes’s account, with independent Rocket, Mantis and Groot coverage. No new actionable findings in the merge integration since 4e8847e. The 21 feature patches are unchanged; the upload-test resolution preserves cumulative-budget coverage, and the changed composer/timeline and agent-service callers retain the queue and recovery integration.
Reviewed head 69fe407 against base c27adc9. The public description has no attached images and identifies earlier revisions for its validation claims. Source-only: no tests, builds or app execution; current CI, native restart, real multi-window recovery, rendered focus/scroll behavior, accessibility and performance remain unverified. This COMMENT review is not approval.
Port the test-only fixture correction from #2250 at 2682dc9. Preserve drag step, handover, and anchor assertions. Co-authored-by: Kalvin Chau <kalvin@block.xyz> Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source follow-up via Wes’s account, with independent Rocket, Mantis and Groot coverage. No new actionable findings in the test-only change since 69fe407. Production code is unchanged. The shorter viewport and adaptive drag distance preserve the minimum six steps, required anchor handover, per-step same-message/under-4px checks, and pointer cleanup on failure; no browser cases or assertions were removed.
Reviewed head 7c19e21 against base c27adc9. Public description and commit text inspected; no attached images. Source-only: no local tests, builds or app execution. The hosted-check snapshot for this head reports success except skipped Windows validation. Run 41715083173 reports this case passing 10/10 per engine without retries on synthetic merge da8e8d6; one shard report includes additional git-diff metadata, and no before/after timing comparison was established. These results do not establish native restart, real multi-window recovery, rendered focus/accessibility, or the absence of intermittent failures. This COMMENT review is not approval.
Keep locked receipt adoption and atomic invitation replacement. Adapt the queued-save fixture to a receipt whose recovery is not already durable. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account.
Verdict: no blockers in the merge integration at 86eeb7c06. Since 7c19e21b0 (already clear), this head only merges main 92d4c36c4. Mongo and Mordecai each reviewed it independently.
Checked:
- Outbox: main's legacy-invitation retirement and atomic replacement work together with this PR's lock-held
recover,recovery(key),restoredReceipts/forgetand the forced incremental save. The dirty check still covers every persistedOutgoingEventfield. The replacement stays out of the snapshot until its save succeeds. The adapted "does not expose replacement intent" test still exercises main's original invariant (1 passed, focused run). - Composer/agents: failed or retried uploads (#2265) and rejected pastes (#2271) cannot reach a saved follow-up as incomplete attachments, because queue saves only receive completed descriptors. Removing an agent (#1855) deletes its session prompts but keeps saved follow-ups, which recheck recipients on release. That is the existing contract.
Non-blocking, inherited from main (#2107), not introduced here: two windows that hydrate the same expired legacy invitation can each create a replacement. That leaves two durable operations holding member-add:<channel>:<person>. The legacy acknowledge branch trusts the window's local predecessor and never takes the shared receipt lock or re-reads storage. A real-IndexedDB probe (two Outbox instances, offline publish, Members recover→acknowledge sequence) reproduces it in Chromium and WebKit, both at this head and on main 92d4c36c4. The fix belongs in a separate PR: retire under the receipt lock and revalidate shared predecessor/successor ownership inside it. Locking alone misses the sequential case.
Validation gaps: source review plus targeted probes only; no broad local suites. Hosted CI at this head is still running. One failure so far: message-overlays.spec.mjs:706 (chromium 8/9), where the hover preview never opened. That happens before any reaction is sent, and the file is outside this diff. I have not established that it's a flake. Native restart, real multi-window behavior and human acceptance remain unverified.
This COMMENT review is not approval.
Co-authored-by: Codex <noreply@openai.com> * origin/main: (327 commits) feat(bestie): remove your Bestie, and let remote agents be Bestie after a warning (#2041) feat(messages): open the sender's profile from the byline name (#2253) feat(native-mcp): one-command launcher and MCP registration for agent driving (#2272) perf(composer): stop per-keystroke chip re-renders and reparses in long rich drafts (#2251) fix(agents): prevent history from forging prompt requests (#2223) Enable automatic key backup in preview releases (#2325) feat(read-state): n-blob NIP-RS sync with a 30-day floor, frontier only (#1286) feat(huddles): provide read-only huddleStatus for optional consumers (#2327) feat(search): show verified names in people search (#2322) Fix voice-note sending and restore waveform previews (#2283) Preserve Solo queued messages across restarts (#1704) feat(plugins): resolve attachment renderers by order, like panels and links (#2284) fix(channels): keep muted DM names from becoming bold (#2184) fix: command-click opening desktop message links in the browser (#2288) AIDA-2390: Explain oversized paste rejection (#2271) AIDA-2399: Make failed uploads easy to retry or remove (#2265) AIDA-2338: Keep sidebar headings readable at large sizes (#2263) chore(plugins): export added-tool types and clarify upload docs (#2292) feat(solo): open chats for community viewing (#2107) feat(dev): read BUZZ_ settings from .env.local into the desktop launch environment (#2254) ... Signed-off-by: Daniel Rouleau <drouleau@squareup.com> # Conflicts: # docs/browser-testing.md # docs/local-artifact-storage.md # src/app/services.ts # src/features/artifact-storage/service.test.ts # src/features/artifact-storage/service.ts # src/features/artifact-storage/store.ts # src/features/communities/service.ts # tests/browser/artifact-storage.spec.mjs # tests/integration/browser-ci.test.mjs
…w-tab * origin/main: (21 commits) fix: prevent false leave failures after membership updates (#2310) feat(bestie): remove your Bestie, and let remote agents be Bestie after a warning (#2041) feat(messages): open the sender's profile from the byline name (#2253) feat(native-mcp): one-command launcher and MCP registration for agent driving (#2272) perf(composer): stop per-keystroke chip re-renders and reparses in long rich drafts (#2251) fix(agents): prevent history from forging prompt requests (#2223) Enable automatic key backup in preview releases (#2325) feat(read-state): n-blob NIP-RS sync with a 30-day floor, frontier only (#1286) feat(huddles): provide read-only huddleStatus for optional consumers (#2327) feat(search): show verified names in people search (#2322) Fix voice-note sending and restore waveform previews (#2283) Preserve Solo queued messages across restarts (#1704) feat(plugins): resolve attachment renderers by order, like panels and links (#2284) fix(channels): keep muted DM names from becoming bold (#2184) fix: command-click opening desktop message links in the browser (#2288) AIDA-2390: Explain oversized paste rejection (#2271) AIDA-2399: Make failed uploads easy to retry or remove (#2265) AIDA-2338: Keep sidebar headings readable at large sizes (#2263) chore(plugins): export added-tool types and clarify upload docs (#2292) feat(solo): open chats for community viewing (#2107) ... # Conflicts: # src/features/relay/outbox.ts
* origin/main: (26 commits) fix(identity): read FI backup status through the native forwarder (#2344) fix(relay): show the relay's reason when it refuses a write (#2031) perf(relay): look up archived identities in a per-snapshot set (#2359) fix(messages): keep link previews clear of message hover actions (#2351) fix(security): close workflow, Canvas, upload, and broker gaps (#2360) fix: prevent false leave failures after membership updates (#2310) feat(bestie): remove your Bestie, and let remote agents be Bestie after a warning (#2041) feat(messages): open the sender's profile from the byline name (#2253) feat(native-mcp): one-command launcher and MCP registration for agent driving (#2272) perf(composer): stop per-keystroke chip re-renders and reparses in long rich drafts (#2251) fix(agents): prevent history from forging prompt requests (#2223) Enable automatic key backup in preview releases (#2325) feat(read-state): n-blob NIP-RS sync with a 30-day floor, frontier only (#1286) feat(huddles): provide read-only huddleStatus for optional consumers (#2327) feat(search): show verified names in people search (#2322) Fix voice-note sending and restore waveform previews (#2283) Preserve Solo queued messages across restarts (#1704) feat(plugins): resolve attachment renderers by order, like panels and links (#2284) fix(channels): keep muted DM names from becoming bold (#2184) fix: command-click opening desktop message links in the browser (#2288) ... Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Tyler Longwell <tlongwell@squareup.com> Signed-off-by: Tyler Longwell <tlongwell@squareup.com> # Conflicts: # src/bundled/huddles/index.tsx # src/features/huddle/service.test.ts
Solo follow-ups queued above the composer survive restart with their text, completed attachments, references, and model/effort choices. **Resume** waits for interrupted requests before releasing saved messages one at a time; **Don’t resume** leaves saved rows paused. Fresh submissions remain distinct from restored rows. Release revalidates membership and recipient availability and coordinates windows through the existing chat/row locks and durable Outbox receipts. Completion evidence releases successors; uncertain live predecessors remain paused. Dismissal and abandonment retire only the matching handoff responsibility, while window liveness prevents closed senders from blocking fresh queues. Resume and receipt retries transfer ownership. Remove checks the shared journal before enabling deletion, including after recovery-save failure. Timeline anchoring now distinguishes Virtua’s automatic corrections from reader movement, including both coalesced into one native scroll event. The existing hold preserves fractional geometry, unmounted anchors, and retargeting after the reader moves. Manual-Retry fixture fixes from earlier commits remain included. Validation at `3e15988fa`: - Full mandatory push gate passed: 841 test files, 14,548 tests passed and 20 skipped (1014.02s), plus typechecking, design checks, and push guards. - All 621 tests across the complete queue persistence, composer, and three Outbox files pass (66.46s). Gated incremental-journal regressions reproduce both dismissal-after-lookup and local-owner data loss before the fix. - All 24 cases in the complete queue and Outbox-storage browser files pass across Chromium/WebKit (59.3s), including both adoption/dismissal orderings and preservation of concurrent accepted status in real IndexedDB. - Receipt adoption and dismissal now share a lock. Queue consumption occurs synchronously under that lock only after durable ownership is established. Existing matching receipts are not rewritten; missing hydrated receipts retain the queue row. Independent review is clear. Earlier validation at `b6831c3fe`: - Full mandatory push gate passed: 841 test files, 14,546 tests passed and 20 skipped (731.46s), plus typechecking, design checks, and push guards. - All 481 composer tests pass after adding the recovery lookup to its Outbox fixture. - All 138 tests across the complete queue persistence and three Outbox files pass (7.45s). The stale-window regression fails before the fix against an incremental journal fixture. Manual Send and storage-event recovery retain the message; local failed saves, Retry ownership, and newer shared receipts stay protected. - All 16 existing queue browser cases pass across Chromium/WebKit. Two new real IndexedDB cases pass (2.6s) and fail without the in-flight ownership guard: a stale window cannot delete another window's renewed receipt after forgetting its old copy. - Hydrated receipts are checked against shared storage before a queue row is consumed. Stale receipts release their local delivery fence and incremental deletion ownership; locally owned unsaved delivery remains protected. Independent review is clear. - Native restart and real-window recovery remain unverified; browser storage checks cover the IndexedDB boundary, not native acceptance. Earlier validation at `899196ae2`: - Full mandatory push gate passed: 841 test files, 14,539 tests passed and 20 skipped (746.44s), plus typechecking and design checks. - All 73 queue persistence tests pass (7.48s). The original failed-dismissal regression failed before the repair. Coverage includes sending the retained row after dismissal and restoration, retrying cleanup after storage errors through Send or Remove, releasing a waiting successor, and preserving newer handoff/receipt ownership. Independent queue review is clear. - All 16 queue browser cases pass across Chromium and WebKit on the same production sources (40.5s). No browser tests were added in this follow-up. - Durable dismissal clears only the matching saved sending flag under the existing locks. Cleanup failure retains evidence for a later Send/Remove retry and renders the storage error. An older completion cannot overwrite a newer event’s handoff. - At `bd6314458`, all 50 history/live-status browser cases passed across Chromium and WebKit, alongside 67 timeline restoration tests. Its temporary expired-route probe passed four cases; no assertions, timeouts, or retries were weakened. - This PR adds ten browser scenarios in total; none were removed. The required gate also exposed two existing fixture failures: a month-boundary calendar assertion and a 500 MiB upload spool exceeding the unchanged timeout. The follow-up ports the date fixture fix from main (`802bf5aaf`) and preserves actual upload-limit and cumulative-stream coverage without writing that disk-sized fixture. Both complete files passed locally: 46 tests in 5.45s, versus 2 failures/43 passes in 5.63s before the fixture repairs. Existing shared Button/IconButton controls are retained. Native restart, native scrolling, and renewed human acceptance remain unverified. Hosted CI is separate from the local evidence above. The PR’s ready-for-review status is preserved. --------- Signed-off-by: klopez4212 <klopez4212@gmail.com> Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Co-authored-by: Kalvin Chau <kalvin@block.xyz> Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
## TL;DR Any agent running on this device can now read and change Buzz settings when you ask it to, through generic `settings_list`, `settings_get` and `settings_set` MCP tools. There is no per-agent switch and no UI change. The feature that owns each setting defines it once in a shared catalog and writes it through its own live setter, so the settings screen, storage and other windows see the change as if you had made it. ## What agents can change | Key | Values | Owner | | --- | --- | --- | | `appearance.mode` | `system`, `light`, `dark` | `shared/theme/settings.ts` | | `appearance.interface_size` | 80–200, steps of 10 (percent) | same | | `appearance.accent` | the accent ids | same | | `appearance.vibe` | `low` (Chillin’), `high` (Buzzin’) | same | | `conversations.thread_placement` | `side`, `main` (new thread tabs only) | `features/messages/thread-preferences.ts` | | `conversations.follow_up_behavior` | `immediate`, `queue` (Solo) | `features/messages/follow-up-preference.ts` | | `conversations.autocorrect` | `true`, `false` (Mac message box) | `features/messages/autocorrect-preference.ts` | | `agents.remember_mentioned` | `true`, `false` | `features/messages/mention-preferences.ts` | `src/app/device-settings.ts` composes the catalog. `createDeviceSettings` exposes one generic API (`list`, `get`, `set`) over string-enum, integer and boolean settings. Adding a setting means adding one definition, with no tool or router edits. Values must match exactly, with no aliases, case folding or clamping. Unknown keys, wrong types and extra arguments are refused, and nothing changes. Each write is read back. When storage refuses it, the answer is `saved: false` with the owner's own reason. **Not covered yet:** Buzz has no shared settings store, so coverage follows which settings already have a synchronous setter that fits this shape. - Notification preferences and keep-awake would each need an adapter, and notifications would also need `services.ts` construction order changed. - Shortcuts take object values. - Plugins' own settings, such as Focus, exist only inside the plugin. - Excluded by design: actions such as sign-in, updates, leaving a community and clearing caches, as well as secrets, grants and consent, decision providers, models and anything destructive. ## Who gets the tools - **Every agent run.** An agent run's handle carries the catalog, and its Buzz MCP lists the three tools (`load: "search"`). Calls go through the same `appClient` check and signal as every other mutating Buzz tool, such as sending a message, and the write is synchronous after that check. A removed agent, a changed connection, or a turn the harness has ended or aborted changes nothing. Stop behaves as it does for those tools in each harness. Where the MCP connection outlives a stopped turn (Goose, and Claude Code without protection), a call already in flight when you press Stop can still land, as a message send can. - **Not plugins or external connections.** A plugin's outside-run `Agents.handle` has no settings. Tools a plugin adds get the agent's client without `settings`. External MCP connections use that non-run handle and also get none. ## Revision An earlier revision gated this behind a per-agent, off-by-default **Change Buzz settings** switch in Agent details. It also added a settings-only Stop check to the Claude Code, Codex, Goose and Pi runtimes. Following product direction, both are removed. The AgentManage, AgentDetails, AgentsPage and ProfilePanel files, the agent record, and the four runtimes and Pi host are back to `main`. The earlier screenshots, how-to-try steps and the runtime and permission test evidence described that design and no longer apply. ## Overlapping work This PR does not build on any of these and changes none of their behavior: - #2211: external auth and catalog. - #1878 and #839 (now on main): Claude and Goose protection, whose pre-tool checks still review a settings call. - #2150 and #2186: execution target. - #2180: themes. Theme selection is left out of the catalog. - #1704: queued follow-ups. This PR only exposes the existing behavior preference. ## Validation Run on a Blox Linux workstation against this exact tree: - `tsc --noEmit` and biome on the touched files pass. - vitest passes for 10 files (922 tests, after rebasing onto #2280). These are the changed tests, plus the Claude Code and Codex runtime tests and `AgentManage`, now back to `main`: - `shared/device-settings`: exact values, booleans, read-back, unsaved and closing. - `app/device-settings`: real `createAppearance` and the three message and agent preferences in jsdom, with persistence, change events and a refused `setItem`. - `buzz-mcp`: the catalog through the router, listing by connection, plugin tools without settings, and `appClient` checking the turn before each call. - `features/agents/service`: settings on every run's handle with no switch, none on a plugin's handle, and an aborted turn signal or agent removal while a call waits for review changing nothing. No browser journeys were added or removed. Deferred: the native app hasn't been run with a live agent turn. The steps below cover that. ## How to try it 1. Ask any agent running on this device to switch Buzz to dark mode. The app should switch at once, and **Settings → Appearance** should show Dark. 2. Ask it to open new threads in the main pane, then open a thread. It should open in the main pane, while open threads stay put. 3. Ask it to queue Solo follow-ups. **Settings → Appearance → Follow-up behavior** should show Queue. 4. Ask it for something outside the list, such as a theme. It should say that setting isn't available, and nothing should change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Kyle Zemek <kylez@squareup.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Solo follow-ups queued above the composer survive restart with their text, completed attachments, references, and model/effort choices. Resume waits for interrupted requests before releasing saved messages one at a time; Don’t resume leaves saved rows paused. Fresh submissions remain distinct from restored rows.
Release revalidates membership and recipient availability and coordinates windows through the existing chat/row locks and durable Outbox receipts. Completion evidence releases successors; uncertain live predecessors remain paused. Dismissal and abandonment retire only the matching handoff responsibility, while window liveness prevents closed senders from blocking fresh queues. Resume and receipt retries transfer ownership. Remove checks the shared journal before enabling deletion, including after recovery-save failure.
Timeline anchoring now distinguishes Virtua’s automatic corrections from reader movement, including both coalesced into one native scroll event. The existing hold preserves fractional geometry, unmounted anchors, and retargeting after the reader moves. Manual-Retry fixture fixes from earlier commits remain included.
Validation at
3e15988fa:Earlier validation at
b6831c3fe:Earlier validation at
899196ae2:bd6314458, all 50 history/live-status browser cases passed across Chromium and WebKit, alongside 67 timeline restoration tests. Its temporary expired-route probe passed four cases; no assertions, timeouts, or retries were weakened.The required gate also exposed two existing fixture failures: a month-boundary calendar assertion and a 500 MiB upload spool exceeding the unchanged timeout. The follow-up ports the date fixture fix from main (
802bf5aaf) and preserves actual upload-limit and cumulative-stream coverage without writing that disk-sized fixture. Both complete files passed locally: 46 tests in 5.45s, versus 2 failures/43 passes in 5.63s before the fixture repairs.Existing shared Button/IconButton controls are retained. Native restart, native scrolling, and renewed human acceptance remain unverified. Hosted CI is separate from the local evidence above. The PR’s ready-for-review status is preserved.