fix(coding-agent): require token boundary for #<number>; add pr/issue qualifier
Address review feedback on #3224 (roboomp + Codex): #<number> now requires a token boundary before # (start/whitespace/quote/paren/</=), matching the internal-URL boundary set, so owner/repo#N, foo#N, C#12, and URL fragments no longer match and corrupt on accept. Also detect a pr/pull/issue qualifier word before the number (pr #3164 / issue #3164); when present, only that kind is offered and the qualifier is consumed on accept.
This commit is contained in:
oldschoola
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { KeybindingsManager as AppKeybindingsManager } from "@oh-my-pi/pi-coding-agent/config/keybindings";
|
||||
import { getGithubRefPrefix, getGithubRefSuggestions } from "@oh-my-pi/pi-coding-agent/modes/github-ref-autocomplete";
|
||||
import { getGithubRefContext, getGithubRefSuggestions } from "@oh-my-pi/pi-coding-agent/modes/github-ref-autocomplete";
|
||||
import { createPromptActionAutocompleteProvider } from "@oh-my-pi/pi-coding-agent/modes/prompt-action-autocomplete";
|
||||
|
||||
function makeProvider() {
|
||||
@@ -18,30 +18,62 @@ function makeProvider() {
|
||||
});
|
||||
}
|
||||
|
||||
describe("github-ref autocomplete — prefix detection", () => {
|
||||
it("matches the last #<digits> token ending at the cursor", () => {
|
||||
expect(getGithubRefPrefix("#3164")).toBe("#3164");
|
||||
expect(getGithubRefPrefix("look at #3164")).toBe("#3164");
|
||||
expect(getGithubRefPrefix("see #1 and #3164")).toBe("#3164");
|
||||
describe("github-ref autocomplete — token detection", () => {
|
||||
it("matches a standalone #<number> ending at the cursor", () => {
|
||||
expect(getGithubRefContext("#3164")).toEqual({ prefix: "#3164", qualifier: null, number: "3164" });
|
||||
expect(getGithubRefContext("look at #3164")).toEqual({ prefix: "#3164", qualifier: null, number: "3164" });
|
||||
// only the token ending at the cursor (the $ anchor) wins
|
||||
expect(getGithubRefContext("see #1 then #3164")).toEqual({
|
||||
prefix: "#3164",
|
||||
qualifier: null,
|
||||
number: "3164",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not match bare #, text, or mixed tokens", () => {
|
||||
expect(getGithubRefPrefix("#")).toBeNull();
|
||||
expect(getGithubRefPrefix("#copy")).toBeNull();
|
||||
expect(getGithubRefPrefix("#3164abc")).toBeNull();
|
||||
expect(getGithubRefPrefix("#3a")).toBeNull();
|
||||
// zero / leading zeros are not valid GitHub numbers
|
||||
expect(getGithubRefPrefix("#0")).toBeNull();
|
||||
expect(getGithubRefPrefix("#00")).toBeNull();
|
||||
expect(getGithubRefPrefix("#0123")).toBeNull();
|
||||
it("requires a token boundary before # so embedded hashes don't match", () => {
|
||||
// cross-repo reference, mid-word, URL fragment — none should offer candidates
|
||||
expect(getGithubRefContext("owner/repo#3164")).toBeNull();
|
||||
expect(getGithubRefContext("foo#3164")).toBeNull();
|
||||
expect(getGithubRefContext("C#12")).toBeNull();
|
||||
expect(getGithubRefContext("https://github.com/can1357/oh-my-pi#3164")).toBeNull();
|
||||
expect(getGithubRefContext("path/#3164")).toBeNull();
|
||||
});
|
||||
|
||||
it("does not match bare #, text, mixed, zero, or leading zeros", () => {
|
||||
expect(getGithubRefContext("#")).toBeNull();
|
||||
expect(getGithubRefContext("#copy")).toBeNull();
|
||||
expect(getGithubRefContext("#3164abc")).toBeNull();
|
||||
expect(getGithubRefContext("#3a")).toBeNull();
|
||||
// a space after the digits closes the token
|
||||
expect(getGithubRefPrefix("#3164 ")).toBeNull();
|
||||
expect(getGithubRefPrefix("no hash here")).toBeNull();
|
||||
expect(getGithubRefContext("#3164 ")).toBeNull();
|
||||
expect(getGithubRefContext("#0")).toBeNull();
|
||||
expect(getGithubRefContext("#0123")).toBeNull();
|
||||
});
|
||||
|
||||
it("detects a pr/pull/issue qualifier word immediately before the number", () => {
|
||||
expect(getGithubRefContext("pr #3164")).toEqual({ prefix: "pr #3164", qualifier: "pr", number: "3164" });
|
||||
expect(getGithubRefContext("PR #3164")).toEqual({ prefix: "PR #3164", qualifier: "pr", number: "3164" });
|
||||
expect(getGithubRefContext("pull #3164")).toEqual({ prefix: "pull #3164", qualifier: "pr", number: "3164" });
|
||||
expect(getGithubRefContext("issue #3164")).toEqual({
|
||||
prefix: "issue #3164",
|
||||
qualifier: "issue",
|
||||
number: "3164",
|
||||
});
|
||||
// the word right before the number is the qualifier, even with leading text
|
||||
expect(getGithubRefContext("look at the issue #3164")?.qualifier).toBe("issue");
|
||||
});
|
||||
|
||||
it("does not treat arbitrary words, or qualifiers glued to a path, as the type", () => {
|
||||
expect(getGithubRefContext("review #3164")?.qualifier).toBeNull();
|
||||
// "pr" inside "src/pr" is preceded by '/', not a boundary, so it is not a qualifier
|
||||
expect(getGithubRefContext("src/pr #3164")?.qualifier).toBeNull();
|
||||
// a qualifier with no space before the # is not recognized
|
||||
expect(getGithubRefContext("pr#3164")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("github-ref autocomplete — suggestions", () => {
|
||||
it("offers a PR and an Issue candidate for #<number>", () => {
|
||||
it("offers both candidates when no qualifier is given", () => {
|
||||
const result = getGithubRefSuggestions("#3164");
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.prefix).toBe("#3164");
|
||||
@@ -51,16 +83,26 @@ describe("github-ref autocomplete — suggestions", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("returns null for non-numeric tokens", () => {
|
||||
it("offers only the PR candidate for a pr/pull qualifier", () => {
|
||||
const result = getGithubRefSuggestions("pr #3164");
|
||||
expect(result!.prefix).toBe("pr #3164");
|
||||
expect(result!.items).toEqual([{ value: "pr://3164", label: "PR #3164", description: "GitHub pull request" }]);
|
||||
});
|
||||
|
||||
it("offers only the Issue candidate for an issue qualifier", () => {
|
||||
const result = getGithubRefSuggestions("issue #3164");
|
||||
expect(result!.items).toEqual([{ value: "issue://3164", label: "Issue #3164", description: "GitHub issue" }]);
|
||||
});
|
||||
|
||||
it("returns null for embedded or non-ref text", () => {
|
||||
expect(getGithubRefSuggestions("owner/repo#3164")).toBeNull();
|
||||
expect(getGithubRefSuggestions("#copy")).toBeNull();
|
||||
expect(getGithubRefSuggestions("#")).toBeNull();
|
||||
expect(getGithubRefSuggestions("#3164abc")).toBeNull();
|
||||
expect(getGithubRefSuggestions("#0")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("github-ref autocomplete — provider integration", () => {
|
||||
it("yields the ref candidates and rewrites the token to the chosen internal URL", async () => {
|
||||
it("yields both candidates and rewrites the token to the chosen internal URL", async () => {
|
||||
const provider = makeProvider();
|
||||
const suggestions = await provider.getSuggestions(["review #3164"], 0, 12);
|
||||
expect(suggestions).not.toBeNull();
|
||||
@@ -69,7 +111,6 @@ describe("github-ref autocomplete — provider integration", () => {
|
||||
|
||||
const pr = suggestions!.items[0]!;
|
||||
const issue = suggestions!.items[1]!;
|
||||
|
||||
const prResult = provider.applyCompletion(["review #3164"], 0, 12, pr, suggestions!.prefix);
|
||||
expect(prResult.lines).toEqual(["review pr://3164 "]);
|
||||
expect(prResult.cursorCol).toBe("review pr://3164 ".length);
|
||||
@@ -78,14 +119,23 @@ describe("github-ref autocomplete — provider integration", () => {
|
||||
expect(issueResult.lines).toEqual(["review issue://3164 "]);
|
||||
});
|
||||
|
||||
it("leaves #<text> and bare # to the prompt-action menu (no github-ref candidates)", async () => {
|
||||
it("constrains to the named type and consumes the qualifier on accept", async () => {
|
||||
const provider = makeProvider();
|
||||
const suggestions = await provider.getSuggestions(["review pr #3164"], 0, 15);
|
||||
expect(suggestions).not.toBeNull();
|
||||
expect(suggestions!.prefix).toBe("pr #3164");
|
||||
expect(suggestions!.items.map(item => item.value)).toEqual(["pr://3164"]);
|
||||
|
||||
const pr = suggestions!.items[0]!;
|
||||
const result = provider.applyCompletion(["review pr #3164"], 0, 15, pr, suggestions!.prefix);
|
||||
// the "pr " qualifier is replaced along with the number, not left dangling
|
||||
expect(result.lines).toEqual(["review pr://3164 "]);
|
||||
});
|
||||
|
||||
it("does not offer candidates for embedded hashes (falls through to other providers)", async () => {
|
||||
const provider = makeProvider();
|
||||
const isRef = (value: string) => value.startsWith("pr://") || value.startsWith("issue://");
|
||||
|
||||
const textSuggestions = await provider.getSuggestions(["#copy"], 0, 5);
|
||||
expect(textSuggestions?.items.every(item => !isRef(item.value))).toBe(true);
|
||||
|
||||
const bareSuggestions = await provider.getSuggestions(["#"], 0, 1);
|
||||
expect(bareSuggestions?.items.every(item => !isRef(item.value))).toBe(true);
|
||||
const embedded = await provider.getSuggestions(["owner/repo#3164"], 0, 15);
|
||||
expect(embedded?.items.every(item => !isRef(item.value)) ?? true).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user