Thread forking + session-based side chats + timeline scroll preservation#85
Open
brsbl wants to merge 40 commits into
Open
Thread forking + session-based side chats + timeline scroll preservation#85brsbl wants to merge 40 commits into
brsbl wants to merge 40 commits into
Conversation
Server/domain foundation, timeline message-action scaffolding, and
timeline scroll-position preservation for the thread-forking + side-chats
spec.
- S1: add startedOnBehalfOf to createThreadRequest; plumb it through
provisioning so a forked thread-start turn renders as an agent-initiated
"Message from {source}" and is seeded WITHOUT a provider run
(OQ1 -> approach A via thread-provisioning). Add nullable childOrigin
(domain + migration 0024 + persist + response + list filter) and a
server-derived canSpawnChild depth-cap boolean. Foundation tests.
- S2: register Fork/SideChat icons; extract reusable MessageActionBar
(reuses CopyButton, hover-reveal); forward row identity to assistant props.
- S5: in-memory per-thread scroll-anchor store; throttled capture +
restore-by-row in BottomAnchoredScrollBody (converges on ResizeObserver
settle, falls back to bottom); tests.
Typecheck 5/5 packages; @bb/db 281, @bb/app 1046, server-contract +
seed-without-run server tests pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- [P1] Boundary coherence + anti-forgery in createThreadFromRequest: startedOnBehalfOf requires parentThreadId and senderThreadId === parentThreadId (transitively validated live + same-project); childOrigin requires parentThreadId. Side-chat combo (childOrigin + null startedOnBehalfOf) still valid. +5 tests. - [P1] Scroll fallback-to-bottom now scrolls inline when the saved anchor row never appears (was relying on a later resize that may never fire); de-masked the test (mutation-verified). - [P2] Log a warning when the seed-without-run provisioning->idle transition no-ops (stranded seed observable). - [P2] Name the message hover-reveal group (group/message + group-focus-within) to match the disclosure/tab-pill convention. - Correct the AppCreateThreadRequest comment: the server schema owns the null default at runtime; the client echoes null only to satisfy the output-typed wire body (input-typed contract is a noted follow-up). Typecheck @bb/server, @bb/server-contract, @bb/app pass; boundary + scroll + message tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fork wiring and session-based side chats, built on the batch-1 foundation. - S3 (fork): buildForkThreadRequest + useForkThreadFromMessage create the forked thread (anchor message; startedOnBehalfOf/childOrigin/parentThreadId all = source; managed worktree from the source branch HEAD; provider/model/ permission inherited), navigate to it, composer auto-focuses on mount (Approach A: server seeds the anchor without a provider run). Fork button wired into the agent message-action footer with a canSpawnChild depth guard; forked anchor renders with the Fork icon via childOrigin; "Forks" lineage list in thread metadata via a targeted parentThreadId+childOrigin query. Consumes the row-identity props (resolves a batch-1 finding). - S4 (side chat): side-chat panel tab kind; openSideChat opens a tab with a PromptBoxInternal composer (OQ4: it drives a non-route thread directly); lazy child-thread creation on first submit with an agent-only main-thread snapshot (spawning message + N=3 preceding), personal workspace, readonly; keep-mounted SideChatTabDeck (BrowserTabDeck pattern); send-back via the existing useSendThreadMessage (senderThreadId now forwarded). onSideChat wired next to onFork under the shared depth guard. @bb/app typecheck pass; full app suite 149 files / 1075 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- [P0] Side chats now run in a same-project readonly worktree (shared child-thread-environment resolver, reused by fork + side chat) instead of a personal workspace, which 400'd in standard projects and broke the same-project parentThreadId/send-back invariants. Spec updated. - [P1] Side-chat context snapshot anchors on the full source message text (persisted on the tab) instead of the truncated tab title, so the window centers on the real spawning message. - [P1] Fork button is dropped (not inert) when the source has no host. - [P1] Server validates senderThreadId is same-project as the target in resolveMessageSenderThreadId (rejects cross-project attribution forgery on the send/queued paths); +test. - [P2] Fork icon only on the seed anchor row, not every cross-thread agent message; remove dead sourceTurnId field; synchronous re-entrancy guards against double-create (fork + side chat); deleted side-chat child shows a "no longer available" state instead of endless waiting. @bb/app 149 files / 1084 tests; @bb/server typecheck + public-thread-data 55 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- [P1] Fork/side-chat child spawns no longer overwrite the project's stored execution defaults (childOrigin carve-out in shouldRememberProjectExecutionDefaults), so a side chat's forced readonly mode / inherited model can't silently become the project default. +test. - [P2] optimisticallyInsertThread honors the childOrigin list filter, so a side chat no longer flashes in the source thread's "Forks" list before refetch. +test. - [P2] Side-chat lazy create waits for a host-backed source's environment to resolve before building the request, avoiding a personal-workspace fallback that 400s in standard projects. Deferred (noted in PR): tighten the single-thread cache write-side generics to ThreadResponse (currently ThreadWithRuntime) — benign today, latent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Completes the previously-deferred final-review P2: the threadQueryKey (single-thread) cache slot is read as ThreadResponse but was written through helpers typed ThreadWithRuntime, so a future writer could drop the required canSpawnChild/childOrigin without a type error. Type updateCachedThread, the create/send/stop transaction writers, and their previousThread fields against ThreadResponse for that slot (list/sidebar entries stay ThreadWithRuntime). Type-only change; typecheck clean and cache/mutation tests green (every existing writer already supplied a full ThreadResponse). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address the three P2 findings from the full-PR review: - [P2] Couple startedOnBehalfOf and childOrigin at the create boundary: startedOnBehalfOf now requires childOrigin, so a seed-without-run thread is always a tagged child (can't be untagged or silently reshape project execution defaults). +test. - [P2] hasAnyThreadMetadata accounts for the lazily-fetched Forks row (the gate now takes hasForks, computed via a deduped useThreads), so a forks-only thread no longer flashes the "No thread details" fallback while the environment query loads. +test. - [P2] Gate side-chat "Send to main thread" on the child thread being idle, so a mid-stream partial can't be posted into the main thread. @bb/server typecheck + boundary tests (9) green; @bb/app typecheck + full suite (149 files / 1087 tests) green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The scroll-anchor capture effect's cleanup read scrollAnchorCaptureThrottleRef .current directly, which eslint flags as possibly-stale. Capture the (stable, never-reassigned) throttle-state object into a local inside the effect and use it in the cleanup — same live trailingTimeout is cleared, lint clean. @bb/app lint + typecheck pass; scroll-preservation tests (6) green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve conflicts with 24 commits of main: - ThreadMetadataContent.tsx: keep the Forks row + main's new environmentDisplayHost prop on EnvironmentRow. - ThreadMetadataContent.test.tsx: combine the ForksRow/hasAnyThreadMetadata tests with main's EnvironmentRow tests and EnvironmentDisplayHostContext import. - ProjectList.test.tsx: take main's deletion. - environment-provisioning.test.ts: add the now-required childOrigin/ startedOnBehalfOf to main's new createThreadFromRequest call. Typecheck 6/6; @bb/app lint + full suite (150 files / 1093 tests); @bb/server thread + environment suites (97) green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Forking or opening a side chat from a personal-project thread failed with "Personal project threads must use a personal workspace". resolveChildThread Environment keyed on hostId !== null, but a personal-project thread has a host running a *personal* workspace, so it built a managed worktree the server rejects. Route to a personal workspace whenever the source's workspace is personal (carrying the host), not only when there is no host. Verified live: fork + side chat from a personal-project thread now 201 (was 400). +regression tests in both builders. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the native title tooltips on the per-message copy / fork / side-chat
icon buttons with the Radix Tooltip primitive (faster, styled, consistent).
MessageActionBar wraps each button in a Tooltip under a local TooltipProvider.
CopyButton is now forwardRef + prop-spreading so it can be a Tooltip `asChild`
trigger; a caller can pass `title={undefined}` to drop the native tooltip when
supplying its own (its other 4 usages keep the default title). aria-labels are
retained on all three buttons.
@bb/app typecheck + lint + full suite (150 files / 1095 tests) green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A side chat (or fork) is a child thread, so completing/failing a turn fired bb's child->parent notification, posting a "child finished" system message into the main thread (which the main agent then replied to). That's wrong for user-initiated branches — the user drives them directly. Extract isAgentDelegatedChildThread(thread) = has a parent AND childOrigin is null, and gate all three child->parent notification sites on it (turn-completion in internal/events, command-failure in thread-lifecycle, needs-attention in internal/interactive-requests). Agent-delegated children (childOrigin null) still notify their parent; forks/side chats do not. @bb/server typecheck + thread-parent (incl. new helper tests) + child-thread- notifications / interactive-requests / parenting suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nd-side-chats # Conflicts: # apps/app/src/components/secondary-panel/ThreadSecondaryPanel.tsx # apps/app/src/components/secondary-panel/useThreadFileTabs.ts # apps/app/src/components/thread/timeline/ConversationMessageContent.tsx # apps/app/src/components/ui/icon.tsx # apps/app/src/lib/api.ts # apps/app/src/lib/fixed-panel-tabs-state.test.ts # apps/app/src/lib/fixed-panel-tabs-state.ts # apps/app/src/views/thread-detail/ThreadDetailSecondaryContent.tsx # apps/app/src/views/thread-detail/ThreadDetailView.tsx # apps/app/src/views/thread-detail/threadSecondaryPanelSelection.ts # apps/server/src/services/threads/thread-provisioning.ts # packages/db/drizzle/meta/0024_snapshot.json # packages/db/drizzle/meta/_journal.json
The loopback/LAN firewall allows a same-origin subresource only when the requesting frame's origin matches the committed main-frame origin, derived from `details.frame.origin`. Electron reports a blank origin for a document's initial subresource requests (before it has run script and committed its origin), so a loopback SPA dev server's own JS modules were cancelled and the page rendered blank. Add resolveRequestingFrameLocalOriginKey: prefer the frame origin, and for a top frame with a blank origin fall back to the frame's committed URL. The fallback is top-frame-only, so a sub-iframe presenting a blank origin still cannot be mistaken for the trusted main frame. Use it in the onBeforeRequest firewall. @bb/desktop typecheck + browser policy/view-manager/main-ipc suites green (incl. new resolver unit tests + a same-origin-SPA-subresource regression that also asserts a blank-origin sub-iframe stays blocked). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The tab-strip useMemo body uses main's consolidated activeFixedSecondaryTabId (plus our activeSideChatTabId); drop the now-unused granular active*Path/Id fields from its dependency list. Those fields are still consumed elsewhere for file-preview content, so the hook destructuring keeps them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nd-side-chats # Conflicts: # apps/app/src/components/thread/timeline/ConversationMessageContent.tsx # apps/app/src/views/thread-detail/ThreadDetailView.tsx
The parent thread's prompt-context banner counted every active child thread, including user-opened forks and side chats, so opening a side chat showed "1 active child thread" on the main thread. Filter the banner's child list to agent-delegated children (childOrigin === null) — the same distinction used for parent notifications — so only delegated work the parent is waiting on appears. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the side chat's single bottom "Send to main thread" button (which always sent the latest reply, ambiguously) with a per-message action: each side-chat agent reply now carries a "send to main thread" icon in its action bar, so the user hands back exactly the answer they want. Threads a ThreadTimelineSendToMainMessageHandler from the side-chat timeline host (SideChatTabContent) through ThreadTimelineRows → ConversationMessageContent → MessageActionBar, mirroring the existing fork/side-chat per-message wiring. The action is supplied only inside a side chat (the main timeline has none) and is gated on the child being idle so a mid-stream partial can't be posted. Delivery still uses the cross-thread senderThreadId transport. Drops the now-dead lastAssistantText/canSendBack/bottom-button plumbing. @bb/app typecheck + lint clean; new MessageActionBar test covers the action rendering, firing, and that it is not gated by the fork/side-chat depth flag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Four changes from the design discussion: - Shared conversation-context snapshot. Generalize the side-chat snapshot into buildConversationContextSnapshot (fork + side-chat scopes). Its header now carries the parent thread id + total message count + a `bb thread show <id>` pointer, so a child agent can pull the full parent on demand instead of being limited to the bounded window. Wider default window for forks (10) than side chats (6). - Seed forks with prior context. Thread the source timeline rows into buildForkThreadRequest and prepend an agent-only lead-up snapshot (the messages before the anchor; the anchor itself stays the visible seed) so a fork isn't amnesiac about how it reached the branch point. - Drop the "(fork)" title suffix. Omit the title so a fork auto-titles from its first real turn (distinct from the source) rather than echoing the parent name. - Hide side chats from the sidebar tree. projectThreadGroups skips childOrigin === "side-chat" so only forks (and agent-delegated children) nest there; side chats stay panel-only. @bb/app typecheck + lint clean; new conversation-context-snapshot tests (both scopes + the bb-thread-show pointer) and updated fork-thread-request tests green. (projectThreadGroups.test.ts can't run locally — pre-existing @pierre/diffs navigator load issue on main — but the filter typechecks and is exercised in CI.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…filter The sidebar-filter test (and 4 other node-environment test files) couldn't load: they transitively import @pierre/diffs, which reads navigator.userAgent at module load, and Node < 21 has no global navigator. Add a minimal navigator stub to the shared vitest setup (only when one isn't already present, so jsdom/browser envs are untouched). With the file loadable again, refactor buildProjectThreadGroups to drop childOrigin === "side-chat" threads up front (so they neither nest nor resurface as orphan roots in the cycle-recovery pass) and add a test asserting a side chat is excluded from the tree while a fork stays nested under its parent. @bb/app full suite now green: 160 files / 1218 tests (previously 5 files failed to load). Typecheck + lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Fork threads now read "Forked from <source>" in the prompt-context banner (and "This thread was forked from …" in the expanded body) instead of the generic "Parent <source>"; driven by a new relationship: "parent" | "fork" on ThreadPromptParentThreadSection, set from thread.childOrigin. Also drop the redundant "Parent " prefix from the id-based loading fallback. - Thread-header pill shows "fork" for forks (was always "child"); the header prop becomes childPillLabel: "child" | "fork" | null. - Pill padding bumped (px-1.5 py-0 rounded-sm → px-2 py-0.5 rounded) so it reads like a tag rather than a tight chip. - Stories: pill story shows the child/fork pills; banner story adds a "Forked from …" variant. New ThreadDetailHeader pill tests. @bb/app typecheck + lint clean; header + banner suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fork threads now show a small Fork icon before their title in the sidebar tree (thread.childOrigin === "fork"), so a fork reads as a fork at a glance alongside its nesting under the source. Adds a "fork" row to the ThreadRow story. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A turn-backward arrow reads as "return/hand this reply back to the parent" — truer to the action than the prior vague ArrowUpRight, and a clean pair with the ArrowTurnForward the message renders with when it lands in the parent. Registers the ArrowTurnBackward icon. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d builders The feature made childOrigin (on Thread) and startedOnBehalfOf/childOrigin (on CreateThreadRequest) required-nullable, but main's CLI fixture, CLI spawn, and integration request helpers — reintroduced by the catch-up merge — built those objects without the fields, so @bb/cli and @bb/integration-tests failed typecheck in CI (the frontend already handles this via AppCreateThreadRequest; these typed Hono/SDK clients can't omit defaulted fields, so they pass the explicit null a normal thread actually has). - command-output-fixtures.makeThread: childOrigin: null - thread spawn command + integration createHostThread/createReuseThread: startedOnBehalfOf: null, childOrigin: null - update thread-spawn command-output expectations to match the request body Root cause was only typechecking @bb/app locally; full `turbo run typecheck` across all 32 packages is now green, as are lint and @bb/cli (226) / @bb/app (1218) tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…llowlist A later merge dropped the allowlist entry our own earlier fix added (commits 1a44eb0/514f807), so the contract guard test went red once typecheck stopped masking it. Restore the documented entry for the main-inherited optional field. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- [P1] Document why CLI/integration thread builders pass explicit startedOnBehalfOf/childOrigin nulls (the typed $post client requires the schema-defaulted fields; z.input would re-optionalize the SDK arg type but the underlying $post still requires them, so the null lives at the call site). - [P2] Remove the dead `windowSize` override from buildConversationContextSnapshot (only tests ever set it); boundary tests now exercise the real per-scope default windows. - [P2] Document resolveAnchorIndex's newest-first match + the fork tail-as-anchor fallback + the duplicate-text limitation (mitigated by the bb-thread-show pointer). - [P2] Model childOrigin "side-chat" explicitly in the detail view: a side-chat thread opened directly now shows a "side chat" pill and a "Side chat of …" banner instead of a misleading "child"/"Parent". Consolidated the banner's per-relationship copy into one PARENT_SECTION_COPY source of truth; added a side-chat pill test + banner story. @bb/app full suite (1222) + @bb/cli typecheck + lint green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e path) The sidebar worktree icon was GitBranch, which reads as "branch" (the same glyph used for the branch name in env summaries) and doesn't match the info page. Point getEnvironmentWorkspaceDisplayIconName at FolderGit (FolderGitTwoIcon): a worktree is a git-backed folder, it matches the convention PR #86 adds to the info-page workspace-path row, and it's clearly distinct from the fork glyph. The registry entry is placed to match PR #86 exactly so the two branches merge cleanly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…convention
Pills are all one component with color variants (uniform padding); the only
casing outlier was the story's hard-coded "Failed". Lowercase it ("failed") to
match how real status/type pills render (the workflow-run status enum is all
lowercase; thread-type pills are child/fork/side chat). Descriptive multi-word
labels (e.g. "Invalid local path") intentionally stay sentence case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pill set leading-none (line-height = font size), and the inner truncate span (overflow:hidden) then clipped descenders (the g in "managed"/"manager"). Drop leading-none so the text uses text-xs's natural 16px line-height; descenders fit and the pill reads a touch less tight. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The earlier change only touched headerTitle (the row's tooltip/aria). The visible
group label is built separately and rendered `{environmentName ?? "Worktree"}` +
a ": " separator, so a no-name worktree group still showed "Worktree: <branch>".
Render the branch alone when there's no custom env name (the folder-git icon
already signals "worktree"); keep "name · branch" when named and the "Worktree"
fallback when there's neither.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…omposer)
Delete the bare side-chat composer and render the side chat through the same
FollowUpPromptBox the main thread uses — one composer to keep in sync, not two.
The footer (model + workspace + permission) renders read-only for the side chat
by extending the existing locked-provider pattern: ExecutionControls now renders
the MODEL as a static OptionDisplay label when model.onChange is omitted, and
FollowUpPromptBox renders the PERMISSION as a static label when permission.onChange
is omitted. ExecutionModelConfig.onChange, ExecutionControlsProps.reasoning, and
ExecutionPermissionConfig.onChange become optional; the main thread still passes
them (ThreadDetailPromptArea), so its pickers stay interactive.
Side chats inherit the parent's provider/model and are always read-only, so they
pass locked execution + { value: "readonly" } permission + a ThreadEnvironmentSummary
workspace strip, matching the main thread's layout. NewThreadPromptBox guards on
the now-optional permission.onChange. New tests cover the locked model + permission
rendering; locked-footer story added.
@bb/app typecheck + lint + full suite (1224) green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the border-t separator above the side-chat composer (it had a border the main thread's doesn't), and render the "Ask a question…" empty state with the same centered icon + sm muted text layout as the new-tab empty state, instead of a top-aligned panel. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The composer's container query (@container promptbox-shell, max-width 34rem) hides data-promptbox-full-label and shows data-promptbox-compact-label when narrow. The locked model/permission OptionDisplays (side-chat read-only footer) only set `value` (the full label) with no `compactValue`, so in the < 34rem side panel the full label hid and nothing rendered. Pass compactValue so the locked model + permission show at the same positions as the main thread composer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The compactValue addition makes OptionDisplay render the locked label in both the full and compact spans (CSS shows one per container width); jsdom has no container query, so getByText now matches twice. Use getAllByText. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…static labels)
The read-only side-chat footer rendered custom static OptionDisplay labels for
model/permission instead of the actual pickers, so the two composers diverged:
the side chat showed "GPT-5.5" vs the main thread's "5.5", "Read only" vs "Read",
and the model landed in the wrong spot — each divergence patched with another
spot fix (compactValue, custom options). The composer is supposed to be ONE shared
component; the only difference should be that the side chat is read-only.
Render the SAME controls, just disabled:
- Add `disabled` to OptionPicker → ModelReasoningPicker / PermissionModePicker:
the trigger renders as a non-interactive (disabled) button with the identical
selected label, no menu/popover. Dimmed via the existing muted treatment.
- ExecutionControls: drop the static locked-model OptionDisplay + compactValue;
render the real ModelReasoningPicker with `disabled`. onChange/reasoning are
required again (the relaxed-to-optional signalling is replaced by `disabled`).
- FollowUpPromptBox: drop the permission OptionDisplay + compactValue; render the
real PermissionModePicker with `disabled`, gated by a new `readOnly` prop.
- SideChatTabContent: build execution + permission configs via the same
useThreadCreationOptions the main thread uses, so the disabled pickers show
identical labels ("5.5", "Read"); pass `readOnly`. Removed the custom
SIDE_CHAT_PERMISSION_OPTIONS / options:[] hacks.
- NewThreadPromptBox: revert the EditablePermissionConfig + onChange guard (the
shared onChange is required again).
- Keep the elastic-min-height fix (stack === null → default) so the side chat
input matches the main thread's height.
Resolves the review's P1 (dead permission.onChange optionality) and P2 (three
hand-rolled locked-label paths). Main thread composer unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Clicking edit on a queued message restored its text into the draft but left the composer unfocused, so you had to click in before typing. Add a declarative `focusEndKey` to the prompt box (PromptBoxInternal → FollowUpPromptBox): when it changes, the editor focuses with the caret at the end. Unlike the passive scope autofocus, it is NOT gated by the coarse-pointer soft-keyboard guard, since it follows a deliberate click. ThreadDetailPromptArea bumps it after the edit restores the draft. Test: focusEndKey change focuses the editor even under a coarse pointer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… config
- [P1] Editing a queued message put the caret at the START of the restored text.
The focusEndKey focus ran (useLayoutEffect) before the passive content-sync
setContent in the same commit, so focus("end") targeted the pre-edit content
and setContent then mapped the caret to the start. Move the focus to a passive
effect defined AFTER the content-sync effect; React's definition order runs it
after setContent. The test now changes content + key in one commit and asserts
the caret lands at the end.
- [P2] The side chat wired the creation hook's setters as picker onChange even
though the pickers render disabled (unreachable). Pass noop, drop the unused
setters, correct the comment.
- [P2] The side-chat permission label could drift from the actually-created
read-only reach for a hypothetical non-readonly multi-mode provider. Source the
displayed value and the create request from one SIDE_CHAT_PERMISSION_MODE
constant.
@bb/app typecheck + lint + full suite (1224) green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The side-chat composer wrapper used px-2/pb-2 (8px) while the main thread composer sits in a px-4 wrapper with a 16px bottom gap, so the side chat box rendered 8px lower and 8px narrower per side. Match px-4/pb-4: measured against the main composer the two now share identical box width (548px), input-box top, height, model-row baseline, and bottom edge — only the pane x differs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A reviewer empirically showed the caret test passed identically with the old buggy effect ordering: jsdom's ProseMirror always places the caret at the end after setContent, regardless of focus order, so the focusOffset assertion could not distinguish the fix from the regression (a "passes when broken" test). Drop the selection/offset assertions and keep only focus + content (what jsdom can prove); the caret-at-end guarantee is documented as resting on the focus effect being declared after the content-sync effect, covered by manual QA. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the approved spec
thread-forking-and-side-chats-e159end to end: fork a thread from any agent message, spawn context-aware side chats in a right-panel tab, and preserve timeline scroll position across thread navigation. Built and reviewed across five sessions (S1–S5) with per-batch and final adversarial reviews; all findings fixed.What's in it
1. Fork (
TrendingUpDownIcon, per-agent-message action) — creates a child thread, navigates to it, and renders the originating message as a "Message from {source}" anchor with the fork icon, composer focused. The forked agent does not auto-run — it waits for the user's first direction.parentThreadId+childOrigin: "fork"; provider/model/permission inherited.parentThreadId+childOriginquery).requestThreadProvision+ aseedWithoutRunflag.2. Side chat (
MessageAdd02Icon) — a child thread rendered in a kept-mounted right-panel tab, created lazily on the user's first message. Seeds the first turn with anagent-onlysnapshot of the main thread (spawning message + 3 preceding), runs readonly, and can "Send to main thread" (reuses cross-thread send).3. Scroll preservation — in-memory per-thread anchor (by row id, not pixels); continuous throttled capture + restore that coordinates with the existing stick-to-bottom logic and survives the
key={threadId}remount.Contract / data changes
createThreadRequest: discriminatedstartedOnBehalfOf+childOrigin(server-boundary defaults), end-to-end through provisioning → thread-start dispatch.childOrigincolumn (migration0024) + a server-derivedcanSpawnChilddepth-cap field on the response.MessageActionBar(copy + fork + side chat) replacing the inline copy footer.Security / correctness hardening (from review)
startedOnBehalfOf ⇒ senderThreadId === parentThreadId;childOrigin ⇒ parentThreadId.resolveMessageSenderThreadIdnow rejects cross-projectsenderThreadId(forgery/cross-project-write guard).childOriginhonored in optimistic list-cache inserts; double-submit re-entrancy guards.ThreadResponseso a future writer can't silently drop the requiredcanSpawnChild/childOrigin.Testing
@bb/app149 files / ~1085 tests;@bb/db281 (migration replay);@bb/servercontract + send + seed-without-run + project-defaults suites green. (One unrelatedAppTabContentiframe test flakes only under parallel load; passes in isolation.)Notes
.otto/directory (spec, tasks, per-session plans) is included as the work's audit trail and can be dropped from the PR if undesired.🤖 Generated with Claude Code