Skip to content

fix(test): replace Unix-only assumptions with cross-platform alternatives #14906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Hona merged 1 commit into from
Feb 24, 2026
Merged

fix(test): replace Unix-only assumptions with cross-platform alternatives #14906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 6 additions & 4 deletions packages/opencode/test/tool/bash.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { describe, expect, test } from "bun:test"
import os from "os"
import path from "path"
import { BashTool } from "../../src/tool/bash"
import { Instance } from "../../src/project/instance"
Expand Down Expand Up @@ -138,14 +139,14 @@ describe("tool.bash permissions", () => {
await bash.execute(
{
command: "ls",
workdir: "/tmp",
description: "List /tmp",
workdir: os.tmpdir(),
description: "List temp dir",
},
testCtx,
)
const extDirReq = requests.find((r) => r.permission === "external_directory")
expect(extDirReq).toBeDefined()
expect(extDirReq!.patterns).toContain("/tmp/*")
expect(extDirReq!.patterns).toContain(path.join(os.tmpdir(), "*"))
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

The test expects patterns to be normalized with forward slashes, but reviewing bash.ts (lines 145-148), the bash tool doesn't appear to normalize glob patterns before adding them to ctx.ask when the directory doesn't start with "/". On Windows, os.tmpdir() returns something like "C:\Users...\Temp", which path.join would convert to "C:\Users...\Temp*" with backslashes. Consider verifying that the bash tool normalizes patterns consistently with assertExternalDirectory (external-directory.ts:21 uses .replaceAll("\\", "/")), or if normalization happens elsewhere in the request pipeline, consider adding a code comment explaining this behavior for future maintainers.

Copilot uses AI. Check for mistakes.
},
})
})
Expand Down Expand Up @@ -366,7 +367,8 @@ describe("tool.bash truncation", () => {
ctx,
)
expect((result.metadata as any).truncated).toBe(false)
expect(result.output).toBe("hello\n")
const eol = process.platform === "win32" ? "\r\n" : "\n"
expect(result.output).toBe(`hello${eol}`)
},
})
})
Expand Down
4 changes: 2 additions & 2 deletions packages/opencode/test/tool/external-directory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe("tool.assertExternalDirectory", () => {

const directory = "/tmp/project"
const target = "/tmp/outside/file.txt"
const expected = path.join(path.dirname(target), "*")
const expected = path.join(path.dirname(target), "*").replaceAll("\\", "/")

await Instance.provide({
directory,
Expand All @@ -91,7 +91,7 @@ describe("tool.assertExternalDirectory", () => {

const directory = "/tmp/project"
const target = "/tmp/outside"
const expected = path.join(target, "*")
const expected = path.join(target, "*").replaceAll("\\", "/")

await Instance.provide({
directory,
Expand Down
15 changes: 11 additions & 4 deletions packages/opencode/test/tool/write.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,19 +293,26 @@ describe("tool.write", () => {
})

describe("error handling", () => {
test("throws error for paths outside project", async () => {
test("throws error when OS denies write access", async () => {
await using tmp = await tmpdir()
const outsidePath = "/etc/passwd"
const readonlyPath = path.join(tmp.path, "readonly.txt")

// Create a read-only file
await fs.writeFile(readonlyPath, "test", "utf-8")
await fs.chmod(readonlyPath, 0o444)

await Instance.provide({
directory: tmp.path,
fn: async () => {
const { FileTime } = await import("../../src/file/time")
FileTime.read(ctx.sessionID, readonlyPath)

const write = await WriteTool.init()
await expect(
write.execute(
{
filePath: outsidePath,
content: "test",
filePath: readonlyPath,
content: "new content",
},
ctx,
),
Expand Down
Loading