feat(sdk): expose sandbox, safe_mode, insecure, worktree CLI flags - #6479
feat(sdk): expose sandbox, safe_mode, insecure, worktree CLI flags#6479juhuan wants to merge 1 commit into
Conversation
Add boolean options to both Python and TS SDKs: - sandbox (--sandbox): run in sandbox mode - safe_mode (--safe-mode): disable all customizations for troubleshooting - insecure (--insecure): skip TLS certificate verification - worktree (--worktree): enable Git worktree mode
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Hi @juhuan — thanks for the PR! Before we dive into the code, we need the PR body to follow the PR template. The current description uses ## Summary / ## Test plan headings, but the template requires:
## What this PR does— describe the change in prose## Why it's needed— motivation and user-facing benefit## Reviewer Test Planwith### How to verify,### Evidence (Before & After), and### Tested onsubsections## Risk & Scope— main risks, out-of-scope items, breaking changes## Linked Issues— related issues- Chinese translation in a
<details>block
These headings aren't bureaucracy — they help reviewers understand why the change exists and how to verify it without guessing. Could you update the PR body to match the template?
中文说明
@juhuan 你好，感谢你的 PR！在审查代码之前，需要 PR 正文遵循 PR 模板。当前描述使用了 ## Summary / ## Test plan 标题，但模板要求：
## What this PR does— 用文字描述变更内容## Why it's needed— 动机和用户价值## Reviewer Test Plan包含### How to verify、### Evidence (Before & After)、### Tested on子章节## Risk & Scope— 主要风险、不在范围内的内容、破坏性变更## Linked Issues— 关联的 issue- 中文翻译 放在
<details>块中
这些标题不是形式主义——它们帮助 reviewer 理解为什么要做这个改动以及如何验证，而不是靠猜。能否按模板更新 PR 正文？
— Qwen Code · qwen3.7-max
| } | ||
|
|
||
| if (this.options.worktree) { | ||
| args.push('--worktree'); |
There was a problem hiding this comment.
[Suggestion] The CLI defines --worktree as type: 'string' in yargs (config.ts:881), but the SDK appends it as a bare --worktree with no value. This works today because --worktree is the last argument — yargs treats the missing value as "". However, if any future change appends another flag after --worktree, yargs will silently consume it as the worktree slug.
Passing an explicit empty string makes this position-independent:
| args.push('--worktree'); | |
| if (this.options.worktree) { | |
| args.push('--worktree', ''); | |
| } |
Additionally, the CLI supports slug (--worktree my-feature) and PR reference (--worktree=#123) modes that the SDK's boolean type cannot express. Consider typing worktree as boolean | string to expose the CLI's full capability, and emitting --worktree <value> when a string is provided.
The same fix should be applied to the Python SDK's transport.py.
— qwen3.7-max via Qwen Code /review
| } | ||
|
|
||
| if (this.options.insecure) { | ||
| args.push('--insecure'); |
There was a problem hiding this comment.
[Suggestion] The CLI logs a prominent warning to stderr when --insecure is passed, but the SDK's default stderr handling ('ignore' unless debug: true or a stderr callback is configured) silently discards it. An SDK consumer enabling insecure: true programmatically gets no visible indication that TLS verification is disabled.
Consider emitting an SDK-side warning:
if (this.options.insecure) {
console.warn('[qwen-code-sdk] TLS certificate verification is disabled (--insecure)');
}The same concern applies to the Python SDK's transport.py, where stderr defaults to subprocess.DEVNULL.
— qwen3.7-max via Qwen Code /review
| * When resume is provided, this should match the resume ID. | ||
| */ | ||
| sessionId?: string; | ||
| sandbox?: boolean; |
There was a problem hiding this comment.
[Suggestion] These four new fields in TransportOptions lack JSDoc, while the same fields in QueryOptions (lines 469-503) have full documentation with descriptions, @default values, and a security warning on insecure. Neighboring fields in this same type (continue, resume, sessionId) all have JSDoc blocks.
Consider adding JSDoc to match QueryOptions, or at minimum a one-line description and @default false for each. The insecure field especially benefits from the MITM warning at the type surface.
— qwen3.7-max via Qwen Code /review
💡 Suggestion: Consolidate SDK PRsHi @juhuan, thanks for the comprehensive SDK work! We noticed you have 15 open PRs that all modify the same core files ( The problem
Suggestion: regroup into 2 PRsWe recommend closing the current 15 PRs and reopening them as 2 consolidated PRs: PR 1 — Covers pure SDK-side option additions (~9 current PRs):
PR 2 — Covers features that also involve CLI-side
This keeps a reasonable separation of concerns while eliminating the merge-conflict chain and making review much more manageable. /cc @juhuan |
|
Closing in favor of consolidated PRs (see suggestion comment above). Please reopen as 2 grouped PRs. |
3 participants
Summary
sandbox(Python) /sandbox(TS) → CLI--sandboxflagsafe_mode(Python) /safeMode(TS) → CLI--safe-modeflaginsecure→ CLI--insecureflag (skip TLS verification)worktree→ CLI--worktreeflag (Git worktree mode)Test plan