fix: handle ad-hoc spawn with free-text issue strings (#155)
* fix: handle "invalid issue format" error for ad-hoc spawn gh CLI returns "invalid issue format" when the issue argument is free-text rather than a valid issue number/URL. This error was not matched by isIssueNotFoundError, causing ad-hoc spawns to fail instead of gracefully falling through to ad-hoc mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: sanitize ad-hoc issue text into valid git branch names When tracker lookup fails (ad-hoc mode), the raw free-text was used directly as a branch name, causing git to reject it. Now: - Only use tracker.branchName() when the issue was actually resolved - Slugify ad-hoc text (lowercase, replace non-alphanumeric with hyphens, trim, cap at 60 chars) to produce valid branch names e.g. "fix the cold start issue" → feat/fix-the-cold-start-issue Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: preserve casing for branch-safe issue IDs, only slugify free-text The slug sanitization was lowercasing all issue IDs without a resolved issue (e.g. INT-9999 → int-9999). Now only applies sanitization when the issueId contains characters invalid for git branch names. Adds tests for slug sanitization, casing preservation, truncation, empty-slug fallback, and isIssueNotFoundError patterns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: assert tracker.branchName not called for unresolved issues Adds assertion to existing test that tracker.branchName is NOT called when the issue wasn't found. Adds end-to-end test for "invalid issue format" error flowing through spawn with free-text slugification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: reject '..' in branch-safe check, verify generatePrompt skipped The isBranchSafe regex allowed '..' which is invalid in git refs. Also asserts that tracker.generatePrompt is not called when the issue wasn't resolved, and adds a test for the '..' edge case. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: trim dashes after truncation, not before Bugbot correctly noted that .slice(0, 60) after dash-trim can reintroduce a trailing dash. Reorder so trim runs after slice. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Prateek <karnalprateek@gmail.com>
This commit is contained in:
parent
e5105133c5
commit
4cda43795b
|
|
@ -9,6 +9,7 @@ import { getSessionsDir, getProjectBaseDir } from "../paths.js";
|
|||
import {
|
||||
SessionNotRestorableError,
|
||||
WorkspaceMissingError,
|
||||
isIssueNotFoundError,
|
||||
type OrchestratorConfig,
|
||||
type PluginRegistry,
|
||||
type Runtime,
|
||||
|
|
@ -160,6 +161,76 @@ describe("spawn", () => {
|
|||
expect(session.issueId).toBe("INT-100");
|
||||
});
|
||||
|
||||
it("sanitizes free-text issueId into a valid branch slug", async () => {
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
const session = await sm.spawn({ projectId: "my-app", issueId: "fix login bug" });
|
||||
|
||||
expect(session.branch).toBe("feat/fix-login-bug");
|
||||
});
|
||||
|
||||
it("preserves casing for branch-safe issue IDs without tracker", async () => {
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
const session = await sm.spawn({ projectId: "my-app", issueId: "INT-9999" });
|
||||
|
||||
expect(session.branch).toBe("feat/INT-9999");
|
||||
});
|
||||
|
||||
it("sanitizes issueId with special characters", async () => {
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
const session = await sm.spawn({
|
||||
projectId: "my-app",
|
||||
issueId: "Fix: user can't login (SSO)",
|
||||
});
|
||||
|
||||
expect(session.branch).toBe("feat/fix-user-can-t-login-sso");
|
||||
});
|
||||
|
||||
it("truncates long slugs to 60 characters", async () => {
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
const session = await sm.spawn({
|
||||
projectId: "my-app",
|
||||
issueId: "this is a very long issue description that should be truncated to sixty characters maximum",
|
||||
});
|
||||
|
||||
expect(session.branch!.replace("feat/", "").length).toBeLessThanOrEqual(60);
|
||||
});
|
||||
|
||||
it("does not leave trailing dash after truncation", async () => {
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
// Craft input where the 60th char falls on a word boundary (dash)
|
||||
const session = await sm.spawn({
|
||||
projectId: "my-app",
|
||||
issueId: "ab ".repeat(30), // "ab ab ab ..." → "ab-ab-ab-..." truncated at 60
|
||||
});
|
||||
|
||||
const slug = session.branch!.replace("feat/", "");
|
||||
expect(slug).not.toMatch(/-$/);
|
||||
expect(slug).not.toMatch(/^-/);
|
||||
});
|
||||
|
||||
it("falls back to sessionId when issueId sanitizes to empty string", async () => {
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
const session = await sm.spawn({ projectId: "my-app", issueId: "!!!" });
|
||||
|
||||
// Slug is empty after sanitization, falls back to sessionId
|
||||
expect(session.branch).toMatch(/^feat\/app-\d+$/);
|
||||
});
|
||||
|
||||
it("sanitizes issueId containing '..' (invalid in git branch names)", async () => {
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
const session = await sm.spawn({ projectId: "my-app", issueId: "foo..bar" });
|
||||
|
||||
// '..' is invalid in git refs, so it should be slugified
|
||||
expect(session.branch).toBe("feat/foo-bar");
|
||||
});
|
||||
|
||||
it("uses tracker.branchName when tracker is available", async () => {
|
||||
const mockTracker: Tracker = {
|
||||
name: "mock-tracker",
|
||||
|
|
@ -385,11 +456,48 @@ describe("spawn", () => {
|
|||
|
||||
expect(session.issueId).toBe("INT-9999");
|
||||
expect(session.branch).toBe("feat/INT-9999");
|
||||
// tracker.branchName and generatePrompt should NOT be called when issue wasn't resolved
|
||||
expect(mockTracker.branchName).not.toHaveBeenCalled();
|
||||
expect(mockTracker.generatePrompt).not.toHaveBeenCalled();
|
||||
// Workspace and runtime should still be created
|
||||
expect(mockWorkspace.create).toHaveBeenCalled();
|
||||
expect(mockRuntime.create).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("succeeds with ad-hoc free-text when tracker returns 'invalid issue format'", async () => {
|
||||
const mockTracker: Tracker = {
|
||||
name: "mock-tracker",
|
||||
getIssue: vi.fn().mockRejectedValue(new Error("invalid issue format: fix login bug")),
|
||||
isCompleted: vi.fn().mockResolvedValue(false),
|
||||
issueUrl: vi.fn().mockReturnValue(""),
|
||||
branchName: vi.fn().mockReturnValue(""),
|
||||
generatePrompt: vi.fn().mockResolvedValue(""),
|
||||
};
|
||||
|
||||
const registryWithTracker: PluginRegistry = {
|
||||
...mockRegistry,
|
||||
get: vi.fn().mockImplementation((slot: string) => {
|
||||
if (slot === "runtime") return mockRuntime;
|
||||
if (slot === "agent") return mockAgent;
|
||||
if (slot === "workspace") return mockWorkspace;
|
||||
if (slot === "tracker") return mockTracker;
|
||||
return null;
|
||||
}),
|
||||
};
|
||||
|
||||
const sm = createSessionManager({
|
||||
config,
|
||||
registry: registryWithTracker,
|
||||
});
|
||||
|
||||
const session = await sm.spawn({ projectId: "my-app", issueId: "fix login bug" });
|
||||
|
||||
expect(session.issueId).toBe("fix login bug");
|
||||
expect(session.branch).toBe("feat/fix-login-bug");
|
||||
expect(mockTracker.branchName).not.toHaveBeenCalled();
|
||||
expect(mockWorkspace.create).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fails on tracker auth errors", async () => {
|
||||
const mockTracker: Tracker = {
|
||||
name: "mock-tracker",
|
||||
|
|
@ -1612,3 +1720,33 @@ describe("PluginRegistry.loadBuiltins importFn", () => {
|
|||
expect(importedPackages).toContain("@composio/ao-plugin-runtime-tmux");
|
||||
});
|
||||
});
|
||||
|
||||
describe("isIssueNotFoundError", () => {
|
||||
it("matches 'Issue X not found'", () => {
|
||||
expect(isIssueNotFoundError(new Error("Issue INT-9999 not found"))).toBe(true);
|
||||
});
|
||||
|
||||
it("matches 'could not resolve to an Issue'", () => {
|
||||
expect(isIssueNotFoundError(new Error("Could not resolve to an Issue"))).toBe(true);
|
||||
});
|
||||
|
||||
it("matches 'no issue with identifier'", () => {
|
||||
expect(isIssueNotFoundError(new Error("No issue with identifier ABC-123"))).toBe(true);
|
||||
});
|
||||
|
||||
it("matches 'invalid issue format'", () => {
|
||||
expect(isIssueNotFoundError(new Error("Invalid issue format: fix login bug"))).toBe(true);
|
||||
});
|
||||
|
||||
it("does not match unrelated errors", () => {
|
||||
expect(isIssueNotFoundError(new Error("Unauthorized"))).toBe(false);
|
||||
expect(isIssueNotFoundError(new Error("Network timeout"))).toBe(false);
|
||||
expect(isIssueNotFoundError(new Error("API key not found"))).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for non-error values", () => {
|
||||
expect(isIssueNotFoundError(null)).toBe(false);
|
||||
expect(isIssueNotFoundError(undefined)).toBe(false);
|
||||
expect(isIssueNotFoundError("string")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -391,10 +391,22 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager {
|
|||
let branch: string;
|
||||
if (spawnConfig.branch) {
|
||||
branch = spawnConfig.branch;
|
||||
} else if (spawnConfig.issueId && plugins.tracker) {
|
||||
} else if (spawnConfig.issueId && plugins.tracker && resolvedIssue) {
|
||||
branch = plugins.tracker.branchName(spawnConfig.issueId, project);
|
||||
} else if (spawnConfig.issueId) {
|
||||
branch = `feat/${spawnConfig.issueId}`;
|
||||
// If the issueId is already branch-safe (e.g. "INT-9999"), use as-is.
|
||||
// Otherwise sanitize free-text (e.g. "fix login bug") into a valid slug.
|
||||
const id = spawnConfig.issueId;
|
||||
const isBranchSafe =
|
||||
/^[A-Za-z0-9][A-Za-z0-9._-]*$/.test(id) && !id.includes("..");
|
||||
const slug = isBranchSafe
|
||||
? id
|
||||
: id
|
||||
.toLowerCase()
|
||||
.replace(/[^a-z0-9]+/g, "-")
|
||||
.slice(0, 60)
|
||||
.replace(/^-+|-+$/g, "");
|
||||
branch = `feat/${slug || sessionId}`;
|
||||
} else {
|
||||
branch = `session/${sessionId}`;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1069,7 +1069,9 @@ export function isIssueNotFoundError(err: unknown): boolean {
|
|||
// GitHub: "no issue found" or "could not resolve to an Issue"
|
||||
message.includes("could not resolve to an issue") ||
|
||||
// Linear: "Issue <id> not found" or "No issue with identifier"
|
||||
message.includes("no issue with identifier")
|
||||
message.includes("no issue with identifier") ||
|
||||
// GitHub: "invalid issue format" (ad-hoc free-text strings)
|
||||
message.includes("invalid issue format")
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue