v5 data platform, Phase 5: inventory editing, the tool editor, and the three admin queues - #37
Conversation
/admin gains the surfaces the lab runs on (spec §5.3, §5.6): the review table with its needs-attention flags, a tool editor offered both from a row and as a full-screen sheet over a machine's own page, and the maintenance, corrections and project-moderation queues. The editor's concurrency token is a string, not a Date — `extract(epoch from updated_at)::text`, minted and compared by the same expression. Postgres has microseconds and a JS Date has milliseconds, so a Date comparison matches nothing on Neon while matching forever on PGlite: every save a bogus conflict in production and no test anywhere to say so. A unit, resource or photo write touches its tool in the same transaction, so the tool is the token for the whole panel, and a refused child write rolls that touch back rather than spending every open panel's token over a write that never happened. Every server action checks its own permission through one shared gate — a server action is a POST endpoint with a generated name, reachable without the page that offers the control — and each queue's test proves it is its own permission by granting only the adjacent ones. An audit write that fails after its change committed stays a warning on a success, through the channel /admin/users already had rather than a second one. Closing the seams between the four parts: - Migration 0004 writes the three `user.id` foreign keys the plan asked for and no part wrote — tools.last_reviewed_by, maintenance_logs.assigned_to_user_id, projects.published_by — all `on delete set null`, sharing one helper with actorColumns(). Phase 5 is the first code to write two of them, so until now an id naming no row went in without complaint. - .admin-shell now supplies the --td-* tokens its .td-* utilities read. Those tokens were scoped to .tool-detail, and an unresolvable var() computes to `unset` rather than falling back: the amber "saved, but the audit log did not record it" line was rendering the colour of body text. - A resource's lost PDF raises files_not_attached instead of reporting itself as a lost photo; the queues' refusal codes are declared once rather than twice; markToolReviewed names its permission the same way its siblings do. E2E covers the flow the phase exists for: a SuperMaker at a 390px viewport marks a unit out of service from the machine's own page and the public tool page stops saying it is available. That last step is the cache-tag seam nothing smaller could reach — the test was confirmed red by pointing invalidateCatalog() at the wrong tag, where it failed with the page still reading "Available". Gate from v5/ with every environment variable unset: lint 0 errors (3 pre-existing warnings), typecheck clean, 137 files / 1711 tests, 67 Playwright tests, spec:coverage 73 items / 0 undocumented, and a build in which /tools/[id] and all five /admin routes are still Partial Prerender. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AjvfabH9CRceoC9GfjjSvE
… the truth
Every one of the four reviewers' blockers held against the code.
**Hide did not hide.** The editor's Hide control flips `resources.published`,
the panel tags the row "Hidden" and the assistant stops reading it — but the
public tool page listed every resource of a visible tool, with a comment saying
so deliberately. An internal SOP somebody restricted stayed world-readable. The
comment's premise expired when `resources.published` gained its `default true`:
a hidden resource is now always one a person chose to hide. `loadTools` filters
on it, like `listResourcesForTool` already did. (The blob itself is still
public at its random pathname — hiding removes the link, not the document.)
**A unit row posted its whole ten-minute-old copy.** The rows are keyed by unit
id and never remount, so after a conflict reload the row still held what it
opened with, and Save sent all five text fields — reverting a second editor's
serial number against a fresh token, with no conflict and no audit event.
`notes`, which has no box at all, was rewritten on every save. Now each field
the person has not touched rebases onto the newer server value, each field they
have is theirs, and Save posts only the second kind.
**`refreshChildren` left the tool alone.** Publish, Unpublish, Archive, Restore
and "Looks good" all commit through it, so the panel said "Saved" over a badge
still reading Published and a button still offering to unpublish — the page
asserting what the database no longer holds, and a second click writing a second
audit event. It now takes the tool's *state* too, and still not its text.
**`revalidateTag(tag, "minutes")` was stale-while-revalidate.** Next 16 reads
the named profile, sets `expired = now + 3600s` and serves the old entry to the
next reader; it also skips marking the path revalidated. A student scanning a
QR label seconds after a publish got the pre-publish miss. `{ expire: 0 }`
expires it where it stands. The profile was wrong twice over: the catalogue
reads use `CATALOG_CACHE`, not anything named in `next.config`.
Each fix has a regression test, and each was watched failing without it.
lint, typecheck, vitest (1720), playwright (67), spec:coverage and build all run
with every environment variable unset.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjvfabH9CRceoC9GfjjSvE
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 927bbec27d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| attachments, | ||
| and(eq(attachments.ownerType, "resource"), eq(attachments.ownerId, resources.id)) | ||
| ) | ||
| .where(inArray(resources.toolId, toolIds)) |
There was a problem hiding this comment.
When a tool's only manual is hidden, this query still counts it because it does not filter on resources.published; the public catalog now omits that resource, but the inventory row reports noManual: false, so the no_manual and “Needs attention” filters miss a tool whose visitors have no accessible manual. Restrict this aggregation to published resources.
AGENTS.md reference: v5/AGENTS.md:L238-L243
Useful? React with 👍 / 👎.
| setTitle(""); | ||
| setType(""); | ||
| setUrl(""); | ||
| setFile(null); |
There was a problem hiding this comment.
If addResource is refused—for example because another edit caused a revision conflict or the entered URL fails server validation—onAdd returns immediately while these setters erase the title, type, URL, and selected file before the result is known. The user must reconstruct the form, and an already-uploaded PDF is left orphaned for the sweep; clear this state only after a confirmed successful action.
AGENTS.md reference: v5/AGENTS.md:L219-L226
Useful? React with 👍 / 👎.
| </select> | ||
| </div> |
There was a problem hiding this comment.
After a conflict reload, the category and location selects are the only editable tool fields that never render theirValue(...). If both editors changed either select, the current editor receives a fresh revision and can retry their value without ever seeing or being able to choose the other editor's value, contrary to the conflict workflow implemented for the other fields; add the conflict comparison/control beside both selects.
AGENTS.md reference: v5/AGENTS.md:L219-L226
Useful? React with 👍 / 👎.
| const tool = await findToolByIdOrSlug(idOrSlug, { includeDrafts: true }); | ||
| if (!tool) notFound(); | ||
|
|
||
| return <DetailShell tool={tool} />; |
There was a problem hiding this comment.
For an authorized user opening a draft slug, this branch returns only DetailShell; the EditToolControl in page.tsx is rendered exclusively on the published-tool branch. Consequently drafts are viewable at their own page but cannot be edited or published through the specified phone-first overlay, forcing staff back to the inventory table. Render the same editor control for this authorized draft view.
AGENTS.md reference: v5/AGENTS.md:L212-L218
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DNV63U6ERy45Gj2TThBfyC
Phase 5 of the spec (§5.3). Stacked on #35. 135 files, +19,227.
This is the phase that makes the lab run on this app instead of on Notion.
What it adds
/admin/inventory— every tool including drafts and archived, with filters for state, category, location, free text, and Needs attention (no photo, no manual, unlinked units, open tickets, never reviewed). Filters live in the URL. That filter is the inventory review./tools/<slug>— the same panel over the detail page, phone-first, because a SuperMaker marking a printer out of service is standing next to the machine./admin/maintenance,/admin/corrections,/admin/projects.catalog.view_drafts; everyone else gets the real 404.The bug the survey found before a line was written
The obvious optimistic-concurrency check is green on PGlite and broken on Neon.
updated_atistimestamptz, maintained by theset_updated_at()trigger usingnow(). Real Postgresnow()has microsecond resolution; both drivers parsetimestamptzinto a JSDate, which truncates to milliseconds. Sowhere updated_at = <Date read back earlier>matches zero rows on Neon — every single save would report a bogus "someone else changed this tool".And PGlite's
now()is millisecond-resolution, so a test written the obvious way passes forever and the bug only ever appears in production.Phase 5 therefore carries an opaque string revision token (
extract(epoch from updated_at)::text, minted and compared by the same expression), pinned by a test that stages a microsecond timestamp with the trigger disabled.Four major findings, found by review and fixed
Each was confirmed red before its fix.
invalidateCatalogused theminutesprofile, making the catalog tag stale-while-revalidate rather than expired, so the first reader after a save still got the pre-save page. The catalogue reads did not use that profile at all.Verification
Every command with every environment variable unset, on PGlite + MSW, no network:
vitest— 137 files / 1,720 tests (I re-ran this myself: 1,720 passed)playwright— 67 passed, 0 flakylint0 errors (3 pre-existing warnings) ·typecheckclean ·spec:coverage73 · 0 undocumented ·buildgreen with no databaseTeeth check on the new E2E: with
invalidateCatalog()pointed at a misspelled tag and retries off,tool-editor.spec.ts:151failed on exactly the silent-staleness assertion it exists for. The test has teeth.Six minor findings left open, deliberately
Carried forward rather than rushed. None blocks the demo:
failedif the follow-up load throws, then conflicts against the person's own writedeleteUnit's history check can race a ticket committing between the count and the DELETEShared, not duplicated
record()andwarn()moved tosrc/lib/admin/audit-warning.ts;/admin/usersnow uses the shared pair. A failed audit write stays a warning on a success across every admin surface.🤖 Generated with Claude Code
https://claude.ai/code/session_01AjvfabH9CRceoC9GfjjSvE