feat(linear): OAuth migration with per-workspace token storage (Phase 2.0b)

[isadeks]

Summary

Migrates Linear integration from per-installer Personal API Keys to OAuth — installers run bgagent linear setup, the CLI does the OAuth dance directly with Linear, and the resulting access_token/refresh_token lands in a per-workspace Secrets Manager secret (bgagent-linear-oauth-<slug>). All Linear-touching Lambdas (webhook processor, orchestrator) and the agent runtime resolve tokens at invocation time from Secrets Manager via a workspace-id → registry → secret-arn lookup, with lazy refresh on a 60s expiry threshold.

This unblocks teammate onboarding (one OAuth install per Linear workspace, all teammates in the workspace can submit tasks) and removes the personal-token sharing pattern.

Approach

• AgentCore Identity parked — initial design used GetResourceOauth2Token USER_FEDERATION, but the call hangs in IN_PROGRESS forever (filed upstream as a parked path). Pivoted to direct OAuth with the CLI hosting an ephemeral localhost callback server.
• Wave A — OAuth helpers + per-workspace Secrets Manager pattern (StoredLinearOauthToken schema includes client_id + client_secret co-located so refresh works without env vars).
• Wave B — bgagent linear setup rewritten: PKCE S256, actor=app, browser open, callback capture, code-for-token exchange, store in Secrets Manager, write registry + user-mapping rows.
• Wave C — webhook processor + orchestrator + agent runtime now resolve tokens via LinearWorkspaceRegistryTable lookup; cognito-sub stamped into channel_metadata.linear_oauth_secret_arn so the agent reads the secret directly.

Notable fixes shipped along the way

• esbuild stubs import.meta = {} when bundling ESM into CJS, breaking @aws/durable-execution-sdk-js@1.1.3's fileURLToPath(import.meta.url) at module load. Fixed via define + banner substitution. Refs [Bug]: v1.1.3 breaks when bundled to CJS - Regression from 1.1.2 aws/aws-durable-execution-sdk-js#543.
• OAuth callback switched from self-signed-cert HTTPS to plain http://localhost:8080 per RFC 8252 §7.3 — providers (incl. Linear) treat localhost as a TLS-exempt special case, and the cert warning was scaring testers off mid-setup.
• Linear scope param requires space-separated values per RFC 6749 §3.3, not comma-separated (caught by "Invalid redirect_uri" error after a comma was rejected).

Test plan

• CDK + CLI unit tests pass locally
• End-to-end smoke on backgroundagent-dev:
  • bgagent linear setup completes OAuth dance, writes secret + registry row
  • Labeling a Linear issue triggers webhook → task creation → orchestrator → agent runtime
  • Agent comments back on Linear issue + opens PR
• Reviewer to verify per-workspace secret model handles multi-workspace installs (followup task tracked: chore(deps): bump basic-ftp from 5.3.0 to 5.3.1 #67 bgagent linear add-workspace)
• Reviewer to confirm migration story for existing PAK installs (followup: D4 runbook, chore(deps): bump fast-uri from 3.1.0 to 3.1.2 #71)

Followups (parked, not blocking this PR)

• Stable HTTPS callback (vs ephemeral localhost) — local-dev tradeoff, not a blocker
• bgagent linear add-workspace for installing in additional workspaces
• Migration runbook for installs still on PAK secrets

🤖 Generated with Claude Code

[krokoko]

Principal Architect Review

Overall: The per-workspace OAuth architecture is sound — multi-tenant isolation, PKCE S256, RFC 8252-compliant flows, fail-closed semantics. The team navigated real undocumented AWS service bugs (AgentCore Identity hanging, ContextVar threading, SDK package naming) and landed on a clean direct-OAuth design. Strong test coverage throughout.

Verdict: Approve with conditions (one blocking concern).

---

🔴 Blocking: Token Refresh Race Condition

Linear rotates the refresh token on every use (one-shot grants). Three independent refresh paths (Lambda TS, agent Python, concurrent warm containers) have no coordination mechanism:

1. Lambda A reads expired token → refreshes with refresh_token_v1 → succeeds → writes refresh_token_v2
2. Lambda B reads same expired token → refreshes with refresh_token_v1 → invalid_grant (token already rotated)

The code references "Linear's idempotency-on-replay grace window (30 min documented)" but no public citation exists. If this grace window doesn't hold, a single concurrent invocation permanently bricks the workspace's token chain.

Fix (10 lines in each resolver): After a refresh failure with invalid_grant, re-read the secret from Secrets Manager (another process may have already refreshed) and retry once before returning null.

---

🟡 Non-Blocking Concerns

#	Issue	Recommendation
1	No migration path from PAK — LinearApiTokenSecret is deleted with no rollback mechanism or runbook	Add drain-queue guidance; or keep the old resource for one release cycle
2	Vestigial AgentCore Identity code — bedrock-agentcore==1.9.1 dep + thread-propagation in server.py not used by the final Secrets Manager path	Strip if unused, or document it's kept for 2.0c resume
3	Type duplication — StoredLinearOauthToken (CLI) and StoredOauthToken (Lambda) define the same schema independently	Add a contract test asserting key-set equality, or extract shared type
4	Secret sprawl — CLI-created secrets have no cleanup command	Track as tech debt; consider bgagent linear remove-workspace
5	No positive-path refresh logging — operators diagnosing intermittent 401s have no breadcrumbs	Add INFO log on successful refresh with workspace_id + new expiry
6	CallbackResult type allows impossible states — all-nullable struct instead of discriminated union	Refactor to { kind: 'direct-oauth'; code: string; state: string }
7	getOauthSecret validates only 3/11 fields — client_id/client_secret missing discovered only at refresh time	Validate all required fields at deserialization boundary
8	22-commit narrative — iterative discovery diary makes git bisect harder	Squash-merge to 4-5 logical commits

---

🟢 Strengths

• Per-workspace token isolation with clean DDB registry indirection
• Client credentials co-located in secret JSON — Lambdas refresh without extra env vars
• Fail-closed everywhere — missing tokens cause MCP auth rejection, never silent degradation
• RFC 8252 §7.3 HTTP localhost callback (no self-signed cert warnings)
• esbuild import.meta.url shim is exactly right with upstream issue references
• 265-line test suite for the OAuth resolver covering happy path, expiry, refresh failure, and cache invalidation

---

Reviewed with automated architecture, security, and type-design analysis.

[krokoko]

Follow-up: Silent Failure Analysis

Additional findings from error-handling deep-dive (supplements the architectural review above):

---

🔴 Critical: Unguarded import in background thread (agent/src/server.py)

if workload_access_token:
    from bedrock_agentcore.runtime.context import BedrockAgentCoreContext
    BedrockAgentCoreContext.set_workload_access_token(workload_access_token)

This bare from ... import inside _run_task_background() has no try/except. If bedrock-agentcore is missing or the module structure changes, it crashes the entire task pipeline thread — _background_pipeline_failed gets set, /ping returns 503, but there's no clear log message explaining why.

Compare to the defensive pattern used 50 lines away in config.py:

except ImportError as e:
    log("WARN", f"resolve_linear_api_token: boto3 unavailable ({e}); skipping")

Fix: Wrap in try/except (ImportError, AttributeError) with a WARN log. The agent can proceed — resolve_linear_api_token falls back to direct Secrets Manager read anyway.

---

🟡 High: Cache not invalidated on fetch-level refresh failures

In linear-oauth-resolver.ts, invalidateLinearOauthCache() is only called inside the !resp.ok branch (line ~1491). If fetchImpl(LINEAR_TOKEN_ENDPOINT, ...) itself throws (network timeout, DNS failure), the catch block at line ~1469 returns null without invalidating the cache.

Result: the stale expiring token remains cached for 60s. Every subsequent call re-reads it, re-tries refresh, fails again — hammering Linear's token endpoint in a tight loop until the cache TTL expires.

Fix: Call invalidateLinearOauthCache(current.workspace_id, secretArn) in the fetch-level catch block as well.

---

🟡 High: isWebhookSecretConfigured bare catch swallows IAM errors

} catch {
    return false;
}

AccessDeniedException, DecryptionFailureException, and other actionable errors are all swallowed → returns false → setup re-prompts for webhook secret. User thinks they need a new webhook secret when the real problem is IAM permissions.

Fix: Catch specific expected errors (ResourceNotFoundException) as false; log and surface others.

---

🟡 Medium: admin invite-user half-creates user on password-set failure

AdminCreateUser succeeds → AdminSetUserPasswordCommand fails (password policy stricter than generator, partial IAM) → user is stuck in FORCE_CHANGE_PASSWORD with no cleanup or guidance.

Fix: Wrap in try/catch with a CliError that tells the admin the user exists in a broken state and suggests manual cleanup.

---

🟡 Medium: _is_expiring in Python returns True on malformed expires_at with no log

except ValueError:
    return True

If the stored expires_at is consistently malformed (e.g., Lambda wrote bad data during a previous refresh), this triggers a refresh on every single task invocation with zero diagnostic trace.

Fix: Add log("WARN", f"_is_expiring: malformed expires_at '{expires_at_iso}'") before returning True.

[isadeks]

Review response — 12/13 items addressed

Thanks for the careful review. Address-by-address:

🔴 Blockers / Critical

#	Item	Commit	Notes
1	Refresh-token race (BLOCKER)	acb75cf	On invalid_grant from Linear, re-read the secret from Secrets Manager (bypassing cache); if the refresh_token rotated, use the new one. Symmetric implementation in both the TS resolver (linear-oauth-resolver.ts) and Python resolver (config.py). Caps at one re-read + one retry — no infinite loops.
2	Unguarded bedrock_agentcore import in server.py (CRITICAL)	acb75cf	Wrapped in try/except (ImportError, AttributeError) with a _warn_cw log. The Linear token resolver has its own Secrets Manager fallback path, so a missing SDK no longer kills the entire pipeline thread.

🟡 Silent-failure follow-up review

#	Item	Commit
-	Cache invalidation on fetch-level refresh failure	acb75cf
-	isWebhookSecretConfigured bare-catch swallowing IAM errors	e90a092
-	admin invite-user half-creates on password failure	e90a092
-	_is_expiring malformed-expires_at log	acb75cf

🟡 Architectural non-blocking

#	Item	Commit
1	PAK migration path	e90a092 — expanded the "Migration from 2.0a (PAK) to 2.0b (OAuth)" section in LINEAR_SETUP_GUIDE.md with a pre-deploy checklist, what-survives-vs-not, and an explicit "fix-forward" rollback note (the original PAK secret is gone with the CFN resource).
2	Vestigial AgentCore Identity code	e90a092 — bedrock-agentcore==1.9.1 is kept with an inline comment explaining why (the workload-token bridge in server.py still imports it; bridge is now wrapped in try/except per item 2). The seam stays clean for an eventual 2.0c resume.
3	Type duplication / contract drift	8e1d578 — added cdk/test/contracts/stored-oauth-token-parity.test.ts that regex-parses both StoredOauthToken (Lambda) and StoredLinearOauthToken (CLI) interface declarations and asserts equal field sets. Also asserts the new STORED_OAUTH_TOKEN_REQUIRED_FIELDS const matches the interface so the validator can't drift from the type.
4	Secret cleanup command	Deferred per your "Track as tech debt." Tracked as an internal followup.
5	Positive-path refresh logging	acb75cf — linear_oauth_refresh_ok INFO log on each successful refresh, with workspace_id, workspace_slug, and new_expires_at.
6	CallbackResult discriminated union	8e1d578 — split into | { kind: 'agentcore'; sessionId } | { kind: 'direct-oauth'; code; state }. Updated the resolver site, the consumer in bgagent linear setup, and the test file to use exhaustive narrowing. The setup wizard now errors clearly if it gets the AgentCore shape (parked path) instead of silently passing nulls.
7	Validate all 11 fields in getOauthSecret	8e1d578 — extracted STORED_OAUTH_TOKEN_REQUIRED_FIELDS next to the interface and validates the full set at deserialization. Bad secrets fail fast at fetch with a structured log naming the missing fields.
8	22-commit narrative	Will squash to 4-5 logical commits at merge time. Keeping the granular commits for now so review-trail-by-commit remains useful through approval.

Verification

• Unit tests pass: CDK 13/13 (resolver + new contract), CLI 286/286, agent 15/15
• mise //cdk:test clean
• ESLint + ruff + tsc clean
• Deployed to backgroundagent-dev and re-ran the full Linear-driven flow end-to-end: Linear issue ABCA-78 → agent → PR chore(deps): bump astro from 6.1.6 to 6.1.10 #86 in isadeks/abca-linear-demo. No regressions in the existing happy path; the resolver refactor is transparent on cached-fresh-token reads.

Open to follow-up review on any of the above.