Validate Linear branchName before spawn
This commit is contained in:
parent
adb3958d26
commit
f92c5f6c9d
|
|
@ -137,6 +137,82 @@ describe("spawn", () => {
|
|||
expect(session.branch).toBe("ABC-1234");
|
||||
});
|
||||
|
||||
it("uses tracker.branchName when Issue omits branchName", async () => {
|
||||
const mockTracker: Tracker = {
|
||||
name: "mock-tracker",
|
||||
getIssue: vi.fn().mockResolvedValue({
|
||||
id: "INT-100",
|
||||
title: "T",
|
||||
description: "",
|
||||
url: "https://tracker.test/INT-100",
|
||||
state: "open",
|
||||
labels: [],
|
||||
}),
|
||||
isCompleted: vi.fn().mockResolvedValue(false),
|
||||
issueUrl: vi.fn().mockReturnValue(""),
|
||||
branchName: vi.fn().mockReturnValue("custom/INT-100-my-feature"),
|
||||
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: "INT-100" });
|
||||
expect(session.branch).toBe("custom/INT-100-my-feature");
|
||||
expect(mockTracker.branchName).toHaveBeenCalledWith("INT-100", expect.anything());
|
||||
});
|
||||
|
||||
it("falls back to tracker.branchName when Issue.branchName is not git-safe", async () => {
|
||||
const mockTracker: Tracker = {
|
||||
name: "mock-tracker",
|
||||
getIssue: vi.fn().mockResolvedValue({
|
||||
id: "INT-100",
|
||||
title: "T",
|
||||
description: "",
|
||||
url: "https://tracker.test/INT-100",
|
||||
state: "open",
|
||||
labels: [],
|
||||
branchName: "bad branch with spaces",
|
||||
}),
|
||||
isCompleted: vi.fn().mockResolvedValue(false),
|
||||
issueUrl: vi.fn().mockReturnValue(""),
|
||||
branchName: vi.fn().mockReturnValue("feat/INT-100"),
|
||||
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: "INT-100" });
|
||||
expect(session.branch).toBe("feat/INT-100");
|
||||
});
|
||||
|
||||
it("sanitizes free-text issueId into a valid branch slug", async () => {
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
|
|
|
|||
|
|
@ -2,7 +2,12 @@ import { describe, it, expect, afterEach } from "vitest";
|
|||
import { mkdtempSync, writeFileSync, rmSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { isRetryableHttpStatus, normalizeRetryConfig, readLastJsonlEntry } from "../utils.js";
|
||||
import {
|
||||
isGitBranchNameSafe,
|
||||
isRetryableHttpStatus,
|
||||
normalizeRetryConfig,
|
||||
readLastJsonlEntry,
|
||||
} from "../utils.js";
|
||||
import { parsePrFromUrl } from "../utils/pr.js";
|
||||
|
||||
describe("readLastJsonlEntry", () => {
|
||||
|
|
@ -87,6 +92,30 @@ describe("readLastJsonlEntry", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("isGitBranchNameSafe", () => {
|
||||
it("accepts typical Linear-style branch names", () => {
|
||||
expect(isGitBranchNameSafe("feature/foo-bar-123")).toBe(true);
|
||||
expect(isGitBranchNameSafe("feat/INT-123")).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects empty, @, lock suffix, double dots, and leading dot", () => {
|
||||
expect(isGitBranchNameSafe("")).toBe(false);
|
||||
expect(isGitBranchNameSafe("@")).toBe(false);
|
||||
expect(isGitBranchNameSafe("foo.lock")).toBe(false);
|
||||
expect(isGitBranchNameSafe("a..b")).toBe(false);
|
||||
expect(isGitBranchNameSafe(".hidden")).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects characters invalid in git refs", () => {
|
||||
expect(isGitBranchNameSafe("bad branch")).toBe(false);
|
||||
expect(isGitBranchNameSafe("x:y")).toBe(false);
|
||||
expect(isGitBranchNameSafe("x~y")).toBe(false);
|
||||
expect(isGitBranchNameSafe("x?y")).toBe(false);
|
||||
expect(isGitBranchNameSafe("x[y]")).toBe(false);
|
||||
expect(isGitBranchNameSafe("a\nb")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("retry utilities", () => {
|
||||
it("marks 429 and 5xx statuses as retryable", () => {
|
||||
expect(isRetryableHttpStatus(429)).toBe(true);
|
||||
|
|
|
|||
|
|
@ -98,6 +98,7 @@ export {
|
|||
shellEscape,
|
||||
escapeAppleScript,
|
||||
validateUrl,
|
||||
isGitBranchNameSafe,
|
||||
isRetryableHttpStatus,
|
||||
normalizeRetryConfig,
|
||||
readLastJsonlEntry,
|
||||
|
|
|
|||
|
|
@ -73,6 +73,7 @@ import {
|
|||
} from "./global-pause.js";
|
||||
import { sessionFromMetadata } from "./utils/session-from-metadata.js";
|
||||
import { safeJsonParse } from "./utils/validation.js";
|
||||
import { isGitBranchNameSafe } from "./utils.js";
|
||||
import { resolveAgentSelection, resolveSessionRole } from "./agent-selection.js";
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
|
|
@ -952,9 +953,11 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
if (spawnConfig.branch) {
|
||||
branch = spawnConfig.branch;
|
||||
} else if (spawnConfig.issueId && plugins.tracker && resolvedIssue) {
|
||||
branch = resolvedIssue.branchName
|
||||
? resolvedIssue.branchName
|
||||
: plugins.tracker.branchName(spawnConfig.issueId, project);
|
||||
const fromIssue = resolvedIssue.branchName;
|
||||
branch =
|
||||
fromIssue && isGitBranchNameSafe(fromIssue)
|
||||
? fromIssue
|
||||
: plugins.tracker.branchName(spawnConfig.issueId, project);
|
||||
} else if (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.
|
||||
|
|
|
|||
|
|
@ -33,6 +33,28 @@ export function validateUrl(url: string, label: string): void {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Conservative subset of git `check-ref-format` rules for branch-like names.
|
||||
* Used before passing tracker-supplied names to `git worktree` / `checkout -b`.
|
||||
*
|
||||
* Slashes are allowed (e.g. `feature/foo-bar`).
|
||||
*/
|
||||
export function isGitBranchNameSafe(name: string): boolean {
|
||||
if (!name) return false;
|
||||
if (name === "@" || name.startsWith(".") || name.endsWith(".") || name.endsWith("/")) return false;
|
||||
if (name.endsWith(".lock")) return false;
|
||||
if (name.includes("..")) return false;
|
||||
if (name.includes("@{")) return false;
|
||||
if (name.startsWith("/")) return false;
|
||||
for (let i = 0; i < name.length; i++) {
|
||||
const c = name.charCodeAt(i);
|
||||
if (c <= 0x1f || c === 0x7f) return false;
|
||||
}
|
||||
// Space and git-forbidden punctuation (see git-check-ref-format)
|
||||
if (/[\s~^:?*[\\]/.test(name)) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true if an HTTP status code should be retried.
|
||||
* Retry only 429 (rate-limit) and 5xx (server) failures.
|
||||
|
|
|
|||
|
|
@ -220,7 +220,7 @@ describe.skipIf(!canRun)("tracker-linear (integration)", () => {
|
|||
|
||||
expect(issue.branchName).toBeDefined();
|
||||
expect(typeof issue.branchName).toBe("string");
|
||||
expect(issue.branchName).toContain(issueIdentifier);
|
||||
expect(issue.branchName!.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("isCompleted returns false for an open issue", async () => {
|
||||
|
|
|
|||
|
|
@ -296,7 +296,7 @@ function createLinearTracker(query: GraphQLTransport): Tracker {
|
|||
labels: node.labels.nodes.map((l) => l.name),
|
||||
assignee: node.assignee?.displayName ?? node.assignee?.name,
|
||||
priority: node.priority,
|
||||
...(node.branchName ? { branchName: node.branchName } : {}),
|
||||
branchName: node.branchName ?? undefined,
|
||||
};
|
||||
},
|
||||
|
||||
|
|
@ -429,7 +429,7 @@ function createLinearTracker(query: GraphQLTransport): Tracker {
|
|||
labels: node.labels.nodes.map((l) => l.name),
|
||||
assignee: node.assignee?.displayName ?? node.assignee?.name,
|
||||
priority: node.priority,
|
||||
...(node.branchName ? { branchName: node.branchName } : {}),
|
||||
branchName: node.branchName ?? undefined,
|
||||
}));
|
||||
},
|
||||
|
||||
|
|
@ -622,7 +622,7 @@ function createLinearTracker(query: GraphQLTransport): Tracker {
|
|||
labels: node.labels.nodes.map((l) => l.name),
|
||||
assignee: node.assignee?.displayName ?? node.assignee?.name,
|
||||
priority: node.priority,
|
||||
...(node.branchName ? { branchName: node.branchName } : {}),
|
||||
branchName: node.branchName ?? undefined,
|
||||
};
|
||||
|
||||
// Assign after creation (Linear's issueCreate uses assigneeId, not display name)
|
||||
|
|
|
|||
|
|
@ -136,9 +136,9 @@ for ISSUE in "${ISSUES[@]}"; do
|
|||
|
||||
# Build initial prompt
|
||||
if [ "$SESSION_PREFIX" = "splitly" ]; then
|
||||
INITIAL_PROMPT="Please start working on GitHub issue #$ISSUE_NUM. First, fetch the issue details using: gh issue view $ISSUE_NUM --repo $GITHUB_REPO. Then implement the solution in the branch already prepared for this session."
|
||||
INITIAL_PROMPT="Please start working on GitHub issue #$ISSUE_NUM. First, fetch the issue details using: gh issue view $ISSUE_NUM --repo $GITHUB_REPO. Then implement the solution in the branch already prepared for this session (confirm with \`git branch --show-current\` in the worktree if needed)."
|
||||
else
|
||||
INITIAL_PROMPT="Please start working on $ISSUE, fetch ticket info, and implement the task in the branch already prepared for this session."
|
||||
INITIAL_PROMPT="Please start working on $ISSUE, fetch ticket info, and implement the task in the branch already prepared for this session (confirm with \`git branch --show-current\` in the worktree if needed)."
|
||||
fi
|
||||
|
||||
# Send prompt to the running Claude session
|
||||
|
|
|
|||
Loading…
Reference in New Issue