Skip to content

Commit 605dcff

Browse files
shivamhwpclaude
andcommitted
fix(server): reply once to full-access OpenCode asks and name the auto-reply path
OpenCode stores "always" grants per directory, not per session. On a shared external server an "always" from a full-access thread would silently widen what a supervised thread on the same directory is allowed to do, so reply "once" instead. Every ask in full access is auto-answered anyway. Also extract the full-access branch into autoReplyFullAccess so the id bookkeeping has a name and a boundary, add an emitUnsafe sibling next to emit for the one publish that must not yield after its terminal check, collapse the two identical auto-approve tests into one parameterized case, and note the upstream ruleset quirk in the provider internals doc. Co-Authored-By: Claude Fable 5.1 <[email protected]>
1 parent 520e356 commit 605dcff

3 files changed

Lines changed: 134 additions & 171 deletions

File tree

‎apps/server/src/provider/Layers/OpenCodeAdapter.test.ts‎

Lines changed: 84 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -2628,148 +2628,92 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => {
26282628
}),
26292629
);
26302630

2631-
it.effect("auto-approves doom-loop asks on the parent session in full access", () =>
2632-
Effect.gen(function* () {
2633-
const adapter = yield* OpenCodeAdapter;
2634-
const threadId = asThreadId("thread-full-access-doom-loop");
2635-
runtimeMock.state.subscribedEvents = [
2636-
{
2637-
id: "evt-doom-loop",
2638-
type: "permission.asked",
2639-
properties: {
2640-
id: "per_doom_loop",
2641-
sessionID: "http://127.0.0.1:9999/session",
2642-
permission: "doom_loop",
2643-
patterns: ["bash"],
2644-
metadata: {},
2645-
always: [],
2646-
},
2647-
},
2648-
{
2649-
id: "evt-doom-loop-replied",
2650-
type: "permission.replied",
2651-
properties: {
2652-
sessionID: "http://127.0.0.1:9999/session",
2653-
requestID: "per_doom_loop",
2654-
reply: "always",
2655-
},
2656-
},
2657-
{
2658-
id: "evt-sentinel-question",
2659-
type: "question.asked",
2660-
properties: {
2661-
id: "que_sentinel",
2662-
sessionID: "http://127.0.0.1:9999/session",
2663-
questions: [],
2664-
},
2665-
},
2666-
];
2667-
2668-
const eventsFiber = yield* adapter.streamEvents.pipe(
2669-
Stream.filter((event) => event.threadId === threadId),
2670-
Stream.takeUntil((event) => event.type === "user-input.requested"),
2671-
Stream.runCollect,
2672-
Effect.forkChild,
2673-
);
2674-
yield* adapter.startSession({
2675-
provider: ProviderDriverKind.make("opencode"),
2676-
threadId,
2677-
runtimeMode: "full-access",
2678-
});
2679-
const events = Array.from(yield* Fiber.join(eventsFiber).pipe(Effect.timeout("1 second")));
2680-
2681-
NodeAssert.deepEqual(runtimeMock.state.permissionReplyCalls, [
2682-
{ requestID: "per_doom_loop", reply: "always" },
2683-
]);
2684-
NodeAssert.equal(
2685-
events.some((event) => event.type === "request.opened"),
2686-
false,
2687-
);
2688-
NodeAssert.equal(
2689-
events.some((event) => event.type === "request.resolved"),
2690-
false,
2691-
);
2692-
2693-
yield* adapter.stopSession(threadId);
2694-
}),
2695-
);
2696-
2697-
it.effect("auto-approves child-session asks in full access", () =>
2698-
Effect.gen(function* () {
2699-
const adapter = yield* OpenCodeAdapter;
2700-
const threadId = asThreadId("thread-full-access-child");
2701-
runtimeMock.state.subscribedEvents = [
2702-
{
2703-
id: "evt-child-created",
2704-
type: "session.created",
2705-
properties: {
2706-
sessionID: "ses_child_full",
2707-
info: {
2708-
id: "ses_child_full",
2709-
parentID: "http://127.0.0.1:9999/session",
2710-
title: "Child session",
2631+
it.effect.each([
2632+
{
2633+
name: "a doom-loop ask on the parent session",
2634+
requestId: "per_doom_loop",
2635+
sessionID: "http://127.0.0.1:9999/session",
2636+
permission: "doom_loop",
2637+
patterns: ["bash"],
2638+
always: [] as string[],
2639+
},
2640+
{
2641+
name: "a child-session ask",
2642+
requestId: "per_child_full",
2643+
sessionID: "ses_child_full",
2644+
permission: "read",
2645+
patterns: ["/repo/settings.env"],
2646+
always: ["/repo/settings.env"],
2647+
},
2648+
])(
2649+
"auto-approves $name in full access",
2650+
({ requestId, sessionID, permission, patterns, always }) =>
2651+
Effect.gen(function* () {
2652+
const adapter = yield* OpenCodeAdapter;
2653+
const threadId = asThreadId(`thread-full-access-${requestId}`);
2654+
runtimeMock.state.subscribedEvents = [
2655+
{
2656+
id: "evt-child-created",
2657+
type: "session.created",
2658+
properties: {
2659+
sessionID: "ses_child_full",
2660+
info: {
2661+
id: "ses_child_full",
2662+
parentID: "http://127.0.0.1:9999/session",
2663+
title: "Child session",
2664+
},
27112665
},
27122666
},
2713-
},
2714-
{
2715-
id: "evt-child-permission",
2716-
type: "permission.asked",
2717-
properties: {
2718-
id: "per_child_full",
2719-
sessionID: "ses_child_full",
2720-
permission: "read",
2721-
patterns: ["/repo/settings.env"],
2722-
metadata: {},
2723-
always: ["/repo/settings.env"],
2667+
{
2668+
id: "evt-permission",
2669+
type: "permission.asked",
2670+
properties: { id: requestId, sessionID, permission, patterns, metadata: {}, always },
27242671
},
2725-
},
2726-
{
2727-
id: "evt-child-permission-replied",
2728-
type: "permission.replied",
2729-
properties: {
2730-
sessionID: "ses_child_full",
2731-
requestID: "per_child_full",
2732-
reply: "always",
2672+
{
2673+
id: "evt-permission-replied",
2674+
type: "permission.replied",
2675+
properties: { sessionID, requestID: requestId, reply: "once" },
27332676
},
2734-
},
2735-
{
2736-
id: "evt-sentinel-question",
2737-
type: "question.asked",
2738-
properties: {
2739-
id: "que_sentinel",
2740-
sessionID: "http://127.0.0.1:9999/session",
2741-
questions: [],
2677+
// The suppressed ask emits nothing, so an empty question serves as a
2678+
// sentinel that closes the collected stream once the pump is past it.
2679+
{
2680+
id: "evt-sentinel-question",
2681+
type: "question.asked",
2682+
properties: {
2683+
id: "que_sentinel",
2684+
sessionID: "http://127.0.0.1:9999/session",
2685+
questions: [],
2686+
},
27422687
},
2743-
},
2744-
];
2688+
];
27452689

2746-
const eventsFiber = yield* adapter.streamEvents.pipe(
2747-
Stream.filter((event) => event.threadId === threadId),
2748-
Stream.takeUntil((event) => event.type === "user-input.requested"),
2749-
Stream.runCollect,
2750-
Effect.forkChild,
2751-
);
2752-
yield* adapter.startSession({
2753-
provider: ProviderDriverKind.make("opencode"),
2754-
threadId,
2755-
runtimeMode: "full-access",
2756-
});
2757-
const events = Array.from(yield* Fiber.join(eventsFiber).pipe(Effect.timeout("1 second")));
2690+
const eventsFiber = yield* adapter.streamEvents.pipe(
2691+
Stream.filter((event) => event.threadId === threadId),
2692+
Stream.takeUntil((event) => event.type === "user-input.requested"),
2693+
Stream.runCollect,
2694+
Effect.forkChild,
2695+
);
2696+
yield* adapter.startSession({
2697+
provider: ProviderDriverKind.make("opencode"),
2698+
threadId,
2699+
runtimeMode: "full-access",
2700+
});
2701+
const events = Array.from(yield* Fiber.join(eventsFiber).pipe(Effect.timeout("1 second")));
27582702

2759-
NodeAssert.deepEqual(runtimeMock.state.permissionReplyCalls, [
2760-
{ requestID: "per_child_full", reply: "always" },
2761-
]);
2762-
NodeAssert.equal(
2763-
events.some((event) => event.type === "request.opened"),
2764-
false,
2765-
);
2766-
NodeAssert.equal(
2767-
events.some((event) => event.type === "request.resolved"),
2768-
false,
2769-
);
2703+
NodeAssert.deepEqual(runtimeMock.state.permissionReplyCalls, [
2704+
{ requestID: requestId, reply: "once" },
2705+
]);
2706+
NodeAssert.equal(
2707+
events.some((event) => event.type === "request.opened"),
2708+
false,
2709+
);
2710+
NodeAssert.equal(
2711+
events.some((event) => event.type === "request.resolved"),
2712+
false,
2713+
);
27702714

2771-
yield* adapter.stopSession(threadId);
2772-
}),
2715+
yield* adapter.stopSession(threadId);
2716+
}),
27732717
);
27742718

27752719
it.effect("surfaces the approval when the full-access auto-reply fails", () =>
@@ -2812,7 +2756,7 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => {
28122756
// Exactly one auto-reply attempt: the fallback surfaces the dialog
28132757
// instead of retrying the reply.
28142758
NodeAssert.deepEqual(runtimeMock.state.permissionReplyCalls, [
2815-
{ requestID: "per_doom_loop_failed", reply: "always" },
2759+
{ requestID: "per_doom_loop_failed", reply: "once" },
28162760
]);
28172761

28182762
yield* adapter.stopSession(threadId);
@@ -2866,16 +2810,17 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => {
28662810
runtimeMock.state.transientErrorSessionIds.delete(childId);
28672811
yield* advanceTestClock(250);
28682812
NodeAssert.deepEqual(runtimeMock.state.permissionReplyCalls, [
2869-
{ requestID: request.id, reply: "always" },
2813+
{ requestID: request.id, reply: "once" },
28702814
]);
28712815

2816+
// Drain the microtask queue so the pump has consumed the terminal reply
2817+
// before the in-flight auto-reply is allowed to fail.
28722818
terminalEvent.resolve({
28732819
id: "evt-reply",
28742820
type: "permission.replied",
2875-
properties: { sessionID: childId, requestID: request.id, reply: "always" },
2821+
properties: { sessionID: childId, requestID: request.id, reply: "once" },
28762822
});
2877-
yield* Effect.yieldNow;
2878-
yield* Effect.yieldNow;
2823+
yield* Effect.promise(() => new Promise<void>((resolve) => setImmediate(resolve)));
28792824
releaseReply.resolve(undefined);
28802825
yield* advanceTestClock(250);
28812826

‎apps/server/src/provider/Layers/OpenCodeAdapter.ts‎

Lines changed: 48 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,12 @@ export function makeOpenCodeAdapter(
10011001

10021002
const emit = (event: ProviderRuntimeEvent) =>
10031003
Queue.offer(runtimeEvents, event).pipe(Effect.asVoid);
1004+
// Synchronous publish for callers that must not yield between a state
1005+
// check and the enqueue, e.g. reopening an approval only if its terminal
1006+
// event has not landed yet.
1007+
const emitUnsafe = (event: ProviderRuntimeEvent) => {
1008+
Queue.offerUnsafe(runtimeEvents, event);
1009+
};
10041010
const writeNativeEvent = (
10051011
threadId: ThreadId,
10061012
event: {
@@ -1603,6 +1609,42 @@ export function makeOpenCodeAdapter(
16031609
return false;
16041610
});
16051611

1612+
// Full access means the user already granted everything, but two upstream
1613+
// paths never consult the session ruleset we send: doom-loop detection
1614+
// (evaluated against the agent ruleset only) and subagent sessions (which
1615+
// keep only deny and external-directory rules). Answer those asks here.
1616+
//
1617+
// Reply "once", not "always": OpenCode stores "always" grants per
1618+
// directory, so on a shared external server an "always" from a full-access
1619+
// thread would silently widen what a supervised thread on the same
1620+
// directory is allowed to do.
1621+
const autoReplyFullAccess = Effect.fn("autoReplyFullAccess")(function* (
1622+
context: OpenCodeSessionContext,
1623+
request: PermissionRequest,
1624+
) {
1625+
// Mark before awaiting: retry and recovery fibers re-enter the ask path,
1626+
// and the matching `permission.replied` can arrive, while the SDK call
1627+
// is in flight. Marked ids skip the ask and swallow the terminal event.
1628+
context.resolvedRequestIds.add(request.id);
1629+
context.autoRepliedRequestIds.add(request.id);
1630+
const replied = yield* runOpenCodeSdk("permission.reply", () =>
1631+
context.client.permission.reply({ requestID: request.id, reply: "once" }),
1632+
).pipe(
1633+
Effect.as(true),
1634+
Effect.orElseSucceed(() => false),
1635+
);
1636+
if (replied) {
1637+
return "replied" as const;
1638+
}
1639+
// Fall back to the dialog. The id stays resolved so a recovered copy of
1640+
// this ask cannot reopen after the user answers; `pendingPermissions`
1641+
// gates re-asks while the dialog is open.
1642+
context.autoRepliedRequestIds.delete(request.id);
1643+
return context.emittedTerminalRequestIds.has(request.id)
1644+
? ("already-terminal" as const)
1645+
: ("fallback" as const);
1646+
});
1647+
16061648
const emitPendingOpenCodeRequest = Effect.fn("emitPendingOpenCodeRequest")(function* (
16071649
context: OpenCodeSessionContext,
16081650
event: OpenCodeAskedRequestEvent,
@@ -1617,50 +1659,25 @@ export function makeOpenCodeAdapter(
16171659
return;
16181660
}
16191661
if (context.session.runtimeMode === "full-access") {
1620-
// OpenCode still asks in full access when it detects a doom loop
1621-
// (evaluated against the agent ruleset only) and inside subagent
1622-
// sessions (which drop the parent's wildcard allow). Answer those
1623-
// here so the user never sees an approval they already granted.
1624-
//
1625-
// Record the ids before awaiting the reply: retry and recovery
1626-
// fibers can re-enter this function, and the matching
1627-
// `permission.replied` can land, while the SDK call is in flight.
1628-
// Marking first makes those paths skip the ask instead of racing
1629-
// to emit a duplicate `request.opened` or a stray
1630-
// `request.resolved`.
1631-
context.resolvedRequestIds.add(request.id);
1632-
context.autoRepliedRequestIds.add(request.id);
1633-
const replied = yield* runOpenCodeSdk("permission.reply", () =>
1634-
context.client.permission.reply({ requestID: request.id, reply: "always" }),
1635-
).pipe(
1636-
Effect.as(true),
1637-
Effect.orElseSucceed(() => false),
1638-
);
1639-
if (replied) {
1662+
const outcome = yield* autoReplyFullAccess(context, request);
1663+
if (outcome !== "fallback") {
16401664
return;
16411665
}
1642-
// The reply failed, so fall back to surfacing the approval. The id
1643-
// stays in `resolvedRequestIds`: `pendingPermissions` already gates
1644-
// re-asks while the dialog is open, and clearing it would let a
1645-
// recovered copy of this ask reopen after the user answers.
1646-
context.autoRepliedRequestIds.delete(request.id);
16471666
}
16481667
const base = yield* buildEventBase({
16491668
threadId: context.session.threadId,
16501669
turnId: context.activeTurnId,
16511670
requestId: request.id,
16521671
raw,
16531672
});
1654-
// Check and publish in one synchronous step. On a retry or recovery
1655-
// fiber the event pump can deliver the terminal `permission.replied`
1656-
// at any yield point (including a failed auto-reply above), and a
1657-
// `request.opened` published after that `request.resolved` would leave
1658-
// a dialog that can never close.
1673+
// No yield between this check and the publish: a terminal
1674+
// `permission.replied` delivered on the pump in between would leave a
1675+
// dialog that can never close.
16591676
if (context.emittedTerminalRequestIds.has(request.id)) {
16601677
return;
16611678
}
16621679
context.pendingPermissions.set(request.id, request);
1663-
Queue.offerUnsafe(runtimeEvents, {
1680+
emitUnsafe({
16641681
...base,
16651682
type: "request.opened",
16661683
payload: {

‎docs/internals/providers.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ attachment to the provider adapter. Each adapter decides what its provider inges
125125
Claude receives the attachment directory as an allowed additional directory. Codex keeps its
126126
configured sandbox policy, so access depends on that policy and the selected runtime mode. OpenCode
127127
allows all paths in full-access mode and requests approval for directories outside the workspace in
128-
restricted modes. Cursor and Grok use their own provider permission rules.
128+
restricted modes. Upstream OpenCode evaluates doom-loop and subagent asks against the agent ruleset
129+
only, ignoring the session ruleset T3 sends, so the adapter answers those asks itself in full access. Cursor and Grok use their own provider permission rules.
129130

130131
The server does not copy attachments into a project or bypass provider approval rules. If an agent
131132
cannot read an attachment, the user must approve the access or select a runtime mode that permits it.

0 commit comments

Comments
 (0)