Skip to content

Resolve upstream merge conflict + comply with new pin-all-deps CI gate - #2

Merged
Uzair-Ahmed-Dahraj merged 11 commits into
add-shopify-on-cloudflare-templatefrom
fix/resolve-upstream-conflicts
Aug 20, 2026
Merged

Resolve upstream merge conflict + comply with new pin-all-deps CI gate#2
Uzair-Ahmed-Dahraj merged 11 commits into
add-shopify-on-cloudflare-templatefrom
fix/resolve-upstream-conflicts

Conversation

@superkindhq

@superkindhq superkindhq Bot commented Aug 18, 2026

Copy link
Copy Markdown

Resolves the merge conflict between add-shopify-on-cloudflare-template and cloudflare/templates:main, and brings the branch into compliance with two new CI gates added upstream after this branch was cut.

Why the conflict

Upstream main moved 9 commits ahead: cloudflare#1126 (tooling bumps → pnpm-lock.yaml + all templates.json hashes), cloudflare#1114 (syncpack "pin all deps" + check:deps), plus the E2E harness overhaul (root fixtures.ts, template-coverage.spec.ts). Conflicted files: pnpm-lock.yaml (46 hunks), templates.json (1 hunk).

What this does

1. Conflict resolution — took upstream's pnpm-lock.yaml/templates.json and regenerated with the repo's own tooling (pnpm install re-adds the shopify workspace entry; pnpm run fix:lockfiles regenerates the npm lockfile + package_json_hash).

2. New syncpack gate compliance (cloudflare#1114)

  • All shopify template deps exact-pinned (was ^ ranges — now a CI failure).
  • 5 versionGroups in .syncpackrc.json (the repo's sanctioned mechanism, same as remix's React 18 exemption), each forced by a hard constraint:
    • React ^18@shopify/app-bridge-react@4.1.5 + @shopify/polaris@10.45.0 peer-cap at React 18; React 19 = ERESOLVE
    • eslint 8.57.1eslint-config-airbnb@19 doesn't support eslint 9 (8.57.1 satisfies both airbnb and typescript-eslint ^8.57.0)
    • eslint-plugin-react-hooks 4.6.0 — airbnb peers ^4.3.0
    • hono 4.13.2 — high advisories ≤4.12.33 (GHSA-r38q-p5jb-mjvw JWT auth-bypass, GHSA-f9qg-mpr5-mjfw XSS)
    • react-router-dom 7.18.2 — high advisory ≤7.17.0 (turbo-stream RCE)
  • Cross-template alignment is mandatory (HighestSemverMismatch): shopify's @typescript-eslint@8.59.4/plugin-react-swc@4.3.1 pins require version bumps in remix, to-do-list, workflows, x402 templates.

3. Test fix — E2E backlink locator needed exact: true: the GitHub link text (github.com/devkindhq/…) substring-matches "Devkind" and broke Playwright strict mode.

Verification (all executed locally)

Gate Result
check:deps ✅ 450 valid
check:templates / check:lockfiles / check:prettier
turbo check + cf-typegen (build + deploy dry-run) ✅ 3/3
vitest ✅ 21/21
Playwright E2E (incl. new coverage spec) ✅ 5/5 + 1/1
npm audit ✅ 0 high / 0 critical (4 moderates: drizzle-kit chain, same as upstream baseline)

⚠️ For upstream maintainers (out of scope here)

The hono ≤4.12.33 and react-router ≤7.17.0 high advisories affect the 13 upstream templates still on hono 4.11.1 / react-router 7.9.6 — upstream CI doesn't run npm audit, so this ships unnoticed. Recommend a monorepo-wide bump as a separate PR.


Carries #1 (timing-safe webhook HMAC port of devkindhq/shopify-on-cloudflare#9) through the merge. Merge this → branch is conflict-free against cloudflare/templates:main and CI-compliant; upstream PR cloudflare#1075 updates automatically.

scottbuscemi and others added 11 commits August 3, 2026 18:16
…d Cloudflare Access (cloudflare#1100)

Co-authored-by: chythra-w1 <chythra-w1@users.noreply.github.com>
Co-authored-by: James Delorey <jdelorey@cloudflare.com>
…ith ctx.access (cloudflare#1125)

Co-authored-by: chythra-w1 <chythra-w1@users.noreply.github.com>
Co-authored-by: James Delorey <jdelorey@cloudflare.com>
…dflare-template

# Conflicts:
#	pnpm-lock.yaml
#	templates.json
…with new pin-all-deps CI gate

- Resolve pnpm-lock.yaml and templates.json conflicts against upstream main
- Comply with new syncpack 'pin all deps' rule (upstream cloudflare#1114): exact-pin all shopify template deps
- versionGroups for peer-dep-constrained stack (same mechanism as remix's React 18):
  * React ^18 (@shopify/app-bridge-react@4 + @shopify/polaris@10 peer-cap at 18)
  * eslint 8.57.1 + eslint-plugin-react-hooks 4.6.0 (eslint-config-airbnb is eslint-8-only)
  * hono 4.13.2 (high advisories <=4.12.33: JWT auth-bypass GHSA-r38q-p5jb-mjvw, XSS GHSA-f9qg-mpr5-mhfp)
  * react-router-dom 7.18.2 (high advisory <=7.17.0: turbo-stream RCE)
- Cross-template syncpack alignment (HighestSemverMismatch): bump @typescript-eslint/plugin-react-swc in remix, to-do-list, workflows, x402
- Fix E2E backlink locator: exact match (GitHub link text contains 'devkindhq', broke strict mode)

Gates: check:deps/templates/lockfiles/prettier OK; turbo check+cf-typegen 3/3; vitest 21/21; playwright 5/5; npm audit 0 high/0 critical

@superkindhq superkindhq Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

devkind-code-review: templates #2 — upstream merge + pin-all-deps compliance

Mode: comment-only · multi-agent (delegate analysis; every load-bearing claim re-executed by the parent at head 93f1b54, base add-shopify-on-cloudflare-template, merge-base ada0b83).** Diff too large for the API (HTTP 406, >20k lines) — reviewed via local clone, PR's own base used for all diffs.

Checks executed (real runs at head)

Check Result
pnpm install --frozen-lockfile (workspace) ✅ clean
turbo check (lint + syncpack + prettier) ✅ 16/16 tasks pass
pnpm run test (workspace) ✅ all template suites green
shopify template vitest ✅ 6 files / 21 tests pass (matches PR-body claim)
shopify template tsc --noEmit 6 errors — see finding 1
npm audit (shopify, prod) ✅ 0 vulnerabilities (matches claim; 4 moderate dev-only)
Secret scan (diff vs base) ✅ clean
templates.json fork delta vs upstream ✅ clean — 4 hash updates + shopify entry, no hand-edits
Coverage spec template-coverage.spec.ts dynamically enforces a spec per template; shopify has one

Findings

warning — shopify template fails tsc --noEmit with 6 errors after the react-router-dom 6→7 pin

web/App.tsx (Routes/Route/Link, lines 9–14) and web/bugsnag.tsx:27 — classic TS2786 signature-mismatch class. Root cause: this PR pins react-router-dom ^6.6.17.18.2 with @types/react ^18.0.0; RRD v7 ships its own types built against React 19 types, so the JSX component signatures clash with the React 18 @types in the template. CI never runs tsc (only pnpm -w check + tests + e2e), which is why this sailed through green. Either align types (RRD 7 + @types/react@19) or add a type-check task to the template's turbo.json check task so this class can't ship silently. Verified at the head worktree; base pinned RRD 6.6.1, so this is PR-introduced.

warning — PR body cites two advisories that don't exist

The hono justification (body line 17) cites GHSA-r38q-p5jb-mjvw ("JWT auth-bypass") and GHSA-f9qg-mpr5-mjfw ("XSS") — both return 404 from the OSV database (re-verified by the parent). The 6 advisories named in .syncpackrc.json labels are all real (200). The two dead IDs exist only in the PR description, so the narrative justification for the hono 4.13.2 pin is unverifiable — fix the body (correct IDs or drop the claim; the pin itself is defensible on real advisories affecting <=4.12.33).

Verdict

Merge mechanics are clean: fork-vs-upstream deltas are exactly the documented decisions (.syncpackrc version groups, shopify template addition, template hash regeneration), tests and audits genuinely pass, no secrets. The two warnings are documentation/type hygiene, not merge blockers — but fix the tsc gap before this becomes the base for future template work. Comment-only.

"hono": "4.13.2",
"react": "^18.0.0",
"react-dom": "^18.0.0",
"react-router-dom": "7.18.2"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

[warning] RRD 7.18.2 + @types/react 18 breaks tsc --noEmit (6 errors, web/App.tsx:9-14, web/bugsnag.tsx:27 — TS2786 signature clash; RRD v7 types assume React 19). CI doesn't run tsc, so nothing caught it. Align the types or add a type-check task to turbo.json.

@Uzair-Ahmed-Dahraj
Uzair-Ahmed-Dahraj merged commit 93f1b54 into add-shopify-on-cloudflare-template Aug 20, 2026
4 checks passed
@Uzair-Ahmed-Dahraj

Copy link
Copy Markdown

For the record: this auto-closed as merged when #3 landed, because #3 was branched on top of this one, so these commits reached the base branch with it. The upstream merge, the timing-safe HMAC fix, and the Playwright strict-mode fix are all in.

#3 changed one thing on top: how the new check:deps gate is satisfied. Our two mismatched dev tools are aligned down to what upstream already uses, rather than bumping the four other templates up. That takes the files touched outside our template from 12 to 4, with zero other templates modified.

On the two points raised here:

  1. Not forced by CI. With the four templates reverted and ours aligned, check:deps passes: pin all deps 450 valid, Default Version Group 424 already valid. The earlier test reverted the templates without aligning ours, which is a different scenario. fix:deps resolves upward, but that is an autofixer preference rather than a gate requirement.

  2. No security regression. Upstream main already sits where the security fix landed (wrangler 4.123.0 across 35 templates, @cloudflare/vite-plugin 1.52.1 across 11), so there is nothing to downgrade on the undici chain. Kept: wrangler 4.123.0, vite-plugin 1.52.1, vitest 4.1.10, hono 4.13.2, react-router-dom 7.18.2. Only @typescript-eslint/* and @vitejs/plugin-react-swc moved, and neither is in an advisory chain. npm audit: 0 high, 4 moderate (drizzle-kit -> esbuild only).

Separate issue worth picking up: the template's own npm run lint reports 426 formatting errors (no-tabs 206, indent 101, quotes 65). The monorepo prettier writes tabs and double quotes while the template's airbnb config wants spaces and single quotes. Pre-existing on both branches and invisible to CI, since turbo only runs check.

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.

4 participants