fix: make Edit tool lines schema compatible with Vertex AI strict validation (#2408)
This commit is contained in:
@@ -6,6 +6,7 @@ import { tmpdir } from "node:os"
|
|||||||
import { dirname, join } from "node:path"
|
import { dirname, join } from "node:path"
|
||||||
import { pathToFileURL } from "node:url"
|
import { pathToFileURL } from "node:url"
|
||||||
import { tool } from "@opencode-ai/plugin"
|
import { tool } from "@opencode-ai/plugin"
|
||||||
|
import { createHashlineEditTool } from "../tools/hashline-edit"
|
||||||
import { normalizeToolArgSchemas } from "./normalize-tool-arg-schemas"
|
import { normalizeToolArgSchemas } from "./normalize-tool-arg-schemas"
|
||||||
|
|
||||||
const tempDirectories: string[] = []
|
const tempDirectories: string[] = []
|
||||||
@@ -19,6 +20,13 @@ function getNestedRecord(record: Record<string, unknown>, key: string): Record<s
|
|||||||
return isRecord(value) ? value : undefined
|
return isRecord(value) ? value : undefined
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function getRecordAtPath(record: Record<string, unknown>, path: string[]): Record<string, unknown> | undefined {
|
||||||
|
return path.reduce<Record<string, unknown> | undefined>(
|
||||||
|
(currentRecord, key) => (currentRecord ? getNestedRecord(currentRecord, key) : undefined),
|
||||||
|
record,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
async function loadSeparateHostZodModule(): Promise<typeof import("zod")> {
|
async function loadSeparateHostZodModule(): Promise<typeof import("zod")> {
|
||||||
const pluginPackageDirectory = dirname(Bun.resolveSync("@opencode-ai/plugin/package.json", import.meta.dir))
|
const pluginPackageDirectory = dirname(Bun.resolveSync("@opencode-ai/plugin/package.json", import.meta.dir))
|
||||||
const sourceZodDirectory = join(pluginPackageDirectory, "node_modules", "zod")
|
const sourceZodDirectory = join(pluginPackageDirectory, "node_modules", "zod")
|
||||||
@@ -94,4 +102,29 @@ describe("normalizeToolArgSchemas", () => {
|
|||||||
expect(afterQuery?.title).toBe("Query")
|
expect(afterQuery?.title).toBe("Query")
|
||||||
expect(afterQuery?.examples).toEqual(["issue 2314"])
|
expect(afterQuery?.examples).toEqual(["issue 2314"])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("collapses hashline lines union into a Vertex-compatible array schema", async () => {
|
||||||
|
// given
|
||||||
|
const hostZod = await loadSeparateHostZodModule()
|
||||||
|
const toolDefinition = createHashlineEditTool()
|
||||||
|
|
||||||
|
// when
|
||||||
|
const beforeSchema = serializeWithHostZod(hostZod, toolDefinition.args)
|
||||||
|
const beforeLines = getRecordAtPath(beforeSchema, ["properties", "edits", "items", "properties", "lines"])
|
||||||
|
|
||||||
|
normalizeToolArgSchemas(toolDefinition)
|
||||||
|
|
||||||
|
const afterSchema = serializeWithHostZod(hostZod, toolDefinition.args)
|
||||||
|
const afterLines = getRecordAtPath(afterSchema, ["properties", "edits", "items", "properties", "lines"])
|
||||||
|
const afterItems = afterLines ? getNestedRecord(afterLines, "items") : undefined
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(beforeLines?.type).toBeUndefined()
|
||||||
|
expect(Array.isArray(beforeLines?.anyOf)).toBe(true)
|
||||||
|
expect(afterLines?.type).toBe("array")
|
||||||
|
expect(afterLines?.nullable).toBe(true)
|
||||||
|
expect(afterLines?.anyOf).toBeUndefined()
|
||||||
|
expect(afterItems?.type).toBe("string")
|
||||||
|
expect(afterLines?.description).toBe("Replacement or inserted lines. null/[] deletes with replace")
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -9,11 +9,106 @@ type SchemaWithJsonSchemaOverride = ToolArgSchema & {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||||
|
return typeof value === "object" && value !== null && !Array.isArray(value)
|
||||||
|
}
|
||||||
|
|
||||||
function stripRootJsonSchemaFields(jsonSchema: Record<string, unknown>): Record<string, unknown> {
|
function stripRootJsonSchemaFields(jsonSchema: Record<string, unknown>): Record<string, unknown> {
|
||||||
const { $schema: _schema, ...rest } = jsonSchema
|
const { $schema: _schema, ...rest } = jsonSchema
|
||||||
return rest
|
return rest
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isNullSchema(jsonSchema: Record<string, unknown>): boolean {
|
||||||
|
return jsonSchema.type === "null"
|
||||||
|
}
|
||||||
|
|
||||||
|
function isStringSchema(jsonSchema: Record<string, unknown>): boolean {
|
||||||
|
return jsonSchema.type === "string"
|
||||||
|
}
|
||||||
|
|
||||||
|
function isStringArraySchema(jsonSchema: Record<string, unknown>): boolean {
|
||||||
|
if (jsonSchema.type !== "array") {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
const items = jsonSchema.items
|
||||||
|
return isRecord(items) && items.type === "string"
|
||||||
|
}
|
||||||
|
|
||||||
|
function collapseNullableUnion(
|
||||||
|
jsonSchema: Record<string, unknown>,
|
||||||
|
variants: Record<string, unknown>[],
|
||||||
|
): Record<string, unknown> | null {
|
||||||
|
const nonNullVariants = variants.filter((variant) => !isNullSchema(variant))
|
||||||
|
|
||||||
|
if (nonNullVariants.length !== 1 || variants.length !== nonNullVariants.length + 1) {
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
|
||||||
|
const { anyOf: _anyOf, ...schemaWithoutAnyOf } = jsonSchema
|
||||||
|
return {
|
||||||
|
...nonNullVariants[0],
|
||||||
|
...schemaWithoutAnyOf,
|
||||||
|
nullable: true,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function collapseStringOrStringArrayUnion(
|
||||||
|
jsonSchema: Record<string, unknown>,
|
||||||
|
variants: Record<string, unknown>[],
|
||||||
|
): Record<string, unknown> | null {
|
||||||
|
const nonNullVariants = variants.filter((variant) => !isNullSchema(variant))
|
||||||
|
const stringVariant = nonNullVariants.find(isStringSchema)
|
||||||
|
const stringArrayVariant = nonNullVariants.find(isStringArraySchema)
|
||||||
|
|
||||||
|
if (!stringVariant || !stringArrayVariant || nonNullVariants.length !== 2) {
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
|
||||||
|
const { anyOf: _anyOf, ...schemaWithoutAnyOf } = jsonSchema
|
||||||
|
|
||||||
|
return {
|
||||||
|
...stringArrayVariant,
|
||||||
|
...schemaWithoutAnyOf,
|
||||||
|
nullable: variants.length !== nonNullVariants.length,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeAnyOfUnion(jsonSchema: Record<string, unknown>): Record<string, unknown> {
|
||||||
|
const anyOf = jsonSchema.anyOf
|
||||||
|
if (!Array.isArray(anyOf) || jsonSchema.type !== undefined) {
|
||||||
|
return jsonSchema
|
||||||
|
}
|
||||||
|
|
||||||
|
const variants = anyOf.filter(isRecord)
|
||||||
|
if (variants.length !== anyOf.length) {
|
||||||
|
return jsonSchema
|
||||||
|
}
|
||||||
|
|
||||||
|
return collapseNullableUnion(jsonSchema, variants) ?? collapseStringOrStringArrayUnion(jsonSchema, variants) ?? jsonSchema
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeJsonSchemaValue(jsonSchema: unknown): unknown {
|
||||||
|
if (Array.isArray(jsonSchema)) {
|
||||||
|
return jsonSchema.map((item) => normalizeJsonSchemaValue(item))
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!isRecord(jsonSchema)) {
|
||||||
|
return jsonSchema
|
||||||
|
}
|
||||||
|
|
||||||
|
return normalizeJsonSchema(jsonSchema)
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeJsonSchema(jsonSchema: Record<string, unknown>): Record<string, unknown> {
|
||||||
|
const normalized: Record<string, unknown> = {}
|
||||||
|
for (const [key, value] of Object.entries(jsonSchema)) {
|
||||||
|
normalized[key] = normalizeJsonSchemaValue(value)
|
||||||
|
}
|
||||||
|
|
||||||
|
return normalizeAnyOfUnion(normalized)
|
||||||
|
}
|
||||||
|
|
||||||
function attachJsonSchemaOverride(schema: SchemaWithJsonSchemaOverride): void {
|
function attachJsonSchemaOverride(schema: SchemaWithJsonSchemaOverride): void {
|
||||||
if (schema._zod.toJSONSchema) {
|
if (schema._zod.toJSONSchema) {
|
||||||
return
|
return
|
||||||
@@ -24,7 +119,7 @@ function attachJsonSchemaOverride(schema: SchemaWithJsonSchemaOverride): void {
|
|||||||
delete schema._zod.toJSONSchema
|
delete schema._zod.toJSONSchema
|
||||||
|
|
||||||
try {
|
try {
|
||||||
return stripRootJsonSchemaFields(tool.schema.toJSONSchema(schema))
|
return normalizeJsonSchema(stripRootJsonSchemaFields(tool.schema.toJSONSchema(schema)))
|
||||||
} finally {
|
} finally {
|
||||||
schema._zod.toJSONSchema = originalOverride
|
schema._zod.toJSONSchema = originalOverride
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -192,6 +192,25 @@ describe("createHashlineEditTool", () => {
|
|||||||
expect(result).toContain("non-empty")
|
expect(result).toContain("non-empty")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("treats replace with null lines as deletion", async () => {
|
||||||
|
//#given
|
||||||
|
const filePath = path.join(tempDir, "delete-line.txt")
|
||||||
|
fs.writeFileSync(filePath, "line1\nline2\nline3")
|
||||||
|
const line2Hash = computeLineHash(2, "line2")
|
||||||
|
|
||||||
|
//#when
|
||||||
|
await tool.execute(
|
||||||
|
{
|
||||||
|
filePath,
|
||||||
|
edits: [{ op: "replace", pos: `2#${line2Hash}`, lines: null }],
|
||||||
|
},
|
||||||
|
createMockContext(),
|
||||||
|
)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(fs.readFileSync(filePath, "utf-8")).toBe("line1\nline3")
|
||||||
|
})
|
||||||
|
|
||||||
it("supports file rename with edits", async () => {
|
it("supports file rename with edits", async () => {
|
||||||
//#given
|
//#given
|
||||||
const filePath = path.join(tempDir, "source.txt")
|
const filePath = path.join(tempDir, "source.txt")
|
||||||
|
|||||||
Reference in New Issue
Block a user