Repository navigation
feat(status): report remote server listener, grants and connected clients - #26047
kambarakun wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📥 CommitsReviewing files that changed from the base of the PR and between ad05c4b4d0b0e20811a4815d51d966f4f9859b40 and ce7136c. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds optional remote-server diagnostics to status output. The runtime reports listener state, validated grant totals and entries, and counts of authenticated clients and their connections. The status RPC exposes the data through an opt-in parameter and limits access by caller scope. The CLI requests diagnostics for JSON status output. Tests cover collection, access boundaries, and default omission. Documentation explains the fields, limits, and unavailable states. Priority: ➖ Normal Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to JSON status remains compatible with older hosts, and the grant count reflects the documented device entries. No issue identified here blocks merging. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue [#25647] requests a status field for whether the app is advertised as a Remote Orca Server. The PR reports listener state, but listener state does not establish advertisement or external reachab… |
Add a separate field for the configured or advertised server state, or update [#25647] to remove that coding requirement before merge. |
| Docstring Coverage | |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 26 files. (1 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Out of Scope Changes check |  |
The status schema, CLI opt-in, server-state collection, authorization checks, sanitization, transport aggregation, tests, and documentation support [#25647]. The test relocation supports contract cove… |
| Title check | |
The title clearly and concisely describes the main change: reporting remote server listener, grant, and connected-client status. |
| Description check | |
The description follows the template and provides complete scope, rationale, linked issue, testing evidence, limitations, security considerations, and review decisions. The Visual Proof section does n… |
Full details: Linked Issues check
Explanation
Issue [#25647] requests a status field for whether the app is advertised as a Remote Orca Server. The PR reports listener state, but listener state does not establish advertisement or external reachability. The PR implements the requested listener, grant, and connected-client data, but it does not implement the advertised-state requirement.
Full details: Docstring Coverage
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 26 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
|
Two scope decisions for maintainer review:
I have clarified the PR description to separate advertisement from reachability. Both scope choices remain open for maintainer review; the implementation is unchanged. |
Expose bounded server diagnostics only when an authorized CLI requests them. Keep ordinary status probes and mobile responses unchanged, and distinguish unavailable data from an empty registry.
ad05c4b to
ce7136c
Compare
ELI5
orca status --jsoncan report the answering server's listener, access grants and currently connected paired clients. Users no longer need to combine OS connection tools with the credentials file for these facts.What Changed
Before, JSON status omitted remote-server diagnostics. After, an optional
remoteServerblock reports:disabled/not_listening.truncatedwhen necessary. There is no pagination. Unreadable or malformed inventories, including missing/empty tokens and duplicate IDs, returnunavailablerather than misleading counts.Only explicit JSON status opts in among built-in callers. Text status, startup/open polling and terminal-send preflight do not collect diagnostics. The authenticated local owner-token transport and paired
runtimepeers can provide the reader; mobile, SSH bridge credentials and internal calls without that reader cannot. Request parameters cannot confer permission. Tokens, keys, pairing URLs, push/relay credentials and administrative grant lists are not serialized.The four commits were rebased onto
cce31b4b4deeee9812f59e2ae81d3ce58bcebd8c. Main's new caller permissions, admission flow and sharedrpcContextFromTransportare retained. Both dispatcher implementations now match main; the reader passes through the shared constructor. A real-socket regression covers SSH bridge credentials with remote control both enabled and disabled. An existing relay-binding validator was moved unchanged into a named module to respect the 300-line limit.Why
The RPC server already owns the listener, registry and authenticated sockets. Reading that state on demand avoids OS-specific probes, stale copies and accidentally reporting the local machine for a remote request. The read does not create grants or widen network exposure.
A new optional request parameter and result field keep ordinary status lightweight and allow older hosts to omit the block. This does not introduce a protocol opcode, persistence change or new permissions framework.
Two choices remain for maintainer review:
orca-ide status --jsondoes not show whether the app is advertised as a Remote Orca Server or which clients are connected #25647?Linked Issue
Fixes #25647
The closing scope is subject to the advertisement decision above. If a separate sharing/configuration state is required, the implementation or closing linkage must be adjusted before merge.
Visual Proof
No graphical UI changes. Automated built-CLI captures use a real RPC server, temporary HOME/profile, loopback and synthetic grants. Current captures are from head
ce7136c6c0c1911907eb387327216f0a54f036f3(tree4fdce34e341c4773cf070d8591e55426e150b0f0).The listener reports
listening,127.0.0.1and its allocated port. The querying paired socket is included. Text status and pairedopen --jsondo not call the collector. Synthetic credentials are absent from output, and the server port closes after the fixture.Historical before capture: the built CLI at base
c0273b1ff7baomittedremoteServer. The new base also has no such field by source inspection; a new before CLI capture was not run.Testing
Current head above, macOS arm64, Node 24.16.0, Bun 1.4.2 and pnpm 12.8.1, with
ORCA_BACKGROUND_LAUNCH=1and frozen dependencies:pnpm tc:node,pnpm tc:cli,pnpm verify:rpc-params-catalog, changed-code quality against the recorded base, targeted oxlint/oxfmt andgit diff --checkpassed. Catalog regeneration produced no further change.tsc -p config/tsconfig.cli.json --outDir out --composite false --incremental false, thenverify-cli-bin.mjs; the global CLI installation hook was not invoked. The real-server smoke above passed.5272afeda68c2fe2bbdc3f09159c66688dc27328passed.Cross-version suite is not fully green. With that baseline and
vitest run --config config/vitest.config.ts tests/e2e/cross-version-wire/ --maxWorkers 2, 174/182 tests passed. Seven failures inqoder-history-search-downgradeandsession-search-agent-set-downgradereproduced with the same assertions on the unmodified new base in a separate clean worktree, using the same command and versions (both comparisons: seven failed / eleven passed). Those failures concern agent-list downgrade expectations and are outside this feature. The eighth case stops before execution because its fixedv1.4.222tag is absent locally. No tags or tests were changed to bypass it.A separate source-level check imported the v1.4.223 status handler/parser in-process and accepted the new optional parameter while omitting the new field. It was not a live released-server/client test. Missing-field replies are also covered by CLI tests.
Not run: full unit/lint/desktop build, a live released server with the new status parameter, Windows/Linux, live SSH/Tailscale/cloud relay or manual UI checks. New-head GitHub CI is separate from these local results.
Historical evidence: fork CI attempt 2 passed on
de84b652and its fixed review base, after tag supplementation. Its first-attempt failures and baseline reproductions remain recorded in fork PR #1. Neither that CI nor reviews ofad05c4b4cover this rebased head.AI Disclosure
OpenAI Codex implemented the original feature and supervised this integration. Claude Opus 5.5 performed the rebase and verification; Codex independently reviewed the final diff, authorization boundaries, range-diff and logs, and ran the five-file test subset.
Review
The four commit intents are preserved. Supervisor review found no additional feature regression in the reviewed changes; the cross-version failures and untested environments above remain explicit. Maintainer decisions on advertisement scope and runtime metadata access are still pending. Earlier CodeRabbit/Copilot reviews apply only to their recorded revisions, not this head.
Agent skill upstream boundary
Notes
A paired query describes the answering host. SSH bridge and legacy internal routes omit diagnostics rather than substituting another host's state. Counts describe paired identities, not physical devices. Listener state is not an external reachability test. The implementation uses existing in-process state across platforms; cross-platform execution remains unverified.
X: @kambarakun
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)This is one server-to-CLI feature across 26 files, including eight test files. CLI evidence replaces graphical screenshots; full-check success is not claimed.