Fix frontend, backend, and deployment bugs (deep review) - #42
Merged
Conversation
- webhooks: verify GitHub HMAC against the verbatim request bytes via a scoped content-type parser (re-stringified JSON is not byte-identical, so verification failed whenever a secret was configured) - ws: sequence welcome -> missed-message replay so RedisHub deploys can't deliver replayed events before welcome - recordings: require the recordings/ key prefix so the route can't be used to presign arbitrary bucket keys on S3 deploys - tests: re-probe frameable when targetMode flips without a new targetUrl so resolveDeliveryMode stops serving a stale decision - branches: upsertCommit ON CONFLICT DO NOTHING so cross-branch pushes of the same SHA don't clobber the first write's metadata - cors: normalize FOLDO_WEB_ORIGIN entries to URL origins so trailing slashes don't silently CORS-block the canvas https://claude.ai/code/session_01T3cCon5oGQcDKPASj9d8Wu
- comments: allow empty text on POST /api/comments — the pin-drop flow
deliberately creates the comment empty, so every online pin drop 400d
and rolled itself back mid-typing
- ImageFrame: resolve relative /api/uploads URLs against API_BASE; the
web app is a different origin so uploaded images rendered broken
- replies: dedupe comment.reply.added against the REST response (the
server broadcasts to the sender too), authors saw replies twice
- reconnect: rehydrate via patch that preserves presence, dispatches,
activeTestSessions and wsStatus — the wholesale set() wiped remote
cursors permanently, blanked in-flight dispatch UI, and stuck the
status indicator on "connecting"
- comments: roll back the optimistic text patch when the PATCH fails
- tests: handle test.created/updated/deleted broadcasts (previously
silently dropped) via a testsRevision counter that live-refreshes an
open TestsPanel; add reducer exhaustiveness check so protocol/reducer
drift fails typecheck
- plugin registry: make install() idempotent by manifest id so a second
bootPlugins (dev HMR) doesn't double every surface
- canvas: cancel the pending cursor rAF on unmount so we don't
broadcast a bogus {0,0} cursor after teardown
https://claude.ai/code/session_01T3cCon5oGQcDKPASj9d8Wu
- railway.json: Railway config-as-code is service-scoped — the root
file's `services` map is not in the schema and was silently ignored,
so every service built with root Nixpacks instead of its Dockerfile
and the server lost its /health deploy gate. Split into
apps/{server,web,sample-app,shotter}/railway.json (point each Railway
service's config-file path at its file); drop the shotter
startCommand that fought the Dockerfile CMD
- shotter: pin playwright-core to exactly 1.58.0 to match the
mcr.microsoft.com/playwright:v1.58.0-jammy browser revisions — the
lockfile had drifted to 1.60.0, which builds and passes /health but
fails every POST /shot at runtime; switch HEALTHCHECK from wget to
node fetch (wget isn't guaranteed in the image)
- web Dockerfile: declare ARG/ENV VITE_SHOTTER_URL so the documented
shotter activation runbook actually reaches the Vite build (also
typed in vite-env.d.ts)
- post-deploy smoke: FOLDO_PROD_BASE arrives as '' from the default
repository_dispatch wiring; use || so it still falls back to the
prod default instead of producing invalid request URLs
- serve.json (web + sample-app): scope the 1-year immutable rule to
Vite's content-hashed assets/; unhashed public/ files (icons, logos,
marketing images) now get max-age=3600 so a redesign under the same
filename isn't stale for a year — verified live against the built
dist with the exact Dockerfile CMD
- docs: update CLAUDE.md / DEPLOYMENT.md / RAILWAY_READINESS.md /
marketing self-host page for the per-service config layout; remove
stale better-sqlite3 comment
https://claude.ai/code/session_01T3cCon5oGQcDKPASj9d8Wu
Server: - cors: only normalize http(s) FOLDO_WEB_ORIGIN entries — new URL().origin is the literal 'null' for extension schemes, which both broke those entries and allowlisted the dangerous `Origin: null` - webhooks: delegate parsing to Fastify's default JSON parser so the raw-body route keeps secure-json-parse prototype-poisoning protection; drop the dead RawServerDefault hack - branches: restore ON CONFLICT DO UPDATE for commit metadata — in Git message/author are properties of the sha (re-deliveries are identical), and the update is what heals the stub rows seeded by the new-branch route; branch_id intentionally stays first-write - tests: skip the frameable re-probe when a PATCH re-sends the current targetMode (it's a blocking external GET) - ws: issue the two hub reads in parallel and order only the sends — removes a serialized Redis round-trip per reconnect Web: - shared resolveApiUrl() in api/client (honors __FOLDO_API__) replaces three divergent copies of API-relative URL resolution (ImageFrame was also using the wrong API_BASE constant) - idempotent boardStore.addReply() replaces the dedupe convention spread across the reducer and both reply call sites - comment edit rollback restores only the text onto the current store entry, and only if the optimistic text is still showing — no longer clobbers concurrent replies/saves - abandoned pin-drop comments (closed empty, no replies) are deleted instead of persisting as ghost pins, covering both the post-swap and in-flight-create paths - reconnect rehydrate now refetches dispatches (REST list), clears the transient activeTestSessions set (a completed-past-the-replay-buffer session would stick the badge forever), and bumps testsRevision; hydrate/rehydrate share one GetBoardResponse-typed slice builder - TestsPanel: replace the revision-ref dance with a plain dependency- driven effect - plugin registry: replace-by-id on duplicate install so dev HMR gets fresh closures instead of pinning disposed module state Docs: add the Railway config-file-path migration as a P0 action item. https://claude.ai/code/session_01T3cCon5oGQcDKPASj9d8Wu
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A deep bug-hunting pass across the backend, frontend, and deployment configuration, followed by a second adversarial review of the diff itself. ~20 verified bugs fixed across four commits.
Backend (
apps/server)welcomeand be dropped by clients. Reads stay parallel; sends are now ordered./api/recordings/*would presign any S3 key; now restricted to therecordings/namespace.frameablewhen a test'stargetModeflips without resending the URL (re-probed only on a real mode change — it's a blocking external GET).FOLDO_WEB_ORIGINentries are normalized to URL origins for http(s) only — trailing slashes no longer break matching, and extension-scheme entries no longer collapse to the dangerous literal'null'origin.typeof text !== 'string') — required by the pin-drop flow below.Frontend (
apps/web)/api/uploads/...URLs resolved against the web origin. New sharedresolveApiUrl()inapi/client(honors__FOLDO_API__) replaces three divergent hand-rolled copies.boardStore.addReply()used by all three call sites.activeTestSessionsset (a completion missed past the replay buffer would stick the badge forever); bumpstestsRevisionso an open TestsPanel refetches.test.created/updated/deletedbroadcasts were silently dropped; now live-refresh an open TestsPanel, with a reducer exhaustiveness check so future protocol drift fails typecheck.install()replaces-by-id (dev HMR safety); canvas cancels its pending cursor rAF on unmount.Deployment
railway.jsonwas dead config: Railway config-as-code is service-scoped and has noservicesmap, so every service was building with root Nixpacks instead of its Dockerfile, and the server had no/healthdeploy gate. Split intoapps/{server,web,sample-app,shotter}/railway.json.docs/USER-ACTIONS-REQUIRED.md(new P0 item).playwright-coreto 1.60.0 against the v1.58.0 browser image. Pinned in lockstep, with a Dockerfile comment tying the two together. Healthcheck switched fromwget(not guaranteed in the image) tonodefetch.ARG VITE_SHOTTER_URLso the documented shotter activation runbook actually reaches the Vite build.FOLDO_PROD_BASEfrom the defaultrepository_dispatchwiring now falls back to the prod default instead of producing invalid URLs.serve.json(web + sample-app): the 1-yearimmutablecache rule is scoped to Vite's content-hashedassets/; unhashedpublic/files getmax-age=3600. Verified live against the built dist with the exact Dockerfile CMD.Test plan
npm run typecheckclean across all workspacesnpx vitest run— 139 passednpm --workspace @foldo/web run build+ bundle-size gate (70% headroom)prettier --checkcleanhttps://claude.ai/code/session_01T3cCon5oGQcDKPASj9d8Wu
Generated by Claude Code