refactor(permission): effectify PermissionNext + fix InstanceState ALS bug (#17511)

packages/opencode/test/permission/next.test.ts

@@ -1,10 +1,32 @@
 import { test, expect } from "bun:test"
 import os from "os"
+import { Bus } from "../../src/bus"
+import { runtime } from "../../src/effect/runtime"
 import { PermissionNext } from "../../src/permission/next"
+import * as S from "../../src/permission/service"
 import { PermissionID } from "../../src/permission/schema"
 import { Instance } from "../../src/project/instance"
 import { tmpdir } from "../fixture/fixture"
-import { SessionID } from "../../src/session/schema"
+import { MessageID, SessionID } from "../../src/session/schema"
+
+async function rejectAll(message?: string) {
+  for (const req of await PermissionNext.list()) {
+    await PermissionNext.reply({
+      requestID: req.id,
+      reply: "reject",
+      message,
+    })
+  }
+}
+
+async function waitForPending(count: number) {
+  for (let i = 0; i < 20; i++) {
+    const list = await PermissionNext.list()
+    if (list.length === count) return list
+    await Bun.sleep(0)
+  }
+  return PermissionNext.list()
+}
 
 // fromConfig tests
 
@@ -511,6 +533,84 @@ test("ask - returns pending promise when action is ask", async () => {
       // Promise should be pending, not resolved
       expect(promise).toBeInstanceOf(Promise)
       // Don't await - just verify it returns a promise
+      await rejectAll()
+      await promise.catch(() => {})
+    },
+  })
+})
+
+test("ask - adds request to pending list", async () => {
+  await using tmp = await tmpdir({ git: true })
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const ask = PermissionNext.ask({
+        sessionID: SessionID.make("session_test"),
+        permission: "bash",
+        patterns: ["ls"],
+        metadata: { cmd: "ls" },
+        always: ["ls"],
+        tool: {
+          messageID: MessageID.make("msg_test"),
+          callID: "call_test",
+        },
+        ruleset: [],
+      })
+
+      const list = await PermissionNext.list()
+      expect(list).toHaveLength(1)
+      expect(list[0]).toMatchObject({
+        sessionID: SessionID.make("session_test"),
+        permission: "bash",
+        patterns: ["ls"],
+        metadata: { cmd: "ls" },
+        always: ["ls"],
+        tool: {
+          messageID: MessageID.make("msg_test"),
+          callID: "call_test",
+        },
+      })
+
+      await rejectAll()
+      await ask.catch(() => {})
+    },
+  })
+})
+
+test("ask - publishes asked event", async () => {
+  await using tmp = await tmpdir({ git: true })
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      let seen: PermissionNext.Request | undefined
+      const unsub = Bus.subscribe(PermissionNext.Event.Asked, (event) => {
+        seen = event.properties
+      })
+
+      const ask = PermissionNext.ask({
+        sessionID: SessionID.make("session_test"),
+        permission: "bash",
+        patterns: ["ls"],
+        metadata: { cmd: "ls" },
+        always: ["ls"],
+        tool: {
+          messageID: MessageID.make("msg_test"),
+          callID: "call_test",
+        },
+        ruleset: [],
+      })
+
+      expect(await PermissionNext.list()).toHaveLength(1)
+      expect(seen).toBeDefined()
+      expect(seen).toMatchObject({
+        sessionID: SessionID.make("session_test"),
+        permission: "bash",
+        patterns: ["ls"],
+      })
+
+      unsub()
+      await rejectAll()
+      await ask.catch(() => {})
     },
   })
 })
@@ -532,6 +632,8 @@ test("reply - once resolves the pending ask", async () => {
         ruleset: [],
       })
 
+      await waitForPending(1)
+
       await PermissionNext.reply({
         requestID: PermissionID.make("per_test1"),
         reply: "once",
@@ -557,6 +659,8 @@ test("reply - reject throws RejectedError", async () => {
         ruleset: [],
       })
 
+      await waitForPending(1)
+
       await PermissionNext.reply({
         requestID: PermissionID.make("per_test2"),
         reply: "reject",
@@ -567,6 +671,36 @@ test("reply - reject throws RejectedError", async () => {
   })
 })
 
+test("reply - reject with message throws CorrectedError", async () => {
+  await using tmp = await tmpdir({ git: true })
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const ask = PermissionNext.ask({
+        id: PermissionID.make("per_test2b"),
+        sessionID: SessionID.make("session_test"),
+        permission: "bash",
+        patterns: ["ls"],
+        metadata: {},
+        always: [],
+        ruleset: [],
+      })
+
+      await waitForPending(1)
+
+      await PermissionNext.reply({
+        requestID: PermissionID.make("per_test2b"),
+        reply: "reject",
+        message: "Use a safer command",
+      })
+
+      const err = await ask.catch((err) => err)
+      expect(err).toBeInstanceOf(PermissionNext.CorrectedError)
+      expect(err.message).toContain("Use a safer command")
+    },
+  })
+})
+
 test("reply - always persists approval and resolves", async () => {
   await using tmp = await tmpdir({ git: true })
   await Instance.provide({
@@ -582,6 +716,8 @@ test("reply - always persists approval and resolves", async () => {
         ruleset: [],
       })
 
+      await waitForPending(1)
+
       await PermissionNext.reply({
         requestID: PermissionID.make("per_test3"),
         reply: "always",
@@ -633,6 +769,8 @@ test("reply - reject cancels all pending for same session", async () => {
         ruleset: [],
       })
 
+      await waitForPending(2)
+
       // Catch rejections before they become unhandled
       const result1 = askPromise1.catch((e) => e)
       const result2 = askPromise2.catch((e) => e)
@@ -650,6 +788,144 @@ test("reply - reject cancels all pending for same session", async () => {
   })
 })
 
+test("reply - always resolves matching pending requests in same session", async () => {
+  await using tmp = await tmpdir({ git: true })
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const a = PermissionNext.ask({
+        id: PermissionID.make("per_test5a"),
+        sessionID: SessionID.make("session_same"),
+        permission: "bash",
+        patterns: ["ls"],
+        metadata: {},
+        always: ["ls"],
+        ruleset: [],
+      })
+
+      const b = PermissionNext.ask({
+        id: PermissionID.make("per_test5b"),
+        sessionID: SessionID.make("session_same"),
+        permission: "bash",
+        patterns: ["ls"],
+        metadata: {},
+        always: [],
+        ruleset: [],
+      })
+
+      await waitForPending(2)
+
+      await PermissionNext.reply({
+        requestID: PermissionID.make("per_test5a"),
+        reply: "always",
+      })
+
+      await expect(a).resolves.toBeUndefined()
+      await expect(b).resolves.toBeUndefined()
+      expect(await PermissionNext.list()).toHaveLength(0)
+    },
+  })
+})
+
+test("reply - always keeps other session pending", async () => {
+  await using tmp = await tmpdir({ git: true })
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const a = PermissionNext.ask({
+        id: PermissionID.make("per_test6a"),
+        sessionID: SessionID.make("session_a"),
+        permission: "bash",
+        patterns: ["ls"],
+        metadata: {},
+        always: ["ls"],
+        ruleset: [],
+      })
+
+      const b = PermissionNext.ask({
+        id: PermissionID.make("per_test6b"),
+        sessionID: SessionID.make("session_b"),
+        permission: "bash",
+        patterns: ["ls"],
+        metadata: {},
+        always: [],
+        ruleset: [],
+      })
+
+      await waitForPending(2)
+
+      await PermissionNext.reply({
+        requestID: PermissionID.make("per_test6a"),
+        reply: "always",
+      })
+
+      await expect(a).resolves.toBeUndefined()
+      expect((await PermissionNext.list()).map((x) => x.id)).toEqual([PermissionID.make("per_test6b")])
+
+      await rejectAll()
+      await b.catch(() => {})
+    },
+  })
+})
+
+test("reply - publishes replied event", async () => {
+  await using tmp = await tmpdir({ git: true })
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const ask = PermissionNext.ask({
+        id: PermissionID.make("per_test7"),
+        sessionID: SessionID.make("session_test"),
+        permission: "bash",
+        patterns: ["ls"],
+        metadata: {},
+        always: [],
+        ruleset: [],
+      })
+
+      await waitForPending(1)
+
+      let seen:
+        | {
+            sessionID: SessionID
+            requestID: PermissionID
+            reply: PermissionNext.Reply
+          }
+        | undefined
+      const unsub = Bus.subscribe(PermissionNext.Event.Replied, (event) => {
+        seen = event.properties
+      })
+
+      await PermissionNext.reply({
+        requestID: PermissionID.make("per_test7"),
+        reply: "once",
+      })
+
+      await expect(ask).resolves.toBeUndefined()
+      expect(seen).toEqual({
+        sessionID: SessionID.make("session_test"),
+        requestID: PermissionID.make("per_test7"),
+        reply: "once",
+      })
+      unsub()
+    },
+  })
+})
+
+test("reply - does nothing for unknown requestID", async () => {
+  await using tmp = await tmpdir({ git: true })
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      await PermissionNext.reply({
+        requestID: PermissionID.make("per_unknown"),
+        reply: "once",
+      })
+      expect(await PermissionNext.list()).toHaveLength(0)
+    },
+  })
+})
+
 test("ask - checks all patterns and stops on first deny", async () => {
   await using tmp = await tmpdir({ git: true })
   await Instance.provide({
@@ -689,3 +965,74 @@ test("ask - allows all patterns when all match allow rules", async () => {
     },
   })
 })
+
+test("ask - should deny even when an earlier pattern is ask", async () => {
+  await using tmp = await tmpdir({ git: true })
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const ask = PermissionNext.ask({
+        sessionID: SessionID.make("session_test"),
+        permission: "bash",
+        patterns: ["echo hello", "rm -rf /"],
+        metadata: {},
+        always: [],
+        ruleset: [
+          { permission: "bash", pattern: "echo *", action: "ask" },
+          { permission: "bash", pattern: "rm *", action: "deny" },
+        ],
+      })
+
+      const out = await Promise.race([
+        ask.then(
+          () => ({ ok: true as const, err: undefined }),
+          (err) => ({ ok: false as const, err }),
+        ),
+        Bun.sleep(100).then(() => "timeout" as const),
+      ])
+
+      if (out === "timeout") {
+        await rejectAll()
+        await ask.catch(() => {})
+        throw new Error("ask timed out instead of denying immediately")
+      }
+
+      expect(out.ok).toBe(false)
+      expect(out.err).toBeInstanceOf(PermissionNext.DeniedError)
+      expect(await PermissionNext.list()).toHaveLength(0)
+    },
+  })
+})
+
+test("ask - abort should clear pending request", async () => {
+  await using tmp = await tmpdir({ git: true })
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const ctl = new AbortController()
+      const ask = runtime.runPromise(
+        S.PermissionService.use((svc) =>
+          svc.ask({
+            sessionID: SessionID.make("session_test"),
+            permission: "bash",
+            patterns: ["ls"],
+            metadata: {},
+            always: [],
+            ruleset: [{ permission: "bash", pattern: "*", action: "ask" }],
+          }),
+        ),
+        { signal: ctl.signal },
+      )
+
+      await waitForPending(1)
+      ctl.abort()
+      await ask.catch(() => {})
+
+      try {
+        expect(await PermissionNext.list()).toHaveLength(0)
+      } finally {
+        await rejectAll()
+      }
+    },
+  })
+})

packages/opencode/test/tool/read.test.ts

@@ -183,7 +183,7 @@ describe("tool.read env file permissions", () => {
                   askedForEnv = true
                 }
                 if (rule.action === "deny") {
-                  throw new PermissionNext.DeniedError(agent.permission)
+                  throw new PermissionNext.DeniedError({ ruleset: agent.permission })
                 }
               }
             },

packages/opencode/test/util/instance-state.test.ts

@@ -1,5 +1,5 @@
 import { afterEach, expect, test } from "bun:test"
-import { Effect } from "effect"
+import { Duration, Effect, Layer, ManagedRuntime, ServiceMap } from "effect"
 
 import { Instance } from "../../src/project/instance"
 import { InstanceState } from "../../src/util/instance-state"
@@ -114,6 +114,129 @@ test("InstanceState is disposed on disposeAll", async () => {
   )
 })
 
+test("InstanceState.get reads correct directory per-evaluation (not captured once)", async () => {
+  await using a = await tmpdir()
+  await using b = await tmpdir()
+
+  // Regression: InstanceState.get must be lazy (Effect.suspend) so the
+  // directory is read per-evaluation, not captured once at the call site.
+  // Without this, a service built inside a ManagedRuntime Layer would
+  // freeze to whichever directory triggered the first layer build.
+
+  interface TestApi {
+    readonly getDir: () => Effect.Effect<string>
+  }
+
+  class TestService extends ServiceMap.Service<TestService, TestApi>()("@test/ALS-lazy") {
+    static readonly layer = Layer.effect(
+      TestService,
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make((dir) => Effect.sync(() => dir))
+        // `get` is created once during layer build — must be lazy
+        const get = InstanceState.get(state)
+
+        const getDir = Effect.fn("TestService.getDir")(function* () {
+          return yield* get
+        })
+
+        return TestService.of({ getDir })
+      }),
+    )
+  }
+
+  const rt = ManagedRuntime.make(TestService.layer)
+
+  try {
+    const resultA = await Instance.provide({
+      directory: a.path,
+      fn: () => rt.runPromise(TestService.use((s) => s.getDir())),
+    })
+    expect(resultA).toBe(a.path)
+
+    // Second call with different directory must NOT return A's directory
+    const resultB = await Instance.provide({
+      directory: b.path,
+      fn: () => rt.runPromise(TestService.use((s) => s.getDir())),
+    })
+    expect(resultB).toBe(b.path)
+  } finally {
+    await rt.dispose()
+  }
+})
+
+test("InstanceState.get isolates concurrent fibers across real delays, yields, and timer callbacks", async () => {
+  await using a = await tmpdir()
+  await using b = await tmpdir()
+  await using c = await tmpdir()
+
+  // Adversarial: concurrent fibers with real timer delays (macrotask
+  // boundaries via setTimeout/Bun.sleep), explicit scheduler yields,
+  // and many async steps. If ALS context leaks or gets lost at any
+  // point, a fiber will see the wrong directory.
+
+  interface TestApi {
+    readonly getDir: () => Effect.Effect<string>
+  }
+
+  class TestService extends ServiceMap.Service<TestService, TestApi>()("@test/ALS-adversarial") {
+    static readonly layer = Layer.effect(
+      TestService,
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make((dir) => Effect.sync(() => dir))
+
+        const getDir = Effect.fn("TestService.getDir")(function* () {
+          // Mix of async boundary types to maximise interleaving:
+          // 1. Real timer delay (macrotask — setTimeout under the hood)
+          yield* Effect.promise(() => Bun.sleep(1))
+          // 2. Effect.sleep (Effect's own timer, uses its internal scheduler)
+          yield* Effect.sleep(Duration.millis(1))
+          // 3. Explicit scheduler yields
+          for (let i = 0; i < 100; i++) {
+            yield* Effect.yieldNow
+          }
+          // 4. Microtask boundaries
+          for (let i = 0; i < 100; i++) {
+            yield* Effect.promise(() => Promise.resolve())
+          }
+          // 5. Another Effect.sleep
+          yield* Effect.sleep(Duration.millis(2))
+          // 6. Another real timer to force a second macrotask hop
+          yield* Effect.promise(() => Bun.sleep(1))
+          // NOW read the directory — ALS must still be correct
+          return yield* InstanceState.get(state)
+        })
+
+        return TestService.of({ getDir })
+      }),
+    )
+  }
+
+  const rt = ManagedRuntime.make(TestService.layer)
+
+  try {
+    const [resultA, resultB, resultC] = await Promise.all([
+      Instance.provide({
+        directory: a.path,
+        fn: () => rt.runPromise(TestService.use((s) => s.getDir())),
+      }),
+      Instance.provide({
+        directory: b.path,
+        fn: () => rt.runPromise(TestService.use((s) => s.getDir())),
+      }),
+      Instance.provide({
+        directory: c.path,
+        fn: () => rt.runPromise(TestService.use((s) => s.getDir())),
+      }),
+    ])
+
+    expect(resultA).toBe(a.path)
+    expect(resultB).toBe(b.path)
+    expect(resultC).toBe(c.path)
+  } finally {
+    await rt.dispose()
+  }
+})
+
 test("InstanceState dedupes concurrent lookups for the same directory", async () => {
   await using tmp = await tmpdir()
   let n = 0