Skip to content

fix(native-chat): show "Sending…" on a message until the host confirms it - #24606

Merged
brennanb2025 merged 7 commits into
mainfrom
brennanb2025/chat-failed-send-returns-to-composer
Oct 4, 2026
Merged

brennanb2025 merged 7 commits into
mainfrom
brennanb2025/chat-failed-send-returns-to-composer

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​773 $\color{#cf222e}{\Huge{\mathbf{−}}}$​401 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​372
Prod 13 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​172 $\color{#cf222e}{\Huge{\mathbf{−}}}$​47 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​125

ELI5

In Orca's native chat, a message you send waits in a local queue until the agent's host confirms it has the message. If that confirmation gets lost (the connection drops, or you reopen the chat while a message is still sending), Orca sends it again on its own, under the same message id, until the host answers. Today the chat shows a red "Message delivery is unconfirmed." line with a Retry button while that happens, which flashes on every reopen mid-send and asks you to do something Orca is already doing. An earlier version of this PR removed the flash but left the message looking exactly like a delivered one until the resend settled it.

Now every message Orca hasn't seen confirmed shows a quiet grey "Sending…" under it, where its time normally appears, from the moment you send it until the host's record shows it has the message (pending or delivered). There's no red text and no Retry, because Orca is already sending it. Messages that really did fail still show why, as on main: one the host recorded and then rejected shows as not sent, with the host's reason and no Retry, and one the host never recorded keeps its Retry. That includes a send made while the host can't be reached at all: it fails to connect, Orca doesn't resend it, and it shows "Your message was not sent." with Retry, exactly as today.

What Changed

The problem. Every send waits in a local queue (the outbox) until the host that runs the agent answers. When the answer is lost, Orca resends the message automatically, under the same id, with a growing wait between tries (up to 16 seconds), until the host's record for it (its journal row) appears. The host records a message id only once, so the resend can't deliver it twice. Two things went wrong around this:

  • On main, each lost answer showed "Message delivery is unconfirmed." with a Retry until the resend confirmed it, so it flashed on every reopen mid-send, and the Retry only repeated what Orca was already doing.
  • With only the flash removed, a message whose answer was lost looked like any delivered message for as long as the resend ran, which has no limit while each answer keeps getting lost. Messages waiting behind it, and a new message still on its way, also looked delivered.

Before / after (what you see)

Case Before (main) After
A message whose answer was lost (a dropped connection, or the chat reopened mid-send), and the host has no record of it yet Red "Message delivery is unconfirmed." with Retry, usually flashing until the automatic resend confirmed it Grey "Sending…" under the message, in place of its time, until the host has a record of it. It then changes once: it disappears, turns into the host's not-sent line (no Retry) if that record says the message was rejected, turns into the unconfirmed line with Retry if its outcome is unknown, or turns into "Your message was not sent." with Retry if the resend can't connect to the host
A send made while the host can't be reached (it can't connect) "Your message was not sent." with Retry; Orca does not resend it Unchanged, never "Sending…"
A new message on its way, or one waiting behind another message Nothing: it looked like a delivered message "Sending…" until the host's record shows it has the message. On a healthy connection this shows for about one round trip
The host's record says the message is pending or delivered Nothing marked Unchanged, and never "Sending…"
The host's record says the message was rejected, including a message that was back in the queue under the same id Drawn from the host's record as not sent, with the host's reason and no Retry (main's rule since #24710) Unchanged, and never "Sending…"
The host's record says the message's outcome is unknown (including one written after a host restart), and it is first in the queue "Unconfirmed" with Retry Unchanged, and never "Sending…"
A second message whose outcome is unknown, waiting behind the first (for example, two sends in flight when the host restarts) Nothing: it looked like a delivered message "Sending…", until it reaches the front of the queue and shows its own line, or its resend is answered
A message whose answer was lost and that waits for your Retry, or one that was still going out when you pressed Stop "Unconfirmed" with Retry (only you can move it) Unchanged, and never "Sending…"
The Retry you pressed on such a message, while it is on its way Nothing: the line and Retry went away and the message looked delivered "Sending…" until the host answers

Mechanism.

  • structured-agent-session-delivery-notices.ts decides what each queued message's row says. A message in a failure state (rejected, held for Retry, or stopped in front of the queue with no automatic resend) keeps exactly its line and Retry. Every other message now gets a "sending" notice until its journal row says the host has it (pending or accepted). A row with an unknown outcome does not count, so a message you retried or a second message in doubt never looks delivered meanwhile. A rejected row makes the message the host's: main shows it as not sent from that row (or from the queue's copy until the row loads), never as sending. A message whose answer was lost and that Orca is resending on its own counts as sending, not failed; the rule for that, structuredAgentSessionEntryResendsUnconfirmed (new, src/shared/structured-agent-session-outbox-unconfirmed-resend.ts), is the automatic resend's own condition, which use-structured-agent-session-outbox-unconfirmed-probe.ts now reads from there with no change in behaviour.
  • The notices read the queue through the same reconcile step the transcript uses (reconcileStructuredAgentSessionOutboxWithQueue, with the loaded rows, as the transcript passes them). Without it, a resent message whose journal row had just arrived showed the red line for one render before the queue caught up, so the marker would have gone "Sending…" → red line → nothing. With it, the marker changes once.
  • NativeChatDeliveryNotice (in NativeChatMessageRow.tsx) now has two shapes: sending, or a failure line with optional Retry. The user row's time and copy button sit in a small row under the bubble that appears on hover. While the message is sending, that row stays visible with "Sending…" in the time's place, muted, and the copy button keeps its own hover reveal, so the row keeps its height when the marker clears. Everything is derived from the queue and journal rows Orca already holds; nothing new is stored.
  • use-structured-agent-session-delivery-notices.ts (the hook main added to build the notices) gives them the journal's rows whenever the queue is non-empty, as well as whenever a message is shown as not sent (main's rule), since every queued message now needs to know whether its row exists. The rows' list only changes when a batch carries new rows, not on every streamed update. The loaded rows are passed only while the queue holds a message and a rejection exists, the one case the reconcile reads them, so a streaming turn rebuilds no notice otherwise.
  • One new string, components.native-chat.messageSending ("Sending…"), in all six locale catalogs, using each catalog's existing translation of the question card's "Sending…".

No wire, host or saved-field change. Messages saved by an older build are read the same way.

Why

The common pattern for a chat client that quietly resends unsent messages under the same id is a muted "Pending" in place of the message's time until the server acknowledges it: no red text and no Retry. The resend stays quiet, and an unconfirmed message never passes for a delivered one. This PR follows that, and keeps the red line and Retry only for messages that actually failed or that only the user can move.

Alternatives considered:

  • Say nothing while Orca resends (this PR's earlier version): rejected. A message whose answer was lost looked delivered until the resend settled it, with no limit while each answer kept getting lost.
  • Delay the red line with a timer, or cap the resend and then show it: rejected. A timer hides or shows the line whether or not anything is resending, and the Retry it would show only repeats the same resend under the same id.
  • A separate banner or status line for unconfirmed messages: rejected. It would say one state in a second place; the message's own row is where the common pattern puts it.
  • Store an "acknowledged" flag on each saved message: rejected. The journal row already records it, so the marker is derived from it.
  • Give the message back to the composer: rejected for this change. It may already have reached the agent, and sending it again from the composer would use a new id and could deliver it twice.

Differences from the common pattern

  • Follows the common pattern: a send whose answer was lost is resent quietly under the same message id, with a growing wait, and the host records an id only once. Orca's queue is saved, so the resend survives a reload.
  • Follows the common pattern: a muted marker in the message's own row, in place of its time, until the host confirms it, with no red text and no Retry. It shows from the moment the message is sent, so a normal send shows it for about one round trip, as the common pattern does.
  • Intended: the marker is "Sending…", not "Pending". "Sending…" is the plainer word, and the chat already uses it for a question answer on its way.
  • Intended: the marker is shown without hover. Orca's user rows show their time and copy button only on hover, so the marker takes the time's place but stays visible. The copy button stays in the row with its usual hover reveal, as in the common pattern, and the row's height doesn't change when the marker clears.
  • Intended: the marker clears when the host's journal row says it has the message (pending or accepted), not when the send's own reply arrives. The journal row is the host's record that Orca already reads for every other delivery state, and a pending row is the same point at which the common pattern stops showing its marker. A reply that says the host has the message but is waiting on an earlier turn keeps "Sending…" until that row arrives, normally at the same moment.
  • Intended: no limit on resend tries while each answer is lost. The marker stays for as long as the message is unconfirmed, so it never reads as delivered. This covers lost answers and a reopen mid-send, not a host outage: a send or resend that can't connect to the host fails and shows "Your message was not sent." with Retry.
  • Intended: on a fast connection a new message shows "Sending…" for a frame or two before its time appears. This is the common pattern's own pending label, not a flash of new UI.
  • Temporary: some sends still show a line and Retry on the message itself, which the common pattern never does. A send the host refused before recording it, or that failed, shows why with Retry (one the host recorded and then rejected shows as not sent with no Retry, main's rule). "Message delivery is unconfirmed." with Retry still shows for a send the host recorded with an unknown outcome, for one you already pressed Retry on, and for one that was still going out when you pressed Stop. Follow-up PR to come: one settled outcome per send, said once, with the text back in an empty composer where it was never delivered.

Linked Issue

No issue.

Visual Proof

Captured on a separate test Mac in hidden, isolated test builds with a stand-in agent. Before = main at 1dc6157614c; after = this PR at 059e79e4188, before main's #24710 merged in. None of the captured states involves a rejected message, which #24710 owns. The app's saved outbox was seeded where a state needed it, and a page observer recorded when each label appeared. The test Mac was heavily loaded by other work, so the timings below are upper bounds.

  • Reopening a chat while a message is mid-send (the host has no record of it yet). Before: "Message delivery is unconfirmed." and Retry under the message for about 1 s. After: a muted "Sending…" under the message from the first paint for about 1 s, never a red line, then it clears once. Delivered once in both.

Before: reopened mid-send message shows "Message delivery is unconfirmed." and Retry
After: same moment, a muted "Sending…" under the message, no red line, no Retry

  • A message that stays unconfirmed for over a minute (it waits behind a message only your Retry can move). Before: the waiting message looks sent. After: it shows "Sending…" the whole time (observed from 1.2 s to past 76 s); the message in front keeps its line and Retry.

Before, 60 s: a message waiting behind a held one looks sent
After, 60 s: the waiting message still shows "Sending…"; the held one keeps its line and Retry

  • Copy while sending, and after it settles. The copy button sits beside "Sending…" on hover, and the row under the message measured 24 px both while sending and after "Sending…" gave way to the time, so nothing below it moves.

After, hover: copy button beside "Sending…"
After, settled: delivered once, "Sending…" replaced by the time

  • A normal send to the local stand-in host showed "Sending…" for about 25–29 ms (one or two frames) before the host recorded it; no screenshot could catch it, so this is from the observer. A host that takes longer to record a message shows it for longer.

No new pop-up, label or line appears in the after build other than "Sending…". Changed from the earlier set: these replace the screenshots taken at 4959afa60b7, before the marker existed. Not captured: a real host outage (a send that can't reach the host shows "Your message was not sent." with Retry, unchanged in both builds; the test setup has no remote host), and a message whose host record has an unknown outcome at the front of the queue (unchanged; covered by unit tests).

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • structured-agent-session-delivery-notices.test.ts: a message whose answer was lost and that has no journal row says only that it is sending, with no Retry; so do a message on its way, one waiting its turn, and one behind the message the queue stopped on. So do a message while the Retry you pressed is queued or on its way (including after a host restart), and the second of two messages in doubt. A message put back in the queue over a rejected row, alone or behind a stopped one, says it was not sent in the host's words with no Retry, never "Sending…", whether the queue's copy or the loaded row draws it. Each stops saying it once the journal holds a pending or accepted row for it, and a resent message whose row just arrived never shows the red line on the way. A message rejected before the host recorded it, held for Retry, unconfirmed with a host record, retried by you, or outlived by a Stop says only its reason, with Retry, and never "Sending…".

  • NativeChatStructuredSessionDelivery.test.tsx: a new send whose answer is lost shows "Sending…" from the moment it is sent, with no line or Retry, is resent with the same request, and the marker goes when it is confirmed; a chat reopened mid-send shows "Sending…" until the resend settles it; with a host record of unknown outcome, or after a Stop, it shows the line and Retry and no "Sending…".

  • NativeChatMessageRow.test.tsx: "Sending…" is muted, sits in the time's place, is visible without hover, and has no Retry; the copy button is there with its hover reveal; once confirmed, the same row holds the time again.

  • Ablations: with the four changed source files put back to the previous head (4959afa60b7 merged with main) and the new tests kept, 8 tests fail, including the lost-answer send showing no "Sending…" in both the unit and the chat-level test. Without the reconcile step in the notices, 2 tests fail, each showing the red "unconfirmed" line on a resent message whose row had just arrived. With the marker ending at any journal row (the rule at b5cc3c83695), 6 tests fail: the three Retry cases, both second-in-doubt cases and the requeued one. With the row at b5cc3c83695, where the marker replaced the whole row, 2 tests fail: the copy button is missing while sending, and the row is not kept.

  • Ran 29 outbox, delivery-notice, send-disposition, message-row, message-list and structured-chat test files (317/317 passed), plus the 8 locale catalog test files (53/53).

  • After merging main's fix(native-chat): show a message Orca accepted and then failed to deliver as "Not sent" in the chat #24710: ran 49 files (the 29 above, the split-out resend suite, and every native-chat and shared projection/outbox test file fix(native-chat): show a message Orca accepted and then failed to deliver as "Not sent" in the chat #24710 added or changed): 410/410 passed. With the notices hook passing the journal's rows only while a message is rejected (main's rule alone), 4 chat-level tests fail: a message reopened mid-send, or one the host recorded with an unknown outcome, never shows its "unconfirmed" line and Retry once that row arrives.

  • oxlint, the changed-code quality gate (including React Doctor and the design system) and the localization checks pass. The web and node typechecks ran through the machine's typecheck queue after the merge; the only errors are "cannot find module" for stream-json / stream-chain, a dependency main added that is missing from the local install, in files this PR does not touch.

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Renderer-only. The same on macOS, Linux, Windows and SSH hosts. The "Sending…" marker covers a send whose answer was lost and a chat reopened mid-send; it is not an outage safety net. When the host can't be reached, a send fails to connect, Orca does not resend it, and its message shows "Your message was not sent." with Retry, as on main. A later follow-up changes that so the text goes back to the composer instead. The outage cue is the host-scoped notice from the separate open PR #24898, which says in the chat that the host is reconnecting or offline. Each PR is complete on its own: with or without #24898, every message in Orca's send queue shows either why it did not go through or "Sending…" until the host's record says it has the message (pending or delivered). One new string in all six locale catalogs. Mobile is untouched.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Author: @BrennanKB5

@brennanb2025
brennanb2025 marked this pull request as ready for review October 2, 2026 19:18
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8ad26dc7-e9a2-4ccc-b1ea-215194225d14
📥 Commits

Reviewing files that changed from the base of the PR and between 0f5fd45 and b5ce0d6.

📒 Files selected for processing (4)
  • config/reliability-gates.jsonc
  • src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.probe.test.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

Unconfirmed outbox entries can be automatically resent under their existing operation ID when no matching journal submission exists and the entry has not been retried or outlived by Stop. The session displays “Sending…” while delivery remains unresolved. When journal submissions report an unknown outcome, the session displays a notice with a Retry option. Tests cover reopened entries, live and recovered unknown outcomes, Stop history, and probe timing.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to b5ce0

This update reorganizes the delivery probe tests into their own suite and adds them to the reliability gate. No concrete risk to user-facing behavior was found, and the change is ready to merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b5ce0

The inspected changes preserve message identity and the controls that stop automatic retries after a host response, user retry, or Stop. No new security issue was established. Confidence remains limited because recovery across different runtime targets and the exact before-and-after behavior were not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The inspected recovery path forwards an existing session outbox entry to agentSession.send on the supplied runtime target. The shared eligibility helper neither chooses a target nor grants agent, credential, or tool authority. Cross-target ledger sharing and downstream execution exposure were not established.

Trust Boundaries and Controls

  • observed — Automatic transport recovery is distinguished from user-authorized retry: matching journal rows terminate automatic eligibility, and prior explicit retries and Stop marks prevent another automatic replay. The current-session and attached-owner checks remain in the production probe path.

Resilience and Maintainability Implications

  • observed — The target-change test verifies timer restart and a subsequent client call, but does not verify host identity, shared ledger scope, or provider execution across targets. Preserved operation identity and generation invalidation are useful controls without constituting an end-to-end cross-target guarantee.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.24% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 34 functions across 22 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main user-facing change: native chat shows “Sending…” until the host confirms the message.
Description check ✅ Passed The description is comprehensive and explains the behavior, rationale, implementation, visual changes, and testing. It states “No issue,” although the template requires a linked issue for outside cont…
Full details: Docstring Coverage

Explanation

Docstring coverage is 38.24% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 34 functions across 22 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch brennanb2025/chat-failed-send-returns-to-composer
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one minor suggestion inline.

Reviewed changes

  • A provably-unsent send returns to an empty composer — disposeStructuredAgentSessionSendRefusal now drops an entry the host never recorded (structuredAgentSessionRefusalProvesUnsent) when an empty composer shows the chat, hands its text/images back through the existing Stop restore path (restoreWithdrawn.unrecorded, renamed from byStop), and puts the refusal words on the chat's error line once. Launch prompts never return.
  • Settlement rules extracted into structured-agent-session-outbox-settlement.ts — the proof (structuredAgentSessionRefusalProvesUnsent), the lost-outcome predicate (structuredAgentSessionSubmissionOutcomeLost), and structuredAgentSessionEntryConfirmingByItself, now read by the drain, the unconfirmed probe, and the delivery notices.
  • In-doubt sends no longer flash as failed — the probe and the notices suppress a message Orca confirms under its own id; reconcile drops an entry whose journal row is a recovered unknown and says sendOutcomeLost once.
  • Copy and tests — new sendOutcomeLost sentence across the copy module, en-runtime-required.json, and all six locale catalogs; new/updated tests for the composer return, the lost outcome, and the no-notice confirmation.

I verified the proof extraction is behavior-identical to the old inline condition, that retained reads the un-staged wire entry (so a first attempt is distinguished from the staged stored entry), and that the reconcile lost-drop cannot lose the message from the transcript: it only fires when the recovered-unknown submission is present in submissions, whose non-rejected items the journal projection renders. The one gap I found is in the new emptiness check.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025
brennanb2025 marked this pull request as draft October 2, 2026 20:44
@brennanb2025 brennanb2025 changed the title fix(native-chat): a send the host never recorded goes back to the composer fix(native-chat): no "unconfirmed" line while Orca resends a send on its own Oct 2, 2026
…its own

A send whose answer was lost (for example after reopening the chat mid-send)
showed "Message delivery is unconfirmed." with a Retry for the moment before
the automatic resend under the same message id confirmed it. One rule now
decides both: the resend runs, and the notice stays quiet, only while the
send's answer is lost, the user never retried it, it was not still going out
when the user pressed Stop, and the host has no record of it yet. Once the
host has any record of it, the line and Retry are exactly as before.
@brennanb2025
brennanb2025 force-pushed the brennanb2025/chat-failed-send-returns-to-composer branch from af8b9a9 to 4959afa Compare October 2, 2026 23:05
@brennanb2025
brennanb2025 marked this pull request as ready for review October 2, 2026 23:30

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e8f1426f-a017-41f5-be70-f094f9285f4d
📥 Commits

Reviewing files that changed from the base of the PR and between 08cc8c1 and 4959afa.

📒 Files selected for processing (6)
  • src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx
  • src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.test.ts
  • src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session-outbox-unconfirmed-probe.ts
  • src/shared/structured-agent-session-outbox-unconfirmed-resend.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Comment on lines +158 to +159
if (structuredAgentSessionEntryResendsUnconfirmed(entry, submissions)) {
continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
# Locate the probe owner and check whether a visible session can retain a detached owner.
rg -n -C 8 \
  'useStructuredAgentSessionOutboxUnconfirmedProbe\(|owner\.attached|attached:|isVisible' \
  src/renderer/src/components/native-chat \
  -g '*.ts' -g '*.tsx'

Repository: stablyai/orca

Length of output: 45667


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- probe ---'
cat -n src/renderer/src/components/native-chat/use-structured-agent-session-outbox-unconfirmed-probe.ts | sed -n '1,180p'
printf '%s\n' '--- outbox hook ---'
cat -n src/renderer/src/components/native-chat/use-structured-agent-session-outbox.ts | sed -n '1,290p'
printf '%s\n' '--- notices ---'
cat -n src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts | sed -n '1,230p'
printf '%s\n' '--- owner references ---'
rg -n -C 6 'create.*owner|owner\s*=|attached\s*:|attached\b|useStructuredAgentSessionOutbox\(' src/renderer/src/components/native-chat -g '*.ts' -g '*.tsx' | sed -n '1,500p'
printf '%s\n' '--- notice call sites ---'
rg -n -C 8 'structuredAgentSessionDeliveryNotices|deliveryNotices|structuredAgentSessionEntryResendsUnconfirmed' src/renderer/src/components/native-chat -g '*.ts' -g '*.tsx' | sed -n '1,500p'

Repository: stablyai/orca

Length of output: 42517


🏁 Script executed:

set -eu
echo '--- probe ---'
cat -n src/renderer/src/components/native-chat/use-structured-agent-session-outbox-unconfirmed-probe.ts | sed -n '1,180p'
echo '--- outbox hook ---'
cat -n src/renderer/src/components/native-chat/use-structured-agent-session-outbox.ts | sed -n '1,290p'
echo '--- notices ---'
cat -n src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts | sed -n '1,230p'
echo '--- owner references ---'
rg -n -C 6 'create.*owner|owner\s*=|attached\s*:|attached\b|useStructuredAgentSessionOutbox\(' src/renderer/src/components/native-chat -g '*.ts' -g '*.tsx' | sed -n '1,500p'
echo '--- notice call sites ---'
rg -n -C 8 'structuredAgentSessionDeliveryNotices|deliveryNotices|structuredAgentSessionEntryResendsUnconfirmed' src/renderer/src/components/native-chat -g '*.ts' -g '*.tsx' | sed -n '1,500p'

Repository: stablyai/orca

Length of output: 42578


🏁 Script executed:

set -eu
echo '--- owner hook ---'
cat -n src/renderer/src/runtime/structured-agent-session-accepted-send-capability.ts | sed -n '1,280p'
echo '--- owner hook call sites ---'
rg -n -C 12 'useStructuredAgentSessionOutboxOwnerChange|useStructuredAgentSessionOutbox\(' src/renderer/src -g '*.ts' -g '*.tsx' | sed -n '1,700p'
echo '--- structured session outbox/controller wiring ---'
rg -n -C 12 'useStructuredAgentSessionOutbox|structuredAgentSessionDeliveryNotices|isVisible|fence' src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx src/renderer/src/components/native-chat -g 'use-structured-agent-session-controller*.ts' -g '*controller*.ts' -g 'NativeChatStructuredSession.tsx' | sed -n '1,700p'

Repository: stablyai/orca

Length of output: 42414


🏁 Script executed:

set -eu
echo '--- structured session hook ---'
cat -n src/renderer/src/components/native-chat/use-structured-agent-session.ts | sed -n '1,190p'
echo '--- fence state definitions and producers ---'
rg -n -C 10 'fence\s*[:=]|transportState|useStructuredAgentSessionTransport|fenceRef' src/renderer/src/components/native-chat src/renderer/src/runtime -g '*.ts' -g '*.tsx' | sed -n '1,900p'
echo '--- visibility retention boundary ---'
rg -n -C 12 'NativeChatStructuredSession|useStructuredAgentSession\(|enabled: isVisible|isVisible' src/renderer/src/components/native-chat/NativeChatResolvedView.tsx src/renderer/src/components/native-chat -g '*.ts' -g '*.tsx' | sed -n '1,800p'

Repository: stablyai/orca

Length of output: 42250


Keep the notice and Retry while the owner is detached.

When transportState.fence is null, owner.attached is false. The probe returns without scheduling a resend, but this branch still suppresses the notice because the resend predicate matches. A visible session can retain and render that outbox entry while the fence is null. Gate this suppression on owner attachment so the notice and Retry remain available until the probe can run.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one behavioral edge worth confirming.

Reviewed changes

  • Shared resend rule extracted — new src/shared/structured-agent-session-outbox-unconfirmed-resend.ts holds structuredAgentSessionEntryResendsUnconfirmed (in doubt, never retried, not outlived by a Stop, no journal row yet).
  • Probe reads the rule — use-structured-agent-session-outbox-unconfirmed-probe.ts derives probeId from it and drops the separate probeSettled check; verified behavior-identical (the extra state === 'unconfirmed' conjunct is implied by blocker, and the submission check only moves into probeId).
  • Notices go quiet while resending — structured-agent-session-delivery-notices.ts skips any entry the rule matches.
  • Journal rows reach the notices for in-doubt sends — NativeChatStructuredSession.tsx now passes controller.submissions when the outbox has an unconfirmed entry, not only when one was rejected (readsRows).
  • Tests — new coverage in structured-agent-session-delivery-notices.test.ts and NativeChatStructuredSessionDelivery.test.tsx; existing unconfirmed fixtures were updated to keep their notice under the new suppression.

I traced the predicate against the base inline probe condition and the reconcile/dispatch paths. The one gap is that the suppression is broader than the "Orca is resending it" condition it is named for.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review summary

The problem. In native chat, if you reopened a chat (or the connection dropped) while a message was still sending, that message briefly showed a red "Message delivery is unconfirmed." line and a Retry button. Orca was already resending it on its own and confirmed it a moment later, so the line flashed and the Retry was never needed.

What changes for you. That message now looks like any other message in the chat while Orca resends it, with no line and no Retry. As soon as the agent's host has any record of the message, the chat behaves exactly as before. One rule now decides both when Orca resends such a message and when the line is hidden, so the line can only be hidden while a resend is actually responsible for the message.

What the review changed. The first version of this PR also tried two more things. Review found both were wrong, so they were removed and the PR now ships only the fix above:

  • Giving a refused message back to the composer. It only fired for a kind of refusal a normally typed message almost never gets (a clock more than five minutes off, or a message saved more than a day earlier), so the red line and Retry stayed for the refusals people actually hit. It also had bugs: the returned text was kept only in memory, so a reload lost it, and the explanation line was wiped as soon as the next message was answered.
  • Dropping a message from the send queue once the host said it had lost track of it. This could make a message disappear with its text gone. If you quit Orca mid-send, reopened, and sent something else, the host could later decide the first message was never delivered. That message was then hidden, and nothing kept a copy of the text. This was the most serious problem the review found, and it is gone with the change.
  • Older hosts. On hosts built before September 9, a message could have looked like it was sending forever while every later message waited behind it. The final rule hides the line only while the host has no record at all, so older hosts behave as before.
  • The PR text was corrected, and the branch was squashed so no commit describes behaviour that no longer ships.

Deferred (not in this PR).

  • The red "not sent" / "unconfirmed" lines with Retry that remain, for refused or failed sends and for messages whose outcome the host can't know, are the real fix still owed. A follow-up stack is planned: a failed send leaves the chat with its text back in the composer, and the host's record shows what happened. Its first pieces are open as drafts and not yet reviewed: fix(native-chat): an unsent draft survives a reload or quit #24905 (drafts survive a reload) and refactor(native-chat): move the phone's send-id retention rule into shared code #24915 (shared rule so a resend of the same text reuses the same message id).
  • If a remote host stays unreachable for a long time, the message stays quiet while Orca keeps retrying, and later messages wait behind it. The chat's existing "Reconnecting to this chat…" line shows when the chat's own connection fails. Whether every kind of dropped SSH connection reaches that line was not confirmed.
  • Small items: one code comment (use-structured-agent-session-outbox-unconfirmed-probe.ts:44) says the message is quiet "only while it is resent", but it is also quiet for a moment after a reopen before the chat loads. The Stop code spells the same resend rule a second time; the follow-up folds it in.

What was verified.

  • Two independent review rounds and a readiness pass found no remaining defects in the final change.
  • 280 tests across 27 files pass. Removing the rule from the notices, or ignoring the host's record, or not passing the host's records to the notices, each makes 3 to 4 tests fail.
  • Web and node typechecks and CI pass. One CI test shard failed once on an unrelated error in main-process code this PR doesn't touch, then passed on rerun.
  • Checked in the real app on a separate test Mac, final code against the base, with a stand-in agent: after reopening mid-send, the base showed "Message delivery is unconfirmed." with Retry for about a second, and this PR showed no line and no Retry at any point. Both delivered the message exactly once, and the PR adds no new label, line or pop-up. Screenshots are in the description.

Not verified.

  • In the real app: a message the host has a record of with an unknown outcome, which should still show the line and Retry as before. The test setup can't create that host record, so only unit tests cover it.
  • Whether every kind of dropped SSH connection reaches the "Reconnecting to this chat…" line during a long outage.
  • A real (not stand-in) agent; the change is in the chat's own display logic, not in how the agent runs.

A send whose answer was lost was resent quietly under the same id but looked
like any delivered message. Every message still in the outbox with no failure
to show now says "Sending…", muted, in place of its time, until the journal
holds a row for it. Rows that say a message did not go through keep only that
line and its Retry. The notices read the outbox through the same reconcile as
the transcript, so a row that lands clears the marker in one step.
@brennanb2025 brennanb2025 changed the title fix(native-chat): no "unconfirmed" line while Orca resends a send on its own fix(native-chat): show "Sending…" on a message until the host confirms it Oct 4, 2026
…the time's slot

A message the user retried, a second message in doubt, or one requeued over a
rejected row had a journal row that did not hold it, so it showed nothing and
looked sent. "Sending…" now stays until the row is pending or accepted.

The marker took the place of the whole meta row, so the copy button went away
while sending and the row jumped when it cleared. The row now stays mounted:
copy keeps its hover reveal and "Sending…" sits where the time goes.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 4, 2026 08:19
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Follow-up: "Sending…" until the host confirms a message

The problem with the earlier version. It removed the flashing "Message delivery is unconfirmed." line, but left a message whose answer was lost looking exactly like a delivered one while Orca resent it in the background. If the answers kept getting lost, the message looked sent but never was.

What changes for you now. Any message Orca hasn't seen confirmed shows a muted "Sending…" where its time goes: while it's queued, while it's on its way, and while Orca is quietly resending it. It clears once, when the host records the message, and then the time and copy button show on hover as before. No red line and no Retry for those states; a message that actually failed keeps only its reason and Retry. This follows the common pattern of a quiet pending marker in place of the time.

What review changed in this round.

  • A message you had pressed Retry on, and a second uncertain message waiting behind the first, used to show nothing while on their way. They now show "Sending…" too.
  • The marker first replaced the whole time-and-copy row, which shifted the chat by a few pixels when it cleared and hid copy while sending. It now takes only the time's spot, so nothing moves and copy works while sending.
  • The description no longer claims this covers a host outage. When the host can't be reached, a send shows "Your message was not sent." with Retry, the same as today, and is not resent; a separate open change (fix(native-chat): stop flashing a reconnecting line on a stream drop #24898) adds a notice that the host is reconnecting or offline.

Deferred.

Verified.

  • Tests: 317 pass across 29 files. With the new code removed, the tests for a lost-answer message, a retried message, a waiting message and copy-while-sending fail.
  • Lint, localization checks, the web typecheck and CI all pass.
  • In the real app on a test Mac, current main against this PR, with a stand-in agent:
    • On reopen mid-send, main showed the red line and Retry for about 1 s, and this PR showed "Sending…" for about 1 s. Both delivered once.
    • A message held unconfirmed showed "Sending…" for over a minute.
    • The row kept its height when the marker cleared.
    • Nothing else new appeared.

Not verified.

  • A real host outage, because the test setup has no remote host.
  • A real (not stand-in) agent.
  • The exact marker duration on a quiet machine; the test Mac was heavily loaded.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Sending… replaces the flashed "unconfirmed" line — an outbox message the host has not confirmed now shows a muted Sending… where its time normally is, with no Retry, instead of the red "Message delivery is unconfirmed." line that flashed on every reopen mid-send.
  • Shared resend rule — new structured-agent-session-outbox-unconfirmed-resend.ts holds structuredAgentSessionEntryResendsUnconfirmed, read by both the auto-resend probe and the notices; the probe's separate probeSettled check folds into it and the behavior is unchanged.
  • Notices read reconciled entries — structuredAgentSessionDeliveryNotices now reconciles the outbox through reconcileStructuredAgentSessionOutboxWithQueue (the same call the transcript uses) so a row that just landed is answered before the outbox commits it and the marker never flashes the red line on its way out.
  • Journal rows reach the notices whenever the outbox is non-empty — NativeChatStructuredSession gates on outbox.length > 0 rather than hasRejected; hasRejected still gates startFailures.
  • User-row meta restructured — NativeChatDeliveryNotice is now a two-shape union; UserMessageMeta keeps Sending… in the time's slot visible without hover, preserves the copy button's hover reveal, and keeps the row's height when the marker clears.
  • Copy + tests — components.native-chat.messageSending added to all six catalogs; new unit and chat-level coverage for the lost-answer send, reopen mid-send, Stop-outlived, immediate rejection, second-in-doubt, and requeued-after-rejection cases.

I traced every branch of the notice decision against the admission/reconcile paths and the probe's resend condition, and the result matches the PR's before/after table (including that a pending/accepted row clears the marker while an unknown/rejected row does not). The new tests distinguish the marker-clearing rule from an any-row rule rather than asserting a tautology. I also ran the changed-code quality gate, the design-system lint on the two changed components, the web typecheck, and the notices (39), message-row (16), delivery-session (20), projection (6), outbox-hook (21), relaunch-hold (14), and start-failure (3) suites — all green.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

…led-send-returns-to-composer

# Conflicts:
#	src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Merge of origin/main reconciled into the delivery tests — 0f5fd45 merges main; the only conflict was NativeChatStructuredSessionDelivery.test.tsx, resolved by keeping this branch's Sending… tests and adopting main's useProbeClock/advanceProbeClock fake-timer helpers for the tests main had converted (main PR #25187). The branch's new unconfirmed/Stop tests still use real setTimeout waits, and main's send-then-click-Retry version of the force-retry test was replaced by the branch's seeded-outbox version — the click path remains covered by the retry-head tests.
  • Production unchanged — every non-test file in the PR is byte-identical to the previously reviewed head (059e79e); the notice decision, the shared resend predicate, the two-shape NativeChatDeliveryNotice, and the outbox.length > 0 journal-row gate are all as previously traced.

I re-read the full PR diff, confirmed the merge brought no production change (git diff 059e79e 0f5fd45 on the six prod source files is empty), and ran NativeChatStructuredSessionDelivery.test.tsx (20/20 green, ~15s).

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

NativeChatStructuredSessionDelivery.test.tsx went over the 800-line lint
limit after main was merged. The eight tests for the automatic probe of an
unconfirmed message move unchanged to
NativeChatStructuredSessionDelivery.probe.test.tsx, which uses the shared
structured-session test harness for its mocks. The outbox seeding and probe
clock helpers move into that harness so both files share them.

The at-most-once reliability gate now lists the new file, with a fresh
evidence run.
@pullfrog

pullfrog Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

no API key found — Pullfrog needs at least one LLM provider API key (e.g. ANTHROPIC_API_KEY, OPENAI_API_KEY, GEMINI_API_KEY) configured as a GitHub Actions secret.

To fix: add the key as a GitHub Actions secret (referenced from your workflow's env: block) or as a Pullfrog secret in the console — or switch this repo to a different model (free models need no key).

Open repo secrets → · Configure model → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | 𝕏

…led-send-returns-to-composer

Main now lets the host's row own a message it recorded and then rejected.
The "Sending…" marker follows that: the notices read the outbox through the
same reconcile as the transcript, now with the loaded rows, so a message whose
row was rejected reads as not sent in the host's words and never as sending.
The delivery-notice hook passes the journal's rows whenever the outbox holds a
message, so the marker still clears on a pending or accepted row.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Rebased the notice path onto main's rewrite — the merge of origin/main (which landed #24710's reconcileStructuredAgentSessionOutboxWithQueue and rejected-in-place projection) moved the branch's journal-row gating out of NativeChatStructuredSession.tsx and into use-structured-agent-session-delivery-notices.ts; equivalent behavior, now journalRows/loadedItems inside the hook.
  • Sending… gated on the same rule — structuredAgentSessionDeliveryNotices now reconciles its entries before deciding, keeps the failure branch for a rejected/held/blocked-head message, and emits { sending: true } only when no pending/accepted submission exists for it. structuredAgentSessionEntryResendsUnconfirmed continues to gate the auto-resend cases.
  • Test split — the delivery probe tests moved to their own NativeChatStructuredSessionDelivery.probe.test.tsx, config/reliability-gates.jsonc lists it, and the notices tests were updated to main's rule (a requeued message over a rejected row shows as not sent, never Sending…).

I traced the reconcile/gating interaction against the base and re-ran the changed suites at this head: notices (41), message-row (16), delivery (12) + probe (8), hook (3), unsent-message (5), and send-disposition (27) — all green. The two prior inline threads are resolved.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025
brennanb2025 merged commit 3b01ba6 into main Oct 4, 2026
33 checks passed
brennanb2025 added a commit that referenced this pull request Oct 5, 2026
…ected message

Main's #24710 draws a message Orca recorded and then rejected where the host
placed it, as "Not sent", from the host's own history. The host side (the
rejection moves the message to its place; a failed start writes its rejections
and its row in one transaction) is taken as written.

On the client, C2's settlement stays the only place an outbox entry ends:
- #24710's reconcile module is folded into C2's: an entry whose rejected row is
  not loaded yet stays, never sent again, and draws the message until that row
  loads (or the host's window for its id ends). Nothing new is stored; the held
  submission says it was rejected. A batch that settles nothing writes nothing.
- One "not sent" surface: main's rule hides a rejected message a live card
  holds or a later copy of the same text superseded, in the rows and the
  notices alike, on the desktop and the phone. The notice stays C2's muted one,
  and notices are kept as the same objects across unchanged batches.
- No Retry: a message refused before it was recorded still goes back to the
  conversation's draft with the reason, as before.
- A rejected /compact stays in the chat (#24918's rule), since the command's
  own reply says nothing once the journal shows it.
- The phone and the desktop both draw these rows; only the host's outline,
  which older clients read, leaves them out.
brennanb2025 added a commit that referenced this pull request Oct 5, 2026
Brings main through d3afb5c, which includes #24606 as it landed: in the
native chat that is the version C2 already merged, so those files keep C2's
resolution, and the locales keep C2's messageNotConfirmed line beside main's
new keys.

From #24918:
- Not-sent rows come back at their journal places, so the phone draws them
  where the host recorded them.
- A command's reply stays silent only once the loaded journal draws it as not
  sent (structuredAgentSessionJournalShowsRejection), so a pending command a
  Stop then withdraws still gets its reply.
- The phone's echo matching no longer double-counts not-sent rows.
- The delivery-line tone test is taken in C2's terms: a recorded rejection
  reads muted from its row or from this client's copy, and a message on its
  way reads only as sending. #24918's `notSent`/Retry notice shape stays out.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant