Skip to content

fix(desktop): branch tiled slash commands from their own session - #73251

Open
frizikk wants to merge 6 commits into
NousResearch:mainfrom
frizikk:fix/desktop-branch-command-target
Open

fix(desktop): branch tiled slash commands from their own session#73251
frizikk wants to merge 6 commits into
NousResearch:mainfrom
frizikk:fix/desktop-branch-command-target

Conversation

@frizikk

@frizikk frizikk commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #73207.

Desktop session tiles already pass their runtime ID into the shared slash dispatcher. The native /branch action dropped that target and called the foreground-only branch helper with no arguments, so a command entered in a tile created a child from whichever primary chat was in front.

This PR forwards the invocation runtime through the existing branch action and resolves offscreen branch state from that runtime's isolated cache. The target transcript, busy state, stored parent identity, owning profile, and workspace now stay together, including an explicitly detached/empty workspace rather than borrowing the foreground cwd. Live runtime branches synchronously lease the invocation socket before asynchronous profile lookup; production pruning preserves that lease, disconnected requests reconnect once and retry on the exact same transport, concurrent reconnects coalesce by owner generation, and every completion path releases the lease. Domain errors are never replayed. A delayed branch runtime response reconciles profile-scoped approval metadata only into the source profile, even after the foreground switches. Delayed children are persisted in the invocation profile/layout namespace and no longer steal focus or consume the foreground route-resume edge after profile or route intent changes. Their runtime metadata remains isolated from foreground stores. The existing foreground message-action path remains unchanged, and an explicit target that disappears during dispatch fails closed instead of falling through to an unrelated conversation.

This is adjacent to, but not duplicated by:

Related Issue

Fixes #73207

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • apps/desktop/src/app/session/hooks/use-prompt-actions/slash.ts
    • forwards the slash invocation's target runtime to the native branch action.
  • apps/desktop/src/app/contrib/wiring.tsx
    • carries the optional target through the existing branch wrapper and injects the gateway request binder.
  • apps/desktop/src/app/gateway/hooks/use-gateway-request.ts
    • exposes the production profile-scoped gateway request lease to session actions.
  • apps/desktop/src/app/session/hooks/use-session-actions/index.ts
    • branches offscreen runtimes from their own cached transcript, busy state, stored identity, profile, and cwd;
    • acquires a source command lease before async profile resolution and dispatches runtime-scoped session.branch through its recovery-aware requester;
    • hydrates the child tile without overwriting foreground model/cwd/control stores;
    • snapshots profile/layout/route intent and suppresses stale focus effects;
    • keeps foreground creation/route-resume ownership out of offscreen branch requests;
    • preserves the existing foreground branch path;
    • fails closed when an explicit target runtime no longer has state.
  • apps/desktop/src/app/session/hooks/use-session-actions/utils.ts
    • lets optimistic children carry an explicitly resolved profile/workspace, including explicit detached cwd, without changing the active gateway;
    • applies runtime approval metadata to an explicit owner profile instead of the mutable foreground profile.
  • apps/desktop/src/store/gateway.ts
    • adds exact-owner command leases that survive activation/prewarm pruning, coalesce primary/secondary reconnects by owner generation, revalidate registry identity after awaits, retry only disconnect failures on the same transport, and defer pruning until the final lease releases.
  • apps/desktop/src/store/gateway.test.ts
    • exercises the production registry's primary recovery, secondary pruning, concurrent reconnect coalescing, exact request retry, non-disconnect behavior, terminal retry propagation, and teardown invalidation.
  • apps/desktop/src/store/session-states.ts
    • persists delayed child tiles into an explicit profile namespace without mutating the profile currently on screen.
  • Regression tests cover dispatcher forwarding and the full session-action identity boundary, including a busy foreground chat, a differently profiled tile, deferred profile resolution, an in-flight profile switch, source-requester enforcement, approval-cache ownership, lease release, production pruning/reconnect behavior, stale-completion suppression, optimistic child metadata, and foreground-state isolation.

How to Test

  1. Open session A as the primary Desktop chat.
  2. Open a different session B in a tile or split pane.
  3. Type /branch in session B.
  4. Confirm the child uses B's transcript and is nested under B, while A remains untouched.

Commands run locally:

NODE_ENV=test NODE_OPTIONS='--max-old-space-size=8192 --localstorage-file=/tmp/hermes-vitest-localstorage-pr73251-final.json' \
  npm --workspace apps/desktop exec -- vitest run --project ui \
  src/app/session/hooks/use-prompt-actions/index.test.tsx \
  src/app/session/hooks/use-session-actions.test.tsx \
  src/app/session/hooks/use-session-actions/utils.test.ts \
  src/app/session/hooks/use-message-stream/utils.test.ts \
  src/app/gateway/hooks/use-gateway-request.test.ts \
  src/app/gateway/hooks/use-gateway-boot.test.tsx \
  src/app/session/hooks/use-route-resume.test.tsx \
  src/store/gateway.test.ts \
  src/store/session-states.test.ts
# 9 files, 292 tests passed

NODE_ENV=test npm --workspace apps/desktop run typecheck
# pass: renderer + electron + e2e TypeScript projects

NODE_ENV=test npm --workspace apps/desktop exec -- eslint \
  src/app/contrib/wiring.tsx \
  src/app/gateway/hooks/use-gateway-request.ts \
  src/app/session/hooks/use-session-actions.test.tsx \
  src/app/session/hooks/use-session-actions/index.ts \
  src/app/session/hooks/use-session-actions/utils.test.ts \
  src/app/session/hooks/use-session-actions/utils.ts \
  src/store/gateway.test.ts \
  src/store/gateway.ts
# pass: 0 errors, 0 warnings

npm --workspace apps/desktop exec -- prettier --check \
  src/app/contrib/wiring.tsx \
  src/app/gateway/hooks/use-gateway-request.ts \
  src/app/session/hooks/use-session-actions.test.tsx \
  src/app/session/hooks/use-session-actions/index.ts \
  src/app/session/hooks/use-session-actions/utils.test.ts \
  src/app/session/hooks/use-session-actions/utils.ts \
  src/store/gateway.test.ts \
  src/store/gateway.ts
# pass: all matched files use Prettier formatting

NODE_ENV=production npm --workspace apps/desktop run build
# pass: production stamp 057a817d6379

git diff --check HEAD^ HEAD
# pass

The earlier parent candidate's complete Desktop UI run reported 2771 passed, 5 failed, while its exact detached origin/main baseline reported 2766 passed, 6 failed; every candidate failure reproduced there with the same assertion (locale-sensitive number/date formatting and unrelated billing tests), and the baseline had one additional floating-pane order flake. The remediation commit was then validated through the expanded 292-test affected lane, renderer/electron/e2e typechecks, changed-file lint and formatting checks, a clean production build, and git diff --check.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — N/A: Desktop renderer-only change; the relevant Vitest/typecheck/build gates are documented above, and the full UI baseline is disclosed
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: CachyOS Linux, Node 25.4.0, npm 11.7.0

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A: no user-facing command semantics changed
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — renderer-only TypeScript; no platform-specific APIs
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

No visual styling changes. The fail-before behavior was captured by two deterministic hook regressions:

expected session.branch { session_id: "tile-runtime", count: 2 }
received session.branch { session_id: "foreground-runtime", count: 1 }

expected branchCurrentSession(undefined, "tile-runtime")
received branchCurrentSession()

Both now pass, along with all 292 tests in the affected hook/helper/store files. The cross-profile regressions also verify that asynchronous profile lookup cannot reroute session.branch, a profile switch during the pending RPC cannot mutate/reveal the newer profile's tiles or approval cache, foreground stores remain unchanged, and the child persists under the invocation layout with the target profile/workspace/runtime metadata. Additional exact-seam regressions cover explicit detached cwd, offscreen route-resume ownership, primary generation invalidation, reconnect coalescing, domain-error no-replay, activation/prewarm pruning, and deferred pruning after the final lease release.

Disposable mutation probes demonstrated that both original findings are test-sensitive:

  • restoring approval writes through the mutable active profile failed both named ownership regressions (expected work off, received smart);
  • bypassing the leased requester with sourceGateway.request.bind(sourceGateway) failed the source-requester regression because the production lease was never invoked.

Refreshed candidate identity:

  • commit: 057a817d6379aaad9a92051dad989ae34ecb9121
  • tree: 7072b73f912f0e2a71fdcf1450ad82e87f12be52
  • parent: 2d5dd972325b83f34a2e24e298a8afd4e7ea1f4f
  • frozen base: 936dd7346fd7fd8107af1ce7fc019c07c001c1bd

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) needs-decision Awaiting maintainer decision before any implementation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 28, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #73236 for #73207: both correct tile /branch targeting, but this patch additionally changes isolated runtime/profile/layout handling and fail-closed behavior. Please choose the intended session-routing semantics rather than treating either as a duplicate.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the targeted runtime-aware fix. The reported defect is confirmed on current main: apps/desktop/src/app/session/hooks/use-prompt-actions/slash.ts:474-480 ignores the tile invocation context and calls the foreground-only branch action, while apps/desktop/src/app/session/hooks/use-session-actions/index.ts:1217-1258 reads foreground runtime, transcript, and stored-session state. Tile dispatch does supply a runtime target at apps/desktop/src/app/contrib/hooks/use-session-tile-delegate.ts:54-55.

No blocking defect was identified in static review. The linked #73207 discussion also distinguishes this implementation from #73236 because this patch retains runtime-scoped live state and fails closed for a missing explicit target.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 30, 2026
@frizikk
frizikk force-pushed the fix/desktop-branch-command-target branch 2 times, most recently from 69509c4 to d018654 Compare August 5, 2026 17:56
@frizikk
frizikk force-pushed the fix/desktop-branch-command-target branch from d018654 to ee812d8 Compare August 10, 2026 11:34
@frizikk

frizikk commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Rebased and repaired at ee812d8f24b77e319f71037663426f6286130474 (base 03fa32c92dd445eb64c7f67434dd91b32c40701d). /branch completion now checks the captured tile anchor and normalizes no-anchor null/undefined states, so a same-profile tile switch cannot steal focus while stable branches still reveal normally. Verified: focused desktop suite 44 passed + typecheck. Independent exact-SHA review: PASS.

@frizikk
frizikk force-pushed the fix/desktop-branch-command-target branch from ee812d8 to 057a817 Compare August 11, 2026 18:20
@frizikk

frizikk commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed onto current main and resolved the Desktop branch-target ownership conflicts, including offscreen route state, explicit detached cwd, and gateway lease/reconnect semantics. The exact candidate and verification evidence are in the updated PR body.

Forward the slash invocation runtime into the branch action and resolve offscreen transcript, busy state, stored identity, profile, and workspace from that runtime cache.

Pin live runtime branches to their invocation gateway across async profile lookup, persist delayed children in the invocation profile namespace, and suppress stale focus effects after profile or route intent changes. Child runtime metadata remains isolated from foreground stores, while missing explicit targets fail closed.
@frizikk
frizikk force-pushed the fix/desktop-branch-command-target branch from 057a817 to fa153cc Compare August 17, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop /branch in a session tile branches the foreground chat

3 participants