fix(setup): harden mint cleanup, Windows browser, container detection, helm cwd

Review findings from #5911:
- a post-mint completeApproval failure no longer routes into releaseMint — the
  mint lock now outlives the approval (shared TTL), so a cleanup blip can't leave
  a re-mintable window and orphan a key; cleanup is best-effort after the key ships
- compose doctor --fix writes the feature-flag twin to the layout's primary env
  (root .env on a compose install), not always apps/sim/.env
- Windows opens the browser via `cmd /c start "" <url>` — `start` is a shell
  builtin, so spawning it directly ENOENT'd and the handoff never opened
- managed-container detection filters loosely and pins the exact name in code;
  Docker's `name=^x$` anchor matches the internal `/x` form and often missed,
  skipping the reuse branch
- the shared helm/kind run helper pins cwd to the repo root, matching helm test,
  so `helm upgrade --install ./helm/sim` works from any working directory

Author: TheodoreSpeaks

apps/sim/app/api/cli/auth/poll/route.test.ts

@@ -81,6 +81,19 @@ describe('POST /api/cli/auth/poll', () => {
     expect(mockCompleteApproval).not.toHaveBeenCalled()
   })
 
+  it('still returns the key when post-mint cleanup fails — never releases the lock', async () => {
+    mockPollApproval.mockResolvedValue({ status: 'approved', userId: 'user-1' })
+    mockCompleteApproval.mockRejectedValue(new Error('redis blip'))
+    const response = await POST(pollRequest({ request: REQUEST, verifier: VERIFIER }))
+    expect(response.status).toBe(200)
+    await expect(response.json()).resolves.toEqual({
+      status: 'complete',
+      key: { id: 'key-1', apiKey: 'sk-test' },
+    })
+    // A cleanup failure must not release the mint lock — that would allow a re-mint.
+    expect(mockReleaseMint).not.toHaveBeenCalled()
+  })
+
   it('rejects a malformed verifier before touching the store', async () => {
     const response = await POST(pollRequest({ request: REQUEST, verifier: 'too-short' }))
     expect(response.status).toBe(400)

apps/sim/app/api/cli/auth/poll/route.ts

@@ -35,17 +35,28 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
     return NextResponse.json({ status: 'pending' })
   }
 
+  let key: Awaited<ReturnType<typeof generateCopilotApiKey>>
   try {
-    const key = await generateCopilotApiKey(result.userId, cliKeyName())
-    await completeApproval(requestId)
-    logger.info('Minted CLI key on approved poll', { userId: result.userId })
-    return NextResponse.json({ status: 'complete', key })
+    key = await generateCopilotApiKey(result.userId, cliKeyName())
   } catch (error) {
+    // Mint failed — release the reservation so a later poll can retry.
     await releaseMint(requestId)
     const status = error instanceof CopilotApiKeyError ? error.upstreamStatus : undefined
     return NextResponse.json(
       { error: 'Failed to generate copilot API key' },
       { status: status ?? 500 }
     )
   }
+
+  // Mint succeeded — the key exists. Consuming the approval is best-effort: a
+  // cleanup failure must NOT release the lock (that would let a later poll mint
+  // a second, orphaned key). The record and lock share a TTL and expire together.
+  await completeApproval(requestId).catch((error) => {
+    logger.error('Failed to consume CLI approval after minting', {
+      error,
+      userId: result.userId,
+    })
+  })
+  logger.info('Minted CLI key on approved poll', { userId: result.userId })
+  return NextResponse.json({ status: 'complete', key })
 })

apps/sim/lib/cli-auth/approval-store.test.ts

@@ -69,10 +69,11 @@ describe('cli-auth approval store', () => {
         status: 'approved',
         userId: 'user-1',
       })
-      // NX lock on the mint key with a TTL; the approval itself is NOT deleted here.
+      // NX lock on the mint key; TTL matches the approval so they expire together
+      // and a failed cleanup can't leave a re-mintable window. Record not deleted here.
       const [key, val, px, ttl, nx] = mockSet.mock.calls[0]
       expect(key).toContain('cli:auth:mint:')
-      expect([val, px, ttl, nx]).toEqual(['1', 'PX', 30_000, 'NX'])
+      expect([val, px, ttl, nx]).toEqual(['1', 'PX', 120_000, 'NX'])
       expect(mockDel).not.toHaveBeenCalled()
     })
 

apps/sim/lib/cli-auth/approval-store.ts

@@ -17,7 +17,12 @@ import { getRedisClient } from '@/lib/core/config/redis'
  */
 
 const APPROVAL_TTL_MS = 120_000
-const MINT_LOCK_TTL_MS = 30_000
+// The mint lock must outlive the approval it guards: if a mint succeeds but the
+// cleanup delete fails, the still-held lock is what stops a later poll from
+// re-minting the now-orphaned key. Matching the approval TTL means the record
+// and the lock expire together, so there is never a window where the approval
+// is redeemable but the lock is gone.
+const MINT_LOCK_TTL_MS = APPROVAL_TTL_MS
 
 interface ApprovalRecord {
   /** BASE64URL(SHA256(pollSecret)) — the CLI proves possession of the secret at poll time. */

scripts/setup/checks.ts

@@ -375,7 +375,8 @@ function checkCoherence(ctx: CheckContext): Finding[] {
       status: 'fail',
       message: `${setSide} is set but its twin ${missingSide} disagrees — server and browser will render different features`,
       fix: `doctor --fix sets ${missingSide}=${value}`,
-      autofix: () => writeEnvValues('sim', { [missingSide]: value }),
+      // Write to the layout's primary env (root on a compose install), not always sim.
+      autofix: () => writeEnvValues(sim.target, { [missingSide]: value }),
     })
   }
 

scripts/setup/cli-auth.ts

@@ -11,8 +11,13 @@ const POLL_INTERVAL_MS = 2000
 
 function openBrowser(url: string): void {
   if (process.env.SIM_SETUP_NO_BROWSER) return
-  const command =
-    process.platform === 'darwin' ? 'open' : process.platform === 'win32' ? 'start' : 'xdg-open'
+  if (process.platform === 'win32') {
+    // `start` is a cmd builtin, not an executable — spawning it directly ENOENTs.
+    // The empty '' is start's window-title argument, which it needs before the URL.
+    spawnSync('cmd', ['/c', 'start', '', url], { stdio: 'ignore' })
+    return
+  }
+  const command = process.platform === 'darwin' ? 'open' : 'xdg-open'
   spawnSync(command, [url], { stdio: 'ignore' })
 }
 

scripts/setup/detect.ts

@@ -71,16 +71,23 @@ function commandOutput(command: string, args: string[]): string | null {
 
 function detectContainer(dockerRunning: boolean, name: string): Detection['dbContainer'] {
   if (!dockerRunning) return null
+  // Docker's `name=^x$` anchor matches against the internal `/x` form and often
+  // misses, so filter loosely (substring) and pin the exact name in code.
   const out = commandOutput('docker', [
     'ps',
     '-a',
     '--filter',
-    `name=^${name}$`,
+    `name=${name}`,
     '--format',
-    '{{.State}}\t{{.Labels}}',
+    '{{.Names}}\t{{.State}}\t{{.Labels}}',
   ])
   if (!out) return null
-  const [state, labels = ''] = out.split('\t')
+  const row = out
+    .split('\n')
+    .map((line) => line.split('\t'))
+    .find(([containerName]) => containerName === name)
+  if (!row) return null
+  const [, state, labels = ''] = row
   return {
     state: state === 'running' ? 'running' : 'stopped',
     managed: labels.includes(MANAGED_LABEL),

scripts/setup/modes/k8s.ts

@@ -16,7 +16,9 @@ const LOCAL_CONTEXT_PREFIXES = ['kind-', 'docker-desktop', 'minikube', 'orbstack
  * by any process on the machine, so secrets must never travel that way.
  */
 function run(command: string, args: string[], failMessage: string, input?: string): string {
-  const result = spawnSync(command, args, { encoding: 'utf8', input })
+  // `helm upgrade --install ./helm/sim` uses chart paths relative to the repo
+  // root, so pin cwd regardless of where the wizard was invoked from.
+  const result = spawnSync(command, args, { encoding: 'utf8', input, cwd: ROOT })
   if (result.status !== 0) {
     throw new Error(`${failMessage}: ${result.stderr.trim() || result.stdout.trim()}`)
   }