feat: cross-workspace item copy (PLAN-2357) - #1048
Draft
xarmian wants to merge 3 commits into
Draft
Conversation
Phase 1 of PLAN-2357. Durable record of "this item was copied/moved from workspace A to workspace B", backing the forward redirect (TASK-2359) and the destination's back-pointer. Implements DR-2 / DR-2a. Paired, dual-dialect, forward-only migrations (migrations/077 + pgmigrations/055). archived_source distinguishes a move from a plain copy (INTEGER on SQLite, BOOLEAN on Postgres, written through dialect.BoolToInt). source_seq is a NULLABLE per-source move ordinal that exists solely so two moves inside the same second are orderable — created_at is second-precision RFC3339, so archive -> restore -> move again would otherwise resolve to an arbitrary destination. Partial index (source_item_id, source_seq DESC) WHERE archived_source, deliberately NOT unique: restore-then-move-again legitimately repeats. The back direction IS uniquely indexed — a destination item is created by exactly one copy, in the same transaction that writes its provenance row, so a duplicate there would silently change which source the back-pointer names. Cascade is asymmetric on purpose, inverting item_collection_moves: the archived source is precisely the row whose pointer must survive, so source_item_id carries no FK at all; target_item_id cascades, because a pointer at a vanished destination is worse than no pointer. Store accessors: a tx-taking insert helper (no self-committing variant — the row must land in the copy transaction), a forward lookup returning a SET newest-first, and a back lookup. The insert rejects an archived row with no seq and a copy row with one, so DR-2a's ordering invariant is enforced at the write boundary rather than assumed. NULL ordering is normalized with COALESCE because SQLite and Postgres disagree on DESC NULL placement. Also wires workspace purge, which the two-workspace shape requires: both workspace columns are RESTRICT references, so a purge clearing only one direction would fail outright when the purged workspace sits on the other end. Tests cover insert-in-tx, forward lookup with multiple destinations ordered newest-first and scoped to one source, back lookup, rollback leaving no row, and both DR-2a criteria. The ordering and scoping tests use fixed row IDs whose lexical order contradicts the expected answer, so deleting the ordering term or the WHERE clause under test fails them on every run rather than half the time; verified by mutating the production query. Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT
An item MOVED to another workspace — copied, then archived — can now say where it went. GET on a single item gains an optional `moved_to` block naming each destination in displayable terms (workspace slug + item ref + title + collection slug), so a consumer can render a link without a second call. No HTTP redirect, no resolver change. The ACL gate is the point. A destination is revealed only after the caller independently passes AuthorizeCrossWorkspaceRead (TASK-2358) with an ITEM scope on the destination item itself. Workspace-level access is not sufficient: a restricted member of the destination workspace, or a guest holding one unrelated item grant there, has a role in that workspace while having no right to the copied item's collection. A caller who fails that check sees NO hint a destination exists. The key is omitted entirely — not a null, not an empty array, not a boolean — so the response is byte-identical to an archived item with no move record at all. A structurally distinguishable response is itself the leak. Restore decision: the block is OMITTED for a non-archived source. Restoring a moved-out source leaves two live items with the same content in two workspaces, which is legitimate, but at that instant the source has not moved anywhere and the response must stop asserting that it did. Past-tense provenance is the back-pointer question and applies equally to plain copies, which this field must never claim as moves. Also honored: DR-2a (only archived_source rows feed the pointer; plain copies are back-pointer material only), per-destination filtering over the forward lookup's SET with no short-circuit on the first hit or first denial, newest-first ordering, a scan bound on the per-GET authorization cost, and deliberate isolation of the hand-rolled public share-link DTO — pinned by an explicit negative test that freezes its key set. Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT
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
1 participant
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.
Implements [[PLAN-2357]] — cross-workspace item copy. Copy is the primitive; move is copy + soft-delete the source (
--archive-source).Goal: take an item in workspace A and land it in workspace B, choosing the destination collection and resolving the field mapping, from both the CLI and the web UI.
pad item copy IDEA-2353 --to-workspace pad-web --collection tasks --dry-runreports the field mapping; without--dry-runit lands the copy; with--archive-sourceit archives the source and the archived source advertises where it went; and the same flow is reachable from the item pane menu in the web UI.Decision Record
The full DR-1…DR-17 lives in PLAN-2357. The load-bearing ones for review:
item_workspace_movestable.archived_sourcedistinguishes a move from a copy;moved_toresolves to the newestarchived_source = truerow ordered bysource_seq(RFC3339 is second-precision, socreated_atties).hashtextkey → both collection rows sorted by ID → source re-read under lock). A new store op, not a composition ofCreateItem+DeleteItem, with a creation-parity checklist (slug, seq, versions, wiki-links, status transitions).requireEditPermissiongives the wrong answer for a second workspace and the OAuth consent allow-list is enforced in exactly one middleware. Checks are collection- and item-level, in order, on both the source and destination sides, and on the dry-run as well as the mutation.pad-attachment:refs in the final payload, get fresh UUIDs, and the copied body is rewritten. Ref resolution is scoped to the source workspace or it is a confused-deputy hole.Tasks
Phase 1 — Foundation
item_workspace_movesprovenance table (dual-dialect migrations + store accessors)Phase 2 — Copy backend + CLI
CopyItemAcrossWorkspaces(DR-9, DR-14, DR-16)pad item copyCLI (DR-13)Phase 3 — Web UI + validation fix
Modal.svelte(DR-7)Rollout and rollback
This ships irreversible paired schema migrations (
migrations/077+pgmigrations/055), and Pad's migration guard deliberately refuses an older binary against a newer schema (internal/store/migration_guard.go:91). Redeploying the previous binary will be rejected by the guard, not silently degraded — once the migrations have applied, recovery is a pre-migration database snapshot or a forward fix. Whoever is on call should not be surprised by this.The new table is additive and nothing reads it until TASK-2359 lands, so the schema change itself is low-risk; the risk is in the one-way door.
Test plan
make check(lint →go test ./...→ govulncheck → web-check → web-test) on the integrated branch tip.make test-pg— mandatory, this plan touchesinternal/storeand ships paired migrations. SQLite passing does not prove Postgres passes.cd web && npm run checkonce Phase 3 lands any.svelte.Merged with
--merge(not--squash) — the per-task commit history is the artifact this model exists to produce.🤖 Generated with Claude Code
https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT