fix(cli): linear setup must not clobber a workspace's webhook signing secret (#611)

[isadeks]

Summary

Fixes #611. Re-running bgagent linear setup <slug> on an already-installed workspace overwrote its per-workspace webhook signing secret with the stack-wide fallback (a different workspace's secret once >1 is installed) → webhook 401 {"error":"Invalid signature"} for every workspace after the first. Live-hit on demo-abca after a token re-auth; recovery required update-webhook-secret.

Fix

• New pure helper resolveWebhookSecretAction() (cli/src/linear-oauth.ts): preserve | mirror-stackwide | prompt. An existing valid per-workspace secret always wins; else mirror stack-wide (warn for additional workspaces); else prompt.
• setup reads the existing per-workspace secret before the OAuth overwrite and preserves it.
• 5 unit tests incl. the multi-workspace re-run regression.

Governance

Approved issue #611, conforming branch fix/611-….

Test

cli build green — 610 tests (compile + eslint clean).

[scottschreckengaust]

Verdict: Request changes

The design is right and the diagnosis is excellent — extracting the three-way decision into the pure resolveWebhookSecretAction() is the correct move, the helper is well-documented, and its five unit tests cover every branch including the malformed-lin_wh_ fall-through. The receiver semantics confirm the bug is real and serious: verifyLinearRequestForWorkspace (cdk/src/handlers/shared/linear-verify.ts:224-239) treats a per-workspace signature mismatch as fatal with no stack-wide fallback (linear-webhook.ts:125-129), so clobbering workspace #2's secret with workspace #1's stack-wide value does 401 every delivery. Good catch, good structure.

But the fix is only as safe as the value fed into the pure helper, and the impure pre-read that produces that value fails open. Under any Secrets Manager error other than "not found" on a genuine re-run, the code silently re-triggers the exact clobber this PR exists to prevent — while printing green checkmarks. That, plus the absence of any end-to-end test proving acceptance criterion #1, is why this is Request changes rather than Approve-with-nits. The pure helper is tested; the fix is not.

Governance (ADR-003) — PASS

• Backing issue #611 carries the approved label (and bug); acceptance criteria match the implementation.
• Branch fix/611-linear-setup-webhook-secret conforms to (feat|fix|chore|docs)/<issue>-desc.
• Base is fresh: merge-base == origin/main == 22705e8. All 8 CI checks green (CodeQL, build agentcore, secrets/deps/workflow scan, PR-title, dead-code advisory).
• No prior reviews/comments to reconcile.

Vision alignment — PASS

Advances bounded blast radius (a re-run/re-auth on one workspace no longer silently breaks signature verification for the others) and reviewable outcomes (the failure mode was an opaque 401; the fix keeps recovery to the dedicated update-webhook-secret command). No tenet is traded; no ADR needed.

Blocking issues

B1 — The pre-read catch {} fails OPEN and re-introduces the clobber this PR fixes. (cli/src/commands/linear.ts:621-623)

The bare catch {} swallows every error from GetSecretValue and treats it as "first install — nothing to preserve." That assumption only holds for ResourceNotFoundException. For AccessDenied, KMSAccessDeniedException (CMK disabled / key-policy / grant revoked), ThrottlingException, InternalServiceError, or a network fault — all of which occur on a secret that does exist — existingWebhookSecret stays undefined. Traced forward: resolveWebhookSecretAction(undefined, true) → mirror-stackwide (linear-oauth.ts:333-334) → the stack-wide secret (workspace #1's) is read at line 727 and merged into this workspace's bundle at lines 764-771. That is the original bug, now re-armed by an IAM/KMS/throttle hiccup, and because the clobber is a successful PutSecretValue the operator sees only "Setup complete." It surfaces hours later as webhook 401s.

This is strictly worse than the anti-pattern the sibling reader isWebhookSecretConfigured (lines 169-194) was deliberately written to avoid — its own comment (178-182) warns that a bare catch { return false } masks IAM problems, and it re-throws a CliError with an IAM hint on any non-ResourceNotFoundException. The degraded path there merely re-prompts (harmless); here it destroys a working secret (harmful). It is also an AI004 silent-success-masking pattern (empty catch + masking default, no logging, no nosemgrep justification). Fix: fail closed — catch narrowly, treat only ResourceNotFoundException as "nothing to preserve," and re-throw an actionable CliError on everything else, mirroring isWebhookSecretConfigured. Note the JSON.parse at line 616 is inside the same try: a corrupt-but-present bundle should surface, not silently become mirror-stackwide. See inline suggestion.

B2 — No test proves acceptance criterion #1 end-to-end; the load-bearing wiring is uncovered.

Issue #611's AC #1 ("re-running setup does NOT change webhook_signing_secret") and AC #3 ("regression test for the second-workspace re-run case") are asserted only by proxy: the five new tests exercise the pure resolveWebhookSecretAction in isolation (cli/test/linear-oauth.test.ts:286-316). Nothing exercises the setup action itself — not the read-before-overwrite capture (linear.ts:612-623), not its ordering against the strip-and-write at line 646, not the mirror-back at 764-771, and not the B1 fail-open path. A future refactor that moved the read past line 646, or the B1 error path, would leave every current test green while re-breaking the fix. The scaffolding to close this already exists: cli/test/commands/linear-helpers.test.ts:45-62,197-208 module-mocks SecretsManagerClient.send and drives setup via parseAsync. Fix: add a setup-action test where the prior per-workspace bundle holds lin_wh_thisWorkspace, the stack-wide holds a different lin_wh_other, and assert the final per-workspace PutSecretValue payload still contains lin_wh_thisWorkspace. Add a companion test that a non-ResourceNotFoundException on the pre-read does not silently proceed to overwrite (guards B1).

Non-blocking suggestions / nits

N1 — Stale header comment contradicts the new code. (cli/src/commands/linear.ts:699-706) The block still reads "…Either way the stored value is this workspace's signing secret — lift it into the per-workspace bundle without prompting." That is the removed behavior; the whole point of the fix is that a populated stack-wide secret is not necessarily this workspace's. A maintainer following this comment would revert the fix. Rewrite it to describe the preserve → mirror-stackwide → prompt decision. (Not blocking, but high maintainability risk — please fix in this PR.)

N2 — preserve path double-writes and logs a misleading message. (cli/src/commands/linear.ts:646 + 764-772) In the preserve case, line 646 writes the bundle without the secret, then line 770 re-writes it with the same secret that was already stored — two secret versions to land where storage already was, and it prints "Mirrored signing secret to per-workspace OAuth bundle" when nothing was mirrored. Folding existingWebhookSecret into the initial stored object (...(existingWebhookSecret ? { webhook_signing_secret: existingWebhookSecret } : {})) would make line 646 write the correct bundle once, let the preserve path skip the re-write, and — importantly — close a narrow correctness window: today, if any fallible step between 646 and 770 throws (the DynamoDB PutCommand at 656, or isWebhookSecretConfigured at 707, which is designed to throw), the bundle is left persisted without webhook_signing_secret and the 401 returns until update-webhook-secret is run. Optional but recommended alongside B1.

N3 — lin_wh_ prefix as validity check is a heuristic, not authentication, but it is consistent with every sibling in this file — leave as-is.

Documentation — PASS (nothing required)

No user-facing command, flag, env var, or contract changed — this is an internal idempotency/safety fix to existing setup behavior. docs/guides/LINEAR_SETUP_GUIDE.md already documents setup and update-webhook-secret; no update needed, and the Starlight mirror is untouched (no mutation-check risk). Not a roadmap item.

Tests & CI

• Pure-helper coverage: strong. Integration/action coverage of the actual fix: missing (see B2).
• No CDK construct/stack changes → bootstrap synth-coverage: not applicable; no Cedar engine pins moved; no types.ts change (the CLI change is self-contained — webhook_signing_secret already exists and matches on both StoredLinearOauthToken (cli/src/linear-oauth.ts:101) and the CDK resolver (cdk/src/handlers/shared/linear-oauth-resolver.ts:93), no drift).
• All 8 CI checks green on head 45a770b.

Review agents run

• pr-review-toolkit:silent-failure-hunter — invoked. Independently identified B1 as fail-open/AI004; I verified the trace by hand (confirmed stored at lines 624-638 omits webhook_signing_secret, so line 646 strips it; confirmed resolveWebhookSecretAction(undefined, true) → mirror-stackwide).
• pr-review-toolkit:pr-test-analyzer — invoked. Confirmed B2: helper-only coverage, existing mock scaffolding in linear-helpers.test.ts makes the integration test affordable.
• pr-review-toolkit:code-reviewer — invoked. Surfaced N1 (stale comment), N2 (double-write + failure window), and confirmed no secret leakage into logs and no types.ts drift.
• /security-review — done by hand (secrets-handling change): fail-closed analysis (B1), no secret leakage into stdout/errors (the mirror-stackwide catch at line 732 prints only the SDK .message), prefix-validation robustness (N3). No sub-agent for this dimension was available; covered manually.
• Omitted: type-design-analyzer (the new WebhookSecretAction discriminated union is small and idiomatic; no design concern), comment-analyzer (folded into N1 by hand).

Human heuristics

• Proportionality — PASS. A pure decision function + a read-before-write is proportional to the bug; no over-abstraction.
• Coherence — CONCERN. The pre-read (linear.ts:613-623) diverges from the sibling reader isWebhookSecretConfigured (169-194) that solves the identical "read a SM secret in setup" problem — same concept, inconsistent error handling (B1).
• Clarity — CONCERN. The stale header comment (N1, lines 699-706) actively describes the removed behavior; the empty catch (B1) hides failures behind a plausible "first install" default (AI004).
• Appropriateness — CONCERN. Tests assert what the helper does, not what the fix should do end-to-end (B2, AI005); the integration path was verified against no test at all, only the pure function.

_{🤖 Generated with Claude Code}

[isadeks]

Addressed in e26085d:

B1 (fail-open re-armed the clobber) — fixed, fail closed. Extracted the pre-read into readExistingWebhookSecret() (in linear-oauth.ts, next to resolveWebhookSecretAction). Only ResourceNotFoundException → "nothing to preserve"; a corrupt-but-present bundle (JSON.parse) or ANY other SM error (AccessDenied / KMSAccessDenied / Throttling / network) now propagates, and setup re-throws a CliError with an IAM/KMS hint — mirroring the sibling isWebhookSecretConfigured you pointed to. No more silent mirror-stackwide-over-a-working-secret behind a green checkmark.

B2 (fix untested end-to-end) — added 7 tests. Rather than mock the full OAuth dance (browser + callback server + code exchange + prompts) to reach the seam, I extracted the load-bearing logic into readExistingWebhookSecret and unit-tested it directly: preserve-not-clobber (returns lin_wh_thisWorkspace → resolveWebhookSecretAction → preserve), ResourceNotFound→undefined, malformed/absent secret, and the fail-closed THROW on AccessDenied / KMS / Throttling / corrupt-JSON (the exact B1 path). That guards both AC#1 and the B1 regression at the function boundary.

N1 (stale comment) — fixed. Rewrote the Step-6 header to describe the preserve → mirror-stackwide → prompt decision (it still described the removed "stored value is this workspace's secret — lift it in" behavior).

N2 (double-write + failure window) — fixed. Folded existingWebhookSecret into the INITIAL stored bundle so the OAuth-secret write lands it in one PutSecretValue; the preserve path skips the re-write (guarded by !== stored.webhook_signing_secret), closing the window where a throw between the two writes left the bundle persisted without the secret.

N3 — leaving the lin_wh_ prefix heuristic as-is per your note (consistent with every sibling). Full cli build green (616 tests).

[scottschreckengaust]

Verdict: Approve with nits

The re-review clears the prior Request changes. The two blocking items from my earlier review are genuinely resolved in the current head (c9a5fc7), verified against the worktree — not just asserted in the commit message — and the one residual gap (an end-to-end setup-action regression test) is a should-fix follow-up, not a merge blocker, because the load-bearing logic on both sides of the untested seam is now independently unit-tested, cli typecheck is clean, and all 8 CI checks are green.

Re-review of prior blocking claims (independently verified against head, not taken on faith)

B1 (fail-open pre-read re-armed the #611 clobber) — RESOLVED. The bare catch {} is gone. The pre-read is extracted to readExistingWebhookSecret() (cli/src/linear-oauth.ts:364-379), which fails closed: only the caller-supplied isNotFound predicate (ResourceNotFoundException) yields undefined (genuine first install); every other GetSecretValue error — and a corrupt-but-present bundle (JSON.parse throws) — propagates. The setup caller (cli/src/commands/linear.ts:629-641) re-wraps a propagated error in a CliError with an actionable secretsmanager:GetSecretValue / kms:Decrypt hint, mirroring the sibling isWebhookSecretConfigured. I confirmed the fail-closed path with tests: readExistingWebhookSecret throws on AccessDenied, KMSAccessDeniedException, ThrottlingException, InternalServiceError, and malformed JSON (cli/test/linear-oauth.test.ts:307-329). Ran the suite: 70/70 pass; linear-oauth.ts at 100% statement coverage.

N1 (stale Step-6 header describing removed behavior) — RESOLVED. The header (linear.ts:726-739) now describes the preserve → mirror-stackwide → prompt decision. A maintainer can no longer follow the comment back into the bug.

N2 (double-write + mid-setup failure window) — RESOLVED. The preserved secret is folded into the initial stored bundle (linear.ts:663) so the first upsertOauthSecret lands the correct payload in one write, and the mirror-back is guarded by webhookSigningSecret !== stored.webhook_signing_secret (linear.ts:799) so the preserve path skips the redundant re-write. I traced all three branches: preserve → guard equal → skip (correct); mirror-stackwide/prompt → stored.webhook_signing_secret undefined → guard true → write (correct). Window closed.

Remaining nit (should-fix follow-up, non-blocking)

B2 (partially addressed) — the setup-action wiring is still asserted only by inspection. The fail-closed seam is now well-tested (7 new readExistingWebhookSecret cases) and the pure decision is fully tested (resolveWebhookSecretAction), which covers the safety half of the original B2. But no test drives the setup action end-to-end to prove issue #611's AC#3 ("regression test for the second-workspace re-run case") against the final PutSecretValue payload. Coverage confirms the gap: linear.ts:741-842 (the entire secretAction branch and the mirror-back guard) is uncovered. The scaffolding to close it already exists — cli/test/commands/linear-helpers.test.ts module-mocks SecretsManagerClient.send and drives setup via parseAsync. Recommended follow-up: a setup-action test where the prior per-workspace bundle holds lin_wh_thisWorkspace, the stack-wide holds a different lin_wh_other, and the final per-workspace PutSecretValue payload is asserted to still contain lin_wh_thisWorkspace. I am not blocking on it because a refactor that re-broke the wiring would have to defeat both independently-tested halves, and the guard logic is simple and verified by inspection — but it is the one thing that would make the fix self-defending. File it as a P2 test-debt follow-up.

N3 (lin_wh_ prefix as a validity heuristic) — unchanged, consistent with every sibling in the file. Leave as-is.

Vision alignment — PASS

Advances bounded blast radius (a re-run/re-auth on one workspace no longer silently breaks signature verification for the others) and reviewable outcomes (recovery stays scoped to the dedicated update-webhook-secret command). No tenet traded; no ADR needed.

Governance (ADR-003) — PASS

Backing issue #611 carries approved (+ bug); the implementation matches its acceptance criteria (AC#1 and AC#2 satisfied; AC#3 covered in substance by the fail-closed + pure-helper tests, with the literal end-to-end regression test outstanding — see B2). Branch fix/611-linear-setup-webhook-secret conforms. Head c9a5fc7 merges current main.

Security — PASS

Secrets-handling change, reviewed for leakage and fail-closed behavior. No secret value reaches stdout/stderr on any path: error messages print only the SDK error name/message (linear.ts:630-640, 765), never the secret. The change is strictly fail-closed (an IAM/KMS/throttle fault now aborts setup with a clear error rather than silently mirroring the wrong secret). No IAM/Cedar/network/CDK surface touched.

Documentation — PASS (nothing required)

No user-facing command, flag, env var, or contract changed — internal idempotency/safety fix to existing setup behavior. docs/guides/LINEAR_SETUP_GUIDE.md already documents setup and update-webhook-secret. Starlight mirror untouched (no mutation-check risk). No types.ts drift: webhook_signing_secret? is optional and identical on StoredLinearOauthToken (cli/src/linear-oauth.ts:101) and the CDK resolver (cdk/src/handlers/shared/linear-oauth-resolver.ts:93).

Tests & CI

• Ran test/linear-oauth.test.ts, test/commands/linear.test.ts, test/commands/linear-helpers.test.ts at head: 70/70 pass. cli tsc --noEmit: clean.
• Pure-helper + fail-closed-seam coverage: strong. End-to-end setup-action coverage: still missing (B2 nit; linear.ts:741-842 uncovered).
• No CDK construct/stack changes → bootstrap synth-coverage: not applicable. No Cedar engine pin moved.
• All 8 CI checks green on head c9a5fc7.

Review agents run

• silent-failure-hunter (scope: the pre-read catch + readExistingWebhookSecret) — covered. Independently confirmed the catch now fails closed and re-throws with an IAM/KMS hint; no AI004 masking-default remains; JSON.parse failures surface. Verified by the fail-closed THROW tests actually running green.
• pr-test-analyzer (scope: test coverage) — covered. Confirmed via a real coverage run that linear.ts:741-842 is uncovered → the residual B2 nit; the pure helper and fail-closed seam are fully covered.
• code-reviewer (scope: guidelines/quality) — covered. No secret leakage, no types.ts drift, routing correct (CLI-only change lands in cli/), preserve/mirror/prompt branch logic verified by inspection.
• comment-analyzer (scope: comment accuracy) — covered. The fold comment (linear.ts:656-662) and the mirror-back guard comment (linear.ts:795-798) accurately describe current behavior; the previously-stale N1 header is fixed.
• type-design-analyzer (scope: new WebhookSecretAction discriminated union) — covered. The preserve | mirror-stackwide | prompt union is small, idiomatic, readonly, and makes the invalid state (a preserve without a secret) unrepresentable. No concern.
• The pr-review-toolkit subagents are not exposed as callable tools in this headless environment; each agent's scope was executed by hand with the actual test/typecheck/coverage runs above (not a substitute hand-wave — the analyses were driven by executing the suite and reading the current worktree). None omitted for scope; /security-review covered inline (secrets-handling change) since no dedicated sub-agent is available here.

Human heuristics

• Proportionality — PASS. A pure decision function plus a fail-closed read-before-write is proportional to the bug; no over-abstraction.
• Coherence — PASS (was CONCERN). The pre-read now mirrors the sibling isWebhookSecretConfigured error contract (fail closed + CliError with IAM hint) — the earlier divergence is gone.
• Clarity — PASS (was CONCERN). The stale header is rewritten; the empty catch is replaced with a fail-closed, actionable error.
• Appropriateness — CONCERN (narrowed). Tests now assert what the fix does at both seams (fail-closed pre-read + pure decision), but not yet the full setup-action payload end-to-end (B2, AI005). Acceptable to merge; file the regression test as follow-up.

_{Re-review at head c9a5fc7. Prior blocking items B1/N1/N2 verified resolved against the worktree; B2 downgraded to a should-fix follow-up.}

[isadeks]

Closed the one remaining nit — B2 (issue #611 AC#3), the end-to-end setup-action regression test — in bc27dc48.

cli/test/commands/linear-setup-webhook.test.ts drives bgagent linear setup demo --no-browser through parseAsync with the OAuth flow mocked, for the second-workspace re-run case:

• the prior per-workspace bundle carries lin_wh_thisWorkspace,
• the stack-wide secret holds a different lin_wh_otherWorkspace (the exact fix(cli): linear setup clobbers a second workspace's webhook signing secret with the stack-wide one → 401 Invalid signature #611 clobber source).

It then asserts the final per-workspace PutSecretValue payload still contains lin_wh_thisWorkspace (the preserve path, folded into the initial bundle — never the stack-wide other), that no PutSecretValue ever targets the stack-wide ARN (rotation stays update-webhook-secret's job), and that the registry row is still written (setup ran to completion). This covers the previously-uncovered linear.ts:741-842 (secretAction branch + mirror-back guard) — a refactor that re-broke the wiring now fails a test, not just inspection.

Implementation notes matching your sketch:

• Follows the linear-helpers.test.ts / jira.test.ts precedent (module-mock SecretsManagerClient.send, CFN, DDB, awaitOauthCallback, swap global.fetch).
• Partial-mocks ../linear-oauth (requireActual + override only exchangeAuthorizationCode) so the real readExistingWebhookSecret + resolveWebhookSecretAction under test still run.
• awaitOauthCallback echoes the wizard's internally-generated state (scraped from the auth URL printed under --no-browser), so the state check passes without a real socket.

cli gate green: 621 tests, compile + eslint clean. N3 (lin_wh_ prefix heuristic) left as-is per your note.

[isadeks]

⚠️ Heads-up: the failing "Secrets, deps, and workflow scan" check is not from this PR — same pre-existing, repo-wide pattern as before, just a different dep.

• osv-scanner now flags svgo@4.0.1 (GHSA-2p49-hgcm-8545, CVSS 8.2, fix 4.0.2) — a newly-published advisory on a pre-existing transitive dep.
• This PR's diff is one test file (cli/test/commands/linear-setup-webhook.test.ts) — it touches zero deps. svgo@4.0.1 is on main too (via the ^4.0.1 range), so every open PR fails this scan until yarn.lock re-resolves svgo to 4.0.2.
• Astro is already fine on this branch (7.1.3, from the fix(deps): bump astro to 7.1.3 + re-resolve brace-expansion to clear osv-scanner (#636) #637 merge).

Everything else is green (CodeQL ×4, build, dead-code, PR-title). The fix belongs in its own small yarn.lock re-resolve against main (like #637 did for astro), not in this test-only PR.

[scottschreckengaust]

Verdict: Approve

This re-review (head bc27dc48) clears the final should-fix nit from the prior Approve with nits (dismissed) review. I independently re-verified all prior blocking claims (B1/N1/N2) against the current worktree — not the commit messages — and each is genuinely resolved. The one residual gap (B2: the end-to-end setup-action regression test for issue #611 AC#3) is now closed by cli/test/commands/linear-setup-webhook.test.ts. The fix is correct, fails closed, preserves an existing per-workspace secret, and is now self-defending against a future refactor of the load-bearing branch. Ship it.

Vision alignment — PASS

Advances bounded blast radius (a re-run/re-auth on one workspace no longer silently clobbers signature verification for the others) and reviewable outcomes (recovery stays scoped to the dedicated update-webhook-secret command; the failure mode was an opaque 401). No tenet traded; no ADR needed.

Governance (ADR-003) — PASS

Backing issue #611 carries approved (+ bug); AC#1/#2/#3 now all satisfied. Branch fix/611-linear-setup-webhook-secret conforms to (fix)/<issue>-desc. merge-base == current origin/main (238eb30) — base is fresh.

Re-verification of prior blocking claims (checked against head bc27dc4, not taken on faith)

B1 (fail-open pre-read re-armed the #611 clobber) — RESOLVED. readExistingWebhookSecret (cli/src/linear-oauth.ts:362-380) fails closed: only the caller-supplied ResourceNotFoundException predicate yields undefined (genuine first install); every other GetSecretValue error AND a corrupt-but-present bundle (JSON.parse throws) propagate. The setup caller (cli/src/commands/linear.ts:629-641) re-wraps the propagated error in a CliError with an actionable secretsmanager:GetSecretValue / kms:Decrypt hint, mirroring the sibling isWebhookSecretConfigured (linear-oauth.ts:170-195). Six fail-closed tests cover AccessDenied / KMSAccessDenied / Throttling / InternalServiceError / corrupt-JSON (cli/test/linear-oauth.test.ts:344-366). The AI004 masking-default is gone.

B2 (fix untested end-to-end / #611 AC#3) — RESOLVED. cli/test/commands/linear-setup-webhook.test.ts drives the real setup action via parseAsync (mocked OAuth flow) for the second-workspace re-run: the prior per-workspace bundle holds lin_wh_thisWorkspace, the stack-wide holds a different lin_wh_otherWorkspace (the exact #611 clobber source), and the test asserts every per-workspace PutSecretValue payload still carries lin_wh_thisWorkspace and never the other, that no PutSecretValue ever targets the stack-wide ARN (rotation stays update-webhook-secret's job), and that the registry row is written (setup ran to completion). This covers the previously-uncovered linear.ts:741-842. It requireActuals ../linear-oauth so the real readExistingWebhookSecret + resolveWebhookSecretAction run under test, and routes SM commands against real @aws-sdk/client-secrets-manager command classes (instanceof on GetSecretValueCommand / PutSecretValueCommand) — verified against real API shapes, not self-written mocks (no AI001/AI005 concern).

N1 (stale Step-6 header) — RESOLVED. The header (linear.ts:710-739) now describes the preserve → mirror-stackwide → prompt decision; a maintainer can no longer follow the comment back into the bug.

N2 (double-write + mid-setup failure window) — RESOLVED. The preserved secret is folded into the initial stored bundle (linear.ts:663) so the first upsertOauthSecret lands the correct payload in one write; the mirror-back is guarded by webhookSigningSecret !== stored.webhook_signing_secret (linear.ts:799) so the preserve path skips the redundant re-write. Traced all three branches: preserve → guard equal → skip; mirror-stackwide/prompt → stored.webhook_signing_secret undefined → guard true → write. Window closed.

Non-blocking

N3 (lin_wh_ prefix as a validity heuristic) — unchanged, consistent with every sibling in the file. Leave as-is.

Security — PASS

Secrets-handling change, reviewed for leakage and fail-closed behavior. No secret value reaches stdout/stderr on any path — error messages print only the SDK error name/message (linear.ts:630-640; the mirror-stackwide catch at 765), never the secret. The change is strictly fail-closed: an IAM/KMS/throttle fault now aborts setup with a clear error rather than silently mirroring the wrong secret. No IAM/Cedar/network/CDK surface touched.

Documentation — PASS (nothing required)

Internal idempotency/safety fix to existing setup behavior — no user-facing command, flag, env var, or contract changed. docs/guides/LINEAR_SETUP_GUIDE.md already documents setup and update-webhook-secret. Starlight mirror untouched (no mutation-check risk). No types.ts drift: webhook_signing_secret? is optional and identical on StoredLinearOauthToken (cli/src/linear-oauth.ts) and the CDK resolver (cdk/src/handlers/shared/linear-oauth-resolver.ts). No CDK construct/stack change → bootstrap synth-coverage not applicable.

Tests & CI

• Pure-helper (resolveWebhookSecretAction), fail-closed seam (readExistingWebhookSecret), and now the end-to-end setup-action wiring are all covered. The previously-uncovered linear.ts:741-842 is now exercised.
• I could not run the suite locally (the pre-staged worktree has no node_modules), but the build (agentcore) check is GREEN on head bc27dc48 — that job runs mise run build, which includes mise //cli:build (tsc compile + jest test + eslint). So the full CLI suite, including the new regression test, compiled, ran green, and passed eslint in CI. The prior reviewer also ran 70/70 green at c9a5fc7.
• CI secrets-scan (RED) — NOT a blocker on this PR. I inspected the failing job log directly. The scoped gitleaks run (security:secrets:range, range 238eb30..bc27dc48 — exactly this PR's diff) reports "no leaks found" (3 commits scanned). The sole failure is security:deps (osv-scanner): svgo@4.0.1 in yarn.lock (GHSA-2p49-hgcm-8545, CVSS 8.2, fix 4.0.2). svgo is a transitive dep present on main too; this PR's diff touches zero dependencies (CLI source + tests only). This is the known repo-wide, newly-published-advisory pattern (same class as #637's astro re-resolve) — it will red-fail every open PR until a small yarn.lock re-resolve lands on main. Determination: sibling/full-tree artifact, NOT a real secret in this diff. No literal high-entropy secret appears in the diff — the only lin_wh_* strings are obvious test placeholders (lin_wh_thisWorkspace, lin_wh_otherWorkspace, lin_wh_existing, lin_wh_other). Recommend a rebase once svgo is bumped on main so the merge-queue re-scan stays green.

Review agents run

The pr-review-toolkit subagents are not exposed as callable tools in this headless environment; each agent's scope was executed by hand against the current worktree, driven by reading the actual code and the CI job logs (not a hand-wave):

• silent-failure-hunter (scope: the pre-read catch + readExistingWebhookSecret) — covered. Confirmed the pre-read fails closed and the caller re-throws with an IAM/KMS hint; no AI004 masking-default remains; JSON.parse failures surface. Backed by the six fail-closed THROW tests.
• pr-test-analyzer (scope: test coverage) — covered. Confirmed the new end-to-end test drives secretAction + mirror-back guard (linear.ts:741-842) and asserts the final per-workspace PutSecretValue payload; closes AC#3.
• code-reviewer (scope: guidelines/quality) — covered. No secret leakage, no types.ts drift, routing correct (CLI-only change in cli/), preserve/mirror/prompt branch logic verified.
• comment-analyzer (scope: comment accuracy) — covered. The Step-6 header, the fold comment (linear.ts:656-663), and the mirror-back guard comment (linear.ts:795-798) accurately describe current behavior; the previously-stale N1 header is fixed.
• type-design-analyzer (scope: WebhookSecretAction discriminated union) — covered. preserve | mirror-stackwide | prompt is small, idiomatic, readonly, and makes preserve-without-a-secret unrepresentable. No concern.
• /security-review — covered inline (secrets-handling change): fail-closed analysis, no secret leakage into stdout/errors. None omitted for scope.

Human heuristics

• Proportionality — PASS. A pure decision function plus a fail-closed read-before-write is proportional to the bug; no over-abstraction.
• Coherence — PASS. The pre-read now mirrors the sibling isWebhookSecretConfigured error contract (fail closed + CliError with IAM hint); the earlier divergence is gone.
• Clarity — PASS. The stale header is rewritten; the empty catch is replaced with a fail-closed, actionable error.
• Appropriateness — PASS (was CONCERN). Tests now assert what the fix should do at all three seams — the pure decision, the fail-closed pre-read, AND the full setup-action PutSecretValue payload end-to-end. The integration test verifies against real SDK command classes, not only self-written mocks.

_{Re-review at head bc27dc4. Prior B1/N1/N2 verified resolved against the worktree; B2 (issue #611 AC#3) now closed by cli/test/commands/linear-setup-webhook.test.ts. RED CI = osv-scanner svgo@4.0.1 (transitive, on main; zero deps touched here) — scoped gitleaks range is clean. Approve; recommend a rebase once svgo is bumped on main.}