Skip to content

feat(sdk): add extra_args option to both SDKs - #6469

Closed
juhuan wants to merge 1 commit into
QwenLM:mainfrom
juhuan:feat/sdk-extra-args
Closed

feat(sdk): add extra_args option to both SDKs#6469
juhuan wants to merge 1 commit into
QwenLM:mainfrom
juhuan:feat/sdk-extra-args

Conversation

@juhuan

@juhuan juhuan commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Add extra_args (Python) / extraArgs (TypeScript) option — an escape hatch for passing arbitrary CLI flags to the qwen process that the SDK doesn't explicitly support.

  • Python SDK: extra_args: list[str] | None in QueryOptions, validated to exclude SDK-managed flags (--input-format, --output-format, --channel)
  • TypeScript SDK: extraArgs?: string[] in TransportOptions and QueryOptions, with Zod schema validation

Test plan

  • Python SDK tests pass (pytest — 58 passed)
  • TypeScript SDK typecheck passes (tsc --noEmit)
  • TypeScript SDK tests pass (vitest run — 1163 passed)

Add `extra_args` (Python) / `extraArgs` (TypeScript) option that allows
passing arbitrary CLI flags to the qwen process. This is an escape hatch
for CLI features not explicitly supported by the SDK.

Python SDK:
- `extra_args: list[str] | None` in QueryOptions
- Appended to CLI arguments in build_cli_arguments
- Validates that SDK-managed flags (--input-format, --output-format,
  --channel) are not included

TypeScript SDK:
- `extraArgs?: string[]` in TransportOptions and QueryOptions
- Appended in ProcessTransport.buildCliArguments

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey @juhuan — thanks for the PR! The feature itself (escape hatch for extra CLI flags in both SDKs) looks like a reasonable idea.

Before diving into the code, though: the PR body doesn't match the project's PR template. The template requires these sections:

  • What this PR does + Why it's needed (currently "Summary" — close, but the template splits motivation from description)
  • Reviewer Test Plan with subsections: How to verify, Evidence (Before & After), Tested on (OS table) — currently "Test plan" is just a checkbox list
  • Risk & Scope — missing entirely
  • Linked Issues — missing entirely

Could you restructure the PR body to follow the template? It helps reviewers (and future-you) understand the motivation, verify the change, and assess risk. Happy to re-run triage once it's updated.

中文说明

感谢 PR!extra_args / extraArgs 作为 SDK 的逃逸口是合理的方向。

不过 PR 正文没有按照项目 PR 模板来写,缺少以下必要章节:

  • What this PR does + Why it's needed(目前是 "Summary",模板要求把动机和描述分开)
  • Reviewer Test Plan,包含 How to verifyEvidence (Before & After)Tested on(OS 表格)子章节
  • Risk & Scope — 完全缺失
  • Linked Issues — 完全缺失

请按模板调整 PR 正文,方便 reviewer 理解和验证改动。更新后可以重新触发 triage。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No tests added for extraArgs/extra_args feature (both SDKs) — The new validation logic (Python conflicting-flag guard), transport argument building (both SDKs), and from_mapping deserialization (Python) all lack test coverage. This is the core safety mechanism and its regressions will go undetected.

— qwen3.7-max via Qwen Code /review

"--channel",
}
for arg in options.extra_args:
if arg in _CONFLICTING_FLAGS:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Critical] The check if arg in _CONFLICTING_FLAGS uses exact string matching, so --input-format=json or --channel=custom (with =) bypasses validation entirely. The SDK itself uses --channel=SDK (equals syntax), proving the CLI accepts this form.

Suggested change
if arg in _CONFLICTING_FLAGS:
for arg in options.extra_args:
flag_name = arg.split("=", 1)[0]
if flag_name in _CONFLICTING_FLAGS:

— qwen3.7-max via Qwen Code /review

args.push('--session-id', this.options.sessionId);
}

if (this.options.extraArgs && this.options.extraArgs.length > 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Critical] The TypeScript SDK has no conflicting-flag validation for extraArgs. The JSDoc on TransportOptions.extraArgs states "Cannot include --input-format, --output-format, or --channel (managed by the SDK)" but this is documentation-only — the Zod schema accepts z.array(z.string()).optional() with no refinement, and buildCliArguments blindly spreads the array. A user passing extraArgs: ['--input-format', 'text'] silently breaks the stream-json protocol.

Add runtime validation matching the Python SDK:

const CONFLICTING_FLAGS = new Set(['--input-format', '--output-format', '--channel']);
for (const arg of this.options.extraArgs ?? []) {
  const name = arg.split('=', 1)[0];
  if (CONFLICTING_FLAGS.has(name)) {
    throw new Error(`extraArgs cannot include '${name}' — it is managed by the SDK`);
  }
}

— qwen3.7-max via Qwen Code /review

raise ValidationError("path_to_qwen_executable cannot be empty")

if options.extra_args:
_CONFLICTING_FLAGS = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] _CONFLICTING_FLAGS is defined inside the if options.extra_args: block, but the file's convention is module-level constants (see _VALID_PERMISSION_MODES, _VALID_AUTH_TYPES at lines 13-14). Move it to module scope for consistency and discoverability:

Suggested change
_CONFLICTING_FLAGS = {
_CONFLICTING_FLAGS = frozenset({"--input-format", "--output-format", "--channel"})

Also, the blocklist only covers 3 of ~15 SDK-managed flags. Since extra_args is appended last, a user can silently override --model, --approval-mode, --resume, --session-id, etc. via last-wins CLI semantics. Consider either expanding the blocklist to cover all SDK-managed flags, or updating the docstring/JSDoc to warn that only the three protocol-breaking flags are hard-blocked.

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

💡 Suggestion: Consolidate SDK PRs

Hi @juhuan, thanks for the comprehensive SDK work! We noticed you have 15 open PRs that all modify the same core files (transport.py, types.py, queryOptionsSchema.ts, types.ts, ProcessTransport.ts, createQuery.ts) and were created on the same day.

The problem

  • Merge conflicts: Since all 15 PRs touch the same files, whichever merges first will cause conflicts in the remaining 14.
  • Review overhead: Reviewing 15 near-identical PRs separately is inefficient and risks fatigue.
  • CI cost: 15 separate CI runs for the same lint/typecheck passes.

Suggestion: regroup into 2 PRs

We recommend closing the current 15 PRs and reopening them as 2 consolidated PRs:

PR 1 — feat(sdk): expose transport and query options in both SDKs

Covers pure SDK-side option additions (~9 current PRs):

PR 2 — feat(sdk): add control request methods to both SDKs

Covers features that also involve CLI-side ControlDispatcher changes (~4 current PRs):

This keeps a reasonable separation of concerns while eliminating the merge-conflict chain and making review much more manageable.

/cc @juhuan

@wenshao

wenshao commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Closing in favor of consolidated PRs (see suggestion comment above). Please reopen as 2 grouped PRs.

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.

3 participants