Skip to content
Closed
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
111 changes: 111 additions & 0 deletions packages/opencode/test/id/id.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { describe, test, expect } from "bun:test"
import { Identifier } from "../../src/id/id"

describe("Identifier: prefix format and length", () => {
test("ascending() generates ID with correct prefix", () => {
const id = Identifier.ascending("session")
expect(id).toMatch(/^ses_/)
})

test("descending() generates ID with correct prefix", () => {
const id = Identifier.descending("message")
expect(id).toMatch(/^msg_/)
})

test("ID has expected total length (prefix + _ + 26 hex/base62 chars)", () => {
// "ses" (3) + "_" (1) + 26 = 30
const id = Identifier.ascending("session")
expect(id.length).toBe(30)
})

test("tool prefix is 4 chars (outlier)", () => {
// "tool" (4) + "_" (1) + 26 = 31
const id = Identifier.ascending("tool")
expect(id).toMatch(/^tool_/)
expect(id.length).toBe(31)
})
})

describe("Identifier: ascending sort order", () => {
test("IDs with increasing timestamps sort ascending (string order)", () => {
const t = 1700000000000
const a = Identifier.create("session", false, t)
const b = Identifier.create("session", false, t + 1)
expect(a < b).toBe(true)
})

test("multiple IDs at same timestamp are unique and ascending", () => {
const t = 1700000001000
const ids = Array.from({ length: 10 }, () => Identifier.create("session", false, t))
const unique = new Set(ids)
expect(unique.size).toBe(10)
for (let i = 1; i < ids.length; i++) {
expect(ids[i - 1] < ids[i]).toBe(true)
}
})
})

describe("Identifier: descending sort order", () => {
test("IDs with increasing timestamps sort descending (string order)", () => {
const t = 1700000002000
const a = Identifier.create("session", true, t)
const b = Identifier.create("session", true, t + 1)
// Later timestamp → smaller string for descending
expect(a > b).toBe(true)
})
})

describe("Identifier: timestamp comparison", () => {
test("timestamp() preserves relative ordering for ascending IDs", () => {
const t1 = 1700000003000
const t2 = 1700000004000
const id1 = Identifier.create("session", false, t1)
const id2 = Identifier.create("session", false, t2)
// timestamp() may not recover the exact input due to 48-bit storage,
// but it must preserve relative ordering (used for cleanup cutoffs)
expect(Identifier.timestamp(id1)).toBeLessThan(Identifier.timestamp(id2))
})

test("timestamp() returns same value for IDs created at same time", () => {
const t = 1700000005000
const id1 = Identifier.create("session", false, t)
const id2 = Identifier.create("session", false, t)
// Both IDs at same timestamp should produce the same (or very close) extracted timestamp
// The counter increment adds at most a few units that divide away
expect(Identifier.timestamp(id1)).toBe(Identifier.timestamp(id2))
})
})

describe("Identifier: given passthrough", () => {
test("returns given ID as-is when prefix matches", () => {
const given = "ses_abcdef1234567890abcdef1234"
const result = Identifier.ascending("session", given)
expect(result).toBe(given)
})

test("throws when given ID has wrong prefix", () => {
expect(() => Identifier.ascending("session", "msg_abc")).toThrow(
"does not start with ses",
)
})
})

describe("Identifier: schema validation", () => {
test("schema accepts valid session ID", () => {
const s = Identifier.schema("session")
const id = Identifier.ascending("session")
expect(s.safeParse(id).success).toBe(true)
})

test("schema rejects ID with wrong prefix", () => {
const s = Identifier.schema("session")
expect(s.safeParse("msg_abc123").success).toBe(false)
})

test("schema for tool prefix works (4-char prefix)", () => {
const s = Identifier.schema("tool")
const id = Identifier.ascending("tool")
expect(s.safeParse(id).success).toBe(true)
expect(s.safeParse("ses_abc").success).toBe(false)
})
})
96 changes: 96 additions & 0 deletions packages/opencode/test/shell/shell.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { describe, test, expect, beforeEach, afterEach } from "bun:test"
import { Shell } from "../../src/shell/shell"

describe("Shell.acceptable: blacklist enforcement", () => {
let savedShell: string | undefined

beforeEach(() => {
savedShell = process.env.SHELL
// Reset the lazy caches so each test starts fresh
Shell.acceptable.reset()
Shell.preferred.reset()
})

afterEach(() => {
if (savedShell !== undefined) {
process.env.SHELL = savedShell
} else {
delete process.env.SHELL
}
Shell.acceptable.reset()
Shell.preferred.reset()
})

test("returns SHELL when set to bash", () => {
process.env.SHELL = "/bin/bash"
expect(Shell.acceptable()).toBe("/bin/bash")
})

test("returns SHELL when set to zsh", () => {
process.env.SHELL = "/usr/bin/zsh"
expect(Shell.acceptable()).toBe("/usr/bin/zsh")
})

test("rejects fish and returns fallback", () => {
process.env.SHELL = "/usr/bin/fish"
const result = Shell.acceptable()
expect(result).not.toBe("/usr/bin/fish")
// Fallback should be a real shell path
expect(result.length).toBeGreaterThan(0)
})

test("rejects nu (nushell) and returns fallback", () => {
process.env.SHELL = "/usr/bin/nu"
const result = Shell.acceptable()
expect(result).not.toBe("/usr/bin/nu")
expect(result.length).toBeGreaterThan(0)
})

test("shell containing 'nu' in name but not basename is not blacklisted", () => {
// /opt/menu/bin/bash — basename is "bash", not "nu"
process.env.SHELL = "/opt/nushell/bin/bash"
expect(Shell.acceptable()).toBe("/opt/nushell/bin/bash")
})

test("returns fallback when SHELL is unset", () => {
delete process.env.SHELL
const result = Shell.acceptable()
expect(result.length).toBeGreaterThan(0)
// On Linux/macOS, fallback should be a valid shell path
expect(result).toMatch(/\/(bash|zsh|sh|cmd\.exe)$/)
})
Comment on lines +55 to +61
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

Regex won't match Windows cmd.exe paths.

The comment on line 59 says "On Linux/macOS" but the regex includes cmd\.exe. Windows paths use backslashes (e.g., C:\Windows\System32\cmd.exe), so the pattern requiring a forward slash will never match cmd.exe.

Either remove cmd\.exe from the pattern to align with the comment, or adjust the regex to handle both path separators:

Option 1: Remove cmd.exe (match comment scope)
-    expect(result).toMatch(/\/(bash|zsh|sh|cmd\.exe)$/)
+    expect(result).toMatch(/\/(bash|zsh|sh)$/)
Option 2: Handle both separators
-    expect(result).toMatch(/\/(bash|zsh|sh|cmd\.exe)$/)
+    expect(result).toMatch(/[/\\](bash|zsh|sh|cmd\.exe)$/)
📝 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("returns fallback when SHELL is unset", () => {
delete process.env.SHELL
const result = Shell.acceptable()
expect(result.length).toBeGreaterThan(0)
// On Linux/macOS, fallback should be a valid shell path
expect(result).toMatch(/\/(bash|zsh|sh|cmd\.exe)$/)
})
test("returns fallback when SHELL is unset", () => {
delete process.env.SHELL
const result = Shell.acceptable()
expect(result.length).toBeGreaterThan(0)
// On Linux/macOS, fallback should be a valid shell path
expect(result).toMatch(/[/\\](bash|zsh|sh|cmd\.exe)$/)
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/shell/shell.test.ts` around lines 55 - 61, The test
"returns fallback when SHELL is unset" calls Shell.acceptable() and currently
uses a regex requiring a leading forward slash but also includes "cmd\.exe",
which can never match Windows paths with backslashes; update the test to either
(a) limit the assertion to POSIX shells by removing "cmd\.exe" from the regex,
or (b) handle platforms explicitly by branching on process.platform and
asserting a POSIX-style regex (/\/(bash|zsh|sh)$/) on 'linux'/'darwin' and a
Windows-style regex (e.g., /\\(?:Windows\\)?(?:System32\\)?cmd\.exe$/ or
checking for 'cmd.exe' via /cmd\.exe$/) on 'win32'; modify the test for "returns
fallback when SHELL is unset" and reference Shell.acceptable() so it validates
the correct path format per platform.

})

describe("Shell.preferred: no blacklist filtering", () => {
let savedShell: string | undefined

beforeEach(() => {
savedShell = process.env.SHELL
Shell.preferred.reset()
})

afterEach(() => {
if (savedShell !== undefined) {
process.env.SHELL = savedShell
} else {
delete process.env.SHELL
}
Shell.preferred.reset()
})

test("returns SHELL even when blacklisted (fish)", () => {
process.env.SHELL = "/usr/bin/fish"
expect(Shell.preferred()).toBe("/usr/bin/fish")
})

test("returns SHELL even when blacklisted (nu)", () => {
process.env.SHELL = "/usr/bin/nu"
expect(Shell.preferred()).toBe("/usr/bin/nu")
})

test("returns fallback when SHELL is unset", () => {
delete process.env.SHELL
const result = Shell.preferred()
expect(result.length).toBeGreaterThan(0)
})
})
Loading