diff --git a/packages/mosaic/src/fleet/v1-v2-migration.spec.ts b/packages/mosaic/src/fleet/v1-v2-migration.spec.ts index f23656f..75c24e7 100644 --- a/packages/mosaic/src/fleet/v1-v2-migration.spec.ts +++ b/packages/mosaic/src/fleet/v1-v2-migration.spec.ts @@ -7,6 +7,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { loadFleetRoster } from '../commands/fleet.js'; import { executeFleetReconcile } from './fleet-reconciler.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 { collectShippedFleetMigrationDispositions, @@ -17,6 +18,21 @@ import { } from './v1-v2-migration.js'; 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( dirname(fileURLToPath(import.meta.url)), '..', @@ -413,6 +429,267 @@ describe('v1-to-v2 migration preview', (): void => { 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([ ['socket_name', " socket_name: ''\n holder_session: _holder"], ['socketName', " socketName: ''\n holder_session: _holder"], diff --git a/packages/mosaic/src/fleet/v1-v2-migration.ts b/packages/mosaic/src/fleet/v1-v2-migration.ts index 4c3cb9e..69961d2 100644 --- a/packages/mosaic/src/fleet/v1-v2-migration.ts +++ b/packages/mosaic/src/fleet/v1-v2-migration.ts @@ -348,7 +348,8 @@ export async function previewV1ToV2Migration( assertV1Root(raw, blockers); validateV1FieldCatalog(raw, blockers); - blockPresentEmptyV1Fields(raw, blockers); + blockPresentEmptyV1Fields(raw, decisions.fleetHost, blockers); + blockLossyV1StringNormalization(raw, decisions.fleetHost, blockers); const tmux = record(raw.tmux); const defaults = record(raw.defaults); const runtimes = normalizeRuntimes(record(raw.runtimes), blockers); @@ -439,13 +440,13 @@ export async function previewV1ToV2Migration( continue; } - const runtime = stringValue(rawAgent.runtime); + const runtime = exactStringValue(rawAgent.runtime); if (!isRuntime(runtime)) { blockers.push( 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( requestedClass, decision.classDisposition, @@ -454,7 +455,7 @@ export async function previewV1ToV2Migration( blockers, ); const requestedToolPolicy = - stringValue(rawAgent.tool_policy) || stringValue(rawAgent.toolPolicy); + exactStringValue(rawAgent.tool_policy) ?? exactStringValue(rawAgent.toolPolicy); const toolPolicy = resolveToolPolicy( requestedToolPolicy, decision.toolPolicyDisposition, @@ -511,7 +512,7 @@ export async function previewV1ToV2Migration( if (!runtime || !className || !toolPolicy || !desiredState) continue; candidateAgents.push({ name, - alias: stringValue(rawAgent.alias) || name, + alias: exactStringValue(rawAgent.alias) ?? name, class: className, runtime, provider: decision.provider, @@ -519,10 +520,10 @@ export async function previewV1ToV2Migration( reasoning: decision.reasoning, tool_policy: toolPolicy, working_directory: - stringValue(rawAgent.working_directory) || - stringValue(rawAgent.workingDirectory) || - stringValue(defaults.working_directory) || - stringValue(defaults.workingDirectory) || + exactStringValue(rawAgent.working_directory) ?? + exactStringValue(rawAgent.workingDirectory) ?? + exactStringValue(defaults.working_directory) ?? + exactStringValue(defaults.workingDirectory) ?? '~/src', persistent_persona: persistentPersona, reset_between_tasks: @@ -596,12 +597,14 @@ export async function previewV1ToV2Migration( tmux: { socket_name: socketName, holder_session: - stringValue(tmux.holder_session) || stringValue(tmux.holderSession) || '_holder', + exactStringValue(tmux.holder_session) ?? + exactStringValue(tmux.holderSession) ?? + '_holder', }, defaults: { working_directory: - stringValue(defaults.working_directory) || - stringValue(defaults.workingDirectory) || + exactStringValue(defaults.working_directory) ?? + exactStringValue(defaults.workingDirectory) ?? '~/src', runtime: defaultRuntime, }, @@ -769,8 +772,8 @@ function classifyAgentLocality( agent: Record, fleetHost: string | undefined, ): AgentLocality { - const host = stringValue(agent.host); - const ssh = stringValue(agent.ssh); + const host = exactStringValue(agent.host); + const ssh = exactStringValue(agent.ssh); const hasHost = agent.host !== undefined; const hasSsh = agent.ssh !== undefined; if (!hasHost && !hasSsh) return 'local'; @@ -837,6 +840,7 @@ function inventoryEntry(path: string): MigrationInventoryEntry { function blockPresentEmptyV1Fields( raw: Record, + fleetHost: string | undefined, blockers: MigrationBlocker[], ): void { const tmux = record(raw.tmux); @@ -852,7 +856,14 @@ function blockPresentEmptyV1Fields( ); } 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( record(agentValue), ['working_directory', 'workingDirectory'], @@ -881,6 +892,55 @@ function blockPresentEmptyField( } } +function blockLossyV1StringNormalization( + raw: Record, + 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, + 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, blockers: MigrationBlocker[]): void { rejectUnknownKeys(raw, ROOT_FIELDS, 'root', blockers); requireObjectWhenPresent(raw.tmux, 'tmux', blockers); @@ -942,7 +1002,7 @@ function validateV1FieldCatalog(raw: Record, blockers: Migratio 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( blocker('invalid-v1-field-type', `${path}.runtime`, 'Agent runtime is required.'), ); @@ -1219,7 +1279,7 @@ function normalizeRuntimes( } const runtime = record(value); const resetCommand = - stringValue(runtime.reset_command) || stringValue(runtime.resetCommand) || '/clear'; + exactStringValue(runtime.reset_command) ?? exactStringValue(runtime.resetCommand) ?? '/clear'; result[name] = { reset_command: resetCommand }; } return result; @@ -1233,7 +1293,7 @@ function resolveClass( blockers: MigrationBlocker[], ): string | undefined { const canonical = canonicalizeRoleClass(requested).canonicalClass; - if (canonical !== requested || AUTOMATIC_CLASSES.has(canonical)) { + if (AUTOMATIC_CLASSES.has(canonical)) { if (disposition !== undefined) { blockers.push( blocker(