feat(vcs): JJ#2461
Open
juliusmarminge wants to merge 9 commits intomainfrom
Open
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
juliusmarminge
changed the base branch from
main
to
t3code/pluggable-git-integration
juliusmarminge
changed the title
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Contributor
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR adds a new VCS driver for Jujutsu (jj), introducing significant new capability including 500+ lines of driver code, VCS-specific UI terminology, and workflow behavior changes. Open review comments identify potential runtime issues that should be addressed. You can customize Macroscope's approvability policy. Learn more. |
juliusmarminge
commented
May 2, 2026
Member
Author
juliusmarminge
left a comment
juliusmarminge
left a comment
There was a problem hiding this comment.
Reviewed this against the pluggable VCS/source-control plan. This is a useful direction, especially the driver registration, shared contract tests, and making the web UI aware that not every repository is Git. I do not think we should treat this as final JJ support yet.
What I would change before this becomes the long-term shape:
- Prefer JJ detection over Git for colocated JJ repositories.
VcsDriverRegistrycurrently tries Git first, sojj --gitrepos will resolve as Git unless project config pins JJ. That makes auto-detect wrong for one of the most likely JJ setups. - Do not represent JJ power only through the generic
VcsDriverplus disabled Git actions. The generic core is fine for status/diff/process basics, but JJ needs a typed extension surface for changes, bookmarks, workspaces, and evolution operations. Otherwise the app will only ever support the lowest common denominator. - Revisit the capability names.
supportsWorktrees: trueis misleading unless the generic API actually exposes worktree operations. If this means JJ workspaces, that should be a JJ capability/workflow, not a Git-term capability flag. - Make
JjVcsDriverfully Effect-native. It currently uses rawnode:fs/node:path/node:ostemp directory handling. New driver code should follow the service style from this phase: detailed errors,FileSystem/Path, scoped acquire/release cleanup, and the shared process/scoping discipline. - The
git check-ignore --no-indexapproach may be acceptable as a Git-compatible ignore fallback, but it should be explicitly modeled as that. It should not become the core JJ ignore semantics by accident.
The big architectural point: this PR should introduce jj as a first-class VCS with generic core operations plus JJ-specific extension points, not as a driver that mostly proves it can fit into a Git-shaped surface.
github-actions
Bot
added
size:XL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duplicated utility functions across VCS drivers
- Extracted splitNullSeparatedPaths and chunkPathsForCheckIgnore into a shared VcsPathUtils.ts module and updated both GitVcsDriver.ts and JjVcsDriver.ts to import from it, eliminating the duplication.
Or push these changes by commenting:
@cursor push 31735f3ec7
Preview (31735f3ec7)
diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts
--- a/apps/server/src/vcs/GitVcsDriver.ts
+++ b/apps/server/src/vcs/GitVcsDriver.ts
@@ -21,6 +21,7 @@
import { makeGitVcsDriverCore } from "./GitVcsDriverCore.ts";
import { VcsDriver, type VcsDriverShape } from "./VcsDriver.ts";
import { nowFreshness } from "./VcsFreshness.ts";
+import { chunkPathsForCheckIgnore, splitNullSeparatedPaths } from "./VcsPathUtils.ts";
import { VcsProcess, type VcsProcessShape } from "./VcsProcess.ts";
export interface ExecuteGitInput {
@@ -202,7 +203,6 @@
) {}
const WORKSPACE_FILES_MAX_OUTPUT_BYTES = 16 * 1024 * 1024;
-const GIT_CHECK_IGNORE_MAX_STDIN_BYTES = 256 * 1024;
const WORKSPACE_GIT_HARDENED_CONFIG_ARGS = [
"-c",
"core.fsmonitor=false",
@@ -210,47 +210,6 @@
"core.untrackedCache=false",
] as const;
-function splitNullSeparatedPaths(input: string, truncated: boolean): string[] {
- const parts = input.split("\0");
- if (parts.length === 0) return [];
-
- if (truncated && parts[parts.length - 1]?.length) {
- parts.pop();
- }
-
- return parts.filter((value) => value.length > 0);
-}
-
-function chunkPathsForGitCheckIgnore(relativePaths: ReadonlyArray<string>): string[][] {
- const chunks: string[][] = [];
- let chunk: string[] = [];
- let chunkBytes = 0;
-
- for (const relativePath of relativePaths) {
- const relativePathBytes = Buffer.byteLength(relativePath) + 1;
- if (chunk.length > 0 && chunkBytes + relativePathBytes > GIT_CHECK_IGNORE_MAX_STDIN_BYTES) {
- chunks.push(chunk);
- chunk = [];
- chunkBytes = 0;
- }
-
- chunk.push(relativePath);
- chunkBytes += relativePathBytes;
-
- if (chunkBytes >= GIT_CHECK_IGNORE_MAX_STDIN_BYTES) {
- chunks.push(chunk);
- chunk = [];
- chunkBytes = 0;
- }
- }
-
- if (chunk.length > 0) {
- chunks.push(chunk);
- }
-
- return chunks;
-}
-
const gitCommand = (
process: VcsProcessShape,
operation: string,
@@ -392,7 +351,7 @@
}
const ignoredPaths = new Set<string>();
- const chunks = chunkPathsForGitCheckIgnore(relativePaths);
+ const chunks = chunkPathsForCheckIgnore(relativePaths);
for (const chunk of chunks) {
const result = yield* gitCommand(
diff --git a/apps/server/src/vcs/JjVcsDriver.ts b/apps/server/src/vcs/JjVcsDriver.ts
--- a/apps/server/src/vcs/JjVcsDriver.ts
+++ b/apps/server/src/vcs/JjVcsDriver.ts
@@ -3,22 +3,11 @@
import { VcsOutputDecodeError, VcsProcessExitError } from "@t3tools/contracts";
import { VcsDriver, type VcsDriverShape } from "./VcsDriver.ts";
import { nowFreshness } from "./VcsFreshness.ts";
+import { chunkPathsForCheckIgnore, splitNullSeparatedPaths } from "./VcsPathUtils.ts";
import { VcsProcess, type VcsProcessShape } from "./VcsProcess.ts";
const WORKSPACE_FILES_MAX_OUTPUT_BYTES = 16 * 1024 * 1024;
-const CHECK_IGNORE_MAX_STDIN_BYTES = 256 * 1024;
-function splitNullSeparatedPaths(input: string, truncated: boolean): string[] {
- const parts = input.split("\0");
- if (parts.length === 0) return [];
-
- if (truncated && parts[parts.length - 1]?.length) {
- parts.pop();
- }
-
- return parts.filter((value) => value.length > 0);
-}
-
function splitLineSeparatedPaths(input: string, truncated: boolean): string[] {
const lines = input.split(/\r?\n/g);
if (truncated && lines[lines.length - 1]?.length) {
@@ -28,36 +17,6 @@
return lines.map((line) => line.trim()).filter((line) => line.length > 0);
}
-function chunkPathsForCheckIgnore(relativePaths: ReadonlyArray<string>): string[][] {
- const chunks: string[][] = [];
- let chunk: string[] = [];
- let chunkBytes = 0;
-
- for (const relativePath of relativePaths) {
- const relativePathBytes = Buffer.byteLength(relativePath) + 1;
- if (chunk.length > 0 && chunkBytes + relativePathBytes > CHECK_IGNORE_MAX_STDIN_BYTES) {
- chunks.push(chunk);
- chunk = [];
- chunkBytes = 0;
- }
-
- chunk.push(relativePath);
- chunkBytes += relativePathBytes;
-
- if (chunkBytes >= CHECK_IGNORE_MAX_STDIN_BYTES) {
- chunks.push(chunk);
- chunk = [];
- chunkBytes = 0;
- }
- }
-
- if (chunk.length > 0) {
- chunks.push(chunk);
- }
-
- return chunks;
-}
-
const processCommand = (
process: VcsProcessShape,
command: string,
diff --git a/apps/server/src/vcs/VcsPathUtils.ts b/apps/server/src/vcs/VcsPathUtils.ts
new file mode 100644
--- /dev/null
+++ b/apps/server/src/vcs/VcsPathUtils.ts
@@ -1,0 +1,42 @@
+const CHECK_IGNORE_MAX_STDIN_BYTES = 256 * 1024;
+
+export function splitNullSeparatedPaths(input: string, truncated: boolean): string[] {
+ const parts = input.split("\0");
+ if (parts.length === 0) return [];
+
+ if (truncated && parts[parts.length - 1]?.length) {
+ parts.pop();
+ }
+
+ return parts.filter((value) => value.length > 0);
+}
+
+export function chunkPathsForCheckIgnore(relativePaths: ReadonlyArray<string>): string[][] {
+ const chunks: string[][] = [];
+ let chunk: string[] = [];
+ let chunkBytes = 0;
+
+ for (const relativePath of relativePaths) {
+ const relativePathBytes = Buffer.byteLength(relativePath) + 1;
+ if (chunk.length > 0 && chunkBytes + relativePathBytes > CHECK_IGNORE_MAX_STDIN_BYTES) {
+ chunks.push(chunk);
+ chunk = [];
+ chunkBytes = 0;
+ }
+
+ chunk.push(relativePath);
+ chunkBytes += relativePathBytes;
+
+ if (chunkBytes >= CHECK_IGNORE_MAX_STDIN_BYTES) {
+ chunks.push(chunk);
+ chunk = [];
+ chunkBytes = 0;
+ }
+ }
+
+ if (chunk.length > 0) {
+ chunks.push(chunk);
+ }
+
+ return chunks;
+}You can send follow-ups to the cloud agent here.
juliusmarminge
commented
May 2, 2026
Member
Author
juliusmarminge
left a comment
juliusmarminge
left a comment
There was a problem hiding this comment.
Second pass after the follow-up commits. This fixed several concrete issues from the first review: JJ is now detected before Git, temp directory handling moved to Effect FileSystem, and the capability flags are less misleading. That is progress.
I still do not think this should merge as the long-term JJ shape yet.
Required changes / architectural blockers:
JjVcsDriverstill only implements the genericVcsDriversurface (execute,detectRepository,isInsideWorkTree,listWorkspaceFiles,filterIgnoredPaths). That proves JJ can be detected, but it does not establish the extension-method model we need. A JJ PR should introduce a typed JJ extension service or extension registry shape now, even if only a small set is implemented. Examples: current change, bookmarks, workspace operations, operation log/evolution primitives. Without that, JJ support remains lowest-common-denominator VCS support with Git UI disabled.packages/contracts/src/vcs.tsnow has generic capabilities likesupportsBookmarksandsupportsAtomicSnapshot, but no typed API behind them. These flags are not enough for extensibility. Capabilities should either describe operations exposed on the generic core, or point to typed extension services. Otherwise callers can see that a capability exists but have no safe way to use it.ignoreClassifier: "git-compatible-fallback"is better than silently pretending this is native, butJjVcsDriver.filterIgnoredPathsstill shells out to Git and creates a synthetic bare Git dir. If this stays, it needs to be very clearly treated as a compatibility fallback and isolated from the eventual JJ-native extension path. We should not build future JJ behavior on top of Git semantics by default.- The UI path still mostly treats non-Git VCS as “disable Git actions and swap some labels”. That is acceptable for a detection-only slice, but not for claiming JJ as the first extension-capable VCS driver. The PR should either add the extension point now or explicitly narrow the scope/title to detection/status-only JJ support.
The core requirement from the plan is: generic VCS core plus VCS-specific extension methods. This PR improves the generic core, but it still does not add the extension-method half. I would hold the line here so we do not merge a shape that makes future JJ features feel bolted on.
juliusmarminge
force-pushed
the
t3code/jj-driver-vsc
branch
from
1a1976b to
e2e40c2
Compare
Comment on lines
+219
to
+227
| function fileSystemError(operation: string, cwd: string, detail: string, cause: unknown) { | ||
| return new VcsOutputDecodeError({ | ||
| operation, | ||
| command: "git check-ignore", | ||
| cwd, | ||
| detail, | ||
| cause, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🟢 Low vcs/JjVcsDriver.ts:219
fileSystemError hardcodes command: "git check-ignore" but is called from makeScopedTempGitDir when temp directory creation fails. This causes filesystem errors to report the wrong command, misleading anyone debugging the actual failure. Consider passing the correct command name as a parameter, or using a more generic error type for filesystem operations.
-function fileSystemError(operation: string, cwd: string, detail: string, cause: unknown) {
- return new VcsOutputDecodeError({
- operation,
- command: "git check-ignore",
- cwd,
- detail,
- cause,
- });
-}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/vcs/JjVcsDriver.ts around lines 219-227:
`fileSystemError` hardcodes `command: "git check-ignore"` but is called from `makeScopedTempGitDir` when temp directory creation fails. This causes filesystem errors to report the wrong command, misleading anyone debugging the actual failure. Consider passing the correct command name as a parameter, or using a more generic error type for filesystem operations.
Evidence trail:
apps/server/src/vcs/JjVcsDriver.ts lines 219-226 (fileSystemError hardcodes command: "git check-ignore"), lines 229-236 (makeScopedTempGitDir calls fileSystemError on temp dir creation failure), lines 406-416 (makeScopedTempGitDir called from filterIgnoredPaths). packages/contracts/src/vcs.ts lines 104-118 (VcsOutputDecodeError class with message template "VCS output decode failed in ${this.operation}: ${this.command}").
Comment on lines
+427
to
+433
| return yield* new VcsProcessExitError({ | ||
| operation, | ||
| command: "git init --bare", | ||
| cwd, | ||
| exitCode: initResult.exitCode, | ||
| detail: initResult.stderr.trim() || "git init --bare failed", | ||
| }); |
Contributor
There was a problem hiding this comment.
vcs/JjVcsDriver.ts:427
yield* new VcsProcessExitError({...}) at line 427 yields a plain error class instead of an Effect, causing a runtime type error when the generator tries to interpret it as an Effect. The correct pattern is yield* Effect.fail(new VcsProcessExitError({...})) as used at lines 313-320.
Suggested change
| return yield* new VcsProcessExitError({ | |
| operation, | |
| command: "git init --bare", | |
| cwd, | |
| exitCode: initResult.exitCode, | |
| detail: initResult.stderr.trim() || "git init --bare failed", | |
| }); | |
| return yield* Effect.fail(new VcsProcessExitError({ |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/vcs/JjVcsDriver.ts around lines 427-433:
`yield* new VcsProcessExitError({...})` at line 427 yields a plain error class instead of an Effect, causing a runtime type error when the generator tries to interpret it as an Effect. The correct pattern is `yield* Effect.fail(new VcsProcessExitError({...}))` as used at lines 313-320.
Evidence trail:
packages/contracts/src/vcs.ts:75 - VcsProcessExitError extends Schema.TaggedErrorClass which makes instances yieldable Effects. apps/server/src/vcs/JjVcsDriver.ts:427 - the yield* new VcsProcessExitError pattern. apps/server/src/vcs/JjVcsDriver.ts:313-320 - Effect.fail() used in non-generator context (ternary). git_grep for 'yield\* new Vcs' shows 8+ uses of this pattern across the codebase (VcsProcess.ts:218, GitVcsDriver.ts:429,484, CheckpointStore.ts:147,165,253).
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Thrown error becomes defect instead of typed error
- Changed decodeJjCurrentChange to return Effect.Effect<JjCurrentChange | null, VcsOutputDecodeError> using Effect.fail/Effect.succeed, and updated the call site from Effect.map to Effect.flatMap so errors route to the typed error channel.
Or push these changes by commenting:
@cursor push 02cac6dd61
Preview (02cac6dd61)
diff --git a/apps/server/src/vcs/JjVcsDriver.ts b/apps/server/src/vcs/JjVcsDriver.ts
--- a/apps/server/src/vcs/JjVcsDriver.ts
+++ b/apps/server/src/vcs/JjVcsDriver.ts
@@ -80,27 +80,32 @@
return record.split("\0").map((value) => value.trim());
}
-function decodeJjCurrentChange(raw: string, cwd: string): JjCurrentChange | null {
+function decodeJjCurrentChange(
+ raw: string,
+ cwd: string,
+): Effect.Effect<JjCurrentChange | null, VcsOutputDecodeError> {
const trimmed = raw.trim();
if (trimmed.length === 0) {
- return null;
+ return Effect.succeed(null);
}
const [changeId, commitId, description] = parseNullRecord(trimmed);
if (!changeId) {
- throw new VcsOutputDecodeError({
- operation: "JjVcsDriver.currentChange",
- command: "jj log",
- cwd,
- detail: "jj current change output did not include a change id",
- });
+ return Effect.fail(
+ new VcsOutputDecodeError({
+ operation: "JjVcsDriver.currentChange",
+ command: "jj log",
+ cwd,
+ detail: "jj current change output did not include a change id",
+ }),
+ );
}
- return {
+ return Effect.succeed({
changeId,
commitId: commitId || null,
description: description || null,
- };
+ });
}
function decodeJjBookmarkList(raw: string): ReadonlyArray<JjBookmark> {
@@ -370,7 +375,7 @@
timeoutMs: 5_000,
maxOutputBytes: 64 * 1024,
},
- ).pipe(Effect.map((result) => decodeJjCurrentChange(result.stdout, cwd)));
+ ).pipe(Effect.flatMap((result) => decodeJjCurrentChange(result.stdout, cwd)));
const listBookmarks: JjVcsDriverShape["listBookmarks"] = (cwd) =>
jjCommand(You can send follow-ups to the cloud agent here.
juliusmarminge
force-pushed
the
t3code/jj-driver-vsc
branch
3 times, most recently
from
1b93238 to
2df8ce2
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Remote-only status fallback creates misleading unknown VCS kind
- Added 'unknown' to the supported kinds in resolveVcsActionPresentation so the transient state from remoteUpdated fallback does not incorrectly disable git workflow controls.
Or push these changes by commenting:
@cursor push 408123a544
Preview (408123a544)
diff --git a/apps/web/src/vcsPresentation.ts b/apps/web/src/vcsPresentation.ts
--- a/apps/web/src/vcsPresentation.ts
+++ b/apps/web/src/vcsPresentation.ts
@@ -58,7 +58,8 @@
kind: VcsDriverKind | null | undefined,
): VcsActionPresentation {
const terms = resolveVcsTerms(kind);
- const supportsGitWorkflowActions = kind === undefined || kind === null || kind === "git";
+ const supportsGitWorkflowActions =
+ kind === undefined || kind === null || kind === "git" || kind === "unknown";
return {
supportsGitWorkflowActions,You can send follow-ups to the cloud agent here.
juliusmarminge
force-pushed
the
t3code/jj-driver-vsc
branch
from
2df8ce2 to
e544202
Compare
juliusmarminge
force-pushed
the
t3code/pluggable-git-integration
branch
from
7aa00ff to
f4180a4
Compare
juliusmarminge
force-pushed
the
t3code/jj-driver-vsc
branch
from
e544202 to
31fb100
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Dead
"sapling"branch inresolveVcsTerms- Removed the unreachable
kind === "sapling"branch sinceVcsDriverKindonly includes"git" | "jj" | "unknown".
- Removed the unreachable
Or push these changes by commenting:
@cursor push 00f33428b1
Preview (00f33428b1)
diff --git a/apps/web/src/vcsPresentation.ts b/apps/web/src/vcsPresentation.ts
--- a/apps/web/src/vcsPresentation.ts
+++ b/apps/web/src/vcsPresentation.ts
@@ -40,13 +40,6 @@
};
}
- if (kind === "sapling") {
- return {
- ...gitLikeTerms,
- systemName: "Sapling",
- };
- }
-
if (kind === "git") {
return gitLikeTerms;
}You can send follow-ups to the cloud agent here.
juliusmarminge
force-pushed
the
t3code/jj-driver-vsc
branch
from
31fb100 to
156065c
Compare
| }; | ||
| }); | ||
|
|
||
| const currentChange: JjVcsDriverShape["currentChange"] = (cwd) => |
Contributor
There was a problem hiding this comment.
🟡 Medium vcs/JjVcsDriver.ts:359
decodeJjCurrentChange throws VcsOutputDecodeError synchronously, but it's called inside Effect.map() at line 376. In Effect, synchronous exceptions in Effect.map become defects (untyped failures) rather than typed errors in the error channel. The return type Effect.Effect<JjCurrentChange | null, VcsError> promises a typed VcsError, but the thrown exception bypasses this channel. Consider wrapping the decoding in Effect.try or Effect.catchAll to convert the throw into a typed error, or use Effect.gen with yield* like the pattern at line 430.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/vcs/JjVcsDriver.ts around line 359:
`decodeJjCurrentChange` throws `VcsOutputDecodeError` synchronously, but it's called inside `Effect.map()` at line 376. In Effect, synchronous exceptions in `Effect.map` become defects (untyped failures) rather than typed errors in the error channel. The return type `Effect.Effect<JjCurrentChange | null, VcsError>` promises a typed `VcsError`, but the thrown exception bypasses this channel. Consider wrapping the decoding in `Effect.try` or `Effect.catchAll` to convert the throw into a typed error, or use `Effect.gen` with `yield*` like the pattern at line 430.
Evidence trail:
apps/server/src/vcs/JjVcsDriver.ts lines 83-103 (decodeJjCurrentChange throws VcsOutputDecodeError at line 91); apps/server/src/vcs/JjVcsDriver.ts line 376 (called inside Effect.map); packages/contracts/src/vcs.ts lines 104-118 (VcsOutputDecodeError definition) and lines 146-154 (VcsError union includes VcsOutputDecodeError); Effect-TS documentation at https://github.com/Effect-TS/effect/blob/main/packages/effect/src/Effect.ts confirms Effect.sync is for non-throwing code, Effect.try is for code that throws; Effect-TS language-service at https://github.com/Effect-TS/language-service lists 'tryCatchInEffectGen' diagnostic that discourages try/catch in Effect generators in favor of Effect error handling, confirming throws bypass typed error channels.
Comment on lines
+36
to
+49
| const knownSourceControlDiscoveryKeys = new Set<string>(); | ||
|
|
||
| export const sourceControlDiscoveryStateAtom = Atom.family((key: string) => { | ||
| knownSourceControlDiscoveryKeys.add(key); | ||
| return Atom.make(INITIAL_SOURCE_CONTROL_DISCOVERY_STATE).pipe( | ||
| Atom.keepAlive, | ||
| Atom.withLabel(`source-control-discovery:${key}`), | ||
| ); | ||
| }); | ||
|
|
||
| export const EMPTY_SOURCE_CONTROL_DISCOVERY_ATOM = Atom.make( | ||
| EMPTY_SOURCE_CONTROL_DISCOVERY_STATE, | ||
| ).pipe(Atom.keepAlive, Atom.withLabel("source-control-discovery:null")); | ||
|
|
Contributor
There was a problem hiding this comment.
🟢 Low src/sourceControlDiscoveryState.ts:36
knownSourceControlDiscoveryKeys is module-scoped but mutated by each manager instance's reset() method. If multiple SourceControlDiscoveryManager instances are created (e.g., in parallel tests), calling reset() on one manager clears the shared Set, so subsequent reset() calls on other managers skip resetting atoms they should have tracked.
-const knownSourceControlDiscoveryKeys = new Set<string>();
+
export const sourceControlDiscoveryStateAtom = Atom.family((key: string) => {
- knownSourceControlDiscoveryKeys.add(key);
return Atom.make(INITIAL_SOURCE_CONTROL_DISCOVERY_STATE).pipe(
Atom.keepAlive,
Atom.withLabel(`source-control-discovery:${key}`),
);
});
export const EMPTY_SOURCE_CONTROL_DISCOVERY_ATOM = Atom.make(
EMPTY_SOURCE_CONTROL_DISCOVERY_STATE,
).pipe(Atom.keepAlive, Atom.withLabel("source-control-discovery:null"));🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/client-runtime/src/sourceControlDiscoveryState.ts around lines 36-49:
`knownSourceControlDiscoveryKeys` is module-scoped but mutated by each manager instance's `reset()` method. If multiple `SourceControlDiscoveryManager` instances are created (e.g., in parallel tests), calling `reset()` on one manager clears the shared `Set`, so subsequent `reset()` calls on other managers skip resetting atoms they should have tracked.
Evidence trail:
packages/client-runtime/src/sourceControlDiscoveryState.ts line 36: `const knownSourceControlDiscoveryKeys = new Set<string>();` — module-scoped mutable set.
Line 38-39: `sourceControlDiscoveryStateAtom` family adds keys to this shared set.
Lines 177-182: `reset()` iterates the shared set, resets atoms, then calls `.clear()` on it.
Line 86: `createSourceControlDiscoveryManager` is a factory function creating per-instance `refreshInFlight` but sharing the module-level `knownSourceControlDiscoveryKeys`.
156065c to
d75f5df
Compare
Contributor
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: Double async
realPathinrefreshStatuspath- Extracted
refreshLocalStatusCore(no normalization) sorefreshStatuscalls it directly after its own normalization, eliminating the redundant secondrealPathcall.
- Extracted
Or push these changes by commenting:
@cursor push 87975b1c96
Preview (87975b1c96)
diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.ts b/apps/server/src/vcs/VcsStatusBroadcaster.ts
--- a/apps/server/src/vcs/VcsStatusBroadcaster.ts
+++ b/apps/server/src/vcs/VcsStatusBroadcaster.ts
@@ -201,13 +201,19 @@
return mergeGitStatusParts(local, remote);
});
+ const refreshLocalStatusCore = Effect.fn("VcsStatusBroadcaster.refreshLocalStatusCore")(
+ function* (cwd: string) {
+ yield* workflow.invalidateLocalStatus(cwd);
+ const local = yield* workflow.localStatus({ cwd });
+ return yield* updateCachedLocalStatus(cwd, local, { publish: true });
+ },
+ );
+
const refreshLocalStatus: VcsStatusBroadcasterShape["refreshLocalStatus"] = Effect.fn(
"VcsStatusBroadcaster.refreshLocalStatus",
)(function* (rawCwd) {
const cwd = yield* normalizeCwd(rawCwd);
- yield* workflow.invalidateLocalStatus(cwd);
- const local = yield* workflow.localStatus({ cwd });
- return yield* updateCachedLocalStatus(cwd, local, { publish: true });
+ return yield* refreshLocalStatusCore(cwd);
});
const refreshRemoteStatus = Effect.fn("VcsStatusBroadcaster.refreshRemoteStatus")(function* (
@@ -223,7 +229,7 @@
)(function* (rawCwd) {
const cwd = yield* normalizeCwd(rawCwd);
const [local, remote] = yield* Effect.all([
- refreshLocalStatus(cwd),
+ refreshLocalStatusCore(cwd),
refreshRemoteStatus(cwd),
]);
return mergeGitStatusParts(local, remote);You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit d75f5df. Configure here.
- Detect and route jj repositories alongside git - Thread VCS kind through status and UI labels - Add jj driver contract coverage and shared freshness helper
added 8 commits
May 2, 2026 19:03
- Replace ad hoc temp-dir handling with scoped FileSystem APIs - Provide NodeServices in VCS driver and status broadcaster tests - Normalize CWDs through FileSystem realpath
- Detect jj before git for colocated repositories - Add ignoreClassifier to VCS capabilities - Fix trace2 hook exit code reporting
juliusmarminge
force-pushed
the
t3code/jj-driver-vsc
branch
from
d75f5df to
a251619
Compare
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
VcsDriver,VcsProcess, registry, and Git/JJ driver split.Testing
bun fmtbun lintbun typecheckNote
Medium Risk
Adds a new VCS driver and changes the status contract to require a
kindfield, which can affect validation/serialization and behavior across server + web. Also touches workflow routing and status caching/normalization, so regressions could surface in repo detection and Git-only action flows.Overview
Adds first-class Jujutsu (
jj) support by introducing a newJjVcsDriver(repo detection, workspace file listing, remotes, ignore filtering via git fallback, bookmarks/workspaces/current change) and registering it inVcsDriverRegistry, with auto-detection preferring JJ over Git for colocated repos.Extends VCS status contracts/results to include a required
kind(git/jj/unknown) and refactors status composition (GitWorkflowServicenow merges local+remote parts and returns explicit non-Git local status). UpdatesVcsStatusBroadcasterto normalize CWDs viaFileSystem.realPathand propagates the new status shape through tests.Updates the web UI to be VCS-aware: introduces
vcsPresentationterminology (branches vs bookmarks/workspaces), adjusts labels/errors, and disables Git-only workflow actions when the active repo kind isn’t Git. CI/release workflows now installjjvia a new compositesetup-jjaction.Reviewed by Cursor Bugbot for commit a251619. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Jujutsu (jj) VCS driver with auto-detection and VCS-aware UI terminology
JjVcsDriverin JjVcsDriver.ts supporting repository detection, file listing, remote/bookmark/workspace listing, command execution, and ignore filtering via agit check-ignorefallback.VcsDriverRegistryto attempt JJ detection before Git when auto-detecting, and registers JJ as a known driver.kindfield ('git','jj', or'unknown') toVcsStatusLocalShapeand all status result objects, propagated throughGitWorkflowService,GitManager, shared contracts, and stream event handlers.BranchToolbarBranchSelector,ChatHeader,DiffPanel, andGitActionsControlwith VCS-aware labels via a newvcsPresentation.tsmodule; Git-specific workflow actions are disabled with a tooltip for non-Git repos.jjCLI in CI via a new reusable setup-jj composite action used in both quality and release workflows.VcsStatusLocalShapenow requires akindfield; any producer or consumer of this contract that does not includekindwill fail schema validation.Macroscope summarized a251619.