Scope git checkout effects and update Effect/Node dependency set - #148
Conversation
- run `gitCheckout` via `Effect.scoped` to satisfy new `Scope` requirement - update `GitCore.checkoutBranch` service signature to include `Scope.Scope` - upgrade Effect catalog refs and `@types/node` to latest pinned versions
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Scoped checkout immediately interrupts background upstream refresh
- Changed Effect.forkScoped to Effect.forkDetach so the background upstream refresh fiber runs independently of the enclosing scope, and removed the now-unnecessary Effect.scoped wrapper and Scope.Scope type requirement.
Or push these changes by commenting:
@cursor push de7f50ec6b
Preview (de7f50ec6b)
diff --git a/apps/server/src/git/Layers/GitCore.ts b/apps/server/src/git/Layers/GitCore.ts
--- a/apps/server/src/git/Layers/GitCore.ts
+++ b/apps/server/src/git/Layers/GitCore.ts
@@ -1161,7 +1161,7 @@
});
// Refresh upstream refs in the background so checkout remains responsive.
- yield* Effect.forkScoped(
+ yield* Effect.forkDetach(
refreshCheckedOutBranchUpstream(input.cwd).pipe(Effect.catch(() => Effect.void)),
);
});
diff --git a/apps/server/src/git/Services/GitCore.ts b/apps/server/src/git/Services/GitCore.ts
--- a/apps/server/src/git/Services/GitCore.ts
+++ b/apps/server/src/git/Services/GitCore.ts
@@ -7,7 +7,7 @@
* @module GitCore
*/
import { ServiceMap } from "effect";
-import type { Effect, Scope } from "effect";
+import type { Effect } from "effect";
import type {
GitCheckoutInput,
GitCreateBranchInput,
@@ -151,7 +151,7 @@
*/
readonly checkoutBranch: (
input: GitCheckoutInput,
- ) => Effect.Effect<void, GitCommandError, Scope.Scope>;
+ ) => Effect.Effect<void, GitCommandError>;
/**
* Initialize a repository in the provided directory.
diff --git a/apps/server/src/wsServer.ts b/apps/server/src/wsServer.ts
--- a/apps/server/src/wsServer.ts
+++ b/apps/server/src/wsServer.ts
@@ -689,7 +689,7 @@
case WS_METHODS.gitCheckout: {
const body = stripRequestTag(request.body);
- return yield* Effect.scoped(git.checkoutBranch(body));
+ return yield* git.checkoutBranch(body);
}
case WS_METHODS.gitInit: {| case WS_METHODS.gitCheckout: { | ||
| const body = stripRequestTag(request.body); | ||
| return yield* git.checkoutBranch(body); | ||
| return yield* Effect.scoped(git.checkoutBranch(body)); |
There was a problem hiding this comment.
Scoped checkout immediately interrupts background upstream refresh
High Severity
Wrapping git.checkoutBranch(body) with Effect.scoped(...) creates a short-lived scope that closes as soon as the checkout effect completes. Inside checkoutBranch, Effect.forkScoped is used to fork a background fiber for refreshCheckedOutBranchUpstream, which performs a network git fetch. When Effect.scoped closes the scope immediately after checkout, it interrupts the forked fiber — effectively killing the background refresh before the fetch can complete. The comment "Refresh upstream refs in the background so checkout remains responsive" describes the intended behavior, but this change silently breaks it.
Additional Locations (1)
| "turn/start", | ||
| new Error(`Invalid attachment id '${attachment.id}'.`), | ||
| ), | ||
| return yield* toRequestError( |
There was a problem hiding this comment.
🔴 Critical Layers/CodexAdapter.ts:492
yield* toRequestError(...) yields the error object directly, but the rest of this file uses Effect.fail(error) to fail Effects. Consider wrapping with Effect.fail() for consistency and to avoid a potential TypeError if the error isn't iterable.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/CodexAdapter.ts around line 492:
`yield* toRequestError(...)` yields the error object directly, but the rest of this file uses `Effect.fail(error)` to fail Effects. Consider wrapping with `Effect.fail()` for consistency and to avoid a potential `TypeError` if the error isn't iterable.
Evidence trail:
apps/server/src/provider/Layers/CodexAdapter.ts lines 492-495 (yield* toRequestError usage), lines 73-88 (toRequestError definition returns ProviderAdapterError), lines 460 and 538 (Effect.fail pattern elsewhere), apps/server/src/provider/Errors.ts lines 57-68 (ProviderAdapterRequestError is a Schema.TaggedErrorClass, not an Effect)
Merges `upstream/main` at `02297e3db` into the fork, 35 commits from base `0f602b337`. Merge commit, not a rebase. Tracker entry: `docs/fork/upstream-merge-log.md`, 2026-09-11. `170` files landed against `166` in the upstream range; fork delta `756` files. The gap is six named files and reconciles: `ThreadStatusIndicators.test.tsx` and `sandboxControl.placement.test.tsx` landed as fork-test fixes amended into the merge, the three fork documents landed with it, and `PreviewLocalServerCard.tsx` was re-deleted per the inventory's deliberate-deletion list. `duplicate-adds.mjs` and `resolution-check.mjs` are clean — nothing landed as one side whole. ## What upstream shipped, and where it stands on Moatless ### Usable as-is These run on the fork's backend with no further work. - **A compact right-panel surface menu** (pingdotgg#11111) — the add-surface launcher goes from a card grid to keyboard-shortcut rows. This is where the fork's sandbox status badge lives, so the badge was re-stated on upstream's row rather than replayed; see the conflict notes below. - **Multiple-linked-PR badges, simplified** (pingdotgg#11104, pingdotgg#11180, pingdotgg#11101) — a thread with several links now shows the total linked count coloured by aggregate status, instead of naming a primary and counting the extras. This is **live on Moatless**: the backend serves `thread.pullRequests` and reports `threadPullRequests`, so the badge resolves. It is also a behaviour change a user will see, and it is what broke the fork's own badge test — the only thing that caught it. - **Settled threads recede in the sidebar** (pingdotgg#11101) — a settled row dims until hover or focus. Rides the settlement state Moatless already serves. - **Environments in the command palette** (pingdotgg#10722) — searching now returns environments beside threads and projects, with a subtitle. - **A blue/orange diff palette** (pingdotgg#10671) — client-side theme only. - **Question answers folded into tool activity** (pingdotgg#11014) — the chat timeline renders an answer to an agent's async question inside the tool call that asked it, rather than as a separate turn. The client half (`client-runtime/src/work-log/userInput.ts`, `shared/src/toolActivity.ts`, `MessagesTimeline.tsx`) works off data Moatless already sends. The server half is in the third bucket. - **Unpriced model activity is flagged** (pingdotgg#11021) — usage rows with tokens and no price read as unpriced instead of `$0.00`. Shared merge logic over the `server.getUsageSummary` the backend serves. - **Return to picture-in-picture when the right panel closes** (pingdotgg#11102) and **aligned floating-preview corners** (pingdotgg#10915) — both land in the hosted preview surface and both were taken; the fork's framed-runtime condition in `ThreadPreviewMiniPlayer.tsx` still covers the case upstream's `framed` check does not. - **Collapse a tool call by clicking its expanded label** (pingdotgg#11017), **provider update text fitting inside sidebar notices** (pingdotgg#11034), **no seams in the topbar scroll fade** (pingdotgg#10914), **centered PR unavailable states** (pingdotgg#11110), **no sidebar PR link icon** (pingdotgg#11179). - **Mobile** (pingdotgg#11128, pingdotgg#11127, pingdotgg#11114, pingdotgg#11115, pingdotgg#11118 / pingdotgg#11079 / reverted in pingdotgg#11098, pingdotgg#11113) — shared markdown renderer rename, composer transition and final-frame fixes, tablet close controls for files and terminal, and playback preserved across fullscreen transitions. ### Unsupported in Moatless / needs implementation - **The device hub** — the whole of pingdotgg#10677, pingdotgg#10854, pingdotgg#10855 and pingdotgg#10856: iOS simulators and Android emulators a person and an agent can share. Server side is `apps/server/src/device/` (`LocalDeviceHost.ts` drives the machine the server runs on, `SshDeviceHost.ts` drives another over SSH, `DeviceHubProxy.ts` fronts the video and accessibility streams) plus an MCP toolkit at `apps/server/src/mcp/toolkits/device/` that gives the agent tap/type/screenshot. Client side is a `device` right-panel surface (`apps/web/src/components/device/`) and a Device hosts settings page. Eight methods plus one stream, all newly declaring `UnsupportedMethodError` in `packages/contracts/src/rpc.ts`: `device.configure`, `device.list`, `device.testHost`, `device.open`, `device.close`, `device.shutdown`, `device.detail`, `device.action`, and the `subscribeDeviceState` push stream. **Nothing gates the surface on a capability.** `ChatView.tsx` passes `deviceAvailable={activeThreadRef !== null}`, so the launcher offers a Device row on every thread. `subscribeDeviceState` never resolves on Moatless, so the state stays empty, `onboardingCompleted` is false, and clicking the row opens `DeviceSetup` rather than the panel — whose first step, "Enable the device hub", calls `device.configure` and shows the refusal. A dead end a person can walk into. One additive `FEATURES.deviceHub` read on the two `deviceAvailable` props would drop the row instead; **this merge did not add it**, and it is recorded as the open work in `docs/fork/gaps.md` under _The device hub_. Implementing it on the backend is a real question rather than a stub: the hub needs Xcode or the Android SDK on whatever host it drives, and its stream is a second connection beside the RPC one. - **Label and reviewer updates without redundant reloads** (pingdotgg#11117) — optimistic cache writes in `client-runtime/src/state/pullRequests.ts` over `pullRequests.update`. Rides the `pullRequests.*` group, which Moatless does not serve and which the `pullRequests` capability already keeps off, so it changes nothing here until that group lands. - **Emphasised primary PR actions** (pingdotgg#11105) and **save a PR body with Cmd/Ctrl+Enter** (pingdotgg#10660) — same surface, same condition. - **Zed remote links accepting root paths and Windows servers** (pingdotgg#11044) — builds an SSH open target the Electron shell hands to a local editor. That path needs a desktop shell, so it does not reach this fork's browser client; `shell.openInEditor` remains unsupported. Not a fork target, listed for completeness. - **Marketing** (pingdotgg#11146, pingdotgg#11145) — `apps/marketing` is upstream's own site; inherited and inert here. ### Backend behavior to consider reproducing in Moatless Server-side fixes upstream made to its own runtime. Moatless implements the same contract, so each is worth checking against its own implementation. - **Claude launch args override the derived permission mode** (pingdotgg#11026, `apps/server/src/provider/Layers/ClaudeAdapter.ts`). Upstream derives a permission mode from the thread's settings and then appends the agent's launch args; an explicit `--permission-mode` in those args used to be overridden by the derived one instead of winning. If Moatless derives a permission mode the same way, a user who set the flag explicitly is being ignored in the same place. Cheapest of the three to check. - **Project identity resolved before legacy PR relinks** (pingdotgg#11045, `apps/server/src/orchestration/Layers/OrchestrationEngine.ts`). A legacy pull-request link is relinked to its thread on load; upstream now resolves the project's repository identity first, so a relink cannot bind a link to the wrong project when two projects share a branch name. The fork fills `thread.pullRequests` from a Task's GitHub bindings, so it has the same ordering question: the binding has to know which project it belongs to before it is attached. - **Question answers in the published activity payload** (pingdotgg#11014, `apps/server/src/orchestration/ActivityPayloadProjection.ts`). The projection now folds `projectQuestionToolInput` into the activity payload for both `mcp_tool_call` and plain tool items, which is what lets a client render the answer inside the tool call. Moatless does not report `agentActivityPublishing`, so nothing reads this today — but if it ever publishes activity, this is the shape to publish. ## Conflicts and how they were resolved 11 conflicted files, resolved on the verdicts `preflight.mjs` printed. Additive on both sides, both kept: `MessagesTimeline.tsx` (imports), `client-runtime/src/rpc/client.ts` (the fork's four subscription tags against upstream's `subscribeDeviceState`), `rightPanelStore.ts` (the fork's `sandbox` kind against upstream's `device`), `rpc.ts` and `RpcAuthorization.ts` (the fork's thread-server / sandbox / subtasks methods and scopes against upstream's eight `device.*` ones), and `ChatView.tsx` (both right-panel arms, both `onAdd*` props at the inline and sheet call sites, and upstream's extended `closePreviewPanel` under the fork's proactive preview-open effect). `PreviewEmptyState.tsx` and `ThreadPreviewMiniPlayer.tsx` took upstream's `DiscoveryList` and `rounded-[inherit]` with the fork's sandbox-read error line and framed `hasPreviewSurface` condition re-stated on top. `PreviewLocalServerCard.tsx` was a modify/delete conflict and was re-deleted. `pnpm-lock.yaml` auto-merged this time, so `--theirs` had nothing to do; it was reset to `upstream/main` and the fork edges re-derived with `vp i`. Two findings worth reading: **A `converged` delta can lose the line it was anchored to.** pingdotgg#11111 rebuilt the add-surface menu from cards into rows, and the fork's sandbox badge in `RightPanelTabs.tsx` (7 conflicts, the hard one) had no literal home left. It was re-stated on upstream's row — between the label and the `Kbd`, additively, no prop threaded and no upstream JSX re-indented — rather than replayed. The fork's placement test caught the other half: upstream's rows stopped rendering the action `description` at all, in its own Device action too, so the test was asserting on markup nobody emits. It now asserts the rendered label. **A silent auto-merge changed behaviour with no marker, no type error and no resolution-check hit.** pingdotgg#11104 and pingdotgg#11180 changed what the multi-PR badge counts and hoisted `state` onto both badge shapes with a new `draft` colour. Only the fork's `ThreadStatusIndicators.test.tsx` failed, on `+1` against `+2`. Fixture gained the now-required `state`; the expectation was updated to upstream's semantics. ## Inventory and gaps - New `pathPolicy` rows for the paths this merge decided without a cached verdict: `right-panel-surfaces`, `client-runtime-rpc-client`, `thread-status-indicators`, `fork-sandbox-components`. `inventory-check.mjs` is clean. - New gaps entry: _The device hub_, under **Methods the backend does not dispatch**. It names the nine union entries holding it open, the missing `FEATURES.deviceHub` gate, and what closing it costs. - Owned-concern sweep: 11 keyword hits, all false positives. Ten are device-hub files matching the `client-identity` concern on `host`/`proxy` — that concern is about device *pairing* identity, not simulators — and the eleventh, `McpProviderSession.test.ts`, matched on `session`. No concern entry. ## Verification `verify.mjs`: `duplicate-adds`, `tripwires`, `resolution-check`, `unsupported-methods`, `fmt:check`, `lint` and `typecheck` pass. `test` is red on **one** package, and it is not this merge: `@t3tools/desktop`'s `scripts/browser-secret-native.test.mjs > bundled libsecret helper` cannot find `libsecret-1` in this sandbox's pkg-config path. A missing system package, not a code defect — 1 file failed of 102, and the standing entry is in `docs/fork/gaps.md` under _The desktop suite needs libsecret_. One thing to know about reading that log: four packages did not finish under `vp run -r test` and were each re-run alone — mobile `157` files, `t3` `315`, web `389`, relay `30`, all passing. The truncated parallel pass reported a failure that does not exist, `shikiReviewHighlighter.test.ts > initializes source and snippet highlighting without a warmup`, which passes in the alone run and which neither side of this merge touches. The retry lines at the end of the log are the result; the parallel output above them is not. Not reported green. The fork has no CI on pull requests (`docs/fork/gaps.md`, _Nothing checks a pull request_), so this is the whole of the evidence. --- Moatless task: https://moatless.soaplabstest.com/tasks/211b4f8f-7de2-46c8-9ded-330d0cda4add



Summary
git.checkoutBranchexecution withEffect.scoped(...)in the WebSocket handler to align with the updatedGitCoreservice effect requirementsGitCoreservice typing socheckoutBranchexplicitly requiresScope.ScopeCodexAdapterattachment error flow to returntoRequestError(...)directly in the effect pipeline8881a9band upgrades@types/nodeto^24.10.13across root/server/desktopTesting
bun lintbun typecheckNote
Medium Risk
Updates Effect and Node type dependencies and changes
git.checkoutBranchto run in aScope, which could impact runtime behavior of git checkout/background upstream refresh if scoping is incorrect.Overview
Scopes git checkout execution.
GitCoreShape.checkoutBranchnow requiresScope.Scopein its environment, and the WebSocketgitCheckouthandler wraps the call inEffect.scoped(...)so the backgroundforkScopedupstream refresh is tied to a proper scope.Dependency/tooling updates. Bumps the Effect catalog refs to
8881a9band upgrades@types/nodeto^24.10.13across root/server/desktop (with correspondingbun.lockchanges).Minor refactors/formatting. Small non-functional cleanups in
GitCoreparsing/branch logic, provider runtime ingestion, wsServer import formatting, and a small adjustment to Codex attachment invalid-id error handling.Written by Cursor Bugbot for commit 8105951. This will update automatically on new commits. Configure here.
Note
Scope WS git checkout handling with
Effect.scopedin wsServer.ts to ensureGitCoreShape.checkoutBranchruns with aScopeRequire
Scope.ScopeforGitCoreShape.checkoutBranchand wrap the WS git checkout call inEffect.scoped; update Effect-related package pins and bump@types/nodeto 24.10.13.📍Where to Start
Start with the WS router
createServerswitch handlingWS_METHODS.gitCheckoutin wsServer.ts, then review theGitCoreShapechange in apps/server/src/git/Services/GitCore.ts.📊 Macroscope summarized 8105951. 5 files reviewed, 4 issues evaluated, 1 issue filtered, 1 comment posted
🗂️ Filtered Issues
apps/server/src/git/Layers/GitCore.ts — 0 comments posted, 1 evaluated, 1 filtered
checkoutBranchmethod spawns a background taskrefreshCheckedOutBranchUpstreamusingEffect.forkScoped. This attaches the background fiber's lifecycle to theScopeprovided by the caller. WhencheckoutBranchreturns, if the caller-providedScopeis closed (which is the standard behavior when usingEffect.scopedto execute an operation), the background fiber will be immediately interrupted before it can complete. This causes the upstream reference refresh to silently fail, leaving the repository state potentially stale, defeating the stated purpose of keeping the checkout responsive. [ Out of scope ]