Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions packages/opencode/src/altimate/tool-zod-compat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { SessionV1 } from "@opencode-ai/core/v1/session"
import type { Agent } from "@/agent/agent"
import type { SessionID, MessageID } from "../session/schema"
import type { Context as NewContext, DefWithoutID } from "../tool/tool"
import { fillerSafeParameters } from "../tool/filler-parameters"

export type Metadata = { [key: string]: any; error?: string }

Expand Down Expand Up @@ -173,23 +174,27 @@ export function zodToJsonSchema(schema: z.ZodType): JSONSchema7 {

/** Build a `DefWithoutID` from a resolved legacy tool object (no Effect wrapper). */
function legacyDefToDef(legacy: LegacyToolDef): DefWithoutID {
const validationSchema = zodToJsonSchema(legacy.parameters)
return {
description: legacy.description,
// altimate_change start — decode THROUGH zod's `parse` so defaults, coercions and transforms
// are applied to the value `execute` receives (and that callers see via decode). A plain
// Schema.declare only validates and passes the raw input through, silently dropping zod
// `.default()`/`.transform()`. Validation failures still surface as Effect SchemaErrors whose
// nested issue.actual carries the original input (used by formatValidationError below).
parameters: Schema.declare<unknown>((u): u is unknown => legacy.parameters.safeParse(u).success).pipe(
Schema.decodeTo(Schema.Unknown, {
decode: SchemaGetter.transform((u: unknown) => legacy.parameters.parse(u)),
encode: SchemaGetter.transform((u: unknown) => u),
}),
parameters: fillerSafeParameters(
Schema.declare<unknown>((u): u is unknown => legacy.parameters.safeParse(u).success).pipe(
Schema.decodeTo(Schema.Unknown, {
decode: SchemaGetter.transform((u: unknown) => legacy.parameters.parse(u)),
encode: SchemaGetter.transform((u: unknown) => u),
}),
),
validationSchema,
),
// altimate_change end
// altimate_change start — upstream_fix: use an explicit JSON schema when a legacy
// tool needs hidden compatibility arguments that should not be advertised.
jsonSchema: legacy.jsonSchema ?? zodToJsonSchema(legacy.parameters),
jsonSchema: legacy.jsonSchema ?? validationSchema,
// altimate_change end
...(legacy.formatValidationError
? {
Expand Down
21 changes: 18 additions & 3 deletions packages/opencode/src/altimate/tools/sql-check.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import z from "zod"
import { Tool } from "../../tool/tool"
import { zodToJsonSchema } from "../tool-zod-compat"
import * as Precedence from "../workspace/precedence"
import * as Registry from "../native/connections/registry"
import { classifyAndCheck, isSingleQuery } from "./sql-classify"
Expand Down Expand Up @@ -31,7 +32,7 @@ const parameters = z
right_col: column.optional().describe("Right output column for keys"),
left_warehouse: z.string().optional().describe("Optional assertion: must equal warehouse"),
right_warehouse: z.string().optional().describe("Optional assertion: must equal warehouse"),
query_timeout_ms: z.number().int().min(0).max(3_600_000).optional(),
query_timeout_ms: z.number().int().min(1).max(3_600_000).optional(),
})
.strict()

Expand Down Expand Up @@ -61,12 +62,24 @@ const publicParameters = z
right_col: z.string().optional().describe("Right output column for keys"),
left_warehouse: z.string().optional().describe("Optional assertion: must equal warehouse"),
right_warehouse: z.string().optional().describe("Optional assertion: must equal warehouse"),
query_timeout_ms: z.number().int().min(0).max(3_600_000).optional(),
query_timeout_ms: z
.number()
.int()
.min(1)
.max(3_600_000)
.optional()
.describe("Query deadline in ms; omitted or 0 uses the common connection policy"),
})
.strict()

export type SqlCheckPublicInput = z.input<typeof publicParameters>

// Coverage requires a pattern, and an empty pattern is meaningful for that check.
const toolParameters = z.discriminatedUnion("check", [
publicParameters.extend({ check: z.literal("coverage"), pattern: publicParameters.shape.pattern.unwrap() }),
publicParameters.extend({ check: z.enum(["fanout", "keys", "grain", "nulls"]) }),
])

/**
* Treat "", [] and null as absent so strict-schema callers can leave unused fields empty.
* A coverage check's `pattern` is the one field where "" is a value (match empty strings).
Expand Down Expand Up @@ -440,7 +453,9 @@ export const SqlCheckTool = Tool.define("sql_check", {
"Use sql_materialize to put cross-warehouse or other-engine sources into scratch first. " +
"Key normalization is diagnostic and may collapse unrelated values; no automatic fixes. Coverage uses regex except SQLite's explicit LIKE fallback. " +
"Leave fields a check does not use empty.",
parameters: publicParameters,
parameters: toolParameters,
// Strict-schema providers require a flat object at the root of the wire schema.
jsonSchema: zodToJsonSchema(publicParameters),
async execute(args, ctx) {
try {
// altimate_change start — validate and authorize reads before workspace redirects
Expand Down
10 changes: 5 additions & 5 deletions packages/opencode/src/altimate/tools/sql-execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import { normalizeError } from "./response-normalization"
import { readSql } from "./sql-check"

const MAX_SQL_ROWS = 10_000
const LIMIT_ERROR = `limit must be a finite positive integer between 1 and ${MAX_SQL_ROWS}`
const LIMIT_ERROR = "limit must be a finite positive integer"
const MAX_GENERATED_SQL_BYTES = 1024 * 1024
// Below the default tool-output cap, so a redirect never spills the SQL into a truncated file.
const MAX_REDIRECT_SQL_BYTES = 40 * 1024
Expand Down Expand Up @@ -71,18 +71,18 @@ const parameters = z.object({
.number({ error: LIMIT_ERROR })
.int(LIMIT_ERROR)
.min(1, LIMIT_ERROR)
.max(MAX_SQL_ROWS, LIMIT_ERROR)
.transform((limit) => Math.min(limit, MAX_SQL_ROWS))
.optional()
.default(100)
.describe(`Max rows to return (1-${MAX_SQL_ROWS}; default 100)`),
.describe(`Max rows to return (default 100; larger values are capped at ${MAX_SQL_ROWS})`),
query_timeout_ms: z
.number()
.int()
.min(0)
.min(1)
.max(3_600_000)
.optional()
.describe(
"Optional per-query execution deadline in ms. Omitted or 0 uses the configured default. Bounded by the remaining run budget.",
"Optional per-query execution deadline in ms. Omit or use 0 to use the configured deadline (connection setting or environment), bounded by the remaining run budget. Set a positive value only when a specific query needs a longer or shorter budget.",
),
})

Expand Down
5 changes: 2 additions & 3 deletions packages/opencode/src/altimate/tools/sql-materialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,16 +465,15 @@ export const SqlMaterializeTool = Tool.define("sql_materialize", {
.number()
.int()
.min(0)
.max(HARD_MAX_BYTES)
.optional()
.describe("Serialized row byte cap enforced during transfer (default 256 MiB, hard limit 2 GiB; 0 = default)"),
query_timeout_ms: z
.number()
.int()
.min(0)
.min(1)
.max(3_600_000)
.optional()
.describe("Query deadline in ms; omitted or 0 uses the configured default, bounded by the remaining run budget"),
.describe("Source query deadline in ms; omitted or 0 uses the configured default, bounded by the remaining run budget"),
}),
async execute(args, ctx): Promise<{ title: string; metadata: Record<string, any>; output: string }> {
// altimate_change start — validate and authorize source reads before workspace redirects
Expand Down
62 changes: 62 additions & 0 deletions packages/opencode/src/tool/filler-parameters.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import type { JSONSchema7 } from "@ai-sdk/provider"
import { Result, Schema, SchemaGetter } from "effect"

const normalized = new WeakSet<Schema.Top>()

/** Retry a rejected call with optional fillers removed; required values and nested payloads retain their meaning. */
export function fillerSafeParameters<T>(parameters: Schema.Decoder<T>, jsonSchema: JSONSchema7): Schema.Decoder<T> {
// Legacy tools may publish a simpler wire schema than their validation schema.
if (normalized.has(parameters)) return parameters
const decodeResult = Schema.decodeUnknownResult(parameters)
const stripped = Schema.Unknown.pipe(
Schema.decodeTo(parameters, {
decode: SchemaGetter.transform((input) => {
if (!input || typeof input !== "object" || Array.isArray(input)) return input
const args = input as Record<string, unknown>
const properties = new Set<string>()
const required = new Set<string>()
const collect = (schema: JSONSchema7 | boolean) => {
if (typeof schema === "boolean") return
for (const name of Object.keys(schema.properties ?? {})) properties.add(name)
for (const name of schema.required ?? []) required.add(name)
Comment thread
anandgupta42 marked this conversation as resolved.
for (const child of schema.allOf ?? []) collect(child)
for (const child of schema.anyOf ?? schema.oneOf ?? []) {
// Literal discriminants determine which arguments are required for this call.
if (
typeof child !== "boolean" &&
Object.entries(child.properties ?? {}).some(
([key, field]) =>
typeof field !== "boolean" &&
child.required?.includes(key) &&
key in args &&
((field.const !== undefined && field.const !== args[key]) ||
(field.enum !== undefined && !field.enum.includes(args[key] as never))),
)
)
continue
collect(child)
}
}
collect(jsonSchema)
const isFiller = (key: string, value: unknown) =>
properties.has(key) &&
!required.has(key) &&
(value === null || value === "" || value === 0 || (Array.isArray(value) && value.length === 0))
const base = Object.fromEntries(Object.entries(args).filter(([key, value]) => !isFiller(key, value)))
// Keep each filler that is valid on its own so intentional siblings survive an unrelated filler.
const valid = (candidate: Record<string, unknown>) => Result.isSuccess(decodeResult(candidate))
Comment thread
anandgupta42 marked this conversation as resolved.
const kept = Object.entries(args).filter(
([key, value]) => isFiller(key, value) && valid({ ...base, [key]: value }),
)
const merged = { ...base, ...Object.fromEntries(kept) }
return kept.length > 0 && valid(merged) ? merged : base
Comment thread
anandgupta42 marked this conversation as resolved.
}),
encode: SchemaGetter.transform((value) => value),
}),
)
// Validate the arguments as sent first so intentional zero/empty values survive; only a call the
// schema rejects is retried with optional fillers removed.
const result = Schema.Union([parameters, stripped]) as unknown as Schema.Decoder<T>
Comment thread
anandgupta42 marked this conversation as resolved.
normalized.add(result)
return result
}
41 changes: 32 additions & 9 deletions packages/opencode/src/tool/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ import { AppRuntime } from "@/effect/app-runtime"
// altimate_change start — upstream_fix: task.background schema follows RuntimeFlags.
import { RuntimeFlags } from "@/effect/runtime-flags"
// altimate_change end
// altimate_change start — upstream_fix: plugin tools share the optional filler contract
import { Result, Schema } from "effect"
import { fillerSafeParameters } from "./filler-parameters"
import { ToolJsonSchema } from "./json-schema"
// altimate_change end
// altimate_change start — Effect Context.Service facade so v1.17.9 consumers that compose
// ToolRegistry into the Effect runtime (yield* ToolRegistry.Service / .defaultLayer / .node)
// compile. Delegates to the existing namespace functions; behavior preserved.
Expand Down Expand Up @@ -157,6 +162,30 @@ import { TrainingImportTool } from "../altimate/tools/training-import"
import { DbtPrReviewTool } from "../altimate/tools/dbt-pr-review"
// altimate_change end

// altimate_change start — upstream_fix: plugin tools share the optional filler contract
function fillerSafePlugin(tool: Tool.DefWithoutID): Tool.DefWithoutID {
const schema = ToolJsonSchema.fromTool({ ...tool, id: "plugin" })
const parameters = fillerSafeParameters(tool.parameters, schema)
const decode = Schema.decodeUnknownResult(parameters)
const known = new Set(Object.keys(schema.properties ?? {}))
return {
...tool,
parameters,
// Plugin handlers historically received raw arguments: use the normalized form only when it
// validates, and keep any arguments the schema does not list.
execute: (args, ctx) => {
const decoded = decode(args)
if (Result.isFailure(decoded) || !decoded.success || typeof decoded.success !== "object")
return tool.execute(args, ctx)
const extra = Object.fromEntries(
Object.entries(args as Record<string, unknown>).filter(([key]) => !known.has(key)),
)
return tool.execute({ ...extra, ...(decoded.success as object) }, ctx)
},
}
}
// altimate_change end

export namespace ToolRegistry {
const log = Log.create({ service: "tool.registry" })

Expand Down Expand Up @@ -275,7 +304,7 @@ export namespace ToolRegistry {
...(result.attachments ? { attachments: result.attachments as any } : {}),
}
},
}),
}).pipe(Effect.map(fillerSafePlugin)),
}
}
// altimate_change end
Expand Down Expand Up @@ -522,10 +551,7 @@ export namespace ToolRegistry {
return all(plugins, configInput)
}

export async function ids(
plugins?: Awaited<ReturnType<typeof Plugin.list>>,
configInput?: RegistryConfigInput,
) {
export async function ids(plugins?: Awaited<ReturnType<typeof Plugin.list>>, configInput?: RegistryConfigInput) {
return all(plugins, configInput).then((x) => x.map((t) => t.id))
}
// altimate_change end
Expand Down Expand Up @@ -645,10 +671,7 @@ export namespace ToolRegistry {
})
: Effect.succeed(undefined)
const bridgeWithPlugins = <T>(
fn: (
plugins?: Awaited<ReturnType<typeof Plugin.list>>,
configInput?: RegistryConfigInput,
) => Promise<T>,
fn: (plugins?: Awaited<ReturnType<typeof Plugin.list>>, configInput?: RegistryConfigInput) => Promise<T>,
): Effect.Effect<T> =>
Effect.gen(function* () {
const plugins = yield* resolvePlugins()
Expand Down
16 changes: 14 additions & 2 deletions packages/opencode/src/tool/tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import {
// altimate_change start — humanize tool-call titles at the source
import { describeToolCall } from "../altimate/tool-label"
// altimate_change end
// altimate_change start — upstream_fix: normalize optional strict-schema fillers before validation
import { fillerSafeParameters } from "./filler-parameters"
import { ToolJsonSchema } from "./json-schema"
// altimate_change end

interface Metadata {
[key: string]: any
Expand Down Expand Up @@ -269,9 +273,15 @@ function wrap<Parameters extends Schema.Decoder<unknown>, Result extends Metadat
Effect.gen(function* () {
const toolInfo = typeof init === "function" ? { ...(yield* init(ctx)) } : { ...init }
// altimate_change end
// altimate_change start — upstream_fix: share filler handling across all tool schemas
const jsonSchema = ToolJsonSchema.fromTool({ ...toolInfo, id })
const parameters = fillerSafeParameters(toolInfo.parameters, jsonSchema)
// altimate_change end
// Compile the parser closure once per tool init; `decodeUnknownEffect`
// allocates a new closure per call, so hoisting avoids re-closing it.
const decode = Schema.decodeUnknownEffect(toolInfo.parameters)
// altimate_change start — upstream_fix: validate after optional fillers become omissions
const decode = Schema.decodeUnknownEffect(parameters)
// altimate_change end
const execute = toolInfo.execute
toolInfo.execute = (args, ctx) => {
const attrs = {
Expand Down Expand Up @@ -333,7 +343,9 @@ function wrap<Parameters extends Schema.Decoder<unknown>, Result extends Metadat
}
}).pipe(Effect.orDie, Effect.withSpan("Tool.execute", { attributes: attrs }))
}
return toolInfo
// altimate_change start — upstream_fix: expose the same decoder to schema and batch callers
return { ...toolInfo, parameters, jsonSchema }
Comment thread
anandgupta42 marked this conversation as resolved.
Comment thread
anandgupta42 marked this conversation as resolved.
// altimate_change end
})
}

Expand Down
Loading
Loading