Skip to content

edit facade: every required-field error names the legacy field, not the one the schema publishes #778

Description

@vitaliyslion

Describe the bug

The edit tool publishes per-operation JSON schemas through capabilities. Calls that satisfy those schemas work — match / replacement are lowered correctly by normalizeFacadeArguments. But the errors are emitted after that lowering, by the legacy handlers, in the legacy vocabulary. Three of edit.file's four published field names appear in no error anywhere in the surface:

published required field error actually emitted
match (edit.file) old_string is required
replacement (edit.file) new_string is required
target (edit.file) path is required
match (edit.symbol) old_source is required
content (edit.write) content is required — the only one that matches

A caller that omits a required field is told to send a field its own schema forbids. It has no way to converge: the correct name is never mentioned, and the named one is not in its schema.

Two adjacent edit.batch messages compound it:

  • edit.batch given target.file returns prose that reads as "you omitted target.file", while its own data.reason says unsupported_target — the opposite: the field was supplied and batch has no use for it.
  • edit.batch with changes omitted is silently rerouted to the transaction-status path and answers transaction_id is required for status, rather than reporting the missing edit payload. edit.batch's published required is only ["operation"], so the schema never claims changes is needed either.

To Reproduce

From inside any repo the daemon tracks. <file> is any tracked file. Every call is read-only — each omits required fields.

  1. Read the published schema:

    gortex call capabilities --arg domain=edit --arg operation=file --arg detail=schema
    

    required: ["match","operation","replacement","target"], additionalProperties: false.

  2. Case A — omit a schema-required field.

    gortex call edit --arg operation=file --arg target:='{"file":"<file>"}'
    

    Error: old_string is required. Expected match is required.

    The same substitution holds for the rest of the table: drop replacementnew_string is required; drop targetpath is required; the edit.symbol variant → old_source is required.

  3. Case B — edit.batch given target.file.

    gortex call edit --arg operation=batch --arg target:='{"file":"<file>"}' --arg dry_run=true
    

    → refused:

    {"error_code":"invalid_argument","message":"edit.batch has no target.file to act on — it would be
    ignored, so the call is refused rather than answered as if it had not been sent","data":{"accepted_by":
    ["file","write"],"domain":"edit","field":"target.file","operation":"batch","reason":"unsupported_target"}}
    
  4. Case C — edit.batch with changes omitted.

    gortex call edit --arg operation=batch --arg dry_run=true
    

    Error: transaction_id is required for status. Expected something naming changes.

Reproduces identically over native MCP (edit with {"operation":"file","target":{"file":"go.mod"}}old_string is required), so this is the shared dispatcher, not a CLI artifact.

Expected behavior

  • A missing required field should be reported under the name the schema publishes (match, replacement, target), not the legacy name it lowers to. The check runs in the legacy handler, after normalizeFacadeArguments has already rewritten the key, so the public name is gone by the time the error is built.
  • Case B's message should match its own data.reason: the field was supplied and is unsupported, not omitted.
  • Case C should report the missing changes payload rather than answering as a transaction-status call.

Environment:

  • OS: Linux x86_64 (kernel 7.2)
  • Go version: go1.27.0
  • Gortex version: gortex v0.64.1+173cad87 (daemon build; repo at 0f462eff)

Additional context

Published required arrays, for reference:

operation required additionalProperties
edit.file ["match","operation","replacement","target"] false
edit.symbol ["match","operation","replacement","target"] false
edit.batch ["operation"] false
edit.write ["content","operation","target"] false

Where it lives: two schemas are produced per facade tool and only one is a contract. The tools/list schema (facadeToolDefinitionWithOperations, internal/mcp/facade_tools.go:149) is open — no additionalProperties, empty required. The capabilities schema (facadePublicCapabilitySchema, internal/mcp/facade_schema.go) is synthesized separately and hardcodes "additionalProperties": false at facade_schema.go:264, deriving required by mapping legacyRequired back through public paths. Nothing validates against the second one. The lowering that erases the public name is normalizeFacadeArguments (internal/mcp/facade_tools.go:1966-1988); the errors come from handleEditFile (internal/mcp/tools_fileops.go:786) and batchTransactionStatus (internal/mcp/batch_transaction.go:1162).

Not part of this report: additionalProperties: false is also unenforced — old_string / new_string, and any unknown key (--arg wibble=nonsense), are accepted and silently dropped with no _ignored_options rider. That is the deliberate facade exemption documented at docs/mcp.md:165 and already tracked as #636 (follow-up to #597 / #614). Worth noting there, though, that facadePublicCapabilitySchema publishes additionalProperties: false for a surface #636 describes as "open by design" — the closed schema is advertised, not just unenforced. Same family as #542, where capabilities advertised an envelope the facade rejected.

Observed in practice: in an agent session driven by a small model, 20 of 20 edit calls failed, including the same call re-sent 11 times verbatim — the error named a field the model was not permitted to send, so nothing in the response let it converge. It abandoned edit and completed the change with shell sed and heredocs, bypassing the parse gate, drift guard and auto-reindex.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions