feat(cli/run, background-agent): manage session permissions for CLI and background tasks
- Deny question prompts in CLI run mode since there's no TUI to answer them
- Inherit parent session permission rules in background task sessions
- Force deny questions while preserving other parent permission settings
- Add test coverage for permission inheritance behavior
🤖 Generated with assistance of OhMyOpenCode
This commit is contained in:
parent
e663d7b335
commit
1324fee30f
@ -1,6 +1,8 @@
|
|||||||
import { describe, it, expect, beforeEach, mock, spyOn } from "bun:test"
|
/// <reference types="bun-types" />
|
||||||
import { resolveSession } from "./session-resolver"
|
|
||||||
import type { OpencodeClient } from "./types"
|
import { beforeEach, describe, expect, it, mock, spyOn } from "bun:test";
|
||||||
|
import { resolveSession } from "./session-resolver";
|
||||||
|
import type { OpencodeClient } from "./types";
|
||||||
|
|
||||||
const createMockClient = (overrides: {
|
const createMockClient = (overrides: {
|
||||||
getResult?: { error?: unknown; data?: { id: string } }
|
getResult?: { error?: unknown; data?: { id: string } }
|
||||||
@ -58,7 +60,9 @@ describe("resolveSession", () => {
|
|||||||
const result = resolveSession({ client: mockClient, sessionId })
|
const result = resolveSession({ client: mockClient, sessionId })
|
||||||
|
|
||||||
// then
|
// then
|
||||||
await expect(result).rejects.toThrow(`Session not found: ${sessionId}`)
|
await Promise.resolve(
|
||||||
|
expect(result).rejects.toThrow(`Session not found: ${sessionId}`)
|
||||||
|
)
|
||||||
expect(mockClient.session.get).toHaveBeenCalledWith({
|
expect(mockClient.session.get).toHaveBeenCalledWith({
|
||||||
path: { id: sessionId },
|
path: { id: sessionId },
|
||||||
})
|
})
|
||||||
@ -77,7 +81,12 @@ describe("resolveSession", () => {
|
|||||||
// then
|
// then
|
||||||
expect(result).toBe("new-session-id")
|
expect(result).toBe("new-session-id")
|
||||||
expect(mockClient.session.create).toHaveBeenCalledWith({
|
expect(mockClient.session.create).toHaveBeenCalledWith({
|
||||||
body: { title: "oh-my-opencode run" },
|
body: {
|
||||||
|
title: "oh-my-opencode run",
|
||||||
|
permission: [
|
||||||
|
{ permission: "question", action: "deny", pattern: "*" },
|
||||||
|
],
|
||||||
|
},
|
||||||
})
|
})
|
||||||
expect(mockClient.session.get).not.toHaveBeenCalled()
|
expect(mockClient.session.get).not.toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
@ -98,7 +107,12 @@ describe("resolveSession", () => {
|
|||||||
expect(result).toBe("retried-session-id")
|
expect(result).toBe("retried-session-id")
|
||||||
expect(mockClient.session.create).toHaveBeenCalledTimes(2)
|
expect(mockClient.session.create).toHaveBeenCalledTimes(2)
|
||||||
expect(mockClient.session.create).toHaveBeenCalledWith({
|
expect(mockClient.session.create).toHaveBeenCalledWith({
|
||||||
body: { title: "oh-my-opencode run" },
|
body: {
|
||||||
|
title: "oh-my-opencode run",
|
||||||
|
permission: [
|
||||||
|
{ permission: "question", action: "deny", pattern: "*" },
|
||||||
|
],
|
||||||
|
},
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
@ -116,7 +130,9 @@ describe("resolveSession", () => {
|
|||||||
const result = resolveSession({ client: mockClient })
|
const result = resolveSession({ client: mockClient })
|
||||||
|
|
||||||
// then
|
// then
|
||||||
await expect(result).rejects.toThrow("Failed to create session after all retries")
|
await Promise.resolve(
|
||||||
|
expect(result).rejects.toThrow("Failed to create session after all retries")
|
||||||
|
)
|
||||||
expect(mockClient.session.create).toHaveBeenCalledTimes(3)
|
expect(mockClient.session.create).toHaveBeenCalledTimes(3)
|
||||||
})
|
})
|
||||||
|
|
||||||
@ -134,7 +150,9 @@ describe("resolveSession", () => {
|
|||||||
const result = resolveSession({ client: mockClient })
|
const result = resolveSession({ client: mockClient })
|
||||||
|
|
||||||
// then
|
// then
|
||||||
await expect(result).rejects.toThrow("Failed to create session after all retries")
|
await Promise.resolve(
|
||||||
|
expect(result).rejects.toThrow("Failed to create session after all retries")
|
||||||
|
)
|
||||||
expect(mockClient.session.create).toHaveBeenCalledTimes(3)
|
expect(mockClient.session.create).toHaveBeenCalledTimes(3)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@ -19,14 +19,18 @@ export async function resolveSession(options: {
|
|||||||
return sessionId
|
return sessionId
|
||||||
}
|
}
|
||||||
|
|
||||||
let lastError: unknown
|
|
||||||
for (let attempt = 1; attempt <= SESSION_CREATE_MAX_RETRIES; attempt++) {
|
for (let attempt = 1; attempt <= SESSION_CREATE_MAX_RETRIES; attempt++) {
|
||||||
const res = await client.session.create({
|
const res = await client.session.create({
|
||||||
body: { title: "oh-my-opencode run" },
|
body: {
|
||||||
|
title: "oh-my-opencode run",
|
||||||
|
// In CLI run mode there's no TUI to answer questions.
|
||||||
|
permission: [
|
||||||
|
{ permission: "question", action: "deny" as const, pattern: "*" },
|
||||||
|
],
|
||||||
|
} as any,
|
||||||
})
|
})
|
||||||
|
|
||||||
if (res.error) {
|
if (res.error) {
|
||||||
lastError = res.error
|
|
||||||
console.error(
|
console.error(
|
||||||
pc.yellow(`Session create attempt ${attempt}/${SESSION_CREATE_MAX_RETRIES} failed:`)
|
pc.yellow(`Session create attempt ${attempt}/${SESSION_CREATE_MAX_RETRIES} failed:`)
|
||||||
)
|
)
|
||||||
@ -44,9 +48,6 @@ export async function resolveSession(options: {
|
|||||||
return res.data.id
|
return res.data.id
|
||||||
}
|
}
|
||||||
|
|
||||||
lastError = new Error(
|
|
||||||
`Unexpected response: ${JSON.stringify(res, null, 2)}`
|
|
||||||
)
|
|
||||||
console.error(
|
console.error(
|
||||||
pc.yellow(
|
pc.yellow(
|
||||||
`Session create attempt ${attempt}/${SESSION_CREATE_MAX_RETRIES}: No session ID returned`
|
`Session create attempt ${attempt}/${SESSION_CREATE_MAX_RETRIES}: No session ID returned`
|
||||||
|
|||||||
@ -1415,7 +1415,7 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
|
|||||||
function createMockClient() {
|
function createMockClient() {
|
||||||
return {
|
return {
|
||||||
session: {
|
session: {
|
||||||
create: async () => ({ data: { id: `ses_${crypto.randomUUID()}` } }),
|
create: async (_args?: any) => ({ data: { id: `ses_${crypto.randomUUID()}` } }),
|
||||||
get: async () => ({ data: { directory: "/test/dir" } }),
|
get: async () => ({ data: { directory: "/test/dir" } }),
|
||||||
prompt: async () => ({}),
|
prompt: async () => ({}),
|
||||||
promptAsync: async () => ({}),
|
promptAsync: async () => ({}),
|
||||||
@ -1520,6 +1520,55 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("task transitions pending→running when slot available", () => {
|
describe("task transitions pending→running when slot available", () => {
|
||||||
|
test("should inherit parent session permission rules (and force deny question)", async () => {
|
||||||
|
// given
|
||||||
|
const createCalls: any[] = []
|
||||||
|
const parentPermission = [
|
||||||
|
{ permission: "question", action: "allow" as const, pattern: "*" },
|
||||||
|
{ permission: "plan_enter", action: "deny" as const, pattern: "*" },
|
||||||
|
]
|
||||||
|
|
||||||
|
const customClient = {
|
||||||
|
session: {
|
||||||
|
create: async (args?: any) => {
|
||||||
|
createCalls.push(args)
|
||||||
|
return { data: { id: `ses_${crypto.randomUUID()}` } }
|
||||||
|
},
|
||||||
|
get: async () => ({ data: { directory: "/test/dir", permission: parentPermission } }),
|
||||||
|
prompt: async () => ({}),
|
||||||
|
promptAsync: async () => ({}),
|
||||||
|
messages: async () => ({ data: [] }),
|
||||||
|
todo: async () => ({ data: [] }),
|
||||||
|
status: async () => ({ data: {} }),
|
||||||
|
abort: async () => ({}),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
manager.shutdown()
|
||||||
|
manager = new BackgroundManager({ client: customClient, directory: tmpdir() } as unknown as PluginInput, {
|
||||||
|
defaultConcurrency: 5,
|
||||||
|
})
|
||||||
|
|
||||||
|
const input = {
|
||||||
|
description: "Test task",
|
||||||
|
prompt: "Do something",
|
||||||
|
agent: "test-agent",
|
||||||
|
parentSessionID: "parent-session",
|
||||||
|
parentMessageID: "parent-message",
|
||||||
|
}
|
||||||
|
|
||||||
|
// when
|
||||||
|
await manager.launch(input)
|
||||||
|
await new Promise(resolve => setTimeout(resolve, 50))
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(createCalls).toHaveLength(1)
|
||||||
|
const permission = createCalls[0]?.body?.permission
|
||||||
|
expect(permission).toEqual([
|
||||||
|
{ permission: "plan_enter", action: "deny", pattern: "*" },
|
||||||
|
{ permission: "question", action: "deny", pattern: "*" },
|
||||||
|
])
|
||||||
|
})
|
||||||
|
|
||||||
test("should transition first task to running immediately", async () => {
|
test("should transition first task to running immediately", async () => {
|
||||||
// given
|
// given
|
||||||
const config = { defaultConcurrency: 5 }
|
const config = { defaultConcurrency: 5 }
|
||||||
|
|||||||
@ -236,13 +236,17 @@ export class BackgroundManager {
|
|||||||
const parentDirectory = parentSession?.data?.directory ?? this.directory
|
const parentDirectory = parentSession?.data?.directory ?? this.directory
|
||||||
log(`[background-agent] Parent dir: ${parentSession?.data?.directory}, using: ${parentDirectory}`)
|
log(`[background-agent] Parent dir: ${parentSession?.data?.directory}, using: ${parentDirectory}`)
|
||||||
|
|
||||||
|
const inheritedPermission = (parentSession as any)?.data?.permission
|
||||||
|
const permissionRules = Array.isArray(inheritedPermission)
|
||||||
|
? inheritedPermission.filter((r: any) => r?.permission !== "question")
|
||||||
|
: []
|
||||||
|
permissionRules.push({ permission: "question", action: "deny" as const, pattern: "*" })
|
||||||
|
|
||||||
const createResult = await this.client.session.create({
|
const createResult = await this.client.session.create({
|
||||||
body: {
|
body: {
|
||||||
parentID: input.parentSessionID,
|
parentID: input.parentSessionID,
|
||||||
title: `${input.description} (@${input.agent} subagent)`,
|
title: `${input.description} (@${input.agent} subagent)`,
|
||||||
permission: [
|
permission: permissionRules,
|
||||||
{ permission: "question", action: "deny" as const, pattern: "*" },
|
|
||||||
],
|
|
||||||
} as any,
|
} as any,
|
||||||
query: {
|
query: {
|
||||||
directory: parentDirectory,
|
directory: parentDirectory,
|
||||||
|
|||||||
65
src/features/background-agent/spawner.test.ts
Normal file
65
src/features/background-agent/spawner.test.ts
Normal file
@ -0,0 +1,65 @@
|
|||||||
|
import { describe, test, expect } from "bun:test"
|
||||||
|
|
||||||
|
import { createTask, startTask } from "./spawner"
|
||||||
|
|
||||||
|
describe("background-agent spawner.startTask", () => {
|
||||||
|
test("should inherit parent session permission rules (and force deny question)", async () => {
|
||||||
|
//#given
|
||||||
|
const createCalls: any[] = []
|
||||||
|
const parentPermission = [
|
||||||
|
{ permission: "question", action: "allow" as const, pattern: "*" },
|
||||||
|
{ permission: "plan_enter", action: "deny" as const, pattern: "*" },
|
||||||
|
]
|
||||||
|
|
||||||
|
const client = {
|
||||||
|
session: {
|
||||||
|
get: async () => ({ data: { directory: "/parent/dir", permission: parentPermission } }),
|
||||||
|
create: async (args?: any) => {
|
||||||
|
createCalls.push(args)
|
||||||
|
return { data: { id: "ses_child" } }
|
||||||
|
},
|
||||||
|
promptAsync: async () => ({}),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const task = createTask({
|
||||||
|
description: "Test task",
|
||||||
|
prompt: "Do work",
|
||||||
|
agent: "explore",
|
||||||
|
parentSessionID: "ses_parent",
|
||||||
|
parentMessageID: "msg_parent",
|
||||||
|
})
|
||||||
|
|
||||||
|
const item = {
|
||||||
|
task,
|
||||||
|
input: {
|
||||||
|
description: task.description,
|
||||||
|
prompt: task.prompt,
|
||||||
|
agent: task.agent,
|
||||||
|
parentSessionID: task.parentSessionID,
|
||||||
|
parentMessageID: task.parentMessageID,
|
||||||
|
parentModel: task.parentModel,
|
||||||
|
parentAgent: task.parentAgent,
|
||||||
|
model: task.model,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const ctx = {
|
||||||
|
client,
|
||||||
|
directory: "/fallback",
|
||||||
|
concurrencyManager: { release: () => {} },
|
||||||
|
tmuxEnabled: false,
|
||||||
|
onTaskError: () => {},
|
||||||
|
}
|
||||||
|
|
||||||
|
//#when
|
||||||
|
await startTask(item as any, ctx as any)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(createCalls).toHaveLength(1)
|
||||||
|
expect(createCalls[0]?.body?.permission).toEqual([
|
||||||
|
{ permission: "plan_enter", action: "deny", pattern: "*" },
|
||||||
|
{ permission: "question", action: "deny", pattern: "*" },
|
||||||
|
])
|
||||||
|
})
|
||||||
|
})
|
||||||
@ -58,13 +58,17 @@ export async function startTask(
|
|||||||
const parentDirectory = parentSession?.data?.directory ?? directory
|
const parentDirectory = parentSession?.data?.directory ?? directory
|
||||||
log(`[background-agent] Parent dir: ${parentSession?.data?.directory}, using: ${parentDirectory}`)
|
log(`[background-agent] Parent dir: ${parentSession?.data?.directory}, using: ${parentDirectory}`)
|
||||||
|
|
||||||
|
const inheritedPermission = (parentSession as any)?.data?.permission
|
||||||
|
const permissionRules = Array.isArray(inheritedPermission)
|
||||||
|
? inheritedPermission.filter((r: any) => r?.permission !== "question")
|
||||||
|
: []
|
||||||
|
permissionRules.push({ permission: "question", action: "deny" as const, pattern: "*" })
|
||||||
|
|
||||||
const createResult = await client.session.create({
|
const createResult = await client.session.create({
|
||||||
body: {
|
body: {
|
||||||
parentID: input.parentSessionID,
|
parentID: input.parentSessionID,
|
||||||
title: `Background: ${input.description}`,
|
title: `Background: ${input.description}`,
|
||||||
permission: [
|
permission: permissionRules,
|
||||||
{ permission: "question", action: "deny" as const, pattern: "*" },
|
|
||||||
],
|
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
} as any,
|
} as any,
|
||||||
query: {
|
query: {
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user