Prototype the review queue, and index the three prototypes - #66
Conversation
There was a problem hiding this comment.
Automated review — pass run by Codex via the codex-rescue subagent, verified by Claude Code (Opus 5). Every finding was checked against the source before posting.
The shape holds, and the invariants NOTES.md names are the ones actually in the code: no verify-a-profile verb, no admin Withdraw, owner assignment one-way, nothing embedded. The prod guard was verified against a production build rather than read — /prototype and /prototype/admin both prerender to status: 404. The three items already recorded in NOTES.md and the description were not re-raised.
1 major, 4 minor, 4 nits.
Start with the major. The certificate URL is never on screen, only linked as "Open certificate" — and the URL is what makes Marcus Bell and Tomas Novak catchable by reading. Showing it as text embeds nothing, frames nothing and issues no request, so the "nothing renders a certificate" reasoning does not argue against it.
| return { | ||
| ...profile, | ||
| credentials, | ||
| lastVerifiedAt: stamps.length ? stamps.sort().at(-1)! : null, |
There was a problem hiding this comment.
minor — Undo check can move lastVerifiedAt backwards, manufacturing an "Edited since checked" that nobody earned.
Two clicks from a fresh load, on Priya Raghavan (q3):
updatedAt2026-08-12T11:05:00Z,lastVerifiedAt2026-08-12T11:05:00Z→hasDrifted()false.- Click Undo check on "Tool use and function calling" (
q3c2). ItsverifiedAtis nulled, so thestampsreduction on this line falls back toq3c1's2026-08-10T08:30:00Z. updatedAt > lastVerifiedAtis now true and her queue row reads "Edited since checked" — for a profile nobody edited.
This is a different mechanism from the known updated_at problem in the description: that one is the left side of the comparison being bumped by an admin write, this is the right side sliding down. A lastVerifiedAt derived as max() over live rows is not monotonic, and drift now decides queue membership, so it has to be.
Same handler, second point: lines 55-56 also null verifiedBy/verifiedAt. NOTES.md:31 calls those "the substance, not bookkeeping", and undo erases the fact that a named human ever looked while recording nothing about who reversed it. Revocation is a second attestation, not the absence of the first — drawing it as Undo is what would teach the built version otherwise.
There was a problem hiding this comment.
Two points, and only the first is fixed — leaving this open for the second.
The monotonicity bug is fixed in 0a61b2f. lastVerifiedAt is now a max over the current value and the live stamps, so it only ever goes up. Your reproduction is now a regression check: before the fix, undoing q3c2 on Priya made "Edited since checked" appear three times on screen (her queue row, the profile footer, and Kofi who genuinely has drifted); after it, only Kofi. That the stamp has to be monotonic once drift decides queue membership rather than merely marking a row is the argument, and it is in the comment.
The second point is deferred, not dismissed. Undo nulling verifiedBy/verifiedAt erases the fact that a named human looked and records nothing about who reversed it — I agree that revocation is a second attestation rather than the absence of the first. The fix is a column, though (a revocation row, or unverified_by/unverified_at), and no schema lands here before the model it encodes is settled. Written up under Still open in NOTES.md next to the in-progress question, so it reaches docs/spec/profile-and-credentials.md rather than being invented in a drawing. Happy to be pushed if you would rather see a shape for it in the prototype first.
|
Review actioned — eight of the nine findings fixed across 0a61b2f and 0219d55, one half-deferred. The major. The evidence URL is now on screen as text under the unchanged link. That was the finding worth the most: without it, Marcus Bell's Drive host and Tomas Novak's copy of Priya's slug — two of the four adversarial fixtures — were invisible on the only screen where the judgement happens. NOTES.md gained a paragraph saying why text is the opposite decision from rendering a certificate rather than a softening of it, since the section it sits under otherwise reads as forbidding this. Fixed. The credentials panel is read-only on Left open: half of the Checked with |
`/prototype/admin` — one queue, profile-centric, with the two Bluehex-owned axes kept apart. `status` is admission control and governs whether a profile is visible; `verified` is credential attestation, per credential, and governs only whether the badge shows. A profile can be approved and unvouched for, and the spec calls that the normal case, so the queue cannot be a single list with an Approve button. It is a queue rather than a table: membership is decided by what is outstanding, so acting on a profile clears it and the list can reach empty. Visibility sits above the credentials because admission control is the earlier decision and the profile you have just read is the evidence for it, while the certificates are the evidence for the other one. The nine invented people are adversarial on purpose, and four of them are awkward in different ways. Two are awkward while being entirely innocent — Aroha Ngata, whose certificate carries her legal name rather than the name she goes by, and Hae-Won Park, who claims an earned credential with no evidence URL and so can be approved and can never carry the badge. A design that makes either look like Marcus Bell, who is spam, is wrong. `NOTES.md` records the invariants the built version has to hold, chiefly that there is no verify-a-profile action and the UI must never grow one. It also records two things that outlive the drawing: nothing may render a certificate inside the admin page, since a third-party page can refuse framing and fetching it server-side is SSRF against a host running Supabase locally; and an in-progress credential cannot be verified by any mechanism, which is an argument that it belongs in the bio rather than the credential table. Both are noted, neither is acted on here. `/prototype` indexes the three surfaces. It lands last because it links to all of them.
Every certificate rendered as the identical string `Open certificate`, so the URL — the thing the admin is actually judging — never appeared. That cost two of the four awkward fixtures their point: Marcus Bell's Drive link was indistinguishable from a Skilljar certificate page until you opened a tab, and Tomas Novak's byte-identical copy of Priya's URL was invisible to the one reader who might recognise the slug. The link stays exactly as it was, with the URL under it as text. None of the reasoning in NOTES.md applies to text: it embeds nothing, frames nothing and issues no request, and the argument there is against Bluehex fetching or rendering what is at the other end. The credentials panel had no status guard, so `Verify` stayed live on a rejected profile and would stamp a named human onto spam. `outstanding()` already returns nothing for `rejected` and `withdrawn`, and the footer said "Nothing left on this profile." above two working buttons. `CredentialRow` now takes the profile's status and renders the action column read-only for those two, and the badge line says the profile is not visible rather than counting work nobody is going to do. `Undo check` could move `lastVerifiedAt` backwards: a `max()` over the live credential rows is not monotonic, so undoing the newest check fell back to an older stamp and `updated_at` overtook it. Two clicks from a fresh load put "Edited since checked" on Priya Raghavan, who nobody had edited. Since drift decides queue membership rather than merely marking a row, that is a phantom queue item, and the stamp now only ever goes up. Undoing a check is a statement about one credential; it does not unmake the fact that a human looked at the profile that day. That the undo also erases `verifiedBy`/`verifiedAt` outright is left alone and written up in NOTES.md — revocation is a second attestation and wants a column, which is a spec decision rather than a prototype tweak. Three smaller things on the same surface. The arrow-key shortcut called `preventDefault()` on all four arrows with no modifier check, which took away scrolling on a long page and swallowed `Alt+ArrowLeft` for Back; it is left and right only now, and never with a modifier. The badge line called Hae-Won Park's uncheckable credential "still unchecked", describing as pending work what the row below correctly calls permanent — the distinction `queue-data.ts` keeps between `unchecked` and `checkable` now reaches the header. And the note field had no accessible name, so it announced as its own placeholder.
The page said "the two surfaces that do not exist yet" above three entries, and the header comment said it twice more. It is also no longer true of `/prototype/directory`, which draws the directory that ships today, so the sentence now separates that one from the two that are still only drawings. `design.open` went with it. The type declared it, no entry ever set it, and the ternary reading it could only ever render "Settled" — left over from when the variants and the switcher existed.
0219d55 to
ae5cb55
Compare
The deploy workflow fires on a push to `main` and nothing else. That is the right default and stays the default — shipping should normally be a consequence of landing on `main` rather than a button someone remembers to press. It leaves one gap, and we hit it today. A workflow disabled by hand creates **no run** for a push that arrives while it is off — not a queued run, not a skipped one, nothing at all. So when `Vercel Deploy` was re-enabled after #63–#66 merged as a stack, there was no run to re-run and `main` sat ahead of production with no way to reconcile the two except pushing a commit whose only purpose was to be pushed. Re-running the most recent deploy run is not a substitute and is worse than doing nothing: it redeploys the commit that run was for, which here is `497cde4` from 13 August. ## The ref guard `workflow_dispatch` runs against whichever ref the caller picks in the dropdown, and every step in this job passes `--prod`. Without a guard, selecting any branch there ships that branch to production. `if: github.ref == 'refs/heads/main'` sits on the job rather than inside the deploy step, so a wrong ref costs a skipped run rather than a half-finished one. ## Verification Parsed with `yaml.safe_load`: triggers are `push` and `workflow_dispatch`, and the job carries the ref condition. The workflow itself is unchanged below that — same pinned action SHAs, same concurrency group, same steps. Note that merging this will itself trigger a production deploy of `main`, since it is a push to `main` and the workflow is enabled again. That is the intended outcome as well as the fix.
1 participant
Last of a four-PR stack. Stacked on #65; this PR's own diff is
src/app/prototype/admin/plus the/prototypeindex, which lands here because it links to all three surfaces./prototype/admin— one queue, profile-centric, with the two Bluehex-owned axes kept apart.statusis admission control and governs whether a profile is visible at all;verifiedis credential attestation, per credential, and governs only whether the badge shows. A profile can be approved and unvouched for and the spec calls that the normal case, so this cannot be a single list with an Approve button — which is the design everyone reaches for first.This is a drawing, not an implementation. Prod-guarded. A real admin surface is gated on
bluehex_admin, a Postgres role stamped onto the access token per ADR-0001, and needs #49/#50 and application auth first.It is a queue rather than a table
Membership is decided by what is outstanding, so acting on a profile clears it and the list can reach empty. That sounds like a scale concern and is not — a list that never shrinks feels wrong at nine profiles as much as at two hundred.
Two consequences that only showed up because the fixtures fight back. Rejecting a spam profile has to clear it outright, certificates and all, since checking evidence on a profile nobody can see is work with no consumer. And a profile that has drifted — edited since we checked it — but whose credentials are all verified had no action that closed it; "Check again" does, and it is the same verb a second time rather than a new one, since the badge means a named human looked on a given day and looking again moves the day.
The nine invented people are adversarial on purpose
Four are awkward, and two are awkward while being entirely innocent:
A design that makes Aroha or Hae-Won look like Marcus is wrong, and that is the most useful thing this population can tell you.
Three things in NOTES.md that outlive the drawing
AGENTS.mdwarns fails open rather than loudly.evidence_urlis submitted by an untrusted practitioner and the admin viewing it is the one principal who can setverified. Framing it lets that page navigate the top frame away or draw a sign-in prompt to phish the session; fetching it server-side to proxy or screenshot it is SSRF against a host running Supabase onlocalhost:54321; even a bare remote image leaks the reviewer's address and the timing of the review back to the person under review. A link withrel="noopener noreferrer"and the URL as text, and nothing embedded.One further finding belongs to the schema. An admin status change bumping
updated_atmakes approving a profile you just verified put it straight back in the queue as drifted.NOTES.mdhad called that false positive cheaper than a column to suppress, which was true while drift was only a marker and stopped being true once drift decided queue membership. The drift timestamp wants moving by the practitioner-writable columns only — a trigger scoped to the same list the column grants already enumerate.Not included
src/app/prototype/variant-switcher.tsxexists on the working branch and is not in this stack. Nothing imports it any more, and dead code should not ship.pnpm lintpasses.pnpm buildandpnpm test:e2ehave not been run.