Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/opencode/src/tasks/job-commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { runComposer } from "./composer"
import { SessionPrompt } from "../session/prompt"
import { Worktree } from "../worktree"
import { Instance } from "../project/instance"
import * as PulseUtils from "./pulse-utils"

const log = Log.create({ service: "taskctl.tool.job-commands" })

Expand Down Expand Up @@ -81,7 +82,8 @@ export async function executeStart(projectId: string, params: any, ctx: any): Pr

try {
const { $ } = await import("bun")
const result = await $`git checkout -b ${safeFeatureBranch} dev`.cwd(cwd).quiet().nothrow()
const base = await PulseUtils.defaultBranch(cwd)
const result = await $`git checkout -b ${safeFeatureBranch} ${base}`.cwd(cwd).quiet().nothrow()
if (result.exitCode !== 0) {
const stderr = result.stderr ? new TextDecoder().decode(result.stderr) : "Unknown error"
log.error("failed to create feature branch", { issueNumber, featureBranch: safeFeatureBranch, error: stderr })
Expand Down
30 changes: 20 additions & 10 deletions packages/opencode/src/tasks/pulse-scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,20 +151,29 @@ async function spawnDeveloper(task: Task, jobId: string, projectId: string, pmSe

const now = new Date().toISOString()

let devBranchExists = false
// Detect the default branch dynamically
let base: string
try {
const res = await $`git rev-parse --verify dev`.quiet().nothrow().cwd(worktreeInfo.directory)
base = await PulseUtils.defaultBranch(worktreeInfo.directory)
} catch (e) {
log.warn("failed to detect default branch, using fallback", { taskId: task.id, error: String(e) })
base = "dev"
}

let baseBranchExists = false
try {
const res = await $`git rev-parse --verify ${base}`.quiet().nothrow().cwd(worktreeInfo.directory)
if (res.exitCode === 0) {
devBranchExists = true
baseBranchExists = true
}
} catch (e) {
log.warn("failed to verify dev branch exists", { taskId: task.id, error: String(e) })
log.warn("failed to verify base branch exists", { taskId: task.id, baseBranch: base, error: String(e) })
}

let baseCommit: string | null = null
if (devBranchExists) {
if (baseBranchExists) {
try {
const res = await $`git merge-base dev HEAD`.quiet().nothrow().cwd(worktreeInfo.directory)
const res = await $`git merge-base ${base} HEAD`.quiet().nothrow().cwd(worktreeInfo.directory)
if (res.exitCode === 0 && res.stdout) {
baseCommit = new TextDecoder().decode(res.stdout).trim()
}
Expand All @@ -174,19 +183,20 @@ async function spawnDeveloper(task: Task, jobId: string, projectId: string, pmSe
if (headRes.exitCode === 0 && headRes.stdout) {
const headCommit = new TextDecoder().decode(headRes.stdout).trim()
if (baseCommit === headCommit) {
log.warn("worktree HEAD equals merge-base (likely worktree-on-dev), setting base_commit to null", {
log.warn("worktree HEAD equals merge-base (likely worktree-on-base), setting base_commit to null", {
taskId: task.id,
base_commit: baseCommit,
base_branch: base,
})
baseCommit = null
}
}
}
} catch (e) {
log.warn("failed to capture base_commit", { taskId: task.id, error: String(e) })
log.warn("failed to capture base_commit", { taskId: task.id, baseBranch: base, error: String(e) })
}
} else {
log.warn("dev branch not found in worktree, skipping merge-base capture", { taskId: task.id })
log.warn("base branch not found in worktree, skipping merge-base capture", { taskId: task.id, baseBranch: base })
}

const parentSession = await Session.get(pmSessionId).catch(() => null)
Expand Down Expand Up @@ -377,7 +387,7 @@ cd ${safeWorktree}
git diff ${validatedBaseCommit}..HEAD
\`\`\`

This ensures you only review changes made by the developer, not commits that were already in dev.
This ensures you only review changes made by the developer, not commits that were already in the base branch.

Read the changed files in the worktree, run typecheck, and record your verdict with taskctl verdict.`

Expand Down
69 changes: 67 additions & 2 deletions packages/opencode/src/tasks/pulse-utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { $ } from "bun"
import { existsSync } from "fs"
import { Log } from "../util/log"

const log = Log.create({ service: "taskctl.pulse.utils" })
Expand All @@ -24,19 +25,83 @@ export function validateBaseCommit(baseCommit: string | null | undefined): strin
return baseCommit
}

/**
* Validate a branch name is safe to use in shell commands
* Only allow alphanumeric chars, hyphens, underscores, slashes, and dots
*/
function safeBranchName(name: string): string | null {
// Trim all Unicode whitespace variations
const trimmed = name.replace(/^[\s\uFEFF]+|[\s\uFEFF]+$/g, "")
// Only allow safe branch name characters: alphanumeric, hyphen, underscore, slash, dot
return /^[a-zA-Z0-9][a-zA-Z0-9._\-/]*$/.test(trimmed) ? trimmed : null
}

/**
* Check if there are committed changes in the worktree (for adversarial review validation)
* This is extracted into a separate file for easier test mocking
*/
export async function hasCommittedChanges(worktreePath: string, baseCommit: string | null): Promise<boolean> {
try {
const validated = validateBaseCommit(baseCommit) ?? "dev"
const validated = validateBaseCommit(baseCommit)
if (!validated) {
log.warn("invalid base commit ref", { baseCommit, worktreePath })
return false
}

if (!existsSync(worktreePath)) return false

const diffCheck = await $`git diff ${validated}..HEAD --stat`.quiet().nothrow().cwd(worktreePath)
if (diffCheck.exitCode !== 0) return false
const diffOutput = new TextDecoder().decode(diffCheck.stdout).trim()
return diffOutput.length > 0
} catch (e) {
log.warn("failed to check for committed changes", { worktreePath, baseCommit, error: String(e) })
return false
}
}
}

/**
* Detect the default branch of a git repository dynamically
* Tries symbolic-ref first (fastest), falls back to ls-remote, then defaults to 'dev'
*/
export async function defaultBranch(cwd: string): Promise<string> {
try {
// Try symbolic-ref first (fastest, works when origin/HEAD is set)
const ref = await $`git symbolic-ref refs/remotes/origin/HEAD --short`.quiet().nothrow().cwd(cwd)
if (ref.exitCode === 0) {
const name = new TextDecoder().decode(ref.stdout).trim().replace(/^origin\//, "")
const validated = safeBranchName(name)
if (validated) {
log.debug("detected default branch via symbolic-ref", { branch: validated })
return validated
}
}
} catch (e) {
log.debug("symbolic-ref attempt failed", { error: String(e) })
}

try {
// Fallback: use git ls-remote which is faster and more reliable
// Shell timeout 5 enforces the 5 second limit (enforces at shell level via timeout command)
const show = await $`timeout 5 git ls-remote --symref origin HEAD`.quiet().nothrow().cwd(cwd)
if (show.exitCode === 0) {
const output = new TextDecoder().decode(show.stdout)
// Parse output like: "ref: refs/heads/main HEAD"
const match = output.match(/ref:\s*refs\/heads\/([^\t\n\r]+)/)
if (match?.[1]) {
const branch = match[1].trim()
const validated = safeBranchName(branch)
if (validated) {
log.debug("detected default branch via ls-remote", { branch: validated })
return validated
}
}
}
} catch (e) {
log.debug("ls-remote attempt failed", { error: String(e) })
}

// Last resort default
log.warn("could not detect default branch, using fallback", { fallback: "dev" })
return "dev"
}
209 changes: 209 additions & 0 deletions packages/opencode/test/tasks/pulse-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
import { describe, it, expect, beforeEach, afterEach, mock } from "bun:test"
import path from "path"
import { $ } from "bun"
import { defaultBranch, validateBaseCommit, hasCommittedChanges } from "../../src/tasks/pulse-utils"

describe("pulse-utils", () => {
describe("validateBaseCommit", () => {
it("returns null for undefined", () => {
expect(validateBaseCommit(undefined)).toBeNull()
})

it("returns null for null", () => {
expect(validateBaseCommit(null)).toBeNull()
})

it("validates SHA-1 hashes", () => {
const sha1 = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b"
expect(validateBaseCommit(sha1)).toBe(sha1)
})

it("validates SHA-256 hashes", () => {
const sha256 = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2a1b2c3d4e5f6a1b2c3d4e5"
expect(validateBaseCommit(sha256)).toBe(sha256)
})

it("validates simple branch names", () => {
expect(validateBaseCommit("dev")).toBe("dev")
expect(validateBaseCommit("main")).toBe("main")
expect(validateBaseCommit("feature-name")).toBe("feature-name")
expect(validateBaseCommit("feature/test")).toBe("feature/test")
})

it("rejects command injection attempts", () => {
expect(validateBaseCommit("dev && rm -rf /")).toBeNull()
expect(validateBaseCommit("dev; echo hack")).toBeNull()
expect(validateBaseCommit("`whoami`")).toBeNull()
expect(validateBaseCommit("$(whoami)")).toBeNull()
})

it("rejects names with leading hyphen (flag injection)", () => {
expect(validateBaseCommit("-rf")).toBeNull()
})

it("rejects names with trailing special chars", () => {
expect(validateBaseCommit("dev-")).toBeNull()
})
})

describe("defaultBranch", () => {
let testDir: string

beforeEach(async () => {
// Create a temporary git repository for testing
testDir = `/tmp/test-repo-${Date.now()}`
await $`mkdir -p ${testDir}`.quiet()
await $`cd ${testDir} && git init`.quiet()
await $`cd ${testDir} && git config user.email "test@example.com"`.quiet()
await $`cd ${testDir} && git config user.name "Test User"`.quiet()
})

afterEach(async () => {
// Cleanup test directory
await $`rm -rf ${testDir}`.quiet().nothrow()
})

it("returns dev as fallback when no remote is configured", async () => {
const branch = await defaultBranch(testDir)
expect(branch).toBe("dev")
})

it("detects default branch from git symbolic-ref when available", async () => {
// Create an origin remote
const remoteDir = `/tmp/test-remote-${Date.now()}`
await $`mkdir -p ${remoteDir}`.quiet()
await $`cd ${remoteDir} && git init --bare`.quiet()

// Add remote to test repo
await $`cd ${testDir} && git remote add origin ${remoteDir}`.quiet()

// Create a commit and push
await $`cd ${testDir} && echo "test" > file.txt && git add file.txt && git commit -m "init"`.quiet()
await $`cd ${testDir} && git branch -M main && git push -u origin main 2>/dev/null || true`.quiet().nothrow()

// Set the symbolic ref (this simulates a real GitHub setup)
await $`cd ${remoteDir} && git symbolic-ref HEAD refs/heads/main`.quiet()

// Fetch to update remote tracking branches
await $`cd ${testDir} && git fetch origin 2>/dev/null || true`.quiet().nothrow()

// The defaultBranch function should detect main
const branch = await defaultBranch(testDir)
expect(branch).toBe("main")

// Cleanup remote
await $`rm -rf ${remoteDir}`.quiet()
})

it("handles git errors gracefully", async () => {
// Use a non-existent directory
const branch = await defaultBranch("/non/existent/path")
expect(branch).toBe("dev")
})

it("detects branch from ls-remote when symbolic-ref is not available", async () => {
// Create an origin remote
const remoteDir = `/tmp/test-remote-${Date.now()}`
await $`mkdir -p ${remoteDir}`.quiet()
await $`cd ${remoteDir} && git init --bare`.quiet()

// Add remote to test repo
await $`cd ${testDir} && git remote add origin ${remoteDir}`.quiet()

// Create a commit and push to dev branch
await $`cd ${testDir} && echo "test" > file.txt && git add file.txt && git commit -m "init"`.quiet()
await $`cd ${testDir} && git branch -M dev && git push -u origin dev 2>/dev/null || true`.quiet().nothrow()

// Set the symbolic ref on the remote to dev
await $`cd ${remoteDir} && git symbolic-ref HEAD refs/heads/dev`.quiet()

// Fetch to update remote tracking branches
await $`cd ${testDir} && git fetch origin 2>/dev/null || true`.quiet().nothrow()

// Even though symbolic-ref is set, the fallback ls-remote should also detect it correctly
const branch = await defaultBranch(testDir)
expect(branch).toBe("dev")

// Cleanup remote
await $`rm -rf ${remoteDir}`.quiet()
})

it("validates and sanitizes detected branch names", async () => {
// Create an origin remote
const remoteDir = `/tmp/test-remote-${Date.now()}`
await $`mkdir -p ${remoteDir}`.quiet()
await $`cd ${remoteDir} && git init --bare`.quiet()

// Add remote to test repo
await $`cd ${testDir} && git remote add origin ${remoteDir}`.quiet()

// Create a commit and push
await $`cd ${testDir} && echo "test" > file.txt && git add file.txt && git commit -m "init"`.quiet()
await $`cd ${testDir} && git branch -M main && git push -u origin main 2>/dev/null || true`.quiet().nothrow()

// Set a valid symbolic ref
await $`cd ${remoteDir} && git symbolic-ref HEAD refs/heads/main`.quiet()

// Fetch to update remote tracking branches
await $`cd ${testDir} && git fetch origin 2>/dev/null || true`.quiet().nothrow()

// Should detect main (valid branch name)
const branch = await defaultBranch(testDir)
expect(branch).toBe("main")

// Cleanup remote
await $`rm -rf ${remoteDir}`.quiet()
})

it("falls back to dev when both detection methods fail", async () => {
// Create a local repo with unreachable remote
const localRepo = `/tmp/test-local-${Date.now()}`
await $`mkdir -p ${localRepo}`.quiet()
await $`cd ${localRepo} && git init`.quiet()
await $`cd ${localRepo} && git config user.email "test@example.com"`.quiet()
await $`cd ${localRepo} && git config user.name "Test User"`.quiet()

// Add a fake remote that won't work
await $`cd ${localRepo} && git remote add origin "invalid://nowhere"`.quiet()

// Should fallback to dev
const branch = await defaultBranch(localRepo)
expect(branch).toBe("dev")

// Cleanup
await $`rm -rf ${localRepo}`.quiet()
})
})

describe("hasCommittedChanges", () => {
let testDir: string

beforeEach(async () => {
testDir = `/tmp/test-worktree-${Date.now()}`
await $`mkdir -p ${testDir}`.quiet()
await $`cd ${testDir} && git init`.quiet()
await $`cd ${testDir} && git config user.email "test@example.com"`.quiet()
await $`cd ${testDir} && git config user.name "Test User"`.quiet()
})

afterEach(async () => {
await $`rm -rf ${testDir}`.quiet().nothrow()
})

it("returns true when changes are committed", async () => {
// Create initial commit on 'main' or 'master'
await $`cd ${testDir} && echo "initial" > file.txt && git add file.txt && git commit -m "init"`.quiet()

// Create another branch and make a commit
await $`cd ${testDir} && git checkout -b feature 2>/dev/null || git switch -c feature`.quiet()
await $`cd ${testDir} && echo "changed" > file.txt && git add file.txt && git commit -m "change"`.quiet()

// Get the base commit (which should show differences)
const baseCommit = await $`cd ${testDir} && git rev-parse HEAD~1`.quiet().text()
const base = baseCommit.trim()

const hasChanges = await hasCommittedChanges(testDir, base)
expect(hasChanges).toBe(true)
})
})
})