Repository navigation
fix(cli): resolve workspaceDir and origin remoteUrl via git in worktrees #5105
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import { readdir } from "node:fs/promises"; | |
| import { basename, dirname, isAbsolute, join, relative } from "node:path"; | ||
| import { findWorkspaceDir, resolveLockfile, resolvePackageJSON, resolveTSConfig } from "pkg-types"; | ||
| import { generateCode, loadFile } from "./imports/magicast.js"; | ||
| import { getGitTopLevel } from "./utilities/gitMeta.js"; | ||
| import { logger } from "./utilities/logger.js"; | ||
| import { | ||
| additionalFiles, | ||
|
|
@@ -203,7 +204,7 @@ async function resolveConfig( | |
| const packageJsonPath = await resolvePackageJSON(cwd); | ||
| const tsconfigPath = await safeResolveTsConfig(cwd); | ||
| const lockfilePath = await resolveLockfile(cwd); | ||
| const workspaceDir = await findWorkspaceDir(cwd); | ||
| const workspaceDir = (await getGitTopLevel(cwd)) ?? (await findWorkspaceDir(cwd)); | ||
|
Contributor
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. 🟥 Nested workspace secrets enter deployment archives For a nested workspace, Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| const workingDir = result.configFile | ||
| ? dirname(result.configFile) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,20 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { getCiBranch, sanitizeGitRemoteUrl } from "./gitMeta.js"; | ||
| import { execFileSync } from "node:child_process"; | ||
| import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join } from "node:path"; | ||
| import { afterEach, describe, expect, it, vi } from "vitest"; | ||
| import { createGitMeta, getCiBranch, getGitTopLevel, sanitizeGitRemoteUrl } from "./gitMeta.js"; | ||
|
|
||
| const tempDirs: string[] = []; | ||
|
|
||
| afterEach(async () => { | ||
| vi.unstubAllEnvs(); | ||
| await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); | ||
| }); | ||
|
|
||
| function runGit(cwd: string, args: string[]): string { | ||
| return execFileSync("git", args, { cwd, encoding: "utf8" }).trim(); | ||
| } | ||
|
|
||
| describe("sanitizeGitRemoteUrl", () => { | ||
| it.each([ | ||
|
|
@@ -87,3 +102,103 @@ describe("getCiBranch", () => { | |
| ).toBe("feature/login"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("git worktree support", () => { | ||
| async function createRepoWithWorktrees() { | ||
| const baseDir = await mkdtemp(join(tmpdir(), "trigger-git-worktree-")); | ||
| tempDirs.push(baseDir); | ||
|
|
||
| const repoDir = join(baseDir, "repo"); | ||
| await mkdir(repoDir, { recursive: true }); | ||
|
|
||
| runGit(repoDir, ["init", "-b", "main"]); | ||
| runGit(repoDir, ["config", "user.name", "Test User"]); | ||
| runGit(repoDir, ["config", "user.email", "test@example.com"]); | ||
| runGit(repoDir, [ | ||
| "remote", | ||
| "add", | ||
| "origin", | ||
| "https://token@example.com/triggerdotdev/trigger.dev.git", | ||
| ]); | ||
|
|
||
| await writeFile(join(repoDir, ".gitignore"), ".worktrees/\n"); | ||
| await writeFile(join(repoDir, "index.ts"), "export const version = 1;\n"); | ||
| runGit(repoDir, ["add", "."]); | ||
| runGit(repoDir, ["commit", "-m", "initial main commit"]); | ||
| const mainSha = runGit(repoDir, ["rev-parse", "HEAD"]); | ||
|
|
||
| const externalWorktreeDir = join(baseDir, "worktree-external"); | ||
| runGit(repoDir, ["worktree", "add", "-b", "release/external", externalWorktreeDir]); | ||
| await writeFile(join(externalWorktreeDir, "index.ts"), "export const version = 2;\n"); | ||
| runGit(externalWorktreeDir, ["add", "index.ts"]); | ||
| runGit(externalWorktreeDir, ["commit", "-m", "external worktree commit"]); | ||
| const externalSha = runGit(externalWorktreeDir, ["rev-parse", "HEAD"]); | ||
|
|
||
| const nestedWorktreeDir = join(repoDir, ".worktrees", "release"); | ||
| runGit(repoDir, ["worktree", "add", "-b", "release/nested", nestedWorktreeDir]); | ||
| await writeFile(join(nestedWorktreeDir, "index.ts"), "export const version = 3;\n"); | ||
| runGit(nestedWorktreeDir, ["add", "index.ts"]); | ||
| runGit(nestedWorktreeDir, ["commit", "-m", "nested worktree commit"]); | ||
| const nestedSha = runGit(nestedWorktreeDir, ["rev-parse", "HEAD"]); | ||
|
|
||
| // Make the outer checkout dirty to verify nested worktrees do not inherit its status | ||
| await writeFile(join(repoDir, "index.ts"), "export const version = 99;\n"); | ||
|
|
||
| return { | ||
| repoDir, | ||
| mainSha, | ||
| externalWorktreeDir, | ||
| externalSha, | ||
| nestedWorktreeDir, | ||
| nestedSha, | ||
| }; | ||
| } | ||
|
|
||
| it("reads sanitized origin remoteUrl and commit metadata from an external linked worktree", async () => { | ||
| vi.stubEnv("GITHUB_ACTIONS", ""); | ||
| vi.stubEnv("TRIGGER_GITHUB_APP", ""); | ||
|
Comment on lines
+158
to
+159
Contributor
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. 🟡 Worktree tests stub CI state Both new worktree tests use Learn moreThe worktree integration tests mutate process-wide CI variables with Example: In a GitHub Actions run, the first worktree test replaces Recommended fix: Make the local Git metadata path testable without stubbing process environment, such as by passing an explicit environment or metadata source to the relevant function and using that option in both tests. Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| const { externalWorktreeDir, externalSha } = await createRepoWithWorktrees(); | ||
|
|
||
| await expect(getGitTopLevel(externalWorktreeDir)).resolves.toBe(externalWorktreeDir); | ||
| await expect(createGitMeta(externalWorktreeDir)).resolves.toEqual({ | ||
| source: "local", | ||
| remoteUrl: "https://example.com/triggerdotdev/trigger.dev.git", | ||
| commitAuthorName: "Test User", | ||
| commitMessage: "external worktree commit", | ||
| commitRef: "release/external", | ||
| commitSha: externalSha, | ||
| dirty: false, | ||
| }); | ||
| }); | ||
|
|
||
| it("resolves nested worktree root and metadata instead of the outer checkout", async () => { | ||
| vi.stubEnv("GITHUB_ACTIONS", ""); | ||
| vi.stubEnv("TRIGGER_GITHUB_APP", ""); | ||
|
|
||
| const { repoDir, mainSha, nestedWorktreeDir, nestedSha } = await createRepoWithWorktrees(); | ||
| const nestedSubdir = join(nestedWorktreeDir, "packages", "app"); | ||
| await mkdir(nestedSubdir, { recursive: true }); | ||
|
|
||
| await expect(getGitTopLevel(nestedWorktreeDir)).resolves.toBe(nestedWorktreeDir); | ||
| await expect(getGitTopLevel(nestedSubdir)).resolves.toBe(nestedWorktreeDir); | ||
|
|
||
| const outerMeta = await createGitMeta(repoDir); | ||
| expect(outerMeta).toMatchObject({ | ||
| commitRef: "main", | ||
| commitSha: mainSha, | ||
| dirty: true, | ||
| }); | ||
|
|
||
| const nestedMeta = await createGitMeta(nestedWorktreeDir); | ||
| expect(nestedMeta).toEqual({ | ||
| source: "local", | ||
| remoteUrl: "https://example.com/triggerdotdev/trigger.dev.git", | ||
| commitAuthorName: "Test User", | ||
| commitMessage: "nested worktree commit", | ||
| commitRef: "release/nested", | ||
| commitSha: nestedSha, | ||
| dirty: false, | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import fs from "fs/promises"; | ||
| import { join } from "path"; | ||
| import { join, resolve } from "path"; | ||
| import ini from "ini"; | ||
| import git from "git-last-commit"; | ||
| import { x } from "tinyexec"; | ||
|
|
@@ -15,7 +15,7 @@ export async function createGitMeta(directory: string): Promise<GitMeta | undefi | |
| } | ||
|
|
||
| // Fall back to git commands for local development | ||
| const remoteUrl = await getOriginUrl(join(directory, ".git/config")); | ||
| const remoteUrl = await getOriginUrl(directory); | ||
|
|
||
| const [commitResult, dirtyResult] = await Promise.allSettled([ | ||
| getLastCommit(directory), | ||
|
|
@@ -125,6 +125,33 @@ async function isDirty(directory: string): Promise<boolean> { | |
| } | ||
| } | ||
|
|
||
| export async function getGitTopLevel(directory: string): Promise<string | undefined> { | ||
| try { | ||
| const result = await x( | ||
| "git", | ||
| ["--no-optional-locks", "rev-parse", "--show-toplevel", "--show-cdup"], | ||
| { | ||
| nodeOptions: { | ||
| cwd: directory, | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| if (result.exitCode !== 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const [topLevel, cdup = ""] = result.stdout.trimEnd().split(/\r?\n/); | ||
| if (!topLevel) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return resolve(directory, cdup || "."); | ||
|
Contributor
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. 🟥 Symlinked projects upload outside files For a symlink to a repository subdirectory, Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| async function parseGitConfig(configPath: string) { | ||
| try { | ||
| return ini.parse(await fs.readFile(configPath, "utf8")); | ||
|
|
@@ -138,8 +165,25 @@ function pluckOriginUrl(gitConfig: { [key: string]: any }): string | undefined { | |
| return gitConfig['remote "origin"']?.url; | ||
| } | ||
|
|
||
| async function getOriginUrl(configPath: string): Promise<string | null> { | ||
| const gitConfig = await parseGitConfig(configPath); | ||
| async function getOriginUrl(directory: string): Promise<string | null> { | ||
| try { | ||
| const result = await x("git", ["--no-optional-locks", "config", "--get", "remote.origin.url"], { | ||
|
Contributor
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. |
||
| nodeOptions: { | ||
| cwd: directory, | ||
| }, | ||
| }); | ||
|
|
||
| if (result.exitCode === 0) { | ||
| const originUrl = result.stdout.trim(); | ||
| if (originUrl) { | ||
| return originUrl; | ||
| } | ||
| } | ||
| } catch { | ||
| // Fall back to parsing .git/config if invoking git fails | ||
| } | ||
|
|
||
| const gitConfig = await parseGitConfig(join(directory, ".git/config")); | ||
| if (!gitConfig) { | ||
| return null; | ||
| } | ||
|
|
||
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.
🟡 Worktree fix absent from release notes
workspaceDirchanges deployment behavior in linked worktrees, but this public CLI fix has no changeset. Customers cannot find the fix in release notes.Learn more
This public CLI fix changes whether users can deploy from linked worktrees. The repository requires a patch changeset for user-facing bug fixes in public packages; none was added here. Without it, the release notes omit the fix.
Example: A user blocked from deploying in a nested worktree upgrades
trigger.dev. The upgrade fixes their deployment, but the published changelog does not tell them about it.Recommended fix: Add a patch changeset for
trigger.devdescribing the restored ability to deploy from linked Git worktrees in user-facing language.Was this helpful? React with 👍 or 👎 to provide feedback.