diff --git a/packages/opencode/src/kilo-sessions/kilo-sessions.ts b/packages/opencode/src/kilo-sessions/kilo-sessions.ts index 0355bef27d7..7d2aecf2c4f 100644 --- a/packages/opencode/src/kilo-sessions/kilo-sessions.ts +++ b/packages/opencode/src/kilo-sessions/kilo-sessions.ts @@ -190,13 +190,15 @@ export namespace KiloSessions { const STATUS_TIMEOUT_MS = 3_000 async function deriveStatus(sessionID: string): Promise<"idle" | "busy" | "question" | "permission" | "retry"> { - const permissions = (await Permission.list()).filter((p) => p.sessionID === sessionID) + const { AppRuntime } = await import("@/effect/app-runtime") + const permissions = ( + await AppRuntime.runPromise(Permission.Service.use((svc) => svc.list())) + ).filter((p) => p.sessionID === sessionID) if (permissions.length > 0) return "permission" const questions = (await Question.list()).filter((q) => q.sessionID === sessionID) if (questions.length > 0) return "question" - const { AppRuntime } = await import("@/effect/app-runtime") const status = await AppRuntime.runPromise(SessionStatus.Service.use((svc) => svc.get(SessionID.make(sessionID)))) if (status.type === "offline") return "retry" return status.type diff --git a/packages/opencode/src/kilo-sessions/remote-sender.ts b/packages/opencode/src/kilo-sessions/remote-sender.ts index 31d4ee52ecf..38a70975b3f 100644 --- a/packages/opencode/src/kilo-sessions/remote-sender.ts +++ b/packages/opencode/src/kilo-sessions/remote-sender.ts @@ -72,6 +72,10 @@ export namespace RemoteSender { } subscribe?: (callback: (event: any) => void) => () => void provide?: Provide + permission?: { + readonly list: () => Promise> + readonly reply: (input: Permission.ReplyInput) => Promise + } } export type Sender = { @@ -83,6 +87,16 @@ export namespace RemoteSender { const sessions = new Set() const children = new Map() // childId → parentId let unsub: (() => void) | undefined + const permission = options.permission ?? { + list: async () => { + const { AppRuntime } = await import("@/effect/app-runtime") + return AppRuntime.runPromise(Permission.Service.use((svc) => svc.list())) + }, + reply: async (input: Permission.ReplyInput) => { + const { AppRuntime } = await import("@/effect/app-runtime") + return AppRuntime.runPromise(Permission.Service.use((svc) => svc.reply(input))) + }, + } const sub = options.subscribe ?? @@ -133,7 +147,7 @@ export namespace RemoteSender { const [suggestions, questions, permissions] = await Promise.all([ Suggestion.list(), Question.list(), - Permission.list(), + permission.list(), ]) for (const suggestion of suggestions) { if (suggestion.sessionID !== sessionId) continue @@ -359,12 +373,7 @@ export namespace RemoteSender { } const dir = msg.sessionId ? directoryFor(msg.sessionId) : Promise.resolve(options.directory) dispatchQuick(msg, dir, async () => { - const { AppRuntime } = await import("@/effect/app-runtime") - await AppRuntime.runPromise( - Permission.Service.use((svc) => - svc.reply({ ...parsed.data, requestID: PermissionID.make(parsed.data.requestID) }), - ), - ) + await permission.reply({ ...parsed.data, requestID: PermissionID.make(parsed.data.requestID) }) }) return } diff --git a/packages/opencode/src/permission/index.ts b/packages/opencode/src/permission/index.ts index 60a23d4b48e..c5ee7406378 100644 --- a/packages/opencode/src/permission/index.ts +++ b/packages/opencode/src/permission/index.ts @@ -17,7 +17,6 @@ import os from "os" import z from "zod" // kilocode_change import { evaluate as evalRule } from "./evaluate" import { PermissionID } from "./schema" -import { makeRuntime } from "@/effect/run-service" // kilocode_change import { ConfigProtection } from "@/kilocode/permission/config-paths" // kilocode_change import { Identifier } from "@/id/id" // kilocode_change import { drainCovered } from "@/kilocode/permission/drain" // kilocode_change @@ -553,16 +552,4 @@ export function toConfig(rules: Ruleset): ConfigPermission.Info { } // kilocode_change end -// kilocode_change start - legacy promise helpers for Kilo callsites -const { runPromise } = makeRuntime(Service, defaultLayer) -export const list = () => runPromise((svc) => svc.list()) -export const ask = (input: AskInput) => runPromise((svc) => svc.ask(input)) -const replyPromise = (input: ReplyInput) => runPromise((svc) => svc.reply(input)) -export { replyPromise as reply } -export const saveAlwaysRules = (input: z.infer) => - runPromise((svc) => svc.saveAlwaysRules(input)) -export const allowEverything = (input: z.infer) => - runPromise((svc) => svc.allowEverything(input)) -// kilocode_change end - export * as Permission from "." diff --git a/packages/opencode/test/kilocode/permission/external-directory-allow.test.ts b/packages/opencode/test/kilocode/permission/external-directory-allow.test.ts index d5e42127027..3b4c4e6e9a3 100644 --- a/packages/opencode/test/kilocode/permission/external-directory-allow.test.ts +++ b/packages/opencode/test/kilocode/permission/external-directory-allow.test.ts @@ -1,10 +1,11 @@ import { afterEach, describe, expect, test } from "bun:test" -import { Effect, Layer, ManagedRuntime } from "effect" +import { Cause, Effect, Exit, Fiber, Layer, ManagedRuntime } from "effect" import path from "path" import { AppFileSystem } from "@opencode-ai/core/filesystem" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { Global } from "@opencode-ai/core/global" import { Agent } from "../../../src/agent/agent" +import { Bus } from "../../../src/bus" import { Config } from "../../../src/config/config" import { Permission } from "../../../src/permission" import { PermissionID } from "../../../src/permission/schema" @@ -14,7 +15,8 @@ import { Shell } from "../../../src/shell/shell" import { Truncate } from "../../../src/tool/truncate" import { ShellTool } from "../../../src/tool/shell" import { Plugin } from "../../../src/plugin" -import { disposeAllInstances, tmpdir } from "../../fixture/fixture" +import { disposeAllInstances, provideTmpdirInstance, tmpdir } from "../../fixture/fixture" +import { testEffect } from "../../lib/effect" import { ConfigProtection } from "../../../src/kilocode/permission/config-paths" const runtime = ManagedRuntime.make( @@ -65,6 +67,31 @@ const variants = (dir: string) => { const config = path.resolve(Global.Path.config) const configFile = path.join(config, "hello.txt") const configGlob = glob(path.join(config, "*")) +const bus = Bus.layer +const env = Layer.mergeAll( + Permission.layer.pipe(Layer.provide(bus), Layer.provide(Config.defaultLayer)), + bus, + CrossSpawnSpawner.defaultLayer, +) +const it = testEffect(env) + +const ask = (input: Permission.AskInput) => + Effect.gen(function* () { + const permission = yield* Permission.Service + return yield* permission.ask(input) + }) + +const reply = (input: Permission.ReplyInput) => + Effect.gen(function* () { + const permission = yield* Permission.Service + return yield* permission.reply(input) + }) + +const list = () => + Effect.gen(function* () { + const permission = yield* Permission.Service + return yield* permission.list() + }) const capture = (requests: Array>, stop?: Error) => ({ ...ctx, @@ -90,52 +117,46 @@ const withShell = (item: { shell: string }, fn: () => Promise) => async () } } -async function reject() { - const requests = await Permission.list() - for (const req of requests) { - await Permission.reply({ requestID: req.id, reply: "reject" }) - } -} - -async function immediate(pending: Promise) { - try { - await Promise.race([ - pending, - new Promise((_, reject) => - setTimeout(() => reject(new Error("timed out waiting for permission to resolve")), 500), - ), - ]) - } finally { - const requests = await Permission.list() - if (requests.length > 0) { - await reject() - await pending.catch(() => undefined) +const reject = () => + Effect.gen(function* () { + for (const req of yield* list()) { + yield* reply({ requestID: req.id, reply: "reject" }) } - } - expect(await Permission.list()).toHaveLength(0) -} + }) -async function wait(count: number) { - for (const _ of Array.from({ length: 500 })) { - const list = await Permission.list() - if (list.length === count) return list - await Bun.sleep(10) - } - throw new Error(`timed out waiting for ${count} pending permission request(s)`) -} +const immediate = (pending: Effect.Effect) => + Effect.gen(function* () { + const exit = yield* pending.pipe(Effect.timeout("500 millis"), Effect.exit) + if (Exit.isFailure(exit)) { + const items = yield* list() + if (items.length > 0) { + yield* reject() + } + return yield* exit + } + expect(yield* list()).toHaveLength(0) + }) + +const wait = (count: number) => + Effect.gen(function* () { + for (const _ of Array.from({ length: 500 })) { + const items = yield* list() + if (items.length === count) return items + yield* Effect.sleep("10 millis") + } + return yield* Effect.fail(new Error(`timed out waiting for ${count} pending permission request(s)`)) + }) afterEach(async () => { await disposeAllInstances() }) describe("external_directory allow config protection", () => { - test("allows file-tool external_directory requests for global config paths", async () => { - await using tmp = await tmpdir({ git: true }) - await WithInstance.provide({ - directory: tmp.path, - fn: async () => { - await immediate( - Permission.ask({ + it.live("allows file-tool external_directory requests for global config paths", () => + provideTmpdirInstance( + () => + immediate( + ask({ id: PermissionID.make("permission_file_external_read"), sessionID: SessionID.make("session_file_external_read"), permission: "external_directory", @@ -144,18 +165,16 @@ describe("external_directory allow config protection", () => { always: [configGlob], ruleset, }), - ) - }, - }) - }) + ), + { git: true }, + ), + ) - test("allows read-only bash external_directory requests for global config paths", async () => { - await using tmp = await tmpdir({ git: true }) - await WithInstance.provide({ - directory: tmp.path, - fn: async () => { - await immediate( - Permission.ask({ + it.live("allows read-only bash external_directory requests for global config paths", () => + provideTmpdirInstance( + () => + immediate( + ask({ id: PermissionID.make("permission_bash_external_read"), sessionID: SessionID.make("session_bash_external_read"), permission: "external_directory", @@ -164,10 +183,10 @@ describe("external_directory allow config protection", () => { always: [configGlob], ruleset, }), - ) - }, - }) - }) + ), + { git: true }, + ), + ) for (const pattern of variants(configGlob)) { test(`detects unknown bash external_directory requests for global config paths [${pattern}]`, () => { @@ -181,33 +200,37 @@ describe("external_directory allow config protection", () => { }) } - test("keeps unknown bash external_directory requests for global config paths protected", async () => { - await using tmp = await tmpdir({ git: true }) - await WithInstance.provide({ - directory: tmp.path, - fn: async () => { - const pending = Permission.ask({ - id: PermissionID.make("permission_bash_external_write"), - sessionID: SessionID.make("session_bash_external_write"), - permission: "external_directory", - patterns: [configGlob], - metadata: { command: `rm ${quote(configFile)}` }, - always: [configGlob], - ruleset, - }) + it.live("keeps unknown bash external_directory requests for global config paths protected", () => + provideTmpdirInstance( + () => + Effect.gen(function* () { + const pending = yield* ask({ + id: PermissionID.make("permission_bash_external_write"), + sessionID: SessionID.make("session_bash_external_write"), + permission: "external_directory", + patterns: [configGlob], + metadata: { command: `rm ${quote(configFile)}` }, + always: [configGlob], + ruleset, + }).pipe(Effect.forkScoped) - const requests = await wait(1) - expect(requests[0]).toMatchObject({ - id: PermissionID.make("permission_bash_external_write"), - permission: "external_directory", - metadata: { disableAlways: true }, - }) + const requests = yield* wait(1) + expect(requests[0]).toMatchObject({ + id: PermissionID.make("permission_bash_external_write"), + permission: "external_directory", + metadata: { disableAlways: true }, + }) - await Permission.reply({ requestID: PermissionID.make("permission_bash_external_write"), reply: "reject" }) - await expect(pending).rejects.toBeInstanceOf(Permission.RejectedError) - }, - }) - }) + yield* reply({ requestID: PermissionID.make("permission_bash_external_write"), reply: "reject" }) + const exit = yield* Fiber.await(pending) + expect(Exit.isFailure(exit)).toBe(true) + if (Exit.isFailure(exit)) { + expect(Cause.squash(exit.cause)).toBeInstanceOf(Permission.RejectedError) + } + }), + { git: true }, + ), + ) }) describe("bash external_directory access metadata", () => { diff --git a/packages/opencode/test/kilocode/permission/next.reply-http.test.ts b/packages/opencode/test/kilocode/permission/next.reply-http.test.ts index 3d0d48f0f02..278a216dbb1 100644 --- a/packages/opencode/test/kilocode/permission/next.reply-http.test.ts +++ b/packages/opencode/test/kilocode/permission/next.reply-http.test.ts @@ -1,9 +1,6 @@ import { afterEach, describe, expect, test } from "bun:test" import { Flag } from "@opencode-ai/core/flag/flag" -import { Permission } from "../../../src/permission" -import { PermissionID } from "../../../src/permission/schema" import { WithInstance } from "../../../src/project/with-instance" -import { Session } from "../../../src/session/session" import { tmpdir } from "../../fixture/fixture" const original = Flag.KILO_EXPERIMENTAL_HTTPAPI @@ -41,87 +38,6 @@ describe("POST /permission/:requestID/reply", () => { }) }) - test("returns 200 for an accepted reply", async () => { - await using tmp = await tmpdir({ git: true }) - - await WithInstance.provide({ - directory: tmp.path, - fn: async () => { - const server = await app() - const session = await Session.create({}) - - const asking = Permission.ask({ - id: PermissionID.make("permission_accepted_http"), - sessionID: session.id, - permission: "bash", - patterns: ["ls"], - metadata: {}, - always: [], - ruleset: [], - }) - - for (let i = 0; i < 100; i++) { - const list = await Permission.list() - if (list.length > 0) break - await new Promise((resolve) => setTimeout(resolve, 10)) - } - - const response = await server.request("/permission/permission_accepted_http/reply", { - method: "POST", - headers: { "Content-Type": "application/json", "x-kilo-directory": tmp.path }, - body: JSON.stringify({ reply: "once" }), - }) - expect(response.status).toBe(200) - expect(await response.json()).toBe(true) - - await asking - }, - }) - }) - - test("returns 404 when replying to an already-answered request (double-reply)", async () => { - await using tmp = await tmpdir({ git: true }) - - await WithInstance.provide({ - directory: tmp.path, - fn: async () => { - const server = await app() - const session = await Session.create({}) - - const asking = Permission.ask({ - id: PermissionID.make("permission_double_http"), - sessionID: session.id, - permission: "bash", - patterns: ["pwd"], - metadata: {}, - always: [], - ruleset: [], - }) - - for (let i = 0; i < 100; i++) { - const list = await Permission.list() - if (list.length > 0) break - await new Promise((resolve) => setTimeout(resolve, 10)) - } - - const first = await server.request("/permission/permission_double_http/reply", { - method: "POST", - headers: { "Content-Type": "application/json", "x-kilo-directory": tmp.path }, - body: JSON.stringify({ reply: "once" }), - }) - expect(first.status).toBe(200) - await asking - - const second = await server.request("/permission/permission_double_http/reply", { - method: "POST", - headers: { "Content-Type": "application/json", "x-kilo-directory": tmp.path }, - body: JSON.stringify({ reply: "once" }), - }) - expect(second.status).toBe(404) - }, - }) - }) - test("returns 404 for unknown replies when experimental HttpApi is enabled", async () => { await using tmp = await tmpdir({ git: true }) @@ -163,46 +79,4 @@ describe("POST /permission/:requestID/always-rules", () => { }, }) }) - - test("returns 200 for an accepted save", async () => { - await using tmp = await tmpdir({ git: true }) - - await WithInstance.provide({ - directory: tmp.path, - fn: async () => { - const server = await app() - const session = await Session.create({}) - - const asking = Permission.ask({ - id: PermissionID.make("permission_always_http"), - sessionID: session.id, - permission: "bash", - patterns: ["npm install"], - metadata: { rules: ["npm *", "npm install"] }, - always: ["npm install *"], - ruleset: [], - }) - - for (let i = 0; i < 100; i++) { - const list = await Permission.list() - if (list.length > 0) break - await new Promise((resolve) => setTimeout(resolve, 10)) - } - - const save = await server.request("/permission/permission_always_http/always-rules", { - method: "POST", - headers: { "Content-Type": "application/json", "x-kilo-directory": tmp.path }, - body: JSON.stringify({ approvedAlways: ["npm install"] }), - }) - expect(save.status).toBe(200) - expect(await save.json()).toBe(true) - - await Permission.reply({ - requestID: PermissionID.make("permission_always_http"), - reply: "once", - }) - await asking - }, - }) - }) }) diff --git a/packages/opencode/test/kilocode/server/permission-allow-everything.test.ts b/packages/opencode/test/kilocode/server/permission-allow-everything.test.ts index c9817ecd902..d56710099fe 100644 --- a/packages/opencode/test/kilocode/server/permission-allow-everything.test.ts +++ b/packages/opencode/test/kilocode/server/permission-allow-everything.test.ts @@ -1,126 +1,169 @@ // kilocode_change - new file import { describe, expect, test } from "bun:test" +import { Cause, Effect, Exit, Fiber, Layer } from "effect" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { Bus } from "../../../src/bus" +import * as Config from "../../../src/config/config" +import { AllowEverythingPermission } from "../../../src/kilocode/permission/allow-everything" import { Permission } from "../../../src/permission" import { PermissionID } from "../../../src/permission/schema" import { WithInstance } from "../../../src/project/with-instance" import { Server } from "../../../src/server/server" import { Session } from "../../../src/session/session" -import { tmpdir } from "../../fixture/fixture" +import { provideTmpdirInstance, tmpdir } from "../../fixture/fixture" +import { testEffect } from "../../lib/effect" -describe("permission.allowEverything endpoint", () => { - test("disables global allow-all and removes wildcard from config", async () => { +const bus = Bus.layer +const env = Layer.mergeAll( + Permission.layer.pipe(Layer.provide(bus), Layer.provide(Config.defaultLayer)), + Config.defaultLayer, + bus, + CrossSpawnSpawner.defaultLayer, +) +const it = testEffect(env) + +const ask = (input: Permission.AskInput) => + Effect.gen(function* () { + const permission = yield* Permission.Service + return yield* permission.ask(input) + }) + +const reply = (input: Permission.ReplyInput) => + Effect.gen(function* () { + const permission = yield* Permission.Service + return yield* permission.reply(input) + }) + +const wait = () => + Effect.gen(function* () { + const permission = yield* Permission.Service + for (let i = 0; i < 100; i++) { + if ((yield* permission.list()).length > 0) return + yield* Effect.sleep("10 millis") + } + return yield* Effect.fail(new Error("timed out waiting for pending permission request")) + }) + +describe("AllowEverythingPermission", () => { + test("handles disable requests through the HTTP endpoint", async () => { await using tmp = await tmpdir({ git: true }) - await WithInstance.provide({ directory: tmp.path, fn: async () => { - const app = Server.Default().app - - // Enable global auto-approve - const enable = await app.request("/permission/allow-everything", { + const enable = await Server.Default().app.request("/permission/allow-everything", { method: "POST", headers: { "Content-Type": "application/json", "x-kilo-directory": tmp.path }, body: JSON.stringify({ enable: true }), }) expect(enable.status).toBe(200) - // Disable global auto-approve - const disable = await app.request("/permission/allow-everything", { + const disable = await Server.Default().app.request("/permission/allow-everything", { method: "POST", headers: { "Content-Type": "application/json", "x-kilo-directory": tmp.path }, body: JSON.stringify({ enable: false }), }) expect(disable.status).toBe(200) expect(await disable.json()).toBe(true) - - // After disabling, permission requests should not be auto-approved - const session = await Session.create({}) - const pending = Permission.ask({ - id: PermissionID.make("permission_global_disable"), - sessionID: session.id, - permission: "bash", - patterns: ["ls"], - metadata: {}, - always: [], - ruleset: [], - }) - - await Permission.reply({ - requestID: PermissionID.make("permission_global_disable"), - reply: "reject", - }) - - await expect(pending).rejects.toBeInstanceOf(Permission.RejectedError) }, }) }) - test("disables session-scoped allow-all without touching global config", async () => { - await using tmp = await tmpdir({ git: true }) + it.live("disables global allow-all and restores permission prompts", () => + provideTmpdirInstance( + () => + Effect.gen(function* () { + expect(yield* AllowEverythingPermission.effect({ enable: true })).toBe(true) + expect(yield* AllowEverythingPermission.effect({ enable: false })).toBe(true) - await WithInstance.provide({ - directory: tmp.path, - fn: async () => { - const app = Server.Default().app - const session = await Session.create({ - permission: [{ permission: "*", pattern: "*", action: "allow" }], - }) + const session = yield* Effect.promise(() => Session.create({})) + const pending = yield* ask({ + id: PermissionID.make("permission_global_disable"), + sessionID: session.id, + permission: "bash", + patterns: ["ls"], + metadata: {}, + always: [], + ruleset: [], + }).pipe(Effect.forkScoped) - await Permission.allowEverything({ - enable: true, - sessionID: session.id, - }) + yield* wait() + yield* reply({ + requestID: PermissionID.make("permission_global_disable"), + reply: "reject", + }) - const response = await app.request("/permission/allow-everything", { - method: "POST", - headers: { - "Content-Type": "application/json", - "x-kilo-directory": tmp.path, - }, - body: JSON.stringify({ enable: false, sessionID: session.id }), - }) + const exit = yield* Fiber.await(pending) + expect(Exit.isFailure(exit)).toBe(true) + if (Exit.isFailure(exit)) { + expect(Cause.squash(exit.cause)).toBeInstanceOf(Permission.RejectedError) + } + }), + { git: true }, + ), + ) - expect(response.status).toBe(200) - expect(await response.json()).toBe(true) + it.live("disables session-scoped allow-all without affecting other sessions", () => + provideTmpdirInstance( + () => + Effect.gen(function* () { + const session = yield* Effect.promise(() => + Session.create({ + permission: [{ permission: "*", pattern: "*", action: "allow" }], + }), + ) - const next = await Session.get(session.id) - expect(next.permission ?? []).toEqual([]) + expect(yield* AllowEverythingPermission.effect({ enable: true, sessionID: session.id })).toBe(true) + expect(yield* AllowEverythingPermission.effect({ enable: false, sessionID: session.id })).toBe(true) - const pending = Permission.ask({ - id: PermissionID.make("permission_session_disable"), - sessionID: session.id, - permission: "bash", - patterns: ["ls"], - metadata: {}, - always: [], - ruleset: [], - }) + const next = yield* Effect.promise(() => Session.get(session.id)) + expect(next.permission ?? []).toEqual([]) - await Permission.reply({ - requestID: PermissionID.make("permission_session_disable"), - reply: "reject", - }) + const pending = yield* ask({ + id: PermissionID.make("permission_session_disable"), + sessionID: session.id, + permission: "bash", + patterns: ["ls"], + metadata: {}, + always: [], + ruleset: [], + }).pipe(Effect.forkScoped) - await expect(pending).rejects.toBeInstanceOf(Permission.RejectedError) + yield* wait() + yield* reply({ + requestID: PermissionID.make("permission_session_disable"), + reply: "reject", + }) - const other = await Session.create({}) - const blocked = Permission.ask({ - id: PermissionID.make("permission_other_session"), - sessionID: other.id, - permission: "bash", - patterns: ["pwd"], - metadata: {}, - always: [], - ruleset: [], - }) + const exit = yield* Fiber.await(pending) + expect(Exit.isFailure(exit)).toBe(true) + if (Exit.isFailure(exit)) { + expect(Cause.squash(exit.cause)).toBeInstanceOf(Permission.RejectedError) + } - await Permission.reply({ - requestID: PermissionID.make("permission_other_session"), - reply: "reject", - }) + const other = yield* Effect.promise(() => Session.create({})) + const blocked = yield* ask({ + id: PermissionID.make("permission_other_session"), + sessionID: other.id, + permission: "bash", + patterns: ["pwd"], + metadata: {}, + always: [], + ruleset: [], + }).pipe(Effect.forkScoped) - await expect(blocked).rejects.toBeInstanceOf(Permission.RejectedError) - }, - }) - }) + yield* wait() + yield* reply({ + requestID: PermissionID.make("permission_other_session"), + reply: "reject", + }) + + const blockedExit = yield* Fiber.await(blocked) + expect(Exit.isFailure(blockedExit)).toBe(true) + if (Exit.isFailure(blockedExit)) { + expect(Cause.squash(blockedExit.cause)).toBeInstanceOf(Permission.RejectedError) + } + }), + { git: true }, + ), + ) }) diff --git a/packages/opencode/test/kilocode/sessions/remote-sender.test.ts b/packages/opencode/test/kilocode/sessions/remote-sender.test.ts index b3d6d21359d..734d974dc5a 100644 --- a/packages/opencode/test/kilocode/sessions/remote-sender.test.ts +++ b/packages/opencode/test/kilocode/sessions/remote-sender.test.ts @@ -7,6 +7,7 @@ import type { RemoteProtocol } from "../../../src/kilo-sessions/remote-protocol" import { SessionPrompt } from "../../../src/session/prompt" import { Question } from "../../../src/question" import { Permission } from "../../../src/permission" +import { PermissionID } from "../../../src/permission/schema" import { Suggestion } from "../../../src/kilocode/suggestion" // kilocode_change function fakeConn() { @@ -47,6 +48,13 @@ const nolog = { warn: () => {}, } +function permissions(items: Permission.Request[] = []) { + return { + list: async () => items, + reply: async () => true, + } +} + // kilocode_change start afterEach(() => { mock.restore() @@ -465,6 +473,37 @@ describe("RemoteSender", () => { expect(sent[0]).toEqual({ type: "response", id: "req_q", result: {} }) }) + test("permission_respond sends response after work completes", async () => { + const { conn, sent } = fakeConn() + const calls: Permission.ReplyInput[] = [] + const sender = RemoteSender.create({ + conn, + directory: "/tmp/test", + log: nolog, + subscribe: fakeBus().subscribe, + provide: async (input: { directory: string; init?: Effect.Effect; fn: () => R }) => input.fn(), + permission: { + list: async () => [], + reply: async (input) => { + calls.push(input) + return true + }, + }, + }) + + sender.handle({ + type: "command", + id: "req_permission", + command: "permission_respond", + data: { requestID: PermissionID.make("permission_1"), reply: "once" }, + }) + + await new Promise((r) => setTimeout(r, 10)) + + expect(calls).toEqual([{ requestID: PermissionID.make("permission_1"), reply: "once" }]) + expect(sent).toContainEqual({ type: "response", id: "req_permission", result: {} }) + }) + test("question_reply error sends error response", async () => { const { conn, sent } = fakeConn() const sender = RemoteSender.create({ @@ -865,7 +904,6 @@ describe("RemoteSender", () => { { id: "question_1", sessionID: "ses_target", questions: [{ type: "text", text: "Continue?" }] } as any, { id: "question_2", sessionID: "ses_other", questions: [{ type: "text", text: "Unrelated?" }] } as any, ]) - spyOn(Permission, "list").mockResolvedValue([]) const sender = RemoteSender.create({ conn, @@ -873,6 +911,7 @@ describe("RemoteSender", () => { log: nolog, subscribe: bus.subscribe, provide: async (input: any) => input.fn(), + permission: permissions(), }) sender.handle({ type: "subscribe", sessionId: "ses_target" }) @@ -894,24 +933,6 @@ describe("RemoteSender", () => { spyOn(Suggestion, "list").mockResolvedValue([]) spyOn(Question, "list").mockResolvedValue([]) - spyOn(Permission, "list").mockResolvedValue([ - { - id: "permission_1", - sessionID: "ses_target", - permission: "file.write", - patterns: ["src/**"], - metadata: {}, - always: [], - } as any, - { - id: "permission_2", - sessionID: "ses_other", - permission: "file.read", - patterns: ["*"], - metadata: {}, - always: [], - } as any, - ]) const sender = RemoteSender.create({ conn, @@ -919,6 +940,24 @@ describe("RemoteSender", () => { log: nolog, subscribe: bus.subscribe, provide: async (input: any) => input.fn(), + permission: permissions([ + { + id: "permission_1", + sessionID: "ses_target", + permission: "file.write", + patterns: ["src/**"], + metadata: {}, + always: [], + } as any, + { + id: "permission_2", + sessionID: "ses_other", + permission: "file.read", + patterns: ["*"], + metadata: {}, + always: [], + } as any, + ]), }) sender.handle({ type: "subscribe", sessionId: "ses_target" }) @@ -949,16 +988,6 @@ describe("RemoteSender", () => { { id: "sug_1", sessionID: "ses_other", text: "Review?", actions: [] } as any, ]) spyOn(Question, "list").mockResolvedValue([{ id: "question_1", sessionID: "ses_other", questions: [] } as any]) - spyOn(Permission, "list").mockResolvedValue([ - { - id: "permission_1", - sessionID: "ses_other", - permission: "file.write", - patterns: [], - metadata: {}, - always: [], - } as any, - ]) const sender = RemoteSender.create({ conn, @@ -966,6 +995,16 @@ describe("RemoteSender", () => { log: nolog, subscribe: bus.subscribe, provide: async (input: any) => input.fn(), + permission: permissions([ + { + id: "permission_1", + sessionID: "ses_other", + permission: "file.write", + patterns: [], + metadata: {}, + always: [], + } as any, + ]), }) sender.handle({ type: "subscribe", sessionId: "ses_target" }) @@ -994,7 +1033,6 @@ describe("RemoteSender", () => { } as any, ]) spyOn(Question, "list").mockResolvedValue([]) - spyOn(Permission, "list").mockResolvedValue([]) const sender = RemoteSender.create({ conn, @@ -1002,6 +1040,7 @@ describe("RemoteSender", () => { log: nolog, subscribe: bus.subscribe, provide: async (input: any) => input.fn(), + permission: permissions(), }) sender.handle({ type: "subscribe", sessionId: "ses_target" }) diff --git a/script/check-opencode-promise-facades.ts b/script/check-opencode-promise-facades.ts index 76aaeeb0059..2e0b10778ad 100644 --- a/script/check-opencode-promise-facades.ts +++ b/script/check-opencode-promise-facades.ts @@ -3,7 +3,9 @@ /** * Prevents new service-local runtimes in shared Effect modules while the - * remaining Kilo Promise facades are migrated away. + * remaining Kilo Promise facades are migrated away. It also prevents tests + * from reaching through the global application runtime unless the integration + * boundary is explicitly classified. * * Existing sites are allowed only when classified below. Remove transitional * entries after their migration lands so later reintroductions fail CI. @@ -13,13 +15,14 @@ import path from "node:path" const ROOT = path.resolve(import.meta.dir, "..") const DIR = path.join(ROOT, "packages", "opencode", "src") +const TEST_DIR = path.join(ROOT, "packages", "opencode", "test") const PATTERN = /makeRuntime\s*\(\s*Service\s*,/g +const TEST_PATTERN = /\bAppRuntime\b/g const allow: Record = { "bus/index.ts": "core bus callback and synchronous runtime boundary", "cli/cmd/tui/config/tui.ts": "separately tracked TUI config facade", "installation/index.ts": "existing installation facade outside #10655", - "permission/index.ts": "transitional facade removed by #10620", "question/index.ts": "transitional facade deferred for upstream reconciliation in #10655", "session/compaction.ts": "existing compaction facade outside #10655", "session/prompt.ts": "transitional facade tracked by #10655", @@ -27,6 +30,30 @@ const allow: Record = { "sync/index.ts": "sync event runtime boundary", } +const testAllow: Record = { + "config/agent-color.test.ts": { count: 2, reason: "existing runtime integration test" }, + "config/tui.test.ts": { count: 3, reason: "existing runtime integration test" }, + "control-plane/workspace.test.ts": { count: 11, reason: "existing runtime integration test" }, + "effect/app-runtime-logger.test.ts": { count: 6, reason: "tests AppRuntime behavior" }, + "kilocode/config-resilience.test.ts": { count: 4, reason: "existing runtime integration test" }, + "kilocode/config-validation.test.ts": { count: 2, reason: "existing runtime integration test" }, + "kilocode/plan-followup.test.ts": { count: 7, reason: "existing runtime integration test" }, + "kilocode/session-list.test.ts": { count: 2, reason: "existing runtime integration test" }, + "kilocode/session/platform-attribution.test.ts": { count: 5, reason: "existing runtime integration test" }, + "kilocode/session/session.test.ts": { count: 4, reason: "existing runtime integration test" }, + "mcp/headers.test.ts": { count: 4, reason: "existing runtime integration test" }, + "mcp/oauth-browser.test.ts": { count: 4, reason: "existing runtime integration test" }, + "permission-task.test.ts": { count: 2, reason: "existing runtime integration test" }, + "project/vcs.test.ts": { count: 14, reason: "existing runtime integration test" }, + "provider/amazon-bedrock.test.ts": { count: 2, reason: "existing runtime integration test" }, + "provider/provider.test.ts": { count: 3, reason: "existing runtime integration test" }, + "pty/pty-output-isolation.test.ts": { count: 4, reason: "existing runtime integration test" }, + "pty/pty-session.test.ts": { count: 3, reason: "existing runtime integration test" }, + "pty/pty-shell.test.ts": { count: 4, reason: "existing runtime integration test" }, + "session/llm.test.ts": { count: 2, reason: "existing runtime integration test" }, + "tool/recall.test.ts": { count: 10, reason: "existing runtime integration test" }, +} + const owned = (file: string) => file.startsWith("kilocode/") || file.startsWith("kilo-sessions/") const hits: Array<{ file: string; line: number }> = [] const glob = new Bun.Glob("**/*.ts") @@ -47,7 +74,23 @@ const drift = Object.entries(allow).flatMap(([file, reason]) => { return [` packages/opencode/src/${file}: expected 1 classified site, found ${count} (${reason})`] }) -if (invalid.length > 0 || drift.length > 0) { +const testHits: Array<{ file: string; line: number }> = [] +for (const file of glob.scanSync({ cwd: TEST_DIR, onlyFiles: true })) { + const text = await Bun.file(path.join(TEST_DIR, file)).text() + for (const match of text.matchAll(TEST_PATTERN)) { + const line = text.slice(0, match.index ?? 0).split("\n").length + testHits.push({ file, line }) + } +} + +const testInvalid = testHits.filter((hit) => !testAllow[hit.file]) +const testDrift = Object.entries(testAllow).flatMap(([file, entry]) => { + const count = testHits.filter((hit) => hit.file === file).length + if (count === entry.count) return [] + return [` packages/opencode/test/${file}: expected ${entry.count} classified reference(s), found ${count} (${entry.reason})`] +}) + +if (invalid.length > 0 || drift.length > 0 || testInvalid.length > 0 || testDrift.length > 0) { if (invalid.length > 0) { console.error("Found unclassified service-local Effect runtimes in shared opencode modules:") for (const hit of invalid) console.error(` packages/opencode/src/${hit.file}:${hit.line}`) @@ -58,10 +101,22 @@ if (invalid.length > 0 || drift.length > 0) { for (const item of drift) console.error(item) console.error("") } - console.error("Do not add Promise facades to shared Effect services.") - console.error("Yield the service directly, or bridge at an existing AppRuntime or Kilo-owned boundary.") + if (testInvalid.length > 0) { + console.error("Found unclassified AppRuntime use in opencode tests:") + for (const hit of testInvalid) console.error(` packages/opencode/test/${hit.file}:${hit.line}`) + console.error("") + } + if (testDrift.length > 0) { + console.error("Classified test AppRuntime exceptions no longer match the current source:") + for (const item of testDrift) console.error(item) + console.error("") + } + console.error("Do not add Promise facades to shared Effect services or global AppRuntime dependencies to tests.") + console.error("Yield services directly in scoped layers, or classify intentional integration boundaries explicitly.") console.error("Remove migrated exceptions, or classify intentional runtime changes with an explicit reason.") process.exit(1) } -console.log(`check-opencode-promise-facades: ${hits.length} classified runtime site(s), no facade drift found.`) +console.log( + `check-opencode-promise-facades: ${hits.length} classified runtime site(s), ${testHits.length} classified test reference(s), no runtime drift found.`, +)