Skip to content

Scope git checkout effects and update Effect/Node dependency set #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
juliusmarminge merged 2 commits into from
Mar 3, 2026
Merged

Scope git checkout effects and update Effect/Node dependency set #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
681d3d8
Scope git checkout effects and bump Effect/Node dependencies
juliusmarminge Mar 2, 2026
8105951
fmt
juliusmarminge Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/desktop/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
},
"devDependencies": {
"@t3tools/contracts": "workspace:*",
"@types/node": "^22.10.2",
"@types/node": "^24.10.13",
"electronmon": "^2.0.2",
"tsdown": "^0.20.3",
"typescript": "^5.7.3",
Expand Down
2 changes: 1 addition & 1 deletion apps/server/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"@t3tools/contracts": "workspace:*",
"@t3tools/web": "workspace:*",
"@types/bun": "^1.3.9",
"@types/node": "^22.10.2",
"@types/node": "^24.10.13",
"@types/ws": "^8.5.13",
"tsdown": "^0.20.3",
"typescript": "^5.7.3",
Expand Down
117 changes: 58 additions & 59 deletions apps/server/src/git/Layers/GitCore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,7 @@ function parseRemoteRefWithRemoteNames(
return null;
}

function parseTrackingBranchByUpstreamRef(
stdout: string,
upstreamRef: string,
): string | null {
function parseTrackingBranchByUpstreamRef(stdout: string, upstreamRef: string): string | null {
for (const line of stdout.split("\n")) {
const trimmedLine = line.trim();
if (trimmedLine.length === 0) {
Expand Down Expand Up @@ -389,9 +386,12 @@ const makeGitCore = Effect.gen(function* () {
});

const resolveDefaultBranchName = (cwd: string): Effect.Effect<string | null, GitCommandError> =>
executeGit("GitCore.resolveDefaultBranchName", cwd, ["symbolic-ref", "refs/remotes/origin/HEAD"], {
allowNonZeroExit: true,
}).pipe(
executeGit(
"GitCore.resolveDefaultBranchName",
cwd,
["symbolic-ref", "refs/remotes/origin/HEAD"],
{ allowNonZeroExit: true },
).pipe(
Effect.map((result) => {
if (result.code !== 0) {
return null;
Expand Down Expand Up @@ -888,33 +888,34 @@ const makeGitCore = Effect.gen(function* () {

const [defaultRef, worktreeList, remoteBranchResult, remoteNamesResult, branchLastCommit] =
yield* Effect.all(
[
executeGit(
"GitCore.listBranches.defaultRef",
input.cwd,
["symbolic-ref", "refs/remotes/origin/HEAD"],
{
timeoutMs: 5_000,
allowNonZeroExit: true,
},
),
executeGit(
"GitCore.listBranches.worktreeList",
input.cwd,
["worktree", "list", "--porcelain"],
{
timeoutMs: 5_000,
allowNonZeroExit: true,
},
),
remoteBranchResultEffect,
remoteNamesResultEffect,
branchRecencyPromise,
],
{ concurrency: "unbounded" },
);
[
executeGit(
"GitCore.listBranches.defaultRef",
input.cwd,
["symbolic-ref", "refs/remotes/origin/HEAD"],
{
timeoutMs: 5_000,
allowNonZeroExit: true,
},
),
executeGit(
"GitCore.listBranches.worktreeList",
input.cwd,
["worktree", "list", "--porcelain"],
{
timeoutMs: 5_000,
allowNonZeroExit: true,
},
),
remoteBranchResultEffect,
remoteNamesResultEffect,
branchRecencyPromise,
],
{ concurrency: "unbounded" },
);

const remoteNames = remoteNamesResult.code === 0 ? parseRemoteNames(remoteNamesResult.stdout) : [];
const remoteNames =
remoteNamesResult.code === 0 ? parseRemoteNames(remoteNamesResult.stdout) : [];
if (remoteBranchResult.code !== 0 && remoteBranchResult.stderr.trim().length > 0) {
yield* Effect.logWarning(
`GitCore.listBranches: remote branch lookup returned code ${remoteBranchResult.code} for ${input.cwd}: ${remoteBranchResult.stderr.trim()}. Falling back to an empty remote branch list.`,
Expand Down Expand Up @@ -1112,24 +1113,23 @@ const makeGitCore = Effect.gen(function* () {
{ concurrency: "unbounded" },
);

const localTrackingBranch =
remoteExists
? yield* executeGit(
"GitCore.checkoutBranch.localTrackingBranch",
input.cwd,
["for-each-ref", "--format=%(refname:short)\t%(upstream:short)", "refs/heads"],
{
timeoutMs: 5_000,
allowNonZeroExit: true,
},
).pipe(
Effect.map((result) =>
result.code === 0
? parseTrackingBranchByUpstreamRef(result.stdout, input.branch)
: null,
),
)
: null;
const localTrackingBranch = remoteExists
? yield* executeGit(
"GitCore.checkoutBranch.localTrackingBranch",
input.cwd,
["for-each-ref", "--format=%(refname:short)\t%(upstream:short)", "refs/heads"],
{
timeoutMs: 5_000,
allowNonZeroExit: true,
},
).pipe(
Effect.map((result) =>
result.code === 0
? parseTrackingBranchByUpstreamRef(result.stdout, input.branch)
: null,
),
)
: null;

const localTrackedBranchCandidate = deriveLocalBranchNameFromRemoteRef(input.branch);
const localTrackedBranchTargetExists =
Expand All @@ -1145,16 +1145,15 @@ const makeGitCore = Effect.gen(function* () {
).pipe(Effect.map((result) => result.code === 0))
: false;

const checkoutArgs =
localInputExists
? ["checkout", input.branch]
: remoteExists && !localTrackingBranch && localTrackedBranchTargetExists
const checkoutArgs = localInputExists
? ["checkout", input.branch]
: remoteExists && !localTrackingBranch && localTrackedBranchTargetExists
? ["checkout", input.branch]
: remoteExists && !localTrackingBranch
? ["checkout", "--track", input.branch]
: remoteExists && localTrackingBranch
? ["checkout", localTrackingBranch]
: ["checkout", input.branch];
? ["checkout", "--track", input.branch]
: remoteExists && localTrackingBranch
? ["checkout", localTrackingBranch]
: ["checkout", input.branch];

yield* executeGit("GitCore.checkoutBranch.checkout", input.cwd, checkoutArgs, {
timeoutMs: 10_000,
Expand Down
6 changes: 4 additions & 2 deletions apps/server/src/git/Services/GitCore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @module GitCore
*/
import { ServiceMap } from "effect";
import type { Effect } from "effect";
import type { Effect, Scope } from "effect";
import type {
GitCheckoutInput,
GitCreateBranchInput,
Expand Down Expand Up @@ -149,7 +149,9 @@ export interface GitCoreShape {
/**
* Checkout an existing branch and refresh its upstream metadata in background.
*/
readonly checkoutBranch: (input: GitCheckoutInput) => Effect.Effect<void, GitCommandError>;
readonly checkoutBranch: (
input: GitCheckoutInput,
) => Effect.Effect<void, GitCommandError, Scope.Scope>;

/**
* Initialize a repository in the provided directory.
Expand Down
3 changes: 1 addition & 2 deletions apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,7 @@ const make = Effect.gen(function* () {
: event.threadId !== undefined
? ProviderThreadId.makeUnsafe(event.threadId)
: null;
const providerThreadId =
providerThreadIdFromEvent ?? sessionProviderThreadId ?? null;
const providerThreadId = providerThreadIdFromEvent ?? sessionProviderThreadId ?? null;
const status =
event.type === "turn.started"
? "running"
Expand Down
10 changes: 4 additions & 6 deletions apps/server/src/provider/Layers/CodexAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,12 +489,10 @@ const makeCodexAdapter = (options?: CodexAdapterLiveOptions) =>
attachment,
});
if (!attachmentPath) {
return yield* Effect.fail(
toRequestError(
input.sessionId,
"turn/start",
new Error(`Invalid attachment id '${attachment.id}'.`),
),
return yield* toRequestError(

@macroscopeapp macroscopeapp Bot Mar 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 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)

input.sessionId,
"turn/start",
new Error(`Invalid attachment id '${attachment.id}'.`),
);
}
const bytes = yield* fileSystem.readFile(attachmentPath).pipe(
Expand Down
8 changes: 6 additions & 2 deletions apps/server/src/wsServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ import {
normalizeAttachmentRelativePath,
resolveAttachmentRelativePath,
} from "./attachmentPaths";
import { createAttachmentId, resolveAttachmentPath, resolveAttachmentPathById } from "./attachmentStore.ts";
import {
createAttachmentId,
resolveAttachmentPath,
resolveAttachmentPathById,
} from "./attachmentStore.ts";
import { parseBase64DataUrl } from "./imageMime.ts";

/**
Expand Down Expand Up @@ -685,7 +689,7 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return<

case WS_METHODS.gitCheckout: {
const body = stripRequestTag(request.body);
return yield* git.checkoutBranch(body);
return yield* Effect.scoped(git.checkoutBranch(body));

@cursor cursor Bot Mar 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)

Fix in Cursor Fix in Web

}

case WS_METHODS.gitInit: {
Expand Down
Loading