Repository: stateful/vscode-runme
Pull request #654 (2023-07-03 21:06:55)

don't prompt for preexisting environment variables
Ignores `export` lines if they would override an environment variable in the system store or `.env` files.

Solves #643


Patch:
diff --git a/package-lock.json b/package-lock.json
index 19d70563..2cc338a0 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
 {
   "name": "runme",
-  "version": "1.6.1",
+  "version": "1.7.0-edge.1",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "runme",
-      "version": "1.6.1",
+      "version": "1.7.0-edge.1",
       "hasInstallScript": true,
       "license": "Apache-2.0",
       "dependencies": {
diff --git a/package.json b/package.json
index f9c937dc..d0dbfd17 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "runme",
-  "version": "1.6.1",
+  "version": "1.7.0-edge.1",
   "publisher": "stateful",
   "displayName": "Runme runs Markdown",
   "description": "Execute commands inside your runbooks, docs, and READMEs.",
diff --git a/src/extension/executors/runner.ts b/src/extension/executors/runner.ts
index 0cd78edc..a7df6a67 100644
--- a/src/extension/executors/runner.ts
+++ b/src/extension/executors/runner.ts
@@ -87,7 +87,12 @@ export async function executeRunner(
 
   let cellText = exec.cell.document.getText()
 
-  const commands = await parseCommandSeq(cellText, promptEnv, prepareCmdSeq)
+  const commands = await parseCommandSeq(
+    cellText,
+    promptEnv,
+    new Set([...(environment?.initialEnvs() ?? []), ...Object.keys(envs)]),
+    prepareCmdSeq
+  )
   if (!commands) {
     return false
   }
diff --git a/src/extension/executors/utils.ts b/src/extension/executors/utils.ts
index bc1d1bfe..cf11f4ae 100644
--- a/src/extension/executors/utils.ts
+++ b/src/extension/executors/utils.ts
@@ -301,6 +301,7 @@ function resolveOrAbsolute(parent?: string, child?: string): string | undefined
 export async function parseCommandSeq(
   cellText: string,
   promptForEnv = DEFAULT_PROMPT_ENV,
+  skipEnvs?: Set<string>,
   parseBlock?: (block: string) => string[]
 ): Promise<string[] | undefined> {
   parseBlock ??= (s) => (s ? s.split('\n') : [])
@@ -322,11 +323,18 @@ export async function parseCommandSeq(
   let offset = 0
 
   for (const { hasStringValue, key, match, type, value, regexpMatch } of exportMatches) {
-    let userValue: string
+    let userValue: string | undefined
+
+    let skip = false
 
     switch (type) {
       case 'prompt':
         {
+          if (skipEnvs?.has(key)) {
+            skip = true
+            break
+          }
+
           const userInput = await promptUserForVariable(key, value, hasStringValue)
 
           if (userInput === undefined) {
@@ -351,7 +359,9 @@ export async function parseCommandSeq(
     const prior = cellText.slice(offset, regexpMatch.index)
     parsedCommandBlocks.push({ type: 'block', content: prior })
 
-    parsedCommandBlocks.push({ type: 'single', content: `export ${key}="${userValue}"` })
+    if (!skip && userValue !== undefined) {
+      parsedCommandBlocks.push({ type: 'single', content: `export ${key}="${userValue}"` })
+    }
 
     offset = regexpMatch.index + match.length
   }
diff --git a/src/extension/kernel.ts b/src/extension/kernel.ts
index 594220be..761c16b9 100644
--- a/src/extension/kernel.ts
+++ b/src/extension/kernel.ts
@@ -557,6 +557,7 @@ export class Kernel implements Disposable {
           if (this.runner !== runner) {
             return
           }
+
           this.environment = env
         } catch (e: any) {
           window.showErrorMessage(`Failed to create environment for gRPC Runner: ${e.message}`)
diff --git a/src/extension/provider/runmeTask.ts b/src/extension/provider/runmeTask.ts
index 0852d783..64655de8 100644
--- a/src/extension/provider/runmeTask.ts
+++ b/src/extension/provider/runmeTask.ts
@@ -128,13 +128,18 @@ export class RunmeTaskProvider implements TaskProvider {
       new CustomExecution(async () => {
         const cwd = options.cwd || (await getCellCwd(cell, notebook, Uri.file(filePath)))
 
-        const cellContent = 'value' in cell ? cell.value : cell.document.getText()
-        const commands = await parseCommandSeq(cellContent, undefined, prepareCmdSeq)
-
         const envs: Record<string, string> = {
           ...(await getWorkspaceEnvs(Uri.file(filePath))),
         }
 
+        const cellContent = 'value' in cell ? cell.value : cell.document.getText()
+        const commands = await parseCommandSeq(
+          cellContent,
+          undefined,
+          new Set([...(environment?.initialEnvs() ?? []), ...Object.keys(envs)]),
+          prepareCmdSeq
+        )
+
         if (!environment) {
           Object.assign(envs, process.env)
         }
diff --git a/src/extension/runner.ts b/src/extension/runner.ts
index acedc79a..b7821440 100644
--- a/src/extension/runner.ts
+++ b/src/extension/runner.ts
@@ -23,6 +23,7 @@ import {
 import { RunnerServiceClient } from './grpc/client'
 import { getSystemShellPath } from './executors/utils'
 import RunmeServer from './server/runmeServer'
+import { convertEnvList } from './utils'
 
 type ExecuteDuplex = DuplexStreamingCall<ExecuteRequest, ExecuteResponse>
 
@@ -81,7 +82,9 @@ export interface IRunner extends Disposable {
 
 interface IRunnerChild extends DisposableAsync {}
 
-export interface IRunnerEnvironment extends IRunnerChild {}
+export interface IRunnerEnvironment extends IRunnerChild {
+  initialEnvs(): Set<string>
+}
 
 export type RunnerExitReason =
   | {
@@ -253,12 +256,7 @@ export class GrpcRunner implements IRunner {
       return undefined
     }
 
-    return session.envs.reduce((prev, curr) => {
-      const [key, value = ''] = curr.split(/\=(.*)/s)
-      prev[key] = value
-
-      return prev
-    }, {} as Record<string, string>)
+    convertEnvList(session.envs)
   }
 
   // TODO: create a gRPC endpoint for this so it can be done without making a
@@ -744,7 +742,11 @@ export class GrpcRunnerProgramSession implements IRunnerProgramSession {
 }
 
 export class GrpcRunnerEnvironment implements IRunnerEnvironment {
-  constructor(private readonly client: RunnerServiceClient, private readonly session: Session) {}
+  initialEnvKeys: Set<string>
+
+  constructor(private readonly client: RunnerServiceClient, private readonly session: Session) {
+    this.initialEnvKeys = new Set(Object.keys(convertEnvList(session.envs)))
+  }
 
   getRunmeSession(): Session {
     return this.session
@@ -761,6 +763,10 @@ export class GrpcRunnerEnvironment implements IRunnerEnvironment {
   private async delete() {
     return await this.client.deleteSession({ id: this.getSessionId() })
   }
+
+  initialEnvs(): Set<string> {
+    return this.initialEnvKeys
+  }
 }
 
 function terminalDimensionsToWinsize({ rows, columns }: TerminalDimensions): Winsize {
diff --git a/src/extension/utils.ts b/src/extension/utils.ts
index 45b2cbc4..5f52c0e0 100644
--- a/src/extension/utils.ts
+++ b/src/extension/utils.ts
@@ -405,6 +405,7 @@ export function getWorkspaceFolder(uri?: Uri): WorkspaceFolder | undefined {
   } while (testPath !== path.dirname(testPath))
 }
 
+// TODO: use gRPC project mode for this
 export async function getWorkspaceEnvs(uri?: Uri): Promise<Record<string, string>> {
   const res: Record<string, string> = {}
   const workspaceFolder = getWorkspaceFolder(uri)
@@ -544,3 +545,12 @@ export async function fileOrDirectoryExists(path: Uri): Promise<boolean> {
 export function isMultiRootWorkspace(): boolean {
   return (workspace.workspaceFolders && workspace.workspaceFolders.length > 1) || false
 }
+
+export function convertEnvList(envs: string[]): Record<string, string | undefined> {
+  return envs.reduce((prev, curr) => {
+    const [key, value = ''] = curr.split(/\=(.*)/s)
+    prev[key] = value
+
+    return prev
+  }, {} as Record<string, string | undefined>)
+}


Test patch:
diff --git a/tests/extension/executors/utils.test.ts b/tests/extension/executors/utils.test.ts
index f94f0f50..0a83d7e9 100644
--- a/tests/extension/executors/utils.test.ts
+++ b/tests/extension/executors/utils.test.ts
@@ -312,7 +312,7 @@ suite('parseCommandSeq', () => {
       'echo $TEST_MULTILINE'
     ]
 
-    const res = await parseCommandSeq(cmdLines.join('\n'), true, getCmdSeq)
+    const res = await parseCommandSeq(cmdLines.join('\n'), true, undefined, getCmdSeq)
 
     expect(res).toBeTruthy()
     expect(res).toStrictEqual([
diff --git a/tests/extension/runner.test.ts b/tests/extension/runner.test.ts
index 42cae839..ec245a25 100644
--- a/tests/extension/runner.test.ts
+++ b/tests/extension/runner.test.ts
@@ -21,6 +21,12 @@ vi.mock('../../src/extension/utils', () => ({
   getGrpcHost: vi.fn().mockReturnValue('127.0.0.1:7863'),
   getAnnotations: vi.fn().mockReturnValue({ }),
   isWindows: vi.fn().mockReturnValue(false),
+  convertEnvList: vi.fn().mockImplementation((envs: string[]) => envs.reduce((prev, curr) => {
+    const [key, value = ''] = curr.split(/\=(.*)/s)
+    prev[key] = value
+
+    return prev
+  }, {} as Record<string, string | undefined>)),
 }))
 
 vi.mock('vscode', async () => ({
@@ -41,8 +47,9 @@ let id = 0
 
 const resetId = () => { id = 0 }
 
-const createSession = vi.fn(() => ({
-  id: (id++).toString()
+const createSession = vi.fn((envs?: string[]) => ({
+  id: (id++).toString(),
+  envs: [...envs ?? [], 'foo=bar'],
 }))
 
 const deleteSession = vi.fn(async () => ({
@@ -72,10 +79,10 @@ vi.mock('../../src/extension/grpc/client', () => ({
   RunnerServiceClient: class {
     constructor() {}
 
-    async createSession() {
+    async createSession(request: { envs?: string[] }) {
       return {
         response: {
-          session: createSession()
+          session: createSession(request.envs)
         }
       }
     }
@@ -145,6 +152,16 @@ suite('grpc Runner', () => {
     expect(deleteSession).toBeCalledTimes(1)
   })
 
+  test('environment has loaded variables', async () => {
+    const { runner } = createGrpcRunner()
+    const environment = await runner.createEnvironment(['bar=baz'])
+
+    const initialEnvs = environment.initialEnvs()
+
+    // { 'foo': 'bar', 'bar': 'baz' }
+    expect(initialEnvs).toStrictEqual(new Set(['foo', 'bar']))
+  })
+
   test('cannot create environment if server not initialized', async () => {
     const { runner } = createGrpcRunner(false)
     await expect(runner.createEnvironment()).rejects.toThrowError('Client is not active!')
diff --git a/tests/extension/utils.test.ts b/tests/extension/utils.test.ts
index f7114de5..82692643 100644
--- a/tests/extension/utils.test.ts
+++ b/tests/extension/utils.test.ts
@@ -26,7 +26,8 @@ import {
   bootFile,
   isShellLanguage,
   fileOrDirectoryExists,
-  isMultiRootWorkspace
+  isMultiRootWorkspace,
+  convertEnvList,
 } from '../../src/extension/utils'
 import { ENV_STORE, DEFAULT_ENV } from '../../src/extension/constants'
 import { CellAnnotations } from '../../src/types'
@@ -671,3 +672,19 @@ suite('isMultiRootWorkspace', () => {
     expect(isMultiRootWorkspace()).toStrictEqual(false)
   })
 })
+
+suite('convertEnvList', () => {
+  test('can handle basic example', () => {
+    expect(
+      convertEnvList([
+        'a=1',
+        'b=2',
+        'c=3\n4',
+      ])
+    ).toStrictEqual({
+      'a': '1',
+      'b': '2',
+      'c': '3\n4',
+    })
+  })
+})