fix(#1237): let ps/install work on a roster-v2 fleet, and refuse add/remove honestly
On a roster-v2 fleet, `ps`, `install`, `install-systemd`, `add` and `remove` all failed in the v1 parser. The consequence was that a greenfield v2 box could never get its unit templates placed, so nothing downstream could start. The read-only commands get a narrow version-agnostic view of the roster (version, socket name, holder session, and per agent name/alias/runtime). This is deliberately not a v2 -> v1 downshift. A downshifted FleetRoster would be accepted by generateAgentEnvValues, which would make a third writer of fleet/agents/<name>.env.generated through the v1 mapping and break the #791 single-SSOT invariant that projectRosterV2AgentGeneratedEnv is documented to hold. The view is too small to write a roster or an env file back from, so that misuse is unavailable rather than merely discouraged. So on a v2 roster `install` places the tool files and the unit templates, enables the units, and writes no generated env at all. Env belongs to `apply` and `regen`, both already v2-native. That change alone would have traded an init-time failure for a boot-time one. `install` enables mosaic-agent@<name>.service (WantedBy=default.target) without starting it, so a reboot between `install` and the first `apply` would run ExecStart against an absent env file and fail every seat unit, further from its cause. The unit template now carries ConditionPathExists=%h/.config/mosaic/fleet/agents/%i.env.generated which skips an enabled-but-unconfigured unit cleanly and starts it on the next start once the reconciler has written env. On v1 it is a no-op, since v1 `install` writes env itself. Found in review by scooby. `add` and `remove` are not routed to `create` and `delete`. They are different operations: the v1 pair edits the roster and drives systemd, the v2 pair is documented as changing desired state without runtime actions. `add` also collects four fields where a v2 agent requires eleven, so routing it would mean inventing an operator's provider, alias, reasoning and tool policy. On v2 both now fail with the real two-step sequence instead. Tests: 8 new, 7 of which are red before this change. Includes the greenfield case scooby asked for — `ps` on a fresh v2 install with nothing running is rc=0 and lists every agent stopped, since that is the command an operator runs to find out why there is no seat. Note for anyone verifying this: a correct fix here shows `install` rc=0 and `start` rc=0 and still no live seat. #1240 (tmux absent) is upstream, #1241 (start reports lifecycle-complete over dead panes) and the missing agent runtime are downstream. A dead pane after this change is not a regression here. Refs #1237, #791, #1240, #1241
This commit is contained in:
@@ -34,6 +34,7 @@ export {
|
||||
resolveInstalledFleetRosterPath,
|
||||
} from '../fleet/fleet-roster-v1.js';
|
||||
export type { FleetAgent, FleetRoster } from '../fleet/fleet-roster-v1.js';
|
||||
import { parseRosterV2 } from '../fleet/roster-v2.js';
|
||||
import {
|
||||
registerFleetAgentCrudCommands,
|
||||
type FleetAgentCrudCommandDeps,
|
||||
@@ -820,7 +821,7 @@ export function buildEnableLingerCommand(user: string): string[] {
|
||||
*/
|
||||
export async function enableFleetUnits(
|
||||
runner: CommandRunner,
|
||||
roster: FleetRoster,
|
||||
roster: { readonly agents: readonly { readonly name: string }[] },
|
||||
opts: { enable?: boolean },
|
||||
): Promise<void> {
|
||||
if (opts.enable === false) {
|
||||
@@ -1527,7 +1528,8 @@ export function registerFleetCommand(program: Command, deps: FleetCommandDeps =
|
||||
.option('--no-enable', 'Skip enabling units for boot-survival')
|
||||
.action(async (opts: { enable?: boolean }) => {
|
||||
await installFleet(cmd, frameworkRoot);
|
||||
const roster = await loadRosterForCommand(cmd);
|
||||
// Unit enablement needs agent names only, so it reads either version.
|
||||
const roster = await loadRosterReadModel(cmd);
|
||||
await enableFleetUnits(runner, roster, opts);
|
||||
});
|
||||
|
||||
@@ -1537,7 +1539,8 @@ export function registerFleetCommand(program: Command, deps: FleetCommandDeps =
|
||||
.option('--no-enable', 'Skip enabling units for boot-survival')
|
||||
.action(async (opts: { enable?: boolean }) => {
|
||||
await installFleet(cmd, frameworkRoot);
|
||||
const roster = await loadRosterForCommand(cmd);
|
||||
// Unit enablement needs agent names only, so it reads either version.
|
||||
const roster = await loadRosterReadModel(cmd);
|
||||
await enableFleetUnits(runner, roster, opts);
|
||||
});
|
||||
|
||||
@@ -1688,7 +1691,9 @@ export function registerFleetCommand(program: Command, deps: FleetCommandDeps =
|
||||
.action(async (opts: { json?: boolean }) => {
|
||||
const commandOpts = cmd.opts<{ mosaicHome: string; roster?: string }>();
|
||||
const activePaths = resolveFleetPaths(commandOpts.mosaicHome);
|
||||
const roster = await loadRosterForCommand(cmd);
|
||||
// ps only reads, so it takes the version-agnostic read model rather than
|
||||
// the v1 parser, which rejects a v2 roster outright.
|
||||
const roster = await loadRosterReadModel(cmd);
|
||||
const { tenant_id, host } = getDefaultTenantAndHost();
|
||||
const nowMs = Date.now();
|
||||
|
||||
@@ -1908,6 +1913,9 @@ export function registerFleetCommand(program: Command, deps: FleetCommandDeps =
|
||||
start: boolean;
|
||||
},
|
||||
) => {
|
||||
if (await usesRosterV2ControlPlane(cmd)) {
|
||||
throw new Error(rosterV2MutationGuidance('add', 'create', name));
|
||||
}
|
||||
if (!VALID_FLEET_RUNTIMES.includes(opts.runtime)) {
|
||||
throw new Error(
|
||||
`Invalid runtime "${opts.runtime}". Valid runtimes: ${VALID_FLEET_RUNTIMES.join(', ')}.`,
|
||||
@@ -1973,6 +1981,9 @@ export function registerFleetCommand(program: Command, deps: FleetCommandDeps =
|
||||
.description('Remove an agent from the fleet roster')
|
||||
.option('--keep-files', 'Skip deleting env and heartbeat files')
|
||||
.action(async (name: string, opts: { keepFiles?: boolean }) => {
|
||||
if (await usesRosterV2ControlPlane(cmd)) {
|
||||
throw new Error(rosterV2MutationGuidance('remove', 'delete', name));
|
||||
}
|
||||
const commandOpts = cmd.opts<{ mosaicHome: string; roster?: string }>();
|
||||
const activePaths = resolveFleetPaths(commandOpts.mosaicHome);
|
||||
const rosterPath = await resolveRosterPath(commandOpts.mosaicHome, commandOpts.roster);
|
||||
@@ -2331,7 +2342,9 @@ export function registerFleetAgentCommands(
|
||||
async function installFleet(cmd: Command, frameworkRoot: string): Promise<void> {
|
||||
const activePaths = resolveFleetPaths(cmd.opts<{ mosaicHome: string }>().mosaicHome);
|
||||
assertDefaultMosaicHomeForSystemd(activePaths.mosaicHome);
|
||||
const roster = await loadRosterForCommand(cmd);
|
||||
// Read model first: every file this function places is roster-independent, and
|
||||
// the v1 parser would reject a v2 roster before any of them were written.
|
||||
const roster = await loadRosterReadModel(cmd);
|
||||
await ensureFleetHolderIdentity(activePaths.mosaicHome);
|
||||
await mkdir(activePaths.fleetToolsDir, { recursive: true });
|
||||
await mkdir(activePaths.tmuxToolsDir, { recursive: true });
|
||||
@@ -2391,16 +2404,30 @@ async function installFleet(cmd: Command, frameworkRoot: string): Promise<void>
|
||||
join(activePaths.systemdUserDir, '[email protected]'),
|
||||
);
|
||||
|
||||
for (const agent of roster.agents) {
|
||||
// On roster v2 the reconciler owns the generated env: `apply` writes it and
|
||||
// `regen` rebuilds it, both from projectRosterV2AgentGeneratedEnv. Writing it
|
||||
// here too — necessarily through the v1 mapping — would be the third writer of
|
||||
// one file and would break the #791 single-SSOT invariant. So v2 gets the tool
|
||||
// files and the units, and nothing else.
|
||||
if (roster.version === 2) {
|
||||
console.log(
|
||||
`Installed fleet tools and systemd units for ${roster.agents.length} agent(s). ` +
|
||||
`Generated env is owned by the reconciler on roster v2 — run: mosaic fleet apply --expected-generation <n>`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const v1Roster = await loadRosterForCommand(cmd);
|
||||
for (const agent of v1Roster.agents) {
|
||||
await writeAgentEnvironmentProjection({
|
||||
mosaicHome: activePaths.mosaicHome,
|
||||
agentEnvDir: activePaths.agentEnvDir,
|
||||
agentName: agent.name,
|
||||
generated: generateAgentEnvValues(roster, agent),
|
||||
generated: generateAgentEnvValues(v1Roster, agent),
|
||||
});
|
||||
}
|
||||
|
||||
console.log(`Installed fleet files for ${roster.agents.length} agent(s).`);
|
||||
console.log(`Installed fleet files for ${v1Roster.agents.length} agent(s).`);
|
||||
}
|
||||
|
||||
async function loadRosterForCommand(cmd: Command): Promise<FleetRoster> {
|
||||
@@ -2427,6 +2454,77 @@ async function usesRosterV2ControlPlane(cmd: Command): Promise<boolean> {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* `add`/`remove` and `create`/`delete` are not two spellings of one operation.
|
||||
* The v1 pair edits the roster *and* drives systemd; the v2 pair is documented
|
||||
* as changing desired state "without runtime actions", leaving convergence to
|
||||
* `apply`. `add` also collects four fields where a v2 agent requires eleven, so
|
||||
* routing it to `create` would mean inventing provider, alias, reasoning and
|
||||
* tool-policy defaults on the operator's behalf. Refusing with the real command
|
||||
* is honest; silently guessing an agent's provider is not.
|
||||
*/
|
||||
function rosterV2MutationGuidance(
|
||||
v1Command: 'add' | 'remove',
|
||||
v2Command: 'create' | 'delete',
|
||||
name: string,
|
||||
): string {
|
||||
const target = v2Command === 'delete' ? ` ${name}` : '';
|
||||
return (
|
||||
`mosaic fleet ${v1Command} does not operate on a roster-v2 fleet. ` +
|
||||
`Roster v2 separates desired state from convergence:\n` +
|
||||
` 1. mosaic fleet ${v2Command}${target} --expected-generation <current> ` +
|
||||
`${v2Command === 'create' ? "--agent '<json>' " : ''}` +
|
||||
`(edits the roster only)\n` +
|
||||
` 2. mosaic fleet apply --expected-generation <new> (converges systemd and tmux)\n` +
|
||||
`Read the current generation with: mosaic fleet status`
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* The read-only fields shared by roster v1 and v2, for the commands that only
|
||||
* ever *read* the roster (`ps`, and unit enablement inside `install`).
|
||||
*
|
||||
* This is deliberately NOT a v2→v1 downshift. A downshifted `FleetRoster` would
|
||||
* be accepted by `generateAgentEnvValues`, and that would make a third writer of
|
||||
* `fleet/agents/<name>.env.generated` — through the v1 mapping — breaking the
|
||||
* #791 single-SSOT invariant that {@link projectRosterV2AgentGeneratedEnv} is
|
||||
* documented to hold. Keeping the read model this small makes that misuse
|
||||
* impossible: there is nothing here to write a roster or an env file back from.
|
||||
*/
|
||||
interface FleetRosterReadModel {
|
||||
readonly version: 1 | 2;
|
||||
readonly tmux: { readonly socketName: string; readonly holderSession: string };
|
||||
readonly agents: readonly {
|
||||
readonly name: string;
|
||||
readonly alias?: string;
|
||||
readonly runtime: string;
|
||||
}[];
|
||||
}
|
||||
|
||||
/** Reads either roster version into the shared read-only view. */
|
||||
async function loadRosterReadModel(cmd: Command): Promise<FleetRosterReadModel> {
|
||||
const opts = cmd.opts<{ mosaicHome: string; roster?: string }>();
|
||||
const path = await resolveRosterPath(opts.mosaicHome, opts.roster);
|
||||
if (!(await usesRosterV2ControlPlane(cmd))) {
|
||||
const v1 = await loadRosterAtPath(cmd, path);
|
||||
return {
|
||||
version: 1,
|
||||
tmux: { socketName: v1.tmux.socketName, holderSession: v1.tmux.holderSession },
|
||||
agents: v1.agents,
|
||||
};
|
||||
}
|
||||
try {
|
||||
const v2 = parseRosterV2(await readFleetRosterText(path), 'yaml');
|
||||
return {
|
||||
version: 2,
|
||||
tmux: { socketName: v2.tmux.socketName, holderSession: v2.tmux.holderSession },
|
||||
agents: v2.agents,
|
||||
};
|
||||
} catch (error) {
|
||||
reportFleetRosterConfigurationError(cmd, error);
|
||||
}
|
||||
}
|
||||
|
||||
async function loadRosterFromAgentCommand(
|
||||
command: Command,
|
||||
mosaicHomeOverride?: string,
|
||||
|
||||
Reference in New Issue
Block a user