Repository navigation
LOCAL: let a managed server link its update instructions - #17
Merged
Merged
Conversation
A headless Linux `orca serve` host cannot install its own updates, so a paired desktop tells the user to update it through its service manager. On a host whose install is owned by its deployment that advice is wrong: the next converge restores the pinned build. The host now reads ORCA_SERVER_UPDATE_HELP_URL once at startup. When it is an absolute https URL of at most 2048 characters with no control characters, and the host reports `manual-service-update-required`, the remote-update support carries it as an optional `helpUrl`. Anything else is ignored with one warning, and an unset variable leaves the wire byte-identical to before. The client re-validates the value with the same parser where it reads the status reply, dropping a malformed link without losing the rest of the support. The update dialog and the settings server row then show that the deployment manages updates, with a "How to update" link that opens in the external browser. A host that publishes no link renders exactly as before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Review complete. No blocking issues — approved ✅; 1 nitpick below. 🧹 Nitpicks (1) — 🟢 1 low
Adds a deployment-managed help link to the remote server update flow, with no significant issues found. Reviewed commit: 1714bc2 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This was referenced Oct 22, 2026
This was referenced Oct 30, 2026
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
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.
ELI5
When a server can't update itself, Orca tells you to update it "through the service manager". On our dev VMs that advice is wrong. Ansible installs a pinned Orca build there, so an update done by hand is undone at the next run. This lets the server give a link to its real update instructions, and the desktop shows that link.
What Changed
Before: a paired Linux
orca servehost with an update available shows "Update Orca on the server host… through the service manager that starts it" in the update dialog and in the Settings server row.After: if the host was started with
ORCA_SERVER_UPDATE_HELP_URL=https://…, those two places instead say "Updates for this server are managed by its deployment." and add a How to update link. The link opens in the external browser. Hosts without the variable look exactly as before.How:
src/main/updater/server-update-help-url-env.ts). It accepts only an absolutehttps:URL of at most 2048 characters with no control characters, and otherwise logs one warning and ignores it. It adds the URL as an optionalhelpUrlonRemoteServerUpdateSupport, and only when it reportsmanual-service-update-required. When the variable is unset, the key is left out entirely, so what the host sends is byte-identical to today.readRemoteServerUpdateSupportchecks the value again with the same parser where the status reply is read. A malformedhelpUrlis dropped without dropping the rest of the status.Why
docs/reference/remote-wire-compatibility.md: it is a new optional field on an existing message. There is no new opcode or RPC, and no capability flag is needed. Old clients ignore the field and new clients treat a missing or invalid value as absent. Nothing on the path rejects unknown fields: the CLI passes the status through whole, and mobile's parser allows extra fields and doesn't read this one.Linked Issue
None. This is fork-only (
LOCAL:). The host-side setting that uses it is in SwapLabsInc/SwapLabs#114 (devvms), for fork hosts only.Visual Proof
Not attached. A hidden-renderer Playwright check (Electron headless,
ORCA_BACKGROUND_LAUNCH=1) of the update dialog passed in dark and light themes. It showed the managed text and link, a click reached a stubbedshell:openUrlwith the URL, the window didn't navigate, and a host withouthelpUrlshowed today's text. The Settings server row was checked only in unit tests.Testing
pnpm tcpasses.pnpm testpasses on the new, touched and existing updater, remote-server-update, i18n and CLI-transport suites: 67 files and 695 tests passed, with 1 file and 14 tests skipped. I re-ran the 7 new test files myself: 82 tests passed.oxlintandcheck:code-quality:changedare clean, and the fourverify:localization-*scripts pass.What the tests cover:
helpUrlis present only when updates are manual and the value is valid.Not tested:
I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Written by Claude (Fable 5.1 as the implementing worker; Opus 5.5 coordinating and reviewing).
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
docs/yet.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.