Skip to content

refactor(host): read host settings from HostEnvironment in root src and remaining packages - #3775

Merged
thymikee merged 9 commits into
refactor/host-environmentfrom
refactor/host-environment-root
Oct 17, 2026
Merged

thymikee merged 9 commits into
refactor/host-environmentfrom
refactor/host-environment-root

Conversation

@thymikee

@thymikee thymikee commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

Summary

Part of #3751 (umbrella #3747). Configuration reads in root src/**, packages/remote and provider-webdriver take the host explicitly instead of reading process.env/process.cwd(): CLI command handlers (new host param, set once per run in cli.ts), option/binding/auth resolvers, the connection profile chain, remote artifact preparation and materialization (threaded through the daemon-client transport), and the platform runtime gateway, inventory gateways, diagnostics, toolchain and deployment hosts. Released entries keep their process default in a wrapper: resolveRemoteConfigProfile, createLocalArtifactAdapter.

Behavior note: a relative --provider-app path on a request with no cwd now fails with provider_app_cwd_missing; the base fills the lease cwd from the host.

Remaining, handed to owners

  1. feat(daemon): the runtime owns its state and never the process #3772: session-journal cap, device-selection env, replay-port/maestro/ad-script replay env and cwd.
  2. refactor(platforms): read host settings from HostEnvironment in platform and capture packages #3750 / refactor(platforms): read host settings from HostEnvironment in platform and capture packages #3777: adb port environment (platform-runtime-android-adb-host.ts), app-log output and process env, platform-* packages.
  3. Plugin v2 stack: provider-device-runtimes.ts, provider-credential-fingerprint.ts, cli/connection/provider-policy.ts, plugins/*.
  4. feat(host): capture HostEnvironment once at entry points #3763: update-check.ts, replay-test reporters and progress, color.ts, diff.ts, commands/replay/index.ts, doctorCliOutput, plugins.ts, cli-config.ts (the MCP caller must pass env first).

Validation

check:affected --run green at 8eff3d9ad6, rebased on 916db0b. Root src keeps 5 commandHostEnvironment() reads (4 command execution, 1 doctor output colour); 23 branch-wide against 56 on the base. Live run at an earlier head: real daemon, macOS open/snapshot/close, plugins list, Android doctor.

@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.38 MB 5.38 MB -79 B
Package (unpacked) 5.38 MB 5.38 MB -79 B
Package (download) 1.62 MB 1.62 MB +208 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 29.7 ms 28.3 ms -1.4 ms
CLI --help 55.9 ms 53.2 ms -2.7 ms

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 58 files

You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread test/wire-compat/ledger.json Outdated
Comment thread src/cli/replay-test/reporters/custom.ts Outdated
Comment thread test/wire-compat/ledger.json Outdated
Comment thread src/sdk/remote-config.ts
@thymikee
thymikee force-pushed the refactor/host-environment-root branch from c3fe1a5 to 0b6ab69 Compare October 17, 2026 06:34
@thymikee

Copy link
Copy Markdown
Member Author

The code looks good at 0b6ab69, but I cannot call it done yet because live evidence is still missing. I did not run tests. This read relies on the green checks (12 reported, none failing) and on tracing the code. I did not check out-of-tree plugin hosts that call provider-webdriver helpers directly. I also did not confirm that the AsyncLocalStorage scope reaches every long-lived daemon timer or stream created before the scope that startDaemonRuntime opens. There are no conflicts. No finding blocks the merge. Please decide who owns the provider hub-app cwd default (first note below), and Cubic has not reviewed this head yet.

Not blocking, take or leave: (1) BrowserStack (browserstack.ts:174) falls back to commandHostEnvironment().cwd for the hub-app cwd, but TestMu goes through the plugin.ts wrapper (https://github.com/callstack/agent-device/blob/0b6ab69/packages/provider-webdriver/src/plugin.ts#L40), which falls back to process.cwd(). (2) src/io.ts:17 adds createLocalArtifactAdapterAt, and both callers pass cwd explicitly, which createLocalArtifactAdapter already accepts. (3) The custom.test.ts:142 test binds { env: process.env } and then changes process.env.HOME. The old os.homedir() reads HOME too, so the test passes on the old code. Use withCommandHost with { env: { HOME: home }, cwd } and leave process.env.HOME alone, so the old code fails. (4) daemon-composition.ts:91 holds env but createHostDiagnostics reads commandHostEnvironment() ambiently, and the PR body's Remaining list omits the env defaults in plugins/connection.ts, plugins/store.ts, cli/connection/provider-policy.ts and provider-device-runtimes.ts.

Could the ambient-host pattern stay and still be simpler? Fill provider request.cwd once from the bound host, or in one helper that both hub providers call. Then resolveHubAppReference takes a required cwd and the plugin.ts wrapper goes away. Use createLocalArtifactAdapter({ cwd }) instead of a new At variant. Pass the env the composition already holds into createHostDiagnostics. Nothing has to change first, because provider-webdriver is private and its plugin subpath has no external compatibility need.

Of the four Cubic threads, all four are fixed at this head, so please resolve them: #3775 (comment) (ledger now changes only the resolveMaterializedArtifactPath entry), #3775 (comment) (custom.ts uses resolveUserPath with the host env; the test gap is note 3 above), #3775 (comment) (ledger rationale names the cwd fallback), #3775 (comment) (remote-config.test.ts asserts the released entry reads process.env).

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 13 files (changes from recent commits).

You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread packages/provider-webdriver/src/webdriver-utils.ts Outdated
@thymikee

Copy link
Copy Markdown
Member Author

The earlier review at 0b6ab69 asked for a live run. At 6a0fb60 the code looks fine, but there is still no live run, and the PR has merge conflicts with main.

Please rebase onto refactor/host-environment and resolve the conflicts. I did not check the conflict content.

Fallow audit is failing, and it looks unrelated to this change. The waitForDaemonStartup finding in daemon-client-lifecycle.ts comes from a stale merge base, because this PR has no diff in that file, and a rebase should clear it. The unused platformRuntimeModules export in https://github.com/callstack/agent-device/blob/6a0fb60/src/platform-runtime.ts#L126 already existed before this PR and is only used by a test fixture. Please remove the export or reroute the fixture, not add an allowlist. The Smoke Tests jobs were still queued.

I still need one live daemon run at this head. (a) Run agent-device devices --platform android and doctor --platform android with ANDROID_HOME or ANDROID_SDK_ROOT set only in the env the daemon started with. The device list and the toolchain check must name that SDK path. (b) Run a BrowserStack or TestMu --provider-app ./relative.apk from a CLI cwd that differs from the daemon's cwd. The upload must resolve the path against the client host cwd, not the daemon process cwd. (c) Run diff screenshot or record with a relative output path. The file must land under the CLI cwd. I did not run tests or the fallow audit myself.

The Cubic thread on resolveHubAppReference still applies: #3775 (comment). Without cwd outside a host scope, webdriver-utils.ts now throws host_environment_unscoped, and that entry has shipped since v0.21.21. Please accept the break with a CHANGELOG line, or restore the fallback.

Not blocking: the same break also reaches out-of-tree callers of agent-device/plugins/webdriver, which the old process.cwd() fallback served. You can take it or leave it.

@thymikee
thymikee force-pushed the refactor/host-environment-root branch 2 times, most recently from 49a6474 to 8278b13 Compare October 17, 2026 10:17
@thymikee

Copy link
Copy Markdown
Member Author

Review round at 8278b13565 (rebased on 225e0bd).

  • Hub-app cwd (note 1): resolveHubAppReference no longer reads a process or ambient directory, and the plugin.ts wrapper and BrowserStack's fallback are gone. BrowserStack and TestMu both go through it, so they agree: a relative app path needs request.cwd, otherwise a typed provider_app_cwd_missing INVALID_ARGS. The daemon fills request.cwd from the client's meta; filling it from the host when absent is a one-line change in packages/daemon/src/sessions/handlers/lease.ts:332, handed to feat(daemon): the runtime owns its state and never the process #3772. The 0.21.28 agent-device/plugins/webdriver entry changes for a caller that omitted cwd.
  • Note 2: createLocalArtifactAdapterAt is gone; createLocalArtifactAdapter is untouched.
  • Note 3: the ~/ reporter test now runs under withCommandHost({ HOME }) without touching process.env; custom reporter path expansion in this PR was dropped with the ambient reads (reporters are in the handed-off list).
  • Note 4: createHostDiagnostics(env) takes the env the composition holds; the remaining env defaults are listed in the PR body.
  • Design correction: configuration reads take the host explicitly (remote artifacts, platform runtime gateway/inventory/diagnostics/toolchain/deployment, CLI option/binding/auth, connection profiles). Non-test commandHostEnvironment() sites outside command execution: 115 before, 82 now (base 81; the +1 is two reads I added next to sibling reads in auth.ts and connection.ts, minus one removed).
  • Live evidence at 8278b13565: a real daemon on an isolated state dir: plugins list with the host's AGENT_DEVICE_HOME, Android doctor, macOS open com.apple.finder / snapshot -i / close, all successful, daemon stopped cleanly. Not covered: iOS simulator and Android snapshot helper (no free device, helper not built), and ALS scope reaching timers created before startDaemonRuntime; that is feat(daemon): the runtime owns its state and never the process #3772's.

@thymikee

Copy link
Copy Markdown
Member Author

Thanks for the update. The cwd fallback change and the ledger fixes at 8278b13 look good, but one regression remains in the Node client, and CI is green (12 checks, none failing).

Fixed since the earlier review: the ledger rationales now name the host cwd, the remote-config test covers the process.env default, and the reporter change is out of the diff.

Still open: commit 2ad2ed7 replaced the cwd fallback in resolveHubAppReference with a provider_app_cwd_missing refusal (webdriver-utils.ts#L181). The daemon passes only req.meta.cwd (lease.ts#L332). The Node client builds meta.cwd from options.cwd and config.cwd only, even though it already holds host (agent-device-client.ts#L89). A Node SDK call to client.leases.allocate or open with provider browserstack or testmu, a relative providerApp such as ./app.apk, and no cwd used to resolve against the daemon's cwd. It now fails with INVALID_ARGS provider_app_cwd_missing. The CLI and MCP always send cwd, so only the Node surface regresses. The PR body says the daemon fills request.cwd from the client, which does not hold for this surface. The rule: every client surface that holds a HostEnvironment names that host's cwd in request meta, so a relative client path never reaches the daemon without its directory. The owner is the Node client's request construction, so please set cwd: merged.cwd ?? host.cwd in execute, as the CLI and MCP do. Please add a test that drives client.leases.allocate through a capturing transport and asserts meta.cwd equals the injected host cwd. Please also add a CHANGELOG Unreleased line: out-of-tree callers of agent-device/plugins/webdriver resolveHubAppReference that omit cwd now get provider_app_cwd_missing.

Not blocking, and you can take or leave it: createPlatformModuleRegistryForHost in platform-runtime.ts#L85 is exported only for its test, the same shape as the earlier platformRuntimeModules ask, so keep it module-local and test through createPlatformDeviceInventoryGateways(env), or accept it as the test seam.

I read the code at 8278b13. I did not run tests, typecheck or the live daemon, and I know the author's live runs only from the comment, which has no transcript. I relaxed two earlier asks after reading the code, because the daemon env and the remote-artifact host.cwd fallback keep their old behavior. I did not live-verify a relative BrowserStack or TestMu app path, and I did not confirm how the build maps the root ./remote-config export.

On the other threads: the provider_app_cwd_missing break still applies and is the finding above. These are fixed or no longer apply, so please resolve them: ledger digests (fixed), custom reporter HOME (out of the diff, handed to #3763), materializeRemoteArtifacts rationale (fixed), remote-config env test (fixed).

Before merge, the Node client must send meta.cwd from its host with that test, and the CHANGELOG needs the new line.

@thymikee
thymikee force-pushed the refactor/host-environment-root branch from 8278b13 to 8eff3d9 Compare October 17, 2026 11:39
@thymikee

Copy link
Copy Markdown
Member Author

Applied every reviewer recommendation at 8eff3d9ad6 (rebased on 916db0b, gate green).

  • Hub-app cwd, one place: both hub providers resolve the app through resolveHubAppReference; no wrapper, no process or ambient fallback. A relative path with no request.cwd fails with typed provider_app_cwd_missing. The base's own change now fills the lease cwd from the bound host, so I dropped my duplicate daemon edit.
  • createLocalArtifactAdapterAt: removed; the CLI passes cwd: host.cwd to createLocalArtifactAdapter.
  • Reporter test (note 3): the reporter change it covered is no longer in this PR, so there is no test to fix; the reporters are in the handoff list.
  • createHostDiagnostics(env): takes the env the composition holds; remaining env defaults are listed in the body.
  • Narrowing commandHostEnvironment(): CLI handlers now receive host, so connection, daemon, device, auth, recording, replay, screenshot, proxy, ended-lease and connection-runtime read it explicitly. Root src has 5 sites left (4 command execution in the platform hosts, 1 doctor output colour); branch-wide 23 against 56 on the base, the rest in platform-* (refactor(platforms): read host settings from HostEnvironment in platform and capture packages #3750).
  • Stray export: platformRuntimeModules is private.
  • Test helpers: host-taking commands are bound once in test-utils/host-bound-commands.ts, which keeps the large suites under the size ratchet.

@thymikee

Copy link
Copy Markdown
Member Author

The earlier findings at 6a0fb60 are fixed at 8eff3d9, and no code problem remains. The ledger.json rationales now match the changed declarations, and remote-config.test.ts now covers the process.env default of the released wrapper. In-tree provider routes are also fixed, because lease.ts always fills cwd.

One ask remains before merge. resolveHubAppReference is re-exported from plugin.ts and ships as agent-device/plugins/webdriver. An out-of-tree plugin that passes a relative app and no cwd used to resolve against process.cwd(). At webdriver-utils.ts:168 it now fails with INVALID_ARGS, details.reason: provider_app_cwd_missing. Please add one CHANGELOG Unreleased line saying resolveHubAppReference now requires cwd for a relative app path and otherwise fails with that error. Both earlier reviews asked for this entry.

Not blocking, and you can take or leave it: the title of commit eef3345 says "fill the lease cwd from the host", but that commit has no lease.ts change, so you could reword it when squashing.

All 12 checks pass at 8eff3d9, and there are no conflicts. I read the code at this commit only. I did not run tests, typecheck, fallow, or a live daemon. Your live run (Android doctor, plugins list, macOS open/snapshot/close) was at 8278b13. The five commits are identical at this head, so I count it as covering this commit. A relative BrowserStack or TestMu --provider-app upload has not been run live.

The inline thread on webdriver-utils.ts:168 still stands: #3775 (comment). The CHANGELOG line closes it. These threads no longer apply, so please resolve them: the ledger.json rationales are rewritten (#3775 (comment)), custom.ts is no longer in the diff and its ~/ expansion is handed to #3763 (#3775 (comment)), the resolveMaterializedArtifactPath rationale now names the host cwd (#3775 (comment)), and the process.env default test is added (#3775 (comment)).

@thymikee

Copy link
Copy Markdown
Member Author

Applied the 12:36Z review at ae8f733bcd (on f744554, gate green):

  • CHANGELOG: an Unreleased line records that resolveHubAppReference (via agent-device/plugins/webdriver) now needs cwd for a relative app path and otherwise fails with INVALID_ARGS, details.reason: provider_app_cwd_missing; it used to resolve against process.cwd().
  • Commit message: the CLI-handlers commit is reworded to "pass the run's host to CLI command handlers", since the lease cwd fill comes from the base.

The branch is based on the current refactor/host-environment. I will rebase once the plugin v2 merge lands on it, rerun the gate and push.

@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto refactor/host-environment c9aa2f2 (plugin contract v2 and the merge of main) at 0d8e1b5d8a. Two conflicts resolved: daemon-composition.ts keeps the composition's env going to the runtime gateway, inventory gateways and claim recovery inside the new registry try/catch, and cli/commands/daemon.ts takes the host from the handler params. Two new handler suites got a host. check:affected --run green on this head.

@thymikee
thymikee force-pushed the refactor/host-environment-root branch from ae8f733 to 0d8e1b5 Compare October 17, 2026 13:31
thymikee and others added 9 commits October 17, 2026 15:54
…ation and materialization

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…iagnostics, toolchain and deployment hosts

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…king directory

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… options, binding and auth

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…path

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@thymikee
thymikee force-pushed the refactor/host-environment-root branch from 0d8e1b5 to e248942 Compare October 17, 2026 14:03
@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready: at e248942 the earlier findings from 8eff3d9 are fixed and I found nothing new in the code. Smoke Tests are still queued or in progress with no failure so far, and the diff touches the route they run (CLI handler host threading, daemon-composition env wiring and daemonCommand), so a failure there would need attribution to this PR. I did not run tests, typecheck, fallow or check:affected, so the "gate green" claim is unverified. The live run (Android doctor, plugins list, macOS open/snapshot/close) was at 8278b13. The patch is the same at this head apart from conflict-context fixes in daemon-composition.ts and daemon.ts, so I count it as covering this head. I did not rerun daemon status or a v2-plugin connect with the threaded host after the rebase. I also did not exercise a relative BrowserStack or TestMu --provider-app upload live. No conflicts are known.

Not blocking: the new Unreleased line in CHANGELOG.md says resolveHubAppReference shipped through agent-device/plugins/webdriver, but that entry only exports the type WebDriverPluginOptions and @agent-device/provider-webdriver is private, so it was never a public export. You could describe the visible effect instead (a relative --provider-app on a lease request with no working directory, for example @agent-device/testmu on an older host, fails with provider_app_cwd_missing) or drop the line, and it is fine to take or leave. Base code that came in with the rebase (primeProviderConnection, isConnectProviderName, connectProviderNamesForError and plugins.ts) still defaults to process.env. On the CLI that matches the captured host env, so I left it to the #3750/#3751 follow-up.

On the open inline threads, none of the P1/P2 threads still apply and no lower-priority threads remain. These can be resolved: #3775 (comment) (the CHANGELOG now records the cwd requirement and the provider_app_cwd_missing error), #3775 (comment) (the ledger rationales were rewritten at 8eff3d9), #3775 (comment) (custom.ts is not in this diff, and the ~/ expansion goes to #3763), #3775 (comment) (the rationale names the host cwd since 8eff3d9) and #3775 (comment) (remote-config.test.ts covers the process.env default).

Merge once Smoke Tests finish green.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 17, 2026
@thymikee
thymikee merged commit 597f793 into refactor/host-environment Oct 17, 2026
12 of 13 checks passed
@thymikee
thymikee deleted the refactor/host-environment-root branch October 17, 2026 14:51
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-17 14:51 UTC

thymikee added a commit that referenced this pull request Oct 17, 2026
…bals

* commit '870cf663c4':
  fix: align replay, SDK, and session boundaries with ADRs (#3762)
  refactor(host): read host settings from HostEnvironment in root src and remaining packages (#3775)
  fix(daemon-client): document the drain timeout and tighten ensureDaemon follow-ups (#3783)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant