From b7c0f43064d017922fc00cd69658c22ee91774d6 Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Tue, 16 Jun 2026 21:50:45 +0000 Subject: [PATCH] Fix autogen to fail-open on missing asset manifests (#4) Reviewed-on: https://git.westgate.pw/ShadowsOverWestgate/sow-tools/pulls/4 Co-authored-by: vickydotbat Co-committed-by: vickydotbat --- docs/consumer-contract.md | 9 +- internal/topdata/autogen.go | 158 ++++++++++++++++++---- internal/topdata/autogen_preserve_test.go | 109 +++++++++++++++ 3 files changed, 247 insertions(+), 29 deletions(-) create mode 100644 internal/topdata/autogen_preserve_test.go diff --git a/docs/consumer-contract.md b/docs/consumer-contract.md index 75bc224..dc6f114 100644 --- a/docs/consumer-contract.md +++ b/docs/consumer-contract.md @@ -24,14 +24,17 @@ quoting stays correct. ## In CI (preferred) -Run the consumer job inside the pinned image and the binaries are on `PATH`: +**OPERATOR NOTE: Stop. Do not do it this way.** +**Use the pinned `prod.yml` version. That's what it's there for.** + +~~Run the consumer job inside the pinned image and the binaries are on `PATH`:~~ ```yaml container: - image: registry.westgate.pw/deployment/crucible: # pinned, immutable + image: registry.westgate.pw/deployment/crucible: # pinned, immutable ``` -No host install, no `$HOME` layout, no developer machine assumptions. +~~No host install, no `$HOME` layout, no developer machine assumptions.~~ ## `NWN_ROOT` rule diff --git a/internal/topdata/autogen.go b/internal/topdata/autogen.go index 8af0265..5cdde09 100644 --- a/internal/topdata/autogen.go +++ b/internal/topdata/autogen.go @@ -3,6 +3,7 @@ package topdata import ( "bytes" "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -16,6 +17,19 @@ import ( const autogenManifestCacheMaxAge = time.Hour +// errAutogenManifestUnavailable marks a released autogen manifest that could not +// be located or fetched (network failure, missing release/asset, empty payload, +// or an undeterminable sow-assets repo). It is deliberately distinct from an +// asset that was found but reports a row removed: for an optional consumer this +// sentinel triggers a fail-open path that *preserves* the consumer's existing +// pinned lock entries instead of pruning them, so StrRef/row IDs are not +// reshuffled while the asset source is merely temporarily out of reach. +var errAutogenManifestUnavailable = errors.New("autogen manifest unavailable") + +func unavailableAutogenManifest(err error) error { + return fmt.Errorf("%w: %w", errAutogenManifestUnavailable, err) +} + type autogenManifest struct { ID string `json:"id"` Repo string `json:"repo"` @@ -61,9 +75,18 @@ func applyAutogenConsumers(p *project.Project, collected []nativeCollectedDatase } manifest, err := resolveAutogenConsumerManifest(p, consumer, progress) if err != nil { - if consumer.Optional && isIgnorableOptionalAutogenError(err) { + if consumer.Optional && errors.Is(err, errAutogenManifestUnavailable) { + // Fail open: the asset manifest is merely unreachable, not + // authoritatively empty. Build without its rows, but keep the + // consumer's already-pinned lock entries so their IDs are not + // freed and reshuffled on a later build once the asset returns. + preserved, perr := preserveAutogenConsumerLockEntries(result, consumer) + if perr != nil { + return nil, perr + } + result = preserved if progress != nil { - progress(fmt.Sprintf("Skipping optional autogen consumer %s: %v", consumer.ID, err)) + progress(fmt.Sprintf("Autogen consumer %s: released manifest unavailable (%v); preserving existing lock entries, generating no new rows", consumer.ID, err)) } continue } @@ -90,32 +113,115 @@ func applyAutogenConsumers(p *project.Project, collected []nativeCollectedDatase func autogenConsumerTargetsCollectedDataset(collected []nativeCollectedDataset, consumer project.AutogenConsumerConfig) bool { for _, dataset := range collected { - switch consumer.Mode { - case "parts_rows": - if isAutogenEligiblePartsDataset(dataset) { - return true - } - case "accessory_visualeffects": - if dataset.Dataset.Name == "visualeffects" { - return true - } - case "cachedmodels_rows": - if dataset.Dataset.Name == "cachedmodels" { - return true - } + if autogenConsumerTargetsDataset(dataset, consumer) { + return true } } return false } -func isIgnorableOptionalAutogenError(err error) bool { - if err == nil { +// preserveAutogenConsumerLockEntries restores the lock keys that an autogen +// consumer owns from the on-disk lockfile back into the in-memory dataset, +// reusing this build's already-collected (and pruned) state. It is called only +// when the consumer's released manifest is unavailable, so the rows themselves +// are not regenerated; we just keep the key->ID pins alive. Keys are matched +// against the consumer's own naming policy so unrelated (authored) rows that were +// legitimately removed are not resurrected. +func preserveAutogenConsumerLockEntries(collected []nativeCollectedDataset, consumer project.AutogenConsumerConfig) ([]nativeCollectedDataset, error) { + matches := autogenConsumerManagedLockKeyMatcher(consumer) + if matches == nil { + // No managed-key matcher for this mode: nothing safe to preserve. + return collected, nil + } + + result := append([]nativeCollectedDataset(nil), collected...) + for i, dataset := range result { + if !autogenConsumerTargetsDataset(dataset, consumer) { + continue + } + if strings.TrimSpace(dataset.Dataset.LockPath) == "" { + continue + } + onDisk, err := loadLockfile(dataset.Dataset.LockPath) + if err != nil { + // No readable lockfile means there is nothing pinned to preserve. + continue + } + lockData := dataset.LockData + if lockData == nil { + lockData = map[string]int{} + } + restored := 0 + for key, rowID := range onDisk { + if _, present := lockData[key]; present { + continue + } + if !matches(key) { + continue + } + lockData[key] = rowID + restored++ + } + if restored == 0 { + continue + } + result[i].LockData = lockData + result[i].LockModified = !lockDataEqual(onDisk, lockData) + } + return result, nil +} + +// autogenConsumerManagedLockKeyMatcher returns a predicate identifying the lock +// keys an autogen consumer is responsible for, or nil for modes whose keys we +// cannot scope precisely (in which case preservation is skipped rather than +// risk resurrecting unrelated keys). +func autogenConsumerManagedLockKeyMatcher(consumer project.AutogenConsumerConfig) func(string) bool { + switch consumer.Mode { + case "accessory_visualeffects": + policy := resolveHeadVisualeffectsPolicy(consumer, nil) + delimiter := policy.Delimiter + if delimiter == "" { + delimiter = "/" + } + prefix := "visualeffects:" + groups := make(map[string]struct{}, len(policy.Groups)) + for group, groupPolicy := range policy.Groups { + token := applyHeadVisualeffectCase(headVisualeffectGroupToken(group, groupPolicy, policy), policy) + if strings.TrimSpace(token) != "" { + groups[token] = struct{}{} + } + } + if len(groups) == 0 { + return nil + } + return func(key string) bool { + if !strings.HasPrefix(key, prefix) { + return false + } + rest := key[len(prefix):] + idx := strings.Index(rest, delimiter) + if idx <= 0 { + return false + } + _, ok := groups[rest[:idx]] + return ok + } + default: + return nil + } +} + +func autogenConsumerTargetsDataset(dataset nativeCollectedDataset, consumer project.AutogenConsumerConfig) bool { + switch consumer.Mode { + case "parts_rows": + return isAutogenEligiblePartsDataset(dataset) + case "accessory_visualeffects": + return dataset.Dataset.Name == "visualeffects" + case "cachedmodels_rows": + return dataset.Dataset.Name == "cachedmodels" + default: return false } - message := err.Error() - return strings.Contains(message, "HTTP 404") || - strings.Contains(message, "not found") || - strings.Contains(message, "entries is empty") } func resolveAutogenConsumerManifest(p *project.Project, consumer project.AutogenConsumerConfig, progress func(string)) (*autogenManifest, error) { @@ -366,14 +472,14 @@ func resolveReleasedAutogenManifest(p *project.Project, consumer project.Autogen spec, err := deriveSowAssetsRepoSpec(p) if err != nil { - return nil, err + return nil, unavailableAutogenManifest(err) } manifestURL, err := resolveAutogenManifestAssetURL(spec, consumer.Manifest.ReleaseTag, consumer.Manifest.AssetName) if err != nil { if consumer.Mode == "parts_rows" { - return nil, fmt.Errorf("%s", strings.Replace(err.Error(), "autogen manifest", "parts manifest", 1)) + return nil, unavailableAutogenManifest(fmt.Errorf("%s", strings.Replace(err.Error(), "autogen manifest", "parts manifest", 1))) } - return nil, err + return nil, unavailableAutogenManifest(err) } if progress != nil { progress(fmt.Sprintf("Fetching released autogen manifest from %s...", manifestURL)) @@ -381,9 +487,9 @@ func resolveReleasedAutogenManifest(p *project.Project, consumer project.Autogen manifest, err := fetchAutogenManifest(manifestURL) if err != nil { if consumer.Mode == "parts_rows" { - return nil, fmt.Errorf("%s", strings.Replace(err.Error(), "autogen manifest", "parts manifest", 1)) + return nil, unavailableAutogenManifest(fmt.Errorf("%s", strings.Replace(err.Error(), "autogen manifest", "parts manifest", 1))) } - return nil, err + return nil, unavailableAutogenManifest(err) } if manifest.ID != "" && manifest.ID != consumer.Producer { return nil, fmt.Errorf("autogen manifest %s declared id %q, expected %q", consumer.Manifest.AssetName, manifest.ID, consumer.Producer) diff --git a/internal/topdata/autogen_preserve_test.go b/internal/topdata/autogen_preserve_test.go new file mode 100644 index 0000000..7d62d0a --- /dev/null +++ b/internal/topdata/autogen_preserve_test.go @@ -0,0 +1,109 @@ +package topdata + +import ( + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "git.westgate.pw/ShadowsOverWestgate/sow-tools/internal/project" +) + +// When an optional autogen consumer's released manifest cannot be fetched, the +// build must fail open: it keeps the consumer's already-pinned lock entries +// (so their IDs are not freed and reshuffled later) and generates no new rows. +func TestApplyAutogenConsumersPreservesLockEntriesWhenManifestUnavailable(t *testing.T) { + root := testProjectRoot(t) + + // 404 for every request → released manifest is unavailable (not empty). + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + t.Cleanup(srv.Close) + t.Setenv("SOW_ASSETS_SERVER_URL", srv.URL) + t.Setenv("SOW_ASSETS_REPO", "ShadowsOverWestgate/sow-assets") + + p := testProject(root) + p.Config.Autogen.Consumers = []project.AutogenConsumerConfig{ + { + ID: "accessory_visualeffects", + Producer: "accessory_visualeffects", + Dataset: "visualeffects", + Mode: "accessory_visualeffects", + Optional: true, + Root: "vfxs", + Include: []string{"head_accessories/**/*.mdl"}, + Derive: project.AutogenDeriveConfig{Kind: "model_stem", GroupFrom: "first_path_segment"}, + Manifest: project.AutogenManifestConfig{ReleaseTag: "head-vfx-manifest-current", AssetName: "sow-accessory-vfx-manifest.json", CacheName: "sow-accessory-vfx-manifest.json"}, + }, + } + + // On-disk lock pins an autogen-owned accessory key plus an authored key. + lockPath := filepath.Join(root, "visualeffects-lock.json") + writeFile(t, lockPath, `{"visualeffects:existing":17,"visualeffects:head_accessories/hat/bandana":10101}`+"\n") + + // Simulate post-prune collected state: the accessory key has already been + // dropped from in-memory LockData (as pruneLockDataToActiveRows would do). + collected := []nativeCollectedDataset{ + { + Dataset: nativeDataset{Name: "visualeffects", Kind: nativeDatasetBase, LockPath: lockPath}, + Columns: []string{"Label"}, + Rows: []map[string]any{{"id": 17, "key": "visualeffects:existing"}}, + LockData: map[string]int{"visualeffects:existing": 17}, + }, + } + + got, err := applyAutogenConsumers(p, collected, nil) + if err != nil { + t.Fatalf("expected fail-open build, got error: %v", err) + } + if len(got) != 1 { + t.Fatalf("expected one dataset, got %d", len(got)) + } + + // The pinned accessory id must survive with its exact value (no jumble). + if id, ok := got[0].LockData["visualeffects:head_accessories/hat/bandana"]; !ok || id != 10101 { + t.Fatalf("expected preserved accessory lock id 10101, got %v (present=%v) lock=%#v", id, ok, got[0].LockData) + } + // The authored key is untouched. + if id, ok := got[0].LockData["visualeffects:existing"]; !ok || id != 17 { + t.Fatalf("expected authored key retained at 17, got %#v", got[0].LockData) + } + // No row is generated for the unavailable accessory entry. + for _, row := range got[0].Rows { + if key, _ := row["key"].(string); key == "visualeffects:head_accessories/hat/bandana" { + t.Fatalf("expected no generated row while manifest unavailable, got %#v", row) + } + } +} + +// A key that is NOT owned by the consumer (an authored row legitimately removed) +// must not be resurrected by the preservation path. +func TestPreserveAutogenConsumerLockEntriesIgnoresUnownedKeys(t *testing.T) { + root := testProjectRoot(t) + lockPath := filepath.Join(root, "visualeffects-lock.json") + writeFile(t, lockPath, `{"visualeffects:retired_authored":3,"visualeffects:head_accessories/hat/bandana":10101}`+"\n") + + consumer := project.AutogenConsumerConfig{ + ID: "accessory_visualeffects", + Dataset: "visualeffects", + Mode: "accessory_visualeffects", + } + collected := []nativeCollectedDataset{ + { + Dataset: nativeDataset{Name: "visualeffects", Kind: nativeDatasetBase, LockPath: lockPath}, + LockData: map[string]int{}, + }, + } + + got, err := preserveAutogenConsumerLockEntries(collected, consumer) + if err != nil { + t.Fatalf("preserveAutogenConsumerLockEntries failed: %v", err) + } + if _, ok := got[0].LockData["visualeffects:head_accessories/hat/bandana"]; !ok { + t.Fatalf("expected owned accessory key preserved, got %#v", got[0].LockData) + } + if _, ok := got[0].LockData["visualeffects:retired_authored"]; ok { + t.Fatalf("expected unowned authored key NOT resurrected, got %#v", got[0].LockData) + } +}