diff --git a/src/globalConfig/accessor.tsx b/src/globalConfig/accessor.tsx index 6210f285b..c45595062 100644 --- a/src/globalConfig/accessor.tsx +++ b/src/globalConfig/accessor.tsx @@ -45,22 +45,6 @@ export class DefaultGlobalConfigAccessor implements GlobalConfigAccessor { const configFileData = await this.readConfigFile(); - // if no installationId is present, generate one and merge it into the file data - if (!configFileData.installationId) { - configFileData.installationId = DEFAULT_GLOBAL_CONFIG.installationId; - this.logger.info(`no installationId found, persisting one`); - - try { - await this.writeToConfigFile(configFileData); - } catch (e) { - const error = e instanceof Error ? e : new Error(String(e)); - this.logger - .child({ errorName: error.name, errorMessage: error.message }) - .warn(`failed to write initial config file data`); - // best effort - } - } - this.cachedConfig = applyOverrides(DEFAULT_GLOBAL_CONFIG, configFileData); return this.cachedConfig; } diff --git a/src/globalConfig/config.tsx b/src/globalConfig/config.tsx index 290462754..0a5e99d00 100644 --- a/src/globalConfig/config.tsx +++ b/src/globalConfig/config.tsx @@ -1,7 +1,7 @@ import type { DeepPartial, GlobalConfig } from "./types"; /** - * Default values for the global config. Includes a unique installationId for each process. + * Default values for the global config. */ export const DEFAULT_GLOBAL_CONFIG: GlobalConfig = { telemetry: { @@ -9,7 +9,7 @@ export const DEFAULT_GLOBAL_CONFIG: GlobalConfig = { audit: false, endpoint: "https://telemetry.agentcore.aws.dev", }, - installationId: crypto.randomUUID(), + installationId: undefined, }; /** diff --git a/src/globalConfig/types.tsx b/src/globalConfig/types.tsx index b171666d3..06b284cb5 100644 --- a/src/globalConfig/types.tsx +++ b/src/globalConfig/types.tsx @@ -25,8 +25,13 @@ export const globalConfigFileSchema = z.object({ /** The raw shape stored on disk for overriding defaults. */ export type GlobalConfigFileData = z.infer; -/** The fully resolved config after applying defaults — all fields required. */ -export type GlobalConfig = DeepRequired; +/** + * The fully resolved config after applying defaults. All fields are required + * except installationId, which is unset until the first run persists one. + */ +export type GlobalConfig = DeepRequired> & { + installationId?: string; +}; /** Manages access to a set of configuration values for the CLI */ export interface GlobalConfigAccessor { diff --git a/src/handlers/config/config.test.tsx b/src/handlers/config/config.test.tsx index 6a81b50e0..4b0f8d658 100644 --- a/src/handlers/config/config.test.tsx +++ b/src/handlers/config/config.test.tsx @@ -154,24 +154,14 @@ describe("config", () => { expect(JSON.parse(readOutput)).toBe(newEndpoint); }); - test("writes installationId when config missing, preserves it when present", async () => { - await rm(configPath); - const firstOutput = await run(["installationId"]); - const firstId = JSON.parse(firstOutput); - expect(firstId).toMatch(/^[0-9a-f-]{36}$/); - - const secondOutput = await run(["installationId"]); - expect(JSON.parse(secondOutput)).toBe(firstId); - }); - - test("writes installationId when config exists but installationId is missing", async () => { + test("does not set a key with no default value (installationId); shows undefined", async () => { await writeFile(configPath, JSON.stringify({ telemetry: { enabled: true } })); - const firstOutput = await run(["installationId"]); - const firstId = JSON.parse(firstOutput); + const output = await run(["installationId", "11111111-1111-1111-1111-111111111111"]); + expect(output.trim()).toBe("undefined"); - const secondOutput = await run(["installationId"]); - expect(JSON.parse(secondOutput)).toBe(firstId); + const readBack = await run(["installationId"]); + expect(readBack.trim()).toBe("undefined"); }); test("creates the config directory if it does not exist", async () => { diff --git a/src/handlers/config/handler.tsx b/src/handlers/config/handler.tsx index f610ef68b..c8cf436fe 100644 --- a/src/handlers/config/handler.tsx +++ b/src/handlers/config/handler.tsx @@ -40,11 +40,15 @@ export const createConfigHandler = () => jsonRenderer.renderJson(scopedConfig); return; } - const coercedValue = coerceValue( - getAtPath(DEFAULT_GLOBAL_CONFIG, args.key), - args.value, - args.key, - ); + // Keys with no default value (e.g. system-managed installationId) are not + // settable via the CLI; show the current value instead of guessing a type. + const defaultValue = getAtPath(DEFAULT_GLOBAL_CONFIG, args.key); + if (defaultValue === undefined) { + jsonRenderer.renderJson(getAtPath(globalConfig, args.key)); + return; + } + + const coercedValue = coerceValue(defaultValue, args.value, args.key); // update value at key with given value. await globalConfigAccessor.set( diff --git a/src/index.ts b/src/index.ts index c46290b6d..996eca21e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -20,7 +20,7 @@ import { FsReadWriteJson } from "./io"; import { createFileLogger, LOG_LEVEL } from "./logging"; import { runWithExitCode } from "./runnable"; import { DefaultGlobalConfigAccessor } from "./globalConfig"; -import { DefaultTelemetryClient } from "./telemetry"; +import { DefaultTelemetryClient, printFirstRunNotice } from "./telemetry"; import { AgentCoreCLIError } from "./errors"; import { PACKAGE_VERSION } from "./constants"; import { CommandRunMetricEventKey, ValueContext } from "./router"; @@ -61,6 +61,17 @@ process.exit( exit_reason: "success", }); + const globalConfig = await globalConfigAccessor.get(); + const isFirstRun = globalConfig.installationId === undefined; + if (isFirstRun) { + try { + await globalConfigAccessor.set({ ...globalConfig, installationId: crypto.randomUUID() }); + } catch (e) { + const error = AgentCoreCLIError.fromError(e); + rootLogger.child({ error: error.json() }).warn("failed to persist installationId"); + } + } + try { rootLogger.info(`running CLI`); @@ -112,6 +123,8 @@ process.exit( } await telemetryClient.shutdown(); await rootLogger.end(); + + printFirstRunNotice(isFirstRun, globalConfig.telemetry.enabled, io.stderr); } }), ); diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx index bad75db7b..f2e2f6961 100644 --- a/src/telemetry/client.test.tsx +++ b/src/telemetry/client.test.tsx @@ -107,10 +107,13 @@ describe("DefaultTelemetryClient", () => { const enabledSessionId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; const disabledSessionId = "ffffffff-1111-2222-3333-444444444444"; + const installationId = "00000000-0000-0000-0000-000000000000"; + const enabledConfigAccessor = new TestGlobalConfigAccessor(); const enabledConfig = await enabledConfigAccessor.get(); await enabledConfigAccessor.set({ ...enabledConfig, + installationId, telemetry: { ...enabledConfig.telemetry, audit: true }, }); @@ -118,6 +121,7 @@ describe("DefaultTelemetryClient", () => { const disabledConfig = await disabledConfigAccessor.get(); await disabledConfigAccessor.set({ ...disabledConfig, + installationId, telemetry: { ...disabledConfig.telemetry, audit: false }, }); @@ -158,7 +162,7 @@ describe("DefaultTelemetryClient", () => { attrs: { "service.name": "agentcore-cli", "service.version": PACKAGE_VERSION, - "agentcore-cli.installation_id": enabledConfig.installationId, + "agentcore-cli.installation_id": installationId, "agentcore-cli.session_id": enabledSessionId, "os.type": os.type(), "os.version": os.release(), @@ -321,6 +325,7 @@ describe("OtelHistogramSink", () => { const globalConfigAccessor = new TestGlobalConfigAccessor({ initialConfigData: { ...DEFAULT_GLOBAL_CONFIG, + installationId: "00000000-0000-0000-0000-000000000000", telemetry: { enabled, audit: false, diff --git a/src/telemetry/index.tsx b/src/telemetry/index.tsx index 6f28d1424..9bf7a8953 100644 --- a/src/telemetry/index.tsx +++ b/src/telemetry/index.tsx @@ -1,2 +1,3 @@ export { DefaultTelemetryClient } from "./client"; +export { printFirstRunNotice } from "./notice"; export { type AttributesOf, type MetricEvent } from "./types"; diff --git a/src/telemetry/notice.test.ts b/src/telemetry/notice.test.ts new file mode 100644 index 000000000..2667b42c4 --- /dev/null +++ b/src/telemetry/notice.test.ts @@ -0,0 +1,26 @@ +import { test, describe, expect } from "bun:test"; +import { printFirstRunNotice } from "./notice"; + +describe("printFirstRunNotice", () => { + test.each([ + [true, true, 1], + [true, false, 0], + [false, true, 0], + [false, false, 0], + ])( + "isFirstRun=%p telemetryEnabled=%p writes the notice %p time(s)", + (isFirstRun, telemetryEnabled, expectedWrites) => { + const written: string[] = []; + + printFirstRunNotice(isFirstRun, telemetryEnabled, { + write: (text) => void written.push(text), + }); + + expect(written).toHaveLength(expectedWrites); + if (expectedWrites > 0) { + expect(written[0]).toContain("collects aggregated, anonymous usage analytics"); + expect(written[0]).toContain("agentcore config telemetry.enabled false"); + } + }, + ); +}); diff --git a/src/telemetry/notice.ts b/src/telemetry/notice.ts new file mode 100644 index 000000000..af5c06731 --- /dev/null +++ b/src/telemetry/notice.ts @@ -0,0 +1,21 @@ +/** + * Writes the telemetry-collection notice to the given stream on the first run of + * the CLI, unless telemetry is already disabled. + */ +export function printFirstRunNotice( + isFirstRun: boolean, + telemetryEnabled: boolean, + out: { write(text: string): void }, +): void { + if (!isFirstRun || !telemetryEnabled) return; + + out.write( + [ + "", + "The AgentCore CLI collects aggregated, anonymous usage analytics to help improve the tool.", + "To opt out: agentcore config telemetry.enabled false", + "To audit: agentcore config telemetry.audit true", + "", + ].join("\n"), + ); +}