diff --git a/.agents/README.md b/.agents/README.md index 5f56d8db..900fc6e0 100644 --- a/.agents/README.md +++ b/.agents/README.md @@ -248,3 +248,5 @@ Builtin transport: [direct HTTP](notes/implemented/architecture/2026-10-18-nativ - [Consented Express managed API keys](notes/implemented/architecture/2026-10-27-managed-api-key.md) · [verification](evidence/verify/2026-10-27-managed-api-key.md). - [Managed-key supported CI acceptance](evidence/verify/2026-10-27-managed-api-key-ci.md). + +- [Unused-code cleanup](notes/proposed/process/2026-10-27-dead-code-cleanup.md) · [verification](evidence/verify/2026-10-27-dead-code-cleanup.md). diff --git a/.agents/evidence/verify/2026-10-27-dead-code-cleanup.md b/.agents/evidence/verify/2026-10-27-dead-code-cleanup.md new file mode 100644 index 00000000..36756c5a --- /dev/null +++ b/.agents/evidence/verify/2026-10-27-dead-code-cleanup.md @@ -0,0 +1,40 @@ +# Verification: Scoped unused-code cleanup + +- Date: 2026-10-27 +- Issue: https://github.com/Alibaba-Agent-Sandbox/python-sdk/issues/146 +- Base revision: `259cc1ef9b154346bb8d5de0689b4b624d97baff` +- Functional patch SHA256: `4eec7a76ae4dac0d950d18451b75276046622295daa021cb294df8ae02a74d47` (Git binary diff against the base, excluding records/docs/changelog). +- Result: PARTIAL +- Environment: Linux; Node 22.22.2, TypeScript 5.9.3, ESLint 10.12.0; final Python checks use system Python 3.13.13; Ruff 0.17.3. + +## What was verified + +The scoped deletion preserves observable behavior, public contracts, generator ownership and historical evidence. Work is isolated from pre-existing workspace edits. Scope is the functional patch above plus its bilingual documentation, changelog where applicable, decision and evidence; current-head CI will bind verification to the pushed commit. + +## Commands and results + +| Command / check | Result | Evidence | +| --- | --- | --- | +| Ruff lint and format checks, both packages | PASS | Isolated development environment. | +| Mypy, both packages | PASS | Client: 101 source files; server completed before its test step. | +| Client `pytest tests/ -m "not integration"` | PASS | 2,345 passed, 26 skipped, 10 deselected. | +| Server `pytest tests/ -m "not integration"` | FAIL locally | 878 passed, 2 raw HTTP transport tests failed. | +| `make ci` | PARTIAL / exit 2 | Stopped at the two server tests; client and distribution steps were executed separately. | +| `make build && make dist-check` | PASS | Both wheels/sdists, twine, py.typed and clean combined installation/imports. | +| `bash scripts/check-repo-health.sh` | PASS | 106 pass, 1 warning, 0 failures before final evidence update. | +| Existing-key service listing | PASS | Read-only CLI preflight against the existing configured service. | +| `make e2e` plus corrected single-case rerun | PASS in combined results | Initial run: 129 passed, 1 failed, 11 skipped, 2 deselected. The only failure lacked ABX_BIN; after supplying the immutable CLI artifact, that exact test passed (445.88 seconds). No product source changed between runs; the initial failure remains recorded. | +| Remote CI at `0e5a572bef3c7d5068a4e80a80584980343e2a76` | PASS | Full Python 3.10/3.11/3.12 client/server matrix, public contract, runtime fixtures and Node provider engine; Docs links also passed. | +| `python scripts/e2e-cleanup.py --run-id "$E2E_RUN_ID"` | PASS | Dry run: 0 candidates, 0 deletions. Subsequent independent SDK inventory: 0 total sandboxes, 0 owned by this run. | + +The failing server cases are `test_upgrade_without_a_key_is_a_400_not_a_hang` (501 instead of 400) and `test_transfer_encoding_rejected_in_policy_mode` (502 instead of 400). An independent HTTPServer negative control always sends 400: malformed WebSocket traffic is intercepted before reaching that handler and returns 501; chunked POST reaches the handler but the observed response becomes 502. An ordinary explicit-port request reaches it and returns 400. This isolates local forwarding behavior from the unchanged server implementation. Remote CI must validate the original tests on normal networking; no security assertion was weakened. + +Initial environment attempts are not passes: the downloaded Python 3.12 lacks memfd/pidfd support used by process tests; a shared template environment installs Playwright and makes a strict unused-import-ignore check fail. Final SDK checks use a separate system Python 3.13 environment without Playwright. + +## Scope and further analysis + +Only private `_a()` is removed from runtime source. Public aliases/exports, generated schema, package versions and server source are unchanged. Example-import findings were already resolved upstream. Compatibility parameters and async-generator `yield` statements remain. `_cleanup_browser` needs a lifecycle-specific teardown analysis and regression before any removal; no resource leak is asserted from a definition-only result. + +## Cleanup and remaining gates + +Real-component acceptance totals 130 passing cases across the initial run and corrected one-case rerun. Eleven optional/provider/volume/conditional cases remain skipped; two slow cases were deselected. AK/SK key preparation and renewal were not attempted. The CLI provider rerun verified create/info/list/exec/files and two idempotent kills. Independent post-run inventory was empty: none leaked. This metadata-only evidence update still requires current-head CI/review before merge; the passing CI result above belongs to the exact earlier code head and is not misrepresented as a future-head result. diff --git a/.agents/notes/proposed/process/2026-10-27-dead-code-cleanup.md b/.agents/notes/proposed/process/2026-10-27-dead-code-cleanup.md new file mode 100644 index 00000000..af83593e --- /dev/null +++ b/.agents/notes/proposed/process/2026-10-27-dead-code-cleanup.md @@ -0,0 +1,48 @@ +# Decision: Remove verified unused code and record candidate dispositions + +Status: proposed + +## Problem + +The private _a alias-construction wrapper has no repository caller. The older audit example-import findings are already resolved on main. + +Issue: https://github.com/Alibaba-Agent-Sandbox/python-sdk/issues/146. + +## Decision + +Remove only the unused private configuration wrapper. Preserve Alias, public exports, schema generation, package versions, callback signatures and compatibility arguments. + +## API Design + +No supported command, option, public export or runtime protocol is removed. This is scoped maintenance; scanner reachability is not an external-usage proof. + +## Alternatives considered + +- Delete every scanner finding: rejected because callbacks, dynamic registration, public exports and side-effectful expressions need semantic review. +- Patch the old workspace checkout: rejected because accepted main already resolves some findings and unrelated local changes must remain intact. +- Keep all unused private bindings: retains unnecessary code and, for TypeScript, lets new unused declarations accumulate. + +## Dependencies + +No runtime dependency or version changes. +Each owning repository has its own Issue, branch, record and validation. No package release or manual deployment is part of this task. + +## Test Strategy + +Run both packages through required lint, formatting, type, test and distribution checks; existing real-component acceptance and current-head CI remain required. +Initial failures, missing prerequisites and later reruns remain separate evidence. Existing behavior tests are reused for deletion-only cleanup; no tests merely assert that source text disappeared. + +## Acceptance criteria + +- Verified unused bindings are removed while initialization and observable behavior remain. +- Public interfaces, attribution, generated-source ownership and historical evidence are preserved. +- The candidate review below distinguishes removed, upstream-resolved and retained cases. +- Applicable verification and current-head remote CI/review gates pass before merge. + +## Consequences + +StreamReader.buffer_size explicitly preserves its signature contract. Unreachable yield statements in test async generators determine their function kind and must remain. _cleanup_browser is definition-only in the inspected server tree, but resource teardown ownership needs a lifecycle-specific decision and tests; this patch does not remove it or claim a resource leak. No server runtime implementation is changed. + +## Evidence + +- [Verification](../../../evidence/verify/2026-10-27-dead-code-cleanup.md) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bd127a7..d9c229b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Changed +- Remove an uncalled private configuration alias helper while preserving public configuration schema exports and compatibility signatures. + - Normalize Python MCP background-task `sandbox_id` handles to the actual `provider:id`. Return the opaque handle unchanged when querying/cancelling; callers parsing a raw ID must update. Legacy raw IDs still resolve within a unique lifecycle cache or the SDK's default-provider scope. / Python MCP 后台任务的 `sandbox_id` 句柄规范为实际 `provider:id`；查询和取消须原样回传，拆分原始 ID 的调用者需要更新。历史原始 ID 仍可在无歧义的生命周期缓存或 SDK 默认提供方范围内解析。 - Add an optional real desktop acceptance runner with isolated pinned dependencies, private ingress, screenshot/input, user handover and replay documentation. / 新增可选真实桌面验收运行器、隔离固定依赖及私有入口、画面输入、用户接管和回放说明。 diff --git a/client-sdk/src/agent_sandbox/utils/config_keys.py b/client-sdk/src/agent_sandbox/utils/config_keys.py index 80a4e404..b817f15b 100644 --- a/client-sdk/src/agent_sandbox/utils/config_keys.py +++ b/client-sdk/src/agent_sandbox/utils/config_keys.py @@ -133,10 +133,6 @@ def validate_provider_id(provider_id: str) -> str | None: return None -def _a(name: str, kind: str, surface: str = "env") -> Alias: - return Alias(name, kind, surface) - - _G_CONN = "Connection" _G_AUTH = "Sandbox authentication" _G_ALI = "Alibaba Cloud credentials" diff --git a/docs/en/design/architecture.md b/docs/en/design/architecture.md index cfbf4ce1..d3840405 100644 --- a/docs/en/design/architecture.md +++ b/docs/en/design/architecture.md @@ -272,3 +272,8 @@ The CLI (`abx`) formerly lived in `src/agent_sandbox/cli/` (Click commands under Provider configuration discovery is assembled lazily by the package composition root. Transport consumes a low-level `ProviderSchema` value and an injected resolver from `_provider_schema.py`; it does not import the provider registry. Builtin configuration resolves without provider discovery. Registered, entry-point and installed providers retain the registry selection and derive/default/inheritance rules. Builtin platform/envd HTTP uses one transport attempt; see [configuration](../reference/configuration.md#transport-attempts). Engine read recovery remains separate. + + +## Unused-code review + +Private configuration helpers may be removed after source and test consumer review; public schema exports stay stable. Compatibility parameters, framework callbacks and async-generator marker statements need semantic review instead of automatic deletion. Browser resource teardown is a separate lifecycle concern and must be verified before changing its ownership. diff --git a/docs/zh/design/architecture.md b/docs/zh/design/architecture.md index c4dac3f2..fa87d730 100644 --- a/docs/zh/design/architecture.md +++ b/docs/zh/design/architecture.md @@ -270,3 +270,8 @@ CLI（`abx`）原先位于 `src/agent_sandbox/cli/`（`commands/` 下的 Click 提供方配置发现由包入口延迟装配。传输配置只消费 `_provider_schema.py` 中的低层 `ProviderSchema` 值与注入的解析器，不导入提供方 registry。内置配置不触发提供方发现；注册、entry-point 和已安装提供方仍遵循原有选择、派生、默认值及继承规则。 内置平台/envd HTTP 每次执行一次传输尝试，见[配置说明](../reference/configuration.md#传输尝试)；引擎读取恢复另行处理。 + + +## 未使用代码审查 + +私有配置 helper 可在核对源码和测试消费者后清理，公共配置 schema 导出保持稳定。兼容参数、框架回调以及声明异步生成器的语句需要语义复核，不能自动删除。浏览器资源释放属于独立的生命周期问题，变更其归属前须完成相应验证。