feat: warn on unbound shell variables

Author: tbrandenburg

src/generation/shell-generator.test.ts

@@ -0,0 +1,50 @@
+import { describe, expect, it } from 'vitest';
+
+import { FlowshWorkflow } from '../dsl/types.js';
+import { generateShellScript } from './shell-generator.js';
+
+const createWorkflow = (command: string, environmentVariables: string[] = []): FlowshWorkflow => {
+  return {
+    metadata: { name: 'Test Workflow' },
+    environment_variables: environmentVariables.map(variable => ({ variable })),
+    graph: {
+      nodes: [
+        { id: 'start', type: 'start', data: { title: 'Start' } },
+        { id: 'code', type: 'code', data: { command } },
+        { id: 'end', type: 'end', data: { title: 'End' } },
+      ],
+      edges: [
+        { id: 'start-to-code', source: 'start', target: 'code' },
+        { id: 'code-to-end', source: 'code', target: 'end' },
+      ],
+    },
+  };
+};
+
+describe('generateShellScript', () => {
+  it('warns about potentially unbound variables referenced in the script', () => {
+    const workflow = createWorkflow('echo "$UNBOUND_VAR"');
+    const result = generateShellScript(workflow);
+
+    expect(result.success).toBe(true);
+    expect(result.warnings).toContain(
+      "Potentially unbound variable 'UNBOUND_VAR' referenced in generated script"
+    );
+  });
+
+  it('does not warn when variables are declared in environment_variables', () => {
+    const workflow = createWorkflow('echo "$KNOWN_VAR"', ['KNOWN_VAR']);
+    const result = generateShellScript(workflow);
+
+    expect(result.success).toBe(true);
+    expect(result.warnings.some(warning => warning.includes('KNOWN_VAR'))).toBe(false);
+  });
+
+  it('does not warn for known shell variables', () => {
+    const workflow = createWorkflow('echo "$BASH_VERSION"');
+    const result = generateShellScript(workflow);
+
+    expect(result.success).toBe(true);
+    expect(result.warnings.some(warning => warning.includes('BASH_VERSION'))).toBe(false);
+  });
+});

src/generation/shell-generator.ts

@@ -255,14 +255,10 @@ export function generateShellScript(
       progressTracker.complete();
     }
 
-    const provisionalScript = scriptParts.filter(part => part.trim() !== '').join('\n\n');
-    const internalVariableExports = collectInternalVariableExports(provisionalScript);
-    if (internalVariableExports) {
-      const insertIndex = variableSetup.trim() ? 2 : 1;
-      scriptParts.splice(insertIndex, 0, internalVariableExports);
-    }
-
     const script = scriptParts.filter(part => part.trim() !== '').join('\n\n');
+    const declaredVariables = collectDeclaredVariables(workflow, allVariables);
+    const unboundWarnings = detectPotentialUnboundVariables(script, declaredVariables);
+    warnings.push(...unboundWarnings);
 
     if (monitor) {
       monitor.finish(true);
@@ -475,11 +471,64 @@ function generateVariableSetup(workflow: FlowshWorkflow, variables: Map<string,
 }
 
 /**
- * Collect internal variable assignments and export them for subshell usage.
+ * Collect variables declared through workflow configuration and node registration.
  */
-function collectInternalVariableExports(shellScript: string): string {
-  const exportPattern = /^\s*export\s+([A-Z_][A-Z0-9_]*)/gm;
-  const assignmentPattern = /^(?!\s*(?:local|declare|typeset)\b)\s*([A-Z_][A-Z0-9_]*)=/;
+function collectDeclaredVariables(
+  workflow: FlowshWorkflow,
+  variables: Map<string, string>
+): Set<string> {
+  const declared = new Set<string>(variables.keys());
+  const envVars = workflow.environment_variables || workflow.spec?.environment_variables || [];
+  const conversationVars =
+    workflow.conversation_variables || workflow.spec?.conversation_variables || [];
+
+  for (const envVar of envVars) {
+    if (envVar.variable) {
+      declared.add(envVar.variable);
+    }
+  }
+
+  for (const convVar of conversationVars) {
+    if (convVar.variable) {
+      declared.add(convVar.variable);
+    }
+  }
+
+  return declared;
+}
+
+function detectPotentialUnboundVariables(
+  shellScript: string,
+  declaredVariables: Set<string>
+): string[] {
+  const exportPattern = /^\s*export\s+([A-Z_][A-Z0-9_]*)(?:=|$)/gm;
+  const assignmentPattern = /^(?!\s*(?:local|declare|typeset|readonly)\b)\s*([A-Z_][A-Z0-9_]*)=/;
+  const referencePattern = /\$\{?([A-Z_][A-Z0-9_]*)\}?/g;
+  const knownShellVars = new Set([
+    'BASH_VERSION',
+    'BASH_SOURCE',
+    'BASH_LINENO',
+    'FUNCNAME',
+    'LINENO',
+    'PWD',
+    'OLDPWD',
+    'HOME',
+    'PATH',
+    'IFS',
+    'SHELLOPTS',
+    'UID',
+    'EUID',
+    'PPID',
+    'SHLVL',
+    'OSTYPE',
+    'MACHTYPE',
+    'HOSTNAME',
+    'HOSTTYPE',
+    'TERM',
+    'COLUMNS',
+    'LINES',
+    'RANDOM',
+  ]);
 
   const exportedVars = new Set<string>();
   for (const match of shellScript.matchAll(exportPattern)) {
@@ -488,26 +537,45 @@ function collectInternalVariableExports(shellScript: string): string {
     }
   }
 
-  const foundVars: string[] = [];
-  const seenVars = new Set<string>();
+  const assignedVars = new Set<string>();
   for (const line of shellScript.split('\n')) {
-    const match = line.match(assignmentPattern);
-    if (!match || !match[1]) {
+    const trimmed = line.trim();
+    if (!trimmed || trimmed.startsWith('#')) {
       continue;
     }
-    const varName = match[1];
-    if (!exportedVars.has(varName) && !seenVars.has(varName)) {
-      seenVars.add(varName);
-      foundVars.push(varName);
+    const match = trimmed.match(assignmentPattern);
+    if (match?.[1]) {
+      assignedVars.add(match[1]);
     }
   }
 
-  if (foundVars.length === 0) {
-    return '';
+  const warningVars = new Set<string>();
+  for (const line of shellScript.split('\n')) {
+    const trimmed = line.trim();
+    if (!trimmed || trimmed.startsWith('#')) {
+      continue;
+    }
+    const matches = trimmed.matchAll(referencePattern);
+    for (const match of matches) {
+      const varName = match[1];
+      if (!varName) {
+        continue;
+      }
+      if (
+        declaredVariables.has(varName) ||
+        assignedVars.has(varName) ||
+        exportedVars.has(varName) ||
+        knownShellVars.has(varName)
+      ) {
+        continue;
+      }
+      warningVars.add(varName);
+    }
   }
 
-  const exportLines = foundVars.map(varName => `export ${varName}`);
-  return `# Internal Variable Exports\n${exportLines.join('\n')}`;
+  return [...warningVars].sort().map(varName => {
+    return `Potentially unbound variable '${varName}' referenced in generated script`;
+  });
 }
 
 /**