Skip to content

fix(sessions): disable message send when offline#2121

Open
pauldambra wants to merge 2 commits intomainfrom
posthog-code/disable-send-when-offline
Open

fix(sessions): disable message send when offline#2121
pauldambra wants to merge 2 commits intomainfrom
posthog-code/disable-send-when-offline

Conversation

@pauldambra
Copy link
Copy Markdown
Member

Summary

When the connectivity store reports offline, the session view's PromptInput now disables submit. Previously, pressing Enter offline cleared the editor without sending anything — losing the user's text.

The task-creation composer (TaskInput) already gated submit on isOnline; this brings the follow-up-message composer in SessionView in line by adding !isOnline to submitDisabledExternal. Reuses the existing disable mechanism, so:

  • The send button greys out
  • Enter is a no-op (handled in useTiptapEditor via submitDisabledRef)
  • The typed message stays in the editor and sends as soon as connectivity returns

Refs SupportHog ticket #758.

Test plan

  • In a session with an active agent, simulate offline (DevTools → Network → Offline). Confirm the "No internet connection" toast appears.
  • Type a message and press Enter. Confirm the text is preserved in the editor and the send button is disabled.
  • Restore connectivity. Confirm the message can be sent.
  • Verify the existing TaskInput (new task composer) behavior is unchanged.

Created with PostHog Code

When the connectivity store reports offline, the session view's PromptInput
now disables submit. Previously, pressing Enter offline cleared the editor
without actually sending the message — losing the user's text. The
task-creation composer already gated submit on `isOnline`; this brings the
follow-up-message composer in line.

Refs #758

Generated-By: PostHog Code
Task-Id: a29b652e-dc35-43cb-b560-f91d52f56732
@pauldambra pauldambra marked this pull request as ready for review May 10, 2026 20:17
Copy link
Copy Markdown
Member Author

paul-reviewer

ok so a 3-line change to stop the message disappearing into the void when we're offline. stamp 🚢 — but a few thoughts:

  1. silly question... do we want a toast / inline hint when the user tries to send while offline? right now the button just greys out and Enter becomes a no-op. the editor keeps the text which is the win, but a first-time user might think the app is just broken. the "no internet connection" sonner toast is already up after 5s of offline, so maybe it's fine — i'm lazily asking rather than checking myself.

  2. observability — we now silently swallow Enter presses when offline. i wonder if it's worth a posthog.capture("send_blocked_offline") (or similar) so we can see in production how often this is actually saving people. would also tell us if the 5s debounce on the connectivity toast is too long. not blocking.

  3. nit picking — submitDisabledExternal={handoffInProgress || !isOnline} is fine but reads as a list of "things that disable submit" mashed together. the same pattern in TaskInput.tsx uses !canSubmit || isCreatingTask || !isOnline and canSubmit itself already includes isOnline. two places, two slightly different shapes. not asking you to dedupe (sandi metz, wrong-abstraction risk), but worth a thought. ship as you see fit.

  4. love that this leans on the existing submitDisabledRef mechanism inside useTiptapEditor — no new disable path, no new clear-vs-keep branching. perfetto.

  5. tests — i'm not going to demand them for 3 lines, but SessionView has zero coverage of the offline path. if we ever stop passing submitDisabledExternal for some other refactor, this regresses silently. data-attr on the send button + an e2e that toggles connectivity would catch it next time.

stamp anyway 🚢

Copy link
Copy Markdown
Member Author

xp-reviewer

A small, focused fix — the intent is clear from the diff alone, which is a good sign. The change reuses the existing submitDisabledExternal plumbing rather than inventing a new disable path, which keeps the design simple. A few observations against the Four Rules:

Rule 2 (expresses every idea): submitDisabledExternal={handoffInProgress || !isOnline} is two unrelated reasons OR'd together at the call site. Reads fine today, but the why of each clause isn't surfaced — a future reader sees a boolean expression, not "we disable when offline because messages would be lost". A small refactor would be:

const cannotSend = handoffInProgress || !isOnline
// ...
submitDisabledExternal={cannotSend}

Not strictly necessary at two clauses, but if a third reason appears, that's the moment to do it. ThreeStrikesAndYouRefactor.

Rule 3 (OnceAndOnlyOnce): This is now the second composer that checks !isOnline for the same reason — TaskInput already does. I would not dedupe yet. The shapes are different (TaskInput's canSubmit rolls auth, path, online, empty-state into one; SessionView only cares about handoff + online), and forcing a shared useCanSubmit() hook now would need flags or option args to handle the divergence. That's premature deduplication — the wrong abstraction is worse than a little duplication. Wait for the third place.

Rule 1 (passes all tests): No test was added. For a 3-line guard this is borderline YAGNI vs. genuine coverage, but the connectivity-gated send is exactly the kind of subtle behavior that disappears in a refactor without anyone noticing — there is no failing test if a future change drops submitDisabledExternal. A single unit test asserting that the prop reflects !isOnline would lock the invariant. Not a blocker for shipping; consider it.

YAGNI win: You did not introduce a new "offline-aware composer" abstraction, a context provider, or a higher-order component. You added one line that uses infrastructure that already exists. That is the correct shape for this change.

Ship it.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 10, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/code/src/renderer/features/sessions/components/SessionView.tsx:623-625
When the user has already typed a message and then goes offline, `submitBlocked` becomes true via `submitDisabledExternal`, so the submit button tooltip falls back to `"Enter a message"` — even though text is present. `submitTooltipOverride` exists precisely to override this default; passing it when offline makes the disabled state self-explanatory.

```suggestion
                          submitDisabledExternal={
                            handoffInProgress || !isOnline
                          }
                          submitTooltipOverride={
                            !isOnline ? "No internet connection" : undefined
                          }
```

Reviews (1): Last reviewed commit: "fix(sessions): disable message send when..." | Re-trigger Greptile

Comment thread apps/code/src/renderer/features/sessions/components/SessionView.tsx
Without `submitTooltipOverride`, the disabled send button tooltip falls back
to "Enter a message" even when text is present, hiding the real reason.
Override it to "No internet connection" while offline so the disabled state
is self-explanatory.

Generated-By: PostHog Code
Task-Id: a29b652e-dc35-43cb-b560-f91d52f56732
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 10, 2026

Reviews (2): Last reviewed commit: "fix(sessions): tooltip explains why send..." | Re-trigger Greptile

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