[SCAL-327336] Fix org tools disappearing after token expiry: reconcile token before getSessionInfo

[rohitthughtspot]

SCAL-327336

Problem

After a cold-start reconnect (~24h, once the frozen props access token has expired), list_orgs/switch_org disappear and feature-flag gating misbehaves — even though the keep-warm token is still valid.

Cause

sessionInfo is fetched once at init via getSessionInfo(), which previously ran before postInit reconciled the keep-warm global token from the token-store DO. On a post-expiry reconnect the init fetch authenticated with the dead frozen props token, failed, and left sessionInfo null for the DO's lifetime. Org-tool visibility and flag gating read sessionInfo, so they silently degraded — while the DO still held a valid token (data calls, which reconcile the DO token at call time, kept working).

Fix

• Reorder — a preInit() hook reconciles the global token from the token-store DO before initializeService()/getSessionInfo, so the init fetch uses the kept-warm token and sessionInfo populates correctly.
• ensureSessionInfo() — fallback in listTools that refetches session info if it's still null (e.g. a transient init failure), guarded by an in-flight promise so concurrent list calls share one fetch (no duplicate getSessionInfo / MixpanelTracker).
• isOrgsEnabled() defaults to true when sessionInfo is absent, so org tools stay visible in the brief window before the refetch completes.
• callTool reconciles the global token per call keyed on the absence of an active org token.
• Extract postInit's active-org bootstrap into ensureActiveOrg().

Test

Regression test reproduces the failure (init getSessionInfo fails on an expired props token; DO holds a valid token) and asserts the repair restores sessionInfo and the org tools. Full suite passes (695 tests).

Supersedes #184 (rebased onto latest main as a single clean commit).

🤖 Generated with Claude Code

[Rifdhan]

@copilot review this change and identify any gaps or potential issues, but don't make any code changes yourself

[Copilot]

@copilot review this change and identify any gaps or potential issues, but don't make any code changes yourself

Here's a review of the changes with the notable gaps and potential issues:

---

What the PR does well

• The root cause (expired props token used before DO token reconciliation) is correctly identified and addressed.
• preInit hooking before initializeService is the right architectural fix.
• The concurrency guard in ensureSessionInfo (single in-flight promise, .finally cleanup) is correctly implemented.
• isOrgsEnabled() defaulting to true when sessionInfo is null is a sensible "fail-open" choice for the repair window.
• The initGlobalTokenAndReconcileWithStorage logic correctly prefers a newer stored token and refuses to seed an expired props token into the DO.

---

Gaps and potential issues

1. ensureSessionInfo is only called in listTools, not callTool

Feature flags from sessionInfo — isSpotterDataSourceDiscoveryEnabled() and isSpotterChatHistoryEnabled() — are also consumed inside callTool (e.g. in createAgentConversation at line 732–733). If a client invokes a tool directly without a prior listTools, sessionInfo could still be null and those flags will fail-open (return true). That's a safe failure mode, but it means the repair path isn't fully covered for callTool.

2. Unbounded retries in ensureSessionInfo after repeated failures

If initializeService() always throws (e.g. the cluster is down), sessionInfo stays null and the .finally clears sessionInfoPromise. Every subsequent listTools call re-triggers initializeService, resulting in an unbounded sequence of getSessionInfo network requests — one per listTools invocation, with no back-off or per-session retry cap. This could be noisy in logs and put unnecessary load on the upstream cluster during an outage.

3. callTool reconciliation condition is semantically wider than the old code

Old: if (!this.areOrgToolsAvailable() && oauth)
New: if (oauth && !this.activeOrgToken)

The new condition reconciles the global token even when org tools are available but no active org is selected (the old code would have skipped reconciliation in that case, since areOrgToolsAvailable() would be true). This is actually the correct intent, but it's a behavioural change beyond what the PR description states. It's worth calling out explicitly, as it affects all OAuth users in an org-enabled cluster who haven't selected an org.

4. grantHasRefreshToken now set in preInit (subtle coupling)

grantHasRefreshToken is set at the very start of preInit, before the inner try block. Since preInit itself is wrapped in a try/catch in init(), any unexpected throw before that assignment (unlikely, since it's a simple property read) would leave grantHasRefreshToken = false and silently disable org tools. Minor theoretical risk, but the coupling is worth being aware of.

5. ensureActiveOrg has an unused recorder parameter

postInit calls await this.ensureActiveOrg() with no argument; the recorder?: MetricsRecorder parameter is a dead parameter today (it's passed on to forceRecreateActiveOrgToken but will always be undefined in this call path). Not a bug, but it's dead surface area.

6. Test for trackers.size is an internal-implementation assertion

The concurrency test asserts (server as any).trackers.size to verify no duplicate trackers are added. This couples the test to the internal structure of the Trackers class. If Trackers is ever refactored, the test breaks for a structural reason rather than a behavioural one. A more robust assertion would be to count getSessionInfo invocations directly (which the test already does via warmSessionInfoCalls.count) and rely on that as the single source of truth.

---

Summary

The core fix is sound and addresses the problem correctly. The most actionable gaps are #1 (session repair not covering callTool) and #2 (unbounded retries). #3 is a silent behaviour change worth documenting. The rest are minor.