Skip to content

fix(gateway): resolve a session row's model under its own profile, not the launcher's - #325

Merged
OmarB97 merged 1 commit into
mainfrom
fix/session-row-launch-profile-model-fork-2026-08-02
Aug 2, 2026
Merged

fix(gateway): resolve a session row's model under its own profile, not the launcher's#325
OmarB97 merged 1 commit into
mainfrom
fix/session-row-launch-profile-model-fork-2026-08-02

Conversation

@OmarB97

@OmarB97 OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

What's wrong

_ensure_session_db_row writes the first DB row for a desktop/TUI session. It is called from prompt.submit on the RPC thread — before the per-turn set_hermes_home_override the turn thread binds — so every HERMES_HOME read inside it answers for the launch profile.

The model fallback is where that becomes permanent:

row_model = str(override.get("model") or "").strip() or _resolve_model()

With no composer model, _resolve_model()_load_cfg(), which honours a bound override but has none here, so it reads the launch profile's config.yaml. A session created under profile: "worker" gets the launcher's default model written into worker's state.db.

And it cannot be repaired. _insert_session_row upserts under model = COALESCE(sessions.model, excluded.model) — first writer wins — so the agent's own correct lazy-create in run_agent.py is a no-op against it.

This is the same failure the comment directly above that line was written to close:

Writing the global default here used to win the INSERT-OR-IGNORE race against the agent's own correct lazy-create

That was fixed for the composer-override case. The no-override + foreign-profile case still hit it.

The fix

Bind the session's own profile_home around the resolution (new _profile_home_bound context manager). The fallback now names the model that profile is actually configured for — the same value _make_agent resolves under the same binding — so both writers agree and COALESCE has nothing to freeze.

An explicit composer pick still wins untouched: the anti-race intent is unchanged, and a session with no profile_home takes the identical path it did before.

All four callers of the helper — prompt.submit, the /model switch marker, /title, and handoff — are on unbound RPC threads, so fixing it inside the helper covers the whole class rather than the one call path that surfaced it.

Also in this PR

profile_name was never passed. _insert_session_row has always accepted it; this caller never sent it. The agent backfills it on its first turn, so an unattributed row is usually only briefly wrong — but a session whose turn never runs (a spawn that dies before its first prompt) keeps a NULL profile_name forever. "default" maps to NULL, mirroring the agent's own normalization so the two writers cannot disagree under COALESCE.

Sibling mis-binding, same root cause. session.create and both lazy-resume responses computed profile_name (and their model fallback) with nothing bound, while the deferred build — the one caller that does bind the home — computed them correctly. A client creating a session under profile: "worker" was told "launcher", then watched both fields flip when session.info landed. _session_info now resolves the name from the session record instead of the ambient home, which fixes it at all ~28 call sites rather than one; _lazy_resume_info takes the home explicitly.

Evidence

Two rows in a real meshboard-game-dev profile db, from sessions whose turn never ran:

model_config.provider profile_name
ai-router (the root profile's provider) NULL

Healthy rows in the same db carry meshboard-qualified-local and meshboard-game-dev. The provider on those two rows came from the client's composer pick (a separate, desktop-side concern), but the NULL profile_name is this PR's second half, and it is what makes such a row permanently unattributed.

Note for anyone re-checking that db: both profiles happen to name the same default model, so the model column alone does not discriminate. The model half of the defect is proven by the tests below, not by those rows.

Verification

Real temp profiles, no mocks — added to tests/test_profile_isolation_runtime.py, whose existing brief is exactly this bug class. Two on-disk profiles whose config.yaml name different defaults, driven through the real _ensure_session_db_row, real SessionDB, and the real session.create RPC.

On the parent commit, 5 of the 6 new tests fail:

assert 'launcher/model-A' == 'worker/model-B'
assert 'launcher' == 'worker'

The sixth (test_explicit_composer_pick_still_wins) guards the existing anti-race behavior and passes on both — that is the point of it.

With the fix: 6/6 pass. Wider run of tests/test_tui_gateway_server.py tests/tui_gateway/ tests/test_profile_isolation_runtime.py gives 876 passed, 4 failed — the same 4 failures the parent commit produces on this machine (pre-existing, host-config-dependent), so no regressions. tests/hermes_state/ + related session suites: 513 passed.

Risk

Low. No change to the system prompt, message alternation, or anything cached per conversation. A session with no profile_home (the ordinary local path) resolves exactly as before — _profile_home_bound(None) is a no-op. The behavior change is confined to sessions that already carry a foreign profile home, where the previous value was wrong.

…t the launcher's

`_ensure_session_db_row` writes the first DB row for a desktop/TUI session.
It runs from `prompt.submit` on the RPC thread — *before* the per-turn
`set_hermes_home_override` that the turn thread binds — so every HERMES_HOME
read inside it answers for the LAUNCH profile.

The model fallback is where that becomes permanent. When the composer sends
no explicit `model`, the helper fell through to `_resolve_model()`, which
reads `_load_cfg()`, which honours a bound override but had none here. A
session created under `profile: "worker"` therefore had the *launch*
profile's default model stamped into the *worker* profile's `state.db`. And
because `_insert_session_row` upserts under
`model = COALESCE(sessions.model, excluded.model)`, the agent's own correct
lazy-create (`run_agent.py`) cannot repair it — first writer wins.

This is the same failure the comment above that line was written to close
("Writing the global default here used to win the INSERT-OR-IGNORE race
against the agent's own correct lazy-create"). It was fixed for the
composer-override case; the no-override + foreign-profile case still hit it.

Bind the session's own `profile_home` around the resolution. The fallback
now names the model that profile is actually configured for — the same value
`_make_agent` resolves under the same binding — so both writers agree and
COALESCE has nothing to freeze. An explicit composer pick still wins
untouched, so the anti-race intent is unchanged. All four callers of the
helper (prompt.submit, the /model switch marker, /title, and handoff) are on
unbound RPC threads, so fixing it inside the helper covers the whole class.

Also pass `profile_name`, which `_insert_session_row` has always accepted and
this caller never sent. The agent backfills it on its first turn, so an
unattributed row is usually only briefly wrong — but a session whose turn
never runs (a spawn that dies before its first prompt) keeps a NULL
`profile_name` forever. Two such rows are what surfaced this. "default" maps
to NULL, mirroring the agent's own normalization so the two writers cannot
disagree under COALESCE.

Sibling mis-binding, same root cause: `session.create` and the two
lazy-resume responses computed `profile_name` (and their model fallback)
with nothing bound, while the deferred build — the one caller that *does*
bind the home — computed them correctly. A client creating a session under
`profile: "worker"` was told "launcher", then watched both fields flip when
`session.info` landed. `_session_info` now resolves the name from the session
record rather than the ambient home, which fixes it at all ~28 call sites
instead of one; `_lazy_resume_info` takes the home explicitly.

Verified against real temp profiles (`tests/test_profile_isolation_runtime.py`),
not mocks: two on-disk profiles whose `config.yaml` name different defaults,
driven through the real `_ensure_session_db_row`, real `SessionDB`, and the
real `session.create` RPC. Five of the six new tests fail on the parent
commit; the sixth guards the existing anti-race behavior and passes on both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@OmarB97
OmarB97 merged commit c213924 into main Aug 2, 2026
31 checks passed
OmarB97 added a commit that referenced this pull request Aug 2, 2026
…ile (#329)

Sibling of #325, same root cause on the other half of the composer. Provider
names are per-profile vocabulary — `providers:` / `custom_providers:` live in
each profile's own config.yaml — and three RPC handlers resolved them on the
RPC thread with no HERMES_HOME bound, so they all answered for the LAUNCH
profile.

Measured on two real temp profiles ("launcher" and "worker", each with its own
provider), driving the real RPCs:

  model.options for a WORKER session
    → provider 'launch-router', model 'launcher/model-A', and the provider
      list contained launch-router but not worker-local

  config.set key=model applying worker's OWN provider to a WORKER session
    → error: "Unknown provider 'worker-local'"

So the picker could only offer names the session's profile does not define,
and refused the one name it does. Picking from that list is how a session ends
up stamped with a provider its own profile cannot resolve — the turn then dies
in agent init on "Unknown provider", leaving the 0-message husk #321 fixed for
`hermes desktop spawn --provider`. This is the desktop composer's path to the
same dead end.

Three sites, one binding each:

* `model.options` — build the picker under the profile it is FOR.
* `config.set key=model` — apply the pick under the same profile the picker
  offered it from. `_apply_model_switch` reads `providers:` through
  `load_config()` and asks `resolve_runtime_provider` to name the target, all
  HERMES_HOME reads. Its body moves to `_config_set_model` unchanged so the
  caller can wrap it; `switch_model` persists through `save_config()`, which
  resolves the same home, so a `--global` write from a foreign-profile session
  now lands in that profile's config.yaml rather than the launcher's.
* `model.save_key` — the picker's "connect" action. `.env` is per-profile, so
  the key has to land in the profile whose providers are being shown; the
  session lookup moves above the write to make that possible. Without this the
  model.options fix would leave a just-connected row still unauthenticated.

`model.disconnect` is deliberately untouched: it takes no session and is a
global provider disconnect, so there is nothing to scope it by.

A live session is resolved by its OWN home — that is where its turn will run.
For the picker's other opening, a chat that does not exist yet, there is no
session to ask, so `model.options` and `model.save_key` now also accept
`profile`. That is not a new policy: the REST twin `/api/model/options` has
always taken it and documents it ("``profile`` scopes the picker context"), so
this closes a divergence between two surfaces whose response shapes are
otherwise 1:1. The desktop sends it via a new `apiRequestProfile()` accessor
over the same `_apiProfile` its REST calls already scope with — which is what
fixes the picker opened before a chat exists, the one path that reaches the
gateway with no session id.

Session first, `profile` second, so an existing session can never be
mis-scoped by a stale client hint.

Verified against real temp profiles in tests/test_profile_isolation_runtime.py
(no mocks) plus the desktop unit tests. Four of the seven new Python tests fail
on the parent commit; the rest are controls — a launch-profile session must
still take the unbound path, and a provider only the LAUNCH profile defines
must still be rejected (it was ACCEPTED before this, which is the exact
mechanism that stamped a foreign provider onto a session row).

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
OmarB97 pushed a commit that referenced this pull request Aug 2, 2026
`_get_db()` is a cached module global bound to `hermes_state.DEFAULT_DB_PATH`,
evaluated at *import* time from the launch profile's home. A session created
with `profile: "<other>"` keeps its rows in `<root>/profiles/<other>/state.db`,
reached through the profile-aware `_session_db(session)` helper.

#323 (/undo) and #324 (`_finalize_session`) each fixed one site of this. This
audits every remaining `_get_db()` call in `tui_gateway/server.py` that runs
with a live `session` dict in hand and fixes the twelve that are wrong.

Reproduced against real SessionDB files under a temp home with two profiles:

- `session.branch` died outright with "branch failed: FOREIGN KEY constraint
  failed" — the branch row is an FK child of the parent's row, which the launch
  db does not have. The child now inherits the parent's profile end to end (row,
  agent handle, `profile_home`, and the `_profile_home_bound` binding around
  `_resolve_model()` that #325 established), because fixing only the write would
  have left its rows in one db and its live session pointing at another.
- `prompt.submit`'s edit truncation raised the same FK error, which also skipped
  `deactivate_turn_outcomes_from_ordinal` in the same `try`, and left the profile
  db holding the full pre-edit history — so resuming brought the edited-away
  turns back. Same class as #323.
- Both notification-ownership checks resolved the compression chain in the wrong
  db, so a post-compression profile session stopped recognising its own
  pre-compression dispatches and the fail-closed gate of NousResearch#55578 dropped the
  delegation completion. Both now share `_resolve_session_lineage_key`.
- `/history` replaced the live window with an empty launch-db read and answered
  "No conversation history yet." mid-conversation; `/context` under-reported.
- Titles: the read paths, the post-turn `pending_title` apply, and
  `maybe_auto_title` all used the launch db, so a profile session's title was
  either invisible or never persisted at all.
- `/status` found no row, so it had no Title and reported Created/Last Activity
  as "now"; `_background_agent_kwargs` wrote a profile session's background
  transcript into the launcher's state.db.

Where the handle must outlive the call — `maybe_auto_title`'s daemon thread,
`_background_agent_kwargs`' background agent, and the notification poll loop —
the fix uses the agent's own long-lived handle rather than a `_session_db()` one
that would be closed on block exit (or churn schema write locks per poll),
matching `_persist_live_session_runtime`.

Left launch-scoped deliberately: `session.list`, `session.most_recent`,
`session.delete`, `projects.*` and `insights.get` hold no session and take no
profile — the desktop routes them per-profile by connecting to that profile's
gateway (apps/desktop/src/store/projects.ts:252).

Verified with 20 tests over real SessionDB files under a temp HERMES_HOME with
two profiles, no db mocks; 16 fail on the parent commit and 4 are controls that
pass on both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
OmarB97 added a commit that referenced this pull request Aug 2, 2026
… db (#333)

`_get_db()` is a cached module global bound to `hermes_state.DEFAULT_DB_PATH`,
evaluated at *import* time from the launch profile's home. A session created
with `profile: "<other>"` keeps its rows in `<root>/profiles/<other>/state.db`,
reached through the profile-aware `_session_db(session)` helper.

#323 (/undo) and #324 (`_finalize_session`) each fixed one site of this. This
audits every remaining `_get_db()` call in `tui_gateway/server.py` that runs
with a live `session` dict in hand and fixes the twelve that are wrong.

Reproduced against real SessionDB files under a temp home with two profiles:

- `session.branch` died outright with "branch failed: FOREIGN KEY constraint
  failed" — the branch row is an FK child of the parent's row, which the launch
  db does not have. The child now inherits the parent's profile end to end (row,
  agent handle, `profile_home`, and the `_profile_home_bound` binding around
  `_resolve_model()` that #325 established), because fixing only the write would
  have left its rows in one db and its live session pointing at another.
- `prompt.submit`'s edit truncation raised the same FK error, which also skipped
  `deactivate_turn_outcomes_from_ordinal` in the same `try`, and left the profile
  db holding the full pre-edit history — so resuming brought the edited-away
  turns back. Same class as #323.
- Both notification-ownership checks resolved the compression chain in the wrong
  db, so a post-compression profile session stopped recognising its own
  pre-compression dispatches and the fail-closed gate of NousResearch#55578 dropped the
  delegation completion. Both now share `_resolve_session_lineage_key`.
- `/history` replaced the live window with an empty launch-db read and answered
  "No conversation history yet." mid-conversation; `/context` under-reported.
- Titles: the read paths, the post-turn `pending_title` apply, and
  `maybe_auto_title` all used the launch db, so a profile session's title was
  either invisible or never persisted at all.
- `/status` found no row, so it had no Title and reported Created/Last Activity
  as "now"; `_background_agent_kwargs` wrote a profile session's background
  transcript into the launcher's state.db.

Where the handle must outlive the call — `maybe_auto_title`'s daemon thread,
`_background_agent_kwargs`' background agent, and the notification poll loop —
the fix uses the agent's own long-lived handle rather than a `_session_db()` one
that would be closed on block exit (or churn schema write locks per poll),
matching `_persist_live_session_runtime`.

Left launch-scoped deliberately: `session.list`, `session.most_recent`,
`session.delete`, `projects.*` and `insights.get` hold no session and take no
profile — the desktop routes them per-profile by connecting to that profile's
gateway (apps/desktop/src/store/projects.ts:252).

Verified with 20 tests over real SessionDB files under a temp HERMES_HOME with
two profiles, no db mocks; 16 fail on the parent commit and 4 are controls that
pass on both.

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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