Skip to content

Commit b617ad5

Browse files
authored
Merge branch 'dev' into fix/copilot-premium-request-detection
2 parents 59c7f00 + d1f05b0 commit b617ad5

21 files changed

Lines changed: 274 additions & 242 deletions

packages/opencode/specs/effect-migration.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -229,24 +229,24 @@ Still open:
229229

230230
## Tool interface → Effect
231231

232-
Once individual tools are effectified, change `Tool.Info` (`tool/tool.ts`) so `init` and `execute` return `Effect` instead of `Promise`. This lets tool implementations compose natively with the Effect pipeline rather than being wrapped in `Effect.promise()` at the call site. Requires:
232+
`Tool.Def.execute` and `Tool.Info.init` already return `Effect` on this branch. Tool definitions should now stay Effect-native all the way through initialization instead of using Promise-returning init callbacks. Tools can still use lazy init callbacks when they need instance-bound state at init time, but those callbacks should return `Effect`, not `Promise`. Remaining work is:
233233

234-
1. Migrate each tool to return Effects
235-
2. Update `Tool.define()` factory to work with Effects
236-
3. Update `SessionPrompt` to `yield*` tool results instead of `await`ing
234+
1. Migrate each tool body to return Effects
235+
2. Keep `Tool.define()` inputs Effect-native
236+
3. Update remaining callers to `yield*` tool initialization instead of `await`ing
237237

238238
### Tool migration details
239239

240-
Until the tool interface itself returns `Effect`, use this transitional pattern for migrated tools:
240+
With `Tool.Info.init()` now effectful, use this transitional pattern for migrated tools that still need Promise-based boundaries internally:
241241

242242
- `Tool.defineEffect(...)` should `yield*` the services the tool depends on and close over them in the returned tool definition.
243-
- Keep the bridge at the Promise boundary only. Prefer a single `Effect.runPromise(...)` in the temporary `async execute(...)` implementation, and move the inner logic into `Effect.fn(...)` helpers instead of scattering `runPromise` islands through the tool body.
243+
- Keep the bridge at the Promise boundary only inside the tool body when required by external APIs. Do not return Promise-based init callbacks from `Tool.define()`.
244244
- If a tool starts requiring new services, wire them into `ToolRegistry.defaultLayer` so production callers resolve the same dependencies as tests.
245245

246246
Tool tests should use the existing Effect helpers in `packages/opencode/test/lib/effect.ts`:
247247

248248
- Use `testEffect(...)` / `it.live(...)` instead of creating fake local wrappers around effectful tools.
249-
- Yield the real tool export, then initialize it: `const info = yield* ReadTool`, `const tool = yield* Effect.promise(() => info.init())`.
249+
- Yield the real tool export, then initialize it: `const info = yield* ReadTool`, `const tool = yield* info.init()`.
250250
- Run tests inside a real instance with `provideTmpdirInstance(...)` or `provideInstance(tmpdirScoped(...))` so instance-scoped services resolve exactly as they do in production.
251251

252252
This keeps migrated tool tests aligned with the production service graph today, and makes the eventual `Tool.Info``Effect` cleanup mostly mechanical later.

packages/opencode/src/session/prompt.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ export namespace SessionPrompt {
102102
const instruction = yield* Instruction.Service
103103
const state = yield* SessionRunState.Service
104104
const revert = yield* SessionRevert.Service
105+
const sys = yield* SystemPrompt.Service
106+
const llm = yield* LLM.Service
105107

106108
const run = {
107109
promise: <A, E>(effect: Effect.Effect<A, E>) =>
@@ -180,21 +182,24 @@ export namespace SessionPrompt {
180182
const msgs = onlySubtasks
181183
? [{ role: "user" as const, content: subtasks.map((p) => p.prompt).join("\n") }]
182184
: yield* MessageV2.toModelMessagesEffect(context, mdl)
183-
const text = yield* Effect.promise(async (signal) => {
184-
const result = await LLM.stream({
185+
const text = yield* llm
186+
.stream({
185187
agent: ag,
186188
user: firstInfo,
187189
system: [],
188190
small: true,
189191
tools: {},
190192
model: mdl,
191-
abort: signal,
192193
sessionID: input.session.id,
193194
retries: 2,
194195
messages: [{ role: "user", content: "Generate a title for this conversation:\n" }, ...msgs],
195196
})
196-
return result.text
197-
})
197+
.pipe(
198+
Stream.filter((e): e is Extract<LLM.Event, { type: "text-delta" }> => e.type === "text-delta"),
199+
Stream.map((e) => e.text),
200+
Stream.mkString,
201+
Effect.orDie,
202+
)
198203
const cleaned = text
199204
.replace(/<think>[\s\S]*?<\/think>\s*/g, "")
200205
.split("\n")
@@ -1462,8 +1467,8 @@ NOTE: At any point in time through this workflow you should feel free to ask the
14621467
yield* plugin.trigger("experimental.chat.messages.transform", {}, { messages: msgs })
14631468

14641469
const [skills, env, instructions, modelMsgs] = yield* Effect.all([
1465-
Effect.promise(() => SystemPrompt.skills(agent)),
1466-
Effect.promise(() => SystemPrompt.environment(model)),
1470+
sys.skills(agent),
1471+
Effect.sync(() => sys.environment(model)),
14671472
instruction.system().pipe(Effect.orDie),
14681473
MessageV2.toModelMessagesEffect(msgs, model),
14691474
])
@@ -1687,9 +1692,15 @@ NOTE: At any point in time through this workflow you should feel free to ask the
16871692
Layer.provide(Plugin.defaultLayer),
16881693
Layer.provide(Session.defaultLayer),
16891694
Layer.provide(SessionRevert.defaultLayer),
1690-
Layer.provide(Agent.defaultLayer),
1691-
Layer.provide(Bus.layer),
1692-
Layer.provide(CrossSpawnSpawner.defaultLayer),
1695+
Layer.provide(
1696+
Layer.mergeAll(
1697+
Agent.defaultLayer,
1698+
SystemPrompt.defaultLayer,
1699+
LLM.defaultLayer,
1700+
Bus.layer,
1701+
CrossSpawnSpawner.defaultLayer,
1702+
),
1703+
),
16931704
),
16941705
)
16951706
const { runPromise } = makeRuntime(Service, defaultLayer)
Lines changed: 45 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Ripgrep } from "../file/ripgrep"
1+
import { Context, Effect, Layer } from "effect"
22

33
import { Instance } from "../project/instance"
44

@@ -33,44 +33,52 @@ export namespace SystemPrompt {
3333
return [PROMPT_DEFAULT]
3434
}
3535

36-
export async function environment(model: Provider.Model) {
37-
const project = Instance.project
38-
return [
39-
[
40-
`You are powered by the model named ${model.api.id}. The exact model ID is ${model.providerID}/${model.api.id}`,
41-
`Here is some useful information about the environment you are running in:`,
42-
`<env>`,
43-
` Working directory: ${Instance.directory}`,
44-
` Workspace root folder: ${Instance.worktree}`,
45-
` Is directory a git repo: ${project.vcs === "git" ? "yes" : "no"}`,
46-
` Platform: ${process.platform}`,
47-
` Today's date: ${new Date().toDateString()}`,
48-
`</env>`,
49-
`<directories>`,
50-
` ${
51-
project.vcs === "git" && false
52-
? await Ripgrep.tree({
53-
cwd: Instance.directory,
54-
limit: 50,
55-
})
56-
: ""
57-
}`,
58-
`</directories>`,
59-
].join("\n"),
60-
]
36+
export interface Interface {
37+
readonly environment: (model: Provider.Model) => string[]
38+
readonly skills: (agent: Agent.Info) => Effect.Effect<string | undefined>
6139
}
6240

63-
export async function skills(agent: Agent.Info) {
64-
if (Permission.disabled(["skill"], agent.permission).has("skill")) return
41+
export class Service extends Context.Service<Service, Interface>()("@opencode/SystemPrompt") {}
6542

66-
const list = await Skill.available(agent)
43+
export const layer = Layer.effect(
44+
Service,
45+
Effect.gen(function* () {
46+
const skill = yield* Skill.Service
6747

68-
return [
69-
"Skills provide specialized instructions and workflows for specific tasks.",
70-
"Use the skill tool to load a skill when a task matches its description.",
71-
// the agents seem to ingest the information about skills a bit better if we present a more verbose
72-
// version of them here and a less verbose version in tool description, rather than vice versa.
73-
Skill.fmt(list, { verbose: true }),
74-
].join("\n")
75-
}
48+
return Service.of({
49+
environment(model) {
50+
const project = Instance.project
51+
return [
52+
[
53+
`You are powered by the model named ${model.api.id}. The exact model ID is ${model.providerID}/${model.api.id}`,
54+
`Here is some useful information about the environment you are running in:`,
55+
`<env>`,
56+
` Working directory: ${Instance.directory}`,
57+
` Workspace root folder: ${Instance.worktree}`,
58+
` Is directory a git repo: ${project.vcs === "git" ? "yes" : "no"}`,
59+
` Platform: ${process.platform}`,
60+
` Today's date: ${new Date().toDateString()}`,
61+
`</env>`,
62+
].join("\n"),
63+
]
64+
},
65+
66+
skills: Effect.fn("SystemPrompt.skills")(function* (agent: Agent.Info) {
67+
if (Permission.disabled(["skill"], agent.permission).has("skill")) return
68+
69+
const list = yield* skill.available(agent)
70+
71+
return [
72+
"Skills provide specialized instructions and workflows for specific tasks.",
73+
"Use the skill tool to load a skill when a task matches its description.",
74+
// the agents seem to ingest the information about skills a bit better if we present a more verbose
75+
// version of them here and a less verbose version in tool description, rather than vice versa.
76+
Skill.fmt(list, { verbose: true }),
77+
].join("\n")
78+
}),
79+
})
80+
}),
81+
)
82+
83+
export const defaultLayer = layer.pipe(Layer.provide(Skill.defaultLayer))
7684
}

packages/opencode/src/tool/bash.ts

Lines changed: 48 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -454,52 +454,53 @@ export const BashTool = Tool.define(
454454
}
455455
})
456456

457-
return async () => {
458-
const shell = Shell.acceptable()
459-
const name = Shell.name(shell)
460-
const chain =
461-
name === "powershell"
462-
? "If the commands depend on each other and must run sequentially, avoid '&&' in this shell because Windows PowerShell 5.1 does not support it. Use PowerShell conditionals such as `cmd1; if ($?) { cmd2 }` when later commands must depend on earlier success."
463-
: "If the commands depend on each other and must run sequentially, use a single Bash call with '&&' to chain them together (e.g., `git add . && git commit -m \"message\" && git push`). For instance, if one operation must complete before another starts (like mkdir before cp, Write before Bash for git operations, or git add before git commit), run these operations sequentially instead."
464-
log.info("bash tool using shell", { shell })
465-
466-
return {
467-
description: DESCRIPTION.replaceAll("${directory}", Instance.directory)
468-
.replaceAll("${os}", process.platform)
469-
.replaceAll("${shell}", name)
470-
.replaceAll("${chaining}", chain)
471-
.replaceAll("${maxLines}", String(Truncate.MAX_LINES))
472-
.replaceAll("${maxBytes}", String(Truncate.MAX_BYTES)),
473-
parameters: Parameters,
474-
execute: (params: z.infer<typeof Parameters>, ctx: Tool.Context) =>
475-
Effect.gen(function* () {
476-
const cwd = params.workdir
477-
? yield* resolvePath(params.workdir, Instance.directory, shell)
478-
: Instance.directory
479-
if (params.timeout !== undefined && params.timeout < 0) {
480-
throw new Error(`Invalid timeout value: ${params.timeout}. Timeout must be a positive number.`)
481-
}
482-
const timeout = params.timeout ?? DEFAULT_TIMEOUT
483-
const ps = PS.has(name)
484-
const root = yield* parse(params.command, ps)
485-
const scan = yield* collect(root, cwd, ps, shell)
486-
if (!Instance.containsPath(cwd)) scan.dirs.add(cwd)
487-
yield* ask(ctx, scan)
488-
489-
return yield* run(
490-
{
491-
shell,
492-
name,
493-
command: params.command,
494-
cwd,
495-
env: yield* shellEnv(ctx, cwd),
496-
timeout,
497-
description: params.description,
498-
},
499-
ctx,
500-
)
501-
}),
502-
}
503-
}
457+
return () =>
458+
Effect.sync(() => {
459+
const shell = Shell.acceptable()
460+
const name = Shell.name(shell)
461+
const chain =
462+
name === "powershell"
463+
? "If the commands depend on each other and must run sequentially, avoid '&&' in this shell because Windows PowerShell 5.1 does not support it. Use PowerShell conditionals such as `cmd1; if ($?) { cmd2 }` when later commands must depend on earlier success."
464+
: "If the commands depend on each other and must run sequentially, use a single Bash call with '&&' to chain them together (e.g., `git add . && git commit -m \"message\" && git push`). For instance, if one operation must complete before another starts (like mkdir before cp, Write before Bash for git operations, or git add before git commit), run these operations sequentially instead."
465+
log.info("bash tool using shell", { shell })
466+
467+
return {
468+
description: DESCRIPTION.replaceAll("${directory}", Instance.directory)
469+
.replaceAll("${os}", process.platform)
470+
.replaceAll("${shell}", name)
471+
.replaceAll("${chaining}", chain)
472+
.replaceAll("${maxLines}", String(Truncate.MAX_LINES))
473+
.replaceAll("${maxBytes}", String(Truncate.MAX_BYTES)),
474+
parameters: Parameters,
475+
execute: (params: z.infer<typeof Parameters>, ctx: Tool.Context) =>
476+
Effect.gen(function* () {
477+
const cwd = params.workdir
478+
? yield* resolvePath(params.workdir, Instance.directory, shell)
479+
: Instance.directory
480+
if (params.timeout !== undefined && params.timeout < 0) {
481+
throw new Error(`Invalid timeout value: ${params.timeout}. Timeout must be a positive number.`)
482+
}
483+
const timeout = params.timeout ?? DEFAULT_TIMEOUT
484+
const ps = PS.has(name)
485+
const root = yield* parse(params.command, ps)
486+
const scan = yield* collect(root, cwd, ps, shell)
487+
if (!Instance.containsPath(cwd)) scan.dirs.add(cwd)
488+
yield* ask(ctx, scan)
489+
490+
return yield* run(
491+
{
492+
shell,
493+
name,
494+
command: params.command,
495+
cwd,
496+
env: yield* shellEnv(ctx, cwd),
497+
timeout,
498+
description: params.description,
499+
},
500+
ctx,
501+
)
502+
}),
503+
}
504+
})
504505
}),
505506
)

packages/opencode/src/tool/multiedit.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export const MultiEditTool = Tool.define(
1010
"multiedit",
1111
Effect.gen(function* () {
1212
const editInfo = yield* EditTool
13-
const edit = yield* Effect.promise(() => editInfo.init())
13+
const edit = yield* editInfo.init()
1414

1515
return {
1616
description: DESCRIPTION,

0 commit comments

Comments
 (0)