Skip to content

fix(infra): settle stuck observability destination updates without the API - #1482

Merged
Makisuo merged 1 commit into
mainfrom
fix/alchemy-observability-resume-update
Oct 15, 2026
Merged

Makisuo merged 1 commit into
mainfrom
fix/alchemy-observability-resume-update

Conversation

@Makisuo

@Makisuo Makisuo commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Why prd deploys still fail after #1479

The prd deploy still fails after #1479 (run 38657858388). Both observability destinations plan update, the update gets a 403, and the api Worker is not updated.

#1479 patched diff, but the plan never consults it here. The first beta.83 deploy failed partway through, which left both rows in state at status: "updating". For an updating row, Plan.ts resumes the update whether diff returns noop or update. The resumed reconcile lists the account's destinations, which the deploy token is not allowed to do, so it fails with a 403 on every run.

Fix

The alchemy patch now also makes reconcile return the stored attributes when the last-applied url, headers and enabled match the desired ones. No API call is made, and destinationConf is re-wrapped as Redacted. The stuck rows then commit as updated, and later deploys diff to noop. If any of those values changes, for example a rotated ingest key, reconcile still observes and PATCHes as before. That path still needs list access for the token.

Verification

  • Reinstalled with the bun cache cleared. Both hunks are present in the installed lib and src.
  • Called the patched reconcile directly with a stub Cloudflare environment:
    • unchanged props return Success with a Redacted destinationConf, and no API call is made
    • a Redacted header with the same value also returns Success
    • a changed header goes on to the API (it fails in the stub because there are no credentials)
  • The real check is the next prd deploy after merge.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

…e API

The first beta.83 prd deploy left workers-observability-{logs,traces} in
state status "updating" after its 403. Alchemy resumes an interrupted
update whatever the diff returns, so #1479's diff patch never applied and
every prd deploy still reconciled, listed destinations, got a 403, and
skipped the api Worker.

reconcile now returns the stored attributes, with destinationConf
wrapped as Redacted, when the last-applied url, headers and enabled flag
match the desired ones. Those rows commit as "updated" without any API
call and later deploys diff to noop. A real change still observes and
PATCHes as before.
@maple-review-bot

maple-review-bot Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
Contained to a deploy-time patch; correctness rests on alchemy resuming updating rows through reconcile, whose plan code is not in this repo.
quality 100/100 · no findings · tests not needed · risk medium

Extends the alchemy beta.83 patch so a resumed destination update whose stored url, headers and enabled already match the desired props returns the stored attributes instead of listing the account (the 403). Only the two patched library files change, so merging is safe.

  • reconcile now takes olds and short-circuits when the destination is already desired
  • New sameDesired helper reuses sameHeaders for url, headers and enabled

Before merge

  • Manual · Watch the next prd deploy: both destinations should settle as updated and later deploys should diff to noop
What was checked
  • Recounted all 8 hunk headers against their bodies in patches/alchemy@2.0.0-beta.83.patch; counts and ascending order hold, so the patch still applies
  • Alchemy does pass olds to reconcile: this file's read already takes it, and the repo's own providers destructure it (packages/alchemy-maple/src/AlertDestination.ts:153)
  • Only two hunks are added to the patch body; the rest is byte-identical to the base (git diff 30a99abac 7f653b79d -- patches/...), and the 7-char index lines match the other patches here

7f653b7 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ad17928c-cb4d-43f1-8bad-0452c5f415bf
📥 Commits

Reviewing files that changed from the base of the PR and between 30a99ab and 7f653b7.

📒 Files selected for processing: 1
  • patches/alchemy@2.0.0-beta.83.patch
+-------------------------+
| Loose code sinks ships. |
+-------------------------+
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Makisuo
Makisuo merged commit ee5452a into main Oct 15, 2026
35 of 36 checks passed
@Makisuo
Makisuo deleted the fix/alchemy-observability-resume-update branch October 15, 2026 10:41
@maple-review-bot

Copy link
Copy Markdown

Merged with 1 step from "Before merge" still open. If it is done, tick it on the review comment; if not, now is the time.

  • Manual · Watch the next prd deploy: both destinations should settle as updated and later deploys should diff to noop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant