Open
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.
Astro-Han
added
enhancement
Contributor
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughPersist session executionContext and add worktree management: DB migration and backfill, canonicalization, worktree registry with source, Enter/Exit tools and git guards, session/tool execution tied to executionContext, UI for worktrees (settings, badge), workspace-chip normalization, schema/OpenAPI updates, and many tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant SessionSvc as Session Service
participant WorktreeSvc as Worktree Registry
participant Git as Git Repo
participant DB as Database
User->>Client: trigger EnterWorktree (name or path)
Client->>SessionSvc: invoke EnterWorktree tool
SessionSvc->>SessionSvc: guard: no in-flight tool / no running subagents
alt name (managed)
SessionSvc->>WorktreeSvc: request make/lookup slug
WorktreeSvc->>Git: create worktree under .worktrees/pawwork (git worktree)
WorktreeSvc->>DB: upsert registry entry (source="created")
else path (existing)
SessionSvc->>Git: canonicalize path & verify same repo (gitCommonDir)
SessionSvc->>WorktreeSvc: register existing by path (source="existing")
end
SessionSvc->>DB: update session.execution_context (activeDirectory, activeWorktree, lastChangedAt)
SessionSvc->>Client: return result (state, slug, branch, metadata)
User->>Client: trigger ExitWorktree
Client->>SessionSvc: invoke ExitWorktree tool
SessionSvc->>SessionSvc: guard: no in-flight tool / no running subagents
SessionSvc->>DB: set execution_context.activeDirectory = ownerDirectory, clear activeWorktree
SessionSvc->>Client: return exit result with previous worktree metadata
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Astro-Han
added
P1
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/pages/layout/pawwork-worktree-badge.tsx`:
- Line 27: The badge currently renders only props.name in the span (see
pawwork-worktree-badge component) so the branch is hidden in the title/tooltip;
update the visible text to include both the slug and branch (e.g., render
`${props.name} · ${props.branch}` or similar) inside the <span
className="min-w-0 truncate"> while keeping the existing title/tooltip behavior,
and ensure any prop used (props.branch) exists on the component's props type or
is safely defaulted.
In `@packages/opencode/src/session/session.ts`:
- Around line 723-740: The updateExecutionContext function can leave
activeDirectory and activeWorktree out of sync; change its merge logic so they
are kept in sync: when the caller explicitly provides activeWorktree (the
"activeWorktree" key exists) derive activeDirectory from that value (if
activeWorktree is null -> set activeDirectory = ownerDirectory; if non-null ->
set activeDirectory to the worktree's directory field), and when the caller
explicitly provides activeDirectory and it equals the ownerDirectory (i.e., the
session was moved back to ownerDirectory) clear activeWorktree (set it to
undefined) to avoid stale badges; update the computation of next in
updateExecutionContext to implement these rules (refer to the
updateExecutionContext function, current.executionContext, and the
activeWorktree/activeDirectory fields).
🪄 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: f630010c-9e2c-47e4-9dc3-ad02223084ce
📒 Files selected for processing (5)
packages/app/src/context/prompt.test.tspackages/app/src/pages/layout/pawwork-worktree-badge.tsxpackages/opencode/src/session/execution-context-store.tspackages/opencode/src/session/session.tspackages/opencode/test/session/session.test.ts
11 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/components/settings-worktrees-helpers.ts`:
- Around line 25-30: The errorText function can throw when calling
JSON.stringify on circular or non-serializable values; update errorText to guard
the stringify call by wrapping JSON.stringify(error) in a try/catch and
returning a safe fallback (e.g., String(error) or a constant like
"<unserializable error>") when stringify throws or returns undefined; ensure you
modify the errorText function to keep the existing branches for string and
object-with-message and only use the guarded stringify fallback at the end.
In `@packages/app/src/components/settings-worktrees.tsx`:
- Around line 50-77: The boundSessions helper currently recomputes and re-enters
sync.child(...) on every row render; wrap its logic in a Solid createMemo (or
equivalent memoization) so the Map<string, BoundSession> is computed once and
reused across renders. Specifically, convert the boundSessions function into a
memoized value (using createMemo) that captures sync, data, and any dependencies
(directories derived from sync.data.project and data()), and return the memoized
Map; keep the same internal logic and preserve symbols like boundSessions,
sync.child, entryDirectory, and BoundSession so callers continue to use the same
identifier without re-scanning directories.
In `@packages/app/src/context/prompt.test.ts`:
- Around line 22-52: The promptSession() test helper hardcodes dirty() to true;
change it to a local boolean (e.g., let isDirty = false) and have dirty() return
that state, set isDirty = true inside set(next, ...) and inside context mutation
helpers (context.add, context.remove, context.removeComment,
context.updateComment, context.replaceComments) so any change marks dirty, and
clear isDirty = false in reset() so reset() clears the dirty flag; keep function
names promptSession, set, reset, and the context methods to locate and update
the implementation.
In `@packages/app/src/pages/layout/pawwork-worktree-badge.test.tsx`:
- Around line 57-70: The test currently only asserts visible text and title but
doesn't verify that the PawworkWorktreeBadge forwards interaction props to the
underlying Button; update the test for PawworkWorktreeBadge to assert that the
rendered root Button receives the provided onClick handler, the aria-label, and
the disabled state: capture the onClick function passed into
PawworkWorktreeBadge and assert tree.props.onClick === that function, assert
tree.props['aria-label'] === "Open worktrees", and assert tree.props.disabled
=== false (or the expected default) so the component contract for
PawworkWorktreeBadge → Button forwarding is covered.
In `@packages/opencode/src/session/session.ts`:
- Around line 730-739: The code currently uses "activeWorktree" in input to
detect intent, but callers may spread an optional activeWorktree: undefined
which should be treated as omission; change the detection to test the actual
value (e.g., const hasActiveWorktree = input.activeWorktree !== undefined) and
then compute activeDirectory and activeWorktree based on that flag and
canonicalDirectory(ownerDirectory) as before so that an explicit undefined value
does not override a provided activeDirectory in the same update; update
references to hasActiveWorktree, activeDirectory, and activeWorktree accordingly
(functions/values: activeWorktree, activeDirectory, hasActiveWorktree, input,
current.executionContext.ownerDirectory, canonicalDirectory).
In `@packages/opencode/src/tool/exit-worktree.ts`:
- Around line 1-6: The fast-path that prints "Exited worktree" currently
compares activeDirectory to projectRoot using raw string equality; canonicalize
both paths before comparing so trailing-separator variants match the root case
(e.g., normalize or resolve activeDirectory and projectRoot, strip trailing
separators or use path.resolve/path.normalize) and only write the no-op update
when the canonicalized paths differ; update the equality check in the
exit-worktree flow (the block that checks activeDirectory/projectRoot and emits
"Exited worktree") to use the normalized values.
In `@packages/opencode/test/tool/enter-worktree.test.ts`:
- Around line 16-40: The file reimplements a custom Effect test harness
(Layer.mergeAll, toolContext, run) instead of using the shared helpers; replace
the custom setup by converting tests to use testEffect(...) (and it.live(...)
where tests touch real OS/time/child processes) and the fixture helpers from
fixture/fixture.ts; remove Layer.mergeAll(...) and the run(...) wrapper and wire
tests to the standard test/lib/effect.ts harness so the Effect environment is
provided by testEffect and any live dependencies use it.live and the provided
fixture helpers for session/agent setup instead of toolContext.
🪄 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: 78c77414-214f-4a31-b81c-ad4a6603080a
📒 Files selected for processing (14)
packages/app/e2e/app/shell-frame.spec.tspackages/app/src/components/settings-worktree-row.tsxpackages/app/src/components/settings-worktrees-helpers.tspackages/app/src/components/settings-worktrees.tsxpackages/app/src/components/titlebar.tsxpackages/app/src/context/prompt.test.tspackages/app/src/index.csspackages/app/src/pages/layout/pawwork-worktree-badge.test.tsxpackages/app/src/pages/layout/pawwork-worktree-badge.tsxpackages/opencode/src/session/execution-context-store.tspackages/opencode/src/session/session.tspackages/opencode/src/tool/exit-worktree.tspackages/opencode/test/session/session.test.tspackages/opencode/test/tool/enter-worktree.test.ts
Owner
Author
Code Review: Kimi K2.6This is a large, well-structured PR that introduces session execution context for worktrees. The core design is sound: Status: This review was written against commit P0 — Blocking (must fix before merge)
P0-1 detail: P0-2 detail: The P1 — Should fix (address or explicitly acknowledge before merge)
P2 — Nice to have (follow-up OK)
P3 — Nitpick
SummaryFixed since review (✅): 7 items — SQL pushdown, registry cleanup, subagent inheritance, state-machine tests, worktree-remove tests, canonical comparison, dead code removal. Still open: 10 items — 2 P0 (Windows path, Instance cache), 2 P1 (boundSessions perf, Badge reactivity), 4 P2 (errorText, gitignore, source derivation, description format), 3 P3 (null/undefined, magic number, padding). Overall: Approve with comments. The P0 and remaining P1 items should be addressed or explicitly acknowledged before merge. The design is solid and the follow-up velocity is good. |
Astro-Han
commented
May 1, 2026
Owner
Author
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Supplementary inline review — issues not covered by CodeRabbit. See main comment for the full prioritized list.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/components/settings-worktrees.tsx`:
- Around line 28-33: The current Promise.all call in the rows computation causes
all lookups to fail if any sdk.client.worktree.list rejects; change the logic to
use Promise.allSettled over projectRoots().map(...) so each ownerDirectory
request is awaited independently, then filter the settled results for status ===
'fulfilled', extract their values, flatten them into rows (preserving the
ownerDirectory and typing as WorktreeInfo), and optionally log or ignore
rejected entries so healthy owners' worktrees still display; update the code
that constructs rows (the Promise.all call) to perform this
settled/filter/flatten flow around sdk.client.worktree.list and projectRoots().
In `@packages/opencode/src/session/session.ts`:
- Around line 877-881: The public export updateExecutionContext currently
forwards raw input to svc.updateExecutionContext without Zod validation;
wrap/validate the incoming input using the same fn(...) / Zod parsing used by
neighboring session exports so malformed activeWorktree objects can't be
persisted. Specifically, before calling runPromise((svc) =>
svc.updateExecutionContext(input)), validate the input (or at least
input.activeWorktree when not null) against the SessionExecutionContext contract
(ensure activeWorktree contains directory, name, branch, source) and pass the
validated/parsed value to svc.updateExecutionContext; keep null allowed for
activeWorktree if intended.
In `@packages/opencode/src/tool/exit-worktree.ts`:
- Around line 19-60: The guard checks (hasInFlightToolCallsExcept,
hasRunningSubagents) and the execution-context update must be done atomically
for the session identified by ctx.sessionID: replace the current separate checks
+ sessions.updateExecutionContext call with a single session-scoped CAS/modify
operation (e.g., sessions.modify / sessions.compareAndSwap / sessions.withLock)
that reads the session executionContext, re-checks
hasInFlightToolCallsExcept(session, ctx.callID) and hasRunningSubagents(session)
while holding the lock/inside the modify callback, and either returns a failure
Error (unchanged state) if busy or returns the new executionContext with
activeDirectory set to exec.ownerDirectory and activeWorktree nulled; ensure you
capture exec.activeWorktree into the returned metadata (previous) only after the
CAS/modify succeeds so the transition is atomic and preserves the same error
messages and metadata shape.
🪄 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: 8ed6b174-f6a4-432e-9900-53d88b55df52
📒 Files selected for processing (9)
packages/app/src/components/settings-worktrees-helpers.test.tspackages/app/src/components/settings-worktrees-helpers.tspackages/app/src/components/settings-worktrees.tsxpackages/app/src/context/prompt.test.tspackages/app/src/pages/layout/pawwork-worktree-badge.test.tsxpackages/opencode/src/session/session.tspackages/opencode/src/tool/exit-worktree.tspackages/opencode/test/session/session.test.tspackages/opencode/test/tool/enter-worktree.test.ts
Astro-Han
commented
May 1, 2026
Owner
Author
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Code Review: Kimi K2.6
This is a large, well-structured PR that introduces session execution context for worktrees. The core design is sound: executionContext as a persisted session property with ownerDirectory / activeDirectory separation, backfill for legacy rows, and state-machine guards for enter/exit transitions. Test coverage is good across opencode, app, and UI layers.
P0 (Blocking): 2 issues — a Windows path canonicalization edge case and a potential cache-invalidation gap in Instance.provide.
P1 (Should fix): 4 issues — performance of findActiveWorktreeBinding, Worktree.remove registry cleanup consistency, boundSessions recomputation cost, and PawworkWorktreeBadge reactivity.
P2 (Nice to have): 3 issues — errorText fallback behavior, ensureWorktreesIgnored git-config robustness, and enter-worktree.txt description polish.
P3 (Nit): 3 issues — null/undefined normalization, titlebar magic-number comment, and composer padding rationale.
Overall: approve with comments. The P0 and P1 items should be addressed or explicitly acknowledged before merge.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/ui/src/components/message-part.tsx (1)
726-734:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
toolStateError()in the context-tool fallback too.Line 729 still returns raw
part.state.error, so grouped context-tool rows can bypass the normalization you just added for the main tool-card path and surface a non-string payload again. ReusingtoolStateError(part.state)here keeps legacy/hydrated error handling consistent.Suggested fix
function contextToolDetail(part: ToolPart): string | undefined { const info = getToolInfo(part.tool, part.state.input ?? {}, toolStateMetadata(part.state)) if (info.subtitle) return info.subtitle - if (part.state.status === "error") return part.state.error + if (part.state.status === "error") return toolStateError(part.state) if ((part.state.status === "running" || part.state.status === "completed") && part.state.title) return part.state.title const description = part.state.input?.description if (typeof description === "string") return description🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/message-part.tsx` around lines 726 - 734, In contextToolDetail, replace returning the raw part.state.error with the normalized helper by calling toolStateError(part.state) so grouped context-tool rows use the same legacy/hydrated error normalization as the main tool-card; update the function contextToolDetail to import/consume toolStateError and return toolStateError(part.state) when part.state.status === "error".packages/opencode/src/session/session.ts (1)
61-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winParse
execution_contextbefore returning it.
row.execution_contextis trusted verbatim here. If an older/corrupt row is missing one of the required worktree fields, this still returns anInfothat later crashes when code canonicalizesactiveDirectory/ownerDirectoryor reads badge metadata. Safe-parse the JSON here and fall back tolegacyExecutionContext(row, project)on failure so read paths stay recoverable.Suggested hardening
export function fromRow(row: SessionRow, project?: ProjectFallback): Info { + const executionContext = + row.execution_context === null + ? legacyExecutionContext(row, project) + : ((): SessionExecutionContext => { + const parsed = SessionExecutionContext.safeParse(row.execution_context) + return parsed.success ? parsed.data : legacyExecutionContext(row, project) + })() + const summary = row.summary_additions !== null || row.summary_deletions !== null || row.summary_files !== null ? { @@ - executionContext: row.execution_context ?? legacyExecutionContext(row, project), + executionContext, time: {Based on learnings: Session execution context contract — in
packages/sdk/openapi.json,executionContext.activeWorktreerequiresdirectory,name,branch, andsource.🤖 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 61 - 90, The code currently returns row.execution_context verbatim in fromRow; instead, safe-parse and validate it: in function fromRow, attempt to JSON.parse(row.execution_context) (or otherwise deserialize) and verify required activeWorktree fields (directory, name, branch, source) and any other contract fields; if parsing fails or validation misses required fields, call legacyExecutionContext(row, project) and return that value for executionContext; ensure the final executionContext assigned is either the validated parsed object or the legacyExecutionContext fallback (do not return raw row.execution_context).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/components/settings-worktrees.tsx`:
- Around line 28-39: The current worktree aggregation uses Promise.allSettled
and then treats an all-failed set as an empty result (rows empty), which makes
the UI show "no worktrees" instead of an error; change the logic in the function
that maps projectRoots() + sdk.client.worktree.list to detect if every
result.status === "rejected" (or equivalently rows.flat().length === 0 &&
results.some(r => r.status === "rejected")), and in that case surface an error
(throw or return an error sentinel) rather than returning an empty array; apply
the same check to the duplicate block at 114-126 so that a complete failure of
all owner lookups produces an error state instead of the empty-state UI.
In `@packages/opencode/src/session/session.ts`:
- Around line 473-475: The new createNext always seeds executionContext with
rootContext(...) which causes fork() to lose an existing session's managed
worktree binding; change the seeding logic so fork/createNext preserves the
current session's executionContext when present (i.e. use the existing
ctx.executionContext if set) and only fall back to rootContext(ctx.project.vcs
=== "git" ? ctx.worktree : input.directory) otherwise; update the
createNext/fork call sites that set executionContext to use this conditional so
relative-path tools keep operating against the original session's worktree.
In `@packages/opencode/src/worktree/index.ts`:
- Around line 230-268: upsertRegistry and removeRegistry perform
read/modify/write on ProjectTable.sandboxes without atomicity, so concurrent
calls can clobber each other; fix by serializing mutations for a project using
the InstanceState per-project mutex helper: wrap the entire read → modify →
write sequence (the logic inside upsertRegistry/removeRegistry that uses
readRegistry, writeRegistry and canonical) in an InstanceState.runExclusive or
equivalent lock obtained for Instance.project.id (import InstanceState from
src/effect/instance-state.ts), or convert writeRegistry to perform the update
inside a database transaction/CAS that validates the previous sandboxes value
before writing; ensure the unique symbols referenced (upsertRegistry,
removeRegistry, writeRegistry, readRegistry, ProjectTable.sandboxes,
Instance.project.id) are guarded so registry rewrites are serialized and cleaned
up via InstanceState per coding guidelines.
In `@packages/opencode/test/project/worktree-remove.test.ts`:
- Around line 80-98: The test is platform-specific because it shells out to
`which git` and writes a `#!/bin/bash` shim; guard the setup so it doesn't run
on Windows by either wrapping the shim/setup and assertions with the existing
`wintest` skip guard or by early-skipping when process.platform === "win32"
before calling `which git`, creating `shim` or invoking `Worktree.remove`;
ensure you reference and protect the code that calls `which git`, writes `shim`
(the `shim` variable and Bun.write block), and the subsequent `Worktree.remove`
assertions so Windows CI will skip this test or use an alternative
Windows-compatible shim.
In `@packages/opencode/test/session/state-machine-guard.test.ts`:
- Around line 32-37: The current test "detects pending and running tool calls
except the current call" only covers when both "pending" and "running" appear
together; add two new isolated tests that verify the guard behavior when only
"pending" is present and when only "running" is present. Use the same helpers
sessionsWithParts and toolPart to construct sessions: one test should include
toolPart("current","running") and toolPart("other-pending","pending") and assert
the guard returns true/false as appropriate, and the other should include
toolPart("current","running") and toolPart("other-running","running") with the
same assertions; ensure test names clearly indicate "pending only" and "running
only" cases so the guard remains pinned to both behaviors.
---
Outside diff comments:
In `@packages/opencode/src/session/session.ts`:
- Around line 61-90: The code currently returns row.execution_context verbatim
in fromRow; instead, safe-parse and validate it: in function fromRow, attempt to
JSON.parse(row.execution_context) (or otherwise deserialize) and verify required
activeWorktree fields (directory, name, branch, source) and any other contract
fields; if parsing fails or validation misses required fields, call
legacyExecutionContext(row, project) and return that value for executionContext;
ensure the final executionContext assigned is either the validated parsed object
or the legacyExecutionContext fallback (do not return raw
row.execution_context).
In `@packages/ui/src/components/message-part.tsx`:
- Around line 726-734: In contextToolDetail, replace returning the raw
part.state.error with the normalized helper by calling
toolStateError(part.state) so grouped context-tool rows use the same
legacy/hydrated error normalization as the main tool-card; update the function
contextToolDetail to import/consume toolStateError and return
toolStateError(part.state) when part.state.status === "error".
🪄 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: 69eb6e67-3109-44bc-bf07-0976aa22e658
📒 Files selected for processing (9)
packages/app/src/components/settings-worktrees.tsxpackages/opencode/src/session/execution-context-store.tspackages/opencode/src/session/execution-context.tspackages/opencode/src/session/session.tspackages/opencode/src/tool/agent.tspackages/opencode/src/worktree/index.tspackages/opencode/test/project/worktree-remove.test.tspackages/opencode/test/session/state-machine-guard.test.tspackages/ui/src/components/message-part.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/worktree/gitignore-guard.ts`:
- Around line 42-64: The guard currently skips the git status check when the
variable before === undefined, allowing a locally deleted but tracked .gitignore
to be overwritten; change the logic in gitignore-guard.ts so the git status
invocation (the call to git(root, [... "status" ..., ".gitignore"])) runs
unconditionally before writing the file, keep the existing error handling that
throws GitignoreGuardError when status.code !== 0, and still throw a
GitignoreGuardError when status.stdout.trim() is non-empty; in short,
remove/adjust the before === undefined short-circuit around the git(...) check
so the dirty-state guard always executes prior to the code that writes the new
.gitignore.
🪄 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: e2620c93-fe5a-44f7-a4f4-3fc43ee8660a
📒 Files selected for processing (2)
packages/opencode/src/worktree/gitignore-guard.tspackages/opencode/test/worktree/gitignore-guard.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: smoke-macos-arm64
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-desktop
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-opencode
- 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/worktree/gitignore-guard.test.tspackages/opencode/src/worktree/gitignore-guard.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/worktree/gitignore-guard.test.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 360
File: packages/sdk/openapi.json:10181-10207
Timestamp: 2026-05-01T12:00:48.110Z
Learning: Astro-Han/pawwork (PR `#360`): Session execution context contract — In packages/sdk/openapi.json, executionContext.activeWorktree requires directory, name, branch, and source. This was fixed in commit 993fcfa1 and propagated to generated SDK types and DB row typing.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 360
File: packages/opencode/src/tool/exit-worktree.ts:19-60
Timestamp: 2026-05-01T18:02:24.479Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/exit-worktree.ts` and `packages/opencode/src/tool/enter-worktree.ts`, PR `#360`), the TOCTOU window between the `hasInFlightToolCallsExcept`/`hasRunningSubagents` guard checks and the `sessions.updateExecutionContext(...)` call is a known, accepted limitation. There is no session-scoped CAS/modify API yet; adding one requires a broader session state architecture change deferred to a dedicated follow-up PR. The current non-atomic guard is covered by focused tests in commit 68074b768. Do NOT re-flag the absence of atomic CAS transition locking in `exit-worktree.ts` or `enter-worktree.ts` until a session-scoped lock/modify primitive is introduced.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:12.228Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/prompt.ts` and `packages/opencode/src/session/processor.ts`, PR `#264`), the loop-gate race condition between `buildLoopContext()` and `recordSyntheticBlock`/`recordSyntheticStop` is intentionally handled via idempotence guards (re-check sigKey presence / `hasStopped` inside the record helpers) rather than a full per-parent `Effect.Mutex`. Threading a `Map<MessageID, Mutex>` through the processor was considered too large a surface change for this edge case; the residual TOCTOU window only produces extra synthetic parts with no behavioral drift on the "turn ends" contract. A code comment documents the trade-off and points to a full-mutex follow-up if the race is observed in practice. Do NOT re-flag the absence of a per-parent mutex as a blocking issue in future reviews.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/tool.ts:34-42
Timestamp: 2026-04-28T04:56:25.162Z
Learning: In `packages/opencode/src/tool/tool.ts` (Astro-Han/pawwork, PR `#270`), the `Def.execute` signature uses `ctx: Context` (without the `M` generic) instead of `ctx: Context<M>`, meaning `ctx.metadata(...)` is not checked against the same `M` as `ExecuteResult<M>`. This is inherited upstream behavior adopted via the graft strategy in PR `#270` (an upstream-sync PR per `project_upstream_strategy.md`). Fixing it here would mix refactor + bugfix intents and drift the diff from the upstream baseline. The fix is deferred to a dedicated follow-up PR or upstream report. Do NOT re-flag the missing `Context<M>` generic in `Def.execute` during upstream-sync PRs; only flag it in a PawWork-authored PR that directly touches `tool.ts` type signatures.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/patch/index.ts:337-346
Timestamp: 2026-04-28T04:38:10.087Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/patch/index.ts`), the BOM-transition surfacing gap — where `Bom.split` strips BOM before building `unified_diff`/`new_content` but `Bom.join` later re-attaches BOM on disk write, meaning BOM changes are not reflected in the diff payload — is intentionally deferred. PR `#270` is an upstream-sync graft; fixing the issue here would mix refactor + bugfix intents and drift the diff from the upstream baseline needed for clean future grafts. A dedicated follow-up PR (or upstream report) will address this. Do NOT re-flag the missing BOM-change surfacing in `ApplyPatchFileUpdate`/`ApplyPatchFileChange` as a blocking issue in PR `#270` or in future sync PRs that carry the same upstream baseline.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/edit.ts:35-45
Timestamp: 2026-04-28T04:38:13.519Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork, PR `#270`), the module-scoped `Map<string, Semaphore.Semaphore>` (`locks`) and `lock(filePath)` helper are intentionally left as a global, never-cleaned registry. PR `#270` is a pure upstream-sync graft and mixing in a bugfix would drift the diff from the upstream baseline. The fix (moving per-file semaphores into `InstanceState`/`ScopedCache` for per-instance lifecycle management) is tracked as a follow-up to land in its own PR or be reported upstream. Do NOT re-flag the absence of InstanceState wiring for the lock map in this file until the follow-up PR is opened.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:39.511Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:15.483Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:172-175
Timestamp: 2026-04-28T04:56:22.159Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the `files dies on nonexistent directory` test hardcodes `/tmp/nonexistent-dir-12345` as the missing-directory path. This is upstream-inherited behaviour adopted wholesale via the graft strategy (`project_upstream_strategy.md`). The fix — replacing the hardcoded path with a platform-neutral guaranteed-nonexistent child path derived from the `tmpdir` fixture — is intentionally deferred to a follow-up PR or upstream report, to avoid mixing bugfix + refactor intents and drifting the diff from the upstream baseline future syncs need. Do NOT re-flag the hardcoded `/tmp/nonexistent-dir-12345` path in this file as a blocking or actionable issue until the follow-up PR lands.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:311-328
Timestamp: 2026-04-28T05:36:21.974Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the ripgrep binary download flow (`HttpClientRequest.get` → `fs.writeWithDirs` → `extract`) does NOT verify a pinned SHA-256 checksum. Adding per-platform hash verification (15.1.0 × 6 platforms) requires a dedicated manifest + `crypto.subtle.digest` step and is intentionally deferred to a follow-up PR. Existing mitigations: (1) `VERSION` is pinned to `"15.1.0"`, (2) downloads are over HTTPS from GitHub Releases, (3) PawWork production builds ship a bundled `rg` binary so this download path is a last-resort fallback (system PATH → cached → download). Do NOT re-flag the missing checksum verification in this file until the dedicated follow-up PR lands.
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : When using the `tmpdir` function with git repository support, pass the `git: true` option to initialize a git repo with a root commit.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.tspackages/opencode/src/worktree/gitignore-guard.ts
📚 Learning: 2026-04-28T04:56:22.159Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:172-175
Timestamp: 2026-04-28T04:56:22.159Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the `files dies on nonexistent directory` test hardcodes `/tmp/nonexistent-dir-12345` as the missing-directory path. This is upstream-inherited behaviour adopted wholesale via the graft strategy (`project_upstream_strategy.md`). The fix — replacing the hardcoded path with a platform-neutral guaranteed-nonexistent child path derived from the `tmpdir` fixture — is intentionally deferred to a follow-up PR or upstream report, to avoid mixing bugfix + refactor intents and drifting the diff from the upstream baseline future syncs need. Do NOT re-flag the hardcoded `/tmp/nonexistent-dir-12345` path in this file as a blocking or actionable issue until the follow-up PR lands.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.tspackages/opencode/src/worktree/gitignore-guard.ts
📚 Learning: 2026-04-28T05:36:28.779Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:28.779Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.tspackages/opencode/src/worktree/gitignore-guard.ts
📚 Learning: 2026-04-28T05:36:28.940Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/grep.test.ts:16-22
Timestamp: 2026-04-28T05:36:28.940Z
Learning: In `packages/opencode/test/tool/grep.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a manual `ManagedRuntime.make(Layer.mergeAll(...))` setup and raw `bun:test` test cases rather than the `testEffect(...)` harness. The migration to `testEffect` + `it.live(...)` + `provideTmpdirInstance(...)` is a whole-file rewrite deferred to a dedicated sweep PR that will migrate all tool tests still on the manual ManagedRuntime pattern together. Do NOT re-flag the ManagedRuntime setup in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.ts
📚 Learning: 2026-04-28T04:38:23.632Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:23.632Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.tspackages/opencode/src/worktree/gitignore-guard.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `tmpdir` function from `fixture/fixture.ts` to create temporary directories for tests with automatic cleanup. Use `await using` syntax to ensure automatic cleanup when the variable goes out of scope.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `init` option in `tmpdir` to define custom setup functions that can return extra data accessible via `tmp.extra`, and use the `dispose` option for custom cleanup logic.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.ts
📚 Learning: 2026-04-22T08:49:47.800Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.ts
📚 Learning: 2026-04-27T11:18:47.596Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/agent/agent.test.ts:440-447
Timestamp: 2026-04-27T11:18:47.596Z
Learning: In `packages/opencode/test/agent/agent.test.ts` (Astro-Han/pawwork), all agent-permission tests intentionally use the manual `tmpdir()` + `Instance.provide(...)` pattern. Do not flag individual tests in this file for conversion to `provideTmpdirInstance(...)` or `provideInstance(...)`; a full harness migration would be a separate PR if the pattern ever needs to change.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `config` option in `tmpdir` to write an `opencode.json` config file during test setup by passing a partial Config.Info object.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.ts
📚 Learning: 2026-04-29T13:27:31.536Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 329
File: packages/app/src/pages/session/session-view-controller.test.ts:5-45
Timestamp: 2026-04-29T13:27:31.536Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/session/session-view-controller.test.ts` and related Solid + Bun test files), the Bun + Solid test environment does NOT reliably exercise `createMemo((current) => ...)` signal recomputation the way a browser runtime does. Adding signal-driven transition tests (e.g., using `createSignal` + `createRoot` to flip reactive inputs) is misleading in this environment because the reactive invalidation/recomputation path is not faithfully replicated. The correct strategy is: cover pure transition logic with plain unit tests (e.g., `nextSessionViewState`), and cover the browser reactive path with E2E tests (e.g., session-switch E2E spec). Do NOT re-flag the absence of signal-driven `createSessionViewController` tests in this environment as a gap.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.ts
📚 Learning: 2026-04-23T08:51:00.819Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:00.819Z
Learning: For pawwork tests under packages/opencode/test/**, auth.json teardown may intentionally combine `Filesystem.write` (from `packages/opencode/src/util/filesystem.ts`) with `node:fs/promises` `unlink` for cleanup. Do not flag this as inconsistent style; it is the established/intentional pattern because `Filesystem` does not provide a `remove`/`unlink` helper.
Applied to files:
packages/opencode/test/worktree/gitignore-guard.test.ts
📚 Learning: 2026-04-28T08:29:07.177Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:07.177Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.
Applied to files:
packages/opencode/src/worktree/gitignore-guard.ts
📚 Learning: 2026-04-28T04:38:13.519Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/edit.ts:35-45
Timestamp: 2026-04-28T04:38:13.519Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork, PR `#270`), the module-scoped `Map<string, Semaphore.Semaphore>` (`locks`) and `lock(filePath)` helper are intentionally left as a global, never-cleaned registry. PR `#270` is a pure upstream-sync graft and mixing in a bugfix would drift the diff from the upstream baseline. The fix (moving per-file semaphores into `InstanceState`/`ScopedCache` for per-instance lifecycle management) is tracked as a follow-up to land in its own PR or be reported upstream. Do NOT re-flag the absence of InstanceState wiring for the lock map in this file until the follow-up PR is opened.
Applied to files:
packages/opencode/src/worktree/gitignore-guard.ts
📚 Learning: 2026-05-01T18:02:24.479Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 360
File: packages/opencode/src/tool/exit-worktree.ts:19-60
Timestamp: 2026-05-01T18:02:24.479Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/exit-worktree.ts` and `packages/opencode/src/tool/enter-worktree.ts`, PR `#360`), the TOCTOU window between the `hasInFlightToolCallsExcept`/`hasRunningSubagents` guard checks and the `sessions.updateExecutionContext(...)` call is a known, accepted limitation. There is no session-scoped CAS/modify API yet; adding one requires a broader session state architecture change deferred to a dedicated follow-up PR. The current non-atomic guard is covered by focused tests in commit 68074b768. Do NOT re-flag the absence of atomic CAS transition locking in `exit-worktree.ts` or `enter-worktree.ts` until a session-scoped lock/modify primitive is introduced.
Applied to files:
packages/opencode/src/worktree/gitignore-guard.ts
📚 Learning: 2026-04-28T07:28:02.333Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:364-368
Timestamp: 2026-04-28T07:28:02.333Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), both `files()` and `search()` do not short-circuit on an already-aborted `AbortSignal` before `spawner.spawn(...)`. The `check(input.cwd)` effect before spawn already provides a natural cancellation point for interrupted fibers. Upstream `dev:packages/opencode/src/file/ripgrep.ts` has identical ordering with no pre-spawn `signal.aborted` guard; adding one would diverge from the graft baseline. The fix (explicit pre-spawn `signal.aborted` check) is deferred to a dedicated PawWork ripgrep cancellation pass or upstream PR. Do NOT re-flag the missing pre-spawn cancellation short-circuit in `files()`/`search()` in upstream-sync PRs.
Applied to files:
packages/opencode/src/worktree/gitignore-guard.ts
📚 Learning: 2026-04-28T05:36:21.974Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:311-328
Timestamp: 2026-04-28T05:36:21.974Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the ripgrep binary download flow (`HttpClientRequest.get` → `fs.writeWithDirs` → `extract`) does NOT verify a pinned SHA-256 checksum. Adding per-platform hash verification (15.1.0 × 6 platforms) requires a dedicated manifest + `crypto.subtle.digest` step and is intentionally deferred to a follow-up PR. Existing mitigations: (1) `VERSION` is pinned to `"15.1.0"`, (2) downloads are over HTTPS from GitHub Releases, (3) PawWork production builds ship a bundled `rg` binary so this download path is a last-resort fallback (system PATH → cached → download). Do NOT re-flag the missing checksum verification in this file until the dedicated follow-up PR lands.
Applied to files:
packages/opencode/src/worktree/gitignore-guard.ts
📚 Learning: 2026-04-28T04:38:10.087Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/patch/index.ts:337-346
Timestamp: 2026-04-28T04:38:10.087Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/patch/index.ts`), the BOM-transition surfacing gap — where `Bom.split` strips BOM before building `unified_diff`/`new_content` but `Bom.join` later re-attaches BOM on disk write, meaning BOM changes are not reflected in the diff payload — is intentionally deferred. PR `#270` is an upstream-sync graft; fixing the issue here would mix refactor + bugfix intents and drift the diff from the upstream baseline needed for clean future grafts. A dedicated follow-up PR (or upstream report) will address this. Do NOT re-flag the missing BOM-change surfacing in `ApplyPatchFileUpdate`/`ApplyPatchFileChange` as a blocking issue in PR `#270` or in future sync PRs that carry the same upstream baseline.
Applied to files:
packages/opencode/src/worktree/gitignore-guard.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/worktree/gitignore-guard.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/worktree/gitignore-guard.ts
🔇 Additional comments (2)
packages/opencode/src/worktree/gitignore-guard.ts (1)
25-31: Good normalization of.gitignorepatterns before matching.This correctly handles comments/whitespace and common
.worktreesentry variants without duplicating entries.packages/opencode/test/worktree/gitignore-guard.test.ts (1)
9-44: Solid coverage for the new guard behavior.The suite validates both happy paths and key refusal paths (
local changesandhidden untracked) with correct fixture setup.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/app/src/components/settings-worktrees.tsx (1)
28-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSurface a load failure instead of rendering the empty state.
When every owner lookup fails, this still collapses to
[], so the UI shows the “no worktrees” copy even though the data never loaded. Keep an error signal here so the empty state only appears for a real zero-row response.Also applies to: 114-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/components/settings-worktrees.tsx` around lines 28 - 39, The current fetch logic collapses all-settled failures into an empty array so the UI shows the "no worktrees" state even when all owner lookups failed; update the Promise.allSettled handling around results/rows/byDirectory (the block using projectRoots(), sdk.client.worktree.list and constructing WorktreeInfo) to detect when every result.status === "rejected" and surface an error (e.g., throw or return a failure sentinel) instead of returning [] so the caller can render a load-failure state; apply the same change to the other occurrence of this pattern (the block at the other referenced location) so both places distinguish "no rows" from "all lookups failed."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/app/src/components/settings-worktrees.tsx`:
- Around line 28-39: The current fetch logic collapses all-settled failures into
an empty array so the UI shows the "no worktrees" state even when all owner
lookups failed; update the Promise.allSettled handling around
results/rows/byDirectory (the block using projectRoots(),
sdk.client.worktree.list and constructing WorktreeInfo) to detect when every
result.status === "rejected" and surface an error (e.g., throw or return a
failure sentinel) instead of returning [] so the caller can render a
load-failure state; apply the same change to the other occurrence of this
pattern (the block at the other referenced location) so both places distinguish
"no rows" from "all lookups failed."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f80f86d7-499e-4fd7-85a2-85677329537a
📒 Files selected for processing (3)
packages/app/src/components/settings-worktrees-helpers.test.tspackages/app/src/components/settings-worktrees-helpers.tspackages/app/src/components/settings-worktrees.tsx
📜 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). (5)
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-desktop
🧰 Additional context used
📓 Path-based instructions (1)
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-helpers.test.tspackages/app/src/components/settings-worktrees.tsxpackages/app/src/components/settings-worktrees-helpers.ts
🧠 Learnings (45)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 360
File: packages/sdk/openapi.json:10181-10207
Timestamp: 2026-05-01T12:00:48.110Z
Learning: Astro-Han/pawwork (PR `#360`): Session execution context contract — In packages/sdk/openapi.json, executionContext.activeWorktree requires directory, name, branch, and source. This was fixed in commit 993fcfa1 and propagated to generated SDK types and DB row typing.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 360
File: packages/opencode/src/tool/exit-worktree.ts:19-60
Timestamp: 2026-05-01T18:02:24.479Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/exit-worktree.ts` and `packages/opencode/src/tool/enter-worktree.ts`, PR `#360`), the TOCTOU window between the `hasInFlightToolCallsExcept`/`hasRunningSubagents` guard checks and the `sessions.updateExecutionContext(...)` call is a known, accepted limitation. There is no session-scoped CAS/modify API yet; adding one requires a broader session state architecture change deferred to a dedicated follow-up PR. The current non-atomic guard is covered by focused tests in commit 68074b768. Do NOT re-flag the absence of atomic CAS transition locking in `exit-worktree.ts` or `enter-worktree.ts` until a session-scoped lock/modify primitive is introduced.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:12.228Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/prompt.ts` and `packages/opencode/src/session/processor.ts`, PR `#264`), the loop-gate race condition between `buildLoopContext()` and `recordSyntheticBlock`/`recordSyntheticStop` is intentionally handled via idempotence guards (re-check sigKey presence / `hasStopped` inside the record helpers) rather than a full per-parent `Effect.Mutex`. Threading a `Map<MessageID, Mutex>` through the processor was considered too large a surface change for this edge case; the residual TOCTOU window only produces extra synthetic parts with no behavioral drift on the "turn ends" contract. A code comment documents the trade-off and points to a full-mutex follow-up if the race is observed in practice. Do NOT re-flag the absence of a per-parent mutex as a blocking issue in future reviews.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/patch/index.ts:337-346
Timestamp: 2026-04-28T04:38:10.087Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/patch/index.ts`), the BOM-transition surfacing gap — where `Bom.split` strips BOM before building `unified_diff`/`new_content` but `Bom.join` later re-attaches BOM on disk write, meaning BOM changes are not reflected in the diff payload — is intentionally deferred. PR `#270` is an upstream-sync graft; fixing the issue here would mix refactor + bugfix intents and drift the diff from the upstream baseline needed for clean future grafts. A dedicated follow-up PR (or upstream report) will address this. Do NOT re-flag the missing BOM-change surfacing in `ApplyPatchFileUpdate`/`ApplyPatchFileChange` as a blocking issue in PR `#270` or in future sync PRs that carry the same upstream baseline.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/tool.ts:34-42
Timestamp: 2026-04-28T04:56:25.162Z
Learning: In `packages/opencode/src/tool/tool.ts` (Astro-Han/pawwork, PR `#270`), the `Def.execute` signature uses `ctx: Context` (without the `M` generic) instead of `ctx: Context<M>`, meaning `ctx.metadata(...)` is not checked against the same `M` as `ExecuteResult<M>`. This is inherited upstream behavior adopted via the graft strategy in PR `#270` (an upstream-sync PR per `project_upstream_strategy.md`). Fixing it here would mix refactor + bugfix intents and drift the diff from the upstream baseline. The fix is deferred to a dedicated follow-up PR or upstream report. Do NOT re-flag the missing `Context<M>` generic in `Def.execute` during upstream-sync PRs; only flag it in a PawWork-authored PR that directly touches `tool.ts` type signatures.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/edit.ts:35-45
Timestamp: 2026-04-28T04:38:13.519Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork, PR `#270`), the module-scoped `Map<string, Semaphore.Semaphore>` (`locks`) and `lock(filePath)` helper are intentionally left as a global, never-cleaned registry. PR `#270` is a pure upstream-sync graft and mixing in a bugfix would drift the diff from the upstream baseline. The fix (moving per-file semaphores into `InstanceState`/`ScopedCache` for per-instance lifecycle management) is tracked as a follow-up to land in its own PR or be reported upstream. Do NOT re-flag the absence of InstanceState wiring for the lock map in this file until the follow-up PR is opened.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:39.511Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:172-175
Timestamp: 2026-04-28T04:56:22.159Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the `files dies on nonexistent directory` test hardcodes `/tmp/nonexistent-dir-12345` as the missing-directory path. This is upstream-inherited behaviour adopted wholesale via the graft strategy (`project_upstream_strategy.md`). The fix — replacing the hardcoded path with a platform-neutral guaranteed-nonexistent child path derived from the `tmpdir` fixture — is intentionally deferred to a follow-up PR or upstream report, to avoid mixing bugfix + refactor intents and drifting the diff from the upstream baseline future syncs need. Do NOT re-flag the hardcoded `/tmp/nonexistent-dir-12345` path in this file as a blocking or actionable issue until the follow-up PR lands.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:15.483Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/edit.ts:48-48
Timestamp: 2026-04-28T08:14:35.518Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork), the `filePath` schema description (`"The absolute path to the file to modify"`) is upstream-inherited from `dev:packages/opencode/src/tool/edit.ts:48`. The runtime actually accepts relative paths (resolved via `Instance.directory` at lines 79-81), but the description fix is intentionally deferred to a single PawWork-authored description-cleanup PR that will also cover the identical mismatch in `packages/opencode/src/tool/write.ts:24`. Do NOT re-flag the too-narrow `filePath` description in upstream-sync PRs; flag it only in the dedicated description-cleanup PR.
📚 Learning: 2026-04-22T08:49:47.800Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-28T05:36:28.940Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/grep.test.ts:16-22
Timestamp: 2026-04-28T05:36:28.940Z
Learning: In `packages/opencode/test/tool/grep.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a manual `ManagedRuntime.make(Layer.mergeAll(...))` setup and raw `bun:test` test cases rather than the `testEffect(...)` harness. The migration to `testEffect` + `it.live(...)` + `provideTmpdirInstance(...)` is a whole-file rewrite deferred to a dedicated sweep PR that will migrate all tool tests still on the manual ManagedRuntime pattern together. Do NOT re-flag the ManagedRuntime setup in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-28T05:36:28.779Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:28.779Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-27T12:59:49.844Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/test/session/prompt-effect.test.ts:0-0
Timestamp: 2026-04-27T12:59:49.844Z
Learning: In `packages/opencode/test/session/prompt-effect.test.ts` and `packages/opencode/src/session/diagnostics.ts` (PR `#264`), the recovery reminder copy differs between signature kinds: the input-repeat variant says "repeated the same tool input 3 times" (uses a literal count), while the target-repeat variant says "failed against the same target multiple times" (uses "multiple times" with no count). Assertions that check for injected reminder text in LLM inputs must accept both phrasings when a scenario produces both `input:` and `target:` signatures (e.g., `read` tool with a `filePath` parameter). Do NOT narrow the assertion to only the input-variant phrasing.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.tspackages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-29T13:27:31.536Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 329
File: packages/app/src/pages/session/session-view-controller.test.ts:5-45
Timestamp: 2026-04-29T13:27:31.536Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/session/session-view-controller.test.ts` and related Solid + Bun test files), the Bun + Solid test environment does NOT reliably exercise `createMemo((current) => ...)` signal recomputation the way a browser runtime does. Adding signal-driven transition tests (e.g., using `createSignal` + `createRoot` to flip reactive inputs) is misleading in this environment because the reactive invalidation/recomputation path is not faithfully replicated. The correct strategy is: cover pure transition logic with plain unit tests (e.g., `nextSessionViewState`), and cover the browser reactive path with E2E tests (e.g., session-switch E2E spec). Do NOT re-flag the absence of signal-driven `createSessionViewController` tests in this environment as a gap.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.tspackages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-27T11:19:24.963Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch-auth.test.ts:0-0
Timestamp: 2026-04-27T11:19:24.963Z
Learning: In `packages/opencode/test/tool/websearch-auth.test.ts` (Astro-Han/pawwork), the tests intentionally use a small local `runWith` runner with raw `bun:test` and `Effect.runPromise` rather than the `testEffect` harness. Each test case injects a custom in-memory `Auth.Service` layer; switching to `testEffect` would be style-only churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `config` option in `tmpdir` to write an `opencode.json` config file during test setup by passing a partial Config.Info object.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-28T04:38:23.632Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:23.632Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-28T04:56:22.159Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:172-175
Timestamp: 2026-04-28T04:56:22.159Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the `files dies on nonexistent directory` test hardcodes `/tmp/nonexistent-dir-12345` as the missing-directory path. This is upstream-inherited behaviour adopted wholesale via the graft strategy (`project_upstream_strategy.md`). The fix — replacing the hardcoded path with a platform-neutral guaranteed-nonexistent child path derived from the `tmpdir` fixture — is intentionally deferred to a follow-up PR or upstream report, to avoid mixing bugfix + refactor intents and drifting the diff from the upstream baseline future syncs need. Do NOT re-flag the hardcoded `/tmp/nonexistent-dir-12345` path in this file as a blocking or actionable issue until the follow-up PR lands.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.tspackages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Define `const it = testEffect(...)` near the top of the test file and keep the test body inside `Effect.gen(function* () { ... })`. Yield services directly with `yield* MyService.Service` or `yield* MyTool`.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-28T04:56:23.566Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/tool-define.test.ts:2-9
Timestamp: 2026-04-28T04:56:23.566Z
Learning: In `packages/opencode/test/tool/tool-define.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a `ManagedRuntime.make(Layer.mergeAll(Truncate.defaultLayer, Agent.defaultLayer))` wrapper and raw `bun:test` `test(...)` cases rather than the `testEffect(...)` harness. This code was adopted wholesale from upstream via the graft strategy (per `project_upstream_strategy.md`). Migrating to `testEffect` is deferred to a follow-up PR so as not to mix refactor + harness-migration intents and to avoid drifting the diff from the upstream baseline needed for clean future grafts. Do NOT re-flag the `ManagedRuntime.make` usage in this file as needing harness migration until that follow-up lands.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-27T11:18:47.332Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/mcp-exa.test.ts:1-186
Timestamp: 2026-04-27T11:18:47.332Z
Learning: In `packages/opencode/test/tool/mcp-exa.test.ts` (Astro-Han/pawwork), the tests intentionally use raw `bun:test` async cases with `Effect.runPromise(...)` and per-case `HttpClient.make(...)` fakes rather than the `testEffect(...)` harness. The maintainer has explicitly decided not to migrate, because the HttpClient fake wiring is itself the behavior under test and switching to `testEffect` would be style churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Avoid custom `ManagedRuntime`, `attach(...)`, or ad hoc `run(...)` wrappers in Effect tests when `testEffect(...)` already provides the runtime.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.ts
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.tspackages/app/src/components/settings-worktrees.tsxpackages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-04-23T15:10:21.635Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 191
File: packages/app/src/components/session/pawwork-skill-meta.ts:38-39
Timestamp: 2026-04-23T15:10:21.635Z
Learning: This repo configures Tailwind v4 with `--color-*: initial`, which effectively breaks standard Tailwind palette utilities (e.g., `text-violet-500` can resolve to no CSS variable and render as a no-op/black). For brand/accent colors that are not backed by semantic design tokens, use inline styles with the exact hex value (e.g., `style={{ color: '#8B5FBF' }}` / `homeIconStyle: { color: '#8B5FBF' }`) and add a short comment explaining that Tailwind palette utilities won’t work due to the `--color-*: initial` setup. Do not suggest replacing these inline hex colors with Tailwind palette classes anywhere in this repo.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.test.tspackages/app/src/components/settings-worktrees.tsxpackages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-05-01T12:00:48.110Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 360
File: packages/sdk/openapi.json:10181-10207
Timestamp: 2026-05-01T12:00:48.110Z
Learning: Astro-Han/pawwork (PR `#360`): Session execution context contract — In packages/sdk/openapi.json, executionContext.activeWorktree requires directory, name, branch, and source. This was fixed in commit 993fcfa1 and propagated to generated SDK types and DB row typing.
Applied to files:
packages/app/src/components/settings-worktrees.tsxpackages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-04-28T04:38:13.519Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/edit.ts:35-45
Timestamp: 2026-04-28T04:38:13.519Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork, PR `#270`), the module-scoped `Map<string, Semaphore.Semaphore>` (`locks`) and `lock(filePath)` helper are intentionally left as a global, never-cleaned registry. PR `#270` is a pure upstream-sync graft and mixing in a bugfix would drift the diff from the upstream baseline. The fix (moving per-file semaphores into `InstanceState`/`ScopedCache` for per-instance lifecycle management) is tracked as a follow-up to land in its own PR or be reported upstream. Do NOT re-flag the absence of InstanceState wiring for the lock map in this file until the follow-up PR is opened.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-23T08:51:04.230Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:04.230Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/util/filesystem.ts`), the `Filesystem` utility does NOT expose a `remove` or `unlink` helper. The established repository pattern for auth.json teardown in tests (e.g. `provider.test.ts`, `amazon-bedrock.test.ts`, `workspace-adaptor.test.ts`) is to combine `Filesystem.write` with `node:fs/promises unlink`. Do not flag this mixed usage as inconsistent — it is the correct and intentional pattern.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-28T08:29:07.177Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:07.177Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-30T10:27:01.380Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 342
File: packages/app/src/components/dialog-delete-session.tsx:22-31
Timestamp: 2026-04-30T10:27:01.380Z
Learning: In `packages/app/src/components/dialog-delete-session.tsx` (Astro-Han/pawwork, PR `#342`), `handleDelete` uses a `deleting` signal with `try/finally` but intentionally omits an explicit `catch` block. The caller (`deleteSession` in `packages/app/src/pages/layout.tsx` around line 1083) already swallows errors with a toast, so `onConfirm` does not reject in practice. The `try/finally` is sufficient to keep the Delete button usable if a future caller throws. Do NOT re-flag the missing explicit catch in this component.
Applied to files:
packages/app/src/components/settings-worktrees.tsxpackages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-05-01T18:02:24.479Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 360
File: packages/opencode/src/tool/exit-worktree.ts:19-60
Timestamp: 2026-05-01T18:02:24.479Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/exit-worktree.ts` and `packages/opencode/src/tool/enter-worktree.ts`, PR `#360`), the TOCTOU window between the `hasInFlightToolCallsExcept`/`hasRunningSubagents` guard checks and the `sessions.updateExecutionContext(...)` call is a known, accepted limitation. There is no session-scoped CAS/modify API yet; adding one requires a broader session state architecture change deferred to a dedicated follow-up PR. The current non-atomic guard is covered by focused tests in commit 68074b768. Do NOT re-flag the absence of atomic CAS transition locking in `exit-worktree.ts` or `enter-worktree.ts` until a session-scoped lock/modify primitive is introduced.
Applied to files:
packages/app/src/components/settings-worktrees.tsxpackages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-04-27T10:33:12.228Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:12.228Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/prompt.ts` and `packages/opencode/src/session/processor.ts`, PR `#264`), the loop-gate race condition between `buildLoopContext()` and `recordSyntheticBlock`/`recordSyntheticStop` is intentionally handled via idempotence guards (re-check sigKey presence / `hasStopped` inside the record helpers) rather than a full per-parent `Effect.Mutex`. Threading a `Map<MessageID, Mutex>` through the processor was considered too large a surface change for this edge case; the residual TOCTOU window only produces extra synthetic parts with no behavioral drift on the "turn ends" contract. A code comment documents the trade-off and points to a full-mutex follow-up if the race is observed in practice. Do NOT re-flag the absence of a per-parent mutex as a blocking issue in future reviews.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-24T00:02:53.315Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 203
File: packages/app/e2e/sidebar/sidebar-session-links.spec.ts:34-55
Timestamp: 2026-04-24T00:02:53.315Z
Learning: In Astro-Han/pawwork E2E tests (`packages/app/e2e/**/*.spec.ts`), `project.trackDirectory()` and `project.trackSession()` cannot be called before `project.open()` — the `project` fixture throws until `open()` initializes its internal state. The correct pattern is: call `project.trackSession(sessionID)` from inside the `beforeGoto` callback (where state already exists), call `project.trackDirectory(directory)` and cross-workspace `project.trackSession(id, directory)` immediately after `project.open()` returns, and rely on explicit `finally` cleanup (e.g. `cleanupSession` / `cleanupTestProject`) for any resources created before `open()` that cannot yet be tracked via the fixture.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-26T16:34:57.130Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 247
File: packages/ui/src/components/message-part.tsx:1322-1324
Timestamp: 2026-04-26T16:34:57.130Z
Learning: In Astro-Han/pawwork (`packages/ui/src/components/message-part.tsx`), the `taskId` createMemo and `childSessionId` createMemo both intentionally read only from `partMetadata().sessionId` (populated post-execution), not from `input.task_id` / `input.subagent_session_id`. This has always been the case — the original code never read the input field either. Adding an `input.subagent_session_id` fallback would be a new capability, not a bug fix. Do NOT flag the absence of this fallback as a regression in PR `#247` or future PRs unless there is a concrete case where metadata is not populated.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-28T05:36:25.084Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/session/compaction.ts:169-191
Timestamp: 2026-04-28T05:36:25.084Z
Learning: In `packages/opencode/src/session/compaction.ts` (Astro-Han/pawwork), the internal helper `splitTurn` intentionally returns a raw `Effect.gen(...)` rather than being wrapped with `Effect.fnUntraced`. Converting `splitTurn` (and similar plain-Effect.gen internal helpers in this file, e.g. `turns`, `completedCompactions`, `buildPrompt`) to `Effect.fnUntraced` is a repo-wide convention sweep deferred to a dedicated follow-up PR. The same sweep covers `tool/tool.ts` and other helpers flagged in the same sync. Do NOT re-flag the absence of `Effect.fnUntraced` on `splitTurn` or other internal helpers in `compaction.ts` during upstream-sync PRs; flag it only in the dedicated convention-sweep PR.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-24T03:51:56.211Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:56.211Z
Learning: In Astro-Han/pawwork E2E tests (packages/app/e2e/fixtures.ts), `project.prompt(text)` internally calls `trackSession(next.sessionID, active.directory)` after the prompt submission is observed and the active session is resolved. Tests that obtain a `sessionID` from `project.prompt()` do NOT need to call `project.trackSession(sessionID)` manually — it is already registered for teardown. Calling it again would be duplicate ownership.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-28T12:01:19.354Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/session.ts:518-542
Timestamp: 2026-04-28T12:01:19.354Z
Learning: In `packages/opencode/src/session/session.ts` (Astro-Han/pawwork, PR `#287`), the `updatePart` subtask guard intentionally allows first writes (where `existing === undefined`) to pass through without calling `lifecycleFieldsChanged`. This is required for `Session.fork()`, migration, and import paths that replay historical `SubtaskPart` rows via `updatePart()` outside any `SubagentRunWriterContext`. Only mutations of an already-persisted row (`existing` is defined) are policed. Do NOT suggest adding a lifecycle check on first-write in this guard.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-28T07:28:02.333Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:364-368
Timestamp: 2026-04-28T07:28:02.333Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), both `files()` and `search()` do not short-circuit on an already-aborted `AbortSignal` before `spawner.spawn(...)`. The `check(input.cwd)` effect before spawn already provides a natural cancellation point for interrupted fibers. Upstream `dev:packages/opencode/src/file/ripgrep.ts` has identical ordering with no pre-spawn `signal.aborted` guard; adding one would diverge from the graft baseline. The fix (explicit pre-spawn `signal.aborted` check) is deferred to a dedicated PawWork ripgrep cancellation pass or upstream PR. Do NOT re-flag the missing pre-spawn cancellation short-circuit in `files()`/`search()` in upstream-sync PRs.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-28T07:27:52.354Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:45-72
Timestamp: 2026-04-28T07:27:52.354Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork), `PathText`, the `lines` field inside `SearchMatch`, and `submatches[].match` accept only `{text: string}` and do NOT handle ripgrep's `{bytes: base64}` JSON variant (emitted for non-UTF-8 paths/match text). This matches the upstream `dev:packages/opencode/src/file/ripgrep.ts` baseline exactly. The fix — adding `Schema.Union([Schema.Struct({text: Schema.String}), Schema.Struct({bytes: Schema.String})])` for each location plus base64-decode in downstream consumers — is intentionally deferred to either a dedicated PawWork PR or an upstream-first fix inherited via the next sync. Do NOT re-flag the missing `{bytes: ...}` union variants in upstream-sync PRs; flag it only in a PawWork-authored PR that directly touches these schema definitions or their downstream consumers.
Applied to files:
packages/app/src/components/settings-worktrees.tsxpackages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-04-28T05:36:21.974Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:311-328
Timestamp: 2026-04-28T05:36:21.974Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the ripgrep binary download flow (`HttpClientRequest.get` → `fs.writeWithDirs` → `extract`) does NOT verify a pinned SHA-256 checksum. Adding per-platform hash verification (15.1.0 × 6 platforms) requires a dedicated manifest + `crypto.subtle.digest` step and is intentionally deferred to a follow-up PR. Existing mitigations: (1) `VERSION` is pinned to `"15.1.0"`, (2) downloads are over HTTPS from GitHub Releases, (3) PawWork production builds ship a bundled `rg` binary so this download path is a last-resort fallback (system PATH → cached → download). Do NOT re-flag the missing checksum verification in this file until the dedicated follow-up PR lands.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-28T06:51:58.496Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/todo.ts:9-18
Timestamp: 2026-04-28T06:51:58.496Z
Learning: In `packages/opencode/src/tool/todo.ts` (Astro-Han/pawwork), `TodoItem.status` and `TodoItem.priority` are intentionally declared as plain `Schema.String` rather than closed literal unions. This matches the upstream opencode baseline (`dev:packages/opencode/src/tool/todo.ts`). The tightening — `Schema.Literals(["pending","in_progress","completed","cancelled"])` for `status` and `Schema.Literals(["high","medium","low"])` for `priority` — is tracked as a follow-up under the harness/tool-set-v1 series (issue `#129`) to land either as part of a tool-schema tightening sweep or upstream-first. Do NOT re-flag the free-form strings for these fields in upstream-sync PRs; flag it only in a PawWork-authored PR or the dedicated sweep that touches `TodoItem` schema.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-28T04:56:25.162Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/tool.ts:34-42
Timestamp: 2026-04-28T04:56:25.162Z
Learning: In `packages/opencode/src/tool/tool.ts` (Astro-Han/pawwork, PR `#270`), the `Def.execute` signature uses `ctx: Context` (without the `M` generic) instead of `ctx: Context<M>`, meaning `ctx.metadata(...)` is not checked against the same `M` as `ExecuteResult<M>`. This is inherited upstream behavior adopted via the graft strategy in PR `#270` (an upstream-sync PR per `project_upstream_strategy.md`). Fixing it here would mix refactor + bugfix intents and drift the diff from the upstream baseline. The fix is deferred to a dedicated follow-up PR or upstream report. Do NOT re-flag the missing `Context<M>` generic in `Def.execute` during upstream-sync PRs; only flag it in a PawWork-authored PR that directly touches `tool.ts` type signatures.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-22T09:32:58.310Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:58.310Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-28T04:38:10.087Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/patch/index.ts:337-346
Timestamp: 2026-04-28T04:38:10.087Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/patch/index.ts`), the BOM-transition surfacing gap — where `Bom.split` strips BOM before building `unified_diff`/`new_content` but `Bom.join` later re-attaches BOM on disk write, meaning BOM changes are not reflected in the diff payload — is intentionally deferred. PR `#270` is an upstream-sync graft; fixing the issue here would mix refactor + bugfix intents and drift the diff from the upstream baseline needed for clean future grafts. A dedicated follow-up PR (or upstream report) will address this. Do NOT re-flag the missing BOM-change surfacing in `ApplyPatchFileUpdate`/`ApplyPatchFileChange` as a blocking issue in PR `#270` or in future sync PRs that carry the same upstream baseline.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-24T17:08:46.780Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:46.780Z
Learning: In Astro-Han/pawwork PR `#224`, the first-occurrence `PawWork 爪印` branding rule originally specified in issue `#196` was superseded by an updated Chinese-branding spec. On all zh UI surfaces in `packages/app/src/i18n/zh.ts` (e.g., `dialog.model.unpaid.freeModels.title`, `session.new.subtitle`, `sidebar.gettingStarted.line1`), the correct and intentional target is fully localized `爪印` branding — no `PawWork` prefix. Do NOT flag these strings as missing the first-occurrence `PawWork 爪印` rule in future reviews.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-28T04:38:15.483Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:15.483Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-20T17:03:40.214Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 73
File: packages/opencode/src/cli/cmd/tui/context/sync.tsx:486-489
Timestamp: 2026-04-20T17:03:40.214Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/cli/cmd/tui/context/sync.tsx`), `sync.ready` returning `true` when `process.env.OPENCODE_FAST_BOOT` is set is intentional. The plugin-facing data properties `state.config` (initialized to `{}`) and `state.provider` (initialized to `[]`) expose safe-empty defaults, so they are safe to access before bootstrap completes. Do not flag these as needing null-guards or conditional patterns to match `vcs` — the difference is intentional because `vcs` starts as `undefined` while the others have initialized defaults. Changing this would alter the plugin API contract without a concrete failing case.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-25T12:52:35.631Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/desktop-electron/src/main/ipc.ts:238-263
Timestamp: 2026-04-25T12:52:35.631Z
Learning: In Astro-Han/pawwork (`packages/desktop-electron/src/main/ipc.ts`), `deps.getServerReadyData()` (backed by `serverReady.promise` in `index.ts`) resolves once at server startup and remains settled; it is not expected to reject in practice. Do not flag the absence of a try-catch around it in the `export-session` IPC handler — the network/fetch layer in `server-client.ts` already has a 10-second AbortController timeout and returns a typed `{ok: false, error}` payload, covering the real failure modes.
Applied to files:
packages/app/src/components/settings-worktrees.tsx
📚 Learning: 2026-04-28T08:14:35.518Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/edit.ts:48-48
Timestamp: 2026-04-28T08:14:35.518Z
Learning: In `packages/opencode/src/tool/edit.ts` (Astro-Han/pawwork), the `filePath` schema description (`"The absolute path to the file to modify"`) is upstream-inherited from `dev:packages/opencode/src/tool/edit.ts:48`. The runtime actually accepts relative paths (resolved via `Instance.directory` at lines 79-81), but the description fix is intentionally deferred to a single PawWork-authored description-cleanup PR that will also cover the identical mismatch in `packages/opencode/src/tool/write.ts:24`. Do NOT re-flag the too-narrow `filePath` description in upstream-sync PRs; flag it only in the dedicated description-cleanup PR.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-04-27T10:33:02.677Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/diagnostics.ts:245-252
Timestamp: 2026-04-27T10:33:02.677Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/diagnostics.ts`, PR `#264`), `loopLastError` is stored as raw (unscrubbed) error text in loop metadata intentionally. The trust boundary for sensitive data scrubbing is the renderer (`LoopRenderer.render` already applies `scrubErrorText` before any user/model-facing output), not the storage layer. Failed tool parts also already store raw errors in `state.error` (PR `#204` default behavior), so scrubbing only `loopLastError` would not close that pre-existing leak and would add lossy storage. If export-side hardening is needed, the correct approach is a single sanitizer pass over both `state.error` and metadata in the export layer. Do NOT suggest scrubbing `loopLastError` at the `observeToolError` call site.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-04-21T16:57:25.580Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 102
File: packages/opencode/src/config/agent.ts:108-119
Timestamp: 2026-04-21T16:57:25.580Z
Learning: In `packages/opencode/src/config/agent.ts` (Astro-Han/pawwork), `ConfigPermission.Info` only accepts permission objects or the three action strings `"ask"`, `"allow"`, `"deny"`, and transforms those action strings into `{ "*": action }` before `normalize()` runs. By the time `normalize()` is reached, `configuredPermission` is always either `undefined` or a `Record<string, Rule>` — never a raw arbitrary string. The `Object.assign(permission, configuredPermission)` pattern is therefore safe. Do not flag it as corrupting string permission references.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-04-27T08:58:05.265Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:531-538
Timestamp: 2026-04-27T08:58:05.265Z
Learning: In `packages/opencode/src/session/prompt.ts` (and the broader Astro-Han/pawwork codebase), `yield* new ErrorClass()` is ONLY valid for classes that extend `Schema.TaggedErrorClass` (which implements Effect's Yieldable interface). Plain `Error` subclasses (e.g. `BlockedLoopError`, `LoopStopError` in prompt.ts, or inline `new Error(...)` instances) are NOT yieldable and MUST be wrapped: `yield* Effect.fail(new PlainError(...))`. Do NOT suggest replacing `yield* Effect.fail(new SomePlainError(...))` with `yield* new SomePlainError(...)` unless the class extends `Schema.TaggedErrorClass`.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.ts
📚 Learning: 2026-04-28T14:46:45.226Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 302
File: packages/opencode/test/session/retry.test.ts:298-315
Timestamp: 2026-04-28T14:46:45.226Z
Learning: In Astro-Han/pawwork (PR `#302`, `packages/opencode/src/provider/error.ts` + `packages/opencode/test/session/retry.test.ts`), upstream anomalyco/opencode#24063 intentionally treats ALL OpenAI streaming `server_error` chunks as retryable (`isRetryable: true`). Do NOT request a test asserting `isRetryable: false` for a `server_error` stream chunk; the only negative regression needed is a non-`server_error` code (e.g. `bad_request`) to confirm the retry gate stays closed for other error codes.
Applied to files:
packages/app/src/components/settings-worktrees-helpers.ts
🔇 Additional comments (1)
packages/app/src/components/settings-worktrees-helpers.test.ts (1)
4-13: Good coverage for theerrorTextedge cases.These tests lock in the circular-input and empty-object behavior that was previously easy to regress.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/components/settings-worktrees.tsx`:
- Around line 86-104: The current deleting signal (deleting and setDeleting)
only stores one directory so concurrent handleDelete calls overwrite it and a
later finally can clear another request’s loading state; change deleting to
track multiple in-flight directories (e.g., use a Set<string> or array) and
update setDeleting usage in handleDelete, adding the directory at start and
removing it in finally instead of replacing/clearing the single string; adjust
any UI checks that read deleting (e.g., checks like deleting() === directory) to
test membership (e.g., deletingSet.has(directory) or includes) so each row’s
loading state is independently managed while reusing the existing handleDelete,
setDeleting and deleting symbols.
In `@packages/opencode/src/session/session.ts`:
- Around line 769-797: The loop in Session.findActiveWorktreeBinding calls
fromRow(row) without providing a project fallback, so parseExecutionContext may
use row.directory instead of the true git project root; update the call inside
findActiveWorktreeBinding to pass the project as the ProjectFallback (i.e.,
supply Instance.project or equivalent) into fromRow(row) so hydration and
parseExecutionContext get the correct project context when reconstructing
executionContext and activeWorktree.
🪄 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: 22a41c79-28c7-41b9-b91b-c2847262de5b
📒 Files selected for processing (9)
packages/app/src/components/settings-worktrees.tsxpackages/opencode/src/session/session.tspackages/opencode/src/worktree/gitignore-guard.tspackages/opencode/src/worktree/index.tspackages/opencode/test/project/worktree-remove.test.tspackages/opencode/test/session/session.test.tspackages/opencode/test/session/state-machine-guard.test.tspackages/opencode/test/worktree/gitignore-guard.test.tspackages/ui/src/components/message-part.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/opencode/src/session/session.ts (1)
769-797:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegressed:
findActiveWorktreeBinding()still hydrates rows without project fallback.This lookup now filters in SQL, but it still does
fromRow(row)instead of the sameProjectTable-backed fallback used byget(),children(),list(), andlistGlobal(). If a non-null but invalidexecution_contextrow slips through,parseExecutionContext()falls back torow.directoryinstead of the git project root, so this guard can miss a real active worktree binding.Suggested fix
const rows = yield* db((d) => d .select() .from(SessionTable) .where( @@ ) .all(), ) + const ids = [...new Set(rows.filter(needsProjectFallback).map((row) => row.project_id))] + const projects = new Map<string, ProjectFallback>() + if (ids.length > 0) { + const items = yield* db((d) => + d + .select({ id: ProjectTable.id, worktree: ProjectTable.worktree, vcs: ProjectTable.vcs }) + .from(ProjectTable) + .where(inArray(ProjectTable.id, ids)) + .all(), + ) + for (const item of items) projects.set(item.id, item) + } for (const row of rows) { - const session = fromRow(row) + const session = fromRow(row, projects.get(row.project_id)) const exec = session.executionContext if (canonicalDirectory(exec.activeDirectory) === canonicalDirectory(exec.ownerDirectory)) continue🤖 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 769 - 797, findActiveWorktreeBinding currently calls fromRow(row) which can hydrate executionContext without the ProjectTable fallback and let parseExecutionContext fall back to row.directory (missing the git project root), so change the hydration to use the same ProjectTable-backed fallback used by get(), children(), list(), and listGlobal(): instead of fromRow(row) hydrate the session row using the project-aware helper (the same code path those functions use — e.g., the project-backed fromRow/getSessionWithProjectFallback helper or equivalent that consults ProjectTable and parseExecutionContext), so parseExecutionContext will fall back to the project root and the active-worktree guard cannot be bypassed by an invalid execution_context.
🤖 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/worktree/index.ts`:
- Around line 303-315: registerExistingByPath currently persists any directory
even if it isn’t an attached worktree; before calling upsertRegistry verify the
canonicalized target is listed by this repo’s git worktrees by running git
worktree list --porcelain (for the current Instance.worktree) and matching the
target path; if the target is not present, throw/return an error and do not call
upsertRegistry. Update registerExistingByPath (which calls canonical,
lookupByDirectory, git(...), and upsertRegistry) to perform this validation and
fail safely to avoid creating registry-only entries that Worktree.remove might
later treat as removable and pass to cleanDirectory.
In `@packages/opencode/test/session/session.test.ts`:
- Around line 208-225: The test "backfills legacy null executionContext rows" is
asserting against raw tmp.path but the code under test stores canonicalized
paths; change the assertions to compare against a canonicalized path by calling
canonicalDirectory(tmp.path) (await it) and use that returned value (e.g.,
canonicalPath) in the two expects that check execution_context.ownerDirectory
and execution_context.activeDirectory, and update any later legacy-read
assertions in the same test that currently expect tmp.path verbatim to use the
same canonicalDirectory(tmp.path) result; reference
SessionNs.backfillExecutionContext and the test block name to locate where to
replace tmp.path with the canonicalized value.
In `@packages/opencode/test/session/state-machine-guard.test.ts`:
- Around line 1-77: Tests are using raw test(...) + Effect.runPromise(...)
instead of the repo test harness; change them to the Effect harness by importing
testEffect from "test/lib/effect.ts", adding const it = testEffect(import.meta),
and replace each test(...) with it(...) whose body is an Effect.gen(function* ()
{ ... }) generator; inside the generator, call hasInFlightToolCallsExcept and
hasRunningSubagents via yield* or Effect.call/yield patterns instead of
Effect.runPromise, and adapt assertions to use Effect assertions (or yield
results and assert with expect) while keeping helper factories
sessionsWithParts, toolPart, and subagents unchanged and referencing those
symbols in the new tests.
In `@packages/ui/src/components/message-part.tsx`:
- Around line 795-798: toolStateError currently drops non-string errors so
ToolErrorCard never renders for Error objects or structured payloads; update
toolStateError to normalize any error into a user-readable string: if
state.error is a string return it, else if state.error is an Error return
state.error.message, else if it's an object return JSON.stringify(state.error)
(or a safe fallback) so ToolErrorCard (referenced where toolStateError is used)
receives a string and the error UI displays for all error shapes.
---
Duplicate comments:
In `@packages/opencode/src/session/session.ts`:
- Around line 769-797: findActiveWorktreeBinding currently calls fromRow(row)
which can hydrate executionContext without the ProjectTable fallback and let
parseExecutionContext fall back to row.directory (missing the git project root),
so change the hydration to use the same ProjectTable-backed fallback used by
get(), children(), list(), and listGlobal(): instead of fromRow(row) hydrate the
session row using the project-aware helper (the same code path those functions
use — e.g., the project-backed fromRow/getSessionWithProjectFallback helper or
equivalent that consults ProjectTable and parseExecutionContext), so
parseExecutionContext will fall back to the project root and the active-worktree
guard cannot be bypassed by an invalid execution_context.
🪄 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: 5c0d7d66-b3d5-442f-8f22-b02f96efd82a
📒 Files selected for processing (9)
packages/app/src/components/settings-worktrees.tsxpackages/opencode/src/session/session.tspackages/opencode/src/worktree/gitignore-guard.tspackages/opencode/src/worktree/index.tspackages/opencode/test/project/worktree-remove.test.tspackages/opencode/test/session/session.test.tspackages/opencode/test/session/state-machine-guard.test.tspackages/opencode/test/worktree/gitignore-guard.test.tspackages/ui/src/components/message-part.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
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)
19-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winScope
waitReady()to the temp repo.This resolves on the first global
worktree.readyevent, regardless of which test emitted it. Since bootstrap runs in the background, another worktree finishing at the same time can satisfy this promise and make the assertions inspect the wrong instance. Filter onevt.directoryfor the current temp root (or pass an expected directory/name intowaitReady).🤖 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 19 - 36, waitReady currently listens to GlobalBus "event" and resolves on the first Worktree.Event.Ready event globally, causing race failures; update waitReady to scope to the current temp repo by filtering the incoming evt for evt.payload.type === Worktree.Event.Ready.type AND evt.directory (or evt.payload.properties.name/branch) matching the expected temp root (pass the expected directory/name into waitReady or capture it from the test setup), only then clear the timer, remove the listener (GlobalBus.off("event", on)) and resolve; reference the waitReady function, GlobalBus, Worktree.Event.Ready, evt.directory and the test's temp root variable when implementing the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/components/settings-worktrees.tsx`:
- Around line 28-39: The Promise.allSettled branch currently treats any
fulfilled response from sdk.client.worktree.list(...) as success even when the
SDK returned an error payload; update the mapping passed to
projectRoots().map(...) so each async function inspects res.error and, if
present, throws (e.g., throw new Error(errorText(res.error))) so that
Promise.allSettled records it as rejected; keep the successful path returning
(res.data ?? []).map(... as WorktreeInfo) so downstream logic that checks
results for rejected entries and uses errorText(failed?.reason) behaves
correctly.
In `@packages/opencode/src/session/session.ts`:
- Around line 241-246: UpdateExecutionContextInput and
FindActiveWorktreeBindingInput must reject empty or relative paths at the API
boundary because those flow into canonicalDirectory(), which resolves relative
paths against process.cwd() and can cause wrong persisted roots; update the zod
schemas to validate that activeDirectory (and any directory field on
ActiveWorktree if present) is a non-empty string and an absolute path (use
Node's path.isAbsolute in a zod.refine) and change
FindActiveWorktreeBindingInput from plain z.string() to a refined string that is
non-empty and absolute; ensure error messages are clear so callers know the
input must be an absolute path.
In `@packages/opencode/src/worktree/index.ts`:
- Around line 239-257: writeRegistry currently updates ProjectTable.sandboxes
directly but never emits the updated project row, leaving subscribers with stale
data; change writeRegistry (the Effect.fnUntraced function) so that after the
db.update completes you perform a follow-up Database.use select on ProjectTable
where id === Instance.project.id to fetch the updated project row, then invoke
the existing project-update broadcaster used by
Project.addSandbox/Project.removeSandbox (or the same code path those methods
use to emit project updates) with the newly selected row so subscribers receive
the updated project state.
- Around line 217-229: The asInfo helper currently maps legacy string rows to
source: "existing", which breaks lookupBySlug/EnterWorktree reuse; update asInfo
so that when entry is a bare string it passes source: "managed" into Info.parse,
and likewise default entry.source to "managed" when undefined in the object
branch; this preserves managed semantics for legacy sandboxes so
lookupBySlug(...) and EnterWorktree(name=...) will correctly match existing
managed worktrees (see asInfo, Info.parse, lookupBySlug, EnterWorktree).
In `@packages/opencode/test/session/session.test.ts`:
- Around line 283-333: The tests currently only exercise SessionNs.get() for
legacy/invalid execution_context fallbacks; add equivalent assertions that the
project-root fallback is applied when reading sessions via SessionNs.list() and
SessionNs.listGlobal(): create sessions with execution_context set to null and
to an invalid object (as in the existing tests), then call SessionNs.list(...)
and SessionNs.listGlobal(...) and assert each returned session's
executionContext.ownerDirectory and executionContext.activeDirectory equal the
canonical project root (use the same tmpdir/subdir setup and
Database.update(...) mutation patterns and SessionNs.remove(...) cleanup),
ensuring the fallback logic in list() and listGlobal() is pinned.
---
Outside diff comments:
In `@packages/opencode/test/project/worktree.test.ts`:
- Around line 19-36: waitReady currently listens to GlobalBus "event" and
resolves on the first Worktree.Event.Ready event globally, causing race
failures; update waitReady to scope to the current temp repo by filtering the
incoming evt for evt.payload.type === Worktree.Event.Ready.type AND
evt.directory (or evt.payload.properties.name/branch) matching the expected temp
root (pass the expected directory/name into waitReady or capture it from the
test setup), only then clear the timer, remove the listener
(GlobalBus.off("event", on)) and resolve; reference the waitReady function,
GlobalBus, Worktree.Event.Ready, evt.directory and the test's temp root variable
when implementing 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: 29f0b000-4d35-48f7-b5c7-779907e1a649
📒 Files selected for processing (7)
packages/app/src/components/settings-worktrees.tsxpackages/opencode/src/session/session.tspackages/opencode/src/worktree/index.tspackages/opencode/test/project/worktree.test.tspackages/opencode/test/session/session.test.tspackages/opencode/test/session/state-machine-guard.test.tspackages/ui/src/components/message-part.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opencode/src/worktree/index.ts (1)
547-554:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFail closed when the target is missing from
git worktree list.If
locateWorktree(...)misses, Line 548-552 still treats any existing directory as safe to recursively delete. A bad SDK/API call can therefore pointWorktree.remove()at an arbitrary directory and this service will wipe it even though it was never tracked as a worktree. Please require a registry hit before any filesystem cleanup here, and ideally avoid auto-deleting stalesource === "existing"entries that are no longer attached.Suggested guard
if (!entry?.path) { + const tracked = yield* lookupByDirectory(directory) + if (!tracked) { + throw new RemoveFailedError({ message: "Worktree not found" }) + } const directoryExists = yield* fs.exists(directory).pipe(Effect.orDie) - if (directoryExists) { + if (directoryExists && tracked.source === "created") { yield* stopFsmonitor(directory) yield* cleanDirectory(directory) } yield* removeRegistry(input.directory).pipe(Effect.catch(() => Effect.void)) return true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/worktree/index.ts` around lines 547 - 554, The current block deletes an on-disk directory if it exists even when locateWorktree() didn't find a tracked worktree; change the logic in Worktree.remove / the block around locateWorktree so you first query the registry for an entry for input.directory (use the existing removeRegistry and/or a dedicated registry lookup function) and only perform stopFsmonitor(directory) and cleanDirectory(directory) if the registry confirms this directory was a tracked worktree; if the registry entry exists but has source === "existing" do not auto-delete the filesystem (only remove or update the registry), and keep the removeRegistry(...).pipe(Effect.catch(...)) behavior to tolerate registry errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/components/settings-worktrees.tsx`:
- Around line 90-97: The deletion handler allows races and clears the wrong row:
update handleDelete to early-return if the directory is already being deleted
(prevent duplicate requests), add/remove the directory from the deleting Set
using functional updates (e.g., setDeleting(prev => { const s = new Set(prev);
s.add(directory); return s }) and in finally remove it similarly), and only
clear the confirmation for the specific row (i.e., call setConfirming(undefined)
only if the current confirming value equals the directory you just deleted).
Also wrap the SDK call in try/finally so the deleting Set is always cleaned up
and use the same directory variable when calling sdk.client.worktree.remove and
refetch to ensure scoping.
In `@packages/opencode/src/session/session.ts`:
- Around line 61-66: parseExecutionContext currently discards the entire
execution_context when safeParse fails, which loses valid
ownerDirectory/activeDirectory; update parseExecutionContext to preserve
directory fields and only clear or repair the malformed activeWorktree instead
of falling back to legacyExecutionContext: call
SessionExecutionContext.safeParse(row.execution_context) and if it fails,
attempt to extract and validate ownerDirectory and activeDirectory from
row.execution_context (or keep existing row.ownerDirectory/activeDirectory) and
then validate activeWorktree shape (directory, name, branch, source) — if
activeWorktree is malformed set it to null/undefined (or a safe default) but
keep the rest of the parsed context; alternatively implement a recovery path
that normalizes activeWorktree and returns the merged object so only
activeWorktree is reset while preserving other fields (also consider adding the
same repair logic in the backfill path that writes NULLs).
In `@packages/opencode/test/project/worktree-remove.test.ts`:
- Around line 15-57: The test currently uses unixtest which skips Windows,
leaving the session-binding removal guard untested on Windows; change
unixtest("refuses to remove a worktree bound to an active session", ...) to a
cross-platform test (e.g., test("refuses to remove a worktree bound to an active
session", ...)) so the code path that exercises Instance.provide,
Worktree.remove and Session.updateExecutionContext runs on Windows as well;
ensure no UNIX-only helpers remain in the body and keep the same assertions and
cleanup.
In `@packages/opencode/test/project/worktree.test.ts`:
- Around line 182-183: Worktree.create() returns before the worktree is fully
booted, so replace the fixed Bun.sleep(1000) with a readiness synchronization
call: after creating the worktree with withInstance(..., () => Worktree.create({
name: "feature-a" })), call await waitReady(created) (or the existing waitReady
helper) to block until the created worktree is fully ready before proceeding to
operations like remove(); this ensures deterministic tests on CI/Windows.
In `@packages/opencode/test/session/prompt-effect.test.ts`:
- Around line 1383-1390: The test currently silently returns when no completed
bash tool part is found (if (!tool) return), allowing false positives; replace
that early return with a failing assertion so the test fails when no
CompletedToolPart with type === "tool", tool === "bash", and state.status ===
"completed" is found after MessageV2.filterCompactedEffect(chat.id) yields msgs
(i.e., assert that tool is defined or throw an error with a clear message
referencing the missing bash CompletedToolPart).
In `@packages/opencode/test/session/session.test.ts`:
- Around line 317-351: Extend the existing test (the "synthesizes invalid
executionContext from the project root on read" scenario) to also cover the
backward-compat regression where a DB row has a valid ownerDirectory and
activeDirectory but activeWorktree is present without its required source field;
after creating the session via SessionNs.create and injecting the malformed DB
row using Database.update(SessionTable).set({ execution_context: {
ownerDirectory: ..., activeDirectory: ..., activeWorktree: { directory: ...,
name: ..., branch: ... } } }), verify that SessionNs.get(session.id),
SessionNs.list() and SessionNs.listGlobal() all normalize the executionContext
so activeWorktree is either fixed/removed and the activeDirectory/ownerDirectory
remain correct (use canonicalDirectory(tmp.path) as expectedRoot) and then
remove the session with SessionNs.remove(session.id).
---
Outside diff comments:
In `@packages/opencode/src/worktree/index.ts`:
- Around line 547-554: The current block deletes an on-disk directory if it
exists even when locateWorktree() didn't find a tracked worktree; change the
logic in Worktree.remove / the block around locateWorktree so you first query
the registry for an entry for input.directory (use the existing removeRegistry
and/or a dedicated registry lookup function) and only perform
stopFsmonitor(directory) and cleanDirectory(directory) if the registry confirms
this directory was a tracked worktree; if the registry entry exists but has
source === "existing" do not auto-delete the filesystem (only remove or update
the registry), and keep the removeRegistry(...).pipe(Effect.catch(...)) behavior
to tolerate registry errors.
🪄 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: a1f166fc-a293-4337-abd1-2426aaa247f4
📒 Files selected for processing (8)
packages/app/src/components/settings-worktrees.tsxpackages/opencode/src/session/prompt.tspackages/opencode/src/session/session.tspackages/opencode/src/worktree/index.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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
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