diff --git a/README.md b/README.md index 53f426695..05c84fa85 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,25 @@ Global flags (declared at the root, available on every command): | `--debug` | Debug logging. | | `--endpoint-url` | Override the service endpoint URL (e.g. for testing against a stub). | +### Experimental features + +Some features ship behind a feature flag while they settle. A flag is an +environment variable and is **enabled only when its trimmed value is exactly +`1`**; any other value (`0`, `true`, `yes`, empty) leaves it off. Features +behind a flag may change or vanish without notice, so scripts should not depend +on them. + +| Variable | Feature | +| ---------------------------------------------- | ------------------------------------------------------- | +| `AGENTCORE_CLI_EXPERIMENTAL_IMPERATIVE_DEPLOY` | Reserved for imperative harness deployment (see below). | + +```bash +AGENTCORE_CLI_EXPERIMENTAL_IMPERATIVE_DEPLOY=1 agentcore project deploy +``` + +Flags are read once when the process starts; a debug log of a run +(`~/.agentcore/logs/output`) records which were enabled. + ### Invoke a project resource Run `agentcore project invoke` from inside a project to choose a deployed diff --git a/src/featureFlags/env.test.ts b/src/featureFlags/env.test.ts new file mode 100644 index 000000000..bcce98f58 --- /dev/null +++ b/src/featureFlags/env.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, test } from "bun:test"; +import { EnvFeatureFlags } from "./env"; +import { FEATURE_FLAGS } from "./types"; + +const VARIABLE = FEATURE_FLAGS.imperativeDeploy; + +describe("EnvFeatureFlags", () => { + test("a flag whose variable is unset is disabled", () => { + const flags = new EnvFeatureFlags({}); + expect(flags.isEnabled("imperativeDeploy")).toBe(false); + expect(flags.enabled()).toEqual([]); + }); + + test("exactly '1' enables a flag", () => { + const flags = new EnvFeatureFlags({ [VARIABLE]: "1" }); + expect(flags.isEnabled("imperativeDeploy")).toBe(true); + expect(flags.enabled()).toEqual(["imperativeDeploy"]); + }); + + test("surrounding whitespace around '1' is ignored", () => { + expect(new EnvFeatureFlags({ [VARIABLE]: " 1 " }).isEnabled("imperativeDeploy")).toBe(true); + expect(new EnvFeatureFlags({ [VARIABLE]: "\t1\n" }).isEnabled("imperativeDeploy")).toBe(true); + }); + + // The contract is "=1 enables", not truthiness: a value that reads as "on" in + // other tools must still leave the experiment off here. + test.each(["0", "true", "yes", "on", "", "11", "1.0"])("%j does not enable a flag", (value) => { + const flags = new EnvFeatureFlags({ [VARIABLE]: value }); + expect(flags.isEnabled("imperativeDeploy")).toBe(false); + expect(flags.enabled()).toEqual([]); + }); + + test("an undefined value (present key, no value) is disabled", () => { + expect(new EnvFeatureFlags({ [VARIABLE]: undefined }).isEnabled("imperativeDeploy")).toBe( + false, + ); + }); + + test("environment variables that are not flags are ignored", () => { + const flags = new EnvFeatureFlags({ + AGENTCORE_CLI_EXPERIMENTAL_SOMETHING_ELSE: "1", + IMPERATIVE_DEPLOY: "1", + imperativeDeploy: "1", + }); + expect(flags.enabled()).toEqual([]); + }); + + test("reads the environment once, at construction", () => { + const env: Record = { [VARIABLE]: "1" }; + const flags = new EnvFeatureFlags(env); + env[VARIABLE] = "0"; + expect(flags.isEnabled("imperativeDeploy")).toBe(true); + }); +}); diff --git a/src/featureFlags/env.ts b/src/featureFlags/env.ts new file mode 100644 index 000000000..37725368d --- /dev/null +++ b/src/featureFlags/env.ts @@ -0,0 +1,36 @@ +import { FEATURE_FLAGS, type FeatureFlag, type FeatureFlags } from "./types"; + +/** The one value that switches a flag on. Anything else — including "true" — leaves it off. */ +const ENABLED_VALUE = "1"; + +/** + * Feature flags read from environment variables. + * + * Every flag maps to one variable (see {@link FEATURE_FLAGS}) and is enabled only + * when that variable's trimmed value is exactly "1". The contract is deliberately + * narrow — no truthiness rules — so an experiment cannot be turned on by accident + * and the README can state it in one line. + * + * The environment is read once at construction, not on every call: a flag that + * flipped mid-command could leave a deploy half in one mode and half in another. + * `src/index.ts` passes `process.env`; nothing here reaches for it. + */ +export class EnvFeatureFlags implements FeatureFlags { + private readonly enabledFlags: ReadonlySet; + + constructor(processEnv: Record) { + const enabled = new Set(); + for (const [flag, variable] of Object.entries(FEATURE_FLAGS) as [FeatureFlag, string][]) { + if (processEnv[variable]?.trim() === ENABLED_VALUE) enabled.add(flag); + } + this.enabledFlags = enabled; + } + + isEnabled(flag: FeatureFlag): boolean { + return this.enabledFlags.has(flag); + } + + enabled(): FeatureFlag[] { + return [...this.enabledFlags]; + } +} diff --git a/src/featureFlags/index.ts b/src/featureFlags/index.ts new file mode 100644 index 000000000..d3157d9c2 --- /dev/null +++ b/src/featureFlags/index.ts @@ -0,0 +1,2 @@ +export { FEATURE_FLAGS, type FeatureFlag, type FeatureFlags } from "./types"; +export { EnvFeatureFlags } from "./env"; diff --git a/src/featureFlags/types.ts b/src/featureFlags/types.ts new file mode 100644 index 000000000..a12532b96 --- /dev/null +++ b/src/featureFlags/types.ts @@ -0,0 +1,20 @@ +/** Every experimental switch the CLI knows, keyed by a stable code name. */ +export const FEATURE_FLAGS = { + imperativeDeploy: "AGENTCORE_CLI_EXPERIMENTAL_IMPERATIVE_DEPLOY", +} as const; + +export type FeatureFlag = keyof typeof FEATURE_FLAGS; + +/** + * Answers whether an experimental feature is switched on for this process. + * + * Declared here (rather than in a handler's types file) because a feature flag + * is cross-cutting: handlers, screens, and middleware all read it, and none of + * them owns it. Implementations live beside it. + */ +export interface FeatureFlags { + /** True when the flag's switch is on for this process. Never throws. */ + isEnabled(flag: FeatureFlag): boolean; + /** The flags currently enabled, for logging/diagnostics. */ + enabled(): FeatureFlag[]; +} diff --git a/src/handlers/index.tsx b/src/handlers/index.tsx index 15dfbf7b3..3731a6311 100644 --- a/src/handlers/index.tsx +++ b/src/handlers/index.tsx @@ -11,17 +11,29 @@ import { createConfigHandler } from "./config/"; import { createProjectHandler } from "./project/index.ts"; import { createUpdateHandler } from "./update/index.tsx"; import { renderTui } from "../tui"; -import { withRegion, withJsonRenderer, withLogging, withGlobalConfigAccessor } from "../middleware"; +import { + withRegion, + withJsonRenderer, + withLogging, + withGlobalConfigAccessor, + withFeatureFlags, +} from "../middleware"; import type { AppIO } from "../io"; import type { Core } from "./types.tsx"; import type { Logger } from "../logging"; import type { GlobalConfigAccessor } from "../globalConfig"; +import { EnvFeatureFlags, type FeatureFlags } from "../featureFlags"; import { PACKAGE_VERSION } from "../constants"; export interface RootHandlerConfig { io: AppIO; logger: Logger; globalConfigAccessor: GlobalConfigAccessor; + /** + * The process's experimental switches. Optional so the many test call sites + * that never touch a flag need no change; the default enables nothing. + */ + featureFlags?: FeatureFlags; } export function createRootHandler(core: Core, config: RootHandlerConfig): Router { @@ -47,6 +59,10 @@ export function createRootHandler(core: Core, config: RootHandlerConfig): Router // machine-readable output without touching the process streams directly. root.use(withJsonRenderer(io)); + // Pin the feature flags before the logger so withLogging can record which + // experiments were on for the command. + root.use(withFeatureFlags(config.featureFlags ?? new EnvFeatureFlags({}))); + // Inject a logger into each handler. root.use(withLogging({ logger })); diff --git a/src/handlers/root.test.tsx b/src/handlers/root.test.tsx index b01f3c17d..7ce8330f6 100644 --- a/src/handlers/root.test.tsx +++ b/src/handlers/root.test.tsx @@ -1,6 +1,14 @@ import { test, expect, describe } from "bun:test"; import { createRootHandler } from "./index"; -import { createSilentLogger, TestCoreClient, TestGlobalConfigAccessor, testIO } from "../testing"; +import { createHandler, FeatureFlagsKey } from "../router"; +import type { FeatureFlag } from "../featureFlags"; +import { + createSilentLogger, + TestCoreClient, + TestFeatureFlags, + TestGlobalConfigAccessor, + testIO, +} from "../testing"; describe("createRootHandler", () => { test("builds the agentcore command tree with its subcommands", () => { @@ -23,4 +31,51 @@ describe("createRootHandler", () => { "update", ]); }); + + // Every command, CLI or TUI, must be able to ask which experiments are on; the + // flags are pinned at the root so a leaf mounted anywhere sees them. + test("pins the injected feature flags for every command", async () => { + let seen: FeatureFlag[] | undefined; + const root = createRootHandler(new TestCoreClient(), { + io: testIO().io, + logger: createSilentLogger(), + globalConfigAccessor: new TestGlobalConfigAccessor(), + featureFlags: new TestFeatureFlags(["imperativeDeploy"]), + }); + root.handler( + createHandler({ + name: "probe", + description: "reads the flags", + handle: async (ctx) => { + seen = ctx.require(FeatureFlagsKey).enabled(); + }, + }), + ); + + await root.route(["node", "agentcore", "probe"]); + + expect(seen).toEqual(["imperativeDeploy"]); + }); + + test("defaults to no feature flags when the config omits them", async () => { + let seen: FeatureFlag[] | undefined; + const root = createRootHandler(new TestCoreClient(), { + io: testIO().io, + logger: createSilentLogger(), + globalConfigAccessor: new TestGlobalConfigAccessor(), + }); + root.handler( + createHandler({ + name: "probe", + description: "reads the flags", + handle: async (ctx) => { + seen = ctx.require(FeatureFlagsKey).enabled(); + }, + }), + ); + + await root.route(["node", "agentcore", "probe"]); + + expect(seen).toEqual([]); + }); }); diff --git a/src/index.ts b/src/index.ts index ed6241af9..a173f7352 100644 --- a/src/index.ts +++ b/src/index.ts @@ -18,6 +18,7 @@ import { DefaultTelemetryClient, printFirstRunNotice } from "./telemetry"; import { AgentCoreCLIError } from "./errors"; import { PACKAGE_VERSION } from "./constants"; import { CommandRunMetricEventKey, ValueContext } from "./router"; +import { EnvFeatureFlags } from "./featureFlags"; process.exit( await runWithExitCode(async (argv: string[]) => { @@ -78,6 +79,9 @@ process.exit( io, logger: rootLogger, globalConfigAccessor, + // The only place the app reads the process environment for flags; read + // once here so a flag cannot flip partway through a command. + featureFlags: new EnvFeatureFlags(process.env), }); const context = ValueContext.EmptyContext().withValue( diff --git a/src/middleware/index.tsx b/src/middleware/index.tsx index 4838ad322..865280279 100644 --- a/src/middleware/index.tsx +++ b/src/middleware/index.tsx @@ -4,3 +4,4 @@ export { withJsonRenderer } from "./withJsonRenderer"; export { withLogging } from "./withLogging"; export { withGlobalConfigAccessor } from "./withGlobalConfigAccessor"; export { withProject } from "./withProject"; +export { withFeatureFlags } from "./withFeatureFlags"; diff --git a/src/middleware/withFeatureFlags.test.tsx b/src/middleware/withFeatureFlags.test.tsx new file mode 100644 index 000000000..7f7dd9101 --- /dev/null +++ b/src/middleware/withFeatureFlags.test.tsx @@ -0,0 +1,37 @@ +import { describe, expect, test } from "bun:test"; +import { createHandler, FeatureFlagsKey, Router } from "../router"; +import type { FeatureFlag } from "../featureFlags"; +import { TestFeatureFlags } from "../testing"; +import { withFeatureFlags } from "./withFeatureFlags"; + +// A leaf that reports what it sees on the context, so the test asserts the +// middleware's effect the way a real handler would observe it. +function routeAndRead(flags: TestFeatureFlags): Promise<{ seen: FeatureFlag[]; on: boolean }> { + return new Promise((resolve, reject) => { + const app = new Router("myapp", "test app"); + app.use(withFeatureFlags(flags)); + app.handler( + createHandler({ + name: "leaf", + description: "reads the feature flags", + handle: async (ctx) => { + const seen = ctx.require(FeatureFlagsKey); + resolve({ seen: seen.enabled(), on: seen.isEnabled("imperativeDeploy") }); + }, + }), + ); + app.route(["node", "myapp", "leaf"]).catch(reject); + }); +} + +describe("withFeatureFlags", () => { + test("pins the injected instance on the context for the leaf", async () => { + const result = await routeAndRead(new TestFeatureFlags(["imperativeDeploy"])); + expect(result).toEqual({ seen: ["imperativeDeploy"], on: true }); + }); + + test("an instance with nothing enabled reports every flag off", async () => { + const result = await routeAndRead(new TestFeatureFlags()); + expect(result).toEqual({ seen: [], on: false }); + }); +}); diff --git a/src/middleware/withFeatureFlags.tsx b/src/middleware/withFeatureFlags.tsx new file mode 100644 index 000000000..457ef027e --- /dev/null +++ b/src/middleware/withFeatureFlags.tsx @@ -0,0 +1,21 @@ +import { FeatureFlagsKey, type Middleware } from "../router"; +import type { FeatureFlags } from "../featureFlags"; + +/** + * Middleware that pins a {@link FeatureFlags} instance on the context, making it + * available to every handler and screen beneath the mount point via + * `ctx.require(FeatureFlagsKey).isEnabled(...)`. + */ +export function withFeatureFlags(flags: FeatureFlags): Middleware { + return (h) => ({ + name: () => h.name(), + description: () => h.description(), + flags: () => h.flags(), + arguments: () => h.arguments(), + doesSupportTui: () => h.doesSupportTui(), + children: () => h.children(), + handle: async (ctx, handlerFlags, args) => { + await h.handle(ctx.withValue(FeatureFlagsKey, flags), handlerFlags, args); + }, + }); +} diff --git a/src/middleware/withLogging.test.ts b/src/middleware/withLogging.test.ts index 94359f865..b1cf36b62 100644 --- a/src/middleware/withLogging.test.ts +++ b/src/middleware/withLogging.test.ts @@ -2,9 +2,10 @@ import { test, describe, beforeEach, afterEach } from "bun:test"; import z from "zod"; import { Router, createHandler, flag } from "../router"; import { withLogging } from "./withLogging"; +import { withFeatureFlags } from "./withFeatureFlags"; import { createFileLogger } from "../logging/fileLogger"; import { LOG_LEVEL, type AsyncLogger } from "../logging/types"; -import { assertLogsMatch } from "../testing"; +import { assertLogsMatch, TestFeatureFlags } from "../testing"; import { join } from "node:path"; import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; @@ -83,4 +84,40 @@ describe("withLogging", () => { }, ]); }); + + // A debug log of a run has to answer "which experiments were on"; the line is + // written only when one is, so unflagged runs keep their existing log shape. + test("names the enabled feature flags once per command", async () => { + const app = new Router("myapp", "test app"); + app.use(withFeatureFlags(new TestFeatureFlags(["imperativeDeploy"]))); + app.use(withLogging({ logger })); + app.handler(createHandler({ name: "flagged", description: "runs", handle: async () => {} })); + + await app.route(["node", "myapp", "flagged"]); + + await assertLogsMatch(tempDir, [ + { + filter: (log: any) => + log.msg === "experimental feature flags enabled" && + log.featureFlags?.length === 1 && + log.featureFlags[0] === "imperativeDeploy" && + log.commandPath === "/myapp/flagged", + expectedCount: 1, + }, + ]); + }); + + test("writes no feature-flag line when nothing is enabled", async () => { + const app = new Router("myapp", "test app"); + app.use(withFeatureFlags(new TestFeatureFlags())); + app.use(withLogging({ logger })); + app.handler(createHandler({ name: "plain", description: "runs", handle: async () => {} })); + + await app.route(["node", "myapp", "plain"]); + + await assertLogsMatch(tempDir, [ + { filter: (log: any) => log.msg === "command executed successfully", expectedCount: 1 }, + { filter: (log: any) => log.msg === "experimental feature flags enabled", expectedCount: 0 }, + ]); + }); }); diff --git a/src/middleware/withLogging.tsx b/src/middleware/withLogging.tsx index 46d484f53..902cd5bf3 100644 --- a/src/middleware/withLogging.tsx +++ b/src/middleware/withLogging.tsx @@ -1,6 +1,6 @@ import type { Logger } from "../logging"; import type { Flag, Middleware } from "../router"; -import { LoggerKey, PathKey } from "../router"; +import { FeatureFlagsKey, LoggerKey, PathKey } from "../router"; interface WithLoggingConfig { logger: Logger; @@ -43,6 +43,13 @@ export function withLogging(config: WithLoggingConfig): Middleware { const safeFlags = redactSensitiveFlags(flags, h.flags()); logger.child({ flags: safeFlags, args }).debug("executing command"); + // Which experiments were on is the first thing a debug log of a run needs + // to answer; logged only when something is enabled so the common case + // adds no noise. Absent (no withFeatureFlags above this point) means none. + const featureFlags = ctx.value(FeatureFlagsKey)?.enabled() ?? []; + if (featureFlags.length > 0) { + logger.child({ featureFlags }).debug("experimental feature flags enabled"); + } await h.handle(ctx.withValue(LoggerKey, logger), flags, args); logger.debug("command executed successfully"); }, diff --git a/src/router/index.tsx b/src/router/index.tsx index 9f4ed3625..fb2637c64 100644 --- a/src/router/index.tsx +++ b/src/router/index.tsx @@ -7,6 +7,7 @@ export { GlobalConfigAccessorKey, CommandRunMetricEventKey, ProjectKey, + FeatureFlagsKey, type DefaultHandle, type DefaultHandlerProvider, isDefaultHandlerProvider, diff --git a/src/router/router.tsx b/src/router/router.tsx index 5b7f41e86..704f4535c 100644 --- a/src/router/router.tsx +++ b/src/router/router.tsx @@ -8,6 +8,7 @@ import { Command, CommanderError } from "commander"; import type { Logger } from "../logging"; import type { GlobalConfigAccessor } from "../globalConfig"; import type { Project } from "../handlers/project/types"; +import type { FeatureFlags } from "../featureFlags"; import { type MetricEvent } from "../telemetry"; // CommandKey exposes the Commander Command for the executing leaf via context. @@ -24,6 +25,11 @@ export const GlobalConfigAccessorKey: ContextKey = contextKey("globalConfigAccessor"); export const ProjectKey = contextKey("project"); +// FeatureFlagsKey exposes the process's experimental switches. Pinned at the root +// by withFeatureFlags, so a handler or screen anywhere in the tree can ask +// `ctx.require(FeatureFlagsKey).isEnabled("...")`. +export const FeatureFlagsKey: ContextKey = contextKey("featureFlags"); + // RoutedCommand keeps the compiled handler and Commander command tree together. // TUI consumers can therefore read handler metadata without module-level state. class RoutedCommand extends Command { diff --git a/src/testing/featureFlags.tsx b/src/testing/featureFlags.tsx new file mode 100644 index 000000000..b4e1a3518 --- /dev/null +++ b/src/testing/featureFlags.tsx @@ -0,0 +1,21 @@ +import type { FeatureFlag, FeatureFlags } from "../featureFlags"; + +/** + * In-memory {@link FeatureFlags} for tests: enabled flags are whatever the test + * lists, so a test can turn an experiment on without touching the environment. + */ +export class TestFeatureFlags implements FeatureFlags { + private readonly enabledFlags: ReadonlySet; + + constructor(enabled: FeatureFlag[] = []) { + this.enabledFlags = new Set(enabled); + } + + isEnabled(flag: FeatureFlag): boolean { + return this.enabledFlags.has(flag); + } + + enabled(): FeatureFlag[] { + return [...this.enabledFlags]; + } +} diff --git a/src/testing/index.tsx b/src/testing/index.tsx index 580731fb7..50fef11c4 100644 --- a/src/testing/index.tsx +++ b/src/testing/index.tsx @@ -34,3 +34,4 @@ export { } from "./renderScreen"; export { createSilentLogger, assertLogsMatch, type LogQuery } from "./logging"; export { TestGlobalConfigAccessor } from "./globalConfig"; +export { TestFeatureFlags } from "./featureFlags"; diff --git a/src/testing/renderScreen.tsx b/src/testing/renderScreen.tsx index d8aa079ca..58064d4a4 100644 --- a/src/testing/renderScreen.tsx +++ b/src/testing/renderScreen.tsx @@ -1,6 +1,6 @@ import { render, cleanup } from "ink-testing-library"; import { QueryClient } from "@tanstack/react-query"; -import { ValueContext, compile, CommandKey, type Context } from "../router"; +import { ValueContext, compile, CommandKey, FeatureFlagsKey, type Context } from "../router"; import { RegionKey, JsonKey, DebugKey, EndpointKey } from "../handlers/keys"; import { JsonRendererKey } from "../tui"; import { createRootHandler } from "../handlers"; @@ -10,6 +10,8 @@ import { testIO } from "./testIO"; import { tick, waitFor } from "./timing"; import { createSilentLogger } from "./logging"; import { TestGlobalConfigAccessor } from "./globalConfig"; +import { TestFeatureFlags } from "./featureFlags"; +import type { FeatureFlags } from "../featureFlags"; // TUI test harness. // @@ -27,22 +29,30 @@ import { TestGlobalConfigAccessor } from "./globalConfig"; // RouterScreen walks it to resolve each menu's subcommands), the global flags // (region/json/debug), and a no-op JsonRenderer. Compiling the real handler tree // keeps the command menus faithful to the production command structure. -function baseContext(core: TestCoreClient, endpointUrl?: string): Context { +function baseContext( + core: TestCoreClient, + endpointUrl?: string, + featureFlags: FeatureFlags = new TestFeatureFlags(), +): Context { const rootCommand = compile( createRootHandler(core, { io: testIO().io, logger: createSilentLogger(), globalConfigAccessor: new TestGlobalConfigAccessor(), + featureFlags, }), ValueContext.EmptyContext(), ); + // Screens read the flags off the context the same way handlers do, so the + // instance handed to the root handler is also pinned here. return ValueContext.EmptyContext() .withValue(CommandKey, rootCommand) .withValue(RegionKey, "us-east-1") .withValue(EndpointKey, endpointUrl) .withValue(JsonKey, false) .withValue(DebugKey, false) + .withValue(FeatureFlagsKey, featureFlags) .withValue(JsonRendererKey, { renderJson: () => {}, renderJsonLine: () => {} }); } @@ -67,6 +77,8 @@ export interface RenderScreenOptions { // exercise cache behavior. queryClient?: QueryClient; endpointUrl?: string; + // featureFlags turns experiments on for the screen under test; defaults to none. + featureFlags?: FeatureFlags; } export interface RenderScreenResult { @@ -124,7 +136,7 @@ export function cleanupScreens(): void { // and returns handles to read frames and send input. export function renderScreen(path: string, options: RenderScreenOptions = {}): RenderScreenResult { const core = options.core ?? new TestCoreClient(); - const base = options.ctx ?? baseContext(core, options.endpointUrl); + const base = options.ctx ?? baseContext(core, options.endpointUrl, options.featureFlags); const ctx = options.withContext?.(base) ?? base; const queryClient = options.queryClient ?? testQueryClient();