diff --git a/internal/bundle/discover.go b/internal/bundle/discover.go index e7e75f0..a8ba491 100644 --- a/internal/bundle/discover.go +++ b/internal/bundle/discover.go @@ -120,10 +120,15 @@ func Discover(ctx context.Context, database *store.Store, options DiscoverOption if err := persistFallbacks(ctx, database, observed, matchedBindings, now, options.HomeDir, &result); err != nil { return result, err } - result.Pruned, err = database.PruneUnconfiguredBundles(ctx, now) + prunedInstallations, err := database.PruneUnconfiguredInstallations(ctx, now) if err != nil { return result, err } + prunedBundles, err := database.PruneUnconfiguredBundles(ctx, now) + if err != nil { + return result, err + } + result.Pruned = prunedInstallations + prunedBundles if err := refreshDiscoveryCounts(ctx, database, &result); err != nil { return result, err } @@ -281,8 +286,15 @@ func resolveProbe(ctx context.Context, probe string, recipe Recipe, artifact Art return matchedInstallation{}, false } path = normalizeInstallationPath(path, options.HomeDir) - version := "" + packageIdentity := recipe.ID + version, observedHash := "", "" metadata := map[string]any{"probe": probe} + if artifact.Driver == "npm" { + if packageName, packageVersion, packageHash := nearestPackageMetadata(path); packageName != "" { + packageIdentity, version, observedHash = packageName, packageVersion, packageHash + metadata["npm_package"] = packageName + } + } if recipe.ID == "tooltend" && options.BuildVersion != "" && options.BuildVersion != "dev" { version = strings.TrimPrefix(options.BuildVersion, "v") } @@ -295,7 +307,7 @@ func resolveProbe(ctx context.Context, probe string, recipe Recipe, artifact Art } metadata["code_signature_valid"] = verifyAppSignature(ctx, path) } - return matchedInstallation{path: path, packageIdentity: recipe.ID, version: version, metadata: metadata}, true + return matchedInstallation{path: path, packageIdentity: packageIdentity, version: version, hash: observedHash, metadata: metadata}, true } func persistRecipeMatch(ctx context.Context, database *store.Store, recipe Recipe, confidence model.BundleConfidence, matches map[string][]matchedInstallation, now time.Time, result *DiscoverResult) error { @@ -697,20 +709,69 @@ func countMatches(values map[string][]matchedInstallation) int { } func dedupeMatches(values []matchedInstallation) []matchedInstallation { - seen := map[string]int{} result := make([]matchedInstallation, 0, len(values)) for _, value := range values { - key := value.path + "\x00" + value.packageIdentity + "\x00" + value.sourceIdentity - if index, exists := seen[key]; exists { - result[index].consumers = append(result[index].consumers, value.consumers...) - continue + merged := false + for index := range result { + if samePhysicalEvidence(result[index], value) { + result[index] = mergeMatchEvidence(result[index], value) + merged = true + break + } + } + if !merged { + result = append(result, value) } - seen[key] = len(result) - result = append(result, value) } return result } +func samePhysicalEvidence(left, right matchedInstallation) bool { + if left.path == "" || left.path != right.path { + return false + } + if left.packageIdentity == right.packageIdentity && left.sourceIdentity == right.sourceIdentity { + return true + } + // A command/path probe is weaker evidence for an already observed path. It + // must enrich that physical installation instead of creating a second row + // merely because the probe cannot know the source identity. + return probeMatch(left) || probeMatch(right) +} + +func mergeMatchEvidence(left, right matchedInstallation) matchedInstallation { + if probeMatch(left) && !probeMatch(right) { + left, right = right, left + } + if left.packageIdentity == "" { + left.packageIdentity = right.packageIdentity + } + if left.sourceIdentity == "" { + left.sourceIdentity = right.sourceIdentity + } + if left.version == "" { + left.version = right.version + } + if left.hash == "" { + left.hash = right.hash + } + left.consumers = append(left.consumers, right.consumers...) + if left.metadata == nil { + left.metadata = map[string]any{} + } + for key, value := range right.metadata { + if _, exists := left.metadata[key]; !exists { + left.metadata[key] = value + } + } + return left +} + +func probeMatch(value matchedInstallation) bool { + _, ok := value.metadata["probe"] + return ok +} + func normalizeInstallationPath(path, home string) string { path = strings.TrimSpace(path) if path == "" { @@ -768,6 +829,36 @@ func actualInstalledVersion(path, packageIdentity, observed string) (string, str return "", "" } +func nearestPackageMetadata(path string) (string, string, string) { + cursor := path + if info, err := os.Stat(cursor); err == nil && !info.IsDir() { + cursor = filepath.Dir(cursor) + } + for depth := 0; depth < 8; depth++ { + data, err := os.ReadFile(filepath.Join(cursor, "package.json")) + if err == nil { + var pkg struct { + Name string `json:"name"` + Version string `json:"version"` + } + if json.Unmarshal(data, &pkg) == nil && strings.TrimSpace(pkg.Name) != "" { + digest := sha256.Sum256(data) + version := "" + if exactVersion(pkg.Version) { + version = strings.TrimPrefix(strings.TrimSpace(pkg.Version), "v") + } + return strings.TrimSpace(pkg.Name), version, hex.EncodeToString(digest[:]) + } + } + parent := filepath.Dir(cursor) + if parent == cursor { + break + } + cursor = parent + } + return "", "", "" +} + var semverLike = regexp.MustCompile(`^v?[0-9]+\.[0-9]+\.[0-9]+(?:[-+][0-9A-Za-z.-]+)?$`) func exactVersion(value string) bool { diff --git a/internal/bundle/discover_test.go b/internal/bundle/discover_test.go index 030b51b..93b6f56 100644 --- a/internal/bundle/discover_test.go +++ b/internal/bundle/discover_test.go @@ -56,7 +56,12 @@ func TestDiscoverDeduplicatesPhysicalInstallAndReadsPackageMetadata(t *testing.T t.Fatal(err) } } - result, err := Discover(ctx, database, DiscoverOptions{HomeDir: t.TempDir(), Executable: "/missing/tooltend", LookupPath: func(string) (string, error) { return "", os.ErrNotExist }, Now: func() time.Time { return now }}) + result, err := Discover(ctx, database, DiscoverOptions{HomeDir: t.TempDir(), Executable: "/missing/tooltend", LookupPath: func(command string) (string, error) { + if command == "oa-skills" { + return linkOne, nil + } + return "", os.ErrNotExist + }, Now: func() time.Time { return now }}) if err != nil { t.Fatal(err) } @@ -71,7 +76,7 @@ func TestDiscoverDeduplicatesPhysicalInstallAndReadsPackageMetadata(t *testing.T if err != nil { t.Fatal(err) } - if len(installations) != 1 || installations[0].ObservedVersion != "1.2.3" { + if len(installations) != 1 || installations[0].ObservedVersion != "1.2.3" || installations[0].PackageIdentity != "@it/oa-skills" { t.Fatalf("installations = %#v", installations) } consumers, err := database.ListConsumerBindings(ctx, installations[0].ID) @@ -83,6 +88,95 @@ func TestDiscoverDeduplicatesPhysicalInstallAndReadsPackageMetadata(t *testing.T } } +func TestDiscoverPrunesStaleProbeWhenBindingProvidesRicherEvidence(t *testing.T) { + database, err := store.OpenRW(filepath.Join(t.TempDir(), "state.db")) + if err != nil { + t.Fatal(err) + } + defer database.Close() + ctx := context.Background() + firstSeen := time.Now().UTC().Add(-time.Minute) + packageRoot := filepath.Join(t.TempDir(), "node_modules", "@it", "oa-skills") + if err := os.MkdirAll(filepath.Join(packageRoot, "bin"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(packageRoot, "package.json"), []byte(`{"name":"@it/oa-skills","version":"2.0.0"}`), 0o600); err != nil { + t.Fatal(err) + } + physical := filepath.Join(packageRoot, "bin", "oa-skills") + if err := os.WriteFile(physical, []byte("binary"), 0o755); err != nil { + t.Fatal(err) + } + lookup := func(command string) (string, error) { + if command == "oa-skills" { + return physical, nil + } + return "", os.ErrNotExist + } + if _, err := Discover(ctx, database, DiscoverOptions{HomeDir: t.TempDir(), Executable: "/missing/tooltend", LookupPath: lookup, Now: func() time.Time { return firstSeen }}); err != nil { + t.Fatal(err) + } + bundleValue, err := database.GetBundleBySlug(ctx, "citadel") + if err != nil { + t.Fatal(err) + } + installations, err := database.ListInstallations(ctx, bundleValue.ID) + if err != nil || len(installations) != 1 || installations[0].SourceIdentity != "" || installations[0].PackageIdentity != "@it/oa-skills" || installations[0].ObservedVersion != "2.0.0" { + t.Fatalf("probe installations = %#v err=%v", installations, err) + } + + now := firstSeen.Add(time.Minute) + source := model.Source{ID: "source", Kind: model.SourceNPM, Locator: "https://registry.npmjs.org/@it/oa-skills", PackageName: "@it/oa-skills", IdentityHash: "source-hash", CreatedAt: now, UpdatedAt: now} + if err := database.UpsertSource(ctx, source); err != nil { + t.Fatal(err) + } + component := model.LogicalComponent{ID: "component", Kind: model.ComponentCLI, Name: "@it/oa-skills", SourceID: source.ID, LogicalKey: "oa-skills", CreatedAt: now, UpdatedAt: now} + if err := database.UpsertComponent(ctx, component); err != nil { + t.Fatal(err) + } + if err := database.UpsertBinding(ctx, model.Binding{ID: "codex", ComponentID: component.ID, Host: model.HostCodex, Scope: model.ScopeGlobal, InstallPath: physical, Classification: model.ClassificationClean, LastSeenAt: now}); err != nil { + t.Fatal(err) + } + result, err := Discover(ctx, database, DiscoverOptions{HomeDir: t.TempDir(), Executable: "/missing/tooltend", LookupPath: lookup, Now: func() time.Time { return now }}) + if err != nil { + t.Fatal(err) + } + installations, err = database.ListInstallations(ctx, bundleValue.ID) + if err != nil || len(installations) != 1 || installations[0].SourceIdentity != "source-hash" { + t.Fatalf("merged installations = %#v err=%v", installations, err) + } + if result.Pruned == 0 { + t.Fatal("stale probe installation was not pruned") + } +} + +func TestBuiltinRecipesDoNotTreatSkillsAsCLIsOrDerivedHooks(t *testing.T) { + catalog, err := LoadCatalog("") + if err != nil { + t.Fatal(err) + } + mainlineRecipe, ok := catalog.Get("mainline") + if !ok { + t.Fatal("mainline recipe not found") + } + mainlineSkill := observedInstallation{component: model.LogicalComponent{Name: "mainline", Kind: model.ComponentSkill}, dependencies: []model.Dependency{{PackageIdentity: "cli:mainline"}}} + for _, artifact := range mainlineRecipe.Artifacts { + if (artifact.Key == "cli" || artifact.Key == "hooks") && artifactMatches(artifact, mainlineSkill) { + t.Fatalf("mainline skill matched %s artifact", artifact.Key) + } + } + sherlogRecipe, ok := catalog.Get("sherlog") + if !ok { + t.Fatal("sherlog recipe not found") + } + sherlogSkill := observedInstallation{component: model.LogicalComponent{Name: "sherlog", Kind: model.ComponentSkill}} + for _, artifact := range sherlogRecipe.Artifacts { + if artifact.Key == "cli" && artifactMatches(artifact, sherlogSkill) { + t.Fatal("sherlog skill matched CLI artifact") + } + } +} + func TestRecipeRejectsShellStrings(t *testing.T) { data := []byte(` schema = "bundle-recipe-v1" diff --git a/internal/bundle/recipes/mainline.toml b/internal/bundle/recipes/mainline.toml index 19b062e..d6542dd 100644 --- a/internal/bundle/recipes/mainline.toml +++ b/internal/bundle/recipes/mainline.toml @@ -40,6 +40,3 @@ name = "Mainline generated hooks" kind = "hook" driver = "mainline-hooks" required = false -[[artifacts.selectors]] -field = "dependency" -equals = "cli:mainline" diff --git a/internal/bundle/recipes/sherlog.toml b/internal/bundle/recipes/sherlog.toml index d6cc0ab..525938a 100644 --- a/internal/bundle/recipes/sherlog.toml +++ b/internal/bundle/recipes/sherlog.toml @@ -17,6 +17,9 @@ health_argv = ["sherlog", "--version"] [[artifacts.selectors]] field = "name" equals = "sherlog|shlog" +[[artifacts.selectors]] +field = "kind" +equals = "cli" [[artifacts]] key = "skill" diff --git a/internal/store/bundles.go b/internal/store/bundles.go index bfa50ec..04b490d 100644 --- a/internal/store/bundles.go +++ b/internal/store/bundles.go @@ -119,6 +119,18 @@ func (s *Store) PruneUnconfiguredBundles(ctx context.Context, seenBefore time.Ti return result.RowsAffected() } +// PruneUnconfiguredInstallations removes stale discovery evidence inside +// bundles that the user has not configured. Configured bundles retain their +// installation graph until an explicit lifecycle action changes it. +func (s *Store) PruneUnconfiguredInstallations(ctx context.Context, seenBefore time.Time) (int64, error) { + result, err := s.db.ExecContext(ctx, `DELETE FROM installations + WHERE last_seen_at