fix(provider): sync compatibility fixes#304
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughPreserves existing model Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ProviderTransform
participant ModelRegistry
participant ProviderSDK
Client->>ProviderTransform: Send assistant message + model id
ProviderTransform->>ModelRegistry: Lookup model metadata (api.id, variants)
alt model is DeepSeek (by api.id)
ProviderTransform->>ProviderTransform: Ensure assistant message has reasoning part (inject or normalize)
end
ProviderTransform->>ProviderSDK: Emit normalized model messages + providerOptions
ProviderSDK->>ProviderSDK: Apply provider-specific options (toolStreaming, schema sanitization)
ProviderSDK-->>Client: Deliver processed response/messages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces several provider-specific enhancements and bug fixes, including updated dependencies for @openrouter/ai-sdk-provider, improved handling of interleaved capabilities during model overrides, and the addition of reasoning content for DeepSeek models. It also implements tool streaming restrictions for certain Anthropic SDK configurations and adds a sanitization layer for Moonshot/Kimi JSON schemas to handle specific API constraints. Feedback was provided regarding the use of broad string inclusion checks for model identification; it is recommended to use more explicit regular expressions or canonical identifiers to prevent unintended behavior across different model versions.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/provider/transform.ts`:
- Around line 553-556: The switch case for "@ai-sdk/openai-compatible" declares
openaiCompatibleEfforts in the case scope which violates noSwitchDeclarations;
fix it by wrapping the entire case body in its own block (e.g., case
"@ai-sdk/openai-compatible": { ... } ), move the declaration of
openaiCompatibleEfforts, the conditional check using
model.api.id.includes("deepseek-v4"), and the return Object.fromEntries(...)
into that block, and then close the block so the switch remains valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 774b0fd8-f3a3-40c8-bfc5-1adc55713d16
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
packages/opencode/package.jsonpackages/opencode/src/provider/provider.tspackages/opencode/src/provider/transform.tspackages/opencode/test/provider/provider.test.tspackages/opencode/test/provider/transform.test.tspackages/opencode/test/session/message-v2.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: smoke-macos-arm64
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-desktop
- GitHub Check: unit-opencode
- GitHub Check: unit-app
- GitHub Check: typecheck
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.ts: UseEffect.gen(function* () { ... })for Effect composition
UseEffect.fn("Domain.method")for named/traced effects andEffect.fnUntracedfor internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer.pipe()wrappers
UseEffect.callbackfor callback-based APIs
PreferDateTime.nowAsDateovernew Date(yield* Clock.currentTimeMillis)when you need aDatein Effect code
UseSchema.Classfor multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
UseSchema.TaggedErrorClassfor typed errors in Effect schemas
UseSchema.Defectinstead ofunknownfor defect-like causes in Effect code
InEffect.gen/Effect.fn, preferyield* new MyError(...)overyield* Effect.fail(new MyError(...))for direct early-failure branches
UsemakeRuntimefromsrc/effect/run-service.tsfor all services; it returns{ runPromise, runFork, runCallback }backed by a sharedmemoMapthat deduplicates layers
UseInstanceStatefromsrc/effect/instance-state.tsfor per-directory or per-project state that needs per-instance cleanup; do work directly in theInstanceState.makeclosure whereScopedCachehandles run-once semantics
UseEffect.addFinalizerorEffect.acquireReleaseinside theInstanceState.makeclosure for cleanup (subscriptions, process teardown, etc.)
UseEffect.forkScopedinside theInstanceState.makeclosure for background stream consumers — the fiber is interrupted when the instance is disposed
PreferFileSystem.FileSysteminstead of rawfs/promisesfor effectful file I/O in Effect services
PreferChildProcessSpawner.ChildProcessSpawnerwithChildProcess.make(...)instead of custom process wrappers in Effect services
PreferHttpClient.HttpClientinstead of rawfetchin Effect services
PreferPath.Path,Config,Clock, andDateTimeservices when those concerns are already inside Effect code
For backgroun...
Files:
packages/opencode/src/provider/provider.tspackages/opencode/test/provider/provider.test.tspackages/opencode/test/session/message-v2.test.tspackages/opencode/test/provider/transform.test.tspackages/opencode/src/provider/transform.ts
packages/opencode/test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (packages/opencode/test/AGENTS.md)
packages/opencode/test/**/*.test.{ts,tsx}: Use thetmpdirfunction fromfixture/fixture.tsto create temporary directories for tests with automatic cleanup. Useawait usingsyntax to ensure automatic cleanup when the variable goes out of scope.
When using thetmpdirfunction with git repository support, pass thegit: trueoption to initialize a git repo with a root commit.
Use theconfigoption intmpdirto write anopencode.jsonconfig file during test setup by passing a partial Config.Info object.
Use theinitoption intmpdirto define custom setup functions that can return extra data accessible viatmp.extra, and use thedisposeoption for custom cleanup logic.
UsetestEffect(...)fromtest/lib/effect.tsfor tests that exercise Effect services or Effect-based workflows.
Useit.effect(...)when the test should run withTestClockandTestConsole. Useit.live(...)when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers fromfixture/fixture.tsover building manual runtimes in tests: usetmpdirScoped()for scoped temp directories,provideInstance(dir)(effect)for low-level binding without directory creation,provideTmpdirInstance(...)for single temp instance binding, orprovideTmpdirServer(...)for tests that also need the test LLM server.
Defineconst it = testEffect(...)near the top of the test file and keep the test body insideEffect.gen(function* () { ... }). Yield services directly withyield* MyService.Serviceoryield* MyTool.
Avoid customManagedRuntime,attach(...), or ad hocrun(...)wrappers in Effect tests whentestEffect(...)already provides the runtime.
When a test needs instance-local state, preferprovideTmpdirInstance(...)orprovideInstance(...)over manualInstance.provide(...)inside Promise-style tests.
Files:
packages/opencode/test/provider/provider.test.tspackages/opencode/test/session/message-v2.test.tspackages/opencode/test/provider/transform.test.ts
🧠 Learnings (23)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 211
File: packages/opencode/src/provider/models.ts:113-179
Timestamp: 2026-04-24T06:50:05.142Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/provider/models.ts`), `PublishModel` and `PublishProvider` intentionally duplicate fields from `Model` and `Provider` with looser optionality. The split is a deliberate behavior boundary: `PublishModel`/`PublishProvider` (with `.passthrough()`) are used for lenient publish-time validation of incoming catalogs, while `Model`/`Provider` are used for strict runtime normalization. Do not flag this duplication as a maintenance burden or suggest extracting a shared base schema — the explicit separation is intentional and tied to the models-refresh reliability path introduced in PR `#211`.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 302
File: packages/opencode/test/session/retry.test.ts:298-315
Timestamp: 2026-04-28T14:46:41.449Z
Learning: In Astro-Han/pawwork (PR `#302`, `packages/opencode/src/provider/error.ts` + `packages/opencode/test/session/retry.test.ts`), upstream anomalyco/opencode#24063 intentionally treats ALL OpenAI streaming `server_error` chunks as retryable (`isRetryable: true`). Do NOT request a test asserting `isRetryable: false` for a `server_error` stream chunk; the only negative regression needed is a non-`server_error` code (e.g. `bad_request`) to confirm the retry gate stays closed for other error codes.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/lsp.ts:23-32
Timestamp: 2026-04-28T07:28:14.317Z
Learning: In `packages/opencode/src/tool/lsp.ts` (Astro-Han/pawwork, PR `#270`), the `Parameters` schema requires `line` and `character` for all operations, including `workspaceSymbol` and `documentSymbol` which never use coordinates. This matches the upstream `dev:packages/opencode/src/tool/lsp.ts:23-32` baseline exactly — both fields are declared as required `Schema.Number` with `>= 1` checks. The fix (per-operation schema split, or making `line`/`character` optional with handler-side presence validation for operations that need them like `goToDefinition`/`findReferences`) is deferred to a follow-up PR or upstream report to avoid mixing refactor + bug-fix intents and drifting the diff from the upstream baseline. Do NOT re-flag the required coordinates on `workspaceSymbol`/`documentSymbol` in upstream-sync PRs; flag it only in a PawWork-authored PR that directly touches `lsp.ts` parameter validation.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:45-72
Timestamp: 2026-04-28T07:27:49.810Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork), `PathText`, the `lines` field inside `SearchMatch`, and `submatches[].match` accept only `{text: string}` and do NOT handle ripgrep's `{bytes: base64}` JSON variant (emitted for non-UTF-8 paths/match text). This matches the upstream `dev:packages/opencode/src/file/ripgrep.ts` baseline exactly. The fix — adding `Schema.Union([Schema.Struct({text: Schema.String}), Schema.Struct({bytes: Schema.String})])` for each location plus base64-decode in downstream consumers — is intentionally deferred to either a dedicated PawWork PR or an upstream-first fix inherited via the next sync. Do NOT re-flag the missing `{bytes: ...}` union variants in upstream-sync PRs; flag it only in a PawWork-authored PR that directly touches these schema definitions or their downstream consumers.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:11.771Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/patch/index.ts:337-346
Timestamp: 2026-04-28T04:38:05.946Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/patch/index.ts`), the BOM-transition surfacing gap — where `Bom.split` strips BOM before building `unified_diff`/`new_content` but `Bom.join` later re-attaches BOM on disk write, meaning BOM changes are not reflected in the diff payload — is intentionally deferred. PR `#270` is an upstream-sync graft; fixing the issue here would mix refactor + bugfix intents and drift the diff from the upstream baseline needed for clean future grafts. A dedicated follow-up PR (or upstream report) will address this. Do NOT re-flag the missing BOM-change surfacing in `ApplyPatchFileUpdate`/`ApplyPatchFileChange` as a blocking issue in PR `#270` or in future sync PRs that carry the same upstream baseline.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 73
File: packages/opencode/src/cli/cmd/tui/context/sync.tsx:486-489
Timestamp: 2026-04-20T17:03:40.214Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/cli/cmd/tui/context/sync.tsx`), `sync.ready` returning `true` when `process.env.OPENCODE_FAST_BOOT` is set is intentional. The plugin-facing data properties `state.config` (initialized to `{}`) and `state.provider` (initialized to `[]`) expose safe-empty defaults, so they are safe to access before bootstrap completes. Do not flag these as needing null-guards or conditional patterns to match `vcs` — the difference is intentional because `vcs` starts as `undefined` while the others have initialized defaults. Changing this would alter the plugin API contract without a concrete failing case.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:35.312Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/tool.ts:34-42
Timestamp: 2026-04-28T04:56:21.528Z
Learning: In `packages/opencode/src/tool/tool.ts` (Astro-Han/pawwork, PR `#270`), the `Def.execute` signature uses `ctx: Context` (without the `M` generic) instead of `ctx: Context<M>`, meaning `ctx.metadata(...)` is not checked against the same `M` as `ExecuteResult<M>`. This is inherited upstream behavior adopted via the graft strategy in PR `#270` (an upstream-sync PR per `project_upstream_strategy.md`). Fixing it here would mix refactor + bugfix intents and drift the diff from the upstream baseline. The fix is deferred to a dedicated follow-up PR or upstream report. Do NOT re-flag the missing `Context<M>` generic in `Def.execute` during upstream-sync PRs; only flag it in a PawWork-authored PR that directly touches `tool.ts` type signatures.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:02.858Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.
📚 Learning: 2026-04-24T06:50:05.142Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 211
File: packages/opencode/src/provider/models.ts:113-179
Timestamp: 2026-04-24T06:50:05.142Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/provider/models.ts`), `PublishModel` and `PublishProvider` intentionally duplicate fields from `Model` and `Provider` with looser optionality. The split is a deliberate behavior boundary: `PublishModel`/`PublishProvider` (with `.passthrough()`) are used for lenient publish-time validation of incoming catalogs, while `Model`/`Provider` are used for strict runtime normalization. Do not flag this duplication as a maintenance burden or suggest extracting a shared base schema — the explicit separation is intentional and tied to the models-refresh reliability path introduced in PR `#211`.
Applied to files:
packages/opencode/src/provider/provider.tspackages/opencode/test/provider/provider.test.tspackages/opencode/test/provider/transform.test.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T14:46:33.806Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 302
File: packages/opencode/src/plugin/codex.ts:0-0
Timestamp: 2026-04-28T14:46:33.806Z
Learning: In `packages/opencode/src/plugin/codex.ts` (Astro-Han/pawwork), the Provider SDK v1 hook `Model` type omits the `input` field on `limit`, but opencode uses `limit.input` for compaction budget. The pattern to avoid a bare `ts-expect-error` is to declare a local typed intersection `const limit: typeof model.limit & { input: number } = { context: ..., input: ..., output: ... }` and assign that to `model.limit`. Do not flag the intersection cast as unnecessary — it is the intentional way to type-safely add `input` without suppressing the whole assignment.
Applied to files:
packages/opencode/src/provider/provider.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-21T16:00:44.910Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 102
File: packages/opencode/src/config/model-id.ts:9-11
Timestamp: 2026-04-21T16:00:44.910Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/config/model-id.ts`), `ConfigModelID` uses a regex that requires a non-empty provider segment and a non-empty model segment separated by `/`, but intentionally allows additional slashes within the model segment. This is because `Provider.parseModel` supports model IDs like `synthetic/hf:zai-org/GLM-4.7`. Do not flag the permissive slash handling as a bug.
Applied to files:
packages/opencode/src/provider/provider.tspackages/opencode/test/provider/provider.test.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-27T08:58:00.665Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:531-538
Timestamp: 2026-04-27T08:58:00.665Z
Learning: When using Effect (e.g., `yield*` with `Effect`-style generator yielding), only use `yield* new SomeErrorClass(...)` if `SomeErrorClass` extends `Schema.TaggedErrorClass` (i.e., it implements Effect’s Yieldable interface). For plain `Error` subclasses (like `BlockedLoopError` / `LoopStopError`) or inline `new Error(...)` values, they are not yieldable and must be wrapped as `yield* Effect.fail(new PlainError(...))`. Do not recommend changing `yield* Effect.fail(new SomePlainError(...))` to `yield* new SomePlainError(...)` unless the error class extends `Schema.TaggedErrorClass`.
Applied to files:
packages/opencode/src/provider/provider.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/provider/provider.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-22T09:32:52.806Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/opencode/test/provider/provider.test.ts:64-85
Timestamp: 2026-04-22T09:32:52.806Z
Learning: For tests under `packages/opencode/test/provider/` (e.g., `*.test.ts`), keep the test harness pattern consistent across the whole file: if the file intentionally uses AppRuntime-based async helpers (`run`, `list`, `getProvider`, etc.) instead of `testEffect(...)`, do not partially convert individual tests to `testEffect` while leaving others on the async helper pattern. If the harness pattern must change, migrate the entire file as a cohesive harness migration (ideally as a separate PR) to avoid internal inconsistency.
Applied to files:
packages/opencode/test/provider/provider.test.tspackages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-27T11:18:47.596Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/agent/agent.test.ts:440-447
Timestamp: 2026-04-27T11:18:47.596Z
Learning: In `packages/opencode/test/agent/agent.test.ts` (Astro-Han/pawwork), all agent-permission tests intentionally use the manual `tmpdir()` + `Instance.provide(...)` pattern. Do not flag individual tests in this file for conversion to `provideTmpdirInstance(...)` or `provideInstance(...)`; a full harness migration would be a separate PR if the pattern ever needs to change.
Applied to files:
packages/opencode/test/provider/provider.test.tspackages/opencode/test/session/message-v2.test.tspackages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `config` option in `tmpdir` to write an `opencode.json` config file during test setup by passing a partial Config.Info object.
Applied to files:
packages/opencode/test/provider/provider.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : When a test needs instance-local state, prefer `provideTmpdirInstance(...)` or `provideInstance(...)` over manual `Instance.provide(...)` inside Promise-style tests.
Applied to files:
packages/opencode/test/provider/provider.test.tspackages/opencode/test/session/message-v2.test.ts
📚 Learning: 2026-04-28T05:36:25.456Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/grep.test.ts:16-22
Timestamp: 2026-04-28T05:36:25.456Z
Learning: In `packages/opencode/test/tool/grep.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a manual `ManagedRuntime.make(Layer.mergeAll(...))` setup and raw `bun:test` test cases rather than the `testEffect(...)` harness. The migration to `testEffect` + `it.live(...)` + `provideTmpdirInstance(...)` is a whole-file rewrite deferred to a dedicated sweep PR that will migrate all tool tests still on the manual ManagedRuntime pattern together. Do NOT re-flag the ManagedRuntime setup in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/opencode/test/provider/provider.test.tspackages/opencode/test/session/message-v2.test.tspackages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-28T04:56:21.338Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/tool-define.test.ts:2-9
Timestamp: 2026-04-28T04:56:21.338Z
Learning: In `packages/opencode/test/tool/tool-define.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a `ManagedRuntime.make(Layer.mergeAll(Truncate.defaultLayer, Agent.defaultLayer))` wrapper and raw `bun:test` `test(...)` cases rather than the `testEffect(...)` harness. This code was adopted wholesale from upstream via the graft strategy (per `project_upstream_strategy.md`). Migrating to `testEffect` is deferred to a follow-up PR so as not to mix refactor + harness-migration intents and to avoid drifting the diff from the upstream baseline needed for clean future grafts. Do NOT re-flag the `ManagedRuntime.make` usage in this file as needing harness migration until that follow-up lands.
Applied to files:
packages/opencode/test/provider/provider.test.tspackages/opencode/test/session/message-v2.test.tspackages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-23T08:51:00.819Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:00.819Z
Learning: For pawwork tests under packages/opencode/test/**, auth.json teardown may intentionally combine `Filesystem.write` (from `packages/opencode/src/util/filesystem.ts`) with `node:fs/promises` `unlink` for cleanup. Do not flag this as inconsistent style; it is the established/intentional pattern because `Filesystem` does not provide a `remove`/`unlink` helper.
Applied to files:
packages/opencode/test/provider/provider.test.tspackages/opencode/test/session/message-v2.test.tspackages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-22T09:32:54.556Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/opencode/test/provider/provider.test.ts:64-85
Timestamp: 2026-04-22T09:32:54.556Z
Learning: In `packages/opencode/test/provider/provider.test.ts`, the file intentionally uses AppRuntime-based async helpers (`run`, `list`, `getProvider`, etc.) rather than `testEffect(...)` for all tests. Converting individual tests to `testEffect` while leaving the rest on the async pattern would create internal inconsistency. A full harness migration of this file is the right approach if the pattern needs to change, but that should be a separate PR.
Applied to files:
packages/opencode/test/session/message-v2.test.ts
📚 Learning: 2026-04-28T11:24:35.312Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:35.312Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/opencode/test/session/message-v2.test.tspackages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-27T12:59:45.694Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/test/session/prompt-effect.test.ts:0-0
Timestamp: 2026-04-27T12:59:45.694Z
Learning: In session prompt/diagnostics tests (e.g., when verifying injected LLM “recovery reminder” copy), do not assert a single exact reminder phrasing tied to signature kind. If a scenario can produce both `input:` and `target:` signatures (for example, a `read` tool call with a `filePath` parameter), accept either phrasing: the input-repeat variant may say “repeated the same tool input 3 times” (literal count), and the target-repeat variant may say “failed against the same target multiple times” (“multiple times” without a count). Your assertions should allow both variants rather than narrowing to only the input-variant text.
Applied to files:
packages/opencode/test/session/message-v2.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Define `const it = testEffect(...)` near the top of the test file and keep the test body inside `Effect.gen(function* () { ... })`. Yield services directly with `yield* MyService.Service` or `yield* MyTool`.
Applied to files:
packages/opencode/test/session/message-v2.test.ts
📚 Learning: 2026-04-27T11:19:24.963Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch-auth.test.ts:0-0
Timestamp: 2026-04-27T11:19:24.963Z
Learning: In `packages/opencode/test/tool/websearch-auth.test.ts` (Astro-Han/pawwork), the tests intentionally use a small local `runWith` runner with raw `bun:test` and `Effect.runPromise` rather than the `testEffect` harness. Each test case injects a custom in-memory `Auth.Service` layer; switching to `testEffect` would be style-only churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/opencode/test/session/message-v2.test.tspackages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-27T11:18:47.332Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/mcp-exa.test.ts:1-186
Timestamp: 2026-04-27T11:18:47.332Z
Learning: In `packages/opencode/test/tool/mcp-exa.test.ts` (Astro-Han/pawwork), the tests intentionally use raw `bun:test` async cases with `Effect.runPromise(...)` and per-case `HttpClient.make(...)` fakes rather than the `testEffect(...)` harness. The maintainer has explicitly decided not to migrate, because the HttpClient fake wiring is itself the behavior under test and switching to `testEffect` would be style churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/opencode/test/session/message-v2.test.tspackages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-22T08:49:47.800Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-28T06:47:20.342Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/question.ts:6-16
Timestamp: 2026-04-28T06:47:20.342Z
Learning: In Astro-Han/pawwork, `packages/opencode/src/tool/question.ts` is a permanent PawWork carve-out: `Question.Prompt` (defined in `packages/opencode/src/question/index.ts`) remains a Zod schema, NOT an Effect Schema. The `ts-expect-error` boundary in `tool/registry.ts` marks the Zod ↔ Effect Schema seam. The dropped `packages/opencode/test/tool/question.test.ts` covered the old Zod-mixed surface. Schema-bound regression tests for this tool are intentionally deferred until `question/index.ts` migrates to Effect Schema in its own dedicated PR. Do NOT request restoring question tool tests in upstream-sync PRs; flag it only in the PR that migrates `question/index.ts` to Effect Schema.
Applied to files:
packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-27T11:18:47.298Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch.test.ts:21-78
Timestamp: 2026-04-27T11:18:47.298Z
Learning: In `packages/opencode/test/tool/websearch.test.ts`, the tests intentionally use manual `Effect.runPromise` with explicit `Effect.provide(...)` chains (including `Layer.succeed(Auth.Service, ...)`, `Layer.succeed(HttpClient.HttpClient, http)`, `WebSearchAuth.layer`, `Truncate.defaultLayer`, and `Agent.defaultLayer`) rather than the `testEffect(...)` harness. This is by design: the fake Auth and HTTP recovery-metadata layers must be explicitly injected and kept visible/scoped at the test site. Do NOT suggest migrating these tests to `testEffect` or removing the manual layer provides.
Applied to files:
packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-28T04:56:13.350Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/tool.ts:8-10
Timestamp: 2026-04-28T04:56:13.350Z
Learning: In `packages/opencode/src/tool/tool.ts` (Astro-Han/pawwork, PR `#270`), the `Metadata` interface uses `[key: string]: any` as its index signature. This is upstream-inherited code adopted wholesale via the graft strategy (per `project_upstream_strategy.md`). The fix — tightening to `Record<string, unknown>` and explicitly narrowing framework-owned fields like `truncated` — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag the `any` index signature on `Metadata` in PR `#270` or in future upstream-sync PRs that carry the same upstream baseline.
Applied to files:
packages/opencode/src/provider/transform.ts
🪛 Biome (2.4.13)
packages/opencode/src/provider/transform.ts
[error] 554-554: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (7)
packages/opencode/src/provider/provider.ts (1)
1189-1212: Good fix: preserve object-valuedinterleavedmetadata during config merges.This keeps config-only overrides from silently disabling downstream interleaved reasoning handling for models whose default metadata uses
{ field: ... }.packages/opencode/test/provider/provider.test.ts (1)
1164-1198: Useful regression coverage for the merge fix.This pins the
zhipuai/glm-5override case that used to drop object-valuedinterleaveddefaults while only renaming the model.packages/opencode/test/session/message-v2.test.ts (1)
839-910: Nice regression test for OpenRouter reasoning details.This exercises the transform boundary that now skips interleaved extraction for
@openrouter/ai-sdk-providerand keepsproviderOptions.openrouter.reasoning_detailsintact.packages/opencode/test/provider/transform.test.ts (4)
925-1007: Good coverage for Moonshot/Kimi schema sanitization.These cases cover the
$refsibling stripping, the Kimi-outside-Moonshot path, and tuple-styleitemsnormalization.
2315-2356: Looks good.The DeepSeek V4
maxvariant closes the new reasoning-effort gap, and the V3/no-reasoning cases keep the older branches covered.
241-306: Test case correctly exercises the production branch—no provider ID gating for toolStreaming.The
options()function gatestoolStreamingonly onmodel.api.npm(SDK package) andmodel.api.id(model ID), not onproviderID. The test case withproviderID: "test"correctly verifies both paths: it exercises the condition!model.api.id.includes("claude") && model.api.npm === "@ai-sdk/anthropic"for the non-Claude Anthropic case, which is what the implementation checks. TheproviderIDvalue does not affect this logic.
1132-1219: The new test fixtures are correctly shaped and match the runtime DeepSeek model definitions.The
deepseek-chatmodel in the fixture file has"reasoning": falseand nointerleavedfield, so the new fixtures correctly useinterleaved: false(boolean). This intentionally tests the first branch at line 178 (model.api.id.includes("deepseek")), which adds an empty reasoning block to assistant messages. The second branch at line 195 only triggers wheninterleavedis an object with afieldproperty, which applies to thedeepseek-reasonermodel and is separately tested at line 1010 withinterleaved: { field: "reasoning_content" }. Both code paths are covered appropriately.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/provider/transform.ts`:
- Around line 1069-1079: The sanitizeMoonshot routine strips root-level $defs
when it collapses objects with a $ref to just { $ref }, causing unresolved
references; update the sanitizeMoonshot function (used under the
model.providerID === "moonshotai" || model.api.id.includes("kimi") branch) so
that when it detects an object with a string $ref it returns an object that
preserves any existing $defs (e.g., return { $ref: obj.$ref, ...(obj.$defs ? {
$defs: obj.$defs } : {}) }) instead of dropping them, and ensure this
preservation also applies for root schemas passed to schema =
sanitizeMoonshot(schema).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5b9c133e-e856-4726-8d08-d7d8e1003253
📒 Files selected for processing (2)
packages/opencode/src/provider/transform.tspackages/opencode/test/provider/transform.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.ts: UseEffect.gen(function* () { ... })for Effect composition
UseEffect.fn("Domain.method")for named/traced effects andEffect.fnUntracedfor internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer.pipe()wrappers
UseEffect.callbackfor callback-based APIs
PreferDateTime.nowAsDateovernew Date(yield* Clock.currentTimeMillis)when you need aDatein Effect code
UseSchema.Classfor multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
UseSchema.TaggedErrorClassfor typed errors in Effect schemas
UseSchema.Defectinstead ofunknownfor defect-like causes in Effect code
InEffect.gen/Effect.fn, preferyield* new MyError(...)overyield* Effect.fail(new MyError(...))for direct early-failure branches
UsemakeRuntimefromsrc/effect/run-service.tsfor all services; it returns{ runPromise, runFork, runCallback }backed by a sharedmemoMapthat deduplicates layers
UseInstanceStatefromsrc/effect/instance-state.tsfor per-directory or per-project state that needs per-instance cleanup; do work directly in theInstanceState.makeclosure whereScopedCachehandles run-once semantics
UseEffect.addFinalizerorEffect.acquireReleaseinside theInstanceState.makeclosure for cleanup (subscriptions, process teardown, etc.)
UseEffect.forkScopedinside theInstanceState.makeclosure for background stream consumers — the fiber is interrupted when the instance is disposed
PreferFileSystem.FileSysteminstead of rawfs/promisesfor effectful file I/O in Effect services
PreferChildProcessSpawner.ChildProcessSpawnerwithChildProcess.make(...)instead of custom process wrappers in Effect services
PreferHttpClient.HttpClientinstead of rawfetchin Effect services
PreferPath.Path,Config,Clock, andDateTimeservices when those concerns are already inside Effect code
For backgroun...
Files:
packages/opencode/test/provider/transform.test.tspackages/opencode/src/provider/transform.ts
packages/opencode/test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (packages/opencode/test/AGENTS.md)
packages/opencode/test/**/*.test.{ts,tsx}: Use thetmpdirfunction fromfixture/fixture.tsto create temporary directories for tests with automatic cleanup. Useawait usingsyntax to ensure automatic cleanup when the variable goes out of scope.
When using thetmpdirfunction with git repository support, pass thegit: trueoption to initialize a git repo with a root commit.
Use theconfigoption intmpdirto write anopencode.jsonconfig file during test setup by passing a partial Config.Info object.
Use theinitoption intmpdirto define custom setup functions that can return extra data accessible viatmp.extra, and use thedisposeoption for custom cleanup logic.
UsetestEffect(...)fromtest/lib/effect.tsfor tests that exercise Effect services or Effect-based workflows.
Useit.effect(...)when the test should run withTestClockandTestConsole. Useit.live(...)when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers fromfixture/fixture.tsover building manual runtimes in tests: usetmpdirScoped()for scoped temp directories,provideInstance(dir)(effect)for low-level binding without directory creation,provideTmpdirInstance(...)for single temp instance binding, orprovideTmpdirServer(...)for tests that also need the test LLM server.
Defineconst it = testEffect(...)near the top of the test file and keep the test body insideEffect.gen(function* () { ... }). Yield services directly withyield* MyService.Serviceoryield* MyTool.
Avoid customManagedRuntime,attach(...), or ad hocrun(...)wrappers in Effect tests whentestEffect(...)already provides the runtime.
When a test needs instance-local state, preferprovideTmpdirInstance(...)orprovideInstance(...)over manualInstance.provide(...)inside Promise-style tests.
Files:
packages/opencode/test/provider/transform.test.ts
🧠 Learnings (28)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 302
File: packages/opencode/test/session/retry.test.ts:298-315
Timestamp: 2026-04-28T14:46:41.449Z
Learning: In Astro-Han/pawwork (PR `#302`, `packages/opencode/src/provider/error.ts` + `packages/opencode/test/session/retry.test.ts`), upstream anomalyco/opencode#24063 intentionally treats ALL OpenAI streaming `server_error` chunks as retryable (`isRetryable: true`). Do NOT request a test asserting `isRetryable: false` for a `server_error` stream chunk; the only negative regression needed is a non-`server_error` code (e.g. `bad_request`) to confirm the retry gate stays closed for other error codes.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 211
File: packages/opencode/src/provider/models.ts:113-179
Timestamp: 2026-04-24T06:50:05.142Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/provider/models.ts`), `PublishModel` and `PublishProvider` intentionally duplicate fields from `Model` and `Provider` with looser optionality. The split is a deliberate behavior boundary: `PublishModel`/`PublishProvider` (with `.passthrough()`) are used for lenient publish-time validation of incoming catalogs, while `Model`/`Provider` are used for strict runtime normalization. Do not flag this duplication as a maintenance burden or suggest extracting a shared base schema — the explicit separation is intentional and tied to the models-refresh reliability path introduced in PR `#211`.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:02.858Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/patch/index.ts:337-346
Timestamp: 2026-04-28T04:38:05.946Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/patch/index.ts`), the BOM-transition surfacing gap — where `Bom.split` strips BOM before building `unified_diff`/`new_content` but `Bom.join` later re-attaches BOM on disk write, meaning BOM changes are not reflected in the diff payload — is intentionally deferred. PR `#270` is an upstream-sync graft; fixing the issue here would mix refactor + bugfix intents and drift the diff from the upstream baseline needed for clean future grafts. A dedicated follow-up PR (or upstream report) will address this. Do NOT re-flag the missing BOM-change surfacing in `ApplyPatchFileUpdate`/`ApplyPatchFileChange` as a blocking issue in PR `#270` or in future sync PRs that carry the same upstream baseline.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:11.771Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:45-72
Timestamp: 2026-04-28T07:27:49.810Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork), `PathText`, the `lines` field inside `SearchMatch`, and `submatches[].match` accept only `{text: string}` and do NOT handle ripgrep's `{bytes: base64}` JSON variant (emitted for non-UTF-8 paths/match text). This matches the upstream `dev:packages/opencode/src/file/ripgrep.ts` baseline exactly. The fix — adding `Schema.Union([Schema.Struct({text: Schema.String}), Schema.Struct({bytes: Schema.String})])` for each location plus base64-decode in downstream consumers — is intentionally deferred to either a dedicated PawWork PR or an upstream-first fix inherited via the next sync. Do NOT re-flag the missing `{bytes: ...}` union variants in upstream-sync PRs; flag it only in a PawWork-authored PR that directly touches these schema definitions or their downstream consumers.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/lsp.ts:23-32
Timestamp: 2026-04-28T07:28:14.317Z
Learning: In `packages/opencode/src/tool/lsp.ts` (Astro-Han/pawwork, PR `#270`), the `Parameters` schema requires `line` and `character` for all operations, including `workspaceSymbol` and `documentSymbol` which never use coordinates. This matches the upstream `dev:packages/opencode/src/tool/lsp.ts:23-32` baseline exactly — both fields are declared as required `Schema.Number` with `>= 1` checks. The fix (per-operation schema split, or making `line`/`character` optional with handler-side presence validation for operations that need them like `goToDefinition`/`findReferences`) is deferred to a follow-up PR or upstream report to avoid mixing refactor + bug-fix intents and drifting the diff from the upstream baseline. Do NOT re-flag the required coordinates on `workspaceSymbol`/`documentSymbol` in upstream-sync PRs; flag it only in a PawWork-authored PR that directly touches `lsp.ts` parameter validation.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:12.228Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/prompt.ts` and `packages/opencode/src/session/processor.ts`, PR `#264`), the loop-gate race condition between `buildLoopContext()` and `recordSyntheticBlock`/`recordSyntheticStop` is intentionally handled via idempotence guards (re-check sigKey presence / `hasStopped` inside the record helpers) rather than a full per-parent `Effect.Mutex`. Threading a `Map<MessageID, Mutex>` through the processor was considered too large a surface change for this edge case; the residual TOCTOU window only produces extra synthetic parts with no behavioral drift on the "turn ends" contract. A code comment documents the trade-off and points to a full-mutex follow-up if the race is observed in practice. Do NOT re-flag the absence of a per-parent mutex as a blocking issue in future reviews.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 73
File: packages/opencode/src/cli/cmd/tui/context/sync.tsx:486-489
Timestamp: 2026-04-20T17:03:40.214Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/cli/cmd/tui/context/sync.tsx`), `sync.ready` returning `true` when `process.env.OPENCODE_FAST_BOOT` is set is intentional. The plugin-facing data properties `state.config` (initialized to `{}`) and `state.provider` (initialized to `[]`) expose safe-empty defaults, so they are safe to access before bootstrap completes. Do not flag these as needing null-guards or conditional patterns to match `vcs` — the difference is intentional because `vcs` starts as `undefined` while the others have initialized defaults. Changing this would alter the plugin API contract without a concrete failing case.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:311-328
Timestamp: 2026-04-28T05:36:18.200Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the ripgrep binary download flow (`HttpClientRequest.get` → `fs.writeWithDirs` → `extract`) does NOT verify a pinned SHA-256 checksum. Adding per-platform hash verification (15.1.0 × 6 platforms) requires a dedicated manifest + `crypto.subtle.digest` step and is intentionally deferred to a follow-up PR. Existing mitigations: (1) `VERSION` is pinned to `"15.1.0"`, (2) downloads are over HTTPS from GitHub Releases, (3) PawWork production builds ship a bundled `rg` binary so this download path is a last-resort fallback (system PATH → cached → download). Do NOT re-flag the missing checksum verification in this file until the dedicated follow-up PR lands.
📚 Learning: 2026-04-28T04:56:21.338Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/tool-define.test.ts:2-9
Timestamp: 2026-04-28T04:56:21.338Z
Learning: In `packages/opencode/test/tool/tool-define.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a `ManagedRuntime.make(Layer.mergeAll(Truncate.defaultLayer, Agent.defaultLayer))` wrapper and raw `bun:test` `test(...)` cases rather than the `testEffect(...)` harness. This code was adopted wholesale from upstream via the graft strategy (per `project_upstream_strategy.md`). Migrating to `testEffect` is deferred to a follow-up PR so as not to mix refactor + harness-migration intents and to avoid drifting the diff from the upstream baseline needed for clean future grafts. Do NOT re-flag the `ManagedRuntime.make` usage in this file as needing harness migration until that follow-up lands.
Applied to files:
packages/opencode/test/provider/transform.test.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-22T09:32:52.806Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/opencode/test/provider/provider.test.ts:64-85
Timestamp: 2026-04-22T09:32:52.806Z
Learning: For tests under `packages/opencode/test/provider/` (e.g., `*.test.ts`), keep the test harness pattern consistent across the whole file: if the file intentionally uses AppRuntime-based async helpers (`run`, `list`, `getProvider`, etc.) instead of `testEffect(...)`, do not partially convert individual tests to `testEffect` while leaving others on the async helper pattern. If the harness pattern must change, migrate the entire file as a cohesive harness migration (ideally as a separate PR) to avoid internal inconsistency.
Applied to files:
packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-24T06:50:05.142Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 211
File: packages/opencode/src/provider/models.ts:113-179
Timestamp: 2026-04-24T06:50:05.142Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/provider/models.ts`), `PublishModel` and `PublishProvider` intentionally duplicate fields from `Model` and `Provider` with looser optionality. The split is a deliberate behavior boundary: `PublishModel`/`PublishProvider` (with `.passthrough()`) are used for lenient publish-time validation of incoming catalogs, while `Model`/`Provider` are used for strict runtime normalization. Do not flag this duplication as a maintenance burden or suggest extracting a shared base schema — the explicit separation is intentional and tied to the models-refresh reliability path introduced in PR `#211`.
Applied to files:
packages/opencode/test/provider/transform.test.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T05:36:25.456Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/grep.test.ts:16-22
Timestamp: 2026-04-28T05:36:25.456Z
Learning: In `packages/opencode/test/tool/grep.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a manual `ManagedRuntime.make(Layer.mergeAll(...))` setup and raw `bun:test` test cases rather than the `testEffect(...)` harness. The migration to `testEffect` + `it.live(...)` + `provideTmpdirInstance(...)` is a whole-file rewrite deferred to a dedicated sweep PR that will migrate all tool tests still on the manual ManagedRuntime pattern together. Do NOT re-flag the ManagedRuntime setup in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/opencode/test/provider/transform.test.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-27T11:19:24.963Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch-auth.test.ts:0-0
Timestamp: 2026-04-27T11:19:24.963Z
Learning: In `packages/opencode/test/tool/websearch-auth.test.ts` (Astro-Han/pawwork), the tests intentionally use a small local `runWith` runner with raw `bun:test` and `Effect.runPromise` rather than the `testEffect` harness. Each test case injects a custom in-memory `Auth.Service` layer; switching to `testEffect` would be style-only churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-27T11:18:47.596Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/agent/agent.test.ts:440-447
Timestamp: 2026-04-27T11:18:47.596Z
Learning: In `packages/opencode/test/agent/agent.test.ts` (Astro-Han/pawwork), all agent-permission tests intentionally use the manual `tmpdir()` + `Instance.provide(...)` pattern. Do not flag individual tests in this file for conversion to `provideTmpdirInstance(...)` or `provideInstance(...)`; a full harness migration would be a separate PR if the pattern ever needs to change.
Applied to files:
packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-27T11:18:47.332Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/mcp-exa.test.ts:1-186
Timestamp: 2026-04-27T11:18:47.332Z
Learning: In `packages/opencode/test/tool/mcp-exa.test.ts` (Astro-Han/pawwork), the tests intentionally use raw `bun:test` async cases with `Effect.runPromise(...)` and per-case `HttpClient.make(...)` fakes rather than the `testEffect(...)` harness. The maintainer has explicitly decided not to migrate, because the HttpClient fake wiring is itself the behavior under test and switching to `testEffect` would be style churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-28T11:24:35.312Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:35.312Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/opencode/test/provider/transform.test.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-22T08:49:47.800Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/opencode/test/provider/transform.test.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T06:47:20.342Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/question.ts:6-16
Timestamp: 2026-04-28T06:47:20.342Z
Learning: In Astro-Han/pawwork, `packages/opencode/src/tool/question.ts` is a permanent PawWork carve-out: `Question.Prompt` (defined in `packages/opencode/src/question/index.ts`) remains a Zod schema, NOT an Effect Schema. The `ts-expect-error` boundary in `tool/registry.ts` marks the Zod ↔ Effect Schema seam. The dropped `packages/opencode/test/tool/question.test.ts` covered the old Zod-mixed surface. Schema-bound regression tests for this tool are intentionally deferred until `question/index.ts` migrates to Effect Schema in its own dedicated PR. Do NOT request restoring question tool tests in upstream-sync PRs; flag it only in the PR that migrates `question/index.ts` to Effect Schema.
Applied to files:
packages/opencode/test/provider/transform.test.tspackages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-27T11:18:47.298Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch.test.ts:21-78
Timestamp: 2026-04-27T11:18:47.298Z
Learning: In `packages/opencode/test/tool/websearch.test.ts`, the tests intentionally use manual `Effect.runPromise` with explicit `Effect.provide(...)` chains (including `Layer.succeed(Auth.Service, ...)`, `Layer.succeed(HttpClient.HttpClient, http)`, `WebSearchAuth.layer`, `Truncate.defaultLayer`, and `Agent.defaultLayer`) rather than the `testEffect(...)` harness. This is by design: the fake Auth and HTTP recovery-metadata layers must be explicitly injected and kept visible/scoped at the test site. Do NOT suggest migrating these tests to `testEffect` or removing the manual layer provides.
Applied to files:
packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-23T08:51:00.819Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:00.819Z
Learning: For pawwork tests under packages/opencode/test/**, auth.json teardown may intentionally combine `Filesystem.write` (from `packages/opencode/src/util/filesystem.ts`) with `node:fs/promises` `unlink` for cleanup. Do not flag this as inconsistent style; it is the established/intentional pattern because `Filesystem` does not provide a `remove`/`unlink` helper.
Applied to files:
packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-21T16:00:44.910Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 102
File: packages/opencode/src/config/model-id.ts:9-11
Timestamp: 2026-04-21T16:00:44.910Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/config/model-id.ts`), `ConfigModelID` uses a regex that requires a non-empty provider segment and a non-empty model segment separated by `/`, but intentionally allows additional slashes within the model segment. This is because `Provider.parseModel` supports model IDs like `synthetic/hf:zai-org/GLM-4.7`. Do not flag the permissive slash handling as a bug.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T08:29:02.858Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:02.858Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T04:38:11.771Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:11.771Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T04:56:18.533Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:172-175
Timestamp: 2026-04-28T04:56:18.533Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the `files dies on nonexistent directory` test hardcodes `/tmp/nonexistent-dir-12345` as the missing-directory path. This is upstream-inherited behaviour adopted wholesale via the graft strategy (`project_upstream_strategy.md`). The fix — replacing the hardcoded path with a platform-neutral guaranteed-nonexistent child path derived from the `tmpdir` fixture — is intentionally deferred to a follow-up PR or upstream report, to avoid mixing bugfix + refactor intents and drifting the diff from the upstream baseline future syncs need. Do NOT re-flag the hardcoded `/tmp/nonexistent-dir-12345` path in this file as a blocking or actionable issue until the follow-up PR lands.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T05:36:22.128Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/session/compaction.ts:169-191
Timestamp: 2026-04-28T05:36:22.128Z
Learning: In `packages/opencode/src/session/compaction.ts` (Astro-Han/pawwork), the internal helper `splitTurn` intentionally returns a raw `Effect.gen(...)` rather than being wrapped with `Effect.fnUntraced`. Converting `splitTurn` (and similar plain-Effect.gen internal helpers in this file, e.g. `turns`, `completedCompactions`, `buildPrompt`) to `Effect.fnUntraced` is a repo-wide convention sweep deferred to a dedicated follow-up PR. The same sweep covers `tool/tool.ts` and other helpers flagged in the same sync. Do NOT re-flag the absence of `Effect.fnUntraced` on `splitTurn` or other internal helpers in `compaction.ts` during upstream-sync PRs; flag it only in the dedicated convention-sweep PR.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T14:46:41.449Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 302
File: packages/opencode/test/session/retry.test.ts:298-315
Timestamp: 2026-04-28T14:46:41.449Z
Learning: In Astro-Han/pawwork (PR `#302`, `packages/opencode/src/provider/error.ts` + `packages/opencode/test/session/retry.test.ts`), upstream anomalyco/opencode#24063 intentionally treats ALL OpenAI streaming `server_error` chunks as retryable (`isRetryable: true`). Do NOT request a test asserting `isRetryable: false` for a `server_error` stream chunk; the only negative regression needed is a non-`server_error` code (e.g. `bad_request`) to confirm the retry gate stays closed for other error codes.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T14:46:33.806Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 302
File: packages/opencode/src/plugin/codex.ts:0-0
Timestamp: 2026-04-28T14:46:33.806Z
Learning: In `packages/opencode/src/plugin/codex.ts` (Astro-Han/pawwork), the Provider SDK v1 hook `Model` type omits the `input` field on `limit`, but opencode uses `limit.input` for compaction budget. The pattern to avoid a bare `ts-expect-error` is to declare a local typed intersection `const limit: typeof model.limit & { input: number } = { context: ..., input: ..., output: ... }` and assign that to `model.limit`. Do not flag the intersection cast as unnecessary — it is the intentional way to type-safely add `input` without suppressing the whole assignment.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T07:28:14.317Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/lsp.ts:23-32
Timestamp: 2026-04-28T07:28:14.317Z
Learning: In `packages/opencode/src/tool/lsp.ts` (Astro-Han/pawwork, PR `#270`), the `Parameters` schema requires `line` and `character` for all operations, including `workspaceSymbol` and `documentSymbol` which never use coordinates. This matches the upstream `dev:packages/opencode/src/tool/lsp.ts:23-32` baseline exactly — both fields are declared as required `Schema.Number` with `>= 1` checks. The fix (per-operation schema split, or making `line`/`character` optional with handler-side presence validation for operations that need them like `goToDefinition`/`findReferences`) is deferred to a follow-up PR or upstream report to avoid mixing refactor + bug-fix intents and drifting the diff from the upstream baseline. Do NOT re-flag the required coordinates on `workspaceSymbol`/`documentSymbol` in upstream-sync PRs; flag it only in a PawWork-authored PR that directly touches `lsp.ts` parameter validation.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T04:56:13.350Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/tool.ts:8-10
Timestamp: 2026-04-28T04:56:13.350Z
Learning: In `packages/opencode/src/tool/tool.ts` (Astro-Han/pawwork, PR `#270`), the `Metadata` interface uses `[key: string]: any` as its index signature. This is upstream-inherited code adopted wholesale via the graft strategy (per `project_upstream_strategy.md`). The fix — tightening to `Record<string, unknown>` and explicitly narrowing framework-owned fields like `truncated` — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag the `any` index signature on `Metadata` in PR `#270` or in future upstream-sync PRs that carry the same upstream baseline.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T11:24:23.345Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:8-16
Timestamp: 2026-04-28T11:24:23.345Z
Learning: In Astro-Han/pawwork, the established pattern for service-internal errors (e.g., in `packages/opencode/src/session/subagent-run.ts`) is plain classes (with a `readonly _tag` discriminant) or `NamedError.create` (zod-based), NOT `Schema.TaggedErrorClass`. Converting individual modules to `Schema.TaggedErrorClass` one-off would diverge from the surrounding code. The migration to `Schema.TaggedErrorClass` for service-internal errors is intentionally deferred to a dedicated repo-wide error-class sweep PR. Do NOT flag plain error classes in `subagent-run.ts` or other service-internal modules as needing `Schema.TaggedErrorClass` conversion until that sweep PR lands.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T04:56:21.528Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/tool.ts:34-42
Timestamp: 2026-04-28T04:56:21.528Z
Learning: In `packages/opencode/src/tool/tool.ts` (Astro-Han/pawwork, PR `#270`), the `Def.execute` signature uses `ctx: Context` (without the `M` generic) instead of `ctx: Context<M>`, meaning `ctx.metadata(...)` is not checked against the same `M` as `ExecuteResult<M>`. This is inherited upstream behavior adopted via the graft strategy in PR `#270` (an upstream-sync PR per `project_upstream_strategy.md`). Fixing it here would mix refactor + bugfix intents and drift the diff from the upstream baseline. The fix is deferred to a dedicated follow-up PR or upstream report. Do NOT re-flag the missing `Context<M>` generic in `Def.execute` during upstream-sync PRs; only flag it in a PawWork-authored PR that directly touches `tool.ts` type signatures.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-27T10:33:12.228Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:12.228Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/prompt.ts` and `packages/opencode/src/session/processor.ts`, PR `#264`), the loop-gate race condition between `buildLoopContext()` and `recordSyntheticBlock`/`recordSyntheticStop` is intentionally handled via idempotence guards (re-check sigKey presence / `hasStopped` inside the record helpers) rather than a full per-parent `Effect.Mutex`. Threading a `Map<MessageID, Mutex>` through the processor was considered too large a surface change for this edge case; the residual TOCTOU window only produces extra synthetic parts with no behavioral drift on the "turn ends" contract. A code comment documents the trade-off and points to a full-mutex follow-up if the race is observed in practice. Do NOT re-flag the absence of a per-parent mutex as a blocking issue in future reviews.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T04:38:11.727Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/edit.ts:35-45
Timestamp: 2026-04-28T04:38:11.727Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork, PR `#270`), the module-scoped `Map<string, Semaphore.Semaphore>` (`locks`) and `lock(filePath)` helper are intentionally left as a global, never-cleaned registry. PR `#270` is a pure upstream-sync graft and mixing in a bugfix would drift the diff from the upstream baseline. The fix (moving per-file semaphores into `InstanceState`/`ScopedCache` for per-instance lifecycle management) is tracked as a follow-up to land in its own PR or be reported upstream. Do NOT re-flag the absence of InstanceState wiring for the lock map in this file until the follow-up PR is opened.
Applied to files:
packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-27T08:58:00.665Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:531-538
Timestamp: 2026-04-27T08:58:00.665Z
Learning: When using Effect (e.g., `yield*` with `Effect`-style generator yielding), only use `yield* new SomeErrorClass(...)` if `SomeErrorClass` extends `Schema.TaggedErrorClass` (i.e., it implements Effect’s Yieldable interface). For plain `Error` subclasses (like `BlockedLoopError` / `LoopStopError`) or inline `new Error(...)` values, they are not yieldable and must be wrapped as `yield* Effect.fail(new PlainError(...))`. Do not recommend changing `yield* Effect.fail(new SomePlainError(...))` to `yield* new SomePlainError(...)` unless the error class extends `Schema.TaggedErrorClass`.
Applied to files:
packages/opencode/src/provider/transform.ts
This was referenced Apr 28, 2026
This was referenced May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Syncs the provider compatibility follow-up slice from #298 PR 2. This preserves existing interleaved reasoning metadata on config overrides, adds DeepSeek V4 reasoning effort handling, preserves OpenRouter reasoning details, disables unsupported Anthropic SDK tool streaming paths, sanitizes Moonshot/Kimi tool schemas, and updates the OpenRouter SDK to 2.8.1.
Why
#298 tracks missed upstream runtime/provider fixes after PR #270 and PR #302. This PR handles the provider compatibility subset so DeepSeek and Kimi-style providers behave more reliably across OpenAI-compatible, OpenRouter, Moonshot, and Anthropic SDK-backed request paths.
Related Issue
Refs #298
How To Verify
bun install --frozen-lockfile bun --cwd packages/opencode test test/provider/transform.test.ts test/provider/provider.test.ts test/session/message-v2.test.tsScreenshots or Recordings
Not applicable. No visible UI changes.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
Dependency Updates
Bug Fixes
Tests