022ab8edcf2ad861fbc18257b6cde964c9c29b52
8
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 synAdminandClaude Opus 5
|
022ab8edcf |
fix(local): install dependencies when the lockfile changes, not only when node_modules is absent
Starting the stack locally failed to build with four copies of
error TS2307: Cannot find module '@simplewebauthn/server'
naming a package that is right there in package.json. That reads as a broken checkout rather than a missing install, which is why it costs more than it should.
The cause is one line in Install-IfMissing. It asked whether node_modules existed and returned early if it did, which is true for anyone who has ever run the script. So a branch that ADDS a dependency never installs it: the pull brings a new package.json and a new lockfile, the script says dependencies already installed, and the build then fails on an import the source is entirely right to make.
The passkeys work is what surfaced it, adding @simplewebauthn/server to the backend and @simplewebauthn/browser to the frontend, but nothing about it is specific to those. Any dependency added on any branch would have done the same, and the failure would have looked equally unrelated to its cause each time.
It now compares timestamps instead. npm writes node_modules/.package-lock.json describing exactly what it put there, so holding that against package-lock.json answers the question actually being asked: is what is installed what is currently asked for. A pull that changes dependencies makes the lockfile newer and this notices; a pull that does not leaves the check skipping the install exactly as before, which is the whole reason the check exists.
Both branches were exercised against the real working tree: stale before installing, up to date after.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
 bermudalambandClaude Opus 5
|
4e2357356b |
fix(scripts): make the database the e2e suite reads a recorded fact (#273)
The end-to-end helper opens its own connection to read a password-reset token, and nothing guaranteed it pointed at the database the application was actually using. With -E2eDb the app runs on redefined_e2e at 55501 while the helper kept its redefined_local default at 55500, so the app wrote to one database and the suite read another — and the specs failed for a reason that had nothing to do with them. start-local.ps1 now records the coordinates it chose in .local/database.json, and run-tests.ps1 reads them into TEST_PGHOST, TEST_PGPORT, TEST_PGUSER, TEST_PGPASSWORD and TEST_PGDATABASE before Playwright starts. The answer now comes from one place, written by the thing that made the decision at the moment it made it. It is written after Start-Database rather than before, so the file never names a database that failed to come up, and removed by -Stop, so a stopped stack does not leave a record pointing at a container that is gone. Setting all five closes the second fault in the same change. Invoke-IntegrationSuite sets TEST_PGPORT and PowerShell keeps it for the rest of the session, so a -Suite all run leaked the integration port into the e2e run that followed — with none of the matching credentials, leaving the helper offering redefined_local's password to the integration database. Overwriting every one of them is what makes that leak harmless. A missing record throws rather than falling back. A default is what produced both faults in the first place: always plausible, silently wrong, and it fails in ways that look like application bugs rather than configuration. The guard test is the point of the change as much as the fix is. This is the third instance today of two files having to agree with nothing comparing them — #107 and #118 were envValidation against a compose file, #287 was the workflow against start-local.ps1, and this is three files rather than two. The test pins the whole chain: that the writer records the five settings, that the runner reads each of them from the record rather than a default, and that the helper reads no connection variable the runner does not set. Removing a single line from the runner fails four of its assertions, which was checked rather than assumed. What it cannot do is run PowerShell, so these are text assertions against the scripts. That is weaker than executing them and still catches the drift that actually happened. Closes #273 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
1902db6d04 |
feat(intake): offer background removal on the submission page, ticked (#281)
Adds a checkbox to the public submission page that lets a sender opt out of background removal, ticked by default because most items look better cut out and the reverse default would mean almost nobody got it. It only renders when the server reports the sidecar is configured, matching the intake link's new backgroundRemoval flag from Task 5 — an unconfigured environment gets no checkbox rather than one that would do nothing. submitItem now takes removeBackground as a required fourth parameter, sent as the multipart string 'true' or 'false' to match the backend's exact-string opt-out contract. Making the parameter required rather than optional was deliberate, so the compiler would catch any call site left unupdated; the frontend build (which also type-checks tests/ via tsconfig.test.json) confirmed the only call site, in Submit.tsx, was updated. scripts/start-local.ps1 now sets REMBG_URL for the local backend so the checkbox is visible during local and e2e runs; the value need not resolve, since no e2e submission reaches the sidecar without a configured drafting step. Adds two e2e cases to intake-submit.spec.ts: the checkbox appears ticked by default, and a sender can uncheck it and still submit successfully. Both are written per the task-7 brief but not run in this session, since running Playwright requires the full local stack (database, backend, frontend dev server) which was not started. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
175d21d11c |
fix(scripts): refuse to reuse a backend that is serving the previous database (#257)
start-local.ps1 said "something is already listening on 3000; leaving it alone" and carried on. That is safe only while the database has not changed underneath that process. When it has, the old backend is serving a database it no longer owns and its in-memory state describes rows that no longer exist. The rate limiter is the sharpest example. It keys on customer id and keeps buckets in memory for an hour, so a recreated database restarting ids at 1 hands a brand-new customer a previous run's spent allowance. That is #257: the resend-verification allowance test failing roughly one full run in three, never in isolation, with three refusal toasts where one was expected. Reproduced deterministically rather than reasoned about. Fresh database and fresh backend: three of three pass. Recreate the database only, leaving the same backend running: the same test fails with exactly the reported "resolved to 3 elements". Control — recreate the ids again but restart the backend as well: passes. So the variable is the process outliving the database, not the id restart on its own. That also explains why #257 could not find the mechanism. It had ruled out contention, a mis-keyed limiter, a shared fixture and identity reuse in the test helpers, all correctly. The recycling happens outside the suite entirely, in a process the suite never sees. Now it refuses, names the reason, and says to run -Stop. A run that stops loudly is recoverable; one that quietly tests the wrong thing is not — and a stale listener on 3000 has already produced two wrong measurements in this project, a rate-limiter reading and an e2e run reported as 23 passed when the backend was talking to a deleted database. DatabaseIsNew is set when -Fresh removes the container, when the container is created, and always under -E2eDb, whose tmpfs storage means it comes up empty whether created or restarted. Verified by AST-parsing the script and confirming every reference to the flag is script-scoped — a function-local read would see $null and the guard would never fire. The script is deliberately never executed from an agent shell. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
6a90e957f2 |
test(e2e): give the suite a throwaway database of its own (#186)
The e2e suite ran against the development database and nothing truncated it. Every run seeded more fixtures and left them, so the unfiltered storefront grew monotonically — 1,662 items by the time #186 was filed — until rendering it outran the assertions' timeouts. It failed locally, passed in CI where the database is fresh, and got steadily worse, which is the combination nobody can act on. start-local.ps1 -E2eDb runs the stack against a separate container on a separate port with tmpfs storage, so it starts empty every time. Migrations already run on every start, so an empty volume is a working one. The development database is untouched, so anything set up there by hand survives. Deliberately a third database rather than sharing either existing one. The integration suite truncates between tests, so an e2e run sharing with it would have its fixtures deleted underneath it (#116) — different container, different port, different credentials, so the mistake is impossible rather than discouraged. Verified by recreating the database from the compose file, confirming it came up with zero items, and running the full suite against it: 157 of 157. An earlier attempt at this reported 23 passed and 53 not run, which was worthless — a stale backend from a previous run still held port 3000 and was talking to a database I had already deleted. The port is checked before the run now, and the same mistake produced a wrong rate-limiter measurement earlier in this work. Pagination is the other half of #186 and is filed separately: a shop that renders its whole catalogue in one page is worth fixing on its own merits, not as a side effect of a test fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
06933aec75 |
fix(scripts): make the alias check fail closed, and correct three stale docs (#208)
The alias check was a negative match on an allowlist of English error strings, which returned True for empty output, for $null, and for "exit status 1: Access is denied." — so an alias switch producing nothing, or failing on the symlink permission error this file's own header warns about, was reported as success while the old version kept running. That is the bug #198 was filed about, narrowed rather than removed, and it also broke whenever nvm reworded an error. It is now a positive match on "Now using node v<what is actually running>". The floor check moves ahead of the switch and reads the constant rather than the result. Where it sat, $major was always whatever NODE_VERSION says, so it validated the switch it had just made instead of the pin it exists to guard, and could never fire. Use-NodeLatest is now Use-PinnedNode. In a change whose whole subject is that "latest" means something people do not expect, the name was an avoidable trap. The restore default moves beside NODE_VERSION. It deliberately is not a param default: a param block runs before the dot-source, so $script:DEFAULT_NODE_VERSION is still $null there and the restore would have quietly restored nothing — leaving the machine on the pinned version, which is the exact failure the restore exists to prevent. It is resolved after the dot-source instead, and an explicit -DefaultNodeVersion still wins. Three documents described behaviour the code no longer has: README's "both scripts run nvm use latest", run-tests.ps1's .DESCRIPTION, and project-context.md's instruction to agents. All corrected, and project-context.md now also says not to run these scripts from an agent shell, which is how this machine once ended up with no Node at all. Part 4 of the issue is partly stale: backend/package.json already declares engines >=20.9.0. frontend now matches it. The larger question — whether local should be pinned to the Node 20 that CI and the production image actually run — is a decision rather than an oversight and is left open on the issue. Verified by parsing all three scripts with the PowerShell AST parser, which does not execute them. They are deliberately never run from an agent shell. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalamb
|
9ac2fa3ba1 |
feat(scripts): switch Node automatically, and add a test runner (#140)
start-local.ps1 knew exactly what was wrong when Node was too old and then made you fix it by hand. Assert-NodeVersion read node --version, found a major below 20, and threw a message telling you to run `nvm use 24.13.1` and start again in a new shell. A good error for a problem the script could simply solve — and since nvm's default here is 18.16.1, it was hit on every fresh shell. It now runs `nvm use latest` itself and -Stop puts the machine back to 18.16.1. The revert also runs when a start fails partway: without that, a run dying in migrations leaves the machine switched with nothing started, and the -Stop that would restore it is never reached. nvm rewrites a machine-global symlink rather than changing one shell, so this changes the Node version for every terminal on the machine while a script runs. That is the intent — the point is to work in whatever shell is already open — but it is announced every time rather than done quietly. The switch is verified rather than trusted. nvm-windows exits 0 for switches that did not happen: a version it cannot find, a symlink it cannot rewrite without elevation, and — observed here — a rewrite issued immediately after another one, where the directory symlink is briefly still the old target. That last case turned up while testing this change: `nvm use latest` reported success and left Node on 18.16.1. So the result is read back and retried once, and nvm's own output is captured rather than discarded, because suppressing it hid the only message that explained the failure. run-tests.ps1 runs the suites: -Suite unit|integration|e2e|all. One script with a parameter rather than three, because the version switch, the database bring-up and the TEST_PGPORT handling are shared and three copies would drift. The integration suite gets its own throwaway Postgres started and stopped around it, in a finally so a failing suite still tidies up. The e2e suite checks the backend is answering first and says what to start, rather than leaving twenty-five specs to fail on a refused connection that names nothing. `all` runs cheapest and most isolated first, so a break several suites would show is reported by the one that localises it best. The version switching lives in scripts/NodeVersion.ps1, dot-sourced by both, since two copies of it would drift and the half that drifts is the half nobody runs. Closes #140 |
||
|
bermudalamb
|
40b483fc30 |
feat(scripts): a PowerShell script to start the local environment (#125)
Bringing the app up locally was seven or eight commands in a particular order: a Postgres container on a port that is not blocked, the six environment variables the backend refuses to boot without, migrations, a TypeScript build, the backend, then Vite. None of it hard, all of it tedious, and documented nowhere outside CI workflows written for a Linux runner. scripts/start-local.ps1 does the sequence. It reuses an existing container rather than recreating one, skips npm install when node_modules is already there, leaves alone anything already listening on a port it wanted, and waits on pg_isready and a 200 from /api/config rather than sleeping a fixed number of seconds. -Fresh recreates the database, -Stop tears everything down by the process ids it recorded rather than by port, since killing by port would also kill whatever else happened to be listening. Two things this got wrong first time round, both found by running it rather than by reading it. $ErrorActionPreference = 'Stop' does not stop a PowerShell script when a native executable exits non-zero, only when a cmdlet throws. Every command here is node, npm or docker, so the first run printed a stack trace from a failed migration, carried straight on, and reported a healthy stack sitting on a database with no tables in it. That is the worst kind of wrong: a green summary over a broken environment. Native calls now go through Invoke-Checked, which tests $LASTEXITCODE and throws. The migration failed because node on the PATH was v18.16.1. node-pg-migrate pulls in an lru-cache that calls diagnostics_channel.tracingChannel, which does not exist before Node 20, and the failure surfaces as "(0 , U.tracingChannel) is not a function" from a minified file - which says nothing whatsoever about Node versions. The script now checks the major version first and says what to do about it, so the confusing crash becomes one clear line before anything else runs. The port default is 55500 rather than anything near 55432, which is reserved by Hyper-V on this machine. Docker's message when it cannot bind a reserved port does not mention reservations, so the failure path names the likely cause and prints the netsh command that lists the reserved ranges. Verification, all observed rather than assumed: the version guard was made to fire on Node 18 and produced the intended message. A -Fresh run on Node 24 applied all six migrations, and psql then showed thirteen tables where the broken run had none. /api/config and the storefront both answer 200. -Stop stopped both tracked processes and the container. A second run with the dev server already up detected it and left it alone rather than failing. .local/ holds the logs, pids and uploads, and is gitignored. Closes #125 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |