From 4cda43795bac02163a6094ed65d006ad0fd912c8 Mon Sep 17 00:00:00 2001 From: Karan Vaidya Date: Fri, 27 Feb 2026 05:40:03 -0800 Subject: [PATCH] fix: handle ad-hoc spawn with free-text issue strings (#155) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 * 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 * 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 * 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 * 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 --------- Co-authored-by: Claude Opus 4.6 Co-authored-by: Prateek --- .../src/__tests__/session-manager.test.ts | 138 ++++++++++++++++++ packages/core/src/session-manager.ts | 16 +- packages/core/src/types.ts | 4 +- 3 files changed, 155 insertions(+), 3 deletions(-) diff --git a/packages/core/src/__tests__/session-manager.test.ts b/packages/core/src/__tests__/session-manager.test.ts index 9877f2ad2..a88ad5cfc 100644 --- a/packages/core/src/__tests__/session-manager.test.ts +++ b/packages/core/src/__tests__/session-manager.test.ts @@ -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); + }); +}); diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index a8ae1e82c..a6b5ca038 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -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}`; } diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 9bbfe786f..617c60b28 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -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 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") ); }