fix(verifier): vendor the LLM model registry into .workflows-lib - #2850
Conversation
Compare-mode verification has been reporting "compare mode requires two cross-family verifier judges; available families: none" on every repo even though OPENAI_API_KEY and CLAUDE_API_STRANSKE are configured. The verifier sparse-checkout of stranske/Workflows vendored `tools` but not `config`, and tools/llm_registry.py resolves config/llm_slots.json and config/model_registry.json relative to that vendored tree, so both judge slots resolved to no model and the LLM evaluation never ran. agents-verify-to-issue-v2.yml vendors the same client for followup_issue_generator.py and had the same omission.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Workflow source neededPR #2850 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd123ea6c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughVerifier workflows now sparse-checkout the shared ChangesLLM registry checkout
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Automated Status SummaryHead SHA: 274d387
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/workflows/test_workflow_llm_installs.py`:
- Around line 284-296: Update
test_bundled_llm_config_resolves_two_cross_family_judges to accept monkeypatch,
clear ENV_SLOT_CONFIG with monkeypatch.delenv, and set DEFAULT_SLOT_CONFIG_PATH
and DEFAULT_MODEL_REGISTRY_CONFIG_PATH to the bundled LLM_CONFIG_PATHS via
monkeypatch.setattr before calling load_slot_config. Preserve the existing
file-existence and cross-family assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3721d794-02de-43b0-b508-443e1d9e09b5
📒 Files selected for processing (3)
.github/workflows/agents-verify-to-issue-v2.yml.github/workflows/reusable-agents-verifier.ymltests/workflows/test_workflow_llm_installs.py
agents-verify-to-new-pr.yml runs the same scripts/langchain client as agents-verify-to-issue-v2.yml but sparse-checked out `tools` without `config`, so verify:create-new-pr still resolved no model on either judge slot and fell back to the non-LLM path while verify:create-issue got the fix. Replace the hand-maintained parametrize list with discovery over workflows that vendor `tools` and reference scripts/langchain, so a future workflow vendoring the client inherits the guard, and add a coverage test that fails if discovery stops seeing the three known verifier surfaces. Pin the bundled-config judge test to the vendored files with monkeypatch so an ambient LANGCHAIN_SLOT_CONFIG / LANGCHAIN_MODEL_REGISTRY_CONFIG override cannot make it pass.
…umer copy templates/consumer-repo/.github/workflows/agents-verify-to-new-pr.yml is a byte-parity copy (not allowlisted baseline drift), so the root-only edit tripped Health 74 Template Drift. Consumer repos run this workflow, so the copy needs the same `config` entry for verify:create-new-pr to resolve the judge slots there too. Verified with the exact failing command: python scripts/check_template_drift.py --allowlist config/template-drift-allowlist.txt -> unallowlisted drift 0 (was 1).
|
Opener lane (cursor) — review findings addressed; fix widened to the third verifier surface. Two commits on top of
To stop this recurring, the guard no longer uses a hand-maintained parametrize list. Also pinned
Validation. Deliberate break: removing Both review threads replied and resolved. Awaiting CI on |
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
Disagreement
Unique Insights
🔍 LangSmith Traces |
Closer verifier disposition — unanimous PASS; one concern was real and is now fixed in #2861This PR carried anthropic's three concerns, audited against 1. "The full content of 2. "If any other workflow vendors
Both reach Fixed in bounded follow-up #2861, which adds 3. "The second acceptance criterion (two cross-family judges) depends on pre-existing config content, asserted by the new test but not independently verified." Independently verified two ways. Disposition: PASS accepted. This PR closes no source issue, so no issue disposition applies. Remaining debt is tracked in #2861. |
…context (#2872) `Health 45 Agents Guard / guard` cannot be a required status check. It is a commit status posted by agents-guard.yml only inside the "Report agents guard commit status" step, whose condition is `always() && steps.eligibility.outputs.should-run == 'true'`, and eligibility requires one of the agent labels (agent:codex, agents:auto-pilot, ...). On any PR without such a label the status is never posted, and a required check that never reports leaves the PR permanently un-mergeable. Verified absent on the heads of all 12 most recent PRs (#2850-#2862), where the only statuses are `Gate / gate` and `CodeRabbit`. This was not merely theoretical: health-44 passes .github/config/required-contexts.json to `enforce_gate_branch_protection.py --apply` on workflow_dispatch/workflow_call whenever an enforcement token is present, so dispatching that workflow today would have added the context and frozen every non-agent PR in the repo. Remove it from the required-contexts set (shared config, the tool's DEFAULT_CONTEXTS, both health-40 invocations) and record why in the config so it is not re-added. `Gate / gate` remains required-worthy: it is posted on every PR by pr-00-gate.yml and passes on all 12 most recent PRs. agents-guard.yml still posts its status; it is simply informational rather than gating. Also give health-40's "Root allowlist guard" `if: always()`. Steps run sequentially, so the failing branch-protection snapshot step was skipping it — which is how six unlisted repo-root files accumulated unseen for a month. Refs #2858. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Root cause
Every
verify:comparerun across the fleet has been returningCONCERNSwith:This has been attributed to missing/unconfigured verifier judges and escalated to the owner on at least ten sources. It is not a credentials problem.
Manager-Database(and the other affected repos) already have bothOPENAI_API_KEYandCLAUDE_API_STRANSKEconfigured, and the workflow does pass them into the compare step.The real cause is in the vendored library.
reusable-agents-verifier.ymlsparse-checks-outstranske/Workflowsinto.workflows-libwithscriptsandtools, but notconfig.tools/llm_registry.pyresolves its configuration relative to that vendored tree:So the run log for the manual compare dispatch on merged
Manager-Database#1474(run 30160098574) shows:With no registry, every slot resolves to an empty model,
build_chat_clientsreturns no clients, andpr_verifier.pyshort-circuits to theavailable families: nonefallback before any judge is invoked.Fix
Add
configto the sparse-checkout of both workflows that vendortoolsfor the LangChain client:reusable-agents-verifier.yml— evaluate/compare judges.agents-verify-to-issue-v2.yml—followup_issue_generator.py, which builds the same client and had the identical omission.With
configvendored, the bundled configuration resolves two cross-family judges whose keys are already present as secrets:Test gate
tests/workflows/test_workflow_llm_installs.py:test_llm_workflows_vendor_the_model_registry_config— any workflow vendoringtoolsfromstranske/Workflowsmust also vendorconfig.test_bundled_llm_config_resolves_two_cross_family_judges— the shipped config resolves models for at least two provider families.Deliberate-break demonstration: reverting only the two workflow edits fails the gate with
Restoring the edits passes. Full local run:
717 passed, 6 skippedintests/workflows;ruffandblackclean.Impact
This is the shared root cause behind the verifier blockers currently parked for owner decision on
Manager-Database#1463/#1262/#1267/#1489,Pension-Data#637,trip-planner#1493/#1495/#1496,learning-management-system#392,Workflows#2654, andInv-Man-Intake#851. Those were all reported as "configure two cross-family judges" decisions; they should be re-dispatchable once this merges, since re-dispatch will then run against a changed configuration rather than reproducing the same missing-registry fingerprint.Non-goals
No change to model selection policy, judge model choices, or the separate GitHub Models access gate tracked in
#2768/#2819.Summary by CodeRabbit
Bug Fixes
Tests