Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions src/globalConfig/accessor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be simpler to return isFirstRun in the output here? Would that avoid having to make many of these other changes?

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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/globalConfig/config.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
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: {
enabled: true,
audit: false,
endpoint: "https://telemetry.agentcore.aws.dev",
},
installationId: crypto.randomUUID(),
installationId: undefined,
};

/**
Expand Down
9 changes: 7 additions & 2 deletions src/globalConfig/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,13 @@ export const globalConfigFileSchema = z.object({
/** The raw shape stored on disk for overriding defaults. */
export type GlobalConfigFileData = z.infer<typeof globalConfigFileSchema>;

/** The fully resolved config after applying defaults — all fields required. */
export type GlobalConfig = DeepRequired<GlobalConfigFileData>;
/**
* 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<Omit<GlobalConfigFileData, "installationId">> & {
installationId?: string;
};

/** Manages access to a set of configuration values for the CLI */
export interface GlobalConfigAccessor {
Expand Down
20 changes: 5 additions & 15 deletions src/handlers/config/config.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
14 changes: 9 additions & 5 deletions src/handlers/config/handler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
15 changes: 14 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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`);

Expand Down Expand Up @@ -112,6 +123,8 @@ process.exit(
}
await telemetryClient.shutdown();
await rootLogger.end();

printFirstRunNotice(isFirstRun, globalConfig.telemetry.enabled, io.stderr);
}
}),
);
7 changes: 6 additions & 1 deletion src/telemetry/client.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,21 @@ 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 },
});

const disabledConfigAccessor = new TestGlobalConfigAccessor();
const disabledConfig = await disabledConfigAccessor.get();
await disabledConfigAccessor.set({
...disabledConfig,
installationId,
telemetry: { ...disabledConfig.telemetry, audit: false },
});

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -321,6 +325,7 @@ describe("OtelHistogramSink", () => {
const globalConfigAccessor = new TestGlobalConfigAccessor({
initialConfigData: {
...DEFAULT_GLOBAL_CONFIG,
installationId: "00000000-0000-0000-0000-000000000000",
telemetry: {
enabled,
audit: false,
Expand Down
1 change: 1 addition & 0 deletions src/telemetry/index.tsx
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export { DefaultTelemetryClient } from "./client";
export { printFirstRunNotice } from "./notice";
export { type AttributesOf, type MetricEvent } from "./types";
26 changes: 26 additions & 0 deletions src/telemetry/notice.test.ts
Original file line number Diff line number Diff line change
@@ -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");
}
},
);
});
21 changes: 21 additions & 0 deletions src/telemetry/notice.ts
Original file line number Diff line number Diff line change
@@ -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"),
);
}
Loading