diff --git a/apps/desktop/electron/desktop-plugins-root.test.ts b/apps/desktop/electron/desktop-plugins-root.test.ts index 49b7a17b45..bc8596157d 100644 --- a/apps/desktop/electron/desktop-plugins-root.test.ts +++ b/apps/desktop/electron/desktop-plugins-root.test.ts @@ -102,6 +102,54 @@ describe('reconcileUnifiedDesktopHalves', () => { expect(fs.existsSync(path.join(appRoot, 'media'))).toBe(false) }) + it.skipIf(process.platform === 'win32')( + 'materializes a SYMLINKED dev package and re-syncs on any byte change, even with an older mtime', + async () => { + // `ln -s ~/src/my-plugin ~/.hermes/plugins/` is how a plugin is developed in + // place; the Python loader follows it, but the reconcile skipped the link + // (Dirent.isDirectory() is false for a symlink) — the desktop half never loaded. + const home = makeHome() + const appRoot = path.join(home, 'desktop-plugins') + const checkout = path.join(home, 'src', 'media') + const source = path.join(checkout, 'desktop', 'plugin.js') + write(source, 'v1') + fs.mkdirSync(path.join(home, 'plugins'), { recursive: true }) + fs.symlinkSync(checkout, path.join(home, 'plugins', 'media')) + + expect(await reconcileUnifiedDesktopHalves(home, appRoot)).toEqual([path.join(appRoot, 'media')]) + expect(fs.readFileSync(path.join(appRoot, 'media', 'plugin.js'), 'utf8')).toBe('v1') + const marker = JSON.parse(fs.readFileSync(path.join(appRoot, 'media', PACKAGE_MARKER), 'utf8')) + expect(marker).toMatchObject({ + linked: true, + package: 'media', + source: path.join(home, 'plugins', 'media', 'desktop') + }) + expect(await reconcileUnifiedDesktopHalves(home, appRoot)).toEqual([]) + + // An edit that lands an OLDER mtime (git stash pop, cp -p) still re-syncs. + write(source, 'v2') + const past = new Date(Date.now() - 3_600_000) + fs.utimesSync(source, past, past) + expect(await reconcileUnifiedDesktopHalves(home, appRoot)).toEqual([path.join(appRoot, 'media')]) + expect(fs.readFileSync(path.join(appRoot, 'media', 'plugin.js'), 'utf8')).toBe('v2') + } + ) + + it('keeps the mtime gate for an installed (non-linked) package', async () => { + const home = makeHome() + const appRoot = path.join(home, 'desktop-plugins') + const source = path.join(home, 'plugins', 'media', 'desktop', 'plugin.js') + write(source, 'v1') + await reconcileUnifiedDesktopHalves(home, appRoot) + const marker = JSON.parse(fs.readFileSync(path.join(appRoot, 'media', PACKAGE_MARKER), 'utf8')) + expect(marker.linked).toBeUndefined() + + write(source, 'v2') + const past = new Date(Date.now() - 3_600_000) + fs.utimesSync(source, past, past) + expect(await reconcileUnifiedDesktopHalves(home, appRoot)).toEqual([]) + }) + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( 'skips a package the app cannot read and still materializes its siblings', async () => { diff --git a/apps/desktop/electron/desktop-plugins-root.ts b/apps/desktop/electron/desktop-plugins-root.ts index 7b85676c63..e4dcc213bd 100644 --- a/apps/desktop/electron/desktop-plugins-root.ts +++ b/apps/desktop/electron/desktop-plugins-root.ts @@ -29,6 +29,10 @@ export interface DesktopHalfMarker { source: string /** mtimeMs of the source `plugin.js` at copy time; a newer source re-copies. */ sourceMtimeMs: number + /** The package folder is a symlink (a dev checkout linked into `plugins/`): + * the copy re-syncs on ANY byte difference and the renderer watches the + * source, so saving the checkout hot-reloads the pane. */ + linked?: boolean /** Where the PACKAGE came from, so "Install here" can install its agent half * into another profile: the catalog sidecar's repo/sha, else the git remote. */ repo?: string @@ -78,11 +82,30 @@ export async function ensureDir(dir: string): Promise { return dir } -async function listDirs(dir: string): Promise { +async function listDirs(dir: string, followLinks = false): Promise { try { const entries = await fs.promises.readdir(dir, { withFileTypes: true }) - return entries.filter(entry => entry.isDirectory()).map(entry => entry.name) + // `Dirent.isDirectory()` is false for a symlink. A package root follows + // links to directories: `ln -s ~/src/my-plugin ~/.hermes/plugins/` is how a + // plugin is developed in place, and the Python loader already follows it. + const linkedDir = async (name: string) => { + try { + return (await fs.promises.stat(path.join(dir, name))).isDirectory() + } catch { + return false + } + } + + const names: string[] = [] + + for (const entry of entries) { + if (entry.isDirectory() || (followLinks && entry.isSymbolicLink() && (await linkedDir(entry.name)))) { + names.push(entry.name) + } + } + + return names } catch { return [] } @@ -194,11 +217,13 @@ export async function materializeDesktopHalf( const target = path.join(appRoot, packageName) const existing = await readMarker(target) + const linked = (await fs.promises.lstat(packageDir)).isSymbolicLink() const marker: DesktopHalfMarker = { package: packageName, source: sourceDir, sourceMtimeMs: stat.mtimeMs, + ...(linked ? { linked } : {}), ...(await packageOrigin(packageDir)) } @@ -229,7 +254,15 @@ export async function materializeDesktopHalf( } } - if (existing && existing.source === sourceDir && existing.sourceMtimeMs >= stat.mtimeMs) { + // An installed package re-copies when its source is newer. A linked dev + // checkout compares bytes: mtime alone misses a `cp -p` / `git stash pop` + // that lands an older timestamp, and the copy would stay stale silently. + if ( + existing && + existing.source === sourceDir && + Boolean(existing.linked) === linked && + (linked ? await sameFile(path.join(target, 'plugin.js'), entry) : existing.sourceMtimeMs >= stat.mtimeMs) + ) { return null } } @@ -305,7 +338,7 @@ export async function reconcileUnifiedDesktopHalves(hermesHome: string, appRoot: for (const home of await localHomes(hermesHome)) { const pluginsRoot = path.join(home, 'plugins') - for (const name of await listDirs(pluginsRoot)) { + for (const name of await listDirs(pluginsRoot, true)) { if (seen.has(name)) { continue } diff --git a/apps/desktop/src/contrib/runtime-loader.test.ts b/apps/desktop/src/contrib/runtime-loader.test.ts index cd3a62f330..c177c9c35a 100644 --- a/apps/desktop/src/contrib/runtime-loader.test.ts +++ b/apps/desktop/src/contrib/runtime-loader.test.ts @@ -1187,3 +1187,95 @@ describe('manual "Reload desktop plugins" (#91503)', () => { } }) }) + +describe('linked dev package: the source checkout drives the desktop half', () => { + it('watches the SOURCE plugin.js and a save there re-syncs the copy and reloads it', async () => { + const root = '/local/.hermes/desktop-plugins' + const sourceDir = '/local/.hermes/plugins/devpkg/desktop' + desktopPluginsRoot.mockResolvedValue(root) + readDir.mockImplementation(async dir => { + if (dir === root) { + return { entries: [{ isDirectory: true, name: 'devpkg', path: `${root}/devpkg` }] } + } + + if (dir === `${root}/devpkg`) { + return { + entries: [ + { isDirectory: false, name: '.hermes-package.json', path: `${root}/devpkg/.hermes-package.json` }, + { isDirectory: false, name: 'plugin.js', path: `${root}/devpkg/plugin.js` } + ] + } + } + + return { entries: [] } + }) + + const registerV1 = vi.fn() + const registerV2 = vi.fn() + Object.assign(globalThis, { __devV1: registerV1, __devV2: registerV2 }) + let copy = 'export default { id: "devpkg", register: globalThis.__devV1 }' + readFileText.mockImplementation(async file => + file.endsWith('.hermes-package.json') + ? { text: JSON.stringify({ linked: true, package: 'devpkg', source: sourceDir, sourceMtimeMs: 1 }) } + : { text: copy } + ) + watchPreviewFile.mockImplementation(async file => ({ id: `w:${file}` })) + + // Electron's reconcile is the one writer of the copy: it lands the new bytes. + const reconcileDesktopPlugins = vi.fn(async () => { + copy = 'export default { id: "devpkg", register: globalThis.__devV2 }' + + return [`${root}/devpkg`] + }) + + Object.assign((window as unknown as { hermesDesktop: object }).hermesDesktop, { reconcileDesktopPlugins }) + + const createObjectURL = vi + .spyOn(URL, 'createObjectURL') + .mockImplementation( + blob => + `data:text/javascript;base64,${Buffer.from((blob as unknown as { parts: string[] }).parts.join('')).toString('base64')}` + ) + + const revokeObjectURL = vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => undefined) + const RealBlob = globalThis.Blob + vi.stubGlobal( + 'Blob', + class { + parts: string[] + constructor(parts: string[]) { + this.parts = parts + } + } + ) + + // watchRuntimePlugins is once-per-module and an earlier test already ran it; + // a fresh module graph gives this test its own change listener. + vi.resetModules() + const loader = await import('./runtime-loader') + const store = await import('./plugins-store') + + try { + loader.watchRuntimePlugins() + await vi.waitFor(() => expect(watchPreviewFile).toHaveBeenCalledWith(`${sourceDir}/plugin.js`)) + await store.setPluginEnabled('devpkg', true) + expect(registerV1).toHaveBeenCalledTimes(1) + + // The developer saves the checkout, not the copy. + const onChange = onPreviewFileChanged.mock.calls[0][0] as (event: { id: string }) => void + onChange({ id: `w:${sourceDir}/plugin.js` }) + + await vi.waitFor(() => expect(registerV2).toHaveBeenCalledTimes(1)) + expect(reconcileDesktopPlugins).toHaveBeenCalledTimes(1) + // The replaced copy's watch is re-bound to the new inode. + expect(stopPreviewFileWatch).toHaveBeenCalledWith(`w:${root}/devpkg/plugin.js`) + } finally { + createObjectURL.mockRestore() + revokeObjectURL.mockRestore() + vi.stubGlobal('Blob', RealBlob) + loader.unloadRuntimePlugin('devpkg') + delete (globalThis as unknown as { __devV1?: unknown }).__devV1 + delete (globalThis as unknown as { __devV2?: unknown }).__devV2 + } + }) +}) diff --git a/apps/desktop/src/contrib/runtime-loader.ts b/apps/desktop/src/contrib/runtime-loader.ts index 9b29471533..f9c4b55025 100644 --- a/apps/desktop/src/contrib/runtime-loader.ts +++ b/apps/desktop/src/contrib/runtime-loader.ts @@ -587,6 +587,8 @@ async function diskRoots(): Promise { const PACKAGE_MARKER = '.hermes-package.json' interface PackageMarker { + /** The package is a symlinked dev checkout; `source` is its `desktop/` dir. */ + linkedSource?: string origin?: { catalogName?: string; repo?: string; sha?: string } package: string } @@ -608,9 +610,11 @@ async function readPackageMarker(desktop: Window['hermesDesktop'], folder: strin const parsed = JSON.parse(read.text) as { catalogName?: string + linked?: boolean package?: string repo?: string sha?: string + source?: string } if (!parsed.package) { @@ -618,6 +622,7 @@ async function readPackageMarker(desktop: Window['hermesDesktop'], folder: strin } return { + linkedSource: parsed.linked && parsed.source ? parsed.source : undefined, origin: parsed.repo ? { catalogName: parsed.catalogName, repo: parsed.repo, sha: parsed.sha } : undefined, package: parsed.package } @@ -637,6 +642,10 @@ interface DiskPlugin { id: null | string /** Origin label (folder name) — the toast/inventory name for load errors. */ origin: string + /** Linked dev package only: the SOURCE `desktop/plugin.js` this copy came from. */ + sourceFile?: string + /** Watch on `sourceFile` — a save re-syncs the copy, then reloads it. */ + sourceWatchId?: null | string watchId: null | string } @@ -802,6 +811,49 @@ async function watchDiskPluginFile(desktop: NonNullable } } +/** Watch a linked dev package's SOURCE `plugin.js`. Without it the developer + * edits their checkout while the app keeps running the copy in + * `desktop-plugins/` — the copy is only refreshed on Rescan or restart. */ +async function watchDiskPluginSource(desktop: NonNullable, record: DiskPlugin): Promise { + if (!record.sourceFile || record.sourceWatchId) { + return + } + + try { + const watch = await desktop.watchPreviewFile(record.sourceFile) + + record.sourceWatchId = isReadFileErrorResult(watch) ? null : watch.id + } catch { + record.sourceWatchId = null + } +} + +/** A linked source changed: copy it over the app-root half (Electron's + * reconcile — the one writer of that folder), then reload every copy that + * pass replaced and re-bind its watch, which the folder swap orphaned. */ +async function resyncLinkedDiskPlugins(): Promise { + const desktop = window.hermesDesktop + + if (!desktop?.reconcileDesktopPlugins) { + return + } + + const touched = new Set((await desktop.reconcileDesktopPlugins().catch(() => [])) ?? []) + + for (const record of disk.values()) { + // The folder of `//plugin.js`, either separator (Windows paths). + if (!touched.has(record.file.replace(/[\\/][^\\/]*$/, ''))) { + continue + } + + if (await loadDiskPlugin(record)) { + await watchDiskPluginFile(desktop, record) + } else { + void scanDiskPlugins() + } + } +} + /** Reconcile the disk root with the inventory. `reloadKnown` (the manual * "Reload desktop plugins" command) also re-reads every already-known entry * file: an installer that atomically replaces a plugin folder keeps the @@ -875,6 +927,8 @@ async function scanDiskPlugins(reloadKnown = false): Promise { origin: dir.name, packageName: marker?.package, packageOrigin: marker?.origin, + sourceFile: marker?.linkedSource ? `${marker.linkedSource}/plugin.js` : undefined, + sourceWatchId: null, watchId: null } @@ -887,6 +941,7 @@ async function scanDiskPlugins(reloadKnown = false): Promise { } await watchDiskPluginFile(desktop, record) + await watchDiskPluginSource(desktop, record) } } @@ -919,6 +974,10 @@ function retireDiskPlugin(file: string, record: DiskPlugin): void { void window.hermesDesktop?.stopPreviewFileWatch(record.watchId) } + if (record.sourceWatchId) { + void window.hermesDesktop?.stopPreviewFileWatch(record.sourceWatchId) + } + disk.delete(file) } @@ -985,6 +1044,12 @@ export function watchRuntimePlugins(): void { } for (const record of disk.values()) { + if (record.sourceWatchId === id) { + void resyncLinkedDiskPlugins() + + return + } + if (record.watchId === id) { void loadDiskPlugin(record).then(readable => { if (!readable) { diff --git a/hermes_cli/plugin_dev.py b/hermes_cli/plugin_dev.py index 456c87c46e..7151703e76 100644 --- a/hermes_cli/plugin_dev.py +++ b/hermes_cli/plugin_dev.py @@ -4,6 +4,7 @@ check routes through the real runtime contracts instead of a parallel scanner."" from __future__ import annotations import inspect +import json import os import shutil import socket @@ -332,6 +333,40 @@ def _check_manifest_v2(report: "DoctorReport", manifest: Any) -> None: report.warning(f"config_schema key {skey!r} declares unknown type {stype!r}") +def _check_desktop_half_copy(report: DoctorReport, path: Path) -> None: + """Warn when the Desktop app runs a stale copy of this package's ``desktop/plugin.js``. + + Electron copies a unified package's desktop half to ``/desktop-plugins//`` and loads + THAT copy, so editing the package in place changes nothing on screen until the copy is refreshed. + The copy carries a ``.hermes-package.json`` marker whose ``source`` names the folder it came from. + """ + source = path / "desktop" / "plugin.js" + if not source.is_file(): + return + from hermes_constants import get_default_hermes_root + + app_root = get_default_hermes_root() / "desktop-plugins" + try: + copies = sorted(app_root.glob("*/.hermes-package.json")) + except OSError: + return + for marker_file in copies: + try: + marker = json.loads(marker_file.read_text(encoding="utf-8-sig")) + if Path(marker["source"]).resolve() != source.parent.resolve(): + continue + if (marker_file.parent / "plugin.js").read_bytes() == source.read_bytes(): + return + except (OSError, ValueError, KeyError, TypeError): + continue + report.warning( + f"Desktop runs a stale copy of desktop/plugin.js ({marker_file.parent}); your edits are not " + "loaded. Refresh it with Capabilities → Plugins → Rescan in the Desktop app (or restart it). " + "To develop in place, symlink the checkout into plugins/ instead: the Desktop then re-syncs " + "the copy on every save.") + return + + def doctor_plugin(target: str | os.PathLike[str] | None = None) -> DoctorReport: """Validate one plugin through Hermes' real scanner and registration path.""" try: @@ -392,6 +427,8 @@ def doctor_plugin(target: str | os.PathLike[str] | None = None) -> DoctorReport: report.error(str(exc)) except Exception as exc: report.error(f"unexpected validation failure: {type(exc).__name__}: {exc}") + # Outside the runtime sandbox: it swaps HERMES_HOME for a temp dir, and the copy lives in the real one. + _check_desktop_half_copy(report, path) return report diff --git a/tests/hermes_cli/test_plugin_doctor_desktop_copy.py b/tests/hermes_cli/test_plugin_doctor_desktop_copy.py new file mode 100644 index 0000000000..04cb45b6d2 --- /dev/null +++ b/tests/hermes_cli/test_plugin_doctor_desktop_copy.py @@ -0,0 +1,60 @@ +"""``hermes plugins doctor`` warns when Desktop runs a stale copy of a unified package's desktop half.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + + +def _plugin(root: Path) -> Path: + plugin = root / "plugins" / "uni" + (plugin / "desktop").mkdir(parents=True) + (plugin / "plugin.yaml").write_text("name: uni\nversion: 0.1.0\n", encoding="utf-8") + (plugin / "__init__.py").write_text("def register(ctx):\n pass\n", encoding="utf-8") + (plugin / "desktop" / "plugin.js").write_text("export default { id: 'uni' } // v1\n", encoding="utf-8") + return plugin + + +def _copy(home: Path, plugin: Path) -> Path: + copy = home / "desktop-plugins" / "uni" + copy.mkdir(parents=True) + (copy / "plugin.js").write_bytes((plugin / "desktop" / "plugin.js").read_bytes()) + (copy / ".hermes-package.json").write_text( + json.dumps({"package": "uni", "source": str(plugin / "desktop"), "sourceMtimeMs": 1}), encoding="utf-8") + return copy + + +def _stale_warnings(report) -> list[str]: + return [f.message for f in report.findings if "stale copy" in f.message] + + +@pytest.fixture +def home(tmp_path, monkeypatch) -> Path: + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + return home + + +def test_doctor_warns_when_the_desktop_copy_is_stale(home: Path) -> None: + from hermes_cli.plugin_dev import doctor_plugin + + plugin = _plugin(home) + copy = _copy(home, plugin) + assert _stale_warnings(doctor_plugin(plugin)) == [] + + (plugin / "desktop" / "plugin.js").write_text("export default { id: 'uni' } // v2\n", encoding="utf-8") + report = doctor_plugin(plugin) + + [warning] = _stale_warnings(report) + assert str(copy) in warning + assert "Rescan" in warning + assert report.ok # a warning, never a CI failure + + +def test_doctor_is_silent_without_a_desktop_copy(home: Path) -> None: + from hermes_cli.plugin_dev import doctor_plugin + + assert _stale_warnings(doctor_plugin(_plugin(home))) == [] diff --git a/website/docs/developer-guide/desktop-plugin-sdk.md b/website/docs/developer-guide/desktop-plugin-sdk.md index c853f84c0a..4277c1fe8e 100644 --- a/website/docs/developer-guide/desktop-plugin-sdk.md +++ b/website/docs/developer-guide/desktop-plugin-sdk.md @@ -1543,6 +1543,32 @@ such damage and replaced on the next **Rescan**, while a marker-less folder that *does* hold a `plugin.js` is a standalone plugin you installed by hand and is never overwritten. +#### Developing a unified package + +The app loads the **copy** in `desktop-plugins//`, not your package. An +installed package (catalog, Git URL, `file://` path) is refreshed only when its +source `plugin.js` is newer than the copy and something asks — **Rescan** in +**Capabilities → Plugins**, an install/update through the app, or a restart — +so editing `~/.hermes/plugins//desktop/plugin.js` in place does nothing +on screen until then. Develop from a checkout linked into `plugins/` instead: + +```bash +ln -s ~/src/my-plugin ~/.hermes/plugins/my-plugin +``` + +A linked package's copy is marked `"linked": true`. The app watches the +checkout's `desktop/plugin.js`; every save re-syncs the copy (by content, not +mtime, so `git stash pop` counts) and hot-reloads the plugin. Catalog and Git +installs keep the mtime rule. + +`hermes plugins doctor ` warns when the copy differs from your +source and names the copy it found: + +```text +WARN: Desktop runs a stale copy of desktop/plugin.js (~/.hermes/desktop-plugins/my-plugin); +your edits are not loaded. Refresh it with Capabilities → Plugins → Rescan … +``` + Two enable switches still apply, on purpose, and both default to **off**: the desktop half ships opt-in — it inventories in **Capabilities → Plugins** but stays disabled until the user toggles it — matching the Python half's diff --git a/website/docs/developer-guide/plugins/index.md b/website/docs/developer-guide/plugins/index.md index 80751ac727..aff9cf1cab 100644 --- a/website/docs/developer-guide/plugins/index.md +++ b/website/docs/developer-guide/plugins/index.md @@ -251,7 +251,10 @@ cd ~/.hermes/plugins/calculator manifest parser, namespaced import, `register(ctx)`, hook registry, and tool registry used by Hermes itself. It reports invalid hook names, callbacks that do not accept `**kwargs`, registration failures, and drift between declared and -registered tools/hooks. Pass `--ci` to exit non-zero on an error: +registered tools/hooks. For a package with a `desktop/plugin.js`, it also warns +when the Desktop app is running a stale copy of it (see +[Developing a unified package](../desktop-plugin-sdk.md#developing-a-unified-package)). +Pass `--ci` to exit non-zero on an error: ```bash hermes plugins doctor . --ci