Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ export const makeOrchestrationIntegrationHarness = (
refreshStatus: () => Effect.die("refreshStatus should not be called in this test"),
streamStatus: () => Stream.empty,
observeLocalStatus: () => Stream.empty,
observeMissingCheckouts: () => Stream.empty,
}),
),
Layer.provideMerge(
Expand Down
52 changes: 50 additions & 2 deletions apps/server/src/git/GitWorkflowService.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { assert, describe, it, vi } from "@effect/vitest";
import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";
import * as NodeServices from "@effect/platform-node/NodeServices";
import * as NodeOS from "node:os";
import * as NodePath from "node:path";

import * as GitManager from "./GitManager.ts";
import * as GitWorkflowService from "./GitWorkflowService.ts";
Expand All @@ -9,6 +12,7 @@ import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts";

function makeLayer(input: { readonly detect: VcsDriverRegistry.VcsDriverRegistryShape["detect"] }) {
return GitWorkflowService.layer.pipe(
Layer.provideMerge(NodeServices.layer),
Layer.provide(
Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({
detect: input.detect,
Expand Down Expand Up @@ -43,6 +47,7 @@ describe("GitWorkflowService", () => {
}),
);
const testLayer = GitWorkflowService.layer.pipe(
Layer.provideMerge(NodeServices.layer),
Layer.provide(
Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({
detect: () => Effect.succeed(makeGitHandle("/repo")),
Expand All @@ -67,7 +72,7 @@ describe("GitWorkflowService", () => {
it.effect("returns an empty local status when no VCS repository is detected", () =>
Effect.gen(function* () {
const workflow = yield* GitWorkflowService.GitWorkflowService;
const status = yield* workflow.localStatus({ cwd: "/not-a-repo" });
const status = yield* workflow.localStatus({ cwd: NodeOS.tmpdir() });

assert.deepStrictEqual(status, {
isRepo: false,
Expand All @@ -94,7 +99,7 @@ describe("GitWorkflowService", () => {
it.effect("returns an empty full status when no VCS repository is detected", () =>
Effect.gen(function* () {
const workflow = yield* GitWorkflowService.GitWorkflowService;
const status = yield* workflow.status({ cwd: "/not-a-repo" });
const status = yield* workflow.status({ cwd: NodeOS.tmpdir() });

assert.deepStrictEqual(status, {
isRepo: false,
Expand Down Expand Up @@ -123,12 +128,53 @@ describe("GitWorkflowService", () => {
),
);

// Both cases reach here as "no driver handle", but they mean opposite things
// to the user: an empty directory invites `git init`, a deleted checkout must
// not. Offering to initialize a repository at a path the user's agent removed
// would quietly replace their work with an unrelated empty repo.
it.effect("marks a status whose directory no longer exists", () =>
Effect.gen(function* () {
const workflow = yield* GitWorkflowService.GitWorkflowService;
const gone = NodePath.join(NodeOS.tmpdir(), "threadlines-definitely-not-here");

const local = yield* workflow.localStatus({ cwd: gone });
const full = yield* workflow.status({ cwd: gone });

assert.isFalse(local.isRepo);
assert.isTrue(local.pathMissing);
assert.isTrue(full.pathMissing);
}).pipe(
Effect.provide(
makeLayer({
detect: () => Effect.succeed(null),
}),
),
),
);

it.effect("leaves pathMissing unset for a directory that is simply not a repository", () =>
Effect.gen(function* () {
const workflow = yield* GitWorkflowService.GitWorkflowService;
const status = yield* workflow.localStatus({ cwd: NodeOS.tmpdir() });

assert.isFalse(status.isRepo);
assert.isUndefined(status.pathMissing);
}).pipe(
Effect.provide(
makeLayer({
detect: () => Effect.succeed(null),
}),
),
),
);

it.effect("does not call GitManager status methods when no VCS repository is detected", () => {
const localStatus = vi.fn();
const remoteStatus = vi.fn();
const status = vi.fn();

const testLayer = GitWorkflowService.layer.pipe(
Layer.provideMerge(NodeServices.layer),
Layer.provide(
Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({
detect: () => Effect.succeed(null),
Expand Down Expand Up @@ -215,6 +261,7 @@ describe("GitWorkflowService", () => {
});

const testLayer = GitWorkflowService.layer.pipe(
Layer.provideMerge(NodeServices.layer),
Layer.provide(
Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({
resolve: () => Effect.succeed(makeGitHandle()),
Expand Down Expand Up @@ -252,6 +299,7 @@ describe("GitWorkflowService", () => {
const mergeRef = vi.fn();
const pushCurrentBranch = vi.fn();
const testLayer = GitWorkflowService.layer.pipe(
Layer.provideMerge(NodeServices.layer),
Layer.provide(
Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({
resolve: () => Effect.succeed(makeGitHandle()),
Expand Down
26 changes: 21 additions & 5 deletions apps/server/src/git/GitWorkflowService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as nodePath from "node:path";

import * as Context from "effect/Context";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";

import {
Expand Down Expand Up @@ -66,6 +67,7 @@ import {
type GitWorktreeEntry,
} from "../vcs/GitVcsDriver.ts";
import { VcsDriverRegistry, type VcsDriverHandle } from "../vcs/VcsDriverRegistry.ts";
import { checkoutPresence } from "../vcs/CheckoutPresence.ts";

export interface GitWorkflowServiceShape {
readonly status: (
Expand Down Expand Up @@ -189,9 +191,10 @@ const unsupportedGitCommand = (operation: string, cwd: string, detail: string) =
detail,
});

function nonRepositoryLocalStatus(): VcsStatusLocalResult {
function nonRepositoryLocalStatus(pathMissing = false): VcsStatusLocalResult {
return {
isRepo: false,
...(pathMissing ? { pathMissing: true } : {}),
hasPrimaryRemote: false,
isDefaultRef: false,
refName: null,
Expand All @@ -205,9 +208,9 @@ function nonRepositoryLocalStatus(): VcsStatusLocalResult {
};
}

function nonRepositoryStatus(): VcsStatusResult {
function nonRepositoryStatus(pathMissing = false): VcsStatusResult {
return {
...nonRepositoryLocalStatus(),
...nonRepositoryLocalStatus(pathMissing),
hasUpstream: false,
aheadCount: 0,
behindCount: 0,
Expand Down Expand Up @@ -237,6 +240,19 @@ export const make = Effect.fn("makeGitWorkflowService")(function* () {
const registry = yield* VcsDriverRegistry;
const git = yield* GitVcsDriver;
const gitManager = yield* GitManager;
const fileSystem = yield* FileSystem.FileSystem;

/**
* "Not a repository" and "not there at all" both arrive here as a null
* driver handle, and the UI turns the first into an "Initialize Git" call to
* action. Offering that for a checkout the user's agent deleted is worse than
* unhelpful, so the two are separated before the status leaves the server.
*/
const isPathMissing = (cwd: string) =>
checkoutPresence(cwd).pipe(
Effect.provideService(FileSystem.FileSystem, fileSystem),
Effect.map((presence) => presence === "missing"),
);

const ensureGit = Effect.fn("GitWorkflowService.ensureGit")(function* (
operation: string,
Expand Down Expand Up @@ -356,7 +372,7 @@ export const make = Effect.fn("makeGitWorkflowService")(function* () {
? gitManager
.status(input)
.pipe(Effect.map((status) => withRepositoryContext(status, input.cwd, handle)))
: Effect.succeed(nonRepositoryStatus()),
: isPathMissing(input.cwd).pipe(Effect.map(nonRepositoryStatus)),
),
),
localStatus: (input) =>
Expand All @@ -366,7 +382,7 @@ export const make = Effect.fn("makeGitWorkflowService")(function* () {
? gitManager
.localStatus(input)
.pipe(Effect.map((status) => withRepositoryContext(status, input.cwd, handle)))
: Effect.succeed(nonRepositoryLocalStatus()),
: isPathMissing(input.cwd).pipe(Effect.map(nonRepositoryLocalStatus)),
),
),
remoteStatus: (input, options) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ describe("CheckpointReactor", () => {
refreshStatus: () => Effect.die("refreshStatus should not be called in this test"),
streamStatus: () => Stream.empty,
observeLocalStatus: () => Stream.empty,
observeMissingCheckouts: () => Stream.empty,
});

const layer = CheckpointReactorLive.pipe(
Expand Down
Loading
Loading