feat(project): add 'project add credentials' (AgentCore Identity api-key + oauth) by tejaskash · Pull Request #2046 · aws/agentcore-cli

tejaskash · 2026-08-19T20:31:42Z

What

Adds the project-spec authoring path for AgentCore Identity credential providers:

agentcore project add credentials api-key --name <name> [--api-key -|file://p] [--api-key-secret-reference <json>]
agentcore project add credentials oauth   --name <name> [--discovery-url u --client-id c --scopes ...]
                                          [--vendor V --provider-configuration <json>]
                                          [--client-secret -|file://p] [--client-secret-reference <json>]

Custom OAuth uses guided flags. Vendored OAuth uses --vendor (a free string, so new vendors need no CLI release) plus a complete --provider-configuration JSON.
Secret flags accept only stdin (-) or file:// sources. Inline values are rejected so keys never reach shell history. A single trailing newline is stripped. Multi-line values are rejected.
An omitted secret writes a commented placeholder to agentcore/.env.local (AGENTCORE_CREDENTIAL_<NAME>[_CLIENT_SECRET]) and prints a fill-before-deploy notice. Existing keys are never overwritten.
Secrets Manager references ({"secretId","jsonKey"}) are recorded in the spec (secretRef / clientSecretRef) and skip .env.local.
Schema reshape in projectSchemas/credential.ts: providerConfig (rejects nested clientSecret / apiKey), clientId, secretRef / clientSecretRef. The usage field is dropped. Deep validation lives in the schema so every writer path enforces it.

Structure

Each leaf command is its own file (add/credentials/api-key, add/credentials/oauth), matching the command tree. Shared helpers live in add/credentials/shared.ts.
.env.local writes go through an EnvLocalFile class with upsert and rollback, so a failed spec write reverses the secrets file in the same cleanup path as scaffolded files.

Reuse

SourceResolver (stdin and file sources), parseSecretReference, and the identity oauth2 config.ts helpers (parseProviderConfigFlags, validateProviderConfigMode). buildProviderConfigInput is not used here, because SDK input construction happens at deploy time.

Not in this PR

Deploy-side work. project deploy already throws NotImplementedError. The lifecycle design (CFN-owned providers via the new CfnApiKeyCredentialProvider / CfnOAuth2CredentialProvider L1s, convergent pre and post-deploy secret sync from .env.local, hash tracking in deployed state) lands with deploy.
The @aws/agentcore-cdk credential construct and the schema mirror in that repo (L3 PR 333).
README. The project command tree is undocumented there today, so no partial documentation was added.
Migration for providers created imperatively by the current CLI (a name collision on first CFN create) is a deploy-task edge case.

Testing

Command-seam tests drive the real router, handler, FsProjectManager, and filesystem across the three credential shapes and the failure paths. Schema tests cover the reshape. EnvLocalFile has unit tests for rollback (delete when created, restore prior content, no-op when nothing written). Full suite: 1605 pass.
E2E on the built artifact (bun run build, node dist/index.js): a real project, all three credential shapes, and the inline-secret and duplicate-name failures exit non-zero with no partial writes.

OAuth credentials now model the two authoring paths: guided custom OAuth (clientId, discoveryUrl, scopes) and vendored providers (free-form vendor + complete providerConfig). providerConfig rejects nested secret material so secrets can only travel via .env.local or Secrets Manager references, which both credential types record as secretRef/clientSecretRef. The unused usage field is dropped. This shape is the shared contract with @aws/agentcore-cdk (mirror change lands on that repo's refactor branch).

Credentials are the first spec-only resource in FsProjectManager.addResource: no scaffolding, the spec entry lands in agentcore.json and secret values or commented placeholders land in agentcore/.env.local (existing keys are never overwritten, and a missing .gitignore guard for .env.local is restored). Secret flags accept only stdin ('-') or file:// sources; inline values are rejected so keys stay out of shell history. External Secrets Manager references skip .env.local entirely and are recorded in the spec.

…path The standalone newline test duplicated the file-secret pipe end to end; the combined test feeds an echo-style newline-terminated file and asserts the stored value, covering both behaviors in one test.

Inline the single-caller placeholder notice, extract the duplicated secret-reference exclusivity parse shared by both leaves, and simplify the envEntries guard.

codecov-commenter · 2026-08-19T20:34:43Z

Codecov Report

❌ Patch coverage is 98.90710% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.21%. Comparing base (b566b33) to head (7e67b71).

Files with missing lines	Patch %	Lines
src/core/project/manager.tsx	88.57%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           refactor    #2046      +/-   ##
============================================
+ Coverage     97.19%   97.21%   +0.02%     
============================================
  Files           389      394       +5     
  Lines         23415    23757     +342     
============================================
+ Hits          22758    23096     +338     
- Misses          657      661       +4

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

E2E verification of 'project build' against the published @aws/agentcore-cdk found a contract drift: its ConfigIO requires credential names of 3-255 characters while the CLI allowed 1-128, so a 2-character name passed 'add' and then broke synth. Tighten the CLI to the intersection (3-128, service character set) until the L3 schema aligns with the service's 1-128 rule.

tejaskash · 2026-08-19T20:37:21Z

Additional E2E verification: built the artifact and ran 'project build' (real npm install + CDK synth via published @aws/agentcore-cdk 0.1.0-alpha.45) with all three credential shapes in agentcore.json. Synth passes; the published ConfigIO strips the new fields harmlessly since nothing consumes them before deploy work lands.

One live contract drift found and contained in 1cd5d14: ConfigIO requires credential names of 3-255 chars while the CLI allowed 1-128, so a 2-char name passed 'add' and then broke 'build'. The CLI now enforces the intersection (3-128). The L3 refactor-branch follow-up should align its name rule with the service's 1-128 and add the new fields (providerConfig, clientId, secretRef, clientSecretRef).

Hweinstock · 2026-08-19T22:22:39Z

      throw err;
    }
+
+    if (input.resourceType === "credential" && input.envEntries?.length) {


Is there a way we can handle this in the same pattern as scaffolding such that it:

rolls back entire process on failure.

avoids a second conditional branch in this function.

One potential way I see to do this would be to implement some interface:

interface LocalEnvAccessor { upsert(entries: LocalEnvEntries[]): Promise<LocalEnvAccessor>; rollback(silent?: boolean): Promise<void> }

Then here, we instantiate it with the project, call upsert in the credential scaffold case, then call rollback where we rollback the scaffolding.

This would also move the envLocal.ts to a class implementation, which feels consistent with how we've done things like SourceResolver.

I think there is still one rollback case here. .env.local is updated before the complete spec is validated, but rollback only runs when json.write fails. I tried the invalid name ab: the command failed validation and left agentcore.json unchanged, but the environment entry remained. Could validation happen before the env write, or be included in the same rollback boundary?

Fixed. The whole-spec safeParse now runs inside the same try as the write, so a rejected spec runs the .env.local rollback together with the scaffolded-file cleanup rather than leaving the staged secret behind.

Hweinstock · 2026-08-19T22:25:36Z

+import type { AddProjectResourceConfig } from "../types";
+import type { AddResourceInput, EnvLocalEntry } from "../../types";
+
+export function createAddCredentialsHandler(config: AddProjectResourceConfig): Router {


should we continue to match the command tree with the file tree + 1:1 of file to handler pattern?

Hweinstock · 2026-08-19T22:30:46Z

@@ -1,8 +1,11 @@
 import z from "zod";
 import { PaymentProviderSchema } from "./payment";
+// Min 3 keeps names inside what @aws/agentcore-cdk's ConfigIO accepts (3-255)


i feel like this comment is at risk of becoming stale if the referenced implementation changes.

Hweinstock · 2026-08-19T22:44:28Z

+      for (const key of skipped) {
+        yield { message: `'${key}' already exists in ${ENV_LOCAL_RELATIVE_PATH}; left unchanged` };
+      }
+      if (await ensureGitignoreCoversEnvLocal(project.rootPath)) {


don't we already scaffold the project with it in the .gitignore here

agentcore-cli/src/assets/templates/shared/gitignore.template

Line 2 in 14e0440

.env.local

? If the user removes it later, that feels like their own (wrong) choice.

Hweinstock · 2026-08-19T22:58:24Z

+async function run(args: string[], opts?: { core?: TestCoreClient; stdin?: string }) {
  const io = testIO();
+  // testIO's stdin is a PassThrough; pre-filling and ending it simulates piped input.
+  if (opts?.stdin !== undefined) (io.io.stdin as unknown as PassThrough).end(opts.stdin);


whats the difference between this pattern and

agentcore-cli/src/cli/commands/invoke/__tests__/resolve-prompt.test.ts

Line 30 in 0cd32ba

Readable.from(['from-stdin'])

?

Maybe OOS here, but is there a way we can standardize on something in

agentcore-cli/src/testing/testIO.tsx

Line 36 in 33e2d2f

export function testIO({ isTTY = false }: TestIOOptions = {}): TestIO {

so every consumer doesn't need to reinvent?

envLocal read/write now uses readTextFile and atomicWrite from src/io instead of raw node:fs calls, matching how the rest of core does file IO and making the secrets-file write atomic. Adds coverage for the create-when-missing .env.local branch, which the scaffolded template previously masked. Also drops a review-marker comment.

…drop redundant gitignore write, standardize testIO stdin

…credentials # Conflicts: # src/core/project/manager.tsx # src/handlers/project/add/index.ts # src/handlers/project/types.ts

aidandaly24 · 2026-08-21T17:28:56Z

+/** Reads a file, returning null when it does not exist so callers can tell empty from absent. */
+async function readOrNull(path: string): Promise<string | null> {
+  try {
+    return await readTextFile(path);


I tried this through the built CLI with node dist/index.js, and it fails here with Bun is not defined because readTextFile() uses Bun.file(). I think this needs a Node-compatible read path, otherwise the Bun tests pass but the published artifact cannot add a credential that writes to .env.local.

Good catch. The root cause was in readTextFile itself (src/io/fileRead.ts), which called Bun.file, a global that exists only under the Bun runtime. The published bundle targets node, so this broke every caller of that function, not only this path (eval.tsx reads files the same way). Fixed it there by reading through node:fs/promises, which keeps the AbortSignal support. Verified the built artifact with node dist/index.js.

aidandaly24 · 2026-08-21T17:29:48Z

+        continue;
+      }
+      const separator = content === "" || content.endsWith("\n") ? "" : "\n";
+      content += `${separator}# ${entry.comment}\n${entry.key}=${entry.value ?? ""}\n`;


I think we need to quote or escape these values before writing them as dotenv. For example, left#right is written as-is, but parseEnv() reads it back as only left. Surrounding spaces are changed too. Silently changing credential material will probably make the provider fail later and be difficult to diagnose.

Fixed. Values are single-quoted before they are written. node:util parseEnv treats everything inside single quotes as literal, so left#right, leading and trailing spaces, $, and backslashes all read back byte for byte. The one value single quotes cannot represent is a value that itself contains a single quote, so that case is rejected with a message that points the caller at a Secrets Manager reference instead. A parseEnv round-trip test covers it.

aidandaly24 · 2026-08-21T17:30:32Z

+
+/** Derives the .env.local variable name a credential's secret is stored under. */
+export function credentialEnvVarName(credentialName: string, suffix = ""): string {
+  return `AGENTCORE_CREDENTIAL_${credentialName.replace(/-/g, "_").toUpperCase()}${suffix}`;


One edge case here: foo-bar and foo_bar are both valid credential names, but they both become AGENTCORE_CREDENTIAL_FOO_BAR. I tried adding both and the second command succeeds, but it reuses the first secret because the existing key is skipped. Maybe normalized-name collisions should be rejected, or the mapping should remain one-to-one? What do you think?

Fixed. addCredentialToProject now rejects a new name that derives the same environment variable as an existing credential, and the error names both. Test added in project.test.ts.

aidandaly24 · 2026-08-21T17:32:51Z

      throw err;
    }
+
+    if (input.resourceType === "credential" && input.envEntries?.length) {


I think there is still one rollback case here. .env.local is updated before the complete spec is validated, but rollback only runs when json.write fails. I tried the invalid name ab: the command failed validation and left agentcore.json unchanged, but the environment entry remained. Could validation happen before the env write, or be included in the same rollback boundary?

jariy17

Pretty good beside the upsert Name function

jariy17 · 2026-08-21T17:52:23Z

+        skipped.push(entry.key);
+        continue;
+      }
+      const separator = content === "" || content.endsWith("\n") ? "" : "\n";


nit: Leave a code comment show the format if you can.
// # <entry.comment>
<entry.key>=<entry.value>

jariy17 · 2026-08-21T17:53:08Z

+}
+
+/** Reads a file, returning null when it does not exist so callers can tell empty from absent. */
+async function readOrNull(path: string): Promise<string | null> {


private this function if no class uses.

Fixed. readOrNull is now a private method on EnvLocalFile.

jariy17 · 2026-08-21T18:11:08Z

+   * are left unchanged so user-managed values survive re-runs. Returns the keys
+   * written and those skipped.
+   */
+  async upsert(entries: EnvLocalEntry[]): Promise<{ written: string[]; skipped: string[] }> {


This doesn't update existing keys, it just skips. So the word upsert doesn't work here. Maybe insertIfNew...

Fixed. Renamed to insertIfNew.

jariy17 · 2026-08-21T18:19:05Z

+      if (!flags.name)
+        throw new InputValidationError("required option '--name <name>' not specified");
+
+      const secretRef = parseExclusiveSecretRef(


Should we combine these flags because they do the same thing and only one of them can be used?

The two flags carry different meanings, so I kept them apart. --api-key takes the secret value itself (from stdin or a file) and the CLI stores it in .env.local. --api-key-secret-reference names a secret the caller already keeps in Secrets Manager and stores no value. parseExclusiveSecretRef makes sure only one is given. Folding them into one flag would force the CLI to guess whether the argument is a reference or a raw secret, and that ambiguity is how a real secret ends up read as a reference or the reverse.

jariy17 · 2026-08-21T18:21:32Z

@@ -0,0 +1,115 @@
+import z from "zod";


This would be a good example of using templates. We can provide default templates for Google Oauth, Okta and so on.

Agreed that vendor templates for Google, Okta, and the rest would be a good addition. It is out of scope for this change, which lands the guided-custom and vendored paths. Noted as a follow-up.

jariy17 · 2026-08-21T18:24:55Z

      return "harnesses";
    case "runtime":
      return "runtimes";
+    case "credential":


Side Note: I really want to abstract this.

Leaving it for now. The switch is exhaustive over ProjectResource, and the inferred return type is what keeps unrelated spec keys (name, managedBy) from leaking in, which the doc comment above it calls out. An abstraction here would need to preserve that per-case return typing to earn its place, so I would rather keep the direct mapping until a second use appears.

jariy17 · 2026-08-21T18:27:35Z

+ * into shell history and process listings, so only stdin and files are allowed.
+ * A single trailing newline is stripped (echo and editors add one).
+ */
+export async function resolveSecretFlag(


I feel like this should be part of the SourceResolver named resolveSecretValueFlags because it could be use in other locations

Leaving resolveSecretFlag local for now. Its only callers today are the two credential handlers, which share it through shared.ts. Moving it onto SourceResolver would widen that class for a single use. When a second, unrelated caller needs the same stdin-or-file-only rule, that is the point to lift it.

It's should be used in runtime too now: https://github.com/aws/agentcore-cli/pull/2035/changes#r3833097791. Shift it into the SourceResolver when you rebase :)

Done. Moved it onto SourceResolver as resolveSecret (commit a37b46d). It throws SourceResolutionError, which is already a subtype of InputValidationError, so the io package stays free of handler-level error types. Both credential handlers now call resolver.resolveSecret(...), and the runtime handler in #2035 can do the same.

- readTextFile: read via node:fs so the node-target bundle works (Bun.file is a Bun-runtime global, absent under node); options.signal passes through - .env.local values are single-quoted so node:util parseEnv round-trips them byte-for-byte; reject values containing a single quote with a clear message - move spec validation inside the rollback try so a rejected spec reverses the staged .env.local write instead of leaving it behind - reject two credential names that derive the same environment variable - rename EnvLocalFile.upsert to insertIfNew (never overwrites)

jariy17 · 2026-08-21T19:34:18Z

+ * into shell history and process listings, so only stdin and files are allowed.
+ * A single trailing newline is stripped (echo and editors add one).
+ */
+export async function resolveSecretFlag(


It's should be used in runtime too now: https://github.com/aws/agentcore-cli/pull/2035/changes#r3833097791. Shift it into the SourceResolver when you rebase :)

The stdin-or-file-only rule now has a second caller (runtime, PR 2035), so resolveSecretFlag moves off the credentials shared module and onto SourceResolver as resolveSecret. It throws SourceResolutionError (already a subtype of InputValidationError), which keeps the io package from depending on handler-level error types. Also bump the generated CDK app template's aws-cdk-lib to ~2.266.0 so it satisfies the @aws/agentcore-cdk peer floor once the credential constructs release.

…credentials # Conflicts: # src/core/project/manager.tsx # src/handlers/project/add/index.ts

tejaskash added 4 commits August 19, 2026 16:11

test(project): fold trailing-newline coverage into the api-key happy …

31decfe

…path The standalone newline test duplicated the file-secret pipe end to end; the combined test feeds an echo-style newline-terminated file and asserts the stored value, covering both behaviors in one test.

refactor(project): apply ponytail review to credentials add

a82b5cf

Inline the single-caller placeholder notice, extract the duplicated secret-reference exclusivity parse shared by both leaves, and simplify the envEntries guard.

github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Aug 19, 2026

github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Aug 19, 2026

Hweinstock reviewed Aug 19, 2026

View reviewed changes

tejaskash added 3 commits August 19, 2026 22:00

Address review: EnvLocalFile class with rollback, 1:1 handler files, …

9f6190f

…drop redundant gitignore write, standardize testIO stdin

Merge remote-tracking branch 'origin/refactor' into feat/project-add-…

224f6c4

…credentials # Conflicts: # src/core/project/manager.tsx # src/handlers/project/add/index.ts # src/handlers/project/types.ts

aidandaly24 reviewed Aug 21, 2026

View reviewed changes

jariy17 requested changes Aug 21, 2026

View reviewed changes

jariy17 mentioned this pull request Aug 21, 2026

feat(project): scaffold add runtime handler #2035

Merged

jariy17 previously approved these changes Aug 21, 2026

View reviewed changes

tejaskash dismissed jariy17’s stale review via a37b46d August 21, 2026 20:13

Merge remote-tracking branch 'origin/refactor' into feat/project-add-…

7e67b71

…credentials # Conflicts: # src/core/project/manager.tsx # src/handlers/project/add/index.ts

jariy17 approved these changes Aug 21, 2026

View reviewed changes

aidandaly24 approved these changes Aug 21, 2026

View reviewed changes

aidandaly24 merged commit 38c30ae into refactor Aug 21, 2026
11 checks passed

aidandaly24 deleted the feat/project-add-credentials branch August 21, 2026 22:06

Uh oh!

Conversation

tejaskash commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Structure

Reuse

Not in this PR

Testing

Uh oh!

codecov-commenter commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tejaskash commented Aug 19, 2026

Uh oh!

Hweinstock Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jariy17 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

tejaskash commented Aug 19, 2026 •

edited

Loading

codecov-commenter commented Aug 19, 2026 •

edited

Loading

Hweinstock Aug 19, 2026 •

edited

Loading