Repository navigation
chore(gates): confine process globals to declared entry points - #3823
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
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 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 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 CI is still pending. Smoke Tests is running, and its route overlaps this diff. Before merge, R86 must count a value use of |
97a0ad2 to
e223d37
Compare
|
Addressed the review of Review mapping
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 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
All five receipts have The cumulative final head also passed Final-head Android live proof Source 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" --jsonEvery command: exit 0, {"backend":"android-helper","helperVersion":"0.21.29-dev","helperTransport":"persistent-session","nodeCount":164}While logs were active, the owned process was exactly:
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 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 Full coverage and GitHub-authoritative native/device/provider lanes remain CI obligations. No remaining implementation objection; no merge or |
|
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. |
e223d37 to
95c290d
Compare
|
Coverage follow-up: |
…3753) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
95c290d to
6aefc10
Compare
|
Second coverage fix: |
|
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. |
|
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: 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 |
|
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, andsrc/daemon-client/daemon-client{,-ensure,-ensuring}.ts. The futureagent-device/hostslot 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 6aefc10590621fe4672b6f3a0aa30e57a79d4796passed; receiptrun-g1AWGp,exactCommit=true.Coverage failures were fixture races:
7e2cae20edreplaces a directory-order assumption with explicit files;288e6cceaewaits 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
e223d37244in #3823’s review-response comment. This round changes only test fixtures and ancestry. Full coverage and GitHub-owned lanes remain CI obligations.