From ceb0be482e35a45a1ad5fab88c4c342893dc83d3 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 14 Aug 2026 14:49:11 -0400 Subject: [PATCH] refactor(artwork): make the trace nil-safe in one place only add already handles a nil trace, so record's own guard was dead; Steps was the odd one out and would panic where every other method tolerates nil. --- core/artwork/resolve.go | 4 +--- core/artwork/trace.go | 3 +++ core/artwork/trace_test.go | 1 + 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index 74a6b8acb..cc15c4a81 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -56,9 +56,7 @@ func (c *chainState) try(candidate string, res resolution, ok bool) (resolution, } func (c *chainState) record(candidate string, out Outcome, detail string) { - if c.trace != nil { - c.trace.add(traceStep{Candidate: candidate, Outcome: out, Detail: detail}) - } + c.trace.add(traceStep{Candidate: candidate, Outcome: out, Detail: detail}) } // exhausted is the outcome when no source in the chain yielded an image. diff --git a/core/artwork/trace.go b/core/artwork/trace.go index db5677c58..6cca9408d 100644 --- a/core/artwork/trace.go +++ b/core/artwork/trace.go @@ -52,6 +52,9 @@ func (t *chainTrace) add(step traceStep) { } func (t *chainTrace) Steps() []traceStep { + if t == nil { + return nil + } t.mu.Lock() defer t.mu.Unlock() return slices.Clone(t.steps) diff --git a/core/artwork/trace_test.go b/core/artwork/trace_test.go index d6c594979..524c02ff3 100644 --- a/core/artwork/trace_test.go +++ b/core/artwork/trace_test.go @@ -56,6 +56,7 @@ var _ = Describe("chainTrace", func() { It("does not panic when the trace is nil", func() { var t *chainTrace Expect(func() { t.add(traceStep{Candidate: "cover.*", Outcome: OutcomeMiss}) }).ToNot(Panic()) + Expect(t.Steps()).To(BeEmpty(), "a nil trace collects nothing, so reading it must be as safe as writing it") }) It("is safe to use concurrently", func() {