Skip to content

Join the profile this process already holds on a repeated session_start - #258

Merged
jonaslsaa merged 2 commits into
jonaslsaa:mainfrom
hustfyb:fix/session-start-keeps-open-profile
Oct 31, 2026
Merged

jonaslsaa merged 2 commits into
jonaslsaa:mainfrom
hustfyb:fix/session-start-keeps-open-profile

Conversation

@hustfyb

@hustfyb hustfyb commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

What changed

src/index.ts: when session_start asks for the profile this process already holds, keep the open
activation instead of taking the lock again.

const openProfile = async (name: string, ctx: ExtensionContext) => {
  const dir = profilePath(name);
  if (!existsSync(dir)) throw new Error(`Profile ${name} does not exist. …`);
+ // A host may start the session again while this process still holds the profile: T3 Code resumes and
+ // replaces sessions under a running extension host. Taking the lock again would meet our own socket and
+ // fail as if a second window owned the profile, so keep the open activation, as a connected window does.
+ if (active?.name === name) { await restoreMain(ctx, active); status(ctx); return; }
  const unlock = await holdProfile(…);

restoreMain is called so the second start still re-applies the profile's pinned main model, exactly
as the normal open path does.

Why

A host that replaces the session under a running extension (T3 Code does this; it runs one Pi process
for its threads and passes --mode rpc --session <file>) sends a second session_start into an
instance that already has the profile open. openProfile then calls holdProfile, which meets its own
lock socket: lockProfile gets EADDRINUSE, readOwner answers with this process's own description
(<profile> · PID <own pid> · …), and ProfileBusyError is raised.

For the failure the crash is not a menu: in session_start's catch,

if (!(error instanceof ProfileBusyError) || ctx.mode !== 'tui') throw error;

an RPC host rethrows, fault is set, and the host's session start fails. The TUI path instead shows the
"open in another window" menu, which is why this only bites non-interactive hosts.

Test

test/reopen.test.ts starts a fixture session, then emits a second session_start (reason: 'reload')
into the same extension instance and asserts that no busy/missing-profile note is reported and that the
profile is still usable afterwards.

  • on main: fails with Profile already running: fixture · PID <own pid> · …
  • with the change: passes

Checked with npm run check (tsc, clean) and npm test: 364 tests, 363 pass, 1 skipped, 0 fail.

Scope

Only the "same process, same profile" case changes; any other profile name (a real second window, a
/optchat profile switch) still goes through holdProfile and the existing menus. A profile switch in
one process still leaves the previous activation's lock in place, which this change does not touch — say
if you want that handled too and I will send it separately.

@hustfyb
hustfyb requested a review from jonaslsaa as a code owner October 31, 2026 02:38
@hustfyb

hustfyb commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

The pending check is GitHub's fork-approval gate, not a failure: the Check run for this branch is
action_required, so nothing has run yet (workflows from a fork wait for a maintainer to approve them).

Evidence in the meantime — the same commit's full Check workflow (ubuntu test + windows) passes on my
fork: https://github.com/hustfyb/optchat-t3/actions/runs/41161271349

Locally it is also npm run check clean and npm test at 364 tests, 1 skipped, 0 failed.

hustfyb and others added 2 commits October 31, 2026 22:27
T3 Code runs many Pi sessions in one process and replaces the session under the
running extension. session_start then reaches an instance whose profile is
already open, and openProfile took the lock again: holdProfile meets this
process's own socket, readOwner answers with our own description, and
ProfileBusyError takes the session down, because outside the TUI it is rethrown.

Keep the open activation when the requested name is the one already active, and
re-apply the profile's main model as a fresh open would.

test/reopen.test.ts emits a second session_start into the same instance; on main
it fails with "Profile already running: … · PID <own pid> · …".
@jonaslsaa
jonaslsaa force-pushed the fix/session-start-keeps-open-profile branch from 92297a5 to 165f746 Compare October 31, 2026 21:28
@jonaslsaa

Copy link
Copy Markdown
Owner

Thanks for this, and for the clear write-up! The bug is real. I reproduced it with pi --mode rpc: after a session switch, OptChat throws on its own lock and then stops the profile, so the next prompt gets refused.

The cause turned out to be slightly different from a reload, though. Pi's RPC mode binds extensions twice for the same session after new_session, switch_session, fork and clone: once through setRebindSession inside the runtime, and again through the explicit rebindSession() in the RPC handler. So session_start fires twice for the same session, with no shutdown between them.

The old-lock part doesn't hold up: when you switch to a session bound to another profile, Pi sends session_shutdown first, and that releases the previous lock. I checked it from outside the process.

On top of your commit I pushed:

  • a guard so the open profile is kept only when the session ID matches; a different session that arrives without a shutdown still fails loudly
  • a comment naming Pi's double bind
  • a test that calls bindExtensions twice, the same way Pi does (it fails on main and passes with the fix)

Merging once CI is green. Thanks again!

@jonaslsaa
jonaslsaa merged commit a1d7868 into jonaslsaa:main Oct 31, 2026
2 checks passed
@jonaslsaa jonaslsaa mentioned this pull request Oct 31, 2026
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.

2 participants