Skip to content

feat(computer-use): add platform binding selection with typed unavailable results - #5306

Open
wy471x wants to merge 6 commits into
apache:mainfrom
wy471x:feat_create-platform-abstraction-layer-for-cross-platform-backends-
Open

wy471x wants to merge 6 commits into
apache:mainfrom
wy471x:feat_create-platform-abstraction-layer-for-cross-platform-backends-

Conversation

@wy471x

@wy471x wy471x commented Sep 14, 2026

Copy link
Copy Markdown

Summary

Computer Use named its executor with a hardcoded process.platform !== 'darwin'
check, so three different outcomes — unsupported platform, missing executor,
failed backend — collapsed into the same untagged none, and adding a platform
could only mean copying the supervisor. This makes the platform decision an
explicit binding table and makes every "no backend" result typed.

  • CU_PLATFORM_BACKEND_BINDINGS (today the single darwin → maka-cu row) is the
    one place a platform names its executor; selectComputerUseBackend consults it
    instead of testing process.platform inline.
  • SelectedComputerUseBackend now carries unavailableReason
    (unsupported_platform | missing_executable | backend_failed), so a
    capability surface can say why Computer Use is off instead of guessing.
  • MakaCuSelection and createComputerUseHost take a platform seam that
    defaults to process.platform. The Darwin selection assertions that used to
    skip themselves off-macOS (if (process.platform !== 'darwin') return;) now
    run on every CI OS.
  • MakaCuService remains the only supervised-child lifecycle for every
    maka.cu/2 executor. The unused childArgs seam is dropped — nothing could
    reach it (MakaCuBackendOptions never declared the field and
    createMakaCuBackend never forwarded it) — and the header comment now states
    that platform policy belongs to the backend and the Desktop composition, never
    to the process lifecycle.

macOS behaviour is unchanged: darwin still selects maka-cu with the same
tools and the same binary/digest checks. Non-Darwin previously returned an
untagged none; it now returns none plus
unavailableReason: 'unsupported_platform'.

Refs #3896

Review focus

The issue proposes a ComputerUseBackend interface with per-OS implementations
and Linux/Windows stubs. This PR deliberately takes the other route: a backend
that silently no-ops is the failure mode selection exists to prevent, and all
three platforms share one maka.cu/2 contract and one supervisor, so their only
real divergence is which native binary the Desktop provisions. Adding a platform
is therefore a row in the binding table plus its native executor and Desktop
provenance work — not a second backend implementation. An unbound platform fails
closed with a typed reason instead. If the named interface is wanted anyway,
please say so on the issue and I will follow it.

Verification

Run on the merge commit 6ee1b186b (macOS 15 arm64, Node 22.23.2), from the
repository root:

  • npm run lint — pass (3591 files); npm run format:check — pass (2122 files)
  • npm run clean && npm run build — pass; npm run typecheck — pass for every workspace
  • packages/computer-use: 120 tests pass (node --test "dist/**/*.test.js")
  • apps/desktop: 2636 tests pass
    (node --test --test-force-exit "dist/main/**/*.test.js" scripts/dev-app-runtime.test.mjs scripts/vite-workspace-packages.test.mjs)
  • node scripts/ci-test-plan.mjs --base origin/main --head HEAD selects
    code=true, e2e=true and the packages/computer-use,apps/desktop
    workspaces for this diff; the other code=true gates pass as well —
    check:tui-copy, check:renderer-architecture --base,
    check:locale-hygiene --base, astryx:theme --check,
    astryx:surface-inventory, and knip for apps/desktop and packages/ui
  • Desktop Electron e2e: 37 passed, 1 failed
    (new-task-reload.spec.tssessions:archive reported a live derived effect
    after 38 windows had already run). That spec passes in isolation (2/2) and
    exercises no Computer Use path.

Not run: the Storybook lane (not selected by the planner), the packaged and
notarized Computer Use path against a real window, and check:asf-headers
locally it flags only the untracked .commandcode/ workspace directory, which is
not part of this branch.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Command Code wrote the platform binding selection, the typed
unavailable reasons, the platform seams and their tests; the author reviewed,
edited and verified the result. Both commits carry Generated-by: Command Code.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

…abstraction-layer-for-cross-platform-backends-
…vice

The option was declared on MakaCuServiceOptions but no caller could reach it. MakaCuBackendOptions never declared the field, and createMakaCuBackend never forwarded it, so neither the selector nor the Desktop composition could set it. That left an interface promise with a host default, no consumer, and no test able to tell a correct value from an empty argv or a doubled host argument.

host is still required and still hardcoded at the spawn site, with its rationale unchanged. The seam comes back when a second platform actually needs a different argv, driven by a real requirement.

Generated-by: Command Code
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 14, 2026

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technical conclusion: NO-GO due to one P2.

This change adds a darwin -> maka-cu platform binding, platform injection seams for cross-platform tests, typed unavailable results, and removes the unused child-argument seam. The fail-closed selector behavior works, but the new unavailable reason is dropped before Desktop capability reporting, so the user-facing distinction described by this PR is not implemented.

Validation completed on this exact head: build:test; full typecheck, lint, format, ASF-header, locale, TUI-copy, Astryx, renderer-architecture (112/112), and E2E-budget checks; Computer Use 120/120; Desktop 2492/2492; focused changed-path tests 67/67; Electron E2E 38/38; and git diff --check. The head cleanly merges with current main c22768c3b0dc47518f6f8584e864f86f0b1e5379. GitHub currently exposes only the successful label check, not a hosted test check.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

* platform" a typed fact a capability UI can distinguish from a missing
* executable or a construction failure instead of three flavours of `none`.
*/
unavailableReason?: 'unsupported_platform' | 'missing_executable' | 'backend_failed';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve this reason through the production capability snapshot. assembleDesktopNativeCapabilities() keeps the selected object, but runtime-host-boot.ts:1726-1734 forwards only backendId; then computerUseServiceHealth() maps every backendId === 'none' to cu_executor_undistributable. A compiled production-path probe produced the same projected health for unsupported_platform, missing_executable, and backend_failed. Consequently the Permission/Health UI still reports the integrity/distribution copy even on an unsupported OS, and cannot provide the distinction this field promises. Please thread the reason into the capability input/reason-code mapping and cover the snapshot or IPC output, not only the selector result.

@wy471x wy471x Sep 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 9db8cd7ca (fix) and de34dc1f2 (tests). The reason now survives the whole path:

  • computerUseServiceHealth() takes the selection's unavailableReason as a third argument and projects it instead of collapsing on backendId === 'none': unsupported_platform → the new cu_platform_unsupported code (copy added for zh-CN, zh-TW and en), backend_failedcu_backend_unavailable, missing_executablecu_executor_undistributable. A none carrying no typed reason keeps the code it had.
  • runtime-host-boot.ts forwards native.computerUse.unavailableReason into that call, so the capability input carries the distinction rather than only the selection.
  • Both capability layers now name that reason. The feature layer reported cu_artifact_missing for an unbound platform — the integrity/distribution copy you flagged — so it now names the same cause the probe does; cu_artifact_missing is left for a snapshot that never reached the capability at all.

On the coverage: asserting this inside capability-snapshot.ts is not possible from node --test, because that module imports Electron at module scope (importing it fails with Named export 'Notification' not found), so the row projection was moved to computer-use-capability-reasons.ts, outside that import graph. computer-use-capability-reasons.test.ts composes computerUseServiceHealth() with the row and pins unsupported_platform, missing_executable and backend_failed as three different codes, plus the live-executor and never-assembled cases; computer-use-host.test.ts additionally asserts that an unbound platform reaches the health projection as cu_platform_unsupported instead of an integrity failure.

Verified: @maka/core 842 tests, the affected Computer Use and capability-copy desktop tests, tsc (main plus every workspace), biome check, knip --workspace apps/desktop and check:locale-hygiene. I did not re-run the full apps/desktop suite end to end: on this machine node_modules has a react without an act export (Object.keys(require('react')) has no act), so 392 React-harness tests fail with act is not a function before reaching any Computer Use code — none of them import the modules touched here.

wy471x and others added 2 commits September 15, 2026 10:55
…ting

Selection grew a typed `unavailableReason` so a capability surface could say
why Computer Use is off, but no production path read it: the Desktop boot
forwarded only `backendId` to `computerUseServiceHealth()`, which mapped every
`none` to `cu_executor_undistributable`, and the capability's feature layer
reported `cu_artifact_missing` for the same case. An unbound platform therefore
reached the Permission/Health UI as a failed integrity check — the three states
the reason distinguishes arrived as one copy.

The reason now survives the whole path. `computerUseServiceHealth()` takes it as
a third argument and projects `unsupported_platform` to the new
`cu_platform_unsupported` code, `backend_failed` to `cu_backend_unavailable`,
and `missing_executable` to `cu_executor_undistributable`; a `none` carrying no
typed reason keeps the undistributable code it had. Both capability layers name
that same cause, so the feature row no longer contradicts the probe row.

The row projection lives in `computer-use-capability-reasons.ts` rather than in
`capability-snapshot.ts` because the snapshot module imports Electron, which a
`node --test` process cannot load; keeping the projection out of that import
graph is what makes it assertable, the same reason `app-icon-ipc` injects
Electron instead of importing it.

macOS behaviour is unchanged: `darwin` still selects `maka-cu` with the same
tools and the same binary/digest checks.

Generated-by: Command Code

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
…selection

The typed reason is only worth carrying if the projection keeps the three ways
to have no backend apart, so these tests assert what the capability surface
shows. The first pins the reason-code mapping, including that a `none` with no
typed reason keeps the undistributable code, and that a platform with no binding
reaches the health projection as a platform fact. The second composes that
projection with the row and asserts the three reasons arrive as three different
codes, with the live executor and the never-assembled snapshot keeping their own
artifact and probe reasons.

Generated-by: Command Code

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants