chore: sync workflow templates - #2007
Closed
stranske wants to merge 1 commit into
Closed
Conversation
Automated sync from stranske/Workflows Template hash: 76689bc445fd Changes synced from sync-manifest.yml
stranske
added
sync
stranske
had a problem deploying
to
agent-standard
June 22, 2026 04:14 — with 
GitHub Actions
Failure
GitHub Actions
Failure
📝 WalkthroughWalkthroughAdds ChangesLLM Registry and Blocked-Model Enforcement
Orchestrator Skill and Repo Validation Fixes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
2 tasks
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc3cd27ac1
ℹ️ 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".
Comment on lines
+253
to
+255
| if is_model_blocked(provider, model, registry=registry): | ||
| logger.warning("Skipping blocked LLM slot override: %s/%s", provider, model) | ||
| continue |
There was a problem hiding this comment.
When LANGCHAIN_MODEL or LANGCHAIN_SLOT1_MODEL is blocked for the first slot, this continue removes that slot from the resolved slot list. The callers then re-apply positional overrides over the shortened list (tools/langchain_client.py lines 486-488), so a blocked OpenAI override such as gpt-4o-mini makes the OpenAI slot disappear and the same override is sent to Anthropic/GitHub instead of falling back to those slots' configured models. With both OpenAI and Anthropic credentials set this returns anthropic/gpt-4o-mini, bypassing the blocked-model policy and producing an invalid provider/model request.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@scripts/runner_lib/core.py`:
- Around line 426-431: The orchestrator_skill_summary_path variable is read from
environment context and if it is an absolute path, it is used without
validation, which allows arbitrary file inclusion. Modify the logic to ensure
that orchestrator_summary_path is always constrained to the workspace directory.
Instead of allowing absolute paths to be used directly, either reject absolute
paths or resolve them relative to the workspace root. The check should validate
that the final resolved path is actually within the workspace directory before
using it. Apply the same fix to the other occurrences mentioned at lines 461-465
and 955.
- Around line 369-372: The pack names from configuration files are not validated
against path traversal patterns before being used in the checkout_path that is
passed to shutil.rmtree(), creating a security vulnerability where names like
"../../../etc" can escape the workspace directory. Add path traversal validation
to reject pack names containing dangerous patterns such as "..", "/", or other
directory traversal characters. This validation should be integrated into the
existing _require_nonempty_string() function or a new _validate_pack_name()
function, ensuring all pack names are sanitized before checkout_path is
constructed and passed to shutil.rmtree().
In `@tools/langchain_client.py`:
- Around line 281-287: The blocked model override check in auto-slot mode only
validates against slots[0].provider when selected_provider is not set, but the
override could be applied to any slot in the list. Modify the blocking
validation logic to check the model_override against all available slots (not
just the first one) to ensure that blocked combinations are caught regardless of
which slot is ultimately selected for the override.
In `@tools/llm_registry.py`:
- Around line 83-95: The load_model_registry() function needs to be hardened
against malformed configuration data. Currently it assumes the "models" value
from payload is iterable and the "quality" value from raw_entry is a dict, but
invalid JSON shapes can cause crashes instead of failing gracefully. Add type
validation checks to ensure that the "models" field is actually a list or dict
before iterating, and verify that quality_payload is actually a dict before
calling .items() on it. If either validation fails, skip processing that entry
or use the default empty dict to allow the function to fail open with an empty
registry instead of crashing the application.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d6fc95fa-f30f-4f4f-99ab-75f84d69845f
📒 Files selected for processing (5)
scripts/orchestrator_skill.pyscripts/reference_packs.pyscripts/runner_lib/core.pytools/langchain_client.pytools/llm_registry.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
stranske/Workflows(auto-detected)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Manager-Database repository: use Prefect 2.x and import schedules from prefect.client.schemas.schedules
Files:
scripts/orchestrator_skill.pyscripts/reference_packs.pyscripts/runner_lib/core.pytools/langchain_client.pytools/llm_registry.py
🔀 Multi-repo context stranske/Workflows
Perfect! I now have comprehensive context. Let me compile my findings:
Linked repositories findings
stranske/Workflows
Key Cross-Repo Dependencies and Validation Changes:
1. Repository Name Validation (_validate_repo)
[::stranske/Workflows::]
Both scripts/orchestrator_skill.py and scripts/reference_packs.py implement the same stricter validation:
- Old behavior: Only checked for presence of
/and no leading/trailing slashes, allowing formats likeowner/name/extra - New behavior: Splits on
/and requires exactly 2 non-empty parts (owner/name format only)
Test coverage already exists:
tests/scripts/test_orchestrator_skill.py::test_parse_rejects_nested_repo_names()— verifiesowner/repo/extrais rejectedtests/scripts/test_reference_packs.py::test_parse_reference_packs_rejects_nested_repo_names()— same validation test- All existing test fixtures use valid
owner/nameformat (e.g.,"trend/research","stranske/Workflows")
Integration point:
scripts/runner_lib/core.py:materialize_orchestrator_skill()andmaterialize_reference_packs()call these validators indirectly via module imports [::stranske/Workflows::], ensuring the validation is applied before materialization
2. New llm_registry.py Module
[::stranske/Workflows::]
Consumers:
tools/langchain_client.pyimports and delegates to llm_registry functions:normalize_provider,is_model_blocked,load_model_registry,select_model_for_tier,resolve_slots[::stranske/Workflows::]templates/consumer-repo/tools/langchain_client.pycontains a synced copy that also imports from llm_registry [::stranske/Workflows::]
Breaking behavior in langchain_client.py:
build_chat_client()now returnsNone(with warning) if the resolved provider/model is blocked [::stranske/Workflows::]build_chat_clients()now skips or returns[]for blocked models instead of attempting suppressed construction [::stranske/Workflows::]- This is a defensive enhancement, not an API signature break—callers already check for
Nonereturns
3. runner_lib/core.py Changes
[::stranske/Workflows::]
New imports and behavior:
- Now imports
contextliband usescontextlib.suppress(FileNotFoundError)when deleting stale checkout directories [::stranske/Workflows::] - When
materialize_orchestrator_skill()is called with a pack reference, it now safely removes the previous materialization before re-materializing [::stranske/Workflows::] - This prevents stale files from persisting—test
test_materialize_orchestrator_skill_clears_stale_pack_checkout()verifies this works [::stranske/Workflows::]
Prompt assembly change:
- Previously always read
.reference/ORCHESTRATOR_SKILL.md; now computesorchestrator_summary_pathdynamically - If
materialize_orchestrator_skillis enabled, it captures the returned path; otherwise it reads fromORCHESTRATOR_SKILL_SUMMARY_PATHenvironment variable [::stranske/Workflows::] - The orchestrator skill section is added only when the summary file exists—backward compatible [::stranske/Workflows::]
4. Sync Manifest Dependency Ordering
[::stranske/Workflows::]
Critical ordering constraint verified by test:
test_sync_manifest_ships_reference_packs_before_runner_lib()intests/scripts/test_reference_packs.pyasserts that:scripts/reference_packs.pyappears beforescripts/runner_lib/in.github/sync-manifest.ymlscripts/orchestrator_skill.pyappears beforescripts/runner_lib/[::stranske/Workflows::]
- This ordering ensures that when
runner_libimports these modules, they are already in place in consumer repos [::stranske/Workflows::]
5. Template Sync Points
[::stranske/Workflows::]
templates/consumer-repo/tools/langchain_client.pyandtemplates/consumer-repo/tools/llm_registry.pyare synced copies [::stranske/Workflows::]- These must remain in sync to avoid import mismatches in consumer repositories [::stranske/Workflows::]
Summary
No breaking changes detected for existing public APIs. The changes are:
- Validation strictness: More correct—rejects invalid
owner/repo/extraformats that the old code may have accepted - New defensive checks: LLM model blocking logic added but returns
Nonegracefully rather than breaking callers - Internal improvements: Safer cleanup, dynamic summary path resolution (backward compatible)
- Dependency order: Already tested to ensure correct sync ordering in consumer repos
🔇 Additional comments (5)
scripts/orchestrator_skill.py (1)
83-84: LGTM!scripts/reference_packs.py (1)
86-87: LGTM!tools/llm_registry.py (1)
1-82: LGTM!Also applies to: 96-277
tools/langchain_client.py (1)
15-280: LGTM!Also applies to: 328-371, 489-491
scripts/runner_lib/core.py (1)
8-8: LGTM!
Comment on lines
369
to
+372
| checkout_path = workspace_path / matching[0].checkout_path | ||
| with contextlib.suppress(FileNotFoundError): | ||
| shutil.rmtree(checkout_path) | ||
| materialize_reference_packs( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify how checkout paths are produced and whether they are normalized/bounded.
fd -i "reference_packs.py"
fd -i "orchestrator_skill.py"
fd -i "core.py"
echo "---- checkout path construction / validation ----"
rg -n -C4 'def build_checkout_plan|checkout_path|resolve\(|relative_to\(|\.\.' scripts/reference_packs.py scripts/orchestrator_skill.py scripts/runner_lib/core.py
echo "---- tests covering checkout-path safety ----"
rg -n -C3 'checkout_path|path traversal|relative_to|materialize_orchestrator_skill' testsRepository: stranske/Portable-Alpha-Extension-Model
Length of output: 20592
🏁 Script executed:
#!/bin/bash
echo "=== Finding DEFAULT_CHECKOUT_PATH ==="
rg -n "DEFAULT_CHECKOUT_PATH\s*=" scripts/
echo "=== Checking if checkout_path is configurable or always hardcoded ==="
rg -n "checkout_path" scripts/reference_packs.py | head -20
echo "=== Full orchestrator_skill.py to find DEFAULT_CHECKOUT_PATH ==="
rg -n "^DEFAULT_CHECKOUT_PATH|^[A-Z_]*.*=.*checkout" scripts/orchestrator_skill.py
echo "=== Checking config schema for any user-provided checkout_path ==="
rg -n "checkout_path" scripts/reference_packs.py scripts/orchestrator_skill.py | grep -v "plan\|Plan\|class\|checkout_path:"
echo "=== Testing Path behavior with .. ==="
python3 - <<'PY'
from pathlib import Path
workspace = Path("/home/user/workspace").resolve()
# Simulate checkout_path with traversal
test_paths = [
".reference/pack1",
".reference/../../../etc/passwd",
".reference/../../etc",
]
for cp in test_paths:
result = workspace / cp
print(f"workspace / '{cp}' = {result}")
print(f" Escapes workspace? {not str(result).startswith(str(workspace))}")
PYRepository: stranske/Portable-Alpha-Extension-Model
Length of output: 1774
🏁 Script executed:
#!/bin/bash
echo "=== Check pack.name validation in reference_packs.py ==="
rg -n -B5 -A10 "def _validate_repo|def _require_nonempty_string|def parse_reference_packs" scripts/reference_packs.py | head -80
echo "=== Check all validation functions ==="
rg -n "^def _validate|^def _require" scripts/reference_packs.py
echo "=== Look at full pack name processing ==="
rg -n -B3 -A3 'pack\.name|"name"' scripts/reference_packs.py
echo "=== Test if rmtree resolves paths ==="
python3 - <<'PY'
from pathlib import Path
import tempfile
import shutil
import os
# Create test structure
with tempfile.TemporaryDirectory() as tmpdir:
workspace = Path(tmpdir)
# Create directories
(workspace / ".reference").mkdir()
(workspace / ".reference" / "pack1").mkdir()
(workspace / ".reference" / "pack1" / "test.txt").write_text("data")
# Create a crafted path with ..
escaped_path = workspace / ".reference/../../etc"
print(f"Crafted path: {escaped_path}")
print(f"Path exists: {escaped_path.exists()}")
print(f"Resolves to: {escaped_path.resolve()}")
print(f"Resolved exists: {escaped_path.resolve().exists()}")
# Does rmtree treat .. differently?
test_path = workspace / ".reference/../.reference/pack1"
print(f"\nTest path: {test_path}")
print(f"Resolves to: {test_path.resolve()}")
if test_path.exists():
print("Before rmtree, contents:", list((workspace / ".reference").iterdir()))
# shutil.rmtree(test_path) # Don't actually delete
print("rmtree would delete resolved path")
PYRepository: stranske/Portable-Alpha-Extension-Model
Length of output: 3739
🏁 Script executed:
#!/bin/bash
echo "=== Check orchestrator_skill.py pack name handling ==="
rg -n -B3 -A3 "pack_name|pack_override" scripts/orchestrator_skill.py | head -60
echo "=== Search for ANY validation of pack names ==="
rg -n "validate.*pack|pack.*validate" scripts/
echo "=== Check tests for pack name edge cases ==="
rg -n "\.\.|path.*traversal|escape" tests/
echo "=== Verify: Can pack.name in config contain .. ? ==="
python3 - <<'PY'
# Simulate what happens when pack.name = "../../../etc"
pack_name = "../../../etc"
checkout_path = f".reference/{pack_name}"
print(f"pack.name = {pack_name}")
print(f"checkout_path = {checkout_path}")
from pathlib import Path
workspace_path = Path("/home/user/workspace").resolve()
full_path = workspace_path / checkout_path
print(f"workspace_path / checkout_path = {full_path}")
print(f"Escapes workspace? {not str(full_path.resolve()).startswith(str(workspace_path))}")
PYRepository: stranske/Portable-Alpha-Extension-Model
Length of output: 4101
🏁 Script executed:
#!/bin/bash
echo "=== Check _reject_local_runtime_values function ==="
rg -n -B2 -A10 "def _reject_local_runtime_values" scripts/orchestrator_skill.py
echo "=== Verify orchestrator_skill.py pack validation ==="
rg -n -B5 -A5 "pack_name = _require_nonempty_string" scripts/orchestrator_skill.py
echo "=== Final test: construct the malicious path for rmtree ==="
python3 - <<'PY'
from pathlib import Path
import shutil
# Simulate the actual code path from lines 369-371
workspace_path = Path("/home/user/workspace").resolve()
# Scenario: pack.name from config = "../../../etc/passwd"
pack_name_from_config = "../../../etc/passwd"
checkout_path_str = f".reference/{pack_name_from_config}"
checkout_path = workspace_path / checkout_path_str
print(f"Constructed path: {checkout_path}")
print(f"Resolved path: {checkout_path.resolve()}")
print(f"Within workspace? {str(checkout_path.resolve()).startswith(str(workspace_path))}")
# The issue: rmtree would receive this unresolved path
print(f"\nrmtree argument: {checkout_path}")
print(f"rmtree would resolve and delete: {checkout_path.resolve()}")
PYRepository: stranske/Portable-Alpha-Extension-Model
Length of output: 1391
🏁 Script executed:
#!/bin/bash
echo "=== Check FORBIDDEN_VALUE_MARKERS ==="
rg -n "FORBIDDEN_VALUE_MARKERS" scripts/orchestrator_skill.py -B5 -A5
echo "=== Verify: is there ANY normalization before rmtree? ==="
rg -n -B8 -A2 "shutil.rmtree" scripts/runner_lib/core.py
echo "=== Does Path.resolve() get called before rmtree anywhere? ==="
rg -n "resolve\(\)" scripts/runner_lib/core.py | grep -A2 -B2 "rmtree\|checkout"
echo "=== Final confirmation test ==="
python3 - <<'PY'
from pathlib import Path
# Exact code path from core.py line 369-371
workspace_path = Path("/workspace").resolve()
# Attacker sets pack.name = "../../../etc" in .github/reference_packs.json
# This creates checkout_path = f".reference/{../../../etc}"
checkout_path_from_config = ".reference/../../../etc"
checkout_path = workspace_path / checkout_path_from_config
print("Code at line 369-371:")
print(f" checkout_path = workspace_path / matching[0].checkout_path")
print(f" = {workspace_path} / {checkout_path_from_config}")
print(f" = {checkout_path}")
print()
print(f"Code at line 371:")
print(f" shutil.rmtree(checkout_path)")
print(f" receives: {checkout_path}")
print(f" rmtree resolves to: {checkout_path.resolve()}")
print(f" ESCAPES WORKSPACE? {not str(checkout_path.resolve()).startswith(str(workspace_path))}")
PYRepository: stranske/Portable-Alpha-Extension-Model
Length of output: 3139
Add path traversal validation to pack names before rmtree operation.
The security concern is valid. Pack names from both .github/reference_packs.json and .github/orchestrator_skill.json configs are validated only for non-empty strings and forbidden markers, but do not prevent path traversal characters like ... When pack.name = "../../../etc" is interpolated into checkout_path = f".reference/{pack.name}" and then joined with workspace_path, the resulting path .reference/../../../etc is passed to shutil.rmtree() which internally resolves it to /etc, escaping the workspace.
Remediation: Add path traversal validation to _require_nonempty_string() or introduce a _validate_pack_name() function that rejects names containing .., /, or other dangerous patterns—consistent with the existing validation in _validate_paths() at lines 101-102 of scripts/reference_packs.py.
🤖 Prompt for 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.
In `@scripts/runner_lib/core.py` around lines 369 - 372, The pack names from
configuration files are not validated against path traversal patterns before
being used in the checkout_path that is passed to shutil.rmtree(), creating a
security vulnerability where names like "../../../etc" can escape the workspace
directory. Add path traversal validation to reject pack names containing
dangerous patterns such as "..", "/", or other directory traversal characters.
This validation should be integrated into the existing
_require_nonempty_string() function or a new _validate_pack_name() function,
ensuring all pack names are sanitized before checkout_path is constructed and
passed to shutil.rmtree().
Comment on lines
+426
to
+431
| orchestrator_summary_raw = context.get("orchestrator_skill_summary_path") | ||
| orchestrator_summary_path = ( | ||
| Path(str(orchestrator_summary_raw)) if orchestrator_summary_raw else None | ||
| ) | ||
| if orchestrator_summary_path and not orchestrator_summary_path.is_absolute(): | ||
| orchestrator_summary_path = workspace / orchestrator_summary_path |
There was a problem hiding this comment.
Constrain ORCHESTRATOR_SKILL_SUMMARY_PATH to the workspace before reading.
orchestrator_skill_summary_path is read from environment and consumed directly if it points to any existing file. That allows arbitrary local file inclusion into the assembled prompt when this env var is influenced upstream.
🔧 Proposed fix
else:
orchestrator_summary_raw = context.get("orchestrator_skill_summary_path")
- orchestrator_summary_path = (
- Path(str(orchestrator_summary_raw)) if orchestrator_summary_raw else None
- )
- if orchestrator_summary_path and not orchestrator_summary_path.is_absolute():
- orchestrator_summary_path = workspace / orchestrator_summary_path
+ orchestrator_summary_path = None
+ if orchestrator_summary_raw:
+ candidate = Path(str(orchestrator_summary_raw))
+ if not candidate.is_absolute():
+ candidate = workspace / candidate
+ candidate = candidate.resolve()
+ try:
+ candidate.relative_to(workspace)
+ except ValueError as exc:
+ raise ValueError(
+ "orchestrator_skill_summary_path must be inside workspace"
+ ) from exc
+ orchestrator_summary_path = candidateAlso applies to: 461-465, 955-955
🤖 Prompt for 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.
In `@scripts/runner_lib/core.py` around lines 426 - 431, The
orchestrator_skill_summary_path variable is read from environment context and if
it is an absolute path, it is used without validation, which allows arbitrary
file inclusion. Modify the logic to ensure that orchestrator_summary_path is
always constrained to the workspace directory. Instead of allowing absolute
paths to be used directly, either reject absolute paths or resolve them relative
to the workspace root. The check should validate that the final resolved path is
actually within the workspace directory before using it. Apply the same fix to
the other occurrences mentioned at lines 461-465 and 955.
Comment on lines
+281
to
+287
| if model_override: | ||
| override_provider = selected_provider or (slots[0].provider if slots else "") | ||
| if override_provider and _is_model_blocked(override_provider, model_override): | ||
| logger.warning( | ||
| "Refusing blocked LLM model override: %s/%s", override_provider, model_override | ||
| ) | ||
| return None |
There was a problem hiding this comment.
Blocked override check is bound to the wrong provider in auto-slot mode.
The precheck validates model_override against slots[0].provider, but the override may actually be applied to a later slot/provider. That allows blocked combinations through when slot1 is skipped.
Suggested fix
slots = _resolve_slots()
model_override = model or os.environ.get(ENV_MODEL)
- if model_override:
- override_provider = selected_provider or (slots[0].provider if slots else "")
- if override_provider and _is_model_blocked(override_provider, model_override):
- logger.warning(
- "Refusing blocked LLM model override: %s/%s", override_provider, model_override
- )
- return None
used_override = False
for slot in slots:
slot_model = model_override if model_override and not used_override else slot.model
+ if _is_model_blocked(slot.provider, slot_model):
+ logger.warning("Skipping blocked LLM model override: %s/%s", slot.provider, slot_model)
+ continue
if slot.provider == PROVIDER_OPENAI and openai_token:
with contextlib.suppress(Exception):
client = _build_openai_client(Also applies to: 289-323
🤖 Prompt for 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.
In `@tools/langchain_client.py` around lines 281 - 287, The blocked model override
check in auto-slot mode only validates against slots[0].provider when
selected_provider is not set, but the override could be applied to any slot in
the list. Modify the blocking validation logic to check the model_override
against all available slots (not just the first one) to ensure that blocked
combinations are caught regardless of which slot is ultimately selected for the
override.
Comment on lines
+83
to
+95
| for raw_entry in payload.get("models", []): | ||
| if not isinstance(raw_entry, dict): | ||
| logger.warning("Ignoring invalid model registry entry in %s; expected object", path) | ||
| continue | ||
| provider = normalize_provider(str(raw_entry.get("provider", ""))) | ||
| model = str(raw_entry.get("model_id", "")).strip() | ||
| if not provider or not model: | ||
| continue | ||
| quality_payload = raw_entry.get("quality", {}) | ||
| quality = { | ||
| str(tier).upper(): float(score) | ||
| for tier, score in quality_payload.items() | ||
| if isinstance(score, int | float) |
There was a problem hiding this comment.
Harden registry schema handling to avoid crashes on malformed config.
load_model_registry() still assumes "models" is iterable and "quality" is a dict. Invalid JSON shapes can raise and take down client resolution instead of failing open to an empty registry.
Suggested fix
def load_model_registry() -> list[ModelRegistryEntry]:
@@
- entries: list[ModelRegistryEntry] = []
- for raw_entry in payload.get("models", []):
+ raw_models = payload.get("models", [])
+ if not isinstance(raw_models, list):
+ logger.warning("Invalid model registry format in %s; expected models list", path)
+ return []
+
+ entries: list[ModelRegistryEntry] = []
+ for raw_entry in raw_models:
if not isinstance(raw_entry, dict):
logger.warning("Ignoring invalid model registry entry in %s; expected object", path)
continue
@@
- quality_payload = raw_entry.get("quality", {})
- quality = {
- str(tier).upper(): float(score)
- for tier, score in quality_payload.items()
- if isinstance(score, int | float)
- }
+ quality_payload = raw_entry.get("quality", {})
+ if not isinstance(quality_payload, dict):
+ logger.warning(
+ "Ignoring invalid quality block in model registry entry for %s/%s",
+ provider,
+ model,
+ )
+ quality_payload = {}
+ quality = {
+ str(tier).upper(): float(score)
+ for tier, score in quality_payload.items()
+ if isinstance(score, (int, float))
+ }🤖 Prompt for 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.
In `@tools/llm_registry.py` around lines 83 - 95, The load_model_registry()
function needs to be hardened against malformed configuration data. Currently it
assumes the "models" value from payload is iterable and the "quality" value from
raw_entry is a dict, but invalid JSON shapes can cause crashes instead of
failing gracefully. Add type validation checks to ensure that the "models" field
is actually a list or dict before iterating, and verify that quality_payload is
actually a dict before calling .items() on it. If either validation fails, skip
processing that entry or use the default empty dict to allow the function to
fail open with an empty registry instead of crashing the application.
Owner
Author
|
Closing as stale: newer replacement sync PR #2011 exists from Workflows sync wave sync/workflows-591316374281 after stranske/Workflows#2498 merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Source SHA:
0b04de717dcadc23aea9e2eca0b8679d27e90666Template hash:
76689bc445fdSync branch:
sync/workflows-76689bc445fdConsumer repo:
stranske/Portable-Alpha-Extension-ModelManifest:
.github/sync-manifest.ymlSummary by CodeRabbit
Bug Fixes
owner/nameformat exactly.New Features
Refactor