diff --git a/.github/workflows/pnpm-policy-inventory.yml b/.github/workflows/pnpm-policy-inventory.yml index 28e4c9c..b737d3b 100644 --- a/.github/workflows/pnpm-policy-inventory.yml +++ b/.github/workflows/pnpm-policy-inventory.yml @@ -46,8 +46,10 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + # `...` includes workspace dependencies: pnpm-policy imports yamlize, + # which imports nested-obj, and tsc needs their built type declarations. - name: Build pnpm-policy - run: pnpm --filter pnpm-policy run build + run: pnpm --filter 'pnpm-policy...' run build # The npm search endpoint is anonymous — no registry token needed — but it # rate-limits bursts, so requests are spaced out. diff --git a/packages/pnpm-policy/__tests__/policy.test.ts b/packages/pnpm-policy/__tests__/policy.test.ts index 057acfc..2f996d9 100644 --- a/packages/pnpm-policy/__tests__/policy.test.ts +++ b/packages/pnpm-policy/__tests__/policy.test.ts @@ -128,6 +128,17 @@ describe('resolvePolicy', () => { expect(commentAt(comments.before, ['minimumReleaseAgeExclude'])).toContain('pyramation'); }); + it('agrees with the number of maintainers it names', () => { + const one = resolve({ maintainers: ['pyramation'] }); + expect(commentAt(one.comments.before, ['minimumReleaseAgeExclude'])).toContain( + 'pyramation publishes on npm' + ); + const two = resolve({ maintainers: ['pyramation', 'dan'] }); + expect(commentAt(two.comments.before, ['minimumReleaseAgeExclude'])).toContain( + 'pyramation, dan publish on npm' + ); + }); + it('carries each exception reason to the line it exempts', () => { const { comments, settings } = resolve({ exceptions: [{ package: 'left-pad', reason: 'CVE-2026-1', until: '2027-01-01' }] diff --git a/packages/pnpm-policy/__tests__/workspace.test.ts b/packages/pnpm-policy/__tests__/workspace.test.ts index f04fe9e..33db8e4 100644 --- a/packages/pnpm-policy/__tests__/workspace.test.ts +++ b/packages/pnpm-policy/__tests__/workspace.test.ts @@ -60,6 +60,12 @@ describe('applyPolicy', () => { expect(out).not.toContain('allowBuilds'); }); + it('separates an appended key from the content it lands after', () => { + const out = applyPolicy('packages:\n - packages/*\n', policyFor()); + expect(out).toContain(` - packages/*\n\n# ${MANAGED_MARKER}`); + expect(out).not.toMatch(/\n\n\n/); + }); + it('is idempotent — a second run changes nothing', () => { const once = applyPolicy('packages:\n - packages/*\n', policyFor()); expect(applyPolicy(once, policyFor())).toBe(once); diff --git a/packages/pnpm-policy/src/policy.ts b/packages/pnpm-policy/src/policy.ts index 821bbe6..1bd181e 100644 --- a/packages/pnpm-policy/src/policy.ts +++ b/packages/pnpm-policy/src/policy.ts @@ -104,7 +104,9 @@ function buildComments( [ `Exempt from the wait: ${sources.join(', ')}.`, config.maintainers.length - ? `First-party membership comes from what ${config.maintainers.join(', ')} publish on npm — waiting on your own release protects nothing.` + ? `First-party membership comes from what ${config.maintainers.join(', ')} ${ + config.maintainers.length === 1 ? 'publishes' : 'publish' + } on npm — waiting on your own release protects nothing.` : 'First-party membership comes from the inventory.' ].join('\n') ]); diff --git a/packages/pnpm-policy/src/workspace.ts b/packages/pnpm-policy/src/workspace.ts index 809d83f..78f7a78 100644 --- a/packages/pnpm-policy/src/workspace.ts +++ b/packages/pnpm-policy/src/workspace.ts @@ -57,14 +57,18 @@ export function applyPolicy( } const first = Object.keys(policy.settings).find((key) => managed.includes(key)); - const before = policy.comments.before.map( - ([path, text]): [CommentPath, string] => - // The stamp goes on the first managed key rather than the top of the file, - // where it would fight with whatever the workspace already says there. + const before = policy.comments.before.map(([path, text]): [CommentPath, string] => { + // The stamp goes on the first managed key rather than the top of the file, + // where it would fight with whatever the workspace already says there. + const body = options.marker !== false && path.length === 1 && path[0] === first - ? [path, `${MANAGED_MARKER}\n\n${text}`] - : [path, text] - ); + ? `${MANAGED_MARKER}\n\n${text}` + : text; + // A leading blank line keeps each managed block visually separate, which + // matters most for a key appended to the end of an existing file: it lands + // flush against whatever was already the last line. + return [path, path.length === 1 ? `\n${body}` : body]; + }); applyComments(doc, { before, inline: policy.comments.inline }); diff --git a/packages/yamlize/__tests__/comments.test.ts b/packages/yamlize/__tests__/comments.test.ts index 8faecc2..4552c29 100644 --- a/packages/yamlize/__tests__/comments.test.ts +++ b/packages/yamlize/__tests__/comments.test.ts @@ -110,4 +110,18 @@ describe('applyComments', () => { applyComments(doc, { before: { age: 'current' } }); expect(doc.toString()).toBe('# current\nage: 1\n'); }); + + it('opens a blank line when the comment starts with one', () => { + const doc = parseDocument('packages:\n - packages/*\n'); + doc.set('age', 1); + applyComments(doc, { before: { age: '\nthe wait' } }); + expect(doc.toString()).toBe('packages:\n - packages/*\n\n# the wait\nage: 1\n'); + }); + + it('does not open one otherwise', () => { + const doc = parseDocument('packages:\n - packages/*\n'); + doc.set('age', 1); + applyComments(doc, { before: { age: 'the wait' } }); + expect(doc.toString()).toBe('packages:\n - packages/*\n# the wait\nage: 1\n'); + }); }); diff --git a/packages/yamlize/src/comments.ts b/packages/yamlize/src/comments.ts index 63e1ab0..6f2cf10 100644 --- a/packages/yamlize/src/comments.ts +++ b/packages/yamlize/src/comments.ts @@ -23,6 +23,20 @@ function commentText(text: string): string { .join('\n'); } +/** + * A leading blank line in the comment asks for a blank line in the output. + * + * The `yaml` package models that as `spaceBefore` on the node rather than as + * part of the comment, so it is stripped from the text and set on the node — + * otherwise a comment written as `'\ntext'` renders flush against whatever + * precedes it, which is exactly the separation the author was asking for. + */ +function splitSpaceBefore(text: string): { spaceBefore: boolean; text: string } { + return text.startsWith('\n') + ? { spaceBefore: true, text: text.slice(1) } + : { spaceBefore: false, text }; +} + /** Read either comment map form as a list of path/text pairs. */ function commentEntries(map: CommentMap | undefined): Array<[YamlPath, string]> { if (!map) return []; @@ -132,9 +146,14 @@ export function applyComments(doc: Document, comments: CommentOptions): void { doc.comment = commentText(comments.footer); } - for (const [path, text] of commentEntries(comments.before)) { - const node = commentTarget(doc, path, false) as { commentBefore?: string } | undefined; - if (node) node.commentBefore = commentText(text); + for (const [path, raw] of commentEntries(comments.before)) { + const { spaceBefore, text } = splitSpaceBefore(raw); + const node = commentTarget(doc, path, false) as + | { commentBefore?: string; spaceBefore?: boolean } + | undefined; + if (!node) continue; + node.commentBefore = commentText(text); + if (spaceBefore) node.spaceBefore = true; } for (const [path, text] of commentEntries(comments.inline)) {