Skip to content

fix(cosh-ng): [platform,cli] honor svc dry-run#1426

Open
samchu-zsl wants to merge 2 commits into
alibaba:mainfrom
samchu-zsl:fix/cosh-ng/svc-dry-run
Open

fix(cosh-ng): [platform,cli] honor svc dry-run#1426
samchu-zsl wants to merge 2 commits into
alibaba:mainfrom
samchu-zsl:fix/cosh-ng/svc-dry-run

Conversation

@samchu-zsl

@samchu-zsl samchu-zsl commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Description

Make service action dry-runs return before querying systemd, matching the package dry-run contract and keeping previews independent of service existence. Preserve action and service-name validation, all real service execution paths, and the existing public response types. Strengthen platform and CLI regression coverage across start, stop, restart, enable, and disable. A second atomic commit removes an unchanged-main redundant formatting borrow exposed by Rust 1.97 so the required cosh-ng CI gate can proceed without changing runtime behavior.

Related Issue

closes #1361

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional change)
  • Performance improvement
  • CI/CD or build changes

Scope

  • cosh (copilot-shell)
  • cosh-ng (cosh-ng)
  • sec-core (agent-sec-core)
  • skill (os-skills)
  • sight (agentsight)
  • tokenless (tokenless)
  • ckpt (ws-ckpt)
  • memory (agent-memory)
  • anolisa (anolisa-cli)
  • skillfs (SkillFS)
  • Multiple / Project-wide

Checklist

  • I have read the Contributing Guide
  • My code follows the project's code style
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the documentation accordingly
  • For cosh: Lint passes, type check passes, and tests pass
  • For cosh-ng: cargo clippy --workspace --all-targets -- -D warnings and cargo fmt --all -- --check pass
  • For sec-core (Rust): cargo clippy -- -D warnings and cargo fmt --check pass
  • For sec-core (Python): Ruff format and pytest pass
  • For skill: Skill directory structure is valid and shell scripts pass syntax check
  • For sight: cargo clippy -- -D warnings and cargo fmt --check pass
  • For tokenless: cargo clippy -- -D warnings and cargo fmt --check pass
  • For memory (Linux only): cargo clippy --all-targets --locked -- -D warnings, cargo fmt --check, and cargo test pass
  • For anolisa: cargo clippy --all-targets --locked -- -D warnings, cargo fmt --all --check, and cargo test --locked pass
  • For skillfs: cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, and cargo test --workspace pass
  • Lock files are up to date (package-lock.json / Cargo.lock)

Testing

Passed after rebasing onto origin/main:

  • cargo fmt --all -- --check
  • cargo test --package cosh-core (275 passed; 0 failed across unit and integration targets)
  • cargo test --package cosh-platform svc::tests::test_svc_action_dry_run_skips_status_query_for_all_actions -- --exact (1 passed)
  • cargo test --package cosh-cli --test cli_integration test_svc_actions_dry_run_nonexistent_service_succeed -- --exact (1 passed)
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo build --workspace --release
  • CI-equivalent commitlint 19.x for both commits (0 problems; 0 warnings)
  • GitHub Actions Test cosh-ng: Rust 1.97 formatting, all-targets Clippy, and cargo test --workspace passed

Earlier focused and manual validation also passed:

  • cargo test --package cosh-platform -- --skip test_parse_installed_version_bash (175 passed; 0 failed; 1 filtered out)
  • cargo test --package cosh-cli --test cli_integration -- --skip test_pkg_search_bash_shows_installed (53 passed; 0 failed; 1 filtered out)
  • cargo run --quiet --package cosh-cli -- svc start cosh-nonexistent-test-svc-1361 --dry-run
  • cargo run --quiet --package cosh-cli -- pkg install cosh-nonexistent-test-pkg-1361 --dry-run

The local toolchain is Rust/Clippy 1.91. The previous Linux CI RED on Rust 1.97 was clippy::useless_borrows_in_formatting in unchanged main code; the second commit applies Clippy's exact one-line suggestion. The refreshed Linux Test cosh-ng job passed in 8m51s, providing the final Rust 1.97 and complete workspace gate.

The unskipped package baseline and broader workspace checks are not green on this macOS host. Two pre-existing Nix/Homebrew package-manager baseline tests fail: pkg::tests::test_parse_installed_version_bash and test_pkg_search_bash_shows_installed. With both package tests skipped, two broad workspace runs exposed seven unrelated cosh-shell PTY timing failures; every failure passed an isolated exact rerun. No package-manager or cosh-shell code was changed for this fix.

Additional Notes

The manual dry-run comparison was performed on macOS, where the regression previously surfaced as a failed systemctl spawn. The automated test uses a guaranteed nonexistent service and covers all five service actions without executing host mutations. Both dry-run state fields use the existing Unknown("(dry-run)") representation, avoiding a public API change. The Rust 1.97 cleanup is isolated in its own [core] commit and preserves the generated bypass id and later uses of tc.id.

Return the preview result before querying systemd so dry-run stays independent
of service existence and host capabilities.

Preserve validation and real execution paths. Reuse the existing Unknown state
to avoid a public API change.

Assisted-by: Codex:0.144.0-alpha.4
Signed-off-by: Shenglong Zhu <samchu.zsl@alibaba-inc.com>
Rust 1.97 rejects the explicit formatting borrow under the workspace warnings gate.
Rely on format_args to borrow tc.id implicitly, preserving output and ownership.

Assisted-by: Codex:0.144.0-alpha.4
Signed-off-by: Shenglong Zhu <samchu.zsl@alibaba-inc.com>
@samchu-zsl samchu-zsl force-pushed the fix/cosh-ng/svc-dry-run branch from 651217e to f49cfc5 Compare July 10, 2026 06:05
@samchu-zsl samchu-zsl marked this pull request as ready for review July 10, 2026 06:15
@SunnyQjm

SunnyQjm commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

PR number: #1426
head_sha: f49cfc5
reviewed_at: 2026-07-10T06:50:33Z

Findings

  • 未发现 blocking package/module/public API 组织问题。
  • 未发现需要 blocking 的 dry-run 行为问题。平台层 src/cosh-ng/crates/cosh-platform/src/svc.rs:102 在 action whitelist 后、systemd status 查询前返回 dry-run preview;CLI 层仍在 src/cosh-ng/crates/cosh-cli/src/cmd/svc.rs:77:86:95:104:113 对各 service action 先执行 validate_svc_name;新增 CLI regression src/cosh-ng/crates/cosh-cli/tests/cli_integration.rs:907 覆盖 start/stop/restart/enable/disable 对不存在服务的 dry-run 成功路径。

Structure / API review

  • 改动只落在 cosh-platformcosh-cli 测试和一个独立 cosh-core Rust 1.97 clippy cleanup;没有新增 crate、workspace dependency、lockfile 或 public response type。
  • 未触碰 cosh-shell,因此本地 specs/cosh-ng-code-organization/README.mdstandard.mdreview.mdvalidation.md 中针对 cosh-shell root/module/public API 的 blocking 项未被触发。
  • cosh-coreformat!("{}-bypass", tc.id) 变更是独立 commit,未混入 service dry-run 行为修改。

Remaining risk / validation not run

  • 我没有在本地重跑 cargo 测试;本轮只做监控评审,验证依据是 PR diff、GitHub reported checks 和作者列出的 focused/CI evidence。
  • cosh_platform::svc::svc_action 作为平台层函数本身仍依赖 caller 先做 service-name validation;当前 CLI 路径满足这一点,但未来新增直接 caller 时应继续显式调用 validate_svc_name

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[cosh-ng] svc --dry-run 对不存在的服务返回失败,与 pkg --dry-run 语义不一致

2 participants