Skip to content

Add positron.workspace.registerConfigurationMigrations extension API#14298

Open
samclark2015 wants to merge 4 commits into
mainfrom
samclark2015/extension-settings-migration-api
Open

Add positron.workspace.registerConfigurationMigrations extension API#14298
samclark2015 wants to merge 4 commits into
mainfrom
samclark2015/extension-settings-migration-api

Conversation

@samclark2015

@samclark2015 samclark2015 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds positron.workspace.registerConfigurationMigrations to the Positron extension API, allowing extensions to declaratively migrate configuration keys they previously contributed.

Each ConfigurationMigrationSpec specifies a source key and destination key. The main thread validates ownership — the requesting extension must have contributed the source key, unless its publisher appears in product.trustedExtensionPublishers — then registers the migration with the internal ConfigurationMigrationRegistry, which handles user/workspace/folder scopes and language overrides automatically.

Also fixes a latent bug: ConfigurationMigrationRegistry.registerConfigurationMigrations was not firing its onDidRegisterConfigurationMigration event, so ConfigurationMigrationWorkbenchContribution never processed migrations registered after workbench init.

Usage:

import * as positron from 'positron';

positron.workspace.registerConfigurationMigrations([{
    key: 'positron.assistant.consoleActions.enable',
    migrateTo: 'console.assistantActions.enabled',
}]);

Release Notes

New Features

Bug Fixes

  • N/A

Validation Steps

  1. In a Positron extension, call positron.workspace.registerConfigurationMigrations([{ key: 'old.key', migrateTo: 'new.key' }])
  2. Set old.key to a non-default value in user settings
  3. Activate the extension and verify the value is copied to new.key and old.key is cleared
  4. Verify a non-trusted extension attempting to migrate a key it doesn't own logs a warning and is rejected (probably impractical as all our extensions are trusted)

Exposes a settings migration capability to extensions via the Positron
API. Extensions call `positron.workspace.registerConfigurationMigrations`
with an array of `{ key, migrateTo }` specs; the main thread validates
ownership (source extension must match, or publisher must be in
`trustedExtensionPublishers`) and delegates to the existing
`ConfigurationMigrationRegistry`, which handles all scopes and language
overrides automatically.

Also fixes a latent bug where `ConfigurationMigrationRegistry` fired no
event after registering migrations, so `ConfigurationMigrationWorkbenchContribution`
never processed dynamically-registered migrations.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown

E2E Tests 🚀
This PR will run tests tagged with: @:critical

Note

No feature tags detected. If this PR needs feature coverage, add the tag above and retrigger the workflow.

readme  valid tags  why these tags?

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown

PETE's assessment 🧪

Verdict: 🟢 Adequate -- Every substantive source change is covered: the Mocha test exercises all five ownership branches of $registerConfigurationMigrations, and the Vitest pins the event-firing bug fix.

What changed

  • Adds the positron.workspace.registerConfigurationMigrations extension API: ext-host stub forwards to the main thread, which validates source-key ownership (registered-owner, namespace-owner, or posit-trusted bypass) before registering migrations with the internal ConfigurationMigrationRegistry.
  • Fixes a latent bug in ConfigurationMigrationRegistry.registerConfigurationMigrations -- it now fires onDidRegisterConfigurationMigration so dynamically-registered migrations are processed.
  • Plus the .d.ts API declaration and protocol/ext-host plumbing (no independent logic).

Tests in this PR

  • Unit (Vitest/Mocha) ✅ (added mainThreadConfiguration.test.ts cases + configuration.vitest.ts)
  • Extension host ✅ (not applicable -- ownership logic lives in the main thread, testable without an activated ext host)
  • E2E (Playwright) ✅ (not applicable -- no user-visible rendered workflow; the API is invoked programmatically)

Existing coverage

The new tests are the primary coverage:

  • src/vs/workbench/api/test/browser/mainThreadConfiguration.test.ts -- covers owned-key acceptance + migrateFn value mapping, unowned-key rejection + warning, posit-publisher bypass, namespace ownership for an unregistered (renamed) key, and rejection of an unregistered key outside the namespace. This maps 1:1 to every branch in the approved.filter(...) logic.
  • src/vs/workbench/common/test/configuration.vitest.ts -- asserts onDidRegisterConfigurationMigration fires on registration, pinning the bug fix in place.

Suggested additions

None.

Deployment note (optional)

N/A -- the change is platform-agnostic main-thread/ext-host logic with no browser//electron-sandbox/ split, path/process/filesystem handling, or UIKind/isWeb branching.


PETE (Positron Extreme Test Experiment) - LLM-based test-coverage advisor, in pilot. Triggers on PR open and on /recheck-tests comments. Wrong verdict? Comment /recheck-tests (or /rePETE) on this PR to re-run. Please share feedback on how PETE performed here.

samclark2015 and others added 2 commits June 16, 2026 10:03
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes the IProductService dependency from registerConfigurationMigrations
and hardcodes 'posit' as the trusted publisher rather than reading
trustedExtensionPublishers from product.json.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@samclark2015 samclark2015 marked this pull request as ready for review June 17, 2026 15:15
@samclark2015

Copy link
Copy Markdown
Contributor Author

/repete

const configurationProperties = Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Configuration).getConfigurationProperties();
const normalizedExtensionId = extensionId.toLowerCase();
const publisher = normalizedExtensionId.split('.')[0];
const isTrusted = publisher === 'posit';

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.

Your PR description says this will be controlled by product.trustedExtensionPublishers, I'm curious what your reasoning was for removing that to hardcoding it? Can we also add rstudio as trusted - this is unfortunately still the publisher of the workbench extension.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had done this initially, but wound up hard-coding due to concerns about side effects. It looks like this will also suppress the “Do you trust publisher X?” for Posit & RStudio if we add this. That sounds reasonable to me, thoughts?

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.

That's a win!

Comment thread src/vs/workbench/api/browser/mainThreadConfiguration.ts
Comment thread src/positron-dts/positron.d.ts
- Guard migrateFn against overwriting a destination key that already has a
  user-set value at the same config target (use accessor() per codebase pattern)
- Detect policy conflicts at registration time: log per-key admin errors and
  show a single batched user notification when old keys are policy-enforced but
  new keys are not
- Update positron.d.ts JSDoc to document both behaviors

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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