feat: compile-time feature flag for preview/GA consolidation#1341
Open
jesseturner21 wants to merge 12 commits into
Open
feat: compile-time feature flag for preview/GA consolidation#1341jesseturner21 wants to merge 12 commits into
jesseturner21 wants to merge 12 commits into
Conversation
…lidation Replace the dual-branch (main/preview) workflow with a single branch using a compile-time __PREVIEW__ constant. esbuild's define block replaces the constant at build time, enabling dead code elimination for GA builds while keeping all harness/preview code in the same source tree. Key changes: - Add src/cli/feature-flags.ts with isPreviewEnabled() wrapper - Configure esbuild define block and vitest define for __PREVIEW__ - Gate harness commands (add tool, remove tool/harness) behind isPreviewEnabled() - Gate harness UI screens and create flow behind the flag - Add globalThis shim in index.ts for tsx dev mode - Update bundle.mjs to produce dual tarballs (GA + preview) - Support ESBUILD_OUTFILE env var for isolated test builds - Port all harness-related source files from preview branch - Add preview-flag.test.ts verifying dead code elimination
Contributor
Package TarballHow to installgh release download pr-1341-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz |
Contributor
|
Claude Security Review: the review run failed before completing. See the run for details. |
Prettier requires unquoted object keys when valid identifiers.
Contributor
|
Claude Security Review: the review run failed before completing. See the run for details. |
Contributor
Coverage Report
|
Harness features are gated behind BUILD_PREVIEW=1 and eliminated from GA bundles. Integration and e2e tests that exercise harness commands must skip when running against the default (GA) build.
Contributor
|
Claude Security Review: the review run failed before completing. See the run for details. |
…mory cleanup - Wrap harness-related CLI options in invoke command behind isPreviewEnabled() so they don't leak into GA build's --help output - Wrap harness-related CLI options in create command behind isPreviewEnabled() - Fix remove harness leaving orphaned memory entries in agentcore.json - Fix deploy preflight rejecting harness-only projects - Add integration test for harness re-add after removal
Contributor
|
Claude Security Review: the review run failed before completing. See the run for details. |
- isHint check now comes after isExec (matching preview branch) so exec messages always render as magenta, not gray - When a project has both runtimes and harnesses and no flag is given, show a clear error listing both --runtime and --harness options
Contributor
|
Claude Security Review: the review run failed before completing. See the run for details. |
The browser in `agentcore dev` could not invoke harnesses because the /invocations POST handler never checked for harnessName in the request body. Add early routing to handleHarnessInvocation when present. The deploy progress box was missing because useDevDeploy did not pass verbose: true or onResourceEvent to handleDeploy, so the switchableIoHost was never created and CloudFormation messages never reached the TUI. Constraint: handleHarnessInvocation handler already existed but was unreachable Rejected: Adding a separate /harness-invocations endpoint | breaks frontend contract Confidence: high Scope-risk: narrow
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
The browser frontend could not discover harnesses because /api/status did not include harness data or selectedHarness in its response. The deploy box was still not showing because switchableIoHost was only created with verbose:true, but preview also creates it when onDeployMessage is provided. Also wire onDeployMessage into the setOnMessage callback so both callbacks receive deploy events. Constraint: frontend polls /api/status to discover available targets Rejected: Separate /api/harnesses endpoint | adds complexity for no benefit Confidence: high Scope-risk: narrow
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
The frontend Agent Inspector crashes with "Cannot read properties of undefined (reading 'find')" when switching to a harness because the /api/resources endpoint was missing the harnesses array entirely. Constraint: Frontend expects harnesses array with deploymentStatus and deployed fields Confidence: high Scope-risk: narrow
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
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
Consolidates the main and preview branches into a single branch using a compile-time feature flag.
npm run bundlenow produces two tarballs from the same source:This eliminates the merge-conflict-prone dual-branch workflow while ensuring GA customers never see unreleased features.
How it works
src/cli/feature-flags.tsexportsisPreviewEnabled()backed by a__PREVIEW__constantdefineblock replaces__PREVIEW__withtrueorfalseat bundle timefalse, esbuild's minifier eliminates all dead code paths — GA bundles contain zero harness logicChanges
Feature flag infrastructure
src/cli/feature-flags.ts— wrapper function for compile-time constantesbuild.config.mjs—define: { __PREVIEW__: process.env.BUILD_PREVIEW === '1' ? 'true' : 'false' }vitest.config.ts— matching define for test environmentsscripts/bundle.mjs— produces dual tarballs (GA build, thenBUILD_PREVIEW=1rebuild)Command gating
invoke/command.tsx— 13 harness options (--harness,--harness-arn,--model-id, etc.) conditionally registeredcreate/command.tsx— 8 harness options (--model-id,--api-key-arn,--truncation-strategy, etc.) conditionally registeredcli.ts—add tool/remove toolcommands gatedprimitives/registry.ts—harnessPrimitiveconditionally instantiatedTUI gating
useCreateFlow.ts— type selector (Harness/Agent/Skip) only shown in previewuseInvokeFlow.ts— harness targets only displayed in previewdev/command.tsx— harness deploy + invoke flow gatedBug fixes (found during testing)
remove harnessnow also removes the associated<name>MemoryentryhasHarnessesto the resource checkqueueMicrotask+setModeisHintcheck afterisExecto preserve magenta exec stylingTests
src/cli/__tests__/preview-flag.test.ts— verifies dead code elimination in both buildsinteg-tests/add-remove-harness.test.ts— harness lifecycle + memory cleanup + re-add after removalBUILD_PREVIEW=1guard)Verification
Extensively tested with parallel subagents across CLI, TUI, and AWS deployment:
invoke --helpshows no harness optionscreate --helpshows no harness optionsadd --helpshows no harness/tool subcommandsBUILD_PREVIEW=1env var cannot bypass flag--harnessflag