IDEA-2893 + IDEA-2894: record the accepted carry disclosure, and make the drop-reason mapper testable

internal/server/copy_preflight_drop_reason_parity_test.go

@@ -38,26 +38,35 @@ func readWebFile(t *testing.T, rel string) string {
 	return string(b)
 }
 
-// dropReasonSwitchBody returns just the body of CopyItemDialog's `dropReason`
-// function. Scoped rather than searched whole: the component has other
-// switches, and one of them matching `case 'not_found':` for an unrelated
-// purpose would satisfy this test while the dialog still rendered the raw enum
-// — a guard that passes on the wrong evidence.
-func dropReasonSwitchBody(t *testing.T, src string) string {
+// dropReasonMapBody returns the body of the renderer's message map.
+//
+// REPOINTED IN IDEA-2894, and the repointing is the reason this helper still
+// earns its keep. The mapper moved out of CopyItemDialog.svelte into
+// `web/src/lib/items/copyDropReasons.ts` so it could be unit-tested at all,
+// and this gate FAILED LOUDLY on that move — exactly as the fatal below says
+// it should — rather than passing on a file that no longer contained what it
+// was reading for. A parity gate that cannot tell "no such reason" from "no
+// such function" is worse than none.
+//
+// Scoped to the map's body rather than searching the file whole for the same
+// reason the old version scoped to the switch: the module also exports the
+// reason LIST, and a reason present there but missing a message would satisfy
+// a whole-file search while the dialog still rendered the raw enum.
+func dropReasonMapBody(t *testing.T, src string) string {
 	t.Helper()
-	const marker = "function dropReason(reason: string): string {"
+	const marker = "const MESSAGES: Record<CopyDropReason, string> = {"
 	start := strings.Index(src, marker)
 	if start < 0 {
-		t.Fatalf("CopyItemDialog.svelte no longer declares %q — this gate is reading for a "+
-			"function that moved or was renamed, so its green means nothing until it is repointed", marker)
+		t.Fatalf("copyDropReasons.ts no longer declares %q — this gate is reading for a "+
+			"declaration that moved or was renamed, so its green means nothing until it is "+
+			"repointed", marker)
 	}
 	rest := src[start+len(marker):]
-	// The function's own closing brace: the first line that is exactly a tab
-	// followed by `}`, matching the component's indentation for a top-level
-	// declaration in <script>.
-	end := strings.Index(rest, "\n\t}")
+	// The literal's own closing brace: the first line that is exactly `};` at
+	// column zero, matching a top-level declaration in the module.
+	end := strings.Index(rest, "\n};")
 	if end < 0 {
-		t.Fatalf("could not find the end of dropReason's body")
+		t.Fatalf("could not find the end of the MESSAGES map")
 	}
 	return rest[:end]
 }
@@ -70,18 +79,30 @@ func TestCopyPreflightDropReasonsAreRenderedByTheDialog(t *testing.T) {
 	}
 
 	types := readWebFile(t, "web/src/lib/types/index.ts")
-	dialog := readWebFile(t, "web/src/lib/components/items/CopyItemDialog.svelte")
-	switchBody := dropReasonSwitchBody(t, dialog)
+	renderer := readWebFile(t, "web/src/lib/items/copyDropReasons.ts")
+	messages := dropReasonMapBody(t, renderer)
 
 	for _, reason := range reasons {
 		t.Run(reason, func(t *testing.T) {
 			if !strings.Contains(types, "| '"+reason+"'") {
 				t.Errorf("ItemCopyPreflightDropped['reason'] in web/src/lib/types/index.ts has no "+
 					"member %q — the server can send it and the client's type says it cannot", reason)
 			}
-			if !strings.Contains(switchBody, "case '"+reason+"':") {
-				t.Errorf("CopyItemDialog's dropReason() has no case for %q, so the dialog falls "+
-					"through to `default: return reason` and shows a user the raw enum string", reason)
+			// BOTH halves of the renderer, because they fail differently.
+			// Missing from the LIST and the module's own completeness test
+			// cannot see it either — that test iterates the list, so a reason
+			// absent from it is absent from the test as well, and this gate is
+			// the only thing that notices.
+			if !strings.Contains(renderer, "\t'"+reason+"',") {
+				t.Errorf("COPY_DROP_REASONS in web/src/lib/items/copyDropReasons.ts does not list "+
+					"%q. That list drives the module's own completeness test, so a reason missing "+
+					"from it is invisible to that test too — this gate is the only place it shows.",
+					reason)
+			}
+			if !strings.Contains(messages, reason+":") {
+				t.Errorf("copyDropReasons' MESSAGES map has no entry for %q, so "+
+					"copyDropReasonMessage falls through to returning the raw string and shows a "+
+					"user the enum", reason)
 			}
 		})
 	}

internal/store/relation_referents.go

@@ -638,6 +638,46 @@ func (s *Store) MigrateRelationReferentsQ(
 		}
 	default:
 		// Same workspace: resolve, keep what resolves, drop what does not.
+		//
+		// WHAT SURVIVES HERE IS OBSERVABLE, AND THAT IS ACCEPTED (IDEA-2893,
+		// lead ruling day 58; the measurements it rests on are on that idea's
+		// trail, which is where to check this reasoning rather than take it).
+		//
+		// A carried value naming a live item in a collection the mover cannot
+		// see resolves and survives; one naming nothing is dropped. So a mover
+		// can tell those two apart, and on a stored REF they additionally
+		// learn the target's canonical id.
+		//
+		// It is accepted for a reason that was MEASURED rather than assumed,
+		// and the measurement is the part worth keeping:
+		//
+		//   1. NOT ENUMERABLE. A caller cannot choose what to test. Every
+		//      write door refuses a caller-supplied ref naming an item they
+		//      cannot see — create, update and fields_patch all answer 400
+		//      with the COLLAPSED `not_found` wording — so no door turns a
+		//      chosen value into a carried one. This can only ever confirm a
+		//      value already sitting in an item the caller can read and did
+		//      not put there.
+		//   2. THEY ALREADY HAVE THE VALUE. An ordinary GET returns the raw
+		//      stored relation value verbatim; reads apply no redaction. The
+		//      increment is "it currently resolves", plus the ref->id mapping.
+		//
+		// Every way of closing it costs more than the increment. Redacting the
+		// response closes nothing, because the canonical id is written into
+		// the blob and comes straight back from a plain GET. Not canonicalising
+		// removes only the id half and makes a relation value stop meaning one
+		// thing everywhere. Dropping by the MOVER's visibility silently
+		// destroys a valid relation because of who moved the item, which is
+		// the failure this whole carry rule exists to prevent. Canonicalising
+		// only for movers who can see the target would make the STORED BYTES
+		// depend on who performed the move.
+		//
+		// THE ONE THING THAT WOULD CLOSE IT is carrying unresolvable values
+		// verbatim instead of dropping them, so survival stops signalling
+		// anything — and that is exactly the drop-and-report rule three lines
+		// below, which exists to keep dangling referents out of the blob. So
+		// this comment is also a warning: if you ever change that rule, you
+		// are changing this too, in the other direction.
 		issues, resolveErr := s.ResolveRelationReferentsQ(q, workspaceID, schema, carried)
 		if resolveErr != nil {
 			return nil, nil, resolveErr

web/src/lib/components/items/CopyItemDialog.svelte

@@ -47,6 +47,7 @@ user hunting for an item that provably does not exist.
 	import Modal from '$lib/components/common/Modal.svelte';
 	import FieldEditor from '$lib/components/fields/FieldEditor.svelte';
 	import { api, PadApiError } from '$lib/api/client';
+	import { copyDropReasonMessage } from '$lib/items/copyDropReasons';
 	import { canEditCollection } from '$lib/utils/permissions';
 	import { parseSchema } from '$lib/types';
 	import type {
@@ -931,58 +932,11 @@ user hunting for an item that provably does not exist.
 		return String(v);
 	}
 
-	function dropReason(reason: string): string {
-		switch (reason) {
-			case 'no_target_field':
-				return 'no matching field in the destination';
-			case 'incompatible_type':
-				return 'the destination field has a different type';
-			case 'undeclared_source_field':
-				return 'not declared by this item’s own collection';
-			case 'assignee_not_a_member':
-				return 'the assignee is not a member of the destination';
-			case 'agent_role_not_portable':
-				return 'agent roles are workspace-local';
-			// BUG-2674 added this reason server-side and nothing here learned it,
-			// so it rendered through the fallback as the raw enum string. Rare
-			// while only `github_pr` produced it; routine since TASK-2878, which
-			// emits it for every carried relation value on a cross-workspace copy.
-			//
-			// NEUTRAL WORDING, for the same reason `not_found` below has it,
-			// and it took a second reviewer to see that the same rule applied
-			// here (codex round 18). This reason is emitted for EVERY carried
-			// cross-workspace relation WITHOUT resolving the target, so the
-			// value may name a live item, a deleted one, one the caller cannot
-			// see, or nothing at all — and `github_pr` reaches it too, where
-			// the referent is not in any workspace. "It points at something in
-			// the source workspace" asserted both existence and location, and
-			// the response says neither.
-			case 'referent_not_portable':
-				return 'this reference cannot be carried to the destination';
-			// The three same-workspace referent failures (TASK-2878). Worth
-			// separate sentences: "no such item" and "wrong collection" send the
-			// reader to different fixes, and a missing target is a schema problem
-			// rather than anything about this item.
-			// NEUTRAL WORDING, deliberately. `not_found` is what the server
-			// collapses a hidden target to as well as a missing one — telling
-			// them apart is the existence oracle it exists to prevent — so a
-			// sentence asserting non-existence is both wrong for half the
-			// cases and a claim the response cannot support.
-			case 'not_found':
-				return 'the item it refers to could not be found';
-			case 'wrong_collection':
-				return 'it refers to an item outside the field’s collection';
-			// Covers both "no target collection declared" and "the declared
-			// collection is not in this workspace"; the first wording named
-			// only the former and misdiagnosed the latter.
-			case 'target_missing':
-				return 'the field has no valid collection to link to';
-			case 'invalid_shape':
-				return 'the destination field’s default is not a valid reference';
-			default:
-				return reason;
-		}
-	}
+	// Extracted to `$lib/items/copyDropReasons` (IDEA-2894) so the mapping
+	// can be tested: two review rounds found defects in it and neither fix
+	// was pinned by anything, because it lived here unexported with no test
+	// file for this component.
+	const dropReason = copyDropReasonMessage;
 
 	/** Focus the first destination control when the dialog opens, so keyboard
 	 *  users land on the thing they must choose rather than the heading. */

web/src/lib/items/copyDropReasons.test.ts

@@ -0,0 +1,62 @@
+import { describe, it, expect } from 'vitest';
+import {
+	COPY_DROP_REASONS,
+	copyDropReasonMessage,
+	type CopyDropReason,
+} from './copyDropReasons';
+
+// The two tests IDEA-2894 exists to make possible. Neither could be written
+// while the mapper was inline in CopyItemDialog.svelte and unexported, which
+// is why two review-round fixes to it shipped unpinned.
+
+describe('copyDropReasonMessage', () => {
+	it('has a sentence for every reason the server can emit', () => {
+		// This is round 12's OTHER finding as a test: BUG-2674 added
+		// `referent_not_portable` server-side, nothing here learned it, and it
+		// rendered through the fallback as the raw enum string in front of a
+		// user. A reason that maps to itself is that defect.
+		const unmapped = COPY_DROP_REASONS.filter((r) => copyDropReasonMessage(r) === r);
+		expect(unmapped, 'reasons rendering as their own raw enum string').toEqual([]);
+	});
+
+	it('never claims a target exists, or says where it is', () => {
+		// Round 12 (`not_found` asserted non-existence) and round 18
+		// (`referent_not_portable` asserted existence AND location) were the
+		// same defect twice. The server collapses a HIDDEN target to
+		// `not_found`, and emits `referent_not_portable` without resolving the
+		// target at all, so any sentence that settles the question is a claim
+		// the response does not support.
+		//
+		// Asserted over the two reasons that carry the hazard rather than all
+		// ten: `wrong_collection` legitimately says the target is outside the
+		// field's collection, and it may — the server only emits it to a
+		// caller who can SEE the target.
+		const mustStayNeutral: CopyDropReason[] = ['not_found', 'referent_not_portable'];
+		const forbidden = [
+			/does not exist/i,
+			/no such/i,
+			/deleted/i,
+			/in the source workspace/i,
+			/points at/i,
+		];
+		for (const reason of mustStayNeutral) {
+			const message = copyDropReasonMessage(reason);
+			for (const pattern of forbidden) {
+				expect(
+					pattern.test(message),
+					`"${reason}" says "${message}", which asserts something the response does not`
+				).toBe(false);
+			}
+		}
+	});
+
+	it('shows an unknown reason verbatim rather than inventing a sentence', () => {
+		// A reason this build has never heard of means the server is ahead of
+		// the client. Showing the enum is more honest than a made-up sentence
+		// or a hidden row — and it is what makes the completeness test above
+		// meaningful rather than vacuous.
+		expect(copyDropReasonMessage('a_reason_from_a_newer_server')).toBe(
+			'a_reason_from_a_newer_server'
+		);
+	});
+});

web/src/lib/items/copyDropReasons.ts

@@ -0,0 +1,97 @@
+/**
+ * The copy/move dialog's drop-reason vocabulary, and the sentences shown for
+ * each one.
+ *
+ * EXTRACTED FROM `CopyItemDialog.svelte` (IDEA-2894). It lived inline in the
+ * component, unexported, with no test file for the dialog at all — and two
+ * separate review rounds found defects in it:
+ *
+ *   - round 12: the UI asserted NON-EXISTENCE from `not_found`, which the
+ *     server also emits for a target the caller merely cannot see. Telling
+ *     those apart is the existence oracle the server works to prevent.
+ *   - round 18: `referent_not_portable` read "it points at something in the
+ *     source workspace", claiming both existence and location for a reason
+ *     emitted WITHOUT resolving the target — and which `github_pr` also
+ *     reaches, where the referent is in no workspace at all.
+ *
+ * Neither fix was pinned by anything. A third defect of the same shape would
+ * have been found the same way — by a reviewer happening to read it — or not
+ * at all.
+ */
+
+/**
+ * Every drop reason the server can put in a preflight `dropped[].reason` or a
+ * copy's dropped-field report.
+ *
+ * KEPT IN THE SAME ORDER as the Go declarations so a diff of one against the
+ * other is readable. The first five are the migrate-level reasons in
+ * `handlers_items_copy_preflight.go`; the last five are the relation-level
+ * `RelationIssueReason` constants in `internal/store/relation_referents.go`.
+ *
+ * This list is the CONTRACT the completeness test checks the map against. It
+ * is duplicated from Go rather than generated, so it can go stale — which is
+ * exactly what happened when BUG-2674 added `referent_not_portable`
+ * server-side and nothing here learned it, and the reason rendered through the
+ * fallback as a raw enum string. The test cannot catch a reason added to Go
+ * and not added here; what it CAN catch is a reason added here without a
+ * sentence, and it makes this list the one place to update.
+ */
+export const COPY_DROP_REASONS = [
+	'no_target_field',
+	'incompatible_type',
+	'undeclared_source_field',
+	'assignee_not_a_member',
+	'agent_role_not_portable',
+	'referent_not_portable',
+	'not_found',
+	'wrong_collection',
+	'target_missing',
+	'invalid_shape',
+] as const;
+
+export type CopyDropReason = (typeof COPY_DROP_REASONS)[number];
+
+const MESSAGES: Record<CopyDropReason, string> = {
+	no_target_field: 'no matching field in the destination',
+	incompatible_type: 'the destination field has a different type',
+	undeclared_source_field: 'not declared by this item’s own collection',
+	assignee_not_a_member: 'the assignee is not a member of the destination',
+	agent_role_not_portable: 'agent roles are workspace-local',
+
+	// NEUTRAL WORDING (codex round 18). Emitted for EVERY carried
+	// cross-workspace relation WITHOUT resolving the target, so the value may
+	// name a live item, a deleted one, one the caller cannot see, or nothing
+	// at all — and `github_pr` reaches it too, where the referent is not in
+	// any workspace. Any sentence about WHERE the target is, or THAT it
+	// exists, is a claim the response does not support.
+	referent_not_portable: 'this reference cannot be carried to the destination',
+
+	// NEUTRAL WORDING (codex round 12). `not_found` is what the server
+	// collapses a HIDDEN target to as well as a missing one — telling them
+	// apart is the existence oracle the collapse exists to prevent — so a
+	// sentence asserting non-existence is both wrong for half the cases and a
+	// claim the response cannot support.
+	not_found: 'the item it refers to could not be found',
+
+	wrong_collection: 'it refers to an item outside the field’s collection',
+
+	// Covers both "no target collection declared" and "the declared collection
+	// is not in this workspace"; the first wording named only the former and
+	// misdiagnosed the latter.
+	target_missing: 'the field has no valid collection to link to',
+
+	invalid_shape: 'the destination field’s default is not a valid reference',
+};
+
+/**
+ * The sentence to show for a drop reason.
+ *
+ * An UNKNOWN reason returns the raw string. That is deliberate and is not a
+ * fallback to be tidied away: a reason this build has never heard of means the
+ * server is ahead of the client, and showing the enum is more honest than
+ * inventing a sentence for it or hiding the row entirely. The completeness
+ * test exists so that the known vocabulary never reaches this branch.
+ */
+export function copyDropReasonMessage(reason: string): string {
+	return MESSAGES[reason as CopyDropReason] ?? reason;
+}