refactor(skills): centralize observation matching (#16134)

src/main/skills/skill-bundle-creation.ts

@@ -20,7 +20,11 @@ import {
 import { summarizeSkillMarkdown } from '../../shared/skill-metadata'
 import { renameSkillPathWithWindowsRetry } from './skill-filesystem-retry'
 import { extractSkillBundleArchive } from './skill-bundle-extraction'
-import { observeSkillPackage, type ObservedSkillPackage } from './skill-package-identity'
+import {
+  observedSkillPackagesMatch,
+  observeSkillPackage,
+  type ObservedSkillPackage
+} from './skill-package-identity'
 import { writeSkillTarGzip, type SkillTarWriteEntry } from './skill-package-tar'
 import { startSkillPhaseOperation } from './skill-operation-observability'
 
@@ -42,23 +46,6 @@ export type SkillBundleCreationDependencies = { afterSourcesObserved?: () => Pro
 
 const SOURCE_OBSERVATION_CONCURRENCY = 4
 
-function observationsMatch(left: ObservedSkillPackage, right: ObservedSkillPackage): boolean {
-  return (
-    left.files.length === right.files.length &&
-    left.files.every((file, index) => {
-      const other = right.files[index]
-      return (
-        file.path === other.path &&
-        file.size === other.size &&
-        file.executable === other.executable &&
-        file.classification === other.classification &&
-        file.exactSha256 === other.exactSha256 &&
-        file.identitySha256 === other.identitySha256
-      )
-    })
-  )
-}
-
 function bundleEntry(input: {
   id: string
   name: string
@@ -104,7 +91,7 @@ async function stageSkill(input: {
     process.platform,
     process.platform === 'win32'
   )
-  if (!observationsMatch(input.sourceObservation, stagedObservation)) {
+  if (!observedSkillPackagesMatch(input.sourceObservation, stagedObservation)) {
     throw new Error('skill-package-source-changed-during-staging')
   }
   const stagedSummary = summarizeSkillMarkdown(

src/main/skills/skill-package-creation.ts

@@ -11,7 +11,11 @@ import {
 import { summarizeSkillMarkdown } from '../../shared/skill-metadata'
 import { renameSkillPathWithWindowsRetry } from './skill-filesystem-retry'
 import { startSkillPhaseOperation } from './skill-operation-observability'
-import { observeSkillPackage, type ObservedSkillPackage } from './skill-package-identity'
+import {
+  observedSkillPackagesMatch,
+  observeSkillPackage,
+  type ObservedSkillPackage
+} from './skill-package-identity'
 import { extractSkillPackageArchive } from './skill-package-extraction'
 import { writeSkillTarGzip, type SkillTarWriteEntry } from './skill-package-tar'
 
@@ -26,23 +30,6 @@ export type SkillPackageCreationDependencies = {
   afterSourceObserved?: () => Promise<void>
 }
 
-function observationsMatch(left: ObservedSkillPackage, right: ObservedSkillPackage): boolean {
-  return (
-    left.files.length === right.files.length &&
-    left.files.every((file, index) => {
-      const other = right.files[index]
-      return (
-        file.path === other.path &&
-        file.size === other.size &&
-        file.executable === other.executable &&
-        file.classification === other.classification &&
-        file.exactSha256 === other.exactSha256 &&
-        file.identitySha256 === other.identitySha256
-      )
-    })
-  )
-}
-
 function packageManifest(input: {
   packageId: string
   versionId: string
@@ -98,7 +85,7 @@ async function createSkillPackageArchiveUnobserved(
       errorOnExist: true
     })
     const stagedObservation = await observeSkillPackage(stagedSkill)
-    if (!observationsMatch(sourceObservation, stagedObservation)) {
+    if (!observedSkillPackagesMatch(sourceObservation, stagedObservation)) {
       throw new Error('skill-package-source-changed-during-staging')
     }
     const summary = summarizeSkillMarkdown(await readFile(join(stagedSkill, 'SKILL.md'), 'utf8'))

src/main/skills/skill-package-identity.ts

@@ -23,6 +23,26 @@ export type ObservedSkillPackage = {
   treeEntries: SkillGitTreeFileEntry[]
 }
 
+export function observedSkillPackagesMatch(
+  left: ObservedSkillPackage,
+  right: ObservedSkillPackage
+): boolean {
+  return (
+    left.files.length === right.files.length &&
+    left.files.every((file, index) => {
+      const other = right.files[index]
+      return (
+        file.path === other.path &&
+        file.size === other.size &&
+        file.executable === other.executable &&
+        file.classification === other.classification &&
+        file.exactSha256 === other.exactSha256 &&
+        file.identitySha256 === other.identitySha256
+      )
+    })
+  )
+}
+
 // Why: package identity compares a live user directory against a tree the generator read
 // from a clean checkout, so anything the OS deposits on its own counts as drift the user
 // never caused. One Finder visit writes .DS_Store, and that alone made the copy