feat(online-eval): add --output-config and --tags, --description on update, and widen the managed role

src/core/eval.tsx

@@ -123,6 +123,8 @@ import type {
   DatasetUpdateProgressEvent,
   DatasetUpdateResult,
   RoleScopeWarning,
+  RoleScopeKind,
+  OnlineEvalOutputConfig,
   CoreEvalClient,
   CreateConfigurationBundleInput,
   CreateConfigBasedABTestInput,
@@ -989,6 +991,9 @@ export class EvalClient implements CoreEvalClient {
           options.region,
           logGroupNamesOf(dataSourceConfig),
           await evaluatorKmsKeys(input.evaluatorIds ?? [], control),
+          // Read only to widen the write scope to the chosen destination; the
+          // request object below still gets the caller's object untouched.
+          { outputConfig: input.outputConfig },
         )
       ).roleArn;
 
@@ -998,8 +1003,10 @@ export class EvalClient implements CoreEvalClient {
       rule: toRule(input.samplingRate, input.sessionTimeoutMinutes, input.filters),
       dataSourceConfig,
       evaluators: input.evaluatorIds?.map((evaluatorId) => ({ evaluatorId })),
+      outputConfig: input.outputConfig,
       evaluationExecutionRoleArn,
       enableOnCreate: input.enableOnCreate ?? true,
+      tags: input.tags,
     });
 
     // A role provisioned moments ago may not be assumable yet (IAM is eventually
@@ -1237,6 +1244,19 @@ export class EvalClient implements CoreEvalClient {
         ? dataSourceConfig
         : undefined;
 
+    // A new destination moves the role's *write* scope the same way a new source
+    // moves its read scope. Supplying --output-config counts as a move without
+    // comparing it to the stored one: re-granting an identical scope is a no-op
+    // (the policy's name is a hash of its document, so the write is idempotent
+    // and the revoke below is skipped), which is cheaper than a deep compare.
+    const outputMoved = update.outputConfig !== undefined;
+    // The policy has to cover the destination in force after this update, not
+    // just a new one — otherwise moving only the source would drop the write
+    // scope for a destination set earlier.
+    const effectiveOutputConfig = update.outputConfig ?? current.outputConfig;
+    const scopeKind: RoleScopeKind =
+      movedTo !== undefined && outputMoved ? "input-and-output" : outputMoved ? "output" : "input";
+
     const configName = current.onlineEvaluationConfigName;
     const roleArn = update.evaluationExecutionRoleArn ?? current.evaluationExecutionRoleArn;
     const managedRoleName =
@@ -1246,26 +1266,38 @@ export class EvalClient implements CoreEvalClient {
       isManagedOnlineEvalRole(roleArn, configName)
         ? configName
         : undefined;
-    const refreshManagedRole = movedTo !== undefined && managedRoleName !== undefined;
-
-    if (movedTo !== undefined && managedRoleName === undefined && roleArn) {
+    const scopeChanged = movedTo !== undefined || outputMoved;
+    const refreshManagedRole = scopeChanged && managedRoleName !== undefined;
+    // Names the groups the caller has to grant by hand when the CLI cannot: the
+    // moved source, the moved destination, or both.
+    const affectedLogGroups = [
+      ...(movedTo !== undefined ? logGroupNamesOf(movedTo) : []),
+      ...(outputMoved ? destinationLogGroupNames(update.outputConfig, dataSourceConfig) : []),
+    ];
+
+    if (scopeChanged && managedRoleName === undefined && roleArn) {
       roleScopeWarning = {
         reason: "custom-role",
         roleArn,
-        logGroupNames: logGroupNamesOf(movedTo),
+        scope: scopeKind,
+        logGroupNames: affectedLogGroups,
       };
-    } else if (movedTo !== undefined && !refreshManagedRole && roleArn) {
+    } else if (scopeChanged && !refreshManagedRole && roleArn) {
       // managed role, but the caller declined the refresh
       roleScopeWarning = {
         reason: "update-declined",
         roleArn,
-        logGroupNames: logGroupNamesOf(movedTo),
+        scope: scopeKind,
+        logGroupNames: affectedLogGroups,
       };
     }
 
     if (refreshManagedRole && update.updateRole !== false) {
       const iam = this.clients.iam({ region: options.region });
-      const newLogGroups = logGroupNamesOf(movedTo);
+      // The source in force after this update, which is the current one when only
+      // the output destination moved — reading `movedTo` here would drop the
+      // query scope entirely in that case.
+      const newLogGroups = dataSourceConfig ? logGroupNamesOf(dataSourceConfig) : [];
       const oldLogGroups = current.dataSourceConfig
         ? logGroupNamesOf(current.dataSourceConfig)
         : [];
@@ -1290,23 +1322,26 @@ export class EvalClient implements CoreEvalClient {
         options.region,
         newLogGroups,
         kmsKeys,
-        resourceNameFromArn(roleArn!),
+        { roleName: resourceNameFromArn(roleArn!), outputConfig: effectiveOutputConfig },
       );
       const oldPolicyName = scopePolicyName(
         executionPolicy(
           options.region,
           accountIdFromRoleArn(managedRoleArn),
           oldLogGroups,
           kmsKeys,
+          current.outputConfig,
         ),
       );
 
       const response = await control.send(
         new UpdateOnlineEvaluationConfigCommand({
           onlineEvaluationConfigId: id,
+          description: update.description,
           rule: toRule(samplingPercentage, sessionTimeoutMinutes, filters),
           dataSourceConfig,
           evaluators,
+          outputConfig: update.outputConfig,
         }),
       );
 
@@ -1323,6 +1358,7 @@ export class EvalClient implements CoreEvalClient {
           roleScopeWarning = {
             reason: "stale-scope",
             roleArn: roleArn!,
+            scope: scopeKind,
             logGroupNames: oldLogGroups,
           };
         }
@@ -1333,9 +1369,11 @@ export class EvalClient implements CoreEvalClient {
     const response = await control.send(
       new UpdateOnlineEvaluationConfigCommand({
         onlineEvaluationConfigId: id,
+        description: update.description,
         rule: toRule(samplingPercentage, sessionTimeoutMinutes, filters),
         dataSourceConfig,
         evaluators,
+        outputConfig: update.outputConfig,
         evaluationExecutionRoleArn: update.evaluationExecutionRoleArn,
       }),
     );
@@ -2201,6 +2239,23 @@ function logGroupNamesOf(dataSourceConfig: DataSourceConfig): string[] {
     : [];
 }
 
+// destinationLogGroupNames names the groups results are written into, for a
+// warning telling the caller what to grant by hand. SOURCE_LOG_GROUP writes back
+// into the sampled groups, so it reports those; a named group reports itself; and
+// the service-managed default reports nothing, since no caller-owned group is
+// involved.
+function destinationLogGroupNames(
+  outputConfig: OnlineEvalOutputConfig | undefined,
+  dataSourceConfig: DataSourceConfig | undefined,
+): string[] {
+  const cloudWatch = outputConfig?.cloudWatchConfig;
+  if (!cloudWatch) return [];
+  if (cloudWatch.resultDestination === "SOURCE_LOG_GROUP") {
+    return dataSourceConfig ? logGroupNamesOf(dataSourceConfig) : [];
+  }
+  return cloudWatch.logGroupName ? [cloudWatch.logGroupName] : [];
+}
+
 // runtimeIdFromLogGroup recovers the runtime id embedded in a log group path
 // produced by runtimeLogGroup, so an update can re-derive dataSourceConfig for a
 // new --endpoint without the caller passing --agent again. Returns undefined for

src/core/onlineEvalExecutionRole.test.ts

@@ -110,3 +110,79 @@ test("gives identical policies the same name", () => {
     scopePolicyName(executionPolicy(REGION, ACCOUNT, ["/a*", "/b*"], [])),
   );
 });
+
+// --- result destination scoping -------------------------------------------
+
+function writeStatement(policy: string) {
+  return statements(policy).find((s) => s.Sid === "WriteEvaluationResults");
+}
+
+const SERVICE_RESULTS = `arn:aws:logs:${REGION}:${ACCOUNT}:log-group:/aws/bedrock-agentcore/evaluations/*`;
+
+// This is the invariant that keeps existing configs working. The policy document
+// is hashed to name the inline policy, so a config with no custom destination has
+// to render byte-for-byte what it was granted under — including Resource being a
+// bare string rather than a one-element array.
+test("a config with no output destination keeps the service namespace as a bare string", () => {
+  const write = writeStatement(executionPolicy(REGION, ACCOUNT, LOG_GROUPS, []));
+
+  expect(write?.Resource).toBe(SERVICE_RESULTS);
+  expect(Array.isArray(write?.Resource)).toBe(false);
+});
+
+test("a customer-named dedicated group is granted alongside the service namespace", () => {
+  const write = writeStatement(
+    executionPolicy(REGION, ACCOUNT, LOG_GROUPS, [], {
+      cloudWatchConfig: {
+        logGroupName: "/company/agent-evaluations",
+        resultDestination: "DEDICATED_LOG_GROUP",
+      },
+    }),
+  );
+
+  expect(write?.Resource).toEqual([
+    SERVICE_RESULTS,
+    `arn:aws:logs:${REGION}:${ACCOUNT}:log-group:/company/agent-evaluations*`,
+  ]);
+  // The service creates the group when it does not exist, which needs this.
+  expect(write?.Action).toContain("logs:CreateLogGroup");
+});
+
+test("SOURCE_LOG_GROUP grants writes to the groups the traces are read from", () => {
+  const write = writeStatement(
+    executionPolicy(REGION, ACCOUNT, LOG_GROUPS, [], {
+      cloudWatchConfig: { resultDestination: "SOURCE_LOG_GROUP" },
+    }),
+  );
+
+  // Scoped to the runtime prefix, matching how the query statement scopes them.
+  expect(write?.Resource).toEqual([
+    SERVICE_RESULTS,
+    `arn:aws:logs:${REGION}:${ACCOUNT}:log-group:/aws/bedrock-agentcore/runtimes/orders-agent-abc123*`,
+  ]);
+});
+
+test("a destination already inside the service namespace adds nothing", () => {
+  // What the service echoes back for a default config. The wildcard above
+  // already covers it, and adding it would change the document — and so the
+  // policy's name — for a config that never asked for a custom destination.
+  const write = writeStatement(
+    executionPolicy(REGION, ACCOUNT, LOG_GROUPS, [], {
+      cloudWatchConfig: {
+        logGroupName: "/aws/bedrock-agentcore/evaluations/online-evaluations/results/default",
+        resultDestination: "DEDICATED_LOG_GROUP",
+      },
+    }),
+  );
+
+  expect(write?.Resource).toBe(SERVICE_RESULTS);
+});
+
+test("changing the destination changes the policy name, so a re-scope is a new grant", () => {
+  const before = executionPolicy(REGION, ACCOUNT, LOG_GROUPS, []);
+  const after = executionPolicy(REGION, ACCOUNT, LOG_GROUPS, [], {
+    cloudWatchConfig: { logGroupName: "/company/agent-evaluations" },
+  });
+
+  expect(scopePolicyName(after)).not.toBe(scopePolicyName(before));
+});

src/core/onlineEvalExecutionRole.tsx

@@ -83,6 +83,56 @@ function runtimeLogGroupPrefix(logGroupName: string): string {
   return match?.[1] ?? logGroupName;
 }
 
+// The namespace the service writes results into when no destination is
+// configured. Reserved: a customer-supplied logGroupName cannot sit under it.
+const SERVICE_RESULT_PREFIX = "/aws/bedrock-agentcore/evaluations/";
+
+// resultWriteArns lists the log-group ARNs the execution role must be able to
+// write results into.
+//
+// The service-managed namespace is always included, for two reasons: it is where
+// results go when no destination is configured, and keeping it unconditional
+// means a config created before custom destinations existed still renders the
+// byte-identical policy document it was granted under. That matters because the
+// document's hash is the inline policy's name (see scopePolicyName), so a
+// document that shifts under an existing config would orphan its policy.
+function resultWriteArns(
+  logs: string,
+  sampledArns: string[],
+  outputConfig: OnlineEvalResultDestination | undefined,
+): string | string[] {
+  const arns = [`${logs}:${SERVICE_RESULT_PREFIX}*`];
+  const cloudWatch = outputConfig?.cloudWatchConfig;
+
+  if (cloudWatch?.resultDestination === "SOURCE_LOG_GROUP") {
+    // Results are written back into the groups the traces were read from, which
+    // the query scope already enumerates.
+    arns.push(...sampledArns);
+  } else if (
+    cloudWatch?.logGroupName &&
+    !cloudWatch.logGroupName.startsWith(SERVICE_RESULT_PREFIX)
+  ) {
+    // A trailing wildcard covers both the group ARN and its `:*` stream form,
+    // matching how the sampled groups above are scoped.
+    arns.push(`${logs}:${cloudWatch.logGroupName}*`);
+  }
+
+  // A lone ARN stays a bare string rather than becoming a one-element array.
+  // IAM treats the two identically, but this document's exact text is hashed to
+  // name the inline policy (see scopePolicyName), so wrapping it would rename
+  // every policy already attached to a config that has no custom destination —
+  // orphaning the grant it is currently running on.
+  return arns.length === 1 ? arns[0]! : arns;
+}
+
+// OnlineEvalResultDestination is the part of the online-evaluation OutputConfig
+// that changes what the role must be allowed to write. Narrowed to what the
+// policy needs rather than taking the SDK type, so the request object cannot be
+// reached from here — the destination is read to compute ARNs, never mutated.
+export type OnlineEvalResultDestination = {
+  cloudWatchConfig?: { logGroupName?: string; resultDestination?: string } | undefined;
+};
+
 // executionPolicy grants the permissions CreateOnlineEvaluationConfig validates
 // at creation time. Exported for assertion: the policy body is not observable
 // through the recorded IAM fixtures, whose responses are empty.
@@ -98,6 +148,7 @@ export function executionPolicy(
   accountId: string,
   logGroupNames: string[],
   kmsKeyArns: string[],
+  outputConfig?: OnlineEvalResultDestination,
 ): string {
   const logs = `arn:aws:logs:${region}:${accountId}:log-group`;
   const spansArn = `${logs}:aws/spans`;
@@ -134,6 +185,8 @@ export function executionPolicy(
         Resource: [`${spansArn}*`, ...sampledArns],
       },
       {
+        // logs:CreateLogGroup is needed because the service creates a
+        // customer-named result group that does not exist yet.
         Sid: "WriteEvaluationResults",
         Effect: "Allow",
         Action: [
@@ -142,7 +195,7 @@ export function executionPolicy(
           "logs:DescribeLogStreams",
           "logs:PutLogEvents",
         ],
-        Resource: `${logs}:/aws/bedrock-agentcore/evaluations/*`,
+        Resource: resultWriteArns(logs, sampledArns, outputConfig),
       },
       {
         Sid: "IndexSpans",
@@ -201,6 +254,15 @@ export function scopePolicyName(policyDocument: string): string {
   return `${POLICY_PREFIX}-${fingerprint(policyDocument)}`;
 }
 
+// GrantScopeOptions carries the two settings that only some callers need:
+// `roleName` when the role's actual name cannot be re-derived from the config
+// name (an update reads it off the stored ARN), and `outputConfig` when results
+// go somewhere other than the service-managed namespace.
+export type GrantScopeOptions = {
+  roleName?: string;
+  outputConfig?: OnlineEvalResultDestination;
+};
+
 // grantOnlineEvalScope creates the execution role for `configName` if it does not
 // exist and attaches the inline policy for this scope, returning the role ARN and
 // the policy name written. The caller revokes the superseded scope once whatever
@@ -211,7 +273,7 @@ export async function grantOnlineEvalScope(
   region: string,
   logGroupNames: string[],
   kmsKeyArns: string[] = [],
-  roleName = onlineEvalExecutionRoleName(configName),
+  { roleName = onlineEvalExecutionRoleName(configName), outputConfig }: GrantScopeOptions = {},
 ): Promise<{ roleArn: string; policyName: string }> {
   let roleArn: string;
   try {
@@ -234,6 +296,7 @@ export async function grantOnlineEvalScope(
     accountIdFromRoleArn(roleArn),
     logGroupNames,
     kmsKeyArns,
+    outputConfig,
   );
   const policyName = scopePolicyName(policyDocument);
   await iam.send(