Skip to content

fix(desktop): route project session branches by owner - #97359

Closed
evan-bradford wants to merge 4 commits into
NousResearch:mainfrom
evan-bradford:fix/project-session-branch-owner
Closed

evan-bradford wants to merge 4 commits into
NousResearch:mainfrom
evan-bradford:fix/project-session-branch-owner

Conversation

@evan-bradford

@evan-bradford evan-bradford commented Aug 28, 2026 •

Copy link
Copy Markdown

Summary

Create a Desktop conversation branch through the parent session's owning profile when that parent comes from a profile-scoped Project tree.

This fixes the observed case where a session owned by a non-default profile was visible under a Project, but Branch could not resolve/read it or created the child through the active/default profile.

Root cause

Profile ownership was lost across three boundaries:

  1. Profile-scoped projects.tree / projects.project_sessions rows were not self-describing.
  2. Sidebar and tile branch callbacks forwarded only the session ID.
  3. branchStoredSession() could let an ownerless Recents duplicate mask the authoritative Project-tree row, and an explicitly forwarded profile skipped or mis-scoped parent metadata resolution (losing the worktree cwd or borrowing it from a same-ID session in another profile).

Fix

  • Stamp the authoritative requested profile onto project-tree preview and nested session rows after scoped DB construction.
  • Include project-tree sessions in stored-session owner resolution.
  • Prefer a profile-bearing row over an ownerless duplicate for the same stored ID.
  • Forward optional owner profile through sidebar and tile branch actions.
  • Route parent message reads and session.create through that profile.
  • Resolve parent metadata inside the explicitly forwarded profile, preserving the correct worktree cwd and failing closed rather than borrowing another profile's metadata.
  • Preserve profile-less/single-profile behavior by omitting profile routing when no owner exists.

Regression coverage

  • Parent exists only in a non-default profile Project tree.
  • Ownerless Recents duplicate cannot mask an owned Project-tree row.
  • Explicit owner profile plus uncached parent still resolves worktree cwd.
  • Same stored ID in two profiles cannot route to one profile while inheriting the other's cwd.
  • Genuinely profile-less parent remains unscoped.
  • Sidebar wiring and session-tile delegation carry ownership.
  • Profile-scoped project overview and drill-in rows carry the requested profile.
  • Native Electron E2E: visible persisted parent row → Session actions → Branch → nested child.

Verification on current main

  • Desktop targeted Vitest: 123 passed
  • Python profile/project suites: 39 passed
  • Desktop typecheck: passed
  • Touched-file ESLint: passed
  • Production Desktop build + assert-dist-built: passed
  • Added-line security scan: no hardcoded-secret, shell-injection, eval/exec, or pickle findings
  • Initial independent review caught same-ID cross-profile metadata leakage; the exact default-vs-work CWD case was reproduced RED and is GREEN in follow-up 2fc2408fc
  • Independent filesystem-aware runtime-head review of 2fc2408fc — PASS, no blockers
  • Test-only tip b5f9c2619 additionally pins explicit-profile fetch failure against a foreign same-ID cache row
  • Native macOS E2E on exact runtime head 2fc2408fc: arm64 production build, then visible persisted parent → Session actions → Branch → nested child — 1/1 passed in 11.1s

Relationship to open PRs

Cache/profile safety

No prompt, message history, memory, or toolset state is mutated. Ownership is carried as request/display metadata and used only to select the existing profile-scoped session APIs.

@evan-bradford
evan-bradford force-pushed the fix/project-session-branch-owner branch from fa61ba1 to e72bc2d Compare August 28, 2026 18:45
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) area/profiles Multi-profile isolation, HERMES_HOME scoping sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 28, 2026
@evan-bradford
evan-bradford marked this pull request as draft August 28, 2026 19:07
@evan-bradford
evan-bradford marked this pull request as ready for review August 28, 2026 19:15
@evan-bradford

Copy link
Copy Markdown
Author

Live macOS deployment verification — 2026-08-28

I built the combined reviewed source at 96d0deb87d8726ed784e83595177bf2a42c70b1b natively on Apple Silicon and installed it atomically on the real Desktop client.

Provenance and packaging:

  • host/target: Darwin arm64
  • executable: Mach-O 64-bit executable arm64
  • bundle: com.nousresearch.hermes, package type APPL
  • strict ad-hoc signature verification: passed
  • installed install-stamp.json: exact commit 96d0deb87d8726ed784e83595177bf2a42c70b1b
  • isolated native Electron branch E2E on the same combined source: 1/1 passed in 13.9s

After backend restart and app relaunch, the actual affected parent session appears exactly once through the sfb project drill-in, carries profile: "sfb", and carries its correct linked-worktree CWD. The installed app is connected to the restarted backend with a matching current token and one visible native window.

I intentionally did not create a throwaway branch in the production session DB; the exact branch interaction is exercised by the native Electron E2E, while live verification remained read-only.

Launch/default project-tree rows are now stamped profile="default" by
stamp_profile (the cross-profile ownership fix), which forwards an explicit
profile into branchStoredSession. When that explicit profile equals the
ACTIVE gateway profile ("default" on a launch-profile backend) and the
by-id getSession miss resolves to undefined, the fail-closed arm dropped the
parent row's cwd, so the created branch had no workspace and nothing appeared
where the user clicked until a tree/session refresh repainted it.

Soft-fall to the cached row when the explicit profile equals the active
gateway profile — that is not a cross-profile claim, so no metadata can
leak; genuinely foreign profiles still fail closed as before (sfb routing
unchanged). Adds a regression test reproducing the launch-profile miss and
asserting cwd/parent survive. Verified: 100/100 targeted tests, eslint
clean. Follow-up to NousResearch#97359 which exposed this launch-row corner.
@evan-bradford

Copy link
Copy Markdown
Author

Follow-up: keep launch-profile parent metadata in branch owner path (51b6f2fe)

This PR's explicit-profile arm went fail-closed for the launch profile — a regression exposed in rollout (default-profile branches appeared to fail; the branch was actually created but with no workspace until a tree/session refresh repainted it).

Root cause

stamp_profile now stamps default-profile project-tree rows profile="default" (default IS the launch home, so _response_profile_name falls to _current_profile_name()). branchStoredSession then forwards an explicit "default" into resolveStoredSession(id, "default"), whose fail-closed string-arm (utils.ts:1348-1351) returns undefined on a by-id getSession miss — dropping the parent's cwd before forkBranch. The branch lands with no workspace → nothing appears where the click happened → navigate-away-and-back (refetch) "fixes" it.

Fix

Soft-fall to the cached row only when the explicit profile equals the active gateway profile — that is not a cross-profile claim, so no metadata can leak. Genuinely foreign profiles still fail closed (sfb routing unchanged; the existing 'work' foreign-profile tests still pass).

Verification

  • New regression test (launch-profile by-id miss) was RED before the fix (asserted cwd/parent survive — failed with cwd dropped).
  • GREEN after: this worktree's targeted suite use-session-actions.test.tsx + resolve-stored-session.test.ts → 100/100 passed.
  • eslint clean on both files; change is 47 lines in 2 files.

The launch-row stamp corner was previously flagged as untested (Opus review finding #5 on the cross-profile branch fix) — this closes that gap on the branch side.

@evan-bradford

Copy link
Copy Markdown
Author

Owner note for any agent resuming this PR: commit 51b6f2fe3 was authored and pushed today (Aug 28) from the 'Fix branch creation in sfb profile' branch session's child, coordinating the default-profile launch-row regression follow-up. Treat it as owned work — not a conflict. Diagnostics, the RED-GREEN regression test, and the full verification (100/100 targeted tests, lint clean) are in this thread's previous comment. Do not rebase it away; if the branch needs force-pushing for any reason, preserve or re-cherry-pick 51b6f2fe3 on top.

@OutThisLife

Copy link
Copy Markdown

Superseded by #100879, which consolidates the full branch-load path.\n\nCarried from here: the gateway-side profile stamping on scoped project rows (), committed under your authorship, plus preferring a self-describing row over an ownerless Recents duplicate. The routing half is subsumed by full owner routing \u2014 a profile name alone can't disambiguate two connections that both expose , which is the case in the original report. The owner now resolves inside , so the sidebar/tile profile passthrough isn't needed.

@OutThisLife OutThisLife closed this Sep 2, 2026
@OutThisLife

Copy link
Copy Markdown

Superseded by #100879, which consolidates the full branch-load path.

Carried from here: the gateway-side profile stamping on scoped project rows (stamp_profile), committed under your authorship, plus preferring a self-describing row over an ownerless Recents duplicate.

The routing half is subsumed by full (connection, profile) owner routing — a profile name alone can't disambiguate two connections that both expose default, which is the case in the original report. The owner now resolves inside branchStoredSession, so the sidebar/tile profile passthrough isn't needed.

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

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists 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.

3 participants