fix: harden websearch follow-up states - #274
Conversation
Astro-Han
added
bug
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds saved-key "quota exceeded" handling end-to-end: new i18n keys and UI rendering, dialog status callback wiring, credential versioning threaded through auth and MCP error paths, IPC rollback on tool invalidation, and tests covering version-aware failures and saved-quota behavior. Changes
Sequence DiagramsequenceDiagram
participant User
participant Dialog as DialogConnectWebSearch
participant Settings as SettingsGeneral
participant Auth as WebSearchAuth
participant MCP as McpExa
participant IPC as ElectronMain
User->>Dialog: Save or remove credential
Dialog->>Auth: saveKey / removeKey (persist credentialVersion)
Auth->>MCP: make call (includes credential + credentialVersion)
MCP-->>Auth: success or Failure (includes credentialVersion)
Auth->>Auth: update saved metadata (status, credentialVersion)
Dialog->>Settings: props.onStatusChanged?()
Settings->>Auth: webSearchStatusActions.refetch()
Settings-->>User: re-render (chip: savedQuota if applicable)
User->>Settings: Toggle web search enabled
Settings->>IPC: send websearch-set-enabled
IPC->>IPC: invalidateWebSearchTools(allInstances)
IPC-->>Settings: success or error (rollback invalidation on failure)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces tracking for Exa API key search quotas and implements a versioning system for credentials to ensure that failure states are only applied to the specific key version that triggered them. It also adds a rollback mechanism to the desktop IPC handler for web search activation and updates the UI to handle saved key quota exhaustion. Feedback was provided regarding a logical inconsistency in the saveKey function, which prevents initial key storage when an environment variable is present but allows subsequent updates, potentially leading to a confusing user experience.
354a15e to
10553d3
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/components/dialog-connect-websearch.tsx`:
- Around line 34-35: The title and body branch for saved-state conflict when
both s.needsAttention and s.quotaExceeded are true; unify precedence by checking
s.quotaExceeded before s.needsAttention in both the title branch (the lines
using if (s.source === "saved" && ...) that return
language.t("dialog.websearch.title...")) and the body branch (the corresponding
saved-state checks around the body rendering), so that when s.source === "saved"
the quotaExceeded case is handled first and the needsAttention case is fallback.
🪄 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: 2a5af4e8-4a63-4146-8162-74e431eecbf5
📒 Files selected for processing (14)
packages/app/src/components/dialog-connect-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch.tsxpackages/app/src/components/settings-general.tsxpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/session/settings-websearch-source.test.tspackages/app/src/pages/session/websearch-toasts.test.tspackages/app/src/pages/session/websearch-toasts.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/opencode/src/tool/mcp-exa.tspackages/opencode/src/tool/websearch-auth.tspackages/opencode/test/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.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). (1)
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (5)
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/pages/session/settings-websearch-source.test.tspackages/app/src/pages/session/websearch-toasts.test.tspackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/session/websearch-toasts.tspackages/app/src/components/settings-general.tsxpackages/app/src/components/dialog-connect-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch.tsx
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/main/websearch-ipc-source.test.tspackages/desktop-electron/src/main/ipc.ts
packages/opencode/**/*.ts
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.ts: UseEffect.gen(function* () { ... })for Effect composition
UseEffect.fn("Domain.method")for named/traced effects andEffect.fnUntracedfor internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer.pipe()wrappers
UseEffect.callbackfor callback-based APIs
PreferDateTime.nowAsDateovernew Date(yield* Clock.currentTimeMillis)when you need aDatein Effect code
UseSchema.Classfor multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
UseSchema.TaggedErrorClassfor typed errors in Effect schemas
UseSchema.Defectinstead ofunknownfor defect-like causes in Effect code
InEffect.gen/Effect.fn, preferyield* new MyError(...)overyield* Effect.fail(new MyError(...))for direct early-failure branches
UsemakeRuntimefromsrc/effect/run-service.tsfor all services; it returns{ runPromise, runFork, runCallback }backed by a sharedmemoMapthat deduplicates layers
UseInstanceStatefromsrc/effect/instance-state.tsfor per-directory or per-project state that needs per-instance cleanup; do work directly in theInstanceState.makeclosure whereScopedCachehandles run-once semantics
UseEffect.addFinalizerorEffect.acquireReleaseinside theInstanceState.makeclosure for cleanup (subscriptions, process teardown, etc.)
UseEffect.forkScopedinside theInstanceState.makeclosure for background stream consumers — the fiber is interrupted when the instance is disposed
PreferFileSystem.FileSysteminstead of rawfs/promisesfor effectful file I/O in Effect services
PreferChildProcessSpawner.ChildProcessSpawnerwithChildProcess.make(...)instead of custom process wrappers in Effect services
PreferHttpClient.HttpClientinstead of rawfetchin Effect services
PreferPath.Path,Config,Clock, andDateTimeservices when those concerns are already inside Effect code
For backgroun...
Files:
packages/opencode/test/tool/mcp-exa.test.tspackages/opencode/src/tool/websearch-auth.tspackages/opencode/test/tool/websearch-auth.test.tspackages/opencode/src/tool/mcp-exa.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/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.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 (24)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:58.310Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
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.
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:21.835Z
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.
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:56.086Z
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.
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: 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: 193
File: packages/app/src/pages/layout/sidebar-items.tsx:102-107
Timestamp: 2026-04-23T15:26:07.250Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/layout/sidebar-items.tsx`), the `indicator()` function in `SessionRow` intentionally renders `props.leadingSlot` (the pin button) only as a fallback when no status indicator (running/permission/error/unseen) is active. When a higher-priority status wins the slot, the pin button is removed from the DOM — this is a deliberate design choice for the merged leading slot (`#150`). The keyboard unpin path is preserved via: (1) focusing the row anchor triggers `group-focus-within` which reveals the dots menu trigger, then Tab → Enter → "Unpin Session"; (2) the context menu (right-click / Shift+F10) exposes "Unpin Session". The "always render + CSS overlay" approach was considered but rejected due to z-index/pointer-events complexity; residual `...` slot behavior is tracked in `#192`. Do NOT flag the absence of the pin button from the DOM when a status is active as an accessibility regression.
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:54.895Z
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.
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.
📚 Learning: 2026-04-27T11:19:21.835Z
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:21.835Z
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/app/src/pages/session/settings-websearch-source.test.tspackages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/desktop-electron/src/main/ipc.tspackages/app/src/pages/session/websearch-toasts.test.tspackages/opencode/src/tool/websearch-auth.tspackages/app/src/components/dialog-connect-websearch-source.test.tspackages/opencode/test/tool/websearch-auth.test.tspackages/opencode/src/tool/mcp-exa.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/app/src/pages/session/settings-websearch-source.test.tspackages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/app/src/pages/session/websearch-toasts.test.tspackages/app/src/components/dialog-connect-websearch-source.test.tspackages/opencode/test/tool/websearch-auth.test.ts
📚 Learning: 2026-04-27T11:18:45.847Z
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:45.847Z
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/app/src/pages/session/settings-websearch-source.test.tspackages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/desktop-electron/src/main/ipc.tspackages/opencode/src/tool/websearch-auth.tspackages/app/src/components/dialog-connect-websearch-source.test.tspackages/opencode/test/tool/websearch-auth.test.tspackages/opencode/src/tool/mcp-exa.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 `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/app/src/pages/session/settings-websearch-source.test.tspackages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/app/src/pages/session/websearch-toasts.test.tspackages/app/src/pages/session/websearch-toasts.tspackages/app/src/components/dialog-connect-websearch-source.test.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/pages/session/settings-websearch-source.test.tspackages/app/src/pages/session/websearch-toasts.test.tspackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/session/websearch-toasts.tspackages/app/src/components/settings-general.tsxpackages/app/src/components/dialog-connect-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch.tsx
📚 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/pages/session/settings-websearch-source.test.tspackages/app/src/pages/session/websearch-toasts.test.tspackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/session/websearch-toasts.tspackages/app/src/components/settings-general.tsxpackages/app/src/components/dialog-connect-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch.tsx
📚 Learning: 2026-04-27T11:18:45.896Z
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:45.896Z
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/desktop-electron/src/main/websearch-ipc-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.test.tspackages/opencode/src/tool/mcp-exa.ts
📚 Learning: 2026-04-24T13:03:10.835Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 222
File: packages/desktop-electron/src/main/index.ts:686-692
Timestamp: 2026-04-24T13:03:10.835Z
Learning: In `packages/desktop-electron/src/main/index.ts`, the `checkForUpdates()` function intentionally uses recursive self-invocation for the "Retry" path in the update-check failure dialog. This is mandated by the v5.2 design spec (`#213`): "Await the retry recursion and log any rejection so support can see repeated failures." Because retries are user-paced (require a button click), all prior async frames have already unwound through microtasks before the next attempt, so there is no stack/frame-nesting problem in practice. Do not suggest refactoring this to an iterative loop.
Applied to files:
packages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-27T11:18:45.149Z
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:45.149Z
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/desktop-electron/src/main/websearch-ipc-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.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/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/desktop-electron/src/main/websearch-ipc-source.test.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/websearch-ipc-source.test.tspackages/desktop-electron/src/main/ipc.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/websearch-ipc-source.test.tspackages/desktop-electron/src/main/ipc.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/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.test.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/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/i18n/zh.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/tool/websearch-auth.tspackages/opencode/src/tool/mcp-exa.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/websearch-auth.test.ts
📚 Learning: 2026-04-24T05:39:56.086Z
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:56.086Z
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/app/src/components/dialog-connect-websearch.tsx
📚 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/tool/mcp-exa.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/src/tool/mcp-exa.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/tool/mcp-exa.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/src/tool/mcp-exa.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/tool/mcp-exa.ts
🔇 Additional comments (27)
packages/desktop-electron/src/main/ipc.ts (2)
214-223: Good consolidation of Web Search invalidation flow.Extracting
invalidateWebSearchTools()and using it for the first refresh removes duplicated logic and keeps failure semantics consistent across instances.
235-243: Rollback path is now correctly hardened.Awaiting rollback invalidation and logging rollback-specific per-instance failures is a solid reliability improvement for partial-refresh failure cases.
packages/desktop-electron/src/main/websearch-ipc-source.test.ts (1)
37-38: Nice contract-test expansion for rollback guarantees.These assertions correctly lock in the new rollback invalidation path and rollback warning message in the IPC source contract.
packages/app/src/i18n/en.ts (1)
849-887: Saved-quota i18n additions look consistent.The new keys cleanly separate invalid-key recovery from saved-key quota recovery across chip, secondary text, dialog, and toast copy.
packages/app/src/pages/session/websearch-toasts.test.ts (1)
32-49: Good contract update for saved-key quota toasts.The new
descriptionKeyexpectation aligns with the saved-source quota path, and the anti-secret assertions remain in place.packages/app/src/components/dialog-connect-websearch-source.test.ts (1)
35-38: Nice regression guard for parent-status callback wiring.This catches accidental removal of the post-save/remove status notification path.
packages/opencode/test/tool/mcp-exa.test.ts (2)
83-118: Strong coverage for credential-version-aware failures.This test directly protects the version-threading contract for saved-key invalidation handling.
285-291: Env quota copy contract is well pinned.Good assertion to prevent regressions that incorrectly route env users toward editable saved-key flows.
packages/app/src/pages/session/settings-websearch-source.test.ts (1)
26-68: Good contract coverage expansion for saved-quota state.The added assertions meaningfully lock i18n completeness and status-refresh wiring.
packages/app/src/components/settings-general.tsx (1)
189-317: Web Search status refresh wiring looks correct.Passing
webSearchStatusActions.refetchintoDialogConnectWebSearchis a good fix for post-save/remove state staleness.packages/app/src/i18n/zh.ts (1)
738-775: Chinese localization coverage for saved-quota flow is complete.The new keys line up with the UI branches introduced in this PR.
packages/app/src/components/dialog-connect-websearch.tsx (1)
56-57: Parent refresh callback wiring is a solid addition.Calling
props.onStatusChanged?.()after save/remove cleanly enables the settings row to refresh without coupling this dialog to parent internals.Also applies to: 82-83
packages/app/src/pages/session/websearch-toasts.ts (1)
14-17: LGTM! Correctly branches quota-exceeded toasts by credential source.The type union now includes
"toast.websearch.savedQuota.description", and the toast selection logic properly distinguishes between anonymous and saved sources for quota exhaustion. The nested ternary structure is consistent with the existing pattern.Also applies to: 57-79
packages/opencode/test/tool/websearch-auth.test.ts (4)
43-56: LGTM! Proper env isolation and credentialVersion assertion.Explicitly deleting
process.env.EXA_API_KEYensures the test exercises the saved-key path without env interference, and assertingcredentialVersion: expect.any(String)validates the new versioning metadata.
59-69: LGTM! Correctly tests env-backed key protection.This test verifies that
saveKey()returns the env-derived status and does not persist a new saved credential when an env key exists without a prior saved key.
132-158: LGTM! Validates version-aware failure rejection.This test ensures stale failures (with older
credentialVersion) do not overwrite a newly saved key, which is critical for preventing race conditions between in-flight requests and key updates.
160-192: LGTM! Comprehensive quota-exceeded state tracking.The test verifies that
markNeedsAttentionwithquota_exceededsetsstatus: "quota_exceeded"andreason: "quota_exceeded"while preserving the credential version, and thatsvc.status()returnsquotaExceeded: truewithneedsAttention: false.packages/opencode/src/tool/websearch-auth.ts (4)
33-35: LGTM! Simple credential versioning scheme.The
nextCredentialVersion()function generates a reasonably unique version using base-36 timestamp and random suffix. This is sufficient for distinguishing in-flight requests from key updates without needing cryptographic guarantees.
45-64: LGTM! Status derivation correctly reads metadata markers.The
statusFrom()function properly extractsneedsAttentionandquotaExceededflags from the storedmetadata.statusfield rather than hardcoding values, enabling the saved-quota-exceeded state to flow through to the UI.
86-88: LGTM! Prevents overwriting env-backed state.The early return when
!currentSaved && env.source === "env"ensures that attempting to save a key when only an env key exists returns the env status without persisting a saved credential, which aligns with the PR objective.
122-136: LGTM! Version-aware failure handling prevents stale overwrites.The version comparison at line 124 ensures that failures from in-flight requests with older credentials cannot corrupt the state of a newly saved key. The metadata update correctly sets
statusto either"quota_exceeded"or"needs_attention"while preserving other fields via spread.packages/opencode/src/tool/mcp-exa.ts (6)
6-6: LGTM! Type extensions for credential versioning.The
CredentialandFailuretypes are correctly extended with optionalversionandcredentialVersionfields respectively, enabling version-aware failure tracking throughout the error pipeline.Also applies to: 14-14
45-47: LGTM! Clean credential version accessor.The
credentialVersion()helper correctly returnsundefinedfor anonymous credentials and the credential's version for saved/env sources, which aligns with the versioning semantics.
68-72: LGTM! Conditional credentialVersion spread avoids undefined properties.The pattern
...(input.credentialVersion === undefined ? {} : { credentialVersion: input.credentialVersion })correctly avoids addingcredentialVersion: undefinedto failure objects, keeping the serialized payloads clean.Also applies to: 82-88
101-101: LGTM! Updated env quota message.The message now correctly instructs users to "Update the environment variable and retry" instead of the previous Settings-focused wording, which is appropriate since env keys cannot be managed through the app UI.
121-141: LGTM! SSE parsing correctly threads credential version.Both
decodeSseDataandparseSseaccept and propagatecredentialVersionthrough all error paths, ensuring failures from SSE decoding are properly attributed to the credential version that was used for the request.Also applies to: 143-172
216-249: LGTM! All call() failure paths include credential version.The
call()function correctly passescredentialVersion(credential)to all failure construction sites: timeouts (line 223), network errors (line 232), HTTP error responses (line 245), and SSE parsing (line 249). This ensures end-to-end version tracking.
10553d3 to
ee24de5
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
PR #274 adds saved-key quota tracking, credential versioning to prevent stale-key marking, rollback tool invalidation, and the onStatusChanged refresh callback. The overall design is sound. Below are P2/P3 observations organized by file.
P2
- dialog-connect-websearch.tsx — saved+quotaExceeded Match block is a near-duplicate of saved+needsAttention (differs only by status text line)
- websearch-auth.ts — nextCredentialVersion() uses Date.now()+Math.random() instead of crypto.randomUUID()
- settings-general.tsx — chip/secondary text conditionals growing linearly with each new state; consider a lookup map
P3
- ipc.ts — rollback reuses stale directories closure snapshot; re-fetch Instance.directories() before rollback invalidation
- websearch-auth.ts — anonymous quota marker shares AUTH_KEY namespace with saved keys (key: "" sentinel)
- zh.ts — new keys mix English "Key" with Chinese, following existing pattern but inconsistent with fully-Chinese keys
ee24de5 to
0680d38
Compare
|
Non-inline P3 outcome: I am leaving the anonymous quota marker in the existing WebSearchAuth AUTH_KEY namespace. It stores key: "" plus metadata only, save/remove already overwrite or clear it, env status takes precedence, and splitting it into a second persisted key would add migration/cleanup surface without a concrete failing path. |
0680d38 to
a694518
Compare
There was a problem hiding this comment.
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/app/src/components/settings-general.tsx`:
- Line 196: The saved-quota message must take precedence over the invalid-key
message: in settings-general.tsx change the conditional order so the check for
s.source === "saved" && s.quotaExceeded runs before the s.source ===
"invalid-key" branch (both in the chip rendering where s.source is evaluated and
in the description block where the same precedence occurs around the language.t
calls). Locate the two places referencing s.source (the chip line using
language.t("settings.general.webSearch.chip.savedQuota") and the description
block around the language.t keys at ~294-303) and reorder the conditionals so
quotaExceeded is evaluated first, returning the savedQuota text when true.
In `@packages/opencode/test/tool/websearch-auth.test.ts`:
- Around line 72-92: The test currently doesn't assert that credentialVersion
was rotated; capture the original version from
auth.data.get(WebSearchAuth.AUTH_KEY).metadata.credentialVersion before calling
WebSearchAuth.Service.use (saveKey), then after the call assert that
auth.data.get(WebSearchAuth.AUTH_KEY).metadata.credentialVersion is a string and
!== the original value; keep existing assertions for key and other metadata
(reference functions/values: WebSearchAuth.Service.use, saveKey,
WebSearchAuth.AUTH_KEY, auth.data.get).
🪄 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: d48efd35-7f68-4317-9105-21d65164cba1
📒 Files selected for processing (14)
packages/app/src/components/dialog-connect-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch.tsxpackages/app/src/components/settings-general.tsxpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/session/settings-websearch-source.test.tspackages/app/src/pages/session/websearch-toasts.test.tspackages/app/src/pages/session/websearch-toasts.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/opencode/src/tool/mcp-exa.tspackages/opencode/src/tool/websearch-auth.tspackages/opencode/test/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.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). (3)
- GitHub Check: smoke-macos-arm64
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (5)
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/pages/session/websearch-toasts.test.tspackages/app/src/pages/session/settings-websearch-source.test.tspackages/app/src/pages/session/websearch-toasts.tspackages/app/src/components/settings-general.tsxpackages/app/src/components/dialog-connect-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch.tsxpackages/app/src/i18n/zh.tspackages/app/src/i18n/en.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/main/websearch-ipc-source.test.tspackages/desktop-electron/src/main/ipc.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
packages/opencode/**/*.ts
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.ts: UseEffect.gen(function* () { ... })for Effect composition
UseEffect.fn("Domain.method")for named/traced effects andEffect.fnUntracedfor internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer.pipe()wrappers
UseEffect.callbackfor callback-based APIs
PreferDateTime.nowAsDateovernew Date(yield* Clock.currentTimeMillis)when you need aDatein Effect code
UseSchema.Classfor multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
UseSchema.TaggedErrorClassfor typed errors in Effect schemas
UseSchema.Defectinstead ofunknownfor defect-like causes in Effect code
InEffect.gen/Effect.fn, preferyield* new MyError(...)overyield* Effect.fail(new MyError(...))for direct early-failure branches
UsemakeRuntimefromsrc/effect/run-service.tsfor all services; it returns{ runPromise, runFork, runCallback }backed by a sharedmemoMapthat deduplicates layers
UseInstanceStatefromsrc/effect/instance-state.tsfor per-directory or per-project state that needs per-instance cleanup; do work directly in theInstanceState.makeclosure whereScopedCachehandles run-once semantics
UseEffect.addFinalizerorEffect.acquireReleaseinside theInstanceState.makeclosure for cleanup (subscriptions, process teardown, etc.)
UseEffect.forkScopedinside theInstanceState.makeclosure for background stream consumers — the fiber is interrupted when the instance is disposed
PreferFileSystem.FileSysteminstead of rawfs/promisesfor effectful file I/O in Effect services
PreferChildProcessSpawner.ChildProcessSpawnerwithChildProcess.make(...)instead of custom process wrappers in Effect services
PreferHttpClient.HttpClientinstead of rawfetchin Effect services
PreferPath.Path,Config,Clock, andDateTimeservices when those concerns are already inside Effect code
For backgroun...
Files:
packages/opencode/test/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.test.tspackages/opencode/src/tool/websearch-auth.tspackages/opencode/src/tool/mcp-exa.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/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.test.ts
🧠 Learnings (38)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:58.310Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:46.780Z
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.
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: 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.
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.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/src/pages/layout/sidebar-items.tsx:102-107
Timestamp: 2026-04-23T15:26:07.250Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/layout/sidebar-items.tsx`), the `indicator()` function in `SessionRow` intentionally renders `props.leadingSlot` (the pin button) only as a fallback when no status indicator (running/permission/error/unseen) is active. When a higher-priority status wins the slot, the pin button is removed from the DOM — this is a deliberate design choice for the merged leading slot (`#150`). The keyboard unpin path is preserved via: (1) focusing the row anchor triggers `group-focus-within` which reveals the dots menu trigger, then Tab → Enter → "Unpin Session"; (2) the context menu (right-click / Shift+F10) exposes "Unpin Session". The "always render + CSS overlay" approach was considered but rejected due to z-index/pointer-events complexity; residual `...` slot behavior is tracked in `#192`. Do NOT flag the absence of the pin button from the DOM when a status is active as an accessibility regression.
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.
📚 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/app/src/pages/session/websearch-toasts.test.tspackages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/app/src/pages/session/settings-websearch-source.test.tspackages/app/src/pages/session/websearch-toasts.tspackages/app/src/components/dialog-connect-websearch-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/app/src/components/dialog-connect-websearch.tsxpackages/opencode/test/tool/websearch-auth.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/app/src/pages/session/websearch-toasts.test.tspackages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/app/src/pages/session/settings-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/app/src/components/dialog-connect-websearch.tsxpackages/opencode/test/tool/websearch-auth.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/app/src/pages/session/websearch-toasts.test.tspackages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/desktop-electron/src/main/ipc.tspackages/app/src/pages/session/settings-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/app/src/components/dialog-connect-websearch.tsxpackages/opencode/test/tool/websearch-auth.test.tspackages/opencode/src/tool/websearch-auth.tspackages/opencode/src/tool/mcp-exa.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/app/src/pages/session/websearch-toasts.test.tspackages/desktop-electron/src/main/websearch-ipc-source.test.tspackages/desktop-electron/src/main/ipc.tspackages/app/src/pages/session/settings-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.test.tspackages/opencode/src/tool/websearch-auth.tspackages/opencode/src/tool/mcp-exa.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/pages/session/websearch-toasts.test.tspackages/app/src/pages/session/settings-websearch-source.test.tspackages/app/src/pages/session/websearch-toasts.tspackages/app/src/components/settings-general.tsxpackages/app/src/components/dialog-connect-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch.tsxpackages/app/src/i18n/zh.tspackages/app/src/i18n/en.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/pages/session/websearch-toasts.test.tspackages/app/src/pages/session/settings-websearch-source.test.tspackages/app/src/pages/session/websearch-toasts.tspackages/app/src/components/settings-general.tsxpackages/app/src/components/dialog-connect-websearch-source.test.tspackages/app/src/components/dialog-connect-websearch.tsxpackages/app/src/i18n/zh.tspackages/app/src/i18n/en.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/desktop-electron/src/main/websearch-ipc-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.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/desktop-electron/src/main/websearch-ipc-source.test.tspackages/opencode/test/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.test.tspackages/opencode/src/tool/mcp-exa.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/websearch-ipc-source.test.tspackages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-24T17:12:26.774Z
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:26.774Z
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/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-24T13:03:14.694Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 222
File: packages/desktop-electron/src/main/index.ts:686-692
Timestamp: 2026-04-24T13:03:14.694Z
Learning: In `packages/desktop-electron/src/main/index.ts`, the `checkForUpdates()` function intentionally uses recursive self-invocation for the "Retry" path in the update-check failure dialog. This is mandated by the v5.2 design spec (`#213`): "Await the retry recursion and log any rejection so support can see repeated failures." Because retries are user-paced (require a button click), all prior async frames have already unwound through microtasks before the next attempt, so there is no stack/frame-nesting problem in practice. Do not suggest refactoring this to an iterative loop.
Applied to files:
packages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-25T11:52:43.532Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 233
File: packages/desktop-electron/src/main/server.ts:60-60
Timestamp: 2026-04-25T11:52:43.532Z
Learning: In Astro-Han/pawwork (`packages/desktop-electron/src/main/index.ts`), `ensureLoopbackNoProxy()` (defined at line 551) is called inside `setupApp()` at line 263, which runs synchronously before `initialize()` (line 321) and `spawnLocalServer()` (line 395). It unconditionally upserts `127.0.0.1`, `localhost`, and `::1` into both `process.env.NO_PROXY` and `process.env.no_proxy`. This guarantees that `configureProxyDispatcher(process.env)` in `server.ts` always receives a `process.env` that already excludes loopback from proxying. Do not flag the absence of a loopback-merge inside `configureProxyDispatcher` or `normalizeProxyConfig` — the exclusion is already enforced at a higher layer before those functions are ever called.
Applied to files:
packages/desktop-electron/src/main/ipc.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/desktop-electron/src/main/ipc.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/app/src/components/settings-general.tsxpackages/app/src/components/dialog-connect-websearch.tsx
📚 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 locator assertions like `toBeVisible()`, `toHaveCount(0)`, and `toHaveAttribute(...)` for normal UI state verification
Applied to files:
packages/app/src/components/dialog-connect-websearch-source.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/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.test.tspackages/opencode/src/tool/websearch-auth.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/tool/mcp-exa.test.tspackages/opencode/test/tool/websearch-auth.test.ts
📚 Learning: 2026-04-24T17:08:46.780Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:46.780Z
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/components/dialog-connect-websearch.tsxpackages/app/src/i18n/zh.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/app/src/components/dialog-connect-websearch.tsx
📚 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/components/dialog-connect-websearch.tsx
📚 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/app/src/components/dialog-connect-websearch.tsx
📚 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/app/src/components/dialog-connect-websearch.tsx
📚 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/app/src/components/dialog-connect-websearch.tsx
📚 Learning: 2026-04-23T15:26:07.250Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/src/pages/layout/sidebar-items.tsx:102-107
Timestamp: 2026-04-23T15:26:07.250Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/layout/sidebar-items.tsx`), the `indicator()` function in `SessionRow` intentionally renders `props.leadingSlot` (the pin button) only as a fallback when no status indicator (running/permission/error/unseen) is active. When a higher-priority status wins the slot, the pin button is removed from the DOM — this is a deliberate design choice for the merged leading slot (`#150`). The keyboard unpin path is preserved via: (1) focusing the row anchor triggers `group-focus-within` which reveals the dots menu trigger, then Tab → Enter → "Unpin Session"; (2) the context menu (right-click / Shift+F10) exposes "Unpin Session". The "always render + CSS overlay" approach was considered but rejected due to z-index/pointer-events complexity; residual `...` slot behavior is tracked in `#192`. Do NOT flag the absence of the pin button from the DOM when a status is active as an accessibility regression.
Applied to files:
packages/app/src/components/dialog-connect-websearch.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/components/dialog-connect-websearch.tsx
📚 Learning: 2026-04-22T09:32:58.310Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:58.310Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
Applied to files:
packages/app/src/components/dialog-connect-websearch.tsx
📚 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/i18n/zh.tspackages/app/src/i18n/en.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/tool/websearch-auth.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 `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/tool/websearch-auth.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/tool/websearch-auth.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/tool/websearch-auth.tspackages/opencode/src/tool/mcp-exa.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.TaggedErrorClass` for typed errors in Effect schemas
Applied to files:
packages/opencode/src/tool/websearch-auth.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/tool/websearch-auth.tspackages/opencode/src/tool/mcp-exa.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/tool/mcp-exa.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/src/tool/mcp-exa.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/tool/mcp-exa.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/src/tool/mcp-exa.ts
🔇 Additional comments (14)
packages/desktop-electron/src/main/websearch-ipc-source.test.ts (1)
37-39: Good rollback contract coverage added.These assertions correctly lock in the rollback pass (fresh directories + rollback invalidation + rollback warning log), which matches the failure-recovery behavior this PR is targeting.
Based on learnings: raw-source
toContainassertions are intentionally used as a lightweight contract guard in desktop-electron main source contract tests.packages/desktop-electron/src/main/ipc.ts (2)
213-223: Helper extraction for websearch invalidation is solid.
invalidateWebSearchToolsremoves duplication and keeps the multi-instance invalidation behavior explicit and consistent in the initial pass.
235-244: Rollback now correctly targets a fresh instance snapshot.Re-reading
Instance.directories()before rollback invalidation and logging rollback-specific failures closes the stale-snapshot gap and improves recovery diagnostics.packages/app/src/i18n/en.ts (1)
849-887: Saved-quota i18n coverage and recovery copy look consistent.These additions cleanly separate saved-key quota messaging from invalid-key messaging and align with the intended state split.
packages/app/src/pages/session/websearch-toasts.test.ts (1)
44-44: Quota toast assertion update is correct.The expected description key now correctly targets the saved-key quota path.
packages/app/src/components/dialog-connect-websearch-source.test.ts (1)
35-47: New source-contract checks are valuable and targeted.The callback notification assertion and savedQuota-before-failed ordering guard both directly protect the intended dialog behavior.
packages/opencode/test/tool/mcp-exa.test.ts (1)
83-118: Excellent regression coverage for versioned failures and env-quota messaging.These assertions close important gaps around stale-failure attribution and source-specific recovery copy.
Also applies to: 285-291
packages/app/src/pages/session/settings-websearch-source.test.ts (1)
32-68: Settings source-contract updates look solid.The new checks correctly guard savedQuota localization presence and post-dialog status refresh wiring.
packages/app/src/components/settings-general.tsx (1)
189-189: Status refresh callback wiring is correctly hooked up.Passing
webSearchStatusActions.refetchinto the dialog is the right way to keep the settings row state in sync after save/remove.Also applies to: 317-317
packages/app/src/i18n/zh.ts (1)
738-778: zh Web Search copy updates are coherent and complete.The saved-quota additions and revised recovery wording are consistent with the new state model.
packages/app/src/pages/session/websearch-toasts.ts (1)
14-17: Source-aware quota toast routing is correctly implemented.The new union type and
saved-specific description branch cleanly enforce distinct recovery copy for saved-key quota failures.Also applies to: 58-71
packages/app/src/components/dialog-connect-websearch.tsx (1)
34-35: Saved-quota precedence and form reuse look correct.The title/body ordering is now consistent for
saved + quotaExceeded, and the shared saved-key form removes the duplication that previously made those branches easy to drift apart.Also applies to: 100-135, 178-223
packages/opencode/src/tool/mcp-exa.ts (1)
60-80: Credential version is threaded through the full failure pipeline.The new classification and SSE/request plumbing keep
credentialVersionattached in every saved-key failure path, which is exactly what the auth layer needs to ignore stale failures after a key is replaced.Also applies to: 121-172, 216-249
packages/opencode/src/tool/websearch-auth.ts (1)
14-16: The env guard and version gate match the runtime contract.Blocking new saves only for pure env-backed state, while allowing an existing saved key to be updated and shielding it from stale failure reports, closes the two regressions this PR is targeting.
Also applies to: 88-145
a694518 to
050deae
Compare
1 participant
Summary
Fixes follow-up Web Search state regressions after PR #271: env-backed status no longer accepts saved-key writes, stale saved-key failures no longer mark a newly saved key, saved-key quota is shown separately from invalid-key state, and Web Search toggle rollback refreshes instance tool caches again.
Why
The last review pass found one P1 and three P2 issues where the Web Search UI/runtime could disagree with the actual credential state or leave stale tool caches after rollback. These fixes keep the runtime state, saved credential metadata, and user-facing recovery copy aligned.
Related Issue
Follow-up to PR #271 review findings. No separate issue.
How To Verify
Also ran:
Screenshots or Recordings
Not attached. The UI-facing change is status/copy branching for existing Web Search settings states and is covered by source contract tests.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
New Features
Bug Fixes
i18n
Tests