Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .agents/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
40 changes: 40 additions & 0 deletions .agents/evidence/verify/2026-10-27-dead-code-cleanup.md
Original file line number Diff line number Diff line change
@@ -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.
48 changes: 48 additions & 0 deletions .agents/notes/proposed/process/2026-10-27-dead-code-cleanup.md
Original file line number Diff line number Diff line change
@@ -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)
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. / 新增可选真实桌面验收运行器、隔离固定依赖及私有入口、画面输入、用户接管和回放说明。
Expand Down
4 changes: 0 additions & 4 deletions client-sdk/src/agent_sandbox/utils/config_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 5 additions & 0 deletions docs/en/design/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
5 changes: 5 additions & 0 deletions docs/zh/design/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 导出保持稳定。兼容参数、框架回调以及声明异步生成器的语句需要语义复核,不能自动删除。浏览器资源释放属于独立的生命周期问题,变更其归属前须完成相应验证。
Loading