Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion code/cli/src/cli/commands/SyncCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Command } from 'commander';

import { remoteSourceLayer } from '../../resolver/Layer.js';
import { resolveResources } from '../../resolver/Resolver.js';
import { resolveEffectiveSet } from '../../resolver/ResolverContext.js';
import { validateEffectiveSet } from '../../resolver/ResolverValidation.js';
import {
createSettingsLoadPlan,
Expand Down Expand Up @@ -350,7 +351,9 @@ export const executeSyncCommand = (
directRemoteSources,
sourcePhase,
});
return finishSync(merged.issues, remoteSettingsPhase, sourcePhase, transitiveClosure);
const result = finishSync(merged.issues, remoteSettingsPhase, sourcePhase, transitiveClosure);
const ambiguityWarnings = resolveEffectiveSet(input).ambiguityWarnings.map((warning) => `warning: ${warning}`);
return { ...result, messages: [...result.messages, ...ambiguityWarnings] };
};

export const createSyncCommand = (dependencies: SyncCommandDependencies = {}): CommandObject => ({
Expand Down
137 changes: 137 additions & 0 deletions code/cli/src/resolver/AmbiguityWarnings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
// Reports source-ref and resource-slug disagreements without changing deterministic precedence.
import type { LoadedSettingsFile } from '../settings/SettingsLoader.js';
import type { SourceReference } from '../settings/Settings.js';
import {
encodeRemoteSourceSelection,
isRemoteSource,
normalizeGitUri,
normalizeRemoteSourceUri,
redactSourceUriCredentials,
} from '../sources/SourceCache.js';
import type { RemoteSourceReference } from '../sources/SourceCache.js';
import type { DeclaredRemoteSource } from '../sources/TransitiveSources.js';
import type { EffectiveResourceSet, ResolvedResource, ResourceKind } from './Resource.js';

interface SourceDeclaration {
readonly source: RemoteSourceReference;
readonly declaredBy: string;
}

const repositoryKey = (source: RemoteSourceReference): string =>
redactSourceUriCredentials(normalizeGitUri(normalizeRemoteSourceUri(source)));

const repositoryDisplay = (source: RemoteSourceReference): string => {
if (source.github !== undefined) return `github:${source.github}`;
return redactSourceUriCredentials(normalizeGitUri(source.uri));
};

const refDisplay = (source: RemoteSourceReference): string => source.ref ?? '(default)';

const directDeclarations = (files: readonly LoadedSettingsFile[]): readonly SourceDeclaration[] =>
[...files]
.reverse()
.flatMap((file) =>
(file.settings.sources ?? [])
.filter(isRemoteSource)
.map((source) => ({ source, declaredBy: file.location.path })),
);

const replacedSourceListWarnings = (
files: readonly LoadedSettingsFile[],
effectiveSources: readonly SourceReference[],
): readonly string[] => {
const replacingIndex = files.findLastIndex((file) => file.settings.sources !== undefined);
if (replacingIndex < 1) return [];

const replacingFile = files[replacingIndex];
const effectiveRepositories = new Set(effectiveSources.filter(isRemoteSource).map(repositoryKey));
const reportedRepositories = new Set<string>();
const warnings: string[] = [];

for (const file of files.slice(0, replacingIndex).reverse()) {
for (const source of (file.settings.sources ?? []).filter(isRemoteSource)) {
const key = repositoryKey(source);
if (effectiveRepositories.has(key) || reportedRepositories.has(key)) continue;
reportedRepositories.add(key);
warnings.push(
`source '${repositoryDisplay(source)}' declared by '${file.location.path}' was replaced by '${replacingFile.location.path}' and is not in the effective configuration`,
);
}
}

return warnings;
};

const selectedDeclarations = (
effectiveSources: readonly SourceReference[],
direct: readonly SourceDeclaration[],
transitive: readonly DeclaredRemoteSource[],
): readonly SourceDeclaration[] => {
const selectedDirect = effectiveSources.filter(isRemoteSource).map((source) => {
const selection = encodeRemoteSourceSelection(source);
// Every effective direct source came from one of the loaded files used to produce settings.
return direct.find((entry) => encodeRemoteSourceSelection(entry.source) === selection)!;
});
return [...selectedDirect, ...transitive];
};

export const sourceRefAmbiguityWarnings = (
files: readonly LoadedSettingsFile[],
effectiveSources: readonly SourceReference[],
transitiveDeclarations: readonly DeclaredRemoteSource[],
): readonly string[] => {
const direct = directDeclarations(files);
const declarations: readonly SourceDeclaration[] = [...direct, ...transitiveDeclarations];
const selected = selectedDeclarations(effectiveSources, direct, transitiveDeclarations);
const byRepository = new Map<string, SourceDeclaration[]>();

for (const declaration of declarations) {
const key = repositoryKey(declaration.source);
const entries = byRepository.get(key) ?? [];
entries.push(declaration);
byRepository.set(key, entries);
}

const warnings: string[] = [...replacedSourceListWarnings(files, effectiveSources)];
for (const entries of byRepository.values()) {
if (new Set(entries.map((entry) => entry.source.ref)).size < 2) continue;
const key = repositoryKey(entries[0].source);
const winner = selected.find((entry) => repositoryKey(entry.source) === key);
if (winner === undefined) continue;
const declarationsText = entries
.map((entry) => `'${entry.declaredBy}' declares ref '${refDisplay(entry.source)}'`)
.join('; ');
warnings.push(
`Ambiguous source repository '${repositoryDisplay(entries[0].source)}': ${declarationsText}; declaration from '${winner.declaredBy}' at ref '${refDisplay(winner.source)}' won.`,
);
}

return warnings;
};

const slugWarning = (kind: ResourceKind, resource: ResolvedResource, context?: string): string | undefined => {
const definitions = [resource.winner, ...resource.shadowed];
const labels = [...new Set(definitions.map((definition) => definition.layer.label))];
if (labels.length < 2) return undefined;
return `Ambiguous ${kind} slug '${resource.slug}'${context ?? ''} is supplied by ${labels.map((label) => `'${label}'`).join(', ')}; '${resource.winner.layer.label}' won.`;
};

const resourceWarnings = (
kind: ResourceKind,
resources: Iterable<ResolvedResource> | undefined,
context?: string,
): readonly string[] =>
resources === undefined
? []
: [...resources].flatMap((resource) => {
const warning = slugWarning(kind, resource, context);
return warning === undefined ? [] : [warning];
});

export const slugAmbiguityWarnings = (set: EffectiveResourceSet): readonly string[] => [
...resourceWarnings('agent', set.resources.get('agent')?.values()),
...resourceWarnings('skill', set.resources.get('skill')?.values()),
...[...set.agentResources].flatMap(([agent, kinds]) =>
resourceWarnings('skill', kinds.get('skill')?.values(), ` for agent '${agent}'`),
),
];
4 changes: 4 additions & 0 deletions code/cli/src/resolver/Layer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from '../sources/SourceCache.js';
import type { RemoteSourceReference } from '../sources/SourceCache.js';
import { expandTransitiveSources } from '../sources/TransitiveSources.js';
import type { DeclaredRemoteSource } from '../sources/TransitiveSources.js';
import type { Settings, SourceReference } from '../settings/Settings.js';
import type { Layer } from './Resource.js';

Expand Down Expand Up @@ -42,6 +43,8 @@ const sourceLayer = (input: LayerDiscoveryInput, source: SourceReference): Layer

export interface LayerDiscoveryResult {
readonly layers: readonly Layer[];
/** Accepted transitive declarations, including duplicates, retained for ambiguity diagnostics. */
readonly transitiveDeclarations: readonly DeclaredRemoteSource[];
/**
* Configured remote sources whose cache is absent, reported with `outfitter sync` guidance rather
* than silently dropped (OFTR-004.2.18). Reported, never fatal: a private catalog the enterprise
Expand Down Expand Up @@ -113,6 +116,7 @@ export const discoverLayers = (input: LayerDiscoveryInput): LayerDiscoveryResult

return {
layers: candidates.filter((layer) => existsSync(layer.root)),
transitiveDeclarations: expansion.declarations,
unsynchronized,
warnings: [...invalid, ...expansion.warnings],
};
Expand Down
18 changes: 16 additions & 2 deletions code/cli/src/resolver/ResolverContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { loadSettingsWithCachedRemoteSettings } from '../settings/SettingsLoader.js';
import type { SettingsLoadIssue } from '../settings/SettingsLoader.js';
import type { Settings } from '../settings/Settings.js';
import { slugAmbiguityWarnings, sourceRefAmbiguityWarnings } from './AmbiguityWarnings.js';
import { discoverLayers } from './Layer.js';
import type { EffectiveResourceSet } from './Resource.js';
import { resolveResources } from './Resolver.js';
Expand All @@ -18,17 +19,30 @@ export interface ResolveResult {
readonly settingsIssues: readonly SettingsLoadIssue[];
/** Non-fatal guidance: uncached remote sources plus transitive-source skip warnings. */
readonly warnings: readonly string[];
/** Ambiguity-only subset, exposed so sync can report shared resolution diagnostics after fetching. */
readonly ambiguityWarnings: readonly string[];
}

/** The single shared resolution path used by list, validate, run, and dump. */
export const resolveEffectiveSet = (input: ResolveInput): ResolveResult => {
const loadedSettings = loadSettingsWithCachedRemoteSettings(input);
const discovered = discoverLayers({ ...input, settings: loadedSettings.settings });
const set = resolveResources(discovered.layers);
const ambiguityWarnings = [
...sourceRefAmbiguityWarnings(
loadedSettings.files,
// Settings merging always materializes the default empty source list.
loadedSettings.settings.sources!,
discovered.transitiveDeclarations,
),
...slugAmbiguityWarnings(set),
];

return {
set: resolveResources(discovered.layers),
set,
settings: loadedSettings.settings,
settingsIssues: loadedSettings.issues,
warnings: [...discovered.unsynchronized, ...discovered.warnings],
warnings: [...discovered.unsynchronized, ...discovered.warnings, ...ambiguityWarnings],
ambiguityWarnings,
};
};
6 changes: 5 additions & 1 deletion code/cli/src/sources/TransitiveSources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ export interface TransitiveExpansionInput {
export interface TransitiveExpansionResult {
/** Newly discovered remote sources, breadth-first by depth then declaration order (OFTR-004.6.3). */
readonly sources: readonly DeclaredRemoteSource[];
/** Every accepted declaration, including duplicates suppressed from the resolved source graph. */
readonly declarations: readonly DeclaredRemoteSource[];
readonly warnings: readonly string[];
}

Expand Down Expand Up @@ -190,6 +192,7 @@ const declarationsFromParent = (

export const expandTransitiveSources = (input: TransitiveExpansionInput): TransitiveExpansionResult => {
const sources: DeclaredRemoteSource[] = [];
const declarations: DeclaredRemoteSource[] = [];
const warnings: string[] = [];
const visited = new Set<string>();

Expand All @@ -214,6 +217,7 @@ export const expandTransitiveSources = (input: TransitiveExpansionInput): Transi
warnings.push(...declared.warnings);

for (const entry of declared.sources) {
declarations.push(entry);
const key = encodeRemoteSourceSelection(entry.source);
if (visited.has(key)) continue;
visited.add(key);
Expand All @@ -225,5 +229,5 @@ export const expandTransitiveSources = (input: TransitiveExpansionInput): Transi
frontier = next;
}

return { sources, warnings };
return { sources, declarations, warnings };
};
Loading
Loading