Skip to content
Open
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
84 changes: 84 additions & 0 deletions packages/opencode/test/altimate/sql-analyze-format.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { describe, test, expect } from "bun:test"
import { Dispatcher } from "../../src/altimate/native"
import { registerAllSql } from "../../src/altimate/native/sql/register"
import type { SqlAnalyzeResult } from "../../src/altimate/native/types"

// Ensure sql.analyze is registered
registerAllSql()

// ---------------------------------------------------------------------------
// sql.analyze Dispatcher — success semantics and result shape
//
// The AI-5975 fix changed success semantics: finding issues IS a successful
// analysis (success:true). Previously it returned success:false when issues
// were found, causing ~4,000 false "unknown error" telemetry entries per day.
// ---------------------------------------------------------------------------

describe("sql.analyze: success semantics (AI-5975 regression)", () => {
test("query with lint issues still returns success:true", async () => {
// SELECT * is a known lint trigger — must still be a successful analysis
const result = await Dispatcher.call("sql.analyze", {
sql: "SELECT * FROM users",
dialect: "snowflake",
}) as SqlAnalyzeResult
// KEY INVARIANT: finding issues is a SUCCESSFUL analysis
expect(result.success).toBe(true)
// Verify issues were actually found (not a vacuous pass)
expect(result.issue_count).toBeGreaterThan(0)
expect(result.confidence).toBe("high")
})

test("issue_count matches issues array length", async () => {
const result = await Dispatcher.call("sql.analyze", {
sql: "SELECT * FROM orders JOIN customers",
dialect: "snowflake",
}) as SqlAnalyzeResult
expect(result.issues.length).toBeGreaterThan(0)
expect(result.issue_count).toBe(result.issues.length)
})
})

describe("sql.analyze: issue structure", () => {
test("lint issues have required fields", async () => {
const result = await Dispatcher.call("sql.analyze", {
sql: "SELECT * FROM users",
dialect: "snowflake",
}) as SqlAnalyzeResult
const lintIssues = result.issues.filter((i) => i.type === "lint")
// Guard against vacuous pass — SELECT * must produce lint findings
expect(lintIssues.length).toBeGreaterThan(0)
for (const issue of lintIssues) {
expect(issue.severity).toBeDefined()
expect(issue.message).toBeDefined()
expect(typeof issue.recommendation).toBe("string")
expect(issue.confidence).toBe("high")
}
})

test("issue types are limited to lint, semantic, safety", async () => {
const result = await Dispatcher.call("sql.analyze", {
sql: "SELECT * FROM users WHERE 1=1",
dialect: "snowflake",
}) as SqlAnalyzeResult
expect(result.issues.length).toBeGreaterThan(0)
const validTypes = ["lint", "semantic", "safety"]
for (const issue of result.issues) {
expect(validTypes).toContain(issue.type)
}
})
})

describe("sql.analyze: result shape", () => {
test("successful result has all required properties", async () => {
const result = await Dispatcher.call("sql.analyze", {
sql: "SELECT 1 LIMIT 1",
}) as SqlAnalyzeResult
expect(result).toHaveProperty("success")
expect(result).toHaveProperty("issues")
expect(result).toHaveProperty("issue_count")
expect(result).toHaveProperty("confidence")
expect(result).toHaveProperty("confidence_factors")
expect(Array.isArray(result.issues)).toBe(true)
expect(Array.isArray(result.confidence_factors)).toBe(true)
})
Comment on lines +72 to +83
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Assert success-path explicitly in the “successful result” test.

The current assertions validate shape only; the failure path also has the same top-level shape. Add an explicit success assertion so this test enforces what its name promises.

✅ Suggested patch
   test("successful result has all required properties", async () => {
     const result = await Dispatcher.call("sql.analyze", {
       sql: "SELECT 1 LIMIT 1",
     }) as SqlAnalyzeResult
+    expect(result.success).toBe(true)
+    expect(result.error).toBeUndefined()
     expect(result).toHaveProperty("success")
     expect(result).toHaveProperty("issues")
     expect(result).toHaveProperty("issue_count")
     expect(result).toHaveProperty("confidence")
     expect(result).toHaveProperty("confidence_factors")
     expect(Array.isArray(result.issues)).toBe(true)
     expect(Array.isArray(result.confidence_factors)).toBe(true)
   })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test("successful result has all required properties", async () => {
const result = await Dispatcher.call("sql.analyze", {
sql: "SELECT 1 LIMIT 1",
}) as SqlAnalyzeResult
expect(result).toHaveProperty("success")
expect(result).toHaveProperty("issues")
expect(result).toHaveProperty("issue_count")
expect(result).toHaveProperty("confidence")
expect(result).toHaveProperty("confidence_factors")
expect(Array.isArray(result.issues)).toBe(true)
expect(Array.isArray(result.confidence_factors)).toBe(true)
})
test("successful result has all required properties", async () => {
const result = await Dispatcher.call("sql.analyze", {
sql: "SELECT 1 LIMIT 1",
}) as SqlAnalyzeResult
expect(result.success).toBe(true)
expect(result.error).toBeUndefined()
expect(result).toHaveProperty("success")
expect(result).toHaveProperty("issues")
expect(result).toHaveProperty("issue_count")
expect(result).toHaveProperty("confidence")
expect(result).toHaveProperty("confidence_factors")
expect(Array.isArray(result.issues)).toBe(true)
expect(Array.isArray(result.confidence_factors)).toBe(true)
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/altimate/sql-analyze-format.test.ts` around lines 72 -
83, The test "successful result has all required properties" currently only
checks the shape of SqlAnalyzeResult returned by Dispatcher.call("sql.analyze")
but doesn't assert the success path; update the test to explicitly assert that
result.success is true (e.g., expect(result.success).toBe(true)) after the call
so the test actually enforces a successful result rather than merely the
response shape; keep the existing shape checks for result.issues and
result.confidence_factors.

})
94 changes: 94 additions & 0 deletions packages/opencode/test/command/hints-discover-mcps.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { describe, test, expect } from "bun:test"
import { Command } from "../../src/command/index"
import { Instance } from "../../src/project/instance"
import { tmpdir } from "../fixture/fixture"

// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------

async function withInstance(fn: () => Promise<void>) {
await using tmp = await tmpdir({ git: true })
await Instance.provide({ directory: tmp.path, fn })
}

// ---------------------------------------------------------------------------
// Command.hints() — template hint extraction
// ---------------------------------------------------------------------------

describe("Command.hints()", () => {
test("extracts and sorts placeholders lexicographically", () => {
// Lexicographic sort means $10 sorts before $2 — this documents
// the actual contract (not numeric ordering)
const result = Command.hints("Use $10 then $2 and $1")
expect(result).toEqual(["$1", "$10", "$2"])
})

test("extracts $ARGUMENTS placeholder", () => {
const result = Command.hints("Run with $ARGUMENTS")
expect(result).toEqual(["$ARGUMENTS"])
})

test("extracts both numbered and $ARGUMENTS", () => {
const result = Command.hints("Use $1 and $ARGUMENTS")
expect(result).toEqual(["$1", "$ARGUMENTS"])
})

test("deduplicates repeated numbered placeholders", () => {
const result = Command.hints("$1 and $1 then $2")
expect(result).toEqual(["$1", "$2"])
})

test("returns empty array for template with no placeholders", () => {
const result = Command.hints("No placeholders here")
expect(result).toEqual([])
})

test("includes $0 as a valid placeholder", () => {
const result = Command.hints("$0 $1")
expect(result).toEqual(["$0", "$1"])
})
})

// ---------------------------------------------------------------------------
// discover-and-add-mcps builtin command (#409)
// ---------------------------------------------------------------------------

describe("discover-and-add-mcps builtin command", () => {
test("is registered in Command.Default constants", () => {
expect(Command.Default.DISCOVER_MCPS).toBe("discover-and-add-mcps")
})

test("is present in command list", async () => {
await withInstance(async () => {
const commands = await Command.list()
const names = commands.map((c) => c.name)
expect(names).toContain("discover-and-add-mcps")
})
})

test("has correct metadata", async () => {
await withInstance(async () => {
const cmd = await Command.get("discover-and-add-mcps")
expect(cmd).toBeDefined()
expect(cmd.name).toBe("discover-and-add-mcps")
expect(cmd.source).toBe("command")
expect(cmd.description).toContain("MCP")
})
})

test("template references mcp_discover tool", async () => {
await withInstance(async () => {
const cmd = await Command.get("discover-and-add-mcps")
const template = await cmd.template
expect(template).toContain("mcp_discover")
})
})

test("is not a subtask", async () => {
await withInstance(async () => {
const cmd = await Command.get("discover-and-add-mcps")
expect(cmd.subtask).toBeUndefined()
})
})
})
1 change: 1 addition & 0 deletions packages/opencode/test/fixture/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export async function tmpdir<T>(options?: TmpDirOptions<T>) {
await $`git config core.fsmonitor false`.cwd(dirpath).quiet()
await $`git config user.email "test@opencode.test"`.cwd(dirpath).quiet()
await $`git config user.name "Test"`.cwd(dirpath).quiet()
await $`git config commit.gpgsign false`.cwd(dirpath).quiet()
await $`git commit --allow-empty -m "root commit ${dirpath}"`.cwd(dirpath).quiet()
}
if (options?.config) {
Expand Down
Loading