[Philip Lombardi (plombardi89)]

Summary

This PR adds designs/agent-v2/, a design for an experimental v2 of the unbounded agent. It is opened for commentary on the design; there are no code changes.

v2 would ship inside the existing binary as unbounded-agent v2 <command>, logging a warning that it is experimental. It has two goals:

1. A defined interface for hosting kubelet. NodeRuntime has two implementations:
   • host: kubelet and containerd run directly on the machine.
   • nspawn-container: today's behavior, and the parity reference.
2. Consumers use the binary, not the library. AKS Flex Node would customize the agent binary instead of composing its own agent from pkg/agent. The hook points are:
   • a versioned configuration file;
   • declarative extensions for extra node and host content, ordering, and cleanup;
   • exec plugins for desired state, status, credentials and preflight;
   • lifecycle hooks.

All v2 code would start in internal/agentv2. Moving anything to an exported package requires an explicit, recorded decision.

How to review

Start with the README. It covers the purpose, goals, architecture and terms, and lists the other documents with who should read each one:

Document	Covers
01-cli-and-configuration.md	The v2 commands, the configuration file, install-time and goal settings, credentials
02-extensions.md	Lifecycle hooks, declarative extensions, the plugin protocol, and the AKS Flex Node mapping
03-node-runtime.md	The NodeRuntime interface and its rules, the node bundle, slot states, boot units, the two runtimes
04-control-and-state.md	Goal sources, status, credentials, the node manager, state files, the daemon's reconcile, MachineOperations
05-lifecycles.md	Bootstrap, repave, NodeReboot, AgentUpgrade and reset
06-host-changes.md	Everything v2 creates on a host, the host network reset, host preparation and its reversal
07-host-runtime-cleanup.md	How the host runtime stops, cleans up and removes a node
08-v1-compatibility.md	Parity with v1, intended behavior changes, copied code, converting a v1 configuration
09-implementation.md	Package layout, export policy, risks
10-migration-from-v1.md	How a v1 host moves to v2 without replacing its node, early or at the cutover at the first official v2 release, with a safe rollback
11-testing.md	Running the v1 end-to-end suites on v2, the migration suite, checking reset with a host snapshot, CI

Where review is most needed:

• The external contract, in documents 01 and 02.
• The internal interfaces, in documents 03 and 04.
• host runtime cleanup, in document 07.
• Migration from v1, in document 10, and testing, in document 11.
• Behavior changes from v1, in document 08, section 2. v2 should have the same outcomes as v1; each remaining difference states its reason.

[Philip Lombardi (plombardi89)]

AI-assisted review (Claude via OpenCode). These findings were checked against the code where cited, but please verify each one before acting on it.

Line references like L123 point to designs/agent-v2.md at da8073e4f. Code citations point to the doc's baseline (dca6c964d).

Overall: The goals are clear, the v1 research is thorough, and most of the citations are accurate. The main problems are:

• several state-machine gaps that would cause real bugs, such as a bootstrap token change triggering an unwanted wipe;
• a hook and plugin surface that is larger than any stated consumer needs;
• internal inconsistencies between sections 3, 5.6, 10 and 17.

The doc could also be much shorter. Section 10 alone is about 460 lines of implementation detail.

1. Gaps to resolve before implementation

1. Re-rendering the bundle can trigger an unwanted wipe.

   • The bundle is not saved; it is rendered again when needed, including by the boot hooks (L826-827, L725).
   • Render needs a NodeSpec, and Resolve takes bootstrap credentials and a host probe (L791). The bootstrap credential may come from a plugin "called before every provision" (L586).
   • A new token changes the bundle's content and therefore its digest. The host runtime treats a changed digest at Start as a slot change and wipes node state (L1505-1508). "Same digest is a no-op" (L978) also stops holding during Recover.
   • v1's pre-start hook needs only the applied config, not credentials (pkg/agent/nspawnlifecycle/lifecycle.go:67-93).
   • Suggestion: save the bundle, or a secret-free manifest of it, in the slot at Provision. Keep secrets out of the digest. Have Hook take only the device and NVIDIA inputs it needs.

2. Two records of the active slot, with no rule for when they disagree.

   • state.Applied.ActiveSlot (L949) and the host runtime's active-slot.json (L1503) both track it. ErrSlotActive (L753) means the runtime also decides what is "active".
   • A crash after Start B writes the runtime's record but before the node manager commits (L1125) leaves the two out of step. Recover (L1133-1137) only looks at Pending and A's phase.
   • The slot phases (L728) and the host runtime's active-slot phases (L1511-1521) are never mapped to each other, so what Inspect returns for host is undefined.

3. Nothing defines which slot starts after a host reboot.

   • Section 8.3 says systemd starts "the active slot's boot unit" (L1041), but the interface never says that Start enables a slot or that Stop disables it.
   • v1 enables the machine when it starts it (pkg/agent/phases/nodestart/nspawn.go:99) and disables it on reset (pkg/agent/phases/reset/machine.go:39).
   • The doc should also say that lifecycle hooks do not fire when systemd restarts the node after a host reboot.

4. Repaves and stuck states can fail with no defined outcome.

   • If B never becomes ready, Recover keeps going back to "continue from Start B" (L1135). Nothing defines a backoff, a terminal state, a rollback, or what status is reported.
   • Nothing says how the node manager reacts when Stop(A) fails with ErrNodeBusy or ErrMountsRemain.
   • In the Decide table, the row for a Node never observed is "Wait" (L1089). A node whose kubelet never registers can never be repaved, because there is no Node object to delete.

5. The Decide table is missing the case it was designed for.

   • Level-triggered observation is meant to catch a Node deleted while the daemon was down (L1096-1099).
   • If kubelet or someone else recreates the Node in that window, the daemon sees it as "Present" with a different UID. No row covers that.
   • "Mark applied" for changes that do not cause drift (L1091, L1094) does not say whether those changes are applied to the running node or wait for the next repave.

6. The doc does not say which fields a goal may change.

   • goal.get returns a full AgentConfiguration (L536), which includes runtime.type, plugins, hooks, control, credentials and extensions.
   • It is undefined whether a goal can change the runtime (see Applied.Runtime, L948) or remove an extension. If an extension is removed, its host units and files would stay until a reset (L501).
   • Machina goals carry v1 config, and machina API changes are a non-goal (L160). That v1 config converts with the default runtime, nspawn-container (L347). So it is unclear how a machina-managed host would ever choose host.
   • Bootstrap calls goal.register but not goal.get (L1012), so the goal version recorded at bootstrap is undefined.

7. The new pre target conflicts with how the nftables flush is re-run.

   • In v1 the flush is pulled in directly by the machine unit (nftables-flush.service:21, RequiredBy=systemd-nspawn@.service). The pre-start hook is also a separate one-shot unit pulled in with Requires= (service-override.conf:37-38).
   • v2 puts unbounded-node-pre.target in between (L508), and itself says that starting an active target does not run its members again (L1584-1585). Only the host runtime is said to restart those units explicitly.
   • NodeReboot on nspawn-container is systemctl restart systemd-nspawn@<m> (L1166). Section 10.9 says both that the flush re-runs on each start (L1586-1588) and that starting an active target does not re-run its members (L1584-1585). With the target in between, both cannot hold for nspawn-container. This should become a validation task.
   • Section 8.3 says the pre-start hook is an ExecStartPre (L1042), but v1 runs it as a separate one-shot unit (config-regeneration.service:11). The doc should pick one.

2. Internal contradictions

Where	Contradiction
L311-320 vs L359-360	The example config sets both runtime.nspawnContainer and runtime.host, which validation rejects.
D6 (L173) vs 5.6 (L571-573), 9.1	v2 is said to share only the binary layout. It also shares nftables-flush.service, kubelet.service, containerd.service, localdns.service and /var/lib/machines/kube{1,2}. The doc covers v1 detecting v2 (L661) but not v2 detecting v1. On nspawn-container, a v1 host looks the same as a v2 host.
G4 (L151), the pkg/agent non-goal (L162), §14.5 (L1822) vs L661-666	"v1 unchanged" and "pkg/agent frozen", yet pkg/agent/phases/host/preflight_existing_deployment.go changes, and the change reaches Flex. The new NVIDIA Container Toolkit artifact also changes hack/cmd/agent-artifacts-builder.
5.3 (L382) vs L350, L539	Plugins are said to provide "daemon credentials", but credentials.daemon.type has no plugin value. No config field selects the credentials.cluster method.
L366-368 vs L587	The runtime is said to make the kubelet exec command visible inside the node, yet the Flex mapping also needs a sharedHostPaths entry. Section 9 never covers it.
L1478-1480	Preflight requires cgroup.kill, but a fallback for its absence is also described. The fallback can never run.
L1305 vs L1575	"Delete only what is provably owned" vs deleting iptables chains by KUBE-* and CNI-* prefix.
L1338 vs L1757	Original sysctls are recorded in two places: the host runtime's ownership record and the host preparation undo log (which applies to both runtimes).
L1374-1376 vs the table at L1380-1400	Four cleanup classes are defined, but the table also uses process, mount, network namespace, staged and as declared.
L1386 vs L1529-1534	/var/run/cdi/nvidia.yaml goes through the /var/run symlink, which the safe remover refuses (inference: /var/run links to /run on systemd distros). Use /run/cdi/. The path is also classed volatile, but the Wipe step for persistent paths lists it.
Rule 4 (L764) vs L1505-1508	The rule is "the first Start after Provision is fresh", but host wipes only when the slot or digest changes. Re-provisioning with the same digest does not wipe.
L767 vs L1424	"Restart keeps all state", yet the host restart clears volatile paths.
L1742	Shared hostnet returns []host.Leftover, so a package used by both runtimes depends on the host runtime.
L1632 vs L540	Leftovers are said to go to the status sink, but status.report has no field for them.
L534 vs L410-412	A plugin that fails describe fails startup, yet "a broken hook can never block stopping or cleanup". It is unclear whether v2 reset calls describe.
L1893 vs L261	One says the help text shows "(experimental)", the other "Experimental v2 agent (unstable)".
L52-53	v1's agent-upgrade is listed as visible, but it is hidden (cmd/agent/internal/cmd/agentupgrade.go:70).

3. Underspecified areas

• Hooks:
  • With wildcard subscriptions, it is unclear whether describe must list every concrete hook.<event> method.
  • The example config subscribes aks to node.start.after twice, once directly and once through node.*.after, with different failure policies (L335-338). Is it called twice?
  • When a wildcard entry has no failure policy, does each concrete event use its own default?
  • On a resumed bootstrap, a running A skips its steps and their events (L1017). A failed node.start.after hook with policy Fail is then never retried.
  • During a repave, node.start.after fires after the commit (L1125-1126), so a Fail failure has nothing to fail.
• Plugins:
  • Timeouts are "configurable" (L545), but the plugin schema has no timeout field.
  • Response-size limits are not given.
  • The path checks do not say whether parent directories are checked.
  • The status.report phase values are not listed, and "versions" in goal.register is not defined.
• Credentials: if both the config's cluster.apiServer/caCertBase64 and the bootstrap provider's response supply these values, it is not said which wins.
• State files:
  • Only owned-paths.json has a schemaVersion. The doc says the on-host layout may change without notice, and an upgrade can roll back to last-good (L1191), so compatibility between binary versions is undefined.
  • node-state.json stores Config, which can include credentials.bootstrap.static.token, but its file mode is not stated.
  • What happens when a .sha256 check fails is not stated.
• Reset:
  • When reset runs from the CLI, it is unclear when the running daemon is stopped (L1225).
  • The diagram leaves out /etc/unbounded/agent-v2/, and does not say whether the shared binary directory is removed.
• On-host list: Section 5.6 is presented as the complete footprint that replaces Flex's uninstall.sh (L599). It leaves out:
  • the temporary containerd under /run/unbounded-agent-v2/containerd-maint/;
  • /etc/sysctl.d/99-kubernetes.conf;
  • nftables-clear.nft, the apt drop-ins, the docker daemon.json edit, fstab edits and unit masks;
  • the LocalDNS slice and network unit;
  • the CDI file;
  • extension host files;
  • the first-boot unit.
• Extensions:
  • No rule stops an extension's node or host files from colliding with built-in paths such as /etc/containerd/config.toml.
  • Nothing covers an extension host file that already exists.
  • It is unclear which unit nodeDependencies.requires attaches to on host.
  • sharedHostPaths entries where hostPath and nodePath differ are undefined on host.
• host runtime pre-start hook: the doc does not say which unit runs it (L1237, L1042). Restart also runs it directly (L1424).
• Smaller items:
  • What "from the host" means for v2 agent-upgrade, and whether upgrade hooks fire when it is run from the CLI.
  • Where components.* and artifacts.downloads overlap.
  • How v1 config fields map to v2 when converting.
  • Whether adoptPreserveMaxBytes applies per path or in total.
  • adoptExistingPaths[].mode reuses the name of the file mode field. Something like policy would be clearer.

4. Clarity and terminology

• Decision IDs:
  • Section 3's D1-D6 and section 17's D1-D5 repeat each other.
  • O1-O11 (no O5) and H-D1 to H-D6 are never defined.
  • O6 is recorded as decided (L1901) and also listed as open (L1921).
• Event names: the middle part is called a "state", but the values (install, start, repave) are actions, and the image-import example matches no event (L391). <subject>.<action>.<before|after> would be clearer.
• "A" and "B": the repave diagrams and lifecycle table (L443, L1112-1128) use A and B as roles, but they are also the literal slot names. After the first repave the active slot is b. Use "old" and "new".
• "Hook": the word means four things: plugin lifecycle hooks, NodeRuntime.Hook, v2 runtime-hook and HookPhase. Renaming the runtime side to "boot hook" would separate them.
• "Slot" for host: the glossary says a slot "holds a node" (L133). On host, a slot only holds staged binaries and files; the node's state lives in shared paths.
• G2 "parity": the intended differences from v1 are spread across sections 8.4, 8.5, 10.9, 11, 12 and 13. Collect them in one "Behavior changes from v1" table and reword G2 as "parity except where listed".
• Upstream behavior: some statements are not marked (inference), such as the target member re-run behavior, kubelet certificates living in /var/lib/kubelet, and cgroup.kill needing Linux 5.14.

5. Simplification

Design:

1. Defer lifecycle hooks or cut them down. None of the AKS Flex Node mappings in section 5.7 (L581-599) uses a lifecycle hook. The 16 events, wildcards, default failure policies and hook config have no identified consumer. If they stay, use one hook method with the event in the payload. That also solves the describe question above.
2. Defer Preserve adoption and retainContainerImages. Preserve needs backups with xattrs, SELinux labels and a size cap. retainContainerImages needs a temporary containerd and prototype V2. The defaults, refusing existing content and wiping images, are already the simple behavior.
3. Use one record for the active slot. Either the runtime owns it and reports it through Inspect, or the node manager does. Drop NeverStarted, which repeats Phase == Provisioned.
4. Keep the nftables flush as a direct dependency of the boot unit instead of routing it through the pre target.
5. Let systemd do the stopping. Use systemctl stop unboundednode.slice with a stop timeout. systemd already sends SIGTERM and then SIGKILL across everything in the slice (inference). Keep the cgroup.kill logic as verification and fallback.
6. Unify credentials. Use one exec or plugin mechanism, with a clear table of which credential each type serves.
7. Make control.operations a boolean, since its values are only machina and disabled.
8. Optionally split NodeRuntime (14 methods) into a planning interface (HostRequirements, Preflight, OwnedResources, Layout) and a lifecycle interface.

Document:

• Move section 10 into a companion designs/agent-v2-host-runtime.md, and keep the main doc to the external contract, interfaces and lifecycles.
• Merge sections 3 and 17 into one decision table with a status column.
• Move the host network reset out of section 10, since it applies to both runtimes.

6. Citation accuracy

Of the 93 local citations, 83 are accurate. The other 10 point to the wrong lines or only partly support the claim; none point to the wrong code entirely.

Doc line	Problem
L58 coordinator.go:29-38	These lines are only the Stages interface. The lock is at :72, the run classification at :83-115, and the stage loop at :126-135.
L62 phases.go:17-20	Serial is at :62 and Parallel at :87.
L79 bootstrap.go:220	This line is about saving the applied config. The machinectl list call is at pkg/agent/phases/reset/machine.go:256.
L83 lifecycle.go:67-129	This is the Go code. The systemd wiring is service-override.conf:37-38,48 plus config-regeneration.service.
L365 config.go:615-623	authorization.mode is also reserved (:604-612).
L1241 resolve.go:163-166	Applies only when LocalDNS is enabled. It is the resolvConf config field, not a --resolv-conf flag (kubelet.go:129-130).
L1580 nftables-flush.service:20-21	This is RequiredBy=, not ordering. The ordering is service-override.conf:39.
L1759 configure_docker.go:30-66	Masking docker is in disable_systemd_unit.go:30-33,59-75.
L1391 bootstrap_helpers.go:263	The directory is created at :314-319.
L1238 Containerfile:21-31	Off by one; the block is lines 22-32.

Two more code-versus-doc disagreements could go into section 19:

• api/machina/v1alpha3/machineoperation_types.go:51-53 says "custom operations may be supported", but the enum on the next line is closed. This matters for open question 1.
• internal/provision/assets/unbounded-agent-uninstall.sh reverses host preparation, but UnboundedAgentUninstallScript (internal/provision/script.go:35) is only called from tests. This is relevant to section 11's claim that v1 never reverses host preparation.

[Philip Lombardi (plombardi89)]

AI-assisted response (Claude via OpenCode), reviewed with the PR author. Section numbers refer to the latest commit. The host runtime cleanup now lives in designs/agent-v2-host-runtime.md, called "host runtime doc" below.

Thanks for the thorough review. I verified each claim against the code before acting on it, and all but one checked out exactly (see citations below). Here is what changed, grouped like the review.

1. Gaps: all fixed

1. Re-rendering triggers a wipe. Provision now saves the bundle manifest in the slot. The digest covers secret files' paths and modes but not their contents, so a refreshed token cannot change it. Boot hooks and Recover read the saved manifest and never render again. Hook became BootHook(ctx, phase, slot), with no bundle and no credentials (7.1, 7.2).
2. Two records of the active slot. The runtime is now the only authority on which slot is enabled and running. Applied.ActiveSlot and NeverStarted are gone. Recover reconciles by bundle digest using a table in 7.7. Host runtime doc §8 maps the host runtime's record to the SlotPhase that Inspect returns.
3. Which slot starts after a reboot. Start enables the slot and disables the other; Stop disables it, as v1 does with machinectl. No lifecycle hooks fire for a boot started by systemd (7.1, rules 6 and 7; section 9 table).
4. Failed repaves and stuck states. These now match v1, by decision (D33, D34): retries never stop and use controller-runtime's default backoff; there is no rollback and the old node is not restarted. Each attempt reports Failed with the attempt count. A failed Stop(old) ends the attempt before Start(new) (8.5). A node that never registers reports NodeNotRegistered after 10 minutes, but is not repaved without a Node deletion. That matches v1, and recovery stays manual (8.4). We decided against both a final failure state and repaving without a deletion.
5. Decide gaps. Added rows for a Node that was deleted and recreated, and defined what "record the goal version as applied" means (8.4).
6. Which fields a goal may change. Settings are now split into install-time settings and goal settings. goal.get returns goal settings only, so a goal cannot change the runtime, plugins, hooks or extensions. Machina-provisioned hosts always get nspawn-container, and the doc now says so. Bootstrap records an empty goal version, which the first reconcile settles, as in v1 (5.2, 7.3; D35).
7. The pre target breaks re-running the flush. unbounded-node-pre.target is removed. Prerequisites attach directly to each runtime's boot unit, as v1's flush does. The pre-start boot hook is a separate oneshot unit, as in v1, not ExecStartPre. Validation task V5 covers re-running on restart (8.3, 11).

2. Contradictions: all fixed

• The example config shows one runtime block.
• D6 now lists the unit and machine names shared with v1. v2 also detects a v1 install (6.3).
• G4, the pkg/agent non-goal and the export policy list the exceptions: the detection check and the artifacts builder change (6.3, 15).
• credentials.daemon gains plugin, which selects credentials.cluster. The runtime makes the kubelet exec command visible in the node, so the Flex mapping no longer needs a shared host path.
• The cgroup.kill fallback is dropped. The principle of deleting only what is owned is scoped to files. Original sysctl values live only in the undo log. The owned-resources table has a "Cleared at" column. The CDI spec moved to /run/cdi.
• Rule 4 and Restart are reworded.
• Leftover moved to the shared runtime package, and status.report carries leftovers.
• Reset never fails because of a plugin.
• The help text is consistent, and v1's agent-upgrade is listed as hidden.

3. Underspecified areas: all addressed

• Hooks:
  • one hook method, with the event name in the payload;
  • overlapping subscriptions from one plugin are rejected;
  • wildcard entries use each event's default policy;
  • on a resumed bootstrap, after events fire again with resumed: true;
  • node.start.after fires before the repave commit (5.3).
• Plugins: timeouts per method, a 1 MiB output limit, checks on parent directories, the status.report phases and the goal.register fields (5.5).
• Credentials and artifacts:
  • which credential each setting serves, and which values win;
  • components holds versions, artifacts.downloads holds sources (5.2).
• State files: a schemaVersion on each, only additive changes so a rollback can still read them, StateCorrupt on a bad checksum, and mode 0600 (7.7).
• Reset: the CLI stops the daemon first, and reset removes /etc/unbounded/agent-v2/ and the binary directory (8.8).
• What v2 creates on the host: a complete list in four groups (5.6).
• Extensions: collisions, existing host files, and the rule that hostPath equals nodePath on host (5.4).
• Smaller items:
  • the v2 agent-upgrade path (8.7);
  • the adoption field is now policy, and the size cap is per path;
  • appendix A maps v1 configuration fields to v2.

4. Clarity: all fixed

• Sections 3 and 17 are one decisions table (D1-D37) with a status column, and the undefined O and H IDs are gone. O6 is split into D11 (decided) and D12 (open).
• Events are named <subject>.<action>.<before|after>.
• Repaves say old and new, not A and B.
• The runtime side is called "boot hook" (BootHook, v2 boot-hook).
• The glossary entry for slot is fixed.
• G2 now says "parity except where listed", backed by a new behavior-changes table (14.1).
• The missing (inference) marks are added.

5. Simplification

• Accepted:
  • 3, one record of the active slot;
  • 4, the flush as a direct dependency;
  • 5, systemd stops the slice, with cgroup.kill as a backstop;
  • 7, control.operations.enabled;
  • all three document changes: the companion doc, merged decisions, and the network reset moved to the main doc as section 11.
• Partly accepted: 6, unify credentials. Daemon credentials are now csr, kubeconfig or plugin, and the doc has a table of which credential each setting serves. Kubelet's exec stays separate: kubelet runs that executable itself, through client-go's exec credential protocol, so it cannot use our plugin protocol.
• Not adopted:
  • 1, defer hooks. The maintainers decided to keep them in the first implementation (D37). Their semantics are fixed instead; see section 3 above.
  • 2, defer Preserve and retainContainerImages. Kept in the first implementation by decision (D36). Validation task V2 still gates image retention.
  • 8, split NodeRuntime. In Go, the code that uses an interface declares the small one it needs: hostprep can take an interface with just HostRequirements, preflight one with just Preflight. Splitting the implementer's side adds names without adding safety, and the conformance suite tests a runtime as a whole.

6. Citations

• Nine fixed as described.
• One correction to the review: bootstrap.go:220 is the machinectl list lookup (reset.RegisteredMachine), but it decides whether to save the applied config, not whether to build the rootfs. The doc now cites :161 for the rootfs decision, :220 for the applied-config decision, and pkg/agent/phases/reset/machine.go:256 for machinectl list.
• Section 19 gains both suggested disagreements, plus one found while fixing citations: a v1 repave reuses a partially unpacked rootfs after a crash (pkg/agent/phases/rootfs/oci/task.go:61-64). v2 avoids it with a completion marker.

Commits

Commit	What
2e2d905	Restructure only: companion doc, merged decisions, network reset moved
91f58bc	Citations and factual fixes
b11be46	State model and lifecycles (gaps 1-6)
1f96397	systemd wiring (gap 7)
02464ef	Hook semantics
602acc0	Contradictions and underspecified areas
dd42a6a	Clarity and behavior-changes table
43959cd	Agent binary slots vs node slots wording

Still open for reviewers, in section 18: MachineOperation kind scoping (D12), separate v2 state names (D6), the default runtime (D29), copying rendering templates (D30), the boot hook binary (D31), and observing the Node on every reconcile (D32).

[Philip Lombardi (plombardi89)]

AI-assisted review (Claude via OpenCode). Findings were checked against the code where cited, but please verify each one before acting on it.

This review covers everything since my first review. Line references like 03-node-runtime.md:247 are to designs/agent-v2/ at 4160c05ed. Code citations are at the design's new baseline, e132dfada.

Pushed to the branch:

• 56729bd68 moves the baseline from dca6c964d to e132dfada, so every citation points at one commit. It updates the citations that moved and adds HostOSReboot where v1's handler list, the reconcile diagram and the startup gate were missing it. It deliberately stops before #1034, whose kind renames are still in progress.

• 4160c05ed fixes wording and consistency, with no design change:

  • the Teardown flowchart;
  • which unit host enables;
  • the 0600 rule, and schemaVersion on the active-slot record;
  • nodeDependencies.requires wiring;
  • the Pending state field renamed to Repave;
  • the NodeChanged trigger;
  • the six LocalDNS template variables;
  • glossary terms;
  • stale references.

  Each item is listed in its commit message.

The earlier review: nearly every finding from it is fixed in the docs, not just in the reply. Two notes on the reply:

• It cites decision IDs (D11-D37) that no longer exist, because the decisions table was removed.
• It says the first reconcile settles an empty goal version. The docs instead record the goal version only when the daemon handles a Node deletion (04-control-and-state.md §1). The docs are consistent with each other; only the reply is wrong.

1. Migration and cutover (doc 10)

1. At the first official v2 release (N+1), a migration failure can loop.
   • §6 says any failure before Committed rolls back to v1 automatically (10:203).
   • The rollback's step 3 is "start v1's daemon" (10:246). At N+1 the daemon unit runs <current> daemon, which is N+1's v2 daemon. That daemon would start the migration again.
   • §2 describes the intended path instead (10:53-57): N+1 exits, and the recovery unit restores N, whose daemon undoes the migration.
   • Suggestion: state that N+1 never performs the rollback itself. It should exit with a normal failure code, not the deferred exit code the unit treats as success (unbounded-agent-daemon.service:17-22).
2. After the rollback window closes, a daemon crash loop can leave the host without an agent.
   • v1's recovery unit restores last-good after any repeated daemon failure, not only during an upgrade (unbounded-agent-daemon.service:8-10, unbounded-agent-daemon-recovery.sh:11,26).
   • After a cutover, last-good is N. Once the first repave deletes the backup, N cannot undo anything, and it refuses a v2 host (08 §4).
   • Suggestion: when the backup is deleted, point last-good at the running v2 binary.
3. The health check before commit is circular at N+1. 10:198-202 reuses v1's "stable for 3 seconds within 30" check (hostupgrade.go:23-26). At N+1 the daemon running the migration is the daemon being checked.
4. The early migration from the cluster is underspecified (10:103-134):
   • who publishes the AgentUpgrade result;
   • how the v2 daemon learns the migration committed (10:117) and which operation it belongs to, given v1 and v2 keep separate upgrade signal files;
   • how the v2 daemon's startup gets the install lock while v2 migrate holds it (10:168) (inference).
5. "Upgrade to N first" cannot reach the operation as written (10:63-66, 11:193). v1's recovery script records a fixed message (unbounded-agent-daemon-recovery.sh:19-20). N+1 would have to record its own reason before exiting.
6. The check "no other v1 MachineOperation in progress" (10:144) can't be computed for selector operations. They share one status across agents, and agent kinds other than HostOSReboot record no per-Machine targets (#1040). Suggest counting only machineRef operations for this Machine.

2. Node runtime

7. kubelet and containerd restarts would flush the firewall and look like a reboot.
   • Each host node unit Requires= unbounded-node-prestart.service (03:247).
   • The doc says these oneshots run again on every start of a unit that requires them (03:275-280).
   • kubelet and containerd have Restart=always (pkg/agent/phases/nodestart/assets/kubelet.service:8, containerd.service:12).
   • So each kubelet or containerd crash could re-run flush ruleset on a live node, and write a new boot ID, which gives the Node a false Rebooted event (inference about systemd's handling of Requires= on automatic restarts).
   • v1 never does this, and it isn't in the behavior-changes table (08 §2).
   • Suggestion: have only unbounded-node.target require prestart. Order the node units After= it without Requires=, and extend V5 and V6 to cover a kubelet restart.
8. A refreshed bootstrap token may never reach the slot.
   • Secret contents are left out of the digest (03:199-201).
   • "Provision, same digest is a no-op" (03:225).
   • A repave retry that fetches a new token would keep the expired one in the slot.
   • Suggestion: Provision always rewrites secret files.
9. The Recover table can match two rows (04:164-167, inference).
   • On nspawn-container, Layout does not depend on the slot.
   • The image reference is in neither NodeSpec nor NodeBundle.
   • So an image-only repave can give the same digest for old and new.
   • Suggestion: compare Repave.Slot with the enabled slot, or put the image reference in the digest.

3. Interaction with the machina API (cross-checked against #1045's findings and issues #1038-#1044)

10. The rule for agent-owned kinds misses HostOSReboot itself.
    • 04:268-272 treats the known kinds plus future Node*/Agent* kinds as the agent's. HostOSReboot is a Host* kind the agent owns, so a future agent-owned Host* kind sent to an older agent would stay pending forever, the case this rule exists for.
    • Failing an unknown selector operation would also fail it for every matching agent: status is shared, and agents skip it once it is terminal (pkg/agent/daemon/machineoperation.go:105-107, #1040).
    • Suggestion: an explicit list of agent kinds, and UnsupportedOperation only for machineRef operations.
11. Which Machine fields the machina status reporter writes is unspecified (02:199-213, 04 §1).
    • machina owns Machine.status.phase and branches on it (cmd/machina/machina/controller/machine_controller.go:276-310). v2 must not write its Pending/Ready/Failed phases there.
    • NodeNotRegistered, RepaveFailed, StateMismatch and StateCorrupt have no field on the Machine without an API change, which is a non-goal.
    • AgentBootstrapped reasons include NSpawnFailed, which the host runtime needs a mapping for.
12. Gaps v2 inherits but doesn't mention:
    • #1039, provisioning ignores configurationRef: after bootstrap or migration, any Machine whose referenced version differs from what the node was built from will show Pending, since v2 now evaluates drift on every reconcile. That differs from v1, which only checks on a Node deletion. HostReplace and metalman reinstalls also produce v1 configurations, so a host runtime host comes back as nspawn-container (01:210-215 mentions only machina).
    • MachineConfigurationVersion's agent block on host: the block is required, and its image overrides the nspawn image. How it maps onto a host install is unspecified, and the e2e template has no agent block, so tests won't catch it.
    • #1043, TTL: the agent never deletes operations. The fleet migration's one-operation-per-Machine pattern (10:132-134) piles up on clusters without machine-ops-controller or metalman. v2's copy of the reconciler is where #1043's fix would go.
    • #1044, HostOSReboot serialization: v2's handler could wait for older out-of-band operations on the same Machine, or defer to #1044 explicitly (05 §4).
13. The new agentMode parameter isn't listed among changes outside internal/agentv2 (08 §4). The parameters API comment and the CRD reference list the accepted keys exhaustively, so both need updating.

4. Testing (doc 11)

14. The cutover variants need builds of N and N+1 before those releases exist (11:190-193). Say how CI produces them, for example a build setting that selects N or N+1 behavior.
15. The test for a Node deleted while the daemon is stopped (11:206-207) needs kubelet to register again. That only happens on a kubelet restart (inference), so the test should say how it triggers one.
16. The host snapshot (11:127-133) needs an allowlist for sysctl -a keys that change on their own, such as entropy, inode and dentry counters.
17. At N+1, v1 commands are removed, so AGENT_PROFILE=v1 jobs stop working. §6 should say what replaces them as required jobs.

5. Optional

• The reasons for keeping lifecycle hooks and Preserve/retainContainerImages in the first implementation went away with the decisions table. One sentence each, where those features are described, would save reviewers asking again.