6d320fd867294f58e782d64ed482de608e37d84d
15
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 synAdminandClaude Opus 5
|
6d320fd867 |
feat(passkeys): schema, dependency and per-environment Relying Party (#37)
Groundwork only. Nothing reads any of this yet, and no behaviour changes. The Relying Party ID is derived from PUBLIC_URL rather than written down, because it is the one value in this feature that cannot be corrected afterwards: a credential is bound to it permanently, and a wrong one surfaces only as a customer unable to sign in with a passkey that no longer matches anything. PUBLIC_URL is what every customer-facing link is already built from, so the ID is correct wherever those links are, and wrong only where they were already wrong. hostname rather than host, so a port cannot reach an ID that must not contain one. Local development is the exception the issue's table did not cover. envValidation requires PUBLIC_URL only when SMTP is configured, so a local setup that cannot send mail legitimately has none and falls back to localhost, which browsers treat as a secure context. Two origins there rather than one: the app is served by Vite on 5173 during development and by Express on 3000 once built, and those differ only by port, which is not part of the RP ID. The challenge table is separate from customer_tokens, and the reason is structural rather than preference. customer_tokens.customer_id is NOT NULL, and an authentication challenge is issued before anyone is identified — a discoverable-credential sign-in has no customer to attach to at the moment the challenge exists. Storing it there would mean making that column nullable for every other kind of token. Two of the issue's open decisions are deliberately not made here, because they belong to the ceremony that enforces them rather than to the schema. What to do when the signature counter fails to increase is #39's: many synced passkeys report zero forever, so treating a non-increase as cloning is wrong for them and right for a hardware key, and this only has to hold the value. Whether a disabled account can authenticate is also #39's, and the schema takes the position that it should not cost the customer their devices: credentials survive disabling and are refused at the ceremony, so re-enabling does not mean re-registering everything. Deletion is different and is settled here — credentials cascade with the customer, since one outliving its owner could authenticate as an account that no longer exists. signature_counter is BIGINT because the spec allows a 32-bit unsigned value, which overflows a signed INTEGER at half its range. That is the first bigint column in this schema, so the generated mirror gains the Int8 alias with it. Both tables are added to resetDb's TRUNCATE list and to REQUIRED_TABLES, and the schema mirror is updated by hand to match what kysely-codegen emits — placement and all, so a real regenerate produces no diff. Skipping either is how #56 turned a green local run into a red main; the mirror drift guard exists precisely to catch it, and schemaLoss's count moves from 18 to 20 with them. Verified: tsc clean for src and tests, lint 0 errors with no new warnings, 494 unit tests across 34 suites including nine new ones for the RP derivation, frontend build green, and the migration parses. Not verified: the migration has not been run against a database, and the integration suite needs one this machine cannot provide. Worth knowing before this goes further: #313 changes the domain, and every passkey registered before that cutover stops working at it. This code needs no change — it follows PUBLIC_URL — but the credentials do not survive. That is free while production is not live and nobody holds one, and it stops being free the day the shop opens. Closes #37 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
0c297f8465 |
docs(ops): feature flags are not worth a manager here (#318)
The spike asked whether this project should adopt feature flag management and whether a free option fits. The answer is that it already has two flag mechanisms, uses both idiomatically, and is not short of a third. Environment variables do per-environment gating, with a consistent and documented convention: unset means the feature does not exist and the application is working rather than broken. QA runs with several deliberately empty, which is how it cannot reach PayPal, cannot email anyone and cannot report browsing into the live Brevo account. That is a feature flag system; it is simply not called one. admin_settings does runtime tuning by whoever runs the shop, and its own header already states the principle a flag manager would sell: a setting is changed by the person running the shop rather than the person deploying it. drafting_model and intake_notify_email live there for exactly that reason. What is missing is one value type. admin_settings supports hours, count, text and choice, and has no boolean — so the single thing a manager would add that this project lacks is an admin-flippable on/off, in a store that already does everything else. That is a definition entry, a parse branch and a control. It is not a reason to run another service on a NAS that already hosts Gitea, the runner, QA and production. No tool was compared until that was established, because a spike that starts by comparing platforms will always find one. The comparison is included for whenever the answer changes, with the constraint that matters most stated plainly: fail-safe behaviour beats features, and a flag service that takes the shop down when unreachable is worse than no flags. admin_settings reads from the database the application already cannot run without, so it has no failure mode of its own. The issue said a spike that cannot name a concrete change needing a flag should conclude not yet, and it cannot. Seven of the nine open issues are the Passkeys epic, which is naturally incremental and gates itself by ordering, and #313, whose trust proxy change cannot be flagged at all — it is only correct once Cloudflare is in front, so a runtime toggle would let it be wrong on purpose. The strongest hypothetical is a kill switch for passkey login, because #41 records it as the one feature that cannot be fully proven in QA, and that is still not enough while passkeys are not being built and production is not live. The conditions that would change the answer are written down rather than left implied, so this does not get reopened from scratch. Unleash carries a caveat worth keeping: its OSS Edge sunsets on 2026-12-31, which makes a free tool acquire a paid dependency inside a year. Closes #318 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
4b1ffffc25 |
fix(admin): refetch Inventory when its tab becomes visible (#327)
Two items published from the Review queue did not appear in Inventory. Publishing was never at fault: the items were published, the rows were right, and the API returned them. The Inventory tab was showing a list it had fetched earlier and never refreshed. antd keeps a tab pane mounted once it has been rendered, and Inventory is the default tab, so its pane mounts at page load whether or not anyone looks at it. Its only fetch runs from an effect depending on the filters, so it fires on mount and when a filter changes and at no other time. Open the admin, switch to the Review queue, publish, switch back, and nothing has re-run — the table still holds the list built before the submissions existed. A browser reload shows them, which is what makes this read as publishing being broken rather than as a stale table. The tab is controlled now and tells Inventory whether it is the one on screen, and Inventory refetches when that becomes true. Guarded on visibility rather than fetching unconditionally, because the pane lives for the life of the page and would otherwise keep refetching while hidden. destroyInactiveTabPane on the Tabs would also have fixed it, by remounting, and was rejected: it discards every tab's state on every switch — filters, scroll position, a half-filled form — and refetches all of them repeatedly, which is a much larger behavioural change than this bug is worth. The existing publish test passes against the broken behaviour, and that is the reason this went unnoticed. It asserts against GET /api/admin/items, and the API was always correct. The new test asserts through the UI and never reloads the page: Inventory is opened before publishing, so its list is fetched while the item still carries its submission-timestamp name, and the assertion afterwards looks for the name given at publish — which a stale list cannot contain. A reload anywhere in it would make it pass against the bug it exists to catch. AdminPage's comment claimed only the active panel is mounted. It is only the active panel that is visible; the rest stay in the DOM. That mistaken belief is the shape of this bug, so the comment now says which it is and why it matters. Not fixed here: Categories, Tags, Upload links and Customers are mounted-once children of the same Tabs and are all changeable from elsewhere, so they very likely share this. Recorded on the issue rather than assumed to be fine. Verified: tsc clean for src and tests, lint 0 errors, production build green. The new test needs CI to run — the e2e suite wants a browser and a database this machine cannot provide. Closes #327 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
2bc9440b38 |
test(ci): record which Postgres actually answered (#154)
This adds the measurement #154 has needed twice and never had. It does not fix the failure and does not guess at it: the issue has been wrong twice from reasoning ahead of evidence, and the point here is to make the next occurrence answer the question rather than reopen it. The observation that rules out every explanation so far is that the schema comes back. A suite fails because orders does not exist, and a later suite truncating that same table passes. A dropped database does not un-drop itself, so this was never one database losing its schema. More than one server answering to one name produces exactly this, and Docker embedded DNS round-robins every container sharing an alias, so a leftover service container from an earlier run fits every observation including the empty dmesg that killed the OOM theory. globalSetup now logs every address the database host resolves to. More than one is the answer outright. One address means this reading is wrong too, and the next suspect is a single container restarted with a fresh data directory. Alongside it, both globalSetup and a failing assertSchemaPresent record which server actually answered. pg_postmaster_start_time is what settles that and needs no special rights: two Postgres instances cannot share one, so differing values within a single run are proof, where a differing inet_server_addr alone could be argued to be one container that moved. The failure message now says to compare the two rather than leaving the reader to know that is the interesting comparison. Logged on a passing run as well as a failing one, deliberately. A failing run's addresses mean nothing without a passing run's to compare them against, and this issue has twice suffered from having only the failure to look at. Neither can throw. A diagnostic that fails the run it was added to explain is worse than no diagnostic, so both are wrapped and both degrade to a printed reason. The failure path costs one extra round trip, taken only when the schema is already known to be missing. assertSchemaPresent is not on the hot path — resetDb calls it only when its TRUNCATE has already failed. Verified: tsc clean, typecheck:tests clean, lint 0 errors with no new warnings, and the four message patterns schemaLoss.integration.test.ts asserts on are all still present. The probe reads only pg_catalog functions, so it still answers against the dropped schema that suite creates. Refs #154 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
d7bfd47797 |
chore(ci): a manually-run workflow to delete old Actions runs (#324)
Gitea 1.27.3 expires a run's logs and artifacts but never the run record itself, so the Actions list grows without limit and fills with entries whose logs are already gone. Clearing it meant 455 API calls from a scratch file on one machine, which is the wrong home for something that has to happen again every few weeks. Dry run unless apply is typed as true. The operation cannot be undone and there is no confirmation once it starts, so the harmless answer has to be the default rather than the one you get by leaving a box alone. Manual only, deliberately. The list is an annoyance rather than a problem, and a cron that quietly deletes history deserves to be a decision taken on its own rather than one that arrives bundled with the tool. A run that is not completed is never a candidate, which is also what stops the cleanup deleting the run it is executing in. Ages a run by started_at, falling back to completed_at. That fallback is the whole reason this is worth committing rather than repeating from memory: a run cancelled before it ever started reports an epoch started_at while carrying a real completed_at, so reading only the first makes every cancelled run look undateable. The manual pass did exactly that and left nineteen runs from three weeks earlier in a list that was supposed to hold seven days. Neither timestamp usable still means keep — an epoch read as 1969 would delete the runs that have not happened yet. Uses a dedicated ACTIONS_CLEANUP_TOKEN secret rather than the automatic per-job token, since deleting a run may be beyond what that token permits. If it turns out to be enough, the secret and the env line both go. Host and repository come from the run's own context, so the file carries no hostname and survives the move #313 may yet make. No npm install: the script uses node's own https module, so there is nothing to fetch and nothing to break when a dependency moves. Verified by running the script against the live instance: a dry run reported 19 stale cancelled runs the earlier pass had missed, and applying it removed them with no failures. Closes #324 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
5bba28bfda |
fix(tests): the error-handling suite would not compile, and nothing local said so (#307)
The trigger added in #322 does not typecheck. pg declares query with several overloads and jest.spyOn resolves the mock argument against the last of them, whose parameter list is empty, so the inferred type of a rejection value is never and an Error cannot be assigned to it. ts-jest compiles each suite as it runs, so this surfaced as a suite that failed to run rather than as a test that failed — which is why CI reported 446 tests passing, zero failing, and the step red anyway. The cast is the type system rather than a shortcut, and says so in the file: every overload rejects on failure, and the cast only chooses which one to check against. The reason this reached main is the part worth keeping. The backend has a tsconfig.test.json covering scripts and tests, and nothing was running it — build compiles src alone, and I had been reporting tsc clean on that basis while touching test files it never looked at. The frontend build has run its equivalent all along, so the gap was one workspace wide and invisible from the other. It is now a script, typecheck:tests, and it reproduces this failure in seconds against no database. That matters beyond this fix: the standing excuse for integration regressions here has been that the suite needs Postgres and a Node this machine cannot run, and a type error in a test was never actually in that category — it only looked like it because the check that finds it was never invoked. Not added to build. The Dockerfile runs that, and a production image should not fail to build because a test file has a type error. Verified: typecheck:tests clean, build tsc clean, lint 0 errors with no new warnings, 485 unit tests passing. The suite itself still needs CI to run. Refs #307 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
8956b9f122 |
fix(items): read the id strictly, and stop the error test needing a route that does not (#307)
Item 5. GET /api/items/:id was the last route reading its id with a bare Number(), so an unreadable id reached Postgres and came back to the caller as a 500 for an item that cannot exist. It answers 404 now, like every other id-taking route since #207. It could not be fixed on its own, which is why it stayed. errorHandling.integration.test.ts used this route's looseness as its way of making a handler reject: tightening the parse would have left that test green while removing the thing it tests. So the test now fails a database call directly, with a spy on pool.query against a route that makes one. That is the failure the error middleware actually exists for, and it does not depend on any route declining to validate — the previous comment's own conclusion, that moving the trigger to cart.ts would only move the wart. Two things fixed along the way that the issue asked about but that switching to readId would not have delivered on its own. readId was not as strict as its name suggests. Number reads 5.0, 1e2, 0x10 and +5 as 5, 100, 16 and 5 — every one a positive integer, so every check readId made passed and the route fetched a real row for a URL nobody wrote. /items/5.0 answered with item 5. This never raised an error and so never announced itself; the issue noticed it only because #308 converted the comparison to a real integer. An id is a string of digits, so it is matched against digits before being parsed. readId is also now bounded at the top of a 32-bit serial. Above that Postgres raises 22003 rather than returning nothing, which is the same wrong answer to the caller as the 22P02 the function was written to prevent — a 500 for an id that identifies nothing. The leak assertion was passing for the wrong reason. It checks the response does not contain "syntax" or "items", and the error it was checking against happened to contain both only by accident of which route was used. The injected failure now contains both words deliberately, and the whole message is asserted against as well, so a future error format cannot slip through by wording itself differently. Verified: tsc clean, lint 0 errors with no new warnings, 485 unit tests passing across 33 suites — seven of them new, covering the inputs above. The integration suite cannot run on this machine, so whether the rewritten error test passes is for CI to say. Item 4, coverage, is not closed by this and cannot be closed yet: the SonarQube scan step has been skipped on every recent run because it is gated on the earlier steps succeeding, and those steps were failing. The dashboard is therefore stale. Reported on the issue rather than guessed at. Refs #307 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
5b40ac25db |
fix(tests): regenerate the schema mirror and name the controls #56 moved (#320)
PR #317 was merged while its run was still failing, so four failures landed on main: one integration and three end-to-end. All of them are consequences of #56, and none could have been caught on the dev machine, which has no database and no browser to run those suites with. The schema mirror was never regenerated. The migration added three columns to customers and src/db-kysely/schema.ts still described the table without them, which is the drift guard from #305 doing exactly what it exists for. Hand-edited to match what kysely-codegen emits — alphabetical, and Generated on the column that has a default — because regenerating properly needs a live database. The other three are the same mistake three times: a control addressed by position, and the position moved. An unscoped getByRole('checkbox') became ambiguous once the register form had two consents. A toHaveCount(2) on the account modal's switches became three. And favoriteAlertsSwitch was getByRole('switch').last(), which did not error when a switch was appended below it — it silently retargeted, toggled analytics consent instead of favourite alerts, and then failed on a text assertion in favorites.spec.ts, naming neither the file nor the control actually at fault. The reason position was ever used is that antd's Switch renders a bare role="switch" with no accessible name; the adjacent Text is a sibling, not a label. So each one now carries an explicit aria-label and is addressed by it. That is what makes them addressable from a test, and it is what a screen reader needed regardless — the fix and the accessibility improvement are the same change. The count assertion stays, but alongside naming each switch, because a count on its own would pass if two of them were swapped for each other. Two coverage gaps closed while here, both properties the compliance work in #56 depends on and neither previously asserted anywhere a customer could see: the analytics checkbox is unchecked on the register form, and the account toggle is off for a new customer. Quebec's Law 25 s.8.1 requires profiling to start off, the integration suite asserts the server half of that, and nothing asserted the half rendered on screen. The fourth Playwright entry, the logged-out header surviving a reload, is reported flaky rather than failed and passed on retry. Left alone; it is unrelated to #56 and #257 covers flakes in this suite. Verified: backend tsc clean, frontend tsc against the test config clean, production build green, both lint suites 0 errors, 478 unit tests passing. The integration and e2e suites still cannot run here, so whether this actually clears run 875's failures is for CI to say — which is the same gap that produced them. Closes #320 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
218be6d298 |
test(e2e): make the resend allowance failure say why (#257)
The test asserted on toasts, and toasts were the wrong instrument twice over. It failed roughly one full-suite run in three with a strict mode violation — getByText(/already sent several/) resolving to three refusal toasts where one was expected — and two investigations could not establish the mechanism. The second investigation corrected the arithmetic the first depended on: antd toasts auto-dismiss, so the number visible at the moment of an assertion is a lower bound on how many refusals happened rather than a count. Three visible refusals is equally consistent with four where the first had already faded. That removed the only evidence anyone had for the original theory that the customer's bucket held two hits before the test clicked anything, which left the issue with a symptom and no way to read it. So this asserts the sequence of response statuses instead. Toasts are a lossy, timing-dependent rendering of the thing the test is actually about, and the responses are the behaviour itself. A failure now reports what happened: four 429s means the bucket really did carry hits from somewhere else, while more than four entries means the UI sent more requests than there were clicks. Either reading identifies the mechanism from one failing run, where before it needed a temporary probe re-added and the suite run until it failed again. Each click now waits for its own response. The previous "await expect(resend).toBeEnabled()" looked like pacing but was a no-op, since the button is never disabled, so four requests raced. Removing that ordering means a failure cannot be blamed on it. The copy assertion stays, because it is what the test is named for, but scoped with .first() so strict mode does not treat several identical toasts as ambiguous. This does not fix the underlying flake, and is not meant to. The issue asks for the mechanism to be found before a fix is attempted rather than guessed at, and nothing here changes the limiter or the store. Verified by typecheck and lint only. The e2e suite needs a database and a browser this machine cannot run, so whether this passes is for CI to say — the same gap that let the integration regression through earlier today. Refs #257 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
e7196f440b |
test(customers): let the public shape guard see analytics_consent (#56)
The register route now returns analytics_consent, and customers.integration.test.ts asserts the exact key set the public customer shape may contain. That test failed in CI, which is the guard doing its job rather than a problem with it: its whole point is that the shape cannot quietly grow, and a field appearing without someone deciding it belongs there is what it exists to catch. This field does belong there, so the expected set gains it. Three cases added while here, all of them properties the compliance work depends on and none of them observable from a unit test. Analytics consent is off for a registration that does not mention it, which is what Quebec's Law 25 s.8.1 requires and needs the column default, the register route and the stored wording to agree. Opting in to marketing alone leaves analytics off, which is the bundling GDPR treats as invalid and the mistake this branch already made once. And an analytics-only opt-in works with marketing left off, so the granularity holds in both directions rather than only the convenient one. Found by CI rather than locally: the integration suite needs a database this machine has no Docker to run, which was called out as unverified when the change went up. Typechecked, linted and the 478 unit tests still pass, but the assertion itself is only proven by the next CI run. Refs #56 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
25bec50902 |
docs(privacy): disclose every cookie and browser-storage item (#56)
Completes the consent picture the rest of this branch builds. The claim worth being able to check is that no cookie needing permission is set before it is asked for, so the policy now lists everything rather than asserting it: the rd_session sign-in cookie, which is strictly necessary and therefore exempt, the two preferences kept in localStorage and never sent anywhere, and Brevo's cookie, which cannot exist unless analytics consent was given because the script that would set it is never loaded otherwise. No cookie banner, and that is a finding rather than an omission. ePrivacy requires consent before storing anything non-essential, and this application does not store anything non-essential until the customer has asked for the feature that needs it. A banner would be asking permission for things that are either exempt or already separately consented to, which teaches people to dismiss the one consent that does matter. Described in terms of what each thing does rather than by category, since a list of cookie names tells a customer nothing they can act on. Refs #56 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
955049eac9 |
fix(privacy): separate analytics consent from email consent (#56)
The previous commit widened the marketing consent sentence to cover the Brevo tracker, so one checkbox carried both purposes. That is the specific pattern GDPR rejects: consent has to be granular, and current EDPB guidance treats bundling tracking consent with subscription consent as invalid because the customer cannot accept one purpose and refuse the other. Quebec's Law 25 s.8.1 is stricter again — profiling technology has to be off until the person switches it on, with no pre-ticked box and no consent inherited from agreeing to something else. Building to both standards was the decision, since the storefront is publicly reachable and anyone can register. So the marketing sentence is restored to exactly what it was, which leaves every existing email consent valid and untouched, and analytics gets its own column, its own sentence, its own checkbox at registration, its own toggle in the account page and its own endpoint. A customer can now hold either, both, or neither, and withdrawing one does not disturb the other. The migration defaults analytics_consent to false, which is both the honest answer — none of the existing customers was ever asked — and what Law 25 requires. Nothing about this change opts anybody in. Two details that are compliance requirements rather than wording preferences. The sentence names Brevo instead of saying "our email provider", because informed consent means the customer can tell who receives their data and a description they cannot act on is not disclosure. And the account toggle is as prominent and as easy to switch off as it is to switch on, because withdrawal has to be as easy as consenting. The analytics endpoint is separate from the marketing one rather than a second field on it, so that a single call cannot change an answer the customer did not touch — the bundling problem moved from the form into the API. The unit tests now assert the two consents stay apart in both directions, including that the marketing sentence still says nothing about tracking, because re-bundling them would otherwise pass silently and is the mistake this project already made once. Verified: backend tsc clean, both lint suites 0 errors with no new warnings, 478 unit tests passing across 33 suites, frontend production build green. Not verified: the migration has not been run against a database, and integration and e2e need a Node this machine does not have active. None of this is legal advice and the wording is worth a lawyer's eye before it ships. Refs #56 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
ac3f6e91f5 |
feat(analytics): report consenting customers' activity to Brevo (#56)
Loads Brevo's web tracker for a signed-in customer who has consented, reports route changes as page views, and tracks the three events the issue asked for: added_to_cart, favorited, and checkout_completed. The four design questions were settled on the issue in August and this implements those answers. The consent gate is the part worth reading. The decision recorded on the issue was "gate it behind consent", but the sentence customers actually agreed to named only email: "I want to receive occasional emails about new one-of-a-kind items". Gating a tracker on `marketing_consent` while that was the stored wording would have treated "email me about new items" as authorisation to send someone's browsing to a third party, which it does not say — and this project stores the wording verbatim against each customer precisely so that a record says what the customer saw. So the sentence is widened here, and the tracker is gated on `analytics_consent`, a field the server computes by comparing the wording stored against a customer with the current constant. Changing the sentence therefore does not retroactively widen anybody's consent: everyone who agreed to the old text keeps their email consent and is not tracked until they re-consent through the account page. A boolean alone could not tell those two populations apart, which is the whole reason the text is stored per customer. `analyticsConsent` is exported and has its own unit test, because "agreeing to the old wording does not authorise tracking" is the rule that silently tracks people if it regresses — their flag really is true. QA stays out of the live Brevo account by construction rather than by remembering. The key is per-environment, the tracker never loads without one, and `docker-compose.qa.yml` sets an empty literal with no stack variable behind it, so nothing can inherit a value from the host or be pasted in from production's stack. Same reasoning as QA_DB_PASSWORD and the QA_SMTP_ names beside it. Events are reported from the API layer rather than the UI call sites, so no caller can add to the cart or favorite an item without it being counted, and each fires only after the response was accepted — a refused add is not reported as one. The two checkout completions each name their processor, because a demo purchase charges nothing and counting it as a sale would overstate revenue. The privacy policy gains an analytics section in this change rather than a follow-up, since the published policy previously described none of this and would otherwise have lagged the code. It is deliberate about the limits: withdrawing consent stops further reporting, but anything already sent stays with Brevo, and a script already injected cannot be un-injected — `stopBrevoTracking` stops calls, it does not unload sa.js. That is said in the code too, because "tracking stops" reads as a stronger promise than any web tracker can make. Verified: backend tsc clean, both lint suites 0 errors with no new warnings, 474 unit tests passing across 33 suites, and the frontend production build green including the compose-environment guard. Not verified: integration and e2e, which need a database and a Node this machine does not have active, and no real Brevo key was exercised — the tracker has never been observed reporting to an actual account. Closes #56 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
2866901187 |
fix(admin): keep a modal's controls on screen on a phone (#314)
antd treats a Modal's width as a fixed pixel value and does not adapt it to the viewport, so the item editor's `width={720}` in Admin.tsx still laid out at 720px on a roughly 390px screen. The footer's OK and Cancel are right-aligned inside that width and the close X sits in the modal's top-right corner, so all three ended up off-screen, and antd sets `overflow: hidden` on the body while a modal is open, so the page behind could not be scrolled to reach them either. The dialog had no way out short of the browser's back button.
Capped in styles.css rather than at each call site, because `Modal.confirm` — used by the review queue when publishing at a price nobody chose — has no call site to edit and would have been left broken by a per-modal fix. The same rule covers the 640 and 480 widths in Customers.tsx and the preview drawer, all of which overflow a phone for the same reason. `max-width` beats the inline `width` antd writes on the element, so none of it needs `!important`.
Bounding `.ant-modal-body` rather than the modal is what actually keeps the buttons reachable: the footer is a sibling of the body, not a child, so a tall form scrolls inside the modal while the footer stays where it is. Sizing the modal alone would have moved the overflow rather than removed it.
Verified in the built bundle rather than only in source — `@media (max-width: 767px)` and all three rules are present in dist after a production build, which is the step that distinguishes a fix that shipped from one that merely compiled. Not verified visually on a device: this is a layout change with no unit coverage, and the five overlays worth checking at 390x844 are listed on the issue.
MDEditor's `preview="live"` still splits the pane in two and is cramped at this width. That is a behaviour change rather than a layout fix and is deliberately left out; it is recorded on the issue instead.
Closes #314
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
synAdminandClaude Opus 5
|
3e3932b231 |
docs(ops): the free tier cannot be applied to this hostname (#285)
The spike asked what Cloudflare's free plan would and would not do for a self-hosted site on a dynamic-DNS hostname. The answer is narrower than either option the issue anticipated: the free plan supports only full setup, which requires delegating the zone's nameservers at its registrar. The zone here is synology.me and belongs to Synology, so we cannot delegate it. Partial (CNAME) setup is Business or Enterprise, and subdomain NS delegation is Enterprise. The blocker is therefore not that the useful features are paid — it is that the hostname cannot go on the plan at all. The issue asked for exactly this kind of finding to be recorded rather than treated as a reason to widen the spike. The remaining questions are answered anyway, on the assumption that a domain we control is bought later, so that decision is made with the consequences already known rather than discovered afterwards. Two of those consequences are worth pulling out. Cloudflare appends to X-Forwarded-For rather than replacing it, so a second proxy in front of Nginx Proxy Manager makes `trust proxy: 1` in app.ts resolve req.ip to a Cloudflare edge address instead of the client. Four limiters key on req.ip and would silently stop distinguishing callers while continuing to look healthy, which is the dangerous direction for that failure to go. Separately, Bot Fight Mode cannot be skipped with WAF or Page Rules on any plan because it does not run on the Ruleset Engine, and Cloudflare documents that it may challenge API traffic — a challenged POST /webhooks/paypal is a capture notification we never receive. Everything asserted about this repository was checked against the code; the Cloudflare behaviour was checked against their documentation on 2026-09-05 and not against a live account, because one cannot be set up here. What was not established is listed at the end of the document rather than left implied. Closes #285 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |