Issue 289: a verb answered during a handover could not run its own build

2026-10-07 02:14:08 +02:00
parent 35d81e962e
commit fd0bb826b3
1 changed files with 90 additions and 0 deletions
@@ -0,0 +1,90 @@
+---
+status: located
+opened: 2026-10-07
+located-in: [mesh-controller cmd/mesh-controller, mesh-controller internal/link, mesh-host internal/witness, mesh-tools node-tools/internal/bus]
+fixed-by: mesh-controller PR #106, mesh-host PR #46, mesh-tools PR #19
+amended-design:
+---
+
+# 289. A verb answered during a handover could not run its own build
+
+## Symptom
+
+On 2026-10-07, a new controller build was rolling out on the control node. A seat verb reached the
+controller still serving, the old build, and failed:
+
+> `mesh-controller.rotate failed: could not run secret rotate …: fork/exec
+> <daemon root>/.witness/mesh-controller/previous/mesh-controller: permission denied`
+
+Seconds later the new controller served the same call without trouble. Nothing said the first call
+could be asked again. It read as a refusal, and the words pointed at file permissions, not at a
+handover.
+
+## Diagnosis
+
+Two halves, each harmless alone.
+
+**The controller runs every verb as a command of its own binary** (ADR 0035, ADR 0154). It found that
+binary with the path the running executable has *now*. That path is correct when the process starts and
+wrong as soon as the file moves.
+
+**The node-engine's witness moves the running build while it still runs** (to-be 45 §8). Placing a new
+build does four things, in this order:
+
+1. renames the running build's directory into `.witness/<name>/previous/`;
+2. unpacks the new build in its place;
+3. restarts the unit, which stops the old process and then starts the new one;
+4. once the new build is proved, deletes `previous/`.
+
+Between step 1 and the end of step 3, the old controller still holds the lease and still answers verbs.
+Its build now sat under `.witness/`, which was created `0700` and owned by root. The controller runs as
+its own user, so it could not enter that directory, and every verb it started failed with "permission
+denied". Had the call come after step 4 with the old process still draining, it would have failed with
+"no such file". The kernel still had the image, but the controller asked for it by path.
+
+Ruled out: the new build. The same verb succeeded on it seconds later, and the two builds ran the same
+code path.
+
+## Fix
+
+The order of a witnessed update stays as it is: move aside, unpack, restart (old stops, then new
+starts), prove, retire. What changes is that every step is safe for a process that is still running:
+
+- **The controller runs its verbs from its own running image** (mesh-controller). On Linux it runs
+  `/proc/self/exe`, which stays valid while the process lives, wherever the file is renamed or
+  however it is deleted. A verb it still cannot start, or one that arrives once the controller has
+  begun to stop, is refused as a **handover**. The refusal says that nothing was done and asks the
+  caller to try again, and the answer carries a mark (`retry: handing-over`). It is never shown as a
+  permission error.
+- **A replaced build stays reachable, and is deleted only once nothing runs from it** (node-engine).
+  The directories the witness keeps beside a process are `0711`: the process's own user can enter
+  them, and only root can list them. A build that is no longer needed is retired: it is renamed
+  under `.witness/<name>/retired/` and deleted only when no process on the machine runs from it. The
+  kernel says which processes those are: each process's executable is read from `/proc`. That covers
+  the PID the old build ran as and every command it started. A build on trial that is replaced while
+  it runs is retired in the same way. The witness sweeps retired builds on every look.
+- **The caller asks once more** (node tools). A seat call refused with the handover mark is asked one
+  more time after a pause long enough for the restart. A second handover refusal is reported as one,
+  and no third attempt is made.
+
+## How it is checked
+
+- mesh-controller `TestAVerbRunsWhileItsBuildIsMovedOrDeleted` starts a copy of the controller's test
+  binary, moves it into a closed directory (or deletes it), and only then runs a verb. On the commit
+  before the fix it fails with the same "permission denied" seen live, and with "no such file" for the
+  deleted case. `TestAVerbThatCannotRunIsRefusedAsAHandover` and `TestAHandoverRefusalIsMarkedRetryable`
+  cover the refusal.
+- mesh-host `TestABuildIsKeptReachableAndUndeletedWhileAProcessRunsFromIt` runs a real process from a
+  build, places the next one, proves it, and checks that the old build is still there until the process
+  stops, then gone after the next sweep. `TestABuildOnTrialReplacedIsRetiredNotDeletedUnderItsProcess`
+  and `TestAProcessRunsFromABuildDeletedUnderIt` cover the other two ways a build leaves.
+- mesh-tools `TestAHandoverRefusalIsAskedOnceMore` checks three cases on a bus of its own: a marked
+  refusal is asked once more, an unmarked one is final, and two in a row are reported.
+
+## Left open
+
+- To-be 45 §8 does not say when a kept build may be deleted. It should say: "only once no process runs
+  from it". That is a design amendment for when this issue resolves.
+- A process with no witness has its directory deleted in place when a new build arrives. None of those
+  run themselves today. A process that someday does would meet the same fault, and would need the same
+  retirement.