diff --git a/decisions/192-native-host-process-identity-via-argv0.md b/decisions/192-native-host-process-identity-via-argv0.md index 25295678b..2949cbdf1 100644 --- a/decisions/192-native-host-process-identity-via-argv0.md +++ b/decisions/192-native-host-process-identity-via-argv0.md @@ -44,7 +44,9 @@ Two findings settled the mechanism: | Windows | accepted | reads/writes the **console title** (`before` came back as `Administrator: Windows PowerShell`), nothing to do with the process at all | A host that set its own title would be fighting its own name, so production code - never writes it — the test only records the probe. + never writes it — the test only records the probe. This applies to every spawn + in the repo, not just this one, so it has its own record: + [decision 195](195-process-title-is-not-a-process-name.md). ## Decision @@ -91,7 +93,8 @@ The carrier is the detached host's **`argv0`**, formatted by one pure function. The full path is still argv[1], and the record keeps `host.executable`. - `DEV3_TASK_SEQ` / `DEV3_PANE_ID` are new ambient vars inside task panes. `configureTestIsolation` scrubs the whole injected task context so a suite run - by an agent cannot silently read the agent's own task. + by an agent cannot silently read the agent's own task — + [decision 196](196-test-isolation-scrubs-inherited-task-env.md). ## Alternatives considered diff --git a/decisions/195-process-title-is-not-a-process-name.md b/decisions/195-process-title-is-not-a-process-name.md new file mode 100644 index 000000000..0b6afe7de --- /dev/null +++ b/decisions/195-process-title-is-not-a-process-name.md @@ -0,0 +1,78 @@ +# 195 — `process.title` is not a process name under Bun; use `argv0` + +## Context + +Any time this repo wants a spawned process to be identifiable in a system process +viewer, `process.title = "..."` is the obvious first reach. It is the wrong one. +[Decision 192](192-native-host-process-identity-via-argv0.md) chose `argv0` for +native terminal hosts; this record exists separately because the finding applies +to **every** spawn in the codebase, not just that one. + +## Investigation + +Measured on all three supported platforms (seq 1383), by spawning a probe and +inspecting it from outside — never from what the process says about itself. + +**Bun accepts the assignment on every platform and never throws**, so a smoke +test that only reads `process.title` back proves nothing at all. What actually +happens differs three ways: + +| Platform | Where the write lands | +|---|---| +| macOS | overwrites the argv area **in place**, bounded by the length of the *original* `argv0` | +| Linux | sets `/proc//comm`, truncated to 15 chars, and rewrites cmdline | +| Windows | sets the **console title** — reading it back returned `Administrator: Windows PowerShell`, nothing to do with the process | + +The macOS bound is what makes this a trap rather than a limitation. A probe +spawned as plain `bun` (3 chars) silently ignored a longer title, which read as +"`process.title` is a no-op on macOS" — the wrong conclusion, drawn from one +shallow check. A probe spawned with a long `argv0` had that `argv0` **destroyed** +by the title write. Title and `argv0` share one buffer and fight over it. + +## Decision + +Production code does not write `process.title`. The carrier for process identity +is the `argv0` option of `node:child_process.spawn`, which under Bun does **not** +disturb the child's own `process.argv` — the child still sees the real +`execPath` at `[0]`, its script at `[1]`, and its own arguments at `[2..]`, so +argv-based verb and entrypoint parsing keeps working. + +`argv0` reaches macOS `ps -o comm=` (verbatim, untruncated) and `ps -o args=`, +Linux `/proc/cmdline` (`ps -o args=`, htop), and the Windows Task Manager +Details → Command line column plus Process Explorer. libuv passes the executable +as `lpApplicationName` and builds `lpCommandLine` from argv, so the Windows +`.exe` image name is untouched. + +Two viewers can **never** show anything but the executable basename, and no argv +trick changes that: macOS Activity Monitor's Process Name column (which has no +command-line column at all) and the Windows Task Manager image-name column. +Verified by screenshot — two identical processes off one carrier binary, one with +`argv0` overridden, render identically. The only lever there is per-task copies +or renames of the binary, which breaks signing, packaging, and the Windows +image-name contract. Ship a CLI fallback instead (`dev3 doctor --processes`). + +The probe and the per-platform assertions, negatives included, live in +`src/bun/native-terminal-registry/__tests__/process-naming-visibility.test.ts` +and run on all three CI runners, so none of these facts can drift into a false +claim unnoticed. + +## Risks + +- The two basename-only viewers stay basename-only. Anyone who checks only + Activity Monitor still needs the CLI fallback. +- The probe currently lives under the native-terminal-registry tests. A future + non-terminal caller that wants process naming should reuse it rather than + re-measure from scratch. + +## Alternatives considered + +- **`process.title` with a length-padded `argv0`** — reserving a long enough + `argv0` would make the macOS write land, but the value still means three + different things on three platforms, and on Windows it is the console title. + Two mechanisms for one job, with the platform matrix doubled. +- **Per-task copies or renames of the executable** — the only way to reach the + basename-only columns, and forbidden: signing, packaged layout, and the + Windows image-name contract all key on that name. +- **Trusting the documented behaviour instead of measuring** — this is exactly + how the "no-op on macOS" misreading happened. Every row of the table above + comes from a live process inspected from outside. diff --git a/decisions/196-test-isolation-scrubs-inherited-task-env.md b/decisions/196-test-isolation-scrubs-inherited-task-env.md new file mode 100644 index 000000000..1a65cc6aa --- /dev/null +++ b/decisions/196-test-isolation-scrubs-inherited-task-env.md @@ -0,0 +1,60 @@ +# 196 — Test isolation scrubs the agent's own task environment + +## Context + +Most test runs in this repo are started **by an agent, inside a dev3 task pane**. +That pane exports the task's own context, so every suite inherits +`DEV3_TASK_ID`, `DEV3_TASK_TITLE`, `DEV3_WORKTREE_PATH`, `DEV3_BRANCH_NAME` — +and, when the task runs on the native terminal backend, that host's +`DEV3_NATIVE_SESSION_ID` / `_LAUNCH` / `_COLS` / `_ROWS` as well. CI inherits +none of it. + +That asymmetry produces the most expensive kind of failure: a test that passes +in CI and fails only for the agent, or the reverse — and the agent has no reason +to suspect its own environment, so it goes looking in the diff. + +## Investigation + +Found while adding `DEV3_TASK_SEQ` (seq 1383), but the bug predates that work. +`src/bun/__tests__/native-host-runtime.test.ts` → "omits the opt-in proof flags +when they were not requested" asserts the launcher's env has no +`DEV3_NATIVE_SESSION_COLS`. The launcher builds its env as `{...process.env, ...}`, +so it read the *agent's own pane* value and failed — locally only, on unmodified +code. Several older tests already worked around the same class by hand, deleting +`DEV3_TASK_ID` in a `beforeEach` (see `src/cli/__tests__/context.test.ts`). + +## Decision + +`configureTestIsolation` (`test-isolation.ts`), which already sandboxes `HOME`, +`TMPDIR`, `DEV3_HOME` and the XDG roots, now also deletes the inherited task +context: every key in `INHERITED_TASK_CONTEXT_ENV`, plus every key starting with +`DEV3_NATIVE_SESSION_`. Every suite therefore starts from "no task in scope", +and a test that wants one sets it explicitly. + +`DEV3_NATIVE_SESSIONS_DIR` (plural) is a test-owned override and deliberately +does not match that prefix. Guarded by `src/bun/__tests__/test-isolation.test.ts`, +which both asserts the scrub happened and enumerates the list — so **a new env +var injected into task panes must be added to `INHERITED_TASK_CONTEXT_ENV` in the +same change**, or that test fails on purpose. + +Corollary for new code: a module that needs task context should take env as an +**argument** rather than reading `process.env`. That is why +`src/bun/native-terminal-registry/process-naming.ts` carries a test asserting its +own source contains no `process.env` at all. + +## Risks + +- A future test that genuinely wants the ambient agent task must now set the var + itself. That is the intended trade: explicit beats inherited. +- The prefix sweep is broad by design. A future `DEV3_NATIVE_SESSION_*` var meant + to survive into tests would need a different name, which is the right pressure. + +## Alternatives considered + +- **Keep deleting vars per-suite in `beforeEach`** — the status quo that let this + through. It only protects the suites whose author already knew about the trap. +- **Scrub every `DEV3_*` var** — would also drop `DEV3_HOME`, `DEV3_LOG_DIR` and + `DEV3_TEST_ROOT`, which this same function deliberately sets. +- **Have CI export the same vars so both sides match** — makes the environments + agree by making both wrong, and would feed a real task's identity to suites + that should never see one.