fix-workflows: stop reformatting generated workflows

[jwbron]

The bug

The YAML fixer already skips compiled workflows:

(entry.name.endsWith(".yml") || entry.name.endsWith(".yaml")) &&
!entry.name.endsWith(".lock.yml")

But the separate oxfmt step in action.yml globbed .github/workflows/**/*.yml with no exclusions, so it reformatted the very files the fixer deliberately left alone. cli.ts had the same glob.

The consequence isn't a failed run; it's worse than that. The Show fix instructions step then annotates the generated file with ::warning file=…::This file needs updating. Run: pnpm dlx github:Khan/actions#fix-workflows-v3, and a maintainer who follows that hint commits a reformatted lock that no longer matches gh aw compile.

Why nobody noticed

Khan/actions doesn't run this action on itself, and webapp and frontend don't carry compiled agentic workflows in a repo that does. Khan/agent-settings#48 is the first install where both are true, and it reproduced immediately: two advisory annotations on that PR, on review.lock.yml and agentics-maintenance.yml.

I confirmed the reformatting is real rather than theoretical by running the pinned oxfmt@0.44.0 with this action's own .oxfmtrc.json over a copy of a 123KB review.lock.yml: the checksum changed.

The fix

Both invocations exclude generator-owned workflows, and one list (GENERATED_WORKFLOWS) drives every site that acts on it: the fixer calls isGeneratedWorkflow, cli.ts builds its negated globs from generatedWorkflowSkipGlobs(), and action.yml (a composite bash step, so it has nothing to import) has its hand-copied literals pinned to that function by a test. The negations are recursive, matching the positive glob they subtract from.

agentics-maintenance.yml joins *.lock.yml in that set. gh-aw regenerates it unconditionally (deleting it doesn't stick) and it is not named *.lock.yml, so nothing was skipping it at all; the fixer wanted 6 checkout-followed-by-setup changes and 12 runs-on rewrites in a file no human can fix.

I deliberately did not implement the general "skip anything a generator owns" rule. Review pointed out the cheap version of it, and it checks out: gh-aw stamps DO NOT EDIT. into the first line of both files it emits (agentics-maintenance.yml included, from a different generator than the lock compiler), so no .gitattributes parsing is needed to detect them.

It still isn't a drop-in change, and the comment on GENERATED_WORKFLOWS now says why. oxfmt selects files by glob, and a glob cannot read a header; teaching only the fixer to sniff would leave a marked-but-unlisted workflow skipped by the fixer and still reformatted by the formatter, which is the exact divergence this PR closes. A content rule has to reach both sites at once, which means the formatter taking a computed file list instead of a glob. That's the deferred change now, and the comment names it.

Why negated globs and not ignorePatterns

The obvious fix is ignorePatterns in .oxfmtrc.json. It does not work here, and I tested it rather than assuming: that config lives in the action's directory (--config "${{ github.action_path }}/.oxfmtrc.json"), not the repo being formatted, and its patterns don't resolve against the working directory (the lock was still rewritten). Negated positional globs do work (verified: oxfmt reported on 1 files instead of 2, and the lock's checksum was unchanged).

The globs were originally duplicated in action.yml and cli.ts, which was the part I liked least; review was right that half of it was avoidable. cli.ts now derives them, so action.yml is the only copy, and the test below pins it.

Tests

Six cases in actions/fix-workflows/generated-workflows.test.ts, split into its own file because adding them put index.test.ts over the repo's 1000-line cap:

• the predicate skips locks and agentics-maintenance.yml, and does not skip the lookalikes (lock-threads.yml, agentics-maintenance-notes.yml), since the suffix test is .lock.yml rather than lock anywhere;
• action.yml's literal globs equal generatedWorkflowSkipGlobs(), and its positive glob is still recursive so the negations have something to subtract from. I mutated action.yml two ways to confirm this bites: dropping one negation fails it, and reverting to the old non-recursive pair fails it too. A decoy '!…' in a later step does not, since the assertion is scoped to the oxfmt step.

Behavior checks beyond the unit tests, over a fixture with a top-level lock, a nested deep.lock.yml, the real agentics-maintenance.yml and one hand-written workflow:

• the pinned oxfmt@0.44.0 with the recursive negations reports on 1 files and leaves all three generated checksums unchanged; without them it reports 4 files and rewrites all three (which also confirms **/ matches files directly in .github/workflows, so one pattern covers both depths);
• cli.ts end-to-end reports no violations, formats 1 file, and leaves all three generated files byte-identical.

1623 tests pass (60 in actions/fix-workflows); typecheck, eslint and prettier clean.

[changeset-bot]

🦋 Changeset detected

Latest commit: 7246edc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
fix-workflows	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[jaredly]

these deduplication suggestions look like good ideas

[github-actions]

Review Guidance

Common patterns

2 files: Add negated oxfmt globs to exclude generator-owned workflows (*.lock.yml and agentics-maintenance.yml) from the formatting step, accompanied by an identical comment explaining why .oxfmtrc.json ignorePatterns cannot substitute (config resolves relative to the action directory, not the repo under format)

-                ".github/workflows/**/*.yml"
+                ".github/workflows/**/*.yml" \
+                '!.github/workflows/**/*.lock.yml' \
+                '!.github/workflows/**/agentics-maintenance.yml'

[github-actions]

All 6 prior review threads are resolved.
Note: correctness-reviewer not assessed this run (correctness-reviewer output unavailable).
Note: divergence tripwire re-armed a full review (unreviewed share 0.86).

[github-actions]

suggestion (non-blocking): generatedWorkflowSkipGlobs()'s exact returned globs are never asserted. The isGeneratedWorkflow predicate is well covered, but generatedWorkflowSkipGlobs() is only checked via expect(literals).toEqual(generatedWorkflowSkipGlobs()), which verifies action.yml matches the function, not that the function produces correct globs. A direct assertion of the exact strings would regression-lock the negated-glob format that the whole fix depends on.

Suggested change

	export function generatedWorkflowSkipGlobs(): string[] {
	it("produces recursive negated globs that subtract the generated set", () => {
	expect(generatedWorkflowSkipGlobs()).toEqual([
	"!.github/workflows/**/*.lock.yml",
	"!.github/workflows/**/agentics-maintenance.yml",
	]);
	});

[github-actions]

thought (non-blocking): The stated harm is the annotation, but the annotation step itself is left unguarded. Per the PR's own framing, the real damage is the Show fix instructions step blindly annotating everything git diff reports under .github/workflows/ (confirmed in action.yml: it loops over git diff --name-only with no filter). This fix closes the two known mutators but leaves the harmful-hint layer trusting them forever; filtering generated files out of the warning loop would make the stated failure impossible regardless of which upstream step touches them.

A sketch, not a committable replacement:

In the `Show fix instructions` loop, skip (or specially word the warning for) files matching the generated set — or, per the other finding, files carrying the `DO NOT EDIT.` marker — so the desync-inviting hint can never point at compiler output.

[github-actions]

suggestion (non-blocking): The 'a glob cannot read a header' premise doesn't hold for a bash step, and it's what forces this whole name-list apparatus. The comment defers the content-marker rule because 'the oxfmt step selects files by glob, and a glob cannot read a header' — but the oxfmt step is shell: bash, which can trivially compute a marker-filtered file list (e.g. grep -L 'DO NOT EDIT.' .github/workflows/*.yml fed to oxfmt as explicit paths; verified both lock files and gh-aw output generally carry that marker — on line 3, incidentally, not 'their first line' as the comment states). That would let the fixer and both formatter invocations share one content rule today, deleting GENERATED_WORKFLOWS, generatedWorkflowSkipGlobs(), the hand-copied literals, and the 81-line test that string-parses action.yml to pin them. The deferred refactor looks cheaper than the drift-prevention machinery being added to avoid it.

A sketch, not a committable replacement:

Have the fixer skip files whose first few lines contain gh-aw's `DO NOT EDIT.` marker, and have both oxfmt invocations format an explicit file list filtered the same way (bash: `grep -L`; cli.ts: reuse the fixer's predicate), instead of maintaining a name/suffix list with a cross-file drift test.