feat: add an environment-backed feature flag client wired through the context - #2221
Closed
AlexanderRichey wants to merge 1 commit into
Closed
feat: add an environment-backed feature flag client wired through the context#2221AlexanderRichey wants to merge 1 commit into
AlexanderRichey wants to merge 1 commit into
Conversation
… context Introduces a small feature-flag facility so experimental behavior can ship behind an explicit switch. The first (and only) flag is AGENTCORE_CLI_EXPERIMENTAL_IMPERATIVE_DEPLOY, which nothing consumes yet; the next commit builds the imperative harness deployment on it. Design: - src/featureFlags/ declares FEATURE_FLAGS (code name -> env var), the consumer-facing FeatureFlags interface, and EnvFeatureFlags, which is constructed with an injected processEnv (never process.env directly) and reads it once so a flag cannot flip mid-command. The contract is narrow on purpose: a flag is on only when the variable's trimmed value is exactly "1". - FeatureFlagsKey joins the other root context keys; withFeatureFlags pins an instance the same way withGlobalConfigAccessor does. createRootHandler installs it for every command (CLI and TUI), defaulting to an instance with nothing enabled so the ~65 existing test call sites need no change. src/index.ts is the one place that passes process.env. - withLogging writes one debug line naming the enabled flags, only when at least one is on, so unflagged runs keep their exact log shape. - TestFeatureFlags (src/testing/) and a featureFlags option on renderScreen let handler and screen tests turn an experiment on. No behavior changes for users; README gains an "Experimental features" subsection documenting the =1 contract. Read first: src/featureFlags/env.ts, src/middleware/withFeatureFlags.tsx, src/handlers/index.tsx, src/middleware/withLogging.tsx. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014D5SZ5sApMQHrwd87VjDar
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Contributor
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice, tightly-scoped bit of plumbing. A few things I checked and liked:
- Middleware order is correct:
withFeatureFlagsis registered beforewithLogginginsrc/handlers/index.tsx, which matches the intent thatwithLoggingbe able to readFeatureFlagsKeyoff the context. - Env read once, at construction (
src/featureFlags/env.ts+src/index.ts): the "flag can't flip mid-command" property is enforced by the constructor snapshotting into aSet, and there's a dedicated test for it. Good. - Strict
"1"contract: table-driven test covers"0","true","yes","on","","11","1.0", plus surrounding whitespace andundefined. The README documents the same contract. - Tests use real dependencies:
TestFeatureFlagsis a proper in-memory implementation of theFeatureFlagsinterface rather than a jest/bun mock, and the router tests exercise the middleware end-to-end throughRouter.route(...)with a real leaf handler. No mocking ofwithLogginginternals orctxshape — good. - Screen harness parity:
src/testing/renderScreen.tsxthreads the sameFeatureFlagsinstance into both the root handler config and the baseContext, so screens and handlers see the same source of truth.
Two things worth calling out, neither blocking:
- No telemetry for enabled flags. The debug log line (
"experimental feature flags enabled") is local-only. SinceimperativeDeployis currently "reserved" and doesn't gate any behavior yet, this is fine to defer, but when the first consumer lands it would be worth attachingfeatureFlags: FeatureFlag[]to theCommandRunMetricEventso adoption of experimental paths is measurable, not just greppable in~/.agentcore/logs/output. enabled()return type could bereadonly FeatureFlag[]since callers only iterate/log it — minor.
Ship it.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2221 +/- ##
=========================================
Coverage 97.07% 97.08%
=========================================
Files 544 548 +4
Lines 37866 37905 +39
=========================================
+ Hits 36760 36799 +39
Misses 1106 1106 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Hweinstock
reviewed
Sep 4, 2026
| export class EnvFeatureFlags implements FeatureFlags { | ||
| private readonly enabledFlags: ReadonlySet<FeatureFlag>; | ||
|
|
||
| constructor(processEnv: Record<string, string | undefined>) { |
Contributor
There was a problem hiding this comment.
what do you think about leveraging the global config for this? This could allow customers could do something like:
agentcore config experiments.imperativeDeploy true
and
agentcore config experiments
[insert JSON here with all experiment settings]
We could still allow an env var override to take precedence too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a small feature-flag facility so experimental behavior can ship behind an explicit switch. This is the first commit of #2216, split out on its own so the wiring can land ahead of the imperative deploy that consumes it. The only flag it declares is
AGENTCORE_CLI_EXPERIMENTAL_IMPERATIVE_DEPLOY, which nothing reads yet; there is no user-visible behavior change beyond one debug log line.How it works
src/featureFlags/declaresFEATURE_FLAGS(code name → environment variable), the consumer-facingFeatureFlagsinterface (isEnabled,enabled), andEnvFeatureFlags. The class is constructed with an injectedprocessEnvand never readsprocess.envitself; values are read once at construction so a flag cannot flip mid-command. The contract is deliberately narrow: a flag is on only when the variable's trimmed value is exactly1. Any other value (0,true,yes, empty) is off.FeatureFlagsKeyjoins the other root context keys insrc/router/router.tsx;withFeatureFlagspins an instance the same waywithGlobalConfigAccessordoes.createRootHandlerinstalls it for every command (CLI and TUI), defaulting to an instance with nothing enabled so the existing test call sites need no change.src/index.tsis the one place that passesprocess.env.withLoggingwrites a single debug line,experimental feature flags enabled, naming the enabled flags, only when at least one is on, so unflagged runs keep their exact log shape.TestFeatureFlags(exported fromsrc/testing) and afeatureFlagsoption onrenderScreenlet handler and screen tests turn an experiment on.=1contract and that flagged features may change or vanish.Verification
bun run lint:check,bun run format:check,bun run typecheck,bun test(2983 tests, 0 failures at this commit), andbun run buildall pass.bun run src/index.ts --helpworks. WithAGENTCORE_CLI_EXPERIMENTAL_IMPERATIVE_DEPLOY=1, aharness list --region us-east-1 --jsonrun wrote{"msg":"experimental feature flags enabled","featureFlags":["imperativeDeploy"],…}to~/.agentcore/logs/output-*.log; with the variable set to0or unset, no such line was written and the command still listed harnesses, so the middleware chain is intact.Reviewer guide
src/featureFlags/env.ts,src/middleware/withFeatureFlags.tsx,src/handlers/index.tsx,src/middleware/withLogging.tsx,src/testing/featureFlags.tsx.🤖 Generated with Claude Code
https://claude.ai/code/session_014D5SZ5sApMQHrwd87VjDar