Codex round 1 named four; three were real P1s and I verified each in the
code before accepting it. Codex could not run anything ("Go could not
create its build cache because the workspace is read-only"), so every
finding here is a static read that I confirmed and pinned.
## A THIRD ORIGIN, not two
`items.MigrateFields` injects the DESTINATION schema's defaults for keys the
source item has nothing for. My classifier split on `supplied` versus
everything-else, so a destination default was filed as CARRIED — and on a
cross-workspace copy every carried relation drops without a lookup. The
destination's own default was discarded and reported `referent_not_portable`,
which is flatly false about a value the destination chose.
There are three origins: SUPPLIED (refuse on failure), CARRIED from the
source item (cross-workspace: drop as not-portable), and DESTINATION DEFAULT
(resolve against the destination in BOTH modes; drop with the resolver's own
reason on failure, because nobody in this request typed it). Telling the last
two apart needs the source field map, which all four migrate doors have as
`currentFields`, so it is now a parameter.
Empty values are skipped at every origin. An empty relation is a cleared
field, not a referent, and reporting it as dropped tells a user they lost
something they never had.
WHAT THE FIRST VERSION OF THIS TEST PROVED: nothing. The mutant that reverts
the classifier SURVIVED it. `ValidateFields` re-injects the default after my
resolver deleted the key, so the value comes back either way and
`StillDropped` filters the false report out — the end state is identical
unless the default is a REF. A UUID default is already its own canonical
form, so "resolved" and "dropped then re-injected raw" produce the same
bytes. With `PEOP-1` as the default the mutant is DETECTED, because only a
resolved default lands as the id.
## SUPPLIED OVERRIDES AT THE MIGRATE DOORS SKIPPED THE VISIBILITY CHECK
The four write doors go through `s.resolveRelationReferents`, which adds
`checkItemVisible` on top of the store resolver. The migrate doors called
`store.MigrateRelationReferents` directly — it is a store function and cannot
answer a request-scoped question — so their SUPPLIED half, which this unit's
own rule calls an ordinary write, resolved against the database alone. A
caller able to edit both collections could point a relation at an item they
cannot see.
The ROLE is the part worth getting right. For `move` it is `workspaceRole(r)`.
For copy and preflight it is the caller's role in the DESTINATION, and
`CrossWorkspaceAccess.Role` is exactly that — its own doc says never to
substitute `workspaceRole(r)`. So `resolveRelationReferents` now takes the
role explicitly (`resolveRelationReferentsAs`), and the new
`refuseInvisibleRelationOverrides` runs at all three doors with the right one.
At the copy it runs in the HANDLER, before the store call: a pre-write
refusal must not open a transaction to roll it back, and the preflight runs
the identical check — DR-6's "the preview IS the copy" only holds if both
doors refuse the same request.
## BULK COLLECTION MOVE DISCARDED FIELDS SILENTLY
`bulkMoveCollection` has populated `result.Dropped` since MigrateFields
existed and NOTHING read it — the only reference in the file was my own
append. BUG-2674 fixed the single-item door and left this one, so a bulk move
discarded values with no record anywhere. Pre-existing, and routing relation
drops into the same dead list is what made it mine to fix.
Reported on the activity row, same key, same joined-string shape and the same
BUG-2628 reason as `handleMoveItem`, filtered against the final map so the
report is true when written. Threaded as an out-parameter, deliberately: only
this branch produces drops, the caller needs them for ONE activity row per
item, and a third return value would put `nil` in fourteen unrelated returns.
## THE P2, AND WHAT IT IS NOT PINNED BY
`resolveRelationReferents` did `if item == nil { continue }` after the
visibility read — "treat a race as someone else's 404". It now refuses with
the same `not_found` the resolver would have given moments later. This whole
unit exists to keep a dangling referent out of the blob, and a target that
vanished mid-request is the one case where waving it through would have been
deliberate.
NO TEST. Reproducing it means deleting a row between two reads inside one
request, and a test that faked that would pin the fake. Stated here rather
than left to look covered.
## Counterfactuals
Every fix has a mutant that its own test detects, each build-checked first:
classifier reverted -> DETECTED (destination default); empty-skip removed ->
DETECTED; visibility helper neutered -> DETECTED at both the move door and
the copy/preflight pair; bulk-move report removed -> DETECTED.
Gates: internal/server ok 224.6s · internal/store ok 261.5s · internal/items
ok · internal/mcp ok 15.1s · go vet clean · gofmt clean · make lint 0 issues.
xarmian
ed03d488da