mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
refactor!: drop edit_files search/replace wire compat (#29221)
Remove the deprecated `search`/`replace` keys from the `edit_files` wire type. `FileEdit` now encodes and decodes `old_text`/`new_text` only; the shims that carried the old keys through the rename (#28467) are gone. **Breaking change:** chat lifecycle hook consumers must use `old_text` and `new_text` for `edit_files`. A `pre_tool_use` `input_override` that still sends `search`/`replace` fails with `old_text must not be empty`. Fixes CODAGT-483
This commit is contained in:
@@ -2048,18 +2048,6 @@ func testTool[Arg, Ret any](t *testing.T, tool toolsdk.Tool[Arg, Ret], tb toolsd
|
||||
return ret, err
|
||||
}
|
||||
|
||||
// TestEditFileTools_DecodeDeprecatedKeys pins that deprecated-key
|
||||
// MCP args survive decoding into the typed edit args.
|
||||
func TestEditFileTools_DecodeDeprecatedKeys(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var multi toolsdk.WorkspaceEditFilesArgs
|
||||
require.NoError(t, json.Unmarshal([]byte(
|
||||
`{"workspace":"w","files":[{"path":"/p","edits":[{"search":"foo","replace":"bar"}]}]}`), &multi))
|
||||
require.Equal(t, "foo", multi.Files[0].Edits[0].OldText)
|
||||
require.Equal(t, "bar", multi.Files[0].Edits[0].NewText)
|
||||
}
|
||||
|
||||
func TestWithRecovery(t *testing.T) {
|
||||
t.Parallel()
|
||||
t.Run("OK", func(t *testing.T) {
|
||||
|
||||
@@ -1377,62 +1377,15 @@ func DefaultReadFileLinesLimits() ReadFileLinesLimits {
|
||||
// FileEdit is a single old_text -> new_text replacement applied
|
||||
// to one file. The fields use old_text/new_text instead of the
|
||||
// earlier search/replace because models confused the direction
|
||||
// (CODAGT-312). MarshalJSON and UnmarshalJSON keep the deprecated
|
||||
// search/replace keys on the wire for rollout compatibility; remove
|
||||
// both in the first release after Coder Agents GA (2026-09)
|
||||
// (CODAGT-483).
|
||||
// (CODAGT-312); the old keys are not decoded.
|
||||
type FileEdit struct {
|
||||
OldText string `json:"old_text" description:"Existing text in the file to replace. Matching is fuzzy: whitespace and indentation differences are tolerated."`
|
||||
NewText string `json:"new_text" description:"Text that replaces old_text."`
|
||||
ReplaceAll bool `json:"replace_all,omitempty" description:"Replace every match of old_text instead of erroring when it matches more than once."`
|
||||
}
|
||||
|
||||
// MarshalJSON emits both the current and the deprecated
|
||||
// "search"/"replace" keys so agents that predate the rename keep
|
||||
// decoding the request while coderd upgrades ahead of running
|
||||
// workspaces.
|
||||
func (e FileEdit) MarshalJSON() ([]byte, error) {
|
||||
type wire FileEdit
|
||||
return json.Marshal(struct {
|
||||
wire
|
||||
Search string `json:"search"`
|
||||
Replace string `json:"replace"`
|
||||
}{wire: wire(e), Search: e.OldText, Replace: e.NewText})
|
||||
}
|
||||
|
||||
// UnmarshalJSON accepts the deprecated "search"/"replace" keys so
|
||||
// callers that predate the rename keep working. The fallback applies
|
||||
// only when both new fields are empty, which identifies an old-wire
|
||||
// caller; if either new field is set, an explicitly empty value
|
||||
// (e.g. new_text="" for a deletion) is preserved.
|
||||
//
|
||||
// The fallback decodes the deprecated keys through the same struct
|
||||
// tags the pre-rename type used, so its behavior is identical to the
|
||||
// old decoder, including case-insensitive key matching.
|
||||
func (e *FileEdit) UnmarshalJSON(data []byte) error {
|
||||
type wire FileEdit
|
||||
var w wire
|
||||
if err := json.Unmarshal(data, &w); err != nil {
|
||||
return err
|
||||
}
|
||||
*e = FileEdit(w)
|
||||
if e.OldText == "" && e.NewText == "" {
|
||||
var legacy struct {
|
||||
Search string `json:"search"`
|
||||
Replace string `json:"replace"`
|
||||
}
|
||||
if err := json.Unmarshal(data, &legacy); err != nil {
|
||||
return err
|
||||
}
|
||||
e.OldText = legacy.Search
|
||||
e.NewText = legacy.Replace
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// FileEdits carries the model-facing schema descriptions so the
|
||||
// chat tool's generated schema includes per-field guidance; see
|
||||
// FileEdit for the removal target.
|
||||
// chat tool's generated schema includes per-field guidance.
|
||||
type FileEdits struct {
|
||||
Path string `json:"path" description:"The absolute path of the file to edit, for example /home/coder/project/main.go."`
|
||||
Edits []FileEdit `json:"edits" description:"Edits that replace old text with new text, applied to this file in order."`
|
||||
|
||||
@@ -146,82 +146,17 @@ func (f *fakeResolver) LookupIP(_ context.Context, network, host string) ([]net.
|
||||
return f.hostMap[host], f.err
|
||||
}
|
||||
|
||||
// TestFileEdit_MarshalEmitsDeprecatedKeys pins the coderd->agent
|
||||
// wire compatibility: the request JSON must keep carrying the
|
||||
// pre-rename search/replace keys so running agents on older versions
|
||||
// decode edits while coderd upgrades first.
|
||||
func TestFileEdit_MarshalEmitsDeprecatedKeys(t *testing.T) {
|
||||
// TestFileEdit_JSONIgnoresDeprecatedKeys pins the removal of the
|
||||
// search/replace wire compatibility: the encoder emits only the
|
||||
// current keys and the decoder does not read the deprecated ones.
|
||||
func TestFileEdit_JSONIgnoresDeprecatedKeys(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
edit := workspacesdk.FileEdit{OldText: "old", NewText: "new", ReplaceAll: true}
|
||||
data, err := json.Marshal(edit)
|
||||
data, err := json.Marshal(workspacesdk.FileEdit{OldText: "old", NewText: "new", ReplaceAll: true})
|
||||
require.NoError(t, err)
|
||||
require.JSONEq(t, `{"old_text":"old","new_text":"new","replace_all":true}`, string(data))
|
||||
|
||||
// The pre-rename FileEdit shape an old agent decodes into.
|
||||
var oldAgent struct {
|
||||
Search string `json:"search"`
|
||||
Replace string `json:"replace"`
|
||||
ReplaceAll bool `json:"replace_all"`
|
||||
}
|
||||
require.NoError(t, json.Unmarshal(data, &oldAgent))
|
||||
require.Equal(t, "old", oldAgent.Search)
|
||||
require.Equal(t, "new", oldAgent.Replace)
|
||||
require.True(t, oldAgent.ReplaceAll)
|
||||
|
||||
// The new decode path sees the same values.
|
||||
var round workspacesdk.FileEdit
|
||||
require.NoError(t, json.Unmarshal(data, &round))
|
||||
require.Equal(t, edit, round)
|
||||
}
|
||||
|
||||
// TestFileEdit_UnmarshalAcceptsDeprecatedKeys pins the decode side
|
||||
// of FileEdit's deprecated-key fallback.
|
||||
func TestFileEdit_UnmarshalAcceptsDeprecatedKeys(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
in string
|
||||
want workspacesdk.FileEdit
|
||||
}{
|
||||
{
|
||||
name: "OldKeysOnly",
|
||||
in: `{"search":"old","replace":"new"}`,
|
||||
want: workspacesdk.FileEdit{OldText: "old", NewText: "new"},
|
||||
},
|
||||
{
|
||||
name: "OldKeysWithReplaceAll",
|
||||
in: `{"search":"old","replace":"new","replace_all":true}`,
|
||||
want: workspacesdk.FileEdit{OldText: "old", NewText: "new", ReplaceAll: true},
|
||||
},
|
||||
{
|
||||
name: "NewKeysWinWhenSet",
|
||||
in: `{"old_text":"o","new_text":"n","search":"old","replace":"legacy"}`,
|
||||
want: workspacesdk.FileEdit{OldText: "o", NewText: "n"},
|
||||
},
|
||||
{
|
||||
name: "ExplicitEmptyNewTextPreserved",
|
||||
in: `{"old_text":"o","new_text":"","search":"old","replace":"legacy"}`,
|
||||
want: workspacesdk.FileEdit{OldText: "o", NewText: ""},
|
||||
},
|
||||
{
|
||||
name: "OldKeysDeletion",
|
||||
in: `{"search":"old","replace":""}`,
|
||||
want: workspacesdk.FileEdit{OldText: "old", NewText: ""},
|
||||
},
|
||||
{
|
||||
name: "CaseVariantsAccepted",
|
||||
in: `{"SEARCH":"old","REPLACE":"new"}`,
|
||||
want: workspacesdk.FileEdit{OldText: "old", NewText: "new"},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var got workspacesdk.FileEdit
|
||||
require.NoError(t, json.Unmarshal([]byte(tt.in), &got))
|
||||
require.Equal(t, tt.want, got)
|
||||
})
|
||||
}
|
||||
var got workspacesdk.FileEdit
|
||||
require.NoError(t, json.Unmarshal([]byte(`{"search":"old","replace":"new"}`), &got))
|
||||
require.Equal(t, workspacesdk.FileEdit{}, got)
|
||||
}
|
||||
|
||||
@@ -79,7 +79,7 @@ Coder's built-in tools decode that JSON with Go, which matches property names ca
|
||||
Coder rejects a built-in tool call whose input repeats a key or spells a schema property with different capitalization, before dispatching `pre_tool_use`.
|
||||
This check doesn't cover dynamic and MCP tools, because the client and the workspace agent execute those calls rather than `coderd`.
|
||||
A policy that gates them must validate their input itself.
|
||||
The chat `edit_files` tool reads `old_text` and `new_text` and, for rollout compatibility, also accepts the deprecated `search` and `replace` keys when the new fields are empty. A policy that gates edit content should inspect `old_text` and `new_text`, and `search`/`replace` while the compatibility window lasts.
|
||||
The chat `edit_files` tool reads `old_text` and `new_text`; a policy that gates edit content must inspect those keys.
|
||||
|
||||
For `user_prompt_submit`, `prompt` concatenates the original submitted text parts, and `parts` carries the original structured message, including non-text parts such as file references.
|
||||
These values are captured before the consumer's override or injected context changes the stored prompt.
|
||||
|
||||
Reference in New Issue
Block a user