fix(ci): hosted location and --coordinator flag gaps (D-1402, D-1409)

free/ci/cmd/nodes_remote.go

@@ -17,7 +17,6 @@ import (
 	"bytes"
 	"context"
 	"errors"
-	"flag"
 	"os"
 	"slices"
 	"strings"
@@ -54,11 +53,10 @@ type nodeAdminState struct {
 var nodeAdminDeps atomic.Pointer[nodeAdminState]
 
 func init() {
-	// Add --coordinator to the flag sets the lifecycle owns (help and the generated reference list it).
+	// Add --coordinator to the flag sets the lifecycle owns (help and the generated reference list it); `nodes set` and
+	// `nodes upgrade` declare it in their own flag sets.
 	for _, sub := range []string{"add", "list", "show", "test", "drain", "enable", "revoke", "remove", "rotate"} {
-		lifecycle.RegisterFlags(sub, func(fs *flag.FlagSet) {
-			fs.String("coordinator", "", "run this on the TEAM coordinator at https://host[:port] (an admin or member user credential; not for add, pair or test)")
-		})
+		lifecycle.RegisterFlags(sub, lifecycle.AddCoordinatorFlag)
 	}
 	if err := clientapi.RegisterNodeRoutes(func() clientapi.NodeRunner {
 		if nodeAdminDeps.Load() == nil {

free/ci/cmd/nodes_remote_test.go

@@ -360,3 +360,19 @@ func TestNodesRemoteUpgradeClaimIsPerNodeAndRefusesNotQueues(t *testing.T) {
 		rel()
 	}
 }
+
+// TestRoutedNodesFlagSetsListCoordinator: help and the generated flag reference come from each subcommand's
+// own flag set, so every routed `nodes` command must declare --coordinator there (D-1409).
+func TestRoutedNodesFlagSetsListCoordinator(t *testing.T) {
+	for sub := range nodesRoutedSubs {
+		key := "nodes " + sub
+		e, ok := v15Subcommands[key]
+		if !ok || e.Flags == nil {
+			t.Errorf("%s: no V15 entry with a flag set", key)
+			continue
+		}
+		if e.Flags().Lookup("coordinator") == nil {
+			t.Errorf("%s: its flag set does not declare --coordinator", key)
+		}
+	}
+}

free/ci/cmd/nodes_set.go

@@ -43,6 +43,7 @@ func nodesSetFlagSet(f *nodesSetFlags, errOut io.Writer) *flag.FlagSet {
 	fs.StringVar(&f.interactive, "interactive", "", "protect (background work waits while the operator is active) or ignore")
 	fs.StringVar(&f.onBattery, "on-battery", "", "pause (no new leases), drain (no new leases and drain the node) or allow")
 	fs.StringVar(&f.persistence, "persistence", "", "persistent or intermittent (a laptop that sleeps)")
+	lifecycle.AddCoordinatorFlag(fs)
 	return fs
 }
 

free/ci/internal/agent/update/cli.go

@@ -67,6 +67,7 @@ func FlagSet(out io.Writer) *flag.FlagSet {
 	fs.Duration("drain-wait", DefaultDrainWait, "how long to wait for running jobs before giving up")
 	fs.Duration("hello-wait", DefaultHelloWait, "how long the upgraded agent has to say hello before the upgrade is rolled back")
 	fs.Bool("json", false, "print one v1 JSON envelope")
+	lifecycle.AddCoordinatorFlag(fs)
 	return fs
 }
 

free/ci/internal/nodes/lifecycle/hooks.go

@@ -132,6 +132,13 @@ func RegisterDefaultSource(fn DefaultSourceFunc) {
 	ext.defSource = fn
 }
 
+// CoordinatorUsage is the help text of --coordinator, the one flag every routed `nodes` subcommand takes.
+const CoordinatorUsage = "run this on the TEAM coordinator at https://host[:port] (an admin or member user credential; not for add, pair or test)"
+
+// AddCoordinatorFlag declares --coordinator on a `nodes` flag set so help and the generated reference list it.
+// The cmd dispatcher takes the flag out of argv before the subcommand parses, so the value is never read here.
+func AddCoordinatorFlag(fs *flag.FlagSet) { fs.String("coordinator", "", CoordinatorUsage) }
+
 // RegisterFlags lets a later Ticket add flags to a subcommand.
 func RegisterFlags(subcommand string, fn FlagsFunc) {
 	if subcommand == "" || fn == nil {

free/ci/internal/nodes/registry/registry.go

@@ -7,6 +7,7 @@ import (
 	"github.com/nself-org/plugins/free/ci/internal/model"
 	"github.com/nself-org/plugins/free/ci/internal/store"
 	"reflect"
+	"strings"
 	"time"
 )
 
@@ -151,6 +152,10 @@ func (r *Registry) UpdateCapability(ctx context.Context, id string, in model.Cap
 		in.Availability.Persistence = old.Availability.Persistence
 		in.Availability.OnBattery = old.Availability.OnBattery
 		in.Availability.InteractivePolicy = old.Availability.InteractivePolicy
+		if !strings.HasPrefix(actor, PolicyActorPrefix) {
+			in.Location.Kind = keepHostedKind(old.Location.Kind, in.Location.Kind)
+			in.Identity.Labels = keepProviderLabels(old.Identity.Labels, in.Identity.Labels)
+		}
 		in.SecretsEligible = eligible(in)
 		if reportedIsolation.Value != nil && old.Trust.Isolation.Value != nil && isolationRank(*reportedIsolation.Value) < isolationRank(*old.Trust.Isolation.Value) {
 			in.Trust.Isolation = reportedIsolation
@@ -165,6 +170,43 @@ func (r *Registry) UpdateCapability(ctx context.Context, id string, in model.Cap
 	})
 	return n, drift, err
 }
+
+// PolicyActorPrefix marks a coordinator-side policy actor (the burst provider is "policy:burst"). Only such an
+// actor may set or clear a node's hosted location or its nself.* labels; a node report may not (D-1402), because
+// a hosted runner skips the AuthorizedProjects check in sched eligibility.
+const PolicyActorPrefix = "policy:"
+
+func isHosted(f model.Fact[string]) bool { return f.Value != nil && *f.Value == "hosted" }
+
+// keepHostedKind returns the reported location kind, except that a report cannot make a node hosted or stop it
+// being hosted: then the stored kind stays.
+func keepHostedKind(old, reported model.Fact[string]) model.Fact[string] {
+	if isHosted(old) != isHosted(reported) {
+		return old
+	}
+	return reported
+}
+
+// keepProviderLabels returns the reported labels with the nself.* ones replaced by the stored nself.* ones.
+func keepProviderLabels(old, reported []string) []string {
+	const prefix = "nself."
+	var out []string
+	for _, l := range reported {
+		if !strings.HasPrefix(l, prefix) {
+			out = append(out, l)
+		}
+	}
+	for _, l := range old {
+		if strings.HasPrefix(l, prefix) {
+			out = append(out, l)
+		}
+	}
+	if reported == nil && out == nil {
+		return nil
+	}
+	return out
+}
+
 func isolationRank(i model.Isolation) int {
 	for n, v := range model.EnumValues("Isolation") {
 		if v == string(i) {

free/ci/internal/nodes/registry/registry_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"slices"
 	"strings"
 	"sync"
 	"testing"
@@ -287,3 +288,52 @@ func TestTrustRequiresAdminAuthority(t *testing.T) {
 		t.Fatal("store accepted zero authority")
 	}
 }
+
+// TestNodeReportCannotSetHostedOrProviderLabels: a node report may not make itself hosted (which skips the
+// AuthorizedProjects check) or claim an nself.* label; the burst policy actor still can (D-1402).
+func TestNodeReportCannotSetHostedOrProviderLabels(t *testing.T) {
+	r, c := setup(t)
+	ctx := context.Background()
+	id := c.Identity.ID
+	hosted := "hosted"
+	report := c
+	report.Location.Kind = model.Fact[string]{Value: &hosted, Source: "self", ObservedAt: time.Now().UTC(), Confidence: "known"}
+	report.Identity.Labels = append(append([]string(nil), c.Identity.Labels...), "nself.repo=acme/app", "nself.provider=github", "gpu=none")
+	for _, actor := range []string{"agent", "probe", "system-install"} {
+		n, _, err := r.UpdateCapability(ctx, id, report, actor)
+		if err != nil {
+			t.Fatalf("%s: %v", actor, err)
+		}
+		if k := n.Capability.Location.Kind.Value; k != nil && *k == "hosted" {
+			t.Fatalf("%s: a node report made the node hosted", actor)
+		}
+		for _, l := range n.Capability.Identity.Labels {
+			if strings.HasPrefix(l, "nself.") {
+				t.Fatalf("%s: a node report added label %q", actor, l)
+			}
+		}
+		if !slices.Contains(n.Capability.Identity.Labels, "gpu=none") {
+			t.Fatalf("%s: an ordinary label was dropped: %v", actor, n.Capability.Identity.Labels)
+		}
+	}
+	// The coordinator-side policy actor sets both, and a later node report can neither clear nor replace them.
+	n, _, err := r.UpdateCapability(ctx, id, report, PolicyActorPrefix+"burst")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if k := n.Capability.Location.Kind.Value; k == nil || *k != "hosted" || !slices.Contains(n.Capability.Identity.Labels, "nself.repo=acme/app") {
+		t.Fatalf("policy actor could not set hosted and labels: %v %v", n.Capability.Location.Kind.Value, n.Capability.Identity.Labels)
+	}
+	back := c
+	back.Identity.Labels = []string{"nself.repo=evil/other"}
+	n, _, err = r.UpdateCapability(ctx, id, back, "agent")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if k := n.Capability.Location.Kind.Value; k == nil || *k != "hosted" {
+		t.Fatal("a node report cleared the hosted location")
+	}
+	if !slices.Contains(n.Capability.Identity.Labels, "nself.repo=acme/app") || slices.Contains(n.Capability.Identity.Labels, "nself.repo=evil/other") {
+		t.Fatalf("a node report replaced a provider label: %v", n.Capability.Identity.Labels)
+	}
+}