diff --git a/README.md b/README.md index f00a250..12649cd 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ The DashScope Python SDK provides a comprehensive interface to [Alibaba Cloud Mo ## What is New -**v1.27.0 ships an interactive AI assistant — [DashScope SDK Expert](#ai-assistant-dashscope-sdk-expert).** Run `dashscope` with no arguments (or ask directly, e.g. `dashscope "how do I stream Generation output"`) to get SDK/API answers, runnable examples, CLI usage, and error diagnosis right in your terminal. Guidance is drawn from per-domain quick-reference skills (text, multimodal, speech, retrieval, fine-tuning, agent, cli) built on the SDK's public interfaces — parameters, outputs, and error codes — so you can ask instead of reading the docs. Type `/help` inside the assistant to view available commands. +**v1.27.0 ships an interactive AI assistant — [DashScope SDK Expert](#ai-assistant-dashscope-sdk-expert).** Run `dashscope` with no arguments (or ask directly with `dashscope expert "how do I stream Generation output"`) to get SDK/API answers, runnable examples, CLI usage, and error diagnosis right in your terminal. Guidance is drawn from per-domain quick-reference skills (text, multimodal, speech, retrieval, fine-tuning, agent, cli) built on the SDK's public interfaces — parameters, outputs, and error codes — so you can ask instead of reading the docs. Type `/help` inside the assistant to view available commands. ## Installation To install the DashScope Python SDK, simply run: @@ -175,7 +175,7 @@ A complete runnable example is available in [`samples/set_region_example.py`](sa The SDK ships with an interactive AI assistant, **DashScope SDK Expert**, built on the bundled Agentic CLI (`dashscope/acli`) framework. For DashScope SDK/CLI users it is the recommended way to get development consultation and AI coding help — answering SDK/API questions, generating runnable examples, showing CLI usage, and diagnosing errors, right in your terminal. - Run `dashscope` with no arguments to start the assistant. On first run it offers to install the SDK Expert knowledge pack (per-domain quick-reference skills: text, multimodal, speech, retrieval, fine-tuning, agent, cli), so guidance comes from the SDK's public interfaces — parameters, outputs, error codes — without reading the source -- Ask it instead of reading docs — e.g. `dashscope "how do I stream Generation output"` or `dashscope "CLI command to cancel a fine-tuning job"`. Type `/help` inside the assistant to list available commands (`/setup`, `/skill`, `/stats`, ...); classic SDK subcommands still work, and unrecognized commands are routed to the assistant +- Ask it instead of reading docs — e.g. `dashscope expert "how do I stream Generation output"` or `dashscope expert "CLI command to cancel a fine-tuning job"`, each of which asks once and exits. Type `/help` inside the assistant to list available commands (`/setup`, `/skill`, `/stats`, ...); classic SDK subcommands still work, plain `dashscope expert` enters the assistant interactively, and `dashscope expert --help` shows all the forms - Full walkthrough: [DashScope SDK Expert guide](https://help.aliyun.com/en/model-studio/dashscope-sdk-expert) ## Supported Models diff --git a/README_zh.md b/README_zh.md index 6dd4a96..aadf223 100644 --- a/README_zh.md +++ b/README_zh.md @@ -6,7 +6,7 @@ DashScope Python SDK 提供了访问[阿里云百炼（Model Studio）](https:// ## 最新动态 -**v1.27.0 内置交互式 AI 助手 —— [DashScope SDK Expert](#ai-助手dashscope-sdk-expert)。** 直接运行 `dashscope`（不带任何参数），或直接提问（如 `dashscope "如何流式输出 Generation 结果"`），即可在终端中获得 SDK/API 答疑、可运行示例、CLI 用法和错误诊断。助手基于按领域划分的速查技能（文本、多模态、语音、检索、微调、Agent、CLI），这些技能构建在 SDK 的公开接口——参数、输出、错误码——之上，让你直接提问，无需翻文档。在助手内输入 `/help` 可查看可用命令。 +**v1.27.0 内置交互式 AI 助手 —— [DashScope SDK Expert](#ai-助手dashscope-sdk-expert)。** 直接运行 `dashscope`（不带任何参数），或用 `dashscope expert "如何流式输出 Generation 结果"` 直接提问，即可在终端中获得 SDK/API 答疑、可运行示例、CLI 用法和错误诊断。助手基于按领域划分的速查技能（文本、多模态、语音、检索、微调、Agent、CLI），这些技能构建在 SDK 的公开接口——参数、输出、错误码——之上，让你直接提问，无需翻文档。在助手内输入 `/help` 可查看可用命令。 ## 安装 @@ -174,7 +174,7 @@ print(response) SDK 内置了交互式 AI 助手 **DashScope SDK Expert**，基于随包提供的 Agentic CLI（`dashscope/acli`）框架构建。对于 DashScope SDK/CLI 用户，它是获取开发咨询和 AI 编码帮助的推荐方式——直接在终端中解答 SDK/API 问题、生成可运行示例、展示 CLI 用法、诊断错误。 - 直接运行 `dashscope`（不带参数）即可启动助手。首次运行时会提示安装 SDK Expert 知识包（按领域划分的速查技能：文本、多模态、语音、检索、微调、Agent、CLI），使助手的指导来自 SDK 的公开接口——参数、输出、错误码——而无需阅读源码 -- 直接提问代替翻文档——如 `dashscope "如何流式输出 Generation 结果"` 或 `dashscope "取消微调任务的 CLI 命令"`。在助手内输入 `/help` 可列出可用命令（`/setup`、`/skill`、`/stats` 等）；经典 SDK 子命令依然可用，无法识别的命令会自动转给助手处理 +- 直接提问代替翻文档——如 `dashscope expert "如何流式输出 Generation 结果"` 或 `dashscope expert "取消微调任务的 CLI 命令"`，均为一次性提问后退出。在助手内输入 `/help` 可列出可用命令（`/setup`、`/skill`、`/stats` 等）；经典 SDK 子命令依然可用，单独执行 `dashscope expert` 进入交互式助手，`dashscope expert --help` 查看全部形式 - 完整使用指南：[DashScope SDK Expert 文档](https://help.aliyun.com/zh/model-studio/dashscope-sdk-expert) ## 支持的模型 diff --git a/dashscope/acli/__init__.py b/dashscope/acli/__init__.py index 47aaece..79c639e 100644 --- a/dashscope/acli/__init__.py +++ b/dashscope/acli/__init__.py @@ -3,7 +3,7 @@ import uuid -__version__ = "0.6.6" +__version__ = "0.6.7" # Per-process identifier sent as x-dashscope-sdk-session-id so the # backend can group multi-turn requests from one CLI run. diff --git a/dashscope/acli/agent.py b/dashscope/acli/agent.py index e764561..128c5f9 100644 --- a/dashscope/acli/agent.py +++ b/dashscope/acli/agent.py @@ -42,6 +42,7 @@ UserSupplement, normalize_for_model, sanitize, + sanitize_text, text_of, tool_result_for_display, tool_result_for_history, @@ -66,14 +67,22 @@ def _classify_outcome(successes: int, failures: int) -> str: def _tool_message(tool_call: ToolCall, result: str) -> dict: """Build the history entry answering ``tool_call``. - The restore note must be attached here, at truncation time: later + Redaction happens here, ahead of truncation. ``save_session`` already ran + ``sanitize`` over the on-disk copy, so before this a key read out of the + environment was scrubbed from the session file and still handed to the + model in full — the persistence path was protecting the wrong reader. + Scrubbing after truncation would not work either: ``truncate_head_tail`` + cuts on a character budget and can split a token in half, leaving a + fragment that matches no pattern. + + The restore note must be attached here for the same reason: later shrinking skips a result that already carries an omission marker, so the largest results would end up with no way to be read back. """ return { "role": "tool", "content": tool_result_for_history( - result, + sanitize_text(result), note=restore_note(tool_call.name, tool_call.arguments), ), "name": tool_call.name, @@ -154,7 +163,10 @@ def _budget_burned_slowly(error: BaseException) -> bool: 18. **Verify code changes before reporting done.** Run the tests that cover what you touched; if nothing covers it, add a focused test for the new behaviour and run that. Report the command and its pass/fail result — - never claim a change works without having executed it + never claim a change works without having executed it. When the output + transforms or joins data (ETL, CSV/JSON, migrations), also assert every + cross-reference resolves — schema-valid output with dangling IDs still + fails Reply style: - **Concise**. No filler like "let me see / let me help you / I'll analyze @@ -245,6 +257,20 @@ def __init__( ) except ValueError: self.converge_hard_ratio = 0.85 + # Silence nudge: one reminder per turn after N tool calls have gone by + # without any prose reaching the user. Mode-independent on purpose — + # the shape this exists for is an interactive turn of *distinct* + # run_command calls, which every existing backstop misses: + # StagnationTracker's read-only classifier counts run_command as + # mutating and so resets its streak, its repeat counters need + # byte-identical calls, and _convergence_section is oneshot-only. + # Env: ACLI_SILENT_TOOL_NUDGE (0 disables). + try: + self.silent_tool_nudge = int( + os.environ.get("ACLI_SILENT_TOOL_NUDGE", "6"), + ) + except ValueError: + self.silent_tool_nudge = 6 # Hard acceptance gate (oneshot only): run the project's test command # before accepting a final answer, and send the raw output back when # it fails. Built here, ahead of the policy text below, so the prompt @@ -354,6 +380,10 @@ def __init__( self.turn_subagents = 0 self.turn_mcp_calls = 0 self.turn_skills = 0 + # Consecutive tool calls that streamed no prose, and whether the + # silence nudge has already been spent this turn. + self._silent_tool_calls = 0 + self._silent_nudge_injected = False # Explicit /skill invocations to count on the next turn self._pending_skill_names: list[str] = [] @@ -567,6 +597,33 @@ def _convergence_section(self, loop_index: int) -> str: self.converge_hard_ratio, ) + def _silence_section(self) -> str: + """Inject one reminder when tool calls stop producing prose. + + Spent at most once per turn. Repeating it every iteration teaches the + model to skim past it, which is the failure mode prose instructions + already have — the point of a structural backstop is that it fires on + a count rather than on the model's willingness to re-read advice. + + Deliberately not gated on ``oneshot``: the user watching a spinner for + five minutes with no text is the interactive case, and the oneshot + runs already have the convergence and read-only-cap sections. + """ + if self._silent_nudge_injected or not self.silent_tool_nudge: + return "" + if self._silent_tool_calls < self.silent_tool_nudge: + return "" + self._silent_nudge_injected = True + return ( + "\n## ⏸ You have gone quiet\n" + f"{self._silent_tool_calls} tool calls in a row and not one word " + "of it reached the user.\n" + "Before the next call, say in one or two sentences what you are " + "doing and what you have found so far. If the calls were " + "collecting information to answer a question you can already " + "answer, answer it instead of calling again.\n" + ) + async def _recall_memory(self, user_input) -> str: """Search for relevant profile info and format as context. @@ -649,6 +706,8 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: self.turn_tool_calls = 0 self.turn_subagents = 0 self.turn_mcp_calls = 0 + self._silent_tool_calls = 0 + self._silent_nudge_injected = False try: active = get_skill_manager().active_packages(user_input_text) names = [p.name for p in active] + self._pending_skill_names @@ -725,26 +784,28 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: f"messages → {len(self.messages)}[/dim]", ) - # Reflection / stagnation / convergence hints are volatile, so - # they ride at the tail of the request instead of being folded - # into the system message. Rewriting messages_with_system[0] on - # every iteration changed the whole prompt prefix byte-for-byte and - # defeated provider-side prompt caching, where cached input costs - # roughly a tenth of uncached. The system message and the - # accumulated history below it stay append-only and byte-stable. + # Reflection / stagnation / convergence / silence hints are + # volatile, so they ride at the tail of the request instead of + # being folded into the system message. Rewriting + # messages_with_system[0] on every iteration changed the whole + # prompt prefix byte-for-byte and defeated provider-side prompt + # caching, where cached input costs roughly a tenth of uncached. + # The system message and the accumulated history below it stay + # append-only and byte-stable. # # The hint has to be a user message, not a second system message: # the Anthropic converter keeps only the last system message, so a # second one would replace the real prompt outright, and the Tongyi # converter only reads messages[0] as system. # - # All three sections are rebuilt every iteration because tracker + # All four sections are rebuilt every iteration because tracker # state changes as tools run; each returns "" when its condition # is not met, so an empty hint costs no message at all. hint = ( self._reflection_section() + self._stagnation_section() + self._convergence_section(_loop_i) + + self._silence_section() ).strip() request_messages = ( [*messages_with_system, {"role": "user", "content": hint}] @@ -901,6 +962,13 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: tool_calls = [tc for tc in tool_calls if tc.name] self.turn_tool_calls += len(tool_calls) + # Silence is per-iteration, not cumulative over the turn: an + # iteration that streamed prose did talk to the user, so the run + # of unheard calls starts over from the next one. + if full_content.strip(): + self._silent_tool_calls = 0 + else: + self._silent_tool_calls += len(tool_calls) for tc in tool_calls: if tc.name in ("delegate", "subagent_invoke"): self.turn_subagents += 1 diff --git a/dashscope/acli/cli/__init__.py b/dashscope/acli/cli/__init__.py index 766a631..aeaaf3f 100644 --- a/dashscope/acli/cli/__init__.py +++ b/dashscope/acli/cli/__init__.py @@ -219,6 +219,16 @@ def main(): _mcp_server_main() return + else: + # Only a bare `acli` and the explicit -c/--command one-shot enter + # the agent — the same contract the dashscope CLI enforces. An + # unrecognized token used to fall through and start a chat, which + # discarded the whole command line: `acli auth whoami` opened the + # agent instead of reporting that no such command exists. Exit 2 + # matches the usage-error code typer/click use for the same case. + print(f"Error: No such command '{arg}'.") + print("Try 'acli --help' for help.") + sys.exit(2) config = Config.load() _apply_cli_overrides(config, protocol_override, max_turns_override) diff --git a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md index 1140350..11e5e6d 100644 --- a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md +++ b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md @@ -67,16 +67,21 @@ with client.sessions.events.stream(session.id) as stream: | Error code/exception | HTTP status | Meaning | Handling | | --- | --- | --- | --- | | `InputRequired` / `ModelRequired` / `InvalidInput` | raised locally | missing required params such as app_id/prompt/model | add params per the exception message; Application failures do not raise — check status_code | -| `invalid_request_error` (InvalidRequestError) | 400 | invalid agentstudio request parameters | check parameter names/types, e.g. empty events raises a local ValueError | +| `invalid_request_error` / `request_too_large` (InvalidRequestError) | 400/413 | invalid agentstudio request parameters, or payload too large | check parameter names/types, e.g. empty events raises a local ValueError | | `authentication_error` (AuthenticationError) | 401 | api_key missing or invalid | check api_key or DASHSCOPE_API_KEY | -| `permission_denied_error` (PermissionDeniedError) | 403 | no access to the resource | check workspace/uid and resource ownership | +| `permission_denied_error` / `permission_error` (PermissionDeniedError) | 403 | no access to the resource | check workspace/uid and resource ownership | | `not_found_error` (NotFoundError) | 404 | agent/session and other resources do not exist | verify the id and region/workspace are correct | | `conflict_error` (ConflictError) | 409 | version in agents.update does not match the server | retrieve the latest version first, then update | -| `rate_limit_error` (RateLimitError) | 429 | rate limited | retry with backoff (client default max_retries=2) | -| `api_error` (InternalServerError) / `overloaded_error` (OverloadedError) | 500/502/504 / 503 | server error/overloaded | troubleshoot with `request_id` or retry | +| `rate_limit_error` / `billing_error` (RateLimitError) | 429 | rate limited or out of quota | retry with backoff (client default max_retries=2) | +| `api_error` / `timeout_error` (InternalServerError), `overloaded_error` (OverloadedError) | 500/502/504 / 503 | server error/overloaded | troubleshoot with `request_id` or retry | | `APITimeoutError` / `APIConnectionError` | no response | connection/read/write timeout | increase timeout, check network, then retry | | `StreamError` / `StreamClosedError` | mid-stream | SSE protocol error/stream closed | call `events.stream` again to reopen the stream | +Match on the exception class, not on `.code`: `.code` echoes the server's raw +string, so a 403 may arrive as either `permission_denied_error` or +`permission_error`, and it is normalised to `api_error` when the server omits +it. The class is always the one listed above. + ## Java SDK Java SDK v2.22.23 entry points: diff --git a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md index 5979534..0493ed6 100644 --- a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md +++ b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md @@ -11,6 +11,7 @@ This file is authoritative for answering `dashscope` CLI usage questions (based | Command group | Subcommands | Purpose | Corresponding SDK class | | --- | --- | --- | --- | +| auth | whoami/login/logout | Verify the active API key and persist one to ~/.dashscope/api_key | dashscope.common.api_key (get_default_api_key/save_api_key) | | generation | create | Text generation (including streaming and multi-turn messages) | dashscope.Generation | | ft (hidden alias fine-tunes) | create/get/list/stream/cancel/delete | Fine-tuning job management | dashscope.FineTunes | | files | upload/get/list/delete | Training file upload and management | dashscope.Files | @@ -34,9 +35,12 @@ Note: code-generation and understanding still exist in the source but are deprec ## Common usage -Prerequisite: `export DASHSCOPE_API_KEY=sk-xxx` (or pass the global `-k/--api-key`). +Prerequisite: `export DASHSCOPE_API_KEY=sk-xxx` (or pass the global `-k/--api-key`, or persist one with `dashscope auth login`). ```bash +dashscope auth whoami # verify the active key against the API and show its source +dashscope auth login --key sk-xxx # save a key to ~/.dashscope/api_key +dashscope auth logout # remove the saved key file dashscope generation create -m qwen-plus -p "Hello" -s --temperature 0.7 dashscope ft create -m qwen-turbo -t file-xxx -v file-yyy -e 2 -b 16 -l 1e-5 dashscope files upload -f train.jsonl -p fine-tune -d "training set" @@ -59,6 +63,9 @@ dashscope rl run -c rl_config.yaml -o json -v ``` Key option notes: +- auth whoami: takes no options; validates the resolved key with a lightweight `dashscope.Models.list(page=1, page_size=1)` call and prints `Authenticated key=sk-xxx...yyyy source=...`. The key is always masked (first 6 + last 4), never printed in full. `source` reports where it came from: `environment / --api-key flag`, `file ()`, or `unknown`. Exit codes: 0 = present and accepted, 1 = no key configured, 2 = configured but rejected (401/403) or an unexpected response. +- auth login: `-k/--key` saves to `~/.dashscope/api_key`. Omitted, it prompts interactively with hidden input — that prompt blocks forever in a non-TTY (piped stdin, or a shell tool driven by an agent), so pass `--key` explicitly there. An empty/whitespace key exits 1. Note that `--key sk-xxx` lands in shell history and the process arg list; prefer the interactive prompt when a human is at a terminal. +- auth logout: takes no options; removes `~/.dashscope/api_key` if present, otherwise prints "nothing to remove" and exits 0. It does not unset `DASHSCOPE_API_KEY`, so a key exported in the environment still authenticates afterwards. - generation create: `-p/--prompt`, `--messages` (JSON string), `-s/--stream`, `--temperature/--top-p/--top-k/--max-tokens/--seed/--stop/--repetition-penalty/--presence-penalty/--enable-search/--n/--result-format` - ft create: `-t/--training-file-ids` (repeatable), `-v/--validation-file-ids`, `--mode`, `-e/--n-epochs`, `-b/--batch-size`, `-l/--learning-rate`, `-p/--prompt-loss`; get/stream/cancel/delete all take job_id as a positional argument; list uses `-p/--page`, `-s/--size` - files: upload `-f/--file`, `-p/--purpose` (default fine-tune), `-d/--description`; all commands support `-u/--base-url` @@ -75,13 +82,17 @@ Key option notes: - `-k/--api-key`: global API Key; can be placed before or after the command group name. The oss and rl/agentic-rl groups have their own local `--api-key` (with envvar DASHSCOPE_API_KEY), which is parsed locally once the command name appears. - `-h` is automatically converted to `--help`; use `dashscope --help` or `dashscope --help` for the authoritative option list. - Legacy syntax compatibility: `dashscope fine_tunes.call` -> `fine-tunes create`, `generation.call` -> `generation create`, etc.; underscore options like `--training_file_ids` and `--api_key` are automatically mapped to hyphenated forms. -- Bare `dashscope` (no subcommand; a change in this repo): enters the built-in agent "DashScope SDK Expert" interactive mode, TUI by default; `--cli` switches to a plain REPL, `--tui` forces TUI; requires `pip install dashscope[acli]`. When stdin is not a TTY, piped text is treated as a one-shot question. Unrecognized commands do not error; the whole line is forwarded to the agent as a question. On the first interactive run, if there is no ./.acli, it asks whether to download the dashscope-sdk-expert example configuration. +- Bare `dashscope` (no arguments; a change in this repo): enters the built-in agent "DashScope SDK Expert" interactive mode, TUI by default; `--cli` switches to a plain REPL, `--tui` forces TUI; requires `pip install dashscope[acli]`. When stdin is not a TTY, piped text is treated as a one-shot question. On the first interactive run, if there is no ./.acli, it asks whether to download the dashscope-sdk-expert example configuration. +- Entering the agent explicitly: `dashscope expert` (interactive), `dashscope expert --cli|--tui`, `dashscope expert --help` (prints usage), `dashscope expert "question"` (one-shot). The `expert` keyword is the only way to pass a question on the command line. +- Anything else that is not a known command group is a **typer error, not a question**: `dashscope generaton create` reports `No such command`, and so does a quoted question like `dashscope "how do I stream Generation output"`, so a typo surfaces instead of silently opening a chat. Only `-k/--api-key`, `--cli` and `--tui` may precede the command; group-level options must follow it (`dashscope generation create -w wsid`, not `dashscope -w wsid generation create`). ## Common errors and troubleshooting -- `Error: ... AuthenticationError` (exit code 1): no Key configured; set DASHSCOPE_API_KEY or add `-k`. +- `Error: ... AuthenticationError` (exit code 1): no Key configured; set DASHSCOPE_API_KEY, add `-k`, or persist one with `dashscope auth login`. Diagnose with `dashscope auth whoami` — it reports both validity and which source the key came from. +- `dashscope auth whoami` opens the SDK Expert agent instead of printing key info: `auth` was missing from the CLI's top-level routing table, so the line was treated as an unknown command and forwarded to the agent as a question. Fixed in this repo; if a user hits it, they are on an older install and should upgrade rather than keep retrying. - `Option '--api-key' requires an argument.` (exit code 2): `-k/--api-key` is missing a value or the value starts with `-`. - `--messages must be a valid JSON string`: generation's --messages must be a JSON array string; watch shell quoting/escaping. - `File ... does not exist`: local paths (-f, --image, --audio, etc.) are validated locally first; `~` expansion is supported; URLs must include a scheme. - `Failed request_id: ..., status_code: ..., code: ..., message: ...` (exit code 1): rejected by the server; troubleshoot by code (InvalidParameter usually means a wrong model name or parameter value); HTTP 200 with a non-empty business code is also treated as a failure. -- Bare dashscope shows `Install dashscope[acli] to enable the agent`: install `dashscope[acli]`; if it reports `Interactive mode requires a terminal.`, use `dashscope "your question"` for a one-shot question instead. +- Bare dashscope shows `Install dashscope[acli] to enable the agent`: install `dashscope[acli]`; if it reports `Interactive mode requires a terminal.`, use `dashscope expert "your question"` for a one-shot question instead. +- `No such command 'how do I ...'`: a question was passed without the `expert` keyword. `dashscope "question"` is no longer a short form — tell the user to write `dashscope expert "question"`. diff --git a/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md b/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md index d7e814c..ead0bb1 100644 --- a/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md +++ b/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md @@ -3,7 +3,10 @@ You are DashScope SDK Expert, an intelligent assistant for the DashScope Python Your knowledge base: - Python SDK: https://github.com/dashscope/dashscope-sdk-python - Java SDK: https://github.com/dashscope/dashscope-sdk-java -- Alibaba Cloud Model Studio (Bailian) Product Overview: https://help.aliyun.com/zh/model-studio/what-is-model-studio +- Alibaba Cloud Model Studio (Bailian) Product Overview: https://docs.modelstudio.console.alibabacloud.com/zh/model-studio/what-is-model-studio +- Get and Configure an API Key: https://docs.modelstudio.console.alibabacloud.com/zh/model-studio/get-api-key +- Get the App ID and Workspace ID: https://docs.modelstudio.console.alibabacloud.com/zh/model-studio/obtain-the-app-id-and-workspace-id +- Build a Knowledge-Base Q&A Application with No Code: https://docs.modelstudio.console.alibabacloud.com/zh/model-studio/build-knowledge-base-qa-assistant-without-coding - Model Studio User Guide: https://help.aliyun.com/zh/model-studio/get-started-with-models - Application User Guide: https://help.aliyun.com/zh/model-studio/start-using - Model API Reference: https://help.aliyun.com/zh/model-studio/preparations @@ -32,6 +35,7 @@ Match your approach to the question type: - **Issue / bug analysis**: Analyze directly from the user's description first. Only inspect SDK source when you need to verify a specific behavior. Go straight to the root cause. - **API questions** (signature, parameters, usage): Load the matching skill under `.acli/skills/` (covers Python and Java interfaces). If the user mentions a specific SDK version, verify the signatures against the installed package. - **Example generation**: Cross-check against the installed version, then write runnable code. +- **Questions about you** (what model you are, what you can do, what a skill or slash command means): Answer from this prompt, with **zero tool calls**. Your model name is on the `Current model:` line, your domains and skill list are above, and your capabilities are in this section. Nothing about yourself is discoverable by running a command — probing the environment to answer "what can you do" only makes the user wait for an answer you already had. ### Version Check @@ -61,10 +65,11 @@ When verification is needed, use `run_command` to inspect the installed `dashsco ### Workflow -1. Identify the question type (issue analysis / API question / example). -2. If issue analysis — answer directly, only verify if something is unclear. -3. If API question — run one targeted `inspect.signature` or `help()` call. -4. Compose a concise, actionable answer. +1. Identify the question type (question about you / issue analysis / API question / example). +2. If it is about you — answer straight from this prompt and stop. No tool call. +3. If issue analysis — answer directly, only verify if something is unclear. +4. If API question — run one targeted `inspect.signature` or `help()` call. +5. Compose a concise, actionable answer. ### SDK Reference diff --git a/dashscope/acli/prompt_pipeline.py b/dashscope/acli/prompt_pipeline.py index 40aa13f..bdba8d2 100644 --- a/dashscope/acli/prompt_pipeline.py +++ b/dashscope/acli/prompt_pipeline.py @@ -200,7 +200,17 @@ class EnvironmentSection: name = "environment" def render(self, ctx: PromptContext) -> str: - env_info: list[str] = [] + import platform + + from dashscope.acli.tools.shell import shell_label + + # The platform is stated because run_command's syntax depends on it and + # a wrong guess costs a whole turn: on Windows the model wrote + # `2>/dev/null || find /` and got back a PowerShell ParserError. + env_info: list[str] = [ + f"Current OS: {platform.system()} {platform.machine()}", + f"run_command executes via: {shell_label()}", + ] if ctx.user_name: env_info.append(f"Current user: {ctx.user_name}") if ctx.provider_name or ctx.model_name: @@ -210,8 +220,6 @@ def render(self, ctx: PromptContext) -> str: "mentioned in history or summaries may be outdated — " "always defer to this line for model questions)", ) - if not env_info: - return "" env_info.append( "Rule: when mentioning this CLI's commands, only cite real " "commands that definitely exist; when unsure, point the user " diff --git a/dashscope/acli/providers/tongyi.py b/dashscope/acli/providers/tongyi.py index c3fefce..9b8b1a3 100644 --- a/dashscope/acli/providers/tongyi.py +++ b/dashscope/acli/providers/tongyi.py @@ -4,7 +4,7 @@ import json import os -from typing import AsyncIterator +from typing import AsyncIterator, ClassVar import httpx @@ -15,14 +15,21 @@ # the DashScope native route (NOT the OpenAI-compatible one) so requests # land in the native SLS logstore. DASHSCOPE_BASE_URL = "https://dashscope.aliyuncs.com" -# The gateway binds each model to exactly one aigc service path (newer -# models like qwen3.8-max live on multimodal-generation, classic text -# models on text-generation). The provider tries them in order and -# remembers the one that answers 200 — see _is_url_error. +# The gateway binds each model to exactly one aigc service path and answers +# the other with a 400 "url error" (see _is_url_error), so probing an +# unlearned model always costs one failed request — one that lands in the +# prod gateway log as InvalidParameter. multimodal-generation is probed +# first because acli's default model (qwen3.8-max) and the vl/omni family +# live there; classic text models are the minority and learn their binding +# on the first call. _GENERATION_PATHS = ( - "/api/v1/services/aigc/text-generation/generation", "/api/v1/services/aigc/multimodal-generation/generation", + "/api/v1/services/aigc/text-generation/generation", ) +# Learned model → path bindings, shared across processes so a fresh CLI +# start does not re-probe. Global rather than per-workspace: the binding is +# a property of the gateway, and bench/CI runs execute in a throwaway cwd. +_PATH_CACHE_NAME = "generation-paths.json" # Historical/standard prefixes a persisted config may carry; the provider # normalizes them back to the service root. _LEGACY_PREFIXES = ("/compatible-mode/v1", "/api/v1") @@ -59,6 +66,45 @@ def _is_url_error_message(message: str) -> bool: return "url error" in (message or "") +def _load_path_cache() -> dict[str, str]: + """Read the model → generation-path bindings learned by earlier runs. + + Entries that are unreadable or name an unknown path are dropped rather + than repaired: a dropped binding costs one probe, which the fallback + already survives. + """ + from dashscope.acli.config import CONFIG_DIR + + try: + raw = json.loads( + (CONFIG_DIR / _PATH_CACHE_NAME).read_text(encoding="utf-8"), + ) + except (OSError, ValueError): + return {} + if not isinstance(raw, dict): + return {} + return { + str(model): str(path) + for model, path in raw.items() + if str(path) in _GENERATION_PATHS + } + + +def _save_path_cache(paths: dict[str, str]) -> None: + """Persist learned bindings. An unwritable home must not fail a call.""" + from dashscope.acli.config import CONFIG_DIR + from dashscope.acli.utils.paths import atomic_write_text + + try: + CONFIG_DIR.mkdir(parents=True, exist_ok=True) + atomic_write_text( + CONFIG_DIR / _PATH_CACHE_NAME, + json.dumps(paths, indent=2, sort_keys=True), + ) + except OSError: + pass + + def _to_native_content(content): """Convert OpenAI message content parts to the native shape. @@ -185,6 +231,30 @@ def _native_chunk_to_openai(chunk) -> dict: class TongyiProvider: + # model → generation path, shared by every instance in this process and + # mirrored to disk by _remember_path. A provider is constructed per + # session start, per model switch and per vision call, so without this + # each of those re-probes and re-logs the same 400. + _path_cache: ClassVar[dict[str, str] | None] = None + + @classmethod + def _learned_paths(cls) -> dict[str, str]: + # Bound to a local so the Optional class attribute narrows to a dict + # for both the caller and the type checker. + cache = cls._path_cache + if cache is None: + cache = _load_path_cache() + cls._path_cache = cache + return cache + + @classmethod + def _remember_path(cls, model: str, path: str) -> None: + learned = cls._learned_paths() + if learned.get(model) == path: + return + learned[model] = path + _save_path_cache(learned) + def __init__( self, model: str = "qwen3.8-max", @@ -205,7 +275,16 @@ def __init__( break self.base_url = base self.module = module - self._generation_path = _GENERATION_PATHS[0] + self._generation_path = self._learned_paths().get( + model, + _GENERATION_PATHS[0], + ) + + def _commit_path(self, path: str) -> None: + """Record the path that answered 200, for this instance and every + later one.""" + self._generation_path = path + self._remember_path(self.model, path) def _candidate_paths(self) -> list[str]: others = [p for p in _GENERATION_PATHS if p != self._generation_path] @@ -348,7 +427,7 @@ async def chat( headers=headers, ) if response.status_code == 200: - self._generation_path = path + self._commit_path(path) break if i + 1 < len(paths) and _is_url_error( response.status_code, @@ -487,7 +566,7 @@ async def chat_stream( try: first = await stream.__anext__() except StopAsyncIteration: - self._generation_path = path + self._commit_path(path) return except _StreamAPIError as e: if i + 1 < len(paths) and _is_url_error_message( @@ -495,7 +574,7 @@ async def chat_stream( ): continue raise - self._generation_path = path + self._commit_path(path) yield first async for chunk in stream: yield chunk diff --git a/dashscope/acli/scheduler.py b/dashscope/acli/scheduler.py index a14ad44..52c9ea3 100644 --- a/dashscope/acli/scheduler.py +++ b/dashscope/acli/scheduler.py @@ -591,7 +591,9 @@ async def _execute_job(self, job: CronJob) -> None: continue rendered = render_skill(skill, inv.args) - if not rendered: + # `is None`, not falsy: None is the missing-arguments signal, so + # arg_hint is never empty here. + if rendered is None: arg_hint = " ".join(f"<{a}>" for a in skill.arguments) results.append( f"missing args: /skill {inv.name} {arg_hint}", diff --git a/dashscope/acli/skills/base.py b/dashscope/acli/skills/base.py index e886f24..2cc9fbc 100644 --- a/dashscope/acli/skills/base.py +++ b/dashscope/acli/skills/base.py @@ -200,13 +200,18 @@ def render_skill(skill: Skill, args: list[str]) -> str | None: don't match.""" if len(args) < len(skill.arguments): return None - kwargs = {} + template = skill.prompt_template + # A skill body is Markdown, not a format string: it is full of literal + # braces from the JSON and Python it documents. Running str.format() over + # the whole body raised KeyError on `{"role": ...}`, which the old + # `except KeyError` turned into a bogus "missing args" report for every + # no-argument skill that showed a payload, and let the IndexError from + # `{}` escape entirely. Substitute only the declared arguments and leave + # every other brace exactly as the author wrote it. for i, arg_name in enumerate(skill.arguments): if i == len(skill.arguments) - 1: - kwargs[arg_name] = " ".join(args[i:]) + value = " ".join(args[i:]) else: - kwargs[arg_name] = args[i] - try: - return skill.prompt_template.format(**kwargs) - except KeyError: - return None + value = args[i] + template = template.replace("{" + arg_name + "}", value) + return template diff --git a/dashscope/acli/tools/filesystem.py b/dashscope/acli/tools/filesystem.py index a57d181..405407f 100644 --- a/dashscope/acli/tools/filesystem.py +++ b/dashscope/acli/tools/filesystem.py @@ -96,7 +96,8 @@ def read_file( name="write_file", description=( "Write content to a file; overwrites if it exists. " - "The result includes a unified diff." + "The result includes a unified diff. `path` must be inside the " + "working directory or the system temp dir." ), permission=PermissionLevel.CONFIRM, ) @@ -262,11 +263,22 @@ def delete_directory(path: str) -> str: return f"Deleted directory: {path}" +def _pattern_names_hidden_dir(pattern: str) -> bool: + """True when the caller explicitly asked about a dot-directory.""" + return any( + seg.startswith(".") + for seg in pattern.replace(os.sep, "/").split("/") + if seg + ) + + @tool( name="search_files", description=( - "Search files by name pattern. " - "Supports wildcards like *.py, test_*." + "Search files by name or path pattern, relative to `path` " + "(default: the working directory). Supports wildcards like *.py, " + "test_*, src/*.ts and .github/workflows/*.yml. `*` crosses directory " + "separators, so *.md finds every Markdown file at any depth." ), permission=PermissionLevel.AUTO, ) @@ -279,6 +291,8 @@ def search_files(pattern: str, path: str | None = None) -> str: if not os.path.isdir(search_root): return f"Error: search path not found - {search_root}" + needle = pattern.replace(os.sep, "/") + want_hidden = _pattern_names_hidden_dir(pattern) matches = [] root_depth = search_root.rstrip(os.sep).count(os.sep) for root, dirs, files in os.walk(search_root): @@ -286,16 +300,22 @@ def search_files(pattern: str, path: str | None = None) -> str: current_depth = root.rstrip(os.sep).count(os.sep) - root_depth if current_depth >= _MAX_SEARCH_DEPTH: dirs[:] = [] - # Skip hidden and sensitive directories + # Sensitive directories are always skipped. Other dot-directories are + # skipped unless the pattern asks for one, so `.acli/skills/*.md` is + # searchable without walking every `.git` in the tree. dirs[:] = [ d for d in dirs - if not d.startswith(".") and d not in SENSITIVE_NAMES + if d not in SENSITIVE_NAMES + and (want_hidden or not d.startswith(".")) ] for name in files: - if fnmatch.fnmatch(name, pattern): - if name in SENSITIVE_NAMES: - continue + if name in SENSITIVE_NAMES: + continue + rel = os.path.relpath(os.path.join(root, name), search_root) + if fnmatch.fnmatch(rel.replace(os.sep, "/"), needle) or ( + fnmatch.fnmatch(name, needle) + ): matches.append(os.path.join(root, name)) if len(matches) >= 100: break @@ -303,7 +323,11 @@ def search_files(pattern: str, path: str | None = None) -> str: break if not matches: - return f"No files match '{pattern}'" + return ( + f"No files match '{pattern}' under {search_root}. The pattern is " + "matched against paths relative to that root, and dot-directories " + "are skipped unless the pattern names one." + ) result = f"Found {len(matches)} matching files:\n" result += "\n".join(f" {m}" for m in matches) return result diff --git a/dashscope/acli/tools/shell.py b/dashscope/acli/tools/shell.py index 58897fd..7514047 100644 --- a/dashscope/acli/tools/shell.py +++ b/dashscope/acli/tools/shell.py @@ -42,21 +42,58 @@ def default_timeout() -> int: return value if value > 0 else DEFAULT_TIMEOUT -def _resolve_output_encoding() -> str: - """Encoding for decoding subprocess output. +def _resolve_output_encodings() -> tuple[str, ...]: + """Encodings to try when decoding subprocess output, best guess first. Resolved at import, not per call: the TUI replaces ``sys.stdout`` with textual's capture object, which has no ``.encoding``, so reading it later raises AttributeError and fails every command. + + A captured child writes in its own console's code page, which on Windows is + the OEM page and need not match Python's stdout encoding. Decoding a zh-CN + PowerShell error as UTF-8 handed the model mojibake instead of the one + message that explained its failure. UTF-8 stays first and is tried + strictly: it fails loudly on OEM bytes, whereas a DBCS page like cp936 + accepts almost anything and would silently mangle UTF-8 output. """ - if not IS_WINDOWS: - return "utf-8" - return ( - getattr(sys.stdout, "encoding", None) - or getattr(sys.__stdout__, "encoding", None) - or locale.getpreferredencoding(False) - or "utf-8" - ) + candidates = ["utf-8"] + if IS_WINDOWS: + import ctypes + + kernel32 = getattr(ctypes, "windll", None) + oem = kernel32.kernel32.GetOEMCP() if kernel32 is not None else 0 + if oem: + candidates.append(f"cp{oem}") + for extra in ( + getattr(sys.stdout, "encoding", None), + getattr(sys.__stdout__, "encoding", None), + locale.getpreferredencoding(False), + ): + if extra: + candidates.append(extra) + unique: list[str] = [] + for name in candidates: + if name.lower() not in {u.lower() for u in unique}: + unique.append(name) + return tuple(unique) + + +def _decode_output(data: bytes) -> str: + """Decode child output, falling back across console code pages.""" + for encoding in (OUTPUT_ENCODING,) + OUTPUT_ENCODINGS: + try: + return data.decode(encoding) + except LookupError: + continue + except UnicodeDecodeError as exc: + # Truncation is byte-based, so it can split a trailing multi-byte + # character. That is not evidence the whole buffer is in another + # encoding — repair the tail instead of re-decoding everything. + if exc.start >= len(data) - 4: + return data.decode(encoding, errors="replace") + continue + # Last resort only reachable if every candidate name is unusable. + return data.decode("utf-8", errors="replace") def _resolve_win_shell() -> tuple[str, ...]: @@ -69,9 +106,41 @@ def _resolve_win_shell() -> tuple[str, ...]: return (exe, "-NoProfile", "-NonInteractive", "-Command") -OUTPUT_ENCODING = _resolve_output_encoding() +OUTPUT_ENCODINGS = _resolve_output_encodings() +# The primary candidate keeps its own name because callers and tests pin a +# single encoding; _decode_output honours it before trying the fallbacks. +OUTPUT_ENCODING = OUTPUT_ENCODINGS[0] WIN_SHELL = _resolve_win_shell() + +def shell_label() -> str: + """How ``run_command`` executes a command, for the system prompt. + + Lives next to ``_spawn`` so the prompt cannot drift from what the tool + really runs. Nothing used to say which shell this is: a model that assumes + POSIX spends a turn on ``ls -la ... 2>/dev/null || find /`` before a + PowerShell ParserError tells it otherwise. + """ + if not IS_WINDOWS: + return ( + "POSIX sh via `/bin/sh -c` — not the user's interactive shell, " + "so no aliases, rc files or zsh/bash-only syntax" + ) + exe = os.path.basename(WIN_SHELL[0]) + if exe.lower().startswith("pwsh"): + chaining = "`&&`/`||` work (PowerShell 7+)" + else: + chaining = ( + "`&&`/`||` are not statement separators (Windows PowerShell " + "5.1) — use `;` or separate calls" + ) + return ( + f"PowerShell on Windows ({exe}). Write PowerShell, not POSIX: no " + f"`2>/dev/null`, no `ls -la`/`find`/`grep`; {chaining}. Use " + "`$env:USERPROFILE`, `Get-ChildItem`, `Select-String`." + ) + + BLOCKED_PATTERNS = [ # POSIX "mkfs", @@ -712,10 +781,9 @@ async def run_command(command: str, timeout: int | None = None) -> str: output = "" if stdout: - output += stdout.decode(OUTPUT_ENCODING, errors="replace") + output += _decode_output(stdout) if stderr: - err = stderr.decode(OUTPUT_ENCODING, errors="replace") - output += "\n[stderr]\n" + err + output += "\n[stderr]\n" + _decode_output(stderr) if len(output) > MAX_OUTPUT_LENGTH: output = ( diff --git a/dashscope/acli/tools/skills.py b/dashscope/acli/tools/skills.py index 5c0d479..af5c693 100644 --- a/dashscope/acli/tools/skills.py +++ b/dashscope/acli/tools/skills.py @@ -67,7 +67,10 @@ async def use_skill(name: str, args: list[str] | None = None) -> str: f"{skill.mcp_service}; ask the user to connect first." ) rendered = render_skill(skill, [str(a) for a in (args or [])]) - if not rendered: + # `is None`, not falsy: render_skill only returns None when arguments + # are missing, so the hint below always lists at least one name. An + # empty body is a valid (if useless) skill, not a missing argument. + if rendered is None: hint = " ".join(f"<{a}>" for a in skill.arguments) return f"Error: missing args. '{name}' requires: {hint}" agent = get_agent() diff --git a/dashscope/acli/utils/paths.py b/dashscope/acli/utils/paths.py index 41a32e1..d529987 100644 --- a/dashscope/acli/utils/paths.py +++ b/dashscope/acli/utils/paths.py @@ -61,22 +61,61 @@ def validate_path(path: str) -> str: return path +def _temp_write_roots() -> tuple[str, ...]: + """Realpath'd system temp directories a write is allowed to land in. + + ``run_command`` can already write here — the shell is not path-guarded — + ``delete_file`` reaches anywhere behind its own prompt, and the opt-in OS + sandbox confines writes to "the workspace and temp areas". A cwd-only rule + for write_file was therefore stricter than every layer around it without + protecting anything, and it cost more than a refused call: models reach + for /tmp for scratch files by instinct, so the retry put the scratch file + in the user's repo instead, which is the worse outcome. + + ``tempfile.gettempdir()`` is what makes this portable — it honours + TMPDIR/TEMP/TMP, which on macOS is a per-user /var/folders path rather than + /tmp. /tmp is listed separately because that is what a model actually + writes, and realpath is what ties it to /private/tmp there. + """ + import tempfile + + candidates = [tempfile.gettempdir()] + if os.name != "nt": + candidates += ["/tmp", "/private/tmp"] + roots: list[str] = [] + for candidate in candidates: + if not candidate: + continue + try: + resolved = os.path.realpath(candidate) + except OSError: + continue + if resolved not in roots and os.path.isdir(resolved): + roots.append(resolved) + return tuple(roots) + + def validate_write_path(path: str) -> str: """Validate a path for write operations (stricter than read). - In addition to sensitive-path blocking, prevents writes outside cwd. + In addition to sensitive-path blocking, confines writes to the working + directory and the system temp dir. """ path = validate_path(path) # Resolve both sides with realpath: abspath doesn't follow symlinks, so # a symlink inside cwd pointing outside would otherwise pass the check. abs_path = os.path.realpath(path) - cwd = os.path.realpath(".") - if not abs_path.startswith(cwd + os.sep) and abs_path != cwd: - raise ValueError( - f"Write path '{path}' is outside the current working " - "directory (path traversal guard)", - ) - return path + roots = (os.path.realpath("."),) + _temp_write_roots() + inside = any( + abs_path == root or abs_path.startswith(root + os.sep) + for root in roots + ) + if inside: + return path + raise ValueError( + f"Write path '{path}' is outside the writable roots: the working " + "directory and the system temp dir (path traversal guard)", + ) def atomic_write_text(path: Path, text: str, encoding: str = "utf-8") -> None: diff --git a/dashscope/cli/__init__.py b/dashscope/cli/__init__.py index 0da6246..93de0d1 100644 --- a/dashscope/cli/__init__.py +++ b/dashscope/cli/__init__.py @@ -21,7 +21,7 @@ from rich.markup import escape # noqa: E402 import dashscope # noqa: E402 -from dashscope.cli.common import err_console # noqa: E402 +from dashscope.cli.common import console, err_console # noqa: E402 from dashscope.common.error import AuthenticationError # noqa: E402 from dashscope.common.utils import set_sdk_client # noqa: E402 from dashscope.cli import ( # noqa: E402 @@ -91,6 +91,7 @@ } _TOP_LEVEL_COMMANDS = { + "auth", "generation", "ft", "fine-tunes", @@ -115,6 +116,28 @@ _COMMANDS_WITH_LOCAL_API_KEY = {"oss", "rl", "agentic-rl"} + +# Routing keyword for the bundled agent, handled in main() before typer sees +# argv. Deliberately NOT a member of _TOP_LEVEL_COMMANDS: that set mirrors the +# typer-registered command groups, and `expert` is not one of them. +_EXPERT_COMMAND = "expert" + +_EXPERT_USAGE = """[bold]DashScope SDK Expert[/bold] — the built-in AI assistant. + +[bold]Usage:[/bold] + dashscope interactive assistant (TUI by default) + dashscope expert the same, spelled explicitly + dashscope expert --cli interactive, plain REPL instead of the TUI + dashscope expert --tui interactive, force the TUI + dashscope expert "question" ask once and exit + +With no argument, piped stdin is read as a one-shot prompt. +SDK subcommands (generation, ft, files, models, ...) dispatch as usual; an +unrecognized command is reported as an error rather than treated as a +question, so ask one with [bold]dashscope expert "question"[/bold]. + +Requires [bold]pip install dashscope[acli][/bold].""" + _LEGACY_COMMANDS_WITH_SIZE_OPTION = { "files.list", "fine_tunes.list", @@ -325,7 +348,10 @@ def _maybe_offer_example_download(): def _route_to_expert(command, tui=None): - """Run the vendored acli agent (dashscope with no/unknown subcommand).""" + """Run the vendored acli agent (bare ``dashscope`` or ``dashscope expert``). + + ``command`` is a one-shot prompt, or None for the interactive REPL. + """ try: from dashscope.acli.cli.handlers_key import _GUIDE_DOC, _doc_locale from dashscope.acli.ui.embedded import run @@ -348,7 +374,7 @@ def _route_to_expert(command, tui=None): if not piped: err_console.print( "[red]Error:[/red] Interactive mode requires a terminal. " - 'Pass a prompt instead, e.g. dashscope "your question".', + 'Pass a prompt instead, e.g. dashscope expert "your question".', ) sys.exit(2) command = piped @@ -511,20 +537,38 @@ def main(): forced_tui = True argv = [a for a in argv if a != "--tui"] + first_cmd = _first_non_option(argv) + # `rl` is an optional extra: report a missing dependency only when the # user asks for that command, never on unrelated invocations. Without # this gate the dispatch below reaches typer, which only knows that `rl` # is in _TOP_LEVEL_COMMANDS but was never registered — "No such command" # does not tell the user how to get it. - if ( - _RL_IMPORT_ERROR is not None - and _first_non_option(argv) in _RL_COMMAND_NAMES - ): + if _RL_IMPORT_ERROR is not None and first_cmd in _RL_COMMAND_NAMES: err_console.print( f"[red]Error:[/red] {_rl_unavailable_detail(_RL_IMPORT_ERROR)}", ) sys.exit(1) + # No arguments at all → the agent, interactively. -k/--api-key and + # --cli/--tui were stripped above, so `dashscope -k sk-x` lands here too. + if len(argv) == 1: + _route_to_expert(None, tui=forced_tui) + return + + # `dashscope expert ...` → the agent, explicitly. Handled before the + # --help gate so `dashscope expert --help` describes the agent instead of + # reaching typer, which has no such command. + if first_cmd == _EXPERT_COMMAND: + idx = argv.index(_EXPERT_COMMAND) + expert_args = argv[idx + 1 :] + if "--help" in expert_args or "-h" in expert_args: + console.print(_EXPERT_USAGE) + return + prompt = " ".join(expert_args).strip() + _route_to_expert(prompt or None, tui=forced_tui) + return + # Top-level --help / -h → show help and exit normally if "--help" in argv or "-h" in argv: argv = _translate_help_shortcut(argv) @@ -532,20 +576,11 @@ def main(): app() return - # No subcommand → enter the agent (acli) directly - first_cmd = _first_non_option(argv) - if first_cmd is None: - _route_to_expert(None, tui=forced_tui) - return - - # Unknown command → route to the agent with the original user input. - # Use the cleaned argv so an extracted -k/--api-key is not leaked into - # the prompt text. - if first_cmd not in _TOP_LEVEL_COMMANDS: - _route_to_expert(" ".join(argv[1:]), tui=forced_tui) - return - - # Direct command execution (backward compatible) + # Anything that is not bare `dashscope` and not `dashscope expert ...` + # belongs to typer, so a typo or a stray option gets a real error instead + # of silently opening a chat. Forwarding an unrecognized token as a + # question used to swallow whole command lines -- `dashscope auth whoami` + # opened a session whose first message was the command itself. argv = _translate_help_shortcut(argv) sys.argv = argv diff --git a/dashscope/version.py b/dashscope/version.py index 0ed5fd5..42b1753 100644 --- a/dashscope/version.py +++ b/dashscope/version.py @@ -1,4 +1,4 @@ # -*- coding: utf-8 -*- # Copyright (c) Alibaba, Inc. and its affiliates. -__version__ = "1.27.6" +__version__ = "1.27.7" diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index eb41d5c..7806808 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -10,6 +10,24 @@ from tests.unit.mock_server import create_mock_server, run_server +@pytest.fixture(autouse=True) +def isolated_generation_path_cache(monkeypatch, tmp_path): + """Keep TongyiProvider's learned model→path cache off the real ~/.acli. + + The cache is persisted across processes by design, so without this a test + run would write bindings into the developer's home directory — and would + read back whatever is already there, making the probe-order assertions + depend on machine state instead of on the code. + """ + from dashscope.acli import config as config_module + from dashscope.acli.providers.tongyi import TongyiProvider + + monkeypatch.setattr(config_module, "CONFIG_DIR", tmp_path / "acli-home") + # Class-level state survives across tests; monkeypatch restores the + # previous value on teardown, so each test starts from "nothing learned". + monkeypatch.setattr(TongyiProvider, "_path_cache", None) + + @pytest.fixture def mock_disable_data_inspection_env(monkeypatch): monkeypatch.setenv(DASHSCOPE_DISABLE_DATA_INSPECTION_ENV, "true") diff --git a/tests/unit/test_acli_skill_render.py b/tests/unit/test_acli_skill_render.py new file mode 100644 index 0000000..c53da9f --- /dev/null +++ b/tests/unit/test_acli_skill_render.py @@ -0,0 +1,143 @@ +# -*- coding: utf-8 -*- +# Copyright (c) Alibaba, Inc. and its affiliates. +"""Every bundled dashscope-sdk-expert skill must render through ``use_skill``. + +This is the dashscope-only half of the ``render_skill`` regression net: the 13 +skill files under ``examples/dashscope-sdk-expert/.acli/skills/`` ship in this +wheel and exist nowhere else, so they are the real corpus that broke. + +``render_skill`` used to run ``str.format()`` over the whole skill body. A body +is Markdown full of literal JSON and Python braces, so ``text-generation.md`` -- +which documents ``messages=[{"role": "user", ...}]`` and declares no arguments +at all -- raised ``KeyError('"role"')``. The old ``except KeyError`` turned that +into ``Error: missing args`` for a skill that takes none, so 6 of the 13 skills +were unusable through ``use_skill`` while ``/skill`` and the catalog kept +advertising them. + +The assertions here are deliberately about observable output rather than about +the substitution mechanism: declared placeholders disappear, their values +arrive, and every other brace comes back byte-identical. +""" + +# pylint: disable=protected-access,redefined-outer-name + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +import dashscope.acli +from dashscope.acli.skills.base import Skill, _parse_skill_md, render_skill + +SKILLS_DIR = ( + Path(dashscope.acli.__file__).parent + / "examples" + / "dashscope-sdk-expert" + / ".acli" + / "skills" +) + +# Distinct per position so a swapped argument order shows up as a missing +# sentinel instead of quietly passing. +SENTINEL = "ZZSENTINEL{}ZZ" + +_BRACE_SPAN = re.compile(r"\{[^{}]*\}") + + +def _bundled_skill_files() -> list[Path]: + return sorted(SKILLS_DIR.glob("*.md")) + + +def _load(path: Path) -> Skill: + skill = _parse_skill_md(path) + assert skill is not None, f"{path.name} did not parse" + return skill + + +def _args_for(skill: Skill) -> list[str]: + return [SENTINEL.format(i) for i in range(len(skill.arguments))] + + +@pytest.fixture(scope="module") +def bundled_skills() -> list[Skill]: + paths = _bundled_skill_files() + assert paths, f"no skill files found under {SKILLS_DIR}" + return [_load(p) for p in paths] + + +def test_the_bundled_corpus_is_the_one_that_ships(bundled_skills): + """Guard against the test going vacuous if ``examples/`` is ever dropped. + + A silent sync that stops copying the skills directory would otherwise leave + every assertion below passing over an empty corpus. + """ + names = {s.name for s in bundled_skills} + assert len(bundled_skills) == 13 + assert names == {path.stem for path in _bundled_skill_files()} + # Both shapes matter: the bug hit no-argument skills carrying JSON hardest, + # while the substitution path is only exercised by the ones with arguments. + assert any(s.arguments for s in bundled_skills) + assert any(not s.arguments for s in bundled_skills) + assert sum(1 for s in bundled_skills if "{" in s.prompt_template) >= 10 + + +@pytest.mark.parametrize("path", _bundled_skill_files(), ids=lambda p: p.stem) +def test_every_skill_renders_without_raising(path): + skill = _load(path) + rendered = render_skill(skill, _args_for(skill)) + assert rendered is not None, f"{skill.name} reported missing args" + assert rendered.strip() + + +@pytest.mark.parametrize("path", _bundled_skill_files(), ids=lambda p: p.stem) +def test_undeclared_braces_are_left_exactly_as_authored(path): + """A JSON sample in the body is documentation, not a placeholder.""" + skill = _load(path) + rendered = render_skill(skill, _args_for(skill)) + assert rendered is not None + + declared = {f"{{{a}}}" for a in skill.arguments} + literals = [ + span + for span in _BRACE_SPAN.findall(skill.prompt_template) + if span not in declared + ] + for span in literals: + assert span in rendered, f"{skill.name} lost {span!r}" + + +@pytest.mark.parametrize("path", _bundled_skill_files(), ids=lambda p: p.stem) +def test_declared_arguments_are_substituted(path): + skill = _load(path) + args = _args_for(skill) + rendered = render_skill(skill, args) + assert rendered is not None + + for name, value in zip(skill.arguments, args): + assert f"{{{name}}}" not in rendered, f"{skill.name} kept {{{name}}}" + assert value in rendered, f"{skill.name} dropped the value of {name}" + + +def test_a_no_argument_skill_renders_to_its_own_body(bundled_skills): + """The exact regression: ``text-generation`` takes no args and shows JSON.""" + skill = next(s for s in bundled_skills if s.name == "text-generation") + assert skill.arguments == [] + assert '{"role": "user", "content": "Hello"}' in skill.prompt_template + assert render_skill(skill, []) == skill.prompt_template + + +def test_too_few_arguments_still_reports_none(bundled_skills): + """``None`` is the missing-args signal; it must stay reachable. + + Both ``use_skill`` and the scheduler branch on ``rendered is None`` to print + the ``requires: `` hint. If rendering always succeeded, that hint would + become dead code and a genuinely short call would send a half-substituted + prompt to the model. + """ + translate = next(s for s in bundled_skills if s.name == "translate") + assert translate.arguments == ["target_lang", "text"] + assert render_skill(translate, []) is None + assert render_skill(translate, ["English"]) is None + assert render_skill(translate, ["English", "hello there"]) is not None diff --git a/tests/unit/test_cli_agent_route.py b/tests/unit/test_cli_agent_route.py index f662fba..7da8d57 100644 --- a/tests/unit/test_cli_agent_route.py +++ b/tests/unit/test_cli_agent_route.py @@ -1,13 +1,17 @@ # -*- coding: utf-8 -*- -"""Tests for the dashscope → acli agent route (expert subcommand removal). +"""Tests for the dashscope → acli agent route. Covers: - _cleanup_legacy_expert_sync: removes old managed-marker files from ~/.acli without touching user-owned files. - _route_to_expert: runs embedded acli with the global -k api key passed through, and offers the example download only for interactive no-arg runs. + On non-TTY stdin, piped text becomes a one-shot prompt and an empty pipe + exits 2 telling the user the `expert` keyword form. - _maybe_offer_example_download: accept/decline/marker/workspace gating. -- main(): unknown commands route to the agent with cleaned argv (no key leak). +- main(): only a bare `dashscope` or an explicit `expert` keyword reach the + agent; everything else is dispatched to typer so typos, stray options and + unrecognized commands produce a real error. """ # pylint: disable=protected-access,redefined-outer-name,unused-argument @@ -48,6 +52,33 @@ def _make_tty(monkeypatch): monkeypatch.setattr(sys, "stdin", TtyStdin()) +class NonTtyStdin: + """Piped stdin, optionally empty or unreadable.""" + + def __init__(self, data=""): + self._data = data + + def isatty(self): + return False + + def read(self): + if self._data is None: + raise OSError("stdin unavailable") + return self._data + + +@pytest.fixture() +def captured_err(monkeypatch): + lines: list = [] + + class Recorder: + def print(self, *args, **kwargs): + lines.append(" ".join(str(a) for a in args)) + + monkeypatch.setattr(cli, "err_console", Recorder()) + return lines + + class TestCleanupLegacyExpertSync: def test_removes_managed_files(self, fake_config_dir): skills = fake_config_dir / "skills" @@ -158,6 +189,57 @@ def test_cleanup_runs_before_agent( assert not managed.exists() assert captured_run["command"] == "hi" + def test_piped_stdin_becomes_a_one_shot_prompt( + self, + fake_config_dir, + captured_run, + monkeypatch, + ): + monkeypatch.setattr(cli, "_maybe_offer_example_download", lambda: None) + monkeypatch.setattr(sys, "stdin", NonTtyStdin(" 怎么用 Generation \n")) + + cli._route_to_expert(None) + + assert captured_run["command"] == "怎么用 Generation" + + def test_empty_pipe_names_the_expert_keyword( + self, + fake_config_dir, + captured_run, + captured_err, + monkeypatch, + ): + """The refusal is the user's only recovery instruction. + + It told people to run `dashscope "your question"` for two releases + after that form stopped routing to the agent, so the one message that + could have fixed their command line sent them to `No such command`. + """ + monkeypatch.setattr(sys, "stdin", NonTtyStdin(" \n")) + + with pytest.raises(SystemExit) as exc: + cli._route_to_expert(None) + + assert exc.value.code == 2 + assert not captured_run + message = "\n".join(captured_err) + assert 'dashscope expert "your question"' in message + assert 'e.g. dashscope "' not in message + + def test_unreadable_stdin_refuses_the_same_way( + self, + fake_config_dir, + captured_err, + monkeypatch, + ): + monkeypatch.setattr(sys, "stdin", NonTtyStdin(None)) + + with pytest.raises(SystemExit) as exc: + cli._route_to_expert(None) + + assert exc.value.code == 2 + assert 'dashscope expert "your question"' in "\n".join(captured_err) + class TestOfferExampleDownload: def test_accept_downloads_example( @@ -256,13 +338,20 @@ def fail_input(prompt=""): class TestMainRouting: - def test_unknown_command_routes_cleaned_text(self, monkeypatch): + def test_unknown_single_token_reaches_typer(self, monkeypatch): + """`dashscope "question"` is not a short form for the agent. + + An unrecognized token must error in typer so a mistyped command stays + visible; asking a question is `dashscope expert "question"`. + """ routed: list = [] + invoked: list = [] monkeypatch.setattr( cli, "_route_to_expert", - lambda command, tui=False: routed.append(command), + lambda command, tui=None: routed.append(command), ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) monkeypatch.setattr( sys, "argv", @@ -271,16 +360,39 @@ def test_unknown_command_routes_cleaned_text(self, monkeypatch): cli.main() - # The extracted -k value must not leak into the routed prompt text. - assert routed == ["你好，未知命令"] + assert not routed + assert invoked == [True] + # The global key is still extracted on the typer path. assert dashscope.api_key == "sk-secret" + def test_quoted_command_line_reaches_typer(self, monkeypatch): + """A whole command line passed as one shell word is not a question. + + It used to open a chat whose first user message was the command + itself, which is indistinguishable in the session log from a user who + meant to ask something. + """ + routed: list = [] + invoked: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append(command), + ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) + monkeypatch.setattr(sys, "argv", ["dashscope", "auth whoami"]) + + cli.main() + + assert not routed + assert invoked == [True] + def test_no_args_routes_interactive(self, monkeypatch): routed: list = [] monkeypatch.setattr( cli, "_route_to_expert", - lambda command, tui=False: routed.append(command), + lambda command, tui=None: routed.append(command), ) monkeypatch.setattr(sys, "argv", ["dashscope"]) @@ -288,20 +400,164 @@ def test_no_args_routes_interactive(self, monkeypatch): assert routed == [None] - def test_expert_is_no_longer_a_command(self, monkeypatch): + def test_expert_keyword_is_not_a_typer_command(self): + """`expert` is a routing keyword handled inside main(), never a typer + group — it must not appear in any table typer dispatches from.""" assert "expert" not in cli._TOP_LEVEL_COMMANDS assert "expert" not in cli._COMMANDS_WITH_LOCAL_API_KEY + registered = { + g.name or g.typer_instance.info.name + for g in cli.app.registered_groups + } + assert "expert" not in registered + + def test_expert_keyword_forwards_prompt(self, monkeypatch): routed: list = [] monkeypatch.setattr( cli, "_route_to_expert", - lambda command, tui=False: routed.append(command), + lambda command, tui=None: routed.append((command, tui)), ) monkeypatch.setattr(sys, "argv", ["dashscope", "expert", "chat"]) cli.main() - assert routed == ["expert chat"] + # The keyword itself is consumed; only the question is forwarded. + assert routed == [("chat", True)] + + def test_bare_expert_is_interactive(self, monkeypatch): + routed: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append((command, tui)), + ) + monkeypatch.setattr(sys, "argv", ["dashscope", "expert"]) + + cli.main() + + assert routed == [(None, True)] + + def test_expert_cli_flag_selects_plain_repl(self, monkeypatch): + routed: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append((command, tui)), + ) + monkeypatch.setattr(sys, "argv", ["dashscope", "expert", "--cli"]) + + cli.main() + + assert routed == [(None, False)] + + def test_expert_help_prints_usage_without_routing( + self, + monkeypatch, + capsys, + ): + routed: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append((command, tui)), + ) + monkeypatch.setattr(cli, "app", lambda: routed.append("typer")) + monkeypatch.setattr(sys, "argv", ["dashscope", "expert", "--help"]) + + cli.main() + + # Must not fall through to typer, which has no `expert` command. + assert not routed + assert "DashScope SDK Expert" in capsys.readouterr().out + + def test_multiword_unknown_command_reaches_typer(self, monkeypatch): + """A typo'd command group must error in typer, not open a chat.""" + routed: list = [] + invoked: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append(command), + ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) + monkeypatch.setattr(sys, "argv", ["dashscope", "generaton", "create"]) + + cli.main() + + assert not routed + assert invoked == [True] + + def test_stray_leading_option_reaches_typer(self, monkeypatch): + """`-w/--workspace` is a group option, so its value must not be + mistaken for the command name and routed into the agent.""" + routed: list = [] + invoked: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append(command), + ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) + monkeypatch.setattr( + sys, + "argv", + ["dashscope", "-w", "wsid", "generation", "create"], + ) + + cli.main() + + assert not routed + assert invoked == [True] + + def test_unknown_flag_reaches_typer(self, monkeypatch): + routed: list = [] + invoked: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append(command), + ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) + monkeypatch.setattr(sys, "argv", ["dashscope", "--foo"]) + + cli.main() + + assert not routed + assert invoked == [True] + + def test_registered_groups_are_all_routable(self): + """A group registered with typer but absent from _TOP_LEVEL_COMMANDS + is dispatched to typer anyway now, but the set still drives global + -k/--api-key parsing, so the two must not drift apart. + + Only this direction is asserted: `rl`/`agentic-rl` stay in the set even + when the optional extra is not installed. + """ + registered = { + g.name or g.typer_instance.info.name + for g in cli.app.registered_groups + } + assert registered <= cli._TOP_LEVEL_COMMANDS + + def test_auth_whoami_executes_instead_of_routing_to_agent( + self, + monkeypatch, + ): + routed: list = [] + invoked: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append(command), + ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) + monkeypatch.setattr(sys, "argv", ["dashscope", "auth", "whoami"]) + + cli.main() + + assert not routed + assert invoked == [True] class TestBundledExample: diff --git a/tests/unit/test_cli_auth.py b/tests/unit/test_cli_auth.py new file mode 100644 index 0000000..f3997b4 --- /dev/null +++ b/tests/unit/test_cli_auth.py @@ -0,0 +1,241 @@ +# -*- coding: utf-8 -*- +# Copyright (c) Alibaba, Inc. and its affiliates. +"""Behaviour of ``dashscope auth whoami`` and ``dashscope auth login``. + +Pins what a user sees on the terminal: the guidance printed when no key is +configured, the masked key and its source on success, and the file ``login`` +writes. Every test runs against a throwaway ``~/.dashscope`` and a stubbed +``Models.list``, so no real key file is touched and no request is sent. +""" + +# pylint: disable=redefined-outer-name,unused-argument + +from http import HTTPStatus +from types import SimpleNamespace + +import pytest +from typer.testing import CliRunner + +import dashscope +from dashscope.cli import auth as auth_cli +from dashscope.common import api_key as api_key_mod +from dashscope.common.constants import ( + DASHSCOPE_API_KEY_ENV, + DASHSCOPE_API_KEY_FILE_PATH_ENV, +) + +KEY = "sk-unittest-0123456789" +MASKED = "sk-uni...6789" + + +def _flat(text): + """Undo rich's 80-column hard wrapping so messages compare whole.""" + return "".join(text.splitlines()) + + +def _response(status_code, code=None, message=None): + return SimpleNamespace( + status_code=status_code, + code=code, + message=message, + ) + + +def _stub_models_list(monkeypatch, rsp=None, exc=None): + """Replace the single validation call ``whoami`` makes; record its args.""" + calls = [] + + def fake_list(**kwargs): + calls.append(kwargs) + if exc is not None: + raise exc + return rsp + + monkeypatch.setattr(dashscope.Models, "list", fake_list) + return calls + + +@pytest.fixture +def key_store(monkeypatch, tmp_path): + """Redirect every api-key lookup to an empty throwaway home.""" + monkeypatch.delenv(DASHSCOPE_API_KEY_ENV, raising=False) + monkeypatch.delenv(DASHSCOPE_API_KEY_FILE_PATH_ENV, raising=False) + monkeypatch.setattr(dashscope, "api_key", None) + monkeypatch.setattr(dashscope, "api_key_file_path", None) + + cache_dir = tmp_path / ".dashscope" + key_file = cache_dir / "api_key" + cache_dir.mkdir(parents=True, exist_ok=True) + # ``save_api_key``/``get_default_api_key`` read the constant from + # dashscope.common.api_key, while the printed path and the source check in + # auth read its own imported copy. Both bindings have to move, or login and + # whoami silently disagree about where the key lives. + monkeypatch.setattr( + api_key_mod, + "DEFAULT_DASHSCOPE_CACHE_PATH", + cache_dir, + ) + monkeypatch.setattr( + api_key_mod, + "DEFAULT_DASHSCOPE_API_KEY_FILE_PATH", + key_file, + ) + monkeypatch.setattr( + auth_cli, + "DEFAULT_DASHSCOPE_API_KEY_FILE_PATH", + key_file, + ) + return SimpleNamespace(dir=cache_dir, file=key_file) + + +class TestAuthWhoami: + def test_no_key_prints_login_guidance(self, monkeypatch, key_store): + calls = _stub_models_list(monkeypatch, rsp=_response(HTTPStatus.OK)) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 1 + text = _flat(result.output) + assert "Error: No API key configured." in text + guidance = ( + "Run dashscope auth login or set " + "DASHSCOPE_API_KEY to configure one." + ) + assert guidance in text + assert not calls, "must not reach the API without a key" + + def test_masks_key_and_reports_the_file_source( + self, + monkeypatch, + key_store, + ): + key_store.file.write_text(KEY, encoding="utf-8") + calls = _stub_models_list(monkeypatch, rsp=_response(HTTPStatus.OK)) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 0 + text = _flat(result.output) + assert f"Authenticated key={MASKED}" in text + assert f"source=file ({key_store.file})" in text + assert KEY not in text + assert calls == [{"page": 1, "page_size": 1}] + + def test_reports_an_environment_key_as_its_own_source( + self, + monkeypatch, + key_store, + ): + monkeypatch.setattr(dashscope, "api_key", KEY) + _stub_models_list(monkeypatch, rsp=_response(HTTPStatus.OK)) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 0 + text = _flat(result.output) + assert f"Authenticated key={MASKED}" in text + assert "source=environment / --api-key flag" in text + + def test_short_key_is_not_partially_revealed( + self, + monkeypatch, + key_store, + ): + short = "sk-abc123" + key_store.file.write_text(short, encoding="utf-8") + _stub_models_list(monkeypatch, rsp=_response(HTTPStatus.OK)) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 0 + text = _flat(result.output) + assert "key=***" in text + assert short not in text + + def test_rejected_key_exits_2(self, monkeypatch, key_store): + key_store.file.write_text(KEY, encoding="utf-8") + _stub_models_list( + monkeypatch, + rsp=_response( + HTTPStatus.UNAUTHORIZED, + "InvalidApiKey", + "Invalid API-key provided.", + ), + ) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 2 + text = _flat(result.output) + assert "Invalid API key" in text + assert f"source=file ({key_store.file})" in text + assert "code=InvalidApiKey" in text + assert KEY not in text + assert MASKED not in text + + def test_api_failure_exits_2_with_the_reason( + self, + monkeypatch, + key_store, + ): + key_store.file.write_text(KEY, encoding="utf-8") + _stub_models_list(monkeypatch, exc=RuntimeError("no route to host")) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 2 + assert "Error: API call failed: no route to host" in _flat( + result.output, + ) + assert KEY not in result.output + + +class TestAuthLogin: + def test_prompts_for_the_key_and_saves_it(self, key_store): + result = CliRunner().invoke(auth_cli.app, ["login"], input=f"{KEY}\n") + + assert result.exit_code == 0 + text = _flat(result.output) + assert "Enter your DashScope API key: " in text + assert f"Saved API key to {key_store.file}" in text + assert key_store.file.read_text(encoding="utf-8") == KEY + assert KEY not in text, "hide_input must keep the key off screen" + + def test_key_flag_skips_the_prompt(self, key_store): + result = CliRunner().invoke(auth_cli.app, ["login", "-k", KEY]) + + assert result.exit_code == 0 + assert "Enter your DashScope API key" not in result.output + assert f"Saved API key to {key_store.file}" in _flat(result.output) + assert key_store.file.read_text(encoding="utf-8") == KEY + + def test_surrounding_whitespace_is_stripped(self, key_store): + result = CliRunner().invoke( + auth_cli.app, + ["login"], + input=f" {KEY} \n", + ) + + assert result.exit_code == 0 + assert key_store.file.read_text(encoding="utf-8") == KEY + + def test_blank_key_is_refused_and_nothing_is_written(self, key_store): + result = CliRunner().invoke(auth_cli.app, ["login", "-k", " "]) + + assert result.exit_code == 1 + assert "Error: API key cannot be empty." in _flat(result.output) + assert not key_store.file.exists() + + def test_whoami_accepts_what_login_wrote(self, monkeypatch, key_store): + """The two commands must agree on the key file location.""" + saved = CliRunner().invoke(auth_cli.app, ["login", "-k", KEY]) + assert saved.exit_code == 0 + calls = _stub_models_list(monkeypatch, rsp=_response(HTTPStatus.OK)) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 0 + text = _flat(result.output) + assert f"Authenticated key={MASKED}" in text + assert f"source=file ({key_store.file})" in text + assert calls == [{"page": 1, "page_size": 1}] diff --git a/tests/unit/test_tongyi_native.py b/tests/unit/test_tongyi_native.py index 2fc4670..36ff2cc 100644 --- a/tests/unit/test_tongyi_native.py +++ b/tests/unit/test_tongyi_native.py @@ -17,10 +17,15 @@ from dashscope.acli.providers.tongyi import ( _GENERATION_PATHS, + _PATH_CACHE_NAME, TongyiProvider, ) -TEXT_GEN, MULTIMODAL_GEN = _GENERATION_PATHS +# Spelled out rather than destructured from _GENERATION_PATHS: a positional +# unpack silently swaps these two names the moment the probe order changes, +# inverting every assertion below while most of them still pass. +TEXT_GEN = "/api/v1/services/aigc/text-generation/generation" +MULTIMODAL_GEN = "/api/v1/services/aigc/multimodal-generation/generation" ROOT = "https://dashscope.aliyuncs.com" MESSAGES = [{"role": "user", "content": "hi"}] @@ -272,7 +277,6 @@ async def test_chat_posts_native_body(fake_http): resp = await _provider().chat(MESSAGES, tools=TOOLS) cap = fake_http.requests[0] - assert cap["url"] == ROOT + TEXT_GEN body = cap["body"] assert body["model"] == "qwen-plus" assert body["input"] == {"messages": MESSAGES} @@ -293,7 +297,6 @@ async def test_chat_stream_posts_native_body_with_sse_headers(fake_http): chunks = [c async for c in provider.chat_stream(MESSAGES, tools=TOOLS)] cap = fake_http.requests[0] - assert cap["url"] == ROOT + TEXT_GEN params = cap["body"]["parameters"] assert params["result_format"] == "message" assert params["stream"] is True @@ -362,27 +365,63 @@ def test_base_url_normalized_to_service_root(given, expected): # --------------------------------------------------------------------------- -# endpoint fallback: text-generation ⇄ multimodal-generation +# endpoint selection: multimodal-generation first, text-generation fallback # --------------------------------------------------------------------------- -async def test_chat_falls_back_to_multimodal_path(fake_http): - fake_http.enqueue_post(URL_ERROR_BODY, status_code=400) +def test_multimodal_path_is_probed_first(): + """Pinned on purpose. The gateway binds each model to one aigc path and + rejects the other with a 400 "url error" that lands in the prod log as + InvalidParameter, so the probe order decides who pays it. acli's default + model (qwen3.8-max) plus the vl/omni family are multimodal-bound; the + classic text models are the minority. Flip this and every fallback + assertion below flips with it.""" + assert _GENERATION_PATHS == (MULTIMODAL_GEN, TEXT_GEN) + + +async def test_chat_multimodal_model_needs_no_probe(fake_http): + """The common case: first request already hits the right path, and the + part-list content multimodal-generation returns is flattened.""" fake_http.enqueue_post(MULTIMODAL_RESPONSE) + resp = await _provider(model="qwen3.8-max").chat(MESSAGES) + + assert [r["url"] for r in fake_http.requests] == [ROOT + MULTIMODAL_GEN] + assert resp.content == "hello there" + assert resp.reasoning_content == "thinking..." + assert resp.usage["cached_tokens"] == 5 + + +async def test_stream_multimodal_model_needs_no_probe(fake_http): + fake_http.enqueue_stream(MULTIMODAL_SSE_LINES) provider = _provider(model="qwen3.8-max") + chunks = [c async for c in provider.chat_stream(MESSAGES)] + + assert [r["url"] for r in fake_http.requests] == [ROOT + MULTIMODAL_GEN] + assert "th" in [c.delta_reasoning_content for c in chunks] + assert [c.delta_content for c in chunks if c.delta_content] == [ + "Hel", + "lo", + ] + assert chunks[-1].usage["total_tokens"] == 9 + + +async def test_chat_falls_back_to_text_path(fake_http): + fake_http.enqueue_post(URL_ERROR_BODY, status_code=400) + fake_http.enqueue_post(NATIVE_RESPONSE) + provider = _provider(model="qwen-plus") resp = await provider.chat(MESSAGES) assert [r["url"] for r in fake_http.requests] == [ - ROOT + TEXT_GEN, ROOT + MULTIMODAL_GEN, + ROOT + TEXT_GEN, ] assert resp.content == "hello there" - assert resp.reasoning_content == "thinking..." - assert resp.usage["cached_tokens"] == 5 + assert resp.usage["total_tokens"] == 18 # working path is cached: the next call skips the 400 round-trip fake_http.enqueue_post(NATIVE_RESPONSE) await provider.chat(MESSAGES) - assert fake_http.requests[-1]["url"] == ROOT + MULTIMODAL_GEN + assert len(fake_http.requests) == 3 + assert fake_http.requests[-1]["url"] == ROOT + TEXT_GEN async def test_chat_no_fallback_on_other_400(fake_http): @@ -395,27 +434,25 @@ async def test_chat_no_fallback_on_other_400(fake_http): assert len(fake_http.requests) == 1 -async def test_stream_falls_back_to_multimodal_path(fake_http): +async def test_stream_falls_back_to_text_path(fake_http): fake_http.enqueue_stream( [], status_code=400, body=json.dumps(URL_ERROR_BODY, ensure_ascii=False), ) - fake_http.enqueue_stream(MULTIMODAL_SSE_LINES) - provider = _provider(model="qwen3.8-max") + fake_http.enqueue_stream(NATIVE_SSE_LINES) + provider = _provider(model="qwen-plus") chunks = [c async for c in provider.chat_stream(MESSAGES)] assert [r["url"] for r in fake_http.requests] == [ - ROOT + TEXT_GEN, ROOT + MULTIMODAL_GEN, + ROOT + TEXT_GEN, ] - reasoning = [c.delta_reasoning_content for c in chunks] - assert "th" in reasoning contents = [c.delta_content for c in chunks if c.delta_content] assert contents == ["Hel", "lo"] last = chunks[-1] assert last.finish_reason == "stop" - assert last.usage["total_tokens"] == 9 + assert last.usage["total_tokens"] == 13 async def test_stream_falls_back_on_sse_error_event(fake_http): @@ -429,27 +466,27 @@ async def test_stream_falls_back_on_sse_error_event(fake_http): "data:" + json.dumps(URL_ERROR_BODY, ensure_ascii=False), ], ) - fake_http.enqueue_stream(MULTIMODAL_SSE_LINES) - provider = _provider(model="qwen3.8-max") + fake_http.enqueue_stream(NATIVE_SSE_LINES) + provider = _provider(model="qwen-plus") chunks = [c async for c in provider.chat_stream(MESSAGES)] assert [r["url"] for r in fake_http.requests] == [ - ROOT + TEXT_GEN, ROOT + MULTIMODAL_GEN, + ROOT + TEXT_GEN, ] assert [c.delta_content for c in chunks if c.delta_content] == [ "Hel", "lo", ] # the working path is cached only after a real chunk arrived - assert provider._generation_path == MULTIMODAL_GEN + assert provider._generation_path == TEXT_GEN async def test_stream_sse_url_error_raises_after_paths_exhausted(fake_http): event = ["data:" + json.dumps(URL_ERROR_BODY, ensure_ascii=False)] fake_http.enqueue_stream(list(event)) fake_http.enqueue_stream(list(event)) - provider = _provider(model="qwen3.8-max") + provider = _provider() with pytest.raises(RuntimeError, match="url error"): async for _ in provider.chat_stream(MESSAGES): pass @@ -461,13 +498,89 @@ async def test_stream_no_fallback_after_content_started(fake_http): "data:" + json.dumps(URL_ERROR_BODY, ensure_ascii=False), ] fake_http.enqueue_stream(lines) - provider = _provider(model="qwen3.8-max") + provider = _provider() with pytest.raises(RuntimeError, match="url error"): async for _ in provider.chat_stream(MESSAGES): pass assert len(fake_http.requests) == 1 +# --------------------------------------------------------------------------- +# learned-path cache: a probe costs a 400 in the prod gateway log, so the +# answer is remembered per model — in-process and on disk +# --------------------------------------------------------------------------- + + +async def test_learned_path_skips_the_probe_in_a_new_instance(fake_http): + """A provider is constructed per session start, per model switch and per + vision call; only the first of those may pay the probe.""" + fake_http.enqueue_post(URL_ERROR_BODY, status_code=400) + fake_http.enqueue_post(NATIVE_RESPONSE) + await _provider().chat(MESSAGES) + assert len(fake_http.requests) == 2 + + fake_http.enqueue_post(NATIVE_RESPONSE) + await _provider().chat(MESSAGES) + assert len(fake_http.requests) == 3 + assert fake_http.requests[-1]["url"] == ROOT + TEXT_GEN + + +async def test_learned_path_survives_a_new_process(fake_http, monkeypatch): + """Bench and CI runs execute in throwaway containers, so the binding has + to outlive the process to be worth anything.""" + fake_http.enqueue_post(URL_ERROR_BODY, status_code=400) + fake_http.enqueue_post(NATIVE_RESPONSE) + await _provider().chat(MESSAGES) + assert len(fake_http.requests) == 2 + + # Simulate a fresh process: empty in-process cache, same home directory. + monkeypatch.setattr(TongyiProvider, "_path_cache", None) + fake_http.enqueue_post(NATIVE_RESPONSE) + await _provider().chat(MESSAGES) + assert len(fake_http.requests) == 3 + assert fake_http.requests[-1]["url"] == ROOT + TEXT_GEN + + +async def test_path_cache_is_written_next_to_the_global_config(fake_http): + from dashscope.acli import config as config_module + + fake_http.enqueue_post(URL_ERROR_BODY, status_code=400) + fake_http.enqueue_post(NATIVE_RESPONSE) + await _provider().chat(MESSAGES) + + cache_file = config_module.CONFIG_DIR / _PATH_CACHE_NAME + assert json.loads(cache_file.read_text(encoding="utf-8")) == { + "qwen-plus": TEXT_GEN, + } + + +def test_path_cache_drops_unknown_paths(monkeypatch): + """A model the gateway rebinds, or a hand-edited cache, must fall back to + probing rather than POST to a path that no longer exists.""" + from dashscope.acli import config as config_module + + cache_file = config_module.CONFIG_DIR / _PATH_CACHE_NAME + cache_file.parent.mkdir(parents=True, exist_ok=True) + cache_file.write_text( + json.dumps({"qwen-plus": "/api/v1/services/aigc/bogus/generation"}), + encoding="utf-8", + ) + monkeypatch.setattr(TongyiProvider, "_path_cache", None) + + assert _provider()._generation_path == MULTIMODAL_GEN + + +def test_path_cache_survives_a_corrupt_file(monkeypatch): + from dashscope.acli import config as config_module + + cache_file = config_module.CONFIG_DIR / _PATH_CACHE_NAME + cache_file.parent.mkdir(parents=True, exist_ok=True) + cache_file.write_text("not json at all", encoding="utf-8") + monkeypatch.setattr(TongyiProvider, "_path_cache", None) + + assert _provider()._generation_path == MULTIMODAL_GEN + + # --------------------------------------------------------------------------- # non-stream response parsing # ---------------------------------------------------------------------------