From 816f958547ed9a342218f4369a6fb5f144eae8da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A7=90=E6=9E=9D?= Date: Tue, 18 Aug 2026 09:44:10 +0800 Subject: [PATCH 1/2] test(agent): characterize configuration compatibility --- .github/workflows/ci.yml | 8 ++ AGENTS.md | 5 ++ Makefile | 5 +- action/main_test.py | 113 +++++++++++++++++++++++++++++ docs/design/agent-configuration.md | 101 ++++++++++++++++++++++++++ docs/guide/cli-reference.md | 5 +- docs/zh/guide/cli-reference.md | 5 +- 7 files changed, 239 insertions(+), 3 deletions(-) create mode 100644 action/main_test.py create mode 100644 docs/design/agent-configuration.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7dd27118..63f0a132 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,6 +89,14 @@ jobs: - name: Test run: go test -race -timeout 120s -covermode=atomic -coverpkg=./... -coverprofile=coverage.out ./... + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: '3.12' + + - name: Test GitHub Action adapter + run: make test-action + - name: Report coverage run: | set -euo pipefail diff --git a/AGENTS.md b/AGENTS.md index c2fa6547..52d9d5f2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,7 +59,12 @@ go build -o bin/skill-up ./cmd/skill-up ```bash # Unit tests (race detector enabled — always use this) make test + # equivalent: go test -race ./... +go test -race ./... + +# GitHub Action adapter tests (requires Python 3) +make test-action # Run a single test go test -race -run TestFoo ./internal/config/ diff --git a/Makefile b/Makefile index d0e1f080..bdb086e8 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build test vet fmt fmt-check lint lint-new revive verify tidy clean install hooks e2e lint-tools coverage coverage-badge +.PHONY: build test test-action vet fmt fmt-check lint lint-new revive verify tidy clean install hooks e2e lint-tools coverage coverage-badge CMD := ./cmd/skill-up VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev") @@ -28,6 +28,9 @@ build: hooks test: go test -race ./... +test-action: + python3 -m unittest discover -s action -p '*_test.py' + vet: go vet ./... diff --git a/action/main_test.py b/action/main_test.py new file mode 100644 index 00000000..fe0b4e0f --- /dev/null +++ b/action/main_test.py @@ -0,0 +1,113 @@ +#!/usr/bin/env python3 +"""Characterization tests for the public GitHub Action adapter.""" + +import importlib.util +import pathlib +import unittest + + +MODULE_PATH = pathlib.Path(__file__).with_name("main.py") +SPEC = importlib.util.spec_from_file_location("skill_up_action_main", MODULE_PATH) +ACTION = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(ACTION) + + +class ActionConfigurationContractTest(unittest.TestCase): + def test_protocol_selects_provider_endpoint(self): + cases = [ + ("claude_code", "dashscope", "", "https://dashscope.aliyuncs.com/apps/anthropic"), + ("codex", "dashscope", "", "https://dashscope.aliyuncs.com/compatible-mode/v1"), + ("qwen_code", "dashscope", "", "https://dashscope.aliyuncs.com/compatible-mode/v1"), + ("qodercli", "dashscope", "", ""), + ("", "dashscope", "", ""), + ("claude_code", "dashscope", "https://gateway.example/v1", "https://gateway.example/v1"), + ] + + for engine, provider, explicit_url, expected in cases: + with self.subTest(engine=engine, provider=provider, explicit_url=explicit_url): + self.assertEqual( + ACTION.resolve_base_url(engine, provider, explicit_url), + expected, + ) + + def test_model_ref_preserves_historical_action_translation(self): + cases = [ + ("codex", "dashscope", "qwen3.6-plus", "dashscope/qwen3.6-plus"), + ("codex", "dashscope", "org/model", "org/model"), + ("claude_code", "dashscope", "qwen3.6-plus", "qwen3.6-plus"), + ("qodercli", "qoder", "auto", "auto"), + ("qwen_code", "dashscope", "qwen3.6-plus", "qwen3.6-plus"), + ("", "dashscope", "qwen3.6-plus", "qwen3.6-plus"), + ("codex", "dashscope", "", ""), + ] + + for engine, provider, model, expected in cases: + with self.subTest(engine=engine, provider=provider, model=model): + self.assertEqual( + ACTION.compose_model_ref(provider, model, engine), + expected, + ) + + def test_explicit_engine_routes_auth_without_cli_api_key(self): + argv = ACTION.build_skill_up_argv( + "skill-up run", + "codex", + "qwen3.6-plus", + "dashscope", + "", + "/tmp/reports", + "evals/eval.yaml", + api_key="secret", + ) + + self.assertEqual(argv[:6], [ + "skill-up", "run", "--engine", "codex", + "--model", "dashscope/qwen3.6-plus", + ]) + self.assertNotIn("--api-key", argv) + self.assertEqual( + ACTION.engine_env("codex", "secret", "https://gateway.example/v1"), + { + "OPENAI_API_KEY": "secret", + "OPENAI_BASE_URL": "https://gateway.example/v1", + }, + ) + + def test_empty_engine_delegates_to_eval_yaml(self): + argv = ACTION.build_skill_up_argv( + "skill-up run", + "", + "org/model", + "dashscope", + "", + "/tmp/reports", + "evals/eval.yaml", + api_key="secret", + ) + + self.assertNotIn("--engine", argv) + self.assertEqual(argv[2:6], ["--api-key", "secret", "--model", "org/model"]) + + def test_absent_credentials_leave_agent_login_untouched(self): + for engine in ACTION.ENGINE_PROTOCOL: + with self.subTest(engine=engine): + self.assertEqual(ACTION.engine_env(engine, "", ""), {}) + self.assertEqual(ACTION.provider_env("dashscope", "", ""), {}) + self.assertEqual(ACTION.unified_base_url_env("", ""), {}) + + def test_empty_engine_exports_both_protocol_endpoints(self): + self.assertEqual( + ACTION.unified_base_url_env("dashscope", ""), + { + "OPENAI_BASE_URL": "https://dashscope.aliyuncs.com/compatible-mode/v1", + "ANTHROPIC_BASE_URL": "https://dashscope.aliyuncs.com/apps/anthropic", + }, + ) + + def test_unknown_engine_fails_fast(self): + with self.assertRaisesRegex(ValueError, "unknown engine"): + ACTION.resolve_base_url("unknown", "dashscope", "") + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/design/agent-configuration.md b/docs/design/agent-configuration.md new file mode 100644 index 00000000..cc368267 --- /dev/null +++ b/docs/design/agent-configuration.md @@ -0,0 +1,101 @@ +# Agent configuration semantics + +This document records the current `v1alpha1` behavior that future agent +configuration refactors must preserve. It is a compatibility contract, not a +claim that every current precedence rule is ideal. + +## Terms + +| Term | Meaning | +| --- | --- | +| Engine | The CLI adapter skill-up executes, such as `claude_code`, `codex`, `qodercli`, `qwen_code`, or a custom engine. | +| Protocol | The API protocol selected by the engine adapter. It is not selected by `provider`. | +| Provider | The namespace used to look up model, credential, and endpoint configuration. | +| Model name | The upstream model identifier. It may contain `/` when the upstream service uses an opaque slashed identifier. | +| Requested configuration | Values supplied by eval YAML, CLI flags, environment variables, or the credential file. | +| Effective configuration | Values the adapter actually passes to the agent CLI after adapter-specific normalization. | +| Local-login delegation | No credential is injected; the agent CLI may use its existing local login state. This does not prove that the login is valid. | + +The engine determines the protocol. For example, `provider: dashscope` with a +`codex` engine uses an OpenAI-compatible endpoint, while the same provider with +`claude_code` uses an Anthropic-compatible endpoint. + +## Built-in engine behavior + +| Engine | Protocol and auth surface | Model behavior | Unsupported or delegated behavior | +| --- | --- | --- | --- | +| `claude_code` | Anthropic-compatible; `ANTHROPIC_API_KEY` and `ANTHROPIC_BASE_URL` | Passes an explicit model through to Claude Code | Missing credentials delegate to Claude Code's local login. | +| `codex` | OpenAI-compatible; `OPENAI_API_KEY` and `OPENAI_BASE_URL` | A non-OpenAI provider requires a base URL before skill-up emits a custom Codex provider and model override | Without the required custom-provider endpoint, the model override is omitted and Codex uses local settings. Missing credentials may delegate to local login. | +| `qodercli` | Qoder-managed auth; `QODER_PERSONAL_ACCESS_TOKEN` or local login | Supports `lite`, `efficient`, `auto`, `performance`, and `ultimate` | Other model values and `base_url` are ignored with diagnostic logs. A generic provider API key is not a Qoder PAT. | +| `qwen_code` | OpenAI-compatible; `OPENAI_API_KEY`, `OPENAI_BASE_URL`, and `OPENAI_MODEL` | Passes an explicit model to Qwen Code | Missing credentials may delegate to Qwen OAuth or another existing local login. | +| Custom engine | Defined by `engine.custom` | Receives the configured provider/model values through session input and template variables | Capabilities and auth behavior belong to the custom-engine contract. | + +`Agent.Check` only inspects installation or availability. Credential checks are +static and do not make a model request. The first real agent run is therefore +the first validation of a delegated local login. + +## Current resolution order + +The current runner path resolves values in these stages: + +1. Load the eval YAML and apply `--engine` and `--model` overrides. +2. Load `~/.skill-up/credentials.yaml` and provider-scoped environment values. +3. Resolve provider-scoped `MODEL`, `API_KEY`, and `BASE_URL` values. A + provider-scoped model environment variable currently overrides the YAML + model; provider environment credentials override the credential file. +4. Apply explicit CLI `--model` and `--api-key` last. +5. Let the selected adapter normalize unsupported values and construct its + command and environment. + +This explains why requested and effective values can differ today. A later +phase should retain both instead of reconstructing effective configuration from +the eval YAML in reports. + +## Legacy slashed model compatibility + +`--model provider/name` is a public, historical CLI form and remains supported. +Because `/` is also valid inside an opaque upstream model ID, skill-up uses the +following compatibility behavior: + +| Input | Current interpretation | +| --- | --- | +| `--model openai/gpt-4` | `provider=openai`, `name=gpt-4`; `openai` and `anthropic` are always-known framework namespaces. | +| `--model dashscope/qwen3.6-plus` with DashScope credentials or endpoint configured | `provider=dashscope`, `name=qwen3.6-plus`. | +| `--model anthropic_modelscope/deepseek-v4-pro` with no matching provider configuration | `provider=""`, `name=anthropic_modelscope/deepseek-v4-pro`; the full ID is preserved. | +| YAML `model.provider` plus `model.name` | Always treated as an explicit pair, including when local login is the only auth source. | + +Provider detection is a disambiguation signal, not an authentication check. A +known provider can still fail authentication during the real agent run. + +## Public GitHub Action compatibility + +The root GitHub Action exposes separate `engine`, `provider`, and `model` +inputs. Its current translation is part of the public compatibility surface: + +- an explicit `codex` engine folds a bare provider and model into the legacy + `provider/model` CLI value so skill-up constructs Codex custom-provider + configuration; +- other explicit engines receive the model name without a provider prefix and + get protocol-specific environment variables; +- an empty engine does not add `--engine`, allowing eval YAML to select the + adapter, and exports both protocol endpoint variables when a known provider + is selected; +- absent credentials are not synthesized, preserving agent-local login flows. + +These translations are covered by `action/main_test.py`. Any future explicit +`--provider` flag must be additive: tagged Actions and historical +`--model provider/name` commands must remain valid. + +## Known gaps for later phases + +- Provider, protocol, credential source, and effective model are not yet held + in one immutable resolved configuration. +- Nested provider endpoints are flattened before the adapter protocol is known. +- Provider-scoped `MODEL` currently overrides an explicit YAML model. +- Adapters may ignore unsupported explicit values rather than failing before + case execution. +- Reports do not consistently distinguish requested configuration from the + effective adapter configuration. + +See [Issue #196](https://github.com/alibaba/skill-up/issues/196) for the staged +cleanup plan. diff --git a/docs/guide/cli-reference.md b/docs/guide/cli-reference.md index 469f9c63..50a76293 100644 --- a/docs/guide/cli-reference.md +++ b/docs/guide/cli-reference.md @@ -29,7 +29,7 @@ skill-up run [path] [flags] | `--output-dir` | `-workspace/` next to the skill dir | Output directory for reports and artifacts | | `--iteration` | `0` (auto) | Repeat selected cases for stability/flakiness sampling. `0` auto-appends one run after the latest `iteration-N/` without summarizing history; positive `N` runs N samples and writes `iteration-1/` … `iteration-N/`; when `N > 1`, the terminal summary covers only samples from the current command | | `--engine` | From config | Override engine name | -| `--model` | From config | Override model (format: `provider/name`) | +| `--model` | From config | Override model. The legacy `provider/name` form is split when the prefix is a configured provider; otherwise the complete value is preserved as an opaque model ID. | | `--parallelism` | From config | Override `cases.parallelism`. Allowed range: 1–256 | | `--baseline` | From config | Override `benchmark.enabled` to `true` for this run | | `--api-key` | — | Pass an API key (higher precedence than env vars) | @@ -57,6 +57,9 @@ skill-up run ./evals/eval.yaml --exclude-case-name "*-old" --exclude-case-name " # Override engine and model skill-up run ./evals/eval.yaml --engine codex --model openai/gpt-4 +# Preserve an opaque upstream model ID when its prefix is not a configured provider +skill-up run ./evals/eval.yaml --engine claude_code --model anthropic_modelscope/deepseek-v4-pro + # Temporarily override case parallelism skill-up run ./evals/eval.yaml --parallelism 4 diff --git a/docs/zh/guide/cli-reference.md b/docs/zh/guide/cli-reference.md index f309e717..d9e7c55c 100644 --- a/docs/zh/guide/cli-reference.md +++ b/docs/zh/guide/cli-reference.md @@ -29,7 +29,7 @@ skill-up run [path] [flags] | `--output-dir` | 与 skill 目录同级的 `-workspace/` | 报告和产物的输出目录 | | `--iteration` | `0`（auto） | 重复运行已选用例，用于稳定性/flaky 采样。`0` 表示在最新 `iteration-N/` 后自动追加一轮，但不汇总历史结果；正整数 `N` 表示运行 N 次采样，产物写入 `iteration-1/` 到 `iteration-N/`；当 `N > 1` 时，终端摘要只覆盖本次命令执行的采样 | | `--engine` | 配置文件中的值 | 覆盖 Engine 名称 | -| `--model` | 配置文件中的值 | 覆盖模型（格式：`provider/name`） | +| `--model` | 配置文件中的值 | 覆盖模型。兼容历史 `provider/name` 写法：前缀是已配置 provider 时拆分，否则将完整值作为不透明模型 ID 透传。 | | `--parallelism` | 配置文件中的值 | 覆盖 `cases.parallelism`，用于临时调整用例并行数，取值范围为 1 到 256 | | `--baseline` | 配置文件中的值 | 为本次运行覆盖 `benchmark.enabled` 为 `true` | | `--api-key` | — | 传入 API Key（优先级高于环境变量） | @@ -50,6 +50,9 @@ skill-up run ./evals/eval.yaml --exclude-case-name "*-old" --exclude-case-name " # 指定 Engine 和模型 skill-up run ./evals/eval.yaml --engine codex --model openai/gpt-4 +# 当前缀不是已配置 provider 时，保留包含斜杠的上游模型 ID +skill-up run ./evals/eval.yaml --engine claude_code --model anthropic_modelscope/deepseek-v4-pro + # 临时覆盖用例并行数 skill-up run ./evals/eval.yaml --parallelism 4 From 0e8752fc4492146a88f1698b133d9aea8cb39a7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A7=90=E6=9E=9D?= Date: Tue, 18 Aug 2026 15:47:56 +0800 Subject: [PATCH 2/2] test(action): cover composed Codex configuration --- action/main_test.py | 54 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/action/main_test.py b/action/main_test.py index fe0b4e0f..d8e9574f 100644 --- a/action/main_test.py +++ b/action/main_test.py @@ -3,7 +3,10 @@ import importlib.util import pathlib +import tempfile import unittest +from types import SimpleNamespace +from unittest import mock MODULE_PATH = pathlib.Path(__file__).with_name("main.py") @@ -13,6 +16,57 @@ class ActionConfigurationContractTest(unittest.TestCase): + def test_main_composes_codex_argv_and_environment(self): + inputs = SimpleNamespace( + engine="codex", + model="qwen3.6-plus", + provider="dashscope", + api_key="secret", + base_url="", + open_sandbox_api_key="", + skill_target="evals/eval.yaml", + skill_up_version="0.7.0", + skill_up_command="skill-up run", + parallelism="", + agent_install_command="", + ) + final_run = mock.Mock(return_value=SimpleNamespace(returncode=0)) + + with tempfile.TemporaryDirectory() as workspace: + with ( + mock.patch.object(ACTION, "parse_inputs", return_value=inputs), + mock.patch.object(ACTION.shutil, "which", return_value="/usr/bin/skill-up"), + mock.patch.object(ACTION, "_run"), + mock.patch.object(ACTION, "get_skill_up_version", return_value=(0, 7, 0)), + mock.patch.object(ACTION.subprocess, "run", final_run), + mock.patch.dict(ACTION.os.environ, {"GITHUB_WORKSPACE": workspace}, clear=True), + mock.patch("builtins.print"), + self.assertRaises(SystemExit) as exit_context, + ): + ACTION.main() + + self.assertEqual(exit_context.exception.code, 0) + final_run.assert_called_once() + argv = final_run.call_args.args[0] + run_env = final_run.call_args.kwargs["env"] + + engine_flag_index = argv.index("--engine") + self.assertEqual( + argv[engine_flag_index:engine_flag_index + 4], + ["--engine", "codex", "--model", "dashscope/qwen3.6-plus"], + ) + self.assertEqual(run_env["OPENAI_MODEL"], "qwen3.6-plus") + self.assertEqual(run_env["OPENAI_API_KEY"], "secret") + self.assertEqual( + run_env["OPENAI_BASE_URL"], + "https://dashscope.aliyuncs.com/compatible-mode/v1", + ) + self.assertEqual(run_env["DASHSCOPE_API_KEY"], "secret") + self.assertEqual( + run_env["DASHSCOPE_BASE_URL"], + "https://dashscope.aliyuncs.com/compatible-mode/v1", + ) + def test_protocol_selects_provider_endpoint(self): cases = [ ("claude_code", "dashscope", "", "https://dashscope.aliyuncs.com/apps/anthropic"),