Skip to content

fix(envFilter): plumb PM_WRITE + FRICTION sidecar env vars to subprocesses (MNG-741)#1355

Merged
zbigniewsobiecki merged 1 commit into
devfrom
fix/pm-write-sidecar-envvar-allowlist
May 12, 2026
Merged

fix(envFilter): plumb PM_WRITE + FRICTION sidecar env vars to subprocesses (MNG-741)#1355
zbigniewsobiecki merged 1 commit into
devfrom
fix/pm-write-sidecar-envvar-allowlist

Conversation

@zbigniewsobiecki
Copy link
Copy Markdown
Member

Summary

Three consecutive planning runs failed identically on prod (2026-05-12) — all planning agent, all codex engine, all "Done." outputs, all rejected by the requiresPMWrite completion gate:

Work item Run Duration
MNG-741 44c1bc3f 11m 19s
MNG-736 98ae7010 12m 49s
MNG-739 1534ab74 16m 05s

Same error on each: "Agent completed but no PM write (checklist creation) was recorded".

Root cause

src/gadgets/sessionState.ts defines five sidecar env-var constants. src/backends/sidecarManager.ts injects all five paths into projectSecrets. But src/backends/shared/envFilter.ts's SHARED_ALLOWED_ENV_EXACT — the bottleneck for what survives the env-allowlist on the way to the subprocess engine — listed only three:

  PR_SIDECAR_ENV_VAR,
  PUSHED_CHANGES_SIDECAR_ENV_VAR,
  REVIEW_SIDECAR_ENV_VAR,
+ PM_WRITE_SIDECAR_ENV_VAR,    // ← was silently filtered out
+ FRICTION_SIDECAR_ENV_VAR,    // ← same drift

So when the planning agent ran cascade-tools pm add-checklist ..., the CLI handler (src/cli/pm/add-checklist.ts:46) read process.env[PM_WRITE_SIDECAR_ENV_VAR], got undefined, and writePMWriteSidecar (src/gadgets/session/core/sidecar.ts:6-9) bailed early. No sidecar, no evidence, gate fails — on every planning run, every engine.

FRICTION_SIDECAR_ENV_VAR had the same drift, but the friction code path has fallbacks (reportFriction.ts:194-196 reads process.env ?? getFrictionSidecarPath() ?? DEFAULT_FRICTION_SIDECAR_PATH) so it limped along. Fix is defensive — restores design intent.

The broader bug class: nothing enforced that every sidecar constant from sessionState.ts was in the allowlist. Drift was silent.

Fix

  1. src/backends/shared/envFilter.ts — add PM_WRITE_SIDECAR_ENV_VAR and FRICTION_SIDECAR_ENV_VAR to the import and the allowlist. Comment links the choice to the MNG-741 incident.
  2. Regression net in tests/unit/backends/shared-envFilter.test.ts — new "sidecar env-var allowlist invariant" describe block iterates all five constants and asserts (a) set membership and (b) filterProcessEnv round-trip survival. A future maintainer adding a sixth sidecar without registering it fails CI with a precise message.

Test plan

  • TDD-first: 4 failing tests added before the fix (2 each for PM_WRITE + FRICTION). Verified red, then green after the import + allowlist edit.
  • Full unit-backends suite: 47 files, all green.
  • Typecheck + lint clean (no new warnings).
  • Pre-push hooks (lint, auth-header-provenance, typecheck, commitlint) all green.
  • Post-deploy verification: trigger a fresh planning run (e.g. cascade runs trigger --project cascade --agent planning --workItemId <new>). Verify it completes successfully and the work item has a checklist attached. Backfill MNG-741/736/739 manually.

Out of scope

  • In-process gadget classes that don't write sidecars (src/gadgets/AddChecklist.ts, CreatePR.ts, CreatePRReview.ts delegate to core functions without writing sidecars). Today every engine shells out to cascade-tools so the CLI path is the only used path — latent, not active. Worth a follow-up that factors the sidecar write into a shared post-action hook.
  • End-to-end "agent calls add-checklist → engine sees sidecar" worker-shaped integration test. Would be worth adding alongside a future hook refactor.

🤖 Generated with Claude Code

…esses

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) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - verified the env-filter fix and targeted regression test; all PR checks are green. I reviewed this as the sidecar env hotfix described in the PR, not as completion of the broader MNG-741 checklist-idempotency work item.

🕵️ codex · gpt-5.5 · run details

// 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 = [
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: this list is still manual, so it does not enforce the stated invariant for future sidecars. Adding a new *_SIDECAR_ENV_VAR export in sessionState.ts without updating this array would still pass. To make the regression net match the PR description, derive the list from the module exports or export an authoritative sidecar env-var list from sessionState.ts.

@zbigniewsobiecki zbigniewsobiecki merged commit e496850 into dev May 12, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants