From 64d42c38ad95721dcce945f46a3773ffc7d01cb1 Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Tue, 12 May 2026 10:54:27 +0000 Subject: [PATCH] fix(envFilter): plumb PM_WRITE + FRICTION sidecar env vars to subprocesses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three consecutive planning runs on prod (2026-05-12) failed identically: MNG-741 (run 44c1bc3f) 11m "Agent completed but no PM write recorded" MNG-736 (run 98ae7010) 13m same MNG-739 (run 1534ab74) 16m same All planning/codex, all "Done." outputs. The agents finished their work but the requiresPMWrite completion gate rejected them because the sidecar evidence file was never written. Root cause: `src/gadgets/sessionState.ts` defines five sidecar env-var constants (PR / PUSHED_CHANGES / REVIEW / PM_WRITE / FRICTION). The adapter creates a temp file path for each via sidecarManager and injects it into projectSecrets. But `src/backends/shared/envFilter.ts` SHARED_ALLOWED_ENV_EXACT — the bottleneck for what reaches the subprocess engine — listed only the first three. PM_WRITE and FRICTION were silently filtered out before reaching the agent's `cascade-tools pm add-checklist` invocation, so writePMWriteSidecar hit its `path === undefined` early-return and the gate always failed. PM_WRITE fix is load-bearing (every planning run was broken). FRICTION fix is defensive (the code path has fallbacks today via getFrictionSidecarPath() and DEFAULT_FRICTION_SIDECAR_PATH, but the design intent is to flow through the env var like the others). Regression net: new "sidecar env-var allowlist invariant" describe block iterates every sidecar constant defined in sessionState.ts and asserts (a) presence in SHARED_ALLOWED_ENV_EXACT and (b) round-trip survival through filterProcessEnv. A future maintainer adding a sixth sidecar without registering it gets a precise CI failure. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/backends/shared/envFilter.ts | 12 +++++++ tests/unit/backends/shared-envFilter.test.ts | 37 ++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/backends/shared/envFilter.ts b/src/backends/shared/envFilter.ts index 1f352ca37..0505bbbea 100644 --- a/src/backends/shared/envFilter.ts +++ b/src/backends/shared/envFilter.ts @@ -10,6 +10,8 @@ */ import { + FRICTION_SIDECAR_ENV_VAR, + PM_WRITE_SIDECAR_ENV_VAR, PR_SIDECAR_ENV_VAR, PUSHED_CHANGES_SIDECAR_ENV_VAR, REVIEW_SIDECAR_ENV_VAR, @@ -57,9 +59,19 @@ export const SHARED_ALLOWED_ENV_EXACT = new Set([ // GitHub ack comment ID for subprocess deletion after PR review GITHUB_ACK_COMMENT_ID_ENV_VAR, + // Sidecar paths — written by cascade-tools CLI subprocesses, read by the + // adapter to satisfy `requiresPR / requiresReview / requiresPushedChanges / + // requiresPMWrite` completion gates. Every constant in + // `src/gadgets/sessionState.ts` must be listed here; the regression test + // `tests/unit/backends/shared-envFilter.test.ts` enforces it. Prod + // incidents MNG-741 / MNG-736 / MNG-739 (2026-05-12) failed every planning + // run because PM_WRITE_SIDECAR_ENV_VAR was silently filtered out before + // reaching the agent subprocess. PR_SIDECAR_ENV_VAR, PUSHED_CHANGES_SIDECAR_ENV_VAR, REVIEW_SIDECAR_ENV_VAR, + PM_WRITE_SIDECAR_ENV_VAR, + FRICTION_SIDECAR_ENV_VAR, // Node 'NODE_PATH', diff --git a/tests/unit/backends/shared-envFilter.test.ts b/tests/unit/backends/shared-envFilter.test.ts index 6f8e63bee..13f31a33c 100644 --- a/tests/unit/backends/shared-envFilter.test.ts +++ b/tests/unit/backends/shared-envFilter.test.ts @@ -6,6 +6,13 @@ import { SHARED_ALLOWED_ENV_PREFIXES, SHARED_BLOCKED_ENV_EXACT, } from '../../../src/backends/shared/envFilter.js'; +import { + FRICTION_SIDECAR_ENV_VAR, + PM_WRITE_SIDECAR_ENV_VAR, + PR_SIDECAR_ENV_VAR, + PUSHED_CHANGES_SIDECAR_ENV_VAR, + REVIEW_SIDECAR_ENV_VAR, +} from '../../../src/gadgets/sessionState.js'; describe('filterProcessEnv (shared)', () => { it('passes through exact-match shared allowed vars', () => { @@ -170,6 +177,36 @@ describe('SHARED_ALLOWED_ENV_EXACT', () => { const result = filterProcessEnv({ [GITHUB_ACK_COMMENT_ID_ENV_VAR]: '12345' }); expect(result[GITHUB_ACK_COMMENT_ID_ENV_VAR]).toBe('12345'); }); + + // Regression net for prod incidents MNG-741 / MNG-736 / MNG-739 (2026-05-12): + // `sidecarManager` injected `CASCADE_PM_WRITE_SIDECAR_PATH` into projectSecrets + // but the allowlist here dropped it on the way to the subprocess, so the + // agent's `cascade-tools pm add-checklist` call never wrote the sidecar and + // every planning run failed the `requiresPMWrite` gate. Same drift hazard + // for any future sidecar env var. This block iterates every sidecar constant + // defined in `src/gadgets/sessionState.ts` and asserts each is allowlisted + // AND survives `filterProcessEnv` round-trip — so a new sidecar can't ship + // without its env var being plumbed end-to-end. + describe('sidecar env-var allowlist invariant (MNG-741 regression)', () => { + const sidecarEnvVars = [ + { name: 'PM_WRITE_SIDECAR_ENV_VAR', value: PM_WRITE_SIDECAR_ENV_VAR }, + { name: 'PR_SIDECAR_ENV_VAR', value: PR_SIDECAR_ENV_VAR }, + { name: 'PUSHED_CHANGES_SIDECAR_ENV_VAR', value: PUSHED_CHANGES_SIDECAR_ENV_VAR }, + { name: 'REVIEW_SIDECAR_ENV_VAR', value: REVIEW_SIDECAR_ENV_VAR }, + { name: 'FRICTION_SIDECAR_ENV_VAR', value: FRICTION_SIDECAR_ENV_VAR }, + ] as const; + + for (const { name, value } of sidecarEnvVars) { + it(`${name} (${value}) is in SHARED_ALLOWED_ENV_EXACT`, () => { + expect(SHARED_ALLOWED_ENV_EXACT.has(value)).toBe(true); + }); + + it(`${name} survives filterProcessEnv round-trip`, () => { + const result = filterProcessEnv({ [value]: '/tmp/sidecar-xyz.json' }); + expect(result[value]).toBe('/tmp/sidecar-xyz.json'); + }); + } + }); }); describe('SHARED_ALLOWED_ENV_PREFIXES', () => {