Просмотр исходного кода

make @file references in custom slash commands more robust (#2203)

Co-authored-by: Adam Spiers <[email protected]>
Co-authored-by: rekram1-node <[email protected]>
Adam Spiers 5 месяцев назад
Родитель
Сommit
47d4c87bdd

+ 48 - 0
packages/opencode/src/session/file-reference.ts

@@ -0,0 +1,48 @@
+import os from "os"
+import path from "path"
+
+/**
+ * Regular expression to match @ file references in text
+ * Matches @ followed by file paths, excluding commas, periods at end of sentences, and backticks
+ * Does not match when preceded by word characters or backticks (to avoid email addresses and quoted references)
+ */
+export const fileRegex = /(?<![\w`])@(\.?[^\s`,.]*(?:\.[^\s`,.]+)*)/g
+
+/**
+ * File part type for chat input
+ */
+export type FilePart = {
+  type: "file"
+  url: string
+  filename: string
+  mime: string
+}
+
+/**
+ * Processes file references in a template string and returns file parts
+ * @param template - The template string containing @file references
+ * @param basePath - The base path to resolve relative file paths against
+ * @returns Array of file parts for the chat input
+ */
+export function processFileReferences(template: string, basePath: string): FilePart[] {
+  // intentionally doing match regex doing bash regex replacements
+  // this is because bash commands can output "@" references
+  const matches = template.matchAll(fileRegex)
+
+  const parts: FilePart[] = []
+  for (const match of matches) {
+    const filename = match[1]
+    const filepath = filename.startsWith("~/")
+      ? path.join(os.homedir(), filename.slice(2))
+      : path.resolve(basePath, filename)
+
+    parts.push({
+      type: "file",
+      url: `file://${filepath}`,
+      filename,
+      mime: "text/plain",
+    })
+  }
+
+  return parts
+}

+ 3 - 19
packages/opencode/src/session/index.ts

@@ -1,5 +1,4 @@
 import path from "path"
-import os from "os"
 import { spawn } from "child_process"
 import { Decimal } from "decimal.js"
 import { z, ZodSchema } from "zod"
@@ -51,6 +50,7 @@ import { ulid } from "ulid"
 import { defer } from "../util/defer"
 import { Command } from "../command"
 import { $ } from "bun"
+import { processFileReferences } from "./file-reference"
 
 export namespace Session {
   const log = Log.create({ service: "session" })
@@ -1229,7 +1229,6 @@ export namespace Session {
   })
   export type CommandInput = z.infer<typeof CommandInput>
   const bashRegex = /!`([^`]+)`/g
-  const fileRegex = /@([^\s]+)/g
 
   export async function command(input: CommandInput) {
     log.info("command", input)
@@ -1238,10 +1237,6 @@ export namespace Session {
 
     let template = command.template.replace("$ARGUMENTS", input.arguments)
 
-    // intentionally doing match regex doing bash regex replacements
-    // this is because bash commands can output "@" references
-    const fileMatches = template.matchAll(fileRegex)
-
     const bash = Array.from(template.matchAll(bashRegex))
     if (bash.length > 0) {
       const results = await Promise.all(
@@ -1264,19 +1259,8 @@ export namespace Session {
       },
     ] as ChatInput["parts"]
 
-    for (const match of fileMatches) {
-      const filename = match[1]
-      const filepath = filename.startsWith("~/")
-        ? path.join(os.homedir(), filename.slice(2))
-        : path.join(Instance.worktree, filename)
-
-      parts.push({
-        type: "file",
-        url: `file://${filepath}`,
-        filename,
-        mime: "text/plain",
-      })
-    }
+    const fileReferenceParts = processFileReferences(template, Instance.worktree)
+    parts.push(...fileReferenceParts)
 
     return prompt({
       sessionID: input.sessionID,

+ 102 - 0
packages/opencode/test/session/fileRegex.test.ts

@@ -0,0 +1,102 @@
+import { describe, expect, test, beforeAll, mock } from "bun:test"
+
+describe("processFileReferences", () => {
+  let result: any
+
+  beforeAll(async () => {
+    mock.module("os", () => ({ default: { homedir: () => "/home/fake-user" } }))
+    const { processFileReferences } = await import("../../src/session/file-reference")
+    const template = `This is a @valid/path/to/a/file and it should also match at
+the beginning of a line:
+
+@another-valid/path/to/a/file
+
+but this is not:
+
+   - Adds a "Co-authored-by:" footer which clarifies which AI agent
+     helped create this commit, using an appropriate \`noreply@...\`
+     or \`[email protected]\` email address.
+
+We also need to deal with files followed by @commas, ones
+with @file-extensions.md, even @multiple.extensions.bak,
+hidden directorys like @.config/ or files like @.bashrc
+and ones at the end of a sentence like @foo.md.
+
+Also shouldn't forget @/absolute/paths.txt with and @/without/extensions,
+as well as @~/home-files and @~/paths/under/home.txt.
+
+If the reference is \`@quoted/in/backticks\` then it shouldn't match at all.`
+    result = processFileReferences(template, "/base")
+  })
+
+  test("should extract exactly 12 file references", () => {
+    expect(result.length).toBe(12)
+  })
+
+  test("all files should have correct type and mime", () => {
+    result.forEach((file: any) => {
+      expect(file.type).toBe("file")
+      expect(file.mime).toBe("text/plain")
+    })
+  })
+
+  test("should extract valid/path/to/a/file", () => {
+    expect(result[0].filename).toBe("valid/path/to/a/file")
+    expect(result[0].url).toBe("file:///base/valid/path/to/a/file")
+  })
+
+  test("should extract another-valid/path/to/a/file", () => {
+    expect(result[1].filename).toBe("another-valid/path/to/a/file")
+    expect(result[1].url).toBe("file:///base/another-valid/path/to/a/file")
+  })
+
+  test("should extract paths ignoring comma after", () => {
+    expect(result[2].filename).toBe("commas")
+    expect(result[2].url).toBe("file:///base/commas")
+  })
+
+  test("should extract a path with a file extension and comma after", () => {
+    expect(result[3].filename).toBe("file-extensions.md")
+    expect(result[3].url).toBe("file:///base/file-extensions.md")
+  })
+
+  test("should extract a path with multiple dots and comma after", () => {
+    expect(result[4].filename).toBe("multiple.extensions.bak")
+    expect(result[4].url).toBe("file:///base/multiple.extensions.bak")
+  })
+
+  test("should extract hidden directory", () => {
+    expect(result[5].filename).toBe(".config/")
+    expect(result[5].url).toBe("file:///base/.config")
+  })
+
+  test("should extract hidden file", () => {
+    expect(result[6].filename).toBe(".bashrc")
+    expect(result[6].url).toBe("file:///base/.bashrc")
+  })
+
+  test("should extract a file ignoring period at end of sentence", () => {
+    expect(result[7].filename).toBe("foo.md")
+    expect(result[7].url).toBe("file:///base/foo.md")
+  })
+
+  test("should extract an absolute path with an extension", () => {
+    expect(result[8].filename).toBe("/absolute/paths.txt")
+    expect(result[8].url).toBe("file:///absolute/paths.txt")
+  })
+
+  test("should extract an absolute path without an extension", () => {
+    expect(result[9].filename).toBe("/without/extensions")
+    expect(result[9].url).toBe("file:///without/extensions")
+  })
+
+  test("should extract an absolute path in home directory", () => {
+    expect(result[10].filename).toBe("~/home-files")
+    expect(result[10].url).toBe("file:///home/fake-user/home-files")
+  })
+
+  test("should extract an absolute path under home directory", () => {
+    expect(result[11].filename).toBe("~/paths/under/home.txt")
+    expect(result[11].url).toBe("file:///home/fake-user/paths/under/home.txt")
+  })
+})