The quality gate has been grading the entire codebase rather than what changed. The server's new-code period is PREVIOUS_VERSION, but sonar.projectVersion was never set anywhere — not here, not in the workflow — so every analysis recorded "not provided" and there was no previous version to diff against. SonarQube's fallback is to treat everything as new.
The tell was visible in the measures all along: new_lines read 9460 against a total ncloc of 5496, and new_coverage tracked overall coverage to within two points. Both are what you would expect if "new code" meant "all code", and neither is surprising enough to notice unless you go looking. This is the fourth time the project has hit the same shape — a tool reporting a plausible number for something other than what was asked.
Verified with a scan against the scratch key rather than by reasoning about the config. The analysis now records version 1.0.0, ncloc holds at 5557 so nothing was silently dropped, and new_lines falls from the whole codebase to 151 — the window is now the diff.
One consequence worth expecting rather than discovering: a narrow window makes new_coverage volatile. The verification scan reported 0.0% on two lines to cover, because two uncovered lines is all it takes. The number will settle as commits accumulate and the window widens, but the gate will be jumpy for the first few merges, and it will not simply turn green on its own.
Left at 1.0.0 to match both package.json files. Nothing enforces that they stay in step, so the comment says to move all three together.
Refs #79
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The documented convention allowed dropping the issue number when no issue existed. The clause looks harmless — it was meant for trivial work — but in practice it turns "there is no issue yet" into "there is no issue ever", and two branches went that way this week: the app icon, merged as #70, and the header mark, which had to be filed retroactively as #76. Both were legitimate work that the tracker never heard about, and the convention is what permitted it.
The cost is not bookkeeping. The issue is where the reasoning, the rejected alternatives, and the verification end up — the record that outlives the conversation that produced it. A branch with nothing behind it leaves that nowhere but a commit message.
Everything else about the convention is unchanged: the type prefixes, the (#N) subject, the Closes #N body line, and the point that Gitea builds the link from the commit rather than the branch name.
Closes#77
Puts the monogram to the left of "Redefined Designs" in the site header.
Inline SVG rather than an <img src="/favicon.svg"> for two reasons. The tile is drawn in currentColor so it follows the antd text token and inverts with the app's own theme switch — the favicon file inverts on prefers-color-scheme, which tracks the operating system and would disagree with the app whenever someone toggles the theme themselves. And the letters are knocked out with a mask rather than painted in the background colour, so the mark sits correctly on any surface instead of carrying a backdrop that only matches this header.
The mask id is generated per instance, since two marks on one page would otherwise share an id and the second would reference the first. It is stripped to alphanumerics because React's generated ids contain colons.
Marked aria-hidden: the wordmark beside it already says the name, so announcing it again would be noise.
Verified by screenshotting the header in both themes rather than by reasoning about the colours.
Two standing documents rather than one, because they are different kinds of thing: one is a contract the pipeline must keep, the other is work not yet done.
The coverage contract names the seven requirements that keep SonarQube's number real, and what specifically breaks if each lapses. It exists because coverage does not fail loudly — it reports a smaller number, which looks exactly like tests covering less. That is the third time this project has met a tool that succeeds while measuring nothing, after #67 and #60, so the failure mode is written down alongside how to check the guard still fires.
The identity document covers CI authenticating to SonarQube as admin rather than a restricted account, raised as a "Related" note in #61 and split out so a permissions change is not buried in a CI-config commit. It spells out the revoke step explicitly, since the workflow goes green one step earlier and stopping there leaves the old credential valid.
SonarQube reported 0% coverage for 78 unit, 134 integration and 83 end-to-end tests, so the coverage-on-new-code gate — the most useful thing SonarQube offers a project this size — has been failing permanently while looking configured. It now reports 69.6%, verified by a real scan.
Backend coverage comes from both suites, written to separate directories because jest writes coverage/lcov.info by default and the second run would silently overwrite the first. Both are needed rather than just the fast one: the unit suite alone reports 11%, because everything in src/routes is exercised by the integration suite. That suite is manual-only after hanging for 3h12m post-run, so it runs here with --forceExit and the job carries a hard timeout; jest confirmed during testing that it would otherwise have hung.
The frontend had no unit tests at all, so its coverage comes from Playwright driving an istanbul-instrumented dev server, collected per test by an auto-fixture and merged with nyc. The 17 specs now import from a local fixtures module that re-exports @playwright/test, which is what lets the fixture attach without touching each test body.
Instrumentation is gated behind COVERAGE=true and loaded by dynamic import, since vite-plugin-istanbul is ESM-only while vite.config.ts evaluates as CommonJS. Both directions were checked rather than assumed: a normal build contains no instrumentation, and the dev server instruments nested modules as well as top-level ones — the first attempt used an include glob of src/* which would have silently missed everything under src/admin and src/cart.
coverage:report fails when nothing was collected instead of writing an empty report, and that guard was fired deliberately to confirm it works. This project has been bitten twice by tools succeeding while measuring nothing — SonarQube skipping the whole frontend and still exiting EXECUTION SUCCESS in #67, and an ESLint matcher silently matching no files during #60 — and coverage has exactly that shape: an uninstrumented dev server lets every test pass while gathering nothing, and the 0% that follows reads as lost coverage rather than broken collection.
Worth knowing when reading the numbers: end-to-end coverage flatters. Istanbul marks a line covered when the browser ran it, so a component rendered during a test counts as covered with nothing asserting anything about it. Recorded in the design doc and the project context rather than left to be discovered.
Also declares sonar.tests so test files are analysed under the test rule set rather than as production code.
Closes#61
Records what #61 still needs after #67 delivered two of its four asks, and the two constraints that shape the rest: backend route logic is covered only by the integration suite, which is manual-only because of a post-run hang, and the frontend has no unit tests at all so its coverage has to come from instrumenting the app and collecting from Playwright.
Also records the thing most likely to mislead later — end-to-end coverage marks a line covered when the browser merely ran it, so the frontend number will read considerably better than the testing behind it, and the 80% gate will be easier to clear on frontend changes than backend ones. Accepted deliberately, because the alternative leaves every frontend pull request failing a gate it cannot satisfy.
Refs #61
The app had no favicon at all, so every tab showed the browser's default globe. Adds an RD monogram as frontend/public/favicon.svg plus a 180x180 apple-touch-icon.png, and links both from index.html.
The mark is the letters knocked out of a filled tile rather than drawn on their own. At 16px — which is the only size that really decides a favicon — a solid shape with a bright interior stays findable in a strip of twenty tabs, where bare strokes turn to grey fuzz. Every candidate was rendered at 16, 32, 64 and 128 in headless Chromium and looked at rather than trusted from the path data, which is what caught the two drafts that did not survive: one had the D's bowl extending past the viewBox and clipping flat, because a stroked arc reaches half the stroke width beyond its centreline; another shared a stem between a mirrored R and the D and read as a Cyrillic Я.
Colours are the accent already defined in src/main.tsx, and the SVG inverts with the theme for the same reason that accent does. Chrome ignores media queries inside a favicon and keeps the light rendering; that is acceptable here rather than worked around, because the pale letters still carry the mark against a dark tab strip. The Apple icon is full-bleed with no inner corner radius, since iOS applies its own mask and would otherwise show a small rounded tile floating inside a larger rounded tile.
Also sets theme-color for both schemes so the browser chrome on Android and in an installed PWA matches the app rather than fighting it.
Verified: build clean, lint clean, and both files land in dist/, which the Dockerfile copies wholesale into the image's public directory.
SonarQube was analysing none of frontend/src. All 34 files were indexed and reported ncloc 0, so the quality gate had been measuring roughly a third of the codebase while appearing to cover it. The scanner log gives the cause: frontend/tsconfig.json sets "moduleResolution": "bundler", which is correct for Vite, and the TypeScript bundled with SonarQube 9.9 predates 5.0 and rejects it. Building the frontend program throws, every frontend file is dropped, and the scan still exits EXECUTION SUCCESS — which is why a green job hid this indefinitely.
Adds frontend/tsconfig.sonar.json, an analysis-only mirror that differs only in using "node", and points the scan at it via sonar.typescript.tsconfigPaths. The app's own tsconfig is deliberately untouched: "bundler" is right for the build, and changing it to satisfy an old analyser would let the tool dictate the build. The mirror cannot use `extends` — the old compiler validates the base file while reading it, so the error just moves to pointing at tsconfig.json.
Verified locally against a scratch project: 59/59 files analysed, no skips. Analysed lines go from 2,069 to 5,481, code smells from 2 to 14, security hotspots from 3 to 4, and technical debt from 21 to 85 minutes. The frontend had been hiding twelve code smells and a hotspot, which is part of why the React problems behind #60 and #62 had to be found by hand.
A standalone copy drifts, and drift here does not fail anything — it silently returns to skipping the frontend while reporting success. scripts/check-sonar-tsconfig.js compares the two and fails when they diverge in anything but moduleResolution, and runs before the scan so the scan is never what discovers it. Confirmed it catches drift by introducing some.
Moves scan settings into sonar-project.properties at the repo root so a local scan and the CI scan analyse the same thing, leaving only the host and token in secrets. Adds scripts/scan-local.sh, which runs the scanner in Docker because it needs Java 11+ and the dev machine has Java 8, and which defaults to a scratch project key: the server is Community edition with no branch analysis, so any scan overwrites the single main analysis of whichever key it is given.
Closes#67
TypeScript's strict mode checks types and nothing else, so nothing enforced the React hook rules, the SonarJS rules, or unhandled-promise detection. Adds a flat config per workspace, a lint script in each, and a lint job in tests.yml.
The rule selection is the substance of this change and is measured rather than guessed. A full-strength config reports 435 violations across 50 files, but 325 of those are the no-unsafe-* family from recommendedTypeChecked, every one downstream of pool.query() returning any rows and untyped fetch responses. Typing those boundaries is the whole of #65, so enabling the rules here would ship a linter whose output is three-quarters another issue's backlog — the reliable way to teach everyone to ignore lint output. This enables recommended plus the two type-aware rules that catch defects rather than describe type debt, which leaves 110 findings.
Both configs downgrade every preset to a warning and then list the error rules explicitly at the bottom, so the CI gate is readable in one place instead of inferred from four presets' defaults. Errors are no-floating-promises, no-misused-promises, rules-of-hooks, exhaustive-deps and jsx-a11y/alt-text; everything else warns. No --max-warnings flag is needed because ESLint already exits non-zero on errors and zero on warnings. no-misused-promises runs with checksVoidReturn.attributes false, since onClick={async () => ...} is idiomatic React and safe when the handler catches its own errors — at the default it flags every antd button in the admin screens, 25 of its 28 hits, and a rule that is 89% noise gets switched off within a week.
The 37 errors this surfaced were mostly not the mechanical fix they looked like. The plan assumed the 30 floating promises were fire-and-forget loaders that already handled their own failures, which was true of the one sampled when writing the design and false for most of the rest: Admin, Categories, Customers, Tags, Settings, Account and CustomerAuthContext all had no rejection handling at all, so `void` on them would have hidden real failures rather than annotated deliberate ones. Each of those loaders now catches and surfaces the failure before the call site voids it. The CustomerAuthContext one was a live bug — a rejected fetchMe left loading true forever, rendering as a permanent spinner instead of a signed-out page.
Admin's load became a useCallback so its effect can name it honestly rather than suppress the dependency, Categories' drop handler was split so the function antd receives returns void as its type says, and Cart's effect now names refreshCartContext, which is a useCallback with an empty dependency list and so cannot re-run it. The only disable added is in asyncRoute, where returning a promise where Express expects void is the entire point of the wrapper and the promise cannot reject.
Two of the issue's premises did not survive measurement, both recorded in the spec: exhaustive-deps flags 2 cases rather than the 10 inferred from empty dependency arrays, and the backend was already clean on the defect rules because #59 wrapped every async route.
Verified: lint, build, 78 unit, 134 integration and 83 e2e all pass in both workspaces, and the CI gate was confirmed to fail by introducing a deliberate violation rather than by assuming the job is wired correctly.
Closes#60
Hard-wrapped at 100 columns, which assumes a viewer width neither Gitea's web UI nor the VS Code markdown preview has, so the wrap points landed mid-sentence for the person reading it. Paragraphs and list items are now one line each; tables, code fences and the header block keep their own breaks because those are structure rather than wrapped prose.
Refs #60
Records the measurement the design rests on — 435 violations from a full-strength config, of which 325 are the no-unsafe-* family — and the decision not to enable recommendedTypeChecked, since those 325 all trace to untyped pool.query rows and fetch responses, which is the whole of #65. Also records two of the issue's premises that measurement contradicts: exhaustive-deps flags 2 rather than the 10 the issue inferred from empty dependency arrays, and the backend is already clean on the defect rules because #59 wrapped every async route.
Refs #60
The note said the older route files were still unwrapped, which stopped being true with #59. Records where the guarantee now reaches — including the second `webhookRouter` and the globally-mounted `attachCustomer`, both of which an audit grepping for `router.` misses — and points at the test that enforces it, so the next person adding a route learns it from a failing build rather than from this file.
Express 4 does not forward a rejected promise from an async handler, so an unwrapped async route never responds at all — the request hangs until the client gives up, nothing reaches the error middleware, and monitoring sees an open connection rather than a 500. That silence is the shape of the 2026-08-17 incident, where an unhandled rejection left every item query hanging and the storefront rendered it as an empty shop. `asyncRoute` was written in response, but it was only applied to some routes: 30 handlers added afterwards were still bare, including register, login, the whole cart, and PayPal checkout.
Wraps all 30, plus two the issue's inventory missed. `attachCustomer` is a bare async middleware mounted globally in app.ts, so a rejection in its session lookup would hang every request in the application — including the 25 handlers that were already wrapped correctly, which meant the guarantee did not actually hold anywhere. The PayPal webhook registers on a second router named `webhookRouter`, so an audit grepping for `router.` walked straight past it.
Adds a unit test that scans the route sources and fails on any registration whose handler is not wrapped. A convention already half-forgotten once will be forgotten again, and enforcement is what the issue asked for; ESLint would be the better home for it but there is no ESLint in this repo yet (#60). The test walks parens rather than lines, so it also catches a handler whose `async` sits on its own line, and it matches any `*Router` name rather than just `router` — the two ways the existing bare handlers escaped notice. It is deleted along with `asyncRoute` if the project moves to Express 5, which forwards rejections natively.
No behaviour changes on the success path; the failure path turns a hung request into a logged 500.
Closes#59
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Notes that sign-in now has one implementation shared by the auth routes and the cart prompt, and where to change it. Records the marketing consent wording as a cross-boundary duplication in the same class as TAG_COLORS, but with a sharper failure mode: the server stores its copy verbatim so the record says what the customer saw, and a drifted label defeats that silently rather than loudly. An end-to-end test now spans the two sides.
Also records that switching the active Node version for a test run is fine provided it is switched back to 18.16.1 afterwards, and expands the modal-route entry to the full route list now that four auth routes use it.
/login, /register and /forgot-password rendered bare cards with no site header. They linked to each other and nowhere else, so a customer who clicked Log in from the storefront and changed their mind had no way back except the browser's back button. All four auth routes are now modals over the page the customer was already on, reusing the backdrop-location arrangement from #51: a direct visit or a link from an email opens over the storefront, so closing always lands somewhere real. They remain real routes, because /reset-password links to /login and customers may have bookmarks.
Signing in or registering now returns the customer to the page behind, signed in, rather than moving them to /account. Someone who signs in while browsing wants to carry on browsing, and this is already how the cart and favorites prompts behave when they resume an interrupted action.
The larger half of this is removing the duplication. Signing in existed twice — as these routes and again inside the prompt shown when a signed-out visitor adds to the cart or favorites something — and the two had already drifted. There were three different wordings of the marketing consent in circulation: the register page's, a shorter one in the prompt, and the string the server actually stores. The server keeps that text verbatim so the consent record says what the customer saw, which none of the three did. Both callers now render one shared AuthForm whose checkbox is the exact string the server records, and a test asserts that wording so it cannot drift again silently.
Steps within the auth flow replace rather than push, so switching between tabs or stepping to password recovery leaves the whole detour as a single history entry and closing returns to where it started instead of walking back through every tab that was looked at.
The privacy policy link opens in a new tab: following it in place would discard a part-filled signup form, and /privacy still has no way back of its own until #52.
Test changes follow from the destination change rather than being incidental. Nineteen assertions across seven specs waited for /account after signing in; they now assert the header shows a signed-in customer, which is the condition actually being waited for. Modal submits are scoped to their dialog, because the storefront behind now offers a Log in button of its own and an unscoped locator matched both. Assertions that follow a server round-trip were given a realistic timeout — the 5s default is too tight for a bcrypt hash plus re-rendering the storefront behind the modal.
Verified with 83 end-to-end tests, all passing, and type checking clean. No backend changes.
Closes#50
Branches follow Conventional Branch — `<type>/<description>` with types feature, bugfix, hotfix, release, chore — carrying the issue number so the work is identifiable from `git branch`. Commits stay Conventional Commits with the number appended to the subject and a `Closes #N` line in the body. Spells out which half does what, because it is easy to assume the branch name links the work: Gitea builds the reference from a `#N` in a commit message or PR and never from the branch name.
Records the backdrop-location arrangement in `AppRoutes` as the pattern the sibling navigational dead-end issues should copy rather than each inventing their own.
Adds two testing lessons that cost real time here: the 5s default expect timeout is too tight for anything waiting on a bcrypt round-trip, and the local database's accumulated junk eventually stops being harmless clutter and starts producing flake that rotates between unrelated specs on every run.
/account had no header and no links of any kind, so once a customer opened it the only way back to the storefront was the browser's back button or editing the URL.
It is now a modal rendered over whatever the customer was looking at, while staying a real route. Opening it from the header pushes /account and names the current page as the backdrop, so closing returns there with filters intact, and the browser's Back button does the same thing as the close control. Entering /account directly — a bookmark, the link in a verification email, or the redirect after registering — has no page behind it and falls back to the storefront, so closing always lands somewhere real. Keeping it a route means the URL still works: bookmarkable, shareable, and refreshable with the view still open, which the header link and the four post-authentication redirects already depend on.
Deleting an account now clears the session as well. Previously it removed the account server-side and navigated home without touching the auth context, so the header went on offering "My Account" for an account that no longer existed until the next reload. That was always wrong, but the modal makes it visible rather than merely stale, because the storefront is rendered behind and the wrong header is on screen throughout.
The modal body is capped and scrolls, and the order history table scrolls within itself, so the view survives a phone without pushing its own title and close control off-screen.
Also scopes the account switch locator in the favorites spec to the modal, since the storefront now renders behind the account view and has a theme switch of its own, and gives the post-registration wait a realistic timeout — it waits on a bcrypt round-trip rather than a render, and the 5s default was surfacing as a flake on whichever test lost the race under parallel load.
Closes#51
Notes that the default local Node (18.16.1) cannot run either the integration suite or Playwright, and that neither failure names the version as the cause — the integration one reads like a broken lru-cache dependency. Records the newer version's path so a single command can be run against it without switching what the user has active.
Adds two e2e lessons from this change: isVisible() does not wait, so guarding an optional dialog with it loses the race and leaves an antd modal open to intercept every later click; and under fullyParallel a test that mutates a shared fixture races its siblings.
Records that the storefront listing sold items is now load-bearing rather than merely tolerated, since the favorites filter deliberately shows sold favorites, and points at the favorites filter as the pattern for any future filter dimension that depends on who is asking.
Adds favorites as another dimension of the existing storefront filter rather than a separate view, so it lives in the URL, shows up as a removable chip, and combines with category, tags, and price by AND like everything else. A customer can ask for "my favorites under $500 in Furniture" instead of only "my favorites".
Sold favorites are included. The storefront shows sold items everywhere else, and a favorite that has just sold is often exactly what the customer came back to look at after being emailed about it in #34. Hiding them would make items disappear from a list the customer curated themselves. Anyone wanting only what they can still buy can combine the toggle with the status filter.
Which customer "my favorites" means comes from the session, never from the query string, so a hand-edited URL cannot name someone else's favorites. A signed-out visitor sees the toggle and gets the same inline register/login prompt the heart button and Add to Cart already use; signing in resolves the gate and the filter applies on its own. A bookmarked favorites link whose session has expired says so rather than rendering an empty grid, which would tell the visitor they have no favorites instead of that we do not know who they are. The API answers 401 for the same reason, and the admin inventory refuses the filter outright rather than ignoring it.
The shared SQL builder now requires callers to say whose favorites they mean, even when that is nobody, and throws instead of dropping the clause — a future caller that forgets the guard fails loudly rather than quietly returning the whole catalogue.
Verified with 59 unit tests, 134 backend integration tests, and 70 end-to-end tests, all passing, with type checking clean on both sides.
Deleting an item cascades its favorites away, so anyone watching it lost the record silently and heard nothing. Recipients are now collected before the delete, since after it there is nobody left to look up, and the mail is sent only once the delete has succeeded so nobody hears about a withdrawal that did not happen.
Items that had already sold are excluded. Their favoriters were told at the point of sale, and a second "no longer available" for the same item reads as a duplicate rather than news.
The wording differs from the sale notification — withdrawn rather than sold — because the customer did not lose out to another buyer and saying so would be untrue.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Customers can favorite and unfavorite items from the storefront, opt in to being told when a favorite is sold to someone else, and manage that preference from their account page.
The opt-in is a consent of its own rather than the existing marketing flag. Being told that a specific item you asked about has gone is a narrower thing than agreeing to marketing, and folding one into the other would leave marketing_consent_text no longer describing what was actually agreed to. It is recorded the same way as the marketing consent — flag, timestamp, and the exact wording shown — and accepting it does not set marketing_consent.
The prompt appears only after a customer has actually favorited something, so the reason for asking is concrete rather than an abstract marketing ask, and it says plainly that it is separate from marketing email. Declining keeps the favorite.
Notifications fire when an item reaches sold, either through checkout or an admin marking it sold, and never to the buyer — telling someone the item they just bought is unavailable reads as a bug. Reserved is deliberately not a trigger: reservations expire and get released, so a "gone" email would often be about an item still for sale. Disabled accounts are excluded, per #33.
completeCheckout now returns the sold item ids and the buyer so its three call sites can notify after COMMIT. Sending inside the transaction would email people about a sale that then rolled back, and would hold the transaction open for SMTP. Each message is sent independently so one bad address cannot stop the rest, and the sale has already succeeded regardless.
Favoriting while signed out opens the existing inline register/login modal, exactly as Add to Cart does, and completes the favorite on success.
Also fixes a latent bug in the same component: while the session was still resolving, `customer` is null for a signed-in visitor too, so clicking Add to Cart or the new heart in that window prompted them to sign in again. Both now ignore clicks until the session has resolved, and the control shows as loading meanwhile.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds customers.disabled_at, admin disable/enable endpoints, a Status
column and toggle on the Customers tab, and enforcement across every
path that authenticates.
Enforcement lives in attachCustomer, which previously validated only the
session token and its expiry and never read the customer row. Register,
login and password reset all mint sessions, so a single check in the
middleware covers every path rather than three separate ones — and it
means an existing rd_session cookie stops working at once instead of at
its 30-day expiry. Disabling also deletes the sessions outright, so
eviction does not wait for the next request.
Disabling releases the items the customer was holding, in the same
transaction. A disabled account cannot check out, so leaving its
reservations would keep one-of-a-kind stock off the storefront for up to
the cart expiry window for no purpose. Guarded on 'reserved' so a sold
item is never resurrected. Re-enabling restores sign-in but does not give
the items back — they may since have sold.
Sign-in returns an explicit 403 rather than a generic credential failure.
That does confirm the address has an account, which sits awkwardly beside
the deliberately non-enumerating reset in #32; the trade was made the
other way because a disabled customer told "invalid email or password"
resets their password, succeeds, is still locked out, and concludes the
site is broken. The check runs only after the password verifies, so it is
not a bulk membership oracle, and /register already reveals existence.
A reset token issued before the disable no longer mints a session, and no
new tokens are issued for a disabled account — while still answering 200,
so that endpoint stays non-enumerating.
Self-service GDPR export and deletion are blocked along with everything
else, so those requests now need servicing by hand. Worth checking the
privacy policy does not promise unconditional self-service.
Also fixes an unrelated bug the e2e run surfaced: the admin inventory
fired a request per keystroke in the price fields with no sequencing, so
an older response could land after a newer one and repaint stale rows.
Only the most recently issued request may now set state.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The QA image is now built by Portainer from this repository rather than
by Gitea Actions. Deployed as a Git repository stack, "Pull and redeploy"
pulls the repo, builds from the Dockerfile, and recreates the containers
in one action.
This removes the runner from the loop entirely. Three dispatches failed
without ever building: the runner refuses privileged containers, so the
dind service was never created. Working around that needed either the
host Docker socket mounted into the runner or privileged containers
enabled runner-wide, and both hand every workflow on every branch
root-equivalent control of the NAS, production included. Portainer
already holds the socket — that is how it manages containers — so
building there needs no new privilege at all.
pull_policy: build is what keeps it honest. Without it the stack reuses
whatever is tagged redefined-designs:qa, which is exactly how a redeploy
appears to succeed while still serving old code — a failure this project
has already hit twice.
Deleting qa-build.yml also drops the registry, the REGISTRY_TOKEN and
BREVO_API_KEY secrets, and the notification email. The email existed
because CI worked asynchronously and had to tell you when it finished;
redeploying from Portainer is synchronous, so the browser already does.
Losing the per-commit image tags is a real cost — rollback becomes
"rebuild from the ref you want" rather than retagging a specific build.
README changes for this are deliberately not in this commit: that file
also carries uncommitted work of Thom's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The job was holding the runner for hours on every push and pull request.
On 2026-08-18 it ran 3h12m, and because the runner is sequential it kept
frontend-e2e queued behind it for that whole time.
It is not a slow suite. That run executed 89 tests in 87 seconds, then
logged "Jest did not exit one second after the test run has completed"
and sat there until something killed it — an open handle keeping the
event loop alive after the run finishes. The same run also failed with
relation "orders" does not exist, so the schema was missing in CI even
though globalSetup migrates. Neither is root-caused yet; both follow the
job into its new home.
So the new workflow carries a 15-minute timeout. The suite needs about
90 seconds, so this is not a performance budget — it is a stop that turns
a post-run hang into minutes of wasted runner time rather than hours.
Tests.yml keeps a comment saying where the job went, so its absence reads
as deliberate rather than as something lost in an edit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first dispatch failed at the wait step with nothing to act on. The
run logs showed the dind service container was never created — no service
startup output at all, and teardown reporting "No such container" for the
ID it had recorded — which is what act_runner does when it refuses a
privileged container.
From the failing step, that is indistinguishable from dockerd simply
being slow, so the step now says which one it is: if the service host
resolves, the container exists and dockerd is not serving plain TCP on
2375; if it does not resolve, the service never started and the runner
needs container.privileged.
Also raises the wait from 30s to 90s. The NAS took the full 30s before
failing, so the old ceiling was too close to the observed time to
distinguish slow from broken.
The docker CLI fallback is retained: the runner image has no docker
binary, and the static install worked.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a workflow_dispatch job that builds a chosen ref, pushes it to the
Gitea container registry as :qa plus a commit-sha tag, and emails when it
is ready. It deliberately does not restart the QA stack — redeploying
stays a human action in Portainer.
The build runs against a Docker-in-Docker service rather than the NAS's
Docker socket. Mounting the host socket into the runner would give every
workflow on every branch root-equivalent control of the NAS, production
included; pushing to a registry means the image does not need to survive
in the build daemon.
The QA stack now pulls that image instead of requiring a local build. The
previous arrangement meant the image existed only if someone remembered
to build it, which produced two confusing failures already: a Docker Hub
"pull access denied" when the tag was missing, and a silent stale-image
deploy when the build had not been rerun.
Two runner capabilities cannot be verified from here — privileged service
containers for dind, and a docker CLI in the runner image. The workflow
checks both and fails with an explanation rather than a connection
refused, and validates all five required secrets and variables up front
rather than part-way through a build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds "Forgot password?" to the login page, a request page, and a reset
page reached by a one-hour, single-use token delivered by email. Reuses
customer_tokens with a new password_reset kind alongside verify_email.
The request endpoint always answers 200, whether or not the address has
an account, so it cannot be used to test addresses for membership. Note
/register still reveals existence through its 409 on a duplicate, so this
protection is currently partial; closing that is its own change.
Completing a reset deletes every session for that customer. A reset
prompted by a compromise has to evict the intruder, and leaving a 30-day
cookie alive would defeat the point. It also marks the address verified,
since receiving the mail is exactly what verification proves, and
supersedes any outstanding token so an older link in the inbox cannot be
resurrected.
Introduces the first rate limiting in the codebase, on the request
endpoint only. The limiter is keyed on caller *and* submitted address:
keying on IP alone would let one person lock out everyone behind the same
proxy, and everything arrives via Nginx Proxy Manager. Applying that same
limiter to the reset endpoint, which carries no address, collapsed every
caller into one shared bucket -- so that endpoint is deliberately
unlimited instead, protected by a 32-byte single-use token whose bcrypt
work only runs after the token matches.
The e2e tests read the issued token directly from Postgres rather than
through a test-support endpoint. An endpoint returning a reset token for
an arbitrary address is account takeover for every customer if it is ever
reachable, and an environment gate is thin protection against that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven reported items, of which the first four had two root causes.
The active tab was invisible in dark mode because colorPrimary was
hardcoded to #1a1a1a in both themes. The accent now inverts with the
theme, and colorTextLightSolid inverts with it, or a near-white accent
would get antd's default white label and disappear.
The Category tab, Tag tab, and item-form category selector ignored the
theme entirely. antd declares main: lib/index.js and module: es/index.js,
so importing from 'antd' resolves to the ES build while 'antd/lib/...'
loads the CommonJS one — two copies, two React contexts, and no
ConfigProvider for anything deep-imported. Switching those files to
antd/es/* keeps the deep-import convention and shares the instance. This
was introduced by my own use of the lib path; es is correct under Vite.
Two storefront components had the same latent bug.
"Colour" is now "Color".
The Customers tab shows how many items each customer is holding, as a
link opening the item list with a Release button. Release mirrors the
customer's own cart removal — drop the cart row, return the item to
available, guarded on 'reserved' so it can never resurrect a sold item —
and deliberately sends no email about an action the customer did not
take. The count is a subquery rather than another join, which would have
multiplied rows and inflated order_count and total_spent_cents.
The Inventory tab filters by category, tags, price, and status, reusing
the storefront's parser and query builder so the two cannot drift.
Reserved is one option in a Status filter rather than a standalone toggle.
Also fixes two defects the screenshots exposed: the reserved-count link
bubbled to the row handler and opened the customer drawer behind the
dialog, and .admin-category-node had no CSS at all, so the tree node name,
item count, and actions ran together as one string.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records the two-environment layout and a numbered checklist every change
follows, with QA review as a required stop rather than a judgement call.
Production is not where a bad deploy should be found, which is what
happened with the categories/tags release.
Production promotion now retags the image QA reviewed rather than
rebuilding, so what ships is exactly what was tested, and the README
carries the full command sequence with a verification gate at each step.
Also records the two gates that have already failed here: confirming a
pushed commit is actually on the branch, and that a schema/code ordering
problem cannot be caught by any local suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Logging out destroyed the server session and navigated home, but never
told CustomerAuthContext, so `customer` stayed in React state and the
header kept offering "My Account" instead of "Log in" and "Sign up". A
reload appeared to fix it, because fetchMe then returned null, which is
why the symptom looked intermittent. The cart badge had the same cause:
CartContext only clears its items once `customer` goes null.
Logout now lives on the auth context, which clears `customer` itself
rather than triggering a refetch — a refetch would leave a window where
the session is gone but the UI still shows the customer signed in.
logoutCustomer also ignored res.ok. A failed logout leaves the session
cookie valid, so reporting success signed the customer back in on their
next reload. It now rejects, and the account page reports the failure and
stays put instead of pretending. Logout is not routed through handle()
because the endpoint answers 204 with no body.
Navigation uses replace, so Back no longer returns to the account page,
which would only bounce to /login now the session is gone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deploying the stack first failed with "pull access denied for
redefined-designs, repository does not exist or may require docker
login". The README had the Portainer stack creation ahead of the build,
so no local redefined-designs:qa image existed and Compose fell back to
pulling from Docker Hub, where the repository does not exist.
The build step now comes first in the one-time setup, and the review
workflow says to rebuild before restarting the stack — the stack builds
nothing itself and would otherwise run whatever was last tagged :qa.
pull_policy: never makes the failure legible: a missing local image now
reports itself as missing rather than as a registry authentication
problem. Noted as safe to remove if the NAS's Compose ever rejects the
key, since it only affects the error message.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A disposable stack for reviewing merged-but-undeployed changes online,
running alongside production on the same NAS.
Every value that could collide with production is changed: container
names, host port (32751), volume paths, database name, and image tag.
The Postgres data directory is the critical one — sharing production's
would mean QA writing into production's database files.
QA deliberately differs from production in three ways. DEMO_MODE=true
with no PayPal credentials, so the full cart and checkout path is
exercisable with no route to live PayPal. No SMTP configuration, so a QA
run cannot email anyone; the mailer already degrades gracefully when
unset. And restart: "no", so a NAS reboot does not quietly bring QA back
up and leave it running.
The stack must be named redefined-designs-qa in Portainer: the stack name
becomes the compose project name, and reusing production's would make
compose reconcile the two against each other and remove the production
containers.
README covers the one-time NAS directory setup, the review workflow, and
how to reset QA data.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The wireframes behind the storefront filter layout decisions lived only in
.superpowers/brainstorm/, which is gitignored, so they were lost to anyone
reading the repo.
That directory stays ignored — it also holds a brainstorming-session token,
PID files, and absolute local paths, none of which belong in the repo. The
mockups themselves are design artifacts, so they are copied into the specs
directory and wrapped as standalone pages: the tool serves them as fragments
inside its own frame, so its style tokens and toggleSelect helper are inlined
to make them open in a browser with no server and no network.
Both rejected layouts and the rejected mobile variant are kept alongside the
chosen ones — the comparison is the part worth preserving.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reported from testing: the item form said "Item added" for a save that
never happened.
saveItem and the other admin calls returned res.json() without checking
res.ok, so a 4xx/5xx resolved normally and every caller reported success
for a write the server had rejected. That is worse than failing outright,
because nothing prompts the user to look for the missing row. All admin
calls now throw on a non-OK response, and the handlers report the error,
keep the form open so entered values survive, and only claim success once
the server has accepted the write. Mark sold/available previously did
nothing visible on failure at all.
Categories can now be created from the item form, as tags already could.
Previously a category that did not exist yet meant abandoning a
half-filled form for the Categories tab. New categories are created at
the top level; nesting stays in the Categories tab.
The control lives in its own component: inline, every keystroke
re-rendered the whole Inventory component and rebuilt the category tree,
which visibly jittered the open popup. It sits above the tree rather than
below it, where a long list both hid it and made its position depend on
the list's measured height. The tree no longer expands everything on
open, which does not scale past a screenful; it has search instead.
The app now honours prefers-reduced-motion by disabling antd transitions,
and the e2e suite runs with that preference set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The storefront showed no inventory after deploying the categories/tags
release. No data was lost: the code queried categories/item_tags/
items.category_id against a database where the migration had not been
run, and that failure was invisible at every layer.
Three changes, each addressing one layer:
Migrations now run at container start, so deployed code cannot be ahead
of the schema and the easily-forgotten manual `docker exec migrate.js
up` step disappears. migrate.js waits for Postgres to accept
connections first, since the NAS brings the DB container up slower than
the app, and still exits non-zero so a bad migration stops the
container rather than serving a half-migrated schema.
Express 4 does not forward a rejected async handler, and no error
middleware was mounted, so a failing query never responded at all. Async
routes are now wrapped and an error middleware guarantees a 500. A hung
request is indistinguishable from an empty result in the UI, which is
how a schema mismatch came to read as "the store has no items".
The storefront now separates "request failed" from "no items" and offers
a retry. fetchItems/fetchFilterOptions throw on a non-OK response rather
than returning the parsed error body, which would have been set as the
item list and crashed the grid on .map.
Also restores the project-context update from 7fb5764, which was left
out of PR #24 and ended up dangling.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Storefront gains a Filters drawer holding the category tree, colour-coded
tag pills, and a price range, with applied filters shown as removable
chips. Filter state lives in the URL query string, so a filtered view is
shareable and the back button works. Item cards now show their category
and tags.
Admin gains Categories and Tags tabs, and the item form gains a category
TreeSelect plus a tags Select that creates new tags on the fly.
The admin category tree tracks expansion in state rather than using
defaultExpandAll: that prop is evaluated once at mount, so a branch added
afterwards rendered collapsed and its children were unreachable. Creating
or moving a node now expands its parent. Caught by the new admin e2e spec.
The chip row is marked as a named group so its "Clear all" stays
distinguishable from the drawer's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a self-referencing categories tree, a tag registry with
deterministic colours, and item_tags, plus admin CRUD for both.
GET /api/items now accepts category, tags, min_price and max_price.
Category matching walks the subtree with a recursive CTE so selecting a
parent includes everything filed beneath it; tags match with AND via a
count check, since ANY() alone would return items carrying only one of
them. Malformed filter params return 400 rather than being ignored, so a
broken link doesn't quietly list the whole catalogue.
GET /api/filters serves the drawer its tree, tags, and price bounds in
one request.
Item image/tag aggregation moves from LEFT JOIN + GROUP BY to scalar
subqueries. Joining two one-to-many relations multiplies their rows, so
an item with 2 images and 3 tags would have repeated every image three
times once tags were added.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records the resolved requirements for issue #23: manual category tree
(no rule engine), single category per item with descendant matching,
central tag registry with hashed-then-overridable colours, AND semantics
for multi-tag filtering, and a drawer-plus-chips storefront filter UI.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
getByLabel('Email') matched two elements on /register -- the email input and
the marketing consent checkbox, whose label "Send me occasional emails"
contains the substring. Playwright's strict mode failed both tests that
filled it.
getByRole('textbox', ...) narrows by role, so the checkbox no longer collides.
Applied to the /login usage too, so the same concept reads the same way
throughout the file.
These failures were latent: the frontend-e2e job died at the build step
before it ever reached the tests, so CI never reported them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Verification emails linked to /verify-email?token=..., but no such route
existed in main.tsx and nothing in the frontend ever called
POST /api/customers/verify-email. The SPA catch-all served index.html, no
route matched, and the page rendered blank -- so the token was never
redeemed and accounts stayed unverified forever.
The gap was invisible until SMTP was configured, because no verification
email had ever actually been delivered.
Adds VerifyEmail.tsx (verifying / verified / failed states), a verifyEmail
call in customerApi, and the route. The request is pinned to a single
firing via a ref: the endpoint deletes the token on success, so StrictMode's
double effect invocation in dev would otherwise overwrite the success state
with "invalid or expired token".
Verified end to end against a local stack: a real token flips
customers.email_verified to true and is consumed from customer_tokens.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
npm treats NODE_ENV=production as --omit=dev, so `npm install` in the
frontend-e2e job skipped typescript and the build died on `tsc: not found`.
The same env var would have stripped vite and @playwright/test from the
frontend install, and flipped the session cookie to Secure on a run served
over plain http.
The reported summarize crash was a symptom: the job aborted before Playwright
ran, but Summarize is `if: always()` and threw ENOENT on the missing JSON,
burying the real failure. Both summarize scripts now report the missing file
and exit 0 -- the job still fails via its own step.
Also split build from start, replaced `sleep 3` with a readiness poll against
/api/config, and dump the backend log when e2e fails.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
routes/paypal.ts and routes/demo.ts were the pre-cart single-item
checkout flow. Nothing has imported them since the cart flow landed:
app.ts mounts only cartCheckout, the frontend calls /api/checkout/cart/*,
and no test touches them. They duplicated PAYPAL_BASE, getAccessToken,
and a second handler for the /webhooks/paypal mount.
Also extract openCheckout() from /paypal/create and /demo/purchase in
cartCheckout.ts, which repeated the same address-ownership check, cart
lock, and checkouts/checkout_items inserts. It returns a discriminated
union so callers keep control of the transaction and the response. Add
CartItem/LockedCart interfaces, dropping the (it: any) casts.
Note: paypal.ts was the only writer of items.reserved_until and
items.paypal_order_id. Those columns are now write-dead; the schema is
left alone for a separate migration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
node-cron, node-pg-migrate, and @types/node-cron are declared in
package.json but were missing from the lockfile, so npm ci fails and a
plain npm install silently rewrites the lock. Regenerated to match.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- S5693: MAX_IMAGE_BYTES was 8 * 1024 * 1024 (8,388,608), just over the
8,000,000-byte ceiling the rule treats as safe, so the hotspot on the
multer storage config never cleared. Use 8_000_000.
- S5689: Express advertises its stack in X-Powered-By by default, which
tells an attacker what to aim exploits at. Disable the header.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Introduces the `package-lock.json` file to the frontend directory to ensure consistent dependency versions across development environments and deployments. This prevents dependency drift and improves build reliability.
SonarQube flagged three hotspots in the admin router: multer was
configured with no content length limits, and stored filenames were
derived from Date.now() plus Math.random().
- Cap the multipart body on every dimension: 6 files, 8 MiB per image,
8 fields, 64 KiB per field. Without limits a single request could
fill the uploads volume.
- Generate stored filenames with crypto.randomUUID() so paths are not
predictable. Image ordering is unaffected; sort_order already drives it.
- Wrap the upload middleware to translate MulterError into 413/400 JSON.
The app mounts no error handler, so a limit rejection would otherwise
surface as an HTML 500.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bermudalamb
