mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-10 15:05:40 +00:00
7659ad3cd3
* feat(server): the copy preflight says when a relation's target is not usable (IDEA-2899) TASK-2869 made a `needs_value` relation row collectable as soon as it names a target collection. Naming one is not having one: the slug can name a collection that has been DELETED, or one this caller cannot READ. The dialog then mounts a picker that can return nothing and, because the row is not blocked, Confirm stays disabled carrying only the generic required-field message — the user is told a value is missing and never told that no value is reachable. `collection_unavailable` on the needs_value row is the server saying so. THE CLIENT CANNOT COMPUTE THIS, which is why it belongs here. The dialog's destination collection list is filtered through `canEditCollection`, because it drives the copy-INTO picker; a relation TARGET needs only READ access, so a perfectly usable target routinely does not appear in that list. Testing against it would refuse rows the user could have filled in — over-blocking, which is the worse failure and invisible to whoever hits it. `visibleCollectionIDs` is the read-scoped view, and its NAV-LENIENT shape is right here rather than merely tolerable: it includes a collection reachable only through an item-level grant, and the question is "could a picker here return anything at all". One granted item is a picker with one row. DELETED and UNREADABLE are deliberately not distinguished. Same consequence, no client branch would differ — and separating them would tell a caller who cannot read a collection that it nonetheless exists. `omitempty` on a BOOL drops `false`, so the field is phrased NEGATIVELY. Present-and-true means the server checked and the target is unusable; ABSENT means available, or a server that does not report. A client must block only on an explicit true, so absence stays "no information" rather than becoming a value — the rule `access_epoch` follows on the item doors, and the one whose violation cost two review rounds on IDEA-2898 this morning. Costs nothing on the common path: a destination schema declaring no relation field runs no query at all. Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8 * test(server): pin the type gate on collection_unavailable (IDEA-2899) Found by a surviving mutant rather than by inspection: dropping the `def.Type == "relation"` gate left every other test in the file green. Nothing stops a schema declaring `collection` on a field of another type — the validator does not police keys it has no use for — and such a field would then pick up a flag whose meaning is defined only for relations. The dialog would block a perfectly collectable `select` because some relation elsewhere in the same schema points at a collection that happens to be gone. The fixture is the discriminating one: ONE deleted collection, TWO required rows that name it, and only one of them means anything by it. Six mutants on this half, all killed: flag never set, flag always set, deleted target not flagged, unreadable target not flagged, type gate dropped, and the nil-visible-set case (an admin's "no filtering" read as "nothing visible", which would flag every target for the callers who can see everything). Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8 * feat(web): block a relation whose target is unavailable, and stop advising a command that cannot work (IDEA-2899) The client half. `isCollectable` now refuses a relation row the server has flagged, so the row lands in `blockedFields`, Confirm is disabled with a reason, and no picker mounts that could only come back empty. `collection_unavailable !== true` is STRICT on purpose. The field is absent when the target is fine and absent from a server that predates it, so absence must read as "no information". (Over the domain the type admits — `boolean | undefined` — the truthiness spelling is EQUIVALENT and a mutant swapping it in survives; that is recorded in the source rather than papered over with an off-contract fixture. The strict form is kept because it states the contract where the next edit will read it, and the inverse spelling would block every row against an older server.) THE PART THAT IS NOT WIRING: the existing blocked-field notice said the field "is a required <type> field. This dialog can't collect a value for that type safely" and then printed `pad item copy … --field key=value`. Both halves are FALSE here. The type is perfectly collectable; the TARGET is gone. And the CLI runs as the same user against the same referent validation, so the command it prints is refused for exactly the reason the user is already stuck — advice that sends someone to do work that cannot succeed is worse than no advice. So the message branches on `uncollectableReason`, names the collection and the destination workspace, and the CLI line is now gated on `cliFillableField` — the first blocked row the CLI can ACTUALLY fill. `blockedFields[0]` was correct while every blocked row was type-shaped; with an unavailable relation sorted first it named the one field `--field` cannot set either. Eleven unit tests on `copyNeedsValue`, plus a source pin on the dialog whose own measured limit is in its docblock. Client mutants: 7 real, 6 killed, 1 recorded as equivalent with the domain argument that makes it equivalent. Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8 * feat(cli): the copy preview marks an unavailable relation target and stops suggesting it (IDEA-2899) Caught by `TestItemCopyMirrorsMatchServerShapes`, not by me. The CLI keeps a mirror of the preflight response, and adding a field server-side without mirroring it fails that test by design — a mirror that silently lags is a mirror that lies. Working exactly as intended, and the reason this half exists at all. Mirroring the field turned out to be the smaller part. The CLI already prints `target collection: people` for a relation row, and it builds an `Add: --field owner_ref=<value>` suggestion from every unsupplied row. Both are wrong when the target is unavailable: the first sends a user looking for a ref in a collection they cannot read, and the second hands them a command the referent validation refuses for exactly the reason they are already stuck. So the target line is marked NOT AVAILABLE, and the row is excluded from the suggestion with a sentence saying why — modelled on the empty-key branch, which was written for the identical reason (a `--field =<value>` nobody can run) and is three lines away. That the same defect had to be fixed in two places is the shape worth naming: the dialog and the CLI independently built "here is how to supply it" from "here is a field needing a value", and neither had a notion of a field that CANNOT be supplied. The empty-key case was the first instance and was fixed locally; this is the second. Five mutants on this half, all killed: suppression removed, suppression applied to everything, the unavailable label dropped, the explanation dropped, and the mirror field ignored. The available-target control leg is a separate test so the omitempty contract is exercised on this surface too. Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8 * fix(cli): route all three "how to supply it" sites through one predicate (IDEA-2899) Review found the fix applied at one door and not its siblings — my own recurring shape, arriving again. THREE places tell a CLI user how to resolve an unsatisfied field: the detailed `renderItemCopyNeedsValue`, the `--dry-run` summary, and the error the command returns. The first commit fixed the render. The other two went on printing `--field key=value` at someone for whom no value exists — and the ERROR is the line a script or a hurried reader actually sees, so it was the worst of the three to leave. `itemCopyUnfillable` is now the single definition all three consult. Not because three call sites are tidier than one, but because three sites independently answering "how do I supply this" is exactly how they diverged in the first place. The dry-run summary branches three ways rather than two, because the MIXED case is the one a boolean gets wrong: some fields can be supplied and some cannot, and collapsing that either suppresses advice the user needs or offers advice they cannot use. The error hint is suppressed only when NO field can be supplied — with one fillable field left, `--field key=value` is still true. Also pins the BOUNDARY the same review probed: a target collection that is live and readable but EMPTY is deliberately not flagged. The symptom looks identical — an empty picker — but the cases differ where it matters. An unavailable target is unfixable from inside the dialog, so blocking costs the user nothing they had; an empty collection is resolved by creating the item and retrying, and blocking would refuse a copy they were about to complete. It would also cost a live-visible-item count per relation target on a dry run the UI calls on every keystroke. The weaker case — an empty picker that says nothing about WHY — is filed as IDEA-2905 and belongs to the picker. Ten mutants across this round, all killed, including both directions on the error hint and both directions on the dry-run branch. Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8 * fix: unfillable means EITHER reason, and a select never names a relation target (IDEA-2899) Review round 2, two findings, both real and both about a rule stated in one place and enforced in another. **"Unfillable" answered for one of two reasons.** An EMPTY KEY cannot be supplied either — `--field =value` is rejected by this command's own parser, and the detailed render has explained that since Codex round 6. Only that render knew: the --dry-run summary and the returned error went on advising `--field` for those rows, because the predicate I extracted last commit covered the relation reason alone. A predicate named "unfillable" that answers for half its name is a worse trap than no predicate — right at the site that defined it, wrong everywhere it was reused, which is precisely what extracting it was meant to prevent. Two functions now: `itemCopyUnfillable` (either reason — advice), and `itemCopyUnavailableTarget` (the relation half — the render's own sentence, since the two explanations are not interchangeable to a reader). `itemCopyUnavailableTarget` deliberately does NOT also exclude empty keys, though my first version did. A row can carry both faults, and a mutant removing that exclusion survived every test — correctly, because all it changes is printing two sentences that are both TRUE about such a row. The guard was tidiness dressed as a rule; a condition nothing can distinguish is one the next reader has to re-derive. **`Collection` was emitted for non-relation fields**, while its own doc said it is empty for every other type. That was a claim about the schemas people write, not a property of the code: a `select` carrying `"collection": "people"` is storable — field validation has no use for the key and does not police it — and the value was copied straight through, so the CLI printed "target collection: people" beneath a select. A relation fact asserted about a field that has none. `relationTargetSlug` makes the documented contract true at the only place that can make it true; my own type-gate test had created exactly that shape and asserted only the FLAG, not the slug. Three mutants on these fixes, all killed. Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8 * fix(cli): the explanation now names the reason that actually applies (IDEA-2899) Review round 3, and the sharpest miss of this unit — my own, one commit old. Broadening what a predicate ACTS on silently broadened what a sentence SAYS. Once `itemCopyUnfillable` counted empty keys as well as unavailable relation targets, a set of empty-key rows selected the all-unfillable branch and was explained as "the relation target is not available to you" — a false statement about rows that contain no relation at all. Same in the returned error, which is the line a script sees. The tell was there to be read: a sentence that was TRUE while the predicate was narrower is a sentence to re-read the moment it widens. I broadened the predicate deliberately, wrote a commit message about how a half-answering predicate is a trap, and left the sentence describing the half. `itemCopyUnfillableWhy` names the reasons actually present — relation targets, empty keys, or both — and the two one-sentence sites consult it. The detailed render is unchanged: it explains each reason where the row is printed, which is why it uses the narrower count. Four mutants, all killed, including the two that matter: the explanation always saying "relation" (the defect) and never saying it (the same defect pointing the other way). The test carries a mixed-reason leg, because a sentence that picks one of two true reasons is the failure a single-reason fixture cannot see. Also corrected: three comments claiming `itemCopyUnfillable` is relation-only or that the detailed render consults it. Both stopped being true last commit. Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8 * fix: one row can carry both faults, and four docs said this was simpler than it is (IDEA-2899) Review round 4. Four findings, no P1s, and the first is the one worth the round. **A `continue` between the two counts.** `itemCopyUnfillableWhy` counted a row as an unavailable relation target and then skipped the empty-key check, so ONE row carrying both faults reported only the first. My mixed-case test used TWO rows with one fault each — a different input, and the only one it exercised. Two rows with one fault each and one row with two are not the same fixture, and I built the weaker one while writing a commit message about fixtures that cannot discriminate. **The dialog could still print `--field =value`.** `cliFillableField` excluded unavailable relation targets and not empty keys, so a required `json` field the destination reported with no key was type-shaped, blocked, and still offered a command the CLI's own parser rejects. The CLI has refused those since Codex round 6; the web side had never learned it. Same defect, other surface — which is the third time this unit has fixed one door and not its sibling. **Cardinality.** "no --field can supply it" for several fields, and "reported them with an empty key" for one. Both sites now agree with their counts, and the empty-key phrase is neutral on number so it reads correctly after either. **Four documents claimed every needs_value row is resolvable with an override** — the CLI renderer's docblock, the server's `NeedsValue` field, the CLI mirror type, and the dialog's collectability comment. That was true when each was written and this unit falsified all four; a reader following any of them would conclude the CLI had simply forgotten to print a flag. Two mutants on the fixes, both killed: the `continue` restored, and the dialog's empty-key exclusion removed. Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8 * refactor(cli): one tally, because the rounds said the branching was the problem (IDEA-2899) Four review rounds returned 2, 2, 2 and 4 findings. The counts looked like slow convergence; the DISTRIBUTION was the finding. Every defect after round 1 lived in this one layer — how the CLI and the dialog say "here is how to supply it" — while the server half that computes availability stayed clean throughout. The layer had accreted exactly the way IDEA-2898's cold path did: a count, then a second count for the other reason, then a phrase function, then a `continue` between two counters that made a dual-fault row report half of itself. Round 4 fixed something round 3 introduced to fix something round 2 introduced. That is not a run of bad luck, it is a shape. So this round removes branches instead of adding a seventh guard. `itemCopyTally` walks the rows once and returns what every caller needs; `AllUnfillable()` is the condition both one-sentence sites test, and `Why()` is the phrase both interpolate. Three helpers become one type. There is no second definition of "unfillable" to drift from the first, and no sentence describing a subset of what a predicate counts, because the sentence and the count come from the same walk. `Unfillable` is deliberately NOT `UnavailableTarget + EmptyKey`: one row can carry both, and double-counting makes `Unfillable == Total` false for a set that is entirely unfillable — the comparison every caller makes. A mutant does the addition and dies. Five mutants, all killed. The last needed a new test rather than a new fixture: `AllUnfillable`'s `Total > 0` guard is unreachable from both current callers, so a mutant removing it survived every command-level test. Keeping an unreachable guard and calling it defence is how a promise becomes a lie, so the tally is now unit-tested directly — an empty set is not "entirely unfillable", and a future caller outside the `len() > 0` gate would otherwise be told silently that nothing can be supplied. Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8
516 lines
22 KiB
Go
516 lines
22 KiB
Go
package cli
|
|
|
|
import (
|
|
"bytes"
|
|
"encoding/json"
|
|
"errors"
|
|
"fmt"
|
|
"io"
|
|
"net/http"
|
|
"net/url"
|
|
"time"
|
|
|
|
"github.com/PerpetualSoftware/pad/internal/models"
|
|
)
|
|
|
|
// Cross-workspace item copy — the CLI client half of PLAN-2357 / TASK-2366.
|
|
//
|
|
// Two endpoints, one request shape (deliberately — see the server's
|
|
// handlers_items_copy_preflight.go header):
|
|
//
|
|
// POST /workspaces/{ws}/items/{ref}/copy/preflight → ItemCopyPreflight
|
|
// POST /workspaces/{ws}/items/{ref}/copy → ItemCopyResult
|
|
//
|
|
// The response types below MIRROR internal/server's. To be clear about why,
|
|
// because the obvious guess is wrong: this is NOT an import cycle. Nothing
|
|
// in internal/server imports internal/cli, and this package could import
|
|
// server tomorrow — the mirror test below does exactly that.
|
|
//
|
|
// It is a deliberate layering choice, the same one internal/cli/bootstrap.go
|
|
// already records ("otherwise has no dependency on internal/server"): this
|
|
// package is the HTTP client, and a client that reaches into the server's
|
|
// package for its wire types stops being separable from it and starts
|
|
// linking the store, the migrations and the router into anything that wants
|
|
// to talk to a Pad API. Mirroring is also what cli.DeletedWorkspace and the
|
|
// bootstrap constants already do, so this follows the house pattern rather
|
|
// than inventing one.
|
|
//
|
|
// The cost of mirroring is drift, and that is paid for:
|
|
// TestItemCopyMirrorsMatchServerShapes lives in the external cli_test
|
|
// package (which CAN import server) and walks both shapes, so a server-side
|
|
// rename is a red build rather than a field that silently stops rendering.
|
|
//
|
|
// ── DR-13: THE MUTATING COPY IS NEVER RETRIED ────────────────────────────
|
|
//
|
|
// v1 has no idempotency key. A retry after a request that already committed
|
|
// creates a DUPLICATE item, and a caller who lost the response cannot tell
|
|
// which happened. CopyItem therefore:
|
|
//
|
|
// 1. runs on a DEDICATED *http.Client AND a dedicated transport
|
|
// (copyHTTPClient / copyTransport), so a retrying http.Client OR a
|
|
// retrying RoundTripper installed on the shared client cannot reach it.
|
|
// The transport half matters most: retry in Go is almost always a
|
|
// RoundTripper wrapper, which a merely-dedicated *http.Client would
|
|
// inherit;
|
|
// 2. sends a body net/http itself cannot replay — the reader is wrapped so
|
|
// Request.GetBody stays nil, which disables the transport's own
|
|
// "nothing was written, try the next connection" retry;
|
|
// 3. refuses redirects rather than re-issuing the POST at a new location;
|
|
// 4. reports an unrecoverable-outcome failure as ErrCopyOutcomeUnknown so
|
|
// the command layer can tell the user to CHECK the destination instead
|
|
// of re-running.
|
|
//
|
|
// TestCopyItem_* in client_items_copy_test.go guard all four. Do not route
|
|
// CopyItem through the shared post()/httpClient helpers.
|
|
|
|
// copyRequestTimeout bounds the mutating copy. It is deliberately longer
|
|
// than the shared client's 10s: a copy that clones many attachment rows and
|
|
// fans out activity/webhook writes can legitimately outlast a normal API
|
|
// call, and a client-side timeout is exactly the ambiguous outcome DR-13
|
|
// wants to make rare. It is still bounded — a hung connection must not
|
|
// wedge the CLI forever.
|
|
const copyRequestTimeout = 60 * time.Second
|
|
|
|
// ErrCopyOutcomeUnknown marks a mutating-copy failure where the copy may or
|
|
// may not have committed: any transport-level failure (timeout, reset, EOF)
|
|
// and the server's own 500 `copy_failed`. Test with CopyOutcomeUnknown.
|
|
var ErrCopyOutcomeUnknown = errors.New("the copy's outcome is unknown")
|
|
|
|
// ErrCopyCommitted marks a failure that happened AFTER the server confirmed
|
|
// the copy: a 2xx whose body could not be read, or could not be decoded.
|
|
// The copy HAPPENED; only the report is missing.
|
|
//
|
|
// It exists so the command layer can keep these off the non-zero exit path.
|
|
// A script that sees a failing exit code from `pad item copy` will
|
|
// reasonably conclude the copy did not happen, and the obvious recovery —
|
|
// running it again — is precisely the DR-13 duplicate. Test with
|
|
// CopyCommitted.
|
|
var ErrCopyCommitted = errors.New("the copy committed but its result could not be read")
|
|
|
|
// ItemCopyRequest is the wire body for BOTH endpoints.
|
|
type ItemCopyRequest struct {
|
|
// TargetWorkspace is the destination workspace slug (a UUID is also
|
|
// accepted server-side). Required.
|
|
TargetWorkspace string `json:"target_workspace"`
|
|
// TargetCollection is the destination collection slug. Required.
|
|
TargetCollection string `json:"target_collection"`
|
|
// FieldOverrides maps destination-schema field key → value. A key the
|
|
// destination schema does not declare is a 400; a null value UNSETS
|
|
// the key rather than persisting a literal null.
|
|
FieldOverrides map[string]any `json:"field_overrides,omitempty"`
|
|
// ArchiveSource is the MOVE path: copy, then archive the source.
|
|
ArchiveSource bool `json:"archive_source"`
|
|
}
|
|
|
|
// ItemCopyPreflightSource identifies the item being copied.
|
|
type ItemCopyPreflightSource struct {
|
|
WorkspaceSlug string `json:"workspace_slug"`
|
|
CollectionSlug string `json:"collection_slug"`
|
|
Ref string `json:"ref,omitempty"`
|
|
Slug string `json:"slug"`
|
|
Title string `json:"title"`
|
|
}
|
|
|
|
// ItemCopyPreflightDestination identifies where the copy would land.
|
|
type ItemCopyPreflightDestination struct {
|
|
WorkspaceSlug string `json:"workspace_slug"`
|
|
WorkspaceName string `json:"workspace_name"`
|
|
CollectionSlug string `json:"collection_slug"`
|
|
CollectionName string `json:"collection_name"`
|
|
}
|
|
|
|
// ItemCopyPreflightCarried is one field that survives to the destination.
|
|
type ItemCopyPreflightCarried struct {
|
|
Key string `json:"key"`
|
|
Label string `json:"label,omitempty"`
|
|
Type string `json:"type,omitempty"`
|
|
Value any `json:"value"`
|
|
// From is "migrated", "override" or "default".
|
|
From string `json:"from"`
|
|
}
|
|
|
|
// ItemCopyPreflightDropped is one value that will not be copied.
|
|
type ItemCopyPreflightDropped struct {
|
|
Key string `json:"key"`
|
|
Label string `json:"label,omitempty"`
|
|
// Kind is "field" or "assignment".
|
|
Kind string `json:"kind"`
|
|
// Reason is one of no_target_field / incompatible_type /
|
|
// undeclared_source_field / assignee_not_a_member /
|
|
// agent_role_not_portable.
|
|
Reason string `json:"reason"`
|
|
}
|
|
|
|
// ItemCopyPreflightNeedsValue is one destination field the copy cannot satisfy
|
|
// on its own.
|
|
//
|
|
// Usually the caller resolves it with an override. Not always: a row carrying
|
|
// `CollectionUnavailable` names a relation target that is deleted or unreadable
|
|
// by this caller, and a row with an EMPTY key cannot be named by `--field` — so
|
|
// the copy cannot proceed at all, and the CLI says so instead of advising a
|
|
// flag (IDEA-2899).
|
|
type ItemCopyPreflightNeedsValue struct {
|
|
Key string `json:"key"`
|
|
Label string `json:"label,omitempty"`
|
|
Type string `json:"type,omitempty"`
|
|
Options []string `json:"options,omitempty"`
|
|
// Collection is the target collection SLUG for a `relation` field, empty
|
|
// otherwise (TASK-2869). Mirrored here because
|
|
// TestItemCopyMirrorsMatchServerShapes requires this struct to match the
|
|
// server's field for field — a mirror that silently lags is a mirror that
|
|
// lies, and the CLI renders these rows.
|
|
Collection string `json:"collection,omitempty"`
|
|
// CollectionUnavailable is true when Collection names a target this caller
|
|
// cannot use — deleted, or not readable by them (IDEA-2899). Mirrored for
|
|
// the same reason Collection is, and the CLI uses it for the same decision
|
|
// the dialog does: a row whose target is unavailable must not appear in the
|
|
// `--field key=<value>` suggestion, because there is no value to supply.
|
|
//
|
|
// ABSENT means available OR a server that does not report; only an explicit
|
|
// true says the target is unusable.
|
|
CollectionUnavailable bool `json:"collection_unavailable,omitempty"`
|
|
Required bool `json:"required"`
|
|
// Reason is "missing_required" or "invalid_value".
|
|
Reason string `json:"reason"`
|
|
Message string `json:"message,omitempty"`
|
|
}
|
|
|
|
// ItemCopyPreflightFields is DR-15's bucketing. The three names are the
|
|
// contract; all three slices are always present server-side.
|
|
type ItemCopyPreflightFields struct {
|
|
Carried []ItemCopyPreflightCarried `json:"carried"`
|
|
Dropped []ItemCopyPreflightDropped `json:"dropped"`
|
|
NeedsValue []ItemCopyPreflightNeedsValue `json:"needs_value"`
|
|
}
|
|
|
|
// ItemCopyPreflightWarnings is DR-15's full warning set.
|
|
type ItemCopyPreflightWarnings struct {
|
|
ChildCount int `json:"child_count"`
|
|
ChildrenOrphaned bool `json:"children_orphaned"`
|
|
DroppedParent bool `json:"dropped_parent"`
|
|
OutgoingLinks map[string]int `json:"outgoing_links"`
|
|
IncomingLinks map[string]int `json:"incoming_links"`
|
|
DroppedAssignee bool `json:"dropped_assignee"`
|
|
DroppedAgentRole bool `json:"dropped_agent_role"`
|
|
AttachmentCount int `json:"attachment_count"`
|
|
AttachmentBytes int64 `json:"attachment_bytes"`
|
|
UnresolvableRefCount int `json:"unresolvable_ref_count"`
|
|
// RelationshipsPartial marks ChildCount, ChildrenOrphaned,
|
|
// DroppedParent, OutgoingLinks and IncomingLinks as a FLOOR rather
|
|
// than a total: at least one relationship hangs off an item this
|
|
// caller may not see and was not counted (TASK-2369). A bare bool by
|
|
// design — how many, of what type and where are exactly what the
|
|
// server's ACL filter withholds.
|
|
//
|
|
// ChildrenOrphaned is the exception when rendering: a plain copy
|
|
// archives nothing, so `false` is complete there however much is
|
|
// hidden. renderItemCopyPreflight qualifies that line only on a move.
|
|
RelationshipsPartial bool `json:"relationships_partial"`
|
|
}
|
|
|
|
// ItemCopyPreflight is the dry-run's 200 response.
|
|
type ItemCopyPreflight struct {
|
|
Source ItemCopyPreflightSource `json:"source"`
|
|
Destination ItemCopyPreflightDestination `json:"destination"`
|
|
ArchiveSource bool `json:"archive_source"`
|
|
// Valid means EXACTLY ONE THING: NeedsValue is empty. It is not a
|
|
// prediction that the copy will succeed.
|
|
Valid bool `json:"valid"`
|
|
Fields ItemCopyPreflightFields `json:"fields"`
|
|
Warnings ItemCopyPreflightWarnings `json:"warnings"`
|
|
}
|
|
|
|
// ItemCopyResultSource identifies what was copied, and whether it survived.
|
|
type ItemCopyResultSource struct {
|
|
WorkspaceSlug string `json:"workspace_slug"`
|
|
CollectionSlug string `json:"collection_slug"`
|
|
Ref string `json:"ref,omitempty"`
|
|
Slug string `json:"slug"`
|
|
Title string `json:"title"`
|
|
Archived bool `json:"archived"`
|
|
Seq int64 `json:"seq,omitempty"`
|
|
}
|
|
|
|
// ItemCopyResultDestination is where the copy landed.
|
|
type ItemCopyResultDestination struct {
|
|
WorkspaceSlug string `json:"workspace_slug"`
|
|
WorkspaceName string `json:"workspace_name"`
|
|
CollectionSlug string `json:"collection_slug"`
|
|
CollectionName string `json:"collection_name"`
|
|
Ref string `json:"ref,omitempty"`
|
|
Slug string `json:"slug"`
|
|
Seq int64 `json:"seq,omitempty"`
|
|
}
|
|
|
|
// ItemCopyResultWarnings is the after-the-fact counterpart to the
|
|
// preflight's warning block. Deliberately narrower — the relationship
|
|
// counters are preview-only.
|
|
type ItemCopyResultWarnings struct {
|
|
DroppedFields []string `json:"dropped_fields"`
|
|
DroppedAssignee bool `json:"dropped_assignee"`
|
|
DroppedAgentRole bool `json:"dropped_agent_role"`
|
|
AttachmentCount int `json:"attachment_count"`
|
|
AttachmentBytes int64 `json:"attachment_bytes"`
|
|
UnresolvableRefCount int `json:"unresolvable_ref_count"`
|
|
}
|
|
|
|
// ItemCopyResult is the mutating copy's 201 response.
|
|
type ItemCopyResult struct {
|
|
Source ItemCopyResultSource `json:"source"`
|
|
Destination ItemCopyResultDestination `json:"destination"`
|
|
ArchiveSource bool `json:"archive_source"`
|
|
Item *models.Item `json:"item"`
|
|
Warnings ItemCopyResultWarnings `json:"warnings"`
|
|
}
|
|
|
|
// CopyItemPreflight runs the DRY RUN. It mutates nothing in the copy's own
|
|
// domain, so it is safe to call repeatedly and safe to retry.
|
|
//
|
|
// Returns the decoded preview AND the server's raw response bytes, so
|
|
// `--format json` can hand a script the endpoint's own contract rather than
|
|
// a CLI-shaped re-encoding of it.
|
|
func (c *Client) CopyItemPreflight(wsSlug, itemRef string, req ItemCopyRequest) (*ItemCopyPreflight, json.RawMessage, error) {
|
|
raw, err := c.postCopyJSON(c.httpClient, itemCopyPath(wsSlug, itemRef)+"/preflight", req, false)
|
|
if err != nil {
|
|
return nil, nil, err
|
|
}
|
|
var out ItemCopyPreflight
|
|
if err := json.Unmarshal(raw, &out); err != nil {
|
|
return nil, raw, fmt.Errorf("decode copy preflight response: %w", err)
|
|
}
|
|
return &out, raw, nil
|
|
}
|
|
|
|
// CopyItem performs the MUTATING cross-workspace copy.
|
|
//
|
|
// NEVER RETRY THIS CALL (DR-13). See the file header for the four
|
|
// mechanisms that enforce it. On failure, check CopyOutcomeUnknown(err)
|
|
// before saying anything to the user about what happened.
|
|
func (c *Client) CopyItem(wsSlug, itemRef string, req ItemCopyRequest) (*ItemCopyResult, json.RawMessage, error) {
|
|
hc := c.copyHTTPClient()
|
|
// The transport is this call's own, so its idle connections have nobody
|
|
// to serve afterwards.
|
|
defer hc.CloseIdleConnections()
|
|
|
|
raw, err := c.postCopyJSON(hc, itemCopyPath(wsSlug, itemRef), req, true)
|
|
if err != nil {
|
|
return nil, nil, err
|
|
}
|
|
var out ItemCopyResult
|
|
if err := json.Unmarshal(raw, &out); err != nil {
|
|
// The server answered 2xx, so the copy COMMITTED — we simply
|
|
// cannot render it. Not ErrCopyOutcomeUnknown: the outcome is
|
|
// known and it is "succeeded".
|
|
return nil, raw, fmt.Errorf("%w: decoding the response: %v", ErrCopyCommitted, err)
|
|
}
|
|
return &out, raw, nil
|
|
}
|
|
|
|
// itemCopyPath builds the copy endpoint's path with each dynamic segment
|
|
// escaped exactly once.
|
|
//
|
|
// Most of this client concatenates slugs into paths bare, and for the usual
|
|
// kebab-case slug or `TASK-5` ref that is indistinguishable from this. It is
|
|
// not good enough HERE: `..`, `/`, `?` and `#` in a ref would silently
|
|
// re-route or truncate the request, and this is the one endpoint in the CLI
|
|
// where landing on a DIFFERENT URL than intended could mutate the wrong
|
|
// thing. Escaping is a no-op for every legitimate ref, so it costs nothing
|
|
// and removes the class.
|
|
func itemCopyPath(wsSlug, itemRef string) string {
|
|
return "/workspaces/" + url.PathEscape(wsSlug) + "/items/" + url.PathEscape(itemRef) + "/copy"
|
|
}
|
|
|
|
// CopyCommitted reports whether err is a post-commit reporting failure —
|
|
// the copy happened, only its result was lost. Callers must not present
|
|
// these as failures of the copy, and must not exit non-zero on them.
|
|
func CopyCommitted(err error) bool {
|
|
return err != nil && errors.Is(err, ErrCopyCommitted)
|
|
}
|
|
|
|
// CopyOutcomeUnknown reports whether err leaves it genuinely unknown
|
|
// whether the copy committed. True for the server's 500 `copy_failed` and
|
|
// for any transport-level failure on the mutating call; false for every
|
|
// 4xx, which is a refusal the server made BEFORE writing anything.
|
|
func CopyOutcomeUnknown(err error) bool {
|
|
if err == nil {
|
|
return false
|
|
}
|
|
if errors.Is(err, ErrCopyOutcomeUnknown) {
|
|
return true
|
|
}
|
|
var apiErr *APIError
|
|
if errors.As(err, &apiErr) {
|
|
return apiErr.Code == "copy_failed"
|
|
}
|
|
return false
|
|
}
|
|
|
|
// copyHTTPClient is the dedicated client for the mutating copy (DR-13
|
|
// mechanism 1): its own transport, its own timeout, and no redirect
|
|
// following.
|
|
//
|
|
// The caller must CloseIdleConnections when done — the transport is not
|
|
// shared, so its pool would otherwise outlive the call.
|
|
func (c *Client) copyHTTPClient() *http.Client {
|
|
return &http.Client{
|
|
Transport: c.copyTransport(),
|
|
Timeout: copyRequestTimeout,
|
|
// DR-13 mechanism 3. A 307/308 would otherwise re-send the POST
|
|
// body at a new URL — a retry by another name.
|
|
CheckRedirect: func(*http.Request, []*http.Request) error {
|
|
return http.ErrUseLastResponse
|
|
},
|
|
}
|
|
}
|
|
|
|
// copyTransport picks the RoundTripper the mutating copy runs on.
|
|
//
|
|
// A dedicated *http.Client is NOT enough on its own: retry behaviour in Go
|
|
// is usually implemented as a RoundTripper wrapper, and a wrapper installed
|
|
// on the shared client's Transport would be inherited by any client that
|
|
// reuses it. So the choice is made here, by type:
|
|
//
|
|
// - a plain *http.Transport is CLONED. Its only retry is the narrow
|
|
// nothing-written replay that mechanism 2 already disables, and cloning
|
|
// preserves whatever proxy, TLS and dialer configuration it carries.
|
|
// The clone brings its own connection pool, which costs one extra
|
|
// handshake per copy — an acceptable price, and the reason CopyItem
|
|
// closes idle connections afterwards.
|
|
//
|
|
// - anything else is a WRAPPER of unknown behaviour, which is exactly the
|
|
// shape a retrying RoundTripper takes. It is not used.
|
|
//
|
|
// The second branch has a real cost: a future auth- or tracing-wrapping
|
|
// RoundTripper would be bypassed here too, and this is the one call in the
|
|
// CLI where that could look like a mysterious connection failure. That is
|
|
// deliberate. DR-13's hazard is a duplicated item nobody can detect after
|
|
// the fact; a copy that visibly fails to connect is the better failure, and
|
|
// there is no way to tell the two kinds of wrapper apart from the outside.
|
|
// If a wrapper ever becomes load-bearing for this client, the fix is to
|
|
// give Client an explicit non-retrying transport field — not to start
|
|
// trusting c.httpClient.Transport here.
|
|
func (c *Client) copyTransport() http.RoundTripper {
|
|
base := c.httpClient.Transport
|
|
if base == nil {
|
|
base = http.DefaultTransport
|
|
}
|
|
if t, ok := base.(*http.Transport); ok {
|
|
return t.Clone()
|
|
}
|
|
if def, ok := http.DefaultTransport.(*http.Transport); ok {
|
|
return def.Clone()
|
|
}
|
|
return &http.Transport{}
|
|
}
|
|
|
|
// postCopyJSON posts body to path and returns the response's raw JSON.
|
|
//
|
|
// mutating selects the DR-13 posture: a mutating call gets a body net/http
|
|
// cannot replay and reports transport failures as ErrCopyOutcomeUnknown.
|
|
func (c *Client) postCopyJSON(hc *http.Client, path string, body any, mutating bool) (json.RawMessage, error) {
|
|
data, err := json.Marshal(body)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
req, err := c.newCopyRequest(path, data, mutating)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
resp, err := hc.Do(req)
|
|
if err != nil {
|
|
if mutating {
|
|
// DR-13 mechanism 4: no response reached us, so the copy may
|
|
// have committed anyway.
|
|
return nil, fmt.Errorf("%w: request failed: %v", ErrCopyOutcomeUnknown, err)
|
|
}
|
|
return nil, fmt.Errorf("request failed: %w", err)
|
|
}
|
|
defer resp.Body.Close()
|
|
|
|
raw, readErr := io.ReadAll(resp.Body)
|
|
if resp.StatusCode >= 400 {
|
|
return nil, parseErrorBody(resp.StatusCode, raw)
|
|
}
|
|
if resp.StatusCode >= 300 {
|
|
// Only reachable on the mutating path, where CheckRedirect hands
|
|
// the 3xx back instead of following it.
|
|
return nil, fmt.Errorf("unexpected redirect (%d) to %q; refusing to re-send the request", resp.StatusCode, resp.Header.Get("Location"))
|
|
}
|
|
if readErr != nil {
|
|
if mutating {
|
|
// A 2xx header already arrived, so the server committed. Only
|
|
// the body was lost — a reporting failure, not a copy failure.
|
|
return nil, fmt.Errorf("%w: reading the response body: %v", ErrCopyCommitted, readErr)
|
|
}
|
|
return nil, fmt.Errorf("read response: %w", readErr)
|
|
}
|
|
return json.RawMessage(raw), nil
|
|
}
|
|
|
|
// newCopyRequest builds the POST for either copy endpoint.
|
|
//
|
|
// mutating is DR-13 mechanism 2. net/http will REPLAY a request whose
|
|
// connection died before any byte was written — but only when it can rebuild
|
|
// the body, which means only when Request.GetBody is set. http.NewRequest
|
|
// sets GetBody automatically for the readers it recognises, *bytes.Reader
|
|
// among them.
|
|
//
|
|
// That replay is narrow and usually harmless. It is still not acceptable
|
|
// here: "nothing was written" is the TRANSPORT's belief about one connection
|
|
// attempt, and this command's contract to the user is that a copy leaves the
|
|
// process at most once. Hiding the reader behind an opaque type leaves
|
|
// GetBody nil, so the decision belongs to us rather than to a heuristic.
|
|
//
|
|
// The preflight is left replayable — it is read-only, and a lost connection
|
|
// there costs nothing.
|
|
//
|
|
// Extracted from postCopyJSON so TestCopyItem_MutatingRequestIsNotReplayable
|
|
// can assert the property directly. Asserting it end-to-end is not possible:
|
|
// provoking the transport's nothing-written path requires winning a race
|
|
// against an idle-connection close, so a network-level test would pass for
|
|
// the wrong reason.
|
|
func (c *Client) newCopyRequest(path string, data []byte, mutating bool) (*http.Request, error) {
|
|
var reader io.Reader = bytes.NewReader(data)
|
|
if mutating {
|
|
reader = &opaqueReader{r: reader}
|
|
}
|
|
req, err := c.newRequest(http.MethodPost, path, reader)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
// opaqueReader defeats net/http's length sniffing too, so restore the
|
|
// Content-Length the server would otherwise not see.
|
|
req.ContentLength = int64(len(data))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
return req, nil
|
|
}
|
|
|
|
// opaqueReader hides a reader's concrete type from http.NewRequest so that
|
|
// Request.GetBody stays nil. See newCopyRequest.
|
|
type opaqueReader struct{ r io.Reader }
|
|
|
|
func (o *opaqueReader) Read(p []byte) (int, error) { return o.r.Read(p) }
|
|
|
|
// PrintRawJSON writes a server response verbatim except for indentation.
|
|
//
|
|
// json.Indent is a LEXICAL transform: it does not decode, so key order,
|
|
// numeric literals (int64 byte counts and seq values especially) and every
|
|
// field the CLI does not model survive byte-for-byte. Re-encoding through a
|
|
// Go value would silently drop unknown fields and could round large
|
|
// integers through float64 — which is exactly what "the endpoint's response
|
|
// unchanged" is there to prevent.
|
|
func PrintRawJSON(w io.Writer, raw json.RawMessage) error {
|
|
var buf bytes.Buffer
|
|
if err := json.Indent(&buf, raw, "", " "); err != nil {
|
|
// Not valid JSON to indent — emit the bytes exactly as received
|
|
// rather than inventing a shape.
|
|
buf.Reset()
|
|
buf.Write(raw)
|
|
}
|
|
buf.WriteByte('\n')
|
|
_, err := w.Write(buf.Bytes())
|
|
return err
|
|
}
|