-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix(session): resolve workspaces without git #716
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.
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
090f28b
e965ca9
2667b20
0106a64
2dba26a
39c6c2a
fe4f2a4
4718eda
8023616
8e6a998
6f45241
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,67 @@ | ||
| import { ensureGitRepository } from "./git.mjs"; | ||
| import fs from "node:fs"; | ||
| import path from "node:path"; | ||
|
|
||
| export function resolveWorkspaceRoot(cwd) { | ||
| function resolvesToGitDirectory(candidatePath) { | ||
| const candidateStats = fs.statSync(candidatePath); | ||
| if (candidateStats.isDirectory()) { | ||
| return true; | ||
| } | ||
| if (!candidateStats.isFile()) { | ||
| return false; | ||
| } | ||
|
|
||
| const match = /^gitdir: (.+?)(?:\r?\n|$)/.exec(fs.readFileSync(candidatePath, "utf8")); | ||
| if (!match) { | ||
| return false; | ||
| } | ||
| const target = path.resolve(path.dirname(candidatePath), match[1]); | ||
| return fs.statSync(target).isDirectory(); | ||
| } | ||
|
|
||
| function findMarkerWorkspace(canonicalCwd) { | ||
| const cwdStats = fs.statSync(canonicalCwd); | ||
| let current = cwdStats.isFile() ? path.dirname(canonicalCwd) : canonicalCwd; | ||
|
|
||
| while (true) { | ||
| try { | ||
| if (resolvesToGitDirectory(path.join(current, ".git"))) { | ||
| return fs.realpathSync.native(current); | ||
| } | ||
| } catch (error) { | ||
| if (error?.code !== "ENOENT" && error?.code !== "ENOTDIR") { | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| const parent = path.dirname(current); | ||
| if (parent === current) { | ||
| return null; | ||
| } | ||
| current = parent; | ||
| } | ||
| } | ||
|
|
||
| export function resolveWorkspaceRoot(cwd, env = process.env) { | ||
| try { | ||
| return ensureGitRepository(cwd); | ||
| const canonicalCwd = fs.realpathSync.native(cwd); | ||
| const markerWorkspace = findMarkerWorkspace(canonicalCwd); | ||
|
|
||
| if (env?.GIT_WORK_TREE) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a process sets a valid Useful? React with 👍 / 👎. |
||
| try { | ||
| const configuredWorkTree = path.resolve(cwd, env.GIT_WORK_TREE); | ||
| const configuredGitDirectory = env.GIT_DIR ? path.resolve(cwd, env.GIT_DIR) : null; | ||
| const hasRepository = configuredGitDirectory | ||
| ? resolvesToGitDirectory(configuredGitDirectory) | ||
| : Boolean(markerWorkspace); | ||
| if (hasRepository && fs.statSync(configuredWorkTree).isDirectory()) { | ||
| return fs.realpathSync.native(configuredWorkTree); | ||
| } | ||
| } catch { | ||
| // Invalid Git environment overrides do not suppress normal marker discovery. | ||
| } | ||
| } | ||
|
|
||
| return markerWorkspace ?? cwd; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a checkout's Useful? React with 👍 / 👎. |
||
| } catch { | ||
| return cwd; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| import childProcess from "node:child_process"; | ||
| import fs from "node:fs"; | ||
| import path from "node:path"; | ||
| import { syncBuiltinESMExports } from "node:module"; | ||
| import test from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
|
|
||
| import { makeTempDir } from "./helpers.mjs"; | ||
| import { resolveStateDir } from "../plugins/codex/scripts/lib/state.mjs"; | ||
| import { resolveWorkspaceRoot } from "../plugins/codex/scripts/lib/workspace.mjs"; | ||
|
|
||
| function makeNestedWorkspace(gitMarker) { | ||
| const workspace = makeTempDir(); | ||
| const nested = path.join(workspace, "packages", "app"); | ||
| fs.mkdirSync(nested, { recursive: true }); | ||
| if (gitMarker === "directory") { | ||
| fs.mkdirSync(path.join(workspace, ".git")); | ||
| } else { | ||
| const gitDirectory = path.join(workspace, "metadata.git"); | ||
| fs.mkdirSync(gitDirectory); | ||
| fs.writeFileSync(path.join(workspace, ".git"), "gitdir: metadata.git\n", "utf8"); | ||
| } | ||
| return { workspace: fs.realpathSync.native(workspace), nested }; | ||
| } | ||
|
|
||
| test("resolveWorkspaceRoot honors an environment-configured work tree without a .git marker", () => { | ||
| const worktree = makeTempDir(); | ||
| const gitDirectory = makeTempDir(); | ||
| const nested = path.join(worktree, "packages", "app"); | ||
| fs.mkdirSync(nested, { recursive: true }); | ||
|
|
||
| assert.equal( | ||
| resolveWorkspaceRoot(nested, { GIT_DIR: gitDirectory, GIT_WORK_TREE: "../.." }), | ||
| fs.realpathSync.native(worktree) | ||
| ); | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot honors GIT_WORK_TREE alone after finding the current repository", () => { | ||
| const outer = makeNestedWorkspace("directory"); | ||
| const configuredWorktree = makeTempDir(); | ||
| assert.equal( | ||
| resolveWorkspaceRoot(outer.nested, { GIT_WORK_TREE: configuredWorktree }), | ||
| fs.realpathSync.native(configuredWorktree) | ||
| ); | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot ignores an environment work tree with a missing GIT_DIR", () => { | ||
| const outer = makeNestedWorkspace("directory"); | ||
| assert.equal( | ||
| resolveWorkspaceRoot(outer.nested, { GIT_DIR: "missing.git", GIT_WORK_TREE: "../.." }), | ||
| outer.workspace | ||
| ); | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot follows a gitfile supplied through GIT_DIR", () => { | ||
| const cwd = makeTempDir(); | ||
| const metadata = makeTempDir(); | ||
| const gitDirectory = path.join(metadata, "actual.git"); | ||
| const gitfile = path.join(metadata, "linked.git"); | ||
| const worktree = makeTempDir(); | ||
| fs.mkdirSync(gitDirectory); | ||
| fs.writeFileSync(gitfile, "gitdir: actual.git\n", "utf8"); | ||
|
|
||
| assert.equal( | ||
| resolveWorkspaceRoot(cwd, { GIT_DIR: gitfile, GIT_WORK_TREE: worktree }), | ||
| fs.realpathSync.native(worktree) | ||
| ); | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot ignores GIT_WORK_TREE alone outside a repository", () => { | ||
| const cwd = makeTempDir(); | ||
| const configuredWorktree = makeTempDir(); | ||
| assert.equal(resolveWorkspaceRoot(cwd, { GIT_WORK_TREE: configuredWorktree }), cwd); | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot discovers a checkout without starting a child process", () => { | ||
| const { workspace, nested } = makeNestedWorkspace("directory"); | ||
| const originalSpawnSync = childProcess.spawnSync; | ||
| childProcess.spawnSync = () => { | ||
| throw new Error("workspace resolution must not start a child process"); | ||
| }; | ||
| syncBuiltinESMExports(); | ||
|
|
||
| try { | ||
| assert.equal(resolveWorkspaceRoot(nested), workspace); | ||
| } finally { | ||
| childProcess.spawnSync = originalSpawnSync; | ||
| syncBuiltinESMExports(); | ||
| } | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot accepts a linked-worktree gitfile", () => { | ||
| const { workspace, nested } = makeNestedWorkspace("file"); | ||
|
|
||
| assert.equal(resolveWorkspaceRoot(nested), workspace); | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot ignores an ordinary file named .git", () => { | ||
| const outer = makeNestedWorkspace("directory"); | ||
| const nestedWorkspace = path.join(outer.workspace, "vendor", "not-a-repo"); | ||
| const cwd = path.join(nestedWorkspace, "src"); | ||
| fs.mkdirSync(cwd, { recursive: true }); | ||
| fs.writeFileSync(path.join(nestedWorkspace, ".git"), "not a gitfile\n", "utf8"); | ||
|
|
||
| assert.equal(resolveWorkspaceRoot(cwd), outer.workspace); | ||
| }); | ||
|
|
||
| for (const invalidMarker of ["gitdir:/tmp/metadata\n", "gitdir:\t/tmp/metadata\n", "metadata\ngitdir: /tmp/metadata\n", "gitdir: missing.git\n"]) { | ||
| test(`resolveWorkspaceRoot rejects malformed gitfile ${JSON.stringify(invalidMarker)}`, () => { | ||
| const outer = makeNestedWorkspace("directory"); | ||
| const nestedWorkspace = path.join(outer.workspace, "vendor", "not-a-repo"); | ||
| const cwd = path.join(nestedWorkspace, "src"); | ||
| fs.mkdirSync(cwd, { recursive: true }); | ||
| fs.writeFileSync(path.join(nestedWorkspace, ".git"), invalidMarker, "utf8"); | ||
|
|
||
| assert.equal(resolveWorkspaceRoot(cwd), outer.workspace); | ||
| }); | ||
| } | ||
|
|
||
| test("resolveWorkspaceRoot follows a symlinked git directory", () => { | ||
| const workspace = makeTempDir(); | ||
| const gitDirectory = makeTempDir(); | ||
| const nested = path.join(workspace, "packages", "app"); | ||
| fs.mkdirSync(nested, { recursive: true }); | ||
| fs.symlinkSync(gitDirectory, path.join(workspace, ".git"), process.platform === "win32" ? "junction" : "dir"); | ||
|
|
||
| assert.equal(resolveWorkspaceRoot(nested), fs.realpathSync.native(workspace)); | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot selects the nearest nested working tree", () => { | ||
| const { workspace } = makeNestedWorkspace("directory"); | ||
| const nestedWorkspace = path.join(workspace, "vendor", "nested"); | ||
| const cwd = path.join(nestedWorkspace, "src"); | ||
| fs.mkdirSync(path.join(nestedWorkspace, ".git"), { recursive: true }); | ||
| fs.mkdirSync(cwd); | ||
|
|
||
| assert.equal(resolveWorkspaceRoot(cwd), fs.realpathSync.native(nestedWorkspace)); | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot starts at a file cwd's parent", () => { | ||
| const { workspace, nested } = makeNestedWorkspace("directory"); | ||
| const file = path.join(nested, "index.js"); | ||
| fs.writeFileSync(file, "export {};\n", "utf8"); | ||
|
|
||
| assert.equal(resolveWorkspaceRoot(file), workspace); | ||
| }); | ||
|
|
||
| test("workspace aliases resolve to the same canonical root and state identity", (t) => { | ||
| const { workspace } = makeNestedWorkspace("directory"); | ||
| const nested = path.join(workspace, "packages", "app"); | ||
| const alias = `${workspace}-alias`; | ||
| try { | ||
| fs.symlinkSync(workspace, alias, process.platform === "win32" ? "junction" : "dir"); | ||
| } catch (error) { | ||
| if (process.platform === "win32" && ["EPERM", "EACCES"].includes(error?.code)) { | ||
| t.skip(`junction creation unavailable: ${error.code}`); | ||
| return; | ||
| } | ||
| throw error; | ||
| } | ||
| const aliasedNested = path.join(alias, "packages", "app"); | ||
|
|
||
| assert.equal(resolveWorkspaceRoot(aliasedNested), workspace); | ||
| assert.equal(resolveWorkspaceRoot(aliasedNested), resolveWorkspaceRoot(nested)); | ||
| assert.equal(resolveStateDir(aliasedNested), resolveStateDir(nested)); | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot preserves a non-repository cwd", () => { | ||
| const cwd = makeTempDir(); | ||
|
|
||
| assert.equal(resolveWorkspaceRoot(cwd), cwd); | ||
| }); | ||
|
|
||
| test("resolveWorkspaceRoot preserves an inaccessible or missing cwd", () => { | ||
| const cwd = path.join(makeTempDir(), "missing", "directory"); | ||
|
|
||
| assert.equal(resolveWorkspaceRoot(cwd), cwd); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a nested
.gitfile contains an otherwise valid header followed by extra content (for example,gitdir: /valid/.git\ntrailing-data), Git rejects the gitfile, but this expression accepts only its first line andfindMarkerWorkspaceselects the nested directory as a workspace. That creates a bogus state/job namespace and scopes task sandboxing to the child instead of continuing to the enclosing valid workspace; require the match to consume the complete gitfile.Useful? React with 👍 / 👎.