fix: the Go inventory forgets departed and opted-out workloads (ADR 0018)

cmd/agent/main.go

@@ -272,6 +272,18 @@ func run(ctx context.Context, logger *slog.Logger, clientset kubernetes.Interfac
 		}
 		inventoryMu.Lock()
 		defer inventoryMu.Unlock()
+		// Forget first, then publish. The inventory accumulates pushed facts, so
+		// nothing else would ever remove a record whose workload is gone — or
+		// whose pod the customer opted out of, which stops new facts arriving
+		// without removing the ones already held (ADR 0018). The live index is
+		// the same one workload metadata is built from, so the two payloads
+		// cannot disagree about which workloads exist.
+		evicted := goStore.Retain(inventory.LiveKeys(podWatcher.Pods()))
+		goStore.RetainNodes(nodeWatcher.Names())
+		if evicted.Records > 0 || evicted.Builds > 0 {
+			logger.Info("go inventory forgot departed workloads",
+				"records", evicted.Records, "builds", evicted.Builds)
+		}
 		goInventorySeq++
 		if err := spool.WriteGoInventory(goInventorySeq, time.Now(), goStore.Coverage(), goStore.Snapshot()); err != nil {
 			logger.Error("spooling go inventory", "error", err)

docs/adr/0018-inventory-records-live-only-while-their-workload-does.md

@@ -0,0 +1,118 @@
+# 0018. The Go inventory forgets: a record lives only while its workload does
+
+Date: 2026-08-21
+Status: Accepted
+
+Amends [ADR 0017](0017-build-facts-keyed-by-digest.md): its `coverage` block
+gains a lifetime rule for `nodes_reported`, and the record-lifetime question it
+recorded as "not addressed here" is answered. ADR 0017 is otherwise unchanged.
+
+## Context
+
+`inventory.Store` was the one place in the agent whose keyspace only ever grew.
+`Ingest` had exactly one operation on it — assign — and no code path anywhere
+removed a record. A record entered when a node reported a binary that joined,
+and left only when the controller restarted.
+
+This was not an oversight so much as a missing half. Every other payload is
+derived from state the controller can re-read: `workload_metadata` is rebuilt
+from the live informer index on every flush, so it drops what the cluster
+dropped without anything having to remember that it should. The inventory
+cannot be rebuilt that way, because it is a join of facts the *nodes push* with
+state the controller holds, and node reports arrive only once per scan
+interval. Accumulating was the right answer to that. Eviction was the part that
+was never written.
+
+Three consequences, in ascending order of seriousness.
+
+A deleted workload's record kept shipping in every snapshot, so the backend saw
+a Go service that no longer existed. Worse, `workload_metadata` had already
+forgotten it, so the two payloads disagreed about which workloads exist —
+detectable by joining them, but only by a consumer who thought to look.
+
+**The serious one: an opt-out that did not apply to what had already been
+collected.** When a customer annotates a pod with `rebuildstack.co/collect:
+"false"`, `PodWatcher` drops it from the index immediately, and the controller
+stops naming it in the scan scope it hands to nodes (ADR 0015). Both halves
+worked. What followed was that *no new fact ever arrived* about that pod — and
+a store that only accumulates has no way to act on the absence of facts. The
+record already held kept leaving the cluster, with its namespace, workload name,
+container name and main module path, in every snapshot, for as long as the
+controller ran. `docs/security.md` §11 said "the exclusion applies at the
+collection stage". For this payload it applied only to facts not yet collected.
+
+The mechanism to fix it was already in place and had one consumer too few:
+`indexPod`'s own comment says the index "drops a pod the moment" it is excluded,
+and the metadata snapshot was built on exactly that guarantee.
+
+## Decision
+
+**On every flush, the inventory forgets whatever the controller's own filtered
+pod index no longer supports.** A record whose (namespace, workload kind,
+workload name, container) is absent from the live index is dropped before the
+payload is written; the dependency set of any build no surviving record
+references is dropped with it.
+
+The live index is `PodWatcher.Pods()` — the same source `workload_metadata` is
+derived from. That is the substance of the decision, not an implementation
+detail: the two payloads now agree about which workloads exist *by
+construction*, because they read one index, rather than by two code paths
+happening to stay in step.
+
+Three properties follow, each of which was a decision:
+
+**No empty-input guard.** `Retain` with an empty live set empties the
+inventory. A guard would have been the intuitive safety measure and would have
+been wrong: a cluster where nothing passes the filters legitimately has an empty
+inventory, and a guard would pin the last non-empty snapshot forever — the exact
+failure this ADR exists to end. It is safe without one because the index is only
+ever mutated by informer events and never wiped: `PodWatcher` registers its
+handlers after the caches sync, and a relist replays as adds and updates. An
+empty index means an empty cluster, not an unsynced one.
+
+**`deps` and `written` are dropped together.** Dropping a dependency set while
+leaving its digest marked as written would mean that if the image ever came
+back, its dependency set would be re-ingested and never sent — silently, with
+nothing failing and no test naturally covering it. This is the failure mode the
+two-step `PendingDependencies`/`MarkDependenciesWritten` split of ADR 0017
+exists to make visible, and it deserves the same care here.
+
+**`nodes_reported` gets the same treatment.** ADR 0017 described it as
+cumulative since a `since` instant. That was wrong in the same way, and this
+corrects it: a node that has left the cluster is dropped from the count. Its
+whole purpose is comparison against the node count in `node_metadata`, and a
+count that outlives its nodes exceeds the fleet — so a scaled-down cluster with
+a broken DaemonSet would have looked complete, which is precisely the case the
+block was added to expose.
+
+## Consequences
+
+**Easier.** The opt-out promise is now true of every payload, and provable: the
+end-to-end test annotates a running pod and asserts its record leaves the
+payload. Memory is bounded by what currently runs rather than by everything ever
+observed. `go_inventory` and `workload_metadata` can be joined without orphans.
+
+**Harder / given up.** A record's absence now carries meaning it did not carry
+before — "this workload is not currently collected", not "we have not learned
+about it yet" — and the backend must not confuse the two. The `coverage` block
+is what distinguishes them.
+
+A short-lived workload flickers. A CronJob whose pods live thirty seconds
+appears in the snapshot covering that minute and is gone from the next. This is
+accepted, and it is the same reasoning that kept the "interesting modules" list
+out of the agent in ADR 0017: `go_inventory` is a current-state snapshot, and
+history is accumulated by the backend across the sequence of snapshots, which
+carry `sequence` and `captured_at` for exactly that. "Workloads that have ever
+been Go" is a question the analysis answers, not the agent.
+
+Such a workload also re-sends its dependency payload each time it returns, since
+the written mark goes with the set. One payload per reappearance, byte-identical
+under a key whose ingest is an upsert: churn, not error.
+
+Finally, a pod that a node scanned before the controller's informer saw it loses
+its record for at most one flush cycle and regains it on the next scan. The
+alternative — trusting a fact about a pod the controller cannot confirm exists —
+is the one this agent consistently refuses (ADR 0015).
+
+This ADR records a decision implemented in the same pull request, per the
+process in [`README.md`](README.md).

docs/adr/README.md

@@ -57,3 +57,4 @@ stays in place.
 - [0015. The node asks the controller which pods it may scan, and fails closed](0015-node-scan-scope.md)
 - [0016. Prometheus is not a data source — not as an option, not for backfill](0016-no-prometheus-as-a-source.md)
 - [0017. Immutable build facts ship once under the image digest; snapshots say when they were taken](0017-build-facts-keyed-by-digest.md)
+- [0018. The Go inventory forgets: a record lives only while its workload does](0018-inventory-records-live-only-while-their-workload-does.md)

docs/backend-requirements.md

@@ -163,14 +163,25 @@ complete it is** ([ADR 0017](adr/0017-build-facts-keyed-by-digest.md)).
 of the facts inside it. `go_inventory` additionally carries a `coverage` block
 whose counters are cumulative from the `since` instant carried with them:
 
-- `nodes_reported` — how many nodes have delivered at least one scan. Read
-  against the node count in `node_metadata`, a shortfall means part of the fleet
-  is not reporting, and the inventory is incomplete for reasons no record can
-  express.
+- `nodes_reported` — how many nodes *currently in the cluster* have delivered at
+  least one scan; a node that has left is not counted. Read against the node
+  count in `node_metadata`, a shortfall means part of the fleet is not
+  reporting, and the inventory is incomplete for reasons no record can express.
 - `facts_received` / `facts_joined` / `facts_unjoined` / `facts_undigested` —
   what arrived and what could not be attributed. **The backend MUST NOT read a
   missing workload as "not a Go workload"** when these say otherwise.
 
+**A `go_inventory` record exists only while its workload does**
+([ADR 0018](adr/0018-inventory-records-live-only-while-their-workload-does.md)).
+The agent drops records whose workload container has left its filtered pod index
+— deleted, or opted out by annotation — so a record's disappearance between two
+snapshots means "no longer collected", not "not observed this time". The backend
+MUST treat a superseding snapshot as the complete current truth and MUST NOT
+carry forward records absent from it. A short-lived workload (a CronJob's pods)
+may legitimately appear in one snapshot and not the next; accumulating "has ever
+been a Go workload" across snapshots is the backend's job, and `sequence` and
+`captured_at` are what it orders them by.
+
 **`go_dependencies` is write-once and keyed by image digest.** A build's
 dependency module paths never change, so the payload is immutable given its key
 and the agent sends it once per distinct build. The backend MUST upsert it by

docs/security.md

@@ -471,10 +471,13 @@ already collects — into two payloads:
 
 - **`go_inventory`** — one record per (namespace, workload, container) carrying
   the Go version, main module path, image digest, and PGO flag. Replicas and
-  nodes running the same build collapse to one record. It also carries a
-  `coverage` block: how many nodes have delivered a report at all, and how many
-  facts were received, joined, and not joined. Those are cluster-wide counts,
-  and they name nothing.
+  nodes running the same build collapse to one record. A record exists only
+  while its workload does: on every flush the inventory drops whatever the
+  agent's filtered pod index no longer holds, so a deleted or opted-out
+  workload leaves the payload rather than lingering as a last known state
+  (ADR 0018). It also carries a `coverage` block: how many of the cluster's
+  nodes have delivered a report, and how many facts were received, joined, and
+  not joined. Those are cluster-wide counts, and they name nothing.
 - **`go_dependencies`** — the dependency module paths of one build, keyed by its
   image digest (ADR 0017). One payload per distinct build, sent once, joined
   back to workloads through the image digest in `go_inventory`. **Module paths
@@ -635,7 +638,14 @@ metadata:
     rebuildstack.co/collect: "false"
 ```
 
-The exclusion applies at the collection stage.
+The exclusion applies at the collection stage, and it applies to what was
+already collected. The pod leaves the agent's index immediately, it stops being
+named in the scan scope the controller gives nodes (§10.2), and the next
+snapshot of each payload no longer carries it — including the Go inventory,
+which is assembled from facts nodes push and therefore has to be told to forget
+rather than simply stop learning (ADR 0018). Nothing about an opted-out pod
+survives in a later payload; a workload that stops being collected disappears
+from the next snapshot, it does not linger as a last known state.
 
 ---
 

internal/inventory/store.go

@@ -14,6 +14,7 @@ import (
 
 	"k8s.io/apimachinery/pkg/types"
 
+	"github.com/RebuildStackCo/runtime-agent/internal/collector"
 	"github.com/RebuildStackCo/runtime-agent/internal/nodescan"
 )
 
@@ -94,9 +95,12 @@ type Store struct {
 	deps    map[string]BuildDependencies
 	written map[string]struct{}
 
-	// nodesReported is the set of nodes that have delivered at least one report.
-	// Compared against the node count in the node-metadata payload it is what
-	// makes a DaemonSet that never started visible downstream.
+	// nodesReported is the set of nodes still in the cluster that have delivered
+	// at least one report. Compared against the node count in the node-metadata
+	// payload it is what makes a DaemonSet that never started visible
+	// downstream — which is why departed nodes are pruned from it (ADR 0018):
+	// a count that outlives the nodes would exceed the fleet and hide the gap
+	// it exists to show.
 	nodesReported map[string]struct{}
 
 	// since is when this store started counting: the base of every cumulative
@@ -209,6 +213,115 @@ func (s *Store) PendingDependencies() []BuildDependencies {
 	return out
 }
 
+// LiveKeys is the set of record keys the controller's own filtered pod index
+// currently supports: one per container of every pod that passed the customer's
+// filters. It is the same index workload metadata is derived from, which is the
+// point — the two payloads then agree by construction rather than by
+// convention (ADR 0018).
+//
+// Init containers are included. A native sidecar is declared as one and runs
+// for the pod's whole life, so excluding them would evict its record on every
+// flush and re-add it on the next node scan.
+func LiveKeys(pods []collector.PodInfo) []Key {
+	out := make([]Key, 0, len(pods))
+	for _, p := range pods {
+		for _, c := range p.Containers {
+			out = append(out, Key{
+				Namespace:    p.Namespace,
+				WorkloadKind: p.Workload.Kind,
+				WorkloadName: p.Workload.Name,
+				Container:    c.Name,
+			})
+		}
+	}
+	return out
+}
+
+// Evicted is what one Retain pass removed, for the coverage log.
+type Evicted struct {
+	// Records is records whose workload container is gone from the cluster.
+	Records int
+	// Builds is dependency sets no surviving record references any more.
+	Builds int
+}
+
+// Retain drops every record whose key is absent from live, and then every
+// dependency set no surviving record still references.
+//
+// The store accumulates records from facts the nodes push, so without this it
+// only ever grows: a deleted workload's record would keep shipping in every
+// snapshot until the controller restarted, and — the reason this is not merely
+// stale data — so would the record of a pod the customer opted out of. Once the
+// pod leaves the scan scope (ADR 0015) no new fact arrives to correct or remove
+// it, and the opt-out that was supposed to apply at collection would apply only
+// to facts not yet collected. Retaining against the live index is what makes
+// "the exclusion applies at the collection stage" true of this payload too.
+//
+// live may legitimately be empty — a cluster where nothing passes the filters
+// has an empty inventory — so this does not guard against it. That is safe
+// because the pod index is only ever mutated by informer events, never wiped:
+// PodWatcher registers its handlers after the caches sync, and a relist replays
+// as adds and updates. An empty index therefore means an empty cluster, not an
+// unsynced one.
+//
+// A record for a pod whose node scanned it before the controller's informer saw
+// it survives at most one flush cycle: it is evicted here, and re-added by the
+// next scan once the informer has caught up.
+func (s *Store) Retain(live []Key) Evicted {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	set := make(map[Key]struct{}, len(live))
+	for _, k := range live {
+		set[k] = struct{}{}
+	}
+	var ev Evicted
+	for k := range s.records {
+		if _, ok := set[k]; !ok {
+			delete(s.records, k)
+			ev.Records++
+		}
+	}
+
+	referenced := make(map[string]struct{}, len(s.records))
+	for _, r := range s.records {
+		if r.ImageDigest != "" {
+			referenced[r.ImageDigest] = struct{}{}
+		}
+	}
+	for digest := range s.deps {
+		if _, ok := referenced[digest]; ok {
+			continue
+		}
+		// deps and written must be dropped together. Dropping only deps would
+		// leave the digest marked as already written, so if that image ever came
+		// back its dependency set would be re-ingested and never sent again —
+		// silently, with nothing failing.
+		delete(s.deps, digest)
+		delete(s.written, digest)
+		ev.Builds++
+	}
+	return ev
+}
+
+// RetainNodes drops reporting nodes that are no longer in the cluster, so
+// nodes_reported stays comparable with the node count in the node-metadata
+// payload. Without it a cluster that scaled down would report more nodes than
+// it has, and a scaled-down fleet with a broken DaemonSet would look complete.
+func (s *Store) RetainNodes(live []string) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	set := make(map[string]struct{}, len(live))
+	for _, n := range live {
+		set[n] = struct{}{}
+	}
+	for n := range s.nodesReported {
+		if _, ok := set[n]; !ok {
+			delete(s.nodesReported, n)
+		}
+	}
+}
+
 // MarkDependenciesWritten records that the set for digest reached the sink, so
 // it is not written again. The bookkeeping is in memory only, and losing it is
 // harmless: after a restart the set is written once more, and since the payload
@@ -288,12 +401,16 @@ func (s *Store) Counters() Counters {
 // resolve is dropped. Both are counts here, never identities (CLAUDE.md
 // invariant 6).
 //
-// Every counter is cumulative from Since, which is carried alongside them so a
-// consumer never has to know the base from documentation (ADR 0013's principle,
-// applied to a structural payload).
+// The fact counters are cumulative from Since, which is carried alongside them
+// so a consumer never has to know the base from documentation (ADR 0013's
+// principle, applied to a structural payload).
 type Coverage struct {
-	Since         time.Time `json:"since"`
-	NodesReported int       `json:"nodes_reported"`
+	Since time.Time `json:"since"`
+	// NodesReported is how many nodes *currently in the cluster* have reported
+	// since Since. It is not a running total: a node that has left is dropped,
+	// so this stays directly comparable with the node count in node_metadata
+	// (ADR 0018).
+	NodesReported int `json:"nodes_reported"`
 	// FactsReceived is every binary in every report; Joined is those attributed
 	// to a workload container; Unjoined is those that could not be. Undigested
 	// counts joined facts whose container had no image digest yet, so their