CI: a typo in sync/canonical.yml or a manifest class: passes the PR check, then stops every refresh #21

Follow-up from the review of #14.

What happens

#14 adds two hand-edited inputs: sync/canonical.yml and the class: of a manifest stanza. Only tools/promote validates them, at start-up, before it plans anything (load_manifest, tools/promote:389, and load_canonical_map, tools/promote:418). An unknown series, a consumer series named as canonical, an entry with no series, a YAML error or an unknown class makes every promote run exit 1.

Nothing runs that validation before merge. A pull request that touches only sync/** gets one check, the check job of drift-check.yml, which runs tools/drift-check. The refresh job is skipped on pull requests, and tests.yml's path filter covers only tools/** (and tests.yml itself). drift-check never reads sync/canonical.yml, and consumer_series (tools/drift-check:171-173) reads any class other than consumer as source. So the edit merges green.

After the merge, CI shows nothing until a canonical copy's text moves upstream, because only then does the refresh job run. Promote then stops at start-up, so every entry is held back, not just the one the edit was meant to settle (elsewhere #14 makes sure one refused entry does not hold back the rest), and no refresh pull request is opened. The run goes red: the refresh job's guard sees what a crash part-way looks like (a non-zero exit with no summary [refresh] line) and reports that the writes may be partial, although nothing was written. This repeats on every refresh until the file is corrected.

Reproduction (with the tools/tests harness)

repo = make_repo(["intro", "intermediate", "dp"], consumers=["dp"])
repo.lecture("intro", "career", lecture_text("Career"))
repo.lecture("intermediate", "mle", lecture_text("MLE"))
assert repo.promote("career", "mle").returncode == 0

# A pull request that touches only sync/**: its one check passes.
repo.write_canonical_map({"mle": "intermedate"})
assert repo.drift_check().returncode <= 1  # 0 here; the check step accepts 0 or 1

# After the merge, an unrelated canonical copy moves upstream.
repo.lecture("intro", "career", lecture_text("Career", "an upstream fix"))
repo.move_pin("intro")
assert repo.drift_check().returncode == 1  # refresh_needed: the refresh job runs

res = repo.promote("--refresh")
assert res.returncode == 1 and res.stderr.startswith("error: ")
assert "summary [refresh]" not in res.stdout + res.stderr  # what the refresh job's guard checks
assert "an upstream fix" not in repo.pool("career.md").read_text()  # career is held back too

promote's stderr:

error: invalid entries in .../sync/canonical.yml:
       mle: unknown series 'intermedate'

The other edits behave the same way. In each case drift-check exits 0 (clean) and promote exits 1 with no summary line:

On a copy of the real pool and mirror at the pinned SHAs, changing career: intermediate to career: intermediat, appending calvo: dp, or setting class: consumers on dp behaves the same way: drift-check is clean with exit 0, and tools/promote --refresh --exclude dp jax --dry-run exits 1 with the matching error and no summary line. With the committed files, that dry run plans 41 entries and refuses none.

When it bites

It cannot happen yet, because the committed files are valid. It becomes possible with the first hand edit after #14 merges, and the documented workflow calls for such edits. A refused refresh entry tells the operator to "name the canonical series in sync/canonical.yml" (tools/promote:761-765, and also the refresh pull request and the README). Map lines change or go as the renames and merges in QuantEcon/project-monorepo#76 land, and QuantEcon/project-monorepo#24 edits both files when it retires dp-test. #14 is also adding two new errors to load_canonical_map: a repeated key, and two keys that name one lecture. These checks also run only inside promote, so more bad edits will pass the PR check and then stop the refresh. This is new with #14: main has no sync/canonical.yml and no class:.

Suggested fix

1. Validate both files in drift-check, using promote's own loaders so that the two tools cannot disagree. Since drift-check: judge a non-canonical copy by promotion's own rule #17, drift-check already imports tools/promote (promote_module(), tools/drift-check:180-195). Call load_manifest() and load_canonical_map() at start-up, and use the consumer set that load_manifest() returns in place of consumer_series. The loaders raise SystemExit with a message, which exits with status 1, and the check step accepts 0 or 1 (test "$rc" -le 1), so catch it and raise a DriftError (exit 2); left as it is, the pull request stays green. drift-check.yml's pull_request paths already include sync/**.
2. Add sync/** to tests.yml's paths:, with a test that runs the same loaders on the committed sync/manifest.yml and sync/canonical.yml. The loaders need no mirror.
3. Optionally, also run tools/promote --refresh --dry-run, with CI's exclusions, in the check job on pull requests. This also catches a bad PROMOTE_EXCLUDE and exercises promote changes on real data. However, it needs a mirror of every considered series, and it should fail only on an exit without a summary line, so that a pull request is not red because of an entry the nightly refresh already refuses.
4. Optionally, give promote's configuration errors an exit status of their own (today they exit 1, as a refused entry does). The guard could then say that nothing was written and repeat promote's error: line in its annotation.

The test above fails once drift-check exits 2 on these edits. A fix made only in the workflows would leave it passing, so that fix needs its own test, such as the one in item 2.