pad/internal/mcp/dispatch_http_advanced.go

package mcp

import (
	"context"
	"encoding/json"
	"fmt"
	"net/http"
	"net/http/httptest"
	"net/url"
	"strings"

	"github.com/mark3labs/mcp-go/mcp"

	"github.com/PerpetualSoftware/pad/internal/models"
)

// resolveAssignName rewrites a `--assign <name|email>` input into
// `assigned_user_id <uuid>` by hitting the workspace-members
// endpoint and finding a matching user. Mirrors the CLI's behaviour
// in cmd/pad/main.go's itemCreateCmd / itemUpdateCmd / itemListCmd —
// without this resolution, agents passing human-friendly assignee
// values would silently get empty results (the store filters by
// `i.assigned_user_id = ?` UUID, no name fallback).
//
// Returns the input map with `assign` replaced by `assigned_user_id`
// when a match is found, or unchanged when `assign` is missing /
// empty. Mismatches return a clear error so agents know to pass a
// different name.
//
// The returned map is always a fresh map — the caller's reference
// isn't mutated, matching the no-mutation contract of the rest of
// the dispatcher.
func (d *HTTPHandlerDispatcher) resolveAssignName(
	ctx context.Context,
	user *models.User,
	input map[string]any,
) (map[string]any, error) {
	rawAssign, present := input["assign"]
	if !present {
		return input, nil
	}
	assign, _ := rawAssign.(string)
	if assign == "" {
		// An EMPTY `assign` is deliberately still a no-op, and this is
		// the one place in TASK-2571 where the fix stops short. Do not
		// "finish the job" by mapping it to a clear without reading
		// this first — codex round 1 proposed exactly that.
		//
		// `assign` is SCHEMA-DECLARED (catalog_item.go), unlike
		// `assigned_user_id`, which an agent can only reach by knowing
		// it exists. Every other schema-declared string on this mapper
		// — title, content, comment, tags — follows one convention:
		// empty means NOT PROVIDED. An MCP client that fills declared
		// optional params with "" rather than omitting them is
		// therefore harmless today; making `assign: ""` mean "clear"
		// would turn that same client into one that silently unassigns
		// every item it touches. Destructive, silent, and inconsistent
		// with the four params beside it.
		//
		// The gap is real — an agent reading the schema will reach for
		// `assign: ""` and get a lie — but the remedy is explicit
		// `clear_assigned_user` / `clear_agent_role` params (option (b)
		// on TASK-2571, deferred by the lead as additive sugar), not a
		// destructive meaning bolted onto an optional string. Tracked
		// separately; the MCP instructions name the ID form meanwhile.
		return input, nil
	}
	// Already-resolved? If the caller used `--field assigned_user_id=<uuid>`
	// that's a separate input key — we don't touch it. If the caller
	// passed both `assign` and `assigned_user_id`, the explicit ID
	// wins; drop the assign value to avoid the resolution lookup.
	out := cloneStringMap(input)
	if existingID, _ := out["assigned_user_id"].(string); existingID != "" {
		delete(out, "assign")
		return out, nil
	}

	workspace, _ := input["workspace"].(string)
	if workspace == "" {
		return nil, fmt.Errorf("workspace is required to resolve --assign")
	}

	userID, err := d.lookupAssigneeID(ctx, user, workspace, assign)
	if err != nil {
		return nil, err
	}
	out["assigned_user_id"] = userID
	delete(out, "assign")
	return out, nil
}

// resolveRoleSlug rewrites a `--role <slug>` input into
// `agent_role_id <uuid>` by hitting the agent-roles endpoint and
// finding a matching role. Mirrors the CLI's behaviour in
// itemCreateCmd / itemUpdateCmd which treats `--role` as a slug or
// ID and resolves to the column UUID before sending the create/
// update — without resolution, agents passing slugs would silently
// get empty results (the store filters by `i.agent_role_id = ?`
// UUID, with slug accepted only on the LIST endpoint, not the
// item-mutation handlers).
//
// Symmetric to resolveAssignName: returns the input map with `role`
// replaced by `agent_role_id` when a match is found, or unchanged
// when `role` is missing / empty. Mismatches return a clear error.
//
// The handleGetAgentRole endpoint at /agent-roles/{roleID} accepts
// either a UUID or a slug as roleID, so this single GET resolves
// both. If the caller passed an explicit `agent_role_id` alongside
// `--role`, the explicit ID wins (matches the --assign precedence
// in resolveAssignName).
//
// The returned map is always a fresh copy — the caller's reference
// isn't mutated.
func (d *HTTPHandlerDispatcher) resolveRoleSlug(
	ctx context.Context,
	user *models.User,
	input map[string]any,
) (map[string]any, error) {
	rawRole, present := input["role"]
	if !present {
		return input, nil
	}
	role, _ := rawRole.(string)
	if role == "" {
		// Empty `role` stays a no-op for the same reason an empty
		// `assign` does — see the long note in resolveAssignName.
		return input, nil
	}
	out := cloneStringMap(input)
	if existingID, _ := out["agent_role_id"].(string); existingID != "" {
		// Explicit ID wins over slug; drop the role key to avoid the
		// resolution lookup below.
		delete(out, "role")
		return out, nil
	}

	workspace, _ := input["workspace"].(string)
	if workspace == "" {
		return nil, fmt.Errorf("workspace is required to resolve --role")
	}

	roleID, err := d.lookupRoleID(ctx, user, workspace, role)
	if err != nil {
		return nil, err
	}
	out["agent_role_id"] = roleID
	delete(out, "role")
	return out, nil
}

// lookupRoleID issues an in-handler GET against
// /api/v1/workspaces/{ws}/agent-roles/{slug} and returns the role's
// canonical id. The handler accepts either UUID or slug for roleID
// (see handleGetAgentRole), so callers can pass a slug like
// "implementer" or a pre-resolved UUID interchangeably.
//
// Goes through buildAuthedRequest so d.Apply (the OAuth-scope hook)
// sees this prefetch the same as a top-level dispatch — no scope
// bypass during role resolution.
//
// Errors:
//
//   - underlying handler returns 404 → "no agent role matches --role %q"
//     (clearer than the raw 404 body for agents).
//   - other non-2xx → wrapped error with body.
//   - response shape doesn't include id → error.
func (d *HTTPHandlerDispatcher) lookupRoleID(
	ctx context.Context,
	user *models.User,
	workspace string,
	role string,
) (string, error) {
	path := "/api/v1/workspaces/" + url.PathEscape(workspace) +
		"/agent-roles/" + url.PathEscape(role)
	req, err := d.buildAuthedRequest(ctx, http.MethodGet, path, nil, user)
	if err != nil {
		return "", fmt.Errorf("build agent-role request: %w", err)
	}
	rec := httptest.NewRecorder()
	d.Handler.ServeHTTP(rec, req)
	if rec.Code == http.StatusNotFound {
		return "", fmt.Errorf("no agent role matches --role %q", role)
	}
	if rec.Code >= 400 {
		body := strings.TrimSpace(rec.Body.String())
		if body == "" {
			body = http.StatusText(rec.Code)
		}
		return "", fmt.Errorf("look up agent role: %d %s", rec.Code, body)
	}

	var resp struct {
		ID string `json:"id"`
	}
	if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
		return "", fmt.Errorf("parse agent-role response: %w", err)
	}
	if resp.ID == "" {
		return "", fmt.Errorf("agent-role response missing id for %q", role)
	}
	return resp.ID, nil
}

// lookupAssigneeID issues an in-handler GET against
// /api/v1/workspaces/{ws}/members and returns the user_id whose
// name OR email matches `assign`. Case-insensitive on both fields.
//
// Errors:
//
//   - underlying handler returns non-2xx → wrapped error with body.
//   - response shape doesn't match expected {members:[...]} → error.
//   - no member matches → "no workspace member matches --assign %q".
func (d *HTTPHandlerDispatcher) lookupAssigneeID(
	ctx context.Context,
	user *models.User,
	workspace string,
	assign string,
) (string, error) {
	path := "/api/v1/workspaces/" + url.PathEscape(workspace) + "/members"
	// Goes through buildAuthedRequest so d.Apply (the OAuth-scope
	// hook) sees this prefetch the same as a top-level dispatch —
	// no scope bypass during assignee resolution.
	req, err := d.buildAuthedRequest(ctx, http.MethodGet, path, nil, user)
	if err != nil {
		return "", fmt.Errorf("build members request: %w", err)
	}
	rec := httptest.NewRecorder()
	d.Handler.ServeHTTP(rec, req)
	if rec.Code >= 400 {
		body := strings.TrimSpace(rec.Body.String())
		if body == "" {
			body = http.StatusText(rec.Code)
		}
		return "", fmt.Errorf("list workspace members: %d %s", rec.Code, body)
	}

	// Response shape: {"members":[{user_id, user_name, user_email, ...}, ...], "invitations":[...]}
	var resp struct {
		Members []struct {
			UserID    string `json:"user_id"`
			UserName  string `json:"user_name"`
			UserEmail string `json:"user_email"`
		} `json:"members"`
	}
	if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
		return "", fmt.Errorf("parse members response: %w", err)
	}

	for _, m := range resp.Members {
		if strings.EqualFold(m.UserName, assign) || strings.EqualFold(m.UserEmail, assign) {
			return m.UserID, nil
		}
	}
	return "", fmt.Errorf("no workspace member matches --assign %q", assign)
}

// dispatchItemUpdate handles `pad item update <ref>` with full CLI
// parity, including the read-modify-write merge of the fields JSON.
//
// The handler at handleUpdateItem treats input.Fields as a complete
// replacement (json_extract-friendly), but the CLI does a GET first
// to read existing fields, merges in new --status / --priority /
// --field overrides, then PATCHes the merged result. Without this
// dispatch path, an MCP `item.update --status done` would erase
// every other field the schema set — Codex caught the equivalent
// shape regression on item.create in PR #343.
//
// Sequence:
//
//  1. GET /api/v1/workspaces/{ws}/items/{ref} — read current state.
//  2. Merge: existing.fields + input.{status, priority, category,
//     parent} + parsed --field key=value pairs. Last-write-wins per
//     key (matches CLI; --field can override --status).
//  3. PATCH /api/v1/workspaces/{ws}/items/{ref} with the merged
//     payload.
//
// Returns the PATCH response packaged like any other dispatch result
// (structured JSON if 2xx + JSON body, IsError-flagged if non-2xx).
func (d *HTTPHandlerDispatcher) dispatchItemUpdate(
	ctx context.Context,
	input map[string]any,
	user *models.User,
) (*mcp.CallToolResult, error) {
	const cmdKey = "item update"

	workspace, _ := input["workspace"].(string)
	ref, _ := input["ref"].(string)
	if workspace == "" {
		return validationFailedResult(cmdKey, "workspace is required",
			"Pass `workspace=<slug>` or set a session default via pad_set_workspace."), nil
	}
	if ref == "" {
		return validationFailedResult(cmdKey, "ref is required",
			"Pass `ref=<TASK-N>` (or whichever item ref to update)."), nil
	}

	itemPath := "/api/v1/workspaces/" + url.PathEscape(workspace) +
		"/items/" + url.PathEscape(ref)

	// `--role` is now resolved at the dispatcher level (TASK-968):
	// Dispatch's preprocess step rewrites it to `agent_role_id`
	// before reaching this method, so by the time we get here a slug
	// has already been resolved to a UUID. The `--field
	// agent_role_id=<uuid>` workaround that the older rejection
	// pointed at still works (lifted via liftFieldsToColumns below)
	// and is preserved as the explicit-ID escape hatch when an agent
	// already knows the UUID and wants to skip the slug lookup.

	// Step 1: GET the existing item first so a not-found / permission
	// error surfaces cleanly (mirrors the CLI's "not found" UX) before we
	// attempt the PATCH. We NO LONGER read fields here for a client-side
	// read-modify-write merge — TASK-2022 moved field merging server-side
	// via `fields_patch` (see Step 2), which closes the lost-write race the
	// old GET-merge-PATCH suffered (IDEA-1480). The prefetch remains only
	// as an existence/permission pre-check.
	//
	// Goes through buildAuthedRequest so d.Apply (the OAuth-scope
	// hook) sees this prefetch the same as a top-level dispatch.
	prefetchReq, err := d.buildAuthedRequest(ctx, http.MethodGet, itemPath, nil, user)
	if err != nil {
		return dispatcherErrorResult(cmdKey, "build prefetch request", err), nil
	}
	prefetchRec := httptest.NewRecorder()
	d.Handler.ServeHTTP(prefetchRec, prefetchReq)
	if prefetchRec.Code >= 400 {
		// Mirror the CLI's "not found" UX — the handler's 404 body
		// already contains a clear message; package it the same way
		// any other tool error would be packaged. Pass d.Lister so
		// the workspace-not-found envelope's available_workspaces
		// list is filtered by the OAuth allow-list (TASK-977). Use
		// prefetchReq.Context() so the lister sees the same
		// auth/token state buildHTTPRequest + d.Apply attached
		// (Codex review #379 round 1 — same fix as executeRequest).
		return packageHTTPResponse(prefetchReq.Context(), cmdKey, prefetchRec.Result(), d.Lister)
	}

	// Step 2: Build the PATCH payload.
	payload := map[string]any{}
	// String-shaped fields: copy through when non-empty.
	for _, key := range []string{"title", "content", "comment"} {
		if v, ok := input[key].(string); ok && v != "" {
			payload[key] = v
		}
	}
	// `tags` is array<string> on the MCP schema as of BUG-1432; the
	// dispatcher forwards it verbatim (array, JSON-encoded string, or
	// CLI back-compat string) and lets ItemUpdate.UnmarshalJSON's
	// flex parser (BUG-1144) normalize. Pre-BUG-1432 this loop
	// filtered on `string` only, so a schema-conforming
	// `tags: ["a"]` was silently dropped — Codex review #547
	// round 1 [P1] caught that.
	//
	// Empty string is a no-op (matches the pre-fix behaviour for
	// non-tags string fields): ItemUpdate treats `tags: ""` as an
	// explicit empty-string write, which would corrupt the JSONB
	// column on Postgres (500) and the TEXT column on SQLite. Empty
	// array `[]` is intentionally NOT filtered — that's a legitimate
	// "clear all tags" update. Codex review #547 round 3 [P2].
	if v, ok := input["tags"]; ok && v != nil {
		if s, isString := v.(string); !isString || s != "" {
			payload["tags"] = v
		}
	}
	// Empty string is forwarded, NOT filtered — it means "clear this
	// assignment" (TASK-2571).
	//
	// The filter above it was right when it was written: `""` had no
	// defined meaning at the store, so passing it through bound an
	// empty string into a FK column and failed with a driver-specific
	// 500. BUG-2566 gave `""` clear-to-NULL semantics for exactly these
	// two columns, and the HTTP surface has inherited that since — so
	// filtering here now makes MCP the odd surface out: an agent has no
	// way to unassign an item, and `assigned_user_id=""` is a silent
	// no-op rather than either a clear or an error.
	//
	// This is a deliberate behaviour change on the MCP surface. Anyone
	// sending `""` today gets a no-op; they will now get a clear. That
	// is the correct reading of the input — nobody sends an empty
	// assignment ID meaning "leave it alone" — and the no-op is the
	// surprising half of the pair.
	//
	// NOTE the contrast with `tags` immediately above, whose empty-string
	// filter STAYS (codex #547 r3 P2): `tags: ""` is not a clear, it is a
	// corrupt write into a JSONB/TEXT column. Same-looking guard, opposite
	// justification — do not "unify" them.
	if v, ok := input["assigned_user_id"].(string); ok {
		payload["assigned_user_id"] = v
	}
	if v, ok := input["agent_role_id"].(string); ok {
		payload["agent_role_id"] = v
	}
	if b, ok := input["pinned"].(bool); ok {
		payload["pinned"] = b
	}
	// The canonical clear form (IDEA-2584). Forwarded verbatim, same shape as
	// `pinned` above — NOT guarded on the value being true.
	//
	// A `&& b` guard here would read as the thing protecting a client that
	// pads every declared param with its zero value, and it would be lying:
	// what actually makes `false` inert is the STORE, which clears only on
	// `input.ClearAssignedUser || <empty-string form>`. The guard would drop a
	// key that was already harmless. Pinned by
	// TestMCPUpdate_ClearFalseIsNotAClear, which fails if this ever forwards a
	// hardcoded true instead of the caller's value.
	//
	// Forwarded verbatim (see the note above); the CONFLICT CHECK lives after
	// the --field lift below, because that lift can put a competing
	// assigned_user_id into the payload after this point.
	if b, ok := input["clear_assigned_user"].(bool); ok {
		payload["clear_assigned_user"] = b
	}
	if b, ok := input["clear_agent_role"].(bool); ok {
		payload["clear_agent_role"] = 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
	}
	// TASK-2022: forward the optimistic-concurrency token so remote MCP
	// callers get the same 409 update_conflict guard the CLI/HTTP paths do.
	if v, ok := input["expected_updated_at"].(string); ok && v != "" {
		payload["expected_updated_at"] = v
	}

	// Field-level PATCH (TASK-2022). Send ONLY the changed keys as
	// `fields_patch`; the server shallow-merges them onto the item's current
	// fields inside the write transaction. This replaces the old client-side
	// GET-merge-PATCH of a full `fields` blob, which lost concurrent
	// single-field changes (the IDEA-1480 lost-write race). Named flags
	// (status / priority / category / parent) then --field entries.
	if hasFieldChanges(input) {
		patch := map[string]any{}
		for _, key := range []string{"status", "priority", "category", "parent"} {
			if v, ok := promotedParamValue(input[key]); ok {
				patch[key] = v
			}
		}
		if rawFields, ok := input["field"]; ok {
			extra, err := parseFieldKVP(rawFields)
			if err != nil {
				return validationFailedResult(cmdKey, "parse --field: "+err.Error(),
					"--field expects key=value entries (string array or single string)."), nil
			}
			for k, v := range extra {
				patch[k] = v
			}
		}
		// The `fields` object with its JSON types intact, applied LAST so it
		// wins over the stringified copy of itself in `field` — same reasoning
		// as mapItemCreate (BUG-2850). Without this an update through the
		// object param stringifies exactly as a create did.
		for k, v := range nativeFields(input) {
			patch[k] = v
		}
		// Lift recognized column keys (agent_role_id, assigned_user_id)
		// out of the patch onto the top-level payload so the handler writes
		// the column instead of stuffing the value inert in the JSON. Same
		// shape mapItemCreate uses; matches the workaround the --role
		// rejection points at.
		liftFieldsToColumns(patch, payload)
		// BUG-2078: clear_parent, checked here rather than alongside
		// clear_assigned_user/clear_agent_role below. Those two are top-level
		// ItemUpdate columns forwarded verbatim into `payload`; "parent" is a
		// fields_patch PSEUDO-key that only extractParentLink
		// (internal/server/handlers_items.go) understands — a present key
		// with an empty value means "clear", an absent key means "untouched".
		// So the clear signal has to be written into `patch`, and it must
		// happen AFTER the named-flag loop, the --field overlay, AND
		// liftFieldsToColumns above — every one of those can leave a
		// competing `patch["parent"]` behind, same reasoning as the
		// assigned-user conflict check below.
		//
		// Checked against BOTH "parent" and "plan" (codex round 1 [P1]):
		// extractParentLink resolves the link from EITHER key with no early
		// exit — `for _, key := range []string{"parent", "plan"}` — so a
		// competing value reaching the wire as `field: ["plan=<ref>"]`
		// bypassed a check that only looked at patch["parent"], and its
		// "plan" entry then overwrote this method's `patch["parent"] = ""`
		// server-side (the LATER key in that loop wins because neither
		// iteration exits early). Checking both closes the alias bypass.
		if clear, _ := input["clear_parent"].(bool); clear {
			// Refuse when the TARGET COLLECTION'S SCHEMA declares its own
			// "parent" or "plan" field (codex round 2 finding #2).
			// extractParentLink (internal/server/handlers_items.go ~L606-610,
			// pre-existing documented policy: "Skip this if the schema
			// actually defines a field with that key") skips hierarchy
			// handling ENTIRELY for a schema-shadowed key and instead lets it
			// fall through as an ordinary field write — so on a shadowed
			// collection, `patch["parent"] = ""` below would report success
			// while silently blanking the user's data field AND leaving the
			// real hierarchy link untouched. Reproduced empirically before
			// this guard existed: IsError=false, hierarchy link unchanged,
			// fields blob "parent" -> "".
			//
			// The wire shape {"parent":""} cannot distinguish clear-hierarchy
			// intent from a legitimate blank-my-schema-field write once it
			// reaches the server — the ambiguity is created HERE, at the
			// surface that accepted clear_parent, so this surface must refuse
			// rather than push the decision server-side.
			//
			// One extra GetCollection-equivalent call, paid ONLY on this path
			// (clear_parent=true) — the common update path fetches no schema
			// today and shouldn't start paying for one.
			shadowedKey, serr := d.collectionSchemaShadowsParent(ctx, user, workspace, prefetchRec)
			if serr != nil {
				return dispatcherErrorResult(cmdKey, "check collection schema for clear_parent", serr), nil
			}
			if shadowedKey != "" {
				return validationFailedResult(cmdKey,
					fmt.Sprintf("this collection defines its own %q field, so clear_parent can't be expressed for it", shadowedKey),
					""), nil
			}
			for _, key := range []string{"parent", "plan"} {
				if v, _ := patch[key].(string); v != "" {
					return validationFailedResult(cmdKey,
						fmt.Sprintf("clear_parent conflicts with setting a parent in the same update (via %q)", key),
						"Drop one: either clear the parent or set it, not both."), nil
				}
			}
			patch["parent"] = ""
		}
		// Only emit fields_patch when it still carries schema fields after
		// the column lift — otherwise a role-only update would send an empty
		// patch object (harmless, but avoids a needless fields write). A
		// clear_parent-only update deliberately bypasses this: patch["parent"]
		// = "" has len 1, so fields_patch is still emitted — that key IS the
		// payload for a bare clear.
		if len(patch) > 0 {
			payload["fields_patch"] = patch
		}
	}

	// A simultaneous set-and-clear is REJECTED, matching the CLI, so the two
	// transports can't disagree about a contradiction (codex round 1).
	//
	// Placed HERE — after --assign/--role resolution AND after
	// liftFieldsToColumns — because every one of those can put a competing
	// assigned_user_id into the payload, and the lift in particular runs
	// below the param handling. Checking earlier would have missed
	// `field: ["assigned_user_id=<uuid>"]` entirely; an earlier draft of this
	// code did exactly that and its comment claimed otherwise.
	//
	// Why reject rather than pick a winner: the store's branch order is
	// `if AssignedUserID != "" { set } else if ClearAssignedUser { clear }`,
	// so sending both silently makes the clear a no-op. Neither silent
	// outcome is defensible for a contradiction the caller typed.
	if clear, _ := payload["clear_assigned_user"].(bool); clear {
		if v, _ := payload["assigned_user_id"].(string); v != "" {
			return validationFailedResult(cmdKey,
				"clear_assigned_user conflicts with assigning a user in the same update",
				"Drop one: either clear the assignment or set it, not both."), nil
		}
	}
	if clear, _ := payload["clear_agent_role"].(bool); clear {
		if v, _ := payload["agent_role_id"].(string); v != "" {
			return validationFailedResult(cmdKey,
				"clear_agent_role conflicts with setting a role in the same update",
				"Drop one: either clear the role or set it, not both."), nil
		}
	}

	body, err := json.Marshal(payload)
	if err != nil {
		return dispatcherErrorResult(cmdKey, "encode body", err), nil
	}

	// Step 3: PATCH.
	return d.executeRequest(ctx, cmdKey, user, http.MethodPatch, itemPath, body)
}

// collectionSchemaShadowsParent reports whether the item's collection
// declares its own schema field named "parent" or "plan" — the case where
// extractParentLink (internal/server/handlers_items.go ~L606-610) treats
// that key as an ORDINARY data field instead of hierarchy, so a
// clear_parent request would silently blank the field and leave the real
// hierarchy link untouched (codex round 2 finding #2 / BUG-2078). Returns
// the shadowed key name ("parent" or "plan") when found, "" when the
// collection is clear to use clear_parent normally.
//
// prefetchItem is the Step-1 prefetch dispatchItemUpdate already ran (its
// body is the full item, including collection_slug) — reused here rather
// than re-fetched, so the ONLY new network call this adds is the
// collection lookup itself, and only on the clear_parent path.
func (d *HTTPHandlerDispatcher) collectionSchemaShadowsParent(
	ctx context.Context,
	user *models.User,
	workspace string,
	prefetchItem *httptest.ResponseRecorder,
) (string, error) {
	var item struct {
		CollectionSlug string `json:"collection_slug"`
	}
	if err := json.Unmarshal(prefetchItem.Body.Bytes(), &item); err != nil {
		return "", fmt.Errorf("parse prefetched item: %w", err)
	}
	if item.CollectionSlug == "" {
		return "", nil
	}

	path := "/api/v1/workspaces/" + url.PathEscape(workspace) +
		"/collections/" + url.PathEscape(item.CollectionSlug)
	req, err := d.buildAuthedRequest(ctx, http.MethodGet, path, nil, user)
	if err != nil {
		return "", fmt.Errorf("build collection request: %w", err)
	}
	rec := httptest.NewRecorder()
	d.Handler.ServeHTTP(rec, req)
	if rec.Code >= 400 {
		body := strings.TrimSpace(rec.Body.String())
		if body == "" {
			body = http.StatusText(rec.Code)
		}
		return "", fmt.Errorf("get collection %q: %d %s", item.CollectionSlug, rec.Code, body)
	}

	var coll struct {
		Schema string `json:"schema"`
	}
	if err := json.Unmarshal(rec.Body.Bytes(), &coll); err != nil {
		return "", fmt.Errorf("parse collection response: %w", err)
	}
	var schema models.CollectionSchema
	if coll.Schema != "" {
		// Schema-fetch/parse failure degrades to "not shadowed" rather than
		// erroring the whole update — mirrors the CLI's collSchema fetch,
		// which the same trade-off already accepts (cmd/pad/cmd_item.go: a
		// failed GetCollection there leaves collSchema at its zero value and
		// --field values just stay untyped strings instead of blocking the
		// update).
		_ = json.Unmarshal([]byte(coll.Schema), &schema)
	}
	for _, f := range schema.Fields {
		if f.Key == "parent" || f.Key == "plan" {
			return f.Key, nil
		}
	}
	return "", nil
}

// promotedParamValue reads one of the promoted item params (status, priority,
// category, parent) off the dispatch input, reporting whether it was actually
// supplied.
//
// A NON-STRING SCALAR COUNTS (codex round 6). Both call sites used to read
// `input[key].(string)`, which was right while these params could only arrive
// as CLI-shaped strings — but reshapeItemFields now promotes `fields:{…}`
// values with their JSON types intact, so `fields:{"priority":3}` arrives as a
// float64. The type assertion dropped it at BOTH sites: hasFieldChanges said
// there was nothing to change, so the dispatcher skipped the fields_patch
// branch entirely and answered SUCCESS having written nothing. A silent
// no-op — the exact failure mode BUG-2850 exists to remove — reintroduced by
// the fix for it, and asymmetric with create, where mapItemCreate has always
// passed non-strings through for the handler to validate.
//
// Empty string stays "not supplied", which is the invariant every other
// declared string param on this tool holds; a non-string is always supplied,
// since no zero value can be confused with absence. The server still
// validates the value against the collection schema — this decides whether it
// is SENT, not whether it is correct.
func promotedParamValue(raw any) (any, bool) {
	switch v := raw.(type) {
	case nil:
		return nil, false
	case string:
		return v, v != ""
	case bool, float64, int, int64, json.Number:
		return v, true
	default:
		// Structures never reach here: reshapeItemFields refuses a non-string
		// for the hierarchy keys and stringifies the rest, and a caller
		// sending one directly is refused by strict input validation.
		return nil, false
	}
}

// hasFieldChanges reports whether the input has any value that
// should trigger field-merging on update. Mirrors the CLI's check
// at cmd/pad/main.go itemUpdateCmd around the `hasFieldChanges`
// boolean — without this guard, dispatching `item update TASK-1
// --content "x"` would do an unnecessary GET-merge-PATCH of
// fields, churning the audit log entry for no reason.
func hasFieldChanges(input map[string]any) bool {
	// The native `fields` map counts as a change on its own (BUG-2850). A
	// NESTED-only update — fields:{"spec":[…]} — emits no `field` entry at
	// all, because a structure has no key=value encoding, so without this the
	// update reports success and writes nothing. That is the silent-drop shape
	// this bug is about, arriving through the fix for it.
	if len(nativeFields(input)) > 0 {
		return true
	}
	for _, key := range []string{"status", "priority", "category", "parent"} {
		if _, ok := promotedParamValue(input[key]); ok {
			return true
		}
	}
	if rawFields, ok := input["field"]; ok && rawFields != nil {
		switch x := rawFields.(type) {
		case string:
			return x != ""
		case []any:
			return len(x) > 0
		case []string:
			return len(x) > 0
		}
	}
	// BUG-2078: clear_parent alone (no other field touched) must still enter
	// the fields_patch-building branch below — it's the only way "parent" as
	// a present-but-empty key reaches the payload.
	if b, ok := input["clear_parent"].(bool); ok && b {
		return true
	}
	return false
}

// maybeInjectWorkspace defaults the `workspace` input from the
// dispatcher's WorkspaceLister when the caller didn't pass one
// explicitly (TASK-1076).
//
// The lister already encodes the right policy via the OAuth-token
// allow-list (internal/mcp/dispatch_http_lister.go):
//
//   - PAT auth (no allow-list)        → all of the user's workspaces
//   - Wildcard token (`["*"]`)        → all of the user's workspaces
//   - Specific allow-list             → intersection with the user's
//     memberships
//
// Inject ONLY when exactly one workspace results — that's the case
// where defaulting is unambiguous. Zero (no memberships, or
// allow-list disjoint from memberships) → leave alone; the route
// mapper's "missing required input" error is the agent's signal to
// pass workspace= explicitly. Multiple → also leave alone; agents
// should pick which workspace they mean rather than the dispatcher
// silently choosing one (the latter would be a real audience-confusion
// hazard for write operations).
//
// Caller-passed workspace ALWAYS wins (the early-return on the
// existing-value branch). Lister == nil paths (tests + non-OAuth
// transports) skip injection entirely so behavior stays unchanged
// for them — no Lister means no defaulting policy to apply.
//
// Mutations are applied to a copy; the caller's input map is not
// modified in place.
func (d *HTTPHandlerDispatcher) maybeInjectWorkspace(
	ctx context.Context,
	input map[string]any,
) map[string]any {
	if d.Lister == nil {
		return input
	}
	if existing, ok := input["workspace"].(string); ok && existing != "" {
		return input
	}
	workspaces, err := d.Lister.ListWorkspaces(ctx)
	if err != nil || len(workspaces) != 1 {
		return input
	}
	out := make(map[string]any, len(input)+1)
	for k, v := range input {
		out[k] = v
	}
	out["workspace"] = workspaces[0].Slug
	return out
}