From 3c0a1815318477f4f52d4d54432e0f5fe586fbfe Mon Sep 17 00:00:00 2001 From: Jamie Nelson Date: Sun, 11 Oct 2026 11:29:20 +1100 Subject: [PATCH 1/3] fix: preserve empty metadata descriptions and cover reviewed errors --- __tests__/action.test.ts | 40 ++++++++--- __tests__/integration-readme-contract.test.ts | 51 ++++++++++++++ __tests__/save.test.ts | 67 +++++++++++++++++++ src/Action.ts | 3 +- src/sections/update-usage.ts | 6 +- 5 files changed, 153 insertions(+), 14 deletions(-) create mode 100644 __tests__/save.test.ts diff --git a/__tests__/action.test.ts b/__tests__/action.test.ts index f81848ca..fe6d94a0 100644 --- a/__tests__/action.test.ts +++ b/__tests__/action.test.ts @@ -137,13 +137,27 @@ describe('Action', () => { }); }); - describe('nested metadata validation', () => { + describe('metadata validation', () => { const metadata = { name: 'Fixture action', description: 'Fixture description', runs: { using: 'node24', main: 'index.js' }, }; + it.each(['node24', 42, true])('rejects scalar runs %j with metadata context', async (runs) => { + vi.mocked(fs.readFileSync).mockReturnValueOnce(YAML.stringify({ ...metadata, runs })); + const { default: Action } = await import('../src/Action.js'); + + expect(() => new Action(actTestYmlPath, mockLogTask)).toThrow( + expect.objectContaining({ + message: expect.stringContaining(`Failed to load ${actTestYmlPath}`), + cause: expect.objectContaining({ + message: `Invalid action metadata syntax in ${actTestYmlPath}.`, + }), + }), + ); + }); + it.each([ { inputs: { option: null } }, { inputs: { option: 42 } }, @@ -154,19 +168,23 @@ describe('Action', () => { ])('rejects malformed nested metadata through YAML loading: %j', async (nested) => { vi.mocked(fs.readFileSync).mockReturnValueOnce(YAML.stringify({ ...metadata, ...nested })); const { default: Action } = await import('../src/Action.js'); - expect(() => new Action(actTestYmlPath, mockLogTask)).toThrow('Invalid action metadata syntax'); + expect(() => new Action(actTestYmlPath, mockLogTask)).toThrow( + 'Invalid action metadata syntax', + ); }); it('accepts scalar defaults and nullish descriptions through YAML loading', async () => { - vi.mocked(fs.readFileSync).mockReturnValueOnce(YAML.stringify({ - ...metadata, - inputs: { - enabled: { description: 'Enabled', default: false, required: false }, - retries: { description: 'Retries', default: 3 }, - optional: { default: null, description: null }, - }, - outputs: { result: { description: null } }, - })); + vi.mocked(fs.readFileSync).mockReturnValueOnce( + YAML.stringify({ + ...metadata, + inputs: { + enabled: { description: 'Enabled', default: false, required: false }, + retries: { description: 'Retries', default: 3 }, + optional: { default: null, description: null }, + }, + outputs: { result: { description: null } }, + }), + ); const { default: Action } = await import('../src/Action.js'); const action = new Action(actTestYmlPath, mockLogTask); expect(action.inputs?.enabled?.default).toBe(false); diff --git a/__tests__/integration-readme-contract.test.ts b/__tests__/integration-readme-contract.test.ts index 54162947..765ef248 100644 --- a/__tests__/integration-readme-contract.test.ts +++ b/__tests__/integration-readme-contract.test.ts @@ -242,6 +242,57 @@ describe('README generation contract', () => { expect(sectionBody(await generate(), 'outputs')).toContain(key); }); + it.each([undefined, '', 'null'])( + 'renders empty descriptions for YAML descriptions written as %j', + async (description) => { + const descriptionLine = + description === undefined ? '' : ` description: ${description}\n`; + fs.writeFileSync( + actionPath, + `name: Nullish description action +description: Metadata with empty descriptions +inputs: + option: +${descriptionLine} default: false +outputs: + result: +${descriptionLine} value: output +runs: + using: node24 + main: index.js +`, + ); + const before = 'Keep this prefix.\n'; + const after = '\nKeep this suffix.\n'; + fs.writeFileSync(readmePath, `${before}${README_WITH_MARKERS}${after}`); + + const readme = await generate(false); + + for (const section of ['inputs', 'outputs']) { + const rows = sectionBody(readme, section) + .split('\n') + .filter((line) => line.startsWith('|')); + const cells = rows[2] + .split('|') + .slice(1, -1) + .map((cell) => cell.trim()); + expect(cells[0]).toBe( + section === 'inputs' ? 'option' : 'result', + ); + expect(cells[1]).toBe(''); + } + expect(sectionBody(readme, 'inputs')).toContain('false'); + expect(sectionBody(readme, 'usage')).toContain("option: ''"); + expect( + sectionBody(readme, 'usage') + .match(/^\s*# Description:.*$/gmu) + ?.map((line) => line.trim()), + ).toEqual(['# Description:']); + expect(readme.startsWith(before)).toBe(true); + expect(readme.endsWith(after)).toBe(true); + }, + ); + it('ignores leading blank lines before selecting input and output table paragraphs', async () => { const action = ACTION_YML.replace( 'description: A plain input with a default', diff --git a/__tests__/save.test.ts b/__tests__/save.test.ts new file mode 100644 index 00000000..6dc494c4 --- /dev/null +++ b/__tests__/save.test.ts @@ -0,0 +1,67 @@ +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +import { afterEach, describe, expect, it, vi } from 'vite-plus/test'; + +import { GHActionDocsConfig } from '../src/config.js'; +import type Inputs from '../src/inputs.js'; +import LogTask from '../src/logtask/index.js'; +import save from '../src/save.js'; + +function inputsWithSave(saveEnabled: boolean, configPath: string): Inputs { + const config = { + owner: 'fixture-owner', + repo: 'fixture-repo', + save: saveEnabled, + }; + + return { + config: { get: vi.fn(() => config) }, + configPath, + } as unknown as Inputs; +} + +describe('save', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('persists the resolved config when save is enabled', async () => { + const tempDirectory = fs.mkdtempSync(path.join(os.tmpdir(), 'ghadocs-save-')); + const configPath = path.join(tempDirectory, '.ghadocs.json'); + + try { + await save(inputsWithSave(true, configPath), new LogTask('save-test')); + + expect(JSON.parse(fs.readFileSync(configPath, 'utf8'))).toMatchObject({ + owner: 'fixture-owner', + repo: 'fixture-repo', + }); + } finally { + fs.rmSync(tempDirectory, { recursive: true, force: true }); + } + }); + + it('rejects with the original persistence error when save is enabled', async () => { + const failure = new Error('config persistence failed'); + const persist = vi.spyOn(GHActionDocsConfig.prototype, 'save').mockRejectedValue(failure); + const configPath = path.join(os.tmpdir(), 'ghadocs-save-failure', '.ghadocs.json'); + + await expect(save(inputsWithSave(true, configPath), new LogTask('save-test'))).rejects.toBe( + failure, + ); + expect(persist).toHaveBeenCalledWith(configPath); + }); + + it('does not attempt persistence when save is disabled', async () => { + const persist = vi.spyOn(GHActionDocsConfig.prototype, 'save'); + + await save( + inputsWithSave(false, path.join(os.tmpdir(), 'ghadocs-save-disabled', '.ghadocs.json')), + new LogTask('save-test'), + ); + + expect(persist).not.toHaveBeenCalled(); + }); +}); diff --git a/src/Action.ts b/src/Action.ts index c54d3647..31aceb4d 100644 --- a/src/Action.ts +++ b/src/Action.ts @@ -135,8 +135,7 @@ export default class Action implements ActionYaml { (isRecord(value) && Object.values(value).every( (entry) => - isRecord(entry) && - (entry.description == null || typeof entry.description === 'string'), + isRecord(entry) && (entry.description == null || typeof entry.description === 'string'), )); if (!validDescriptions(record.inputs) || !validDescriptions(record.outputs)) { return false; diff --git a/src/sections/update-usage.ts b/src/sections/update-usage.ts index a2130d5e..5ab4fc8a 100644 --- a/src/sections/update-usage.ts +++ b/src/sections/update-usage.ts @@ -43,7 +43,11 @@ export default async function updateUsage( for (const key of Object.keys(inp)) { const input = inp[key]; if (input !== undefined) { - descriptionPromises[key] = wrapDescription(`Description: ${input.description}`, [], indent); + descriptionPromises[key] = wrapDescription( + `Description: ${input.description ?? ''}`, + [], + indent, + ); } } From a5e2d31f836cd779be511973c0138dd1a9030a0b Mon Sep 17 00:00:00 2001 From: Jamie Nelson Date: Sun, 11 Oct 2026 11:44:30 +1100 Subject: [PATCH 2/3] fix: verify empty metadata descriptions through the contract checker --- __tests__/integration-readme-contract.test.ts | 43 ++++++++++++++++++- scripts/verify-readme-contract.mjs | 2 +- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/__tests__/integration-readme-contract.test.ts b/__tests__/integration-readme-contract.test.ts index 765ef248..a9e1f176 100644 --- a/__tests__/integration-readme-contract.test.ts +++ b/__tests__/integration-readme-contract.test.ts @@ -243,7 +243,7 @@ describe('README generation contract', () => { }); it.each([undefined, '', 'null'])( - 'renders empty descriptions for YAML descriptions written as %j', + 'renders and verifies empty descriptions for YAML descriptions written as %j', async (description) => { const descriptionLine = description === undefined ? '' : ` description: ${description}\n`; @@ -264,7 +264,27 @@ runs: ); const before = 'Keep this prefix.\n'; const after = '\nKeep this suffix.\n'; - fs.writeFileSync(readmePath, `${before}${README_WITH_MARKERS}${after}`); + const originalReadme = `${before}${[ + '', + '', + '', + '', + '', + '', + '', + ].join('\n')}${after}`; + fs.writeFileSync(readmePath, originalReadme); + fs.writeFileSync( + path.join(tempDir, '.ghadocs.json'), + JSON.stringify({ + owner: 'contract-owner', + repo: 'contract-repo', + paths: { action: actionPath, readme: readmePath }, + sections: ['usage', 'inputs', 'outputs'], + prettier: false, + versioning: { source: 'explicit', override: '1.2.3', prefix: 'v', badge: false }, + }), + ); const readme = await generate(false); @@ -290,6 +310,25 @@ runs: ).toEqual(['# Description:']); expect(readme.startsWith(before)).toBe(true); expect(readme.endsWith(after)).toBe(true); + + const verify = (candidate: string) => { + fs.writeFileSync(readmePath, candidate); + return spawnSync( + process.execPath, + [VERIFIER_SCRIPT, actionPath, readmePath, path.join(tempDir, 'README.original.md')], + { cwd: tempDir, encoding: 'utf8' }, + ); + }; + fs.writeFileSync(path.join(tempDir, 'README.original.md'), originalReadme); + const valid = verify(readme); + expect(valid.status, `${valid.stdout}\n${valid.stderr}`).toBe(0); + expect(valid.stdout).toContain('All contract checks passed'); + + const staleDescription = readme.replace(' # Description:', ' # Description: stale'); + expect(staleDescription).not.toBe(readme); + const stale = verify(staleDescription); + expect(stale.status).not.toBe(0); + expect(stale.stdout).toContain('usage comments do not exactly project action.yml'); }, ); diff --git a/scripts/verify-readme-contract.mjs b/scripts/verify-readme-contract.mjs index 9b3016f8..477a25e2 100644 --- a/scripts/verify-readme-contract.mjs +++ b/scripts/verify-readme-contract.mjs @@ -599,7 +599,7 @@ if (usage === null) { .join('\n'); const generatedUsageComments = async (declaration) => { - const formatted = await format(`Description: ${declaration?.description}`, { + const formatted = await format(`Description: ${declaration?.description ?? ''}`, { semi: false, parser: 'markdown', proseWrap: 'always', From beb2180abd2d9b225af0b8db0a37201c2616021a Mon Sep 17 00:00:00 2001 From: Jamie Nelson Date: Sun, 11 Oct 2026 12:19:53 +1100 Subject: [PATCH 3/3] fix: verify declared defaults using their rendered projection Preserve absent, empty, null, and multiline default distinctions in the exact README verifier. Exercise real YAML generation under both formatting settings and require corrupted default projections to fail. --- __tests__/integration-readme-contract.test.ts | 129 ++++++++++++++++++ scripts/verify-readme-contract.mjs | 17 ++- 2 files changed, 141 insertions(+), 5 deletions(-) diff --git a/__tests__/integration-readme-contract.test.ts b/__tests__/integration-readme-contract.test.ts index a9e1f176..08bf6154 100644 --- a/__tests__/integration-readme-contract.test.ts +++ b/__tests__/integration-readme-contract.test.ts @@ -332,6 +332,135 @@ runs: }, ); + it.each([false, true])( + 'renders and verifies declared default values with pretty %s', + async (pretty) => { + fs.writeFileSync( + actionPath, + `name: Declared defaults action +description: Metadata with supported default values +inputs: + absent_default: + description: No default + empty_default: + description: Empty default + default: '' + null_default: + description: Null default + default: null + false_default: + description: False default + default: false + zero_default: + description: Zero default + default: 0 + multiline_default: + description: Multiline default + default: |- + first + second + blank_line_default: + description: Blank line default + default: |- + before + + after +runs: + using: node24 + main: index.js +`, + ); + const originalReadme = [ + 'Keep this prefix. ', + '', + '', + '', + '', + 'Keep this suffix. ', + '', + ].join('\n'); + const originalReadmePath = path.join(tempDir, 'README.original.md'); + fs.writeFileSync(readmePath, originalReadme); + fs.writeFileSync(originalReadmePath, originalReadme); + fs.writeFileSync( + path.join(tempDir, '.ghadocs.json'), + JSON.stringify({ + owner: 'contract-owner', + repo: 'contract-repo', + paths: { action: actionPath, readme: readmePath }, + sections: ['usage', 'inputs'], + prettier: pretty, + versioning: { source: 'explicit', override: '1.2.3', prefix: 'v', badge: false }, + }), + ); + + const readme = await generate(pretty); + const usage = sectionBody(readme, 'usage'); + expect( + usage + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.startsWith('# Default:')), + ).toEqual([ + '# Default:', + '# Default: null', + '# Default: false', + '# Default: 0', + '# Default: first', + '# Default: before', + ]); + expect(usage).toContain(' # Default: first\n # second'); + expect(usage).toContain(' # Default: before\n #\n # after'); + const defaultCells = sectionBody(readme, 'inputs') + .split('\n') + .filter((line) => line.startsWith('|')) + .slice(2) + .map((line) => line.split('|')[3].trim()); + expect(defaultCells).toEqual([ + '', + '', + 'null', + 'false', + '0', + 'first
second', + 'before

after', + ]); + + const verify = (candidate: string) => { + fs.writeFileSync(readmePath, candidate); + return spawnSync( + process.execPath, + [VERIFIER_SCRIPT, actionPath, readmePath, originalReadmePath], + { cwd: tempDir, encoding: 'utf8' }, + ); + }; + const valid = verify(readme); + expect(valid.status, `${valid.stdout}\n${valid.stderr}`).toBe(0); + expect(valid.stdout).toContain('All contract checks passed'); + + const corruptions = [ + { + readme: readme.replace(' # Default:\n', ' # Default: stale\n'), + diagnostic: 'usage comments do not exactly project action.yml: empty_default', + }, + { + readme: readme.replace('null', ''), + diagnostic: '`null_default` default in the inputs table does not match action.yml', + }, + { + readme: readme.replace(' # second', ' # second'), + diagnostic: 'usage comments do not exactly project action.yml: multiline_default', + }, + ]; + for (const corruption of corruptions) { + expect(corruption.readme).not.toBe(readme); + const invalid = verify(corruption.readme); + expect(invalid.status).not.toBe(0); + expect(invalid.stdout).toContain(corruption.diagnostic); + } + }, + ); + it('ignores leading blank lines before selecting input and output table paragraphs', async () => { const action = ACTION_YML.replace( 'description: A plain input with a default', diff --git a/scripts/verify-readme-contract.mjs b/scripts/verify-readme-contract.mjs index 477a25e2..89e6d49e 100644 --- a/scripts/verify-readme-contract.mjs +++ b/scripts/verify-readme-contract.mjs @@ -607,8 +607,12 @@ if (usage === null) { }); const lines = formatted.split('\n'); if (declaration?.default !== undefined) { - const defaultLine = `Default: ${declaration.default}`; - lines.push(prettierEnabled ? defaultLine.trimEnd() : defaultLine); + const label = 'Default: '; + const [first, ...continuations] = String(declaration.default).split(/\r\n|\n|\r/); + lines.push(`${label}${first}`.trimEnd()); + for (const line of continuations) { + lines.push(`${' '.repeat(label.length)}${line}`.trimEnd()); + } } return lines.join('\n'); }; @@ -737,9 +741,12 @@ await tableSection('inputs', inputs, (declaration) => [ { label: 'description', expected: firstParagraph(declaration?.description), markdown: true }, { label: 'default', - // An empty-string default is indistinguishable from no default once it is - // in a table cell, so it is not asserted. `false` and `0` are. - expected: declaration?.default === undefined || declaration.default === '' ? '' : declaration.default, + // Missing and empty-string defaults have empty cells. Every other declared + // value is rendered as text, including `false`, `0`, and `null`. + expected: + declaration?.default === undefined || declaration.default === '' + ? '' + : String(declaration.default), code: 'html', }, // The generator emits the flag emphasised — `update-inputs.ts` writes