mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-11 13:28:57 +00:00
xarmian
06ccabddb0
* fix(web): settings permissions are sticky, so the owner-only tab survives the /me window (BUG-2978)
Deep-linking `/{user}/{ws}/settings#danger` landed on General for a workspace
OWNER — 0/10 loads, at both 390px and 1280px, while `#storage` and `#members`
were 10/10.
The hash-restoration effect was not the fault, which is where I looked first.
Instrumenting it showed the effect applying `danger` correctly at 219ms and
losing it at 244ms. `workspaceStore.setCurrent` clears `currentMembership` to
null before `/me` resolves and the permission helpers treat unknown as
no-access by design; this route calls `setCurrent` TWICE per load, once from
the workspace layout and once from the page's own `load()`. So
`canEditWorkspace` reads true -> false -> true, the owner-only tab drops out of
the tab set during the false window, the effect's snap-back branch moves
`activeTab` off the now-invalid `danger`, and `pendingHash` was already
consumed — nothing restores it when the permission returns. Only the owner-only
tab could hit this, which is exactly why `#storage` never did.
The page now reads its permissions through sticky state that updates only when
membership is definitively known, reset on a real workspace switch — the same
two-effect shape the dashboard already uses for its owner-gated CTA, and for
the same reason (CONVE-606). `isOwner` and `canExport` get the same treatment,
because they flicker identically and gate ~15 controls on this page: read
straight from the store, an ordinary owner load makes the Save buttons, the
invite form and the delete controls go readonly and then come back.
One reset effect owns all three. Two effects testing the same
`wsSlug !== lastPermSlug` could never both fire, since whichever ran first
would have already updated the marker — an error in my first draft of this fix.
Default stays false, so owner-only chrome still never flashes before `/me`
confirms; the server-side owner check remains the enforcement boundary.
Measured after the fix: 40/40 deep links land across `#danger`, `#storage`,
`#members` and `#collections` at both widths, against 0/10 for `#danger` on
main.
The regression spec settles and then asserts ONCE, rather than using an
auto-retrying assertion: the pre-fix failure is "correct, then reverted", so a
retrying matcher can observe the correct intermediate state and pass on a
broken build. The owner leg asserts again after a further wait so a later
revert is still caught, and carries a non-vacuity check that the fixture user
really is an owner.
Claude-Session: https://claude.ai/code/session_01WS9QAnxk1gA3LBha3PvKVm
* test(web): make the BUG-2978 guard the unit test, because the e2e leg does not discriminate
The e2e spec I wrote for this fix PASSES ON THE UNFIXED BUILD. I checked, which
is the only reason this is a commit and not a false green: on the e2e fixture
the layout's `setCurrent` and the page's own land inside a single unresolved
`/me` window, so membership never goes known -> unknown -> known and the flicker
the bug needs never occurs. The ordering is a property of a small fast fixture
workspace, not of the product — the real workspace produces it readily.
So the e2e spec is relabelled a smoke leg and says plainly, in its own docblock,
that it does not guard this bug and which test does.
The guard is a jsdom test that drives the two `/me` resolutions by hand, which
makes the sequence deterministic rather than dependent on fixture speed. Run
against the unfixed page it FAILS; against the fixed page it passes.
Claude-Session: https://claude.ai/code/session_01WS9QAnxk1gA3LBha3PvKVm
* fix(web): tell "membership not fetched yet" apart from "no access" (BUG-2978, codex round 1)
The sticky permission cache from e6ed6fdd updated only when
`currentMembership !== null`, and null means BOTH "not fetched yet" and "no
access". So the cache held the last good answer forever once the answer became
a denial: an owner removed from the workspace, or a `/me` that 403s, kept the
Danger Zone tab, the Save buttons and the delete controls on screen
indefinitely. Server enforcement meant it was stale UI rather than an escalation
— which is why it is a behaviour regression I introduced and not a hole that was
already there.
The store now says which of the two a null is. `membershipKnown` is false while
a membership fetch is in flight and true once one has settled either way,
including the 403 path and the workspace-did-not-resolve path. The settings page
gates its cache on that instead, so the cache still covers exactly the in-flight
window it was written for and nothing else.
Second test leg, and it is the complement rather than a repeat: it drives `/me`
to REJECT and asserts the owner-only tab leaves. Each leg fails against the
code it guards and passes against the other — leg one fails on the pre-fix page,
leg two fails on e6ed6fdd's non-null gate. Verified by running all three
versions, not by reading them.
Dropped `vi.resetModules()` from the test's afterEach: it gave the second test a
fresh module graph including a second copy of the Svelte runtime, whose
`$effect` did not recognise the first copy's component context, and the remount
died with `effect_orphan`.
Found by Codex review round 1 (P2 + nit).
Claude-Session: https://claude.ai/code/session_01WS9QAnxk1gA3LBha3PvKVm
* fix(web): settle membershipKnown when workspace creation fails (BUG-2978, codex round 2)
`create()` clears membership at entry for the same reason `setCurrent` does, and
a rejected `api.workspaces.create()` returned before anything settled the flag —
so a failed create left `membershipKnown` false permanently. That is the worst
value for it to be stuck at: consumers cache THROUGH the false window, so a page
still mounted would hold its permissions and never update again. Membership is
already null at that point, so settling it true says "no access", which is what
the null already meant before the flag existed.
Also narrows a claim I made in the round-1 comments and commit message:
`membershipKnown` is not "false only while a membership fetch is in flight". It
is false from the start of any call that will replace membership — workspace
resolution and creation included — and true once that call settles on every
exit. The prose now says that, because the flag is a contract other consumers
will gate on and the narrower sentence was the one that made this path look
impossible.
Four store-level tests cover the arms: in-flight, 403, workspace-not-resolved,
and failed create. The last one was mutation-tested — removing the line that
settles the flag makes exactly that leg fail and leaves the other three green.
Found by Codex review round 2 (P2 + nit).
Claude-Session: https://claude.ai/code/session_01WS9QAnxk1gA3LBha3PvKVm
* fix(web): a failed create says nothing about the current workspace (BUG-2978, codex round 3)
Round 2's fix was pointed at the wrong thing. `create()` cleared membership at
ENTRY, mirroring `setCurrent` — but `setCurrent` is switching to a workspace it
already names, while a create that fails leaves the current workspace exactly as
it was. So the clear was asserting something about the wrong workspace, and
settling `membershipKnown` on the failure path made that assertion louder rather
than removing it: it turned "we don't know" into "no access to the workspace you
are still looking at", which hid a mounted settings page's owner controls until
the next `setCurrent`. I fixed the symptom my flag exposed instead of the clear
that was wrong underneath it.
Nothing is cleared now until the create has SUCCEEDED, so a failure changes no
state at all. The sequence token is claimed after the call rather than before,
which keeps the guard it was written for: this create still supersedes anything
started before it, and a `setCurrent` started after it still supersedes this.
Two test legs the previous round missed, both of which it should have had:
- a failed create leaves the CURRENT membership and `isOwner` intact — the flag
alone could not have caught this, which is why the old leg passed while the
regression was live;
- `membershipKnown` is false while the WORKSPACE is still resolving, not merely
while `/me` is, which is the contract the prose now claims.
Two stale comments corrected: the settings page still said `membershipKnown` is
"false only while a fetch is in flight" (the claim 7fb49e09 narrowed in the
store but not here), and the hash-restoration comment still named
`workspaceStore.canEditWorkspace` as `validTabIds`' dependency when it has been
the sticky `canEditWs` since e6ed6fdd.
For the record, since a pushed commit message cannot be edited: 4a1bfd2a's
"false only while a membership fetch is in flight" is wrong in the same way, and
this is the correction.
Found by Codex review round 3 (P2 + two nits).
Claude-Session: https://claude.ai/code/session_01WS9QAnxk1gA3LBha3PvKVm
* fix(web): observe the sequence token at entry, claim it on success (BUG-2978, codex round 4)
Moving the sequence claim after the create call in d7ccaeaf fixed the failure
path and opened an ordering hole: a create that STARTED EARLIER but resolved
later would override a `setCurrent` the user began in between, switching the
store out from under a navigation that was the newer intent. The comment I wrote
in that commit claimed the opposite, which is the part worth flagging — the
prose asserted the property the code had just stopped having.
Both obvious spellings get one ordering wrong. Claiming at entry (the original)
makes a FAILED create invalidate an in-flight `setCurrent`, whose writes are then
discarded on the seq check with nothing left to resolve membership. Claiming
only after the call is the hole above.
Reading the token at entry and comparing before claiming gives all three
orderings: a navigation started after this create wins, a navigation still in
flight from before loses to it, and a failed create claims nothing so it
invalidates nothing.
New leg for the ordering itself, mutation-tested: restoring the claim-after-call
spelling fails exactly that leg and leaves the other five green.
Two nits from the same round: the top-level comment still said every create exit
settles the flag "including a create that threw", which stopped being true when
failed creates went back to changing no state; and the failed-create leg's title
claimed the current workspace was untouched while asserting only the permission,
so it now asserts the workspace identity too.
Found by Codex review round 4 (P2 + two nits).
Claude-Session: https://claude.ai/code/session_01WS9QAnxk1gA3LBha3PvKVm
* fix(web): a workspace that was created belongs in the list, whoever wins the selection race (BUG-2978, codex round 5)
Two concurrent creates both succeed on the server, so both workspaces exist —
but only one can be selected, and every spelling of this token protocol so far
dropped the other from `workspaces` entirely, differing only in which one. The
entry-claim original dropped the earlier-STARTED create's workspace; round 4's
version dropped the later-COMPLETING one. Neither loss was chosen by anyone, and
the visible symptom is the same either way: a workspace the user just created is
invisible until the next `loadAll`.
The list is additive, so the append moves ahead of the token check and no longer
depends on the race at all. Only the SELECTION is raced, and that stays
first-to-complete and is now documented as deliberately unspecified beyond
that — with two creates in flight there is no intent to honour, and the part a
user would notice missing no longer hangs on the answer.
Mutation-tested: moving the append back behind the token check fails exactly the
new leg and leaves the other six green.
Nit from the same round: the `membershipSeq` comment said every setCurrent and
create call increments it, which stopped being true when create moved to
observing the token at entry. It now says which call claims and when.
The remaining round-5 nit is 4a1bfd2a's commit message, which still carries the
"false only while a membership fetch is in flight" overstatement. A pushed
commit message cannot be edited; d7ccaeaf records the correction and the PR body
carries it where a reader will actually meet it.
Found by Codex review round 5 (P2 + nit).
Claude-Session: https://claude.ai/code/session_01WS9QAnxk1gA3LBha3PvKVm
* docs(web): correct two store comments, and record what round 6 found out of scope (BUG-2978)
Two comment corrections, both places where the prose was wider than the code:
- `membershipKnown` is settled by membership RESOLUTION, not by a membership
fetch. A workspace that does not resolve at all settles it without any `/me`
request being made, which the old wording excluded.
- `recoverIfMissing` credited `loading` with preventing a duplicate list call.
The actual guard is `inFlightFor`, which JOINS the run already in flight;
`loading` is a rendering signal and is not consulted for that.
Round 6's P2 is filed as BUG-2981 rather than fixed here: a `loadAll` that
STARTS before a `create` completes commits a server list that legitimately
predates the new workspace, replacing the array and erasing the append. It is
PRE-EXISTING — verified against origin/main, where `create` does the same
wholesale append into the same array — and this branch only changed when that
append happens relative to the membership token. It is a different operation
pair, its fix contains a real design choice about what a list snapshot older
than a create means, and this is a settings-tab fix already five rounds deep.
Widening it there is how a small fix acquires a large blast radius.
Found by Codex review round 6 (two nits; its P2 filed).
Claude-Session: https://claude.ai/code/session_01WS9QAnxk1gA3LBha3PvKVm
Pad Web UI
SvelteKit 2 + Svelte 5 frontend for Pad, compiled to static files and embedded into the Go binary.
Development
npm install
npm run dev # Dev server at localhost:5173 (proxies API to localhost:7777)
npm run build # Production build to build/
npm run check # Type checking with svelte-check
When developing, run the Go backend separately with make dev from the project root.
Building for Production
Do not build in isolation. Always use make build from the project root — this builds the web frontend, then compiles the Go binary with the build output embedded via //go:embed.
Stack
- Svelte 5 with runes (
$state,$derived,$effect) - SvelteKit 2 with
adapter-static(SPA mode) - Tiptap block editor with markdown round-trip
- svelte-dnd-action for drag-and-drop in board/list views
- SSE for real-time updates
- TypeScript throughout
Structure
src/
routes/ SvelteKit pages
+layout.svelte App shell (sidebar + main)
+page.svelte Landing/redirect
[workspace]/
+page.svelte Dashboard (collections, phases, activity)
+layout.svelte SSE connection per workspace
[collection]/
+page.svelte Collection view (board/list)
[collection]/[item]/
+page.svelte Item detail + editor
conventions/ Purpose-built conventions page
playbooks/ Purpose-built playbooks page
settings/ Workspace settings
lib/
api/client.ts HTTP API client
components/
layout/ Sidebar, navigation
editor/ Tiptap editor, raw markdown editor
fields/ FieldEditor, relation picker
items/ ItemCard, ItemDetail
collections/ BoardView, ListView
common/ StatusBadge, badges, modals
search/ CommandPalette
stores/ Svelte 5 reactive stores
workspace.svelte.ts Workspace state
collections.svelte.ts Collection + item state
ui.svelte.ts Sidebar, mobile state
types/index.ts TypeScript types and constants
app.css Global styles and design tokens