carlos / platform Public

release/host: scope the canary-TTL nag to the hourly root pass (review)

Fix round on the canary-TTL guard rail: adoptHost's nag was firing on
every Pass/PassFilter call, which meant the in-process canaryAdoptLoop
(every 2-3s, canary/* routes only, built for FAST pickup) would nag an
unmoving expired canary every 2-3 seconds forever instead of hourly.

Pass/PassFilter now take a variadic PassOption; the one option,
NagExpiredCanaries(w io.Writer), is set ONLY by RunAdopt (carlos-adopt
.service, hourly) — the same split PR #9's config sync already uses.
adoptLoop and canaryAdoptLoop call Pass/PassFilter exactly as before
(no option, no signature change at those call sites), so the nag
defaults to off everywhere except the root path.
TestPassFilterDoesNotNagWithoutTheOption and
TestPassNagsExactlyOnceWithTheOption pin the split directly.

Also confirmed and pinned (no code change needed — it already worked
this way): countExpiredCanaries resolves each distinct
(account, app, channel) pointer at most once per call, and statusLoop
calls it once per 5-minute tick, so N routes on one canary channel
cost exactly one store read. TestCountExpiredCanariesReadsEachChannelOnce
pins it with a Get-counting store wrapper.

Paul Campbell 2 months ago 37f20f459210f24bdf22961da748afcd7f55ec8a parent 8f6ac06

4 files changed, +228 −14

internal/host/agent.go +16 −3
internal/host/host_test.go +39 −0
internal/release/adopt.go +53 −11
internal/release/adopt_test.go +120 −0

diff --git a/internal/host/agent.go b/internal/host/agent.go

index c63999cbd..d6b3da2b4 100644

--- a/internal/host/agent.go

+++ b/internal/host/agent.go

@@ -264,8 +264,14 @@ func isCanaryRoute(rt registry.Route) bool {

// answer "none" about routes that don't exist.

// Pointers are resolved at most once per distinct (account, app, channel)

-// among the routes given, mirroring adopt.go's Pass — cheap even when many

-// routes on this box share one canary channel.

+// among the routes given (the local `cache` map below), mirroring adopt.go's

+// Pass — this function is called once per 5-minute status tick

+// (statusLoop), so N routes on this box following the same canary channel

+// cost exactly ONE release.Channel() store read for that tick, not N

+// (2026-08-03 review: confirmed and pinned by

+// TestCountExpiredCanariesReadsEachChannelOnce). The cache is local to one

+// call/one tick — nothing persists across ticks, which is fine at a

+// 5-minute cadence.

func countExpiredCanaries(ctx context.Context, routes []registry.Route, store release.Store, pub ed25519.PublicKey, now time.Time) *int {

type key struct{ acct, app, channel string }

cache := map[key]*release.ChannelPointer{}

@@ -701,7 +707,14 @@ func RunAdopt(args []string) {

}

- if err := release.Pass(ctx, reg, store, pub, release.OptDir(), restartRoute); err != nil {

+ // NagExpiredCanaries(os.Stdout) is set HERE ONLY (2026-08-03 review) — the

+ // canary-TTL nag (spec §5) belongs to the hourly root pass

+ // (carlos-adopt.service, this verb's ExecStart), the same split PR #9's

+ // config sync above uses. adoptLoop and canaryAdoptLoop in this same

+ // package call release.Pass/PassFilter with no options at all, so an

+ // expired canary nags once an hour from here, never every 2-3 seconds

+ // from the in-process fast loop.

+ if err := release.Pass(ctx, reg, store, pub, release.OptDir(), restartRoute, release.NagExpiredCanaries(os.Stdout)); err != nil {

log.Fatalf("adopt: %v", err)

}

diff --git a/internal/host/host_test.go b/internal/host/host_test.go

index 3916fcb67..b2fb565f0 100644

--- a/internal/host/host_test.go

+++ b/internal/host/host_test.go

@@ -641,6 +641,45 @@ func TestCountExpiredCanariesSkipsPlatformAliases(t *testing.T) {

}

+// getCountingStore wraps a real release.Store and counts Get calls — used to

+// prove countExpiredCanaries' per-call cache actually dedupes store reads,

+// not just channel-pointer resolutions in memory.

+type getCountingStore struct {

+ release.Store

+ gets int

+func (s *getCountingStore) Get(ctx context.Context, key string) ([]byte, string, error) {

+ s.gets++

+ return s.Store.Get(ctx, key)

+// TestCountExpiredCanariesReadsEachChannelOnce pins the caching half of the

+// 2026-08-03 review: statusLoop calls countExpiredCanaries once per

+// 5-minute tick, and this function must resolve a given canary channel's

+// pointer AT MOST ONCE per call however many of this box's routes follow

+// it — N routes sharing one channel costs one store.Get, not N.

+func TestCountExpiredCanariesReadsEachChannelOnce(t *testing.T) {

+ real := release.NewDirStore(t.TempDir())

+ priv, pub := testCanaryKeys(t)

+ writeBackdatedPointer(t, real, priv, "bab", "myapp", "canary/old", "v1",

+ time.Now().Add(-(release.CanaryTTL + 24*time.Hour)))

+ counting := &getCountingStore{Store: real}

+ routes := []registry.Route{

+ {Host: "a.example.com", Kind: "instance", App: "myapp", Channel: "canary/old", Account: "bab"},

+ {Host: "b.example.com", Kind: "instance", App: "myapp", Channel: "canary/old", Account: "bab"},

+ {Host: "c.example.com", Kind: "instance", App: "myapp", Channel: "canary/old", Account: "bab"},

+ }

+ got := countExpiredCanaries(context.Background(), routes, counting, pub, time.Now())

+ if got == nil || *got != 3 {

+ t.Fatalf("count = %v, want 3 (all three routes share the one expired channel)", got)

+ }

+ if counting.gets != 1 {

+ t.Errorf("store.Get called %d times for 3 routes on ONE channel, want exactly 1 — the per-tick cache must dedupe by (account, app, channel)", counting.gets)

+ }

// shipVersion ships a minimal one-artifact release so Promote has something

// real to point at — release.Promote refuses to point a channel at a

// version nothing shipped.

diff --git a/internal/release/adopt.go b/internal/release/adopt.go

index 5406e9cb0..d01ca2262 100644

--- a/internal/release/adopt.go

+++ b/internal/release/adopt.go

@@ -9,6 +9,7 @@ import (

"crypto/ed25519"

"errors"

"fmt"

+ "io"

"os"

"runtime"

"time"

@@ -20,6 +21,36 @@ import (

// tests can exercise resolution without faking runtime.GOOS/GOARCH.

var adoptPlatform = runtime.GOOS + "-" + runtime.GOARCH

+// PassOption configures one call to Pass or PassFilter beyond their required

+// arguments. Currently the only option is NagExpiredCanaries; the type

+// exists so a call site opts in explicitly and a future option composes

+// the same way, rather than every option becoming its own trailing

+// parameter.

+type PassOption func(*passConfig)

+type passConfig struct {

+ // nag is where the canary-TTL nag (spec §5) is printed; nil (the

+ // default — every call site that does not pass NagExpiredCanaries)

+ // means the nag never fires at all, not even to a discarded writer.

+ nag io.Writer

+// NagExpiredCanaries opts a Pass/PassFilter call into spec §5's nag: one

+// line per expired canary/* route the call sees, written to w.

+//

+// This belongs to the HOURLY ROOT adopt pass alone — carlos-adopt.service's

+// RunAdopt, which is the only caller that sets it (2026-08-03 review). The

+// same split PR #9's config sync uses, for the same reason: the in-process

+// adoptLoop (hourly + jitter, inside the sandboxed carlos-edge.service) and

+// canaryAdoptLoop (every 2-3 seconds, canary/* routes only — the whole

+// point of which is FAST pickup of a freshly-promoted canary) never set

+// this. An expired canary sitting still on the fast loop would otherwise

+// nag every 2-3 seconds forever; gating it to the hourly root pass keeps

+// the nag journal-friendly — once an hour, not a poll-storm of log lines.

+func NagExpiredCanaries(w io.Writer) PassOption {

+ return func(c *passConfig) { c.nag = w }

// Pass is one full adoption sweep: group instance routes by (App, Channel),

// resolve each channel pointer once, verify signatures and artifact sha256s,

// download/stage missing versions, and repoint live/<host> symlinks.

@@ -38,8 +69,8 @@ var adoptPlatform = runtime.GOOS + "-" + runtime.GOARCH

// surfaces at wake, recovered by Rollback, which repoints back. Hibernating

// hosts are processed AFTER every awake host, so an awake host discovers a

// boot-broken build (and poisons it) before any sleeper is moved onto it.

-func Pass(ctx context.Context, reg *registry.Registry, s Store, pub ed25519.PublicKey, optDir string, restart func(host string) error) error {

- return PassFilter(ctx, reg, s, pub, optDir, restart, nil)

+func Pass(ctx context.Context, reg *registry.Registry, s Store, pub ed25519.PublicKey, optDir string, restart func(host string) error, opts ...PassOption) error {

+ return PassFilter(ctx, reg, s, pub, optDir, restart, nil, opts...)

}

// PassFilter is Pass narrowed to the routes extra additionally accepts

@@ -52,7 +83,11 @@ func Pass(ctx context.Context, reg *registry.Registry, s Store, pub ed25519.Publ

// no window"), so sweeping just them every few seconds instead of hourly

// can't turn into a fleet-wide poll storm the way sweeping every route

// that fast would.

-func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed25519.PublicKey, optDir string, restart func(host string) error, extra func(registry.Route) bool) error {

+func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed25519.PublicKey, optDir string, restart func(host string) error, extra func(registry.Route) bool, opts ...PassOption) error {

+ var cfg passConfig

+ for _, opt := range opts {

+ opt(&cfg)

+ }

routes, err := reg.List()

if err != nil {

return err

@@ -127,12 +162,12 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551

continue

}

ptr := pointers[groupKey{RouteAccount(rt), rt.App, routeChannel(rt)}]

- adoptHost(ctx, reg, s, optDir, bake, poisoned, rt, ptr, restart, now)

+ adoptHost(ctx, reg, s, optDir, bake, poisoned, rt, ptr, restart, now, cfg.nag)

}

for _, rt := range hibernating {

ptr := pointers[groupKey{RouteAccount(rt), rt.App, routeChannel(rt)}]

// restart=nil: hibernating hosts get the pure repoint — never woken.

- adoptHost(ctx, reg, s, optDir, bake, poisoned, rt, ptr, nil, now)

+ adoptHost(ctx, reg, s, optDir, bake, poisoned, rt, ptr, nil, now, cfg.nag)

}

return nil

}

@@ -182,18 +217,25 @@ func RouteAccount(rt registry.Route) string {

// pointer didn't resolve; skip quietly, the group phase already logged).

// restart==nil is the symlink-only path (hibernating hosts, or a caller with

// no proc manager): repoint and record, never wake, never health-gate.

-func adoptHost(ctx context.Context, reg *registry.Registry, s Store, optDir string, bake *FirstSeen, poisoned map[string]bool, rt registry.Route, ptr *ChannelPointer, restart func(host string) error, now time.Time) {

+// nag is passConfig.nag from the call's PassOptions — nil (the ordinary

+// case) means the canary-TTL nag below never fires, never even builds its

+// string; see NagExpiredCanaries for who sets it and why.

+func adoptHost(ctx context.Context, reg *registry.Registry, s Store, optDir string, bake *FirstSeen, poisoned map[string]bool, rt registry.Route, ptr *ChannelPointer, restart func(host string) error, now time.Time, nag io.Writer) {

if ptr == nil {

return

}

// The nag (spec §5 "Canaries expire"): visibility only, and deliberately

// BEFORE the up-to-date short-circuit below — an expired canary that

// isn't moving (the common case: the branch shipped once and nobody

- // touched it since) must still be seen every pass, not just the passes

- // that happen to change something. Nothing here blocks adopt, restart or

- // promote; it only prints.

- if CanaryExpired(ptr, now) {

- fmt.Printf(" %s: canary %s (%s) has been expired %s — still serving, nothing blocked\n",

+ // touched it since) must still be seen every pass it's part of, not just

+ // the passes that happen to change something. Nothing here blocks adopt,

+ // restart or promote; it only (optionally) prints. Gated on nag != nil

+ // rather than always printing to os.Stdout: see NagExpiredCanaries — this

+ // must stay silent on the 2-3-second canary fast loop and speak only on

+ // the hourly root adopt pass, or an expired canary sitting still would

+ // nag every few seconds forever instead of once an hour.

+ if nag != nil && CanaryExpired(ptr, now) {

+ fmt.Fprintf(nag, " %s: canary %s (%s) has been expired %s — still serving, nothing blocked\n",

rt.Host, ptr.Channel, ptr.Version, now.Sub(ptr.PromotedAt).Round(time.Hour))

}

channel := routeChannel(rt)

diff --git a/internal/release/adopt_test.go b/internal/release/adopt_test.go

index 498253d49..cab85113b 100644

--- a/internal/release/adopt_test.go

+++ b/internal/release/adopt_test.go

@@ -1,6 +1,7 @@

package release

import (

+ "bytes"

"context"

"crypto/ed25519"

"encoding/base64"

@@ -8,6 +9,7 @@ import (

"errors"

"os"

"path/filepath"

+ "strings"

"testing"

"time"

@@ -456,3 +458,121 @@ func TestPassAdoptsAnExpiredCanary(t *testing.T) {

t.Error("re-promoting an expired canary must refresh its lease, not leave it expired")

}

+// TestPassFilterDoesNotNagWithoutTheOption pins the fast-loop half of the

+// nag's scoping (2026-08-03 review): internal/host/agent.go's canaryAdoptLoop

+// calls PassFilter every 2-3 seconds for canary/* routes only, and NEVER

+// passes NagExpiredCanaries — so an expired canary that is sitting still

+// must produce ZERO nag lines from that path, not one every few seconds.

+// This is PassFilter called exactly the way canaryAdoptLoop calls it (no

+// options) against the same expired fixture TestPassAdoptsAnExpiredCanary

+// uses.

+func TestPassFilterDoesNotNagWithoutTheOption(t *testing.T) {

+ s := NewDirStore(t.TempDir())

+ ctx := context.Background()

+ priv, pub := testKeys(t)

+ opt := t.TempDir()

+ reg := testRegistry(t)

+ shipRelease(t, s, "bab", "myapp", "v1", adoptPlatform, []byte("build one"))

+ writeBackdatedPointer(t, s, priv, "bab", "myapp", "canary/x", "v1",

+ time.Now().Add(-(CanaryTTL + 30*24*time.Hour)))

+ addRoute(t, reg, "old-canary.example.com", "myapp", "canary/x", false)

+ var buf bytes.Buffer

+ withCapturedStdout(t, &buf, func() {

+ if err := PassFilter(ctx, reg, s, pub, opt, func(string) error { return nil }, isCanaryTestRoute); err != nil {

+ t.Fatalf("PassFilter: %v", err)

+ }

+ })

+ if strings.Contains(buf.String(), "expired") {

+ t.Errorf("PassFilter without NagExpiredCanaries printed a nag line:\n%s", buf.String())

+ }

+// TestPassNagsExactlyOnceWithTheOption pins the root path's half: RunAdopt

+// (carlos-adopt.service, hourly) passes NagExpiredCanaries(os.Stdout) to

+// Pass, and that call must print EXACTLY ONE nag line for one expired

+// canary route — journal-friendly, once per hourly pass, not once per host

+// revisit.

+func TestPassNagsExactlyOnceWithTheOption(t *testing.T) {

+ s := NewDirStore(t.TempDir())

+ ctx := context.Background()

+ priv, pub := testKeys(t)

+ opt := t.TempDir()

+ reg := testRegistry(t)

+ shipRelease(t, s, "bab", "myapp", "v1", adoptPlatform, []byte("build one"))

+ writeBackdatedPointer(t, s, priv, "bab", "myapp", "canary/x", "v1",

+ time.Now().Add(-(CanaryTTL + 30*24*time.Hour)))

+ addRoute(t, reg, "old-canary.example.com", "myapp", "canary/x", false)

+ var buf bytes.Buffer

+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, NagExpiredCanaries(&buf)); err != nil {

+ t.Fatalf("Pass: %v", err)

+ }

+ got := strings.Count(buf.String(), "expired")

+ if got != 1 {

+ t.Errorf("nag lines containing \"expired\" = %d, want exactly 1:\n%s", got, buf.String())

+ }

+ if !strings.Contains(buf.String(), "old-canary.example.com") {

+ t.Errorf("nag line does not name the route:\n%s", buf.String())

+ }

+// TestPassDoesNotNagAFreshCanaryEvenWithTheOption: the option opts a CALL

+// into nagging; it does not turn every canary into a nag target. A fresh

+// canary must stay silent whether or not NagExpiredCanaries is set.

+func TestPassDoesNotNagAFreshCanaryEvenWithTheOption(t *testing.T) {

+ s := NewDirStore(t.TempDir())

+ ctx := context.Background()

+ priv, pub := testKeys(t)

+ opt := t.TempDir()

+ reg := testRegistry(t)

+ shipRelease(t, s, "bab", "myapp", "v1", adoptPlatform, []byte("build one"))

+ if err := Promote(ctx, s, priv, "bab", "myapp", "canary/x", "v1"); err != nil {

+ t.Fatalf("Promote: %v", err)

+ }

+ addRoute(t, reg, "fresh-canary.example.com", "myapp", "canary/x", false)

+ var buf bytes.Buffer

+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, NagExpiredCanaries(&buf)); err != nil {

+ t.Fatalf("Pass: %v", err)

+ }

+ if strings.Contains(buf.String(), "expired") {

+ t.Errorf("a freshly-promoted canary produced a nag line:\n%s", buf.String())

+ }

+// isCanaryTestRoute mirrors host.isCanaryRoute (unexported, different

+// package) for the one fast-loop-shaped test above that needs a canary-only

+// filter without importing internal/host.

+func isCanaryTestRoute(rt registry.Route) bool { return strings.HasPrefix(rt.Channel, "canary/") }

+// withCapturedStdout redirects os.Stdout to buf for the duration of fn —

+// used ONLY to prove PassFilter's default (no NagExpiredCanaries) path

+// prints nothing canary-related even via the package's ordinary fmt.Printf

+// progress lines, which is the honest thing to assert against (adoptHost's

+// other progress lines still go to os.Stdout; NagExpiredCanaries is what

+// redirects the NAG line specifically to an injected writer instead).

+func withCapturedStdout(t *testing.T, buf *bytes.Buffer, fn func()) {

+ t.Helper()

+ r, w, err := os.Pipe()

+ if err != nil {

+ t.Fatalf("os.Pipe: %v", err)

+ }

+ orig := os.Stdout

+ os.Stdout = w

+ defer func() { os.Stdout = orig }()

+ done := make(chan struct{})

+ go func() {

+ buf.ReadFrom(r)

+ close(done)

+ }()

+ fn()

+ w.Close()

+ <-done