fix(items,server,web): reserved metadata survives a move; referential metadata travels only within its context (BUG-2674) - #1165
Merged
Conversation
…lds are reported (BUG-2674)
Moving an item destroyed its implementation notes, decision log and linked-PR
metadata. Well-formed data, on a routine documented operation, silently, with a
success message.
Reproduced before the fix: a note written through `pad item note` — correct
shape, visible on every surface — was gone after `pad item move`, leaving
fields as `{"status":"new"}`.
## Why it happened
items.MigrateFields drops every key absent from the TARGET schema. The reserved
keys — implementation_notes, decision_log, github_pr, convention — are system
metadata that NO collection schema declares; each renders from its own dedicated
surface rather than as a generic field. So they are absent from every targetDefs
and were dropped on every move.
That blindness is structural, not incidental: any code path reasoning about
fields BY CONSULTING A SCHEMA cannot see these keys. It is the shared root of
this bug and of BUG-2627, where the CLI types a --field value by schema lookup
and these keys fall through to a raw string.
## The enumeration comes first, deliberately
Before this there were four constants and exactly ONE non-test consumer treating
them as a set — an inline || chain in a CLI display path. Naming the set inline
again here would have created the SECOND hand-maintained list, which is the
generator pattern behind both bugs reproduced inside its own fix: the next
reserved field lands in the constants, gets wired into whichever surface
prompted it, and silently misses the other.
So models.IsReservedItemField is now the single place that knows, MigrateFields
consults it, and the CLI's || chain is converted to it — the only way it is
provably THE list rather than A list. (formatChangeValue keeps its per-key
switch: it needs to know WHICH reserved key it has, to say "notes" vs "entries",
not whether the key is reserved.)
`convention` is IN the set, settled with evidence rather than by the principle
alone: 35 of 36 conventions in a live workspace do not store the key at all, and
the one that does holds a blob that is a redundant mirror of the alias keys
beside it. No user types a `convention` object — ApplyItemConventionMetadata
writes it, via library activation and the web form. System-stamped.
## Contract
System-minted non-referential data carries; anything dropped is reported.
PLAN-2357 DR-17 settled the analogous case — tags carry because "there is no
workspace-scoped foreign key to break, so dropping them would lose information
for no safety reason". These are the same shape: inert JSON with nothing that
could dangle in a destination. The plan's carry list simply never considered
them, so there was no deliberate semantics to defer to. DR-17's own heading is
"None of this may be silent."
## The reporting half
MigrateResult.Dropped has always existed and the single-move handler has always
thrown it away, so the only record of a field disappearing was the field being
gone. It now rides the move's audit metadata — not the response body, which is
the bare item and would break every consumer, and the activity timeline is where
someone asking "what happened to my item" looks. Joined into one string because
that map is map[string]string and a raw array renders as a Go map literal in the
timeline (BUG-2628).
## Verified
Unit: reserved keys carry with their payload INTACT (asserted on the value, not
merely the key — a carry that re-encoded or zeroed it would pass a presence
check), and bypass schema matching entirely, so a target declaring
`implementation_notes` as `text` cannot coerce them. Mutants run: guard removed
-> both new tests fail; carried-but-also-reported-dropped -> the not-dropped
assertion fails; carry-everything -> the control leg fails alongside three
pre-existing tests.
Live, against a server built from this branch: the note survives the move
byte-identical, and the move's activity metadata carries
`dropped_fields: "priority, status"` for the values the target schema genuinely
has no home for.
## Known scope limit
The BULK move path still discards its Dropped list — a reporting gap only, since
the carry-through lives in MigrateFields and bulk inherits it. Threading the list
out crosses two function boundaries whose signatures serve every bulk operation,
so it is a refactor of the bulk dispatch's return contract rather than a line.
Filed as BUG-2683 rather than smuggled in here.
Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V
…-through (BUG-2674) Round 1 raised no P1 and four P2s. Three were real defects introduced or exposed by the carry-through; one was a genuine overclaim in the previous commit. All four closed here, each mutation-verified rather than asserted. ## A schema may no longer declare a reserved key MigrateFields carries these keys by identity, but every caller then validates against the target schema — and ValidateFieldsDetailed iterates schema.Fields, so it DOES see a declared key. A target declaring implementation_notes as `text` would receive the carried array and reject it, turning a move that previously destroyed the notes into one that fails outright. That is a worse failure than the one being fixed: loud, but it blocks an operation that used to work. The gate already existed — validateNoReservedFieldKeys, with its grandfathering — and listed only parent/plan. The four metadata keys join it, sourced from models.ReservedItemFieldKeys() so the two lists cannot drift. Forbidding the declaration is the honest fix; coercing the value, or skipping validation for a key the schema genuinely declares, would be guessing at which meaning the author wanted. The web's RESERVED_FIELD_KEYS gains the same four, preserving the existing deliberate asymmetry (the client lowercases and is therefore stricter than the server's exact match) so the UI steers authors away before the 400. ## The copy preflight no longer under-reports `carried` is built by walking the DESTINATION SCHEMA, and these keys are declared by no schema anywhere — so after the carry-through they appeared in NEITHER bucket. A copy of an item whose content is its notes would report "nothing carries over" while in fact retaining them. Before the carry-through they at least showed under `dropped`, accurately. Reporting in neither is a regression in the preflight's honesty, which is the same defect class as the move that reported nothing. They are now appended to `carried` after the schema-ordered entries, marked `type: "system"` with a rendered label since they have no author-supplied one. The bucket's doc comment says so: a client must no longer assume every `carried` entry resolves to a destination FieldDef. ## The audit report now reaches a human The previous commit claimed the activity timeline is where someone asks "what happened to my item" — true, and the timeline renderer ignored the key, so the report existed only for API and CLI consumers. Stored-but-invisible is not reported. TimelineActivityCard renders the dropped keys on a move. ## Test aliasing The "untouched" assertions compared the result against the SAME objects passed in, so an in-place mutation would change both sides and DeepEqual would stay true. The expectations are now independent deep copies — the only thing that makes "untouched" mean untouched. ## Mutants, each run Preflight pass removed -> the carried assertion fails. Timeline block disabled -> the render assertion fails. Timeline action guard dropped -> the non-move negative leg fails (a presence-only test would have passed it). Reserved-set helper returning everything -> the IsReservedItemField control leg fails. ## Not fixed here Codex's remaining observation — that a cross-workspace copy now carries github_pr into a workspace whose repository it does not describe, and leaves a convention blob detectable on an item outside the conventions collection — is a product question about what a copy MEANS, not a defect in this mechanism. Raised for a ruling rather than decided inside a bug fix. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V
…s context (BUG-2674) Lead ruling on the copy-semantics fork Codex round 1 raised. It does not add an exception to the carry rule — it applies the qualifier the rule already had. The contract was "system-minted NON-REFERENTIAL data carries". github_pr is referential: it names a repository that is a property of the SOURCE workspace's project, and it hydrates into code_context and renders as a live PR link. Carried into another workspace that link is a false statement about the destination's project, not preserved information. implementation_notes and decision_log describe the item's own history and are true wherever the item is. So the rule stays one sentence: non-referential system data carries everywhere; referential system data carries only where its referent's context still holds. ## Scope is a required argument MigrateFields takes items.MigrateScope. Required rather than defaulted because BOTH wrong answers lose something: SameWorkspace on a cross-workspace copy carries a PR link into a workspace it does not describe, and CrossWorkspace on an ordinary move DROPS metadata from an item whose repo context never changed. A caller that must name its scope cannot pick one by omission. The two move handlers pass SameWorkspace as a property of the endpoint, not a guess — a move changes an item's COLLECTION and cannot change its workspace. The copy and its preflight COMPUTE it by comparing workspace ids rather than assuming cross-workspace, because that endpoint accepts a target_workspace equal to the source; hardcoding would drop a github_pr from a same-workspace duplicate. Both sides use the same helper, or the preview promises a carry the copy drops — the DR-6 divergence the shared endpoint exists to prevent. ## The drop is reported, with a reason that explains itself PLAN-2357 DR-17: "None of this may be silent." It would be perverse to reintroduce a silent drop inside this fix's own new branch. The preflight reports it as `referent_not_portable` rather than the generic `no_target_field`. That generic reason would be actively misleading here: no schema declares these keys ANYWHERE, so "the destination has no such field" is equally true of the source and explains nothing about why the value is being left behind. ## Verified Mutants run: scope ignored (always carry) -> the cross-workspace leg fails; generic reason on the preflight drop -> the reason assertion fails. The same-workspace leg and the non-referential-sibling leg are what stop an implementation that ignores scope in EITHER direction from passing — each half alone is satisfiable by a constant. Gates re-run for THIS commit: lint 0 · go test ./... 0 · make test-pg 0 (3282). Web gates NOT re-run and not claimed: this commit touches no web file (the web half of BUG-2674 shipped in 82577a7 and is unchanged here). ## Noted, not fixed handlers_items_copy_preflight.go already documents the same defect class for RELATION fields — a same-named relation carries a SOURCE-workspace item id across workspaces and is reported as a clean carry — and says the fix "belongs in MigrateFields, for both callers at once". MigrateScope is now the mechanism that comment asks for, but wiring relation fields through it is a separate change with its own semantics to settle. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V
… drop reports, scope coverage (BUG-2674) Round 2 raised no P1 and three P2s plus a nit. All four were real; two are defects in round 1's own fixes. ## Grandfathered schemas that already declare a reserved key Round 1 added the four metadata keys to validateNoReservedFieldKeys, which stops the collision being CREATED — and that gate deliberately GRANDFATHERS schemas that already have one. I did not follow through: such a FieldDef still reached ValidateFieldsDetailed, met the system-owned array MigrateFields hands through by identity, and rejected it. A collection whose only sin is a field name someone was once allowed to pick would fail every move and copy. ValidateFieldsDetailed now skips reserved keys outright. That is not "ignoring validation": these values have no user-authored schema to validate against, by design — the schema entry is the anomaly, not the value. ValidateFields inherits it through the same call. This also closes the second half of the same finding: the preflight could report one key in BOTH needs_value and carried, because the issue came from validating a key the carried-append also emits. No issue, no collision. ## Dropped reports that were no longer true MigrateFields computes Dropped BEFORE overrides merge and before defaults are injected, so a key it lists may have been supplied moments later. Both the move audit (which I added in this branch) and the preflight's dropped bucket reported those anyway — claiming "we discarded your due_date" about an item that HAS a due_date. That is worse than the silence it replaced: silence at least does not send someone hunting for data sitting on the item, and a report that cries loss over visible data teaches the reader to distrust the channel. items.StillDropped filters against the FINAL map so the report is true at the moment it is written. ## Scope coverage attachments_copy_plan_test models a copy from workspace A into B and passed SameWorkspace — the wrong scope stated confidently in a test whose whole subject is a cross-workspace copy. It came from the bulk edit that threaded the argument through, which picked a value rather than reading each fixture. And nothing proved the MUTATING copy honours scope at all, so a call site passing the wrong one — precisely the mistake a required argument exists to prevent — would have shipped green. TestCopyEndpoint_ReferentialMetadataTravels- OnlyWithinItsWorkspace covers both directions end to end. Mutant run: the store call site pinned to SameWorkspace now fails the cross-workspace leg. ## The nit was an overclaim, so it is fixed in the code 38fa8fe said the copy and its preflight "use the same helper". They did not — the helper lived in the server package and the store duplicated the comparison inline, which is how a preview and its copy drift apart. items.ScopeFor now lives in the package that defines the type and both call it. Gates: lint 0 (after a gofmt fix lint caught) · go test ./... 0 · make test-pg 0 (3283). No web file touched; web gates not re-run. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V
…d finish the drop-report fix (BUG-2674)
Codex round 3, no P1, two P2s. Both say round 2's fixes were applied at the
wrong altitude — correct in the case in front of me, wrong for the callers I
did not enumerate.
## The validation skip was global; the problem is local
Round 2 made ValidateFieldsDetailed skip reserved keys. That validator is shared
with create, full update, artifact import and every bulk path — none of which
migrate anything. On a GRANDFATHERED schema (one that already declared a
reserved key before the round-1 gate), those paths genuinely did validate the
key, and the skip stopped them: arbitrary junk could be written into
implementation_notes through create, while fields_patch kept rejecting it via
ValidatePartialFields. Full and partial updates disagreeing about the same key
is a worse bug than the one I was fixing.
Reverted. items.SchemaForMigratedFields strips reserved FieldDefs from the
schema used to validate the OUTPUT of a migration, and only the four migration
and copy sites call it. Create and update keep enforcing the declaration,
because on those paths the user really is authoring that key.
## StillDropped reached two of three surfaces
The move audit and the preflight were filtered; the MUTATING copy was not.
migrateCopyFields returned the raw pre-override list and the 201 response
exposes it as warnings.dropped_fields — so one request could report the key
carried in the preview, PERSIST it, and still call it dropped in the copy's own
response. Three surfaces, two answers.
## And StillDropped's own test was too weak
Presence is not the test — present-and-non-nil is. The move path writes
overrides straight into the map including a nil, where the copy path deletes the
key, so `{"due_date": null}` on a move left the key present carrying nothing.
Treating that as restored suppresses a REAL drop, which is the silent loss this
change exists to end.
## A mutant survived, and the fixture was why
`out.Fields = schema.Fields[:0]` + appends mutates the caller's backing array.
The first version of the input-not-mutated assertion passed it twice: once
because it checked length (Go passes the struct by value, so the caller's slice
HEADER survives), and again after fixing that, because the reserved key was LAST
in the fixture — the one surviving field was written back into the slot it
already occupied. With the reserved key FIRST the corruption lands in slot 0 and
the mutant dies. Recorded in the test, because the next person writing a
"does not mutate its input" assertion in Go will reach for len() too.
## Comment accuracy
The reserved-set doc claimed callers "inherit additions without edits". True for
membership tests, false for the three places that need something a set cannot
supply — referentialItemFieldKeys, reservedFieldLabel, and the web's separate
RESERVED_FIELD_KEYS. Now listed, with the test that fires as the reminder. The
collections-handler comment described only parent/plan and now says it covers
two unrelated groups.
Gates: lint 0 · go test ./... 0 · make test-pg 0 (3285). No web file touched.
## Flagged, not fixed
The preflight labels a destination DEFAULT as from:"migrated" when the source
had the key but migration dropped it — origin is keyed on presence in the source
map, not on where the final value came from. Pre-existing and untouched by this
branch; filed separately rather than folded in.
Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V
…ride holes, duplicate carried entries (BUG-2674)
Round 4, no P1, three P2s. All three are the same case I kept half-fixing: a
GRANDFATHERED schema that declares a reserved key.
## Reserved declarations were still live in the defaults pass
MigrateFields carried reserved keys by identity but then ran the target schema's
defaults/required loop over them unchanged. A legacy Default was injected into
system metadata as though a user had authored it, and a legacy Required produced
a migration ERROR — which bulk move rejects on BEFORE reaching the
stripped-schema validation. So a legacy target requiring implementation_notes
failed bulk move while single move and copy succeeded: same key, same item, two
answers depending on which button was pressed.
## Overrides were a hole straight through the rule
A field override naming a reserved key was merged and then validated against the
STRIPPED schema — i.e. not validated at all. Two consequences, the second worse
than the first:
- arbitrary junk could be written into implementation_notes / decision_log,
bypassing the append guard BUG-2627 exists to enforce;
- on a cross-workspace copy, an override could reintroduce the github_pr that
MigrateFields had just dropped for leaving its workspace — defeating the
scope rule by the simplest available route.
The copy paths now gate overrides against the stripped schema, so a reserved key
is undeclared there by construction and takes the existing malformed_override
refusal. The MOVE path had no declared-key gate at all and gets a dedicated one
(items.ReservedOverrideKeys). Refused rather than silently dropped: a caller who
asked for a value and got an item without it has no way to tell.
## The preflight emitted reserved keys twice
The carried walk iterated the raw target schema, so a grandfathered declaration
was emitted there AND appended again by the reserved pass. The existing
preflight/copy parity helper collapses carried entries into a map, so it could
not see it — a check that de-duplicates before comparing cannot detect
duplication. The walk now uses the stripped schema.
## Two mutants survived, and both were the test's fault
- The defaults fix had no test at all. Written after the fact, it fails on the
unfixed code on both halves (injected default, spurious required error).
- The override test passed with the stripping REMOVED, because the ordinary
destination does not declare github_pr — so UndeclaredOverrideKeys refuses it
either way. Only a schema that DECLARES the key distinguishes the two
implementations. The grandfathered fixture added for that fails the mutant
with the PR link visibly written onto the copy.
Also added the falsy-value legs to StillDropped (false / 0 / "" are
restorations, not absences — a truthiness filter would report them lost) and
drove SchemaForMigratedFields off the canonical set so a mutant stripping only
implementation_notes fails.
Gates: lint 0 · go test ./... 0 · make test-pg 0 (3289). No web file touched.
Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V
Codex round 5. The previous commit message said reserved keys are refused "on any path". True only for FIELD-OVERRIDE maps — the same-workspace move, the copy preflight and the mutating copy. An ordinary `fields` / `fields_patch` map still reaches them from the CLI, MCP, the web editor, artifact import, and Pad's own note / decision / convention / GitHub writers, which is by design for the system writers and a pre-existing exposure for the rest. The doc comment now says which paths it covers and, more importantly, what it is NOT — a general write gate. That distinction is the kind a future reader would otherwise take on trust from the function name. Round 5 was asked a different question than rounds 1-4: not "what is wrong with this diff" but "enumerate every path that could meet a declared reserved key, and is this approach right at all". It found ~10 further latent sites (create, full and partial update, artifact import, bulk status/priority, terminal options, unique_scope, computed, the web field editor, search, share presentation) — all PRE-EXISTING, none regressions from this branch, and all in the same grandfathered-schema case rounds 3, 4 and 5 kept surfacing. They are filed as BUG-2685 with the full map rather than patched here. Four rounds each finding another site is evidence about the DESIGN — reserved metadata living in the generic fields blob means every schema-aware consumer has to remember a special rule — and that is TASK-2657's territory, not a bigger version of this bug. This branch's scope was: a move destroys system metadata. That is fixed, tested and mutation-verified. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V
…reads them; ToolSurfaceVersion 0.22 (BUG-2674) Caught by the pre-push step my own record exists for: I had documented this change carefully in commit messages, the PR body and the item trail — every one of them read by a human REVIEWING the work — and not at all in the artifacts read by the agent or operator ACTING on it. That is the same miss twice before, both times in this exact file. `field` is accepted for `pad_item.action=move` (catalog_item.go), so the refusal this branch adds is a limit an MCP agent will hit. It now says so in the param's own description and in instructions.md, which is the text agents receive at handshake. CLAUDE.md's `pad item move` and `pad item copy` blocks — the operator- facing reference — gain the carry rules and the github_pr exception. ## ToolSurfaceVersion 0.21 -> 0.22 BEHAVIOR bump on the v0.9 / v0.16 / v0.17 grounds: no tool, action enum or param SHAPE changed, but two things an agent can observe did. A move used to DESTROY implementation_notes / decision_log / github_pr / convention, silently, and now preserves them; drops of ordinary fields are reported in the move's activity entry instead of vanishing. And a `field` setter naming one of those keys answers `malformed_override` instead of writing it — a write that was never legitimate, since it bypassed BUG-2627's append guard and could reintroduce a github_pr the migration had just dropped. Compat posture stated deliberately: a caller passing such a setter today gets a 400 where it previously got a silent corrupt write. Relying on the old behaviour is relying on a defect — the same reading v0.17 took for the fields-blob shadowing. The bump was not free, which is the point: TestInstructionsMDVersionMatchesTool- Surface and TestReadmeVersionMatchesToolSurface both went red and forced the two other surfaces to be updated. That is the enforcement working — a version constant nobody could change without visiting every place it is published. Gates re-run for this commit: lint 0 · go test ./... 0 · make test-pg 0 (3289). CI was already 7/7 green on f6775bc; pushing this restarts it, which is the correct trade against shipping agent-facing docs that describe the old behaviour. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V
xarmian
added a commit
to b4rk13/pad
that referenced
this pull request
Aug 20, 2026
…22 -> 0.24 (0.22/0.23 were taken by PerpetualSoftware#1165/PerpetualSoftware#1166) Mechanical drift absorption, maintainer-side: main took 0.22 (BUG-2674) and 0.23 (BUG-2627 part 2) while this PR was in review, so its fields- object + strict-validation bump becomes 0.24. Conflicts resolved in favor of main's newer refusal semantics; the PR's fields sentence and strict-validation line are grafted into the current docs. CLAUDE.md's MCP section (stale at 0.21 on main) gains brief 0.22/0.23 entries pointing at version.go. Claude-Session: https://claude.ai/code/session_01BhQoeaWXxJbvw86ezzK8dt
5 tasks
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
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.
Closes BUG-2674.
The defect
Moving an item destroyed its implementation notes, decision log and linked-PR metadata. Well-formed data, on a routine documented operation, silently, with a success message.
Reproduced before the fix — a note written through the canonical
pad item notepath, correct shape, visible on every surface:items.MigrateFieldsdrops every key absent from the target schema, and the reserved keys —implementation_notes,decision_log,github_pr,convention— are system metadata that no collection schema declares. So they were absent from everytargetDefsand dropped on every move.That blindness is structural: any code path reasoning about fields by consulting a schema is, by construction, unable to see them. It is the shared root of this bug and of BUG-2627.
The contract
PLAN-2357 DR-17 settled the analogous case — tags carry because "there is no workspace-scoped foreign key to break, so dropping them would lose information for no safety reason." The plan's carry list simply never considered these keys, so there was no deliberate semantics to defer to. DR-17's own heading is "None of this may be silent."
github_pris the referential one: it names a repository belonging to the source workspace's project and renders as a live PR link. Carried into another workspace that link is a false statement, not preserved information. So it carries on an intra-workspace move and drops — reported asreferent_not_portable— on a cross-workspace copy. Notes and decisions describe the item's own history and are true wherever the item is.conventioncarries, settled with evidence rather than by the principle alone: 35 of 36 conventions in a live workspace don't store the key at all, and the one that does holds a redundant mirror of its alias keys. No user types aconventionobject.What changed
models.IsReservedItemField) — added first, deliberately. Naming the set inline would have created the second hand-maintained list, which is the generator pattern behind both bugs, reproduced inside its own fix. The pre-existing inline||chain converts to it.MigrateFieldscarries reserved keys by identity, before any schema lookup, and skips them in the defaults/required pass so a grandfathered declaration can't inject a default or raise a spurious required error.items.MigrateScopeis a required argument. Both wrong answers lose something —SameWorkspaceon a cross-workspace copy carries a PR link into a workspace it doesn't describe;CrossWorkspaceon a move drops metadata whose context never changed. A caller that must name its scope can't pick one by omission. Moves passSameWorkspaceas a property of the endpoint; copy and preflight compute it via the shareditems.ScopeFor, since that endpoint accepts a same-workspace target.validateNoReservedFieldKeysgate, which grandfathers existing ones), with the web'sRESERVED_FIELD_KEYSmatched.items.ReservedOverrideKeys. Without this, an override could reintroduce thegithub_prmigration had just dropped, defeating the scope rule by the simplest route.items.StillDroppedso a key restored by an override or default isn't falsely reported lost.Forbidding the declaration is the honest fix; coercing the value, or skipping validation for a key the schema genuinely declares, would be guessing at which meaning the author wanted.
Verification
Every mutant was run and the killing assertion recorded: guard removed · carry-everything · scope ignored · generic drop reason · wrong-but-compiling type parameter · presence-only
StillDropped· in-place schema mutation · prepend · fresh fields map · unstripped override gate.Two mutants survived first time, and both were the test's fault — recorded in the test comments because the next reader will make the same mistakes:
len(). Go passes the struct by value, so the caller's slice header survives an in-place rewrite. And after fixing that, the fixture still hid it: with the reserved key last, the surviving field is written back into the slot it already occupied. Reserved key first makes the corruption visible.github_pr— the gate refuses it either way. Only a schema that declares the key distinguishes the two implementations. A guard with two branches needs an input where the branches differ.Live, against a server built from this branch: the note survives the move byte-identical, and the move's activity carries
dropped_fields: "priority, status"for values the target genuinely has no home for.Gates
make lint— 0 issuesgo test ./...— exit 0make test-pg— exit 0, 3289 testsnpm run check— 0 errors ·npm run test— 1710 passed (run for82577a74, the only commit touching web; later commits touch no web file)Codex rounds, and why the loop stopped
Findings ran 4 → 3 → 2 → 3 P2s, never a P1. From round 3 onward every finding was the same case: a grandfathered schema declaring a reserved key. Round 5 was pointed at a different question — enumerate every path that could hit this, and is the approach right at all — and returned ~10 further latent sites plus a design assessment.
All ~10 are pre-existing, checked site by site against this diff, none regressions. They're filed as BUG-2685 with the full path map and both design directions, cross-linked to TASK-2657 (collection traits) where the structural fix belongs. Also filed: BUG-2683 (bulk move discards its dropped report — a refactor of the bulk dispatch's return contract, not a line) and BUG-2684 (preflight labels a default as
from:"migrated").Stopping is legitimate because those are filed, not despite it. The interpretation is recorded on CONVE-735 for the next seat.
https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V