Skip to content

fix(providers/command-code): send required stream payload - #2271

Closed
ddarkr wants to merge 5 commits into
diegosouzapw:release/v3.8.0from
ddarkr:fix/command-code-stream-payload-clean
Closed

ddarkr wants to merge 5 commits into
diegosouzapw:release/v3.8.0from
ddarkr:fix/command-code-stream-payload-clean

Conversation

@ddarkr

@ddarkr ddarkr commented May 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Force Command Code upstream wrapper payloads to include skills: "" and params.stream: true.
  • Align Command Code validation probe payload with the upstream-required shape and default it to deepseek/deepseek-v4-flash.
  • Update focused Command Code tests for runtime payloads, validation payloads, and default validation model selection.

Verification

  • node --import tsx/esm --test tests/unit/command-code-executor.test.ts
  • node --import tsx/esm --test --test-name-pattern="validateCommandCodeProvider" tests/unit/provider-validation-specialty.test.ts
  • node --import tsx/esm --test --test-name-pattern="Command Code" tests/unit/responses-handler.test.ts
  • npm run typecheck:core

@ddarkr
ddarkr requested a review from diegosouzapw as a code owner May 15, 2026 04:06

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a skills field (defaulting to an empty array) and hardcodes the stream parameter to true within the CommandCodeExecutor and its validation logic. Corresponding unit tests have been updated to align with these changes. Feedback suggests making the skills field configurable via the input body to increase flexibility. Additionally, it is recommended to refactor the validation logic to utilize the CommandCodeExecutor directly, which would eliminate code duplication and prevent potential desynchronization as the API evolves.

Comment thread open-sse/executors/commandCode.ts Outdated
},
memory: "",
taste: "",
skills: [],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of hardcoding skills to an empty array, consider allowing it to be passed from the input body if present. This would make the executor more flexible for clients that might want to leverage specific Command Code skills while still defaulting to an empty array as required by the upstream wrapper.

Suggested change
skills: [],
skills: Array.isArray(input.skills) ? input.skills : [],

Comment thread src/lib/providers/validation.ts Outdated
},
memory: "",
taste: "",
skills: [],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Command Code request wrapper structure and headers are now duplicated between this validator and the CommandCodeExecutor. This duplication (including the hardcoded version 0.24.1 and the new skills field) increases the risk of desync as the provider API evolves.

Consider refactoring validateCommandCodeProvider to use the CommandCodeExecutor directly to centralize the payload transformation and header logic. This pattern is already established in this file for other providers (e.g., validateClaudeOAuthInline at line 658).

diegosouzapw pushed a commit that referenced this pull request May 15, 2026
- Force skills: "" and params.stream: true in Command Code wrapper
- Align validation probe payload with upstream-required shape
- Default validation model to deepseek/deepseek-v4-flash

Co-authored-by: ddarkr <ddarkr@users.noreply.github.com>
diegosouzapw added a commit that referenced this pull request May 15, 2026
- #2269: ignore .playwright-mcp/ artifacts (@backryun)
- #2271: Command Code stream payload fix (@ddarkr)
- #2273: Android/Termux headless support (@t-way666)
@diegosouzapw

Copy link
Copy Markdown
Owner

✅ Merged into release/v3.8.0 — thank you @ddarkr!

All 5 commits cherry-picked and integrated. The Command Code executor now properly includes skills: "" and forces params.stream: true, and the validation probe defaults to deepseek/deepseek-v4-flash. Credit added to the CHANGELOG. 🎯

HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…pw#2271)

- Force skills: "" and params.stream: true in Command Code wrapper
- Align validation probe payload with upstream-required shape
- Default validation model to deepseek/deepseek-v4-flash

Co-authored-by: ddarkr <ddarkr@users.noreply.github.com>
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…2271, diegosouzapw#2273

- diegosouzapw#2269: ignore .playwright-mcp/ artifacts (@backryun)
- diegosouzapw#2271: Command Code stream payload fix (@ddarkr)
- diegosouzapw#2273: Android/Termux headless support (@t-way666)
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…pw#2271)

- Force skills: "" and params.stream: true in Command Code wrapper
- Align validation probe payload with upstream-required shape
- Default validation model to deepseek/deepseek-v4-flash

Co-authored-by: ddarkr <ddarkr@users.noreply.github.com>
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…2271, diegosouzapw#2273

- diegosouzapw#2269: ignore .playwright-mcp/ artifacts (@backryun)
- diegosouzapw#2271: Command Code stream payload fix (@ddarkr)
- diegosouzapw#2273: Android/Termux headless support (@t-way666)
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…pw#2271)

- Force skills: "" and params.stream: true in Command Code wrapper
- Align validation probe payload with upstream-required shape
- Default validation model to deepseek/deepseek-v4-flash

Co-authored-by: ddarkr <ddarkr@users.noreply.github.com>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…2271, diegosouzapw#2273

- diegosouzapw#2269: ignore .playwright-mcp/ artifacts (@backryun)
- diegosouzapw#2271: Command Code stream payload fix (@ddarkr)
- diegosouzapw#2273: Android/Termux headless support (@t-way666)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants