feat(automations): host selector for remote environment automations - #10347
feat(automations): host selector for remote environment automations#10347innocarpe wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughThe change centralizes automation host key validation, target resolution, and runtime host option construction. The Automations page now validates host selections, refreshes against the resolved target, rejects stale refresh results, preserves explicit selections during pending navigation, resets related state when the host changes, and displays a host selector when multiple hosts exist. Unit tests cover precedence, normalization, invalid entries, and unavailable selections. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/automations/AutomationsPage.tsx (1)
1003-1048: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMake host switches race-safe.
After Line 1055 changes the target, old automations remain actionable until refresh completes, and an older request can later overwrite the newer host’s data at Lines 1034-1046. Invalidate host-scoped results synchronously and gate every refresh commit with a monotonically increasing request ID.
Proposed sequencing guard
+ const refreshGenerationRef = useRef(0) const refresh = useCallback(async () => { + const generation = ++refreshGenerationRef.current setIsLoading(true) // ... try { const [nextAutomations, nextRuns, nextExternalManagers] = await Promise.all([ // ... ]) + if (generation !== refreshGenerationRef.current) return // commit results } finally { - setIsLoading(false) + if (generation === refreshGenerationRef.current) { + setIsLoading(false) + } } }, [automationHostTargetKey, selectAutomationId, settings]) const handleAutomationHostTargetChange = useCallback((nextKey: string) => { + refreshGenerationRef.current += 1 + // Clear or suppress prior host's automation selection/data here. setAutomationHostTargetKey(nextKey)Also applies to: 1050-1058
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0559b7ad-3e9a-4f20-bd33-aef877b071a1
📒 Files selected for processing (3)
src/renderer/src/components/automations/AutomationsPage.tsxsrc/renderer/src/components/automations/automation-host-client.test.tssrc/renderer/src/components/automations/automation-host-client.ts
394765e to
5ce2c7e
Compare
Sync update (
|
6ba8809 to
5313338
Compare
Sync update (
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8905574e-1227-4486-86fc-3256bd62eb31
📒 Files selected for processing (3)
src/renderer/src/components/automations/AutomationsPage.tsxsrc/renderer/src/components/automations/automation-host-client.test.tssrc/renderer/src/components/automations/automation-host-client.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/src/components/automations/AutomationsPage.tsx
- src/renderer/src/components/automations/automation-host-client.test.ts
The Automations page listed only through the global active runtime focus, so connected remote environments with automations stayed invisible. Add a Local + connected-runtime selector on the page and prefer that selection when listing, without changing the global active runtime. Closes stablyai#9964
Bare "environment:" must not override the global list fallback. Fall through to getAutomationListTarget instead. Addresses CodeRabbit review on stablyai#10187.
5313338 to
9f67efd
Compare
Sync update (
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d53920dc-cdcb-427b-b64c-11c57d4481fa
📒 Files selected for processing (3)
src/renderer/src/components/automations/AutomationsPage.tsxsrc/renderer/src/components/automations/automation-host-client.test.tssrc/renderer/src/components/automations/automation-host-client.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/src/components/automations/automation-host-client.test.ts
Sync update (
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/renderer/src/components/automations/AutomationsPage.tsx (1)
837-841: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftTrack the loaded host for pending navigation.
Lines 837-841 retain the explicit selector value during pending navigation. The completion effect then compares the pending target with that explicit value at Line 469 and returns.
If the user selects Local and opens a pending remote run, the page loads the remote data but never selects the automation or run. Record the target committed by the current refresh generation. Use that loaded target to complete pending navigation. Add a regression for Local selection with a pending remote run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 86d106bf-8a8d-4b6d-b040-af71b04ca7c4
📒 Files selected for processing (1)
src/renderer/src/components/automations/AutomationsPage.tsx
Summary
Fixes #9964.
Why
CLI
orca automations list --environment server-01already works, but the UI always listed viaactiveRuntimeEnvironmentId(or local). Projects from a connected remote could show in the sidebar while the Automations page stayed empty.Test plan
automation-host-clientunit tests (resolve host target + option list)ELI5
Automations UI always used the globally active runtime, so remote automations were hard to see. A host selector lets you pick Local or each connected environment without changing global focus.