Skip to content

fix(widgets): reject unknown patch edit fields, list accepted aliases#31

Open
nsyring wants to merge 1 commit intoagent0ai:mainfrom
nsyring:fix/widget-patch-edit-validation
Open

fix(widgets): reject unknown patch edit fields, list accepted aliases#31
nsyring wants to merge 1 commit intoagent0ai:mainfrom
nsyring:fix/widget-patch-edit-validation

Conversation

@nsyring
Copy link
Copy Markdown

@nsyring nsyring commented Apr 26, 2026

fix(widgets): reject unknown fields and document accepted aliases on patchWidget edits

Summary

spaces/storage.js accepts widget patch edits with unrecognized field names (e.g. replaceWith, path, with) without surfacing an error and silently produces a no-op or destructive patch. This PR adds an unknown-field validator and lists the accepted replacement-text aliases in the existing error messages so a misnamed edit fails fast with a precise hint instead of corrupting the renderer.

Why

LLM patch outputs frequently default to field names from VS Code edit APIs, OpenAI tool-call schemas, jsondiffpatch, etc. — for example replaceWith instead of the canonical content/replace/text/value aliases the widget patch validator currently accepts. Today the validator silently drops unrecognized fields, producing two failure modes I reproduced repeatedly during local widget development:

Failure mode A — silent line deletion. An edit like

patchWidget("embedded-browser", {
  edits: [
    { path: "renderer", line: 264, replaceWith: "..." },
    { path: "renderer", line: 265, replaceWith: "..." }
  ]
})

is accepted by the validator. line is treated as alias for both from and to so each edit becomes a single-line replace at line 264, 265, etc. None of the replaceWith payloads is read because replaceWith is not in the recognized aliases set. The replace path falls through to contentLines: hasContent ? ... : [], treating the missing replacement as an explicit "delete this line" intent. Six replacement edits become six silent line deletions, the runtime reports Widget "X" saved, rendered ok, loaded to TRANSIENT, and the agent receives a positive result for what was actually a destructive change.

Failure mode B — uninformative error. When a more obviously broken edit does fail (e.g. an insert without any replacement field), the existing message — "Insert edits must include replacement text in \content`." — does not list the other accepted aliases, so the agent often retries with another non-canonical name (with, body, newContent`, ...) and still fails.

The widget-skill SKILL.md already pins the canonical shapes and explicitly notes that line, startLine/endLine, range, text, and replace are tolerated aliases. The validator did not carry that contract through to the actual checks.

What changed

Single-file change in app/L0/_all/mod/_core/spaces/storage.js:

  1. WIDGET_PATCH_CONTENT_FIELD_ALIASES constant lists the recognized replacement-text aliases: content (canonical), text, replace, value, plus newly added replaceWith, replacement, newText. The existing readWidgetPatchContentField(...) is rewritten to iterate this list rather than hard-code a four-branch ladder. No behavior change for existing callers; the three new aliases unblock common LLM patch shapes that previously had to be hand-corrected.

  2. WIDGET_PATCH_KNOWN_EDIT_FIELDS constant captures the full set of edit-object fields the validator recognizes: every content alias plus find, search, from, to, line, startLine, endLine, range, mode, kind. New listUnknownWidgetPatchEditFields(...) helper returns any keys outside that set.

  3. normalizeWidgetPatchEdit(...) rejects unknown fields at the top of the function, before any other validation. The error message lists the accepted replacement-text aliases, the accepted coordinate aliases, and the find/search snippet shape so the agent can self-correct on the next turn. This closes the silent-deletion path because a typo'd content field name now fails with a clear name rather than being interpreted as an absent replacement.

  4. Existing insert-error message is widened to list all accepted replacement-text aliases instead of only content, matching the rest of the validator's diagnostic style.

  5. space-widgets/SKILL.md updated to match the validator's new strict-mode contract. The "patch vs rewrite" subsection now distinguishes coordinate aliases from content aliases, lists replaceWith/replacement/newText alongside the previously documented content fields, and explicitly states that other field names will be rejected fast. path, replaceText, and body are called out as common typo'd field names so the agent does not generate them.

No API change, no test touched (the patch validator is not exercised by unit tests in this repo today; adding one would require either exporting applyWidgetPatchEdits or providing a module resolver for the /mod/... absolute imports that spaces/storage.js uses, both of which are outside this PR's scope).

Worked example — silent deletion case

Before:

edits: [{ path: "renderer", line: 264, replaceWith: "x" }]
// path: ignored, line: from=to=264, replaceWith: not an alias → no content
// validator returns { kind: "replace", from: 264, to: 264, contentLines: [] }
// → silently deletes line 264, runtime says "saved, rendered ok"

After:

Error: Widget patch edit has unknown field `path`. Valid replacement-text
fields are `content`, `text`, `replace`, `value`, `replaceWith`,
`replacement`, `newText`; valid coordinates are `from`/`to` (canonical) or
`line`/`startLine`/`endLine`/`range` aliases; exact-snippet edits use
`find` (or `search`) plus a replacement field.

The agent receives the diagnostic, drops path, replaces replaceWith with content if it must, and retries.

Test plan

  • node --check app/L0/_all/mod/_core/spaces/storage.js passes
  • Existing tests/spaces_widget_import_test.mjs and tests/spaces_prompt_context_test.mjs continue to pass (validator is not exercised but the file imports cleanly)
  • Manual unknown-field detection table verified: {find,replace}, {from,to,content}, {line,replace}, {range,text} all return zero unknowns; {path,line,replaceWith} flags path; {from,to,with} flags with; {line,replaceWith} returns zero unknowns (i.e. now passes through as expected)
  • Manual verification across two providers in npm run desktop:pack builds:
    • gpt-5.4 via OpenAI Codex provider — captured the failure mode reliably (LLM-generated patch with path: "renderer", line: N, replaceWith: ... shape) and confirmed it now fails fast with the new error message; the agent self-corrects to find/replaceWith on the next turn and the patch then applies as intended
    • Local qwen3-coder model — produces canonical find/replace shapes that already passed the validator pre-PR, and continues to pass post-PR. No regression for well-formed edits

Out of scope (possible follow-ups)

  • A delete: true marker on line edits to disambiguate "intentional delete" from "missing content field," replacing the implicit content-omitted convention. The current PR keeps the implicit convention to avoid touching the SKILL contract.
  • Symmetric unknown-field detection on the metadata-side patch fields (name, cols, rows, etc.) handled by the surrounding patchWidget body. Those are a separate code path and outside the line-edit validator changed here.

🤖 Generated with Claude Code

The widget patch validator previously accepted edits with unrecognized
field names, silently producing destructive results. An LLM emitting
`{ path: "renderer", line: 264, replaceWith: "..." }` would have its
`replaceWith` payload dropped (not in the recognized aliases set). The
remaining `line` field is read as both `from` and `to`, the absent
replacement falls through to `contentLines: []`, and the validator
reports success on what is actually a single-line deletion. The runtime
prints the standard "saved, rendered ok" status and the agent treats
the destructive patch as the intended result.

Two changes close the loophole:

- `WIDGET_PATCH_KNOWN_EDIT_FIELDS` lists every recognized edit field
  (content aliases, coordinate aliases, snippet aliases); the new
  `listUnknownWidgetPatchEditFields(...)` helper returns anything
  outside that set, and `normalizeWidgetPatchEdit(...)` throws on the
  first unknown field with a message that lists the valid replacement
  and coordinate aliases so the agent can self-correct.

- `replaceWith`, `replacement`, and `newText` are added to the
  recognized content aliases. LLM patch outputs default to those names
  from VS Code, OpenAI tool-call, and similar conventions; they unblock
  the common patch shape without forcing every caller to rewrite the
  field name first.

The existing insert-without-replacement error message is widened to
list all accepted replacement-text aliases instead of only `content`.

No API change, no Skill rewrite, no tests touched.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant