Skip to content

Commit f3e6ab7

Browse files
mradwankhalilmradwankhalil
committed
refactor(plugin): simplify lifecycle yields and burst proofs
1 parent df40f88 commit f3e6ab7

2 files changed

Lines changed: 30 additions & 139 deletions

File tree

‎packages/core/src/plugin/scheduling.ts‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,14 @@
11
export * as PluginScheduling from "./scheduling.js"
22

3-
import { Effect, Semaphore } from "effect"
3+
import { Effect } from "effect"
44

55
// Setup runs in registration order, but an entire generation must not occupy one
66
// JavaScript turn. The live Clock's zero-duration sleep yields through Effect's
77
// native scheduler (not a resolved promise), so HTTP, streaming and other
8-
// locations progress below the operation-count auto-yield threshold. Serialize
9-
// admission across locations so a scheduler batch cannot resume every cold
10-
// location at once. Release BEFORE setup: asynchronous plugin work stays
11-
// concurrent and can request another location without a process-wide deadlock.
8+
// locations progress below the operation-count auto-yield threshold.
129
// Keep the Clock injectable: virtual-clock tests must not wait for wall time.
13-
const turn = Semaphore.makeUnsafe(1)
14-
export const checkpoint = turn.withPermit(Effect.sleep(0)).pipe(Effect.withSpan("Plugin.init.eventLoopCheckpoint"))
15-
// The same admission, registered as each plugin scope's first finalizer. Its own
10+
const turn = Effect.sleep(0)
11+
export const checkpoint = turn.pipe(Effect.withSpan("Plugin.init.eventLoopCheckpoint"))
12+
// The same yield, registered as each plugin scope's first finalizer. Its own
1613
// span name attributes close-path yields in traces and marks the release build.
17-
export const scopeCheckpoint = turn
18-
.withPermit(Effect.sleep(0))
19-
.pipe(Effect.withSpan("Plugin.scopeClose.eventLoopCheckpoint"))
14+
export const scopeCheckpoint = turn.pipe(Effect.withSpan("Plugin.scopeClose.eventLoopCheckpoint"))

‎packages/core/test/plugin-burst.test.ts‎

Lines changed: 24 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -1,163 +1,59 @@
11
import { expect } from "bun:test"
2-
import { Effect, Exit, Fiber } from "effect"
2+
import { Effect, Exit } from "effect"
33
import { Plugin } from "@opencode/core/plugin"
4-
import { PluginScheduling } from "@opencode/core/plugin/scheduling"
54
import { testEffect } from "./lib/effect"
65
import { PluginTestLayer } from "./plugin/fixture"
76

87
const it = testEffect(PluginTestLayer)
98

10-
it.live("allows plugin setup to activate a second location without holding global admission", () =>
11-
Effect.gen(function* () {
12-
const first = yield* Plugin.Service
13-
let nested = false
14-
yield* first.activate([{
15-
id: "first-location",
16-
revision: "1",
17-
effect: () => Effect.gen(function* () {
18-
const second = yield* Plugin.Service
19-
expect(second).not.toBe(first)
20-
yield* second.activate([{
21-
id: "second-location",
22-
revision: "1",
23-
effect: () => Effect.sync(() => { nested = true }),
24-
}])
25-
}).pipe(Effect.provide(PluginTestLayer), Effect.orDie),
26-
}]).pipe(Effect.timeout("5 seconds"))
27-
expect(nested).toBe(true)
28-
expect((yield* first.list())[0]?.state.status).toBe("active")
29-
}),
30-
)
31-
32-
it.live("continues location cleanup after a finalizer defect with exactly-once LIFO order", () =>
33-
Effect.gen(function* () {
34-
const plugins = yield* Plugin.Service
35-
const stopped: number[] = []
36-
yield* plugins.activate(Array.from({ length: 3 }, (_, index) => ({
37-
id: `defective-close-${index}`,
38-
revision: "1",
39-
effect: () => Effect.addFinalizer(() => Effect.gen(function* () {
40-
stopped.push(index)
41-
if (index === 1) yield* Effect.die("fixture cleanup defect")
42-
})),
43-
})))
44-
const exit = yield* plugins.close(Exit.void).pipe(Effect.exit)
45-
expect(Exit.isFailure(exit)).toBe(true)
46-
expect(stopped).toEqual([2, 1, 0])
47-
yield* plugins.awaitActivation
48-
}),
49-
)
50-
51-
it.live("services native timers during full location shutdown, preserving LIFO cleanup", () =>
52-
Effect.gen(function* () {
53-
const plugins = yield* Plugin.Service
54-
const stopped: number[] = []
55-
let ticks = 0
56-
const observed: number[] = []
57-
yield* plugins.activate(Array.from({ length: 12 }, (_, index) => ({
58-
id: `close-${index}`,
59-
revision: "1",
60-
effect: () => Effect.addFinalizer(() => Effect.sync(() => {
61-
observed.push(ticks)
62-
stopped.push(index)
63-
const end = performance.now() + 5
64-
while (performance.now() < end) {}
65-
})),
66-
})))
67-
const timer = setInterval(() => { ticks++ }, 0)
68-
yield* Effect.addFinalizer(() => Effect.sync(() => clearInterval(timer)))
69-
yield* plugins.close(Exit.void)
70-
expect(stopped).toEqual(Array.from({ length: 12 }, (_, index) => 11 - index))
71-
expect(observed.every((value, index) => index === 0 || value > observed[index - 1]!)).toBe(true)
72-
yield* plugins.close(Exit.void)
73-
expect(stopped).toHaveLength(12)
74-
}),
75-
)
76-
77-
it.live("services native timers between overlapping initialization checkpoints", () =>
78-
Effect.gen(function* () {
79-
let ticks = 0
80-
const observed: number[] = []
81-
const timer = setInterval(() => { ticks++ }, 0)
82-
yield* Effect.addFinalizer(() => Effect.sync(() => clearInterval(timer)))
83-
yield* Effect.forEach([0, 1, 2], () => Effect.gen(function* () {
84-
for (let index = 0; index < 4; index++) {
85-
yield* PluginScheduling.checkpoint
86-
observed.push(ticks)
87-
const end = performance.now() + 5
88-
while (performance.now() < end) {}
89-
}
90-
}), { concurrency: "unbounded", discard: true })
91-
expect(observed).toHaveLength(12)
92-
expect(observed.every((value, index) => index === 0 || value > observed[index - 1]!)).toBe(true)
93-
}),
94-
)
95-
96-
it.live("services native timers between ordered synchronous plugin activations", () =>
9+
it.live("activate burst yields to the event loop", () =>
9710
Effect.gen(function* () {
9811
const plugins = yield* Plugin.Service
99-
yield* plugins.awaitActivation
100-
let observed = false
101-
let observedBetween = false
10212
const started: number[] = []
103-
const timer = setTimeout(() => { observed = true }, 0)
13+
let observed = 0
14+
let timer: ReturnType<typeof setTimeout> | undefined
10415
yield* Effect.addFinalizer(() => Effect.sync(() => clearTimeout(timer)))
105-
yield* plugins.activate(Array.from({ length: 12 }, (_, index) => ({
16+
yield* plugins.activate(Array.from({ length: 3 }, (_, index) => ({
10617
id: `burst-${index}`,
10718
revision: "1",
10819
effect: () => Effect.sync(() => {
109-
// CPU work below the fiber operation-count auto-yield threshold.
20+
// Arm inside the first plugin so an initial yield cannot satisfy the test.
21+
if (index === 0) timer = setTimeout(() => { observed = started.length }, 0)
22+
// CPU work stays below the fiber operation-count auto-yield threshold.
11023
const end = performance.now() + 5
11124
while (performance.now() < end) {}
11225
started.push(index)
113-
if (index === 1) observedBetween = observed
11426
}),
11527
})))
116-
expect(started).toEqual(Array.from({ length: 12 }, (_, index) => index))
117-
expect(observedBetween).toBe(true)
28+
expect(started).toEqual([0, 1, 2])
29+
expect(observed).toBeGreaterThan(0)
30+
expect(observed).toBeLessThan(started.length)
11831
expect((yield* plugins.list()).every((plugin) => plugin.state.status === "active")).toBe(true)
11932
}),
12033
)
12134

122-
it.live("services native timers between reverse-order plugin finalizers", () =>
35+
it.live("close burst yields to the event loop", () =>
12336
Effect.gen(function* () {
12437
const plugins = yield* Plugin.Service
125-
let observed = false
126-
let observedBetween = false
12738
const stopped: number[] = []
128-
yield* plugins.activate(Array.from({ length: 12 }, (_, index) => ({
129-
id: `teardown-${index}`,
39+
let observed = 0
40+
let timer: ReturnType<typeof setTimeout> | undefined
41+
yield* Effect.addFinalizer(() => Effect.sync(() => clearTimeout(timer)))
42+
yield* plugins.activate(Array.from({ length: 3 }, (_, index) => ({
43+
id: `close-${index}`,
13044
revision: "1",
13145
effect: () => Effect.addFinalizer(() => Effect.sync(() => {
46+
if (index === 2) timer = setTimeout(() => { observed = stopped.length }, 0)
13247
const end = performance.now() + 5
13348
while (performance.now() < end) {}
13449
stopped.push(index)
135-
if (index === 10) observedBetween = observed
13650
})),
13751
})))
138-
const timer = setTimeout(() => { observed = true }, 0)
139-
yield* Effect.addFinalizer(() => Effect.sync(() => clearTimeout(timer)))
140-
yield* plugins.activate([])
141-
expect(stopped).toEqual(Array.from({ length: 12 }, (_, index) => 11 - index))
142-
expect(observedBetween).toBe(true)
143-
expect(yield* plugins.list()).toEqual([])
144-
}),
145-
)
146-
147-
it.live("cancels pending setup without leaking readiness holds", () =>
148-
Effect.gen(function* () {
149-
const plugins = yield* Plugin.Service
150-
let started = false
151-
const fiber = yield* plugins.activate([{
152-
id: "cancelled",
153-
revision: "1",
154-
effect: () => Effect.sync(() => { started = true }),
155-
}]).pipe(Effect.forkChild({ startImmediately: true }))
156-
yield* Fiber.interrupt(fiber)
157-
yield* plugins.awaitActivation
158-
expect(started).toBe(false)
159-
expect(yield* plugins.list()).toEqual([])
160-
yield* plugins.activate([{ id: "replacement", revision: "1", effect: () => Effect.void }])
161-
expect((yield* plugins.list()).map((plugin) => plugin.id)).toEqual([Plugin.ID.make("replacement")])
52+
yield* plugins.close(Exit.void)
53+
expect(stopped).toEqual([2, 1, 0])
54+
expect(observed).toBeGreaterThan(0)
55+
expect(observed).toBeLessThan(stopped.length)
56+
yield* plugins.close(Exit.void)
57+
expect(stopped).toEqual([2, 1, 0])
16258
}),
16359
)

0 commit comments

Comments
 (0)