Skip to content

refactor: consolidate default hub configuration into infra - #387

Open
gblanc-1a wants to merge 1 commit into
AmadeusITGroup:mainfrom
gblanc-1a:refactor/default-hubs
Open

refactor: consolidate default hub configuration into infra#387
gblanc-1a wants to merge 1 commit into
AmadeusITGroup:mainfrom
gblanc-1a:refactor/default-hubs

Conversation

@gblanc-1a

Copy link
Copy Markdown
Contributor

Description

Two divergent copies of the default-hub configuration existed and had drifted apart:

packages/infra/src/hub/default-hubs.ts apps/vscode-extension/src/config/default-hubs.ts
Hubs Amadeus + Prompt Registry Community github/awesome-copilot + promptregistry/community-hub
JSON config/default-hubs.json config/defaultHubs.json (camelCase)
Consumer CLI init extension first-run selector

The extension's hardcoded fallback pointed at a hub that does not exist (promptregistry/community-hub), and the two icon fields meant different things — a VS Code codicon in one, a CLI emoji in the other. This makes infra the single source of truth for both delivery layers.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📝 Documentation update
  • ♻️ Code refactoring (no functional changes)
  • ⚡ Performance improvement
  • 🧪 Test coverage improvement
  • 🔧 Configuration/build changes

Mostly a refactor, but it does fix one real defect — see the recommended flag below.

Related Issues

None. Prerequisite for the auth-diagnostics work that replaces #374, which needs isDefaultHub.

Changes Made

  • Fixed an order-dependent bug: both entries in apps/vscode-extension/config/defaultHubs.json carried recommended: true, and getRecommendedHub() returns the first match. Only the Amadeus hub is recommended now. The community hub consequently loses its ⭐ and "(recommended)" suffix in the first-run picker — the intended behavior.
  • Added isDefaultHub(reference) and isRecommendedDefaultHub(reference), comparing type + location case-insensitively and ignoring the git ref (a default hub pinned to another branch is still the same hub).
  • Split icon (plain text/emoji, for the CLI) from a new optional codicon (VS Code selector), so one config serves both hosts without either rendering the other's value. extension.ts now uses $(${hub.codicon ?? 'cloud'}).
  • Deleted apps/vscode-extension/src/config/default-hubs.ts and its stale README, which documented filenames (defaultHubs.ts) that no longer existed.
  • Moved the JSON to packages/infra/config/default-hubs.json — the path the loader actually reads — and added config to infra's package.json files so it ships when published.
  • Replaced the h.name === 'Amadeus' string compare in packages/cli/src/commands/init.ts with isRecommendedDefaultHub.
  • Updated the JSON schema (icon is now plain text, codicon added), the root README path, docs/contributor-guide/architecture/validation.md, and dropped the now-dead !config/ allowlist from .vscodeignore.

Testing

Test Coverage

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing completed
  • All existing tests pass

New packages/infra/test/hub/default-hubs.test.ts — 11 tests covering both predicates (same reference, different ref, case-insensitive location, wrong type, unrelated hub) plus two invariants that guard this PR's fix: exactly one entry is recommended, and every entry has both an icon and a codicon.

Manual Testing Steps

  1. pnpm --filter "@ai-primitives-hub/*" build — clean.
  2. Vitest: core 228, infra 682 (+11 new), app 554, cli 286 passing.
  3. pnpm -C apps/vscode-extension run compile — clean, confirming the extension resolves getEnabledDefaultHubs/isDefaultHub from infra.
  4. pnpm -C apps/vscode-extension run test:unit — 2196 passing (no extension test imported the deleted module).
  5. eslint src test --fix in infra, cli, and the extension — clean.

Not covered by automated tests: the first-run picker's rendered labels. The codicon split is asserted at the config level (every entry has one), not at the UI level.

Tested On

  • macOS

  • Windows

  • Linux

  • VS Code Stable

  • VS Code Insiders

The loader resolves its JSON path via __dirname, which differs between the webpack bundle and a plain Node run — worth a sanity check on another platform, though both paths fall back to the identical hardcoded list.

Screenshots

None captured. The visible change is in the first-run hub picker: the community hub no longer shows ⭐/"(recommended)".

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Documentation

  • README.md updated
  • JSDoc comments added/updated
  • No documentation changes needed

Root README's repository-structure table pointed config/ at a path that does not exist; it now points at packages/infra/config/. New packages/infra/config/README.md replaces the stale extension-side one.

Additional Notes

Two judgement calls worth challenging:

  1. The JSON now lives in packages/infra/config/, outside dist/. The loader reads __dirname/../../config/default-hubs.json, so in the webpack-bundled extension it will not resolve and the hardcoded list is used instead. That list is byte-equivalent to the JSON, so behavior is identical either way — but it does mean the JSON is only an override for non-bundled (CLI) use. If you want it authoritative in the VSIX too, it needs a copy step.
  2. codicon is a new optional schema field. The alternative was to render the emoji directly in the picker and drop codicons entirely, which changes the UI. I preserved current rendering instead.

Reviewer Guidelines

Please pay special attention to:

  • The recommended flag change: the community hub visibly loses its star. Confirm that is wanted.
  • isDefaultHub ignoring ref — the auth fix uses this predicate to decide "no access to this hub is expected, not an error", so a too-broad match would silence a genuine failure on a fork.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0.

Two divergent copies existed: packages/infra/src/hub/default-hubs.ts
(Amadeus + community, config/default-hubs.json) and the extension's
src/config/default-hubs.ts (awesome-copilot + a non-existent community
hub, config/defaultHubs.json). The infra module is now the single source
of truth for both delivery layers:

- add isDefaultHub()/isRecommendedDefaultHub(), comparing type+location
  case-insensitively and ignoring the git ref
- split icon (plain text, CLI) from codicon (VS Code selector) so one
  config can serve both hosts
- move the JSON to packages/infra/config/default-hubs.json (the path the
  loader reads) and drop the camelCase extension copy
- give only the recommended hub recommended: true; two made
  getRecommendedHub() order-dependent
- replace the h.name === 'Amadeus' compare in cli init with the predicate

No behavior change beyond the recommended-flag fix.
try {
// Try to load from JSON file in packages/infra/config/. `__dirname` is
// available at runtime because this package compiles to CommonJS.
const configPath = path.join(__dirname, '..', '..', 'config', 'default-hubs.json');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @gblanc-1a thanks a lot for the this refractor, very nice to manage it at one place ❤️
It seems this path still returns false in the next line, so in the end the json file is not used instead it always uses the hardcoded config.
But I will have quick discussion with you once you back to discuss and I drafted the pr to you fork and once we align we can go with the merge
https://github.com/gblanc-1a/prompt-registry/pull/13/changes/d7418759dfdc313d395cad140220022a89157085..75679ec5f2679ba86732fb2acaa23db84d2744c6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants