fix(core,tooling): keep the harness's own ports out of the Agent's environment and off the dev server (#100)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: J.Li <jl2388@cam.ac.uk>

packages/core/src/environment/tools/command/session-manager.ts

@@ -32,6 +32,50 @@ const HARDENED_ENV: NodeJS.ProcessEnv = {
   GIT_PAGER: "cat",
 };
 
+/**
+ * Harness-owned variables **removed** from the child environment (removed, not blanked: a
+ * program that checks `PORT` for presence rather than value must see nothing at all).
+ *
+ * `PORT` / `HOST` are stripped because they are never about the command being run. On the
+ * serving paths they are the harness's own listener: `penguin web` / `penguin server` write both
+ * into their own `process.env` as the channel to the server module (see the CLI's `startServer`).
+ * On the CLI-only paths (`penguin run`, `penguin chat`, the REPL) nothing listens at all, but the
+ * CLI still loads `dotenv/config`, so a `PORT` there is the one the *user's own project* picked
+ * for *its* server. `npm run dev`, Vite, Next and most Express templates read `PORT`, so either
+ * way an inherited value makes a server the Agent starts bind a port it was never asked to take —
+ * the harness's own in the first case, one already spoken for in the second. A command that needs
+ * a particular port should be told so in its own invocation (or through the vault), never by
+ * ambient inheritance.
+ *
+ * `PENGUIN_CLI_ENTRY` is internal plumbing: the CLI uses it to tell the server which script to
+ * re-run for self-update. It means nothing to any other program and leaks the install path.
+ *
+ * `PENGUIN_WEB_DIST` is *not* internal — it is a documented deployment override (see the
+ * configuration reference and the server README) — and is stripped anyway because it names this
+ * installation's front-end build. In the self-development case an Agent that starts a
+ * PenguinHarness server would otherwise serve the deployment's assets instead of the ones it just
+ * built in the workspace, silently and with no error to read.
+ *
+ * Deliberately **not** stripped: `PENGUIN_HOME`, `PENGUIN_WEB_DB` and the rest of the user-facing
+ * `PENGUIN_*` settings. Those select the *data* an Agent-started harness works against, and the
+ * self-development case may legitimately want the same data root — sharing state is a config
+ * decision, whereas serving a deployment's code from a workspace checkout never is.
+ */
+const STRIPPED_ENV_KEYS = new Set(["PORT", "HOST", "PENGUIN_CLI_ENTRY", "PENGUIN_WEB_DIST"]);
+
+/** The host environment minus {@link STRIPPED_ENV_KEYS}. */
+function hostEnvForChild(): NodeJS.ProcessEnv {
+  const env: NodeJS.ProcessEnv = {};
+  // Matched case-insensitively rather than deleting the upper-case spellings: Windows resolves
+  // environment names without regard to case but stores whatever casing was written, so a
+  // `set Port=3000` before `penguin web` would survive a `delete env.PORT` and still reach the
+  // child as PORT. On POSIX the two are distinct names and only the exact one exists.
+  for (const [key, value] of Object.entries(process.env)) {
+    if (!STRIPPED_ENV_KEYS.has(key.toUpperCase())) env[key] = value;
+  }
+  return env;
+}
+
 export class CommandSessionManager {
   private readonly registry = new BackgroundRegistry<ManagedSession>({
     idPrefix: "proc",
@@ -55,8 +99,10 @@ export class CommandSessionManager {
       cwd: opts.cwd,
       // Spread order is priority: vault overrides host variables of the same name, but must
       // come before HARDENED_ENV — the hardening entries (GIT_EDITOR/PAGER etc. that prevent
-      // interactive hangs) must never be overridable by vault.
-      env: { ...process.env, ...this.vault, ...HARDENED_ENV },
+      // interactive hangs) must never be overridable by vault. The host side is stripped of
+      // the harness's own variables first (see STRIPPED_ENV_KEYS); the vault still wins, so a
+      // user who genuinely wants PORT in commands can set it there.
+      env: { ...hostEnvForChild(), ...this.vault, ...HARDENED_ENV },
     });
   }
 

packages/core/src/internal/ports.ts

@@ -6,5 +6,30 @@
  * runtime.
  */
 
+/**
+ * Port allocation across the repo (documented here because it is the one place a reader
+ * looks for it; the dev ports themselves live in vite configs and package.json scripts,
+ * neither of which can import this module):
+ *
+ * | port | who                                | where                                       |
+ * | ---- | ---------------------------------- | ------------------------------------------- |
+ * | 7364 | installed server / Web UI          | `DEFAULT_SERVER_PORT` below                 |
+ * | 7365 | `pnpm dev:web` (Vite)              | `packages/web/vite.config.ts`               |
+ * | 7366 | `pnpm dev:landing` (Vite)          | `packages/landing/vite.config.ts`           |
+ * | 7367 | `pnpm dev:docs` (Vite)             | `packages/docs/vite.config.ts`              |
+ * | 7368 | `pnpm dev:server` (dev backend)    | `packages/server/package.json` `dev`        |
+ * | 7369 | `pnpm penguin web` (dev CLI)       | the root and cli `penguin` scripts          |
+ *
+ * The development backend deliberately does **not** share 7364 with an installed one: the
+ * two are routinely running at once, and before they were split, `pnpm dev` either failed
+ * to bind or -- worse -- the Vite proxy silently talked to the installed server instead of
+ * the one being worked on. The dev data root is separated for the same reason.
+ *
+ * The dev CLI gets a third port rather than reusing the backend's 7368 because the two also
+ * run at once: a harness started as `pnpm penguin web` is exactly what asks an Agent to run
+ * `pnpm dev` in this repo, and sharing the number would reintroduce that collision one step
+ * to the left -- `dev:server` failing to bind, or the Vite proxy answering from the harness.
+ */
+
 /** Default main server / Web UI port; deliberately avoids common defaults like 3000/8080. */
 export const DEFAULT_SERVER_PORT = 7364;