Skip to content

test: retire redundant smokes and prove CLI contracts - #5539

Open
songoow wants to merge 3 commits into
mainfrom
codex/smoke-test-audit-sweep
Open

songoow wants to merge 3 commits into
mainfrom
codex/smoke-test-audit-sweep

Conversation

@songoow

@songoow songoow commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

Goal and delivered outcome

Retire duplicated canary/DSH smoke entrypoints while preserving their behavioral owners and unique coverage. The new canary test crosses the actual CLI parser/dispatcher and reads two on-disk shard receipts. DSH-specific assertions move into the existing pytest owner; TraeX-specific process/file/permission/timeout checks remain.

This is a test/validation-documentation change: no production logic, workflow, permission contract, scoring, or real model/provider invocation changes.

Changes

  • Delete examples/canary/smoke-fleet-health-smoke.py; existing pytest retains cadence, complete/failed/invalid receipt, bounded-output and privacy assertions. Add all-pass/one-failed-shard CLI-directory cases.
  • Delete examples/dsh-turn-host-adapter-smoke.py; migrate structured signed authority, real host-result validator acceptance, fail-closed candidate handling and independent text limits into tests/test_dsh_goal_mode.py. Existing signed/unsigned/tamper and module/legacy subprocess tests plus two DSH e2e smokes retain the other assertions.
  • Remove only the three direct shared-action extraction cases from the TraeX smoke. Both adapters re-export the same host_candidate implementation. Keep TraeX result-file handling, permission forwarding, subprocess roundtrip and descendant timeout cleanup.
  • Update the two DSH validation documents to point at pytest.

Seven files, +135/-459. No new abstraction or compatibility branch.

Validation and evidence repair

Current tested head: 3f85c7f27b74d7d559c4238f9938a055be558823; integrated main: 99839aeb8fed5fae38a5d319391cd050672a6508.

This revision merges current main without expanding the cleanup. Its seven-path PR diff is identical to the prior head's diff against its original base. Runtime behavior and retained coverage are unchanged. The earlier approval applies to 10e5c18a, not automatically to this new head.

Validation Result Boundary
Source-checkout pytest: tests/canary/test_smoke_fleet_health.py tests/test_dsh_goal_mode.py 85 passed Real CLI shard-directory read, adapter contract and real result validator
TraeX adapter smoke; generic DSH e2e; built-in DSH e2e All passed; TraeX 9 checks Synthetic/fake runners, real process/writeback/quota/replay paths; no real provider qualification
Prior-head independent mutations: first receipt shard only; loosened classification limit; dropped required_reads Each caught by retained tests; not rerun in this integration Retained tests and changed implementation boundaries are unchanged
Risk-based canary premerge over all seven changed paths 18/18 passed; four direct checks passed Catalog/canary, semantic policy, compile/diff and public-private boundary
First canary attempt 17/18; missing TypeScript dev dependency Preserved setup failure; npm ci --ignore-scripts, targeted semantic rerun and full premerge rerun passed; no thresholds changed
Required GitHub CI Prior-head failures attributed below; new-head result remains separate Old approval/CI evidence is not new-head merge readiness

Required CI attribution

The prior-head review held approval for missing baseline attribution. That evidence gap was repaired without weakening checks. Preserve that historical attribution; any different failure on the new head must be assessed separately:

  • Original immutable base 5d790b49dd723c1f366d69ac7c8ce35beafb155d, Python Tests run 37137303204, and PR run 37138103593: four JUnit artifacts per run were compared. 71 testcase identities and complete failure messages are exactly equal, not merely equal failure counts.
  • The remaining test_unaccepted_request_recovers_only_after_owned_retirement[lifetime] timeout has the same testcase and full error message on independent main 99839aeb8fed5fae38a5d319391cd050672a6508, run 37139797259, which does not include this PR. Its delegation fixture, preview test/transport/bridge, MCP/runtime and Python Tests workflow are unchanged from the original base. The intervening main change is an unrelated research capability.
  • All three runs use the same four-shard pytest command/configuration. PR CI merge tree 9454c1160f84145baffadfd3765e4860ad0d14ed has the original base and reviewed head as its parents. All 72 head failures therefore have independent unchanged control signatures; failed pytest/merge-gate checks are aggregators of those shard failures.

These are existing main/CI-owner failures. They remain visible and must be resolved before required merge gates pass. No unrelated runtime fix, timeout increase, assertion removal or CI bypass is bundled here.

Acceptance and scope

The fixed-version review basis is What Counts As A Good Smoke, especially real-boundary evidence, semantic pressure and consolidation without lost coverage.

Frontend/Lark impact: none; the shipped CLI behavior is unchanged. The previous approval remains recorded on its original head; current-head review and CI are still required. No review dismissal or merge is performed. All three PR commits have DCO sign-offs.

The bounded refactor pass found no further shared rule to extract: the existing shared candidate owner and pytest owners already carry the removed smokes' semantics. Additional fixture/runtime cleanup would expand this PR's reason for change and is handled independently in #5535.

songoow and others added 2 commits October 4, 2026 00:41
…moke

examples/canary/smoke-fleet-health-smoke.py called build_smoke_fleet_health()
directly, like the unit tests, so it never exercised the CLI path the
full-public workflow depends on. Its own assertions were also self-derived
(script uniqueness and cadence totals read from the same inventory), and the
unit tests already cover every receipt rule it checked.

Add a pytest case that runs `--format json canary smoke-health --receipt DIR`
through the real CLI with a two-shard receipt directory, matching the
workflow step, for both an all-pass and a one-failure shard set. Mutating the
directory reader to read one shard, or dropping --receipt in the CLI dispatch,
fails the new test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: song <liusongstep@gmail.com>
Both turn-host adapters delegate signing, prompt rendering, parsing and
result shaping to loopx/control_plane/turn_driver/host_candidate.py, and
tests/test_dsh_goal_mode.py already covered most cases of
examples/dsh-turn-host-adapter-smoke.py against the same objects.

Move the assertions only the smoke held into pytest: real host-result
validator acceptance for material, wait, missing-block and unsupported-kind
results; bounded free-text fields against independent contract limits; and
structured signed authority in the rendered prompt. Then delete the smoke.
Signature tampering stays covered by tests/test_turn_envelope.py and the
legacy script subprocess path by test_legacy_launcher_runs_the_same_contract.

The TraeX smoke stays because no pytest covers its structured-result file,
permission forwarding or descendant timeout cleanup; only its three cases
that call the shared extract_action_text object directly are removed.

Mutations that loosen a text limit, drop required_reads from the authority,
or rewrite the result turn_key each fail the migrated tests.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: song <liusongstep@gmail.com>
@songoow
songoow requested a review from huangruiteng as a code owner October 3, 2026 16:46

@songoow songoow left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent — gpt-6.1-sol (OpenAI); runtime_reported; reasoning_effort=medium

Request changes conclusion (author-owned PR; GitHub blocks formal self-review)

Reviewed head: 10e5c18a2328f1a29ee73ea36d2cf190b0b07918

动机

维护 smoke 测试的人需要删除重复检查时保留能抓住真实回归的覆盖。

旧 smoke 与 pytest 重复,且 smoke-health 没走实际 CLI。新测试读取两份分片文件并检查 ready、失败传播和输出隐私;独有 DSH 断言迁入已有测试文件。

受影响 pytest 85 个用例通过,TraeX smoke 和两条 DSH 端到端 smoke 通过;三种故障注入均被保留测试抓住。

本 PR 不改变生产行为,不调用真实模型,也不声称这次合并已经满足整个 CI 门禁。

改动思路

把共享规则交给已有 pytest owner,adapter 专属端到端链条继续保留。相比只删除文件,新增 CLI 文件读取与故障敏感性覆盖让这批清理具有独立价值。

具体改动

删除 canary fleet-health smoke 和 DSH adapter smoke;TraeX 仅删除共享签名抽取的三个重复用例,保留文件结果、权限和超时子进程清理。pytest 补充 CLI 双分片读取、ready/隐私断言,以及 DSH 结构化 authority、真实 validator、fail-closed wait 与长度限制;两份说明文档改指 pytest。DSH 防篡改测试仍由 test_run_dsh_host_rejects_untyped_requests_as_contract_rejected 覆盖,共享函数是直接 re-export。

对主干的风险

本地 85 个 pytest 用例通过;TraeX smoke 9 项、generic DSH e2e、built-in DSH e2e 均通过。三种独立故障注入(目录只读第一份分片、放宽 classification 长度、丢弃 required_reads)均令保留测试按预期失败。端到端使用 synthetic/fake runner,未验证真实模型服务。

[P1] 验证归因缺口: 两个 PR 都有四个 test-shard、pytest 与 merge-gate 失败。读取日志后,可见 architecture inventory/budget、monitor settlement 与 collaboration authority 等失败;变更文件未出现在这些 CI 失败摘要中,但尚未完成全部失败在不可变 base/head 上的同命令、同签名归因。不能据此宣称是本 PR 回归,也不能将这些红灯当作已证实无关。所需门禁由 .github/workflows/python-tests.yml 的 pytest/merge-gate 聚合定义。

最小补证:同步当前 main 后重新跑 required CI;若仍失败,逐项给出不可变 base 与 reviewed head 的相同命令和失败签名,确认改动未触及因果路径。不要降低门禁或用本地 focused tests 代替。

我的整体评价

实现方向合理,当前 diff 未发现可确认的新增生产缺陷;维护价值已由实际路径和回归检测验证。但 required CI 的归因缺口仍阻止 APPROVE,本轮为 REQUEST_CHANGES(验证证据待补)。这不要求为了消除红灯而把无关修复混入本 PR。future-facing pass:复用现有 helper/pytest owner 已足够,未发现值得在本 PR 增加的额外抽象。没有 frontend/Lark 配置或交互改动;CLI 只是被测试,生产入口未变。

English verdict: REQUEST_CHANGES - head 10e5c18; the maintenance approach is sound, but required CI failures remain incompletely attributed against an immutable baseline. 85 pytest passed; traex smoke: 9 checks; generic DSH e2e passed; built-in DSH e2e passed; mutations receipt_first/length/authority all failed in intended retained tests.

@songoow songoow left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent — gpt-6.1-sol (OpenAI); runtime_reported; reasoning_effort=medium

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

Reviewed head: 10e5c18a2328f1a29ee73ea36d2cf190b0b07918. 本次按新的基线证据重新审查完整 PR;代码提交未改变,上轮结论未被直接继承。

动机

维护 smoke 测试的人需要删除重复检查时保留能抓住真实回归的覆盖。

旧 smoke 与 pytest 重复,且 smoke-health 没走实际 CLI。新测试读取两份分片文件并检查 ready、失败传播和输出隐私;独有 DSH 断言迁入已有测试文件。

受影响测试 85/85 通过,三条相关 smoke 通过,三种故障注入均被保留测试抓住;上轮 required CI 归因缺口已经补齐。

本 PR 不改变生产行为,不调用真实模型,也不声称这次合并已经满足整个 CI 门禁。

独立审阅依据为 docs/development/good-smokes.md,固定版本 5d790b49dd723c1f366d69ac7c8ce35beafb155d;当前项目仍使用同一维护与验证合同。

改动思路

采用现有 pytest owner 承接共享规则,保留无法由纯规则测试替代的 TraeX 与 DSH 端到端路径。相比维持两个相似检查入口,这个方案减少重复维护;相比直接删脚本,它先迁移独有断言并增加真实 CLI 文件读取覆盖,没有添加新生产抽象。

合同逐项核对:

  • Start With The Contract / 从合同开始:smoke-health 测试经过真实 parser/dispatcher,读取两份磁盘分片;没有把内部 helper 成功冒充 CLI 证明。
  • Prefer Semantic Pressure / 优先验证语义压力:失败分片使 ready=false;缺失/不支持的候选回退 wait;三种故障注入均被保留测试抓住。
  • Consolidate Without Losing Coverage / 合并时不丢失覆盖:已逐项读取旧脚本并找到 keeper,核对共享实现、剩余专属路径、引用、fleet/catalog 与公开边界检查。

具体改动

七个文件:删除 fleet-health 和 DSH adapter 两个 smoke;只从 TraeX smoke 删除三个直接调用共享 action 抽取函数的用例;在两个已有 pytest 文件补充覆盖;两个 DSH 验证说明改指 pytest。净改动 +135/-459,无生产逻辑、配置、CI workflow 或权限合同变更。

关键代码讲解

  • _run_smoke_health_cli 和 test_cli_merges_a_shard_directory_into_the_workflow_ready_gate:真实 cli.main 读取两个分片,检查覆盖/readiness/输出隐私,同时覆盖一个分片失败。旧 fleet-health 的 cadence、完整 receipt、失败/非法 receipt、大小与隐私断言仍在已有 pytest。
  • test_prompt_carries_the_signed_authority_without_prose_reconstruction:保留 primary_action、required_reads、write_scope、workspace_guard 和提示词结构;unsigned 与 action tamper 的拒绝仍由已有测试保护。
  • test_build_result_is_accepted_by_the_real_host_result_validator、test_build_result_bounds_every_free_text_field:覆盖 material/wait/missing/unsupported 的真实 validator 接受性和独立长度上限。旧 DSH subprocess roundtrip 仍由 module/legacy launcher pytest 和两个 e2e smoke 保护。
  • TraeX 的 extract_action_text 与 DSH 都直接 re-export 同一个 host_candidate 实现;TraeX 专属结构化结果文件、权限转发、真实子进程 roundtrip 和 descendant timeout cleanup 继续保留。

对主干的风险

本轮独立工作区、Node 22.22.3、source-checkout 验证:两个受影响 pytest 文件 85/85 通过;TraeX smoke 9 项通过;generic DSH 与 built-in DSH 两条 e2e 通过(含 writeback、quota 一次计费、无副作用 replay、容量失败重试)。三种 mutation:只读第一份 receipt、放宽 classification 上限、丢掉 required_reads,均导致 keeper 失败。最终 canary premerge 18/18 通过,四个 direct checks 通过,公开边界扫描干净。首次 17/18 的缺失 TypeScript 依赖失败已保留;按仓库提示执行 npm ci --ignore-scripts 后重跑通过。未运行真实模型或 SDK provider。

上轮 CI 归因缺口已经补齐,依据不是“相同失败数”:

  1. 原始不可变 base 5d790b49dd723c1f366d69ac7c8ce35beafb155d 的 Python Tests / 37137303204,四份 JUnit 中 71 个 testcase 身份与完整错误消息和 PR CI / 37138103593 完全一致。
  2. 剩余 test_unaccepted_request_recovers_only_after_owned_retirement[lifetime] 的 5 秒 timeout,在不含本 PR 的独立 main 99839aeb8fed5fae38a5d319391cd050672a6508 的 Python Tests / 37139797259 也有完全相同的 testcase 和错误消息。原始 base → 此控制版本的 delegation fixture、preview test/transport/bridge、runtime、MCP 和 Python Tests workflow 无 diff;只增加了无关研究能力。未修改 timeout,也未将失败归为本 PR 新回归。
  3. 三次 CI 使用相同四分片 pytest 命令/配置;PR CI 实际 merge tree 9454c1160f84145baffadfd3765e4860ad0d14ed 的两个父提交已核对为原始 base 与 reviewed head。所有 72 个 head 失败逐项有独立控制签名;pytest 和 merge-gate 是这些失败的聚合,仍为红灯。

因此这些红灯属于已独立归因的主干既有失败,代码评审可通过,合并门禁仍不通过。没有删除失败断言、放宽预算、改 timeout 或混入无关 runtime 修复。保留原失败与控制证据。

我的整体评价

APPROVE:本批独立维护目标已经满足,共享与 adapter 专属覆盖均有实际路径及回归敏感性证据,删除不是仅依据名字/行数相似。上轮阻塞通过补齐逐项基线证据解除;PR 说明已如实披露 CI 仍红,原讨论保留。

future-facing pass:现有 pytest owner、共享 host_candidate 和保留 e2e 已形成足够窄的维护边界,进一步拆 helper 会增加结构而没有实际收益。无 frontend/Lark 生产交互变化;真实 CLI 只是新增验证对象。required CI 故障仍由现有主干/CI owner 处理,本结论不授予合并权限。

English verdict: APPROVE - head 10e5c18; full smoke-to-keeper review and fresh positive/negative/mutation checks support the consolidation. All 72 CI failures are independently matched to unchanged controls (71 exact original-base signatures, one exact independent-main timeout); required CI and merge readiness remain on hold. 85 pytest cases and three related smokes passed; no real model/provider invocation.

…-20261004

Signed-off-by: song <liusongstep@gmail.com>
@songoow

songoow commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

Published latest-main integration at 3f85c7f27b74d7d559c4238f9938a055be558823.

The seven-path cleanup diff is byte-for-byte unchanged from the previous head against its original base: no extra production fix or speculative abstraction was added. The body now distinguishes current verification from prior-head approval, mutation evidence and CI attribution.

On the new head: 85 pytest cases, 9 TraeX checks, generic and built-in DSH end-to-end smokes passed. Standard premerge passed all 18 selected checks plus 4 direct checks after installing the missing TypeScript development dependency; the initial environment failure is disclosed. These use synthetic/fake hosts, not real model-provider qualification.

The earlier review remains attached to its original head. Old CI failure attribution is historical evidence, not proof that any new failure is harmless. Current-head CI/review still apply. No review dismissal or merge.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant