Review this repo's PRs via OpenRouter in a single step #75
No reviewers
Labels
No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No assignees
3 participants
Notifications
Due date
No due date set.
Reference
marvin8/wuming!75
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/issue-74-openrouter-own-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?
The
pr-reviewjob now reviews every PR in a single WuMing invocation through OpenRouter instead of two invocations against Moonshot and DeepSeek:action.ymlexposes the four backend inputs added in #73 (openrouter_key,openai_base_url,openai_key,openai_model) and updates thebackenddescription to list all six valuesci.ymlmerges the Kimi and DeepSeek review steps into one OpenRouter run:code/securitystay on Kimi K2.7 Code (moonshotai/kimi-k2.7-code);config/docs/shell/testsmove to DeepSeek V4.1 Flash (deepseek/deepseek-v4.1-flash) — the same DeepSeek flash line, one version newer than the previousdeepseek-v4-flash(slugs verified against the live OpenRouter catalog)deepseek,moonshot,openrouter) now fail fast at startup when their API key is empty; the genericopenaibackend still allows keyless local serversCloses #74
WuMing
Found 5 issue(s). See inline comments below.
@ -20,2 +20,2 @@backend: moonshotmoonshot_key: ${{ secrets.wuming_moonshot_key }}backend: openrouteropenrouter_key: ${{ secrets.wuming_openrouter_key }}config [MEDIUM]
The job now depends on a repo secret
wuming_openrouter_keythat does not exist yet (per the PR note). Becauseopenrouter_keyin action.yml is declaredrequired: falsewithdefault: "", a missing secret is not rejected up front — the action runs with an empty key and fails deep inside the review, producing a confusing error. Create the secret before merging (or add an explicit guard step that fails fast when the key is empty).@ -29,3 +23,1 @@deepseek_key: ${{ secrets.wuming_deepseek_key }}pr_number: ${{ github.event.pull_request.number }}agents: config:deepseek-v4-flash,docs:deepseek-v4-flash,shell:deepseek-v4-flash,tests:deepseek-v4-flashagents: code:moonshotai/kimi-k2.7-code,security:moonshotai/kimi-k2.7-code,config:deepseek/deepseek-v4.1-flash,docs:deepseek/deepseek-v4.1-flash,shell:deepseek/deepseek-v4.1-flash,tests:deepseek/deepseek-v4.1-flashconfig [MEDIUM]
The model assignments are not actually preserved: the four config/docs/shell/tests agents previously ran
deepseek-v4-flash, but now usedeepseek/deepseek-v4.1-flash— a different model version, not just an OpenRouter vendor-namespaced form of the old slug. This silently changes review behaviour (and will hard-fail the whole job if the slug does not exist in the OpenRouter catalog). If the intent was a pure migration, use the vendor-prefixed slug of the existing model (e.g.deepseek/deepseek-v4-flash); if the upgrade is deliberate, call it out explicitly rather than describing it as 'preserving the model assignments'.@ -52,2 +66,4 @@required: falsedefault: ""agents:description: "Comma-separated list of agents, e.g. code:deepseek-v4-flash,security. Valid names: code, config, docs, performance, security, shell, tests."config [LOW]
The
agentsinput documentation was not updated for the new backends: it still shows only a bare model name (code:deepseek-v4-flash) and lists valid agent names, but never explains that OpenRouter requires fullvendor/modelslugs (e.g.moonshotai/kimi-k2.7-code). The newopenrouter_keydescription refers to 'OpenRouter vendor/model slugs' without saying where they go. Document theagent:vendor/modelformat (and per-backend model naming) in this description so users of the six documented backends know what to pass.@ -52,0 +54,4 @@required: falsedefault: ""openai_base_url:description: Base URL of any OpenAI-compatible endpoint (e.g. https://openrouter.ai/api/v1, http://localhost:1234/v1). Required when backend=openai.security [LOW]
A02: The OpenAI-compatible base URL example includes an unencrypted HTTP endpoint (http://localhost:1234/v1). Since openai_key will be sent to this URL, documenting an HTTP example risks API keys being transmitted in plaintext. Remove the HTTP example and only show HTTPS endpoints, or add explicit guidance that HTTP must not be used for remote/production endpoints.
@ -45,3 +45,3 @@## Choosing a backendWuMing supports four AI backends. Set `WUMING_BACKEND` to select one.WuMing supports six AI backends. Set `WUMING_BACKEND` to select one.docs [MEDIUM]
The backend count was bumped from four to six, but the "Choosing a backend" section that follows only documents four backends: DeepSeek (line 49), Kimi/Moonshot (ends line 102), OpenRouter (line 104) and the generic OpenAI-compatible endpoint (line 120). The max-tokens note further down (lines 160-162) also only names those same four as OpenAI-compatible, so readers are told there are six backends but can never discover the remaining two. Either add short subsections (with
WUMING_BACKENDvalue + required env vars) for the two missing backends, or keep the number in sync with what is actually documented. Verify the value againstaction.yml'sbackenddescription, and make sure the equivalent sentence in the Forgejo Actions guide is updated to the same number so the two guides do not contradict each other.53e7d2e54786df38571daction.yml✅ Fixed in commit
86df385— theopenai_base_urldescription now states: "Use HTTPS for remote endpoints; plain HTTP only for local servers." The localhost example stays because keyless local servers (LM Studio, vLLM) are the intended use case for plain HTTP, but the guidance is now explicit.action.yml✅ Fixed in commit
86df385— theagentsdescription now shows avendor/modelexample (security:moonshotai/kimi-k2.7-code), notes model names are backend-specific, and theopenrouter_keydescription now says explicitly: "override per agent via agents: name:vendor/model (e.g. code:moonshotai/kimi-k2.7-code)".docs/getting-started/deploying-on-woodpecker.md🔴 Not a defect — the section documents all six. Current headers in
deploying-on-woodpecker.md:### DeepSeek (default)(line 49),### Anthropic (Claude)(line 64),### Ollama (self-hosted)(line 77),### Moonshot (Kimi)(line 90),### OpenRouter(line 104),### Any OpenAI-compatible endpoint (generic)(line 120). The diff hunks this review saw only contain the newly added sections, so the pre-existing Anthropic and Ollama sections were outside the reviewed context.WuMing
Found 3 issue(s). See inline comments below.
@ -52,2 +67,3 @@default: ""agents:description: "Comma-separated list of agents, e.g. code:deepseek-v4-flash,security. Valid names: code, config, docs, performance, security, shell, tests."description: "Comma-separated list of agents, e.g. code:deepseek-v4-flash,security:moonshotai/kimi-k2.7-code. Model names are backend-specific; OpenRouter uses vendor/model slugs. Valid names: code, config, docs, performance, security, shell, tests."config [LOW]
The example model in the agents description (
code:deepseek-v4-flash) is the old, un-prefixed slug and is not a valid OpenRouter vendor/model id, which directly contradicts the following sentence ('OpenRouter uses vendor/model slugs'). Users copying this example for backend=openrouter will get an invalid-model error. Update the example to a prefixed slug, e.g.code:deepseek/deepseek-v4.1-flash,security:moonshotai/kimi-k2.7-code, to match the values actually used in .forgejo/workflows/ci.yml.@ -96,2 +112,4 @@WUMING_OLLAMA_URL: ${{ inputs.ollama_url }}WUMING_MOONSHOT_KEY: ${{ inputs.moonshot_key }}WUMING_OPENROUTER_KEY: ${{ inputs.openrouter_key }}WUMING_OPENAI_BASE_URL: ${{ inputs.openai_base_url }}security [HIGH]
The generic OpenAI backend passes a user-controlled
openai_base_urldirectly to the HTTP client viaWUMING_OPENAI_BASE_URLwithout allow-list validation. An attacker who can influence this input can redirect LLM requests to an internal or cloud metadata endpoint (e.g., 169.254.169.254) and exfiltrate the API key, constituting an A10 Server-Side Request Forgery vulnerability. Add an allow-list of permitted base URLs or restrict to known safe endpoints before use.@ -111,0 +115,4 @@with:forgejo_token: ${{ secrets.WUMING_FORGEJO_TOKEN }}backend: openrouteropenrouter_key: ${{ secrets.WUMING_OPENROUTER_KEY }}docs [LOW]
The example reads the secret
WUMING_OPENROUTER_KEY, while the PR note says the secret that must exist on this repo iswuming_openrouter_key. Align the secret name/casing in the docs with the name actually used byci.yml; otherwise the snippet is not copy-pasteable and readers will end up with an empty OpenRouter key..forgejo/workflows/ci.ymlCorrect observation, and the move is intentional (maintainer decision): V4.1 Flash is the current generation of the same DeepSeek flash line and matches the new
openrouterpreset default. The PR body's "preserving the model assignments" wording was inaccurate and has been corrected to state the version change explicitly. Exact parity was considered (deepseek/deepseek-v4-flashexists on OpenRouter) and declined in favour of the newer flash..forgejo/workflows/ci.yml✅ Two parts. The secret now exists — this PR's own
pr-reviewran end-to-end through OpenRouter and posted its review successfully. And the confusing-failure concern is addressed in commitaf7066a: hosted backends (deepseek,moonshot,openrouter) now raiseValueError("WUMING_<KEY> is required when WUMING_BACKEND=<backend>")at startup when the key is empty. The genericopenaibackend deliberately still allows an empty key for keyless local servers, covered by a test.WuMing
Found 6 issue(s). See inline comments below.
@ -52,0 +58,4 @@required: falsedefault: ""openai_key:description: API key for the generic OpenAI-compatible endpoint. Required when backend=openai.config [LOW]
This description states the key is "Required when backend=openai", but the PR (and the fail-fast change it describes) explicitly keeps the generic
openaibackend keyless for local servers. This contradiction will mislead users of local OpenAI-compatible endpoints (e.g. http://localhost:1234/v1) into thinking a key is mandatory. Reword to reflect that the key is optional and only needed for endpoints that authenticate, e.g. "API key for the generic OpenAI-compatible endpoint. Optional; leave empty for keyless local servers."@ -259,3 +264,4 @@)return OpenAICompatibleBackend(base_url,getattr(config, key_attribute),code [LOW]
The API key was already fetched into
api_keyon line 259 and validated. Callinggetattr(config, key_attribute)again is redundant and slightly obscures the validated value. Passapi_keydirectly toOpenAICompatibleBackend.@ -814,6 +814,34 @@ def test_build_backend_openai_requires_base_url() -> None:expect(raised).to_be_truthy()@test("_build_backend raises ValueError when a hosted preset has an empty key")code [LOW]
The looped test uses a single
raisedflag, so a failure does not indicate which backend failed to raise. Prefer parameterized tests or includebackend_namein the assertion message for clearer diagnostics.@ -817,0 +823,4 @@]:raised = Falsetry:_build_backend(_make_config(backend_name), MagicMock())code [MEDIUM]
The test relies on
_make_configdefaults to leave the relevant API key empty. If_make_configis later changed to provide a default key, this test will either stop exercising the empty-key path or fail with a confusing message. Explicitly clear the key for the backend under test, e.g.config = replace(_make_config(backend_name), **{key_attribute: ""}).@ -817,0 +827,4 @@except ValueError as error:raised = Trueenv_var = f"WUMING_{key_attribute.upper()}"expect(str(error)).to_contain(env_var)code [LOW]
The assertion only checks that the env var name appears in the error message. It should also verify the backend name is included, e.g.
expect(str(error)).to_contain(backend_name), to ensure the message matches the implementation.@ -817,0 +834,4 @@@test("_build_backend allows an empty key for the generic openai backend")def test_build_backend_openai_allows_empty_key() -> None:config = replace(_make_config("openai"),code [MEDIUM]
The test claims to verify that the generic openai backend allows an empty key, but it does not explicitly set
openai_key="". It depends on_make_config("openai")defaulting to an empty key. Addopenai_key=""to thereplace(...)call so the precondition is explicit and the test remains valid if defaults change.action.yml🔴 Not a defect — the description intentionally shows one example per style:
code:deepseek-v4-flashis a valid model name for the DeepSeek backend (it is the DeepSeek preset's default model), andsecurity:moonshotai/kimi-k2.7-codeshows the OpenRoutervendor/modelform. The sentence that follows — "Model names are backend-specific; OpenRouter uses vendor/model slugs" — explains exactly this distinction, so a reader onbackend=openrouteris directed to the slug form.docs/getting-started/deploying-on-forgejo-actions.md🔴 Not a defect — the two names refer to different repos on purpose. The docs snippets are templates for readers configuring their own repos, where the secret name is whatever the reader creates (the guide's snippets consistently use the
WUMING_*uppercase convention, as withWUMING_DEEPSEEK_KEYandWUMING_MOONSHOT_KEY)..forgejo/workflows/ci.ymlreferences this repo's actual secret (wuming_openrouter_key), matching how this repo has always named its secrets (previouslywuming_moonshot_key,wuming_deepseek_key).src/wuming/main.py✅ Fixed in commit
6c6d8e9— the validatedapi_keylocal is now passed directly toOpenAICompatibleBackend.tests/test_main.py✅ Fixed in commit
6c6d8e9— the parametrised test now explicitly clears the key:replace(_make_config(backend_name), **{key_attribute: ""}), so the empty-key precondition no longer depends on_make_configdefaults.tests/test_main.py✅ Fixed in commit
6c6d8e9—openai_key=""is now an explicit part of thereplace(...)call.tests/test_main.py✅ Fixed in commit
6c6d8e9— the test now asserts bothWUMING_<KEY>andbackend_nameappear in the message.tests/test_main.py✅ Fixed in commit
6c6d8e9— the loop is now a real parametrised test (@test(...).cases(...)with one case per backend), so each backend reports independently.action.yml✅ Fixed in commit
6c6d8e9— theopenai_keydescription now reads: "May be empty for keyless local servers (e.g. LM Studio); required for hosted endpoints." The same contradiction was corrected in the README configuration table and the project agent docs (AGENTS.md), which carried the identical wording from #73.WuMing
Found 3 issue(s). See inline comments below.
@ -257,2 +257,4 @@if preset is not None:base_url, default_model, key_attribute = presetapi_key = getattr(config, key_attribute)if not api_key:code [LOW]
The guard
if not api_keyrejects empty strings and None, but it accepts all-whitespace strings (e.g.," "). A whitespace-only key will be passed to the provider and fail later. Considerif not api_key or not api_key.strip():and add a test for whitespace-only keys.@ -817,0 +828,4 @@_build_backend(config, MagicMock())except ValueError as error:raised = Trueexpect(str(error)).to_contain(f"WUMING_{key_attribute.upper()}")code [LOW]
Deriving the expected env-var name with the same
key_attribute.upper()expression used in the implementation makes this assertion tautological. If the implementation used the wrong prefix or attribute name, the test would still pass. Hardcode the expected strings per backend (e.g.,"WUMING_DEEPSEEK_KEY").@ -817,0 +842,4 @@openai_model="my-model",)backend = _build_backend(config, MagicMock())expect(backend).not_.to_be_none()code [LOW]
Only checking
backendis not None is a weak assertion for the keyless-openai case. The test should also verify the returned object is anOpenAICompatibleBackendand that the empty key, base URL, and model were propagated correctly; otherwise regressions that silently substitute defaults could be missed.src/wuming/main.py✅ Fixed in commit
bd214a4— the guard is nowif not api_key or not api_key.strip():, and a new test covers a whitespace-onlydeepseek_keyraising the startupValueError.tests/test_main.py✅ Fixed in commit
bd214a4— each case now carries a hardcodedexpected_env_var(WUMING_DEEPSEEK_KEY,WUMING_MOONSHOT_KEY,WUMING_OPENROUTER_KEY) asserted verbatim, so a wrong prefix or attribute name in the implementation can no longer pass.tests/test_main.py✅ Fixed in commit
bd214a4— the keyless-openai test is now behavioural: it routes a MockTransport through_build_backend, asserts the backend type, then verifies the propagated configuration end-to-end (request URLhttp://localhost:1234/v1/chat/completions, modelmy-model, and noAuthorizationheader).WuMing
Found 13 issue(s). See inline comments below.
@ -97,3 +97,3 @@- `WUMING_OPENROUTER_KEY` — OpenRouter API key (required if backend=openrouter; default model `deepseek/deepseek-v4.1-flash`)- `WUMING_OPENAI_BASE_URL` — base URL of any OpenAI-compatible endpoint (required if backend=openai)- `WUMING_OPENAI_KEY` — API key for the generic OpenAI-compatible endpoint (required if backend=openai)- `WUMING_OPENAI_KEY` — API key for the generic OpenAI-compatible endpoint; may be empty for keyless local servers (e.g. LM Studio)docs [LOW]
Same wording problem as the README table: the previous '(required if backend=openai)' qualifier was removed entirely, while the
WUMING_OPENAI_BASE_URLandWUMING_OPENAI_MODELentries still say they are required. State that the key is required for hosted OpenAI-compatible endpoints and only optional for keyless local servers, to keep the env-var list internally consistent.@ -150,7 +150,7 @@ See the [Forgejo/GitHub Actions guide](https://wuming.marvin8.zone/getting-start| `WUMING_MOONSHOT_KEY` | — | Moonshot (Kimi) API key (required if backend=moonshot) || `WUMING_OPENROUTER_KEY` | — | OpenRouter API key (required if backend=openrouter); default model `deepseek/deepseek-v4.1-flash`, override per agent via `name:model` using OpenRouter `vendor/model` slugs || `WUMING_OPENAI_BASE_URL` | — | Base URL of any OpenAI-compatible endpoint, e.g. `https://openrouter.ai/api/v1` (required if backend=openai) |docs [LOW]
This row still advertises
https://openrouter.ai/api/v1as the example for the genericopenaibackend, which now duplicates the dedicatedopenrouterbackend added in this PR and may steer users to the wrong (and less capable) configuration. Use an example from a provider without a dedicated backend (e.g.https://api.groq.com/openai/v1) and point readers to theopenrouterbackend for OpenRouter.@ -151,3 +151,3 @@| `WUMING_OPENROUTER_KEY` | — | OpenRouter API key (required if backend=openrouter); default model `deepseek/deepseek-v4.1-flash`, override per agent via `name:model` using OpenRouter `vendor/model` slugs || `WUMING_OPENAI_BASE_URL` | — | Base URL of any OpenAI-compatible endpoint, e.g. `https://openrouter.ai/api/v1` (required if backend=openai) || `WUMING_OPENAI_KEY` | — | API key for the generic OpenAI-compatible endpoint (required if backend=openai) || `WUMING_OPENAI_KEY` | — | API key for the generic OpenAI-compatible endpoint; may be empty for keyless local servers (e.g. LM Studio) |docs [LOW]
The rewrite dropped the '(required if backend=openai)' qualifier that the two neighbouring rows (
WUMING_OPENAI_BASE_URL,WUMING_OPENAI_MODEL) still carry, so the table no longer says when an API key is actually needed. Keep the requirement and add the keyless exception, e.g. 'API key for the generic OpenAI-compatible endpoint (required for hosted endpoints; may be empty for keyless local servers such as LM Studio)'.@ -50,2 +50,4 @@required: falsedefault: ""openrouter_key:description: "OpenRouter API key. Required when backend=openrouter. Default model deepseek/deepseek-v4.1-flash; override per agent via agents: name:vendor/model (e.g. code:moonshotai/kimi-k2.7-code)."config [LOW]
The OpenRouter default model is documented inside the
openrouter_keydescription instead of a dedicated input, and there is no way to change the default for all agents (only per-agent overrides viaagents). This is inconsistent with the generic backend, which exposesopenai_modelas the default-model input. Consider adding anopenrouter_modelinput (andWUMING_OPENROUTER_MODELenv mapping) or explicitly documenting that the OpenRouter default is fixed and must be overridden per agent.@ -52,0 +54,4 @@required: falsedefault: ""openai_base_url:description: Base URL of any OpenAI-compatible endpoint (e.g. https://openrouter.ai/api/v1, http://localhost:1234/v1). Required when backend=openai. Use HTTPS for remote endpoints; plain HTTP only for local servers.config [MEDIUM]
The generic
openaibackend accepts an arbitrary base URL, and nothing enforces the documented "Use HTTPS for remote endpoints" guidance. A misconfigured plain-HTTP remote URL will transmitopenai_keyin cleartext. Recommend rejecting non-HTTPS base URLs unless the host is a loopback/local address (e.g. localhost, 127.0.0.1, ::1), rather than relying on a doc comment.@ -52,0 +62,4 @@required: falsedefault: ""openai_model:description: Default model for the generic OpenAI-compatible endpoint. Required when backend=openai.config [MEDIUM]
openai_model(andopenai_base_url) are documented as required when backend=openai, but unlike the hosted backends they are not validated/failed fast. The PR explicitly adds fail-fast for deepseek/moonshot/openrouter keys while leavingopenaiunvalidated, so a user setting backend=openai without a model (or base URL) gets a late, opaque error from the HTTP client instead of a clear startup message. Add a startup check that errors out when backend=openai and openai_base_url/openai_model are empty.@ -52,2 +67,3 @@default: ""agents:description: "Comma-separated list of agents, e.g. code:deepseek-v4-flash,security. Valid names: code, config, docs, performance, security, shell, tests."description: "Comma-separated list of agents, e.g. code:deepseek-v4-flash,security:moonshotai/kimi-k2.7-code. Model names are backend-specific; OpenRouter uses vendor/model slugs. Valid names: code, config, docs, performance, security, shell, tests."config [LOW]
The example mixes model namespaces from two different backends in a single
agentsvalue (code:deepseek-v4-flashis a native DeepSeek model,security:moonshotai/kimi-k2.7-codeis an OpenRouter slug). Sincebackenddefaults todeepseek, a user copying this example verbatim would get an invalid model for thesecurityagent (and the reverse with backend=openrouter). Give one example per backend, e.g.code:deepseek-v4-flash,securityfor deepseek andcode:moonshotai/kimi-k2.7-code,security:deepseek/deepseek-v4.1-flashfor openrouter.@ -111,0 +121,4 @@Obtain a key at [openrouter.ai/keys](https://openrouter.ai/keys). One account givesaccess to models from many providers (DeepSeek, Moonshot, Anthropic, Google, Metaand more). The default model is `deepseek/deepseek-v4.1-flash`; override per agentwith OpenRouter `vendor/model` slugs via per-agent model selection.docs [MEDIUM]
The new OpenRouter section tells readers to override models 'with OpenRouter
vendor/modelslugs' but shows no example, and the only model-override example in this guide ('Selecting agents',code:deepseek-v4-pro,config,security) uses a bare, non-namespaced slug that is invalid on OpenRouter. Add a namespaced example here, e.g.agents: code:moonshotai/kimi-k2.7-code,config:deepseek/deepseek-v4.1-flash, and note that bare DeepSeek/Kimi slugs used elsewhere in this page are for the native backends only.@ -111,0 +131,4 @@forgejo_token: ${{ secrets.WUMING_FORGEJO_TOKEN }}backend: openaiopenai_base_url: https://llm.example/v1openai_key: ${{ secrets.WUMING_OPENAI_KEY }}docs [LOW]
The example always supplies
openai_keyfrom a secret, but the prose immediately below (lines 139-140) says the key may be empty for keyless local servers. Either drop the line from the example or add a comment such as '# omit for keyless local servers (e.g. LM Studio)' so the example and the note agree.@ -104,0 +115,4 @@Obtain a key at [openrouter.ai/keys](https://openrouter.ai/keys). One account givesaccess to models from many providers (DeepSeek, Moonshot, Anthropic, Google, Metaand more). The default model is `deepseek/deepseek-v4.1-flash`; override per agentwith OpenRouter `vendor/model` slugs via per-agent model selection.docs [MEDIUM]
Same gap as the Forgejo guide: the OpenRouter section requires
vendor/modelslugs for per-agent overrides but gives no example, while the 'Selecting agents' example below usescode:deepseek-v4-pro, which would fail on OpenRouter. Add a namespaced example (e.g.WUMING_AGENTS: code:moonshotai/kimi-k2.7-code,config:deepseek/deepseek-v4.1-flash) so readers do not copy an invalid model slug.@ -104,0 +123,4 @@environment:WUMING_BACKEND: openaiWUMING_OPENAI_BASE_URL: https://llm.example/v1WUMING_OPENAI_KEY:docs [LOW]
This example passes
WUMING_OPENAI_KEYfrom a secret even though the note at lines 134-135 states it may be empty for keyless local servers such as LM Studio. Add a comment that this variable can be omitted for keyless endpoints, or provide a keyless variant of the snippet.@ -257,2 +257,4 @@if preset is not None:base_url, default_model, key_attribute = presetapi_key = getattr(config, key_attribute)if not api_key or not api_key.strip():code [MEDIUM]
The key-validation guard applies indiscriminately to every entry in _OPENAI_COMPATIBLE_PRESETS. If the generic 'openai' backend is also represented in that dict, this check would reject keyless local servers and contradict the PR requirement that the generic openai backend still allows an empty key. Make the hosted-backend validation explicit—e.g. validate only for deepseek/moonshot/openrouter—or ensure openai is handled by a separate, keyless code path.
@ -817,0 +835,4 @@expected_env_var="WUMING_OPENROUTER_KEY",),)def test_build_backend_hosted_preset_requires_key(code [LOW]
The new tests only cover empty/whitespace keys for hosted presets. There is no positive test verifying that deepseek/moonshot/openrouter with a valid, non-empty key still successfully return an OpenAICompatibleBackend. Add a parameterized happy-path case to guard against regressions in the normal success path.
README.md✅ Fixed in commit
673b944— theWUMING_OPENAI_KEYrow now reads: "may be empty for keyless local servers (e.g. LM Studio), required for hosted endpoints", so the table states when the key is actually needed.AGENTS.md✅ Fixed in commit
673b944— same wording as the README row now: "may be empty for keyless local servers (e.g. LM Studio), required for hosted endpoints".README.md✅ Fixed in commit
673b944— the generic-backend row now uses a neutral example (https://llm.example/v1) instead of duplicating the OpenRouter URL, so readers are not steered onto the dedicated backend by accident.docs/getting-started/deploying-on-forgejo-actions.md✅ Fixed in commit
673b944— the per-agent model section now includes an OpenRouter example:agents: code:moonshotai/kimi-k2.7-code,security:moonshotai/kimi-k2.7-code.docs/getting-started/deploying-on-woodpecker.md✅ Fixed in commit
673b944— same OpenRouter slug example added to the Woodpecker guide's per-agent section (WUMING_AGENTS: code:moonshotai/kimi-k2.7-code,...).docs/getting-started/deploying-on-forgejo-actions.md✅ Fixed in commit
673b944— theopenai_keyline in the example now carries the comment# omit for keyless local servers, matching the prose.docs/getting-started/deploying-on-woodpecker.md✅ Fixed in commit
673b944— same comment added to the Woodpecker example (# omit for keyless local servers).src/wuming/main.py🔴 Not reachable —
_OPENAI_COMPATIBLE_PRESETScontains only the three hosted presets (deepseek,moonshot,openrouter); the genericopenaibackend is handled by a separate branch in_build_backendwith its own checks and deliberately no key requirement. A future entry in that dict would by definition be a hosted preset with a key requirement, so the guard's placement matches the data it guards.tests/test_main.py🔴 False premise — positive coverage already exists:
test_build_backend_deepseek_preset,test_build_backend_moonshot_preset, andtest_build_backend_openrouter_preseteach set a non-empty key, build the backend, and complete a full request through a MockTransport, asserting endpoint URL, default model, and auth header. These predate the key-validation change.action.yml🔴 False premise —
WUMING_OPENAI_BASE_URLandWUMING_OPENAI_MODELare fail-fast validated in_build_backend(ValueErrorat startup when either is empty), since commitd98e9d0. Covered bytest_build_backend_openai_requires_base_urlandtest_build_backend_openai_requires_model.action.yml🔴 Same rationale as the earlier reply on the
agentsdescription (see the 🔴 reply to the previous wave's finding): the two examples deliberately illustrate both naming styles — a bare slug for the DeepSeek backend and avendor/modelslug for OpenRouter — directly matching the "Model names are backend-specific" sentence that follows. The description is backend-agnostic: it cannot use only one namespace without being wrong for the other backend.src/wuming/main.py🔴 Declined (maintainer decision).
WUMING_OPENAI_BASE_URLis operator-controlled CI configuration: whoever can set it in a workflow already has write access to the repo and its secrets, so there is no attacker-controlled input path — the threat model for SSRF exfiltration doesn't apply. Allow-listing endpoints would defeat the purpose of the generic backend (any OpenAI-compatible endpoint), and the hosted presets (deepseek,moonshot,openrouter) use fixed URLs and are unaffected.action.yml🔴 Declined (maintainer decision, same family as the SSRF finding on
src/wuming/main.py). The "Use HTTPS for remote endpoints" wording is guidance for operators, not an enforced invariant: the generic backend explicitly supports plain-HTTP local servers (LM Studio, vLLM), and the operator choosing a remote HTTP URL is the same trust level that already controls the workflow and its secrets. Enforcement here would block legitimate setups without closing a real attack path.action.yml🔁 Tracked as a follow-up: marvin8/wuming#76 — "Allow a global default-model override for the openrouter backend". The per-agent
name:vendor/modelsyntax covers per-run choices today; a global default knob for hosted presets (parallel toWUMING_OPENAI_MODELon the generic backend) is a feature, not a defect in this PR.