Skip to content

fix: commit package-lock.json for setup-node npm cache - #5

Merged
adityash8 merged 4 commits into
mainfrom
fix/add-package-lock
Sep 9, 2026
Merged

adityash8 merged 4 commits into
mainfrom
fix/add-package-lock

Conversation

@adityash8

Copy link
Copy Markdown
Owner

Description

Commits package-lock.json, generated via npm install --package-lock-only against the current package.json. Fixes #4.

Why

.github/workflows/ci.yml sets cache: 'npm' on actions/setup-node@v4, but no lockfile was committed to the repo — the cache-key step fails before install/typecheck/lint/build/deploy ever run. This has been the case since the very first CI run (2025-09-28, run #18078790069) and again on the latest push (2026-06-16, commit febaae1, run #27598295781).

This matches the workflow's existing npm ci step, which also requires a lockfile to run correctly, and is one of the two options laid out in #4 (commit a lockfile vs. drop cache: 'npm') — chosen because it doesn't touch the protected CI config file itself.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Testing

  • I have tested this change locally
  • Verified locally: npm install --package-lock-only generates the lockfile without touching node_modules (gitignored), then npm ci against the generated lockfile succeeds (452 packages installed, no lockfile errors). No dependency versions changed — the lockfile just pins what package.json already specifies.
  • CI on this PR will be the first real end-to-end verification against actions/setup-node@v4's cache mode; watching the run after opening.

Checklist

  • My code follows the style guidelines of this project
  • My changes generate no new warnings (beyond pre-existing npm audit/deprecation warnings unrelated to this change)

Scope note

Deliberately narrow: only the lockfile is added. Does not touch ci.yml, does not bump any dependency versions, and does not address the pre-existing npm audit findings (1 critical, 3 high, 3 moderate) surfaced by npm ci — those are a separate, larger decision (see similar precedent in exit-zero#4) and out of scope for unblocking CI.

Status

Left as draft and labeled needs-owner per maintainer-loop policy — this repo is still in its break-in period (no auto-merge), and generating a fresh lockfile is a decision (npm vs. pnpm/yarn, willingness to pin current transitive versions) that should get an explicit owner look before merge, even though the mechanics are low-risk.

Ref #4.


Generated by the maintainer-orchestrator loop.


Generated by Claude Code

Fixes #4. actions/setup-node@v4 with cache: 'npm' requires a
committed lockfile to compute the cache key; none existed, so the
Setup Node.js step failed before install/typecheck/lint/build ever
ran. Generated via 'npm install --package-lock-only' against the
current package.json and verified 'npm ci' succeeds against it.

Does not change any dependency versions or touch ci.yml.
@gemini-code-assist

Copy link
Copy Markdown

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

Copy link
Copy Markdown
Owner Author

CI update: this PR's fix worked mechanically — npm ci now succeeds against the committed lockfile for the first time (452 packages installed cleanly). type-check then failed, but on a pre-existing bug unrelated to this PR's scope: unescaped quotes inside an inline SVG data-URI className in components/hero.tsx break JSX parsing. Confirmed pre-existing on main (not introduced here). Filed as #6 — that's the real blocker to CI going green, not this lockfile change. Leaving this PR as-is (draft, needs-owner); no action needed on this PR itself beyond the standard break-in-period review.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

CI verification result (run 28921885622):

Good news / bad news:

So: this PR does exactly what it claims (fixes the lockfile/cache root cause of #4) and CI is demonstrably further along than it has ever been, but the pipeline is not fully green — it's blocked on #7 now. Leaving this as a draft, needs-owner, not merging. Once #7 is resolved separately, this PR's fix should carry CI the rest of the way (assuming no other latent issues surface downstream — lint/build haven't run yet either).


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Correction: the hero.tsx JSX bug is tracked at #7, not #6 (#6 was a concurrent duplicate filed by a separate process during this same maintenance pass and has been closed in favor of #7). No other change to the analysis above.


Generated by Claude Code

The background-pattern div embedded an SVG data URI with raw double
quotes (width="60", viewBox="0 0 60 60", etc.) inside a double-quoted
className, prematurely terminating the attribute string. tsc --noEmit
reported 7 JSX syntax errors (TS2657/TS1003/TS1382/TS17002/...) and CI's
type-check step failed. Percent-encode the inner quotes (%22) and spaces
(%20) so the data URI is a single valid token — no behavior change to
the decorative pattern, purely fixes the parse.

Copy link
Copy Markdown
Owner Author

Nice — saw your fix(hero) commit landed and it worked: the #7 JSX parse error is gone. CI advanced further and hit one more pre-existing type-check issue, filed as #8: three pages call useState([])/useState(null) with no generic, so TS infers never[]/null and later setState(...)/property-access calls on that state fail to type-check (app/dashboard/page.tsx, app/search/page.tsx, app/services/[id]/page.tsx). Not touching your branch further since you're actively driving this one — just flagging what CI found next.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Babysit sweep update: the lockfile did unblock npm ci, and CI then failed on a genuine, pre-existing JSX syntax error in components/hero.tsx (an SVG data-URI with raw double quotes closing the className string early — 7 tsc errors). I pushed a fix for that (53731be, URL-encoding the quotes/spaces), which cleared those errors.

tsc --noEmit now gets further and surfaces a separate pre-existing class of type errors, unrelated to this PR's scope:

  • app/dashboard/page.tsx — useState([]) infers never[], so .id/.status/.service/.price/... don't exist (TS2345/TS2339)
  • app/search/page.tsx:76 — same useState([]) → never[]
  • app/services/[id]/page.tsx:59 — useState(null) typed as null, object assignment fails

These are all untyped useState initializers that need real domain types (Booking / Service shapes) rather than a mechanical one-liner — a typing decision, and these pages are payments-adjacent (booking status, pricing/currency). Leaving that for you rather than pushing any into payment-adjacent code. Once those useState calls are typed, type-check → build → deploy should proceed. Scope of this PR (lockfile + the hero.tsx parse fix) is otherwise complete.


Generated by Claude Code

npm ci now succeeds (lockfile committed in this PR), so the CI type-check
step runs and surfaces pre-existing tsc errors: useState([]) inferred
never[] and useState(null) inferred null in three page components, making
every property access on the state fail under --noEmit.

Add explicit interfaces matching the existing mock-data shapes (mirroring
the Service interfaces the child components already declare) and normalize
the dynamic route id to a string so it stays assignable to the child
component props. No runtime behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Td4LAqitAAacmG7iWjn7Fu

Copy link
Copy Markdown
Owner Author

Maintainer loop (babysit pass): committing the lockfile let npm ci succeed, which advanced CI to the Run type check step — that then failed on pre-existing tsc errors (useState([]) inferring never[], useState(null) inferring null) in app/dashboard, app/search, and app/services/[id]. Pushed 3be793c to type those hooks against the existing mock-data shapes (no runtime change); npm run type-check is now clean locally.

CI will still be red at the next step (Run linter) — but that's a separate, pre-existing issue I deliberately did not chase here:

  1. No committed ESLint config, so next lint drops into its interactive setup prompt and exits 1 in CI (same root cause as exit-zero#5, which handled the lint config as its own dedicated PR).
  2. Once a config is added, next lint surfaces ~9 real react/no-unescaped-entities errors across 7 files — one of which is app/auth/signin/page.tsx, an auth file outside this loop's autonomy boundary.

Leaving the ESLint config + entity cleanup for a separate owner-reviewed change rather than expanding this lockfile PR into auth-adjacent territory.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Update: the useState-typing fix for #8 landed and type-check now passes. next build then ran (first time ever) and failed on a fourth pre-existing issue, filed as #9 — lib/supabase.ts exports both the browser and server Supabase clients from one module, and a "use client" page (app/auth/signin/page.tsx) importing it pulls the server-only next/headers import into the client bundle, which Next's RSC boundary correctly rejects. Fix sketch is in #9 (split into client/server modules). This one touches the Supabase client wiring, so flagging rather than editing your branch further.


Generated by Claude Code

@adityash8
adityash8 marked this pull request as ready for review September 9, 2026 06:33
@adityash8
adityash8 merged commit 9d4af77 into main Sep 9, 2026
2 of 4 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-09T06:39:45.153726Z 01dcd00 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cursor cursor Bot 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.

Approved. Cursor Bugbot was not running on this PR after the first check poll, and no approval policy required human review. No reviewers were assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 01dcd00347

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread package-lock.json
Comment on lines +916 to +917
"engines": {
"node": ">=22.0.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Pin Supabase to a CI-compatible release

The lockfile pins @supabase/supabase-js and all of its subpackages to 2.110.1, which declares Node >=22, while .github/workflows/ci.yml runs only Node 18.x and 20.x. Consequently, both CI jobs install an unsupported runtime combination, and any environment enforcing package engines will fail during npm ci before type-checking or building; pin a Node-18/20-compatible Supabase release or update the CI/runtime matrix to Node 22.

Useful? React with 👍 / 👎.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI red on main since PR #1 (2026-06-16): setup-node npm cache fails — no lockfile committed

2 participants