Resolve upstream merge conflict + comply with new pin-all-deps CI gate - #2
Conversation
…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
There was a problem hiding this comment.
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.1 → 7.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" |
There was a problem hiding this comment.
[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.
93f1b54
into
add-shopify-on-cloudflare-template
|
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 On the two points raised here:
Separate issue worth picking up: the template's own |
Resolves the merge conflict between
add-shopify-on-cloudflare-templateandcloudflare/templates:main, and brings the branch into compliance with two new CI gates added upstream after this branch was cut.Why the conflict
Upstream
mainmoved 9 commits ahead: cloudflare#1126 (tooling bumps →pnpm-lock.yaml+ alltemplates.jsonhashes), cloudflare#1114 (syncpack "pin all deps" +check:deps), plus the E2E harness overhaul (rootfixtures.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.jsonand regenerated with the repo's own tooling (pnpm installre-adds the shopify workspace entry;pnpm run fix:lockfilesregenerates the npm lockfile +package_json_hash).2. New syncpack gate compliance (cloudflare#1114)
^ranges — now a CI failure)..syncpackrc.json(the repo's sanctioned mechanism, same as remix's React 18 exemption), each forced by a hard constraint:@shopify/app-bridge-react@4.1.5+@shopify/polaris@10.45.0peer-cap at React 18; React 19 = ERESOLVEeslint-config-airbnb@19doesn't support eslint 9 (8.57.1 satisfies both airbnb andtypescript-eslint ^8.57.0)^4.3.0HighestSemverMismatch): shopify's@typescript-eslint@8.59.4/plugin-react-swc@4.3.1pins 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)
check:depscheck:templates/check:lockfiles/check:prettiercheck+cf-typegen(build + deploy dry-run)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:mainand CI-compliant; upstream PR cloudflare#1075 updates automatically.