From 1f4acd13d48968dd3c95565dbf39e5e70e607f23 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 10 May 2026 20:39:43 +0200 Subject: [PATCH] fix(env-loader): assertRickyWorkflowEnv honors START_FROM resume signal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `assertRickyWorkflowEnv(['NANGO_SECRET_KEY', ...])` is injected at the top of `main()` and runs at module-load before the SDK has a chance to honor `--start-from`. That made `ricky run --start-from ` impossible whenever the missing env var wasn't actually used by the resumed step — the upstream-only step needed it, but it was being skipped. Concrete repro from this thread: a workflow generated against the proactive-pr-remediation spec asserted NANGO_SECRET_KEY at module load. The user's first attempt had it set; a subsequent `--start-from final-hard-validation --previous-run-id ` resume did not. The resumed step (deterministic shell: tsc + scoped npm test + git diff + grep) does not use NANGO_SECRET_KEY at all, but the assert fired before the SDK could route execution to that step. Fix: when process.env.START_FROM is set (the SDK exports this for --start-from resumes — see src/local/entrypoint.ts:799), the helper warns-and-continues instead of throwing. Resumed steps that genuinely need a missing env var will still fail with their own signal at the point of use; the check just stops blocking resumes where it has no information about whether the missing var matters. First-run invocations (no START_FROM) still throw fast — the original fail-fast contract for fresh runs is preserved. Both injected variants are updated: - rickyWorkflowEnvLoaderSource (full helper bundle, used when env loading is being added to a workflow that has neither the loader nor the assert) - rickyWorkflowEnvAssertSource (assert-only helper, used when the loader is already present but the assert is missing) Tests: - New regression case in auto-fix-loop.test.ts: "injects an env-assert helper that honors START_FROM for --start-from resumes" — asserts the injected source contains the process.env.START_FROM check, the console.warn message acknowledging --start-from, and that the throw path is preserved for non-resume invocations. - Existing 29 auto-fix-loop tests still pass; full ricky suite at 1066 / 1066 (was 1064). Out of scope (worth a follow-up): - The proper long-term fix is per-step env declaration so the renderer can scope assertions to the steps that actually consume each variable. That requires SDK changes beyond this PR. The warn-and- continue gate is the pragmatic unblock for the common case (resuming past an assertion that doesn't apply to the resumed surface). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/local/auto-fix-loop.test.ts | 28 ++++++++++++++++++++++++++++ src/local/auto-fix-loop.ts | 21 +++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/local/auto-fix-loop.test.ts b/src/local/auto-fix-loop.test.ts index 50b73747..74cca802 100644 --- a/src/local/auto-fix-loop.test.ts +++ b/src/local/auto-fix-loop.test.ts @@ -609,6 +609,34 @@ describe('runWithAutoFix', () => { expect(repair?.content).toContain('MISSING_ENV_VAR:'); }); + // Regression: assertRickyWorkflowEnv used to throw at module-load time + // unconditionally, before the SDK had a chance to honour --start-from. + // That made resuming with --start-from impossible if the resumed step + // didn't actually need the missing env var (the upstream-only step did, + // but it was being skipped). The injected helper now warns-and-continues + // when process.env.START_FROM is set so resumed steps can run and any + // step that genuinely needs a missing value still fails with its own + // signal at the point of use. + it('injects an env-assert helper that honors START_FROM for --start-from resumes', () => { + const repair = repairWorkflowDeterministically({ + artifactPath: 'workflows/generated/foo.ts', + artifactContent: workflowContent(), + evidence: missingEnvEvidence(), + response: blockerResponse('MISSING_ENV_VAR', 'run-1', 'runtime-launch'), + }); + + expect(repair?.applied).toBe(true); + // Resume signal acknowledged. + expect(repair?.content).toContain('process.env.START_FROM'); + // Warn-and-continue path uses console.warn rather than throwing so the + // SDK can proceed to the resumed step. + expect(repair?.content).toMatch(/console\.warn\([^)]*Skipping env-var assertion/); + expect(repair?.content).toMatch(/--start-from active/); + // The non-resume path still throws fast — preserves the original + // contract for first-run invocations. + expect(repair?.content).toContain('throw new Error(`MISSING_ENV_VAR:'); + }); + // Regression: when a master-rendered workflow embeds a `node --input-type=module` // HEREDOC inside a .step({ command: ... }) string, the embedded shell text // contains the literal substring `from 'node:fs'`. The previous import-detection diff --git a/src/local/auto-fix-loop.ts b/src/local/auto-fix-loop.ts index c0cf6118..fc00d8b3 100644 --- a/src/local/auto-fix-loop.ts +++ b/src/local/auto-fix-loop.ts @@ -822,9 +822,19 @@ function loadRickyWorkflowEnv(cwd = process.cwd()) { function assertRickyWorkflowEnv(names: string[]): void { const missing = names.filter((name) => !process.env[name]); - if (missing.length > 0) { - throw new Error(\`MISSING_ENV_VAR: \${missing.join(', ')}. Add missing values to .env.local or export them before rerunning.\`); + if (missing.length === 0) return; + // When the workflow is being resumed via --start-from, the SDK sets + // process.env.START_FROM. Skipped upstream steps may have been the + // ones that needed these env vars; the resumed steps may not. Warn + // instead of failing fast so the resume can proceed and any step that + // actually needs a missing value will fail naturally with its own + // signal. Without this, --start-from on a workflow whose missing env + // var isn't in the resumed step's path is unrecoverable from the CLI. + if (process.env.START_FROM) { + console.warn(\`[ricky] Skipping env-var assertion (--start-from active): missing \${missing.join(', ')}. Resumed steps that actually need these will fail with their own error.\`); + return; } + throw new Error(\`MISSING_ENV_VAR: \${missing.join(', ')}. Add missing values to .env.local or export them before rerunning.\`); } function unquoteRickyWorkflowEnvValue(value: string): string { @@ -839,9 +849,12 @@ function unquoteRickyWorkflowEnvValue(value: string): string { function rickyWorkflowEnvAssertSource(): string { return `function assertRickyWorkflowEnv(names: string[]): void { const missing = names.filter((name) => !process.env[name]); - if (missing.length > 0) { - throw new Error(\`MISSING_ENV_VAR: \${missing.join(', ')}. Add missing values to .env.local or export them before rerunning.\`); + if (missing.length === 0) return; + if (process.env.START_FROM) { + console.warn(\`[ricky] Skipping env-var assertion (--start-from active): missing \${missing.join(', ')}. Resumed steps that actually need these will fail with their own error.\`); + return; } + throw new Error(\`MISSING_ENV_VAR: \${missing.join(', ')}. Add missing values to .env.local or export them before rerunning.\`); }`; }