Repository navigation
refactor(renderer): split web preload API - #16181
Conversation
|
Warning Review limit reachedNext included review available in 37 seconds. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (61)
📝 WalkthroughWalkthroughAdded browser-specific preload API factories across runtime, storage, settings, filesystem, Git, hosted review, terminal, UI, agent, diagnostics, and capability namespaces. Runtime calls now support environment selection, queueing, metadata persistence, ownership tagging, and worktree resolution. Browser storage supports settings, sessions, onboarding, caches, and keybindings. Tests cover API composition, runtime error and timeout behavior, client reuse, and worktree cache invalidation. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (5)
src/renderer/src/web/preload-api/web-git-api.ts (1)
217-238: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse intent-named localization IDs for new messages.
These new user-facing messages use hash-based
auto.web.web.preload.api.*IDs. Replace them with intent-named localization IDs. Use named placeholders when a message needs interpolation.
src/renderer/src/web/preload-api/web-git-api.ts#L217-L238: replace the three new hash-based IDs.src/renderer/src/web/preload-api/web-github-api.ts#L117-L121: replace the new hash-based ID.Based on learnings, new user-facing strings must use intent-named localization IDs; hash-based IDs are grandfathered only.
Source: Learnings
src/renderer/src/web/preload-api/web-shell-api.ts (1)
7-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReturn
Promise<void>from the web shell methods.
window.openreturnsWindow | null, butShellApideclaresPromise<void>. Theas nevercasts hide this mismatch and return the wrong runtime value. Callwindow.openlocally, then resolve withundefined; handle a blocked popup locally if required.src/renderer/src/web/preload-api/web-browser-api.ts (1)
89-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnchecked casts hide preload stub drift in three web API factories. Each site erases the declared
PreloadApishape with a cast, so the compiler cannot verify the stub against the contract. A later change to any of these interfaces will compile cleanly and fail at runtime in the web client instead of at typecheck time.
src/renderer/src/web/preload-api/web-browser-api.ts#L89-L101: remove the twoas unknown ascasts and letcreateBrowserApiandcreateEmulatorApitypecheck against their declared return types; if one member forces the cast, narrow the cast to that member.src/renderer/src/web/preload-api/web-telemetry-api.ts#L7-L9: removeas neverand type the consent-state literal against the declared return type oftelemetryGetConsentState.src/renderer/src/web/preload-api/web-updater-api.ts#L12-L12: removeas neverand return a value that satisfies the declared updater status type.src/renderer/src/web/preload-api/web-ui-api.ts (1)
32-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated host-state merge block.
Lines 32-43 and lines 85-96 build the same
nextobject fromlocalandresult.ui. The two copies must stay in sync. If a new merged field is added to one copy only,getandrecordFeatureInteractionreturn different shapes for the same host response.♻️ Proposed extraction
function reconcileHostUiState(local: PersistedUIState, hostUi: PairedUiState): PersistedUIState { return { ...mergeHostWebUIState(local, hostUi), osc52ClipboardDefaultOnNoticePending: mergeOsc52ClipboardNoticePending(local, hostUi), featureInteractions: mergeFeatureInteractionState( local.featureInteractions, hostUi.featureInteractions ), contextualToursSeenIds: mergeContextualTourSeenIds( local.contextualToursSeenIds, hostUi.contextualToursSeenIds ) } }Then call it in both
getandrecordFeatureInteraction, followed by the sharedwriteJsonandzoomLevelassignment.src/renderer/src/web/preload-api/web-runtime-calls.ts (1)
29-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared envelope body from
callRuntimeEnvelopeandcallEnvironmentEnvelope.Both functions contain the same disconnect pre-check, queue enqueue, disconnect post-check, and
updateEnvironmentFromResponsesequence. Only the environment resolution differs. A future change to the disconnect semantics must be applied twice.♻️ Proposed refactor
+async function callEnvelopeForEnvironment<TResult>( + environment: StoredWebRuntimeEnvironment, + method: string, + params?: unknown, + timeoutMs?: number +): Promise<RuntimeRpcResponse<TResult>> { + if (manuallyDisconnectedEnvironmentIds.has(environment.id)) { + return manuallyDisconnectedResponse(environment) + } + const response = await runtimeCallQueuePool.enqueue(environment.id, method, () => { + if (manuallyDisconnectedEnvironmentIds.has(environment.id)) { + return Promise.resolve(manuallyDisconnectedResponse(environment)) + } + return getClientForEnvironment(environment).call(method, params, { timeoutMs }) + }) + if (manuallyDisconnectedEnvironmentIds.has(environment.id)) { + return manuallyDisconnectedResponse(environment) + } + updateEnvironmentFromResponse(environment, response) + return response as RuntimeRpcResponse<TResult> +} + export async function callRuntimeEnvelope<TResult = unknown>( method: string, params?: unknown, timeoutMs?: number ): Promise<RuntimeRpcResponse<TResult>> { - const environment = requireActiveEnvironment() - ... + return callEnvelopeForEnvironment<TResult>(requireActiveEnvironment(), method, params, timeoutMs) } export async function callEnvironmentEnvelope<TResult = unknown>( selector: string, method: string, params?: unknown, timeoutMs?: number ): Promise<RuntimeRpcResponse<TResult>> { - const environment = resolveEnvironment(selector) - ... + return callEnvelopeForEnvironment<TResult>(resolveEnvironment(selector), method, params, timeoutMs) }
StoredWebRuntimeEnvironmentneeds a type import from../web-runtime-environment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f84706b6-d6ad-4108-95bc-9cbaeee73394
📒 Files selected for processing (61)
config/max-lines-baseline.txtsrc/renderer/src/web/preload-api/web-agent-accounts-api.tssrc/renderer/src/web/preload-api/web-agent-hooks-api.tssrc/renderer/src/web/preload-api/web-agent-status-api.tssrc/renderer/src/web/preload-api/web-ai-vault-api.tssrc/renderer/src/web/preload-api/web-app-api.tssrc/renderer/src/web/preload-api/web-browser-api.tssrc/renderer/src/web/preload-api/web-cli-api.tssrc/renderer/src/web/preload-api/web-clipboard-api.tssrc/renderer/src/web/preload-api/web-diagnostics-api.tssrc/renderer/src/web/preload-api/web-e2e-api.tssrc/renderer/src/web/preload-api/web-e2e-config.tssrc/renderer/src/web/preload-api/web-fallback-api.tssrc/renderer/src/web/preload-api/web-filesystem-api.tssrc/renderer/src/web/preload-api/web-git-api.tssrc/renderer/src/web/preload-api/web-github-api.tssrc/renderer/src/web/preload-api/web-github-cache-api.tssrc/renderer/src/web/preload-api/web-github-routes.tssrc/renderer/src/web/preload-api/web-gitlab-api.tssrc/renderer/src/web/preload-api/web-gitlab-routes.tssrc/renderer/src/web/preload-api/web-host-capability-api.tssrc/renderer/src/web/preload-api/web-keybinding-normalization.tssrc/renderer/src/web/preload-api/web-keybindings-api.tssrc/renderer/src/web/preload-api/web-macos-tcc-api.tssrc/renderer/src/web/preload-api/web-memory-api.tssrc/renderer/src/web/preload-api/web-mobile-api.tssrc/renderer/src/web/preload-api/web-native-chat-api.tssrc/renderer/src/web/preload-api/web-notifications-api.tssrc/renderer/src/web/preload-api/web-onboarding-api.tssrc/renderer/src/web/preload-api/web-orca-profiles-api.tssrc/renderer/src/web/preload-api/web-platform-api.tssrc/renderer/src/web/preload-api/web-preference-normalization.tssrc/renderer/src/web/preload-api/web-preferences-store.tssrc/renderer/src/web/preload-api/web-rate-limits-api.tssrc/renderer/src/web/preload-api/web-repositories-api.tssrc/renderer/src/web/preload-api/web-review-api.tssrc/renderer/src/web/preload-api/web-runtime-api.tssrc/renderer/src/web/preload-api/web-runtime-calls.tssrc/renderer/src/web/preload-api/web-runtime-environments-api.tssrc/renderer/src/web/preload-api/web-runtime-session.tssrc/renderer/src/web/preload-api/web-runtime-worktree-catalog.tssrc/renderer/src/web/preload-api/web-settings-api.tssrc/renderer/src/web/preload-api/web-shell-api.tssrc/renderer/src/web/preload-api/web-star-nag-api.tssrc/renderer/src/web/preload-api/web-storage.tssrc/renderer/src/web/preload-api/web-telemetry-api.tssrc/renderer/src/web/preload-api/web-terminal-api.tssrc/renderer/src/web/preload-api/web-ui-api.tssrc/renderer/src/web/preload-api/web-updater-api.tssrc/renderer/src/web/preload-api/web-workspace-ports-api.tssrc/renderer/src/web/preload-api/web-workspace-session-api.tssrc/renderer/src/web/preload-api/web-worktrees-api.tssrc/renderer/src/web/web-preload-api-clipboard.test.tssrc/renderer/src/web/web-preload-api-composition.test.tssrc/renderer/src/web/web-preload-api-github.test.tssrc/renderer/src/web/web-preload-api-gitlab.test.tssrc/renderer/src/web/web-preload-api-runtime-calls.test.tssrc/renderer/src/web/web-preload-api-runtime-environment.test.tssrc/renderer/src/web/web-preload-api-workspace-catalog.test.tssrc/renderer/src/web/web-preload-api.tssrc/renderer/src/web/web-runtime-client-timeout-budget.test.ts
💤 Files with no reviewable changes (1)
- config/max-lines-baseline.txt
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.
| Partial<PreloadApi>['minimaxCredentials'] | ||
| > { | ||
| const notConfigured = { configured: false } | ||
| const unsupportedError = new Error('MiniMax cookie storage is only available in the desktop app.') |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Localize new web fallback messages.
These messages can reach users but bypass translate. Add intent-named localization IDs and use translate at each site.
src/renderer/src/web/preload-api/web-agent-accounts-api.ts#L7-L7: Localize the MiniMax unsupported error.src/renderer/src/web/preload-api/web-agent-hooks-api.ts#L25-L25: Localize the hook-status detail.src/renderer/src/web/preload-api/web-cli-api.ts#L17-L17: Localize the CLI-status detail.src/renderer/src/web/preload-api/web-diagnostics-api.ts#L34-L38: Localize unavailable diagnostics errors.src/renderer/src/web/preload-api/web-host-capability-api.ts#L130-L130: Localize the computer-use next step.src/renderer/src/web/preload-api/web-host-capability-api.ts#L162-L189: Localize skill-operation errors.
Based on learnings: New user-facing TypeScript strings must use intent-named localization IDs with named interpolation placeholders.
📍 Affects 5 files
src/renderer/src/web/preload-api/web-agent-accounts-api.ts#L7-L7(this comment)src/renderer/src/web/preload-api/web-agent-hooks-api.ts#L25-L25src/renderer/src/web/preload-api/web-cli-api.ts#L17-L17src/renderer/src/web/preload-api/web-diagnostics-api.ts#L34-L38src/renderer/src/web/preload-api/web-host-capability-api.ts#L130-L130src/renderer/src/web/preload-api/web-host-capability-api.ts#L162-L189
Source: Learnings
| return callRuntimeResult<AiVaultListResult>('aiVault.listSessions', { | ||
| limit: args?.limit, | ||
| force: args?.force, | ||
| scopePaths: args?.scopePaths, | ||
| executionHostId | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Forward all list request fields.
AiVaultListArgs includes unlimited and requestToken, but this RPC payload omits both. An unlimited request becomes a capped request, and the runtime loses request correlation. Forward both fields.
Proposed fix
limit: args?.limit,
+ unlimited: args?.unlimited,
force: args?.force,
scopePaths: args?.scopePaths,
+ requestToken: args?.requestToken,
executionHostId📝 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.
| return callRuntimeResult<AiVaultListResult>('aiVault.listSessions', { | |
| limit: args?.limit, | |
| force: args?.force, | |
| scopePaths: args?.scopePaths, | |
| executionHostId | |
| }) | |
| return callRuntimeResult<AiVaultListResult>('aiVault.listSessions', { | |
| limit: args?.limit, | |
| unlimited: args?.unlimited, | |
| force: args?.force, | |
| scopePaths: args?.scopePaths, | |
| requestToken: args?.requestToken, | |
| executionHostId | |
| }) |
| error: translate('auto.web.web.preload.api.fb290366b2', 'Unavailable on web.') | ||
| }), | ||
| copyLatestDiagnostics: () => | ||
| Promise.resolve({ | ||
| ok: false, | ||
| error: translate('auto.web.web.preload.api.fb290366b2', 'Unavailable on web.') |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use an intent-named localization ID.
The new auto.web.web.preload.api.fb290366b2 ID is hash-style. Replace it with an intent-named ID at both call sites.
Based on learnings: New user-facing TypeScript strings must use intent-named localization IDs.
Source: Learnings
| onChanged: (callback) => { | ||
| webKeybindingListeners.add(callback) | ||
| const onStorage = (event: StorageEvent): void => { | ||
| if (event.key === KEYBINDINGS_STORAGE_KEY) { | ||
| callback(getWebKeybindingSnapshot()) | ||
| } | ||
| } | ||
| window.addEventListener('storage', onStorage) | ||
| return () => { | ||
| webKeybindingListeners.delete(callback) | ||
| window.removeEventListener('storage', onStorage) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep each subscription separate.
If a caller registers the same callback twice, the Set stores it once. Either disposer then removes both registrations. Local updates also notify the callback only once.
Store a wrapper listener for each onChanged call and remove that wrapper in its matching disposer.
Proposed fix
onChanged: (callback) => {
- webKeybindingListeners.add(callback)
+ const listener = (snapshot: KeybindingFileSnapshot): void => callback(snapshot)
+ webKeybindingListeners.add(listener)
const onStorage = (event: StorageEvent): void => {
if (event.key === KEYBINDINGS_STORAGE_KEY) {
callback(getWebKeybindingSnapshot())
}
}
window.addEventListener('storage', onStorage)
return () => {
- webKeybindingListeners.delete(callback)
+ webKeybindingListeners.delete(listener)
window.removeEventListener('storage', onStorage)
}
}📝 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.
| onChanged: (callback) => { | |
| webKeybindingListeners.add(callback) | |
| const onStorage = (event: StorageEvent): void => { | |
| if (event.key === KEYBINDINGS_STORAGE_KEY) { | |
| callback(getWebKeybindingSnapshot()) | |
| } | |
| } | |
| window.addEventListener('storage', onStorage) | |
| return () => { | |
| webKeybindingListeners.delete(callback) | |
| window.removeEventListener('storage', onStorage) | |
| onChanged: (callback) => { | |
| const listener = (snapshot: KeybindingFileSnapshot): void => callback(snapshot) | |
| webKeybindingListeners.add(listener) | |
| const onStorage = (event: StorageEvent): void => { | |
| if (event.key === KEYBINDINGS_STORAGE_KEY) { | |
| callback(getWebKeybindingSnapshot()) | |
| } | |
| } | |
| window.addEventListener('storage', onStorage) | |
| return () => { | |
| webKeybindingListeners.delete(listener) | |
| window.removeEventListener('storage', onStorage) |
| const webOrcaProfileAuthStatus = () => | ||
| Promise.resolve({ | ||
| activeProfileId: DEFAULT_LOCAL_ORCA_PROFILE_ID, | ||
| configured: false, | ||
| state: 'unconfigured' as const, | ||
| persistence: 'none' as const, | ||
| setupMessage: 'Orca Cloud sign-in is not available in the browser fallback.' | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Route setupMessage through translate.
setupMessage is a user-facing display field on the auth-status result. Line 14 hardcodes English. Every other user-facing message in this cohort uses translate with an intent-named ID, for example web-runtime-environments-api.ts Line 65 and web-runtime-session.ts Line 84.
🌐 Proposed fix
+import { translate } from '`@/i18n/i18n`'
+
export function createWebOrcaProfilesApi(): Partial<PreloadApi> {
const webOrcaProfileAuthStatus = () =>
Promise.resolve({
activeProfileId: DEFAULT_LOCAL_ORCA_PROFILE_ID,
configured: false,
state: 'unconfigured' as const,
persistence: 'none' as const,
- setupMessage: 'Orca Cloud sign-in is not available in the browser fallback.'
+ setupMessage: translate(
+ 'auto.web.webOrcaProfilesApi.cloudSignInUnavailable',
+ 'Orca Cloud sign-in is not available in the browser fallback.'
+ )
})Based on learnings, for newly added user-facing strings in stablyai/orca TypeScript files, follow config/i18n-translation-source.md and use intent-named localization IDs.
📝 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.
| const webOrcaProfileAuthStatus = () => | |
| Promise.resolve({ | |
| activeProfileId: DEFAULT_LOCAL_ORCA_PROFILE_ID, | |
| configured: false, | |
| state: 'unconfigured' as const, | |
| persistence: 'none' as const, | |
| setupMessage: 'Orca Cloud sign-in is not available in the browser fallback.' | |
| }) | |
| import { translate } from '@/i18n/i18n' | |
| const webOrcaProfileAuthStatus = () => | |
| Promise.resolve({ | |
| activeProfileId: DEFAULT_LOCAL_ORCA_PROFILE_ID, | |
| configured: false, | |
| state: 'unconfigured' as const, | |
| persistence: 'none' as const, | |
| setupMessage: translate( | |
| 'auto.web.webOrcaProfilesApi.cloudSignInUnavailable', | |
| 'Orca Cloud sign-in is not available in the browser fallback.' | |
| ) | |
| }) |
Source: Learnings
| const previousEnvironment = webRuntimeState.activeEnvironment | ||
| closeActiveRuntimeClients() | ||
| webRuntimeState.activeEnvironment = createStoredWebRuntimeEnvironment({ | ||
| name, | ||
| offer, | ||
| previousEnvironment | ||
| }) | ||
| manuallyDisconnectedEnvironmentIds.clear() | ||
| saveStoredWebRuntimeEnvironment(webRuntimeState.activeEnvironment) | ||
| return { environment: redactStoredWebRuntimeEnvironment(webRuntimeState.activeEnvironment) } | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Save the environment before you replace the active in-memory environment.
addFromPairingCode assigns webRuntimeState.activeEnvironment at Line 43 and calls saveStoredWebRuntimeEnvironment at Line 49. If browser storage rejects the write, the method throws after the in-memory swap. The session then points at an environment that is not persisted, and the previous environment stays in storage. A reload silently returns the user to the old host.
verifyAndAddFromPairingCode already guards this case at Lines 133-145 with the stated rule that a storage failure must leave the current host usable. Apply the same ordering here.
🐛 Proposed fix
const previousEnvironment = webRuntimeState.activeEnvironment
- closeActiveRuntimeClients()
- webRuntimeState.activeEnvironment = createStoredWebRuntimeEnvironment({
+ const nextEnvironment = createStoredWebRuntimeEnvironment({
name,
offer,
previousEnvironment
})
+ // Why: a browser storage failure must leave the currently active host usable.
+ saveStoredWebRuntimeEnvironment(nextEnvironment)
+ closeActiveRuntimeClients()
manuallyDisconnectedEnvironmentIds.clear()
- saveStoredWebRuntimeEnvironment(webRuntimeState.activeEnvironment)
- return { environment: redactStoredWebRuntimeEnvironment(webRuntimeState.activeEnvironment) }
+ webRuntimeState.activeEnvironment = nextEnvironment
+ return { environment: redactStoredWebRuntimeEnvironment(nextEnvironment) }📝 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.
| const previousEnvironment = webRuntimeState.activeEnvironment | |
| closeActiveRuntimeClients() | |
| webRuntimeState.activeEnvironment = createStoredWebRuntimeEnvironment({ | |
| name, | |
| offer, | |
| previousEnvironment | |
| }) | |
| manuallyDisconnectedEnvironmentIds.clear() | |
| saveStoredWebRuntimeEnvironment(webRuntimeState.activeEnvironment) | |
| return { environment: redactStoredWebRuntimeEnvironment(webRuntimeState.activeEnvironment) } | |
| }, | |
| const previousEnvironment = webRuntimeState.activeEnvironment | |
| const nextEnvironment = createStoredWebRuntimeEnvironment({ | |
| name, | |
| offer, | |
| previousEnvironment | |
| }) | |
| // Why: a browser storage failure must leave the currently active host usable. | |
| saveStoredWebRuntimeEnvironment(nextEnvironment) | |
| closeActiveRuntimeClients() | |
| manuallyDisconnectedEnvironmentIds.clear() | |
| webRuntimeState.activeEnvironment = nextEnvironment | |
| return { environment: redactStoredWebRuntimeEnvironment(nextEnvironment) } | |
| }, |
| export function toLegacyDetectedWorktreeResult( | ||
| repoId: string, | ||
| worktrees: Worktree[] | ||
| ): DetectedWorktreeListResult { | ||
| return { | ||
| repoId, | ||
| authoritative: true, | ||
| source: 'session-fallback', | ||
| worktrees: worktrees.map((worktree) => ({ | ||
| ...worktree, | ||
| ownership: 'orca-managed', | ||
| selectedCheckout: false, | ||
| visible: true | ||
| })) | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Resolve DetectedWorktreeListResult.authoritative semantics and its consumers.
set -euo pipefail
# Definition of the result contract.
fd -t f 'types.ts' src/shared/worktree --exec rg -n -C 5 'authoritative|DetectedWorktreeListResult|source' {}
# Consumers that branch on `authoritative`.
rg -nP --type=ts -C 6 '\bauthoritative\b' -g '!**/*.test.ts'
# Other producers of session-fallback results, for comparison.
rg -nP --type=ts -C 4 "'session-fallback'"Repository: stablyai/orca
Length of output: 3331
🏁 Script executed:
#!/bin/bash
set -u
echo '--- candidate files ---'
fd -t f . src | rg 'worktree|repo|detected|catalog' | head -200
echo '--- result type and source definitions ---'
rg -n -C 12 --type ts 'type DetectedWorktree(ListResult|ListSource)|interface DetectedWorktree|DetectedWorktreeListResult|DetectedWorktreeListSource' src
echo '--- authoritative references ---'
rg -n -C 10 --type ts --glob '!**/*.test.ts' 'authoritative' src
echo '--- session-fallback references ---'
rg -n -C 10 --type ts "'session-fallback'" srcRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
echo '--- files declaring or using the result type ---'
rg -l --type ts 'DetectedWorktree(ListResult|ListSource)|toLegacyDetectedWorktreeResult|session-fallback' src
echo '--- exact source/type declarations ---'
rg -n -C 15 --type ts 'DetectedWorktreeListSource|DetectedWorktreeListResult' src/shared src/renderer/src/web src/renderer/src/store src/renderer/src/components
echo '--- exact worktree authoritative property uses ---'
rg -n -C 8 --type ts '(result|snapshot|catalog|detectedWorktrees|detectedWorktreeList)\??\.authoritative|authoritative\s*===|authoritative\s*&&|authoritative\s*\?' src/renderer/src src/shared
echo '--- catalog implementation ---'
cat -n src/renderer/src/web/preload-api/web-runtime-worktree-catalog.ts | sed -n '1,230p'Repository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
echo '--- shared worktree result contract ---'
cat -n src/shared/worktree/types.ts | sed -n '175,235p'
echo '--- fallback catalog implementation ---'
cat -n src/renderer/src/web/preload-api/web-runtime-worktree-catalog.ts | sed -n '1,145p'
echo '--- worktree listing authoritative uses ---'
rg -n -C 12 --type ts \
'authoritative|session-fallback|worktree\.list|detectedList|missingWorktreeTeardowns' \
src/renderer/src/store/slices/worktrees/listing \
src/renderer/src/store/slices/worktrees/teardown \
src/renderer/src/store/slices/degraded-repo-worktree-validity.ts \
src/renderer/src/store/slices/worktree-helpers.ts
echo '--- all non-test result property consumers ---'
rg -n -C 8 --type ts --glob '!**/*.test.ts' \
'(detected|result|snapshot)\.authoritative|authoritative:\s*(true|false)' \
src/renderer/src/store/slices src/renderer/src/web src/shared/external-worktree-inbox.tsRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
echo '--- legacy converter usages ---'
rg -n -C 15 --type ts --glob '!**/*.test.ts' \
'toLegacyDetectedWorktreeResult|callRuntimeDetectedWorktrees|fetchDetectedWorktrees' \
src/renderer/src src/preload src/main
echo '--- removed-id reconciliation callers ---'
rg -n -C 18 --type ts --glob '!**/*.test.ts' \
'getRemovedWorktreeIdsAfterAuthoritativeScan|teardownMissingWorktreeTerminalsBestEffort|purgeRemovedWorktrees|removedWorktreeIds' \
src/renderer/src/store/slices/worktrees
echo '--- refresh and fetch flow ---'
cat -n src/renderer/src/store/slices/worktrees/listing/detected-worktree-refresh.ts | sed -n '1,260p'
cat -n src/renderer/src/store/slices/worktrees/listing/fetch-all-worktrees.ts | sed -n '1,230p'Repository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from pathlib import Path
catalog = Path("src/renderer/src/web/preload-api/web-runtime-worktree-catalog.ts").read_text()
ownership = Path("src/renderer/src/store/slices/worktrees/listing/worktree-host-ownership.ts").read_text()
teardown = Path("src/renderer/src/store/slices/worktrees/teardown/missing-worktree-terminal-teardown.ts").read_text()
checks = {
"web fallback is authoritative": "authoritative: true" in catalog,
"web fallback is session fallback": "source: 'session-fallback'" in catalog,
"web fallback forces visible": "visible: true" in catalog,
"web fallback forces Orca ownership": "ownership: 'orca-managed'" in catalog,
"store fallback is authoritative": "authoritative: true" in ownership,
"store fallback is session fallback": "source: 'session-fallback'" in ownership,
"removed-id teardown gates only on authoritative": "if (!detected.authoritative" in teardown,
"removed-id teardown computes absent IDs": "filter((worktreeId) => !detectedIds.has(worktreeId))" in teardown,
}
for name, result in checks.items():
print(f"{name}: {'present' if result else 'MISSING'}")
PYRepository: stablyai/orca
Length of output: 504
🏁 Script executed:
#!/bin/bash
set -u
echo '--- detected refresh result handling ---'
cat -n src/renderer/src/store/slices/worktrees/listing/fetch-detected-worktrees.ts | sed -n '1,260p'
echo '--- provider fallback handling ---'
cat -n src/renderer/src/store/slices/worktrees/listing/detected-worktree-provider-request.ts | sed -n '20,150p'
echo '--- authoritative removal call sites ---'
rg -n -C 10 --type ts --glob '!**/*.test.ts' \
'getRemovedWorktreeIdsAfterAuthoritativeScan|teardownMissingWorktreeTerminalsBestEffort' \
src/renderer/src
echo '--- fallback-specific tests and expectations ---'
rg -n -C 12 --type ts --glob '*test.ts' \
'session-fallback|toLegacyDetectedWorktreeResult|authoritative.*true|method_not_found' \
src/renderer/src src/shared src/preloadRepository: stablyai/orca
Length of output: 50370
Mark session-fallback results as non-authoritative. worktree.list returns only sidebar-visible rows, but authoritative results drive missing-worktree reconciliation. source does not protect consumers. Set authoritative: false in this converter and its duplicate in worktree-host-ownership.ts.
| updatePRBotAuthorOverride: (args) => updateRuntimePRBotAuthorOverride(args), | ||
| listFonts: () => Promise.resolve([]), | ||
| onChanged: () => noopUnsubscribe | ||
| } satisfies Partial<WebSettingsApi> as unknown as WebSettingsApi, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Compare PreloadApi['settings'] members against the web implementation.
set -euo pipefail
# Locate the settings member declaration on PreloadApi.
fd -t f 'api-types.ts' src/preload --exec rg -n -A 60 '^\s*settings\??:' {}
# Members implemented by the web settings API.
ast-grep outline src/renderer/src/web/preload-api/web-settings-api.ts --items all
# Other implementations of the same surface, for a member-by-member comparison.
rg -nP --type=ts -C 3 "satisfies Partial<WebSettingsApi>"Repository: stablyai/orca
Length of output: 2925
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- SettingsApi declaration ---'
rg -n -A 120 -B 10 'interface SettingsApi|type SettingsApi' src/preload
echo '--- WebSettingsApi declaration ---'
rg -n -A 140 -B 10 'interface WebSettingsApi|type WebSettingsApi' src/renderer/src/web/preload-api
echo '--- Web settings implementation ---'
cat -n src/renderer/src/web/preload-api/web-settings-api.ts
echo '--- Settings API references ---'
rg -n --type=ts 'createWebSettingsApi|WebSettingsApi|PreloadApi\[['\"'\"']settings['\"'\"'\]' src/renderer/src src/preloadRepository: stablyai/orca
Length of output: 24911
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Settings preview implementations and call sites ---'
rg -n -C 4 --type=ts --type=tsx 'previewGhosttyImport|previewWarpThemeImport' .
echo '--- Web preload API assembly ---'
rg -n -C 8 'createWebSettingsApi|settings:' src/renderer/src/web src/preload
echo '--- Desktop settings implementation ---'
rg -n -C 5 --type=ts 'previewGhosttyImport|previewWarpThemeImport' src/preload src
echo '--- Relevant tests ---'
rg -n -C 5 --glob '*.{test,spec}.{ts,tsx}' 'createWebSettingsApi|previewGhosttyImport|previewWarpThemeImport|settings\.getSync' .Repository: stablyai/orca
Length of output: 239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Settings preview implementations and call sites ---'
rg -n -C 4 -g '*.ts' -g '*.tsx' 'previewGhosttyImport|previewWarpThemeImport' .
echo '--- Web preload API assembly ---'
rg -n -C 8 -g '*.ts' -g '*.tsx' 'createWebSettingsApi|settings:' src/renderer/src/web src/preload
echo '--- Desktop settings implementation ---'
rg -n -C 5 -g '*.ts' -g '*.tsx' 'previewGhosttyImport|previewWarpThemeImport' src/preload src
echo '--- Relevant tests ---'
rg -n -C 5 -g '*.test.ts' -g '*.test.tsx' -g '*.spec.ts' -g '*.spec.tsx' \
'createWebSettingsApi|previewGhosttyImport|previewWarpThemeImport|settings\.getSync' .Repository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Preview references outside tests ---'
rg -n -g '*.ts' -g '*.tsx' -g '!*.test.ts' -g '!*.test.tsx' \
'previewGhosttyImport|previewWarpThemeImport' src
echo '--- Web API construction ---'
rg -n -C 6 -g '*.ts' -g '*.tsx' \
'createWebSettingsApi|createWeb.*Api|web-preload-api' src/renderer/src/web src/renderer/src
echo '--- Settings API type and desktop surface ---'
cat -n src/preload/api/settings-api.ts | sed -n '1,24p'
cat -n src/renderer/src/web/preload-api/web-settings-api.ts | sed -n '20,108p'Repository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
terms = ("previewGhosttyImport", "previewWarpThemeImport")
for root in (Path("src/renderer/src"), Path("src/preload"), Path("src/main")):
for path in root.rglob("*"):
if path.suffix not in {".ts", ".tsx"} or ".test." in path.name:
continue
text = path.read_text(errors="replace")
hits = [(i + 1, line.strip()) for i, line in enumerate(text.splitlines())
if any(term in line for term in terms)]
if hits:
print(path)
for line_no, line in hits:
print(f" {line_no}: {line}")
PY
echo '--- Web API installation ---'
rg -n -A 35 -B 10 'installWebPreloadApi|createWebSettingsApi' \
src/renderer/src/web/web-preload-api.ts
echo '--- Explicit unsupported-operation patterns ---'
rg -n -C 3 -g '*.ts' -g '*.tsx' \
'only available in the desktop app|not available in.*browser|unsupported.*browser|desktop-only' \
src/renderer/src/web/preload-api src/renderer/src/webRepository: stablyai/orca
Length of output: 18307
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Renderer call sites ---'
cat -n src/renderer/src/components/onboarding/ThemeStep.tsx | sed -n '65,145p'
cat -n src/renderer/src/components/settings/useGhosttyImport.ts | sed -n '1,80p'
cat -n src/renderer/src/components/settings/useWarpThemeImport.ts | sed -n '1,90p'
echo '--- Fallback proxy implementation ---'
rg -n -C 8 'function withFallback|const withFallback|withFallback\s*=' src/renderer/src
rg -n -C 8 'createFallbackProxy' src/renderer/src/web src/renderer/src
echo '--- Web settings tests and related assertions ---'
rg -n -C 5 'settings\.(previewGhosttyImport|previewWarpThemeImport)|previewGhosttyImport|previewWarpThemeImport' \
src/renderer/src/web/web-preload-api-settings.test.ts \
src/renderer/src/components/settings src/renderer/src/components/onboardingRepository: stablyai/orca
Length of output: 49920
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Complete fallback behavior ---'
cat -n src/renderer/src/web/preload-api/web-fallback-api.ts | sed -n '1,140p'
echo '--- Web settings test file structure and fallback assertions ---'
rg -n -C 3 'fallback|settings|Ghostty|Warp|desktop app|available in the browser' \
src/renderer/src/web/web-preload-api-settings.test.ts \
src/renderer/src/web/web-preload-api*.test.ts
echo '--- Runtime behavior expectations for import hooks ---'
cat -n src/renderer/src/components/settings/useWarpThemeImport.test.ts | sed -n '190,225p'
cat -n src/renderer/src/components/settings/useGhosttyImport.test.ts | sed -n '150,175p'Repository: stablyai/orca
Length of output: 50370
Provide type-correct web implementations for the missing preview methods.
withFallback returns a generic preview result, not undefined. previewWarpThemeImport therefore lacks themes, and useWarpThemeImport fails at result.themes.map(...). Add explicit results or desktop-only rejections for both methods, then remove the unsafe cast.
| export function readJson<T>(key: string, fallback: T): T { | ||
| const raw = window.localStorage.getItem(key) | ||
| if (!raw) { | ||
| return cloneJson(fallback) | ||
| } | ||
| try { | ||
| return { ...cloneJson(fallback), ...JSON.parse(raw) } as T | ||
| } catch { | ||
| return cloneJson(fallback) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Backfill nested persisted state.
Line 29 only merges top-level fields. An older stored onboarding checklist replaces the complete default checklist, so newly added checklist keys are absent. Normalize nested persisted state per domain before returning it.
(cherry picked from commit d3c37b8)
Ports preservesTranscriptOrder into main's split web preload API (#16181 moved createNativeChatApi to preload-api/web-native-chat-api.ts) and unions it with main's new capabilities.transcriptPending wire field.
Architecture
src/renderer/src/web/web-preload-api.tsto composition and installation only.web/preload-api/domain modules: runtime session/calls/catalog state, persistence and normalization, provider routes, browser/clipboard/filesystem/git adapters, and desktop-unavailable capability implementations.inline src/renderer/src/web/web-preload-api.tsmax-lines baseline entry.Preserved regression contracts
Root line count
All changed production TypeScript modules are at most 297 lines. All changed tests are at most 721 lines.
Validation
pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/web/web-preload-api-clipboard.test.ts src/renderer/src/web/web-preload-api-composition.test.ts src/renderer/src/web/web-preload-api-github.test.ts src/renderer/src/web/web-preload-api-gitlab.test.ts src/renderer/src/web/web-preload-api-runtime-calls.test.ts src/renderer/src/web/web-preload-api-runtime-environment.test.ts src/renderer/src/web/web-preload-api-workspace-catalog.test.ts src/renderer/src/web/web-runtime-client-timeout-budget.test.ts— 8 files passed, 63 tests passed.pnpm exec oxlint --deny-warnings— passed with zero warnings.pnpm run typecheck— passed.pnpm run check:max-lines-ratchet— passed: 186 grandfathered suppressions, no new bypasses.