Skip to content

Simplify session creation: modal → right-drawer quick-start#1801

Draft
kasiazjc wants to merge 14 commits into
mainfrom
feature-simplify-session-creation
Draft

Simplify session creation: modal → right-drawer quick-start#1801
kasiazjc wants to merge 14 commits into
mainfrom
feature-simplify-session-creation

Conversation

@kasiazjc

@kasiazjc kasiazjc commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Starting a session no longer opens a modal you have to fill in and dismiss. "Add session" now opens the session drawer directly, ready to type into.

  • Quick-start: every "Add session" entry point (branch card, board canvas, event-stream panel) skips the modal — if we can resolve a tool for you, the session is created immediately and the drawer opens on it.
  • Tool resolution: your explicit default (new Settings option) → else your most-recently-used tool → else genuinely unresolved.
  • First-time / unresolved case: drawer still opens instantly (no modal), showing a tile picker in the same chrome as a real session, so nothing visually jumps when you pick one.
  • Inline title editing: click the "Untitled session" header text to rename, in place. No dialog.
  • Auto-title: if you never set one, the title fills in from your first prompt once it completes (cheap heuristic, no LLM call).
  • Switch tool: available from the "..." menu, but only before a session has run anything — since the tool choice is baked into every task once one exists.
  • The old full modal isn't deleted — it's demoted to an "Advanced setup..." escape hatch for the rare case of wanting to set title/prompt/MCP/env vars before the session exists.

Why / context

The old flow: click "Add session" → modal → pick a tool → (often) same tool as last time → fill in a title you'll probably rename anyway → submit → then the session drawer opens. That's a form in front of every single session, for a choice that's almost always "same as last time."

This follows the "quick-create" pattern from Linear/Notion (create first, refine inline) and matches how Claude/ChatGPT auto-title new threads. The friction removed compounds — session creation is the single highest-frequency action on the board.

Nothing that needed the modal is gone: MCP/tool config for the rare cases that need it upfront is one click away via "Advanced setup...". Board-zone flows that intentionally require a picker (show_picker, always_new triggers) are untouched — this branch makes zero changes to SessionCanvas.tsx or the zone-trigger modal (confirmed by diff), see Validation below for the caveat on live-testing that path.

Implementation notes

  • resolveDefaultAgenticTool: explicit preferences.default_agentic_tool → most-recently-created session's tool → undefined (no hardcoded fallback).
  • chooseAgenticTool is the single action behind both tool-choice surfaces (empty-state tiles and the in-drawer "Switch tool" affordance) — creates the session, resolves MCP inheritance the same way the old modal did, remembers the pick as the new quick-start default, and navigates to the result. "Switch tool" passes replacingSessionId to swap out a never-prompted session transparently.
  • agentic_tool can't change mid-session (different SDK per tool), so "Switch tool" is hidden the moment a session has run one task.
  • Auto-title hooks into the existing task-completion patch path in TasksService; fires once, only if the title is still empty when the first task completes — an explicit title (even one typed mid-task) always wins.
  • default_agentic_tool lives in the existing UserPreferences JSON blob — no migration needed.
  • Includes an unrelated-but-picked-up perf commit (77800e48, progressive board mount) that was already merged to main and is just part of this branch's history — not new work in this PR.

Validation / test plan

  • pnpm lint
  • pnpm typecheck
  • pnpm test (core, daemon, ui — including new coverage for tool resolution, chooseAgenticTool, auto-title heuristic, PendingToolChoicePanel, inline title edit, switch-tool affordance). All green: core 2737, ui 726–734, daemon 1392–1395 passed across multiple runs.
  • Rebased on current main (77800e48), no conflicts, re-verified lint/typecheck/tests clean after rebase.
  • Manual click-through completed for: quick-start with a resolved tool (skips picker, opens drawer with focused prompt), quick-start with no default set (tile picker, prompt/settings disabled until a tool is chosen), inline title edit (placeholder, click-to-edit, save on Enter, cancel on Escape), auto-title firing after first prompt completes, switch-tool appearing/disappearing correctly based on task history.
  • Board-zone show_picker trigger flow — not yet confirmed via live click-through. Diff-isolation gives high confidence (this branch touches zero lines in SessionCanvas.tsx, ZoneConfigModal.tsx, or the zone-trigger modal), but I was not able to reliably reproduce the drag-and-drop trigger in the environment used for this pass (Docker-based env unavailable). Remaining live QA, including this flow, to be finished in a follow-up pass.

Risks / rollout / rollback

  • Additive preference field, no schema migration. Old modal path still exists and is reachable, so nothing is a one-way door.
  • Main behavior change users will notice: "Add session" no longer asks first. If that surprises anyone, "Advanced setup..." is the fallback and a default tool can be unset in Settings to force the picker again.

Out of scope / follow-ups

  • Auto-title currently uses a heuristic (truncate first prompt); wiring the real ai_title event some CLIs already emit is a nice-to-have follow-up, not required for this to ship.
  • Final live manual QA pass (see unchecked item above) to be completed once the Docker-based dev environment is available again.

kasiazjc added 10 commits July 3, 2026 11:21
Lets a user mark a primary agentic tool for quick-start session
creation. Lives in UserPreferences (already a flexible JSON blob), so
no DB migration is needed.
Lets a user set/clear their primary tool for quick-start session
creation, backed by the new preferences.default_agentic_tool field.
Resolves the tool "Add session" should use with no picker: explicit
default_agentic_tool preference, else the user's most-recently-created
session's tool, else unresolved (caller falls back to a tile picker).
One mechanism behind both tool-choice entry points: the quick-start
empty-state tiles (nothing to replace yet) and the in-drawer "Switch
tool" affordance (silently swaps out the current, never-prompted
session). Creates a session with the chosen tool, resolves MCP
inheritance the same way NewSessionModal already does, remembers the
pick as the new quick-start default, and navigates to the result.
Exposed via AppActionsContext (onChooseAgenticTool/availableAgents)
so it's reachable from inside SessionPanel without prop drilling.
Wires every "Add session" entry point (branch card, board canvas
node, event-stream panel) to skip the old blocking modal: when a
tool resolves (explicit default or this user's last-used tool), the
session is created immediately and the drawer opens directly on it.

When nothing resolves (first-ever session for this user), the drawer
still opens immediately instead of a modal, showing
PendingToolChoicePanel — tiles for each available tool, matching the
session drawer's own header/footer chrome so the panel doesn't
visibly change shape between "picking a tool" and "using a session".
Picking a tile creates the session via the same chooseAgenticTool
action and swaps this panel out for the real one.

The old NewSessionModal survives as an "Advanced setup..." escape
hatch reachable from the empty state, for the rare case someone wants
to set title/prompt/MCP/env vars before the session exists.
Replaces the static header title with a click-to-edit field:
"Untitled session" placeholder when there's no explicit title or
description, click swaps in a borderless Input seeded from the
explicit title only, Enter/blur saves via the existing onUpdateSession
action, Escape cancels without saving.
Reuses the same chooseAgenticTool action the quick-start empty state
uses, with the current session passed as replacingSessionId. Only
offered via the "..." menu when the session has zero tasks (never
prompted yet) — tool is a per-session SDK choice baked into every
task, so it can't safely change once a conversation exists.
Cheap, synchronous heuristic (collapse whitespace, cut at ~60 chars on
a word boundary) with no LLM call — tool-agnostic since it only reads
Task.full_prompt, already stored regardless of which agentic tool ran
it. Wired into the existing task-completion hook in TasksService.patch:
fires once, when a session's first task completes successfully with
still no explicit title. A user-set title (including one typed while
the task was still running) always wins, since by the time the hook
runs session.title would already be non-empty.
Tiles render per available agent, picking one calls onChoose with the
tool id, and the advanced-setup escape hatch only shows up when a
handler is provided.
Also hardens canSwitchTool against a missing session.tasks array
(session.tasks is typed as always-present, but defend anyway rather
than let a malformed/legacy record crash the whole panel) — the
existing SessionPanel test only avoided this because it doesn't wire
onChooseAgenticTool, short-circuiting before the access.
@kasiazjc

kasiazjc commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Review: quick-start session creation flow (high-effort pass)

Verified directly against source (not just diff context) — apps/agor-daemon/src/services/sessions.ts, tasks.ts, apps/agor-ui/src/components/App/App.tsx, SessionPanel.tsx, resolveDefaultAgenticTool.ts, PendingToolChoicePanel.tsx. No zero-blocking-issues disclaimer applies here — there are real blocking findings below.

Blocking

  1. agentic_tool immutability is only a client convention — no server-side guard. SessionsService.patch() (apps/agor-daemon/src/services/sessions.ts:811-834) has no field-level check preventing agentic_tool from being changed on a session that already has tasks. The whole "Switch tool disappears after the first task" story (canSwitchTool in SessionPanel.tsx:753) is enforced only by hiding a menu item client-side. Any other caller of sessions.patch (a stale tab, the MCP session-update tool, CLI, a future UI surface) can set agentic_tool on a session with existing tasks, silently desyncing it from the tool that actually produced those tasks/messages. This was explicitly called out as a risk to verify in the PR description — confirmed it's not enforced server-side.

  2. TOCTOU on the "Switch tool" replacingSessionId swap can delete a session with an in-flight task. canSwitchTool (SessionPanel.tsx:753) is evaluated once when the menu/modal opens. handleSwitchTool (SessionPanel.tsx:754-763) never re-checks session.tasks.length before calling onChooseAgenticTool(...). chooseAgenticTool (App.tsx:796-825) then calls client.service('sessions').remove(replacingSessionId) (App.tsx:809), and SessionsService.remove() (apps/agor-daemon/src/services/sessions.ts:766-805) cascade-deletes unconditionally — no check for a running/in-flight task. In this multiplayer canvas, if a collaborator (or another tab, or an MCP agor_sessions_prompt call) sends the first prompt to that session between the modal opening and the tile being clicked, the swap silently deletes the now-in-flight session and its task with no confirmation and no server-side block.

  3. Auto-title can be permanently disabled by a degenerate first prompt (e.g. image-only paste). deriveTitleFromPrompt (packages/core/src/sessions/derive-title-from-prompt.ts:14-25) returns '' for an empty/whitespace-only prompt, and the call site (tasks.ts:619-625) additionally short-circuits on task.full_prompt being falsy — either way autoTitle ends up '' and the title patch is skipped (tasks.ts:633). Combined with the "fires only once, on session.tasks?.length === 1" gate (tasks.ts:622), a session whose first prompt is image-only (or otherwise text-empty) — or whose first task fails/is stopped and only a later retry succeeds — never gets an auto-title, ever. Worth deciding if that's acceptable or if the trigger should be "first task with a non-empty derived title," not strictly task index 1.

Non-blocking / worth a look

  1. resolveDefaultAgenticTool never validates against currently available tools. (apps/agor-ui/src/utils/resolveDefaultAgenticTool.ts:19-37, consumed at App.tsx:831-841.) If a user's preference or last-used tool has since become unavailable (credentials revoked, tool disabled), quick-start still resolves it and skips the picker — handleQuickStartSession calls chooseAgenticTool directly instead of falling back to setPendingToolChoiceBranchId, so the user only discovers the problem when session creation fails, instead of being routed to a picker over currently-valid tools.

  2. Narrow but real race on the resolver's "most-recently-used tool" fallback. It's derived from sessionById (resolveDefaultAgenticTool.ts:29-35), which is hydrated client-side with archived: false + a bounded $limit: PAGINATION.DEFAULT_LIMIT query (apps/agor-ui/src/hooks/useAgorData.ts), not a true "last session for this user" query. On a busy/shared instance this cache can miss the user's actual most recent session (paged out or archived), silently resolving a stale tool or null instead.

  3. Swallowed failure in the replace-session cleanup leaves an orphaned session with no user-facing signal. App.tsx:807-813client.service('sessions').remove(replacingSessionId) failure is only console.error'd; the flow still updates the user's default_agentic_tool preference and navigates to the new session as if nothing went wrong. A transient failure here leaves two live sessions on the branch with zero indication to the user.

  4. Narrow race between an explicit title edit and the auto-title patch. tasks.ts:558 reads session once; tasks.ts:619-636 computes autoTitle from that single stale !session.title read and unconditionally includes it in the later sessions().patch() call with no re-check/CAS. If a user's own title PATCH commits in the small window between the read and this write, the auto-title patch silently overwrites it — contradicting the "explicit title always wins" comment at tasks.ts:613-618. Window is narrow (no await in between other than a fire-and-forget .then()), but it's a real gap, not a hypothetical one.

  5. "Switch tool" modal highlights the wrong tile while switching. SessionPanel.tsx:1552: selectedAgentId={switchingTool ? session.agentic_tool : null} highlights the session's current (old) tool tile during the async request, not the tile the user just clicked — the opposite of PendingToolChoicePanel's choosingTool pattern (PendingToolChoicePanel.tsx:53,119), which correctly highlights + overlays the clicked tile. There's also no loading spinner/overlay here at all, unlike the empty-state picker.

Confirmed non-issue

  • Board-zone show_picker / always_new trigger flows are genuinely untouched. Traced the drop-into-zone path (SessionCanvas.tsx, zone-trigger.ts, branches.ts MCP tool) — it's a fully separate code path that never touches resolveDefaultAgenticTool / chooseAgenticTool / handleQuickStartSession. No regression here.

Cleanup (reuse / simplification, lower priority)

  • apps/agor-ui/src/utils/resolveQuickStartMcpServerIds.ts re-derives the branch>user-default MCP precedence that resolveSessionDefaults() (packages/core/src/sessions/resolve-session-defaults.ts) already centralizes for 4 daemon call sites — its own docstring admits it "mirrors" that walk. Consider calling the shared resolver instead of maintaining a second copy.
  • derive-title-from-prompt.ts is a 4th independent "truncate at word boundary + ellipsis" implementation (different threshold/ellipsis char) alongside existing ones in CardNode.tsx and BranchCard.tsx.
  • chooseAgenticTool (App.tsx:796-825) duplicates handleCreateSession's create+navigate sequence rather than composing on top of it, and does createawait remove(replacingSessionId)navigate sequentially even though the removal doesn't need to block navigation.
  • Two parallel pieces of state model the same "a new-session flow is open for branch X" concept — pendingToolChoiceBranchId and newSessionBranchId (App.tsx) — requiring manual sync at every close/transition call site instead of one discriminated state.
  • tasks.ts (1258 LOC) and SessionPanel.tsx (1567 LOC) both exceed the 500-LOC "propose refactor before testing" threshold in context/guidelines/testing.md; this PR adds more logic + new test files to both without flagging it.

kasiazjc added 4 commits July 3, 2026 14:11
…witch-tool TOCTOU

Review findings #1 and #2 on PR #1801: "Switch tool disappears after the
first task" and agentic_tool immutability were only client-side
conventions — nothing on the server stopped a direct sessions.patch from
changing agentic_tool on a session with existing tasks, or the
switch-tool swap from cascade-deleting a session that gained a task
between the picker opening and the tile being clicked (TOCTOU on a
multiplayer canvas).

SessionsService.patch() now rejects an agentic_tool change on a session
that already has tasks, for both single and batch patches. remove()
refuses a swap-triggered removal (marked via query._swapReplace) when
the target session has gained a task since the swap was initiated,
while leaving a normal user-intentional delete of a session with
history unaffected.
Client side of review finding #2: chooseAgenticTool now marks the
"replace the old session" removal with query._swapReplace so the
daemon's TOCTOU guard (SessionsService.remove) can tell it apart from a
normal user-intentional delete.

Also addresses finding #6: a failed cleanup removal (including the new
guard's Conflict) was only console.error'd — the flow continued as if
nothing happened, leaving an orphaned session with zero user-facing
signal. Now shows a themed error message so the user knows the old
session may still be visible on the branch.
…first

Review finding #3: auto-title was gated on session.tasks?.length === 1,
so a degenerate first prompt (image-only paste, or any text-empty
prompt) permanently disabled auto-titling for that session — the
heuristic never got another chance. Same for a first task that failed
and only a later retry succeeded.

The gate is now just "the session still has no title" — fires on any
completed task, keeps retrying until a non-empty title is derived or an
explicit title is set (which always wins, unchanged from before).
…t the old one

Review finding #8: the "Switch tool" modal highlighted the session's
current (old) tool tile during the async request instead of the tile
the user just clicked — the opposite of PendingToolChoicePanel's
choosingTool pattern, which correctly highlights + overlays the clicked
tile. There was also no loading indicator at all.

switchingTool now tracks the clicked tool id (string | null) instead of
a bare boolean, so the grid highlights the right tile and shows the same
Spin overlay the quick-start empty-state picker already has.
@kasiazjc

kasiazjc commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the 3 blocking findings, plus one low-risk cleanup

All server-side, covered by new unit/integration tests against a real database (dbTest), no Docker/live click-through needed for any of this. Commits: 57b8219e, e2807515, c9dd7067, 5033cd5f.

#1agentic_tool immutability now enforced server-side. SessionsService.patch() rejects an agentic_tool change (single or batch) on a session that already has a task, regardless of caller — stale tab, MCP tool, CLI. Unrelated-field patches and same-value patches still go through. Tests: sessions.agentic-tool-guard.test.ts.

#2 — TOCTOU on the "Switch tool" swap. The client now marks the replace-session removal with query._swapReplace; SessionsService.remove() refuses that specific removal (Conflict) if the target session has gained a task since the swap was initiated, while a normal user-intentional delete of a session with history is untouched. Tests: sessions.swap-remove-guard.test.ts, simulating a task landing between swap-initiation and swap-completion.

#3 — Auto-title no longer permanently disabled by a degenerate first prompt. Dropped the session.tasks?.length === 1 gate — it now fires on any completed task while the session still has no title, so an image-only first prompt (or a failed-then-retried first task) doesn't leave the session titleless forever. Tests cover: image-only first prompt → no title yet → second task with real text → title fires.

#8 (non-blocking, fixed anyway) — "Switch tool" now highlights the tile you just clicked instead of the session's old tool, with the same loading overlay PendingToolChoicePanel already has. switchingTool went from a bare boolean to the clicked tool id, mirroring choosingTool there.

#6 (non-blocking, fixed as part of #2) — a failed swap-cleanup removal (including the new Conflict case) now surfaces a themed error message instead of only console.error, so a leftover orphaned session isn't silent.

Left for later — flagging, not fixing blind

  • feat: add zone deletion with session cleanup options #4 (quick-start resolves a tool without checking it's still available — credentials revoked, tool disabled) and feat: improve conversation footer UX with sticky positioning and runn… #5 (the "most-recently-used tool" fallback can miss on a bounded/paginated client cache) both need either new query capability or a live pass to be confident about the fix, not just unit coverage. Deferred.
  • fix(ui): add missing users dependency to usePresence useMemo #7 (narrow window where an explicit title edit can race the auto-title patch) — real per the review, but a proper fix wants a DB-level conditional update (no optimistic-concurrency field exists on Session today) rather than a narrower read-then-write, which would just shrink the window instead of closing it. Deferred rather than rushed.
  • Cleanup: resolveQuickStartMcpServerIds duplicating resolveSessionDefaults's walk — looked at wiring it through the shared resolver, but that also computes permission_config/model_config we'd discard just for the MCP list, trading a small single-purpose function for tighter coupling to a larger multi-concern one. Skipped rather than rush a refactor under this pass.
  • The two 500+ LOC files (tasks.ts, SessionPanel.tsx) — not restructured, per the review's own note that's a bigger call, not a fix-in-place.

Full lint/typecheck/test suite green across core (2737), daemon (1405, +10 new), and ui (735, +7 new). Rebased on current main, no conflicts. Live manual QA (the board-zone show_picker flow specifically) is still the one item pending from the earlier pass — Docker's down again today, that resumes separately.

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