feat: default LSP off, scope TS-ecosystem to nearest package#236
Merged
Conversation
Prepend tsconfig.json and package.json to lockfile root markers via new JavascriptPackageRoot helper, applied to TypeScript, Vue, ESLint, Svelte, Astro, and YamlLS. Prevents tsserver from analyzing the entire monorepo when only one package is in use. Filter Object.values(LSPServer) iteration in lsp/index.ts so the new helper export does not leak into the server registry. Refs #232
Reading a file no longer triggers lsp.touchFile. Aligns with industry baseline (Codex CLI, Gemini CLI, and Claude Code all skip this in their read paths). LSP work now happens only on edit/write/apply_patch where diagnostics are actually consumed. Refs #232
New service holds lspEnabled in an Effect Ref, defaulting to false. LSP and ToolRegistry layers will consume this value at state init in subsequent commits; the PawWork-side coordinator (Task 10) sets the Ref on toggle changes. Public helpers go through makeRuntime (sharing the singleton memoMap with AppRuntime) so the Ref is one cell across all consumers, while avoiding a circular import between Settings and AppRuntime. Refs #232
Replace cfg.lsp reads in lsp/index.ts state init with a Settings service read. PawWork is the sole source of truth for LSP enablement; opencode user config lsp records (custom server entries and per-server disabled flags) are no longer consulted at runtime. The cfg.lsp schema stays in place for upstream compatibility but is dead code under PawWork. Also includes: - Existing lifecycle/index LSP tests opt back in via Settings.setLspEnabled(true) in beforeEach since the default flipped to off. - CI Windows shard coverage updated to include test/settings.test.ts in the opencode-config-project shard (the contract test enforces that every test file is covered by exactly one shard). Refs #232
Drop the OPENCODE_EXPERIMENTAL_LSP_TOOL flag; gate lsp tool inclusion in builtin[] by lspEnabled at state-init time. All registry enumeration paths (tools, all, ids, named) read the same cached builtin[], so they stay consistent automatically. Layer dependency on Settings is wired through ToolRegistry.defaultLayer; manual session-test layer composers also provide Settings.defaultLayer. Refs #232
shutdownAll drains s.spawning Promises (raw JS Promises wrapped in Effect.promise that scope finalizers cannot cancel) and calls client.shutdown on every entry in s.clients. invalidate clears the InstanceState cache so the next access re-runs state init with fresh Settings.lspEnabled. Both methods are exposed via Service.of and as public re-exports so the PawWork coordinator (Task 10) can call them directly. Mock LSP services in session tests get matching no-op stubs. Refs #232
Clear the InstanceState cache so the next tools()/ids() call re-runs state init with fresh Settings.lspEnabled. Used by the PawWork coordinator (Task 10) after settings flips to make the lsp tool appear or disappear without a session restart. Refs #232
Add packages field to LSPServer.Info as the single source of truth for npm packages each server downloads. Export LSP_SERVER_PACKAGES set derived from those entries. In schedule()'s .catch branch, suppress s.broken.add when the error is Npm.InstallFailedError. Install failures (network/disk) are transient and should be retried on the next edit. Genuine spawn errors (process crash, init handshake) and missing tools (Module.resolve null) still poison s.broken. Refs #232
Define LSP.Event.InstallFailed via BusEvent.define so the type is included in the SSE event schema. Npm.add publishes the event when the failed package is in LSP_SERVER_PACKAGES; the lookup is performed through dynamic import to avoid an npm <-> lsp circular module load. App-side global-sync subscribes to the new event and shows an error toast (i18n keys land in Task 11). The SDK's generated event union does not yet include the new type, so the comparison casts to string; the cast can drop after the next sdk regen. Refs #232
Renderer settings store change calls window.api.setLspEnabled. Main process IPC handler "lsp-set-enabled" coordinates the runtime-side flip in order: 1. Settings.setLspEnabled writes the Ref 2. On going false, LSP.shutdownAll kills running processes 3. LSP.invalidate forces re-init with the new value 4. ToolRegistry.invalidate refreshes the lsp tool visibility The opencode runtime is in-process with Electron main, so the coordinator imports the namespaces directly via virtual:opencode-server. Settings/LSP/ToolRegistry are added to opencode's node entry exports. Refs #232
Default off. Toggling persists to settings store and triggers the
runtime-side coordinator (Task 10) via window.api.setLspEnabled to
invalidate caches and shut down processes.
Toggle row title is "Language Server Protocol (LSP)" / 语言服务器协议（LSP）
with the description framed in user-task terms ("Detect type errors and
symbol references when editing code"). Toast strings cover the
lsp.server.install.failed event consumer in global-sync.
Refs #232
LSP.invalidate and ToolRegistry.invalidate read InstanceState.directory, which requires an Instance ALS context. The IPC handler runs outside any Instance, so calls were throwing LocalContext.NotFound. Expose Instance.directories() to enumerate active instance roots, then loop them in the lsp-set-enabled handler and run shutdownAll/invalidate inside Instance.provide so each project's caches and processes are reset. Refs #232
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 57 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR implements user-controlled LSP (Language Server Protocol) enablement, adding a settings toggle to control language server activation across the application. It includes Settings module infrastructure, Electron IPC bridge implementation, LSP service lifecycle management (shutdown/invalidation), LSP install failure notifications, and comprehensive test coverage. Removes automatic LSP warming from file reads. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsUI as Settings UI<br/>(Renderer)
participant IPC as Electron IPC<br/>(Bridge)
participant MainProc as Main Process
participant Settings as Settings<br/>Service
participant LSP as LSP<br/>Service
participant ToolReg as Tool<br/>Registry
participant Instance as Instance<br/>(Per-project)
User->>SettingsUI: Toggle LSP Switch
SettingsUI->>IPC: invoke("lsp-set-enabled", true/false)
IPC->>MainProc: ipcMain.handle received
MainProc->>Settings: Settings.setLspEnabled(value)
Settings->>Settings: Update internal Ref<boolean>
MainProc->>Instance: Iterate all directories
Instance->>LSP: LSP.invalidate()<br/>(force re-init)
LSP->>LSP: Clear instance state
alt value is false
LSP->>LSP: LSP.shutdownAll()<br/>(stop all servers)
end
MainProc->>ToolReg: ToolRegistry.invalidate()
ToolReg->>ToolReg: Re-compute tool list<br/>based on new<br/>settings.lspEnabled()
MainProc->>IPC: Return Promise<void>
IPC->>SettingsUI: Resolve
SettingsUI->>User: UI reflects new state
sequenceDiagram
participant npm as npm<br/>Module
participant LSPPkgs as LSP_SERVER_<br/>PACKAGES
participant Bus as Event Bus
participant Renderer as Renderer<br/>(Toast UI)
npm->>npm: add(package) called
npm->>npm: Install fails
npm->>LSPPkgs: isInstallFailure?<br/>Check if pkg in set
alt Package is LSP server
npm->>Bus: Emit LSP.Event.InstallFailed<br/>{ pkg, error }
Bus->>Renderer: Dispatch event
Renderer->>Renderer: Render error toast<br/>with localized msg
else Not an LSP package
npm->>npm: Throw InstallFailedError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a user-facing toggle for the Language Server Protocol (LSP) within the application settings. Key changes include the addition of a new Settings service to manage the lspEnabled state, IPC handlers to synchronize this setting between the renderer and the main process, and updates to the LSP and Tool Registry services to dynamically enable or disable functionality based on the user's preference. Additionally, the PR includes localized strings for the new settings and error toasts for LSP installation failures. Feedback indicates a synchronization gap where the persisted setting in the renderer is not communicated to the main process upon application startup, potentially leaving the backend out of sync until the setting is manually toggled.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/app/src/context/settings.tsx`:
- Around line 244-248: The setter setLspEnabled currently calls
window.api.setLspEnabled(...) fire-and-forget which can leave UI state
inconsistent if the async IPC rejects; update setLspEnabled to await the promise
returned by window.api.setLspEnabled (or at least attach .then/.catch) and
handle rejection by logging the error and reverting the persisted UI state via
setStore("general", "lspEnabled", ...) (or otherwise synchronizing UI with the
runtime), ensuring no unhandled promise rejections and clear error logging.
In `@packages/app/src/i18n/en.ts`:
- Around line 837-840: The translation key "toast.lsp.installFailed.description"
uses single-brace tokens "{pkg}" and "{error}" but the project expects the
double-brace interpolation format; update the value for
toast.lsp.installFailed.description to use "{{pkg}}" and "{{error}}" instead so
runtime interpolation matches the existing token syntax.
In `@packages/app/src/i18n/zh.ts`:
- Around line 726-729: The toast.lsp.installFailed.description uses single-brace
tokens but the i18n system expects double-brace placeholders; update the value
for "toast.lsp.installFailed.description" to use {{pkg}} and {{error}} (e.g.
"toast.lsp.installFailed.description": "{{pkg}}: {{error}}") so the package and
error values are interpolated correctly by the i18n renderer.
In `@packages/desktop-electron/src/main/ipc.ts`:
- Around line 181-191: The loop that calls Instance.provide for each directory
can abort on the first failure, leaving some directories with stale
LSP/ToolRegistry state; update the logic that iterates Instance.directories() so
each Instance.provide(...) call is isolated (either run all provides with
Promise.allSettled or wrap each iteration in a try/catch) and ensure that on
failure you still continue processing remaining directories and still call
LSP.shutdownAll(), LSP.invalidate(), and ToolRegistry.invalidate() for each
instance as appropriate; reference Instance.directories(), Instance.provide(),
LSP.shutdownAll(), LSP.invalidate(), and ToolRegistry.invalidate() when applying
the change.
In `@packages/opencode/src/lsp/index.ts`:
- Around line 170-180: The LSP gate is effectively default-on because
Settings.defaultLayer seeds the ref with true; update the settings default so
lspEnabled() returns false unless explicitly enabled. Change the seed for
Settings.defaultLayer (the Ref initialized via Ref.make(true) in
packages/opencode/src/settings/index.ts) to Ref.make(false) or otherwise make
Settings.Service/Settings.lspEnabled() consult an opt-in flag (env/CLI) so LSP
is disabled by default; ensure ToolRegistry and any other callers of
Settings.Service (e.g. the LSP instantiation in InstanceState.make<State> and
the other occurrence flagged around line 515) use the revised default behavior.
- Around line 226-233: When a spawn() fails with Npm.InstallFailedError the code
must mark the server as temporarily broken to avoid retriggering installs on
every LSP interaction; update the catch in server.spawn() so that for both
general errors and InstallFailedError you add the server key to s.broken, and
for install failures also set a cooldown (e.g., record a nextRetry timestamp or
schedule removal from s.broken after a short backoff) so subsequent
touchFile/hover/definition calls skip immediate retries; reference s.broken,
server.spawn, and Npm.InstallFailedError when making the change.
In `@packages/opencode/test/lsp/lspEnabled.test.ts`:
- Around line 17-31: The test is manually wiring runtime via Instance.provide
and AppRuntime.runPromise; replace that pattern with the Effect test harness:
rewrite the test to use testEffect(...) with it.live(...) and swap manual
provisioning for provideTmpdirInstance(...) (or provideInstance(...) where
appropriate) so the Effect is run via the test harness and tmpdir/Instance
lifecycle is handled automatically; locate usages of Instance.provide,
AppRuntime.runPromise, Settings.Service, LSP.Service and LSP.touchFile in this
file and replace the outer Instance.provide + AppRuntime.runPromise wrapper with
a testEffect that yields the same Effect body and is provided with
provideTmpdirInstance (or provideInstance) so cleanup and live filesystem
behavior match the suite.
- Around line 39-63: The test enables Settings.lspEnabled but never resets it,
causing later tests to inherit lspEnabled=true; in the finally block after
spy.mockRestore() ensure you reset the shared setting by invoking the Settings
service and calling settings.setLspEnabled(false) (use the same runtime pattern
as the test—e.g., run a short Effect/AppRuntime call that accesses
Settings.Service and calls setLspEnabled(false)) before disposing instances so
the global memoized layer is returned to its original state.
In `@packages/opencode/test/lsp/server.test.ts`:
- Around line 17-25: Replace the manual makeFixture + fs.mkdtempSync and the
direct Instance.provide(...) usage with the shared test helpers: use await using
tmp = await tmpdir(...) to create the temp tree, write files into tmp.path
(replacing makeFixture behavior), and create the LSP instance with
provideTmpdirInstance(...) (or provideInstance(...) if no tmpdir-specific helper
exists) so the temp dir and instance are automatically cleaned up; locate
makeFixture and any Instance.provide(...) calls in this test and swap them to
use tmpdir + provideTmpdirInstance/provideInstance accordingly.
In `@packages/opencode/test/settings.test.ts`:
- Around line 6-27: Tests use AppRuntime.runPromise and mutate shared
Settings.Service state (setLspEnabled) without cleanup; switch to the standard
Effect test harness by declaring const it = testEffect(...) at the top and
convert each test to use it(...) with Effect.gen bodies, and ensure mutated
state is restored between tests (for example, call
Settings.Service.setLspEnabled(false) in a finally/ensuring block or use a
scoped Layer that provides a fresh Settings.Service per test) so
Settings.Service.lspEnabled and Settings.Service.setLspEnabled are exercised
with isolated state.
🪄 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: 05d7b03d-8e0a-430c-afdd-2033ee660b4b
📒 Files selected for processing (30)
.github/workflows/ci.ymlpackages/app/src/app.tsxpackages/app/src/components/settings-general.tsxpackages/app/src/context/global-sync.tsxpackages/app/src/context/settings.tsxpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/env.d.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/preload/types.tspackages/opencode/src/effect/app-runtime.tspackages/opencode/src/lsp/index.tspackages/opencode/src/lsp/server.tspackages/opencode/src/node.tspackages/opencode/src/npm/index.tspackages/opencode/src/project/instance.tspackages/opencode/src/settings/index.tspackages/opencode/src/tool/read.tspackages/opencode/src/tool/registry.tspackages/opencode/test/github/ci-workflow.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lifecycle.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/settings.test.tspackages/opencode/test/tool/read.test.tspackages/opencode/test/tool/registry.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). (10)
- GitHub Check: smoke-macos-arm64
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-opencode
- GitHub Check: unit-desktop
- GitHub Check: e2e-artifacts
- GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (5)
packages/opencode/**/*.ts
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.ts: UseEffect.gen(function* () { ... })for Effect composition
UseEffect.fn("Domain.method")for named/traced effects andEffect.fnUntracedfor internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer.pipe()wrappers
UseEffect.callbackfor callback-based APIs
PreferDateTime.nowAsDateovernew Date(yield* Clock.currentTimeMillis)when you need aDatein Effect code
UseSchema.Classfor multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
UseSchema.TaggedErrorClassfor typed errors in Effect schemas
UseSchema.Defectinstead ofunknownfor defect-like causes in Effect code
InEffect.gen/Effect.fn, preferyield* new MyError(...)overyield* Effect.fail(new MyError(...))for direct early-failure branches
UsemakeRuntimefromsrc/effect/run-service.tsfor all services; it returns{ runPromise, runFork, runCallback }backed by a sharedmemoMapthat deduplicates layers
UseInstanceStatefromsrc/effect/instance-state.tsfor per-directory or per-project state that needs per-instance cleanup; do work directly in theInstanceState.makeclosure whereScopedCachehandles run-once semantics
UseEffect.addFinalizerorEffect.acquireReleaseinside theInstanceState.makeclosure for cleanup (subscriptions, process teardown, etc.)
UseEffect.forkScopedinside theInstanceState.makeclosure for background stream consumers — the fiber is interrupted when the instance is disposed
PreferFileSystem.FileSysteminstead of rawfs/promisesfor effectful file I/O in Effect services
PreferChildProcessSpawner.ChildProcessSpawnerwithChildProcess.make(...)instead of custom process wrappers in Effect services
PreferHttpClient.HttpClientinstead of rawfetchin Effect services
PreferPath.Path,Config,Clock, andDateTimeservices when those concerns are already inside Effect code
For backgroun...
Files:
packages/opencode/src/effect/app-runtime.tspackages/opencode/src/project/instance.tspackages/opencode/test/github/ci-workflow.test.tspackages/opencode/test/tool/read.test.tspackages/opencode/src/node.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/settings.test.tspackages/opencode/src/tool/read.tspackages/opencode/test/tool/registry.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/src/settings/index.tspackages/opencode/test/lsp/index.test.tspackages/opencode/src/npm/index.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.tspackages/opencode/src/tool/registry.tspackages/opencode/src/lsp/index.tspackages/opencode/src/lsp/server.ts
packages/desktop-electron/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Renderer process should only call
window.apifromsrc/preload
Files:
packages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/env.d.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/app.tsxpackages/app/src/context/global-sync.tsxpackages/app/src/i18n/en.tspackages/app/src/components/settings-general.tsxpackages/app/src/context/settings.tsxpackages/app/src/i18n/zh.ts
packages/opencode/test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (packages/opencode/test/AGENTS.md)
packages/opencode/test/**/*.test.{ts,tsx}: Use thetmpdirfunction fromfixture/fixture.tsto create temporary directories for tests with automatic cleanup. Useawait usingsyntax to ensure automatic cleanup when the variable goes out of scope.
When using thetmpdirfunction with git repository support, pass thegit: trueoption to initialize a git repo with a root commit.
Use theconfigoption intmpdirto write anopencode.jsonconfig file during test setup by passing a partial Config.Info object.
Use theinitoption intmpdirto define custom setup functions that can return extra data accessible viatmp.extra, and use thedisposeoption for custom cleanup logic.
UsetestEffect(...)fromtest/lib/effect.tsfor tests that exercise Effect services or Effect-based workflows.
Useit.effect(...)when the test should run withTestClockandTestConsole. Useit.live(...)when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers fromfixture/fixture.tsover building manual runtimes in tests: usetmpdirScoped()for scoped temp directories,provideInstance(dir)(effect)for low-level binding without directory creation,provideTmpdirInstance(...)for single temp instance binding, orprovideTmpdirServer(...)for tests that also need the test LLM server.
Defineconst it = testEffect(...)near the top of the test file and keep the test body insideEffect.gen(function* () { ... }). Yield services directly withyield* MyService.Serviceoryield* MyTool.
Avoid customManagedRuntime,attach(...), or ad hocrun(...)wrappers in Effect tests whentestEffect(...)already provides the runtime.
When a test needs instance-local state, preferprovideTmpdirInstance(...)orprovideInstance(...)over manualInstance.provide(...)inside Promise-style tests.
Files:
packages/opencode/test/github/ci-workflow.test.tspackages/opencode/test/tool/read.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/settings.test.tspackages/opencode/test/tool/registry.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.ts
packages/desktop-electron/src/main/ipc.ts
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Main process should register IPC handlers in
src/main/ipc.ts
Files:
packages/desktop-electron/src/main/ipc.ts
🧠 Learnings (51)
📚 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 `makeRuntime` from `src/effect/run-service.ts` for all services; it returns `{ runPromise, runFork, runCallback }` backed by a shared `memoMap` that deduplicates layers
Applied to files:
packages/opencode/src/effect/app-runtime.tspackages/opencode/test/settings.test.tspackages/opencode/src/settings/index.tspackages/opencode/src/lsp/index.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} : Avoid custom `ManagedRuntime`, `attach(...)`, or ad hoc `run(...)` wrappers in Effect tests when `testEffect(...)` already provides the runtime.
Applied to files:
packages/opencode/src/effect/app-runtime.tspackages/opencode/test/tool/read.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/settings.test.tspackages/opencode/test/lsp/lspEnabled.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/src/effect/app-runtime.tspackages/opencode/test/tool/read.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/settings.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.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/src/effect/app-runtime.tspackages/opencode/test/tool/read.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/settings.test.tspackages/opencode/src/tool/read.tspackages/opencode/test/tool/registry.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/src/settings/index.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/desktop-electron/src/main/env.d.tspackages/opencode/test/lsp/lifecycle.test.tspackages/opencode/src/tool/registry.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 : Prefer `Path.Path`, `Config`, `Clock`, and `DateTime` services when those concerns are already inside Effect code
Applied to files:
packages/opencode/src/effect/app-runtime.tspackages/opencode/src/tool/read.tspackages/opencode/src/settings/index.tspackages/opencode/src/lsp/index.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use `testEffect(...)` from `test/lib/effect.ts` for tests that exercise Effect services or Effect-based workflows.
Applied to files:
packages/opencode/src/effect/app-runtime.tspackages/opencode/test/github/ci-workflow.test.tspackages/opencode/test/tool/read.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/settings.test.tspackages/opencode/src/tool/read.tspackages/opencode/src/settings/index.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `config` option in `tmpdir` to write an `opencode.json` config file during test setup by passing a partial Config.Info object.
Applied to files:
.github/workflows/ci.ymlpackages/opencode/test/github/ci-workflow.test.tspackages/opencode/test/lsp/server.test.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 `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
Applied to files:
packages/opencode/src/project/instance.tspackages/opencode/src/tool/read.tspackages/opencode/test/tool/registry.test.tspackages/opencode/src/tool/registry.tspackages/opencode/src/lsp/index.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : When a test needs instance-local state, prefer `provideTmpdirInstance(...)` or `provideInstance(...)` over manual `Instance.provide(...)` inside Promise-style tests.
Applied to files:
packages/opencode/src/project/instance.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/settings.test.tspackages/opencode/test/tool/registry.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.ts
📚 Learning: 2026-04-20T14:36:08.774Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/desktop-electron/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:08.774Z
Learning: Applies to packages/desktop-electron/src/**/*.{ts,tsx,js,jsx} : Renderer process should only call `window.api` from `src/preload`
Applied to files:
packages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/preload/index.ts
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.
Applied to files:
packages/app/src/app.tsxpackages/app/src/context/global-sync.tsxpackages/app/src/i18n/en.tspackages/app/src/components/settings-general.tsxpackages/app/src/context/settings.tsxpackages/app/src/i18n/zh.ts
📚 Learning: 2026-04-23T15:10:21.635Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 191
File: packages/app/src/components/session/pawwork-skill-meta.ts:38-39
Timestamp: 2026-04-23T15:10:21.635Z
Learning: This repo configures Tailwind v4 with `--color-*: initial`, which effectively breaks standard Tailwind palette utilities (e.g., `text-violet-500` can resolve to no CSS variable and render as a no-op/black). For brand/accent colors that are not backed by semantic design tokens, use inline styles with the exact hex value (e.g., `style={{ color: '#8B5FBF' }}` / `homeIconStyle: { color: '#8B5FBF' }`) and add a short comment explaining that Tailwind palette utilities won’t work due to the `--color-*: initial` setup. Do not suggest replacing these inline hex colors with Tailwind palette classes anywhere in this repo.
Applied to files:
packages/app/src/app.tsxpackages/app/src/context/global-sync.tsxpackages/app/src/i18n/en.tspackages/app/src/components/settings-general.tsxpackages/app/src/context/settings.tsxpackages/app/src/i18n/zh.ts
📚 Learning: 2026-04-25T09:19:30.734Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 231
File: packages/desktop-electron/src/main/index.ts:537-537
Timestamp: 2026-04-25T09:19:30.734Z
Learning: In Astro-Han/pawwork (`packages/desktop-electron/src/main/`), the convention for IPC registration is that `index.ts` (the bootstrap entry) directly calls each registration function. `registerIpcHandlers({...})` in `src/main/ipc.ts` is a single fat options-bag with inline handler bodies. New, cohesive IPC features (e.g., About) are placed in sub-modules like `src/main/ipc/about.ts`, which own their own exported types, helpers, and a `register*Ipc()` function. The bootstrap in `index.ts` calls each `register*Ipc()` directly — this is intentional, not fragmentation. Do NOT suggest routing sub-module IPC registrations through `ipc.ts`.
Applied to files:
packages/desktop-electron/src/preload/index.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : When using the `tmpdir` function with git repository support, pass the `git: true` option to initialize a git repo with a root commit.
Applied to files:
packages/opencode/test/github/ci-workflow.test.tspackages/opencode/test/lsp/server.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `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.
Applied to files:
packages/opencode/test/github/ci-workflow.test.tspackages/opencode/test/tool/read.test.tspackages/opencode/test/tool/registry.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lifecycle.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `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.
Applied to files:
packages/opencode/test/github/ci-workflow.test.tspackages/opencode/test/tool/read.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/tool/registry.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lifecycle.test.ts
📚 Learning: 2026-04-22T08:49:47.800Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/opencode/test/github/ci-workflow.test.tspackages/opencode/test/tool/read.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.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/github/ci-workflow.test.tspackages/opencode/test/tool/read.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/settings.test.tspackages/opencode/test/tool/registry.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Prefer the `project` fixture for tests that need a dedicated project with LLM mocking
Applied to files:
packages/opencode/test/github/ci-workflow.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.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/app/src/context/global-sync.tsx
📚 Learning: 2026-04-20T17:03:40.214Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 73
File: packages/opencode/src/cli/cmd/tui/context/sync.tsx:486-489
Timestamp: 2026-04-20T17:03:40.214Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/cli/cmd/tui/context/sync.tsx`), `sync.ready` returning `true` when `process.env.OPENCODE_FAST_BOOT` is set is intentional. The plugin-facing data properties `state.config` (initialized to `{}`) and `state.provider` (initialized to `[]`) expose safe-empty defaults, so they are safe to access before bootstrap completes. Do not flag these as needing null-guards or conditional patterns to match `vcs` — the difference is intentional because `vcs` starts as `undefined` while the others have initialized defaults. Changing this would alter the plugin API contract without a concrete failing case.
Applied to files:
packages/app/src/context/global-sync.tsx
📚 Learning: 2026-04-24T17:12:23.931Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/desktop-electron/electron-builder.config.ts:14-18
Timestamp: 2026-04-24T17:12:23.931Z
Learning: In Astro-Han/pawwork, the `localizedMacDisplayNameByChannel` map in `packages/desktop-electron/electron-builder.config.ts` is intentionally kept separate from `localizedAppDisplayName` in `packages/desktop-electron/src/main/app-display-name.ts`. The former is a build-time packaging helper; the latter is a runtime UI helper that localizes the current app name by locale. Coupling them would introduce a build-time dependency on runtime main logic. Do not suggest deduplicating or sharing this mapping — the explicit local table is covered by focused regression tests in `electron-builder-app-update.test.ts`.
Applied to files:
packages/app/src/context/global-sync.tsxpackages/desktop-electron/src/main/env.d.tspackages/app/src/i18n/zh.ts
📚 Learning: 2026-04-22T05:32:29.012Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 98
File: packages/desktop-electron/src/main/menu-labels.ts:1-2
Timestamp: 2026-04-22T05:32:29.012Z
Learning: In Astro-Han/pawwork, the app i18n layer (`packages/app/src/i18n/`) only contains `en.ts` and `zh.ts`, and `normalizeLocale` (in `packages/app/src/context/language.tsx`) only returns `"en"` or `"zh"`. The desktop `MenuLocale = "en" | "zh"` union in `packages/desktop-electron/src/main/menu-labels.ts` is intentionally limited to these two locales and is not a broader restriction — do not flag it as overly restrictive or suggest adding other locales.
Applied to files:
packages/app/src/context/global-sync.tsxpackages/app/src/i18n/en.tspackages/desktop-electron/src/main/env.d.tspackages/app/src/i18n/zh.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/tool/read.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/settings.test.tspackages/opencode/src/tool/read.tspackages/opencode/test/tool/registry.test.tspackages/opencode/src/settings/index.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.tspackages/opencode/src/tool/registry.tspackages/opencode/src/lsp/index.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/packages/app/src/testing/**/*.ts : Test-only hooks must be inert unless explicitly enabled and should not add normal-runtime listeners, reactive subscriptions, or per-update allocations
Applied to files:
packages/opencode/test/tool/read.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/settings.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use `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.
Applied to files:
packages/opencode/test/tool/read.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/settings.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Import test utilities from `../fixtures` instead of `playwright/test`
Applied to files:
packages/opencode/test/tool/read.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/lsp/server.test.tspackages/opencode/test/lsp/index.test.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 `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
Applied to files:
packages/opencode/test/tool/read.test.tspackages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/tool/read.tspackages/opencode/src/settings/index.ts
📚 Learning: 2026-04-24T17:08:44.294Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:44.294Z
Learning: In Astro-Han/pawwork PR `#224`, the first-occurrence `PawWork 爪印` branding rule originally specified in issue `#196` was superseded by an updated Chinese-branding spec. On all zh UI surfaces in `packages/app/src/i18n/zh.ts` (e.g., `dialog.model.unpaid.freeModels.title`, `session.new.subtitle`, `sidebar.gettingStarted.line1`), the correct and intentional target is fully localized `爪印` branding — no `PawWork` prefix. Do NOT flag these strings as missing the first-occurrence `PawWork 爪印` rule in future reviews.
Applied to files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use fixture-managed cleanup with `withSession(sdk, title, callback)` for temporary sessions instead of calling `sdk.session.delete(...)` directly
Applied to files:
packages/opencode/test/session/snapshot-tool-race.test.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 `Effect.addFinalizer` or `Effect.acquireRelease` inside the `InstanceState.make` closure for cleanup (subscriptions, process teardown, etc.)
Applied to files:
packages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/lsp/index.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 `Effect.cached` when multiple concurrent callers should share a single in-flight computation rather than storing `Fiber | undefined` or `Promise | undefined` manually
Applied to files:
packages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/tool/read.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 `Effect.gen(function* () { ... })` for Effect composition
Applied to files:
packages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/tool/read.tspackages/opencode/src/settings/index.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 `Effect.callback` for callback-based APIs
Applied to files:
packages/opencode/test/session/snapshot-tool-race.test.tspackages/opencode/test/session/prompt-effect.test.ts
📚 Learning: 2026-04-24T03:51:54.050Z
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:54.050Z
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/test/session/prompt-effect.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/prompt-effect.test.ts
📚 Learning: 2026-04-23T15:25:31.118Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/e2e/sidebar/sidebar-leading-slot.spec.ts:5-55
Timestamp: 2026-04-23T15:25:31.118Z
Learning: In Astro-Han/pawwork E2E tests, reaching a real "running" session state requires the `project` fixture (for model bootstrap) plus `llm.wait(1)` orchestration. The bare `sdk` fixture used by `withSession` does not trigger an LLM call even with `agent: "build"` + `system` prompt set via `sdk.session.promptAsync`, so simulating running state is not lightweight in the current test infrastructure.
Applied to files:
packages/opencode/test/session/prompt-effect.test.ts
📚 Learning: 2026-04-25T09:19:30.734Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 231
File: packages/desktop-electron/src/main/index.ts:537-537
Timestamp: 2026-04-25T09:19:30.734Z
Learning: In Astro-Han/pawwork (packages/desktop-electron/src/main/), follow the IPC registration convention: the bootstrap entry (packages/desktop-electron/src/main/index.ts) should directly call each module’s exported register*Ipc() function. Do not route/centralize these sub-module IPC registrations through src/main/ipc.ts. Keep sub-module IPC features cohesive (e.g., src/main/ipc/about.ts should own its types/helpers and expose register*Ipc()), and allow index.ts to aggregate by calling each register*Ipc() directly.
Applied to files:
packages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/env.d.ts
📚 Learning: 2026-04-25T12:52:32.462Z
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:32.462Z
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/desktop-electron/src/main/ipc.tspackages/opencode/src/npm/index.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test one feature per test file
Applied to files:
packages/opencode/test/settings.test.tspackages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.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 : Prefer `FileSystem.FileSystem` instead of raw `fs/promises` for effectful file I/O in Effect services
Applied to files:
packages/opencode/src/tool/read.tspackages/opencode/src/settings/index.tspackages/opencode/src/lsp/server.ts
📚 Learning: 2026-04-25T12:52:35.671Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/opencode/src/session/export.ts:24-31
Timestamp: 2026-04-25T12:52:35.671Z
Learning: In `packages/opencode/src/session/export.ts`, the `hashFile` async helper intentionally uses raw `node:fs/promises` (`fs.readFile`) rather than `FileSystem.FileSystem`. It is a small, self-contained helper called inside `Effect.promise(...)` with broad failure tolerance (try/catch returning `undefined`). Using `FileSystem.FileSystem` here would add unnecessary layers without payoff. The `FileSystem.FileSystem` preference applies to Effect-service-style operations, not thin async boundary helpers like `hashFile`.
Applied to files:
packages/opencode/src/tool/read.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 : Prefer `ChildProcessSpawner.ChildProcessSpawner` with `ChildProcess.make(...)` instead of custom process wrappers in Effect services
Applied to files:
packages/opencode/src/tool/read.tspackages/opencode/src/lsp/index.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 : For background loops or scheduled tasks, use `Effect.repeat` or `Effect.schedule` with `Effect.forkScoped` in the layer definition
Applied to files:
packages/opencode/src/tool/read.tspackages/opencode/src/settings/index.tspackages/opencode/src/lsp/index.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 : In `Effect.gen` / `Effect.fn`, prefer `yield* new MyError(...)` over `yield* Effect.fail(new MyError(...))` for direct early-failure branches
Applied to files:
packages/opencode/src/tool/read.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : When validating routing, assert against canonical or resolved workspace slugs using shared helpers from `../actions` to account for Windows canonicalization
Applied to files:
packages/opencode/test/lsp/server.test.ts
📚 Learning: 2026-04-25T12:52:47.074Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/opencode/src/share/session.ts:27-27
Timestamp: 2026-04-25T12:52:47.074Z
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/src/settings/index.tspackages/opencode/test/lsp/lspEnabled.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use lowercase, descriptive test names (e.g., 'sidebar can be toggled')
Applied to files:
packages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Prefer fluent helpers and drivers when they make intent obvious and reduce locator-heavy noise in tests
Applied to files:
packages/opencode/test/lsp/index.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : In terminal tests, type through the browser using `runTerminal()` and `waitTerminalReady()` instead of writing to the PTY through the SDK
Applied to files:
packages/opencode/test/lsp/index.test.tspackages/opencode/test/lsp/lspEnabled.test.tspackages/opencode/test/lsp/lifecycle.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use `expect.poll(...)` for probing mock or backend state rather than transient DOM visibility
Applied to files:
packages/opencode/test/lsp/lifecycle.test.ts
🔇 Additional comments (22)
packages/opencode/src/project/instance.ts (1)
78-80: Good addition: explicit cached-instance directory accessor.Exposing a snapshot of cached directories here is a clean way to support lifecycle coordination (like invalidate/shutdown flows) without leaking
cachedirectly.packages/opencode/src/settings/index.ts (1)
12-29: Settings service wiring looks correct and coherent.
Ref<boolean>(false)+ service accessors +makeRuntimewrappers cleanly implement the default-off feature flag with a simple integration surface..github/workflows/ci.yml (1)
399-399: Nice CI shard update.Including
test/settings.test.tsin this shard keeps Windows advisory coverage aligned with the new settings behavior tests.packages/opencode/src/effect/app-runtime.ts (1)
36-36: App runtime layer integration is correct.Adding
Settings.defaultLayerintoAppLayeris the right runtime wiring step for feature-flagged LSP behavior.Also applies to: 59-59
packages/opencode/test/github/ci-workflow.test.ts (1)
57-57: Workflow contract test stays in sync.Good update to the pinned shard command so the guard test reflects the CI workflow change exactly.
packages/opencode/test/lsp/index.test.ts (1)
10-15: Per-test LSP flag setup/teardown is a solid test isolation change.Enabling in
beforeEachand resetting inafterEachmakes these spawn assertions deterministic under the new default-off setting.packages/app/src/app.tsx (1)
85-85: Type surface update is appropriate.The optional
setLspEnabledbridge typing cleanly matches renderer usage while remaining safe for environments wherewindow.apiis absent.packages/opencode/test/tool/read.test.ts (1)
578-588: Useful regression guard for read-path LSP warming removal.This test directly protects the intended behavior change by preventing accidental reintroduction of
touchFile/warm-hook logic inread.ts.packages/opencode/test/session/snapshot-tool-race.test.ts (1)
44-44: Good test harness alignment with new LSP/Settings requirements.The added
Settingslayer and theshutdownAll/invalidatemock methods keep this test’s dependency graph and LSP mock contract consistent with the updated runtime behavior.Also applies to: 102-103, 132-132
packages/desktop-electron/src/preload/types.ts (1)
97-97: API typing update is clean and consistent.Adding
setLspEnabledtoElectronAPIkeeps the preload contract explicit for the new IPC pathway.packages/app/src/context/global-sync.tsx (1)
344-354: Good addition of user-visible LSP install failure feedback.This event handler cleanly maps the new bus event into an error toast with localized title/description.
packages/desktop-electron/src/preload/index.ts (1)
83-83: Preload IPC bridge wiring looks correct.The new
setLspEnabledmethod follows the established preload pattern and keeps renderer/main separation intact.packages/opencode/src/tool/read.ts (1)
2-2: Import cleanup is aligned with the read-path behavior change.This keeps the module surface focused after removing LSP warming from the read flow.
packages/opencode/src/node.ts (1)
7-10: Public export surface update is appropriate.These re-exports expose the new runtime control points needed by the desktop integration layer.
packages/app/src/components/settings-general.tsx (1)
302-312: LSP toggle row integration looks consistent and correct.The new row follows the existing settings row pattern and correctly wires
checked/onChangetosettings.general.packages/opencode/test/session/prompt-effect.test.ts (2)
150-151: LSP test double now correctly mirrors lifecycle APIs.Adding
shutdownAllandinvalidatekeeps the mock aligned with the updatedLSP.Servicecontract and prevents interface drift in this suite.
177-180: Good dependency wiring for ToolRegistry test runtime.Providing
Settings.defaultLayerhere is the right fix after gatinglsptool visibility through settings.packages/opencode/test/tool/registry.test.ts (1)
501-553: Coverage for LSP-gated registry behavior is solid.These cases validate both static toggles and post-
invalidate()recomputation, which directly protects the new runtime toggle path.packages/opencode/test/lsp/lifecycle.test.ts (1)
26-34: Lifecycle setup now correctly controls LSP enablement per test.Enabling in
beforeEachand restoring inafterEachmakes these tests deterministic against the new settings gate.packages/opencode/src/lsp/server.ts (2)
35-43: Root marker consolidation is a strong improvement.Using
JavascriptPackageRoot()across TS/Vue/ESLint/Svelte/Astro/YamlLS makes root resolution behavior consistent and directly addresses monorepo over-expansion.Also applies to: 109-109, 138-138, 166-166, 1020-1020, 1048-1048, 1307-1307
1978-1982:LSP_SERVER_PACKAGESas a derived set is a good single source of truth.This centralization reduces drift between server definitions and npm-install failure handling paths.
packages/opencode/src/npm/index.ts (1)
31-45: Install-failure signaling is well integrated.Emitting
lsp.server.install.failedonly for known LSP server packages, then rethrowingInstallFailedError, cleanly supports retryable failures plus user-facing visibility.Also applies to: 118-132
Astro-Han
commented
Apr 25, 2026
Astro-Han
left a comment
Owner
Author
There was a problem hiding this comment.
Review pass: three concrete inline findings, graded by P-level.
Repo i18n strings interpolate via {{var}}; the new toast description
was using single braces, so {pkg} and {error} would render literally
instead of being replaced with the failed package and error message.
Refs #232
…PC failure Replace fire-and-forget IPC in setLspEnabled with a createEffect that runs after the persisted store rehydrates. The effect sends the current value to the Electron main process whenever it changes (covering both the startup case where the renderer loaded a persisted true while the main-process Ref defaulted to false, and the runtime toggle case). On IPC rejection it flips the store back so the UI reflects the actual runtime state instead of drifting silently. Refs #232
Switch the per-Instance loop to Promise.allSettled so a failure on one project does not leave subsequent projects with stale LSP/ToolRegistry caches. Rejected results are logged with the directory but do not abort the toggle for the remaining instances. Refs #232
Track per-server-key timestamps in a new s.installCooldownUntil map. After a Npm.InstallFailedError the entry stays out of s.broken (so the user can recover once the network or registry is back) but is skipped for INSTALL_RETRY_COOLDOWN_MS (60s). This prevents every subsequent touchFile/hover/definition from re-triggering an arborist install and spamming lsp.server.install.failed toasts on offline machines. Refs #232
Convert test/settings.test.ts, test/lsp/server.test.ts, and test/lsp/lspEnabled.test.ts to testEffect(...) + provideTmpdirInstance per packages/opencode/test/AGENTS.md. Spy-based gates keep bun:test spyOn since it composes with Effect just fine. Each test that mutates Settings.lspEnabled now resets it to false in finally so later tests do not inherit the toggle through the singleton memoMap. Also widen the install-failure contract assertion to match the new cooldown branch shape (s.installCooldownUntil + s.broken.add(key)). Refs #232
Typescript and Astro language servers were always resolving typescript/lib/tsserver.js from Instance.directory, even though the NearestRoot fix already narrowed the spawn cwd to the nearest package. In a monorepo where packages/app installs its own typescript, the tsserver SDK could mismatch the package's version. Try the resolved package root first and fall back to the worktree root only when the package does not have its own typescript install (covers hoisted monorepos). Refs #232
The renderer caches LSP status and only refreshes on lsp.updated. After LSP.shutdownAll kills clients, the cached status would otherwise keep showing the previous connected servers in the popover until the next unrelated event. Publish Updated when shutdown actually had clients to drop, so the toggle-off path stays visually consistent with the runtime state. Refs #232
7 tasks
This was referenced Apr 27, 2026
This was referenced Jun 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make LSP opt-in (default off) with a Settings → General toggle, and prevent the TypeScript LSP from blowing up to monorepo-root scope when active.
Settings.ServiceholdslspEnabledin an EffectRef; LSP and ToolRegistry layers read it at state-init time so the lsp tool and server registry both flip together.window.api.setLspEnabled→ Electron main IPC handler enters each activeInstancescope, runsLSP.shutdownAll(only on going false), thenLSP.invalidateandToolRegistry.invalidateso caches andtsserverprocesses are reset per-project.JavascriptPackageRoothelper prependstsconfig.jsonandpackage.jsonto lockfile root markers, applied to TypeScript / Vue / ESLint / Svelte / Astro / YamlLS.read.tsno longer triggerslsp.touchFileon read (aligns with Codex CLI / Gemini CLI / Claude Code baseline).LSPServer.Info.packagesbecomes the single source of truth for npm-installed servers; install failures fromNpm.addare classified as transient and skips.brokenpoison so the next edit can retry.lsp.server.install.failedbus event surfaces an error toast in the renderer.Why
Issue #232: opening PawWork's own monorepo caused
tsserverto balloon to ~3.4 GB / 97% CPU because the TS server treated the worktree root (lockfile) as the project root. Two failure modes were tangled: (1) LSP cost was unconditional even when users did not need diagnostics, (2) TS root resolution found the lockfile before the nearesttsconfig.json. The toggle gives users the off switch; the NearestRoot fix keeps the on path bounded to one package.Related Issue
Closes #232
How To Verify
Manual smoke (run on macOS, in
bun run dev:desktop):.ts→ps aux | grep -iE "language-server|tsserver|typescript"empty.packages/app/src/app.tsx→ exactly onetypescript-language-serverplus itstsserver.jsworkers, all rooted atpackages/app/, RSS well under 1 GB.Screenshots or Recordings
Settings → General toggle row added at the bottom of the existing list; copy is
Language Server Protocol (LSP)/Detect type errors and symbol references when editing code. No layout reflow.Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
Release Notes
New Features
Improvements