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
11 changes: 10 additions & 1 deletion src/tools/team-spawn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ export async function executeTeamSpawn(
.get(teamInfo.teamId, args.name)
if (existing) throw new Error(`Teammate "${args.name}" already exists in team "${teamInfo.teamName}"`)

const isReadOnly = agent === "plan" || agent === "explore"
// Prefer the agent's own permission rules (V2) so any agent that denies
// edits — not only the built-in plan/explore — is treated as read-only and
// never gets a worktree or a shell allow. Fall back to the built-in names.
const isReadOnly = (await deps.client.agent?.readOnly(agent).catch(() => undefined))
?? (agent === "plan" || agent === "explore")
const useWorktree = args.worktree !== false && !isReadOnly && !isWorktreeDirectory(deps.directory)
const usePlanApproval = args.plan_approval === true

Expand Down Expand Up @@ -167,6 +171,11 @@ export async function executeTeamSpawn(
if (worktreeDir) {
permission.push(
{ permission: "edit", pattern: `${worktreeDir}/**`, action: "allow" },
// V2 checks files inside the session's location as paths relative to
// it (the teammate's location is the worktree), so the absolute pattern
// above never matches there and edits fell back to an unanswerable ask.
// Files outside the worktree remain gated by external_directory.
{ permission: "edit", pattern: "*", action: "allow" },
)
if (!isReadOnly) {
permission.push({ permission: "bash", pattern: "*", action: "allow" })
Expand Down
17 changes: 17 additions & 0 deletions src/tools/team-tasks-add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,23 @@ export async function executeTeamTasksAdd(
): Promise<string> {
const teamInfo = requireTeamMember(deps, sessionId)

// Reject unknown dependency IDs before inserting anything: a task blocked on
// an ID that never existed can never unblock (issue #40). IDs are generated
// here, so new tasks can only depend on existing ones and no cycle can form.
const unknown = new Set<string>()
for (const task of args.tasks) {
for (const depId of task.depends_on ?? []) {
const exists = deps.db.query("SELECT 1 FROM team_task WHERE id = ? AND team_id = ?").get(depId, teamInfo.teamId)
if (!exists) unknown.add(depId)
}
}
if (unknown.size > 0) {
throw new Error(
`Unknown task ID${unknown.size > 1 ? "s" : ""} in depends_on: ${[...unknown].join(", ")}. ` +
"Use IDs from team_tasks_list, or add the prerequisite tasks first and reference the IDs they return.",
)
}

const ids: string[] = []
const now = Date.now()

Expand Down
4 changes: 4 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ export interface PermissionRule {
* Makes mocking trivial in tests.
*/
export interface PluginClient {
/** Optional agent introspection (V2). Undefined answers fall back to name checks. */
agent?: {
readOnly(agent: string): Promise<boolean | undefined>
}
session: {
create(options: {
parentID?: string
Expand Down
89 changes: 76 additions & 13 deletions src/v2-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { runCommand } from "./process"

/** V2 plugin context subset Ensemble needs (structural, mock-friendly). */
export interface V2Context {
/** Plugin location; `project.id` is required by the 2.0.19+ worktree API. */
location?: { directory?: string; project?: { id?: string } }
session: {
create(input: Record<string, unknown>): Promise<{ id: string }>
prompt(input: Record<string, unknown>): Promise<unknown>
Expand All @@ -17,12 +19,39 @@ export interface V2Context {
worktree: {
create(input: Record<string, unknown>): Promise<{ directory: string }>
remove(input: Record<string, unknown>): Promise<unknown>
list(): Promise<Array<{ directory: string }>>
refresh(): Promise<unknown>
list(input?: Record<string, unknown>): Promise<Array<{ directory: string }>>
refresh(input?: Record<string, unknown>): Promise<unknown>
}
permission: {
rules(input: Record<string, unknown>): Promise<unknown>
/** Present on 2.0.3–2.0.18 only; removed in 2.0.19 (see issue #40). */
rules?(input: Record<string, unknown>): Promise<unknown>
}
/** Agent lookup (2.0.19+): exposes each agent's resolved permission rules. */
agent?: {
get(input: Record<string, unknown>): Promise<unknown>
}
}

type PermissionRuleV2 = { action?: unknown; resource?: unknown; effect?: unknown }

/** OpenCode wildcard match: `*` matches any run of characters. */
function wildcardMatch(value: string, pattern: string): boolean {
const escaped = pattern.replace(/[.+?^${}()|[\]\\]/g, "\\$&").replace(/\*/g, ".*")
return new RegExp(`^${escaped}$`).test(value)
}

/**
* True when an agent's own rules deny editing everywhere. OpenCode evaluates
* the last matching rule, so this mirrors `evaluate("edit", "*")` over the
* agent ruleset. Undefined when the rules carry no edit decision.
*/
export function agentRulesAreReadOnly(rules: unknown): boolean | undefined {
if (!Array.isArray(rules)) return undefined
const match = [...(rules as PermissionRuleV2[])].reverse().find((rule) =>
typeof rule?.action === "string" && typeof rule?.resource === "string"
&& wildcardMatch("edit", rule.action) && (rule.resource === "*" || rule.resource === "**"))
if (!match) return undefined
return match.effect === "deny"
}

/** Prefix marking a workspace id that is really a local directory (see below). */
Expand All @@ -32,6 +61,8 @@ const DIR_WORKSPACE_PREFIX = "v2dir:"
export interface V2ClientDeps {
/** Current branch checked out in a directory, or null when unknown. */
gitBranch?: (directory: string) => Promise<string | null>
/** Create and check out a branch in a directory (best-effort). */
gitCreateBranch?: (directory: string, branch: string) => Promise<void>
/** Worktree directory currently on a branch, or null when none. */
gitDir?: (branch: string) => Promise<string | null>
/** RPC emitter for companion events (team_view navigation). */
Expand All @@ -49,6 +80,15 @@ async function gitBranchDefault(directory: string): Promise<string | null> {
}
}

/** Create and check out a branch in a directory via git. */
async function gitCreateBranchDefault(directory: string, branch: string): Promise<void> {
try {
await runCommand(["git", "switch", "-c", branch], { cwd: directory })
} catch {
// Merge reports the missing branch; spawning must not fail here.
}
}

/** Concatenate the text parts of a promptAsync payload into one string. */
function joinParts(parts: Array<{ type: "text"; text: string }>): string {
return parts
Expand Down Expand Up @@ -88,7 +128,24 @@ function joinParts(parts: Array<{ type: "text"; text: string }>): string {
*/
export function createV2Client(ctx: V2Context, deps: V2ClientDeps = {}): PluginClient {
const gitBranch = deps.gitBranch ?? gitBranchDefault
const gitCreateBranch = deps.gitCreateBranch ?? gitCreateBranchDefault
// 2.0.19+ worktree endpoints require the project; older hosts ignore it.
const project = (): Record<string, unknown> => {
const projectID = ctx.location?.project?.id
return projectID ? { projectID } : {}
}
return {
agent: {
readOnly: async (agent: string) => {
if (!ctx.agent) return undefined
try {
const info = (await ctx.agent.get({ agentID: agent })) as { permissions?: unknown; data?: { permissions?: unknown } } | undefined
return agentRulesAreReadOnly(info?.permissions ?? info?.data?.permissions)
} catch {
return undefined
}
},
},
session: {
create: async (options) => {
const input: Record<string, unknown> = { title: options.title }
Expand All @@ -114,11 +171,12 @@ export function createV2Client(ctx: V2Context, deps: V2ClientDeps = {}): PluginC
}
}
const created = await ctx.session.create(input)
if (permissions) {
// Session.create permissions lose to agent rules (last match wins
// and agent rules apply later), which strips team tools from
// read-only agents. Re-assert here: rules() evaluates after the
// agent's rules, restoring the tools. OQ-V2-rules.
if (permissions && typeof ctx.permission.rules === "function") {
// 2.0.3–2.0.18 merged agent rules after session rules, so create()
// permissions lost to the agent's and had to be re-asserted. From
// 2.0.19 the session ruleset merges last (core permission.ts
// `merge(agent.permissions, session.permissions)` with last-match
// evaluation) and rules() no longer exists, so create() is enough.
await ctx.permission.rules({ sessionID: created.id, permissions })
}
return { data: { id: created.id } }
Expand Down Expand Up @@ -177,16 +235,21 @@ export function createV2Client(ctx: V2Context, deps: V2ClientDeps = {}): PluginC
worktree: {
create: async (options) => {
const name = (options.worktreeCreateInput as { name?: string } | undefined)?.name ?? "ensemble-worktree"
const created = await ctx.worktree.create({ name })
const branch = (await gitBranch(created.directory)) ?? name
const created = await ctx.worktree.create({ ...project(), name })
let branch = await gitBranch(created.directory)
if (!branch) {
// 2.0.32+ creates worktrees detached; team_merge needs a named branch.
await gitCreateBranch(created.directory, name)
branch = name
}
return { data: { name, branch, directory: created.directory } }
},
remove: async (options) => {
const directory = (options.worktreeRemoveInput as { directory: string }).directory
return ctx.worktree.remove({ directory, force: false })
return ctx.worktree.remove({ ...project(), directory, force: false })
},
list: async () => {
const entries = await ctx.worktree.list()
const entries = await ctx.worktree.list(project())
return {
data: entries.map((entry) => ({
name: entry.directory.split("/").pop() ?? entry.directory,
Expand All @@ -195,7 +258,7 @@ export function createV2Client(ctx: V2Context, deps: V2ClientDeps = {}): PluginC
})),
}
},
reset: async () => ctx.worktree.refresh(),
reset: async () => ctx.worktree.refresh(project()),
},
workspace: {
create: async (options) => {
Expand Down
3 changes: 2 additions & 1 deletion src/v2-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ import { vlog } from "./log"

/** V2 setup context subset (structural, mock-friendly). */
export interface V2SetupContext {
location: { directory: string }
location: { directory: string; project?: { id?: string } }
agent?: V2Context["agent"]
options: Record<string, unknown>
session: V2Context["session"] & {
hook(name: string, cb: (event: never) => unknown): Promise<{ dispose(): Promise<void> }>
Expand Down
19 changes: 19 additions & 0 deletions test/tools/team-spawn.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -830,11 +830,30 @@ describe("team_spawn — agent mode enforcement", () => {
const opts = createCall!.args[0] as { permission?: Array<{ permission: string; pattern: string; action: string }> }
expect(opts.permission).toEqual([
{ permission: "edit", pattern: "/tmp/worktree-ensemble-test-project-my-team#t1-builder/**", action: "allow" },
{ permission: "edit", pattern: "*", action: "allow" },
{ permission: "bash", pattern: "*", action: "allow" },
...TEAM_TOOL_PERMISSIONS,
])
})

test("an agent whose own rules deny edits is spawned read-only without a worktree", async () => {
const client = deps.client as unknown as { agent?: { readOnly(agent: string): Promise<boolean | undefined> } }
client.agent = { readOnly: async (agent) => agent === "code-reviewer" }
try {
await executeTeamSpawn(deps, { name: "reviewer", agent: "code-reviewer", prompt: "Review it" }, "lead-sess")
expect(deps.client.calls.some(c => c.method === "worktree.create")).toBe(false)
const createCall = deps.client.calls.find(c => c.method === "session.create")
const opts = createCall!.args[0] as { permission?: Array<{ permission: string; pattern: string; action: string }> }
expect(opts.permission).toEqual([
{ permission: "edit", pattern: "*", action: "deny" },
{ permission: "bash", pattern: "*", action: "deny" },
...TEAM_TOOL_PERMISSIONS,
])
} finally {
delete client.agent
}
})

test("plan agent gets agent type on promptAsync without deprecated tools field", async () => {
await executeTeamSpawn(deps, { name: "planner", agent: "plan", prompt: "Plan it" }, "lead-sess")

Expand Down
11 changes: 11 additions & 0 deletions test/tools/team-tasks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ describe("team_tasks_list", () => {
expect(result).toContain("pending")
})

test("rejects unknown depends_on IDs without adding any task (issue #40)", async () => {
await executeTeamTasksAdd(deps, { tasks: [{ content: "Task A", priority: "high" }] }, "sess-alice")
const before = (deps.db.query("SELECT COUNT(*) as c FROM team_task").get() as { c: number }).c
await expect(executeTeamTasksAdd(deps, { tasks: [
{ content: "Task B", priority: "medium" },
{ content: "Task C", priority: "medium", depends_on: ["task_does_not_exist"] },
] }, "sess-alice")).rejects.toThrow(/Unknown task ID in depends_on: task_does_not_exist/)
const after = (deps.db.query("SELECT COUNT(*) as c FROM team_task").get() as { c: number }).c
expect(after).toBe(before)
})

test("rejects if not in a team", async () => {
await expect(executeTeamTasksList(deps, "random-sess"))
.rejects.toThrow("not in a team")
Expand Down
86 changes: 80 additions & 6 deletions test/v2-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ function mockCtx() {
calls.push({ domain: "worktree", method: "remove", input })
return undefined
},
list: async () => {
calls.push({ domain: "worktree", method: "list", input: {} })
list: async (input: Record<string, unknown> = {}) => {
calls.push({ domain: "worktree", method: "list", input })
return [{ directory: "/wt/ensemble-proj-team-alice" }, { directory: "/wt/other" }]
},
refresh: async () => {
calls.push({ domain: "worktree", method: "refresh", input: {} })
refresh: async (input: Record<string, unknown> = {}) => {
calls.push({ domain: "worktree", method: "refresh", input })
return undefined
},
},
Expand Down Expand Up @@ -218,11 +218,32 @@ describe("v2-client adapter (issue #36, DRY: reuses all tool logic)", () => {
})
})

test("worktree.create falls back to the requested name when git lookup fails", async () => {
test("worktree.create names a detached worktree after the requested name", async () => {
const { ctx } = mockCtx()
const client = createV2Client(ctx as never, { gitBranch: mockGit({}) })
const created: Array<[string, string]> = []
const client = createV2Client(ctx as never, {
gitBranch: mockGit({}),
gitCreateBranch: async (directory, branch) => {
created.push([directory, branch])
},
})
const result = await client.worktree.create({ worktreeCreateInput: { name: "ensemble-x" } })
expect(result.data?.branch).toBe("ensemble-x")
expect(created).toEqual([["/wt/ensemble-proj-team-alice", "ensemble-x"]])
})

test("worktree.create leaves a worktree that is already on a branch alone", async () => {
const { ctx } = mockCtx()
let createCalls = 0
const client = createV2Client(ctx as never, {
gitBranch: async () => "existing",
gitCreateBranch: async () => {
createCalls++
},
})
const result = await client.worktree.create({ worktreeCreateInput: { name: "ensemble-x" } })
expect(result.data?.branch).toBe("existing")
expect(createCalls).toBe(0)
})

test("worktree.remove/list/reset map to V2 remove/list/refresh", async () => {
Expand Down Expand Up @@ -261,3 +282,56 @@ describe("v2-client adapter (issue #36, DRY: reuses all tool logic)", () => {
expect(calls.length).toBe(0)
})
})

describe("createV2Client — OpenCode 2.0.19+ compatibility (issue #40)", () => {
test("session.create works when permission.rules no longer exists", async () => {
const { calls, ctx } = mockCtx()
const { rules: _removed, ...permission } = ctx.permission
const client = createV2Client({ ...ctx, permission } as never)
const result = await client.session.create({
title: "alice",
permission: [{ permission: "team_message", pattern: "*", action: "allow" }],
})
expect(result.data?.id).toBe("ses_new123")
expect(calls.find((c) => c.method === "create")?.input).toMatchObject({
permissions: [{ action: "team_message", resource: "*", effect: "allow" }],
})
expect(calls.some((c) => c.method === "rules")).toBe(false)
})

test("worktree calls pass the location project id required by 2.0.19+", async () => {
const { calls, ctx } = mockCtx()
const client = createV2Client({ ...ctx, location: { directory: "/p", project: { id: "prj_1" } } } as never, {
gitBranch: async () => "b",
})
await client.worktree.create({ worktreeCreateInput: { name: "ensemble-x" } })
await client.worktree.remove({ worktreeRemoveInput: { directory: "/wt/ensemble-x" } })
await client.worktree.list()
await client.worktree.reset({ worktreeResetInput: { directory: "/wt/ensemble-x" } })
const worktreeCalls = calls.filter((c) => c.domain === "worktree")
expect(worktreeCalls.map((c) => (c.input as { projectID?: string }).projectID)).toEqual(["prj_1", "prj_1", "prj_1", "prj_1"])
expect(worktreeCalls[0]?.input).toMatchObject({ projectID: "prj_1", name: "ensemble-x" })
})

test("agent.readOnly reads the agent's last matching edit rule", async () => {
const { ctx } = mockCtx()
const rulesFor: Record<string, unknown> = {
reviewer: [{ action: "*", resource: "*", effect: "allow" }, { action: "edit", resource: "*", effect: "deny" }],
builder: [{ action: "edit", resource: "*", effect: "deny" }, { action: "*", resource: "*", effect: "allow" }],
plain: [{ action: "read", resource: "*", effect: "allow" }],
}
const client = createV2Client({
...ctx,
agent: { get: async (input: Record<string, unknown>) => ({ permissions: rulesFor[input["agentID"] as string] }) },
} as never)
expect(await client.agent?.readOnly("reviewer")).toBe(true)
expect(await client.agent?.readOnly("builder")).toBe(false)
expect(await client.agent?.readOnly("plain")).toBeUndefined()
})

test("agent.readOnly is undefined when the host has no agent domain", async () => {
const { ctx } = mockCtx()
const client = createV2Client(ctx as never)
expect(await client.agent?.readOnly("anything")).toBeUndefined()
})
})
Loading