-
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 7 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,51 @@ | ||
| import { ensureGitRepository } from "./git.mjs"; | ||
| import fs from "node:fs"; | ||
| import path from "node:path"; | ||
|
|
||
| export function resolveWorkspaceRoot(cwd) { | ||
| export function resolveWorkspaceRoot(cwd, env = process.env) { | ||
| try { | ||
| return ensureGitRepository(cwd); | ||
| if (env?.GIT_DIR && env?.GIT_WORK_TREE) { | ||
| try { | ||
| const configuredGitDirectory = path.resolve(cwd, env.GIT_DIR); | ||
| const configuredWorkTree = path.resolve(cwd, env.GIT_WORK_TREE); | ||
| if (fs.statSync(configuredGitDirectory).isDirectory() && fs.statSync(configuredWorkTree).isDirectory()) { | ||
|
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 Useful? React with 👍 / 👎. |
||
| return fs.realpathSync.native(configuredWorkTree); | ||
| } | ||
| } catch { | ||
| // Invalid Git environment overrides do not suppress normal marker discovery. | ||
| } | ||
| } | ||
|
|
||
| const canonicalCwd = fs.realpathSync.native(cwd); | ||
| const cwdStats = fs.statSync(canonicalCwd); | ||
| let current = cwdStats.isFile() ? path.dirname(canonicalCwd) : canonicalCwd; | ||
|
|
||
| while (true) { | ||
| try { | ||
| const markerPath = path.join(current, ".git"); | ||
| const markerStats = fs.statSync(markerPath); | ||
|
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 is configured with Useful? React with 👍 / 👎. |
||
| let validGitFile = false; | ||
| if (markerStats.isFile()) { | ||
| const match = /^gitdir: (.+?)(?:\r?\n|$)/.exec(fs.readFileSync(markerPath, "utf8")); | ||
| if (match) { | ||
| const gitDirectory = path.resolve(current, match[1]); | ||
| validGitFile = fs.statSync(gitDirectory).isDirectory(); | ||
| } | ||
| } | ||
| if (markerStats.isDirectory() || validGitFile) { | ||
| 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 cwd; | ||
| } | ||
| current = parent; | ||
| } | ||
| } catch { | ||
| return cwd; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,153 @@ | ||
| 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 ignores GIT_WORK_TREE without GIT_DIR", () => { | ||
| const outer = makeNestedWorkspace("directory"); | ||
| assert.equal(resolveWorkspaceRoot(outer.nested, { GIT_WORK_TREE: "/tmp/unrelated" }), outer.workspace); | ||
| }); | ||
|
|
||
| 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 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 invoked inside a real repository with only
GIT_WORK_TREEset (for example, fromrepo/packages/appwithGIT_WORK_TREE=/tmp/wt), Git treats that variable as the working-tree root—its docs say it sets “the path to the root of the working tree” (Git environment docs)—but this pair-only guard skips it and the marker walk returnsrepo. Before this change,ensureGitRepositorydelegated togit rev-parse --show-toplevel, so task/status state and the app-server sandbox used the configured worktree; now they are incorrectly scoped to the metadata checkout. Handle the valid work-tree-only case after confirming the repository marker.Useful? React with 👍 / 👎.