Review — end-to-end ✅ (approved with small cleanup pushed)

What this ships

Three coordinated bugs fixed in profile switching (#1200), plus a flaky-test stabilisation:

Plus 9 new tests in tests/test_profile_switch_1200.py (5 direct verification + 4 named regression guards).

Traced against upstream hermes-agent

Profile system is shared between WebUI and hermes-agent CLI. Pulled fresh nousresearch/hermes-agent tarball.

The agent reads profile state from ~/.hermes/profiles/<name>/ directly — no dependence on WebUI's request-scoped thread-local profile. The PR's fix reads from the target profile's resolved home directory, which is the same directory the agent uses. Cross-tool consistency: a workspace persisted by the agent CLI under <profile>/webui_state/last_workspace.txt would also be picked up by the WebUI's switch-profile flow. ✅

The webui_state/ subdirectory under each profile home is WebUI-specific (set up at api/workspace.py:46). The agent CLI writes its own state under different paths. No conflict.

End-to-end trace

Bug 1 — thread-local mismatch confirmation

api/profiles.py:94-104 — get_active_profile_name() returns _tls.profile (set per-request from cookie) or falls back to _active_profile (process-global). During a process_wide=False switch:

1. Request arrives with the OLD profile cookie → set_request_profile(OLD) was called by the routing layer
2. Handler calls switch_profile(NEW, process_wide=False)
3. Inside the function, _tls.profile is still OLD (we haven't received a new request yet)
4. The PR's pre-fix code called get_last_workspace() → get_active_profile_name() → returns OLD → reads OLD's last_workspace.txt → wrong workspace
5. Set-Cookie response is sent. NEXT request will have NEW cookie set in TLS, but THIS response already has the wrong default_workspace baked in.

The new code bypasses the thread-local by reading from home / 'webui_state' / 'last_workspace.txt' where home was already resolved at line 261-263 from the user-supplied name parameter. Direct, no thread-local indirection. ✅

Fallback chain order is sensible:

1. last_workspace.txt — most recent user-chosen for this profile
2. cfg.workspace / cfg.default_workspace — explicit profile config keys
3. cfg.terminal.cwd — agent CLI's working dir (most common shared key)
4. DEFAULT_WORKSPACE — boot-time constant

Each step verifies is_dir() so a stale path doesn't propagate.

Bug 2 — cache invalidation timing

api/routes.py:1553-1562:

result = switch_profile(name, process_wide=False)
from api.config import invalidate_models_cache
invalidate_models_cache()
return j(handler, result, extra_headers={'Set-Cookie': build_profile_cookie(name)})

invalidate_models_cache() clears BOTH the in-memory cache and the on-disk cache (api/config.py:1148-1172). The PR's claim about disk being single-per-server (not per-profile) is correct — verified by reading the cache implementation; full invalidation is necessary.

Order: switch happens, cache cleared, response sent with cookie. The next request (with NEW cookie) hitting /api/models cold-builds from the new profile's config. ✅

Bug 3 — early-return chip update

static/ui.js:1856-1864:

if(!S.session){
  ...
  if(typeof syncAppTitlebar==='function') syncAppTitlebar();
  // Update profile chip even when no session is active (e.g. right after profile switch)
  const _profileLabel=$('profileChipLabel');
  if(_profileLabel) _profileLabel.textContent=S.activeProfile||'default';
  return;
}

Verified the main-path chip update at the end of the function reads the same S.activeProfile value, so the new branch matches semantics. ✅

Bug 4 — flaky test stabilisation

The original tests asserted clientNow - serverNow === 3_600_000 (exact ms equality). Both clientNow and serverNow call Date.now(), but at different physical times — so a 1ms increment between the two calls produces a diff of 3,600,001 ms, failing the equality check. The fix samples t0 before and t1 after, takes the midpoint, asserts within ±5ms. Sound stabilisation pattern.

What I caught — latent NameError in the workspace fallback

The original PR's switch_profile code did a redundant local import:

from pathlib import Path as _Path  # but Path is already imported at line 17

In the exception handler:

except Exception:
    try:
        from api.config import DEFAULT_WORKSPACE as _DW2
        default_workspace = str(_DW2)
    except Exception:
        default_workspace = str(_Path.home())  # _Path may be unbound

If the OUTER try fails before line from pathlib import Path as _Path runs (e.g., the from api.config import DEFAULT_WORKSPACE as _DW import on the line above raises), then _Path is never bound. The double-fallback at str(_Path.home()) would raise NameError rather than gracefully producing a path.

In practice this is unreachable — api.config is already imported at module load time (line 17), so a second import attempt won't suddenly fail. But the code surface is unnecessary.

What I pushed — 5634f21

Removed the redundant Path as _Path aliasing; uses the module-level Path everywhere in the workspace-derivation block. Same behaviour, no latent NameError, cleaner. CI re-ran green on 5634f21.

Edge-case trace

Tests

• PR's 9 new profile-switch tests + my pushed cleanup: pass.
• #1144 stabilised tests: 23/23 in test_issue1144_session_time_sync.py, including my prior fractional-offset additions.
• Local full suite: 2728 passed, 47 skipped, 0 failures (the macOS test_sprint3 quirk now fixed by my #1186 in v0.50.231).
• CI on PR after my push: ✅ test (3.11), ✅ test (3.12), ✅ test (3.13).

Other audit — confirmed correct

• JS syntax: node --check passes.
• No agent coupling: profile system is shared but the read pattern matches what the agent CLI does.
• Profile lock not held during workspace lookup: the fallback chain runs after with _profile_lock exits, which is correct (the lookup is read-only against a stable home path).
• Browser QA in PR description: 3 manual switch sequences (default→camanji, camanji→webui, webui→default) all show correct chip/model/workspace immediately. Matches the static-test contract.

Minor observations (non-blocking)

• The process_wide=False branch reads cfg via direct yaml.safe_load rather than get_config() — that's intentional (the PR comment explains: _cfg_cache is process-global and would still hold the OLD profile's config). The slight code duplication is a worthwhile trade-off for correctness.
• _DW / _DW2 aliasing in the import statements (from api.config import DEFAULT_WORKSPACE as _DW) is unusual but harmless — protects against accidental shadowing within the function. Could be tightened to a single _DW outside the try block.
• The cookie + cache-clear + response ordering means the cache is invalidated even if cookie-setting fails. Not a real concern since cookie-setting is just a header append.

Recommendation

Approved (with cleanup pushed). Three real, distinct bugs in profile switching all addressed at the right layer: backend reads from the target profile's home directly (closing the thread-local race), routes call cache-invalidate after the switch (closing the model-list race), frontend chip updates from the no-session path. The flaky-test fix using midpoint averaging is the right pattern. My cleanup removes a latent NameError + redundant import without changing behaviour. Comprehensive 9-test coverage with explicit named regression guards. CI green; full suite clean. Parked at approval — ready for the release agent's merge/tag pipeline.