mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-10 15:05:40 +00:00
fix: three codex round-15 findings — two real, one unobservable (TASK-2878)
Round 15 (internal/server) returned four findings. One re-reported the known
IDEA-2886 gap, already recorded unfixed. The other three are handled here, and
two of the three were only settled by trying to kill them.
1. IMPORT SKIPPED THE DEFAULT CLEANUP (real, fixed, pinned). Step 2 of
resolveRelationsForWrite returned early whenever the caller's values had
issues — correct while every caller of that path REFUSED, since steps 3 and
4 only prepare a field map nobody stores. Checkpoint 12's carry posture
added a caller that does not refuse, and the early return stayed: an
artifact holding one junk relation skipped the late-default pass AND the
default-visibility drop, stored whatever the destination schema injected,
and reported none of it. The early return is now posture-aware; the carry
report is threaded through the remaining steps rather than lost.
This is the fourth time in this unit that one of my own fixes opened the
next round's defect, and the first where the interval was a day.
Mutant: restore the unconditional early return -> FAIL, warnings list
`role` only and `owner_ref` is stored raw as 42.
2. LATE DEFAULTS BYPASSED THE VISIBILITY COLLAPSE (real, fixed, pinned).
store.ResolveLateRelationDefaults cannot know who is asking, so every issue
it returns carries the raw reason, and every door renders those into a
caller-visible message. `wrong_collection` is the one reason that names a
LIVE item, so a schema default pointing at an item the caller cannot see
announced that it EXISTS — the oracle round 3 closed on the main pass,
reopened through the door round 10 added. Collapse hoisted into
collapseInvisibleRelationIssues and applied at ALL FIVE late-default sites,
not the one the reviewer named (CONVE-18).
Mutant: remove the collapse at the move door -> FAIL, the 400 reads
`"LV-2" is not an item in collection "people"` to a caller who cannot see
LV-2. Control leg included: a caller who CAN see the target keeps the
specific reason, so collapsing everything to not_found fails too.
3. STALE CARRIED-SOURCE SET (kept as robustness, NOT a bug fix, documented).
Reviewer named the preflight. Grepping the class found three doors with the
shape — and the mutants say only one of them could ever have been wrong,
and even that one is unobservable:
- 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. Restoring the stale form leaves their tests
green because there is nothing there to break. Both edits REVERTED.
- the preflight does hand its visibility call a variable captured before
that append, but restoring it leaves every test green too: a default
whose target the caller cannot see is already collapsed to not_found by
the MAIN pass and dropped before this check runs. Probed on both builds —
the owner's preflight discloses the default's id, a restricted editor's
reports owner_ref dropped as not_found either way.
Kept because 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 repair it. The disposition is in the helper's doc comment.
A regression test was WRITTEN for this and then DELETED: it passed against
the unfixed build, and a test that cannot fail is worse than none — it reads
as a guard while guarding nothing. CONVE-18 asks for the population of a
class; it does not license assuming every member is defective.
Gates: build, go vet, gofmt clean on this tip. The full SQLite suite
(server/store/items/mcp) was STILL RUNNING when this was committed and its
result is NOT claimed here. Postgres has NOT been re-run since 21dfb115.
Both are the successor's first two steps.
Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8
This commit is contained in:
@@ -234,3 +234,79 @@ func TestImportArtifactCarriesUnresolvableRelationValue(t *testing.T) {
|
||||
t.Fatalf("import carried an unresolvable relation without saying so: %v", resp.Warnings)
|
||||
}
|
||||
}
|
||||
|
||||
// An import carrying ONE unresolvable caller value must still clean up the
|
||||
// destination schema's own broken defaults (codex round 15).
|
||||
//
|
||||
// The write path's step 2 returned early when the caller's values had issues —
|
||||
// correct while every caller of that path REFUSED, since steps 3 and 4 only
|
||||
// prepare a field map nobody stores. The carry posture made a caller that does
|
||||
// not refuse, and the early return stayed: an artifact holding one junk
|
||||
// relation skipped the late-default pass AND the default-visibility drop
|
||||
// entirely, storing whatever the destination schema injected and reporting
|
||||
// none of it.
|
||||
//
|
||||
// This fixture is the two existing artifact-relation tests composed, and the
|
||||
// composition is the whole point — each half passes on its own against the
|
||||
// unfixed build, and only together do they reach the skipped steps:
|
||||
//
|
||||
// - `role`, an unresolvable CALLER value, which is what fires the early
|
||||
// return (TestImportArtifactCarriesUnresolvableRelationValue);
|
||||
// - `owner_ref`, a broken destination DEFAULT that step 3 is what drops
|
||||
// (TestImportArtifactReportsDroppedRelationDefault).
|
||||
func TestImportArtifactCleansBrokenDefaultsDespiteACarriedJunkValue(t *testing.T) {
|
||||
srv := testServer(t)
|
||||
ws := createWSForTest(t, srv)
|
||||
|
||||
rr := doRequest(srv, "PATCH", "/api/v1/workspaces/"+ws+"/collections/conventions", map[string]any{
|
||||
"schema": `{"fields":[{"key":"status","type":"select","options":["draft","active"]},{"key":"role","type":"relation","collection":"nobody"},{"key":"owner_ref","type":"relation","collection":"nobody","default":42}]}`,
|
||||
})
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("narrow conventions schema: expected 200, got %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
|
||||
art := artifact.Artifact{
|
||||
Kind: artifact.KindConvention,
|
||||
FormatVersion: artifact.FormatVersion,
|
||||
Title: "Convention with junk AND a broken default",
|
||||
Fields: map[string]any{"status": "active", "role": "just some text"},
|
||||
Body: "Body.\n",
|
||||
}
|
||||
data, err := artifact.Encode(art)
|
||||
if err != nil {
|
||||
t.Fatalf("encode: %v", err)
|
||||
}
|
||||
|
||||
rr2 := doArtifactRequest(srv, "POST", "/api/v1/workspaces/"+ws+"/import-artifact", data)
|
||||
if rr2.Code != http.StatusCreated {
|
||||
t.Fatalf("import REFUSED (%d); import carries, never refuses: %s", rr2.Code, rr2.Body.String())
|
||||
}
|
||||
var resp struct {
|
||||
Ref string `json:"ref"`
|
||||
Warnings []string `json:"warnings"`
|
||||
}
|
||||
if err := json.Unmarshal(rr2.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode: %v\n%s", err, rr2.Body.String())
|
||||
}
|
||||
warnings := strings.Join(resp.Warnings, "\n")
|
||||
|
||||
// CONTROL: the carried junk value still reaches the report. If this leg
|
||||
// fails the fixture never fired the early return, and the assertion below
|
||||
// would be measuring a path that was never skipped.
|
||||
if !strings.Contains(warnings, "role") {
|
||||
t.Fatalf("the carried junk value was not reported, so the early return this test "+
|
||||
"exists to reach was never taken: %v", resp.Warnings)
|
||||
}
|
||||
|
||||
show := doRequest(srv, "GET", "/api/v1/workspaces/"+ws+"/items/"+resp.Ref, nil)
|
||||
if show.Code != http.StatusOK {
|
||||
t.Fatalf("GET the imported item: %d %s", show.Code, show.Body.String())
|
||||
}
|
||||
if strings.Contains(show.Body.String(), `"owner_ref":42`) {
|
||||
t.Fatalf("a carried junk value made the import skip the late-default pass, and the "+
|
||||
"schema's broken default was stored raw: %s", show.Body.String())
|
||||
}
|
||||
if !strings.Contains(warnings, "owner_ref") {
|
||||
t.Fatalf("the broken destination default was neither cleaned nor reported: %v", resp.Warnings)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -785,7 +785,7 @@ func (s *Server) createItemChecked(r *http.Request, workspaceID string, coll *mo
|
||||
// for one value, and after coercion so the value is in its final form.
|
||||
// The four steps live in one place — see resolveRelationsForWrite.
|
||||
relRefusals, droppedDefaults, relErr := s.resolveRelationsForWrite(
|
||||
r, workspaceID, workspaceRole(r), schema, fieldMap, relBefore)
|
||||
r, workspaceID, workspaceRole(r), schema, fieldMap, relBefore, posture)
|
||||
if relErr != nil {
|
||||
return nil, &itemCreateError{http.StatusInternalServerError, "internal_error", "Failed to resolve relation references"}
|
||||
}
|
||||
@@ -1267,7 +1267,7 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
|
||||
// Referent validation for relation values (TASK-2878) — the same four
|
||||
// steps the create door runs; see resolveRelationsForWrite.
|
||||
relRefusals, writeDropped, relErr := s.resolveRelationsForWrite(
|
||||
r, workspaceID, workspaceRole(r), schema, fieldMap, relBefore)
|
||||
r, workspaceID, workspaceRole(r), schema, fieldMap, relBefore, relationsRefuse)
|
||||
if relErr != nil {
|
||||
writeInternalError(w, relErr)
|
||||
return
|
||||
@@ -2469,6 +2469,10 @@ func (s *Server) handleMoveItem(w http.ResponseWriter, r *http.Request) {
|
||||
writeInternalError(w, lateErr)
|
||||
return
|
||||
}
|
||||
if cerr := s.collapseInvisibleRelationIssues(r, workspaceID, workspaceRole(r), lateDropped); cerr != nil {
|
||||
writeInternalError(w, cerr)
|
||||
return
|
||||
}
|
||||
// A REQUIRED relation whose default did not resolve cannot be left as a
|
||||
// drop: the key is deleted AFTER validation passed, so nothing re-checks
|
||||
// it and the item would land with a required field absent, reported valid
|
||||
|
||||
@@ -567,6 +567,9 @@ func (s *Server) bulkFieldUpdate(r *http.Request, workspaceID string, item *mode
|
||||
if lateErr != nil {
|
||||
return nil, &bulkOpError{message: "Failed to resolve relation references", code: "internal_error"}
|
||||
}
|
||||
if cerr := s.collapseInvisibleRelationIssues(r, workspaceID, workspaceRole(r), lateDropped); cerr != nil {
|
||||
return nil, &bulkOpError{message: "Failed to resolve relation references", code: "internal_error"}
|
||||
}
|
||||
if req := store.RequiredRelationIssues(schema, lateDropped); len(req) > 0 {
|
||||
return nil, &bulkOpError{
|
||||
message: "required fields missing: " + relationIssuesMessage(req),
|
||||
@@ -819,6 +822,9 @@ func (s *Server) bulkMoveCollection(r *http.Request, workspaceID string, item *m
|
||||
if lateErr != nil {
|
||||
return nil, &bulkOpError{message: "Failed to resolve relation references", code: "internal_error"}
|
||||
}
|
||||
if cerr := s.collapseInvisibleRelationIssues(r, workspaceID, workspaceRole(r), lateDropped); cerr != nil {
|
||||
return nil, &bulkOpError{message: "Failed to resolve relation references", code: "internal_error"}
|
||||
}
|
||||
// A REQUIRED relation whose default did not resolve cannot be left as a
|
||||
// drop: the key is deleted AFTER validation passed, so nothing re-checks
|
||||
// it and the item would land with a required field absent, reported valid
|
||||
|
||||
@@ -805,6 +805,10 @@ func (s *Server) handleCopyItemPreflight(w http.ResponseWriter, r *http.Request)
|
||||
writeInternalError(w, fmt.Errorf("copy preflight: resolve relation defaults: %w", lateErr))
|
||||
return
|
||||
}
|
||||
if cerr := s.collapseInvisibleRelationIssues(r, dst.WorkspaceID(), dst.Role, lateDropped); cerr != nil {
|
||||
writeInternalError(w, fmt.Errorf("copy preflight: relation issue visibility: %w", cerr))
|
||||
return
|
||||
}
|
||||
// A REQUIRED relation whose default did not resolve becomes a needs_value
|
||||
// row rather than a drop, so the preview says what the copy will do:
|
||||
// the copy REFUSES this request, and `valid` must be false (codex round
|
||||
@@ -819,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, carriedSource))
|
||||
notDefaultKeys(input.FieldOverrides, carriedAfterRelationDrops(currentFields, migrated.Dropped, relDropped)))
|
||||
if invErr != nil {
|
||||
writeInternalError(w, fmt.Errorf("copy preflight: relation default visibility: %w", invErr))
|
||||
return
|
||||
|
||||
@@ -197,6 +197,48 @@ func collapseIssue(issues []store.RelationIssue, key string, reason store.Relati
|
||||
}
|
||||
}
|
||||
|
||||
// collapseInvisibleRelationIssues rewrites `wrong_collection` to `not_found`
|
||||
// on any issue whose target the requester cannot see — the same collapse
|
||||
// resolveRelationReferentsAs applies to the MAIN pass, hoisted so the LATE
|
||||
// pass gets it too.
|
||||
//
|
||||
// `store.ResolveLateRelationDefaults` is a store function and cannot know who
|
||||
// is asking, so every issue it returns carries the raw reason. Those issues
|
||||
// reach a caller: each door feeds them to RequiredRelationIssues and renders
|
||||
// the result into a 400 or a preflight `needs_value` row. `wrong_collection`
|
||||
// is the one reason that names a LIVE item, so an invisible target announced
|
||||
// that way is the existence oracle round 3 closed, reopened through the door
|
||||
// round 10 added (codex round 15).
|
||||
//
|
||||
// Reviewer named ONE site; this is applied at all five late-default sites,
|
||||
// per CONVE-18 — the class is "a store-resolved issue reaching a caller
|
||||
// without passing the visibility collapse", not the one call it was spotted at.
|
||||
func (s *Server) collapseInvisibleRelationIssues(r *http.Request, workspaceID, role string, issues []store.RelationIssue) error {
|
||||
for i := range issues {
|
||||
if issues[i].Reason != store.RelationTargetWrongCollection {
|
||||
continue
|
||||
}
|
||||
target, terr := s.store.ResolveRelationTarget(workspaceID, issues[i].Value)
|
||||
if terr != nil {
|
||||
return terr
|
||||
}
|
||||
if target == nil {
|
||||
// Vanished between the two reads. The reason still SAYS the value
|
||||
// named something a moment ago, which is the same disclosure.
|
||||
issues[i].Reason = store.RelationTargetNotFound
|
||||
continue
|
||||
}
|
||||
seen, verr := s.checkItemVisible(workspaceID, target, currentUser(r), role, isBearerAuth(r))
|
||||
if verr != nil {
|
||||
return verr
|
||||
}
|
||||
if !seen {
|
||||
issues[i].Reason = store.RelationTargetNotFound
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// refuseRelationIssues writes the 400 a write door owes and reports whether it
|
||||
// did. One function so the six refusing doors cannot phrase the same refusal
|
||||
// three different ways, and so a client matching on the error code sees one
|
||||
@@ -279,6 +321,56 @@ 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
|
||||
@@ -412,21 +504,36 @@ func (s *Server) resolveRelationsForWrite(
|
||||
schema models.CollectionSchema,
|
||||
fieldMap map[string]any,
|
||||
presentBefore map[string]bool,
|
||||
posture relationPosture,
|
||||
) (refusals []store.RelationIssue, dropped []string, err error) {
|
||||
issues, err := s.resolveRelationReferents(r, workspaceID, schema, fieldMap)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
if callerIssues := store.IssuesForCallerInput(issues, presentBefore); len(callerIssues) > 0 {
|
||||
callerIssues := store.IssuesForCallerInput(issues, presentBefore)
|
||||
if len(callerIssues) > 0 && posture == relationsRefuse {
|
||||
// The write is about to be REFUSED, so steps 3 and 4 would only
|
||||
// prepare a field map nobody stores.
|
||||
return callerIssues, nil, nil
|
||||
}
|
||||
// A CARRYING door does not stop here, and this is the round-15 defect:
|
||||
// the early return was written when every caller of this path refused, and
|
||||
// stayed when the artifact-import door began carrying instead. An import
|
||||
// holding ONE unresolvable caller value would skip the late-default pass
|
||||
// AND the default-visibility drop entirely, storing whatever the
|
||||
// destination schema injected — including a default the caller cannot see
|
||||
// — and reporting none of it. The refusals are still returned; they are
|
||||
// the carry report, not a stop.
|
||||
|
||||
lateDropped, err := s.store.ResolveLateRelationDefaults(workspaceID, schema, fieldMap, presentBefore)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
if cerr := s.collapseInvisibleRelationIssues(r, workspaceID, role, lateDropped); cerr != nil {
|
||||
return nil, nil, cerr
|
||||
}
|
||||
if required := store.RequiredRelationIssues(schema, lateDropped); len(required) > 0 {
|
||||
return required, nil, nil
|
||||
return append(callerIssues, required...), nil, nil
|
||||
}
|
||||
|
||||
invisible, err := s.dropInvisibleRelationDefaults(r, workspaceID, role, schema, fieldMap, presentBefore)
|
||||
@@ -438,10 +545,14 @@ func (s *Server) resolveRelationsForWrite(
|
||||
// deleting a key after validation has passed leaves a required field
|
||||
// absent regardless of which list recorded it (codex round 13).
|
||||
if required := store.RequiredRelationIssues(schema, invisible); len(required) > 0 {
|
||||
return required, nil, nil
|
||||
return append(callerIssues, required...), nil, nil
|
||||
}
|
||||
for _, ri := range append(lateDropped, invisible...) {
|
||||
dropped = append(dropped, ri.Key)
|
||||
}
|
||||
return nil, dropped, nil
|
||||
// callerIssues is EMPTY on a refusing door — it returned above — and holds
|
||||
// the carry report on a carrying one. Returning it here rather than `nil`
|
||||
// is what keeps an import's unresolvable values named in the response now
|
||||
// that the carrying door runs to the end.
|
||||
return callerIssues, dropped, nil
|
||||
}
|
||||
|
||||
@@ -1087,3 +1087,70 @@ func TestRelationDoors_NullSourceDoesNotExemptDefaultFromVisibility(t *testing.T
|
||||
t.Fatalf("the hidden default was stored: %#v", v)
|
||||
}
|
||||
}
|
||||
|
||||
// A relation issue raised by the LATE default pass must pass through the same
|
||||
// visibility collapse the main pass applies (codex round 15).
|
||||
//
|
||||
// `store.ResolveLateRelationDefaults` cannot know who is asking, so it returns
|
||||
// the raw reason. `wrong_collection` is the one reason that names a LIVE item,
|
||||
// and every door renders these issues into a caller-visible message — so a
|
||||
// schema default pointing at an item the caller cannot see announced that the
|
||||
// item EXISTS. Same oracle round 3 closed on the main pass, reopened through
|
||||
// the late-default door round 10 added.
|
||||
func TestRelationDoors_LateDefaultWrongCollectionDoesNotDiscloseExistence(t *testing.T) {
|
||||
f := newDoorFixture(t)
|
||||
other := mustSchemaCollection(t, f.srv, f.ws.ID, "Late Vaults", `{"fields":[]}`)
|
||||
secret, err := f.srv.store.CreateItem(f.ws.ID, other.ID, models.ItemCreate{
|
||||
Title: "Late Secret", CreatedBy: f.owner.ID,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateItem(secret): %v", err)
|
||||
}
|
||||
|
||||
// REQUIRED, so the unresolved default becomes a refusal the caller reads
|
||||
// rather than a silent drop.
|
||||
dst := mustSchemaCollection(t, f.srv, f.ws.ID, "Late Default Door", fmt.Sprintf(`{"fields":[
|
||||
{"key":"status","label":"Status","type":"select","options":["open","done"]},
|
||||
{"key":"owner_ref","label":"Owner","type":"relation","collection":%q,"required":true,"default":%q}
|
||||
]}`, f.people.Slug, secret.Ref))
|
||||
|
||||
move := func(u *models.User, role string) string {
|
||||
t.Helper()
|
||||
item := f.seed(`{"status":"open"}`)
|
||||
rr := f.callAs(u, role, f.srv.handleMoveItem, "POST",
|
||||
"/api/v1/workspaces/"+f.ws.Slug+"/items/"+item.Slug+"/move",
|
||||
map[string]string{"itemSlug": item.Slug},
|
||||
map[string]any{"target_collection": dst.Slug})
|
||||
if rr.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400 for an unresolved REQUIRED relation default, got %d: %s",
|
||||
rr.Code, rr.Body.String())
|
||||
}
|
||||
return rr.Body.String()
|
||||
}
|
||||
|
||||
// CONTROL: a caller who CAN see the target keeps the specific, useful
|
||||
// reason. Without this leg, collapsing every reason to not_found would
|
||||
// pass the assertion below while destroying the message's value.
|
||||
seeing := move(f.owner, "owner")
|
||||
if !strings.Contains(seeing, "is not an item in collection") {
|
||||
t.Fatalf("a caller who CAN see the target lost the wrong_collection reason: %s", seeing)
|
||||
}
|
||||
|
||||
blind := mustUser(t, f.srv, "blind-late@example.com", "blindlate", "")
|
||||
if err := f.srv.store.AddWorkspaceMember(f.ws.ID, blind.ID, "editor"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||
}
|
||||
if err := f.srv.store.SetMemberCollectionAccess(f.ws.ID, blind.ID, "specific",
|
||||
[]string{f.tasks.ID, dst.ID, f.people.ID}); err != nil {
|
||||
t.Fatalf("SetMemberCollectionAccess: %v", err)
|
||||
}
|
||||
|
||||
hidden := move(blind, "editor")
|
||||
if strings.Contains(hidden, "is not an item in collection") {
|
||||
t.Fatalf("the late-default refusal tells a caller who cannot see the target that it "+
|
||||
"EXISTS: %s", hidden)
|
||||
}
|
||||
if strings.Contains(hidden, secret.ID) {
|
||||
t.Fatalf("the late-default refusal handed back the hidden item's canonical id: %s", hidden)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -745,6 +745,27 @@ 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 {
|
||||
|
||||
Reference in New Issue
Block a user