[isadeks]

Closes #914.

The bug

bgagent linear app-template printed the hosted consent page as the only Linear Redirect URI. On the vault path Linear redirects to the AgentCore provider callback, so an operator who registered exactly what the template printed still failed consent with Invalid redirect_uri parameter for the application. Hit live on a three-workspace dev stack.

Which URI Linear checks, per setup path

Path	redirect_uri Linear validates
setup with the Identity vault	AgentCore provider callback, minted when setup first registers the app
setup on a vault stack that falls back to Secrets Manager (AgentCore unavailable on first onboarding; also setup --code resuming one)	the hosted consent page
no vault, or add-workspace	the CLI's loopback listener

The original template only handled the second row. An earlier revision of this PR overcorrected and claimed the consent page is never a redirect URI, which was wrong for that fallback. Thanks @ayushtr-aws for catching it.

What the template prints now

• Redirect URIs field: the vault-path URI.
  • Provider exists → the AgentCore callback.
  • Vault, first run → Leave empty — bgagent linear setup <slug> prints the URI to add, then stops. Linear's create-app form accepts an empty Redirect URIs field (confirmed on the live form).
  • No vault → the loopback listener.
• After the form: Identity vault consent page: <url> — "Add it as a Redirect URI only if setup falls back to Secrets Manager — setup says so when it does." (setup does say so, linear.ts around :1156.)
• Traps: a line for add-workspace's loopback when the field doesn't show it.

setup prints the callback on every consent

provider.created was standing in for "this URI is registered on the Linear app". It is true once per provider lifetime, while the real condition stays false until someone pastes the URI. So anyone who interrupted the first run got no remedy afterwards.

Lookup failures are surfaced, not swallowed

getStackOutput(...).catch(() => null) rendered a vault stack as no-vault on any credentials or region error. Separately, an unreadable provider rendered "leave it empty" for an onboarded workspace. Both now produce a ⚠ Could not read … line. probeLinearVaultCallbackUrl returns found | absent | unreadable and replaces the null-returning lookup.

Tests

chooseSetupRedirect is extracted from setup. A test derives the fallback's redirect_uri from it and asserts the template tells the operator to register that URI — the invariant that drifted. Also covered:

• the lookup warning (both stack and provider)
• the add-workspace trap
• the probe's absent vs unreadable
• redirectUriBlock no longer passes vacuously when the label is missing

Verified

• Local gate: mise run build passes — 1,002 CLI / 1,800 agent / 4,781 CDK (222 suites) — plus cdk:synth:quiet and eslint.
• Live, against the vault stack:
  • --slug <onboarded> prints that workspace's real callback.
  • A bare run prints the deferred line.
  • A wrong-region run prints the warning instead of a silent no-vault render.

Notes

• Not based on fix(linear): bind webhook routing values to the workspace that signed the delivery #917. Inferring the slug from the registry belongs with its listOnboardedWorkspaceSlugs. That's a natural follow-up once it merges.
• Docs: one sentence in LINEAR_SETUP_GUIDE.md on the empty first-run field and the fallback. Mirror regenerated via mise //docs:sync.

[ayushtr-aws]

Verdict: Request changes (posted as a comment). The vault-path fix is right, and printing the callback on every consent is a real improvement. Two problems block it, though. The PR rests on "Linear never redirects to the consent page", which is false for one setup path that is still supported. And the new comments contradict each other.

Governance: #914 is approved and P1. The branch name is compliant and CI is green.

Blocking

1. On the Secrets-Manager path, the consent page is Linear's redirect_uri.

   • cli/src/commands/linear.ts:1110-1111: setupHostedUrl = vaultAccessToken ? undefined : consentPageUrl; setupRedirectUri = setupHostedUrl ?? CALLBACK_URL. The authorize URL and the token exchange both send this value.
   • A vault stack reaches this path in two ways:
     • (a) The first-onboarding fallback when upsertLinearCredentialProvider throws and isVaultUnavailableError returns true (:1009-1013). That covers anything other than Validation/Conflict/AccessDenied/Unauthorized: throttling, network errors, ResourceNotFound.
     • (b) The --code resume, which replays pending.redirectUri.
   • setup itself says so at :1155-1161: "That exact URL must be registered as a Redirect URI…".
   • The template now labels the consent page "nothing to paste" (:359) and the field "Leave empty" (:334). An operator who follows it exactly then hits Invalid redirect_uri on the fallback.
   • The comments at :297-303 and :353-356 ("never Linear's redirect_uri", "no flow uses") are wrong for this path. They will mislead whoever touches it next.
   • Fix: keep the consent page out of the main field. Change the label to something like Identity vault consent page — add as a Redirect URI only if setup falls back to Secrets Manager (setup will say so), and narrow the comments to "on the vault path".

2. Contradictory comments in renderLinearAppTemplate.

   • :310-315 ("Loopback stays listed even when the vault is deployed… Linear requires at least one entry… dropping it would leave the field empty") describes the opposite of the code below it (:316-340). It looks left over from an earlier revision. Delete it.
   • Also fix cli/test/commands/linear.test.ts:359-363. It says the callback and the loopback "both belong", but the test asserts the loopback is absent.

3. Is "Leave empty" actually possible? The only statement in the repo on this is the stale comment above, which claims Linear requires at least one Redirect URI to create the app. The description says a bare run "defers to setup". It doesn't say an app was actually created with this field empty. Please confirm on Linear's create-app form. If an entry is required, a first vault run can't finish the form, and the template needs a placeholder strategy (for example, the loopback marked "temporary, replace after setup").

Non-blocking

• linear.ts:1092-1094: the stale user-facing text "confirm the Linear app lists both redirect URIs printed above". Only one is printed now.
• linear.ts:185-189: the hostedConsentUrl JSDoc still says it "is listed as a callback URL".
• The add-workspace trap was removed (old :344-350). Vault-stack operators using add-workspace now get no hint that they need the loopback until it fails at :1725. A one-line trap would cover it.
• linear.ts:245: getStackOutput(...).catch(() => null) now chooses between the three branches. With expired credentials or a wrong region, it silently renders the no-vault branch (register loopback, deploy the vault) on a stack that already has the vault. Consider rendering "could not read stack outputs" separately. The same applies to lookupLinearVaultCallbackUrl swallowing errors, which renders "Leave empty" for an already-onboarded --slug.
• cli/test/commands/linear.test.ts:164-169: redirectUriBlock passes vacuously if 'Redirect URIs' isn't found, because slice(-1, end) returns '', and several new tests make only not.toContain assertions. Add expect(start).toBeGreaterThanOrEqual(0); expect(end).toBeGreaterThan(start).
• Tests don't cover the setup-side change at all: the callback printed on every consent-required (:1063-1068), the error at :1092, and the Secrets-Manager fallback's redirect_uri choice. The last one would have caught blocker 1.
• The commit body still says the field "renders the same two-URI list". That's stale, so squash with an updated message.
• docs/guides/LINEAR_SETUP_GUIDE.md:73 ("paste exactly what the template prints") reads oddly now that the template says "Leave empty". One sentence on the empty-field first run would help.

Documentation

• No generated-mirror impact: no docs/ sources changed.
• The guide is not contradicted, but it says nothing about the first-run empty field (see above).

Tests & CI

• All checks pass.
• Template-side coverage is good. setup-side coverage is missing.
• Bootstrap synth-coverage: not applicable (CLI only).

Review agents run

• Run: code-reviewer, comment-analyzer, pr-test-analyzer, silent-failure-hunter.
• Omitted:
  • type-design-analyzer: no new types; the return type was only reformatted.
  • /security-review: no IAM, Cedar, network or secrets changes.

Human heuristics

• Proportionality: concern. The comment-to-code ratio in renderLinearAppTemplate (:291-340) is very high, and it is internally inconsistent. Trim it to the one true explanation.
• Coherence: concern. Inside one file, the template says the consent page is never a redirect URI, while setup (:1155) says it must be registered.
• Clarity: concern. "Nothing to paste" hides a real requirement on the fallback path, and swallowed lookup errors render the wrong branch with no signal (:245).
• Appropriateness: pass, with a caveat. The vault path was verified live, but the "Leave empty" claim about Linear's form needs checking against the real form.

[isadeks]

Thanks @ayushtr-aws — thorough and right on all three blockers. Addressed in a30ffabe, squashed onto current main.

Blocking

1. Consent page on the Secrets-Manager path — you're right, and it was the PR's core premise. chooseSetupRedirect (extracted from setup's :1111-1112) sends the consent page whenever vaultAccessToken is unset, so both the first-onboarding fallback and the --code resume use it. Changes:
   • The template prints it after the form as Identity vault consent page: <url> — Add it as a Redirect URI only if setup falls back to Secrets Manager — setup says so when it does. The "nothing to paste" label is gone.
   • The comments now describe the redirect URI per path (vault / Secrets-Manager fallback / no-vault or add-workspace), so nothing claims "never".
   • New invariant test: it takes the fallback's redirect_uri from chooseSetupRedirect itself and asserts the template tells the operator to register it. That is the drift that caused this.
2. Contradictory comments. Replaced the three stacked comment blocks in renderLinearAppTemplate with one accurate one. Also fixed the linear.test.ts comment you cited, plus two more test comments that repeated the "never a redirect URI" premise.
3. Empty field. Confirmed on Linear's live create-app form: it accepts an empty Redirect URIs field. So "Leave empty" stands on a first vault run, and the stale "Linear requires at least one entry" comment is gone.

Non-blocking — all taken

• :1092 now names the single vault callback instead of "both redirect URIs".
• hostedConsentUrl JSDoc corrected.
• The add-workspace trap is back, rendered only when the field doesn't show the loopback.
• Swallowed lookups.
  • Stack-output failures no longer render a vault stack as no-vault; they produce ⚠ Could not read stack <name> in <region> (<Error>).
  • New probeLinearVaultCallbackUrl returns found | absent | unreadable, so an unreadable provider warns instead of saying "leave it empty". It replaces the null-returning lookup, and its tests were ported.
  • Verified live: a wrong-region run now prints the warning.
• redirectUriBlock asserts the label and Public: were both found, so not.toContain checks can no longer pass vacuously.
• Setup-side coverage: chooseSetupRedirect is now tested directly for both the vault and fallback legs.
• Commit squashed with an accurate message; the PR description is rewritten too, since it repeated the false premise.
• LINEAR_SETUP_GUIDE.md: one sentence on the empty first-run field and the fallback. The mirror was regenerated via mise //docs:sync.

Verification

• mise run build passes locally: 1,002 CLI / 1,800 agent / 4,781 CDK, plus cdk:synth:quiet and eslint.
• Live against a vault stack:
  • --slug prints the workspace's real callback.
  • A bare run prints the deferred line.
  • A wrong-region run prints the lookup warning.

[ayushtr-aws]

Verdict: Approve with nits

This fixes #914, an approved P1 bug, at the root cause. The template now prints the redirect_uri that each setup path actually sends. Setup reprints the vault callback on every consent, so it no longer relies on provider.created as a proxy. Lookup failures now show up as warnings instead of rendering as a no-vault stack. The change is scoped to the CLI and the docs mirror is in sync. The vision point (reviewable, unattended onboarding) is improved, and no tenets are traded away.

I traced each path against the template's claims, and they hold:

• First vault run: setup prints the callback and returns before consent.
• Later vault runs: setup prints the callback before the authorize URL.
• Secrets Manager fallback on a vault stack: setup warns that the consent page must be registered (about linear.ts:1173, not :1156).
• --code resume: setup reuses pending.redirectUri.
• No vault: setup uses the loopback.

ResourceNotFoundException is the documented not-found error for GetOauth2CredentialProvider. Nothing still calls lookupLinearVaultCallbackUrl.

Non-blocking (worth fixing here or in a follow-up)

1. The add-workspace trap prints the wrong URI when --aws-callback-url is set. cli/src/commands/linear.ts:391 interpolates callbackUrl (opts.awsCallbackUrl ?? loopback, :304), but add-workspace always sends CALLBACK_URL (:1692). The old code hard-coded the loopback here. Fix: use CALLBACK_URL.
2. A vault callback without a consent page drops the loopback. This is linear.ts:340-351 and :390. Example: a stack redeployed without the vault but with a leftover AgentCore provider, run with app-template --slug acme. A manual --vault-callback-url with no stack, or a mistyped --stack-name, does the same. In each case the field lists only the vault callback, and the loopback note and trap disappear. setup then sends the loopback, which is not registered. Before this PR the loopback was printed whenever hostedConsentUrl was absent. Fix: put the callback in the field only when hostedConsentUrl is also set, and otherwise fall through to the loopback branch.
3. lookupWarning contradicts URLs the user passed. With --vault-callback-url / --hosted-consent-url and expired credentials, the template says "may be wrong — check credentials" right above the URL the user typed (:331-336, :655-657). Fix: drop the failures for lookups the user overrode.
4. The headline behaviour has no test. Printing the callback on every consent (:1068-1081) is untested, so deleting the block passes CI. Fix: extract a small pure helper or add one action test with created: false and consent required. Also untested: --code resume exchanging with the saved hosted URI.
5. The chooseSetupRedirect invariant test is partly tautological (linear.test.ts:244). It feeds the same literal to both functions by hand, covers only the vault-callback-present case, and depends on lines[at + 1] layout. Fix: loop over all (vaultAccessToken, consentPageUrl, vaultCallbackUrl) combinations and assert that redirectUri appears in the field or in the conditional section.
6. A provider with no callbackUrl is reported as absent (linear-vault.ts:179). The template then says "Leave empty" for a provider that exists. Fix: map it to unreadable (errorName: 'MissingCallbackUrl'). Relatedly, if (provider.callbackUrl) at :1076 skips the notice silently when upsert returns ''.

Nits

• No region still renders no-vault without a warning. linear.ts:249 was deferred in #914, but the new failures list is the natural place to add "no region — stack/vault not checked" when stackName or slug is set. Also consider falling back to AWS_REGION.
• The warning carries only err.name. A DNS or endpoint failure from a wrong --region reads as (Error). Append a truncated message.
• "…prints the URI to add, then stops" (:347) is inaccurate without --slug when a provider already exists. In that case setup prints the callback and then continues to consent. Suggest "prints the URI to add before consent".
• chooseSetupRedirect returns a loopback redirectUri on the vault leg, where it is never sent. A discriminated union (vault | hosted | loopback) would remove the duplicated fact, and it would also make the test at linear.test.ts:258 meaningful.
• Comment hygiene:
  • The JSDoc on VaultCallbackProbe describes the function, so probeLinearVaultCallbackUrl has no JSDoc of its own.
  • Drop the dated "verified on the live form, 2026-10" (:343).
  • Trim the history narration (:1066-1073, the second paragraph in linear-vault.ts:152-164, "#914 review" / "blocker 1" in tests).
  • Write the #917 reference at :261 as TODO(#917).

Docs

LINEAR_SETUP_GUIDE.md is updated to match the template output. The Starlight mirror is regenerated and in sync.

Tests & CI

The 3 touched suites (114 tests) pass locally. Semgrep silent-success-masking is clean on both touched files, so removing the nosemgrep is safe. CodeQL, secrets/deps, dead-code and the title check pass. build (agentcore) was still pending when I reviewed, so this approval assumes it goes green. Bootstrap synth-coverage does not apply (no CDK changes).

Review agents run

• code-reviewer
• silent-failure-hunter
• pr-test-analyzer
• type-design-analyzer
• comment-analyzer

Omitted: /security-review. There are no IAM, Cedar, network, secrets or input-gateway changes; the redirect-URI handling was covered by code-reviewer.

Human heuristics

• Proportionality: pass. The fix is targeted, and the probe union is justified. linear.ts is 2,924 lines, but that predates this PR.
• Coherence: pass. Terms match the setup guide.
• Clarity: concern, minor. The history-narrating comments and chooseSetupRedirect's duplicated return fields (above).
• Appropriateness: pass. It was verified live against a vault stack. The test gaps in non-blocking items 4 and 5 are the main residual risk.