From 59e881e24ee37547c0141d2653008d540730bd55 Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Mon, 14 Sep 2026 10:59:35 +0300 Subject: [PATCH 1/7] feat(vscode): checkmark and hover for image reference results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A values file whose images all resolve now reads as verified rather than merely quiet: every reference a registry confirmed gets a checkmark after its repository value, and hovering any checked reference explains what happened — which registry answered, or why the reference could not be answered for at all. The three surfaces (diagnostics, checkmarks, hovers) are projections of one `ReferenceCheck` record built once per document open, so they cannot drift apart about a reference. The checkmark names the answering registry only when it differs from the host the file itself names; nothing can differ until the registry override set lands, but the rule belongs with the decoration that implements it. An unverifiable verdict now has a second thing it must never render as. It already never produces a diagnostic; it never produces a checkmark either, because positive confirmation the tool did not actually obtain is the same lie as a phantom error. Closes #21 --- apps/vscode/src/extension.test.ts | 218 +++++++++++++++++++++++++++++- apps/vscode/src/extension.ts | 201 ++++++++++++++++++++++++--- apps/vscode/test/vscode-stub.ts | 117 ++++++++++++++-- 3 files changed, 508 insertions(+), 28 deletions(-) diff --git a/apps/vscode/src/extension.test.ts b/apps/vscode/src/extension.test.ts index b8bc9ff..894e0e1 100644 --- a/apps/vscode/src/extension.test.ts +++ b/apps/vscode/src/extension.test.ts @@ -1,13 +1,22 @@ import * as vscode from 'vscode'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { emitDidOpenTextDocument, getLastDiagnosticCollection } from '../test/vscode-stub'; +import { + createTextEditorStub, + emitDidChangeVisibleTextEditors, + emitDidOpenTextDocument, + getLastDiagnosticCollection, + getRegisteredHoverProvider, + setVisibleTextEditors, + type TextEditorStub, +} from '../test/vscode-stub'; import { activate, deactivate } from './extension'; -/** Builds a fake `vscode.TextDocument`, with a real `positionAt` so range assertions are exact. */ +/** Builds a fake `vscode.TextDocument`, with a real `positionAt`/`offsetAt` pair so range assertions are exact. */ function createFakeDocument(path: string, text: string, languageId = 'yaml'): vscode.TextDocument { return { uri: { path, toString: () => path }, languageId, + version: 1, getText: () => text, positionAt: (offset: number) => { const before = text.slice(0, offset); @@ -17,6 +26,16 @@ function createFakeDocument(path: string, text: string, languageId = 'yaml'): vs return new vscode.Position(line, character); }, + offsetAt: (position: vscode.Position) => { + const lines = text.split('\n'); + let offset = 0; + + for (let line = 0; line < position.line; line += 1) { + offset += (lines[line]?.length ?? 0) + '\n'.length; + } + + return offset + position.character; + }, } as unknown as vscode.TextDocument; } @@ -32,6 +51,49 @@ function fakeFetchResponse(status: number, body: unknown = {}): { status: number }; } +/** Simulates an edit: VS Code bumps a document's `version` on every change. */ +function bumpVersion(document: vscode.TextDocument): void { + (document as { version: number }).version += 1; +} + +/** Stages `document` as the only visible editor, then fires the open event. */ +async function openInVisibleEditor(document: vscode.TextDocument): Promise { + const editor = createTextEditorStub(document); + + setVisibleTextEditors([editor]); + await emitDidOpenTextDocument(document); + + return editor; +} + +/** The decoration options an editor's most recent `setDecorations` call carried. */ +function getLastDecorations(editor: TextEditorStub): vscode.DecorationOptions[] { + const { calls } = editor.setDecorations.mock; + const lastCall = calls[calls.length - 1] as [unknown, vscode.DecorationOptions[]] | undefined; + + if (lastCall === undefined) { + throw new Error('expected setDecorations to have been called'); + } + + return lastCall[1]; +} + +/** Asks the registered hover provider for a hover at `offset` in `document`. */ +function hoverAt(document: vscode.TextDocument, offset: number): vscode.Hover | undefined { + return getRegisteredHoverProvider()?.provideHover(document, document.positionAt(offset)) as vscode.Hover | undefined; +} + +/** The plain text of a hover's single content entry. */ +function getHoverText(hover: vscode.Hover | undefined): string { + const [content] = hover?.contents ?? []; + + if (content === undefined || typeof content === 'string') { + throw new Error('expected a hover carrying one MarkdownString'); + } + + return content.value; +} + /** Asserts a diagnostics `.set()` call carried exactly one diagnostic, and returns it. */ function getSingleDiagnostic(fileDiagnostics: readonly vscode.Diagnostic[] | undefined): vscode.Diagnostic { expect(fileDiagnostics).toHaveLength(1); @@ -61,6 +123,8 @@ describe('extension', () => { for (const subscription of context.subscriptions) { subscription.dispose(); } + + setVisibleTextEditors([]); }); it('should create an output channel and register it for disposal on activate', () => { @@ -136,6 +200,156 @@ describe('extension', () => { expect(collection?.set).toHaveBeenCalledWith(document.uri, []); }); + it('should render a checkmark on the repository of a reference that exists', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const editor = await openInVisibleEditor(document); + const decorations = getLastDecorations(editor); + + expect(decorations).toHaveLength(1); + expect(decorations[0]?.renderOptions?.after?.contentText).toContain('✓'); + + const repositoryStart = VALUES_YAML.indexOf('docker.io/library/nginx'); + + expect(decorations[0]?.range).toEqual( + new vscode.Range(document.positionAt(repositoryStart), document.positionAt(repositoryStart + 'docker.io/library/nginx'.length)) + ); + }); + + it('should omit the registry name from the checkmark when it matches the host the file names', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const editor = await openInVisibleEditor(document); + + expect(getLastDecorations(editor)[0]?.renderOptions?.after?.contentText).toBe(' ✓'); + }); + + it('should render no checkmark when the tag does not exist, leaving only the diagnostic', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(404, { errors: [{ code: 'MANIFEST_UNKNOWN' }] })); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const editor = await openInVisibleEditor(document); + + expect(getLastDecorations(editor)).toEqual([]); + + const diagnostic = getSingleDiagnostic(getLastDiagnosticCollection()?.set.mock.calls[0]?.[1] as vscode.Diagnostic[] | undefined); + + expect(diagnostic.message).toContain('1.19'); + }); + + it('should render neither a checkmark nor a diagnostic when the reference is unverifiable', async () => { + const fetch = vi.fn().mockRejectedValue(new Error('getaddrinfo ENOTFOUND')); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const editor = await openInVisibleEditor(document); + + expect(getLastDecorations(editor)).toEqual([]); + expect(getLastDiagnosticCollection()?.set).toHaveBeenCalledWith(document.uri, []); + }); + + it('should report the registry that answered when hovering a verified reference', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + await openInVisibleEditor(document); + + const hoverText = getHoverText(hoverAt(document, VALUES_YAML.indexOf('docker.io/library/nginx'))); + + expect(hoverText).toContain('docker.io'); + expect(hoverText).toContain('Verified'); + }); + + it('should report why a reference could not be verified when hovering an unverifiable one', async () => { + const fetch = vi.fn().mockRejectedValue(new Error('getaddrinfo ENOTFOUND')); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + await openInVisibleEditor(document); + + const hoverText = getHoverText(hoverAt(document, VALUES_YAML.indexOf('1.19'))); + + expect(hoverText).toContain('Not verified'); + expect(hoverText).toContain('could not be reached'); + }); + + it('should provide no hover for a reference that does not exist, which already speaks through its diagnostic', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(404, { errors: [{ code: 'MANIFEST_UNKNOWN' }] })); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + await openInVisibleEditor(document); + + expect(hoverAt(document, VALUES_YAML.indexOf('1.19'))).toBeUndefined(); + }); + + it('should provide no hover outside any checked reference', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + await openInVisibleEditor(document); + + expect(hoverAt(document, VALUES_YAML.indexOf('image:'))).toBeUndefined(); + }); + + it('should create the checkmark decoration type and register a yaml hover provider on activate', () => { + activate(context, { fetch: vi.fn() }); + + expect(vscode.window.createTextEditorDecorationType).toHaveBeenCalledWith({ + after: { color: new vscode.ThemeColor('charts.green'), margin: '0 0 0 0.5em' }, + }); + expect(vscode.languages.registerHoverProvider).toHaveBeenCalledWith({ language: 'yaml' }, expect.anything()); + }); + + it('should re-apply checkmarks to an editor that becomes visible after the document was checked', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + + setVisibleTextEditors([]); + await emitDidOpenTextDocument(document); + + const editor = createTextEditorStub(document); + await emitDidChangeVisibleTextEditors([editor]); + + expect(getLastDecorations(editor)[0]?.renderOptions?.after?.contentText).toBe(' ✓'); + }); + + it('should drop checkmarks rather than re-project them onto text edited since the check', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + await openInVisibleEditor(document); + + bumpVersion(document); + + const editor = createTextEditorStub(document); + await emitDidChangeVisibleTextEditors([editor]); + + expect(getLastDecorations(editor)).toEqual([]); + }); + + it('should provide no hover once the document has been edited past the checked version', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + await openInVisibleEditor(document); + + bumpVersion(document); + + expect(hoverAt(document, VALUES_YAML.indexOf('docker.io/library/nginx'))).toBeUndefined(); + }); + it('should ignore a document that is not the conventional values file name', async () => { const fetch = vi.fn(); activate(context, { fetch }); diff --git a/apps/vscode/src/extension.ts b/apps/vscode/src/extension.ts index 4ebb40d..4b6d0ec 100644 --- a/apps/vscode/src/extension.ts +++ b/apps/vscode/src/extension.ts @@ -1,14 +1,45 @@ import * as vscode from 'vscode'; import { extractImageReferences, type ImageReference, type SourceRange } from 'helm'; -import { checkImageExistence, type FetchLike } from 'oci-registry'; +import { checkImageExistence, resolveExplicitHost, type FetchLike, type ImageVerdict, type UnverifiableReason } from 'oci-registry'; const DIAGNOSTIC_COLLECTION_NAME = 'infra-tools-images'; +const CHECKMARK = '✓'; + +// A table rather than a switch, so adding a reason to `UnverifiableReason` +// fails the build here instead of silently hovering with no explanation. +const UNVERIFIABLE_REASON_TEXT: Record = { + 'no-registry': 'the repository names no registry host.', + 'missing-credential': 'the registry requires credentials this extension cannot supply yet.', + 'network-error': 'the registry could not be reached.', + 'unexpected-response': 'the registry answered in a form this extension does not understand.', + 'malformed-reference': 'the tag is not a valid OCI tag.', +}; + // The conventional Helm values file name only. Matching any YAML file // beneath a chart directory (and excluding its templates directory) is // Helm chart-context knowledge this ticket doesn't implement yet. const VALUES_FILE_NAME_PATTERN = /^values\.ya?ml$/i; +/** An image reference that names a tag, the only kind this feature checks today. */ +type TaggedImageReference = ImageReference & { readonly tag: NonNullable }; + +/** + * One checked image reference. Built once per document open, then projected + * onto every surface the result is shown on — diagnostics, checkmarks, and + * hovers — so the three can never disagree about a reference. + */ +interface ReferenceCheck { + readonly reference: TaggedImageReference; + readonly verdict: ImageVerdict; +} + +/** A document's checks, tagged with the document version they describe. */ +interface DocumentChecks { + readonly version: number; + readonly checks: readonly ReferenceCheck[]; +} + interface ActivateDependencies { /** * The fetch implementation existence checks use. Defaults to the @@ -32,9 +63,52 @@ function activate(context: vscode.ExtensionContext, dependencies: ActivateDepend const diagnostics = vscode.languages.createDiagnosticCollection(DIAGNOSTIC_COLLECTION_NAME); context.subscriptions.push(diagnostics); + const checksByDocument = new Map(); + + // `contentText` is per-decoration because it names the answering registry + // when that differs from the host the file names; everything shared lives + // on the one type. + const checkmarkDecorationType = vscode.window.createTextEditorDecorationType({ + after: { + color: new vscode.ThemeColor('charts.green'), + margin: '0 0 0 0.5em', + }, + }); + context.subscriptions.push(checkmarkDecorationType); + context.subscriptions.push( vscode.workspace.onDidOpenTextDocument(async (document) => { - await checkImageReferencesInDocument(document, diagnostics, fetchImpl); + const checked = await checkImageReferencesInDocument(document, fetchImpl); + + if (checked === undefined) { + return; + } + + checksByDocument.set(document.uri.toString(), checked); + diagnostics.set(document.uri, diagnosticsFor(document, checksAsOf(checked, document))); + applyCheckmarks(vscode.window.visibleTextEditors, checksByDocument, checkmarkDecorationType); + }) + ); + + context.subscriptions.push( + vscode.languages.registerHoverProvider( + { language: 'yaml' }, + { + provideHover(document: vscode.TextDocument, position: vscode.Position): vscode.Hover | undefined { + const checked = checksByDocument.get(document.uri.toString()); + + return checked === undefined ? undefined : hoverFor(document, position, checksAsOf(checked, document)); + }, + } + ) + ); + + // A document can be checked before its editor is visible, and split view + // and tab switches hand out editors that carry no decorations yet, so the + // stored checks are re-applied whenever the visible set changes. + context.subscriptions.push( + vscode.window.onDidChangeVisibleTextEditors((editors) => { + applyCheckmarks(editors, checksByDocument, checkmarkDecorationType); }) ); } @@ -55,34 +129,33 @@ function isHelmValuesFile(document: vscode.TextDocument): boolean { } /** - * Extracts and checks a document's image references, then replaces its - * diagnostics with the result. Runs on document open only — checking again - * as the developer types, and clearing stale results when a chart's - * `appVersion` changes, are later tickets. + * Extracts and checks a document's image references. Returns `undefined` for + * a document this feature has nothing to say about, which is not the same as + * a checked document that produced no findings — the caller replaces a + * document's diagnostics and decorations only when it gets an array back. + * Runs on document open only — checking again as the developer types, and + * clearing stale results when a chart's `appVersion` changes, are later + * tickets. */ -async function checkImageReferencesInDocument( - document: vscode.TextDocument, - diagnostics: vscode.DiagnosticCollection, - fetch: FetchLike -): Promise { +async function checkImageReferencesInDocument(document: vscode.TextDocument, fetch: FetchLike): Promise { if (!isHelmValuesFile(document)) { - return; + return undefined; } + const version = document.version; + let references: ImageReference[]; try { references = extractImageReferences(document.getText()); } catch { // A YAML syntax error is the YAML language service's diagnostic to // raise, not this feature's — stay silent rather than compete with it. - return; + return undefined; } // A tagless reference resolves through `appVersion`, a later ticket's job — // nothing to check yet. - const taggedReferences = references.filter( - (reference): reference is ImageReference & { tag: NonNullable } => reference.tag !== undefined - ); + const taggedReferences = references.filter((reference): reference is TaggedImageReference => reference.tag !== undefined); const checks = await Promise.all( taggedReferences.map(async (reference) => ({ @@ -95,6 +168,21 @@ async function checkImageReferencesInDocument( })) ); + return { version, checks }; +} + +/** + * A document's checks, or none when the text has moved on since they were + * taken. The recorded offsets belong to the version that was checked; + * projecting them onto edited text would slide a checkmark onto whatever now + * sits at that offset. Nothing is the honest answer until the next check. + */ +function checksAsOf(checked: DocumentChecks, document: vscode.TextDocument): readonly ReferenceCheck[] { + return checked.version === document.version ? checked.checks : []; +} + +/** The diagnostics a document's checks call for. */ +function diagnosticsFor(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): vscode.Diagnostic[] { const fileDiagnostics: vscode.Diagnostic[] = []; for (const { reference, verdict } of checks) { @@ -121,7 +209,86 @@ async function checkImageReferencesInDocument( } } - diagnostics.set(document.uri, fileDiagnostics); + return fileDiagnostics; +} + +/** + * The checkmarks a document's checks call for: one per reference that + * exists, and nothing at all for any other verdict — an unverifiable + * reference is not confirmation. + */ +function checkmarksFor(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): vscode.DecorationOptions[] { + const decorations: vscode.DecorationOptions[] = []; + + for (const { reference, verdict } of checks) { + if (verdict.kind !== 'exists') { + continue; + } + + // Naming the registry only when it differs from the one the file names + // keeps the annotation informative instead of restating the line. Today + // it can never differ; the registry override set that makes it possible + // is a later ticket. + const namedHost = resolveExplicitHost(reference.repository.text)?.host; + const contentText = verdict.registry === namedHost ? ` ${CHECKMARK}` : ` ${CHECKMARK} ${verdict.registry}`; + + decorations.push({ + range: rangeOf(document, reference.repository.range), + renderOptions: { after: { contentText } }, + }); + } + + return decorations; +} + +/** + * Re-applies each editor's stored checkmarks. An editor showing a checked + * document always gets a `setDecorations` call, empty array included, so a + * reference that stops verifying loses the checkmark it used to have. + */ +function applyCheckmarks( + editors: readonly vscode.TextEditor[], + checksByDocument: ReadonlyMap, + decorationType: vscode.TextEditorDecorationType +): void { + for (const editor of editors) { + const checked = checksByDocument.get(editor.document.uri.toString()); + + if (checked !== undefined) { + editor.setDecorations(decorationType, checkmarksFor(editor.document, checksAsOf(checked, editor.document))); + } + } +} + +/** + * What hovering a position in a checked document reports: the registry that + * confirmed the reference, or why it could not be checked. A not-found + * verdict gets no hover — it already speaks through its diagnostic, and it + * names no registry to report. + */ +function hoverFor(document: vscode.TextDocument, position: vscode.Position, checks: readonly ReferenceCheck[]): vscode.Hover | undefined { + const offset = document.offsetAt(position); + const check = checks.find(({ reference }) => containsOffset(reference.repository.range, offset) || containsOffset(reference.tag.range, offset)); + + if (check === undefined) { + return undefined; + } + + const { verdict } = check; + + if (verdict.kind === 'exists') { + return new vscode.Hover(new vscode.MarkdownString(`Verified on \`${verdict.registry}\`.`)); + } + + if (verdict.kind === 'unverifiable') { + return new vscode.Hover(new vscode.MarkdownString(`Not verified: ${UNVERIFIABLE_REASON_TEXT[verdict.reason]}`)); + } + + return undefined; +} + +function containsOffset(range: SourceRange, offset: number): boolean { + return offset >= range.start && offset < range.end; } function rangeOf(document: vscode.TextDocument, range: SourceRange): vscode.Range { diff --git a/apps/vscode/test/vscode-stub.ts b/apps/vscode/test/vscode-stub.ts index ecdfe24..f8ec3c5 100644 --- a/apps/vscode/test/vscode-stub.ts +++ b/apps/vscode/test/vscode-stub.ts @@ -1,5 +1,3 @@ -import { vi } from 'vitest'; - /** * Minimal stand-in for the `vscode` module. * @@ -8,13 +6,11 @@ import { vi } from 'vitest'; * `import * as vscode from 'vscode'` in tests without booting a real VS Code * instance. Extend this stub as the extension grows — don't add per-file * `vi.mock('vscode', …)` factories. + * + * @packageDocumentation */ -const window = { - createOutputChannel: vi.fn(() => ({ - appendLine: vi.fn(), - dispose: vi.fn(), - })), -}; + +import { vi } from 'vitest'; class Position { public constructor( @@ -30,6 +26,25 @@ class Range { ) {} } +class ThemeColor { + public constructor(public readonly id: string) {} +} + +class MarkdownString { + public constructor(public readonly value: string = '') {} +} + +class Hover { + public readonly contents: (MarkdownString | string)[]; + + public constructor( + contents: MarkdownString | string | (MarkdownString | string)[], + public readonly range?: Range + ) { + this.contents = Array.isArray(contents) ? contents : [contents]; + } +} + // This reproduces the real `vscode.DiagnosticSeverity` enum's member names // and values exactly — extension code does `vscode.DiagnosticSeverity.Error` // against the real `@types/vscode` declaration, so the stub's runtime shape @@ -79,8 +94,26 @@ function createDiagnosticCollectionStub(): DiagnosticCollectionStub { return stub; } +/** The subset of `vscode.HoverProvider` the extension registers. */ +interface HoverProviderStub { + readonly provideHover: (document: unknown, position: unknown) => unknown; +} + +// Registered providers are removed again on dispose, so a test that disposes +// its context's subscriptions leaves no provider behind for the next one. +let hoverProviders: HoverProviderStub[] = []; + const languages = { createDiagnosticCollection: vi.fn(() => createDiagnosticCollectionStub()), + registerHoverProvider: vi.fn((_selector: unknown, provider: HoverProviderStub) => { + hoverProviders.push(provider); + + return { + dispose: vi.fn(() => { + hoverProviders = hoverProviders.filter((registered) => registered !== provider); + }), + }; + }), }; /** Test-only helper: the most recently created diagnostic collection. */ @@ -88,6 +121,25 @@ function getLastDiagnosticCollection(): DiagnosticCollectionStub | undefined { return diagnosticCollections[diagnosticCollections.length - 1]; } +/** Test-only helper: the most recently registered, still-undisposed hover provider. */ +function getRegisteredHoverProvider(): HoverProviderStub | undefined { + return hoverProviders[hoverProviders.length - 1]; +} + +/** + * Test-only stand-in for `vscode.TextEditor`: the document it shows, and a + * spy recording every `setDecorations` call made against it. + */ +interface TextEditorStub { + readonly document: unknown; + readonly setDecorations: ReturnType; +} + +/** Test-only helper that builds a {@link TextEditorStub}. Not part of the real `vscode` API. */ +function createTextEditorStub(document: unknown): TextEditorStub { + return { document, setDecorations: vi.fn() }; +} + /** * A minimal `vscode.Event`-shaped emitter: `event` is what extension code * subscribes through, `fire` is a test-only helper (not part of the real @@ -122,6 +174,17 @@ function createEventEmitterStub(): { } const onDidOpenTextDocumentEmitter = createEventEmitterStub(); +const onDidChangeVisibleTextEditorsEmitter = createEventEmitterStub(); + +const window = { + createOutputChannel: vi.fn(() => ({ + appendLine: vi.fn(), + dispose: vi.fn(), + })), + createTextEditorDecorationType: vi.fn(() => ({ key: 'decoration-type', dispose: vi.fn() })), + visibleTextEditors: [] as readonly TextEditorStub[], + onDidChangeVisibleTextEditors: onDidChangeVisibleTextEditorsEmitter.event, +}; const workspace = { onDidOpenTextDocument: onDidOpenTextDocumentEmitter.event, @@ -136,4 +199,40 @@ async function emitDidOpenTextDocument(document: unknown): Promise { await onDidOpenTextDocumentEmitter.fire(document); } -export { Diagnostic, DiagnosticSeverity, emitDidOpenTextDocument, getLastDiagnosticCollection, languages, Position, Range, window, workspace }; +/** + * Test-only helper that replaces `window.visibleTextEditors`. Not part of the + * real `vscode` API — tests set it to stage which editors the extension can + * decorate, and reset it so one test's editors never leak into another. + */ +function setVisibleTextEditors(editors: readonly TextEditorStub[]): void { + window.visibleTextEditors = editors; +} + +/** + * Test-only helper that fires `window.onDidChangeVisibleTextEditors` after + * updating `window.visibleTextEditors`, the order real VS Code uses. + */ +async function emitDidChangeVisibleTextEditors(editors: readonly TextEditorStub[]): Promise { + setVisibleTextEditors(editors); + await onDidChangeVisibleTextEditorsEmitter.fire(editors); +} + +export type { TextEditorStub }; +export { + createTextEditorStub, + Diagnostic, + DiagnosticSeverity, + emitDidChangeVisibleTextEditors, + emitDidOpenTextDocument, + getLastDiagnosticCollection, + getRegisteredHoverProvider, + Hover, + languages, + MarkdownString, + Position, + Range, + setVisibleTextEditors, + ThemeColor, + window, + workspace, +}; From f588091dd368996e9674536fdf328182fa17ef6a Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:32:02 +0300 Subject: [PATCH 2/7] fix(vscode): never let a disposed editor kill the extension host VS Code does not await event listeners, so anything escaping the async `onDidOpenTextDocument` callback becomes an unhandled rejection, and an unhandled rejection terminates the extension host. Closing the tab while the registry check is still in flight was enough to trigger it, because `setDecorations` throws on an editor that has since been disposed. The window is as long as the network call. The listener now catches at that boundary and reports to the output channel. `applyCheckmarks` additionally guards each editor separately, so one disposed editor no longer aborts the loop and costs every other visible editor its checkmarks. Driving the real bundle in a Node harness with a `vscode` fake reproduced the fatal rejection before the fix and shows it surviving after. Both new tests were mutation-checked against the guards they cover. --- apps/vscode/src/extension.test.ts | 38 +++++++++++++++++++++++++++++++ apps/vscode/src/extension.ts | 37 ++++++++++++++++++++++-------- 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/apps/vscode/src/extension.test.ts b/apps/vscode/src/extension.test.ts index 894e0e1..2b1f256 100644 --- a/apps/vscode/src/extension.test.ts +++ b/apps/vscode/src/extension.test.ts @@ -350,6 +350,44 @@ describe('extension', () => { expect(hoverAt(document, VALUES_YAML.indexOf('docker.io/library/nginx'))).toBeUndefined(); }); + it('should survive an editor disposed mid-check, since an unhandled rejection kills the extension host', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const disposedEditor: TextEditorStub = { + document, + setDecorations: vi.fn(() => { + throw new Error('TextEditor#setDecorations: editor disposed'); + }), + }; + + setVisibleTextEditors([disposedEditor]); + + await expect(emitDidOpenTextDocument(document)).resolves.toBeUndefined(); + expect(getLastDiagnosticCollection()?.set).toHaveBeenCalledWith(document.uri, []); + }); + + it('should still decorate the surviving editors when one of them was disposed', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + await openInVisibleEditor(document); + + const disposedEditor: TextEditorStub = { + document, + setDecorations: vi.fn(() => { + throw new Error('TextEditor#setDecorations: editor disposed'); + }), + }; + const survivingEditor = createTextEditorStub(document); + + await emitDidChangeVisibleTextEditors([disposedEditor, survivingEditor]); + + expect(getLastDecorations(survivingEditor)[0]?.renderOptions?.after?.contentText).toBe(' ✓'); + }); + it('should ignore a document that is not the conventional values file name', async () => { const fetch = vi.fn(); activate(context, { fetch }); diff --git a/apps/vscode/src/extension.ts b/apps/vscode/src/extension.ts index 4b6d0ec..1757655 100644 --- a/apps/vscode/src/extension.ts +++ b/apps/vscode/src/extension.ts @@ -78,15 +78,25 @@ function activate(context: vscode.ExtensionContext, dependencies: ActivateDepend context.subscriptions.push( vscode.workspace.onDidOpenTextDocument(async (document) => { - const checked = await checkImageReferencesInDocument(document, fetchImpl); - - if (checked === undefined) { - return; + // VS Code never awaits an event listener, so anything escaping this + // async callback becomes an unhandled rejection, and an unhandled + // rejection takes the whole extension host down with it. Closing the + // tab mid-check is enough to do it: `setDecorations` throws on an + // editor that has since been disposed. Nothing this feature can fail + // at is worth a dead extension host. + try { + const checked = await checkImageReferencesInDocument(document, fetchImpl); + + if (checked === undefined) { + return; + } + + checksByDocument.set(document.uri.toString(), checked); + diagnostics.set(document.uri, diagnosticsFor(document, checksAsOf(checked, document))); + applyCheckmarks(vscode.window.visibleTextEditors, checksByDocument, checkmarkDecorationType); + } catch (error) { + channel.appendLine(`Checking image references in ${document.uri.toString()} failed: ${String(error)}`); } - - checksByDocument.set(document.uri.toString(), checked); - diagnostics.set(document.uri, diagnosticsFor(document, checksAsOf(checked, document))); - applyCheckmarks(vscode.window.visibleTextEditors, checksByDocument, checkmarkDecorationType); }) ); @@ -254,8 +264,17 @@ function applyCheckmarks( for (const editor of editors) { const checked = checksByDocument.get(editor.document.uri.toString()); - if (checked !== undefined) { + if (checked === undefined) { + continue; + } + + try { editor.setDecorations(decorationType, checkmarksFor(editor.document, checksAsOf(checked, editor.document))); + } catch { + // An editor disposed between the check and this call throws here, and + // this runs while editors are being torn down. One dead editor must + // not cost every other visible editor its checkmarks. + continue; } } } From 029eca3ac6fd48b17d919eab40d81c57a69b5a17 Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:39:59 +0300 Subject: [PATCH 3/7] feat(vscode): mark every checked reference, not only the verified ones A reference with no mark is indistinguishable from an extension that never ran, which is exactly how this landed in practice: most real references are unverifiable today, so a values file rendered nothing at all and looked broken rather than unanswered. Every checked reference now carries one inline mark. A green check for verified, a red cross beside the existing squiggle for a reference that does not exist, and a muted question mark for one the tool could not answer. The unverifiable mark is deliberately a question mark and deliberately muted. It reports a fact about the developer's machine, not a defect in the file, and styling it as a failure would recreate the cry-wolf problem the unverifiable verdict exists to prevent. The invariant it protects is untouched: unverifiable still produces no diagnostic and never reaches the Problems panel. Glyph and colour ride on each decoration rather than on the decoration type, so all three marks share one type and one setDecorations call per editor still replaces the lot. Which mark an outcome renders as is an exhaustive switch over the verdict union, so a new verdict kind fails the build here. This contradicts two of #21's acceptance criteria as literally written, which said a not-found reference renders only its diagnostic and an unverifiable one renders nothing. Raised on the PR. --- apps/vscode/src/extension.test.ts | 62 +++++++++++++++++--- apps/vscode/src/extension.ts | 96 +++++++++++++++++++++---------- 2 files changed, 119 insertions(+), 39 deletions(-) diff --git a/apps/vscode/src/extension.test.ts b/apps/vscode/src/extension.test.ts index 2b1f256..66e9986 100644 --- a/apps/vscode/src/extension.test.ts +++ b/apps/vscode/src/extension.test.ts @@ -41,6 +41,23 @@ function createFakeDocument(path: string, text: string, languageId = 'yaml'): vs const VALUES_YAML = ['image:', ' repository: docker.io/library/nginx', ' tag: 1.19', ''].join('\n'); +/** Three references whose outcomes differ, so one file exercises all three marks at once. */ +const MIXED_VALUES_YAML = [ + 'good:', + ' image:', + ' repository: registry.example.com/good', + ' tag: "1.0"', + 'bad:', + ' image:', + ' repository: registry.example.com/bad', + ' tag: "2.0"', + 'unknown:', + ' image:', + ' repository: registry.example.com/unknown', + ' tag: "3.0"', + '', +].join('\n'); + /** A canned fetch `Response`-shaped object for the injected fetch fake. */ function fakeFetchResponse(status: number, body: unknown = {}): { status: number; ok: boolean; json: () => Promise } { return { @@ -228,28 +245,32 @@ describe('extension', () => { expect(getLastDecorations(editor)[0]?.renderOptions?.after?.contentText).toBe(' ✓'); }); - it('should render no checkmark when the tag does not exist, leaving only the diagnostic', async () => { + it('should render a cross alongside the diagnostic when the tag does not exist', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(404, { errors: [{ code: 'MANIFEST_UNKNOWN' }] })); activate(context, { fetch }); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); const editor = await openInVisibleEditor(document); + const [mark] = getLastDecorations(editor); - expect(getLastDecorations(editor)).toEqual([]); + expect(mark?.renderOptions?.after?.contentText).toBe(' ✗'); + expect(mark?.renderOptions?.after?.color).toEqual(new vscode.ThemeColor('errorForeground')); const diagnostic = getSingleDiagnostic(getLastDiagnosticCollection()?.set.mock.calls[0]?.[1] as vscode.Diagnostic[] | undefined); expect(diagnostic.message).toContain('1.19'); }); - it('should render neither a checkmark nor a diagnostic when the reference is unverifiable', async () => { + it('should render a muted question mark and no diagnostic when the reference is unverifiable', async () => { const fetch = vi.fn().mockRejectedValue(new Error('getaddrinfo ENOTFOUND')); activate(context, { fetch }); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); const editor = await openInVisibleEditor(document); + const [mark] = getLastDecorations(editor); - expect(getLastDecorations(editor)).toEqual([]); + expect(mark?.renderOptions?.after?.contentText).toBe(' ?'); + expect(mark?.renderOptions?.after?.color).toEqual(new vscode.ThemeColor('descriptionForeground')); expect(getLastDiagnosticCollection()?.set).toHaveBeenCalledWith(document.uri, []); }); @@ -299,12 +320,10 @@ describe('extension', () => { expect(hoverAt(document, VALUES_YAML.indexOf('image:'))).toBeUndefined(); }); - it('should create the checkmark decoration type and register a yaml hover provider on activate', () => { + it('should create one mark decoration type and register a yaml hover provider on activate', () => { activate(context, { fetch: vi.fn() }); - expect(vscode.window.createTextEditorDecorationType).toHaveBeenCalledWith({ - after: { color: new vscode.ThemeColor('charts.green'), margin: '0 0 0 0.5em' }, - }); + expect(vscode.window.createTextEditorDecorationType).toHaveBeenCalledWith({ after: { margin: '0 0 0 0.5em' } }); expect(vscode.languages.registerHoverProvider).toHaveBeenCalledWith({ language: 'yaml' }, expect.anything()); }); @@ -388,6 +407,33 @@ describe('extension', () => { expect(getLastDecorations(survivingEditor)[0]?.renderOptions?.after?.contentText).toBe(' ✓'); }); + it('should mark every reference in one pass, whatever each outcome was', async () => { + // eslint-disable-next-line @typescript-eslint/promise-function-async -- canned per-URL responses, nothing to await + const fetch = vi.fn((url: string) => { + if (url.includes('/good/')) { + return Promise.resolve(fakeFetchResponse(200)); + } + + if (url.includes('/bad/')) { + return Promise.resolve(fakeFetchResponse(404, { errors: [{ code: 'MANIFEST_UNKNOWN' }] })); + } + + return Promise.reject(new Error('getaddrinfo ENOTFOUND')); + }); + activate(context, { fetch }); + + const document = createFakeDocument('/repo/chart/values.yaml', MIXED_VALUES_YAML); + const editor = await openInVisibleEditor(document); + const marks = getLastDecorations(editor); + + expect(marks.map((mark) => mark.renderOptions?.after?.contentText)).toEqual([' ✓', ' ✗', ' ?']); + expect(marks.map((mark) => mark.renderOptions?.after?.color)).toEqual([ + new vscode.ThemeColor('charts.green'), + new vscode.ThemeColor('errorForeground'), + new vscode.ThemeColor('descriptionForeground'), + ]); + }); + it('should ignore a document that is not the conventional values file name', async () => { const fetch = vi.fn(); activate(context, { fetch }); diff --git a/apps/vscode/src/extension.ts b/apps/vscode/src/extension.ts index 1757655..e7e222b 100644 --- a/apps/vscode/src/extension.ts +++ b/apps/vscode/src/extension.ts @@ -4,7 +4,25 @@ import { checkImageExistence, resolveExplicitHost, type FetchLike, type ImageVer const DIAGNOSTIC_COLLECTION_NAME = 'infra-tools-images'; -const CHECKMARK = '✓'; +/** What a reference's inline mark says at a glance, before any hover. */ +type MarkKind = 'verified' | 'missing' | 'unchecked'; + +/** + * The glyph and theme colour each outcome renders as. + * + * `unchecked` is deliberately muted and deliberately a question mark rather + * than a cross. It reports that the tool could not answer, which is a fact + * about the developer's machine, not a defect in the file. Styling it like + * an error would recreate exactly the cry-wolf problem the unverifiable + * verdict exists to prevent. It still has to render something, because a + * reference with no mark at all is indistinguishable from an extension that + * never ran. + */ +const MARKS: Record = { + verified: { glyph: '✓', color: 'charts.green' }, + missing: { glyph: '✗', color: 'errorForeground' }, + unchecked: { glyph: '?', color: 'descriptionForeground' }, +}; // A table rather than a switch, so adding a reason to `UnverifiableReason` // fails the build here instead of silently hovering with no explanation. @@ -65,16 +83,13 @@ function activate(context: vscode.ExtensionContext, dependencies: ActivateDepend const checksByDocument = new Map(); - // `contentText` is per-decoration because it names the answering registry - // when that differs from the host the file names; everything shared lives - // on the one type. - const checkmarkDecorationType = vscode.window.createTextEditorDecorationType({ - after: { - color: new vscode.ThemeColor('charts.green'), - margin: '0 0 0 0.5em', - }, + // Glyph and colour both ride on each decoration, since both vary per + // outcome; only the gap from the value is shared, so one type covers all + // three marks and one `setDecorations` call per editor replaces the lot. + const markDecorationType = vscode.window.createTextEditorDecorationType({ + after: { margin: '0 0 0 0.5em' }, }); - context.subscriptions.push(checkmarkDecorationType); + context.subscriptions.push(markDecorationType); context.subscriptions.push( vscode.workspace.onDidOpenTextDocument(async (document) => { @@ -93,7 +108,7 @@ function activate(context: vscode.ExtensionContext, dependencies: ActivateDepend checksByDocument.set(document.uri.toString(), checked); diagnostics.set(document.uri, diagnosticsFor(document, checksAsOf(checked, document))); - applyCheckmarks(vscode.window.visibleTextEditors, checksByDocument, checkmarkDecorationType); + applyMarks(vscode.window.visibleTextEditors, checksByDocument, markDecorationType); } catch (error) { channel.appendLine(`Checking image references in ${document.uri.toString()} failed: ${String(error)}`); } @@ -118,7 +133,7 @@ function activate(context: vscode.ExtensionContext, dependencies: ActivateDepend // stored checks are re-applied whenever the visible set changes. context.subscriptions.push( vscode.window.onDidChangeVisibleTextEditors((editors) => { - applyCheckmarks(editors, checksByDocument, checkmarkDecorationType); + applyMarks(editors, checksByDocument, markDecorationType); }) ); } @@ -222,29 +237,48 @@ function diagnosticsFor(document: vscode.TextDocument, checks: readonly Referenc return fileDiagnostics; } +/** Which mark an outcome renders as. Exhaustive, so a new verdict kind fails the build. */ +function markKindOf(verdict: ImageVerdict): MarkKind { + switch (verdict.kind) { + case 'exists': + return 'verified'; + case 'repository-not-found': + case 'tag-not-found': + return 'missing'; + case 'unverifiable': + return 'unchecked'; + } +} + +/** + * The answering registry, named only when it differs from the host the file + * names, so the mark carries information instead of restating the line. + * Today it can never differ; the registry override set that makes it + * possible is a later ticket. + */ +function registrySuffixOf(reference: TaggedImageReference, verdict: ImageVerdict): string { + if (verdict.kind !== 'exists' || verdict.registry === resolveExplicitHost(reference.repository.text)?.host) { + return ''; + } + + return ` ${verdict.registry}`; +} + /** - * The checkmarks a document's checks call for: one per reference that - * exists, and nothing at all for any other verdict — an unverifiable - * reference is not confirmation. + * The marks a document's checks call for: exactly one per checked reference, + * whatever the outcome. Colour rides on each decoration rather than on the + * decoration type so that all three marks share one type, and one + * `setDecorations` call per editor still replaces the lot. */ -function checkmarksFor(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): vscode.DecorationOptions[] { +function marksFor(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): vscode.DecorationOptions[] { const decorations: vscode.DecorationOptions[] = []; for (const { reference, verdict } of checks) { - if (verdict.kind !== 'exists') { - continue; - } - - // Naming the registry only when it differs from the one the file names - // keeps the annotation informative instead of restating the line. Today - // it can never differ; the registry override set that makes it possible - // is a later ticket. - const namedHost = resolveExplicitHost(reference.repository.text)?.host; - const contentText = verdict.registry === namedHost ? ` ${CHECKMARK}` : ` ${CHECKMARK} ${verdict.registry}`; + const { glyph, color } = MARKS[markKindOf(verdict)]; decorations.push({ range: rangeOf(document, reference.repository.range), - renderOptions: { after: { contentText } }, + renderOptions: { after: { contentText: ` ${glyph}${registrySuffixOf(reference, verdict)}`, color: new vscode.ThemeColor(color) } }, }); } @@ -252,11 +286,11 @@ function checkmarksFor(document: vscode.TextDocument, checks: readonly Reference } /** - * Re-applies each editor's stored checkmarks. An editor showing a checked + * Re-applies each editor's stored marks. An editor showing a checked * document always gets a `setDecorations` call, empty array included, so a - * reference that stops verifying loses the checkmark it used to have. + * reference whose outcome changed loses the mark it used to have. */ -function applyCheckmarks( +function applyMarks( editors: readonly vscode.TextEditor[], checksByDocument: ReadonlyMap, decorationType: vscode.TextEditorDecorationType @@ -269,7 +303,7 @@ function applyCheckmarks( } try { - editor.setDecorations(decorationType, checkmarksFor(editor.document, checksAsOf(checked, editor.document))); + editor.setDecorations(decorationType, marksFor(editor.document, checksAsOf(checked, editor.document))); } catch { // An editor disposed between the check and this call throws here, and // this runs while editors are being torn down. One dead editor must From 5fb9afd8ccd568383e42292e04e0a30cdca3f8ef Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Tue, 15 Sep 2026 11:32:14 +0300 Subject: [PATCH 4/7] feat(vscode): mark tagless references instead of dropping them A reference with no tag was filtered out before any verdict existed, so it rendered nothing at all. That is the same failure the marks were added to fix: nothing is indistinguishable from a tool that never ran, and a tagless image is normal in charts that let the tag default to appVersion. Tagless references now come through as a check with a `no-tag` reason, so they carry the muted question mark and a hover that says the reference names no tag to check. No registry request is made for them. Resolving the tag from the chart's appVersion stays a later ticket; this only stops the reference from being invisible until then. `UncheckedReason` widens the registry package's `UnverifiableReason` with the outcomes the extension settles before it asks anything, and `ReferenceVerdict` widens `ImageVerdict` the same way. The reason table is keyed on the wider type, so a new reason still fails the build rather than hovering with no explanation. `TaggedImageReference` had no callers left and is gone; the two places that assumed a tag now read it as optional rather than asserting it away. Digest-pinned references parse as tagless and so pick up the same question mark, where #17 called for no marker at all. Left as is deliberately: we don't use digests, and telling them apart would mean surfacing the digest key out of the helm package for no gain. --- apps/vscode/src/extension.test.ts | 25 +++++++++++++ apps/vscode/src/extension.ts | 58 ++++++++++++++++++++----------- 2 files changed, 62 insertions(+), 21 deletions(-) diff --git a/apps/vscode/src/extension.test.ts b/apps/vscode/src/extension.test.ts index 66e9986..3aa7ddf 100644 --- a/apps/vscode/src/extension.test.ts +++ b/apps/vscode/src/extension.test.ts @@ -434,6 +434,31 @@ describe('extension', () => { ]); }); + it('should mark a tagless reference as unchecked instead of dropping it silently', async () => { + const fetch = vi.fn(); + activate(context, { fetch }); + + const taglessYaml = ['image:', ' repository: registry.example.com/svc', ' pullPolicy: IfNotPresent', ''].join('\n'); + const document = createFakeDocument('/repo/chart/values.yaml', taglessYaml); + const editor = await openInVisibleEditor(document); + const [mark] = getLastDecorations(editor); + + expect(fetch).not.toHaveBeenCalled(); + expect(mark?.renderOptions?.after?.contentText).toBe(' ?'); + expect(getLastDiagnosticCollection()?.set).toHaveBeenCalledWith(document.uri, []); + }); + + it('should explain a tagless reference on hover', async () => { + const fetch = vi.fn(); + activate(context, { fetch }); + + const taglessYaml = ['image:', ' repository: registry.example.com/svc', ' pullPolicy: IfNotPresent', ''].join('\n'); + const document = createFakeDocument('/repo/chart/values.yaml', taglessYaml); + await openInVisibleEditor(document); + + expect(getHoverText(hoverAt(document, taglessYaml.indexOf('registry.example.com/svc')))).toContain('no tag'); + }); + it('should ignore a document that is not the conventional values file name', async () => { const fetch = vi.fn(); activate(context, { fetch }); diff --git a/apps/vscode/src/extension.ts b/apps/vscode/src/extension.ts index e7e222b..3d9f1c1 100644 --- a/apps/vscode/src/extension.ts +++ b/apps/vscode/src/extension.ts @@ -24,9 +24,19 @@ const MARKS: Record | { readonly kind: 'unverifiable'; readonly reason: UncheckedReason }; + +// A table rather than a switch, so adding a reason to `UncheckedReason` // fails the build here instead of silently hovering with no explanation. -const UNVERIFIABLE_REASON_TEXT: Record = { +const UNVERIFIABLE_REASON_TEXT: Record = { + 'no-tag': 'the reference names no tag to check.', 'no-registry': 'the repository names no registry host.', 'missing-credential': 'the registry requires credentials this extension cannot supply yet.', 'network-error': 'the registry could not be reached.', @@ -39,17 +49,14 @@ const UNVERIFIABLE_REASON_TEXT: Record = { // Helm chart-context knowledge this ticket doesn't implement yet. const VALUES_FILE_NAME_PATTERN = /^values\.ya?ml$/i; -/** An image reference that names a tag, the only kind this feature checks today. */ -type TaggedImageReference = ImageReference & { readonly tag: NonNullable }; - /** * One checked image reference. Built once per document open, then projected * onto every surface the result is shown on — diagnostics, checkmarks, and * hovers — so the three can never disagree about a reference. */ interface ReferenceCheck { - readonly reference: TaggedImageReference; - readonly verdict: ImageVerdict; + readonly reference: ImageReference; + readonly verdict: ReferenceVerdict; } /** A document's checks, tagged with the document version they describe. */ @@ -178,18 +185,21 @@ async function checkImageReferencesInDocument(document: vscode.TextDocument, fet return undefined; } - // A tagless reference resolves through `appVersion`, a later ticket's job — - // nothing to check yet. - const taggedReferences = references.filter((reference): reference is TaggedImageReference => reference.tag !== undefined); - + // A tagless reference resolves through the chart's `appVersion`, a later + // ticket's job, so there is nothing to ask a registry yet. It still comes + // through as a check: dropping it here is what made a real reference + // render nothing at all, which is indistinguishable from a broken tool. const checks = await Promise.all( - taggedReferences.map(async (reference) => ({ + references.map(async (reference) => ({ reference, - verdict: await checkImageExistence({ - repository: reference.repository.text, - tag: reference.tag.text, - fetch, - }), + verdict: + reference.tag === undefined + ? ({ kind: 'unverifiable', reason: 'no-tag' } as const) + : await checkImageExistence({ + repository: reference.repository.text, + tag: reference.tag.text, + fetch, + }), })) ); @@ -226,7 +236,10 @@ function diagnosticsFor(document: vscode.TextDocument, checks: readonly Referenc } else if (verdict.kind === 'tag-not-found') { fileDiagnostics.push( new vscode.Diagnostic( - rangeOf(document, reference.tag.range), + // A tag-not-found verdict can only come back for a reference that + // named a tag, so the fallback is unreachable; it exists so the + // type stays honest rather than being asserted away. + rangeOf(document, reference.tag?.range ?? reference.repository.range), `Tag '${verdict.tag}' not found in '${verdict.repository}'.`, vscode.DiagnosticSeverity.Error ) @@ -238,7 +251,7 @@ function diagnosticsFor(document: vscode.TextDocument, checks: readonly Referenc } /** Which mark an outcome renders as. Exhaustive, so a new verdict kind fails the build. */ -function markKindOf(verdict: ImageVerdict): MarkKind { +function markKindOf(verdict: ReferenceVerdict): MarkKind { switch (verdict.kind) { case 'exists': return 'verified'; @@ -256,7 +269,7 @@ function markKindOf(verdict: ImageVerdict): MarkKind { * Today it can never differ; the registry override set that makes it * possible is a later ticket. */ -function registrySuffixOf(reference: TaggedImageReference, verdict: ImageVerdict): string { +function registrySuffixOf(reference: ImageReference, verdict: ReferenceVerdict): string { if (verdict.kind !== 'exists' || verdict.registry === resolveExplicitHost(reference.repository.text)?.host) { return ''; } @@ -321,7 +334,10 @@ function applyMarks( */ function hoverFor(document: vscode.TextDocument, position: vscode.Position, checks: readonly ReferenceCheck[]): vscode.Hover | undefined { const offset = document.offsetAt(position); - const check = checks.find(({ reference }) => containsOffset(reference.repository.range, offset) || containsOffset(reference.tag.range, offset)); + const check = checks.find( + ({ reference }) => + containsOffset(reference.repository.range, offset) || (reference.tag !== undefined && containsOffset(reference.tag.range, offset)) + ); if (check === undefined) { return undefined; From 79dc2cd3c8d008b4ddac3910985dabda469db829 Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:57:05 +0300 Subject: [PATCH 5/7] refactor(vscode): split the extension into one module per surface extension.ts had grown to 367 lines holding the check pipeline, all three render surfaces, and the activation wiring. It is now 78 lines of wiring only, with each surface in its own module: reference-check owns what a values file is and what checking one produces, diagnostics, marks, and hover each own one surface, and source-range holds the two range helpers they share. Comments are cut back to the ones carrying a why the code cannot show. The MARKS table explained at length what its three entries already say; that is gone, leaving only the reason `unchecked` is a muted question mark rather than a cross. The test file is deliberately untouched in this commit. An unchanged suite passing is the evidence that no behaviour moved with the code; splitting the tests to mirror the modules is the next commit. --- apps/vscode/src/diagnostics.ts | 39 ++++ apps/vscode/src/extension.ts | 315 ++--------------------------- apps/vscode/src/hover.ts | 44 ++++ apps/vscode/src/marks.ts | 96 +++++++++ apps/vscode/src/reference-check.ts | 88 ++++++++ apps/vscode/src/source-range.ts | 12 ++ 6 files changed, 292 insertions(+), 302 deletions(-) create mode 100644 apps/vscode/src/diagnostics.ts create mode 100644 apps/vscode/src/hover.ts create mode 100644 apps/vscode/src/marks.ts create mode 100644 apps/vscode/src/reference-check.ts create mode 100644 apps/vscode/src/source-range.ts diff --git a/apps/vscode/src/diagnostics.ts b/apps/vscode/src/diagnostics.ts new file mode 100644 index 0000000..43c18fd --- /dev/null +++ b/apps/vscode/src/diagnostics.ts @@ -0,0 +1,39 @@ +import * as vscode from 'vscode'; +import type { ReferenceCheck } from './reference-check'; +import { rangeOf } from './source-range'; + +/** + * The diagnostics a document's checks call for. Only the two not-found + * verdicts qualify: that an unverifiable verdict never renders as an error + * is the one invariant this feature must not break, because an expired token + * or an unreachable registry must never look like a missing image. + */ +function diagnosticsFor(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): vscode.Diagnostic[] { + const fileDiagnostics: vscode.Diagnostic[] = []; + + for (const { reference, verdict } of checks) { + if (verdict.kind === 'repository-not-found') { + fileDiagnostics.push( + new vscode.Diagnostic( + rangeOf(document, reference.repository.range), + `Repository '${verdict.repository}' not found.`, + vscode.DiagnosticSeverity.Error + ) + ); + } else if (verdict.kind === 'tag-not-found') { + fileDiagnostics.push( + new vscode.Diagnostic( + // Unreachable fallback: only a reference that named a tag can come + // back tag-not-found. It keeps the type honest without an assertion. + rangeOf(document, reference.tag?.range ?? reference.repository.range), + `Tag '${verdict.tag}' not found in '${verdict.repository}'.`, + vscode.DiagnosticSeverity.Error + ) + ); + } + } + + return fileDiagnostics; +} + +export { diagnosticsFor }; diff --git a/apps/vscode/src/extension.ts b/apps/vscode/src/extension.ts index 3d9f1c1..b2ab114 100644 --- a/apps/vscode/src/extension.ts +++ b/apps/vscode/src/extension.ts @@ -1,83 +1,20 @@ import * as vscode from 'vscode'; -import { extractImageReferences, type ImageReference, type SourceRange } from 'helm'; -import { checkImageExistence, resolveExplicitHost, type FetchLike, type ImageVerdict, type UnverifiableReason } from 'oci-registry'; +import type { FetchLike } from 'oci-registry'; +import { diagnosticsFor } from './diagnostics'; +import { hoverFor } from './hover'; +import { applyMarks, createMarkDecorationType } from './marks'; +import { checkImageReferencesInDocument, checksAsOf, type DocumentChecks } from './reference-check'; const DIAGNOSTIC_COLLECTION_NAME = 'infra-tools-images'; -/** What a reference's inline mark says at a glance, before any hover. */ -type MarkKind = 'verified' | 'missing' | 'unchecked'; - -/** - * The glyph and theme colour each outcome renders as. - * - * `unchecked` is deliberately muted and deliberately a question mark rather - * than a cross. It reports that the tool could not answer, which is a fact - * about the developer's machine, not a defect in the file. Styling it like - * an error would recreate exactly the cry-wolf problem the unverifiable - * verdict exists to prevent. It still has to render something, because a - * reference with no mark at all is indistinguishable from an extension that - * never ran. - */ -const MARKS: Record = { - verified: { glyph: '✓', color: 'charts.green' }, - missing: { glyph: '✗', color: 'errorForeground' }, - unchecked: { glyph: '?', color: 'descriptionForeground' }, -}; - -/** - * Why a reference went unanswered: every reason the registry package can - * report, plus the ones this extension settles before it asks anything. - */ -type UncheckedReason = UnverifiableReason | 'no-tag'; - -/** A registry verdict, widened by the outcomes this extension decides itself. */ -type ReferenceVerdict = Exclude | { readonly kind: 'unverifiable'; readonly reason: UncheckedReason }; - -// A table rather than a switch, so adding a reason to `UncheckedReason` -// fails the build here instead of silently hovering with no explanation. -const UNVERIFIABLE_REASON_TEXT: Record = { - 'no-tag': 'the reference names no tag to check.', - 'no-registry': 'the repository names no registry host.', - 'missing-credential': 'the registry requires credentials this extension cannot supply yet.', - 'network-error': 'the registry could not be reached.', - 'unexpected-response': 'the registry answered in a form this extension does not understand.', - 'malformed-reference': 'the tag is not a valid OCI tag.', -}; - -// The conventional Helm values file name only. Matching any YAML file -// beneath a chart directory (and excluding its templates directory) is -// Helm chart-context knowledge this ticket doesn't implement yet. -const VALUES_FILE_NAME_PATTERN = /^values\.ya?ml$/i; - -/** - * One checked image reference. Built once per document open, then projected - * onto every surface the result is shown on — diagnostics, checkmarks, and - * hovers — so the three can never disagree about a reference. - */ -interface ReferenceCheck { - readonly reference: ImageReference; - readonly verdict: ReferenceVerdict; -} - -/** A document's checks, tagged with the document version they describe. */ -interface DocumentChecks { - readonly version: number; - readonly checks: readonly ReferenceCheck[]; -} - interface ActivateDependencies { - /** - * The fetch implementation existence checks use. Defaults to the - * platform's global `fetch`; only tests have a reason to override it — - * production activation never does. - */ + /** Only tests override this; production activation uses the platform's `fetch`. */ readonly fetch?: FetchLike; } /** - * Called by the extension host when the extension activates. Registers a - * diagnostics collection and checks a Helm values file's image references - * against their registries whenever one is opened. + * Called by the extension host on activation. Wires the three surfaces a + * check is shown on and re-checks a Helm values file whenever one opens. */ function activate(context: vscode.ExtensionContext, dependencies: ActivateDependencies = {}): void { const channel = vscode.window.createOutputChannel('Infra Tools'); @@ -89,23 +26,13 @@ function activate(context: vscode.ExtensionContext, dependencies: ActivateDepend context.subscriptions.push(diagnostics); const checksByDocument = new Map(); - - // Glyph and colour both ride on each decoration, since both vary per - // outcome; only the gap from the value is shared, so one type covers all - // three marks and one `setDecorations` call per editor replaces the lot. - const markDecorationType = vscode.window.createTextEditorDecorationType({ - after: { margin: '0 0 0 0.5em' }, - }); + const markDecorationType = createMarkDecorationType(); context.subscriptions.push(markDecorationType); context.subscriptions.push( vscode.workspace.onDidOpenTextDocument(async (document) => { - // VS Code never awaits an event listener, so anything escaping this - // async callback becomes an unhandled rejection, and an unhandled - // rejection takes the whole extension host down with it. Closing the - // tab mid-check is enough to do it: `setDecorations` throws on an - // editor that has since been disposed. Nothing this feature can fail - // at is worth a dead extension host. + // VS Code never awaits a listener, so anything escaping this callback + // is an unhandled rejection, and that takes the extension host down. try { const checked = await checkImageReferencesInDocument(document, fetchImpl); @@ -135,9 +62,8 @@ function activate(context: vscode.ExtensionContext, dependencies: ActivateDepend ) ); - // A document can be checked before its editor is visible, and split view - // and tab switches hand out editors that carry no decorations yet, so the - // stored checks are re-applied whenever the visible set changes. + // A document can be checked before its editor is visible, and tab switches + // hand out editors carrying no decorations yet. context.subscriptions.push( vscode.window.onDidChangeVisibleTextEditors((editors) => { applyMarks(editors, checksByDocument, markDecorationType); @@ -149,219 +75,4 @@ function deactivate(): void { // Nothing to clean up yet. } -/** Whether a document is the conventional Helm values file. */ -function isHelmValuesFile(document: vscode.TextDocument): boolean { - if (document.languageId !== 'yaml') { - return false; - } - - const fileName = document.uri.path.split('/').pop() ?? ''; - - return VALUES_FILE_NAME_PATTERN.test(fileName); -} - -/** - * Extracts and checks a document's image references. Returns `undefined` for - * a document this feature has nothing to say about, which is not the same as - * a checked document that produced no findings — the caller replaces a - * document's diagnostics and decorations only when it gets an array back. - * Runs on document open only — checking again as the developer types, and - * clearing stale results when a chart's `appVersion` changes, are later - * tickets. - */ -async function checkImageReferencesInDocument(document: vscode.TextDocument, fetch: FetchLike): Promise { - if (!isHelmValuesFile(document)) { - return undefined; - } - - const version = document.version; - - let references: ImageReference[]; - try { - references = extractImageReferences(document.getText()); - } catch { - // A YAML syntax error is the YAML language service's diagnostic to - // raise, not this feature's — stay silent rather than compete with it. - return undefined; - } - - // A tagless reference resolves through the chart's `appVersion`, a later - // ticket's job, so there is nothing to ask a registry yet. It still comes - // through as a check: dropping it here is what made a real reference - // render nothing at all, which is indistinguishable from a broken tool. - const checks = await Promise.all( - references.map(async (reference) => ({ - reference, - verdict: - reference.tag === undefined - ? ({ kind: 'unverifiable', reason: 'no-tag' } as const) - : await checkImageExistence({ - repository: reference.repository.text, - tag: reference.tag.text, - fetch, - }), - })) - ); - - return { version, checks }; -} - -/** - * A document's checks, or none when the text has moved on since they were - * taken. The recorded offsets belong to the version that was checked; - * projecting them onto edited text would slide a checkmark onto whatever now - * sits at that offset. Nothing is the honest answer until the next check. - */ -function checksAsOf(checked: DocumentChecks, document: vscode.TextDocument): readonly ReferenceCheck[] { - return checked.version === document.version ? checked.checks : []; -} - -/** The diagnostics a document's checks call for. */ -function diagnosticsFor(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): vscode.Diagnostic[] { - const fileDiagnostics: vscode.Diagnostic[] = []; - - for (const { reference, verdict } of checks) { - // 'exists' and 'unverifiable' both produce no diagnostic. That an - // unverifiable verdict never renders as an error is the one invariant - // this feature must never break — an expired token or an unreachable - // registry must never look like a missing image. - if (verdict.kind === 'repository-not-found') { - fileDiagnostics.push( - new vscode.Diagnostic( - rangeOf(document, reference.repository.range), - `Repository '${verdict.repository}' not found.`, - vscode.DiagnosticSeverity.Error - ) - ); - } else if (verdict.kind === 'tag-not-found') { - fileDiagnostics.push( - new vscode.Diagnostic( - // A tag-not-found verdict can only come back for a reference that - // named a tag, so the fallback is unreachable; it exists so the - // type stays honest rather than being asserted away. - rangeOf(document, reference.tag?.range ?? reference.repository.range), - `Tag '${verdict.tag}' not found in '${verdict.repository}'.`, - vscode.DiagnosticSeverity.Error - ) - ); - } - } - - return fileDiagnostics; -} - -/** Which mark an outcome renders as. Exhaustive, so a new verdict kind fails the build. */ -function markKindOf(verdict: ReferenceVerdict): MarkKind { - switch (verdict.kind) { - case 'exists': - return 'verified'; - case 'repository-not-found': - case 'tag-not-found': - return 'missing'; - case 'unverifiable': - return 'unchecked'; - } -} - -/** - * The answering registry, named only when it differs from the host the file - * names, so the mark carries information instead of restating the line. - * Today it can never differ; the registry override set that makes it - * possible is a later ticket. - */ -function registrySuffixOf(reference: ImageReference, verdict: ReferenceVerdict): string { - if (verdict.kind !== 'exists' || verdict.registry === resolveExplicitHost(reference.repository.text)?.host) { - return ''; - } - - return ` ${verdict.registry}`; -} - -/** - * The marks a document's checks call for: exactly one per checked reference, - * whatever the outcome. Colour rides on each decoration rather than on the - * decoration type so that all three marks share one type, and one - * `setDecorations` call per editor still replaces the lot. - */ -function marksFor(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): vscode.DecorationOptions[] { - const decorations: vscode.DecorationOptions[] = []; - - for (const { reference, verdict } of checks) { - const { glyph, color } = MARKS[markKindOf(verdict)]; - - decorations.push({ - range: rangeOf(document, reference.repository.range), - renderOptions: { after: { contentText: ` ${glyph}${registrySuffixOf(reference, verdict)}`, color: new vscode.ThemeColor(color) } }, - }); - } - - return decorations; -} - -/** - * Re-applies each editor's stored marks. An editor showing a checked - * document always gets a `setDecorations` call, empty array included, so a - * reference whose outcome changed loses the mark it used to have. - */ -function applyMarks( - editors: readonly vscode.TextEditor[], - checksByDocument: ReadonlyMap, - decorationType: vscode.TextEditorDecorationType -): void { - for (const editor of editors) { - const checked = checksByDocument.get(editor.document.uri.toString()); - - if (checked === undefined) { - continue; - } - - try { - editor.setDecorations(decorationType, marksFor(editor.document, checksAsOf(checked, editor.document))); - } catch { - // An editor disposed between the check and this call throws here, and - // this runs while editors are being torn down. One dead editor must - // not cost every other visible editor its checkmarks. - continue; - } - } -} - -/** - * What hovering a position in a checked document reports: the registry that - * confirmed the reference, or why it could not be checked. A not-found - * verdict gets no hover — it already speaks through its diagnostic, and it - * names no registry to report. - */ -function hoverFor(document: vscode.TextDocument, position: vscode.Position, checks: readonly ReferenceCheck[]): vscode.Hover | undefined { - const offset = document.offsetAt(position); - const check = checks.find( - ({ reference }) => - containsOffset(reference.repository.range, offset) || (reference.tag !== undefined && containsOffset(reference.tag.range, offset)) - ); - - if (check === undefined) { - return undefined; - } - - const { verdict } = check; - - if (verdict.kind === 'exists') { - return new vscode.Hover(new vscode.MarkdownString(`Verified on \`${verdict.registry}\`.`)); - } - - if (verdict.kind === 'unverifiable') { - return new vscode.Hover(new vscode.MarkdownString(`Not verified: ${UNVERIFIABLE_REASON_TEXT[verdict.reason]}`)); - } - - return undefined; -} - -function containsOffset(range: SourceRange, offset: number): boolean { - return offset >= range.start && offset < range.end; -} - -function rangeOf(document: vscode.TextDocument, range: SourceRange): vscode.Range { - return new vscode.Range(document.positionAt(range.start), document.positionAt(range.end)); -} - export { activate, deactivate }; diff --git a/apps/vscode/src/hover.ts b/apps/vscode/src/hover.ts new file mode 100644 index 0000000..1a354a7 --- /dev/null +++ b/apps/vscode/src/hover.ts @@ -0,0 +1,44 @@ +import * as vscode from 'vscode'; +import type { ReferenceCheck, UncheckedReason } from './reference-check'; +import { containsOffset } from './source-range'; + +// A table rather than a switch, so adding a reason fails the build here +// instead of hovering with no explanation. +const UNCHECKED_REASON_TEXT: Record = { + 'no-tag': 'the reference names no tag to check.', + 'no-registry': 'the repository names no registry host.', + 'missing-credential': 'the registry requires credentials this extension cannot supply yet.', + 'network-error': 'the registry could not be reached.', + 'unexpected-response': 'the registry answered in a form this extension does not understand.', + 'malformed-reference': 'the tag is not a valid OCI tag.', +}; + +/** + * What hovering a checked reference reports. A not-found verdict gets no + * hover: it already speaks through its diagnostic, and it names no registry. + */ +function hoverFor(document: vscode.TextDocument, position: vscode.Position, checks: readonly ReferenceCheck[]): vscode.Hover | undefined { + const offset = document.offsetAt(position); + const check = checks.find( + ({ reference }) => + containsOffset(reference.repository.range, offset) || (reference.tag !== undefined && containsOffset(reference.tag.range, offset)) + ); + + if (check === undefined) { + return undefined; + } + + const { verdict } = check; + + if (verdict.kind === 'exists') { + return new vscode.Hover(new vscode.MarkdownString(`Verified on \`${verdict.registry}\`.`)); + } + + if (verdict.kind === 'unverifiable') { + return new vscode.Hover(new vscode.MarkdownString(`Not verified: ${UNCHECKED_REASON_TEXT[verdict.reason]}`)); + } + + return undefined; +} + +export { hoverFor }; diff --git a/apps/vscode/src/marks.ts b/apps/vscode/src/marks.ts new file mode 100644 index 0000000..9499fcc --- /dev/null +++ b/apps/vscode/src/marks.ts @@ -0,0 +1,96 @@ +import * as vscode from 'vscode'; +import type { ImageReference } from 'helm'; +import { resolveExplicitHost } from 'oci-registry'; +import { checksAsOf, type DocumentChecks, type ReferenceCheck, type ReferenceVerdict } from './reference-check'; +import { rangeOf } from './source-range'; + +type MarkKind = 'verified' | 'missing' | 'unchecked'; + +// `unchecked` is a muted question mark rather than a cross on purpose: it +// reports a fact about the developer's machine, not a defect in the file, +// and styling it as a failure is how a linter earns being switched off. +const MARKS: Record = { + verified: { glyph: '✓', color: 'charts.green' }, + missing: { glyph: '✗', color: 'errorForeground' }, + unchecked: { glyph: '?', color: 'descriptionForeground' }, +}; + +/** Exhaustive, so a new verdict kind fails the build here. */ +function markKindOf(verdict: ReferenceVerdict): MarkKind { + switch (verdict.kind) { + case 'exists': + return 'verified'; + case 'repository-not-found': + case 'tag-not-found': + return 'missing'; + case 'unverifiable': + return 'unchecked'; + } +} + +/** + * The answering registry, named only when it differs from the host the file + * names, so the mark carries information instead of restating the line. + */ +function registrySuffixOf(reference: ImageReference, verdict: ReferenceVerdict): string { + if (verdict.kind !== 'exists' || verdict.registry === resolveExplicitHost(reference.repository.text)?.host) { + return ''; + } + + return ` ${verdict.registry}`; +} + +/** + * One mark per checked reference, whatever the outcome. Glyph and colour + * both ride on the decoration rather than the type, so all three marks share + * one type and one `setDecorations` call per editor replaces the lot. + */ +function marksFor(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): vscode.DecorationOptions[] { + const decorations: vscode.DecorationOptions[] = []; + + for (const { reference, verdict } of checks) { + const { glyph, color } = MARKS[markKindOf(verdict)]; + + decorations.push({ + range: rangeOf(document, reference.repository.range), + renderOptions: { after: { contentText: ` ${glyph}${registrySuffixOf(reference, verdict)}`, color: new vscode.ThemeColor(color) } }, + }); + } + + return decorations; +} + +function createMarkDecorationType(): vscode.TextEditorDecorationType { + return vscode.window.createTextEditorDecorationType({ after: { margin: '0 0 0 0.5em' } }); +} + +/** + * Re-applies each editor's stored marks. An editor showing a checked + * document always gets a `setDecorations` call, empty array included, so a + * reference whose outcome changed loses the mark it used to have. + */ +function applyMarks( + editors: readonly vscode.TextEditor[], + checksByDocument: ReadonlyMap, + decorationType: vscode.TextEditorDecorationType +): void { + for (const editor of editors) { + const checked = checksByDocument.get(editor.document.uri.toString()); + + if (checked === undefined) { + continue; + } + + try { + editor.setDecorations(decorationType, marksFor(editor.document, checksAsOf(checked, editor.document))); + } catch { + // This runs while editors are being torn down, and `setDecorations` + // throws on a disposed one. One dead editor must not cost every other + // visible editor its marks. + continue; + } + } +} + +export { applyMarks, createMarkDecorationType, marksFor }; +export type { MarkKind }; diff --git a/apps/vscode/src/reference-check.ts b/apps/vscode/src/reference-check.ts new file mode 100644 index 0000000..aa8282e --- /dev/null +++ b/apps/vscode/src/reference-check.ts @@ -0,0 +1,88 @@ +import type * as vscode from 'vscode'; +import { extractImageReferences, type ImageReference } from 'helm'; +import { checkImageExistence, type FetchLike, type ImageVerdict, type UnverifiableReason } from 'oci-registry'; + +// Matching any YAML file beneath a chart directory, and excluding that +// chart's templates directory, is Helm chart-context knowledge this ticket +// doesn't implement yet. +const VALUES_FILE_NAME_PATTERN = /^values\.ya?ml$/i; + +/** Every reason the registry package reports, plus the ones settled before asking it. */ +type UncheckedReason = UnverifiableReason | 'no-tag'; + +type ReferenceVerdict = Exclude | { readonly kind: 'unverifiable'; readonly reason: UncheckedReason }; + +/** + * One checked reference, projected onto all three surfaces, so diagnostics, + * marks, and hovers can never disagree about a reference. + */ +interface ReferenceCheck { + readonly reference: ImageReference; + readonly verdict: ReferenceVerdict; +} + +/** A document's checks, tagged with the document version they describe. */ +interface DocumentChecks { + readonly version: number; + readonly checks: readonly ReferenceCheck[]; +} + +function isHelmValuesFile(document: vscode.TextDocument): boolean { + if (document.languageId !== 'yaml') { + return false; + } + + const fileName = document.uri.path.split('/').pop() ?? ''; + + return VALUES_FILE_NAME_PATTERN.test(fileName); +} + +/** + * Checks a document's image references. `undefined` means this feature has + * nothing to say about the document at all, which is not the same as a + * checked document that produced no findings: the caller replaces a + * document's diagnostics and marks only when it gets checks back. + */ +async function checkImageReferencesInDocument(document: vscode.TextDocument, fetch: FetchLike): Promise { + if (!isHelmValuesFile(document)) { + return undefined; + } + + const version = document.version; + + let references: ImageReference[]; + try { + references = extractImageReferences(document.getText()); + } catch { + // A YAML syntax error is the YAML language service's to raise, not this + // feature's. Stay silent rather than compete with it. + return undefined; + } + + // A tagless reference has nothing to ask a registry until `appVersion` + // resolution lands, but it still comes through as a check. Dropping it is + // what made real references render nothing, which reads as a broken tool. + const checks = await Promise.all( + references.map(async (reference) => ({ + reference, + verdict: + reference.tag === undefined + ? ({ kind: 'unverifiable', reason: 'no-tag' } as const) + : await checkImageExistence({ repository: reference.repository.text, tag: reference.tag.text, fetch }), + })) + ); + + return { version, checks }; +} + +/** + * A document's checks, or none once the text has moved on. Recorded offsets + * belong to the version that was checked, so projecting them onto edited + * text would slide a mark onto whatever now sits at that offset. + */ +function checksAsOf(checked: DocumentChecks, document: vscode.TextDocument): readonly ReferenceCheck[] { + return checked.version === document.version ? checked.checks : []; +} + +export { checkImageReferencesInDocument, checksAsOf, isHelmValuesFile }; +export type { DocumentChecks, ReferenceCheck, ReferenceVerdict, UncheckedReason }; diff --git a/apps/vscode/src/source-range.ts b/apps/vscode/src/source-range.ts new file mode 100644 index 0000000..c5eca8d --- /dev/null +++ b/apps/vscode/src/source-range.ts @@ -0,0 +1,12 @@ +import * as vscode from 'vscode'; +import type { SourceRange } from 'helm'; + +function rangeOf(document: vscode.TextDocument, range: SourceRange): vscode.Range { + return new vscode.Range(document.positionAt(range.start), document.positionAt(range.end)); +} + +function containsOffset(range: SourceRange, offset: number): boolean { + return offset >= range.start && offset < range.end; +} + +export { containsOffset, rangeOf }; From 1cd9940761c62d79bcddde15754c335e6082affb Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Tue, 15 Sep 2026 15:10:04 +0300 Subject: [PATCH 6/7] test(vscode): split the test file to mirror the source modules 481 lines in one file became 640 across seven, and 25 tests became 33. Each module's rules are now asserted against that module directly instead of through `activate` and a fetch fake, which is what made the single file long. `extension.test.ts` keeps only what it is the right seam for: that the surfaces are registered and that an opened values file drives all three. `createFakeDocument` and `fakeFetchResponse` move to `test/`, alongside the vscode stub, since four test files now need them. One test pins behaviour rather than intent. `extractImageReferences` uses the yaml package's `parseDocument`, which collects syntax errors on the document instead of throwing, so a malformed values file yields zero references rather than reaching the catch in `checkImageReferencesInDocument`. The test says so, with a comment explaining why the result is empty checks. --- apps/vscode/src/diagnostics.test.ts | 82 ++++++ apps/vscode/src/extension.test.ts | 354 ++---------------------- apps/vscode/src/hover.test.ts | 115 ++++++++ apps/vscode/src/marks.test.ts | 123 ++++++++ apps/vscode/src/reference-check.test.ts | 99 +++++++ apps/vscode/test/fake-document.ts | 36 +++ apps/vscode/test/fake-fetch.ts | 14 + 7 files changed, 491 insertions(+), 332 deletions(-) create mode 100644 apps/vscode/src/diagnostics.test.ts create mode 100644 apps/vscode/src/hover.test.ts create mode 100644 apps/vscode/src/marks.test.ts create mode 100644 apps/vscode/src/reference-check.test.ts create mode 100644 apps/vscode/test/fake-document.ts create mode 100644 apps/vscode/test/fake-fetch.ts diff --git a/apps/vscode/src/diagnostics.test.ts b/apps/vscode/src/diagnostics.test.ts new file mode 100644 index 0000000..1946656 --- /dev/null +++ b/apps/vscode/src/diagnostics.test.ts @@ -0,0 +1,82 @@ +import * as vscode from 'vscode'; +import { describe, expect, it } from 'vitest'; +import type { SourceRange } from 'helm'; +import { createFakeDocument } from '../test/fake-document'; +import { diagnosticsFor } from './diagnostics'; +import type { ReferenceCheck, ReferenceVerdict, UncheckedReason } from './reference-check'; + +const REPOSITORY = 'registry.example.com/svc'; +const TAG = '1.0'; +const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: "${TAG}"`, ''].join('\n'); + +// A record rather than a list, so a new reason fails the build here instead +// of going untested. +const UNCHECKED_REASONS: Record = { + 'no-tag': true, + 'no-registry': true, + 'missing-credential': true, + 'network-error': true, + 'unexpected-response': true, + 'malformed-reference': true, +}; + +/** The source range of `text`'s first occurrence in {@link VALUES_YAML}. */ +function rangeOfText(text: string): SourceRange { + const start = VALUES_YAML.indexOf(text); + + return { start, end: start + text.length }; +} + +/** The document range covering `text`'s first occurrence in {@link VALUES_YAML}. */ +function documentRangeOfText(document: vscode.TextDocument, text: string): vscode.Range { + const { start, end } = rangeOfText(text); + + return new vscode.Range(document.positionAt(start), document.positionAt(end)); +} + +/** A check over the single reference in {@link VALUES_YAML}, carrying that file's real offsets. */ +function createCheck(verdict: ReferenceVerdict): ReferenceCheck { + return { + reference: { + repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, + tag: { text: TAG, range: rangeOfText(TAG) }, + }, + verdict, + }; +} + +describe('diagnostics', () => { + it('should report an error naming the missing repository, positioned on the repository value', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const fileDiagnostics = diagnosticsFor(document, [createCheck({ kind: 'repository-not-found', repository: REPOSITORY })]); + + expect(fileDiagnostics).toHaveLength(1); + expect(fileDiagnostics[0]?.severity).toBe(vscode.DiagnosticSeverity.Error); + expect(fileDiagnostics[0]?.message).toContain(REPOSITORY); + expect(fileDiagnostics[0]?.range).toEqual(documentRangeOfText(document, REPOSITORY)); + }); + + it('should report an error naming the missing tag, positioned on the tag value', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const fileDiagnostics = diagnosticsFor(document, [createCheck({ kind: 'tag-not-found', repository: REPOSITORY, tag: TAG })]); + + expect(fileDiagnostics).toHaveLength(1); + expect(fileDiagnostics[0]?.severity).toBe(vscode.DiagnosticSeverity.Error); + expect(fileDiagnostics[0]?.message).toContain(TAG); + expect(fileDiagnostics[0]?.range).toEqual(documentRangeOfText(document, TAG)); + }); + + it('should report nothing for a reference that exists', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + + expect(diagnosticsFor(document, [createCheck({ kind: 'exists', registry: 'registry.example.com' })])).toEqual([]); + }); + + it('should report nothing for any unverifiable reason, since an unreachable registry is not a missing image', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + + for (const reason of Object.keys(UNCHECKED_REASONS) as UncheckedReason[]) { + expect(diagnosticsFor(document, [createCheck({ kind: 'unverifiable', reason })])).toEqual([]); + } + }); +}); diff --git a/apps/vscode/src/extension.test.ts b/apps/vscode/src/extension.test.ts index 3aa7ddf..1f1e3c4 100644 --- a/apps/vscode/src/extension.test.ts +++ b/apps/vscode/src/extension.test.ts @@ -1,5 +1,7 @@ import * as vscode from 'vscode'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { bumpVersion, createFakeDocument } from '../test/fake-document'; +import { fakeFetchResponse } from '../test/fake-fetch'; import { createTextEditorStub, emitDidChangeVisibleTextEditors, @@ -11,67 +13,9 @@ import { } from '../test/vscode-stub'; import { activate, deactivate } from './extension'; -/** Builds a fake `vscode.TextDocument`, with a real `positionAt`/`offsetAt` pair so range assertions are exact. */ -function createFakeDocument(path: string, text: string, languageId = 'yaml'): vscode.TextDocument { - return { - uri: { path, toString: () => path }, - languageId, - version: 1, - getText: () => text, - positionAt: (offset: number) => { - const before = text.slice(0, offset); - const lines = before.split('\n'); - const line = lines.length - 1; - const character = lines[lines.length - 1]?.length ?? 0; - - return new vscode.Position(line, character); - }, - offsetAt: (position: vscode.Position) => { - const lines = text.split('\n'); - let offset = 0; - - for (let line = 0; line < position.line; line += 1) { - offset += (lines[line]?.length ?? 0) + '\n'.length; - } - - return offset + position.character; - }, - } as unknown as vscode.TextDocument; -} - -const VALUES_YAML = ['image:', ' repository: docker.io/library/nginx', ' tag: 1.19', ''].join('\n'); - -/** Three references whose outcomes differ, so one file exercises all three marks at once. */ -const MIXED_VALUES_YAML = [ - 'good:', - ' image:', - ' repository: registry.example.com/good', - ' tag: "1.0"', - 'bad:', - ' image:', - ' repository: registry.example.com/bad', - ' tag: "2.0"', - 'unknown:', - ' image:', - ' repository: registry.example.com/unknown', - ' tag: "3.0"', - '', -].join('\n'); - -/** A canned fetch `Response`-shaped object for the injected fetch fake. */ -function fakeFetchResponse(status: number, body: unknown = {}): { status: number; ok: boolean; json: () => Promise } { - return { - status, - ok: status >= 200 && status < 300, - // eslint-disable-next-line @typescript-eslint/promise-function-async -- trivial canned response, nothing to await - json: () => Promise.resolve(body), - }; -} - -/** Simulates an edit: VS Code bumps a document's `version` on every change. */ -function bumpVersion(document: vscode.TextDocument): void { - (document as { version: number }).version += 1; -} +const REPOSITORY = 'docker.io/library/nginx'; +const TAG = '1.19'; +const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: ${TAG}`, ''].join('\n'); /** Stages `document` as the only visible editor, then fires the open event. */ async function openInVisibleEditor(document: vscode.TextDocument): Promise { @@ -100,17 +44,6 @@ function hoverAt(document: vscode.TextDocument, offset: number): vscode.Hover | return getRegisteredHoverProvider()?.provideHover(document, document.positionAt(offset)) as vscode.Hover | undefined; } -/** The plain text of a hover's single content entry. */ -function getHoverText(hover: vscode.Hover | undefined): string { - const [content] = hover?.contents ?? []; - - if (content === undefined || typeof content === 'string') { - throw new Error('expected a hover carrying one MarkdownString'); - } - - return content.value; -} - /** Asserts a diagnostics `.set()` call carried exactly one diagnostic, and returns it. */ function getSingleDiagnostic(fileDiagnostics: readonly vscode.Diagnostic[] | undefined): vscode.Diagnostic { expect(fileDiagnostics).toHaveLength(1); @@ -155,179 +88,46 @@ describe('extension', () => { expect(() => deactivate()).not.toThrow(); }); - it('should set no diagnostics when the referenced image exists', async () => { - const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - await emitDidOpenTextDocument(document); - - const collection = getLastDiagnosticCollection(); - - expect(collection?.set).toHaveBeenCalledWith(document.uri, []); - }); - - it('should set an error diagnostic naming the missing tag, positioned on the tag value, when the tag does not exist', async () => { - const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(404, { errors: [{ code: 'MANIFEST_UNKNOWN' }] })); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - await emitDidOpenTextDocument(document); - - const collection = getLastDiagnosticCollection(); - const diagnostic = getSingleDiagnostic(collection?.set.mock.calls[0]?.[1] as vscode.Diagnostic[] | undefined); - - expect(diagnostic.severity).toBe(vscode.DiagnosticSeverity.Error); - expect(diagnostic.message).toContain('1.19'); - - const tagStart = VALUES_YAML.indexOf('1.19'); - - expect(diagnostic.range).toEqual(new vscode.Range(document.positionAt(tagStart), document.positionAt(tagStart + '1.19'.length))); - }); - - it('should set an error diagnostic naming the missing repository when the repository does not exist', async () => { - const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(404, { errors: [{ code: 'NAME_UNKNOWN' }] })); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - await emitDidOpenTextDocument(document); - - const collection = getLastDiagnosticCollection(); - const diagnostic = getSingleDiagnostic(collection?.set.mock.calls[0]?.[1] as vscode.Diagnostic[] | undefined); - - expect(diagnostic.severity).toBe(vscode.DiagnosticSeverity.Error); - expect(diagnostic.message).toContain('docker.io/library/nginx'); - - const repositoryStart = VALUES_YAML.indexOf('docker.io/library/nginx'); - - expect(diagnostic.range).toEqual( - new vscode.Range(document.positionAt(repositoryStart), document.positionAt(repositoryStart + 'docker.io/library/nginx'.length)) - ); - }); - - it('should set no diagnostics when the registry is unreachable, since unverifiable never renders as an error', async () => { - const fetch = vi.fn().mockRejectedValue(new Error('getaddrinfo ENOTFOUND')); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - await emitDidOpenTextDocument(document); - - const collection = getLastDiagnosticCollection(); - - expect(collection?.set).toHaveBeenCalledWith(document.uri, []); - }); - - it('should render a checkmark on the repository of a reference that exists', async () => { - const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - const editor = await openInVisibleEditor(document); - const decorations = getLastDecorations(editor); - - expect(decorations).toHaveLength(1); - expect(decorations[0]?.renderOptions?.after?.contentText).toContain('✓'); - - const repositoryStart = VALUES_YAML.indexOf('docker.io/library/nginx'); + it('should register a yaml hover provider on activate', () => { + activate(context, { fetch: vi.fn() }); - expect(decorations[0]?.range).toEqual( - new vscode.Range(document.positionAt(repositoryStart), document.positionAt(repositoryStart + 'docker.io/library/nginx'.length)) - ); + expect(vscode.languages.registerHoverProvider).toHaveBeenCalledWith({ language: 'yaml' }, expect.anything()); }); - it('should omit the registry name from the checkmark when it matches the host the file names', async () => { - const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - const editor = await openInVisibleEditor(document); + it('should create one mark decoration type on activate', () => { + activate(context, { fetch: vi.fn() }); - expect(getLastDecorations(editor)[0]?.renderOptions?.after?.contentText).toBe(' ✓'); + expect(vscode.window.createTextEditorDecorationType).toHaveBeenCalledWith({ after: { margin: '0 0 0 0.5em' } }); }); - it('should render a cross alongside the diagnostic when the tag does not exist', async () => { + it('should set both diagnostics and marks when a values file opens', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(404, { errors: [{ code: 'MANIFEST_UNKNOWN' }] })); activate(context, { fetch }); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); const editor = await openInVisibleEditor(document); - const [mark] = getLastDecorations(editor); - - expect(mark?.renderOptions?.after?.contentText).toBe(' ✗'); - expect(mark?.renderOptions?.after?.color).toEqual(new vscode.ThemeColor('errorForeground')); - const diagnostic = getSingleDiagnostic(getLastDiagnosticCollection()?.set.mock.calls[0]?.[1] as vscode.Diagnostic[] | undefined); - expect(diagnostic.message).toContain('1.19'); - }); - - it('should render a muted question mark and no diagnostic when the reference is unverifiable', async () => { - const fetch = vi.fn().mockRejectedValue(new Error('getaddrinfo ENOTFOUND')); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - const editor = await openInVisibleEditor(document); - const [mark] = getLastDecorations(editor); - - expect(mark?.renderOptions?.after?.contentText).toBe(' ?'); - expect(mark?.renderOptions?.after?.color).toEqual(new vscode.ThemeColor('descriptionForeground')); - expect(getLastDiagnosticCollection()?.set).toHaveBeenCalledWith(document.uri, []); - }); - - it('should report the registry that answered when hovering a verified reference', async () => { - const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - await openInVisibleEditor(document); - - const hoverText = getHoverText(hoverAt(document, VALUES_YAML.indexOf('docker.io/library/nginx'))); - - expect(hoverText).toContain('docker.io'); - expect(hoverText).toContain('Verified'); - }); - - it('should report why a reference could not be verified when hovering an unverifiable one', async () => { - const fetch = vi.fn().mockRejectedValue(new Error('getaddrinfo ENOTFOUND')); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - await openInVisibleEditor(document); - - const hoverText = getHoverText(hoverAt(document, VALUES_YAML.indexOf('1.19'))); - - expect(hoverText).toContain('Not verified'); - expect(hoverText).toContain('could not be reached'); - }); - - it('should provide no hover for a reference that does not exist, which already speaks through its diagnostic', async () => { - const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(404, { errors: [{ code: 'MANIFEST_UNKNOWN' }] })); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - await openInVisibleEditor(document); - - expect(hoverAt(document, VALUES_YAML.indexOf('1.19'))).toBeUndefined(); + expect(diagnostic.severity).toBe(vscode.DiagnosticSeverity.Error); + expect(diagnostic.message).toContain(TAG); + expect(getLastDecorations(editor)[0]?.renderOptions?.after?.contentText).toBe(' ✗'); }); - it('should provide no hover outside any checked reference', async () => { + it('should hover a checked reference, and stop once the document has been edited past the check', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); activate(context, { fetch }); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); await openInVisibleEditor(document); - expect(hoverAt(document, VALUES_YAML.indexOf('image:'))).toBeUndefined(); - }); + expect(hoverAt(document, VALUES_YAML.indexOf(REPOSITORY))).toBeDefined(); - it('should create one mark decoration type and register a yaml hover provider on activate', () => { - activate(context, { fetch: vi.fn() }); + bumpVersion(document); - expect(vscode.window.createTextEditorDecorationType).toHaveBeenCalledWith({ after: { margin: '0 0 0 0.5em' } }); - expect(vscode.languages.registerHoverProvider).toHaveBeenCalledWith({ language: 'yaml' }, expect.anything()); + expect(hoverAt(document, VALUES_YAML.indexOf(REPOSITORY))).toBeUndefined(); }); - it('should re-apply checkmarks to an editor that becomes visible after the document was checked', async () => { + it('should re-apply marks to an editor that becomes visible after the document was checked', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); activate(context, { fetch }); @@ -342,33 +142,6 @@ describe('extension', () => { expect(getLastDecorations(editor)[0]?.renderOptions?.after?.contentText).toBe(' ✓'); }); - it('should drop checkmarks rather than re-project them onto text edited since the check', async () => { - const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - await openInVisibleEditor(document); - - bumpVersion(document); - - const editor = createTextEditorStub(document); - await emitDidChangeVisibleTextEditors([editor]); - - expect(getLastDecorations(editor)).toEqual([]); - }); - - it('should provide no hover once the document has been edited past the checked version', async () => { - const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - await openInVisibleEditor(document); - - bumpVersion(document); - - expect(hoverAt(document, VALUES_YAML.indexOf('docker.io/library/nginx'))).toBeUndefined(); - }); - it('should survive an editor disposed mid-check, since an unhandled rejection kills the extension host', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); activate(context, { fetch }); @@ -387,94 +160,11 @@ describe('extension', () => { expect(getLastDiagnosticCollection()?.set).toHaveBeenCalledWith(document.uri, []); }); - it('should still decorate the surviving editors when one of them was disposed', async () => { - const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - await openInVisibleEditor(document); - - const disposedEditor: TextEditorStub = { - document, - setDecorations: vi.fn(() => { - throw new Error('TextEditor#setDecorations: editor disposed'); - }), - }; - const survivingEditor = createTextEditorStub(document); - - await emitDidChangeVisibleTextEditors([disposedEditor, survivingEditor]); - - expect(getLastDecorations(survivingEditor)[0]?.renderOptions?.after?.contentText).toBe(' ✓'); - }); - - it('should mark every reference in one pass, whatever each outcome was', async () => { - // eslint-disable-next-line @typescript-eslint/promise-function-async -- canned per-URL responses, nothing to await - const fetch = vi.fn((url: string) => { - if (url.includes('/good/')) { - return Promise.resolve(fakeFetchResponse(200)); - } - - if (url.includes('/bad/')) { - return Promise.resolve(fakeFetchResponse(404, { errors: [{ code: 'MANIFEST_UNKNOWN' }] })); - } - - return Promise.reject(new Error('getaddrinfo ENOTFOUND')); - }); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', MIXED_VALUES_YAML); - const editor = await openInVisibleEditor(document); - const marks = getLastDecorations(editor); - - expect(marks.map((mark) => mark.renderOptions?.after?.contentText)).toEqual([' ✓', ' ✗', ' ?']); - expect(marks.map((mark) => mark.renderOptions?.after?.color)).toEqual([ - new vscode.ThemeColor('charts.green'), - new vscode.ThemeColor('errorForeground'), - new vscode.ThemeColor('descriptionForeground'), - ]); - }); - - it('should mark a tagless reference as unchecked instead of dropping it silently', async () => { - const fetch = vi.fn(); - activate(context, { fetch }); - - const taglessYaml = ['image:', ' repository: registry.example.com/svc', ' pullPolicy: IfNotPresent', ''].join('\n'); - const document = createFakeDocument('/repo/chart/values.yaml', taglessYaml); - const editor = await openInVisibleEditor(document); - const [mark] = getLastDecorations(editor); - - expect(fetch).not.toHaveBeenCalled(); - expect(mark?.renderOptions?.after?.contentText).toBe(' ?'); - expect(getLastDiagnosticCollection()?.set).toHaveBeenCalledWith(document.uri, []); - }); - - it('should explain a tagless reference on hover', async () => { - const fetch = vi.fn(); - activate(context, { fetch }); - - const taglessYaml = ['image:', ' repository: registry.example.com/svc', ' pullPolicy: IfNotPresent', ''].join('\n'); - const document = createFakeDocument('/repo/chart/values.yaml', taglessYaml); - await openInVisibleEditor(document); - - expect(getHoverText(hoverAt(document, taglessYaml.indexOf('registry.example.com/svc')))).toContain('no tag'); - }); - - it('should ignore a document that is not the conventional values file name', async () => { + it('should issue no request for a document that is not a values file', async () => { const fetch = vi.fn(); activate(context, { fetch }); - const document = createFakeDocument('/repo/chart/deployment.yaml', VALUES_YAML); - await emitDidOpenTextDocument(document); - - expect(fetch).not.toHaveBeenCalled(); - }); - - it('should ignore a document whose language is not yaml', async () => { - const fetch = vi.fn(); - activate(context, { fetch }); - - const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML, 'plaintext'); - await emitDidOpenTextDocument(document); + await emitDidOpenTextDocument(createFakeDocument('/repo/chart/deployment.yaml', VALUES_YAML)); expect(fetch).not.toHaveBeenCalled(); }); diff --git a/apps/vscode/src/hover.test.ts b/apps/vscode/src/hover.test.ts new file mode 100644 index 0000000..3d97057 --- /dev/null +++ b/apps/vscode/src/hover.test.ts @@ -0,0 +1,115 @@ +import type * as vscode from 'vscode'; +import { describe, expect, it } from 'vitest'; +import type { SourceRange } from 'helm'; +import { createFakeDocument } from '../test/fake-document'; +import { hoverFor } from './hover'; +import type { ReferenceCheck, ReferenceVerdict, UncheckedReason } from './reference-check'; + +const REPOSITORY = 'registry.example.com/svc'; +const TAG = '1.0'; +const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: "${TAG}"`, ''].join('\n'); + +const VERIFIED_VERDICT: ReferenceVerdict = { kind: 'exists', registry: 'mirror.example.com' }; + +// The sentence each reason owes the reader, as a record so a new reason fails +// the build here instead of hovering with someone else's explanation. +const UNCHECKED_REASON_SENTENCES: Record = { + 'no-tag': 'the reference names no tag to check.', + 'no-registry': 'the repository names no registry host.', + 'missing-credential': 'the registry requires credentials this extension cannot supply yet.', + 'network-error': 'the registry could not be reached.', + 'unexpected-response': 'the registry answered in a form this extension does not understand.', + 'malformed-reference': 'the tag is not a valid OCI tag.', +}; + +/** The source range of `text`'s first occurrence in {@link VALUES_YAML}. */ +function rangeOfText(text: string): SourceRange { + const start = VALUES_YAML.indexOf(text); + + return { start, end: start + text.length }; +} + +/** A check over the single reference in {@link VALUES_YAML}, carrying that file's real offsets. */ +function createCheck(verdict: ReferenceVerdict): ReferenceCheck { + return { + reference: { + repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, + tag: { text: TAG, range: rangeOfText(TAG) }, + }, + verdict, + }; +} + +/** A check over the same reference written without a tag, which is what a `no-tag` verdict comes from. */ +function createTaglessCheck(verdict: ReferenceVerdict): ReferenceCheck { + return { + reference: { + repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, + tag: undefined, + }, + verdict, + }; +} + +/** The hover at the first character of `text`'s occurrence in {@link VALUES_YAML}. */ +function hoverAtText(document: vscode.TextDocument, checks: readonly ReferenceCheck[], text: string): vscode.Hover | undefined { + return hoverFor(document, document.positionAt(VALUES_YAML.indexOf(text)), checks); +} + +/** The plain text of a hover's single content entry. */ +function getHoverText(hover: vscode.Hover | undefined): string { + const [content] = hover?.contents ?? []; + + if (content === undefined || typeof content === 'string') { + throw new Error('expected a hover carrying one MarkdownString'); + } + + return content.value; +} + +describe('hover', () => { + it('should report the registry that answered for a verified reference', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const hoverText = getHoverText(hoverAtText(document, [createCheck(VERIFIED_VERDICT)], REPOSITORY)); + + expect(hoverText).toContain('Verified'); + expect(hoverText).toContain('mirror.example.com'); + }); + + it('should render its own sentence for every reason a reference went unverified', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + + for (const [reason, sentence] of Object.entries(UNCHECKED_REASON_SENTENCES) as [UncheckedReason, string][]) { + const hoverText = getHoverText(hoverAtText(document, [createCheck({ kind: 'unverifiable', reason })], REPOSITORY)); + + expect(hoverText).toBe(`Not verified: ${sentence}`); + } + }); + + it('should explain a tagless reference, which carries no tag to hover in the first place', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const check = createTaglessCheck({ kind: 'unverifiable', reason: 'no-tag' }); + + expect(getHoverText(hoverAtText(document, [check], REPOSITORY))).toContain('no tag'); + }); + + it('should provide no hover for a reference that does not exist, which already speaks through its diagnostic', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + + expect(hoverAtText(document, [createCheck({ kind: 'repository-not-found', repository: REPOSITORY })], REPOSITORY)).toBeUndefined(); + expect(hoverAtText(document, [createCheck({ kind: 'tag-not-found', repository: REPOSITORY, tag: TAG })], TAG)).toBeUndefined(); + }); + + it('should provide no hover outside any checked reference', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + + expect(hoverAtText(document, [createCheck(VERIFIED_VERDICT)], 'image:')).toBeUndefined(); + }); + + it('should resolve a position inside the tag to the same reference as one inside the repository', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const checks = [createCheck(VERIFIED_VERDICT)]; + + expect(getHoverText(hoverAtText(document, checks, TAG))).toBe(getHoverText(hoverAtText(document, checks, REPOSITORY))); + }); +}); diff --git a/apps/vscode/src/marks.test.ts b/apps/vscode/src/marks.test.ts new file mode 100644 index 0000000..6ece276 --- /dev/null +++ b/apps/vscode/src/marks.test.ts @@ -0,0 +1,123 @@ +import * as vscode from 'vscode'; +import { describe, expect, it, vi } from 'vitest'; +import type { SourceRange } from 'helm'; +import { bumpVersion, createFakeDocument } from '../test/fake-document'; +import { createTextEditorStub, type TextEditorStub } from '../test/vscode-stub'; +import { applyMarks, createMarkDecorationType, marksFor } from './marks'; +import type { DocumentChecks, ReferenceCheck, ReferenceVerdict } from './reference-check'; + +const REPOSITORY = 'registry.example.com/svc'; +const TAG = '1.0'; +const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: "${TAG}"`, ''].join('\n'); + +const VERIFIED_VERDICT: ReferenceVerdict = { kind: 'exists', registry: 'registry.example.com' }; +const MISSING_VERDICT: ReferenceVerdict = { kind: 'tag-not-found', repository: REPOSITORY, tag: TAG }; +const UNCHECKED_VERDICT: ReferenceVerdict = { kind: 'unverifiable', reason: 'network-error' }; + +/** The source range of `text`'s first occurrence in {@link VALUES_YAML}. */ +function rangeOfText(text: string): SourceRange { + const start = VALUES_YAML.indexOf(text); + + return { start, end: start + text.length }; +} + +/** A check over the single reference in {@link VALUES_YAML}, carrying that file's real offsets. */ +function createCheck(verdict: ReferenceVerdict): ReferenceCheck { + return { + reference: { + repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, + tag: { text: TAG, range: rangeOfText(TAG) }, + }, + verdict, + }; +} + +/** The stub editors, as the editor list `applyMarks` takes. */ +function asEditors(editors: readonly TextEditorStub[]): readonly vscode.TextEditor[] { + return editors as unknown as readonly vscode.TextEditor[]; +} + +/** One document's stored checks, tagged with the version it currently has. */ +function createChecksByDocument(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): Map { + return new Map([[document.uri.toString(), { version: document.version, checks }]]); +} + +/** An editor that throws the way a disposed one does. */ +function createDisposedEditorStub(document: vscode.TextDocument): TextEditorStub { + return { + document, + setDecorations: vi.fn(() => { + throw new Error('TextEditor#setDecorations: editor disposed'); + }), + }; +} + +describe('marks', () => { + it('should produce one mark per check, whatever each outcome was', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const checks = [createCheck(VERIFIED_VERDICT), createCheck(MISSING_VERDICT), createCheck(UNCHECKED_VERDICT)]; + + expect(marksFor(document, checks)).toHaveLength(checks.length); + }); + + it('should render its own glyph and colour for each of verified, missing and unchecked', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const marks = marksFor(document, [createCheck(VERIFIED_VERDICT), createCheck(MISSING_VERDICT), createCheck(UNCHECKED_VERDICT)]); + + expect(marks.map((mark) => mark.renderOptions?.after?.contentText)).toEqual([' ✓', ' ✗', ' ?']); + expect(marks.map((mark) => mark.renderOptions?.after?.color)).toEqual([ + new vscode.ThemeColor('charts.green'), + new vscode.ThemeColor('errorForeground'), + new vscode.ThemeColor('descriptionForeground'), + ]); + }); + + it('should name the answering registry only when it differs from the host the file names', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const [namedHost] = marksFor(document, [createCheck(VERIFIED_VERDICT)]); + const [otherHost] = marksFor(document, [createCheck({ kind: 'exists', registry: 'mirror.example.com' })]); + + expect(namedHost?.renderOptions?.after?.contentText).toBe(' ✓'); + expect(otherHost?.renderOptions?.after?.contentText).toBe(' ✓ mirror.example.com'); + }); + + it('should place the mark on the repository value', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const [mark] = marksFor(document, [createCheck(VERIFIED_VERDICT)]); + const { start, end } = rangeOfText(REPOSITORY); + + expect(mark?.range).toEqual(new vscode.Range(document.positionAt(start), document.positionAt(end))); + }); + + it('should leave an editor whose document has no stored checks alone', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const editor = createTextEditorStub(document); + + applyMarks(asEditors([editor]), new Map(), createMarkDecorationType()); + + expect(editor.setDecorations).not.toHaveBeenCalled(); + }); + + it('should still decorate the surviving editors when one of them was disposed', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const checks = [createCheck(VERIFIED_VERDICT)]; + const decorationType = createMarkDecorationType(); + const survivingEditor = createTextEditorStub(document); + + applyMarks(asEditors([createDisposedEditorStub(document), survivingEditor]), createChecksByDocument(document, checks), decorationType); + + expect(survivingEditor.setDecorations).toHaveBeenCalledWith(decorationType, marksFor(document, checks)); + }); + + it('should drop marks rather than re-project them onto text edited since the check', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const checksByDocument = createChecksByDocument(document, [createCheck(VERIFIED_VERDICT)]); + const decorationType = createMarkDecorationType(); + const editor = createTextEditorStub(document); + + bumpVersion(document); + applyMarks(asEditors([editor]), checksByDocument, decorationType); + + expect(editor.setDecorations).toHaveBeenCalledWith(decorationType, []); + }); +}); diff --git a/apps/vscode/src/reference-check.test.ts b/apps/vscode/src/reference-check.test.ts new file mode 100644 index 0000000..93ef51a --- /dev/null +++ b/apps/vscode/src/reference-check.test.ts @@ -0,0 +1,99 @@ +import type * as vscode from 'vscode'; +import { describe, expect, it, vi } from 'vitest'; +import type { FetchLike } from 'oci-registry'; +import { bumpVersion, createFakeDocument } from '../test/fake-document'; +import { fakeFetchResponse } from '../test/fake-fetch'; +import { checkImageReferencesInDocument, checksAsOf, isHelmValuesFile, type DocumentChecks } from './reference-check'; + +const REPOSITORY = 'docker.io/library/nginx'; +const TAG = '1.19'; +const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: ${TAG}`, ''].join('\n'); +const TAGLESS_VALUES_YAML = ['image:', ' repository: registry.example.com/svc', ' pullPolicy: IfNotPresent', ''].join('\n'); + +/** Checks a document this feature is expected to have something to say about. */ +async function checkValuesFile(document: vscode.TextDocument, fetch: FetchLike): Promise { + const checked = await checkImageReferencesInDocument(document, fetch); + + if (checked === undefined) { + throw new Error('expected the document to be checked'); + } + + return checked; +} + +describe('reference-check', () => { + it('should ignore a document that is not the conventional values file name', async () => { + const fetch = vi.fn(); + const document = createFakeDocument('/repo/chart/deployment.yaml', VALUES_YAML); + + expect(isHelmValuesFile(document)).toBe(false); + await expect(checkImageReferencesInDocument(document, fetch)).resolves.toBeUndefined(); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('should ignore a document whose language is not yaml', async () => { + const fetch = vi.fn(); + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML, 'plaintext'); + + expect(isHelmValuesFile(document)).toBe(false); + await expect(checkImageReferencesInDocument(document, fetch)).resolves.toBeUndefined(); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('should return no checks for a values file whose YAML does not parse', async () => { + const fetch = vi.fn(); + const document = createFakeDocument('/repo/chart/values.yaml', ['image:', ' repository: [unclosed', ''].join('\n')); + + // The extractor collects YAML errors instead of throwing, so a malformed + // file yields zero references rather than the `undefined` that means this + // feature has nothing to say about the document. + await expect(checkValuesFile(document, fetch)).resolves.toEqual({ version: document.version, checks: [] }); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('should ask no registry about a tagless reference, and report it as unverifiable', async () => { + const fetch = vi.fn(); + const document = createFakeDocument('/repo/chart/values.yaml', TAGLESS_VALUES_YAML); + + const { checks } = await checkValuesFile(document, fetch); + + expect(checks).toHaveLength(1); + expect(checks[0]?.verdict).toEqual({ kind: 'unverifiable', reason: 'no-tag' }); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('should return the verdict the registry answer implies for a tagged reference', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + + const { checks } = await checkValuesFile(document, fetch); + + expect(checks).toHaveLength(1); + expect(checks[0]?.reference.repository.text).toBe(REPOSITORY); + expect(checks[0]?.verdict).toEqual({ kind: 'exists', registry: 'docker.io' }); + expect(fetch).toHaveBeenCalledTimes(1); + }); + + it('should tag the checks with the version of the document they describe', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + + bumpVersion(document); + + const { version } = await checkValuesFile(document, fetch); + + expect(version).toBe(document.version); + }); + + it('should hand back the checks at the version they describe, and none once the document has moved on', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const checked = await checkValuesFile(document, fetch); + + expect(checksAsOf(checked, document)).toEqual(checked.checks); + + bumpVersion(document); + + expect(checksAsOf(checked, document)).toEqual([]); + }); +}); diff --git a/apps/vscode/test/fake-document.ts b/apps/vscode/test/fake-document.ts new file mode 100644 index 0000000..9745590 --- /dev/null +++ b/apps/vscode/test/fake-document.ts @@ -0,0 +1,36 @@ +import * as vscode from 'vscode'; + +/** Builds a fake `vscode.TextDocument`, with a real `positionAt`/`offsetAt` pair so range assertions are exact. */ +function createFakeDocument(path: string, text: string, languageId = 'yaml'): vscode.TextDocument { + return { + uri: { path, toString: () => path }, + languageId, + version: 1, + getText: () => text, + positionAt: (offset: number) => { + const before = text.slice(0, offset); + const lines = before.split('\n'); + const line = lines.length - 1; + const character = lines[lines.length - 1]?.length ?? 0; + + return new vscode.Position(line, character); + }, + offsetAt: (position: vscode.Position) => { + const lines = text.split('\n'); + let offset = 0; + + for (let line = 0; line < position.line; line += 1) { + offset += (lines[line]?.length ?? 0) + '\n'.length; + } + + return offset + position.character; + }, + } as unknown as vscode.TextDocument; +} + +/** Simulates an edit: VS Code bumps a document's `version` on every change. */ +function bumpVersion(document: vscode.TextDocument): void { + (document as { version: number }).version += 1; +} + +export { bumpVersion, createFakeDocument }; diff --git a/apps/vscode/test/fake-fetch.ts b/apps/vscode/test/fake-fetch.ts new file mode 100644 index 0000000..bfad001 --- /dev/null +++ b/apps/vscode/test/fake-fetch.ts @@ -0,0 +1,14 @@ +const SUCCESS_STATUS_START = 200; +const SUCCESS_STATUS_END = 300; + +/** A canned fetch `Response`-shaped object for the injected fetch fake. */ +function fakeFetchResponse(status: number, body: unknown = {}): { status: number; ok: boolean; json: () => Promise } { + return { + status, + ok: status >= SUCCESS_STATUS_START && status < SUCCESS_STATUS_END, + // eslint-disable-next-line @typescript-eslint/promise-function-async -- trivial canned response, nothing to await + json: () => Promise.resolve(body), + }; +} + +export { fakeFetchResponse }; From 2fb3b8f0bcea45618b2481d6f8deaa6f47edd951 Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Tue, 15 Sep 2026 15:10:29 +0300 Subject: [PATCH 7/7] refactor(vscode): drop the unreachable YAML syntax-error guard `extractImageReferences` cannot throw on malformed YAML. It uses the yaml package's `parseDocument`, which collects syntax errors on the document rather than throwing, unlike `parse`. Fifteen malformed inputs were probed across two independent runs and none threw, so the catch never ran and the comment above it described behaviour this feature does not have. Removing it loses no safety. The open listener's own catch, added when a disposed editor could kill the extension host, covers any genuine fault from the extractor. The gap it claimed to cover is real and is now visible rather than papered over: a malformed values file yields whatever the parser salvaged, and `image: {repository: x` salvages a reference that then gets checked and marked. Making the extension stay quiet on a file the YAML language service is already complaining about needs `packages/helm` to surface parse errors, which is its own change. --- apps/vscode/src/reference-check.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/apps/vscode/src/reference-check.ts b/apps/vscode/src/reference-check.ts index aa8282e..17cb046 100644 --- a/apps/vscode/src/reference-check.ts +++ b/apps/vscode/src/reference-check.ts @@ -50,14 +50,10 @@ async function checkImageReferencesInDocument(document: vscode.TextDocument, fet const version = document.version; - let references: ImageReference[]; - try { - references = extractImageReferences(document.getText()); - } catch { - // A YAML syntax error is the YAML language service's to raise, not this - // feature's. Stay silent rather than compete with it. - return undefined; - } + // No guard around this: the extractor collects YAML syntax errors rather + // than throwing, so a malformed file yields whatever it could salvage. The + // open listener's own catch covers a genuine fault. + const references = extractImageReferences(document.getText()); // A tagless reference has nothing to ask a registry until `appVersion` // resolution lands, but it still comes through as a check. Dropping it is