feat(tui): redesign permission prompts
This commit is contained in:
parent
9db5073eab
commit
09c7cbe718
9 changed files with 389 additions and 87 deletions
|
|
@ -15,6 +15,7 @@ const PermissionEffect = Permission.Effect
|
|||
export { PermissionEffect as Effect }
|
||||
export { Rule, Ruleset } from "@opencode-ai/schema/permission"
|
||||
const missingAgentPermissions: Permission.Ruleset = [{ action: "*", resource: "*", effect: "deny" }]
|
||||
const externalFollowupLimit = 256
|
||||
|
||||
export const ID = Permission.ID
|
||||
export type ID = typeof ID.Type
|
||||
|
|
@ -108,9 +109,15 @@ export interface Interface {
|
|||
|
||||
export class Service extends Context.Service<Service, Interface>()("@opencode/v2/Permission") {}
|
||||
|
||||
interface ExternalFollowup {
|
||||
readonly action: "edit" | "read"
|
||||
readonly resources: ReadonlyArray<string>
|
||||
}
|
||||
|
||||
interface Pending {
|
||||
readonly request: Request
|
||||
readonly agent?: AgentV2.ID
|
||||
readonly externalFollowup?: ExternalFollowup
|
||||
readonly deferred: Deferred.Deferred<void, DeclinedError | CorrectedError>
|
||||
}
|
||||
|
||||
|
|
@ -123,6 +130,7 @@ const layer = Layer.effect(
|
|||
const sessions = yield* SessionStore.Service
|
||||
const saved = yield* PermissionSaved.Service
|
||||
const pending = new Map<ID, Pending>()
|
||||
const externalFollowups = new Map<string, ExternalFollowup>()
|
||||
|
||||
yield* Effect.addFinalizer(() =>
|
||||
Effect.forEach(pending.values(), (item) => Deferred.fail(item.deferred, new DeclinedError()), {
|
||||
|
|
@ -131,6 +139,7 @@ const layer = Layer.effect(
|
|||
Effect.ensuring(
|
||||
Effect.sync(() => {
|
||||
pending.clear()
|
||||
externalFollowups.clear()
|
||||
}),
|
||||
),
|
||||
),
|
||||
|
|
@ -178,11 +187,52 @@ const layer = Layer.effect(
|
|||
}
|
||||
}
|
||||
|
||||
const create = (request: Request, agent?: AgentV2.ID) =>
|
||||
function sourceKey(input: Pick<AssertInput, "sessionID" | "source">) {
|
||||
if (input.source?.type !== "tool") return
|
||||
return JSON.stringify([input.sessionID, input.source.messageID, input.source.callID])
|
||||
}
|
||||
|
||||
function externalFollowup(input: AssertInput) {
|
||||
if (input.action !== "external_directory") return
|
||||
const value = input.metadata?.followup
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) return
|
||||
const action = Reflect.get(value, "action")
|
||||
const resources = Reflect.get(value, "resources")
|
||||
if (action !== "edit" && action !== "read") return
|
||||
if (!Array.isArray(resources) || !resources.every((resource) => typeof resource === "string")) return
|
||||
return { action, resources: [...resources] } satisfies ExternalFollowup
|
||||
}
|
||||
|
||||
function rememberExternalFollowup(input: Pending) {
|
||||
if (!input.externalFollowup) return
|
||||
const key = sourceKey(input.request)
|
||||
if (!key) return
|
||||
externalFollowups.set(key, input.externalFollowup)
|
||||
if (externalFollowups.size > externalFollowupLimit) {
|
||||
const oldest = externalFollowups.keys().next()
|
||||
if (!oldest.done) externalFollowups.delete(oldest.value)
|
||||
}
|
||||
return key
|
||||
}
|
||||
|
||||
function consumeExternalFollowup(input: AssertInput) {
|
||||
const key = sourceKey(input)
|
||||
if (!key) return false
|
||||
const followup = externalFollowups.get(key)
|
||||
if (!followup) return false
|
||||
externalFollowups.delete(key)
|
||||
return (
|
||||
followup.action === input.action &&
|
||||
followup.resources.length === input.resources.length &&
|
||||
followup.resources.every((resource, index) => resource === input.resources[index])
|
||||
)
|
||||
}
|
||||
|
||||
const create = (request: Request, agent?: AgentV2.ID, followup?: ExternalFollowup) =>
|
||||
Effect.uninterruptible(
|
||||
Effect.gen(function* () {
|
||||
const deferred = yield* Deferred.make<void, DeclinedError | CorrectedError>()
|
||||
const item = { request, agent, deferred }
|
||||
const item = { request, agent, externalFollowup: followup, deferred }
|
||||
if (pending.has(request.id))
|
||||
return yield* Effect.die(new Error(`Duplicate pending permission ID: ${request.id}`))
|
||||
pending.set(request.id, item)
|
||||
|
|
@ -204,6 +254,7 @@ const layer = Layer.effect(
|
|||
Effect.uninterruptibleMask((restore) =>
|
||||
Effect.gen(function* () {
|
||||
const result = yield* evaluateInput(input)
|
||||
const approved = consumeExternalFollowup(input)
|
||||
if (result.effect === "deny") {
|
||||
return yield* new BlockedError({
|
||||
rules: relevant(input, result.rules),
|
||||
|
|
@ -211,8 +262,9 @@ const layer = Layer.effect(
|
|||
resources: input.resources,
|
||||
})
|
||||
}
|
||||
if (approved) return
|
||||
if (result.effect === "allow") return
|
||||
const item = yield* create(request(input), input.agent)
|
||||
const item = yield* create(request(input), input.agent, externalFollowup(input))
|
||||
return yield* restore(Deferred.await(item.deferred)).pipe(
|
||||
Effect.catchTag("PermissionV2.DeclinedError", (error) => Effect.die(error)),
|
||||
Effect.ensuring(
|
||||
|
|
@ -262,7 +314,9 @@ const layer = Layer.effect(
|
|||
resources: existing.request.save,
|
||||
})
|
||||
}
|
||||
yield* Deferred.succeed(existing.deferred, undefined)
|
||||
const followup = rememberExternalFollowup(existing)
|
||||
const resumed = yield* Deferred.succeed(existing.deferred, undefined)
|
||||
if (!resumed && followup) externalFollowups.delete(followup)
|
||||
pending.delete(input.requestID)
|
||||
if (input.reply !== "always" || !existing.request.save?.length) return
|
||||
|
||||
|
|
|
|||
|
|
@ -142,6 +142,7 @@ export const Plugin = {
|
|||
yield* unableToEdit(
|
||||
permission.assert({
|
||||
...LocationMutation.externalDirectoryPermission(external),
|
||||
metadata: { followup: { action: "edit", resources: [target.resource] } },
|
||||
sessionID: context.sessionID,
|
||||
agent: context.agent,
|
||||
source: permissionSource,
|
||||
|
|
|
|||
|
|
@ -105,9 +105,11 @@ export const Plugin = {
|
|||
const external = target.externalDirectory
|
||||
if (external) externalDirectories.set(external.resource, external)
|
||||
}
|
||||
const resources = [...new Set(targets.map(({ target }) => target.resource))]
|
||||
for (const external of externalDirectories.values()) {
|
||||
yield* permission.assert({
|
||||
...LocationMutation.externalDirectoryPermission(external),
|
||||
metadata: { followup: { action: "edit", resources } },
|
||||
sessionID: context.sessionID,
|
||||
agent: context.agent,
|
||||
source,
|
||||
|
|
@ -115,7 +117,7 @@ export const Plugin = {
|
|||
}
|
||||
yield* permission.assert({
|
||||
action: "edit",
|
||||
resources: [...new Set(targets.map(({ target }) => target.resource))],
|
||||
resources,
|
||||
save: ["*"],
|
||||
sessionID: context.sessionID,
|
||||
agent: context.agent,
|
||||
|
|
|
|||
|
|
@ -70,6 +70,7 @@ export const Plugin = {
|
|||
if (external)
|
||||
yield* permission.assert({
|
||||
...LocationMutation.externalDirectoryPermission(external),
|
||||
metadata: { followup: { action: "read", resources: [target.resource] } },
|
||||
sessionID: context.sessionID,
|
||||
agent: context.agent,
|
||||
source,
|
||||
|
|
|
|||
|
|
@ -72,6 +72,7 @@ export const Plugin = {
|
|||
if (external)
|
||||
yield* permission.assert({
|
||||
...LocationMutation.externalDirectoryPermission(external),
|
||||
metadata: { followup: { action: "edit", resources: [target.resource] } },
|
||||
sessionID: context.sessionID,
|
||||
agent: context.agent,
|
||||
source,
|
||||
|
|
|
|||
|
|
@ -86,7 +86,7 @@ function assertion(input: Partial<PermissionV2.AssertInput> = {}) {
|
|||
} satisfies PermissionV2.AssertInput
|
||||
}
|
||||
|
||||
function waitForRequest() {
|
||||
function waitForRequest(input = assertion()) {
|
||||
return Effect.gen(function* () {
|
||||
const service = yield* PermissionV2.Service
|
||||
const events = yield* EventV2.Service
|
||||
|
|
@ -97,7 +97,7 @@ function waitForRequest() {
|
|||
: Effect.void,
|
||||
)
|
||||
yield* Effect.addFinalizer(() => unsubscribe)
|
||||
const fiber = yield* service.assert(assertion()).pipe(Effect.forkScoped)
|
||||
const fiber = yield* service.assert(input).pipe(Effect.forkScoped)
|
||||
const request = yield* Deferred.await(asked)
|
||||
return { service, fiber, request }
|
||||
})
|
||||
|
|
@ -266,6 +266,88 @@ describe("PermissionV2", () => {
|
|||
}),
|
||||
)
|
||||
|
||||
it.effect("uses external approval only for the exact blocking follow-up", () =>
|
||||
Effect.gen(function* () {
|
||||
yield* setup()
|
||||
const source = { type: "tool" as const, messageID: "msg_1", callID: "call_1" }
|
||||
const followup = { action: "edit" as const, resources: ["/tmp/work/file.ts"] }
|
||||
const external = yield* waitForRequest(
|
||||
assertion({
|
||||
id: PermissionV2.ID.create("per_external"),
|
||||
action: "external_directory",
|
||||
resources: ["/tmp/work/*"],
|
||||
metadata: { followup },
|
||||
source,
|
||||
}),
|
||||
)
|
||||
yield* external.service.reply({ requestID: external.request.id, reply: "once" })
|
||||
yield* Fiber.join(external.fiber)
|
||||
|
||||
yield* external.service.assert(assertion({ ...followup, source }))
|
||||
expect(yield* external.service.list()).toEqual([])
|
||||
|
||||
const repeated = yield* waitForRequest(
|
||||
assertion({ id: PermissionV2.ID.create("per_repeated"), ...followup, source }),
|
||||
)
|
||||
yield* repeated.service.reply({ requestID: repeated.request.id, reply: "reject" })
|
||||
yield* Fiber.await(repeated.fiber)
|
||||
|
||||
const next = yield* waitForRequest(
|
||||
assertion({
|
||||
id: PermissionV2.ID.create("per_external_2"),
|
||||
action: "external_directory",
|
||||
resources: ["/tmp/work/*"],
|
||||
metadata: { followup },
|
||||
source,
|
||||
}),
|
||||
)
|
||||
yield* next.service.reply({ requestID: next.request.id, reply: "once" })
|
||||
yield* Fiber.join(next.fiber)
|
||||
|
||||
const mismatch = yield* waitForRequest(
|
||||
assertion({
|
||||
id: PermissionV2.ID.create("per_mismatch"),
|
||||
action: "edit",
|
||||
resources: ["/tmp/work/other.ts"],
|
||||
source,
|
||||
}),
|
||||
)
|
||||
yield* mismatch.service.reply({ requestID: mismatch.request.id, reply: "reject" })
|
||||
yield* Fiber.await(mismatch.fiber)
|
||||
}),
|
||||
)
|
||||
|
||||
it.effect("does not retain an external follow-up after a configured denial", () =>
|
||||
Effect.gen(function* () {
|
||||
yield* setup()
|
||||
const source = { type: "tool" as const, messageID: "msg_2", callID: "call_2" }
|
||||
const followup = { action: "edit" as const, resources: ["/tmp/work/file.ts"] }
|
||||
const external = yield* waitForRequest(
|
||||
assertion({
|
||||
id: PermissionV2.ID.create("per_external_deny"),
|
||||
action: "external_directory",
|
||||
resources: ["/tmp/work/*"],
|
||||
metadata: { followup },
|
||||
source,
|
||||
}),
|
||||
)
|
||||
yield* external.service.reply({ requestID: external.request.id, reply: "once" })
|
||||
yield* Fiber.join(external.fiber)
|
||||
|
||||
yield* setRules([{ action: "edit", resource: "*", effect: "deny" }])
|
||||
expect(yield* external.service.assert(assertion({ ...followup, source })).pipe(Effect.flip)).toBeInstanceOf(
|
||||
PermissionV2.BlockedError,
|
||||
)
|
||||
|
||||
yield* setRules([])
|
||||
const afterDeny = yield* waitForRequest(
|
||||
assertion({ id: PermissionV2.ID.create("per_after_deny"), ...followup, source }),
|
||||
)
|
||||
yield* afterDeny.service.reply({ requestID: afterDeny.request.id, reply: "reject" })
|
||||
yield* Fiber.await(afterDeny.fiber)
|
||||
}),
|
||||
)
|
||||
|
||||
it.effect("defects when an asked permission is declined", () =>
|
||||
Effect.gen(function* () {
|
||||
yield* setup()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue