diff --git a/packages/core/src/core/discovery/platform-files-discovery.ts b/packages/core/src/core/discovery/platform-files-discovery.ts index 53dccf7c..34638e17 100644 --- a/packages/core/src/core/discovery/platform-files-discovery.ts +++ b/packages/core/src/core/discovery/platform-files-discovery.ts @@ -13,6 +13,8 @@ import { discoverFiles } from './file-discovery.js'; import { normalizePathForProcessing } from '../../utils/path-normalization.js'; import { WORKSPACE_DISCOVERY_EXCLUDES } from '../../constants/workspace.js'; import { DIR_PATTERNS } from '../../constants/index.js'; +import { extractToPatternString } from '../../core/flows/to-pattern-extractor.js'; +import { extractDefaultPattern } from '../../core/flows/switch-resolver.js'; /** * Process platform subdirectories (rules/commands/agents) within a base directory @@ -35,7 +37,7 @@ async function discoverPlatformFiles( const platformDirs = new Set(); for (const flow of definition.export) { - const toPattern = typeof flow.to === 'string' ? flow.to : Object.keys(flow.to)[0]; + const toPattern = extractToPatternString(flow.to, extractDefaultPattern); if (toPattern) { // Extract directory from pattern (e.g., ".cursor/rules/{name}.mdc" -> "rules") const parts = toPattern.split('/'); diff --git a/packages/core/src/core/flows/flow-execution-coordinator.ts b/packages/core/src/core/flows/flow-execution-coordinator.ts index 269d4f15..efd6c099 100644 --- a/packages/core/src/core/flows/flow-execution-coordinator.ts +++ b/packages/core/src/core/flows/flow-execution-coordinator.ts @@ -19,7 +19,8 @@ import { import { resolveRecursiveGlobTargetRelativePath } from '../glob-target-mapping.js'; import { logger } from '../../utils/logger.js'; import { stripPlatformSuffixFromFilename } from './platform-suffix-handler.js'; -import { resolveSwitchExpression, isSwitchExpression } from './switch-resolver.js'; +import { resolveSwitchExpression } from './switch-resolver.js'; +import { extractToPatternString } from './to-pattern-extractor.js'; import { normalizePathForProcessing } from '../../utils/path-normalization.js'; import { deriveResourceLeafFromPackageName } from '../../utils/plugin-naming.js'; import { deduplicateTargets } from '../../utils/workspace-index-helpers.js'; @@ -201,20 +202,10 @@ async function processSourceFile( } }; - // Resolve target path - handle switch expressions and FlowPattern - let rawToPattern: string; - if (typeof flow.to === 'string') { - rawToPattern = flow.to; - } else if (isSwitchExpression(flow.to)) { - // Resolve switch expression to concrete target path - rawToPattern = resolveSwitchExpression(flow.to, sourceContext); - } else if (typeof flow.to === 'object' && flow.to !== null && 'pattern' in flow.to) { - // FlowPattern { pattern: "...", schema?: "..." } - use pattern, not Object.keys - rawToPattern = (flow.to as { pattern: string }).pattern; - } else { - // Multi-target flows - use first target path key - rawToPattern = Object.keys(flow.to as object)[0] ?? ''; - } + const rawToPattern = extractToPatternString( + flow.to, + (sw) => resolveSwitchExpression(sw, sourceContext) + ) ?? ''; const resolvedToPattern = resolvePattern(rawToPattern, sourceContext, capturedName); const targetAbs = resolveTargetFromGlob( sourceAbs, diff --git a/packages/core/src/core/flows/flow-source-discovery.ts b/packages/core/src/core/flows/flow-source-discovery.ts index f320a530..25386e75 100644 --- a/packages/core/src/core/flows/flow-source-discovery.ts +++ b/packages/core/src/core/flows/flow-source-discovery.ts @@ -8,19 +8,19 @@ import { promises as fs } from 'fs'; import { join, dirname, basename, relative } from 'path'; import { minimatch } from 'minimatch'; -import type { Flow, FlowContext, SwitchExpression } from '../../types/flows.js'; +import type { Flow, FlowContext, FlowPatternValue, SwitchExpression } from '../../types/flows.js'; import { exists } from '../../utils/fs.js'; import { getAllPlatforms } from '../platforms.js'; import type { Platform } from '../platforms.js'; import { logger } from '../../utils/logger.js'; import { normalizePathForProcessing } from '../../utils/path-normalization.js'; -function isFlowPatternValue(value: any): value is { pattern: string; schema?: string } { +export function isFlowPatternValue(value: unknown): value is FlowPatternValue { return ( typeof value === 'object' && value !== null && 'pattern' in value && - typeof (value as any).pattern === 'string' + typeof (value as { pattern: unknown }).pattern === 'string' ); } diff --git a/packages/core/src/core/flows/switch-resolver.ts b/packages/core/src/core/flows/switch-resolver.ts index 1c13dc1f..e349eb3b 100644 --- a/packages/core/src/core/flows/switch-resolver.ts +++ b/packages/core/src/core/flows/switch-resolver.ts @@ -247,6 +247,19 @@ export function isSwitchExpression(value: unknown): value is SwitchExpression { ); } +/** + * Extract the default branch's pattern string from a SwitchExpression. + * Used by call sites that lack a runtime FlowContext (e.g. workspace-add, + * uninstall, listing) and must fall back to the default branch. + * + * Returns undefined when the expression has no default. + */ +export function extractDefaultPattern(expr: SwitchExpression): string | undefined { + const d = expr.$switch.default; + if (d === undefined) return undefined; + return extractPattern(d); +} + function isValidSwitchCaseValue(value: unknown): boolean { if (typeof value === 'string') { return true; diff --git a/packages/core/src/core/flows/to-pattern-extractor.ts b/packages/core/src/core/flows/to-pattern-extractor.ts new file mode 100644 index 00000000..c15fe510 --- /dev/null +++ b/packages/core/src/core/flows/to-pattern-extractor.ts @@ -0,0 +1,22 @@ +/** + * Resolve a single target pattern string from a Flow's `to` field. + * Handles string, FlowPatternValue, MultiTargetFlows, and SwitchExpression. + * + * Flow.to is currently typed as `string | MultiTargetFlows | SwitchExpression` + * but runtime/platforms.jsonc also allows FlowPatternValue. The cast inside + * `Object.keys(to as MultiTargetFlows)` is the marker for that future widening. + */ +import type { Flow, MultiTargetFlows, SwitchExpression } from '../../types/flows.js'; +import { isSwitchExpression } from './switch-resolver.js'; +import { isFlowPatternValue } from './flow-source-discovery.js'; + +export function extractToPatternString( + to: Flow['to'], + resolveSwitch: (expr: SwitchExpression) => string | undefined +): string | undefined { + if (typeof to === 'string') return to; + if (to === null || typeof to !== 'object') return undefined; + if (isSwitchExpression(to)) return resolveSwitch(to); + if (isFlowPatternValue(to)) return to.pattern; + return Object.keys(to as MultiTargetFlows)[0]; +} diff --git a/packages/core/src/core/install/helpers/conflict-detection.ts b/packages/core/src/core/install/helpers/conflict-detection.ts index 712358c6..fb87dfb6 100644 --- a/packages/core/src/core/install/helpers/conflict-detection.ts +++ b/packages/core/src/core/install/helpers/conflict-detection.ts @@ -10,6 +10,8 @@ import type { FlowContext } from '../../../types/flows.js'; import type { FlowConflictReport } from '../strategies/types.js'; import { discoverFlowSources } from '../../flows/flow-source-discovery.js'; import { resolvePattern } from '../../flows/flow-source-discovery.js'; +import { extractToPatternString } from '../../flows/to-pattern-extractor.js'; +import { resolveSwitchExpression } from '../../flows/switch-resolver.js'; /** * Represents a package that writes to a target file @@ -41,15 +43,15 @@ export async function trackTargetFiles( flowContext: FlowContext ): Promise { const flowSources = await discoverFlowSources(flows, packageRoot, flowContext); - + const resolveSwitch = (sw: Parameters[0]) => + resolveSwitchExpression(sw, flowContext); + for (const [flow, sources] of flowSources) { if (sources.length > 0) { - // Determine target path from flow - const targetPath = typeof flow.to === 'string' - ? resolvePattern(flow.to, flowContext) - : Object.keys(flow.to)[0]; - - // Track this package writing to this target + const rawPattern = extractToPatternString(flow.to, resolveSwitch); + if (!rawPattern) continue; + const targetPath = resolvePattern(rawPattern, flowContext); + if (!fileTargets.has(targetPath)) { fileTargets.set(targetPath, []); } diff --git a/packages/core/src/core/install/strategies/flow-based-strategy.ts b/packages/core/src/core/install/strategies/flow-based-strategy.ts index 41b04386..8d9b3da2 100644 --- a/packages/core/src/core/install/strategies/flow-based-strategy.ts +++ b/packages/core/src/core/install/strategies/flow-based-strategy.ts @@ -23,7 +23,8 @@ import { extractCapturedName, getFirstFromPattern, } from '../../flows/flow-source-discovery.js'; -import { resolveSwitchExpression, isSwitchExpression } from '../../flows/switch-resolver.js'; +import { resolveSwitchExpression } from '../../flows/switch-resolver.js'; +import { extractToPatternString } from '../../flows/to-pattern-extractor.js'; import { buildImportFlowContext, discoverAndFilterSources, @@ -277,17 +278,10 @@ export class FlowBasedInstallStrategy extends BaseStrategy { } }; - let rawToPattern: string; - if (typeof flow.to === 'string') { - rawToPattern = flow.to; - } else if (isSwitchExpression(flow.to)) { - rawToPattern = resolveSwitchExpression(flow.to, sourceContext); - } else if (typeof flow.to === 'object' && flow.to !== null && 'pattern' in flow.to) { - rawToPattern = (flow.to as { pattern: string }).pattern; - } else { - rawToPattern = Object.keys(flow.to as object)[0] ?? ''; - } - + const rawToPattern = extractToPatternString( + flow.to, + (sw) => resolveSwitchExpression(sw, sourceContext) + ) ?? ''; const resolvedToPattern = resolvePattern(rawToPattern, sourceContext, capturedName); const targetAbs = resolveTargetFromGlob( sourceAbs, diff --git a/packages/core/src/core/openpackage.ts b/packages/core/src/core/openpackage.ts index ad9d0fd2..f9c96f38 100644 --- a/packages/core/src/core/openpackage.ts +++ b/packages/core/src/core/openpackage.ts @@ -9,6 +9,8 @@ import { getLocalPackageYmlPath, getLocalPackagesDir } from '../utils/paths.js'; import { findFilesByExtension, findDirectoriesContainingFile } from '../utils/file-processing.js'; import { getDetectedPlatforms, getPlatformDefinition, type Platform } from './platforms.js'; import { arePackageNamesEquivalent } from '../utils/package-name.js'; +import { extractToPatternString } from './flows/to-pattern-extractor.js'; +import { extractDefaultPattern } from './flows/switch-resolver.js'; /** * Package metadata from openpackage directory @@ -308,7 +310,7 @@ export async function checkExistingPackageInMarkdownFiles( const platformDirs = new Set(); for (const flow of def.export) { - const toPattern = typeof flow.to === 'string' ? flow.to : Object.keys(flow.to)[0]; + const toPattern = extractToPatternString(flow.to, extractDefaultPattern); if (toPattern) { // Extract directory from pattern const parts = toPattern.split('/'); diff --git a/packages/core/src/core/platform/platform-file.ts b/packages/core/src/core/platform/platform-file.ts index a6bf0321..59b9eeff 100644 --- a/packages/core/src/core/platform/platform-file.ts +++ b/packages/core/src/core/platform/platform-file.ts @@ -14,6 +14,8 @@ import { import { DIR_PATTERNS, FILE_PATTERNS, type UniversalSubdir } from '../../constants/index.js'; import { getFirstPathComponent, parsePathWithPrefix, normalizePathForProcessing } from '../../utils/path-normalization.js'; import { mapUniversalToPlatform } from './platform-mapper.js'; +import { extractToPatternString } from '../flows/to-pattern-extractor.js'; +import { extractDefaultPattern } from '../flows/switch-resolver.js'; /** * Parse a registry or universal path to extract subdir and relative path info @@ -110,7 +112,7 @@ export function getPlatformSpecificFilename(universalPath: string, platform: Pla // For array patterns, use the first pattern const fromPattern = Array.isArray(flow.from) ? flow.from[0] : flow.from; if (fromPattern.includes(universalSubdir)) { - const toPattern = typeof flow.to === 'string' ? flow.to : Object.keys(flow.to)[0]; + const toPattern = extractToPatternString(flow.to, extractDefaultPattern); if (toPattern) { // Extract extension from 'to' pattern const toExtMatch = toPattern.match(/\.[^./]+$/); diff --git a/packages/core/src/core/platform/platform-mapper.ts b/packages/core/src/core/platform/platform-mapper.ts index 1a607615..fcab06c3 100644 --- a/packages/core/src/core/platform/platform-mapper.ts +++ b/packages/core/src/core/platform/platform-mapper.ts @@ -13,6 +13,8 @@ import { logger } from '../../utils/logger.js'; import { type UniversalSubdir } from '../../constants/index.js'; import { normalizePathForProcessing, findSubpathIndex } from '../../utils/path-normalization.js'; import { extractFirstComponent } from '../universal-patterns.js'; +import { extractToPatternString } from '../flows/to-pattern-extractor.js'; +import { extractDefaultPattern } from '../flows/switch-resolver.js'; /** * Extract pattern string from a flow pattern value @@ -337,18 +339,7 @@ export function mapWorkspaceFileToUniversal( } if (!matchedFromPattern) continue; - // Extract universal subdir from 'to' pattern (handle $switch in to) - let toPattern: string | undefined; - if (typeof flow.to === 'string') { - toPattern = flow.to; - } else if (typeof flow.to === 'object' && flow.to !== null && '$switch' in flow.to) { - const sw = (flow.to as { $switch: { cases?: Array<{ pattern: string; value: unknown }>; default?: unknown } }).$switch; - // For workspace add, use default (workspace paths like .opencode/... not ~/.config/...) - const d = sw.default; - toPattern = typeof d === 'string' ? d : (typeof d === 'object' && d !== null && 'pattern' in d) ? (d as { pattern: string }).pattern : undefined; - } else if (typeof flow.to === 'object' && flow.to !== null) { - toPattern = Object.keys(flow.to)[0]; - } + const toPattern = extractToPatternString(flow.to, extractDefaultPattern); if (!toPattern) continue; const toParts = toPattern.split('/'); diff --git a/tests/core/add/add-flow-based-mapping.test.ts b/tests/core/add/add-flow-based-mapping.test.ts index 6d12bec8..0b1b6661 100644 --- a/tests/core/add/add-flow-based-mapping.test.ts +++ b/tests/core/add/add-flow-based-mapping.test.ts @@ -73,6 +73,37 @@ describe('add flow-based mapping', { concurrency: 1 }, () => { } }); + // Regression: GH #54 — FlowPattern.to was extracted as the literal "pattern". + test('.claude/agents/*.md → agents/*.md (FlowPattern to)', async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'opkg-add-flow-test-')); + const originalCwd = process.cwd(); + + try { + process.chdir(tmp); + writeWorkspacePackageManifest(tmp); + ensureDir(path.join(tmp, '.claude')); + + const workspaceFile = path.join(tmp, '.claude', 'agents', 'monorepo-navigator.md'); + writeFile(workspaceFile, '# Monorepo Navigator\n\nAgent body.'); + + const result = await runAddToSourcePipeline(undefined, workspaceFile, {}); + + assert.ok(result.success, result.error); + assert.equal(result.data?.filesAdded, 1); + + const expectedPath = path.join(tmp, '.openpackage', 'agents', 'monorepo-navigator.md'); + assert.ok(fileExists(expectedPath), `Expected file not found at: ${expectedPath}`); + + const wrongPath = path.join(tmp, '.openpackage', 'pattern'); + assert.ok(!fileExists(wrongPath), `File should not be saved as literal "pattern": ${wrongPath}`); + const wrongPathNested = path.join(tmp, '.openpackage', 'pattern', 'monorepo-navigator.md'); + assert.ok(!fileExists(wrongPathNested), `File should not be saved under "pattern/": ${wrongPathNested}`); + } finally { + process.chdir(originalCwd); + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + test('.cursor/agents/*.md → agents/*.md', async () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'opkg-add-flow-test-')); const originalCwd = process.cwd(); diff --git a/tests/core/flows/unit/to-pattern-extractor.test.ts b/tests/core/flows/unit/to-pattern-extractor.test.ts new file mode 100644 index 00000000..ae2e4878 --- /dev/null +++ b/tests/core/flows/unit/to-pattern-extractor.test.ts @@ -0,0 +1,64 @@ +/** + * Unit tests for extractToPatternString + * + * Regression coverage for GH issue #54: a `to:` field shaped as + * `{ pattern: "...", schema: "..." }` was previously falling through to the + * multi-target catch-all and producing the literal string "pattern". + */ + +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { extractToPatternString } from '../../../../packages/core/src/core/flows/to-pattern-extractor.js'; +import type { Flow, SwitchExpression } from '../../../../packages/core/src/types/flows.js'; + +describe('extractToPatternString', () => { + const failOnSwitch = (sw: SwitchExpression): string | undefined => { + throw new Error(`switch resolver should not have been called: ${JSON.stringify(sw)}`); + }; + + it('returns the string as-is when `to` is a plain string', () => { + const result = extractToPatternString('agents/foo.md', failOnSwitch); + assert.strictEqual(result, 'agents/foo.md'); + }); + + it('returns the .pattern property when `to` is a FlowPattern (issue #54)', () => { + const to = { + pattern: 'agents/**/*.md', + schema: './schemas/formats/universal-agent.schema.json', + } as unknown as Flow['to']; + const result = extractToPatternString(to, failOnSwitch); + assert.strictEqual(result, 'agents/**/*.md'); + }); + + it('returns the first key when `to` is a MultiTargetFlows object', () => { + const to: Flow['to'] = { + '.cursor/rules/{name}.mdc': { merge: 'replace' }, + '.cursor/rules/extra.mdc': { merge: 'deep' }, + }; + const result = extractToPatternString(to, failOnSwitch); + assert.strictEqual(result, '.cursor/rules/{name}.mdc'); + }); + + it('delegates to the resolver callback when `to` is a SwitchExpression', () => { + const to: SwitchExpression = { + $switch: { + field: '$$targetRoot', + cases: [ + { pattern: '~/', value: '.config/opencode' }, + ], + default: 'fallback.md', + }, + }; + let received: SwitchExpression | undefined; + const result = extractToPatternString(to, (sw) => { + received = sw; + return 'resolved-by-callback.md'; + }); + assert.strictEqual(result, 'resolved-by-callback.md'); + assert.strictEqual(received, to); + }); + + it('returns undefined for null/non-object inputs', () => { + assert.strictEqual(extractToPatternString(null as unknown as Flow['to'], failOnSwitch), undefined); + }); +}); diff --git a/tests/run-tests.ts b/tests/run-tests.ts index 3c76ad5c..489d7df7 100644 --- a/tests/run-tests.ts +++ b/tests/run-tests.ts @@ -52,6 +52,7 @@ const testFiles: string[] = [ 'tests/core/flows/transforms/toml-transforms.test.ts', 'tests/core/flows/unified-platform-model.test.ts', 'tests/core/flows/unit/switch-resolution.test.ts', + 'tests/core/flows/unit/to-pattern-extractor.test.ts', // Core - Platforms 'tests/core/platforms/platform-extension-filter.test.ts',