ccSWR stale-serve trap on owner profile (PUT-editable bio, no ETag) #385
Labels
No labels
actions
bug
cli
duplicate
enhancement
fork
forum
git storage
help wanted
insights
invalid
issues
moderation
oidc
ownership transfer
packages
pr/merge protection rules
projects
pull requests
question
releases
sponsorships
tags
webhooks
wiki
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
crueber/walhub#385
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Follow-up flagged by the #381 review (PR #383 findings). internal/api/profile.go:200 serves the PUT-editable bio under ccSWR with no ETag — the same stale-serve trap class #381 fixed on the summary. Either move to ccMutable (no-cache + ETag) per the #381 pattern or document why SWR is safe there.
Fix open as PR #388: #388 — profile GET moved to ccMutable + content ETag (p over the served doc, covering bio/display_name/location/timezone/updated_at + can_edit), with TestProfileMutableClassAndETagEconomics (200→304→edit→200→304). internal/api -race green, 95.3% cover (profile.go 100%), gofmt/vet clean, 07_api §4/§8/Decisions updated, no new deps. Avatar question: no avatar projection on this route (rides me.avatar_url, #376).
Review of PR #388 (
b61c51f, fix/issue-385) — verified independently in scratch worktree /tmp/pr388, no browser (tests + reasoning; no browser-facing UI change — JSON API + docs only).(1) Mutable projections complete — internal/api/profile.go OwnerProfile (struct profile.go:52-60): PUT-editable display_name/location/timezone/bio_markdown + server-stamped updated_at + request-scoped can_edit all ride profileETag via json.Marshal of the full doc (profile.go:223-229); owner slug in hash too. Nothing missed. Avatar correctly absent: no avatar field on the struct; rides GET /api/v1/me avatar_url (#376) as the §8 doc line states.
(2) ETag sound — deterministic (struct-order JSON + FNV-1a/32 hex, 'p' prefix — exact #384 detailedETag shape with 'd'). can_edit in hash → per-viewer ETags (grant-only change busts revalidation). No cross-user leak: writeCached (env.go:742-754) compares If-None-Match against the per-request ETag, so user A presenting user B's ETag mismatches and gets 200 with A's own body; 304 only on exact self-match. Cache-Control private, no-cache. Correct.
(3) Class exact — reuses per-package ccMutable ('private, no-cache', env.go:701), no new constant; test asserts exact equality AND absence of stale-while-revalidate (profile_test.go).
(4) Economics proven — TestProfileMutableClassAndETagEconomics: PUT→200→304→bio-edit→200 (new ETag, fresh body asserted)→304. Unchanged profiles still zero-body 304.
(5) Law 6 unchanged — no new store calls (single exact-key sidecar GET in readProfile untouched); route off push/sync/checkpoint budgets as the Decisions note states.
(6) Gate results (scratch worktree, independent): go test ./internal/api/ -race -count=1 PASS; coverage 95.3% package (≥95 gate holds), profile.go 100% every func; gofmt clean; go vet clean; go build ./... clean. Docs: 07_api.md §4 scope + §8 profile line + Decisions entry all updated in-commit (law 12). go.mod/go.sum untouched (no new deps). #381 (ccMutable mutability rule) / #384 (content-hash ETag) pattern fidelity confirmed. No leftover ccSWR in profile*.go.
Non-blocking nit (no fix pushed): profileETag doc comment lists owner slug but defers can_edit to the next sentence, while the Decisions entry lists can_edit but not the owner slug — both substantively accurate (hash covers the whole struct JSON; owner is the URL key), just enumerated in different orders.
MERGE RECOMMENDATION: ready to merge.
Fixed by PR #388 (review clean — per-viewer ETags, no cross-user leak, economics proven, #381/#384 fidelity), merged. Closing.