Skip to content

chore(gates): confine process globals to declared entry points - #3823

Merged
thymikee merged 1 commit into
chore/process-globals-commandsfrom
chore/process-globals-gate
Oct 19, 2026
Merged

thymikee merged 1 commit into
chore/process-globals-commandsfrom
chore/process-globals-gate

Conversation

@thymikee

@thymikee thymikee commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Summary

Part of #3753. Part of #3747. Closes #3753.

Top of #3826 → #3827 → #3828 → #3829 → #3823. This PR retains the gate history; its own diff is three files, 404 gross lines.

R86 rejects any process value capture outside declared entries, including calls, aliases, spreads, returns, imports, and destructuring. Nonconfiguration members and lexical shadowing remain permitted. Remove the two Apple library entries and the registered-test cast.

Released captures are centrally declared for #3759: src/io.ts, src/sdk/{remote-config,android-adb,metro}.ts, src/provider-limrun-runtime.ts, src/mcp/command-tools.ts, and src/daemon-client/daemon-client{,-ensure,-ensuring}.ts. The future agent-device/host slot remains reserved for #3757. Executable/worker entries and test-support classification remain explicit.

Validation

Head 6aefc10590621fe4672b6f3a0aa30e57a79d4796.

pnpm check:affected --run --base chore/process-globals-commands --head 6aefc10590621fe4672b6f3a0aa30e57a79d4796 passed; receipt run-g1AWGp, exactCommit=true.

Coverage failures were fixture races: 7e2cae20ed replaces a directory-order assumption with explicit files; 288e6cceae waits for bridge evidence contents rather than file creation. Both fixes are inherited by all layers. Upper patches are unchanged by range-diff.

Prior full-check and Android daemon/provider evidence stays attributed to e223d37244 in #3823’s review-response comment. This round changes only test fixtures and ancestry. Full coverage and GitHub-owned lanes remain CI obligations.

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.44 MB 5.44 MB 0 B
Package (unpacked) 5.44 MB 5.44 MB 0 B
Package (download) 1.64 MB 1.64 MB -6 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 37.3 ms 38.3 ms +1.0 ms
CLI --help 73.7 ms 74.7 ms +1.0 ms

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed 97a0ad2. The R86 gate has a gap that lets the same process reads through, so this needs one more change.

The rule at https://github.com/callstack/agent-device/blob/97a0ad2/scripts/layering/process-globals-policy.ts#L80 flags process only as the object of a member access, in destructuring, or in an import. A bare process passed as a value is never checked. So captureHostEnvironment(process).env.FOO, someFn(process) and const p = process; p.env all pass in any module, including packages/daemon. At this commit, 11 non-entry production files capture the process this way: src/io.ts:14, src/sdk/remote-config.ts:16, src/sdk/android-adb.ts:19 and :26, src/sdk/metro.ts:19, src/provider-limrun-runtime.ts:30, src/mcp/command-tools.ts:185, and src/daemon-client/daemon-client{,-ensure,-ensuring}.ts. Five of these are new in this PR and replace the process.env and cwd reads it removed. This means the issue's done-when does not hold yet, and the spelling works as an unlisted allowlist. The rule should be: a process capture (any value use of the process binding, other than a non-configuration member such as .platform, .pid or .stdout) happens only in a file listed in PROCESS_ENTRIES. Please make R86 flag process used as a call argument, initializer, spread or return value outside entries. Then either add the released SDK capture sites (src/io.ts, src/sdk/.ts, src/provider-limrun-runtime.ts, src/daemon-client/, src/mcp/command-tools.ts) to PROCESS_ENTRIES next to the reserved agent-device/host slot, or move their capture to a caller that is already an entry. Please add self-tests for captureHostEnvironment(process) and const p = process; p.env. I read the AST visitor to find this. I did not run the gate or its self-tests.

The Android adb path also needs live proof. src/platform-runtime-android-adb-host.ts now takes the adb env from commandHostEnvironment(), which throws host_environment_unscoped outside a scope, where it used to read process.env. Please show a green Android leg of CI Smoke Tests, or a local Android run through the daemon (open, snapshot, logs start/stop with logcat, close) whose output and daemon log contain no host_environment_unscoped. The Limrun or provider path (createLimrunRuntimeDependencies(host)) also needs one run showing adb commands succeed. I did not check every public android-adb SDK export that could reach requireAndroidAdbHost().environment outside a command-host scope, for example the platform-android sdk.ts default env.

The threading itself looks right, and SessionStore.host gives the daemon side one source. Would it be simpler to keep the one declaration as the full list of process captures, with R86 treating a value use of process as a capture and the released SDK defaults listed as declared entries beside the reserved agent-device/host slot? Then #3759 could delete them from that one list when it removes the released defaults, and nothing else needs to change first for the gate fix. Fully removing the SDK capture sites depends on #3757 and #3759.

Not blocking, and you can take or leave these: PROCESS_ENTRIES lists packages/platform-apple/src/runner/runner-process-launch.ts and packages/platform-apple/src/os/macos/helper.ts, but both are library modules the daemon calls with no process global today, so listing only real executables and worker or child entry files would keep the list honest; the diff is about 3,800 lines across 230 files, above the 2,500-line limit in docs/agents/pull-requests.md, and the five commits could land as a stack (daemon, replay, runtime, commands, then the gate); the registered-gate test feeds its input through an as Parameters<...>[0] cast; and process.exitCode is not in PROCESS_MEMBERS although companion-tunnel.ts sets it, which is fine since the issue does not list it.

CI is still pending. Smoke Tests is running, and its route overlaps this diff. Before merge, R86 must count a value use of process as a capture and declare the released SDK sites, and the Android smoke leg must pass.

@thymikee

thymikee commented Oct 19, 2026 •

Copy link
Copy Markdown
Member Author

Addressed the review of 97a0ad2791. Rebased onto a661e43d83, including #3818 and #3820. Range-diff preserves all four cleanup commits; this round changes the gate and adds one fixture correction. Independent review of e223d37244b29277b5e92db68e1f9e75c5010e2f has no remaining findings.

Review mapping

  1. R86 capture gap: e223d37244 makes bare process values captures, including calls, initializers, spreads, returns, dynamic members, imported aliases/namespace defaults, and declaration/assignment destructuring. It respects lexical bindings. The requested captureHostEnvironment(process) and const p = process; p.env regressions fail outside entries. Released SDK captures are all in PROCESS_ENTRIES, beside the reserved feat(host): startAgentDevice() for embedder-owned daemons #3757 host slot; 0.22: remove superseded plugin, client and environment APIs #3759 removes them later. Ten self-tests pass.
  2. Android scope proof: runtime layer refactor(runtime): use owning hosts at process and adb boundaries #3828, commit 8c5c1aa171, retains explicit scopes for local adb dispatch and createLimrunRuntimeDependencies(host). Final-head live evidence is below. Every released android-adb export was audited: the standalone shell/exec-out helpers establish scope; executor-based helpers use the supplied executor. The private platform SDK defaults are reached through scoped runtime mechanics; toolchain preparation supplies env explicitly. No additional SDK scope fix was needed.
  3. Recommended changes: (a) removed the Apple helper/runner library entries in e223d37244; neither contains a process capture; (b) split into the five independently validated PRs below, with chore(gates): confine process globals to declared entry points #3823 retained as the top gate layer; (c) replaced the registered-test cast with typed LayeringContext input in e223d37244; (d) left optional process.exitCode unchanged because other files also set it, including ios-snapshot-engine/replay.ts, the Maestro child, and the CLI update entry.

The Android build exposed an existing TV fixture defect: the fake adb returned an empty IME list after the bundled APK became available. The same focused command timed out at 5,000 ms on unchanged base a661e43d83 and original daemon head ae8c75ebcf. Commit e16dd8a969 sets testIme: false for this D-pad-only scenario; the same test passes in 911 ms. It changes no timeout or production readiness invariant.

Stack and exact-head receipts

Native GitHub stack #3830, bottom to top. Each body contains “Part of #3753” and “Part of #3747”; only #3823 closes #3753. Every push used force-with-lease (including gh stack push).

All five receipts have status=passed, exactCommit=true. The daemon compares with a661e43d83; upper layers compare with their declared parent branch. Commands were foreground pnpm check:affected --run --head <SHA> plus --base <parent> for upper layers. Related tests passed: daemon 2,577; replay 6,104; runtime 992; commands 2,147. The gate layer runs the R86/layering self-tests and selected repository checks.

The cumulative final head also passed pnpm check --head e223d37244b29277b5e92db68e1f9e75c5010e2f: receipt run-WrQdpI, exactCommit=true, all three stages passed. Full unit suite: 15,147 passed, one skipped, 1,776 files passed. Local smoke: 73 passed, five skipped. The clean package install imported all 15 released entry points.

Final-head Android live proof

Source e223d37244b29277b5e92db68e1f9e75c5010e2f. Built with pnpm build and pnpm build:android. Dedicated Android 15 AVD R86_3753, serial emulator-5580; isolated state /private/tmp/agent-device-r86-3753, session r86-3753. A fresh source-built daemon exercised this checkout.

r86_repo=/Users/michal/.t3/worktrees/agent-device/chore-process-globals-gate
r86_state=/private/tmp/agent-device-r86-3753
node "$r86_repo/bin/agent-device.mjs" open com.android.settings --platform android --serial emulator-5580 --session r86-3753 --state-dir "$r86_state" --json
node "$r86_repo/bin/agent-device.mjs" snapshot -i --platform android --serial emulator-5580 --session r86-3753 --state-dir "$r86_state" --json
node "$r86_repo/bin/agent-device.mjs" logs start --platform android --serial emulator-5580 --session r86-3753 --state-dir "$r86_state" --json
node "$r86_repo/bin/agent-device.mjs" logs stop --platform android --serial emulator-5580 --session r86-3753 --state-dir "$r86_state" --json
node "$r86_repo/bin/agent-device.mjs" close --platform android --serial emulator-5580 --session r86-3753 --state-dir "$r86_state" --json

Every command: exit 0, success: true. Snapshot excerpt:

{"backend":"android-helper","helperVersion":"0.21.29-dev","helperTransport":"persistent-session","nodeCount":164}

While logs were active, the owned process was exactly:

adb -s emulator-5580 logcat -v time --pid 2714

app.log contained Android logcat records (--------- beginning of main and Settings PID 2714 records). Start returned started: true; stop returned stopped: true; close returned Closed: r86-3753.

The provider path used the exact factory with real local adb, without cloud allocation or credentials:

import { captureHostEnvironment } from '/Users/michal/.t3/worktrees/agent-device/chore-process-globals-gate/packages/host-kit/src/process.ts';
import { createLimrunRuntimeDependencies } from '/Users/michal/.t3/worktrees/agent-device/chore-process-globals-gate/src/sdk/limrun-runtime-dependencies.ts';
const deps = createLimrunRuntimeDependencies(captureHostEnvironment(process));
for (const invocation of [
  deps.android.hostAdbInvocation(['version']),
  deps.android.deviceAdbInvocation('emulator-5580', ['get-state']),
]) {
  const result = await deps.host.runAdb(invocation);
  console.log(result.exitCode, result.stdout.trim());
}

Run with node --experimental-strip-types. Output excerpts:

provider host.runAdb version 0 Android Debug Bridge version 1.0.41
Version 37.0.1-15733141
provider host.runAdb -s emulator-5580 get-state 0 device
public sdk runAndroidShell 0 uid=2000(shell) ...
public sdk runAndroidExecOut 0 uid=2000(shell) ...
provider apps 18
provider foreground { appId: 'com.android.settings', activity: 'com.android.settings.Settings' }
provider logcat bytes 794
public SDK port reverse mappings 0

Provider app discovery, keyboard state, foreground lookup, logcat reads, and standalone public shell/exec-out calls all succeeded. Command outputs and the daemon log had zero host_environment_unscoped matches. The session and daemon were closed; the dedicated emulator was shut down and deleted. Other devices/sessions were left alone.

Full coverage and GitHub-authoritative native/device/provider lanes remain CI obligations. No remaining implementation objection; no merge or ready-for-human change.

@thymikee

thymikee commented Oct 19, 2026 •

Copy link
Copy Markdown
Member Author

Re-ran the failed Apple job once (run 39275119905): RunnerTests.testAlertActivationDoesNotConsultInterruptionHandlingWithANotificationBanner failed at RunnerTests+AlertObservationTests.swift:229 because the banner fixture lacked notification authorization; the same log shows XCTest tree captures exceeding their 8 s slice. 97a0ad2 changes no Swift files, so this is a suspected runner/fixture flake, pending rerun evidence.

@thymikee
thymikee force-pushed the chore/process-globals-gate branch from e223d37 to 95c290d Compare October 19, 2026 16:55
@thymikee

Copy link
Copy Markdown
Member Author

Coverage follow-up: 7e2cae20ed fixes the daemon progress fixture by passing explicit files; the failing assertion assumed filesystem directory order. Restacked this unchanged gate patch to 95c290d831; range-diff is identical. Foreground check:affected passed at 95c290d831 (run-FBJZNT, exactCommit=true). CI is running on the pushed head.

…3753)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee
thymikee force-pushed the chore/process-globals-gate branch from 95c290d to 6aefc10 Compare October 19, 2026 17:25
@thymikee

Copy link
Copy Markdown
Member Author

Second coverage fix: 288e6cceae polls the detached bridge’s expected cwd contents; the shell creates the evidence file before writing it, and CI read an empty file (run 39289751890). Head 6aefc10590 inherits both this fix and 7e2cae20ed; upper-layer range-diff is unchanged. Foreground check:affected passed (run-g1AWGp, exactCommit=true). Cold self-review found no issue in the test-only diff. CI is running on this head.

@thymikee

thymikee commented Oct 19, 2026 •

Copy link
Copy Markdown
Member Author

Android smoke rerun passed (run 39294052369, attempt 2, final head 6aefc10). live Android emulator fixture E2E originally hit android-app-close-timeout (process, 2 s); boot logged repeated adb device offline, while the failure artifact already showed no target pid, launcher resumed, and no crash. One rerun only; no timeout or production change.

@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready at 6aefc10. The gate now confines process globals to the declared entry points, and the earlier evidence request no longer applies: this PR is gate-only tooling, so it needs no live run. That request now applies to the stacked base PRs (#3826-#3829) that carry the commandHostEnvironment() threading.

Not blocking, and you can take it or leave it: PROCESS_ENTRIES in process-globals-policy.ts lists packages/capture-kit/src/png-worker.ts, but that file has no process reference. The entry is an unused allowlist slot, and the entry test at process-globals-policy.test.ts:57 keeps it in place. Removing it from both places would keep the allowlist exact.

I did not run the gate or its self-tests at this head. Whether the whole tree passes rests on your affected-checks run and the CI job. Repo Guards & Compatibility, which runs the layering gate, is still pending. It must pass on 6aefc10. The diff touches only scripts/layering, so the Smoke, Integration and device routes are not affected. The stacked base PRs must also land first. There are no conflicts.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 19, 2026
@thymikee
thymikee merged commit f8eddcb into main Oct 19, 2026
22 of 30 checks passed
@thymikee
thymikee deleted the chore/process-globals-gate branch October 19, 2026 17:43
@github-actions

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

thymikee added a commit to jipeng6036-del/agent-device that referenced this pull request Oct 20, 2026
* origin/main: (28 commits)
  feat(gesture): add gesture tap with 2 to 5 simultaneous contacts (callstack#3847)
  chore: mark 0.21.29 as released
  chore(diagnostics): trace Apple prepare phases and subprocess lifecycle (callstack#3846)
  feat(sdk): start an embedded runtime from the main entry (callstack#3843)
  fix(alert): align local Apple and Limrun button choices (callstack#3825)
  docs(adr): drop the withdrawn host allocator design from ADR 0021 (callstack#3845)
  feat(screenshot): support selector crops on CoreDevice iPhones (callstack#3839)
  feat(scroll): support explicit budgets for long target searches (callstack#3837)
  fix(apple): key the toolchain fingerprint memo on the resolved xcode-select dir (callstack#3726)
  feat(android): hand rotation back to the sensor with orientation auto (callstack#3822)
  fix(sdk): scope released entries and deferred provider calls (callstack#3833)
  test(android): prepare the snapshot helper before landmark waits (callstack#3834)
  perf(ci): overlap iOS simulator boot with the native build (callstack#3836)
  docs(ios): record SpringBoard sessions as verified on a physical iPhone (callstack#3840)
  feat(apple): admit two-finger gestures on physical iOS devices (callstack#3838)
  perf(android): bound target readiness and confirm moved clears (callstack#3815)
  perf(android): reuse live helper for input and capture (callstack#3811)
  chore(gates): forbid process globals outside declared entry points (callstack#3753) (callstack#3823)
  refactor(commands): resolve command inputs and output from the caller host (callstack#3753) (callstack#3829)
  refactor(runtime): use the owning host environment at process boundaries (callstack#3753) (callstack#3828)
  ...
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.

chore(layering): forbid process globals outside entry points

1 participant