From 5fd53af3e792db24552e8b1cd8fcdfc8d2d99e8e Mon Sep 17 00:00:00 2001 From: rldyourmnd Date: Mon, 24 Aug 2026 03:31:56 +0500 Subject: [PATCH] refactor(queue): replace a dead store with the reason it is not needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code scanning flagged `go/useless-assignment-to-local` at queue_intent.go:673. It is a true positive: `intent = existing` is the last statement of the `if exists` branch, `intent` is redeclared by `:=` on the next iteration, and nothing reads it in between. The line was not harmless, because it read as though the journal wanted the canonical record afterwards. It does not: the `exists` branch deliberately makes no write-back, since `journal.Intents` already holds `existing` and that record carries mutable lease state a freshly rebuilt `intent` does not. Overwriting it would discard progress. The comment now says that; the assignment said nothing and implied the opposite. Behaviour is unchanged — the store had no reader. The overlay is digest-pinned by `config/garm-derivative.yaml` and `scripts/build-garm-nddev.sh` verifies each file against it before building, so editing it without updating the pin would have failed later, in a different tool, for someone else. The manifest digest is updated and the script's generated region re-rendered with the repository's own command, `gha-fleet render-garm-build`, whose `-check` mode then reports `current: true`. Worth noting for anyone reading this file: `third_party/...` matches no packages in this module, so `go vet` and `go test` never see the overlay. It is compiled only inside `build-garm-nddev.sh`, against a GARM checkout. This finding came from the scanner, not from the test suite, because the test suite cannot reach it. Verified: `go vet ./...` clean, `go test -race ./...` every package ok, `go build -trimpath ./...` ok, `render-garm-build -check` current. --- config/garm-derivative.yaml | 2 +- scripts/build-garm-nddev.sh | 2 +- third_party/garm/overlay/workers/scaleset/queue_intent.go | 6 +++++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/config/garm-derivative.yaml b/config/garm-derivative.yaml index 30d15e6..f095e53 100644 --- a/config/garm-derivative.yaml +++ b/config/garm-derivative.yaml @@ -81,7 +81,7 @@ patches: purpose: Bind authoritative repository identity before deleting a stale queued database duplicate for a GitHub in-progress job without releasing its running central admission intent; binding failures retain the row for retry, while terminal or absent jobs still release capacity exactly once. overlays: - path: third_party/garm/overlay/workers/scaleset/queue_intent.go - sha256: 0793ef4a19052849f265dfe40a06a8f3330bef053ea2fff84e080b269a85f7b7 + sha256: 18be93eabada82a284ecd50948af4e41ecd10b6eb59ae4f1e2ce1c1367e1618f purpose: Fsync-backed central queue admission where disabled scale sets cannot record or acquire work, sparse JobAssigned owns a bounded provisional token without refreshing its TTL, JobAvailable or the authoritative queued DB row binds repository identity, authoritative JobStarted rehydrates missing running ownership, and state_entered_at remains stable across same-phase lease refreshes. - path: third_party/garm/overlay/workers/scaleset/queue_intent_test.go sha256: 02449e9feaf09de10f225323053b1efbc75e810a0f7bba5113b9b27fb1856c65 diff --git a/scripts/build-garm-nddev.sh b/scripts/build-garm-nddev.sh index de69c86..c6da99b 100755 --- a/scripts/build-garm-nddev.sh +++ b/scripts/build-garm-nddev.sh @@ -92,7 +92,7 @@ readonly overlay_paths=( "third_party/garm/overlay/workers/provider/nddev_create_retry_test.go" ) readonly overlay_sha256s=( - "0793ef4a19052849f265dfe40a06a8f3330bef053ea2fff84e080b269a85f7b7" + "18be93eabada82a284ecd50948af4e41ecd10b6eb59ae4f1e2ce1c1367e1618f" "02449e9feaf09de10f225323053b1efbc75e810a0f7bba5113b9b27fb1856c65" "5f0166c596ed3946ba72e6dbca850af7573f514186acb62d39ee21ec0021ca1c" "9b6f69301d4442f459315e11e72748cf54c407098975fe4302bb69cb259ce495" diff --git a/third_party/garm/overlay/workers/scaleset/queue_intent.go b/third_party/garm/overlay/workers/scaleset/queue_intent.go index 85022fb..5c38cf1 100644 --- a/third_party/garm/overlay/workers/scaleset/queue_intent.go +++ b/third_party/garm/overlay/workers/scaleset/queue_intent.go @@ -670,7 +670,11 @@ func (c *queueIntentCoordinator) ObserveLifecycle(scaleSet params.ScaleSet, enti if !queueIntentCoreIdentityEqual(existing, intent) { return fmt.Errorf("duplicate assigned intent %q changed immutable identity", intent.Key) } - intent = existing + // Deliberately no write-back: the journal already holds `existing`, + // which carries mutable state this freshly rebuilt `intent` does not, + // so overwriting it would discard progress. This line used to read + // `intent = existing`, which looked load-bearing while being dead — + // `intent` is redeclared next iteration and nothing follows here. } else { journal.Intents[intent.Key] = intent ensureRepositoryState(journal, config, intent.Repository)