diff --git a/apps/server/scripts/acp-mock-agent.ts b/apps/server/scripts/acp-mock-agent.ts index bc7828dd854..79657fd6a90 100644 --- a/apps/server/scripts/acp-mock-agent.ts +++ b/apps/server/scripts/acp-mock-agent.ts @@ -35,6 +35,8 @@ const emitStaleXAiPromptCompleteBeforeSecondHang = process.env.T3_ACP_EMIT_STALE_XAI_PROMPT_COMPLETE_BEFORE_SECOND_HANG === "1"; const emitOverlappingXAiPromptCompleteOutOfOrder = process.env.T3_ACP_EMIT_OVERLAPPING_XAI_PROMPT_COMPLETE_OUT_OF_ORDER === "1"; +const failAuthenticate = process.env.T3_ACP_FAIL_AUTHENTICATE === "1"; +const authenticateEmail = process.env.T3_ACP_AUTHENTICATE_EMAIL; const failPrompt = process.env.T3_ACP_FAIL_PROMPT === "1"; const failSetConfigOption = process.env.T3_ACP_FAIL_SET_CONFIG_OPTION === "1"; const exitOnSetConfigOption = process.env.T3_ACP_EXIT_ON_SET_CONFIG_OPTION === "1"; @@ -307,7 +309,11 @@ const program = Effect.gen(function* () { }), ); - yield* agent.handleAuthenticate(() => Effect.succeed({})); + yield* agent.handleAuthenticate(() => + failAuthenticate + ? Effect.fail(AcpError.AcpRequestError.authRequired()) + : Effect.succeed(authenticateEmail ? { _meta: { email: authenticateEmail } } : {}), + ); yield* agent.handleCreateSession(() => Effect.succeed({ diff --git a/apps/server/src/provider/Layers/GrokProvider.test.ts b/apps/server/src/provider/Layers/GrokProvider.test.ts index 000243869c9..c2a315c7031 100644 --- a/apps/server/src/provider/Layers/GrokProvider.test.ts +++ b/apps/server/src/provider/Layers/GrokProvider.test.ts @@ -1,3 +1,7 @@ +// @effect-diagnostics nodeBuiltinImport:off - locates the ACP mock agent for the fake Grok CLI. +import * as NodePath from "node:path"; +import * as NodeURL from "node:url"; + import * as NodeServices from "@effect/platform-node/NodeServices"; import { describe, expect, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; @@ -10,6 +14,39 @@ import { buildInitialGrokProviderSnapshot, checkGrokProviderStatus } from "./Gro const decodeGrokSettings = Schema.decodeSync(GrokSettings); +const mockAgentPath = NodePath.join( + NodePath.dirname(NodeURL.fileURLToPath(import.meta.url)), + "../../../scripts/acp-mock-agent.ts", +); + +// The API-key branch is covered in GrokAcpSupport.test.ts; drop the key here so +// a developer's real credentials cannot change what the probe negotiates. +const { XAI_API_KEY: _ignoredApiKey, ...acpProbeEnv } = process.env; + +/** A `grok` stand-in that answers `--version` itself and defers `agent stdio` to the ACP mock. */ +const writeMockGrokCli = () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const dir = yield* fs.makeTempDirectoryScoped({ prefix: "t3code-grok-acp-" }); + const grokPath = path.join(dir, "grok"); + yield* fs.writeFileString( + grokPath, + [ + "#!/bin/sh", + 'if [ "$1" = "--version" ]; then', + ' printf "grok-cli 0.0.99\\n"', + " exit 0", + "fi", + // @effect-diagnostics-next-line preferSchemaOverJson:off - quotes paths for the shell wrapper. + `exec ${JSON.stringify(process.execPath)} ${JSON.stringify(mockAgentPath)} "$@"`, + "", + ].join("\n"), + ); + yield* fs.chmod(grokPath, 0o755); + return grokPath; + }); + describe("buildInitialGrokProviderSnapshot", () => { it.effect("returns a disabled snapshot when settings.enabled is false", () => Effect.gen(function* () { @@ -107,4 +144,41 @@ it.layer(NodeServices.layer)("checkGrokProviderStatus", (it) => { expect(snapshot.message).toContain("ACP startup failed"); }), ); + + it.effect("reports the account the Grok CLI authenticates as", () => + Effect.gen(function* () { + const snapshot = yield* Effect.scoped( + Effect.gen(function* () { + const grokPath = yield* writeMockGrokCli(); + return yield* checkGrokProviderStatus( + decodeGrokSettings({ enabled: true, binaryPath: grokPath }), + { ...acpProbeEnv, T3_ACP_AUTHENTICATE_EMAIL: "grok-user@example.com" }, + ); + }), + ); + + expect(snapshot.status).toBe("ready"); + expect(snapshot.installed).toBe(true); + expect(snapshot.auth).toEqual({ status: "authenticated", email: "grok-user@example.com" }); + }), + ); + + it.effect("reports an unauthenticated CLI when the agent demands sign-in", () => + Effect.gen(function* () { + const snapshot = yield* Effect.scoped( + Effect.gen(function* () { + const grokPath = yield* writeMockGrokCli(); + return yield* checkGrokProviderStatus( + decodeGrokSettings({ enabled: true, binaryPath: grokPath }), + { ...acpProbeEnv, T3_ACP_FAIL_AUTHENTICATE: "1" }, + ); + }), + ); + + expect(snapshot.status).toBe("error"); + expect(snapshot.installed).toBe(true); + expect(snapshot.auth).toEqual({ status: "unauthenticated" }); + expect(snapshot.message).toContain("not authenticated"); + }), + ); }); diff --git a/apps/server/src/provider/Layers/GrokProvider.ts b/apps/server/src/provider/Layers/GrokProvider.ts index 934eecdb5ae..bf33116c7ab 100644 --- a/apps/server/src/provider/Layers/GrokProvider.ts +++ b/apps/server/src/provider/Layers/GrokProvider.ts @@ -29,7 +29,12 @@ import { enrichProviderSnapshotWithVersionAdvisory, type ProviderMaintenanceCapabilities, } from "../providerMaintenance.ts"; -import { makeGrokAcpRuntime, resolveGrokAcpBaseModelId } from "../acp/GrokAcpSupport.ts"; +import { + grokAuthFailureFromAcpCause, + grokAuthFromAcpAuthenticate, + makeGrokAcpRuntime, + resolveGrokAcpBaseModelId, +} from "../acp/GrokAcpSupport.ts"; const GROK_PRESENTATION = { displayName: "Grok", @@ -123,7 +128,7 @@ function buildGrokDiscoveredModelsFromSessionModelState( .filter((model): model is ServerProviderModel => model !== undefined); } -const discoverGrokModelsViaAcp = ( +const probeGrokViaAcp = ( grokSettings: GrokSettings, environment: NodeJS.ProcessEnv = process.env, ) => @@ -137,7 +142,10 @@ const discoverGrokModelsViaAcp = ( clientInfo: { name: "t3-code-provider-probe", version: "0.0.0" }, }); const started = yield* acp.start(); - return buildGrokDiscoveredModelsFromSessionModelState(started.sessionSetupResult.models); + return { + models: buildGrokDiscoveredModelsFromSessionModelState(started.sessionSetupResult.models), + auth: grokAuthFromAcpAuthenticate(started.authenticateResult, environment), + }; }).pipe(Effect.scoped); const runGrokVersionCommand = ( @@ -251,7 +259,7 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func }); } - const discoveryExit = yield* discoverGrokModelsViaAcp(grokSettings, environment).pipe( + const discoveryExit = yield* probeGrokViaAcp(grokSettings, environment).pipe( Effect.timeoutOption(GROK_ACP_MODEL_DISCOVERY_TIMEOUT_MS), Effect.exit, ); @@ -259,6 +267,7 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func yield* Effect.logWarning("Grok ACP model discovery failed", { errorTag: causeErrorTag(discoveryExit.cause), }); + const authFailure = grokAuthFailureFromAcpCause(discoveryExit.cause); return buildServerProvider({ presentation: GROK_PRESENTATION, enabled: grokSettings.enabled, @@ -268,8 +277,10 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func installed: true, version, status: "error", - auth: { status: "unknown" }, - message: "Grok CLI is installed but ACP startup failed. Check server logs for details.", + auth: authFailure?.auth ?? { status: "unknown" }, + message: + authFailure?.message ?? + "Grok CLI is installed but ACP startup failed. Check server logs for details.", }, }); } @@ -291,10 +302,10 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func }, }); } - const discoveredModels = discoveryExit.value.value; + const probeResult = discoveryExit.value.value; const models = - discoveredModels.length > 0 - ? grokModelsFromSettings(grokSettings.customModels, discoveredModels) + probeResult.models.length > 0 + ? grokModelsFromSettings(grokSettings.customModels, probeResult.models) : fallbackModels; return buildServerProvider({ @@ -306,7 +317,7 @@ export const checkGrokProviderStatus = Effect.fn("checkGrokProviderStatus")(func installed: true, version, status: "ready", - auth: { status: "unknown" }, + auth: probeResult.auth, }, }); }); diff --git a/apps/server/src/provider/acp/AcpSessionRuntime.ts b/apps/server/src/provider/acp/AcpSessionRuntime.ts index 09fce6d56f9..33edf094773 100644 --- a/apps/server/src/provider/acp/AcpSessionRuntime.ts +++ b/apps/server/src/provider/acp/AcpSessionRuntime.ts @@ -89,6 +89,8 @@ export interface AcpSessionRequestLogEvent { export interface AcpSessionRuntimeStartResult { readonly sessionId: string; readonly initializeResult: EffectAcpSchema.InitializeResponse; + /** Agent response to `authenticate`, the only place most agents report account identity. */ + readonly authenticateResult: EffectAcpSchema.AuthenticateResponse; readonly sessionSetupResult: | EffectAcpSchema.LoadSessionResponse | EffectAcpSchema.NewSessionResponse @@ -545,7 +547,7 @@ export const make = ( methodId: options.authMethodId, } satisfies EffectAcpSchema.AuthenticateRequest; - yield* runLoggedRequest( + const authenticateResult = yield* runLoggedRequest( "authenticate", authenticatePayload, acp.agent.authenticate(authenticatePayload), @@ -650,6 +652,7 @@ export const make = ( const nextState = { sessionId, initializeResult, + authenticateResult, sessionSetupResult, modelConfigId: extractModelConfigId(sessionSetupResult), } satisfies AcpStartedState; diff --git a/apps/server/src/provider/acp/GrokAcpCliProbe.test.ts b/apps/server/src/provider/acp/GrokAcpCliProbe.test.ts index 222fc4a12d5..6897aa4805a 100644 --- a/apps/server/src/provider/acp/GrokAcpCliProbe.test.ts +++ b/apps/server/src/provider/acp/GrokAcpCliProbe.test.ts @@ -13,7 +13,7 @@ import * as Effect from "effect/Effect"; import { ChildProcessSpawner } from "effect/unstable/process"; import { describe, expect } from "vite-plus/test"; -import { makeGrokAcpRuntime } from "./GrokAcpSupport.ts"; +import { grokAuthFromAcpAuthenticate, makeGrokAcpRuntime } from "./GrokAcpSupport.ts"; const makeProbeRuntime = Effect.gen(function* () { const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; @@ -35,6 +35,19 @@ describe.runIf(process.env.T3_GROK_ACP_PROBE === "1")("Grok ACP CLI probe", () = }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); + it.effect("authenticate carries credentials the provider snapshot can report", () => + Effect.gen(function* () { + const runtime = yield* makeProbeRuntime; + const started = yield* runtime.start(); + + // A successful `authenticate` is the only auth signal the Grok CLI gives + // us. If this regresses, the settings card falls back to claiming + // authentication could not be verified. + const auth = grokAuthFromAcpAuthenticate(started.authenticateResult, process.env); + expect(auth.status).toBe("authenticated"); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + it.effect("session/new advertises typed SessionModelState with at least one model", () => Effect.gen(function* () { const runtime = yield* makeProbeRuntime; diff --git a/apps/server/src/provider/acp/GrokAcpSupport.test.ts b/apps/server/src/provider/acp/GrokAcpSupport.test.ts index 02d60976b24..0b70df83e06 100644 --- a/apps/server/src/provider/acp/GrokAcpSupport.test.ts +++ b/apps/server/src/provider/acp/GrokAcpSupport.test.ts @@ -1,10 +1,13 @@ import { describe, expect, it } from "@effect/vitest"; +import * as Cause from "effect/Cause"; import * as Effect from "effect/Effect"; import * as EffectAcpErrors from "effect-acp/errors"; import { applyGrokAcpModelSelection, buildGrokAcpSpawnInput, + grokAuthFailureFromAcpCause, + grokAuthFromAcpAuthenticate, resolveGrokAcpBaseModelId, } from "./GrokAcpSupport.ts"; @@ -35,6 +38,53 @@ describe("buildGrokAcpSpawnInput", () => { }); }); +describe("grokAuthFromAcpAuthenticate", () => { + it("reports the account email Grok returns from authenticate", () => { + expect( + grokAuthFromAcpAuthenticate({ + _meta: { email: " grok-user@example.com ", auth_mode: "Oidc", team_id: "team-1" }, + }), + ).toEqual({ status: "authenticated", email: "grok-user@example.com" }); + }); + + it("labels API key credentials when authenticate reports no account", () => { + expect(grokAuthFromAcpAuthenticate({}, { XAI_API_KEY: "secret" })).toEqual({ + status: "authenticated", + type: "API key", + }); + }); + + it("treats a bare authenticate success as authenticated without identity", () => { + expect(grokAuthFromAcpAuthenticate({ _meta: { email: " " } }, {})).toEqual({ + status: "authenticated", + }); + expect(grokAuthFromAcpAuthenticate({ _meta: null }, {})).toEqual({ status: "authenticated" }); + }); +}); + +describe("grokAuthFailureFromAcpCause", () => { + it("maps an ACP auth-required failure to an unauthenticated snapshot", () => { + const failure = grokAuthFailureFromAcpCause( + Cause.fail(EffectAcpErrors.AcpRequestError.authRequired()), + ); + expect(failure?.auth).toEqual({ status: "unauthenticated" }); + expect(failure?.message).toContain("not authenticated"); + }); + + it("leaves unrelated ACP failures to the generic startup message", () => { + expect( + grokAuthFailureFromAcpCause( + Cause.fail(EffectAcpErrors.AcpRequestError.invalidParams("session id not known")), + ), + ).toBeUndefined(); + expect( + grokAuthFailureFromAcpCause( + Cause.fail(new EffectAcpErrors.AcpSpawnError({ command: "grok", cause: "boom" })), + ), + ).toBeUndefined(); + }); +}); + describe("applyGrokAcpModelSelection", () => { const makeRecordingRuntime = (failure?: EffectAcpErrors.AcpError) => { const modelCalls: Array = []; diff --git a/apps/server/src/provider/acp/GrokAcpSupport.ts b/apps/server/src/provider/acp/GrokAcpSupport.ts index c928b3ed80e..d5e23bf6130 100644 --- a/apps/server/src/provider/acp/GrokAcpSupport.ts +++ b/apps/server/src/provider/acp/GrokAcpSupport.ts @@ -1,7 +1,9 @@ -import { type GrokSettings, ProviderDriverKind } from "@t3tools/contracts"; +import { type GrokSettings, ProviderDriverKind, type ServerProviderAuth } from "@t3tools/contracts"; +import * as Cause from "effect/Cause"; import * as Crypto from "effect/Crypto"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Schema from "effect/Schema"; import * as Scope from "effect/Scope"; import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner"; import * as EffectAcpErrors from "effect-acp/errors"; @@ -16,6 +18,9 @@ const GROK_OAUTH2_REFERRER_ENV = "GROK_OAUTH2_REFERRER"; const T3_CODE_OAUTH_REFERRER = "t3code"; const GROK_AUTH_METHOD_API_KEY = "xai.api_key"; const GROK_AUTH_METHOD_CACHED_TOKEN = "cached_token"; +const GROK_API_KEY_AUTH_TYPE = "API key"; +const ACP_AUTH_REQUIRED_ERROR_CODE = -32000; +const isAcpRequestError = Schema.is(EffectAcpErrors.AcpRequestError); const GROK_DRIVER_KIND = ProviderDriverKind.make("grok"); type GrokAcpRuntimeGrokSettings = Pick; @@ -51,6 +56,57 @@ function resolveGrokAuthMethodId(environment: NodeJS.ProcessEnv | undefined): st : GROK_AUTH_METHOD_CACHED_TOKEN; } +function trimmedMetaString( + meta: EffectAcpSchema.AuthenticateResponse["_meta"], + key: string, +): string | undefined { + if (meta === null || meta === undefined) { + return undefined; + } + const value = meta[key]; + if (typeof value !== "string") { + return undefined; + } + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : undefined; +} + +/** + * Derives the account identity Grok reports from a successful `authenticate`. + * A success is proof of working credentials on its own, so the identity fields + * are best-effort: builds that answer with a bare `{}` still count as + * authenticated. + */ +export function grokAuthFromAcpAuthenticate( + response: EffectAcpSchema.AuthenticateResponse, + environment?: NodeJS.ProcessEnv, +): ServerProviderAuth { + const email = trimmedMetaString(response._meta, "email"); + if (email) { + return { status: "authenticated", email }; + } + return resolveGrokAuthMethodId(environment) === GROK_AUTH_METHOD_API_KEY + ? { status: "authenticated", type: GROK_API_KEY_AUTH_TYPE } + : { status: "authenticated" }; +} + +/** + * Recognizes an ACP startup failure the user can fix by signing in, so the + * settings card can say so instead of blaming a generic startup error. + */ +export function grokAuthFailureFromAcpCause( + cause: Cause.Cause, +): { readonly auth: ServerProviderAuth; readonly message: string } | undefined { + const failure = Cause.squash(cause); + if (!isAcpRequestError(failure) || failure.code !== ACP_AUTH_REQUIRED_ERROR_CODE) { + return undefined; + } + return { + auth: { status: "unauthenticated" }, + message: "Grok CLI is installed but not authenticated. Run `grok` and sign in.", + }; +} + export const makeGrokAcpRuntime = ( input: GrokAcpRuntimeInput, ): Effect.Effect< diff --git a/docs/fork/0009-grok-reports-its-authenticated-account.md b/docs/fork/0009-grok-reports-its-authenticated-account.md new file mode 100644 index 00000000000..288b9d8bb11 --- /dev/null +++ b/docs/fork/0009-grok-reports-its-authenticated-account.md @@ -0,0 +1,36 @@ +# 0009: Grok reports the account it is signed in as + +- PR: [TrogonStack/t3code#18](https://github.com/TrogonStack/t3code/pull/18) +- Status: active + +## What you can do now + +- See which account a working Grok install is signed in as in Settings, + blurred until you click it, the same way Codex and Claude already report + theirs. A ready Grok provider no longer says its authentication could not be + verified. +- Tell a signed-out Grok CLI apart from a broken one. Missing credentials read + as not authenticated with a prompt to sign in, instead of looking like any + other startup failure. +- Pick from the models your account actually has, since a working sign-in is + what the model list comes from. + +## Why + +Settings exists to answer one question: is this provider working. A provider +that reported itself ready and unverifiable in the same breath answered that +question with a shrug, and the only way to find out was to start a thread and +see whether it failed. Grok tells us who is signed in every time T3 Code +starts it up, so this was information we already had and did not show. + +The signed-out case matters just as much. Anyone who has not signed in needs +to be told to sign in, not handed a generic failure that reads like a bug in +T3 Code. + +## Upstream considerations + +Nothing here is fork-specific, so this belongs upstream as an ordinary bug +fix. Submit it, then delete this entry once it merges. It touches the shared +Grok provider and the shared ACP session runtime, so a sync must not drop it. +The runtime change is additive, which keeps the rebase burden small, but it is +the piece most likely to move under us upstream. diff --git a/docs/fork/README.md b/docs/fork/README.md index ac08c2b358c..324bc69819e 100644 --- a/docs/fork/README.md +++ b/docs/fork/README.md @@ -27,11 +27,12 @@ Each entry uses these sections: ## Ledger -| # | Divergence | PR | Status | -| ---- | ---------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | ------ | -| 0003 | [Native subagent threads for Claude orchestrators](./0003-native-subagent-threads.md) | [#3](https://github.com/TrogonStack/t3code/pull/3) | active | -| 0006 | [Fork schema on its own migration ledger](./0006-fork-migration-ledger.md) | [#13](https://github.com/TrogonStack/t3code/pull/13), [#16](https://github.com/TrogonStack/t3code/pull/16) | active | -| 0007 | [API-key Codex installs are not reported as broken](./0007-codex-api-key-auth-is-supported.md) | [#15](https://github.com/TrogonStack/t3code/pull/15) | active | -| 0008 | [Drop a folder on the sidebar to add a project](./0008-drop-a-folder-to-add-a-project.md) | [#17](https://github.com/TrogonStack/t3code/pull/17) | active | -| 0010 | [Pull request conventions of our own](./0010-fork-pull-request-conventions.md) | [#19](https://github.com/TrogonStack/t3code/pull/19) | active | -| 0011 | [Follow the background work a thread left running](./0011-follow-background-work.md) | [#20](https://github.com/TrogonStack/t3code/pull/20) | active | +| # | Divergence | PR | Status | +| ---- | ----------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | ------ | +| 0003 | [Native subagent threads for Claude orchestrators](./0003-native-subagent-threads.md) | [#3](https://github.com/TrogonStack/t3code/pull/3) | active | +| 0006 | [Fork schema on its own migration ledger](./0006-fork-migration-ledger.md) | [#13](https://github.com/TrogonStack/t3code/pull/13), [#16](https://github.com/TrogonStack/t3code/pull/16) | active | +| 0007 | [API-key Codex installs are not reported as broken](./0007-codex-api-key-auth-is-supported.md) | [#15](https://github.com/TrogonStack/t3code/pull/15) | active | +| 0008 | [Drop a folder on the sidebar to add a project](./0008-drop-a-folder-to-add-a-project.md) | [#17](https://github.com/TrogonStack/t3code/pull/17) | active | +| 0009 | [Grok reports the account it is signed in as](./0009-grok-reports-its-authenticated-account.md) | [#18](https://github.com/TrogonStack/t3code/pull/18) | active | +| 0010 | [Pull request conventions of our own](./0010-fork-pull-request-conventions.md) | [#19](https://github.com/TrogonStack/t3code/pull/19) | active | +| 0011 | [Follow the background work a thread left running](./0011-follow-background-work.md) | [#20](https://github.com/TrogonStack/t3code/pull/20) | active |