fix(idc): address cubic review findings

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LJV2dCcAYvf1JuxquCMZAd

Author: waleedlatif1

apps/docs/content/docs/integrations/identity_center.mdx

@@ -22,12 +22,14 @@ With AWS IAM Identity Center, you can:
 - **Monitor assignment status**: Poll the provisioning status of create/delete operations, which are asynchronous in AWS
 - **List accounts in your organization**: Enumerate all AWS accounts in your AWS Organizations structure to populate access request dropdowns
 - **Manage groups**: List groups, resolve group IDs by display name, and enumerate group memberships for group-based access grants
-- **Audit an account's access**: List every assignment on a given AWS account, then resolve each principal ID back to the user or group behind it
+- **Audit an account's access**: List the assignments on a given AWS account for one permission set, then resolve each principal ID back to the user or group behind it
 
 In Sim, the AWS Identity Center integration is designed to power **TEAM (Temporary Elevated Access Management)** workflows — automated pipelines where users request elevated access, approvers approve or deny it, access is provisioned with a time limit, and auto-revocation removes it when the window expires. This replaces manual console-based access management with auditable, agent-driven workflows that integrate with Slack, email, ticketing systems, and CloudTrail for full traceability.
 
 The same operations support the reverse direction — access review. Starting from an account, an agent can list its assignments, resolve the principals, expand groups into their members, and produce a written report of exactly who can reach that account and through which permission set.
 
+One detail shapes how that review has to be built. AWS requires a permission set ARN alongside the account ID on this call, so *List Assignments For Account* returns only the assignments granted through that one permission set — not every assignment on the account. To cover an account completely, run *List Permission Sets* for the instance first, then call *List Assignments For Account* once per permission set and combine the results. Skipping that loop silently omits access granted through the permission sets you did not ask about.
+
 Two AWS behaviors are worth knowing. Creating and deleting an account assignment are **asynchronous**: both return a request ID, and each has its own status poller — use *Check Assignment Status* for creations and *Check Assignment Deletion Status* for deletions, as the two request-ID types are not interchangeable. And the account-listing operations call AWS Organizations, which is global per partition; the block resolves the correct endpoint for commercial, GovCloud, and China regions automatically.
 {/* MANUAL-CONTENT-END */}
 
@@ -192,10 +194,10 @@ Resolve an Identity Store user ID to the user behind it. Use to turn the princip
 | --------- | ---- | ----------- |
 | `userId` | string | Identity Store user ID |
 | `userName` | string | Username in the Identity Store |
-| `displayName` | string | Display name of the user |
-| `email` | string | Primary email address |
-| `userStatus` | string | Account status: ENABLED or DISABLED |
-| `title` | string | Job title |
+| `displayName` | string | Display name of the user, or null when the Identity Store omits it |
+| `email` | string | Primary email address, or null when the user has no email attribute |
+| `userStatus` | string | Account status \(ENABLED or DISABLED\), or null when the Identity Store omits it |
+| `title` | string | Job title, or null when the Identity Store omits it |
 | `externalIds` | array | External identity provider IDs linked to the user |
 |   ↳ `issuer` | string | Identity provider that issued the ID |
 |   ↳ `id` | string | Identifier at the issuer |

apps/sim/blocks/blocks/identity_center.ts

@@ -45,7 +45,7 @@ export const IdentityCenterBlock: BlockConfig<IdentityCenterBaseResponse> = {
         ],
         list_assignments_for_account: [
           { text: 'List assignments on account', field: 'accountId', core: true },
-          { text: 'for permission set', field: 'permissionSetArn' },
+          { text: 'for permission set', field: 'permissionSetArn', core: true },
         ],
         describe_user: [{ text: 'Look up the user with ID', field: 'userId', core: true }],
         describe_group: [{ text: 'Look up the group with ID', field: 'groupId', core: true }],

apps/sim/lib/api/contracts/tools/aws/identity-center-list-accounts.test.ts

@@ -0,0 +1,48 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it } from 'vitest'
+import { awsIdentityCenterListAccountsContract } from '@/lib/api/contracts/tools/aws/identity-center-list-accounts'
+import { awsIdentityCenterListAssignmentsForAccountContract } from '@/lib/api/contracts/tools/aws/identity-center-list-assignments-for-account'
+
+const connection = {
+  region: 'us-east-1',
+  accessKeyId: 'AKIAEXAMPLE',
+  secretAccessKey: 'secret',
+}
+
+/**
+ * Organizations documents a 100,000-character maximum for the `ListAccounts`
+ * continuation token, well past the Identity Store bound the rest of the family
+ * shares.
+ *
+ * @see https://docs.aws.amazon.com/organizations/latest/APIReference/API_ListAccounts.html
+ */
+describe('identity center list-accounts nextToken bound', () => {
+  it('accepts an Organizations token longer than the Identity Store bound', () => {
+    const parsed = awsIdentityCenterListAccountsContract.body?.safeParse({
+      ...connection,
+      nextToken: 'a'.repeat(100_000),
+    })
+    expect(parsed?.success).toBe(true)
+  })
+
+  it('still rejects a token past the documented Organizations maximum', () => {
+    const parsed = awsIdentityCenterListAccountsContract.body?.safeParse({
+      ...connection,
+      nextToken: 'a'.repeat(100_001),
+    })
+    expect(parsed?.success).toBe(false)
+  })
+
+  it('leaves the SSO Admin token bound where it was', () => {
+    const parsed = awsIdentityCenterListAssignmentsForAccountContract.body?.safeParse({
+      ...connection,
+      instanceArn: 'arn:aws:sso:::instance/ssoins-0123456789abcdef',
+      accountId: '111111111111',
+      permissionSetArn: 'arn:aws:sso:::permissionSet/ssoins-0123456789abcdef/ps-0123456789abcdef',
+      nextToken: 'a'.repeat(100_000),
+    })
+    expect(parsed?.success).toBe(false)
+  })
+})

apps/sim/lib/api/contracts/tools/aws/identity-center-list-accounts.ts

@@ -2,7 +2,7 @@ import { z } from 'zod'
 import {
   identityCenterAccountsMaxResultsSchema,
   identityCenterConnectionShape,
-  identityCenterNextTokenSchema,
+  identityCenterOrganizationsNextTokenSchema,
 } from '@/lib/api/contracts/tools/aws/identity-center-shared'
 import type {
   ContractBody,
@@ -14,7 +14,7 @@ import { defineRouteContract } from '@/lib/api/contracts/types'
 const Schema = z.object({
   ...identityCenterConnectionShape,
   maxResults: identityCenterAccountsMaxResultsSchema.optional(),
-  nextToken: identityCenterNextTokenSchema.optional(),
+  nextToken: identityCenterOrganizationsNextTokenSchema.optional(),
 })
 
 const ResponseSchema = z.object({

apps/sim/lib/api/contracts/tools/aws/identity-center-shared.ts

@@ -109,6 +109,18 @@ export const identityCenterNextTokenSchema = z
   .min(1, 'Pagination token cannot be empty')
   .max(65535, 'Pagination token is too long')
 
+/**
+ * Organizations pagination tokens are documented far longer than the Identity
+ * Store bound above, so `ListAccounts` gets its own ceiling rather than sharing
+ * one that would reject a valid continuation token before AWS sees it.
+ *
+ * @see https://docs.aws.amazon.com/organizations/latest/APIReference/API_ListAccounts.html
+ */
+export const identityCenterOrganizationsNextTokenSchema = z
+  .string()
+  .min(1, 'Pagination token cannot be empty')
+  .max(100000, 'Pagination token is too long')
+
 /** Every list operation in the family except Organizations `ListAccounts`. */
 export const identityCenterMaxResultsSchema = z
   .number()

apps/sim/lib/internal/identity-center/concurrency.test.ts

@@ -93,4 +93,34 @@ describe('withThrottleRetry', () => {
     await expect(withThrottleRetry(operation, controller.signal)).rejects.toThrow()
     expect(operation).not.toHaveBeenCalled()
   })
+
+  it('exits a throttling backoff as soon as the caller aborts', async () => {
+    const controller = new AbortController()
+    let calls = 0
+    let abortedAt = 0
+
+    /**
+     * Aborts on the third attempt, whose pending backoff is at least 640ms
+     * (200ms base doubled twice, minus the 20% jitter floor). A backoff that
+     * ignored the signal would therefore keep the caller waiting far longer
+     * than the assertion below allows.
+     */
+    const operation = vi.fn(async () => {
+      calls++
+      if (calls === 3) {
+        abortedAt = Date.now()
+        controller.abort()
+      }
+      throw throttlingError()
+    })
+
+    const rejection = await withThrottleRetry(operation, controller.signal).catch(
+      (error: unknown) => error
+    )
+    const elapsedSinceAbort = Date.now() - abortedAt
+
+    expect((rejection as Error).name).toBe('AbortError')
+    expect(operation).toHaveBeenCalledTimes(3)
+    expect(elapsedSinceAbort).toBeLessThan(200)
+  })
 })

apps/sim/lib/internal/identity-center/concurrency.ts

@@ -1,4 +1,4 @@
-import { sleep } from '@sim/utils/helpers'
+import { interruptibleSleep } from '@sim/utils/helpers'
 import { backoffWithJitter } from '@sim/utils/retry'
 
 /**
@@ -34,6 +34,10 @@ function isThrottlingError(error: unknown): boolean {
  * Runs an AWS call, retrying with jittered exponential backoff while the
  * service reports throttling. Non-throttling failures and aborts propagate
  * immediately.
+ *
+ * The backoff itself is interruptible: an abort during the wait resolves it
+ * early, so the next loop iteration's `throwIfAborted` fires without the caller
+ * having to sit out the remaining delay.
  */
 export async function withThrottleRetry<T>(
   operation: () => Promise<T>,
@@ -45,7 +49,10 @@ export async function withThrottleRetry<T>(
       return await operation()
     } catch (error) {
       if (attempt >= MAX_THROTTLE_ATTEMPTS || !isThrottlingError(error)) throw error
-      await sleep(backoffWithJitter(attempt, null, { baseMs: 200, maxMs: 5_000 }))
+      await interruptibleSleep(
+        backoffWithJitter(attempt, null, { baseMs: 200, maxMs: 5_000 }),
+        signal
+      )
     }
   }
 }

apps/sim/tools/generated/tool-outputs.ts

@@ -0,0 +1,50 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it } from 'vitest'
+import { describeUserTool } from '@/tools/identity_center/describe_user'
+
+/**
+ * Identity Store `DescribeUser` marks `DisplayName`, `Emails`, `UserStatus` and
+ * `Title` optional, so the transform substitutes `null` for each. The output
+ * metadata has to advertise that same shape.
+ *
+ * @see https://docs.aws.amazon.com/singlesignon/latest/IdentityStoreAPIReference/API_DescribeUser.html
+ */
+const NULLABLE_OUTPUTS = ['displayName', 'email', 'userStatus', 'title'] as const
+
+function jsonResponse(body: unknown): Response {
+  return new Response(JSON.stringify(body), {
+    status: 200,
+    headers: { 'content-type': 'application/json' },
+  })
+}
+
+describe('identity_center_describe_user outputs', () => {
+  it('keeps the tool id byte-identical', () => {
+    expect(describeUserTool.id).toBe('identity_center_describe_user')
+  })
+
+  it.each(NULLABLE_OUTPUTS)('declares %s nullable rather than optional', (key) => {
+    const output = describeUserTool.outputs?.[key]
+    expect(output).toBeDefined()
+    expect(output?.nullable).toBe(true)
+    expect(output?.optional).toBeUndefined()
+  })
+
+  it('emits null for every field the Identity Store omitted', async () => {
+    const result = await describeUserTool.transformResponse?.(
+      jsonResponse({ userId: 'u-1', userName: 'someone' })
+    )
+
+    expect(result?.output).toEqual({
+      userId: 'u-1',
+      userName: 'someone',
+      displayName: null,
+      email: null,
+      userStatus: null,
+      title: null,
+      externalIds: [],
+    })
+  })
+})

apps/sim/tools/identity_center/describe_user.test.ts

@@ -79,14 +79,26 @@ export const describeUserTool: InternalToolConfig<
   outputs: {
     userId: { type: 'string', description: 'Identity Store user ID' },
     userName: { type: 'string', description: 'Username in the Identity Store' },
-    displayName: { type: 'string', description: 'Display name of the user', optional: true },
-    email: { type: 'string', description: 'Primary email address', optional: true },
+    displayName: {
+      type: 'string',
+      description: 'Display name of the user, or null when the Identity Store omits it',
+      nullable: true,
+    },
+    email: {
+      type: 'string',
+      description: 'Primary email address, or null when the user has no email attribute',
+      nullable: true,
+    },
     userStatus: {
       type: 'string',
-      description: 'Account status: ENABLED or DISABLED',
-      optional: true,
+      description: 'Account status (ENABLED or DISABLED), or null when the Identity Store omits it',
+      nullable: true,
+    },
+    title: {
+      type: 'string',
+      description: 'Job title, or null when the Identity Store omits it',
+      nullable: true,
     },
-    title: { type: 'string', description: 'Job title', optional: true },
     externalIds: {
       type: 'array',
       description: 'External identity provider IDs linked to the user',