fix(registry,cli): an mcp entry's fields must agree with its transport (TASK-071)

[lilyshen0722]

TASK-071. Vera's ruling on #1764 was "change the schema, not the guard"; her follow-up corrected the placement: the write that daemons execute is a backend write, so tightening only the CLI's validateEnvironmentSpec would not have validated the field where it is actually written.

A correction to my own first draft, found by Vera in review (70201). I wrote that PATCH /api/registry/pods/:podId/agents/:name was "the only backend writer of config.environment". It is not. POST /api/registry/install takes config from the body (install.ts:111), passes it through normalizeConfigMap — a passthrough for a plain object — and stores it as the installation config (:534), unchecked. So an entry that contradicts its own transport could still be written at install time, which is the whole defect one route over. Both writers now call the same validator, and both comments that stated the false count have been corrected to name both.

The rule

A transport decides the entry, so the fields have to agree with it:

transport	required	forbidden
http, sse	url (non-empty string)	command
stdio, or absent	command (non-empty argv array)	url
anything else	— refused, and judged no further

transport absent means stdio deliberately: that is the historical default, and stored rows predate the explicit declaration.

What was measured before writing any code

1. command is an argv array in every consumer — connectStdioMcp destructures const [cmd, ...args] = command (cli/src/lib/adapters/pi-mcp-client.mjs:90-91) and isStdioServer is Array.isArray(s?.command) && s.command.length > 0 (:342), while url is always the string form (:344). A string command is currently dropped in silence by readServers (:355), so the array requirement is part of the agreement rule rather than a separate nicety.
2. The consumers branch on field shape, not on transport, and pi-mcp-client's own comment says a hand-built entry carrying both resolves url-first (:80-85). One stored record, three adapters, no single answer to "what runs?" — which is why the write-time check has to be the transport rule and not an inference from which field is present.
3. Not reusing validateMcpComponent (backend/utils/pluginManifestParser.ts:777): that validates a plugin manifest and normalizes, dropping every field its shape does not carry. A seat environment entry carries env (the default declaration puts a token reference there), so routing this path through it would trade a shape bug for a data-loss bug.

Two write sites, one rule

• backend/utils/environmentSpecValidation.ts (new) — called from both backend writers: the agent-config PATCH (before any write) and the install handler (before AgentInstallation.install), each returning a typed 400 { code: 'invalid_environment_spec', fields: [{ field, message }] } with every offending entry reported, not the first. In the install handler the check sits immediately after installConfig is built, because a 400 that arrives after the side effect is a report rather than a refusal.
• cli/src/lib/environment.js — the same block, for the other write path, an operator-authored --environment <file>.

They are mirrored, not shared: this is the CJS backend and that is a published ESM package with its own dependency closure, so a runtime import is not available. The wording is kept parallel, each names the other in a comment, and both test files pin the same cases.

The reviewer's gate

send a PATCH with {transport:'http', url, command} and expect a typed 400

That is the first test in backend/__tests__/unit/routes/registry.environment-mcp-shape.test.js, which also asserts save was never called; the install twins are in registry.install-environment-mcp-shape.test.js, where the second test asserts AgentInstallation.install was never called.

The deliberate limit, pinned by a test

Only the environment this request declares is validated, never the merged stored value. Otherwise a row that already holds a malformed entry becomes unpatchable for its other fields, and refusing old records is not what this rule is for. leaves a row that already holds a malformed entry patchable for its other fields pins exactly that; mutation M7 (validate the stored value too) reddens it.

Evidence

• 14 new tests (11 route + 3 install); whole backend/__tests__/unit/routes = 126 suites / 923 tests green; full CLI suite = 40 suites / 643 tests green.
• Eleven mutations, each reddening a distinct witness, no survivors. Nine on the rule itself (http/sse stops forbidding command, stops requiring url; stdio stops forbidding url; a string accepted as command; the early return on an unknown transport dropped; the route stops calling the validator; the stored value validated too; two CLI equivalents). Two on the second writer: M10 install stops calling the validator (both install tests red), M11 the install check runs after the side effect (only the ordering test red). M13/M14 witness the control, one half each — M13 makes the validator refuse any mcp array, M14 makes the install check refuse whenever environment is present at all — because an over-refusal is as much a defect as a missing refusal, and a control nothing can redden is decoration. Each applied alone, source restored byte-identical after each.
• npm run lint:ts 0 errors; the new backend helper adds no warnings (the two max-len warnings in install.ts are on lines unchanged from main); eslint src in cli exit 0.

Version

cli 0.1.60 — cli/src moved so the guard requires a bump; 0.1.58 is #1777's (merged as bd801882) and 0.1.59 is the TASK-052 baseline's, per connector-ops in the pod.