Skip to content

fix(upstream): permission config + provider/runtime fixes (PR5/5)#301

Merged
Astro-Han merged 11 commits into
devfrom
chore/reopen-pr272
Apr 28, 2026
Merged

fix(upstream): permission config + provider/runtime fixes (PR5/5)#301
Astro-Han merged 11 commits into
devfrom
chore/reopen-pr272

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Apr 28, 2026

Copy link
Copy Markdown
Owner

Replacement for #272, which was closed accidentally after its stacked base #270 had already merged. This PR reapplies only the four PR5 commits on top of current dev, so it excludes the already-merged PR4/tool-framework base changes.

Summary

Stage F + G + H + I of the upstream sync from anomalyco/opencode v1.14.20..17701628bd. Reopened on top of dev after #270 merged. 4 commits, 11 files, all PawWork carve-outs preserved.

Stages included

  • Stage F — Permission config (1 file)
    • #24222 fix permission config order (drop wildcard sort, preserve user key order)
    • #24235 IntelliSense for all tool permission keys via InfoZod override
  • Stage G.1 — Provider tail (8 files)
    • DeepSeek (#24146 #24157 #24163 #24180), OpenRouter sdk bump #24435, Codex #23925 (already incorporated in PawWork's helper), Mistral #23735, Kimi #23696, Vertex #24573, OpenAI #24063 #24212, reasoning transforms split #24574
  • Stage G.2 — Runtime (2 files)
    • #24215 shell cwd: pass cwd as positional arg ('$1') instead of capturing $PWD pre-init
    • #23770 tool_output config schema (max_lines / max_bytes)
    • #23797 BOM, #19054 bare-repo, #23870 compaction already in HEAD (no-op)
  • Stage I — SDK regen (1 file: types.gen.ts)
    • Reflects PermissionConfig intersection record + Config.tool_output

Stage G.3 + H — deferred to follow-up

  • G.3 (#24482 agent-create deny default): upstream rewrote agent.ts from tools-based to permissions-based selection; PawWork still uses tools-based path. Fix needs reapplication on PawWork's own structure.
  • H (#20602 shell selection): upstream rewrote shell.ts BLACKLIST/LOGIN/POSIX into META dict + heavy pty refactor; PawWork has structural divergence. Slice does not compose without porting full upstream architecture.

Both tracked in follow-up task to re-sync tool .ts files (descriptions stay carve-out, logic syncs upstream).

Carve-outs preserved

  • packages/opencode/src/permission/index.ts — kept on PawWork (legacy task→agent alias, list() endpoint)
  • packages/opencode/src/plugin/codex.ts — kept on PawWork (helper refactor, functionally equivalent)
  • packages/opencode/src/session/compaction.ts + agent/prompt/compaction.txt — kept on PawWork (anchored summary template diverged from upstream redesign)
  • packages/opencode/src/{format,patch}/index.ts, session/message-v2.ts, tool/{bash,write,edit,truncate,apply_patch}.ts — kept on PawWork structure (namespace wrappers, Zod params, divergent format.file API)
  • packages/opencode/package.json — kept PawWork workspace deps; only @openrouter bumped to 2.8.1
  • packages/app/src/components/settings-general.tsx + packages/app/src/i18n/en.ts + packages/web/src/content/docs/*.mdx — dropped per UI rewrite + web docs carve-out
  • Tests kept on HEAD where PawWork tests have diverged

Test plan

  • git diff --check origin/dev...HEAD
  • bun run --cwd packages/opencode typecheck
  • bun run --cwd packages/sdk/js typecheck
  • Reviewer to verify permission config IntelliSense in editor
  • Reviewer to verify shell cwd fix in interactive bash session

Refs #209
Closes #272

Summary by CodeRabbit

  • Improvements

    • Enhanced DeepSeek reasoning for v4+ (adds new effort levels including "max")
    • More accurate Mistral small-model reasoning detection and narrower exposure of effort variants
    • Provider-specific trimming of empty message content and adjusted tool/streaming behavior
    • Targeted enablement for Kimi K2.5 variants
  • Bug Fixes

    • Shell session wrapper now reliably changes directory and uses an updated invocation marker
    • Improved server-error handling to produce retryable API errors
  • Tests

    • Expanded tests for provider variants, reasoning behavior, and error scenarios
  • Chores

    • Test coverage and variant assertions updated

Sync from anomalyco/opencode v1.14.20..17701628bd:

* fix permission config order (#24222) — drop wildcard sort, preserve
  user key order so permission precedence matches the order users write
  in config (66f93035b0)
* core: full IntelliSense for all tool permission keys via InfoZod
  override (1b92c95425)

Carve-outs preserved:
- packages/opencode/src/permission/index.ts kept on PawWork (legacy
  task→agent alias, list() endpoint, refactored reply state machine)
- tests kept on HEAD (PawWork tests diverged after agent rename)
- packages/sdk/js + openapi.json kept on HEAD; will regenerate in
  Stage I

Refs anomalyco/opencode#24222 #24235
PawWork issue: #209 (PR5 of upstream sync)
DeepSeek (#24146 preserve empty reasoning_content, #24157 variants,
#24163 max support, #24180 ensure assistant reasoning), OpenRouter
sdk bump (#24435), Codex (#23925 model logic — already incorporated
into PawWork's shouldKeepCodexOAuthModel helper), Mistral Small
reasoning (#23735), Kimi variants (#23696), Google Vertex tool call
streaming default (#24573), OpenAI retry/oauth (#24063, #24212),
reasoning transforms split (#24574).

Carve-outs preserved:
- packages/opencode/src/plugin/codex.ts kept on PawWork (helper
  refactor of allowed model logic — upstream change is functionally
  equivalent)
- packages/opencode/package.json kept PawWork-specific workspace
  deps (@opencode-ai/core, @opencode-ai/util); only @openrouter
  bump applied

Refs anomalyco/opencode #23696 #23735 #23925 #24063 #24146 #24157
#24163 #24180 #24212 #24435 #24573 #24574
PawWork issue: #209
Functional changes from upstream batch (#23797 BOM, #23770 truncate
config, #23870 compaction, #19054 bare-repo, #24215 shell cwd):

* session/prompt.ts: shell invocations now pass cwd as positional arg
  ("$1") instead of capturing $PWD before sourcing shell init files
  (#24215)
* config/config.ts: add tool_output { max_lines, max_bytes } schema
  for user-configurable truncation thresholds (#23770)

Already incorporated in HEAD (no-op cherry-picks):
- BOM round-trip (Bom.syncFile already wired in edit/write/apply_patch)
- Project bare-repo cache (isBareRepo + readCachedProjectId(common)
  already in project.ts)

Deferred to Post-merge tool re-sync (#19):
- Truncate config wiring inside truncate.ts (namespace structure
  diverged; needs structural merge)
- Session compaction rewrite (#23870 SUMMARY_TEMPLATE/Turn/Tail
  redesign vs PawWork's anchored template)

Refs anomalyco/opencode #19054 #23770 #23797 #23870 #24215
PawWork issue: #209
Stage I regen reflects:
- PermissionConfig gains explicit tool keys with intersection record
  type (Stage F: #24222 + #24235 IntelliSense)
- Config.tool_output { max_lines, max_bytes } block added
  (Stage G.2: #23770 truncation thresholds)

Refs: #209
@Astro-Han Astro-Han added enhancement New feature or request P3 Low priority upstream Tracked upstream or vendor behavior labels Apr 28, 2026
@coderabbitai

coderabbitai Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ac1a335a-d88d-4398-a824-31fe7da1c6c3

📥 Commits

Reviewing files that changed from the base of the PR and between d325e27 and 0366c75.

📒 Files selected for processing (2)
  • packages/opencode/src/session/prompt.ts
  • packages/opencode/test/session/message-v2.test.ts
📜 Recent 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). (11)
  • GitHub Check: smoke-macos-arm64
  • GitHub Check: unit-windows-desktop
  • GitHub Check: unit-windows-opencode-session
  • GitHub Check: unit-windows-opencode-config-project
  • GitHub Check: unit-windows-opencode-server-tools
  • GitHub Check: unit-windows-app
  • GitHub Check: unit-desktop
  • GitHub Check: unit-opencode
  • GitHub Check: typecheck
  • GitHub Check: e2e-artifacts
  • GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts

📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)

packages/opencode/**/*.ts: Use Effect.gen(function* () { ... }) for Effect composition
Use Effect.fn("Domain.method") for named/traced effects and Effect.fnUntraced for internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer .pipe() wrappers
Use Effect.callback for callback-based APIs
Prefer DateTime.nowAsDate over new Date(yield* Clock.currentTimeMillis) when you need a Date in Effect code
Use Schema.Class for multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
Use Schema.TaggedErrorClass for typed errors in Effect schemas
Use Schema.Defect instead of unknown for defect-like causes in Effect code
In Effect.gen / Effect.fn, prefer yield* new MyError(...) over yield* Effect.fail(new MyError(...)) for direct early-failure branches
Use makeRuntime from src/effect/run-service.ts for all services; it returns { runPromise, runFork, runCallback } backed by a shared memoMap that deduplicates layers
Use InstanceState from src/effect/instance-state.ts for per-directory or per-project state that needs per-instance cleanup; do work directly in the InstanceState.make closure where ScopedCache handles run-once semantics
Use Effect.addFinalizer or Effect.acquireRelease inside the InstanceState.make closure for cleanup (subscriptions, process teardown, etc.)
Use Effect.forkScoped inside the InstanceState.make closure for background stream consumers — the fiber is interrupted when the instance is disposed
Prefer FileSystem.FileSystem instead of raw fs/promises for effectful file I/O in Effect services
Prefer ChildProcessSpawner.ChildProcessSpawner with ChildProcess.make(...) instead of custom process wrappers in Effect services
Prefer HttpClient.HttpClient instead of raw fetch in Effect services
Prefer Path.Path, Config, Clock, and DateTime services when those concerns are already inside Effect code
For backgroun...

Files:

  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/src/session/prompt.ts
packages/opencode/test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (packages/opencode/test/AGENTS.md)

packages/opencode/test/**/*.test.{ts,tsx}: Use the tmpdir function from fixture/fixture.ts to create temporary directories for tests with automatic cleanup. Use await using syntax to ensure automatic cleanup when the variable goes out of scope.
When using the tmpdir function with git repository support, pass the git: true option to initialize a git repo with a root commit.
Use the config option in tmpdir to write an opencode.json config file during test setup by passing a partial Config.Info object.
Use the init option in tmpdir to define custom setup functions that can return extra data accessible via tmp.extra, and use the dispose option for custom cleanup logic.
Use testEffect(...) from test/lib/effect.ts for tests that exercise Effect services or Effect-based workflows.
Use it.effect(...) when the test should run with TestClock and TestConsole. Use it.live(...) when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers from fixture/fixture.ts over building manual runtimes in tests: use tmpdirScoped() for scoped temp directories, provideInstance(dir)(effect) for low-level binding without directory creation, provideTmpdirInstance(...) for single temp instance binding, or provideTmpdirServer(...) for tests that also need the test LLM server.
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.
Avoid custom ManagedRuntime, attach(...), or ad hoc run(...) wrappers in Effect tests when testEffect(...) already provides the runtime.
When a test needs instance-local state, prefer provideTmpdirInstance(...) or provideInstance(...) over manual Instance.provide(...) inside Promise-style tests.

Files:

  • packages/opencode/test/session/message-v2.test.ts
🧠 Learnings (31)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 280
File: packages/opencode/src/config/config.ts:148-155
Timestamp: 2026-04-28T03:46:07.505Z
Learning: In `packages/opencode/src/config/config.ts` (Astro-Han/pawwork), `Config.Info` is an Effect Schema with a `.zod` accessor derived via `withStatics`. There are zero callers of `Config.Info.parse`, `Config.Info.safeParse`, `Config.parse`, or `Config.safeParse` in the codebase. The `.zod` accessor pattern (`Config.Info.zod`) is the documented PawWork convention for all Hono validators and Zod-style decoders. Do NOT suggest adding `.parse`/`.safeParse` compatibility shims to `Config.Info` — they would be dead surface area with no consumer.
📚 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/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.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-28T18:53:46.234Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 308
File: packages/opencode/test/server/global-session-list.test.ts:224-259
Timestamp: 2026-04-28T18:53:46.234Z
Learning: In `packages/opencode/test/server/global-session-list.test.ts` (Astro-Han/pawwork), all `it.live(...)` test bodies intentionally use `Effect.promise(async () => { ... })` rather than `Effect.gen(function* () { ... })`. This is the established local convention for the route/pagination tests in this file (e.g., "supports cursor pagination", "session route orders by creation time", "session routes omit undefined optional fields"). Do NOT flag individual `it.live` cases in this file as needing conversion to `Effect.gen`; any harness-style migration would need to cover the entire file.

Applied to files:

  • packages/opencode/test/session/message-v2.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/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/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.ts
📚 Learning: 2026-04-28T13:10:01.345Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/tool/agent.ts:89-102
Timestamp: 2026-04-28T13:10:01.345Z
Learning: In `packages/opencode/src/tool/agent.ts` (Astro-Han/pawwork, PR `#287`), `makeReadLastCompletedAssistantText` intentionally scans the full child session message history (no `limit`) to find the latest completed assistant text part for `partial_result` on cancellation. Cancellation is a rare path so the extra read cost is acceptable; tool-heavy children can push the last stable assistant text far back. Do NOT suggest re-adding a fixed small limit (e.g., `limit: 5`) to this function.

Applied to files:

  • packages/opencode/test/session/message-v2.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.ts
📚 Learning: 2026-04-28T04:38:21.935Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:21.935Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.

Applied to files:

  • packages/opencode/test/session/message-v2.test.ts
📚 Learning: 2026-04-28T05:36:24.561Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.

Applied to files:

  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/src/session/prompt.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-25T12:52:49.735Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/opencode/src/share/session.ts:27-27
Timestamp: 2026-04-25T12:52:49.735Z
Learning: In `packages/opencode/src/share/session.ts`, the local `ensureEnabled` closure is intentionally duplicated from `ShareRuntime.ensureEnabled` in `runtime.ts`. Using the shared `ShareRuntime.ensureEnabled` effect directly would leak `CloudShareGate` back into the `R` (requirement) type of `share`, `unshare`, and `create`, because the shared effect re-yields the service rather than capturing an already-resolved gate reference. The current pattern resolves `gate` once at the top of the outer `Effect.gen` and then closes over it in a local `ensureEnabled`, keeping the inner effects' requirement types clean. A comment in the file points to `runtime.ts` to make the intentional duplication discoverable. The real fix would be refactoring `ensureEnabled` to accept a gate parameter, but that change is larger than the DRY benefit warrants. Do not flag this duplication as a maintenance issue.

Applied to files:

  • packages/opencode/test/session/message-v2.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/session/message-v2.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/session/message-v2.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/session/message-v2.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/session/message-v2.test.ts
📚 Learning: 2026-04-24T03:51:56.211Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:56.211Z
Learning: In Astro-Han/pawwork E2E tests (packages/app/e2e/fixtures.ts), `project.prompt(text)` internally calls `trackSession(next.sessionID, active.directory)` after the prompt submission is observed and the active session is resolved. Tests that obtain a `sessionID` from `project.prompt()` do NOT need to call `project.trackSession(sessionID)` manually — it is already registered for teardown. Calling it again would be duplicate ownership.

Applied to files:

  • packages/opencode/src/session/prompt.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/session/prompt.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/session/prompt.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/session/prompt.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/session/prompt.ts
📚 Learning: 2026-04-28T07:27:59.666Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:364-368
Timestamp: 2026-04-28T07:27:59.666Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), both `files()` and `search()` do not short-circuit on an already-aborted `AbortSignal` before `spawner.spawn(...)`. The `check(input.cwd)` effect before spawn already provides a natural cancellation point for interrupted fibers. Upstream `dev:packages/opencode/src/file/ripgrep.ts` has identical ordering with no pre-spawn `signal.aborted` guard; adding one would diverge from the graft baseline. The fix (explicit pre-spawn `signal.aborted` check) is deferred to a dedicated PawWork ripgrep cancellation pass or upstream PR. Do NOT re-flag the missing pre-spawn cancellation short-circuit in `files()`/`search()` in upstream-sync PRs.

Applied to files:

  • packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-28T05:36:18.200Z
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.

Applied to files:

  • packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-24T05:39:58.329Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:58.329Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.

Applied to files:

  • packages/opencode/src/session/prompt.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/session/prompt.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/session/prompt.ts
📚 Learning: 2026-04-27T10:32:59.274Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/diagnostics.ts:245-252
Timestamp: 2026-04-27T10:32:59.274Z
Learning: In pawwork’s session diagnostics/loop error handling (e.g., packages/opencode/src/session/diagnostics.ts), keep `loopLastError` stored as raw/unscrubbed error text in loop metadata. The intended trust boundary for sensitive-data scrubbing is the renderer: `LoopRenderer.render` must apply `scrubErrorText` before any user/model-facing output. Do not recommend scrubbing `loopLastError` at the `observeToolError` call site, because failed tool parts already store raw errors in `state.error` (pre-existing leak) and scrubbing only `loopLastError` would be lossy without closing the gap. If export-side hardening is required, implement a single sanitizer pass in the export layer over both `state.error` and the loop/metadata fields, rather than sanitizing only `loopLastError` earlier in the storage path.

Applied to files:

  • packages/opencode/src/session/prompt.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 this repo’s service-internal code (e.g., session modules), follow the established error-class convention: use plain classes with a `readonly _tag` discriminant and/or `NamedError.create` (zod-based) for service-internal errors. Do NOT request one-off conversions of such plain/service-internal errors to `Schema.TaggedErrorClass`; that migration is intentionally deferred until the dedicated repo-wide error-class sweep PR lands.

Applied to files:

  • packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-27T10:33:08.974Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:08.974Z
Learning: For Astro-Han/pawwork session processing code under packages/opencode/src/session/ (e.g., prompt.ts and processor.ts), this specific loop-gate TOCTOU/race between buildLoopContext() and recordSyntheticBlock/recordSyntheticStop is intentionally handled via idempotence guards (re-check sigKey presence and hasStopped inside the record helpers). Do not treat the absence of a per-parent Effect.Mutex as a blocking review issue for this known edge case; the residual window should only create extra synthetic parts and must not change the "turn ends" contract. Respect the existing code comment documenting the trade-off, and only escalate to a full-mutex approach if the race is observed in practice.

Applied to files:

  • packages/opencode/src/session/prompt.ts
🔇 Additional comments (4)
packages/opencode/test/session/message-v2.test.ts (2)

857-910: Good boundary coverage for step-start handling.

These two tests correctly lock in the split-vs-drop behavior and reduce regression risk around assistant message chunking.


1059-1082: Nice regression test for OpenAI stream server_error retry semantics.

The expectations are precise and aligned with current MessageV2.fromError + provider-error parsing behavior.

packages/opencode/src/session/prompt.ts (2)

971-972: Good fail-closed shell wrapper behavior.

The cd -- "$OPENCODE_SHELL_CWD" || exit $? guard before eval is the right safety behavior, and the wrapper arg rename is applied consistently in both zsh and bash branches.

Also applies to: 975-975, 985-986, 989-989


1007-1011: Env scoping change looks correct.

Conditionally adding OPENCODE_SHELL_CWD only for bash/zsh avoids leaking an internal variable to unrelated shell branches while preserving required behavior for the wrapper paths.

Also applies to: 1016-1016


📝 Walkthrough

Walkthrough

Adds an explicit Zod InfoZod for permission config and enforces normalization; updates provider message transforms (DeepSeek, Mistral, Kimi, Anthropic/Bedrock, OpenAI-compatible) including empty-content filtering and reasoning-effort gating; changes shell wrappers to pass cwd via OPENCODE_SHELL_CWD; and expands related tests.

Changes

Cohort / File(s) Summary
Permission Config Schema
packages/opencode/src/config/permission.ts
Add InfoZod matching supported permission shapes, apply transform(normalizeInput), export Info with ZodOverride, and adjust Zod imports.
Provider Transform Logic
packages/opencode/src/provider/transform.ts
Add filterEmptyMessageContent, DeepSeek preprocessing (inject empty assistant reasoning part), replace isDeepSeekV4ID with deepseekMajorVersion, add DeepSeek v4+ max effort, restrict Mistral reasoning to supported small models and require model reasoning capability, and provider-specific content filtering for Anthropic/Bedrock.
Shell Execution Runtime
packages/opencode/src/session/prompt.ts
Switch shell wrappers to read target dir from OPENCODE_SHELL_CWD/positional arg, cd with quoted env value and exit on failure, unset env var after cd, and change wrapper marker from pawworkopencode; spawn env includes OPENCODE_SHELL_CWD for zsh/bash.
Provider Transform Tests
packages/opencode/test/provider/transform.test.ts
Add options tests (e.g., disable toolStreaming for Vertex Anthropic; Kimi “thinking” only for K2.5 with fixed budget), tighten Mistral variant expectations, and extend DeepSeek variant coverage (v3 excluded; v4+ includes max).
Message & Retry Tests
packages/opencode/test/session/message-v2.test.ts, packages/opencode/test/session/retry.test.ts
Reorder two toModelMessages tests (outputs unchanged); add MessageV2.fromError test for OpenAI-style server_error producing retryable APIError with original message and full JSON as responseBody; augment mocked chunk fields in retry test.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

Possibly related PRs

Suggested labels

bug, P1, harness

Poem

🐇
I hopped through Zod fields, neat and spry,
Poked in reasoning parts for DeepSeek to try,
Whispered cwd to shells, then watched them run,
Tuned Mistral’s voice till checks were done,
Tests sing, I nibble—review and bunny-hop, done!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main change: permission config and provider/runtime fixes for PR5/5 of the upstream sync, which matches the changeset across 11 files.
Description check ✅ Passed The PR description covers all required template sections: Summary (stage details), Why (upstream sync reopening context), Related Issue (#272, #270, #209), How To Verify (typecheck commands with completion status), Checklist items addressed. Some checklist items correctly marked incomplete for reviewer action (IntelliSense, shell cwd verification).
Linked Issues check ✅ Passed The PR successfully implements all four coding objectives from issue #272: Stage F permission config (order/IntelliSense), Stage G.1 provider updates (DeepSeek/OpenRouter/Mistral/Kimi/Vertex/OpenAI), Stage G.2 runtime fixes (shell cwd, tool_output schema), and Stage I SDK regeneration. Deferred stages (G.3, H) are appropriately tracked separately.
Out of Scope Changes check ✅ Passed All code changes remain in scope: permission config (1 file), provider transform updates (1 file), session prompt shell fixes (1 file), provider/session tests (3 files), and SDK regen. No unrelated refactors, dependency changes beyond @openrouter bump, or carve-out modifications introduced.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/reopen-pr272

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the @openrouter/ai-sdk-provider dependency and introduces several enhancements to model handling, including context window limits for gpt-5.5, improved OpenAI error parsing, and refined message normalization for various providers. It also implements a Zod-based permission schema to maintain key order, adds configuration for tool output truncation thresholds, and refactors shell command execution for better reliability. Review feedback suggests simplifying the Zod permission schema by removing a redundant intersection and adopting semantic versioning for Deepseek model identification to ensure future compatibility.

Comment thread packages/opencode/src/config/permission.ts
Comment thread packages/opencode/src/provider/transform.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/config/permission.ts`:
- Around line 67-84: InfoZod's explicit object schema is missing the canonical
"agent" key which breaks IntelliSense and the rename contract; add an "agent"
entry to the object with the same type as similar entries (e.g., agent:
zod(Rule).optional()) so it appears alongside task, lsp, and other Rule keys,
updating the InfoZod schema in permission.ts to include that property.

In `@packages/opencode/src/provider/transform.ts`:
- Around line 755-764: The switch case for "@ai-sdk/mistral" declares const
mistralId without block scoping which trips the noSwitchDeclarations rule; wrap
the entire case body (the lines checking model.capabilities.reasoning, computing
const mistralId, the id checks for "mistral-small-2603"/"mistral-small-latest",
and the return of high: { reasoningEffort: "high" }) in a pair of braces { ... }
so mistralId and any other declarations are block-scoped to that case.
- Around line 574-579: The switch case for "@ai-sdk/openai-compatible" declares
const efforts in the shared switch scope — wrap the entire case body in a block
(add { ... } after the case) so efforts is block-scoped; inside the block keep
the existing logic that clones WIDELY_SUPPORTED_EFFORTS, conditionally pushes
"max" when model.api.id.includes("deepseek-v4"), and returns
Object.fromEntries(efforts.map(effort => [effort, { reasoningEffort: effort
}])); apply the same fix to the "@ai-sdk/groq" case (where groqEffort is
declared) so groqEffort is declared inside its own block.
🪄 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: e8d21062-e2df-4f53-ae0a-90c704eaee40

📥 Commits

Reviewing files that changed from the base of the PR and between 6461f6c and 86b7910.

⛔ Files ignored due to path filters (2)
  • bun.lock is excluded by !**/*.lock
  • packages/sdk/js/src/v2/gen/types.gen.ts is excluded by !**/gen/**
📒 Files selected for processing (9)
  • packages/opencode/package.json
  • packages/opencode/src/config/permission.ts
  • packages/opencode/src/plugin/codex.ts
  • packages/opencode/src/provider/error.ts
  • packages/opencode/src/provider/transform.ts
  • packages/opencode/src/session/prompt.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/test/session/retry.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). (7)
  • GitHub Check: unit-windows-desktop
  • GitHub Check: unit-windows-opencode-config-project
  • GitHub Check: typecheck
  • GitHub Check: unit-opencode
  • GitHub Check: smoke-macos-arm64
  • GitHub Check: e2e-artifacts
  • GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts

📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)

packages/opencode/**/*.ts: Use Effect.gen(function* () { ... }) for Effect composition
Use Effect.fn("Domain.method") for named/traced effects and Effect.fnUntraced for internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer .pipe() wrappers
Use Effect.callback for callback-based APIs
Prefer DateTime.nowAsDate over new Date(yield* Clock.currentTimeMillis) when you need a Date in Effect code
Use Schema.Class for multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
Use Schema.TaggedErrorClass for typed errors in Effect schemas
Use Schema.Defect instead of unknown for defect-like causes in Effect code
In Effect.gen / Effect.fn, prefer yield* new MyError(...) over yield* Effect.fail(new MyError(...)) for direct early-failure branches
Use makeRuntime from src/effect/run-service.ts for all services; it returns { runPromise, runFork, runCallback } backed by a shared memoMap that deduplicates layers
Use InstanceState from src/effect/instance-state.ts for per-directory or per-project state that needs per-instance cleanup; do work directly in the InstanceState.make closure where ScopedCache handles run-once semantics
Use Effect.addFinalizer or Effect.acquireRelease inside the InstanceState.make closure for cleanup (subscriptions, process teardown, etc.)
Use Effect.forkScoped inside the InstanceState.make closure for background stream consumers — the fiber is interrupted when the instance is disposed
Prefer FileSystem.FileSystem instead of raw fs/promises for effectful file I/O in Effect services
Prefer ChildProcessSpawner.ChildProcessSpawner with ChildProcess.make(...) instead of custom process wrappers in Effect services
Prefer HttpClient.HttpClient instead of raw fetch in Effect services
Prefer Path.Path, Config, Clock, and DateTime services when those concerns are already inside Effect code
For backgroun...

Files:

  • packages/opencode/src/plugin/codex.ts
  • packages/opencode/test/session/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/opencode/src/provider/error.ts
  • packages/opencode/src/session/prompt.ts
  • packages/opencode/src/config/permission.ts
  • packages/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 the tmpdir function from fixture/fixture.ts to create temporary directories for tests with automatic cleanup. Use await using syntax to ensure automatic cleanup when the variable goes out of scope.
When using the tmpdir function with git repository support, pass the git: true option to initialize a git repo with a root commit.
Use the config option in tmpdir to write an opencode.json config file during test setup by passing a partial Config.Info object.
Use the init option in tmpdir to define custom setup functions that can return extra data accessible via tmp.extra, and use the dispose option for custom cleanup logic.
Use testEffect(...) from test/lib/effect.ts for tests that exercise Effect services or Effect-based workflows.
Use it.effect(...) when the test should run with TestClock and TestConsole. Use it.live(...) when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers from fixture/fixture.ts over building manual runtimes in tests: use tmpdirScoped() for scoped temp directories, provideInstance(dir)(effect) for low-level binding without directory creation, provideTmpdirInstance(...) for single temp instance binding, or provideTmpdirServer(...) for tests that also need the test LLM server.
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.
Avoid custom ManagedRuntime, attach(...), or ad hoc run(...) wrappers in Effect tests when testEffect(...) already provides the runtime.
When a test needs instance-local state, prefer provideTmpdirInstance(...) or provideInstance(...) over manual Instance.provide(...) inside Promise-style tests.

Files:

  • packages/opencode/test/session/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/test/provider/transform.test.ts
🧠 Learnings (35)
📓 Common learnings
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/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/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/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.
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/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.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/edit.ts:48-48
Timestamp: 2026-04-28T08:14:31.436Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork), the `filePath` schema description (`"The absolute path to the file to modify"`) is upstream-inherited from `dev:packages/opencode/src/tool/edit.ts:48`. The runtime actually accepts relative paths (resolved via `Instance.directory` at lines 79-81), but the description fix is intentionally deferred to a single PawWork-authored description-cleanup PR that will also cover the identical mismatch in `packages/opencode/src/tool/write.ts:24`. Do NOT re-flag the too-narrow `filePath` description in upstream-sync PRs; flag it only in the dedicated description-cleanup PR.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/todo.ts:9-18
Timestamp: 2026-04-28T06:51:54.812Z
Learning: In `packages/opencode/src/tool/todo.ts` (Astro-Han/pawwork), `TodoItem.status` and `TodoItem.priority` are intentionally declared as plain `Schema.String` rather than closed literal unions. This matches the upstream opencode baseline (`dev:packages/opencode/src/tool/todo.ts`). The tightening — `Schema.Literals(["pending","in_progress","completed","cancelled"])` for `status` and `Schema.Literals(["high","medium","low"])` for `priority` — is tracked as a follow-up under the harness/tool-set-v1 series (issue `#129`) to land either as part of a tool-schema tightening sweep or upstream-first. Do NOT re-flag the free-form strings for these fields in upstream-sync PRs; flag it only in a PawWork-authored PR or the dedicated sweep that touches `TodoItem` schema.
📚 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/plugin/codex.ts
  • packages/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/plugin/codex.ts
  • 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/plugin/codex.ts
  • packages/opencode/src/provider/error.ts
  • packages/opencode/src/session/prompt.ts
  • packages/opencode/src/config/permission.ts
  • 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/plugin/codex.ts
  • packages/opencode/src/provider/error.ts
  • packages/opencode/src/session/prompt.ts
  • packages/opencode/src/config/permission.ts
  • packages/opencode/src/provider/transform.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/retry.test.ts
  • packages/opencode/test/session/message-v2.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/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
📚 Learning: 2026-04-25T12:52:35.631Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/desktop-electron/src/main/ipc.ts:238-263
Timestamp: 2026-04-25T12:52:35.631Z
Learning: In Astro-Han/pawwork (`packages/desktop-electron/src/main/ipc.ts`), `deps.getServerReadyData()` (backed by `serverReady.promise` in `index.ts`) resolves once at server startup and remains settled; it is not expected to reject in practice. Do not flag the absence of a try-catch around it in the `export-session` IPC handler — the network/fetch layer in `server-client.ts` already has a 10-second AbortController timeout and returns a typed `{ok: false, error}` payload, covering the real failure modes.

Applied to files:

  • packages/opencode/test/session/retry.test.ts
📚 Learning: 2026-04-21T12:14:30.524Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 93
File: packages/opencode/test/server/cors-middleware.test.ts:15-23
Timestamp: 2026-04-21T12:14:30.524Z
Learning: In `packages/opencode/test/server/cors-middleware.test.ts` (and similar Hono-based test files), `app.request(...)` on the Hono `Hono` app type returns `Promise<Response> | Response` — a union that includes a non-PromiseLike branch. `Effect.promise(...)` requires a `PromiseLike`, so wrapping with `Promise.resolve(app.request(...))` is necessary to normalize the union type. Removing the wrapper causes a TS2322 typecheck error in `packages/opencode`.

Applied to files:

  • packages/opencode/test/session/retry.test.ts
  • packages/opencode/test/session/message-v2.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/session/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/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/session/message-v2.test.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/opencode/src/provider/transform.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.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.ts
  • packages/opencode/test/provider/transform.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/session/message-v2.test.ts
  • packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-28T05:36:24.561Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.

Applied to files:

  • packages/opencode/test/session/message-v2.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-28T04:38:21.935Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:21.935Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.

Applied to files:

  • packages/opencode/test/session/message-v2.test.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-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-28T07:27:49.810Z
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.

Applied to files:

  • packages/opencode/src/provider/error.ts
📚 Learning: 2026-04-24T03:51:56.211Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:56.211Z
Learning: In Astro-Han/pawwork E2E tests (packages/app/e2e/fixtures.ts), `project.prompt(text)` internally calls `trackSession(next.sessionID, active.directory)` after the prompt submission is observed and the active session is resolved. Tests that obtain a `sessionID` from `project.prompt()` do NOT need to call `project.trackSession(sessionID)` manually — it is already registered for teardown. Calling it again would be duplicate ownership.

Applied to files:

  • packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-27T10:32:59.274Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/diagnostics.ts:245-252
Timestamp: 2026-04-27T10:32:59.274Z
Learning: In pawwork’s session diagnostics/loop error handling (e.g., packages/opencode/src/session/diagnostics.ts), keep `loopLastError` stored as raw/unscrubbed error text in loop metadata. The intended trust boundary for sensitive-data scrubbing is the renderer: `LoopRenderer.render` must apply `scrubErrorText` before any user/model-facing output. Do not recommend scrubbing `loopLastError` at the `observeToolError` call site, because failed tool parts already store raw errors in `state.error` (pre-existing leak) and scrubbing only `loopLastError` would be lossy without closing the gap. If export-side hardening is required, implement a single sanitizer pass in the export layer over both `state.error` and the loop/metadata fields, rather than sanitizing only `loopLastError` earlier in the storage path.

Applied to files:

  • packages/opencode/src/session/prompt.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 this repo’s service-internal code (e.g., session modules), follow the established error-class convention: use plain classes with a `readonly _tag` discriminant and/or `NamedError.create` (zod-based) for service-internal errors. Do NOT request one-off conversions of such plain/service-internal errors to `Schema.TaggedErrorClass`; that migration is intentionally deferred until the dedicated repo-wide error-class sweep PR lands.

Applied to files:

  • packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-27T10:33:08.974Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:08.974Z
Learning: For Astro-Han/pawwork session processing code under packages/opencode/src/session/ (e.g., prompt.ts and processor.ts), this specific loop-gate TOCTOU/race between buildLoopContext() and recordSyntheticBlock/recordSyntheticStop is intentionally handled via idempotence guards (re-check sigKey presence and hasStopped inside the record helpers). Do not treat the absence of a per-parent Effect.Mutex as a blocking review issue for this known edge case; the residual window should only create extra synthetic parts and must not change the "turn ends" contract. Respect the existing code comment documenting the trade-off, and only escalate to a full-mutex approach if the race is observed in practice.

Applied to files:

  • packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-28T03:46:07.505Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 280
File: packages/opencode/src/config/config.ts:148-155
Timestamp: 2026-04-28T03:46:07.505Z
Learning: In `packages/opencode/src/config/config.ts` (Astro-Han/pawwork), `Config.Info` is an Effect Schema with a `.zod` accessor derived via `withStatics`. There are zero callers of `Config.Info.parse`, `Config.Info.safeParse`, `Config.parse`, or `Config.safeParse` in the codebase. The `.zod` accessor pattern (`Config.Info.zod`) is the documented PawWork convention for all Hono validators and Zod-style decoders. Do NOT suggest adding `.parse`/`.safeParse` compatibility shims to `Config.Info` — they would be dead surface area with no consumer.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-28T02:24:27.738Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 280
File: packages/opencode/src/tool/todo.ts:7-22
Timestamp: 2026-04-28T02:24:27.738Z
Learning: In `packages/opencode/src/tool/todo.ts` (Astro-Han/pawwork, PR `#280`), the `parameters` constant for `TodoWriteTool` intentionally duplicates the `Todo.Info` fields inline rather than referencing `Todo.Info` or `zodObject(Todo.Info)`. After the Effect Schema migration in this PR, `Todo.Info` is an Effect Schema, so `zodObject(Todo.Info)` returns `z.ZodObject<any>` — which erases per-field types that `Tool.define`'s `execute()` inference depends on. The inline Zod schema is a deliberate temporary measure; tool schemas will migrate to Effect Schema as a separate slice per `specs/effect/schema.md`. Do NOT suggest replacing the inline schema with `z.array(Todo.Info)` or `zodObject(Todo.Info)` until that migration lands.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-21T16:57:25.580Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 102
File: packages/opencode/src/config/agent.ts:108-119
Timestamp: 2026-04-21T16:57:25.580Z
Learning: In `packages/opencode/src/config/agent.ts` (Astro-Han/pawwork), `ConfigPermission.Info` only accepts permission objects or the three action strings `"ask"`, `"allow"`, `"deny"`, and transforms those action strings into `{ "*": action }` before `normalize()` runs. By the time `normalize()` is reached, `configuredPermission` is always either `undefined` or a `Record<string, Rule>` — never a raw arbitrary string. The `Object.assign(permission, configuredPermission)` pattern is therefore safe. Do not flag it as corrupting string permission references.

Applied to files:

  • packages/opencode/src/config/permission.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/src/config/permission.ts
📚 Learning: 2026-04-28T01:24:29.645Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 280
File: packages/opencode/src/session/projectors.ts:0-0
Timestamp: 2026-04-28T01:24:29.645Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/projectors.ts`), `SyncEvent.Definition.schema` preserves the literal Zod type through `z.infer<Def["schema"]>`, so `data.info` inside a `SyncEvent.project(Session.Event.Created, ...)` callback is already typed as `Session.Info` by the TypeScript compiler. No `as Session.Info` cast or `Session.Info.parse()` call is needed at this persistence boundary — a typecheck-clean removal of any such cast is sufficient.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-20T14:36:21.288Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.288Z
Learning: Applies to packages/opencode/**/*.ts : Use branded schemas (`Schema.brand`) for single-value types in Effect

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-20T14:36:21.288Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.288Z
Learning: Applies to packages/opencode/**/*.ts : Use `Schema.Class` for multi-field data in Effect schemas

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-20T14:36:21.288Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.288Z
Learning: Applies to packages/opencode/**/*.ts : Use `Schema.Defect` instead of `unknown` for defect-like causes in Effect code

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-28T07:27:41.971Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:29-118
Timestamp: 2026-04-28T07:27:41.971Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork), the internal Effect schemas `TimeStats`, `Stats`, `PathText`, `Begin`, `End`, `Summary`, and the exported `SearchMatch`/`Match`/`Result` union are all declared as `Schema.Struct` — matching upstream `dev:packages/opencode/src/file/ripgrep.ts` lines 60-118. PR `#270` is an upstream-sync graft per `project_upstream_strategy.md`. Converting these to `Schema.Class` would diverge from the upstream baseline and create recurring merge friction on future syncs. The `Schema.Class` convention applies to PawWork-authored schemas, not to upstream-grafted shapes in this file. Do NOT re-flag the use of `Schema.Struct` for these schemas in upstream-sync PRs; flag it only in a PR that simultaneously converts the upstream side, or in a dedicated PawWork carve-out PR for ripgrep schemas.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-21T16:57:20.257Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 102
File: packages/opencode/src/config/command.ts:23-29
Timestamp: 2026-04-21T16:57:20.257Z
Learning: In packages/opencode/src/config/ config modules, treat both `Schema.Struct` and `Schema.Class` as valid schema patterns and do not require converting `Schema.Struct` to `Schema.Class`. If `Schema.Struct` is used in a config schema (e.g., in files like `command.ts`, `formatter.ts`, `provider.ts`, `permission.ts`, `lsp.ts`, `config.ts`, `keybinds.ts`), accept it as intentional; the selection should be based on upstream alignment and the local schema context, not on a rule that one form must replace the other.

Applied to files:

  • packages/opencode/src/config/permission.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] 575-575: 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)


[error] 760-760: 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 (4)
packages/opencode/src/plugin/codex.ts (1)

453-461: Scoped gpt-5.5 limit override looks correct.

The override is cleanly gated and only applies to the intended model family, so it won’t affect unrelated Codex models.

packages/opencode/src/session/prompt.ts (1)

948-977: CWD handoff fix is correctly applied for interactive shells.

Passing cwd as $1 and cd -- "$1" makes the execution directory explicit and avoids stale/pre-init working-directory capture.

packages/opencode/test/provider/transform.test.ts (1)

2116-2161: Mistral reasoning-variant coverage is well scoped.

These additions clearly lock in the intended split between supported small-model reasoning variants and unsupported large-model behavior.

packages/opencode/src/provider/error.ts (1)

126-161: No changes needed. The server_error case correctly matches body?.error?.code === "server_error", and test fixtures confirm OpenAI payloads include both type and code fields, with .code being the discriminator used in the switch statement. Implementation is correct and well-tested.

Comment thread packages/opencode/src/config/permission.ts Outdated
Comment thread packages/opencode/src/provider/transform.ts Outdated
Comment thread packages/opencode/src/provider/transform.ts Outdated
@Astro-Han Astro-Han changed the title sync(upstream): permission config + provider/runtime fixes (PR5/5) fix(upstream): permission config + provider/runtime fixes (PR5/5) Apr 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 55-73: Extract the duplicated empty-content filtering into a
shared helper (e.g., isNonEmptyMessage or filterEmptyMessages) that accepts a
ModelMessage and returns a boolean or maps messages to possibly undefined; move
the logic that checks typeof msg.content === "string" (exclude empty ""),
handles Array.isArray(msg.content) by filtering parts where part.type is "text"
or "reasoning" && part.text !== "" and drops messages with filtered.length ===
0, and otherwise returns the original msg; then replace both provider-specific
blocks (the branches checking model.api.npm === "@ai-sdk/anthropic" and the
other identical branch) to call this helper and preserve the existing type
guard/filter signature (keep the final .filter((msg): msg is ModelMessage =>
...)) so TypeScript types remain correct.

In `@packages/opencode/test/provider/transform.test.ts`:
- Around line 2611-2643: The test only checks that
ProviderTransform.variants(v4) and ProviderTransform.variants(v5) contain keys
but not that the "max" variant has the correct payload shape; update the test to
assert the structure/content of the "max" variant for v4 and v5 by retrieving
ProviderTransform.variants(v4)["max"] and ProviderTransform.variants(v5)["max"]
and asserting expected fields/types (e.g., presence of prompt/template, model id
or engine, and any expected numeric or string parameters) so regressions where
"max" exists but maps to an incorrect payload are caught; keep the existing
key-presence checks and add these payload-shape assertions referencing
ProviderTransform.variants, v4, v5, and the "max" variant.
🪄 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: ea353868-a1e5-4709-b2aa-b2339249d9a7

📥 Commits

Reviewing files that changed from the base of the PR and between 86b7910 and fb5bfee.

📒 Files selected for processing (3)
  • packages/opencode/src/config/permission.ts
  • packages/opencode/src/provider/transform.ts
  • packages/opencode/test/provider/transform.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). (11)
  • GitHub Check: unit-app
  • GitHub Check: unit-windows-opencode-server-tools
  • GitHub Check: unit-windows-opencode-session
  • GitHub Check: unit-windows-opencode-config-project
  • GitHub Check: unit-windows-desktop
  • GitHub Check: unit-windows-app
  • GitHub Check: unit-desktop
  • GitHub Check: typecheck
  • GitHub Check: smoke-macos-arm64
  • GitHub Check: e2e-artifacts
  • GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts

📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)

packages/opencode/**/*.ts: Use Effect.gen(function* () { ... }) for Effect composition
Use Effect.fn("Domain.method") for named/traced effects and Effect.fnUntraced for internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer .pipe() wrappers
Use Effect.callback for callback-based APIs
Prefer DateTime.nowAsDate over new Date(yield* Clock.currentTimeMillis) when you need a Date in Effect code
Use Schema.Class for multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
Use Schema.TaggedErrorClass for typed errors in Effect schemas
Use Schema.Defect instead of unknown for defect-like causes in Effect code
In Effect.gen / Effect.fn, prefer yield* new MyError(...) over yield* Effect.fail(new MyError(...)) for direct early-failure branches
Use makeRuntime from src/effect/run-service.ts for all services; it returns { runPromise, runFork, runCallback } backed by a shared memoMap that deduplicates layers
Use InstanceState from src/effect/instance-state.ts for per-directory or per-project state that needs per-instance cleanup; do work directly in the InstanceState.make closure where ScopedCache handles run-once semantics
Use Effect.addFinalizer or Effect.acquireRelease inside the InstanceState.make closure for cleanup (subscriptions, process teardown, etc.)
Use Effect.forkScoped inside the InstanceState.make closure for background stream consumers — the fiber is interrupted when the instance is disposed
Prefer FileSystem.FileSystem instead of raw fs/promises for effectful file I/O in Effect services
Prefer ChildProcessSpawner.ChildProcessSpawner with ChildProcess.make(...) instead of custom process wrappers in Effect services
Prefer HttpClient.HttpClient instead of raw fetch in Effect services
Prefer Path.Path, Config, Clock, and DateTime services when those concerns are already inside Effect code
For backgroun...

Files:

  • packages/opencode/src/config/permission.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/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 the tmpdir function from fixture/fixture.ts to create temporary directories for tests with automatic cleanup. Use await using syntax to ensure automatic cleanup when the variable goes out of scope.
When using the tmpdir function with git repository support, pass the git: true option to initialize a git repo with a root commit.
Use the config option in tmpdir to write an opencode.json config file during test setup by passing a partial Config.Info object.
Use the init option in tmpdir to define custom setup functions that can return extra data accessible via tmp.extra, and use the dispose option for custom cleanup logic.
Use testEffect(...) from test/lib/effect.ts for tests that exercise Effect services or Effect-based workflows.
Use it.effect(...) when the test should run with TestClock and TestConsole. Use it.live(...) when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers from fixture/fixture.ts over building manual runtimes in tests: use tmpdirScoped() for scoped temp directories, provideInstance(dir)(effect) for low-level binding without directory creation, provideTmpdirInstance(...) for single temp instance binding, or provideTmpdirServer(...) for tests that also need the test LLM server.
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.
Avoid custom ManagedRuntime, attach(...), or ad hoc run(...) wrappers in Effect tests when testEffect(...) already provides the runtime.
When a test needs instance-local state, prefer provideTmpdirInstance(...) or provideInstance(...) over manual Instance.provide(...) inside Promise-style tests.

Files:

  • packages/opencode/test/provider/transform.test.ts
🧠 Learnings (39)
📓 Common learnings
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/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.
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/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/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/tool/edit.ts:48-48
Timestamp: 2026-04-28T08:14:31.436Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork), the `filePath` schema description (`"The absolute path to the file to modify"`) is upstream-inherited from `dev:packages/opencode/src/tool/edit.ts:48`. The runtime actually accepts relative paths (resolved via `Instance.directory` at lines 79-81), but the description fix is intentionally deferred to a single PawWork-authored description-cleanup PR that will also cover the identical mismatch in `packages/opencode/src/tool/write.ts:24`. Do NOT re-flag the too-narrow `filePath` description in upstream-sync PRs; flag it only in the dedicated description-cleanup PR.
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.
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.
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.
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.
📚 Learning: 2026-04-21T16:57:25.580Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 102
File: packages/opencode/src/config/agent.ts:108-119
Timestamp: 2026-04-21T16:57:25.580Z
Learning: In `packages/opencode/src/config/agent.ts` (Astro-Han/pawwork), `ConfigPermission.Info` only accepts permission objects or the three action strings `"ask"`, `"allow"`, `"deny"`, and transforms those action strings into `{ "*": action }` before `normalize()` runs. By the time `normalize()` is reached, `configuredPermission` is always either `undefined` or a `Record<string, Rule>` — never a raw arbitrary string. The `Object.assign(permission, configuredPermission)` pattern is therefore safe. Do not flag it as corrupting string permission references.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-28T02:24:27.738Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 280
File: packages/opencode/src/tool/todo.ts:7-22
Timestamp: 2026-04-28T02:24:27.738Z
Learning: In `packages/opencode/src/tool/todo.ts` (Astro-Han/pawwork, PR `#280`), the `parameters` constant for `TodoWriteTool` intentionally duplicates the `Todo.Info` fields inline rather than referencing `Todo.Info` or `zodObject(Todo.Info)`. After the Effect Schema migration in this PR, `Todo.Info` is an Effect Schema, so `zodObject(Todo.Info)` returns `z.ZodObject<any>` — which erases per-field types that `Tool.define`'s `execute()` inference depends on. The inline Zod schema is a deliberate temporary measure; tool schemas will migrate to Effect Schema as a separate slice per `specs/effect/schema.md`. Do NOT suggest replacing the inline schema with `z.array(Todo.Info)` or `zodObject(Todo.Info)` until that migration lands.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-28T03:46:07.505Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 280
File: packages/opencode/src/config/config.ts:148-155
Timestamp: 2026-04-28T03:46:07.505Z
Learning: In `packages/opencode/src/config/config.ts` (Astro-Han/pawwork), `Config.Info` is an Effect Schema with a `.zod` accessor derived via `withStatics`. There are zero callers of `Config.Info.parse`, `Config.Info.safeParse`, `Config.parse`, or `Config.safeParse` in the codebase. The `.zod` accessor pattern (`Config.Info.zod`) is the documented PawWork convention for all Hono validators and Zod-style decoders. Do NOT suggest adding `.parse`/`.safeParse` compatibility shims to `Config.Info` — they would be dead surface area with no consumer.

Applied to files:

  • packages/opencode/src/config/permission.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/src/config/permission.ts
📚 Learning: 2026-04-28T01:24:29.645Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 280
File: packages/opencode/src/session/projectors.ts:0-0
Timestamp: 2026-04-28T01:24:29.645Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/projectors.ts`), `SyncEvent.Definition.schema` preserves the literal Zod type through `z.infer<Def["schema"]>`, so `data.info` inside a `SyncEvent.project(Session.Event.Created, ...)` callback is already typed as `Session.Info` by the TypeScript compiler. No `as Session.Info` cast or `Session.Info.parse()` call is needed at this persistence boundary — a typecheck-clean removal of any such cast is sufficient.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-28T06:51:54.812Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/todo.ts:9-18
Timestamp: 2026-04-28T06:51:54.812Z
Learning: In `packages/opencode/src/tool/todo.ts` (Astro-Han/pawwork), `TodoItem.status` and `TodoItem.priority` are intentionally declared as plain `Schema.String` rather than closed literal unions. This matches the upstream opencode baseline (`dev:packages/opencode/src/tool/todo.ts`). The tightening — `Schema.Literals(["pending","in_progress","completed","cancelled"])` for `status` and `Schema.Literals(["high","medium","low"])` for `priority` — is tracked as a follow-up under the harness/tool-set-v1 series (issue `#129`) to land either as part of a tool-schema tightening sweep or upstream-first. Do NOT re-flag the free-form strings for these fields in upstream-sync PRs; flag it only in a PawWork-authored PR or the dedicated sweep that touches `TodoItem` schema.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-28T07:27:41.971Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:29-118
Timestamp: 2026-04-28T07:27:41.971Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork), the internal Effect schemas `TimeStats`, `Stats`, `PathText`, `Begin`, `End`, `Summary`, and the exported `SearchMatch`/`Match`/`Result` union are all declared as `Schema.Struct` — matching upstream `dev:packages/opencode/src/file/ripgrep.ts` lines 60-118. PR `#270` is an upstream-sync graft per `project_upstream_strategy.md`. Converting these to `Schema.Class` would diverge from the upstream baseline and create recurring merge friction on future syncs. The `Schema.Class` convention applies to PawWork-authored schemas, not to upstream-grafted shapes in this file. Do NOT re-flag the use of `Schema.Struct` for these schemas in upstream-sync PRs; flag it only in a PR that simultaneously converts the upstream side, or in a dedicated PawWork carve-out PR for ripgrep schemas.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-28T07:27:49.810Z
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.

Applied to files:

  • packages/opencode/src/config/permission.ts
  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-21T16:57:20.257Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 102
File: packages/opencode/src/config/command.ts:23-29
Timestamp: 2026-04-21T16:57:20.257Z
Learning: In packages/opencode/src/config/ config modules, treat both `Schema.Struct` and `Schema.Class` as valid schema patterns and do not require converting `Schema.Struct` to `Schema.Class`. If `Schema.Struct` is used in a config schema (e.g., in files like `command.ts`, `formatter.ts`, `provider.ts`, `permission.ts`, `lsp.ts`, `config.ts`, `keybinds.ts`), accept it as intentional; the selection should be based on upstream alignment and the local schema context, not on a rule that one form must replace the other.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-20T14:21:56.373Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 71
File: packages/app/src/components/session/session-status-connections.tsx:146-147
Timestamp: 2026-04-20T14:21:56.373Z
Learning: In the Astro-Han/pawwork repository (SolidJS app), `sync.data.config` is always initialized to `{}` at `packages/app/src/context/global-sync.tsx` line 71 and is never `undefined` at runtime. Non-optional property access like `sync.data.config.plugin` is intentional and consistent with the pattern used in `packages/app/src/components/status-popover-body.tsx` line 243. Do not flag `sync.data.config.plugin` as needing optional chaining.

Applied to files:

  • packages/opencode/src/config/permission.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/config/permission.ts
  • packages/opencode/src/provider/transform.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/src/config/permission.ts
  • packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-26T15:35:36.505Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 245
File: packages/opencode/src/question/index.ts:21-24
Timestamp: 2026-04-26T15:35:36.505Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/question/index.ts`), `Question.Option.description` is intentionally **required** (`z.string()`, not `.optional()`). This matches the upstream opencode contract and all current callers (e.g. `PlanExitTool`) always supply a description. The defensive `<Show when={props.description}>` rendering in `session-question-dock.tsx` is a standard guard, not a signal that the field is intended to be optional. Do NOT suggest making `Option.description` optional without a dedicated follow-up that covers schema + tool description + dock copy + tests.

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-26T16:34:57.130Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 247
File: packages/ui/src/components/message-part.tsx:1322-1324
Timestamp: 2026-04-26T16:34:57.130Z
Learning: In Astro-Han/pawwork (`packages/ui/src/components/message-part.tsx`), the `taskId` createMemo and `childSessionId` createMemo both intentionally read only from `partMetadata().sessionId` (populated post-execution), not from `input.task_id` / `input.subagent_session_id`. This has always been the case — the original code never read the input field either. Adding an `input.subagent_session_id` fallback would be a new capability, not a bug fix. Do NOT flag the absence of this fallback as a regression in PR `#247` or future PRs unless there is a concrete case where metadata is not populated.

Applied to files:

  • packages/opencode/src/config/permission.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/config/permission.ts
  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-20T14:36:21.288Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.288Z
Learning: Applies to packages/opencode/**/*.ts : Use branded schemas (`Schema.brand`) for single-value types in Effect

Applied to files:

  • packages/opencode/src/config/permission.ts
📚 Learning: 2026-04-20T14:36:21.288Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.288Z
Learning: Applies to packages/opencode/**/*.ts : Use `Schema.Class` for multi-field data in Effect schemas

Applied to files:

  • packages/opencode/src/config/permission.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/config/permission.ts
  • 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/config/permission.ts
  • packages/opencode/src/provider/transform.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/transform.test.ts
  • packages/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-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.ts
  • packages/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-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.ts
  • packages/opencode/src/provider/transform.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.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-28T04:38:21.935Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:21.935Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.

Applied to files:

  • packages/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} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.

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-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/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-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-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-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-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-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
🔇 Additional comments (4)
packages/opencode/src/provider/transform.ts (2)

199-221: DeepSeek/OpenRouter transform ordering looks correct.

The Line 199-215 DeepSeek normalization plus Line 217-221 OpenRouter interleaved bypass cleanly preserves OpenRouter reasoning details while keeping DeepSeek-specific reasoning handling for other providers.


440-445: Variant gating updates are consistent and safely scoped.

The new deepseekMajorVersion helper (Line 440-445), DeepSeek exclusion refinement (Line 452-460), DeepSeek v4+ max effort for openai-compatible (Line 580-587), and Mistral/Groq case handling (Line 763-773, Line 779-790) are internally consistent and avoid the prior switch-scope lint pitfall.

Also applies to: 452-460, 580-587, 763-773, 779-790

packages/opencode/test/provider/transform.test.ts (1)

2116-2161: Mistral reasoning coverage is a good addition.

These tests accurately lock the intended behavior split between mistral-small-* (reasoning variant available) and mistral-large-* (no reasoning variants), including the capabilities.reasoning gate.

packages/opencode/src/config/permission.ts (1)

61-103: The permission precedence concern is already addressed by the codebase design. Permission precedence is determined by Permission.fromConfig() (permission/index.ts:287-315), which explicitly re-sorts all entries by wildcard-ness before building the ruleset. This happens regardless of the input object's key order. The existing test "permission config canonicalises known keys first, preserves rest-key insertion order" (config.test.ts:2106) explicitly validates that key reordering doesn't affect precedence, with reference to the wildcard-before-specifics sorting logic. The InfoZod Zod override is safe and does not introduce a regression.

Comment thread packages/opencode/src/provider/transform.ts
Comment thread packages/opencode/test/provider/transform.test.ts

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Upstream sync PR with 4 commits, 11 files. Carve-outs are correctly preserved. A few issues found ranging from P0 (blocking lint) to P3 (nits). Details in inline comments below.

P0 — Blocking

  • Biome lint failure: noSwitchDeclarations errors in transform.ts on the @ai-sdk/openai-compatible and @ai-sdk/mistral switch cases. Both declare const without block braces. CI will fail.

P1 — Functional / Regression Risk

  • permission.ts InfoZod missing canonical agent key: The Zod override exposes the legacy task key but omits agent, breaking IntelliSense for the canonical permission key post-rename (#128). This contradicts the carve-out comment in the Effect schema.
  • transform.ts variants exclusion list drops mistral: Removing id.includes("mistral") from the early-return list means Mistral models now fall through to provider-specific variant logic. While the new @ai-sdk/mistral case handles this, models routed through OpenRouter or other proxies with mistral in their model.id may now receive unexpected reasoningEffort options. Verify no regression for openrouter/mistral-* models.
  • transform.ts kimi matching broadened invariants: k2p5k2p and kimi-k2.5kimi-k2. widens the match. This could catch unintended model IDs (e.g. future kimi-k2.9). Consider whether the broader pattern is intentional or should be anchored.

P2 — Code Quality / Maintenance

  • transform.ts anthropic/bedrock duplication: The upstream split duplicated ~20 lines of identical message-filtering logic. Extract a shared helper to keep the divergence maintainable.
  • transform.ts deepseek reasoning injection runs before interleaved transform: Empty reasoning parts are injected at line 200, then the interleaved field extractor (line 218) strips them out for non-OpenRouter providers. The ordering is correct but fragile — a future reordering could double-inject or lose reasoning. Add a brief comment explaining the sequencing constraint.
  • transform.ts @ts-expect-error in codex.ts: The input: 272_000 limit field uses @ts-expect-error with a vague justification ("incorrect type for v1 sdk but works"). If the SDK types change, this suppresses legitimate type errors. Consider a more precise suppression or a runtime type guard.

P3 — Nits

  • types.gen.ts tool_output schema added but not wired: The SDK types now expose Config.tool_output, but the PR description notes truncate.ts wiring is deferred. Fine for a staged sync, but ensure the follow-up task (#19) is tracked.
  • PR test plan incomplete: Two reviewer checkboxes remain unchecked (permission IntelliSense, shell cwd interactive). Not blocking, but worth confirming before merge.

Comment thread packages/opencode/src/provider/transform.ts
Comment thread packages/opencode/src/provider/transform.ts Outdated
Comment thread packages/opencode/src/provider/transform.ts Outdated
Comment thread packages/opencode/src/provider/transform.ts
Comment thread packages/opencode/src/provider/transform.ts Outdated
Comment thread packages/opencode/src/provider/transform.ts
Comment thread packages/opencode/src/config/permission.ts Outdated
Comment thread packages/opencode/src/provider/transform.ts Outdated
Comment thread packages/opencode/src/plugin/codex.ts Outdated
Comment thread packages/sdk/js/src/v2/gen/types.gen.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/opencode/src/plugin/codex.ts (1)

453-460: 🧹 Nitpick | 🔵 Trivial

Prefer a typed intersection over a blanket @ts-expect-error.

The SDK-specific shape mismatch looks confined to input, so suppressing the whole assignment weakens type safety. A local intersection keeps the override checked without hiding unrelated errors.

♻️ Proposed fix
           if (model.id.includes("gpt-5.5")) {
-            model.limit = {
+            const limit: typeof model.limit & { input: number } = {
               context: 400_000,
-              // `@ts-expect-error` v1 SDK Model.limit omits input, but Codex plan models use it downstream.
               input: 272_000,
               output: 128_000,
             }
+            model.limit = limit
           }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/src/plugin/codex.ts` around lines 453 - 460, Replace the
blanket `@ts-expect-error` when assigning to model.limit for gpt-5.5 models with a
narrow typed intersection that only augments the missing "input" property;
specifically, cast the RHS (or a small local object) to Model["limit"] & {
input: number } (or an equivalent intersection type) before assigning to
model.limit inside the if (model.id.includes("gpt-5.5")) block so you preserve
type-checking for other properties while allowing the required input field
override.
🤖 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 843-845: Add a regression test in transform.test.ts that asserts
transform(input) sets result["toolStreaming"] to false when input.model.api.npm
=== "@ai-sdk/google-vertex/anthropic"; construct a minimal input object with
model.api.npm set to that string, call the same transform function used by other
tests, and expect the returned result.toolStreaming to strictly equal false so
the provider-specific default is pinned and cannot regress silently.

---

Duplicate comments:
In `@packages/opencode/src/plugin/codex.ts`:
- Around line 453-460: Replace the blanket `@ts-expect-error` when assigning to
model.limit for gpt-5.5 models with a narrow typed intersection that only
augments the missing "input" property; specifically, cast the RHS (or a small
local object) to Model["limit"] & { input: number } (or an equivalent
intersection type) before assigning to model.limit inside the if
(model.id.includes("gpt-5.5")) block so you preserve type-checking for other
properties while allowing the required input field override.
🪄 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: e0f24d0e-4229-4856-b11c-46d8ea6dabd1

📥 Commits

Reviewing files that changed from the base of the PR and between fb5bfee and 3f7a866.

📒 Files selected for processing (3)
  • packages/opencode/src/plugin/codex.ts
  • packages/opencode/src/provider/transform.ts
  • packages/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: Use Effect.gen(function* () { ... }) for Effect composition
Use Effect.fn("Domain.method") for named/traced effects and Effect.fnUntraced for internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer .pipe() wrappers
Use Effect.callback for callback-based APIs
Prefer DateTime.nowAsDate over new Date(yield* Clock.currentTimeMillis) when you need a Date in Effect code
Use Schema.Class for multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
Use Schema.TaggedErrorClass for typed errors in Effect schemas
Use Schema.Defect instead of unknown for defect-like causes in Effect code
In Effect.gen / Effect.fn, prefer yield* new MyError(...) over yield* Effect.fail(new MyError(...)) for direct early-failure branches
Use makeRuntime from src/effect/run-service.ts for all services; it returns { runPromise, runFork, runCallback } backed by a shared memoMap that deduplicates layers
Use InstanceState from src/effect/instance-state.ts for per-directory or per-project state that needs per-instance cleanup; do work directly in the InstanceState.make closure where ScopedCache handles run-once semantics
Use Effect.addFinalizer or Effect.acquireRelease inside the InstanceState.make closure for cleanup (subscriptions, process teardown, etc.)
Use Effect.forkScoped inside the InstanceState.make closure for background stream consumers — the fiber is interrupted when the instance is disposed
Prefer FileSystem.FileSystem instead of raw fs/promises for effectful file I/O in Effect services
Prefer ChildProcessSpawner.ChildProcessSpawner with ChildProcess.make(...) instead of custom process wrappers in Effect services
Prefer HttpClient.HttpClient instead of raw fetch in Effect services
Prefer Path.Path, Config, Clock, and DateTime services when those concerns are already inside Effect code
For backgroun...

Files:

  • packages/opencode/src/plugin/codex.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/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 the tmpdir function from fixture/fixture.ts to create temporary directories for tests with automatic cleanup. Use await using syntax to ensure automatic cleanup when the variable goes out of scope.
When using the tmpdir function with git repository support, pass the git: true option to initialize a git repo with a root commit.
Use the config option in tmpdir to write an opencode.json config file during test setup by passing a partial Config.Info object.
Use the init option in tmpdir to define custom setup functions that can return extra data accessible via tmp.extra, and use the dispose option for custom cleanup logic.
Use testEffect(...) from test/lib/effect.ts for tests that exercise Effect services or Effect-based workflows.
Use it.effect(...) when the test should run with TestClock and TestConsole. Use it.live(...) when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers from fixture/fixture.ts over building manual runtimes in tests: use tmpdirScoped() for scoped temp directories, provideInstance(dir)(effect) for low-level binding without directory creation, provideTmpdirInstance(...) for single temp instance binding, or provideTmpdirServer(...) for tests that also need the test LLM server.
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.
Avoid custom ManagedRuntime, attach(...), or ad hoc run(...) wrappers in Effect tests when testEffect(...) already provides the runtime.
When a test needs instance-local state, prefer provideTmpdirInstance(...) or provideInstance(...) over manual Instance.provide(...) inside Promise-style tests.

Files:

  • packages/opencode/test/provider/transform.test.ts
🧠 Learnings (40)
📓 Common learnings
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.
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/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/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/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/tool/edit.ts:48-48
Timestamp: 2026-04-28T08:14:31.436Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork), the `filePath` schema description (`"The absolute path to the file to modify"`) is upstream-inherited from `dev:packages/opencode/src/tool/edit.ts:48`. The runtime actually accepts relative paths (resolved via `Instance.directory` at lines 79-81), but the description fix is intentionally deferred to a single PawWork-authored description-cleanup PR that will also cover the identical mismatch in `packages/opencode/src/tool/write.ts:24`. Do NOT re-flag the too-narrow `filePath` description in upstream-sync PRs; flag it only in the dedicated description-cleanup PR.
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.
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.
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.
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.
📚 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/plugin/codex.ts
  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T13:10:01.345Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/tool/agent.ts:89-102
Timestamp: 2026-04-28T13:10:01.345Z
Learning: In `packages/opencode/src/tool/agent.ts` (Astro-Han/pawwork, PR `#287`), `makeReadLastCompletedAssistantText` intentionally scans the full child session message history (no `limit`) to find the latest completed assistant text part for `partial_result` on cancellation. Cancellation is a rare path so the extra read cost is acceptable; tool-heavy children can push the last stable assistant text far back. Do NOT suggest re-adding a fixed small limit (e.g., `limit: 5`) to this function.

Applied to files:

  • packages/opencode/src/plugin/codex.ts
  • 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/plugin/codex.ts
  • 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/plugin/codex.ts
  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-20T14:36:21.288Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.288Z
Learning: Applies to packages/opencode/**/*.ts : Use `Schema.Defect` instead of `unknown` for defect-like causes in Effect code

Applied to files:

  • packages/opencode/src/plugin/codex.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/plugin/codex.ts
  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-21T12:14:30.524Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 93
File: packages/opencode/test/server/cors-middleware.test.ts:15-23
Timestamp: 2026-04-21T12:14:30.524Z
Learning: In `packages/opencode/test/server/cors-middleware.test.ts` (and similar Hono-based test files), `app.request(...)` on the Hono `Hono` app type returns `Promise<Response> | Response` — a union that includes a non-PromiseLike branch. `Effect.promise(...)` requires a `PromiseLike`, so wrapping with `Promise.resolve(app.request(...))` is necessary to normalize the union type. Removing the wrapper causes a TS2322 typecheck error in `packages/opencode`.

Applied to files:

  • packages/opencode/src/plugin/codex.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/src/plugin/codex.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/plugin/codex.ts
  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-27T10:08:57.448Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/test/session/diagnostics.test.ts:313-318
Timestamp: 2026-04-27T10:08:57.448Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/diagnostics.ts`, PR `#264`), `SessionDiagnostics.LoopAction` is a TypeScript string literal union type (`"observe" | "block" | "stop"`), NOT a runtime enum. There is no runtime form to introspect via `Object.values()`. The test pattern `const all: SessionDiagnostics.LoopAction[] = ["observe", "block", "stop"]; expect(all).toHaveLength(3)` is intentional and correct — it provides compile-time exhaustiveness via the type annotation. Do NOT suggest replacing this with `Object.values()` introspection or flag the runtime length check as insufficient.

Applied to files:

  • packages/opencode/src/plugin/codex.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/plugin/codex.ts
  • 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/plugin/codex.ts
  • packages/opencode/src/provider/transform.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/transform.test.ts
  • packages/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-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.ts
  • packages/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.ts
  • packages/opencode/src/provider/transform.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.ts
  • packages/opencode/src/provider/transform.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} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.

Applied to files:

  • packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-28T04:38:21.935Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:21.935Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.

Applied to files:

  • packages/opencode/test/provider/transform.test.ts
📚 Learning: 2026-04-27T12:59:49.844Z
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:49.844Z
Learning: In `packages/opencode/test/session/prompt-effect.test.ts` and `packages/opencode/src/session/diagnostics.ts` (PR `#264`), the recovery reminder copy differs between signature kinds: the input-repeat variant says "repeated the same tool input 3 times" (uses a literal count), while the target-repeat variant says "failed against the same target multiple times" (uses "multiple times" with no count). Assertions that check for injected reminder text in LLM inputs must accept both phrasings when a scenario produces both `input:` and `target:` signatures (e.g., `read` tool with a `filePath` parameter). Do NOT narrow the assertion to only the input-variant phrasing.

Applied to files:

  • packages/opencode/test/provider/transform.test.ts
  • packages/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.ts
📚 Learning: 2026-04-24T05:48:39.493Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:39.493Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, the Model chip trigger button carries `data-action="prompt-model"` (around line 1187) and the Variant chip trigger button carries `data-action="prompt-model-variant"` (around line 1231), both set via `triggerProps`. These are therefore already captured by any `[data-action]` selector sweep in E2E tests and do not need a separate `[data-component]` query to be included in parity assertions — though unioning both is kept as belt-and-suspenders in `collectBarSet`.

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-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/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-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-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-28T07:27:49.810Z
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.

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-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: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-28T04:38:05.946Z
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.

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-28T06:51:54.812Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/todo.ts:9-18
Timestamp: 2026-04-28T06:51:54.812Z
Learning: In `packages/opencode/src/tool/todo.ts` (Astro-Han/pawwork), `TodoItem.status` and `TodoItem.priority` are intentionally declared as plain `Schema.String` rather than closed literal unions. This matches the upstream opencode baseline (`dev:packages/opencode/src/tool/todo.ts`). The tightening — `Schema.Literals(["pending","in_progress","completed","cancelled"])` for `status` and `Schema.Literals(["high","medium","low"])` for `priority` — is tracked as a follow-up under the harness/tool-set-v1 series (issue `#129`) to land either as part of a tool-schema tightening sweep or upstream-first. Do NOT re-flag the free-form strings for these fields in upstream-sync PRs; flag it only in a PawWork-authored PR or the dedicated sweep that touches `TodoItem` schema.

Applied to files:

  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T07:27:41.971Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:29-118
Timestamp: 2026-04-28T07:27:41.971Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork), the internal Effect schemas `TimeStats`, `Stats`, `PathText`, `Begin`, `End`, `Summary`, and the exported `SearchMatch`/`Match`/`Result` union are all declared as `Schema.Struct` — matching upstream `dev:packages/opencode/src/file/ripgrep.ts` lines 60-118. PR `#270` is an upstream-sync graft per `project_upstream_strategy.md`. Converting these to `Schema.Class` would diverge from the upstream baseline and create recurring merge friction on future syncs. The `Schema.Class` convention applies to PawWork-authored schemas, not to upstream-grafted shapes in this file. Do NOT re-flag the use of `Schema.Struct` for these schemas in upstream-sync PRs; flag it only in a PR that simultaneously converts the upstream side, or in a dedicated PawWork carve-out PR for ripgrep schemas.

Applied to files:

  • packages/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/transform.ts
🔇 Additional comments (6)
packages/opencode/src/provider/transform.ts (3)

20-34: Good consolidation of empty-content filtering.

This helper removes duplicated logic and keeps Anthropic/Bedrock behavior aligned while preserving non-text parts.

Also applies to: 71-78


183-206: Ordering and OpenRouter guard are well handled.

The Line 183 sequencing note plus the Line 205 OpenRouter bypass makes the DeepSeek/interleaved interaction explicit and less regression-prone.


566-573: DeepSeek v4+ effort gating is clean and lint-safe.

Block scoping is correct, and deepseekMajorVersion(...) keeps "max" enablement narrowly targeted.

packages/opencode/test/provider/transform.test.ts (3)

361-421: Kimi anthropic gating coverage is strong.

Positive and negative cases are both asserted, which protects against accidental match broadening.


2178-2237: Mistral variant tests now match provider-specific intent.

Coverage for small/latest, large, and proxy-routed IDs clearly captures the new branching behavior.


2687-2729: DeepSeek v4/v5 assertions are now robust.

Great upgrade from key-only checks to full variant payload assertions, especially for "max".

Comment thread packages/opencode/src/provider/transform.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

# Conflicts:
#	packages/opencode/src/provider/transform.ts
#	packages/opencode/test/session/message-v2.test.ts
@Astro-Han

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/opencode/src/session/prompt.ts (1)

965-991: ⚠️ Potential issue | 🟠 Major

Fail fast if cd cannot switch to the requested directory.

cd -- "$1" is unchecked in both wrappers, so if the directory is missing or becomes inaccessible after spawn, the command still evals in the shell's current directory. That defeats the cwd fix and can run against the wrong tree.

🔧 Suggested fix
-                  cd -- "$1"
+                  cd -- "$1" || exit 1
                   eval ${JSON.stringify(input.command)}

Apply the same change in both the zsh and bash branches.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/src/session/prompt.ts` around lines 965 - 991, The zsh and
bash shell wrappers in the args templates currently run `cd -- "$1"` without
checking its result, so update both template strings in
packages/opencode/src/session/prompt.ts (the zsh and bash branches that build
args arrays) to fail fast if the cd fails — i.e., after the cd command add a
guard that exits non‑zero when cd cannot change directory (so the subsequent
eval of input.command does not run in the wrong tree); apply the same change to
both the zsh and bash templates that contain `cd -- "$1"` and `eval
${JSON.stringify(input.command)}`.
🤖 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/test/session/message-v2.test.ts`:
- Around line 784-855: The file contains a duplicated test case named "preserves
OpenRouter reasoning details through provider transform" (the block that builds
openrouterModel, reasoningDetails, input and asserts
ProviderTransform.message(MessageV2.toModelMessages(...))) — remove the
second/verbatim duplicate of that entire test block so only the original test
(using openrouterModel, reasoningDetails, MessageV2.toModelMessages, and
ProviderTransform.message) remains to avoid running the same assertions twice.

---

Outside diff comments:
In `@packages/opencode/src/session/prompt.ts`:
- Around line 965-991: The zsh and bash shell wrappers in the args templates
currently run `cd -- "$1"` without checking its result, so update both template
strings in packages/opencode/src/session/prompt.ts (the zsh and bash branches
that build args arrays) to fail fast if the cd fails — i.e., after the cd
command add a guard that exits non‑zero when cd cannot change directory (so the
subsequent eval of input.command does not run in the wrong tree); apply the same
change to both the zsh and bash templates that contain `cd -- "$1"` and `eval
${JSON.stringify(input.command)}`.
🪄 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: db3254e4-9c74-4dd5-ae8b-30fea083d236

📥 Commits

Reviewing files that changed from the base of the PR and between 3f7a866 and d325e27.

📒 Files selected for processing (5)
  • packages/opencode/src/provider/transform.ts
  • packages/opencode/src/session/prompt.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/test/session/retry.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts

📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)

packages/opencode/**/*.ts: Use Effect.gen(function* () { ... }) for Effect composition
Use Effect.fn("Domain.method") for named/traced effects and Effect.fnUntraced for internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer .pipe() wrappers
Use Effect.callback for callback-based APIs
Prefer DateTime.nowAsDate over new Date(yield* Clock.currentTimeMillis) when you need a Date in Effect code
Use Schema.Class for multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
Use Schema.TaggedErrorClass for typed errors in Effect schemas
Use Schema.Defect instead of unknown for defect-like causes in Effect code
In Effect.gen / Effect.fn, prefer yield* new MyError(...) over yield* Effect.fail(new MyError(...)) for direct early-failure branches
Use makeRuntime from src/effect/run-service.ts for all services; it returns { runPromise, runFork, runCallback } backed by a shared memoMap that deduplicates layers
Use InstanceState from src/effect/instance-state.ts for per-directory or per-project state that needs per-instance cleanup; do work directly in the InstanceState.make closure where ScopedCache handles run-once semantics
Use Effect.addFinalizer or Effect.acquireRelease inside the InstanceState.make closure for cleanup (subscriptions, process teardown, etc.)
Use Effect.forkScoped inside the InstanceState.make closure for background stream consumers — the fiber is interrupted when the instance is disposed
Prefer FileSystem.FileSystem instead of raw fs/promises for effectful file I/O in Effect services
Prefer ChildProcessSpawner.ChildProcessSpawner with ChildProcess.make(...) instead of custom process wrappers in Effect services
Prefer HttpClient.HttpClient instead of raw fetch in Effect services
Prefer Path.Path, Config, Clock, and DateTime services when those concerns are already inside Effect code
For backgroun...

Files:

  • packages/opencode/test/session/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/src/session/prompt.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/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 the tmpdir function from fixture/fixture.ts to create temporary directories for tests with automatic cleanup. Use await using syntax to ensure automatic cleanup when the variable goes out of scope.
When using the tmpdir function with git repository support, pass the git: true option to initialize a git repo with a root commit.
Use the config option in tmpdir to write an opencode.json config file during test setup by passing a partial Config.Info object.
Use the init option in tmpdir to define custom setup functions that can return extra data accessible via tmp.extra, and use the dispose option for custom cleanup logic.
Use testEffect(...) from test/lib/effect.ts for tests that exercise Effect services or Effect-based workflows.
Use it.effect(...) when the test should run with TestClock and TestConsole. Use it.live(...) when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers from fixture/fixture.ts over building manual runtimes in tests: use tmpdirScoped() for scoped temp directories, provideInstance(dir)(effect) for low-level binding without directory creation, provideTmpdirInstance(...) for single temp instance binding, or provideTmpdirServer(...) for tests that also need the test LLM server.
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.
Avoid custom ManagedRuntime, attach(...), or ad hoc run(...) wrappers in Effect tests when testEffect(...) already provides the runtime.
When a test needs instance-local state, prefer provideTmpdirInstance(...) or provideInstance(...) over manual Instance.provide(...) inside Promise-style tests.

Files:

  • packages/opencode/test/session/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/test/provider/transform.test.ts
🧠 Learnings (44)
📓 Common learnings
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.
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/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: 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: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.
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/edit.ts:48-48
Timestamp: 2026-04-28T08:14:31.436Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork), the `filePath` schema description (`"The absolute path to the file to modify"`) is upstream-inherited from `dev:packages/opencode/src/tool/edit.ts:48`. The runtime actually accepts relative paths (resolved via `Instance.directory` at lines 79-81), but the description fix is intentionally deferred to a single PawWork-authored description-cleanup PR that will also cover the identical mismatch in `packages/opencode/src/tool/write.ts:24`. Do NOT re-flag the too-narrow `filePath` description in upstream-sync PRs; flag it only in the dedicated description-cleanup PR.
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.
📚 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/test/session/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/src/provider/transform.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/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/opencode/src/provider/transform.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/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
📚 Learning: 2026-04-21T12:14:30.524Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 93
File: packages/opencode/test/server/cors-middleware.test.ts:15-23
Timestamp: 2026-04-21T12:14:30.524Z
Learning: In `packages/opencode/test/server/cors-middleware.test.ts` (and similar Hono-based test files), `app.request(...)` on the Hono `Hono` app type returns `Promise<Response> | Response` — a union that includes a non-PromiseLike branch. `Effect.promise(...)` requires a `PromiseLike`, so wrapping with `Promise.resolve(app.request(...))` is necessary to normalize the union type. Removing the wrapper causes a TS2322 typecheck error in `packages/opencode`.

Applied to files:

  • packages/opencode/test/session/retry.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/session/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/opencode/src/provider/transform.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/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/opencode/src/provider/transform.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/session/retry.test.ts
  • packages/opencode/test/session/message-v2.test.ts
  • packages/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
  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T05:36:24.561Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.

Applied to files:

  • packages/opencode/test/session/message-v2.test.ts
  • packages/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/session/message-v2.test.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/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/session/message-v2.test.ts
  • packages/opencode/test/provider/transform.test.ts
  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-24T03:51:56.211Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:56.211Z
Learning: In Astro-Han/pawwork E2E tests (packages/app/e2e/fixtures.ts), `project.prompt(text)` internally calls `trackSession(next.sessionID, active.directory)` after the prompt submission is observed and the active session is resolved. Tests that obtain a `sessionID` from `project.prompt()` do NOT need to call `project.trackSession(sessionID)` manually — it is already registered for teardown. Calling it again would be duplicate ownership.

Applied to files:

  • packages/opencode/src/session/prompt.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/session/prompt.ts
  • 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/session/prompt.ts
  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-27T10:32:59.274Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/diagnostics.ts:245-252
Timestamp: 2026-04-27T10:32:59.274Z
Learning: In pawwork’s session diagnostics/loop error handling (e.g., packages/opencode/src/session/diagnostics.ts), keep `loopLastError` stored as raw/unscrubbed error text in loop metadata. The intended trust boundary for sensitive-data scrubbing is the renderer: `LoopRenderer.render` must apply `scrubErrorText` before any user/model-facing output. Do not recommend scrubbing `loopLastError` at the `observeToolError` call site, because failed tool parts already store raw errors in `state.error` (pre-existing leak) and scrubbing only `loopLastError` would be lossy without closing the gap. If export-side hardening is required, implement a single sanitizer pass in the export layer over both `state.error` and the loop/metadata fields, rather than sanitizing only `loopLastError` earlier in the storage path.

Applied to files:

  • packages/opencode/src/session/prompt.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 this repo’s service-internal code (e.g., session modules), follow the established error-class convention: use plain classes with a `readonly _tag` discriminant and/or `NamedError.create` (zod-based) for service-internal errors. Do NOT request one-off conversions of such plain/service-internal errors to `Schema.TaggedErrorClass`; that migration is intentionally deferred until the dedicated repo-wide error-class sweep PR lands.

Applied to files:

  • packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-27T10:33:08.974Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:08.974Z
Learning: For Astro-Han/pawwork session processing code under packages/opencode/src/session/ (e.g., prompt.ts and processor.ts), this specific loop-gate TOCTOU/race between buildLoopContext() and recordSyntheticBlock/recordSyntheticStop is intentionally handled via idempotence guards (re-check sigKey presence and hasStopped inside the record helpers). Do not treat the absence of a per-parent Effect.Mutex as a blocking review issue for this known edge case; the residual window should only create extra synthetic parts and must not change the "turn ends" contract. Respect the existing code comment documenting the trade-off, and only escalate to a full-mutex approach if the race is observed in practice.

Applied to files:

  • packages/opencode/src/session/prompt.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-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
  • packages/opencode/src/provider/transform.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.ts
  • packages/opencode/src/provider/transform.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} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.

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-27T12:59:49.844Z
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:49.844Z
Learning: In `packages/opencode/test/session/prompt-effect.test.ts` and `packages/opencode/src/session/diagnostics.ts` (PR `#264`), the recovery reminder copy differs between signature kinds: the input-repeat variant says "repeated the same tool input 3 times" (uses a literal count), while the target-repeat variant says "failed against the same target multiple times" (uses "multiple times" with no count). Assertions that check for injected reminder text in LLM inputs must accept both phrasings when a scenario produces both `input:` and `target:` signatures (e.g., `read` tool with a `filePath` parameter). Do NOT narrow the assertion to only the input-variant phrasing.

Applied to files:

  • packages/opencode/test/provider/transform.test.ts
  • packages/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.ts
📚 Learning: 2026-04-24T05:48:39.493Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:39.493Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, the Model chip trigger button carries `data-action="prompt-model"` (around line 1187) and the Variant chip trigger button carries `data-action="prompt-model-variant"` (around line 1231), both set via `triggerProps`. These are therefore already captured by any `[data-action]` selector sweep in E2E tests and do not need a separate `[data-component]` query to be included in parity assertions — though unioning both is kept as belt-and-suspenders in `collectBarSet`.

Applied to files:

  • packages/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} : 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/provider/transform.test.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-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-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-28T07:27:49.810Z
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.

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-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: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-28T04:38:05.946Z
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.

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-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-28T07:27:41.971Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:29-118
Timestamp: 2026-04-28T07:27:41.971Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork), the internal Effect schemas `TimeStats`, `Stats`, `PathText`, `Begin`, `End`, `Summary`, and the exported `SearchMatch`/`Match`/`Result` union are all declared as `Schema.Struct` — matching upstream `dev:packages/opencode/src/file/ripgrep.ts` lines 60-118. PR `#270` is an upstream-sync graft per `project_upstream_strategy.md`. Converting these to `Schema.Class` would diverge from the upstream baseline and create recurring merge friction on future syncs. The `Schema.Class` convention applies to PawWork-authored schemas, not to upstream-grafted shapes in this file. Do NOT re-flag the use of `Schema.Struct` for these schemas in upstream-sync PRs; flag it only in a PR that simultaneously converts the upstream side, or in a dedicated PawWork carve-out PR for ripgrep schemas.

Applied to files:

  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T06:51:54.812Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/todo.ts:9-18
Timestamp: 2026-04-28T06:51:54.812Z
Learning: In `packages/opencode/src/tool/todo.ts` (Astro-Han/pawwork), `TodoItem.status` and `TodoItem.priority` are intentionally declared as plain `Schema.String` rather than closed literal unions. This matches the upstream opencode baseline (`dev:packages/opencode/src/tool/todo.ts`). The tightening — `Schema.Literals(["pending","in_progress","completed","cancelled"])` for `status` and `Schema.Literals(["high","medium","low"])` for `priority` — is tracked as a follow-up under the harness/tool-set-v1 series (issue `#129`) to land either as part of a tool-schema tightening sweep or upstream-first. Do NOT re-flag the free-form strings for these fields in upstream-sync PRs; flag it only in a PawWork-authored PR or the dedicated sweep that touches `TodoItem` schema.

Applied to files:

  • packages/opencode/src/provider/transform.ts
📚 Learning: 2026-04-28T13:10:01.345Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/tool/agent.ts:89-102
Timestamp: 2026-04-28T13:10:01.345Z
Learning: In `packages/opencode/src/tool/agent.ts` (Astro-Han/pawwork, PR `#287`), `makeReadLastCompletedAssistantText` intentionally scans the full child session message history (no `limit`) to find the latest completed assistant text part for `partial_result` on cancellation. Cancellation is a rare path so the extra read cost is acceptable; tool-heavy children can push the last stable assistant text far back. Do NOT suggest re-adding a fixed small limit (e.g., `limit: 5`) to this function.

Applied to files:

  • packages/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/transform.ts
📚 Learning: 2026-04-28T04:38:21.935Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:21.935Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.

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-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/src/provider/transform.ts
🔇 Additional comments (6)
packages/opencode/src/provider/transform.ts (1)

20-34: Looks good — these transform updates are coherent and regression-focused.

The helper extraction, DeepSeek ordering guard comment, and provider-specific variant gating changes are consistent and avoid the earlier drift risks.

Also applies to: 79-86, 191-192, 433-437, 448-448, 573-576, 754-764, 770-781

packages/opencode/test/session/message-v2.test.ts (1)

1132-1154: Good retryability regression coverage for streamed server_error.

This assertion cleanly pins MessageV2.fromError behavior for OpenAI-style stream chunks with preserved message/body.

packages/opencode/test/session/retry.test.ts (1)

298-318: Nice fixture update — more realistic OpenAI stream error payload shape.

Including sequence_number, error.type, and param makes this regression test stronger while preserving the intended retryability assertion.

packages/opencode/test/provider/transform.test.ts (3)

122-138: Good targeted coverage for Vertex Anthropic tool streaming.

This is a narrow, high-signal assertion and fits cleanly alongside the existing ProviderTransform.options coverage.


446-506: No issue—tests will correctly exercise the thinking logic.

The thinking enable logic (lines 910–915 of transform.ts) checks api.npm === "@ai-sdk/anthropic" and whether api.id includes "k2p5" or "kimi-k2.5" or "kimi-k2p5"—it does not key off providerID. The test fixture provides all required values correctly, so the assertions will hit the intended code path and pass.

			> Likely an incorrect or invalid review comment.

2540-3091: The DeepSeek test fixtures correctly use ID shapes that the matcher expects.

The deepseekMajorVersion function uses a regex pattern (^|[/:])deepseek-v(\d+)(?:[.\-/]|$) that intentionally handles both formats: the raw api.id like "deepseek-v3" (from earlier tests) and the prefixed form like "deepseek/deepseek-v3.2" used in the new fixtures. The regex matches successfully in both cases and extracts the correct version number. The new v3/v3.2 models will return empty objects (correctly excluded), while v4/v4-pro/v5 models will include the "max" effort as expected.

			> Likely an incorrect or invalid review comment.

Comment thread packages/opencode/test/session/message-v2.test.ts
# Conflicts:
#	packages/opencode/src/session/prompt.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/session/prompt.ts`:
- Around line 971-973: The wrapper currently runs eval
${JSON.stringify(input.command)} even if cd -- "$OPENCODE_SHELL_CWD" fails;
update the wrapper so the cd command aborts on failure (e.g. use an explicit
conditional or || exit) before reaching the eval to avoid running the user's
command from the wrong directory; apply the same fix to both occurrences that
use OPENCODE_SHELL_CWD and eval ${JSON.stringify(input.command)} in this file.
- Around line 1007-1010: The OPENCODE_SHELL_CWD env var is being injected
globally into ChildProcess.make but only consumed by the bash/zsh wrapper
branches; remove OPENCODE_SHELL_CWD from the shared env object passed to
ChildProcess.make and instead add OPENCODE_SHELL_CWD: cwd to the env for the two
wrapper cases that read it (the bash and zsh wrapper branches), keeping the
existing spread of shellEnv.env and TERM in all branches and preserving
extendEnv: true so other shells do not inherit the unused internal variable.
🪄 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: 7bca4d30-f44a-4edc-bd15-3b8a02fb3f81

📥 Commits

Reviewing files that changed from the base of the PR and between d325e27 and c6e6ae7.

📒 Files selected for processing (2)
  • packages/opencode/src/session/prompt.ts
  • packages/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: unit-windows-opencode-config-project
  • GitHub Check: unit-windows-opencode-session
  • GitHub Check: unit-windows-desktop
  • GitHub Check: unit-windows-opencode-server-tools
  • GitHub Check: unit-windows-app
  • GitHub Check: unit-app
  • GitHub Check: unit-desktop
  • GitHub Check: unit-opencode
  • GitHub Check: typecheck
  • GitHub Check: smoke-macos-arm64
  • GitHub Check: e2e-artifacts
  • GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts

📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)

packages/opencode/**/*.ts: Use Effect.gen(function* () { ... }) for Effect composition
Use Effect.fn("Domain.method") for named/traced effects and Effect.fnUntraced for internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer .pipe() wrappers
Use Effect.callback for callback-based APIs
Prefer DateTime.nowAsDate over new Date(yield* Clock.currentTimeMillis) when you need a Date in Effect code
Use Schema.Class for multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
Use Schema.TaggedErrorClass for typed errors in Effect schemas
Use Schema.Defect instead of unknown for defect-like causes in Effect code
In Effect.gen / Effect.fn, prefer yield* new MyError(...) over yield* Effect.fail(new MyError(...)) for direct early-failure branches
Use makeRuntime from src/effect/run-service.ts for all services; it returns { runPromise, runFork, runCallback } backed by a shared memoMap that deduplicates layers
Use InstanceState from src/effect/instance-state.ts for per-directory or per-project state that needs per-instance cleanup; do work directly in the InstanceState.make closure where ScopedCache handles run-once semantics
Use Effect.addFinalizer or Effect.acquireRelease inside the InstanceState.make closure for cleanup (subscriptions, process teardown, etc.)
Use Effect.forkScoped inside the InstanceState.make closure for background stream consumers — the fiber is interrupted when the instance is disposed
Prefer FileSystem.FileSystem instead of raw fs/promises for effectful file I/O in Effect services
Prefer ChildProcessSpawner.ChildProcessSpawner with ChildProcess.make(...) instead of custom process wrappers in Effect services
Prefer HttpClient.HttpClient instead of raw fetch in Effect services
Prefer Path.Path, Config, Clock, and DateTime services when those concerns are already inside Effect code
For backgroun...

Files:

  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/src/session/prompt.ts
packages/opencode/test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (packages/opencode/test/AGENTS.md)

packages/opencode/test/**/*.test.{ts,tsx}: Use the tmpdir function from fixture/fixture.ts to create temporary directories for tests with automatic cleanup. Use await using syntax to ensure automatic cleanup when the variable goes out of scope.
When using the tmpdir function with git repository support, pass the git: true option to initialize a git repo with a root commit.
Use the config option in tmpdir to write an opencode.json config file during test setup by passing a partial Config.Info object.
Use the init option in tmpdir to define custom setup functions that can return extra data accessible via tmp.extra, and use the dispose option for custom cleanup logic.
Use testEffect(...) from test/lib/effect.ts for tests that exercise Effect services or Effect-based workflows.
Use it.effect(...) when the test should run with TestClock and TestConsole. Use it.live(...) when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers from fixture/fixture.ts over building manual runtimes in tests: use tmpdirScoped() for scoped temp directories, provideInstance(dir)(effect) for low-level binding without directory creation, provideTmpdirInstance(...) for single temp instance binding, or provideTmpdirServer(...) for tests that also need the test LLM server.
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.
Avoid custom ManagedRuntime, attach(...), or ad hoc run(...) wrappers in Effect tests when testEffect(...) already provides the runtime.
When a test needs instance-local state, prefer provideTmpdirInstance(...) or provideInstance(...) over manual Instance.provide(...) inside Promise-style tests.

Files:

  • packages/opencode/test/session/message-v2.test.ts
🧠 Learnings (26)
📓 Common learnings
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.
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/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: 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: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.
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/edit.ts:48-48
Timestamp: 2026-04-28T08:14:31.436Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork), the `filePath` schema description (`"The absolute path to the file to modify"`) is upstream-inherited from `dev:packages/opencode/src/tool/edit.ts:48`. The runtime actually accepts relative paths (resolved via `Instance.directory` at lines 79-81), but the description fix is intentionally deferred to a single PawWork-authored description-cleanup PR that will also cover the identical mismatch in `packages/opencode/src/tool/write.ts:24`. Do NOT re-flag the too-narrow `filePath` description in upstream-sync PRs; flag it only in the dedicated description-cleanup PR.
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.
📚 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/test/session/message-v2.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-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.ts
📚 Learning: 2026-04-28T18:53:46.234Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 308
File: packages/opencode/test/server/global-session-list.test.ts:224-259
Timestamp: 2026-04-28T18:53:46.234Z
Learning: In `packages/opencode/test/server/global-session-list.test.ts` (Astro-Han/pawwork), all `it.live(...)` test bodies intentionally use `Effect.promise(async () => { ... })` rather than `Effect.gen(function* () { ... })`. This is the established local convention for the route/pagination tests in this file (e.g., "supports cursor pagination", "session route orders by creation time", "session routes omit undefined optional fields"). Do NOT flag individual `it.live` cases in this file as needing conversion to `Effect.gen`; any harness-style migration would need to cover the entire file.

Applied to files:

  • packages/opencode/test/session/message-v2.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/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/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.ts
📚 Learning: 2026-04-28T05:36:24.561Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.

Applied to files:

  • packages/opencode/test/session/message-v2.test.ts
  • packages/opencode/src/session/prompt.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.ts
📚 Learning: 2026-04-28T04:38:21.935Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:21.935Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.

Applied to files:

  • packages/opencode/test/session/message-v2.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-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/session/message-v2.test.ts
📚 Learning: 2026-04-25T12:52:49.735Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/opencode/src/share/session.ts:27-27
Timestamp: 2026-04-25T12:52:49.735Z
Learning: In `packages/opencode/src/share/session.ts`, the local `ensureEnabled` closure is intentionally duplicated from `ShareRuntime.ensureEnabled` in `runtime.ts`. Using the shared `ShareRuntime.ensureEnabled` effect directly would leak `CloudShareGate` back into the `R` (requirement) type of `share`, `unshare`, and `create`, because the shared effect re-yields the service rather than capturing an already-resolved gate reference. The current pattern resolves `gate` once at the top of the outer `Effect.gen` and then closes over it in a local `ensureEnabled`, keeping the inner effects' requirement types clean. A comment in the file points to `runtime.ts` to make the intentional duplication discoverable. The real fix would be refactoring `ensureEnabled` to accept a gate parameter, but that change is larger than the DRY benefit warrants. Do not flag this duplication as a maintenance issue.

Applied to files:

  • packages/opencode/test/session/message-v2.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/session/message-v2.test.ts
📚 Learning: 2026-04-28T13:10:01.345Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/tool/agent.ts:89-102
Timestamp: 2026-04-28T13:10:01.345Z
Learning: In `packages/opencode/src/tool/agent.ts` (Astro-Han/pawwork, PR `#287`), `makeReadLastCompletedAssistantText` intentionally scans the full child session message history (no `limit`) to find the latest completed assistant text part for `partial_result` on cancellation. Cancellation is a rare path so the extra read cost is acceptable; tool-heavy children can push the last stable assistant text far back. Do NOT suggest re-adding a fixed small limit (e.g., `limit: 5`) to this function.

Applied to files:

  • packages/opencode/test/session/message-v2.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/session/message-v2.test.ts
📚 Learning: 2026-04-24T03:51:56.211Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:56.211Z
Learning: In Astro-Han/pawwork E2E tests (packages/app/e2e/fixtures.ts), `project.prompt(text)` internally calls `trackSession(next.sessionID, active.directory)` after the prompt submission is observed and the active session is resolved. Tests that obtain a `sessionID` from `project.prompt()` do NOT need to call `project.trackSession(sessionID)` manually — it is already registered for teardown. Calling it again would be duplicate ownership.

Applied to files:

  • packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-24T05:39:58.329Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:58.329Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.

Applied to files:

  • packages/opencode/src/session/prompt.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/src/session/prompt.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/session/prompt.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/session/prompt.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/session/prompt.ts
📚 Learning: 2026-04-27T10:32:59.274Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/diagnostics.ts:245-252
Timestamp: 2026-04-27T10:32:59.274Z
Learning: In pawwork’s session diagnostics/loop error handling (e.g., packages/opencode/src/session/diagnostics.ts), keep `loopLastError` stored as raw/unscrubbed error text in loop metadata. The intended trust boundary for sensitive-data scrubbing is the renderer: `LoopRenderer.render` must apply `scrubErrorText` before any user/model-facing output. Do not recommend scrubbing `loopLastError` at the `observeToolError` call site, because failed tool parts already store raw errors in `state.error` (pre-existing leak) and scrubbing only `loopLastError` would be lossy without closing the gap. If export-side hardening is required, implement a single sanitizer pass in the export layer over both `state.error` and the loop/metadata fields, rather than sanitizing only `loopLastError` earlier in the storage path.

Applied to files:

  • packages/opencode/src/session/prompt.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 this repo’s service-internal code (e.g., session modules), follow the established error-class convention: use plain classes with a `readonly _tag` discriminant and/or `NamedError.create` (zod-based) for service-internal errors. Do NOT request one-off conversions of such plain/service-internal errors to `Schema.TaggedErrorClass`; that migration is intentionally deferred until the dedicated repo-wide error-class sweep PR lands.

Applied to files:

  • packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-27T10:33:08.974Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:08.974Z
Learning: For Astro-Han/pawwork session processing code under packages/opencode/src/session/ (e.g., prompt.ts and processor.ts), this specific loop-gate TOCTOU/race between buildLoopContext() and recordSyntheticBlock/recordSyntheticStop is intentionally handled via idempotence guards (re-check sigKey presence and hasStopped inside the record helpers). Do not treat the absence of a per-parent Effect.Mutex as a blocking review issue for this known edge case; the residual window should only create extra synthetic parts and must not change the "turn ends" contract. Respect the existing code comment documenting the trade-off, and only escalate to a full-mutex approach if the race is observed in practice.

Applied to files:

  • packages/opencode/src/session/prompt.ts
🔇 Additional comments (2)
packages/opencode/test/session/message-v2.test.ts (2)

857-910: Looks good.

These step-start boundary tests cover the split/drop behavior cleanly and match the existing toModelMessages contract.


1059-1081: Looks good.

This new OpenAI server_error case matches the current retryable error path and verifies both the retry flag and preserved response body.

Comment thread packages/opencode/src/session/prompt.ts Outdated
Comment thread packages/opencode/src/session/prompt.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request P3 Low priority upstream Tracked upstream or vendor behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant