Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
All notable changes to this project are documented here. The format follows
[Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project uses semantic versioning.

## [Unreleased]

### Added
- `audit --fix` (CLI): writes the missing dependency folders into each workflow's `on.<event>.paths` list and turns bare directories into `dir/**`, editing only those lists. Each edit is checked by re-parsing the workflow; anything it can't change safely is reported and left alone.

## [1.3.2] - 2026-10-06

### Fixed
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ It annotates the workflow file and lists in the job summary:

Run it locally with `npx github:continuous-actions/dynamic-monorepo audit`. On a sample of active public monorepos, about a third had at least one of these problems.

**Prefer not to add an action?** `npx github:continuous-actions/dynamic-monorepo audit --fix` writes the missing folders into those `paths:` lists and adds `/**` to bare directories, then you commit the diff. It edits only the `paths` lists and keeps your comments and quoting. A list it can't edit safely (YAML anchors, multi-line entries) is reported instead, along with stale workflow references, which need a human.

## What it detects

A **project** is a folder that contains one of these files. A changed file belongs to the closest project folder above it.
Expand Down
98 changes: 53 additions & 45 deletions dist/cli.js

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions dist/index.js

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions src/audit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export type AuditFinding = {
line: number;
kind: 'missing-dependency' | 'directory-without-glob' | 'missing-workflow-file';
message: string;
/** What `audit --fix` changes in on.<event>.paths; absent when the fix needs a human. */
fix?: { event: string; add?: string; replace?: { from: string; to: string } };
};

const EVENTS = ['push', 'pull_request', 'pull_request_target'];
Expand Down Expand Up @@ -43,8 +45,8 @@ export function auditWorkflows(config: Config, reader: RepoReader): AuditFinding
if (!Array.isArray(paths) || !paths.every((p) => typeof p === 'string')) continue;
const line = lineOf(lines, event, paths[0] as string);
const include = (paths as string[]).filter((p) => !p.startsWith('!'));
const add = (kind: AuditFinding['kind'], message: string) => {
if (!findings.some((f) => f.file === file && f.kind === kind && f.message === message)) findings.push({ file, line, kind, message });
const add = (kind: AuditFinding['kind'], message: string, fix?: AuditFinding['fix']) => {
if (!findings.some((f) => f.file === file && f.kind === kind && f.message === message)) findings.push({ file, line, kind, message, ...(fix && { fix }) });
};

for (const p of include) {
Expand All @@ -53,7 +55,7 @@ export function auditWorkflows(config: Config, reader: RepoReader): AuditFinding
}
const bare = p.replace(/\/+$/, '');
if (!/[*?[]/.test(bare) && dirSet.has(bare)) {
add('directory-without-glob', `\`${p}\` is a directory; GitHub path filters need \`${bare}/**\` to match the files inside it.`);
add('directory-without-glob', `\`${p}\` is a directory; GitHub path filters need \`${bare}/**\` to match the files inside it.`, { event, replace: { from: p, to: `${bare}/**` } });
}
}

Expand Down Expand Up @@ -84,7 +86,7 @@ export function auditWorkflows(config: Config, reader: RepoReader): AuditFinding
const d = config.projects.get(dep)!;
const who = [...users].sort();
const needs = who.length > 3 ? `${who.slice(0, 3).join(', ')} and ${who.length - 3} more` : who.join(', ');
add('missing-dependency', `\`${d.path}/**\` is missing from on.${event}.paths: ${needs} depend${who.length === 1 ? 's' : ''} on \`${dep}\`, so a change there skips this workflow.`);
add('missing-dependency', `\`${d.path}/**\` is missing from on.${event}.paths: ${needs} depend${who.length === 1 ? 's' : ''} on \`${dep}\`, so a change there skips this workflow.`, { event, add: `${d.path}/**` });
}
}
}
Expand Down
41 changes: 37 additions & 4 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
// and `projects` to list what the configuration (or auto-detection) finds.
// npx github:continuous-actions/dynamic-monorepo [projects] [--base origin/main] [--json] [--verbose]

import { existsSync, mkdtempSync, rmSync } from 'node:fs';
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { parseArgs } from 'node:util';
import { ConfigError, TARGETS } from './config.ts';
import { execute, fsReader, loadConfig, type LoadedConfig } from './engine.ts';
import { auditWorkflows } from './audit.ts';
import { auditWorkflows, type AuditFinding } from './audit.ts';
import { applyFixes } from './fix.ts';
import { Git, GitError } from './git.ts';
import { CycleError, Graph } from './graph.ts';
import { CONFIG_FILE, detectionLines, NAME, serialize, textReport } from './report.ts';
Expand All @@ -19,6 +20,7 @@ Usage:
${NAME} [options] What CI would build, test, deploy and docker-build for your changes
${NAME} projects [options] List every project, its folder, targets and dependencies
${NAME} audit [options] Check workflow on.*.paths filters against the dependency graph
${NAME} audit --fix ...and add what's missing to those paths lists

Options:
--base <ref> Compare against the merge-base with this ref
Expand All @@ -30,6 +32,7 @@ Options:
--fetch Allow fetching missing commits from origin
--json Print the full plan as JSON
--verbose List skipped projects and unowned files
--fix (audit) Edit the paths lists; nothing else in the workflows changes
-h, --help Show this help
`;

Expand All @@ -44,6 +47,7 @@ export function cli(argv: string[]): number {
config: { type: 'string', default: CONFIG_FILE }, cwd: { type: 'string', default: process.cwd() },
uncommitted: { type: 'boolean', default: false }, fetch: { type: 'boolean', default: false },
json: { type: 'boolean', default: false }, verbose: { type: 'boolean', default: false },
fix: { type: 'boolean', default: false },
help: { type: 'boolean', short: 'h', default: false },
},
strict: true,
Expand All @@ -52,6 +56,7 @@ export function cli(argv: string[]): number {
args = parsed.values;
command = parsed.positionals.join(' ') || undefined;
if (command !== undefined && command !== 'projects' && command !== 'audit') throw new Error(`unknown command "${command}" (commands: projects, audit)`);
if (parsed.values.fix && command !== 'audit') throw new Error('--fix only applies to the audit command');
} catch (err) {
process.stderr.write(`${(err as Error).message}\n\n${HELP}`);
return 2;
Expand All @@ -62,7 +67,7 @@ export function cli(argv: string[]): number {
}
try {
if (command === 'projects') return listProjects(loadConfig(args.cwd, args.config), args.json);
if (command === 'audit') return runAudit(loadConfig(args.cwd, args.config), args.json);
if (command === 'audit') return runAudit(loadConfig(args.cwd, args.config), args.json, args.fix);
const git = new Git(args.cwd);
const base = args.base ?? defaultBase(git);
let head = args.head;
Expand All @@ -83,14 +88,42 @@ export function cli(argv: string[]): number {
}

/** Checks every workflow's on.*.paths list against the dependency graph. Exit 1 when something is wrong. */
function runAudit({ git, top, config }: LoadedConfig, asJson: boolean): number {
function runAudit({ git, top, config }: LoadedConfig, asJson: boolean, fix: boolean): number {
const findings = auditWorkflows(config, fsReader(top, git));
if (fix) return fixAudit(top, findings, asJson);
if (asJson) process.stdout.write(`${JSON.stringify(findings, null, 2)}\n`);
else if (findings.length === 0) process.stdout.write('No path-filter problems found.\n');
else for (const f of findings) process.stdout.write(`${f.file}:${f.line}: ${f.message}\n`);
return findings.length === 0 ? 0 : 1;
}

/** Writes the fixable findings into the workflow files and reports the rest. Exit 1 if anything is left. */
function fixAudit(top: string, findings: AuditFinding[], asJson: boolean): number {
const fixed: AuditFinding[] = [];
const left: AuditFinding[] = [];
for (const file of new Set(findings.map((f) => f.file))) {
const abs = join(top, file);
const before = readFileSync(abs, 'utf8');
const r = applyFixes(before, findings.filter((f) => f.file === file));
if (r.text !== before) writeFileSync(abs, r.text);
fixed.push(...r.fixed);
left.push(...r.skipped);
}
if (asJson) {
process.stdout.write(`${JSON.stringify({ fixed, left }, null, 2)}\n`);
return left.length === 0 ? 0 : 1;
}
const change = (f: AuditFinding) => (f.fix!.add ? `added \`${f.fix!.add}\`` : `\`${f.fix!.replace!.from}\` → \`${f.fix!.replace!.to}\``);
const out = [
...fixed.map((f) => `fixed ${f.file}: ${change(f)} in on.${f.fix!.event}.paths`),
...left.map((f) => `left ${f.file}:${f.line}: ${f.message}`),
];
if (out.length === 0) out.push('No path-filter problems found.');
else out.push('', `${fixed.length} fixed, ${left.length} left to fix by hand.${fixed.length ? ' Review the edits with `git diff`.' : ''}`);
process.stdout.write(`${out.join('\n')}\n`);
return left.length === 0 ? 0 : 1;
}

/** Prints every project in dependency order, plus how they were found and likely mistakes. */
function listProjects({ config, configRel, noConfigFile, top }: LoadedConfig, asJson: boolean): number {
const graph = new Graph(config.projects.values(), { allowCycles: true });
Expand Down
148 changes: 148 additions & 0 deletions src/fix.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
// `audit --fix`: applies audit findings to the workflow text, editing only the
// `on.<event>.paths` list and keeping comments, quoting and layout. Every edit is
// checked by parsing the result: if anything other than that list changed, or the
// list isn't exactly what was intended, the file is left alone.

import { isDeepStrictEqual } from 'node:util';
import { load, CORE_SCHEMA } from 'js-yaml';
import type { AuditFinding } from './audit.ts';

export type FixResult = { text: string; fixed: AuditFinding[]; skipped: AuditFinding[] };

export function applyFixes(original: string, findings: AuditFinding[]): FixResult {
let text = original;
const fixed: AuditFinding[] = [];
const skipped: AuditFinding[] = [];
for (const f of findings) {
const next = f.fix ? fixOne(text, f.fix) : undefined;
if (next === undefined) skipped.push(f);
else {
text = next;
fixed.push(f);
}
}
return { text, fixed, skipped };
}

type Fix = NonNullable<AuditFinding['fix']>;

function parse(text: string): any {
try {
return load(text, { schema: CORE_SCHEMA });
} catch {
return undefined;
}
}

const onOf = (doc: any) => (doc && typeof doc === 'object' ? ('on' in doc ? 'on' : 'true' in doc ? 'true' : undefined) : undefined);

/** Returns the edited text, or undefined when the edit can't be made safely. */
function fixOne(text: string, fix: Fix): string | undefined {
const before = parse(text);
const key = onOf(before);
const paths: unknown = key && before[key]?.[fix.event]?.paths;
if (!Array.isArray(paths) || !paths.every((p) => typeof p === 'string')) return undefined;
let want: string[];
if (fix.add !== undefined) {
if (paths.includes(fix.add)) return text;
want = [...paths, fix.add];
} else if (fix.replace) {
const { from, to } = fix.replace;
if (!paths.includes(from)) return undefined;
want = paths.includes(to) ? paths.filter((p) => p !== from) : paths.map((p) => (p === from ? to : p));
} else return undefined;

const eol = text.includes('\r\n') ? '\r\n' : '\n';
const lines = text.split(/\r?\n/);
const at = locate(lines, fix.event);
if (!at) return undefined;
if (at.kind === 'flow') {
const line = lines[at.line]!;
const open = line.indexOf('[', at.col);
const close = line.replace(/\s+#[^'"]*$/, '').lastIndexOf(']'); // ignore a `]` in a trailing comment
if (open < 0 || close < open) return undefined;
const inner = line.slice(open + 1, close);
const style = styleOf(inner.trim());
lines[at.line] = fix.add !== undefined
? `${line.slice(0, close).replace(/\s*$/, '')}${inner.trim() ? ', ' : ''}${quote(fix.add, style)}${line.slice(close)}`
: `${line.slice(0, open)}[${want.map((p) => quote(p, style)).join(', ')}]${line.slice(close + 1)}`;
} else {
const style = styleOf(lines[at.items[0]!]!.replace(/^\s*-\s*/, ''));
const indent = /^(\s*-\s*)/.exec(lines[at.items[0]!]!)![1]!;
if (fix.add !== undefined) {
lines.splice(at.items[at.items.length - 1]! + 1, 0, `${indent}${quote(fix.add, style)}`);
} else {
const { from, to } = fix.replace!;
const i = at.items.find((n) => scalarOf(lines[n]!) === from);
if (i === undefined) return undefined;
const comment = /\s+#.*$/.exec(lines[i]!.slice(indent.length).replace(/^(['"]).*?\1/, ''))?.[0] ?? '';
if (want.length < paths.length) lines.splice(i, 1);
else lines[i] = `${indent}${quote(to, styleOf(lines[i]!.slice(indent.length)))}${comment}`;
}
}
const out = lines.join(eol);
// The result must parse to the same document with only this paths list changed.
const after = parse(out);
if (!after) return undefined;
const expected = structuredClone(before);
expected[key!][fix.event].paths = want;
return isDeepStrictEqual(after, expected) ? out : undefined;
}

type Where = { kind: 'flow'; line: number; col: number } | { kind: 'block'; items: number[] };

const indentOf = (l: string) => /^\s*/.exec(l)![0].length;
const blank = (l: string) => /^\s*(#.*)?$/.test(l);
const keyRe = (k: string) => new RegExp(`^(\\s*)(["']?)${k}\\2\\s*:(.*)$`);

/** Finds the `paths:` value under top-level `on:` → `<event>:` in block style. */
function locate(lines: string[], event: string): Where | undefined {
const on = lines.findIndex((l) => /^(["']?)on\1\s*:\s*(#.*)?$/.test(l));
if (on < 0) return undefined;
const ev = child(lines, on, 0, event);
if (!ev) return undefined;
const p = child(lines, ev.line, ev.indent, 'paths');
if (!p) return undefined;
const rest = p.rest.replace(/\s+#.*$/, '').trim();
if (rest.startsWith('[')) return rest.endsWith(']') ? { kind: 'flow', line: p.line, col: lines[p.line]!.indexOf(':') } : undefined;
if (rest !== '') return undefined;
const items: number[] = [];
for (let i = p.line + 1; i < lines.length; i++) {
if (blank(lines[i]!)) continue;
const ind = indentOf(lines[i]!);
if (ind < p.indent || (ind === p.indent && !/^\s*-(\s|$)/.test(lines[i]!))) break;
if (!/^\s*-\s+\S/.test(lines[i]!)) return undefined; // nested or multi-line entries: leave to a human
if (items.length > 0 && ind !== indentOf(lines[items[0]!]!)) return undefined;
items.push(i);
}
return items.length > 0 ? { kind: 'block', items } : undefined;
}

/** The direct child `key:` of the mapping that starts after line `parent` (indented deeper than `parentIndent`). */
function child(lines: string[], parent: number, parentIndent: number, key: string) {
let indent = -1;
for (let i = parent + 1; i < lines.length; i++) {
if (blank(lines[i]!)) continue;
const ind = indentOf(lines[i]!);
if (ind <= parentIndent) return undefined;
if (indent < 0) indent = ind;
if (ind !== indent) continue;
const m = keyRe(key).exec(lines[i]!);
if (m) return { line: i, indent: ind, rest: m[3] ?? "" };
}
return undefined;
}

type Style = 'single' | 'double' | 'plain';
const styleOf = (s: string): Style => (s.startsWith("'") ? 'single' : s.startsWith('"') ? 'double' : 'plain');

function quote(s: string, style: Style): string {
if (style === 'double') return JSON.stringify(s);
if (style === 'plain' && /^[A-Za-z0-9._/-][A-Za-z0-9._/*-]*$/.test(s)) return s;
return `'${s.replace(/'/g, "''")}'`;
}

function scalarOf(line: string): string | undefined {
const v = parse(line.replace(/^\s*-\s*/, '- '));
return Array.isArray(v) && typeof v[0] === 'string' ? v[0] : undefined;
}
55 changes: 53 additions & 2 deletions tests/features.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

import { afterAll, describe, expect, it } from 'vitest';
import { spawnSync } from 'node:child_process';
import { mkdirSync } from 'node:fs';
import { resolve } from 'node:path';
import { mkdirSync, readFileSync } from 'node:fs';
import { join, resolve } from 'node:path';
import { CFG, CONFIG, cleanup, pushPayload, Repo, runAction } from './helpers.ts';

afterAll(cleanup);
Expand Down Expand Up @@ -261,6 +261,57 @@ describe('path-filter audit', () => {
expect(cli.status).toBe(1);
expect(cli.stdout).toMatch(/web.yml:\d+: `packages\/shared\/\*\*` is missing/);
});

describe('--fix', () => {
const CLI = resolve(import.meta.dirname, '..', 'dist', 'cli.js');
const run = (dir: string, ...args: string[]) => spawnSync(process.execPath, [CLI, 'audit', '--fix', ...args], { cwd: dir, encoding: 'utf8' });
const read = (repo: Repo, f: string) => readFileSync(join(repo.dir, f), 'utf8');

it('adds missing folders and globs directories, editing only the paths lists', () => {
const repo = auditRepo();
repo.write({ '.github/workflows/web.yml': wf(" - 'apps/web/**' # the app\n - 'packages/ui/**'\n - '.github/workflows/old-web.yml'").replace('name: web', '# keep me\nname: web') });
const r = run(repo.dir);
expect(r.status).toBe(1); // the stale workflow reference needs a human
expect(r.stdout).toContain("fixed .github/workflows/web.yml: added `packages/shared/**` in on.pull_request.paths");
expect(r.stdout).toContain('fixed .github/workflows/ui.yml: `packages/ui` → `packages/ui/**`');
expect(r.stdout).toMatch(/left {3}\.github\/workflows\/web\.yml:\d+: `\.github\/workflows\/old-web\.yml`/);
expect(read(repo, '.github/workflows/web.yml')).toBe(
wf(" - 'apps/web/**' # the app\n - 'packages/ui/**'\n - '.github/workflows/old-web.yml'\n - 'packages/shared/**'").replace('name: web', '# keep me\nname: web'),
);
expect(read(repo, '.github/workflows/ui.yml')).toBe(wf(" - 'packages/ui/**'\n - 'packages/shared/**'"));
const again = spawnSync(process.execPath, [CLI, 'audit'], { cwd: repo.dir, encoding: 'utf8' });
expect(again.stdout.trim().split('\n')).toHaveLength(1);
expect(again.stdout).toContain('old-web.yml');
});

it('keeps flow lists, plain scalars and CRLF line endings', () => {
const repo = auditRepo();
const flow = 'name: web\r\non:\r\n push:\r\n paths: [apps/web/**, packages/ui/**] # keep [x]\r\njobs:\r\n t:\r\n runs-on: ubuntu-latest\r\n steps:\r\n - run: echo hi\r\n';
repo.write({ '.github/workflows/web.yml': flow, '.github/workflows/ui.yml': null });
expect(run(repo.dir).status).toBe(0);
expect(read(repo, '.github/workflows/web.yml')).toBe(flow.replace('packages/ui/**]', 'packages/ui/**, packages/shared/**]'));
});

it('leaves lists it cannot edit safely untouched', () => {
const repo = auditRepo();
const anchored = "name: web\non:\n push:\n paths: &p\n - 'apps/web/**'\n pull_request:\n paths: *p\njobs:\n t:\n runs-on: ubuntu-latest\n steps:\n - run: echo hi\n";
repo.write({ '.github/workflows/web.yml': anchored, '.github/workflows/ui.yml': null });
repo.write({ 'apps/web/package.json': JSON.stringify({ name: 'web', dependencies: { shared: '*' } }) });
const r = run(repo.dir, '--json');
expect(r.status).toBe(1);
const out = JSON.parse(r.stdout);
expect(out.fixed).toEqual([]);
expect(out.left.length).toBeGreaterThan(0);
expect(read(repo, '.github/workflows/web.yml')).toBe(anchored);
});

it('is only accepted by audit', () => {
const repo = auditRepo();
const r = spawnSync(process.execPath, [CLI, '--fix'], { cwd: repo.dir, encoding: 'utf8' });
expect(r.status).toBe(2);
expect(r.stderr).toContain('--fix only applies to the audit command');
});
});
});

describe('CLI', () => {
Expand Down
Loading