fix: isolate internal auth env from user commands - #327
Merged
Conversation
Astro-Han
added
bug
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
🧰 Additional context used📓 Path-based instructions (2)packages/opencode/**/*.ts📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
Files:
packages/opencode/test/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (packages/opencode/test/AGENTS.md)
Files:
🧠 Learnings (17)📓 Common learnings📚 Learning: 2026-04-29T10:34:22.399ZApplied to files:
📚 Learning: 2026-04-27T11:19:24.963ZApplied to files:
📚 Learning: 2026-04-28T05:36:24.561ZApplied to files:
📚 Learning: 2026-04-27T11:18:47.596ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.032ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.032ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.032ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.032ZApplied to files:
📚 Learning: 2026-04-28T05:36:25.456ZApplied to files:
📚 Learning: 2026-04-23T08:51:00.819ZApplied to files:
📚 Learning: 2026-04-28T04:56:18.533ZApplied to files:
📚 Learning: 2026-04-28T04:56:21.338ZApplied to files:
📚 Learning: 2026-04-27T12:59:49.844ZApplied to files:
📚 Learning: 2026-04-28T11:24:35.312ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.032ZApplied to files:
📚 Learning: 2026-04-27T11:18:47.332ZApplied to files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughAdds environment-sanitization utilities and integrates them so spawned PTYs, shell sessions, the bash tool, and E2E backend startups no longer propagate internal OpenCode server auth environment variables (case-insensitive), while preserving unrelated env entries and allowing explicit overrides. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Test / CLI
participant EnvCreator as createBackendEnv / withoutInternalServerAuthEnv
participant Sandbox as Sandbox (paths)
participant Spawn as spawn / ChildProcess
participant Backend as Backend process
TestRunner->>EnvCreator: request env (base?, sandbox, llmUrl?)
EnvCreator->>Sandbox: derive HOME/CACHE/CONFIG/STATE
EnvCreator->>EnvCreator: merge base/process.env, set OPENCODE_CLIENT & OPENCODE_E2E_LLM_URL
EnvCreator->>EnvCreator: withoutInternalServerAuthEnv (strip auth keys case-insensitive)
EnvCreator->>Spawn: return sanitized env
Spawn->>Backend: start backend with sanitized env
Backend-->>TestRunner: backend runs (health accessible without unexpected Basic Auth)
sequenceDiagram
participant UserCmd as User Command / PTY
participant EnvUtil as withoutInternalServerAuthEnv
participant Shell as Shell / PTY process
participant Tool as Bash / Prompt executor
UserCmd->>EnvUtil: provide merged env (process.env + overrides)
EnvUtil-->>UserCmd: sanitized env (auth vars removed)
UserCmd->>Shell: spawn shell with sanitized env
Tool->>EnvUtil: request shell env for execution
EnvUtil-->>Tool: sanitized env (custom vars preserved)
Shell-->>UserCmd: command output (no leaked auth values)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 docstrings
🧪 Generate unit tests (beta)
Review rate limit: 0/3 reviews remaining, refill in 48 minutes. 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 implements security measures to prevent internal server authentication credentials from being exposed in subprocesses and terminal sessions. It introduces a utility for case-insensitive environment variable removal and applies it to PTY, shell, and bash tool components. Feedback was provided regarding the handling of environment variable casing on Windows and the method used to clear these variables in PTY sessions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app/e2e/backend.ts (1)
61-94: 🧹 Nitpick | 🔵 TrivialAvoid duplicating auth-env filtering logic.
Line 61 and Lines 90-92 reimplement the same sanitizer already introduced in
packages/opencode/src/util/env.ts. Keeping two independent copies increases drift risk when the internal auth key set changes.Consider extracting this into a shared utility package consumed by both
packages/opencodeandpackages/app/e2e.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/e2e/backend.ts` around lines 61 - 94, The createBackendEnv function duplicates the auth-env filtering logic (INTERNAL_SERVER_AUTH_ENV and the deletion loop) already present in packages/opencode/src/util/env.ts; remove the local constant and the for-loop in createBackendEnv and instead call the shared sanitizer utility (exported function from the opencode util, e.g. sanitizeEnv or similar) to strip internal auth variables before returning env, updating imports to consume the shared helper so both packages use a single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/pty/index.ts`:
- Around line 194-199: Currently the code unconditionally sets
env.OPENCODE_SERVER_USERNAME and env.OPENCODE_SERVER_PASSWORD to empty strings;
change this so you only override inherited values when the caller did not supply
them by checking the original input.env (or presence on the incoming env map)
before clobbering—i.e., if input.env.OPENCODE_SERVER_USERNAME is undefined/null
then set env.OPENCODE_SERVER_USERNAME = "" (same for OPENCODE_SERVER_PASSWORD),
otherwise leave the provided values intact so explicit PTY auth envs are
preserved.
In `@packages/opencode/test/pty/pty-session.test.ts`:
- Around line 124-136: The PTY test uses ${OPENCODE_SERVER_USERNAME} and
${OPENCODE_SERVER_PASSWORD} which can be empty and still pass; change the shell
expansions in the Pty.write call to use the "-unset" default (e.g.,
${OPENCODE_SERVER_USERNAME-unset} and ${OPENCODE_SERVER_PASSWORD-unset}) and
update the assertions that inspect the output (the variable output joined and
the expect checks) to explicitly expect "username=unset" and "password=unset"
instead of only checking absence of secret strings; keep the existing checks for
"custom=kept" and the negative checks for "secret" and "PawWork".
In `@packages/opencode/test/tool/bash.test.ts`:
- Around line 157-201: Add a symmetric assertion for the lowercase environment
variable filter: in the test "does not expose internal server auth env to user
commands" after the existing expect(result.output).toContain("username=unset")
and the password/secret assertions, also assert that a lowercase key check (e.g.
expecting "username=unset" for opencode_server_username) is present to mirror
the existing lowercase password coverage; reference the OPENCODE_SERVER_USERNAME
and OPENCODE_SERVER_PASSWORD env vars and the
expect(...).not.toContain("secret") assertion to locate where to add this extra
assertion.
---
Outside diff comments:
In `@packages/app/e2e/backend.ts`:
- Around line 61-94: The createBackendEnv function duplicates the auth-env
filtering logic (INTERNAL_SERVER_AUTH_ENV and the deletion loop) already present
in packages/opencode/src/util/env.ts; remove the local constant and the for-loop
in createBackendEnv and instead call the shared sanitizer utility (exported
function from the opencode util, e.g. sanitizeEnv or similar) to strip internal
auth variables before returning env, updating imports to consume the shared
helper so both packages use a single source of truth.
🪄 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: 1921cd3a-02ec-4e48-ae5b-56af64c9132a
📒 Files selected for processing (9)
packages/app/e2e/backend.test.tspackages/app/e2e/backend.tspackages/opencode/src/pty/index.tspackages/opencode/src/session/prompt.tspackages/opencode/src/tool/bash.tspackages/opencode/src/util/env.tspackages/opencode/test/pty/pty-session.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/tool/bash.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-app
- GitHub Check: typecheck
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-desktop
- GitHub Check: unit-opencode
- GitHub Check: unit-app
- GitHub Check: smoke-macos-arm64
- GitHub Check: e2e-artifacts
- GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (3)
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/util/env.tspackages/opencode/test/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/tool/bash.tspackages/opencode/src/session/prompt.tspackages/opencode/test/pty/pty-session.test.tspackages/opencode/src/pty/index.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/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/pty/pty-session.test.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/e2e/backend.test.tspackages/app/e2e/backend.ts
🧠 Learnings (40)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:311-328
Timestamp: 2026-04-28T05:36:18.200Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the ripgrep binary download flow (`HttpClientRequest.get` → `fs.writeWithDirs` → `extract`) does NOT verify a pinned SHA-256 checksum. Adding per-platform hash verification (15.1.0 × 6 platforms) requires a dedicated manifest + `crypto.subtle.digest` step and is intentionally deferred to a follow-up PR. Existing mitigations: (1) `VERSION` is pinned to `"15.1.0"`, (2) downloads are over HTTPS from GitHub Releases, (3) PawWork production builds ship a bundled `rg` binary so this download path is a last-resort fallback (system PATH → cached → download). Do NOT re-flag the missing checksum verification in this file until the dedicated follow-up PR lands.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:02.858Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:11.771Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 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.
📚 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/util/env.tspackages/opencode/src/tool/bash.tspackages/opencode/src/session/prompt.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/util/env.tspackages/opencode/src/tool/bash.tspackages/opencode/src/session/prompt.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-28T05:36:24.561Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/tool/bash.tspackages/opencode/src/session/prompt.tspackages/opencode/test/pty/pty-session.test.tspackages/app/e2e/backend.test.tspackages/opencode/src/pty/index.tspackages/app/e2e/backend.ts
📚 Learning: 2026-04-28T05:36:25.456Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/grep.test.ts:16-22
Timestamp: 2026-04-28T05:36:25.456Z
Learning: In `packages/opencode/test/tool/grep.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a manual `ManagedRuntime.make(Layer.mergeAll(...))` setup and raw `bun:test` test cases rather than the `testEffect(...)` harness. The migration to `testEffect` + `it.live(...)` + `provideTmpdirInstance(...)` is a whole-file rewrite deferred to a dedicated sweep PR that will migrate all tool tests still on the manual ManagedRuntime pattern together. Do NOT re-flag the ManagedRuntime setup in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/pty/pty-session.test.tspackages/app/e2e/backend.test.ts
📚 Learning: 2026-04-28T04:56:21.338Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/tool-define.test.ts:2-9
Timestamp: 2026-04-28T04:56:21.338Z
Learning: In `packages/opencode/test/tool/tool-define.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a `ManagedRuntime.make(Layer.mergeAll(Truncate.defaultLayer, Agent.defaultLayer))` wrapper and raw `bun:test` `test(...)` cases rather than the `testEffect(...)` harness. This code was adopted wholesale from upstream via the graft strategy (per `project_upstream_strategy.md`). Migrating to `testEffect` is deferred to a follow-up PR so as not to mix refactor + harness-migration intents and to avoid drifting the diff from the upstream baseline needed for clean future grafts. Do NOT re-flag the `ManagedRuntime.make` usage in this file as needing harness migration until that follow-up lands.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/session/prompt.tspackages/app/e2e/backend.test.tspackages/app/e2e/backend.ts
📚 Learning: 2026-04-27T11:19:24.963Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch-auth.test.ts:0-0
Timestamp: 2026-04-27T11:19:24.963Z
Learning: In `packages/opencode/test/tool/websearch-auth.test.ts` (Astro-Han/pawwork), the tests intentionally use a small local `runWith` runner with raw `bun:test` and `Effect.runPromise` rather than the `testEffect` harness. Each test case injects a custom in-memory `Auth.Service` layer; switching to `testEffect` would be style-only churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/tool/bash.tspackages/opencode/src/session/prompt.tspackages/opencode/test/pty/pty-session.test.tspackages/app/e2e/backend.test.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-27T11:18:47.596Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/agent/agent.test.ts:440-447
Timestamp: 2026-04-27T11:18:47.596Z
Learning: In `packages/opencode/test/agent/agent.test.ts` (Astro-Han/pawwork), all agent-permission tests intentionally use the manual `tmpdir()` + `Instance.provide(...)` pattern. Do not flag individual tests in this file for conversion to `provideTmpdirInstance(...)` or `provideInstance(...)`; a full harness migration would be a separate PR if the pattern ever needs to change.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/pty/pty-session.test.tspackages/app/e2e/backend.test.ts
📚 Learning: 2026-04-28T04:56:18.533Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:172-175
Timestamp: 2026-04-28T04:56:18.533Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the `files dies on nonexistent directory` test hardcodes `/tmp/nonexistent-dir-12345` as the missing-directory path. This is upstream-inherited behaviour adopted wholesale via the graft strategy (`project_upstream_strategy.md`). The fix — replacing the hardcoded path with a platform-neutral guaranteed-nonexistent child path derived from the `tmpdir` fixture — is intentionally deferred to a follow-up PR or upstream report, to avoid mixing bugfix + refactor intents and drifting the diff from the upstream baseline future syncs need. Do NOT re-flag the hardcoded `/tmp/nonexistent-dir-12345` path in this file as a blocking or actionable issue until the follow-up PR lands.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/pty/pty-session.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/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/pty/pty-session.test.tspackages/app/e2e/backend.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/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/pty/pty-session.test.tspackages/app/e2e/backend.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : When a test needs instance-local state, prefer `provideTmpdirInstance(...)` or `provideInstance(...)` over manual `Instance.provide(...)` inside Promise-style tests.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/pty/pty-session.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/tool/bash.tspackages/opencode/src/session/prompt.tspackages/opencode/test/pty/pty-session.test.tspackages/app/e2e/backend.test.tspackages/opencode/src/pty/index.tspackages/app/e2e/backend.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/bash.test.tspackages/opencode/src/tool/bash.tspackages/opencode/src/session/prompt.tspackages/app/e2e/backend.test.tspackages/opencode/src/pty/index.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/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/pty/pty-session.test.ts
📚 Learning: 2026-04-27T11:18:47.298Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch.test.ts:21-78
Timestamp: 2026-04-27T11:18:47.298Z
Learning: In `packages/opencode/test/tool/websearch.test.ts`, the tests intentionally use manual `Effect.runPromise` with explicit `Effect.provide(...)` chains (including `Layer.succeed(Auth.Service, ...)`, `Layer.succeed(HttpClient.HttpClient, http)`, `WebSearchAuth.layer`, `Truncate.defaultLayer`, and `Agent.defaultLayer`) rather than the `testEffect(...)` harness. This is by design: the fake Auth and HTTP recovery-metadata layers must be explicitly injected and kept visible/scoped at the test site. Do NOT suggest migrating these tests to `testEffect` or removing the manual layer provides.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/tool/bash.tspackages/opencode/src/session/prompt.tspackages/app/e2e/backend.test.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-24T03:51:56.211Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:56.211Z
Learning: In Astro-Han/pawwork E2E tests (packages/app/e2e/fixtures.ts), `project.prompt(text)` internally calls `trackSession(next.sessionID, active.directory)` after the prompt submission is observed and the active session is resolved. Tests that obtain a `sessionID` from `project.prompt()` do NOT need to call `project.trackSession(sessionID)` manually — it is already registered for teardown. Calling it again would be duplicate ownership.
Applied to files:
packages/opencode/test/session/prompt-effect.test.tspackages/opencode/src/session/prompt.tspackages/opencode/test/pty/pty-session.test.ts
📚 Learning: 2026-04-28T11:24:35.312Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:35.312Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/pty/pty-session.test.tspackages/app/e2e/backend.test.ts
📚 Learning: 2026-04-27T12:59:45.694Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/test/session/prompt-effect.test.ts:0-0
Timestamp: 2026-04-27T12:59:45.694Z
Learning: In session prompt/diagnostics tests (e.g., when verifying injected LLM “recovery reminder” copy), do not assert a single exact reminder phrasing tied to signature kind. If a scenario can produce both `input:` and `target:` signatures (for example, a `read` tool call with a `filePath` parameter), accept either phrasing: the input-repeat variant may say “repeated the same tool input 3 times” (literal count), and the target-repeat variant may say “failed against the same target multiple times” (“multiple times” without a count). Your assertions should allow both variants rather than narrowing to only the input-variant text.
Applied to files:
packages/opencode/test/session/prompt-effect.test.ts
📚 Learning: 2026-04-28T06:47:20.342Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/question.ts:6-16
Timestamp: 2026-04-28T06:47:20.342Z
Learning: In Astro-Han/pawwork, `packages/opencode/src/tool/question.ts` is a permanent PawWork carve-out: `Question.Prompt` (defined in `packages/opencode/src/question/index.ts`) remains a Zod schema, NOT an Effect Schema. The `ts-expect-error` boundary in `tool/registry.ts` marks the Zod ↔ Effect Schema seam. The dropped `packages/opencode/test/tool/question.test.ts` covered the old Zod-mixed surface. Schema-bound regression tests for this tool are intentionally deferred until `question/index.ts` migrates to Effect Schema in its own dedicated PR. Do NOT request restoring question tool tests in upstream-sync PRs; flag it only in the PR that migrates `question/index.ts` to Effect Schema.
Applied to files:
packages/opencode/test/session/prompt-effect.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : When 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/session/prompt-effect.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 `ChildProcessSpawner.ChildProcessSpawner` with `ChildProcess.make(...)` instead of custom process wrappers in Effect services
Applied to files:
packages/opencode/src/tool/bash.tspackages/opencode/src/session/prompt.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-28T04:38:21.935Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:21.935Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.
Applied to files:
packages/opencode/src/tool/bash.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/bash.tspackages/opencode/src/session/prompt.tspackages/opencode/src/pty/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.forkScoped` inside the `InstanceState.make` closure for background stream consumers — the fiber is interrupted when the instance is disposed
Applied to files:
packages/opencode/src/tool/bash.ts
📚 Learning: 2026-04-28T05:36:22.128Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/session/compaction.ts:169-191
Timestamp: 2026-04-28T05:36:22.128Z
Learning: In `packages/opencode/src/session/compaction.ts` (Astro-Han/pawwork), the internal helper `splitTurn` intentionally returns a raw `Effect.gen(...)` rather than being wrapped with `Effect.fnUntraced`. Converting `splitTurn` (and similar plain-Effect.gen internal helpers in this file, e.g. `turns`, `completedCompactions`, `buildPrompt`) to `Effect.fnUntraced` is a repo-wide convention sweep deferred to a dedicated follow-up PR. The same sweep covers `tool/tool.ts` and other helpers flagged in the same sync. Do NOT re-flag the absence of `Effect.fnUntraced` on `splitTurn` or other internal helpers in `compaction.ts` during upstream-sync PRs; flag it only in the dedicated convention-sweep PR.
Applied to files:
packages/opencode/src/tool/bash.tspackages/opencode/src/session/prompt.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/src/tool/bash.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/session/prompt.tspackages/opencode/src/pty/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/session/prompt.ts
📚 Learning: 2026-04-27T10:32:59.274Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/diagnostics.ts:245-252
Timestamp: 2026-04-27T10:32:59.274Z
Learning: In pawwork’s session diagnostics/loop error handling (e.g., packages/opencode/src/session/diagnostics.ts), keep `loopLastError` stored as raw/unscrubbed error text in loop metadata. The intended trust boundary for sensitive-data scrubbing is the renderer: `LoopRenderer.render` must apply `scrubErrorText` before any user/model-facing output. Do not recommend scrubbing `loopLastError` at the `observeToolError` call site, because failed tool parts already store raw errors in `state.error` (pre-existing leak) and scrubbing only `loopLastError` would be lossy without closing the gap. If export-side hardening is required, implement a single sanitizer pass in the export layer over both `state.error` and the loop/metadata fields, rather than sanitizing only `loopLastError` earlier in the storage path.
Applied to files:
packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-28T11:24:23.345Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:8-16
Timestamp: 2026-04-28T11:24:23.345Z
Learning: In this repo’s service-internal code (e.g., session modules), follow the established error-class convention: use plain classes with a `readonly _tag` discriminant and/or `NamedError.create` (zod-based) for service-internal errors. Do NOT request one-off conversions of such plain/service-internal errors to `Schema.TaggedErrorClass`; that migration is intentionally deferred until the dedicated repo-wide error-class sweep PR lands.
Applied to files:
packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-27T10:33:08.974Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:08.974Z
Learning: For Astro-Han/pawwork session processing code under packages/opencode/src/session/ (e.g., prompt.ts and processor.ts), this specific loop-gate TOCTOU/race between buildLoopContext() and recordSyntheticBlock/recordSyntheticStop is intentionally handled via idempotence guards (re-check sigKey presence and hasStopped inside the record helpers). Do not treat the absence of a per-parent Effect.Mutex as a blocking review issue for this known edge case; the residual window should only create extra synthetic parts and must not change the "turn ends" contract. Respect the existing code comment documenting the trade-off, and only escalate to a full-mutex approach if the race is observed in practice.
Applied to files:
packages/opencode/src/session/prompt.ts
📚 Learning: 2026-04-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/pty/pty-session.test.tspackages/app/e2e/backend.test.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-20T14:35:49.291Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/AGENTS.md:0-0
Timestamp: 2026-04-20T14:35:49.291Z
Learning: For local UI changes, run the backend and app dev servers separately using `bun run --conditions=browser ./src/index.ts serve --port 4096` for backend and `bun dev -- --port 4444` for app, then open `http://localhost:4444` to verify changes targeting backend at `http://localhost:4096`
Applied to files:
packages/app/e2e/backend.test.tspackages/app/e2e/backend.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/app/e2e/backend.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/e2e/backend.test.tspackages/app/e2e/backend.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/pty/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.gen(function* () { ... })` for Effect composition
Applied to files:
packages/opencode/src/pty/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/pty/index.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/app/e2e/backend.ts
🔇 Additional comments (5)
packages/opencode/src/util/env.ts (1)
1-8: Good central sanitizer utility.This helper correctly strips internal auth keys with case-insensitive matching and gives a single reusable guardrail for subprocess env construction.
packages/app/e2e/backend.test.ts (1)
5-23: Targeted regression test looks good.This test validates the critical behavior (auth vars stripped, unrelated vars preserved, app client marker retained) and directly guards the reported leak path.
packages/opencode/src/tool/bash.ts (1)
407-411: Shell env sanitization is correctly applied here.Good placement: Line 407 ensures the merged env passed to bash execution cannot inherit internal server auth credentials.
packages/opencode/src/session/prompt.ts (1)
1008-1018: This closes the shell env inheritance leak path cleanly.Using a sanitized explicit env with
extendEnv: falseis the right hardening for session shell subprocesses.packages/opencode/test/session/prompt-effect.test.ts (1)
1252-1289: Strong regression coverage for shell env sanitization.This test cleanly verifies both sides of the contract (internal auth env stripped, user env preserved) and safely restores
process.envinfinally.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/util/env.ts`:
- Around line 3-8: withoutInternalServerAuthEnv currently mutates the passed-in
env object; change it to create and return a shallow copy instead of deleting
keys on the original to avoid caller-side effects—inside the function make a
copy (e.g. const safe = { ...env } as T), iterate Object.keys(safe) and delete
from safe when key.toLowerCase() is in INTERNAL_SERVER_AUTH_ENV, then return
safe; keep the function signature and generic T and reference the existing
function name without modifying other callers.
🪄 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: f72e43fa-6b24-411d-b557-3aef24e0486b
📒 Files selected for processing (5)
packages/app/test/e2e-backend-env.test.tspackages/opencode/src/pty/index.tspackages/opencode/src/util/env.tspackages/opencode/test/pty/pty-session.test.tspackages/opencode/test/tool/bash.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: unit-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-app
- GitHub Check: unit-desktop
- GitHub Check: unit-opencode
- GitHub Check: typecheck
- GitHub Check: smoke-macos-arm64
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (3)
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/test/e2e-backend-env.test.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/bash.test.tspackages/opencode/src/util/env.tspackages/opencode/test/pty/pty-session.test.tspackages/opencode/src/pty/index.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/bash.test.tspackages/opencode/test/pty/pty-session.test.ts
🧠 Learnings (39)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 327
File: packages/opencode/test/pty/pty-session.test.ts:124-136
Timestamp: 2026-04-29T10:34:22.399Z
Learning: In `packages/opencode/test/pty/pty-session.test.ts` (Astro-Han/pawwork, PR `#327`), the `bun-pty` backend re-merges the parent process environment internally after `withoutInternalServerAuthEnv` strips auth values. This means `OPENCODE_SERVER_USERNAME` and `OPENCODE_SERVER_PASSWORD` will be **present-but-empty** (not fully unset) inside the PTY session. Using a `-unset` shell default expansion (e.g. `${OPENCODE_SERVER_USERNAME-unset}`) would never trigger, so asserting `username=unset` / `password=unset` would always fail. The correct assertion strategy is to check `expect(text).not.toContain("secret")` and `expect(text).not.toContain("PawWork")`. Do NOT suggest converting to `-unset` expansion or asserting `username=unset` / `password=unset` for PTY auth-env tests in this file.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:02.858Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:311-328
Timestamp: 2026-04-28T05:36:18.200Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the ripgrep binary download flow (`HttpClientRequest.get` → `fs.writeWithDirs` → `extract`) does NOT verify a pinned SHA-256 checksum. Adding per-platform hash verification (15.1.0 × 6 platforms) requires a dedicated manifest + `crypto.subtle.digest` step and is intentionally deferred to a follow-up PR. Existing mitigations: (1) `VERSION` is pinned to `"15.1.0"`, (2) downloads are over HTTPS from GitHub Releases, (3) PawWork production builds ship a bundled `rg` binary so this download path is a last-resort fallback (system PATH → cached → download). Do NOT re-flag the missing checksum verification in this file until the dedicated follow-up PR lands.
📚 Learning: 2026-04-29T10:34:22.399Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 327
File: packages/opencode/test/pty/pty-session.test.ts:124-136
Timestamp: 2026-04-29T10:34:22.399Z
Learning: In `packages/opencode/test/pty/pty-session.test.ts` (Astro-Han/pawwork, PR `#327`), the `bun-pty` backend re-merges the parent process environment internally after `withoutInternalServerAuthEnv` strips auth values. This means `OPENCODE_SERVER_USERNAME` and `OPENCODE_SERVER_PASSWORD` will be **present-but-empty** (not fully unset) inside the PTY session. Using a `-unset` shell default expansion (e.g. `${OPENCODE_SERVER_USERNAME-unset}`) would never trigger, so asserting `username=unset` / `password=unset` would always fail. The correct assertion strategy is to check `expect(text).not.toContain("secret")` and `expect(text).not.toContain("PawWork")`. Do NOT suggest converting to `-unset` expansion or asserting `username=unset` / `password=unset` for PTY auth-env tests in this file.
Applied to files:
packages/app/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.test.tspackages/opencode/src/pty/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} : 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/app/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.test.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-28T05:36:24.561Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.
Applied to files:
packages/app/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/src/util/env.tspackages/opencode/test/pty/pty-session.test.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-28T04:56:21.338Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/tool-define.test.ts:2-9
Timestamp: 2026-04-28T04:56:21.338Z
Learning: In `packages/opencode/test/tool/tool-define.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a `ManagedRuntime.make(Layer.mergeAll(Truncate.defaultLayer, Agent.defaultLayer))` wrapper and raw `bun:test` `test(...)` cases rather than the `testEffect(...)` harness. This code was adopted wholesale from upstream via the graft strategy (per `project_upstream_strategy.md`). Migrating to `testEffect` is deferred to a follow-up PR so as not to mix refactor + harness-migration intents and to avoid drifting the diff from the upstream baseline needed for clean future grafts. Do NOT re-flag the `ManagedRuntime.make` usage in this file as needing harness migration until that follow-up lands.
Applied to files:
packages/app/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.ts
📚 Learning: 2026-04-20T14:35:49.291Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/AGENTS.md:0-0
Timestamp: 2026-04-20T14:35:49.291Z
Learning: For local UI changes, run the backend and app dev servers separately using `bun run --conditions=browser ./src/index.ts serve --port 4096` for backend and `bun dev -- --port 4444` for app, then open `http://localhost:4444` to verify changes targeting backend at `http://localhost:4096`
Applied to files:
packages/app/test/e2e-backend-env.test.ts
📚 Learning: 2026-04-28T05:36:25.456Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/grep.test.ts:16-22
Timestamp: 2026-04-28T05:36:25.456Z
Learning: In `packages/opencode/test/tool/grep.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a manual `ManagedRuntime.make(Layer.mergeAll(...))` setup and raw `bun:test` test cases rather than the `testEffect(...)` harness. The migration to `testEffect` + `it.live(...)` + `provideTmpdirInstance(...)` is a whole-file rewrite deferred to a dedicated sweep PR that will migrate all tool tests still on the manual ManagedRuntime pattern together. Do NOT re-flag the ManagedRuntime setup in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/app/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.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/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.test.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-28T11:24:35.312Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:35.312Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/app/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.test.ts
📚 Learning: 2026-04-27T11:18:47.596Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/agent/agent.test.ts:440-447
Timestamp: 2026-04-27T11:18:47.596Z
Learning: In `packages/opencode/test/agent/agent.test.ts` (Astro-Han/pawwork), all agent-permission tests intentionally use the manual `tmpdir()` + `Instance.provide(...)` pattern. Do not flag individual tests in this file for conversion to `provideTmpdirInstance(...)` or `provideInstance(...)`; a full harness migration would be a separate PR if the pattern ever needs to change.
Applied to files:
packages/app/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.test.tspackages/opencode/src/pty/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 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/app/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.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/app/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.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/app/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/src/pty/index.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/test/e2e-backend-env.test.tspackages/opencode/test/tool/bash.test.tspackages/opencode/src/pty/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/test/e2e-backend-env.test.ts
📚 Learning: 2026-04-28T04:56:18.533Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:172-175
Timestamp: 2026-04-28T04:56:18.533Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the `files dies on nonexistent directory` test hardcodes `/tmp/nonexistent-dir-12345` as the missing-directory path. This is upstream-inherited behaviour adopted wholesale via the graft strategy (`project_upstream_strategy.md`). The fix — replacing the hardcoded path with a platform-neutral guaranteed-nonexistent child path derived from the `tmpdir` fixture — is intentionally deferred to a follow-up PR or upstream report, to avoid mixing bugfix + refactor intents and drifting the diff from the upstream baseline future syncs need. Do NOT re-flag the hardcoded `/tmp/nonexistent-dir-12345` path in this file as a blocking or actionable issue until the follow-up PR lands.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.test.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-27T12:59:49.844Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/test/session/prompt-effect.test.ts:0-0
Timestamp: 2026-04-27T12:59:49.844Z
Learning: In `packages/opencode/test/session/prompt-effect.test.ts` and `packages/opencode/src/session/diagnostics.ts` (PR `#264`), the recovery reminder copy differs between signature kinds: the input-repeat variant says "repeated the same tool input 3 times" (uses a literal count), while the target-repeat variant says "failed against the same target multiple times" (uses "multiple times" with no count). Assertions that check for injected reminder text in LLM inputs must accept both phrasings when a scenario produces both `input:` and `target:` signatures (e.g., `read` tool with a `filePath` parameter). Do NOT narrow the assertion to only the input-variant phrasing.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.test.ts
📚 Learning: 2026-04-28T08:29:02.858Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:02.858Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.
Applied to files:
packages/opencode/test/tool/bash.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/tool/bash.test.tspackages/opencode/test/pty/pty-session.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/tool/bash.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/tool/bash.test.tspackages/opencode/test/pty/pty-session.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : When a test needs instance-local state, prefer `provideTmpdirInstance(...)` or `provideInstance(...)` over manual `Instance.provide(...)` inside Promise-style tests.
Applied to files:
packages/opencode/test/tool/bash.test.tspackages/opencode/test/pty/pty-session.test.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/util/env.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/util/env.tspackages/opencode/src/pty/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 : 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/pty/pty-session.test.tspackages/opencode/src/pty/index.ts
📚 Learning: 2026-04-24T03:51:56.211Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:56.211Z
Learning: In Astro-Han/pawwork E2E tests (packages/app/e2e/fixtures.ts), `project.prompt(text)` internally calls `trackSession(next.sessionID, active.directory)` after the prompt submission is observed and the active session is resolved. Tests that obtain a `sessionID` from `project.prompt()` do NOT need to call `project.trackSession(sessionID)` manually — it is already registered for teardown. Calling it again would be duplicate ownership.
Applied to files:
packages/opencode/test/pty/pty-session.test.ts
📚 Learning: 2026-04-28T04:38:11.771Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:11.771Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Applied to files:
packages/opencode/test/pty/pty-session.test.tspackages/opencode/src/pty/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 : Prefer `ChildProcessSpawner.ChildProcessSpawner` with `ChildProcess.make(...)` instead of custom process wrappers in Effect services
Applied to files:
packages/opencode/src/pty/index.ts
📚 Learning: 2026-04-23T08:51:04.230Z
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:04.230Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/util/filesystem.ts`), the `Filesystem` utility does NOT expose a `remove` or `unlink` helper. The established repository pattern for auth.json teardown in tests (e.g. `provider.test.ts`, `amazon-bedrock.test.ts`, `workspace-adaptor.test.ts`) is to combine `Filesystem.write` with `node:fs/promises unlink`. Do not flag this mixed usage as inconsistent — it is the correct and intentional pattern.
Applied to files:
packages/opencode/src/pty/index.ts
📚 Learning: 2026-04-26T16:34:57.130Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 247
File: packages/ui/src/components/message-part.tsx:1322-1324
Timestamp: 2026-04-26T16:34:57.130Z
Learning: In Astro-Han/pawwork (`packages/ui/src/components/message-part.tsx`), the `taskId` createMemo and `childSessionId` createMemo both intentionally read only from `partMetadata().sessionId` (populated post-execution), not from `input.task_id` / `input.subagent_session_id`. This has always been the case — the original code never read the input field either. Adding an `input.subagent_session_id` fallback would be a new capability, not a bug fix. Do NOT flag the absence of this fallback as a regression in PR `#247` or future PRs unless there is a concrete case where metadata is not populated.
Applied to files:
packages/opencode/src/pty/index.ts
📚 Learning: 2026-04-28T04:38:11.727Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/edit.ts:35-45
Timestamp: 2026-04-28T04:38:11.727Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork, PR `#270`), the module-scoped `Map<string, Semaphore.Semaphore>` (`locks`) and `lock(filePath)` helper are intentionally left as a global, never-cleaned registry. PR `#270` is a pure upstream-sync graft and mixing in a bugfix would drift the diff from the upstream baseline. The fix (moving per-file semaphores into `InstanceState`/`ScopedCache` for per-instance lifecycle management) is tracked as a follow-up to land in its own PR or be reported upstream. Do NOT re-flag the absence of InstanceState wiring for the lock map in this file until the follow-up PR is opened.
Applied to files:
packages/opencode/src/pty/index.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/opencode/src/pty/index.ts
📚 Learning: 2026-04-27T10:33:12.228Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:12.228Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/prompt.ts` and `packages/opencode/src/session/processor.ts`, PR `#264`), the loop-gate race condition between `buildLoopContext()` and `recordSyntheticBlock`/`recordSyntheticStop` is intentionally handled via idempotence guards (re-check sigKey presence / `hasStopped` inside the record helpers) rather than a full per-parent `Effect.Mutex`. Threading a `Map<MessageID, Mutex>` through the processor was considered too large a surface change for this edge case; the residual TOCTOU window only produces extra synthetic parts with no behavioral drift on the "turn ends" contract. A code comment documents the trade-off and points to a full-mutex follow-up if the race is observed in practice. Do NOT re-flag the absence of a per-parent mutex as a blocking issue in future reviews.
Applied to files:
packages/opencode/src/pty/index.ts
📚 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/opencode/src/pty/index.ts
📚 Learning: 2026-04-28T04:56:21.528Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/tool.ts:34-42
Timestamp: 2026-04-28T04:56:21.528Z
Learning: In `packages/opencode/src/tool/tool.ts` (Astro-Han/pawwork, PR `#270`), the `Def.execute` signature uses `ctx: Context` (without the `M` generic) instead of `ctx: Context<M>`, meaning `ctx.metadata(...)` is not checked against the same `M` as `ExecuteResult<M>`. This is inherited upstream behavior adopted via the graft strategy in PR `#270` (an upstream-sync PR per `project_upstream_strategy.md`). Fixing it here would mix refactor + bugfix intents and drift the diff from the upstream baseline. The fix is deferred to a dedicated follow-up PR or upstream report. Do NOT re-flag the missing `Context<M>` generic in `Def.execute` during upstream-sync PRs; only flag it in a PawWork-authored PR that directly touches `tool.ts` type signatures.
Applied to files:
packages/opencode/src/pty/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 : Prefer `Path.Path`, `Config`, `Clock`, and `DateTime` services when those concerns are already inside Effect code
Applied to files:
packages/opencode/src/pty/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 `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/pty/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.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/pty/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 : Prefer `FileSystem.FileSystem` instead of raw `fs/promises` for effectful file I/O in Effect services
Applied to files:
packages/opencode/src/pty/index.ts
🔇 Additional comments (4)
packages/app/test/e2e-backend-env.test.ts (1)
5-25: Good regression coverage for backend env isolation.This test correctly validates case-insensitive stripping of internal auth vars while ensuring unrelated env values remain intact.
packages/opencode/test/tool/bash.test.ts (1)
157-215: Strong bash-path regression test for env sanitization.Line 157-Line 215 covers both uppercase/lowercase auth keys, validates no secret leakage, and restores mutated
process.envreliably.packages/opencode/test/pty/pty-session.test.ts (1)
93-206: Good PTY regression coverage for auth redaction and explicit override behavior.These tests validate the two intended outcomes: inherited creds are not exposed, and caller-provided PTY auth env overrides are preserved.
packages/opencode/src/pty/index.ts (1)
194-199: No action needed — lowercase auth key inheritance is already prevented.The
withoutInternalServerAuthEnvfunction (inpackages/opencode/src/util/env.tslines 3–7) performs case-insensitive deletion of auth keys: it iterates through all keys and deletes any that matchOPENCODE_SERVER_USERNAMEorOPENCODE_SERVER_PASSWORD(case-insensitive comparison viakey.toLowerCase()). After this sanitization at line 187, the env object contains no auth key variants—uppercase or lowercase—before being passed to bun-pty. The explicit assignment at lines 198–199 then sets only the uppercase keys. The design is correct and complete.> Likely an incorrect or invalid review comment.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/test/util/env.test.ts`:
- Around line 5-21: The test only covers uppercase auth keys; add a mixed-case
key (e.g., "OpEnCoDe_Server_Username") to the env fixture in the "does not
mutate caller-owned env objects" test and assert that
withoutInternalServerAuthEnv strips that mixed-case key from the returned
sanitized object while leaving the original env unchanged and ensuring sanitized
!== env; update the expected sanitized result to still only contain
PAWWORK_E2E_CUSTOM_ENV and keep the original env expectation to include the
mixed-case key.
🪄 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: adc69e05-d926-45f2-8870-cadc4a072eda
📒 Files selected for processing (2)
packages/opencode/src/util/env.tspackages/opencode/test/util/env.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: typecheck
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-desktop
- GitHub Check: unit-opencode
- GitHub Check: unit-app
- GitHub Check: smoke-macos-arm64
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.ts: UseEffect.gen(function* () { ... })for Effect composition
UseEffect.fn("Domain.method")for named/traced effects andEffect.fnUntracedfor internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer.pipe()wrappers
UseEffect.callbackfor callback-based APIs
PreferDateTime.nowAsDateovernew Date(yield* Clock.currentTimeMillis)when you need aDatein Effect code
UseSchema.Classfor multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
UseSchema.TaggedErrorClassfor typed errors in Effect schemas
UseSchema.Defectinstead ofunknownfor defect-like causes in Effect code
InEffect.gen/Effect.fn, preferyield* new MyError(...)overyield* Effect.fail(new MyError(...))for direct early-failure branches
UsemakeRuntimefromsrc/effect/run-service.tsfor all services; it returns{ runPromise, runFork, runCallback }backed by a sharedmemoMapthat deduplicates layers
UseInstanceStatefromsrc/effect/instance-state.tsfor per-directory or per-project state that needs per-instance cleanup; do work directly in theInstanceState.makeclosure whereScopedCachehandles run-once semantics
UseEffect.addFinalizerorEffect.acquireReleaseinside theInstanceState.makeclosure for cleanup (subscriptions, process teardown, etc.)
UseEffect.forkScopedinside theInstanceState.makeclosure for background stream consumers — the fiber is interrupted when the instance is disposed
PreferFileSystem.FileSysteminstead of rawfs/promisesfor effectful file I/O in Effect services
PreferChildProcessSpawner.ChildProcessSpawnerwithChildProcess.make(...)instead of custom process wrappers in Effect services
PreferHttpClient.HttpClientinstead of rawfetchin Effect services
PreferPath.Path,Config,Clock, andDateTimeservices when those concerns are already inside Effect code
For backgroun...
Files:
packages/opencode/test/util/env.test.tspackages/opencode/src/util/env.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/util/env.test.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 327
File: packages/opencode/test/pty/pty-session.test.ts:124-136
Timestamp: 2026-04-29T10:34:22.399Z
Learning: In `packages/opencode/test/pty/pty-session.test.ts` (Astro-Han/pawwork, PR `#327`), the `bun-pty` backend re-merges the parent process environment internally after `withoutInternalServerAuthEnv` strips auth values. This means `OPENCODE_SERVER_USERNAME` and `OPENCODE_SERVER_PASSWORD` will be **present-but-empty** (not fully unset) inside the PTY session. Using a `-unset` shell default expansion (e.g. `${OPENCODE_SERVER_USERNAME-unset}`) would never trigger, so asserting `username=unset` / `password=unset` would always fail. The correct assertion strategy is to check `expect(text).not.toContain("secret")` and `expect(text).not.toContain("PawWork")`. Do NOT suggest converting to `-unset` expansion or asserting `username=unset` / `password=unset` for PTY auth-env tests in this file.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:02.858Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:11.771Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:311-328
Timestamp: 2026-04-28T05:36:18.200Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the ripgrep binary download flow (`HttpClientRequest.get` → `fs.writeWithDirs` → `extract`) does NOT verify a pinned SHA-256 checksum. Adding per-platform hash verification (15.1.0 × 6 platforms) requires a dedicated manifest + `crypto.subtle.digest` step and is intentionally deferred to a follow-up PR. Existing mitigations: (1) `VERSION` is pinned to `"15.1.0"`, (2) downloads are over HTTPS from GitHub Releases, (3) PawWork production builds ship a bundled `rg` binary so this download path is a last-resort fallback (system PATH → cached → download). Do NOT re-flag the missing checksum verification in this file until the dedicated follow-up PR lands.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 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.
📚 Learning: 2026-04-29T10:34:22.399Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 327
File: packages/opencode/test/pty/pty-session.test.ts:124-136
Timestamp: 2026-04-29T10:34:22.399Z
Learning: In `packages/opencode/test/pty/pty-session.test.ts` (Astro-Han/pawwork, PR `#327`), the `bun-pty` backend re-merges the parent process environment internally after `withoutInternalServerAuthEnv` strips auth values. This means `OPENCODE_SERVER_USERNAME` and `OPENCODE_SERVER_PASSWORD` will be **present-but-empty** (not fully unset) inside the PTY session. Using a `-unset` shell default expansion (e.g. `${OPENCODE_SERVER_USERNAME-unset}`) would never trigger, so asserting `username=unset` / `password=unset` would always fail. The correct assertion strategy is to check `expect(text).not.toContain("secret")` and `expect(text).not.toContain("PawWork")`. Do NOT suggest converting to `-unset` expansion or asserting `username=unset` / `password=unset` for PTY auth-env tests in this file.
Applied to files:
packages/opencode/test/util/env.test.tspackages/opencode/src/util/env.ts
📚 Learning: 2026-04-27T11:19:24.963Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch-auth.test.ts:0-0
Timestamp: 2026-04-27T11:19:24.963Z
Learning: In `packages/opencode/test/tool/websearch-auth.test.ts` (Astro-Han/pawwork), the tests intentionally use a small local `runWith` runner with raw `bun:test` and `Effect.runPromise` rather than the `testEffect` harness. Each test case injects a custom in-memory `Auth.Service` layer; switching to `testEffect` would be style-only churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/opencode/test/util/env.test.ts
📚 Learning: 2026-04-28T05:36:24.561Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.
Applied to files:
packages/opencode/test/util/env.test.tspackages/opencode/src/util/env.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `config` option in `tmpdir` to write an `opencode.json` config file during test setup by passing a partial Config.Info object.
Applied to files:
packages/opencode/test/util/env.test.ts
📚 Learning: 2026-04-27T11:18:47.596Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/agent/agent.test.ts:440-447
Timestamp: 2026-04-27T11:18:47.596Z
Learning: In `packages/opencode/test/agent/agent.test.ts` (Astro-Han/pawwork), all agent-permission tests intentionally use the manual `tmpdir()` + `Instance.provide(...)` pattern. Do not flag individual tests in this file for conversion to `provideTmpdirInstance(...)` or `provideInstance(...)`; a full harness migration would be a separate PR if the pattern ever needs to change.
Applied to files:
packages/opencode/test/util/env.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.
Applied to files:
packages/opencode/test/util/env.test.ts
📚 Learning: 2026-04-28T05:36:25.456Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/grep.test.ts:16-22
Timestamp: 2026-04-28T05:36:25.456Z
Learning: In `packages/opencode/test/tool/grep.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a manual `ManagedRuntime.make(Layer.mergeAll(...))` setup and raw `bun:test` test cases rather than the `testEffect(...)` harness. The migration to `testEffect` + `it.live(...)` + `provideTmpdirInstance(...)` is a whole-file rewrite deferred to a dedicated sweep PR that will migrate all tool tests still on the manual ManagedRuntime pattern together. Do NOT re-flag the ManagedRuntime setup in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/opencode/test/util/env.test.ts
📚 Learning: 2026-04-28T04:56:21.338Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/tool-define.test.ts:2-9
Timestamp: 2026-04-28T04:56:21.338Z
Learning: In `packages/opencode/test/tool/tool-define.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a `ManagedRuntime.make(Layer.mergeAll(Truncate.defaultLayer, Agent.defaultLayer))` wrapper and raw `bun:test` `test(...)` cases rather than the `testEffect(...)` harness. This code was adopted wholesale from upstream via the graft strategy (per `project_upstream_strategy.md`). Migrating to `testEffect` is deferred to a follow-up PR so as not to mix refactor + harness-migration intents and to avoid drifting the diff from the upstream baseline needed for clean future grafts. Do NOT re-flag the `ManagedRuntime.make` usage in this file as needing harness migration until that follow-up lands.
Applied to files:
packages/opencode/test/util/env.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/util/env.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : When a test needs instance-local state, prefer `provideTmpdirInstance(...)` or `provideInstance(...)` over manual `Instance.provide(...)` inside Promise-style tests.
Applied to files:
packages/opencode/test/util/env.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/util/env.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/util/env.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/util/env.test.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/util/env.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/util/env.ts
🔇 Additional comments (2)
packages/opencode/src/util/env.ts (2)
3-9: Sanitizer copy-and-filter behavior is correct.This safely preserves input immutability while removing internal auth keys case-insensitively.
11-14: Case-insensitive env lookup helper looks good.The normalization + entry scan correctly returns the matched value regardless of key casing.
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
Why
Fixes #326. PawWork can run with
OPENCODE_SERVER_USERNAMEandOPENCODE_SERVER_PASSWORDin its internal server environment. Those values were leaking into user command subprocesses and isolated E2E backends, causing/global/healthto return 401 and exposing internal auth values to commands/terminals.Related Issue
Fixes #326
How To Verify
Screenshots or Recordings
Not applicable; no visible UI changes.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
New Features
Bug Fixes
Tests