Skip to content

chore(layering): forbid process globals outside entry points #3753

Closed
#3823
Closed
chore(layering): forbid process globals outside entry points#3753
#3823
Labels
in-progressWork is actively in progressready-for-agentrefactor

Description

@thymikee
thymikee
opened on Oct 16, 2026

Part of #3747. Blocked by #3750, #3751, #3752.

Required behavior

  • The rule. A lint or layering rule forbids process.env, process.cwd(), process.on, process.once, process.exit and process.emitWarning in non-test sources outside the declared entry points.
  • Declare the entry points beside the rule:
    • the CLI, the daemon executable and the client factory;
    • the agent-device/host boundary;
    • scripts;
    • worker and child-process entry files: the PNG worker, Apple helpers, and the detached runner launcher.
  • Each worker or child entry receives only explicit, serializable configuration.
  • No baseline and no allowlist.

Done when

  • The rule runs in pnpm check.
  • Adding a process.env read to any non-entry module fails the gate.

Activity

  1. thymikee
    added
    refactor
    ready-for-agent
    in-progressWork is actively in progress
    on Oct 16, 2026

  2. thymikee commented on Oct 19, 2026

    @thymikee
    thymikee
    on Oct 19, 2026
    MemberAuthor

    #3753 cannot be landed yet: #3775's "Remaining, handed to owners" lists were never owned

    I verified origin/main (112d291605) rather than trusting the closure notes. The result is narrower than a blanket gap: the platform and capture packages from #3775's list 2 are genuinely clean and its list 3 is waived by contract, so the residue sits in three places — list 1, list 4, the root half of list 2 — plus one cluster nobody has claimed.

    Verified against main, by #3775's own list

    #3775 "Remaining" Owner named there State on 112d291605
    1. session-journal cap, device-selection env, replay-port/maestro/ad-script replay env and cwd #3772 Not done. #3772 landed its own scope (#3752) and never touched these.
    2. adb port environment, app-log output/process env, platform-* packages #3750 / #3777 Split. platform-* and capture-kit are clean (done for them); the three root platform-runtime-* files are not done, and no package owns them.
    3. provider-device-runtimes.ts, provider-credential-fingerprint.ts, cli/connection/provider-policy.ts, plugins/* Plugin v2 stack Open by contract — released-default waiver until #3759.
    4. update-check.ts, replay-test reporters/progress, color.ts, diff.ts, commands/replay/index.ts, doctorCliOutput, plugins.ts, cli-config.ts #3763 Not done. #3763 landed and never converted them.

    The commandHostEnvironment() half of #3753's premise is accurate as #3798 states it: exactly the two spawn-bound whichCmd lookups (packages/platform-apple/src/core/tool-provider.ts:80, packages/platform-linux/src/tool-provider.ts:91) plus the Linux standalone display fallback (packages/platform-linux/src/tool-provider.ts:139). What is missing is the raw process.env / process.cwd() residue that #3753's rule also forbids.

    The residue: 36 reads / 24 files outside the CLI

    process.env / process.cwd() in production sources outside src/cli, excluding packages/test-kit (test support) and default-parameter sites. The four clusters below sum to 36: 16 daemon (unowned) + 10 + 3 + 7. Three further reads are released-boundary wrappers under the #3751 waiver, listed separately. packages/session-journal is reached as a live read but is written as a default parameter, so it counts in the waiver too.

    No owner anywhere — packages/daemon, 16 reads / 9 files. This is the largest cluster and it is not on any of the four lists above. #3752's "Done when" is only "no process.exit, process.on or process.once under packages/daemon/src", which main satisfies (verified: zero hits); env and cwd reads were never in its scope.

    • execution/app-events.ts:103,105,106,107 — the four AGENT_DEVICE_*_APP_EVENT_URL_TEMPLATE reads
    • core/session-script-publication-capability.ts:47,97,135, core/session-script-writer.ts:163, core/session-replay-transaction.ts:65 — expandSessionPath(..., process.cwd())
    • execution/screenshot-runtime.ts:326 — req.meta?.cwd ?? process.cwd()
    • sessions/handlers/record-runtime.ts:312,321 — cwd: process.cwd() and req.meta?.cwd ?? process.cwd()
    • server/http/http-server.ts:100,101 — options.env ?? process.env, options.cwd ?? process.cwd()
    • resources/artifact-tracking.ts:269 — { ...process.env, COPYFILE_DISABLE: '1' } into tar
    • sessions/handlers/session-doctor-output.ts:44 — shouldColorStatusMarkers(process.env, …)

    Note: packages/daemon/src/server/http/daemon-runtime.ts already receives host: HostEnvironment and threads env through ~30 configuration reads (lines 192–478), so the plumbing exists — these 16 sites sit outside it.

    List 1 — named for #3772, 10 reads / 8 files (plus the session-journal default parameter below)

    • packages/replay-port/src/session-replay-vars.ts:28, daemon-port/session-replay-runtime-session.ts:183, daemon-port/test-command.ts:409, daemon-port/session-test-shard-devices.ts:97,106
    • packages/maestro/src/internal/engine-flow.ts:104, packages/maestro/src/daemon-port/run-script-http-child.ts:9
    • packages/ad-script/src/internal/vars.ts:113 — readReplayShellEnvSource falls back to process.env
    • packages/device-selection/src/dispatch-resolve.ts:218,222
    • packages/session-journal/src/session-event-log-window.ts:66 — default parameter, reached as a live read from session-event-log.ts:90

    List 2, root half — 3 reads / 3 files

    • src/platform-runtime-android-adb-host.ts:30 — const environment = process.env; at module scope
    • src/platform-runtime-app-log-android-transport.ts:31 — { ...process.env, ...adb.options.env }
    • src/platform-runtime-app-log-process.ts:176 — { ...process.env, ...request.env }

    List 4 — named for #3763, 7 reads / 4 files

    • src/commands/output/color.ts:4,8 — FORCE_COLOR, NO_COLOR
    • src/commands/output/diff.ts:160 — process.cwd()
    • src/commands/replay/index.ts:248,275,348 — collectReplayClientShellEnv(process.env) ×2, input.cwd ?? process.cwd()
    • src/commands/schema/cli-config.ts:25 — options.env ?? process.env

    Released-boundary wrappers, excluded from the 40 (inside the #3751 waiver until #3759): src/io.ts:13 (createLocalArtifactAdapter) and src/sdk/remote-config.ts:13 (resolveRemoteConfigProfile), both named in #3775's summary. src/mcp/tool-input-config.ts:16 is the third such wrapper but is not named by #3775 — it waits on "the MCP caller must pass env first" in list 4.

    One correction to the closure note on #3750

    #3750's "Done when" also requires the PNG worker to receive explicit serializable configuration rather than the inherited worker env (packages/capture-kit/src/png-worker-client.ts:113). On main the PNG worker reads no process globals and its entry is clean, so the requirement holds in effect — by absorption into the runtime (#3772's refcounted worker), not by the explicit-config handover #3750 describes. Worth one confirming word from its owner rather than a reopen.

    What I have not done

    No code, no PR, no baseline, no allowlist, per the plan. in-progress is on #3753. I have not touched #3757's branches.

    Needed to proceed

    1. The daemon cluster (16 reads / 9 files) needs an owner. It is on no list. refactor(daemon): daemon runtime owns no process globals #3752 is genuinely closed and cannot carry it; refactor(host): explicit host for configuration reads and one entered runtime store #3794 (open, owner of explicit-host configuration reads) is the closest fit.
    2. List 1 → feat(daemon): the runtime owns its state and never the process #3772 as refactor(host): read host settings from HostEnvironment in root src and remaining packages #3775 states, or one new child owning those 10 reads plus the session-journal default parameter.
    3. List 2 root half → assign. refactor(host): read host settings from HostEnvironment in root src and remaining packages #3775 gives it to 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, but those are package-scoped and these are root platform-runtime-* files, so no package scope holds them.
    4. List 4 → feat(host): capture HostEnvironment once at entry points #3763, as refactor(host): read host settings from HostEnvironment in root src and remaining packages #3775 states.
    5. Decide the rule's treatment of env = process.env default parameters. The 14 sites are waived until 0.22: remove superseded plugin, client and environment APIs #3759 by refactor: read host settings from HostEnvironment in root src and remaining packages #3751, so chore(layering): forbid process globals outside entry points #3753's rule must exempt default parameters until 0.22: remove superseded plugin, client and environment APIs #3759 — or chore(layering): forbid process globals outside entry points #3753 waits for 0.22: remove superseded plugin, client and environment APIs #3759. I cannot read that from the issue text, which says "No baseline and no allowlist." This includes packages/session-journal/src/session-event-log-window.ts:66, which is a default parameter but is reached live from session-event-log.ts:90.
    6. Confirm the CLI's own reads are in scope as an entry point. Outside the four lists, src/cli holds 9 reads / 5 files: update-check.ts:94,95,96, commands/plugins.ts:29,33, replay-test/progress.ts:242, replay-test/reporters/default.ts:106,110, replay-test/reporters/custom.ts:54. All five are named in refactor(host): read host settings from HostEnvironment in root src and remaining packages #3775's list 4 as feat(host): capture HostEnvironment once at entry points #3763's debt, so the issue and refactor(host): read host settings from HostEnvironment in root src and remaining packages #3775 disagree about whether the CLI entry point may read them. src/cli/connection/provider-policy.ts is defaults only, and src/cli/process-exit.ts is the exit wrapper itself.

    Once 1–4 have owners and land, and 5–6 are decided, #3753 becomes mechanical: one layering rule in the existing scripts/layering/ machinery (next free id R86), the entry-point declarations beside it, the commandHostEnvironment() boundary ban, and the two self-tests.

  3. thymikee commented on Oct 19, 2026

    @thymikee
    thymikee
    on Oct 19, 2026
    MemberAuthor

    Coordinator decision on the questions in #3753 and #3747. Thanks for the audit: it changes the plan, not the goal.

    Owners. #3753 owns all 36 reads. The rule is "resolve violations at the owning module", and these reads are what keep the gate from landing, so they belong to the gate's issue. No new issues. Land them in one PR, ahead of the rule, with one commit per cluster:

    1. packages/daemon: the 16 reads take the runtime host the daemon already holds (HostEnvironment.env/.cwd). expandSessionPath(..., process.cwd()) takes host.cwd.
    2. replay-port, maestro, ad-script and device-selection: the 10 reads take the host from their callers.
    3. Root src/platform-runtime-*: the 3 reads take the composition host.
    4. src/commands/**: the 7 reads take the command's host.

    The gate rule itself goes in the final commit. The PR then lands green with no baseline.

    Default parameters (env = process.env).

    • Internal functions (not reachable from a released exports entry): make the parameter required, and have callers pass the host's env. Examples are resolveSessionIdleExpiryMs, resolveDaemonIdleReapMs and resolveSessionEventLogMaxBytes.
    • Released public API: keep the signature, which stays additive in 0.21.x. Take the default from host-kit's process capture rather than a raw process.env, so host-kit stays the one module that reads the process. 0.22: remove superseded plugin, client and environment APIs #3759 removes those defaults in 0.22.

    There is no exemption list either way. List the public ones you keep in the PR body, so #3759 has its removal list.

    "The CLI" as an entry point. src/cli/** is a declared entry point, so its 9 reads may stay. Threading them through a captured host is welcome but not required here.

    test-kit. packages/test-kit is test support, so the rule excludes it the same way it excludes *.test.ts. Say so in the rule's own declaration.

    commandHostEnvironment(): the boundary list from #3798 holds as you confirmed.

  4. thymikee
    added
    needs-infoWaiting on reporter or external input
    and removed
    needs-infoWaiting on reporter or external input
    on Oct 19, 2026
  5. thymikee
    added 3 commits that reference this issue on Oct 19, 2026

    chore(gates): forbid process globals outside declared entry points (#…

    e223d37

    chore(gates): forbid process globals outside declared entry points (#…

    95c290d

    chore(gates): forbid process globals outside declared entry points (#…

    6aefc10
  6. thymikee
    closed this as completedin #3823on Oct 19, 2026
  7. thymikee
    added 5 commits that reference this issue on Oct 19, 2026

    refactor(daemon): resolve configuration from the owning runtime host (#…

    df066e2

    refactor(replay): thread runtime host through replay and device selec…

    d6f3172

    refactor(runtime): use the owning host environment at process boundar…

    267e9a5

    refactor(commands): resolve command inputs and output from the caller…

    7faa04e

    chore(gates): forbid process globals outside declared entry points (#…

    f8eddcb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    in-progressWork is actively in progressready-for-agentrefactor

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions