Skip to content

fix(web): new thread starts in the sidebar's filtered Squadron - #317

Open
tyler-barton-horizon wants to merge 2 commits into
j5/mainfrom
j5/new-thread-honors-squadron-filter
Open

tyler-barton-horizon wants to merge 2 commits into
j5/mainfrom
j5/new-thread-honors-squadron-filter

Conversation

@tyler-barton-horizon

Copy link
Copy Markdown
Collaborator

Problem

With the sidebar scoped to a Squadron, clicking New thread, pressing the chat.new shortcut, or using the chat header button still opened the Squadron picker. Only the landing route honored the filter, so users had to re-pick the Squadron they were already looking at.

Fix

Every new-thread door now resolves through the same function, resolveCurrentThreadNewThreadDestination:

  1. the active thread's durable Squadron home (chat header and palette only),
  2. else the sidebar's ambient Squadron filter,
  3. else the sole ready Squadron,
  4. otherwise the picker.

The picker still appears when the filter points at a Squadron the directory no longer lists or whose environment is unreachable. The command palette gains a "New thread in " row for the filtered Squadron under the same rule. The redundant canCreateThreadWithoutSquadronPicker helper is removed.

Surfaces

  • Entry points: sidebar button, chat.new shortcut, chat header button, command palette, landing route (already correct).
  • Clients: web and desktop. Mobile has no Squadron filter.
  • No contract or provider changes.

Verification

  • SquadronPicker.logic.test.ts and CommandPalette.logic.test.ts pass, with new cases for the filtered, stale, and unavailable Squadron paths.
  • Web typecheck clean; lint on changed files shows only pre-existing warnings.
  • Verified manually in a local dev environment seeded with six Squadrons.

Model: Claude Fable 5.1. Harness: Claude Code in T3 Code.

🤖 Generated with Claude Code

When the sidebar was scoped to a Squadron, the New thread button, the
chat.new shortcut, and the chat header button still opened the Squadron
picker. Only the landing route honored the filter.

Every new-thread door now resolves through
resolveCurrentThreadNewThreadDestination: the active thread's durable
home first, then the sidebar's ambient Squadron filter, then the sole
ready Squadron, otherwise the picker. The command palette offers a
"New thread in <name>" row for the filtered Squadron under the same
rule. The redundant single-Squadron helper is removed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 effective changed lines (test files excluded in mixed PRs). labels Sep 25, 2026
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 25, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: Jacksondr5/j5code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 08f462c0-63d1-4811-aba1-2da6bacfd612


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

@bryantderosier bryantderosier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I reviewed this for correctness, code quality, and security. Nothing is blocking, so I'm approving. Routing every new-thread door through resolveCurrentThreadNewThreadDestination is a real simplification. Removing canCreateThreadWithoutSquadronPicker gets rid of the Sidebar's duplicate exact-one check, and resolveIndexDraftDestination behaves the same as before. The resolver tests cover the filtered, stale, and unavailable cases.

Should-fix (inline below)

  1. The palette row's ⌘N hint doesn't match what ⌘N does (CommandPalette.tsx:1701). The comment says the row follows the same rule as chat.new, but chat.new only reads the Sidebar filter. The palette also skips the draft's chosen Squadron, which the header respects.
  2. The ambient fallback kicks in while a thread's home is still loading (ChatView.tsx:2377, same at CommandPalette.tsx:1704). The header can say "New thread in Alpha" for a thread homed in Bravo, then flip to Bravo once the home loads.
  3. A filtered Squadron with no local folder makes the Sidebar button do nothing (Sidebar.tsx:4396). The header and the palette row have the same problem. The resolver only checks available, not folder.
  4. FORK.md cases 16 and 19 still describe the old rule ("exactly one ready Registrar Squadron retains direct creation" / "choose the sole ready Registrar entry … or open the Squadron picker"). All four component and route files here are upstream-owned. Please update those cases to the new order: home → ambient filter → exact-one → picker. Case 19 should also say the palette's "New thread in " row can now render from the filter or from the sole Squadron. Otherwise the next person who rebases on upstream might "restore" the exact-one-only behavior from the inventory.

Nits

  • _chat.tsx:40: ambientSquadronId holds a ScopedSquadronRef, but ChatView uses that name for the string id and calls the ref ambientSquadronScope. I'd rename it to match ChatView.
  • Sidebar.tsx:4390: a stale filter now blocks the exact-one shortcut (details inline).
  • Sidebar.tsx:4388: setOpenMobile(false) now runs first, and the zero-Squadron branch calls it again through openSquadronCreateFromSidebar(). Harmless, just redundant.
  • SquadronPicker.logic.ts:73: "Every new thread door resolves through here" is true, but each door passes different inputs. Once 1 and 2 are fixed, I'd like one small helper that builds the input (home, else draft, else filter) so the header, palette, and chat.new can't drift apart again.

Security: nothing to report. The resolver matches on both environmentId and squadronId. Stale or unreachable filters open the picker. Squadron names render as escaped React text, and thread creation still goes through the existing server path.

Surfaces: keybinding, palette, header, sidebar, and landing all go through the resolver now. Desktop inherits web, and mobile has no Squadron filter. The branch/fork door deliberately keeps the source thread's home, which I think is right.

scopedThreadKey(scopeThreadRef(activeThread.environmentId, activeThread.id)),
)
: undefined;
// Same rule as the chat.new shortcut: the active thread's durable home,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should-fix: this row's ⌘N hint doesn't match what ⌘N does. chat.new (_chat.tsx:106) only passes the Sidebar filter and never looks at the active thread's home. This row still sets shortcutCommand: "chat.new".

Example: the Sidebar is filtered to Alpha and the open thread is homed in Bravo. The palette shows "New thread in Bravo ⌘N", but pressing ⌘N starts the thread in Alpha. Before this PR the same mismatch fell back to the picker. Now ⌘N quietly goes to a different, specific Squadron.

This input also skips the draft's chosen Squadron, which the header does respect via resolveHeaderSquadronRef(... draftSquadronId). On a draft that picked Charlie with the filter on Bravo, the header says Charlie and this row says Bravo. That's a new mismatch, since the row didn't render for drafts before.

Suggested fix: build this input the same way ChatView does (home, else draft, else filter), ideally with a shared helper in SquadronPicker.logic.ts. Then either have chat.new use the same input, or only show the chat.new hint when this row's destination is what ⌘N would pick. And please fix the comment.

durableHomeId: durableSquadronHome?.id ?? null,
draftSquadronId: draftSquadron.squadronId,
}),
}) ?? ambientSquadronScope,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should-fix: the filter fallback kicks in while the thread's home is still loading. useThreadHomes leaves a thread out of the map until its home read returns, so activeThreadHome is undefined, durableSquadronHome is null, and this falls through to ambientSquadronScope. A thread that truly has no home comes back as kind: "unknown".

Example: the filter is Alpha and I open a thread homed in Bravo from a cold start, or the home read for that environment fails. The header says "New thread in Alpha", then flips to Bravo. A click before the flip starts the thread in Alpha. Before this PR that window used the exact-one shortcut or the picker, so it never picked a specific wrong home.

Suggested fix: only fall back to the filter when the thread is a draft (serverThread === null) or its home resolved to kind: "unknown". While a server thread's home is undefined, pass null so the shortcut or picker applies. CommandPalette.tsx:1704 needs the same rule for activeHome.

if (destination.kind === "single-squadron") {
void startSquadronDraft({
entry,
entry: destination.entry,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should-fix: a filtered Squadron with no local folder makes this button do nothing. The resolver only checks entry.available. buildSquadronPickerEntries sets folder: null when the Squadron's project isn't in projects for that environment, for example before project shells finish loading after a connect, or after the folder was removed. startSquadronDraft then returns null, this void drops it, and on mobile-web the sheet has already closed. Before this PR, a filtered list with more than one Squadron opened the picker, which at least shows that row as disabled.

The ChatView header (handleNewThreadInActiveProject) and the palette row drop the null the same way, and the palette row isn't disabled for folder === null the way buildSquadronPickerRow is. Only chat.new shows the "Squadron folder unavailable" toast.

Suggested fix: have resolveCurrentThreadNewThreadDestination treat entry.folder === null like unavailable and return the picker. That also covers the index route. Please add a folder: null test case.

if (isMobile) setOpenMobile(false);
if (isMobile) setOpenMobile(false);
const destination = resolveCurrentThreadNewThreadDestination(
squadronScopeId,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: a stale filter now blocks the exact-one shortcut. If the filter names a Squadron the directory no longer lists (deleted on another device, so forgetDeletedSquadron never ran here) and exactly one ready Squadron remains, this button, chat.new, and the palette all open the picker. Before this PR, the Sidebar and chat.new would have used the single remaining Squadron. The new test only checks stale-then-picker with two Squadrons. If you think it's worth it, fall through to resolveNewThreadShortcutDestination when the filtered entry is missing, and keep the picker for found-but-unavailable.

const keybindings = useAtomValue(primaryServerKeybindingsAtom);
const projects = useProjects();
const { status: squadronDirectoryStatus, squadrons } = useSquadronDirectory();
const ambientSquadronId = useSquadronAmbientScope();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: this holds a ScopedSquadronRef, not an id. ChatView uses ambientSquadronId for the string id and calls this same value ambientSquadronScope. Renaming it here to ambientSquadronScope keeps the two files consistent.

@Jacksondr5

Copy link
Copy Markdown
Owner

Heads-up: this PR is superseded by work that has now merged.

What changed on j5/main (2026-10-08). J5 is retiring Squadrons and folding their behavior into projects (#412, decided 2026-10-05). The client half has merged (#454, #455, #456):

  • The Squadron picker, draft chip, sidebar Squadron filter, first-run gate and the Create, Rename and Delete Squadron dialogs are gone. Most of apps/web/src/j5/squadron/ is deleted.
  • New threads, drafts, the sidebar filter and Add Project use upstream's project flow again.
  • The Fleet page, Inbox and thread cards read the thread's project.

Still to come. The server migration re-keys the ledger from Squadrons to projects, removes list_squadrons and join_squadron, and renames squadron_id / squadronId to project fields across the server, the shared contracts and the peer protocol. After that, a rename pass removes the word from the remaining code.

For this PR. The problem it describes, that New thread opened the Squadron picker even with the sidebar filtered, no longer exists: there is no Squadron picker. The files it changes are either deleted (apps/web/src/j5/squadron/SquadronPicker.logic.ts and its test) or back to upstream's versions (routes/_chat.tsx, and the new-thread parts of Sidebar.tsx, CommandPalette.tsx and ChatView.tsx).

Upstream's own behavior is back in its place: with more than one project, the New thread button opens upstream's project picker, and shift-click or the second shortcut creates directly in the current project. If you still want the sidebar's project filter to choose where a new thread goes, that would now be a small change on top of upstream's flow and is worth a fresh PR or issue. This one was right about the friction; it is what started the discussion in #412.

Posted by an AI agent on Jackson's behalf.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 effective changed lines (test files excluded in mixed PRs). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants