revert: drop the preflight carried-source robustness change (TASK-2878)

Lead ruling, day-57 02:4xZ: drop round-15 finding 3 from a119cf39 in one
commit. It shipped as a documented robustness change whose mutants survived at
every door, with no regression test, because the test written for it passed
against the unfixed build and was deleted.

The ruling's reasoning, which I agree with: a change with no observable
behaviour and no test is not robustness, it is prose in code. The instrument
answered "not a defect", so there is nothing to fix, and CONVE-30 says the
answer is only as wide as the question I asked — I asked whether the stale set
was reachable, got "no" at three doors and "no, unobservably" at the fourth,
and then shipped a fix anyway on a naming argument.

Reverted, exactly and only:

- handlers_items_copy_preflight.go: the visibility call takes `carriedSource`
  again, the pre-relation-drop snapshot.
- internal/server/relation_referents.go: `carriedAfterRelationDrops` and its
  50-line doc comment removed.
- internal/store/relation_referents.go: `RelationIssueKeys` removed — it had
  exactly one caller, the helper above, and is dead without it.

Findings 1 and 2 from a119cf39 are UNTOUCHED and still pinned. Verified by
diffing against the parent rather than by reading this diff: against 21dfb115
the preflight now shows only the `collapseInvisibleRelationIssues` call
(finding 2), and internal/store/relation_referents.go is byte-identical to the
parent.

The finding stays recorded — here and on TASK-2878's trail — as REPORTED, NOT
A DEFECT, CLASS-CHECKED, so round 16 and later stop re-reporting it. The class
grep and both probe legs are in a119cf39's message, which remains the record of
what was measured.

Gates on this tip: build, `go vet` (server+store), `gofmt -l internal/` all
clean. `go test ./internal/server -run 'TestRelationDoors|TestImportArtifact'`
ok 10.479s — the pins for findings 1 and 2, run because this commit touches
the file one of them lives in. The full SQLite suite and Postgres are owed on
this tip and are NOT claimed here.

Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8
This commit is contained in:
xarmian
2026-09-05 02:38:15 +00:00
parent a119cf39c3
commit ace2d3f56e
3 changed files with 1 additions and 72 deletions
@@ -823,7 +823,7 @@ func (s *Server) handleCopyItemPreflight(w http.ResponseWriter, r *http.Request)
}
invisibleDefaults, invErr := s.dropInvisibleRelationDefaults(r, dst.WorkspaceID(), dst.Role,
items.SchemaForMigratedFields(targetSchema), final,
notDefaultKeys(input.FieldOverrides, carriedAfterRelationDrops(currentFields, migrated.Dropped, relDropped)))
notDefaultKeys(input.FieldOverrides, carriedSource))
if invErr != nil {
writeInternalError(w, fmt.Errorf("copy preflight: relation default visibility: %w", invErr))
return
-50
View File
@@ -321,56 +321,6 @@ func (s *Server) refuseInvisibleRelationOverrides(
return s.resolveRelationReferentsAs(r, workspaceID, role, schema, probe)
}
// carriedAfterRelationDrops is the carried-source set a migrate door hands
// notDefaultKeys, with BOTH drop passes subtracted.
//
// A door computes its carried set from MigrateFields' drops, then runs
// store.MigrateRelationReferents, which drops further keys whose referents did
// not resolve. Reusing the FIRST set for the visibility call classifies a key
// the second pass dropped as still carried; when the destination schema then
// refills that key with a default, the default is exempted from the visibility
// check on the strength of a value that is no longer there, and the response
// can hand back the id of an item the caller cannot see.
//
// The pre-relation-drop set is still the right input to the origin label and
// to the relation classifier itself, which are asking "did this value come
// across from the source?" — a different question from "is the value in hand
// now a destination default?" (codex round 15).
//
// ONLY the preflight needs this, and the reason is worth stating so nobody
// "fixes" the other doors to match. Move and bulk move fold their relation
// drops into `result.Dropped` and then recompute CarriedSourceValues INLINE at
// the visibility call, so they read the already-extended list and were never
// wrong. The preflight extends `migrated.Dropped` the same way but hands the
// visibility call a `carriedSource` VARIABLE captured before that append —
// the snapshot is the defect, not the door.
//
// SHIPS WITH ITS MUTANTS SURVIVING, AND THAT IS RECORDED RATHER THAN HIDDEN.
// Restoring the stale set — at the move door OR here — leaves every test
// green, because no configuration I could construct makes the difference
// observable: a destination default whose target the caller cannot see is
// already collapsed to `not_found` by the MAIN pass and dropped before this
// check is reached. Measured with a probe on both builds — the owner's
// preflight discloses the default's id, and a restricted editor's reports
// `owner_ref` dropped as `not_found` on the fixed AND the unfixed build.
//
// So this is a robustness change, not a bug fix, kept for one reason:
// `carriedSource` should mean what its name says at every use, rather than
// being correct at three uses and stale at the fourth because two later passes
// happen to repair it. A regression test WAS written for this and then
// DELETED, because it passed against the unfixed build — a test that cannot
// fail is worse than no test, since it reads as a guard while guarding
// nothing.
func carriedAfterRelationDrops(sourceFields map[string]any, migrateDropped []string, relDropped []store.RelationIssue) map[string]any {
if len(relDropped) == 0 {
return store.CarriedSourceValues(sourceFields, migrateDropped)
}
lost := make([]string, 0, len(migrateDropped)+len(relDropped))
lost = append(lost, migrateDropped...)
lost = append(lost, store.RelationIssueKeys(relDropped)...)
return store.CarriedSourceValues(sourceFields, lost)
}
// notDefaultKeys is the set a migrate door hands
// dropInvisibleRelationDefaults: the caller's own values plus the values
// carried from the source item. Everything else in the map came from the
-21
View File
@@ -745,27 +745,6 @@ func CarriedSourceValues(sourceFields map[string]any, dropped []string) map[stri
return out
}
// RelationIssueKeys is the key set of a relation-issue slice, for callers that
// need to subtract what the relation pass dropped from a set captured BEFORE
// it ran.
//
// The migrate doors capture their carried-source set from MigrateFields'
// drops, then run MigrateRelationReferents, which drops MORE keys. A set
// captured before the second pass and reused after it names keys that are no
// longer carried — and if the destination schema refills one with a default,
// that default is then treated as carried and skips the visibility check it
// needs (codex round 15).
func RelationIssueKeys(issues []RelationIssue) []string {
if len(issues) == 0 {
return nil
}
out := make([]string, 0, len(issues))
for _, ri := range issues {
out = append(out, ri.Key)
}
return out
}
// hasKey reports whether m declares key. A nil map has no keys, which is how a
// door with no overrides (bulk move) or no source item says so.
func hasKey(m map[string]any, key string) bool {