Reconciler: the only actuator for every operator command, and a ladder that paces faults not operators

[MorningLightMountain713]

Overview

Two things, both about who decides a container's run state.

The crash-recovery ladder paces faults, not every stop. It used to pace all of them, so an operator restarting their own app walked the same escalating waits as a container that could not stay up. A customer restarting their Palworld server six times in twenty-one minutes reached the fifteen-minute rung, which from the outside is indistinguishable from the app refusing to come back. That is what the support ticket was about.

Every operator command is desired state the reconciler converges to. appStart, appStop, appRestart and appKill record the operator's intent and let the single actuator converge on it; no route in appController drives a container. Seventeen appDockerStart/Stop/Restart/Kill calls remain outside the reconciler — install, uninstall, redeploy and backup/restore, mount recovery, and the two-hourly sweep that stops containers FluxOS does not own — and those paths are not what this changes.

First of three in the stack: development ← this ← #1781 ← #1782. #1779 below has merged (63327e97f), so this targets development directly. #1781 and #1782 are based on this branch and merge in stack order.

Why the exit code cannot decide this

The exit code was already persisted — recordExit writes lastExitCode beside lastDiedAt — and nothing read it. It is now an input, but only in the direction it is sound.

For a container that went down on its own: non-zero, or OOMKilled, proves a fault, and zero proves nothing. An image whose entrypoint is a wrapper script ending in exit 0 reports a clean stop for a segfault. Palworld's official image does exactly this:

"${STARTCOMMAND[@]}"     # the game — segfaults here, $? = 139
LogAction "Ending Server"
exit 0                   # unconditional; the game's status is discarded

Its runtime state on a live node records lastExitCode: 0 for a death confirmed as a SIGSEGV. No init we wrap around the container recovers this: the loss happens inside the image's own script, a level below anything we can inject, so --init/tini would faithfully propagate the zero that was already laundered.

Pacing on the code alone would therefore leave that class of container restarting without limit. That is what the ceiling is for, and why it consults nothing but the rate.

The qualifier at the top of this section is load-bearing, and the case it excludes is not yet handled. FluxOS stops containers deliberately — an election moving a g: primary, a backup taking the app down — and those go out as SIGTERM, so an image that does not trap it exits 143. This lineage reads that as a fault and books a rung, which paces a healthy app for the node's own tidy shutdown. It is an improvement on what it replaces, which paced every restart whatever the cause, but it is not the whole rule: the exit code is an unreliable narrator in both directions, and only one of them is addressed here. Teaching the ladder to recognise the node's own drains needs desired state to decide it rather than the exit code, and that is a follow-up PR rather than a widening of this one.

The ceiling, and what it reaches

Five automatic restarts inside five minutes is itself evidence of a fault, whatever Docker reported. It disposes into the same ladder rather than into a failed state a human has to clear — nobody is watching a customer's node at 3am.

It is judged before the restart is recorded, against five entries spanning five gaps, so its reach is window / count. Measured against the real module:

every  59s   paced
every  60s   paced
every  61s   never paced, however long it continues

A container exiting in milliseconds trips it in about a second. Anything slower than one restart a minute is deliberately left alone.

The ladder is measured against running, not against waiting

restartWaitMs clears the ladder once a component has genuinely run for STABLE_RUN_MS, and measures that from its last rung. A component holding rungs therefore earns one for every restart — otherwise the gap being measured is the wait the component just served, and any rung longer than ten minutes clears the ladder every time it fires.

The exit code decides what puts a component on the ladder; it does not decide whether it stays. restartWaitMs takes no crashed argument for that reason — whether to wait depends only on whether rungs stand.

Measured against the real module, driving the reconciler's own loop order for two hours with a container dying two seconds after every start:

container	starts in 2h	rungs reached
reports exit 0 for a segfault	12	30s, 5m, 15m, 30m cap
exits non-zero	7	30s, 5m, 15m, 30m cap
healthy, runs for an hour	2	never paced
operator presses restart	—	never paced, ladder cleared

An operator restart is recognised by the path it takes, not by its exit code: appStart and appRestart clear the operator lock, and setOperatorStopped(id, false) drops the rungs with it. Recognising it by exit code instead hands the same exemption to a laundered segfault, which also exits 0.

Operator commands are desired state

appStart probed docker for "is this container running" and used the answer as a proxy for "should this node be running it". Those diverge both ways: a masterSlave primary whose container is stopped was refused a start though the election names it the writer, and a standby whose container happened to be up was started. It classified g: with containerData.includes('g:'), which mountParser's own comment calls wrong — '/data|g:/db' matches while the flag sits in an invalid non-primary position.

The election owns that decision and the reconciler consults it on every pass, so the handler clears the lock and lets it decide. A component the election is holding is reported as pending, naming the election, rather than as skipped.

appRestart raises a durable restart generation; the reconciler bounces a running container once the generation passes the one it last actuated, and a start of a stopped container satisfies the same request. appKill records a durable force mode which the reconciler honours with appDockerKill where it stops the container.

Every one of the four reports what happened rather than what was asked for: Application X stopped, or will be stopped: docker is not reachable.

A command that addresses nothing is refused rather than reported done. The components are resolved before anything is written, and an app that is installed but whose parts this node cannot work out — a spec that will not decrypt, falling back to a container listing that is empty or that failed outright — yields none. Acting on that empty list wrote no intent and settled vacuously against nothing, so all four answered Application X stopped having touched nothing and recorded nothing for a later pass to pick up. They now say the app was not changed and why, worded for the operator: HomeUI shows the message verbatim.

Monitoring follows the container

startAppMonitoring resets statsStore, and appStart called it on every request — so asking an app to start discarded the series the charts read. appStop stopped the sampler before knowing whether the stop had happened, so an unreachable docker left a running container unmonitored.

The reconciler owns both ends. It stops the sampler on each of the four paths where it stops a container, and ensureAppMonitoring puts one back for a running container that has none without resetting a live one.

The kill endpoint

appKill was implemented, exported and covered by unit tests calling it directly, and no route registered it — on this lineage or on v9. Every route is a string literal, there is no dispatch table, and executeAppGlobalCommand builds a URL against the same route table, so a peer could not reach it either. routeWiring.test.js reads the real route table, so its absence now fails a test.

appKill and appRemove both ask for appownerorfluxteam rather than appownerabove, which takes the node operator out of both. Hosting an app is not owning it: an operator who can remove one can script that against every install and keep a customer's app off their node indefinitely, which the customer experiences as an app that will not stay deployed and cannot diagnose.

There is no operator need on the other side of that. An app cannot exceed what was bought — Memory, MemorySwap, NanoCpus and StorageOpt are set on the container from the spec, so an app inside its allocation is spending cycles the operator sold, and an app outside it is a containment defect to be fixed in the limits rather than papered over on one node. What an operator does keep is the control that is honest about its own cost: stop FluxOS, or drop the node, both of which give up the payment along with the obligation.

That reasoning does not stop at these two, and the rest is already decided. appStart, appStop and appRestart still ask for appownerabove, and a follow-up PR takes the remaining app control away from the node operator entirely, with the frontend controls narrowed alongside it — the same way this one lands with #150. It is separate only so that each gate ships with the control that exposes it.

appownerorfluxteam admits exactly {owner, fluxTeam, fluxSupport}, which is the same set the vetted-app branch below it admitted. That branch existed to keep the operator away from vetted apps, so with the operator out of the gate above it can no longer refuse anyone who reaches it. It goes, and with it two database reads and a vetted lookup on every uninstall.

The same reasoning closes the other end. installapplocally and testappinstall asked for adminandfluxteam on their by-name branch, which resolves an app from the marketplace, the global registry or a permanent message — so an operator reaching it chose which customer's app ran on hardware they control, and for a g:/r: app the new instance syncs that customer's data down to it. Both move to fluxteam. Leaving install open while closing remove would have left the operator able to place an app on their node and unable to take it off.

The temporary-message branch above them is deliberately untouched and stays open to any logged-in user: that is how an app is tested before it is registered, it is addressed by hash rather than by name, and it expires on its own via temporaryAppAllowance.

This is user-visible on a reachable route, so the frontend control that offered it is narrowed to fluxteam in RunOnFlux/fluxos-frontend#150. The two land together. local.vue renders Uninstall to admin, so this merging alone leaves a node operator pressing a button that now refuses them. The same PR fixes the panel refresh behind finding 5: AppControl.vue read response.data.status once and refetched once, so an accepted command showed a green toast beside a status that had not moved yet. It now refreshes on a spreading schedule (0/2/5/10/20s).

Config fallbacks

RESTART_BURST_WINDOW_MS falls back to the value config ships. Every config.fluxapps.x ?? literal in the service layer is a second copy of a shipped value — forty keys, none of them a default the module owns — and tests/unit/configFallbacks.test.js compares each against ZelBack/config/default.js. An expression it cannot evaluate fails rather than being skipped.

config.get() removes the second copy entirely and throws by name on a missing key. That is 40 call sites here and 108 on the v9 lineage, so it is its own change.

The boot-lock harness commits

Three commits touch only test-infra/runner/framework/boot-lock.js and its call site, and are named here because they are otherwise unexplained in a reconciler PR: the boot lock says how long it queued and how long it held, a boot's duration is unreadable without the shape of the fleet, and the boot lock admits a bounded number of boots, not one. They are named by subject rather than by hash: this branch is rebased whenever the one below it moves, and every hash in it changes when that happens.

Fleet boot is serialised host-wide by a semaphore, and nothing recorded what that cost. The first two make acquire report how long a build queued and how many were ahead of it, release report how long it held, and add the shape of the fleet being built — without which a hold duration mixes one-node and ten-node builds indistinguishably.

The third acts on what the first two measured: the gate was 89% boot-lock-held wall clock, 4,439s of 4,978s across 130 boots, so its duration was very nearly the serial sum of the boots it serialised. The lock now admits E2E_BOOT_LOCK_WIDTH boots, default 2. Arrival order is untouched — the queue still drains in the order it formed, which keeps starvation structural rather than statistical.

Measured across two gates of the same 82 suites at MAXN=6, the same 130 boots and a near-identical fleet-size distribution:

	width 1	width 2
gate wall clock	4,978s	4,192s
queue wait, p50	87.8s	24.1s
queue wait, total	11,697s	3,747s
lock held, p50	32.6s	40.2s
lock held, total	4,439s	5,746s

Boots contend and each costs more, but they overlap, and the overlap wins by 786 seconds. E2E_BOOT_LOCK_WIDTH=1 restores the old behaviour without reverting the commit.

All three emit TAP comments (# boot-lock ...), so none can alter a result: run-all.sh tallies with grep -c "^ok ", anchored to line start.

An address change no longer stops at the first composed app

adjustExternalIP restarted the apps that survive an address change by calling appController.appDockerRestart(app.name). That resolves one container by name and a composed app has none under its bare app name — its containers are <component>_<app> — so the first one threw a TypeError that nothing caught, taking with it every app behind it in the loop, the fluxipchanged broadcast, the confirmation transaction and the geolocation update. One log line was the trace.

The surviving apps are now handed to appReconciler.requestRestartOf as durable per-component restart requests, through a seam (setOnAddressChanged, wired in serviceManager) rather than a require — appUninstaller requires fluxNetworkHelper and appReconciler requires appUninstaller, so reaching upward from the network layer closes a cycle, and twenty-odd app-layer modules import it. componentIdsOf is shared with enqueueAll so the rule for what components an app has, including the docker fallback for specs that will not decrypt, has one home.

An address change no longer uninstalls the node's own apps

Before restarting an app, adjustExternalIP asks whether it is already running at the address being moved to, and force-removes it if so — on the sound principle that one instance per IP is what the host port mapping allows, so an instance already there means the ports are gone. It treated any answer as another node's. The node's own registration is one of those answers: it stores its own running-app row locally, at the address benchmark reports, so the row found at that address is usually itself. The app was removed — forced, and broadcast to the network — for being exactly where it belonged.

The address is still compared at IP granularity — that is the question being asked, and a UPnP sibling on another port holds the ports just as surely. Own-ness is the full socket address, which is what separates the two, using the same socketAddressesMatch test the collision check in that file already uses.

A stop is honoured while a data volume is unavailable

The reconciler defers every actuation for a component whose data volume will not mount — a start there writes to the host filesystem instead of the volume. A stop is the exception, because it takes nothing from the app dir, and leaving a container running over a missing volume is the state the mount-safety hold exists to end.

That exception covers the operator's stop as well as the controller's. The operator's outranks the controller's everywhere else in a pass, and an unmountable volume is the state support reaches for a stop in, so a stop that waits for the mount is a stop that never arrives when it is wanted. The force flag is carried through it, so an appkill there is a kill rather than a graceful stop.

An operator command addresses what the app is made of

The components a command acts on come from appReconciler.componentIdsOf, which decrypts the spec to enumerate them and falls back to the running containers when it cannot.

Reading compose off a stored spec answers neither case reliably. An enterprise app keeps its component names inside an encrypted blob, so its stored compose is empty — a whole-app command derived from it addresses nothing, and every one of zero components reaches its desired state, so it reports success. A component name taken verbatim addresses nothing either when it is not one of the app's, and still writes a durable operator lock under it that nothing clears.

componentIdsOf returns one spelling whichever source it used. Docker holds the namespaced name (flux<component>_<app>) where a decrypted compose gives the bare one, so the list carried either depending on whether the spec read. Every consumer until now canonicalised on ingest and so could not tell them apart; the membership check above compares the list against a name an operator typed, and refused every component-level command against an app whose spec would not decrypt. Stripped at the source, so one spelling reaches all three consumers.

containersReachedStopped distinguishes a container that is absent from one that is stopped. dockerActual reports exists separately from running, and reading the pair as one answered "stopped" for a container that was never there — the answer appStart already gave correctly.

The restart generation is raised by the database

requestRestart issues an atomic increment rather than reading the current generation and writing one more than it. getState answers null both for "no record yet" and "the read failed", and treating the second as zero writes a generation below the one already actuated — which the reconciler reads as nothing pending, so the restart is reported and never happens. An increment has no such answer to misread, creates the field at 1 when there is no record, and counts two concurrent requests as two rather than losing one.

The upsert retry that protects a first write from a concurrent one is shared with setFields rather than copied, since without it the loser of that race is dropped silently.

Changes

file	change
appController.js	all four operator handlers record intent and probe the outcome; app:operatorIntent published from inside the reconciler's slot; appDockerRestart deleted
appReconciler.js	honours the restart generation and the force mode; owns monitoring on its stop and steady-state paths; requestRestartOf, and componentIdsOf shared with enqueueAll
fluxNetworkHelper.js	setOnAddressChanged; the surviving apps are handed over once; the duplicate check excludes the node's own registration
serviceManager.js	wires that seam to appReconciler.requestRestartOf
appUninstaller.js	appownerorfluxteam on the uninstall route; the vetted branch it made redundant removed
appsRuntimeState.js	restartGeneration / operatorStopForce; a component holding rungs earns one per restart; twin-merge carries all three
appInspector.js	ensureAppMonitoring; a cleared interval no longer looks live
routes.js	/apps/appkill/:appname?
verificationHelper*.js	appownerorfluxteam
ZelBack/config/default.js	restartBurstCount: 5, restartBurstWindowMs: 300000

globalState.stoppingContainers stays. Its remaining consumers are the uninstaller, redeploy, backup/restore and mount recovery, none of which this touches. The network-change restart is no longer one of them.

Testing

Unit suite 5,635 passing, 0 failing (18 pending) on the current head. Both fixtures up — without mongo the run skips 682 tests and still exits 0, so a bare pass is not a result.

Every guarantee added here was mutated to prove the test would notice: the force mode ignored, every stop forced, the restart request never actuated, the bounce never recorded, a start not satisfying a pending request, a pending outcome reported as success, the handler driving docker again, the intent event published outside the slot, the route removed, the kill privilege widened, the uninstall privilege widened back to the one that admits the node operator, the component identifier left namespaced by one of its two sources, an empty component list reporting success, the ladder cleared by its own wait, and a fallback diverging from config.

Twenty-nine mutations run, twenty-eight caught. The one that was not is worth stating: it reverted a line measuring stability from the last restart in either array rather than from the last rung. Nothing failed, because a component holding rungs earns one for every restart — so the two are always the same value and the line could not change an outcome. It was removed rather than shipped.

One test was removed for being unable to fail, and one rewritten: it asserted that an operator restart is not paced but expressed that through the exit code, which is the exemption a laundered segfault also receives.

On a fleet

The unit tests stub docker, so they prove a handler writes the right intent and nothing further. What a customer sees is a container's run state and a response string, and until now nothing drove either. Six tests were added to four existing suites, and one new suite covers the address-change path:

suite	test
32-…-restart-clears-lock	apprestart bounces a container that is already running, and bounces it once. The suite's existing test restarts one that was down, which is a start — either implementation satisfies it, and the restart generation only decides anything when the container is up.
32-…-operator-stop-durable	appkill stops it, asserted on the forced flag — "the container stopped" passes against a graceful stop too. And the node operator is refused a kill while still allowed a stop, driven from the node's own identity, both halves.
52-masterslave-…	appstart on an instance the election is not electing reports the election rather than claiming a start. Runs first in that suite, before anything there stops an instance — after that, a non-running holder could be one an operator stopped, which is a different answer reaching the same assertion.
54-…-burst-ceiling	the ladder is not cleared by the wait it just served, reaching the rung that outlasts a stable run.
35-…-masterslave-election	a backup's stop costs the elected primary no rung: the hold is released and it converges back to running inside a cycle. The same assertion fails on the branch below, where every restart earns a rung and the component is on the five-minute one by then — so it discriminates against the behaviour this PR changes, proven by running it on both.
56-address-change-restarts-apps	new. Three nodes, one COMPOSED app. Both components restart on their own identifiers after the node's address moves, and a peer accepts the fluxipchanged broadcast — which is what distinguishes a handler that ran to the end from one that stopped at the first app.

Suite 54 previously ran against production's ladder, so the only rung reachable inside a sane timeout was the first — everything past it, including the rung this PR's defect lived at, needed a twenty-one minute wait and so was never driven. Its ladder and stable-run window are now compressed together, preserving the relationship that matters (a rung outlasting the window that clears the ladder: 30s against 20s here, 15m against 10m in production) and asserting that relationship holds, because compressing one without the other leaves the suite green having stopped testing what it exists for.

The backoff actuation now carries the rung. waitMs is what remains of a rung and the worker re-enqueues during a wait, so one rung reports several times with a falling number and two backoffs cannot be compared — which is precisely the question a ladder that resets itself turns on. It reaches the log line too, so support can read how far a component has escalated rather than only how long is left.

Suite 56 changes a node's address for real rather than simulating one: the new address goes onto its interface and the old one comes off, and benchmark's public-IP probe reports the move while the deterministic list still says where it was. Its peers then fail to reach it at the old address because it genuinely is not there, which is what makes it ask benchmark whether it moved — a node that is still reachable never asks. The list being untouched is what lets those same peers still recognise it as the sender of the broadcast that follows.

Simulating either half breaks the other, which is why it is done properly: blocking the node's packets makes a peer's answer arrive after the asking node has given up, so it reads as "I could not ask" rather than "you are unreachable" and never reaches benchmark; hiding the node from peers' lists makes them answer promptly and then reject its broadcast as coming from a stranger.

Two harness assumptions had to go with it, both of them things an address change necessarily violates. The runner reached a node at a fixed address, so a node that moved went blind to its own client — nodeClient.followTo re-points it and reconnects. And the daemon stub identified a node by the address its requests arrive from, so a node that moved lost its identity, read its own address as the not-found fallback, and skipped its availability check on every cycle; it is now recognised at its reported address too.

Run against images rebuilt from the tree under test and verified from inside the image, not from build output. Suite 56 on the current head: 2 tests, 0 failures, twice consecutively at 3m35s and 2m32s. Suite 35: 4 tests, 0 failures.

[Cabecinha84]

PR #1780 — deep review

fix/restart-pacing-crash-only → fix/syncthing-first-run-gate · 350 add / 33 del · 10 files · 5 commits · no prior
reviews or comments (the PR is completely un-reviewed, so nothing here is re-litigating past feedback).

What I actually verified

• Ran the two affected unit files at head: 131 passing, 0 failing — matches the claimed 119 → 131.
• Ran the full unit suite at head: 5105 passing, 5 pending, 23 failing — all 23 are in dockerService tests, which need
  a live Docker daemon and are untouched by this PR. Environmental, not caused by the change.
• Traced every call site of restartWaitMs / recordRestart in ZelBack/ — there are exactly two, both in
  appReconciler.js, both updated. Both new params default to true, so nothing else changes.
• Simulated the real reconcile loop against the actual (non-stubbed) appsRuntimeState module to confirm the emergent
  behaviour, rather than trusting the unit tests' arithmetic.
• Cherry-picked the core commit onto origin/development to test the stacking claim.

The core logic is correct

• crashed is computed only after !actual.reachable (appReconciler.js:735) and actual.indeterminate (:744) have already
  returned, so it is never derived from a degraded inspect — the two error paths of dockerActual that return exitCode:
  null with no oomKilled field are unreachable at that point.
• exitCode is gated on everRan (Status !== 'created'), so a never-run container is correctly not read as a fault.
  OOMKilled comes from the same inspect — no second Docker call, no TOCTOU.
• The sliding window is right. autoRestartWindow is capped at RESTART_BURST_COUNT and burstExceeded reads recent[0] —
  the oldest of the last N — so stale entries can never false-trip. Verified: 4 restarts spread over hours plus one fast
  one does not trip.
• setOperatorStopped(id, false) clears both arrays, and appController calls it before the Docker op on both appStart
  and appRestart, so a deliberate restart genuinely starts from clean.
• The early return in restartWaitMs skips the stable-run ladder reset, which I initially flagged — but it self-heals:
  the ladder is cleared on the first crashed=true call. Verified: 3 crash rungs + weeks of clean restarts + one genuine
  crash → wait 0.
• prepareCollection's twin-merge carries the new field with the correct cap.

Test quality is genuinely high — the new unit tests are specific and the e2e suite 54 asserts the mechanism rather
than the constant.

Findings

1. The burst ceiling has a 12.0-second cliff — and the PR body describes it as "a minute or two" ⟵ the one that
   matters

burstExceeded is evaluated before the append, against 5 entries spanning 5 × interval. So the trip condition is 5d ≤
60000 → d ≤ 12.0s. I measured it directly:

spacing=12.000s -> paced after 6 restarts
spacing=12.001s -> NEVER paced (60 restarts, 720s of hammering)
spacing=13s -> NEVER paced
spacing=20s -> NEVER paced
spacing=60s -> NEVER paced

The PR justifies the gap as "A container that merely restarts every minute or two is not a host problem and is
deliberately left alone." But the gap does not start at a minute or two — it starts at 13 seconds. An
exit-0-laundering container dying every 13s now gets ~277 unpaced container creations per hour, forever, each one a
full containerd shim + netns + veth + iptables + cgroup cycle. That container was previously paced to the 30-minute
cap.

The "Note for review" only owns the 77-second Palworld case. The actual behaviour change covers everything from 13s to
the 10-minute stable-run window. I'd ask for either a second, wider rung (e.g. 20 restarts in 10 minutes also
disposes into the ladder) or an explicit, quantified statement that 13s–10min unpaced is accepted — with the config
comment corrected, because "cause-blind backstop" currently oversells what it catches.

2. boot-lock.js now fails open where it used to fail closed

if (queue.indexOf(ticket) < BOOT_LOCK_WIDTH) { /* acquired */ }

indexOf returns -1 when our ticket is not in the queue, and -1 < 2 is true — so a missing ticket reads as "you hold
the lock". bootQueue() returns [] whenever readdirSync throws, and run-parallel.sh:94 does rm -rf /tmp/e2e-boot-lock.
If the directory ever disappears mid-gate, every waiter acquires simultaneously and the semaphore is silently defeated
— which is precisely the starvation/contention failure the lock was written to prevent.

The old queue[0] === ticket failed closed (kept waiting, eventually threw the explicit "queue is wedged" error).
One-line fix:

const pos = queue.indexOf(ticket);
if (pos >= 0 && pos < BOOT_LOCK_WIDTH) { ... }

Low probability with the current runner, but a silent over-admission is a bad failure mode for a lock and the fix is
free.

3. The stale comment on the failed-start path

appReconciler.js:910-913 still says "pacing is free — the attempt was recorded above, so a persistent failure walks
the backoff ladder instead of hammering." That is no longer directly true: a start failure on a never-run container
has exitCode: null → crashed=false → no ladder entry. It now retries every MANAGED_RETRY_MS (5s) and is bounded only
by the ceiling (5 × 5s = 25s ≤ 60s window, so it does still trip). It works, but the coupling is now implicit and
undocumented: if restartBurstWindowMs is ever tuned below 5 × MANAGED_RETRY_MS — and both are config-driven precisely
so the harness can compress them — a permanently failing start hammers forever. Worth updating the comment and,
ideally, a test pinning that relationship.

4. The stacking claim in the description is factually wrong

▎ "no file in this PR is touched by any branch below it, so this retargets development as the five below it land."

Not so. Overlaps with the branches below:

┌──────────────────────────────────────────┬─────────────────────┐
│ file │ also touched by │
├──────────────────────────────────────────┼─────────────────────┤
│ ZelBack/config/default.js │ #1774, #1777, #1778 │
├──────────────────────────────────────────┼─────────────────────┤
│ appMonitoring/appReconciler.js │ #1774, #1777, #1779 │
├──────────────────────────────────────────┼─────────────────────┤
│ test-infra/runner/framework/boot-lock.js │ #1774 │
├──────────────────────────────────────────┼─────────────────────┤
│ test-infra/runner/framework/test-env.js │ #1774, #1779 │
├──────────────────────────────────────────┼─────────────────────┤
│ tests/unit/appReconciler.test.js │ #1777 │
└──────────────────────────────────────────┴─────────────────────┘

I cherry-picked 0a205a6 onto origin/development: it conflicts on ZelBack/config/default.js. The conflict is trivial
(adjacent config keys), and the intent — merge last, after the stack — is fine. But "retarget and go" is not accurate;
this needs to merge in stack order or be rebased.

5. 6f7016a's commit message contradicts the PR body

matters

burstExceeded is evaluated before the append, against 5 entries spanning 5 × interval. So the trip condition is 5d ≤
60000 → d ≤ 12.0s. I measured it directly:

spacing=12.000s -> paced after 6 restarts
spacing=12.001s -> NEVER paced (60 restarts, 720s of hammering)
spacing=13s -> NEVER paced
spacing=20s -> NEVER paced
spacing=60s -> NEVER paced

The PR justifies the gap as "A container that merely restarts every minute or two is not a host problem and is
deliberately left alone." But the gap does not start at a minute or two — it starts at 13 seconds. An
exit-0-laundering container dying every 13s now gets ~277 unpaced container creations per hour, forever, each one a
full containerd shim + netns + veth + iptables + cgroup cycle. That container was previously paced to the 30-minute
cap.

The "Note for review" only owns the 77-second Palworld case. The actual behaviour change covers everything from 13s to
the 10-minute stable-run window. I'd ask for either a second, wider rung (e.g. 20 restarts in 10 minutes also
disposes into the ladder) or an explicit, quantified statement that 13s–10min unpaced is accepted — with the config
comment corrected, because "cause-blind backstop" currently oversells what it catches.

2. boot-lock.js now fails open where it used to fail closed

if (queue.indexOf(ticket) < BOOT_LOCK_WIDTH) { /* acquired */ }

indexOf returns -1 when our ticket is not in the queue, and -1 < 2 is true — so a missing ticket reads as "you hold
the lock". bootQueue() returns [] whenever readdirSync throws, and run-parallel.sh:94 does rm -rf /tmp/e2e-boot-lock.
If the directory ever disappears mid-gate, every waiter acquires simultaneously and the semaphore is silently defeated
— which is precisely the starvation/contention failure the lock was written to prevent.

The old queue[0] === ticket failed closed (kept waiting, eventually threw the explicit "queue is wedged" error).
One-line fix:

const pos = queue.indexOf(ticket);
if (pos >= 0 && pos < BOOT_LOCK_WIDTH) { ... }

Low probability with the current runner, but a silent over-admission is a bad failure mode for a lock and the fix is
free.

3. The stale comment on the failed-start path

appReconciler.js:910-913 still says "pacing is free — the attempt was recorded above, so a persistent failure walks
the backoff ladder instead of hammering." That is no longer directly true: a start failure on a never-run container
has exitCode: null → crashed=false → no ladder entry. It now retries every MANAGED_RETRY_MS (5s) and is bounded only
by the ceiling (5 × 5s = 25s ≤ 60s window, so it does still trip). It works, but the coupling is now implicit and
undocumented: if restartBurstWindowMs is ever tuned below 5 × MANAGED_RETRY_MS — and both are config-driven precisely
so the harness can compress them — a permanently failing start hammers forever. Worth updating the comment and,
ideally, a test pinning that relationship.

4. The stacking claim in the description is factually wrong

▎ "no file in this PR is touched by any branch below it, so this retargets development as the five below it land."

Not so. Overlaps with the branches below:

┌──────────────────────────────────────────┬─────────────────────┐
│ file │ also touched by │
├──────────────────────────────────────────┼─────────────────────┤
│ ZelBack/config/default.js │ #1774, #1777, #1778 │
├──────────────────────────────────────────┼─────────────────────┤
│ appMonitoring/appReconciler.js │ #1774, #1777, #1779 │
├──────────────────────────────────────────┼─────────────────────┤
│ test-infra/runner/framework/boot-lock.js │ #1774 │
├──────────────────────────────────────────┼─────────────────────┤
│ test-infra/runner/framework/test-env.js │ #1774, #1779 │
├──────────────────────────────────────────┼─────────────────────┤
│ tests/unit/appReconciler.test.js │ #1777 │
└──────────────────────────────────────────┴─────────────────────┘

I cherry-picked 0a205a6 onto origin/development: it conflicts on ZelBack/config/default.js. The conflict is trivial
(adjacent config keys), and the intent — merge last, after the stack — is fine. But "retarget and go" is not accurate;
this needs to merge in stack order or be rebased.

5. 6f7016a's commit message contradicts the PR body

The message ends with "NOT YET VALIDATED AT GATE SCALE." The PR body then reports a full 82/82 gate at that exact SHA
in 70 minutes. The validation happened; the message didn't get amended. Anyone reading git log later inherits a
warning that is no longer true.

6. Observed behaviour the description doesn't mention (not a defect)

Simulating the real loop for an exit-0 container dying every 2s gives waits of [0,0,0,0,0,0, 30s, 5m, 0,0,0,0,0, 15m,
...]. Each paced wait itself breaks the burst window, so the container earns another ~6 free restarts before it
re-trips. It does escalate to the 30m cap, and steady state is ~6 restarts per 30 minutes — bounded and perfectly
acceptable. But it is not the monotone ladder the description implies ("the next restart is paced 30s, and if the
hammering resumes it walks to the 30m cap"). One sentence in the body would set expectations correctly for whoever
debugs this from logs later.

---

Verdict

Not a blind ack — but nothing here is a correctness bug in the reconciler. The change is well-reasoned, the exit-code
asymmetry argument is sound, and the test coverage is unusually good.

What I'd ask for before acking:

1. #1 needs an answer — either a wider second rung, or an explicit, quantified acceptance of the 13s–10min gap with
   the config comment corrected. This is the substantive one.
2. #2 — the pos >= 0 guard. One line, take it.
3. #4 and #5 — description and commit-message accuracy. Cheap, and #4 matters for whoever does the merge.
4. #3 and #6 — comment/description updates; would not block on them alone.

If David pushes back on #1 with "13s–10min unpaced is the intended trade and the customer outcome is worth it," that's
a legitimate call and I'd ack on the strength of the rest — but it should be a stated decision, not a threshold that
reads as tighter than it is.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (fix/syncthing-first-run-gate@dc491ae). Learn more about missing BASE report.

Additional details and impacted files

@@                       Coverage Diff                       @@
##             fix/syncthing-first-run-gate    #1780   +/-   ##
===============================================================
  Coverage                                ?   65.12%           
===============================================================
  Files                                   ?      176           
  Lines                                   ?    33037           
  Branches                                ?        0           
===============================================================
  Hits                                    ?    21516           
  Misses                                  ?    11521           
  Partials                                ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[MorningLightMountain713]

Thanks — the substantive one was right and is fixed, along with three of the others. Branch is now 3581b93ad.

1. The 12-second cliff — fixed by widening the window to 300 seconds (6704fb900). Your arithmetic was correct: the ceiling is judged before the append against five entries spanning five gaps, so its reach is window / count, which at 5-in-60s was 12.0 seconds and nothing slower. I reproduced it against the real module with a control case that must trip, so a false "never paced" would have shown up.

The knob is really that one number. 5-in-300 puts the reach at 60 seconds, which is the band the description always claimed. Widening is safer than it looks in the direction that matters: setOperatorStopped(id, false) clears both arrays before the docker operation, so an operator restarting an app while debugging cannot walk into it, and a wrong trip costs a wait that the next genuine crash or operator start clears.

Framing worth stating, because it is the reason this mattered: the ceiling is not about "crashing". A non-zero exit or an OOM kill is recognised as a fault and paced by that path. A container exiting 0 is never treated as a fault at all, even when it is failing and hiding it — so for an image that launders its exit status, the ceiling is the only thing that would ever pace it. A 13-second invisible band matters more than it sounds.

Palworld sits outside the new reach: roughly 77 seconds at its worst against a 60-second threshold. That margin is 17 seconds on a number the note itself hedges, so the body now states the boundary and the observation together rather than leaving it to be rediscovered.

The reach had no test, and one derived from the same constants cannot notice them being retuned — which is how twelve seconds went unnoticed. Both are pinned now: the mechanism against window / count, and the shipped decision in seconds. The latter fails against the old window.

2. pos >= 0 — taken (3581b93ad). Proven with a probe against the real module before and after: with the directory intact a full queue correctly refuses the waiter; with it removed the old code admitted one, and now both fail closed.

3. The stale comment — corrected. It now says what actually happens: a failed start never ran, so it is not a fault and does not walk the ladder directly; it reaches the ladder by filling the burst window. The relationship you flagged is written next to restartBurstWindowMs, where someone tuning it would see it. The 300-second window also takes that margin from 2.4x to 12x.

4. The stacking claim — corrected, and it was worse than you scoped. Six of the ten files are shared, not one: ZelBack/config/default.js (#1774, #1775, #1777, #1778), appReconciler.js (#1774, #1777, #1779), tests/unit/appReconciler.test.js (#1774, #1777), test-env.js (#1774, #1779), boot-lock.js (#1774) and tests/unit/globalconfig/default.js (#1774). You missed #1775 on the config file, #1774 on the reconciler test, and the globalconfig file entirely. The body now says it merges in stack order.

5. The commit message — not changing it. A commit message is a record of what was true when it was written; we don't amend to keep them current. The PR body carries the current state, and it does.

6. The non-monotone ladder — documented. Each paced wait breaks the burst window, so the container earns roughly six more restarts before it re-trips, settling at about six per 30 minutes. Bounded, as you say, but not the straight walk to the cap the description implied.

Verified at the top of the stack: 85 of 85 suites green at f6c6bc399.

[Cabecinha84]

PR #1780 — deep review at head afc31cc

Verdict: don't ACK yet. The 12-second cliff from the previous round is genuinely fixed, and the boot-lock fix is
correct. But the burst path has a second-order defect that makes the PR's own headline claim about the bound false,
and it re-opens a laundering bug that this very file's doc comment warns about. It's a small fix, not a redesign.

What I verified (and how)

I built a worktree at afc31cc, ran the real modules, and simulated the actual reconciler loop order (restartWaitMs →
if paced, wait and re-enter; else recordRestart → start → run → die) against the real appsRuntimeState with a fake DB
— not the unit tests' arithmetic.

• Unit suite: 5190 passing, 5 pending, 23 failing — all 23 are dockerService tests requiring a live Docker daemon,
  untouched by this PR. appsRuntimeState + appReconciler together: 134 passing, 0 failing.
• Lint: clean (0 errors) on every changed lint-covered file. boot-lock.js and suite 54 are under an ignore pattern.
• CI: build (ubuntu-22.04, 20.x, 7.0) passing.
• Call sites: exactly two (appReconciler.js:874, :906), both updated; both new params default to true, so nothing else
  changes.
• Classification: crashed is computed after the !reachable and indeterminate returns, so it never reads a degraded
  inspect. exitCode is gated on everRan; OOMKilled comes from the same inspect (no second call, no TOCTOU). Five
  table-driven tests cover it.
• The ceiling's premise holds: dockerService.js:1034 creates containers with RestartPolicy: { Name: 'no' }, so every
  restart genuinely flows through the reconciler and lands in the window. Nothing restarts behind its back.
• The reach is now exactly 60s, as claimed. Measured against the real module: 59s spacing → paced; 60s → paced; 61s →
  never paced, ever. Palworld at ~77s stays unpaced. The previous round's finding is properly fixed.
• Mutation check: reverting restartBurstWindowMs to 60000 fails exactly one test — "paces a laundered-exit container
  dying every 30 seconds". The shipped-decision test is non-vacuous, as the author claimed.
• boot-lock pos >= 0: correct and complete. Fail-closed is restored; a vanished ticket now ends at the explicit wedged
  error. bootQueue()'s sort is a stable (ms, pid) ordering, so width 2 is a real FIFO semaphore, not a race.

Finding 1 — the burst path launders its own ladder and never reaches the 30-minute cap ⟵ blocker

restartWaitMs resets the ladder when the previous run provably lasted STABLE_RUN_MS:

const lastRestart = history[history.length - 1]; // restartHistory
const lastDeath = Math.max(state.lastDiedAt || 0, lastFinishedAtMs || 0);
if (lastDeath > lastRestart && lastDeath - lastRestart > STABLE_RUN_MS) { reset; return 0; }

Before this PR, restartHistory received every restart, so lastRestart was always the real one and the difference was
the real run duration. This PR makes recordRestart append to restartHistory only when crashed || burstExceeded. The
automatic exit-0 restarts in between are now invisible to that check — so after any rung longer than 600s, the stale
ladder entry sits more than STABLE_RUN_MS behind the current death and the code concludes the container ran stably for
ten minutes. It did not: it restarted six times in twelve seconds.

Traced against the real module, exit-0 container dying every 2s:

t= 1250s wait= 0s ladder=[10,40,350] window=[342,344,346,348,350]
t= 1252s ... 1258s five unpaced restarts, none recorded in the ladder
t= 1260s wait= 0s ladder=[10,40,350] <-- STABLE-RUN RESET (ladder wiped)
t= 1262s wait= 28s ladder=[1260] <-- back to rung 1

Steady state is a closed cycle 30s → 5m → 15m → reset → 30s, forever. The 30-minute rung is unreachable on this path.

┌─────────────────────────────────────┬──────────────┬──────────────────────────────────┐
│ exit-0 container dying every 2s │ starts in 2h │ rungs observed │
├─────────────────────────────────────┼──────────────┼──────────────────────────────────┤
│ base (fix/syncthing-first-run-gate) │ 7 │ 30s, 5m, 15m, 30m cap │
├─────────────────────────────────────┼──────────────┼──────────────────────────────────┤
│ this PR at afc31cc │ 78 │ 30s, 5m, 15m — cap never reached │
├─────────────────────────────────────┼──────────────┼──────────────────────────────────┤
│ PR body's stated steady state │ ~24 │ "escalates to the 30-minute cap" │
└─────────────────────────────────────┴──────────────┴──────────────────────────────────┘

So the body's "It does escalate to the 30-minute cap, and steady state is about six restarts per 30 minutes" is wrong
in both halves — measured is ~19–20 per 30 min and the cap is never reached. That matters because for a laundered-exit
image the ceiling is, by the PR's own argument, the only thing that ever paces it, so what it actually bounds to is
the whole guarantee.

The same applies to the failed-start retry loop (MANAGED_RETRY_MS = 5000): it trips the ceiling as the new comment
promises, but then cycles the same way — ~39 attempts/hour rather than walking to the cap.

Why no test catches it: every new unit test calls restartWaitMs(id, null, false) and never sets lastDiedAt, so
lastDeath is 0 and the reset branch is structurally unreachable in the unit suite. E2E suite 54 asserts only the first
backoff and stops. The interaction between the burst path and the stable-run reset is untested end to end.

Fix — the stability comparison should use the last actual restart attempt, not the last ladder entry:

const lastAttempt = Math.max(lastRestart, ...(state.autoRestartWindow || [0]));
if (lastDeath > lastAttempt && lastDeath - lastAttempt > STABLE_RUN_MS) { ... }

For a genuinely crashing container the two arrays agree, so nothing changes there; a container that really did run 10
minutes still resets. Worth pairing with a unit test that passes death evidence across a full trip → pace → re-trip
cycle, since that is the shape no current test drives.

Finding 2 — the module default still says 60 seconds

const RESTART_BURST_WINDOW_MS = config.fluxapps.restartBurstWindowMs ?? 60 * 1000;

ZelBack/config/default.js ships 300000, but the in-module fallback was not moved with it. Any config that lacks the
key silently reinstates the 12-second reach that the last round existed to fix — and it would fail silently, because
both reach tests derive from the constant, not from the config. Should be ?? 5 * 60 * 1000. One line.

Finding 3 — the PR body is internally inconsistent after the retune

• Overview: "a container restarting faster than five times in sixty seconds" — that is the old framing; it is now five
  in 300 seconds, reaching 60s apart.
• Changes table still lists restartBurstWindowMs: 60000; the shipped value is 300000.
• The cited SHAs (3581b93, 6704fb9, f6c6bc3) no longer exist on the branch — it was rebased on 2026-08-11 and
  head is afc31cc. The "85 of 85 suites green" attestation therefore points at a tree nobody can resolve. Worth
  re-anchoring to the current head, since that gate run is the only evidence for the harness commits.

Not blocking, worth a line

• The trip itself is unpaced. When the ceiling first fires, restartWaitMs falls through the early return, finds an
  empty restartHistory, and returns 0; pacing starts one restart later. Consistent with the body's [0,0,0,0,0,0, 30s,
  ...], so it looks deliberate — but the burstExceeded doc comment reads as though the crossing restart is the one
  paced, and it is not.
• BOOT_LOCK_WIDTH defaults to 2, changing harness parallelism for everyone. Documented, measured, and
  E2E_BOOT_LOCK_WIDTH=1 reverts it. Fine.
• Suite number 54 is free; seedTestApp genuinely supports { exitCode, exitAfterS } (reconciler-suite.js:418).