From 84a575e638662b1f4db87fc6336866406e1b2b46 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 14:47:27 -0400 Subject: [PATCH 1/3] feat(ci): wire the dirty-tree offload into nself ci --where (P7-AUTO-13) Nothing in cmd registered the run's OffloadHook slot, so offload.Take was reachable only through tests. cmd/v15_offload.go now registers run.SnapshotOffload from init(): on a dirty tree, a bare run or `ci runs rerun` whose --where target can reach a registered node takes a snapshot commit under refs/nself-ci/offload/, the ssh driver bundles it, and the ref is released when the run ends. Untracked files left out are named on stderr, never their content; a tracked secret-shaped file refuses the run with E807. Targets that cannot reach a node keep HEAD unchanged: local, hosted, no --where with nothing registered, and pool/auto/all when only hosted providers are registered (a hosted runner fetches from the forge, where a snapshot never exists). Tests drive runV15 end to end against an in-process ssh node: the node stages the bundle and sees the edited and untracked files but not .env, .env.local or ignored files; the repository state and refs are unchanged afterwards; E807 exits 2 naming the path only. (P7-AUTO-13) --- .../guides/ci/Auto-Placement-and-Pools.md | 2 +- free/ci/cmd/v15_offload.go | 93 ++++++++ free/ci/cmd/v15_offload_node_test.go | 222 ++++++++++++++++++ free/ci/cmd/v15_offload_test.go | 186 +++++++++++++++ 4 files changed, 502 insertions(+), 1 deletion(-) create mode 100644 free/ci/cmd/v15_offload.go create mode 100644 free/ci/cmd/v15_offload_node_test.go create mode 100644 free/ci/cmd/v15_offload_test.go diff --git a/.github/wiki/guides/ci/Auto-Placement-and-Pools.md b/.github/wiki/guides/ci/Auto-Placement-and-Pools.md index ae2f96449..6111fc367 100644 --- a/.github/wiki/guides/ci/Auto-Placement-and-Pools.md +++ b/.github/wiki/guides/ci/Auto-Placement-and-Pools.md @@ -71,7 +71,7 @@ A status of record is the commit status other people and merge rules rely on. It ## Dirty-tree offload -To run uncommitted work on another node, `nself ci` takes a snapshot: a commit of the tracked changes plus untracked files that git does not ignore, whose parent is `HEAD`. The snapshot lives under a private ref in the `refs/nself-ci/offload` namespace and is deleted when the run finishes, fails or is cancelled. Your index, branches and working tree are not touched. A clean tree offloads `HEAD` unchanged. +To run uncommitted work on another node, `nself ci` takes a snapshot: a commit of the tracked changes plus untracked files that git does not ignore, whose parent is `HEAD`. The snapshot lives under a private ref in the `refs/nself-ci/offload` namespace and is deleted when the run finishes, fails or is cancelled. Your index, branches and working tree are not touched. A clean tree offloads `HEAD` unchanged. The snapshot is taken for `--where `, and for a pool, `auto` or `all` when at least one node is registered. A run with `--where local` or `--where hosted`, or with only hosted providers registered, keeps `HEAD`: a hosted runner fetches the revision from the forge, where a snapshot never exists. Secrets are kept out by path and size, not by reading content. Untracked files that look like secrets or are too large are left out and named in the output. A tracked file of that kind refuses the offload. You can preview what would be included: diff --git a/free/ci/cmd/v15_offload.go b/free/ci/cmd/v15_offload.go new file mode 100644 index 000000000..0a2c2754e --- /dev/null +++ b/free/ci/cmd/v15_offload.go @@ -0,0 +1,93 @@ +package main + +// Purpose: the dirty-tree offload of a dev run (P7-AUTO-13, ADR 0032 D15). +// When `nself ci --where ` runs on a tree with uncommitted +// work, the run's revision becomes a snapshot commit (internal/offload) held +// under a private ref, so the NODE-03 bundle transport carries the user's exact +// tree to the node. This file registers the run's OffloadHook slot. +// Inputs: the run root (Options.Root), the --where target the invocation +// resolved (whereChosen), git through the exec funnel. +// Outputs: an OffloadSnapshot whose Release deletes the private ref; the names +// of untracked files left out go to stderr, never their content. +// Constraints: only a target that can reach a registered node takes a +// snapshot: no --where with nothing remote registered, `local`, +// `hosted`, and pool/auto/all with no node keep the HEAD path +// unchanged (a hosted runner fetches the revision from the forge, +// where a snapshot never exists). A clean tree keeps HEAD. A +// tracked secret-shaped or oversized file refuses the run (E807). The +// snapshot run stays dirty: unbound evidence, no status of record. + +import ( + "context" + "crypto/rand" + "encoding/hex" + "fmt" + "io" + "os" + "strings" + + "github.com/nself-org/plugins/free/ci/internal/offload" + "github.com/nself-org/plugins/free/ci/internal/report" + cirun "github.com/nself-org/plugins/free/ci/internal/run" + "github.com/nself-org/plugins/free/ci/internal/where" +) + +// Seams for tests. +var ( + offloadCapture offload.Capture = report.Capture + offloadErr io.Writer // nil: os.Stderr +) + +func init() { + cirun.RegisterOffload(cirun.SnapshotOffload{Build: buildOffload, Excluded: reportExcluded}) +} + +// offloadWanted reports whether a run with this --where target may execute on +// a registered node, the only kind of runner that fetches the revision by +// bundle. A pool, auto or all target with no node registered (only hosted +// providers) keeps HEAD. +func offloadWanted(ctx context.Context, kind string) bool { + switch kind { + case where.KindNode: + return true + case where.KindPool, where.KindAuto, where.KindAll: + return len(whereKnown(ctx, true).Nodes) > 0 + } + return false +} + +// buildOffload snapshots the dirty tree at root, or returns nil to keep HEAD. +func buildOffload(ctx context.Context, root string) (*cirun.OffloadSnapshot, error) { + if root == "" || !offloadWanted(ctx, whereChosen.Kind) { + return nil, nil + } + var nonce [8]byte + if _, err := rand.Read(nonce[:]); err != nil { + return nil, err + } + scratch, err := os.MkdirTemp("", "nself-ci-offload-") + if err != nil { + return nil, err + } + defer func() { _ = os.RemoveAll(scratch) }() // the temporary index and path list + capture := offloadCapture + s, err := offload.Take(ctx, capture, root, scratch, hex.EncodeToString(nonce[:])) + if err != nil { + return nil, err + } + if s.Clean { + return nil, nil + } + return &cirun.OffloadSnapshot{SHA: s.SHA, Excluded: s.Excluded, + Release: func(ctx context.Context) error { return offload.Release(ctx, capture, root, s) }}, nil +} + +// reportExcluded names the untracked files the snapshot left out. +func reportExcluded(names []string) { + w := offloadErr + if w == nil { + w = os.Stderr + } + fmt.Fprintf(w, "nself ci: offload: not sent to the node (secret-shaped or over %d MiB, untracked): %s\n", + offload.MaxFileBytes>>20, strings.Join(names, ", ")) +} diff --git a/free/ci/cmd/v15_offload_node_test.go b/free/ci/cmd/v15_offload_node_test.go new file mode 100644 index 000000000..b8ffd21ee --- /dev/null +++ b/free/ci/cmd/v15_offload_node_test.go @@ -0,0 +1,222 @@ +package main + +// Purpose: a registered ssh node behind an in-process agent session, for the +// cmd-level offload test (P7-AUTO-13). It is the test/sim foreground fixture +// (P7-AUTO-28) reduced to what `nself ci --where ` needs: the node is in +// the store under NSELF_CI_HOME, ssh.Default starts the session over pipes, and +// the node stages the checkout bundle the real driver sends, as `serve --stdio` +// does. No Docker, no network, no real ssh. +// Inputs: the run store directory and a body for the node's job. +// Outputs: what the node's job found in the tree it was given. +// Constraints: ssh.Default is restored after the test; tests do not run in parallel. + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/nself-org/cli/sdk/go/v2/remote" + "github.com/nself-org/plugins/free/ci/internal/agent/checkout" + "github.com/nself-org/plugins/free/ci/internal/agent/session" + "github.com/nself-org/plugins/free/ci/internal/agent/slots" + "github.com/nself-org/plugins/free/ci/internal/model" + "github.com/nself-org/plugins/free/ci/internal/nodes/registry" + "github.com/nself-org/plugins/free/ci/internal/planview" + "github.com/nself-org/plugins/free/ci/internal/protocol" + "github.com/nself-org/plugins/free/ci/internal/store" + "github.com/nself-org/plugins/free/ci/internal/transport/ssh" +) + +type oflKey struct{} + +func (oflKey) KeyID() string { return "k1" } +func (oflKey) PublicString() string { return "pub" } + +// oflSeen is what the node's job found in its checkout. +type oflSeen struct { + err error + head string + files map[string]string // name -> content for the probed names; a missing name is absent +} + +// oflNode is one registered node and the agent behind it. +type oflNode struct { + id, host, state string + probe []string + seen chan oflSeen +} + +// oflStage is the node side of the checkout: the lease announcement and the +// bundle's chunk frames go to the receiver and never reach the session. +type oflStage struct { + session.Carrier + r *checkout.Receiver +} + +func (c *oflStage) Recv() (protocol.Message, error) { + for { + m, err := c.Carrier.Recv() + if err != nil { + return m, err + } + switch b := m.Body.(type) { + case *protocol.Lease: + _ = c.r.Expect(*b) + case *protocol.Chunk: + if b.Stream != checkout.Stream { + return m, nil + } + epoch := uint64(0) + if m.Epoch != nil { + epoch = *m.Epoch + } + _ = c.r.Feed(m.LeaseID, epoch, *b) + continue + } + return m, nil + } +} + +// serve runs one agent session; the job stages the bundle, records the tree and passes. +func (n *oflNode) serve(ctx context.Context, table *slots.Table, in io.Reader, out io.Writer) error { + car, err := session.NewStreamCarrier(in, out, protocol.CarrierPolicy{Carrier: "A", Personal: true, OperatorOwned: true}, true) + if err != nil { + return err + } + recv := checkout.NewReceiver(filepath.Join(n.state, "checkout"), nil) + job := session.RunnerFunc(func(ctx context.Context, j session.JobRun) (session.JobResult, error) { + s := oflSeen{files: map[string]string{}} + src, err := recv.Stage(ctx, j.Lease, j.Root.Dir) + if err != nil { + s.err = err + n.seen <- s + return session.JobResult{}, err + } + if b, e := exec.Command("git", "-C", src, "rev-parse", "HEAD").Output(); e == nil { + s.head = strings.TrimSpace(string(b)) + } + for _, name := range n.probe { + if b, e := os.ReadFile(filepath.Join(src, name)); e == nil { + s.files[name] = string(b) + } + } + n.seen <- s + j.Output([]byte("tree checked\n")) + ev, _ := json.Marshal(model.Evidence{Schema: "ci.evidence/v1", Revision: j.Lease.Checkout.Revision, + Attempt: &model.AttemptRef{ID: j.Lease.AttemptID, N: 1}, Result: "pass"}) + return session.JobResult{Outcome: session.Outcome{State: "passed"}, Evidence: ev}, nil + }) + s, err := session.New(session.Config{Carrier: &oflStage{Carrier: car, r: recv}, Key: oflKey{}, Slots: table, Runner: job, StateDir: n.state, + Hooks: []session.JobHook{}, Coordinator: session.Coordinator{ID: "coord", RootFingerprint: "fp"}, + Facts: session.Facts{AgentVersion: minAgentVersion, MachineID: n.id}}) + if err != nil { + return err + } + return s.Run(ctx) +} + +// start is the ssh Starter: the "connection" is a pair of pipes to n's session. +func (n *oflNode) start(_ context.Context, tgt remote.Target, _ string) (*ssh.Conn, error) { + if !strings.HasSuffix(tgt.Dest, "@"+n.host) { + return nil, fmt.Errorf("no node at %s", tgt.Dest) + } + toAgentR, toAgentW := io.Pipe() + fromAgentR, fromAgentW := io.Pipe() + errR, errW := io.Pipe() + table, err := slots.Open(n.state, 2) + if err != nil { + return nil, err + } + actx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { + e := n.serve(actx, table, toAgentR, fromAgentW) + _ = fromAgentW.Close() + _ = errW.Close() + done <- e + }() + kill := func() error { _ = fromAgentW.Close(); _ = toAgentR.Close(); cancel(); return nil } + return &ssh.Conn{Stdin: toAgentW, Stdout: fromAgentR, Stderr: errR, Wait: func() error { <-done; return nil }, Kill: kill}, nil +} + +// oflRegisterNode registers node id in the store under home and points ssh.Default at it. +func oflRegisterNode(t *testing.T, home, repo, id string, probe ...string) *oflNode { + t.Helper() + n := &oflNode{id: id, host: "10.0.0.5", state: t.TempDir(), probe: probe, seen: make(chan oflSeen, 4)} + ctx := context.Background() + st, err := store.Open(filepath.Join(home, "state.db"), store.Options{}) + if err != nil { + t.Fatal(err) + } + defer func() { _ = st.Close() }() + pin := remote.PinnedHostKeys{Path: filepath.Join(home, "known_hosts")} + if err = pin.Add("nself-ci-"+id, n.host+" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDummyKeyForTestsOnlyDummyKeyForTestsOnly000"); err != nil { + t.Fatal(err) + } + c, err := planview.LocalCapability(planview.LocalFacts{Project: repo, Platform: runtime.GOOS + "/" + runtime.GOARCH, CPUs: 8, MemMB: 8192, Now: time.Now()}) + if err != nil { + t.Fatal(err) + } + lan, owned, uid := "lan", "owned", true + c.Identity.ID, c.Identity.Name, c.Identity.Provider, c.Identity.Transport = id, id, "ssh", "ssh" + c.Identity.SSH = &model.CapabilitySSH{Hostname: n.host, Port: 22, User: "ci"} + c.SecretsEligible = true + c.Location.Kind.Value, c.Economics.Class.Value = &lan, &owned + c.Separation.UIDSeparation = model.Fact[bool]{Value: &uid, Source: "self", ObservedAt: time.Now(), Confidence: "known"} + doc, err := json.Marshal(c) + if err != nil { + t.Fatal(err) + } + if err = st.NodePut(ctx, store.NodeRecord{ID: id, Name: id, State: "offline", Lifecycle: []byte("{}"), Trust: []byte("{}"), + DeployHost: []byte("{}"), Capability: doc, ObservedAt: time.Now().UnixNano()}, "test"); err != nil { + t.Fatal(err) + } + admin, err := store.NewAdminAuthority("test") + if err != nil { + t.Fatal(err) + } + trust := model.CapabilityTrust{ + Accepts: oflAssigned([]model.TrustClass{"owner", "internal"}), + Isolation: oflAssigned(model.Isolation("container")), + Network: oflAssigned([]model.NetworkScope{"none", "internet", "restricted", "lan", "privileged"}), + SecretClasses: oflAssigned([]model.SecretClass{"none"}), + PrivacyZone: oflAssigned(model.PrivacyZone("private-infrastructure")), + } + reg := registry.New(st) + if _, err = reg.SetTrust(ctx, id, trust, admin); err != nil { + t.Fatal(err) + } + if _, err = reg.SetState(ctx, id, "online", "", "test"); err != nil { + t.Fatal(err) + } + d := ssh.Default + home0, start0, ver0, hb0 := d.Home, d.Start, d.SSHVersion, d.Heartbeat + d.Home, d.Start, d.Heartbeat = home, n.start, time.Second + d.SSHVersion = func(context.Context) (remote.Version, error) { return remote.Version{Major: 9, Minor: 6}, nil } + t.Cleanup(func() { d.Home, d.Start, d.SSHVersion, d.Heartbeat = home0, start0, ver0, hb0 }) + return n +} + +func oflAssigned[T any](v T) model.Fact[T] { + return model.Fact[T]{Value: &v, Source: "assigned", ObservedAt: time.Now(), Confidence: "known"} +} + +// wait returns what the node's job saw, or fails when no job reached the node. +func (n *oflNode) wait(t *testing.T) oflSeen { + t.Helper() + select { + case s := <-n.seen: + return s + case <-time.After(60 * time.Second): + t.Fatal("no job reached the node") + } + panic("unreachable") +} diff --git a/free/ci/cmd/v15_offload_test.go b/free/ci/cmd/v15_offload_test.go new file mode 100644 index 000000000..feaffd26d --- /dev/null +++ b/free/ci/cmd/v15_offload_test.go @@ -0,0 +1,186 @@ +package main + +// Purpose: P7-AUTO-13 through the real CLI path. `nself ci --where ` on a +// tree with uncommitted work runs runV15 -> parseBare (--where) -> the +// Coordinator -> the registered OffloadHook -> offload.Take -> the ssh driver's +// bundle -> the node's checkout, and the node's job sees the user's tree. +// Inputs: a committed repository (newEvRepo) made dirty, one registered node. +// Outputs: exit codes, stderr, what the node saw, the repository before/after. +// Constraints: NSELF_V15=1 (newEvRepo); no Docker, no network, no real ssh. + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/nself-org/plugins/free/ci/internal/where" +) + +const ( + oflEnvSecret = "ENV_VALUE_MUST_NOT_TRAVEL" + oflLocalSecret = "ENV_LOCAL_VALUE_MUST_NOT_TRAVEL" +) + +func resetWhere(t *testing.T) { + t.Cleanup(func() { whereValue, whereChosen = "", where.Target{} }) +} + +// dirtyNodeRepo is a committed repository with an ignore file, made dirty the +// way a laptop is: an edited tracked file, a new file, an ignored .env and an +// untracked .env.local; node n1 is registered and reachable. HOME is private: +// git reads the user's global excludes, which may already ignore .env.local. +func dirtyNodeRepo(t *testing.T) (*evRepo, *oflNode, string) { + t.Helper() + resetWhere(t) + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + r := newEvRepo(t) + r.commit("f.txt", "committed\n") + head := r.commit(".gitignore", ".env\nignored.txt\n") + n := oflRegisterNode(t, r.home, r.dir, "n1", "f.txt", "new.txt", ".env", ".env.local", "ignored.txt") + put(t, r.dir, "f.txt", "edited on the laptop\n") + put(t, r.dir, "new.txt", "untracked and not ignored\n") + put(t, r.dir, ".env", oflEnvSecret+"\n") + put(t, r.dir, ".env.local", oflLocalSecret+"\n") + put(t, r.dir, "ignored.txt", "ignored\n") + return r, n, head +} + +// oflRepoState is everything about the user's repository an offload must leave alone. +func oflRepoState(t *testing.T, dir string) string { + t.Helper() + parts := []string{ + git(t, dir, "status", "--porcelain=v2", "--ignored"), + git(t, dir, "for-each-ref"), + git(t, dir, "ls-files", "-s"), + git(t, dir, "rev-parse", "HEAD"), + } + for _, name := range []string{"f.txt", "new.txt", ".env", ".env.local", "ignored.txt", ".gitignore"} { + b, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + t.Fatal(err) + } + parts = append(parts, name+"="+string(b)) + } + return strings.Join(parts, "\n--\n") +} + +func TestCLIWhereNodeOffloadsDirtyTree(t *testing.T) { + r, n, head := dirtyNodeRepo(t) + before := oflRepoState(t, r.dir) + out, errOut, code := r.ci("--where", "n1") + if code != 0 { + t.Fatalf("exit %d\nstdout %s\nstderr %s", code, out, errOut) + } + if whereChosen.Kind != where.KindNode { + t.Fatalf("--where n1 resolved to %q, want a node target", whereChosen.Kind) + } + s := n.wait(t) + if s.err != nil { + t.Fatalf("the node could not stage the checkout: %v", s.err) + } + if s.head == "" || s.head == head { + t.Fatalf("node HEAD %q: want the snapshot commit, not the committed HEAD %s", s.head, head) + } + if parent := git(t, r.dir, "rev-parse", "--verify", "-q", s.head+"^"); parent != head { + t.Errorf("snapshot parent %s, want HEAD %s", parent, head) + } + if s.files["f.txt"] != "edited on the laptop\n" || s.files["new.txt"] != "untracked and not ignored\n" { + t.Errorf("the node did not get the user's tree: %q", s.files) + } + for _, name := range []string{".env", ".env.local", "ignored.txt"} { + if c, ok := s.files[name]; ok { + t.Errorf("%s reached the node (%d bytes)", name, len(c)) + } + } + if !strings.Contains(errOut, "not sent to the node") || !strings.Contains(errOut, ".env.local") { + t.Errorf("the excluded untracked .env.local is not named:\nstderr %s\nstdout %s", errOut, out) + } + for _, secret := range []string{oflEnvSecret, oflLocalSecret} { + if strings.Contains(out+errOut, secret) { + t.Errorf("secret content %q reached the output", secret) + } + } + if after := oflRepoState(t, r.dir); after != before { + t.Errorf("the user's repository changed:\n--- before\n%s\n--- after\n%s", before, after) + } + if refs := git(t, r.dir, "for-each-ref", "refs/nself-ci/"); refs != "" { + t.Errorf("private refs left behind:\n%s", refs) + } +} + +func TestCLIWhereNodeTrackedSecretRefused(t *testing.T) { + resetWhere(t) + r := newEvRepo(t) + r.commit("deploy.pem", "-----BEGIN NOT A REAL KEY-----\n") + oflRegisterNode(t, r.home, r.dir, "n1") + put(t, r.dir, "f.txt", "uncommitted\n") + before := oflRepoState2(t, r.dir) + out, errOut, code := r.ci("--where", "n1") + if code != 2 || !strings.Contains(errOut, "E807") || !strings.Contains(errOut, "deploy.pem") { + t.Fatalf("a tracked secret-shaped file must refuse the offload with E807 and exit 2: exit %d\nstdout %s\nstderr %s", code, out, errOut) + } + if strings.Contains(out+errOut, "NOT A REAL KEY") { + t.Error("the refusal printed the file's content") + } + if after := oflRepoState2(t, r.dir); after != before { + t.Errorf("the user's repository changed:\n--- before\n%s\n--- after\n%s", before, after) + } +} + +// oflRepoState2 is the git part of oflRepoState, for a repository without the probe files. +func oflRepoState2(t *testing.T, dir string) string { + t.Helper() + return strings.Join([]string{git(t, dir, "status", "--porcelain=v2", "--ignored"), git(t, dir, "for-each-ref"), + git(t, dir, "ls-files", "-s")}, "\n--\n") +} + +// TestOffloadOnlyForTargetsThatReachANode: a run with no --where, `local`, +// `hosted`, or pool/auto/all with only hosted providers registered keeps the +// HEAD path and never runs git for a snapshot. +func TestOffloadOnlyForTargetsThatReachANode(t *testing.T) { + resetWhere(t) + was := offloadCapture + t.Cleanup(func() { offloadCapture = was }) + offloadCapture = func(context.Context, ...string) (string, int, error) { + t.Fatal("a snapshot was attempted for a run no node can take") + return "", 1, nil + } + ctx := context.Background() + withKnown(t, where.Known{Providers: []string{"github"}}, "") + for _, kind := range []string{"", where.KindLocal, where.KindHosted, where.KindPool, where.KindAuto, where.KindAll} { + whereChosen = where.Target{Kind: kind} + if s, err := buildOffload(ctx, t.TempDir()); s != nil || err != nil { + t.Errorf("kind %q, providers only: snapshot %v err %v, want the HEAD path", kind, s, err) + } + } + withKnown(t, where.Known{Nodes: []string{"n1"}, Providers: []string{"github"}}, "") + for _, kind := range []string{where.KindNode, where.KindPool, where.KindAuto, where.KindAll} { + if !offloadWanted(ctx, kind) { + t.Errorf("kind %q may run on a node and must offload a dirty tree", kind) + } + } + for _, kind := range []string{"", where.KindLocal, where.KindHosted} { + if offloadWanted(ctx, kind) { + t.Errorf("kind %q never runs on a node", kind) + } + } +} + +// TestOffloadCleanTreeKeepsHEAD: a node target on a clean tree (the hook also +// runs when only the config lives outside the tree) creates no snapshot and no ref. +func TestOffloadCleanTreeKeepsHEAD(t *testing.T) { + resetWhere(t) + r := newEvRepo(t) + whereChosen = where.Target{Kind: where.KindNode} + s, err := buildOffload(context.Background(), r.dir) + if s != nil || err != nil { + t.Fatalf("clean tree: snapshot %v err %v", s, err) + } + if refs := git(t, r.dir, "for-each-ref", "refs/nself-ci/"); refs != "" { + t.Errorf("a clean tree left a ref:\n%s", refs) + } +} From 1488cf0af6f4c3d723e5b57545e812ac8b33ac44 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 15:04:01 -0400 Subject: [PATCH 2/3] fix(ci): snapshot staged files and keep the run's revision the commit (P7-AUTO-13) offload.Take built its temporary index from HEAD, so a file added with `git add` and not committed never reached the snapshot and the node tested a tree other than the user's. Paths the user's index tracks that HEAD lacks, and that exist in the working tree, are now added too. Git runs with GIT_OPTIONAL_LOCKS=0 (status no longer refreshes the user's index) and GIT_LITERAL_PATHSPECS=1 (a file name is never a pattern). A snapshot replaced the run's revision, so a dirty run, including an `auto` run that stayed local, recorded a commit that no ref keeps once the run ends. That breaks `ci runs reproduce` after gc and contradicts P7-CI-41 (a reproduction never uses uncommitted changes). The snapshot is now only what nodes check out: Session.Checkout feeds the lease and the remote evidence check, while the run records HEAD, dirty, and never posts a status of record. The offload unit tests isolate HOME, XDG_CONFIG_HOME and git's global and system config, so a host's global excludes no longer change them. (P7-AUTO-13) --- .../guides/ci/Auto-Placement-and-Pools.md | 2 +- free/ci/cmd/v15_offload.go | 7 ++- free/ci/cmd/v15_offload_test.go | 57 ++++++++++++++++++ free/ci/internal/offload/snapshot.go | 39 ++++++++++-- free/ci/internal/offload/snapshot_test.go | 59 +++++++++++++++++++ free/ci/internal/run/coordinator.go | 6 +- free/ci/internal/run/dispatch_hook.go | 4 +- free/ci/internal/run/hooks_auto.go | 22 ++++--- free/ci/internal/run/hooks_slots_test.go | 6 +- free/ci/internal/run/offload_checkout_test.go | 26 ++++++++ free/ci/internal/run/session.go | 21 +++++-- 11 files changed, 221 insertions(+), 28 deletions(-) create mode 100644 free/ci/internal/run/offload_checkout_test.go diff --git a/.github/wiki/guides/ci/Auto-Placement-and-Pools.md b/.github/wiki/guides/ci/Auto-Placement-and-Pools.md index 6111fc367..2f8ce0d8c 100644 --- a/.github/wiki/guides/ci/Auto-Placement-and-Pools.md +++ b/.github/wiki/guides/ci/Auto-Placement-and-Pools.md @@ -71,7 +71,7 @@ A status of record is the commit status other people and merge rules rely on. It ## Dirty-tree offload -To run uncommitted work on another node, `nself ci` takes a snapshot: a commit of the tracked changes plus untracked files that git does not ignore, whose parent is `HEAD`. The snapshot lives under a private ref in the `refs/nself-ci/offload` namespace and is deleted when the run finishes, fails or is cancelled. Your index, branches and working tree are not touched. A clean tree offloads `HEAD` unchanged. The snapshot is taken for `--where `, and for a pool, `auto` or `all` when at least one node is registered. A run with `--where local` or `--where hosted`, or with only hosted providers registered, keeps `HEAD`: a hosted runner fetches the revision from the forge, where a snapshot never exists. +To run uncommitted work on another node, `nself ci` takes a snapshot: a commit of the tracked changes (staged or not, including files added with `git add` and not yet committed) plus untracked files that git does not ignore, whose parent is `HEAD`. The snapshot lives under a private ref in the `refs/nself-ci/offload` namespace and is deleted when the run finishes, fails or is cancelled. Your index, branches and working tree are not touched. A clean tree offloads `HEAD` unchanged. The snapshot is only what the node checks out: the run records the commit it started from, marked dirty, so `ci runs reproduce` never depends on a snapshot that no ref keeps. The snapshot is taken for `--where `, and for a pool, `auto` or `all` when at least one node is registered. A run with `--where local` or `--where hosted`, or with only hosted providers registered, keeps `HEAD`: a hosted runner fetches the revision from the forge, where a snapshot never exists. Secrets are kept out by path and size, not by reading content. Untracked files that look like secrets or are too large are left out and named in the output. A tracked file of that kind refuses the offload. You can preview what would be included: diff --git a/free/ci/cmd/v15_offload.go b/free/ci/cmd/v15_offload.go index 0a2c2754e..4f72c42b5 100644 --- a/free/ci/cmd/v15_offload.go +++ b/free/ci/cmd/v15_offload.go @@ -2,9 +2,10 @@ package main // Purpose: the dirty-tree offload of a dev run (P7-AUTO-13, ADR 0032 D15). // When `nself ci --where ` runs on a tree with uncommitted -// work, the run's revision becomes a snapshot commit (internal/offload) held -// under a private ref, so the NODE-03 bundle transport carries the user's exact -// tree to the node. This file registers the run's OffloadHook slot. +// work, nodes check out a snapshot commit (internal/offload) held under a +// private ref, so the NODE-03 bundle transport carries the user's exact tree; +// the run still records the commit it started from, dirty. This file +// registers the run's OffloadHook slot. // Inputs: the run root (Options.Root), the --where target the invocation // resolved (whereChosen), git through the exec funnel. // Outputs: an OffloadSnapshot whose Release deletes the private ref; the names diff --git a/free/ci/cmd/v15_offload_test.go b/free/ci/cmd/v15_offload_test.go index feaffd26d..d508f78a8 100644 --- a/free/ci/cmd/v15_offload_test.go +++ b/free/ci/cmd/v15_offload_test.go @@ -15,6 +15,8 @@ import ( "strings" "testing" + "github.com/nself-org/plugins/free/ci/internal/nodes/registry" + "github.com/nself-org/plugins/free/ci/internal/store" "github.com/nself-org/plugins/free/ci/internal/where" ) @@ -110,6 +112,61 @@ func TestCLIWhereNodeOffloadsDirtyTree(t *testing.T) { if refs := git(t, r.dir, "for-each-ref", "refs/nself-ci/"); refs != "" { t.Errorf("private refs left behind:\n%s", refs) } + requireRecordedHEAD(t, r, head) +} + +// requireRecordedHEAD: the run's recorded revision is the commit, never the +// snapshot, which no ref keeps once the run ends (P7-CI-41: a reproduction +// never uses uncommitted changes). +func requireRecordedHEAD(t *testing.T, r *evRepo, head string) { + t.Helper() + st, err := store.Open(filepath.Join(r.home, "state.db"), store.Options{}) + if err != nil { + t.Fatal(err) + } + defer func() { _ = st.Close() }() + rows, err := st.PipelinesByRevision(context.Background(), head) + if err != nil || len(rows) != 1 { + t.Fatalf("runs recorded at HEAD %s: %d %v; the run must record the commit, not the snapshot", head, len(rows), err) + } +} + +// TestCLIAutoDirtyRunStaysLocal: with a node registered, a bare run is `auto` +// and takes a snapshot, but the node is offline, so the job runs here. The run +// records HEAD, the snapshot ref is gone and the repository is unchanged, so no +// recorded revision depends on an unreachable commit. +func TestCLIAutoDirtyRunStaysLocal(t *testing.T) { + r, n, head := dirtyNodeRepo(t) + st, err := store.Open(filepath.Join(r.home, "state.db"), store.Options{}) + if err != nil { + t.Fatal(err) + } + if _, err = registry.New(st).SetState(context.Background(), n.id, "offline", "", "test"); err != nil { + t.Fatal(err) + } + if err = st.Close(); err != nil { + t.Fatal(err) + } + before := oflRepoState(t, r.dir) + out, errOut, code := r.ci("--no-status") + if code != 0 || whereChosen.Kind != where.KindAuto { + t.Fatalf("exit %d kind %q\nstdout %s\nstderr %s", code, whereChosen.Kind, out, errOut) + } + if !strings.Contains(errOut, "not sent to the node") { + t.Fatalf("auto with a node registered must take the snapshot (no exclusion notice):\n%s", errOut) + } + select { + case s := <-n.seen: + t.Fatalf("an offline node ran the job: %+v", s) + default: + } + requireRecordedHEAD(t, r, head) + if refs := git(t, r.dir, "for-each-ref", "refs/nself-ci/"); refs != "" { + t.Errorf("private refs left behind:\n%s", refs) + } + if after := oflRepoState(t, r.dir); after != before { + t.Errorf("the user's repository changed:\n--- before\n%s\n--- after\n%s", before, after) + } } func TestCLIWhereNodeTrackedSecretRefused(t *testing.T) { diff --git a/free/ci/internal/offload/snapshot.go b/free/ci/internal/offload/snapshot.go index 6868abad1..0d0a6c2f6 100644 --- a/free/ci/internal/offload/snapshot.go +++ b/free/ci/internal/offload/snapshot.go @@ -1,8 +1,9 @@ package offload // Purpose: turn a dirty working tree into a revision (P7-AUTO-13): a snapshot -// commit of tracked changes plus untracked files git does not ignore, parent -// HEAD, held under the private ref refs/nself-ci/offload/ so the NODE-03 +// commit of the working tree of every path the user's index tracks (staged or +// not, including a file added and not yet committed) plus untracked files git +// does not ignore, parent HEAD, held under the private ref refs/nself-ci/offload/ so the NODE-03 // bundle transport can carry it. // Inputs: the repository root, a scratch dir and an id. // Outputs: a Snapshot (sha, ref, names excluded) and a Release that deletes @@ -50,7 +51,9 @@ func Take(ctx context.Context, capture Capture, root, scratch, id string) (*Snap if index != "" { argv = append(argv, "GIT_INDEX_FILE="+index) } - argv = append(argv, "GIT_AUTHOR_NAME=nself-ci", "GIT_AUTHOR_EMAIL=ci@localhost", "GIT_COMMITTER_NAME=nself-ci", "GIT_COMMITTER_EMAIL=ci@localhost", + // GIT_OPTIONAL_LOCKS=0: `status` must not refresh the user's index; GIT_LITERAL_PATHSPECS=1: a + // file name is a path, never a pathspec pattern or magic. + argv = append(argv, "GIT_OPTIONAL_LOCKS=0", "GIT_LITERAL_PATHSPECS=1", "GIT_AUTHOR_NAME=nself-ci", "GIT_AUTHOR_EMAIL=ci@localhost", "GIT_COMMITTER_NAME=nself-ci", "GIT_COMMITTER_EMAIL=ci@localhost", "git", "--no-replace-objects", "-c", "core.fsmonitor=false", "-c", "core.hooksPath=/dev/null", "-C", root) out, code, err := capture(ctx, append(argv, args...)...) if err != nil || code != 0 { @@ -97,10 +100,15 @@ func Take(ctx context.Context, capture Capture, root, scratch, id string) (*Snap index, list := filepath.Join(scratch, "index-"+id), filepath.Join(scratch, "paths-"+id) defer os.Remove(index) defer os.Remove(list) - if err := os.WriteFile(list, keep, 0o600); err != nil { + if _, err := g(index, "read-tree", head); err != nil { return nil, err } - if _, err := g(index, "read-tree", head); err != nil { + inHead, err := g(index, "ls-files", "-z") + if err != nil { + return nil, err + } + keep = append(keep, stagedNew(root, split(tracked), split(inHead))...) + if err := os.WriteFile(list, keep, 0o600); err != nil { return nil, err } if _, err := g(index, "add", "-u"); err != nil { @@ -141,6 +149,27 @@ func Release(ctx context.Context, capture Capture, root string, s *Snapshot) err return nil } +// stagedNew lists, NUL-terminated, the paths the user's index tracks that HEAD +// does not (a file added with `git add` and not committed) and that exist in +// the working tree: `add -u` over a HEAD index never sees them. +func stagedNew(root string, tracked, inHead []string) []byte { + head := make(map[string]bool, len(inHead)) + for _, p := range inHead { + head[p] = true + } + var out []byte + for _, p := range tracked { + if head[p] { + continue + } + head[p] = true // an unmerged path is listed once per stage + if _, err := os.Lstat(filepath.Join(root, p)); err == nil { + out = append(append(out, p...), 0) + } + } + return out +} + func split(z string) []string { var out []string for _, p := range strings.Split(z, "\x00") { diff --git a/free/ci/internal/offload/snapshot_test.go b/free/ci/internal/offload/snapshot_test.go index 13ff8d3f8..a9593851a 100644 --- a/free/ci/internal/offload/snapshot_test.go +++ b/free/ci/internal/offload/snapshot_test.go @@ -39,7 +39,24 @@ func write(t *testing.T, root, name, body string) { } } +// hermeticGit keeps the host's git configuration out of the test: Take runs +// git with the process environment, and a global excludes file (for example +// one listing .env.local) would change what counts as untracked. +func hermeticGit(t *testing.T) { + t.Helper() + home := t.TempDir() + global := filepath.Join(home, "gitconfig") + if err := os.WriteFile(global, nil, 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + t.Setenv("GIT_CONFIG_GLOBAL", global) +} + func newRepo(t *testing.T) string { + hermeticGit(t) root := t.TempDir() git(t, root, "init", "-q") write(t, root, "a.txt", "one\n") @@ -155,3 +172,45 @@ func TestSecretPathAndBadID(t *testing.T) { t.Fatal("bad id accepted") } } + +// A file added with `git add` and not committed is part of the user's tree: the +// snapshot carries its working-tree content. A staged file deleted from the +// working tree is not carried. The user's index file is byte-identical after. +func TestSnapshotCarriesStagedNewFile(t *testing.T) { + root := newRepo(t) + write(t, root, "staged.txt", "staged\n") + write(t, root, "edited.txt", "staged version\n") + write(t, root, "gone.txt", "g\n") + git(t, root, "add", "staged.txt", "edited.txt", "gone.txt") + write(t, root, "edited.txt", "working tree version\n") + if err := os.Remove(filepath.Join(root, "gone.txt")); err != nil { + t.Fatal(err) + } + indexPath := filepath.Join(root, ".git", "index") + before, err := os.ReadFile(indexPath) + if err != nil { + t.Fatal(err) + } + s, err := Take(context.Background(), capture, root, t.TempDir(), "r5") + if err != nil { + t.Fatal(err) + } + after, err := os.ReadFile(indexPath) + if err != nil { + t.Fatal(err) + } + defer Release(context.Background(), capture, root, s) + files := "\n" + git(t, root, "ls-tree", "-r", "--name-only", s.SHA) + "\n" + if !strings.Contains(files, "\nstaged.txt\n") { + t.Errorf("a staged new file is missing from the snapshot:%s", files) + } + if got := git(t, root, "show", s.SHA+":edited.txt"); got != "working tree version" { + t.Errorf("edited.txt = %q, want the working-tree content", got) + } + if strings.Contains(files, "\ngone.txt\n") { + t.Errorf("a staged file deleted from the working tree was sent:%s", files) + } + if !bytes.Equal(before, after) { + t.Error("the user's index file changed") + } +} diff --git a/free/ci/internal/run/coordinator.go b/free/ci/internal/run/coordinator.go index df19d313b..be18bce58 100644 --- a/free/ci/internal/run/coordinator.go +++ b/free/ci/internal/run/coordinator.go @@ -179,12 +179,12 @@ func (c *Coordinator) Run(ctx context.Context) (Result, error) { ciexec.WarmRuntimeInfo() // the first job's orphan sweep asks the daemon: let the answer arrive while git and policy run rev := GitRevision(ctx, c.Opts.Root, c.capture()) rev.Dirty = rev.Dirty || c.ConfigOutside - rev, release, err := c.offload(ctx, h, rev) + checkout, release, err := c.offload(ctx, h, rev) // a snapshot is what nodes check out; the run records rev if err != nil { return Result{}, err } defer release() - rev.Dirty = rev.Dirty || c.ConfigOutside // a snapshot revision never hides a config that lives outside the tree + rev.Dirty = rev.Dirty || checkout != rev.SHA // a run whose nodes test a snapshot is a dev run: never a status of record admitted, source, err := c.classify(ctx, h, rev) if err != nil { return Result{}, err @@ -216,7 +216,7 @@ func (c *Coordinator) Run(ctx context.Context) (Result, error) { return Result{}, err } c.exec = newExecutor(staleChecker{st, bootID()}, h.NewRedactor, h.Isolation) - sess.Store, sess.Coordinator, sess.Exec, sess.RunID, sess.Revision = st, coord, c.exec, runID, rev.SHA + sess.Store, sess.Coordinator, sess.Exec, sess.RunID, sess.Revision, sess.Checkout = st, coord, c.exec, runID, rev.SHA, checkout sess.Host, sess.Home, sess.Admission = hostName(), c.Opts.Home, c.Admission sess.Admission.Force = sess.Admission.Force || c.Opts.Force sess.Plan, sess.Record = c.Plan, c.Opts.Record diff --git a/free/ci/internal/run/dispatch_hook.go b/free/ci/internal/run/dispatch_hook.go index 6f36a3ab6..ab5ddf951 100644 --- a/free/ci/internal/run/dispatch_hook.go +++ b/free/ci/internal/run/dispatch_hook.go @@ -104,7 +104,7 @@ func dispatchHook(ctx context.Context, s AttemptSpec) (Admitted, error) { _ = sess.Store.SetPreviousRunner(ctx, id, prev) } } - rd.sink.expect(id, sess.Revision) // a remote carrier may report before this call returns + rd.sink.expect(id, sess.checkoutRevision()) // a remote carrier may report before this call returns; its evidence names the checkout a, err := rd.d.Submit(ctx, queueJob(sess, s, sj, id, max(dec.Slots*cpu, cpu), max(capacity.FreeMemoryMB-capacity.MemoryMB*dec.ReserveMemoryPercent/100, mem))) if err != nil || a.Local { @@ -119,7 +119,7 @@ func dispatchHook(ctx context.Context, s AttemptSpec) (Admitted, error) { // queueJob is the dispatcher's input for one attempt. Only an interactive run // (a person at a terminal) keeps the implicit foreground queue cap. func queueJob(sess *Session, s AttemptSpec, sj sched.Job, id string, localCPU, localMemMB int) dispatch.Job { - return dispatch.Job{AttemptID: id, Sched: sj, Model: *s.Job, Revision: sess.Revision, Foreground: s.Foreground, + return dispatch.Job{AttemptID: id, Sched: sj, Model: *s.Job, Revision: sess.checkoutRevision(), Foreground: s.Foreground, NoDefaultQueueCap: !sess.Interactive, LocalCPU: localCPU, LocalMemMB: localMemMB} } diff --git a/free/ci/internal/run/hooks_auto.go b/free/ci/internal/run/hooks_auto.go index c744b0f8f..632948108 100644 --- a/free/ci/internal/run/hooks_auto.go +++ b/free/ci/internal/run/hooks_auto.go @@ -57,8 +57,12 @@ type OffloadRequest struct { Rev Revision } -// OffloadResult replaces the run's revision when Rev.SHA is set (a snapshot -// commit of a dirty tree). Release runs when the run ends; it may be nil. +// OffloadResult names, when Rev.SHA is set, the revision a node checks out +// for this run (a snapshot commit of a dirty tree). The run itself keeps +// recording the committed revision it started from, dirty and unbound: a +// reproduction never uses uncommitted changes (P7-CI-41), and the snapshot is +// unreachable once Release deletes its ref. Release runs when the run ends; +// it may be nil. type OffloadResult struct { Rev Revision Release func() @@ -147,10 +151,11 @@ func (h AutoHooks) resolved(r AutoHooks) AutoHooks { return out } -// offload runs the offload hook; the returned release is never nil. -func (c *Coordinator) offload(ctx context.Context, h Hooks, rev Revision) (Revision, func(), error) { +// offload runs the offload hook and returns the revision nodes check out +// (rev.SHA when no snapshot was made); the returned release is never nil. +func (c *Coordinator) offload(ctx context.Context, h Hooks, rev Revision) (string, func(), error) { if h.Auto.Offload == nil { - return rev, func() {}, nil + return rev.SHA, func() {}, nil } var res OffloadResult err := guard("offload hook", func() (e error) { @@ -158,15 +163,16 @@ func (c *Coordinator) offload(ctx context.Context, h Hooks, rev Revision) (Revis return }) if err != nil { - return rev, func() {}, err + return rev.SHA, func() {}, err } + checkout := rev.SHA if res.Rev.SHA != "" { - rev = res.Rev + checkout = res.Rev.SHA } if res.Release == nil { res.Release = func() {} } - return rev, res.Release, nil + return checkout, res.Release, nil } // race calls the race hook, if any. diff --git a/free/ci/internal/run/hooks_slots_test.go b/free/ci/internal/run/hooks_slots_test.go index 1bed77086..95dd3b325 100644 --- a/free/ci/internal/run/hooks_slots_test.go +++ b/free/ci/internal/run/hooks_slots_test.go @@ -240,7 +240,8 @@ func TestPublishGateComposes(t *testing.T) { } // TestOffloadAndNotifyFailures: an offload error refuses the run before anything runs; a -// replaced revision is the one the run binds to; a failing or panicking notify hook is a note. +// snapshot revision is only what nodes check out (the run records HEAD, dirty and unbound); +// a failing or panicking notify hook is a note. func TestOffloadAndNotifyFailures(t *testing.T) { l := &slotLog{} r := passRig(t) @@ -259,6 +260,9 @@ func TestOffloadAndNotifyFailures(t *testing.T) { if released != 1 || len(r.Rep.all()) != 0 || !strings.Contains(res.Summary.Status, "not posted") { t.Fatalf("a snapshot revision is dev evidence and is released once: released %d status %q", released, res.Summary.Status) } + if res.Summary.Doc.Revision != testSHA { + t.Fatalf("the run recorded %q: a snapshot is unreachable after release, so the run records HEAD %s", res.Summary.Doc.Revision, testSHA) + } for name, n := range map[string]fakeNotify{"error": {l: l, err: errors.New("sink down")}, "panic": {l: l, panic: true}} { r = passRig(t) diff --git a/free/ci/internal/run/offload_checkout_test.go b/free/ci/internal/run/offload_checkout_test.go new file mode 100644 index 000000000..f9cc2d776 --- /dev/null +++ b/free/ci/internal/run/offload_checkout_test.go @@ -0,0 +1,26 @@ +package run + +import ( + "testing" + + "github.com/nself-org/plugins/free/ci/internal/model" + "github.com/nself-org/plugins/free/ci/internal/sched" +) + +// TestOffloadCheckoutRevision: a node's lease checks out the snapshot and its +// evidence must name it, while the run's own revision stays the commit +// (P7-AUTO-13, P7-CI-41). Without a snapshot the lease checks out the run's revision. +func TestOffloadCheckoutRevision(t *testing.T) { + const head, snap = "1111111111111111111111111111111111111111", "2222222222222222222222222222222222222222" + job := &model.Job{} + if got := queueJob(&Session{Revision: head, Checkout: snap}, AttemptSpec{Job: job}, sched.Job{}, "a1", 1, 1).Revision; got != snap { + t.Errorf("lease revision %q, want the snapshot", got) + } + if got := queueJob(&Session{Revision: head}, AttemptSpec{Job: job}, sched.Job{}, "a1", 1, 1).Revision; got != head { + t.Errorf("lease revision %q, want HEAD without a snapshot", got) + } + s := &remoteSink{by: map[string]*remoteAttempt{}} + if ra := s.expect("a1", (&Session{Revision: head, Checkout: snap}).checkoutRevision()); ra.revision != snap { + t.Errorf("the sink expects %q, want the snapshot the node checked out", ra.revision) + } +} diff --git a/free/ci/internal/run/session.go b/free/ci/internal/run/session.go index a09daedff..724625537 100644 --- a/free/ci/internal/run/session.go +++ b/free/ci/internal/run/session.go @@ -32,11 +32,14 @@ type Session struct { Exec *ciexec.Executor RunID string Revision string - Host string - Home string - Poll time.Duration - Capacity func() ciexec.Capacity - Admission ciexec.AdmissionPolicy + // Checkout is the revision a node checks out: the dirty-tree snapshot of the + // offload hook (P7-AUTO-13), or "" for Revision. + Checkout string + Host string + Home string + Poll time.Duration + Capacity func() ciexec.Capacity + Admission ciexec.AdmissionPolicy // Plan is the run's plan and Record its Options.Record: the dispatch hook maps both // into the scheduler job (P7-AUTO-28); a Session built without them still places. Plan *plan.Plan @@ -148,3 +151,11 @@ func hostName() string { } return safeID(h) } + +// checkoutRevision is the revision a node checks out (Checkout, else Revision). +func (s *Session) checkoutRevision() string { + if s.Checkout != "" { + return s.Checkout + } + return s.Revision +} From fdfa057dbaac33273c173a70305f9f7636cdfdeb Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 15:23:42 -0400 Subject: [PATCH 3/3] fix(ci): a hosted runner never receives a dirty-tree snapshot (P7-AUTO-13) A hosted runner fetches the revision from the forge, where a snapshot commit never exists. When the dispatcher delivers a lease of a snapshot run to a hosted runner, the transport adapter now gives that lease the committed HEAD instead, the run expects evidence naming HEAD, and the run notes "dirty changes not included: hosted runner" once. Leases to enrolled nodes keep the snapshot; runs without one are unchanged. Tests: TestHostedLeaseNeverGetsSnapshot covers node and hosted delivery of one snapshot run and a clean run; TestCLIDirtyRunNodeWithHostedRegistered registers a node and a hosted runner and places on the node through the CLI. (P7-AUTO-13) --- .../guides/ci/Auto-Placement-and-Pools.md | 2 +- free/ci/cmd/v15_offload_node_test.go | 45 +++++++++++--- free/ci/cmd/v15_offload_test.go | 29 +++++++++ free/ci/internal/run/coordinator.go | 1 + free/ci/internal/run/dispatch_hook.go | 10 ++- free/ci/internal/run/offload_checkout_test.go | 61 +++++++++++++++++++ free/ci/internal/run/offload_hook_hosted.go | 51 ++++++++++++++++ 7 files changed, 187 insertions(+), 12 deletions(-) create mode 100644 free/ci/internal/run/offload_hook_hosted.go diff --git a/.github/wiki/guides/ci/Auto-Placement-and-Pools.md b/.github/wiki/guides/ci/Auto-Placement-and-Pools.md index 2f8ce0d8c..7cc38feb8 100644 --- a/.github/wiki/guides/ci/Auto-Placement-and-Pools.md +++ b/.github/wiki/guides/ci/Auto-Placement-and-Pools.md @@ -71,7 +71,7 @@ A status of record is the commit status other people and merge rules rely on. It ## Dirty-tree offload -To run uncommitted work on another node, `nself ci` takes a snapshot: a commit of the tracked changes (staged or not, including files added with `git add` and not yet committed) plus untracked files that git does not ignore, whose parent is `HEAD`. The snapshot lives under a private ref in the `refs/nself-ci/offload` namespace and is deleted when the run finishes, fails or is cancelled. Your index, branches and working tree are not touched. A clean tree offloads `HEAD` unchanged. The snapshot is only what the node checks out: the run records the commit it started from, marked dirty, so `ci runs reproduce` never depends on a snapshot that no ref keeps. The snapshot is taken for `--where `, and for a pool, `auto` or `all` when at least one node is registered. A run with `--where local` or `--where hosted`, or with only hosted providers registered, keeps `HEAD`: a hosted runner fetches the revision from the forge, where a snapshot never exists. +To run uncommitted work on another node, `nself ci` takes a snapshot: a commit of the tracked changes (staged or not, including files added with `git add` and not yet committed) plus untracked files that git does not ignore, whose parent is `HEAD`. The snapshot lives under a private ref in the `refs/nself-ci/offload` namespace and is deleted when the run finishes, fails or is cancelled. Your index, branches and working tree are not touched. A clean tree offloads `HEAD` unchanged. The snapshot is only what the node checks out: the run records the commit it started from, marked dirty, so `ci runs reproduce` never depends on a snapshot that no ref keeps. The snapshot is taken for `--where `, and for a pool, `auto` or `all` when at least one node is registered. A run with `--where local` or `--where hosted`, or with only hosted providers registered, keeps `HEAD`: a hosted runner fetches the revision from the forge, where a snapshot never exists. For the same reason, a job that placement gives to a hosted runner checks out `HEAD` without your uncommitted changes, and the run prints `dirty changes not included: hosted runner`. Secrets are kept out by path and size, not by reading content. Untracked files that look like secrets or are too large are left out and named in the output. A tracked file of that kind refuses the offload. You can preview what would be included: diff --git a/free/ci/cmd/v15_offload_node_test.go b/free/ci/cmd/v15_offload_node_test.go index b8ffd21ee..55dd7e9c4 100644 --- a/free/ci/cmd/v15_offload_node_test.go +++ b/free/ci/cmd/v15_offload_node_test.go @@ -123,11 +123,21 @@ func (n *oflNode) serve(ctx context.Context, table *slots.Table, in io.Reader, o return s.Run(ctx) } -// start is the ssh Starter: the "connection" is a pair of pipes to n's session. -func (n *oflNode) start(_ context.Context, tgt remote.Target, _ string) (*ssh.Conn, error) { - if !strings.HasSuffix(tgt.Dest, "@"+n.host) { - return nil, fmt.Errorf("no node at %s", tgt.Dest) +// oflHosts maps a registered runner's ssh host to its in-process agent (one test at a time). +var oflHosts map[string]*oflNode + +// oflStart is the ssh Starter: it picks the runner by destination host. +func oflStart(ctx context.Context, tgt remote.Target, s string) (*ssh.Conn, error) { + for host, n := range oflHosts { + if strings.HasSuffix(tgt.Dest, "@"+host) { + return n.start(ctx, tgt, s) + } } + return nil, fmt.Errorf("no node at %s", tgt.Dest) +} + +// start opens a session on n: the "connection" is a pair of pipes. +func (n *oflNode) start(_ context.Context, _ remote.Target, _ string) (*ssh.Conn, error) { toAgentR, toAgentW := io.Pipe() fromAgentR, fromAgentW := io.Pipe() errR, errW := io.Pipe() @@ -150,7 +160,15 @@ func (n *oflNode) start(_ context.Context, tgt remote.Target, _ string) (*ssh.Co // oflRegisterNode registers node id in the store under home and points ssh.Default at it. func oflRegisterNode(t *testing.T, home, repo, id string, probe ...string) *oflNode { t.Helper() - n := &oflNode{id: id, host: "10.0.0.5", state: t.TempDir(), probe: probe, seen: make(chan oflSeen, 4)} + return oflRegisterRunner(t, home, repo, id, false, probe...) +} + +// oflRegisterRunner is oflRegisterNode for an enrolled node, or, with hosted, for a +// runner whose capability says it is hosted (sched.RunnerHosted): the dispatcher +// then delivers its lease as a hosted one, over the same in-process session. +func oflRegisterRunner(t *testing.T, home, repo, id string, hosted bool, probe ...string) *oflNode { + t.Helper() + n := &oflNode{id: id, host: id + ".runners.test", state: t.TempDir(), probe: probe, seen: make(chan oflSeen, 4)} ctx := context.Background() st, err := store.Open(filepath.Join(home, "state.db"), store.Options{}) if err != nil { @@ -166,6 +184,9 @@ func oflRegisterNode(t *testing.T, home, repo, id string, probe ...string) *oflN t.Fatal(err) } lan, owned, uid := "lan", "owned", true + if hosted { + lan = "hosted" + } c.Identity.ID, c.Identity.Name, c.Identity.Provider, c.Identity.Transport = id, id, "ssh", "ssh" c.Identity.SSH = &model.CapabilitySSH{Hostname: n.host, Port: 22, User: "ci"} c.SecretsEligible = true @@ -197,11 +218,15 @@ func oflRegisterNode(t *testing.T, home, repo, id string, probe ...string) *oflN if _, err = reg.SetState(ctx, id, "online", "", "test"); err != nil { t.Fatal(err) } - d := ssh.Default - home0, start0, ver0, hb0 := d.Home, d.Start, d.SSHVersion, d.Heartbeat - d.Home, d.Start, d.Heartbeat = home, n.start, time.Second - d.SSHVersion = func(context.Context) (remote.Version, error) { return remote.Version{Major: 9, Minor: 6}, nil } - t.Cleanup(func() { d.Home, d.Start, d.SSHVersion, d.Heartbeat = home0, start0, ver0, hb0 }) + if oflHosts == nil { + d := ssh.Default + home0, start0, ver0, hb0 := d.Home, d.Start, d.SSHVersion, d.Heartbeat + d.Home, d.Start, d.Heartbeat = home, oflStart, time.Second + d.SSHVersion = func(context.Context) (remote.Version, error) { return remote.Version{Major: 9, Minor: 6}, nil } + oflHosts = map[string]*oflNode{} + t.Cleanup(func() { d.Home, d.Start, d.SSHVersion, d.Heartbeat, oflHosts = home0, start0, ver0, hb0, nil }) + } + oflHosts[n.host] = n return n } diff --git a/free/ci/cmd/v15_offload_test.go b/free/ci/cmd/v15_offload_test.go index d508f78a8..982c58bf5 100644 --- a/free/ci/cmd/v15_offload_test.go +++ b/free/ci/cmd/v15_offload_test.go @@ -16,6 +16,7 @@ import ( "testing" "github.com/nself-org/plugins/free/ci/internal/nodes/registry" + cirun "github.com/nself-org/plugins/free/ci/internal/run" "github.com/nself-org/plugins/free/ci/internal/store" "github.com/nself-org/plugins/free/ci/internal/where" ) @@ -169,6 +170,34 @@ func TestCLIAutoDirtyRunStaysLocal(t *testing.T) { } } +// TestCLIDirtyRunNodeWithHostedRegistered: a node and a hosted runner are both +// registered and placement picks the node: the node gets the snapshot, the +// hosted runner gets nothing and no hosted note is printed. (A foreground CLI +// run never places on a hosted runner: the run's world drops them unless a +// `ci serve` coordinator runs the pipeline; test/sim offload_hosted_test.go +// covers a hosted placement.) +func TestCLIDirtyRunNodeWithHostedRegistered(t *testing.T) { + r, n, head := dirtyNodeRepo(t) + h := oflRegisterRunner(t, r.home, r.dir, "h1", true) + out, errOut, code := r.ci("--where", "n1", "--no-status") + if code != 0 { + t.Fatalf("exit %d\nstdout %s\nstderr %s", code, out, errOut) + } + s := n.wait(t) + if s.err != nil || s.head == "" || s.head == head || s.files["f.txt"] != "edited on the laptop\n" || s.files["new.txt"] != "untracked and not ignored\n" { + t.Fatalf("the node did not get the snapshot: head %q err %v files %q", s.head, s.err, s.files) + } + if strings.Contains(out+errOut, cirun.HostedDirtyNote) { + t.Errorf("a node placement printed the hosted note:\n%s%s", out, errOut) + } + select { + case got := <-h.seen: + t.Errorf("the hosted runner ran a job placed on the node: %+v", got) + default: + } + requireRecordedHEAD(t, r, head) +} + func TestCLIWhereNodeTrackedSecretRefused(t *testing.T) { resetWhere(t) r := newEvRepo(t) diff --git a/free/ci/internal/run/coordinator.go b/free/ci/internal/run/coordinator.go index be18bce58..ce2fc21c6 100644 --- a/free/ci/internal/run/coordinator.go +++ b/free/ci/internal/run/coordinator.go @@ -249,6 +249,7 @@ func (c *Coordinator) Run(ctx context.Context) (Result, error) { if end := GitRevision(context.WithoutCancel(ctx), c.Opts.Root, c.capture()); end.SHA != rev.SHA || end.Dirty { rev.Dirty = true // the tree moved or changed while jobs ran: the result is not bound to the start commit } + post.Notes = append(post.Notes, dispatchNotes(runID)...) res := c.conclude(finishIn{St: st, RunID: runID, Rev: rev, Source: source, Policy: pol, Hooks: h, Jobs: rs.outcomes(), Cancelled: rs.userCancel || ctx.Err() != nil, Partial: c.Opts.Job != "" || c.Opts.Pipeline != "", Start: start, Post: post, Recovered: recovered, Skipped: c.Plan.Skipped, Begin: rs.begin}) rs.render.Done(res.Summary) diff --git a/free/ci/internal/run/dispatch_hook.go b/free/ci/internal/run/dispatch_hook.go index ab5ddf951..d54aace8b 100644 --- a/free/ci/internal/run/dispatch_hook.go +++ b/free/ci/internal/run/dispatch_hook.go @@ -47,6 +47,9 @@ type runDispatch struct { qp schedpolicy.QueuePolicy sink *remoteSink // what the carriers of this run's remote leases report to + + notesMu sync.Mutex + notes []string // run notes from delivery (offload_hook_hosted.go) } var ( @@ -173,7 +176,7 @@ func runDispatcher(ctx context.Context, sess *Session) (*runDispatch, error) { rd.d, err = dispatch.New(dispatch.Config{Store: sess.Store, Host: sess.Host, World: rd.world, Poll: sess.Poll, LeaseTTL: RemoteLeaseTTL, Queue: rd.queue, Slots: func() int { return rd.sess.Capacity().CPUs }, Incarnation: func() int64 { return highestIncarnation(rd.sess.Store) }, - Transport: newTransportAdapter(rd.sink, rd.sink, rd.row.Project), + Transport: transportAdapter{sink: rd.sink, reg: rd.sink, root: rd.row.Project, hosted: rd.hostedRevision}, DeliveryError: func(a dispatch.Admitted, err error) { rd.sink.ended(a.AttemptID, err) }}) if err != nil { return nil, err @@ -204,6 +207,8 @@ type transportAdapter struct { sink transport.Sink reg carriers root string // the repository whose revision a node receives + // hosted maps the revision of a lease to a hosted runner (nil = unchanged): see offload_hook_hosted.go. + hosted func(attemptID, revision string) string } func (transportAdapter) Known(kind string) bool { @@ -220,6 +225,9 @@ func (a transportAdapter) Deliver(ctx context.Context, d dispatch.Delivery) erro if RemoteRepo != nil && a.root != "" { RemoteRepo(a.root) } + if d.Hosted && a.hosted != nil { + d.Revision = a.hosted(d.AttemptID, d.Revision) + } refs, err := leaseSecretRefs(d) if err != nil { return err diff --git a/free/ci/internal/run/offload_checkout_test.go b/free/ci/internal/run/offload_checkout_test.go index f9cc2d776..acb245bff 100644 --- a/free/ci/internal/run/offload_checkout_test.go +++ b/free/ci/internal/run/offload_checkout_test.go @@ -1,10 +1,14 @@ package run import ( + "context" + "fmt" "testing" + "time" "github.com/nself-org/plugins/free/ci/internal/model" "github.com/nself-org/plugins/free/ci/internal/sched" + "github.com/nself-org/plugins/free/ci/internal/transport" ) // TestOffloadCheckoutRevision: a node's lease checks out the snapshot and its @@ -24,3 +28,60 @@ func TestOffloadCheckoutRevision(t *testing.T) { t.Errorf("the sink expects %q, want the snapshot the node checked out", ra.revision) } } + +// TestHostedLeaseNeverGetsSnapshot: with a dirty-tree snapshot in the run, a lease +// delivered to an enrolled node checks out the snapshot, while a lease delivered to a +// hosted runner checks out the committed HEAD (a hosted runner fetches from the +// forge, where no snapshot exists), its evidence must name HEAD, and the run notes +// say the dirty changes were not included. A run without a snapshot is unchanged. +// The run engine itself does not place a foreground dev run on a hosted runner today +// (dropProviderRunners, and privacy.hosted_not_authorized outside a public +// repository), so the delivery boundary, where Hosted is known, is what is tested. +func TestHostedLeaseNeverGetsSnapshot(t *testing.T) { + const head, snap = "1111111111111111111111111111111111111111", "2222222222222222222222222222222222222222" + probe := &probeDriver{kind: fmt.Sprintf("offloadprobe%d", time.Now().UnixNano())} + if err := transport.Register(probe); err != nil { + t.Fatal(err) + } + var node model.Capability + node.Identity.ID, node.Identity.Transport = "x1", probe.kind + deliver := func(t *testing.T, rd *runDispatch, attempt, revision string, hosted bool) (lease, expected string) { + t.Helper() + rd.sink.expect(attempt, revision) + d := deliveryFor(node, model.Job{Kind: "test", Run: []string{"true"}}) + d.AttemptID, d.LeaseID, d.Revision, d.Hosted = attempt, "lease-"+attempt, revision, hosted + ad := transportAdapter{hosted: rd.hostedRevision} + if err := ad.Deliver(context.Background(), d); err != nil { + t.Fatal(err) + } + probe.mu.Lock() + defer probe.mu.Unlock() + return probe.lease.Checkout.Revision, rd.sink.attempt(attempt).revision + } + newRD := func(id, checkout string) *runDispatch { + rd := &runDispatch{sess: &Session{RunID: id, Revision: head, Checkout: checkout}, sink: &remoteSink{by: map[string]*remoteAttempt{}}} + runDispatchers.Store(id, rd) + t.Cleanup(func() { runDispatchers.Delete(id) }) + return rd + } + + rd := newRD("r-offload-hosted", snap) + if lease, want := deliver(t, rd, "a1", snap, false); lease != snap || want != snap { + t.Errorf("node lease %s expects %s: want the snapshot", lease, want) + } + if n := dispatchNotes("r-offload-hosted"); len(n) != 0 { + t.Errorf("a node placement noted %v", n) + } + if lease, want := deliver(t, rd, "a2", snap, true); lease != head || want != head { + t.Errorf("hosted lease %s expects %s: want HEAD %s, never the snapshot", lease, want, head) + } + deliver(t, rd, "a3", snap, true) + if n := dispatchNotes("r-offload-hosted"); len(n) != 1 || n[0] != HostedDirtyNote { + t.Errorf("notes %v, want the hosted note once", n) + } + + clean := newRD("r-offload-clean", "") + if lease, want := deliver(t, clean, "a1", head, true); lease != head || want != head || len(dispatchNotes("r-offload-clean")) != 0 { + t.Errorf("a run without a snapshot: hosted lease %s expects %s notes %v", lease, want, dispatchNotes("r-offload-clean")) + } +} diff --git a/free/ci/internal/run/offload_hook_hosted.go b/free/ci/internal/run/offload_hook_hosted.go new file mode 100644 index 000000000..f22c95f66 --- /dev/null +++ b/free/ci/internal/run/offload_hook_hosted.go @@ -0,0 +1,51 @@ +package run + +// Purpose: a hosted runner never receives a dirty-tree snapshot (P7-AUTO-13). +// A hosted runner fetches the revision from the forge, where a snapshot never +// exists; when placement picks one for a run whose nodes check out a snapshot, +// the lease carries the commit the run started from instead, the run expects +// evidence naming that commit, and the run output says the dirty changes were +// not included. +// Inputs: the attempt id and the revision the dispatcher put on the lease. +// Outputs: the revision the hosted lease checks out; a run note. +// Constraints: a lease to an enrolled node is never changed; a run without a +// snapshot is never changed. The note is printed once per run. + +import "slices" + +// HostedDirtyNote is the run note when a hosted runner gets HEAD without the +// uncommitted changes of a dirty tree. +const HostedDirtyNote = "dirty changes not included: hosted runner" + +// hostedRevision is the transport adapter's hosted hook for this run. +func (rd *runDispatch) hostedRevision(attemptID, revision string) string { + snap := rd.sess.Checkout + if snap == "" || snap == rd.sess.Revision || revision != snap { + return revision + } + if ra := rd.sink.attempt(attemptID); ra != nil { + ra.revision = rd.sess.Revision // the hosted runner's evidence names the commit it fetched + } + rd.note(HostedDirtyNote) + return rd.sess.Revision +} + +func (rd *runDispatch) note(n string) { + rd.notesMu.Lock() + defer rd.notesMu.Unlock() + if !slices.Contains(rd.notes, n) { + rd.notes = append(rd.notes, n) + } +} + +// dispatchNotes returns the delivery notes of a run whose dispatcher is still open. +func dispatchNotes(runID string) []string { + v, ok := runDispatchers.Load(runID) + if !ok { + return nil + } + rd := v.(*runDispatch) + rd.notesMu.Lock() + defer rd.notesMu.Unlock() + return append([]string(nil), rd.notes...) +}