Skip to content

fix(commerce-onboarding): don't skip site re-claim on a malformed siteUrl param#4483

Merged
pedrofrxncx merged 1 commit into
mainfrom
fix/commerce-onboarding-malformed-site-url-reclaim
Jul 13, 2026
Merged

fix(commerce-onboarding): don't skip site re-claim on a malformed siteUrl param#4483
pedrofrxncx merged 1 commit into
mainfrom
fix/commerce-onboarding-malformed-site-url-reclaim

Conversation

@pedrofrxncx

@pedrofrxncx pedrofrxncx commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Source

Bug found while reading the most recently merged commit on main (#4439, "GitHub companion repo search + re-claim site on report open"), which added a check to re-claim the Commerce Discovery connection when the requested site differs from the one it's currently claimed for.

The bug

normalizeCommerceSiteUrl returns { ok: false } both for an empty input and for a non-empty but malformed one. The new check used !requestedNormalized.ok to mean "no site was requested, trust the existing claim" — but that condition is also true for a malformed, non-empty site param, which was never the intent.

Failure scenario

  • Org A previously onboarded for https://a.com (connection's claimed site = a.com).
  • Org A opens the onboarding page with a garbled ?siteUrl= param (e.g. a corrupted/edited link) for a different site.
  • claimedForRequestedSite evaluates to true regardless of which site is actually claimed, so setupReady becomes true.
  • The re-claim (triggerInitialSetup) never fires, and openReport skips the COMMERCE_DISCOVERY_RUN trigger entirely (since the malformed URL fails normalizeCommerceSiteUrl again there) — it just opens whatever report the connection happens to already point at, instead of surfacing the invalid-URL error the rest of the component already handles for this exact case (see the initialSiteUrl normalization branch further down).

This silently defeats the wrong-store-diagnostic fix #4439 just added.

Fix

Extracted the check into isConnectionClaimedForSite(requestedSite, claimedSiteUrl) in site-url.ts, which explicitly distinguishes "no site requested" (empty string) from "invalid site requested" (non-empty, fails normalization) — only the former bypasses the match check.

Test

Added isConnectionClaimedForSite unit tests in site-url.test.ts, including a regression test for the malformed-non-empty case that fails on the old inlined logic and passes with the fix.

Verify

bun test apps/mesh/src/commerce-discovery/site-url.test.ts

Checks run locally

  • bun run fmt
  • bunx tsc --noEmit in apps/mesh (pre-existing unrelated errors confirmed present on main before this change too; no new ones introduced)
  • bun test apps/mesh/src/commerce-discovery/site-url.test.ts — 17 pass

Full CI will run the broader suite/lint.


Summary by cubic

Fixes Commerce onboarding so a malformed non-empty siteUrl no longer bypasses site re-claim and skip COMMERCE_DISCOVERY_RUN. Empty siteUrl is treated as “no site requested,” while invalid non-empty values now trigger a re-claim or surface the existing invalid-URL error.

  • Bug Fixes
    • Added isConnectionClaimedForSite() to distinguish empty vs invalid siteUrl and compare normalized origins.
    • Updated commerce-onboarding.tsx to use this helper instead of inline checks.
    • Added unit tests, including a regression test for the malformed non-empty case.

Written for commit 18a428c. Summary will update on new commits.

Review in cubic

…eUrl param

The site-match check added in #4439 (re-claim the connection when the
requested site differs from the one it's currently claimed for) bypassed
itself whenever normalizeCommerceSiteUrl failed — which happens both for an
empty requestedSite (intentional: no site requested, trust the existing
claim) and for a non-empty but malformed one (unintentional). A garbled
?siteUrl query param therefore skipped re-claiming and skipped the
COMMERCE_DISCOVERY_RUN trigger, opening whatever site the connection
happened to already be claimed for instead of surfacing the invalid-URL
error the rest of the component already handles.

Extracted the check into isConnectionClaimedForSite so "no site requested"
and "invalid site requested" are distinguished explicitly, and added a
regression test for the malformed-but-non-empty case.
@pedrofrxncx
pedrofrxncx enabled auto-merge (squash) July 13, 2026 13:24
@pedrofrxncx
pedrofrxncx merged commit b404f89 into main Jul 13, 2026
20 of 21 checks passed
@pedrofrxncx
pedrofrxncx deleted the fix/commerce-onboarding-malformed-site-url-reclaim branch July 13, 2026 13:35
decocms Bot pushed a commit that referenced this pull request Jul 13, 2026
PR: #4483 fix(commerce-onboarding): don't skip site re-claim on a malformed siteUrl param
Bump type: patch

- decocms (apps/mesh/package.json): 4.11.14 -> 4.11.15

Deploy-Scope: both
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