From 4fa0ce4431daf0ba3ff44b59efae06cee3432383 Mon Sep 17 00:00:00 2001 From: Alex Metelli Date: Sat, 14 Mar 2026 12:57:53 +0800 Subject: [PATCH] Change State.Tool() to return value instead of pointer to copy State.Tool() now returns (ToolState, bool) by value, making copy semantics explicit. Call sites in discovery, executor, and state are updated to work with the value type directly. Closes #10 Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 3 +++ internal/discovery/discovery.go | 5 ++++- internal/plan/executor.go | 4 ++-- internal/state/state.go | 10 ++++------ 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7feb70..52a40bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- `State.Tool()` now returns `(ToolState, bool)` by value instead of a pointer to a copy, making copy semantics explicit and eliminating a potential mutation footgun. + ### Fixed - The standalone installer now surfaces `mkdir -p` failures directly instead of suppressing the error and failing later with weaker diagnostics. - Stale version metadata is now cleared when binary verification detects a missing tool, preventing misleading version output for tools that are no longer installed. diff --git a/internal/discovery/discovery.go b/internal/discovery/discovery.go index 6a000f4..82e7f69 100644 --- a/internal/discovery/discovery.go +++ b/internal/discovery/discovery.go @@ -72,7 +72,10 @@ func Reconcile(reg catalog.Registry, st state.State, paths map[string]string) Sn Path: path, Installed: path != "", Ownership: OwnershipMissing, - Receipt: receipt, + } + + if ok { + presence.Receipt = &receipt } if ok { diff --git a/internal/plan/executor.go b/internal/plan/executor.go index 89e747e..32bd830 100644 --- a/internal/plan/executor.go +++ b/internal/plan/executor.go @@ -100,7 +100,7 @@ func ExecuteUpdatePlan(ctx context.Context, plan Plan, st *state.State, verifier } receipt.LastUpdateAttemptAt = time.Now().UTC() - if err := st.AddReceipt(*receipt); err != nil { + if err := st.AddReceipt(receipt); err != nil { return summary, fmt.Errorf("persist update receipt for %s: %w", action.Tool.ID, err) } @@ -198,7 +198,7 @@ func updateReceiptVerification(st *state.State, toolID string, result verify.Ver receipt.LastVerifyOK = result.Verified receipt.LastVerifyError = result.Error receipt.Version = result.Version - if err := st.SetTool(*receipt); err != nil { + if err := st.SetTool(receipt); err != nil { return } } diff --git a/internal/state/state.go b/internal/state/state.go index c68fdc3..f659a50 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -85,19 +85,17 @@ func Save(st State) error { } // Tool returns the persisted state for a tool ID, if present. -func (s State) Tool(id string) (*ToolState, bool) { +func (s State) Tool(id string) (ToolState, bool) { if len(s.Tools) == 0 { - return nil, false + return ToolState{}, false } toolState, ok := s.Tools[id] if !ok { - return nil, false + return ToolState{}, false } - receipt := toolState - - return &receipt, true + return toolState, true } // AddReceipt stores a managed tool receipt.