Skip to content

Add auto-discovery of local assets from filesystem import#386

Merged
pedramamini merged 2 commits intoRunMaestro:mainfrom
ksylvan:kayvan/fix/playbook-assets-copy
Feb 16, 2026
Merged

Add auto-discovery of local assets from filesystem import#386
pedramamini merged 2 commits intoRunMaestro:mainfrom
ksylvan:kayvan/fix/playbook-assets-copy

Conversation

@ksylvan
Copy link
Copy Markdown
Contributor

@ksylvan ksylvan commented Feb 16, 2026

Add auto-discovery of local assets from filesystem import

Summary

  • Added local asset auto-discovery for marketplace playbook imports and expanded tests to validate discovery and merging behavior.
  • When importing local playbooks, assets are now sourced from both manifest entries and files found under an assets/ folder, with deduplication.

Screenshots

Before fix, after importing a playbook with a local assets/ directory:

image

After fix:

image

Files Changed

  • src/main/ipc/handlers/marketplace.ts: Added logic to discover local asset files and merge them with manifest assets before import.
  • src/__tests__/main/ipc/handlers/marketplace.test.ts: Added mocks for fs.readdir/fs.stat and new test cases covering local asset discovery and merging.

Code Changes

  • Local asset discovery and asset list union for local playbooks:
const manifestAssets = marketplacePlaybook.assets ?? [];
let effectiveAssets = manifestAssets;

if (isLocalPath(marketplacePlaybook.path)) {
  const discoveredAssets: string[] = [];
  const resolvedPlaybookPath = resolveTildePath(marketplacePlaybook.path);
  const localAssetsPath = path.join(resolvedPlaybookPath, 'assets');

  try {
    const entries = await fs.readdir(localAssetsPath);
    for (const entry of entries) {
      const entryPath = path.join(localAssetsPath, entry);
      try {
        const stat = await fs.stat(entryPath);
        if (stat.isFile()) {
          discoveredAssets.push(entry);
        }
      } catch (error) {
        logger.warn(`Failed to stat local asset candidate: ${entryPath}`, LOG_CONTEXT, { error });
      }
    }
  } catch (error) {
    if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
      logger.warn(`Failed to read local assets directory: ${localAssetsPath}`, LOG_CONTEXT, { error });
    }
  }

  effectiveAssets = Array.from(new Set([...manifestAssets, ...discoveredAssets]));
}
  • Use effectiveAssets during import:
if (effectiveAssets.length > 0) {
  for (const assetFilename of effectiveAssets) {
    // fetch and write
  }
}
  • New tests validate:
    • Auto-discovery when manifest assets are absent.
    • Merge of discovered and manifest assets with deduplication.

Reason for Changes

  • Local playbooks may omit asset listings in manifests; this change ensures assets under assets/ are still imported without requiring manifest updates.
  • Improves resilience and user experience for locally authored playbooks.

Impact of Changes

  • Local imports now include assets found on disk even when not declared.
  • Remote (GitHub) playbooks remain unchanged, using manifest assets only.
  • Adds logging and graceful handling for missing assets/ directories or unreadable entries.

Test Plan

  • Run existing marketplace IPC handler tests.
  • New tests in marketplace.test.ts cover:
    • Discovery-only assets.
    • Manifest + discovery merge without duplicates.

Additional Notes

  • Potential edge cases:
    • Non-file entries in assets/ are ignored (by stat.isFile()).
    • If assets/ exists but is unreadable, assets may be partially imported; warnings are logged.
    • Deduplication uses filename equality; files with same name but different casing on case-insensitive FS may collide.

…ook import

- Add local asset auto-discovery from `assets/` directory for local playbooks
- Merge discovered filesystem assets with manifest-declared assets without duplicates
- Use `Set` to deduplicate combined manifest and discovered asset lists
- Silently skip `ENOENT` errors when `assets/` directory does not exist
- Log warning on `stat` failures for individual asset candidates
- Log effective asset count after local discovery and merge
- Replace hardcoded `marketplacePlaybook.assets` with computed `effectiveAssets`
- Add tests for auto-discovery when manifest assets are absent
- Add tests for merging discovered and manifest assets without duplicates
- Mock `fs.readdir` and `fs.stat` in marketplace test setup
@pedramamini pedramamini merged commit 7e8a00f into RunMaestro:main Feb 16, 2026
1 check failed
@ksylvan ksylvan deleted the kayvan/fix/playbook-assets-copy branch February 16, 2026 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants