Skip to content

Commit c85639b

Browse files
committed
fix(git): filter non-semver tags, check staged index, and verify tag commit on retry
- Filter out non-semver tags (e.g. pkg@latest) before calling semver.rcompare in getMostRecentPackageTag and getMostRecentPackageStableTag; stray tags previously caused semver.rcompare to throw a TypeError - In commitChanges, replace the isWorkingDirectoryClean check (which inspects the whole worktree including untracked files) with git diff --cached --name-only so we only proceed to git commit when something is actually staged - In createPackageTag, verify the existing local tag resolves to HEAD before treating it as an idempotent hit; a tag pointing to a different commit now falls through and lets git proceed rather than silently succeeding - Add regression test: getMostRecentPackageTag ignores non-semver tags like @latest and returns the correct highest semver tag
1 parent 71d25ff commit c85639b

File tree

2 files changed

+44
-13
lines changed

2 files changed

+44
-13
lines changed

src/core/git.ts

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -410,9 +410,13 @@ export async function commitChanges(
410410
},
411411
});
412412

413-
// Check if there are changes to commit
414-
const isClean = await isWorkingDirectoryClean(workspaceRoot);
415-
if (!isClean.ok || isClean.value) {
413+
// Check if anything was actually staged (git add -u only touches tracked files;
414+
// untracked files would cause isWorkingDirectoryClean to return false even when
415+
// nothing is staged, leading to a "nothing to commit" error from git commit).
416+
const staged = await run("git", ["diff", "--cached", "--name-only"], {
417+
nodeOptions: { cwd: workspaceRoot, stdio: "pipe" },
418+
});
419+
if (staged.stdout.trim() === "") {
416420
return ok(false);
417421
}
418422

@@ -568,12 +572,15 @@ export async function getMostRecentPackageTag(
568572
return ok(undefined);
569573
}
570574

571-
// Sort by semver descending so the highest version comes first
572-
const sorted = tags.sort((a, b) => {
573-
const va = a.slice(a.lastIndexOf("@") + 1);
574-
const vb = b.slice(b.lastIndexOf("@") + 1);
575-
return semver.rcompare(va, vb);
576-
});
575+
// Filter to valid semver only, then sort descending so the highest version comes first.
576+
// Non-semver tags (e.g. "pkg@latest") would cause semver.rcompare to throw.
577+
const sorted = tags
578+
.filter((t) => semver.valid(t.slice(t.lastIndexOf("@") + 1)))
579+
.sort((a, b) => {
580+
const va = a.slice(a.lastIndexOf("@") + 1);
581+
const vb = b.slice(b.lastIndexOf("@") + 1);
582+
return semver.rcompare(va, vb);
583+
});
577584
return ok(sorted[0]);
578585
} catch (error) {
579586
return err(toGitError("getMostRecentPackageTag", error));
@@ -595,7 +602,7 @@ export async function getMostRecentPackageStableTag(
595602
const tags = stdout
596603
.split("\n")
597604
.map((tag) => tag.trim())
598-
.filter(Boolean)
605+
.filter((tag) => Boolean(tag) && semver.valid(tag.slice(tag.lastIndexOf("@") + 1)))
599606
.sort((a, b) => {
600607
const va = a.slice(a.lastIndexOf("@") + 1);
601608
const vb = b.slice(b.lastIndexOf("@") + 1);
@@ -706,13 +713,23 @@ async function createPackageTag(
706713
const tagName = `${packageName}@${version}`;
707714

708715
try {
709-
// Check if this tag already exists locally — if so, skip creation (idempotent retry support)
716+
// Check if this tag already exists locally and points to the same commit as HEAD.
717+
// If it exists but points elsewhere, we must not silently skip — fall through and
718+
// let git tag fail or be overwritten as appropriate.
710719
const existingTagResult = await run("git", ["tag", "--list", tagName], {
711720
nodeOptions: { cwd: workspaceRoot, stdio: "pipe" },
712721
});
713722
if (existingTagResult.stdout.trim() === tagName) {
714-
logger.verbose(`Tag ${farver.green(tagName)} already exists locally, skipping creation`);
715-
return ok(undefined);
723+
// Verify the tag resolves to HEAD so we don't silently ignore a mispointed tag
724+
const [tagCommit, headCommit] = await Promise.all([
725+
run("git", ["rev-list", "-n1", tagName], { nodeOptions: { cwd: workspaceRoot, stdio: "pipe" } }),
726+
run("git", ["rev-parse", "HEAD"], { nodeOptions: { cwd: workspaceRoot, stdio: "pipe" } }),
727+
]);
728+
if (tagCommit.stdout.trim() === headCommit.stdout.trim()) {
729+
logger.verbose(`Tag ${farver.green(tagName)} already exists and points to HEAD, skipping creation`);
730+
return ok(undefined);
731+
}
732+
logger.verbose(`Tag ${farver.green(tagName)} exists but points to a different commit — proceeding`);
716733
}
717734

718735
logger.info(`Creating tag: ${farver.green(tagName)}`);

test/core/git.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,20 @@ describe("git utilities", () => {
325325
expect(result.value).toBe("my-package@1.10.0");
326326
});
327327

328+
it("should ignore non-semver tags like @latest", async () => {
329+
const mockExec = vi.mocked(tinyexec.exec);
330+
mockExec.mockResolvedValue({
331+
stdout: "my-package@latest\nmy-package@1.0.0\nmy-package@2.0.0\n",
332+
stderr: "",
333+
exitCode: 0,
334+
} as any);
335+
336+
const result = await getMostRecentPackageTag("/workspace", "my-package");
337+
338+
assert(result.ok);
339+
expect(result.value).toBe("my-package@2.0.0");
340+
});
341+
328342
it("should return undefined if no tag exists for package", async () => {
329343
const mockExec = vi.mocked(tinyexec.exec);
330344
mockExec.mockResolvedValue({

0 commit comments

Comments
 (0)