fix(mcp): unblock the agent authoring path, and default init to a minimal project - #2
Merged
Conversation
…hemas
pz_add_connection and pz_update_connection failed 100% of the time from any
real MCP client, with "An error occurred invoking '<tool>'." and no diagnostics
anywhere -- the whole connection-authoring surface was unreachable.
The SDK publishes the boolean schema `true` ("any value") for the non-nullable
JsonElement parameter both tools take for `connection`. JsonElement.TryGetProperty
throws on a non-object element, so ArgumentValidatingTool -- which exists to keep
that exact generic string off the wire -- was the thing producing it.
Guard the property-schema kind before reading `type`, and add PZ0609 as the last
catch: an unclassified handler exception now answers a real envelope carrying the
exception text, instead of the SDK's own catch discarding it (`pz mcp` wires no
ILoggerFactory, so it was lost server-side too).
The direct-call tests passed a Dictionary and so never met the published schema;
the new tests drive both tools over a real client transport, where the gap was.
A wholesale replace takes the connection's entities: block with it -- documented, deliberate, and invisible: the envelope reported dropped_comment and nothing else, so an agent that called pz_add_entity and then adjusted one connection option was told ok:true over the wreckage of its own prior call, finding out only at compile. Name them in dropped_entities with a warning pointing back at pz_add_entity, and say so in the tool description so it is known before the call, not after. The report rides the error envelope too -- a dropped entity is usually the very reason self-verify then fails, which is exactly when withholding it hurts most.
The load path's next_steps are written for someone at a shell. Dropped into an empty directory an agent was told to "run 'pz init <name>'" -- a command it cannot run and whose tool equivalent, pz_init_project, went unnamed. PZ0103 was worse: "set the variable before running pz" omits that the server snapshots the environment of its own process, so setting it in any other shell changes nothing until the server restarts, and the agent loops. Rewrite both at the one load seam every handler already funnels through. Keyed on the loader's hint text, so a future rephrasing there degrades to the CLI wording -- a worse hint, not a wrong one.
The SDK infers the boolean schema `true` -- "any value" -- for a JsonElement parameter, so connection/read/write were published with no type and no description: an agent deciding its first pz_add_connection call read the schema and learned nothing about the one shape the handler accepts. Republish them as typed objects pointing at pz_connector_reference for the connector's own option schema. The optional ones keep the `default` the SDK emitted, which is what marks them optional -- required stays unchanged.
An `additionalProperties: false` violation -- a misremembered or misspelled option name, the most common authoring mistake there is -- was reported as "/connection_string: All values fail against the false schema", which names neither the offending key nor what was allowed instead. It now reads "unknown option 'connection_string'" with the accepted set as the next step. Detection is structural, never textual: the evaluation path's last segment is additionalProperties AND resolving its parent in the schema finds one that is literally false. Both halves matter -- additionalProperties is equally legal as a subschema (localfiles' `columns` maps each column name to a type enum that way), and a violation under one of those means "this value is wrong", not "this key is unknown". Matching the library's message text would conflate the two.
…ritten pz_init_project handed every caller the runnable four-pipeline sample. An agent asked for one specific pipeline had to delete six files before starting, and until it did, those demo pipelines compiled -- so pz_run(all: true) moved demo data nobody asked for. The result said only `created: true`, so learning what had landed took another round trip. Add a `minimal` template (project.yml + connections.yml, both commented with the shapes an author needs) and default the tool to it; `minimal: false` still gets the sample, for when a worked example IS the request. Report the template used and every file written, read back off disk rather than predicted. `pz init` is untouched: someone meeting pz for the first time still gets the runnable demo, which is the right thing to learn the shape from.
Makes the CLI default match the MCP tool's: `pz init <name>` now writes project.yml + connections.yml, and the runnable four-pipeline demo moves behind `--sample`. Someone starting their own project no longer deletes six files first -- and, until they did, those demo pipelines compiled, so a first `pz run --all` moved demo data they never wrote. InitCommand.Execute drops its default template argument: which project a caller scaffolds is never incidental, and a default there is how the two front doors would silently drift apart. BREAKING: `pz init` output changes. Callers wanting the demo add `--sample`. README's five-command quickstart and the packaging gate are updated to it; the gate now also asserts the bare verb produces the minimal project, since the template set is chosen from embedded resources and shipping only one of the two directories would otherwise surface as a stranger's first command scaffolding the wrong thing. The template byte-contract scan widens from Templates/init to all of Templates/, so the default output -- and any template added later -- is covered too.
coccor
changed the title
coccor
marked this pull request as ready for review
August 20, 2026 14:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found by smoke-testing the MCP surface end to end as a connected agent would drive it, against the scenario "copy an Azure SQL table to CSV" (backed by a real SQL Server container, 3 rows, table → CSV).
The blocker
pz_add_connectionandpz_update_connectionfailed 100% of the time from any real MCP client —"An error occurred invoking 'pz_add_connection'.", no PZ code, no cause, and nothing logged server-side either (pz mcpwires noILoggerFactory). The whole connection-authoring surface was unreachable, so no agent could author a project through MCP at all.ArgumentValidatingTool— the decorator whose entire purpose is keeping that generic string off the wire — was producing it. The SDK publishes the boolean schematrue("any value") for the non-nullableJsonElementparameter both tools take, andJsonElement.TryGetPropertythrows on a non-object element.The suite was green throughout: every authoring test called
AuthoringToolsdirectly with aDictionary, so none ever met the published schema. The gap was the transport, not the logic. New tests drive both tools over a real client transport.Also fixed
"An error occurred invoking '<tool>'."is now a string no pz MCP client can receive.pz_update_connectionsilently ate entities. A wholesale replace takes theentities:block with it — documented, deliberate, invisible. The envelope reporteddropped_commentand nothing else, so an agent that calledpz_add_entityand then adjusted one connection option was toldok: trueover the wreckage of its own prior call. Now reported asdropped_entities+ a warning, on the error envelope too (a dropped entity is usually the reason self-verify then fails).run 'pz init <name>'— a shell command, with the tool equivalent unnamed. PZ0103 omitted that the server snapshots its own process environment, so setting the variable in another shell changes nothing until the server restarts; without that, the agent loops.connection/read/writepublished no type and no description. Now typed objects pointing atpz_connector_reference./connection_string: All values fail against the false schema— naming neither the offending key nor what was allowed. Nowunknown option 'connection_string', with the accepted set as the next step. Detection is structural (evaluation path ends atadditionalPropertiesand that schema is literallyfalse), so a subschemaadditionalProperties— localfiles'columnstype map — keeps its own message.Breaking:
pz initnow scaffolds a minimal projectBoth front doors default to
project.yml+connections.yml; the runnable four-pipeline demo is behindpz init <name> --sample(minimal: falseforpz_init_project). Nobody starting their own project deletes six files first — and, until they did, those demo pipelines compiled, so a firstpz run --allmoved demo data they never wrote.pz_init_projectalso now reports the template used and every file written.README's five-command quickstart uses
--sample. The packaging gate exercises both templates:--samplefor the offline run proof, and the bare verb asserted to produce exactly the minimal project — the template set is chosen from embedded resources, so shipping only one of the two directories would otherwise surface as a stranger's first command scaffolding the wrong thing. The template byte-contract scan widens fromTemplates/initto all ofTemplates/.Verification
Clean-room replay of the scenario: 8 tool calls, no dead ends, correct CSV on disk, and a project containing exactly
project.yml,connections.yml,pipelines/orders_to_csv.sql— no demo residue,pz_run(all: true)runs only the agent's own pipeline. Full solution suite green (21 projects, 0 failures);scripts/verify-tool-install.shpasses; both CLI paths checked against a built binary (pz init→ two files;pz init --sample→pz run --all→ 12 nodes ok).Needs a matching pz-site PR
/reference/mcp-contract/: new PZ0609;dropped_entities/warningsin thepz_update_connectionresult;connection/read/writepublished as typed objects;pz_init_project's newminimalinput and itstemplate/filesresult fields (default behaviour change);pz_update_connectionandpz_init_projectdescriptions. CLI reference:pz init --sampleand the new default. Plus the PZ0301 message/hint wording wherever error codes are documented, and any getting-started page showingpz init→pz run.Not fixed — reported separately
pz_entity_schemarequires a declared read before it can discover one, which inverts the natural order for an agent exploring a database.