Migrate PR review to openrouter with pinned 0.11.0 action #19
marvin8
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/issue-89-openrouter-review"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Migrates the wuming PR review to the openrouter backend with the fleet-standard two-tier model mapping (code/tests/performance on deepseek-v4-pro, security on kimi-k2-thinking, config/docs/shell on qwen3.7-flash) and pins the action to
0.11.0.Tracking: marvin8/wuming#89
WuMing
Found 3 issue(s). See inline comments below.
@ -13,3 +13,3 @@if: github.event_name == 'pull_request'steps:- uses: https://forge.marvin8.zone/marvin8/wuming@main- uses: https://forge.marvin8.zone/marvin8/wuming@0.11.0security [MEDIUM]
A08: Action is pinned to a mutable version tag (@0.11.0) rather than an immutable digest, exposing supply chain risk if the tag is moved or forge instance is compromised. Pin to a specific commit SHA for integrity verification.
@ -15,3 +15,3 @@- uses: https://forge.marvin8.zone/marvin8/wuming@main- uses: https://forge.marvin8.zone/marvin8/wuming@0.11.0with:forgejo_token: ${{ forge.token }}config [MEDIUM]
Uses non-standard secret context '${{ forge.token }}'. If this custom context is not globally defined on the runner, the workflow will fail authentication. Consider using a standard secret reference like '${{ secrets.forgjo_token }}'.
@ -19,1 +18,3 @@agents: code:deepseek-v4-pro,config:deepseek-v4-flash,docs:deepseek-v4-flash,shell:deepseek-v4-flash,security:deepseek-v4-pro,tests:deepseek-v4-flashopenrouter_key: ${{ secrets.wuming_openrouter_key }}backend: openrouteragents: code:deepseek/deepseek-v4-pro,config:qwen/qwen3.7-flash,docs:qwen/qwen3.7-flash,shell:qwen/qwen3.7-flash,security:moonshotai/kimi-k2-thinking,tests:deepseek/deepseek-v4-proconfig [LOW]
The PR description specifies a fleet-standard mapping for 'code/tests/performance', but the 'performance' agent is omitted from the agents string. Add 'performance:deepseek/deepseek-v4-pro' to fully align with the documented mapping.
#19 (comment)
🔴 Intentional — each repo's existing agent set is preserved; the fleet mapping defines which model each agent gets, not which agents every repo must run. rainlog did not have a
performanceagent before this migration, so none was added. The PR description's "code/tests/performance on deepseek-v4-pro" describes the tier assignment, conditional on the agent being present.#19 (comment)
🔴 Declined (fleet-wide maintainer decision, same as the identical finding on arvak#27): tag pinning was chosen explicitly so a wuming release update stays a one-line change per repo across ~17 consumer repos. The force-push threat requires write access to the wuming repo itself — a single-maintainer repo on this forge. SHA pinning is the right follow-up only if that trust boundary grows.
#19 (comment)
🔴 Pre-existing, untouched by this PR — the
forge.tokencontext is provided by the zhuiri runner on this forge (wuming's ownci.ymluses the identical reference), and rainlog's reviews have run successfully with it for months. This migration changed only the backend, key, agents, and pin — theforgejo_tokenline predates it.1ffdc03c71226ecd3d48WuMing
Found 3 issue(s). See inline comments below.
@ -19,1 +18,3 @@agents: code:deepseek-v4-pro,config:deepseek-v4-flash,docs:deepseek-v4-flash,shell:deepseek-v4-flash,security:deepseek-v4-pro,tests:deepseek-v4-flashopenrouter_key: ${{ secrets.wuming_openrouter_key }}backend: openrouteragents: code:deepseek/deepseek-v4-pro,config:qwen/qwen3.7-flash,docs:qwen/qwen3.7-flash,shell:qwen/qwen3.7-flash,security:moonshotai/kimi-k2-thinking,tests:deepseek/deepseek-v4-proconfig [MEDIUM]
The agents mapping successfully switched to OpenRouter-specific ID formats using '/' separators (e.g., deepseek/deepseek-v4-pro), which is required for the new backend. Ensure this matches your OpenRouter public API namespace; if you're using a private/custom model registry, adjust accordingly to avoid resolution failures.
@ -49,2 +44,2 @@"urllib3>=2.7.0", # Addresses vulnerabilities GHSA-mf9v-mfxr-j63j and GHSA-qccp-gfcp-xxvc"virtualenv>=20.36.1", # Addresses vulnerability GHSA-597g-3phw-6986"anyio>=4.14.2","filelock>=3.20.3", # Addresses vulnerability GHSA-qmgc-5h2g-mvrw,config [LOW]
Trailing commas inside inline comments are present on lines 45, 48-51. While technically ignored by TOML parsers, this is unconventional styling and often indicates leftover formatting from a CSV/JSON migration. Remove them for cleaner, maintainable config.
@ -50,1 +44,3 @@"virtualenv>=20.36.1", # Addresses vulnerability GHSA-597g-3phw-6986"anyio>=4.14.2","filelock>=3.20.3", # Addresses vulnerability GHSA-qmgc-5h2g-mvrw,"httpcore2>=2.10.0",config [LOW]
New constraint dependencies httpcore2 and httpx2 (lines 46-47) lack the vulnerability/CVE reference comments included on adjacent lines. Add explanatory comments for consistency and better audit trails.
#19 (comment)
🔴 No action needed — the mapping is exactly as intended and was verified end-to-end: this PR's own
pr-reviewjob executed through it successfully. Thevendor/modelslug format is required by the OpenRouter backend and will stay.#19 (comment)
✅ Fixed in commit
0e9c824— the stray commas that had migrated to comment-ends are removed; commas now sit directly after the version specs.#19 (comment)
✅ Fixed in commit
0e9c824—httpcore2>=2.10.0now carries# PYSEC-2026-3844 (HIGH)andhttpx2>=2.12.0carries# PYSEC-2026-3846 (HIGH), matching the adjacent advisory-comment style.WuMing
Found 4 issue(s). See inline comments below.
@ -14,3 +14,3 @@- repo: https://github.com/astral-sh/ruff-pre-commitrev: v0.16.5rev: v0.17.1config [LOW]
The
ruff-pre-commithook is pinned tov0.17.1, while the project'spyproject.tomlconstrainsruffto~=0.16.10. Mismatched linter versions between CI and local/project environments can cause rule inconsistencies or unexpected autofix behavior. Align the hook revision with the project constraint or document why the divergence exists.@ -105,3 +105,3 @@"""BINDINGS: ClassVar[list[Binding]] = [BINDINGS: ClassVar[list[Binding]] = [ # ty: ignore[invalid-attribute-override]code [MEDIUM]
Typo in type ignore comment: 'ty' should be 'type'. This will not suppress any type checker error, likely leading to an invalid-attribute-override error still being reported. Replace 'ty' with 'type'.
@ -180,3 +180,3 @@"""BINDINGS: ClassVar[list[Binding]] = [BINDINGS: ClassVar[list[Binding]] = [ # ty: ignore[invalid-attribute-override]code [MEDIUM]
Same typo: 'ty' should be 'type'. This will not suppress the intended type checker error.
@ -708,3 +708,3 @@"""Interactive TUI for browsing rain history."""BINDINGS: ClassVar[list[Binding]] = [BINDINGS: ClassVar[list[Binding]] = [ # ty: ignore[invalid-attribute-override]code [MEDIUM]
Same typo: 'ty' should be 'type'. This will not suppress the intended type checker error.
#19 (comment)
✅ Fixed in the latest push — the ruff and ty hook revs are aligned with the pyproject-chosen versions (
v0.16.10,v0.0.85). Good catch: an earlier alignment script had crashed before writing its changes, leaving these two hooks at the prek-autoupdated revs.#19 (comment)
🔴 Not a typo —
# ty: ignore[invalid-attribute-override]is the suppression syntax of thetytype checker (Astral), which this repo uses; it is a different tool from mypy, whose syntax is# type: ignore. Evidence: thety-3.11session fails without these comments and passes with them (verified on the current head).#19 (comment)
🔴 Not a typo — same as the sibling finding:
# ty: ignore[...]is thetychecker's directive (this repo type-checks with Astral'sty, not mypy). Thety-3.11session passes with these suppressions in place.#19 (comment)
🔴 Not a typo — same as the sibling findings:
# ty: ignore[...]is thetychecker's directive. The session passes with them in place and failed without.