feat(boot): cloud-mode picker auth + reload-resilient core mode#1357
Open
CodeGhost21 wants to merge 3 commits intotinyhumansai:mainfrom
Open
feat(boot): cloud-mode picker auth + reload-resilient core mode#1357CodeGhost21 wants to merge 3 commits intotinyhumansai:mainfrom
CodeGhost21 wants to merge 3 commits intotinyhumansai:mainfrom
Conversation
- BootCheckGate cloud picker now accepts a bearer token alongside the URL; coreRpcClient sends `Authorization: Bearer ...` against the configured cloud core, completing the desktop-side UX for the tinyhumansai#1280 deploy flow - coreMode slice derives initial state synchronously from localStorage (`openhuman_core_mode` + URL/token keys) so reloads (notably the handleIdentityFlip → restartApp path) survive without flipping back to the picker — fixes the picker → snapshot → flip → reload loop in cloud - primeActiveUserId(null) now falls back to the existing OPENHUMAN_ACTIVE_USER_ID seed instead of wiping it; was the actual loop trigger when the local Rust core can't supply an active user id - peekStoredRpcUrl returns null when nothing is stored, so any picker URL wins even when it equals the build-time VITE_OPENHUMAN_CORE_RPC_URL — prior `storedUrl !== CORE_RPC_URL` guard silently routed cloud RPC back to the local sidecar - Developer Options panel: new CoreModeBadge surfaces the active mode + RPC URL + masked token tail so users can verify the picker landed correctly without DevTools - useDictationHotkey + OverlayApp drop direct invoke('core_rpc_url') and route through getCoreHttpBaseUrl() so cloud overrides flow through the socket connections too (was a 127.0.0.1:7788 fallback in cloud mode) - iMessage scanner: drop "core RPC token is not initialized" from WARN to DEBUG when no local sidecar (expected silence in cloud mode)
Contributor
📝 WalkthroughWalkthroughThis PR implements cloud deployment mode with persistent bearer-token authentication. New storage APIs and Redux state support mode selection (local sidecar or cloud remote). The boot gate captures cloud credentials and clears resolution caches on mode switches. RPC client prioritizes stored tokens and URLs over Tauri defaults. Developer options display current mode status. Dictation and overlay apps delegate URL resolution to the shared client helper. ChangesCloud Core Mode & Authentication
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
app/src-tauri/src/imessage_scanner/mod.rs (1)
155-163: ⚡ Quick winInclude
account_id(and keep error detail) in the downgraded DEBUG path.The new DEBUG branch loses correlation context, which makes scanner diagnostics harder when multiple accounts/scanners are active. Keep the downgrade, but log with
account_idand include the error string in both branches.Suggested patch
- if msg.contains("core RPC token is not initialized") { - log::debug!("[imessage] local core not running — skipping tick"); + if msg.contains("core RPC token is not initialized") { + log::debug!( + "[imessage][{}] local core not running — skipping tick err={}", + account_id, + msg + ); } else { - log::warn!("[imessage] tick failed err={}", msg); + log::warn!("[imessage][{}] tick failed err={}", account_id, msg); }As per coding guidelines, “Default to verbose diagnostics on new/changed flows with … correlation fields.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src-tauri/src/imessage_scanner/mod.rs` around lines 155 - 163, The debug branch that handles the "core RPC token is not initialized" case currently drops correlation context; update both logging calls to include the account_id and keep the error message. Specifically, where msg is created and checked (the block using msg.contains("core RPC token is not initialized")), change the debug call log::debug!("[imessage] local core not running — skipping tick"); to include account_id and the error string (e.g. log::debug!("[imessage][account_id={}] local core not running — skipping tick: {}", account_id, msg)); also ensure the else branch warn keeps account_id (e.g. log::warn!("[imessage][account_id={}] tick failed err={}", account_id, msg)); keep the conditional logic intact but add account_id to both log statements.app/src/store/coreModeSlice.ts (1)
34-37: ⚡ Quick winConsider importing storage keys from
configPersistence.tsto avoid duplication.These three constants duplicate the keys defined in
configPersistence.ts. If a key is renamed in one location but not the other, the slice and persistence utilities will silently diverge.Suggested approach
Export the storage key constants from
configPersistence.tsand import them here:// In configPersistence.ts export const RPC_URL_STORAGE_KEY = 'openhuman_core_rpc_url'; export const CORE_TOKEN_STORAGE_KEY = 'openhuman_core_rpc_token'; export const CORE_MODE_STORAGE_KEY = 'openhuman_core_mode';// In coreModeSlice.ts import { RPC_URL_STORAGE_KEY, CORE_TOKEN_STORAGE_KEY, CORE_MODE_STORAGE_KEY } from '../utils/configPersistence';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/store/coreModeSlice.ts` around lines 34 - 37, The three duplicated constants RPC_URL_STORAGE_KEY, CORE_TOKEN_STORAGE_KEY, and CORE_MODE_STORAGE_KEY should be exported from configPersistence.ts and imported into coreModeSlice.ts instead of being re-declared; update configPersistence.ts to export those constants, remove the local declarations in coreModeSlice.ts, and add an import of RPC_URL_STORAGE_KEY, CORE_TOKEN_STORAGE_KEY, and CORE_MODE_STORAGE_KEY into coreModeSlice.ts so the slice and persistence utilities reference the single source-of-truth.app/src/services/__tests__/coreRpcClient.test.ts (1)
672-672: 💤 Low valueMinor:
expect.anything()is unnecessary for single-argument invoke call.The
invoke('core_rpc_token')call has no second argument, soexpect.anything()will never match. The assertion still passes becauseinvokewasn't called with that command at all, but it's misleading.Suggested fix
- expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('core_rpc_token', expect.anything()); + expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('core_rpc_token');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/services/__tests__/coreRpcClient.test.ts` at line 672, The test assertion is using expect.anything() for a non-existent second argument on the invoke call; update the assertion in coreRpcClient.test to assert that vi.mocked(invoke) was not called with the single argument 'core_rpc_token' (remove the expect.anything() and use not.toHaveBeenCalledWith('core_rpc_token')) so the expectation matches the actual single-argument invoke signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/src-tauri/src/imessage_scanner/mod.rs`:
- Around line 155-163: The debug branch that handles the "core RPC token is not
initialized" case currently drops correlation context; update both logging calls
to include the account_id and keep the error message. Specifically, where msg is
created and checked (the block using msg.contains("core RPC token is not
initialized")), change the debug call log::debug!("[imessage] local core not
running — skipping tick"); to include account_id and the error string (e.g.
log::debug!("[imessage][account_id={}] local core not running — skipping tick:
{}", account_id, msg)); also ensure the else branch warn keeps account_id (e.g.
log::warn!("[imessage][account_id={}] tick failed err={}", account_id, msg));
keep the conditional logic intact but add account_id to both log statements.
In `@app/src/services/__tests__/coreRpcClient.test.ts`:
- Line 672: The test assertion is using expect.anything() for a non-existent
second argument on the invoke call; update the assertion in coreRpcClient.test
to assert that vi.mocked(invoke) was not called with the single argument
'core_rpc_token' (remove the expect.anything() and use
not.toHaveBeenCalledWith('core_rpc_token')) so the expectation matches the
actual single-argument invoke signature.
In `@app/src/store/coreModeSlice.ts`:
- Around line 34-37: The three duplicated constants RPC_URL_STORAGE_KEY,
CORE_TOKEN_STORAGE_KEY, and CORE_MODE_STORAGE_KEY should be exported from
configPersistence.ts and imported into coreModeSlice.ts instead of being
re-declared; update configPersistence.ts to export those constants, remove the
local declarations in coreModeSlice.ts, and add an import of
RPC_URL_STORAGE_KEY, CORE_TOKEN_STORAGE_KEY, and CORE_MODE_STORAGE_KEY into
coreModeSlice.ts so the slice and persistence utilities reference the single
source-of-truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4c268609-66ee-408f-9b52-fe2c68fdc55d
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
app/src-tauri/src/imessage_scanner/mod.rsapp/src/components/BootCheckGate/BootCheckGate.tsxapp/src/components/BootCheckGate/__tests__/BootCheckGate.test.tsxapp/src/components/settings/panels/DeveloperOptionsPanel.tsxapp/src/components/settings/panels/__tests__/DeveloperOptionsPanel.test.tsxapp/src/hooks/useDictationHotkey.tsapp/src/overlay/OverlayApp.tsxapp/src/pages/__tests__/Welcome.test.tsxapp/src/services/__tests__/coreRpcClient.test.tsapp/src/services/coreRpcClient.tsapp/src/store/coreModeSlice.test.tsapp/src/store/coreModeSlice.tsapp/src/store/userScopedStorage.tsapp/src/test/test-utils.tsxapp/src/utils/__tests__/configPersistence.test.tsapp/src/utils/configPersistence.ts
Adds focused tests for the previously uncovered surfaces flagged by diff-cover on PR 1357: - userScopedStorage: full primeActiveUserId branch matrix including the null-prime-preserves-prior-seed fix that breaks the cloud-mode identity-flip loop, plus setActiveUserId localStorage round-trip and failure tolerance - coreModeSlice: deriveInitialMode local / cloud / partial / unset branches via fresh module imports with localStorage seeded ahead of module load - coreRpcClient: clearCoreRpcTokenCache forces a re-resolve so the next call picks up a rotated cloud token - BootCheckGate: cloud picker now exercises malformed URL, missing token, token-error reset on type, and the happy path that fires runBootCheck with kind/url/token
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/store/coreModeSlice.test.ts (1)
89-97: ⚡ Quick winTest intent and exercised branch are slightly mismatched.
This case validates “missing token,” but the title claims “URL or token.” Please either rename the test or add a dedicated missing-URL case.
Suggested minimal update
- it('falls back to unset when cloud marker exists but URL or token is missing', async () => { + it('falls back to unset when cloud marker exists but token is missing', async () => { localStorage.clear(); localStorage.setItem('openhuman_core_mode', 'cloud'); localStorage.setItem('openhuman_core_rpc_url', 'https://core.example.com/rpc'); // Token deliberately missing. const mod = await freshImport(); const state = mod.default(undefined, { type: '@@INIT' }); expect(state.mode).toEqual({ kind: 'unset' }); }); + + it('falls back to unset when cloud marker exists but URL is missing', async () => { + localStorage.clear(); + localStorage.setItem('openhuman_core_mode', 'cloud'); + localStorage.setItem('openhuman_core_rpc_token', 'tok-abc'); + const mod = await freshImport(); + const state = mod.default(undefined, { type: '@@INIT' }); + expect(state.mode).toEqual({ kind: 'unset' }); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/store/coreModeSlice.test.ts` around lines 89 - 97, The test "falls back to unset when cloud marker exists but URL or token is missing" exercises the missing-token branch but its title mentions "URL or token"; fix by either renaming the test to mention only the missing token (update the it(...) description) or add a complementary test that sets the cloud marker and token but omits openhuman_core_rpc_url to exercise the missing-URL branch; in both cases keep using freshImport(), the same localStorage keys ('openhuman_core_mode', 'openhuman_core_rpc_url', token key), and the same assertion that the reducer (default export) returns state.mode === { kind: 'unset' } so the test title and exercised branch match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/services/__tests__/coreRpcClient.test.ts`:
- Line 672: The test's assertion using
expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('core_rpc_token',
expect.anything()) misses single-argument calls; update the test to inspect the
mocked invoke calls (vi.mocked(invoke).mock.calls) and assert that none of the
call entries have 'core_rpc_token' as the first element (e.g., filter or find on
the mock.calls array for call[0] === 'core_rpc_token' and expect that result to
be undefined/empty) so both single-argument and multi-argument invocations of
invoke('core_rpc_token', ...) are caught.
- Around line 644-648: The test suite leaves invoke's mockImplementation set
between tests, causing hidden state coupling; add a reset of the mocked invoke
in the beforeEach (e.g. call vi.mocked(invoke).mockReset() or mockClear()) so
invoke's implementation is cleared each test, and update the failing assertion
in the clearCoreRpcTokenCache test to assert
expect(invoke).not.toHaveBeenCalledWith('core_rpc_token') (remove the extra
expect.anything() second-arg check) so it actually verifies no one-argument call
to invoke('core_rpc_token') occurred.
---
Nitpick comments:
In `@app/src/store/coreModeSlice.test.ts`:
- Around line 89-97: The test "falls back to unset when cloud marker exists but
URL or token is missing" exercises the missing-token branch but its title
mentions "URL or token"; fix by either renaming the test to mention only the
missing token (update the it(...) description) or add a complementary test that
sets the cloud marker and token but omits openhuman_core_rpc_url to exercise the
missing-URL branch; in both cases keep using freshImport(), the same
localStorage keys ('openhuman_core_mode', 'openhuman_core_rpc_url', token key),
and the same assertion that the reducer (default export) returns state.mode ===
{ kind: 'unset' } so the test title and exercised branch match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dbfe2fce-6873-4e8c-90b9-cea3b0ecdf17
📒 Files selected for processing (4)
app/src/components/BootCheckGate/__tests__/BootCheckGate.test.tsxapp/src/services/__tests__/coreRpcClient.test.tsapp/src/store/__tests__/userScopedStorage.test.tsapp/src/store/coreModeSlice.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/components/BootCheckGate/tests/BootCheckGate.test.tsx
Comment on lines
+644
to
+648
| beforeEach(() => { | ||
| vi.resetModules(); | ||
| vi.clearAllMocks(); | ||
| vi.stubGlobal('fetch', vi.fn()); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n app/src/services/__tests__/coreRpcClient.test.ts | sed -n '630,685p'Repository: tinyhumansai/openhuman
Length of output: 2714
🏁 Script executed:
cat -n app/src/services/__tests__/coreRpcClient.test.ts | sed -n '1,80p'Repository: tinyhumansai/openhuman
Length of output: 3168
🏁 Script executed:
cat -n app/src/services/__tests__/coreRpcClient.test.ts | sed -n '678,730p'Repository: tinyhumansai/openhuman
Length of output: 2799
🏁 Script executed:
cat -n app/src/services/__tests__/coreRpcClient.test.ts | sed -n '600,645p'Repository: tinyhumansai/openhuman
Length of output: 2327
🏁 Script executed:
# Check if vi.resetModules() behavior is properly understood in context of mock lifecycle
# Let me search for other beforeEach patterns in the file to understand the nesting
grep -n "beforeEach\|describe(" app/src/services/__tests__/coreRpcClient.test.ts | head -30Repository: tinyhumansai/openhuman
Length of output: 395
🏁 Script executed:
cat -n app/src/services/__tests__/coreRpcClient.test.ts | sed -n '498,540p'Repository: tinyhumansai/openhuman
Length of output: 1899
🏁 Script executed:
# Let me check how invoke is actually called in the coreRpcClient source
cat -n app/src/services/coreRpcClient.ts | grep -A 3 -B 3 "invoke"Repository: tinyhumansai/openhuman
Length of output: 2027
🏁 Script executed:
# Check the exact assertion at line 672 - what does invoke get called with?
# And verify the pattern in getCoreRpcUrl matches what should be in getCoreRpcToken
cat -n app/src/services/__tests__/coreRpcClient.test.ts | sed -n '650,677p'Repository: tinyhumansai/openhuman
Length of output: 1523
🏁 Script executed:
# Let me verify Vitest mock behavior - does vi.clearAllMocks clear implementations?
# Check if there are any other patterns in the test that show mockReset vs clearAllMocks
grep -n "mockReset\|clearAllMocks\|mockImplementation" app/src/services/__tests__/coreRpcClient.test.tsRepository: tinyhumansai/openhuman
Length of output: 836
🏁 Script executed:
# Look at what invoke is actually being called with in the test - does it pass a second argument?
grep -A 5 -B 5 "await invoke" app/src/services/coreRpcClient.tsRepository: tinyhumansai/openhuman
Length of output: 812
🏁 Script executed:
# Let me check Vitest documentation behavior by creating a test to understand the matcher
# First, let's verify what arguments invoke is actually called with
rg "invoke\(" app/src/services/coreRpcClient.ts -A 1 | head -20Repository: tinyhumansai/openhuman
Length of output: 180
🏁 Script executed:
# Let me look more closely at test 678 to see if it needs invoke mocked
cat -n app/src/services/__tests__/coreRpcClient.test.ts | sed -n '678,705p'Repository: tinyhumansai/openhuman
Length of output: 1549
🏁 Script executed:
# Verify the fix is necessary by checking if test 678 passes without invoke mock implementation
# Let's also see test 706 which should also be affected
cat -n app/src/services/__tests__/coreRpcClient.test.ts | sed -n '706,730p'Repository: tinyhumansai/openhuman
Length of output: 1316
Reset invoke mock implementation to prevent test-order coupling.
vi.clearAllMocks() clears call history but leaves the mockImplementation from the previous test. At line 678, test clearCoreRpcTokenCache does not set an invoke mock and will inherit the one from line 656, where it throws on 'core_rpc_token'. This creates hidden state coupling. Compare this to the getCoreRpcUrl block (line 500–504), which correctly calls vi.mocked(invoke).mockReset() to ensure isolation.
Additionally, the assertion at line 672 uses not.toHaveBeenCalledWith('core_rpc_token', expect.anything()), checking that the two-argument call never occurred. Since invoke is actually called with only one argument (await invoke('core_rpc_token')), the assertion does not directly verify that invoke('core_rpc_token') was never called—it only confirms the two-argument variant wasn't used.
As per coding guidelines, keep tests deterministic and avoid hidden global state.
Suggested fix
beforeEach(() => {
vi.resetModules();
vi.clearAllMocks();
+ vi.mocked(invoke).mockReset();
+ vi.mocked(isTauri).mockReturnValue(false);
vi.stubGlobal('fetch', vi.fn());
});And strengthen the assertion at line 672:
- expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('core_rpc_token', expect.anything());
+ expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('core_rpc_token');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| beforeEach(() => { | |
| vi.resetModules(); | |
| vi.clearAllMocks(); | |
| vi.stubGlobal('fetch', vi.fn()); | |
| }); | |
| beforeEach(() => { | |
| vi.resetModules(); | |
| vi.clearAllMocks(); | |
| vi.mocked(invoke).mockReset(); | |
| vi.mocked(isTauri).mockReturnValue(false); | |
| vi.stubGlobal('fetch', vi.fn()); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/services/__tests__/coreRpcClient.test.ts` around lines 644 - 648, The
test suite leaves invoke's mockImplementation set between tests, causing hidden
state coupling; add a reset of the mocked invoke in the beforeEach (e.g. call
vi.mocked(invoke).mockReset() or mockClear()) so invoke's implementation is
cleared each test, and update the failing assertion in the
clearCoreRpcTokenCache test to assert
expect(invoke).not.toHaveBeenCalledWith('core_rpc_token') (remove the extra
expect.anything() second-arg check) so it actually verifies no one-argument call
to invoke('core_rpc_token') occurred.
| const { callCoreRpc: freshCallCoreRpc } = await import('../coreRpcClient'); | ||
| await freshCallCoreRpc({ method: 'openhuman.ping' }); | ||
|
|
||
| expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('core_rpc_token', expect.anything()); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find app/src/services -name "*coreRpcClient*" -type fRepository: tinyhumansai/openhuman
Length of output: 149
🏁 Script executed:
wc -l app/src/services/__tests__/coreRpcClient.test.tsRepository: tinyhumansai/openhuman
Length of output: 119
🏁 Script executed:
cat -n app/src/services/__tests__/coreRpcClient.test.tsRepository: tinyhumansai/openhuman
Length of output: 34014
Strengthen the core_rpc_token assertion to catch single-argument calls.
The current matcher requires two arguments: invoke('core_rpc_token', expect.anything()). If code were changed to call invoke('core_rpc_token') with only the command name, this assertion would incorrectly pass. Filter mock calls by command name instead:
Suggested fix
- expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('core_rpc_token', expect.anything());
+ const tokenCalls = vi.mocked(invoke).mock.calls.filter(([cmd]) => cmd === 'core_rpc_token');
+ expect(tokenCalls).toHaveLength(0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('core_rpc_token', expect.anything()); | |
| const tokenCalls = vi.mocked(invoke).mock.calls.filter(([cmd]) => cmd === 'core_rpc_token'); | |
| expect(tokenCalls).toHaveLength(0); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/services/__tests__/coreRpcClient.test.ts` at line 672, The test's
assertion using
expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('core_rpc_token',
expect.anything()) misses single-argument calls; update the test to inspect the
mocked invoke calls (vi.mocked(invoke).mock.calls) and assert that none of the
call entries have 'core_rpc_token' as the first element (e.g., filter or find on
the mock.calls array for call[0] === 'core_rpc_token' and expect that result to
be undefined/empty) so both single-argument and multi-argument invocations of
invoke('core_rpc_token', ...) are caught.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OPENHUMAN_CORE_TOKENalongside the URL;coreRpcClientsendsAuthorization: Bearer …against the configured cloud core, completing the desktop-side UX for the OpenHuman is not available as a one-click cloud deployment #1280 one-click cloud deploy.coreModeslice derives its initial state synchronously fromlocalStorage(a newopenhuman_core_modekey + the existing URL/token keys) so reloads — notablyhandleIdentityFlip→restartApp— survive without flipping back to the picker. Fixes the cloud-mode picker → snapshot → flip → reload loop.primeActiveUserId(null)now falls back to the existingOPENHUMAN_ACTIVE_USER_IDseed instead of wiping it. The wipe was the actual loop trigger when the local Rust core can't supply an active user id (every cloud-mode boot, every transientgetActiveUserIdFromCorefailure).peekStoredRpcUrlreturns null when nothing is stored, so any picker URL wins even when it equals the build-timeVITE_OPENHUMAN_CORE_RPC_URL. The priorstoredUrl !== CORE_RPC_URLguard silently routed cloud RPC back to the local sidecar (ERR_CONNECTION_REFUSED 127.0.0.1:7788) whenever the values matched.CoreModeBadge(Local pill / Cloud pill with masked token tail / unset warning) so users can verify the picker landed correctly without DevTools.useDictationHotkey,OverlayApp, the iMessage scanner — now route throughgetCoreHttpBaseUrl()or short-circuit cleanly in cloud mode (no more WARN-spam when the local sidecar isn't running).Problem
Issue #1280 shipped the headless cloud-deploy artifacts (
.do/app.yaml, Compose, smoke test, docs) in #1304, but the desktop-side UX to actually talk to the deployed core was incomplete:/rpccall.CoreStateProvider's identity-flip path (setActiveUserId(X) → restartApp) causedwindow.location.reload()in dev. Several layers raced against the reload — redux-persist's debounced flush, theprimeActiveUserId(null)overwrite, thestoredUrl !== CORE_RPC_URLmask — leaving the user trapped in an infinite picker → flip → reload loop.invoke('core_rpc_url')directly) emittedERR_CONNECTION_REFUSEDandcore RPC token is not initializedWARNs that drowned the console even when the rest of the app was running fine.Solution
Three layered fixes around the same root concern: respect the user's explicit picker choices, even across reloads.
Picker captures auth. Cloud mode requires both URL and token; both stored synchronously to
localStoragebefore any async dispatch.coreRpcClient.getCoreRpcToken()now resolves stored cloud token first, falling back to the local sidecar invoke.testCoreRpcConnectionaccepts atokenOverrideso the picker can validate a freshly-typed token before persistence.Slice initial state derives from sync localStorage. The new
openhuman_core_modemarker plus the URL/token keys give the slice a synchronous source of truth that always wins on reload, making the gate immune to redux-persist's debounce window.handleSwitchModeclears all three together so legacy "switch back to picker" flows still work.primeActiveUserId(null)is a no-op when localStorage already has a seed. The previousremoveItem(ACTIVE_USER_KEY)wiped the seedsetActiveUserIdhad just written, restarting the identity-flip loop on the next snapshot. Falling back to whatever's stored breaks the cycle without touching any happy-path identity logic.A
CoreModeBadgesurfaces the resolved mode in Developer Options for quick verification (esp. helpful for users debugging "did my picker actually take effect?"). The Tauri-host iMessage scanner downgrades its no-token-yet WARN to DEBUG since a missing local token is the expected state in cloud mode rather than a bug to flag.Submission Checklist
docs/TESTING-STRATEGY.md— 172 tests pass; new: token-priority, picker reload survival, CoreModeBadge state matrix, peekStoredRpcUrl, configPersistence mode/token round-trip.coreModeSlice,configPersistence,coreRpcClient,BootCheckGate,userScopedStorage,DeveloperOptionsPanelis exercised by the new/updated specs above.## Related.fetch/invoke).## Related.Impact
localStorage.getItemat slice init — negligible.localStorage(per-device, pre-login) parallels how the URL was already stored. Documented in the picker copy. No new build-time secret leakage; theVITE_OPENHUMAN_CORE_RPC_TOKENbuild-time approach (closed PR feat(deploy): VITE_OPENHUMAN_CORE_RPC_TOKEN build-time override #1337) was deliberately not revived — public tinyhumans.ai builds ship without any baked secret, and users paste their own token into the picker.coreMode = localblob continue to work; they pick up the syncopenhuman_core_modemirror only when they next switch modes. No persisted-state migration code needed.Related
VITE_OPENHUMAN_CORE_RPC_TOKENoverride) with a runtime picker approach that doesn't require rebuilding the desktop app per deployment.Summary by CodeRabbit
New Features
Bug Fixes
Tests