Add lockfiles and a minimal hermetic CI workflow - #106
Merged
Merged
Conversation
Commits package-lock.json for back-end/front-end/e2e (previously gitignored, which is exactly what let the backend's TypeScript version-skew bug slip through in the first place — npm ci now pins reproducible installs). CI runs build + unit tests only, deliberately excluding anything that needs live Azure blob storage, a real Postgres, or a browser: - back-end: npm ci --ignore-scripts (azure-functions-core-tools' postinstall otherwise pulls a ~1.5GB runtime binary we never invoke in CI) -> tsc build -> vitest unit tests (pure functions only) - front-end: npm ci -> nuxt build Verified nuxt build passes (never run in this repo before, and not implied by `nuxt dev` working) and that both lockfiles are in sync with their package.json via a clean `npm ci` for each package. The black-box API suite and Playwright E2E suite stay local-only for now (packages/back-end/test/api.test.ts, packages/e2e) since they need a running backend against live Azure data plus, for E2E, a downloaded browser — out of scope for "minimal CI" per request. Stacked on add-regression-test-suite: this branch's CI references `npm run test:unit`, which only exists there, not yet on main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
NimeshaKahingala
requested review from
RobertLRead
and
a lite review from Copilot
September 13, 2026 06:27
There was a problem hiding this comment.
🟡 Changes recommended
Update the stale testing and cleanup documentation to reflect the new CI coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds reproducible npm lockfiles and minimal hermetic CI for backend and frontend validation.
Changes:
- Adds lockfiles for backend, frontend, and E2E packages.
- Adds backend build/unit-test and frontend build CI jobs.
- Updates
.gitignoreto retain the lockfiles.
File summaries
| File | Description |
|---|---|
packages/e2e/package-lock.json |
Pins E2E dependencies. |
packages/back-end/package-lock.json |
Locks backend dependencies. |
.gitignore |
Allows selected lockfiles to be committed. |
.github/workflows/ci.yml |
Adds backend and frontend CI jobs. |
Review details
Files not reviewed (1)
- packages/e2e/package-lock.json: Generated file
- Files reviewed: 1/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+24
to
+26
| - run: npm ci --ignore-scripts | ||
| - run: npm run build | ||
| - run: npm run test:unit |
NimeshaKahingala
added a commit
that referenced
this pull request
Sep 13, 2026
…view points
Major correction: GET /getRelatedOKH does NOT ignore the keywords query
param. The earlier claim ("no {keywords} route template means
request.params.keywords must be undefined") was wrong — this Azure
Functions host's RPC binding data flattens query-string values into
request.params even without a matching route segment, empirically
verified (?keywords=<real keyword> matches; ?keywords=<sentinel>
doesn't; case-insensitive). The "always empty" observation earlier
was real but coincidental — every keyword tried didn't happen to
exact-match any real file. Corrected in api.test.ts (now a genuine
positive test of the working filter, using a real discovered keyword
vs a random sentinel), CLEANUP_PLAN.md, TESTING.md, and AGENT.md.
Issue #107 closed with the retraction.
Also from the second Copilot review pass on this PR:
- Real bug: the supply-tree E2E test's 60s assertion timeout was
capped by Playwright's default 30s *test*-level timeout, making it
a no-op — exactly what caused the intermittent failure observed
earlier. Fixed with test.setTimeout(90_000).
- page.waitForRequest (resolves when a request is *created*) swapped
for waitForEvent('requestfailed') (resolves when it actually
fails), so later assertions don't run against a still-in-flight
rejection.
- "app header renders on every page visited above" only visited "/".
Now actually visits both the home page and a product detail page.
- Stale docs fixed: AGENT.md and CLEANUP_PLAN.md described the repo
as having no lockfiles/no CI, contradicting their own Phase 0
checkmarks after #106 merged in. ci.yml gained a header comment
making its intentionally-partial scope explicit (build+unit only,
not the two regression suites).
- Not changed: the review's claim that the unit suite has 12 tests
(not 13) — verified via `vitest run --reporter=verbose`, it's
genuinely 13. The original 20/20 total was already correct.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
package-lock.jsonforback-end/front-end/e2e(previously gitignored — this is exactly what let the backend's TypeScript version-skew bug slip through undetected)..github/workflows/ci.ymlwith two jobs, deliberately kept hermetic (no live Azure blob storage, no Postgres, no browser download):npm ci --ignore-scripts→npm run build→npm run test:unit(--ignore-scriptsskipsazure-functions-core-tools' postinstall, which otherwise pulls a ~1.5GB runtime binary CI never needs)npm ci→npm run build(nuxt buildhad never been run in this repo before — verified locally that it passes)Not included (by design, per "keep CI minimal")
packages/back-end/test/api.test.ts(black-box API tests) andpackages/e2e(Playwright) both depend on live Azure blob data and/or a downloaded browser — they stay local-only pre-merge checks for now, documented indev-docs/TESTING.md.Why this is stacked on
add-regression-test-suiteCI's
npm run test:unitstep only exists on that branch, not yet onmain. This PR should merge after (or be retargeted tomainonce) #105 merges.Test plan
nuxt buildpasses locally (previously unverified in this repo)package.jsonvianpm ci--ignore-scriptsdrops the installedazure-functions-core-toolsfootprint from 1.5GB to 10MB🤖 Generated with Claude Code