fix(2.0b-O2): review-2 batch — error specificity, half-creates, runbook

isadeks · isadeks · commit e90a09237c2e · 2026-05-25T22:37:31.000-04:00
Addresses four PR review items focused on operator UX when things go sideways: - isWebhookSecretConfigured (review high). The bare `catch { return false }` swallowed AccessDeniedException and DecryptionFailureException, making setup re-prompt for a webhook secret when the real problem was IAM. Now: only ResourceNotFoundException returns false; everything else throws a CliError pointing the operator at the IAM gap. Test updated to assert both paths. - admin invite-user half-create (review medium). If AdminCreateUser succeeds but AdminSetUserPassword fails (stricter password policy than generator, partial IAM grant on the Set verb), the user was left in FORCE_CHANGE_PASSWORD with no diagnostic. Wrap the second call in try/catch and throw a CliError that names the user, explains the broken state, and gives both a delete-user CLI and a manual-fix path. - PAK migration runbook (review non-blocking #1). Expanded the "Migration from 2.0a (PAK) to 2.0b (OAuth)" section in LINEAR_SETUP_GUIDE.md with: a pre-deploy checklist, what survives the migration vs what doesn't, an explicit rollback note (fix forward; the original PAK secret is gone with the CFN resource), and the per-step difference between 2.0a-with-Identity (skipped) vs 2.0a-with-PAK (migrate) deploys. - Vestigial AgentCore Identity dep (review non-blocking #2). bedrock-agentcore==1.9.1 is kept in agent/pyproject.toml because the workload-token bridge in server.py still calls it (now wrapped in try/except per review batch 1). Add an inline comment explaining why it's pinned even though Phase 2.0b-O2 reads Secrets Manager directly — it's the seam for resuming the AgentCore Identity path in 2.0c. CLI tests: 13/13 pass.
diff --git a/agent/pyproject.toml b/agent/pyproject.toml
@@ -5,6 +5,16 @@ description = "Background coding agent — runs tasks in isolated cloud environm
 requires-python = ">=3.13"
 dependencies = [
     "boto3==1.43.9", #https://pypi.org/project/boto3/
+    # Vestigial from the parked AgentCore Identity flow (Phase 2.0a).
+    # Phase 2.0b reads per-workspace Linear OAuth tokens directly from
+    # Secrets Manager because AgentCore Identity's USER_FEDERATION
+    # flow has an open service-side bug (see memory/project_oauth_2_0b.md).
+    # Kept here so the workload-token bridge in `server.py` still
+    # imports cleanly when Phase 2.0c eventually resumes the
+    # AgentCore Identity path. The bridge is now wrapped in
+    # try/except (ImportError, AttributeError), so removing this dep
+    # would degrade gracefully — but for now we keep the dep to
+    # preserve the clean code path.
     "bedrock-agentcore==1.9.1", #https://pypi.org/project/bedrock-agentcore/
     "claude-agent-sdk==0.2.82", #https://github.com/anthropics/claude-agent-sdk-python/releases/tag/v0.2.82
     "requests==2.34.2", #https://pypi.org/project/requests/
diff --git a/cli/src/commands/admin.ts b/cli/src/commands/admin.ts
@@ -169,12 +169,31 @@ export function makeAdminCommand(): Command {
           throw err;
         }
 
-        await cognito.send(new AdminSetUserPasswordCommand({
-          UserPoolId: config.user_pool_id,
-          Username: email,
-          Password: tempPassword,
-          Permanent: true,
-        }));
+        // The user has been created at this point. If `AdminSetUserPassword`
+        // fails (stricter password policy than the generator, partial IAM
+        // grant on the Set verb, throttling, etc.) the user is left in
+        // `FORCE_CHANGE_PASSWORD` state — they exist in the pool but
+        // can't actually log in. Surface a clear diagnostic so the
+        // admin knows to either retry the password set manually or
+        // delete the half-created user before re-running.
+        try {
+          await cognito.send(new AdminSetUserPasswordCommand({
+            UserPoolId: config.user_pool_id,
+            Username: email,
+            Password: tempPassword,
+            Permanent: true,
+          }));
+        } catch (err) {
+          const message = err instanceof Error ? err.message : String(err);
+          const errorName = err instanceof Error ? err.name : 'Error';
+          throw new CliError(
+            `User ${email} was created but the password could not be set `
+            + `(${errorName}: ${message}). The user is now stuck in FORCE_CHANGE_PASSWORD `
+            + 'state and cannot log in. Either:\n'
+            + `  1. Delete the user and re-run: aws cognito-idp admin-delete-user --user-pool-id ${config.user_pool_id} --username ${email}\n`
+            + '  2. Or set the password manually via the AWS console once the underlying issue is fixed.',
+          );
+        }
 
         const bundle = encodeBundle(config);
         printInviteSummary(email, tempPassword, bundle);
diff --git a/cli/src/commands/linear.ts b/cli/src/commands/linear.ts
@@ -164,8 +164,22 @@ export async function isWebhookSecretConfigured(
     const result = await client.send(new GetSecretValueCommand({ SecretId: secretArn }));
     const value = result.SecretString;
     return typeof value === 'string' && value.startsWith('lin_wh_');
-  } catch {
-    return false;
+  } catch (err) {
+    // Only treat "secret doesn't exist yet" as a clean false — any
+    // other error (AccessDenied, KMS decrypt failure, throttling) is
+    // actionable and we should surface it. A bare `catch { return
+    // false }` here makes setup re-prompt for a webhook secret when
+    // the real problem is IAM, which is a confusing UX for operators.
+    const errorName = (err as { name?: string }).name;
+    if (errorName === 'ResourceNotFoundException') {
+      return false;
+    }
+    const message = err instanceof Error ? err.message : String(err);
+    throw new CliError(
+      `Failed to read Linear webhook secret '${secretArn}': ${errorName ?? 'Error'}: ${message}. `
+      + 'Likely IAM permission gap — confirm your CLI principal has '
+      + '`secretsmanager:GetSecretValue` on this ARN.',
+    );
   }
 }
 
diff --git a/cli/test/commands/linear.test.ts b/cli/test/commands/linear.test.ts
@@ -215,11 +215,20 @@ describe('isWebhookSecretConfigured', () => {
     expect(await isWebhookSecretConfigured(mockClient, 'arn:secret')).toBe(false);
   });
 
-  test('returns false on Secrets Manager error (best-effort: re-prompt is harmless)', async () => {
-    mockSend.mockRejectedValueOnce(new Error('AccessDenied'));
+  test('returns false on ResourceNotFoundException (secret has not been created yet)', async () => {
+    const err = new Error('Secrets Manager cannot find the specified secret.');
+    err.name = 'ResourceNotFoundException';
+    mockSend.mockRejectedValueOnce(err);
     expect(await isWebhookSecretConfigured(mockClient, 'arn:secret')).toBe(false);
   });
 
+  test('throws on AccessDenied so operators see the IAM gap instead of a confusing re-prompt', async () => {
+    const err = new Error('User is not authorized to perform: secretsmanager:GetSecretValue');
+    err.name = 'AccessDeniedException';
+    mockSend.mockRejectedValueOnce(err);
+    await expect(isWebhookSecretConfigured(mockClient, 'arn:secret')).rejects.toThrow(/IAM permission gap/);
+  });
+
   test('returns false when SecretString is missing', async () => {
     mockSend.mockResolvedValueOnce({});
     expect(await isWebhookSecretConfigured(mockClient, 'arn:secret')).toBe(false);
diff --git a/docs/guides/LINEAR_SETUP_GUIDE.md b/docs/guides/LINEAR_SETUP_GUIDE.md
@@ -226,16 +226,49 @@ The signing secret in Secrets Manager doesn't match the webhook. Re-run `bgagent
 
 ## Migration from 2.0a (PAK) to 2.0b (OAuth)
 
-If your deployment is on Phase 2.0a (personal API key), 2.0b is a **hard cutover** — there is no `--use-pak` fallback flag. Plan for a maintenance window:
+If your deployment is on Phase 2.0a (personal API key), 2.0b is a **hard cutover** — there is no `--use-pak` fallback flag. Plan for a short maintenance window (typically <30 min for a single workspace).
 
-1. **Drain the queue.** Wait for in-flight tasks to finish. In-flight tasks at upgrade time will fail their final Linear comment because the OAuth token isn't yet authorized when the agent looks for it.
-2. **Deploy 2.0b.** `mise //cdk:deploy`. This adds `LinearWorkspaceRegistryTable`, removes `LinearApiTokenSecret` IAM grants from the agent runtime + Lambdas, and removes the `linear-api-key` AgentCore credential provider's role in the runtime.
-3. **For each Linear workspace, run Steps 1–4 above.** Each workspace needs a new Linear OAuth app, a new AgentCore credential provider (`linear-oauth-<slug>`), and a fresh OAuth authorize via `bgagent linear setup`.
-4. **Verify with a test issue.** Apply the `bgagent` label in each onboarded workspace and confirm the agent posts as `bgagent[bot]` (not as the previous PAK owner's Linear identity).
-5. **Decommission the PAK.** Once 2.0b is verified working, revoke the personal API key in Linear settings ([Linear Settings → Security](https://linear.app/settings/account/security) → Personal API keys → revoke). The PAK is no longer used by any code path; revoking it is a clean break.
-6. **Clean up the old api-key credential provider:** `aws bedrock-agentcore-control delete-api-key-credential-provider --name linear-api-key`.
+> **What changes under the hood.** 2.0a stored a single `LinearApiTokenSecret` (one PAK shared by all teammates) and granted the agent runtime `secretsmanager:GetSecretValue` on that one ARN. 2.0b stores a per-workspace `bgagent-linear-oauth-<slug>` secret containing `{access_token, refresh_token, expires_at, client_id, client_secret, …}`, and replaces the single-ARN grant with a `bgagent-linear-oauth-*` prefix grant. The CDK stack drops the `LinearApiTokenSecret` resource entirely, so there's no automated rollback once 2.0b is deployed.
 
-User mappings in `LinearUserMappingTable` survive the migration — they're keyed on Linear identity, which is unchanged. Project mappings in `LinearProjectMappingTable` likewise survive.
+### Pre-deploy checklist
+
+Run these BEFORE deploying 2.0b so you have everything ready when the maintenance window starts:
+
+1. **List your in-flight tasks.** `bgagent list --status RUNNING --status PENDING` — the migration will not corrupt these, but their final Linear comment may fail because the OAuth token isn't yet authorized when the agent runs.
+2. **Pick one Linear workspace to migrate first.** Multi-workspace orgs should rehearse on the lowest-traffic workspace before doing the rest.
+3. **Note the workspace's `urlKey`** (the `<slug>` in `linear.app/<slug>/...`). You'll need it for `bgagent linear setup <slug>`.
+4. **Confirm CLI admin access.** You need an AWS principal with `secretsmanager:CreateSecret` on `bgagent-linear-oauth-*` AND `dynamodb:PutItem` on `LinearWorkspaceRegistryTable`. Without these, `bgagent linear setup` aborts mid-way (the OAuth dance succeeds, the secret write fails — your Linear OAuth app gets stuck with no usable token).
+
+### Migration steps
+
+1. **Drain the queue.** Wait for in-flight tasks to finish. In-flight tasks at deploy time will fail their final Linear comment because their token resolver short-circuits when neither `LinearApiTokenSecret` (gone) nor `bgagent-linear-oauth-<slug>` (not yet created) is present.
+2. **Deploy 2.0b.** `mise //cdk:deploy`. This adds `LinearWorkspaceRegistryTable`, removes the `LinearApiTokenSecret` resource and IAM grants, and adds the `bgagent-linear-oauth-*` prefix grant on the agent runtime + webhook processor + orchestrator.
+3. **For each Linear workspace, run [Steps 1–4 above](#step-by-step-setup).** Each workspace needs:
+   - A new Linear OAuth app (Settings → API → Applications → Create new app, scopes `read,write,app:assignable,app:mentionable`)
+   - `bgagent linear setup <slug>` to run the OAuth dance and write the per-workspace secret
+   - The webhook signing secret pasted into the Secrets Manager `LinearWebhookSecret` resource
+4. **Re-onboard projects.** If 2.0a had `LinearProjectMappingTable` rows, they survive — but verify with `bgagent linear list-projects` that the listed projects still match what's mapped. The mapping rows are keyed on `linear_project_id` UUID which is stable across the migration.
+5. **Verify with a test issue.** Apply the trigger label in each onboarded workspace and confirm the agent posts as `bgagent[bot]` (not as the previous PAK owner's Linear identity). The author byline change is the cleanest signal that OAuth — not the PAK — is on the wire.
+6. **Decommission the PAK.** Once 2.0b is verified working, revoke the personal API key in Linear settings ([Linear Settings → Security](https://linear.app/settings/account/security) → Personal API keys → revoke). The PAK is no longer used by any code path; revoking is a clean break with no rollback.
+
+### Rollback
+
+If 2.0b fails verification and you need to revert before doing the OAuth setup:
+
+- The `LinearApiTokenSecret` CFN resource has been deleted, so a `cdk deploy` of the previous commit will recreate it but **the secret value will be empty**. You'd need to re-paste the PAK value manually.
+- Recommend instead: **fix-forward**. The 2.0b OAuth dance is a 5-minute step per workspace; rolling back is rarely worth the time.
+
+### What survives the migration
+
+- **`LinearUserMappingTable`** — keyed on Linear identity (organization + user UUID), which is unchanged across PAK→OAuth.
+- **`LinearProjectMappingTable`** — keyed on `linear_project_id` UUID, also stable.
+- **`LinearWebhookDedupTable`** — TTL-bounded; rows from the maintenance window will TTL out within 8h.
+- **GitHub PR comments and Linear-issue mappings** in any in-flight task records.
+
+### What does NOT survive
+
+- `LinearApiTokenSecret` Secrets Manager value — gone with the CDK resource.
+- The 2.0a `linear-api-key` AgentCore credential provider (if 2.0a-with-Identity was deployed mid-Phase) — clean it up after with: `aws bedrock-agentcore-control delete-api-key-credential-provider --name linear-api-key`. Phase 2.0b-O2 does not use AgentCore Identity at all, so there's nothing to clean up if you skipped the parked 2.0a-Identity branch.
 
 ## Limits and budgets
 
diff --git a/docs/src/content/docs/using/Linear-setup-guide.md b/docs/src/content/docs/using/Linear-setup-guide.md
@@ -230,16 +230,49 @@ The signing secret in Secrets Manager doesn't match the webhook. Re-run `bgagent
 
 ## Migration from 2.0a (PAK) to 2.0b (OAuth)
 
-If your deployment is on Phase 2.0a (personal API key), 2.0b is a **hard cutover** — there is no `--use-pak` fallback flag. Plan for a maintenance window:
+If your deployment is on Phase 2.0a (personal API key), 2.0b is a **hard cutover** — there is no `--use-pak` fallback flag. Plan for a short maintenance window (typically <30 min for a single workspace).
 
-1. **Drain the queue.** Wait for in-flight tasks to finish. In-flight tasks at upgrade time will fail their final Linear comment because the OAuth token isn't yet authorized when the agent looks for it.
-2. **Deploy 2.0b.** `mise //cdk:deploy`. This adds `LinearWorkspaceRegistryTable`, removes `LinearApiTokenSecret` IAM grants from the agent runtime + Lambdas, and removes the `linear-api-key` AgentCore credential provider's role in the runtime.
-3. **For each Linear workspace, run Steps 1–4 above.** Each workspace needs a new Linear OAuth app, a new AgentCore credential provider (`linear-oauth-<slug>`), and a fresh OAuth authorize via `bgagent linear setup`.
-4. **Verify with a test issue.** Apply the `bgagent` label in each onboarded workspace and confirm the agent posts as `bgagent[bot]` (not as the previous PAK owner's Linear identity).
-5. **Decommission the PAK.** Once 2.0b is verified working, revoke the personal API key in Linear settings ([Linear Settings → Security](https://linear.app/settings/account/security) → Personal API keys → revoke). The PAK is no longer used by any code path; revoking it is a clean break.
-6. **Clean up the old api-key credential provider:** `aws bedrock-agentcore-control delete-api-key-credential-provider --name linear-api-key`.
+> **What changes under the hood.** 2.0a stored a single `LinearApiTokenSecret` (one PAK shared by all teammates) and granted the agent runtime `secretsmanager:GetSecretValue` on that one ARN. 2.0b stores a per-workspace `bgagent-linear-oauth-<slug>` secret containing `{access_token, refresh_token, expires_at, client_id, client_secret, …}`, and replaces the single-ARN grant with a `bgagent-linear-oauth-*` prefix grant. The CDK stack drops the `LinearApiTokenSecret` resource entirely, so there's no automated rollback once 2.0b is deployed.
 
-User mappings in `LinearUserMappingTable` survive the migration — they're keyed on Linear identity, which is unchanged. Project mappings in `LinearProjectMappingTable` likewise survive.
+### Pre-deploy checklist
+
+Run these BEFORE deploying 2.0b so you have everything ready when the maintenance window starts:
+
+1. **List your in-flight tasks.** `bgagent list --status RUNNING --status PENDING` — the migration will not corrupt these, but their final Linear comment may fail because the OAuth token isn't yet authorized when the agent runs.
+2. **Pick one Linear workspace to migrate first.** Multi-workspace orgs should rehearse on the lowest-traffic workspace before doing the rest.
+3. **Note the workspace's `urlKey`** (the `<slug>` in `linear.app/<slug>/...`). You'll need it for `bgagent linear setup <slug>`.
+4. **Confirm CLI admin access.** You need an AWS principal with `secretsmanager:CreateSecret` on `bgagent-linear-oauth-*` AND `dynamodb:PutItem` on `LinearWorkspaceRegistryTable`. Without these, `bgagent linear setup` aborts mid-way (the OAuth dance succeeds, the secret write fails — your Linear OAuth app gets stuck with no usable token).
+
+### Migration steps
+
+1. **Drain the queue.** Wait for in-flight tasks to finish. In-flight tasks at deploy time will fail their final Linear comment because their token resolver short-circuits when neither `LinearApiTokenSecret` (gone) nor `bgagent-linear-oauth-<slug>` (not yet created) is present.
+2. **Deploy 2.0b.** `mise //cdk:deploy`. This adds `LinearWorkspaceRegistryTable`, removes the `LinearApiTokenSecret` resource and IAM grants, and adds the `bgagent-linear-oauth-*` prefix grant on the agent runtime + webhook processor + orchestrator.
+3. **For each Linear workspace, run [Steps 1–4 above](#step-by-step-setup).** Each workspace needs:
+   - A new Linear OAuth app (Settings → API → Applications → Create new app, scopes `read,write,app:assignable,app:mentionable`)
+   - `bgagent linear setup <slug>` to run the OAuth dance and write the per-workspace secret
+   - The webhook signing secret pasted into the Secrets Manager `LinearWebhookSecret` resource
+4. **Re-onboard projects.** If 2.0a had `LinearProjectMappingTable` rows, they survive — but verify with `bgagent linear list-projects` that the listed projects still match what's mapped. The mapping rows are keyed on `linear_project_id` UUID which is stable across the migration.
+5. **Verify with a test issue.** Apply the trigger label in each onboarded workspace and confirm the agent posts as `bgagent[bot]` (not as the previous PAK owner's Linear identity). The author byline change is the cleanest signal that OAuth — not the PAK — is on the wire.
+6. **Decommission the PAK.** Once 2.0b is verified working, revoke the personal API key in Linear settings ([Linear Settings → Security](https://linear.app/settings/account/security) → Personal API keys → revoke). The PAK is no longer used by any code path; revoking is a clean break with no rollback.
+
+### Rollback
+
+If 2.0b fails verification and you need to revert before doing the OAuth setup:
+
+- The `LinearApiTokenSecret` CFN resource has been deleted, so a `cdk deploy` of the previous commit will recreate it but **the secret value will be empty**. You'd need to re-paste the PAK value manually.
+- Recommend instead: **fix-forward**. The 2.0b OAuth dance is a 5-minute step per workspace; rolling back is rarely worth the time.
+
+### What survives the migration
+
+- **`LinearUserMappingTable`** — keyed on Linear identity (organization + user UUID), which is unchanged across PAK→OAuth.
+- **`LinearProjectMappingTable`** — keyed on `linear_project_id` UUID, also stable.
+- **`LinearWebhookDedupTable`** — TTL-bounded; rows from the maintenance window will TTL out within 8h.
+- **GitHub PR comments and Linear-issue mappings** in any in-flight task records.
+
+### What does NOT survive
+
+- `LinearApiTokenSecret` Secrets Manager value — gone with the CDK resource.
+- The 2.0a `linear-api-key` AgentCore credential provider (if 2.0a-with-Identity was deployed mid-Phase) — clean it up after with: `aws bedrock-agentcore-control delete-api-key-credential-provider --name linear-api-key`. Phase 2.0b-O2 does not use AgentCore Identity at all, so there's nothing to clean up if you skipped the parked 2.0a-Identity branch.
 
 ## Limits and budgets
 
