feat(remote): add HarmonyOS proxy lease backend - #2266
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new harmonyos-instance backend is added to the contract but still needs follow-up updates to hardcoded CLI/remote-config validation lists and related platform compatibility/test coverage to avoid runtime/UX breakage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends the remote connection/lease contract surface to recognize HarmonyOS devices by introducing a new harmonyos-instance lease backend and wiring HarmonyOS proxy devices + CLI lease-backend resolution to use it.
Changes:
- Extend shared contracts to include
SessionRuntimeHints.platform: 'harmonyos'andLeaseBackend: 'harmonyos-instance'. - Resolve
--platform harmonyosand HarmonyOS proxy devices to theharmonyos-instancelease backend for allocation/heartbeat/close flows. - Update the CLI error text for cases where a lease backend must be explicitly determined.
File summaries
| File | Description |
|---|---|
src/cli/commands/connection-runtime.ts |
Adds HarmonyOS → harmonyos-instance backend resolution for flags and proxy devices; updates related error messaging. |
packages/kernel/src/contracts.ts |
Expands the shared contract unions for session runtime hints and lease backend backends to include HarmonyOS. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| export type SessionRuntimeHints = { | ||
| platform?: 'ios' | 'android'; | ||
| platform?: 'ios' | 'android' | 'harmonyos'; | ||
| metroHost?: string; |
| const DAEMON_LOCK_POLICIES = ['reject', 'strip'] as const; | ||
| export type DaemonLockPolicy = (typeof DAEMON_LOCK_POLICIES)[number]; | ||
| const LEASE_BACKENDS = ['ios-simulator', 'ios-instance', 'android-instance'] as const; | ||
| const LEASE_BACKENDS = ['ios-simulator', 'ios-instance', 'android-instance', 'harmonyos-instance'] as const; |
| if (flags.leaseBackend) return flags.leaseBackend; | ||
| if (flags.platform === 'android') return 'android-instance'; | ||
| if (flags.platform === 'ios') return 'ios-instance'; | ||
| if (flags.platform === 'harmonyos') return 'harmonyos-instance'; |
|
Thanks for taking this on — HarmonyOS support is a welcome addition and the overall shape of the change looks right. A few things need to land before it works end to end, mostly allowlists that still need the new backend added. Blocking
Worth a look
Docs and tests
CI hasn't run on this yet, so it's worth a rerun once the above is pushed. Happy to help with any of it if useful — thanks again for the contribution! Generated by Claude Code |
There was a problem hiding this comment.
🟡 Changes recommended
The new HarmonyOS backend/runtime literals are added to the shared contract but existing daemon-side validation/allowlists still reject them, which will break real lease admission and runtime-hint usage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
| export type SessionRuntimeHints = { | ||
| platform?: 'ios' | 'android'; | ||
| platform?: 'ios' | 'android' | 'harmonyos'; | ||
| metroHost?: string; |
| export function resolveRequestedLeaseBackend(flags: CliFlags): LeaseBackend | undefined { | ||
| if (flags.leaseBackend) return flags.leaseBackend; | ||
| if (flags.platform === 'android') return 'android-instance'; | ||
| if (flags.platform === 'ios') return 'ios-instance'; | ||
| if (flags.platform === 'harmonyos') return 'harmonyos-instance'; | ||
| return undefined; |
|
Addressed the blocking review items in
Validation: The provider daemon still needs to expose an actual HarmonyOS lease implementation before this can be considered end-to-end; this PR keeps unsupported providers fail-closed and documents that scope. |
There was a problem hiding this comment.
🔵 Needs a closer look
The updated runtime/platform compatibility check introduces a behavior regression that can drop runtime hints for non-leaf platform selectors (e.g., apple) and should be corrected before approval.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/cli/commands/connection-runtime.ts:740
isRuntimeCompatibleWithPlatformnow enforces strict equality for any--platformselector. This is a behavior change from the prior logic (which only enforced forios/android) and can cause runtime hints (e.g., metroHost/metroPort) to be dropped when callers use non-leaf selectors likeapple,vega, etc. If the intent is only to extend the strict check to HarmonyOS, keep the previous guard and addharmonyosto it.
src/tests/remote-connection.test.ts:51- This test uses
as neverto bypass theCliFlagstype, which can hide real type errors. Other tests in this file pass a minimalCliFlagsobject instead; do the same here so the compiler keeps protecting the call site.
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Follow-up pushed in
Validation: The PR remains intentionally contract-level: actual proxy daemon/provider Harmony lease implementation and end-to-end open/heartbeat/close still require maintainer design and follow-up. Until then, proxy OHOS remains unsupported at runtime. |
There was a problem hiding this comment.
🔵 Needs a closer look
The contract/schema now accepts HarmonyOS runtime hints, but daemon-side runtime-hint normalization still only supports iOS/Android, so HarmonyOS runtime hints will fail at runtime.
Review details
Suppressed comments (1)
packages/kernel/src/contracts.ts:13
SessionRuntimeHints.platformanddaemonRuntimeSchemanow accept "harmonyos", but the daemon-side runtime hint normalization still only recognizes ios/android (e.g.src/daemon/session-runtime.tsrejects any other value andtoRuntimePlatformmaps only ios/android). This makes HarmonyOS runtime hints fail at runtime despite the updated contract/schema.
export type SessionRuntimeHints = {
platform?: 'ios' | 'android' | 'harmonyos';
metroHost?: string;
metroPort?: number;
bundleUrl?: string;
launchUrl?: string;
};
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
BLOCKED at
|
af89c1f to
1533c52
Compare
|
Rebased onto current upstream main |
There was a problem hiding this comment.
🟡 Changes recommended
The PR introduces a runtime-hints contract mismatch and a likely compatibility regression (plus missing wire-compat ledger updates) that can break expected behavior and/or CI.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
src/cli/commands/connection-runtime.ts:741
isRuntimeCompatibleWithPlatformnow comparesruntime.platform("ios"|"android"|"harmonyos") directly toCliFlags['platform'](PlatformSelector, including values like "apple", "macos", "vega", etc). This is a behavior change from the previous guard and will drop stored runtime hints when callers use selectors like--platform apple, even though that alias is commonly accepted elsewhere.
if (!runtime.platform || !platform) {
return true;
}
return runtime.platform === platform;
}
packages/kernel/src/contracts.ts:275
- This expands
SessionRuntimeHints.platform/daemonRuntimeSchemato accept "harmonyos", but the daemon-side runtime hint normalization still only supports "ios" and "android" (e.g.src/daemon/session-runtime.tsrejects anything else vianormalizeRuntimePlatformInput/toRuntimePlatform). As-is, the contract says HarmonyOS is allowed while the daemon will still throwINVALID_ARGS, so HarmonyOS runtime hints are not actually supported yet.
export const daemonRuntimeSchema = schema<SessionRuntimeHints>((input, path) => {
const record = expectObject(input, path);
return {
platform: optionalEnum(record, 'platform', ['ios', 'android', 'harmonyos'] as const, path),
metroHost: optionalString(record, 'metroHost', path),
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
| const LEASE_BACKENDS = ['ios-simulator', 'ios-instance', 'android-instance', 'harmonyos-instance'] as const; | ||
| export type LeaseBackend = (typeof LEASE_BACKENDS)[number]; |
| - Added the `harmonyos-instance` remote lease backend so HarmonyOS proxy devices can participate in | ||
| the same explicit allocation, heartbeat, and close lifecycle as other remote instances (#2266). | ||
|
|
||
| - Fixed: `settings airplane on|off` now takes an Android device offline. It is applied through |
|
Addressed the maintainer review in
Validation on the rebased branch: |
There was a problem hiding this comment.
🟡 Changes recommended
It overwrites an existing Unreleased changelog entry and introduces a test that should be colocated with the existing lease-registry-scope test module for consistency.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
CHANGELOG.md:7
- This change replaces (removes) the existing Unreleased changelog entry about strict
wait absent <selector>polling (#2236). If that feature is still unreleased, it should remain in the Unreleased section and this HarmonyOS entry should be added as an additional bullet rather than overwriting it.
- Added the `harmonyos-instance` lease contract and CLI/runtime plumbing as a prerequisite for
HarmonyOS proxy support; provider/daemon allocation remains gated until its end-to-end lifecycle
is implemented and validated (#2266).
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Lite
| test('HarmonyOS platform resolves to its proxy lease backend', () => { | ||
| assert.equal(resolveRequestedLeaseBackend({ platform: 'harmonyos' } as never), 'harmonyos-instance'); | ||
| }); |
| test('normalizeLeaseBackend accepts HarmonyOS instance backend', () => { | ||
| assert.equal(normalizeLeaseBackend('harmonyos-instance'), 'harmonyos-instance'); | ||
| }); |
|
Follow-up |
There was a problem hiding this comment.
🔵 Needs a closer look
It currently drops an existing Unreleased changelog entry and introduces test issues (misplaced test and a type-escaping as never) that should be corrected before approval.
Review details
Suppressed comments (3)
CHANGELOG.md:8
- This edit removes the existing Unreleased changelog entry about strict
wait absentpolling (#2236). Unless that item was intentionally dropped elsewhere, it should be kept and the HarmonyOS lease note added alongside it to avoid losing release notes.
- Added the `harmonyos-instance` lease contract and CLI/runtime plumbing as a prerequisite for
HarmonyOS proxy support; provider/daemon allocation remains gated until its end-to-end lifecycle
is implemented and validated (#2266).
src/daemon/tests/lease-registry.test.ts:15
- This new test exercises
normalizeLeaseBackendfromlease-registry-scope.ts, but it’s being added tolease-registry.test.ts. There is already a dedicatedlease-registry-scope.test.ts; moving this test there keeps tests aligned with the module under test and avoids mixing scope-validation coverage into the registry suite.
test('normalizeLeaseBackend accepts HarmonyOS instance backend', () => {
assert.equal(normalizeLeaseBackend('harmonyos-instance'), 'harmonyos-instance');
});
src/tests/remote-connection.test.ts:52
- Using
as neverhere defeats type-checking and can mask future signature changes toresolveRequestedLeaseBackend. Prefer constructing a realCliFlagsvalue via the existingforceConnectFlagshelper (already imported in this file).
test('HarmonyOS platform resolves to its proxy lease backend', () => {
assert.equal(resolveRequestedLeaseBackend({ platform: 'harmonyos' } as never), 'harmonyos-instance');
});
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Lite
Signed-off-by: Ark <artin@cat.ms>
Signed-off-by: Ark <artin@cat.ms>
Signed-off-by: Ark <artin@cat.ms>
Signed-off-by: Ark <artin@cat.ms>
Signed-off-by: Ark <artin@cat.ms>
Signed-off-by: Ark <artin@cat.ms>
Signed-off-by: Ark <artin@cat.ms>
Signed-off-by: Ark <artin@cat.ms>
Signed-off-by: Ark <artin@cat.ms>
Signed-off-by: Ark <raft-mobile-ark@mail.build>
68c21a0 to
fa0b33c
Compare
|
Follow-up pushed at fa0b33c (rebased onto 367e795). Resolved conflicts without dropping upstream changelog/ledger entries; fixed Node runtime normalization and .ad open/runtime-set roundtrips through one kernel runtime-platform vocabulary; corrected runtime-set guidance. A new proxy inventory/materialization test also exposed missing HarmonyOS serial propagation, now fixed. Local affected gates passed, including 4,834 related tests and wire compatibility; the PR body has exact evidence and RED/GREEN details. Live proxy/device evidence is still NOT RUN because local DevEco startup is blocked on license acceptance. I am not requesting ready-for-human or claiming merge readiness. Could a maintainer approve/trigger the upstream exact-head CI if this fork run requires approval? I will continue with the live lifecycle receipt when the emulator is available. |
There was a problem hiding this comment.
🟡 Changes recommended
A follow-up update is needed so remote-bridge gating recognizes harmonyos-instance; otherwise HarmonyOS proxy leases may silently skip metro/bridge behavior in dependent CLI commands.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 1
- Review effort level: Lite
| const LEASE_BACKENDS = [ | ||
| 'ios-simulator', | ||
| 'ios-instance', | ||
| 'android-instance', | ||
| 'harmonyos-instance', | ||
| ] as const; |
|
The earlier runtime-hint, script roundtrip, and serial propagation gaps are fixed in fa0b33c. No further code findings from this review. The remaining requirement is a live HarmonyOS proxy run covering inventory → open → snapshot/input/logs → close, with lease and artifact evidence; the reported DevEco license prompt still blocks that validation, so ready-for-human should wait. CI separately needs maintainer approval before it can run; this is not a test failure. |
HarmonyOS live proxy receipt — 2026-09-08Exact head: PASS for live proxy inventory → lease allocation/materialization → open → snapshot → selector input → logs → close. Client and proxy are separate processes on the same Mac, communicating over loopback HTTP through Device: HarmonyOS emulator, serial Observations
Artifact provenance
LimitsNo physical OHOS, Linux client, cross-host tunnel, text-entry, Metro/React DevTools, or Raft application repair claim. Input coverage is the selector click with observed navigation. Upstream CI still requires maintainer approval; no merge-ready claim based on this receipt alone. |
|
The reported live proxy run on fa0b33c resolves the remaining validation gap: it covers lease allocation, Settings navigation through a selector click, logs, screenshot, and lease invalidation after close. No remaining code findings; ready for human review. This is same-host HTTP evidence, not a Linux-client or cross-host test, and CI still needs maintainer approval before merging. |
thymikee
added
the
ready-for-human
|
let's rebase this and make sure coverage is good |
|
Coverage exposed two changes that need correction at fa0b33c. The new runtime import from script-utils into kernel/contracts expands the eager import graph through the ad-script facade; preserve the shared platform vocabulary without adding that eager edge, and rerun the eager-closure gate. The new backend-resolution test also grows remote-connection.test.ts past its pinned size; move it into the existing HarmonyOS-specific test file. The reported live proxy validation remains useful, but these code-quality findings need resolving before review readiness. |
thymikee
removed
the
ready-for-human
Signed-off-by: Ark <raft-mobile-ark@mail.build>
|
Addressed the eager-closure and test-size findings in 0baaaa0. The script codec now imports the shared runtime-hint vocabulary as a type only; its local lookup is checked exhaustively with satisfies Record<NonNullable<SessionRuntimeHints["platform"]>, true>, so missing/new/invalid platform entries fail compilation without evaluating kernel/contracts at startup. Moved the backend regression into the HarmonyOS test file and restored the legacy test file to upstream size. Observed both quality gates RED at fa0b33c; all 495 targeted gate/behavior tests now pass, and the full affected gate passes with AWS region overrides unset. The earlier live proxy receipt remains bound to fa0b33c, not relabeled as a new-head device run. Please rerun/review this current exact; no readiness label applied. |
There was a problem hiding this comment.
🔵 Needs a closer look
The Unreleased changelog entry describes HarmonyOS proxy support as still gated/incomplete, which conflicts with the functionality and tests added in this PR.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
CHANGELOG.md:49
- This changelog entry says HarmonyOS proxy support is still gated pending an end-to-end allocation lifecycle, but this PR adds the
harmonyos-instancebackend with allocation/close wiring and tests. Please update the bullet so it reflects the shipped behavior (and doesn’t imply the feature is incomplete).
- Files reviewed: 17/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
The eager-import and test-size findings are fixed at 0baaaa0. No remaining code findings; the earlier live proxy receipt still covers the unchanged device path, while the reported structural gates cover this delta. Ready for human review. Exact-head CI has not run yet. |
thymikee
added
the
ready-for-human
Summary
HarmonyOS devices exposed by a proxy can select the additive
harmonyos-instancelease backend. Selected serials survive command materialization, and HarmonyOS runtime hints survive Node response normalization and.adrecording/replay. Runtime platform validation is derived from one kernel vocabulary; existing non-leafappleselection remains compatible, while leaf mismatches remain strict.Related to #2265. Unsupported providers remain fail-closed; this does not add HarmonyOS capacity to iOS/Android-only cloud providers.
Validation
Published head:
fa0b33c679820943bf15e7228cba3d0dedcfa033, tree6218ca9415334b96080e427d9f81fed6ee880214, base367e795ee72458ef2a340a627e5e9a1680c03712. 18 touched files in the complete PR.Local affected gate passed: format, lint, typecheck, layering, dead-code audit, build, 4,834 related tests in 628 files, and released-surface wire compatibility. The first related run exposed a host
AWS_REGION=ap-southeast-1assumption in the unchanged AWS profile test; the isolated test passed with AWS region overrides unset, then the complete affected gate passed under the same clean environment. No test skipped or unrelated AWS code changed.Freshly rebased onto upstream main; preserved the existing changelog entries and wire ledger. Response normalization and script roundtrip regressions were observed failing before the fix. The proxy route test exposed missing serial propagation before that was corrected. Removing HarmonyOS from the shared runtime vocabulary makes the schema/open normalization regression fail.
Tests cover mocked proxy inventory through backend inference, command materialization, real LeaseRegistry admission and release, plus leaf/non-leaf runtime compatibility. These are not live-device receipts.
Live HarmonyOS proxy inventory → open → snapshot/selector click/logs → close passed on API23 emulator at this exact head on 2026-09-08. Separate client/proxy processes used real loopback HTTP, a scoped
harmonyos-instancelease, live Settings navigation, nonempty app logs and screenshot; post-close heartbeat rejected the inactive lease. Detailed receipt is in the PR comments. This does not cover a Linux client, cross-host tunnel, text entry, Metro, or physical devices. License acceptance was authorized and completed. Exact-head upstream CI is stillaction_requiredpending maintainer approval; the PR is not yet merge-ready.CLI lease grammar, runtime guidance, and Unreleased changelog are updated; no skills changed. The scope crosses the runtime-platform contract, its CLI/Node/daemon/script consumers, and focused tests as required by the end-to-end issue.