Skip to content

Run vendored-sync on every PR instead of almost never - #302

Merged
realmarcin merged 1 commit into
mainfrom
fix/vendored-sync-always-runs
Aug 3, 2026
Merged

Run vendored-sync on every PR instead of almost never#302
realmarcin merged 1 commit into
mainfrom
fix/vendored-sync-always-runs

Conversation

@realmarcin

Copy link
Copy Markdown
Contributor

Part of the cross-Mech sweep tracked in CultureBotAI/TraitMech#198. Reference implementation: CultureBotAI/TraitMech#196. Companion PR: CultureBotAI/MediaIngredientMech#166.

The problem

scripts/check_vendored_sync.sh compares six files against the canonical hub. Only two were reachable through the paths: filter of the workflow the job lived in:

file compared by the checker reachable via trigger_paths?
scripts/validate_id_label_correspondence.py yes, listed directly
src/communitymech/schema/mech_shared.yaml yes, via src/communitymech/schema/**
scripts/chem_formula.py no
tests/test_id_label_empty_adapter.py no
tests/test_id_label_unknown_prefix.py no
tests/test_id_label_plausibility.py no

scripts/check_vendored_sync.sh and scripts/.vendored_canon_ref were not listed either, so editing the checker or silently reverting the pin did not fire the checker.

Why "delete the filter" rather than "extend it"

The two jobs sharing that filter are not comparable in cost: vendored-sync is bash + curl, while label-correspondence needs uv and a cached OAK ontology download. The filter is correct for the expensive one and was inherited by the cheap one.

Extending the list by hand recreates the bug — two lists that must agree, with nothing enforcing it. And deriving the filter from the checker's FILES array is not implementable: GitHub evaluates paths: from static YAML before checkout, so it cannot read the repo.

So the job moves to its own workflow with no filter. There is no longer a list that can drift.

Retry

Running unconditionally means 6 unauthenticated raw.githubusercontent fetches per PR, so a hub blip would fail unrelated work. Three attempts, 5s apart. Exit 2 (missing/empty pinned ref) short-circuits, since retrying cannot fix a local precondition.

The retry is in the workflow, not in check_vendored_sync.sh. That script is vendored byte-identical across the spokes and has no canonical copy in the hub to diff against (#278) — so editing it here would create exactly the cross-repo drift that issue describes, with nothing to detect it.

Verified

  • both workflows parse; vendored-sync resolves to no paths: filter and label-correspondence retains its filter and its own job
  • the real checker passes 6/6 against CultureBotAI/CultureMech@6be694f3
  • the new workflow is byte-identical to MediaIngredientMech's apart from one comment naming the local gate
  • retry logic was exercised against stubs in the companion PR: exit 0 immediate, exit 2 short-circuits with no retry, exit 1 after 2 retries

Note on the sweep

Checking the fleet corrected two things in TraitMech#198:

  • "MIM" is CultureBotAI/MediaIngredientMech. The issue recorded MIM as unverified because gh api CultureBotAI/MIM 404s — that is an abbreviation, not a repo name.
  • CultureMech is out of scope. As the hub it has no check_vendored_sync.sh and no vendored-sync job, so there is nothing to fix there. The sweep is two spokes, not three repos.

🤖 Generated with Claude Code

check_vendored_sync.sh compares six vendored files against the canonical hub.
Only two of them were reachable through the paths: filter of the workflow the
job lived in — scripts/validate_id_label_correspondence.py directly, and
src/communitymech/schema/mech_shared.yaml via the schema glob. Editing
scripts/chem_formula.py or any of the three tests/test_id_label_*.py never fired
the guard, and neither did editing check_vendored_sync.sh itself or
scripts/.vendored_canon_ref.

The cause is not four missing path entries. A bash+curl job was sharing a
paths: filter with a gate that needs uv and a cached OAK ontology download. The
filter is right for the expensive job and was inherited by the cheap one.

Deriving the filter from the checker's own FILES array is not possible — GitHub
evaluates paths: from static YAML before checkout, so it cannot read the repo.
Any hand-maintained list has to be kept in sync with the checker by hand, which
is the same drift this guard exists to catch. So move the job to its own
workflow with no filter rather than re-syncing two lists.

Adds a 3-attempt retry, since the job now makes 6 raw.githubusercontent fetches
on every PR and a hub blip should not fail unrelated work. Exit 2 (missing or
empty pinned ref) short-circuits — a local precondition failure that retrying
cannot fix. The retry lives in the workflow, not in check_vendored_sync.sh:
that script is vendored byte-identical across the spokes and has no canonical
copy in the hub to diff against (#278), so editing it here would create
cross-repo drift that nothing detects.

Verified: both workflows parse, vendored-sync resolves to no paths filter,
label-correspondence keeps its filter and its own job, and the real checker
passes 6/6 against CultureBotAI/CultureMech@6be694f3. The workflow is
byte-identical to the one landing in MediaIngredientMech apart from one comment
naming the local gate.

Part of the cross-Mech sweep for CultureBotAI/TraitMech#198.
Reference implementation: CultureBotAI/TraitMech#196.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 07:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a CI blind spot in CommunityMech’s vendored-file drift guard by moving vendored-sync into its own dedicated workflow that runs on every PR (and on main pushes), instead of inheriting the paths: filter intended for the heavier OAK/uv-backed label correspondence check.

Changes:

  • Add a standalone vendored-sync workflow with no paths: filter, so all six files checked by scripts/check_vendored_sync.sh are always covered.
  • Add workflow-level retry logic (3 attempts, 5s backoff) for transient hub fetch failures, while short-circuiting on local precondition failures (exit 2).
  • Remove the embedded vendored-sync job from label-correspondence.yaml and document why it was split.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
.github/workflows/vendored-sync.yaml New always-on vendored drift guard workflow with retry logic and concurrency settings.
.github/workflows/label-correspondence.yaml Removes the old vendored-sync job and documents the rationale for keeping paths: filtering only on the expensive label gate.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@realmarcin

Copy link
Copy Markdown
Contributor Author

Review

Adversarial pass over this diff, done across both sweep PRs together since they are the same change.

The cost asymmetry is now measured, not asserted

The argument for removing the filter is that the job is too cheap to be worth filtering. Both PRs now have real numbers:

repo vendored-sync the gate it used to share a filter with
MediaIngredientMech 8s id-label-gate 3m22s
CommunityMech 8s label-correspondence 6m16s

25x and 47x. The filter was correct for the expensive job and inherited by the cheap one; that is the whole bug.

Fleet consistency verified, not assumed

  • Pinned ref identical across all three spokes: 6be694f3d6308ac0f4c2e0dcf196e2ff73f6468f.
  • check_vendored_sync.sh byte-identical across all three: 04674a58….
  • Retry logic byte-identical between this workflow and the other spokes — I diffed the extracted run: blocks, not the files, so differing comments do not mask a behavioural difference.
  • The real checker passes 6/6 here.

Filed from this review — TraitMech#209

vendored-sync.yaml is now a fourth de-facto shared file with no drift protection. It cannot go on check_vendored_sync.sh's list, because the hub has no copy to diff against — CultureMech has no check_vendored_sync.sh and no vendored-sync job at all. That is the same hole as CommunityMech#278, and neither is fixable without the other, so they are worth deciding together.

Not urgent: divergence here degrades gracefully rather than producing a false pass.

Corrections this sweep made to TraitMech#198

State

All checks green. Not merging — that is the maintainer's call.

@realmarcin

Copy link
Copy Markdown
Contributor Author

Review

Same fix as MediaIngredientMech#166, in the repo with the same defect. vendored-sync lived inside label-correspondence.yaml and inherited a paths: filter scoped to that workflow's expensive OAK-backed gate, so the drift check almost never fired.

It now has its own workflow with no paths: filter, which is the right answer for the reason #166 states: GitHub evaluates paths: from static YAML before checkout, so a filter can never be derived from the checker's own file list — and a hand-maintained copy of that list is exactly the drift the guard exists to catch.

This supersedes my issue #280, which proposed extending the filter. Extending it would have left two lists to keep in sync; removing it removes the failure class.

Verified disjoint from #305, the only other open PR here, which touches a single kb/communities/ record.

Both checks pass, including label-correspondence at 6m16s. Merging.

Closing #280 as fixed, with the note that the fix taken was better than the one filed.

@realmarcin
realmarcin merged commit 7d4b55d into main Aug 3, 2026
3 checks passed
@realmarcin
realmarcin deleted the fix/vendored-sync-always-runs branch August 3, 2026 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants