fix(fleet): preserve exact v1 source strings
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful

This commit is contained in:
Jarvis
2026-07-16 07:55:10 -05:00
parent 355571182f
commit d63bb0206a
2 changed files with 355 additions and 18 deletions

View File

@@ -7,6 +7,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest';
import { loadFleetRoster } from '../commands/fleet.js'; import { loadFleetRoster } from '../commands/fleet.js';
import { executeFleetReconcile } from './fleet-reconciler.js'; import { executeFleetReconcile } from './fleet-reconciler.js';
import { compareCodePoints } from './deterministic-order.js'; import { compareCodePoints } from './deterministic-order.js';
import type { FleetRoster } from './fleet-roster-v1.js';
import { SHIPPED_FLEET_ARTIFACT_DISPOSITIONS } from './example-profile-dispositions.js'; import { SHIPPED_FLEET_ARTIFACT_DISPOSITIONS } from './example-profile-dispositions.js';
import { import {
collectShippedFleetMigrationDispositions, collectShippedFleetMigrationDispositions,
@@ -17,6 +18,21 @@ import {
} from './v1-v2-migration.js'; } from './v1-v2-migration.js';
const temporaryDirectories: string[] = []; const temporaryDirectories: string[] = [];
type V1StringParityCase = Readonly<{
field: string;
source: () => string;
productionValue?: (roster: FleetRoster) => unknown;
expectedProduction?: unknown;
productionError?: string;
expectedStatus: 'ready' | 'blocked';
blockerCode?: string;
blockerPath?: string;
migrationDecisions?: () => V1ToV2MigrationDecisions;
candidateValue?: (preview: V1ToV2MigrationPreview) => unknown;
expectedCandidate?: unknown;
}>;
const frameworkFleet = resolve( const frameworkFleet = resolve(
dirname(fileURLToPath(import.meta.url)), dirname(fileURLToPath(import.meta.url)),
'..', '..',
@@ -413,6 +429,267 @@ describe('v1-to-v2 migration preview', (): void => {
expect(preview.evidence).not.toHaveProperty('candidate'); expect(preview.evidence).not.toHaveProperty('candidate');
}); });
it.each([
{
field: 'runtime',
source: (): string => v1Source.replace('runtime: pi', 'runtime: " pi "'),
productionValue: (roster: FleetRoster): unknown => roster.agents[0]?.runtime,
expectedProduction: ' pi ',
expectedStatus: 'blocked',
blockerCode: 'unsupported-runtime',
blockerPath: 'agents[0].runtime',
},
{
field: 'class',
source: (): string => v1Source.replace('class: implementer', 'class: " implementer "'),
productionValue: (roster: FleetRoster): unknown => roster.agents[0]?.className,
expectedProduction: 'code',
expectedStatus: 'ready',
candidateValue: (preview: V1ToV2MigrationPreview): unknown =>
preview.canonicalRoster?.agents[0]?.className,
expectedCandidate: 'code',
},
{
field: 'custom class',
source: (): string => v1Source.replace('class: implementer', 'class: " custom-domain "'),
productionValue: (roster: FleetRoster): unknown => roster.agents[0]?.className,
expectedProduction: 'custom-domain',
expectedStatus: 'blocked',
blockerCode: 'class-disposition-required',
blockerPath: 'agents[0].class',
},
{
field: 'empty class with preserve disposition',
source: (): string => v1Source.replace('class: implementer', 'class: ""'),
productionValue: (roster: FleetRoster): unknown => roster.agents[0]?.className,
expectedProduction: '',
expectedStatus: 'blocked',
blockerCode: 'empty-v1-field-disposition-required',
blockerPath: 'agents[0].class',
migrationDecisions: (): V1ToV2MigrationDecisions => ({
...decisions(),
agents: {
...decisions().agents,
coder0: {
...decisions().agents.coder0!,
classDisposition: { action: 'preserve' },
},
},
}),
},
{
field: 'empty tool policy with preserve disposition',
source: (): string => v1Source.replace('tool_policy: implementer', 'tool_policy: ""'),
productionValue: (roster: FleetRoster): unknown => roster.agents[0]?.toolPolicy,
expectedProduction: '',
expectedStatus: 'blocked',
blockerCode: 'empty-v1-field-disposition-required',
blockerPath: 'agents[0].tool_policy',
migrationDecisions: (): V1ToV2MigrationDecisions => ({
...decisions(),
agents: {
...decisions().agents,
coder0: {
...decisions().agents.coder0!,
toolPolicyDisposition: { action: 'preserve' },
},
},
}),
},
{
field: 'tool policy',
source: (): string =>
v1Source.replace('tool_policy: implementer', 'tool_policy: " implementer "'),
productionValue: (roster: FleetRoster): unknown => roster.agents[0]?.toolPolicy,
expectedProduction: ' implementer ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'agents[0].tool_policy',
},
{
field: 'camel-case tool policy',
source: (): string =>
v1Source.replace('tool_policy: implementer', 'toolPolicy: " implementer "'),
productionValue: (roster: FleetRoster): unknown => roster.agents[0]?.toolPolicy,
expectedProduction: ' implementer ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'agents[0].toolPolicy',
},
{
field: 'alias',
source: (): string => v1Source.replace('alias: Coder Zero', 'alias: " Coder Zero "'),
productionValue: (roster: FleetRoster): unknown => roster.agents[0]?.alias,
expectedProduction: ' Coder Zero ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'agents[0].alias',
},
{
field: 'agent working directory',
source: (): string =>
v1Source.replace('working_directory: /srv/coder0', 'working_directory: " /srv/coder0 "'),
productionValue: (roster: FleetRoster): unknown => roster.agents[0]?.workingDirectory,
expectedProduction: ' /srv/coder0 ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'agents[0].working_directory',
},
{
field: 'camel-case agent working directory',
source: (): string =>
v1Source.replace('working_directory: /srv/coder0', 'workingDirectory: " /srv/coder0 "'),
productionValue: (roster: FleetRoster): unknown => roster.agents[0]?.workingDirectory,
expectedProduction: ' /srv/coder0 ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'agents[0].workingDirectory',
},
{
field: 'default working directory',
source: (): string =>
v1Source
.replace('working_directory: /srv/fleet', 'working_directory: " /srv/fleet "')
.replace(' working_directory: /srv/coder0\n', ''),
productionValue: (roster: FleetRoster): unknown => roster.defaults.workingDirectory,
expectedProduction: ' /srv/fleet ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'defaults.working_directory',
},
{
field: 'camel-case default working directory',
source: (): string =>
v1Source
.replace('working_directory: /srv/fleet', 'workingDirectory: " /srv/fleet "')
.replace(' working_directory: /srv/coder0\n', ''),
productionValue: (roster: FleetRoster): unknown => roster.defaults.workingDirectory,
expectedProduction: ' /srv/fleet ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'defaults.workingDirectory',
},
{
field: 'tmux holder session',
source: (): string =>
v1Source.replace('holder_session: _holder', 'holder_session: " _holder "'),
productionValue: (roster: FleetRoster): unknown => roster.tmux.holderSession,
expectedProduction: ' _holder ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'tmux.holder_session',
},
{
field: 'camel-case tmux holder session',
source: (): string =>
v1Source.replace('holder_session: _holder', 'holderSession: " _holder "'),
productionValue: (roster: FleetRoster): unknown => roster.tmux.holderSession,
expectedProduction: ' _holder ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'tmux.holderSession',
},
{
field: 'runtime reset command',
source: (): string => v1Source.replace('reset_command: /new', 'reset_command: " /new "'),
productionValue: (roster: FleetRoster): unknown => roster.runtimes.pi?.resetCommand,
expectedProduction: ' /new ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'runtimes.pi.reset_command',
},
{
field: 'camel-case runtime reset command',
source: (): string => v1Source.replace('reset_command: /new', 'resetCommand: " /new "'),
productionValue: (roster: FleetRoster): unknown => roster.runtimes.pi?.resetCommand,
expectedProduction: ' /new ',
expectedStatus: 'blocked',
blockerCode: 'lossy-v1-string-normalization',
blockerPath: 'runtimes.pi.resetCommand',
},
{
field: 'remote inventory alias',
source: (): string =>
v1Source.replace(
' - name: remote0\n runtime: pi',
' - name: remote0\n alias: " Remote Zero "\n runtime: pi',
),
productionValue: (roster: FleetRoster): unknown => roster.agents[1]?.alias,
expectedProduction: ' Remote Zero ',
expectedStatus: 'ready',
candidateValue: (preview: V1ToV2MigrationPreview): unknown =>
preview.inventoryOnlyAgents.find(({ name }) => name === 'remote0')?.disposition,
expectedCandidate: 'inventory-only',
},
{
field: 'host',
source: (): string =>
v1Source.replace('host: build.example.test', 'host: " build.example.test "'),
productionError: 'unsupported targeting characters',
expectedStatus: 'blocked',
blockerCode: 'invalid-v1-targeting-characters',
blockerPath: 'agents[1].host',
},
{
field: 'SSH target',
source: (): string =>
v1Source.replace(
'ssh: operator@build.example.test',
'ssh: " operator@build.example.test "',
),
productionError: 'unsupported targeting characters',
expectedStatus: 'blocked',
blockerCode: 'invalid-v1-targeting-characters',
blockerPath: 'agents[1].ssh',
},
] satisfies readonly V1StringParityCase[])(
'matches production v1 semantics for padded $field source',
async ({
source: buildSource,
productionValue,
expectedProduction,
productionError,
expectedStatus,
blockerCode,
blockerPath,
migrationDecisions,
candidateValue,
expectedCandidate,
}: V1StringParityCase) => {
const source = buildSource();
const rosterPath = join(await mkdtemp(join(tmpdir(), 'v1-string-parity-')), 'roster.yaml');
temporaryDirectories.push(dirname(rosterPath));
await writeFile(rosterPath, source);
if (productionError) {
await expect(loadFleetRoster(rosterPath)).rejects.toThrow(productionError);
} else {
const productionRoster = await loadFleetRoster(rosterPath);
expect(productionValue?.(productionRoster)).toBe(expectedProduction);
}
const preview = await previewV1ToV2Migration({
source,
decisions: migrationDecisions?.() ?? decisions(),
observations: observations(),
personaDirs: await semanticDirs(),
});
expect(preview.status).toBe(expectedStatus);
if (expectedStatus === 'blocked') {
expect(preview.blockers).toContainEqual(
expect.objectContaining({ code: blockerCode, path: blockerPath }),
);
expect(preview).not.toHaveProperty('canonicalRoster');
expect(preview).not.toHaveProperty('canonicalYaml');
expect(preview.evidence).not.toHaveProperty('candidate');
} else {
expect(preview.blockers).toEqual([]);
expect(candidateValue?.(preview)).toBe(expectedCandidate);
}
},
);
it.each([ it.each([
['socket_name', " socket_name: ''\n holder_session: _holder"], ['socket_name', " socket_name: ''\n holder_session: _holder"],
['socketName', " socketName: ''\n holder_session: _holder"], ['socketName', " socketName: ''\n holder_session: _holder"],

View File

@@ -348,7 +348,8 @@ export async function previewV1ToV2Migration(
assertV1Root(raw, blockers); assertV1Root(raw, blockers);
validateV1FieldCatalog(raw, blockers); validateV1FieldCatalog(raw, blockers);
blockPresentEmptyV1Fields(raw, blockers); blockPresentEmptyV1Fields(raw, decisions.fleetHost, blockers);
blockLossyV1StringNormalization(raw, decisions.fleetHost, blockers);
const tmux = record(raw.tmux); const tmux = record(raw.tmux);
const defaults = record(raw.defaults); const defaults = record(raw.defaults);
const runtimes = normalizeRuntimes(record(raw.runtimes), blockers); const runtimes = normalizeRuntimes(record(raw.runtimes), blockers);
@@ -439,13 +440,13 @@ export async function previewV1ToV2Migration(
continue; continue;
} }
const runtime = stringValue(rawAgent.runtime); const runtime = exactStringValue(rawAgent.runtime);
if (!isRuntime(runtime)) { if (!isRuntime(runtime)) {
blockers.push( blockers.push(
blocker('unsupported-runtime', `${path}.runtime`, 'Runtime must be supported.', name), blocker('unsupported-runtime', `${path}.runtime`, 'Runtime must be supported.', name),
); );
} }
const requestedClass = stringValue(rawAgent.class) || 'worker'; const requestedClass = exactStringValue(rawAgent.class) ?? 'worker';
const className = resolveClass( const className = resolveClass(
requestedClass, requestedClass,
decision.classDisposition, decision.classDisposition,
@@ -454,7 +455,7 @@ export async function previewV1ToV2Migration(
blockers, blockers,
); );
const requestedToolPolicy = const requestedToolPolicy =
stringValue(rawAgent.tool_policy) || stringValue(rawAgent.toolPolicy); exactStringValue(rawAgent.tool_policy) ?? exactStringValue(rawAgent.toolPolicy);
const toolPolicy = resolveToolPolicy( const toolPolicy = resolveToolPolicy(
requestedToolPolicy, requestedToolPolicy,
decision.toolPolicyDisposition, decision.toolPolicyDisposition,
@@ -511,7 +512,7 @@ export async function previewV1ToV2Migration(
if (!runtime || !className || !toolPolicy || !desiredState) continue; if (!runtime || !className || !toolPolicy || !desiredState) continue;
candidateAgents.push({ candidateAgents.push({
name, name,
alias: stringValue(rawAgent.alias) || name, alias: exactStringValue(rawAgent.alias) ?? name,
class: className, class: className,
runtime, runtime,
provider: decision.provider, provider: decision.provider,
@@ -519,10 +520,10 @@ export async function previewV1ToV2Migration(
reasoning: decision.reasoning, reasoning: decision.reasoning,
tool_policy: toolPolicy, tool_policy: toolPolicy,
working_directory: working_directory:
stringValue(rawAgent.working_directory) || exactStringValue(rawAgent.working_directory) ??
stringValue(rawAgent.workingDirectory) || exactStringValue(rawAgent.workingDirectory) ??
stringValue(defaults.working_directory) || exactStringValue(defaults.working_directory) ??
stringValue(defaults.workingDirectory) || exactStringValue(defaults.workingDirectory) ??
'~/src', '~/src',
persistent_persona: persistentPersona, persistent_persona: persistentPersona,
reset_between_tasks: reset_between_tasks:
@@ -596,12 +597,14 @@ export async function previewV1ToV2Migration(
tmux: { tmux: {
socket_name: socketName, socket_name: socketName,
holder_session: holder_session:
stringValue(tmux.holder_session) || stringValue(tmux.holderSession) || '_holder', exactStringValue(tmux.holder_session) ??
exactStringValue(tmux.holderSession) ??
'_holder',
}, },
defaults: { defaults: {
working_directory: working_directory:
stringValue(defaults.working_directory) || exactStringValue(defaults.working_directory) ??
stringValue(defaults.workingDirectory) || exactStringValue(defaults.workingDirectory) ??
'~/src', '~/src',
runtime: defaultRuntime, runtime: defaultRuntime,
}, },
@@ -769,8 +772,8 @@ function classifyAgentLocality(
agent: Record<string, unknown>, agent: Record<string, unknown>,
fleetHost: string | undefined, fleetHost: string | undefined,
): AgentLocality { ): AgentLocality {
const host = stringValue(agent.host); const host = exactStringValue(agent.host);
const ssh = stringValue(agent.ssh); const ssh = exactStringValue(agent.ssh);
const hasHost = agent.host !== undefined; const hasHost = agent.host !== undefined;
const hasSsh = agent.ssh !== undefined; const hasSsh = agent.ssh !== undefined;
if (!hasHost && !hasSsh) return 'local'; if (!hasHost && !hasSsh) return 'local';
@@ -837,6 +840,7 @@ function inventoryEntry(path: string): MigrationInventoryEntry {
function blockPresentEmptyV1Fields( function blockPresentEmptyV1Fields(
raw: Record<string, unknown>, raw: Record<string, unknown>,
fleetHost: string | undefined,
blockers: MigrationBlocker[], blockers: MigrationBlocker[],
): void { ): void {
const tmux = record(raw.tmux); const tmux = record(raw.tmux);
@@ -852,7 +856,14 @@ function blockPresentEmptyV1Fields(
); );
} }
for (const [index, agentValue] of (Array.isArray(raw.agents) ? raw.agents : []).entries()) { for (const [index, agentValue] of (Array.isArray(raw.agents) ? raw.agents : []).entries()) {
blockPresentEmptyField(record(agentValue), ['alias'], `agents[${index}]`, blockers); const agent = record(agentValue);
if (classifyAgentLocality(agent, fleetHost) === 'remote') continue;
blockPresentEmptyField(
agent,
['alias', 'class', 'tool_policy', 'toolPolicy'],
`agents[${index}]`,
blockers,
);
blockPresentEmptyField( blockPresentEmptyField(
record(agentValue), record(agentValue),
['working_directory', 'workingDirectory'], ['working_directory', 'workingDirectory'],
@@ -881,6 +892,55 @@ function blockPresentEmptyField(
} }
} }
function blockLossyV1StringNormalization(
raw: Record<string, unknown>,
fleetHost: string | undefined,
blockers: MigrationBlocker[],
): void {
const tmux = record(raw.tmux);
blockPaddedV1Field(tmux, ['holder_session', 'holderSession'], 'tmux', blockers);
const defaults = record(raw.defaults);
blockPaddedV1Field(defaults, ['working_directory', 'workingDirectory'], 'defaults', blockers);
for (const [runtimeName, runtimeValue] of Object.entries(record(raw.runtimes))) {
blockPaddedV1Field(
record(runtimeValue),
['reset_command', 'resetCommand'],
`runtimes.${runtimeName}`,
blockers,
);
}
for (const [index, agentValue] of (Array.isArray(raw.agents) ? raw.agents : []).entries()) {
const agent = record(agentValue);
if (classifyAgentLocality(agent, fleetHost) === 'remote') continue;
blockPaddedV1Field(
agent,
['alias', 'tool_policy', 'toolPolicy', 'working_directory', 'workingDirectory'],
`agents[${index}]`,
blockers,
);
}
}
function blockPaddedV1Field(
value: Record<string, unknown>,
fields: readonly string[],
path: string,
blockers: MigrationBlocker[],
): void {
for (const field of fields) {
const source = exactStringValue(value[field]);
if (source !== undefined && source !== source.trim()) {
blockers.push(
blocker(
'lossy-v1-string-normalization',
`${path}.${field}`,
'A whitespace-padded v1 value cannot be silently normalized by the v2 compiler.',
),
);
}
}
}
function validateV1FieldCatalog(raw: Record<string, unknown>, blockers: MigrationBlocker[]): void { function validateV1FieldCatalog(raw: Record<string, unknown>, blockers: MigrationBlocker[]): void {
rejectUnknownKeys(raw, ROOT_FIELDS, 'root', blockers); rejectUnknownKeys(raw, ROOT_FIELDS, 'root', blockers);
requireObjectWhenPresent(raw.tmux, 'tmux', blockers); requireObjectWhenPresent(raw.tmux, 'tmux', blockers);
@@ -942,7 +1002,7 @@ function validateV1FieldCatalog(raw: Record<string, unknown>, blockers: Migratio
blocker('invalid-v1-field-type', `${path}.name`, 'A valid agent name is required.'), blocker('invalid-v1-field-type', `${path}.name`, 'A valid agent name is required.'),
); );
} }
if (stringValue(agent.runtime) === undefined) { if (exactStringValue(agent.runtime) === undefined) {
blockers.push( blockers.push(
blocker('invalid-v1-field-type', `${path}.runtime`, 'Agent runtime is required.'), blocker('invalid-v1-field-type', `${path}.runtime`, 'Agent runtime is required.'),
); );
@@ -1219,7 +1279,7 @@ function normalizeRuntimes(
} }
const runtime = record(value); const runtime = record(value);
const resetCommand = const resetCommand =
stringValue(runtime.reset_command) || stringValue(runtime.resetCommand) || '/clear'; exactStringValue(runtime.reset_command) ?? exactStringValue(runtime.resetCommand) ?? '/clear';
result[name] = { reset_command: resetCommand }; result[name] = { reset_command: resetCommand };
} }
return result; return result;
@@ -1233,7 +1293,7 @@ function resolveClass(
blockers: MigrationBlocker[], blockers: MigrationBlocker[],
): string | undefined { ): string | undefined {
const canonical = canonicalizeRoleClass(requested).canonicalClass; const canonical = canonicalizeRoleClass(requested).canonicalClass;
if (canonical !== requested || AUTOMATIC_CLASSES.has(canonical)) { if (AUTOMATIC_CLASSES.has(canonical)) {
if (disposition !== undefined) { if (disposition !== undefined) {
blockers.push( blockers.push(
blocker( blocker(