From 4bad644498a89d536abce80c1131795ff319aaa0 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 19:53:28 +0000 Subject: [PATCH] fix(distributed): name both revisions in the stale error "stale model config revision" reported only that two hashes differed. It named neither, so an operator could not tell an edited configuration from a revision that is not reproducible for one unchanged file, and the failing value appears in no table. The error now carries the revision the request brought and the one the controller holds. It still wraps ErrStaleModelConfigRevision, so callers that classify the error keep working. Signed-off-by: Ettore Di Giacinto Assisted-by: Claude Code:claude-opus-5 [golangci-lint] --- core/services/nodes/registry.go | 19 ++++++++++++++- .../nodes/revision_error_detail_test.go | 24 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 core/services/nodes/revision_error_detail_test.go diff --git a/core/services/nodes/registry.go b/core/services/nodes/registry.go index d6fd59662..1e5d52db0 100644 --- a/core/services/nodes/registry.go +++ b/core/services/nodes/registry.go @@ -1159,11 +1159,28 @@ func requireCurrentRevision(tx *gorm.DB, modelName, revision string) error { return err } if state.ConfigRevision != revision { - return ErrStaleModelConfigRevision + // Name both sides. "stale model config revision" on its own says only + // that two hashes differ, which leaves an operator no way to tell an + // edited configuration from a revision that is not reproducible for one + // unchanged file. + return fmt.Errorf("%w (request carries %s, controller holds %s)", + ErrStaleModelConfigRevision, shortRevision(revision), shortRevision(state.ConfigRevision)) } return nil } +// shortRevision trims a revision for log and error output. The full value is a +// sha256 hex digest; the leading bytes identify it well enough to compare two. +func shortRevision(revision string) string { + if revision == "" { + return "(none)" + } + if len(revision) > 12 { + return revision[:12] + } + return revision +} + func validateRevisionWrite(modelName, revision string, revisionRequired bool) error { if modelName == "" { return fmt.Errorf("model name is required") diff --git a/core/services/nodes/revision_error_detail_test.go b/core/services/nodes/revision_error_detail_test.go new file mode 100644 index 000000000..25be6866a --- /dev/null +++ b/core/services/nodes/revision_error_detail_test.go @@ -0,0 +1,24 @@ +package nodes + +import ( + "errors" + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Stale revision error detail", func() { + It("keeps errors.Is matching so callers can still classify it", func() { + err := fmt.Errorf("%w (request carries %s, controller holds %s)", + ErrStaleModelConfigRevision, shortRevision("aaaabbbbccccdddd"), shortRevision("1111222233334444")) + Expect(errors.Is(err, ErrStaleModelConfigRevision)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("stale model config revision")) + }) + + It("names both revisions so an operator can tell which side moved", func() { + Expect(shortRevision("aaaabbbbccccdddd")).To(Equal("aaaabbbbcccc")) + Expect(shortRevision("short")).To(Equal("short")) + Expect(shortRevision("")).To(Equal("(none)")) + }) +})