feat(server): refuse to mark item terminal while it has open children (IDEA-1494) (#571)

* feat(server): refuse to mark item terminal while it has open children (IDEA-1494)

Server-side guard inside handleUpdateItem that rejects a non-terminal →
terminal done-field transition when the item still has at least one
non-terminal child. Returns HTTP 409 with code=open_children plus a
structured details payload listing each blocking child's
{ref, title, status, collection_slug} so MCP-driven agents can
self-recover (ship the listed children, then retry) and the CLI can
render the same list verbatim.

Escape hatch: `--force` on `pad item update` / `pad item bulk-update`
and `force: true` on the MCP pad_item.action: update / bulk-update
inputs both forward into the same ItemUpdate.Force transport field
the handler consumes before any store mutation.

Trigger conditions are tight: the PATCH must change the done-field key
(resolved via TerminalValuesForDoneField against the parent's schema +
settings) AND the new value must be terminal AND the current value
must NOT already be terminal. Terminal → terminal and no-op terminal
transitions bypass the guard; only entering the terminal set is gated.
Per-child evaluation uses the child's own collection schema so
hierarchical workspaces with custom typed collections work without
extra plumbing.

Tests cover: rejection with one open child (with mutation-safety
assertion on the parent), no children, all-terminal children, --force
override, no-op terminal → terminal, terminal → terminal,
non-terminal → non-terminal, custom collection terminal_options
honored, and a parent task (not a plan) — IDEA-1494 optional extra #3.
MCP coverage asserts --force round-trips through both ExecDispatcher
and HTTPHandlerDispatcher and is omitted when force=false.

* fix(server): open-children guard round 2 — visibility, MCP pass-through, TOCTOU (IDEA-1494)

Three Codex round-1 issues, each fixed with the recommended shape:

P1 — visibility leak. The 409 response previously listed every blocking
child by ref/title/status, including children in collections the caller
couldn't see. The INVARIANT still evaluates against ALL children (it's a
data-integrity gate — a restricted user must not be able to close a
parent whose blockers they can't see), but the response payload now
filters to caller-visible children only. Hidden blockers surface as a
new `details.hidden_blocker_count` field plus an alternate human message
when every blocker is hidden ("blocked by N open children you don't
have access to"). Mirrors the visibility helpers (`visibleCollectionIDs`
+ `isItemVisibleToGuest`) used by the per-parent progress endpoint so
the two paths can't drift.

P2 — MCP code/details pass-through. The HTTP classifier was collapsing
409 into the generic `conflict` code and dropping `details`; the stdio
classifier was matching the human "cannot " message against the
validation regex and surfacing `validation_failed`. Both now surface
`open_children` with the structured details intact:
  - HTTP: classifyHTTPStatusKind's 409 branch extracts the upstream
    code; any non-empty, non-"conflict" code is passed through with
    its `details` RawMessage. Generalizes beyond open_children — any
    future structured 409 from a handler gets the same treatment.
  - Stdio: the CLI writes a `pad-error: {json}\n` marker line on
    stderr before the human-readable block (single source of truth for
    both views), and classifyExecError detects the marker and lifts
    the envelope verbatim. Marker is duplicated as a const between
    internal/cli and internal/mcp to avoid pulling the cli package
    into the classifier just for one string.
A new ErrOpenChildren error code constant + `Details json.RawMessage`
field on ErrorPayload back the wire shape.

P2 — TOCTOU. The guard previously ran in the handler before the store
transaction began; a concurrent child insert / child status flip could
slip between the children-list read and the parent's UPDATE. Fix:
  - New `Store.UpdateItemWithPreCheck(id, input, precheck)` runs the
    caller's invariant check inside the same tx, after acquiring the
    workspace seq lock AND a new parent-children advisory lock keyed
    on the parent ID. UpdateItem is now a thin wrapper passing nil.
  - Every UpdateItem unconditionally acquires the parent-children
    advisory lock for its own parent (if any) AND for itself-as-parent,
    in a fixed order (parent first) so two updaters touching the same
    parent always grab that key before the more-specific one — no
    AB/BA deadlock.
  - New `GetChildItemsTx` reads via the caller's tx; on Postgres the
    advisory lock provides the snapshot guarantee (DISTINCT precludes
    `FOR UPDATE`), on SQLite the global BEGIN IMMEDIATE write lock
    serializes all writers.
  - Handler now passes a precheck closure into UpdateItemWithPreCheck
    at all three call sites (collab-snapshot path, applier-direct-write
    path, main path). The guard's openChildrenGuardError sentinel is
    unwrapped after each call so the 409 surfaces cleanly.

Tests:
  - TestOpenChildrenGuard_VisibilitySanitizesPayload — restricted
    editor sees parent + visible child, hidden child contributes to
    hidden_blocker_count, no leak of ref/title/slug.
  - TestOpenChildrenGuard_AllBlockersHiddenSurfaceGenericMessage —
    open_children=[], hidden_blocker_count>0, message mentions "you
    don't have access to."
  - TestOpenChildrenGuard_TOCTOURace — 8 iterations of a child-flip
    racing a parent-terminal update; asserts the forbidden outcome
    (parent=completed AND child=open) never occurs.
  - TestClassifyHTTPStatus_OpenChildrenPreservesCodeAndDetails +
    inverse generic-409 test.
  - TestClassifyExecError_OpenChildrenMarkerLiftsStructuredPayload +
    no-marker-falls-through inverse.

* fix(server): open-children guard round 3 — 7 Codex findings closed (IDEA-1494)

P1 — visibility fail-closed. The handler was swallowing
visibleCollectionIDs errors, leaving visIDs==nil which the guard
treats as unrestricted, leaking hidden-child metadata. Now surfaces
the error as 500 BEFORE installing the precheck. Test:
TestOpenChildrenGuard_VisibilityLookupErrorFailsClosed closes the
store DB and asserts no 409+children leak.

P1 — link mutations acquire the advisory lock. SetParentLink,
ClearParentLink, CreateItemLink (when link_type ∈ childLinkTypes via
new isChildLinkType helper), DeleteItemLink (same condition), and
RestoreItem now take `pad:parent-children:<id>` in canonical sorted
order via new AcquireParentChildrenLocks helper. SetParentLink locks
BOTH old and new parents (re-parenting case). Race test
TestOpenChildrenGuard_LinkMutationRace asserts the forbidden
"link-committed-before-parent-flip AND parent flip succeeded" never
occurs by comparing link.created_at to parent.updated_at. Documented
semantics: status-wins + link-after-commit is legal under the
invariant "no open children EXIST AT THE MOMENT of transition" —
the post-condition variant ("no open child may EVER attach to a
terminal parent") is intentionally deferred.

P1 — MoveItem bypass closed. New MoveItemWithPreCheck mirrors
UpdateItemWithPreCheck — acquires workspace seq lock + parent-children
locks, re-reads in tx, runs caller precheck. handleMoveItem builds
the same guard closure using the DESTINATION schema for done-field
resolution (conservative — honors the schema the item moves INTO).
CLI gains `pad item move --force`, client gains MoveItemWithForce
that appends `?force=true` to the move endpoint. MCP catalog +
mapItemMove forward `force` through the route mapper. Tests:
TestOpenChildrenGuard_MoveItem_RejectsTerminalWithOpenChildren and
…_ForceOverrides.

P2 — pre-tx field-read TOCTOU. UpdateItemWithPreCheck and
MoveItemWithPreCheck now re-read the item via new getItemTx INSIDE
the tx (after locks) and pass that fresh snapshot to the precheck
closure; the precheck classifies the transition against the in-tx
view, not the handler-side pre-tx capture. Handler precheck closure
swaps `currentFieldsJS` from the in-tx snapshot. Test:
TestOpenChildrenGuard_PrecheckReadsInTxSnapshot stages a between-load
status mutation and asserts the precheck observes the post-mutation
fields.

P2 — bulk-update carries structured errors. cmd/pad/main.go's
updateFailure struct extended with Code + Details
(json.RawMessage). When client.UpdateItem returns *cli.APIError, the
row preserves the structured envelope. Human-text output also
renders the open-children list inline. Chose JSON-envelope route
over per-row stderr markers because bulk-update already produces a
structured envelope and ExecDispatcher returns stdout verbatim on
exit-0 — no classifier change needed. Test:
TestBulkUpdateStructuredFailuresCarryOpenChildrenDetails confirms
the wire shape the CLI lifts.

P3 — marker hardening. Marker bumped to versioned form
`pad-structured-error/v1:` (was `pad-error:`). cli.StructuredErrorMarker
+ mcp.structuredErrorMarker kept in lockstep with cross-references.
mcp.allowedStructuredErrorCodes whitelists known codes (currently
just open_children); unknown codes fall back to regex classification.
Marker must start the line after whitespace trim (embedded markers
ignored). Last-marker-wins to defeat pre-emption attacks. Tests:
TestClassifyExecError_{UnknownStructuredCode,OldMarkerVersion,
MarkerEmbeddedMidLine,LastMarker}.

P3 — soft-deleted collection schemas honored. New GetCollectionAnyState
mirrors childrenDoneFiltersForParent's inclusion rule; guard uses it
so a child still attached to a soft-deleted collection is evaluated
against ITS schema (custom terminal_options) instead of the default-
status fallback (which would mis-classify and false-block). Test:
TestOpenChildrenGuard_SoftDeletedCollectionSchemaHonored seeds a
custom collection, soft-deletes it while a child remains, and
asserts the terminal status is correctly recognized.

Comprehensive store-mutation audit results recorded in the PR
description (every method touching items.fields / items.collection_id
or item_links).

* fix(server): open-children guard round 4 — multi-parent locks, enum parity, PATCH atomicity (IDEA-1494)

Four Codex round-3 (blast-radius lens) findings, each fixed with the
recommended shape.

P1 — multi-parent lock set. acquireParentChildrenLocksForUpdate and
RestoreItem previously used `LIMIT 1` against item_links, so a child
with BOTH a `parent` link to P1 AND an `implements` link to P2 only
locked one of them. The other parent's open-children precheck could
race against the child's status flip and miss it.

Fix: new listParentChildLockKeys helper runs the same query
GetChildItems' inclusion rule uses (childLinkTypes), returns ALL
distinct parent target_ids, and feeds them into the canonical
multi-lock helper. Both UpdateItemWithPreCheck and RestoreItem now
acquire locks on {self} ∪ {all-parents-via-childLinkTypes}. Test:
TestOpenChildrenGuard_MultiParentChildLocksAll races a child status
flip against terminal-updates on both parents simultaneously.

P2 — lock-order asymmetry. The pre-fix codebase had multiple lock-
acquisition shapes: parent-then-self in acquireParentChildrenLocksForUpdate,
single-key in RestoreItem / CreateItemLink / DeleteItemLink /
ClearParentLink, and a sorted multi-key in SetParentLink. Two
concurrent callers using different ad-hoc orderings could AB/BA
deadlock.

Fix: removed the per-call-site AcquireParentChildrenLock helper
entirely. Every site now goes through AcquireParentChildrenLocks
(the canonical sorted multi-lock helper) — including ones that need
only one ID (the variadic call still sorts a one-element slice).
The helper's doc comment explicitly states the contract: "Ad-hoc
single-key acquisition outside this helper is FORBIDDEN — two call
sites taking distinct keys in different orders WILL deadlock."
Test: TestOpenChildrenGuard_NoDeadlockUnderReverseOrderConcurrency
runs reverse-order re-parents with a 5-second timeout; assertion
fails on hang.

P2 — HTTP/stdio code-surface parity. Round 2's HTTP pass-through
("any non-conflict upstream code") silently widened the ErrorCode
enum beyond stdio's allow-list (`open_children` only). Agents saw
different code surfaces depending on which dispatcher delivered
the response.

Fix: HTTP 409 branch in classifyHTTPStatusKind now consults the
same allowedStructuredErrorCodes whitelist stdio does. Codes
outside the set collapse to ErrConflict (no details), matching
what stdio does for an unknown-code structured marker. Doc on
allowedStructuredErrorCodes updated to make the dual-consumer
contract explicit: "Adding a new structured code is a TWO-WAY
change." Tests:
TestClassifyHTTPStatus_UnknownConflictCodeFallsBackToErrConflict
and TestStructuredErrorCodeParityAcrossTransports.

P3 — PATCH atomicity. A combined PATCH with `parent` + `status=terminal`
on an item with open children used to commit the parent-link change
INLINE (before the guard ran) and then reject the field write.
Caller saw 409 but the parent had already moved.

Fix: parent-link mutation is now DEFERRED — captured into outer-
scope vars during fields validation, executed AFTER
UpdateItemWithPreCheck succeeds. A guard rejection returns before
the link write block, so on rejection the link is untouched.
Documented choice: "reorder, don't tx-wrap" — wrapping SetParentLink
into the same store tx would require threading a *sql.Tx through
the SetParentLink API (which is also called from the
handler_item_links path); reordering is the smaller surgery and
gives the correct outcome on the failure direction. A residual
window remains in the OTHER direction (field write commits, link
write fails) — not made worse by the reorder, and called out
inline for a future tx-wrap pass.

Test: TestOpenChildrenGuard_PatchAtomicRejectionPreservesParentLink
sets up target → oldParent → openChild, sends PATCH {parent=newParent,
status=completed}, asserts 409 AND target.parent_link still points
at oldParent.

* fix(server): open-children guard — emit details.open_children as [] not null on hidden-only rejection (IDEA-1494)

cmd/pad/main.go

@@ -3392,6 +3392,7 @@ func updateCmd() *cobra.Command {
 		tags       string
 		fieldFlags []string
 		comment    string
+		force      bool
 	)
 
 	cmd := &cobra.Command{
@@ -3442,6 +3443,9 @@ Examples:
 			if comment != "" {
 				input.Comment = &comment
 			}
+			if force {
+				input.Force = true
+			}
 
 			// Merge field changes with existing fields
 			parentRef := parentFlag
@@ -3529,6 +3533,19 @@ Examples:
 
 			updated, err := client.UpdateItem(ws, slug, input)
 			if err != nil {
+				// IDEA-1494: render the open-children list when the
+				// server rejected the transition because the item has
+				// non-terminal children. The detailed list comes from
+				// the same structured payload MCP clients consume so
+				// the human and machine views agree.
+				if apiErr, ok := err.(*cli.APIError); ok {
+					if oc := apiErr.AsOpenChildren(); oc != nil {
+						cli.WriteOpenChildrenError(os.Stderr, apiErr, oc)
+						// Return a bare error so cobra exits non-zero
+						// without re-printing the (now-rendered) details.
+						return fmt.Errorf("update rejected: open children present")
+					}
+				}
 				return err
 			}
 
@@ -3561,6 +3578,7 @@ Examples:
 	cmd.Flags().StringVar(&tags, "tags", "", "update tags (JSON array)")
 	cmd.Flags().StringArrayVarP(&fieldFlags, "field", "f", nil, "set arbitrary field (repeatable): --field key=value")
 	cmd.Flags().StringVar(&comment, "comment", "", "attach a comment explaining this update (e.g. why status changed)")
+	cmd.Flags().BoolVar(&force, "force", false, "override the open-children guard (allow marking the item terminal even if children are non-terminal)")
 
 	return cmd
 }
@@ -3622,6 +3640,7 @@ func deleteCmd() *cobra.Command {
 // --- move ---
 
 func moveCmd() *cobra.Command {
+	var force bool
 	cmd := &cobra.Command{
 		Use:   "move <ref> <target-collection>",
 		Short: "Move an item to a different collection",
@@ -3659,8 +3678,18 @@ Examples:
 				input["field_overrides"] = overrides
 			}
 
-			moved, err := client.MoveItem(ws, args[0], input)
+			moved, err := client.MoveItemWithForce(ws, args[0], input, force)
 			if err != nil {
+				// IDEA-1494 R3 P1: render the open-children rejection
+				// the same way the regular update path does, so
+				// `pad item move ... --field status=completed` against
+				// a plan with open children fails informatively.
+				if apiErr, ok := err.(*cli.APIError); ok {
+					if oc := apiErr.AsOpenChildren(); oc != nil {
+						cli.WriteOpenChildrenError(os.Stderr, apiErr, oc)
+						return fmt.Errorf("move rejected: open children present")
+					}
+				}
 				return err
 			}
 
@@ -3669,6 +3698,7 @@ Examples:
 		},
 	}
 	cmd.Flags().StringArray("field", nil, "set field values in target collection (key=value)")
+	cmd.Flags().BoolVar(&force, "force", false, "override the open-children guard when the move would write a terminal done-field value")
 	return cmd
 }
 
@@ -6478,6 +6508,7 @@ func bulkUpdateCmd() *cobra.Command {
 	var (
 		status   string
 		priority string
+		force    bool
 	)
 
 	cmd := &cobra.Command{
@@ -6511,9 +6542,17 @@ Examples:
 				Ref     string         `json:"ref"`
 				Applied map[string]any `json:"applied"`
 			}
+			// updateFailure carries the structured server error per row
+			// so MCP-driven agents see code + details (IDEA-1494 R3 P2).
+			// `Error` stays as the human-readable message for backward
+			// compatibility with non-MCP CLI consumers; `Code` and
+			// `Details` populate when the server returned a structured
+			// envelope (currently: only open_children rejections).
 			type updateFailure struct {
-				Ref   string `json:"ref"`
-				Error string `json:"error"`
+				Ref     string          `json:"ref"`
+				Error   string          `json:"error"`
+				Code    string          `json:"code,omitempty"`
+				Details json.RawMessage `json:"details,omitempty"`
 			}
 			updated := make([]updateResult, 0, len(args))
 			failed := make([]updateFailure, 0)
@@ -6553,14 +6592,42 @@ Examples:
 
 				input := models.ItemUpdate{
 					Fields: &fieldsStr,
+					Force:  force,
 				}
 
 				_, err = client.UpdateItem(ws, slug, input)
 				if err != nil {
+					// IDEA-1494 R3 P2: preserve the structured server
+					// error per row so the JSON envelope (and any
+					// MCP-driven agent reading it) sees code +
+					// details — not just a flattened string. The
+					// human text output continues to show the bare
+					// message.
+					row := updateFailure{Ref: slug, Error: err.Error()}
+					if apiErr, ok := err.(*cli.APIError); ok && apiErr.Code != "" {
+						row.Code = apiErr.Code
+						if len(apiErr.Details) > 0 {
+							row.Details = apiErr.Details
+						}
+					}
 					if formatFlag != "json" {
 						fmt.Printf("  %s %s — %s\n", red.Sprint("✗"), slug, err)
+						// For open_children rejections also render the
+						// blocking-child list inline so the human
+						// reader doesn't have to switch to --format
+						// json to see what blocked.
+						if apiErr, ok := err.(*cli.APIError); ok {
+							if oc := apiErr.AsOpenChildren(); oc != nil {
+								for _, c := range oc.OpenChildren {
+									fmt.Printf("      %s — %s (status=%s)\n", c.Ref, c.Title, c.Status)
+								}
+								if oc.HiddenBlockerCount > 0 {
+									fmt.Printf("      (+%d hidden blocker(s) you don't have access to)\n", oc.HiddenBlockerCount)
+								}
+							}
+						}
 					}
-					failed = append(failed, updateFailure{Ref: slug, Error: err.Error()})
+					failed = append(failed, row)
 					continue
 				}
 
@@ -6594,6 +6661,7 @@ Examples:
 
 	cmd.Flags().StringVar(&status, "status", "", "set status for all items")
 	cmd.Flags().StringVar(&priority, "priority", "", "set priority for all items")
+	cmd.Flags().BoolVar(&force, "force", false, "override the open-children guard (allow marking items terminal even if children are non-terminal)")
 
 	return cmd
 }

internal/cli/client.go

@@ -202,8 +202,21 @@ func (c *Client) ListStarredItems(wsSlug string, includeTerminal bool) ([]models
 }
 
 func (c *Client) MoveItem(wsSlug, itemSlug string, input map[string]any) (*models.Item, error) {
+	return c.MoveItemWithForce(wsSlug, itemSlug, input, false)
+}
+
+// MoveItemWithForce is the open-children-guard-aware variant of
+// MoveItem (IDEA-1494 R3 P1). When `force` is true, the URL gets a
+// `?force=true` query so the server-side move handler skips the guard
+// and still records the collection + fields change. Same escape-hatch
+// semantics as `pad item update --force`.
+func (c *Client) MoveItemWithForce(wsSlug, itemSlug string, input map[string]any, force bool) (*models.Item, error) {
 	var result models.Item
-	return &result, c.post("/workspaces/"+wsSlug+"/items/"+itemSlug+"/move", input, &result)
+	path := "/workspaces/" + wsSlug + "/items/" + itemSlug + "/move"
+	if force {
+		path += "?force=true"
+	}
+	return &result, c.post(path, input, &result)
 }
 
 // --- Links ---
@@ -705,14 +718,117 @@ func (c *Client) GetAuditLog(params models.AuditLogParams) ([]models.Activity, e
 // --- HTTP helpers ---
 
 type APIError struct {
-	Code    string `json:"code"`
-	Message string `json:"message"`
+	Code    string          `json:"code"`
+	Message string          `json:"message"`
+	Details json.RawMessage `json:"details,omitempty"`
 }
 
 func (e *APIError) Error() string {
 	return e.Message
 }
 
+// OpenChildEntry mirrors the server-side openChildEntry payload returned
+// inside APIError.Details when Code == "open_children" (IDEA-1494). The
+// CLI renders its human error list from these entries; MCP-driven agents
+// can introspect the same data to self-recover.
+type OpenChildEntry struct {
+	Ref            string `json:"ref"`
+	Title          string `json:"title"`
+	Status         string `json:"status"`
+	CollectionSlug string `json:"collection_slug"`
+}
+
+// OpenChildrenDetails is the parsed shape of APIError.Details when
+// Code == "open_children".
+type OpenChildrenDetails struct {
+	OpenChildren       []OpenChildEntry `json:"open_children"`
+	HiddenBlockerCount int              `json:"hidden_blocker_count"`
+	DoneField          string           `json:"done_field"`
+	AttemptedValue     string           `json:"attempted_value"`
+}
+
+// AsOpenChildren returns the parsed open-children details when this
+// APIError carries them, or nil otherwise. Returns nil for any error
+// other than "open_children" so callers can branch cleanly.
+func (e *APIError) AsOpenChildren() *OpenChildrenDetails {
+	if e == nil || e.Code != "open_children" || len(e.Details) == 0 {
+		return nil
+	}
+	var d OpenChildrenDetails
+	if err := json.Unmarshal(e.Details, &d); err != nil {
+		return nil
+	}
+	return &d
+}
+
+// StructuredErrorMarker is the versioned line prefix the CLI writes
+// to stderr when surfacing a structured error (currently: IDEA-1494's
+// open-children rejection). The JSON line that follows carries the
+// full server-style envelope (code / message / details) so a
+// downstream consumer — the stdio MCP dispatcher's classifyExecError
+// in particular — can detect the rejection and lift the structured
+// payload without parsing free-form human text.
+//
+// Versioned (Codex round-3 P3) so future evolutions of the wire shape
+// don't silently break older parsers — when the payload contract
+// changes incompatibly, bump to `pad-structured-error/v2:` and have
+// the classifier accept both during the transition. The version token
+// is parsed (not just matched as a literal prefix) so older v1-only
+// parsers cleanly ignore unknown versions.
+//
+// IMPORTANT: keep in lockstep with mcp.structuredErrorMarker / the
+// allow-list of structured codes in mcp.allowedStructuredErrorCodes.
+// A change here REQUIRES a corresponding change in
+// internal/mcp/errors.go.
+const StructuredErrorMarker = "pad-structured-error/v1: "
+
+// OpenChildrenErrorMarker is the pre-round-3 marker, retained as a
+// deprecated alias for any out-of-tree consumer that may have hard-
+// coded it. New code MUST use StructuredErrorMarker.
+//
+// Deprecated: use StructuredErrorMarker.
+const OpenChildrenErrorMarker = StructuredErrorMarker
+
+// WriteOpenChildrenError formats an open-children rejection to w in
+// the canonical two-track shape the project guarantees (IDEA-1494 R2):
+//
+//  1. A single `pad-error: {json}\n` line carrying the full structured
+//     payload — consumed by the MCP stdio classifier and anyone else
+//     wanting to introspect the rejection programmatically.
+//  2. Human-readable lines: the message, the per-child list (rendered
+//     from the SAME details struct the JSON line carries — single
+//     source of truth for both views), the hidden-count tag when
+//     applicable, and the `Pass --force to override` reminder.
+//
+// Order matters: machine line first so a consumer that reads stderr
+// line-by-line can dispatch on the first line without buffering all
+// of it. Callers should write nothing else between the marker line
+// and the human block.
+func WriteOpenChildrenError(w io.Writer, apiErr *APIError, oc *OpenChildrenDetails) {
+	envelope := map[string]any{
+		"error": map[string]any{
+			"code":    apiErr.Code,
+			"message": apiErr.Message,
+			"details": oc,
+		},
+	}
+	if data, err := json.Marshal(envelope); err == nil {
+		fmt.Fprintln(w, StructuredErrorMarker+string(data))
+	}
+	fmt.Fprintln(w, apiErr.Message)
+	for _, c := range oc.OpenChildren {
+		fmt.Fprintf(w, "  %s — %s (status=%s)\n", c.Ref, c.Title, c.Status)
+	}
+	if oc.HiddenBlockerCount > 0 {
+		noun := "child"
+		if oc.HiddenBlockerCount != 1 {
+			noun = "children"
+		}
+		fmt.Fprintf(w, "  (+%d hidden %s you don't have access to)\n", oc.HiddenBlockerCount, noun)
+	}
+	fmt.Fprintln(w, "Pass --force to override.")
+}
+
 // --- Attachments ---
 //
 // AttachmentUploadResult mirrors the JSON returned by

internal/mcp/catalog_item.go

@@ -145,6 +145,17 @@ var padItemSchemaParams = []ParamDef{
 	{Name: "reply_to", Type: "string", Description: "Parent comment ID for threading replies. Optional for: comment."},
 	{Name: "comment", Type: "string", Description: "Audit comment explaining the change. Optional for: update."},
 
+	// ── Guard override ── (IDEA-1494)
+	// update/bulk-update reject non-terminal → terminal done-field
+	// transitions while the item still has open (non-terminal)
+	// children, surfacing a 409 with code=open_children plus a
+	// machine-readable details.open_children array (one entry per
+	// blocking child: {ref, title, status, collection_slug}). Setting
+	// force=true skips the guard and still records the transition.
+	// The same flag exists on `pad item update --force` so the CLI
+	// and MCP escape hatches are identical.
+	{Name: "force", Type: "bool", Description: "Override the open-children guard. Optional for: update, bulk-update, move. When the server returns code=open_children, details.open_children lists the blocking child refs so an agent can ship them and retry — or set force=true if the children should be intentionally orphaned."},
+
 	// ── Notes / decisions ──
 	{Name: "summary", Type: "string", Description: "Short note headline. Required for: note."},
 	{Name: "details", Type: "string", Description: "Long-form note body. Optional for: note."},

internal/mcp/catalog_item_force_test.go

@@ -0,0 +1,89 @@
+package mcp
+
+// IDEA-1494 — MCP-level coverage for the `force` parameter on
+// pad_item.action: update / bulk-update. Confirms the catalog's force
+// param translates into the `--force` CLI flag on the dispatched
+// command so the open-children guard's escape hatch is identical on
+// stdio MCP (ExecDispatcher) and the legacy CLI surface.
+
+import (
+	"context"
+	"strings"
+	"testing"
+)
+
+func TestPadItemUpdate_ForcePassesThroughToCLI(t *testing.T) {
+	disp := &fakeDispatcher{}
+	env := ActionEnv{
+		Doc:        liveCmdhelpDoc(t),
+		Workspace:  NewWorkspaceState("docapp"),
+		Dispatcher: disp,
+	}
+
+	handler, ok := padItemTool.Actions["update"]
+	if !ok {
+		t.Fatal("pad_item.action: update is not registered")
+	}
+
+	if _, err := handler(context.Background(), map[string]any{
+		"ref":    "PLAN-5",
+		"status": "completed",
+		"force":  true,
+	}, env); err != nil {
+		t.Fatalf("dispatch error: %v", err)
+	}
+	if !equalStrings(disp.gotPath, []string{"item", "update"}) {
+		t.Fatalf("cmdPath = %v, want [item update]", disp.gotPath)
+	}
+	joined := strings.Join(disp.gotArgs, " ")
+	if !strings.Contains(joined, "--force") {
+		t.Errorf("cliArgs should contain '--force' when force=true; got %q", joined)
+	}
+	if !strings.Contains(joined, "--status completed") {
+		t.Errorf("cliArgs should preserve other flags; got %q", joined)
+	}
+}
+
+func TestPadItemUpdate_ForceFalse_Omitted(t *testing.T) {
+	disp := &fakeDispatcher{}
+	env := ActionEnv{
+		Doc:        liveCmdhelpDoc(t),
+		Workspace:  NewWorkspaceState("docapp"),
+		Dispatcher: disp,
+	}
+
+	handler := padItemTool.Actions["update"]
+	if _, err := handler(context.Background(), map[string]any{
+		"ref":    "PLAN-5",
+		"status": "completed",
+		"force":  false,
+	}, env); err != nil {
+		t.Fatalf("dispatch error: %v", err)
+	}
+	joined := strings.Join(disp.gotArgs, " ")
+	if strings.Contains(joined, "--force") {
+		t.Errorf("cliArgs should NOT contain '--force' when force=false; got %q", joined)
+	}
+}
+
+func TestPadItemBulkUpdate_ForcePassesThroughToCLI(t *testing.T) {
+	disp := &fakeDispatcher{}
+	env := ActionEnv{
+		Doc:        liveCmdhelpDoc(t),
+		Workspace:  NewWorkspaceState("docapp"),
+		Dispatcher: disp,
+	}
+
+	handler := padItemTool.Actions["bulk-update"]
+	if _, err := handler(context.Background(), map[string]any{
+		"refs":   []any{"TASK-1", "TASK-2"},
+		"status": "done",
+		"force":  true,
+	}, env); err != nil {
+		t.Fatalf("dispatch error: %v", err)
+	}
+	joined := strings.Join(disp.gotArgs, " ")
+	if !strings.Contains(joined, "--force") {
+		t.Errorf("cliArgs should contain '--force' for bulk-update force=true; got %q", joined)
+	}
+}

internal/mcp/catalog_readonly_test.go

@@ -463,11 +463,16 @@ func liveCmdhelpDoc(t *testing.T) *cmdhelp.Document {
 			"item update": {
 				Summary: "update item",
 				Args:    mkArgs("ref"),
-				Flags: mkFlags(
-					"workspace", "assign", "category", "comment", "content",
-					"field", "parent", "priority", "role", "status",
-					"tags", "title",
-				),
+				Flags: func() map[string]cmdhelp.Flag {
+					f := mkFlags(
+						"workspace", "assign", "category", "comment", "content",
+						"field", "parent", "priority", "role", "status",
+						"tags", "title",
+					)
+					// IDEA-1494: --force bypasses the open-children guard.
+					f["force"] = cmdhelp.Flag{Type: "bool"}
+					return f
+				}(),
 			},
 			"item delete": {
 				Summary: "delete item",
@@ -502,6 +507,10 @@ func liveCmdhelpDoc(t *testing.T) *cmdhelp.Document {
 				Flags: map[string]cmdhelp.Flag{
 					"workspace": {Type: "string"},
 					"field":     {Type: "[]string", Repeatable: true},
+					// IDEA-1494 R3 P1: move now honors the same
+					// open-children guard override the update path
+					// does.
+					"force": {Type: "bool"},
 				},
 			},
 			"item deps": {
@@ -539,7 +548,11 @@ func liveCmdhelpDoc(t *testing.T) *cmdhelp.Document {
 			"item bulk-update": {
 				Summary: "bulk update",
 				Args:    []cmdhelp.Arg{{Name: "ref", Required: true, Repeatable: true}},
-				Flags:   mkFlags("workspace", "priority", "status"),
+				Flags: func() map[string]cmdhelp.Flag {
+					f := mkFlags("workspace", "priority", "status")
+					f["force"] = cmdhelp.Flag{Type: "bool"}
+					return f
+				}(),
 			},
 			"item note": {
 				Summary: "add note",

internal/mcp/dispatch_http_advanced.go

@@ -349,6 +349,14 @@ func (d *HTTPHandlerDispatcher) dispatchItemUpdate(
 	if b, ok := input["pinned"].(bool); ok {
 		payload["pinned"] = b
 	}
+	// IDEA-1494: forward the open-children guard override. When set,
+	// the server-side handler skips the guard and still records the
+	// status transition. Same wire shape the CLI uses (`force: true`
+	// on the ItemUpdate body), so the HTTP dispatcher and ExecDispatcher
+	// paths share one contract.
+	if b, ok := input["force"].(bool); ok && b {
+		payload["force"] = true
+	}
 
 	// Field merging — the actual reason this command needs a custom
 	// dispatcher rather than a routeSpec entry. Match the CLI's

internal/mcp/dispatch_http_advanced_force_test.go

@@ -0,0 +1,90 @@
+package mcp
+
+// IDEA-1494 — confirm the HTTP dispatcher forwards the open-children
+// guard override (`force`) into the PATCH body. Matches the wire
+// contract handleUpdateItem reads (`force: true` at the top level of
+// the ItemUpdate JSON body).
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+func TestDispatchItemUpdate_ForwardsForceFlag(t *testing.T) {
+	captured := newRequestCapture()
+	mux := http.NewServeMux()
+	mux.HandleFunc("/api/v1/workspaces/docapp/items/PLAN-5", func(w http.ResponseWriter, r *http.Request) {
+		switch r.Method {
+		case http.MethodGet:
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{"ref":"PLAN-5","fields":"{\"status\":\"active\"}"}`))
+		case http.MethodPatch:
+			captured.ServeHTTP(w, r)
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{"ref":"PLAN-5","status":"updated"}`))
+		default:
+			t.Fatalf("unexpected method %s", r.Method)
+		}
+	})
+
+	d := &HTTPHandlerDispatcher{Handler: mux, UserResolver: fixedUserResolver(&models.User{ID: "caller"})}
+	ctx := WithDispatchInput(context.Background(), map[string]any{
+		"workspace": "docapp",
+		"ref":       "PLAN-5",
+		"status":    "completed",
+		"force":     true,
+	})
+	res, err := d.Dispatch(ctx, []string{"item", "update"}, nil)
+	if err != nil || (res != nil && res.IsError) {
+		t.Fatalf("Dispatch err=%v IsError=%v: %#v", err, res != nil && res.IsError, res)
+	}
+	if captured.requestCount != 1 {
+		t.Fatalf("expected 1 PATCH, got %d", captured.requestCount)
+	}
+	var body map[string]any
+	if err := json.Unmarshal([]byte(captured.lastBody), &body); err != nil {
+		t.Fatalf("decode body: %v\n%s", err, captured.lastBody)
+	}
+	got, ok := body["force"].(bool)
+	if !ok || !got {
+		t.Errorf("expected force=true in PATCH body, got %v (%T) — body=%s",
+			body["force"], body["force"], captured.lastBody)
+	}
+}
+
+func TestDispatchItemUpdate_OmitsForceWhenAbsent(t *testing.T) {
+	captured := newRequestCapture()
+	mux := http.NewServeMux()
+	mux.HandleFunc("/api/v1/workspaces/docapp/items/PLAN-5", func(w http.ResponseWriter, r *http.Request) {
+		switch r.Method {
+		case http.MethodGet:
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{"ref":"PLAN-5","fields":"{\"status\":\"active\"}"}`))
+		case http.MethodPatch:
+			captured.ServeHTTP(w, r)
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{"ref":"PLAN-5","status":"updated"}`))
+		}
+	})
+
+	d := &HTTPHandlerDispatcher{Handler: mux, UserResolver: fixedUserResolver(&models.User{ID: "caller"})}
+	ctx := WithDispatchInput(context.Background(), map[string]any{
+		"workspace": "docapp",
+		"ref":       "PLAN-5",
+		"status":    "completed",
+	})
+	if _, err := d.Dispatch(ctx, []string{"item", "update"}, nil); err != nil {
+		t.Fatalf("Dispatch err: %v", err)
+	}
+	var body map[string]any
+	if err := json.Unmarshal([]byte(captured.lastBody), &body); err != nil {
+		t.Fatalf("decode body: %v", err)
+	}
+	if _, present := body["force"]; present {
+		t.Errorf("force should be omitted from PATCH body when not set, got body=%s", captured.lastBody)
+	}
+}

internal/mcp/dispatch_http_project.go

@@ -293,7 +293,14 @@ func (d *HTTPHandlerDispatcher) dispatchItemBulkUpdate(
 			continue
 		}
 		fieldsStr := string(fieldsJSON)
-		patchBody, err := json.Marshal(map[string]any{"fields": fieldsStr})
+		patchPayload := map[string]any{"fields": fieldsStr}
+		// IDEA-1494: forward the open-children guard override per-row.
+		// Same flag shape as `pad item bulk-update --force` so the
+		// override travels through both transports identically.
+		if b, ok := input["force"].(bool); ok && b {
+			patchPayload["force"] = true
+		}
+		patchBody, err := json.Marshal(patchPayload)
 		if err != nil {
 			results = append(results, bulkResult{Ref: ref, Error: rowDispatcherError("encode body", err)})
 			continue

internal/mcp/dispatch_http_routes.go

@@ -1011,6 +1011,12 @@ func mapItemMove(input map[string]any) (string, string, []byte, error) {
 	}
 	urlPath := fmt.Sprintf("/api/v1/workspaces/%s/items/%s/move",
 		url.PathEscape(workspace), url.PathEscape(ref))
+	// IDEA-1494 R3 P1: forward the open-children guard override into
+	// the URL query, matching the server-side contract (handler reads
+	// ?force=true). Same wire shape `pad item move --force` uses.
+	if b, ok := input["force"].(bool); ok && b {
+		urlPath += "?force=true"
+	}
 	return http.MethodPost, urlPath, body, nil
 }
 

internal/mcp/errors.go

@@ -115,6 +115,19 @@ const (
 	// detail for debugging without promising any structured shape.
 	ErrServerError ErrorCode = "server_error"
 
+	// ErrOpenChildren fires when handleUpdateItem rejects a
+	// non-terminal → terminal done-field transition because the
+	// item still has non-terminal children (IDEA-1494). The upstream
+	// 409 body carries a `details.open_children` array (refs +
+	// titles + statuses + collection_slug) plus a
+	// `details.hidden_blocker_count` for blockers the caller can't
+	// see. We surface the upstream `code` and `details` verbatim
+	// rather than collapsing to ErrConflict so MCP-driven agents
+	// can self-recover (ship the listed children, then retry — or
+	// pass `force: true` if the children should be intentionally
+	// orphaned). The hint mentions the same escape hatch.
+	ErrOpenChildren ErrorCode = "open_children"
+
 	// ErrRateLimited fires on HTTP 429 responses — either the
 	// general API limiter (per-user, 600/min, burst 60) or the
 	// MCP per-token limiter (60/min/token, burst 60 post-BUG-1430)
@@ -170,6 +183,15 @@ type ErrorPayload struct {
 	// agents see why the call was rejected.
 	RequiredRole string `json:"required_role,omitempty"`
 	CurrentRole  string `json:"current_role,omitempty"`
+
+	// Details carries the upstream error body's `details` object
+	// verbatim when the upstream sets one. Used by ErrOpenChildren
+	// (IDEA-1494) so MCP-driven agents get the same machine-readable
+	// payload — open_children list, hidden_blocker_count, done_field,
+	// attempted_value — that the HTTP API surfaces. json.RawMessage
+	// because the shape is code-specific; clients re-parse against
+	// the known schema for their code branch.
+	Details json.RawMessage `json:"details,omitempty"`
 }
 
 // WorkspaceHint is a minimal workspace summary surfaced in the
@@ -294,10 +316,114 @@ func envelopeFrom(res *mcp.CallToolResult) ErrorEnvelope {
 // with the raw stderr preserved in Message.
 // ─────────────────────────────────────────────────────────────────────
 
+// structuredErrorMarker mirrors internal/cli.StructuredErrorMarker —
+// the versioned line prefix the CLI writes to stderr when it surfaces
+// a structured error. Duplicated (rather than imported) so the mcp
+// classifier doesn't pull the cli package's dependency graph in for
+// one string. Codex round-3 P3 introduced the versioned shape.
+//
+// IMPORTANT: keep in lockstep with cli.StructuredErrorMarker AND with
+// allowedStructuredErrorCodes below. A wire-shape change requires
+// bumping both packages and reviewing the allow-list.
+const structuredErrorMarker = "pad-structured-error/v1: "
+
+// allowedStructuredErrorCodes is the whitelist of upstream codes the
+// MCP layer will surface verbatim. Two transports consult it:
+//
+//   - Stdio: extractStructuredCLIError gates the `pad-structured-error/v1:`
+//     marker on this set (Codex round-3 P3).
+//   - HTTP: classifyHTTPStatus's 409 branch gates the upstream
+//     `error.code` pass-through on this set (Codex round-4 P2).
+//
+// Routing BOTH transports through the same whitelist keeps the
+// ErrorCode enum's closed-contract honest — agents see the same set
+// of structured codes regardless of which dispatcher delivered the
+// response. Pre-round-4 the HTTP path forwarded any non-"conflict"
+// upstream code, which silently widened the enum and let the two
+// transports diverge.
+//
+// Adding a new structured code is a TWO-WAY change: the pad handler
+// must emit it (and emit `details` with a stable, agent-safe shape)
+// AND it must be added here. The matching ErrorCode constant in the
+// declarations block at the top of this file should also be added
+// for type safety in test assertions.
+var allowedStructuredErrorCodes = map[string]struct{}{
+	"open_children": {}, // IDEA-1494
+}
+
+// extractStructuredCLIError scans stderr for the
+// `pad-structured-error/v1: {json}\n` marker line and lifts the
+// structured error envelope into an MCP result. Returns nil when no
+// marker is found OR the payload fails any of the hardening checks
+// (caller falls back to the regex-based classifiers).
+//
+// Hardening (Codex round-3 P3):
+//   - Versioned marker. Unknown versions return nil.
+//   - Whitelisted codes. Unknown codes return nil.
+//   - Last-marker-wins. If multiple markers appear in stderr (a
+//     pathological case today, but possible if a future caller emits
+//     several), we use the LAST one — that's the most recent
+//     classification the CLI emitted, and a malicious earlier line
+//     can't pre-empt it.
+//   - Marker must start the line after trimming whitespace; markers
+//     embedded mid-line (e.g. in a quoted log message) are ignored.
+func extractStructuredCLIError(stderr string) *mcp.CallToolResult {
+	if !strings.Contains(stderr, structuredErrorMarker) {
+		return nil
+	}
+	var lastPayload string
+	for _, line := range strings.Split(stderr, "\n") {
+		line = strings.TrimSpace(line)
+		if !strings.HasPrefix(line, structuredErrorMarker) {
+			continue
+		}
+		lastPayload = strings.TrimPrefix(line, structuredErrorMarker)
+	}
+	if lastPayload == "" {
+		return nil
+	}
+	var env struct {
+		Error struct {
+			Code    string          `json:"code"`
+			Message string          `json:"message"`
+			Details json.RawMessage `json:"details,omitempty"`
+		} `json:"error"`
+	}
+	if err := json.Unmarshal([]byte(lastPayload), &env); err != nil {
+		// Malformed marker payload — fall through to regex
+		// classification rather than blowing up the agent.
+		return nil
+	}
+	if env.Error.Code == "" {
+		return nil
+	}
+	if _, ok := allowedStructuredErrorCodes[env.Error.Code]; !ok {
+		// Unknown code. Don't forward — regex classifier handles it.
+		return nil
+	}
+	return NewErrorResult(ErrorPayload{
+		Code:    ErrorCode(env.Error.Code),
+		Message: env.Error.Message,
+		Details: env.Error.Details,
+	})
+}
+
 // classifyExecError turns an exec.Cmd failure (err + stderr) into a
 // structured envelope. lookup is optional — when supplied, no_workspace
 // errors get available_workspaces enrichment.
 func classifyExecError(ctx context.Context, cmdPath []string, runErr error, stderr string, lookup WorkspaceLister) *mcp.CallToolResult {
+	// IDEA-1494 R2: the CLI emits a single `pad-error: {json}` line
+	// on stderr when surfacing the open-children rejection (see
+	// internal/cli/client.go::WriteOpenChildrenError). Detect it
+	// before regex classification so we lift the structured `code`
+	// + `details` straight through to the MCP envelope instead of
+	// pattern-matching free-form text and downgrading to
+	// validation_failed. Matched on a literal prefix to keep the
+	// classifier cheap and the contract obvious.
+	if structured := extractStructuredCLIError(stderr); structured != nil {
+		return structured
+	}
+
 	// BUG-987 bug 11: cobra automatically appends a "Usage: ..." block
 	// to stderr when a command fails with a runtime error. That help
 	// text uses the OLD CLI verb names (e.g. `pad item block`) which
@@ -634,6 +760,29 @@ func classifyHTTPStatusKind(
 	case http.StatusNotFound:
 		return classify404(ctx, cmdKey, route, bodyText, bodyMessage, lookup, kind, refOrSlug)
 	case http.StatusConflict:
+		// IDEA-1494 R2/R4 P2: preserve the upstream `code` + `details`
+		// when the handler emitted a structured rejection — but ONLY
+		// for codes that appear in the shared allow-list
+		// (allowedStructuredErrorCodes, defined below). The stdio
+		// classifier already gates on the same set; routing both
+		// transports through the same whitelist keeps the
+		// ErrorCode enum's closed contract honest (round-4 P2:
+		// HTTP was forwarding any non-"conflict" code, diverging
+		// from stdio).
+		//
+		// New structured codes get added to allowedStructuredErrorCodes
+		// in one place; both transports adopt them in lockstep.
+		// Unknown codes fall through to generic ErrConflict here —
+		// matching what stdio does for an unknown-code marker.
+		upstream := extractUpstreamErrorEnvelope(bodyText)
+		if _, allowed := allowedStructuredErrorCodes[upstream.Code]; allowed {
+			return NewErrorResult(ErrorPayload{
+				Code:    ErrorCode(upstream.Code),
+				Message: upstream.Message,
+				Hint:    conflictHintFor(bodyMessage, route),
+				Details: upstream.Details,
+			})
+		}
 		return NewErrorResult(ErrorPayload{
 			Code:    ErrConflict,
 			Message: "Conflict — current state changed beneath this update.",
@@ -787,6 +936,37 @@ func extractUpstreamMessage(body string) string {
 	return ""
 }
 
+// upstreamErrorEnvelope is the broader extractor used when the
+// dispatcher needs to pass through the upstream `code` / `details`
+// fields (not just the message). The pad backend uses the same
+// `{"error":{"code":..., "message":..., "details":{...}}}` shape
+// across every handler that returns writeError-style bodies; this
+// parser lifts those fields when present and reports zero-valued
+// strings / nil RawMessage when not. Single source of truth for the
+// pass-through cases in classifyHTTPStatusKind.
+type upstreamErrorEnvelope struct {
+	Code    string          `json:"code"`
+	Message string          `json:"message"`
+	Details json.RawMessage `json:"details,omitempty"`
+}
+
+func extractUpstreamErrorEnvelope(body string) upstreamErrorEnvelope {
+	body = strings.TrimSpace(body)
+	if body == "" {
+		return upstreamErrorEnvelope{}
+	}
+	if strings.EqualFold(body, "404 page not found") {
+		return upstreamErrorEnvelope{}
+	}
+	var env struct {
+		Error upstreamErrorEnvelope `json:"error"`
+	}
+	if err := json.Unmarshal([]byte(body), &env); err == nil {
+		return env.Error
+	}
+	return upstreamErrorEnvelope{}
+}
+
 // itemMissingHint is the per-error-code hint generator for
 // ErrItemNotFound. References the actual ref + route so the agent
 // can pin the failure without re-parsing the message, plus points

internal/mcp/open_children_passthrough_test.go

@@ -0,0 +1,285 @@
+package mcp
+
+// IDEA-1494 R2 — MCP-level coverage for the open-children code/details
+// pass-through, both HTTP and stdio.
+//
+// The contract:
+//
+//   - HTTP path: when the upstream PATCH returns 409 with code=
+//     "open_children" and a populated details body, the dispatcher
+//     surfaces ErrOpenChildren (not the generic ErrConflict) and
+//     preserves the details RawMessage verbatim. Agents branch on
+//     the code, then re-parse details against the known shape.
+//   - stdio path: when the CLI's stderr carries a `pad-error: {json}`
+//     marker line, classifyExecError detects it, lifts the structured
+//     payload, and surfaces it in the same shape — independent of
+//     transport.
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/cli"
+)
+
+func TestClassifyHTTPStatus_OpenChildrenPreservesCodeAndDetails(t *testing.T) {
+	body := []byte(`{
+		"error": {
+			"code": "open_children",
+			"message": "cannot mark PLAN-5 completed: 2 open children still in a non-terminal state. Pass --force to override.",
+			"details": {
+				"open_children": [
+					{"ref":"TASK-7","title":"a","status":"open","collection_slug":"tasks"},
+					{"ref":"TASK-8","title":"b","status":"open","collection_slug":"tasks"}
+				],
+				"hidden_blocker_count": 0,
+				"done_field": "status",
+				"attempted_value": "completed"
+			}
+		}
+	}`)
+
+	res := classifyHTTPStatus(context.Background(), "item update", 409, body, nil)
+	env, ok := res.StructuredContent.(ErrorEnvelope)
+	if !ok {
+		t.Fatalf("expected ErrorEnvelope, got %T", res.StructuredContent)
+	}
+	if env.Error.Code != ErrOpenChildren {
+		t.Fatalf("code: got %q, want %q", env.Error.Code, ErrOpenChildren)
+	}
+	if env.Error.Message == "" {
+		t.Errorf("message should be preserved from upstream, got empty")
+	}
+	if len(env.Error.Details) == 0 {
+		t.Fatal("details should be preserved as raw JSON, got empty")
+	}
+
+	// Round-trip the details into the shared CLI struct (the same one
+	// MCP-driven agents would unmarshal against). Confirms the shape
+	// survives the HTTP-classifier transit unchanged.
+	var got cli.OpenChildrenDetails
+	if err := json.Unmarshal(env.Error.Details, &got); err != nil {
+		t.Fatalf("details should round-trip into OpenChildrenDetails: %v", err)
+	}
+	if len(got.OpenChildren) != 2 {
+		t.Errorf("open_children len: got %d, want 2", len(got.OpenChildren))
+	}
+	if got.AttemptedValue != "completed" {
+		t.Errorf("attempted_value: got %q, want completed", got.AttemptedValue)
+	}
+	if got.DoneField != "status" {
+		t.Errorf("done_field: got %q, want status", got.DoneField)
+	}
+}
+
+// TestClassifyHTTPStatus_ConflictWithoutCodeFallsToErrConflict pins
+// the inverse: a 409 WITHOUT an upstream code (or with code="conflict"
+// explicitly) still collapses to ErrConflict so we don't break the
+// existing classifier contract for ordinary version-mismatch 409s.
+func TestClassifyHTTPStatus_ConflictWithoutCodeFallsToErrConflict(t *testing.T) {
+	cases := []struct {
+		name string
+		body string
+	}{
+		{"no code at all", `{"error":{"message":"version mismatch"}}`},
+		{"explicit conflict code", `{"error":{"code":"conflict","message":"version mismatch"}}`},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			res := classifyHTTPStatus(context.Background(), "item update", 409, []byte(tc.body), nil)
+			env := res.StructuredContent.(ErrorEnvelope)
+			if env.Error.Code != ErrConflict {
+				t.Errorf("code: got %q, want %q", env.Error.Code, ErrConflict)
+			}
+			if len(env.Error.Details) != 0 {
+				t.Errorf("details should be empty for generic 409, got %s", string(env.Error.Details))
+			}
+		})
+	}
+}
+
+func TestClassifyExecError_OpenChildrenMarkerLiftsStructuredPayload(t *testing.T) {
+	stderr := `Error: connecting to backend
+pad-structured-error/v1: {"error":{"code":"open_children","message":"cannot mark PLAN-5 completed: 1 open child still in a non-terminal state. Pass --force to override.","details":{"open_children":[{"ref":"TASK-7","title":"x","status":"open","collection_slug":"tasks"}],"hidden_blocker_count":0,"done_field":"status","attempted_value":"completed"}}}
+cannot mark PLAN-5 completed: 1 open child still in a non-terminal state. Pass --force to override.
+  TASK-7 — x (status=open)
+Pass --force to override.
+`
+	res := classifyExecError(context.Background(),
+		[]string{"item", "update"},
+		errors.New("exit status 1"),
+		stderr,
+		nil)
+	env, ok := res.StructuredContent.(ErrorEnvelope)
+	if !ok {
+		t.Fatalf("expected ErrorEnvelope, got %T", res.StructuredContent)
+	}
+	if env.Error.Code != ErrOpenChildren {
+		t.Fatalf("code: got %q, want %q", env.Error.Code, ErrOpenChildren)
+	}
+	var details cli.OpenChildrenDetails
+	if err := json.Unmarshal(env.Error.Details, &details); err != nil {
+		t.Fatalf("details did not round-trip: %v", err)
+	}
+	if len(details.OpenChildren) != 1 || details.OpenChildren[0].Ref != "TASK-7" {
+		t.Errorf("open_children mismatch: %+v", details.OpenChildren)
+	}
+}
+
+// TestClassifyExecError_NoMarkerFallsThrough confirms stderr WITHOUT
+// the marker classifies via the existing regex matchers (validation /
+// auth / item-not-found / generic) — i.e. the marker detection is
+// purely additive and doesn't break the pre-IDEA-1494 stderr-classify
+// contracts.
+func TestClassifyExecError_NoMarkerFallsThrough(t *testing.T) {
+	stderr := "Error: invalid status value\n"
+	res := classifyExecError(context.Background(),
+		[]string{"item", "update"},
+		errors.New("exit status 1"),
+		stderr,
+		nil)
+	env := res.StructuredContent.(ErrorEnvelope)
+	if env.Error.Code == ErrOpenChildren {
+		t.Errorf("plain validation stderr must not be classified as open_children")
+	}
+}
+
+// TestClassifyExecError_UnknownStructuredCodeFallsThrough covers
+// Codex round-3 P3 marker hardening: even with a well-formed
+// pad-structured-error/v1 marker, a code that's NOT in the allow-list
+// must not be surfaced — fall back to regex classification instead.
+// Prevents a CLI bug or third-party tool from smuggling an
+// unsanitized code past the MCP boundary.
+func TestClassifyExecError_UnknownStructuredCodeFallsThrough(t *testing.T) {
+	stderr := `pad-structured-error/v1: {"error":{"code":"made_up_code","message":"x"}}
+Error: invalid status value
+`
+	res := classifyExecError(context.Background(),
+		[]string{"item", "update"},
+		errors.New("exit status 1"),
+		stderr,
+		nil)
+	env := res.StructuredContent.(ErrorEnvelope)
+	if env.Error.Code == ErrorCode("made_up_code") {
+		t.Errorf("unknown structured code must not be surfaced; got %q", env.Error.Code)
+	}
+}
+
+// TestClassifyExecError_OldMarkerVersionIgnored confirms that an
+// older (or unrecognized future) marker version is ignored rather
+// than parsed. Validates the "version token is parsed, not just
+// prefix-matched" property — agents on the new mcp build won't be
+// confused by stderr from a stale CLI binary that still uses the
+// pre-round-3 unversioned `pad-error:` shape.
+func TestClassifyExecError_OldMarkerVersionIgnored(t *testing.T) {
+	stderr := `pad-error: {"error":{"code":"open_children","message":"x"}}
+Error: invalid status value
+`
+	res := classifyExecError(context.Background(),
+		[]string{"item", "update"},
+		errors.New("exit status 1"),
+		stderr,
+		nil)
+	env := res.StructuredContent.(ErrorEnvelope)
+	if env.Error.Code == ErrOpenChildren {
+		t.Errorf("pre-v1 unversioned marker must not be lifted; got %q", env.Error.Code)
+	}
+}
+
+// TestClassifyExecError_MarkerEmbeddedMidLineIgnored ensures a marker
+// substring inside a quoted log message can't impersonate a real
+// structured error. The classifier requires the marker at the line's
+// start (after whitespace trim) — embedded variants fall through.
+func TestClassifyExecError_MarkerEmbeddedMidLineIgnored(t *testing.T) {
+	stderr := `Error: backend logged "pad-structured-error/v1: {\"error\":{\"code\":\"open_children\"}}"
+`
+	res := classifyExecError(context.Background(),
+		[]string{"item", "update"},
+		errors.New("exit status 1"),
+		stderr,
+		nil)
+	env := res.StructuredContent.(ErrorEnvelope)
+	if env.Error.Code == ErrOpenChildren {
+		t.Errorf("embedded marker must not be lifted; got %q", env.Error.Code)
+	}
+}
+
+// TestClassifyExecError_LastMarkerWins confirms multiple markers
+// resolve to the LAST one — a malicious earlier line can't pre-empt
+// the CLI's actual final classification.
+func TestClassifyExecError_LastMarkerWins(t *testing.T) {
+	stderr := `pad-structured-error/v1: {"error":{"code":"made_up_code","message":"first"}}
+pad-structured-error/v1: {"error":{"code":"open_children","message":"second","details":{"open_children":[]}}}
+`
+	res := classifyExecError(context.Background(),
+		[]string{"item", "update"},
+		errors.New("exit status 1"),
+		stderr,
+		nil)
+	env := res.StructuredContent.(ErrorEnvelope)
+	if env.Error.Code != ErrOpenChildren {
+		t.Errorf("expected the later marker to win; got %q (message=%q)",
+			env.Error.Code, env.Error.Message)
+	}
+}
+
+// TestClassifyHTTPStatus_UnknownConflictCodeFallsBackToErrConflict
+// covers Codex round-4 P2: HTTP and stdio must agree on the closed
+// set of structured codes they surface. Pre-fix the HTTP path
+// forwarded ANY non-"conflict" code; stdio whitelisted only
+// open_children. The two transports diverged on what an agent saw
+// from an upstream that emitted `code=some_future_code`.
+//
+// Post-fix: both consult allowedStructuredErrorCodes. An upstream
+// 409 with a code NOT in the whitelist collapses to generic
+// ErrConflict on the HTTP path, mirroring what the stdio path does
+// for an unknown-code structured marker.
+func TestClassifyHTTPStatus_UnknownConflictCodeFallsBackToErrConflict(t *testing.T) {
+	body := []byte(`{
+		"error": {
+			"code": "some_future_code",
+			"message": "x",
+			"details": {"anything":1}
+		}
+	}`)
+	res := classifyHTTPStatus(context.Background(), "item update", 409, body, nil)
+	env := res.StructuredContent.(ErrorEnvelope)
+	if env.Error.Code != ErrConflict {
+		t.Errorf("unknown upstream code must collapse to ErrConflict on HTTP path; got %q", env.Error.Code)
+	}
+	if len(env.Error.Details) != 0 {
+		t.Errorf("details from un-whitelisted code must not leak; got %s", string(env.Error.Details))
+	}
+}
+
+// TestStructuredErrorCodeParityAcrossTransports asserts that the same
+// "unknown code" rejection produces the same envelope shape from
+// both transports — ie HTTP doesn't widen the enum behind stdio's
+// back. Round-4 P2.
+func TestStructuredErrorCodeParityAcrossTransports(t *testing.T) {
+	httpRes := classifyHTTPStatus(context.Background(), "item update", 409,
+		[]byte(`{"error":{"code":"made_up_code","message":"x"}}`), nil)
+	httpEnv := httpRes.StructuredContent.(ErrorEnvelope)
+
+	stdioRes := classifyExecError(context.Background(),
+		[]string{"item", "update"},
+		errors.New("exit status 1"),
+		`pad-structured-error/v1: {"error":{"code":"made_up_code","message":"x"}}
+`, nil)
+	stdioEnv := stdioRes.StructuredContent.(ErrorEnvelope)
+
+	if httpEnv.Error.Code == ErrorCode("made_up_code") {
+		t.Errorf("HTTP path leaked unknown code: %q", httpEnv.Error.Code)
+	}
+	if stdioEnv.Error.Code == ErrorCode("made_up_code") {
+		t.Errorf("stdio path leaked unknown code: %q", stdioEnv.Error.Code)
+	}
+	// Both transports must agree: neither surfaces the unknown code.
+	// They CAN classify the fallback differently (HTTP → ErrConflict
+	// for 409, stdio → ErrServerError for an unknown stderr line)
+	// because the upstream signals are genuinely different — the
+	// parity contract is "no transport widens the enum beyond the
+	// whitelist," not "both transports produce identical envelopes."
+}

internal/models/item.go

@@ -582,6 +582,15 @@ type ItemUpdate struct {
 	// (since nil pointer means "don't change" in partial updates)
 	ClearAssignedUser bool `json:"clear_assigned_user,omitempty"`
 	ClearAgentRole    bool `json:"clear_agent_role,omitempty"`
+
+	// Force, when true, overrides the server-side open-children guard
+	// (IDEA-1494) that otherwise rejects a non-terminal → terminal
+	// done-field transition while the item still has non-terminal
+	// children. Transport-only: the store layer never sees it (the
+	// handler consumes it before calling UpdateItem). Used by `pad item
+	// update --force` and MCP `pad_item.action: update` with
+	// `force: true`.
+	Force bool `json:"force,omitempty"`
 }
 
 // ErrInvalidFieldsType / ErrInvalidTagsType are returned by

internal/server/handlers_items.go

@@ -664,6 +664,19 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// IDEA-1494: precheck closure populated below when the patch
+	// includes a fields update AND --force is NOT set. Threads into
+	// the three UpdateItem call sites that follow. nil = no guard.
+	var openChildrenPrecheck func(tx *sql.Tx, existing *models.Item) error
+
+	// IDEA-1494 R4 P3: parent-link change is captured here and
+	// applied AFTER the main UpdateItemWithPreCheck succeeds, so a
+	// guard rejection on the status field doesn't leave a committed
+	// link change behind. Hoisted out of the `if input.Fields != nil`
+	// block so the post-write step can read them.
+	var parentValue string
+	var parentProvided bool
+
 	// If fields are being updated, validate against schema
 	if input.Fields != nil {
 		coll, err := s.store.GetCollection(item.CollectionID)
@@ -687,8 +700,9 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 		// Extract parent from fields — it's managed via item_links, not stored in fields JSON.
 		// Accepts both "parent" and "plan" as the field key.
 		// Skip this if the schema actually defines a field with that key.
-		var parentValue string
-		var parentProvided bool
+		// (parentValue / parentProvided are declared at outer scope so
+		// the deferred link-write block below can read them — see
+		// IDEA-1494 R4 P3.)
 		for _, key := range []string{"parent", "plan"} {
 			if schemaHasField(schema, key) {
 				continue
@@ -740,6 +754,71 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 		// Auto-populate date fields on status changes
 		autoPopulateDates(fieldMap, item.Fields, schema)
 
+		// IDEA-1494 open-children guard. The actual check runs INSIDE
+		// the store transaction (see Store.UpdateItemWithPreCheck) so
+		// the children-list query and the parent's status write share
+		// a snapshot — closes the TOCTOU window flagged in Codex
+		// round 2 (P2). Here we just stage the inputs and the
+		// visibility filters; the closure runs later under the
+		// workspace seq lock + parent-children advisory lock.
+		if !input.Force {
+			var settings models.CollectionSettings
+			if coll.Settings != "" {
+				_ = json.Unmarshal([]byte(coll.Settings), &settings)
+			}
+			// Pre-compute visibility once. The guard's invariant check
+			// considers ALL children (data integrity); the visibility
+			// filter only affects which children appear in the 409
+			// payload — hidden ones are surfaced as a count.
+			//
+			// Fail CLOSED on visibility-lookup error (Codex round-3 P1):
+			// a swallowed error here would leave visIDs==nil, which
+			// openChildrenGuardChildVisible treats as "no restriction"
+			// — leaking metadata for hidden children. Surfacing the
+			// internal error blocks the update entirely rather than
+			// risk an information leak.
+			visIDs, visErr := s.visibleCollectionIDs(r, workspaceID)
+			if visErr != nil {
+				writeInternalError(w, visErr)
+				return
+			}
+			guestFull, guestGranted, gerr := s.guestResourceFilter(r, workspaceID)
+			if gerr != nil {
+				writeInternalError(w, gerr)
+				return
+			}
+			gctx := openChildrenGuardContext{
+				r:                    r,
+				workspaceID:          workspaceID,
+				itemID:               item.ID,
+				parentSchema:         schema,
+				parentSettings:       settings,
+				newFieldMap:          fieldMap,
+				visibleCollectionIDs: visIDs,
+				guestFullCollIDs:     guestFull,
+				guestGrantedItemIDs:  guestGranted,
+			}
+			openChildrenPrecheck = func(tx *sql.Tx, existing *models.Item) error {
+				// Codex round-3 P2: classify the transition against
+				// the in-tx snapshot of the parent's fields, not the
+				// pre-tx capture. A concurrent writer can change the
+				// done-field value between handler-side read and lock
+				// acquisition; using `existing.Fields` (loaded by
+				// UpdateItemWithPreCheck after the tx began) avoids
+				// both false-fire and false-skip classifications.
+				txCtx := gctx
+				txCtx.currentFieldsJS = existing.Fields
+				details, derr := s.runOpenChildrenGuard(tx, txCtx)
+				if derr != nil {
+					return derr
+				}
+				if details != nil {
+					return &openChildrenGuardError{details: details}
+				}
+				return nil
+			}
+		}
+
 		validatedFields, err := json.Marshal(fieldMap)
 		if err != nil {
 			writeError(w, http.StatusInternalServerError, "internal_error", "Failed to marshal validated fields")
@@ -748,22 +827,25 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 		validated := string(validatedFields)
 		input.Fields = &validated
 
-		// Update parent link if parent was provided in the update
-		if parentProvided {
-			if parentValue != "" {
-				actor, _ := actorFromRequest(r)
-				if _, err := s.store.SetParentLink(workspaceID, item.ID, parentValue, actor); err != nil {
-					writeError(w, http.StatusInternalServerError, "internal_error", fmt.Sprintf("failed to update parent link: %v", err))
-					return
-				}
-			} else {
-				// Parent was explicitly set to empty/null — clear the link
-				if err := s.store.ClearParentLink(item.ID); err != nil {
-					writeError(w, http.StatusInternalServerError, "internal_error", fmt.Sprintf("failed to clear parent link: %v", err))
-					return
-				}
-			}
-		}
+		// IDEA-1494 R4 P3: parent-link mutations are DEFERRED until
+		// after the field write succeeds. Pre-fix this block ran the
+		// link update INLINE here — before the precheck even fired —
+		// so a PATCH that combined a parent change with a status flip
+		// could end up with the link committed AND the field write
+		// rejected by the guard. Caller saw 409 but the parent had
+		// already moved.
+		//
+		// Now we record the desired link change and execute it ONLY
+		// when the main UpdateItemWithPreCheck call below succeeds.
+		// A guard rejection short-circuits with the link untouched.
+		// Documented choice: "reorder, don't tx-wrap" — wrapping the
+		// link mutation in the same store tx as UpdateItem would
+		// require threading a tx through SetParentLink (which has
+		// its own tx already, and is also called from the
+		// handler_item_links path). Reordering is the smaller surgery
+		// and preserves atomicity in the failure direction — caller
+		// only sees both writes on success, or neither (with a clear
+		// 409) on rejection.
 	}
 
 	// Designated-applier routing (PLAN-1248 TASK-1257). When the
@@ -880,7 +962,7 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 					return errStaleCollabSnapshot
 				}
 			}
-			updated, uerr := s.store.UpdateItem(item.ID, input)
+			updated, uerr := s.store.UpdateItemWithPreCheck(item.ID, input, openChildrenPrecheck)
 			if uerr != nil {
 				return uerr
 			}
@@ -889,6 +971,10 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 			return nil
 		})
 		if err != nil {
+			if details, ok := asOpenChildrenGuardError(err); ok {
+				writeOpenChildrenError(w, itemRefOrSlug(*item), details)
+				return
+			}
 			if errors.Is(err, errStaleCollabSnapshot) {
 				writeError(w, http.StatusConflict, "stale_collab_snapshot",
 					"This editor's view is out of sync with the server; please reload.")
@@ -944,7 +1030,7 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 		// Store.UpdateItem's content-versioning peek at Title.
 		// Per Codex review round 9.
 		err := s.applyContentViaCollab(r, item.ID, *input.Content, func() error {
-			updated, uerr := s.store.UpdateItem(item.ID, input)
+			updated, uerr := s.store.UpdateItemWithPreCheck(item.ID, input, openChildrenPrecheck)
 			if uerr != nil {
 				return uerr
 			}
@@ -958,8 +1044,15 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 			// or directWrite ran the full UpdateItem inside the
 			// lock (fullWriteHandled tracks which).
 			input.Content = nil
+		} else if details, ok := asOpenChildrenGuardError(err); ok {
+			// IDEA-1494 R2: the open-children guard fired inside the
+			// directWrite callback. Don't let applyContentViaCollab's
+			// "any error falls through" policy retry the write —
+			// rejection is final.
+			writeOpenChildrenError(w, itemRefOrSlug(*item), details)
+			return
 		}
-		// Any error path (e.g. ErrAllAppliersTimedOut, retry
+		// Any other error path (e.g. ErrAllAppliersTimedOut, retry
 		// exhaustion) falls through to direct write — graceful
 		// degradation. The helper logs the specifics so operators
 		// see degraded paths.
@@ -969,9 +1062,18 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 	if fullWriteHandled {
 		updated = fullWriteUpdated
 	} else {
-		updated, err = s.store.UpdateItem(item.ID, input)
+		updated, err = s.store.UpdateItemWithPreCheck(item.ID, input, openChildrenPrecheck)
 	}
 	if err != nil {
+		// IDEA-1494 R2: surface the open-children guard rejection as
+		// the structured 409 BEFORE the UNIQUE-constraint / generic
+		// internal-error paths can swallow it. Same shape the
+		// in-handler write produced pre-R2, but now correctly fires
+		// when the guard ran inside the store tx.
+		if details, ok := asOpenChildrenGuardError(err); ok {
+			writeOpenChildrenError(w, itemRefOrSlug(*item), details)
+			return
+		}
 		// Map UNIQUE constraint races (e.g. concurrent updates that both
 		// pass checkUniqueFields and then both hit the partial unique
 		// index on invocation_slug) to 409 conflict, matching the create
@@ -988,6 +1090,34 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// IDEA-1494 R4 P3: deferred parent-link mutation. Runs ONLY now
+	// that the main UpdateItemWithPreCheck succeeded (the precheck
+	// passed AND the field write committed). A guard rejection above
+	// `return`s before reaching here, so the link is untouched on
+	// the failure path.
+	//
+	// Failure of the link write itself after a successful field
+	// write is still possible (e.g. SetParentLink discovers a cycle
+	// the cycle-check missed, or a DB error). We surface that as a
+	// 500 with the field write already committed — same partial-
+	// success window the pre-IDEA-1494 code had in the OTHER
+	// direction, and not made worse by the reorder. A future tx-
+	// wrap fix could close this entirely; out of scope for round 4.
+	if parentProvided {
+		if parentValue != "" {
+			actor, _ := actorFromRequest(r)
+			if _, err := s.store.SetParentLink(workspaceID, item.ID, parentValue, actor); err != nil {
+				writeError(w, http.StatusInternalServerError, "internal_error", fmt.Sprintf("failed to update parent link: %v", err))
+				return
+			}
+		} else {
+			if err := s.store.ClearParentLink(item.ID); err != nil {
+				writeError(w, http.StatusInternalServerError, "internal_error", fmt.Sprintf("failed to clear parent link: %v", err))
+				return
+			}
+		}
+	}
+
 	// Build rich metadata describing what changed
 	var meta string
 	if changes := diffFields(item.Fields, updated.Fields); changes != "" {
@@ -1259,9 +1389,72 @@ func (s *Server) handleMoveItem(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// IDEA-1494 R3 P1: same open-children guard as the regular update
+	// path. `pad item move ... --field status=completed` would
+	// otherwise bypass it. We classify the proposed transition
+	// against the DESTINATION collection's schema (conservative —
+	// honors the schema the item is moving INTO, so a target-schema
+	// terminal value can't be smuggled in via a source whose schema
+	// doesn't recognize the value as terminal). Visibility filter is
+	// pre-computed and fails closed on lookup error, mirroring the
+	// update path.
+	//
+	// Force override on the move path lives on the URL query
+	// (?force=true). Move's request body shape is already fixed
+	// ({target_collection, field_overrides}) and adding a body
+	// field would require coordinating with the CLI/MCP move
+	// callers; the query param sidesteps that without changing the
+	// JSON contract. Same semantics as `pad item update --force`.
+	moveForce := r.URL.Query().Get("force") == "true"
+
+	var movePrecheck func(tx *sql.Tx, existing *models.Item) error
+	if !moveForce {
+		var destSettings models.CollectionSettings
+		if targetColl.Settings != "" {
+			_ = json.Unmarshal([]byte(targetColl.Settings), &destSettings)
+		}
+		visIDs, visErr := s.visibleCollectionIDs(r, workspaceID)
+		if visErr != nil {
+			writeInternalError(w, visErr)
+			return
+		}
+		guestFull, guestGranted, gerr := s.guestResourceFilter(r, workspaceID)
+		if gerr != nil {
+			writeInternalError(w, gerr)
+			return
+		}
+		mgctx := openChildrenGuardContext{
+			r:                    r,
+			workspaceID:          workspaceID,
+			itemID:               item.ID,
+			parentSchema:         targetSchema,
+			parentSettings:       destSettings,
+			newFieldMap:          result.Fields,
+			visibleCollectionIDs: visIDs,
+			guestFullCollIDs:     guestFull,
+			guestGrantedItemIDs:  guestGranted,
+		}
+		movePrecheck = func(tx *sql.Tx, existing *models.Item) error {
+			txCtx := mgctx
+			txCtx.currentFieldsJS = existing.Fields
+			details, derr := s.runOpenChildrenGuard(tx, txCtx)
+			if derr != nil {
+				return derr
+			}
+			if details != nil {
+				return &openChildrenGuardError{details: details}
+			}
+			return nil
+		}
+	}
+
 	// Move the item
-	moved, err := s.store.MoveItem(item.ID, targetColl.ID, string(fieldsJSON))
+	moved, err := s.store.MoveItemWithPreCheck(item.ID, targetColl.ID, string(fieldsJSON), movePrecheck)
 	if err != nil {
+		if details, ok := asOpenChildrenGuardError(err); ok {
+			writeOpenChildrenError(w, itemRefOrSlug(*item), details)
+			return
+		}
 		writeInternalError(w, err)
 		return
 	}

internal/server/handlers_items_open_children_guard.go

@@ -0,0 +1,345 @@
+package server
+
+// IDEA-1494 — Refuse to mark an item terminal while it still has
+// non-terminal children. The guard fires server-side so it covers
+// every interface (CLI, MCP, web UI) that hits handleUpdateItem.
+//
+// Trigger conditions (all must hold):
+//  1. The PATCH supplies a fields update.
+//  2. The new value for the parent collection's resolved done-field
+//     key is in TerminalValuesForDoneField.
+//  3. The CURRENT value for that key is NOT in the terminal set.
+//     (no-op terminal → terminal and terminal-to-terminal transitions
+//     bypass the guard — only entering the terminal set is gated.)
+//  4. The parent item has at least one non-deleted child whose own
+//     collection schema reports the child as non-terminal.
+//
+// The caller can override the guard with `--force` (CLI) / `force: true`
+// (MCP body field). The override still records the status change.
+//
+// ── Visibility (Codex round 2 P1) ─────────────────────────────────────
+// The invariant itself is a DATA-INTEGRITY gate, not a visibility
+// gate: we evaluate it against ALL children (so a caller with reduced
+// visibility can't close a parent that has children they don't see).
+// The 409 response payload, however, is sanitized — only children the
+// caller is allowed to see appear in `details.open_children`. When
+// hidden children contributed to the rejection (in part or in full)
+// the payload carries a separate `hidden_blocker_count` so MCP-driven
+// agents can distinguish:
+//
+//   - len(open_children)==0 + hidden_blocker_count==0 → would not have
+//     rejected (no path here, but the shape is unambiguous);
+//   - len(open_children)==N + hidden_blocker_count==0 → caller can see
+//     every blocker;
+//   - len(open_children)==N + hidden_blocker_count==M → caller sees N
+//     blockers + M additional that they can't access; recovery requires
+//     either coordination with someone who can or `--force` if their
+//     role permits.
+//
+// ── Atomicity (Codex round 2 P2) ──────────────────────────────────────
+// The guard query runs INSIDE the same store transaction as the
+// UPDATE, so a concurrent child insert / child status flip can't slip
+// between the read and the write. See Store.UpdateItemWithPreCheck +
+// Store.AcquireParentChildrenLocks for the locking shape.
+//
+// ── Response (HTTP 409 Conflict) ──────────────────────────────────────
+//
+//	{
+//	  "error": {
+//	    "code": "open_children",
+//	    "message": "cannot mark TASK-5 completed: ...",
+//	    "details": {
+//	      "open_children": [
+//	        {"ref":"TASK-7","title":"...","status":"open","collection_slug":"tasks"},
+//	        ...
+//	      ],
+//	      "hidden_blocker_count": 0,
+//	      "done_field": "status",
+//	      "attempted_value": "completed"
+//	    }
+//	  }
+//	}
+//
+// `details.open_children` is the canonical machine-readable list — the
+// CLI renders the human message FROM the same list (plus the hidden
+// count) so the two paths agree by construction.
+
+import (
+	"database/sql"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"strings"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+// errOpenChildrenGuard is the sentinel the precheck returns to the
+// store layer when the guard fires. The handler unwraps it via
+// errors.As to lift the structured details back out for the 409
+// response. Using a typed sentinel (rather than a generic error)
+// keeps the store/handler boundary clean — the store just sees "the
+// precheck rejected" and rolls back the tx.
+type openChildrenGuardError struct {
+	details *openChildrenDetails
+}
+
+func (e *openChildrenGuardError) Error() string {
+	return fmt.Sprintf("open-children guard: %d visible + %d hidden blocker(s)",
+		len(e.details.OpenChildren), e.details.HiddenBlockerCount)
+}
+
+// itemRefOrSlug formats an item's issue ref (e.g. "TASK-5") when its
+// collection prefix + item number are populated, falling back to its
+// slug. Avoids pulling internal/cli into the server package for one
+// helper.
+func itemRefOrSlug(it models.Item) string {
+	if it.CollectionPrefix != "" && it.ItemNumber != nil {
+		return fmt.Sprintf("%s-%d", it.CollectionPrefix, *it.ItemNumber)
+	}
+	return it.Slug
+}
+
+// openChildEntry is the per-child payload returned in the structured
+// error. Mirrors what `pad item list --parent X --status non-terminal`
+// would surface so MCP-driven agents can self-recover (e.g. ship the
+// children, then retry).
+type openChildEntry struct {
+	Ref            string `json:"ref"`
+	Title          string `json:"title"`
+	Status         string `json:"status"`
+	CollectionSlug string `json:"collection_slug"`
+}
+
+// openChildrenDetails is the structured details payload returned in
+// the 409 error body. `OpenChildren` is filtered for caller visibility;
+// `HiddenBlockerCount` reports the number of additional blockers that
+// would also need to clear (or be force-overridden) but that the
+// caller can't see.
+type openChildrenDetails struct {
+	OpenChildren       []openChildEntry `json:"open_children"`
+	HiddenBlockerCount int              `json:"hidden_blocker_count"`
+	DoneField          string           `json:"done_field"`
+	AttemptedValue     string           `json:"attempted_value"`
+}
+
+// openChildrenGuardContext bundles the read-only inputs the precheck
+// closure needs from the handler. Kept as a struct so the closure
+// signature stays narrow.
+type openChildrenGuardContext struct {
+	r               *http.Request
+	workspaceID     string
+	itemID          string
+	parentSchema    models.CollectionSchema
+	parentSettings  models.CollectionSettings
+	newFieldMap     map[string]any
+	currentFieldsJS string
+	// visibility filters, all pre-computed once by the handler.
+	visibleCollectionIDs []string // nil = unrestricted
+	guestFullCollIDs     []string
+	guestGrantedItemIDs  []string
+}
+
+// runOpenChildrenGuard executes the guard logic inside the
+// store-layer transaction. Returns:
+//
+//   - (nil, nil) when the guard doesn't apply (no transition, no
+//     children, or all children terminal) — the update should proceed.
+//   - (details, nil) when the guard fires — the caller wraps these in
+//     openChildrenGuardError so the store rolls back. `details` is
+//     already sanitized for visibility.
+//   - (nil, err) on infrastructure errors (DB read failed, etc.).
+func (s *Server) runOpenChildrenGuard(tx *sql.Tx, ctx openChildrenGuardContext) (*openChildrenDetails, error) {
+	doneKey, terminalValues := models.TerminalValuesForDoneField(ctx.parentSchema, ctx.parentSettings)
+
+	// Trigger condition #1: the patch must set the resolved done-field
+	// key to a terminal value.
+	rawNew, ok := ctx.newFieldMap[doneKey]
+	if !ok {
+		return nil, nil
+	}
+	newStr, ok := rawNew.(string)
+	if !ok || newStr == "" {
+		return nil, nil
+	}
+	if !valueInSet(newStr, terminalValues) {
+		return nil, nil
+	}
+	// Trigger condition #3: the current value must NOT already be
+	// terminal. terminal → terminal and no-op terminal transitions
+	// bypass.
+	currentVal := extractFieldString(ctx.currentFieldsJS, doneKey)
+	if currentVal != "" && valueInSet(currentVal, terminalValues) {
+		return nil, nil
+	}
+
+	children, err := s.store.GetChildItemsTx(tx, ctx.itemID)
+	if err != nil {
+		return nil, fmt.Errorf("load children for open-children guard: %w", err)
+	}
+	if len(children) == 0 {
+		return nil, nil
+	}
+
+	// Per-child done evaluation against the child's OWN collection.
+	// Cache schema+settings per child collection. The collection rows
+	// don't need to be read from the same tx — schemas don't mutate
+	// in the kind of races this guard cares about.
+	ctxCache := make(map[string]doneContext)
+	// Codex round-5 P3: initialize as empty (not nil) so the
+	// hidden-only rejection path serializes `open_children: []`
+	// rather than `null`. The contract documents this field as an
+	// array; clients (CLI renderer, MCP agents) `range` over it
+	// even when hidden_blocker_count > 0.
+	open := []openChildEntry{}
+	hidden := 0
+	for i := range children {
+		child := &children[i]
+		dc, cached := ctxCache[child.CollectionID]
+		if !cached {
+			// Codex round-3 P3: include soft-deleted collections so a
+			// child still attached to a soft-deleted collection is
+			// evaluated against its own done-field schema, not the
+			// default-status fallback (which would false-block when
+			// the collection's done-field is e.g. `resolution` with
+			// custom terminal_options). Mirrors the inclusion rule
+			// childrenDoneFiltersForParent uses (items.go ≈2165).
+			if coll, cerr := s.store.GetCollectionAnyState(child.CollectionID); cerr == nil && coll != nil {
+				_ = json.Unmarshal([]byte(coll.Schema), &dc.schema)
+				if coll.Settings != "" {
+					_ = json.Unmarshal([]byte(coll.Settings), &dc.settings)
+				}
+			}
+			ctxCache[child.CollectionID] = dc
+		}
+		// INVARIANT check uses ALL children. A restricted caller still
+		// gets blocked by a non-terminal child they can't see — the
+		// guard's purpose is data integrity, not visibility filtering.
+		if isItemDone(child.Fields, child.CollectionID, map[string]doneContext{child.CollectionID: dc}) {
+			continue
+		}
+
+		// Sanitize the response payload: surface only children this
+		// caller has permission to see. Hidden blockers are counted
+		// separately so the agent knows blocking state exists without
+		// learning ref/title/status of items they can't access.
+		if !s.openChildrenGuardChildVisible(ctx, child) {
+			hidden++
+			continue
+		}
+		childDoneKey, _ := models.TerminalValuesForDoneField(dc.schema, dc.settings)
+		open = append(open, openChildEntry{
+			Ref:            itemRefOrSlug(*child),
+			Title:          child.Title,
+			Status:         extractFieldString(child.Fields, childDoneKey),
+			CollectionSlug: child.CollectionSlug,
+		})
+	}
+	if len(open) == 0 && hidden == 0 {
+		return nil, nil
+	}
+	return &openChildrenDetails{
+		OpenChildren:       open,
+		HiddenBlockerCount: hidden,
+		DoneField:          doneKey,
+		AttemptedValue:     newStr,
+	}, nil
+}
+
+// openChildrenGuardChildVisible mirrors the visibility check used by
+// the per-parent progress endpoint (handlers_items.go around
+// `progVisIDs` / `isCollectionVisible` / `isItemVisibleToGuest`).
+// Returns true when the caller is unrestricted or when the child
+// passes both the collection-level and item-level guest filters.
+func (s *Server) openChildrenGuardChildVisible(gctx openChildrenGuardContext, child *models.Item) bool {
+	// Unrestricted (admin / owner / no grant filtering in play): nil
+	// visibleCollectionIDs means "see everything." Matches the
+	// progress handler's convention.
+	if gctx.visibleCollectionIDs == nil {
+		return true
+	}
+	if !isCollectionVisible(child.CollectionID, gctx.visibleCollectionIDs) {
+		return false
+	}
+	return s.isItemVisibleToGuest(gctx.r, gctx.workspaceID, child, gctx.guestFullCollIDs, gctx.guestGrantedItemIDs)
+}
+
+// writeOpenChildrenError emits the 409 response with both a human
+// message AND the structured details payload. `parentRef` is the
+// already-formatted parent ref (e.g. "PLAN-12"); the message names the
+// parent + child count and points at --force. The phrasing splits on
+// whether any blockers are hidden so the human-readable line carries
+// the same signal `hidden_blocker_count` does for machines.
+func writeOpenChildrenError(w http.ResponseWriter, parentRef string, details *openChildrenDetails) {
+	visible := len(details.OpenChildren)
+	hidden := details.HiddenBlockerCount
+	var msg string
+	switch {
+	case visible > 0 && hidden > 0:
+		msg = fmt.Sprintf("cannot mark %s %s: %d open child(ren) still in a non-terminal state, plus %d additional you don't have access to. Pass --force to override.",
+			parentRef, details.AttemptedValue, visible, hidden)
+	case visible > 0:
+		noun := "child"
+		if visible != 1 {
+			noun = "children"
+		}
+		msg = fmt.Sprintf("cannot mark %s %s: %d open %s still in a non-terminal state. Pass --force to override.",
+			parentRef, details.AttemptedValue, visible, noun)
+	default:
+		// hidden > 0 only — the caller can't see any blocking children.
+		noun := "child"
+		if hidden != 1 {
+			noun = "children"
+		}
+		msg = fmt.Sprintf("cannot mark %s %s: blocked by %d open %s you don't have access to. Pass --force to override (if your role permits).",
+			parentRef, details.AttemptedValue, hidden, noun)
+	}
+
+	writeJSON(w, http.StatusConflict, map[string]any{
+		"error": map[string]any{
+			"code":    "open_children",
+			"message": msg,
+			"details": details,
+		},
+	})
+}
+
+// asOpenChildrenGuardError unwraps a store-layer error that may carry
+// an openChildrenGuardError sentinel. Returns the sanitized details
+// and true when the error is the guard rejecting the precheck; nil +
+// false for any other error (the handler returns those via the
+// generic upstream-error path).
+func asOpenChildrenGuardError(err error) (*openChildrenDetails, bool) {
+	var sentinel *openChildrenGuardError
+	if errors.As(err, &sentinel) {
+		return sentinel.details, true
+	}
+	return nil, false
+}
+
+// valueInSet does a case-insensitive membership check.
+func valueInSet(v string, set []string) bool {
+	low := strings.ToLower(strings.TrimSpace(v))
+	for _, s := range set {
+		if strings.ToLower(s) == low {
+			return true
+		}
+	}
+	return false
+}
+
+// extractFieldString reads a top-level scalar string from a JSON-encoded
+// fields map. Returns "" when the JSON is empty, malformed, the key is
+// missing, or the value isn't a string.
+func extractFieldString(fieldsJSON, key string) string {
+	if fieldsJSON == "" || fieldsJSON == "{}" {
+		return ""
+	}
+	var m map[string]any
+	if err := json.Unmarshal([]byte(fieldsJSON), &m); err != nil {
+		return ""
+	}
+	s, _ := m[key].(string)
+	return s
+}

internal/store/collections.go

@@ -89,6 +89,45 @@ func (s *Store) GetCollection(id string) (*models.Collection, error) {
 	return &c, nil
 }
 
+// GetCollectionAnyState is GetCollection without the
+// `deleted_at IS NULL` filter — used by the open-children guard
+// (IDEA-1494 R3 P3) so a child still attached to a soft-deleted
+// collection is evaluated against ITS collection's actual done-field
+// schema instead of falling back to the default `status` terminal
+// list (which would mis-classify children of custom-done-field
+// collections as non-terminal and produce false blockers).
+//
+// Mirrors the inclusion rule baked into childrenDoneFiltersForParent /
+// doneFiltersForWorkspace, both of which already include soft-deleted
+// collections for exactly this reason.
+func (s *Store) GetCollectionAnyState(id string) (*models.Collection, error) {
+	var c models.Collection
+	var createdAt, updatedAt string
+	var deletedAt *string
+	var isDefault bool
+
+	err := s.db.QueryRow(s.q(`
+		SELECT id, workspace_id, name, slug, prefix, icon, description, schema, settings, sort_order, is_default, is_system, created_at, updated_at, deleted_at
+		FROM collections
+		WHERE id = ?
+	`), id).Scan(
+		&c.ID, &c.WorkspaceID, &c.Name, &c.Slug, &c.Prefix, &c.Icon, &c.Description,
+		&c.Schema, &c.Settings, &c.SortOrder, &isDefault, &c.IsSystem,
+		&createdAt, &updatedAt, &deletedAt,
+	)
+	if err == sql.ErrNoRows {
+		return nil, nil
+	}
+	if err != nil {
+		return nil, fmt.Errorf("get collection (any state): %w", err)
+	}
+	c.IsDefault = isDefault
+	c.CreatedAt = parseTime(createdAt)
+	c.UpdatedAt = parseTime(updatedAt)
+	c.DeletedAt = parseTimePtr(deletedAt)
+	return &c, nil
+}
+
 func (s *Store) GetCollectionBySlug(workspaceID, slug string) (*models.Collection, error) {
 	var id string
 	err := s.db.QueryRow(s.q(`

internal/store/items.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"regexp"
+	"sort"
 	"strings"
 	"time"
 
@@ -272,6 +273,56 @@ func (s *Store) GetItem(id string) (*models.Item, error) {
 	return &item, nil
 }
 
+// getItemTx is the in-transaction variant of GetItem. Used by
+// UpdateItemWithPreCheck to re-read the parent under the workspace +
+// parent-children locks so the invariant precheck classifies the
+// transition against a snapshot that's stable for the rest of the tx
+// (Codex round-3 P2). Soft-deleted items are excluded, matching
+// GetItem's contract.
+func (s *Store) getItemTx(tx *sql.Tx, id string) (*models.Item, error) {
+	var item models.Item
+	var createdAt, updatedAt string
+	var deletedAt *string
+	var pinned bool
+
+	err := tx.QueryRow(s.q(`
+		SELECT i.id, i.workspace_id, i.collection_id, i.title, i.slug, i.content, i.fields, i.tags,
+		       i.pinned, i.sort_order, i.parent_id, i.assigned_user_id, i.agent_role_id, i.role_sort_order,
+		       i.created_by, i.last_modified_by, i.source,
+		       i.item_number, i.seq, i.created_at, i.updated_at, i.deleted_at,
+		       c.slug, c.name, c.icon, c.prefix,
+		       COALESCE(au.name, ''), COALESCE(au.email, ''),
+		       COALESCE(ar.name, ''), COALESCE(ar.slug, ''), COALESCE(ar.icon, '')
+		FROM items i
+		JOIN collections c ON c.id = i.collection_id
+		LEFT JOIN users au ON au.id = i.assigned_user_id
+		LEFT JOIN agent_roles ar ON ar.id = i.agent_role_id
+		WHERE i.id = ? AND i.deleted_at IS NULL
+	`), id).Scan(
+		&item.ID, &item.WorkspaceID, &item.CollectionID, &item.Title, &item.Slug,
+		&item.Content, &item.Fields, &item.Tags,
+		&pinned, &item.SortOrder, &item.ParentID, &item.AssignedUserID, &item.AgentRoleID, &item.RoleSortOrder,
+		&item.CreatedBy, &item.LastModifiedBy, &item.Source,
+		&item.ItemNumber, &item.Seq, &createdAt, &updatedAt, &deletedAt,
+		&item.CollectionSlug, &item.CollectionName, &item.CollectionIcon, &item.CollectionPrefix,
+		&item.AssignedUserName, &item.AssignedUserEmail,
+		&item.AgentRoleName, &item.AgentRoleSlug, &item.AgentRoleIcon,
+	)
+	if err == sql.ErrNoRows {
+		return nil, nil
+	}
+	if err != nil {
+		return nil, fmt.Errorf("get item (tx): %w", err)
+	}
+
+	item.Pinned = pinned
+	item.CreatedAt = parseTime(createdAt)
+	item.UpdatedAt = parseTime(updatedAt)
+	item.DeletedAt = parseTimePtr(deletedAt)
+	hydrateItemComputedMetadata(&item)
+	return &item, nil
+}
+
 func (s *Store) GetItemBySlug(workspaceID, slug string) (*models.Item, error) {
 	var id string
 	err := s.db.QueryRow(s.q(`
@@ -1285,6 +1336,29 @@ func (s *Store) listItemsFTS(workspaceID string, params models.ItemListParams) (
 }
 
 func (s *Store) UpdateItem(id string, input models.ItemUpdate) (*models.Item, error) {
+	return s.UpdateItemWithPreCheck(id, input, nil)
+}
+
+// UpdateItemWithPreCheck is UpdateItem with an optional pre-mutation
+// hook that runs inside the same transaction (and, on Postgres, holds
+// the same workspace advisory lock) as the update itself. Callers can
+// use the hook to enforce cross-row invariants whose decision must be
+// atomic with the write — e.g. the open-children guard (IDEA-1494)
+// needs the children-list query and the parent's status flip to share
+// a tx so a concurrent child insert / child status change can't slip
+// between them.
+//
+// The hook receives the transaction and the freshly-read existing
+// item. Returning a non-nil error rolls the tx back and surfaces the
+// error verbatim — callers can return a sentinel and `errors.Is` it
+// in the handler.
+//
+// Pass a nil precheck for the standard, unchecked update path.
+func (s *Store) UpdateItemWithPreCheck(
+	id string,
+	input models.ItemUpdate,
+	precheck func(tx *sql.Tx, existing *models.Item) error,
+) (*models.Item, error) {
 	existing, err := s.GetItem(id)
 	if err != nil {
 		return nil, err
@@ -1310,6 +1384,66 @@ func (s *Store) UpdateItem(id string, input models.ItemUpdate) (*models.Item, er
 		return nil, err
 	}
 
+	// IDEA-1494 round 2: also acquire the parent-children advisory
+	// lock for THIS item (as a potential parent) AND for its own
+	// parent (when it is itself a child). That gives the open-children
+	// guard a tight serialization:
+	//
+	//   - A parent's UpdateItem precheck holds `pad:parent-children:<parent_id>`
+	//     while reading the children list and writing the parent.
+	//   - A child's UpdateItem holds the same key for its parent
+	//     while it writes itself.
+	//
+	// Result: a child status-flip that would invalidate the parent's
+	// guard cannot interleave between the parent's children-read and
+	// the parent's status-write. SQLite gets this for free from
+	// BEGIN IMMEDIATE; Postgres needs the explicit advisory lock.
+	//
+	// Lock ordering: workspace lock → THIS item's parent lock → THIS
+	// item's own children lock. Both lock keys are namespaced under
+	// `pad:parent-children:` so they only contend on the parent ID;
+	// acquiring two distinct keys in a fixed order can't deadlock.
+	if err := s.acquireParentChildrenLocksForUpdate(tx, id); err != nil {
+		return nil, err
+	}
+
+	// IDEA-1494 round 2: run the caller's invariant check (if any)
+	// AFTER the locks are held but BEFORE any mutation. Closing the
+	// guard-vs-write TOCTOU window relies on this ordering — the
+	// precheck's view of `items` / `item_links` is the same one the
+	// UPDATE below will write against because every concurrent
+	// UpdateItem on this parent (or on any of its children) blocks
+	// on the same advisory key.
+	//
+	// Codex round-3 P2: re-read the item INSIDE the tx (after locks)
+	// and pass that fresh snapshot to the precheck. The pre-tx
+	// `existing` above was loaded without holding the workspace seq
+	// lock or the parent-children lock — a concurrent writer could
+	// have flipped the parent's done-field between that read and
+	// here, which would mis-classify the transition (false-fire or
+	// false-skip). The post-lock re-read sees what the UPDATE will
+	// write against.
+	if precheck != nil {
+		freshExisting, ferr := s.getItemTx(tx, id)
+		if ferr != nil {
+			return nil, fmt.Errorf("re-read item under lock: %w", ferr)
+		}
+		if freshExisting == nil {
+			// Item was deleted between the pre-tx read and the
+			// post-lock re-read. Treat as not-found and let the
+			// handler surface a 404. Returning nil here mirrors the
+			// existing == nil branch above.
+			return nil, nil
+		}
+		if err := precheck(tx, freshExisting); err != nil {
+			return nil, err
+		}
+		// Use the fresh snapshot for the rest of the function too —
+		// otherwise the mutation logic below would proceed from
+		// stale data and undo the integrity gain.
+		existing = freshExisting
+	}
+
 	ts := now()
 
 	// Create version if content is changing
@@ -1578,6 +1712,25 @@ func (s *Store) RestoreItem(id string) (*models.Item, error) {
 		return nil, err
 	}
 
+	// Codex round-3 P1 / round-4 P1: restoring an item resurrects it
+	// as a (potentially non-terminal) child of EVERY parent it's
+	// linked to (one item can have both a `parent` and an
+	// `implements` link). Lock ALL of those parents' children-keys
+	// via the canonical sorted-multi-lock helper so concurrent
+	// UpdateItemWithPreCheck callers on any of them see this
+	// resurrection in their post-lock snapshots.
+	//
+	// Pre-fix this called AcquireParentChildrenLock for a single
+	// LIMIT 1 row — a multi-parent child would have left another
+	// parent's precheck racing the resurrection.
+	parentIDs, err := s.listParentChildLockKeys(tx, id)
+	if err != nil {
+		return nil, err
+	}
+	if err := s.AcquireParentChildrenLocks(tx, parentIDs...); err != nil {
+		return nil, err
+	}
+
 	ts := now()
 	result, err := tx.Exec(s.q(`
 		UPDATE items SET deleted_at = NULL, updated_at = ?, seq = `+nextWorkspaceSeqSubquery+`
@@ -1727,14 +1880,36 @@ func (s *Store) CreateItemLink(workspaceID string, input models.ItemLinkCreate,
 		createdBy = "user"
 	}
 
-	_, err = s.db.Exec(s.q(`
+	tx, err := s.db.Begin()
+	if err != nil {
+		return nil, fmt.Errorf("begin tx: %w", err)
+	}
+	defer tx.Rollback()
+
+	// Codex round-3 P1: when this link puts `sourceID` into the
+	// children-set of `target` (i.e. linkType ∈ childLinkTypes), lock
+	// the target's parent-children key so a concurrent
+	// UpdateItemWithPreCheck on the target can't read 0 open children
+	// while we're about to attach a non-terminal one. Non-child link
+	// types (blocks, supersedes, …) don't affect the children-set so
+	// we skip the lock — keeps the common case lock-free.
+	if isChildLinkType(linkType) {
+		if err := s.AcquireParentChildrenLocks(tx, input.TargetID); err != nil {
+			return nil, err
+		}
+	}
+
+	if _, err := tx.Exec(s.q(`
 		INSERT INTO item_links (id, workspace_id, source_id, target_id, link_type, created_by, created_at)
 		VALUES (?, ?, ?, ?, ?, ?, ?)
-	`), id, workspaceID, sourceID, input.TargetID, linkType, createdBy, ts)
-	if err != nil {
+	`), id, workspaceID, sourceID, input.TargetID, linkType, createdBy, ts); err != nil {
 		return nil, fmt.Errorf("create item link: %w", err)
 	}
 
+	if err := tx.Commit(); err != nil {
+		return nil, fmt.Errorf("commit create item link: %w", err)
+	}
+
 	return s.getItemLink(id)
 }
 
@@ -1882,7 +2057,37 @@ func (s *Store) GetItemLinkByID(id string) (*models.ItemLink, error) {
 }
 
 func (s *Store) DeleteItemLink(id string) error {
-	result, err := s.db.Exec(s.q("DELETE FROM item_links WHERE id = ?"), id)
+	tx, err := s.db.Begin()
+	if err != nil {
+		return fmt.Errorf("begin tx: %w", err)
+	}
+	defer tx.Rollback()
+
+	// Codex round-3 P1: peek the link's type + target before deleting
+	// so we can lock the target's parent-children key when this link
+	// participates in the children-set. Without this, a concurrent
+	// UpdateItemWithPreCheck on the target could read the child as
+	// still attached, decide the parent has no open children, and
+	// commit a terminal status while we orphan a non-terminal child.
+	//
+	// We DON'T lock for non-child link types (blocks, supersedes, …)
+	// — they don't affect the children-set, so contention there is
+	// unnecessary.
+	var linkType, targetID string
+	err = tx.QueryRow(s.q("SELECT link_type, target_id FROM item_links WHERE id = ?"), id).Scan(&linkType, &targetID)
+	if err == sql.ErrNoRows {
+		return sql.ErrNoRows
+	}
+	if err != nil {
+		return fmt.Errorf("peek item link for delete: %w", err)
+	}
+	if isChildLinkType(linkType) {
+		if err := s.AcquireParentChildrenLocks(tx, targetID); err != nil {
+			return err
+		}
+	}
+
+	result, err := tx.Exec(s.q("DELETE FROM item_links WHERE id = ?"), id)
 	if err != nil {
 		return fmt.Errorf("delete item link: %w", err)
 	}
@@ -1890,7 +2095,7 @@ func (s *Store) DeleteItemLink(id string) error {
 	if rows == 0 {
 		return sql.ErrNoRows
 	}
-	return nil
+	return tx.Commit()
 }
 
 // --- Phase Links ---
@@ -1898,6 +2103,13 @@ func (s *Store) DeleteItemLink(id string) error {
 // SetParentLink sets the parent for an item. Since an item can belong to at most
 // one parent, this deletes any existing parent link for the item first.
 // Includes cycle detection to prevent A→B→A or deeper ancestor loops.
+//
+// Codex round-3 P1: acquires `pad:parent-children:<id>` for BOTH the
+// old parent (if any) AND the new parent in sorted order. That makes
+// a concurrent UpdateItemWithPreCheck on either parent block on the
+// same key, closing the link-mutation TOCTOU gap — without this, the
+// guard could read 0 open children while this method was about to
+// attach a non-terminal child.
 func (s *Store) SetParentLink(workspaceID, itemID, parentID, createdBy string) (*models.ItemLink, error) {
 	// Cycle detection: walk the ancestor chain from parentID to ensure itemID is not an ancestor.
 	if err := s.checkParentCycle(itemID, parentID); err != nil {
@@ -1910,6 +2122,25 @@ func (s *Store) SetParentLink(workspaceID, itemID, parentID, createdBy string) (
 	}
 	defer tx.Rollback()
 
+	// Find the existing parent (if any) so we can lock against it too.
+	// The DELETE below targets link_type='parent' specifically, which
+	// matches what the guard's children query treats as the parent
+	// edge (childLinkTypes includes 'parent'); other child-link types
+	// like 'implements' aren't displaced by this method so we don't
+	// need their old parent here.
+	var oldParentID sql.NullString
+	if err := tx.QueryRow(s.q(`
+		SELECT target_id FROM item_links
+		WHERE source_id = ? AND link_type = 'parent'
+		LIMIT 1
+	`), itemID).Scan(&oldParentID); err != nil && err != sql.ErrNoRows {
+		return nil, fmt.Errorf("lookup existing parent: %w", err)
+	}
+
+	if err := s.AcquireParentChildrenLocks(tx, oldParentID.String, parentID); err != nil {
+		return nil, err
+	}
+
 	// Delete existing parent link for this item (if any)
 	if _, err := tx.Exec(s.q(`DELETE FROM item_links WHERE source_id = ? AND link_type = 'parent'`), itemID); err != nil {
 		return nil, fmt.Errorf("delete existing parent link: %w", err)
@@ -1961,12 +2192,36 @@ func (s *Store) checkParentCycle(itemID, parentID string) error {
 }
 
 // ClearParentLink removes the parent link for an item.
+//
+// Codex round-3 P1: runs in a tx and acquires `pad:parent-children:<old>`
+// before the DELETE so a concurrent UpdateItemWithPreCheck on the old
+// parent blocks until this commit. Detaching a child is materially
+// similar to attaching one — the parent's children-set changes either
+// way and the guard must see a consistent view.
 func (s *Store) ClearParentLink(itemID string) error {
-	_, err := s.db.Exec(s.q(`DELETE FROM item_links WHERE source_id = ? AND link_type = 'parent'`), itemID)
+	tx, err := s.db.Begin()
 	if err != nil {
+		return fmt.Errorf("begin tx: %w", err)
+	}
+	defer tx.Rollback()
+
+	var oldParentID sql.NullString
+	if err := tx.QueryRow(s.q(`
+		SELECT target_id FROM item_links
+		WHERE source_id = ? AND link_type = 'parent'
+		LIMIT 1
+	`), itemID).Scan(&oldParentID); err != nil && err != sql.ErrNoRows {
+		return fmt.Errorf("lookup parent for clear: %w", err)
+	}
+	if oldParentID.Valid && oldParentID.String != "" {
+		if err := s.AcquireParentChildrenLocks(tx, oldParentID.String); err != nil {
+			return err
+		}
+	}
+	if _, err := tx.Exec(s.q(`DELETE FROM item_links WHERE source_id = ? AND link_type = 'parent'`), itemID); err != nil {
 		return fmt.Errorf("clear parent link: %w", err)
 	}
-	return nil
+	return tx.Commit()
 }
 
 // GetParentForItem returns the parent link for an item, or nil if it has no parent.
@@ -2303,7 +2558,181 @@ func (s *Store) GetAllItemProgress(workspaceID, collectionSlug string) ([]ItemPr
 // GetChildItems returns all non-deleted child items linked to the given parent
 // via item_links. Returns children from any collection.
 func (s *Store) GetChildItems(parentItemID string) ([]models.Item, error) {
-	rows, err := s.db.Query(s.q(fmt.Sprintf(`
+	return s.getChildItems(s.db, parentItemID)
+}
+
+// GetChildItemsTx is the in-transaction variant of GetChildItems. The
+// underlying query is the same as GetChildItems; using a *sql.Tx ties
+// the read to the caller's transaction so it sees the same snapshot
+// the subsequent UPDATE will write against (IDEA-1494 R2).
+//
+// Atomicity vs. concurrent child mutations is provided by the caller's
+// transaction-scoped locking:
+//
+//   - SQLite: db-wide BEGIN IMMEDIATE write lock (set globally via
+//     `_txlock=immediate`) serializes all writers, so any concurrent
+//     child insert / child update blocks until this tx commits or
+//     rolls back. No additional locking is needed.
+//   - Postgres: the caller is expected to hold a parent-keyed advisory
+//     lock (see AcquireParentChildrenLocks below) so concurrent
+//     mutations on the same parent's children are serialized against
+//     this read.
+//
+// FOR UPDATE is intentionally NOT used — the underlying SELECT carries
+// DISTINCT (necessary because item_links can carry both `parent` and
+// the legacy `plan` link_type for the same edge), and Postgres rejects
+// `SELECT DISTINCT … FOR UPDATE`. The advisory-lock pattern sidesteps
+// that constraint while still giving us a serialized snapshot.
+func (s *Store) GetChildItemsTx(tx *sql.Tx, parentItemID string) ([]models.Item, error) {
+	if tx == nil {
+		return s.GetChildItems(parentItemID)
+	}
+	return s.getChildItems(tx, parentItemID)
+}
+
+// acquireParentChildrenLocksForUpdate is the in-tx helper UpdateItem
+// uses to serialize itself against the open-children guard
+// (IDEA-1494 R2 / R4). It acquires the parent-children advisory lock
+// for:
+//
+//  1. EVERY parent the item is currently a child of (via item_links
+//     of type ∈ childLinkTypes — `parent` AND `implements`).
+//     childLinkTypes is the inclusion rule GetChildItems walks, so
+//     this set is exactly the parents whose guard precheck could see
+//     this item as a child.
+//  2. THIS item itself, as a parent — so any concurrent precheck
+//     running against this item's own children list waits.
+//
+// Codex round-4 P1: pre-fix this helper used `LIMIT 1` and only
+// locked one parent. A child of TWO parents (one via `parent`, one
+// via `implements`) would let a status flip race against the
+// un-locked parent's precheck. The query now returns ALL distinct
+// parent target_ids and we lock every one.
+//
+// All keys (parents + self) are funnelled through
+// AcquireParentChildrenLocks so they're acquired in a single,
+// canonical sorted order — round-4 P2's deadlock-avoidance contract.
+// No call site outside this helper takes pad:parent-children:* locks
+// in any other order.
+func (s *Store) acquireParentChildrenLocksForUpdate(tx *sql.Tx, itemID string) error {
+	if s.dialect.Driver() != DriverPostgres {
+		return nil
+	}
+	parentIDs, err := s.listParentChildLockKeys(tx, itemID)
+	if err != nil {
+		return err
+	}
+	keys := append(parentIDs, itemID)
+	return s.AcquireParentChildrenLocks(tx, keys...)
+}
+
+// listParentChildLockKeys returns every target_id this item is the
+// `source_id` of under a childLinkTypes link — i.e. every parent
+// whose children-set includes this item. Used wherever we need to
+// lock all of an item's parents at once (UpdateItem, RestoreItem,
+// link-mutation paths).
+//
+// IMPORTANT: this MUST stay in lockstep with childLinkTypes (the
+// inclusion rule GetChildItems uses). If a new link type joins the
+// children-set, both the query here and the read query must add it
+// together so lock coverage matches read coverage.
+func (s *Store) listParentChildLockKeys(tx *sql.Tx, itemID string) ([]string, error) {
+	rows, err := tx.Query(s.q(fmt.Sprintf(`
+		SELECT DISTINCT target_id FROM item_links
+		WHERE source_id = ? AND link_type IN (%s)
+	`, childLinkTypeSQL())), itemID)
+	if err != nil {
+		return nil, fmt.Errorf("list parent lock keys: %w", err)
+	}
+	defer rows.Close()
+	var out []string
+	for rows.Next() {
+		var id string
+		if err := rows.Scan(&id); err != nil {
+			return nil, fmt.Errorf("scan parent lock key: %w", err)
+		}
+		if id == "" || id == itemID {
+			continue
+		}
+		out = append(out, id)
+	}
+	return out, rows.Err()
+}
+
+// AcquireParentChildrenLocks is the CANONICAL helper for taking
+// `pad:parent-children:<id>` advisory locks. Every call site that
+// needs to serialize against the open-children guard MUST go through
+// this function — UpdateItemWithPreCheck precheck, MoveItemWithPreCheck
+// precheck, RestoreItem, SetParentLink, ClearParentLink,
+// CreateItemLink (child-link types), DeleteItemLink (child-link
+// types). Ad-hoc single-key acquisition outside this helper is
+// FORBIDDEN — two call sites taking distinct keys in different
+// orders WILL deadlock under contention (the classic AB/BA shape).
+//
+// The contract this helper enforces:
+//
+//  1. Deduplicate. Repeated IDs in the input collapse to one lock.
+//  2. Drop empties. "" / nil entries don't get locked.
+//  3. Acquire in canonical sorted order (string-sort by ID).
+//     Two concurrent callers that share any subset of IDs always
+//     grab the overlap in the same order → no deadlock.
+//
+// SQLite is a no-op because the global BEGIN IMMEDIATE write lock
+// (set via _txlock=immediate in store.go) already serializes every
+// writer; advisory locks would add no protection there.
+//
+// Per Codex round-3 P1 (link-mutations bypass) + round-4 P2
+// (lock-order asymmetry). If you find yourself writing
+// `pg_advisory_xact_lock(... 'pad:parent-children:' ...)` anywhere
+// outside this helper, route it through here instead.
+func (s *Store) AcquireParentChildrenLocks(tx *sql.Tx, parentItemIDs ...string) error {
+	if s.dialect.Driver() != DriverPostgres {
+		return nil
+	}
+	seen := make(map[string]struct{}, len(parentItemIDs))
+	keys := make([]string, 0, len(parentItemIDs))
+	for _, id := range parentItemIDs {
+		if id == "" {
+			continue
+		}
+		if _, ok := seen[id]; ok {
+			continue
+		}
+		seen[id] = struct{}{}
+		keys = append(keys, id)
+	}
+	sort.Strings(keys)
+	for _, k := range keys {
+		if _, err := tx.Exec("SELECT pg_advisory_xact_lock(hashtext('pad:parent-children:' || $1))", k); err != nil {
+			return fmt.Errorf("acquire parent-children lock %q: %w", k, err)
+		}
+	}
+	return nil
+}
+
+// isChildLinkType reports whether the given link type is one the
+// open-children guard counts toward the children-set (i.e. matches
+// the inclusion rule baked into `childLinkTypes` and used by
+// GetChildItems via `childLinkTypeSQL()`). Single source of truth so
+// link-writer lock acquisition can't drift from the read query's set.
+func isChildLinkType(linkType string) bool {
+	for _, t := range childLinkTypes {
+		if t == linkType {
+			return true
+		}
+	}
+	return false
+}
+
+// childQueryer is the small surface the children-list query needs from
+// either *sql.DB or *sql.Tx. Lets getChildItems serve both the unlocked
+// and tx-bound paths from one implementation.
+type childQueryer interface {
+	Query(query string, args ...any) (*sql.Rows, error)
+}
+
+func (s *Store) getChildItems(q childQueryer, parentItemID string) ([]models.Item, error) {
+	rows, err := q.Query(s.q(fmt.Sprintf(`
 		SELECT DISTINCT i.id, i.workspace_id, i.collection_id, i.title, i.slug, i.content, i.fields, i.tags,
 		       i.pinned, i.sort_order, i.parent_id, i.assigned_user_id, i.agent_role_id, i.role_sort_order,
 		       i.created_by, i.last_modified_by, i.source,
@@ -2382,6 +2811,26 @@ func (s *Store) PopulateHasChildren(items []models.Item) {
 // client polling /items-changes?since=cursor would render the item
 // under its old collection until a full refresh.
 func (s *Store) MoveItem(itemID, targetCollectionID, newFieldsJSON string) (*models.Item, error) {
+	return s.MoveItemWithPreCheck(itemID, targetCollectionID, newFieldsJSON, nil)
+}
+
+// MoveItemWithPreCheck is MoveItem with the same precheck escape hatch
+// UpdateItemWithPreCheck offers. Codex round-3 P1: a `pad item move
+// ... --field status=done` writes a terminal done-field value through
+// MoveItem, bypassing the open-children guard wired into the regular
+// UpdateItem path. This variant runs the caller's invariant check
+// inside the move's transaction, after acquiring the workspace seq
+// lock + the parent-children lock for this item's own parent (it CAN
+// itself be a parent — children stay attached across collection
+// changes — so we lock for itself too, matching
+// acquireParentChildrenLocksForUpdate's shape).
+//
+// The precheck receives a fresh in-tx snapshot of the item, same as
+// UpdateItemWithPreCheck (the pre-tx `existing` is replaced).
+func (s *Store) MoveItemWithPreCheck(
+	itemID, targetCollectionID, newFieldsJSON string,
+	precheck func(tx *sql.Tx, existing *models.Item) error,
+) (*models.Item, error) {
 	existing, err := s.GetItem(itemID)
 	if err != nil {
 		return nil, err
@@ -2400,6 +2849,24 @@ func (s *Store) MoveItem(itemID, targetCollectionID, newFieldsJSON string) (*mod
 		return nil, err
 	}
 
+	if err := s.acquireParentChildrenLocksForUpdate(tx, itemID); err != nil {
+		return nil, err
+	}
+
+	if precheck != nil {
+		freshExisting, ferr := s.getItemTx(tx, itemID)
+		if ferr != nil {
+			return nil, fmt.Errorf("re-read item under lock: %w", ferr)
+		}
+		if freshExisting == nil {
+			return nil, sql.ErrNoRows
+		}
+		if err := precheck(tx, freshExisting); err != nil {
+			return nil, err
+		}
+		existing = freshExisting
+	}
+
 	_, err = tx.Exec(s.q(`
 		UPDATE items
 		SET collection_id = ?, fields = ?, updated_at = ?, seq = `+nextWorkspaceSeqSubquery+`