Skip to content

Glossary edits fail CI on a dist-action/glossary copy that nothing reads #95

Description

@mmcky

Symptom

A pure data PR that edits only glossary/<lang>.json fails CI on the "Verify dist-action bundle is up to date" step, with dist-action/ is out of date. Run 'npm run build' and commit the result. #78 hit this: lint, format check, and all 1003 tests pass, and the only failure is bundle drift on dist-action/glossary/fr.json.

This is a foot-gun for glossary work. Editing a glossary requires a build step that is not obvious from the change, and the error message points at dist-action/ — a build artifact the author never touched and has no reason to think about. Per Glossary draft-then-review-style workflows, glossary PRs are going to be routine, including ones authored by native-speaker reviewers who are not set up to run npm run build.

Root cause: the copy is dead weight

build-action.mjs copies glossary/*.json into dist-action/glossary/, and those copies are committed (git ls-files dist-action/ tracks glossary/fa.json, glossary/fr.json, glossary/zh-cn.json). The stated rationale, from the header comment in build-action.mjs, is that the files are "copied as external assets since they're loaded at runtime via fs.readFileSync()". That reasoning is sound for a bundle-adjacent asset, but it assumes the code reads __dirname/glossary. It doesn't — it reads __dirname/../glossary.

action.yml sets main: 'dist-action/index.js', so at runtime __dirname is <action-root>/dist-action, and all three call sites resolve one level up to <action-root>/glossary — the repo's top-level directory, not the bundled copy. The .. survives bundling; the shipped dist-action/index.js contains path4.join(__dirname2, "..", "glossary", ...) at lines 35104, 35332, and 35478, and the string dist-action appears zero times in the bundle. Because the runner unpacks the entire action repository for uses: owner/repo@ref, top-level glossary/ is present on disk alongside dist-action/, so this resolves correctly.

dist-action/glossary/*.json is therefore never read by any code path. The action works because the full repo is checked out, not because of the copy. Deleting the copy step would change nothing at runtime — it would only stop glossary PRs from tripping the bundle check.

Two things make this worse than a harmless duplicate. There is no directory-level fallback: loadGlossary in src/sync-orchestrator.ts joins the dir it's handed, and a mis-resolved path degrades silently to no glossary with a warning rather than throwing — so a wrong path yields untranslated terminology, not a failed run. And nothing enforces that the two copies agree. They are byte-identical today, but since the copies are never read, drift would be invisible rather than breaking. The .. in src/index.ts (lines 84, 446, 698) is itself untested — src/__tests__/sync-orchestrator.test.ts injects the top-level dir directly and bypasses the resolution entirely.

Options

Drop the copy (recommended). Remove the copy step from build-action.mjs, git rm -r dist-action/glossary/, and let the loader keep reading top-level glossary/. Since nothing reads the copies, this is not a behavior change — it deletes dead weight. Glossary edits become pure data PRs that no longer touch the bundle, so the freshness check stops firing on them and reviewers can edit a glossary from the GitHub UI. Worth adding a test that pins the action-root-relative resolution, since it is currently untested and would become the only path.

Keep the copy and make the bundle self-contained. Change the loader to path.join(__dirname, 'glossary') so the copies are actually used. This makes dist-action/ genuinely standalone and no longer reliant on the runner unpacking the whole repo, but it keeps the foot-gun: glossary PRs would still need a rebuild, and it makes the copy load-bearing rather than removing the duplication.

Have CI regenerate and commit the bundle. Orthogonal to the above and a bigger change to the release flow; only worth considering if the duplication is kept for other reasons.

The recommendation is to drop the copy. It removes the duplication, the drift risk, and the foot-gun in one move, and carries no runtime change.

Immediate unblock for #78

Independent of the fix here: check out glossary/fr-programming-terms, run npm run build, and commit the regenerated dist-action/glossary/fr.json.

Metadata

Metadata

Assignees

No one assigned

    Labels

    maintenanceRoutine housekeeping: style, formatting, env & dependency upkeep

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions