test/e2e-scenario/live/runtime-overrides.test.ts (1)

347-380: ⚡ Quick win

Collapse repeated invalid-override assertions into a table-driven loop.

Line 347 through Line 380 repeats the same assertion shape six times, which unnecessarily increases function complexity. Converting this block into case-table iteration keeps the scenario easier to extend and aligns with the repo’s “low complexity” guidance.

♻️ Suggested refactor

-      expect(
-        runOverrideStderr(dockerLog, image, "invalid model override", {
-          NEMOCLAW_MODEL_OVERRIDE: "bad\u0001model",
-        }),
-      ).toContain("control characters");
-      expect(
-        runOverrideStderr(dockerLog, image, "invalid context window", {
-          NEMOCLAW_MODEL_OVERRIDE: "test",
-          NEMOCLAW_CONTEXT_WINDOW: "notanumber",
-        }),
-      ).toContain("must be a positive integer");
-      expect(
-        runOverrideStderr(dockerLog, image, "invalid max tokens", {
-          NEMOCLAW_MODEL_OVERRIDE: "test",
-          NEMOCLAW_MAX_TOKENS: "abc",
-        }),
-      ).toContain("must be a positive integer");
-      expect(
-        runOverrideStderr(dockerLog, image, "invalid reasoning", {
-          NEMOCLAW_MODEL_OVERRIDE: "test",
-          NEMOCLAW_REASONING: "maybe",
-        }),
-      ).toContain('must be "true" or "false"');
-      expect(
-        runOverrideStderr(dockerLog, image, "invalid CORS origin", {
-          NEMOCLAW_CORS_ORIGIN: "ftp://evil.com",
-        }),
-      ).toContain("must start with http");
-      expect(
-        runOverrideStderr(dockerLog, image, "invalid inference API", {
-          NEMOCLAW_MODEL_OVERRIDE: "test",
-          NEMOCLAW_INFERENCE_API_OVERRIDE: "graphql",
-        }),
-      ).toContain("openai-completions");
+      const invalidCases: Array<{
+        label: string;
+        env: Record<string, string>;
+        expected: string;
+      }> = [
+        {
+          label: "invalid model override",
+          env: { NEMOCLAW_MODEL_OVERRIDE: "bad\u0001model" },
+          expected: "control characters",
+        },
+        {
+          label: "invalid context window",
+          env: { NEMOCLAW_MODEL_OVERRIDE: "test", NEMOCLAW_CONTEXT_WINDOW: "notanumber" },
+          expected: "must be a positive integer",
+        },
+        {
+          label: "invalid max tokens",
+          env: { NEMOCLAW_MODEL_OVERRIDE: "test", NEMOCLAW_MAX_TOKENS: "abc" },
+          expected: "must be a positive integer",
+        },
+        {
+          label: "invalid reasoning",
+          env: { NEMOCLAW_MODEL_OVERRIDE: "test", NEMOCLAW_REASONING: "maybe" },
+          expected: 'must be "true" or "false"',
+        },
+        {
+          label: "invalid CORS origin",
+          env: { NEMOCLAW_CORS_ORIGIN: "ftp://evil.com" },
+          expected: "must start with http",
+        },
+        {
+          label: "invalid inference API",
+          env: {
+            NEMOCLAW_MODEL_OVERRIDE: "test",
+            NEMOCLAW_INFERENCE_API_OVERRIDE: "graphql",
+          },
+          expected: "openai-completions",
+        },
+      ];
+
+      for (const c of invalidCases) {
+        expect(runOverrideStderr(dockerLog, image, c.label, c.env)).toContain(c.expected);
+      }

As per coding guidelines, “Keep function complexity low in JavaScript and TypeScript code.”

🤖 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 `@test/e2e-scenario/live/runtime-overrides.test.ts` around lines 347 - 380,
Replace the six repeated expect(...) assertions with a table-driven loop: create
an array of test cases (each containing a label/description, env object like
{NEMOCLAW_MODEL_OVERRIDE: "...", ...}, and expected substring) and iterate
(e.g., forEach or for..of) calling runOverrideStderr(dockerLog, image,
description, env) and asserting the returned stderr contains the expected
substring; update the block around the existing runOverrideStderr, dockerLog,
and image usages so each case uses the same call site and assertion pattern to
reduce duplication.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

.github/workflows/e2e-vitest-scenarios.yaml (3)

393-474: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

gateway-guard-recovery job missing required job filter condition.

All free-standing Vitest jobs in this workflow have a job filter condition (e.g., line 188, 228, 272, 315) to support selective execution via inputs.jobs. The new gateway-guard-recovery job does not have an if condition, so it will always run even when inputs.jobs selects other specific jobs.

🔧 Proposed fix to add job filter

   gateway-guard-recovery:
+    if: ${{ inputs.jobs == '' || contains(format(',{0},', inputs.jobs), ',gateway-guard-recovery,') }}
     runs-on: ubuntu-latest

---

Potential scope creep: gateway-guard-recovery not mentioned in PR objectives.

The PR objectives describe migrating test-runtime-overrides.sh to Vitest and adding the runtime-overrides-vitest job. The gateway-guard-recovery job (lines 393-474) and associated recovery scenario appear unrelated to runtime config overrides. Consider splitting unrelated features into separate PRs for clearer review and history.

🤖 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 @.github/workflows/e2e-vitest-scenarios.yaml around lines 393 - 474, The job
"gateway-guard-recovery" lacks the selective job-run filter used by other Vitest
jobs, so add the same `if` condition that checks github.event.inputs.jobs (e.g.,
using contains(..., 'gateway-guard-recovery') or contains(..., 'all')) to the
gateway-guard-recovery job header to match the pattern used by jobs at lines
near other Vitest jobs; update the job declaration named gateway-guard-recovery
to include that `if` guard so it only runs when selected via inputs.jobs.

---

482-493: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

report-to-pr job missing runtime-overrides-vitest in needs array.

The report-to-pr job aggregates results from scenario jobs but does not list runtime-overrides-vitest in its needs array (lines 484-492), even though that job was added in this PR. The report will not wait for or include the runtime-overrides job results.

📋 Proposed fix to add runtime-overrides-vitest to needs

     needs:
       [
         generate-matrix,
         live-scenarios,
         openshell-version-pin-vitest,
         onboard-negative-paths-vitest,
+        runtime-overrides-vitest,
         openclaw-tui-chat-correlation-vitest,
         gateway-guard-recovery,
       ]

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  report-to-pr:
    runs-on: ubuntu-latest
    needs:
      [
        generate-matrix,
        live-scenarios,
        openshell-version-pin-vitest,
        onboard-negative-paths-vitest,
        runtime-overrides-vitest,
        openclaw-tui-chat-correlation-vitest,
        gateway-guard-recovery,
      ]
    if: ${{ always() && github.event_name == 'workflow_dispatch' }}

🤖 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 @.github/workflows/e2e-vitest-scenarios.yaml around lines 482 - 493, The
report-to-pr job's needs array (job: report-to-pr) is missing the newly added
runtime-overrides-vitest dependency; update the needs list for the report-to-pr
job to include runtime-overrides-vitest so the report waits for and aggregates
that job's results (add runtime-overrides-vitest alongside the existing entries
in the needs array).

---

505-506: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Template injection vulnerability flagged by static analysis.

Lines 505 and 506 directly interpolate inputs.pr_number and inputs.scenarios into JavaScript code via ${{ toJSON(...) }} without sanitization. While toJSON provides some safety, untrusted workflow_dispatch inputs expanded into code contexts can enable code injection attacks.

The current pattern on line 505:

const prNumberInput = ${{ toJSON(inputs.pr_number) }} || '';

Should assign inputs to environment variables first, then read them from process.env in the script:

🛡️ Recommended mitigation

       - name: Post Vitest scenario results to PR
         uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
+        env:
+          PR_NUMBER_INPUT: ${{ inputs.pr_number }}
+          REQUESTED_SCENARIOS: ${{ inputs.scenarios }}
         with:
           script: |
             const needs = ${{ toJSON(needs) }};
             const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
             const workflowBranch = context.ref.replace('refs/heads/', '');
-            const prNumberInput = ${{ toJSON(inputs.pr_number) }} || '';
-            const requestedScenarios = ${{ toJSON(inputs.scenarios) }} || '';
+            const prNumberInput = process.env.PR_NUMBER_INPUT || '';
+            const requestedScenarios = process.env.REQUESTED_SCENARIOS || '';

🧰 Tools

🪛 zizmor (1.25.2)

[error] 505-505: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

---

[error] 506-506: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 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 @.github/workflows/e2e-vitest-scenarios.yaml around lines 505 - 506, Replace
direct template interpolation of inputs into JS by setting them as environment
variables in the workflow and reading them from process.env inside the script:
stop using the expressions that assign to prNumberInput and requestedScenarios
via ${{ toJSON(inputs.pr_number) }} and ${{ toJSON(inputs.scenarios) }}, instead
expose inputs.pr_number and inputs.scenarios into env (e.g., JOB_PR_NUMBER,
JOB_SCENARIOS) and change the script to read process.env.JOB_PR_NUMBER and
process.env.JOB_SCENARIOS into prNumberInput and requestedScenarios (with
defaults and explicit sanitization/parsing as needed) so no untrusted input is
directly interpolated into code execution context.

Source: Linters/SAST tools

.github/workflows/e2e-vitest-scenarios.yaml (1)

66-70: ⚡ Quick win

Skip matrix generation for jobs-only runs.

When inputs.jobs is set, live-scenarios is skipped, but generate-matrix still performs setup and matrix computation. Adding an equivalent gate (or dependency short-circuit via validate-jobs) will cut unnecessary CI time and reduce noisy runs.

Suggested change

   generate-matrix:
+    if: ${{ inputs.jobs == '' }}
     runs-on: ubuntu-latest

Also applies to: 118-121

🤖 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 @.github/workflows/e2e-vitest-scenarios.yaml around lines 66 - 70, The
generate-matrix job currently always runs even when a jobs-only run is
requested; add a gate so matrix computation is skipped when workflow_dispatch
input "jobs" is provided (e.g., add a job-level conditional using the workflow
input like if: ${{ !github.event.inputs.jobs }} or make generate-matrix depend
on validate-jobs and short-circuit via an output/flag from validate-jobs),
update the generate-matrix job declaration (job name generate-matrix) and apply
the same conditional to the other affected job block mentioned (the second
matrix-generation block around lines 118-121) so matrix setup is bypassed for
jobs-only runs.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes