Thanks for catching this — the diagnosis is correct. The api() wrapper in static/workspace.js does window.location.href = '/login?next=...'; return; on 401, which means callers receive undefined and the navigation is async, so any code that runs before the page actually unloads will hit Cannot read property 'session' of undefined. The three guards you added (loadSession, _ensureMessagesLoaded, _loadOlderMessages) cover the hot paths and the diff is clean.

One follow-up I'd ask before merge: there's a fourth callsite with the same shape that this PR doesn't touch — _ensureAllMessagesLoaded() around line 645:

async function _ensureAllMessagesLoaded() {
  if (!_messagesTruncated || !S.session) return;
  const sid = S.session.session_id;
  const data = await api(`/api/session?session_id=...&messages=1&resolve_model=0`);
  const msgs = (data.session.messages || []).filter(m => m && m.role);  // ← same crash
  ...
}

This one is called from undo / export / context-restore paths. Less common than loadSession(), but the same 401-during-await race exists. Could you add the same if (!data || !data.session) return; guard there too? Then the loadSession family is fully covered.

(The _resolveSessionModelForDisplaySoon() callsite at ~line 530 is already safe — it's wrapped in try/catch and uses data && data.session && data.session.model defensively.)

Other than that the patch is good. The early-return on !data (rather than !data.session) for loadSession itself is the right call since we want to bail before the stale-response check too. Happy to merge once _ensureAllMessagesLoaded is patched.

Thanks for the follow-up commit — _ensureAllMessagesLoaded now has the same if (!data || !data.session) return; guard, which closes the last 401-during-await race in the loadSession family. The diff is exactly what I had in mind:

• loadSession() — guards on !data (correctly placed before the stale-response check, so we bail without clearing _loadingSessionId for someone else's request)
• _ensureMessagesLoaded() — !data || !data.session ✓
• _loadOlderMessages() — !data || !data.session and resets _loadingOlder before returning ✓ (good catch on the flag reset — if I had merged my earlier suggestion blindly the lock would have leaked)
• _ensureAllMessagesLoaded() — added per the previous round ✓

LGTM on my end. Leaving final merge call to the maintainer.

Thanks for the follow-up commit eafda3c — separate concern from the 401 fix, but I see what you're addressing. Let me review the mobile dropdown anchor change inline.

// Before
if(!dd||!chip||!footer) return;
const anchor=(panel&&panel.classList.contains('open')&&mobileAction)?mobileAction:chip;

// After
if(!dd||!footer) return;
const anchor=(panel&&panel.classList.contains('open')&&mobileAction)?mobileAction:(chip&&chip.offsetParent?chip:mobileAction);
if(!anchor) return;

The reasoning is sound: on mobile breakpoints composerModelChip is hidden via display:none (not visibility:hidden), so chip.offsetParent === null is the right test for "is the chip actually rendered." Falling back to mobileAction when the chip is hidden lets the dropdown anchor to the visible mobile action button instead of being positioned off-screen relative to a zero-rect element.

Two small concerns:

1. chip.offsetParent returns null for position:fixed elements even when they're visible. That's almost certainly not the case for composerModelChip (it's inside the composer-footer flow), but worth a quick visual confirmation that the chip isn't styled position:fixed anywhere. If it is, the fallback would fire on desktop too and you'd get the wrong anchor.
2. Both anchors null. If neither chip.offsetParent nor mobileAction exists (edge case: composer not yet rendered, or both elements removed), the new if(!anchor) return; correctly bails. Good.

The only thing I'd ask is a comment explaining why the chip might be invisible — the offsetParent trick is correct but non-obvious to a future reader. Something like:

// On mobile, #composerModelChip is display:none and #composerMobileModelAction
// is the visible target. offsetParent===null detects the hidden state cheaply
// (display:none returns null, visible returns ancestor).

Scope concern

This commit is unrelated to the 401-redirect fix in static/sessions.js. I'd prefer to see this split into a separate PR — it makes the merge log cleaner and lets each fix be reverted independently if needed. Not a blocker, but a strong preference. If you want to keep them together, please:

1. Update the PR title to cover both fixes (e.g., fix(sessions+ui): 401 redirect guard + mobile model dropdown anchor)
2. Add a section to the PR body describing the dropdown change separately

Otherwise the original fix(sessions): handle 401 redirect gracefully... title is now misleading.

Verdict on the original fix

The loadSession/_ensureMessagesLoaded/_loadOlderMessages/_ensureAllMessagesLoaded guards are clean — if(!data || !data.session) return; is the minimal correct shape. All four call sites consistently use the same pattern. Good.

If you (a) split the dropdown fix into its own PR or (b) update the title/body to cover both, I'd merge.