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 @@ -7,7 +7,10 @@ import {
getWorktreeBranchAtPath,
listWorktreeBranches,
} from "../shared/branch-search";
import { requireLocalProject } from "../shared/local-project";
import {
requireLocalProject,
requireProjectRepoPath,
} from "../shared/local-project";
import type { TerminalDescriptor } from "../shared/types";

/**
Expand All @@ -21,7 +24,8 @@ export const adopt = protectedProcedure
.input(adoptInputSchema)
.mutation(async ({ ctx, input }) => {
const localProject = requireLocalProject(ctx, input.projectId);
await ensureMainWorkspace(ctx, input.projectId, localProject.repoPath);
const repoPath = requireProjectRepoPath(localProject);
await ensureMainWorkspace(ctx, input.projectId, repoPath);

let branch = input.branch.trim();
if (!branch) {
Expand All @@ -31,7 +35,7 @@ export const adopt = protectedProcedure
});
}

const git = await ctx.git(localProject.repoPath);
const git = await ctx.git(repoPath);

let worktreePath: string;
if (input.worktreePath) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import { and, eq, isNull } from "drizzle-orm";
import { z } from "zod";
import { workspaces } from "../../../../db/schema";
import { protectedProcedure } from "../../../index";
import { requireLocalProject } from "../shared/local-project";
import {
requireLocalProject,
requireProjectRepoPath,
} from "../shared/local-project";
import {
listGitWorktrees,
normalizeWorktreePath,
Expand All @@ -20,7 +23,7 @@ export const listProjectWorktrees = protectedProcedure
.input(z.object({ projectId: z.string() }))
.query(async ({ ctx, input }) => {
const localProject = requireLocalProject(ctx, input.projectId);
const git = await ctx.git(localProject.repoPath);
const git = await ctx.git(requireProjectRepoPath(localProject));
const records = await listGitWorktrees(git);

// Tracking key is (projectId, branch) — same as searchBranches — but
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import {
markRefetchRemote,
shouldRefetchRemote,
} from "../shared/branch-search";
import { findLocalProject } from "../shared/local-project";
import {
findLocalProject,
requireProjectRepoPath,
} from "../shared/local-project";
import type { BranchRow } from "../shared/types";

type BranchAccum = {
Expand All @@ -36,7 +39,7 @@ export const searchBranches = protectedProcedure
};
}

const git = await ctx.git(localProject.repoPath);
const git = await ctx.git(requireProjectRepoPath(localProject));

// Honor `refresh` only if TTL elapsed — prevents thrashing `git fetch`
// on every keystroke when the client tags first-page requests.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { existsSync } from "node:fs";
import { TRPCError } from "@trpc/server";
import { eq } from "drizzle-orm";
import { projects } from "../../../../db/schema";
import type { HostServiceContext } from "../../../../types";
Expand All @@ -24,3 +26,17 @@ export function requireLocalProject(
}
return localProject;
}

// A project directory deleted or moved outside the app is a routine
// lifecycle state, not a bug — classify it here so simple-git's construct
// error can't escape a workspace-creation procedure as a reportable 500.
export function requireProjectRepoPath(localProject: LocalProject): string {

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.

P2: existsSync only checks that the path exists, not that it is a directory — it returns true for regular files too. Since simple-git itself validates the path with statSync(...).isDirectory() before throwing GitConstructError, a repoPath that resolves to a file (rather than a missing path) will pass this guard but still blow up with the original 500 downstream, defeating the purpose of this fix. Use statSync and check isDirectory(), treating ENOENT/ENOTDIR as PROJECT_DIR_MISSING and letting other filesystem errors propagate.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts, line 33:

<comment>`existsSync` only checks that the path exists, not that it is a directory — it returns `true` for regular files too. Since `simple-git` itself validates the path with `statSync(...).isDirectory()` before throwing `GitConstructError`, a `repoPath` that resolves to a file (rather than a missing path) will pass this guard but still blow up with the original 500 downstream, defeating the purpose of this fix. Use `statSync` and check `isDirectory()`, treating `ENOENT`/`ENOTDIR` as `PROJECT_DIR_MISSING` and letting other filesystem errors propagate.</comment>

<file context>
@@ -24,3 +26,17 @@ export function requireLocalProject(
+// A project directory deleted or moved outside the app is a routine
+// lifecycle state, not a bug — classify it here so simple-git's construct
+// error can't escape a workspace-creation procedure as a reportable 500.
+export function requireProjectRepoPath(localProject: LocalProject): string {
+	if (!existsSync(localProject.repoPath)) {
+		throw new TRPCError({
</file context>

if (!existsSync(localProject.repoPath)) {
throw new TRPCError({
code: "NOT_FOUND",
message: "Project directory no longer exists on disk",
cause: { kind: "PROJECT_DIR_MISSING", repoPath: localProject.repoPath },
});
}
Comment on lines +33 to +40

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.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

node <<'NODE'
const { existsSync, mkdtempSync, rmSync, statSync, writeFileSync } = require("node:fs");
const { join } = require("node:path");
const { tmpdir } = require("node:os");

const root = mkdtempSync(join(tmpdir(), "repo-path-check-"));
try {
  const filePath = join(root, "repo");
  writeFileSync(filePath, "");
  if (!existsSync(filePath) || statSync(filePath).isDirectory()) {
    throw new Error("Expected a regular file to exist without being a directory");
  }
} finally {
  rmSync(root, { recursive: true, force: true });
}
NODE

Repository: superset-sh/superset

Length of output: 158


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Locate the reviewed file and inspect the relevant implementation and nearby type/export context.
if [ -f "packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts" ]; then
  echo "== file exists =="
  stat -c '%n %s lines' packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts
  echo
  echo "== imports and exported functions outline =="
  ast-grep outline packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts || true
  echo
  echo "== relevant file contents =="
  sed -n '1,120p' packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts
else
  echo "Reviewed file not found"
  fd -a 'local-project.ts' .
fi

Repository: superset-sh/superset

Length of output: 218


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== node availability =="
node --version || true

echo "== file availability =="
if [ -f "packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts" ]; then
  echo "packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts"
  wc -l packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts
else
  fd -a 'local-project.ts' . | sed -n '1,50p'
fi

echo "== relevant file contents =="
sed -n '1,140p' packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts 2>/dev/null || true

echo "== related paths in workspace creation =="
rg -n "requireProjectRepoPath|localProject\.repoPath|repoPath" packages/host-service/src/trpc/router/workspace-creation packages/host-service/src 2>/dev/null | sed -n '1,120p' || true

echo "== fs behavior probe =="
node <<'NODE'
const fs = require("node:fs");
const { mkdtempSync, rmSync, writeFileSync, mkdirSync, unlinkSync } = require("node:fs");
const path = require("node:path");
const { tmpdir } = require("node:os");

const root = mkdtempSync(path.join(tmpdir(), "repo-path-check-"));
try {
  const filePath = path.join(root, "repo");
  const dirPath = path.join(root, "repo-dir");
  writeFileSync(filePath, "");
  mkdirSync(dirPath);
  
  console.log(JSON.stringify({
    existsSync_file: fs.existsSync(filePath),
    isDirectory_file: fs.statSync(filePath).isDirectory(),
    existsSync_dir: fs.existsSync(dirPath),
    isDirectory_dir: fs.statSync(dirPath).isDirectory(),
    existsSync_missing: fs.existsSync(path.join(root, "missing")),
  }));

  try {
    writeFileSync(path.join(root, "repo"), "x");
  } catch (error) {
    console.log(JSON.stringify({ write_same_file_error_code: error.code }));
  }

  try {
    fs.mkdirSync(filePath, { recursive: true });
  } catch (error) {
    console.log(JSON.stringify({ mkdir_over_existing_file_error_code: error.code }));
  }
} finally {
  rmSync(root, { recursive: true, force: true });
}
NODE

Repository: superset-sh/superset

Length of output: 17250


Validate that repoPath is a directory.

existsSync returns true for regular files, so /tmp/repo passes this guard when it is a file. Since callers pass repoPath into Git and workspace setup paths, use statSync(...).isDirectory() or an equivalent directory check. Treat ENOENT and ENOTDIR as PROJECT_DIR_MISSING; let other filesystem errors propagate.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts`
around lines 33 - 40, Update requireProjectRepoPath to validate that
localProject.repoPath is an actual directory, not merely an existing filesystem
entry. Use a directory stat check, map ENOENT and ENOTDIR failures to the
existing TRPCError with kind PROJECT_DIR_MISSING, and allow other filesystem
errors to propagate.

return localProject.repoPath;
}
Loading