feat(mcp): expand HTTPHandlerDispatcher route table — 10 new commands + routeSpec framework (TASK-966) (#344)

* feat(mcp): expand HTTPHandlerDispatcher route table — 10 new commands + declarative routeSpec framework (TASK-966)

TASK-965 shipped HTTPHandlerDispatcher with `item create` as the
proof-of-concept route. This expansion lays a small declarative
framework (`routeSpec` → `RouteMapper`) and wires the high-value
read + write surface, so an OAuth-authenticated agent connecting via
the future /mcp endpoint (TASK-950) gets a useful tool surface
out-of-the-box rather than 11/12 tools returning "not yet implemented
over HTTP transport."

## Framework

`routeSpec` (in dispatch_http_routes.go) is the declarative shape
shared across simple commands:

  routeSpec{
      method:       http.MethodGet,
      pathTemplate: "/api/v1/workspaces/{workspace}/items/{ref}",
      queryParams:  map[string]string{"q": "query", ...},  // dst→src
      bodyKeys:     []string{"title", "content", ...},
  }

`{placeholder}` segments substitute from the input map (snake_case
keys per TASK-964); `collection` and `target_collection` placeholders
are auto-normalized via `collections.NormalizeSlug`. queryParams maps
URL-query names to input keys and handles type coercion for ints
(json.Number / float64) and bool presence-only treatment. bodyKeys
emits a flat JSON body with empty values omitted.

Commands that don't fit the shape (item.create's fields-rolling,
item.move's nested overrides, item.list's path-varies-on-arg,
item.search's renamed q param, item.comment's message→body rename)
stay as standalone RouteMapper functions. The escape hatch is
deliberate — the simple cases get one-line entries; the weird cases
get full functions with their own test coverage.

## Commands wired (10 new + item.create)

| Cmd | Method | Path / Notes |
| --- | --- | --- |
| item create *(prior)* | POST | /workspaces/{ws}/collections/{coll}/items, fields-rolling |
| item show | GET | /workspaces/{ws}/items/{ref} |
| item delete | DELETE | /workspaces/{ws}/items/{ref} |
| item list | GET | path varies on collection arg; filters → query |
| item move | POST | /items/{ref}/move with target_collection + field_overrides body |
| item search | GET | /search?q=...&workspace=... (cross-workspace) |
| item comment | POST | /items/{ref}/comments, message→body, reply_to→parent_id |
| item comments | GET | /items/{ref}/comments |
| project dashboard | GET | /workspaces/{ws}/dashboard |
| collection list | GET | /workspaces/{ws}/collections |
| role list | GET | /workspaces/{ws}/agent-roles |

## Out of scope

`item update` requires read-modify-write semantics (the CLI fetches
the existing fields JSON, merges in --status / --priority / --field
entries, then PATCHes the merged result; the handler treats Fields
as a complete replacement). Implementing that here would mean making
two HTTP calls per dispatch and adding a new "prefetch" hook to the
framework — out of scope for this PR. Captured as the next follow-up.

`project next` / `project standup` / `project changelog` are CLI-side
compositions (multiple API calls + presentation logic) with no
single backing endpoint. Their HTTP equivalent for an agent is "call
project dashboard and read the suggested_next field." Documented in
the follow-up task.

The remaining ~40 commands (attachments, webhooks, library, github,
workspace audit-log, role create / delete, ...) are tracked in the
follow-up.

## Tests

- Framework unit tests: expandPath (substitution, normalization,
  escaping, error paths), buildQuery (rename, type coercion,
  json.Number support, empty-skip), flatJSONBody (omission rules).
- Per-command unit tests: every wired command has a happy-path
  assertion + at least one error path. Custom mappers
  (item.list / move / search / comment) get table-driven coverage of
  their renames + path-variation behaviour.
- Lock test: TestRouteTable_ContainsExpectedCommands fails loudly if
  an entry gets accidentally deleted.
- Integration smoke (TestHTTPHandlerDispatcher_Integration_ReadPaths)
  drives item create → list → show → project dashboard → collection
  list end-to-end against a real *server.Server, asserting the
  full chain stays wired together after the refactor.

Parent: PLAN-943.

* fix(mcp): item.list parity with CLI per Codex review (round 1)

Codex caught three CLI-parity bugs in mapItemList:

1. Default active-status filter missing. `pad item list` ships a
   broad inclusion list of active statuses unless --status or --all
   is set; the HTTP mapper returned no status filter, so MCP would
   leak done/completed/archived items by default.

2. `--parent <ref>` mapped to query param `parent_id`, which the
   server treats as a literal ID. The CLI uses `parent`, which
   parseItemListParams' unknown-key path routes to resolveParentFilter
   for ref → UUID resolution. Without this, `?parent=PLAN-3` would
   silently match nothing.

3. `--assign <name>` passed straight through as `assigned_user_id`.
   The CLI resolves names → user IDs via a workspace-members lookup
   first; passing the raw name to the store filter (which compares
   against `i.assigned_user_id` UUID) returns nothing.

Fixes:

1. Added `defaultActiveStatusFilter` constant mirroring the CLI's
   hardcoded list at cmd/pad/main.go itemListCmd. Applied when
   neither --status nor --all is set; --all drops it (so done items
   show); explicit --status wins (so the user can pin to any tier).

2. Renamed the query-param target from `parent_id` to `parent` so
   the handler's resolveParentFilter sees it as a field filter and
   does ref→UUID resolution.

3. Reject `--assign` with a clear error pointing agents at
   `--field assigned_user_id=<uuid>` for explicit-ID filtering. Same
   pattern as the existing rejection on item.create. Full name → ID
   prefetch belongs in the same follow-up that handles `--assign` on
   item create / update.

Tests:
- TestRoute_ItemList_AllItemsPath_AppliesDefaultActiveStatusFilter
  asserts the broad inclusion list is on the wire and verifies a
  spot-check of well-known active + terminal statuses.
- TestRoute_ItemList_AllFlagDropsDefaultStatus pins --all behaviour.
- TestRoute_ItemList_ExplicitStatusOverridesDefault pins explicit
  --status precedence.
- TestRoute_ItemList_FiltersAsQuery now asserts `parent` (not
  `parent_id`) is what reaches the wire.
- TestRoute_ItemList_RejectsAssignByName covers the rejection.
- TestRoute_ItemList_NumericLimitFromJSONNumber covers the
  json.Number path through the new numericInput helper.

Parent: PLAN-943.

* fix(mcp): normalize collection alias on item.search per Codex review (round 2)

Codex caught: `pad item search foo --collection task` was passing
"task" through verbatim to /api/v1/search?collection=task, but the
search store filters via `c.slug = ?` (exact match) and 0-matches
shorthand. The CLI normalizes to "tasks" first; mapper now does the
same.

Lifted the mutation into a tiny cloneStringMap helper so the input
map the caller hands us isn't accidentally rewritten — the registry
attaches the original via WithDispatchInput, and downstream code
shouldn't see a mapper's normalization leak back.

Tests:
- TestRoute_ItemSearch_NormalizesCollectionAlias asserts task → tasks
  on the wire.
- TestRoute_ItemSearch_DoesNotMutateInput pins the no-mutation
  contract so future refactors of the helper don't regress.

Parent: PLAN-943.

internal/mcp/dispatch_http.go

@@ -51,10 +51,20 @@ import (
 // handler chain runs (auth, audit, event-bus, webhooks, FTS index),
 // just without forking a subprocess.
 //
-// Scope (TASK-965): this PR ships the framework and one wired
-// command, `item create`, as the proof-of-concept. The remaining ~70
-// MCP-exposed commands are wired in a follow-up before TASK-950 ships
-// the /mcp endpoint to real users — see internal/mcp.routeTable.
+// Scope:
+//
+//   - TASK-965 shipped the framework + `item create` as the
+//     proof-of-concept.
+//   - TASK-966 (this expansion) wires the high-value reads + writes:
+//     item show / list / delete / move / search / comment / comments,
+//     project dashboard, collection list, role list. Commands with
+//     non-trivial shape (item.list's path-varies-on-arg, item.move's
+//     nested overrides) live as standalone RouteMapper functions in
+//     dispatch_http_routes.go; the rest use the declarative routeSpec.
+//
+// Tools the cmdhelp registry advertises but the route table doesn't
+// yet wire produce a clear "not yet implemented over HTTP transport"
+// error rather than failing silently — see Dispatch below.
 type HTTPHandlerDispatcher struct {
 	// Handler is the pad-cloud API router. *server.Server already
 	// satisfies http.Handler via its ServeHTTP method.
@@ -87,12 +97,13 @@ type HTTPHandlerDispatcher struct {
 type RouteMapper func(input map[string]any) (method, path string, body []byte, err error)
 
 // routeTable wires cmdPaths (joined with " ") to RouteMappers.
-// TASK-965 seeds the table with `item create` as the proof-of-concept;
-// follow-up tasks before TASK-950 fill in the rest of the MCP-exposed
-// command surface.
-var routeTable = map[string]RouteMapper{
-	"item create": mapItemCreate,
-}
+//
+// Populated by init() in dispatch_http_routes.go — that file owns the
+// declarative spec for every wired command. Commands not in the
+// table reach Dispatch only when an MCP client invokes them directly
+// and produce a clear "not yet implemented over HTTP transport"
+// error from Dispatch.
+var routeTable map[string]RouteMapper
 
 // Dispatch satisfies the Dispatcher interface. cliArgs are accepted
 // for interface compatibility but ignored — HTTPHandlerDispatcher

internal/mcp/dispatch_http_routes.go

@@ -0,0 +1,521 @@
+package mcp
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/url"
+	"strconv"
+	"strings"
+
+	"github.com/PerpetualSoftware/pad/internal/collections"
+)
+
+// routeSpec is the declarative description of a CLI→HTTP mapping.
+//
+// The framework supports the simple, common shape: substitute path
+// placeholders from input, optionally add query-string params, and
+// optionally pass selected input keys through as a flat JSON body.
+// Commands that don't fit the shape (item.create's fields-rolling,
+// item.move's nested overrides, item.list's path-varies-on-arg) live
+// as standalone RouteMapper functions instead.
+//
+// All input keys are MCP property names (snake_case per TASK-964).
+// `collection` and `target_collection` placeholders are normalized
+// via collections.NormalizeSlug so callers can pass shorthand
+// ("task" → "tasks") without 404s.
+type routeSpec struct {
+	// method is the HTTP method (GET / POST / PATCH / DELETE).
+	method string
+
+	// pathTemplate is a path with {key} placeholders. Each placeholder
+	// is required — missing values produce a clear dispatch-time error.
+	// The "/api/v1/" prefix is included literally.
+	pathTemplate string
+
+	// queryParams maps URL-query parameter names to input keys. For
+	// 1:1 names (input["status"] → ?status=...) just put {"status":"status"}.
+	// Renames work too: {"q":"query"} produces ?q=<input.query>.
+	// Empty/missing values are skipped — same behaviour the CLI gets
+	// from "only set --flag if it has a value."
+	queryParams map[string]string
+
+	// bodyKeys lists input keys that pass through into a flat JSON
+	// body. Empty-string values are omitted (matches the CLI's
+	// "only-when-set" semantic). For nested or transformed bodies use
+	// a standalone RouteMapper instead.
+	bodyKeys []string
+}
+
+// toRouteMapper compiles spec into a RouteMapper closure.
+func (s routeSpec) toRouteMapper() RouteMapper {
+	method := s.method
+	template := s.pathTemplate
+	queryParams := s.queryParams
+	bodyKeys := s.bodyKeys
+	return func(input map[string]any) (string, string, []byte, error) {
+		path, err := expandPath(template, input)
+		if err != nil {
+			return "", "", nil, err
+		}
+		if q := buildQuery(input, queryParams); q != "" {
+			path += "?" + q
+		}
+		var body []byte
+		if len(bodyKeys) > 0 {
+			body, err = flatJSONBody(input, bodyKeys)
+			if err != nil {
+				return "", "", nil, err
+			}
+		}
+		return method, path, body, nil
+	}
+}
+
+// expandPath substitutes {key} placeholders in template using input.
+// Each placeholder must appear as a non-empty string in input;
+// otherwise expandPath returns a clear error (so the dispatcher's
+// reply names the missing input rather than the agent receiving a
+// confusing 404 from the handler tree).
+//
+// The placeholders "collection" / "target_collection" are normalized
+// via collections.NormalizeSlug so shorthand forms like "task" work
+// the same way they do through the CLI.
+func expandPath(template string, input map[string]any) (string, error) {
+	var out strings.Builder
+	out.Grow(len(template))
+	for i := 0; i < len(template); {
+		if template[i] != '{' {
+			out.WriteByte(template[i])
+			i++
+			continue
+		}
+		end := strings.IndexByte(template[i:], '}')
+		if end < 0 {
+			return "", fmt.Errorf("unclosed placeholder in path template %q", template)
+		}
+		name := template[i+1 : i+end]
+		raw, ok := input[name]
+		if !ok || raw == nil {
+			return "", fmt.Errorf("missing required input %q for path placeholder", name)
+		}
+		s, ok := raw.(string)
+		if !ok {
+			return "", fmt.Errorf("input %q must be a string for path placeholder, got %T", name, raw)
+		}
+		if s == "" {
+			return "", fmt.Errorf("input %q must be non-empty for path placeholder", name)
+		}
+		if name == "collection" || name == "target_collection" {
+			s = collections.NormalizeSlug(s)
+		}
+		out.WriteString(url.PathEscape(s))
+		i += end + 1
+	}
+	return out.String(), nil
+}
+
+// buildQuery returns the URL-encoded query string for the mapping
+// (without the leading '?'). Empty mapping → empty string.
+//
+// Numbers from JSON arrive as float64; ints with no fractional part
+// emit without the .0. Booleans only emit when true (matching the
+// CLI's "presence-only" treatment).
+func buildQuery(input map[string]any, mapping map[string]string) string {
+	if len(mapping) == 0 {
+		return ""
+	}
+	q := url.Values{}
+	for dst, src := range mapping {
+		v, ok := input[src]
+		if !ok || v == nil {
+			continue
+		}
+		switch x := v.(type) {
+		case string:
+			if x != "" {
+				q.Set(dst, x)
+			}
+		case bool:
+			if x {
+				q.Set(dst, "true")
+			}
+		case float64:
+			// Cheap int detection — JSON parser gives every number as
+			// float64, but CLI flags in cmdhelp can be "int" type so
+			// most callers pass whole numbers. Emit without the
+			// trailing ".0" so the wire format matches the CLI.
+			if x == float64(int64(x)) {
+				q.Set(dst, strconv.FormatInt(int64(x), 10))
+			} else {
+				q.Set(dst, strconv.FormatFloat(x, 'f', -1, 64))
+			}
+		case json.Number:
+			q.Set(dst, x.String())
+		default:
+			q.Set(dst, fmt.Sprint(v))
+		}
+	}
+	if len(q) == 0 {
+		return ""
+	}
+	return q.Encode()
+}
+
+// flatJSONBody serializes selected input keys into a JSON object.
+// Empty-string values are skipped; nil values are skipped.
+//
+// For more complex shapes (nested objects, key renames, custom field
+// rolling) a standalone RouteMapper is the better fit — see
+// mapItemCreate / mapItemMove for examples.
+func flatJSONBody(input map[string]any, keys []string) ([]byte, error) {
+	body := map[string]any{}
+	for _, k := range keys {
+		v, ok := input[k]
+		if !ok || v == nil {
+			continue
+		}
+		if s, ok := v.(string); ok && s == "" {
+			continue
+		}
+		body[k] = v
+	}
+	return json.Marshal(body)
+}
+
+// initRouteTable replaces the seed routeTable from TASK-965 with the
+// expanded TASK-966 set: framework-driven routeSpecs for the simple
+// commands plus standalone RouteMappers for the few that have
+// non-trivial shape.
+//
+// Every entry here corresponds to a leaf command in the cmdhelp
+// document that survives DefaultExcludes filtering. Commands not in
+// the table reach Dispatch only when an MCP client invokes them
+// directly (the registry advertises the full surface) — those
+// produce a clear "not yet implemented over HTTP transport" error.
+func init() {
+	routeTable = map[string]RouteMapper{
+		// --- Item CRUD-ish ---
+		"item create": mapItemCreate,
+		"item show": routeSpec{
+			method:       http.MethodGet,
+			pathTemplate: "/api/v1/workspaces/{workspace}/items/{ref}",
+		}.toRouteMapper(),
+		"item delete": routeSpec{
+			method:       http.MethodDelete,
+			pathTemplate: "/api/v1/workspaces/{workspace}/items/{ref}",
+		}.toRouteMapper(),
+		"item list":   mapItemList,
+		"item move":   mapItemMove,
+		"item search": mapItemSearch,
+
+		// --- Comments ---
+		"item comment": mapItemComment,
+		"item comments": routeSpec{
+			method:       http.MethodGet,
+			pathTemplate: "/api/v1/workspaces/{workspace}/items/{ref}/comments",
+		}.toRouteMapper(),
+
+		// --- Read-only workspace surfaces ---
+		"project dashboard": routeSpec{
+			method:       http.MethodGet,
+			pathTemplate: "/api/v1/workspaces/{workspace}/dashboard",
+		}.toRouteMapper(),
+		"collection list": routeSpec{
+			method:       http.MethodGet,
+			pathTemplate: "/api/v1/workspaces/{workspace}/collections",
+		}.toRouteMapper(),
+		"role list": routeSpec{
+			method:       http.MethodGet,
+			pathTemplate: "/api/v1/workspaces/{workspace}/agent-roles",
+		}.toRouteMapper(),
+	}
+}
+
+// defaultActiveStatusFilter mirrors the broad inclusion list the
+// CLI sets when neither --status nor --all is provided. Hides
+// terminal statuses (done / completed / archived / etc.) by default
+// without making the dispatcher have to fetch+filter, which would be
+// a behaviour divergence from `pad item list` if we simply omitted
+// the filter (Codex review on PR #344, finding 1).
+//
+// Kept as a constant — pad's status vocabulary is template-driven
+// and changes rarely; the CLI's literal list at cmd/pad/main.go
+// itemListCmd is the source of truth, mirrored here.
+const defaultActiveStatusFilter = "open,in_progress,in-progress,active,draft,raw,exploring,decided,new,triaged,fixing,planned,published,paused,proposed,researching,building,ready,in_sprint,reviewed,planning"
+
+// mapItemList dispatches `pad item list [collection] [filters...]`.
+//
+// The path varies on whether `collection` was supplied:
+//
+//   - With collection:   GET /api/v1/workspaces/{ws}/collections/{coll}/items
+//   - Without:           GET /api/v1/workspaces/{ws}/items
+//
+// Filter parity with the CLI:
+//
+//   - `--status X` → `?status=X` directly.
+//   - Neither `--status` nor `--all` → broad active-status filter
+//     (matches the CLI's hardcoded list so done items don't leak by
+//     default — see defaultActiveStatusFilter).
+//   - `--all` → `?include_archived=true`, and the default-status
+//     filter is dropped so all statuses pass.
+//   - `--parent <ref>` → `?parent=<ref>`. The handler's
+//     resolveParentFilter resolves the ref via the field-filter path.
+//     (Going via `parent_id` would skip ref-resolution and fail for
+//     human-friendly inputs like "PLAN-3"; Codex review caught this.)
+//   - `--role <slug>` → `?agent_role_id=<slug>`. The store accepts
+//     both ID and slug here.
+//   - `--assign <name>` → rejected. The CLI resolves name→ID
+//     server-side via a workspace-members lookup; replicating that
+//     prefetch in the dispatcher belongs in the same follow-up that
+//     handles `assign` on item.create / update. Pass
+//     `--field assigned_user_id=<uuid>` for explicit-ID filtering.
+//   - `--field key=value` (repeatable) → flat query params, picked
+//     up by parseItemListParams' unknown-key → field-filter path.
+func mapItemList(input map[string]any) (string, string, []byte, error) {
+	workspace, _ := input["workspace"].(string)
+	if workspace == "" {
+		return "", "", nil, fmt.Errorf("workspace is required")
+	}
+
+	if v, ok := input["assign"]; ok {
+		if s, ok := v.(string); ok && s != "" {
+			return "", "", nil, fmt.Errorf(
+				"--assign %q is not yet supported by HTTPHandlerDispatcher; "+
+					"the CLI resolves names → user IDs via workspace-members "+
+					"lookup, which we'll add in a follow-up. For now, pass "+
+					"`--field assigned_user_id=<uuid>` for explicit-ID filtering.",
+				s,
+			)
+		}
+	}
+
+	pathBase := "/api/v1/workspaces/" + url.PathEscape(workspace) + "/items"
+	if coll, _ := input["collection"].(string); coll != "" {
+		pathBase = "/api/v1/workspaces/" + url.PathEscape(workspace) +
+			"/collections/" + url.PathEscape(collections.NormalizeSlug(coll)) + "/items"
+	}
+
+	values := url.Values{}
+	add := func(name, value string) {
+		if value != "" {
+			values.Set(name, value)
+		}
+	}
+
+	// Pass-through string filters.
+	if s, _ := input["status"].(string); s != "" {
+		add("status", s)
+	} else if b, _ := input["all"].(bool); !b {
+		// CLI parity: hide terminal statuses by default. --all overrides.
+		add("status", defaultActiveStatusFilter)
+	}
+	if s, _ := input["priority"].(string); s != "" {
+		add("priority", s)
+	}
+	if s, _ := input["sort"].(string); s != "" {
+		add("sort", s)
+	}
+	if s, _ := input["group_by"].(string); s != "" {
+		add("group_by", s)
+	}
+	if s, _ := input["search"].(string); s != "" {
+		add("search", s)
+	}
+	if s, _ := input["tag"].(string); s != "" {
+		add("tag", s)
+	}
+	// Parent filter goes via the unknown-key field-filter path so
+	// resolveParentFilter handles ref→UUID resolution server-side.
+	if s, _ := input["parent"].(string); s != "" {
+		add("parent", s)
+	}
+	if s, _ := input["role"].(string); s != "" {
+		add("agent_role_id", s)
+	}
+
+	// Numeric filters.
+	if n, ok := numericInput(input["limit"]); ok && n > 0 {
+		values.Set("limit", strconv.FormatInt(n, 10))
+	}
+	if n, ok := numericInput(input["offset"]); ok && n > 0 {
+		values.Set("offset", strconv.FormatInt(n, 10))
+	}
+
+	if b, _ := input["all"].(bool); b {
+		values.Set("include_archived", "true")
+	}
+
+	// Repeatable --field key=value pairs become arbitrary query params
+	// (parseItemListParams treats unknown keys as field filters).
+	if rawFields, ok := input["field"]; ok {
+		extra, err := parseFieldKVP(rawFields)
+		if err != nil {
+			return "", "", nil, fmt.Errorf("parse --field: %w", err)
+		}
+		for k, v := range extra {
+			values.Set(k, fmt.Sprint(v))
+		}
+	}
+
+	if encoded := values.Encode(); encoded != "" {
+		pathBase += "?" + encoded
+	}
+	return http.MethodGet, pathBase, nil, nil
+}
+
+// numericInput pulls an int64 out of a JSON-typed input value. JSON
+// decoders deliver numbers as float64 (or json.Number when
+// UseNumber()); we accept both. Returns (0, false) for nil or
+// non-numeric inputs so callers can short-circuit.
+func numericInput(v any) (int64, bool) {
+	switch x := v.(type) {
+	case nil:
+		return 0, false
+	case float64:
+		return int64(x), true
+	case int:
+		return int64(x), true
+	case int64:
+		return x, true
+	case json.Number:
+		n, err := x.Int64()
+		if err != nil {
+			return 0, false
+		}
+		return n, true
+	}
+	return 0, false
+}
+
+// mapItemMove dispatches `pad item move <ref> <target-collection>`.
+//
+// POST /api/v1/workspaces/{ws}/items/{ref}/move with body shape
+// {target_collection: "...", field_overrides: {key: val, ...}, source: "cli"}
+// — same shape the CLI builds in cmd/pad/main.go's moveItemCmd.
+func mapItemMove(input map[string]any) (string, string, []byte, error) {
+	workspace, _ := input["workspace"].(string)
+	ref, _ := input["ref"].(string)
+	target, _ := input["target_collection"].(string)
+	if workspace == "" {
+		return "", "", nil, fmt.Errorf("workspace is required")
+	}
+	if ref == "" {
+		return "", "", nil, fmt.Errorf("ref is required")
+	}
+	if target == "" {
+		return "", "", nil, fmt.Errorf("target_collection is required")
+	}
+
+	payload := map[string]any{
+		"target_collection": collections.NormalizeSlug(target),
+		"actor":             "user",
+		"source":            "cli",
+	}
+	if rawFields, ok := input["field"]; ok {
+		extra, err := parseFieldKVP(rawFields)
+		if err != nil {
+			return "", "", nil, fmt.Errorf("parse --field: %w", err)
+		}
+		if len(extra) > 0 {
+			payload["field_overrides"] = extra
+		}
+	}
+
+	body, err := json.Marshal(payload)
+	if err != nil {
+		return "", "", nil, fmt.Errorf("encode body: %w", err)
+	}
+	urlPath := fmt.Sprintf("/api/v1/workspaces/%s/items/%s/move",
+		url.PathEscape(workspace), url.PathEscape(ref))
+	return http.MethodPost, urlPath, body, nil
+}
+
+// mapItemSearch dispatches `pad item search <query>`.
+//
+// GET /api/v1/search?q=...&workspace=...&[filters]. Workspace lives
+// in the query string here (not the path) — the search handler is
+// cross-workspace by design.
+//
+// `collection` is normalized via collections.NormalizeSlug before
+// going on the wire. The search store filters with `c.slug = ?` and
+// would 0-match shorthand inputs ("task" instead of "tasks") without
+// this — Codex review #344 round 2 finding.
+func mapItemSearch(input map[string]any) (string, string, []byte, error) {
+	query, _ := input["query"].(string)
+	if query == "" {
+		return "", "", nil, fmt.Errorf("query is required")
+	}
+	// Normalize the collection input in-place before buildQuery reads
+	// it. We only mutate the local map so the caller's input isn't
+	// affected — but BuildCLIArgs builds a fresh map per call so this
+	// is also safe in production.
+	if coll, ok := input["collection"].(string); ok && coll != "" {
+		input = cloneStringMap(input)
+		input["collection"] = collections.NormalizeSlug(coll)
+	}
+	q := buildQuery(input, map[string]string{
+		"q":          "query",
+		"workspace":  "workspace",
+		"collection": "collection",
+		"status":     "status",
+		"priority":   "priority",
+		"sort":       "sort",
+		"limit":      "limit",
+		"offset":     "offset",
+	})
+	urlPath := "/api/v1/search"
+	if q != "" {
+		urlPath += "?" + q
+	}
+	return http.MethodGet, urlPath, nil, nil
+}
+
+// cloneStringMap returns a shallow copy of m. Used by mappers that
+// need to normalize a single value before handing the map to a
+// downstream helper, without mutating the caller's reference.
+func cloneStringMap(m map[string]any) map[string]any {
+	out := make(map[string]any, len(m))
+	for k, v := range m {
+		out[k] = v
+	}
+	return out
+}
+
+// mapItemComment dispatches `pad item comment <ref> <message>`.
+//
+// POST /api/v1/workspaces/{ws}/items/{ref}/comments with body shape
+// {body: <message>, parent_id: <reply_to>, source: "cli"} — the
+// handler expects `body` (matching models.CommentCreate), not
+// `message`. Custom mapper because of the rename.
+func mapItemComment(input map[string]any) (string, string, []byte, error) {
+	workspace, _ := input["workspace"].(string)
+	ref, _ := input["ref"].(string)
+	message, _ := input["message"].(string)
+	if workspace == "" {
+		return "", "", nil, fmt.Errorf("workspace is required")
+	}
+	if ref == "" {
+		return "", "", nil, fmt.Errorf("ref is required")
+	}
+	if message == "" {
+		return "", "", nil, fmt.Errorf("message is required")
+	}
+	payload := map[string]any{
+		"body":   message,
+		"source": "cli",
+	}
+	// MCP property name for `--reply-to` is `reply_to` after TASK-964.
+	if v, ok := input["reply_to"]; ok {
+		if s, ok := v.(string); ok && s != "" {
+			payload["parent_id"] = s
+		}
+	}
+	body, err := json.Marshal(payload)
+	if err != nil {
+		return "", "", nil, fmt.Errorf("encode body: %w", err)
+	}
+	urlPath := fmt.Sprintf("/api/v1/workspaces/%s/items/%s/comments",
+		url.PathEscape(workspace), url.PathEscape(ref))
+	return http.MethodPost, urlPath, body, nil
+}

internal/mcp/dispatch_http_routes_test.go

@@ -0,0 +1,716 @@
+package mcp
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"reflect"
+	"sort"
+	"strings"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+	"github.com/PerpetualSoftware/pad/internal/server"
+)
+
+// parseQuery is a thin wrapper around url.ParseQuery that returns the
+// values directly. Convenience for the route tests where the function
+// signature is the bottleneck.
+func parseQuery(s string) (url.Values, error) {
+	return url.ParseQuery(s)
+}
+
+// mustParseQueryFromPath extracts the query-string portion of path
+// and parses it. Test-only — fails the test on malformed input.
+func mustParseQueryFromPath(t *testing.T, path string) url.Values {
+	t.Helper()
+	idx := strings.IndexByte(path, '?')
+	if idx < 0 {
+		t.Fatalf("path %q has no query string", path)
+	}
+	values, err := url.ParseQuery(path[idx+1:])
+	if err != nil {
+		t.Fatalf("parse query in %q: %v", path, err)
+	}
+	return values
+}
+
+// doJSONReq drives a JSON request through srv and returns the
+// recorder. Mirrors the local pattern in
+// internal/server/server_test.go::doRequest, kept package-local so
+// internal/mcp's tests don't depend on test-only code from another
+// package.
+func doJSONReq(t *testing.T, srv *server.Server, method, path string, body any) *httptest.ResponseRecorder {
+	t.Helper()
+	req := mustJSONRequest(t, method, path, body)
+	rec := httptest.NewRecorder()
+	srv.ServeHTTP(rec, req)
+	return rec
+}
+
+// --- Framework helpers ---
+
+func TestExpandPath_BasicSubstitution(t *testing.T) {
+	got, err := expandPath(
+		"/api/v1/workspaces/{workspace}/items/{ref}",
+		map[string]any{"workspace": "docapp", "ref": "TASK-5"},
+	)
+	if err != nil {
+		t.Fatalf("expandPath: %v", err)
+	}
+	want := "/api/v1/workspaces/docapp/items/TASK-5"
+	if got != want {
+		t.Errorf("got %q, want %q", got, want)
+	}
+}
+
+func TestExpandPath_NormalizesCollection(t *testing.T) {
+	got, err := expandPath(
+		"/api/v1/workspaces/{workspace}/collections/{collection}/items",
+		map[string]any{"workspace": "docapp", "collection": "task"},
+	)
+	if err != nil {
+		t.Fatalf("expandPath: %v", err)
+	}
+	if !strings.Contains(got, "/collections/tasks/") {
+		t.Errorf("expected `task` to normalize to `tasks`, got %q", got)
+	}
+}
+
+func TestExpandPath_PathEscapesSpecials(t *testing.T) {
+	got, err := expandPath(
+		"/api/v1/workspaces/{workspace}/items/{ref}",
+		map[string]any{"workspace": "ws/with/slashes", "ref": "weird ref"},
+	)
+	if err != nil {
+		t.Fatalf("expandPath: %v", err)
+	}
+	if strings.Contains(got, " ") {
+		t.Errorf("space not escaped in path: %q", got)
+	}
+	if strings.Count(got, "/") != strings.Count("/api/v1/workspaces//items/", "/") {
+		// 4 literal slashes — the escaped workspace must NOT introduce more.
+		// (`/` in the value should be percent-encoded.)
+		t.Errorf("workspace slashes leaked: %q", got)
+	}
+}
+
+func TestExpandPath_MissingPlaceholderErrors(t *testing.T) {
+	_, err := expandPath("/items/{ref}", map[string]any{})
+	if err == nil {
+		t.Errorf("expected error for missing placeholder")
+	}
+	if !strings.Contains(err.Error(), "ref") {
+		t.Errorf("error should mention missing key %q; got %v", "ref", err)
+	}
+}
+
+func TestExpandPath_NonStringPlaceholderErrors(t *testing.T) {
+	_, err := expandPath("/items/{ref}", map[string]any{"ref": 42})
+	if err == nil {
+		t.Errorf("expected error for non-string placeholder value")
+	}
+}
+
+func TestExpandPath_EmptyPlaceholderErrors(t *testing.T) {
+	_, err := expandPath("/items/{ref}", map[string]any{"ref": ""})
+	if err == nil {
+		t.Errorf("expected error for empty placeholder value")
+	}
+}
+
+func TestExpandPath_UnclosedPlaceholderErrors(t *testing.T) {
+	_, err := expandPath("/items/{ref", map[string]any{"ref": "x"})
+	if err == nil {
+		t.Errorf("expected error for unclosed placeholder")
+	}
+}
+
+func TestBuildQuery_SkipsEmptyAndMissing(t *testing.T) {
+	got := buildQuery(
+		map[string]any{"a": "x", "b": "", "c": nil, "d": "y"},
+		map[string]string{"a": "a", "b": "b", "c": "c", "d": "d", "e": "e"},
+	)
+	values, err := parseQuery(got)
+	if err != nil {
+		t.Fatalf("parse query %q: %v", got, err)
+	}
+	if values.Get("a") != "x" || values.Get("d") != "y" {
+		t.Errorf("expected a=x and d=y; got %v", values)
+	}
+	for _, gone := range []string{"b", "c", "e"} {
+		if values.Has(gone) {
+			t.Errorf("expected %q to be skipped; got %v", gone, values)
+		}
+	}
+}
+
+func TestBuildQuery_RenamesAndTypes(t *testing.T) {
+	got := buildQuery(
+		map[string]any{"query": "OAuth", "limit": float64(50), "all": true, "skip": false},
+		map[string]string{"q": "query", "limit": "limit", "include_archived": "all", "_skip": "skip"},
+	)
+	values, err := parseQuery(got)
+	if err != nil {
+		t.Fatalf("parse query: %v", err)
+	}
+	if values.Get("q") != "OAuth" {
+		t.Errorf("rename q→query lost: %v", values)
+	}
+	if values.Get("limit") != "50" {
+		t.Errorf("expected limit=50 (int form), got %q", values.Get("limit"))
+	}
+	if values.Get("include_archived") != "true" {
+		t.Errorf("expected include_archived=true, got %q", values.Get("include_archived"))
+	}
+	if values.Has("_skip") {
+		t.Errorf("false bool should be skipped; got %v", values)
+	}
+}
+
+func TestBuildQuery_HandlesJSONNumber(t *testing.T) {
+	// JSON decoders configured with UseNumber() produce json.Number.
+	// Make sure the framework handles that without losing precision.
+	dec := json.NewDecoder(strings.NewReader(`{"limit":50}`))
+	dec.UseNumber()
+	var input map[string]any
+	if err := dec.Decode(&input); err != nil {
+		t.Fatalf("decode: %v", err)
+	}
+	got := buildQuery(input, map[string]string{"limit": "limit"})
+	if got != "limit=50" {
+		t.Errorf("got %q, want limit=50", got)
+	}
+}
+
+func TestFlatJSONBody_OmitsEmptyAndNil(t *testing.T) {
+	body, err := flatJSONBody(
+		map[string]any{"a": "x", "b": "", "c": nil, "d": 42, "ignored": "z"},
+		[]string{"a", "b", "c", "d", "missing"},
+	)
+	if err != nil {
+		t.Fatalf("flatJSONBody: %v", err)
+	}
+	var got map[string]any
+	if err := json.Unmarshal(body, &got); err != nil {
+		t.Fatalf("unmarshal: %v\n%s", err, body)
+	}
+	want := map[string]any{"a": "x", "d": float64(42)}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+// --- Per-command mapper tests ---
+
+func TestRoute_ItemShow(t *testing.T) {
+	m, p, body, err := routeTable["item show"](map[string]any{
+		"workspace": "docapp", "ref": "TASK-5",
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if m != http.MethodGet {
+		t.Errorf("method = %q", m)
+	}
+	if p != "/api/v1/workspaces/docapp/items/TASK-5" {
+		t.Errorf("path = %q", p)
+	}
+	if body != nil {
+		t.Errorf("expected nil body for GET; got %s", body)
+	}
+}
+
+func TestRoute_ItemDelete(t *testing.T) {
+	m, p, _, err := routeTable["item delete"](map[string]any{
+		"workspace": "docapp", "ref": "TASK-5",
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if m != http.MethodDelete {
+		t.Errorf("method = %q", m)
+	}
+	if p != "/api/v1/workspaces/docapp/items/TASK-5" {
+		t.Errorf("path = %q", p)
+	}
+}
+
+func TestRoute_ItemList_AllItemsPath_AppliesDefaultActiveStatusFilter(t *testing.T) {
+	// CLI parity: bare `pad item list` hides terminal statuses by
+	// default. The HTTP mapper must apply the same broad inclusion
+	// list — Codex caught a regression where it returned no status
+	// filter and would have leaked done items to agents.
+	m, p, _, err := routeTable["item list"](map[string]any{"workspace": "docapp"})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if m != http.MethodGet {
+		t.Errorf("method = %q", m)
+	}
+	if !strings.HasPrefix(p, "/api/v1/workspaces/docapp/items?") {
+		t.Errorf("expected cross-collection path with query, got %q", p)
+	}
+	values := mustParseQueryFromPath(t, p)
+	got := values.Get("status")
+	if got == "" {
+		t.Errorf("default status filter missing; got query %v", values)
+	}
+	// Spot-check a few well-known active statuses.
+	for _, want := range []string{"open", "in_progress", "draft", "exploring"} {
+		if !strings.Contains(got, want) {
+			t.Errorf("default status filter missing %q; got %q", want, got)
+		}
+	}
+	// And confirm terminal statuses are NOT included.
+	for _, blocked := range []string{"done", "completed", "archived"} {
+		if strings.Contains(got, blocked) {
+			t.Errorf("default status filter incorrectly includes %q; got %q", blocked, got)
+		}
+	}
+}
+
+func TestRoute_ItemList_AllFlagDropsDefaultStatus(t *testing.T) {
+	// `--all` overrides the active-status default. Both
+	// include_archived=true must be set AND the default status
+	// inclusion list must NOT be present (otherwise --all wouldn't
+	// actually let through done items).
+	_, p, _, err := routeTable["item list"](map[string]any{
+		"workspace": "docapp", "all": true,
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	values := mustParseQueryFromPath(t, p)
+	if values.Get("include_archived") != "true" {
+		t.Errorf("include_archived not set under --all; got %v", values)
+	}
+	if values.Has("status") {
+		t.Errorf("status filter must be dropped under --all; got %v", values)
+	}
+}
+
+func TestRoute_ItemList_ExplicitStatusOverridesDefault(t *testing.T) {
+	// An explicit --status pin replaces the active-status default —
+	// not appended to it.
+	_, p, _, err := routeTable["item list"](map[string]any{
+		"workspace": "docapp", "status": "done",
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	values := mustParseQueryFromPath(t, p)
+	if values.Get("status") != "done" {
+		t.Errorf("explicit --status not honored; got %q", values.Get("status"))
+	}
+}
+
+func TestRoute_ItemList_CollectionScopedPath(t *testing.T) {
+	_, p, _, err := routeTable["item list"](map[string]any{
+		"workspace": "docapp", "collection": "task", // shorthand
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	// Path may carry the default-active-status query string from the
+	// no-explicit-status branch — we only assert the path prefix here.
+	if !strings.HasPrefix(p, "/api/v1/workspaces/docapp/collections/tasks/items") {
+		t.Errorf("expected collection-scoped + normalized path, got %q", p)
+	}
+}
+
+func TestRoute_ItemList_FiltersAsQuery(t *testing.T) {
+	_, p, _, err := routeTable["item list"](map[string]any{
+		"workspace": "docapp",
+		"status":    "open",
+		"priority":  "high",
+		"limit":     float64(20),
+		"all":       true,
+		"parent":    "PLAN-3",
+		"role":      "implementer",
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	values := mustParseQueryFromPath(t, p)
+	for k, want := range map[string]string{
+		"status":           "open",
+		"priority":         "high",
+		"limit":            "20",
+		"include_archived": "true",
+		// parent goes through the field-filter path (parseItemListParams
+		// treats unknown keys as fields → resolveParentFilter resolves
+		// "PLAN-3" → UUID). Sending parent_id would skip ref-resolution
+		// and break refs (Codex review #344 finding 2).
+		"parent":        "PLAN-3",
+		"agent_role_id": "implementer",
+	} {
+		if got := values.Get(k); got != want {
+			t.Errorf("query %q = %q, want %q (full path: %s)", k, got, want, p)
+		}
+	}
+	if values.Has("parent_id") {
+		t.Errorf("parent_id incorrectly sent; should use parent (full path: %s)", p)
+	}
+}
+
+func TestRoute_ItemList_RejectsAssignByName(t *testing.T) {
+	// CLI parity: --assign Dave resolves name→UUID via workspace
+	// members lookup. Replicating that prefetch in the dispatcher
+	// belongs in the same follow-up that handles --assign on
+	// item.create / update. For now, reject loudly so agents don't
+	// silently get empty results (Codex review #344 finding 3).
+	_, _, _, err := routeTable["item list"](map[string]any{
+		"workspace": "docapp", "assign": "Dave",
+	})
+	if err == nil {
+		t.Errorf("expected error for --assign by name; got nil")
+		return
+	}
+	if !strings.Contains(err.Error(), "assigned_user_id") {
+		t.Errorf("error should point users at the explicit-ID alternative; got %v", err)
+	}
+}
+
+func TestRoute_ItemList_NumericLimitFromJSONNumber(t *testing.T) {
+	// JSON decoders configured with UseNumber() produce json.Number
+	// instead of float64. The mapper's numeric helper must handle both.
+	dec := json.NewDecoder(strings.NewReader(`{"workspace":"docapp","limit":50}`))
+	dec.UseNumber()
+	var input map[string]any
+	if err := dec.Decode(&input); err != nil {
+		t.Fatalf("decode: %v", err)
+	}
+	_, p, _, err := routeTable["item list"](input)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	values := mustParseQueryFromPath(t, p)
+	if values.Get("limit") != "50" {
+		t.Errorf("limit lost from json.Number input; got %q (path %q)", values.Get("limit"), p)
+	}
+}
+
+func TestRoute_ItemList_FieldKVPLandsAsQueryParam(t *testing.T) {
+	_, p, _, err := routeTable["item list"](map[string]any{
+		"workspace": "docapp",
+		"field":     []any{"trigger=on-implement", "scope=all"},
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	values := mustParseQueryFromPath(t, p)
+	if values.Get("trigger") != "on-implement" {
+		t.Errorf("--field trigger lost: %v", values)
+	}
+	if values.Get("scope") != "all" {
+		t.Errorf("--field scope lost: %v", values)
+	}
+}
+
+func TestRoute_ItemMove(t *testing.T) {
+	m, p, body, err := routeTable["item move"](map[string]any{
+		"workspace":         "docapp",
+		"ref":               "BUG-3",
+		"target_collection": "task", // shorthand
+		"field":             []any{"priority=high"},
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if m != http.MethodPost {
+		t.Errorf("method = %q", m)
+	}
+	if p != "/api/v1/workspaces/docapp/items/BUG-3/move" {
+		t.Errorf("path = %q", p)
+	}
+	var payload map[string]any
+	if err := json.Unmarshal(body, &payload); err != nil {
+		t.Fatalf("decode body: %v\n%s", err, body)
+	}
+	if payload["target_collection"] != "tasks" {
+		t.Errorf("target_collection not normalized: %v", payload)
+	}
+	if payload["source"] != "cli" {
+		t.Errorf("source not stamped: %v", payload)
+	}
+	overrides, ok := payload["field_overrides"].(map[string]any)
+	if !ok || overrides["priority"] != "high" {
+		t.Errorf("field_overrides missing priority=high: %v", payload)
+	}
+}
+
+func TestRoute_ItemMove_RequiresAllThree(t *testing.T) {
+	for _, missing := range []string{"workspace", "ref", "target_collection"} {
+		t.Run("missing-"+missing, func(t *testing.T) {
+			input := map[string]any{
+				"workspace": "ws", "ref": "TASK-1", "target_collection": "tasks",
+			}
+			delete(input, missing)
+			_, _, _, err := routeTable["item move"](input)
+			if err == nil {
+				t.Errorf("expected error for missing %q", missing)
+			}
+		})
+	}
+}
+
+func TestRoute_ItemSearch(t *testing.T) {
+	m, p, _, err := routeTable["item search"](map[string]any{
+		"workspace": "docapp",
+		"query":     "OAuth redirect",
+		"limit":     float64(25),
+		"status":    "open",
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if m != http.MethodGet {
+		t.Errorf("method = %q", m)
+	}
+	if !strings.HasPrefix(p, "/api/v1/search?") {
+		t.Errorf("expected /api/v1/search?... ; got %q", p)
+	}
+	values := mustParseQueryFromPath(t, p)
+	if values.Get("q") != "OAuth redirect" {
+		t.Errorf("q rename lost: %v", values)
+	}
+	if values.Get("workspace") != "docapp" {
+		t.Errorf("workspace not in query: %v", values)
+	}
+	if values.Get("limit") != "25" {
+		t.Errorf("limit lost: %v", values)
+	}
+}
+
+func TestRoute_ItemSearch_NormalizesCollectionAlias(t *testing.T) {
+	// CLI parity: `pad item search foo --collection task` normalizes
+	// to "tasks" before calling /search. The store's search filter
+	// matches exact c.slug = ?, so the alias would 0-match without
+	// normalization (Codex review #344 round 2 finding).
+	_, p, _, err := routeTable["item search"](map[string]any{
+		"workspace":  "docapp",
+		"query":      "OAuth",
+		"collection": "task",
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	values := mustParseQueryFromPath(t, p)
+	if values.Get("collection") != "tasks" {
+		t.Errorf("collection alias not normalized; got %q", values.Get("collection"))
+	}
+}
+
+func TestRoute_ItemSearch_DoesNotMutateInput(t *testing.T) {
+	// The mapper clones the input before mutating to avoid
+	// surprising the caller (the registry attaches the same input
+	// map via WithDispatchInput; downstream code reads it).
+	input := map[string]any{
+		"workspace": "ws", "query": "x", "collection": "task",
+	}
+	_, _, _, err := routeTable["item search"](input)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if input["collection"] != "task" {
+		t.Errorf("input was mutated; got collection=%v", input["collection"])
+	}
+}
+
+func TestRoute_ItemSearch_RequiresQuery(t *testing.T) {
+	_, _, _, err := routeTable["item search"](map[string]any{"workspace": "ws"})
+	if err == nil {
+		t.Errorf("expected error when query missing")
+	}
+}
+
+func TestRoute_ItemComment(t *testing.T) {
+	m, p, body, err := routeTable["item comment"](map[string]any{
+		"workspace": "docapp",
+		"ref":       "TASK-5",
+		"message":   "Looks good to me.",
+		"reply_to":  "comment-id-7",
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if m != http.MethodPost {
+		t.Errorf("method = %q", m)
+	}
+	if p != "/api/v1/workspaces/docapp/items/TASK-5/comments" {
+		t.Errorf("path = %q", p)
+	}
+	var payload map[string]any
+	if err := json.Unmarshal(body, &payload); err != nil {
+		t.Fatalf("decode: %v\n%s", err, body)
+	}
+	if payload["body"] != "Looks good to me." {
+		t.Errorf("message → body rename lost: %v", payload)
+	}
+	if payload["parent_id"] != "comment-id-7" {
+		t.Errorf("reply_to → parent_id rename lost: %v", payload)
+	}
+	if payload["source"] != "cli" {
+		t.Errorf("source not stamped: %v", payload)
+	}
+}
+
+func TestRoute_ItemComments(t *testing.T) {
+	m, p, _, err := routeTable["item comments"](map[string]any{
+		"workspace": "docapp", "ref": "TASK-5",
+	})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if m != http.MethodGet {
+		t.Errorf("method = %q", m)
+	}
+	if p != "/api/v1/workspaces/docapp/items/TASK-5/comments" {
+		t.Errorf("path = %q", p)
+	}
+}
+
+func TestRoute_ProjectDashboard(t *testing.T) {
+	m, p, _, err := routeTable["project dashboard"](map[string]any{"workspace": "docapp"})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if m != http.MethodGet || p != "/api/v1/workspaces/docapp/dashboard" {
+		t.Errorf("got %s %s", m, p)
+	}
+}
+
+func TestRoute_CollectionList(t *testing.T) {
+	m, p, _, err := routeTable["collection list"](map[string]any{"workspace": "docapp"})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if m != http.MethodGet || p != "/api/v1/workspaces/docapp/collections" {
+		t.Errorf("got %s %s", m, p)
+	}
+}
+
+func TestRoute_RoleList(t *testing.T) {
+	m, p, _, err := routeTable["role list"](map[string]any{"workspace": "docapp"})
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if m != http.MethodGet || p != "/api/v1/workspaces/docapp/agent-roles" {
+		t.Errorf("got %s %s", m, p)
+	}
+}
+
+// TestRouteTable_ContainsExpectedCommands locks the set of commands
+// the table claims to support, so an accidental deletion fails loudly
+// rather than silently flipping a tool back to "not yet implemented."
+func TestRouteTable_ContainsExpectedCommands(t *testing.T) {
+	want := []string{
+		"item create", "item show", "item list", "item delete",
+		"item move", "item search", "item comment", "item comments",
+		"project dashboard", "collection list", "role list",
+	}
+	missing := []string{}
+	for _, w := range want {
+		if _, ok := routeTable[w]; !ok {
+			missing = append(missing, w)
+		}
+	}
+	if len(missing) > 0 {
+		sort.Strings(missing)
+		t.Errorf("routeTable missing entries: %v", missing)
+	}
+}
+
+// --- Integration smoke for the new commands ---
+//
+// Drives a small subset (item show + collection list + project
+// dashboard) end-to-end against a real *server.Server. Expanded
+// coverage for the simple read paths is via the unit tests above;
+// this is the "did we wire it together right" check.
+
+func TestHTTPHandlerDispatcher_Integration_ReadPaths(t *testing.T) {
+	srv, st := newPadServer(t)
+
+	wsRec := doJSONReq(t, srv, http.MethodPost, "/api/v1/workspaces",
+		map[string]any{"name": "DocApp"})
+	if wsRec.Code != http.StatusCreated {
+		t.Fatalf("create workspace: %d %s", wsRec.Code, wsRec.Body.String())
+	}
+	var ws models.Workspace
+	if err := json.Unmarshal(wsRec.Body.Bytes(), &ws); err != nil {
+		t.Fatalf("decode workspace: %v", err)
+	}
+
+	user, err := st.CreateUser(models.UserCreate{
+		Email: "dave@example.com", Name: "Dave",
+		Password: "irrelevant",
+	})
+	if err != nil {
+		t.Fatalf("create user: %v", err)
+	}
+	if err := st.AddWorkspaceMember(ws.ID, user.ID, "owner"); err != nil {
+		t.Fatalf("add member: %v", err)
+	}
+
+	d := &HTTPHandlerDispatcher{
+		Handler:      srv,
+		UserResolver: fixedUserResolver(user),
+	}
+
+	// Seed: create one item via the dispatcher (proves item.create
+	// still works after the route-table refactor) and then read it
+	// back via item.show + project.dashboard + collection.list.
+	createCtx := WithDispatchInput(context.Background(), map[string]any{
+		"workspace":  ws.Slug,
+		"collection": "tasks",
+		"title":      "Smoke",
+		"priority":   "high",
+	})
+	if res, err := d.Dispatch(createCtx, []string{"item", "create"}, nil); err != nil || res.IsError {
+		t.Fatalf("seed item create: err=%v IsError=%v %#v", err, res != nil && res.IsError, res)
+	}
+
+	// item show — by listing first to grab the ref.
+	listCtx := WithDispatchInput(context.Background(), map[string]any{
+		"workspace": ws.Slug, "collection": "tasks",
+	})
+	listRes, err := d.Dispatch(listCtx, []string{"item", "list"}, nil)
+	if err != nil || listRes.IsError {
+		t.Fatalf("item list: err=%v IsError=%v %#v", err, listRes != nil && listRes.IsError, listRes)
+	}
+	listed, ok := listRes.StructuredContent.([]any)
+	if !ok || len(listed) == 0 {
+		t.Fatalf("item list returned unexpected shape: %#v", listRes.StructuredContent)
+	}
+	first, _ := listed[0].(map[string]any)
+	ref, _ := first["ref"].(string)
+	if ref == "" {
+		t.Fatalf("item list result missing ref: %#v", first)
+	}
+
+	// item show
+	showCtx := WithDispatchInput(context.Background(), map[string]any{
+		"workspace": ws.Slug, "ref": ref,
+	})
+	showRes, err := d.Dispatch(showCtx, []string{"item", "show"}, nil)
+	if err != nil || showRes.IsError {
+		t.Fatalf("item show: err=%v IsError=%v %#v", err, showRes != nil && showRes.IsError, showRes)
+	}
+
+	// project dashboard
+	dashCtx := WithDispatchInput(context.Background(), map[string]any{"workspace": ws.Slug})
+	dashRes, err := d.Dispatch(dashCtx, []string{"project", "dashboard"}, nil)
+	if err != nil || dashRes.IsError {
+		t.Fatalf("project dashboard: err=%v IsError=%v %#v", err, dashRes != nil && dashRes.IsError, dashRes)
+	}
+
+	// collection list
+	collCtx := WithDispatchInput(context.Background(), map[string]any{"workspace": ws.Slug})
+	collRes, err := d.Dispatch(collCtx, []string{"collection", "list"}, nil)
+	if err != nil || collRes.IsError {
+		t.Fatalf("collection list: err=%v IsError=%v %#v", err, collRes != nil && collRes.IsError, collRes)
+	}
+}