-
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 3 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,32 @@ | ||
| import { ensureGitRepository } from "./git.mjs"; | ||
| import fs from "node:fs"; | ||
| import path from "node:path"; | ||
|
|
||
| export function resolveWorkspaceRoot(cwd) { | ||
| try { | ||
| return ensureGitRepository(cwd); | ||
| 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); | ||
| const validGitFile = markerStats.isFile() && /^gitdir:\s*\S/m.test(fs.readFileSync(markerPath, "utf8")); | ||
|
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 non-repository child directory within an outer checkout contains an ordinary Useful? React with 👍 / 👎. |
||
| 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,114 @@ | ||
| 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 { | ||
| fs.writeFileSync(path.join(workspace, ".git"), "gitdir: /tmp/example.git\n", "utf8"); | ||
| } | ||
| return { workspace: fs.realpathSync.native(workspace), nested }; | ||
| } | ||
|
|
||
| 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); | ||
| }); | ||
|
|
||
| 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 checkout is configured with
GIT_DIRandGIT_WORK_TREEbut has no.gitmarker in the work tree, Git still resolvesrev-parse --show-toplevelto the configured work-tree root, whereas this walk returns each caller's own directory. A task launched fromwork/subis therefore stored and sandboxed undersub, while/codex:statusor task resume run fromworkuses a separate state directory; preserve an environment-aware workspace-root path before falling back to marker discovery.Useful? React with 👍 / 👎.