fix(fleet): lease-broker activation, symlink-safe unit placement, named launch refusal — Wall 6 (#1292) (#1297)
ci/woodpecker/push/publish Pipeline was successful
ci/woodpecker/push/publish Pipeline was successful
Co-authored-by: fargo <[email protected]>
This commit was merged in pull request #1297.
This commit is contained in:
@@ -1,11 +1,13 @@
|
||||
import { constants } from 'node:fs';
|
||||
import { constants, type Stats } from 'node:fs';
|
||||
import {
|
||||
access,
|
||||
chmod,
|
||||
copyFile,
|
||||
lstat,
|
||||
mkdir,
|
||||
open,
|
||||
readFile,
|
||||
readlink,
|
||||
stat,
|
||||
unlink,
|
||||
writeFile,
|
||||
@@ -90,6 +92,8 @@ export type SleepFn = (ms: number) => Promise<void>;
|
||||
|
||||
export interface FleetCommandDeps {
|
||||
runner?: CommandRunner;
|
||||
/** Test seam for the #1292 fleet-start broker preflight (socket presence). */
|
||||
checkBrokerSocket?: (path: string) => Promise<boolean> | boolean;
|
||||
/** Injectable interactive runner for commands needing inherited TTY (e.g., `tmux attach`). */
|
||||
interactiveRunner?: InteractiveRunner;
|
||||
/**
|
||||
@@ -815,6 +819,135 @@ export function buildSystemdEnableCommand(unit: string): string[] {
|
||||
return ['systemctl', '--user', 'enable', unit];
|
||||
}
|
||||
|
||||
/**
|
||||
* Place a unit file into the ACTIVE systemd user directory, never through a
|
||||
* symlink (#1292, measured 2026-08-17).
|
||||
*
|
||||
* ⚠ SET-INDEPENDENCE (fomo-lin, 2026-08-17): the set of unit names carrying
|
||||
* by-path residue and the set of unit names this install copies are
|
||||
* INDEPENDENT. Until 0.0.50 they were disjoint only by accident of which
|
||||
* units the install happened to name — fomo-lin survived copy-through solely
|
||||
* because its one by-path symlink (the broker) was the one unit the install
|
||||
* did NOT copy. Adding the broker to the copy set made the intersection
|
||||
* non-empty on the first run. Whoever adds a fifth unit to the placement
|
||||
* list inherits this helper and its unlink step; do not place units with a
|
||||
* bare copyFile.
|
||||
*
|
||||
* A host provisioned by the enable-by-path convention carries a symlink AT
|
||||
* the unit-name path in ~/.config/systemd/user/ pointing at the shipped
|
||||
* template under ~/.config/mosaic/systemd/user/. Node's copyFile FOLLOWS
|
||||
* that link and overwrites the SEED template instead of placing the active
|
||||
* unit (verified with fs.copyFile on a throwaway systemd user instance) —
|
||||
* silent, rc=0, and it mutates the directory every later reseed reads from.
|
||||
* The same measurement showed `systemctl enable <name>` does NOT rewrite an
|
||||
* existing by-path wants-symlink, so reconciliation must be explicit.
|
||||
*
|
||||
* Placement therefore: if the destination is a symlink, unlink it first
|
||||
* (unlink → copy — copy-then-unlink would mutate the seed and then destroy
|
||||
* the evidence that it did); then copy. Also removes a stale
|
||||
* `default.target.wants/<name>` symlink that points outside the active
|
||||
* directory (readlink — NOT readFile, which follows the link and returns the
|
||||
* target's CONTENT), so the subsequent enable-by-name recreates it against
|
||||
* the active copy. Idempotent: on a clean or already-reconciled destination
|
||||
* every step is a no-op (the copy rewrites identical bytes).
|
||||
*
|
||||
* Returns what was done, for assertions and install reporting.
|
||||
*/
|
||||
export interface PlaceUnitResult {
|
||||
readonly unit: string;
|
||||
readonly destination: string;
|
||||
/** A symlink at the unit-name path was unlinked (by-path residue). */
|
||||
readonly unlinkedDestinationSymlink: boolean;
|
||||
/** A stale wants-symlink pointing outside the active dir was removed. */
|
||||
readonly removedStaleWantsSymlink: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* placeUnitFile failed. Thrown BEFORE any copy: no destination bytes were
|
||||
* written, so a residue target cannot have been clobbered by a copy-through
|
||||
* (#1297 review F2).
|
||||
*/
|
||||
export class UnitPlacementError extends Error {
|
||||
constructor(
|
||||
readonly unit: string,
|
||||
message: string,
|
||||
) {
|
||||
super(message);
|
||||
this.name = UnitPlacementError.name;
|
||||
}
|
||||
}
|
||||
|
||||
function isErrnoException(error: unknown): error is NodeJS.ErrnoException {
|
||||
return error instanceof Error && 'code' in error && typeof error.code === 'string';
|
||||
}
|
||||
|
||||
export async function placeUnitFile(
|
||||
source: string,
|
||||
systemdUserDir: string,
|
||||
unit: string,
|
||||
): Promise<PlaceUnitResult> {
|
||||
const destination = join(systemdUserDir, unit);
|
||||
let unlinkedDestinationSymlink = false;
|
||||
// Destination-absent and unlink-FAILED are different outcomes and must not
|
||||
// share a catch (#1297 review F2): a swallowed unlink error used to fall
|
||||
// through to copyFile through the still-live symlink, silently reintroducing
|
||||
// the exact copy-through this helper exists to prevent.
|
||||
let destinationInfo: Stats | undefined;
|
||||
try {
|
||||
destinationInfo = await lstat(destination);
|
||||
} catch (error) {
|
||||
if (!isErrnoException(error) || error.code !== 'ENOENT') {
|
||||
throw new UnitPlacementError(
|
||||
unit,
|
||||
`cannot inspect destination ${destination}: ${error instanceof Error ? error.message : String(error)}`,
|
||||
);
|
||||
}
|
||||
// ENOENT: absent destination — nothing to unlink, copy below is safe.
|
||||
}
|
||||
if (destinationInfo?.isSymbolicLink()) {
|
||||
try {
|
||||
await unlink(destination);
|
||||
} catch (error) {
|
||||
// Abort BEFORE the copy: proceeding would run copyFile through the
|
||||
// still-live symlink and overwrite the residue target's bytes.
|
||||
throw new UnitPlacementError(
|
||||
unit,
|
||||
`cannot unlink destination symlink ${destination}: ${error instanceof Error ? error.message : String(error)}`,
|
||||
);
|
||||
}
|
||||
unlinkedDestinationSymlink = true;
|
||||
}
|
||||
await copyFile(source, destination);
|
||||
|
||||
let removedStaleWantsSymlink = false;
|
||||
const wantsLink = join(systemdUserDir, 'default.target.wants', unit);
|
||||
try {
|
||||
const wantsInfo = await lstat(wantsLink);
|
||||
if (wantsInfo.isSymbolicLink()) {
|
||||
// readlink — NOT readFile: readFile FOLLOWS the link and returns the
|
||||
// target file's CONTENT, which is not the question being asked.
|
||||
let target: string | undefined;
|
||||
try {
|
||||
target = await readlink(wantsLink);
|
||||
} catch {
|
||||
target = undefined;
|
||||
}
|
||||
// Normalize (systemctl writes absolute targets; a relative one resolves
|
||||
// against the wants dir). A wants-symlink pointing anywhere other than
|
||||
// the active copy (the by-path convention points at the seed template)
|
||||
// survives enable-by-name unchanged — remove it so enable recreates it.
|
||||
if (target !== undefined && resolve(dirname(wantsLink), target) !== destination) {
|
||||
await unlink(wantsLink);
|
||||
removedStaleWantsSymlink = true;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// absent wants link — nothing to reconcile
|
||||
}
|
||||
|
||||
return { unit, destination, unlinkedDestinationSymlink, removedStaleWantsSymlink };
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the systemctl --user disable command for a given unit.
|
||||
* Used by `fleet remove` so a removed agent's enabled unit cannot resurrect on
|
||||
@@ -849,6 +982,22 @@ export async function enableFleetUnits(
|
||||
let succeeded = 0;
|
||||
let failed = 0;
|
||||
|
||||
// The lease broker ships with the fleet and every gated runtime needs it
|
||||
// (#1292): seats die at lease registration without it, and no documented
|
||||
// path ever enabled it. Enabled first — alongside the holder — and the
|
||||
// unit must have been placed by installFleet's placeUnitFile step.
|
||||
const brokerResult = await runner(
|
||||
...splitCommand(buildSystemdEnableCommand('mosaic-lease-broker.service')),
|
||||
);
|
||||
if (brokerResult.exitCode === 0) {
|
||||
succeeded++;
|
||||
} else {
|
||||
failed++;
|
||||
process.stderr.write(
|
||||
`Warning: could not enable mosaic-lease-broker.service: ${brokerResult.stderr || brokerResult.stdout || 'non-zero exit'}\n`,
|
||||
);
|
||||
}
|
||||
|
||||
const holderResult = await runner(
|
||||
...splitCommand(buildSystemdEnableCommand('mosaic-tmux-holder.service')),
|
||||
);
|
||||
@@ -1545,7 +1694,7 @@ export function registerFleetCommand(program: Command, deps: FleetCommandDeps =
|
||||
.description('Install local fleet tools and user systemd units')
|
||||
.option('--no-enable', 'Skip enabling units for boot-survival')
|
||||
.action(async (opts: { enable?: boolean }) => {
|
||||
await installFleet(cmd, frameworkRoot);
|
||||
await installFleet(cmd, frameworkRoot, runner);
|
||||
// Unit enablement needs agent names only, so it reads either version.
|
||||
const roster = await loadRosterReadModel(cmd);
|
||||
await enableFleetUnits(runner, roster, opts);
|
||||
@@ -1556,7 +1705,7 @@ export function registerFleetCommand(program: Command, deps: FleetCommandDeps =
|
||||
.description('Install local fleet tools and user systemd units')
|
||||
.option('--no-enable', 'Skip enabling units for boot-survival')
|
||||
.action(async (opts: { enable?: boolean }) => {
|
||||
await installFleet(cmd, frameworkRoot);
|
||||
await installFleet(cmd, frameworkRoot, runner);
|
||||
// Unit enablement needs agent names only, so it reads either version.
|
||||
const roster = await loadRosterReadModel(cmd);
|
||||
await enableFleetUnits(runner, roster, opts);
|
||||
@@ -1609,6 +1758,37 @@ export function registerFleetCommand(program: Command, deps: FleetCommandDeps =
|
||||
);
|
||||
return;
|
||||
}
|
||||
if (action === 'start') {
|
||||
// Broker preflight (#1292), re-probed on EVERY invocation: a
|
||||
// gated runtime started without a live lease broker dies ~4s in
|
||||
// while the unit reports active (RemainAfterExit) — enabling +
|
||||
// starting here and then RE-CHECKING the socket refuses loudly
|
||||
// instead of reporting rc0 over a doomed start. This is the
|
||||
// second-start check as much as the first: it never trusts unit
|
||||
// ActiveState.
|
||||
await runChecked(runner, [
|
||||
'systemctl',
|
||||
'--user',
|
||||
'enable',
|
||||
'mosaic-lease-broker.service',
|
||||
]);
|
||||
await runChecked(runner, [
|
||||
'systemctl',
|
||||
'--user',
|
||||
'start',
|
||||
'mosaic-lease-broker.service',
|
||||
]);
|
||||
if (!(await brokerSocketPresent(deps))) {
|
||||
console.error(
|
||||
'[fleet] broker-absent: lease broker socket did not appear after enable+start (#1292).',
|
||||
);
|
||||
console.error(
|
||||
'[fleet] remedy: mosaic fleet install (it reconciles either enable convention)',
|
||||
);
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (action === 'restart') {
|
||||
// Serialize the holder+agents teardown/relaunch behind the restart lock
|
||||
// so a re-entrant restart waits for clean shutdown before relaunching,
|
||||
@@ -2367,7 +2547,11 @@ export function registerFleetAgentCommands(
|
||||
});
|
||||
}
|
||||
|
||||
async function installFleet(cmd: Command, frameworkRoot: string): Promise<void> {
|
||||
async function installFleet(
|
||||
cmd: Command,
|
||||
frameworkRoot: string,
|
||||
runner: CommandRunner,
|
||||
): Promise<void> {
|
||||
const activePaths = resolveFleetPaths(cmd.opts<{ mosaicHome: string }>().mosaicHome);
|
||||
assertDefaultMosaicHomeForSystemd(activePaths.mosaicHome);
|
||||
// Read model first: every file this function places is roster-independent, and
|
||||
@@ -2419,18 +2603,40 @@ async function installFleet(cmd: Command, frameworkRoot: string): Promise<void>
|
||||
for (const toolPath of executableToolPaths) {
|
||||
await chmod(toolPath, 0o755);
|
||||
}
|
||||
await copyFile(
|
||||
join(frameworkRoot, 'systemd', 'user', 'mosaic-tmux-holder.service'),
|
||||
join(activePaths.systemdUserDir, 'mosaic-tmux-holder.service'),
|
||||
// Unit placement (#1292): every unit goes through placeUnitFile — never a
|
||||
// bare copyFile — so a by-path-enable symlink at the destination is
|
||||
// unlinked rather than written through (copy-through would silently
|
||||
// overwrite the SEED template, measured 2026-08-17). The lease broker unit
|
||||
// is placed here too: previously the install named three units and omitted
|
||||
// the broker entirely, which is why no documented path ever enabled it.
|
||||
const placedUnits = await Promise.all(
|
||||
[
|
||||
'mosaic-tmux-holder.service',
|
||||
'[email protected]',
|
||||
'[email protected]',
|
||||
'mosaic-lease-broker.service',
|
||||
].map((unit) =>
|
||||
placeUnitFile(join(frameworkRoot, 'systemd', 'user', unit), activePaths.systemdUserDir, unit),
|
||||
),
|
||||
);
|
||||
await copyFile(
|
||||
join(frameworkRoot, 'systemd', 'user', '[email protected]'),
|
||||
join(activePaths.systemdUserDir, '[email protected]'),
|
||||
);
|
||||
await copyFile(
|
||||
join(frameworkRoot, 'systemd', 'user', '[email protected]'),
|
||||
join(activePaths.systemdUserDir, '[email protected]'),
|
||||
const reconciled = placedUnits.filter(
|
||||
(result) => result.unlinkedDestinationSymlink || result.removedStaleWantsSymlink,
|
||||
);
|
||||
if (reconciled.length > 0) {
|
||||
console.log(
|
||||
`Reconciled ${reconciled.length} unit placement(s) from by-path enable residue: ${reconciled.map((r) => r.unit).join(', ')}`,
|
||||
);
|
||||
}
|
||||
// systemd will not see a replaced unit file without a reload; do it once
|
||||
// after all placements, before any enable call below. runCommand never
|
||||
// rejects (it resolves exitCode 127 on spawn error), so a plain await with
|
||||
// an exitCode check matches the rest of this file's systemctl handling.
|
||||
const reloadResult = await runner(...splitCommand(['systemctl', '--user', 'daemon-reload']));
|
||||
if (reloadResult.exitCode !== 0) {
|
||||
process.stderr.write(
|
||||
`Warning: systemctl --user daemon-reload after unit placement failed (non-systemd host?): ${reloadResult.stderr || reloadResult.stdout || 'non-zero exit'}\n`,
|
||||
);
|
||||
}
|
||||
|
||||
// On roster v2 the reconciler owns the generated env: `apply` writes it and
|
||||
// `regen` rebuilds it, both from projectRosterV2AgentGeneratedEnv. Writing it
|
||||
@@ -2627,6 +2833,45 @@ function splitCommand(command: string[]): [string, string[]] {
|
||||
return [bin, args];
|
||||
}
|
||||
|
||||
/**
|
||||
* Lease-broker socket presence for the fleet-start preflight (#1292).
|
||||
* Resolution precedence matches launch.ts's defaultLeaseBrokerSocket and
|
||||
* start-agent-session.sh's broker_socket_path: explicit
|
||||
* MOSAIC_LEASE_BROKER_SOCKET, else $XDG_RUNTIME_DIR/mosaic-lease/broker.sock,
|
||||
* else /run/user/<uid>/mosaic-lease/broker.sock. Pure filesystem check — this
|
||||
* deliberately does NOT consult systemd state: a unit can be active
|
||||
* (RemainAfterExit) with no live socket, and the socket is the thing the
|
||||
* gated runtime connects to. Injectable via deps for tests.
|
||||
*/
|
||||
export function resolveLeaseBrokerSocketForPreflight(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
uid: number = typeof process.getuid === 'function' ? process.getuid() : 0,
|
||||
): string {
|
||||
if (env['MOSAIC_LEASE_BROKER_SOCKET']) return env['MOSAIC_LEASE_BROKER_SOCKET'];
|
||||
const runtimeDir = env['XDG_RUNTIME_DIR'] ?? `/run/user/${uid}`;
|
||||
return join(runtimeDir, 'mosaic-lease', 'broker.sock');
|
||||
}
|
||||
|
||||
export async function brokerSocketPresent(
|
||||
deps: FleetCommandDeps,
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): Promise<boolean> {
|
||||
const check = deps.checkBrokerSocket;
|
||||
const socketPath = resolveLeaseBrokerSocketForPreflight(env);
|
||||
if (check) return check(socketPath);
|
||||
// S_IFSOCK (0xC000) is a file-TYPE constant, not an access() mode (0-7):
|
||||
// access(path, S_IFSOCK) throws ERR_OUT_OF_RANGE on node >= 24 (measured on
|
||||
// v24.18.0, #1297 review F1) and cannot succeed on any node — the old catch
|
||||
// swallowed the throw, so this probe could NEVER return true and every
|
||||
// un-seamed call reported the broker absent. stat() + isSocket() is the real
|
||||
// check and matches the bash side's [ -S ].
|
||||
try {
|
||||
return (await stat(socketPath)).isSocket();
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** All supported fleet profile names. */
|
||||
export type FleetProfile =
|
||||
| 'general'
|
||||
|
||||
Reference in New Issue
Block a user