diff --git a/README.md b/README.md index c506ad5..6224998 100644 --- a/README.md +++ b/README.md @@ -164,7 +164,7 @@ CSP, esbuild options, timing constraints) is in | Group | Fields | | --- | --- | -| Identity | `key`, `displayName`, `settingsNamespace`, `customEditorViewType`, `glspClientId`, `glspClientName` | +| Identity | `key`, `displayName`, `settingsNamespace`, `customEditorViewType`, `glspClientId`, `glspClientName`, `sourceExtensions` (which files this diagram is a view of) | | Commands | `commands` (21 consumer-owned command ids), `operationKinds` (port create/delete kind strings) | | Model & edits | `modelSource` factory, `edits` (`'read-only'` or a strategy with operation modules), `serverModules`, `serverDiagramModule` (library mode only), `storageOptions`, `supportsElementCreation` (`false` empties the creation palette) | | Client | `clientBehavior` (neutral capability flags injected into the webview), `clientAssets` (custom webview bundle — data only), `onWebviewMessage` (inbound message hook) | @@ -172,6 +172,9 @@ CSP, esbuild options, timing constraints) is in Everything optional degrades gracefully: no `chat` → no chat backend, no `runDriver` → no run/stop commands or live glow, no `clientAssets` → the +stock webview bundle, no `sourceExtensions` → the platform filters by +extension nowhere and words its "please open a source file" messages without +naming one. stock webview bundle, no `supportsElementCreation` → the full creation palette, since a diagram that cannot be authored has to say so. diff --git a/packages/extension-core/src/api.ts b/packages/extension-core/src/api.ts index 9dfb9a6..8934fb8 100644 --- a/packages/extension-core/src/api.ts +++ b/packages/extension-core/src/api.ts @@ -248,6 +248,26 @@ export interface DiagramProfile { glspClientName: string; /** Consumer-owned command ids. */ commands: DiagramCommandIds; + /** + * File extensions this product's diagram is a view of — lower-case, leading + * dot: `['.foo', '.bar']`. + * + * Everywhere the platform has to decide whether a URI is one of this + * product's sources — the open-diagram commands, the rename command's + * "which file is active" lookup, the editor provider's save and on-disk + * watchers — it asks this list. User-facing messages are worded from it too, + * so a consumer is told to open a `.foo` file rather than whatever the + * platform was written against. + * + * Declaring nothing is legal and deliberately permissive: the platform then + * filters by extension nowhere, and phrases those messages without naming an + * extension at all. The core cannot know how a product names its files, and a + * guess would fail silently — commands refusing every file, indistinguishable + * from a workspace with no sources. An unrecognised file instead reaches + * {@link DiagramProfile.canOpenSource}, which can refuse it for a reason the + * product actually knows. + */ + sourceExtensions?: string[]; /** Port operation-kind strings injected into the diagram client. */ operationKinds?: DiagramOperationKinds; /** Neutral behavior flags forwarded into the diagram webview. */ diff --git a/packages/extension-core/src/extension/diagram/diagram-editor-provider.ts b/packages/extension-core/src/extension/diagram/diagram-editor-provider.ts index 1c0906b..6a76c64 100644 --- a/packages/extension-core/src/extension/diagram/diagram-editor-provider.ts +++ b/packages/extension-core/src/extension/diagram/diagram-editor-provider.ts @@ -12,6 +12,7 @@ import { statSync } from 'node:fs'; import { WORKFLOW_DIAGRAM_TYPE } from '@dialogram/shared'; import { type DiagramProfile } from '../../api'; import { normalizeSourceUriKey } from './uri-keys'; +import { matchesSourceExtension, sourceWatchGlobs } from './source-extensions'; const RUN_ID_ARG = 'wf:runId'; @@ -79,9 +80,9 @@ export class WorkflowEditorProvider extends GlspEditorProvider { */ private changeDebounceTimers = new Map(); - /** Debounce timer for external (on-disk) .py changes picked up by the watcher. */ + /** Debounce timer for external (on-disk) source changes picked up by the watcher. */ private externalRefreshTimer?: NodeJS.Timeout; - /** When each .py was last saved through the editor, to skip the watcher's + /** When each source file was last saved through the editor, to skip the watcher's * duplicate refresh for an in-editor save (the save handler already refreshed). */ private lastSavedAt = new Map(); @@ -98,11 +99,15 @@ export class WorkflowEditorProvider extends GlspEditorProvider { * so without this filter unrelated generated files thrash the diagram. * * Incident this guards against: in a CMake-based workspace, the build tool - * regenerated `build/test/lit.site.cfg.py` on every configure, which the watcher - * treated as a cross-file import edit and force-reloaded the open diagram + * regenerated a source file under `build/test/` on every configure, which the + * watcher treated as a cross-file import edit and force-reloaded the open diagram * 4+ times in seconds — each reload paying 1-2.3s of source-CLI spawn (`acquire`) * plus a full webview re-render. A file whose path contains any of these * segments is never a legitimate reload trigger. + * + * It matters more now that the watcher can be told to watch everything: a + * profile that declares no source extensions gets `**\/*`, and this filter is + * what keeps a build tree from reloading diagrams it has nothing to do with. */ private static readonly ARTIFACT_PATH_SEGMENTS: ReadonlySet = new Set([ 'build', 'dist', 'out', 'wf-out', 'node_modules', '.git', '__pycache__', '.venv', 'venv' @@ -111,8 +116,8 @@ export class WorkflowEditorProvider extends GlspEditorProvider { /** * True when the URI lives under an artifact / irrelevant directory (see * {@link ARTIFACT_PATH_SEGMENTS}). Segment-based, so it matches at any depth - * (`.../build/test/lit.site.cfg.py`) without matching filenames that merely - * contain a segment word (`build_config.py`). + * (`.../build/test/generated-config`) without matching filenames that merely + * contain a segment word (`build_config`). */ private isUnderArtifactDir(uri: vscode.Uri): boolean { const segments = uri.path.split('/'); @@ -157,15 +162,24 @@ export class WorkflowEditorProvider extends GlspEditorProvider { }) ); - // Catch EXTERNAL .py writes — the chat agent editing via the MCP edit backend, - // git, a formatter, etc. Those bypass the editor (no onDidChange/Save), so - // without this the diagram wouldn't reflect agent-added nodes until a manual + // Catch EXTERNAL source writes — the chat agent editing via the MCP edit + // backend, git, a formatter, etc. Those bypass the editor (no onDidChange/Save), + // so without this the diagram wouldn't reflect agent-added nodes until a manual // save/reopen. onDidChangeTextDocument only fires for in-editor edits. - const sourceWatcher = vscode.workspace.createFileSystemWatcher('**/*.py'); + // + // What to watch comes from the profile, in that order of specificity: + // `watch.globs` is the exact answer when a product has one (it can watch + // more than its own sources — a manifest, a generated index); otherwise the + // globs are derived from the declared extensions; otherwise everything. + // Until this was wired the field existed and nothing read it, and the + // watcher was pinned to one product's extension. const onExternal = (uri: vscode.Uri) => this.handleExternalFileChange(uri); - sourceWatcher.onDidChange(onExternal); - sourceWatcher.onDidCreate(onExternal); - extensionContext.subscriptions.push(sourceWatcher); + for (const glob of profile.watch?.globs ?? sourceWatchGlobs(profile.sourceExtensions)) { + const sourceWatcher = vscode.workspace.createFileSystemWatcher(glob); + sourceWatcher.onDidChange(onExternal); + sourceWatcher.onDidCreate(onExternal); + extensionContext.subscriptions.push(sourceWatcher); + } } /** @@ -417,7 +431,7 @@ export class WorkflowEditorProvider extends GlspEditorProvider { */ protected handleDocumentSave(document: vscode.TextDocument): void { // Only react to source-file saves. - if (!document.uri.path.endsWith('.py')) { + if (!matchesSourceExtension(document.uri.path, this.profile.sourceExtensions)) { return; } @@ -445,7 +459,7 @@ export class WorkflowEditorProvider extends GlspEditorProvider { } /** - * React to a .py changing ON DISK from outside the editor — the chat agent + * React to a source file changing ON DISK from outside the editor — the chat agent * writing via the MCP edit backend, git, a formatter, etc. In-editor edits go * through handleDocumentChange/handleDocumentSave instead, so here we: * - skip files open with unsaved edits (the editor owns those), and @@ -454,15 +468,15 @@ export class WorkflowEditorProvider extends GlspEditorProvider { * save — refresh every open diagram. Debounced to coalesce edit bursts. */ protected handleExternalFileChange(uri: vscode.Uri): void { - if (!uri.path.endsWith('.py') || this.uriToClientId.size === 0) { + if (!matchesSourceExtension(uri.path, this.profile.sourceExtensions) || this.uriToClientId.size === 0) { return; } const canonical = this.canonicalizeUriString(uri); // Always reload when the changed file IS an open diagram's own source. For - // any OTHER watched .py, keep the cross-file-import reload but skip build + // any OTHER watched file, keep the cross-file-import reload but skip build // artifacts / irrelevant trees (see isUnderArtifactDir) so generated files - // like CMake's build/test/lit.site.cfg.py don't thrash open diagrams. + // under a build tree don't thrash open diagrams. const isOwnSource = this.uriToClientId.has(canonical); if (!isOwnSource && this.isUnderArtifactDir(uri)) { return; diff --git a/packages/extension-core/src/extension/diagram/glsp-activation.ts b/packages/extension-core/src/extension/diagram/glsp-activation.ts index d768259..02eb5b5 100644 --- a/packages/extension-core/src/extension/diagram/glsp-activation.ts +++ b/packages/extension-core/src/extension/diagram/glsp-activation.ts @@ -31,6 +31,7 @@ import type { WorkflowEditorProvider } from './diagram-editor-provider'; import { ExecutionOverlayRegistry } from './execution-overlay'; import { forwardExecutionOverlayEvents, type ExecutionOverlayWebviewSink } from './execution-overlay-bridge'; import { readRequestedNetworkName, resolveDiagramOpenTarget, type DiagramOpenTargetArg } from './open-diagram-target'; +import { matchesSourceExtension, sourceFileNoun } from './source-extensions'; import { normalizeSourceUriKey } from './uri-keys'; import { executeViewerCommand, executeViewerOpen, executeViewerReveal } from './viewer-actions'; import { decideDiagramOpen } from './diagram-open-decision'; @@ -1005,13 +1006,21 @@ function registerCalDiagramCommands( connector.dispatchAction(action); }; + // The noun every "which file did you mean?" message in this function uses. + // Built from the profile's declaration so the core never writes an extension + // of its own into text a user reads; plain "source file" when none is declared. + const sourceNoun = sourceFileNoun(profile.sourceExtensions); + // A type predicate, so the callers that go on to use the URI still narrow it. + const isProfileSource = (uri: vscode.Uri | undefined): uri is vscode.Uri => + !!uri && matchesSourceExtension(uri.fsPath, profile.sourceExtensions); + const getActiveWorkflowUri = (): vscode.Uri | undefined => { const diagramUri = getActiveWorkflowDiagramUri(); - if (diagramUri?.fsPath?.endsWith('.py')) { + if (isProfileSource(diagramUri)) { return diagramUri; } const activeEditorUri = vscode.window.activeTextEditor?.document.uri; - if (activeEditorUri?.fsPath?.endsWith('.py')) { + if (isProfileSource(activeEditorUri)) { return activeEditorUri; } return undefined; @@ -1100,10 +1109,11 @@ function registerCalDiagramCommands( const sourceUri = await resolveDiagramOpenTarget(arg, { getActiveWorkflowUri, openTextDocument: vscode.workspace.openTextDocument, - workspaceRoot: vscode.workspace.workspaceFolders?.[0]?.uri.fsPath + workspaceRoot: vscode.workspace.workspaceFolders?.[0]?.uri.fsPath, + sourceExtensions: profile.sourceExtensions }); if (!sourceUri) { - vscode.window.showWarningMessage('Please open a .py file first, or pass a .py path/URI to the command.'); + vscode.window.showWarningMessage(`Please open a ${sourceNoun} first, or pass its path/URI to the command.`); return; } const requestedNetwork = readRequestedNetworkName(arg); @@ -1139,10 +1149,11 @@ function registerCalDiagramCommands( const sourceUri = await resolveDiagramOpenTarget(arg, { getActiveWorkflowUri, openTextDocument: vscode.workspace.openTextDocument, - workspaceRoot: vscode.workspace.workspaceFolders?.[0]?.uri.fsPath + workspaceRoot: vscode.workspace.workspaceFolders?.[0]?.uri.fsPath, + sourceExtensions: profile.sourceExtensions }); if (!sourceUri) { - vscode.window.showWarningMessage('Please open a .py file first, or pass a .py path/URI to the command.'); + vscode.window.showWarningMessage(`Please open a ${sourceNoun} first, or pass its path/URI to the command.`); return; } const requestedNetwork = readRequestedNetworkName(arg); @@ -1195,8 +1206,8 @@ function registerCalDiagramCommands( profile.commands.renameEntityByName, async (args?: { oldName?: string; newName?: string; sourceUri?: string }) => { const sourceUri = args?.sourceUri ? vscode.Uri.parse(args.sourceUri) : getActiveWorkflowUri(); - if (!sourceUri || !sourceUri.fsPath.endsWith('.py')) { - const message = 'No active .py source found. Focus a workflow diagram or .py editor and try again.'; + if (!isProfileSource(sourceUri)) { + const message = `No active ${sourceNoun} found. Focus a diagram or an editor on one and try again.`; void vscode.window.showWarningMessage(message); return { ok: false, message }; } diff --git a/packages/extension-core/src/extension/diagram/open-diagram-target.ts b/packages/extension-core/src/extension/diagram/open-diagram-target.ts index cf7a728..a3fa4df 100644 --- a/packages/extension-core/src/extension/diagram/open-diagram-target.ts +++ b/packages/extension-core/src/extension/diagram/open-diagram-target.ts @@ -1,5 +1,6 @@ import * as path from 'node:path'; import * as vscode from 'vscode'; +import { matchesSourceExtension } from './source-extensions'; export type DiagramOpenTargetArg = vscode.Uri | string | { sourceUri?: string; @@ -38,6 +39,9 @@ export type ResolveDiagramOpenTargetOptions = { getActiveWorkflowUri: () => vscode.Uri | undefined; openTextDocument: (uri: vscode.Uri) => Thenable<{ uri: vscode.Uri }>; workspaceRoot?: string; + /** The profile's declared source extensions; absent/empty accepts any file + * (see `source-extensions.ts` for why the default is permissive). */ + sourceExtensions?: readonly string[]; }; function isUriString(value: string): boolean { @@ -108,7 +112,7 @@ export async function resolveDiagramOpenTarget( const candidate = parseTargetCandidate(arg, options.workspaceRoot); const resolved = candidate ?? options.getActiveWorkflowUri(); - if (!resolved || resolved.scheme !== 'file' || !resolved.fsPath.toLowerCase().endsWith('.py')) { + if (!resolved || resolved.scheme !== 'file' || !matchesSourceExtension(resolved.fsPath, options.sourceExtensions)) { return undefined; } diff --git a/packages/extension-core/src/extension/diagram/source-extensions.ts b/packages/extension-core/src/extension/diagram/source-extensions.ts new file mode 100644 index 0000000..ba9779d --- /dev/null +++ b/packages/extension-core/src/extension/diagram/source-extensions.ts @@ -0,0 +1,110 @@ +/** + * Which files a profile's diagram is a view of. + * + * The core used to answer that with one product's file extension, written out as + * a literal: six path tests plus a file-system watcher glob. That is a product's + * file naming compiled into the platform. A consumer whose sources end in + * anything else got a diagram command that silently refused every file it was + * given, and a warning telling it to open a file of a kind it does not have — + * text it could not change, naming an extension it does not use. + * + * A profile declares its own extensions instead ({@link DiagramProfile.sourceExtensions}), + * and every one of those sites asks here. The platform still names none. + * + * The permissive default is the deliberate half of this. A profile that declares + * nothing gets no filtering at all, rather than a guess: the core cannot know how + * a product names its files, and a wrong guess fails in the worst way available — + * the command does nothing, says nothing useful, and looks like the file is + * missing. Letting an unrecognised file through instead hands the decision to the + * profile's own `canOpenSource`, which can refuse it with a reason that means + * something. + */ + +/** + * Fold a declared list into the form the matcher compares against: lower-case, + * dot-prefixed, no blanks. + * + * The API asks for exactly that form, so this normally changes nothing. It runs + * anyway because the failure it prevents is invisible: `'foo'` without the dot, + * or `'.FOO'` from a product whose files are upper-case, would match no file at + * all, and "matches nothing" is indistinguishable here from "the workspace has + * no sources". Accepting the near-miss costs one pass over a list of two. + */ +export function normalizeSourceExtensions( + extensions: readonly string[] | undefined +): string[] { + if (!extensions) { + return []; + } + const normalized: string[] = []; + for (const raw of extensions) { + const trimmed = raw.trim().toLowerCase(); + if (trimmed === '' || trimmed === '.') { + continue; + } + const withDot = trimmed.startsWith('.') ? trimmed : `.${trimmed}`; + if (!normalized.includes(withDot)) { + normalized.push(withDot); + } + } + return normalized; +} + +/** + * True when `filePath` is one of the profile's source files — or when the + * profile declared none, in which case every path qualifies (see the module + * note on why the default is permissive rather than restrictive). + */ +export function matchesSourceExtension( + filePath: string | undefined, + extensions: readonly string[] | undefined +): boolean { + const declared = normalizeSourceExtensions(extensions); + if (declared.length === 0) { + return true; + } + if (!filePath) { + return false; + } + const lowered = filePath.toLowerCase(); + return declared.some(extension => lowered.endsWith(extension)); +} + +/** + * The noun a user-facing message uses for the file the command wanted: `.foo + * file`, `.foo or .bar file`, `.foo, .bar or .baz file` — and plain `source + * file` when the profile declared nothing. + * + * Messages are built from this rather than written out, because a message that + * names an extension is a message the core cannot write: it would have to know + * the answer to the question this whole module exists to delegate. + */ +export function sourceFileNoun(extensions: readonly string[] | undefined): string { + const declared = normalizeSourceExtensions(extensions); + if (declared.length === 0) { + return 'source file'; + } + if (declared.length === 1) { + return `${declared[0]} file`; + } + const last = declared[declared.length - 1]; + return `${declared.slice(0, -1).join(', ')} or ${last} file`; +} + +/** + * Watcher globs for the profile's sources. + * + * With nothing declared this is `**\/*` — every file in the workspace, which is + * the honest reading of "the platform does not filter by extension", and the + * same trade the matcher makes. It is not as expensive as it looks: the handler + * ignores any path outside an open diagram's tree of interest, and a profile + * that cares about the watcher cost declares either `sourceExtensions` or its + * own `watch.globs`, both of which win over this fallback. + */ +export function sourceWatchGlobs(extensions: readonly string[] | undefined): string[] { + const declared = normalizeSourceExtensions(extensions); + if (declared.length === 0) { + return ['**/*']; + } + return declared.map(extension => `**/*${extension}`); +} diff --git a/packages/extension-core/test/profile-source-extensions.test.ts b/packages/extension-core/test/profile-source-extensions.test.ts new file mode 100644 index 0000000..936165e --- /dev/null +++ b/packages/extension-core/test/profile-source-extensions.test.ts @@ -0,0 +1,168 @@ +/** + * A product says what its source files are called; the platform stops guessing. + * + * The core answered "is this URI one of the product's sources?" by testing the path + * against one product's extension, written out as a literal — in the open-diagram + * commands, the rename command's active-file lookup, the editor provider's save + * handler and its on-disk watcher. For a consumer whose files end in anything else, + * every one of those refused every file, and the warning it got named an extension + * it does not use. + * + * These tests pin both halves of the contract: a declared list filters and words the + * messages, and NO declaration filters nothing — the permissive default, which is + * what the core owes a product whose naming it cannot know. + */ +import { describe, expect, it, vi } from 'vitest'; +import * as vscode from 'vscode'; +import type { DiagramProfile } from '../src/api'; +import { resolveDiagramOpenTarget } from '../src/extension/diagram/open-diagram-target'; +import { + matchesSourceExtension, + normalizeSourceExtensions, + sourceFileNoun, + sourceWatchGlobs +} from '../src/extension/diagram/source-extensions'; + +vi.mock('@eclipse-glsp/vscode-integration', () => ({ + GlspEditorProvider: class { + onDidChangeCustomDocument: unknown; + constructor(protected readonly glspVscodeConnector: any) { + this.onDidChangeCustomDocument = glspVscodeConnector?.onDidChangeCustomDocument; + } + }, + GlspVscodeConnector: class {} +})); + +const { WorkflowEditorProvider } = await import('../src/extension/diagram/diagram-editor-provider'); + +describe('matchesSourceExtension', () => { + it('accepts only the declared extensions', () => { + expect(matchesSourceExtension('/repo/graph.foo', ['.foo', '.bar'])).toBe(true); + expect(matchesSourceExtension('/repo/graph.bar', ['.foo', '.bar'])).toBe(true); + expect(matchesSourceExtension('/repo/graph.baz', ['.foo', '.bar'])).toBe(false); + }); + + it('accepts anything when a profile declares nothing', () => { + // The deliberate default. A core that refused what it did not recognise + // would fail as silence — the command does nothing and the file looks + // absent — instead of letting the profile refuse it for a real reason. + expect(matchesSourceExtension('/repo/graph.anything', undefined)).toBe(true); + expect(matchesSourceExtension('/repo/graph.anything', [])).toBe(true); + }); + + it('ignores case, so an upper-case file still matches', () => { + expect(matchesSourceExtension('/repo/GRAPH.FOO', ['.foo'])).toBe(true); + }); + + it('tolerates a declaration missing the dot rather than matching nothing', () => { + // A near-miss declaration would otherwise match no file at all, which + // looks exactly like a workspace with no sources in it. + expect(normalizeSourceExtensions(['foo', '.BAR', ' ', '.foo'])).toEqual(['.foo', '.bar']); + expect(matchesSourceExtension('/repo/graph.foo', ['foo'])).toBe(true); + }); +}); + +describe('sourceFileNoun', () => { + it('names the declared extensions, and nothing when none are declared', () => { + expect(sourceFileNoun(['.foo'])).toBe('.foo file'); + expect(sourceFileNoun(['.foo', '.bar'])).toBe('.foo or .bar file'); + expect(sourceFileNoun(['.foo', '.bar', '.baz'])).toBe('.foo, .bar or .baz file'); + expect(sourceFileNoun(undefined)).toBe('source file'); + }); +}); + +describe('sourceWatchGlobs', () => { + it('derives one glob per declared extension, and watches everything otherwise', () => { + expect(sourceWatchGlobs(['.foo', '.bar'])).toEqual(['**/*.foo', '**/*.bar']); + expect(sourceWatchGlobs(undefined)).toEqual(['**/*']); + }); +}); + +describe('resolveDiagramOpenTarget with declared source extensions', () => { + const openTextDocument = vi.fn(async (uri: vscode.Uri) => ({ uri })); + + it('opens a file whose extension the profile declared', async () => { + const target = '/repo/designs/pipeline.foo'; + const resolved = await resolveDiagramOpenTarget(target, { + getActiveWorkflowUri: () => undefined, + openTextDocument, + sourceExtensions: ['.foo'] + }); + + expect(resolved?.toString()).toBe(vscode.Uri.file(target).toString()); + }); + + it('refuses a file the profile did not declare', async () => { + const resolved = await resolveDiagramOpenTarget('/repo/designs/notes.txt', { + getActiveWorkflowUri: () => undefined, + openTextDocument, + sourceExtensions: ['.foo'] + }); + + expect(resolved).toBeUndefined(); + }); + + it('opens anything when the profile declared nothing', async () => { + // The control for the case above: the refusal has to come from the + // declaration, not from the platform having an opinion of its own. + const target = '/repo/designs/notes.txt'; + const resolved = await resolveDiagramOpenTarget(target, { + getActiveWorkflowUri: () => undefined, + openTextDocument + }); + + expect(resolved?.toString()).toBe(vscode.Uri.file(target).toString()); + }); +}); + +describe('WorkflowEditorProvider source watching', () => { + function build(profile: Partial): { globs: string[]; provider: any } { + const globs: string[] = []; + const spy = vi.spyOn(vscode.workspace, 'createFileSystemWatcher').mockImplementation(((glob: string) => { + globs.push(glob); + return { + onDidChange: () => ({ dispose: () => undefined }), + onDidCreate: () => ({ dispose: () => undefined }), + dispose: () => undefined + }; + }) as any); + const connector = { onDidChangeCustomDocument: undefined, dispatchAction: () => undefined } as any; + const context = { subscriptions: [] as Array<{ dispose(): void }> } as unknown as vscode.ExtensionContext; + const provider: any = new WorkflowEditorProvider(context, connector, profile as DiagramProfile); + spy.mockRestore(); + return { globs, provider }; + } + + it('watches one glob per declared extension', () => { + expect(build({ sourceExtensions: ['.foo', '.bar'] }).globs).toEqual(['**/*.foo', '**/*.bar']); + }); + + it("prefers the profile's own watch globs, which may be wider than its sources", () => { + // `watch.globs` was declared on the profile and read by nothing until now. + // It is the more specific statement, so it wins over the derived globs. + expect(build({ sourceExtensions: ['.foo'], watch: { globs: ['**/*.foo', '**/manifest.json'] } }).globs) + .toEqual(['**/*.foo', '**/manifest.json']); + }); + + it('watches everything when a profile declares neither', () => { + expect(build({}).globs).toEqual(['**/*']); + }); + + it('ignores an on-disk change to a file outside the declared extensions', () => { + const dispatched: string[] = []; + const { provider } = build({ sourceExtensions: ['.foo'] }); + provider.dispatchModelRefresh = (_clientId: string, uri: string) => dispatched.push(uri); + const source = vscode.Uri.file('/repo/designs/pipeline.foo'); + provider.uriToClientId.set(provider.canonicalizeUriString(source), 'client-0'); + + vi.useFakeTimers(); + provider.handleExternalFileChange(vscode.Uri.file('/repo/designs/README.md')); + vi.runAllTimers(); + expect(dispatched, 'a file the product does not own triggered a reload').toEqual([]); + + provider.handleExternalFileChange(vscode.Uri.file('/repo/designs/helper.foo')); + vi.runAllTimers(); + vi.useRealTimers(); + expect(dispatched.length, 'a declared sibling source failed to trigger a reload').toBe(1); + }); +}); diff --git a/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts b/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts index d728709..9831a82 100644 --- a/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts +++ b/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts @@ -363,6 +363,12 @@ export function createSidecarDiagramProfile(input: SidecarProfileInput) { glspClientId: input.glspClientId, glspClientName: input.glspClientName, commands: input.commands, + // The one extension this builder already knows, handed to the platform so + // its open/rename commands and file watchers filter on it — and so the + // "please open a ... file" warnings name it. Without this the platform + // falls back to accepting anything, which is the right default for a + // profile that cannot say, and the wrong one for a builder that can. + sourceExtensions: [input.sourceExtension], supportsElementCreation: input.supportsElementCreation, operationKinds: input.operationKinds, clientBehavior: input.clientBehavior, diff --git a/packages/sidecar-toolkit/test/sidecar-diagram-profile.test.ts b/packages/sidecar-toolkit/test/sidecar-diagram-profile.test.ts index 9720918..6169402 100644 --- a/packages/sidecar-toolkit/test/sidecar-diagram-profile.test.ts +++ b/packages/sidecar-toolkit/test/sidecar-diagram-profile.test.ts @@ -103,6 +103,11 @@ describe('createSidecarDiagramProfile', () => { useAlternateEntityPalette: undefined }); expect(p.watch).toEqual({ globs: ['**/*.py'] }); + // The same extension reaches the platform as a declaration, so the + // open/rename commands filter on it and their warnings name it. Left + // unset, the platform accepts any file — right for a profile that cannot + // say what its sources are called, wrong for this builder, which can. + expect(p.sourceExtensions).toEqual([baseInput().sourceExtension]); }); it('leaves the creation palette alone unless the product suppresses it', () => { diff --git a/scripts/check-neutrality.sh b/scripts/check-neutrality.sh index f68328f..66886a9 100755 --- a/scripts/check-neutrality.sh +++ b/scripts/check-neutrality.sh @@ -4,8 +4,10 @@ # dialogram platform packages. Run via `npm run check:neutrality`. # # The four core packages (diagram-server, diagram-client, extension-core, -# shared) plus the sidecar-toolkit must carry no product vocabulary. The one -# sanctioned exception is extension-core's legacy-settings-compat.ts, which is +# shared) plus the sidecar-toolkit must carry no product vocabulary — including a +# product's FILE EXTENSION, which reads as punctuation rather than as vocabulary +# and so slipped past the word-based gates for as long as they have existed. The +# one sanctioned exception is extension-core's legacy-settings-compat.ts, which is # permitted to name legacy settings keys. # # Exits non-zero if any gate fails. @@ -86,9 +88,40 @@ else echo 'PASS' fi +echo +echo '== Gate 5: no product source extension in the core ==' +# The gate the other four missed. One product's source extension was compiled +# into extension-core in seven places — six path tests and a file-system watcher +# glob — and every gate above passed the whole time: gates 1 and 2 look for WORDS, +# and an extension is not a word. A consumer whose files end in anything else got +# commands that refused every file it gave them, and a warning telling it to open +# a kind of file it does not have — naming an extension it does not use, in text +# it could not change. +# +# What is banned is narrow on purpose: deciding, from a literal extension, whether +# a PATH is one of the product's source files. That question belongs to the +# profile's `sourceExtensions`, which the core reads through +# `extension-core/src/extension/diagram/source-extensions.ts` — a comparison +# against a value, which no literal-matching pattern can catch. +# +# Deliberately NOT banned: an extension test on something that is not a path into +# a product's sources — filtering directory entries by name inside a directory the +# PLATFORM itself defines (`.wf/skills/*.sh`, `.claude/agents/*.md`). That layout +# is the platform's own, so the platform is the one entitled to name it. +gate5_hits="$(grep -rnE "(fsPath|[Pp]ath)[^;]*\.endsWith\(\s*['\"]\." "${CORE_SRC[@]}" || true) +$(grep -rnE "createFileSystemWatcher\(\s*['\"][^'\"]*\*\.[A-Za-z0-9]" "${CORE_SRC[@]}" || true)" +if [ -n "$(echo "${gate5_hits}" | tr -d '[:space:]')" ]; then + echo 'FAIL: the core decides what a source file is from a hardcoded extension;' + echo ' ask the profile instead (DiagramProfile.sourceExtensions):' + echo "${gate5_hits}" + failures=$((failures + 1)) +else + echo 'PASS' +fi + echo if [ "${failures}" -ne 0 ]; then echo "NEUTRALITY CHECK FAILED (${failures} gate(s) violated)" exit 1 fi -echo 'NEUTRALITY CHECK PASSED (4/4 gates)' +echo 'NEUTRALITY CHECK PASSED (5/5 gates)'