feat: add worktree execution context#360
Conversation
Persisted JSON shape for owner/active directory binding, plus optional active-worktree descriptor. Refs #278.
- Required executionContext field on Session.Info; populated at create time from the user-opened directory captured in session.directory. - fromRow falls back to a synthesized root context when the column is null, so legacy rows surface as if backfilled even before the one-shot UPDATE runs. - toRow persists the field, so any subsequent write durably backfills. - backfillExecutionContext exported for explicit one-shot UPDATE; safe to call repeatedly. Refs #278.
Insert path already includes the field via toRow; this commit adds the partial-update branch so executionContext mutations flow through. Refs #278.
Single mutator for activeDirectory + activeWorktree; ownerDirectory is set at session creation and immutable. activeWorktree=null clears the field for Exit semantics. Refs #278.
Pre-design messages serialised path as a single absolute string;
union now accepts both shapes and lifts the string form to {cwd,
root} where cwd === root. Writers still emit only the modern object
shape. Refs #278.
Per-dispatch lookup of activeDirectory and ownerDirectory ensures EnterWorktree and ExitWorktree take effect on the very next tool call without restructuring the prompt loop. Refs #278.
Promise-shaped wrapper around Instance.provide that always supplies both directory (= activeDirectory) and worktree (= ownerDirectory) to preserve the naming-bridge invariant. Refs #278. Wiring into prompt.ts/processor.ts dispatch loop is deferred: the existing call sites set Instance ALS once at request boundary; mid- turn EnterWorktree updates Session.executionContext but does not yet shift Instance.directory for subsequent tool reads. EnterWorktree itself will rebind via Instance.activate before mutating tool state (see Task 14). Full middleware refactor for cross-request rebinding is a follow-up.
hasInFlightToolCallsExcept queries running/pending tool parts via Session.messages. hasRunningSubagents is currently a stub (returns false) because SubagentRun.activeCounts is private; the parent's running-tool guard already covers the common case since subagents are dispatched through a parent agent tool call. Refs #278.
EnterWorktree binds the session to a git worktree. Three modes: - no-arg: auto-generated slug, new managed worktree - name=: reuse if exists, else create a managed worktree under Global.Path.data/worktree/<projectID>/<name> - path=: take over an external worktree of the same git common-dir State-machine guard refuses transitions while another tool call is running (subagent guard is a stub pending SubagentRun count exposure). A->B switches require ExitWorktree first; A->A is a no-op success. ExitWorktree clears activeWorktree and rebinds activeDirectory = ownerDirectory. From-root is idempotent. Both tools registered in the main toolset. Subagent exclusion is a follow-up (today the registry filter is enabled/disabled only). Refs #278.
When AgentTool spawns a subagent session, it now snapshots the parent's executionContext and applies it to the child immediately after creation. Subagents see the parent's activeDirectory at the moment of dispatch; the parent can mutate later but the child keeps the snapshot. Refs #278.
Adds the new required executionContext field to the Session schema in openapi.json and both v1 and v2 generated TypeScript types. Targeted edit (no regen) per repo convention. Refs #278.
- PawworkWorktreeBadge: portals into the titlebar center slot whenever the active session's executionContext.activeDirectory differs from ownerDirectory and an activeWorktree is bound; shows worktree icon + slug + branch. - Settings → Worktrees tab: lists every PawWork-tracked worktree for the current project (via client.worktree.list()), with a per-row two-step delete. Delete is disabled while any open session in this app instance has the worktree as its activeDirectory. - worktree icon registered in @opencode-ai/ui (placeholder reusing the branch glyph; will be replaced with a dedicated icon). - en + zh i18n strings; zh self-reference uses 爪印. Refs #278.
|
Caution Review failedAn error occurred during the review process. Please try again later. 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 (2)
📜 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). (9)
🧰 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 (3)📚 Learning: 2026-04-23T08:51:00.819ZApplied to files:
📚 Learning: 2026-04-27T08:58:00.665ZApplied to files:
📚 Learning: 2026-04-28T12:49:09.792ZApplied to files:
🔇 Additional comments (9)
📝 WalkthroughWalkthroughAdd persistent session execution context and first-class Git worktree support: DB migration and backfill, canonicalization utilities, a persisted per-project worktree registry with gitignore guard, enter/exit worktree tools and guards, session-bound tool execution, server API/schema updates, UI pages/badges/helpers, and broad test coverage across layers. ChangesWorktree & Session executionContext (single change DAG)
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant ToolSvc as "Tool Runner"
participant WorktreeSvc as "Worktree Registry"
participant Git as "Git"
participant SessionSvc as "Session Service"
participant DB as "Database"
User->>Client: Invoke EnterWorktree (name or path)
Client->>ToolSvc: execute EnterWorktree
ToolSvc->>ToolSvc: run guards (in-flight / subagents)
alt managed (name)
ToolSvc->>WorktreeSvc: makeWorktreeInfo / setup
WorktreeSvc->>Git: git worktree add (.worktrees/pawwork)
WorktreeSvc->>DB: upsert registry entry (source="created")
else existing (path)
ToolSvc->>Git: gitCommonDir to verify same repo
ToolSvc->>WorktreeSvc: registerExistingByPath (source="existing")
end
ToolSvc->>SessionSvc: updateExecutionContext(activeDirectory, activeWorktree)
SessionSvc->>DB: persist execution_context with lastChangedAt
SessionSvc->>Client: respond with result + metadata
User->>Client: Invoke ExitWorktree
Client->>ToolSvc: execute ExitWorktree
ToolSvc->>ToolSvc: run guards
ToolSvc->>SessionSvc: updateExecutionContext(ownerDirectory, clear activeWorktree)
SessionSvc->>DB: persist cleared execution_context
SessionSvc->>Client: return exit result with previous metadata
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opencode/test/project/worktree.test.ts (1)
84-99:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace the fixed 1s delay with
waitReadyfor consistency.This test uses
Bun.sleep(1000)while other tests in this file use thewaitReadypattern for deterministic synchronization. Fixed sleeps can cause flaky failures on slower CI/Windows runs.Suggested fix
test("create returns worktree info and remove cleans up", async () => { await using tmp = await tmpdir({ git: true }) + const ready = waitReady(path.join(tmp.path, ".worktrees", "pawwork")) const info = await withInstance(tmp.path, () => Worktree.create()) expect(info.name).toBeDefined() expect(info.branch).toStartWith("pawwork/") expect(info.directory).toBeDefined() expect(info.source).toBe("created") - // Wait for bootstrap to complete - await Bun.sleep(1000) + await ready const ok = await withInstance(tmp.path, () => Worktree.remove({ directory: info.directory })) expect(ok).toBe(true) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/project/worktree.test.ts` around lines 84 - 99, Replace the fixed Bun.sleep(1000) delay with the deterministic waitReady pattern used elsewhere in this test file: after creating the worktree via withInstance(tmp.path, () => Worktree.create()), call await waitReady(tmp.path) (or the existing waitReady helper used by other tests) before attempting Worktree.remove; this ensures the bootstrap/completion detection uses the same waitReady helper rather than a fixed sleep and references tmp.path, Worktree.create, Worktree.remove, withInstance, and the file's waitReady helper.
♻️ Duplicate comments (1)
packages/opencode/test/session/session.test.ts (1)
62-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCanonicalize expected execution-context paths in these assertions.
Line 62/63, Line 198/199, Line 207/208, Line 214/215, and Line 424/426 still compare against raw paths. On Windows, canonicalization can change casing, causing cross-platform failures for otherwise-correct behavior.
Suggested patch pattern
+ const expectedRoot = canonicalDirectory(tmp.path) + const expectedWorktree = canonicalDirectory(worktree) - expect(info.executionContext.ownerDirectory).toBe(tmp.path) - expect(info.executionContext.activeDirectory).toBe(tmp.path) + expect(info.executionContext.ownerDirectory).toBe(expectedRoot) + expect(info.executionContext.activeDirectory).toBe(expectedRoot) - expect(entered.executionContext.activeDirectory).toBe(worktree) - expect(entered.executionContext.activeWorktree).toEqual(activeWorktree) + expect(entered.executionContext.activeDirectory).toBe(expectedWorktree) + expect(entered.executionContext.activeWorktree).toEqual({ ...activeWorktree, directory: expectedWorktree }) - expect(movedByDirectory.executionContext.activeWorktree).toEqual(activeWorktree) + expect(movedByDirectory.executionContext.activeWorktree).toEqual({ ...activeWorktree, directory: expectedWorktree }) - expect(clearedByWorktree.executionContext.activeDirectory).toBe(tmp.path) + expect(clearedByWorktree.executionContext.activeDirectory).toBe(expectedRoot) - expect(forked.executionContext.ownerDirectory).toBe(tmp.path) - expect(forked.executionContext.activeDirectory).toBe(worktree) - expect(forked.executionContext.activeWorktree).toEqual(activeWorktree) + expect(forked.executionContext.ownerDirectory).toBe(expectedRoot) + expect(forked.executionContext.activeDirectory).toBe(expectedWorktree) + expect(forked.executionContext.activeWorktree).toEqual({ ...activeWorktree, directory: expectedWorktree })Also applies to: 198-199, 207-208, 214-215, 424-426
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/session/session.test.ts` around lines 62 - 63, The assertions compare raw tmp.path strings to info.executionContext.ownerDirectory/activeDirectory which can differ in casing on Windows; canonicalize the expected path before comparing by replacing raw tmp.path with a canonical value (e.g. const expected = fs.realpathSync(tmp.path) or path.resolve/fs.realpathSync) and use expect(info.executionContext.ownerDirectory).toBe(expected) and expect(info.executionContext.activeDirectory).toBe(expected). Apply this change for the occurrences referencing tmp.path in info.executionContext at the identified spots (the assertions using info.executionContext.ownerDirectory and info.executionContext.activeDirectory).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/session/session.ts`:
- Around line 105-119: Make the execution-root fallback mandatory by changing
fromRow(row, project?: ProjectFallback) to require a ProjectFallback (e.g.,
fromRow(row, project: ProjectFallback)); update parseExecutionContext(row,
project?) to accept the required project and use legacyExecutionContext(row,
project) only when project is passed; likewise change needsProjectFallback to
accept the project and compute its result using
SessionExecutionContext.safeParse(...) plus the provided project; update all
call sites of fromRow/parseExecutionContext/needsProjectFallback to pass the
project fallback (project.worktree or equivalent) or introduce an explicit
fromRowWithoutFallback helper if you really need an optional behavior so missing
callers fail at compile time rather than silently using row.directory.
---
Outside diff comments:
In `@packages/opencode/test/project/worktree.test.ts`:
- Around line 84-99: Replace the fixed Bun.sleep(1000) delay with the
deterministic waitReady pattern used elsewhere in this test file: after creating
the worktree via withInstance(tmp.path, () => Worktree.create()), call await
waitReady(tmp.path) (or the existing waitReady helper used by other tests)
before attempting Worktree.remove; this ensures the bootstrap/completion
detection uses the same waitReady helper rather than a fixed sleep and
references tmp.path, Worktree.create, Worktree.remove, withInstance, and the
file's waitReady helper.
---
Duplicate comments:
In `@packages/opencode/test/session/session.test.ts`:
- Around line 62-63: The assertions compare raw tmp.path strings to
info.executionContext.ownerDirectory/activeDirectory which can differ in casing
on Windows; canonicalize the expected path before comparing by replacing raw
tmp.path with a canonical value (e.g. const expected = fs.realpathSync(tmp.path)
or path.resolve/fs.realpathSync) and use
expect(info.executionContext.ownerDirectory).toBe(expected) and
expect(info.executionContext.activeDirectory).toBe(expected). Apply this change
for the occurrences referencing tmp.path in info.executionContext at the
identified spots (the assertions using info.executionContext.ownerDirectory and
info.executionContext.activeDirectory).
🪄 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: ceafbe3d-bbc0-44c2-9ec7-f55eee371d11
📒 Files selected for processing (6)
packages/app/src/components/settings-worktrees.tsxpackages/opencode/src/session/session.tspackages/opencode/test/project/worktree-remove.test.tspackages/opencode/test/project/worktree.test.tspackages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/session/session.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: e2e-artifacts
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-desktop
- GitHub Check: unit-opencode
- GitHub Check: smoke-macos-arm64
- 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/test/session/prompt-effect.test.tspackages/opencode/test/project/worktree.test.tspackages/opencode/test/project/worktree-remove.test.tspackages/opencode/src/session/session.tspackages/opencode/test/session/session.test.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/session/prompt-effect.test.tspackages/opencode/test/project/worktree.test.tspackages/opencode/test/project/worktree-remove.test.tspackages/opencode/test/session/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/src/components/settings-worktrees.tsx
🧠 Learnings (9)
📚 Learning: 2026-04-23T08:51:00.819Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:00.819Z
Learning: For pawwork tests under packages/opencode/test/**, auth.json teardown may intentionally combine `Filesystem.write` (from `packages/opencode/src/util/filesystem.ts`) with `node:fs/promises` `unlink` for cleanup. Do not flag this as inconsistent style; it is the established/intentional pattern because `Filesystem` does not provide a `remove`/`unlink` helper.
Applied to files:
packages/opencode/test/session/prompt-effect.test.tspackages/opencode/test/project/worktree.test.tspackages/opencode/test/project/worktree-remove.test.tspackages/opencode/test/session/session.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.tspackages/opencode/test/session/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/session/session.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/session/session.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/session.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/session.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/session.ts
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-23T15:10:21.635Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 191
File: packages/app/src/components/session/pawwork-skill-meta.ts:38-39
Timestamp: 2026-04-23T15:10:21.635Z
Learning: This repo configures Tailwind v4 with `--color-*: initial`, which effectively breaks standard Tailwind palette utilities (e.g., `text-violet-500` can resolve to no CSS variable and render as a no-op/black). For brand/accent colors that are not backed by semantic design tokens, use inline styles with the exact hex value (e.g., `style={{ color: '#8B5FBF' }}` / `homeIconStyle: { color: '#8B5FBF' }`) and add a short comment explaining that Tailwind palette utilities won’t work due to the `--color-*: initial` setup. Do not suggest replacing these inline hex colors with Tailwind palette classes anywhere in this repo.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
🔇 Additional comments (9)
packages/opencode/test/project/worktree.test.ts (4)
19-31: LGTM!The
waitReadyfunction now correctly filters events by directory prefix. The trailing separator ensures accurate prefix matching (e.g.,pawwork/won't matchpawwork-other/), and the normalization handles cross-platform path differences.
52-54: LGTM!The updated assertions correctly verify the new worktree metadata structure:
branchunderpawwork/namespace,directoryunder.worktrees/pawwork/, andsource: "created"for newly created worktrees. This aligns with theInfoschema definition.
146-158: LGTM!Good test coverage for the
.gitignoreguard. The test correctly verifies that worktree creation is blocked when.gitignorehas uncommitted local changes, and confirms no git worktree was created despite the error.
179-219: LGTM!Comprehensive test coverage for the registry source behavior. The tests correctly verify:
- Created worktrees are slug-addressable with
source: "created"- Existing (externally created) worktrees are path-addressable only with
source: "existing"- Invalid paths (non-worktree directories) are properly rejected
The
waitReadysynchronization is correctly set up beforecreate()and awaited after.packages/opencode/test/session/prompt-effect.test.ts (1)
1316-1396: Good execution-context regression coverage.These two cases pin both
prompt.shell(...)and inlinebashtool execution toexecutionContext.activeWorktree.directory, which is the critical behavioral contract this PR is introducing.packages/opencode/test/project/worktree-remove.test.ts (1)
15-57: Nice coverage on the guarded and partial-failure remove paths.These additions lock in both sides of the new behavior: blocking removal for an actively bound session, and cleaning up the registry even when git branch deletion fails after the directory is already gone.
Also applies to: 114-168
packages/opencode/test/session/session.test.ts (1)
353-398: Great regression coverage for partialactiveWorktreerecovery.This test pins the backward-compat path (missing
source) acrossget,list, andlistGlobal, and verifies root/active directory stability.packages/app/src/components/settings-worktrees.tsx (2)
28-41: Load-path resilience looks solid.Good use of settled fan-out, explicit SDK error promotion, and all-failed escalation to request-failed UI.
90-110: Delete flow state handling is correctly scoped per row.The in-flight
Setguard +finallycleanup prevents overlapping delete-state corruption and avoids duplicate same-directory removes.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/session/session.ts`:
- Around line 76-89: The current rehydration coerces any unknown
activeWorktree.source to "created", which misclassifies external worktrees;
change the logic around activeWorktreeRaw/ActiveWorktree.safeParse so that you
only build/parse an ActiveWorktree when worktree.source is explicitly "existing"
or "created" (do not default unknown values to "created"); if source is
missing/invalid, leave activeWorktree undefined and retain the raw
directory/name/branch data (use canonicalDirectory for normalizing directory)
instead of creating an ActiveWorktree, and ensure
executionContext.activeWorktree is only set when parsed.success is true with a
valid source.
- Around line 105-117: The current safe-parse path accepts pre-fix relative
paths and skips recovery, so update parseExecutionContext and
needsProjectFallback to treat parsed-but-relative execution_contexts as invalid:
after SessionExecutionContext.safeParse(row.execution_context) succeeds, check
that the directory fields (e.g., parsed.data.currentWorkingDirectory or any path
fields used by canonicalDirectory) are absolute (use path.isAbsolute) and only
return parsed.data if they are absolute; otherwise fall through to
recoverExecutionContext() (in parseExecutionContext) or return true from
needsProjectFallback so legacyExecutionContext/recovery runs. Ensure you
reference SessionExecutionContext.safeParse, parsed.data.currentWorkingDirectory
(or the actual path field names), recoverExecutionContext, needsProjectFallback,
and legacyExecutionContext when making the change.
🪄 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: e78b4cca-655c-4d29-90fd-23c440aa4534
📒 Files selected for processing (3)
packages/opencode/src/cli/cmd/stats.tspackages/opencode/src/server/projectors.tspackages/opencode/src/session/session.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: e2e-artifacts
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-opencode
- GitHub Check: unit-desktop
- GitHub Check: analyze-js-ts
- GitHub Check: smoke-macos-arm64
🧰 Additional context used
📓 Path-based instructions (1)
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/server/projectors.tspackages/opencode/src/cli/cmd/stats.tspackages/opencode/src/session/session.ts
🧠 Learnings (5)
📚 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/server/projectors.tspackages/opencode/src/cli/cmd/stats.tspackages/opencode/src/session/session.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/server/projectors.tspackages/opencode/src/cli/cmd/stats.tspackages/opencode/src/session/session.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/session.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/session.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/session.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/session/session.ts`:
- Around line 826-839: Canonicalize execution-context paths before saving:
ensure ownerDirectory, activeDirectory and any activeWorktree.directory are run
through canonicalDirectory and normalized in the SessionExecutionContext you
persist (the next object built in the function), so you don't persist raw input
variants; update the construction of next (and the activeWorktree assignment
logic that currently uses canonicalDirectory(ownerDirectory)) to use
canonicalDirectory(ownerDirectory), canonicalDirectory(activeDirectory) and
canonicalDirectory(activeWorktree.directory) as appropriate so SQL predicate
logic (e.g. in findActiveWorktreeBinding) reliably compares normalized paths.
In `@packages/opencode/test/session/session.test.ts`:
- Around line 53-55: The test writes a file into subdir without ensuring the
directory exists, causing ENOENT; before the Bun.write(path.join(subdir,
".keep"), "") call ensure the directory is created by calling fs.mkdir(subdir, {
recursive: true }) (or alternatively call Bun.write with the createPath: true
option) so that the parent path for the variable subdir (constructed via
path.join(tmp.path, "packages", "app")) exists prior to writing.
🪄 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: 3ae7241d-37e9-406c-9d96-956cd3f5274b
📒 Files selected for processing (2)
packages/opencode/src/session/session.tspackages/opencode/test/session/session.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: e2e-artifacts
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-app
- GitHub Check: unit-opencode
- GitHub Check: unit-desktop
- GitHub Check: smoke-macos-arm64
- 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: 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/session/session.test.tspackages/opencode/src/session/session.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/session/session.test.ts
🧠 Learnings (7)
📚 Learning: 2026-04-23T08:51:00.819Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:00.819Z
Learning: For pawwork tests under packages/opencode/test/**, auth.json teardown may intentionally combine `Filesystem.write` (from `packages/opencode/src/util/filesystem.ts`) with `node:fs/promises` `unlink` for cleanup. Do not flag this as inconsistent style; it is the established/intentional pattern because `Filesystem` does not provide a `remove`/`unlink` helper.
Applied to files:
packages/opencode/test/session/session.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/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/session/session.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/session/session.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/session.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/session.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/session.ts
🔇 Additional comments (2)
packages/opencode/src/session/session.ts (1)
126-134: Recovery cascade is solid (parse → recover → legacy).The read-path fallback order is clean and resilient for mixed legacy/malformed rows.
packages/opencode/test/session/session.test.ts (1)
285-452: Great coverage on legacy hydration paths.Validating
get,list, andlistGlobalagainst null/invalid/relative execution contexts closes an important regression surface.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/session/session.ts`:
- Around line 112-123: normalizeExecutionContext currently canonicalizes
ownerDirectory, activeDirectory, and activeWorktree.directory but leaves
activeWorktree intact even when activeDirectory normalizes back to the owner
root, which can revive stale worktree metadata; update normalizeExecutionContext
so after computing canonical ownerDirectory and activeDirectory it discards
activeWorktree (set to undefined) whenever the normalized activeDirectory equals
the normalized ownerDirectory, otherwise preserve the normalized activeWorktree
as now; reference normalizeExecutionContext and activeWorktree to locate the
change.
- Around line 560-563: The initial executionContext seeded in createNext
(executionContext: input.executionContext ?? rootContext(...)) must be
canonicalized before storing so Session.create and Session.get return identical
values; update createNext to run the same normalization used by fromRow (the
canonicalization routine used when reading rows) on the chosen value
(input.executionContext or rootContext(...)) so that executionContext is stored
in canonical form; reference: executionContext, createNext, fromRow,
Session.create, Session.get.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 15aafe38-5a26-4cc0-9660-4adae05e581b
📒 Files selected for processing (2)
packages/opencode/src/session/session.tspackages/opencode/test/session/session.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: unit-windows-opencode-server-tools
🧰 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/src/session/session.tspackages/opencode/test/session/session.test.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/session/session.test.ts
🧠 Learnings (7)
📚 Learning: 2026-04-27T08:58:00.665Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:531-538
Timestamp: 2026-04-27T08:58:00.665Z
Learning: When using Effect (e.g., `yield*` with `Effect`-style generator yielding), only use `yield* new SomeErrorClass(...)` if `SomeErrorClass` extends `Schema.TaggedErrorClass` (i.e., it implements Effect’s Yieldable interface). For plain `Error` subclasses (like `BlockedLoopError` / `LoopStopError`) or inline `new Error(...)` values, they are not yieldable and must be wrapped as `yield* Effect.fail(new PlainError(...))`. Do not recommend changing `yield* Effect.fail(new SomePlainError(...))` to `yield* new SomePlainError(...)` unless the error class extends `Schema.TaggedErrorClass`.
Applied to files:
packages/opencode/src/session/session.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/session/session.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/session.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/session.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/session.ts
📚 Learning: 2026-04-23T08:51:00.819Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:00.819Z
Learning: For pawwork tests under packages/opencode/test/**, auth.json teardown may intentionally combine `Filesystem.write` (from `packages/opencode/src/util/filesystem.ts`) with `node:fs/promises` `unlink` for cleanup. Do not flag this as inconsistent style; it is the established/intentional pattern because `Filesystem` does not provide a `remove`/`unlink` helper.
Applied to files:
packages/opencode/test/session/session.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/session.test.ts
Astro-Han
left a comment
There was a problem hiding this comment.
Review: Worktree Execution Context — Thorough pass
Thorough review across all 75 files. Findings grouped by severity.
P0 — Must fix (data integrity / security / core feature broken)
1. Migration SQL DEFAULT 'null' kills the entire backfill mechanism
migration.sql:
ALTER TABLE `session` ADD `execution_context` text DEFAULT 'null';The default is the string literal 'null', not SQL NULL. All existing session rows get execution_context = 'null' (4-byte JSON string). Consequences:
backfillExecutionContextRowsusesisNull(SessionTable.execution_context)— never matches these rows. Backfill is dead on arrival.parseExecutionContext→SessionExecutionContext.safeParse('null')fails →recoverExecutionContextcheckstypeof raw !== "object"→ fails → falls tolegacyExecutionContext. Legacy path works as a safety net, so no crash, but the column permanently carries the string'null', forming a split-brain with real JSON objects written later.
Fix: DEFAULT 'null' → DEFAULT NULL (one character).
2. Subagents can call enter-worktree / exit-worktree — session integrity violation
agent.ts L426-432: the tools exclusion map denies agent and optionally todowrite, but never denies enter-worktree or exit-worktree. A subagent that inherits the parent's worktree context can then call ExitWorktree, reverting its own session to the project root while the parent assumes the subagent is still in the worktree. Files edited, shells run, and paths read would silently resolve from the wrong tree.
Fix: Add "enter-worktree": false, "exit-worktree": false to the exclusion map.
3. v1 SDK SubtaskPart completely diverged from v2
packages/sdk/js/src/gen/types.gen.ts L386-394: the v1 SubtaskPart in the Part union is an inline type with only {id, sessionID, messageID, type, prompt, description, agent}. The v2 SubtaskPart (v2/gen/types.gen.ts L608+) has 20+ fields including status, started_at, ended_at, error, recent_events, tool_call_id, parent_session_id, etc. Any external v1 SDK consumer silently loses all subtask lifecycle data. This is a semantic breaking change.
P1 — Should fix before merge
4. EnterWorktree vs ExitWorktree path comparison inconsistency — deadlock states
enter-worktree.ts L113 uses raw string comparison (exec.activeDirectory !== exec.ownerDirectory). exit-worktree.ts L24 uses canonicalDirectory(). If activeDirectory and ownerDirectory resolve to the same physical path but differ in representation (symlink, trailing slash, macOS case), EnterWorktree rejects ("already in another worktree") AND ExitWorktree considers the session "already at root". The user can neither enter nor exit.
5. setup() doesn't roll back .gitignore modification if git worktree add fails
worktree/index.ts ~L383: ensureWorktreesIgnored modifies .gitignore before git worktree add. If the git command fails, the user's working tree is left with uncommitted .gitignore changes they never requested.
6. createFromInfo doesn't clean up on boot failure
worktree/index.ts: setup() succeeds (worktree created, registry written), then boot() fails. The broken worktree persists. Next EnterWorktree(name=...) finds the existing directory, reuses it, and operates in an unbooted worktree.
7. Storage location changed from global to project-local — no migration
worktree/index.ts ~L374: old path was Global.Path.data/worktree/<project-id>/<slug>, new path is <project-root>/.worktrees/pawwork/<slug>. Existing worktrees become invisible. Users upgrading lose management access to all existing worktrees.
8. Instance.provide cache replacement leaks old instance resources
instance.ts ~L79: when matchesOverride fails, a new instance is booted and tracked, but the old one is never disposed. Scoped resources (file watchers, IPC, Effect fibers) are leaked.
9. backfillExecutionContext memoized at module scope — never re-runs
session.ts L182-186: backfillExecutionContextEffect() is called at module load. Sessions imported or synced after startup never get backfilled.
10. recoverExecutionContext sets lastChangedAt: Date.now() for corrupted rows
session.ts ~L99: a recovered row gets a fresh timestamp instead of preserving the original.
11. v1 SDK activeWorktree.branch optional vs v2 required
packages/sdk/js/src/gen/types.gen.ts L11: branch?: string (v1) vs branch: string (v2). The server schema requires branch: z.string().
P2 — Should fix soon
12. rootContext doesn't canonicalize paths — execution-context.ts L19-20 stores paths as-is, but updateExecutionContext and parseExecutionContext use canonicalDirectory. On macOS, /tmp/foo vs /private/tmp/foo breaks equality checks.
13. hasInFlightToolCallsExcept loads entire message history — state-machine-guard.ts L15-27: every enter-worktree/exit-worktree call loads all messages. Only the latest assistant message needs checking.
14. activeForSession reads activeCounts outside the semaphore — subagent-run.ts L147-148: a stale read of 0 allows worktree transition during active subagent execution.
15. updateExecutionContext has no concurrency guard — two parallel enter-worktree calls can both pass the check and write conflicting contexts.
16. canonicalDirectory uses blocking fs.realpathSync.native — execution-context-store.ts L8-13: called per-row in list/listGlobal.
17. v1 SubtaskPart missing model? and command? fields — existed in v2 before this PR, never back-ported.
P3 — Nice to have
canonicalDirectoryis a general utility but lives inexecution-context-store.ts.ProjectFallbacktype is hand-written instead of derived from schema.gitignore-guard.tsusesBun.spawndirectly — untestable with mocks.- No test for concurrent
updateExecutionContext. session.test.tsusessetTimeout(100)for Bus events — flaky.- Error message
"name max 40 chars"is terse.
Summary
| Level | Count | Key themes |
|---|---|---|
| P0 | 3 | Migration kills backfill; subagent escapes worktree; v1/v2 SDK type break |
| P1 | 8 | Path comparison deadlock; no rollback; no migration; cache leak; wrong timestamp |
| P2 | 6 | Uncanonicalized paths; full-table scan; semaphore bypass; concurrency; blocking syscall |
| P3 | 6 | Naming/placement; test quality; error messages |
Astro-Han
left a comment
There was a problem hiding this comment.
Inline comments for key findings. See the earlier review comment for the full P0-P3 breakdown.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/opencode/test/project/worktree.test.ts (1)
89-99:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReuse
waitReady()here instead of sleeping.
Worktree.create()still returns before bootstrap completes, so this fixed 1s delay can race the followingremove()on slow CI or Windows runs. You already added a deterministic readiness helper in this file; use it here too.Suggested fix
- const info = await withInstance(tmp.path, () => Worktree.create()) + const ready = waitReady(path.join(tmp.path, ".worktrees", "pawwork")) + const info = await withInstance(tmp.path, () => Worktree.create()) @@ - // Wait for bootstrap to complete - await Bun.sleep(1000) + await ready🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/project/worktree.test.ts` around lines 89 - 99, Replace the brittle fixed delay with the deterministic readiness helper: after calling Worktree.create() (the call wrapped by withInstance that returns info), call the existing waitReady() helper instead of await Bun.sleep(1000) to wait for bootstrap to finish before invoking Worktree.remove({ directory: info.directory }); ensure you pass the same tmp.path context to waitReady() if it requires it (same way withInstance is used) so the subsequent withInstance(..., () => Worktree.remove(...)) runs only after readiness is confirmed.packages/opencode/src/session/session.ts (1)
56-59:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid treating legacy contexts as freshly changed.
Line 58 rebuilds the fallback through
rootContext(...), solastChangedAtbecomesDate.now()every time a legacy row is synthesized. That makes unchanged sessions look newly modified until something explicitly rewrites the execution context. Userow.time_updated(or another stable persisted value) for this fallback, and keep the backfill store path aligned with the same rule.🩹 Suggested fix
function legacyExecutionContext(row: SessionRow, project: ProjectFallback | undefined) { const ownerDirectoryRaw = project?.vcs === "git" ? (project.worktree ?? row.directory) : row.directory - return rootContext(canonicalDirectory(ownerDirectoryRaw)) + const directory = canonicalDirectory(ownerDirectoryRaw) + return { + ownerDirectory: directory, + activeDirectory: directory, + activeWorktree: undefined, + lastChangedAt: row.time_updated, + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/session/session.ts` around lines 56 - 59, The legacyExecutionContext function currently calls rootContext(canonicalDirectory(...)) which causes lastChangedAt to be set to Date.now() for synthesized legacy rows; instead, build the fallback execution context using the persisted timestamp (row.time_updated or another stable persisted field) so the execution context's lastChangedAt is stable; update legacyExecutionContext (and any code that computes the backfill store path for synthesized sessions) to pass that stable timestamp into the context construction (e.g., when creating the rootContext or equivalent metadata) so the backfill store path follows the same stable-timestamp rule rather than being regenerated with the current time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/session/execution-context.ts`:
- Around line 21-31: Replace the Windows-specific toLowerCase mutation with true
case-insensitive comparisons: remove the trailing .toLowerCase() from
canonicalDirectory (so canonicalDirectory returns the normalized real path
unchanged) and make the analogous change in the canonical() effect in
packages/opencode/src/worktree/index.ts; then update any equality checks that
compared ownerDirectory/activeDirectory (enter-worktree, exit-worktree, session
state) to use a Windows-aware case-insensitive comparison helper (e.g.,
pathEqualsCaseInsensitive(a,b) that on process.platform === "win32" does
a.localeCompare(b, undefined, { sensitivity: "base" }) === 0, otherwise does
strict ===), keeping all original path strings intact to avoid Unicode
case-mapping expansions like U+0130 → U+0069 U+0307.
In `@packages/opencode/src/session/session.ts`:
- Around line 92-101: The parse branch returns recovered.data directly which can
leave stale activeWorktree/activeDirectory invariants intact; after successful
safeParse of SessionExecutionContext, pass recovered.data through
normalizeExecutionContext (using canonicalDirectory as already done) and return
that normalized object instead of recovered.data so the same
normalization/invariant closure occurs as in the main parse path.
In `@packages/opencode/src/worktree/index.ts`:
- Around line 306-309: lookupBySlug is comparing the raw incoming slug to stored
entries where makeWorktreeInfo saved names in slugified form; normalize the
lookup input by slugifying the slug parameter before comparing so "My Feature
Branch!" matches "my-feature-branch". Update Effect.fn("Worktree.lookupBySlug")
to call the same slug utility used by makeWorktreeInfo (or the central slugify
function) and then return entries.find(e => e.name === normalizedSlug &&
e.source === "created"), using readRegistry() as before.
---
Duplicate comments:
In `@packages/opencode/src/session/session.ts`:
- Around line 56-59: The legacyExecutionContext function currently calls
rootContext(canonicalDirectory(...)) which causes lastChangedAt to be set to
Date.now() for synthesized legacy rows; instead, build the fallback execution
context using the persisted timestamp (row.time_updated or another stable
persisted field) so the execution context's lastChangedAt is stable; update
legacyExecutionContext (and any code that computes the backfill store path for
synthesized sessions) to pass that stable timestamp into the context
construction (e.g., when creating the rootContext or equivalent metadata) so the
backfill store path follows the same stable-timestamp rule rather than being
regenerated with the current time.
In `@packages/opencode/test/project/worktree.test.ts`:
- Around line 89-99: Replace the brittle fixed delay with the deterministic
readiness helper: after calling Worktree.create() (the call wrapped by
withInstance that returns info), call the existing waitReady() helper instead of
await Bun.sleep(1000) to wait for bootstrap to finish before invoking
Worktree.remove({ directory: info.directory }); ensure you pass the same
tmp.path context to waitReady() if it requires it (same way withInstance is
used) so the subsequent withInstance(..., () => Worktree.remove(...)) runs only
after readiness is confirmed.
🪄 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: 848a2524-b73f-435f-8428-751571956839
📒 Files selected for processing (14)
packages/opencode/migration/20260501081615_session_execution_context/migration.sqlpackages/opencode/migration/20260501081615_session_execution_context/snapshot.jsonpackages/opencode/src/session/execution-context-store.tspackages/opencode/src/session/execution-context.tspackages/opencode/src/session/session.tspackages/opencode/src/session/subagent-run.tspackages/opencode/src/tool/agent.tspackages/opencode/src/tool/enter-worktree.tspackages/opencode/src/tool/exit-worktree.tspackages/opencode/src/worktree/gitignore-guard.tspackages/opencode/src/worktree/index.tspackages/opencode/test/project/worktree.test.tspackages/opencode/test/session/session.test.tspackages/opencode/test/tool/agent.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). (9)
- GitHub Check: e2e-artifacts
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-opencode
- GitHub Check: analyze-js-ts
- GitHub Check: smoke-macos-arm64
🧰 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/test/tool/agent.test.tspackages/opencode/src/session/execution-context-store.tspackages/opencode/src/session/subagent-run.tspackages/opencode/src/tool/agent.tspackages/opencode/src/session/execution-context.tspackages/opencode/src/tool/enter-worktree.tspackages/opencode/src/tool/exit-worktree.tspackages/opencode/src/worktree/index.tspackages/opencode/src/worktree/gitignore-guard.tspackages/opencode/test/project/worktree.test.tspackages/opencode/src/session/session.tspackages/opencode/test/session/session.test.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/agent.test.tspackages/opencode/test/project/worktree.test.tspackages/opencode/test/session/session.test.ts
packages/opencode/migration/**/*.sql
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
Migration tests should read the per-folder layout without
_journal.jsonfiles
Files:
packages/opencode/migration/20260501081615_session_execution_context/migration.sql
🧠 Learnings (7)
📚 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/agent.test.tspackages/opencode/test/project/worktree.test.tspackages/opencode/test/session/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/session/execution-context-store.tspackages/opencode/src/session/subagent-run.tspackages/opencode/src/tool/agent.tspackages/opencode/src/session/execution-context.tspackages/opencode/src/tool/enter-worktree.tspackages/opencode/src/tool/exit-worktree.tspackages/opencode/src/worktree/index.tspackages/opencode/src/worktree/gitignore-guard.tspackages/opencode/src/session/session.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/session/execution-context-store.tspackages/opencode/src/session/subagent-run.tspackages/opencode/src/tool/agent.tspackages/opencode/src/session/execution-context.tspackages/opencode/src/tool/enter-worktree.tspackages/opencode/src/tool/exit-worktree.tspackages/opencode/src/worktree/index.tspackages/opencode/src/worktree/gitignore-guard.tspackages/opencode/src/session/session.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/execution-context-store.tspackages/opencode/src/session/subagent-run.tspackages/opencode/src/session/execution-context.tspackages/opencode/src/session/session.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/execution-context-store.tspackages/opencode/src/session/subagent-run.tspackages/opencode/src/session/execution-context.tspackages/opencode/src/session/session.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/execution-context-store.tspackages/opencode/src/session/subagent-run.tspackages/opencode/src/session/execution-context.tspackages/opencode/src/session/session.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/session.test.ts
🔇 Additional comments (9)
packages/opencode/src/tool/enter-worktree.ts (6)
1-17: LGTM!Imports are well-organized. The
canonicalDirectoryimport fromexecution-context.tsaddresses the prior P1 concern about inconsistent path comparison withExitWorktree.
19-27: LGTM!Parameter schema is well-defined with clear descriptions documenting the mutual exclusivity constraint.
36-50: LGTM!The guard function correctly prevents worktree transitions while other tool calls or subagents are active in the session.
52-62: LGTM!The
applyEnterhelper properly updates the session's execution context with the worktree info and source.
106-145: LGTM!The
pathmode branch now:
- Validates absolute paths up front (line 107-108)
- Uses
canonicalDirectory()for activeDirectory comparisons (lines 113, 123)- Calls
applyEnter()before returning success (line 137)All prior P0/P1 concerns for this branch have been addressed.
147-188: LGTM!The
namemode branch usescanonicalDirectory()consistently for directory comparisons (lines 150, 159), addressing the prior P1 concerns about inconsistent path handling withExitWorktree.packages/opencode/src/tool/agent.ts (2)
291-344: LGTM!The worktree inheritance logic correctly:
- Snapshots the parent's
executionContextimmediately aftersubagentRun.start- Compares all four
ActiveWorktreefields structurally (lines 331-334), addressing the prior P3 concern about reference equality- Inherits the parent's
activeDirectoryandactiveWorktreewhen they differ (lines 338-343)- Clears the child's worktree with
nullwhen the parent has no active worktree (line 342)
426-434: LGTM!Adding
"enter-worktree": falseand"exit-worktree": falseto the tools exclusion map addresses the prior P0 concern: subagents can no longer change the worktree context, preventing state divergence between parent and child sessions.packages/opencode/test/tool/agent.test.ts (1)
551-558: LGTM!The test assertion correctly expects
"enter-worktree": falseand"exit-worktree": falsein the tools object passed to subagent prompts, validating the implementation change inagent.ts.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/tool/enter-worktree.ts`:
- Around line 128-130: Replace the raw string equality checks on git common
directories with a canonicalized directory comparison: after obtaining
ownerCommon = yield* gitCommonDir(spawner, exec.ownerDirectory) and targetCommon
= yield* gitCommonDir(spawner, canonical), use sameDirectory(ownerCommon,
targetCommon) (and handle null/undefined results appropriately) instead of
ownerCommon !== targetCommon; apply the same change to the later occurrence
around the symbols used at lines 173-175 so path-mode and managed-mode use the
sameDirectory helper for equality checks.
- Around line 136-137: The code is overwriting the registry-provided worktree
provenance by hardcoding "existing" when calling applyEnter; update the call
that currently does yield* applyEnter(ctx.sessionID, { ...info, branch:
info.branch || branch }, "existing") so it preserves the registry's source (from
Worktree.registerExistingByPath) — e.g. pass info.source (or fallback to
"existing") as the third argument so applyEnter receives info.source ||
"existing" and the executionContext.activeWorktree keeps the original
provenance.
🪄 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: d2f6ffe4-d554-491c-9136-55d3c63ea2db
📒 Files selected for processing (7)
packages/opencode/src/session/execution-context.tspackages/opencode/src/session/session.tspackages/opencode/src/tool/enter-worktree.tspackages/opencode/src/tool/exit-worktree.tspackages/opencode/src/worktree/index.tspackages/opencode/test/project/worktree.test.tspackages/opencode/test/session/session.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). (8)
- GitHub Check: e2e-artifacts
- GitHub Check: smoke-macos-arm64
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-app
- GitHub Check: unit-opencode
🧰 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/src/session/execution-context.tspackages/opencode/src/tool/exit-worktree.tspackages/opencode/src/tool/enter-worktree.tspackages/opencode/test/project/worktree.test.tspackages/opencode/src/worktree/index.tspackages/opencode/test/session/session.test.tspackages/opencode/src/session/session.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/project/worktree.test.tspackages/opencode/test/session/session.test.ts
🧠 Learnings (7)
📚 Learning: 2026-04-27T08:58:00.665Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:531-538
Timestamp: 2026-04-27T08:58:00.665Z
Learning: When using Effect (e.g., `yield*` with `Effect`-style generator yielding), only use `yield* new SomeErrorClass(...)` if `SomeErrorClass` extends `Schema.TaggedErrorClass` (i.e., it implements Effect’s Yieldable interface). For plain `Error` subclasses (like `BlockedLoopError` / `LoopStopError`) or inline `new Error(...)` values, they are not yieldable and must be wrapped as `yield* Effect.fail(new PlainError(...))`. Do not recommend changing `yield* Effect.fail(new SomePlainError(...))` to `yield* new SomePlainError(...)` unless the error class extends `Schema.TaggedErrorClass`.
Applied to files:
packages/opencode/src/session/execution-context.tspackages/opencode/src/tool/exit-worktree.tspackages/opencode/src/tool/enter-worktree.tspackages/opencode/src/worktree/index.tspackages/opencode/src/session/session.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/session/execution-context.tspackages/opencode/src/tool/exit-worktree.tspackages/opencode/src/tool/enter-worktree.tspackages/opencode/src/worktree/index.tspackages/opencode/src/session/session.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/execution-context.tspackages/opencode/src/session/session.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/execution-context.tspackages/opencode/src/session/session.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/execution-context.tspackages/opencode/src/session/session.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/project/worktree.test.tspackages/opencode/test/session/session.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/session.test.ts
🔇 Additional comments (11)
packages/opencode/src/worktree/index.ts (1)
416-421: Verify bootstrap preserves the explicit worktree override.This path depends on
Instance.provide({ directory: info.directory, init: InstanceBootstrap, ... })honoringinfo.directoryall the way through bootstrap. Ifprovide()/boot re-derives the instance from on-disk project state, a newly created worktree can be initialized against the owner workspace instead of the target worktree.packages/opencode/src/session/session.ts (10)
54-59: LGTM!The
ProjectFallbacktype andlegacyExecutionContexthelper are well-designed. The fallback correctly prioritizesproject.worktreefor git projects and canonicalizes the result.
61-142: Well-structured execution context recovery and validation.The three-tier parsing strategy (schema validation → field-level recovery → legacy fallback) is robust. Key correctness properties:
isPersistedExecutionContextUsablerejects relative paths from pre-fix rowsnormalizeExecutionContextdrops staleactiveWorktreewhenactiveDirectoryequalsownerDirectory- Recovery uses
row.time_updatedinstead ofDate.now()forlastChangedAt
312-324: Input validation correctly rejects relative paths at the API boundary.The
AbsoluteDirectoryschema withmin(1)andpath.isAbsoluterefinement prevents empty strings and relative paths from flowing intocanonicalDirectory(), which would otherwise resolve them againstprocess.cwd().
524-535: LGTM!The backfill is correctly wired:
Effect.fnprovides tracing per coding guidelines, and the layer invokes the effect factory directly at setup time rather than yielding a module-level memoized value.
563-567: LGTM!The execution context seeding correctly normalizes provided contexts and derives canonical root contexts for new sessions. The git vs non-git distinction for
ownerDirectoryaligns with the PR objectives.
589-625: Efficient project fallback handling.The conditional fetch pattern (
needsProjectFallback→ batch project lookup → pass tofromRow) avoids unnecessary database queries for sessions with valid persisted execution contexts while ensuring legacy rows are correctly hydrated.
746-752: LGTM!Fork correctly preserves the source session's
executionContext, ensuring forked sessions maintain their worktree binding rather than falling back to the ambient instance root.
822-855: LGTM!The
updateExecutionContextimplementation correctly:
- Detects explicit worktree input by value rather than key presence
- Keeps
activeDirectoryandactiveWorktreesynchronized- Canonicalizes all paths before persisting
- Returns
Infowith consistenttime.updated
857-897: LGTM!The
findActiveWorktreeBindingimplementation efficiently filters at the SQL level using JSON predicates before applying canonical path matching in TypeScript. Project fallback hydration is correctly applied to rows that need it.
1072-1086: LGTM!The
listandlistGlobalfunctions follow the same efficient pattern: collect project IDs from rows needing fallback, batch fetch, and pass tofromRow. ThelistGlobalvariant maintains separate maps forProjectInfo(returned in response) andProjectFallback(used for hydration).
The session timeline used --composer-dock-height to keep the latest turn above the composer, but the dock height was only reliably measured during initial ref attachment. The ResizeObserver setup depended on plain let refs, so post-mount composer growth from image attachments, todo docks, or follow-up docks could leave the CSS variable stale. Bind native ResizeObserver instances directly when the prompt/content refs attach, disconnect them on ref replacement and cleanup, and keep the existing sticky-bottom behavior when the measured dock height changes. This makes dynamic composer height changes update --composer-dock-height and push the conversation flow upward. Also increase the extra timeline clearance above the composer from 16px to 32px so the final response has more visual breathing room after #360, and add regression coverage for post-mount dock resizing plus the session composer clearance contract. Verification: - bun test src/pages/session/use-session-scroll-dock.test.ts src/shell-frame-contract.test.ts - bun run typecheck - PR CI passed, including app/unit/typecheck/desktop smoke/windows jobs
Summary
Adds session execution context for PawWork-managed worktrees, including persistence, agent tools, subagent inheritance, registry guards, Settings -> Worktrees, and the titlebar worktree badge.
Also includes follow-up fixes found during testing and review: the Settings context crash, titlebar typography cleanup, composer scroll-bottom regression, worktree deletion guards, legacy message path hydration, and execution-context consistency when entering an existing worktree by path.
Why
Issue #278 defines worktree as a session execution context, not a project switch. The agent should be able to enter and leave a managed worktree while keeping the original project owner context stable.
The optional user-configurable branch prefix setting is intentionally not included because it was outside the accepted #278 design scope.
Related Issue
Closes #278
Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
Reviewers should focus on whether execution_context stays stable across new sessions, resumed sessions, subagents, and enter/exit worktree transitions.
Please also check the Settings -> Worktrees panel and titlebar badge behavior because this PR includes visible UI wiring.
Risk Notes
Medium risk. This touches session persistence, message routing, agent tool behavior, SDK schema, and desktop UI. Legacy sessions and legacy assistant message path rows are handled through compatibility paths, but reviewers should still check that old sessions open normally.
No dependencies, credentials, or local-only docs are included. OpenAPI and generated SDK types were regenerated after schema changes.
How To Verify
Commands run:
The three
bun typecheckcommands were run inpackages/app,packages/opencode, andpackages/sdk/js.Manual UI verification is still pending before merge:
Screenshots or Recordings
Not attached yet. This PR has visible UI changes, so screenshots or a short recording should be added after the manual UI pass.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
New Features
UI Changes
Bug Fixes
Tests