feat(linear): mint OAuth tokens via AgentCore Identity Token Vault, and latch revoked credentials (#809, #812)

[isadeks]

Summary

Closes #809
Closes #812

Mints Linear OAuth tokens through the AgentCore Identity Token Vault instead of storing a long-lived credential, and detects/latches/announces a revoked channel credential instead of surfacing it as a failed user task.

Both issues ship together because the code is interdependent: #812's typed VaultTokenResult is what the vault resolver returns, and #809's recorded vault subject flows through #812's revocation recorder. Splitting them would mean reordering commits that touch the same files for a boundary a reviewer does not benefit from.

Flag-gated on --context enableLinearIdentityVault=true (default off). With the flag off the synthesized template contains none of it — asserted by test.

What changes for an operator

bgagent linear setup <slug> is now the only Linear onboarding command, and it needs one consent.

bgagent linear setup <slug>

• Uses the vault when the deployment has one, Secrets Manager otherwise, announced in one line rather than requiring a flag to be known up front
• Consent runs through a hosted page openable in any browser, so onboarding works from a cloud desktop, SSH session or container — the previous localhost loopback could not
• vault-setup, --hosted and --localhost are gone; there is nothing left to choose
• app-template resolves the URLs it used to demand: the consent page from a stack output, the vault callback from the provider, and the webhook endpoint from the configured API URL

The design decision worth reviewing

The vault subject is derived from the workspace slug, not its organization UUID, and recorded on the registry row as vault_user_id.

The UUID is only knowable after a token exists. Deriving the subject from it therefore forces two consents — one to learn the organization, a second to bind the vault. A slug-derived subject is known before consent, which is what makes single-consent onboarding possible. Both resolvers prefer the recorded value and fall back to the derived form for workspaces onboarded before it was stored, so existing vault installs keep minting under the subject their grant is actually under.

Two deliberate divergences from ADR-016 P1, both noted in the ADR:

1. Per-workspace credential provider (bgagent-linear-oauth-<slug>) rather than one shared provider with per-workspace subjects. Keeps each workspace's grant and rotation isolated. Cost: each workspace adds one redirect URI to the Linear app.
2. The agent self-mints via boto3 on both substrates rather than receiving an injected workload token.

Known limitations, stated plainly

• ECS substrate is live-unverified. The env and task-role grant are wired and unit-tested, but no ECS task definition existed on the test deployment. The agent's mint path is substrate-uniform by design.
• +9 CloudFormation resources with the flag on — 450 → 459 of the hard 500 limit, re-measured on this branch after fix(cdk): reclaim resource and byte headroom under both CFN ceilings (#852) #854 reclaimed 30. The consent page is in a nested stack so it costs the root one resource.
• The vault and the MicroVM substrate are still refused together, and that guard is now obsolete. It was written when the pair synthesized 505 against a hard 500. After fix(cdk): reclaim resource and byte headroom under both CFN ceilings (#852) #854 the same pair measures 476 (483 with the tool gateway), so the combination fits and the fail-fast in cdk/src/stacks/agent.ts now refuses a deployable configuration while quoting a figure that is no longer true. The vault was always wired for MicroVM — platform config carries the workload name, the guest execution role holds the mint grant. Note the covering test asserts the throw, so it pins the guard rather than detecting the headroom; it did not and could not go red when the room appeared. Removing the guard, and adding enableLinearIdentityVault to the resource-budget matrix fix(cdk): reclaim resource and byte headroom under both CFN ceilings (#852) #854 introduced so the combination is genuinely guarded rather than merely permitted, is the follow-up.
• The consent page URL is a generated CloudFront domain — harmless on the vault path. There it is the resourceOauth2ReturnUrl, registered on the AgentCore workload identity's allowlist, which the provisioning custom resource rewrites on every deploy; the CLI reads the current value from the LinearVaultConsentUrl output rather than caching one. So if the domain changes, nothing breaks. What Linear holds on that path is AgentCore's own callback (…/identities/oauth2/callback/<uuid>), which survives re-runs because a duplicate provider name updates in place instead of minting a new id. The instability matters only on the Secrets-Manager (non-vault) leg, where the page really is Linear's redirect_uri and a changed domain means adding the new URI to the app — both variants of that leg print the warning, and Linear accepts several URIs so the stale entry is harmless. A stable custom domain, or serving the page from the existing API Gateway, would retire even that.
• Secrets Manager is retained on every path. Retiring it is ADR-016 P2.

Live verification

End-to-end on a real Linear workspace, with the agent-authored PR as the artifact.

Scope caveat, stated because it changes what this proves: the workspace was a FRESH install, onboarded by the merged setup command in one pass. So the vault mint, the consent flow and the revocation latch are verified — but the path a workspace takes when its Secrets Manager bundle holds no grant and was not written by this flow is covered by unit tests only. Re-verification on a genuinely fresh vault workspace is what takes this out of draft.

• One consent, no localhost, from a cloud desktop
• Vault-minted token verified against Linear as an actor=app agent token (…@oauthapp.linear.app), not a user token
• A labelled issue produced: 👀 reaction, Backlog → In Progress, task created, PR-opened comment, preview screenshot — with the Linear token resolved via the vault on every hop and no Secrets Manager fallback
• Revocation path proven in isolation: row latched status=revoked, one real SNS message, and zero duplicates on repeat events

Defects the live run caught that unit tests could not

Recorded because each is a class, not a one-off:

1. DynamoDB Limit: 1 on a filtered Scan reported an onboarded workspace as absent — Limit caps items read before the filter.
2. AgentCore returns ValidationException, not ConflictException, for a duplicate name. Broke re-runs, and would have broken the next deploy when the custom resource re-issued Create.
3. IAM: GetWorkloadAccessTokenForUserId authorizes against the workload-identity directory, not the named identity; GetResourceOauth2Token against the token vault. Unit tests asserted the ARNs I had chosen, so only a live call could surface it.
4. customParameters are part of the vault's cache key. Resolving without the consent-time set reports "needs consent" against a live grant, silently degrading every caller to Secrets Manager.
5. Linear rejects a dead refresh token with invalid_request, not the RFC's invalid_grant, with the detail in error_description. The revocation branch had never executed.
6. A row parser that copies fields explicitly dropped the vault subject. Every caller threaded it correctly; the parser discarded it. The test seam took three unpacked arguments and could not express the field, so no test at that seam could catch it — the seam now takes the full request.
7. The vault grant was wired to one Lambda. Five others resolve Linear tokens; each fell back to a Secrets Manager token a vault workspace does not maintain, so a task succeeded while its issue showed nothing after the opening comment. The first guard for this pinned a hardcoded count of 4, derived by grepping handlers that import a Linear module directly — which omitted the two that reach a minting resolver two hops away, and whose count-based assertion passed for the very mistake it named. Replaced by a pinned set plus a check computed from the handler source graph.
8. Not from the live run — from rendering the operator alert instead of reading it. The revocation email printed the same bgagent linear setup command twice on the vault path, and a .filter(line => line !== '') intended for one conditional entry stripped every blank-line separator, collapsing the message into a single block. Both were green under toContain, which cannot see a duplicate or a missing paragraph break; the assertions now pin the command count and the separators, and were mutation-checked against the old rendering.

Test plan

• mise run build green — CDK 4411 / 210 suites, CLI 867 / 59 suites, agent 1762, docs build + link check, synth
• security:secrets, security:deps, security:retire, security:sast clean
• Flag-off synth asserted to contain no workload identity, no token grant, no agent env, no consent page
• Every new guard mutation-tested — the fallback classifier, the recorded-subject preference on both resolvers, the fail-closed token preservation, the inherited-secret detection, the announce/latch ordering, the revocation self-heal, the vault-aware health check, and the registry-write and directory-ARN grants each fail when the fix is reverted
• Live-verified end to end on a real workspace, both the vault mint and the revocation latch — see the scope caveat above
• The narrowed IAM grants are now live-verified, and the first attempt was wrong. UpdateWorkloadIdentity authorizes against the bare workload-identity-directory/default, so it cannot be scoped to one identity; these actions do a multi-resource check and IAM names only the first resource it fails, so three successive deploys each surfaced a different AccessDenied. Final grant is all four lifecycle actions on …-directory/default plus …-directory/default/workload-identity/* — still narrower than the original account-wide workload-identity-directory/*, but the consequence is recorded in the construct: IAM cannot fence off the cross-stack allowlist clobber; the stack-derived NAME is what does. Verified by invoking the provisioning handler with an idempotent Update (previously AccessDenied, now clean), then confirming the return URLs were unchanged and the vault still mints a working actor=app token.
• Deployable again. Deploying to a dev account had been failing before it reached any of this, for a reason unrelated to this PR: the branch carried PINNED_LOG_DELIVERY_BY_STACK while both dev accounts had moved to library-named log-delivery resources. A DeliverySource is unique per (resource ARN, log type) account-wide and the runtime ARN does not change, so a pinned id creates a second source for the same runtime and CloudFormation rolls the whole stack back with AlreadyExists. That is fix(agent): log-delivery pin table is keyed by stack name but holds per-account state, so it breaks deploys it was meant to protect #703/fix(agent): stop hard-coding log-delivery logical ids #705; dev was deployed from an integration branch carrying its pin removal, and cdk diff confirmed zero log-delivery churn before deploying.

Two pre-existing gate failures are unrelated to this branch: security:sast:masking reports on 49 files including lines that already carry justified nosemgrep comments (findings in files touched here are annotated), and security:gh-actions reports one medium in .github/workflows/auto-approve.yml, untouched here.

[isadeks]

Cleanup at 706e91f4

Thanks — thorough pass, and the verification style (checking the un-latch, the announcement-claim removal, and the CodeQL findings yourself rather than taking the dismissed reviews' word) is the reason two of these were worth acting on rather than filing.

Fixed

Nit 1 — dangling {@link}. Confirmed: suppressPublisherKmsWildcard appears nowhere in the repo, and that docstring was its only occurrence. Now names OVERFLOW_SUPPRESSIONS in cdk/src/stacks/agent.ts. Agreed this is worse than an ordinary stale comment, since it is the pointer for something that breaks synth when missed.

Nit 3 — vaultMetadata did not cover its own builder. The best of the seven, and exactly right: the guard keys off the object-literal form, the label path assigns onto an existing object, so the one path the helper was written for was the one path its guard could never see. It now calls the helper, covered by a behavioural assertion on the channel_metadata that reaches createTaskCore.

Demonstrated rather than claimed — dropping the vault fields from that path now fails the behavioural test while the structural guard still passes:

✕ LABEL-trigger path actually emits the vault fields (behavioural)
✓ each builder writing the workspace slug spreads vaultMetadata

Both checks are kept, because they fail for different reasons: the structural one catches a builder nobody exercised, the behavioural one catches a builder the pattern cannot describe. A second test pins the absent-provider case, so spreading unconditionally — which would hand the agent a provider name for an SM-only workspace — fails too. linear_workspace_id keeps its existing gating; widening that field's conditions would be a behaviour change you did not ask for.

Nit 6 — duplicated licence block removed.

Nit 2 — taken in the optional form, not the throwing one. You offered either; optional is the right half. AgentCore models sessionUri as optional on the token response, so an absent value is the service's contract rather than a fault, and throwing would refuse a consent the interactive flow still completes from the pasted id. Now readonly sessionUri?: string with the ?? '' gone, which puts the absence in the type where a non-interactive caller has to handle it instead of behind an empty string that reads as a session.

Deferred, with reasons

Nit 4 — agent-side token caching. Real, and I agree it is not a regression (the SM path behaves identically). Deferring because the fix is a design, not a tweak: caching with an expiry still guesses the vault's TTL, so the honest shape is remint-on-401 in linear_reactions.py, which needs its own change and its own test. Worth an issue rather than a hurried cache.

Nit 5 — state-absent path. Agreed it costs nothing to fail closed, but a one-off guard on pending.state treats one field of a record whose other fields are equally unvalidated. If the persisted consent record deserves validation it deserves it as a record, which is a bigger change than this PR should carry.

Nit 7 — test file name. Declining: linear-revocation-recorder.test.ts names the behaviour under test rather than a module it imports, which I would rather keep than rename for filename symmetry.

The adjacent finding is the one I would act on next

cli/src/linear-oauth.ts:279-283 stringifying the raw /oauth/token body into a CliError — you are right that it is untouched here (git diff origin/main...HEAD confirms), and right that it matters: a 200 carrying access_token but missing expires_in or scope prints an access token to the operator's terminal and into any CI log capturing it. Happy to file that as a P1 follow-up with a redaction fix if you would like it tracked rather than left in the review thread.

CI: local mise run build green at this HEAD (CDK 4448 / CLI 895 / agent 1782), and security:deps is now genuinely clean — #844 merged and this branch picked it up, so the repo-wide browserslist advisory that had every PR red is gone here too.

[isadeks]

The adjacent finding is fixed here after all — 2ced095e

I initially said this belonged in a follow-up, quoting your own framing that the function is untouched by this PR. That is true of the function and not of the file: this PR already adds 88 lines to cli/src/linear-oauth.ts, so the scope argument I was leaning on does not hold, and a credential-disclosure path in the Linear OAuth exchange is the same concern the PR exists to improve. Fixed in its own commit.

Your read of the mechanism was exact, and the dangerous case is subtler than "missing token": isLinearTokenResponse requires expires_in and scope, so a 200 carrying a perfectly good access_token alongside — say — a string expires_in failed the guard and printed the credential. Both the access and refresh tokens sat inside the 200-character window.

The message now names the offending field and its expected type, plus the key NAMES present, and never a value:

Linear /oauth/token returned an unexpected shape for the code exchange:
expires_in expected number, got string
(keys present: access_token, expires_in, refresh_token, scope,
token_type)

That is more useful than a truncated blob for diagnosing a contract change, so the redaction costs no debuggability. The regression test asserts the negative directly — neither token value may appear — and is mutation-proven: restoring the raw-body interpolation fails it.

One thing I did not fix here, deliberately. cli/src/jira-oauth.ts:327 carries the identical pattern, so this is a class of two rather than a one-off. That file is not in this PR's diff, and pulling an unrelated surface into a large vault change is exactly the scope creep the reasoning above does not license. It wants the same treatment in its own change — happy to open that as a small PR with an issue, the way #844/#845 went, if you would rather have it tracked than left in this thread.

mise run build green at this HEAD (CDK 4448 / CLI 896 / agent 1782).

[scottschreckengaust]

1. Verdict — Request changes (one blocker)

Re-review at 2ced095e against my prior approve-with-nits at 4db95835 (dismissed by the two new pushes). The delta itself is clean and strictly improving — describeTokenShape closes a real credential-in-stdout leak, sessionUri becoming optional matches the AgentCore contract, and the label-trigger path now routes through the shared vaultMetadata helper. My earlier pass, however, did not check the new vault SDK import against this repo's own Lambda-bundling rule, and that check fails: @aws-sdk/client-bedrock-agentcore is now statically imported into the shared Linear resolver chain, which is bundled into at least six Lambdas that externalize @aws-sdk/* — the exact configuration task-api.ts:565-572, task-orchestrator.ts:390-395 and linear-identity-vault.ts:98 all document as broken for this client. That is the blocker; everything else is non-blocking.

The engineering quality is otherwise unusually high: the tri-state VaultTokenResult, the announce-before-latch with an independent revocation_announced_at claim, the installed_at-conditioned latch, and the UpdateItem-only registry grant are all correct and well argued in place.

2. Prior blockers / open threads

Claim	Status	Evidence at 2ced095e
B1 — transient Secrets Manager error latches a healthy workspace revoked	fixed (re-verified, not trusted)	cdk/src/handlers/shared/linear-oauth-resolver.ts — getOauthSecretForResolve returns null only on definite absence and rethrows otherwise; recordRevocation fires only inside the if (!fetched) branch.
B2 — account-wide bedrock-agentcore-identity!* client-secret read	fixed	cdk/src/constructs/linear-identity-vault.ts:320-332 scopes to bedrock-agentcore-identity!default/oauth2/bgagent-linear-oauth-*.
ayushtr nit — VaultConsentStep empty-string sentinels	fixed	cli/src/linear-vault.ts is now a kind-tagged union; the delta additionally made sessionUri optional.
my nit — dangling {@link suppressPublisherKmsWildcard}	fixed in 706e91f4	repointed at OVERFLOW_SUPPRESSIONS.
my nit — vaultMetadata structural guard could not see the label-trigger builder	fixed in 706e91f4	behavioural test added; all five builders now spread the helper.
CodeQL #41 (URL substring) / #42 (fingerprintToken)	still open, both benign	#41 is a toContain on a fixture host in cli/test/commands/linear.test.ts; #42 is a 12-hex log-correlation tag over a high-entropy token, never compared to a stored secret. Please dismiss them with a reason so the next reviewer can tell them from a regression, and replace the real-looking d2ud1woydykuxp.cloudfront.net fixture with a doc-safe placeholder.

No prior blocking claim regressed. One claim raised by a lens this round I checked and dropped: add-workspace cannot silently erase a vault binding — the ScanCommand dup-check at cli/src/commands/linear.ts:1685-1697 throws for an already-registered workspace before the PutCommand.

3. Vision alignment ✓

Advances fire-and-forget and bounded blast radius (VISION.md) and RFC #249 / ADR-016 Phase 1: a long-lived Linear credential in Secrets Manager is replaced by short-lived vault-minted tokens (#809), and a dead grant becomes an attributable, deduplicated, out-of-band operator alert instead of a silently failing user task (#812). Default-off behind enableLinearIdentityVault. Governance ✓ — #809 and #812 both carry approved and are self-assigned; branch feat/809-linear-identity-vault-pr is conventional.

4. Blocking issues

B1. @aws-sdk/client-bedrock-agentcore is externalized on every Lambda that reaches the new vault code — cdk/src/constructs/linear-integration.ts:225 (externalModules: ['@aws-sdk/*'], inherited by attachmentScreeningBundling, which is what the webhook processor uses at :328).

The import chain is fully static, no dynamic import() anywhere in it:

linear-webhook-processor.ts / fanout-task-events.ts / orchestration-reconciler.ts /
iteration-heartbeat-sweep.ts / github-webhook-processor.ts
  → orchestration-channel-factory.ts:59  (static)
  → orchestration-channel-linear.ts:50   (static)
  → linear-oauth-resolver.ts:35          (static)
  → linear-vault-token.ts:39-43  import { BedrockAgentCoreClient, ... } from '@aws-sdk/client-bedrock-agentcore'

Every one of those five entry points is bundled with externalModules: ['@aws-sdk/*'] — linear-integration.ts:225, fanout-consumer.ts:200, orchestration-reconciler.ts:98, iteration-heartbeat.ts:83, github-screenshot-integration.ts:197 (plus jira-integration.ts:216 / slack-integration.ts:157, whose handlers reach the same factory). This repo has hit this exact wall twice and wrote it down both times:

@aws-sdk/client-bedrock-agentcore in particular has shipped new commands … that are not in the runtime's bundled SDK, so externalizing it causes Lambdas to throw <Command> is not a constructor at runtime — a silent failure mode — task-api.ts:565-572

The bedrock-agentcore-control SDK is not in the Lambda runtime, so it must be bundled (the repo default externalizes @aws-sdk/*, which we override). — linear-identity-vault.ts:97-98

Two possible runtime outcomes, both bad, and neither reachable by any test in this PR (jest disables bundling; CI never deploys):

• Client/commands present but stale → makeClient(BedrockAgentCoreClient, …) at linear-vault-token.ts:147 sits outside the function's try, so a missing constructor throws past the careful unavailable classification, out of resolveLinearTokenViaVault, and out of resolveLinearOauthToken (Step 1b at linear-oauth-resolver.ts:421-428 has no try). Token resolution for vault-onboarded workspaces fails hard, not gracefully.
• Package absent from the runtime image → the top-level require fails at module load, so the Linear webhook processor (and fan-out consumer, reconciler, heartbeat, screenshot processor) fail at cold start, for all workspaces, whether or not enableLinearIdentityVault was passed. That is a total Linear-integration outage introduced by a flag-gated feature.

Fix: replace externalModules: ['@aws-sdk/*'] with the enumerated stable-client list already used by task-api.ts:574-582 / task-orchestrator.ts:405-414 on every construct whose entry reaches orchestration-channel-factory.ts, so client-bedrock-agentcore is bundled. Add a synth-time or unit assertion that no function whose entry transitively imports linear-vault-token.ts lists @aws-sdk/* as external — this failure mode is invisible to the current suite, and the SM fallback would mask the degraded variant of it in production.

5. Non-blocking suggestions

MAJOR — setup strips the vault binding whenever it does not take the vault branch. cli/src/commands/linear.ts:928: useVault = Boolean(consentPageUrl) && !resumed. alreadyOnVault is computed at :927 and correctly used at :954 to refuse a substrate move on a provider-upsert error, but not here. If the stack output is absent (e.g. the last cdk deploy omitted --context enableLinearIdentityVault=true, which must be repeated every time) or the run is a --code resume, a vault-managed workspace takes the direct-OAuth path and the full-item PutCommand at :1305 writes the row without provider_name / vault_user_id, silently demoting it back to a long-lived Secrets-Manager credential behind a green "Setup complete", orphaning the vault provider. The printed reason ("AgentCore Identity not available in <region>") is also a fabricated diagnosis — nothing probed regional availability, only a missing stack output. Carry the prior fields forward, or fail fast when alreadyOnVault && !consentPageUrl.

MAJOR — the CLI minter collapses "vault unreachable" into "grant revoked". cli/src/linear-vault.ts:264 returns null from a bare catch, and cli/src/linear-auth-health.ts:342 adds .catch(() => null), so AccessDenied / throttling / no AgentCore in region all land on state: 'revoked' at :347 with the remedy bgagent linear setup <slug> — which per this PR's own comment at commands/linear.ts:975-985 can replace the Linear installation and invalidate a working grant. platform-doctor.ts renders that as a hard fail. The module 30 lines above already does this right (state: 'unknown' when it has no way to query the vault), and cli/test/linear-auth-health.test.ts:278 asserts exactly that principle. Mirror the cdk-side VaultTokenResult union in the CLI (isVaultUnavailableError is already in that file) and map unavailable → unknown. The nosemgrep rationale at linear-vault.ts:265 ("distinguishing 'no grant' from 'API error' would not change the caller's action") is falsified by the caller this PR adds — please update or drop it.

MAJOR — the un-latch is not env-gated the way the latch is. cdk/src/handlers/shared/linear-oauth-resolver.ts:444: defaultRevocationRecorder is gated on LINEAR_REVOCATION_RECORDING precisely because a read-only role would fail AccessDenied and swallow it, but clearWorkspaceRevocation is called unconditionally. Only the webhook processor holds dynamodb:UpdateItem; the fan-out consumer, orchestrator, reconciler, heartbeat and screenshot processor get grantReadData + grantMintToken. When one of them re-probes a vault_consent_required row and the vault answers with a token, the clear is denied and logger.error fires on every event, while platform doctor keeps reporting a workspace the resolver is successfully serving. Gate it on the same env flag, or special-case AccessDeniedException to warn.

MAJOR — the vault cache-key constants are quadruplicated with only "keep in sync" comments. cdk/src/handlers/shared/linear-vault-token.ts:48/64/76, agent/src/config.py:71/75/83, cli/src/linear-oauth.ts:34, cli/src/linear-vault.ts:50/255, cdk/src/stacks/agent.ts:994. contracts/constants.json exists for exactly this (approval_gate_cap triplication) and has no linear_vault block; there is no parity test. The code names the consequence itself — a one-token divergence makes every resolve a cache miss, which post-#812 is reported as consent-required and can latch a healthy row revoked. Add a linear_vault block plus a parity test alongside cli/test/constants-parity.test.ts.

MAJOR (tests) — one test claims coverage its assertion does not provide. cdk/test/handlers/shared/linear-revocation-recorder.test.ts:211, "the recorded reason distinguishes Linear refusing from a vault inference", asserts only ConditionExpression).toContain('installed_at = :installed') — nothing about the reason. No test drives the default recorder with source: 'vault-consent-required', so the reason ternary at linear-oauth-resolver.ts:301-303 is unverified: collapse it to always write refresh_token_rejected and the suite stays green, while the self-heal re-probe (:397-398) stops matching and a wrongly-latched workspace becomes permanently offline — the exact property #812 adds. Capture ExpressionAttributeValues in the harness and assert :reason on both paths.

MAJOR (docs) — vault + compute_type=lambda-microvm is unsupported and undocumented. docs/guides/LINEAR_SETUP_GUIDE.md:24 recommends --context enableLinearIdentityVault=true with no note that the combination hard-fails synth at 505 resources; the only record is a test title (cdk/test/stacks/agent.test.ts:1721) and the PR body. ADR-016 is silent too, and agent.ts still attaches grantMintToken + LINEAR_VAULT_ENABLED to lambdaMicrovm.executionRole, which reads as supported. Add the incompatibility to the guide (+ mise //docs:sync) and to ADR-016's scope table, and ideally fail fast in AgentStack naming both flags instead of letting the resource counter throw.

Nits

• cdk/src/handlers/shared/linear-oauth-resolver.ts:398 — the re-probe guard uses the raw literal 'vault_consent_required' instead of VAULT_CONSENT_REVOCATION_REASON, defeating the constant whose docblock says the guard is one of the three places that must agree. Related: type the reason (type LinearRevocationReason = 'refresh_token_rejected' | 'vault_consent_required') instead of reason: string at :620, and mirror it into the CLI row type — cli/src/linear-auth-health.ts:283 re-declares the literal independently.
• cdk/src/stacks/agent.ts:633 — "Workload name matches the LinearIdentityVault construct's fixed abca_linear_oauth" is stale: linearVaultWorkloadName(this) (agent.ts:122-130) returns abca_linear_oauth_<StackName> and is passed into the construct, which has no fixed default. Renaming the identity orphans every consent, so this is the worst comment in the file to be wrong.
• cdk/src/handlers/shared/linear-revocation-alert.ts:29 — the file header still says "The registry latch is the dedup key: only the caller whose conditional write actually flipped active → revoked announces", which announceRevocation's own docstring 40 lines below explicitly contradicts (the dedup key is the independent revocation_announced_at claim, and the resolver announces before latching). Acting on the header reintroduces the bug the claim was added to fix. Same class: markWorkspaceRevoked's "that boolean is the dedup key for notification" — no production caller consumes the return value.
• cdk/src/constructs/linear-vault-consent-page.ts:32 and cli/src/commands/linear.ts:869 reference a bgagent linear setup --hosted flag that does not exist (the options are --code, --hosted-consent-url, …; the hosted page is selected automatically whenever the stack publishes LinearVaultConsentUrl, and the guide says "There is no flag").
• cli/src/linear-auth-health.ts:38 — "Read-only: this never consumes a refresh token" is now false for the whole module: :415 calls verifyLinearRefreshAndPersist and :498 issues PutSecretValueCommand. Qualify it as "read-only by default; verifyRefresh opts into a real rotation".
• cdk/src/handlers/shared/linear-oauth-resolver.ts:308 — "the operator has still been told, because the announcement above already went out" is untrue on the no-topic path (:319-327 records without announcing), which is the total-silence case.
• cdk/src/handlers/linear-webhook-processor.ts:910 — points at "the source-level guard below vaultMetadata"; the guard lives in cdk/test/handlers/linear-webhook-processor.test.ts. Name the file.
• agent/src/config.py:135 — platform_client("bedrock-agentcore", …) sits outside the try that catches (ClientError, BotoCoreError), so UnknownServiceError / InvalidRegionError escape a function documented to return "" on any failure and abort the task before the 👀 acknowledgement. Move it inside.
• cdk/src/handlers/shared/linear-oauth-resolver.ts parseOauthSecret in 'without-grant' mode returns StoredOauthToken | null while deliberately skipping validation of access_token/refresh_token/expires_at/scope; give it mode-keyed overloads so a future caller cannot get a grant from the grantless door. Also add readonly revoked_at?: string to RegistryRow, which markWorkspaceRevoked writes and the CLI reads back.
• cli/src/commands/linear.ts:100 — resolveLinearVaultWorkloadName's .catch(() => null) turns a DescribeStacks AccessDenied into the hardcoded abca_linear_oauth, which no stack uses now that the name is stack-scoped; the resulting error tells the operator to redeploy a correct stack.
• Comment proportionality: several new docblocks narrate this PR's own review history ("Third occurrence of the same mistake in this change", "LIVE-CORRECTED", ~30 lines in linear-integration.ts:294-324). The invariants are worth keeping; the archaeology belongs in the PR description. Relatedly, the source-line-adjacency assertion in linear-webhook-processor.test.ts:1092 fails on a harmless key reorder — the new behavioural test is the better guard.
• cli/test/commands/linear.test.ts:294 — replace the real deployment hostname d2ud1woydykuxp.cloudfront.net with https://d111111abcdef8.cloudfront.net/; that also removes what CodeQL #41 latched onto.

6. Documentation

docs/guides/LINEAR_SETUP_GUIDE.md and ADR-016 are substantively updated (ADR-016 explicitly supersedes its own earlier bedrock-agentcore-identity!* guidance — a documented trade, not a silent one), and the deleted LINEAR_PAK_MIGRATION_RUNBOOK.md was removed together with its Starlight mirror, its astro.config.mjs entry and its sync-starlight.mjs entry — no dangling references. Starlight mirrors are in sync. Missing: the vault + lambda-microvm incompatibility (above), and linearVaultHostedReturnUrl — an undocumented context knob that changes the OAuth return-URL allowlist on the workload identity and skips the managed consent page. Document it next to the other two flags with the requirement that it must be an origin the deployer controls.

7. Tests & CI

All checks currently pass. Coverage is genuinely strong: per-branch failure-path tests for the resolver, vault-token, revocation-alert, pending-consent and identity-provisioning handlers, behaviour-level rather than implementation-level assertions, and the delta's new test ("an off-contract 200 must not echo the access token") is exactly the right shape. Gaps: the reason-mapping test above; the agent-side mint request pins customParameters but not scopes / oauth2Flow / resourceOauth2ReturnUrl, so _LINEAR_VAULT_SCOPES can drift silently (agent/tests/test_config.py:387); no test pins that an absent sessionUri yields undefined rather than ''; no command-level test drives makeLinearCommand() setup, so the pasted-state correlation checks (:895, :1112) and resolveLinearVaultWorkloadName are unexercised. cdk/test/stacks/agent.test.ts:1595+ re-synthesizes the full AgentStack per test against the beforeAll convention in cdk/AGENTS.md:33 (#366) with no exemption comment — two of those tests share one context and could share one Template.

Bootstrap synth coverage: PASS with a gap. No cdk/src/bootstrap/policies/*.ts changed, so no version bump is owed; the three new CFN types are added to resource-action-map.ts and the auto-named overflow AWS::IAM::ManagedPolicy is covered by the existing policy/backgroundagent-dev-* statement. Gap: in cdk/test/bootstrap/synth-coverage.test.ts:158 the nested-stack path only asserts RESOURCE_ACTION_MAP membership — run findMissingBootstrapActions over nestedTypes too, or a mapped-but-unpermitted nested type still fails a clean-account first deploy.

8. Review agents run

Folded in: code-review, silent-failure, tests, types, comments, security-governance-docs. None omitted or failed. One reported MAJOR (add-workspace erasing provider_name) was dropped as non-reproducible — the dup-check throws first; two lenses' overlapping CLI-minter findings were merged. Limitation: the suites were not executed locally (the review worktree has no node_modules/.venv and installing would pollute shared state), so every claim here is static reasoning; CI is green, which does not cover B1 — jest disables Lambda bundling and no deploy runs in CI.

9. Human heuristics

• Proportionality — concern. Several docblocks narrate this PR's review history rather than the code's contract (linear-webhook-processor.ts:638, linear-integration.ts:294-324, linear-oauth-resolver.ts:1117 "LIVE-CORRECTED"); once merged, "in this change" has no referent.
• Coherence — concern. The design iterated (stack-scoped identity, claim-based dedup, opt-in refresh, automatic hosted page) and the code plus nearest docstring were updated while four file-header/overview comments kept describing the earlier design (linear-revocation-alert.ts:29, agent.ts:633, linear-auth-health.ts:38, linear-vault-consent-page.ts:32). One sweep of module headers, not four point fixes. Also linear-oauth-resolver.ts:398 bypasses the constant introduced for it.
• Clarity — pass, with one edge. The tri-state unions, the installed_at-scoped conditions and the alerting rationale are exceptionally clear; resolveViaVault still carries a superseded docblock describing a string | null return (linear-oauth-resolver.ts:198-202).
• Appropriateness — concern. The blast radius of B1 is larger than the flag suggests: an unconditional top-level SDK import inside a shared handler module makes a default-off feature able to break Lambdas that never enable it. A bundling assertion, not a comment, is the right guard.

[scottschreckengaust]

B1 withdrawn — the hazard is pre-existing on main, not introduced here

I checked B1 mechanically instead of by static reading, and it does not hold. Bundling each entry with the same esbuild version and externalization CDK uses (0.28.1, external: ['@aws-sdk/*']), counting require("@aws-sdk/client-bedrock-agentcore") in the emitted CJS:

entry	main	2ced095e
fanout-task-events	1	2
github-webhook-processor	1	3
linear-webhook-processor	0	1
orchestration-reconciler	0	1
iteration-heartbeat-sweep	0	1

Reproduce from cdk/:

./node_modules/.bin/esbuild src/handlers/fanout-task-events.ts --bundle \
  --platform=node --target=node24 --format=cjs --external:'@aws-sdk/*' \
  --outfile=/tmp/out.js
grep -c 'require("@aws-sdk/client-bedrock-agentcore")' /tmp/out.js

Two of the five entrypoints already ship that require on main, under the identical externalModules: ['@aws-sdk/*'] — fanout-consumer.ts:200 via shared/memory.ts (CreateEventCommand, RetrieveMemoryRecordsCommand) and github-screenshot-integration.ts:197 via agentcore-browser.ts (StartBrowserSessionCommand). Not tree-shaken; the require reaches the emitted bundle.

That falsifies both runtime scenarios I described:

• "Package absent → cold-start failure → Linear outage regardless of the flag." If @aws-sdk/client-bedrock-agentcore were unresolvable in nodejs24.x, FanOutFn would be dead at module load on every invocation on main today and the memory path would never have worked. The runtime ships a pinned minor of the whole SDK v3 client set, not a curated subset (Building Lambda functions with Node.js — "a specific minor version of the AWS SDK for JavaScript v3, not the latest version").
• "Stale client → <Command> is not a constructor." This needs the pin to lack GetWorkloadAccessTokenForUserIdCommand / GetResourceOauth2TokenCommand. Those are AgentCore Identity data-plane ops from the same client and the same launch wave as the Memory ops main already calls successfully through the externalized client. StopRuntimeSessionCommand — the one we actually got burned by (task-api.ts:565-572) — is a later addition to that same package. One version: either the pin predates AgentCore entirely, and main is broken, or it carries both.

I was also wrong that the flag doesn't contain the blast radius. linear-oauth-resolver.ts:419 gates the call on row.provider_name && workloadName && isVaultEnabled(), so a flag-off deployment constructs nothing. Only the module-level require is flag-independent, and that is the part main already proves resolves.

The one correct observation inside B1 survives, and it's two lines. linear-vault-token.ts:147 builds the client outside the try, in a function whose docstring says "Never throws" — so any SDK-shaped surprise escapes the unavailable classification and out of Step 1b, which has no try of its own. Moving makeClient(BedrockAgentCoreClient, …) inside the try turns that whole failure class into { kind: 'unavailable' } → Secrets-Manager fallback, which is the designed behaviour. That's item 1 below.

The bundling change itself (enumerated externals on every construct reaching orchestration-channel-factory.ts, plus a synth-time assertion) is real hardening, but it now belongs in its own PR scoped to cover main's pre-existing fanout / github-screenshot cases too — fixing it only on the constructs this PR touches would leave the identical pattern deployed and unasserted. I'll file that.

What I'd like fixed here, and what I'm happy to see as a fast-follow

Everything else in my review stands as written, but most of it is fast-follow-able. My line was: defer anything whose cost is paid by the next maintainer reading code; don't defer anything whose cost is paid by a user or operator who can't see it.

Fix in this PR (seven items, five of them one-to-four-liners):

• 1. cdk/src/handlers/shared/linear-vault-token.ts:147 — move makeClient inside the try. Retires B1.
• 2. cli/src/commands/linear.ts:928 — useVault ignores the alreadyOnVault you compute one line above and use correctly at :954. A missing stack output or a --code resume takes the direct-OAuth path, and the full-item PutCommand at :1305 spreads provider_name / vault_user_id conditionally — so they're absent, demoting a vault-managed workspace back to a long-lived Secrets-Manager credential and orphaning the vault provider, behind a green "Setup complete". Carry the prior fields forward, or fail fast when alreadyOnVault && !consentPageUrl. The printed "AgentCore Identity not available in <region>" is also a fabricated diagnosis — nothing probed regional availability, only a missing stack output.
• 3. cli/src/linear-vault.ts:264 + cli/src/linear-auth-health.ts:342 — the bare catch { return null } plus .catch(() => null) maps AccessDenied / throttling / no-AgentCore-in-region onto state: 'revoked' at :347, which platform-doctor.ts renders as a hard fail with the remedy bgagent linear setup <slug> — which per this PR's own comment at commands/linear.ts:975-985 can replace the Linear installation and invalidate a working grant. A transient error must not produce destructive remediation advice. The module 30 lines above already does this right, and cli/test/linear-auth-health.test.ts:278 asserts exactly that principle. Mirror the cdk-side VaultTokenResult union (isVaultUnavailableError is already in the file) and map unavailable → unknown; update or drop the nosemgrep rationale at linear-vault.ts:265, which this PR's own caller falsifies.
• 4. cdk/src/stacks/agent.ts:633 — one line. The comment says the workload name is a fixed abca_linear_oauth; linearVaultWorkloadName(this) (agent.ts:122-130) returns abca_linear_oauth_<StackName>. A maintainer who trusts the comment and "simplifies" the name orphans every consent, irreversibly.
• 5. cdk/src/handlers/shared/linear-revocation-alert.ts:29 — one line. The file header states the inverse of announceRevocation's own docstring 40 lines below. Acting on the header reintroduces the bug feat(linear): detect, latch, and notify on revoked channel credentials — today a dead grant surfaces only as a failed user task #812 exists to fix.
• 6. cdk/test/handlers/shared/linear-revocation-recorder.test.ts:211 — the test is titled for the reason mapping and asserts the ConditionExpression. This is the one category a fast-follow structurally cannot reach: the artifact left behind is a passing, well-named test, indistinguishable from the work being done. Capture ExpressionAttributeValues and assert :reason on both the Linear-refused and vault-inferred paths (~6 lines).
• 7. Docs + scanner hygiene (~4 lines total) — docs/guides/LINEAR_SETUP_GUIDE.md:24 recommends --context enableLinearIdentityVault=true with no note that compute_type=lambda-microvm hard-fails synth at 505 resources; today the only record is a test title (cdk/test/stacks/agent.test.ts:1721). Add it to the guide (+ mise //docs:sync) and ADR-016's scope table. Same pass: dismiss CodeQL docs: refactoring docs #41 / feat(slack): add Slack integration with @mention-based task submission #42 with reasons so the next reviewer can tell them from a regression, and replace the live deployment hostname d2ud1woydykuxp.cloudfront.net at cli/test/commands/linear.test.ts:294 with https://d111111abcdef8.cloudfront.net/ — this is a public sample repo, and it also removes what docs: refactoring docs #41 latched onto.

Explicitly fine as a fast-follow — including three I'd marked MAJOR, which I'm downgrading:

• The un-latch not being env-gated (linear-oauth-resolver.ts:444) — it only runs on an already-latched row, so the surface is narrow.
• The quadruplicated vault cache-key constants + contracts/constants.json parity test — a guardrail, and the four copies are currently consistent.
• agent/src/config.py:135 (platform_client outside the try) — the same defect class as item 1, so take it if you're already there; the agent image pins botocore, which makes UnknownServiceError near-unreachable in practice.
• All remaining nits: the :398 raw literal and reason typing, parseOauthSecret mode-keyed overloads, RegistryRow.revoked_at, resolveLinearVaultWorkloadName's .catch, the two stale --hosted references, linear-auth-health.ts:38, :308, :910, the comment archaeology / proportionality sweep, linear-webhook-processor.test.ts:1092's adjacency assertion, the per-test AgentStack synth at agent.test.ts:1595+, the linearVaultHostedReturnUrl doc, and the nested-type gap in synth-coverage.test.ts:158.

I'll approve as soon as the seven land. The engineering quality assessment in my prior review is unchanged and I'll repeat it here: the tri-state VaultTokenResult, the announce-before-latch with an independent revocation_announced_at claim, the installed_at-conditioned latch, and the UpdateItem-only registry grant are all correct and well argued in place.

[isadeks]

Addressed at 6288d1c7 — three commits

Thank you for withdrawing B1 the way you did. Bundling each entry with CDK's own esbuild config and counting the emitted require is a better standard than the static import-chain reading I would have applied, and it found the thing that settles it: main already ships require("@aws-sdk/client-bedrock-agentcore") from fanout-task-events and github-webhook-processor under the identical externalization, so the module demonstrably resolves in nodejs24.x.

The two-line remnant was real and is fixed: makeClient now sits inside the try, so an SDK surprise degrades to { kind: 'unavailable' } and the Secrets-Manager fallback rather than escaping past the classification into a Step 1b that has no guard.

Every finding below I verified against the code before changing anything. None was a false positive — I looked, and the one thing worth reporting in that direction is that the four cache-key copies already agree, so that work is a guard rather than a repair, and the commit says so.

The four correctness MAJORs

setup stripping the vault binding. Confirmed: alreadyOnVault is used at :954 ("Only a FIRST onboarding falls back") but not at :928, so a missing stack output or a --code resume rewrote the row whole and dropped provider_name/vault_user_id. It now fails fast, naming which of the two causes applies. Your point about the fabricated diagnosis was right too — nothing probed regional availability, only this stack's outputs — so the message now says that instead of sending the operator to the AWS region table.

The CLI collapsing "vault unreachable" into "grant revoked". Fixed as you suggested: mintLinearTokenFromVault returns a tri-state mirroring the cdk-side VaultTokenResult, unavailable maps to unknown, and a latch is no longer overturned by a probe that merely failed. The two callers that genuinely only need a token now say so explicitly at the call site. The nosemgrep rationale claiming the distinction changes no caller's action was falsified by the caller this PR added — it is gone, not reworded.

The un-latch not env-gated like the latch. Gated on recordRevocation, which is present exactly where the registry write is, so the same permission governs both directions and no new env read is introduced. A test pins that a read-only role still serves the token and attempts no write.

The quadruplicated cache key. Now a linear_vault block in contracts/constants.json, enforced the two ways this repo already enforces such things: check:constants-sync forbids config.py re-declaring the literals, and parity tests assert each TypeScript copy equals the contract.

Two things surfaced while wiring that, both of which would have left the guard decorative. agent/src/config.py was not in the gate's PYTHON_CONSUMERS, so the new patterns matched nothing — found by re-declaring a literal and watching the gate stay silent. And the gate's own fixture enumerates the files it copies, so it needed config.py too, or the checker read a path the fixture never created.

Tests and docs

The mislabelled test. You were exactly right, including the consequence: the harness never captured ExpressionAttributeValues, so the reason ternary was unverified and collapsing it to a constant kept the suite green. The harness now captures the values and can drive the vault source, both arms assert :reason, and I confirmed the collapse you described now fails.

Vault + lambda-microvm. AgentStack refuses the combination by name at synth instead of leaving it to the resource counter, whose per-type census never mentions either flag. Documented in the setup guide and ADR-016's substrate notes rather than living only in a test title. The test that pinned the 500-limit symptom now pins the explicit refusal.

Nits

All fixed except two, noted below. Four comments had stopped being true, three of them describing mechanisms this PR changed — the alert header still claimed the latch is the dedup key, agent.ts still called the workload name fixed, linear-auth-health still claimed it never consumes a refresh token, and the error-level justification cited an announcement that has not happened on the no-topic path. Also fixed: the raw 'vault_consent_required' literal, a typed LinearRevocationReason, RegistryRow.revoked_at, the .catch that turned a DescribeStacks AccessDenied into a fallback name no current stack uses, both --hosted references, and the guard-location pointer.

Your proportionality point landed, and I applied it in both directions: the LINEAR_REVOCATION_RECORDING block went from 19 lines to 9 and vaultMetadata's docstring from 18 to 9, keeping every decision — including why granting the reconciler the write is refused — and dropping the archaeology. Where a fact came from a live run rather than reasoning that is still stated, since it is the reason not to "simplify" the code back.

The vaultMetadata structural guard no longer requires adjacency; it scans to the end of the enclosing literal, so a key reorder cannot fail it while a new unspread builder still does. Fixtures use AWS's documentation host now, and both CodeQL alerts are dismissed with reasons.

Deferred, deliberately: the parseOauthSecret mode-keyed overloads. It is a genuine improvement and there is no live defect behind it — the only 'without-grant' caller is webhook verification — so I would rather not reshape a parse path this PR already changed twice. And the commands/linear.ts split, which we agree is a follow-up.

Not fixed, and I would rather you decide: your adjacent finding about cli/src/linear-oauth.ts printing the raw token body was fixed here in 2ced095e, but cli/src/jira-oauth.ts:327 has the identical pattern. That file is not in this PR's diff, so I left it — say the word and it gets its own PR.

mise run build green at this HEAD: CDK 4454, CLI 903, agent 1782.

[scottschreckengaust]

LGTM - consider a followup to address comments