fix: address P1 double-substitution from cubic/coderabbit review

Both bots flagged a backward-compat regression: ${VAR} pass runs after
{env:VAR} pass, so an env value containing literal ${X} got expanded in
the second pass. Reordering only fixed one direction (reverse cascade
still possible when ${VAR}'s output contains {env:Y}).

Correct fix: single-pass substitution with one regex alternation that
evaluates all three patterns against the ORIGINAL text. Output of any
pattern cannot be re-matched by another.

Single regex handles:
  $${VAR} or $${VAR:-default}  → literal escape
  (?<!$)${VAR}[:-default]       → JSON-safe substitution
  {env:VAR}                         → raw text injection

Regression tests added for both cascade directions:
  - env.A="${B}" + {env:A} → literal ${B} stays
  - env.A="{env:B}" + ${A} → literal {env:B} stays

All 34 tests pass. Typecheck + marker guard clean.

Author: anandgupta42

packages/opencode/src/config/paths.ts

@@ -86,43 +86,49 @@ export namespace ConfigPaths {
 
   /** Apply {env:VAR} and {file:path} substitutions to config text. */
   async function substitute(text: string, input: ParseSource, missing: "error" | "empty" = "error") {
-    // altimate_change start — track interpolation stats for telemetry
-    let legacyBraceRefs = 0
-    let legacyBraceUnresolved = 0
-    text = text.replace(/\{env:([^}]+)\}/g, (_, varName) => {
-      legacyBraceRefs++
-      const v = process.env[varName]
-      if (v === undefined || v === "") legacyBraceUnresolved++
-      return v || ""
-    })
-    // altimate_change end
-    // altimate_change start — accept ${VAR} shell/dotenv syntax as alias for {env:VAR}
-    // Users arriving from Claude Code / VS Code / dotenv / docker-compose expect this
-    // convention. Only matches POSIX identifier names to avoid collisions with random
-    // ${...} content. Value is JSON-escaped so it can't break out of the enclosing
-    // string — use {env:VAR} for raw unquoted injection. Supports ${VAR:-default}
-    // for fallback values (docker-compose / POSIX shell convention: default used when
-    // the variable is unset OR empty). Docker-compose convention: $${VAR} escapes to
-    // literal ${VAR}. See issue #635.
+    // altimate_change start — unified env-var interpolation
+    // Single-pass substitution against the ORIGINAL text prevents output of one
+    // pattern being re-matched by another (e.g. {env:A}="${B}" expanding B).
+    // Syntaxes (order tried, in one regex via alternation):
+    //   1. $${VAR} or $${VAR:-default} — literal escape (docker-compose style)
+    //   2. ${VAR} or ${VAR:-default}   — string-safe, JSON-escaped (shell/dotenv)
+    //   3. {env:VAR}                    — raw text injection (backward compat)
+    // Users arriving from Claude Code / VS Code / dotenv / docker-compose expect
+    // ${VAR}. Use {env:VAR} for raw unquoted injection. See issue #635.
     let dollarRefs = 0
     let dollarUnresolved = 0
     let dollarDefaulted = 0
-    text = text.replace(/(?<!\$)\$\{([A-Za-z_][A-Za-z0-9_]*)(?::-([^}]*))?\}/g, (_, varName, fallback) => {
-      dollarRefs++
-      const envValue = process.env[varName]
-      const resolved = envValue !== undefined && envValue !== ""
-      if (!resolved && fallback !== undefined) dollarDefaulted++
-      if (!resolved && fallback === undefined) dollarUnresolved++
-      const value = resolved ? envValue : (fallback ?? "")
-      return JSON.stringify(value).slice(1, -1)
-    })
-    // Unescape: $${VAR} → ${VAR} (user-authored literal preservation, docker-compose style)
-    // Handles both ${VAR} and ${VAR:-default} forms.
     let dollarEscaped = 0
-    text = text.replace(/\$\$(\{[A-Za-z_][A-Za-z0-9_]*(?::-[^}]*)?\})/g, (_, rest) => {
-      dollarEscaped++
-      return "$" + rest
-    })
+    let legacyBraceRefs = 0
+    let legacyBraceUnresolved = 0
+    text = text.replace(
+      /\$\$(\{[A-Za-z_][A-Za-z0-9_]*(?::-[^}]*)?\})|(?<!\$)\$\{([A-Za-z_][A-Za-z0-9_]*)(?::-([^}]*))?\}|\{env:([^}]+)\}/g,
+      (match, escaped, dollarVar, dollarDefault, braceVar) => {
+        if (escaped !== undefined) {
+          // $${VAR} → literal ${VAR}
+          dollarEscaped++
+          return "$" + escaped
+        }
+        if (dollarVar !== undefined) {
+          // ${VAR} / ${VAR:-default} → JSON-escaped string-safe substitution
+          dollarRefs++
+          const envValue = process.env[dollarVar]
+          const resolved = envValue !== undefined && envValue !== ""
+          if (!resolved && dollarDefault !== undefined) dollarDefaulted++
+          if (!resolved && dollarDefault === undefined) dollarUnresolved++
+          const value = resolved ? envValue : (dollarDefault ?? "")
+          return JSON.stringify(value).slice(1, -1)
+        }
+        if (braceVar !== undefined) {
+          // {env:VAR} → raw text injection
+          legacyBraceRefs++
+          const v = process.env[braceVar]
+          if (v === undefined || v === "") legacyBraceUnresolved++
+          return v || ""
+        }
+        return match
+      },
+    )
     // Emit telemetry if any env interpolation happened. Dynamic import avoids a
     // circular dep with @/altimate/telemetry (which imports @/config/config).
     if (dollarRefs > 0 || legacyBraceRefs > 0 || dollarEscaped > 0) {

packages/opencode/test/config/paths-parsetext.test.ts

@@ -250,6 +250,37 @@ describe("ConfigPaths.parseText: ${VAR} substitution (shell/dotenv alias)", () =
     }
   })
 
+  test("single-pass: {env:A} value containing ${B} stays literal (no cascade)", async () => {
+    // Regression test for cubic/coderabbit P1: previously the {env:VAR} pass ran
+    // first, then the ${VAR} pass expanded any ${...} in its output. Single-pass
+    // substitution evaluates both patterns against the ORIGINAL text only.
+    process.env.OPENCODE_TEST_CASCADE_A = "${OPENCODE_TEST_CASCADE_B}"
+    process.env.OPENCODE_TEST_CASCADE_B = "should-not-expand"
+    try {
+      const text = '{"value": "{env:OPENCODE_TEST_CASCADE_A}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      // {env:VAR} is raw injection — its output is NOT re-interpolated
+      expect(result.value).toBe("${OPENCODE_TEST_CASCADE_B}")
+    } finally {
+      delete process.env.OPENCODE_TEST_CASCADE_A
+      delete process.env.OPENCODE_TEST_CASCADE_B
+    }
+  })
+
+  test("single-pass: ${A} value containing {env:B} stays literal (no cascade)", async () => {
+    // Reverse direction: ${VAR} output must not be matched by {env:VAR} pass.
+    process.env.OPENCODE_TEST_CASCADE_C = "{env:OPENCODE_TEST_CASCADE_D}"
+    process.env.OPENCODE_TEST_CASCADE_D = "should-not-expand"
+    try {
+      const text = '{"value": "${OPENCODE_TEST_CASCADE_C}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      expect(result.value).toBe("{env:OPENCODE_TEST_CASCADE_D}")
+    } finally {
+      delete process.env.OPENCODE_TEST_CASCADE_C
+      delete process.env.OPENCODE_TEST_CASCADE_D
+    }
+  })
+
   test("works inside MCP environment config (issue #635 regression)", async () => {
     process.env.OPENCODE_TEST_GITLAB_TOKEN = "glpat-xxxxx"
     try {