Skip to content

improvement(forking): make webhook url mapping clear - #6272

Merged
icecrasher321 merged 2 commits into
stagingfrom
staging-v24
Aug 5, 2026
Merged

improvement(forking): make webhook url mapping clear#6272
icecrasher321 merged 2 commits into
stagingfrom
staging-v24

Conversation

@icecrasher321

Copy link
Copy Markdown
Collaborator

Summary

Make Webhook URL mappings clearly mappable / regen-able.

Type of Change

  • Other: UX Improvement

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercelBot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedAug 5, 2026 12:41am

Request Review

@cursor

cursorBot commented Aug 5, 2026

Copy link
Copy Markdown

PR Summary

Cursor Bugbot is generating a summary for commit e290b7c. Configure here.

@greptile-apps

greptile-appsBot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR makes webhook URL behavior explicit during workspace synchronization and adds resource-level acknowledgment for references deleted from the source.

  • Preserves existing target webhook paths and allows compatible retiring paths to be assigned to arriving triggers.
  • Restricts automatic URL adoption to triggers with the same provider and reports URLs that will stop serving.
  • Presents deleted-reference drops at their true resource-wide scope and verifies them against source liveness on the server.
  • Extends fork API contracts, synchronization results, warnings, and tests for these behaviors.

Confidence Score: 5/5

The PR appears safe to merge.

The provider compatibility fix prevents cross-provider URL adoption, and the resource-level Drop UI now matches the server’s resource-scoped clearing behavior; no blocking failure remains.

Important Files Changed

FilenameOverview
apps/sim/ee/workspace-forking/lib/promote/trigger-urls.tsPlans and validates provider-compatible webhook path preservation, adoption, and retirement reporting.
apps/sim/ee/workspace-forking/lib/copy/deploy-bridge.tsLoads active public target webhook paths and their providers while excluding polling and internal endpoints.
apps/sim/ee/workspace-forking/lib/promote/promote.tsIntegrates transactional drop verification and webhook path resolution into fork promotion.
apps/sim/ee/workspace-forking/lib/promote/cleared-refs.tsVerifies deleted-resource acknowledgments and applies them only to matching source-deleted references.
apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.tsManages resource-level drop decisions and webhook adoption overrides for preview and promotion.
apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsxDisplays webhook URL mappings and accurately communicates the field-wide scope of resource drops.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
Preview[Build fork preview] --> Read[Read active target webhook paths]
Read --> Plan[Build trigger URL plan]
Plan --> Match{Same workflow and provider?}
Match -->|Yes| Choice[Preserve or adopt retiring path]
Match -->|No| Retire[Report retiring URL]
Choice --> Promote[Submit promotion choices]
Retire --> Promote
Promote --> Rebuild[Rebuild and validate plan in transaction]
Rebuild --> Copy[Copy workflows with resolved trigger paths]
Copy --> Deploy[Deploy target workflows]
Deploy --> Result[Report dropped references and retired URLs]
Loading

Reviews (2): Last reviewed commit: "fix(forking): honour drops before the un..." | Re-trigger Greptile

Comment threadapps/sim/ee/workspace-forking/lib/promote/trigger-urls.ts Outdated
Comment threadapps/sim/ee/workspace-forking/components/forks.tsx
Comment threadapps/sim/ee/workspace-forking/lib/promote/promote.ts
…n URL adoption
Review round 1 on #6272.
- Drop was inert for required references: `postCopyUnmappedRequired` gates before
the cleared-ref gate that honours acknowledgments, so a source-deleted reference
on a required field still failed with "map all required ... first". Verified drops
are now resolved once (`verifyForkDropAcknowledgments`) and subtracted from both
gates. Verification is not optional: an unmapped reference of a non-blocking kind
(credential, env-var) never re-blocks downstream, so subtracting raw
acknowledgments would let a crafted payload skip the required gate.
- URL adoption now requires provider equality. A count-only 1:1 pairing could hand a
GitHub URL to an arriving Slack trigger, keeping the endpoint alive while every
request failed signature verification - and reporting the URL as preserved.
- `resolveTriggerId` moved from `lib/webhooks/deploy.ts` to `@/triggers/webhook-url`
so the deploy path and the fork's provider check share one resolution.
- Trigger URL warnings render the full public URL in the heads-up section and name
the URL in the overwrite confirm, where identical workflow names were ambiguous.
- The Drop control renders once per resource and states how many fields it covers;
the remapper clears by reference, so a per-row control implied a choice the write
path cannot honour.
- Export clears `workflow-selector`: nothing on the import path remaps workflow ids
(`import-export.ts` re-creates each workflow under a fresh id), so a preserved
reference dangled - bundle or not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

@greptile

@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

@cursor review

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 57f5565. Configure here.

@icecrasher321
icecrasher321 merged commit 6cc25f6 into stagingAug 5, 2026
30 checks passed
@icecrasher321
icecrasher321 deleted the staging-v24 branch August 5, 2026 08:10
icecrasher321 added a commit that referenced this pull request Aug 5, 2026
…picks (#6290)
Two Cursor findings on #6272, both in the preview layer - the sync's write path
was correct in each case, but the UI stated an outcome that did not match it.
- The heads-up and overwrite confirm read `triggerUrlChanges` straight off the
diff, which the server computes with its DEFAULT resolution before the user
chooses anything. Selecting "Generate new URL" for a trigger that would have
adopted a URL therefore killed that URL with no warning, in the one modal whose
job is to state irreversible consequences (it also over-warned in the reverse
case). The diff now returns the RAW retiring set and the client subtracts the
live choices, so the rows, the heads-up and the confirm cannot disagree.
- The picker let two triggers select the same retiring URL and showed both as
keeping it. Two blocks cannot serve one path (`path_deployment_unique`) and the
resolver awards it to the first slot, so the loser silently got a new URL. A
path another row claimed is now disabled and named, and each row displays its
RESOLVED outcome rather than its raw pick.
The choice resolution is a pure module mirroring `resolveForkTriggerPaths`
(offered-paths guard, first-claim-wins), so the preview and the server agree by
construction rather than by two hand-kept implementations.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
waleedlatif1 added a commit that referenced this pull request Aug 6, 2026
Replaces the import-order guard from the previous commit with the structural fix.
Block configs spread `getTrigger('...').subBlocks` while their module body runs, so
`blocks/*` depends on `triggers/*` by design. Thirteen edges closed the loop back the
other way, which made module evaluation order load-bearing: enter the graph through
`@/triggers` and a block config calls `getTrigger()` before `TRIGGER_REGISTRY` is
initialized, throwing
ReferenceError: Cannot access 'TRIGGER_REGISTRY' before initialization
Eleven deployment routes crashed on import: `POST /api/workflows/[id]/deploy`, the v1
public and admin deploy/rollback/activate routes, both deployment-version routes, and
the three custom-tool deployment routes. All of them funnel through
`lib/webhooks/deploy.ts`, which stayed safe only because it imported a value from
`@/blocks` — biome sorts that above `@/triggers`, so the safe barrel always evaluated
first. #6272 deleted that import as unused cleanup and took the whole surface with it.
The reverse edges came from two places, both layering violations rather than anything
inherent to triggers:
- `triggers/index.ts` imported the mock-payload generator from `trigger-utils`, which
imports `@/blocks` for unrelated helpers. The generator is pure, so it moves to
`lib/workflows/triggers/mock-payload.ts` and both callers import it there.
- Eleven trigger modules statically imported the editor's Zustand stores to read
sub-block values inside `fetchOptions`/`fetchOptionById`. Those reads now go through
`triggers/editor-state.ts`, which loads the stores with a dynamic `import()` —
resolved when the resolver is called, not during module evaluation, so it carries no
initialization-order obligation.
Side effect: `@/triggers` drops from 744 statically reachable modules to 526. The block
registry, the workflow Zustand stores and their React Query graph are no longer pulled
into every server module that imports a trigger.
`scripts/check-trigger-block-cycle.ts` fails the build if a static edge returns, and
reports the shortest offending chain. The existing suite could not have caught this —
`deploy.test.ts` mocks both `@/blocks/registry` and `@/triggers`, and `vitest.setup.ts`
mocks `@/blocks/registry` globally, so it passed 18/18 against the broken code.
waleedlatif1 added a commit that referenced this pull request Aug 6, 2026
* fix(deployment): initialize block registry before triggers
* fix(triggers): break the triggers <-> blocks initialization cycle
Replaces the import-order guard from the previous commit with the structural fix.
Block configs spread `getTrigger('...').subBlocks` while their module body runs, so
`blocks/*` depends on `triggers/*` by design. Thirteen edges closed the loop back the
other way, which made module evaluation order load-bearing: enter the graph through
`@/triggers` and a block config calls `getTrigger()` before `TRIGGER_REGISTRY` is
initialized, throwing
ReferenceError: Cannot access 'TRIGGER_REGISTRY' before initialization
Eleven deployment routes crashed on import: `POST /api/workflows/[id]/deploy`, the v1
public and admin deploy/rollback/activate routes, both deployment-version routes, and
the three custom-tool deployment routes. All of them funnel through
`lib/webhooks/deploy.ts`, which stayed safe only because it imported a value from
`@/blocks` — biome sorts that above `@/triggers`, so the safe barrel always evaluated
first. #6272 deleted that import as unused cleanup and took the whole surface with it.
The reverse edges came from two places, both layering violations rather than anything
inherent to triggers:
- `triggers/index.ts` imported the mock-payload generator from `trigger-utils`, which
imports `@/blocks` for unrelated helpers. The generator is pure, so it moves to
`lib/workflows/triggers/mock-payload.ts` and both callers import it there.
- Eleven trigger modules statically imported the editor's Zustand stores to read
sub-block values inside `fetchOptions`/`fetchOptionById`. Those reads now go through
`triggers/editor-state.ts`, which loads the stores with a dynamic `import()` —
resolved when the resolver is called, not during module evaluation, so it carries no
initialization-order obligation.
Side effect: `@/triggers` drops from 744 statically reachable modules to 526. The block
registry, the workflow Zustand stores and their React Query graph are no longer pulled
into every server module that imports a trigger.
`scripts/check-trigger-block-cycle.ts` fails the build if a static edge returns, and
reports the shortest offending chain. The existing suite could not have caught this —
`deploy.test.ts` mocks both `@/blocks/registry` and `@/triggers`, and `vitest.setup.ts`
mocks `@/blocks/registry` globally, so it passed 18/18 against the broken code.
---------
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local>
Co-authored-by: Waleed Latif <walif6@gmail.com>
Sign up for freeto 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

@icecrasher321