95325c75d4581f591149106a9b8ebe8096a4941e
25
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  bermudalambandClaude Opus 5
|
2fe3aa055f |
docs(ops): make the cutover runbook agree with the compose file about DEMO_MODE (#196)
#190 turned `DEMO_MODE` from a hardcoded compose value into a Portainer stack variable with no default, and updated the compose header. The runbook that actually creates the stack was not updated with it, so the document and the file it deploys have been disagreeing since — in the three places most likely to be read under pressure. Step 2's list of stack variables to record did not include `DEMO_MODE`, and step 6 says to add the variables from steps 2 and 3. An operator following this literally creates a stack that cannot boot. It is now in the list, with a paragraph of its own: it is the only one of the nine that is not a secret, which is exactly why it is the easy one to skip past. The troubleshooting section said `DEMO_MODE` was hardcoded and therefore could not be missing, so its absence from the error list proved nothing. That was the most dangerous sentence in the file — an unset `DEMO_MODE` is now the *first* thing to check rather than something to rule out. The line now says it moved and why. The crash-loop example was the PayPal triple, which cannot occur while `DEMO_MODE` is `true`. The message an operator will actually see during the demo interim — `DEMO_MODE is required and must be exactly 'true' or 'false'` — appeared nowhere in the runbook. Both forms are shown now, in the order they are likely to be hit. Added what neither document said: Compose only *warns* about an unset variable and deploys anyway. In Portainer's stack UI that warning is easy to miss, and the container then crash-loops under `restart: unless-stopped` — loud in the log, invisible in a glance at the stack list. The container log is the signal, not the deploy output. The compose header carried two claims that #190 falsified and did not correct: that the PayPal secrets are required "because DEMO_MODE is false below", and that only secrets are interpolated. Both now describe the file as it is. This is the drift `composeEnvironment.test.ts` exists to prevent, surfacing in the one place no test can reach — the guard keeps the compose file honest about its own intent and cannot see the runbook beside it. Verified: 278 backend unit tests pass, including the compose guard that parses this file, and a sweep for the stale claims finds none left. Closes #196 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalamb
|
1a59edec18 |
docs(ops): prove the backups during the cutover rather than waiting for a schedule (#192)
Step 4b created the directories and step 7 checked the containers were Up. Neither established that a restorable file actually gets written, and those are not the same claim — the database backup does not run until 03:00 and the uploads archive not until Sunday 04:00, so a stack that looks correct at the end of a cutover can be four days from its first evidence. Both tools take a manual trigger, so the wait is unnecessary. The runbook now forces one run of each, checks the sizes are plausible, and greps the dump for `COPY` lines on the real tables — a dump of an empty database succeeds and looks fine, which is the one way this check could otherwise lie. It also confirms the healthchecks agree with where the files landed. A check whose `find` path disagrees with where the tool actually writes reports unhealthy forever while the backups are working perfectly, and that is a thing to discover on the day the stack is built rather than a year later. `starting` corrected to `unhealthy` in the surrounding text: during `start_period` Docker reports `starting`, which is what an operator actually sees and what the previous wording got wrong. Proven against production during the cutover on 2026-08-26 — a 41K dump and a 15M archive, both landing where the healthchecks look. |
||
|
bermudalamb
|
724e9ce19d |
docs(ops): say that the backup directories have to be created (#192)
The stack gained two backup services in #147 and nothing has ever told anyone to create the directories they mount. `backup-and-restore.md` reads from both paths and the compose file mounts both, but no document creates them — while the README does exactly that for QA's data directories, ownership notes and all. Production's cutover runbook said nothing. Hit for real during the cutover: both backup containers sat in Created, never started, and `docker logs` on them reported only that nothing matched the filter, because a container that never ran has no output. Portainer showed them beside the healthy ones and the stack looked deployed. That silence is the reason this is worth a step of its own rather than a footnote. A backup regime that never started is indistinguishable from a working one until someone needs a restore, which is the failure mode the healthchecks in #147 exist to catch — and those healthchecks cannot fire on a container that is not running. The cutover runbook gains the directory creation before the stack is created, and step 7 now counts containers rather than only checking the app: four, all Up, with Created called out as the thing to look for. Counted from the compose file rather than from memory — the first draft said five. `backup-and-restore.md` gains the same note where it describes the destinations, since anyone reading that page is already thinking about paths. No `chown`, deliberately stated: both backup images run as root, unlike the Postgres image whose data directory needs uid 999, and an unnecessary chown instruction is how people learn to run them without thinking. |
||
|
bermudalamb
|
0cec9b3c1f |
docs(ops): name the crash loop the cutover runbook was most likely to cause (#175)
Step 7 listed a missing `ADMIN_GATE_SECRET` — a warning the container starts through — and said nothing about the three PayPal secrets, which are a hard error that crash-loops it. That is backwards: the secrets are the likeliest thing to be missing after a stack replacement, because Portainer stack variables belong to the stack and are discarded with it, and `DEMO_MODE` is false in production so the app refuses to start without them. Hit for real following the runbook. The verify section now shows the actual log block, says what it means, and gives the way to tell a missing variable from a misnamed one: a hardcoded value cannot be missing, so its absence from the error list proves nothing, while an interpolated variable that stays quiet while others complain proves substitution works and those others are simply unset. That is the reading that turns the log into a diagnosis instead of a list. It also says the loop is harmless while the values are fetched — the container refuses before it serves anything or touches data — and where the values live if the old stack is already gone, since the webhook id in particular is readable rather than only recreatable. Step 2 now names every interpolated variable rather than describing them in general, and states the consequence of each class going missing. A general instruction to record the environment is easy to read as already done. |
||
|
bermudalamb
|
60b76cae82 |
docs(plan): correct the cumulative unit test counts (#188)
Task 1 leaves 4 tests and Task 2 adds 7, so the running total is 11 rather than 12, and the same off-by-one carried into Tasks 3, 4 and the completion criteria. Caught by the Task 2 implementer, which flagged the mismatch rather than inventing a test to reach the stated number. |
||
|
bermudalamb
|
e695d91670 |
docs(plan): implementation plan for filter dimensions (#188)
Nine tasks, each ending in something independently testable, in an order where every task leaves both screens working. The dimensions are built and unit-tested first, the shell after them, and the two screens are wired last — so the old components stay in place until the thing replacing them is proven. The two behaviour changes the design accepted are each covered twice: a unit test on the chips that produce them, and an end-to-end assertion on what a person sees. The admin tally reading three for three statuses, and a non-default availability producing a removable chip. Task 8 carries the deletions, deliberately last. Removing `activeFilterCount` and `hasActiveFilters` turns every remaining caller into a compile error, which is the cheapest way to find them. Two defects found reviewing the plan against the code rather than against itself: the test fixture omitted `Category.item_count` and would not have compiled, and Task 9 added a page object method that nothing used — `chooseAvailability` and `filterChip` already exist. The plan also records what must not be read as a regression. Two assertions in the storefront specs fail on every branch because the unpaginated grid cannot render 1,600+ development rows inside Playwright's default timeout, which is #186 and predates this work. Refs #188 |
||
|
bermudalamb
|
20a38b66bd |
docs(design): filter dimensions, one composable component both screens extend (#188)
#169 made the filter drawer shared, which was the right first move and not the finish. Per-screen differences are booleans, the bar around the drawer was never shared at all, and the storefront's availability preset sits outside the system because the shared component cannot express "this belongs in the bar, not the drawer". The design replaces the flags with composition: a screen contributes a list of filter dimensions, each declaring where it renders, how to render it, and what chips it contributes. A screen-specific control becomes an ordinary dimension, appearing in the chip row and counting toward the tally without the shared code knowing what it is. Dimensions are plain data rather than components or context, for a concrete reason rather than a stylistic one. The drawer sets destroyOnHidden, so its sections are unmounted whenever it is closed — exactly when the chip row matters most. Anything that registers on mount would lose those chips the moment the drawer closed, which rules out the otherwise-idiomatic context-and-children approach. The tally becomes the number of chips, so the count and the chip row cannot disagree — today they are computed by two routes and agree by coincidence, which the admin already has to correct by hand. Three visible behaviours change as a result, recorded in the spec rather than left to be discovered. Adding vitest is in scope. The design's value rests on chips() being pure, and the frontend has no unit runner at all, so without one the core of it would ship covered only indirectly and expensively through Playwright. The spec also records what is deliberately untouched: the filter state, the URL serialisation, the backend, and the two e2e assertions already failing on #186 — which must not be read as regressions from this work. Refs #188 |
||
|
bermudalamb
|
cf9003bc2d |
docs(ops): write down how to cut production over to the committed compose file (#175)
#118 put `docker-compose.prod.yml` in the repository and argued at length for why it belongs there. It never produced the procedure for getting production from where it is — a stack living in Portainer's web editor — to where that file expects it to be. What existed was scattered and none of it was a procedure. The compose header described the destination: name the stack this, deploy it as a git repository stack with these settings, rename any variables whose names differ. The README's deployment section is a good runbook for the routine redeploy, and assumes the stack is already in the right form. So the riskiest deployment operation this project has was the one with no steps written down. `docs/ops/production-stack-cutover.md` is those steps, in the order that matters, with the two facts that decide whether the operation is safe stated up front rather than left to be inferred from the volumes block. The first is that all persistent data is bind-mounted from the NAS filesystem rather than held in Docker-managed volumes, so deleting the stack cannot lose the database or the product images. That is what makes the cutover recoverable at all, and step 2 verifies it rather than trusting it — the `docker inspect` there discriminates `bind` from `volume` and says to stop on `volume`, because this runbook does not cover that case. The second is that both services set an explicit `container_name`, so the old containers must be gone before the new stack starts or the deploy fails on a collision that reads like a Portainer bug rather than a sequencing mistake. It also captures what is simply lost if not recorded first. Portainer stack variables belong to the stack and are discarded with it; they are all secrets, and a stack brought back up with a different `DB_PASSWORD` than the data directory was initialised with cannot authenticate against its own database. Recording them is step 2 and it is what makes the rollback credible. The verification section separates the boot warnings that are correct in production — no mail allowlist, no uploads origin yet — from the one that means a variable never arrived. That distinction is the whole failure mode #118 was about: a stack variable whose name matches nothing in the file is substituted nowhere and never reaches the container, and the failure reads as "I set it and it says it is not set". Every diagnostic command in it was run rather than written from memory. The mount inspection was checked against a container that genuinely uses a named volume, to confirm it reports the difference the step depends on. The compose header now points at the runbook for the procedure and keeps the rationale that belongs at the point of use — why there is no `build:`, why most values are hardcoded, why `NODE_ENV` is absent. The README says plainly that its section is the routine deploy and links to the other one. Closes #175 |
||
|
bermudalamb
|
ae20568601 |
docs(ci): correct the #154 hypothesis — the database is wiped before the tests start (#154)
dmesg came back empty, and the evidence that actually settles it was in the original log the whole time. Step 7 `Run migrations` succeeded, then step 9's globalSetup applied all six migrations again from scratch. Both point at the same database — migrate.js reads PGHOST/PGDATABASE and the job sets those and TEST_PG* to the same service and the same redefined_test — so had step 7 migrated it, globalSetup would have printed "No migrations to run!", which is what a local run prints. It found an empty database. The wipe was already happening before the tests started, which makes this a database being reset repeatedly rather than a container dying partway through a heavy run, and accounts cleanly for the empty dmesg: a restart is not a kill. The suspect moves from memory pressure to the runner's handling of `services:`, where act_runner has been uneven across releases. The diagnostics change with it: A3 is dropped because it was designed to catch starvation, A2 is demoted to a fallback, and the new first check needs nothing but the Gitea UI — compare step 7 and step 9's migration output in any failing run. The superseded hypothesis is kept rather than deleted. A future reader finding memory ruled out is better served by seeing why it was suspected and what refuted it than by a document that never mentions it. Refs #154 |
||
|
bermudalamb
|
ebefcbb76b |
feat(ops): schedule database and uploads backups, and document the restore (#147)
The only copy of every customer, order and one-of-a-kind item was the live Postgres data directory, plus whatever the deploy checklist's manual pg_dump happened to have caught. That dump is good and stays, but it only runs when someone deploys: a quiet week meant the newest copy of real customer data was a week old, and nothing bounded the gap. Two services rather than one, because they are different jobs. The database is small, changes constantly and wants a logical dump — daily, gzipped, 7/4/6 daily-weekly-monthly retention. Uploads are large and append-mostly and want an archive — weekly, 56 days, mounted read-only so a backup process cannot damage the thing it is backing up. Forcing both through one tool would serve one of them badly. The dumper is pinned to postgres-backup-local:16 to match the server. pg_dump refuses to dump a server newer than itself, so a floating tag is a backup that stops working the day Postgres is upgraded — silently, because nothing reads a dump until it is needed. It depends_on the database's existing pg_isready healthcheck, which is the constraint that shaped this: a dumper is a client, and without that the first run after a NAS reboot races Postgres coming up. Both carry a staleness healthcheck rather than trusting the schedule. A regime that stopped a month ago is indistinguishable from a working one until a restore is attempted, and `find -mmin` is the cheapest thing that tells them apart. It surfaces in Portainer beside the app rather than somewhere separate to remember to look. Windows are the interval plus grace — 26 hours daily, 9 days weekly — so a late run is not a failure, and start_period covers the first cycle when nothing has been written yet. Verified rather than assumed. Both images were pulled and checked to have a shell and `find`, since a CMD-SHELL healthcheck against an image without one reports unhealthy forever. postgres-backup-local:16 ships pg_dump 16.10 against the postgres:16 server. The healthcheck expression was exercised three ways in the image itself — empty directory, fresh artifact, and one aged three days — and returns unhealthy, healthy, unhealthy. The compose file parses and the #118 drift guard still passes over it. Three things these deliberately do not cover, written into the compose file and the doc rather than left to be discovered: They run while the stack runs, so they cannot protect the stack's own teardown. Deleting the Portainer stack deletes them too. That is why the deploy checklist's manual dump stays, and README now says so where the checklist is. They write to the same volume as the data they protect. That survives a bad migration, a dropped table, a bad deploy and a stack deletion, and not the disk. Getting a copy off /volume1 is a Synology-side job and is what turns this from a convenience into a guarantee. Daily database against weekly uploads leaves a window where a restore pairs the two from different moments. An orphaned image is harmless; a row without its image is a broken thumbnail on one recent item, usually still on the admin's machine. Neither is data loss, and a synchronised snapshot is not worth the complexity to avoid it. The restore procedure leads with practising on a throwaway database, because an untested backup is a file of unknown validity and a truncated dump looks exactly like a good one until it matters. It checks row counts and the pgmigrations head — the migration check being the one most easily skipped and most likely to bite, since a dump older than the code restores a schema the app will fail against. Both open items are listed as unticked in the doc: no off-volume copy exists yet, and no restore has been performed. Until the second is done this documents an untested procedure, and it says so. Refs #147 |
||
|
bermudalamb
|
949734d1e1 |
docs(ci): add the working document for the #154 schema-loss investigation (#154)
The diagnosis so far, what has been ruled out, and the three read-only checks that would confirm or refute it — with slots to paste the output into and a note against each saying what the answer means. Written as a working document rather than a summary because the decisive check has a timing constraint that is easy to miss: the Postgres service container is deleted when the job finishes, so watching it has to happen during the run. Discovering that after the fact costs another full run. It also records the negative result deliberately. If the container is healthy throughout, the hypothesis is wrong and the document says which suspect is next, rather than leaving an abandoned theory for the next reader to re-derive. Refs #154 |
||
|
bermudalambandClaude Opus 5
|
d65eb7b981 |
docs: design for moving Order History onto its own page (#121)
Records why /orders is a page rather than another modal route. The app has both precedents: /account, /login and /register are in MODAL_ROUTES and render over a backdrop, while /cart and /privacy are ordinary pages. Order history is closer to the cart, a list you read rather than a dialog you dismiss. A modal at /account/orders was the smaller change and was rejected: it inherits the same 700px width and 70vh scroll cap, so it moves the table without giving it anything. Tabs inside the modal were rejected for the same reason, since they fix the scrolling and leave the cramping. The doc also records the one part of this that is a fix rather than a move. A failed load currently shows a toast and leaves the table empty, and the toast goes away while the empty table does not, so a customer whose request failed sees exactly what a customer with no orders sees. Loading, failed and empty become three distinct states, with a retry on the failed one. Per-order detail and server-side paging are written down as deliberately out of scope, so that leaving them out reads as a decision rather than an oversight. Refs #121 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
71cbd142c3 |
fix: address the final review of the error boundaries (#62)
Five findings from a whole-branch review, two of which mattered. The catalogue boundary could not be recovered by the very controls it exists to keep alive. When the item grid threw, the header, filter chips and Clear filters stayed mounted — that was the point — but clicking Clear filters only changed the URL. A boundary does not reset on a client-side state change, so the fallback went on rendering over a catalogue that would by then have loaded perfectly well, and the only offered action reloaded the same failing URL. The shop read as permanently broken, which is the failure this whole change exists to prevent, reintroduced one level down. ErrorBoundary now takes an optional resetKey and clears itself when it changes; the catalogue boundary passes the filter key. The page and modal boundaries deliberately do not take one, because their escapes are hard navigations that remount the tree already — recorded on the prop so nobody completes the pattern by symmetry. The client-error endpoint could fill the disk. It is unauthenticated, each accepted report wrote about 8.7 KB, and Docker's default json-file driver has no size cap — so the rate limiter bounded a render loop, as its comment claims, but not a few hundred cheap source addresses. Stack and component stack now truncate at 1000 rather than 4000, which is still around fifteen frames and cuts the worst case to under 3 KB, and the QA compose file caps and rotates the log. Production is a Portainer stack outside this repository and needs the same option applied there; noted in the design doc rather than left implied. Three smaller things. A falsy thrown value defeated the boundary entirely: throw null is legal, and branching on the error object alone treated it as no error, re-rendered the children, threw again, and would have taken the root down — a blank page, the one outcome this is all here to avoid. The boundary now tracks hasError separately and synthesises a real Error for non-Error throws. The modal fallback announced its title twice to a screen reader, once as the dialog's name and once as the heading inside it, so the redundant dialog title is gone. And the design doc claimed the development-only detail shows the component stack when it only ever showed the message; corrected, with a note that the stack still reaches the server log, which is where it is useful. Verified after all five: backend lint 0 errors, 144 integration tests, frontend lint 0 errors and 31 warnings, 87 end-to-end tests, all against a freshly created database. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
557703f86d |
docs: mark the error-boundary design implemented (#62)
Records the two things the design got wrong. antd's Result renders its title as a plain div, so the design's Result usage and its getByRole('heading') assertions contradicted each other and the tests could never have passed as written — resolved by giving the title real heading semantics rather than by loosening the assertion, because an error page with no heading leaves a screen-reader user navigating by headings nothing to find. And import.meta.env had no ambient declaration anywhere in the app, so the DEV gate did not type-check until vite-env.d.ts was added.
The Vite error overlay risk the design flagged did not materialise.
Refs #62
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
bermudalambandClaude Opus 5
|
94e2267eaf |
docs: implementation plan for React error boundaries (#62)
Four tasks, each ending in an independently testable deliverable: the backend endpoint with its own rate limiter, the boundary and fallback components, the three mount points with end-to-end coverage, and the production-gate verification. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
683139b8d8 |
docs: correct the error-boundary design's fallback actions (#62)
Found while working the design into a plan: the obvious implementation of the fallback's escape actions is wrong, and the spec was recommending it. A React error boundary does not reset when the route changes. The original spec justified placing the root boundary inside BrowserRouter on the grounds that the fallback needed router context to offer a way back — but a fallback offering a Link would change the URL and go on rendering the fallback, which reads as the app being permanently broken rather than recovering. Every escape action is therefore a hard navigation: reload, or setting window.location.href. The placement is unchanged, but it is now justified by what the boundary guards rather than by reasoning that does not hold. Three consequences recorded while there. The three fallbacks get distinct titles rather than one shared string, so a customer learns which part failed and the tests get an unambiguous locator for which boundary caught. The modal throw trigger mounts as an unconditional sibling inside its boundary, so /?boom=modal exercises it with the storefront behind rather than depending on /account resolving a session first. And a fourth end-to-end test asserts the report actually reaches /api/client-errors by observing the request, rather than trusting the reporter was called. Also recorded: new files use antd/es deep imports, this project's documented convention — not antd/lib, which #65 notes loads a second React context and breaks ConfigProvider. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
e8374e391a |
docs: design for React error boundaries (#62)
Settles the four questions #62 left open, and records why each alternative lost. Three mount points rather than one: root, the catalogue, and the modal block. The catalogue boundary is the one that earns its keep, because the likeliest throw in this app is a component rendering API data and the item grid renders the most of it per page — containing it there keeps the header, cart badge and filters alive instead of handing the customer one dead page. The modal boundary exists because the modal-route arrangement couples two independent trees: without a boundary between them a throw in Account blanks the storefront behind it, and a throw in the storefront takes the open modal with it. Errors get reported to a new POST /api/client-errors that logs and returns 204, with no storage. A boundary that only shows a message leaves nobody knowing it happened, which is the exact failure shape this project has designed against three times already. A persisted store with an admin screen was rejected as a subsystem larger than the rest of the issue. Rate limiting needs its own limiter rather than the existing one. rateLimit.ts already documents that passwordResetRequestLimiter is keyed on caller and email, and that reusing it where there is no email collapses every caller into one shared bucket — so this endpoint gets a separate limiter keyed on req.ip, which is the real client address because trust proxy is already set. Recorded as rejected: an outermost boundary around the providers, which would sit outside ConfigProvider and need a second hand-styled fallback for a case that is remote — their render bodies are state and JSX with no data mapping. Flagged for revisiting if that stops being true. Also recorded: the rate limiter is deliberately not asserted in the integration suite, because its store is process-wide and a test that exhausts the allowance leaks into every later test keyed on the same address. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalamb
|
261d087a9c |
docs(ci): record the coverage pipeline contract and the CI identity gap
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. |
||
|
bermudalamb
|
7e4084a65f |
docs: design for importing test coverage into SonarQube (#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 |
||
|
bermudalamb
|
c058b3ed2e |
feat(ci): add ESLint to both workspaces with a defect-only CI gate (#60)
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 |
||
|
bermudalamb
|
3cb6a42fb3 |
docs: unwrap the ESLint design doc (#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 |
||
|
bermudalamb
|
8e859e58ec |
docs: design for adding ESLint to both workspaces (#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 |
||
|
bermudalambandClaude Opus 5
|
e40a3d5e1a |
docs: archive the categories and tags design mockups (#23)
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> |
||
|
bermudalambandClaude Opus 5
|
d28fb5634a |
feat(ui): storefront filters and admin category/tag management (#23)
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> |
||
|
bermudalambandClaude Opus 5
|
766358a9fe |
docs: design spec for categories and tags (#23)
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> |