Merge pull request #3748 from code-yeongyu/fix/simple-github-bugs-1948-3564
fix: resolve simple triaged GitHub bugs
This commit is contained in:
@@ -0,0 +1,35 @@
|
|||||||
|
/// <reference types="bun-types" />
|
||||||
|
|
||||||
|
import { afterEach, describe, expect, it, mock } from "bun:test"
|
||||||
|
|
||||||
|
const originalWhich = Bun.which
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
Bun.which = originalWhich
|
||||||
|
mock.restore()
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("getGhCliInfo", () => {
|
||||||
|
it("falls back to gh --version when Bun.which cannot find gh", async () => {
|
||||||
|
// given
|
||||||
|
Bun.which = mock(() => null)
|
||||||
|
mock.module("../spawn-with-timeout", () => ({
|
||||||
|
spawnWithTimeout: mock((command: string[]) => {
|
||||||
|
if (command.join(" ") === "gh --version") {
|
||||||
|
return Promise.resolve({ stdout: "gh version 2.82.1\n", stderr: "", exitCode: 0, timedOut: false })
|
||||||
|
}
|
||||||
|
|
||||||
|
return Promise.resolve({ stdout: "", stderr: "not logged in", exitCode: 1, timedOut: false })
|
||||||
|
}),
|
||||||
|
}))
|
||||||
|
const { getGhCliInfo } = await import("./tools-gh")
|
||||||
|
|
||||||
|
// when
|
||||||
|
const info = await getGhCliInfo()
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(info.installed).toBe(true)
|
||||||
|
expect(info.version).toBe("2.82.1")
|
||||||
|
expect(info.path).toBe(null)
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -80,6 +80,20 @@ async function getGhAuthStatus(): Promise<{
|
|||||||
export async function getGhCliInfo(): Promise<GhCliInfo> {
|
export async function getGhCliInfo(): Promise<GhCliInfo> {
|
||||||
const binaryStatus = await checkBinaryExists("gh")
|
const binaryStatus = await checkBinaryExists("gh")
|
||||||
if (!binaryStatus.exists) {
|
if (!binaryStatus.exists) {
|
||||||
|
const version = await getGhVersion()
|
||||||
|
if (version) {
|
||||||
|
const authStatus = await getGhAuthStatus()
|
||||||
|
return {
|
||||||
|
installed: true,
|
||||||
|
version,
|
||||||
|
path: null,
|
||||||
|
authenticated: authStatus.authenticated,
|
||||||
|
username: authStatus.username,
|
||||||
|
scopes: authStatus.scopes,
|
||||||
|
error: authStatus.error,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
installed: false,
|
installed: false,
|
||||||
version: null,
|
version: null,
|
||||||
|
|||||||
@@ -6,7 +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 { normalizeToolArgSchemas } from "./normalize-tool-arg-schemas"
|
import { normalizeToolArgSchemas, sanitizeJsonSchema } from "./normalize-tool-arg-schemas"
|
||||||
|
|
||||||
const tempDirectories: string[] = []
|
const tempDirectories: string[] = []
|
||||||
|
|
||||||
@@ -95,3 +95,36 @@ describe("normalizeToolArgSchemas", () => {
|
|||||||
expect(afterQuery?.examples).toEqual(["issue 2314"])
|
expect(afterQuery?.examples).toEqual(["issue 2314"])
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("sanitizeJsonSchema", () => {
|
||||||
|
it("rewrites bare $ref values to $defs JSON pointers", () => {
|
||||||
|
// given
|
||||||
|
const schema = {
|
||||||
|
type: "object",
|
||||||
|
properties: {
|
||||||
|
new_encoding: { $ref: "Encoding" },
|
||||||
|
existing_pointer: { $ref: "#/$defs/AlreadyValid" },
|
||||||
|
},
|
||||||
|
$defs: {
|
||||||
|
Encoding: { type: "string" },
|
||||||
|
AlreadyValid: { type: "string" },
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
// when
|
||||||
|
const sanitized = sanitizeJsonSchema(schema)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(sanitized).toEqual({
|
||||||
|
type: "object",
|
||||||
|
properties: {
|
||||||
|
new_encoding: { $ref: "#/$defs/Encoding" },
|
||||||
|
existing_pointer: { $ref: "#/$defs/AlreadyValid" },
|
||||||
|
},
|
||||||
|
$defs: {
|
||||||
|
Encoding: { type: "string" },
|
||||||
|
AlreadyValid: { type: "string" },
|
||||||
|
},
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|||||||
@@ -47,6 +47,14 @@ function isRecord(value: unknown): value is Record<string, unknown> {
|
|||||||
return typeof value === "object" && value !== null && !Array.isArray(value)
|
return typeof value === "object" && value !== null && !Array.isArray(value)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function normalizeJsonSchemaRef(value: string): string {
|
||||||
|
if (value.startsWith("#") || value.includes(":") || value.startsWith("/")) {
|
||||||
|
return value
|
||||||
|
}
|
||||||
|
|
||||||
|
return `#/$defs/${value}`
|
||||||
|
}
|
||||||
|
|
||||||
export function sanitizeJsonSchema(value: unknown, depth = 0, isPropertyName = false): unknown {
|
export function sanitizeJsonSchema(value: unknown, depth = 0, isPropertyName = false): unknown {
|
||||||
if (Array.isArray(value)) {
|
if (Array.isArray(value)) {
|
||||||
return value.map((item) => sanitizeJsonSchema(item, depth + 1, false))
|
return value.map((item) => sanitizeJsonSchema(item, depth + 1, false))
|
||||||
@@ -67,6 +75,11 @@ export function sanitizeJsonSchema(value: unknown, depth = 0, isPropertyName = f
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!isPropertyName && key === "$ref" && typeof nestedValue === "string") {
|
||||||
|
sanitized[key] = normalizeJsonSchemaRef(nestedValue)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
const childIsPropertyName = key === "properties" && !isPropertyName
|
const childIsPropertyName = key === "properties" && !isPropertyName
|
||||||
sanitized[key] = sanitizeJsonSchema(nestedValue, depth + 1, childIsPropertyName)
|
sanitized[key] = sanitizeJsonSchema(nestedValue, depth + 1, childIsPropertyName)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -136,6 +136,19 @@ describe("createCallOmoAgent", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("dynamic custom agent resolution", () => {
|
describe("dynamic custom agent resolution", () => {
|
||||||
|
test("should reject missing subagent_type without throwing", async () => {
|
||||||
|
const mockCtx = createMockCtx(DEFAULT_AGENTS)
|
||||||
|
const toolDef = createCallOmoAgent(mockCtx, mockBackgroundManager, [])
|
||||||
|
const executeFunc = toolDef.execute as Function
|
||||||
|
|
||||||
|
const result = await executeFunc(
|
||||||
|
{ description: "Test", prompt: "Fix bug", run_in_background: true },
|
||||||
|
toolCtx
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(result).toContain("subagent_type is required")
|
||||||
|
})
|
||||||
|
|
||||||
test("should accept a custom agent returned by client.app.agents()", async () => {
|
test("should accept a custom agent returned by client.app.agents()", async () => {
|
||||||
const agents = [...DEFAULT_AGENTS, { name: "bug-fixer", mode: "subagent" }]
|
const agents = [...DEFAULT_AGENTS, { name: "bug-fixer", mode: "subagent" }]
|
||||||
const mockCtx = createMockCtx(agents)
|
const mockCtx = createMockCtx(agents)
|
||||||
|
|||||||
@@ -140,6 +140,10 @@ export function createCallOmoAgent(
|
|||||||
`[call_omo_agent] Starting with agent: ${args.subagent_type}, background: ${args.run_in_background}`,
|
`[call_omo_agent] Starting with agent: ${args.subagent_type}, background: ${args.run_in_background}`,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
if (typeof args.subagent_type !== "string" || args.subagent_type.trim() === "") {
|
||||||
|
return "Error: subagent_type is required."
|
||||||
|
}
|
||||||
|
|
||||||
const callableAgents = await resolveCallableAgents(ctx.client);
|
const callableAgents = await resolveCallableAgents(ctx.client);
|
||||||
|
|
||||||
// Strip ZWSP and case-insensitive agent validation - allows "Explore", "EXPLORE", "explore" etc.
|
// Strip ZWSP and case-insensitive agent validation - allows "Explore", "EXPLORE", "explore" etc.
|
||||||
|
|||||||
Reference in New Issue
Block a user