Skip to content

feat: make open permissions the default#296

Merged
Astro-Han merged 1 commit intodevfrom
codex/feat-open-permissions
Apr 28, 2026
Merged

feat: make open permissions the default#296
Astro-Han merged 1 commit intodevfrom
codex/feat-open-permissions

Conversation

@Astro-Han
Copy link
Copy Markdown
Owner

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

Summary

Make PawWork's default permission mode fully open for external directory access while keeping targeted guardrails for sensitive files and direct deletion commands.

Why

The previous default was confusing: most permissions were open, but file and bash access outside the workspace still requested external_directory approval. For PawWork's default personal-desktop flow, the workspace should be the task's default location and context anchor, not a hard security boundary. This keeps the default experience consistent while leaving restricted mode as future work.

Related Issue

Closes #293

How To Verify

bun install --frozen-lockfile
bun test test/permission/pawwork-defaults.test.ts test/agent/agent.test.ts test/tool/read.test.ts test/tool/trash.test.ts
bun run typecheck

Screenshots or Recordings

Not applicable. This changes harness permission defaults and tests, with no visible UI changes.

Checklist

  • I linked the related issue, or stated why there is no issue
  • This PR has type, scope, and priority labels, or I requested maintainer labeling
  • I listed the relevant verification steps, including tests when behavior changed
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for desktop, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, or generated/local file changes when relevant
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English

Summary by CodeRabbit

  • New Features

    • External directory access now defaults to allow instead of prompting.
  • Bug Fixes

    • Sudo-related shell actions now prompt instead of being automatically denied.
    • Added explicit deny rules for destructive PowerShell/Windows deletion commands.
  • Tests

    • Permission-related tests updated to reflect the new default behaviors.

@Astro-Han Astro-Han added enhancement New feature or request P2 Medium priority harness Model harness, prompts, tool descriptions, and session mechanics labels Apr 28, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9198fa7a-10ff-4b56-8aea-ad81cb56c6fd

📥 Commits

Reviewing files that changed from the base of the PR and between 598a0f1 and 10fb003.

📒 Files selected for processing (4)
  • packages/opencode/src/agent/agent.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/tool/read.test.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: smoke-macos-arm64
  • GitHub Check: unit-windows-app
  • GitHub Check: unit-windows-desktop
  • GitHub Check: unit-windows-opencode-session
  • GitHub Check: unit-windows-opencode-config-project
  • GitHub Check: unit-windows-opencode-server-tools
  • GitHub Check: unit-opencode
  • GitHub Check: unit-app
  • GitHub Check: unit-desktop
  • GitHub Check: typecheck
  • GitHub Check: e2e-artifacts
  • GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts

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

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

Files:

  • packages/opencode/test/tool/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/src/agent/agent.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
packages/opencode/test/**/*.test.{ts,tsx}

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

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

Files:

  • packages/opencode/test/tool/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
🧠 Learnings (30)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:58.310Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 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.
📚 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/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/src/agent/agent.ts
  • packages/opencode/test/permission/pawwork-defaults.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/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/test/permission/pawwork-defaults.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/tool/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/test/permission/pawwork-defaults.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/tool/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/test/permission/pawwork-defaults.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/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/src/agent/agent.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
📚 Learning: 2026-04-21T16:57:25.580Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 102
File: packages/opencode/src/config/agent.ts:108-119
Timestamp: 2026-04-21T16:57:25.580Z
Learning: In `packages/opencode/src/config/agent.ts` (Astro-Han/pawwork), `ConfigPermission.Info` only accepts permission objects or the three action strings `"ask"`, `"allow"`, `"deny"`, and transforms those action strings into `{ "*": action }` before `normalize()` runs. By the time `normalize()` is reached, `configuredPermission` is always either `undefined` or a `Record<string, Rule>` — never a raw arbitrary string. The `Object.assign(permission, configuredPermission)` pattern is therefore safe. Do not flag it as corrupting string permission references.

Applied to files:

  • packages/opencode/test/tool/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/src/agent/agent.ts
  • packages/opencode/test/permission/pawwork-defaults.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/tool/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/test/permission/pawwork-defaults.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/read.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `config` option in `tmpdir` to write an `opencode.json` config file during test setup by passing a partial Config.Info object.

Applied to files:

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

Applied to files:

  • packages/opencode/test/tool/read.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/read.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Define `const it = testEffect(...)` near the top of the test file and keep the test body inside `Effect.gen(function* () { ... })`. Yield services directly with `yield* MyService.Service` or `yield* MyTool`.

Applied to files:

  • packages/opencode/test/tool/read.test.ts
  • packages/opencode/src/agent/agent.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use `it.effect(...)` when the test should run with `TestClock` and `TestConsole`. Use `it.live(...)` when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.

Applied to files:

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

Applied to files:

  • packages/opencode/test/tool/read.test.ts
  • packages/opencode/test/agent/agent.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/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/test/permission/pawwork-defaults.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/read.test.ts
  • packages/opencode/test/agent/agent.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/read.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
📚 Learning: 2026-04-27T11:19:24.963Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch-auth.test.ts:0-0
Timestamp: 2026-04-27T11:19:24.963Z
Learning: In `packages/opencode/test/tool/websearch-auth.test.ts` (Astro-Han/pawwork), the tests intentionally use a small local `runWith` runner with raw `bun:test` and `Effect.runPromise` rather than the `testEffect` harness. Each test case injects a custom in-memory `Auth.Service` layer; switching to `testEffect` would be style-only churn without changing risk coverage. Do not flag these tests as needing harness migration.

Applied to files:

  • packages/opencode/test/agent/agent.test.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/agent/agent.ts
📚 Learning: 2026-04-28T08:29:02.858Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:02.858Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.

Applied to files:

  • packages/opencode/src/agent/agent.ts
📚 Learning: 2026-04-28T05:36:18.200Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:311-328
Timestamp: 2026-04-28T05:36:18.200Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the ripgrep binary download flow (`HttpClientRequest.get` → `fs.writeWithDirs` → `extract`) does NOT verify a pinned SHA-256 checksum. Adding per-platform hash verification (15.1.0 × 6 platforms) requires a dedicated manifest + `crypto.subtle.digest` step and is intentionally deferred to a follow-up PR. Existing mitigations: (1) `VERSION` is pinned to `"15.1.0"`, (2) downloads are over HTTPS from GitHub Releases, (3) PawWork production builds ship a bundled `rg` binary so this download path is a last-resort fallback (system PATH → cached → download). Do NOT re-flag the missing checksum verification in this file until the dedicated follow-up PR lands.

Applied to files:

  • packages/opencode/src/agent/agent.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/agent/agent.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/agent/agent.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/agent/agent.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/agent/agent.ts
📚 Learning: 2026-04-20T14:36:21.288Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.288Z
Learning: Applies to packages/opencode/**/*.ts : Use `InstanceState` from `src/effect/instance-state.ts` for per-directory or per-project state that needs per-instance cleanup; do work directly in the `InstanceState.make` closure where `ScopedCache` handles run-once semantics

Applied to files:

  • packages/opencode/src/agent/agent.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/src/agent/agent.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/agent/agent.ts
📚 Learning: 2026-04-28T08:14:31.436Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/edit.ts:48-48
Timestamp: 2026-04-28T08:14:31.436Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork), the `filePath` schema description (`"The absolute path to the file to modify"`) is upstream-inherited from `dev:packages/opencode/src/tool/edit.ts:48`. The runtime actually accepts relative paths (resolved via `Instance.directory` at lines 79-81), but the description fix is intentionally deferred to a single PawWork-authored description-cleanup PR that will also cover the identical mismatch in `packages/opencode/src/tool/write.ts:24`. Do NOT re-flag the too-narrow `filePath` description in upstream-sync PRs; flag it only in the dedicated description-cleanup PR.

Applied to files:

  • packages/opencode/test/permission/pawwork-defaults.test.ts
🔇 Additional comments (6)
packages/opencode/src/agent/agent.ts (2)

80-109: Permission defaults correctly implement the "open by default" policy.

The changes align with PR objectives:

  • external_directory: {"*": "allow"} enables open access outside workspace
  • sudo * moved to "ask" allows advanced users to approve
  • Destructive commands (dd, mkfs, rm, rmdir, unlink, find * -delete) remain denied
  • PowerShell equivalents (Remove-Item, del, erase, rd) added to deny list
  • Sensitive file reads (.env, .env.*) correctly remain as "ask" while .env.example is allowed

158-160: Explore agent external_directory permission correctly updated.

Consistent with the build agent change, the explore agent now allows external directory access by default.

packages/opencode/test/agent/agent.test.ts (2)

67-79: Test correctly updated to verify explore agent allows external directories.

The test description and assertion at line 75 properly reflect the new default where external_directory evaluates to "allow" instead of "ask".


416-426: Test correctly updated to verify build agent allows external_directory.

The test description and assertion at line 423 properly reflect the new default behavior where external_directory is "allow" rather than "ask".

packages/opencode/test/tool/read.test.ts (1)

263-275: Test correctly updated to verify build agent allows external_directory access.

The test description and assertion properly reflect the new default behavior. The distinction between the tool requesting permission (tested in lines 133-146) and the agent's ruleset evaluating that request as "allow" (tested here) is correctly maintained.

packages/opencode/test/permission/pawwork-defaults.test.ts (1)

21-36: Comprehensive test coverage for updated PawWork permission defaults.

The test assertions correctly verify all permission changes:

  • external_directory now "allow" (line 21)
  • chmod and kill remain allowed as intended (lines 24-25)
  • Destructive commands properly denied: dd, mkfs*, rmdir, find . -delete (lines 26-27, 29, 31)
  • PowerShell destructive commands properly denied (lines 32-35)
  • sudo changed from "deny" to "ask" (line 36)

📝 Walkthrough

Walkthrough

Default agent permission rules changed: external_directory is now allowed by default; bash permission patterns adjusted (sudo moved to "ask", added denies for destructive Windows/Unix commands); tests updated to reflect new permission expectations.

Changes

Cohort / File(s) Summary
Agent Permission Configuration
packages/opencode/src/agent/agent.ts
Removed Skill.Service dependency and dynamic whitelisted-dir logic; set external_directory default to {"*":"allow"}; changed bash sudo * from denyask; added deny patterns for destructive commands (e.g., Remove-Item *, del *, erase *, rd *).
Agent Permission Tests
packages/opencode/test/agent/agent.test.ts
Updated expectations: external_directory now returns "allow" for explore and build agents where previously "ask"; adjusted test descriptions.
PawWork Permission Defaults Tests
packages/opencode/test/permission/pawwork-defaults.test.ts
Changed default external_directory expectation from "ask""allow"; added/updated bash command assertions (allow chmod +x, kill; deny dd, mkfs.ext4, rmdir, find . -delete, Remove-Item, del, erase, rd; sudo rm -rf / now "ask").
External Directory Read Tool Tests
packages/opencode/test/tool/read.test.ts
Updated live test to expect external_directory "allow" for default build agent and adjusted test description.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Poem

🐰 I hop through files with tiny paws,
External paths now open without pause,
Sudo asks kindly before it roams,
Destructive commands stay outside our homes,
Happy hops — safer code, wider lawns!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: make open permissions the default' directly and concisely describes the primary change of the PR.
Description check ✅ Passed The PR description is comprehensive, following the template structure with all required sections: Summary, Why, Related Issue, How To Verify, and a completed Checklist.
Linked Issues check ✅ Passed The code changes fully implement the objectives from issue #293: external_directory defaults to allow, sensitive files remain ask, deletion commands blocked, sudo changed to ask, and tests updated accordingly.
Out of Scope Changes check ✅ Passed All changes are in-scope: permission defaults updated, tests adjusted to reflect new behavior, and no unrelated modifications introduced.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/feat-open-permissions

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

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the default agent permissions to be more permissive, notably changing 'external_directory' access from 'ask' to 'allow' and 'sudo' commands from 'deny' to 'ask'. It also adds several Windows-specific deletion commands to the 'bash' deny list. Review feedback highlights a security concern regarding the removal of high-risk commands like 'dd' and 'chmod' from the deny list and identifies redundant configuration entries for 'external_directory' that should be cleaned up for better maintainability.

Comment thread packages/opencode/src/agent/agent.ts
Comment thread packages/opencode/src/agent/agent.ts Outdated
Comment thread packages/opencode/src/agent/agent.ts Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

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

⚠️ Outside diff range comments (1)
packages/opencode/test/tool/read.test.ts (1)

263-274: ⚠️ Potential issue | 🟠 Major

Finish updating the external-directory read assertions in this file.

This expectation matches the new default, but Lines 133-145, 169-181, and 184-193 in the same file still expect external_directory to prompt when reading outside the project. With packages/opencode/src/agent/agent.ts now defaulting external_directory["*"] to "allow", those tests should stop recording an ask and will fail until their expectations are updated or scoped to a ruleset that still asks.

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

In `@packages/opencode/test/tool/read.test.ts` around lines 263 - 274, Update the
external-directory read assertions in the test file so they match the new
default of external_directory["*"] === "allow": locate the three failing test
blocks (the ones covering Lines 133-145, 169-181, and 184-193 that currently
expect a prompt/ask) and change their expectation from recording an "ask" to
expecting "allow" (or alternatively wrap those tests with a ruleset override
that sets external_directory back to "ask"); ensure you update the assertions
that inspect Permission.evaluate results (the same pattern used in the "default
build agent allows external_directory access outside the project" test) so they
check rule.action === "allow" or otherwise scope the test to a ruleset that
still prompts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/opencode/test/tool/read.test.ts`:
- Around line 263-274: Update the external-directory read assertions in the test
file so they match the new default of external_directory["*"] === "allow":
locate the three failing test blocks (the ones covering Lines 133-145, 169-181,
and 184-193 that currently expect a prompt/ask) and change their expectation
from recording an "ask" to expecting "allow" (or alternatively wrap those tests
with a ruleset override that sets external_directory back to "ask"); ensure you
update the assertions that inspect Permission.evaluate results (the same pattern
used in the "default build agent allows external_directory access outside the
project" test) so they check rule.action === "allow" or otherwise scope the test
to a ruleset that still prompts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fd2e49a1-84e7-41a8-94a2-2ad26334a654

📥 Commits

Reviewing files that changed from the base of the PR and between 74656aa and 598a0f1.

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

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

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

Files:

  • packages/opencode/test/tool/read.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/src/agent/agent.ts
packages/opencode/test/**/*.test.{ts,tsx}

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

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

Files:

  • packages/opencode/test/tool/read.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/agent/agent.test.ts
🧠 Learnings (19)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:58.310Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 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.
📚 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/read.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/src/agent/agent.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/read.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/agent/agent.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/tool/read.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/agent/agent.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/read.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/agent/agent.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/tool/read.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/agent/agent.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `config` option in `tmpdir` to write an `opencode.json` config file during test setup by passing a partial Config.Info object.

Applied to files:

  • packages/opencode/test/tool/read.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/read.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/agent/agent.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/read.test.ts
  • packages/opencode/test/permission/pawwork-defaults.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/read.test.ts
  • packages/opencode/test/agent/agent.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/read.test.ts
  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/agent/agent.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Define `const it = testEffect(...)` near the top of the test file and keep the test body inside `Effect.gen(function* () { ... })`. Yield services directly with `yield* MyService.Service` or `yield* MyTool`.

Applied to files:

  • packages/opencode/test/tool/read.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use `it.effect(...)` when the test should run with `TestClock` and `TestConsole`. Use `it.live(...)` when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.

Applied to files:

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

Applied to files:

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

Applied to files:

  • packages/opencode/test/permission/pawwork-defaults.test.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/src/agent/agent.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/permission/pawwork-defaults.test.ts
📚 Learning: 2026-04-27T11:19:24.963Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch-auth.test.ts:0-0
Timestamp: 2026-04-27T11:19:24.963Z
Learning: In `packages/opencode/test/tool/websearch-auth.test.ts` (Astro-Han/pawwork), the tests intentionally use a small local `runWith` runner with raw `bun:test` and `Effect.runPromise` rather than the `testEffect` harness. Each test case injects a custom in-memory `Auth.Service` layer; switching to `testEffect` would be style-only churn without changing risk coverage. Do not flag these tests as needing harness migration.

Applied to files:

  • packages/opencode/test/agent/agent.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/agent/agent.ts
🔇 Additional comments (1)
packages/opencode/src/agent/agent.ts (1)

97-107: The new Windows delete command denylist is already case-safe.

The Wildcard.match() function in packages/opencode/src/util/wildcard.ts uses case-insensitive regex matching on Windows ("si" flags, line 11). Additionally, packages/opencode/src/tool/bash.ts converts PowerShell commands to lowercase before matching (line 374: const cmd = ps ? tokens[0]?.toLowerCase() : tokens[0]). Together, these ensure that Remove-Item *, remove-item, and REMOVE-ITEM all match correctly and do not fall through to the "*": "allow" fallback.

@Astro-Han Astro-Han force-pushed the codex/feat-open-permissions branch from 598a0f1 to 10fb003 Compare April 28, 2026 12:10
@Astro-Han
Copy link
Copy Markdown
Owner Author

Outside diff range comment on packages/opencode/test/tool/read.test.ts: no code change needed. I verified those earlier tests assert that the read tool emits an external_directory permission request through ctx.ask; they do not assert that the default agent ruleset prompts the user. The default ruleset behavior is covered separately by default build agent allows external_directory access outside the project, and the targeted test run passes.

@Astro-Han Astro-Han merged commit 28bd232 into dev Apr 28, 2026
25 checks passed
@Astro-Han Astro-Han deleted the codex/feat-open-permissions branch April 28, 2026 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request harness Model harness, prompts, tool descriptions, and session mechanics P2 Medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Make open permission mode the default

1 participant