diff --git a/package-lock.json b/package-lock.json index e4c6b70..1d1a5bd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -102,6 +102,7 @@ "os": [ "aix" ], + "peer": true, "engines": { "node": ">=18" } @@ -119,6 +120,7 @@ "os": [ "android" ], + "peer": true, "engines": { "node": ">=18" } @@ -136,6 +138,7 @@ "os": [ "android" ], + "peer": true, "engines": { "node": ">=18" } @@ -153,6 +156,7 @@ "os": [ "android" ], + "peer": true, "engines": { "node": ">=18" } @@ -170,6 +174,7 @@ "os": [ "darwin" ], + "peer": true, "engines": { "node": ">=18" } @@ -187,6 +192,7 @@ "os": [ "darwin" ], + "peer": true, "engines": { "node": ">=18" } @@ -204,6 +210,7 @@ "os": [ "freebsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -221,6 +228,7 @@ "os": [ "freebsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -238,6 +246,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -255,6 +264,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -272,6 +282,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -289,6 +300,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -306,6 +318,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -323,6 +336,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -340,6 +354,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -357,6 +372,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -374,6 +390,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -391,6 +408,7 @@ "os": [ "netbsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -408,6 +426,7 @@ "os": [ "netbsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -425,6 +444,7 @@ "os": [ "openbsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -442,6 +462,7 @@ "os": [ "openbsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -459,6 +480,7 @@ "os": [ "openharmony" ], + "peer": true, "engines": { "node": ">=18" } @@ -476,6 +498,7 @@ "os": [ "sunos" ], + "peer": true, "engines": { "node": ">=18" } @@ -493,6 +516,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">=18" } @@ -510,6 +534,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">=18" } @@ -527,6 +552,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">=18" } diff --git a/packages/core-engine/src/services/source-authority-service.ts b/packages/core-engine/src/services/source-authority-service.ts index a136594..6294e0d 100644 --- a/packages/core-engine/src/services/source-authority-service.ts +++ b/packages/core-engine/src/services/source-authority-service.ts @@ -307,6 +307,25 @@ export class SourceAuthorityService { continue; } + const lockedCommit = this.readLockedCommitSha(lock.revision); + if (source.kind === "git" && lockedCommit) { + try { + const remoteCommit = await this.options.checkoutService.readGitRemoteHeadCommit( + source.locator, + lock.originBranch ? { branch: lock.originBranch } : {}, + ); + if (remoteCommit && remoteCommit === lockedCommit) { + updated.push(this.emptyUpdateResult(sourceId)); + continue; + } + } catch (error) { + warnings.push({ + code: "SOURCE_REMOTE_COMMIT_CHECK_FAILED", + message: `Unable to verify remote commit for '${sourceId}': ${String(error)}`, + }); + } + } + const tempCheckoutPath = path.join( this.options.stateStore.rootPath, "source", @@ -534,6 +553,10 @@ export class SourceAuthorityService { return kind; } + private readLockedCommitSha(revision: SourceRevision): string | undefined { + return "commit" in revision ? revision.commit : undefined; + } + private emptyUpdateResult(sourceId: string): SourceUpdateResultItem { return { sourceId, diff --git a/packages/core-engine/src/services/source-checkout-service.ts b/packages/core-engine/src/services/source-checkout-service.ts index 2ff79c9..13ec63b 100644 --- a/packages/core-engine/src/services/source-checkout-service.ts +++ b/packages/core-engine/src/services/source-checkout-service.ts @@ -256,6 +256,42 @@ export class SourceCheckoutService { }, snapshot.warnings); } + async readGitRemoteHeadCommit( + locator: string, + options: { branch?: string } = {}, + ): Promise { + if (!(await isGitAvailable())) { + return undefined; + } + + const parseCommitSha = (raw: string): string | undefined => { + const line = raw + .split(/\r?\n/) + .map((entry) => entry.trim()) + .find((entry) => entry.length > 0); + const sha = line?.split(/\s+/)[0]?.trim(); + return sha && /^[0-9a-f]{40}$/i.test(sha) ? sha : undefined; + }; + + if (options.branch) { + const branchRef = `refs/heads/${options.branch}`; + const branchOutput = await withNetworkRetries( + () => git(["ls-remote", locator, branchRef], { timeoutMs: 30_000 }), + { attempts: 2 }, + ); + const branchCommit = parseCommitSha(branchOutput); + if (branchCommit) { + return branchCommit; + } + } + + const headOutput = await withNetworkRetries( + () => git(["ls-remote", locator, "HEAD"], { timeoutMs: 30_000 }), + { attempts: 2 }, + ); + return parseCommitSha(headOutput); + } + async normalizeLocator(locator: string): Promise { const trimmed = locator.trim(); diff --git a/packages/core-engine/src/tests/source-authority-service.test.ts b/packages/core-engine/src/tests/source-authority-service.test.ts index c797133..c311595 100644 --- a/packages/core-engine/src/tests/source-authority-service.test.ts +++ b/packages/core-engine/src/tests/source-authority-service.test.ts @@ -1,6 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { describe, expect, test } from "vitest"; +import { describe, expect, test, vi } from "vitest"; import { StateStore } from "@skill-flow/storage/state-store"; import { InventoryService } from "../services/inventory-service.js"; import { SourceAuthorityService } from "../services/source-authority-service.js"; @@ -216,6 +216,86 @@ describe.sequential("SourceAuthorityService", () => { ]); }); + test("updateSources skips git refresh when remote commit is unchanged", async () => { + const stateStore = new StateStore(sandbox.stateRoot); + await stateStore.init(); + const checkoutService = new SourceCheckoutService({ + sourceRoot: path.join(sandbox.stateRoot, "source"), + inventoryService: new InventoryService(), + }); + const service = new SourceAuthorityService({ + stateStore, + checkoutService, + }); + + const preparedCheckoutPath = path.join( + sandbox.stateRoot, + "source", + "git", + ".prepared-git-unchanged", + ); + await fs.mkdir(path.join(preparedCheckoutPath, "skills", "one"), { recursive: true }); + await fs.writeFile( + path.join(preparedCheckoutPath, "skills", "one", "SKILL.md"), + skillDoc("one", "One."), + "utf8", + ); + const committed = await service.commitPreparedSource({ + preparedCheckout: { + locator: "https://github.com/acme/skills.git", + displayName: "Skills", + kind: "git", + sourceId: "git-unchanged", + checkoutPath: preparedCheckoutPath, + leafs: [{ + id: "git-unchanged:skills/one", + sourceId: "git-unchanged", + name: "one", + linkName: "one", + title: "one", + description: "One.", + relativePath: "skills/one", + absolutePath: path.join(preparedCheckoutPath, "skills", "one"), + skillFilePath: path.join(preparedCheckoutPath, "skills", "one", "SKILL.md"), + contentHash: "hash-one", + diagnostics: [], + valid: true, + }], + invalidLeafs: [], + commitSha: "same-sha", + }, + }); + expect(committed.ok).toBe(true); + if (!committed.ok) { + return; + } + + checkoutService.readGitRemoteHeadCommit = vi.fn(async () => "same-sha"); + let prepareCalled = false; + checkoutService.prepareSourceCheckout = vi.fn(async () => { + prepareCalled = true; + throw new Error("prepareSourceCheckout should not be called when commit is unchanged"); + }); + + const updated = await service.updateSources(["git-unchanged"]); + + expect(updated.ok).toBe(true); + if (!updated.ok) { + return; + } + expect(prepareCalled).toBe(false); + expect(updated.data.updated).toEqual([ + expect.objectContaining({ + sourceId: "git-unchanged", + changed: false, + }), + ]); + const state = await stateStore.readState(); + expect(state.lockFile.sources["git-unchanged"]?.leafIds).toEqual([ + "git-unchanged:skills/one", + ]); + }); + test("updateSources keeps successful groups when another group fails mid-batch", async () => { const goodRepo = await createRepo(sandbox.sandboxRoot, { "skills/good/SKILL.md": skillDoc("good", "Good."), diff --git a/packages/core-engine/src/tests/source-checkout-service.test.ts b/packages/core-engine/src/tests/source-checkout-service.test.ts index f60cc86..feddaf4 100644 --- a/packages/core-engine/src/tests/source-checkout-service.test.ts +++ b/packages/core-engine/src/tests/source-checkout-service.test.ts @@ -37,6 +37,24 @@ describe.sequential("SourceCheckoutService", () => { }); }); + test("reads remote HEAD commit for git locators", async () => { + vi.spyOn(gitUtils, "isGitAvailable").mockResolvedValue(true); + vi.spyOn(gitUtils, "git").mockImplementation(async (args) => { + if (args[0] === "ls-remote" && args[2] === "HEAD") { + return "0123456789abcdef0123456789abcdef01234567\tHEAD"; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + const service = new SourceCheckoutService({ + sourceRoot: path.join(sandbox.stateRoot, "source"), + inventoryService: new InventoryService(), + }); + + await expect( + service.readGitRemoteHeadCommit("https://github.com/acme/skills.git"), + ).resolves.toBe("0123456789abcdef0123456789abcdef01234567"); + }); + test("prepares a checkout snapshot without writing authority files", async () => { const repoPath = await createRepo(sandbox.sandboxRoot, { "skills/frontend-design/SKILL.md": skillDoc("frontend-design", "Design frontends."),