v1.13.5 - #61
Merged
Merged
Conversation
A user hit a broken SGA Spaces page. /api/spaces returned a transient 401 and
the page died with "l.find is not a function"; a reload cleared it.
fetch('/api/spaces')
.then(r => r.json())
.then((data: Space[]) => setSpaces(data))
The annotation is a promise the code cannot keep. An error response is still
valid JSON, so r.json() resolved happily with { error: 'Unauthorized' }, that
object went into state typed as Space[], and the page died on the next
spaces.find(). `data.length > 0` had already passed silently -- undefined > 0 is
just false -- so nothing failed until the render. TypeScript cannot catch this:
the cast is asserted at a boundary where the shape is only known at runtime.
Eight call sites shared the pattern. Four degraded harmlessly on their own
(data.groups || [], (requests ?? [])), but two more could crash the same way and
one was worse than a crash:
sga-spaces/page.tsx the reported failure
sga-spaces-tab.tsx .then(setSpaces) -- identical shape, admin view
booking-settings-tab every field falls back to a hardcoded default, so an
error body silently populated the form with 0/0/24 and
an admin pressing Save would have written those over
the real booking limits
settings-modal error body cast to Settings, offered back for saving
archive-tab no .catch at all, so a rejected fetch left
setLoading(false) unreached and the tab hung on its
skeleton
getJson/getJsonArray take the fallback as an argument, which makes the failure
case impossible to leave out and keeps the return type honest.
The Spaces page handles its response directly instead, because there an empty
list is a meaningful answer -- "no rooms configured" -- and quietly substituting
one for a failed request would state something untrue. It now shows a short
error with a Try again button, which is the same remedy the reporter found.
Not fixed here: why /api/spaces 401s in the first place. Both endpoints in the
report use the plain getAuthedUser path, no user is missing a profile row and no
sessions are revoked, so this is a transient auth failure rather than a
regression. What is fixed is that it can no longer take the page down.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
signOut() defaults to scope 'global' -- confirmed in auth-js:
async _signOut({ scope } = { scope: 'global' })
which revokes every refresh token the user holds anywhere. So the 44-minute
idle timer firing on someone's laptop also ended the session on their phone, in
their other tab, and the copy the Edge middleware refreshes. Each of those then
failed its next refresh with a 400 and began serving 401s, which is how a page
that was working a moment ago starts returning Unauthorized.
Supabase's logs over 24h: of 28 refresh_token grants, 9 returned 400 -- 5 from
browsers and 4 from the Edge middleware. Alongside 28 logouts across ~27 users,
and only 3 of 24 refresh tokens ever having a parent (so rotation is barely
occurring, meaning a 400 is a revoked token rather than a rotation race).
The four sign-outs in dashboard-shell -- the Sign Out button and the three idle
paths -- now use scope 'local'. That is what signing out means on a shared
dashboard: this browser, not every device I own.
The four that mean "this account may not be used" keep the global scope, and
say so in a comment: deactivation in force-sign-out.tsx, LoginCard and
onboarding, plus an expired invite. Ending every session is the point there.
This is a user-facing fix regardless of the 401s -- signing out at a library
machine should not log you out on your phone.
Not claimed: that this accounts for every 401. Vercel's runtime logs are not
readable with the token I have, so the reported failure could not be tied to a
specific revocation. Some 400s also fall in hours with no logouts at all.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix: stop an error response from crashing the page (#53)
Next 16 deprecates the middleware file convention; `next dev` prints a warning
pointing at proxy.ts. Mechanical rename: the file, the exported function, and
the wording in its comments. No logic changed, and there were no
middleware-named config flags to update -- the only export beside the function
is `config.matcher`, which keeps its name.
Verified from the build rather than assumed, because a rename that Next silently
ignored would disable the auth cookie refresh rather than fail loudly.
.next/server/functions-config-manifest.json shows:
"/_middleware": {
"runtime": "nodejs",
"matchers": [ ...the same matcher regex as before... ]
}
so it is registered and still scoped to the same paths. (The legacy
middleware-manifest.json is emitted empty; registration moved to the functions
config manifest in Next 16. Worth knowing before reading that file as evidence
of anything.)
Note the runtime line: proxy.ts defaults to Node, where middleware.ts ran on
Edge. No runtime is exported, per the Next 16 default. That is the one
behavioural difference here -- the cookie refresh now runs in the function
region alongside the route handlers and the database, rather than at whichever
edge location is nearest the user. Supabase's logs currently show these refresh
calls arriving with a "Vercel Edge Functions" user agent; after this they should
arrive from us-east-1, which is where Supabase is.
Whether that measurably helps is untested and I would not assume it does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit renamed the file but shipped it still exporting `middleware`, so the build failed with "must export a function ... as a named proxy export". Cause worth recording: `git mv` stages the rename with the original content, and the follow-up `git add` named the old path in its pathspec. That path no longer existed, so git rejected the whole add -- and stderr had been redirected away, so the one message that would have said so was swallowed. The commit therefore carried the rename and none of the edits. The local build passed because it compiled the working tree, which was correct. Only the commit was wrong, and the working tree is not what CI builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
chore: rename middleware.ts to proxy.ts for Next 16
The first page every user hits was re-rendering, and re-querying, on every request -- 0.5-1.1s TTFB measured against production, against 0.23-0.39s for a dashboard redirect. Nothing on it is per-user. It is a comptroller's name, the active semester and a booking count, all readable by `anon` under RLS. It was dynamic only because it built its Supabase client from cookies(), and calling cookies() opts a route into dynamic rendering whether or not the cookies end up mattering. Reading through a cookie-free anon client instead lets the page prerender; `revalidate = 60` regenerates it at most once a minute. In the build it moves from `f /` to `o / 1m`, and locally it serves in 8-29ms from cache rather than being rebuilt per request. Reading as `anon` is also the more correct reading. "Bookings this semester" means all of them, but the cookie client counted only what that visitor's RLS allowed, so a signed-in visitor and a signed-out one saw different numbers for a figure describing the whole organisation. The "Active reservations right now" fact is removed rather than repaired. It filtered on status 'Confirmed', which exists in none of the three tables it queried -- the real vocabulary is Reserved / Tentative / Virtual / Alternate Time / Waitlisted / Unavailable / Cancelled, and 'Confirmed' appears nowhere else in the codebase. Its count was therefore always 0, and the fact is gated on `> 0`, so it has never once been displayed. It also compared Boston booking times against a UTC clock, four hours out, and rolled to the wrong day entirely after 8pm Eastern. Three queries on the entry page's critical path, for something that could not render. That halves the page's database work as a side effect: six round trips down to three, two of which are parallel. Behaviour change: the displayed fact is picked with Math.random() at render, so it now varies per revalidation window rather than per request. Two people signing in within the same minute see the same piece of trivia. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
perf: make the login page cacheable, and drop a fact that never rendered
The installed PWA showed an "SGA" wordmark while the rest of the site is branded Chambers. Both icon files were that wordmark. favicon.ico is the correct mark -- a white key on the navy background -- but it holds a single 32x32 image, so it is no use as a source for a 192px or 512px icon. The usable source is the decorative key on the login page, which is ordinary SVG geometry: two circles and four rounded rects. These icons are that same geometry rendered at size, so they match the favicon rather than approximating it. Provenance, in case they ever need regenerating: the shapes are copied verbatim from the key in app/page.tsx, minus that element's decorative scaleX(-1) rotate(-60deg), which leaves the key upright with the bow left and the teeth down -- the orientation favicon.ico already uses. Background is #0a1628, the same navy as the manifest's theme_color; the key is white; it sits at 78% of the canvas width. Rasterised with sharp. Deliberately full-bleed, with no rounded corners baked in, unlike the favicon. Android applies its own mask to launcher icons, and a rounded square inside that mask reads as a visibly clipped corner. No rounding of the source: the geometry is vector, so 512 is genuinely sharp rather than a 32px favicon scaled up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A weekly occurrence can already diverge from its series on room, time, status and reservation code. This adds two more of the three overrides the issue asks for: purpose, and hidden. Both follow the convention already in place -- a nullable column on weekly_room_occurrences where NULL means inherit -- the difference being that these inherit from the `bookings` row two levels up rather than from weekly_room_bookings. hidden is a nullable boolean rather than `not null default false` because it needs three states, not two: inherit, forced visible, forced hidden. A NOT NULL default would collapse inherit into visible and make it impossible to publish a single week of an otherwise hidden series, which is the more interesting half of what the issue is asking for. That third state changes how visibility is decided, and the filter in lib/my-rooms-data.ts is reworked for it. Previously a booking was dropped when `hidden && !canManage`. Now hidden occurrences are stripped first, and a weekly series survives if any occurrence remains -- so a hidden series with one occurrence forced visible shows that week and nothing else. One-time and tabling bookings have no per-occurrence override and keep the old booking-level rule. The stripping happens server-side, on purpose. Filtering in the client's flatten step would mean sending a hidden occurrence to a browser not allowed to see it and trusting the UI not to draw it, which is the shape of the leak issue #29 already had to be fixed once. purpose resolves with `??` rather than the `||` the neighbouring room and time fields use, since only null should mean inherit. Empty input is normalised to null on write, so a cleared field reads as inherit rather than as a booking whose purpose is blank. NOT included: the per-occurrence `is_event` the issue also asks for. Storing it is trivial, but the Events tab finds events by querying `bookings` where is_event, and its rows are bookings with their sessions nested underneath. Teaching it about occurrence-level events changes both that query and the question of how such an event should be presented in that list -- a design decision rather than a mechanical addition. Left out entirely rather than shipping a column and a toggle that nothing reads. The migration is not applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix: use the Chambers key for the PWA icon (#50)
Completes the issue. The occurrence is the event marker, so a weekly event is one week rather than a whole series, and it appears in the Events tab in its own right. is_event is NOT NULL DEFAULT false on the occurrence, not a nullable override like the purpose and hidden columns in the previous commit. A series is not an event that individual weeks opt out of -- one week is the event -- so the occurrence is authoritative with nothing to inherit. The editor gets a checkbox rather than a three-way select for the same reason, and "Clear all overrides for this date" deliberately leaves it alone, because being an event is not an override of anything. Nothing needs backfilling. Every booking currently flagged is_event is a One-Time Room, and the Administrator UI has never offered Mark Event on weekly bookings at all -- only a badge -- so booking-level is_event stays exactly as it is for one-time and tabling. The Events route cannot reach these through its existing query: filtering an embedded resource narrows the child array without selecting the parent, so a second query fetches flagged occurrences with their ancestry and folds them in. Each becomes its own row, with `<bookingId>:<date>` as its id -- the checklist and the pending-actions highlighting are both keyed by row id, and every flagged week of one series would otherwise collide on the booking's id. The row carries occurrence_date so the detail block prints one date instead of a "Sep 1 - Sep 1" range, and its purpose resolves through the occurrence override. event_tracking had to change shape. booking_id was the primary key, so a booking had exactly one checklist and two flagged weeks of the same series would have shared it -- ticking a form on one would tick it on the other. The target is now (booking_id, occurrence_date), null meaning the booking itself, which is what one-time and tabling events keep using and what every existing row already is. Keyed on the date rather than an occurrence id deliberately: the weekly PATCH handler regenerates occurrences on every save, deleting them all and reinserting with fresh ids, and values survive only by being carried across on the date. A foreign key to weekly_room_occurrences(id) would have dropped every checklist the next time anyone edited the booking. The date is the identifier that write model actually preserves. UNIQUE NULLS NOT DISTINCT is what makes the booking-level row work; a plain unique index treats NULLs as distinct, so the upsert would insert a new row on every toggle instead of updating. Both migrations were applied and rolled back against production to confirm the primary-key swap preserves the existing rows. Migration 20260829001000 is not applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…errides feat: let a weekly occurrence override its purpose and visibility (#55)
package.json, the lockfile's two root entries, and the sidebar label. Only lines 3 and 9 of package-lock.json belong to the project. object-inspect is also on 1.13.4 further down the file, so a find-and-replace across the lockfile would bump a dependency to a version that does not exist and break npm ci. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was
linked to
issues
Aug 28, 2026
Closed
Closed
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
1 participant
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.
No description provided.