feat(web): inline create from the relation picker (TASK-2877) - #1245
Merged
Conversation
PLAN-2857 U8, picker half. When a scoped picker's query matches nothing — or nothing EXACTLY — it offers a trailing "Create "<query>" in <collection>" row, keyboard-reachable like any other row. The affordance is opt-in at the call site: it appears only when the host passes `oncreate`, which is how both of U8's scope rules are expressed without this component knowing either. "Relation fields only" is the Relationships tab passing nothing; the permission gate is the caller's, because "may this user create in the target collection" is the collection-level `canEditCollection` cascade that lives in the workspace store. Result rows and the create row become ONE `options` list, in render and keyboard order, so arrowing onto the create row needs no special case and cannot fall out of step with what is on screen. `activeId` already addressed rows by identity; the create row takes a NUL-prefixed sentinel id in the same namespace, which no UUID can collide with. Two suppressions carry weight and both are pinned: * EXACT-TITLE. Tested against `rawResults` — the source's answer before exclusion and the row bound — because an exact match pushed past `limit` or excluded by the caller would otherwise read as "no such item" and offer to mint a duplicate of a row that exists. This IS the no-duplicate half of the unit's proving test: there is no create-time uniqueness check anywhere, because the second pass at the same text never reaches a create. * LOADING. Mid-flight, "nothing matched" is not yet known. The one assertion that can fail here is `aria-expanded`, not the row's absence: the markup renders the loading branch INSTEAD of the listbox, so a build that offered the row mid-flight would still show no `.picker-create` and merely leak a combobox announcing itself expanded over no listbox. That is trap #1 from this plan's false-green note, met in my own diff. Re-entrant creates are dropped while one is in flight, so two Enters inside a single round trip cannot mint two items — a duplicate the exact-title check cannot catch, since no row exists yet to match. Mutation matrix, all killed: exact-title suppression removed (3 tests), re-entrancy guard removed, `loading` term removed, `collection` term removed, Enter dispatching over `results` (the pre-U8 line), create row prepended rather than trailing.
…ed (TASK-2877)
PLAN-2857 U8, caller half. `FieldEditor` hands the picker an `oncreate`
only when the viewer may create in the field's DECLARED TARGET, so it
decides both of the unit's gates by deciding whether to pass one.
The gate is `canEditCollection` on the target collection — the same
predicate behind the collection page's "+ New" — asked about where the
item would LAND, not about where the user is standing. It needs the
collection's ID, which only the loaded collection list carries; a target
the list does not know yields no create row, because "no answer" must not
read as "allowed".
NO FIELD VALUES ARE SENT, and that is a decision with a receipt. The
server fills every missing key that declares a `Default` and stores the
defaulted map (`items.ValidateFields`, then "Marshal validated/defaulted
fields back" in `createItemChecked`), so the schema's own answer is
already the right one. The collection page's "+ New" guesses
`status.options[0]` instead; driven live against a Colors collection whose
status options are [draft, approved] with `default: approved`, the created
row came back `{"status":"approved"}` — the declared default, which that
heuristic would have gotten wrong. The cost is that a target carrying a
REQUIRED field with no default refuses the create; that surfaces as a
toast naming the field, which is the honest outcome for a row this picker
cannot fill in.
The new item is upserted into `localIndex` under the epoch captured BEFORE
the request (BUG-2098 — a projection resync landing mid-flight means the
response was authorized under a scope that no longer applies). That upsert
is what makes the picker's exact-title suppression true on the very next
keystroke; without it the same text offers to create a second item.
Mutation matrix, all killed: creating in a collection other than the
declared target, the permission gate removed, the upsert removed, and the
epoch read after the request rather than before.
…(TASK-2877)
Codex review round 1, three findings, all confirmed by reading the code
they name rather than taken on the report.
P1 — the create completion had no fence, and there are two ways past it.
`ItemDetail` wraps its fields section in `{#key itemSlug}`, so an item
switch DESTROYS this component; the promise survives, and `onchange` calls
into the persistent parent, whose `updateField` builds its PATCH against
whatever item is current at CALL time. A create started on car A therefore
wrote its colour onto car B. Separately the picker stays open across the
round trip, so the user can settle on another row (or clear the field)
before it lands — and last-write-wins is the wrong rule there, because the
later write is an explicit choice and the earlier one is a promise they
have moved past. A `destroyed` flag and a supersede counter, checked
together, close both. The `localIndex.upsert` deliberately runs BEFORE the
fences: the row exists on the server whatever happened locally, and
withholding it would leave a picker offering to create it a second time.
P2 — `targetCollection` read the global collection list with no freshness
gate, so during a workspace switch a slug match against the PREVIOUS
workspace's rows yielded a foreign collection ID, and `canEditCollection`
answered about that. Same gate `knownCollectionSlugs` already had, which
this derivation was missing.
P2 — the exact-title suppression was asking the RANKING. `warmSearch`
requests `limit + excluded.size` hits, so an exact row the ranker placed
outside that window is simply absent from `rawResults` and the picker
offers a duplicate. The question has an authoritative answer in
`localIndex`, already in RAM, so the warm path now scans the collection
directly. The `rawResults` check stays and is NOT redundant: while the
index is cold there is nothing to scan, and the server's rows are the only
evidence the row exists — pinned by its own leg, which is what killed the
mutant that removed it.
Mutation matrix now 15 mutants, all killed; baseline and restore both
76/76. Two rounds of it earned their keep beyond the fixes: M3 SURVIVED
once the index scan landed, and the mutant was faithful — the suite had no
cold-path exact-match leg, so the surviving mutant found a real hole in my
tests rather than a redundant line in the code.
…tch (TASK-2877) Codex review round 2, two P1s, both confirmed at the lines they name. CANCEL. `oncancel` only closed the picker, so backing out did not supersede an in-flight create — the pending promise then resolved and selected an item the user had just declined. Backing out is as explicit a choice as picking a different row, and now bumps the same counter. WORKSPACE SWITCH. `ItemDetail` keys its fields subtree on `itemSlug` ALONE, so switching workspaces to an item carrying the SAME ref — and every workspace has a TASK-5 — reuses this component rather than remounting it, and `destroyed` never fires. The completion then wrote an item ID from the previous workspace into the new workspace's item. `createRelationTarget` already captured `ws` and `collSlug` before the request; it now compares them to the live props before applying, which is the DR-6b shape `ChildItems.submitCreate` uses for the same reason. The `localIndex.upsert` still runs ahead of all three fences and still uses the CAPTURED workspace: the item genuinely exists in the workspace it was created in, and the fences are about where the VALUE is written, not about hiding a real row. Matrix now 17 mutants, all killed; baseline and restore both 78/78. The two added here — cancel not bumping the counter, and the ws/collection comparison removed — are what stand in for having seen these two tests red before the fix, since pin and fix landed in one edit.
…TASK-2877)
Codex review round 3 P2. `coldSearch`'s catch leaves exactly the state a
successful empty answer leaves — no rows, not loading — and the result
list is right to render both as "No results". The create row is not: an
empty answer is evidence that no such item exists; a failed one is no
evidence at all, and offering to create on no evidence is how a duplicate
gets minted while the index is cold and the network is unhappy. Same rule
the permission gate already follows — no answer must not read as
permission.
A `coldFailed` flag now separates the two, and where it is CLEARED was
settled by the matrix rather than by symmetry. Three reset sites looked
obviously needed and three mutants removing them survived:
* the cold branch of `runQuery` — `loading` is true for that entire
window and already suppresses the row, and both `coldSearch` branches
assign the flag outright when the request settles;
* the empty-query branch — covered twice over, since an empty query
offers no create row at all;
* the workspace-reset effect — same as the first.
All three are gone rather than carrying a comment claiming a protection
they do not provide, which is the disposition this plan's own U3 note
records for an unkillable guard. The ONE reachable reset is the warm
branch: it is the only path that produces a fresh verdict without going
through `coldSearch`, so without it a single network blip suppresses the
affordance for the rest of the session even once the authoritative in-RAM
answer is available. That one has a test, and its mutant dies.
Round 3 also raised a P1 I am NOT taking: typing a new query while a
create is in flight does not cancel it. The three fences that exist —
escape, picking another row, retargeting — each stand for an act that
means "not this one". Typing is not such an act; it is mid-thought, and
the user did explicitly ask for the item that is being created. Treating
it as a cancel would leave the created row orphaned and the field unset,
which is a worse outcome than a field that ends up holding exactly what
was asked for. Told to Codex in the next round rather than left to be
re-flagged.
Matrix: 20 mutants, all killed; baseline and restore both 80/80.
…ror toast (TASK-2877) Codex review round 4. Two taken, one declined. P1 — `bootstrapState === 'ready'` was the wrong authority for the create row. It coexists with `pendingResync`: `localIndex` hydrates from the IDB cache and serves those rows while delta-sync catches up, so during that window an item that EXISTS can be missing from the snapshot. The create row is derived from ABSENCE, and a cache snapshot cannot support that inference — presence still can, since the row was real when it was cached. `indexCanProveAbsence()` is asked ONLY by `showCreate`; search and listing keep using `isWarm`, because showing cached rows during a resync is right and it is only the "therefore no such item exists" step the cache cannot bear. The window is seconds and a duplicate outlives it. That leaves one rule across the whole unit, applied in four places now: offer only where something authoritative has answered. Cold is authorized by `/search` (the server answered); a settled index is authorized by the in-RAM collection; a resyncing index and a failed search authorize nothing. P2 — the failure path was unfenced while the success path was not, so a create the user escaped out of, or one belonging to a workspace they have since left, still threw its error over whatever they were looking at. Same three conditions, same reasoning: the difference between reporting and not is whether they are still waiting on it. DECLINED, with reasons, so it is not re-flagged: the "A->B->A gap" in the workspace fence. The classic gap bites when an identifier can be REBOUND to a different object between capture and compare. Here the pair (workspace slug, item slug) is what the fence compares, and the parent subtree is keyed on the item slug, so returning to the same pair returns to the SAME item — applying the create there is correct, not stale. Item refs are sequential and never reused, so the identifier cannot be rebound within a workspace. Matrix: 22 mutants, all killed; baseline and restore both 82/82. Four anchors went stale this round because the fence now appears on two paths and matched twice — the harness refused to score them rather than silently mutating the wrong copy, which is the reason it checks.
…-2877) Codex review round 5, two P1s, both about `localIndex.reset()` — the sign-out / 403-purge / deleted-workspace path. THE FLAG WAS THE WRONG WAY ROUND. `coldFailed` asked "did the last search fail", and that was false in three states that are not answers at all: before the first request, after a failure, and after a reset drops every row while the query sits in the box. Each one read as "fine" and put a create row on screen backed by nothing. Inverted to `coldAnswered` — set in exactly one place, by the event that earns it, and cleared wherever the answer stops describing what is in the box. A flag that must be cleared everywhere is one that will be missed somewhere; this is the same defect arriving twice (round 3 caught the failure case, round 5 the reset case) because the polarity made silence indistinguishable from success. THE EPOCH FENCE HAD TO BE TWO-SIDED. `upsert`'s own guard refuses a captured epoch BELOW the current one, which catches a resync. But `reset()` DELETES the workspace state and the next bootstrap starts a fresh one at `scopeEpoch` 0 — so a captured 7 is not below 0, sails through, and links a row minted under an identity that no longer holds. `createRelationTarget` now requires equality. The residual is in the code comment rather than papered over: a reset plus resyncs landing back on exactly the captured number would compare equal, which an exposed reset generation would catch and this does not. Also dropped the `loading` term from `showCreate`. It and the per-query `coldAnswered` reset were a redundant PAIR — each survived removal while the other stood, which is one guard and one line that looks like a guard, not defence in depth (this repo has a note about exactly that shape). `coldAnswered` is the one kept: it states the rule (something authoritative has answered FOR THIS QUERY) where `loading` is a UI state that correlates with it. Matrix: 24 mutants, all killed; baseline and restore both 85/85. Killing the per-query reset needed `aria-expanded`, not the row's absence — with `loading` still gating the MARKUP, `.picker-create` is missing either way and asserting on it measures the branch instead of the rule. Third time this suite has been fooled by that same separation. Re-verified end to end in a real browser on this exact build: create row offered for a non-matching query and keyboard-reachable; Enter created COLO-6 "Chartreuse" in COLORS (colors 2 -> 3, cars unchanged) with `status: approved` — the schema's declared default, which the "+ New" `options[0]` heuristic would have gotten wrong; the car's field holds that id; a second pass at the same text offers the existing row and no create; Escape leaves the value untouched; no bare UUID anywhere on the page.
…(TASK-2877)
Codex review round 6, and it corrects the reasoning round 5 shipped.
Round 5 fenced the create on `scopeEpochFor(ws) === epoch` and recorded
the residual as needing a coincidence — a purge plus resyncs landing back
on the captured number. That was wrong, and wrong in the direction that
matters: `reset()` deletes the state and the replacement starts at
`scopeEpoch` 0, which is ALSO the value whenever no projection resync has
ever run. That is the ordinary case, so the equality check passed
trivially across exactly the event it was added to catch. A residual I
called exotic was the default path.
The fix is the signal the store did not expose: `resetGenerationFor(ws)`,
a monotonic per-workspace count of drops, deliberately kept OUTSIDE the
`workspaces` map because `reset()` deletes that entry. Both existing
counters — `scopeEpoch` and the internal `generation` — live on the state
object and restart with its replacement; they are safe only because their
readers hold a REFERENCE to the object, which a caller outside the module
cannot. It is bumped even when the reset found no state to drop, so a
purge racing a first bootstrap does not read as no purge.
`createRelationTarget` now asks two questions rather than one:
* `indexStillOurs()` — is this the index the request was authorized
against? It gates the UPSERT, which was previously unconditional on
the argument that a real row belongs in the index. That argument does
not survive a purge: a brand-new id was never in `upsert`'s fenced
set (nothing to fence — the row did not exist when the purge ran), so
the write lands and is persisted to IDB, resurrecting a row into a
workspace the user may have just lost access to. This is the gap
BUG-2098's own comment describes.
* `stillWaiting()` — is the user still waiting on THIS create? It gates
the link and the toast, and it is now ONE predicate rather than two
hand-copied condition lists. The failure path had drifted from the
success path by exactly the reset half (round 6 P2); sharing the
predicate is what stops that recurring.
Matrix: 27 mutants, all killed; baseline and restore both 91/91. New
store surface carries its own suite, including a CONTROL asserting that
`scopeEpochFor` genuinely cannot answer this question — if that ever stops
holding, the cheaper round-5 fence was sufficient after all and this
accessor should go.
…-2877) Codex review round 7. Two taken, one answered in the code. P1 — `resetGenerationFor` counted `reset()` and missed the OTHER drop. `bootstrap()`'s unauthorized/forbidden branch clears `state.items`, resets the MiniSearch index and wipes the persisted cache without going through `reset()`, so the fence added in round 6 did not see the revocation case it exists for. Both droppers now call one `markWorkspaceDropped(ws)` helper. Two call sites, because deleting the state entry and clearing rows in place are genuinely different operations; the helper is what makes the pairing greppable, and a test fails if a third site starts clearing rows without it. That test is STRUCTURAL, and deliberately so. Reaching the 401 branch through the front door needs a warm cache plus a pending resync plus a 401 from /items-changes — a fixture larger than the invariant it would check, and I tried it first. The invariant that actually has to hold is "clearing rows and counting the drop travel together". The site-count assertion is what keeps it honest: a NEW clear site fails loudly rather than going silently unexamined, which is how this kind of instrument usually rots. Its own mutant (the 401 branch stops counting) dies. P2 — a TRUNCATED cold page is not an answer to "does this exact title exist"; the row may be on a page nobody fetched. `SearchResponse` carries `total`, so `coldAnswered` now requires a complete page. Same defect as trusting the local ranker's window, arriving from the server side — the third variant of one mistake, which is why the rule is now stated once and asked everywhere: offer only where something authoritative has answered. P2 (query change mid-create) was raised for the second time, having been declined in round 3 with reasons that lived only in a commit message — which a reviewer reading the diff never sees. The reasoning is now a comment beside the fences: the three that exist each stand for an act meaning "not this one" (escaping out, choosing another row, landing on a different item or workspace); typing is mid-thought, the user did ask for the item being created, and cancelling would orphan that row with the field still empty. A decision worth keeping is worth putting where the next reader is looking. Matrix: 29 mutants, all killed; baseline and restore both 93/93.
…K-2877)
Codex review round 8, one P1, and the mechanism checks out in the server
source rather than only in the abstract.
Round 7 gated the cold answer on `(res.total ?? rows.length) <= rows.length`.
`store.search` makes that unreliable in exactly the case it was guarding:
when the count query errors it sets `total = -1`, floors it to 0, and then
floors it again to `len(results)` — "Ensure total is never less than actual
results", `internal/store/search.go:604-608`. So a broken count is
indistinguishable on the wire from an exact-fit page, and the check calls
it complete. The `?? rows.length` fallback was the same mistake a second
time: unknown read as fine, which is the polarity error rounds 3 and 5
already went around on `coldFailed`.
Completeness now comes from the PAGE: a page SHORTER than the limit the
server echoes back is proof there is no next page, and that holds whatever
the count did. A full page is not proof either way, so it does not count
as an answer. No `total` in the decision at all.
The U8 fixtures now carry the real response shape. `total`, `limit` and
`offset` are non-optional on `SearchResponse` and the Go handler always
sends them, so `{ results: [] }` was not a smaller version of a real
response — it was one that cannot occur, and it was quietly deciding the
very question these tests are about.
Matrix: 31 mutants, all killed; baseline and restore both 93/93. M28b —
the previous `total`-based implementation — SURVIVED at first, and the
fixture was why: it asserted against `total: 84, limit: 2`, which both
implementations reject. The leg that discriminates is the floored one
(`total: 2` on a full page of 2 with 84 really matching), i.e. the shape
the server actually emits when the count fails. A mutant that survives
because the fixture never reproduces the real failure is a fixture
finding, not a code finding.
…877)
Codex review round 9, two P1s, both declined — and the reasoning goes
beside the fences rather than into a commit message, which is the lesson
round 7 taught when a round-3 decline was re-raised because a reviewer
reading the diff had no way to see it.
A CONCURRENT FIELD CHANGE (SSE, another tab) landing mid-POST is ordinary
last-write-wins on a field the user is actively editing, and it is what
every other type in this component already does — a text field blurred
after a remote change overwrites it too. The race is adjudicated at the
server: `ItemDetail.updateField` sends `expected_updated_at` and
refetch-retries a 409 (BUG-2273 / IDEA-1480). Fencing it here would make
relation fields alone behave differently from every other field, on a rule
the item's own optimistic-concurrency check already enforces.
A LOST RESPONSE on a create that committed is real and is not fixable
here. `item create` has no idempotency key and titles are not unique
(colliding slugs get `-2` suffixes, `store.uniqueSlug`). Nothing
auto-retries — a retry is a person clicking Create again with the picker's
state in front of them — and the repo's standing rule for the identical
shape is exactly that ("Never retry it automatically" for `item copy`).
Filed as IDEA-2880. Deliberately NOT patched client-side: checking for a
same-title item before retrying would rest on the same ranked, paged,
possibly-stale evidence the create row itself rests on, and would look
like a guarantee the client cannot make.
No behaviour change; gates re-run rather than assumed — 2104 web tests,
svelte-check 0 errors.
Codex review round 10. The refresh effect read `collection` inside `untrack`, so a relation field whose declared target CHANGES under an open picker — a schema edit, or an SSE-driven collection refresh; `ItemDetail` does not remount the picker for either — kept listing rows from the collection it used to point at, still selectable under the new scope. Everything else in that effect is untracked to keep it off the keystroke path, and the scope was swept up in that. But `collection` is not a per-keystroke value: it is the question the results answer. Predates this unit — it arrived with the U3 extraction (TASK-2862) — and is fixed here rather than filed because U8 makes `collection` load-bearing in a new way: it is now the destination an inline create writes to, so a stale scope means rows from one collection listed beside a create row aimed at another. The test drives the change through a NEW single-prop setter on `ItemPickerProbe`, not through `rerender`. That distinction is the whole reason the probe exists, and its own header says so: `rerender` replaces the entire props object and re-runs the effect whether or not it tracks the prop under test, so a rerender-driven version of this test passes against the untracked build. Verified rather than assumed — the mutant that restores `untrack` dies against the setter version. Matrix: 32 mutants, all killed; baseline and restore both 94/94.
…-2877)
Codex review round 11 — the tail of round 10's fix, and mine.
The refresh effect now tracks the scope, but it returns early for
server-sourced non-empty queries. That early return is right for an index
DELTA — the index is not that caller's source of truth, and a request per
delta is the rate-limiter pressure the debounce exists to avoid — and
wrong for a scope CHANGE, where the rows on screen are answers to a
different question and stay selectable under the new scope. A scope change
happens when a schema is edited or a pane is retargeted, not per delta, so
the rate-limiter argument does not reach it.
Two lines that looked like guards went, both measured rather than argued:
* `void collection` — the scope pair reads `collection` to build itself,
which IS the subscription, so the separate read added nothing and its
mutant could not be killed.
* the `lastScope !== null` first-run guard — at mount the query box is
empty, and the only reader of `scopeChanged` needs a non-empty query,
so the first run cannot change an outcome either way.
`lastScope` starts null rather than seeded from the props: seeding
captured their mount-time values outside a reactive scope, which
svelte-check flagged (`state_referenced_locally`) — two warnings this
branch introduced and has now removed. svelte-check is back to the six
pre-existing warnings in files this branch does not touch.
Matrix: 34 mutants, all killed; baseline and restore both 95/95.
Codex review round 12, and the tail of round 11's fix. `lastScope` was committed as soon as the effect computed it, before the not-ready branch — which clears the picker and gives up WITHOUT serving the scope. So a scope change arriving while the workspace state is dropped was recorded as handled by the run that handled nothing: at hydration `scopeChanged` read false, a server-sourced picker took the early return, and it sat empty until the user retyped or it remounted. Committed now only by a run that is actually going to serve the scope. Leaving it stale is what keeps the refresh owed. Matrix: 35 mutants, all killed; baseline and restore both 96/96. The new one — committing `lastScope` early again — dies on the added leg.
…(TASK-2877) Codex review round 13, and it collapses round 12's fix into a simpler one. `lastScope` means "the scope the rows on screen answer for". The not-ready branch REMOVES those rows, so afterwards they answer for nothing — which is what null says, and the next run therefore owes a refresh whether or not the scope itself moved. Leaving the old value there meant rehydrating on the SAME workspace and collection compared equal, so a server-sourced picker took the early return and sat empty permanently: its rows were cleared and nothing was left to re-query it. Round 12 deferred the COMMIT past the early return to keep a cold-window scope change from being forgotten. With this invalidation in place that deferral changed no outcome — its mutant could not be killed — so it went and the commit moved back to where the value is computed. One rule stated once, rather than two mechanisms aimed at two halves of it. Also hardened the mutation harness, after it bit: a harness timeout kills the runner with SIGTERM, which does not run `finally`, so an earlier killed run left the working tree MUTATED. I then read a pre-existing test "failing" in that tree and had a plausible defect and a fix half-written before checking the file — the failure was M31's mutant, not my change. The runner now restores from its backups on SIGTERM/SIGINT/SIGHUP. Cheap, and the alternative is reasoning about code nobody wrote. Matrix: 35 mutants, all killed; baseline and restore both 97/97.
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.
Unit U8 of the relation-fields design pass ([[PLAN-2857]]), closing TASK-2877. Blocked only by U3 (merged
7fde7a3c), so it was startable while U1 waited on #1240.What it does
When the scoped relation picker's query matches nothing — or nothing exactly — it offers a trailing
+ Create "<query>" in <Collection>row, keyboard-reachable like any other row. Choosing it creates the item in the field's declared target collection, sets the field to the new item's id, and renders U2's chip.The affordance is opt-in at the call site: the picker offers it only when handed an
oncreate. Both of the unit's scope rules are expressed that way rather than by the picker knowing them — "relation fields only" is the Relationships tab passing nothing, and the permission gate isFieldEditorwithholding it whencanEditCollectionsays no for the target.The proving test, driven end to end in a real browser
Scratch instance on port 7891 with its own temp HOME + SQLite, built from this branch. Nothing shared was touched: not
:7777, not the installed binary, not the shared Postgres on 5445.Chartreuseagainst Cars/Colors. Options were exactly["+ Create \"Chartreuse\" in Colors"];aria-expandedtrue;ArrowDownputaria-activedescendanton the create row; Enter created it.CAR-5.fields.colorholdsCOLO-6's uuid. First leg, end to end.{"status":"approved"}— the schema's declared default. The Colors fixture was deliberatelyoptions: [draft, approved]withdefault: approvedso the two disagree, because the collection page's "+ New" guessesstatus.options[0]and would have writtendraft. This unit sends no fields and lets the server apply defaults (items.ValidateFields→ "Marshal validated/defaulted fields back").["COLO-6 Chartreuse"], create row count 0, Enter selected the existing row, colors still 3. Second leg.align-items: stretchbug that 30 component tests missed.One rule, asked in four places
Every gate here is the same sentence: offer only where something authoritative has answered. It took five review rounds to state it once instead of five times.
readybutpendingResync/searchanswered a complete pageCompleteness is read from the page length against the echoed limit, never from
total: when the count query fails,store.searchsetstotal = -1, floors it to 0, then floors it again tolen(results)(internal/store/search.go:604-608), so a broken count is indistinguishable on the wire from an exact-fit page.Gates
npx vitest run— 124 files, 2108 passed, 0 failednpm run check— 1093 files, 0 errors, 6 warnings, all six pre-existing and in files this branch does not touch (this branch introduced two and removed them again)npx vite build— clean ·make build-go— cleanCodex review: 13 rounds, 20 findings
17 taken, 3 declined with the reasoning in the code — a round-3 decline that lived only in a commit message got re-raised in round 7, because a reviewer reading the diff never sees a commit message.
Round 6 corrected a residual round 5 had dismissed as needing a coincidence:
reset()deletes the workspace state and the replacement starts atscopeEpoch0, which is also the value whenever no resync ever ran — so the equality check passed trivially across the exact event it was added to catch. That needed new store surface,localIndex.resetGenerationFor(ws), deliberately kept outside theworkspacesmap becausereset()deletes that entry.Round 14 could not be obtained: three consecutive 10-minute timeouts at 0 bytes. Not a broken CLI — two other sessions were running concurrent
codex execjobs on this machine. Reported rather than rounded up to CLEAN.Follow-up filed
[[IDEA-2880]] —
item createhas no idempotency key, so a lost response makes a retry duplicate. Deliberately not patched client-side: checking for a same-title item before retrying rests on the same ranked, paged, possibly-stale evidence the create row itself rests on, and would look like a guarantee the client cannot make.