fix(auth): address review feedback

skulidropek · skulidropek · commit a1056419d495 · 2026-05-20T16:16:51.000Z
diff --git a/packages/api/src/services/auth-account-counts.ts b/packages/api/src/services/auth-account-counts.ts
@@ -13,6 +13,8 @@ type HasCredentials = (
 const ignoredAuthAccountEntries: ReadonlySet<string> = new Set([".image"])
 const grokEnvApiKeyNames: ReadonlyArray<string> = ["GROK_DEPLOYMENT_KEY", "GROK_API_KEY", "XAI_API_KEY"]
 
+const credentialCount = (connected: boolean): number => connected ? 1 : 0
+
 const hasFileAtPath = (
   fs: FileSystem.FileSystem,
   filePath: string
@@ -235,16 +237,21 @@ export const countCodexCredentialAccounts = (
       return 0
     }
 
-    let count = yield* _(hasCodexAccountCredentials(fs, root), Effect.map((connected) => connected ? 1 : 0))
+    let count = yield* _(
+      hasCodexAccountCredentials(fs, root).pipe(
+        Effect.orElseSucceed(() => false),
+        Effect.map((connected) => credentialCount(connected))
+      )
+    )
     const entries = yield* _(fs.readDirectory(root))
     for (const entry of entries) {
       if (ignoredAuthAccountEntries.has(entry)) {
         continue
       }
 
       const accountPath = path.join(root, entry)
-      const info = yield* _(fs.stat(accountPath))
-      if (info.type !== "Directory") {
+      const info = yield* _(fs.stat(accountPath), Effect.orElseSucceed(() => null))
+      if (info === null || info.type !== "Directory") {
         continue
       }
 
@@ -268,16 +275,21 @@ export const countAuthCredentialAccounts = (
       return 0
     }
 
+    let count = yield* _(
+      hasCredentials(fs, root).pipe(
+        Effect.orElseSucceed(() => false),
+        Effect.map((connected) => credentialCount(connected))
+      )
+    )
     const entries = yield* _(fs.readDirectory(root))
-    let count = 0
     for (const entry of entries) {
       if (ignoredAuthAccountEntries.has(entry)) {
         continue
       }
 
       const accountPath = path.join(root, entry)
-      const info = yield* _(fs.stat(accountPath))
-      if (info.type !== "Directory") {
+      const info = yield* _(fs.stat(accountPath), Effect.orElseSucceed(() => null))
+      if (info === null || info.type !== "Directory") {
         continue
       }
 
diff --git a/packages/api/tests/auth-menu.test.ts b/packages/api/tests/auth-menu.test.ts
@@ -64,14 +64,24 @@ describe("auth menu service", () => {
         yield* _(fs.makeDirectory(path.join(authRoot, "codex", ".image"), { recursive: true }))
         yield* _(fs.makeDirectory(path.join(authRoot, "grok", ".image"), { recursive: true }))
 
+        yield* _(fs.writeFileString(path.join(authRoot, "claude", ".oauth-token"), "root-claude-oauth\n"))
         yield* _(fs.makeDirectory(path.join(authRoot, "claude", "live"), { recursive: true }))
         yield* _(fs.writeFileString(path.join(authRoot, "claude", "live", ".oauth-token"), "claude-oauth\n"))
         yield* _(fs.makeDirectory(path.join(authRoot, "codex"), { recursive: true }))
         yield* _(fs.writeFileString(path.join(authRoot, "codex", "auth.json"), "{\"tokens\":{\"account_id\":\"default\"}}\n"))
         yield* _(fs.makeDirectory(path.join(authRoot, "codex", "work"), { recursive: true }))
         yield* _(fs.writeFileString(path.join(authRoot, "codex", "work", "auth.json"), "{\"tokens\":{\"account_id\":\"work\"}}\n"))
+        yield* _(fs.symlink(path.join(authRoot, "missing-gemini-account"), path.join(authRoot, "gemini", "broken")))
+        yield* _(fs.writeFileString(path.join(authRoot, "gemini", ".api-key"), "root-gemini-key\n"))
         yield* _(fs.makeDirectory(path.join(authRoot, "gemini", "live"), { recursive: true }))
         yield* _(fs.writeFileString(path.join(authRoot, "gemini", "live", ".api-key"), "gemini-key\n"))
+        yield* _(fs.makeDirectory(path.join(authRoot, "grok", ".grok"), { recursive: true }))
+        yield* _(
+          fs.writeFileString(
+            path.join(authRoot, "grok", ".grok", "auth.json"),
+            `${JSON.stringify({ [grokOidcAuthScope]: { key: "root-xai-oauth" } })}\n`
+          )
+        )
         yield* _(fs.makeDirectory(path.join(authRoot, "grok", "live", ".grok"), { recursive: true }))
         yield* _(
           fs.writeFileString(
@@ -93,10 +103,10 @@ describe("auth menu service", () => {
         )
         const snapshot = yield* _(withProjectsRoot(projectsRoot, service.readAuthMenuSnapshot()))
 
-        expect(snapshot.claudeAuthEntries).toBe(1)
+        expect(snapshot.claudeAuthEntries).toBe(2)
         expect(snapshot.codexAuthEntries).toBe(2)
-        expect(snapshot.geminiAuthEntries).toBe(1)
-        expect(snapshot.grokAuthEntries).toBe(1)
+        expect(snapshot.geminiAuthEntries).toBe(2)
+        expect(snapshot.grokAuthEntries).toBe(2)
       })
     ).pipe(Effect.provide(NodeContext.layer)))
 })
diff --git a/packages/app/src/docker-git/api-auth-codec.ts b/packages/app/src/docker-git/api-auth-codec.ts
@@ -79,13 +79,15 @@ const decodeRequiredAuthSnapshot = (snapshot: RawAuthSnapshot): AuthSnapshot | n
   const requiredValues = [
     snapshot.globalEnvPath,
     snapshot.claudeAuthPath,
+    snapshot.codexAuthPath,
     snapshot.geminiAuthPath,
     snapshot.grokAuthPath,
     snapshot.totalEntries,
     snapshot.githubTokenEntries,
     snapshot.gitTokenEntries,
     snapshot.gitUserEntries,
     snapshot.claudeAuthEntries,
+    snapshot.codexAuthEntries,
     snapshot.geminiAuthEntries,
     snapshot.grokAuthEntries
   ]
diff --git a/packages/app/src/docker-git/menu-auth-helpers.ts b/packages/app/src/docker-git/menu-auth-helpers.ts
@@ -19,6 +19,8 @@ type CredentialDirectoryCounterInput = {
 
 const ignoredAuthAccountEntries: ReadonlySet<string> = new Set([".image"])
 
+const credentialCount = (connected: boolean): number => connected ? 1 : 0
+
 export const hasCodexAccountCredentials = (
   fs: FileSystem.FileSystem,
   accountPath: string
@@ -38,8 +40,8 @@ const countCredentialDirectories = ({
         continue
       }
       const accountPath = path.join(root, entry)
-      const info = yield* _(fs.stat(accountPath))
-      if (info.type !== "Directory") {
+      const info = yield* _(fs.stat(accountPath), Effect.orElseSucceed(() => null))
+      if (info === null || info.type !== "Directory") {
         continue
       }
       const connected = yield* _(hasCredentials(fs, accountPath), Effect.orElseSucceed(() => false))
@@ -54,11 +56,20 @@ const countExistingCredentialDirectories = (
   input: CredentialDirectoryCounterInput
 ): Effect.Effect<number, PlatformError> =>
   input.fs.exists(input.root).pipe(
-    Effect.flatMap((exists) =>
-      exists
-        ? countCredentialDirectories(input)
-        : Effect.succeed(0)
-    )
+    Effect.flatMap((exists) => {
+      if (!exists) {
+        return Effect.succeed(0)
+      }
+      return Effect.all({
+        directoryCount: countCredentialDirectories(input),
+        rootCount: input.hasCredentials(input.fs, input.root).pipe(
+          Effect.orElseSucceed(() => false),
+          Effect.map((connected) => credentialCount(connected))
+        )
+      }).pipe(
+        Effect.map(({ directoryCount, rootCount }) => rootCount + directoryCount)
+      )
+    })
   )
 
 export const countAuthCredentialAccounts = (
@@ -73,21 +84,4 @@ export const countCodexCredentialAccounts = (
   path: Path.Path,
   root: string
 ): Effect.Effect<number, PlatformError> =>
-  fs.exists(root).pipe(
-    Effect.flatMap((exists) => {
-      if (!exists) {
-        return Effect.succeed(0)
-      }
-      return Effect.all({
-        directoryCount: countCredentialDirectories({
-          fs,
-          hasCredentials: hasCodexAccountCredentials,
-          path,
-          root
-        }),
-        rootConnected: hasCodexAccountCredentials(fs, root).pipe(Effect.orElseSucceed(() => false))
-      }).pipe(
-        Effect.map(({ directoryCount, rootConnected }) => directoryCount + (rootConnected ? 1 : 0))
-      )
-    })
-  )
+  countExistingCredentialDirectories({ fs, hasCredentials: hasCodexAccountCredentials, path, root })
diff --git a/packages/app/src/web/actions-codex-oauth.ts b/packages/app/src/web/actions-codex-oauth.ts
@@ -21,7 +21,7 @@ export const runCodexOauthMutation = (
     failureMessage: codexLoginFailureMessage,
     markers: codexLoginStreamMarkers,
     runStream: loginCodexStream,
-    startMessage: "Codex OAuth запущен. Следуй инструкциям в Output.",
+    startMessage: "Codex OAuth started. Follow the instructions in Output.",
     successMessage: (label) => `Saved Codex login (${label}).`,
     values
   })
diff --git a/packages/app/src/web/actions-github-oauth.ts b/packages/app/src/web/actions-github-oauth.ts
@@ -16,7 +16,7 @@ export const runGithubOauthMutation = (
       context.reloadDashboard()
     },
     runStream: loginGithubStream,
-    startMessage: "GitHub OAuth запущен. Следуй инструкциям в Output.",
+    startMessage: "GitHub OAuth started. Follow the instructions in Output.",
     successMessage: (label) => `Saved GitHub token (${label}).`,
     values
   })
diff --git a/packages/app/tests/docker-git/actions-codex-oauth.test.ts b/packages/app/tests/docker-git/actions-codex-oauth.test.ts
@@ -59,6 +59,11 @@ describe("web Codex OAuth action", () => {
 
       runCodexOauthMutation({ label: "" }, context)
 
+      expect(setMessage).toHaveBeenNthCalledWith(
+        1,
+        "Codex OAuth started. Follow the instructions in Output."
+      )
+
       yield* _(waitForAssertion(() => {
         expect(context.setAuthSnapshot).toHaveBeenCalledWith(authSnapshot)
       }))
diff --git a/packages/app/tests/docker-git/actions-github-oauth.test.ts b/packages/app/tests/docker-git/actions-github-oauth.test.ts
@@ -67,6 +67,11 @@ describe("web GitHub OAuth action", () => {
 
       runGithubOauthMutation({ label: "" }, context)
 
+      expect(setMessage).toHaveBeenNthCalledWith(
+        1,
+        "GitHub OAuth started. Follow the instructions in Output."
+      )
+
       yield* _(waitForAssertion(() => {
         expect(reloadDashboard).toHaveBeenCalledTimes(1)
       }))
diff --git a/packages/app/tests/docker-git/api-auth-codec.test.ts b/packages/app/tests/docker-git/api-auth-codec.test.ts
@@ -0,0 +1,34 @@
+import { describe, expect, it } from "@effect/vitest"
+
+import { decodeAuthSnapshot } from "../../src/docker-git/api-auth-codec.js"
+import type { JsonValue } from "../../src/docker-git/api-json.js"
+
+const fullAuthSnapshot = {
+  claudeAuthEntries: 1,
+  claudeAuthPath: "/auth/claude",
+  codexAuthEntries: 2,
+  codexAuthPath: "/auth/codex",
+  geminiAuthEntries: 3,
+  geminiAuthPath: "/auth/gemini",
+  gitTokenEntries: 4,
+  gitUserEntries: 5,
+  githubTokenEntries: 6,
+  globalEnvPath: "/env/global.env",
+  grokAuthEntries: 7,
+  grokAuthPath: "/auth/grok",
+  totalEntries: 8
+} satisfies JsonValue
+
+describe("api auth codec", () => {
+  it("requires Codex fields in auth snapshots", () => {
+    const missingPath = { ...fullAuthSnapshot, codexAuthPath: null } satisfies JsonValue
+    const missingEntries = { ...fullAuthSnapshot, codexAuthEntries: null } satisfies JsonValue
+
+    expect(decodeAuthSnapshot({ snapshot: missingPath })).toBe(null)
+    expect(decodeAuthSnapshot({ snapshot: missingEntries })).toBe(null)
+  })
+
+  it("decodes complete auth snapshots", () => {
+    expect(decodeAuthSnapshot({ snapshot: fullAuthSnapshot })).toEqual(fullAuthSnapshot)
+  })
+})
diff --git a/packages/app/tests/docker-git/menu-auth-helpers.test.ts b/packages/app/tests/docker-git/menu-auth-helpers.test.ts
@@ -0,0 +1,68 @@
+/* jscpd:ignore-start */
+import { NodeContext } from "@effect/platform-node"
+import type { PlatformError } from "@effect/platform/Error"
+import * as FileSystem from "@effect/platform/FileSystem"
+import * as Path from "@effect/platform/Path"
+import { describe, expect, it } from "@effect/vitest"
+import { Effect } from "effect"
+import type * as Scope from "effect/Scope"
+
+import { countAuthCredentialAccounts, countCodexCredentialAccounts } from "../../src/docker-git/menu-auth-helpers.js"
+
+const withTempDir = <A, E, R>(
+  use: (tempDir: string) => Effect.Effect<A, E, R>
+): Effect.Effect<A, E | PlatformError, FileSystem.FileSystem | Exclude<R, Scope.Scope>> =>
+  Effect.scoped(
+    Effect.gen(function*(_) {
+      const fs = yield* _(FileSystem.FileSystem)
+      const tempDir = yield* _(
+        fs.makeTempDirectoryScoped({
+          prefix: "docker-git-auth-helpers-"
+        })
+      )
+      return yield* _(use(tempDir))
+    })
+  )
+
+const hasMarkerCredentials = (
+  fs: FileSystem.FileSystem,
+  accountPath: string
+): Effect.Effect<boolean, PlatformError> =>
+  fs.readFileString(`${accountPath}/.token`).pipe(
+    Effect.map((content) => content.trim().length > 0)
+  )
+
+describe("menu auth helpers", () => {
+  it.effect("counts root credentials and skips broken directory entries", () =>
+    withTempDir((root) =>
+      Effect.gen(function*(_) {
+        const fs = yield* _(FileSystem.FileSystem)
+        const path = yield* _(Path.Path)
+        yield* _(fs.makeDirectory(path.join(root, "work"), { recursive: true }))
+        yield* _(fs.writeFileString(path.join(root, ".token"), "root-token\n"))
+        yield* _(fs.writeFileString(path.join(root, "work", ".token"), "work-token\n"))
+        yield* _(fs.symlink(path.join(root, "missing-account"), path.join(root, "broken")))
+
+        const count = yield* _(countAuthCredentialAccounts(fs, path, root, hasMarkerCredentials))
+
+        expect(count).toBe(2)
+      })
+    ).pipe(Effect.provide(NodeContext.layer)))
+
+  it.effect("counts root and labeled Codex credentials", () =>
+    withTempDir((root) =>
+      Effect.gen(function*(_) {
+        const fs = yield* _(FileSystem.FileSystem)
+        const path = yield* _(Path.Path)
+        yield* _(fs.makeDirectory(path.join(root, "work"), { recursive: true }))
+        yield* _(fs.writeFileString(path.join(root, "auth.json"), "{}\n"))
+        yield* _(fs.writeFileString(path.join(root, "work", "auth.json"), "{}\n"))
+        yield* _(fs.symlink(path.join(root, "missing-account"), path.join(root, "broken")))
+
+        const count = yield* _(countCodexCredentialAccounts(fs, path, root))
+
+        expect(count).toBe(2)
+      })
+    ).pipe(Effect.provide(NodeContext.layer)))
+})
+/* jscpd:ignore-end */
diff --git a/packages/lib/src/usecases/auth-grok-oauth.ts b/packages/lib/src/usecases/auth-grok-oauth.ts
@@ -127,7 +127,7 @@ const fixGrokAuthPermissions = (cwd: string, hostPath: string, containerPath: st
  * @param cwd Working directory used for Docker command execution.
  * @param accountPath Selected docker-git Grok account directory.
  * @param options Auth container image and in-container home path.
- * @returns Effect that completes after device authorization writes credentials and permissions are normalized.
+ * @returns Effect that completes after OAuth/browser flow writes credentials and permissions are normalized.
  * @pure false
  * @effect CommandExecutor; invokes Docker and writes credentials under the selected account path.
  * @invariant successful completion leaves credentials scoped to accountPath and not to project source files.
