feat(home): pin a prompt to the homescreen - #209
Conversation
8ec50f8 to
70a1ad9
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70a1ad9234
ℹ️ 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 (typeof value.title === "string" && value.title.trim()) { | ||
| state.title = value.title.slice(0, PROMPT_PIN_TITLE_MAX_LENGTH); | ||
| } | ||
| if (typeof value.text === "string" && value.text.trim()) { | ||
| state.text = value.text.slice(0, PROMPT_PIN_TEXT_MAX_LENGTH); |
There was a problem hiding this comment.
The editor accepts unrestricted title and prompt lengths, but persistence silently slices them to 200 and 50,000 characters here. After the save response is adopted or the app restarts, users who entered longer content lose the suffix without warning; apply matching input limits or explicit validation before accepting the edit rather than truncating during serialization.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Not fixing this in this PR, but the report is accurate.
The two bounds are deliberate rather than arbitrary. 50,000 matches the prompt bound the send pipeline already enforces (berdctl session create), so persisting more would store text the send path would refuse anyway, and 200 for a display-only title is well past what a launcher row can render.
The residual you identified is not covered by that reasoning, though, and it is real: text over the limit sends in full today but comes back sliced after a restart, so the pin quietly stops matching what was pinned. Accepting it here rather than adding input validation, because reaching it takes a 50,000-character prompt or a 200-character title, and the failure mode is a truncated pin rather than a bad send or lost session. Input-level maxLength on both fields is the fix and is tracked as a follow-up.
70a1ad9 to
5a484ec
Compare
5a484ec to
ff43a2c
Compare
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
Reviewed the full exact three-dot comparison 57e06f6...6885014 by static inspection only, using both the project code-review and wes-review lenses. Found one new non-blocking prompt-integrity issue. Final self-check covered prompt creation/editing, mention selection and replacement, persistence and mode/size transitions, launch and unavailable-agent behavior, experiment gating, shared mention-autocomplete behavior, accessibility, English/Spanish localization, navigation/consent guards, async failure/never-completes/lifecycle/race behavior, test honesty, design-system/project rules, and overlap with supplied threads. Supplied GitHub evidence was inspected: eight check runs were completed successfully, while the combined commit status was recorded as pending; required checks still govern merge readiness. Recommendation: COMMENT.
Deterministic publication result: 0 blocking and 1 non-blocking finding(s) publishable; 1 duplicate(s) suppressed.
## Why
Two latent bugs in the window where a chat session is still being
created. Both are silent: the user sees a message accepted into the
queue and then nothing, with no error and no visible failure state.
**A pre-commit rejection stranded the message forever.** A queued send
rejected before commit was retried exactly once, and that retry bailed
out if the session was not ready when the timer fired:
```ts
if (!isQueuedSessionReady(runtime, ...) || getQueuedMessageKey(retryHead) !== key || ...) {
return; // burns the single retry and gives up
}
```
During draft promotion the session is not ready yet, so the one retry is
spent on the readiness check. Pre-commit rejection is silent and leaves
no store transition behind, so nothing ever re-triggers the drain.
`LAWS/CHAT.md` requires that "when a chat's session becomes ready, that
chat's queue MUST resume dispatching its first message to that session"
— this violated it.
**Quitting mid-creation replayed into an unreachable id.** Queues are
keyed by session id, and a session that is still creating only has a
client-local draft id. Persisting its queue means the next launch
restores a record bound to an id no backend session will ever have, so
it can never drain.
## What
- `useMessageQueue`: replace the single-retry latch with exponential
backoff capped at 30s. A retry that fires while the session is still not
ready re-arms instead of abandoning the record, and backs off as it does
so. Any later readiness transition resets the backoff to the initial 1s.
- `useMessageQueue`: bound *rejections* to 5 per payload while leaving
the *readiness wait* unbounded. See the note below — this is the
important half of the change.
- `queuePersistence`: skip queue writes for sessions with an unsettled
`creationState` (pending or failed). The write becomes an explicit
`null`, so quitting mid-creation drops the message rather than stranding
an undrainable record.
### Why retries are bounded but readiness waiting is not
Retrying until the session is ready is right for a readiness or
ownership race. Applying it to *every* pre-commit rejection is not,
because some rejections are permanent.
`sendQueuedMessageWithAutoCompact` returns `false` whenever compaction
fails (`useChatSessionController.ts:2062`), and each failure appends an
error notification and calls `setChatState(sessionId, "idle")`
(`useChat.ts:424-430`). Since `isQueuedSessionReady` requires `chatState
=== "idle"`, that failure reads as a readiness edge — which both reset
the backoff and fell through to `tryDrainQueuedMessage`. So a
persistently failing send did not retry every second; it looped as fast
as compaction could fail, appending an error notification to the
transcript every pass. A test driving twelve such cycles dispatched **73
times**.
The ceiling is counted at the single drain choke point rather than in
the retry timer, so every trigger is covered (timer, readiness edge,
lease release, store subscription). The counter deliberately survives
readiness edges — the transition a failed send *causes* must not hand it
a fresh budget — while a user edit or a new head does reset it. The
record stays queued with `showInComposer` forced true, so it remains
visible and manually retryable.
Waiting for readiness stays unbounded, since giving up there is the
original stranding bug. But it now backs off to the 30s cap instead of
re-arming at a flat 1s: measured, an unready session woke **960 times in
16 minutes** before this change.
## Risk Assessment
Moderate — this is shared chat code on the send path, not behind any
experiment flag, so it affects every user.
The behavior change is bounded to sessions that are not ready, not yet
created, or persistently rejecting. A send into a ready session that
succeeds is unaffected: the retry and rejection state both clear on
success exactly as before.
The remaining judgement call is the ceiling of 5. Too low and a genuine
multi-attempt ownership race gives up early; too high and a persistent
failure appends more error notifications before stopping. Five spans
~31s of backoff, and the failure mode when it is wrong is a visible
retryable record rather than a lost message. Reviewers should push back
if a real race is known to need more.
Not addressed here: the rejection reason is not actually classified.
`finalize(false)` is reached both from a thrown
`PreCommitSendRejectedError` and from a plain `false` return, and the
`catch` at the call site discards the error. Distinguishing ownership
races from terminal transport/preparation failures by type would be a
better fix than a count, and is a reasonable follow-up.
Validation: `just check` passes clean. `src/features/chat` passes 2505
tests across 169 files. Both new tests were confirmed to fail without
the fix — 73 dispatches instead of 5, and `[1000, 1000, 1000, 1000,
1000]` instead of a growing backoff — so neither is vacuous.
## References
- `LAWS/CHAT.md` — queue dispatch and readiness rules
- Split out of #209, which surfaced the original bugs; that PR now
stacks on this branch
Generated with [Claude Code](https://claude.ai/code)
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Home had no way to keep a prompt you rerun constantly, so you either retyped it or dug back through session history. A pin is a home-canvas card that runs its prompt in one click, sending through global compose to a concrete target rather than opening an empty composer. The collapsed card stays a one-row launcher, so removal lives in the editor behind the pencil instead of adding a second control to the row. Gated behind the prompt-pins experiment, off by default. Depends on the queued-send retry fix in the parent branch: without it a play whose session is still being created is rejected pre-commit and never retried. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Editing the pin debounces the save by 400ms, and the unmount cleanup only cleared that timer. Removing a focused element does not fire blur, so leaving Home within 400ms of the last keystroke dropped the pending save and the pin reopened with the previous title and text. The cleanup now flushes the latest refs instead. Flushing after a removal is harmless because updateWidgetState no-ops once the instance is gone, so an abandoned draft still does not come back, and the callback is read through a ref so the unmount-only cleanup cannot use a first-render copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6885014 to
8c1f8d2
Compare
Swapping the attached agent located the old agent's mention with indexOf over the whole prompt, so it deleted whatever matched first. That text is not necessarily a widget-generated mention: attaching an agent through the persona picker inserts nothing, so an authored "@agent One" in the prose was removed, and with repeated mentions the occurrence chosen was arbitrary. The widget now records the range of the mention it inserts and rewrites only that. A prompt edit earlier in the text shifts the range, so it re-anchors by search, but only when the mention text appears once — duplicates are ambiguous and leave the prompt untouched. The picker and an externally replaced prompt drop the tracking. renderStatefulPin backs the new tests: the widget only reaches the swap path when it sees its own saved agentId come back, which renderPin's fixed instance never does. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
Fresh full review of PR #209 at head SHA 09a49ff. All three existing review threads are suppressed (two resolved with substantive human replies, one unresolved with substantive human reply accepting risk). The previous P2 finding (agent mention deletion via indexOf) is fixed in this SHA with a tracked-range approach (findInsertedMention + insertedMentionRef). No new findings. Eight GitHub Actions check runs completed successfully; combined status pending. Recommend APPROVE.
Deterministic publication result: 0 blocking and 0 non-blocking finding(s) publishable; 3 duplicate(s) suppressed.
2 participants
Why
Home has no way to keep a prompt you run over and over. You either retype it or go digging through session history, which is friction on exactly the prompts you use most. This adds a prompt you can pin to the home canvas and run with one click.
Screen.Recording.2026-08-25.at.9.07.43.AM.mov
What
promptPinwidget with two modes: an editor for writing the prompt, and a compact ready card that acts as a one-row launcher (title + play + pencil). Play sends immediately through global compose to a concrete target rather than dropping you into an empty composer.promptlayout kind plumbed through the home layout mapper and the layout API, so pins persist across restarts.@-mention agent picking, following the existingMentionAutocompletedismissal contract so Escape closes the menu and lets you keep typing.prompt-pinsexperiment, off by default.en+esstrings, plus tests for the widget, catalog, picker, layout mapper, and prompt runner.Risk Assessment
Low. The widget is behind the
prompt-pinsexperiment and off by default, so nobody who does not opt in sees a UI change.The one change that reaches shared code regardless of the flag is the
MentionAutocompletedismissal refactor, which the main composer also uses. That is the part worth close review.The two queue fixes this work surfaced have been split into #212, which this PR now stacks on. The pin depends on the retry fix: without it, a play whose session is still being created is rejected pre-commit and never retried, so the prompt silently never sends.
Validation:
just checkpasses clean, and the touched areas (home,chat,experiments) pass 3096 tests across 217 files. Full Vitest suite comes back1 failed | 7084 passed; the one failure issrc/shared/telemetry/client.test.ts› "still fires when the window session store is unavailable", which fails identically on pristinemainand is pre-existing. Separately,AgentShareDialog.test.tsxis a load-dependent flake — it failed on two full-suite runs with a different failure count each time, then passed on a third and on main's full suite. This branch touches neither file.Manual verification:
@-mention attach including Escape-then-keep-typingReferences
prompt-pinsinsrc/features/experiments/experimentDefinitions.tsLAWS/Generated with Claude Code
Update 2026-08-25
Split the two shared queue fixes out into #212 and rebased this branch on top of it. Those changes were not experiment-gated, so they shipped to everyone while the pin itself stayed behind a default-off flag — the flag gave no blast-radius protection on the riskiest code in the diff. This PR is now 27 files of experiment-gated UI; #212 is the 4 shared chat files. The combined tree is byte-identical to the previous single commit (
70a1ad92), so nothing was dropped or changed in the split.