Repository navigation
Edit agent launch settings on the workspace host - #26479
Conversation
bea35e6 to
f012898
Compare
…-agent-settings-on-host # Conflicts: # src/main/runtime/rpc/methods/client-ui.ts # src/renderer/src/hooks/useDetectedAgents.ts # src/renderer/src/i18n/locales/en.json # src/renderer/src/store/slices/runtime-detected-agents.ts
…ort fence A re-pair already clears every detected-agent list, flag and pending probe for the environment, so the per-record pairing owner keys, the owner-keyed pending probes, the settle-time pairing checks and the hook's ownership gating were never reached. Detection now captures the pairing before the capability wait and fences its RPCs with it; the refresh fallback still refuses after a re-pair. Pins the fence across a capability wait, the needs-update clear, and the agent-launch RPC permission levels.
…elper Main's new owner-routing ratchet counts getActiveRuntimeTarget calls, and this pane's no-workspace default added one. It now reads the Active Server field the way the Agents pane did before host selection, so the default is unchanged.
…-agent-settings-on-host # Conflicts: # src/renderer/src/store/slices/runtime-detected-agents.ts
…rt fence for the refresh fallback A re-pair clears an environment's detected-agent lists, but a mounted tab bar, quick launch or composer that was still waiting for its first list (or whose first probe had failed) never asked again, leaving "Can't reach this workspace's host" until another surface mounted. The hook now re-runs its effect when the target environment's pairing revision changes. The refresh fallback no longer re-checks the pairing in the renderer: it already carries the pairing captured before the wait, and main refuses any mismatch. Pins the Settings host form's remount key on re-pair.
Review summary (final head
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue is established by the supplied changes; normal checks can proceed. Pre-merge checks |
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
eeb2aac2-9f31-4321-ac72-d2600eb367e8
⛔ Files ignored due to path filters (1)
src/shared/rpc-contract/rpc-params-catalog.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (43)
config/scripts/locale-ko-key-overrides.jsonconfig/scripts/vitest-sqlite-runtime-files.mjssrc/main/runtime/agent-launch-settings-rollback.tssrc/main/runtime/orca-runtime-pty-foreground-process-reads.tssrc/main/runtime/rpc/methods/client-ui.tssrc/main/runtime/rpc/rpc-caller-scope.test.tssrc/main/runtime/runtime-client-agent-launch-settings-durability.test.tssrc/main/runtime/runtime-client-agent-launch-settings.test.tssrc/main/runtime/runtime-client-settings.tssrc/renderer/src/components/settings/AgentCatalogRow.tsxsrc/renderer/src/components/settings/AgentLaunchDefaultsEditor.test.tsxsrc/renderer/src/components/settings/AgentLaunchDefaultsEditor.tsxsrc/renderer/src/components/settings/AgentLaunchSettingsCatalog.tsxsrc/renderer/src/components/settings/AgentLaunchSettingsSection.test.tsxsrc/renderer/src/components/settings/AgentLaunchSettingsSection.tsxsrc/renderer/src/components/settings/AgentsPane.test.tsxsrc/renderer/src/components/settings/AgentsPane.tsxsrc/renderer/src/components/settings/HostAgentEnvironmentEditor.tsxsrc/renderer/src/components/settings/HostAgentLaunchSettings.test.tsxsrc/renderer/src/components/settings/HostAgentLaunchSettings.tsxsrc/renderer/src/components/settings/agent-availability-settings.tssrc/renderer/src/components/settings/agent-launch-settings-transport.test.tssrc/renderer/src/components/settings/agent-launch-settings-transport.tssrc/renderer/src/components/settings/setting-ownership.test.tssrc/renderer/src/components/settings/setting-ownership.tssrc/renderer/src/components/settings/use-agent-launch-field-write.tssrc/renderer/src/hooks/useDetectedAgents.re-pair.test.tsxsrc/renderer/src/hooks/useDetectedAgents.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/fr.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/store/slices/runtime-detected-agents-pairing.test.tssrc/renderer/src/store/slices/runtime-detected-agents.tssrc/renderer/src/store/slices/runtime-status.tssrc/shared/agent-launch-runtime-capability.tssrc/shared/agent-launch-settings.tssrc/shared/rpc-contract/agent-launch-settings-params.tssrc/shared/tui-agent-launch-defaults.tssrc/shared/tui-agent-selection.tstests/e2e/host-agent-launch-settings-hook-bookkeeping.unit.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| const initialHost = useAppStore((state) => { | ||
| if (state.activeWorktreeId) { | ||
| return getResolvedExecutionHostIdForWorktree(state, state.activeWorktreeId) | ||
| } | ||
| // Why: with no workspace the pane opens where it did before host selection, the Active Server. | ||
| const activeServer = settings.activeRuntimeEnvironmentId?.trim() | ||
| return activeServer ? toRuntimeExecutionHostId(activeServer) : LOCAL_EXECUTION_HOST_ID | ||
| }) | ||
| const environments = useAppStore((state) => state.runtimeEnvironments) | ||
| const [selected, setSelected] = useState<ExecutionHostId | null>(initialHost) | ||
| if (!selected && initialHost) { | ||
| setSelected(initialHost) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
set -eu
printf '%s\n' '--- callers ---'
rg -n -F --glob '*.{ts,tsx}' -- 'AgentLaunchSettingsSection' src/ || test "$?" -eq 1
printf '%s\n' '--- settings agents identifiers ---'
rg -n -F --glob '*.{ts,tsx}' -- 'AgentsPane' src/ || test "$?" -eq 1
rg -n -F --glob '*.{ts,tsx}' -- 'activeWorktreeId' src/renderer/src/components/settings src/renderer/src 2>/dev/null | head -120 || true
printf '%s\n' '--- candidate settings files ---'
rg --files src/renderer/src | rg 'settings|Settings' | head -120Repository: stablyai/orca
Length of output: 39254
🏁 Script executed:
set -eu
rg -n -F --glob '*.tsx' --glob '*.ts' -- 'AgentLaunchSettingsSection' src/ || test "$?" -eq 1
rg -n -F --glob '*.tsx' --glob '*.ts' -- 'AgentsPane' src/ || test "$?" -eq 1
rg -n -F --glob '*.tsx' --glob '*.ts' -- 'activeWorktreeId' src/renderer/src/components/settings src/renderer/src 2>/dev/null | head -120 || trueRepository: stablyai/orca
Length of output: 30481
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
for p in Path('src/renderer/src').rglob('*.tsx'):
text = p.read_text(errors='replace')
if 'AgentLaunchSettingsSection' in text or 'AgentsPane' in text:
print(f'--- {p} ---')
for i, line in enumerate(text.splitlines(), 1):
if 'AgentLaunchSettingsSection' in line or 'AgentsPane' in line:
lo=max(1, i-12); hi=min(len(text.splitlines()), i+20)
lines=text.splitlines()
for n in range(lo, hi+1):
print(f'{n}: {lines[n-1]}')
PYRepository: stablyai/orca
Length of output: 41891
🏁 Script executed:
set -eu
for f in \
src/renderer/src/components/settings/settings-capability-section-renderers.tsx \
src/renderer/src/app-shell/app-root-surface-settings.ts \
src/renderer/src/components/settings/SettingsPane.tsx \
src/renderer/src/components/settings/Settings.tsx \
src/renderer/src/app-shell/AppWorkspaceShell.tsx; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
nl -ba "$f" | sed -n '1,260p'
fi
done
printf '\n--- settings surface references and keys ---\n'
rg -n -F --glob '*.ts' --glob '*.tsx' -- 'app-root-surface-settings' src/renderer/src || test "$?" -eq 1
rg -n --glob '*.ts' --glob '*.tsx' 'settings.*key|key=.*(workspace|worktree|settings)|activeView.*settings|activeView === .settings.|settingsVisible|settingsOpen' src/renderer/src/app-shell src/renderer/src/components src/renderer/src/hooks | head -200 || trueRepository: stablyai/orca
Length of output: 41261
🏁 Script executed:
nl -ba src/renderer/src/components/settings/AgentLaunchSettingsSection.tsx | sed -n '1,58p'Repository: stablyai/orca
Length of output: 3115
Synchronize the selected host with workspace changes.
Settings remains mounted when activeView stays settings, so switching workspaces does not reinitialize selected. The selector can continue editing the previous workspace’s host.
Suggested fix
-import { useRef, useState } from 'react'
+import { useEffect, useRef, useState } from 'react'
...
const environments = useAppStore((state) => state.runtimeEnvironments)
const [selected, setSelected] = useState<ExecutionHostId | null>(initialHost)
- if (!selected && initialHost) {
+
+ useEffect(() => {
setSelected(initialHost)
- }
+ }, [initialHost])…-agent-settings-on-host # Conflicts: # src/main/runtime/rpc/rpc-caller-scope.test.ts # src/renderer/src/components/settings/AgentsPane.test.tsx # src/renderer/src/components/settings/AgentsPane.tsx
|
Synced with What changed for you.
One unrelated formatting change: the commit hook reformatted one long line from Checks
Live SSH QA rerun on this head (screenshots in the body replaced). Everything passed:
|
…-agent-settings-on-host
* Add host-owned agent launch settings mutations * Edit agent launch defaults on the selected execution host * Await durable writes for host agent launch settings * Fence agent detection by execution-host pairing * Acknowledge host settings before hook reconciliation * Preserve Reset after pending agent launch saves * Simplify agent detection re-pair handling to the clear and the transport fence A re-pair already clears every detected-agent list, flag and pending probe for the environment, so the per-record pairing owner keys, the owner-keyed pending probes, the settle-time pairing checks and the hook's ownership gating were never reached. Detection now captures the pairing before the capability wait and fences its RPCs with it; the refresh fallback still refuses after a re-pair. Pins the fence across a capability wait, the needs-update clear, and the agent-launch RPC permission levels. * Open launch settings on the Active Server without the focus-routing helper Main's new owner-routing ratchet counts getActiveRuntimeTarget calls, and this pane's no-workspace default added one. It now reads the Active Server field the way the Agents pane did before host selection, so the default is unchanged. * Re-probe mounted launch surfaces after a re-pair; rely on the transport fence for the refresh fallback A re-pair clears an environment's detected-agent lists, but a mounted tab bar, quick launch or composer that was still waiting for its first list (or whose first probe had failed) never asked again, leaving "Can't reach this workspace's host" until another surface mounted. The hook now re-runs its effect when the target environment's pairing revision changes. The refresh fallback no longer re-checks the pairing in the renderer: it already carries the pairing captured before the wait, and main refuses any mismatch. Pins the Settings host form's remount key on re-pair. (cherry picked from commit 5035baa) Backport to 1.4.224: 1.4.224 has no Settings host picker, so the Agents pane edits the Active Server's launch settings (settings.activeRuntimeEnvironmentId) instead of a hostScope; detection has no workspace-scoped capability wait, so only the refresh fallback is pairing-fenced. Dropped tests for the host picker's unavailable notice, the hostScope-vs-Active-Server case, and workspace-scoped detection fencing.
ELI5
Settings → Agents edited this computer even when an SSH chat ran on another host. The server read its own launch settings, so arguments and environment values entered on the desktop never reached the chat.
The Agents pane now edits the host chosen in Settings’ Host menu (added on
mainby #27265; it starts on the Active Server). Choose the SSH host there and the pane shows and saves that host’s launch settings, which its agent launcher already reads. This computer’s own preferences stay in a separate section.What Changed
The pane follows the page-wide Host menu from feat(settings): host-owned settings panes choose their host explicitly #27265 rather than adding its own picker, so Accounts, Agents and Terminal can never disagree about which host they show. Its launch section shows:
main’s “no longer saved” notice for a removed server;Local preferences have their own subsection.
Reused the agent catalog and launch editors for the selected host’s default agent, availability, command, arguments, and existing permission defaults. The permission description remains exactly as on main; feat(native-chat): each Claude/Codex chat gets its own permission mode #26116 owns changing it when Chat permissions are separated.
Saved remote environment values stay on their host: the editor receives names only, accepts explicit replacements/removals, and preserves other entries. Switching hosts or re-pairing clears unsaved values.
Added narrowly scoped, validated host settings methods. Each change reads the host’s latest settings inside its existing serialized disk-write boundary, preserving unrelated changes instead of replacing a desktop snapshot. Success waits for the actual commit; a failed write restores fields that no newer write superseded and keeps the typed replacement retryable. Hook installation runs separately after commit and reports failures without delaying acknowledgements or later saves.
Command and Arguments Reset stays actionable while a blur save is pending. The field writer orders distinct values and coalesces duplicate submissions, so the first Reset click takes effect after the preceding save, including when that save fails.
Re-pairing a saved host (possibly to a different machine under the same entry) clears that host’s cached agent lists, in-flight detection and refresh requests, and “update this server” flags in one step, so the replacement starts with nothing from its predecessor. Each detection request records the host’s pairing before it waits on the host’s capability check and sends with that pairing (including an older host’s probe of the SSH target a workspace lives on), so the transport refuses a request started for the old machine instead of delivering it to the replacement. An older server’s refresh fallback carries that same recorded pairing, so the transport refuses it after a re-pair too. A tab bar, quick launch menu or composer that is open during a re-pair probes the replacement again, including when the old machine’s first probe was still running or had already failed. No per-list ownership bookkeeping is stored: Settings and the workspace launch surfaces read the same store and are cleared the same way, though Settings shows the host’s default list while a workspace’s tab bar and quick launch show that workspace’s own list (for example, a WSL workspace on a Windows host).
The two new host settings methods declare caller permissions. Reading launch settings (
settings.getAgentLaunch) is workspace level: it returns variable names only, less thansettings.getalready returns at that level, Changing them (settings.mutateAgentLaunch) needs settings-write, likesettings.update, so paired desktop and web clients can save. Mobile gets neither method.Older servers show an update-required message. Disconnected hosts cannot silently fall back to saving desktop settings. Legacy methods and the mobile method allowlist remain unchanged.
Why
The execution host already owns agent launch settings. Editing that store fixes the disagreement between what Settings shows and what the launcher uses, without adding another persisted copy or importing desktop environment secrets during pairing.
Linked Issue
Reported SSH native-chat reproduction. Related permission changes: #26116 and #26059.
Visual Proof
A live check over SSH on the final head
71a3b26038f, which already includes currentmainwith #26450. A local desktop app was connected to a managed Orca server in a Linux container, using a stand-in Claude (the real CLI never ran).1. Settings' Host menu offers this computer and the SSH host. The menu was added on
mainby #27265 and is shared by Accounts, Agents and Terminal. Settings first opened on this computer (shown as "Local Mac"), even though the active workspace was on the SSH host. That'smain's default, which starts on the Active Server.2. With the SSH host chosen, the Agents pane shows that host's agents and launch settings. This computer's own preferences are in their own section.
3. Claude's arguments and an environment variable are edited for the SSH host. The edits are
--qa-markerandQA_MARKER=pr26479. The saved value is shown by name only; the field is empty and says "Saved value · enter to replace". The host's own settings database holds both.4. A new Claude chat on that host uses them. The chat process launched with
--qa-markerand hadQA_MARKERset to the saved value.5. This computer's own settings did not change. The desktop's launch settings are byte-identical before and after, and choosing this computer in the Host menu shows no
--qa-marker.6. Reset removes the argument on the host. The next new chat launched without
--qa-marker.Not exercised live: re-pairing a host to a different machine (covered by tests), Windows and WSL hosts, and an older server.
Testing
307 targeted tests across 24 files passed on this head (this PR’s tests plus the shared agent-detection, quick launch, inventory and transport pairing-check tests), including host-store mutations, the actual Agents pane’s SSH folder routing, masked values, explicit saves, failed-write retries, successive field saves, old-server compatibility, pairing replacement, Windows environment-name handling, and unchanged mobile access restrictions. Real SQLite tests cover delayed commits, failed-write rollback/retry, queued intents after failure, and accepted variable names such as
__proto__surviving storage and unrelated edits. Re-pairing regressions cover cached lists, replies arriving in both orders, an old refresh fallback (sent only with the old pairing, which main refuses), a mounted launch surface re-probing after a re-pair whose old probe was in flight or had failed, the Settings host form remounting on re-pair, the “update this server” flag, and a workspace detection whose capability check straddles a re-pair, for both the host’s own probe and an older host’s SSH-target probe (each fails if the pairing is not captured before that wait or not sent). Permission regressions pin both new methods’ levels and the SSH CLI refusal of saves with and without the opt-in; swapping the two levels fails them. Main’s cross-version test for workspace-scoped detection against the newest release passed (4 tests). Real-store and rendered-form regressions also verify that pending or failed agent-hook work does not block committed settings or the next form edit. Real pointer-sequence tests verify both focused-field Reset controls, ordered saves after rejection, and duplicate Enter/blur and Reset coalescing.The pane regression fails against the original implementation and passes with the fix: it expects host arguments and a host write while the desktop’s arguments and secrets remain unchanged. Node and renderer typechecks passed through the machine queue on this head. The required
orca-ci-checks --no-typecheckrun passed all 24 checks, including main’s new owner-routing ratchet.CI on head 55c5c95 (this round’s fixes; no new merge of main, GitHub reports the PR mergeable): 14 checks passed and 18 were skipped, with no failed or pending checks. The five test shards, relay integration, static analysis and typecheck, both packaging jobs, the size check, change detection, and verification passed. The Windows packaging job first failed on a 15-second timeout in a Windows batch-launcher test (
preflight-runnable-local-cli.test.ts, "recovers after a hung batch launcher"), which this PR does not touch (600 of that job’s other tests passed); one rerun of the failed jobs passed. On the previous head, shard 1 had likewise needed one rerun for an unrelated native-chat hook test, and an earlier push failed main’s new owner-routing ratchet until the pane’s no-workspace default read the Active Server field directly, with the same result for the user. Skipped checks are recorded as skipped, not passes.Not yet verified: packaged Electron rendering, live managed SSH launches, Linux/Windows integration, or WSL integration. No provider launch code changed, and no real agent credentials were used.
Review
All review rounds’ findings have regression coverage and fixes. The final-head review found the merged re-pair handling carried guards that the re-pair clear already made unreachable; they were removed, and the transport pairing check and the permission levels it found untested are now pinned. The review of that simplification found one removal went too far: open launch surfaces no longer retried after a re-pair, so they could stay on “Can’t reach this workspace’s host”. The hook again re-runs when the host’s pairing changes, the renderer’s duplicate fallback check was dropped in favor of the transport’s, and the Settings remount on re-pair is now pinned. Each new test fails against the previous head (or with the remount key removed) and passes with the fix. The last fix round (9 lines in the hook plus tests) was read by the coordinator rather than a fresh reviewer, and the live SSH check above ran on it. #26116 owns the separate Chat permissions change, and this lane retains main’s current permission wording and behavior.
Agent skill upstream boundary
Notes
The settings mechanism follows the common pattern: the agent-owning server stores configuration, and the editor applies individual changes to that server.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)